Question about vfs_acl_common not setting filesystem permissions anymore

[Uri Simchoni]

On 08/24/2016 01:53 PM, Ralph Böhme wrote:
> Hi Uri,
> 
> I wonder whether this change
> 
>   765e5f1 vfs_acl_common: avoid setting POSIX ACLs if "ignore system acls" is set
> 
> is correct.
> 
> The kernel will still perform permissions checks, so even if the
> ACL-blob permission checks in se_file_access_check() grants access,
> the kernel checks can return EACCESS as the smbd session process runs
> with euid of the authenticated user.
> 
> How is this supposed to work? Maybe I'm missing something.
> 
> Simple example:
> 
> [share]
>     path = /data/share
>     vfs objects = acl_xattr
>     acl_xattr:ignore system acls = yes
> 
> $ ./bin/smbcacls -Uslow%x //localhost/share "dir"
> REVISION:1
> CONTROL:SR|DP
> OWNER:SLOWSERVER\slow
> GROUP:SLOWSERVER\None
> ACL:SLOWSERVER\slow:ALLOWED/OI|CI/FULL
> ACL:SLOWSERVER\fast:ALLOWED/OI|CI/FULL
> 
> $ ls -ld /data/share/dir/
> drwxr-xr-x. 3 slow slow 4096 Aug 24 11:42 /data/share/dir/
> 
> $ ./bin/smbclient -Ufast%x //localhost/share -c "put README dir/README"
> Domain=[SLOW] OS=[Windows 6.1] Server=[Samba 4.6.0pre1-DEVELOPERBUILD]
> NT_STATUS_ACCESS_DENIED opening remote file \dir/README
> 
> This fails even though the NT ACL grants access because the filesytem
> permissions don't. Change filesystem permissions at it works:
> 
> $ chmod 0777 /data/share/inherit_dir/
> $ ./bin/smbclient -Ufast%x //localhost/share -c "put README dir/README"
> Domain=[SLOW] OS=[Windows 6.1] Server=[Samba 4.6.0pre1-DEVELOPERBUILD]
> putting file README as \inherit_dir/README (8650.5 kb/s) (average 8651.4 kb/s)
> 
> The POSIX permissions for directories created by SMB clients are
> governed by "directory mask" which is 0755 by default. Maybe forcing
> "directory mask = 0777" and "create mask = 0777" in
> connect_acl_xattr() and connect_acl_tdb() would work, not
> sure. Otherwise I think we may have to revert this change.
> 
> Cheerio!
> -slow
> 

Yeah you're right - it works with a create mask of 0666 for files and
0777 for dirs. That's how we use it.

When submitting this change, I didn't get into the whole rationale -
just pointed out that the new behavior is consistent with the man page,
and the previous one isn't. If the man page said otherwise I might have
suggested another option or something. What really made me do it or need
it was that sometimes, and always due to lack of proper planning, we
would encounter permission issues because the id mapping has changed and
POSIX acls get in the way, and this in an env that couldn't care less
about POSIX permissions or ACLs. The workaround was to delete all POSIX
ACLs in the tree and fix file ownership and unix permissions.

Thanks,
Uri.