From 771dccecba69e134cbf425ad036b2d1fa01f1ccc Mon Sep 17 00:00:00 2001 From: smitsohu Date: Sat, 26 Jun 2021 12:35:48 +0200 Subject: fs_home.c: improve EUID switching, fix selinux relabeling --- src/firejail/fs_home.c | 76 +++++++++++++++++++++++++++++---------------- src/firejail/fs_whitelist.c | 5 ++- src/firejail/sandbox.c | 2 ++ src/firejail/util.c | 1 + src/firejail/x11.c | 6 ++-- 5 files changed, 61 insertions(+), 29 deletions(-) (limited to 'src') 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); } } -- cgit v1.2.3-70-g09d2