If a user has the take ownership bit set on a file he should be able to take ownership of a file (this option is offered by XP only if this bit is set). Currently when trying to do so we receive an access denied dialog box. To reproduce the problem: * Setup a share with the acl_xattr module * create a file with user A gives read/write rights to the user B * with user B check on Windows that you are not offered the right to change the owner * with user A give the "take ownership right" to user B (security tab then advanced then add then check "take ownership"). * with user B try to take the ownership, windows will allow you to proceed but you'll receive an access denied from the server
Matthieu, I'm guessing that when you tried this on a Windows box the user who was taking the ownership had SE_TAKE_OWNERSHIP privilege, and the user you're trying this with on the Samba box does not. Can you add the SE_TAKE_OWNERSHIP privilege to the user who is taking the ownership on the Samba box and retry please, as this is required in order to do the underlying chown action. Jeremy.
>Matthieu, I'm guessing that when you tried this on a Windows box the user who >was taking the ownership had SE_TAKE_OWNERSHIP privilege, and the user you're >trying this with on the Samba box does not. No the user who created the file was an admin but the user who was granted the "take ownership right" was a simple user. If you are a user with the SE_TAKE_OWNERSHIP privilege, you don't need the "take ownership" right on a file to be allowed to take the ownership. >Can you add the SE_TAKE_OWNERSHIP privilege to the user who is taking >the >ownership on the Samba box and retry please, as this is required in order to do >the underlying chown action. This I tried it's working correctly. This is just the handling of the ACL's bit "take ownership" that is not correct. Jeremy.
Ok I tried to test this on my Win7 client box locally. I created a new "test" user with no privileges and gave that user "take ownership" on a file owned by a different user, but when I tried to take ownership I got a security privilege request asking for an administrator password. I think we need to torture test this to be sure. Jeremy.
I must confess that I didn't try with a windows 7 but I tried with XP one more time and windows server 2003 R2. I have the feeling that UAC is entering into the game to "protect" your computer. You should redo the test on a share remotely or deactivate UAC.
I still can't reproduce this on an XP box :-(. Can you upload a debug level 10 log of this failing against the v3-5-test tree as well as a wireshark trace (as there has been a lot of changes to the ACL code between 3.5.6 and the current v3-5-test tree). Jeremy.
Jeremy, do you mean that you are unable to reproduce the fact that a user that has not SE_TAKE_OWNERSHIP right can still take the ownership of a file if he has be granted the "take ownership" bit on a file in XP or you can't reproduce the problem when using XP as a client to s3.5-test server ?
Jeremy any update on this bug ?
No. I still assert that SE_TAKE_OWNERSHIP privilege is *required* to change file ownership to yourself. You haven't shown me any traces that prove me wrong here.
Jeremy, you have to review your assertion :-) See the attached trace, at packet 300 you can see that the owner is the local administrator group and at packet 321 I'm able to change the owner, if you look at the DACL part of the ACL (in packet 184 you'll see that this user has the right to set the owner on this file). I added a couple of screenshot that shows that if I don't have the right (file without_to_aclright.jpg) then I can't take the ownership ...
Created attachment 7140 [details] capture with a simple user (demo) not in the admin group taking ownership
Created attachment 7141 [details] acl on the file without the take ownership right
Created attachment 7142 [details] acl on the file with the take ownership right
Created attachment 7143 [details] screenshot showning that the change of owner if forbidden on file without_to_aclrights
Ok we should write a torture test to check this against Windows from a non-privileged user. Will have to wait until after thanksgiving though. Jeremy.
Crap, I'm wrong - tested on Win7 :-(. I'll add a patch that enables this for vfs_acl_common.c but I really don't want to do this for "standard" POSIX mapping as rwx maps to FULL CONTROL which means anyone with rwx on a file could chown it. Matthieu, thanks for being so persistent in making me check this. Jeremy.
Ok, I have a fix for this for the vfs_acl_xattr and vfs_acl_tdb modules (in master). I'll check in tomorrow once I've confirmed the patch. Jeremy.
Created attachment 7292 [details] fix for 3.6.next Works for me here (although it exposed another mapping bug). Matthieu, can you test this out and confirm please ? Thanks, Jeremy.
Comment on attachment 7292 [details] fix for 3.6.next Matthieu, can you review for 3.6.next please ?
Patch is incorrect - will crash smbd. Thanks to Andrew Bartlett <abartlet@samba.org> and Ricky Nance <ricky.nance@weaubleau.k12.mo.us> for finding this problem. Fixed patch to follow ! Jeremy
Comment on attachment 7292 [details] fix for 3.6.next Patch crashes smbd. Modified fix to follow.
Created attachment 7384 [details] git-am fix for 3.6.x that contains both fixes from master.
Created attachment 7581 [details] git-am fix for 3.6.x that contains both fixes from master. New patch with fix noticed by 逸群周 <jxxyyzz@gmail.com>.
should we close this bug ?
Looks like with the change introduced with this patchset, we implement give ownership semantics with vfs_acl_xattr|tdb for any user. Testing suggests that this is not the way Windows behaves here. It seems Windows implements SEC_STD_WRITE_OWNER *filesystem permission* for normal users as only allowing "take ownership", only users with SEC_PRIV_RESTORE *privilege* are allowed to "give ownership". Reproducer: - Samba 4 AD DC - Windows 10 member server (IP 10.10.11.102) - on the Windows 10 member server a share "share" with a file "WHATSNEW.txt" with FULL permissions for user "HILLHOUSE\administrator" - BUILTIN\administrators on the Windows 10 system initially has the default priveleges for the builtin admins group including SeTakeOwnershipPrivilege and SeRestorePrivilege Test showing that giving ownership away works with SEC_STD_WRITE_OWNER on the file *and* SEC_PRIV_RESTORE: $ bin/rpcclient -U HILLHOUSE\\administrator%Passw0rd 10.10.11.102 -c "lsaenumacctrights S-1-5-32-544" | egrep "SeTakeOwnershipPrivilege|SeRestorePrivilege" SeTakeOwnershipPrivilege SeRestorePrivilege $ bin/smbcacls -W HILLHOUSE -U Administrator%Passw0rd -m smb3 //10.10.11.102/share "WHATSNEW.txt" | grep OWNER OWNER:WIN10\test2 $ bin/smbcacls -W HILLHOUSE -U Administrator%Passw0rd -m smb3 //10.10.11.102/share "WHATSNEW.txt" -C WIN10\\test1 $ bin/smbcacls -W HILLHOUSE -U Administrator%Passw0rd -m smb3 //10.10.11.102/share "WHATSNEW.txt" | grep OWNER OWNER:WIN10\test1 Removing SEC_PRIV_RESTORE, giving away ownership now fails: $ bin/rpcclient -U HILLHOUSE\\administrator%Passw0rd 10.10.11.102 -c "lsadelpriv S-1-5-32-544 SeRestorePrivilege" $ bin/smbcacls -W HILLHOUSE -U Administrator%Passw0rd -m smb3 //10.10.11.102/share "WHATSNEW.txt" -C WIN10\\test2 ERROR: security descriptor set failed: NT_STATUS_INVALID_OWNER Readding, works again: $ bin/rpcclient -U HILLHOUSE\\administrator%Passw0rd 10.10.11.102 -c "lsaaddpriv S-1-5-32-544 SeRestorePrivilege" $ bin/smbcacls -W HILLHOUSE -U Administrator%Passw0rd -m smb3 //10.10.11.102/share "WHATSNEW.txt" -C WIN10\\test2 $ bin/smbcacls -W HILLHOUSE -U Administrator%Passw0rd -m smb3 //10.10.11.102/share "WHATSNEW.txt" | grep OWNER OWNER:WIN10\test2 Note the error coce NT_STATUS_INVALID_OWNER. This is described in MS-FSA 2.1.5.16: If InputBuffer.OwnerSid is not a valid owner SID for a file in the objectstore, as determined in an implementation-specific manner, the object store MUST return STATUS_INVALID_OWNER. Man, how I love "implementation-specific manners"!
Created attachment 13652 [details] WIP patch for master Something like this...
(In reply to Ralph Böhme from comment #25) Don't you still need to allow the original behaviour if the current token has SeRestore privilege ?
(In reply to Jeremy Allison from comment #26) That is already dealt with in posix_acls.c:try_chown().
Created attachment 13674 [details] Patch for 4.6 backported from master
Created attachment 13675 [details] Patch for 4.7 cherry-picked from master
Re-assigning to Karo/metze for release in 4.6.next, 4.7.next.
Comment on attachment 13674 [details] Patch for 4.6 backported from master See bug 13085.
Comment on attachment 13675 [details] Patch for 4.7 cherry-picked from master See bug 13085.
There's a problem in the test script test_give_owner.sh: /bin/sh on sn-devel-144 is dash which has a XSI compliant echo builtin. That means echo will always interpret escape sequences in strings, eg $ /bin/sh -c 'echo -n "\a" | hexdump -C' 00000000 07 |.| There's no way to prevent the echo command from doing this. In the script test_give_owner.sh echo was used to print an ACE which will contain a DOMAIN\USER substring. If the first character of USER is a valid escape sequence, the resulting string was interpreted as an escape sequence: $ /bin/sh -c 'echo "FILESERVER\asn"' FILESERVERsn The only portable way to print a string without interpreting embedded escape sequences seems to be using printf: $ /bin/sh -c 'printf "%s\n" "FILESERVER\asn"' FILESERVER\asn
Created attachment 13689 [details] Patch for 4.6 backported from master Patch for 4.6 including the fix for the selftest regression.
Created attachment 13690 [details] Patch for 4.7 backported from master
Comment on attachment 13690 [details] Patch for 4.7 backported from master Patch for 4.7 including the fix for the selftest regression.
Karolin please push to 4.6.next, 4.7.next.
(In reply to Jeremy Allison from comment #37) Pushed to autobuild-v4-{7,6}-test.
(In reply to Karolin Seeger from comment #38) Pushed to both branches. Closing out bug report. Thanks!