fs_home.c: improve EUID switching, fix selinux relabeling

5 files changed, 61 insertions, 29 deletions

diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c
index 04f1a0898..1c4f9e07e 100644
--- a/src/firejail/fs_home.c
+++ b/src/firejail/fs_home.c
@@ -35,11 +35,12 @@
 #endif
 
 static void skel(const char *homedir) {
-        char *fname;
+        EUID_ASSERT();
 
         // zsh
         if (!arg_shell_none && (strcmp(cfg.shell,"/usr/bin/zsh") == 0 || strcmp(cfg.shell,"/bin/zsh") == 0)) {
                 // copy skel files
+                char *fname;
                 if (asprintf(&fname, "%s/.zshrc", homedir) == -1)
                         errExit("asprintf");
                 // don't copy it if we already have the file
@@ -64,6 +65,7 @@ static void skel(const char *homedir) {
         // csh
         else if (!arg_shell_none && strcmp(cfg.shell,"/bin/csh") == 0) {
                 // copy skel files
+                char *fname;
                 if (asprintf(&fname, "%s/.cshrc", homedir) == -1)
                         errExit("asprintf");
                 // don't copy it if we already have the file
@@ -88,6 +90,7 @@ static void skel(const char *homedir) {
         // bash etc.
         else {
                 // copy skel files
+                char *fname;
                 if (asprintf(&fname, "%s/.bashrc", homedir) == -1)
                         errExit("asprintf");
                 // don't copy it if we already have the file
@@ -108,6 +111,7 @@ static void skel(const char *homedir) {
 }
 
 static int store_xauthority(void) {
+        EUID_ASSERT();
         if (arg_x11_block)
                 return 0;
 
@@ -118,7 +122,7 @@ static int store_xauthority(void) {
                 errExit("asprintf");
 
         struct stat s;
-        if (lstat_as_user(src, &s) == 0) {
+        if (lstat(src, &s) == 0) {
                 if (S_ISLNK(s.st_mode)) {
                         fwarning("invalid .Xauthority file\n");
                         free(src);
@@ -126,6 +130,7 @@ static int store_xauthority(void) {
                 }
 
                 // create an empty file as root, and change ownership to user
+                EUID_ROOT();
                 FILE *fp = fopen(dest, "we");
                 if (fp) {
                         fprintf(fp, "\n");
@@ -134,10 +139,11 @@ static int store_xauthority(void) {
                 }
                 else
                         errExit("fopen");
+                EUID_USER();
 
                 copy_file_as_user(src, dest, 0600); // regular user
-                fs_logger2("clone", dest);
                 selinux_relabel_path(dest, src);
+                fs_logger2("clone", dest);
                 free(src);
                 return 1; // file copied
         }
@@ -147,6 +153,7 @@ static int store_xauthority(void) {
 }
 
 static int store_asoundrc(void) {
+        EUID_ASSERT();
         if (arg_nosound)
                 return 0;
 
@@ -157,11 +164,11 @@ static int store_asoundrc(void) {
                 errExit("asprintf");
 
         struct stat s;
-        if (lstat_as_user(src, &s) == 0) {
+        if (lstat(src, &s) == 0) {
                 if (S_ISLNK(s.st_mode)) {
                         // make sure the real path of the file is inside the home directory
                         /* coverity[toctou] */
-                        char *rp = realpath_as_user(src);
+                        char *rp = realpath(src, NULL);
                         if (!rp) {
                                 fprintf(stderr, "Error: Cannot access %s\n", src);
                                 exit(1);
@@ -174,6 +181,7 @@ static int store_asoundrc(void) {
                 }
 
                 // create an empty file as root, and change ownership to user
+                EUID_ROOT();
                 FILE *fp = fopen(dest, "we");
                 if (fp) {
                         fprintf(fp, "\n");
@@ -182,10 +190,11 @@ static int store_asoundrc(void) {
                 }
                 else
                         errExit("fopen");
+                EUID_USER();
 
                 copy_file_as_user(src, dest, 0644); // regular user
-                selinux_relabel_path(dest, src);
                 fs_logger2("clone", dest);
+                selinux_relabel_path(dest, src);
                 free(src);
                 return 1; // file copied
         }
@@ -195,6 +204,7 @@ static int store_asoundrc(void) {
 }
 
 static void copy_xauthority(void) {
+        EUID_ASSERT();
         // copy XAUTHORITY_FILE in the new home directory
         char *src = RUN_XAUTHORITY_FILE ;
         char *dest;
@@ -208,15 +218,17 @@ static void copy_xauthority(void) {
         }
 
         copy_file_as_user(src, dest, S_IRUSR | S_IWUSR); // regular user
-        selinux_relabel_path(dest, src);
         fs_logger2("clone", dest);
+        selinux_relabel_path(dest, dest);
         free(dest);
 
-        // delete the temporary file
-        unlink(src);
+        EUID_ROOT();
+        unlink(src); // delete the temporary file
+        EUID_USER();
 }
 
 static void copy_asoundrc(void) {
+        EUID_ASSERT();
         // copy ASOUNDRC_FILE in the new home directory
         char *src = RUN_ASOUNDRC_FILE ;
         char *dest;
@@ -230,12 +242,13 @@ static void copy_asoundrc(void) {
         }
 
         copy_file_as_user(src, dest, S_IRUSR | S_IWUSR); // regular user
-        selinux_relabel_path(dest, src);
         fs_logger2("clone", dest);
+        selinux_relabel_path(dest, dest);
         free(dest);
 
-        // delete the temporary file
-        unlink(src);
+        EUID_ROOT();
+        unlink(src); // delete the temporary file
+        EUID_USER();
 }
 
 // private mode (--private=homedir):
@@ -248,18 +261,18 @@ void fs_private_homedir(void) {
         char *private_homedir = cfg.home_private;
         assert(homedir);
         assert(private_homedir);
-
-        int xflag = store_xauthority();
-        int aflag = store_asoundrc();
+        EUID_ASSERT();
 
         uid_t u = getuid();
         // gid_t g = getgid();
 
+        int xflag = store_xauthority();
+        int aflag = store_asoundrc();
+
         // mount bind private_homedir on top of homedir
         if (arg_debug)
                 printf("Mount-bind %s on top of %s\n", private_homedir, homedir);
         // get file descriptors for homedir and private_homedir, fails if there is any symlink
-        EUID_USER();
         int src = safer_openat(-1, private_homedir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC);
         if (src == -1)
                 errExit("opening private directory");
@@ -287,6 +300,7 @@ void fs_private_homedir(void) {
         EUID_ROOT();
         if (bind_mount_by_fd(src, dst))
                 errExit("mount bind");
+        EUID_USER();
 
         // check /proc/self/mountinfo to confirm the mount is ok
         MountData *mptr = get_last_mount();
@@ -305,6 +319,7 @@ void fs_private_homedir(void) {
 //      if (chmod(homedir, s.st_mode) == -1)
 //              errExit("mount-bind chmod");
 
+        EUID_ROOT();
         if (u != 0) {
                 // mask /root
                 if (arg_debug)
@@ -323,6 +338,7 @@ void fs_private_homedir(void) {
                 selinux_relabel_path("/home", "/home");
                 fs_logger("tmpfs /home");
         }
+        EUID_USER();
 
         skel(homedir);
         if (xflag)
@@ -339,6 +355,7 @@ void fs_private_homedir(void) {
 void fs_private(void) {
         char *homedir = cfg.homedir;
         assert(homedir);
+        EUID_ASSERT();
 
         uid_t u = getuid();
         gid_t g = getgid();
@@ -346,6 +363,7 @@ void fs_private(void) {
         int xflag = store_xauthority();
         int aflag = store_asoundrc();
 
+        EUID_ROOT();
         // mask /root
         if (arg_debug)
                 printf("Mounting a new /root directory\n");
@@ -388,6 +406,7 @@ void fs_private(void) {
 
                 selinux_relabel_path(homedir, homedir);
         }
+        EUID_USER();
 
         skel(homedir);
         if (xflag)
@@ -531,26 +550,29 @@ static void duplicate(char *name) {
 //      set skel files,
 //      restore .Xauthority
 void fs_private_home_list(void) {
-        timetrace_start();
-
         char *homedir = cfg.homedir;
         char *private_list = cfg.home_private_keep;
         assert(homedir);
         assert(private_list);
+        EUID_ASSERT();
 
-        int xflag = store_xauthority();
-        int aflag = store_asoundrc();
+        timetrace_start();
 
         uid_t uid = getuid();
         gid_t gid = getgid();
 
+        int xflag = store_xauthority();
+        int aflag = store_asoundrc();
+
         // create /run/firejail/mnt/home directory
+        EUID_ROOT();
         mkdir_attr(RUN_HOME_DIR, 0755, uid, gid);
         selinux_relabel_path(RUN_HOME_DIR, homedir);
+
         fs_logger_print();      // save the current log
+        EUID_USER();
 
         // copy the list of files in the new home directory
-        EUID_USER();
         if (arg_debug)
                 printf("Copying files in the new home:\n");
         char *dlist = strdup(cfg.home_private_keep);
@@ -589,6 +611,7 @@ void fs_private_home_list(void) {
         if (bind_mount_path_to_fd(RUN_HOME_DIR, fd))
                 errExit("mount bind");
         close(fd);
+        EUID_USER();
 
         // check /proc/self/mountinfo to confirm the mount is ok
         MountData *mptr = get_last_mount();
@@ -596,11 +619,7 @@ void fs_private_home_list(void) {
                 errLogExit("invalid private-home mount");
         fs_logger2("tmpfs", homedir);
 
-        // mask RUN_HOME_DIR, it is writable and not noexec
-        if (mount("tmpfs", RUN_HOME_DIR, "tmpfs", MS_NOSUID | MS_NODEV | MS_STRICTATIME,  "mode=755,gid=0") < 0)
-                errExit("mounting tmpfs");
-        fs_logger2("tmpfs", RUN_HOME_DIR);
-
+        EUID_ROOT();
         if (uid != 0) {
                 // mask /root
                 if (arg_debug)
@@ -620,6 +639,11 @@ void fs_private_home_list(void) {
                 fs_logger("tmpfs /home");
         }
 
+        // mask RUN_HOME_DIR, it is writable and not noexec
+        if (mount("tmpfs", RUN_HOME_DIR, "tmpfs", MS_NOSUID | MS_NODEV | MS_STRICTATIME,  "mode=755,gid=0") < 0)
+                errExit("mounting tmpfs");
+        EUID_USER();
+
         skel(homedir);
         if (xflag)
                 copy_xauthority();
diff --git a/src/firejail/fs_whitelist.c b/src/firejail/fs_whitelist.c
index 370035a4d..7588f69b7 100644
--- a/src/firejail/fs_whitelist.c
+++ b/src/firejail/fs_whitelist.c
@@ -374,9 +374,12 @@ static void tmpfs_topdirs(const TopDir *topdirs) {
         }
 
         // user home directory
-        if (tmpfs_home)
+        if (tmpfs_home) {
                 // checks owner if outside /home
+                EUID_USER();
                 fs_private();
+                EUID_ROOT();
+        }
 
         // /run/user/$UID directory
         if (tmpfs_runuser) {
diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c
index e06ba3617..95be3335f 100644
--- a/src/firejail/sandbox.c
+++ b/src/firejail/sandbox.c
@@ -840,6 +840,7 @@ int sandbox(void* sandbox_arg) {
         // private mode
         //****************************
         if (arg_private) {
+                EUID_USER();
                 if (cfg.home_private) { // --private=
                         if (cfg.chrootdir)
                                 fwarning("private=directory feature is disabled in chroot\n");
@@ -858,6 +859,7 @@ int sandbox(void* sandbox_arg) {
                 }
                 else // --private
                         fs_private();
+                EUID_ROOT();
         }
 
         if (arg_private_dev)
diff --git a/src/firejail/util.c b/src/firejail/util.c
index 0921736f1..8fcaf3f7b 100644
--- a/src/firejail/util.c
+++ b/src/firejail/util.c
@@ -1231,6 +1231,7 @@ unsigned extract_timeout(const char *str) {
 }
 
 void disable_file_or_dir(const char *fname) {
+        assert(geteuid() == 0);
         assert(fname);
 
         EUID_USER();
diff --git a/src/firejail/x11.c b/src/firejail/x11.c
index 0619ff380..896aa2fd3 100644
--- a/src/firejail/x11.c
+++ b/src/firejail/x11.c
@@ -1290,9 +1290,11 @@ void x11_xorg(void) {
         if (envar) {
                 char *rp = realpath(envar, NULL);
                 if (rp) {
-                        if (strcmp(rp, dest) != 0)
-                                // disable_file_or_dir returns with EUID 0
+                        if (strcmp(rp, dest) != 0) {
+                                EUID_ROOT();
                                 disable_file_or_dir(rp);
+                                EUID_USER();
+                        }
                         free(rp);
                 }
         }