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

[Michael Adam]

On 2020-05-13 at 17:55 -0700, Jeremy Allison wrote:
> On Thu, May 14, 2020 at 01:36:12AM +0200, Michael Adam wrote:
> > 
> > Right, and this is done unconditionally in open_file_ntcreate(),
> > I mean without checking lp_store_dos_attributes() first. So the
> > manpage is clearly wrong. store-dos-attributes disables none of
> > the map options. We only see the effect for map archive since
> > the other three default to "no" anyway...
> 
> Yep.
> 
> > What I am wondering is this:
> > If the file is created with SEC_RIGHTS_FILE_ALL, shouldn't it
> > get execute permissions, even if "map archive = no"?
> > After all, "map archive = no" does not prevent execute from
> > being set, it just doesn't set it because of archive...
> 
> No. SEC_RIGHTS_FILE_ALL is only to do with the access permissions
> on the handle, not the permissions on the file.

Hmm, ok, I wasn't fully aware of that.

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.

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.

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.

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


> > And this should be true without vfs_acl_xattr, but afaict also
> > with the module, when system_acls are not ignored (i.e. when the
> > acls are still mapped to posix acls).
> > 
> > So I am missing the piece where the actual desired permissions
> > from the create call are factored into the resulting unix
> > permissoins.
> 
> It's just passed down directly from the mode bits that
> are created in open_file_ntcreate().
> 
> 	open_file_ntcreate()
> 		mode_t unx_mode = unix_mode(conn, new_dos_attributes | FILE_ATTRIBUTE_ARCHIVE,
>                                      smb_fname, parent_dir_fname);
> 
> 		-> open_file(..., unx_mode, ...)
> 			-> fd_open_atomic(..., unx_mode, ...)
> 				-> fd_open(..., unx_mode, ...)
> 					-> non_widelink_open(..., unx_mode, ...)
> 						-> SMB_VFS_OPEN(..., unx_mode, ...)
> 							-> open(name, flags, unx_mode)
> 
> 
> > Well, at least the manpage describes some original intentions
> > as it seems. The current behavior is at least surprising when
> > the man page text is taken into account.
> 
> Yep. The man page is wrong here.
> 
> > Of course, thanks!
> > 
> > Two questions remain:
> > 
> > - Should we implement the behavior stated in the manpage?
> >   (I think yes!)
> 
> Hmmmm. Maybe. Be careful of changing things here,
> it may not be what users expect.
> 
> > - Why is the x-bit not set when SEC_FILE_ALL is requested in create?
> 
> 'cos SEC_FILE_ALL is bugger all to do with on-disk
> file permissions. That's a handle access mode (think
> fd, not file). You have to set a security descriptor for that.

That statement does not seem to be 100% true:
As mentioned above, when opening an existing file,
it seems that the disk file permissions (via acl) are consulted to
determine whether the desired fd-access can be granted.
So in order to grant SEC_FILE_EXECUTE upon later open,
we seem to have to have set execute perms on the file at creation.

From the observed phenomena, something seems to be
off here. Or just working by coincidence?

If acl_xattr is used, we get the file +x perms on disk at creation.
(via acl, also on unix mode if we don't ignore system acls).
If acl_xattr is not used and map archive = no, then +x perms are NOT set.

I did some more tests with the torture code.
If we create a file with
SEC_FILE_READ_ATTRIBUTE | SEC_FILE_READ_DATA
then the execute perms are still set when using acl_xattr.
So this confirms that the setting on disk is not related
to the desired access mode at creation.

But the acl_xattr module behaves very differently from
the pur unix permissions / posix acls mode in that it
seems to set execute permissions more often?


> Remember there's no atomic open_with_posix_acl() call
> in POSIX. So you get what is inherited from the parent
> directory (if it's set up that way). Samba then overlays
> (non-atomically, as there's no other way in POSIX) and
> security descriptor set on the file after it's created.

There is no parent directory acl here afaict.
And even with ignonre_system_acls it works.
(It just doesn't show up on the posix perms then.)


So to sum up:

- When creating a new file, the desired access mode parameter
  does not influence the on-disk permissions/acls that are set on the
  file.

- When opening an existing file, the on-disk perms are consulted
  to detect if access can be granted.

Sorry for the long mail.
Going through this highly convoluted and uncommented code is quite
a journey.. ;-)

Cheers - Michael

> 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/20200514/924a2535/signature.sig>