Merge pull request #2845 from smitsohu/homedir2

improve support for home directories outside /home

8 files changed, 135 insertions, 45 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/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/main.c b/src/firejail/main.c
index 1e2488062..d00147c74 100644
--- a/src/firejail/main.c
+++ b/src/firejail/main.c
@@ -26,7 +26,6 @@
 #include <sys/mount.h>
 #include <sys/wait.h>
 #include <sys/stat.h>
-#include <fcntl.h>
 #include <dirent.h>
 #include <pwd.h>
 #include <errno.h>
@@ -38,6 +37,11 @@
 #include <net/if.h>
 #include <sys/utsname.h>
 
+#include <fcntl.h>
+#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 */
@@ -242,6 +246,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 +304,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 +962,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();
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 <fnmatch.h>
 #include <glob.h>
 #include <dirent.h>
-#include <fcntl.h>
 #include <errno.h>
 
+#include <fcntl.h>
+#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)