Mailing-List: contact cygwin-developers-help AT sourceware DOT cygnus DOT com; run by ezmlm
List-Subscribe: <mailto:cygwin-developers-subscribe AT sourceware DOT cygnus DOT com>
List-Archive: <http://sourceware.cygnus.com/ml/cygwin-developers/>
List-Post: <mailto:cygwin-developers AT sourceware DOT cygnus DOT com>
List-Help: <mailto:cygwin-developers-help AT sourceware DOT cygnus DOT com>, <http://sourceware.cygnus.com/ml/#faqs>
Sender: cygwin-developers-owner AT sourceware DOT cygnus DOT com
Delivered-To: mailing list cygwin-developers AT sourceware DOT cygnus DOT com
Message-ID: <3878E350.21700097@vinschen.de>
Date: Sun, 09 Jan 2000 20:36:48 +0100
From: Corinna Vinschen <corinna AT vinschen DOT de>
X-Mailer: Mozilla 4.7 [en] (WinNT; I)
X-Accept-Language: de,en
MIME-Version: 1.0
To: Chris Faylor <cgf AT cygnus DOT com>
CC: cygdev <cygwin-developers AT sourceware DOT cygnus DOT com>
Subject: ntsec-patch17
Content-Type: multipart/mixed;
 boundary="------------16C137A322C94FF358AE3062"

This is a multi-part message in MIME format.
--------------16C137A322C94FF358AE3062
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hello!

I have found a few small problems in the last ntsec patch:

- If the ACL of the file has contained unrelated entries (not
  attached to user/group/everyone), the order of the newly created
  ACL in case of chmod/chown may not be ok. Especially the `everyone'
  entry should be the last one.

- A call to `acl(SETACL,...)' has created possibly more ACEs than are
  needed. Maybe, this code still demands optimization.

- `aclcheck()' has forced the existence of CLASS_OBJ and DEF_CLASS_OBJ
  entries according to the rules described in the solaris man pages.
  This check has no sense so far CLASS_OBJ and DEF_CLASS_OBJ are not
  fully implemented (has somebody an idea how to do this?).

Have fun,
Corinna


ChangeLog:
==========

Sun Jan  9 20:18:00 2000  Corinna Vinschen  <corinna AT vinschen DOT de>

	* security.cc (alloc_sd): Rearranged order of ACE creation.
	(setacl): Optimized creation of ACEs related to inheritance.
	Code cleanup.
	(aclcheck): Disabled check for existance of (DEF_)CLASS_OBJ.
--------------16C137A322C94FF358AE3062
Content-Type: text/plain; charset=us-ascii;
 name="ntsec-patch17"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="ntsec-patch17"

Index: cygwin/security.cc
===================================================================
RCS file: /src/cvsroot/winsup-000108/cygwin/security.cc,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 security.cc
--- cygwin/security.cc	2000/01/09 10:50:54	1.1.1.1
+++ cygwin/security.cc	2000/01/09 19:25:33
@@ -937,10 +937,6 @@ alloc_sd (uid_t uid, gid_t gid, const ch
   if (! add_access_allowed_ace (acl, ace_off++, group_allow,
                                 group_sid, acl_len, INHERIT_ALL))
     return NULL;
-  // Set allow ACE for everyone
-  if (! add_access_allowed_ace (acl, ace_off++, other_allow,
-                                get_world_sid (), acl_len, INHERIT_ALL))
-    return NULL;
 
   // Get owner and group from current security descriptor
   PSID cur_owner_sid = NULL;
@@ -967,10 +963,12 @@ alloc_sd (uid_t uid, gid_t gid, const ch
               || (group_sid && EqualSid (ace_sid, group_sid))
               || (EqualSid (ace_sid, get_world_sid ())))
             continue;
-          // Add unrelated ACE
+          // Add unrelated ACCESS_DENIED_ACE to the beginning but
+          // behind the owner_deny, ACCESS_ALLOWED_ACE to the end
+          // but in front of the `everyone' ACE.
           if (! AddAce(acl, ACL_REVISION,
                        ace->Header.AceType == ACCESS_DENIED_ACE_TYPE ?
-                       0 : MAXDWORD,
+                       (owner_deny ? 1 : 0) : MAXDWORD,
                        (LPVOID) ace, ace->Header.AceSize))
             {
               __seterrno ();
@@ -980,6 +978,11 @@ alloc_sd (uid_t uid, gid_t gid, const ch
           ++ace_off;
         }
 
+  // Set allow ACE for everyone
+  if (! add_access_allowed_ace (acl, ace_off++, other_allow,
+                                get_world_sid (), acl_len, INHERIT_ALL))
+    return NULL;
+
   // Set AclSize to computed value
   acl->AclSize = acl_len;
   debug_printf ("ACL-Size: %d", acl_len);
@@ -1073,8 +1076,20 @@ set_file_attribute (int use_ntsec, const
                              attribute, myself->logsrv);
 }
 
+static int
+searchace (aclent_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))
+        || !aclp[i].a_type)
+      return i;
+  return -1;
+}
+
 static int
-setacl (const char *file, int nentries, const aclent_t *aclbufp)
+setacl (const char *file, int nentries, aclent_t *aclbufp)
 {
   DWORD sd_size = 4096;
   char sd_buf[4096];
@@ -1147,6 +1162,7 @@ setacl (const char *file, int nentries, 
   PSID sid = (PSID) sidbuf;
   struct passwd *pw;
   struct group *gr;
+  int pos;
 
   if (! InitializeAcl (acl, 3072, ACL_REVISION))
     {
@@ -1164,66 +1180,59 @@ setacl (const char *file, int nentries, 
                  | DELETE | FILE_DELETE_CHILD;
       if (aclbufp[i].a_perm & S_IXOTH)
         allow |= FILE_GENERIC_EXECUTE;
+      // Set inherit property
+      DWORD inheritance = (aclbufp[i].a_type & ACL_DEFAULT)
+                          ? INHERIT_ONLY : DONT_INHERIT;
+      // If a specific acl contains a corresponding default entry with
+      // identical permissions, only one Windows ACE with proper
+      // inheritance bits is created.
+      if (!(aclbufp[i].a_type & ACL_DEFAULT)
+          && (pos = searchace (aclbufp, nentries,
+                               aclbufp[i].a_type | ACL_DEFAULT,
+                               (aclbufp[i].a_type & (USER|GROUP))
+                               ? aclbufp[i].a_id : -1)) >= 0
+          && pos < nentries
+          && aclbufp[i].a_perm == aclbufp[pos].a_perm)
+        {
+          inheritance = INHERIT_ALL;
+          // This eliminates the corresponding default entry.
+          aclbufp[pos].a_type = 0;
+        }
       switch (aclbufp[i].a_type)
         {
         case USER_OBJ:
-          allow |= STANDARD_RIGHTS_ALL & ~DELETE;
-          if (! add_access_allowed_ace (acl, ace_off++, allow,
-                                        owner_sid, acl_len, DONT_INHERIT))
-            return -1;
-          break;
-        case USER:
-          if (!(pw = getpwuid (aclbufp[i].a_id))
-              || ! get_pw_sid (sid, pw)
-              || ! add_access_allowed_ace (acl, ace_off++, allow,
-                                           sid, acl_len, DONT_INHERIT))
-            return -1;
-          break;
-        case GROUP_OBJ:
-          if (! add_access_allowed_ace (acl, ace_off++, allow,
-                                        group_sid, acl_len, DONT_INHERIT))
-            return -1;
-          break;
-        case GROUP:
-          if (!(gr = getgrgid (aclbufp[i].a_id))
-              || ! get_gr_sid (sid, gr)
-              || ! add_access_allowed_ace (acl, ace_off++, allow,
-                                           sid, acl_len, DONT_INHERIT))
-            return -1;
-          break;
-        case OTHER_OBJ:
-          if (! add_access_allowed_ace (acl, ace_off++, allow,
-                                        get_world_sid(), acl_len, DONT_INHERIT))
-            return -1;
-          break;
         case DEF_USER_OBJ:
           allow |= STANDARD_RIGHTS_ALL & ~DELETE;
           if (! add_access_allowed_ace (acl, ace_off++, allow,
-                                        owner_sid, acl_len, INHERIT_ONLY))
+                                        owner_sid, acl_len, inheritance))
             return -1;
           break;
+        case USER:
         case DEF_USER:
           if (!(pw = getpwuid (aclbufp[i].a_id))
               || ! get_pw_sid (sid, pw)
               || ! add_access_allowed_ace (acl, ace_off++, allow,
-                                           sid, acl_len, INHERIT_ONLY))
+                                           sid, acl_len, inheritance))
             return -1;
           break;
+        case GROUP_OBJ:
         case DEF_GROUP_OBJ:
           if (! add_access_allowed_ace (acl, ace_off++, allow,
-                                        group_sid, acl_len, INHERIT_ONLY))
+                                        group_sid, acl_len, inheritance))
             return -1;
           break;
+        case GROUP:
         case DEF_GROUP:
           if (!(gr = getgrgid (aclbufp[i].a_id))
               || ! get_gr_sid (sid, gr)
               || ! add_access_allowed_ace (acl, ace_off++, allow,
-                                           sid, acl_len, INHERIT_ONLY))
+                                           sid, acl_len, inheritance))
             return -1;
           break;
+        case OTHER_OBJ:
         case DEF_OTHER_OBJ:
           if (! add_access_allowed_ace (acl, ace_off++, allow,
-                                        get_world_sid(), acl_len, INHERIT_ONLY))
+                                        get_world_sid(), acl_len, inheritance))
             return -1;
           break;
         }
@@ -1280,18 +1289,6 @@ getace (aclent_t &acl, int type, int id,
 }
 
 static int
-searchace (aclent_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))
-        || !aclp[i].a_type)
-      return i;
-  return -1;
-}
-
-static int
 getacl (const char *file, DWORD attr, int nentries, aclent_t *aclbufp)
 {
   DWORD sd_size = 4096;
@@ -1690,8 +1687,12 @@ aclcheck (aclent_t *aclbufp, int nentrie
   if (!has_user_obj
       || !has_group_obj
       || !has_other_obj
+#if 0
+      // These checks are not ok yet since CLASS_OBJ isn't fully implemented.
       || (has_ug_objs && !has_class_obj)
-      || (has_def_ug_objs && !has_def_class_obj))
+      || (has_def_ug_objs && !has_def_class_obj)
+#endif
+     )
     {
       if (which)
         *which = -1;



--------------16C137A322C94FF358AE3062--