Merge pull request #25622 from YHNdnzj/tmpfiles-X-bit-support

tmpfiles: add conditionalized execute permission (X) support

7 files changed, 283 insertions, 24 deletions

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index a23b9c8946..54f3c501cb 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -446,13 +446,15 @@ L     /tmp/foobar -    -    -     -   /dev/null</programlisting>
           <term><varname>a+</varname></term>
           <listitem><para>Set POSIX ACLs (access control lists), see <citerefentry
           project='man-pages'><refentrytitle>acl</refentrytitle>
-          <manvolnum>5</manvolnum></citerefentry>. If suffixed with <varname>+</varname>, the specified
-          entries will be added to the existing set. <command>systemd-tmpfiles</command> will automatically
-          add the required base entries for user and group based on the access mode of the file, unless base
-          entries already exist or are explicitly specified. The mask will be added if not specified
-          explicitly or already present. Lines of this type accept shell-style globs in place of normal path
-          names. This can be useful for allowing additional access to certain files. Does not follow
-          symlinks.</para></listitem>
+          <manvolnum>5</manvolnum></citerefentry>. Additionally, if 'X' is used, the execute bit is set only
+          if the file is a directory or already has execute permission for some user, as mentioned in
+          <citerefentry project='man-pages'><refentrytitle>setfacl</refentrytitle><manvolnum>1</manvolnum></citerefentry>.
+          If suffixed with <varname>+</varname>, the specified entries will be added to the existing set.
+          <command>systemd-tmpfiles</command> will automatically add the required base entries for user
+          and group based on the access mode of the file, unless base entries already exist or are explicitly
+          specified. The mask will be added if not specified explicitly or already present. Lines of this type
+          accept shell-style globs in place of normal path names. This can be useful for allowing additional
+          access to certain files. Does not follow symlinks.</para></listitem>
         </varlistentry>
 
         <varlistentry>
diff --git a/src/shared/acl-util.c b/src/shared/acl-util.c
index b734ee1e0c..5c0c4e21aa 100644
--- a/src/shared/acl-util.c
+++ b/src/shared/acl-util.c
@@ -209,14 +209,20 @@ int acl_search_groups(const char *path, char ***ret_groups) {
         return ret;
 }
 
-int parse_acl(const char *text, acl_t *ret_acl_access, acl_t *ret_acl_default, bool want_mask) {
-        _cleanup_free_ char **a = NULL, **d = NULL; /* strings are not freed */
-        _cleanup_strv_free_ char **split = NULL;
-        int r = -EINVAL;
-        _cleanup_(acl_freep) acl_t a_acl = NULL, d_acl = NULL;
+int parse_acl(
+                const char *text,
+                acl_t *ret_acl_access,
+                acl_t *ret_acl_access_exec, /* extra rules to apply to inodes subject to uppercase X handling */
+                acl_t *ret_acl_default,
+                bool want_mask) {
+
+        _cleanup_strv_free_ char **a = NULL, **e = NULL, **d = NULL, **split = NULL;
+        _cleanup_(acl_freep) acl_t a_acl = NULL, e_acl = NULL, d_acl = NULL;
+        int r;
 
         assert(text);
         assert(ret_acl_access);
+        assert(ret_acl_access_exec);
         assert(ret_acl_default);
 
         split = strv_split(text, ",");
@@ -224,13 +230,38 @@ int parse_acl(const char *text, acl_t *ret_acl_access, acl_t *ret_acl_default, b
                 return -ENOMEM;
 
         STRV_FOREACH(entry, split) {
-                char *p;
+                _cleanup_strv_free_ char **entry_split = NULL;
+                _cleanup_free_ char *entry_join = NULL;
+                int n;
+
+                n = strv_split_full(&entry_split, *entry, ":", EXTRACT_DONT_COALESCE_SEPARATORS|EXTRACT_RETAIN_ESCAPE);
+                if (n < 0)
+                        return n;
+
+                if (n < 3 || n > 4)
+                        return -EINVAL;
+
+                string_replace_char(entry_split[n-1], 'X', 'x');
+
+                if (n == 4) {
+                        if (!STR_IN_SET(entry_split[0], "default", "d"))
+                                return -EINVAL;
 
-                p = STARTSWITH_SET(*entry, "default:", "d:");
-                if (p)
-                        r = strv_push(&d, p);
-                else
-                        r = strv_push(&a, *entry);
+                        entry_join = strv_join(entry_split + 1, ":");
+                        if (!entry_join)
+                                return -ENOMEM;
+
+                        r = strv_consume(&d, TAKE_PTR(entry_join));
+                } else { /* n == 3 */
+                        entry_join = strv_join(entry_split, ":");
+                        if (!entry_join)
+                                return -ENOMEM;
+
+                        if (!streq(*entry, entry_join))
+                                r = strv_consume(&e, TAKE_PTR(entry_join));
+                        else
+                                r = strv_consume(&a, TAKE_PTR(entry_join));
+                }
                 if (r < 0)
                         return r;
         }
@@ -253,6 +284,20 @@ int parse_acl(const char *text, acl_t *ret_acl_access, acl_t *ret_acl_default, b
                 }
         }
 
+        if (!strv_isempty(e)) {
+                _cleanup_free_ char *join = NULL;
+
+                join = strv_join(e, ",");
+                if (!join)
+                        return -ENOMEM;
+
+                e_acl = acl_from_text(join);
+                if (!e_acl)
+                        return -errno;
+
+                /* The mask must be calculated after deciding whether the execute bit should be set. */
+        }
+
         if (!strv_isempty(d)) {
                 _cleanup_free_ char *join = NULL;
 
@@ -272,6 +317,7 @@ int parse_acl(const char *text, acl_t *ret_acl_access, acl_t *ret_acl_default, b
         }
 
         *ret_acl_access = TAKE_PTR(a_acl);
+        *ret_acl_access_exec = TAKE_PTR(e_acl);
         *ret_acl_default = TAKE_PTR(d_acl);
 
         return 0;
diff --git a/src/shared/acl-util.h b/src/shared/acl-util.h
index d3a341fbe6..978389ed1d 100644
--- a/src/shared/acl-util.h
+++ b/src/shared/acl-util.h
@@ -15,7 +15,12 @@ int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry);
 int calc_acl_mask_if_needed(acl_t *acl_p);
 int add_base_acls_if_needed(acl_t *acl_p, const char *path);
 int acl_search_groups(const char* path, char ***ret_groups);
-int parse_acl(const char *text, acl_t *ret_acl_access, acl_t *ret_acl_default, bool want_mask);
+int parse_acl(
+                const char *text,
+                acl_t *ret_acl_access,
+                acl_t *ret_acl_access_exec,
+                acl_t *ret_acl_default,
+                bool want_mask);
 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);
 
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 2eb6e5ea33..de72df2908 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -138,6 +138,7 @@ typedef struct Item {
         char **xattrs;
 #if HAVE_ACL
         acl_t acl_access;
+        acl_t acl_access_exec;
         acl_t acl_default;
 #endif
         uid_t uid;
@@ -1127,17 +1128,145 @@ static int parse_acls_from_arg(Item *item) {
         /* If append_or_force (= modify) is set, we will not modify the acl
          * afterwards, so the mask can be added now if necessary. */
 
-        r = parse_acl(item->argument, &item->acl_access, &item->acl_default, !item->append_or_force);
+        r = parse_acl(item->argument, &item->acl_access, &item->acl_access_exec,
+                      &item->acl_default, !item->append_or_force);
         if (r < 0)
-                log_warning_errno(r, "Failed to parse ACL \"%s\": %m. Ignoring", item->argument);
+                log_warning_errno(r, "Failed to parse ACL \"%s\", ignoring: %m", item->argument);
 #else
-        log_warning("ACLs are not supported. Ignoring.");
+        log_warning("ACLs are not supported, ignoring.");
 #endif
 
         return 0;
 }
 
 #if HAVE_ACL
+static int parse_acl_cond_exec(
+                const char *path,
+                acl_t access, /* could be empty (NULL) */
+                acl_t cond_exec,
+                const struct stat *st,
+                bool append,
+                acl_t *ret) {
+
+        _cleanup_(acl_freep) acl_t parsed = NULL;
+        acl_entry_t entry;
+        acl_permset_t permset;
+        bool has_exec;
+        int r;
+
+        assert(path);
+        assert(ret);
+        assert(st);
+
+        parsed = access ? acl_dup(access) : acl_init(0);
+        if (!parsed)
+                return -errno;
+
+        /* Since we substitute 'X' with 'x' in parse_acl(), we just need to copy the entries over
+         * for directories */
+        if (S_ISDIR(st->st_mode)) {
+                for (r = acl_get_entry(cond_exec, ACL_FIRST_ENTRY, &entry);
+                     r > 0;
+                     r = acl_get_entry(cond_exec, ACL_NEXT_ENTRY, &entry)) {
+
+                        acl_entry_t parsed_entry;
+
+                        if (acl_create_entry(&parsed, &parsed_entry) < 0)
+                                return -errno;
+
+                        if (acl_copy_entry(parsed_entry, entry) < 0)
+                                return -errno;
+                }
+                if (r < 0)
+                        return -errno;
+
+                goto finish;
+        }
+
+        has_exec = st->st_mode & S_IXUSR;
+
+        if (!has_exec && append) {
+                _cleanup_(acl_freep) acl_t old = NULL;
+
+                old = acl_get_file(path, ACL_TYPE_ACCESS);
+                if (!old)
+                        return -errno;
+
+                for (r = acl_get_entry(old, ACL_FIRST_ENTRY, &entry);
+                     r > 0;
+                     r = acl_get_entry(old, ACL_NEXT_ENTRY, &entry)) {
+
+                        if (acl_get_permset(entry, &permset) < 0)
+                                return -errno;
+
+                        r = acl_get_perm(permset, ACL_EXECUTE);
+                        if (r < 0)
+                                return -errno;
+                        if (r > 0) {
+                                has_exec = true;
+                                break;
+                        }
+                }
+                if (r < 0)
+                        return -errno;
+        }
+
+        /* Check if we're about to set the execute bit in acl_access */
+        if (!has_exec && access) {
+                for (r = acl_get_entry(access, ACL_FIRST_ENTRY, &entry);
+                     r > 0;
+                     r = acl_get_entry(access, ACL_NEXT_ENTRY, &entry)) {
+
+                        if (acl_get_permset(entry, &permset) < 0)
+                                return -errno;
+
+                        r = acl_get_perm(permset, ACL_EXECUTE);
+                        if (r < 0)
+                                return -errno;
+                        if (r > 0) {
+                                has_exec = true;
+                                break;
+                        }
+                }
+                if (r < 0)
+                        return -errno;
+        }
+
+        for (r = acl_get_entry(cond_exec, ACL_FIRST_ENTRY, &entry);
+             r > 0;
+             r = acl_get_entry(cond_exec, ACL_NEXT_ENTRY, &entry)) {
+
+                acl_entry_t parsed_entry;
+
+                if (acl_create_entry(&parsed, &parsed_entry) < 0)
+                        return -errno;
+
+                if (acl_copy_entry(parsed_entry, entry) < 0)
+                        return -errno;
+
+                if (!has_exec) {
+                        if (acl_get_permset(parsed_entry, &permset) < 0)
+                                return -errno;
+
+                        if (acl_delete_perm(permset, ACL_EXECUTE) < 0)
+                                return -errno;
+                }
+        }
+        if (r < 0)
+                return -errno;
+
+finish:
+        if (!append) { /* want_mask = true */
+                r = calc_acl_mask_if_needed(&parsed);
+                if (r < 0)
+                        return r;
+        }
+
+        *ret = TAKE_PTR(parsed);
+
+        return 0;
+}
+
 static int path_set_acl(
                 const char *path,
                 const char *pretty,
@@ -1202,6 +1331,7 @@ static int fd_set_acls(
 
         int r = 0;
 #if HAVE_ACL
+        _cleanup_(acl_freep) acl_t access_with_exec_parsed = NULL;
         struct stat stbuf;
 
         assert(item);
@@ -1224,7 +1354,18 @@ static int fd_set_acls(
                 return 0;
         }
 
-        if (item->acl_access)
+        if (item->acl_access_exec) {
+                r = parse_acl_cond_exec(FORMAT_PROC_FD_PATH(fd),
+                                        item->acl_access,
+                                        item->acl_access_exec,
+                                        st,
+                                        item->append_or_force,
+                                        &access_with_exec_parsed);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to parse conditionalized execute bit for \"%s\": %m", path);
+
+                r = path_set_acl(FORMAT_PROC_FD_PATH(fd), path, ACL_TYPE_ACCESS, access_with_exec_parsed, item->append_or_force);
+        } else if (item->acl_access)
                 r = path_set_acl(FORMAT_PROC_FD_PATH(fd), path, ACL_TYPE_ACCESS, item->acl_access, item->append_or_force);
 
         /* set only default acls to folders */
@@ -1237,7 +1378,7 @@ static int fd_set_acls(
         }
 
         if (r > 0)
-                return -r; /* already warned */
+                return -r; /* already warned in path_set_acl */
 
         /* The above procfs paths don't work if /proc is not mounted. */
         if (r == -ENOENT && proc_mounted() == 0)
@@ -2867,6 +3008,9 @@ static void item_free_contents(Item *i) {
         if (i->acl_access)
                 acl_free(i->acl_access);
 
+        if (i->acl_access_exec)
+                acl_free(i->acl_access_exec);
+
         if (i->acl_default)
                 acl_free(i->acl_default);
 #endif
diff --git a/test/TEST-22-TMPFILES/test.sh b/test/TEST-22-TMPFILES/test.sh
index 46dd990f79..82d497d50f 100755
--- a/test/TEST-22-TMPFILES/test.sh
+++ b/test/TEST-22-TMPFILES/test.sh
@@ -17,6 +17,8 @@ test_append_files() {
 
         sed -i "s/systemd//g" "$initdir/etc/nsswitch.conf"
     fi
+
+    image_install setfacl
 }
 
 do_test "$@"
diff --git a/test/test-systemd-tmpfiles.py b/test/test-systemd-tmpfiles.py
index 791a88497c..369478d31e 100755
--- a/test/test-systemd-tmpfiles.py
+++ b/test/test-systemd-tmpfiles.py
@@ -13,6 +13,7 @@ import subprocess
 import tempfile
 import pwd
 import grp
+from pathlib import Path
 
 try:
     from systemd import id128
@@ -202,6 +203,27 @@ def test_hard_cleanup(*, user):
 def test_base64():
     test_content('f~ {} - - - - UGlmZgpQYWZmClB1ZmYgCg==', "Piff\nPaff\nPuff \n", user=False)
 
+def test_conditionalized_execute_bit():
+    c = subprocess.run(exe_with_args + ['--version', '|', 'grep', '-F', '+ACL'], shell=True, stdout=subprocess.DEVNULL)
+    if c.returncode != 0:
+        return 0
+
+    d = tempfile.TemporaryDirectory(prefix='test-acl.', dir=temp_dir.name)
+    temp = Path(d.name) / "cond_exec"
+    temp.touch()
+    temp.chmod(0o644)
+
+    test_line(f"a {temp} - - - - u:root:Xwr", user=False, returncode=0)
+    c = subprocess.run(["getfacl", "-Ec", temp],
+                       stdout=subprocess.PIPE, check=True, text=True)
+    assert "user:root:rw-" in c.stdout
+
+    temp.chmod(0o755)
+    test_line(f"a+ {temp} - - - - u:root:Xwr,g:root:rX", user=False, returncode=0)
+    c = subprocess.run(["getfacl", "-Ec", temp],
+                       stdout=subprocess.PIPE, check=True, text=True)
+    assert "user:root:rwx" in c.stdout and "group:root:r-x" in c.stdout
+
 if __name__ == '__main__':
     test_invalids(user=False)
     test_invalids(user=True)
@@ -214,3 +236,5 @@ if __name__ == '__main__':
     test_hard_cleanup(user=True)
 
     test_base64()
+
+    test_conditionalized_execute_bit()
diff --git a/test/units/testsuite-22.16.sh b/test/units/testsuite-22.16.sh
new file mode 100755
index 0000000000..15387cddb8
--- /dev/null
+++ b/test/units/testsuite-22.16.sh
@@ -0,0 +1,36 @@
+#!/bin/bash
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
+# Test for conditionalized execute bit ('X' bit)
+set -eux
+set -o pipefail
+
+# shellcheck source=test/units/assert.sh
+. "$(dirname "$0")"/assert.sh
+
+rm -f /tmp/acl_exec
+touch /tmp/acl_exec
+
+# No ACL set yet
+systemd-tmpfiles --create - <<EOF
+a /tmp/acl_exec - - - - u:root:rwX
+EOF
+assert_in 'user:root:rw-' "$(getfacl -Ec /tmp/acl_exec)"
+
+# Set another ACL and append
+setfacl -m g:root:x /tmp/acl_exec
+
+systemd-tmpfiles --create - <<EOF
+a+ /tmp/acl_exec - - - - u:root:rwX
+EOF
+acl="$(getfacl -Ec /tmp/acl_exec)"
+assert_in 'user:root:rwx' "$acl"
+assert_in 'group:root:--x' "$acl"
+
+# Reset ACL (no append)
+systemd-tmpfiles --create - <<EOF
+a /tmp/acl_exec - - - - u:root:rwX
+EOF
+assert_in 'user:root:rw-' "$(getfacl -Ec /tmp/acl_exec)"
+
+rm -f /tmp/acl_exec