[PATCH] Two attempts in fixing default ACL in vfs_acl_xattr

Jeremy Allison jra at samba.org
Fri Jul 15 20:10:45 UTC 2016


On Fri, Jul 15, 2016 at 06:46:20PM +0200, Ralph Boehme wrote:
> Hi!
> 
> For files or directories without an ACL xattr we build an ACL from the
> filesytem mode including user, group and other ACE, plus an ACE for
> system.
> 
> This can lead to unexpected ACLs and doesn't match Windows behaviour,
> eg:
> 
> - a directory with the following ACL:
> 
> $ ./bin/smbcacls -Uslow%pass //localhost/normal ""
> REVISION:1
> CONTROL:SR|DP
> OWNER:SLOW\slow
> GROUP:Unix Group\root
> ACL:SLOW\slow:ALLOWED/0x0/FULL
> 
> So only one non-inheritable(!) ACE.
> 
> - creating a subdirectory:
> 
> $ ./bin/smbclient -Uslow%pass //localhost/normal -c "mkdir dir1"
> 
> - checking whether there's an ACL xattr:
> 
> $ getfattr -m "" /Volumes/normal/dir1
> getfattr: Removing leading '/' from absolute path names
> system.posix_acl_access
> system.posix_acl_default
> user.DOSATTRIB
> 
> So there isn't an ACL xattr, because there where no inheritable ACEs
> on the parent folder.
> 
> - reading the new subdirectories ACL:
> 
> $ ./bin/smbcacls -Uslow%pass //localhost/normal "dir1"
> REVISION:1
> CONTROL:SR|DP
> OWNER:SLOW\slow
> GROUP:Unix Group\slow
> ACL:SLOW\slow:ALLOWED/0x0/FULL
> ACL:Unix Group\slow:ALLOWED/0x0/READ
> ACL:Everyone:ALLOWED/0x0/READ
> ACL:NT Authority\SYSTEM:ALLOWED/0x0/FULL
> 
> The ACES for "SLOW\slow", "Unix Group\slow" and "Everyone" are coming
> from make_default_filesystem_acl(). This is the problem.
> 
> - Windows assigns the following ACL in this situation:
> 
> $ ./bin/smbcacls -UAdministrator%Passw0rd //10.10.10.14/data "dir"
> REVISION:1
> CONTROL:SR|PD|DI|DP
> OWNER:VORDEFINIERT\Administratoren
> GROUP:WIN2008R2\Domänen-Benutzer
> ACL:WIN2008R2\Administrator:ALLOWED/0x0/FULL
> 
> $ ./bin/smbclient -UAdministrator%Passw0rd //10.10.10.14/data -c "mkdir dir\dir1"
> 
> $ ./bin/smbcacls -UAdministrator%Passw0rd //10.10.10.14/data "dir\dir1"
> REVISION:1
> CONTROL:SR|DI|DP
> OWNER:VORDEFINIERT\Administratoren
> GROUP:WIN2008R2\Domänen-Benutzer
> ACL:VORDEFINIERT\Administratoren:ALLOWED/0x0/FULL
> ACL:NT-AUTORITÄT\SYSTEM:ALLOWED/0x0/FULL
> 
> By changing make_default_filesystem_acl() to only adds user and system
> ACE to the ACL of objects that lack an ACL xattr, we match Windows
> behaviour:
> 
> $ ./bin/smbclient -Uslow%pass //localhost/normal -c "mkdir dir2"
> 
> $ ./bin/smbcacls -Uslow%pass //localhost/normal "dir2"
> REVISION:1
> CONTROL:SR|DP
> OWNER:SLOW\slow
> GROUP:Unix Group\slow
> ACL:SLOW\slow:ALLOWED/0x0/FULL
> ACL:NT Authority\SYSTEM:ALLOWED/0x0/FULL
> 
> You can get the desired behaviour by setting all of the following
> parameters:
> 
>   acl_xattr:ignore system acls = yes
>   directory mask = 0700
>   create mask = 0600
> 
> but this is rather unobvious.
> 
> I have a patch (acl-default.patch) that fixes
> make_default_filesystem_acl() to only return user and system ACE.
> 
> For completeness, I've attached another patch (acl-mask.patch) that
> automatically sets the last two parameters in case "ignore system
> acls" is enabled when the acl_xattr module is loaded. This was the
> first attempt to fix this and is probably not what we want.
> 
> Review and comments appreciated. If ok, please don't push yet, I'd
> like to create a bugreport first and add the bug link to the commit
> messages.

Really interesting problem and good solution. Short answer,
for the proposed patchset - Reviewed-by: Jeremy Allison <jra at samba.org>.

The second (hack) patch is too ugly to live :-).

Longer answer, the ACL return here comes from Samba's original
role as a method of exposing the underlying POSIX semantics
of the filesystem to a Windows client.

If that's the intention, the current behavior is correct as
it shows the user exactly what the 'real' underlying POSIX
permissions are on the filesystem (even the addition of the
SYSTEM:FULL_ACCESS ace representing root :-).

For previous versions of Samba that didn't check Windows ACLs
in userspace the existing behavior is correct, as it shows
the client what will happen if they try and do an operation.

However over the years our role has become different, and we're
designing our behavior much more around complete seamless emulation
of a Windows fileserver - checking the stored Windows ACLs
in userspace which can provide additional restrictions on
top of the underlying file system permissions.

Given that premise, your change is completely correct and
fixes unexpected behavior.

There's only once subtle case we need to think about - and
that is the original behavior of a fileserver that wants
to represent and expose underlying filesystem permissions
to the client, and expects all permission changes to be
expressed as ACLs that will map into POSIX-style ACLs.

However for that case I think we're covered in that we
just tell users not to load the acl_xattr module and
they should get the desired behavior.

Long winded way of saying LGTM. Please log the bug
(with your excellent explaination) and push it with
the bug link in the commit message !

Cheers,

	Jeremy.



More information about the samba-technical mailing list