Behaviour mismatch between "store dos attributes" and "map archive" from man smb.conf(5)

[Michael Adam]

On 2020-05-18 at 11:59 -0700, Jeremy Allison wrote:
> On Mon, May 18, 2020 at 05:47:00PM +0530, Anoop C S wrote:
> > 
> > Seems like we have two different mapping methods.
> > 
> > When vfs_acl_xattr is used along with 'ignore system acls' we have
> > special kind of treatment as follows inside make_default_acl_posix():
> > 
> > if (mode & S_IRUSR) {
> >         if (mode & S_IWUSR) {
> >                 access_mask |= SEC_RIGHTS_FILE_ALL;
> >         } else {
> >                 access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
> >         }
> > }
> > if (mode & S_IWUSR) {
> >         access_mask |= SEC_RIGHTS_FILE_WRITE | SEC_STD_DELETE;
> > }
> > 
> > This might be the reason why smb2.read.position is always passing with
> > vfs_acl_xattr + "ignore system acls = yes".  
> 
> Yes, "ignore system acls = yes" special cases a lot of stuff.

Right, but as mentioned before, even without "ignore system
acls", the x-bit will be set on the file upon creation.
So even in that case, EXECUTE access rights are granted upon
opening again and the test passes.

> > Here comes the default mapping method, invoked all the way from
> > smbd_check_access_rights() we have the following in
> > map_canon_ace_perms():
> > 
> > if (directory_ace) {
> >         nt_mask |= ((perms & S_IRUSR) ? UNIX_DIRECTORY_ACCESS_R : 0 );
> >         nt_mask |= ((perms & S_IWUSR) ? UNIX_DIRECTORY_ACCESS_W : 0 );
> >         nt_mask |= ((perms & S_IXUSR) ? UNIX_DIRECTORY_ACCESS_X : 0 );
> > } else {
> >         nt_mask |= ((perms & S_IRUSR) ? UNIX_ACCESS_R : 0 );
> >         nt_mask |= ((perms & S_IWUSR) ? UNIX_ACCESS_W : 0 );
> >         nt_mask |= ((perms & S_IXUSR) ? UNIX_ACCESS_X : 0 );
> > }
> > 
> > Thus an existing file created without 'x' bit will end up with a
> > access_mask without SEC_FILE_EXECUTE.
> 
> An access_mask in the reported Windows ACL, yes.

But the main point is that is ONLY happening if the file on disk
has user +x perms! And this is different from the acl-xattr
behavior.
 
> > Difference in behaviour is because of different mapping methods
> > resulting in different access_mask when used with and without
> > vfs_acl_xattr. Why is it so? Why shouldn't vfs_acl_xattr just deal with
> > setting "security.NTACL" with a value obtained out of a common mapping
> > method?
> 
> It's probably so due to historical reasons.
> 
> Can you write up a comprehensive "how I think this
> should behave" and then we can discuss, make changes ?

We will.

I think the gist of it is this (with a grain of salt),
as far as I understand it:

- There are mappings from posix acls/perms to NT-acls and visa versa.
- Access checks in smbd are done at the nt-acl level.
- The mappings happens both with and without acl_xattr
  (and without ignore-system-acls).
- The main conceptual difference between the two modes is that in
  the acl-xattr case the nt acl gets stored to disk (and becomes
  the source of truth) while in the no-acl-xattr case it stays
  in memory only (and the posix acl remains the source of truth).

The *problem* I see is that in the two cases, *different* mapping
functions are used. I think we should use the *same* mapping
function in both cases to achieve a consistent behavior.
(And FWIW, I currently think the mapping function of the
acl-xattr case should be used, as this is the one tested in
selftest.)

It is understood that this might lead to behavior changes that
are unexpected for or unwanted by some, but we could still keep
the legacy behavior behind an option for a while.

We will follow up with a lot more detail, but I think this is the
crux of it.

Cheers - Michael


> 
> Thanks,
> 
> Jeremy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20200519/25b4c3dd/signature.sig>