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

[Michael Adam]

On 2020-05-14 at 10:45 -0700, Jeremy Allison via samba-technical wrote:
> On Thu, May 14, 2020 at 09:59:04AM +0200, Michael Adam wrote:
> > 
> > To understand, I traced it through. The desired_access is handed
> > through from torture_smb2_testfile to access_mask in
> > open_file_ntcreate, where it is treated separately, and there,
> > unx_mod and access_mask are handed to open_file separately, and
> > not mixed up.
> > 
> > And the initial unix mode that is set does not seem to contain
> > the X bit unless the "map archive" is set.
> 
> Yep, that's the difference in behavior from the man page.

Hmm. That's not really what I meant. I am just trying to
understand the logic of the setting of this initial mode in
general.

> See my explaination and (completely untested) patch :-).
> 
> > But the question is still, why does the smb2.read test pass
> > and not fail at source4/torture/smb2/read.c:270 if I configure
> > "map archive = no" in selftest/target/Samba3.pm:2210 ?
> > In this case (i.e. acl_xattr enabled and "map archive = no"),
> > the file st/nt4_dc/share/smb2_readtest.dat gets +x bits set.
> 
> It's probably the "inherit acls" parameter that the acl_xattr
> module sets by default.

If I disable acl_xattr but set "inherit acls = yes", this still
fails the same way. Only acl_xattr sets the execute perms.
So at least it is not "inherit acls" alone.

> If you single step it I think you'll
> find that after the initial open(name, O_CREAT, 0644) call
> the file on disk has 0644 mode, then the code called in
> source3/smbd/open.c:inherit_new_acl() modifies it to
> add in the X bit, or whatever depending on the synthesized
> NT ACL on the parent directory.
> 
> > If I add "acl_xattr:ignore system acls = true", then the file
> > does NOT get the +x bits, but the test still passes.
> > 
> > If I disable the acl_xattr module though, and keep "map archive =
> > no", then the file gets no +x bits, and the check fails.
> 
> This is probably a test written to confirm we don't change pre-existing
> behavior. So if you want different behavior then fix the
> behavior,

At this point I'm mostly trying to understand.
I still have the impression that the behavior is somewhat
inconsistent and random.
And it seems to me that the test was not written to pass against
exactly the configuration of the tmp share in the test env, with
the awareness that it fails against other standard configurations.
I can't help the impression that this is somewhat accidential.

> mark the test as knownfail, then fix the test to
> match the new behavior and unmark the knownfail.

> > So apparently, while you say that this access mode is for the
> > access mode on the fd, not on the disk-file, when opening an
> > *existing* file, the executable permission of the disk-file is
> > still consulted, in form of the ACL, for determining
> > whether the desired access can be granted to the FD. This seems
> > somewhat asymmetric. But maybe I am not understanding correctly...
> 
> See above. Access mode is on the handle, it's nothing
> to do with the ACL on the file.

That is apparently only true for creating new files.  For opening
existing files however, the desired access mode seems to have a
lot to with the ACL on the file. So this is strangely asymmetric:

(1) We create a file requesting SEC_FILE_EXECUTE.
    The file gets created without any execut perms.
    (without acl_xattr and with map archive = no)
    The create call succees.

(2) Later open of the existing file also requesting
    SEC_FILE_EXECUTE fails.

If I do acl_xattr or map archive = yes though, then #1 also
creates execute bits on the file. And #2 succeeds.

I'll dig some more.

Cheers - Michael
-------------- 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/20200515/86ecf8d1/signature.sig>