Question about vfs_acl_common not setting filesystem permissions anymore

[Jeremy Allison]

On Wed, Aug 24, 2016 at 12:53:15PM +0200, 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.

IMHO setting "acl_xattr:ignore system acls = yes" essentially
means that the underlying file system isn't really POSIX,
or isn't prohibiting access in any way (after all, why set
"ignore system acls = yes" if you don't want to ignore
system acls ? :-).

It really is an unusual use-case mainly for OEMs who
should be setting this up correctly for their users.

So I don't think reverting is the right thing to do here.

I'm OK with changing "directory mask" and "create mask" to
make this work on an underlying POSIX system, and maybe
we need to update the man page to make it explicit that
the admin has to ensure this really is running on a system
that ignores any system acls.