From b55d61228778d9d3d397c99af52350a3ff136659 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Fri, 11 Jun 2021 12:28:43 +0200 Subject: follow-up PR #4349 --- src/firejail/firejail.h | 9 +++++++ src/firejail/fs.c | 18 ++++++++++---- src/firejail/pulseaudio.c | 62 ++++++++++++++++------------------------------- src/firejail/util.c | 9 +++---- src/firejail/x11.c | 15 ++++++++---- 5 files changed, 57 insertions(+), 56 deletions(-) diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index 10133142a..c442a97bf 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -45,6 +45,15 @@ assert(s.st_gid == gid);\ assert((s.st_mode & 07777) == (mode));\ } while (0) +#define ASSERT_PERMS_AS_USER(file, uid, gid, mode) \ + do { \ + assert(file);\ + struct stat s;\ + if (stat_as_user(file, &s) == -1) errExit("stat");\ + assert(s.st_uid == uid);\ + assert(s.st_gid == gid);\ + assert((s.st_mode & 07777) == (mode));\ + } while (0) #define ASSERT_PERMS_FD(fd, uid, gid, mode) \ do { \ struct stat s;\ diff --git a/src/firejail/fs.c b/src/firejail/fs.c index 01182bd2c..bf78f8a17 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -77,7 +77,7 @@ static void disable_file(OPERATION op, const char *filename) { EUID_ROOT(); int err = bind_mount_path_to_fd(RUN_RO_DIR, fd); - if (err < 0) + if (err != 0) err = bind_mount_path_to_fd(RUN_RO_FILE, fd); EUID_USER(); close(fd); @@ -655,8 +655,13 @@ static void fs_remount_rec(const char *dir, OPERATION op) { // resolve a path and remount it void fs_remount(const char *path, OPERATION op, int rec) { assert(path); - assert(geteuid() == 0); - EUID_USER(); + + int called_as_root = 0; + if (geteuid() == 0) + called_as_root = 1; + + if (called_as_root) + EUID_USER(); char *rpath = realpath(path, NULL); if (rpath) { @@ -666,7 +671,9 @@ void fs_remount(const char *path, OPERATION op, int rec) { fs_remount_simple(rpath, op); free(rpath); } - EUID_ROOT(); + + if (called_as_root) + EUID_ROOT(); } // Disable /mnt, /media, /run/mount and /run/media access @@ -821,7 +828,6 @@ void disable_config(void) { // build a basic read-only filesystem -// top level directories could be links, run no after-mount checks void fs_basic_fs(void) { uid_t uid = getuid(); @@ -831,6 +837,7 @@ void fs_basic_fs(void) { if (mount("proc", "/proc", "proc", MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_REC, NULL) < 0) errExit("mounting /proc"); + EUID_USER(); if (arg_debug) printf("Basic read-only filesystem:\n"); if (!arg_writable_etc) { @@ -850,6 +857,7 @@ void fs_basic_fs(void) { fs_remount("/lib64", MOUNT_READONLY, 1); fs_remount("/lib32", MOUNT_READONLY, 1); fs_remount("/libx32", MOUNT_READONLY, 1); + EUID_ROOT(); // update /var directory in order to support multiple sandboxes running on the same root directory fs_var_lock(); diff --git a/src/firejail/pulseaudio.c b/src/firejail/pulseaudio.c index be0f5aea6..f8d4c2f3c 100644 --- a/src/firejail/pulseaudio.c +++ b/src/firejail/pulseaudio.c @@ -75,31 +75,34 @@ void pulseaudio_disable(void) { closedir(dir); } -static void pulseaudio_fallback(const char *path) { - assert(path); - - fmessage("Cannot mount tmpfs on %s/.config/pulse\n", cfg.homedir); - env_store_name_val("PULSE_CLIENTCONFIG", path, SETENV); -} - // disable shm in pulseaudio (issue #69) void pulseaudio_init(void) { - struct stat s; - // do we have pulseaudio in the system? - if (stat(PULSE_CLIENT_SYSCONF, &s) == -1) { + if (access(PULSE_CLIENT_SYSCONF, R_OK)) { if (arg_debug) - printf("%s not found\n", PULSE_CLIENT_SYSCONF); + printf("Cannot read %s\n", PULSE_CLIENT_SYSCONF); return; } + // create ~/.config/pulse directory if not present + char *homeusercfg = NULL; + if (asprintf(&homeusercfg, "%s/.config", cfg.homedir) == -1) + errExit("asprintf"); + if (create_empty_dir_as_user(homeusercfg, 0700)) + fs_logger2("create", homeusercfg); + + free(homeusercfg); + if (asprintf(&homeusercfg, "%s/.config/pulse", cfg.homedir) == -1) + errExit("asprintf"); + if (create_empty_dir_as_user(homeusercfg, 0700)) + fs_logger2("create", homeusercfg); + // create the new user pulseaudio directory + // that will be mounted over ~/.config/pulse if (mkdir(RUN_PULSE_DIR, 0700) == -1) errExit("mkdir"); - selinux_relabel_path(RUN_PULSE_DIR, RUN_PULSE_DIR); - // mount it nosuid, noexec, nodev + selinux_relabel_path(RUN_PULSE_DIR, homeusercfg); fs_remount(RUN_PULSE_DIR, MOUNT_NOEXEC, 0); - // create the new client.conf file char *pulsecfg = NULL; if (asprintf(&pulsecfg, "%s/client.conf", RUN_PULSE_DIR) == -1) @@ -116,37 +119,14 @@ void pulseaudio_init(void) { if (set_perms(RUN_PULSE_DIR, getuid(), getgid(), 0700)) errExit("set_perms"); - // create ~/.config/pulse directory if not present - char *homeusercfg = NULL; - if (asprintf(&homeusercfg, "%s/.config", cfg.homedir) == -1) - errExit("asprintf"); - if (create_empty_dir_as_user(homeusercfg, 0700)) - fs_logger2("create", homeusercfg); - - free(homeusercfg); - if (asprintf(&homeusercfg, "%s/.config/pulse", cfg.homedir) == -1) - errExit("asprintf"); - if (create_empty_dir_as_user(homeusercfg, 0700)) - fs_logger2("create", homeusercfg); - // if ~/.config/pulse exists and there are no symbolic links, mount the new directory // else set environment variable + EUID_USER(); int fd = safer_openat(-1, homeusercfg, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); + EUID_ROOT(); if (fd == -1) { - pulseaudio_fallback(pulsecfg); - goto out; - } - // confirm the actual mount destination is owned by the user - if (fstat(fd, &s) == -1) { // FUSE - if (errno != EACCES) - errExit("fstat"); - close(fd); - pulseaudio_fallback(pulsecfg); - goto out; - } - if (s.st_uid != getuid()) { - close(fd); - pulseaudio_fallback(pulsecfg); + fwarning("not mounting tmpfs on %s\n", homeusercfg); + env_store_name_val("PULSE_CLIENTCONFIG", pulsecfg, SETENV); goto out; } // preserve a read-only mount diff --git a/src/firejail/util.c b/src/firejail/util.c index b8643ff60..edd08bb41 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -981,7 +981,7 @@ int remove_overlay_directory(void) { int fd = safer_openat(-1, path, O_PATH|O_NOFOLLOW|O_CLOEXEC); if (fd == -1) { fprintf(stderr, "Error: cannot open %s\n", path); - _exit(1); + exit(1); } struct stat s; if (fstat(fd, &s) == -1) @@ -991,11 +991,11 @@ int remove_overlay_directory(void) { fprintf(stderr, "Error: %s is a symbolic link\n", path); else fprintf(stderr, "Error: %s is not a directory\n", path); - _exit(1); + exit(1); } if (s.st_uid != getuid()) { fprintf(stderr, "Error: %s is not owned by the current user\n", path); - _exit(1); + exit(1); } // chdir to ~/.firejail if (fchdir(fd) == -1) @@ -1187,7 +1187,6 @@ unsigned extract_timeout(const char *str) { void disable_file_or_dir(const char *fname) { assert(fname); - assert(geteuid() == 0); EUID_USER(); int fd = open(fname, O_PATH|O_CLOEXEC); @@ -1207,7 +1206,7 @@ void disable_file_or_dir(const char *fname) { printf("blacklist %s\n", fname); if (S_ISDIR(s.st_mode)) { if (bind_mount_path_to_fd(RUN_RO_DIR, fd) < 0) - errExit("disable directory"); + errExit("disable directory"); } else { if (bind_mount_path_to_fd(RUN_RO_FILE, fd) < 0) diff --git a/src/firejail/x11.c b/src/firejail/x11.c index 09956b903..0619ff380 100644 --- a/src/firejail/x11.c +++ b/src/firejail/x11.c @@ -1204,14 +1204,13 @@ void x11_xorg(void) { fmessage("Generating a new .Xauthority file\n"); mkdir_attr(RUN_XAUTHORITY_SEC_DIR, 0700, getuid(), getgid()); // create new Xauthority file in RUN_XAUTHORITY_SEC_DIR + EUID_USER(); char tmpfname[] = RUN_XAUTHORITY_SEC_DIR "/.Xauth-XXXXXX"; int fd = mkstemp(tmpfname); if (fd == -1) { fprintf(stderr, "Error: cannot create .Xauthority file\n"); exit(1); } - if (fchown(fd, getuid(), getgid()) == -1) - errExit("chown"); close(fd); // run xauth @@ -1221,8 +1220,6 @@ void x11_xorg(void) { else sbox_run(SBOX_USER | SBOX_CAPS_NONE | SBOX_SECCOMP, 7, RUN_XAUTH_FILE, "-f", tmpfname, "generate", display, "MIT-MAGIC-COOKIE-1", "untrusted"); - // remove xauth copy - unlink(RUN_XAUTH_FILE); // ensure there is already a file ~/.Xauthority, so that bind-mount below will work. char *dest; @@ -1273,10 +1270,12 @@ void x11_xorg(void) { // mount via the link in /proc/self/fd if (arg_debug) printf("Mounting %s on %s\n", tmpfname, dest); + EUID_ROOT(); if (bind_mount_by_fd(src, dst)) { fprintf(stderr, "Error: cannot mount the new .Xauthority file\n"); exit(1); } + EUID_USER(); // check /proc/self/mountinfo to confirm the mount is ok MountData *mptr = get_last_mount(); if (strcmp(mptr->dir, dest) != 0 || strcmp(mptr->fstype, "tmpfs") != 0) @@ -1289,9 +1288,10 @@ void x11_xorg(void) { // blacklist user .Xauthority file if it is not masked already const char *envar = env_get("XAUTHORITY"); if (envar) { - char *rp = realpath_as_user(envar); + char *rp = realpath(envar, NULL); if (rp) { if (strcmp(rp, dest) != 0) + // disable_file_or_dir returns with EUID 0 disable_file_or_dir(rp); free(rp); } @@ -1301,9 +1301,13 @@ void x11_xorg(void) { free(dest); // mask RUN_XAUTHORITY_SEC_DIR + EUID_ROOT(); if (mount("tmpfs", RUN_XAUTHORITY_SEC_DIR, "tmpfs", MS_NOSUID | MS_NODEV | MS_STRICTATIME, "mode=755,gid=0") < 0) errExit("mounting tmpfs"); fs_logger2("tmpfs", RUN_XAUTHORITY_SEC_DIR); + + // cleanup + unlink(RUN_XAUTH_FILE); #endif } @@ -1352,6 +1356,7 @@ void fs_x11(void) { MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_STRICTATIME, "mode=1777,uid=0,gid=0") < 0) errExit("mounting tmpfs on /tmp/.X11-unix"); + selinux_relabel_path("/tmp/.X11-unix", "/tmp/.X11-unix"); fs_logger("tmpfs /tmp/.X11-unix"); // create an empty root-owned file which will have the desired socket bind-mounted over it -- cgit v1.2.3-54-g00ecf