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.

diff --git a/src/core/chown-recursive.c b/src/core/chown-recursive.c
index 7767301f7d91b8fa32f3fa71500b2d565c3fd30c..49d96b73760b39b84e8a96139b5993fe1c13e614 100644 (file)
--- a/src/core/chown-recursive.c
+++ b/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 */
 }

diff --git a/src/core/chown-recursive.h b/src/core/chown-recursive.h
index f3fa40a6563de1a4cc2548d073428ee59084ea39..bfee05f3be56159ec03fc11c8763c32718db9398 100644 (file)
--- a/src/core/chown-recursive.h
+++ b/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);

diff --git a/src/core/execute.c b/src/core/execute.c
index dabb6d824fbc4ce2e6bf8986510f51b8650bb57c..6698f59a4682297123156677397cea02e5475568 100644 (file)
--- a/src/core/execute.c
+++ b/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;
         }

diff --git a/src/test/test-chown-rec.c b/src/test/test-chown-rec.c
index 7e93e487d2ab76c4671bfccdd4bad42c63e64b90..aa11bd270ffda8f8a88e48512419037364061eb2 100644 (file)
--- a/src/test/test-chown-rec.c
+++ b/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);