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

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2026-01-16 at 10:45 -0500, Chuck Lever wrote:
> 
> On Fri, Jan 16, 2026, at 10:25 AM, Jeff Layton wrote:
> > 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.
> 
> We already configure listeners and threads in separate operations.
> The ordering is managed. It's reasonable for the kernel to block
> fh_key changes while the NFS server is in operation.
> 
> I'd much rather set a precedent of several small ops rather than
> one or two Swiss army knives.
>
> 

I disagree. Having all of the server settings as discrete elements was
part of the problem with the old nfsdfs-based interface. We had this
set of discrete knobs that needed to all be twiddled in the correct
order.

The fact that we can send down server parameters in a single block is a
strength of the netlink interface, IMO. We can easily add optional
parameters and I think that's what we should do here.


> > > 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.
> 
> My point is none of this has anything to do with thread count.
> Setting the fh_key needs to be a distinct UI element.

"threads" should probably have been named "service", since it's
basically the method that we use to create the actual service,
culminating with starting threads.

-- 
Jeff Layton <jlayton@kernel.org>