https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=ac4648c13eab48d1e7626d272ae47839da579429

commit ac4648c13eab48d1e7626d272ae47839da579429
Author: Corinna Vinschen <cori...@vinschen.de>
Date:   Thu Jan 28 22:05:49 2016 +0100

    Treat ACLs with extra ACEs for Admins and SYSTEM like a trivial ACL
    
        POSIX.1e requires that chmod changes the MASK rather than the
        GROUP_OBJ value if the ACL is non-trivial.
    
        On Windows, especially on home machines, a standard ACL often
        consists of entries for the user, maybe the group, and additional
        entries for SYSTEM and the Administrators group.  A user calling
        chmod on a file with bog standard Windows perms usually expects
        that chmod changes the GROUP_OBJ perms, but given the rules from
        POSIX.1e we can't do that.
    
        However, since we already treat Admins and SYSTEM special in a
        ACL (they are not used in MASK computations) we go a step in the
        Windows direction to follow user expectations.  If an ACL only
        consists of the three POSIX permissions, plus entries for Admins
        and SYSTEM *only*, then we change the permissions of the GROUP_OBJ
        entry *and* the MASK entry.
    
        * fhandler_disk_file.cc (fhandler_disk_file::chmod): Drop unused
        code.  Add special handling for a "standard" Windows ACL.  Add
        comment to explain.
        * sec_acl.cc (get_posix_access): Allow to return "standard-ness"
        of an ACL to the caller.  Add preceeding comment to explain a bit.
        * security.h (get_posix_access): Align prototype.
    
    Signed-off-by: Corinna Vinschen <cori...@vinschen.de>

Diff:
---
 winsup/cygwin/fhandler_disk_file.cc | 30 +++++++++++++++---------------
 winsup/cygwin/sec_acl.cc            | 18 ++++++++++++++----
 winsup/cygwin/security.h            |  2 +-
 3 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/winsup/cygwin/fhandler_disk_file.cc 
b/winsup/cygwin/fhandler_disk_file.cc
index 4ee67e2..7d729e3 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -787,35 +787,35 @@ fhandler_disk_file::fchmod (mode_t mode)
       gid_t gid;
       tmp_pathbuf tp;
       aclent_t *aclp;
+      bool standard_acl = false;
       int nentries, idx;
 
       if (!get_file_sd (get_handle (), pc, sd, false))
        {
          aclp = (aclent_t *) tp.c_get ();
          if ((nentries = get_posix_access (sd, NULL, &uid, &gid,
-                                           aclp, MAX_ACL_ENTRIES)) >= 0)
+                                           aclp, MAX_ACL_ENTRIES,
+                                           &standard_acl)) >= 0)
            {
              /* 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. */
-             if ((idx = searchace (aclp, nentries, GROUP_OBJ)) >= 0)
-               aclp[idx].a_perm = (mode >> 3) & S_IRWXO;
-             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. */
+                CLASS_OBJ, not GROUP_OBJ.
+
+                Deliberate deviation from POSIX 1003.1e:  If the ACL is a
+                "standard" ACL, that is, it only contains POSIX permissions
+                as well as entries for the Administrators group and SYSTEM,
+                then it's kind of a POSIX-only ACL in a twisted, Windowsy
+                way.  If so, we change GROUP_OBJ and CLASS_OBJ perms. */
+             if (standard_acl
+                 && (idx = searchace (aclp, nentries, GROUP_OBJ)) >= 0)
+               aclp[idx].a_perm = (mode >> 3) & S_IRWXO;
              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 be54423..96c6fc3 100644
--- a/winsup/cygwin/sec_acl.cc
+++ b/winsup/cygwin/sec_acl.cc
@@ -561,11 +561,17 @@ getace (aclent_t &acl, int type, int id, DWORD 
win_ace_mask,
 
 /* From the SECURITY_DESCRIPTOR given in psd, compute user, owner, posix
    attributes, as well as the POSIX acl.  The function returns the number
-   of entries returned in aclbufp, or -1 in case of error. */
+   of entries returned in aclbufp, or -1 in case of error.
+
+   When called from chmod, it also returns the fact if the ACL is a "standard"
+   ACL.  A "standard" ACL is an ACL which only consists of ACEs for owner,
+   group, other, as well as (this is Windows) the Administrators group and
+   SYSTEM.  See fhandler_disk_file::fchmod for how this is used to fake
+   stock POSIX perms even if Administrators and SYSTEM is in the ACE. */
 int
 get_posix_access (PSECURITY_DESCRIPTOR psd,
                  mode_t *attr_ret, uid_t *uid_ret, gid_t *gid_ret,
-                 aclent_t *aclbufp, int nentries)
+                 aclent_t *aclbufp, int nentries, bool *std_acl)
 {
   tmp_pathbuf tp;
   NTSTATUS status;
@@ -852,7 +858,10 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
                         GROUP_OBJ entry. */
                      if (ace_sid != well_known_system_sid
                          && ace_sid != well_known_admins_sid)
-                       class_perm |= lacl[pos].a_perm;
+                       {
+                         class_perm |= lacl[pos].a_perm;
+                         standard_ACEs_only = false;
+                       }
                    }
                }
              /* For a newly created file, we'd like to know if we're running
@@ -861,7 +870,6 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
                 a standard ACL, we apply umask.  That's not entirely correct,
                 but it's probably the best we can do. */
              else if (type & (USER | GROUP)
-                      && just_created
                       && standard_ACEs_only
                       && ace_sid != well_known_system_sid
                       && ace_sid != well_known_admins_sid)
@@ -1104,6 +1112,8 @@ out:
        aclbufp[idx].a_perm &= S_IRWXO;
       aclsort32 (pos, 0, aclbufp);
     }
+  if (std_acl)
+    *std_acl = standard_ACEs_only;
   return pos;
 }
 
diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h
index b4c7a02..0d744df 100644
--- a/winsup/cygwin/security.h
+++ b/winsup/cygwin/security.h
@@ -467,7 +467,7 @@ int searchace (struct acl *, int, int, uid_t id = 
ILLEGAL_UID);
 PSECURITY_DESCRIPTOR set_posix_access (mode_t, uid_t, gid_t, struct acl *, int,
                                       security_descriptor &, bool);
 int get_posix_access (PSECURITY_DESCRIPTOR, mode_t *, uid_t *, gid_t *,
-                     struct acl *, int);
+                     struct acl *, int, bool * = NULL);
 int getacl (HANDLE, path_conv &, int, struct acl *);
 int setacl (HANDLE, path_conv &, int, struct acl *, bool &);

Reply via email to