shared/acl-util: add mask only when needed, always add base ACLs

For ACLs to be valid, a set of entries for user, group, and other
must be always present. Always add those entries.

While at it, only add the mask ACL if it is actually required, i.e.
when at least on ACL for non-owner group or user exists.

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 957910dd6dbdf9232c45a386013f2aba9d87669c..8815bf997017ae475063da2ec70b7f49e1bf4c8a 100644 (file)
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -306,16 +306,16 @@
           <term><varname>a</varname></term>
           <term><varname>a+</varname></term>
           <listitem><para>Set POSIX ACLs (access control lists). If
           <term><varname>a</varname></term>
           <term><varname>a+</varname></term>
           <listitem><para>Set POSIX ACLs (access control lists). If

-          suffixed with <varname>+</varname>, specified mask will be
-          added to existing
-          entries. <command>systemd-tmpfiles</command> does not
-          automatically add the required base entries for user and
-          group to the specified mask, so they must be specified
-          explicitly if <varname>+</varname> is not used. The
-          mask will be added if not specified explicitly.
-          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.  </para></listitem>
+          suffixed with <varname>+</varname>, 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 explictly 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.</para></listitem>

         </varlistentry>
 
         <varlistentry>
         </varlistentry>
 
         <varlistentry>

diff --git a/src/shared/acl-util.c b/src/shared/acl-util.c
index 950f472ddd5329944dbe49e7e48b4776aff18050..a4ff1ab878e12aa3d2bff6dfc0cdbe22c5b47619 100644 (file)
--- a/src/shared/acl-util.c
+++ b/src/shared/acl-util.c
@@ -29,14 +29,14 @@
 
 int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry) {
         acl_entry_t i;
 
 int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry) {
         acl_entry_t i;

-        int found;
+        int r;

 
         assert(acl);
         assert(entry);
 
 
         assert(acl);
         assert(entry);
 

-        for (found = acl_get_entry(acl, ACL_FIRST_ENTRY, &i);
-             found > 0;
-             found = acl_get_entry(acl, ACL_NEXT_ENTRY, &i)) {
+        for (r = acl_get_entry(acl, ACL_FIRST_ENTRY, &i);
+             r > 0;
+             r = acl_get_entry(acl, ACL_NEXT_ENTRY, &i)) {

 
                 acl_tag_t tag;
                 uid_t *u;
 
                 acl_tag_t tag;
                 uid_t *u;


@@ -60,8 +60,7 @@ int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry) {
                         return 1;
                 }
         }
                         return 1;
                 }
         }

-
-        if (found < 0)
+        if (r < 0)

                 return -errno;
 
         return 0;
                 return -errno;
 
         return 0;


@@ -69,14 +68,13 @@ int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry) {
 
 int calc_acl_mask_if_needed(acl_t *acl_p) {
         acl_entry_t i;
 
 int calc_acl_mask_if_needed(acl_t *acl_p) {
         acl_entry_t i;

-        int found;
+        int r;

 
         assert(acl_p);
 
 
         assert(acl_p);
 

-        for (found = acl_get_entry(*acl_p, ACL_FIRST_ENTRY, &i);
-             found > 0;
-             found = acl_get_entry(*acl_p, ACL_NEXT_ENTRY, &i)) {
-
+        for (r = acl_get_entry(*acl_p, ACL_FIRST_ENTRY, &i);
+             r > 0;
+             r = acl_get_entry(*acl_p, ACL_NEXT_ENTRY, &i)) {

                 acl_tag_t tag;
 
                 if (acl_get_tag_type(i, &tag) < 0)
                 acl_tag_t tag;
 
                 if (acl_get_tag_type(i, &tag) < 0)


@@ -84,14 +82,80 @@ int calc_acl_mask_if_needed(acl_t *acl_p) {
 
                 if (tag == ACL_MASK)
                         return 0;
 
                 if (tag == ACL_MASK)
                         return 0;

+                if (IN_SET(tag, ACL_USER, ACL_GROUP))
+                        goto calc;

         }
         }

-
-        if (found < 0)
+        if (r < 0)

                 return -errno;
                 return -errno;

+        return 0;

 
 

+calc:

         if (acl_calc_mask(acl_p) < 0)
                 return -errno;
         if (acl_calc_mask(acl_p) < 0)
                 return -errno;

+        return 1;
+}
+
+int add_base_acls_if_needed(acl_t *acl_p, const char *path) {
+        acl_entry_t i;
+        int r;
+        bool have_user_obj = false, have_group_obj = false, have_other = false;
+        struct stat st;
+        _cleanup_(acl_freep) acl_t basic = NULL;
+
+        assert(acl_p);
+
+        for (r = acl_get_entry(*acl_p, ACL_FIRST_ENTRY, &i);
+             r > 0;
+             r = acl_get_entry(*acl_p, ACL_NEXT_ENTRY, &i)) {
+                acl_tag_t tag;
+
+                if (acl_get_tag_type(i, &tag) < 0)
+                        return -errno;
+
+                if (tag == ACL_USER_OBJ)
+                        have_user_obj = true;
+                else if (tag == ACL_GROUP_OBJ)
+                        have_group_obj = true;
+                else if (tag == ACL_OTHER)
+                        have_other = true;
+                if (have_user_obj && have_group_obj && have_other)
+                        return 0;
+        }
+        if (r < 0)
+                return -errno;

 
 

+        r = stat(path, &st);
+        if (r < 0)
+                return -errno;
+
+        basic = acl_from_mode(st.st_mode);
+        if (!basic)
+                return -errno;
+
+        for (r = acl_get_entry(basic, ACL_FIRST_ENTRY, &i);
+             r > 0;
+             r = acl_get_entry(basic, ACL_NEXT_ENTRY, &i)) {
+                acl_tag_t tag;
+                acl_entry_t dst;
+
+                if (acl_get_tag_type(i, &tag) < 0)
+                        return -errno;
+
+                if ((tag == ACL_USER_OBJ && have_user_obj) ||
+                    (tag == ACL_GROUP_OBJ && have_group_obj) ||
+                    (tag == ACL_OTHER && have_other))
+                        continue;
+
+                r = acl_create_entry(acl_p, &dst);
+                if (r < 0)
+                        return -errno;
+
+                r = acl_copy_entry(dst, i);
+                if (r < 0)
+                        return -errno;
+        }
+        if (r < 0)
+                return -errno;

         return 0;
 }
 
         return 0;
 }
 


@@ -221,15 +285,15 @@ int parse_acl(char *text, acl_t *acl_access, acl_t *acl_default, bool want_mask)
 int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *acl) {
         _cleanup_(acl_freep) acl_t old;
         acl_entry_t i;
 int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *acl) {
         _cleanup_(acl_freep) acl_t old;
         acl_entry_t i;

-        int found, r;
+        int r;

 
         old = acl_get_file(path, type);
         if (!old)
                 return -errno;
 
 
         old = acl_get_file(path, type);
         if (!old)
                 return -errno;
 

-        for (found = acl_get_entry(new, ACL_FIRST_ENTRY, &i);
-             found > 0;
-             found = acl_get_entry(new, ACL_NEXT_ENTRY, &i)) {
+        for (r = acl_get_entry(new, ACL_FIRST_ENTRY, &i);
+             r > 0;
+             r = acl_get_entry(new, ACL_NEXT_ENTRY, &i)) {

 
                 acl_entry_t j;
 
 
                 acl_entry_t j;
 


@@ -239,10 +303,8 @@ int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *acl) {
                 if (acl_copy_entry(j, i) < 0)
                         return -errno;
         }
                 if (acl_copy_entry(j, i) < 0)
                         return -errno;
         }

-
-        r = calc_acl_mask_if_needed(&old);

         if (r < 0)
         if (r < 0)

-                return r;
+                return -errno;

 
         *acl = old;
         old = NULL;
 
         *acl = old;
         old = NULL;

diff --git a/src/shared/acl-util.h b/src/shared/acl-util.h
index 1ad4a2ebc77e93cab85d542d61daf9ee4abda1ce..90e88ffa26373708027626abf475c1a67ac99983 100644 (file)
--- a/src/shared/acl-util.h
+++ b/src/shared/acl-util.h
@@ -31,6 +31,7 @@
 
 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 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 search_acl_groups(char*** dst, const char* path, bool* belong);
 int parse_acl(char *text, acl_t *acl_access, acl_t *acl_default, bool want_mask);
 int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *acl);
 int search_acl_groups(char*** dst, const char* path, bool* belong);
 int parse_acl(char *text, acl_t *acl_access, acl_t *acl_default, bool want_mask);
 int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *acl);

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 3c8993e8942f7b6736cac91d9a958021621941bc..7edeeb742840ffbe96ed32a7b0e8c8b8873ee121 100644 (file)
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -614,22 +614,35 @@ static int get_acls_from_arg(Item *item) {
 }
 
 static int path_set_acl(const char *path, acl_type_t type, acl_t acl, bool modify) {
 }
 
 static int path_set_acl(const char *path, acl_type_t type, acl_t acl, bool modify) {

-        _cleanup_(acl_freep) acl_t cleanme = NULL;
+        _cleanup_(acl_freep) acl_t dup = NULL;

         int r;
 
         if (modify) {
         int r;
 
         if (modify) {

-                r = acls_for_file(path, type, acl, &cleanme);
+                r = acls_for_file(path, type, acl, &dup);

                 if (r < 0)
                         return r;
                 if (r < 0)
                         return r;

-                acl = cleanme;
-        };

 
 

-        r = acl_set_file(path, type, acl);
+                r = calc_acl_mask_if_needed(&dup);
+                if (r < 0)
+                        return r;
+        } else {
+                dup = acl_dup(acl);
+                if (!dup)
+                        return -errno;
+
+                /* the mask was already added earlier if needed */
+        }
+
+        r = add_base_acls_if_needed(&dup, path);
+        if (r < 0)
+                return r;
+
+        r = acl_set_file(path, type, dup);

         if (r < 0) {
                 _cleanup_(acl_free_charpp) char *t;
 
                 r = -errno;
         if (r < 0) {
                 _cleanup_(acl_free_charpp) char *t;
 
                 r = -errno;

-                t = acl_to_any_text(acl, NULL, ',', TEXT_ABBREVIATE);
+                t = acl_to_any_text(dup, NULL, ',', TEXT_ABBREVIATE);

                 log_error_errno(r,
                                 "Setting %s ACL \"%s\" on %s failed: %m",
                                 type == ACL_TYPE_ACCESS ? "access" : "default",
                 log_error_errno(r,
                                 "Setting %s ACL \"%s\" on %s failed: %m",
                                 type == ACL_TYPE_ACCESS ? "access" : "default",