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.

diff --git a/src/core/execute.c b/src/core/execute.c
index d850a68022156935d2c602896fc8783bbfeaafb4..8cc1a0f76b08bb970729b30fffde1bbbaf3d5977 100644 (file)
--- a/src/core/execute.c
+++ b/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 */

diff --git a/src/shared/acl-util.c b/src/shared/acl-util.c
index 5c0c4e21aa06a5f2a202f2c348538564c89636a4..7bfe02573a3eb5761d5ba2cc5406e704fbf9f783 100644 (file)
--- a/src/shared/acl-util.c
+++ b/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;
+}

diff --git a/src/shared/acl-util.h b/src/shared/acl-util.h
index 978389ed1d7abf35797a54733cb9c90508fa3909..ef315c2f11d295e28dd691ceab1d1bb9e46cd27e 100644 (file)
--- a/src/shared/acl-util.h
+++ b/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

diff --git a/src/shared/meson.build b/src/shared/meson.build
index 1e015bd38e5aca23db581699c4b7a9e8476836ae..d643b2bd09358a651e468a88ab3b51d5ce1709aa 100644 (file)
--- a/src/shared/meson.build
+++ b/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

diff --git a/src/test/test-acl-util.c b/src/test/test-acl-util.c
index 093eaaa01b600810c1b7885aacaca0b5d214cb9c..eb9678a7d9478828a5d4235f44e84531c9044e59 100644 (file)
--- a/src/test/test-acl-util.c
+++ b/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);