From: Lennart Poettering Date: Tue, 4 Jul 2023 20:26:52 +0000 (+0200) Subject: execute: fix credential dir handling for fs which support ACLs X-Git-Tag: v254-rc1~33^2~20 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f76ce81b91db1dac0d0a012e1cc903639002dd0a;p=thirdparty%2Fsystemd.git 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 d850a680221..8cc1a0f76b0 100644 --- 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 5c0c4e21aa0..7bfe02573a3 100644 --- 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 978389ed1d7..ef315c2f11d 100644 --- a/src/shared/acl-util.h +++ b/src/shared/acl-util.h @@ -4,6 +4,9 @@ #include #include +int fd_acl_make_read_only_fallback(int fd); +int fd_acl_make_writable_fallback(int fd); + #if HAVE_ACL #include #include @@ -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 1e015bd38e5..d643b2bd093 100644 --- 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 093eaaa01b6..eb9678a7d94 100644 --- 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);