src/libadacl.c: only update entries with matching qualifiers.

This fixes the bug reported by Michał Bartoszkiewicz wherein multiple
named user/group entries clobber one another. Now, when updating an
existing ACL entry with a given one, we check that their qualifiers
match. This can mean one of three things:

  1. Both entries are named user entries, and their UIDs match.

  2. Both entries are named group entries, and their GIDs match.

  3. Both entries are neither named user or group entries, and
     their qualifiers "match vacuously." That is to say: they
     don't have any qualifiers that should match in the first
     place, so we want to update the entry regardless.

This passes the regression test case that I recently added, and (I
hope) fixes the problem entirely.

diff --git a/src/libadacl.c b/src/libadacl.c
index 5bca89fe405dc1a030bdab0ef4ccd55b8d5eab71..aa473c4d696db186e2271d3ed92a09ed9c48596b 100644 (file)
--- a/src/libadacl.c
+++ b/src/libadacl.c
@@ -259,6 +259,17 @@ int acl_update_entry(acl_t aclp, acl_entry_t entry) {
     return ACL_ERROR;
   }
 
+  /* This can allocate memory, so from here on out we have to jump to
+     the "cleanup" label to exit. */
+  void* entry_qualifier = acl_get_qualifier(entry);
+  if (entry_qualifier == NULL &&
+      (entry_tag == ACL_USER || entry_tag == ACL_GROUP)) {
+    /* acl_get_qualifier() can return NULL, but it shouldn't for
+       ACL_USER or ACL_GROUP entries. */
+    perror("acl_update_entry (acl_get_qualifier)");
+    return ACL_ERROR;
+  }
+
   /* Our return value. Default to failure, and change to success if we
      actually update something. */
   int result = ACL_FAILURE;
@@ -277,15 +288,53 @@ int acl_update_entry(acl_t aclp, acl_entry_t entry) {
     }
 
     if (existing_tag == entry_tag) {
-      /* If we update something, we're done and return ACL_SUCCESS */
-      if (acl_set_permset(existing_entry, entry_permset) == ACL_ERROR) {
-       perror("acl_update_entry (acl_set_permset)");
-       result = ACL_ERROR;
-       goto cleanup;
+      /* Our tag types match, but if we have a named user or group
+         entry, then we need to check that the user/group (that is,
+         the qualifier) matches too. */
+      bool qualifiers_match = false;
+
+      /* There are three ways the qualifiers can match... */
+      void* existing_qualifier = acl_get_qualifier(existing_entry);
+      if (existing_qualifier == NULL) {
+        if (existing_tag == ACL_USER || existing_tag == ACL_GROUP) {
+          perror("acl_update_entry (acl_get_qualifier)");
+          result = ACL_ERROR;
+          goto cleanup;
+        }
+        else {
+          /* First, we could be dealing with an entry that isn't a
+             named user or group, in which case they "match
+             vacuously." */
+          qualifiers_match = true;
+        }
       }
 
-      result = ACL_SUCCESS;
-      goto cleanup;
+      /* Otherwise, we have to have matching UIDs or GIDs. */
+      if (entry_tag == ACL_USER) {
+        qualifiers_match = ( *((uid_t*)existing_qualifier)
+                             ==
+                             *((uid_t*)entry_qualifier) );
+      }
+      else if (entry_tag == ACL_GROUP) {
+        qualifiers_match = ( *((gid_t*)existing_qualifier)
+                             ==
+                             *((gid_t*)entry_qualifier) );
+      }
+
+      /* Be sure to free this inside the loop, where memory is allocated. */
+      acl_free(existing_qualifier);
+
+      if (qualifiers_match) {
+        /* If we update something, we're done and return ACL_SUCCESS */
+        if (acl_set_permset(existing_entry, entry_permset) == ACL_ERROR) {
+          perror("acl_update_entry (acl_set_permset)");
+          result = ACL_ERROR;
+          goto cleanup;
+        }
+
+       result = ACL_SUCCESS;
+        goto cleanup;
+      }
     }
 
     get_entry_result = acl_get_entry(aclp, ACL_NEXT_ENTRY, &existing_entry);
@@ -299,6 +348,7 @@ int acl_update_entry(acl_t aclp, acl_entry_t entry) {
   }
 
  cleanup:
+  acl_free(entry_qualifier);
   return result;
 }