Permissions incorrectly ordered on Windows after disabling inheritance

[Walkes, Dan]

Hi everyone,

I'm reposting this message from the samba-general list based on
suggestions from members of that list.

I've noticed a problem with Debian wheezy + samba 3.6.6 configured with
acl_xattr. The following test sequence causes Windows Explorer to
report incorrectly ordered permission entries:
1) Map a share as with "admin" user credentials to a drive letter
on a Windows client
2) Create a folder at the root of the share "rootfolder"
3) Create a subfolder "subfolder1" under "rootfolder"
4) Un-check "Include inheritable permissions from this object's
parent" in the windows security settings dialog for Windows Explorer on
the root folder
5) Create a subfolder "subfolder2" under "subfolder1"
6) Right-click with Windows Explorer and attempt to edit the
permissions of "subfolder2". Windows Explorer pops up a message stating
"The permissions on subfolder2 are incorrectly ordered, which may cause
some entries to be ineffective."

This is reproducible on every Windows client system I've tried including
Windows 7, XP, Server 2008 R2 and Server 2003.
When incorrectly ordered, the permissions look like this as printed by
smbcacls smbcacls //localhost/20120821_3
rootfolder/subfolder1/subfolder2
REVISION:1
CONTROL:0x8004
OWNER:BIZNAS-H5\admin
GROUP:BIZNAS-H5\None
ACL:BIZNAS-H5\admin:ALLOWED/0x0/RWXDPO
ACL:Creator Owner:ALLOWED/OI|CI|IO|I/RWXDPO
ACL:BIZNAS-H5\None:ALLOWED/0x0/RWXDPO
ACL:Creator Group:ALLOWED/OI|CI|IO|I/RWXDPO
ACL:Everyone:ALLOWED/OI|CI|I/RWXDPO

For comparison, here is the same subfolder tree without performing step
4 above to un-check the "Include inheritable perimssions" box from
Windows explorer:
smbcacls //localhost/20120821_3 rootfolder/subfolder1/subfolder2
REVISION:1
CONTROL:0x8004
OWNER:BIZNAS-H5\admin
GROUP:BIZNAS-H5\None
ACL:BIZNAS-H5\admin:ALLOWED/0x0/RWXDPO
ACL:Creator Owner:ALLOWED/OI|CI|IO/RWXDPO
ACL:BIZNAS-H5\None:ALLOWED/0x0/RWXDPO
ACL:Creator Group:ALLOWED/OI|CI|IO/RWXDPO
ACL:Everyone:ALLOWED/OI|CI/RWXDPO admin@BizNAS-H5:/mnt/lvol0$

Note that the ACE entries are in the same order, however in the first
case where Windows reports incorrectly ordered ACE's Creator Owner,
Creator Group and Everyone ACE's include the "I" flag
SEC_ACE_FLAG_INHERITED_ACE

The share folder, rootfolder and subfolder1 permissions are as shown
below (steps 1 through 3)

smbcacls //localhost/20120821_3 rootfolder/..
REVISION:1
CONTROL:0x8004
OWNER:BIZNAS-H5\nobody
GROUP:Unix Group\root
ACL:BIZNAS-H5\nobody:ALLOWED/0x0/FULL
ACL:Unix Group\%naslocal%:ALLOWED/0x0/FULL ACL:Unix
Group\root:ALLOWED/0x0/FULL ACL:BIZNAS-H5\admin:ALLOWED/0x0/FULL
ACL:Everyone:ALLOWED/0x0/
ACL:Creator Owner:ALLOWED/OI|CI|IO/RWXDPO ACL:Creator
Group:ALLOWED/OI|CI|IO/RWXDPO ACL:Everyone:ALLOWED/OI|CI|IO/RWXDPO

smbcacls //localhost/20120821_3 rootfolder
REVISION:1
CONTROL:0x8004
OWNER:BIZNAS-H5\admin
GROUP:BIZNAS-H5\None
ACL:BIZNAS-H5\admin:ALLOWED/0x0/RWXDPO
ACL:Creator Owner:ALLOWED/OI|CI|IO/RWXDPO
ACL:BIZNAS-H5\None:ALLOWED/0x0/RWXDPO
ACL:Creator Group:ALLOWED/OI|CI|IO/RWXDPO
ACL:Everyone:ALLOWED/OI|CI/RWXDPO admin@BizNAS-H5:/mnt/lvol0$

smbcacls //localhost/20120821_3 rootfolder/subfolder1
REVISION:1
CONTROL:0x8004
OWNER:BIZNAS-H5\admin
GROUP:BIZNAS-H5\None
ACL:BIZNAS-H5\admin:ALLOWED/0x0/RWXDPO
ACL:Creator Owner:ALLOWED/OI|CI|IO/RWXDPO
ACL:BIZNAS-H5\None:ALLOWED/0x0/RWXDPO
ACL:Creator Group:ALLOWED/OI|CI|IO/RWXDPO
ACL:Everyone:ALLOWED/OI|CI/RWXDPO

Note that in each case flags OI|CI|IO are set on Creator Owner, Creator
Group and Everyone ACE's, however corresponding subfolders do not have
the "I" flag and SEC_ACE_FLAG_INHERITED_ACE set. I would have expected
this to be set for each inherited permission. Indeed Windows explorer
does mark these permissions as "Inherited From Z:\" where Z:\ is the
mapped share folder.

The value of subfolder1 after step 4 is:

smbcacls //localhost/20120821_3 rootfolder/subfolder1
REVISION:1
CONTROL:0x8d04
OWNER:BIZNAS-H5\admin
GROUP:BIZNAS-H5\None
ACL:BIZNAS-H5\admin:ALLOWED/I/RWXDPO
ACL:Creator Owner:ALLOWED/OI|CI|IO|I/RWXDPO
ACL:BIZNAS-H5\None:ALLOWED/I/RWXDPO
ACL:Creator Group:ALLOWED/OI|CI|IO|I/RWXDPO
ACL:Everyone:ALLOWED/OI|CI|I/RWXDPO

Note that when un-checking "Include inheritable permissions" and adding
existing permissions using Windows Explorer, Windows forces the "I"
SEC_ACE_FLAG_INHERITED_ACE flag on subfolder1 (and all subdirectories
below rootfolder) ACE's including the ACE entries "admin" and "None"
which were actually not inherited but created through the "Creator
Owner" ACE.

When viewing "Advanced Security Settings" on a folder with incorrectly
ordered permissions, Windows provides a "reorder" option. Reordering
the ACE's results in the following permissions:

smbcacls //localhost/20120821_3 rootfolder/subfolder1/subfolder2
REVISION:1
CONTROL:0x8d04
OWNER:BIZNAS-H5\admin
GROUP:BIZNAS-H5\None
ACL:BIZNAS-H5\admin:ALLOWED/0x0/RWXDPO
ACL:BIZNAS-H5\None:ALLOWED/0x0/RWXDPO
ACL:BIZNAS-H5\admin:ALLOWED/I/RWXDPO
ACL:Creator Owner:ALLOWED/OI|CI|IO|I/RWXDPO
ACL:BIZNAS-H5\None:ALLOWED/I/RWXDPO
ACL:Creator Group:ALLOWED/OI|CI|IO|I/RWXDPO
ACL:Everyone:ALLOWED/OI|CI|I/RWXDPO

Note that all "I" SEC_ACE_FLAG_INHERITED_ACE's are listed below entries
with inherit flags cleared - I'm guessing this was the reason for the
incorrect ordering message in Windows. I'm not sure why this is
required by Windows and I haven't come up with a scenario where
permissions are actually ineffective due to this ordering.

Assuming it is a requirement to order permissions in this way, I think
I've noticed two problems which are either samba bugs or some other
problem with my configuration which I've not yet identified.
1) ACE's are not ordered based in SEC_ACE_FLAG_INHERITED_ACE's to
include all permissions with "I" values at the end of the ACE list.
2) Although permissions on folders are marked with OI|CI|IO flags
appear to inherit properly from Windows, the "I" flag is not set in
corresponding ACE's.
My smb.conf configuration is below. I haven't found anything in the man
page for smb.conf which would explain this behavior. I've experimented
with turning off vfs_acl_xattr with this change to smb.conf:
# vfs objects = acl_xattr
dos filemode = yes
inherit acls = yes
force unknown acl user = yes
However in this case I've noticed that Windows does not indicate
permissions are inherited ("Include inheritable permissions from this
object's parent is un-checked") and I'd prefer a configuration which
mimics Windows server implementation as closely as possible.

Full smb.conf configuration:
[global]
workgroup = WORKGROUP
security = user
server string = %h server
obey pam restrictions = Yes
pam password change = Yes
unix password sync = Yes
log level = 0
syslog = 0
log file = /var/log/samba/log.%m
max log size = 1000
local master = No
domain master = No
dns proxy = No
socket options = TCP_NODELAY
panic action = /usr/share/samba/panic-action %d
idmap alloc config: range = 10000-100000
idmap uid = 10000 - 100000
idmap gid = 10000 - 100000
template shell = /bin/bash
winbind enum users = Yes
winbind enum groups = Yes
winbind use default domain = No
winbind refresh tickets = Yes
store dos attributes = yes
ea support = yes
vfs objects = acl_xattr
passdb backend = tdbsam
username map = /etc/samba/smbusers
encrypt passwords = yes
map to guest = Bad User
deadtime = 5
include = /etc/samba/dhcp.conf

[20120821_3]
comment =
path = /tmp/testshare3
map acl inherit = Yes
map archive = No
map read only = No
security mask = 0777
create mask = 0640
directory mask = 0750
delete readonly = yes
directory mode= 0777
create mode= 0777
acl map full control = True
read only = Yes
invalid users =
valid users = "@%naslocal%" "admin"
read list =
write list = "@%naslocal%" "admin"

If anyone has suggestions about any further troubleshooting steps to try
or changes in configuration which may resolve this issue please let me
know. Also if logs for any portion of this sequence would be useful I
can collect them.

Thanks and best regards,
Dan Walkes

[Richard Sharpe]

I have seen similar problems to this before, in the 3.5.x vfs_acl_xattr stuff.

Can you get a packet capture of the step where it all goes wrong? It
would be useful to see the ordering of the ACEs in the SD that is
being set and to see if acl_xattr is adding additional ACEs in the
wrong order.

--
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)

[Andrew Bartlett]

> > If anyone has suggestions about any further troubleshooting steps to try
> > or changes in configuration which may resolve this issue please let me
> > know. Also if logs for any portion of this sequence would be useful I
> > can collect them.
>
> I have seen similar problems to this before, in the 3.5.x vfs_acl_xattr stuff.
>
> Can you get a packet capture of the step where it all goes wrong? It
> would be useful to see the ordering of the ACEs in the SD that is
> being set and to see if acl_xattr is adding additional ACEs in the
> wrong order.

Richard and Dan,

It would be really, really useful to get this into a unit test. I've
created a scheme that will allow this locally in:
source4/scripting/python/samba/tests/ntacls.py

Otherwise or in addition we should add a remote test in smbtorture. The
new test environment work I did recently should help prevent us from
regressing here.

Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org

[Richard Sharpe]

Once I get a capture I can look at the problem, and then try to add a
unit test for this case to ntacls.py

[Walkes, Dan]

Richard and Andrew, thanks very much for your responses. I've responded
inline below

I've captured a series of traces at
ftp://ftp.tandbergdata.com/pub/samba/Traces/ and spent a bit of time
analyzing. I captured one trace to correspond to each of the steps in
my repro steps listed in the first e-mail. As far as "where it all goes
wrong" I'm not sure I know exactly - however here's my understanding
based on review of the traces and some reading of the CIFS spec at
http://msdn.microsoft.com/en-us/library/ee442092(v=prot.13).aspx
1) In the step4 trace at Frame 1840 Windows resets security on
rootfolder\subfolder1 to force the "I" flag on all permissions entries:
---
Frame 1840: 358 bytes on wire (2864 bits), 358 bytes captured (2864
bits)
Arrival Time: Aug 27, 2012 10:56:30.561976000 Mountain Daylight Time
...
NT SET SECURITY DESC Parameters
FID: 0x3c53 (\rootfolder\subfolder1)
[Opened in: 1823]
[Closed in: 1844]
...
NT User (DACL) ACL
Revision: NT4 (2)
Size: 140
Num ACEs: 5
NT ACE:
S-1-5-21-3273740905-2057686771-3188122883-1000 (Domain SID-Domain RID),
flags 0x10, Access Allowed, mask 0x001f01bf
Type: Access Allowed (0)
NT ACE Flags: 0x10 Inherited ACE
...
NT ACE: S-1-3-0 (Creator Owner), flags 0x1b, Access
Allowed, mask 0x001f01bf
Type: Access Allowed (0)
NT ACE Flags: 0x1b Inherited ACE, Inherit Only,
Container Inherit, Object Inherit
...
NT ACE:
S-1-5-21-3273740905-2057686771-3188122883-513 (Domain SID-Domain
Users), flags 0x10, Access Allowed, mask 0x001f01bf
Type: Access Allowed (0)
NT ACE Flags: 0x10 Inherited ACE
---
All ACE's are marked as inherited with 0x10 flag set by Windows
including Creator Owner. I believe S-1-5-21-XXX-1000 =user admin and
S-1-5-21-XXX-513 =group NONE weren't actually inherited but were
created through CREATOR OWNER and CREATOR GROUP ACE's which were
inherited.

2) In the step5 trace - Frame 664 Create AndX Request for FID 0x3C69
path: \rootfolder\subfolder1\New Folder
The Create AndX Request doesn't include ACE's that I can see - and I
can't find this explicitly stated in the protocol documents, but I'm
assuming the samba server is filling in the default ACE's based on
inheritance? Therefore I don't see the details in the wireshark trace
but I suspect the folder is created by samba with ACE for user admin and
group NONE not inherited (0x10 flag cleared) as would be expected since
the permissions were created based on CREATOR OWNER and not inherited.
However the ACE entries remain in the same order as the parent folder
and the Inherited flag remains set set on Creator Owner ACE. This means
S-1-5-21-XXX-1000 =user admin remains ahead of ACE: S-1-3-0 (Creator
Owner) in the ACE list despite the fact that 0x10 is cleared on the user
admin ACE, resulting in an incorrect ordering message.

> >> It would be useful to see the ordering of the ACEs in the SD that
> >> is being set and to see if acl_xattr is adding additional ACEs in
> >> the wrong order.

My guess is that acl_xattr is replacing ACE's for user admin and group
NONE in their current position within the ACL, clearing 0x10 "I" flag
since these weren't inherited but added based on CREATOR OWNER, and not
modifying order of ACE's to account for the setting of the 0x10 inherit
flag on the CREATOR OWNER ACE.

The problem could also be related to the fact that the 0x10 "I" flag
never appears to be set by samba even when setting permissions based on
inheritance. Instead the "I" flags appear to always be copied from the
root folder directly, meaning the I flag would only be set if the root
folder's I flag was set for a specific ACE. It appears OI|CI|IO|I flags
are always set based on the parent folder setting with samba. In the
windows server case I believe OI|CI|IO are copied and the I flag is
always set when the corresponding ACE is there due to inheritance from
the parent (regardless of the parent setting of the "I" flag). I
suspect if samba set the "I" flag on permissions which were in inherited
AND ordered all permissions such that permissions with the I flag set
were at the end of the ACL the behavior would match the windows server
CIFS implementation and there would be no more warnings about incorrect
ordering. I'm new to samba/CIFS details though so I might be missing
something.

> > Richard and Dan,
> >
> > It would be really, really useful to get this into a unit test.
> > I've created a scheme that will allow this locally in:
> > source4/scripting/python/samba/tests/ntacls.py
>
> Once I get a capture I can look at the problem, and then try to add a
> unit test for this case to ntacls.py

Thanks again for looking at this. Let me know if there's anything else
I can do to help.

[Richard Sharpe]

On Mon, Aug 27, 2012 at 4:29 PM, Walkes, Dan <dwa...@tandbergdata.com> wrote:
> Awesome! Thanks!

Looks like the problem is in lib/secdesc.c:se_create_child_secdesc. It
needs to make an ordering pass over the ACL in the SD to ensure that
the ACEs are ordered correctly. At least that is the case in the
Samba 3.5.x code, and I don't think there has been much change there
in 3.6.x.

> -----Original Message-----
> From: Richard Sharpe [mailto:realrich...@gmail.com]
> Sent: Monday, August 27, 2012 5:24 PM
> To: Walkes, Dan
> Subject: Re: Permissions incorrectly ordered on Windows after disabling inheritance
>
> On Mon, Aug 27, 2012 at 4:09 PM, Richard Sharpe <realrich...@gmail.com> wrote:

>> On Mon, Aug 27, 2012 at 3:48 PM, Walkes, Dan <dwa...@tandbergdata.com> wrote:
>>> Richard and Andrew, thanks very much for your responses. I've
>>> responded inline below
>>

>> Thanks for the captures.
>>
>> OK, in looking at Step4UnCheck... we see the Inherited bits added in
>> frame 1840 as you say. Just before that in frame 1833 we see the
>> existing SD queried. Those Creator Owner and Creator Group entries
>> were already there and they are inheritable, but I expect that.
>>
>> Now I will look at the next capture.
>
> OK, I can see the problem. It is in frame 522 of Step 6. The ACE for the owner, which was added as a result of the Creator Owner entry and the Group Owner Sid, which was created as a result of the Creator Group entry are in the wrong place. I will have to check, but I believe that inherited entries should come last, so the ACL has been incorrectly sorted.
>
> It should be easy to fix in the code. I will look at it over the next day or so.

[Jeremy Allison]

On Mon, Aug 27, 2012 at 04:59:34PM -0700, Richard Sharpe wrote:
> On Mon, Aug 27, 2012 at 4:29 PM, Walkes, Dan <dwa...@tandbergdata.com> wrote:
> > Awesome! Thanks!
>
> Looks like the problem is in lib/secdesc.c:se_create_child_secdesc. It
> needs to make an ordering pass over the ACL in the SD to ensure that
> the ACEs are ordered correctly. At least that is the case in the
> Samba 3.5.x code, and I don't think there has been much change there
> in 3.6.x.

Yep, I'm looking at that function now :-). I think there are a few
more errors there than just the ordering pass - let me look at this.

Jeremy.

[Andrew Bartlett]

Let me know if you need help adding these test cases to ntacls.py. That
will also let you test the posix ACL the is output.

[Jeremy Allison]

On Tue, Aug 28, 2012 at 10:10:30AM +1000, Andrew Bartlett wrote:
> On Mon, 2012-08-27 at 17:04 -0700, Jeremy Allison wrote:
> > On Mon, Aug 27, 2012 at 04:59:34PM -0700, Richard Sharpe wrote:
> > > On Mon, Aug 27, 2012 at 4:29 PM, Walkes, Dan <dwa...@tandbergdata.com> wrote:
> > > > Awesome! Thanks!
> > >
> > > Looks like the problem is in lib/secdesc.c:se_create_child_secdesc. It
> > > needs to make an ordering pass over the ACL in the SD to ensure that
> > > the ACEs are ordered correctly. At least that is the case in the
> > > Samba 3.5.x code, and I don't think there has been much change there
> > > in 3.6.x.
> >
> > Yep, I'm looking at that function now :-). I think there are a few
> > more errors there than just the ordering pass - let me look at this.
>
> Let me know if you need help adding these test cases to ntacls.py. That
> will also let you test the posix ACL the is output.

Thanks - this is going to take a few hours stating at, so I'll probably
ask for your help on this tomorrow.

Cheers & thanks !

Jeremy.

[Jeremy Allison]

On Mon, Aug 27, 2012 at 04:59:34PM -0700, Richard Sharpe wrote:
> On Mon, Aug 27, 2012 at 4:29 PM, Walkes, Dan <dwa...@tandbergdata.com> wrote:
> > Awesome! Thanks!
>
> Looks like the problem is in lib/secdesc.c:se_create_child_secdesc. It
> needs to make an ordering pass over the ACL in the SD to ensure that
> the ACEs are ordered correctly. At least that is the case in the
> Samba 3.5.x code, and I don't think there has been much change there
> in 3.6.x.

Actually, looking more closely at this I think it's a pretty
simple bug in that I just forgot to set the SEC_ACE_FLAG_INHERITED_ACE
on inherited ACE's when I create them :-).

Should have a patch to test tomorrow (home now..).

Jeremy.

[Richard Sharpe]

Well, I guess that depends on the semantics of Creator Owner with the
inherited bit set, doesn't it? Does Windows mark the new ACE created
as a result of a Creator Owner ace that has the inherited bit set as
inherited as well?

[Jeremy Allison]

Yep (been testing against Win7). Windows marks *all*
ACE's it creates as part of the inheritance code path
with the SEC_ACE_FLAG_INHERITED_ACE bit.

It doesn't matter what the original inherited bit was.

Jeremy.

[Jeremy Allison]

And here's a COMPLETELY UNTESTED :-) patch.

Compiles, but that's all I can say right now..

I'll test when I get into work on my test environment
tomorrow.

Cheers,

Jeremy.

[Scott Lovenberg]

I'm really punchy, so if this is a stupid question, please be kind about
it. :)
Is there any chance this will trigger that infamous Office bug where the
permissions on a saved file will be reset to the default permissions of the
parent directory regardless of what the actual ACL is on the file that is
being overwritten by the temp file?

I'm looking at deploying Samba-4 in the very near future at work and I'll
do anything to not trigger that bug again. It has literally cost me
sleepless nights. :)

--
Peace and Blessings,
-Scott.

[Jeremy Allison]

On Mon, Aug 27, 2012 at 11:55:03PM -0400, Scott Lovenberg wrote:
>
> I'm really punchy, so if this is a stupid question, please be kind about
> it. :)
> Is there any chance this will trigger that infamous Office bug where the
> permissions on a saved file will be reset to the default permissions of the
> parent directory regardless of what the actual ACL is on the file that is
> being overwritten by the temp file?
>
> I'm looking at deploying Samba-4 in the very near future at work and I'll
> do anything to not trigger that bug again. It has literally cost me
> sleepless nights. :)

Don't know - do you have a bugzilla id number for it ?

Jeremy.

[Scott Lovenberg]

It's 2346 (ref: https://bugzilla.samba.org/show_bug.cgi?id=2346).
Currently closed again. :) It popped up a few times over the course of a
few years.
.

[Jeremy Allison]

Nope, this fix is nothing to do with that bug.

Jeremy.

[Scott Lovenberg]

Awesome. Thanks.

[Jeremy Allison]

It's tested now :-). Works perfectly (i.e. I reproduced the
original reported bug, and the attached patch fixes it).

I'll push this to master, and raise a bug for 3.5.x and 3.6.x.

The nice thing about this is it's not associated with mapping
into POSIX ACLs at all, it's simply a missing bit in the reported
ACE entries (the fact that when re-ordering the Windows client
added a duplicate entry but with the "SEC_ACE_FLAG_INHERITED_ACE"
bit set was the give-away) so it'll be easy to add a RAW-ACLS
testcase for it.

Cheers,

Jeremy.

[Jeremy Allison]

Hmmm. Not quite so simple as it looks :-). Causes us to fail
the raw.acls test. I'll investigate some more, but I'm
definitely on the right track.

[Jeremy Allison]

On Tue, Aug 28, 2012 at 11:15:50AM -0700, Jeremy Allison wrote:
>
> Hmmm. Not quite so simple as it looks :-). Causes us to fail
> the raw.acls test. I'll investigate some more, but I'm
> definitely on the right track.

OK, figured this out I think. More testing then I'll have
something to push..

Jeremy.