From 8bff773d6a7bf70c97b3d5b751df9ec0dd6c8b5d Mon Sep 17 00:00:00 2001 From: smitsohu Date: Tue, 9 Jul 2019 11:59:57 +0200 Subject: add symlink resolution for home directories --- etc/firejail.config | 3 +++ src/firejail/checkcfg.c | 2 ++ src/firejail/firejail.h | 1 + src/firejail/main.c | 45 ++++++++++++++++++++++++++++++++++++++------- 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/etc/firejail.config b/etc/firejail.config index 565796d5a..4c0cb2a41 100644 --- a/etc/firejail.config +++ b/etc/firejail.config @@ -2,6 +2,9 @@ # keyword-argument pairs, one per line. Most features are enabled by default. # Use 'yes' or 'no' as configuration values. +# Allow symbolic links in path of user home directories, default disabled. +# homedir-symlink no + # Enable AppArmor functionality, default enabled. # apparmor yes diff --git a/src/firejail/checkcfg.c b/src/firejail/checkcfg.c index f94b95d60..84054fe76 100644 --- a/src/firejail/checkcfg.c +++ b/src/firejail/checkcfg.c @@ -50,6 +50,7 @@ int checkcfg(int val) { cfg_val[CFG_DISABLE_MNT] = 0; cfg_val[CFG_ARP_PROBES] = DEFAULT_ARP_PROBES; cfg_val[CFG_XPRA_ATTACH] = 0; + cfg_val[CFG_HOMEDIR_SYMLINK] = 0; // open configuration file const char *fname = SYSCONFDIR "/firejail.config"; @@ -85,6 +86,7 @@ int checkcfg(int val) { ptr = line_remove_spaces(buf); if (!ptr) continue; + PARSE_YESNO(CFG_HOMEDIR_SYMLINK, "homedir-symlink") PARSE_YESNO(CFG_FILE_TRANSFER, "file-transfer") PARSE_YESNO(CFG_DBUS, "dbus") PARSE_YESNO(CFG_JOIN, "join") diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index 7664c8037..4bd70697e 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -720,6 +720,7 @@ enum { CFG_PRIVATE_CACHE, CFG_CGROUP, CFG_NAME_CHANGE, + CFG_HOMEDIR_SYMLINK, // CFG_FILE_COPY_LIMIT - file copy limit handled using setenv/getenv CFG_MAX // this should always be the last entry }; diff --git a/src/firejail/main.c b/src/firejail/main.c index 1e2488062..03956ce4d 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -242,6 +242,41 @@ static pid_t require_pid(const char *name) { return pid; } +// return 1 if there is a link somewhere in path of directory +static int has_link(const char *dir) { + assert(dir); + int fd = safe_fd(dir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); + if (fd == -1) { + if (errno == ENOTDIR && is_dir(dir)) + return 1; + } + else + close(fd); + return 0; +} + +static void build_cfg_homedir(const char *dir) { + EUID_ASSERT(); + assert(dir); + if (dir[0] != '/') { + fprintf(stderr, "Error: invalid user directory \"%s\"\n", dir); + exit(1); + } + // symlinks are rejected in many places, provide a solution for home directories + if (checkcfg(CFG_HOMEDIR_SYMLINK)) { + cfg.homedir = realpath(dir, NULL); + if (cfg.homedir) + return; + } + else if (has_link(dir)) { + fprintf(stderr, "Error: path of user directory contains a symbolic link. " + "Please provide resolved path in password database (/etc/passwd) " + "or enable symbolic link resolution in Firejail configuration file.\n"); + exit(1); + } + cfg.homedir = clean_pathname(dir); +} + // init configuration static void init_cfg(int argc, char **argv) { EUID_ASSERT(); @@ -265,15 +300,12 @@ static void init_cfg(int argc, char **argv) { errExit("strdup"); // build home directory name - cfg.homedir = NULL; - if (pw->pw_dir != NULL) { - cfg.homedir = clean_pathname(pw->pw_dir); - assert(cfg.homedir); - } - else { + if (pw->pw_dir == NULL) { fprintf(stderr, "Error: user %s doesn't have a user directory assigned\n", cfg.username); exit(1); } + build_cfg_homedir(pw->pw_dir); + assert(cfg.homedir); cfg.cwd = getcwd(NULL, 0); if (!cfg.cwd && errno != ENOENT) @@ -926,7 +958,6 @@ int main(int argc, char **argv) { // check if the user is allowed to use firejail init_cfg(argc, argv); - assert(cfg.homedir); // get starting timestamp, process --quiet start_timestamp = getticks(); -- cgit v1.2.3-54-g00ecf From 74e5911806d6f456819c65db37b0e29bc1f402d7 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Tue, 9 Jul 2019 14:13:58 +0200 Subject: move to fd based homedir mounts --- src/firejail/fs.c | 9 ++--- src/firejail/fs_home.c | 88 +++++++++++++++++++++++++++++-------------- src/firejail/fs_whitelist.c | 10 ++++- src/firejail/restrict_users.c | 16 +++++++- 4 files changed, 86 insertions(+), 37 deletions(-) diff --git a/src/firejail/fs.c b/src/firejail/fs.c index fe0427466..ce2ca5e2a 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -441,6 +441,8 @@ static int get_mount_flags(const char *path, unsigned long *flags) { // mount a writable tmpfs on directory void fs_tmpfs(const char *dir, unsigned check_owner) { assert(dir); + if (arg_debug) + printf("Mounting tmpfs on %s\n", dir); // get a file descriptor for dir, fails if there is any symlink int fd = safe_fd(dir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); if (fd == -1) @@ -449,12 +451,9 @@ void fs_tmpfs(const char *dir, unsigned check_owner) { if (fstat(fd, &s) == -1) errExit("fstat"); if (check_owner && s.st_uid != getuid()) { - fwarning("no tmpfs mounted on %s: not owned by the current user\n", dir); - close(fd); - return; + fprintf(stderr, "Error: cannot mount tmpfs on %s: not owned by the current user\n", dir); + exit(1); } - if (arg_debug) - printf("Mounting tmpfs on %s\n", dir); // preserve ownership, mode char *options; if (asprintf(&options, "mode=%o,uid=%u,gid=%u", s.st_mode & 07777, s.st_uid, s.st_gid) == -1) diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c index 3f6d78db4..69ad5e2c8 100644 --- a/src/firejail/fs_home.c +++ b/src/firejail/fs_home.c @@ -239,13 +239,16 @@ void fs_private_homedir(void) { // mount bind private_homedir on top of homedir if (arg_debug) printf("Mount-bind %s on top of %s\n", private_homedir, homedir); - // get a file descriptor for private_homedir, fails if there is any symlink - int fd = safe_fd(private_homedir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); - if (fd == -1) + // get file descriptors for homedir and private_homedir, fails if there is any symlink + int src = safe_fd(private_homedir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); + if (src == -1) + errExit("safe_fd"); + int dst = safe_fd(homedir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); + if (dst == -1) errExit("safe_fd"); // check if new home directory is owned by the user struct stat s; - if (fstat(fd, &s) == -1) + if (fstat(src, &s) == -1) errExit("fstat"); if (s.st_uid != getuid()) { fprintf(stderr, "Error: private directory is not owned by the current user\n"); @@ -253,17 +256,27 @@ void fs_private_homedir(void) { } if ((S_IRWXU & s.st_mode) != S_IRWXU) fwarning("no full permissions on private directory\n"); - // mount via the link in /proc/self/fd - char *proc; - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) + // mount via the links in /proc/self/fd + 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(proc, homedir, NULL, MS_NOSUID | MS_NODEV | MS_BIND | MS_REC, NULL) < 0) + if (mount(proc_src, proc_dst, NULL, MS_NOSUID | MS_NODEV | MS_BIND | MS_REC, NULL) < 0) errExit("mount bind"); - free(proc); - close(fd); - - fs_logger3("mount-bind", private_homedir, cfg.homedir); - fs_logger2("whitelist", cfg.homedir); + free(proc_src); + free(proc_dst); + close(src); + close(dst); + // check /proc/self/mountinfo to confirm the mount is ok + MountData *mptr = get_last_mount(); + size_t len = strlen(homedir); + if (strncmp(mptr->dir, homedir, len) != 0 || + (*(mptr->dir + len) != '\0' && *(mptr->dir + len) != '/')) + errLogExit("invalid private mount"); + + fs_logger3("mount-bind", private_homedir, homedir); + fs_logger2("whitelist", homedir); // preserve mode and ownership // if (chown(homedir, s.st_uid, s.st_gid) == -1) // errExit("mount-bind chown"); @@ -310,13 +323,13 @@ void fs_private(void) { int aflag = store_asoundrc(); // mask /home - if (arg_debug) - printf("Mounting a new /home directory\n"); if (u == 0 && arg_allusers) // allow --allusers when starting the sandbox as root ; else { + if (arg_debug) + printf("Mounting a new /home directory\n"); if (arg_allusers) - fwarning("--allusers disabled by --private or --whitelist\n"); + fwarning("allusers option disabled by private or whitelist option\n"); if (mount("tmpfs", "/home", "tmpfs", MS_NOSUID | MS_NODEV | MS_NOEXEC | MS_STRICTATIME, "mode=755,gid=0") < 0) errExit("mounting home directory"); fs_logger("tmpfs /home"); @@ -330,19 +343,24 @@ void fs_private(void) { fs_logger("tmpfs /root"); if (u != 0) { - // create /home/user - if (arg_debug) - printf("Create a new user directory\n"); - if (mkdir(homedir, S_IRWXU) == -1) { - if (mkpath_as_root(homedir) == -1) - errExit("mkpath"); - if (mkdir(homedir, S_IRWXU) == -1 && errno != EEXIST) - errExit("mkdir"); + if (strncmp(homedir, "/home/", 6) == 0) { + // create /home/user + if (arg_debug) + printf("Create a new user directory\n"); + if (mkdir(homedir, S_IRWXU) == -1) { + if (mkpath_as_root(homedir) == -1) + errExit("mkpath"); + if (mkdir(homedir, S_IRWXU) == -1 && errno != EEXIST) + errExit("mkdir"); + } + if (chown(homedir, u, g) < 0) + errExit("chown"); + fs_logger2("mkdir", homedir); + fs_logger2("tmpfs", homedir); } - if (chown(homedir, u, g) < 0) - errExit("chown"); - fs_logger2("mkdir", homedir); - fs_logger2("tmpfs", homedir); + else + // user directory is outside /home, mask it as well + fs_tmpfs(homedir, 1); } skel(homedir, u, g); @@ -528,8 +546,20 @@ void fs_private_home_list(void) { if (arg_debug) printf("Mount-bind %s on top of %s\n", RUN_HOME_DIR, homedir); - if (mount(RUN_HOME_DIR, homedir, NULL, MS_BIND|MS_REC, NULL) < 0) + int fd = safe_fd(homedir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); + if (fd == -1) + errExit("safe_fd"); + char *proc; + if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) + errExit("asprintf"); + if (mount(RUN_HOME_DIR, proc, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); + free(proc); + close(fd); + // check /proc/self/mountinfo to confirm the mount is ok + MountData *mptr = get_last_mount(); + if (strcmp(mptr->dir, homedir) != 0 || strcmp(mptr->fstype, "tmpfs") != 0) + errLogExit("invalid private-home mount"); fs_logger2("tmpfs", homedir); if (uid != 0) { diff --git a/src/firejail/fs_whitelist.c b/src/firejail/fs_whitelist.c index c8fa8c72b..a950d1124 100644 --- a/src/firejail/fs_whitelist.c +++ b/src/firejail/fs_whitelist.c @@ -710,8 +710,16 @@ void fs_whitelist(void) { if (stat(cfg.homedir, &s) == 0) { // keep a copy of real home dir in RUN_WHITELIST_HOME_USER_DIR mkdir_attr(RUN_WHITELIST_HOME_USER_DIR, 0755, getuid(), getgid()); - if (mount(cfg.homedir, RUN_WHITELIST_HOME_USER_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) + int fd = safe_fd(cfg.homedir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); + if (fd == -1) + errExit("safe_fd"); + char *proc; + if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) + errExit("asprintf"); + if (mount(proc, RUN_WHITELIST_HOME_USER_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); + free(proc); + close(fd); // mount a tmpfs and initialize /home/user, overrides --allusers fs_private(); diff --git a/src/firejail/restrict_users.c b/src/firejail/restrict_users.c index 5c5ace90b..ee2e497cb 100644 --- a/src/firejail/restrict_users.c +++ b/src/firejail/restrict_users.c @@ -25,9 +25,13 @@ #include #include #include -#include #include +#include +#ifndef O_PATH +# define O_PATH 010000000 +#endif + #define MAXBUF 1024 // linked list of users @@ -79,8 +83,16 @@ static void sanitize_home(void) { errExit("mkdir"); // keep a copy of the user home directory - if (mount(cfg.homedir, RUN_WHITELIST_HOME_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) + int fd = safe_fd(cfg.homedir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); + if (fd == -1) + errExit("safe_fd"); + char *proc; + if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) + errExit("asprintf"); + if (mount(proc, RUN_WHITELIST_HOME_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); + free(proc); + close(fd); // mount tmpfs in the new home if (mount("tmpfs", "/home", "tmpfs", MS_NOSUID | MS_NODEV | MS_STRICTATIME, "mode=755,gid=0") < 0) -- cgit v1.2.3-54-g00ecf From 9b2bbfcc3184c26b8dda926b2709822015742093 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Tue, 9 Jul 2019 17:38:46 +0200 Subject: main.c: define O_PATH (CentOS 6 fix) --- src/firejail/main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/firejail/main.c b/src/firejail/main.c index 03956ce4d..d00147c74 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -38,6 +37,11 @@ #include #include +#include +#ifndef O_PATH +#define O_PATH 010000000 +#endif + #ifdef __ia64__ /* clone(2) has a different interface on ia64, as it needs to know the size of the stack */ -- cgit v1.2.3-54-g00ecf