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

[Anoop C S]

On Wed, 2020-05-13 at 17:55 -0700, Jeremy Allison via samba-technical
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.

But what if file does not exist and is being created? Consider
smb2.read.position where it starts with an already existing file. If we
remove the file prior to executing this sub-test, it passes even with
"map archive = no"(of course without execute bit set). What does that
mean?

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