From 2cfd4dafc55117795970dc0c1f5655dbbd4588bd Mon Sep 17 00:00:00 2001 From: smitsohu Date: Sat, 23 Jul 2022 13:58:33 +0200 Subject: [PATCH] improve force-nonewprivs security guarantees --- src/firejail/join.c | 22 +++++++++++++++------- src/firejail/main.c | 3 ++- src/firejail/sandbox.c | 27 +++++++++------------------ 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/firejail/join.c b/src/firejail/join.c index 4e636ca27a2..96d891a4954 100644 --- a/src/firejail/join.c +++ b/src/firejail/join.c @@ -133,6 +133,8 @@ static void extract_nogroups(ProcessHandle sandbox) { if (process_rootfs_stat(sandbox, RUN_GROUPS_CFG, &s) == 0) arg_nogroups = 1; + else if (errno != ENOENT) + errExit("stat"); } static void extract_nonewprivs(ProcessHandle sandbox) { @@ -140,6 +142,8 @@ static void extract_nonewprivs(ProcessHandle sandbox) { if (process_rootfs_stat(sandbox, RUN_NONEWPRIVS_CFG, &s) == 0) arg_nonewprivs = 1; + else if (errno != ENOENT) + errExit("stat"); } static void extract_caps(ProcessHandle sandbox) { @@ -477,13 +481,6 @@ void join(pid_t pid, int argc, char **argv, int index) { EUID_USER(); unpin_process(sandbox); - // set nonewprivs - if (arg_nonewprivs == 1) { // not available for uid 0 - int rv = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); - if (arg_debug && rv == 0) - printf("NO_NEW_PRIVS set\n"); - } - int cwd = 0; if (cfg.cwd) { if (chdir(cfg.cwd) == 0) @@ -503,6 +500,17 @@ void join(pid_t pid, int argc, char **argv, int index) { } } + // set nonewprivs +#ifndef HAVE_FORCE_NONEWPRIVS + if (arg_nonewprivs == 1) // not available for uid 0 +#endif + { + if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) != 0) + errExit("prctl"); + if (arg_debug) + printf("NO_NEW_PRIVS set\n"); + } + // drop privileges drop_privs(arg_nogroups); diff --git a/src/firejail/main.c b/src/firejail/main.c index ff88b9f6e09..e6c5b50b0d2 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -1239,7 +1239,8 @@ int main(int argc, char **argv, char **envp) { if (check_arg(argc, argv, "--appimage", 1)) arg_appimage = 1; - // check for force-nonewprivs in /etc/firejail/firejail.config file + // load configuration file /etc/firejail/firejail.config + // and check for force-nonewprivs if (checkcfg(CFG_FORCE_NONEWPRIVS)) arg_nonewprivs = 1; diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c index e72b03e15a0..86423682446 100644 --- a/src/firejail/sandbox.c +++ b/src/firejail/sandbox.c @@ -50,7 +50,6 @@ #include #endif -static int force_nonewprivs = 0; extern int just_run_the_shell; static int monitored_pid = 0; @@ -629,7 +628,6 @@ static void enforce_filters(void) { fmessage("\n** Warning: dropping all Linux capabilities and setting NO_NEW_PRIVS prctl **\n\n"); // enforce NO_NEW_PRIVS arg_nonewprivs = 1; - force_nonewprivs = 1; // disable all capabilities arg_caps_drop_all = 1; @@ -832,14 +830,9 @@ int sandbox(void* sandbox_arg) { exit(rv); } -#ifdef HAVE_FORCE_NONEWPRIVS - bool always_enforce_filters = true; -#else - bool always_enforce_filters = false; -#endif // for --appimage, --chroot and --overlay* we force NO_NEW_PRIVS // and drop all capabilities - if (getuid() != 0 && (arg_appimage || cfg.chrootdir || arg_overlay || always_enforce_filters)) + if (getuid() != 0 && (arg_appimage || cfg.chrootdir || arg_overlay)) enforce_filters(); // need ld.so.preload if tracing or seccomp with any non-default lists @@ -1266,17 +1259,15 @@ int sandbox(void* sandbox_arg) { //**************************************** // Set NO_NEW_PRIVS if desired //**************************************** - if (arg_nonewprivs) { - prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); - - if (prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0) != 1) { - fwarning("cannot set NO_NEW_PRIVS, it requires a Linux kernel version 3.5 or newer.\n"); - if (force_nonewprivs) { - fprintf(stderr, "Error: NO_NEW_PRIVS required for this sandbox, exiting ...\n"); - exit(1); - } +#ifndef HAVE_FORCE_NONEWPRIVS + if (arg_nonewprivs) +#endif + { + if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) != 0) { + fprintf(stderr, "Error: cannot set NO_NEW_PRIVS, it requires a Linux kernel version 3.5 or newer.\n"); + exit(1); } - else if (arg_debug) + if (arg_debug) printf("NO_NEW_PRIVS set\n"); }