Fix permission evaluation for !new_style ACLs

	* security.h (authz_get_user_attribute): Declare bool.
	* sec_helper.cc (authz_ctx::get_user_attribute): Make bool method.
	Set S_IxOTH bits in returned attributes rather than S_IxUSR bits.
	(authz_get_user_attribute): Make bool function.
	* sec_acl.cc (get_posix_access): Introduce cygsid array to keep
	track of all SIDs in the ACL.  Move AuthZ calls into !new_style
	permission post processing.  When not using AuthZ, use
	CheckTokenMembership to collect group permissions.

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

4 files changed, 72 insertions, 54 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 396ce2e..690d5bc 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,5 +1,16 @@
 2015-11-18  Corinna Vinschen  <corinna@vinschen.de>
 
+	* security.h (authz_get_user_attribute): Declare bool.
+	* sec_helper.cc (authz_ctx::get_user_attribute): Make bool method.
+	Set S_IxOTH bits in returned attributes rather than S_IxUSR bits.
+	(authz_get_user_attribute): Make bool function.
+	* sec_acl.cc (get_posix_access): Introduce cygsid array to keep
+	track of all SIDs in the ACL.  Move AuthZ calls into !new_style
+	permission post processing.  When not using AuthZ, use 
+	CheckTokenMembership to collect group permissions.
+
+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
diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc
index a84cd5e..ec6aa7f 100644
--- a/winsup/cygwin/sec_acl.cc
+++ b/winsup/cygwin/sec_acl.cc
@@ -580,7 +580,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
   gid_t gid;
   mode_t attr = 0;
   aclent_t *lacl = NULL;
-  cygpsid ace_sid;
+  cygpsid ace_sid, *aclsid;
   int pos, type, id, idx;
 
   bool owner_eq_group;
@@ -669,6 +669,11 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
   lacl[1].a_id = gid;
   lacl[2].a_type = OTHER_OBJ;
   lacl[2].a_id = ILLEGAL_GID;
+  /* Create array to collect SIDs of all entries in lacl. */
+  aclsid = (cygpsid *) tp.w_get ();
+  aclsid[0] = owner_sid;
+  aclsid[1] = group_sid;
+  aclsid[2] = well_known_world_sid;
 
   /* No ACEs?  Everybody has full access. */
   if (!acl_exists || !acl || acl->AceCount == 0)
@@ -678,15 +683,6 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
       goto out;
     }
 
-  /* If we use the Windows user DB, use Authz to make sure the owner
-     permissions are correctly reflecting the Windows permissions. */
-  if (cygheap->pg.nss_pwd_db ())
-    {
-      mode_t attr = 0;
-      authz_get_user_attribute (&attr, psd, owner_sid);
-      lacl[0].a_perm = attr >> 6;
-    }
-
   /* Files and dirs are created with a NULL descriptor, so inheritence
      rules kick in.  If no inheritable entries exist in the parent object,
      Windows will create entries according to the user token's default DACL.
@@ -730,6 +726,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
 			  lacl[pos].a_type = CLASS_OBJ;
 			  lacl[pos].a_id = ILLEGAL_GID;
 			  lacl[pos].a_perm = CYG_ACE_MASK_TO_POSIX (ace->Mask);
+			  aclsid[pos] = well_known_null_sid;
 			}
 		      has_class_perm = true;
 		      class_perm = lacl[pos].a_perm;
@@ -742,6 +739,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
 			  lacl[pos].a_type = DEF_CLASS_OBJ;
 			  lacl[pos].a_id = ILLEGAL_GID;
 			  lacl[pos].a_perm = CYG_ACE_MASK_TO_POSIX (ace->Mask);
+			  aclsid[pos] = well_known_null_sid;
 			}
 		      has_def_class_perm = true;
 		      def_class_perm = lacl[pos].a_perm;
@@ -832,21 +830,9 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
 	    }
 	  if ((pos = searchace (lacl, MAX_ACL_ENTRIES, type, id)) >= 0)
 	    {
-	      /* If we use the Windows user DB, use Authz to check for user
-		 permissions. */
-	      if (cygheap->pg.nss_pwd_db () && (type & (USER_OBJ | USER)))
-		{
-		  /* We already handle the USER_OBJ entry above. */
-		  if (type == USER)
-		    {
-		      mode_t attr = 0;
-		      authz_get_user_attribute (&attr, psd, ace_sid);
-		      lacl[pos].a_perm = attr >> 6;
-		    }
-		}
-	      else
-		getace (lacl[pos], type, id, ace->Mask, ace->Header.AceType,
-			new_style && type & (USER | GROUP_OBJ | GROUP));
+	      getace (lacl[pos], type, id, ace->Mask, ace->Header.AceType,
+		      new_style && type & (USER | GROUP_OBJ | GROUP));
+	      aclsid[pos] = ace_sid;
 	      if (!new_style)
 		{
 		  /* Fix up CLASS_OBJ value. */
@@ -909,6 +895,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
 	    {
 	      getace (lacl[pos], type, id, ace->Mask, ace->Header.AceType,
 		      new_style && type & (USER | GROUP_OBJ | GROUP));
+	      aclsid[pos] = ace_sid;
 	      if (!new_style)
 		{
 		  /* Fix up DEF_CLASS_OBJ value. */
@@ -940,6 +927,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
       lacl[pos].a_id = ILLEGAL_GID;
       class_perm |= lacl[1].a_perm;
       lacl[pos].a_perm = class_perm;
+      aclsid[pos] = well_known_null_sid;
     }
   /* For ptys, fake a mask if the admins group is neither owner nor group.
      In that case we have an extra ACE for the admins group, and we need a
@@ -954,6 +942,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
       lacl[pos].a_type = CLASS_OBJ;
       lacl[pos].a_id = ILLEGAL_GID;
       lacl[pos].a_perm = lacl[1].a_perm; /* == group perms */
+      aclsid[pos] = well_known_null_sid;
     }
   /* If this is a just created file, and this is an ACL with only standard
      entries, or if standard POSIX permissions are missing (probably no
@@ -979,6 +968,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
 	  lacl[pos].a_type = DEF_USER_OBJ;
 	  lacl[pos].a_id = uid;
 	  lacl[pos].a_perm = lacl[0].a_perm;
+	  aclsid[pos] = well_known_creator_owner_sid;
 	  pos++;
 	}
       if (!(types_def & GROUP_OBJ) && pos < MAX_ACL_ENTRIES)
@@ -988,6 +978,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
 	  lacl[pos].a_perm = lacl[1].a_perm;
 	  /* Note the position of the DEF_GROUP_OBJ entry. */
 	  def_pgrp_pos = pos;
+	  aclsid[pos] = well_known_creator_group_sid;
 	  pos++;
 	}
       if (!(types_def & OTHER_OBJ) && pos < MAX_ACL_ENTRIES)
@@ -995,6 +986,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
 	  lacl[pos].a_type = DEF_OTHER_OBJ;
 	  lacl[pos].a_id = ILLEGAL_GID;
 	  lacl[pos].a_perm = lacl[2].a_perm;
+	  aclsid[pos] = well_known_world_sid;
 	  pos++;
 	}
     }
@@ -1011,6 +1003,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
       lacl[pos].a_perm = def_class_perm;
       if (def_pgrp_pos >= 0)
 	lacl[pos].a_perm |= lacl[def_pgrp_pos].a_perm;
+      aclsid[pos] = well_known_null_sid;
     }
 
   /* Make sure `pos' contains the number of used entries in lacl. */
@@ -1021,26 +1014,36 @@ get_posix_access (PSECURITY_DESCRIPTOR psd,
   if (!new_style)
     for (idx = 0; idx < pos; ++idx)
       {
-	/* Current user?  If the user entry has a deny ACE, don't check. */
-	if (lacl[idx].a_id == myself->uid
-	    && lacl[idx].a_type & (USER_OBJ | USER)
-	    && !(lacl[idx].a_type & ACL_DEFAULT)
-	    && !(lacl[idx].a_perm & DENY_RWX))
+	if (lacl[idx].a_type & (USER_OBJ | USER)
+	    && !(lacl[idx].a_type & ACL_DEFAULT))
 	  {
-	    int gpos;
-	    gid_t grps[NGROUPS_MAX];
-	    cyg_ldap cldap;
-
-	    /* Sum up all permissions of groups the user is member of, plus
-	       everyone perms, and merge them to user perms.  */
-	    mode_t grp_perm = lacl[2].a_perm & S_IRWXO;
-	    int gnum = internal_getgroups (NGROUPS_MAX, grps, &cldap);
-	    for (int g = 0; g < gnum && grp_perm != S_IRWXO; ++g)
-	      if ((gpos = 1, grps[g] == lacl[gpos].a_id)
-		  || (gpos = searchace (lacl, MAX_ACL_ENTRIES, GROUP, grps[g]))
-		     >= 0)
-		grp_perm |= lacl[gpos].a_perm & S_IRWXO;
-	    lacl[idx].a_perm |= grp_perm;
+	    mode_t perm;
+
+	    /* If we use the Windows user DB, utilize Authz to make sure all
+	       user permissions are correctly reflecting the Windows
+	       permissions. */
+	    if (cygheap->pg.nss_pwd_db ()
+		&& authz_get_user_attribute (&perm, psd, owner_sid))
+	      lacl[0].a_perm = perm;
+	    /* Otherwise we only check the current user.  If the user entry
+	       has a deny ACE, don't check. */
+	    else if (lacl[idx].a_id == myself->uid
+		     && !(lacl[idx].a_perm & DENY_RWX))
+	      {
+		/* Sum up all permissions of groups the user is member of, plus
+		   everyone perms, and merge them to user perms.  */
+		BOOL ret;
+
+		perm = lacl[2].a_perm & S_IRWXO;
+		for (int gidx = 1; gidx < pos; ++gidx)
+		  if (lacl[gidx].a_type & (GROUP_OBJ | GROUP)
+		      && CheckTokenMembership (cygheap->user.issetuid ()
+					       ? cygheap->user.imp_token () : NULL,
+					       aclsid[gidx], &ret)
+		      && ret)
+		    perm |= lacl[gidx].a_perm & S_IRWXO;
+		lacl[idx].a_perm |= perm;
+	      }
 	  }
 	/* For all groups, if everyone has more permissions, add everyone
 	   perms to group perms.  Skip groups with deny ACE. */
diff --git a/winsup/cygwin/sec_helper.cc b/winsup/cygwin/sec_helper.cc
index 40b8017..af3307e 100644
--- a/winsup/cygwin/sec_helper.cc
+++ b/winsup/cygwin/sec_helper.cc
@@ -717,7 +717,7 @@ class authz_ctx
 
   friend class authz_ctx_cache;
 public:
-  void get_user_attribute (mode_t *, PSECURITY_DESCRIPTOR, PSID);
+  bool get_user_attribute (mode_t *, PSECURITY_DESCRIPTOR, PSID);
 };
 
 /* Authz handles are not inheritable. */
@@ -779,7 +779,7 @@ authz_ctx_cache::context (PSID user_sid)
 /* Ask Authz for the effective user permissions of the user with SID user_sid
    on the object with security descriptor psd.  We're caching the handles for
    the Authz resource manager and the user contexts. */
-void
+bool
 authz_ctx::get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd,
 			       PSID user_sid)
 {
@@ -802,7 +802,7 @@ authz_ctx::get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd,
 	ctx_hdl = user_ctx_hdl;
     }
   if (!ctx_hdl && !(ctx_hdl = ctx_cache.context (user_sid)))
-    return;
+    return false;
   /* All set, check access. */
   ACCESS_MASK access = 0;
   DWORD error = 0;
@@ -822,16 +822,20 @@ authz_ctx::get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd,
   if (AuthzAccessCheck (0, ctx_hdl, &req, NULL, psd, NULL, 0, &repl, NULL))
     {
       if (access & FILE_READ_BITS)
-	*attribute |= S_IRUSR;
+	*attribute |= S_IROTH;
       if (access & FILE_WRITE_BITS)
-	*attribute |= S_IWUSR;
+	*attribute |= S_IWOTH;
       if (access & FILE_EXEC_BITS)
-	*attribute |= S_IXUSR;
+	*attribute |= S_IXOTH;
+      return true;
     }
+  return false;
 }
 
-void authz_get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd,
-			       PSID user_sid)
+bool
+authz_get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd,
+			  PSID user_sid)
 {
-  authz.get_user_attribute (attribute, psd, user_sid);
+  *attribute = 0;
+  return authz.get_user_attribute (attribute, psd, user_sid);
 }
diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h
index 9c70121..b4c7a02 100644
--- a/winsup/cygwin/security.h
+++ b/winsup/cygwin/security.h
@@ -456,7 +456,7 @@ void set_security_attribute (path_conv &pc, int attribute,
 			     PSECURITY_ATTRIBUTES psa,
 			     security_descriptor &sd_buf);
 
-void authz_get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd,
+bool authz_get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd,
 			       PSID user_sid);
 
 /* sec_acl.cc */