From 3ed5918832344db694d094eefbe2189fd847345d Mon Sep 17 00:00:00 2001 From: netblue30 Date: Sat, 12 Nov 2016 09:52:53 -0500 Subject: set_perms cleanup --- src/firejail/appimage.c | 6 ++--- src/firejail/firejail.h | 1 + src/firejail/fs.c | 29 +++++++---------------- src/firejail/fs_home.c | 17 ++++--------- src/firejail/fs_whitelist.c | 55 ++++++++++++++----------------------------- src/firejail/preproc.c | 24 +++++++------------ src/firejail/pulseaudio.c | 18 +++++--------- src/firejail/restrict_users.c | 6 ++--- src/firejail/util.c | 23 +++++++++++------- src/firejail/x11.c | 24 +++++++------------ 10 files changed, 73 insertions(+), 130 deletions(-) (limited to 'src') diff --git a/src/firejail/appimage.c b/src/firejail/appimage.c index 176326a2b..96c054048 100644 --- a/src/firejail/appimage.c +++ b/src/firejail/appimage.c @@ -98,10 +98,8 @@ void appimage_set(const char *appimage_path) { fprintf(stderr, "Error: cannot create appimage mount point\n"); exit(1); } - if (chmod(mntdir, 0700) == -1) - errExit("chmod"); - if (chown(mntdir, getuid(), getgid()) == -1) - errExit("chown"); + if (set_perms(mntdir, getuid(), getgid(), 0700)) + errExit("set_perms"); EUID_USER(); ASSERT_PERMS(mntdir, getuid(), getgid(), 0700); diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index 435405fd9..282271a64 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -460,6 +460,7 @@ int remove_directory(const char *path); 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); // fs_var.c void fs_var_log(void); // mounting /var/log diff --git a/src/firejail/fs.c b/src/firejail/fs.c index 65b0773ca..3a2fd8c38 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -273,11 +273,8 @@ void fs_blacklist(void) { if (mount(dname1, dname2, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); /* coverity[toctou] */ - if (chown(dname2, s.st_uid, s.st_gid) == -1) - errExit("mount-bind chown"); - /* coverity[toctou] */ - if (chmod(dname2, s.st_mode) == -1) - errExit("mount-bind chmod"); + if (set_perms(dname2, s.st_uid, s.st_gid,s.st_mode)) + errExit("set_perms"); entry = entry->next; continue; @@ -773,10 +770,8 @@ void fs_overlayfs(void) { errExit("mkdir"); } - if (chown(odiff, 0, 0) < 0) - errExit("chown"); - if (chmod(odiff, 0755) < 0) - errExit("chmod"); + if (set_perms(odiff, 0, 0, 0755)) + errExit("set_perms"); char *owork; if(asprintf(&owork, "%s/owork", basedir) == -1) @@ -788,10 +783,8 @@ void fs_overlayfs(void) { errExit("mkdir"); } - if (chown(owork, 0, 0) < 0) + if (set_perms(owork, 0, 0, 0755)) errExit("chown"); - if (chmod(owork, 0755) < 0) - errExit("chmod"); // mount overlayfs if (arg_debug) @@ -850,10 +843,8 @@ void fs_overlayfs(void) { errExit("mkdir"); } - if (chown(hdiff, 0, 0) < 0) - errExit("chown"); - if (chmod(hdiff, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH) < 0) - errExit("chmod"); + 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) errExit("asprintf"); @@ -864,10 +855,8 @@ void fs_overlayfs(void) { errExit("mkdir"); } - if (chown(hwork, 0, 0) < 0) - errExit("chown"); - if (chmod(hwork, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH) < 0) - errExit("chmod"); + 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 if (asprintf(&option, "lowerdir=/home,upperdir=%s,workdir=%s", hdiff, hwork) == -1) diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c index a2532c367..91fbe592a 100644 --- a/src/firejail/fs_home.c +++ b/src/firejail/fs_home.c @@ -398,15 +398,8 @@ int fs_copydir(const char *path, const struct stat *st, int ftype, struct FTW *s else if (ftype == FTW_D) { if (mkdir(dest, s.st_mode) == -1) errExit("mkdir"); - if (chmod(dest, s.st_mode) < 0) { - fprintf(stderr, "Error: cannot change mode for %s\n", path); - exit(1); - } - if (chown(dest, firejail_uid, firejail_gid) < 0) { - fprintf(stderr, "Error: cannot change ownership for %s\n", path); - exit(1); - } - + if (set_perms(dest, firejail_uid, firejail_gid, s.st_mode)) + errExit("set_perms"); #if 0 struct stat s2; if (stat(dest, &s2) == 0) { @@ -590,10 +583,8 @@ void fs_private_home_list(void) { int rv = mkdir(RUN_HOME_DIR, 0755); if (rv == -1) errExit("mkdir"); - if (chown(RUN_HOME_DIR, u, g) < 0) - errExit("chown"); - if (chmod(RUN_HOME_DIR, 0755) < 0) - errExit("chmod"); + if (set_perms(RUN_HOME_DIR, u, g, 0755)) + errExit("set_perms"); ASSERT_PERMS(RUN_HOME_DIR, u, g, 0755); fs_logger_print(); // save the current log diff --git a/src/firejail/fs_whitelist.c b/src/firejail/fs_whitelist.c index 9cd8f7681..9d8021219 100644 --- a/src/firejail/fs_whitelist.c +++ b/src/firejail/fs_whitelist.c @@ -157,10 +157,8 @@ static int mkpath(const char* path, mode_t mode) { } } else { - if (chmod(file_path, mode) == -1) - errExit("chmod"); - if (chown(file_path, uid, gid) == -1) - errExit("chown"); + if (set_perms(file_path, uid, gid, mode)) + errExit("set_perms"); done = 1; } @@ -535,11 +533,8 @@ void fs_whitelist(void) { int rv = mkdir(RUN_WHITELIST_HOME_USER_DIR, 0755); if (rv == -1) errExit("mkdir"); - if (chown(RUN_WHITELIST_HOME_USER_DIR, getuid(), getgid()) < 0) - errExit("chown"); - if (chmod(RUN_WHITELIST_HOME_USER_DIR, 0755) < 0) - errExit("chmod"); - + if (set_perms(RUN_WHITELIST_HOME_USER_DIR, getuid(), getgid(), 0755)) + errExit("set_perms"); if (mount(cfg.homedir, RUN_WHITELIST_HOME_USER_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); @@ -553,10 +548,8 @@ void fs_whitelist(void) { int rv = mkdir(RUN_WHITELIST_TMP_DIR, 1777); if (rv == -1) errExit("mkdir"); - if (chown(RUN_WHITELIST_TMP_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_WHITELIST_TMP_DIR, 1777) < 0) - errExit("chmod"); + if (set_perms(RUN_WHITELIST_TMP_DIR, 0, 0, 1777)) + errExit("set_perms"); if (mount("/tmp", RUN_WHITELIST_TMP_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); @@ -578,10 +571,8 @@ void fs_whitelist(void) { int rv = mkdir(RUN_WHITELIST_MEDIA_DIR, 0755); if (rv == -1) errExit("mkdir"); - if (chown(RUN_WHITELIST_MEDIA_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_WHITELIST_MEDIA_DIR, 0755) < 0) - errExit("chmod"); + if (set_perms(RUN_WHITELIST_MEDIA_DIR, 0, 0, 0755)) + errExit("set_perms"); if (mount("/media", RUN_WHITELIST_MEDIA_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); @@ -606,10 +597,8 @@ void fs_whitelist(void) { int rv = mkdir(RUN_WHITELIST_MNT_DIR, 0755); if (rv == -1) errExit("mkdir"); - if (chown(RUN_WHITELIST_MNT_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_WHITELIST_MNT_DIR, 0755) < 0) - errExit("chmod"); + if (set_perms(RUN_WHITELIST_MNT_DIR, 0, 0, 0755)) + errExit("set_perms"); if (mount("/mnt", RUN_WHITELIST_MNT_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); @@ -632,10 +621,8 @@ void fs_whitelist(void) { int rv = mkdir(RUN_WHITELIST_VAR_DIR, 0755); if (rv == -1) errExit("mkdir"); - if (chown(RUN_WHITELIST_VAR_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_WHITELIST_VAR_DIR, 0755) < 0) - errExit("chmod"); + if (set_perms(RUN_WHITELIST_VAR_DIR, 0, 0, 0755)) + errExit("set_perms"); if (mount("/var", RUN_WHITELIST_VAR_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); @@ -654,10 +641,8 @@ void fs_whitelist(void) { int rv = mkdir(RUN_WHITELIST_DEV_DIR, 0755); if (rv == -1) errExit("mkdir"); - if (chown(RUN_WHITELIST_DEV_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_WHITELIST_DEV_DIR, 0755) < 0) - errExit("chmod"); + if (set_perms(RUN_WHITELIST_DEV_DIR, 0, 0, 0755)) + errExit("set_perms"); if (mount("/dev", RUN_WHITELIST_DEV_DIR, NULL, MS_BIND|MS_REC, "mode=755,gid=0") < 0) errExit("mount bind"); @@ -676,10 +661,8 @@ void fs_whitelist(void) { int rv = mkdir(RUN_WHITELIST_OPT_DIR, 0755); if (rv == -1) errExit("mkdir"); - if (chown(RUN_WHITELIST_OPT_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_WHITELIST_OPT_DIR, 0755) < 0) - errExit("chmod"); + if (set_perms(RUN_WHITELIST_OPT_DIR, 0, 0, 0755)) + errExit("set_perms"); if (mount("/opt", RUN_WHITELIST_OPT_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); @@ -701,10 +684,8 @@ void fs_whitelist(void) { int rv = mkdir(RUN_WHITELIST_SRV_DIR, 0755); if (rv == -1) errExit("mkdir"); - if (chown(RUN_WHITELIST_SRV_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_WHITELIST_SRV_DIR, 0755) < 0) - errExit("chmod"); + if (set_perms(RUN_WHITELIST_SRV_DIR, 0, 0, 0755)) + errExit("set_perms"); if (mount("/srv", RUN_WHITELIST_SRV_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); diff --git a/src/firejail/preproc.c b/src/firejail/preproc.c index 2873571a9..fe5f2eb44 100644 --- a/src/firejail/preproc.c +++ b/src/firejail/preproc.c @@ -78,31 +78,23 @@ void preproc_mount_mnt_dir(void) { // create all seccomp files // as root, create RUN_SECCOMP_I386 file create_empty_file_as_root(RUN_SECCOMP_I386, 0644); - if (chown(RUN_SECCOMP_I386, getuid(), getgid()) == -1) - errExit("chown"); - if (chmod(RUN_SECCOMP_I386, 0644) == -1) - errExit("chmod"); + if (set_perms(RUN_SECCOMP_I386, getuid(), getgid(), 0644)) + errExit("set_perms"); // as root, create RUN_SECCOMP_AMD64 file create_empty_file_as_root(RUN_SECCOMP_AMD64, 0644); - if (chown(RUN_SECCOMP_AMD64, getuid(), getgid()) == -1) - errExit("chown"); - if (chmod(RUN_SECCOMP_AMD64, 0644) == -1) - errExit("chmod"); + if (set_perms(RUN_SECCOMP_AMD64, getuid(), getgid(), 0644)) + errExit("set_perms"); // as root, create RUN_SECCOMP file create_empty_file_as_root(RUN_SECCOMP_CFG, 0644); - if (chown(RUN_SECCOMP_CFG, getuid(), getgid()) == -1) - errExit("chown"); - if (chmod(RUN_SECCOMP_CFG, 0644) == -1) - errExit("chmod"); + if (set_perms(RUN_SECCOMP_CFG, getuid(), getgid(), 0644)) + errExit("set_perms"); // as root, create RUN_SECCOMP_PROTOCOL file create_empty_file_as_root(RUN_SECCOMP_PROTOCOL, 0644); - if (chown(RUN_SECCOMP_PROTOCOL, getuid(), getgid()) == -1) - errExit("chown"); - if (chmod(RUN_SECCOMP_PROTOCOL, 0644) == -1) - errExit("chmod"); + if (set_perms(RUN_SECCOMP_PROTOCOL, getuid(), getgid(), 0644)) + errExit("set_perms"); } } diff --git a/src/firejail/pulseaudio.c b/src/firejail/pulseaudio.c index e1a58c1c8..c76505591 100644 --- a/src/firejail/pulseaudio.c +++ b/src/firejail/pulseaudio.c @@ -106,10 +106,8 @@ void pulseaudio_init(void) { // create the new user pulseaudio directory int rv = mkdir(RUN_PULSE_DIR, 0700); (void) rv; // in --chroot mode the directory can already be there - if (chown(RUN_PULSE_DIR, getuid(), getgid()) < 0) - errExit("chown"); - if (chmod(RUN_PULSE_DIR, 0700) < 0) - errExit("chmod"); + if (set_perms(RUN_PULSE_DIR, getuid(), getgid(), 0700)) + errExit("set_perms"); // create the new client.conf file char *pulsecfg = NULL; @@ -131,10 +129,8 @@ void pulseaudio_init(void) { if (stat(dir1, &s) == -1) { int rv = mkdir(dir1, 0755); if (rv == 0) { - rv = chown(dir1, getuid(), getgid()); - (void) rv; - rv = chmod(dir1, 0755); - (void) rv; + if (set_perms(dir1, getuid(), getgid(), 0755)) + ; // do nothing } } free(dir1); @@ -143,10 +139,8 @@ void pulseaudio_init(void) { if (stat(dir1, &s) == -1) { int rv = mkdir(dir1, 0700); if (rv == 0) { - rv = chown(dir1, getuid(), getgid()); - (void) rv; - rv = chmod(dir1, 0700); - (void) rv; + if (set_perms(dir1, getuid(), getgid(), 0700)) + ; // do nothing } } free(dir1); diff --git a/src/firejail/restrict_users.c b/src/firejail/restrict_users.c index 57e84e5cc..393851148 100644 --- a/src/firejail/restrict_users.c +++ b/src/firejail/restrict_users.c @@ -95,10 +95,8 @@ static void sanitize_home(void) { fs_logger2("mkdir", cfg.homedir); // set mode and ownership - if (chown(cfg.homedir, s.st_uid, s.st_gid) == -1) - errExit("chown"); - if (chmod(cfg.homedir, s.st_mode) == -1) - errExit("chmod"); + if (set_perms(cfg.homedir, s.st_uid, s.st_gid, s.st_mode)) + errExit("set_perms"); // mount user home directory if (mount(RUN_WHITELIST_HOME_DIR, cfg.homedir, NULL, MS_BIND|MS_REC, NULL) < 0) diff --git a/src/firejail/util.c b/src/firejail/util.c index a7712441e..3424d8ab6 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -100,10 +100,8 @@ int mkpath_as_root(const char* path) { } } else { - if (chmod(file_path, 0755) == -1) - errExit("chmod"); - if (chown(file_path, 0, 0) == -1) - errExit("chown"); + if (set_perms(file_path, 0, 0, 0755)) + errExit("set_perms"); done = 1; } @@ -699,10 +697,8 @@ void create_empty_dir_as_root(const char *dir, mode_t mode) { printf("Creating empty %s directory\n", dir); if (mkdir(dir, mode) == -1) errExit("mkdir"); - if (chmod(dir, mode) == -1) - errExit("chmod"); - if (chown(dir, 0, 0) == -1) - errExit("chown"); + if (set_perms(dir, 0, 0, mode)) + errExit("set_perms"); ASSERT_PERMS(dir, 0, 0, mode); } } @@ -725,3 +721,14 @@ void create_empty_file_as_root(const char *fname, mode_t mode) { } } +// return 1 if error +int set_perms(const char *fname, uid_t uid, gid_t gid, mode_t mode) { + assert(fname); + if (chmod(fname, mode) == -1) + return 1; + if (chown(fname, uid, gid) == -1) + return 1; + return 0; +} + + diff --git a/src/firejail/x11.c b/src/firejail/x11.c index 2b1121958..9da6d3e30 100644 --- a/src/firejail/x11.c +++ b/src/firejail/x11.c @@ -137,10 +137,8 @@ void fs_x11(void) { int rv = mkdir(RUN_WHITELIST_X11_DIR, 1777); if (rv == -1) errExit("mkdir"); - if (chown(RUN_WHITELIST_X11_DIR, 0, 0) < 0) - errExit("chown"); - if (chmod(RUN_WHITELIST_X11_DIR, 1777) < 0) - errExit("chmod"); + if (set_perms(RUN_WHITELIST_X11_DIR, 0, 0, 1777)) + errExit("set_perms"); if (mount("/tmp/.X11-unix", RUN_WHITELIST_X11_DIR, NULL, MS_BIND|MS_REC, NULL) < 0) errExit("mount bind"); @@ -706,10 +704,8 @@ void x11_xorg(void) { fprintf(stderr, "Error: cannot create the new .Xauthority file\n"); exit(1); } - if (chown(tmpfname, getuid(), getgid()) == -1) - errExit("chown"); - if (chmod(tmpfname, 0600) == -1) - errExit("chmod"); + if (set_perms(tmpfname, getuid(), getgid(), 0600)) + errExit("set_perms"); // move the temporary file in RUN_XAUTHORITY_SEC_FILE in order to have it deleted // automatically when the sandbox is closed @@ -717,10 +713,8 @@ void x11_xorg(void) { fprintf(stderr, "Error: cannot create the new .Xauthority file\n"); exit(1); } - if (chown(RUN_XAUTHORITY_SEC_FILE, getuid(), getgid()) == -1) - errExit("chown"); - if (chmod(RUN_XAUTHORITY_SEC_FILE, 0600) == -1) - errExit("chmod"); + if (set_perms(RUN_XAUTHORITY_SEC_FILE, getuid(), getgid(), 0600)) + errExit("set_perms"); unlink(tmpfname); // mount @@ -728,10 +722,8 @@ void x11_xorg(void) { fprintf(stderr, "Error: cannot mount the new .Xauthority file\n"); exit(1); } - if (chown(dest, getuid(), getgid()) == -1) - errExit("chown"); - if (chmod(dest, 0600) == -1) - errExit("chmod"); + if (set_perms(dest, getuid(), getgid(), 0600)) + errExit("set_perms"); free(dest); #endif } -- cgit v1.2.3-54-g00ecf