See bug #9190 Regression (change in behavior) of default acl masks as well. We incorrectly apply ACL masks to default ACL entries on directories (we should not) and miss applying them to SMB_ACL_USER and SMB_ACL_GROUP entries. Normally this isn't noticed as the default ACL masks are 0777. Patches for 3.5.x and 3.6.x to follow. Jeremy.
Created attachment 7980 [details] git-am fix for 3.6.next.
Created attachment 7981 [details] git-am fix for 3.5.next
Comment on attachment 7980 [details] git-am fix for 3.6.next. Looks fine for me. Thanks!
Comment on attachment 7981 [details] git-am fix for 3.5.next Looks good to me. Simo looked at them too. Thanks.
Re-assigning to Karolin for inclusion in 3.6.next and 3.5.next. Thanks ! Jeremy.
Pushed to v3-6-test and v3-5-test. Closing out bug report. Thanks!
There is one more fix needed here I missed. Additional patch to follow. Jeremy.
Arg. No, we don't need another patch (sorry). We're technically wrong, but we get away with it :-). Re-closing the bug. Sorry for the noise. Jeremy.
There is one more case here I need to address in 3.5.x, 3.6.x and 4.0.x. Patch to follow. Jeremy.
Created attachment 8169 [details] git-am fix for 3.6.x Another fix needed for bug #9236 - ACL masks incorrectly applied when setting ACLs. Not caught by make test as it's an extreme edge case for strange incoming ACLs. I only found this as I'm making raw.acls and smb2.acls pass against 3.6.x with acl_xattr mapped onto a POSIX backend (which isn't tested in make test). An incoming inheritable ACE entry containing only one permission, WRITE_DATA maps into a POSIX owner perm of "-w-", which violates the principle that the owner of a file/directory can always read. Patches to follow for 3.5.x and 4.0.x. Jeremy
Created attachment 8170 [details] git-am fix for 3.5.x Same fix applies to 3.5.x.
Created attachment 8171 [details] git-am fix for 4.0.0.rc.next Version for 4.0.0.rc.next.
This is needed for 4.0.0rc.next also. Jeremy.
(In reply to comment #10) > Created attachment 8169 [details] > git-am fix for 3.6.x > > Another fix needed for bug #9236 - ACL masks incorrectly applied when > setting ACLs. > > Not caught by make test as it's an extreme edge case for strange > incoming ACLs. I only found this as I'm making raw.acls and smb2.acls > pass against 3.6.x with acl_xattr mapped onto a POSIX backend (which > isn't tested in make test). > > An incoming inheritable ACE entry containing only one permission, > WRITE_DATA maps into a POSIX owner perm of "-w-", which violates > the principle that the owner of a file/directory can always read. > > Patches to follow for 3.5.x and 4.0.x. > > Jeremy Doesn't this create a problem that when the user reads the ACL back they see bits there that they did not add?
Only if they're not using acl_xattr (remember, with acl_xattr they see the exact Windows ACL they set, regardless of POSIX ACL permissions underneath). If they're mapping directly onto POSIX ACLs then yes, they'll see different permissions to those they set, but that's normal for a POSIX ACL mapping. This is a necessary precursor to the fixes I'm adding in bug #9374, which will allow 3.6.x and above to pass both the smb2.acl and the raw.acl tests with acl_xattr mapped onto a POSIX ACL backend. The issue is this: Default ACL is set to be Everyone:WRITE_DATA (no other permissions). Without this patch it creates a default POSIX ACL on the directory of owner:-w- which breaks the assumptions that the rest of the POSIX ACL mapping code makes that the owner can *always* open with O_RDONLY (which the previous code used to enforce - this is replacing that mapping). Hope this explanation helps. Jeremy.
Comment on attachment 8169 [details] git-am fix for 3.6.x Need to get this reviewed and in for 4.0.0 final... The smbtorture ACL fixes also rely on this.
Comment on attachment 8169 [details] git-am fix for 3.6.x I'm fine with it, if Richard is too :)
Comment on attachment 8170 [details] git-am fix for 3.5.x I'm fine with it, if Richard is too :)
Comment on attachment 8171 [details] git-am fix for 4.0.0.rc.next I'm fine with it, if Richard is too :)
As Richard is pretty busy with his new job I'm going to declare victory and assign to Karolin for inclusion in 3.5.x, 3.6.x and 4.0.0.rc.next :-). Jeremy.
(In reply to comment #20) > As Richard is pretty busy with his new job I'm going to declare victory and > assign to Karolin for inclusion in 3.5.x, 3.6.x and 4.0.0.rc.next :-). > > Jeremy. Pushed to v3-5-test, v3-6-test and autobuild-v4-0-test.
Pushed to v4-0-test. Closing out bug report. Thanks!