minijail: Refactor dynamic and static code paths.
This CL uses the same code path for both dynamic and static binaries.
This way we avoid duplicating code, or forgetting to add functionality
to either of the paths.
BUG=chromium:537667
TEST=security_Minijail0 passes.
Change-Id: Ia484180a041dad3c302c3c8ce8bfd5b41d758ccb
Reviewed-on: https://chromium-review.googlesource.com/303380
Commit-Ready: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
diff --git a/libminijail.c b/libminijail.c
index 8a09cad..b934c34 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -18,6 +18,7 @@
#include <sched.h>
#include <signal.h>
#include <stdarg.h>
+#include <stdbool.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
@@ -549,7 +550,7 @@
{
size_t len = strnlen(*buf, *buflength);
if (len == *buflength)
- /* There's no null-terminator */
+ /* There's no null-terminator. */
return NULL;
return consumebytes(len + 1, buf, buflength);
}
@@ -1077,7 +1078,7 @@
int r;
if (sizeof(sz) != bytes)
return -EINVAL;
- if (sz > USHRT_MAX) /* Arbitrary sanity check */
+ if (sz > USHRT_MAX) /* arbitrary sanity check */
return -E2BIG;
buf = malloc(sz);
if (!buf)
@@ -1137,7 +1138,7 @@
sprintf(newenv, "%s%s%s", oldenv, strlen(oldenv) ? " " : "",
PRELOADPATH);
- /* setenv() makes a copy of the string we give it */
+ /* setenv() makes a copy of the string we give it. */
setenv(kLdPreloadEnvVar, newenv, 1);
free(newenv);
return 0;
@@ -1176,39 +1177,60 @@
return dup2(fds[index], fd);
}
+int minijail_run_internal(struct minijail *j, const char *filename,
+ char *const argv[], pid_t *pchild_pid,
+ int *pstdin_fd, int *pstdout_fd, int *pstderr_fd,
+ int use_preload);
+
int API minijail_run(struct minijail *j, const char *filename,
char *const argv[])
{
- return minijail_run_pid_pipes(j, filename, argv,
- NULL, NULL, NULL, NULL);
+ return minijail_run_internal(j, filename, argv, NULL, NULL, NULL, NULL,
+ true);
}
int API minijail_run_pid(struct minijail *j, const char *filename,
char *const argv[], pid_t *pchild_pid)
{
- return minijail_run_pid_pipes(j, filename, argv, pchild_pid,
- NULL, NULL, NULL);
+ return minijail_run_internal(j, filename, argv, pchild_pid,
+ NULL, NULL, NULL, true);
}
int API minijail_run_pipe(struct minijail *j, const char *filename,
char *const argv[], int *pstdin_fd)
{
- return minijail_run_pid_pipes(j, filename, argv, NULL, pstdin_fd,
- NULL, NULL);
+ return minijail_run_internal(j, filename, argv, NULL, pstdin_fd,
+ NULL, NULL, true);
}
int API minijail_run_pid_pipe(struct minijail *j, const char *filename,
char *const argv[], pid_t *pchild_pid,
int *pstdin_fd)
{
- return minijail_run_pid_pipes(j, filename, argv, pchild_pid, pstdin_fd,
- NULL, NULL);
+ return minijail_run_internal(j, filename, argv, pchild_pid, pstdin_fd,
+ NULL, NULL, true);
}
int API minijail_run_pid_pipes(struct minijail *j, const char *filename,
char *const argv[], pid_t *pchild_pid,
int *pstdin_fd, int *pstdout_fd, int *pstderr_fd)
{
+ return minijail_run_internal(j, filename, argv, pchild_pid,
+ pstdin_fd, pstdout_fd, pstderr_fd, true);
+}
+
+int API minijail_run_no_preload(struct minijail *j, const char *filename,
+ char *const argv[])
+{
+ return minijail_run_internal(j, filename, argv, NULL, NULL, NULL, NULL,
+ false);
+}
+
+int minijail_run_internal(struct minijail *j, const char *filename,
+ char *const argv[], pid_t *pchild_pid,
+ int *pstdin_fd, int *pstdout_fd, int *pstderr_fd,
+ int use_preload)
+{
char *oldenv, *oldenv_copy = NULL;
pid_t child_pid;
int pipe_fds[2];
@@ -1221,15 +1243,23 @@
int pid_namespace = j->flags.pids;
int do_init = j->flags.do_init;
- oldenv = getenv(kLdPreloadEnvVar);
- if (oldenv) {
- oldenv_copy = strdup(oldenv);
- if (!oldenv_copy)
- return -ENOMEM;
+ if (use_preload) {
+ oldenv = getenv(kLdPreloadEnvVar);
+ if (oldenv) {
+ oldenv_copy = strdup(oldenv);
+ if (!oldenv_copy)
+ return -ENOMEM;
+ }
+
+ if (setup_preload())
+ return -EFAULT;
}
- if (setup_preload())
- return -EFAULT;
+ if (!use_preload) {
+ if (j->flags.caps)
+ die("Capabilities are not supported without "
+ "LD_PRELOAD");
+ }
/*
* Make the process group ID of this process equal to its PID, so that
@@ -1244,12 +1274,14 @@
}
}
- /*
- * Before we fork(2) and execve(2) the child process, we need to open
- * a pipe(2) to send the minijail configuration over.
- */
- if (setup_pipe(pipe_fds))
- return -EFAULT;
+ if (use_preload) {
+ /*
+ * Before we fork(2) and execve(2) the child process, we need
+ * to open a pipe(2) to send the minijail configuration over.
+ */
+ if (setup_pipe(pipe_fds))
+ return -EFAULT;
+ }
/*
* If we want to write to the child process' standard input,
@@ -1332,24 +1364,28 @@
if (j->flags.userns)
clone_flags |= CLONE_NEWUSER;
child_pid = syscall(SYS_clone, clone_flags, NULL);
- }
- else
+ } else {
child_pid = fork();
+ }
if (child_pid < 0) {
- free(oldenv_copy);
+ if (use_preload) {
+ free(oldenv_copy);
+ }
die("failed to fork child");
}
if (child_pid) {
- /* Restore parent's LD_PRELOAD. */
- if (oldenv_copy) {
- setenv(kLdPreloadEnvVar, oldenv_copy, 1);
- free(oldenv_copy);
- } else {
- unsetenv(kLdPreloadEnvVar);
+ if (use_preload) {
+ /* Restore parent's LD_PRELOAD. */
+ if (oldenv_copy) {
+ setenv(kLdPreloadEnvVar, oldenv_copy, 1);
+ free(oldenv_copy);
+ } else {
+ unsetenv(kLdPreloadEnvVar);
+ }
+ unsetenv(kFdEnvVar);
}
- unsetenv(kFdEnvVar);
j->initpid = child_pid;
@@ -1359,13 +1395,15 @@
if (j->flags.userns)
write_ugid_mappings(j, userns_pipe_fds);
- /* Send marshalled minijail. */
- close(pipe_fds[0]); /* read endpoint */
- ret = minijail_to_fd(j, pipe_fds[1]);
- close(pipe_fds[1]); /* write endpoint */
- if (ret) {
- kill(j->initpid, SIGKILL);
- die("failed to send marshalled minijail");
+ if (use_preload) {
+ /* Send marshalled minijail. */
+ close(pipe_fds[0]); /* read endpoint */
+ ret = minijail_to_fd(j, pipe_fds[1]);
+ close(pipe_fds[1]); /* write endpoint */
+ if (ret) {
+ kill(j->initpid, SIGKILL);
+ die("failed to send marshalled minijail");
+ }
}
if (pchild_pid)
@@ -1377,7 +1415,7 @@
*/
if (pstdin_fd)
*pstdin_fd = setup_pipe_end(stdin_fds,
- 1 /* write end */);
+ 1 /* write end */);
/*
* If we want to read from the child process' standard output,
@@ -1385,7 +1423,7 @@
*/
if (pstdout_fd)
*pstdout_fd = setup_pipe_end(stdout_fds,
- 0 /* read end */);
+ 0 /* read end */);
/*
* If we want to read from the child process' standard error,
@@ -1393,13 +1431,12 @@
*/
if (pstderr_fd)
*pstderr_fd = setup_pipe_end(stderr_fds,
- 0 /* read end */);
+ 0 /* read end */);
return 0;
}
free(oldenv_copy);
-
if (j->flags.userns)
enter_user_namespace(j, userns_pipe_fds);
@@ -1437,16 +1474,20 @@
if (pid_namespace && !do_init)
j->flags.remount_proc_ro = 0;
- /* Strip out flags that cannot be inherited across execve. */
- minijail_preexec(j);
- /* Jail this process and its descendants... */
+ if (use_preload) {
+ /* Strip out flags that cannot be inherited across execve(2). */
+ minijail_preexec(j);
+ } else {
+ j->flags.pids = 0;
+ }
+ /* Jail this process, then execve() the target. */
minijail_enter(j);
if (pid_namespace && do_init) {
/*
* pid namespace: this process will become init inside the new
* namespace. We don't want all programs we might exec to have
- * to know how to be init. Normally |do_init == 1| we fork off
+ * to know how to be init. Normally (do_init == 1) we fork off
* a child to actually run the program. If |do_init == 0|, we
* let the program keep pid 1 and be init.
*
@@ -1461,7 +1502,7 @@
}
/*
- * If we aren't pid-namespaced, or jailed program asked to be init:
+ * If we aren't pid-namespaced, or the jailed program asked to be init:
* calling process
* -> execve()-ing process
* If we are:
@@ -1472,86 +1513,6 @@
_exit(execve(filename, argv, environ));
}
-int API minijail_run_static(struct minijail *j, const char *filename,
- char *const argv[])
-{
- pid_t child_pid;
- int userns_pipe_fds[2];
- int pid_namespace = j->flags.pids;
- int do_init = j->flags.do_init;
-
- if (j->flags.caps)
- die("caps not supported with static targets");
-
- /*
- * If we want to set up a new uid/gid mapping in the user namespace,
- * create the pipe(2) to sync between parent and child.
- */
- if (j->flags.userns) {
- if (pipe(userns_pipe_fds))
- return -EFAULT;
- }
-
- if (pid_namespace) {
- int clone_flags = CLONE_NEWPID | SIGCHLD;
- if (j->flags.userns)
- clone_flags |= CLONE_NEWUSER;
- child_pid = syscall(SYS_clone, clone_flags, NULL);
- }
- else
- child_pid = fork();
-
- if (child_pid < 0) {
- die("failed to fork child");
- }
- if (child_pid > 0 ) {
- j->initpid = child_pid;
-
- if (j->flags.pid_file)
- write_pid_file(j);
-
- if (j->flags.userns)
- write_ugid_mappings(j, userns_pipe_fds);
-
- return 0;
- }
-
- if (j->flags.userns)
- enter_user_namespace(j, userns_pipe_fds);
-
- /* If running an init program, let it decide when/how to mount /proc. */
- if (pid_namespace && !do_init)
- j->flags.remount_proc_ro = 0;
-
- /*
- * We can now drop this child into the sandbox
- * then execve the target.
- */
-
- j->flags.pids = 0;
- minijail_enter(j);
-
- if (pid_namespace && do_init) {
- /*
- * pid namespace: this process will become init inside the new
- * namespace. We don't want all programs we might exec to have
- * to know how to be init. Normally |do_init == 1| we fork off
- * a child to actually run the program. If |do_init == 0|, we
- * let the program keep pid 1 and be init.
- *
- * If we're multithreaded, we'll probably deadlock here. See
- * WARNING above.
- */
- child_pid = fork();
- if (child_pid < 0)
- _exit(child_pid);
- else if (child_pid > 0)
- init(child_pid); /* never returns */
- }
-
- _exit(execve(filename, argv, environ));
-}
-
int API minijail_kill(struct minijail *j)
{
int st;
diff --git a/libminijail.h b/libminijail.h
index 8b2ae0c..07f0762 100644
--- a/libminijail.h
+++ b/libminijail.h
@@ -51,7 +51,7 @@
void minijail_namespace_vfs(struct minijail *j);
void minijail_namespace_enter_vfs(struct minijail *j, const char *ns_path);
void minijail_namespace_net(struct minijail *j);
-/* Implies namespace_vfs and remount_readonly.
+/* Implies namespace_vfs and remount_proc_readonly.
* WARNING: this is NOT THREAD SAFE. See the block comment in </libminijail.c>.
*/
void minijail_namespace_pids(struct minijail *j);
@@ -103,32 +103,32 @@
*/
void minijail_enter(const struct minijail *j);
-/* Run the specified command in the given minijail, execve(3)-style. This is
+/* Run the specified command in the given minijail, execve(2)-style. This is
* required if minijail_namespace_pids() was used.
*/
int minijail_run(struct minijail *j, const char *filename,
char *const argv[]);
-/* Run the specified command in the given minijail, execve(3)-style.
- * Used with static binaries.
+/* Run the specified command in the given minijail, execve(2)-style.
+ * Used with static binaries, or on systems without support for LD_PRELOAD.
*/
-int minijail_run_static(struct minijail *j, const char *filename,
- char *const argv[]);
+int minijail_run_no_preload(struct minijail *j, const char *filename,
+ char *const argv[]);
-/* Run the specified command in the given minijail, execve(3)-style.
+/* Run the specified command in the given minijail, execve(2)-style.
* Update |*pchild_pid| with the pid of the child.
*/
int minijail_run_pid(struct minijail *j, const char *filename,
char *const argv[], pid_t *pchild_pid);
-/* Run the specified command in the given minijail, execve(3)-style.
+/* Run the specified command in the given minijail, execve(2)-style.
* Update |*pstdin_fd| with a fd that allows writing to the child's
* standard input.
*/
int minijail_run_pipe(struct minijail *j, const char *filename,
char *const argv[], int *pstdin_fd);
-/* Run the specified command in the given minijail, execve(3)-style.
+/* Run the specified command in the given minijail, execve(2)-style.
* Update |*pchild_pid| with the pid of the child.
* Update |*pstdin_fd| with a fd that allows writing to the child's
* standard input.
@@ -137,7 +137,7 @@
char *const argv[], pid_t *pchild_pid,
int *pstdin_fd);
-/* Run the specified command in the given minijail, execve(3)-style.
+/* Run the specified command in the given minijail, execve(2)-style.
* Update |*pchild_pid| with the pid of the child.
* Update |*pstdin_fd| with a fd that allows writing to the child's
* standard input.
diff --git a/minijail0.c b/minijail0.c
index 4c5f9d6..2ded926 100644
--- a/minijail0.c
+++ b/minijail0.c
@@ -144,7 +144,8 @@
const char *filter_path;
if (argc > 1 && argv[1][0] != '-')
return 1;
- while ((opt = getopt(argc, argv, "u:g:sS:c:C:P:b:V:f:m:M:vrGhHinpLetIU")) != -1) {
+ while ((opt = getopt(argc, argv,
+ "u:g:sS:c:C:P:b:V:f:m:M:vrGhHinpLetIU")) != -1) {
switch (opt) {
case 'u':
set_user(j, optarg);
@@ -309,8 +310,11 @@
/* Check if target is statically or dynamically linked. */
elftype = get_elf_linkage(argv[0]);
if (elftype == ELFSTATIC) {
- /* Target binary is static. */
- minijail_run_static(j, argv[0], argv);
+ /*
+ * Target binary is statically linked so we cannot use
+ * libminijailpreload.so.
+ */
+ minijail_run_no_preload(j, argv[0], argv);
} else if (elftype == ELFDYNAMIC) {
/*
* Target binary is dynamically linked so we can