The Samba-Bugzilla – Bug 6841
"map acl inherit = yes" not working
Last modified: 2017-06-29 16:25:16 UTC
We have Samba 3.4.2 with ACLs and EA support on OpenSuse 11.0 (PDC/BDC with OpenLDAP 2.4.19). Clients are WinXP Pro SP3. Winbindd ist not used. The Samba binaries come from the OpenSuse standard repository http://download.opensuse.org/repositories/network:/samba:/STABLE/openSUSE_11.0/. Filesystem is XFS.
It seems that at some point in the past "map acl inherit = yes" stopped working. We now have Samba 3.4.2, but this problem started with an earlier version, possibly some 3.2.x or 3.3.x.
When I try to propagate rights to lower level folders or try to inherit rights from higher level folders using the Windows security tab, the rights seem to be copied correctly, but inheritance is never shown. The "Inherit from parent the permission entries .." checkbox is never checked, and the "Inherited from" column always shows <not inherited>, no matter what I do.
The problem might have to do with the SAMBA_PAI extended attribute. For directories with existing version 1 SAMBA_PAI entries, the Windows security editor shows correct inheritance. As soon as rights are modified, a version 2 SAMBA_PAI gets written. The latter, however, seems to be ignored.
I can easily reproduce the bug on an emtpy test share.
On the share, I create folder 'aaa', and folder 'bbb' below 'aaa'.
Owner ist a test user 'brot', primary group is 'users'.
'brot' has full rights, 'users' and 'everyone' have no rights.
This is the typical setup for a user's home directory.
alekto:/backup/small # ls -la
drwxr-xr-x 3 root root 4096 Oct 23 15:04 .
drwxr-xr-x 4 root root 4096 Nov 10 2008 ..
drwx--S---+ 3 brot users 4096 Oct 23 15:18 aaa
alekto:/backup/small # getfacl aaa
# file: aaa
# owner: brot
# group: users
alekto:/backup/small # getfattr -d -e hex aaa
# file: aaa
alekto:/backup/small # ls -la aaa
drwx--S---+ 3 brot users 4096 Oct 23 15:18 .
drwxr-xr-x 3 root root 4096 Oct 23 15:04 ..
drwx--S---+ 2 brot users 4096 Oct 23 15:18 bbb
alekto:/backup/small # getfacl aaa/bbb
# file: aaa/bbb
# owner: brot
# group: users
alekto:/backup/small # getfattr -d -e hex aaa/bbb
# file: aaa/bbb
I will attach smb.conf and level 10 debug output.
Created attachment 4881 [details]
smb.conf for test share
Created attachment 4882 [details]
Level 10 dump of rights propragation from dir aaa down to bbb
Created attachment 4883 [details]
Windows Advanced Security Settings for aaa
This is a snapshot of the Windows Advanced Security Settings for folder aaa.
The dump in the previous attachment was created by checking "Replace permission entries on all child objects ..." and clicking 'Apply'.
Created attachment 4884 [details]
Level 10 dump of rights inheritance to dir bbb from aaa
Created attachment 4885 [details]
Windows Advanced Security Settings for bbb
This is a snapshot of the Windows Advanced Security Settings for folder bbb.
The dump in the previous attachment was created by checking "Inherit from parent the permissions ..." and clicking 'Apply'.
Ok, I've found the problem. There are a couple of bugs here. Patch to follow.
The lucky thing is that *writing* the V2 attributes is ok, so they should still all be stored on disk - just *reading* them had some bugs.
Created attachment 4925 [details]
Patch for 3.4.x
Can you test this patch for 3.4.x please ? I'm pretty sure this is the fix, so I've already committed to master and 3.5.0 - but once you confirm this fixes your problem I'll create a git-am patch and get it into 3.4.4.
Created attachment 4927 [details]
Level 10 dump of subdirectory yyy creation
I tested your patches and I see some improvement but it is still not the way it should be.
- when a subdir is created, rights are copied but inheritance is not there (as before). See yyy_after_creation.jpg
- now I can check "Inherit from parent the permission entries .." and unlike before it stays checked after "Apply". The rights list, however, does not reflect inheritance. See yyy_after_inherit_from_xxx.jpg
- next, after removing all the acl entries of yyy and clicking "Apply", the rights list re-appears with inheritance shown more or less correct. See yyy_after_inherit_from_xxx_and_acls_removed.jpg
- things look different again if I do it the other way round: send the rights down from xxx to yyy using "Replace permission entries on all child objects ..." in the acl editor for xxx. See yyy_after_replace_permissions_on_all_child_objects_of_xxx.jpg
Note the differences to the previous screenshot.
Created attachment 4928 [details]
Rights of subdir yyy after creation
Created attachment 4929 [details]
Rights of subdir yyy after inheritance from xxx
Created attachment 4930 [details]
Rights of subdir yyy after inheritance from xxx and acls removed
Created attachment 4931 [details]
Rights of (freshly created) subdir yyy after rights were propagated from xxx
This patch is definately correct for 3.4.4. Might not fix the entire problem, but does fix the issue that samba.PAI is not currently being read.
Requesting reviews from vl, obnox and metze.
(In reply to comment #13)
> This patch is definately correct for 3.4.4. Might not fix the entire problem,
> but does fix the issue that samba.PAI is not currently being read.
> Requesting reviews from vl, obnox and metze.
See my comment #8. I trust that the patch is correct with respect to reading samba.PAI, but there must be another flaw in the inheritance logic.
Sorry, Jeremy, the patch does not contain any explanation what it does. The patch to master which I had to look up only says "The code to read the new V2 SAMBA_PAI entries had two errors". Very enlightening. Can you be a bit more explicit about what the patch does?
Oh, sorry for the bad patch description, I'm assuming it was obvious :-).
First fix. The function: create_pai_v2_entries() was incorrect in that it only did the for() loop over "paiv->num_entries" - this is the value for number of entries for the "file" acl, but not the number of entries for the "default" acl, which is paiv->num_def_entries. So I added a "num_entries" parameter which is called from the function create_pai_val_v2() with a value of paiv->num_entries in the file acl case, and then with paiv->num_def_entries in the default acl case.
Second fix. In create_pai_v2_entries() there was a logic bug. entry_offset is a char * pointer, pointing at the start of the entry to read. After reading the ace_flags via:
paie->ace_flags = CVAL(entry_offset,0);
the original code incremented entry_offset, to read the next field. Unfortunately the increment at the bottom of the loop :
entry_offset += PAI_V2_ENTRY_LENGTH;
neglected to consider the fact entry_offset had been moved forward by one, and so all subsequent reads had an incorrect offset and read garbage. The fix leaved entry_offset alone within the loop until the bottom, where the full PAI_V2_ENTRY_LENGTH increment is correct, and uses entry_offset+1 to get the subsequent fields when calling get_pai_owner_type().
Hope that's more helpful for the code review.
seems to fix *a* bug, but not this one. It should still go into 3.4 :-)
Re-assigning to Karolin to get the code added to 3.4.4.
(In reply to comment #18)
> Re-assigning to Karolin to get the code added to 3.4.4.
Pushed to v3-4-test.
Re-assigning bug to Jeremy as Volker's comment seems to mean that the bug is not fixed.
This bug still exists in Samba 3.5.6 with Lenny + self-compiled Samba 3.5.6 with EXT3.
after many test This bug still exists in Samba 3.5.6 with squeeze with EXT3 or EXT4.
My Server had to use samba-3.0.33.
I recently upgraded it to samba-3.5.8 .
Results "map acl inherit = yes" is not working.
When in windows clients, the "Inherit from parent the
permission entries .." checkbox is never checked, and the "Inherited from"
column always shows <not inherited>, no matter what I do.
*** Bug 8158 has been marked as a duplicate of this bug. ***
Jeremy, can this be closed?
(In reply to Stefan Metzmacher from comment #25)
Sorry, this has completely dropped off my radar a long time ago.
I'd have to re-evaluate everything again, which I won't be able to get to for a while.