This is the mail archive of the
cygwin-developers@sourceware.cygnus.com
mailing list for the Cygwin project.
ntsec-patch17
- To: Chris Faylor <cgf at cygnus dot com>
- Subject: ntsec-patch17
- From: Corinna Vinschen <corinna at vinschen dot de>
- Date: Sun, 09 Jan 2000 20:36:48 +0100
- CC: cygdev <cygwin-developers at sourceware dot cygnus dot com>
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@vinschen.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.
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;