From ce7b289b099746a98be4d57304fc130c14537411 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Wed, 20 Mar 2019 15:27:34 +0100 Subject: hardening: run more code unprivileged --- src/firejail/firejail.h | 1 + src/firejail/join.c | 77 ++++++++++++++++++++++--------------------------- src/firejail/sandbox.c | 52 ++++++++++++++++----------------- src/firejail/util.c | 10 +++++++ 4 files changed, 71 insertions(+), 69 deletions(-) diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index 01ddf2a14..5291361c8 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -521,6 +521,7 @@ void logsignal(int s); void logmsg(const char *msg); void logargs(int argc, char **argv) ; void logerr(const char *msg); +void set_nice(int inc); int copy_file(const char *srcname, const char *destname, uid_t uid, gid_t gid, mode_t mode); void copy_file_as_user(const char *srcname, const char *destname, uid_t uid, gid_t gid, mode_t mode); void copy_file_from_user_to_root(const char *srcname, const char *destname, uid_t uid, gid_t gid, mode_t mode); diff --git a/src/firejail/join.c b/src/firejail/join.c index 3372c8dc1..46dae0271 100644 --- a/src/firejail/join.c +++ b/src/firejail/join.c @@ -100,9 +100,6 @@ static void extract_command(int argc, char **argv, int index) { // build command build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, index); - - if (arg_debug) - printf("Extracted command #%s#\n", cfg.command_line); } static void extract_nogroups(pid_t pid) { @@ -290,11 +287,8 @@ pid_t switch_to_child(pid_t pid) { void join(pid_t pid, int argc, char **argv, int index) { EUID_ASSERT(); - char *homedir = cfg.homedir; - pid_t parent = pid; - - extract_command(argc, argv, index); + pid_t parent = pid; // in case the pid is that of a firejail process, use the pid of the first child process pid = switch_to_child(pid); @@ -374,19 +368,15 @@ void join(pid_t pid, int argc, char **argv, int index) { EUID_USER(); if (chdir("/") < 0) errExit("chdir"); - if (homedir) { + if (cfg.homedir) { struct stat s; - if (stat(homedir, &s) == 0) { + if (stat(cfg.homedir, &s) == 0) { /* coverity[toctou] */ - if (chdir(homedir) < 0) + if (chdir(cfg.homedir) < 0) errExit("chdir"); } } - // set cpu affinity - if (cfg.cpus) // not available for uid 0 - set_cpu_affinity(); - // set caps filter EUID_ROOT(); if (apply_caps == 1) // not available for uid 0 @@ -417,33 +407,6 @@ void join(pid_t pid, int argc, char **argv, int index) { } EUID_USER(); - // set nice - if (arg_nice) { - errno = 0; - int rv = nice(cfg.nice); - (void) rv; - if (errno) { - fwarning("cannot set nice value\n"); - errno = 0; - } - } - - // set environment, add x11 display - env_defaults(); - if (display) { - char *display_str; - if (asprintf(&display_str, ":%d", display) == -1) - errExit("asprintf"); - setenv("DISPLAY", display_str, 1); - free(display_str); - } - - if (cfg.command_line == NULL) { - assert(cfg.shell); - cfg.command_line = cfg.shell; - cfg.window_title = cfg.shell; - } - int cwd = 0; if (cfg.cwd) { if (chdir(cfg.cwd) == 0) @@ -463,8 +426,38 @@ void join(pid_t pid, int argc, char **argv, int index) { } } + // drop privileges drop_privs(arg_nogroups); - prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0); // kill the child in case the parent died + + // kill the child in case the parent died + prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0); + + extract_command(argc, argv, index); + if (cfg.command_line == NULL) { + assert(cfg.shell); + cfg.command_line = cfg.shell; + cfg.window_title = cfg.shell; + } + if (arg_debug) + printf("Extracted command #%s#\n", cfg.command_line); + + // set cpu affinity + if (cfg.cpus) // not available for uid 0 + set_cpu_affinity(); + + // set nice value + if (arg_nice) + set_nice(cfg.nice); + + // add x11 display + if (display) { + char *display_str; + if (asprintf(&display_str, ":%d", display) == -1) + errExit("asprintf"); + setenv("DISPLAY", display_str, 1); + free(display_str); + } + start_application(0, NULL); // it will never get here!!! diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c index 9bb8e545c..2b5d30158 100644 --- a/src/firejail/sandbox.c +++ b/src/firejail/sandbox.c @@ -1038,17 +1038,6 @@ int sandbox(void* sandbox_arg) { } } - // set nice - if (arg_nice) { - errno = 0; - int rv = nice(cfg.nice); - (void) rv; - if (errno) { - fwarning("cannot set nice value\n"); - errno = 0; - } - } - EUID_ROOT(); // clean /tmp/.X11-unix sockets fs_x11(); @@ -1067,17 +1056,11 @@ int sandbox(void* sandbox_arg) { // set capabilities set_caps(); - // set cpu affinity - if (cfg.cpus) { - save_cpu(); // save cpu affinity mask to CPU_CFG file - EUID_USER(); - set_cpu_affinity(); - EUID_ROOT(); - } + // save cpu affinity mask to CPU_CFG file + save_cpu(); // save cgroup in CGROUP_CFG file - if (cfg.cgroup) - save_cgroup(); + save_cgroup(); // set seccomp #ifdef HAVE_SECCOMP @@ -1125,7 +1108,7 @@ int sandbox(void* sandbox_arg) { // to --join //**************************************** - FILE *fp = create_ready_for_join_file(); + FILE *rj = create_ready_for_join_file(); //**************************************** // create a new user namespace @@ -1175,10 +1158,23 @@ int sandbox(void* sandbox_arg) { } //**************************************** - // drop privileges, fork the application and monitor it + // drop privileges //**************************************** drop_privs(arg_nogroups); - prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0); // kill the sandbox in case the parent died + + // kill the sandbox in case the parent died + prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0); + + //**************************************** + // set cpu affinity + //**************************************** + + if (cfg.cpus) + set_cpu_affinity(); + + //**************************************** + // fork the application and monitor it + //**************************************** pid_t app_pid = fork(); if (app_pid == -1) errExit("fork"); @@ -1196,13 +1192,15 @@ int sandbox(void* sandbox_arg) { printf("AppArmor enabled\n"); } #endif - // set rlimits + // set nice and rlimits + if (arg_nice) + set_nice(cfg.nice); set_rlimits(); - // start app - start_application(0, fp); + + start_application(0, rj); } - fclose(fp); + fclose(rj); int status = monitor_application(app_pid); // monitor application flush_stdin(); diff --git a/src/firejail/util.c b/src/firejail/util.c index 46b392eed..3e2cd13d5 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -250,6 +250,16 @@ void logerr(const char *msg) { closelog(); } + +void set_nice(int inc) { + errno = 0; + int rv = nice(inc); + (void) rv; + if (errno) + fwarning("cannot set nice value\n"); +} + + static int copy_file_by_fd(int src, int dst) { assert(src >= 0); assert(dst >= 0); -- cgit v1.2.3-70-g09d2