From 63e16bfcd9f79c63f3801f51df4840f74fa6f41b Mon Sep 17 00:00:00 2001 From: netblue30 Date: Sun, 13 Nov 2016 10:47:20 -0500 Subject: major cleanup and testing --- src/firejail/appimage.c | 8 +--- src/firejail/firejail.h | 1 + src/firejail/fs.c | 101 +++++++++++++----------------------------------- src/firejail/fs_bin.c | 6 +-- src/firejail/fs_dev.c | 32 +++------------ src/firejail/fs_etc.c | 6 +-- src/firejail/fs_var.c | 23 ++--------- src/firejail/join.c | 14 +++---- src/firejail/util.c | 36 +++++++++++++++++ 9 files changed, 80 insertions(+), 147 deletions(-) (limited to 'src') diff --git a/src/firejail/appimage.c b/src/firejail/appimage.c index 96c054048..a658173eb 100644 --- a/src/firejail/appimage.c +++ b/src/firejail/appimage.c @@ -94,14 +94,8 @@ void appimage_set(const char *appimage_path) { if (asprintf(&mntdir, "%s/.appimage-%u", RUN_FIREJAIL_APPIMAGE_DIR, getpid()) == -1) errExit("asprintf"); EUID_ROOT(); - if (mkdir(mntdir, 0700) == -1) { - fprintf(stderr, "Error: cannot create appimage mount point\n"); - exit(1); - } - if (set_perms(mntdir, getuid(), getgid(), 0700)) - errExit("set_perms"); + mkdir_attr(mntdir, 0700, getuid(), getgid()); EUID_USER(); - ASSERT_PERMS(mntdir, getuid(), getgid(), 0700); // mount char *mode; diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index 282271a64..d7ba539e6 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -461,6 +461,7 @@ void flush_stdin(void); void create_empty_dir_as_root(const char *dir, mode_t mode); void create_empty_file_as_root(const char *dir, mode_t mode); int set_perms(const char *fname, uid_t uid, gid_t gid, mode_t mode); +void mkdir_attr(const char *fname, mode_t mode, uid_t uid, gid_t gid); // fs_var.c void fs_var_log(void); // mounting /var/log diff --git a/src/firejail/fs.c b/src/firejail/fs.c index 3a2fd8c38..7ff7e3c59 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -247,21 +247,13 @@ void fs_blacklist(void) { // process bind command if (strncmp(entry->data, "bind ", 5) == 0) { + struct stat s; char *dname1 = entry->data + 5; char *dname2 = split_comma(dname1); - if (dname2 == NULL) { - fprintf(stderr, "Error: second directory missing in bind command\n"); - entry = entry->next; - continue; - } - struct stat s; - if (stat(dname1, &s) == -1) { - fprintf(stderr, "Error: cannot find %s for bind command\n", dname1); - entry = entry->next; - continue; - } - if (stat(dname2, &s) == -1) { - fprintf(stderr, "Error: cannot find %s for bind command\n", dname2); + if (dname2 == NULL || + stat(dname1, &s) == -1 || + stat(dname2, &s) == -1) { + fprintf(stderr, "Error: invalid bind command, directory missing\n"); entry = entry->next; continue; } @@ -410,10 +402,9 @@ void fs_rdonly(const char *dir) { int rv = stat(dir, &s); if (rv == 0) { // mount --bind /bin /bin - if (mount(dir, dir, NULL, MS_BIND|MS_REC, NULL) < 0) - errExit("mount read-only"); // mount --bind -o remount,ro /bin - if (mount(NULL, dir, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY|MS_REC, NULL) < 0) + if (mount(dir, dir, NULL, MS_BIND|MS_REC, NULL) < 0 || + mount(NULL, dir, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY|MS_REC, NULL) < 0) errExit("mount read-only"); fs_logger2("read-only", dir); } @@ -428,15 +419,15 @@ static void fs_rdwr(const char *dir) { // if the file is outside /home directory, allow only root user uid_t u = getuid(); if (u != 0 && s.st_uid != u) { - fprintf(stderr, "Warning: you are not allowed to change %s to read-write\n", dir); + if (!arg_quiet) + fprintf(stderr, "Warning: you are not allowed to change %s to read-write\n", dir); return; } // mount --bind /bin /bin - if (mount(dir, dir, NULL, MS_BIND|MS_REC, NULL) < 0) - errExit("mount read-write"); // mount --bind -o remount,rw /bin - if (mount(NULL, dir, NULL, MS_BIND|MS_REMOUNT|MS_REC, NULL) < 0) + if (mount(dir, dir, NULL, MS_BIND|MS_REC, NULL) < 0 || + mount(NULL, dir, NULL, MS_BIND|MS_REMOUNT|MS_REC, NULL) < 0) errExit("mount read-write"); fs_logger2("read-write", dir); } @@ -449,37 +440,16 @@ void fs_noexec(const char *dir) { int rv = stat(dir, &s); if (rv == 0) { // mount --bind /bin /bin - if (mount(dir, dir, NULL, MS_BIND|MS_REC, NULL) < 0) - errExit("mount noexec"); // mount --bind -o remount,ro /bin - if (mount(NULL, dir, NULL, MS_BIND|MS_REMOUNT|MS_NOEXEC|MS_NODEV|MS_NOSUID|MS_REC, NULL) < 0) - errExit("mount read-only"); + if (mount(dir, dir, NULL, MS_BIND|MS_REC, NULL) < 0 || + mount(NULL, dir, NULL, MS_BIND|MS_REMOUNT|MS_NOEXEC|MS_NODEV|MS_NOSUID|MS_REC, NULL) < 0) + errExit("mount noexec"); fs_logger2("noexec", dir); } } -void fs_rdonly_noexit(const char *dir) { - assert(dir); - // check directory exists - struct stat s; - int rv = stat(dir, &s); - if (rv == 0) { - int merr = 0; - // mount --bind /bin /bin - if (mount(dir, dir, NULL, MS_BIND|MS_REC, NULL) < 0) - merr = 1; - // mount --bind -o remount,ro /bin - if (mount(NULL, dir, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY|MS_REC, NULL) < 0) - merr = 1; - if (merr) - fprintf(stderr, "Warning: cannot mount %s read-only\n", dir); - else - fs_logger2("read-only", dir); - } -} - // mount /proc and /sys directories void fs_proc_sys_dev_boot(void) { if (arg_debug) @@ -489,10 +459,8 @@ void fs_proc_sys_dev_boot(void) { fs_logger("remount /proc"); // remount /proc/sys readonly - if (mount("/proc/sys", "/proc/sys", NULL, MS_BIND | MS_REC, NULL) < 0) - errExit("mounting /proc/sys"); - - if (mount(NULL, "/proc/sys", NULL, MS_BIND | MS_REMOUNT | MS_RDONLY | MS_REC, NULL) < 0) + if (mount("/proc/sys", "/proc/sys", NULL, MS_BIND | MS_REC, NULL) < 0 || + mount(NULL, "/proc/sys", NULL, MS_BIND | MS_REMOUNT | MS_RDONLY | MS_REC, NULL) < 0) errExit("mounting /proc/sys"); fs_logger("read-only /proc/sys"); @@ -646,12 +614,7 @@ char *fs_check_overlay_dir(const char *subdirname, int allow_reuse) { if (asprintf(&dirname, "%s/.firejail", cfg.homedir) == -1) errExit("asprintf"); if (stat(dirname, &s) == -1) { - /* coverity[toctou] */ - if (mkdir(dirname, 0700)) - errExit("mkdir"); - if (chmod(dirname, 0700) == -1) - errExit("chmod"); - ASSERT_PERMS(dirname, getuid(), getgid(), 0700); + mkdir_attr(dirname, 0700, 0, 0); } else if (is_link(dirname)) { fprintf(stderr, "Error: invalid ~/.firejail directory\n"); @@ -733,11 +696,7 @@ void fs_overlayfs(void) { char *oroot; if(asprintf(&oroot, "%s/oroot", RUN_MNT_DIR) == -1) errExit("asprintf"); - if (mkdir(oroot, 0755)) - errExit("mkdir"); - if (chmod(oroot, 0755) == -1) - errExit("chmod"); - ASSERT_PERMS(oroot, 0, 0, 0755); + mkdir_attr(oroot, 0755, 0, 0); struct stat s; char *basedir = RUN_MNT_DIR; @@ -766,11 +725,9 @@ void fs_overlayfs(void) { // no need to check arg_overlay_reuse if (stat(odiff, &s) != 0) { - if (mkdir(odiff, 0755)) - errExit("mkdir"); + mkdir_attr(odiff, 0755, 0, 0); } - - if (set_perms(odiff, 0, 0, 0755)) + else if (set_perms(odiff, 0, 0, 0755)) errExit("set_perms"); char *owork; @@ -779,11 +736,9 @@ void fs_overlayfs(void) { // no need to check arg_overlay_reuse if (stat(owork, &s) != 0) { - if (mkdir(owork, 0755)) - errExit("mkdir"); + mkdir_attr(owork, 0755, 0, 0); } - - if (set_perms(owork, 0, 0, 0755)) + else if (set_perms(owork, 0, 0, 0755)) errExit("chown"); // mount overlayfs @@ -839,11 +794,9 @@ void fs_overlayfs(void) { // no need to check arg_overlay_reuse if (stat(hdiff, &s) != 0) { - if (mkdir(hdiff, S_IRWXU | S_IRWXG | S_IRWXO)) - errExit("mkdir"); + mkdir_attr(hdiff, S_IRWXU | S_IRWXG | S_IRWXO, 0, 0); } - - if (set_perms(hdiff, 0, 0, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH)) + else if (set_perms(hdiff, 0, 0, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH)) errExit("set_perms"); if(asprintf(&hwork, "%s/hwork", basedir) == -1) @@ -851,11 +804,9 @@ void fs_overlayfs(void) { // no need to check arg_overlay_reuse if (stat(hwork, &s) != 0) { - if (mkdir(hwork, S_IRWXU | S_IRWXG | S_IRWXO)) - errExit("mkdir"); + mkdir_attr(hwork, S_IRWXU | S_IRWXG | S_IRWXO, 0, 0); } - - if (set_perms(hwork, 0, 0, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH)) + else if (set_perms(hwork, 0, 0, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH)) errExit("set_perms"); // no homedir in overlay so now mount another overlay for /home diff --git a/src/firejail/fs_bin.c b/src/firejail/fs_bin.c index c906e32c0..6cc1bf3ab 100644 --- a/src/firejail/fs_bin.c +++ b/src/firejail/fs_bin.c @@ -212,11 +212,7 @@ void fs_private_bin_list(void) { assert(private_list); // create /run/firejail/mnt/bin directory - if (mkdir(RUN_BIN_DIR, 0755) == -1) - errExit("mkdir"); - if (chmod(RUN_BIN_DIR, 0755) == -1) - errExit("chmod"); - ASSERT_PERMS(RUN_BIN_DIR, 0, 0, 0755); + mkdir_attr(RUN_BIN_DIR, 0755, 0, 0); // copy the list of files in the new etc directory // using a new child process without root privileges diff --git a/src/firejail/fs_dev.c b/src/firejail/fs_dev.c index ef5d67b55..d710e98f2 100644 --- a/src/firejail/fs_dev.c +++ b/src/firejail/fs_dev.c @@ -65,11 +65,7 @@ static void deventry_mount(void) { if (arg_debug) printf("mounting %s %s\n", dev[i].run_fname, (dir)? "directory": "file"); if (dir) { - if (mkdir(dev[i].dev_fname, 0755) == -1) - errExit("mkdir"); - if (chmod(dev[i].dev_fname, 0755) == -1) - errExit("chmod"); - ASSERT_PERMS(dev[i].dev_fname, 0, 0, 0755); + mkdir_attr(dev[i].dev_fname, 0755, 0, 0); } else { struct stat s; @@ -130,11 +126,7 @@ void fs_private_dev(void){ // create DRI_DIR // keep a copy of dev directory - if (mkdir(RUN_DEV_DIR, 0755) == -1) - errExit("mkdir"); - if (chmod(RUN_DEV_DIR, 0755) == -1) - errExit("chmod"); - ASSERT_PERMS(RUN_DEV_DIR, 0, 0, 0755); + mkdir_attr(RUN_DEV_DIR, 0755, 0, 0); if (mount("/dev", RUN_DEV_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mounting /dev/dri"); @@ -179,12 +171,7 @@ void fs_private_dev(void){ // create /dev/shm if (arg_debug) printf("Create /dev/shm directory\n"); - if (mkdir("/dev/shm", 01777) == -1) - errExit("mkdir"); - // mkdir sets only the file permission bits - if (chmod("/dev/shm", 01777) < 0) - errExit("chmod"); - ASSERT_PERMS("/dev/shm", 0, 0, 01777); + mkdir_attr("/dev/shm", 01777, 0, 0); fs_logger("mkdir /dev/shm"); // create devices @@ -206,11 +193,7 @@ void fs_private_dev(void){ #endif // pseudo-terminal - if (mkdir("/dev/pts", 0755) == -1) - errExit("mkdir"); - if (chmod("/dev/pts", 0755) == -1) - errExit("chmod"); - ASSERT_PERMS("/dev/pts", 0, 0, 0755); + mkdir_attr("/dev/pts", 0755, 0, 0); fs_logger("mkdir /dev/pts"); create_char_dev("/dev/pts/ptmx", 0666, 5, 2); //"mknod -m 666 /dev/pts/ptmx c 5 2"); fs_logger("mknod /dev/pts/ptmx"); @@ -260,12 +243,7 @@ void fs_dev_shm(void) { if (lnk) { if (!is_dir(lnk)) { // create directory - if (mkdir(lnk, 01777)) - errExit("mkdir"); - // mkdir sets only the file permission bits - if (chmod(lnk, 01777)) - errExit("chmod"); - ASSERT_PERMS(lnk, 0, 0, 01777); + mkdir_attr(lnk, 01777, 0, 0); } if (arg_debug) printf("Mounting tmpfs on %s on behalf of /dev/shm\n", lnk); diff --git a/src/firejail/fs_etc.c b/src/firejail/fs_etc.c index ebcde29a3..7e18840fd 100644 --- a/src/firejail/fs_etc.c +++ b/src/firejail/fs_etc.c @@ -132,11 +132,7 @@ void fs_private_etc_list(void) { } // create /run/firejail/mnt/etc directory - if (mkdir(RUN_ETC_DIR, 0755) == -1) - errExit("mkdir"); - if (chmod(RUN_ETC_DIR, 0755) == -1) - errExit("chmod"); - ASSERT_PERMS(RUN_ETC_DIR, 0, 0, 0755); + mkdir_attr(RUN_ETC_DIR, 0755, 0, 0); fs_logger("tmpfs /etc"); fs_logger_print(); // save the current log diff --git a/src/firejail/fs_var.c b/src/firejail/fs_var.c index 4ff00f3ba..ca50685ad 100644 --- a/src/firejail/fs_var.c +++ b/src/firejail/fs_var.c @@ -98,10 +98,7 @@ static void build_dirs(void) { // create directories under /var/log DirData *ptr = dirlist; while (ptr) { - if (mkdir(ptr->name, ptr->st_mode)) - errExit("mkdir"); - if (chown(ptr->name, ptr->st_uid, ptr->st_gid)) - errExit("chown"); + mkdir_attr(ptr->name, ptr->st_mode, ptr->st_uid, ptr->st_gid); fs_logger2("mkdir", ptr->name); ptr = ptr->next; } @@ -223,18 +220,10 @@ void fs_var_cache(void) { gid = p->pw_gid; } - int rv = mkdir("/var/cache/lighttpd/compress", 0755); - if (rv == -1) - errExit("mkdir"); - if (chown("/var/cache/lighttpd/compress", uid, gid) < 0) - errExit("chown"); + mkdir_attr("/var/cache/lighttpd/compress", 0755, uid, gid); fs_logger("mkdir /var/cache/lighttpd/compress"); - rv = mkdir("/var/cache/lighttpd/uploads", 0755); - if (rv == -1) - errExit("mkdir"); - if (chown("/var/cache/lighttpd/uploads", uid, gid) < 0) - errExit("chown"); + mkdir_attr("/var/cache/lighttpd/uploads", 0755, uid, gid); fs_logger("/var/cache/lighttpd/uploads"); } } @@ -268,11 +257,7 @@ void fs_var_lock(void) { if (lnk) { if (!is_dir(lnk)) { // create directory - if (mkdir(lnk, S_IRWXU|S_IRWXG|S_IRWXO)) - errExit("mkdir"); - if (chmod(lnk, S_IRWXU|S_IRWXG|S_IRWXO)) - errExit("chmod"); - ASSERT_PERMS(lnk, 0, 0, S_IRWXU|S_IRWXG|S_IRWXO); + mkdir_attr(lnk, S_IRWXU|S_IRWXG|S_IRWXO, 0, 0); } if (arg_debug) printf("Mounting tmpfs on %s on behalf of /var/lock\n", lnk); diff --git a/src/firejail/join.c b/src/firejail/join.c index 899166447..628002d35 100644 --- a/src/firejail/join.c +++ b/src/firejail/join.c @@ -229,15 +229,11 @@ void join(pid_t pid, int argc, char **argv, int index) { exit(1); } else { - if (join_namespace(pid, "ipc")) - exit(1); - if (join_namespace(pid, "net")) - exit(1); - if (join_namespace(pid, "pid")) - exit(1); - if (join_namespace(pid, "uts")) - exit(1); - if (join_namespace(pid, "mnt")) + if (join_namespace(pid, "ipc") || + join_namespace(pid, "net") || + join_namespace(pid, "pid") || + join_namespace(pid, "uts") || + join_namespace(pid, "mnt")) exit(1); } diff --git a/src/firejail/util.c b/src/firejail/util.c index 3424d8ab6..d928c6b42 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -731,4 +731,40 @@ int set_perms(const char *fname, uid_t uid, gid_t gid, mode_t mode) { return 0; } +void mkdir_attr(const char *fname, mode_t mode, uid_t uid, gid_t gid) { + assert(fname); + mode &= 07777; +#if 0 + printf("fname %s, uid %d, gid %d, mode %x - ", fname, uid, gid, (unsigned) mode); + if (S_ISLNK(mode)) + printf("l"); + else if (S_ISDIR(mode)) + printf("d"); + else if (S_ISCHR(mode)) + printf("c"); + else if (S_ISBLK(mode)) + printf("b"); + else if (S_ISSOCK(mode)) + printf("s"); + else + printf("-"); + printf( (mode & S_IRUSR) ? "r" : "-"); + printf( (mode & S_IWUSR) ? "w" : "-"); + printf( (mode & S_IXUSR) ? "x" : "-"); + printf( (mode & S_IRGRP) ? "r" : "-"); + printf( (mode & S_IWGRP) ? "w" : "-"); + printf( (mode & S_IXGRP) ? "x" : "-"); + printf( (mode & S_IROTH) ? "r" : "-"); + printf( (mode & S_IWOTH) ? "w" : "-"); + printf( (mode & S_IXOTH) ? "x" : "-"); + printf("\n"); +#endif + if (mkdir(fname, mode) == -1 || + chmod(fname, mode) == -1 || + chown(fname, uid, gid)) { + fprintf(stderr, "Error: failed to create %s directory\n", fname); + errExit("mkdir/chmod"); + } + ASSERT_PERMS(fname, uid, gid, mode); +} -- cgit v1.2.3-54-g00ecf