Fix up POSIX permission handling

* fhandler_disk_file.cc (fhandler_disk_file::fchmod): Disable
	deviation from POSIX 1003.1e in terms of GROUP_OBJ/CLASS_OBJ
	permissions.  Follow POSIX 1003.1e again.  Keep old code in
	for future reference.
	* sec_acl.cc: Accommodate changes in ACE creation in leading
	comment.
	(set_posix_access): Fix user deny ACE creation.  Split group
	deny ACE creation into two steps, one to reflect CLASS_OBJ,
	the other to reflect OTHER_OBJ.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

winsup/cygwin/ChangeLog

@@ -1,3 +1,15 @@
+2015-11-18  Corinna Vinschen  <corinna@vinschen.de>
+
+	* fhandler_disk_file.cc (fhandler_disk_file::fchmod): Disable
+	deviation from POSIX 1003.1e in terms of GROUP_OBJ/CLASS_OBJ
+	permissions.  Follow POSIX 1003.1e again.  Keep old code in
+	for future reference.
+	* sec_acl.cc: Accommodate changes in ACE creation in leading
+	comment.
+	(set_posix_access): Fix user deny ACE creation.  Split group
+	deny ACE creation into two steps, one to reflect CLASS_OBJ,
+	the other to reflect OTHER_OBJ.
+
 2015-11-18  Corinna Vinschen  <corinna@vinschen.de>
 
 	* sec_acl.cc (getacl): Use Authz to fetch correct user permissions.

winsup/cygwin/fhandler_disk_file.cc

@@ -915,6 +915,7 @@ fhandler_disk_file::fchmod (mode_t mode)
 	      /* Overwrite ACL permissions as required by POSIX 1003.1e
 		 draft 17. */
 	      aclp[0].a_perm = (mode >> 6) & S_IRWXO;
+#if 0
 	      /* Deliberate deviation from POSIX 1003.1e here.  We're not
 		 writing CLASS_OBJ *or* GROUP_OBJ, but both.  Otherwise we're
 		 going to be in constant trouble with user expectations. */
@@ -923,6 +924,15 @@ fhandler_disk_file::fchmod (mode_t mode)
 	      if (nentries > MIN_ACL_ENTRIES
 		  && (idx = searchace (aclp, nentries, CLASS_OBJ)) >= 0)
 		aclp[idx].a_perm = (mode >> 3) & S_IRWXO;
+#else
+	      /* POSIXly correct: If CLASS_OBJ is present, chmod only modifies
+		 CLASS_OBJ, not GROUP_OBJ. */
+	      if (nentries > MIN_ACL_ENTRIES
+		  && (idx = searchace (aclp, nentries, CLASS_OBJ)) >= 0)
+		aclp[idx].a_perm = (mode >> 3) & S_IRWXO;
+	      else if ((idx = searchace (aclp, nentries, GROUP_OBJ)) >= 0)
+		aclp[idx].a_perm = (mode >> 3) & S_IRWXO;
+#endif
 	      if ((idx = searchace (aclp, nentries, OTHER_OBJ)) >= 0)
 		aclp[idx].a_perm = mode & S_IRWXO;
 	      if (pc.isdir ())

winsup/cygwin/sec_acl.cc

@@ -37,25 +37,30 @@ details. */
      or
        USER_OBJ deny ACE   == ~USER_OBJ & (GROUP_OBJ | OTHER_OBJ)
 
-   - USER deny.  If a user has different permissions from CLASS_OBJ, or if the
+   - USER deny.  If a user has more permissions than CLASS_OBJ, or if the
      user has less permissions than OTHER_OBJ, deny the excess permissions.
 
-       USER deny ACE       == (USER ^ CLASS_OBJ) | (~USER & OTHER_OBJ)
+       USER deny ACE       == (USER & ~CLASS_OBJ) | (~USER & OTHER_OBJ)
 
    - USER_OBJ  allow ACE
    - USER      allow ACEs
 
-     The POSIX permissions returned for a USER entry are the allow bits alone!
+   The POSIX permissions returned for a USER entry are the allow bits alone!
 
    - GROUP{_OBJ} deny.  If a group has more permissions than CLASS_OBJ,
      or less permissions than OTHER_OBJ, deny the excess permissions.
 
-       GROUP{_OBJ} deny ACEs  == (GROUP & ~CLASS_OBJ) | (~GROUP & OTHER_OBJ)
+       GROUP{_OBJ} deny ACEs  == (GROUP & ~CLASS_OBJ)
 
    - GROUP_OBJ	allow ACE
    - GROUP	allow ACEs
 
-     The POSIX permissions returned for a GROUP entry are the allow bits alone!
+   The POSIX permissions returned for a GROUP entry are the allow bits alone!
+
+   - 2. GROUP{_OBJ} deny.  If a group has less permissions than OTHER_OBJ,
+     deny the excess permissions.
+
+       2. GROUP{_OBJ} deny ACEs  == (~GROUP & OTHER_OBJ)
 
    - OTHER_OBJ	allow ACE
 
@@ -311,7 +316,9 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid,
 	   check_types < CLASS_OBJ;
 	   check_types <<= 2)
 	{
-	  /* Create deny ACEs for users, then groups. */
+	  /* Create deny ACEs for users, then 1st run for groups.  For groups,
+	     only take CLASS_OBJ permissions into account.  Class permissions
+	     are handled in the 2nd deny loop below. */
 	  for (start_idx = idx;
 	       idx < nentries && aclbufp[idx].a_type & check_types;
 	       ++idx)
@@ -327,7 +334,7 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid,
 	      if (aclbufp[idx].a_type & USER_OBJ)
 		deny = ~aclbufp[idx].a_perm & (class_obj | other_obj);
 	      else if (aclbufp[idx].a_type & USER)
-		deny = (aclbufp[idx].a_perm ^ class_obj)
+		deny = (aclbufp[idx].a_perm & ~class_obj)
 		       | (~aclbufp[idx].a_perm & other_obj);
 	      /* Accommodate Windows: Only generate deny masks for SYSTEM
 		 and the Administrators group in terms of the execute bit,
@@ -337,8 +344,7 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid,
 			   || aclsid[idx] == well_known_admins_sid))
 		deny = aclbufp[idx].a_perm & ~(class_obj | S_IROTH | S_IWOTH);
 	      else
-		deny = (aclbufp[idx].a_perm & ~class_obj)
+		deny = (aclbufp[idx].a_perm & ~class_obj);
-		       | (~aclbufp[idx].a_perm & other_obj);
 	      if (!deny)
 		continue;
 	      access = 0;
@@ -391,6 +397,36 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid,
 					   inherit))
 		return NULL;
 	    }
+	  /* 2nd deny loop: Create deny ACEs for groups when they have less
+	     permissions than OTHER_OBJ. */
+	  if (check_types == (GROUP_OBJ | GROUP))
+	    for (idx = start_idx;
+		 idx < nentries && aclbufp[idx].a_type & check_types;
+		 ++idx)
+	      {
+		if (aclbufp[idx].a_type & GROUP && aclsid[idx] == group)
+		  continue;
+		/* Only generate deny masks for SYSTEM and the Administrators
+		   group if they are the primary group. */
+		if (aclbufp[idx].a_type & GROUP
+		    && (aclsid[idx] == well_known_system_sid
+			|| aclsid[idx] == well_known_admins_sid))
+		  deny = 0;
+		else
+		  deny = (~aclbufp[idx].a_perm & other_obj);
+		if (!deny)
+		  continue;
+		access = 0;
+		if (deny & S_IROTH)
+		  access |= FILE_DENY_READ;
+		if (deny & S_IWOTH)
+		  access |= FILE_DENY_WRITE;
+		if (deny & S_IXOTH)
+		  access |= FILE_DENY_EXEC;
+		if (!add_access_denied_ace (acl, access, aclsid[idx], acl_len,
+					    inherit))
+		  return NULL;
+	      }
 	}
       /* For ptys if the admins group isn't in the ACL, add an ACE to make
 	 sure the group has WRITE_DAC and WRITE_OWNER perms. */