Bug 9637 - Renaming directories as guest user in security share mode doesn't work.
Summary: Renaming directories as guest user in security share mode doesn't work.
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.6
Classification: Unclassified
Component: File services (show other bugs)
Version: 3.6.12
Hardware: All All
: P5 normal
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-06 13:03 UTC by Andreas Schneider
Modified: 2013-03-06 09:22 UTC (History)
0 users

See Also:


Attachments
v3-6-test patch (1.29 KB, patch)
2013-02-06 13:03 UTC, Andreas Schneider
vl: review+
jra: review-
Details
log.smbd (981.87 KB, text/plain)
2013-02-11 12:35 UTC, Andreas Schneider
no flags Details
log.smbd with smbclient -U% (449.13 KB, text/plain)
2013-02-27 07:55 UTC, Andreas Schneider
no flags Details
git-am fix for 3.6.next. (1.14 KB, patch)
2013-03-06 00:26 UTC, Jeremy Allison
no flags Details
git-am fix for 3.6.next. (1010 bytes, patch)
2013-03-06 00:35 UTC, Jeremy Allison
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schneider 2013-02-06 13:03:18 UTC
Renaming directories as guest user in security share mode doesn't work.

The following patch disables the acl permission check if we are connected as a guest and have a guest only share.
Comment 1 Andreas Schneider 2013-02-06 13:03:43 UTC
Created attachment 8532 [details]
v3-6-test patch
Comment 2 Volker Lendecke 2013-02-06 16:24:58 UTC
Comment on attachment 8532 [details]
v3-6-test patch

I think this patch is okay, so formally this can go in. As this is a security sensitive question, I would like to ask Jeremy for another review.

Karolin, please hold back this patch until Jeremy approves or nacks this patch.

Thanks,

Volker
Comment 3 Jeremy Allison 2013-02-07 18:18:41 UTC
I hate this patch, sorry. I might hate it less if you could answer the following question. *Why* is renaming a directory is being denied when you have guest access under security=hare. What exact bits are being denied and what is going on.

I would like to see a debug level 10 trace of this and maybe you can go through this with me to understand exactly what is going on. You can't just disable security checks like this on a specific operation.

Jeremy.
Comment 4 Andreas Schneider 2013-02-07 18:31:06 UTC
mkdir /srv/tmp/files
chown nobody:nobody /srv/tmp/files

vim /etc/samba/smb.conf

[global]
  security = share

[myshare]
  path = /srv/tmp/files
  writeable = yes
  guest ok = yes

smbd

Now, use e.g. WinXP to create a directory as guest. You can create it, but you're not allowed to rename it.

The reason is that the standard security descriptor is created for the guest user which denies renaming the directory.


Path the command takes:

create_file_default -> create_file_unixpath -> can_delete_file_in_directory -> can_access_file_acl -> se_access_check -> ACCESS_DENIED


security = share
=================

In can_access_file_acl() the call to SMB_VFS_GET_NT_ACL returns the standard ACL created with make_standard_sec_desc().

We enter se_access_check() with access_mask = 0x40


We loop on bits_remainingq


198             for (i=0; bits_remaining && i < sd->dacl->num_aces; i++) {
(gdb) n
199                     struct security_ace *ace = &sd->dacl->aces[i];
(gdb) p i
$8 = 0
(gdb) n
201                     if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) {
(gdb) n
205                     if (!security_token_has_sid(token, &ace->trustee)) {
(gdb) n
206                             continue;
(gdb) n
198             for (i=0; bits_remaining && i < sd->dacl->num_aces; i++) {
(gdb) n
199                     struct security_ace *ace = &sd->dacl->aces[i];
(gdb) n
201                     if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) {
(gdb) n
205                     if (!security_token_has_sid(token, &ace->trustee)) {
(gdb) n
209                     switch (ace->type) {
(gdb) n
211                             bits_remaining &= ~ace->access_mask;
(gdb) p/x ace->access_mask
$9 = 0x1200a9
(gdb) n
212                             break;
(gdb) p bits_remaining
$10 = 64
(gdb) p/x bits_remaining
$11 = 0x40


bits_remaining is still 0x40 so later we ACCESS_DENIED later.

(gdb) n
253             if (bits_remaining != 0) {
(gdb) n
254                     *access_granted = bits_remaining;
(gdb) n
255                     return NT_STATUS_ACCESS_DENIED;


security = user
================

se_access_check() with access_mask = 0x40

On the first loop at:

for (i=0; bits_remaining && i < sd->dacl->num_aces; i++)

switch (ace->type) {                                                                                                                                                
case SEC_ACE_TYPE_ACCESS_ALLOWED:                                                                                                                                   
   bits_remaining &= ~ace->access_mask;

Sets bits_remaining to 0 and everything is fine.
Comment 5 Andreas Schneider 2013-02-07 18:33:01 UTC
Oh, this worked just fine in 3.5. So everyone upgrading with this configuration from 3.5 to 3.6 runs into the issue.
Comment 6 Volker Lendecke 2013-02-07 21:05:25 UTC
The problem is that we don't have any kind of reasonable token at all with security=share and guest access.
Comment 7 Jeremy Allison 2013-02-07 22:08:36 UTC
Yeah, I'll reproduce and mess with it. Having Everyone in the token and 'w' access in the 'other' field should be enough.

Jeremy.
Comment 8 Jeremy Allison 2013-02-07 22:51:23 UTC
Can't reproduce with smbclient. I have:

[global]
   workgroup = WORKGROUP
   max protocol = SMB2
   log file = /usr/local/samba3.6/var/log.%m.%d
   max log size = 0
   panic action = /bin/sleep 999999
   security = share
   map hidden = no
   map system = no
   map archive = no
   map readonly = no
   store dos attributes = yes
   dos filemode = yes

[tmp]
        path = /tmp
        read only = no
        guest ok = yes

Then I do:

bin/smbclient //127.0.0.1/tmp -U%

smb: \> mkdir foo
smb: \> ren foo bar
smb: \> ls bar
  bar                                 D        0  Thu Feb  7 14:51:12 2013

		38060 blocks of size 1048576. 21302 blocks available
smb: \> 

So this is *NOT* a generic problem in the underlying code - smb.conf can be set to allow this. You need to explain exactly why it doesn't work *for you*.

Jeremy.
Comment 9 Jeremy Allison 2013-02-07 22:52:15 UTC
Just to show it's owned by guest:

$ ls -ld /tmp/bar
drwxr-xr-x 2 nobody nogroup 4096 Feb  7 14:51 /tmp/bar
Comment 10 Andreas Schneider 2013-02-08 09:16:04 UTC
Jeremy you are working around the problem. You use /tmp as a share which has 0777 as permission and the 'other' write access allows you to do it.

magrathea:~ # cat /etc/samba/smb.conf
[global]
        workgroup = DISCWORLD
        security = share
        log level = 10
        debug pid = yes

[guru]
        path = /srv/samba/tmp
        read only = no
        guest ok = yes

smbclient //127.0.0.1/guru -U%
WARNING: The security=share option is deprecated
smb: \> mkdir jra
smb: \> rename jra jeremy
NT_STATUS_ACCESS_DENIED renaming files \jra -> \jeremy

If I do a:

chmod 0777 /srv/samba/tmp

then the operation succeeds ...
Comment 11 Jeremy Allison 2013-02-08 16:54:14 UTC
Then I don't see the problem - smbd is working as designed. You need write access into the parent directory in order to rename anything inside it.

So if you're coming in a guest, then whatever containing directory must have w access for "other" in order for a rename to succeed. The "guest" user is the very definition of "other".

If it allowed renames in 3.5.x that's more of a 3.5.x bug than 3.6.x. 3.6.x is doing the right thing here.

If you want a dropbox, just set the 't' bit on the share directory to prevent people from overwriting each others file - just like the /tmp directory does.

Jeremy.
Comment 12 Jeremy Allison 2013-02-08 18:39:29 UTC
Just as a followup, if you want your specific 'guest' user to be able to rename/write into a share directory, then you'll have to add that specific user as a group member of the group owning the containing directory, and make sure the group bits have 'w' set.
Comment 13 Andreas Schneider 2013-02-09 09:38:26 UTC
What I don't understand here is, why am I allowed to create the directory as guest but can not rename it?

Why does it work with 'security = user'.

Can you please explain that?
Comment 14 Jeremy Allison 2013-02-09 16:50:45 UTC
I can't explain it in your case as you declined my request to upload a debug level 10 trace of what was happening on your box, and instead pasted your interpretation of what was going on.

In order to explain exactly what is happening on your box I would need to see a complete debug level 10 trace.

As far as I can see the code is working exactly as designed in my tests.

Jeremy.
Comment 15 Andreas Schneider 2013-02-11 12:35:43 UTC
Created attachment 8540 [details]
log.smbd
Comment 16 Jeremy Allison 2013-02-20 23:21:10 UTC
You have a user mapping problem.

The ACL on the file/directory gives full access to user:

S-1-5-21-1406987565-2067085585-2387977275-501

who is also set as the owner. That owner is "nobody".

However, the token associated with the logged on user is:

  Security token SIDs (6):
    SID[  0]: S-1-22-1-65534
    SID[  1]: S-1-22-2-65533
    SID[  2]: S-1-22-2-65534
    SID[  3]: S-1-1-0
    SID[  4]: S-1-5-2
    SID[  5]: S-1-5-32-546
   Privileges (0x               0):
   Rights (0x               0):
[2013/02/11 13:34:55.610984,  5, pid=12333] auth/token_util.c:527(debug_unix_user_token)
  UNIX token of user 65534
  Primary group is 65533 and contains 2 supplementary groups
  Group[  0]: 65533
  Group[  1]: 65534

which is why you're getting an ACCESS_DENIED error.

However, earlier in the log we see a token of:

  Security token SIDs (6):
    SID[  0]: S-1-5-21-1406987565-2067085585-2387977275-501
    SID[  1]: S-1-5-21-1406987565-2067085585-2387977275-514
    SID[  2]: S-1-1-0
    SID[  3]: S-1-5-2
    SID[  4]: S-1-5-32-546
    SID[  5]: S-1-22-1-65534
   Privileges (0x               0):
   Rights (0x               0):
[2013/02/11 13:34:47.299290, 10, pid=12314] auth/token_util.c:527(debug_unix_user_token)
  UNIX token of user 65534
  Primary group is 65533 and contains 0 supplementary groups

being created for user "nobody". We can see this from this part of the log:

  pdb_getsampwsid: Building guest account
[2013/02/11 13:34:47.296233,  5, pid=12314] lib/username.c:171(Get_Pwnam_alloc)
  Finding user nobody
[2013/02/11 13:34:47.296259,  5, pid=12314] lib/username.c:116(Get_Pwnam_internals)
  Trying _Get_Pwnam(), username as lowercase is nobody
[2013/02/11 13:34:47.296285,  5, pid=12314] lib/username.c:149(Get_Pwnam_internals)
  Get_Pwnam_internals did find user [nobody]!
[2013/02/11 13:34:47.296313, 10, pid=12314] passdb/pdb_get_set.c:575(pdb_set_username)
  pdb_set_username: setting username nobody, was 
[2013/02/11 13:34:47.296344, 10, pid=12314] passdb/pdb_get_set.c:644(pdb_set_fullname)
  pdb_set_full_name: setting full name nobody, was 
[2013/02/11 13:34:47.296370, 10, pid=12314] passdb/pdb_get_set.c:598(pdb_set_domain)
  pdb_set_domain: setting domain MAGRATHEA, was 
[2013/02/11 13:34:47.296399, 10, pid=12314] passdb/pdb_get_set.c:500(pdb_set_user_sid)
  pdb_set_user_sid: setting user sid S-1-5-21-1406987565-2067085585-2387977275-501
[2013/02/11 13:34:47.296429, 10, pid=12314] passdb/pdb_compat.c:73(pdb_set_user_sid_from_rid)
  pdb_set_user_sid_from_rid:
  	setting user sid S-1-5-21-1406987565-2067085585-2387977275-501 from rid 501
[2013/02/11 13:34:47.296473,  4, pid=12314] smbd/sec_ctx.c:422(pop_sec_ctx)
  pop_sec_ctx (0, 0) - sec_ctx_stack_ndx = 1
[2013/02/11 13:34:47.296502,  5, pid=12314] lib/username.c:171(Get_Pwnam_alloc)
  Finding user nobody
[2013/02/11 13:34:47.296528,  5, pid=12314] lib/username.c:116(Get_Pwnam_internals)
  Trying _Get_Pwnam(), username as lowercase is nobody
[2013/02/11 13:34:47.296554,  5, pid=12314] lib/username.c:149(Get_Pwnam_internals)
  Get_Pwnam_internals did find user [nobody]!

Ah. Looking at your log it seems you're attempting to log on with user asn, and then getting mapped to guest - not attempting to log on with user credentials of -U%, which truly is "nobody". I was using smbclient -U%, you are not.

Your log shows:

[2013/02/11 13:34:50.410779,  3, pid=12326] smbd/sesssetup.c:1552(reply_sesssetup_and_X)
  sesssetupX:name=[WINXP]\[asn]@[192.168.52.63]
[2013/02/11 13:34:50.410822,  6, pid=12326] param/loadparm.c:7490(lp_file_list_changed)
  lp_file_list_changed()
  file /etc/samba/smb.conf -> /etc/samba/smb.conf  last mod_time: Mon Feb 11 13:23:18 2013
  
[2013/02/11 13:34:50.410888,  5, pid=12326] lib/username.c:171(Get_Pwnam_alloc)
  Finding user asn
[2013/02/11 13:34:50.410913,  5, pid=12326] lib/username.c:116(Get_Pwnam_internals)
  Trying _Get_Pwnam(), username as lowercase is asn
[2013/02/11 13:34:50.411040,  5, pid=12326] lib/username.c:149(Get_Pwnam_internals)
  Get_Pwnam_internals did find user [asn]!

Then...

  Finding user nobody
[2013/02/11 13:34:50.432966,  5, pid=12326] lib/username.c:116(Get_Pwnam_internals)
  Trying _Get_Pwnam(), username as lowercase is nobody
[2013/02/11 13:34:50.432993,  5, pid=12326] lib/username.c:149(Get_Pwnam_internals)
  Get_Pwnam_internals did find user [nobody]!
[2013/02/11 13:34:50.433019,  3, pid=12326] smbd/password.c:721(authorise_login)
  authorise_login: ACCEPTED: guest account and guest ok (nobody)
[2013/02/11 13:34:50.433053,  5, pid=12326] lib/username.c:171(Get_Pwnam_alloc)
  Finding user nobody
[2013/02/11 13:34:50.433078,  5, pid=12326] lib/username.c:116(Get_Pwnam_internals)
  Trying _Get_Pwnam(), username as lowercase is nobody
[2013/02/11 13:34:50.433105,  5, pid=12326] lib/username.c:149(Get_Pwnam_internals)
  Get_Pwnam_internals did find user [nobody]!
[2013/02/11 13:34:50.433135, 10, pid=12326] passdb/lookup_sid.c:76(lookup_name)
  lookup_name: Unix User\nobody => domain=[Unix User], name=[nobody]

which is different from user "nobody", so no mapping for the UNIX uids is being done as is done when the token for guest is created.

So smbd sees the file as being owned by "nobody" == S-1-5-21-1406987565-2067085585-2387977275-501

whereas you are logged in as "Unix User\nobody" == S-1-22-1-65534, which is not the same as the guest user. If we can fix this, then that will solve the problem.

As a first step, try again using credentials of -U%, not 'asn' and see if that starts to work.

Jeremy.
Comment 17 Jeremy Allison 2013-02-21 23:27:52 UTC
Can you post a debug level 10 log of the operation you show in comment #10 ? That would be a much simpler log to parse and see if we can create an easy fix. You posted a debug level 10 log from the Windows XP attempt, obviously.

Jeremy.
Comment 18 Andreas Schneider 2013-02-26 09:30:32 UTC
Yes, sorry. Was busy with winbind fixes. I will do it today.
Comment 19 Andreas Schneider 2013-02-27 07:55:47 UTC
Created attachment 8591 [details]
log.smbd with smbclient -U%

$ smbclient //localhost/guru -U%
WARNING: The security=share option is deprecated
Domain=[DISCWORLD] OS=[Unix] Server=[Samba 3.6.13-GIT-b76501d-test]
Server not using user level security and no password supplied.
smb: \> mkdir wurst
smb: \> rename wurst blutwurst
NT_STATUS_ACCESS_DENIED renaming files \wurst -> \blutwurst
smb: \>
Comment 20 Jeremy Allison 2013-03-06 00:26:42 UTC
Created attachment 8609 [details]
git-am fix for 3.6.next.

Andreas can you test this fix ? I think it's the correct one to ensure guest is always treated consistently, but I can't reproduce your case so you'll have to review for me :-).

Thanks !

Jeremy.
Comment 21 Jeremy Allison 2013-03-06 00:35:02 UTC
Created attachment 8610 [details]
git-am fix for 3.6.next.

Doh! Do it right (correctly check status).

Jeremy.
Comment 22 Andreas Schneider 2013-03-06 08:52:17 UTC
Comment on attachment 8610 [details]
git-am fix for 3.6.next.

Tested and works for me. Thanks Jeremy!
Comment 23 Andreas Schneider 2013-03-06 08:52:44 UTC
Karolin, please add the patch to the next 3.6 release. Thanks!
Comment 24 Karolin Seeger 2013-03-06 09:22:01 UTC
Pushed to v3-6-test.
Closing out bug report.

Thanks!