set_perms cleanup

10 files changed, 73 insertions, 130 deletions

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  
 }