Question about vfs_acl_common not setting filesystem permissions anymore

[Ralph Böhme]

On Wed, Aug 24, 2016 at 11:51:14AM -0700, Jeremy Allison wrote:
> 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 ? :-).

I take this slightly differently: "acl_xattr:ignore system acls = yes"
is the most sensible setting if there's no need for cross-protocol
support. Most of the times this will be using a POSIX filesystem.

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

Hm, I guess this would be the usual setup I'd do on a typical
fileserver without the need for cross protocol support. :)

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

Agreed.

> 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.

Ok, I'll prepare patches. Thanks for chiming in!

Cheerio!
-slow