core: drop suid/sgid bit of files/dirs when doing recursive chown

This adds some extra paranoia: when we recursively chown a directory for
use with DynamicUser=1 services we'll now drop suid/sgid from all files
we chown().

Of course, such files should not exist in the first place, and noone
should get access to those dirs who isn't root anyway, but let's better
be safe than sorry, and drop everything we come across.

src/core/chown-recursive.c

@@ -13,7 +13,13 @@
 #include "strv.h"
 #include "user-util.h"
 
-static int chown_one(int fd, const struct stat *st, uid_t uid, gid_t gid) {
+static int chown_one(
+                int fd,
+                const struct stat *st,
+                uid_t uid,
+                gid_t gid,
+                mode_t mask) {
+
         char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1];
         const char *n;
 
@@ -42,13 +48,19 @@ static int chown_one(int fd, const struct stat *st, uid_t uid, gid_t gid) {
          * because on some kernels/file systems trying to change the access mode will succeed but has no effect while
          * on others it actively fails. */
         if (!S_ISLNK(st->st_mode))
-                if (chmod(procfs_path, st->st_mode & 07777) < 0)
+                if (chmod(procfs_path, st->st_mode & 07777 & mask) < 0)
                         return -errno;
 
         return 1;
 }
 
-static int chown_recursive_internal(int fd, const struct stat *st, uid_t uid, gid_t gid) {
+static int chown_recursive_internal(
+                int fd,
+                const struct stat *st,
+                uid_t uid,
+                gid_t gid,
+                mode_t mask) {
+
         _cleanup_closedir_ DIR *d = NULL;
         bool changed = false;
         struct dirent *de;
@@ -87,13 +99,13 @@ static int chown_recursive_internal(int fd, const struct stat *st, uid_t uid, gi
                         if (subdir_fd < 0)
                                 return subdir_fd;
 
-                        r = chown_recursive_internal(subdir_fd, &fst, uid, gid); /* takes possession of subdir_fd even on failure */
+                        r = chown_recursive_internal(subdir_fd, &fst, uid, gid, mask); /* takes possession of subdir_fd even on failure */
                         if (r < 0)
                                 return r;
                         if (r > 0)
                                 changed = true;
                 } else {
-                        r = chown_one(path_fd, &fst, uid, gid);
+                        r = chown_one(path_fd, &fst, uid, gid, mask);
                         if (r < 0)
                                 return r;
                         if (r > 0)
@@ -101,14 +113,19 @@ static int chown_recursive_internal(int fd, const struct stat *st, uid_t uid, gi
                 }
         }
 
-        r = chown_one(dirfd(d), st, uid, gid);
+        r = chown_one(dirfd(d), st, uid, gid, mask);
         if (r < 0)
                 return r;
 
         return r > 0 || changed;
 }
 
-int path_chown_recursive(const char *path, uid_t uid, gid_t gid) {
+int path_chown_recursive(
+                const char *path,
+                uid_t uid,
+                gid_t gid,
+                mode_t mask) {
+
         _cleanup_close_ int fd = -1;
         struct stat st;
 
@@ -129,5 +146,5 @@ int path_chown_recursive(const char *path, uid_t uid, gid_t gid) {
             (!gid_is_valid(gid) || st.st_gid == gid))
                 return 0;
 
-        return chown_recursive_internal(TAKE_FD(fd), &st, uid, gid); /* we donate the fd to the call, regardless if it succeeded or failed */
+        return chown_recursive_internal(TAKE_FD(fd), &st, uid, gid, mask); /* we donate the fd to the call, regardless if it succeeded or failed */
 }

src/core/chown-recursive.h

@@ -3,4 +3,4 @@
 
 #include <sys/types.h>
 
-int path_chown_recursive(const char *path, uid_t uid, gid_t gid);
+int path_chown_recursive(const char *path, uid_t uid, gid_t gid, mode_t mask);

src/core/execute.c

@@ -2194,8 +2194,10 @@ static int setup_exec_directory(
                 if (r < 0)
                         goto fail;
 
-                /* Then, change the ownership of the whole tree, if necessary */
-                r = path_chown_recursive(pp ?: p, uid, gid);
+                /* Then, change the ownership of the whole tree, if necessary. When dynamic users are used we
+                 * drop the suid/sgid bits, since we really don't want SUID/SGID files for dynamic UID/GID
+                 * assignments to exist.*/
+                r = path_chown_recursive(pp ?: p, uid, gid, context->dynamic_user ? 01777 : 07777);
                 if (r < 0)
                         goto fail;
         }

src/test/test-chown-rec.c

@@ -106,7 +106,7 @@ static void test_chown_recursive(void) {
         assert_se(st.st_gid == gid);
         assert_se(has_xattr(p));
 
-        assert_se(path_chown_recursive(t, 1, 2) >= 0);
+        assert_se(path_chown_recursive(t, 1, 2, 07777) >= 0);
 
         p = strjoina(t, "/dir");
         assert_se(lstat(p, &st) >= 0);