Bug 6841 - "map acl inherit = yes" not working
Summary: "map acl inherit = yes" not working
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.4
Classification: Unclassified
Component: File services (show other bugs)
Version: 3.4.2
Hardware: x64 Windows XP
: P3 normal
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL: http://www.wzb.eu
Keywords:
: 8158 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-10-23 10:11 UTC by Peter Rindfuss
Modified: 2020-12-22 02:39 UTC (History)
10 users (show)

See Also:
vl: review+


Attachments
smb.conf for test share (2.37 KB, text/plain)
2009-10-23 10:21 UTC, Peter Rindfuss
no flags Details
Level 10 dump of rights propragation from dir aaa down to bbb (982.46 KB, text/plain)
2009-10-23 10:26 UTC, Peter Rindfuss
no flags Details
Windows Advanced Security Settings for aaa (50.88 KB, image/jpeg)
2009-10-23 10:34 UTC, Peter Rindfuss
no flags Details
Level 10 dump of rights inheritance to dir bbb from aaa (272.42 KB, text/plain)
2009-10-23 10:37 UTC, Peter Rindfuss
no flags Details
Windows Advanced Security Settings for bbb (50.93 KB, image/jpeg)
2009-10-23 10:39 UTC, Peter Rindfuss
no flags Details
Patch for 3.4.x (3.71 KB, patch)
2009-11-06 19:15 UTC, Jeremy Allison
no flags Details
Level 10 dump of subdirectory yyy creation (414.12 KB, text/plain)
2009-11-08 10:09 UTC, Peter Rindfuss
no flags Details
Rights of subdir yyy after creation (50.87 KB, image/jpeg)
2009-11-08 10:10 UTC, Peter Rindfuss
no flags Details
Rights of subdir yyy after inheritance from xxx (50.88 KB, image/jpeg)
2009-11-08 10:11 UTC, Peter Rindfuss
no flags Details
Rights of subdir yyy after inheritance from xxx and acls removed (50.58 KB, image/jpeg)
2009-11-08 10:12 UTC, Peter Rindfuss
no flags Details
Rights of (freshly created) subdir yyy after rights were propagated from xxx (50.44 KB, image/jpeg)
2009-11-08 10:14 UTC, Peter Rindfuss
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Rindfuss 2009-10-23 10:11:11 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
total 16
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
user::rwx
group::---
other::---
default:user::rwx
default:group::---
default:other::---

alekto:/backup/small # getfattr -d -e hex aaa
# file: aaa
user.SAMBA_PAI=0x02049d03000300000036750000000151c300000302ffffffff0b00367500000b0151c300000302ffffffff

alekto:/backup/small # ls -la aaa
total 20
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
user::rwx
group::---
other::---
default:user::rwx
default:group::---
default:other::---

alekto:/backup/small # getfattr -d -e hex aaa/bbb
# file: aaa/bbb
user.SAMBA_PAI=0x02049d03000300030036750000000151c300000302ffffffff0300367500000b0151c300000302ffffffff

I will attach smb.conf and level 10 debug output.
Comment 1 Peter Rindfuss 2009-10-23 10:21:47 UTC
Created attachment 4881 [details]
smb.conf for test share
Comment 2 Peter Rindfuss 2009-10-23 10:26:47 UTC
Created attachment 4882 [details]
Level 10 dump of rights propragation from dir aaa down to bbb
Comment 3 Peter Rindfuss 2009-10-23 10:34:37 UTC
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'.
Comment 4 Peter Rindfuss 2009-10-23 10:37:36 UTC
Created attachment 4884 [details]
Level 10 dump of rights inheritance to dir bbb from aaa
Comment 5 Peter Rindfuss 2009-10-23 10:39:46 UTC
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'.
Comment 6 Jeremy Allison 2009-11-06 19:01:05 UTC
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.
Jeremy.
Comment 7 Jeremy Allison 2009-11-06 19:15:06 UTC
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.
Thanks !
Jeremy.
Comment 8 Peter Rindfuss 2009-11-08 10:09:11 UTC
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.

Observations:

- 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.
Comment 9 Peter Rindfuss 2009-11-08 10:10:27 UTC
Created attachment 4928 [details]
Rights of subdir yyy after creation
Comment 10 Peter Rindfuss 2009-11-08 10:11:34 UTC
Created attachment 4929 [details]
Rights of subdir yyy after inheritance from xxx
Comment 11 Peter Rindfuss 2009-11-08 10:12:27 UTC
Created attachment 4930 [details]
Rights of subdir yyy after inheritance from xxx and acls removed
Comment 12 Peter Rindfuss 2009-11-08 10:14:31 UTC
Created attachment 4931 [details]
Rights of (freshly created) subdir yyy after rights were propagated from xxx
Comment 13 Jeremy Allison 2009-12-01 11:47:17 UTC
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.
Jeremy.
Comment 14 Peter Rindfuss 2009-12-03 10:04:47 UTC
(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.
> Jeremy.
> 

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.
Peter.
Comment 15 Volker Lendecke 2009-12-05 14:33:38 UTC
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?

Thanks,

Volker
Comment 16 Jeremy Allison 2009-12-05 20:32:22 UTC
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.

Jeremy.
Comment 17 Volker Lendecke 2009-12-13 06:12:14 UTC
https://bugzilla.samba.org/attachment.cgi?id=4925&action=view

seems to fix *a* bug, but not this one. It should still go into 3.4 :-)

Volker
Comment 18 Jeremy Allison 2009-12-15 17:31:56 UTC
Re-assigning to Karolin to get the code added to 3.4.4.
Jeremy.
Comment 19 Karolin Seeger 2009-12-18 06:49:08 UTC
(In reply to comment #18)
> Re-assigning to Karolin to get the code added to 3.4.4.
> Jeremy.
> 

Pushed to v3-4-test.
Comment 20 Karolin Seeger 2009-12-18 06:50:31 UTC
Re-assigning bug to Jeremy as Volker's comment seems to mean that the bug is not fixed. 
Comment 21 TAKAHASHI Motonobu 2010-10-09 03:32:58 UTC
This bug still exists in Samba 3.5.6 with Lenny + self-compiled Samba 3.5.6 with EXT3.

Comment 22 zorg 2011-02-28 16:29:50 UTC
 
after many test This bug still exists in Samba 3.5.6 with squeeze with EXT3 or EXT4.
Comment 23 odonata 2011-05-12 09:18:47 UTC
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.
Comment 24 Jeremy Allison 2011-05-25 23:16:37 UTC
*** Bug 8158 has been marked as a duplicate of this bug. ***
Comment 25 Stefan Metzmacher 2017-06-29 14:33:12 UTC
Jeremy, can this be closed?
Comment 26 Jeremy Allison 2017-06-29 16:25:16 UTC
(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.
Comment 27 Björn Jacke 2020-12-22 02:39:55 UTC
Jeremy, I assume everything is fixed here.

If *not*, then please reopen this bug report.