From bcb9e71a21f1ba364ca9a2c203de46640a2a7e8d Mon Sep 17 00:00:00 2001
From: Michael Orlitzky <michael@orlitzky.com>
Date: Tue, 14 Aug 2012 17:12:28 -0400
Subject: [PATCH] Add another test and fool around with the logic trying to get
 it to pass.

---
 run-tests.sh |  50 ++++++++++++++--
 src/aclq.c   | 161 ++++++++++++++++++++++++++-------------------------
 2 files changed, 126 insertions(+), 85 deletions(-)

diff --git a/run-tests.sh b/run-tests.sh
index 7f6838f..a5f7692 100755
--- a/run-tests.sh
+++ b/run-tests.sh
@@ -9,12 +9,13 @@ TESTDIR=test
 acl_reset() {
     # Remove any ACLs on our test directory and remove its contents.
     setfacl --remove-all --recursive "$TESTDIR"
-    rm "${TESTDIR}"/*
+    rm -f "${TESTDIR}"/*
 }
 
 compare() {
     if [[ "${ACTUAL}" == "${EXPECTED}" ]]; then
 	echo "Success."
+	acl_reset
     else
 	echo "Failure."
 	echo "Expected result:"
@@ -32,19 +33,39 @@ mkdir "${TESTDIR}"
 
 # When using a minimal ACL, the default user, group, and other
 # permissions should all be propagated to the mode bits.
-
 TARGET="${TESTDIR}"/foo
-touch "${TARGET}"
-chmod 777 "${TARGET}"
-setfacl -d -m user::r-- "${TESTDIR}"
+setfacl -d -m user::r--  "${TESTDIR}"
 setfacl -d -m group::r-- "${TESTDIR}"
 setfacl -d -m other::r-- "${TESTDIR}"
+touch "${TARGET}"
+chmod 777 "${TARGET}"
+./aclq "${TARGET}"
+
+EXPECTED=$(cat <<EOF
+user::r--
+group::r--
+other::r--
+
+EOF
+)
 
+ACTUAL=`getfacl --omit-header "${TARGET}"`
+compare
+
+# Do the same thing as the last test, except with an extended ACL.
+setfacl -d -m user::r--     "${TESTDIR}"
+setfacl -d -m group::r--    "${TESTDIR}"
+setfacl -d -m other::r--    "${TESTDIR}"
+setfacl -d -m user:mail:rwx "${TESTDIR}"
+touch "${TARGET}"
+chmod 777 "${TARGET}"
 ./aclq "${TARGET}"
 
 EXPECTED=$(cat <<EOF
 user::r--
+user:mail:rwx
 group::r--
+mask::rwx
 other::r--
 
 EOF
@@ -52,3 +73,22 @@ EOF
 
 ACTUAL=`getfacl --omit-header "${TARGET}"`
 compare
+
+# A directory shared by a group, should still be group-writable
+# afterwards.
+setfacl -d -m group:mail:rwx "${TESTDIR}"
+touch "${TARGET}"
+chmod 644 "${TARGET}"
+./aclq "${TARGET}"
+
+EXPECTED=$(cat <<EOF
+user::rw-
+group::r--
+group:mail:rwx	#effective:rw-
+mask::rw-
+other::r--
+EOF
+)
+
+ACTUAL=`getfacl --omit-header "${TARGET}"`
+compare
diff --git a/src/aclq.c b/src/aclq.c
index 27b0224..9fca9b2 100644
--- a/src/aclq.c
+++ b/src/aclq.c
@@ -111,24 +111,6 @@ int has_default_tag_acl(const char* path, acl_tag_t desired_tag) {
 }
 
 
-int has_minimal_default_acl(const char* path) {
-  /* An ACL is minimal if and only if it has no mask. Return 1 if it's
-     minimal, zero if it isn't, and -1 on errors. */
-  int has_dm = has_default_tag_acl(path, ACL_MASK);
-
-  if (has_dm == 0) {
-    return 1;
-  }
-  else if (has_dm == 1) {
-    return 0;
-  }
-  else {
-    perror("has_minimal_default_acl");
-    return -1;
-  }
-}
-
-
 int has_default_user_obj_acl(const char* path) {
   return has_default_tag_acl(path, ACL_USER_OBJ);
 }
@@ -141,6 +123,10 @@ int has_default_other_acl(const char* path) {
   return has_default_tag_acl(path, ACL_OTHER);
 }
 
+int has_default_mask_acl(const char* path) {
+  return has_default_tag_acl(path, ACL_MASK);
+}
+
 
 int get_default_tag_entry(const char* path,
 			  acl_tag_t desired_tag,
@@ -381,7 +367,84 @@ int reapply_default_acl(const char* path) {
      to it, and then later reapply the result via chmod. */
   mode_t path_mode = get_mode(path);
 
+  if (has_default_mask_acl(parent)) {
+    /* The parent has an extended ACL. Extended ACLs use the mask
+       entry. */
+
+    /* For the group bits, we'll use the ACL's mask instead of the group
+       object bits. If the default ACL had a group entry, it should
+       already have propagated (but might be masked). */
+    if (has_default_mask_read(parent)) {
+      path_mode |= S_IRGRP;
+    }
+    else {
+      path_mode &= ~S_IRGRP;
+    }
+    
+    if (has_default_mask_write(parent)) {
+      path_mode |= S_IWGRP;
+    }
+    else {
+      path_mode &= ~S_IWGRP;
+    }
+
+    if (!mode_has_perm(path_mode, S_IXGRP)) {
+      /* The group ACL entry should already have been inherited from the
+	 default ACL. If the source was not group executable, we want to
+	 modify the destination so that it is not group executable
+	 either. In the presence of ACLs, the group permissions come not
+	 from the mode bits, but from the group:: ACL entry. So, to do
+	 this, we remove the group::x entry. */
+      remove_default_group_obj_execute(path);
+    }
+
+    /* We need to determine whether or not to mask the execute
+       bit. This applies not only to the user/group/other entries, but
+       also to all other named entries. If the original file wasn't
+       executable, then the result probably should not be. To
+       determine whether or not "it was executable", we rely on the
+       user execute bits. Obviously this should be done before we
+       twiddle that bit. */
+    if (has_default_mask_execute(parent)) {
+      if (mode_has_perm(path_mode, S_IXUSR)) {
+	/* This just adds the group execute bit, and doesn't actually
+	     grant group execute permissions. */
+	path_mode |= S_IXGRP;
+      }
+    }
+    else {
+      path_mode &= ~S_IXGRP;
+    }
+
+  }
+  else {
+    /* It's a minimal ACL. We'll repeat for the group bits what we
+       already did for the owner/other bits. */
+    if (has_default_group_obj_acl(parent)) {
+      if (has_default_group_obj_read(parent)) {
+	path_mode |= S_IRGRP;
+      }
+      else {
+	path_mode &= ~S_IRGRP;
+      }
+
+
+      if (has_default_group_obj_write(parent)) {
+	path_mode |= S_IWGRP;
+      }
+      else {
+	path_mode &= ~S_IWGRP;
+      }
 
+      /* We don't want to set the execute bit on via the ACL unless it
+	 was on originally. */
+      if (!has_default_group_obj_execute(parent)) {
+	path_mode &= ~S_IXGRP;
+      }
+    }
+  }
+  
+    
   /* If parent has a default user ACL, apply it. */
   if (has_default_user_obj_acl(parent)) {
 
@@ -440,68 +503,6 @@ int reapply_default_acl(const char* path) {
     }
   }
 
-
-  if (has_minimal_default_acl(parent)) {
-    /* With a minimal ACL, we'll repeat for the group bits what we
-       already did for the owner/other bits. */
-    if (has_default_group_obj_acl(parent)) {
-      if (has_default_group_obj_read(parent)) {
-	path_mode |= S_IRGRP;
-      }
-      else {
-	path_mode &= ~S_IRGRP;
-      }
-
-
-      if (has_default_group_obj_write(parent)) {
-	path_mode |= S_IWGRP;
-      }
-      else {
-	path_mode &= ~S_IWGRP;
-      }
-
-      /* We don't want to set the execute bit on via the ACL unless it
-	 was on originally. */
-      if (!has_default_group_obj_execute(parent)) {
-	path_mode &= ~S_IXGRP;
-      }
-    }
-  }
-  else {
-    /* The parent has an extended ACL. Extended ACLs use the mask
-       entry. */
-
-    /* For the group bits, we'll use the ACL's mask instead of the group
-       object bits. If the default ACL had a group entry, it should
-       already have propagated (but might be masked). */
-    if (has_default_mask_read(parent)) {
-      path_mode |= S_IRGRP;
-    }
-
-    if (has_default_mask_write(parent)) {
-      path_mode |= S_IWGRP;
-    }
-
-    /* Honor the mask execute unconditionally. */
-    if (has_default_mask_execute(parent)) {
-      /* This just adds the group execute bit, and doesn't actually
-	 grant group execute permissions. */
-      path_mode |= S_IXGRP;
-    }
-
-    if (!mode_has_perm(path_mode, S_IXGRP)) {
-      /* The group ACL entry should already have been inherited from the
-	 default ACL. If the source was not group executable, we want to
-	 modify the destination so that it is not group executable
-	 either. In the presence of ACLs, the group permissions come not
-	 from the mode bits, but from the group:: ACL entry. So, to do
-	 this, we remove the group::x entry. */
-      if (has_default_group_obj_execute(path)) {
-	remove_default_group_obj_execute(path);
-      }
-    }
-  }
-
   int chmod_result = chmod(path, path_mode);
   if (chmod_result == 0) {
     return 1;
-- 
2.44.2