Write delegation stateid permission checks

Write delegation stateid permission checks

[Shaul Tamari]

Hi,

I have a question regarding NFS4.1 write delegation stateid permission checks.

Is there a difference in how a server should check permissions for a
write delegation stateid that was given when the file was opened with:
1. OPEN4_SHARE_ACCESS_BOTH
2. OPEN4_SHARE_ACCESS_WRITE

Should the server check permissions for read access as well when
OPEN4_SHARE_ACCESS_WRITE is requested and DELEGATION_WRITE is granted
? or there is an assumption that the client will query the server for
read access permissions ?


Thanks,
Shaul

Re: Write delegation stateid permission checks

[Jeff Layton]

On Thu, 2025-01-02 at 11:08 +0200, Shaul Tamari wrote:
> Hi,
> 
> I have a question regarding NFS4.1 write delegation stateid permission checks.
> 
> Is there a difference in how a server should check permissions for a
> write delegation stateid that was given when the file was opened with:
> 1. OPEN4_SHARE_ACCESS_BOTH
> 2. OPEN4_SHARE_ACCESS_WRITE
> 

(cc'ing Sagi since he was looking at this recently)

A write delegation really should have been called a read-write
delegation, because the server has to allow the client to do reads as
well, if you hold one.

The Linux kernel nfs server doesn't currently give out delegations to
OPEN4_SHARE_ACCESS_WRITE-only opens for this reason. You have to
request BOTH in order to get one, because a permission check for write
is not sufficient to allow you to read as well.


> Should the server check permissions for read access as well when
> OPEN4_SHARE_ACCESS_WRITE is requested and DELEGATION_WRITE is granted
> ?
> 

Possibly? When trying to grant a write delegation, the server should
probably also do an opportunistic permission check for read as well,
and only grant the delegation if that passes. If it fails, you could
still allow the open and just not grant the delegation.

ISTR that Sagi may have tried this approach though and there was a
problem with it?

> or there is an assumption that the client will query the server for
> read access permissions ?
> 

The client should always do an ACCESS call to test permissions unless
the user's access matches the ACE that gets sent along with the
delegation.
-- 
Jeff Layton <jlayton@kernel.org>

Re: Write delegation stateid permission checks

[Rick Macklem]

On Thu, Jan 2, 2025 at 5:41 AM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Thu, 2025-01-02 at 11:08 +0200, Shaul Tamari wrote:
> > Hi,
> >
> > I have a question regarding NFS4.1 write delegation stateid permission checks.
> >
> > Is there a difference in how a server should check permissions for a
> > write delegation stateid that was given when the file was opened with:
> > 1. OPEN4_SHARE_ACCESS_BOTH
> > 2. OPEN4_SHARE_ACCESS_WRITE
> >
>
> (cc'ing Sagi since he was looking at this recently)
>
> A write delegation really should have been called a read-write
> delegation, because the server has to allow the client to do reads as
> well, if you hold one.
My interpretation for "write" is that it was a synonym for "exclusive".

>
> The Linux kernel nfs server doesn't currently give out delegations to
> OPEN4_SHARE_ACCESS_WRITE-only opens for this reason. You have to
> request BOTH in order to get one, because a permission check for write
> is not sufficient to allow you to read as well.
I'll admit my interpretation has always been somewhat different.
My version is that a delegation allows a client to do Opens locally,
but does not infer permissions for any given user.

A Jeff notes at the end of his post, a client must do a ACCESS
operation to check for permissions during Opens unless the ACE
in the write delegation reply allows access.

Given the above, a server's decision to issue a write delegation
is orthogonal to file permissions, except for what it returns in the ACE, imho.

>
>
> > Should the server check permissions for read access as well when
> > OPEN4_SHARE_ACCESS_WRITE is requested and DELEGATION_WRITE is granted
> > ?
> >
>
> Possibly? When trying to grant a write delegation, the server should
> probably also do an opportunistic permission check for read as well,
> and only grant the delegation if that passes. If it fails, you could
> still allow the open and just not grant the delegation.
My interpretation is that the server can choose to (or choose not to)
issue a delegation for any file. The only requirement is a working
callback path. The WANT_xxx flags are just hints from the client.

Although it might not make much sense, a server can even issue
a write delegation for a file that is r--r--r--.

Again, this is my interpretation of the RFCs.

>
> ISTR that Sagi may have tried this approach though and there was a
> problem with it?
>
> > or there is an assumption that the client will query the server for
> > read access permissions ?
> >
>
> The client should always do an ACCESS call to test permissions unless
> the user's access matches the ACE that gets sent along with the
> delegation.
Yes, and this is why a write delegation can be issued whenever the server
feels like doing so.

rick

> --
> Jeff Layton <jlayton@kernel.org>
>

Re: Write delegation stateid permission checks

[Chuck Lever]

On 1/2/25 8:41 AM, Jeff Layton wrote:
> On Thu, 2025-01-02 at 11:08 +0200, Shaul Tamari wrote:
>> Hi,
>>
>> I have a question regarding NFS4.1 write delegation stateid permission checks.
>>
>> Is there a difference in how a server should check permissions for a
>> write delegation stateid that was given when the file was opened with:
>> 1. OPEN4_SHARE_ACCESS_BOTH
>> 2. OPEN4_SHARE_ACCESS_WRITE
>>
> 
> (cc'ing Sagi since he was looking at this recently)
> 
> A write delegation really should have been called a read-write
> delegation, because the server has to allow the client to do reads as
> well, if you hold one.

Another way to put it is the write delegation is equivalent to a RW
open. It permits the client to handle opens on this file locally without
having to send another OPEN on the wire (as long as certain conditions
hold true).

So, therefore, the client is permitted to use that write delegation
state ID for READ operations.


> The Linux kernel nfs server doesn't currently give out delegations to
> OPEN4_SHARE_ACCESS_WRITE-only opens for this reason. You have to
> request BOTH in order to get one, because a permission check for write
> is not sufficient to allow you to read as well.
> 
> 
>> Should the server check permissions for read access as well when
>> OPEN4_SHARE_ACCESS_WRITE is requested and DELEGATION_WRITE is granted
>> ?
>>
> 
> Possibly? When trying to grant a write delegation, the server should
> probably also do an opportunistic permission check for read as well,
> and only grant the delegation if that passes. If it fails, you could
> still allow the open and just not grant the delegation.
> 
> ISTR that Sagi may have tried this approach though and there was a
> problem with it?
> 
>> or there is an assumption that the client will query the server for
>> read access permissions ?
>>
> 
> The client should always do an ACCESS call to test permissions unless
> the user's access matches the ACE that gets sent along with the
> delegation.

NFSD currently doesn't return this ACE, so the ACCESS round trip is
still needed.

Basically the ACE tells the client what set of local open(2) calls
are allowed without having to return the write delegation.


-- 
Chuck Lever

Re: Write delegation stateid permission checks

[Rick Macklem]

On Thu, Jan 2, 2025 at 8:01 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On 1/2/25 8:41 AM, Jeff Layton wrote:
> > On Thu, 2025-01-02 at 11:08 +0200, Shaul Tamari wrote:
> >> Hi,
> >>
> >> I have a question regarding NFS4.1 write delegation stateid permission checks.
> >>
> >> Is there a difference in how a server should check permissions for a
> >> write delegation stateid that was given when the file was opened with:
> >> 1. OPEN4_SHARE_ACCESS_BOTH
> >> 2. OPEN4_SHARE_ACCESS_WRITE
> >>
> >
> > (cc'ing Sagi since he was looking at this recently)
> >
> > A write delegation really should have been called a read-write
> > delegation, because the server has to allow the client to do reads as
> > well, if you hold one.
>
> Another way to put it is the write delegation is equivalent to a RW
> open. It permits the client to handle opens on this file locally without
> having to send another OPEN on the wire (as long as certain conditions
> hold true).
>
> So, therefore, the client is permitted to use that write delegation
> state ID for READ operations.
>
>
> > The Linux kernel nfs server doesn't currently give out delegations to
> > OPEN4_SHARE_ACCESS_WRITE-only opens for this reason. You have to
> > request BOTH in order to get one, because a permission check for write
> > is not sufficient to allow you to read as well.
> >
> >
> >> Should the server check permissions for read access as well when
> >> OPEN4_SHARE_ACCESS_WRITE is requested and DELEGATION_WRITE is granted
> >> ?
> >>
> >
> > Possibly? When trying to grant a write delegation, the server should
> > probably also do an opportunistic permission check for read as well,
> > and only grant the delegation if that passes. If it fails, you could
> > still allow the open and just not grant the delegation.
> >
> > ISTR that Sagi may have tried this approach though and there was a
> > problem with it?
> >
> >> or there is an assumption that the client will query the server for
> >> read access permissions ?
> >>
> >
> > The client should always do an ACCESS call to test permissions unless
> > the user's access matches the ACE that gets sent along with the
> > delegation.
>
> NFSD currently doesn't return this ACE, so the ACCESS round trip is
> still needed.
>
> Basically the ACE tells the client what set of local open(2) calls
> are allowed without having to return the write delegation.
Yes, but I am not sure why you say "return the write delegation".
I do not think that has anything to do with the ACE. The client
must return the delegation when a CBRECALL is issued to it
by the server. Otherwise, the client can DELEGRETURN whenever
it feels like it. (Doing so when doing a REMOVE or RENAME can
avoid a CBRECALL.)

My interpretation is that the ACE tells the client when it can do
local opens without bothering to do an ACCESS operation.

rick

>
>
> --
> Chuck Lever
>

Re: Write delegation stateid permission checks

[Sagi Grimberg]

On 02/01/2025 15:41, Jeff Layton wrote:
> On Thu, 2025-01-02 at 11:08 +0200, Shaul Tamari wrote:
>> Hi,
>>
>> I have a question regarding NFS4.1 write delegation stateid permission checks.
>>
>> Is there a difference in how a server should check permissions for a
>> write delegation stateid that was given when the file was opened with:
>> 1. OPEN4_SHARE_ACCESS_BOTH
>> 2. OPEN4_SHARE_ACCESS_WRITE
>>
> (cc'ing Sagi since he was looking at this recently)

And completely dropped the ball on this :\

>
> A write delegation really should have been called a read-write
> delegation, because the server has to allow the client to do reads as
> well, if you hold one.

Assuming the access check passes.

>
> The Linux kernel nfs server doesn't currently give out delegations to
> OPEN4_SHARE_ACCESS_WRITE-only opens for this reason. You have to
> request BOTH in order to get one, because a permission check for write
> is not sufficient to allow you to read as well.
>
>
>> Should the server check permissions for read access as well when
>> OPEN4_SHARE_ACCESS_WRITE is requested and DELEGATION_WRITE is granted
>> ?
>>
> Possibly? When trying to grant a write delegation, the server should
> probably also do an opportunistic permission check for read as well,
> and only grant the delegation if that passes. If it fails, you could
> still allow the open and just not grant the delegation.

Yes, that is what Chuck suggested at the time.

>
> ISTR that Sagi may have tried this approach though and there was a
> problem with it?

Not a problem per se, IIRC the thread left off that we need to sort out
access reference accounting for nfsd_file for both reads and writes for
a single write deleg...

Re: Write delegation stateid permission checks

[Chuck Lever]

On 1/2/25 5:07 PM, Sagi Grimberg wrote:
> 
> 
> 
> On 02/01/2025 15:41, Jeff Layton wrote:
>> On Thu, 2025-01-02 at 11:08 +0200, Shaul Tamari wrote:
>>> Should the server check permissions for read access as well when
>>> OPEN4_SHARE_ACCESS_WRITE is requested and DELEGATION_WRITE is granted
>>> ?
>>>
>> Possibly? When trying to grant a write delegation, the server should
>> probably also do an opportunistic permission check for read as well,
>> and only grant the delegation if that passes. If it fails, you could
>> still allow the open and just not grant the delegation.
> 
> Yes, that is what Chuck suggested at the time.
> 
>>
>> ISTR that Sagi may have tried this approach though and there was a
>> problem with it?
> 
> Not a problem per se, IIRC the thread left off that we need to sort out
> access reference accounting for nfsd_file for both reads and writes for
> a single write deleg...

I've been pondering this recently.

The desired outcome would be to deal with a OPEN(SHARE_ACCESS_WRITE)
by opening the file twice: once, WR_ONLY, for the OPEN state ID, and
once RW, for the write delegation.

Will that work? won't the two opens conflict with each other and
trigger a delegation recall?

Seems like NFSD will need a lot more logic to make this approach
work right.


-- 
Chuck Lever

Re: Write delegation stateid permission checks

[Sagi Grimberg]

On 03/01/2025 0:42, Chuck Lever wrote:
> On 1/2/25 5:07 PM, Sagi Grimberg wrote:
>>
>>
>>
>> On 02/01/2025 15:41, Jeff Layton wrote:
>>> On Thu, 2025-01-02 at 11:08 +0200, Shaul Tamari wrote:
>>>> Should the server check permissions for read access as well when
>>>> OPEN4_SHARE_ACCESS_WRITE is requested and DELEGATION_WRITE is granted
>>>> ?
>>>>
>>> Possibly? When trying to grant a write delegation, the server should
>>> probably also do an opportunistic permission check for read as well,
>>> and only grant the delegation if that passes. If it fails, you could
>>> still allow the open and just not grant the delegation.
>>
>> Yes, that is what Chuck suggested at the time.
>>
>>>
>>> ISTR that Sagi may have tried this approach though and there was a
>>> problem with it?
>>
>> Not a problem per se, IIRC the thread left off that we need to sort out
>> access reference accounting for nfsd_file for both reads and writes for
>> a single write deleg...
>
> I've been pondering this recently.
>
> The desired outcome would be to deal with a OPEN(SHARE_ACCESS_WRITE)
> by opening the file twice: once, WR_ONLY, for the OPEN state ID, and
> once RW, for the write delegation.

Makes sense. I guess we should skip the WR_ONLY open if the client asks for
OPEN_XOR_DELEGATION and will never send a CLOSE.

>
> Will that work? won't the two opens conflict with each other and
> trigger a delegation recall?

I think that nfsd_breaker_owns_lease() should prevent the conflict as 
the two
come from the same client by definition. But I'm maybe misinterpreting it..

Re: Write delegation stateid permission checks

[Martin Wege]

On Thu, Jan 2, 2025 at 2:41 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Thu, 2025-01-02 at 11:08 +0200, Shaul Tamari wrote:
> > Hi,
> >
> > I have a question regarding NFS4.1 write delegation stateid permission checks.
> >
> > Is there a difference in how a server should check permissions for a
> > write delegation stateid that was given when the file was opened with:
> > 1. OPEN4_SHARE_ACCESS_BOTH
> > 2. OPEN4_SHARE_ACCESS_WRITE
> >
>
> (cc'ing Sagi since he was looking at this recently)
>
> A write delegation really should have been called a read-write
> delegation, because the server has to allow the client to do reads as
> well, if you hold one.
>
> The Linux kernel nfs server doesn't currently give out delegations to
> OPEN4_SHARE_ACCESS_WRITE-only opens for this reason. You have to
> request BOTH in order to get one, because a permission check for write
> is not sufficient to allow you to read as well.

So a NFS4.1 client must replace OPEN4_SHARE_ACCESS_WRITE with
OPEN4_SHARE_ACCESS_BOTH to get write delegations?

Thanks,
Martin