Re: [PATCH v2 3/3] NFSD: Sign filehandles

[Chuck Lever <cel@kernel.org>]

On 1/24/26 8:58 AM, Benjamin Coddington wrote:
> Hey Chuck and Neil - Sorry to be late responding here..
> 
> On 23 Jan 2026, at 20:56, NeilBrown wrote:
> 
>> On Sat, 24 Jan 2026, Chuck Lever wrote:
>>>
>>> On Fri, Jan 23, 2026, at 6:38 PM, NeilBrown wrote:
>>>> On Sat, 24 Jan 2026, Chuck Lever wrote:
>>>>> On 1/23/26 5:21 PM, NeilBrown wrote:
>>>>>> On Sat, 24 Jan 2026, Chuck Lever wrote:
>>>>>>>
>>>>>>> On Wed, Jan 21, 2026, at 3:24 PM, Benjamin Coddington wrote:
> ...
>>>>>>>>
>>>>>>>> +#define FH_AT_NONE		0
>>>>>>>> +#define FH_AT_MAC		1
>>>>>>>
>>>>>>> I'm pleased at how much this patch has shrunk since v1.
> 
> Me too, thanks for all the help refining it.
> 
>>>>>>>
>>>>>>> This might not be an actionable review comment, but help me understand
>>>>>>> this particular point. Why do you need both a sign_fh export option
>>>>>>> and a new FH auth type? Shouldn't the server just look for and
>>>>>>> validate FH signatures whenever the sign_fh export option is
>>>>>>> present?
> 
> Its vestigial from the encrypted_fh version which required it because the
> fsid might be encrypted, so NFSD couldn't look up the export to see if it
> was set to encrypt until decrypting the fsid, and needed the auth type to
> know if it was encrypted.
> 
>>>>>> ...and also generate valid signatures on outgoing file handles.
>>>>>>
>>>>>> What does the server do to "look for" an FH signature so that it can
>>>>>> "validate" it?  Answer: it inspects the fh_auth_type to see if it is
>>>>>> FT_AT_MAC.
>>>>>
>>>>> No, NFSD checks the sign_fh export option. At first glance the two
>>>>> seem redundant, and I might hesitate to inspect or not inspect
>>>>> depending on information content received from a remote system. The
>>>>> security policy is defined precisely by the "sign_fh" export option I
>>>>> would think?
> 
> Yes, now its a bit redundant - but it could be used to still accept
> filehandles that are signed after removing a "sign_fh" from an export.  In
> other words, it might be useful to be "be liberal in what you accept from
> others".  It would be essential if future patches wanted to "drain" and
> "fill" clients with signed/plain filehandles using more permissive policies.
> *waves hands wildly*
> 
>>>> So maybe you are thinking that, when sign_fh, is in effect - nfsd
>>>> could always strip off the last 8 bytes, hash the remainder, and check
>>>> the result matches the stripped bytes.
>>>
>>> I’m wondering why there is both — the purpose of having these two
>>> seemingly redundant signals is worth documenting. There was some
>>> discussion a few days ago about whether the root FH could be signed
>>> or not. I thought for a moment or two that maybe when sign_fh is
>>> enabled, there will be one or more file handles on that export that
>>> won’t have a signature, and FT_AT_NONE would set those apart
>>> from the signed FHs. Again, I’d like to see that documented if that is
>>> the case.
> 
> Right now no, not that I know of - the root filehandle is the only one, and
> its easy to detect.
> 
>> I would document it as:
>>
>>  sign_fh is needs to configure server policy
>>  FT_AT_MAC, while technically redundant with sign_fh, is valuable
>>   whehn interpreting NFS packet captures.
> 
> Yes, it would allow a network dissector to locate and parse the MAC.
> 
>>> In addition, I’ve always been told that what comes off the network
>>> is completely untrusted. So, I want some assurance that using the
>>> incoming FH’s auth type as part of the decision to check the signature
>>> conforms with known best practices.
>>>
>>>> Another reason is that it helps people who are looking at network
>>>> packets captures to try to work out what is going wrong.
>>>> Seeing a flag to say "there is a signature" could help.
>>>
>>> Sure. But unconditionally trusting that flag is another question.
>>
>> By the time the code has reached this point it has already
>> unconditionally trusted the RPC header, the NFS opcode, the '1' in
>> fh_version, the fh_fsid_type and the fsid itself.
>>
>> Going further to trust fh_auth_type to the extent that we reject the
>> request if it is 0, and check the MAC if it is 1 - is not significant.

What I'm saying is that if it makes no difference to the security level,
then let's not bother to check it at all.


> Not a great argument, I know, but I think its nice to keep the standard that
> filehandles are independently self-describing.
> 
> We're building server systems that pass around filehandles generated by NFSD
> and it can be a useful signal to those 3rd-party systems that there's a
> signature.  Trond might know more about whether its essential - I'll ask him
> to weigh in here.

Thanks, yes, let's hear from Trond.


> All said - please let me know if the next version should keep it.

There are really two question marks:

1. If I were a security reviewer, I would say that NFSD shouldn't rely
on network input like this to decide whether or not to validate the MAC.
Either the server expects a MAC and uses it to validate, or it doesn't.
For me as a maintainer, that is a risk we probably can deal with
immediately -- would it be OK at least to change the FH verification
code to not use the auth_type to decide when to validate the FH's MAC?

2. Is setting FH_AT_MAC still useful for other reasons? I think we don't
really know whether to keep the auth_type or how to document it until
we've decided on how exactly NFSD will deal with changing the sign_fh
setting while clients have the export mounted.

So, let's leave the field in place and we'll come back to it. If you
want, add a comment like /* XXX is FH_AT_MAC still needed? */


-- 
Chuck Lever