Merge pull request #5271 from smitsohu/nnp

improve force-nonewprivs security guarantees

3 files changed, 26 insertions, 26 deletions

diff --git a/src/firejail/join.c b/src/firejail/join.c
index 4e636ca27..96d891a49 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 ff88b9f6e..e6c5b50b0 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 e72b03e15..864236824 100644
--- a/src/firejail/sandbox.c
+++ b/src/firejail/sandbox.c
@@ -50,7 +50,6 @@
 #include <sys/apparmor.h>
 #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");
         }