* sec_acl.cc (search_ace): Use id == -1, instead of < 0, as wildcard.

	(setacl): Start the search for a matching default at the next entry.
	Invalidate the type of merged entries instead of clearing it.
	Use well_known_creator for default owner and owning group and do
	not try to merge non-default and default entries in these cases.
	(getacl): Recognize well_known_creator for default owner and group.
	(acl_worker): Improve errno settings and streamline the nontsec case.
	* security.cc (write_sd): Remove the call to set_process_privilege.
	(alloc_sd): If the owner changes, call set_process_privilege and return
	immediately on failure. Change inheritance rules: on new directories add
	inherit only allow ACEs for creator_owner, creator_group and everyone.
	Preserve all inheritances through chmod and chown calls. Introduce
	isownergroup to implement the uid == gid case, to keep the inheritance
	code simple. Do not initialize owner_sid and group_sid and stop using
	the variable psd.

3 files changed, 156 insertions, 97 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 406ff5292..72452638c 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,21 @@
+2003-01-12  Pierre Humblet  <pierre.humblet@ieee.org>
+
+	* sec_acl.cc (search_ace): Use id == -1, instead of < 0, as wildcard.
+	(setacl): Start the search for a matching default at the next entry.
+	Invalidate the type of merged entries instead of clearing it.
+	Use well_known_creator for default owner and owning group and do
+	not try to merge non-default and default entries in these cases.
+	(getacl): Recognize well_known_creator for default owner and group.
+	(acl_worker): Improve errno settings and streamline the nontsec case.
+	* security.cc (write_sd): Remove the call to set_process_privilege.
+	(alloc_sd): If the owner changes, call set_process_privilege and return
+	immediately on failure. Change inheritance rules: on new directories add
+	inherit only allow ACEs for creator_owner, creator_group and everyone.
+	Preserve all inheritances through chmod and chown calls. Introduce
+	isownergroup to implement the uid == gid case, to keep the inheritance
+	code simple. Do not initialize owner_sid and group_sid and stop using
+	the variable psd.
+
 2003-01-10  Christopher Faylor  <cgf@redhat.com>
 
 	* net.cc: Use gethostname define from winsock2.h.
diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc
index 3766128fb..ce3ad41bf 100644
--- a/winsup/cygwin/sec_acl.cc
+++ b/winsup/cygwin/sec_acl.cc
@@ -1,6 +1,6 @@
-/* secacl.cc: Sun compatible ACL functions.
+/* sec_acl.cc: Sun compatible ACL functions.
 
-   Copyright 2000, 2001, 2002 Red Hat, Inc.
+   Copyright 2000, 2001, 2002, 2003 Red Hat, Inc.
 
    Written by Corinna Vinschen <corinna@vinschen.de>
 
@@ -41,7 +41,7 @@ searchace (__aclent16_t *aclp, int nentries, int type, int id = -1)
   int i;
 
   for (i = 0; i < nentries; ++i)
-    if ((aclp[i].a_type == type && (id < 0 || aclp[i].a_id == id))
+    if ((aclp[i].a_type == type && (id == -1 || aclp[i].a_id == id))
 	|| !aclp[i].a_type)
       return i;
   return -1;
@@ -137,26 +137,31 @@ setacl (const char *file, int nentries, __aclent16_t *aclbufp)
        * inheritance bits is created.
        */
       if (!(aclbufp[i].a_type & ACL_DEFAULT)
-	  && (pos = searchace (aclbufp, nentries,
+	  && aclbufp[i].a_type & (USER|GROUP|OTHER_OBJ)
+          && (pos = searchace (aclbufp + i + 1, nentries - i - 1,
 			       aclbufp[i].a_type | ACL_DEFAULT,
 			       (aclbufp[i].a_type & (USER|GROUP))
 			       ? aclbufp[i].a_id : -1)) >= 0
-	  && aclbufp[pos].a_type
 	  && aclbufp[i].a_perm == aclbufp[pos].a_perm)
 	{
 	  inheritance = SUB_CONTAINERS_AND_OBJECTS_INHERIT;
-	  /* This eliminates the corresponding default entry. */
-	  aclbufp[pos].a_type = 0;
+          /* This invalidates the corresponding default entry. */
+          aclbufp[pos].a_type = USER|GROUP|ACL_DEFAULT;
 	}
       switch (aclbufp[i].a_type)
 	{
 	case USER_OBJ:
-	case DEF_USER_OBJ:
 	  allow |= STANDARD_RIGHTS_ALL & ~DELETE;
 	  if (!add_access_allowed_ace (acl, ace_off++, allow,
 					owner, acl_len, inheritance))
 	    return -1;
 	  break;
+	case DEF_USER_OBJ:
+	  allow |= STANDARD_RIGHTS_ALL & ~DELETE;
+	  if (!add_access_allowed_ace (acl, ace_off++, allow,
+				       well_known_creator_owner_sid, acl_len, inheritance))
+	    return -1;
+	  break;
 	case USER:
 	case DEF_USER:
 	  if (!(pw = internal_getpwuid (aclbufp[i].a_id))
@@ -166,11 +171,15 @@ setacl (const char *file, int nentries, __aclent16_t *aclbufp)
 	    return -1;
 	  break;
 	case GROUP_OBJ:
-	case DEF_GROUP_OBJ:
 	  if (!add_access_allowed_ace (acl, ace_off++, allow,
 					group, acl_len, inheritance))
 	    return -1;
 	  break;
+	case DEF_GROUP_OBJ:
+	  if (!add_access_allowed_ace (acl, ace_off++, allow,
+				       well_known_creator_group_sid, acl_len, inheritance))
+	    return -1;
+	  break;
 	case GROUP:
 	case DEF_GROUP:
 	  if (!(gr = internal_getgrgid (aclbufp[i].a_id))
@@ -336,6 +345,16 @@ getacl (const char *file, DWORD attr, int nentries, __aclent16_t *aclbufp)
 	      type = USER_OBJ;
 	      id = uid;
 	    }
+	  else if (ace_sid == well_known_creator_group_sid)
+	    {
+	      type = GROUP_OBJ | ACL_DEFAULT;
+	      id = ILLEGAL_GID;
+	    }
+	  else if (ace_sid == well_known_creator_owner_sid)
+	    {
+	      type = USER_OBJ | ACL_DEFAULT;
+	      id = ILLEGAL_GID;
+	    }
 	  else
 	    {
 	      id = ace_sid.get_id (FALSE, &type);
@@ -352,7 +371,7 @@ getacl (const char *file, DWORD attr, int nentries, __aclent16_t *aclbufp)
 	    }
 	  if (!type)
 	    continue;
-	  if (!(ace->Header.AceFlags & INHERIT_ONLY))
+	  if (!(ace->Header.AceFlags & INHERIT_ONLY || type & ACL_DEFAULT))
 	    {
 	      if ((pos = searchace (lacl, MAX_ACL_ENTRIES, type, id)) >= 0)
 		getace (lacl[pos], type, id, ace->Mask, ace->Header.AceType);
@@ -360,6 +379,10 @@ getacl (const char *file, DWORD attr, int nentries, __aclent16_t *aclbufp)
 	  if ((ace->Header.AceFlags & SUB_CONTAINERS_AND_OBJECTS_INHERIT)
 	      && (attr & FILE_ATTRIBUTE_DIRECTORY))
 	    {
+	      if (type == USER_OBJ)
+		type = USER;
+	      else if (type == GROUP_OBJ)
+		type = GROUP;
 	      type |= ACL_DEFAULT;
 	      types_def |= type;
 	      if ((pos = searchace (lacl, MAX_ACL_ENTRIES, type, id)) >= 0)
@@ -475,45 +498,30 @@ acl_worker (const char *path, int cmd, int nentries, __aclent16_t *aclbufp,
 	  set_errno (ENOSYS);
 	  break;
 	case GETACL:
-	  if (nentries < 1)
-	    set_errno (EINVAL);
+	  if (!aclbufp)
+	    set_errno(EFAULT);
+	  else if (nentries < MIN_ACL_ENTRIES)
+	    set_errno (ENOSPC);
 	  else if ((nofollow && !lstat64 (path, &st))
 		   || (!nofollow && !stat64 (path, &st)))
 	    {
-	      __aclent16_t lacl[4];
-	      if (nentries > 0)
-		{
-		  lacl[0].a_type = USER_OBJ;
-		  lacl[0].a_id = st.st_uid;
-		  lacl[0].a_perm = (st.st_mode & S_IRWXU) >> 6;
-		}
-	      if (nentries > 1)
-		{
-		  lacl[1].a_type = GROUP_OBJ;
-		  lacl[1].a_id = st.st_gid;
-		  lacl[1].a_perm = (st.st_mode & S_IRWXG) >> 3;
-		}
-	      if (nentries > 2)
-		{
-		  lacl[2].a_type = OTHER_OBJ;
-		  lacl[2].a_id = ILLEGAL_GID;
-		  lacl[2].a_perm = st.st_mode & S_IRWXO;
-		}
-	      if (nentries > 3)
-		{
-		  lacl[3].a_type = CLASS_OBJ;
-		  lacl[3].a_id = ILLEGAL_GID;
-		  lacl[3].a_perm = S_IRWXU | S_IRWXG | S_IRWXO;
-		}
-	      if (nentries > 4)
-		nentries = 4;
-	      if (aclbufp)
-		memcpy (aclbufp, lacl, nentries * sizeof (__aclent16_t));
-	      ret = nentries;
+	      aclbufp[0].a_type = USER_OBJ;
+	      aclbufp[0].a_id = st.st_uid;
+	      aclbufp[0].a_perm = (st.st_mode & S_IRWXU) >> 6;
+	      aclbufp[1].a_type = GROUP_OBJ;
+	      aclbufp[1].a_id = st.st_gid;
+	      aclbufp[1].a_perm = (st.st_mode & S_IRWXG) >> 3;
+	      aclbufp[2].a_type = OTHER_OBJ;
+	      aclbufp[2].a_id = ILLEGAL_GID;
+	      aclbufp[2].a_perm = st.st_mode & S_IRWXO;
+	      aclbufp[3].a_type = CLASS_OBJ;
+	      aclbufp[3].a_id = ILLEGAL_GID;
+	      aclbufp[3].a_perm = S_IRWXU | S_IRWXG | S_IRWXO;
+	      ret = MIN_ACL_ENTRIES;
 	    }
 	  break;
 	case GETACLCNT:
-	  ret = 4;
+	  ret = MIN_ACL_ENTRIES;
 	  break;
 	}
       syscall_printf ("%d = acl (%s)", ret, path);
@@ -527,19 +535,21 @@ acl_worker (const char *path, int cmd, int nentries, __aclent16_t *aclbufp,
 			 nentries, aclbufp);
 	break;
       case GETACL:
-	if (nentries < 1)
-	  break;
-	return getacl (real_path.get_win32 (),
-		       real_path.file_attributes (),
-		       nentries, aclbufp);
+	if (!aclbufp)
+	  set_errno(EFAULT);
+	else
+	  return getacl (real_path.get_win32 (),
+			 real_path.file_attributes (),
+			 nentries, aclbufp);
+	break;
       case GETACLCNT:
 	return getacl (real_path.get_win32 (),
 		       real_path.file_attributes (),
 		       0, NULL);
       default:
+	set_errno (EINVAL);
 	break;
     }
-  set_errno (EINVAL);
   syscall_printf ("-1 = acl (%s)", path);
   return -1;
 }
diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc
index 777f4082c..6a15b6240 100644
--- a/winsup/cygwin/security.cc
+++ b/winsup/cygwin/security.cc
@@ -1,6 +1,6 @@
 /* security.cc: NT security functions
 
-   Copyright 1997, 1998, 1999, 2000, 2001, 2002 Red Hat, Inc.
+   Copyright 1997, 1998, 1999, 2000, 2001, 2002, 2003 Red Hat, Inc.
 
    Originaly written by Gunther Ebert, gunther.ebert@ixos-leipzig.de
    Completely rewritten by Corinna Vinschen <corinna@vinschen.de>
@@ -1142,14 +1142,6 @@ write_sd (const char *file, PSECURITY_DESCRIPTOR sd_buf, DWORD sd_size)
       return -1;
     }
 
-  /* No need to be thread save. */
-  static BOOL first_time = TRUE;
-  if (first_time)
-    {
-      set_process_privilege (SE_RESTORE_NAME);
-      first_time = FALSE;
-    }
-
   HANDLE fh;
   fh = CreateFile (file,
 		   WRITE_OWNER | WRITE_DAC,
@@ -1558,7 +1550,7 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
     debug_printf ("GetSecurityDescriptorGroup %E");
 
   /* Get SID of owner. */
-  cygsid owner_sid (NO_SID);
+  cygsid owner_sid;
   /* Check for current user first */
   if (uid == myself->uid)
     owner_sid = cygheap->user.sid ();
@@ -1571,8 +1563,13 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
     }
   owner_sid.debug_print ("alloc_sd: owner SID =");
 
+  /* Must have SE_RESTORE_NAME privilege to change owner */
+  if (cur_owner_sid && owner_sid != cur_owner_sid
+      && set_process_privilege (SE_RESTORE_NAME) < 0 )
+    return NULL;
+
   /* Get SID of new group. */
-  cygsid group_sid (NO_SID);
+  cygsid group_sid;
   /* Check for current user first */
   if (gid == myself->gid)
     group_sid = cygheap->user.groups.pgsid;
@@ -1587,7 +1584,6 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
 
   /* Initialize local security descriptor. */
   SECURITY_DESCRIPTOR sd;
-  PSECURITY_DESCRIPTOR psd = NULL;
   if (!InitializeSecurityDescriptor (&sd, SECURITY_DESCRIPTOR_REVISION))
     {
       __seterrno ();
@@ -1684,59 +1680,48 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
 
   /* Add owner and group permissions if SIDs are equal
      and construct deny attributes for group and owner. */
-  DWORD group_deny;
-  if (owner_sid == group_sid)
-    {
-      owner_allow |= group_allow;
-      group_allow = group_deny = 0L;
-    }
-  else
-    {
-      group_deny = ~group_allow & other_allow;
-      group_deny &= ~(STANDARD_RIGHTS_READ
-		      | FILE_READ_ATTRIBUTES | FILE_READ_EA);
-    }
+  BOOL isownergroup;
+  if ((isownergroup = (owner_sid == group_sid)))
+    owner_allow |= group_allow;
+
   DWORD owner_deny = ~owner_allow & (group_allow | other_allow);
   owner_deny &= ~(STANDARD_RIGHTS_READ
 		  | FILE_READ_ATTRIBUTES | FILE_READ_EA
 		  | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA);
 
-  /* Construct appropriate inherit attribute. */
-  DWORD inherit = (attribute & S_IFDIR) ? SUB_CONTAINERS_AND_OBJECTS_INHERIT
-					: NO_INHERITANCE;
+  DWORD group_deny = ~group_allow & other_allow;
+  group_deny &= ~(STANDARD_RIGHTS_READ
+		  | FILE_READ_ATTRIBUTES | FILE_READ_EA);
 
   /* Set deny ACE for owner. */
   if (owner_deny
       && !add_access_denied_ace (acl, ace_off++, owner_deny,
-				 owner_sid, acl_len, inherit))
+				 owner_sid, acl_len, NO_INHERITANCE))
     return NULL;
   /* Set deny ACE for group here to respect the canonical order,
      if this does not impact owner */
-  if (group_deny && !(owner_allow & group_deny))
-    {
-      if (!add_access_denied_ace (acl, ace_off++, group_deny,
-				 group_sid, acl_len, inherit))
-	return NULL;
-      group_deny = 0;
-    }
+  if (group_deny && !(group_deny & owner_allow) && !isownergroup
+      && !add_access_denied_ace (acl, ace_off++, group_deny,
+				 group_sid, acl_len, NO_INHERITANCE))
+    return NULL;
   /* Set allow ACE for owner. */
   if (!add_access_allowed_ace (acl, ace_off++, owner_allow,
-			       owner_sid, acl_len, inherit))
+			       owner_sid, acl_len, NO_INHERITANCE))
     return NULL;
   /* Set deny ACE for group, if still needed. */
-  if (group_deny
+  if (group_deny & owner_allow && !isownergroup
       && !add_access_denied_ace (acl, ace_off++, group_deny,
-				 group_sid, acl_len, inherit))
+				 group_sid, acl_len, NO_INHERITANCE))
     return NULL;
   /* Set allow ACE for group. */
-  if (group_allow
+  if (!isownergroup
       && !add_access_allowed_ace (acl, ace_off++, group_allow,
-				  group_sid, acl_len, inherit))
+				  group_sid, acl_len, NO_INHERITANCE))
     return NULL;
 
   /* Set allow ACE for everyone. */
   if (!add_access_allowed_ace (acl, ace_off++, other_allow,
-			       well_known_world_sid, acl_len, inherit))
+			       well_known_world_sid, acl_len, NO_INHERITANCE))
     return NULL;
   /* Set null ACE for special bits. */
   if (null_allow
@@ -1746,7 +1731,7 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
 
   /* Fill ACL with unrelated ACEs from current security descriptor. */
   PACL oacl;
-  BOOL acl_exists;
+  BOOL acl_exists = FALSE;
   ACCESS_ALLOWED_ACE *ace;
   if (GetSecurityDescriptorDacl (sd_ret, &acl_exists, &oacl, &dummy)
       && acl_exists && oacl)
@@ -1755,19 +1740,26 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
 	{
 	  cygsid ace_sid ((PSID) &ace->SidStart);
 	  /* Check for related ACEs. */
+	  if (ace_sid == well_known_null_sid)
+	    continue;
 	  if ((ace_sid == cur_owner_sid)
 	      || (ace_sid == owner_sid)
 	      || (ace_sid == cur_group_sid)
 	      || (ace_sid == group_sid)
-	      || (ace_sid == well_known_world_sid)
-	      || (ace_sid == well_known_null_sid))
-	    continue;
+	      || (ace_sid == well_known_world_sid))
+	    {
+	      if (ace->Header.AceFlags & SUB_CONTAINERS_AND_OBJECTS_INHERIT)
+		ace->Header.AceFlags |= INHERIT_ONLY;
+	      else
+		continue;
+	    }
 	  /*
 	   * Add unrelated ACCESS_DENIED_ACE to the beginning but
 	   * behind the owner_deny, ACCESS_ALLOWED_ACE to the end.
+	   * FIXME: this would break the order of the inherit_only ACEs
 	   */
 	  if (!AddAce (acl, ACL_REVISION,
-		       ace->Header.AceType == ACCESS_DENIED_ACE_TYPE ?
+		       ace->Header.AceType == ACCESS_DENIED_ACE_TYPE?
 		       (owner_deny ? 1 : 0) : MAXDWORD,
 		       (LPVOID) ace, ace->Header.AceSize))
 	    {
@@ -1777,6 +1769,46 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
 	  acl_len += ace->Header.AceSize;
 	}
 
+  /* Construct appropriate inherit attribute for new directories */
+  if (attribute & S_IFDIR && !acl_exists )
+    {
+      const DWORD inherit = SUB_CONTAINERS_AND_OBJECTS_INHERIT | INHERIT_ONLY;
+
+#if 0 /* FIXME: Not done currently as this breaks the canonical order */
+      /* Set deny ACE for owner. */
+      if (owner_deny
+	  && !add_access_denied_ace (acl, ace_off++, owner_deny,
+				     well_known_creator_owner_sid, acl_len, inherit))
+	return NULL;
+      /* Set deny ACE for group here to respect the canonical order,
+	 if this does not impact owner */
+      if (group_deny && !(group_deny & owner_allow)
+	  && !add_access_denied_ace (acl, ace_off++, group_deny,
+				     well_known_creator_group_sid, acl_len, inherit))
+	return NULL;
+#endif
+      /* Set allow ACE for owner. */
+      if (!add_access_allowed_ace (acl, ace_off++, owner_allow,
+				   well_known_creator_owner_sid, acl_len, inherit))
+	return NULL;
+#if 0 /* FIXME: Not done currently as this breaks the canonical order and
+	 won't be preserved on chown and chmod */
+      /* Set deny ACE for group, conflicting with owner_allow. */
+      if (group_deny & owner_allow
+	  && !add_access_denied_ace (acl, ace_off++, group_deny,
+				     well_known_creator_group_sid, acl_len, inherit))
+	return NULL;
+#endif
+      /* Set allow ACE for group. */
+      if (!add_access_allowed_ace (acl, ace_off++, group_allow,
+				   well_known_creator_group_sid, acl_len, inherit))
+	return NULL;
+      /* Set allow ACE for everyone. */
+      if (!add_access_allowed_ace (acl, ace_off++, other_allow,
+				   well_known_world_sid, acl_len, inherit))
+	return NULL;
+    }
+
   /* Set AclSize to computed value. */
   acl->AclSize = acl_len;
   debug_printf ("ACL-Size: %d", acl_len);
@@ -1801,10 +1833,9 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
       __seterrno ();
       return NULL;
     }
-  psd = sd_ret;
   debug_printf ("Created SD-Size: %d", *sd_size_ret);
 
-  return psd;
+  return sd_ret;
 }
 
 void