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>

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 697082ad4f0d3910a79c07a3fc72884d920a2092..396ce2e6e02fde10d2003b3ce1a974a33ccac429 100644 (file)
--- a/winsup/cygwin/ChangeLog
+++ b/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.

diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
index 9b54d29bed039bf7e7ab6e4d3fb749a24d3a5103..cf87d2c68f7449dbb0dab704526a12c8dd96938a 100644 (file)
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/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 ())

diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc
index 82e9cb53f064f1f5d3ac6607c27ba6c0ddca3b21..a84cd5e7167a80e891a6c7bc138997dd56b3516a 100644 (file)
--- a/winsup/cygwin/sec_acl.cc
+++ b/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)
-                      | (~aclbufp[idx].a_perm & other_obj);
+               deny = (aclbufp[idx].a_perm & ~class_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. */