Re: [PATCH v1 2/4] nfsd: Add a key for signing filehandles

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2026-01-16 at 10:09 -0500, Chuck Lever wrote:
> 
> On Fri, Jan 16, 2026, at 9:59 AM, Jeff Layton wrote:
> > On Fri, 2026-01-16 at 09:32 -0500, Benjamin Coddington wrote:
> > > Expand the nfsd_net to hold a siphash_key_t value "fh_key".
> > > 
> > > Expand the netlink server interface to allow the setting of the 128-bit
> > > fh_key value to be used as a signing key for filehandles.
> > > 
> > > Add a file to the nfsd filesystem to set and read the 128-bit key,
> > > formatted as a uuid.
> > > 
> > > Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
> > > ---
> > >  Documentation/netlink/specs/nfsd.yaml | 12 ++++
> > >  fs/nfsd/netlink.c                     | 15 +++++
> > >  fs/nfsd/netlink.h                     |  1 +
> > >  fs/nfsd/netns.h                       |  2 +
> > >  fs/nfsd/nfsctl.c                      | 85 +++++++++++++++++++++++++++
> > >  fs/nfsd/trace.h                       | 19 ++++++
> > >  include/uapi/linux/nfsd_netlink.h     |  2 +
> > >  7 files changed, 136 insertions(+)
> > > 
> > > diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
> > > index badb2fe57c98..a467888cfa62 100644
> > > --- a/Documentation/netlink/specs/nfsd.yaml
> > > +++ b/Documentation/netlink/specs/nfsd.yaml
> > > @@ -81,6 +81,9 @@ attribute-sets:
> > >        -
> > >          name: min-threads
> > >          type: u32
> > > +      -
> > > +        name: fh-key
> > > +        type: binary
> > >    -
> > >      name: version
> > >      attributes:
> > > @@ -227,3 +230,12 @@ operations:
> > >            attributes:
> > >              - mode
> > >              - npools
> > > +    -
> > > +      name: fh-key-set
> > > +      doc: set encryption key for filehandles
> > > +      attribute-set: server
> > > +      flags: [admin-perm]
> > > +      do:
> > > +        request:
> > > +          attributes:
> > > +            - fh-key
> > 
> > Rather than a new netlink operation, I think we might be better served
> > with just sending the fh-key down as an optional attribute in the
> > "threads" op. It's a per-netns attribute anyway, and the threads
> > setting is handled similarly.
> 
> Setting the FH key in the threads op seems awkward to me.
> Setting a key is optional, but you always set the thread
> count to start the server.
> 
> Key setting is done once; whereas setting the thread count
> can be done many times during operation. It seems like it
> would be easy to mistakenly change the key when setting the
> thread count.
> 
> From a "UI safety" perspective, a separate op makes sense
> to me.
> 

I'm not convinced. We could easily vet that the key doesn't change when
changing the thread count, and either return an error or throw some
sort of warning and ignore the change.

My main thinking here is that you'd want to set up the key at startup
time and never change it, so if the server is already running you
probably want to reject key changes -- otherwise you may have already
given out some unencrypted handles.

If that's the case, then now you have to ensure you run the op to set
the key before issuing "threads".

Why deal with an ordering constraint like that? Optionally passing down
the key with "threads" means we handle it all in one shot.


> What feels a little strange though is where to store the
> key? I was thinking in /etc/exports, but that would make
> the FH key per-export rather than per-server instance.
> 
> That gives a cryptographic benefit, as there would be
> more keying material. But maybe it doesn't make a lot of
> sense from a UX perspective.
> 
> On the other hand, some might like to manage the key by
> storing it in a trusted compute module -- systemd has
> a facility to extract keys from a TCM.
> 

Yeah, there are a lot of possibilities here. I like the idea of
scraping this out of the TPM, but that's not going to be possible
everywhere. We'll also need some alternate method of storing the key in
a secure way on the fs so that nfsdctl can get to it for hosts that
don't have a TPM.
-- 
Jeff Layton <jlayton@kernel.org>