I believe I have found the reason for "The permissions on blah are incorrectly ordered ..."

[Jeremy Allison]

On Wed, Mar 27, 2013 at 02:35:42PM -0700, Richard Sharpe wrote:
> On Wed, Mar 27, 2013 at 1:55 PM, Richard Sharpe
> <realrichardsharpe at gmail.com> wrote:
> >
> > Well, after playing with Windows a bit in this regard, it seems that
> > Samba does have a bug, but not the bug I thought:
> >
> > 1. Samba's behavior with respect to SEC_DESC_DACL_AUTO_INHERITED is
> > correct, and Windows behaves the same way, at least with respect to
> > CREATOR OWNER, howvever it seems to include the inherited flag when it
> > should not:
> >
> > cc1# smbcacls //localhost/cloudfs /cc1/test1/testit2
> > -Unimbus-10/administrator%*********
> > Failed to load upcase.dat, will use lame ASCII-only case sensitivity rules
> > Failed to load lowcase.dat, will use lame ASCII-only case sensitivity rules
> > REVISION:1
> > CONTROL:0x8004
> > OWNER:BUILTIN\Administrators
> > GROUP:NIMBUS-10\Domain Users
> > ACL:BUILTIN\Administrators:ALLOWED/OI|CI|I/FULL
> > ACL:NIMBUS-10\test1:ALLOWED/0x0/FULL                   # <------- Note
> > this one nestled in there
> > ACL:Creator Owner:ALLOWED/OI|CI|IO|I/FULL
> > ACL:NIMBUS-10\funny-group:ALLOWED/OI|CI|I/READ
> > ACL:NIMBUS-10\Domain Users:ALLOWED/OI|CI|I/READ
> >
> > 2. While, Windows orders the ACEs in the canonical order, and Samba
> > does not, however, the failure here might be that Samba is allowing
> > the inherited flag through when it should not.
> >
> > Still investigating.
> >
> > As a side issue, it seems that Windows (at least Win 7 and W2K08) does
> > not set the owner to the person who created the file/folder. They all
> > end up being owned by NT AUTHORITY\SYSTEM on my Win 7 machine.
> > However, this is an issue for another day, but is likely to confuse
> > veteran Windows admins.
> 
> This appears to be the fix:
> 
> --- samba-3.6.12/source3/lib/secdesc.c.orig       2013-01-29
> 00:49:31.000000000 -0800
> +++ samba-3.6.12/source3/lib/secdesc.c  2013-03-27 21:25:35.978421867 -0700
> @@ -592,7 +592,7 @@ NTSTATUS se_create_child_secdesc(TALLOC_
>                 struct security_ace *new_ace = &new_ace_list[new_ace_list_ndx];
>                 const struct dom_sid *ptrustee = &ace->trustee;
>                 const struct dom_sid *creator = NULL;
> -               uint8 new_flags = ace->flags;
> +               uint8 new_flags = ace->flags & ~SEC_ACE_FLAG_INHERITED_ACE;
> 
>                 if (!is_inheritable_ace(ace, container)) {
>                         continue;
> 
> We are trying to OR in the INHERITED flag depending on the flag in the
> type/control field on the parent SD, but are forgetting that it might
> already be in the flags on the ACE we are dealing with. So, we have to
> get rid of it first.

Ah - that makes a lot of sense !

Is there some way we can test this in the raw.acls or smb2.acls test code ?

Jeremy.