This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: ntsec patch 1: uid==gid, chmod, alloc_sd, is_grp_member


At 06:56 PM 11/15/2002 +0100, Corinna Vinschen wrote:
>On Fri, Nov 15, 2002 at 12:29:44PM -0500, Pierre A. Humblet wrote:
>> Alternatively I could add it, but add a check for group 
>> sid is SYSTEM, and then skip the step. That would be very easy
>> to do, and to remove later when ssh is ready.
>> I like this best actually.
>
>Good idea!  Me too.  But that must go into both functions,
>get_attribute_from_acl() and alloc_sd().

OK for get_attribute_from_acl (which is where the /var/empty
problem originates), but why for alloc_sd () ? 
The patch will do the right thing, whereas the current code 
can give rise to unexpected results. See below.

chmod 755 /var/empty  (which has owner == group)
With patch
**********
owner: system
group: system
acl:   system    7
       everybody 5   ==> Effective result 775
Currently
*********
owner: system
group: system
acl:   system    7
       system    5   ==> Note duplicate entry
       everybody 5   ==> Effective result 775

A more interesting case is
chmod 575 /var/empty   (just as an illustration)
With patch
**********
owner: system
group: system
acl:   system    7  ==> Single entry, 5 | 7
       everybody 5  ==> Effective result 775
Currently (if user system is NOT listed as a member of group system in
/etc/group)
*********
owner: system
group: system
acl:   system    5
       system    7
       everybody 5  ==> Effective result 775
Currently (change mkgroup -u to list user system in group system)
*********
owner: system
group: system
acl:	system   deny 2
	system    5
       system    7
       everybody 5  ==> Effective result 555 !!!!!

B.t.w. let's not change mkgroup, it's not necessary with the patch.

>Greetings from him (Michael Hirmke), btw.!
Hi Michael! I now put 2 and 2 together. Michael wrote to me
he was coming back from 5 a week vacation. For a while I was wondering
if October was to Germany as August is to France.

B.t.w. I don't have access to NT on weekends, so it's untested. I don't 
expect any trouble but...

Pierre

2002-11-18  Pierre Humblet <pierre.humblet@ieee.org>

	* security.cc (get_attribute_from_acl): Always test "anti",
	just in case an access_denied ACE follows an access_allowed.
	Handle the case owner_sid == group_sid, with a FIXME. 
	Remove unnecessary tests for non-NULL PSIDs.
	(alloc_sd): Use existing owner and group sids if {ug}id == -1.
	Handle case where owner_sid == group_sid.
	Do not call is_grp_member. Try to preserve canonical ACE order.
	Remove unnecessary tests for non-NULL PSIDs. Reorganize
	debug_printf's.
	(get_initgroups_sidlist): Put well_known_system_sid on left
	side of ==.
	(add_access_denied_ace): Only call GetAce if inherit != 0. 
	(add_access_allowed_ace): Ditto. Use appropriate sizeof.
	* syscalls.cc (chown_worker): Pass {ug}id equal to -1 to 
	alloc_sd, which removes the need to obtain old_{ug}id.
	(chmod): Remove call to get_file_attribute (), simply pass
	{ug}id equal to -1 to alloc_sd.

Attachment: sec.diff
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]