execute: fix credential dir handling for fs which support ACLs

When the credential dir is backed by an fs that supports ACLs we must be
more careful with adjusting the 'x' bit of the directory, as any chmod()
call on the dir will reset the mask entry of the ACL entirely which we
don't want. Hence, do a manual set of ACL changes, that only add/drop
the 'x' bit but otherwise leave the ACL as it is.

This matters if we use tmpfs rather than ramfs to store credentials.

src/core/execute.c

@@ -3182,6 +3182,10 @@ static int acquire_credentials(
         if (dfd < 0)
                 return -errno;
 
+        r = fd_acl_make_writable(dfd); /* Add the "w" bit, if we are reusing an already set up credentials dir where it was unset */
+        if (r < 0)
+                return r;
+
         /* First, load credentials off disk (or acquire via AF_UNIX socket) */
         HASHMAP_FOREACH(lc, context->load_credentials) {
                 _cleanup_close_ int sub_fd = -EBADF;
@@ -3313,8 +3317,9 @@ static int acquire_credentials(
                 left -= add;
         }
 
-        if (fchmod(dfd, 0500) < 0) /* Now take away the "w" bit */
-                return -errno;
+        r = fd_acl_make_read_only(dfd); /* Now take away the "w" bit */
+        if (r < 0)
+                return r;
 
         /* After we created all keys with the right perms, also make sure the credential store as a whole is
          * accessible */

src/shared/acl-util.c

@@ -7,10 +7,13 @@
 
 #include "acl-util.h"
 #include "alloc-util.h"
+#include "errno-util.h"
 #include "string-util.h"
 #include "strv.h"
 #include "user-util.h"
 
+#if HAVE_ACL
+
 int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *ret_entry) {
         acl_entry_t i;
         int r;
@@ -489,3 +492,161 @@ int fd_add_uid_acl_permission(
 
         return 0;
 }
+
+int fd_acl_make_read_only(int fd) {
+        _cleanup_(acl_freep) acl_t acl = NULL;
+        bool changed = false;
+        acl_entry_t i;
+        int r;
+
+        assert(fd >= 0);
+
+        /* Safely drops all W bits from all relevant ACL entries of the file, without changing entries which
+         * are masked by the ACL mask */
+
+        acl = acl_get_fd(fd);
+        if (!acl) {
+
+                if (!ERRNO_IS_NOT_SUPPORTED(errno))
+                        return -errno;
+
+                /* No ACLs? Then just update the regular mode_t */
+                return fd_acl_make_read_only_fallback(fd);
+        }
+
+        for (r = acl_get_entry(acl, ACL_FIRST_ENTRY, &i);
+             r > 0;
+             r = acl_get_entry(acl, ACL_NEXT_ENTRY, &i)) {
+                acl_permset_t permset;
+                acl_tag_t tag;
+                int b;
+
+                if (acl_get_tag_type(i, &tag) < 0)
+                        return -errno;
+
+                /* These three control the x bits overall (as ACL_MASK affects all remaining tags) */
+                if (!IN_SET(tag, ACL_USER_OBJ, ACL_MASK, ACL_OTHER))
+                        continue;
+
+                if (acl_get_permset(i, &permset) < 0)
+                        return -errno;
+
+                b = acl_get_perm(permset, ACL_WRITE);
+                if (b < 0)
+                        return -errno;
+
+                if (b) {
+                        if (acl_delete_perm(permset, ACL_WRITE) < 0)
+                                return -errno;
+
+                        changed = true;
+                }
+        }
+        if (r < 0)
+                return -errno;
+
+        if (!changed)
+                return 0;
+
+        if (acl_set_fd(fd, acl) < 0) {
+                if (!ERRNO_IS_NOT_SUPPORTED(errno))
+                        return -errno;
+
+                return fd_acl_make_read_only_fallback(fd);
+        }
+
+        return 1;
+}
+
+int fd_acl_make_writable(int fd) {
+        _cleanup_(acl_freep) acl_t acl = NULL;
+        acl_entry_t i;
+        int r;
+
+        /* Safely adds the writable bit to the owner's ACL entry of this inode. (And only the owner's! – This
+         * not the obvious inverse of fd_acl_make_read_only() hence!) */
+
+        acl = acl_get_fd(fd);
+        if (!acl) {
+                if (!ERRNO_IS_NOT_SUPPORTED(errno))
+                        return -errno;
+
+                /* No ACLs? Then just update the regular mode_t */
+                return fd_acl_make_writable_fallback(fd);
+        }
+
+        for (r = acl_get_entry(acl, ACL_FIRST_ENTRY, &i);
+             r > 0;
+             r = acl_get_entry(acl, ACL_NEXT_ENTRY, &i)) {
+                acl_permset_t permset;
+                acl_tag_t tag;
+                int b;
+
+                if (acl_get_tag_type(i, &tag) < 0)
+                        return -errno;
+
+                if (tag != ACL_USER_OBJ)
+                        continue;
+
+                if (acl_get_permset(i, &permset) < 0)
+                        return -errno;
+
+                b = acl_get_perm(permset, ACL_WRITE);
+                if (b < 0)
+                        return -errno;
+
+                if (b)
+                        return 0; /* Already set? Then there's nothing to do. */
+
+                if (acl_add_perm(permset, ACL_WRITE) < 0)
+                        return -errno;
+
+                break;
+        }
+        if (r < 0)
+                return -errno;
+
+        if (acl_set_fd(fd, acl) < 0) {
+                if (!ERRNO_IS_NOT_SUPPORTED(errno))
+                        return -errno;
+
+                return fd_acl_make_writable_fallback(fd);
+        }
+
+        return 1;
+}
+#endif
+
+int fd_acl_make_read_only_fallback(int fd) {
+        struct stat st;
+
+        assert(fd >= 0);
+
+        if (fstat(fd, &st) < 0)
+                return -errno;
+
+        if ((st.st_mode & 0222) == 0)
+                return 0;
+
+        if (fchmod(fd, st.st_mode & 0555) < 0)
+                return -errno;
+
+        return 1;
+}
+
+int fd_acl_make_writable_fallback(int fd) {
+        struct stat st;
+
+        assert(fd >= 0);
+
+        if (fstat(fd, &st) < 0)
+                return -errno;
+
+        if ((st.st_mode & 0200) != 0) /* already set */
+                return 0;
+
+        if (fchmod(fd, (st.st_mode & 07777) | 0200) < 0)
+                return -errno;
+
+        return 1;
+}

src/shared/acl-util.h

@@ -4,6 +4,9 @@
 #include <errno.h>
 #include <unistd.h>
 
+int fd_acl_make_read_only_fallback(int fd);
+int fd_acl_make_writable_fallback(int fd);
+
 #if HAVE_ACL
 #include <acl/libacl.h>
 #include <stdbool.h>
@@ -24,6 +27,9 @@ int parse_acl(
 int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *ret);
 int fd_add_uid_acl_permission(int fd, uid_t uid, unsigned mask);
 
+int fd_acl_make_read_only(int fd);
+int fd_acl_make_writable(int fd);
+
 /* acl_free takes multiple argument types.
  * Multiple cleanup functions are necessary. */
 DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(acl_t, acl_free, NULL);
@@ -42,4 +48,13 @@ DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(gid_t*, acl_free_gid_tp, NULL);
 static inline int fd_add_uid_acl_permission(int fd, uid_t uid, unsigned mask) {
         return -EOPNOTSUPP;
 }
+
+static inline int fd_acl_make_read_only(int fd) {
+        return fd_acl_make_read_only_fallback(fd);
+}
+
+static inline int fd_acl_make_writable(int fd) {
+        return fd_acl_make_writable_fallback(fd);
+}
+
 #endif

src/shared/meson.build

@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: LGPL-2.1-or-later
 
 shared_sources = files(
+        'acl-util.c',
         'acpi-fpdt.c',
         'apparmor-util.c',
         'ask-password-api.c',
@@ -189,7 +190,6 @@ syscall_list_h = custom_target(
 
 if conf.get('HAVE_ACL') == 1
         shared_sources += files(
-                'acl-util.c',
                 'devnode-acl.c',
         )
 endif

src/test/test-acl-util.c

@@ -69,4 +69,62 @@ TEST_RET(add_acls_for_user) {
         return 0;
 }
 
+TEST(fd_acl_make_read_only) {
+        _cleanup_(unlink_tempfilep) char fn[] = "/tmp/test-empty.XXXXXX";
+        _cleanup_close_ int fd = -EBADF;
+        const char *cmd;
+        struct stat st;
+
+        fd = mkostemp_safe(fn);
+        assert_se(fd >= 0);
+
+        /* make it more exciting */
+        (void) fd_add_uid_acl_permission(fd, 1, ACL_READ|ACL_WRITE|ACL_EXECUTE);
+
+        assert_se(fstat(fd, &st) >= 0);
+        assert_se((st.st_mode & 0200) == 0200);
+
+        cmd = strjoina("getfacl -p ", fn);
+        assert_se(system(cmd) == 0);
+
+        cmd = strjoina("stat ", fn);
+        assert_se(system(cmd) == 0);
+
+        log_info("read-only");
+        assert_se(fd_acl_make_read_only(fd));
+
+        assert_se(fstat(fd, &st) >= 0);
+        assert_se((st.st_mode & 0222) == 0000);
+
+        cmd = strjoina("getfacl -p ", fn);
+        assert_se(system(cmd) == 0);
+
+        cmd = strjoina("stat ", fn);
+        assert_se(system(cmd) == 0);
+
+        log_info("writable");
+        assert_se(fd_acl_make_writable(fd));
+
+        assert_se(fstat(fd, &st) >= 0);
+        assert_se((st.st_mode & 0222) == 0200);
+
+        cmd = strjoina("getfacl -p ", fn);
+        assert_se(system(cmd) == 0);
+
+        cmd = strjoina("stat ", fn);
+        assert_se(system(cmd) == 0);
+
+        log_info("read-only");
+        assert_se(fd_acl_make_read_only(fd));
+
+        assert_se(fstat(fd, &st) >= 0);
+        assert_se((st.st_mode & 0222) == 0000);
+
+        cmd = strjoina("getfacl -p ", fn);
+        assert_se(system(cmd) == 0);
+
+        cmd = strjoina("stat ", fn);
+        assert_se(system(cmd) == 0);
+}
+
 DEFINE_TEST_MAIN(LOG_INFO);