From 83a5db202d94683b10eb1c3cb88b3d0cc0febfdd Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 5 Mar 2024 22:41:54 +0800 Subject: [PATCH 1/3] tmpfiles: remove one more use of goto and modernization --- src/tmpfiles/tmpfiles.c | 139 +++++++++++++++++----------------------- 1 file changed, 60 insertions(+), 79 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index aad740a2b8..0dd890958c 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1258,96 +1258,78 @@ static int parse_acls_from_arg(Item *item) { #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, + acl_t cond_exec, + acl_t access, /* could be empty (NULL) */ 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); + assert(cond_exec); + assert(ret); - parsed = access ? acl_dup(access) : acl_init(0); + if (!S_ISDIR(st->st_mode)) { + 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; + } + } else + has_exec = true; + + _cleanup_(acl_freep) acl_t 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)) { @@ -1360,6 +1342,7 @@ static int parse_acl_cond_exec( if (acl_copy_entry(parsed_entry, entry) < 0) return -errno; + /* We substituted 'X' with 'x' in parse_acl(), so drop execute bit here if not applicable. */ if (!has_exec) { if (acl_get_permset(parsed_entry, &permset) < 0) return -errno; @@ -1371,7 +1354,6 @@ static int parse_acl_cond_exec( if (r < 0) return -errno; -finish: if (!append) { /* want_mask = true */ r = calc_acl_mask_if_needed(&parsed); if (r < 0) @@ -1476,10 +1458,9 @@ static int fd_set_acls( } if (item->acl_access_exec) { - r = parse_acl_cond_exec(FORMAT_PROC_FD_PATH(fd), - item->acl_access, + r = parse_acl_cond_exec(FORMAT_PROC_FD_PATH(fd), st, item->acl_access_exec, - st, + item->acl_access, item->append_or_force, &access_with_exec_parsed); if (r < 0) From 29a438e764cbfdddd43e175490e2d8c8eb21b79e Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 5 Mar 2024 22:25:44 +0800 Subject: [PATCH 2/3] tmpfiles: do 'X' bit check in an ACL-aware manner Follow-up for 26d98cdd78cb5283f5771bd5866997acc494b067 I.e. stat() cannot be used here. Also, before this commit, the 'X' is only applied if the owner has execute bit set. Now it takes group and other into consideration too. setfacl(1) also has the same behavior. --- src/tmpfiles/tmpfiles.c | 46 +++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 0dd890958c..ede5ca8a96 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1275,33 +1275,43 @@ static int parse_acl_cond_exec( assert(ret); if (!S_ISDIR(st->st_mode)) { - has_exec = st->st_mode & S_IXUSR; + _cleanup_(acl_freep) acl_t old = NULL; - if (!has_exec && append) { - _cleanup_(acl_freep) acl_t old = NULL; + old = acl_get_file(path, ACL_TYPE_ACCESS); + if (!old) + return -errno; - old = acl_get_file(path, ACL_TYPE_ACCESS); - if (!old) + has_exec = false; + + for (r = acl_get_entry(old, ACL_FIRST_ENTRY, &entry); + r > 0; + r = acl_get_entry(old, ACL_NEXT_ENTRY, &entry)) { + + acl_tag_t tag; + + if (acl_get_tag_type(entry, &tag) < 0) return -errno; - for (r = acl_get_entry(old, ACL_FIRST_ENTRY, &entry); - r > 0; - r = acl_get_entry(old, ACL_NEXT_ENTRY, &entry)) { + if (tag == ACL_MASK) + continue; - if (acl_get_permset(entry, &permset) < 0) - return -errno; + /* If not appending, skip ACL definitions */ + if (!append && IN_SET(tag, ACL_USER, ACL_GROUP)) + continue; - r = acl_get_perm(permset, ACL_EXECUTE); - if (r < 0) - return -errno; - if (r > 0) { - has_exec = true; - break; - } - } + 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) { From 22549ff4735d0820934b942998a066a6c612f7b2 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 5 Mar 2024 17:16:49 +0800 Subject: [PATCH 3/3] tmpfiles.d/systemd: use ACL 'X' bit where appropriate --- tmpfiles.d/systemd.conf.in | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tmpfiles.d/systemd.conf.in b/tmpfiles.d/systemd.conf.in index ce7206145e..d36f0694af 100644 --- a/tmpfiles.d/systemd.conf.in +++ b/tmpfiles.d/systemd.conf.in @@ -26,16 +26,13 @@ Z /run/log/journal/%m ~2750 root systemd-journal - - {% if HAVE_ACL %} {% if ENABLE_ADM_GROUP and ENABLE_WHEEL_GROUP %} a+ /run/log/journal - - - - d:group::r-x,d:group:adm:r-x,d:group:wheel:r-x,group::r-x,group:adm:r-x,group:wheel:r-x -a+ /run/log/journal/%m - - - - d:group:adm:r-x,d:group:wheel:r-x,group:adm:r-x,group:wheel:r-x -a+ /run/log/journal/%m/*.journal* - - - - group:adm:r--,group:wheel:r-- +A+ /run/log/journal/%m - - - - d:group:adm:r-x,d:group:wheel:r-x,group:adm:r-X,group:wheel:r-X {% elif ENABLE_ADM_GROUP %} a+ /run/log/journal - - - - d:group::r-x,d:group:adm:r-x,group::r-x,group:adm:r-x -a+ /run/log/journal/%m - - - - d:group:adm:r-x,group:adm:r-x -a+ /run/log/journal/%m/*.journal* - - - - group:adm:r-- +A+ /run/log/journal/%m - - - - d:group:adm:r-x,group:adm:r-X {% elif ENABLE_WHEEL_GROUP %} a+ /run/log/journal - - - - d:group::r-x,d:group:wheel:r-x,group::r-x,group:wheel:r-x -a+ /run/log/journal/%m - - - - d:group:wheel:r-x,group:wheel:r-x -a+ /run/log/journal/%m/*.journal* - - - - group:wheel:r-- +A+ /run/log/journal/%m - - - - d:group:wheel:r-x,group:wheel:r-X {% endif %} {% endif %}