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

[Jeremy Allison]

On Wed, Mar 27, 2013 at 11:25:12AM -0700, Jeremy Allison wrote:
> On Wed, Mar 27, 2013 at 11:12:40AM -0700, Richard Sharpe wrote:
> > Hi folks,
> > 
> > I have, at long last, found the reason for the error message "The
> > permissions on blah are incorrectly ordered, which may cause some
> > entries to be ineffective."
> > 
> > It happens when you use robocopy and force the creation of the target
> > directory and there is a CREATOR OWNER or CREATOR GROUP entry in the
> > parent objects SD.
> > 
> > It happens, I believe because of the following code in
> > se_create_child_secdesc (in master,  3.6.x and 3.5.x) only sets the
> > inherited flag on directories if the SD control field of the parent
> > (we call it the type) contains SEC_DESC_DACL_AUTO_INHERITED (0x0400).
> > 
> > However, [MS-DTYP].pdf, section 2.4.4.1 (ACE_HEADER) in the subsection
> > on AceFlags says:
> > 
> > -----------------------------------------
> > INHERITED_ACE: 0x10
> > 
> > Indicates that the ACE was inherited. The system sets this bit when it
> > propagates an inherited ACE to a child object.<35>
> > -----------------------------------------
> > 
> > The footnote only indicates that the bit is not supported for Windows
> > NT 4.0 (and earlier, I imagine :-)
> > 
> > I am going to do a quick check on Windows Server 2008R2 and if Windows
> > does not do what Samba does, I will create a bug and submit a patch.
> 
> Check out bug https://bugzilla.samba.org/show_bug.cgi?id=9124
> 
> That's where the code change comes from (you reviewed it btw :-).
> 
> Once you think you have a change that works, we need to add
> in tests to raw.acl and smb2.acls to ensure we can differentiate
> between the old behavior and the new behavior and ensure those
> tests also pass against Windows 2012/2008.

So this is the patch for the server you'd need (attached).
You should also read :

http://social.msdn.microsoft.com/Forums/en-US/os_fileservices/thread/11f77b68-731e-407d-b1b3-064750716531

but this is probably about when to set the SEC_DESC_DACL_AUTO_INHERITED
bit on the DACL itself rather than the SEC_ACE_FLAG_INHERITED_ACE bit
on an ACE entry.

Jeremy.
-------------- next part --------------
diff --git a/libcli/security/secdesc.c b/libcli/security/secdesc.c
index d2c5833..a4ff310 100644
--- a/libcli/security/secdesc.c
+++ b/libcli/security/secdesc.c
@@ -639,7 +639,7 @@ NTSTATUS se_create_child_secdesc(TALLOC_CTX *ctx,
 			/* First add the regular ACE entry. */
 			init_sec_ace(new_ace, ptrustee, ace->type,
 				ace->access_mask,
-				set_inherited_flags ? SEC_ACE_FLAG_INHERITED_ACE : 0);
+				SEC_ACE_FLAG_INHERITED_ACE);
 
 			DEBUG(5,("se_create_child_secdesc(): %s:%d/0x%02x/0x%08x"
 				 " inherited as %s:%d/0x%02x/0x%08x\n",
@@ -664,7 +664,7 @@ NTSTATUS se_create_child_secdesc(TALLOC_CTX *ctx,
 
 		init_sec_ace(new_ace, ptrustee, ace->type,
 			     ace->access_mask, new_flags |
-				(set_inherited_flags ? SEC_ACE_FLAG_INHERITED_ACE : 0));
+				SEC_ACE_FLAG_INHERITED_ACE));
 
 		DEBUG(5, ("se_create_child_secdesc(): %s:%d/0x%02x/0x%08x "
 			  " inherited as %s:%d/0x%02x/0x%08x\n",