[PATCH] Ensure that the default ACL contains the standard entries

Christian Franke Christian.Franke@t-online.de
Wed Dec 15 14:12:00 GMT 2010


Hi Corinna,

Corinna Vinschen wrote:
> Hi Christian,
>
> On Dec 10 23:05, Christian Franke wrote:
>    
>> The ACL from Cygwin always contains the three (USER|GROUP|OTHER)_OBJ
>> entries. It might be existing practice elsewhere to return these
>> entries also in the default ACL. The attached patch adds these
>> entries with empty permissions if necessary.
>>
>> The patch would fix this rsync issue:
>> http://cygwin.com/ml/cygwin/2010-11/msg00429.html
>>
>> The logic for DEF_CLASS_OBJ is unchanged.
>>      
>
> This looks good, except that the faked default entries for group and
> other are set to 0.  That's rather unexpected. ...
>
> This is rather easy to fix (and you added comments in that place), ...

New patch attached.


>
> I'm not entirely sure yet, but maybe the acl function should not
> fake these default entries.  From my POV it seems a better approach
> if acl(SETACL) actually creates these entries if *any* default entry
> is in the incoming acl.  And, it should create these entries with
> useful permission values.  This seems to reflect the Linux behaviour
> much closer.  What do you think?

AFIAK a minimal ACL must contain the three entries u/g/o which are 
equivalent to the mode permission bits. The default ACL has likely the 
same requirement.

If this is the case, then I would suggest to do both:

1. Fake these entries in acl(GETACL) if required. This would ensure that 
the default ACL is complete even if the Windows ACL was not created by 
Cygwin.

2. Create these entries in acl(SETACL) if required. This would ensure 
that the following reasonable requirement holds if the Windows ACL was 
created by Cygwin before:

- "getfacl foo | setfacl -f - foo" should not change the (Windows) ACL 
of "foo".


>    Would you implement this?
>    

Yes, but may take some time.


> Btw., while testing your patch I found a bug in setfacl which disallowed
> to delete user/group-specific default entries.  I opted for rewriting the
> function which examines an incoming acl entry (getaclentry).  Would you
> mind to test this bit, too?  The new code accepts a trailing colon, but
> I think that's ok.  The SGI setfacl tool used on Linux is even more
> relaxed syntax-wise and also accepts trailing colons.
>    

I've done a few test, looks good.


An unrelated issue found during testing:

mkdir() may duplicate Windows ACL entries. Testcase (German XP SP3):

$ cd /tmp

$ mkdir 0

$ cd 0

$ setfacl -s 'u::rwx,g::r-x,o:---' .

$ xcacls .
C:\cygwin\tmp\0 SomeDomain\SomeOne:F
                 SomeDomain\Kein:R
                 Jeder:(special access:)
                       READ_CONTROL
                       FILE_READ_ATTRIBUTES

$ for d in 1 2 3 4; do mkdir -m 0750 $d; cd $d; xcacls .; done
[...]

C:\cygwin\tmp\0\1\2\3\4 SomeDomain\SomeOne:F
                         SomeDomain\Kein:R
                         Jeder:(special access:)
                               READ_CONTROL
                               SYNCHRONIZE
                               FILE_READ_ATTRIBUTES

                         Jeder:(OI)(CI)(IO)(special access:)
                                           READ_CONTROL
                                           SYNCHRONIZE
                                           FILE_READ_ATTRIBUTES

                         Jeder:(OI)(CI)(IO)(special access:)
                                           READ_CONTROL
                                           SYNCHRONIZE
                                           FILE_READ_ATTRIBUTES

                         Jeder:(OI)(CI)(IO)(special access:)
                                           READ_CONTROL
                                           SYNCHRONIZE
                                           FILE_READ_ATTRIBUTES

                         ERSTELLER-BESITZER:(OI)(CI)(IO)F
                         ERSTELLERGRUPPE:(OI)(CI)(IO)R
                         ERSTELLER-BESITZER:(OI)(CI)(IO)F
                         ERSTELLERGRUPPE:(OI)(CI)(IO)R
                         Jeder:(OI)(CI)(IO)(special access:)
                                           READ_CONTROL
                                           SYNCHRONIZE
                                           FILE_READ_ATTRIBUTES


Problem in security.cc:alloc_sd() ?

Christian

-------------- next part --------------
A non-text attachment was scrubbed...
Name: sec_acl-default-2.patch
Type: text/x-patch
Size: 2179 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20101215/91b3a945/attachment.bin>


More information about the Cygwin-patches mailing list