Merge pull request #2982 from smitsohu/chroot

Move chroot entirely from path based to file descriptor based mounts

5 files changed, 299 insertions, 276 deletions

diff --git a/src/firejail/chroot.c b/src/firejail/chroot.c
new file mode 100644
index 000000000..f5bb11a76
--- /dev/null
+++ b/src/firejail/chroot.c
@@ -0,0 +1,280 @@
+/*
+ * Copyright (C) 2014-2019 Firejail Authors
+ *
+ * This file is part of firejail project
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifdef HAVE_CHROOT
+#include "firejail.h"
+#include <sys/mount.h>
+#include <sys/sendfile.h>
+#include <errno.h>
+
+#include <fcntl.h>
+#ifndef O_PATH
+# define O_PATH 010000000
+#endif
+
+
+// exit if error
+void fs_check_chroot_dir(void) {
+        EUID_ASSERT();
+        assert(cfg.chrootdir);
+        if (strstr(cfg.chrootdir, "..") ||
+        is_link(cfg.chrootdir) ||
+        !is_dir(cfg.chrootdir))
+                goto errout;
+
+        // check chroot dirname exists, chrooting into the root directory is not allowed
+        char *rpath = realpath(cfg.chrootdir, NULL);
+        if (rpath == NULL || strcmp(rpath, "/") == 0)
+                goto errout;
+
+        char *overlay;
+        if (asprintf(&overlay, "%s/.firejail", cfg.homedir) == -1)
+                errExit("asprintf");
+        if (strncmp(rpath, overlay, strlen(overlay)) == 0) {
+                fprintf(stderr, "Error: invalid chroot directory: no directories in %s are allowed\n", overlay);
+                exit(1);
+        }
+        free(overlay);
+        cfg.chrootdir = rpath;
+        return;
+
+errout:
+        fprintf(stderr, "Error: invalid chroot directory %s\n", cfg.chrootdir);
+        exit(1);
+}
+
+// copy /etc/resolv.conf in chroot directory
+static void copy_resolvconf(int parentfd) {
+        int in = open("/etc/resolv.conf", O_RDONLY|O_CLOEXEC);
+        if (in == -1) {
+                fwarning("/etc/resolv.conf not initialized\n");
+                return;
+        }
+        struct stat src;
+        if (fstat(in, &src) == -1)
+                errExit("fstat");
+        // try to detect if resolv.conf has been bind mounted into the chroot
+        // do nothing in this case in order to not unlink the real file
+        struct stat dst;
+        if (fstatat(parentfd, "etc/resolv.conf", &dst, 0) == 0) {
+                if (src.st_dev == dst.st_dev && src.st_ino == dst.st_ino) {
+                        close(in);
+                        return;
+                }
+        }
+        if (arg_debug)
+                printf("Updating /etc/resolv.conf in chroot\n");
+        unlinkat(parentfd, "etc/resolv.conf", 0);
+        int out = openat(parentfd, "etc/resolv.conf", O_CREAT|O_WRONLY|O_CLOEXEC, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH);
+        if (out == -1)
+                errExit("open");
+        if (sendfile(out, in, NULL, src.st_size) == -1)
+                errExit("sendfile");
+        close(in);
+        close(out);
+}
+
+// exit if error
+static void check_subdir(int parentfd, const char *subdir, int check_writable) {
+        assert(subdir);
+        struct stat s;
+        if (fstatat(parentfd, subdir, &s, AT_SYMLINK_NOFOLLOW) != 0) {
+                fprintf(stderr, "Error: cannot find /%s in chroot directory\n", subdir);
+                exit(1);
+        }
+        if (!S_ISDIR(s.st_mode)) {
+                if (S_ISLNK(s.st_mode))
+                        fprintf(stderr, "Error: chroot /%s is a symbolic link\n", subdir);
+                else
+                        fprintf(stderr, "Error: chroot /%s is not a directory\n", subdir);
+                exit(1);
+        }
+        if (s.st_uid != 0) {
+                fprintf(stderr, "Error: chroot /%s should owned by root\n", subdir);
+                exit(1);
+        }
+        if (check_writable && ((S_IWGRP|S_IWOTH) & s.st_mode) != 0) {
+                fprintf(stderr, "Error: only root user should be given write permission on chroot /%s\n", subdir);
+                exit(1);
+        }
+}
+
+// chroot into an existing directory; mount existing /dev and update /etc/resolv.conf
+void fs_chroot(const char *rootdir) {
+        assert(rootdir);
+
+        // fails if there is any symlink or if rootdir is not a directory
+        int parentfd = safe_fd(rootdir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC);
+        if (parentfd == -1)
+                errExit("safe_fd");
+        // rootdir has to be owned by root and is not allowed to be generally writable,
+        // this also excludes /tmp and friends
+        struct stat s;
+        if (fstat(parentfd, &s) == -1)
+                errExit("fstat");
+        if (s.st_uid != 0) {
+                fprintf(stderr, "Error: chroot directory should be owned by root\n");
+                exit(1);
+        }
+        if (((S_IWGRP|S_IWOTH) & s.st_mode) != 0) {
+                fprintf(stderr, "Error: only root user should be given write permission on chroot directory\n");
+                exit(1);
+        }
+        // check chroot subdirectories; /tmp/.X11-unix and /run are treated separately
+        check_subdir(parentfd, "dev", 0);
+        check_subdir(parentfd, "etc", 1);
+        check_subdir(parentfd, "proc", 0);
+        check_subdir(parentfd, "tmp", 0);
+        check_subdir(parentfd, "var/tmp", 0);
+
+        // mount-bind a /dev in rootdir
+        if (arg_debug)
+                printf("Mounting /dev on chroot /dev\n");
+        // open chroot /dev to get a file descriptor,
+        // then use this descriptor as a mount target
+        int fd = openat(parentfd, "dev", O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC);
+        if (fd == -1)
+                errExit("open");
+        char *proc;
+        if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1)
+                errExit("asprintf");
+        if (mount("/dev", proc, NULL, MS_BIND|MS_REC, NULL) < 0)
+                errExit("mounting /dev");
+        free(proc);
+        close(fd);
+
+        // mount a brand new proc filesystem
+        if (arg_debug)
+                printf("Mounting /proc filesystem on chroot /proc\n");
+        fd = openat(parentfd, "proc", O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC);
+        if (fd == -1)
+                errExit("open");
+        if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1)
+                errExit("asprintf");
+        if (mount("proc", proc, "proc", MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_REC, NULL) < 0)
+                errExit("mounting /proc");
+        free(proc);
+        close(fd);
+
+        // x11
+        if (getenv("FIREJAIL_X11")) {
+                if (arg_debug)
+                        printf("Mounting /tmp/.X11-unix on chroot /tmp/.X11-unix\n");
+                check_subdir(parentfd, "tmp/.X11-unix", 0);
+                fd = openat(parentfd, "tmp/.X11-unix", O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC);
+                if (fd == -1)
+                        errExit("open");
+                if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1)
+                        errExit("asprintf");
+                if (mount("/tmp/.X11-unix", proc, NULL, MS_BIND|MS_REC, NULL) < 0)
+                        errExit("mounting /tmp/.X11-unix");
+                free(proc);
+                close(fd);
+        }
+
+        // some older distros don't have a /run directory, create one by default
+        if (mkdirat(parentfd, "run", 0755) == -1 && errno != EEXIST)
+                errExit("mkdir");
+        check_subdir(parentfd, "run", 1);
+
+        // create /run/firejail directory in chroot
+        if (mkdirat(parentfd, RUN_FIREJAIL_DIR+1, 0755) == -1 && errno != EEXIST)
+                errExit("mkdir");
+        check_subdir(parentfd, RUN_FIREJAIL_DIR+1, 1);
+
+        // create /run/firejail/lib directory in chroot
+        if (mkdirat(parentfd, RUN_FIREJAIL_LIB_DIR+1, 0755) == -1 && errno != EEXIST)
+                errExit("mkdir");
+        check_subdir(parentfd, RUN_FIREJAIL_LIB_DIR+1, 1);
+        // mount lib directory into the chroot
+        fd = openat(parentfd, RUN_FIREJAIL_LIB_DIR+1, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC);
+        if (fd == -1)
+                errExit("open");
+        if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1)
+                errExit("asprintf");
+        if (mount(RUN_FIREJAIL_LIB_DIR, proc, NULL, MS_BIND|MS_REC, NULL) < 0)
+                errExit("mount bind");
+        free(proc);
+        close(fd);
+
+        // create /run/firejail/mnt directory in chroot
+        if (mkdirat(parentfd, RUN_MNT_DIR+1, 0755) == -1 && errno != EEXIST)
+                errExit("mkdir");
+        check_subdir(parentfd, RUN_MNT_DIR+1, 1);
+        // mount the current mnt directory into the chroot
+        fd = openat(parentfd, RUN_MNT_DIR+1, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC);
+        if (fd == -1)
+                errExit("open");
+        if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1)
+                errExit("asprintf");
+        if (mount(RUN_MNT_DIR, proc, NULL, MS_BIND|MS_REC, NULL) < 0)
+                errExit("mount bind");
+        free(proc);
+        close(fd);
+
+        // update chroot resolv.conf
+        copy_resolvconf(parentfd);
+
+#ifdef HAVE_GCOV
+        __gcov_flush();
+#endif
+        // create /run/firejail/mnt/oroot
+        char *oroot = RUN_OVERLAY_ROOT;
+        if (mkdir(oroot, 0755) == -1)
+                errExit("mkdir");
+        // mount the chroot dir on top of /run/firejail/mnt/oroot in order to reuse the apparmor rules for overlay
+        if (asprintf(&proc, "/proc/self/fd/%d", parentfd) == -1)
+                errExit("asprintf");
+        if (mount(proc, oroot, NULL, MS_BIND|MS_REC, NULL) < 0)
+                errExit("mounting rootdir oroot");
+        free(proc);
+        close(parentfd);
+        // chroot into the new directory
+        if (arg_debug)
+                printf("Chrooting into %s\n", rootdir);
+        if (chroot(oroot) < 0)
+                errExit("chroot");
+
+        // create all other /run/firejail files and directories
+        preproc_build_firejail_dir();
+
+        // update /var directory in order to support multiple sandboxes running on the same root directory
+        //      if (!arg_private_dev)
+        //              fs_dev_shm();
+        fs_var_lock();
+        if (!arg_keep_var_tmp)
+                fs_var_tmp();
+        if (!arg_writable_var_log)
+                fs_var_log();
+
+        fs_var_lib();
+        fs_var_cache();
+        fs_var_utmp();
+        fs_machineid();
+
+        // don't leak user information
+        restrict_users();
+
+        // when starting as root, firejail config is not disabled;
+        if (getuid() != 0)
+                disable_config();
+}
+
+#endif // HAVE_CHROOT
diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index a6377261f..fdbeb4691 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -386,18 +386,22 @@ void fs_remount(const char *dir, OPERATION op, unsigned check_mnt);
 void fs_remount_rec(const char *dir, OPERATION op, unsigned check_mnt);
 // mount /proc and /sys directories
 void fs_proc_sys_dev_boot(void);
+// blacklist firejail configuration and runtime directories
+void disable_config(void);
 // build a basic read-only filesystem
 void fs_basic_fs(void);
 // mount overlayfs on top of / directory
 char *fs_check_overlay_dir(const char *subdirname, int allow_reuse);
 void fs_overlayfs(void);
-// chroot into an existing directory; mount exiting /dev and update /etc/resolv.conf
-void fs_chroot(const char *rootdir);
-void fs_check_chroot_dir(const char *rootdir);
 void fs_private_tmp(void);
 void fs_private_cache(void);
 void fs_mnt(const int enforce);
 
+// chroot.c
+// chroot into an existing directory; mount existing /dev and update /etc/resolv.conf
+void fs_check_chroot_dir(void);
+void fs_chroot(const char *rootdir);
+
 // profile.c
 // find and read the profile specified by name from dir directory
 int profile_find_firejail(const char *name, int add_ext);
diff --git a/src/firejail/fs.c b/src/firejail/fs.c
index ce2ca5e2a..f2639f318 100644
--- a/src/firejail/fs.c
+++ b/src/firejail/fs.c
@@ -28,6 +28,7 @@
 #include <dirent.h>
 #include <errno.h>
 
+
 #include <fcntl.h>
 #ifndef O_PATH
 # define O_PATH 010000000
@@ -696,8 +697,8 @@ void fs_proc_sys_dev_boot(void) {
         }
 }
 
-// disable firejail configuration in /etc/firejail and in ~/.config/firejail
-static void disable_config(void) {
+// disable firejail configuration in ~/.config/firejail
+void disable_config(void) {
         struct stat s;
 
         char *fname;
@@ -1123,256 +1124,6 @@ void fs_overlayfs(void) {
 }
 #endif
 
-
-#ifdef HAVE_CHROOT
-// exit if error
-static void fs_check_chroot_subdir(const char *subdir, int parentfd, int check_writable) {
-        assert(subdir);
-        int fd = openat(parentfd, subdir, O_PATH|O_CLOEXEC);
-        if (fd == -1) {
-                if (errno == ENOENT)
-                        fprintf(stderr, "Error: cannot find /%s in chroot directory\n", subdir);
-                else {
-                        perror("open");
-                        fprintf(stderr, "Error: cannot open /%s in chroot directory\n", subdir);
-                }
-                exit(1);
-        }
-        struct stat s;
-        if (fstat(fd, &s) == -1)
-                errExit("fstat");
-        close(fd);
-        if (!S_ISDIR(s.st_mode) || s.st_uid != 0) {
-                fprintf(stderr, "Error: chroot /%s should be a directory owned by root\n", subdir);
-                exit(1);
-        }
-        if (check_writable && ((S_IWGRP|S_IWOTH) & s.st_mode) != 0) {
-                fprintf(stderr, "Error: only root user should be given write permission on chroot /%s\n", subdir);
-                exit(1);
-        }
-}
-
-// exit if error
-void fs_check_chroot_dir(const char *rootdir) {
-        EUID_ASSERT();
-        assert(rootdir);
-
-        char *overlay;
-        if (asprintf(&overlay, "%s/.firejail", cfg.homedir) == -1)
-                errExit("asprintf");
-        if (strncmp(rootdir, overlay, strlen(overlay)) == 0) {
-                fprintf(stderr, "Error: invalid chroot directory: no directories in %s are allowed\n", overlay);
-                exit(1);
-        }
-        free(overlay);
-
-        // fails if there is any symlink or if rootdir is not a directory
-        int parentfd = safe_fd(rootdir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC);
-        if (parentfd == -1) {
-                fprintf(stderr, "Error: invalid chroot directory %s\n", rootdir);
-                exit(1);
-        }
-        // rootdir has to be owned by root and is not allowed to be generally writable,
-        // this also excludes /tmp, /var/tmp and such
-        struct stat s;
-        if (fstat(parentfd, &s) == -1)
-                errExit("fstat");
-        if (s.st_uid != 0) {
-                fprintf(stderr, "Error: chroot directory should be owned by root\n");
-                exit(1);
-        }
-        if (((S_IWGRP|S_IWOTH) & s.st_mode) != 0) {
-                fprintf(stderr, "Error: only root user should be given write permission on chroot directory\n");
-                exit(1);
-        }
-
-        // check subdirectories in rootdir
-        fs_check_chroot_subdir("dev", parentfd, 0);
-        fs_check_chroot_subdir("etc", parentfd, 1);
-        fs_check_chroot_subdir("proc", parentfd, 0);
-        fs_check_chroot_subdir("tmp", parentfd, 0);
-        fs_check_chroot_subdir("var/tmp", parentfd, 0);
-
-        // there should be no checking on <chrootdir>/etc/resolv.conf
-        // the file is replaced with the real /etc/resolv.conf anyway
-#if 0
-        if (asprintf(&name, "%s/etc/resolv.conf", rootdir) == -1)
-                errExit("asprintf");
-        if (stat(name, &s) == 0) {
-                if (s.st_uid != 0) {
-                        fprintf(stderr, "Error: chroot /etc/resolv.conf should be owned by root\n");
-                        exit(1);
-                }
-        }
-        else {
-                fprintf(stderr, "Error: chroot /etc/resolv.conf not found\n");
-                exit(1);
-        }
-        // on Arch /etc/resolv.conf could be a symlink to /run/systemd/resolve/resolv.conf
-        // on Ubuntu 17.04 /etc/resolv.conf could be a symlink to /run/resolveconf/resolv.conf
-        if (is_link(name)) {
-                // check the link points in chroot
-                char *rname = realpath(name, NULL);
-                if (!rname || strncmp(rname, rootdir, strlen(rootdir)) != 0) {
-                        fprintf(stderr, "Error: chroot /etc/resolv.conf is pointing outside chroot\n");
-                        exit(1);
-                }
-        }
-        free(name);
-#endif
-
-        // check x11 socket directory
-        if (getenv("FIREJAIL_X11"))
-                fs_check_chroot_subdir("tmp/.X11-unix", parentfd, 0);
-
-        close(parentfd);
-}
-
-// chroot into an existing directory; mount exiting /dev and update /etc/resolv.conf
-void fs_chroot(const char *rootdir) {
-        assert(rootdir);
-
-        // mount-bind a /dev in rootdir
-        char *newdev;
-        if (asprintf(&newdev, "%s/dev", rootdir) == -1)
-                errExit("asprintf");
-        if (arg_debug)
-                printf("Mounting /dev on %s\n", newdev);
-        if (mount("/dev", newdev, NULL, MS_BIND|MS_REC, NULL) < 0)
-                errExit("mounting /dev");
-        free(newdev);
-
-        // mount a new proc filesystem
-        char *newproc;
-        if (asprintf(&newproc, "%s/proc", rootdir) == -1)
-                errExit("asprintf");
-        if (arg_debug)
-                printf("Mounting /proc filesystem on %s\n", newproc);
-        if (mount("proc", newproc, "proc", MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_REC, NULL) < 0)
-                errExit("mounting /proc");
-        free(newproc);
-
-        // x11
-        if (getenv("FIREJAIL_X11")) {
-                char *newx11;
-                if (asprintf(&newx11, "%s/tmp/.X11-unix", rootdir) == -1)
-                        errExit("asprintf");
-                if (arg_debug)
-                        printf("Mounting /tmp/.X11-unix on %s\n", newx11);
-                if (mount("/tmp/.X11-unix", newx11, NULL, MS_BIND|MS_REC, NULL) < 0)
-                        errExit("mounting /tmp/.X11-unix");
-                free(newx11);
-        }
-
-        // some older distros don't have a /run directory
-        // create one by default
-        char *rundir;
-        if (asprintf(&rundir, "%s/run", rootdir) == -1)
-                errExit("asprintf");
-        struct stat s;
-        if (lstat(rundir, &s) == 0) {
-                if (S_ISLNK(s.st_mode)) {
-                        fprintf(stderr, "Error: chroot /run is a symbolic link\n");
-                        exit(1);
-                }
-                if (!S_ISDIR(s.st_mode) || s.st_uid != 0) {
-                        fprintf(stderr, "Error: chroot /run should be a directory owned by root\n");
-                        exit(1);
-                }
-                if (((S_IWGRP|S_IWOTH) & s.st_mode) != 0) {
-                        fprintf(stderr, "Error: only root user should be given write permission on chroot /run\n");
-                        exit(1);
-                }
-        }
-        else {
-                // several sandboxes could race to create /run
-                if (mkdir(rundir, 0755) == -1 && errno != EEXIST)
-                        errExit("mkdir");
-                ASSERT_PERMS(rundir, 0, 0, 0755);
-        }
-        free(rundir);
-
-        // create /run/firejail directory in chroot
-        if (asprintf(&rundir, "%s/run/firejail", rootdir) == -1)
-                errExit("asprintf");
-        if (mkdir(rundir, 0755) == -1 && errno != EEXIST)
-                errExit("mkdir");
-        ASSERT_PERMS(rundir, 0, 0, 0755);
-        free(rundir);
-
-        // create /run/firejail/lib directory in chroot and mount it
-        if (asprintf(&rundir, "%s%s", rootdir, RUN_FIREJAIL_LIB_DIR) == -1)
-                errExit("asprintf");
-        if (mkdir(rundir, 0755) == -1 && errno != EEXIST)
-                errExit("mkdir");
-        ASSERT_PERMS(rundir, 0, 0, 0755);
-        if (mount(RUN_FIREJAIL_LIB_DIR, rundir, NULL, MS_BIND|MS_REC, NULL) < 0)
-                errExit("mount bind");
-        free(rundir);
-
-        // create /run/firejail/mnt directory in chroot and mount the current one
-        if (asprintf(&rundir, "%s%s", rootdir, RUN_MNT_DIR) == -1)
-                errExit("asprintf");
-        if (mkdir(rundir, 0755) == -1 && errno != EEXIST)
-                errExit("mkdir");
-        ASSERT_PERMS(rundir, 0, 0, 0755);
-        if (mount(RUN_MNT_DIR, rundir, NULL, MS_BIND|MS_REC, NULL) < 0)
-                errExit("mount bind");
-        free(rundir);
-
-        // copy /etc/resolv.conf in chroot directory
-        char *fname;
-        if (asprintf(&fname, "%s/etc/resolv.conf", rootdir) == -1)
-                errExit("asprintf");
-        if (arg_debug)
-                printf("Updating /etc/resolv.conf in %s\n", fname);
-        unlink(fname);
-        if (copy_file("/etc/resolv.conf", fname, 0, 0, 0644) == -1) // root needed
-                fwarning("/etc/resolv.conf not initialized\n");
-        free(fname);
-
-        // chroot into the new directory
-#ifdef HAVE_GCOV
-        __gcov_flush();
-#endif
-        // mount the chroot dir on top of /run/firejail/mnt/oroot in order to reuse the apparmor rules for overlay
-        // and chroot into this new directory
-        if (arg_debug)
-                printf("Chrooting into %s\n", rootdir);
-        char *oroot = RUN_OVERLAY_ROOT;
-        if (mkdir(oroot, 0755) == -1)
-                errExit("mkdir");
-        if (mount(rootdir, oroot, NULL, MS_BIND|MS_REC, NULL) < 0)
-                errExit("mounting rootdir oroot");
-        if (chroot(oroot) < 0)
-                errExit("chroot");
-
-        // create all other /run/firejail files and directories
-        preproc_build_firejail_dir();
-
-        // update /var directory in order to support multiple sandboxes running on the same root directory
-//              if (!arg_private_dev)
-//                      fs_dev_shm();
-        fs_var_lock();
-        if (!arg_keep_var_tmp)
-                fs_var_tmp();
-        if (!arg_writable_var_log)
-                fs_var_log();
-
-        fs_var_lib();
-        fs_var_cache();
-        fs_var_utmp();
-        fs_machineid();
-
-        // don't leak user information
-        restrict_users();
-
-        // when starting as root, firejail config is not disabled;
-        if (getuid() != 0)
-                disable_config();
-}
-#endif
-
 // this function is called from sandbox.c before blacklist/whitelist functions
 void fs_private_tmp(void) {
         // check XAUTHORITY file, KDE keeps it under /tmp
diff --git a/src/firejail/main.c b/src/firejail/main.c
index e8664e914..cbe3292ba 100644
--- a/src/firejail/main.c
+++ b/src/firejail/main.c
@@ -1659,12 +1659,14 @@ int main(int argc, char **argv) {
                                         fprintf(stderr, "Error: --chroot option is not available on Grsecurity systems\n");
                                         exit(1);
                                 }
-
-
-                                invalid_filename(argv[i] + 9, 0); // no globbing
-
                                 // extract chroot dirname
                                 cfg.chrootdir = argv[i] + 9;
+                                if (*cfg.chrootdir == '\0') {
+                                        fprintf(stderr, "Error: invalid chroot option\n");
+                                        exit(1);
+                                }
+                                invalid_filename(cfg.chrootdir, 0); // no globbing
+
                                 // if the directory starts with ~, expand the home directory
                                 if (*cfg.chrootdir == '~') {
                                         char *tmp;
@@ -1672,22 +1674,8 @@ int main(int argc, char **argv) {
                                                 errExit("asprintf");
                                         cfg.chrootdir = tmp;
                                 }
-
-                                if (strstr(cfg.chrootdir, "..") || is_link(cfg.chrootdir)) {
-                                        fprintf(stderr, "Error: invalid chroot directory %s\n", cfg.chrootdir);
-                                        return 1;
-                                }
-
-                                // check chroot dirname exists, don't allow "--chroot=/"
-                                char *rpath = realpath(cfg.chrootdir, NULL);
-                                if (rpath == NULL || strcmp(rpath, "/") == 0) {
-                                        fprintf(stderr, "Error: invalid chroot directory\n");
-                                        exit(1);
-                                }
-                                cfg.chrootdir = rpath;
-
-                                // check chroot directory structure
-                                fs_check_chroot_dir(cfg.chrootdir);
+                                // check chroot directory
+                                fs_check_chroot_dir();
                         }
                         else
                                 exit_err_feature("chroot");
diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c
index 288726d22..80b595a9f 100644
--- a/src/firejail/sandbox.c
+++ b/src/firejail/sandbox.c
@@ -578,7 +578,7 @@ static void enforce_filters(void) {
         force_nonewprivs = 1;
 
         // disable all capabilities
-        fmessage("\n**     Warning: dropping all Linux capabilities     **\n");
+        fmessage("\n**     Warning: dropping all Linux capabilities     **\n\n");
         arg_caps_drop_all = 1;
 
         // drop all supplementary groups; /etc/group file inside chroot