Bug 8673 - NT ACL issue
NT ACL issue
Status: RESOLVED FIXED
Product: Samba 3.6
Classification: Unclassified
Component: User & Group Accounts
3.6.1
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks: 8595
  Show dependency treegraph
 
Reported: 2011-12-20 01:11 UTC by srikumar
Modified: 2012-06-06 12:24 UTC (History)
2 users (show)

See Also:


Attachments
script to reproduce NT ACL delete permission issue (1.54 KB, application/octet-stream)
2011-12-22 03:54 UTC, srikumar
no flags Details
program to get/set NT ACL (1.91 MB, application/octet-stream)
2011-12-22 03:57 UTC, srikumar
no flags Details
git-am fix for 3.6.x (6.06 KB, patch)
2012-01-10 22:18 UTC, Jeremy Allison
jra: review? (metze)
jra: review? (ddiss)
Details
git-am fix for 3.5.x. (4.39 KB, patch)
2012-01-10 23:06 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description srikumar 2011-12-20 01:11:38 UTC
Samba deny to delete a file when local has deny and parent has allow. In Windows Server, this is a exception: delete is allowed if either parent or child has the delete permission allowed.


My observation with NT ACL:

"Delete" Allows or denies deleting the file or folder. If you don't have Delete permission on a file or folder, you can still delete it if you have been granted Delete Subfolders and Files on the parent folder. 

Parent dir: Deny  delete;  File/subdir: Allow delete;  Result: Allow delete

Parent dir: Deny  delete;  File/subdir: Deny  delete;  Result: Deny  delete

Parent dir: Allow delete;  File/subdir: Allow delete;  Result: Allow delete

Parent dir: Allow delete;  File/subdir: Deny  delete;  Result: Allow delete <= NOTE this case (Parent's Allow delete overrides)

Bottom Line: delete is allowed if either parent or child says allow
Comment 1 Jeremy Allison 2011-12-20 18:41:04 UTC
Yes - just confirmed this. Patch to follow.

Jeremy.
Comment 2 Jeremy Allison 2011-12-20 19:50:22 UTC
Can you give me an exact way to reproduce this please ? As far as I can see from looking at the 3.6.x code we already allow delete if either the file allows DELETE or containing directory allows DELETE_CHILD.

The only case it currently might fail is if the caller doesn't have rights to read the ACL on the contained object.

Jeremy.
Comment 3 srikumar 2011-12-21 02:45:32 UTC
Setup:
1) 'luser1' login to samba CIFS share (with NT ACL support) and mapped to n: 
2) 'luser1' creates 'dir1byluser1' inside n: and he has full access
4) 'luser1' sets 'dir1byluser1' with "luser2 full ALLOW"
5) 'luser1' creates a subdir 'subdir1byluser1' under 'dir1byluser1' 
6) 'luser1' sets 'subdir1byluser1' with "luser2 delete DENY"

7) Now login to the same share as 'luser2'
8) Try to delete 'subdir1byluser1' but the access is denied.

-----------------------
c:\>setacl -on n:\dir1byluser1\subdir1byluser1 -ot file -actn list -lst "f:tab;w:d,o,g;i:y" \\?\n:\dir1byluser1\subdir1byluser1

Owner: FC16\luser1
Group: FC16\None

DACL(not_protected+auto_inherited):
FC16\luser2   DELETE deny    container_inherit+object_inherit
FC16\luser2   full   allow   container_inherit+object_inherit +inherited
FC16\luser1   full   allow   container_inherit+object_inherit +inherited

SetACL finished successfully.

c:\>rmdir /S /Q n:\dir1byluser1\subdir1byluser1
Access is denied.
----------------------------
Comment 4 srikumar 2011-12-21 19:25:23 UTC
Setup:
1) 'luser1' login to samba CIFS share (with NT ACL support) and mapped to n: 
2) 'luser1' creates 'dir1byluser1' inside n: and he has full access
4) 'luser1' sets 'dir1byluser1' with "luser2 full ALLOW"
5) 'luser1' creates a subdir 'subdir1byluser1' under 'dir1byluser1' 
6) 'luser1' sets 'subdir1byluser1' with "luser2 delete DENY"

7) Now login to the same share as 'luser2'
8) Try to delete 'subdir1byluser1' but the access is denied.

-----------------------
c:\>setacl -on n:\dir1byluser1\subdir1byluser1 -ot file -actn list -lst "f:tab;w:d,o,g;i:y" \\?\n:\dir1byluser1\subdir1byluser1

Owner: FC16\luser1
Group: FC16\None

DACL(not_protected+auto_inherited):
FC16\luser2   DELETE deny    container_inherit+object_inherit
FC16\luser2   full   allow   container_inherit+object_inherit +inherited
FC16\luser1   full   allow   container_inherit+object_inherit +inherited

SetACL finished successfully.

c:\>rmdir /S /Q n:\dir1byluser1\subdir1byluser1
Access is denied.
----------------------------
Comment 5 Jeremy Allison 2011-12-21 22:35:38 UTC
Again, the setacl.exe command you used to set this up would be very helpful in trying to reproduce and fix.

Thanks !

Jeremy.
Comment 6 srikumar 2011-12-22 03:54:00 UTC
Created attachment 7211 [details]
script to reproduce NT ACL delete permission issue
Comment 7 srikumar 2011-12-22 03:57:36 UTC
Created attachment 7212 [details]
program to get/set NT ACL

This program can also be downloaded from http://sourceforge.net/projects/setacl/files/
Comment 8 Jeremy Allison 2012-01-10 01:09:28 UTC
Working on reproducing this now..

Jeremy.
Comment 9 Jeremy Allison 2012-01-10 01:17:36 UTC
Reproduced this - I see the problem. Give me a little while to code up the fix.

Thanks for your persistence on this !

Jeremy.
Comment 10 srikumar 2012-01-10 03:26:01 UTC
Thanks and let me know if you need me to test any further on this.
Comment 11 Jeremy Allison 2012-01-10 22:18:16 UTC
Created attachment 7236 [details]
git-am fix for 3.6.x

Here is the fix - against 3.6.x. Works for me. Please test and get back to me. This probably won't make 3.6.2, but should be ok for 3.6.3.

Jeremy.
Comment 12 Jeremy Allison 2012-01-10 23:06:57 UTC
Created attachment 7237 [details]
git-am fix for 3.5.x.
Comment 13 srikumar 2012-01-12 00:37:17 UTC
I verified the fix in Samba 3.6.1 and the fix holds good. I even introduced couple of subdirs inbetween to see multi-level inheritance and it holds good.

Thank you.
Comment 14 Jeremy Allison 2012-01-12 00:43:58 UTC
Comment on attachment 7236 [details]
git-am fix for 3.6.x

FYI. Reporter has confirmed this fix is good.

Trying to get review before 3.6.next :-).

Jeremy.
Comment 15 Jeremy Allison 2012-01-19 00:34:57 UTC
Comment on attachment 7236 [details]
git-am fix for 3.6.x

Trying to get review for 3.6.next (now we have a delay...).
Comment 16 Jeremy Allison 2012-01-19 19:27:50 UTC
Re-assigning to Karolin for inclusion in 3.5.next and 3.6.2.

Jeremy.
Comment 17 Karolin Seeger 2012-01-21 20:12:33 UTC
Pushed to v3-5-test and v3-6-test.
Closing out bug report.

Thanks!