The vfs_acl_xattr module in Samba 3.6.6 seems to disregard the Sgid bit of directories. The corresponding group is not inherited to files created in the directory via smbclient (nor is the Sgid bit inherited by directories). In contrast default fACLs for a group are inherited properly. Without the vfs_acl_xattr the Sgid bit is honored. The workaround from Bug 7996 did not change the behaviour. ### ======================= ## example share definition [vfstest] path = /vfstest vfs objects = acl_xattr msdfs root = no writeable = yes browseable = yes public = no dos filemode = no hide unreadable = no create mode = 0744 directory mode = 0755 force create mode = 0 force directory mode = 0 security mask = 0777 directory security mask = 0777 force security mode = 0 force directory security mode = 0 locking = 1 blocking locks = 1 strict locking = 0 oplocks = 1 level2 oplocks = 1 fake oplocks = 0 csc policy = manual nt acl support = 1 inherit acls = 1 inherit owner = no inherit permissions = yes ### ======================= root@master:~# mkdir /vfstest/folder1 root@master:~# chgrp 'Domain Users' /vfstest/folder1/ root@master:~# chmod 2770 /vfstest/folder1/ root@master:~# cd /home/Administrator root@master:~# smbclient //$(hostname)/vfstest -U Administrator \ -c 'cd folder1; put /etc/hosts file1' root@master:~# ls -ld /vfstest/folder1/file1 -rwxr-xr-x 1 Administrator Domain Admins 809 19. Jul 10:58 /vfstest/folder1/file1 root@master:~# getfacl /vfstest/folder1/file1 # file: vfstest/folder1/file1 # owner: Administrator # group: Domain\040Admins user::rwx group::r-x other::r-x root@master:~# getfattr -m '' -d /vfstest/folder1/file1 # file: vfstest/folder1/file1 security.NTACL=0sAwADAAAAAgAEAAIAAQD2GH5yPAGmXJFnegWNEWUFjTLdUd8ClSVst+HRsYZidAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAASAZAAAAIAAAAAAAAAAnAAAAAEFAAAAAAAFFQAAAEIvYDOj1mthqGnm7PQBAAABBQAAAAAABRUAAABCL2Azo9ZrYahp5uwAAgAAAgBkAAMAAAAAACQA/wEfAAEFAAAAAAAFFQAAAEIvYDOj1mthqGnm7PQBAAAAACQAqQASAAEFAAAAAAAFFQAAAEIvYDOj1mthqGnm7AACAAAAABQAqQASAAEBAAAAAAABAAAAAA==
Created attachment 7940 [details] Patch proposal This patch for the inherit_new_acl Funktion in vfs_acl_common.c adds a check for the SGID bit on the parent folder. If it is found, the group_sid is inherited from the parent.
Created attachment 7941 [details] Patch proposal for samba4 git In current Samba4 git the function moved to source3/smbd/open.c
Created attachment 7942 [details] Patch proposal for samba4 git (v 0.2)
(In reply to comment #3) > Created attachment 7942 [details] > Patch proposal for samba4 git (v 0.2) At first glance, the patch seems reasonable (TODOs aside), but as I dig into it some more, I have concerns. The desire to have sgid means something even when we have full NT ACL semantics is a good one, but if we are to do that, perhaps we should directly map the owning group from the directory back into a SID, rather than assume that the SID matches in the NT ACL? There are reasons, such as either an ACL being set with the ntvfs file server or the SID <-> GID mapping not being one-to-one that these may not match up directly. I'm not sure if these matter in the real world. In general, I'm also a little concerned about possible races here - ACL inheritance in user-space is a dodgy thing at the best time, much like ACL evaluation. How would you propose this interacts with 'inheirt owner', as it seems that inherit owner overrides this?
One other thing: What is the proposed interaction between this and a stored NT ACL? The reason I ask is that I'm proposing to change vfs_acl_xattr to hash an unmapped POSIX ACL (and file owner/mode). That would mean that setting an SGID on a directory would invalidate the NT ACL (falling back to a mapped POSIX ACL), as it indicates that a change was made on the POSIX side. Would this match the proposed use case?
> The desire to have sgid means something even when we have full NT ACL semantics > is a good one, but if we are to do that, perhaps we should directly map the > owning group from the directory back into a SID, rather than assume that the > SID matches in the NT ACL? > > There are reasons, such as either an ACL being set with the ntvfs file server > or the SID <-> GID mapping not being one-to-one that these may not match up > directly. I'm not sure if these matter in the real world. Maybe, this is true. I simply followed the coding pattern of "inherit owner". Maybe that would have to be fixed then as well. If we would lookup the GID from the parent directory and map the SID again, then we would have to update it in parent_desc->group_sid for se_create_child_secdesc as well? I would have guessed that the parent_desc->group_sid is authoritative. At least that's what is used for calculation of the security descrption in se_create_child_secdesc a few lines later. > In general, I'm also a little concerned about possible races here - ACL > inheritance in user-space is a dodgy thing at the best time, much like ACL > evaluation. I agree, but this is a general problem pertatining also to the inheritance of pure NTACLs? > How would you propose this interacts with 'inheirt owner', as it seems that > inherit owner overrides this? Yes, I would follow the pattern of samba options beeing authoritative.
> One other thing: What is the proposed interaction between this and a stored > NTACL? > > The reason I ask is that I'm proposing to change vfs_acl_xattr to hash an > unmapped POSIX ACL (and file owner/mode). That would mean that setting an SGID > on a directory would invalidate the NT ACL (falling back to a mapped POSIX > ACL), as it indicates that a change was made on the POSIX side. Would this > match the proposed use case? inherit_new_acl finally calls SMB_VFS_FSET_NT_ACL and the implementation fset_nt_acl_common updates the hash. So I think the proposed approach to this bug would play nicely with this? Since inherit_new_acl currently is only called by create_file_unixpath (or create_file_acl_common in samba 3.6.6), parent-SGID would only be considered during file creation.
I was more thinking about how we get the sgid there in the first place, that is by changing the posix permission on the directory.
*** Bug 8938 has been marked as a duplicate of this bug. ***