This is the mail archive of the
cygwin-patches
mailing list for the Cygwin project.
Re: [PATCH] Ensure that the default ACL contains the standard entries
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
diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc
index 24f2468..72d310e 100644
--- a/winsup/cygwin/sec_acl.cc
+++ b/winsup/cygwin/sec_acl.cc
@@ -357,11 +357,13 @@ getacl (HANDLE handle, path_conv &pc, int nentries, __aclent32_t *aclbufp)
else if (ace_sid == well_known_creator_group_sid)
{
type = GROUP_OBJ | ACL_DEFAULT;
+ types_def |= type;
id = ILLEGAL_GID;
}
else if (ace_sid == well_known_creator_owner_sid)
{
type = USER_OBJ | ACL_DEFAULT;
+ types_def |= type;
id = ILLEGAL_GID;
}
else
@@ -388,13 +390,38 @@ getacl (HANDLE handle, path_conv &pc, int nentries, __aclent32_t *aclbufp)
getace (lacl[pos], type, id, ace->Mask, ace->Header.AceType);
}
}
- /* Include DEF_CLASS_OBJ if any default ace exists */
- if ((types_def & (USER|GROUP))
- && ((pos = searchace (lacl, MAX_ACL_ENTRIES, DEF_CLASS_OBJ)) >= 0))
+ if (types_def && (pos = searchace (lacl, MAX_ACL_ENTRIES, 0)) >= 0)
{
- lacl[pos].a_type = DEF_CLASS_OBJ;
- lacl[pos].a_id = ILLEGAL_GID;
- lacl[pos].a_perm = S_IROTH | S_IWOTH | S_IXOTH;
+ /* Ensure that the default acl contains at
+ least DEF_(USER|GROUP|OTHER)_OBJ entries. */
+ if (!(types_def & USER_OBJ))
+ {
+ lacl[pos].a_type = DEF_USER_OBJ;
+ lacl[pos].a_id = uid;
+ lacl[pos].a_perm = lacl[0].a_perm;
+ pos++;
+ }
+ if (!(types_def & GROUP_OBJ) && pos < MAX_ACL_ENTRIES)
+ {
+ lacl[pos].a_type = DEF_GROUP_OBJ;
+ lacl[pos].a_id = gid;
+ lacl[pos].a_perm = lacl[1].a_perm;
+ pos++;
+ }
+ if (!(types_def & OTHER_OBJ) && pos < MAX_ACL_ENTRIES)
+ {
+ lacl[pos].a_type = DEF_OTHER_OBJ;
+ lacl[pos].a_id = ILLEGAL_GID;
+ lacl[pos].a_perm = lacl[2].a_perm;
+ pos++;
+ }
+ /* Include DEF_CLASS_OBJ if any named default ace exists. */
+ if ((types_def & (USER|GROUP)) && pos < MAX_ACL_ENTRIES)
+ {
+ lacl[pos].a_type = DEF_CLASS_OBJ;
+ lacl[pos].a_id = ILLEGAL_GID;
+ lacl[pos].a_perm = S_IROTH | S_IWOTH | S_IXOTH;
+ }
}
}
if ((pos = searchace (lacl, MAX_ACL_ENTRIES, 0)) < 0)