Isn't skipping this the right thing to do ? After all, we simply don't have permission to see anything on this file - bit like a symlink that points nowhere (which we also skip).

Don't want to add become_root(), as the admin explicitly said YOU CANNOT STAT THIS FILE.

with NFS4 permissions we cannot stat the file but the file is being listed e.g, by a plain "ls", which is not making a stat call. So definetely it needs to be listed by us, too. A client who doesn't get the file listed and then tries to creat the file will finally fail then.

In addition to that - Windows semantics there is a number of metadata readable without that. Timestamps for example are always accessable, no matter if "read attrubutes" is allowed or not. So in order to return the information that a Windows client expect here we need to add a become_root here. We might make the behaviour dependent on whether or not "dos filemode" is set to yes, as this is the parameter that we already now use to turn on Windows semantics in similar cases.

Ralph just pointed out that this is solved in the gpfs module (even though this is a generic NFS4 problem) already with stat_with_capabilities.

Can we please take care that generic problems, even if noticed in gpfs first, get fixed in generic code and not only in the gpfs or other filesystem specific modules? I've run into similar cases before.

When you cannot stat() a directory entry you cannot even tell if it's a directory or file. So how should Samba present it then?

When I last looked into this (several years ago), there was another quirk with the Linux kernel: the kernel wasn't prepared that stat() might be denied.

If one user could stat() the file, the entry was stored in the kernel's stat-cache and when another user that didn't have access to the file tried to stat() the file, the kernel happily returned the entry from the stat cache without even asking the filesystem again.

The result were unreliable directory listings.

Not sure if this has changed in the kernel in the past years.

Regarding the caching bug: Linux doesn't natively support NFS4 ACLs anyway. Have in mind that all other supported Unix flavours (FreeBSD, AIX, Solaris) do natively support NFS4 ACLs.

And true, as we need to figure out if it's a file or directory, I think we indeed need to run the stat with capabilities (as implemented in the gpfs vfs module) or become_root in case the initial stat returns an EACCES.

(In reply to Björn Jacke from comment #5)

"And true, as we need to figure out if it's a file or directory, I think we indeed need to run the stat with capabilities (as implemented in the gpfs vfs module) or become_root in case the initial stat returns an EACCES."

100% disagree with that. If stat is denied, we need to just not return the entry.

If we don't have the rights to tell the client if it's a file or directory, then we shouldn't override that. It might be a secure information leak from the server.

what do you say about this case:

1) from a Windows client the "read attributes" permission if taken away. Note that this is fine in Windows semantics and does not prevent the file/directory to be listed properly.

2) smbd will not show the file any more because smbd isn't able to stat the file any more and thus hides it.

3) the windows client doesn't see the file anymore, then wants to create the file/directory but surprisingly cannot do it (because it already exists in the server, which the server)

This cannot be the behavour that we want.

(In reply to Björn Jacke from comment #7)

> 1) from a Windows client the "read attributes" permission if taken away. Note
> that this is fine in Windows semantics and does not prevent the file/directory
> to be listed properly.

Yes, well that's Windows semantics. Under windows I don't believe you can remove FILE_READ_ATTRIBUTES.

From smbd/open.c:

 155          * If we can access the path to this file, by
 156          * default we have FILE_READ_ATTRIBUTES from the
 157          * containing directory. See the section:
 158          * "Algorithm to Check Access to an Existing File"
 159          * in MS-FSA.pdf.

This is just a difference in semantics between NFSv4 and Windows. In NFSv4 if you remove read attributes you don't get read attributes on the file.

So the question we need to answer here is, should we override NFSv4 semantics here  ? Is it possible we may be violating security if we become_root() to stat a file with "READ_ATTRIBUTES" removed ?

Would we have to wrap every stat call with this ? As you can see, it gets really tricky...

(In reply to Jeremy Allison from comment #8)
Fwiw, this was implemented in the gpfs VFS module years ago by using the DAC_OVERRIDE capability:

https://lists.samba.org/archive/samba-technical/2013-October/095369.html

$ git show 92257ee41fd5d47b4248ae582bb48b71a13c7174

As Christian pointed out, the main reason for implementing this in vfs_gpfs
was to get around the inconsistent behavior when the Linux kernel cache
interacts with the file system. This was seen as a GPFS specific case;
the assumption was that no other Linux file system would implement NFSv4
ACLs in this way.

If that is now a wider issue, i would support moving the functionality
to more common code.

Do we know the exact codepath that is used here? Is that only querying
whether an object is a file or directory? Maybe there could be a specific
VFS call for this case, and only that one is allowed to have elevated
privileges, so that no other information.

Or add a field to the VFS stat call, similar to Linux statx. And if
that only specifies STATX_MODE, then become_root or the capability
is used.

(In reply to Christof Schmitt from comment #10)
> Or add a field to the VFS stat call, similar to Linux statx. And if
> that only specifies STATX_MODE, then become_root or the capability
> is used.

I meant STATX_TYPE, if the object type is the only data that is required
when READ_ATTRIBUTES is missing.

Created attachment 17364 [details]
patch fixing the gpfs capability use on AIX

the mentioned gpfs fix here uses POSIX capabilities. On AIX however, where the gpfs module is also supported, the required POSIX capabilites are not supported. So we need to fall back to become_root in that case. The advantage if we have a become_root fallback in case we have no POSIX capabilites would be that we are able also use preferably use capabilities at many other places, where we're currently simply using become_root().

Comment on attachment 17364 [details]
patch fixing the gpfs capability use on AIX

We can't do this. Look at the use of set_effective_capability() inside dmapi_file_flags() and dmapi_init_session() and NEVER CALL drop_effective_capability() again.

That would leave us as root across all of smbd all the time.

root != capabilities.

good point, I missed that. How about adding a check if the request cap is DAC_OVERRIDE_CAPABILITY ?  Or is this still too suspicious to you? Or should we make that one a dedicated function? Generally you agree that using capabilites by default instead of of become_root() would be nice, right?

(In reply to Björn Jacke from comment #14)

Yeah, I thought about adding the:

if (capability == DAC_OVERRIDE_CAPABILITY) check, but it's still a bit scary :-).

> Generally you agree that using capabilites by default instead of of become_root() would be nice, right?

Yep. I 100% agree with this !

This bug was referenced in samba master:

a1738e8265dd256c5a1064482a6dfccbf9ca44f1

Created attachment 18187 [details]
Backport patch for both 4.18 and 4.19

Reassigning for inclusion in 4.18 and 4.19.

Pushed to autobuild-v4-{19,18}-test.

This bug was referenced in samba v4-18-test:

a2ad66e4933b6fd0a30218b779d5e3e8e9b4750c

This bug was referenced in samba v4-19-test:

af4fe00f2646bcc297053241d51ac841d982a078

Closing out bug report.

Thanks!

This bug was referenced in samba v4-19-stable (Release samba-4.19.3):

af4fe00f2646bcc297053241d51ac841d982a078

This bug was referenced in samba v4-18-stable (Release samba-4.18.9):

a2ad66e4933b6fd0a30218b779d5e3e8e9b4750c