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

On Fri, Jan 23, 2026, at 6:38 PM, NeilBrown wrote:

On Sat, 24 Jan 2026, Chuck Lever wrote:

quoted

On 1/23/26 5:21 PM, NeilBrown wrote:

quoted

On Sat, 24 Jan 2026, Chuck Lever wrote:

quoted

On Wed, Jan 21, 2026, at 3:24 PM, Benjamin Coddington wrote:

quoted

NFS clients may bypass restrictive directory permissions by using
open_by_handle() (or other available OS system call) to guess the
filehandles for files below that directory.

In order to harden knfsd servers against this attack, create a method to
sign and verify filehandles using siphash as a MAC (Message Authentication
Code).  Filehandles that have been signed cannot be tampered with, nor can
clients reasonably guess correct filehandles and hashes that may exist in
parts of the filesystem they cannot access due to directory permissions.

Append the 8 byte siphash to encoded filehandles for exports that have set
the "sign_fh" export option.  The filehandle's fh_auth_type is set to
FH_AT_MAC(1) to indicate the filehandle is signed.  Filehandles received from
clients are verified by comparing the appended hash to the expected hash.
If the MAC does not match the server responds with NFS error _BADHANDLE.
If unsigned filehandles are received for an export with "sign_fh" they are
rejected with NFS error _BADHANDLE.

Link: 
https://lore.kernel.org/linux-nfs/cover.1769026777.git.bcodding@hammerspace.com (local)
Signed-off-by: Benjamin Coddington <redacted>
---
 fs/nfsd/nfsfh.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++--
 fs/nfsd/nfsfh.h |  3 ++
 2 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index ed85dd43da18..ea3473acbf71 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c

@@ -11,6 +11,7 @@
 #include <linux/exportfs.h>

 #include <linux/sunrpc/svcauth_gss.h>
+#include <crypto/utils.h>
 #include "nfsd.h"
 #include "vfs.h"
 #include "auth.h"

@@ -137,6 +138,61 @@ static inline __be32 check_pseudo_root(struct 

dentry *dentry,
 	return nfs_ok;
 }

+/*
+ * Append an 8-byte MAC to the filehandle hashed from the server's 
fh_key:
+ */
+static int fh_append_mac(struct svc_fh *fhp, struct net *net)
+{
+	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+	struct knfsd_fh *fh = &fhp->fh_handle;
+	siphash_key_t *fh_key = nn->fh_key;
+	u64 hash;
+
+	if (!(fhp->fh_export->ex_flags & NFSEXP_SIGN_FH))
+		return 0;
+
+	if (!fh_key) {
+		pr_warn_ratelimited("NFSD: unable to sign filehandles, fh_key not 
set.\n");
+		return -EINVAL;
+	}
+
+	if (fh->fh_size + sizeof(hash) > fhp->fh_maxsize) {
+		pr_warn_ratelimited("NFSD: unable to sign filehandles, fh_size %d 
would be greater"
+			" than fh_maxsize %d.\n", (int)(fh->fh_size + sizeof(hash)), 
fhp->fh_maxsize);
+		return -EINVAL;
+	}
+
+	fh->fh_auth_type = FH_AT_MAC;
+	hash = siphash(&fh->fh_raw, fh->fh_size, fh_key);
+	memcpy(&fh->fh_raw[fh->fh_size], &hash, sizeof(hash));
+	fh->fh_size += sizeof(hash);
+
+	return 0;
+}
+
+/*
+ * Verify that the the filehandle's MAC was hashed from this filehandle
+ * given the server's fh_key:
+ */
+static int fh_verify_mac(struct svc_fh *fhp, struct net *net)
+{
+	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+	struct knfsd_fh *fh = &fhp->fh_handle;
+	siphash_key_t *fh_key = nn->fh_key;
+	u64 hash;
+
+	if (fhp->fh_handle.fh_auth_type != FH_AT_MAC)
+		return -EINVAL;
+
+	if (!fh_key) {
+		pr_warn_ratelimited("NFSD: unable to verify signed filehandles, 
fh_key not set.\n");
+		return -EINVAL;
+	}
+
+	hash = siphash(&fh->fh_raw, fh->fh_size - sizeof(hash),  fh_key);
+	return crypto_memneq(&fh->fh_raw[fh->fh_size - sizeof(hash)], &hash, 
sizeof(hash));
+}
+
 /*
  * Use the given filehandle to look up the corresponding export and
  * dentry.  On success, the results are used to set fh_export and

@@ -166,8 +222,11 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst 

*rqstp, struct net *net,

 	if (--data_left < 0)
 		return error;
-	if (fh->fh_auth_type != 0)
+
+	/* either FH_AT_NONE or FH_AT_MAC */
+	if (fh->fh_auth_type > 1)
 		return error;
+
 	len = key_len(fh->fh_fsid_type) / 4;
 	if (len == 0)
 		return error;

@@ -237,9 +296,14 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst 

*rqstp, struct net *net,

 	fileid_type = fh->fh_fileid_type;

-	if (fileid_type == FILEID_ROOT)
+	if (fileid_type == FILEID_ROOT) {
 		dentry = dget(exp->ex_path.dentry);
-	else {
+	} else {
+		if (exp->ex_flags & NFSEXP_SIGN_FH && fh_verify_mac(fhp, net)) {
+			trace_nfsd_set_fh_dentry_badhandle(rqstp, fhp, -EKEYREJECTED);
+			goto out;
+		}
+
 		dentry = exportfs_decode_fh_raw(exp->ex_path.mnt, fid,
 						data_left, fileid_type, 0,
 						nfsd_acceptable, exp);

@@ -495,6 +559,9 @@ static void _fh_update(struct svc_fh *fhp, struct 

svc_export *exp,
 		fhp->fh_handle.fh_fileid_type =
 			fileid_type > 0 ? fileid_type : FILEID_INVALID;
 		fhp->fh_handle.fh_size += maxsize * 4;
+
+		if (fh_append_mac(fhp, exp->cd->net))
+			fhp->fh_handle.fh_fileid_type = FILEID_INVALID;
 	} else {
 		fhp->fh_handle.fh_fileid_type = FILEID_ROOT;
 	}

diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 5ef7191f8ad8..7fff46ac2ba8 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h

@@ -59,6 +59,9 @@ struct knfsd_fh {
 #define fh_fsid_type		fh_raw[2]
 #define fh_fileid_type		fh_raw[3]

+#define FH_AT_NONE		0
+#define FH_AT_MAC		1

I'm pleased at how much this patch has shrunk since v1.

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?

...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?

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.

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.


-- 
Chuck Lever