From 7d0800682ab3a74e3d463836cd2ca5cd697d542c Mon Sep 17 00:00:00 2001 From: smitsohu Date: Mon, 17 Aug 2020 16:40:52 +0200 Subject: various x11 xorg enhancements 1) copy xauth binary into the sandbox and set mode to 0711, so it runs with cleared dumpable flag for unprivileged users 2) run xauth in an sbox sandbox 3) generate Xauthority file in runtime directory instead of /tmp; this way xauth is able to connect to the X11 socket even if the abstract socket doesn't exist, for example because a new network namespace was instantiated --- src/firejail/x11.c | 173 ++++++++++++++++++++------------------------------ src/include/rundefs.h | 5 +- 2 files changed, 72 insertions(+), 106 deletions(-) (limited to 'src') diff --git a/src/firejail/x11.c b/src/firejail/x11.c index 98ac184d9..1ecb688af 100644 --- a/src/firejail/x11.c +++ b/src/firejail/x11.c @@ -34,7 +34,7 @@ #include #ifndef O_PATH -# define O_PATH 010000000 +#define O_PATH 010000000 #endif @@ -1129,28 +1129,17 @@ void x11_start(int argc, char **argv) { } #endif -// Porting notes: -// -// 1. merge #1100 from zackw: -// Attempting to run xauth -f directly on a file in /run/firejail/mnt/ directory fails on Debian 8 -// with this message: -// xauth: timeout in locking authority file /run/firejail/mnt/sec.Xauthority-Qt5Mu4 -// Failed to create untrusted X cookie: xauth: exit 1 -// For this reason we run xauth on a file in a tmpfs filesystem mounted on /tmp. This was -// a partial merge. -// -// 2. Since we cannot deal with the TOCTOU condition when mounting .Xauthority in user home -// directory, we need to make sure /usr/bin/xauth executable is the real thing, and not -// something picked up on $PATH. -// -// 3. If for any reason xauth command fails, we exit the sandbox. On Debian 8 this happens -// when using a network namespace. Somehow, xauth tries to connect to the abstract socket, -// and it fails because of the network namespace - it should try to connect to the regular -// Unix socket! If we ignore the fail condition, the program will be started on X server without -// the security extension loaded. + void x11_xorg(void) { #ifdef HAVE_X11 + // get DISPLAY env + char *display = getenv("DISPLAY"); + if (!display) { + fputs("Error: --x11=xorg requires an 'outer' X11 server to use.\n", stderr); + exit(1); + } + // check xauth utility is present in the system struct stat s; if (stat("/usr/bin/xauth", &s) == -1) { @@ -1160,26 +1149,27 @@ void x11_xorg(void) { fprintf(stderr, " Fedora: sudo dnf install xorg-x11-xauth\n"); exit(1); } - if (s.st_uid != 0 && s.st_gid != 0) { + if ((s.st_uid != 0 && s.st_gid != 0) || (s.st_mode & S_IWOTH)) { fprintf(stderr, "Error: invalid /usr/bin/xauth executable\n"); exit(1); } - - // get DISPLAY env - char *display = getenv("DISPLAY"); - if (!display) { - fputs("Error: --x11=xorg requires an 'outer' X11 server to use.\n", stderr); + if (s.st_size > 1024 * 1024) { + fprintf(stderr, "Error: /usr/bin/xauth executable is too large\n"); exit(1); } - - // temporarily mount a tempfs on top of /tmp directory - if (mount("tmpfs", "/tmp", "tmpfs", MS_NOSUID | MS_STRICTATIME, "mode=1777,gid=0") < 0) - errExit("mounting /tmp"); - - // create the temporary .Xauthority file + // copy /usr/bin/xauth in the sandbox and set mode to 0711 + // users are not able to trace the running xauth this way if (arg_debug) - printf("Generating a new .Xauthority file\n"); - char tmpfname[] = "/tmp/.tmpXauth-XXXXXX"; + printf("Copying /usr/bin/xauth to %s\n", RUN_XAUTH_FILE); + if (copy_file("/usr/bin/xauth", RUN_XAUTH_FILE, 0, 0, 0711)) { + fprintf(stderr, "Error: cannot copy /usr/bin/xauth executable\n"); + exit(1); + } + + 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 + char tmpfname[] = RUN_XAUTHORITY_SEC_DIR "/.Xauth-XXXXXX"; int fd = mkstemp(tmpfname); if (fd == -1) { fprintf(stderr, "Error: cannot create .Xauthority file\n"); @@ -1189,64 +1179,17 @@ void x11_xorg(void) { errExit("chown"); close(fd); - pid_t child = fork(); - if (child < 0) - errExit("fork"); - if (child == 0) { - drop_privs(1); - clearenv(); -#ifdef HAVE_GCOV - __gcov_flush(); -#endif - if (arg_debug) { - execlp("/usr/bin/xauth", "/usr/bin/xauth", "-v", "-f", tmpfname, - "generate", display, "MIT-MAGIC-COOKIE-1", "untrusted", NULL); - } - else { - execlp("/usr/bin/xauth", "/usr/bin/xauth", "-f", tmpfname, - "generate", display, "MIT-MAGIC-COOKIE-1", "untrusted", NULL); - } - - _exit(127); - } - - // wait for the xauth process to finish - int status; - if (waitpid(child, &status, 0) != child) - errExit("waitpid"); - if (WIFEXITED(status) && WEXITSTATUS(status) == 0) { - /* success */ - } - else if (WIFEXITED(status)) { - fprintf(stderr, "Failed to create untrusted X cookie: xauth: exit %d\n", - WEXITSTATUS(status)); - exit(1); - } - else if (WIFSIGNALED(status)) { - fprintf(stderr, "Failed to create untrusted X cookie: xauth: %s\n", - strsignal(WTERMSIG(status))); - exit(1); - } - else { - fprintf(stderr, "Failed to create untrusted X cookie: " - "xauth: un-decodable exit status %04x\n", status); - exit(1); - } - - // move the temporary file in RUN_XAUTHORITY_SEC_FILE in order to have it deleted - // automatically when the sandbox is closed (rename doesn't work) + // run xauth if (arg_debug) - printf("Copying the new .Xauthority file\n"); - copy_file_from_user_to_root(tmpfname, RUN_XAUTHORITY_SEC_FILE, getuid(), getgid(), 0600); - - /* coverity[toctou] */ - unlink(tmpfname); - umount("/tmp"); - - // mount RUN_XAUTHORITY_SEC_FILE noexec, nodev, nosuid - fs_remount(RUN_XAUTHORITY_SEC_FILE, MOUNT_NOEXEC, 0); + sbox_run(SBOX_USER | SBOX_CAPS_NONE | SBOX_SECCOMP, 8, RUN_XAUTH_FILE, "-v", "-f", tmpfname, + "generate", display, "MIT-MAGIC-COOKIE-1", "untrusted"); + 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 in the usual location, so that bind-mount below will work. + // ensure there is already a file ~/.Xauthority, so that bind-mount below will work. char *dest; if (asprintf(&dest, "%s/.Xauthority", cfg.homedir) == -1) errExit("asprintf"); @@ -1257,13 +1200,12 @@ void x11_xorg(void) { exit(1); } } - - // get a file descriptor for .Xauthority - fd = safe_fd(dest, O_PATH|O_NOFOLLOW|O_CLOEXEC); - if (fd == -1) + // get a file descriptor for ~/.Xauthority + int dst = safe_fd(dest, O_PATH|O_NOFOLLOW|O_CLOEXEC); + if (dst == -1) errExit("safe_fd"); // check if the actual mount destination is a user owned regular file - if (fstat(fd, &s) == -1) + if (fstat(dst, &s) == -1) errExit("fstat"); if (!S_ISREG(s.st_mode) || s.st_uid != getuid()) { if (S_ISLNK(s.st_mode)) @@ -1274,31 +1216,49 @@ void x11_xorg(void) { } // preserve a read-only mount struct statvfs vfs; - if (fstatvfs(fd, &vfs) == -1) + if (fstatvfs(dst, &vfs) == -1) errExit("fstatvfs"); if ((vfs.f_flag & MS_RDONLY) == MS_RDONLY) - fs_remount(RUN_XAUTHORITY_SEC_FILE, MOUNT_READONLY, 0); + fs_remount(RUN_XAUTHORITY_SEC_DIR, MOUNT_READONLY, 0); + + // always mounting the new Xauthority file noexec,nodev,nosuid + fs_remount(RUN_XAUTHORITY_SEC_DIR, MOUNT_NOEXEC, 0); + + // get a file descriptor for the new Xauthority file + int src = safe_fd(tmpfname, O_PATH|O_NOFOLLOW|O_CLOEXEC); + if (src == -1) + errExit("safe_fd"); + if (fstat(src, &s) == -1) + errExit("fstat"); + if (!S_ISREG(s.st_mode)) { + errno = EPERM; + errExit("mounting Xauthority file"); + } // mount via the link in /proc/self/fd if (arg_debug) - printf("Mounting %s on %s\n", RUN_XAUTHORITY_SEC_FILE, dest); - char *proc; - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) + printf("Mounting %s on %s\n", tmpfname, dest); + char *proc_src, *proc_dst; + if (asprintf(&proc_src, "/proc/self/fd/%d", src) == -1) + errExit("asprintf"); + if (asprintf(&proc_dst, "/proc/self/fd/%d", dst) == -1) errExit("asprintf"); - if (mount(RUN_XAUTHORITY_SEC_FILE, proc, "none", MS_BIND, "mode=0600") == -1) { + if (mount(proc_src, proc_dst, NULL, MS_BIND, NULL) == -1) { fprintf(stderr, "Error: cannot mount the new .Xauthority file\n"); exit(1); } - free(proc); - close(fd); // 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) errLogExit("invalid .Xauthority mount"); + free(proc_src); + free(proc_dst); + close(src); + close(dst); ASSERT_PERMS(dest, getuid(), getgid(), 0600); - // blacklist .Xauthority file if it is not masked already + // blacklist user .Xauthority file if it is not masked already char *envar = getenv("XAUTHORITY"); if (envar) { char *rp = realpath(envar, NULL); @@ -1312,6 +1272,11 @@ void x11_xorg(void) { if (setenv("XAUTHORITY", dest, 1) < 0) errExit("setenv"); free(dest); + + // mask RUN_XAUTHORITY_SEC_DIR + 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); #endif } diff --git a/src/include/rundefs.h b/src/include/rundefs.h index f8bcdec52..d56623907 100644 --- a/src/include/rundefs.h +++ b/src/include/rundefs.h @@ -99,8 +99,9 @@ #define RUN_WHITELIST_SHARE_DIR RUN_MNT_DIR "/orig-share" #define RUN_WHITELIST_MODULE_DIR RUN_MNT_DIR "/orig-module" -#define RUN_XAUTHORITY_FILE RUN_MNT_DIR "/.Xauthority" -#define RUN_XAUTHORITY_SEC_FILE RUN_MNT_DIR "/sec.Xauthority" +#define RUN_XAUTHORITY_FILE RUN_MNT_DIR "/.Xauthority" // private options +#define RUN_XAUTH_FILE RUN_MNT_DIR "/xauth" // x11=xorg +#define RUN_XAUTHORITY_SEC_DIR RUN_MNT_DIR "/.sec.Xauthority" // x11=xorg #define RUN_ASOUNDRC_FILE RUN_MNT_DIR "/.asoundrc" #define RUN_HOSTNAME_FILE RUN_MNT_DIR "/hostname" #define RUN_HOSTS_FILE RUN_MNT_DIR "/hosts" -- cgit v1.2.3-54-g00ecf