Description of problem: This bug is confirmed to exist in 3.6.x and 3.5.10. It worked correctly in 3.5.4 and is fixed in 4.0. If you set a default ACL for a group to be rwx and create the directory with mkdir on a shell the group is granted rwx. If you do the same but connect to a samba share the effective rights will be r-x. Cause the mask is not rwx it is r-x. How to reproduce: magrathea:~ # mkdir /tmp/samba magrathea:~ # getfacl /tmp/samba getfacl: Removing leading '/' from absolute path names # file: tmp/samba # owner: root # group: root user::rwx group::r-x other::r-x magrathea:~ # setfacl -m u:bob:rwx /tmp/samba/ magrathea:~ # setfacl -d -m u:bob:rwx /tmp/samba/ magrathea:~ # setfacl -m g:users:rwx /tmp/samba/ magrathea:~ # setfacl -d -m g:users:rwx /tmp/samba/ magrathea:~ # getfacl /tmp/samba/ getfacl: Removing leading '/' from absolute path names # file: tmp/samba/ # owner: root # group: root user::rwx user:bob:rwx group::r-x group:users:rwx mask::rwx other::r-x default:user::rwx default:user:bob:rwx default:group::r-x default:group:users:rwx default:mask::rwx default:other::r-x Switch to user bob: magrathea:~ # su - bob bob@magrathea:~> mkdir /tmp/samba/bob_creates_dir_with_unix bob@magrathea:~> getfacl /tmp/samba/bob_creates_dir_with_unix getfacl: Removing leading '/' from absolute path names # file: tmp/samba/bob_creates_dir_with_unix # owner: bob # group: users user::rwx user:bob:rwx group::r-x group:users:rwx mask::rwx other::r-x default:user::rwx default:user:bob:rwx default:group::r-x default:group:users:rwx default:mask::rwx default:other::r-x Now lets do the same with Samba: smb.conf -> http://xor.cryptomilk.org/samba/smb.conf bob@magrathea:~> smbclient //localhost/guru -Ubob%secret -c "mkdir bob_creates_dir_with_samba" Domain=[DISCWORLD] OS=[Unix] Server=[Samba 3.6.8-GIT-64c7d3e-test] bob@magrathea:~> getfacl /tmp/samba/bob_creates_dir_with_samba/ getfacl: Removing leading '/' from absolute path names # file: tmp/samba/bob_creates_dir_with_samba/ # owner: bob # group: users user::rwx user:bob:rwx #effective:r-x group::r-x group:users:rwx #effective:r-x mask::r-x other::r-x default:user::rwx default:user:bob:rwx default:group::r-x default:group:users:rwx default:mask::rwx default:other::r-x
Confirmed reproduced with 3.6.current. Marking as regression to make sure I fix this before next release. Jeremy.
Ok, this is a config error, not a bug (yeah, I know this is complex and sucks). It's a side effect of the fix for bug #7734. The problem in #7734 was that we were not applying the create masks to files created with "inherit acls". Check out the smb.conf definitions for : create mask directory mask These specify masks that are applied to *ALL* files or directories created by smbd through a Windows (non-unix extensions) connection. People take it on trust that any file or directory created by Samba obeys these masks - they allow those masks to be set on a share definition, and the admin *knows* that no files will violate those masks. So the earlier 3.5.x we considered a bug. You could argue whether "inherit acls" should take precedence over "create mask" or "directory mask" but we decided to make the mask parameters take precedence. So to fix this you need to set: create mask = 0777 directory mask = 0777 in the share definition of your smb.conf and then the inherited ACL will behave as you expect. Now I need to check into the 4.0.0rc1 code and see why it's *not* doing the same :-). Cheers, Jeremy.
Actually, thinking about it some more I think we also apply these masks to any POSIX (unix extensions) create files or directories as well - so they really are an absolute mask on anything Samba creates.
Ok, I've figured out why it works on 4.0.0rc and above. We do an extra step in 4.0, as we only deal with the NT ACL in the core fileserver code now - the POSIX ACL layer is hidden below the VFS. So rather than doing the NT ACL inheritance when creating a new object inside the Windows ACL vfs (i.e. the acl_xattr or acl_tdb layer) we create the file (which leaves the mask r-x as expected) - but then we do an additional Windows ACL set *afterwards*. The Windows ACL set here doesn't look at the create mask or directory mask parameters as it's considered modifying the ACL on an already existing file. The problem is as the create file/directory isn't atomic, we don't have a way to tell the NT -> POSIX mapping layer to treat this as an ACL set on file create, rather than an ACL modify (which uses the "[directory] security mask" parameter instead). Oh. I've thought of a really ugly way to fix this... Inside inherit_new_acl() we save off the original "[directory] security mask" settings and replace them with the settings of "create mask" and "directory mask" before calling the SMB_VFS_FSET_NT_ACL() call... Patch to follow. Jeremy.
Ok, I'm moving this to a 4.0.0rc1 bug. We're actually not enforcing the correct masks on the ACL set. Patch to follow. Jeremy.
There's also a subsidiary bug in that in 3.5.x and 3.6.x we don't apply the security masks to the SMB_ACL_USER or SMB_ACL_GROUP entries in the POSIX ACL when setting it. I'll upload separate patches for that. Jeremy.
Thank you very much Jeremy for looking into this. Could you please open another bug for the 3.6 and 3.5 security mask fixes and keep me in the loop? Thanks!
Created attachment 7979 [details] git-am fix for 4.0.0rc3 Here is the patchset that went into master. It's broken down into micro-patches so you can see what is going on. With this fix the create mask/directory mask will be applied to all files/directories created by smbd. Jeremy.
(In reply to comment #7) > Thank you very much Jeremy for looking into this. Could you please open another > bug for the 3.6 and 3.5 security mask fixes and keep me in the loop? > > Thanks! Done: https://bugzilla.samba.org/show_bug.cgi?id=9236 Jeremy.
Comment on attachment 7979 [details] git-am fix for 4.0.0rc3 Remove review - goal is to remove these parameters for 4.0.0rc3.
Created attachment 7991 [details] git-am fix for 4.0.0rc3 Removes : security mask force security mode directory mask force directory security mode parameters.
Created attachment 7992 [details] git-am fix for 4.0.0rc3 Got list of removed parameters right in commit message :-). Added update to WHATSNEW.txt.
Created attachment 7993 [details] git-am fix for 4.0.0rc3 Ho hum, make sure the masks are set right for the acl make test also.
Created attachment 8033 [details] Documentation fixes on top of attachment 7993 [details]
Comment on attachment 7993 [details] git-am fix for 4.0.0rc3 This looks fine for me, but I would like to have another reviewer giving + too.
Thanks. I'll wait until David or Metze give it their blessing. Jeremy.
Comment on attachment 7993 [details] git-am fix for 4.0.0rc3 Looks ok
Karolin, please pick this and review the docs fix
Pushed to autobuild-v4-0-test. Closing out bug report. Thanks!