[PATCH v1 0/4] kNFSD Signed Filehandles

[PATCH v1 0/4] kNFSD Signed Filehandles

[Benjamin Coddington]

The following series enables the linux NFS server to add a Message
Authentication Code (MAC) to the filehandles it gives to clients.  This
provides additional protection to the exported filesystem against filehandle
guessing attacks.

Filesystems generate their own filehandles through the export_operation
"encode_fh" and a filehandle provides sufficient access to open a file
without needing to perform a lookup.  An NFS client holding a valid
filehandle can remotely open and read the contents of the file referred to
by the filehandle.

In order to acquire a filehandle, you must perform lookup operations on the
parent directory(ies), and the permissions on those directories may
prohibit you from walking into them to find the files within.  This would
normally be considered sufficient protection on a local filesystem to
prohibit users from accessing those files, however when the filesystem is
exported via NFS those files can still be accessed by guessing the correct,
valid filehandles.

Filehandles are easy to guess because they are well-formed.  The
open_by_handle_at(2) man page contains an example C program
(t_name_to_handle_at.c) that can display a filehandle given a path.  Here's
an example filehandle from a fairly modern XFS:

# ./t_name_to_handle_at /exports/foo 
57
12 129    99 00 00 00 00 00 00 00 b4 10 0b 8c

          ^---------  filehandle  ----------^
          ^------- inode -------^ ^-- gen --^

This filehandle consists of a 64-bit inode number and 32-bit generation
number.  Because the handle is well-formed, its easy to fabricate
filehandles that match other files within the same filesystem.  You can
simply insert inode numbers and iterate on the generation number.
Eventually you'll be able to access the file using open_by_handle_at(2).
For a local system, open_by_handle_at(2) requires CAP_DAC_READ_SEARCH, which
protects against guessing attacks by unprivileged users.

In contrast to a local user using open_by_handle(2), the NFS server must
permissively allow remote clients to open by filehandle without being able
to check or trust the remote caller's access. Therefore additional
protection against this attack is needed for NFS case.  We propose to sign
filehandles by appending an 8-byte MAC which is the siphash of the
filehandle from a key set from the nfs-utilities.  NFS server can then
ensure that guessing a valid filehandle+MAC is practically impossible
without knowledge of the MAC's key.  The NFS server performs optional
signing by possessing a key set from userspace and having the "sign_fh"
export option.

Because filehandles are long-lived, and there's no method for expiring them,
the server's key should be set once and not changed.  It also should be
persisted across restarts.  The methods to set the key allow only setting it
once, afterward it cannot be changed.  A separate patchset for nfs-utils
contains the userspace changes required to set the server's key.

I had a previous attempt to solve this problem by encrypting filehandles,
which turned out to be a problematic, poor solution.  The discussion on
that previous attempt is here:
https://lore.kernel.org/linux-nfs/510E10A4-11BE-412D-93AF-C4CC969954E7@hammerspace.com/T/#t

There are some changes from that version:
	- sign filehandles instead of encrypt them (Eric Biggers)
	- fix the NFSEXP_ macros, specifically NFSEXP_ALLFLAGS (NeilBrown)
	- rebase onto cel/nfsd-next (Chuck Lever)
	- condensed/clarified problem explantion (thanks Chuck Lever)
	- add nfsctl file "fh_key" for rpc.nfsd to also set the key

I plan on adding additional work to enable the server to check whether the
8-byte MAC will overflow maximum filehandle length for the protocol at
export time.  There could be some filesystems with 40-byte fileid and
24-byte fsid which would break NFSv3's 64-byte filehandle maximum with an
8-byte MAC appended.  The server should refuse to export those filesystems
when "sign_fh" is requested.

Thanks for any comments and critique.

Benjamin Coddington (4):
  nfsd: Convert export flags to use BIT() macro
  nfsd: Add a key for signing filehandles
  NFSD/export: Add sign_fh export option
  NFSD: Sign filehandles

 Documentation/netlink/specs/nfsd.yaml | 12 ++++
 fs/nfsd/export.c                      |  5 +-
 fs/nfsd/netlink.c                     | 15 +++++
 fs/nfsd/netlink.h                     |  1 +
 fs/nfsd/netns.h                       |  2 +
 fs/nfsd/nfs3xdr.c                     | 20 +++---
 fs/nfsd/nfs4xdr.c                     | 12 ++--
 fs/nfsd/nfsctl.c                      | 87 ++++++++++++++++++++++++++-
 fs/nfsd/nfsfh.c                       | 72 +++++++++++++++++++++-
 fs/nfsd/nfsfh.h                       | 22 +++++++
 fs/nfsd/trace.h                       | 19 ++++++
 include/linux/sunrpc/svc.h            |  1 +
 include/uapi/linux/nfsd/export.h      | 38 ++++++------
 include/uapi/linux/nfsd_netlink.h     |  2 +
 14 files changed, 272 insertions(+), 36 deletions(-)


base-commit: bfd453acb5637b5df881cef4b21803344aa9e7ac
-- 
2.50.1

[PATCH v1 1/4] nfsd: Convert export flags to use BIT() macro

[Benjamin Coddington]

Simplify these defines for consistency, readability, and clarity.

Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
 fs/nfsd/nfsctl.c                 |  2 +-
 include/uapi/linux/nfsd/export.h | 38 ++++++++++++++++----------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 30caefb2522f..8ccc65bb09fd 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -169,7 +169,7 @@ static const struct file_operations exports_nfsd_operations = {
 
 static int export_features_show(struct seq_file *m, void *v)
 {
-	seq_printf(m, "0x%x 0x%x\n", NFSEXP_ALLFLAGS, NFSEXP_SECINFO_FLAGS);
+	seq_printf(m, "0x%lx 0x%lx\n", NFSEXP_ALLFLAGS, NFSEXP_SECINFO_FLAGS);
 	return 0;
 }
 
diff --git a/include/uapi/linux/nfsd/export.h b/include/uapi/linux/nfsd/export.h
index a73ca3703abb..4e712bb02322 100644
--- a/include/uapi/linux/nfsd/export.h
+++ b/include/uapi/linux/nfsd/export.h
@@ -26,22 +26,22 @@
  * Please update the expflags[] array in fs/nfsd/export.c when adding
  * a new flag.
  */
-#define NFSEXP_READONLY		0x0001
-#define NFSEXP_INSECURE_PORT	0x0002
-#define NFSEXP_ROOTSQUASH	0x0004
-#define NFSEXP_ALLSQUASH	0x0008
-#define NFSEXP_ASYNC		0x0010
-#define NFSEXP_GATHERED_WRITES	0x0020
-#define NFSEXP_NOREADDIRPLUS    0x0040
-#define NFSEXP_SECURITY_LABEL	0x0080
-/* 0x100 currently unused */
-#define NFSEXP_NOHIDE		0x0200
-#define NFSEXP_NOSUBTREECHECK	0x0400
-#define	NFSEXP_NOAUTHNLM	0x0800		/* Don't authenticate NLM requests - just trust */
-#define NFSEXP_MSNFS		0x1000	/* do silly things that MS clients expect; no longer supported */
-#define NFSEXP_FSID		0x2000
-#define	NFSEXP_CROSSMOUNT	0x4000
-#define	NFSEXP_NOACL		0x8000	/* reserved for possible ACL related use */
+#define NFSEXP_READONLY			BIT(0)
+#define NFSEXP_INSECURE_PORT	BIT(1)
+#define NFSEXP_ROOTSQUASH		BIT(2)
+#define NFSEXP_ALLSQUASH		BIT(3)
+#define NFSEXP_ASYNC			BIT(4)
+#define NFSEXP_GATHERED_WRITES	BIT(5)
+#define NFSEXP_NOREADDIRPLUS    BIT(6)
+#define NFSEXP_SECURITY_LABEL	BIT(7)
+/* BIT(8) currently unused */
+#define NFSEXP_NOHIDE			BIT(9)
+#define NFSEXP_NOSUBTREECHECK	BIT(10)
+#define NFSEXP_NOAUTHNLM		BIT(11)	/* Don't authenticate NLM requests - just trust */
+#define NFSEXP_MSNFS			BIT(12)	/* do silly things that MS clients expect; no longer supported */
+#define NFSEXP_FSID				BIT(13)
+#define NFSEXP_CROSSMOUNT		BIT(14)
+#define NFSEXP_NOACL			BIT(15)	/* reserved for possible ACL related use */
 /*
  * The NFSEXP_V4ROOT flag causes the kernel to give access only to NFSv4
  * clients, and only to the single directory that is the root of the
@@ -51,11 +51,11 @@
  * pseudofilesystem, which provides access only to paths leading to each
  * exported filesystem.
  */
-#define	NFSEXP_V4ROOT		0x10000
-#define NFSEXP_PNFS		0x20000
+#define NFSEXP_V4ROOT			BIT(16)
+#define NFSEXP_PNFS				BIT(17)
 
 /* All flags that we claim to support.  (Note we don't support NOACL.) */
-#define NFSEXP_ALLFLAGS		0x3FEFF
+#define NFSEXP_ALLFLAGS			BIT(18) - BIT(8) - 1
 
 /* The flags that may vary depending on security flavor: */
 #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
-- 
2.50.1

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

[Benjamin Coddington]

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
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 887525964451..98100ee4bcd6 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -47,6 +47,14 @@ static const struct nla_policy nfsd_pool_mode_set_nl_policy[NFSD_A_POOL_MODE_MOD
 	[NFSD_A_POOL_MODE_MODE] = { .type = NLA_NUL_STRING, },
 };
 
+/* NFSD_CMD_FH_KEY_SET - do */
+static const struct nla_policy nfsd_fh_key_set_nl_policy[NFSD_A_SERVER_FH_KEY + 1] = {
+	[NFSD_A_SERVER_FH_KEY] = {
+		.type = NLA_BINARY,
+		.len = 16
+	},
+};
+
 /* Ops table for nfsd */
 static const struct genl_split_ops nfsd_nl_ops[] = {
 	{
@@ -102,6 +110,13 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
 		.doit	= nfsd_nl_pool_mode_get_doit,
 		.flags	= GENL_CMD_CAP_DO,
 	},
+	{
+		.cmd		= NFSD_CMD_FH_KEY_SET,
+		.doit		= nfsd_nl_fh_key_set_doit,
+		.policy		= nfsd_fh_key_set_nl_policy,
+		.maxattr	= NFSD_A_SERVER_FH_KEY,
+		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+	},
 };
 
 struct genl_family nfsd_nl_family __ro_after_init = {
diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
index 478117ff6b8c..84d578d628e8 100644
--- a/fs/nfsd/netlink.h
+++ b/fs/nfsd/netlink.h
@@ -26,6 +26,7 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info);
 int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info);
 int nfsd_nl_pool_mode_set_doit(struct sk_buff *skb, struct genl_info *info);
 int nfsd_nl_pool_mode_get_doit(struct sk_buff *skb, struct genl_info *info);
+int nfsd_nl_fh_key_set_doit(struct sk_buff *skb, struct genl_info *info);
 
 extern struct genl_family nfsd_nl_family;
 
diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 9fa600602658..c8ed733240a0 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -16,6 +16,7 @@
 #include <linux/percpu-refcount.h>
 #include <linux/siphash.h>
 #include <linux/sunrpc/stats.h>
+#include <linux/siphash.h>
 
 /* Hash tables for nfs4_clientid state */
 #define CLIENT_HASH_BITS                 4
@@ -224,6 +225,7 @@ struct nfsd_net {
 	spinlock_t              local_clients_lock;
 	struct list_head	local_clients;
 #endif
+	siphash_key_t		*fh_key;
 };
 
 /* Simple check to find out if a given net was properly initialized */
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 8ccc65bb09fd..aabd66468413 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/fsnotify.h>
 #include <linux/nfslocalio.h>
+#include <crypto/skcipher.h>
 
 #include "idmap.h"
 #include "nfsd.h"
@@ -49,6 +50,7 @@ enum {
 	NFSD_Ports,
 	NFSD_MaxBlkSize,
 	NFSD_MinThreads,
+	NFSD_Fh_Key,
 	NFSD_Filecache,
 	NFSD_Leasetime,
 	NFSD_Gracetime,
@@ -69,6 +71,7 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size);
 static ssize_t write_ports(struct file *file, char *buf, size_t size);
 static ssize_t write_maxblksize(struct file *file, char *buf, size_t size);
 static ssize_t write_minthreads(struct file *file, char *buf, size_t size);
+static ssize_t write_fh_key(struct file *file, char *buf, size_t size);
 #ifdef CONFIG_NFSD_V4
 static ssize_t write_leasetime(struct file *file, char *buf, size_t size);
 static ssize_t write_gracetime(struct file *file, char *buf, size_t size);
@@ -88,6 +91,7 @@ static ssize_t (*const write_op[])(struct file *, char *, size_t) = {
 	[NFSD_Ports] = write_ports,
 	[NFSD_MaxBlkSize] = write_maxblksize,
 	[NFSD_MinThreads] = write_minthreads,
+	[NFSD_Fh_Key] = write_fh_key,
 #ifdef CONFIG_NFSD_V4
 	[NFSD_Leasetime] = write_leasetime,
 	[NFSD_Gracetime] = write_gracetime,
@@ -950,6 +954,54 @@ static ssize_t write_minthreads(struct file *file, char *buf, size_t size)
 	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%u\n", minthreads);
 }
 
+/*
+ * write_fh_key - Set or report the current NFS filehandle key
+ *
+ * Input:
+ *			buf:		ignored
+ *			size:		zero
+ * OR
+ *
+ * Input:
+ *			buf:		C string containing a parseable UUID
+ *			size:		non-zero length of C string in @buf
+ * Output:
+ *	On success:	passed-in buffer filled with '\n'-terminated C string
+ *			containing the standard UUID format of the server's fh_key
+ *			return code is the size in bytes of the string
+ *	On error:	return code is zero or a negative errno value
+ */
+static ssize_t write_fh_key(struct file *file, char *buf, size_t size)
+{
+	struct nfsd_net *nn = net_generic(netns(file), nfsd_net_id);
+
+	if (size > 35 && size < 38) {
+		siphash_key_t *sip_fh_key;
+		uuid_t uuid_fh_key;
+		int ret;
+
+		/* Is the key already set? */
+		if (nn->fh_key)
+			return -EEXIST;
+
+		ret = uuid_parse(buf, &uuid_fh_key);
+		if (ret)
+			return ret;
+
+		sip_fh_key = kmalloc(sizeof(siphash_key_t), GFP_KERNEL);
+		if (!sip_fh_key)
+			return -ENOMEM;
+
+		memcpy(sip_fh_key, &uuid_fh_key, sizeof(siphash_key_t));
+		nn->fh_key = sip_fh_key;
+
+		trace_nfsd_ctl_fh_key_set((const char *)sip_fh_key, ret);
+	}
+
+	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%pUb\n",
+							nn->fh_key);
+}
+
 #ifdef CONFIG_NFSD_V4
 static ssize_t __nfsd4_write_time(struct file *file, char *buf, size_t size,
 				  time64_t *time, struct nfsd_net *nn)
@@ -1343,6 +1395,7 @@ static int nfsd_fill_super(struct super_block *sb, struct fs_context *fc)
 		[NFSD_Ports] = {"portlist", &transaction_ops, S_IWUSR|S_IRUGO},
 		[NFSD_MaxBlkSize] = {"max_block_size", &transaction_ops, S_IWUSR|S_IRUGO},
 		[NFSD_MinThreads] = {"min_threads", &transaction_ops, S_IWUSR|S_IRUGO},
+		[NFSD_Fh_Key] = {"fh_key", &transaction_ops, S_IWUSR|S_IRUSR},
 		[NFSD_Filecache] = {"filecache", &nfsd_file_cache_stats_fops, S_IRUGO},
 #ifdef CONFIG_NFSD_V4
 		[NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops, S_IWUSR|S_IRUSR},
@@ -2199,6 +2252,37 @@ int nfsd_nl_pool_mode_get_doit(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
+int nfsd_nl_fh_key_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	siphash_key_t *fh_key;
+	struct nfsd_net *nn;
+	int fh_key_len;
+	int ret;
+
+	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_FH_KEY))
+		return -EINVAL;
+
+	fh_key_len = nla_len(info->attrs[NFSD_A_SERVER_FH_KEY]);
+	if (fh_key_len != sizeof(siphash_key_t))
+		return -EINVAL;
+
+	/* Is the key already set? */
+	nn = net_generic(genl_info_net(info), nfsd_net_id);
+	if (nn->fh_key)
+		return -EEXIST;
+
+	fh_key = kmalloc(sizeof(siphash_key_t), GFP_KERNEL);
+	if (!fh_key)
+		return -ENOMEM;
+
+	memcpy(fh_key, nla_data(info->attrs[NFSD_A_SERVER_FH_KEY]), sizeof(siphash_key_t));
+	nn = net_generic(genl_info_net(info), nfsd_net_id);
+	nn->fh_key = fh_key;
+
+	trace_nfsd_ctl_fh_key_set((const char *)fh_key, ret);
+	return ret;
+}
+
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace
@@ -2284,6 +2368,7 @@ static __net_exit void nfsd_net_exit(struct net *net)
 {
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
 
+	kfree_sensitive(nn->fh_key);
 	nfsd_proc_stat_shutdown(net);
 	percpu_counter_destroy_many(nn->counter, NFSD_STATS_COUNTERS_NUM);
 	nfsd_idmap_shutdown(net);
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index d1d0b0dd0545..2e7d2a4cb7e7 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -2240,6 +2240,25 @@ TRACE_EVENT(nfsd_end_grace,
 	)
 );
 
+TRACE_EVENT(nfsd_ctl_fh_key_set,
+	TP_PROTO(
+		const char *key,
+		int result
+	),
+	TP_ARGS(key, result),
+	TP_STRUCT__entry(
+		__array(unsigned char, key, 16)
+		__field(unsigned long, result)
+	),
+	TP_fast_assign(
+		memcpy(__entry->key, key, 16);
+		__entry->result = result;
+	),
+	TP_printk("key=%s result=%ld", __print_hex(__entry->key, 16),
+		__entry->result
+	)
+);
+
 DECLARE_EVENT_CLASS(nfsd_copy_class,
 	TP_PROTO(
 		const struct nfsd4_copy *copy
diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
index e9efbc9e63d8..29e5d3d657ca 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -36,6 +36,7 @@ enum {
 	NFSD_A_SERVER_LEASETIME,
 	NFSD_A_SERVER_SCOPE,
 	NFSD_A_SERVER_MIN_THREADS,
+	NFSD_A_SERVER_FH_KEY,
 
 	__NFSD_A_SERVER_MAX,
 	NFSD_A_SERVER_MAX = (__NFSD_A_SERVER_MAX - 1)
@@ -90,6 +91,7 @@ enum {
 	NFSD_CMD_LISTENER_GET,
 	NFSD_CMD_POOL_MODE_SET,
 	NFSD_CMD_POOL_MODE_GET,
+	NFSD_CMD_FH_KEY_SET,
 
 	__NFSD_CMD_MAX,
 	NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
-- 
2.50.1

[PATCH v1 3/4] NFSD/export: Add sign_fh export option

[Benjamin Coddington]

Setting the "sign_fh" export option sets NFSEXP_SIGN_FH.  In a future patch
NFSD uses this signal to append a MAC onto filehandles for that export.

Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
 fs/nfsd/export.c                 | 5 +++--
 include/uapi/linux/nfsd/export.h | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 2a1499f2ad19..19c7a91c5373 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1349,13 +1349,14 @@ static struct flags {
 	{ NFSEXP_ASYNC, {"async", "sync"}},
 	{ NFSEXP_GATHERED_WRITES, {"wdelay", "no_wdelay"}},
 	{ NFSEXP_NOREADDIRPLUS, {"nordirplus", ""}},
+	{ NFSEXP_SECURITY_LABEL, {"security_label", ""}},
+	{ NFSEXP_SIGN_FH, {"sign_fh", ""}},
 	{ NFSEXP_NOHIDE, {"nohide", ""}},
-	{ NFSEXP_CROSSMOUNT, {"crossmnt", ""}},
 	{ NFSEXP_NOSUBTREECHECK, {"no_subtree_check", ""}},
 	{ NFSEXP_NOAUTHNLM, {"insecure_locks", ""}},
+	{ NFSEXP_CROSSMOUNT, {"crossmnt", ""}},
 	{ NFSEXP_V4ROOT, {"v4root", ""}},
 	{ NFSEXP_PNFS, {"pnfs", ""}},
-	{ NFSEXP_SECURITY_LABEL, {"security_label", ""}},
 	{ 0, {"", ""}}
 };
 
diff --git a/include/uapi/linux/nfsd/export.h b/include/uapi/linux/nfsd/export.h
index 4e712bb02322..6a73955fa5ba 100644
--- a/include/uapi/linux/nfsd/export.h
+++ b/include/uapi/linux/nfsd/export.h
@@ -34,7 +34,7 @@
 #define NFSEXP_GATHERED_WRITES	BIT(5)
 #define NFSEXP_NOREADDIRPLUS    BIT(6)
 #define NFSEXP_SECURITY_LABEL	BIT(7)
-/* BIT(8) currently unused */
+#define NFSEXP_SIGN_FH			BIT(8)
 #define NFSEXP_NOHIDE			BIT(9)
 #define NFSEXP_NOSUBTREECHECK	BIT(10)
 #define NFSEXP_NOAUTHNLM		BIT(11)	/* Don't authenticate NLM requests - just trust */
@@ -55,7 +55,7 @@
 #define NFSEXP_PNFS				BIT(17)
 
 /* All flags that we claim to support.  (Note we don't support NOACL.) */
-#define NFSEXP_ALLFLAGS			BIT(18) - BIT(8) - 1
+#define NFSEXP_ALLFLAGS			BIT(18) - 1
 
 /* The flags that may vary depending on security flavor: */
 #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
-- 
2.50.1

[PATCH v1 4/4] NFSD: Sign filehandles

[Benjamin Coddington]

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.

Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
 fs/nfsd/nfs3xdr.c          | 20 +++++++----
 fs/nfsd/nfs4xdr.c          | 12 ++++---
 fs/nfsd/nfsfh.c            | 72 ++++++++++++++++++++++++++++++++++++--
 fs/nfsd/nfsfh.h            | 22 ++++++++++++
 include/linux/sunrpc/svc.h |  1 +
 5 files changed, 113 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index ef4971d71ac4..f9d0c4892de7 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -120,11 +120,16 @@ svcxdr_encode_nfsstat3(struct xdr_stream *xdr, __be32 status)
 }
 
 static bool
-svcxdr_encode_nfs_fh3(struct xdr_stream *xdr, const struct svc_fh *fhp)
+svcxdr_encode_nfs_fh3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
+						struct svc_fh *fhp)
 {
-	u32 size = fhp->fh_handle.fh_size;
+	u32 size;
 	__be32 *p;
 
+	if (fh_append_mac(fhp, SVC_NET(rqstp)))
+		return false;
+	size = fhp->fh_handle.fh_size;
+
 	p = xdr_reserve_space(xdr, XDR_UNIT + size);
 	if (!p)
 		return false;
@@ -137,11 +142,12 @@ svcxdr_encode_nfs_fh3(struct xdr_stream *xdr, const struct svc_fh *fhp)
 }
 
 static bool
-svcxdr_encode_post_op_fh3(struct xdr_stream *xdr, const struct svc_fh *fhp)
+svcxdr_encode_post_op_fh3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
+							struct svc_fh *fhp)
 {
 	if (xdr_stream_encode_item_present(xdr) < 0)
 		return false;
-	if (!svcxdr_encode_nfs_fh3(xdr, fhp))
+	if (!svcxdr_encode_nfs_fh3(rqstp, xdr, fhp))
 		return false;
 
 	return true;
@@ -772,7 +778,7 @@ nfs3svc_encode_lookupres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
 		return false;
 	switch (resp->status) {
 	case nfs_ok:
-		if (!svcxdr_encode_nfs_fh3(xdr, &resp->fh))
+		if (!svcxdr_encode_nfs_fh3(rqstp, xdr, &resp->fh))
 			return false;
 		if (!svcxdr_encode_post_op_attr(rqstp, xdr, &resp->fh))
 			return false;
@@ -908,7 +914,7 @@ nfs3svc_encode_createres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
 		return false;
 	switch (resp->status) {
 	case nfs_ok:
-		if (!svcxdr_encode_post_op_fh3(xdr, &resp->fh))
+		if (!svcxdr_encode_post_op_fh3(rqstp, xdr, &resp->fh))
 			return false;
 		if (!svcxdr_encode_post_op_attr(rqstp, xdr, &resp->fh))
 			return false;
@@ -1117,7 +1123,7 @@ svcxdr_encode_entry3_plus(struct nfsd3_readdirres *resp, const char *name,
 
 	if (!svcxdr_encode_post_op_attr(resp->rqstp, xdr, fhp))
 		goto out;
-	if (!svcxdr_encode_post_op_fh3(xdr, fhp))
+	if (!svcxdr_encode_post_op_fh3(resp->rqstp, xdr, fhp))
 		goto out;
 	result = true;
 
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 884b792c95a3..f12981b989d1 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2701,9 +2701,13 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
 }
 
 static __be32 nfsd4_encode_nfs_fh4(struct xdr_stream *xdr,
-				   struct knfsd_fh *fh_handle)
+					struct svc_fh *fhp)
 {
-	return nfsd4_encode_opaque(xdr, fh_handle->fh_raw, fh_handle->fh_size);
+	if (fh_append_mac(fhp, SVC_NET(RESSTRM_RQST(xdr))))
+		return nfserr_resource;
+
+	return nfsd4_encode_opaque(xdr, fhp->fh_handle.fh_raw,
+		fhp->fh_handle.fh_size);
 }
 
 /* This is a frequently-encoded type; open-coded for speed */
@@ -3359,7 +3363,7 @@ static __be32 nfsd4_encode_fattr4_acl(struct xdr_stream *xdr,
 static __be32 nfsd4_encode_fattr4_filehandle(struct xdr_stream *xdr,
 					     const struct nfsd4_fattr_args *args)
 {
-	return nfsd4_encode_nfs_fh4(xdr, &args->fhp->fh_handle);
+	return nfsd4_encode_nfs_fh4(xdr, args->fhp);
 }
 
 static __be32 nfsd4_encode_fattr4_fileid(struct xdr_stream *xdr,
@@ -4460,7 +4464,7 @@ nfsd4_encode_getfh(struct nfsd4_compoundres *resp, __be32 nfserr,
 	struct svc_fh *fhp = u->getfh;
 
 	/* object */
-	return nfsd4_encode_nfs_fh4(xdr, &fhp->fh_handle);
+	return nfsd4_encode_nfs_fh4(xdr, fhp);
 }
 
 static __be32
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index ed85dd43da18..b2fb16b7f3c9 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/skcipher.h>
 #include "nfsd.h"
 #include "vfs.h"
 #include "auth.h"
@@ -137,6 +138,62 @@ static inline __be32 check_pseudo_root(struct dentry *dentry,
 	return nfs_ok;
 }
 
+/*
+ * Intended to be called when encoding, appends an 8-byte MAC
+ * to the filehandle hashed from the server's fh_key:
+ */
+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("NFSD: unable to sign filehandles, fh_key not set.\n");
+		return -EINVAL;
+	}
+
+	if (fh->fh_size + sizeof(hash) > fhp->fh_maxsize) {
+		pr_warn("NFSD: unable to sign filehandles, fh_size %lu would be greater"
+			" than fh_maxsize %d.\n", 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("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 memcmp(&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 +223,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 +297,15 @@ 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 {
+		/* Root filehandle always unsigned because rpc.mountd has no key */
+		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);
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 5ef7191f8ad8..d1ae272117f0 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
+
 static inline u32 *fh_fsid(const struct knfsd_fh *fh)
 {
 	return (u32 *)&fh->fh_raw[4];
@@ -226,6 +229,7 @@ __be32	fh_getattr(const struct svc_fh *fhp, struct kstat *stat);
 __be32	fh_compose(struct svc_fh *, struct svc_export *, struct dentry *, struct svc_fh *);
 __be32	fh_update(struct svc_fh *);
 void	fh_put(struct svc_fh *);
+int	fh_append_mac(struct svc_fh *, struct net *net);
 
 static __inline__ struct svc_fh *
 fh_copy(struct svc_fh *dst, const struct svc_fh *src)
@@ -274,6 +278,24 @@ static inline bool fh_fsid_match(const struct knfsd_fh *fh1,
 	return true;
 }
 
+static inline size_t fh_fileid_offset(const struct knfsd_fh *fh)
+{
+	return key_len(fh->fh_fsid_type) + 4;
+}
+
+static inline size_t fh_fileid_len(const struct knfsd_fh *fh)
+{
+	switch (fh->fh_auth_type) {
+	case FH_AT_NONE:
+		return fh->fh_size - fh_fileid_offset(fh);
+		break;
+	case FH_AT_MAC:
+		return fh->fh_size - 8 - fh_fileid_offset(fh);
+		break;
+	}
+	return 0;
+}
+
 /**
  * fh_want_write - Get write access to an export
  * @fhp: File handle of file to be written
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 62152e4f3bcc..96dae45d70ca 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -267,6 +267,7 @@ enum {
 };
 
 #define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : rqst->rq_bc_net)
+#define RESSTRM_RQST(xdr_stream) (container_of(xdr_stream, struct svc_rqst, rq_res_stream))
 
 /*
  * Rigorous type checking on sockaddr type conversions
-- 
2.50.1

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

[Jeff Layton]

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.
-- 
Jeff Layton <jlayton@kernel.org>

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

[Chuck Lever]

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.

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.


-- 
Chuck Lever

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

[Benjamin Coddington]

On 16 Jan 2026, at 10:09, 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.
>
> 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.

I'm hacking out a problem in the nfs-utils patches, should be coming soon -
but if the key is already set, the utils don't halt or report an error.

The patches have the key generated by hashing the contents of a file set via
nfs.conf: fh_key_file=/etc/nfs_fh.key for example.

No additional crypto benefit for having a different key per-export, because
the filehandles start with fsid the siphash naturally computes differently
for each export of a different filesystem.

By all means attack these points on the userspace patches.

Ben

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

[Jeff Layton]

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>

Re: [PATCH v1 1/4] nfsd: Convert export flags to use BIT() macro

[Chuck Lever]

On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
> Simplify these defines for consistency, readability, and clarity.
>
> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
> ---
>  fs/nfsd/nfsctl.c                 |  2 +-
>  include/uapi/linux/nfsd/export.h | 38 ++++++++++++++++----------------
>  2 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 30caefb2522f..8ccc65bb09fd 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -169,7 +169,7 @@ static const struct file_operations 
> exports_nfsd_operations = {
> 
>  static int export_features_show(struct seq_file *m, void *v)
>  {
> -	seq_printf(m, "0x%x 0x%x\n", NFSEXP_ALLFLAGS, NFSEXP_SECINFO_FLAGS);
> +	seq_printf(m, "0x%lx 0x%lx\n", NFSEXP_ALLFLAGS, NFSEXP_SECINFO_FLAGS);
>  	return 0;
>  }
> 
> diff --git a/include/uapi/linux/nfsd/export.h 
> b/include/uapi/linux/nfsd/export.h
> index a73ca3703abb..4e712bb02322 100644
> --- a/include/uapi/linux/nfsd/export.h
> +++ b/include/uapi/linux/nfsd/export.h
> @@ -26,22 +26,22 @@
>   * Please update the expflags[] array in fs/nfsd/export.c when adding
>   * a new flag.
>   */
> -#define NFSEXP_READONLY		0x0001
> -#define NFSEXP_INSECURE_PORT	0x0002
> -#define NFSEXP_ROOTSQUASH	0x0004
> -#define NFSEXP_ALLSQUASH	0x0008
> -#define NFSEXP_ASYNC		0x0010
> -#define NFSEXP_GATHERED_WRITES	0x0020
> -#define NFSEXP_NOREADDIRPLUS    0x0040
> -#define NFSEXP_SECURITY_LABEL	0x0080
> -/* 0x100 currently unused */
> -#define NFSEXP_NOHIDE		0x0200
> -#define NFSEXP_NOSUBTREECHECK	0x0400
> -#define	NFSEXP_NOAUTHNLM	0x0800		/* Don't authenticate NLM requests - 
> just trust */
> -#define NFSEXP_MSNFS		0x1000	/* do silly things that MS clients 
> expect; no longer supported */
> -#define NFSEXP_FSID		0x2000
> -#define	NFSEXP_CROSSMOUNT	0x4000
> -#define	NFSEXP_NOACL		0x8000	/* reserved for possible ACL related use 
> */
> +#define NFSEXP_READONLY			BIT(0)
> +#define NFSEXP_INSECURE_PORT	BIT(1)
> +#define NFSEXP_ROOTSQUASH		BIT(2)
> +#define NFSEXP_ALLSQUASH		BIT(3)
> +#define NFSEXP_ASYNC			BIT(4)
> +#define NFSEXP_GATHERED_WRITES	BIT(5)
> +#define NFSEXP_NOREADDIRPLUS    BIT(6)
> +#define NFSEXP_SECURITY_LABEL	BIT(7)
> +/* BIT(8) currently unused */
> +#define NFSEXP_NOHIDE			BIT(9)
> +#define NFSEXP_NOSUBTREECHECK	BIT(10)
> +#define NFSEXP_NOAUTHNLM		BIT(11)	/* Don't authenticate NLM requests - 
> just trust */
> +#define NFSEXP_MSNFS			BIT(12)	/* do silly things that MS clients 
> expect; no longer supported */
> +#define NFSEXP_FSID				BIT(13)
> +#define NFSEXP_CROSSMOUNT		BIT(14)
> +#define NFSEXP_NOACL			BIT(15)	/* reserved for possible ACL related 
> use */
>  /*
>   * The NFSEXP_V4ROOT flag causes the kernel to give access only to 
> NFSv4
>   * clients, and only to the single directory that is the root of the
> @@ -51,11 +51,11 @@
>   * pseudofilesystem, which provides access only to paths leading to 
> each
>   * exported filesystem.
>   */
> -#define	NFSEXP_V4ROOT		0x10000
> -#define NFSEXP_PNFS		0x20000
> +#define NFSEXP_V4ROOT			BIT(16)
> +#define NFSEXP_PNFS				BIT(17)
> 
>  /* All flags that we claim to support.  (Note we don't support NOACL.) */
> -#define NFSEXP_ALLFLAGS		0x3FEFF
> +#define NFSEXP_ALLFLAGS			BIT(18) - BIT(8) - 1
> 
>  /* The flags that may vary depending on security flavor: */
>  #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
> -- 
> 2.50.1

This might constitute a breaking user space API change. BIT() is
a kernel convention. What defines BIT() for user space consumers
of this header?


-- 
Chuck Lever

Re: [PATCH v1 1/4] nfsd: Convert export flags to use BIT() macro

[Benjamin Coddington]

On 16 Jan 2026, at 10:31, Chuck Lever wrote:

> On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
>> Simplify these defines for consistency, readability, and clarity.
>>
>> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
>> ---
>>  fs/nfsd/nfsctl.c                 |  2 +-
>>  include/uapi/linux/nfsd/export.h | 38 ++++++++++++++++----------------
>>  2 files changed, 20 insertions(+), 20 deletions(-)
>>
>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>> index 30caefb2522f..8ccc65bb09fd 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -169,7 +169,7 @@ static const struct file_operations
>> exports_nfsd_operations = {
>>
>>  static int export_features_show(struct seq_file *m, void *v)
>>  {
>> -	seq_printf(m, "0x%x 0x%x\n", NFSEXP_ALLFLAGS, NFSEXP_SECINFO_FLAGS);
>> +	seq_printf(m, "0x%lx 0x%lx\n", NFSEXP_ALLFLAGS, NFSEXP_SECINFO_FLAGS);
>>  	return 0;
>>  }
>>
>> diff --git a/include/uapi/linux/nfsd/export.h
>> b/include/uapi/linux/nfsd/export.h
>> index a73ca3703abb..4e712bb02322 100644
>> --- a/include/uapi/linux/nfsd/export.h
>> +++ b/include/uapi/linux/nfsd/export.h
>> @@ -26,22 +26,22 @@
>>   * Please update the expflags[] array in fs/nfsd/export.c when adding
>>   * a new flag.
>>   */
>> -#define NFSEXP_READONLY		0x0001
>> -#define NFSEXP_INSECURE_PORT	0x0002
>> -#define NFSEXP_ROOTSQUASH	0x0004
>> -#define NFSEXP_ALLSQUASH	0x0008
>> -#define NFSEXP_ASYNC		0x0010
>> -#define NFSEXP_GATHERED_WRITES	0x0020
>> -#define NFSEXP_NOREADDIRPLUS    0x0040
>> -#define NFSEXP_SECURITY_LABEL	0x0080
>> -/* 0x100 currently unused */
>> -#define NFSEXP_NOHIDE		0x0200
>> -#define NFSEXP_NOSUBTREECHECK	0x0400
>> -#define	NFSEXP_NOAUTHNLM	0x0800		/* Don't authenticate NLM requests -
>> just trust */
>> -#define NFSEXP_MSNFS		0x1000	/* do silly things that MS clients
>> expect; no longer supported */
>> -#define NFSEXP_FSID		0x2000
>> -#define	NFSEXP_CROSSMOUNT	0x4000
>> -#define	NFSEXP_NOACL		0x8000	/* reserved for possible ACL related use
>> */
>> +#define NFSEXP_READONLY			BIT(0)
>> +#define NFSEXP_INSECURE_PORT	BIT(1)
>> +#define NFSEXP_ROOTSQUASH		BIT(2)
>> +#define NFSEXP_ALLSQUASH		BIT(3)
>> +#define NFSEXP_ASYNC			BIT(4)
>> +#define NFSEXP_GATHERED_WRITES	BIT(5)
>> +#define NFSEXP_NOREADDIRPLUS    BIT(6)
>> +#define NFSEXP_SECURITY_LABEL	BIT(7)
>> +/* BIT(8) currently unused */
>> +#define NFSEXP_NOHIDE			BIT(9)
>> +#define NFSEXP_NOSUBTREECHECK	BIT(10)
>> +#define NFSEXP_NOAUTHNLM		BIT(11)	/* Don't authenticate NLM requests -
>> just trust */
>> +#define NFSEXP_MSNFS			BIT(12)	/* do silly things that MS clients
>> expect; no longer supported */
>> +#define NFSEXP_FSID				BIT(13)
>> +#define NFSEXP_CROSSMOUNT		BIT(14)
>> +#define NFSEXP_NOACL			BIT(15)	/* reserved for possible ACL related
>> use */
>>  /*
>>   * The NFSEXP_V4ROOT flag causes the kernel to give access only to
>> NFSv4
>>   * clients, and only to the single directory that is the root of the
>> @@ -51,11 +51,11 @@
>>   * pseudofilesystem, which provides access only to paths leading to
>> each
>>   * exported filesystem.
>>   */
>> -#define	NFSEXP_V4ROOT		0x10000
>> -#define NFSEXP_PNFS		0x20000
>> +#define NFSEXP_V4ROOT			BIT(16)
>> +#define NFSEXP_PNFS				BIT(17)
>>
>>  /* All flags that we claim to support.  (Note we don't support NOACL.) */
>> -#define NFSEXP_ALLFLAGS		0x3FEFF
>> +#define NFSEXP_ALLFLAGS			BIT(18) - BIT(8) - 1
>>
>>  /* The flags that may vary depending on security flavor: */
>>  #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
>> -- 
>> 2.50.1
>
> This might constitute a breaking user space API change. BIT() is
> a kernel convention. What defines BIT() for user space consumers
> of this header?

Doh - good catch.  I can drop this, or maybe add:

#ifndef BIT
#define BIT(n) (1UL << (n))
#endif

Ben

Re: [PATCH v1 1/4] nfsd: Convert export flags to use BIT() macro

[Chuck Lever]

On 1/16/26 10:35 AM, Benjamin Coddington wrote:
> On 16 Jan 2026, at 10:31, Chuck Lever wrote:
> 
>> On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
>>> Simplify these defines for consistency, readability, and clarity.
>>>
>>> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
>>> ---
>>>  fs/nfsd/nfsctl.c                 |  2 +-
>>>  include/uapi/linux/nfsd/export.h | 38 ++++++++++++++++----------------
>>>  2 files changed, 20 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>>> index 30caefb2522f..8ccc65bb09fd 100644
>>> --- a/fs/nfsd/nfsctl.c
>>> +++ b/fs/nfsd/nfsctl.c
>>> @@ -169,7 +169,7 @@ static const struct file_operations
>>> exports_nfsd_operations = {
>>>
>>>  static int export_features_show(struct seq_file *m, void *v)
>>>  {
>>> -	seq_printf(m, "0x%x 0x%x\n", NFSEXP_ALLFLAGS, NFSEXP_SECINFO_FLAGS);
>>> +	seq_printf(m, "0x%lx 0x%lx\n", NFSEXP_ALLFLAGS, NFSEXP_SECINFO_FLAGS);
>>>  	return 0;
>>>  }
>>>
>>> diff --git a/include/uapi/linux/nfsd/export.h
>>> b/include/uapi/linux/nfsd/export.h
>>> index a73ca3703abb..4e712bb02322 100644
>>> --- a/include/uapi/linux/nfsd/export.h
>>> +++ b/include/uapi/linux/nfsd/export.h
>>> @@ -26,22 +26,22 @@
>>>   * Please update the expflags[] array in fs/nfsd/export.c when adding
>>>   * a new flag.
>>>   */
>>> -#define NFSEXP_READONLY		0x0001
>>> -#define NFSEXP_INSECURE_PORT	0x0002
>>> -#define NFSEXP_ROOTSQUASH	0x0004
>>> -#define NFSEXP_ALLSQUASH	0x0008
>>> -#define NFSEXP_ASYNC		0x0010
>>> -#define NFSEXP_GATHERED_WRITES	0x0020
>>> -#define NFSEXP_NOREADDIRPLUS    0x0040
>>> -#define NFSEXP_SECURITY_LABEL	0x0080
>>> -/* 0x100 currently unused */
>>> -#define NFSEXP_NOHIDE		0x0200
>>> -#define NFSEXP_NOSUBTREECHECK	0x0400
>>> -#define	NFSEXP_NOAUTHNLM	0x0800		/* Don't authenticate NLM requests -
>>> just trust */
>>> -#define NFSEXP_MSNFS		0x1000	/* do silly things that MS clients
>>> expect; no longer supported */
>>> -#define NFSEXP_FSID		0x2000
>>> -#define	NFSEXP_CROSSMOUNT	0x4000
>>> -#define	NFSEXP_NOACL		0x8000	/* reserved for possible ACL related use
>>> */
>>> +#define NFSEXP_READONLY			BIT(0)
>>> +#define NFSEXP_INSECURE_PORT	BIT(1)
>>> +#define NFSEXP_ROOTSQUASH		BIT(2)
>>> +#define NFSEXP_ALLSQUASH		BIT(3)
>>> +#define NFSEXP_ASYNC			BIT(4)
>>> +#define NFSEXP_GATHERED_WRITES	BIT(5)
>>> +#define NFSEXP_NOREADDIRPLUS    BIT(6)
>>> +#define NFSEXP_SECURITY_LABEL	BIT(7)
>>> +/* BIT(8) currently unused */
>>> +#define NFSEXP_NOHIDE			BIT(9)
>>> +#define NFSEXP_NOSUBTREECHECK	BIT(10)
>>> +#define NFSEXP_NOAUTHNLM		BIT(11)	/* Don't authenticate NLM requests -
>>> just trust */
>>> +#define NFSEXP_MSNFS			BIT(12)	/* do silly things that MS clients
>>> expect; no longer supported */
>>> +#define NFSEXP_FSID				BIT(13)
>>> +#define NFSEXP_CROSSMOUNT		BIT(14)
>>> +#define NFSEXP_NOACL			BIT(15)	/* reserved for possible ACL related
>>> use */
>>>  /*
>>>   * The NFSEXP_V4ROOT flag causes the kernel to give access only to
>>> NFSv4
>>>   * clients, and only to the single directory that is the root of the
>>> @@ -51,11 +51,11 @@
>>>   * pseudofilesystem, which provides access only to paths leading to
>>> each
>>>   * exported filesystem.
>>>   */
>>> -#define	NFSEXP_V4ROOT		0x10000
>>> -#define NFSEXP_PNFS		0x20000
>>> +#define NFSEXP_V4ROOT			BIT(16)
>>> +#define NFSEXP_PNFS				BIT(17)
>>>
>>>  /* All flags that we claim to support.  (Note we don't support NOACL.) */
>>> -#define NFSEXP_ALLFLAGS		0x3FEFF
>>> +#define NFSEXP_ALLFLAGS			BIT(18) - BIT(8) - 1
>>>
>>>  /* The flags that may vary depending on security flavor: */
>>>  #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
>>> -- 
>>> 2.50.1
>>
>> This might constitute a breaking user space API change. BIT() is
>> a kernel convention. What defines BIT() for user space consumers
>> of this header?
> 
> Doh - good catch.  I can drop this, or maybe add:
> 
> #ifndef BIT
> #define BIT(n) (1UL << (n))
> #endif
I'm happy leaving these as hex constants.


-- 
Chuck Lever

Re: [PATCH v1 1/4] nfsd: Convert export flags to use BIT() macro

[Benjamin Coddington]

On 16 Jan 2026, at 10:38, Chuck Lever wrote:

> On 1/16/26 10:35 AM, Benjamin Coddington wrote:
>> On 16 Jan 2026, at 10:31, Chuck Lever wrote:
>>
>>> On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
>>>> Simplify these defines for consistency, readability, and clarity.
>>>>
>>>> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
>>>> ---
>>>>  fs/nfsd/nfsctl.c                 |  2 +-
>>>>  include/uapi/linux/nfsd/export.h | 38 ++++++++++++++++----------------
>>>>  2 files changed, 20 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>>>> index 30caefb2522f..8ccc65bb09fd 100644
>>>> --- a/fs/nfsd/nfsctl.c
>>>> +++ b/fs/nfsd/nfsctl.c
>>>> @@ -169,7 +169,7 @@ static const struct file_operations
>>>> exports_nfsd_operations = {
>>>>
>>>>  static int export_features_show(struct seq_file *m, void *v)
>>>>  {
>>>> -	seq_printf(m, "0x%x 0x%x\n", NFSEXP_ALLFLAGS, NFSEXP_SECINFO_FLAGS);
>>>> +	seq_printf(m, "0x%lx 0x%lx\n", NFSEXP_ALLFLAGS, NFSEXP_SECINFO_FLAGS);
>>>>  	return 0;
>>>>  }
>>>>
>>>> diff --git a/include/uapi/linux/nfsd/export.h
>>>> b/include/uapi/linux/nfsd/export.h
>>>> index a73ca3703abb..4e712bb02322 100644
>>>> --- a/include/uapi/linux/nfsd/export.h
>>>> +++ b/include/uapi/linux/nfsd/export.h
>>>> @@ -26,22 +26,22 @@
>>>>   * Please update the expflags[] array in fs/nfsd/export.c when adding
>>>>   * a new flag.
>>>>   */
>>>> -#define NFSEXP_READONLY		0x0001
>>>> -#define NFSEXP_INSECURE_PORT	0x0002
>>>> -#define NFSEXP_ROOTSQUASH	0x0004
>>>> -#define NFSEXP_ALLSQUASH	0x0008
>>>> -#define NFSEXP_ASYNC		0x0010
>>>> -#define NFSEXP_GATHERED_WRITES	0x0020
>>>> -#define NFSEXP_NOREADDIRPLUS    0x0040
>>>> -#define NFSEXP_SECURITY_LABEL	0x0080
>>>> -/* 0x100 currently unused */
>>>> -#define NFSEXP_NOHIDE		0x0200
>>>> -#define NFSEXP_NOSUBTREECHECK	0x0400
>>>> -#define	NFSEXP_NOAUTHNLM	0x0800		/* Don't authenticate NLM requests -
>>>> just trust */
>>>> -#define NFSEXP_MSNFS		0x1000	/* do silly things that MS clients
>>>> expect; no longer supported */
>>>> -#define NFSEXP_FSID		0x2000
>>>> -#define	NFSEXP_CROSSMOUNT	0x4000
>>>> -#define	NFSEXP_NOACL		0x8000	/* reserved for possible ACL related use
>>>> */
>>>> +#define NFSEXP_READONLY			BIT(0)
>>>> +#define NFSEXP_INSECURE_PORT	BIT(1)
>>>> +#define NFSEXP_ROOTSQUASH		BIT(2)
>>>> +#define NFSEXP_ALLSQUASH		BIT(3)
>>>> +#define NFSEXP_ASYNC			BIT(4)
>>>> +#define NFSEXP_GATHERED_WRITES	BIT(5)
>>>> +#define NFSEXP_NOREADDIRPLUS    BIT(6)
>>>> +#define NFSEXP_SECURITY_LABEL	BIT(7)
>>>> +/* BIT(8) currently unused */
>>>> +#define NFSEXP_NOHIDE			BIT(9)
>>>> +#define NFSEXP_NOSUBTREECHECK	BIT(10)
>>>> +#define NFSEXP_NOAUTHNLM		BIT(11)	/* Don't authenticate NLM requests -
>>>> just trust */
>>>> +#define NFSEXP_MSNFS			BIT(12)	/* do silly things that MS clients
>>>> expect; no longer supported */
>>>> +#define NFSEXP_FSID				BIT(13)
>>>> +#define NFSEXP_CROSSMOUNT		BIT(14)
>>>> +#define NFSEXP_NOACL			BIT(15)	/* reserved for possible ACL related
>>>> use */
>>>>  /*
>>>>   * The NFSEXP_V4ROOT flag causes the kernel to give access only to
>>>> NFSv4
>>>>   * clients, and only to the single directory that is the root of the
>>>> @@ -51,11 +51,11 @@
>>>>   * pseudofilesystem, which provides access only to paths leading to
>>>> each
>>>>   * exported filesystem.
>>>>   */
>>>> -#define	NFSEXP_V4ROOT		0x10000
>>>> -#define NFSEXP_PNFS		0x20000
>>>> +#define NFSEXP_V4ROOT			BIT(16)
>>>> +#define NFSEXP_PNFS				BIT(17)
>>>>
>>>>  /* All flags that we claim to support.  (Note we don't support NOACL.) */
>>>> -#define NFSEXP_ALLFLAGS		0x3FEFF
>>>> +#define NFSEXP_ALLFLAGS			BIT(18) - BIT(8) - 1
>>>>
>>>>  /* The flags that may vary depending on security flavor: */
>>>>  #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
>>>> -- 
>>>> 2.50.1
>>>
>>> This might constitute a breaking user space API change. BIT() is
>>> a kernel convention. What defines BIT() for user space consumers
>>> of this header?
>>
>> Doh - good catch.  I can drop this, or maybe add:
>>
>> #ifndef BIT
>> #define BIT(n) (1UL << (n))
>> #endif
> I'm happy leaving these as hex constants.

Got it.  I'll drop this on the next version.

Ben

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

[Chuck Lever]

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.


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

-- 
Chuck Lever

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

[Jeff Layton]

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>

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

[Chuck Lever]

On Fri, Jan 16, 2026, at 9:32 AM, 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.

Generally I like to see more "why" in the commit message. This
message just repeats what the diff says.

Since the actual rationale will be lengthy, I would say this is
one of those rare occasions when including a Link: tag that refers
the reader to the cover letter in the lore archive might be helpful.


> 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

Nit: "set signing key for filehandles"


> +      attribute-set: server
> +      flags: [admin-perm]
> +      do:
> +        request:
> +          attributes:
> +            - fh-key
> diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> index 887525964451..98100ee4bcd6 100644
> --- a/fs/nfsd/netlink.c
> +++ b/fs/nfsd/netlink.c
> @@ -47,6 +47,14 @@ static const struct nla_policy 
> nfsd_pool_mode_set_nl_policy[NFSD_A_POOL_MODE_MOD
>  	[NFSD_A_POOL_MODE_MODE] = { .type = NLA_NUL_STRING, },
>  };
> 
> +/* NFSD_CMD_FH_KEY_SET - do */
> +static const struct nla_policy 
> nfsd_fh_key_set_nl_policy[NFSD_A_SERVER_FH_KEY + 1] = {
> +	[NFSD_A_SERVER_FH_KEY] = {
> +		.type = NLA_BINARY,
> +		.len = 16
> +	},
> +};
> +
>  /* Ops table for nfsd */
>  static const struct genl_split_ops nfsd_nl_ops[] = {
>  	{
> @@ -102,6 +110,13 @@ static const struct genl_split_ops nfsd_nl_ops[] = 
> {
>  		.doit	= nfsd_nl_pool_mode_get_doit,
>  		.flags	= GENL_CMD_CAP_DO,
>  	},
> +	{
> +		.cmd		= NFSD_CMD_FH_KEY_SET,
> +		.doit		= nfsd_nl_fh_key_set_doit,
> +		.policy		= nfsd_fh_key_set_nl_policy,
> +		.maxattr	= NFSD_A_SERVER_FH_KEY,
> +		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> +	},
>  };
> 
>  struct genl_family nfsd_nl_family __ro_after_init = {
> diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
> index 478117ff6b8c..84d578d628e8 100644
> --- a/fs/nfsd/netlink.h
> +++ b/fs/nfsd/netlink.h
> @@ -26,6 +26,7 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, 
> struct genl_info *info);
>  int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info 
> *info);
>  int nfsd_nl_pool_mode_set_doit(struct sk_buff *skb, struct genl_info 
> *info);
>  int nfsd_nl_pool_mode_get_doit(struct sk_buff *skb, struct genl_info 
> *info);
> +int nfsd_nl_fh_key_set_doit(struct sk_buff *skb, struct genl_info 
> *info);
> 
>  extern struct genl_family nfsd_nl_family;
> 
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index 9fa600602658..c8ed733240a0 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -16,6 +16,7 @@
>  #include <linux/percpu-refcount.h>
>  #include <linux/siphash.h>
>  #include <linux/sunrpc/stats.h>
> +#include <linux/siphash.h>
> 
>  /* Hash tables for nfs4_clientid state */
>  #define CLIENT_HASH_BITS                 4
> @@ -224,6 +225,7 @@ struct nfsd_net {
>  	spinlock_t              local_clients_lock;
>  	struct list_head	local_clients;
>  #endif
> +	siphash_key_t		*fh_key;
>  };
> 
>  /* Simple check to find out if a given net was properly initialized */
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 8ccc65bb09fd..aabd66468413 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -19,6 +19,7 @@
>  #include <linux/module.h>
>  #include <linux/fsnotify.h>
>  #include <linux/nfslocalio.h>
> +#include <crypto/skcipher.h>
> 
>  #include "idmap.h"
>  #include "nfsd.h"
> @@ -49,6 +50,7 @@ enum {
>  	NFSD_Ports,
>  	NFSD_MaxBlkSize,
>  	NFSD_MinThreads,
> +	NFSD_Fh_Key,
>  	NFSD_Filecache,
>  	NFSD_Leasetime,
>  	NFSD_Gracetime,
> @@ -69,6 +71,7 @@ static ssize_t write_versions(struct file *file, char 
> *buf, size_t size);
>  static ssize_t write_ports(struct file *file, char *buf, size_t size);
>  static ssize_t write_maxblksize(struct file *file, char *buf, size_t 
> size);
>  static ssize_t write_minthreads(struct file *file, char *buf, size_t 
> size);
> +static ssize_t write_fh_key(struct file *file, char *buf, size_t size);
>  #ifdef CONFIG_NFSD_V4
>  static ssize_t write_leasetime(struct file *file, char *buf, size_t 
> size);
>  static ssize_t write_gracetime(struct file *file, char *buf, size_t 
> size);
> @@ -88,6 +91,7 @@ static ssize_t (*const write_op[])(struct file *, 
> char *, size_t) = {
>  	[NFSD_Ports] = write_ports,
>  	[NFSD_MaxBlkSize] = write_maxblksize,
>  	[NFSD_MinThreads] = write_minthreads,
> +	[NFSD_Fh_Key] = write_fh_key,
>  #ifdef CONFIG_NFSD_V4
>  	[NFSD_Leasetime] = write_leasetime,
>  	[NFSD_Gracetime] = write_gracetime,
> @@ -950,6 +954,54 @@ static ssize_t write_minthreads(struct file *file, 
> char *buf, size_t size)
>  	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%u\n", minthreads);
>  }
> 
> +/*
> + * write_fh_key - Set or report the current NFS filehandle key
> + *
> + * Input:
> + *			buf:		ignored
> + *			size:		zero
> + * OR
> + *
> + * Input:
> + *			buf:		C string containing a parseable UUID
> + *			size:		non-zero length of C string in @buf
> + * Output:
> + *	On success:	passed-in buffer filled with '\n'-terminated C string
> + *			containing the standard UUID format of the server's fh_key
> + *			return code is the size in bytes of the string
> + *	On error:	return code is zero or a negative errno value

Nit: it would be nice to explain (briefly) why fh_key rotation during
server operation is prohibited.


> + */
> +static ssize_t write_fh_key(struct file *file, char *buf, size_t size)
> +{
> +	struct nfsd_net *nn = net_generic(netns(file), nfsd_net_id);
> +
> +	if (size > 35 && size < 38) {
> +		siphash_key_t *sip_fh_key;
> +		uuid_t uuid_fh_key;
> +		int ret;
> +
> +		/* Is the key already set? */
> +		if (nn->fh_key)
> +			return -EEXIST;
> +
> +		ret = uuid_parse(buf, &uuid_fh_key);
> +		if (ret)
> +			return ret;
> +
> +		sip_fh_key = kmalloc(sizeof(siphash_key_t), GFP_KERNEL);
> +		if (!sip_fh_key)
> +			return -ENOMEM;
> +
> +		memcpy(sip_fh_key, &uuid_fh_key, sizeof(siphash_key_t));

What protects updates of nn->fh_key from concurrent writers?
A race might result in leaking a previous fh_key buffer, when
it should return EEXIST. So you'll need some mutual exclusion
here -- probably nfsd_mutex.

Ditto for the netlink interface.


> +		nn->fh_key = sip_fh_key;
> +
> +		trace_nfsd_ctl_fh_key_set((const char *)sip_fh_key, ret);
> +	}
> +

If user space reads the key before it has been set, would
nn->fh_key be NULL here?


> +	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%pUb\n",
> +							nn->fh_key);
> +}
> +
>  #ifdef CONFIG_NFSD_V4
>  static ssize_t __nfsd4_write_time(struct file *file, char *buf, size_t 
> size,
>  				  time64_t *time, struct nfsd_net *nn)
> @@ -1343,6 +1395,7 @@ static int nfsd_fill_super(struct super_block 
> *sb, struct fs_context *fc)
>  		[NFSD_Ports] = {"portlist", &transaction_ops, S_IWUSR|S_IRUGO},
>  		[NFSD_MaxBlkSize] = {"max_block_size", &transaction_ops, 
> S_IWUSR|S_IRUGO},
>  		[NFSD_MinThreads] = {"min_threads", &transaction_ops, 
> S_IWUSR|S_IRUGO},
> +		[NFSD_Fh_Key] = {"fh_key", &transaction_ops, S_IWUSR|S_IRUSR},
>  		[NFSD_Filecache] = {"filecache", &nfsd_file_cache_stats_fops, 
> S_IRUGO},
>  #ifdef CONFIG_NFSD_V4
>  		[NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops, 
> S_IWUSR|S_IRUSR},
> @@ -2199,6 +2252,37 @@ int nfsd_nl_pool_mode_get_doit(struct sk_buff 
> *skb, struct genl_info *info)
>  	return err;
>  }
> 
> +int nfsd_nl_fh_key_set_doit(struct sk_buff *skb, struct genl_info 
> *info)
> +{
> +	siphash_key_t *fh_key;
> +	struct nfsd_net *nn;
> +	int fh_key_len;
> +	int ret;
> +
> +	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_FH_KEY))
> +		return -EINVAL;
> +
> +	fh_key_len = nla_len(info->attrs[NFSD_A_SERVER_FH_KEY]);
> +	if (fh_key_len != sizeof(siphash_key_t))
> +		return -EINVAL;
> +
> +	/* Is the key already set? */
> +	nn = net_generic(genl_info_net(info), nfsd_net_id);
> +	if (nn->fh_key)
> +		return -EEXIST;
> +
> +	fh_key = kmalloc(sizeof(siphash_key_t), GFP_KERNEL);
> +	if (!fh_key)
> +		return -ENOMEM;
> +
> +	memcpy(fh_key, nla_data(info->attrs[NFSD_A_SERVER_FH_KEY]), 
> sizeof(siphash_key_t));
> +	nn = net_generic(genl_info_net(info), nfsd_net_id);
> +	nn->fh_key = fh_key;
> +

On success, "ret" hasn't been initialized. And maybe you want
the error flows above to exit through here to get those return
codes captured by the trace point.

> +	trace_nfsd_ctl_fh_key_set((const char *)fh_key, ret);
> +	return ret;
> +}
> +
>  /**
>   * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
>   * @net: a freshly-created network namespace
> @@ -2284,6 +2368,7 @@ static __net_exit void nfsd_net_exit(struct net 
> *net)
>  {
>  	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> 

To save an extra pointer dereference on the hotter paths, maybe
fh_key should be the actual key rather than a pointer. You can
use kfree_sensitive() when freeing the whole struct nfsd_net, or
just memset the fh_key field before freeing nfsd_net.

Just a random thought.


> +	kfree_sensitive(nn->fh_key);
>  	nfsd_proc_stat_shutdown(net);
>  	percpu_counter_destroy_many(nn->counter, NFSD_STATS_COUNTERS_NUM);
>  	nfsd_idmap_shutdown(net);
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index d1d0b0dd0545..2e7d2a4cb7e7 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -2240,6 +2240,25 @@ TRACE_EVENT(nfsd_end_grace,
>  	)
>  );
> 
> +TRACE_EVENT(nfsd_ctl_fh_key_set,
> +	TP_PROTO(
> +		const char *key,
> +		int result
> +	),
> +	TP_ARGS(key, result),
> +	TP_STRUCT__entry(
> +		__array(unsigned char, key, 16)
> +		__field(unsigned long, result)
> +	),
> +	TP_fast_assign(
> +		memcpy(__entry->key, key, 16);
> +		__entry->result = result;
> +	),
> +	TP_printk("key=%s result=%ld", __print_hex(__entry->key, 16),
> +		__entry->result
> +	)
> +);
> +
>  DECLARE_EVENT_CLASS(nfsd_copy_class,
>  	TP_PROTO(
>  		const struct nfsd4_copy *copy
> diff --git a/include/uapi/linux/nfsd_netlink.h 
> b/include/uapi/linux/nfsd_netlink.h
> index e9efbc9e63d8..29e5d3d657ca 100644
> --- a/include/uapi/linux/nfsd_netlink.h
> +++ b/include/uapi/linux/nfsd_netlink.h
> @@ -36,6 +36,7 @@ enum {
>  	NFSD_A_SERVER_LEASETIME,
>  	NFSD_A_SERVER_SCOPE,
>  	NFSD_A_SERVER_MIN_THREADS,
> +	NFSD_A_SERVER_FH_KEY,
> 
>  	__NFSD_A_SERVER_MAX,
>  	NFSD_A_SERVER_MAX = (__NFSD_A_SERVER_MAX - 1)
> @@ -90,6 +91,7 @@ enum {
>  	NFSD_CMD_LISTENER_GET,
>  	NFSD_CMD_POOL_MODE_SET,
>  	NFSD_CMD_POOL_MODE_GET,
> +	NFSD_CMD_FH_KEY_SET,
> 
>  	__NFSD_CMD_MAX,
>  	NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
> -- 
> 2.50.1

-- 
Chuck Lever

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

[Chuck Lever]

On Fri, Jan 16, 2026, at 10:52 AM, Jeff Layton wrote:
> 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.

OK, you rode herd on getting the netlink stuff in, so it's
your call here.

It might be nice to create a new op called "service" and leave
the legacy "threads" op in place for now.


-- 
Chuck Lever

Re: [PATCH v1 3/4] NFSD/export: Add sign_fh export option

[Chuck Lever]

On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
> Setting the "sign_fh" export option sets NFSEXP_SIGN_FH.  In a future patch
> NFSD uses this signal to append a MAC onto filehandles for that export.

Same comment on missing "why" and the use of a Link: tag

Also, let's see some motivation for re-ordering the export flag
table.


> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
> ---
>  fs/nfsd/export.c                 | 5 +++--
>  include/uapi/linux/nfsd/export.h | 4 ++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 2a1499f2ad19..19c7a91c5373 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -1349,13 +1349,14 @@ static struct flags {
>  	{ NFSEXP_ASYNC, {"async", "sync"}},
>  	{ NFSEXP_GATHERED_WRITES, {"wdelay", "no_wdelay"}},
>  	{ NFSEXP_NOREADDIRPLUS, {"nordirplus", ""}},
> +	{ NFSEXP_SECURITY_LABEL, {"security_label", ""}},
> +	{ NFSEXP_SIGN_FH, {"sign_fh", ""}},
>  	{ NFSEXP_NOHIDE, {"nohide", ""}},
> -	{ NFSEXP_CROSSMOUNT, {"crossmnt", ""}},
>  	{ NFSEXP_NOSUBTREECHECK, {"no_subtree_check", ""}},
>  	{ NFSEXP_NOAUTHNLM, {"insecure_locks", ""}},
> +	{ NFSEXP_CROSSMOUNT, {"crossmnt", ""}},
>  	{ NFSEXP_V4ROOT, {"v4root", ""}},
>  	{ NFSEXP_PNFS, {"pnfs", ""}},
> -	{ NFSEXP_SECURITY_LABEL, {"security_label", ""}},
>  	{ 0, {"", ""}}
>  };
> 
> diff --git a/include/uapi/linux/nfsd/export.h 
> b/include/uapi/linux/nfsd/export.h
> index 4e712bb02322..6a73955fa5ba 100644
> --- a/include/uapi/linux/nfsd/export.h
> +++ b/include/uapi/linux/nfsd/export.h
> @@ -34,7 +34,7 @@
>  #define NFSEXP_GATHERED_WRITES	BIT(5)
>  #define NFSEXP_NOREADDIRPLUS    BIT(6)
>  #define NFSEXP_SECURITY_LABEL	BIT(7)
> -/* BIT(8) currently unused */
> +#define NFSEXP_SIGN_FH			BIT(8)
>  #define NFSEXP_NOHIDE			BIT(9)
>  #define NFSEXP_NOSUBTREECHECK	BIT(10)
>  #define NFSEXP_NOAUTHNLM		BIT(11)	/* Don't authenticate NLM requests - 
> just trust */
> @@ -55,7 +55,7 @@
>  #define NFSEXP_PNFS				BIT(17)
> 
>  /* All flags that we claim to support.  (Note we don't support NOACL.) */
> -#define NFSEXP_ALLFLAGS			BIT(18) - BIT(8) - 1
> +#define NFSEXP_ALLFLAGS			BIT(18) - 1
> 
>  /* The flags that may vary depending on security flavor: */
>  #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
> -- 
> 2.50.1

-- 
Chuck Lever

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

[Benjamin Coddington]

On 16 Jan 2026, at 11:11, Chuck Lever wrote:

> On Fri, Jan 16, 2026, at 9:32 AM, 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.
>
> Generally I like to see more "why" in the commit message. This
> message just repeats what the diff says.
>
> Since the actual rationale will be lengthy, I would say this is
> one of those rare occasions when including a Link: tag that refers
> the reader to the cover letter in the lore archive might be helpful.

I'll add a bit more context, and the link.

>> 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
>
> Nit: "set signing key for filehandles"

Got it - there's encryption artifacts in 4/4 too..

>> +      attribute-set: server
>> +      flags: [admin-perm]
>> +      do:
>> +        request:
>> +          attributes:
>> +            - fh-key
>> diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
>> index 887525964451..98100ee4bcd6 100644
>> --- a/fs/nfsd/netlink.c
>> +++ b/fs/nfsd/netlink.c
>> @@ -47,6 +47,14 @@ static const struct nla_policy
>> nfsd_pool_mode_set_nl_policy[NFSD_A_POOL_MODE_MOD
>>  	[NFSD_A_POOL_MODE_MODE] = { .type = NLA_NUL_STRING, },
>>  };
>>
>> +/* NFSD_CMD_FH_KEY_SET - do */
>> +static const struct nla_policy
>> nfsd_fh_key_set_nl_policy[NFSD_A_SERVER_FH_KEY + 1] = {
>> +	[NFSD_A_SERVER_FH_KEY] = {
>> +		.type = NLA_BINARY,
>> +		.len = 16
>> +	},
>> +};
>> +
>>  /* Ops table for nfsd */
>>  static const struct genl_split_ops nfsd_nl_ops[] = {
>>  	{
>> @@ -102,6 +110,13 @@ static const struct genl_split_ops nfsd_nl_ops[] =
>> {
>>  		.doit	= nfsd_nl_pool_mode_get_doit,
>>  		.flags	= GENL_CMD_CAP_DO,
>>  	},
>> +	{
>> +		.cmd		= NFSD_CMD_FH_KEY_SET,
>> +		.doit		= nfsd_nl_fh_key_set_doit,
>> +		.policy		= nfsd_fh_key_set_nl_policy,
>> +		.maxattr	= NFSD_A_SERVER_FH_KEY,
>> +		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
>> +	},
>>  };
>>
>>  struct genl_family nfsd_nl_family __ro_after_init = {
>> diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
>> index 478117ff6b8c..84d578d628e8 100644
>> --- a/fs/nfsd/netlink.h
>> +++ b/fs/nfsd/netlink.h
>> @@ -26,6 +26,7 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb,
>> struct genl_info *info);
>>  int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info
>> *info);
>>  int nfsd_nl_pool_mode_set_doit(struct sk_buff *skb, struct genl_info
>> *info);
>>  int nfsd_nl_pool_mode_get_doit(struct sk_buff *skb, struct genl_info
>> *info);
>> +int nfsd_nl_fh_key_set_doit(struct sk_buff *skb, struct genl_info
>> *info);
>>
>>  extern struct genl_family nfsd_nl_family;
>>
>> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
>> index 9fa600602658..c8ed733240a0 100644
>> --- a/fs/nfsd/netns.h
>> +++ b/fs/nfsd/netns.h
>> @@ -16,6 +16,7 @@
>>  #include <linux/percpu-refcount.h>
>>  #include <linux/siphash.h>
>>  #include <linux/sunrpc/stats.h>
>> +#include <linux/siphash.h>
>>
>>  /* Hash tables for nfs4_clientid state */
>>  #define CLIENT_HASH_BITS                 4
>> @@ -224,6 +225,7 @@ struct nfsd_net {
>>  	spinlock_t              local_clients_lock;
>>  	struct list_head	local_clients;
>>  #endif
>> +	siphash_key_t		*fh_key;
>>  };
>>
>>  /* Simple check to find out if a given net was properly initialized */
>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>> index 8ccc65bb09fd..aabd66468413 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/module.h>
>>  #include <linux/fsnotify.h>
>>  #include <linux/nfslocalio.h>
>> +#include <crypto/skcipher.h>
>>
>>  #include "idmap.h"
>>  #include "nfsd.h"
>> @@ -49,6 +50,7 @@ enum {
>>  	NFSD_Ports,
>>  	NFSD_MaxBlkSize,
>>  	NFSD_MinThreads,
>> +	NFSD_Fh_Key,
>>  	NFSD_Filecache,
>>  	NFSD_Leasetime,
>>  	NFSD_Gracetime,
>> @@ -69,6 +71,7 @@ static ssize_t write_versions(struct file *file, char
>> *buf, size_t size);
>>  static ssize_t write_ports(struct file *file, char *buf, size_t size);
>>  static ssize_t write_maxblksize(struct file *file, char *buf, size_t
>> size);
>>  static ssize_t write_minthreads(struct file *file, char *buf, size_t
>> size);
>> +static ssize_t write_fh_key(struct file *file, char *buf, size_t size);
>>  #ifdef CONFIG_NFSD_V4
>>  static ssize_t write_leasetime(struct file *file, char *buf, size_t
>> size);
>>  static ssize_t write_gracetime(struct file *file, char *buf, size_t
>> size);
>> @@ -88,6 +91,7 @@ static ssize_t (*const write_op[])(struct file *,
>> char *, size_t) = {
>>  	[NFSD_Ports] = write_ports,
>>  	[NFSD_MaxBlkSize] = write_maxblksize,
>>  	[NFSD_MinThreads] = write_minthreads,
>> +	[NFSD_Fh_Key] = write_fh_key,
>>  #ifdef CONFIG_NFSD_V4
>>  	[NFSD_Leasetime] = write_leasetime,
>>  	[NFSD_Gracetime] = write_gracetime,
>> @@ -950,6 +954,54 @@ static ssize_t write_minthreads(struct file *file,
>> char *buf, size_t size)
>>  	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%u\n", minthreads);
>>  }
>>
>> +/*
>> + * write_fh_key - Set or report the current NFS filehandle key
>> + *
>> + * Input:
>> + *			buf:		ignored
>> + *			size:		zero
>> + * OR
>> + *
>> + * Input:
>> + *			buf:		C string containing a parseable UUID
>> + *			size:		non-zero length of C string in @buf
>> + * Output:
>> + *	On success:	passed-in buffer filled with '\n'-terminated C string
>> + *			containing the standard UUID format of the server's fh_key
>> + *			return code is the size in bytes of the string
>> + *	On error:	return code is zero or a negative errno value
>
> Nit: it would be nice to explain (briefly) why fh_key rotation during
> server operation is prohibited.

Ok, will do.

>> + */
>> +static ssize_t write_fh_key(struct file *file, char *buf, size_t size)
>> +{
>> +	struct nfsd_net *nn = net_generic(netns(file), nfsd_net_id);
>> +
>> +	if (size > 35 && size < 38) {
>> +		siphash_key_t *sip_fh_key;
>> +		uuid_t uuid_fh_key;
>> +		int ret;
>> +
>> +		/* Is the key already set? */
>> +		if (nn->fh_key)
>> +			return -EEXIST;
>> +
>> +		ret = uuid_parse(buf, &uuid_fh_key);
>> +		if (ret)
>> +			return ret;
>> +
>> +		sip_fh_key = kmalloc(sizeof(siphash_key_t), GFP_KERNEL);
>> +		if (!sip_fh_key)
>> +			return -ENOMEM;
>> +
>> +		memcpy(sip_fh_key, &uuid_fh_key, sizeof(siphash_key_t));
>
> What protects updates of nn->fh_key from concurrent writers?

Just the check of the null pointer -- but looks like I forgot to check
it here.  It has a TOCTOU anyway, so yes needs and will add the mutex.

> A race might result in leaking a previous fh_key buffer, when
> it should return EEXIST. So you'll need some mutual exclusion
> here -- probably nfsd_mutex.
>
> Ditto for the netlink interface.

Ditto ack.

>> +		nn->fh_key = sip_fh_key;
>> +
>> +		trace_nfsd_ctl_fh_key_set((const char *)sip_fh_key, ret);
>> +	}
>> +
>
> If user space reads the key before it has been set, would
> nn->fh_key be NULL here?

Yes, but scnprintf is smart enough to print "(null)"

>> +	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%pUb\n",
>> +							nn->fh_key);
>> +}
>> +
>>  #ifdef CONFIG_NFSD_V4
>>  static ssize_t __nfsd4_write_time(struct file *file, char *buf, size_t
>> size,
>>  				  time64_t *time, struct nfsd_net *nn)
>> @@ -1343,6 +1395,7 @@ static int nfsd_fill_super(struct super_block
>> *sb, struct fs_context *fc)
>>  		[NFSD_Ports] = {"portlist", &transaction_ops, S_IWUSR|S_IRUGO},
>>  		[NFSD_MaxBlkSize] = {"max_block_size", &transaction_ops,
>> S_IWUSR|S_IRUGO},
>>  		[NFSD_MinThreads] = {"min_threads", &transaction_ops,
>> S_IWUSR|S_IRUGO},
>> +		[NFSD_Fh_Key] = {"fh_key", &transaction_ops, S_IWUSR|S_IRUSR},
>>  		[NFSD_Filecache] = {"filecache", &nfsd_file_cache_stats_fops,
>> S_IRUGO},
>>  #ifdef CONFIG_NFSD_V4
>>  		[NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops,
>> S_IWUSR|S_IRUSR},
>> @@ -2199,6 +2252,37 @@ int nfsd_nl_pool_mode_get_doit(struct sk_buff
>> *skb, struct genl_info *info)
>>  	return err;
>>  }
>>
>> +int nfsd_nl_fh_key_set_doit(struct sk_buff *skb, struct genl_info
>> *info)
>> +{
>> +	siphash_key_t *fh_key;
>> +	struct nfsd_net *nn;
>> +	int fh_key_len;
>> +	int ret;
>> +
>> +	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_FH_KEY))
>> +		return -EINVAL;
>> +
>> +	fh_key_len = nla_len(info->attrs[NFSD_A_SERVER_FH_KEY]);
>> +	if (fh_key_len != sizeof(siphash_key_t))
>> +		return -EINVAL;
>> +
>> +	/* Is the key already set? */
>> +	nn = net_generic(genl_info_net(info), nfsd_net_id);
>> +	if (nn->fh_key)
>> +		return -EEXIST;
>> +
>> +	fh_key = kmalloc(sizeof(siphash_key_t), GFP_KERNEL);
>> +	if (!fh_key)
>> +		return -ENOMEM;
>> +
>> +	memcpy(fh_key, nla_data(info->attrs[NFSD_A_SERVER_FH_KEY]),
>> sizeof(siphash_key_t));
>> +	nn = net_generic(genl_info_net(info), nfsd_net_id);
>> +	nn->fh_key = fh_key;
>> +
>
> On success, "ret" hasn't been initialized. And maybe you want
> the error flows above to exit through here to get those return
> codes captured by the trace point.

Gah, thanks!


>
>> +	trace_nfsd_ctl_fh_key_set((const char *)fh_key, ret);
>> +	return ret;
>> +}
>> +
>>  /**
>>   * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
>>   * @net: a freshly-created network namespace
>> @@ -2284,6 +2368,7 @@ static __net_exit void nfsd_net_exit(struct net
>> *net)
>>  {
>>  	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>>
>
> To save an extra pointer dereference on the hotter paths, maybe
> fh_key should be the actual key rather than a pointer. You can
> use kfree_sensitive() when freeing the whole struct nfsd_net, or
> just memset the fh_key field before freeing nfsd_net.
>
> Just a random thought.

Could do!  ..but its easier to check if a pointer is NULL than to check two
u64's being unset.  That said, having half the key be zero is probably
insane odds.  I also figured to minimize the size of nfsd_net when this is
not used.  I think I like the pointer better, if that's acceptable.

Thank you for your review here.

Ben

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Chuck Lever]

On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
> The following series enables the linux NFS server to add a Message
> Authentication Code (MAC) to the filehandles it gives to clients.  This
> provides additional protection to the exported filesystem against filehandle
> guessing attacks.
>
> Filesystems generate their own filehandles through the export_operation
> "encode_fh" and a filehandle provides sufficient access to open a file
> without needing to perform a lookup.  An NFS client holding a valid
> filehandle can remotely open and read the contents of the file referred to
> by the filehandle.

"open, read, or modify the contents of the file"

Btw, referring to "open" here is a little confusing, since NFSv3 does
not have an on-the-wire OPEN operation. I'm not sure how to clarify.


> In order to acquire a filehandle, you must perform lookup operations on the
> parent directory(ies), and the permissions on those directories may
> prohibit you from walking into them to find the files within.  This would
> normally be considered sufficient protection on a local filesystem to
> prohibit users from accessing those files, however when the filesystem is
> exported via NFS those files can still be accessed by guessing the correct,
> valid filehandles.

Instead: "an exported file can be accessed whenever the NFS server is
presented with the correct filehandle, which can be guessed or acquired
by means other than LOOKUP."


> Filehandles are easy to guess because they are well-formed.  The
> open_by_handle_at(2) man page contains an example C program
> (t_name_to_handle_at.c) that can display a filehandle given a path.  Here's
> an example filehandle from a fairly modern XFS:
>
> # ./t_name_to_handle_at /exports/foo 
> 57
> 12 129    99 00 00 00 00 00 00 00 b4 10 0b 8c
>
>           ^---------  filehandle  ----------^
>           ^------- inode -------^ ^-- gen --^
>
> This filehandle consists of a 64-bit inode number and 32-bit generation
> number.  Because the handle is well-formed, its easy to fabricate
> filehandles that match other files within the same filesystem.  You can
> simply insert inode numbers and iterate on the generation number.
> Eventually you'll be able to access the file using open_by_handle_at(2).
> For a local system, open_by_handle_at(2) requires CAP_DAC_READ_SEARCH, which
> protects against guessing attacks by unprivileged users.
>
> In contrast to a local user using open_by_handle(2), the NFS server must
> permissively allow remote clients to open by filehandle without being able
> to check or trust the remote caller's access. Therefore additional
> protection against this attack is needed for NFS case.  We propose to sign
> filehandles by appending an 8-byte MAC which is the siphash of the
> filehandle from a key set from the nfs-utilities.  NFS server can then
> ensure that guessing a valid filehandle+MAC is practically impossible
> without knowledge of the MAC's key.  The NFS server performs optional
> signing by possessing a key set from userspace and having the "sign_fh"
> export option.

OK, I guess this is where I got the idea this would be an export option.

But I'm unconvinced that this provides any real security. There are
other ways of obtaining a filehandle besides guessing, and nothing
here suggests that guessing is the premier attack methodology.

The fundamental issue is there is no authorization check done by NFS
READ or WRITE. And in the case of root_squash with AUTH_SYS, maybe
even an authorization check at I/O time isn't enough. Note this is
the classic NFS AUTH_SYS security model; it assumes we're all best
of friends.


> Because filehandles are long-lived, and there's no method for expiring them,
> the server's key should be set once and not changed.  It also should be
> persisted across restarts.  The methods to set the key allow only setting it
> once, afterward it cannot be changed.  A separate patchset for nfs-utils
> contains the userspace changes required to set the server's key.

There are some problems here.

- The requirement is: File handles must remain stable while the inode
  generation number remains unchanged (Chinner).

- There's nothing in your implementation that prevents user space
  from providing a different key after a system reboot or server
  restart. In fact, destroying the net namespace removes the ability
  for the server to remember (and thus check) the previously set
  fh_key.

- An fh_key change is safe to do once all exported file systems are
  unexported and unmounted. NFS clients generally don't preserve
  filehandles after they unmount file systems. I say this because I
  think rekeying might become important as the time to break a key
  decreases.


> I had a previous attempt to solve this problem by encrypting 
> filehandles,
> which turned out to be a problematic, poor solution.  The discussion on
> that previous attempt is here:
> https://lore.kernel.org/linux-nfs/510E10A4-11BE-412D-93AF-C4CC969954E7@hammerspace.com/T/#t
>
> There are some changes from that version:
> 	- sign filehandles instead of encrypt them (Eric Biggers)
> 	- fix the NFSEXP_ macros, specifically NFSEXP_ALLFLAGS (NeilBrown)
> 	- rebase onto cel/nfsd-next (Chuck Lever)
> 	- condensed/clarified problem explantion (thanks Chuck Lever)
> 	- add nfsctl file "fh_key" for rpc.nfsd to also set the key
>
> I plan on adding additional work to enable the server to check whether the
> 8-byte MAC will overflow maximum filehandle length for the protocol at
> export time.  There could be some filesystems with 40-byte fileid and
> 24-byte fsid which would break NFSv3's 64-byte filehandle maximum with an
> 8-byte MAC appended.  The server should refuse to export those filesystems
> when "sign_fh" is requested.

Noted, and we might require all that to be in place before merging.


> Thanks for any comments and critique.
>
> Benjamin Coddington (4):
>   nfsd: Convert export flags to use BIT() macro
>   nfsd: Add a key for signing filehandles
>   NFSD/export: Add sign_fh export option
>   NFSD: Sign filehandles
>
>  Documentation/netlink/specs/nfsd.yaml | 12 ++++
>  fs/nfsd/export.c                      |  5 +-
>  fs/nfsd/netlink.c                     | 15 +++++
>  fs/nfsd/netlink.h                     |  1 +
>  fs/nfsd/netns.h                       |  2 +
>  fs/nfsd/nfs3xdr.c                     | 20 +++---
>  fs/nfsd/nfs4xdr.c                     | 12 ++--
>  fs/nfsd/nfsctl.c                      | 87 ++++++++++++++++++++++++++-
>  fs/nfsd/nfsfh.c                       | 72 +++++++++++++++++++++-
>  fs/nfsd/nfsfh.h                       | 22 +++++++
>  fs/nfsd/trace.h                       | 19 ++++++
>  include/linux/sunrpc/svc.h            |  1 +
>  include/uapi/linux/nfsd/export.h      | 38 ++++++------
>  include/uapi/linux/nfsd_netlink.h     |  2 +
>  14 files changed, 272 insertions(+), 36 deletions(-)
>
>
> base-commit: bfd453acb5637b5df881cef4b21803344aa9e7ac
> -- 
> 2.50.1

-- 
Chuck Lever

Re: [PATCH v1 4/4] NFSD: Sign filehandles

[Chuck Lever]

On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
> 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.
>
> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
> ---
>  fs/nfsd/nfs3xdr.c          | 20 +++++++----
>  fs/nfsd/nfs4xdr.c          | 12 ++++---
>  fs/nfsd/nfsfh.c            | 72 ++++++++++++++++++++++++++++++++++++--
>  fs/nfsd/nfsfh.h            | 22 ++++++++++++
>  include/linux/sunrpc/svc.h |  1 +
>  5 files changed, 113 insertions(+), 14 deletions(-)
>
> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> index ef4971d71ac4..f9d0c4892de7 100644
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -120,11 +120,16 @@ svcxdr_encode_nfsstat3(struct xdr_stream *xdr, 
> __be32 status)
>  }
> 
>  static bool
> -svcxdr_encode_nfs_fh3(struct xdr_stream *xdr, const struct svc_fh *fhp)
> +svcxdr_encode_nfs_fh3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> +						struct svc_fh *fhp)
>  {
> -	u32 size = fhp->fh_handle.fh_size;
> +	u32 size;
>  	__be32 *p;
> 
> +	if (fh_append_mac(fhp, SVC_NET(rqstp)))
> +		return false;
> +	size = fhp->fh_handle.fh_size;
> +

This is a layering violation. XDR encoding never alters the content
in the local data structures, and this will be impossible to
convert to xdrgen down the line. All of the NFS version-specific
code should not know or care about FH signing, IMO.

Why can't the new signing logic be contained in fh_compose()
and fh_verify() ?


>  	p = xdr_reserve_space(xdr, XDR_UNIT + size);
>  	if (!p)
>  		return false;
> @@ -137,11 +142,12 @@ svcxdr_encode_nfs_fh3(struct xdr_stream *xdr, 
> const struct svc_fh *fhp)
>  }
> 
>  static bool
> -svcxdr_encode_post_op_fh3(struct xdr_stream *xdr, const struct svc_fh *fhp)
> +svcxdr_encode_post_op_fh3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> +							struct svc_fh *fhp)
>  {
>  	if (xdr_stream_encode_item_present(xdr) < 0)
>  		return false;
> -	if (!svcxdr_encode_nfs_fh3(xdr, fhp))
> +	if (!svcxdr_encode_nfs_fh3(rqstp, xdr, fhp))
>  		return false;
> 
>  	return true;
> @@ -772,7 +778,7 @@ nfs3svc_encode_lookupres(struct svc_rqst *rqstp, 
> struct xdr_stream *xdr)
>  		return false;
>  	switch (resp->status) {
>  	case nfs_ok:
> -		if (!svcxdr_encode_nfs_fh3(xdr, &resp->fh))
> +		if (!svcxdr_encode_nfs_fh3(rqstp, xdr, &resp->fh))
>  			return false;
>  		if (!svcxdr_encode_post_op_attr(rqstp, xdr, &resp->fh))
>  			return false;
> @@ -908,7 +914,7 @@ nfs3svc_encode_createres(struct svc_rqst *rqstp, 
> struct xdr_stream *xdr)
>  		return false;
>  	switch (resp->status) {
>  	case nfs_ok:
> -		if (!svcxdr_encode_post_op_fh3(xdr, &resp->fh))
> +		if (!svcxdr_encode_post_op_fh3(rqstp, xdr, &resp->fh))
>  			return false;
>  		if (!svcxdr_encode_post_op_attr(rqstp, xdr, &resp->fh))
>  			return false;
> @@ -1117,7 +1123,7 @@ svcxdr_encode_entry3_plus(struct nfsd3_readdirres 
> *resp, const char *name,
> 
>  	if (!svcxdr_encode_post_op_attr(resp->rqstp, xdr, fhp))
>  		goto out;
> -	if (!svcxdr_encode_post_op_fh3(xdr, fhp))
> +	if (!svcxdr_encode_post_op_fh3(resp->rqstp, xdr, fhp))
>  		goto out;
>  	result = true;
> 
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 884b792c95a3..f12981b989d1 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2701,9 +2701,13 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
>  }
> 
>  static __be32 nfsd4_encode_nfs_fh4(struct xdr_stream *xdr,
> -				   struct knfsd_fh *fh_handle)
> +					struct svc_fh *fhp)
>  {
> -	return nfsd4_encode_opaque(xdr, fh_handle->fh_raw, fh_handle->fh_size);
> +	if (fh_append_mac(fhp, SVC_NET(RESSTRM_RQST(xdr))))
> +		return nfserr_resource;
> +
> +	return nfsd4_encode_opaque(xdr, fhp->fh_handle.fh_raw,
> +		fhp->fh_handle.fh_size);
>  }
> 
>  /* This is a frequently-encoded type; open-coded for speed */
> @@ -3359,7 +3363,7 @@ static __be32 nfsd4_encode_fattr4_acl(struct 
> xdr_stream *xdr,
>  static __be32 nfsd4_encode_fattr4_filehandle(struct xdr_stream *xdr,
>  					     const struct nfsd4_fattr_args *args)
>  {
> -	return nfsd4_encode_nfs_fh4(xdr, &args->fhp->fh_handle);
> +	return nfsd4_encode_nfs_fh4(xdr, args->fhp);
>  }
> 
>  static __be32 nfsd4_encode_fattr4_fileid(struct xdr_stream *xdr,
> @@ -4460,7 +4464,7 @@ nfsd4_encode_getfh(struct nfsd4_compoundres 
> *resp, __be32 nfserr,
>  	struct svc_fh *fhp = u->getfh;
> 
>  	/* object */
> -	return nfsd4_encode_nfs_fh4(xdr, &fhp->fh_handle);
> +	return nfsd4_encode_nfs_fh4(xdr, fhp);
>  }
> 
>  static __be32
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index ed85dd43da18..b2fb16b7f3c9 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/skcipher.h>

Is this header still needed?


>  #include "nfsd.h"
>  #include "vfs.h"
>  #include "auth.h"
> @@ -137,6 +138,62 @@ static inline __be32 check_pseudo_root(struct 
> dentry *dentry,
>  	return nfs_ok;
>  }
> 
> +/*
> + * Intended to be called when encoding, appends an 8-byte MAC
> + * to the filehandle hashed from the server's fh_key:
> + */
> +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("NFSD: unable to sign filehandles, fh_key not set.\n");

Use pr_warn_ratelimited() instead


> +		return -EINVAL;
> +	}
> +
> +	if (fh->fh_size + sizeof(hash) > fhp->fh_maxsize) {
> +		pr_warn("NFSD: unable to sign filehandles, fh_size %lu would be 
> greater"
> +			" than fh_maxsize %d.\n", 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);

What prevents appending a MAC to the same FH multiple times?


> +
> +	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("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 memcmp(&fh->fh_raw[fh->fh_size - sizeof(hash)], &hash, 
> sizeof(hash));

Let's use crypto_memneq() for this purpose, to avoid timing attacks.


> +}
> +
>  /*
>   * 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 +223,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 +297,15 @@ 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 {
> +		/* Root filehandle always unsigned because rpc.mountd has no key */
> +		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);
> diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> index 5ef7191f8ad8..d1ae272117f0 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
> +
>  static inline u32 *fh_fsid(const struct knfsd_fh *fh)
>  {
>  	return (u32 *)&fh->fh_raw[4];
> @@ -226,6 +229,7 @@ __be32	fh_getattr(const struct svc_fh *fhp, struct 
> kstat *stat);
>  __be32	fh_compose(struct svc_fh *, struct svc_export *, struct dentry 
> *, struct svc_fh *);
>  __be32	fh_update(struct svc_fh *);
>  void	fh_put(struct svc_fh *);
> +int	fh_append_mac(struct svc_fh *, struct net *net);
> 
>  static __inline__ struct svc_fh *
>  fh_copy(struct svc_fh *dst, const struct svc_fh *src)
> @@ -274,6 +278,24 @@ static inline bool fh_fsid_match(const struct 
> knfsd_fh *fh1,
>  	return true;
>  }
> 
> +static inline size_t fh_fileid_offset(const struct knfsd_fh *fh)
> +{
> +	return key_len(fh->fh_fsid_type) + 4;
> +}
> +
> +static inline size_t fh_fileid_len(const struct knfsd_fh *fh)
> +{
> +	switch (fh->fh_auth_type) {
> +	case FH_AT_NONE:
> +		return fh->fh_size - fh_fileid_offset(fh);
> +		break;
> +	case FH_AT_MAC:
> +		return fh->fh_size - 8 - fh_fileid_offset(fh);

Let's define/use symbolic constants here (8) and just above (4).


> +		break;

The break; statements are unreachable.


> +	}
> +	return 0;
> +}
> +
>  /**
>   * fh_want_write - Get write access to an export
>   * @fhp: File handle of file to be written
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index 62152e4f3bcc..96dae45d70ca 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -267,6 +267,7 @@ enum {
>  };
> 
>  #define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : 
> rqst->rq_bc_net)
> +#define RESSTRM_RQST(xdr_stream) (container_of(xdr_stream, struct 
> svc_rqst, rq_res_stream))
> 
>  /*
>   * Rigorous type checking on sockaddr type conversions
> -- 
> 2.50.1

-- 
Chuck Lever

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Benjamin Coddington]

On 16 Jan 2026, at 11:56, Chuck Lever wrote:

> On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
>> The following series enables the linux NFS server to add a Message
>> Authentication Code (MAC) to the filehandles it gives to clients.  This
>> provides additional protection to the exported filesystem against filehandle
>> guessing attacks.
>>
>> Filesystems generate their own filehandles through the export_operation
>> "encode_fh" and a filehandle provides sufficient access to open a file
>> without needing to perform a lookup.  An NFS client holding a valid
>> filehandle can remotely open and read the contents of the file referred to
>> by the filehandle.
>
> "open, read, or modify the contents of the file"
>
> Btw, referring to "open" here is a little confusing, since NFSv3 does
> not have an on-the-wire OPEN operation. I'm not sure how to clarify.
>
>
>> In order to acquire a filehandle, you must perform lookup operations on the
>> parent directory(ies), and the permissions on those directories may
>> prohibit you from walking into them to find the files within.  This would
>> normally be considered sufficient protection on a local filesystem to
>> prohibit users from accessing those files, however when the filesystem is
>> exported via NFS those files can still be accessed by guessing the correct,
>> valid filehandles.
>
> Instead: "an exported file can be accessed whenever the NFS server is
> presented with the correct filehandle, which can be guessed or acquired
> by means other than LOOKUP."
>
>
>> Filehandles are easy to guess because they are well-formed.  The
>> open_by_handle_at(2) man page contains an example C program
>> (t_name_to_handle_at.c) that can display a filehandle given a path.  Here's
>> an example filehandle from a fairly modern XFS:
>>
>> # ./t_name_to_handle_at /exports/foo
>> 57
>> 12 129    99 00 00 00 00 00 00 00 b4 10 0b 8c
>>
>>           ^---------  filehandle  ----------^
>>           ^------- inode -------^ ^-- gen --^
>>
>> This filehandle consists of a 64-bit inode number and 32-bit generation
>> number.  Because the handle is well-formed, its easy to fabricate
>> filehandles that match other files within the same filesystem.  You can
>> simply insert inode numbers and iterate on the generation number.
>> Eventually you'll be able to access the file using open_by_handle_at(2).
>> For a local system, open_by_handle_at(2) requires CAP_DAC_READ_SEARCH, which
>> protects against guessing attacks by unprivileged users.
>>
>> In contrast to a local user using open_by_handle(2), the NFS server must
>> permissively allow remote clients to open by filehandle without being able
>> to check or trust the remote caller's access. Therefore additional
>> protection against this attack is needed for NFS case.  We propose to sign
>> filehandles by appending an 8-byte MAC which is the siphash of the
>> filehandle from a key set from the nfs-utilities.  NFS server can then
>> ensure that guessing a valid filehandle+MAC is practically impossible
>> without knowledge of the MAC's key.  The NFS server performs optional
>> signing by possessing a key set from userspace and having the "sign_fh"
>> export option.
>
> OK, I guess this is where I got the idea this would be an export option.
>
> But I'm unconvinced that this provides any real security. There are
> other ways of obtaining a filehandle besides guessing, and nothing
> here suggests that guessing is the premier attack methodology.

Help me understand you - you're unconvinced that having the server sign
filehandles and verify filehandles prevents clients from fabricating valid
ones?

> The fundamental issue is there is no authorization check done by NFS
> READ or WRITE. And in the case of root_squash with AUTH_SYS, maybe
> even an authorization check at I/O time isn't enough. Note this is
> the classic NFS AUTH_SYS security model; it assumes we're all best
> of friends.

I'm not quite following, READ and WRITE will have their filehandles
validated.. the case you might be talking about is when a file moves or its
access changes after the client has the valid filehandle.  I'm not
attempting to protect against those cases.

>> Because filehandles are long-lived, and there's no method for expiring them,
>> the server's key should be set once and not changed.  It also should be
>> persisted across restarts.  The methods to set the key allow only setting it
>> once, afterward it cannot be changed.  A separate patchset for nfs-utils
>> contains the userspace changes required to set the server's key.
>
> There are some problems here.
>
> - The requirement is: File handles must remain stable while the inode
>   generation number remains unchanged (Chinner).
>
> - There's nothing in your implementation that prevents user space
>   from providing a different key after a system reboot or server
>   restart. In fact, destroying the net namespace removes the ability
>   for the server to remember (and thus check) the previously set
>   fh_key.

Yes - suggestions on how to solve this are welcome.

> - An fh_key change is safe to do once all exported file systems are
>   unexported and unmounted. NFS clients generally don't preserve
>   filehandles after they unmount file systems. I say this because I
>   think rekeying might become important as the time to break a key
>   decreases.

Absolutely.

Ben

Re: [PATCH v1 4/4] NFSD: Sign filehandles

[Benjamin Coddington]

On 16 Jan 2026, at 12:12, Chuck Lever wrote:

> On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
>> 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.
>>
>> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
>> ---
>>  fs/nfsd/nfs3xdr.c          | 20 +++++++----
>>  fs/nfsd/nfs4xdr.c          | 12 ++++---
>>  fs/nfsd/nfsfh.c            | 72 ++++++++++++++++++++++++++++++++++++--
>>  fs/nfsd/nfsfh.h            | 22 ++++++++++++
>>  include/linux/sunrpc/svc.h |  1 +
>>  5 files changed, 113 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
>> index ef4971d71ac4..f9d0c4892de7 100644
>> --- a/fs/nfsd/nfs3xdr.c
>> +++ b/fs/nfsd/nfs3xdr.c
>> @@ -120,11 +120,16 @@ svcxdr_encode_nfsstat3(struct xdr_stream *xdr,
>> __be32 status)
>>  }
>>
>>  static bool
>> -svcxdr_encode_nfs_fh3(struct xdr_stream *xdr, const struct svc_fh *fhp)
>> +svcxdr_encode_nfs_fh3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
>> +						struct svc_fh *fhp)
>>  {
>> -	u32 size = fhp->fh_handle.fh_size;
>> +	u32 size;
>>  	__be32 *p;
>>
>> +	if (fh_append_mac(fhp, SVC_NET(rqstp)))
>> +		return false;
>> +	size = fhp->fh_handle.fh_size;
>> +
>
> This is a layering violation. XDR encoding never alters the content
> in the local data structures, and this will be impossible to
> convert to xdrgen down the line. All of the NFS version-specific
> code should not know or care about FH signing, IMO.
>
> Why can't the new signing logic be contained in fh_compose()
> and fh_verify() ?

It can - I originally had it there.  I moved it after finding that
fh_compose() creates a partial handle requiring fh_update() to fill it in
after file creation, and IIRC there are a couple of other fh_match()
gotcha's buried in the server when you're not signing last thing before the
wire.  Totally doable to move it, I'll work on that for next version - but
I think it will mean that some other parts of the server will need to become
"signed filehandle"-aware.

>>  	p = xdr_reserve_space(xdr, XDR_UNIT + size);
>>  	if (!p)
>>  		return false;
>> @@ -137,11 +142,12 @@ svcxdr_encode_nfs_fh3(struct xdr_stream *xdr,
>> const struct svc_fh *fhp)
>>  }
>>
>>  static bool
>> -svcxdr_encode_post_op_fh3(struct xdr_stream *xdr, const struct svc_fh *fhp)
>> +svcxdr_encode_post_op_fh3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
>> +							struct svc_fh *fhp)
>>  {
>>  	if (xdr_stream_encode_item_present(xdr) < 0)
>>  		return false;
>> -	if (!svcxdr_encode_nfs_fh3(xdr, fhp))
>> +	if (!svcxdr_encode_nfs_fh3(rqstp, xdr, fhp))
>>  		return false;
>>
>>  	return true;
>> @@ -772,7 +778,7 @@ nfs3svc_encode_lookupres(struct svc_rqst *rqstp,
>> struct xdr_stream *xdr)
>>  		return false;
>>  	switch (resp->status) {
>>  	case nfs_ok:
>> -		if (!svcxdr_encode_nfs_fh3(xdr, &resp->fh))
>> +		if (!svcxdr_encode_nfs_fh3(rqstp, xdr, &resp->fh))
>>  			return false;
>>  		if (!svcxdr_encode_post_op_attr(rqstp, xdr, &resp->fh))
>>  			return false;
>> @@ -908,7 +914,7 @@ nfs3svc_encode_createres(struct svc_rqst *rqstp,
>> struct xdr_stream *xdr)
>>  		return false;
>>  	switch (resp->status) {
>>  	case nfs_ok:
>> -		if (!svcxdr_encode_post_op_fh3(xdr, &resp->fh))
>> +		if (!svcxdr_encode_post_op_fh3(rqstp, xdr, &resp->fh))
>>  			return false;
>>  		if (!svcxdr_encode_post_op_attr(rqstp, xdr, &resp->fh))
>>  			return false;
>> @@ -1117,7 +1123,7 @@ svcxdr_encode_entry3_plus(struct nfsd3_readdirres
>> *resp, const char *name,
>>
>>  	if (!svcxdr_encode_post_op_attr(resp->rqstp, xdr, fhp))
>>  		goto out;
>> -	if (!svcxdr_encode_post_op_fh3(xdr, fhp))
>> +	if (!svcxdr_encode_post_op_fh3(resp->rqstp, xdr, fhp))
>>  		goto out;
>>  	result = true;
>>
>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>> index 884b792c95a3..f12981b989d1 100644
>> --- a/fs/nfsd/nfs4xdr.c
>> +++ b/fs/nfsd/nfs4xdr.c
>> @@ -2701,9 +2701,13 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
>>  }
>>
>>  static __be32 nfsd4_encode_nfs_fh4(struct xdr_stream *xdr,
>> -				   struct knfsd_fh *fh_handle)
>> +					struct svc_fh *fhp)
>>  {
>> -	return nfsd4_encode_opaque(xdr, fh_handle->fh_raw, fh_handle->fh_size);
>> +	if (fh_append_mac(fhp, SVC_NET(RESSTRM_RQST(xdr))))
>> +		return nfserr_resource;
>> +
>> +	return nfsd4_encode_opaque(xdr, fhp->fh_handle.fh_raw,
>> +		fhp->fh_handle.fh_size);
>>  }
>>
>>  /* This is a frequently-encoded type; open-coded for speed */
>> @@ -3359,7 +3363,7 @@ static __be32 nfsd4_encode_fattr4_acl(struct
>> xdr_stream *xdr,
>>  static __be32 nfsd4_encode_fattr4_filehandle(struct xdr_stream *xdr,
>>  					     const struct nfsd4_fattr_args *args)
>>  {
>> -	return nfsd4_encode_nfs_fh4(xdr, &args->fhp->fh_handle);
>> +	return nfsd4_encode_nfs_fh4(xdr, args->fhp);
>>  }
>>
>>  static __be32 nfsd4_encode_fattr4_fileid(struct xdr_stream *xdr,
>> @@ -4460,7 +4464,7 @@ nfsd4_encode_getfh(struct nfsd4_compoundres
>> *resp, __be32 nfserr,
>>  	struct svc_fh *fhp = u->getfh;
>>
>>  	/* object */
>> -	return nfsd4_encode_nfs_fh4(xdr, &fhp->fh_handle);
>> +	return nfsd4_encode_nfs_fh4(xdr, fhp);
>>  }
>>
>>  static __be32
>> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
>> index ed85dd43da18..b2fb16b7f3c9 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/skcipher.h>
>
> Is this header still needed?

Nope.

>>  #include "nfsd.h"
>>  #include "vfs.h"
>>  #include "auth.h"
>> @@ -137,6 +138,62 @@ static inline __be32 check_pseudo_root(struct
>> dentry *dentry,
>>  	return nfs_ok;
>>  }
>>
>> +/*
>> + * Intended to be called when encoding, appends an 8-byte MAC
>> + * to the filehandle hashed from the server's fh_key:
>> + */
>> +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("NFSD: unable to sign filehandles, fh_key not set.\n");
>
> Use pr_warn_ratelimited() instead

Yessir!

>> +		return -EINVAL;
>> +	}
>> +
>> +	if (fh->fh_size + sizeof(hash) > fhp->fh_maxsize) {
>> +		pr_warn("NFSD: unable to sign filehandles, fh_size %lu would be
>> greater"
>> +			" than fh_maxsize %d.\n", 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);
>
> What prevents appending a MAC to the same FH multiple times?

The fact that it's the last thing that happens during encoding.  We're going
to change this now, so it will need to be guarded against.  Can use a check
of fh_auth_type.

>> +
>> +	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("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 memcmp(&fh->fh_raw[fh->fh_size - sizeof(hash)], &hash,
>> sizeof(hash));
>
> Let's use crypto_memneq() for this purpose, to avoid timing attacks.

Ok.

>> +}
>> +
>>  /*
>>   * 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 +223,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 +297,15 @@ 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 {
>> +		/* Root filehandle always unsigned because rpc.mountd has no key */
>> +		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);
>> diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
>> index 5ef7191f8ad8..d1ae272117f0 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
>> +
>>  static inline u32 *fh_fsid(const struct knfsd_fh *fh)
>>  {
>>  	return (u32 *)&fh->fh_raw[4];
>> @@ -226,6 +229,7 @@ __be32	fh_getattr(const struct svc_fh *fhp, struct
>> kstat *stat);
>>  __be32	fh_compose(struct svc_fh *, struct svc_export *, struct dentry
>> *, struct svc_fh *);
>>  __be32	fh_update(struct svc_fh *);
>>  void	fh_put(struct svc_fh *);
>> +int	fh_append_mac(struct svc_fh *, struct net *net);
>>
>>  static __inline__ struct svc_fh *
>>  fh_copy(struct svc_fh *dst, const struct svc_fh *src)
>> @@ -274,6 +278,24 @@ static inline bool fh_fsid_match(const struct
>> knfsd_fh *fh1,
>>  	return true;
>>  }
>>
>> +static inline size_t fh_fileid_offset(const struct knfsd_fh *fh)
>> +{
>> +	return key_len(fh->fh_fsid_type) + 4;
>> +}
>> +
>> +static inline size_t fh_fileid_len(const struct knfsd_fh *fh)
>> +{
>> +	switch (fh->fh_auth_type) {
>> +	case FH_AT_NONE:
>> +		return fh->fh_size - fh_fileid_offset(fh);
>> +		break;
>> +	case FH_AT_MAC:
>> +		return fh->fh_size - 8 - fh_fileid_offset(fh);
>
> Let's define/use symbolic constants here (8) and just above (4).

Actually, think I can drop this hunk now - its no longer used after a
refactor.

>> +		break;
>
> The break; statements are unreachable.

Yep.  Thanks Chuck!

Ben

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Chuck Lever]

On 1/16/26 12:17 PM, Benjamin Coddington wrote:
> On 16 Jan 2026, at 11:56, Chuck Lever wrote:
> 
>> On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
>>> The following series enables the linux NFS server to add a Message
>>> Authentication Code (MAC) to the filehandles it gives to clients.  This
>>> provides additional protection to the exported filesystem against filehandle
>>> guessing attacks.
>>>
>>> Filesystems generate their own filehandles through the export_operation
>>> "encode_fh" and a filehandle provides sufficient access to open a file
>>> without needing to perform a lookup.  An NFS client holding a valid
>>> filehandle can remotely open and read the contents of the file referred to
>>> by the filehandle.
>>
>> "open, read, or modify the contents of the file"
>>
>> Btw, referring to "open" here is a little confusing, since NFSv3 does
>> not have an on-the-wire OPEN operation. I'm not sure how to clarify.
>>
>>
>>> In order to acquire a filehandle, you must perform lookup operations on the
>>> parent directory(ies), and the permissions on those directories may
>>> prohibit you from walking into them to find the files within.  This would
>>> normally be considered sufficient protection on a local filesystem to
>>> prohibit users from accessing those files, however when the filesystem is
>>> exported via NFS those files can still be accessed by guessing the correct,
>>> valid filehandles.
>>
>> Instead: "an exported file can be accessed whenever the NFS server is
>> presented with the correct filehandle, which can be guessed or acquired
>> by means other than LOOKUP."
>>
>>
>>> Filehandles are easy to guess because they are well-formed.  The
>>> open_by_handle_at(2) man page contains an example C program
>>> (t_name_to_handle_at.c) that can display a filehandle given a path.  Here's
>>> an example filehandle from a fairly modern XFS:
>>>
>>> # ./t_name_to_handle_at /exports/foo
>>> 57
>>> 12 129    99 00 00 00 00 00 00 00 b4 10 0b 8c
>>>
>>>           ^---------  filehandle  ----------^
>>>           ^------- inode -------^ ^-- gen --^
>>>
>>> This filehandle consists of a 64-bit inode number and 32-bit generation
>>> number.  Because the handle is well-formed, its easy to fabricate
>>> filehandles that match other files within the same filesystem.  You can
>>> simply insert inode numbers and iterate on the generation number.
>>> Eventually you'll be able to access the file using open_by_handle_at(2).
>>> For a local system, open_by_handle_at(2) requires CAP_DAC_READ_SEARCH, which
>>> protects against guessing attacks by unprivileged users.
>>>
>>> In contrast to a local user using open_by_handle(2), the NFS server must
>>> permissively allow remote clients to open by filehandle without being able
>>> to check or trust the remote caller's access.

Btw, "allow ... clients to open by filehandle" is another confusion.

NFSv4 OPEN does do access checking and authorization.

Again, it's NFS READ and WRITE that are not blocked.

NFSv3 READ and WRITE do an intrinsic open.

NFSv4 READ and WRITE permit the use of a special stateid so that an OPEN
isn't necessary to do the I/O (IIRC).


>>> Therefore additional
>>> protection against this attack is needed for NFS case.  We propose to sign
>>> filehandles by appending an 8-byte MAC which is the siphash of the
>>> filehandle from a key set from the nfs-utilities.  NFS server can then
>>> ensure that guessing a valid filehandle+MAC is practically impossible
>>> without knowledge of the MAC's key.  The NFS server performs optional
>>> signing by possessing a key set from userspace and having the "sign_fh"
>>> export option.
>>
>> OK, I guess this is where I got the idea this would be an export option.
>>
>> But I'm unconvinced that this provides any real security. There are
>> other ways of obtaining a filehandle besides guessing, and nothing
>> here suggests that guessing is the premier attack methodology.
> 
> Help me understand you - you're unconvinced that having the server sign
> filehandles and verify filehandles prevents clients from fabricating valid
> ones?

The rationale provided here doesn't convince me that fabrication is the
biggest threat and will give us the biggest bang for our buck if it is
mitigated.

In order to carry out this attack, the attacker has to have access to
the filehandles on an NFS client to examine them. She has to have
access to a valid client IP address to send NFS requests from. Maybe
you can bridge the gap by explaining how a /non-root/ user on an NFS
client might leverage FH fabrication to gain access to another user's
files. I think only the root user has this ability.

I've also tried to convince myself that cryptographic FH validation
could mitigate misdirected WRITEs or READs. An attacker could replace
the FH in a valid NFS request, for example, or a client might send
garbage in an FH. I'm not sure those are real problems, though.

I am keeping in mind that everything here is using AUTH_SYS anyway, so
maybe I'm just an old man yelling at a cloud. Nothing here is blocking
yet, but I want the feature to provide meaningful value (as I think you
do).

Thank you for bearing with me.


-- 
Chuck Lever

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

[Chuck Lever]

On Fri, Jan 16, 2026, at 11:42 AM, Benjamin Coddington wrote:
> On 16 Jan 2026, at 11:11, Chuck Lever wrote:
>> To save an extra pointer dereference on the hotter paths, maybe
>> fh_key should be the actual key rather than a pointer. You can
>> use kfree_sensitive() when freeing the whole struct nfsd_net, or
>> just memset the fh_key field before freeing nfsd_net.
>>
>> Just a random thought.
>
> Could do!  ..but its easier to check if a pointer is NULL than to check two
> u64's being unset.  That said, having half the key be zero is probably
> insane odds.  I also figured to minimize the size of nfsd_net when this is
> not used.  I think I like the pointer better, if that's acceptable.

Agreed, the pointer has the benefit of showing whether the fh_key
has been set or not.

-- 
Chuck Lever

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Trond Myklebust]

On Fri, 2026-01-16 at 14:43 -0500, Chuck Lever wrote:
> On 1/16/26 12:17 PM, Benjamin Coddington wrote:
> > On 16 Jan 2026, at 11:56, Chuck Lever wrote:
> > 
> > > On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
> > > > The following series enables the linux NFS server to add a
> > > > Message
> > > > Authentication Code (MAC) to the filehandles it gives to
> > > > clients.  This
> > > > provides additional protection to the exported filesystem
> > > > against filehandle
> > > > guessing attacks.
> > > > 
> > > > Filesystems generate their own filehandles through the
> > > > export_operation
> > > > "encode_fh" and a filehandle provides sufficient access to open
> > > > a file
> > > > without needing to perform a lookup.  An NFS client holding a
> > > > valid
> > > > filehandle can remotely open and read the contents of the file
> > > > referred to
> > > > by the filehandle.
> > > 
> > > "open, read, or modify the contents of the file"
> > > 
> > > Btw, referring to "open" here is a little confusing, since NFSv3
> > > does
> > > not have an on-the-wire OPEN operation. I'm not sure how to
> > > clarify.
> > > 
> > > 
> > > > In order to acquire a filehandle, you must perform lookup
> > > > operations on the
> > > > parent directory(ies), and the permissions on those directories
> > > > may
> > > > prohibit you from walking into them to find the files within. 
> > > > This would
> > > > normally be considered sufficient protection on a local
> > > > filesystem to
> > > > prohibit users from accessing those files, however when the
> > > > filesystem is
> > > > exported via NFS those files can still be accessed by guessing
> > > > the correct,
> > > > valid filehandles.
> > > 
> > > Instead: "an exported file can be accessed whenever the NFS
> > > server is
> > > presented with the correct filehandle, which can be guessed or
> > > acquired
> > > by means other than LOOKUP."
> > > 
> > > 
> > > > Filehandles are easy to guess because they are well-formed. 
> > > > The
> > > > open_by_handle_at(2) man page contains an example C program
> > > > (t_name_to_handle_at.c) that can display a filehandle given a
> > > > path.  Here's
> > > > an example filehandle from a fairly modern XFS:
> > > > 
> > > > # ./t_name_to_handle_at /exports/foo
> > > > 57
> > > > 12 129    99 00 00 00 00 00 00 00 b4 10 0b 8c
> > > > 
> > > >           ^---------  filehandle  ----------^
> > > >           ^------- inode -------^ ^-- gen --^
> > > > 
> > > > This filehandle consists of a 64-bit inode number and 32-bit
> > > > generation
> > > > number.  Because the handle is well-formed, its easy to
> > > > fabricate
> > > > filehandles that match other files within the same filesystem. 
> > > > You can
> > > > simply insert inode numbers and iterate on the generation
> > > > number.
> > > > Eventually you'll be able to access the file using
> > > > open_by_handle_at(2).
> > > > For a local system, open_by_handle_at(2) requires
> > > > CAP_DAC_READ_SEARCH, which
> > > > protects against guessing attacks by unprivileged users.
> > > > 
> > > > In contrast to a local user using open_by_handle(2), the NFS
> > > > server must
> > > > permissively allow remote clients to open by filehandle without
> > > > being able
> > > > to check or trust the remote caller's access.
> 
> Btw, "allow ... clients to open by filehandle" is another confusion.
> 
> NFSv4 OPEN does do access checking and authorization.
> 
> Again, it's NFS READ and WRITE that are not blocked.
> 
> NFSv3 READ and WRITE do an intrinsic open.
> 
> NFSv4 READ and WRITE permit the use of a special stateid so that an
> OPEN
> isn't necessary to do the I/O (IIRC).
> 
> 
> > > > Therefore additional
> > > > protection against this attack is needed for NFS case.  We
> > > > propose to sign
> > > > filehandles by appending an 8-byte MAC which is the siphash of
> > > > the
> > > > filehandle from a key set from the nfs-utilities.  NFS server
> > > > can then
> > > > ensure that guessing a valid filehandle+MAC is practically
> > > > impossible
> > > > without knowledge of the MAC's key.  The NFS server performs
> > > > optional
> > > > signing by possessing a key set from userspace and having the
> > > > "sign_fh"
> > > > export option.
> > > 
> > > OK, I guess this is where I got the idea this would be an export
> > > option.
> > > 
> > > But I'm unconvinced that this provides any real security. There
> > > are
> > > other ways of obtaining a filehandle besides guessing, and
> > > nothing
> > > here suggests that guessing is the premier attack methodology.
> > 
> > Help me understand you - you're unconvinced that having the server
> > sign
> > filehandles and verify filehandles prevents clients from
> > fabricating valid
> > ones?
> 
> The rationale provided here doesn't convince me that fabrication is
> the
> biggest threat and will give us the biggest bang for our buck if it
> is
> mitigated.
> 
> In order to carry out this attack, the attacker has to have access to
> the filehandles on an NFS client to examine them. She has to have
> access to a valid client IP address to send NFS requests from. Maybe
> you can bridge the gap by explaining how a /non-root/ user on an NFS
> client might leverage FH fabrication to gain access to another user's
> files. I think only the root user has this ability.
> 
> I've also tried to convince myself that cryptographic FH validation
> could mitigate misdirected WRITEs or READs. An attacker could replace
> the FH in a valid NFS request, for example, or a client might send
> garbage in an FH. I'm not sure those are real problems, though.
> 
> I am keeping in mind that everything here is using AUTH_SYS anyway,
> so
> maybe I'm just an old man yelling at a cloud. Nothing here is
> blocking
> yet, but I want the feature to provide meaningful value (as I think
> you
> do).
> 
> Thank you for bearing with me.
> 

Yes, there may be a root squash option that prevents synthesis of root
credentials, but that's not necessarily an impediment:

We assume that the attacker does have access to a userspace NFS client
on the authorised client and that they can synthesize calls.
If so, then GETATTR is unprivileged and will happily give up knowledge
of which owner/group combination to synthesize in an OPEN, READ or
WRITE call.


-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Benjamin Coddington]

On 16 Jan 2026, at 14:43, Chuck Lever wrote:

> On 1/16/26 12:17 PM, Benjamin Coddington wrote:
>> On 16 Jan 2026, at 11:56, Chuck Lever wrote:
>>
>>> On Fri, Jan 16, 2026, at 9:32 AM, Benjamin Coddington wrote:
>>>> The following series enables the linux NFS server to add a Message
>>>> Authentication Code (MAC) to the filehandles it gives to clients.  This
>>>> provides additional protection to the exported filesystem against filehandle
>>>> guessing attacks.
>>>>
>>>> Filesystems generate their own filehandles through the export_operation
>>>> "encode_fh" and a filehandle provides sufficient access to open a file
>>>> without needing to perform a lookup.  An NFS client holding a valid
>>>> filehandle can remotely open and read the contents of the file referred to
>>>> by the filehandle.
>>>
>>> "open, read, or modify the contents of the file"
>>>
>>> Btw, referring to "open" here is a little confusing, since NFSv3 does
>>> not have an on-the-wire OPEN operation. I'm not sure how to clarify.
>>>
>>>
>>>> In order to acquire a filehandle, you must perform lookup operations on the
>>>> parent directory(ies), and the permissions on those directories may
>>>> prohibit you from walking into them to find the files within.  This would
>>>> normally be considered sufficient protection on a local filesystem to
>>>> prohibit users from accessing those files, however when the filesystem is
>>>> exported via NFS those files can still be accessed by guessing the correct,
>>>> valid filehandles.
>>>
>>> Instead: "an exported file can be accessed whenever the NFS server is
>>> presented with the correct filehandle, which can be guessed or acquired
>>> by means other than LOOKUP."
>>>
>>>
>>>> Filehandles are easy to guess because they are well-formed.  The
>>>> open_by_handle_at(2) man page contains an example C program
>>>> (t_name_to_handle_at.c) that can display a filehandle given a path.  Here's
>>>> an example filehandle from a fairly modern XFS:
>>>>
>>>> # ./t_name_to_handle_at /exports/foo
>>>> 57
>>>> 12 129    99 00 00 00 00 00 00 00 b4 10 0b 8c
>>>>
>>>>           ^---------  filehandle  ----------^
>>>>           ^------- inode -------^ ^-- gen --^
>>>>
>>>> This filehandle consists of a 64-bit inode number and 32-bit generation
>>>> number.  Because the handle is well-formed, its easy to fabricate
>>>> filehandles that match other files within the same filesystem.  You can
>>>> simply insert inode numbers and iterate on the generation number.
>>>> Eventually you'll be able to access the file using open_by_handle_at(2).
>>>> For a local system, open_by_handle_at(2) requires CAP_DAC_READ_SEARCH, which
>>>> protects against guessing attacks by unprivileged users.
>>>>
>>>> In contrast to a local user using open_by_handle(2), the NFS server must
>>>> permissively allow remote clients to open by filehandle without being able
>>>> to check or trust the remote caller's access.
>
> Btw, "allow ... clients to open by filehandle" is another confusion.
>
> NFSv4 OPEN does do access checking and authorization.
>
> Again, it's NFS READ and WRITE that are not blocked.

Ok - I will try to be more precise next time.. something like:
.. the NFS server allows operations against the file ..

>
> NFSv3 READ and WRITE do an intrinsic open.
>
> NFSv4 READ and WRITE permit the use of a special stateid so that an OPEN
> isn't necessary to do the I/O (IIRC).
>
>
>>>> Therefore additional
>>>> protection against this attack is needed for NFS case.  We propose to sign
>>>> filehandles by appending an 8-byte MAC which is the siphash of the
>>>> filehandle from a key set from the nfs-utilities.  NFS server can then
>>>> ensure that guessing a valid filehandle+MAC is practically impossible
>>>> without knowledge of the MAC's key.  The NFS server performs optional
>>>> signing by possessing a key set from userspace and having the "sign_fh"
>>>> export option.
>>>
>>> OK, I guess this is where I got the idea this would be an export option.
>>>
>>> But I'm unconvinced that this provides any real security. There are
>>> other ways of obtaining a filehandle besides guessing, and nothing
>>> here suggests that guessing is the premier attack methodology.
>>
>> Help me understand you - you're unconvinced that having the server sign
>> filehandles and verify filehandles prevents clients from fabricating valid
>> ones?
>
> The rationale provided here doesn't convince me that fabrication is the
> biggest threat and will give us the biggest bang for our buck if it is
> mitigated.

Ahh - ok.  I'm not trying to be convincing that its the biggest threat.  For
us, its a threat that doesn't have any counter measure yet.  For snooping
filehandles, we can guard against that with transport encryption (two
options there).  We're assuming that network security is a solved problem.

> In order to carry out this attack, the attacker has to have access to
> the filehandles on an NFS client to examine them.

Maybe - you can also fab them up from reading code or do it in the lab.
Maybe I should stop saying "after looking, easy to fab", and instead say
"easy to fab".

> She has to have access to a valid client IP address to send NFS requests
> from. Maybe you can bridge the gap by explaining how a /non-root/ user on
> an NFS client might leverage FH fabrication to gain access to another
> user's files. I think only the root user has this ability.

Let's not assume we have a perfect linux client.  We don't know what the
client implementation allows, or what bugs might be present there, or if
that implementation has been compromised in some way.  Whatever the client
is, we assume it might be able to send guessed filehandles because the
client implementation is out of the server's control and scope of trust.

That said, let's say we /do/ have a linux client - there's no reason to say
it can't be the root user there where root is squashed to that client, but
the root user can still use the path_to_filehandle_at() interfaces (as root
after all) to discover some filehandles and try to open by handle.

> I've also tried to convince myself that cryptographic FH validation
> could mitigate misdirected WRITEs or READs. An attacker could replace
> the FH in a valid NFS request, for example, or a client might send
> garbage in an FH. I'm not sure those are real problems, though.

For what we're trying to fix, those aren't in scope.  This work really isn't
interested in protecting against modifications on the wire or intercepting
other client's traffic on the wire and using that information.

> I am keeping in mind that everything here is using AUTH_SYS anyway, so
> maybe I'm just an old man yelling at a cloud. Nothing here is blocking
> yet, but I want the feature to provide meaningful value (as I think you
> do).
>
> Thank you for bearing with me.

Of course, I very much appreciate the improvements only these iterations can
produce.  Thanks very much, Chuck.

Ben

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

[kernel test robot]

Hi Benjamin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bfd453acb5637b5df881cef4b21803344aa9e7ac]

url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Coddington/nfsd-Convert-export-flags-to-use-BIT-macro/20260116-223927
base:   bfd453acb5637b5df881cef4b21803344aa9e7ac
patch link:    https://lore.kernel.org/r/c49d28aade36c044f0533d03b564ff65e00d9e05.1768573690.git.bcodding%40hammerspace.com
patch subject: [PATCH v1 2/4] nfsd: Add a key for signing filehandles
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260117/202601170520.GITVT8Iy-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260117/202601170520.GITVT8Iy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601170520.GITVT8Iy-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/nfsd/nfsctl.c:2282:50: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
    2282 |         trace_nfsd_ctl_fh_key_set((const char *)fh_key, ret);
         |                                                         ^~~
   fs/nfsd/nfsctl.c:2260:9: note: initialize the variable 'ret' to silence this warning
    2260 |         int ret;
         |                ^
         |                 = 0
   1 warning generated.


vim +/ret +2282 fs/nfsd/nfsctl.c

  2254	
  2255	int nfsd_nl_fh_key_set_doit(struct sk_buff *skb, struct genl_info *info)
  2256	{
  2257		siphash_key_t *fh_key;
  2258		struct nfsd_net *nn;
  2259		int fh_key_len;
  2260		int ret;
  2261	
  2262		if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_FH_KEY))
  2263			return -EINVAL;
  2264	
  2265		fh_key_len = nla_len(info->attrs[NFSD_A_SERVER_FH_KEY]);
  2266		if (fh_key_len != sizeof(siphash_key_t))
  2267			return -EINVAL;
  2268	
  2269		/* Is the key already set? */
  2270		nn = net_generic(genl_info_net(info), nfsd_net_id);
  2271		if (nn->fh_key)
  2272			return -EEXIST;
  2273	
  2274		fh_key = kmalloc(sizeof(siphash_key_t), GFP_KERNEL);
  2275		if (!fh_key)
  2276			return -ENOMEM;
  2277	
  2278		memcpy(fh_key, nla_data(info->attrs[NFSD_A_SERVER_FH_KEY]), sizeof(siphash_key_t));
  2279		nn = net_generic(genl_info_net(info), nfsd_net_id);
  2280		nn->fh_key = fh_key;
  2281	
> 2282		trace_nfsd_ctl_fh_key_set((const char *)fh_key, ret);
  2283		return ret;
  2284	}
  2285	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 4/4] NFSD: Sign filehandles

[kernel test robot]

Hi Benjamin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bfd453acb5637b5df881cef4b21803344aa9e7ac]

url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Coddington/nfsd-Convert-export-flags-to-use-BIT-macro/20260116-223927
base:   bfd453acb5637b5df881cef4b21803344aa9e7ac
patch link:    https://lore.kernel.org/r/9c9cd6131574a45f2603c9248ba205a79b00fea3.1768573690.git.bcodding%40hammerspace.com
patch subject: [PATCH v1 4/4] NFSD: Sign filehandles
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20260117/202601170612.qqi8Q8Xx-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260117/202601170612.qqi8Q8Xx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601170612.qqi8Q8Xx-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/nfsd/nfsfh.c:162:30: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
     161 |                 pr_warn("NFSD: unable to sign filehandles, fh_size %lu would be greater"
         |                                                                    ~~~
         |                                                                    %u
     162 |                         " than fh_maxsize %d.\n", fh->fh_size + sizeof(hash), fhp->fh_maxsize);
         |                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/printk.h:565:37: note: expanded from macro 'pr_warn'
     565 |         printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
         |                                    ~~~     ^~~~~~~~~~~
   include/linux/printk.h:512:60: note: expanded from macro 'printk'
     512 | #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
         |                                                     ~~~    ^~~~~~~~~~~
   include/linux/printk.h:484:19: note: expanded from macro 'printk_index_wrap'
     484 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ~~~~    ^~~~~~~~~~~
   1 warning generated.


vim +162 fs/nfsd/nfsfh.c

   140	
   141	/*
   142	 * Intended to be called when encoding, appends an 8-byte MAC
   143	 * to the filehandle hashed from the server's fh_key:
   144	 */
   145	int fh_append_mac(struct svc_fh *fhp, struct net *net)
   146	{
   147		struct nfsd_net *nn = net_generic(net, nfsd_net_id);
   148		struct knfsd_fh *fh = &fhp->fh_handle;
   149		siphash_key_t *fh_key = nn->fh_key;
   150		u64 hash;
   151	
   152		if (!(fhp->fh_export->ex_flags & NFSEXP_SIGN_FH))
   153			return 0;
   154	
   155		if (!fh_key) {
   156			pr_warn("NFSD: unable to sign filehandles, fh_key not set.\n");
   157			return -EINVAL;
   158		}
   159	
   160		if (fh->fh_size + sizeof(hash) > fhp->fh_maxsize) {
   161			pr_warn("NFSD: unable to sign filehandles, fh_size %lu would be greater"
 > 162				" than fh_maxsize %d.\n", fh->fh_size + sizeof(hash), fhp->fh_maxsize);
   163			return -EINVAL;
   164		}
   165	
   166		fh->fh_auth_type = FH_AT_MAC;
   167		hash = siphash(&fh->fh_raw, fh->fh_size, fh_key);
   168		memcpy(&fh->fh_raw[fh->fh_size], &hash, sizeof(hash));
   169		fh->fh_size += sizeof(hash);
   170	
   171		return 0;
   172	}
   173	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 4/4] NFSD: Sign filehandles

[NeilBrown]

On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>  
> -	if (fileid_type == FILEID_ROOT)
> +	if (fileid_type == FILEID_ROOT) {
>  		dentry = dget(exp->ex_path.dentry);
> -	else {
> +	} else {
> +		/* Root filehandle always unsigned because rpc.mountd has no key */

I don't think this is correct.
rpc.mountd always asks the kernel for a filehandle, so it doesn't need a
key.

However signing the root filehandle would be pointless as the client can
"PUT_ROOTFH" without needing to provide a signature.
So I'm happy with the root not being signed, I'm not happy with the
justification.

NeilBrown

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[NeilBrown]

On Sat, 17 Jan 2026, Benjamin Coddington wrote:
> The following series enables the linux NFS server to add a Message
> Authentication Code (MAC) to the filehandles it gives to clients.  This
> provides additional protection to the exported filesystem against filehandle
> guessing attacks.
> 
> Filesystems generate their own filehandles through the export_operation
> "encode_fh" and a filehandle provides sufficient access to open a file
> without needing to perform a lookup.  An NFS client holding a valid
> filehandle can remotely open and read the contents of the file referred to
> by the filehandle.

A *trusted* NFS client holding a valid filehandle can remotely access
the corresponding file without reference to access-path restrictions
that might be imposed by the ancestor directories or the server exports.

I think that last part is key to understanding what you are trying to
do.  You are trying to enforce path-based restriction in NFS.  And due
to the various ways that a path and be traversed - e.g.  different
principles for different components, or some traversal on server, some
on client - this effectively means you need a capability framework.

So you are enhancing the filehandle to act as a capability by adding a
MAC.

> 
> In order to acquire a filehandle, you must perform lookup operations on the
> parent directory(ies), and the permissions on those directories may
> prohibit you from walking into them to find the files within.  This would
> normally be considered sufficient protection on a local filesystem to
> prohibit users from accessing those files, however when the filesystem is
> exported via NFS those files can still be accessed by guessing the correct,
> valid filehandles.
> 
> Filehandles are easy to guess because they are well-formed.  The
> open_by_handle_at(2) man page contains an example C program
> (t_name_to_handle_at.c) that can display a filehandle given a path.  Here's
> an example filehandle from a fairly modern XFS:
> 
> # ./t_name_to_handle_at /exports/foo 
> 57
> 12 129    99 00 00 00 00 00 00 00 b4 10 0b 8c
> 
>           ^---------  filehandle  ----------^
>           ^------- inode -------^ ^-- gen --^
> 
> This filehandle consists of a 64-bit inode number and 32-bit generation
> number.  Because the handle is well-formed, its easy to fabricate
> filehandles that match other files within the same filesystem.  You can
> simply insert inode numbers and iterate on the generation number.
> Eventually you'll be able to access the file using open_by_handle_at(2).
> For a local system, open_by_handle_at(2) requires CAP_DAC_READ_SEARCH, which
> protects against guessing attacks by unprivileged users.

Iterating a 32 bit generation number would be expected to take a long
time to succeed - except that they tend to cluster early.  Though in
your example the msb is 1!

Do you have exploit code which demonstrates unauthorised access to a
given inode number?  What runtime?  Could attack-detection in the server
be a simple effective counter-measure?  Should we do that anyway?

Supporting and encouraging the use of 64-bit generation numbers, and
starting at a random offset would do a lot to make guessing harder.
A crypto key could be part of making this number hard to guess.

> 
> In contrast to a local user using open_by_handle(2), the NFS server must
> permissively allow remote clients to open by filehandle without being able
> to check or trust the remote caller's access.

I find the above sufficiently vague and confusing that I cannot tell if
I agree...

open_by_handle_at(2) requires CAP_DAC_READ_SEARCH precisely because no
path-based restrictions are imposed.
The NFS server cannot require CAP_DAC_READ_SEARCH and assumes the trusted 
client performs the necessary path-based access checks.
When the client cannot be completely trusted, and path-based access
controls are depended on, extra support is needed for NFS.


>                                                        Therefore additional
> protection against this attack is needed for NFS case.  We propose to sign
> filehandles by appending an 8-byte MAC which is the siphash of the
> filehandle from a key set from the nfs-utilities.  NFS server can then
> ensure that guessing a valid filehandle+MAC is practically impossible
> without knowledge of the MAC's key.  The NFS server performs optional
> signing by possessing a key set from userspace and having the "sign_fh"
> export option.


NeilBrown

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Benjamin Coddington]

On 16 Jan 2026, at 20:24, NeilBrown wrote:

> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>> The following series enables the linux NFS server to add a Message
>> Authentication Code (MAC) to the filehandles it gives to clients.  This
>> provides additional protection to the exported filesystem against filehandle
>> guessing attacks.
>>
>> Filesystems generate their own filehandles through the export_operation
>> "encode_fh" and a filehandle provides sufficient access to open a file
>> without needing to perform a lookup.  An NFS client holding a valid
>> filehandle can remotely open and read the contents of the file referred to
>> by the filehandle.
>
> A *trusted* NFS client holding a valid filehandle can remotely access
> the corresponding file without reference to access-path restrictions
> that might be imposed by the ancestor directories or the server exports.

Mind if I use your words next time?  I'm thinking that most of this
cover-letter should end up in the docs.

> I think that last part is key to understanding what you are trying to
> do.  You are trying to enforce path-based restriction in NFS.  And due
> to the various ways that a path and be traversed - e.g.  different
> principles for different components, or some traversal on server, some
> on client - this effectively means you need a capability framework.
>
> So you are enhancing the filehandle to act as a capability by adding a
> MAC.

Yes, that's a great way to think about it.

>> In order to acquire a filehandle, you must perform lookup operations on the
>> parent directory(ies), and the permissions on those directories may
>> prohibit you from walking into them to find the files within.  This would
>> normally be considered sufficient protection on a local filesystem to
>> prohibit users from accessing those files, however when the filesystem is
>> exported via NFS those files can still be accessed by guessing the correct,
>> valid filehandles.
>>
>> Filehandles are easy to guess because they are well-formed.  The
>> open_by_handle_at(2) man page contains an example C program
>> (t_name_to_handle_at.c) that can display a filehandle given a path.  Here's
>> an example filehandle from a fairly modern XFS:
>>
>> # ./t_name_to_handle_at /exports/foo
>> 57
>> 12 129    99 00 00 00 00 00 00 00 b4 10 0b 8c
>>
>>           ^---------  filehandle  ----------^
>>           ^------- inode -------^ ^-- gen --^
>>
>> This filehandle consists of a 64-bit inode number and 32-bit generation
>> number.  Because the handle is well-formed, its easy to fabricate
>> filehandles that match other files within the same filesystem.  You can
>> simply insert inode numbers and iterate on the generation number.
>> Eventually you'll be able to access the file using open_by_handle_at(2).
>> For a local system, open_by_handle_at(2) requires CAP_DAC_READ_SEARCH, which
>> protects against guessing attacks by unprivileged users.
>
> Iterating a 32 bit generation number would be expected to take a long
> time to succeed - except that they tend to cluster early.  Though in
> your example the msb is 1!

Trond posited that with a 1ms round-trip and 50 parallel GETATTRs it only
takes one day.

> Do you have exploit code which demonstrates unauthorised access to a
> given inode number?  What runtime?  Could attack-detection in the server
> be a simple effective counter-measure?  Should we do that anyway?

Yes, its a modification of t_open_by_handle_at.c example program in the
open_by_handle_at(2) man page.  On my single system NFS client and server
with a local mount, I averaged 16usec per open, and discovered my target
filehandle in less than an hour.  I didn't have any network latency to worry
about, but I think it still shows its possible and a determined attacker can
do it.

The server could be modified to notice elevated counts of error returns for
a client and then try to notify about it.   But, I don't think it will be
simple - I imagine it would need a lot of tunable (how many failed fh, at
what rate..  etc) because you need to tune the system to make a signal from
the noise of regular operations and returns.  That tuning can be worked
around by a very determined attacker.  You end up in a behavior
detection/modification feedback loop and the server's not guaranteed to
catch everything.  Still it would be another layer of defense-in-depth that
would have value.

> Supporting and encouraging the use of 64-bit generation numbers, and
> starting at a random offset would do a lot to make guessing harder.
> A crypto key could be part of making this number hard to guess.
>
>>
>> In contrast to a local user using open_by_handle(2), the NFS server must
>> permissively allow remote clients to open by filehandle without being able
>> to check or trust the remote caller's access.
>
> I find the above sufficiently vague and confusing that I cannot tell if
> I agree...
>
> open_by_handle_at(2) requires CAP_DAC_READ_SEARCH precisely because no
> path-based restrictions are imposed.
> The NFS server cannot require CAP_DAC_READ_SEARCH and assumes the trusted
> client performs the necessary path-based access checks.
> When the client cannot be completely trusted, and path-based access
> controls are depended on, extra support is needed for NFS.

Much better than mine, I'll use it.  Thanks for looking Neil.

Ben

>>                                                        Therefore additional
>> protection against this attack is needed for NFS case.  We propose to sign
>> filehandles by appending an 8-byte MAC which is the siphash of the
>> filehandle from a key set from the nfs-utilities.  NFS server can then
>> ensure that guessing a valid filehandle+MAC is practically impossible
>> without knowledge of the MAC's key.  The NFS server performs optional
>> signing by possessing a key set from userspace and having the "sign_fh"
>> export option.
>
>
> NeilBrown

Re: [PATCH v1 4/4] NFSD: Sign filehandles

[Benjamin Coddington]

On 16 Jan 2026, at 19:54, NeilBrown wrote:

> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>>
>> -	if (fileid_type == FILEID_ROOT)
>> +	if (fileid_type == FILEID_ROOT) {
>>  		dentry = dget(exp->ex_path.dentry);
>> -	else {
>> +	} else {
>> +		/* Root filehandle always unsigned because rpc.mountd has no key */
>
> I don't think this is correct.
> rpc.mountd always asks the kernel for a filehandle, so it doesn't need a
> key.
>
> However signing the root filehandle would be pointless as the client can
> "PUT_ROOTFH" without needing to provide a signature.
> So I'm happy with the root not being signed, I'm not happy with the
> justification.

ah yes - I will just drop the comment.

Ben

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[NeilBrown]

On Sat, 17 Jan 2026, Benjamin Coddington wrote:
> On 16 Jan 2026, at 20:24, NeilBrown wrote:
> 
> > On Sat, 17 Jan 2026, Benjamin Coddington wrote:
> >> The following series enables the linux NFS server to add a Message
> >> Authentication Code (MAC) to the filehandles it gives to clients.  This
> >> provides additional protection to the exported filesystem against filehandle
> >> guessing attacks.
> >>
> >> Filesystems generate their own filehandles through the export_operation
> >> "encode_fh" and a filehandle provides sufficient access to open a file
> >> without needing to perform a lookup.  An NFS client holding a valid
> >> filehandle can remotely open and read the contents of the file referred to
> >> by the filehandle.
> >
> > A *trusted* NFS client holding a valid filehandle can remotely access
> > the corresponding file without reference to access-path restrictions
> > that might be imposed by the ancestor directories or the server exports.
> 
> Mind if I use your words next time?  I'm thinking that most of this
> cover-letter should end up in the docs.

Please do!

> >
> > Iterating a 32 bit generation number would be expected to take a long
> > time to succeed - except that they tend to cluster early.  Though in
> > your example the msb is 1!
> 
> Trond posited that with a 1ms round-trip and 50 parallel GETATTRs it only
> takes one day.
> 
> > Do you have exploit code which demonstrates unauthorised access to a
> > given inode number?  What runtime?  Could attack-detection in the server
> > be a simple effective counter-measure?  Should we do that anyway?
> 
> Yes, its a modification of t_open_by_handle_at.c example program in the
> open_by_handle_at(2) man page.  On my single system NFS client and server
> with a local mount, I averaged 16usec per open, and discovered my target
> filehandle in less than an hour.  I didn't have any network latency to worry
> about, but I think it still shows its possible and a determined attacker can
> do it.

This information would be useful to include in the cover letter/documentation.

> 
> The server could be modified to notice elevated counts of error returns for
> a client and then try to notify about it.   But, I don't think it will be
> simple - I imagine it would need a lot of tunable (how many failed fh, at
> what rate..  etc) because you need to tune the system to make a signal from
> the noise of regular operations and returns.  That tuning can be worked
> around by a very determined attacker.  You end up in a behavior
> detection/modification feedback loop and the server's not guaranteed to
> catch everything.  Still it would be another layer of defense-in-depth that
> would have value.

I would like to explore this, at least for the defense-in-depth
rationale.  Maybe it would also supply some protection wehn sign_fh
isn't enabled.

We would need to monitor the result of exportfs_decode_fh_raw() for
stale vs non-stale, and if the proportion of stale filehandles (on a
given export) exceeds some low threshold (1%?) over a modest time period
(5 minutes?) then .... what?

I don't think a hard fail would be a good idea, but maybe serialise
future requests from that auth_domain and impose a delay on any stale
filehandles until the proportion drops below some lower threshold??

Does anyone else thinks this would be worth pursuing?

Thanks,
NeilBrown

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Christian Brauner]

On Fri, Jan 16, 2026 at 09:32:10AM -0500, Benjamin Coddington wrote:
> The following series enables the linux NFS server to add a Message
> Authentication Code (MAC) to the filehandles it gives to clients.  This
> provides additional protection to the exported filesystem against filehandle
> guessing attacks.
> 
> Filesystems generate their own filehandles through the export_operation
> "encode_fh" and a filehandle provides sufficient access to open a file
> without needing to perform a lookup.  An NFS client holding a valid
> filehandle can remotely open and read the contents of the file referred to
> by the filehandle.
> 
> In order to acquire a filehandle, you must perform lookup operations on the
> parent directory(ies), and the permissions on those directories may
> prohibit you from walking into them to find the files within.  This would
> normally be considered sufficient protection on a local filesystem to
> prohibit users from accessing those files, however when the filesystem is
> exported via NFS those files can still be accessed by guessing the correct,
> valid filehandles.
> 

I really like this concept (I think Lennart has talked about support for
this before?).

Fwiw, I would really like if nsfs and pidfs file handles were signed
specifically because they're not supposed to persist across reboot. But
we can't do that in a backward compatbile way because userspace accesses
the file handle directly to get e.g., the inode number out of it.

But for new types of file handles for such pseudo-fses I think this
would be very lovely to have. It would probably mean generating a
per-boot key that is used to sign such file handles.

For nfs I understand that you probably outsource the problem to
userspace how to generate and share the key.

> Filehandles are easy to guess because they are well-formed.  The
> open_by_handle_at(2) man page contains an example C program
> (t_name_to_handle_at.c) that can display a filehandle given a path.  Here's
> an example filehandle from a fairly modern XFS:
> 
> # ./t_name_to_handle_at /exports/foo 
> 57
> 12 129    99 00 00 00 00 00 00 00 b4 10 0b 8c
> 
>           ^---------  filehandle  ----------^
>           ^------- inode -------^ ^-- gen --^
> 
> This filehandle consists of a 64-bit inode number and 32-bit generation
> number.  Because the handle is well-formed, its easy to fabricate
> filehandles that match other files within the same filesystem.  You can
> simply insert inode numbers and iterate on the generation number.
> Eventually you'll be able to access the file using open_by_handle_at(2).
> For a local system, open_by_handle_at(2) requires CAP_DAC_READ_SEARCH, which
> protects against guessing attacks by unprivileged users.

Fyi, it's not so simple. For some filesystems like pidfs and nsfs they
have custom permission checks and are available to unprivileged users.
But they also don't do any path lookup ofc.

> 
> In contrast to a local user using open_by_handle(2), the NFS server must
> permissively allow remote clients to open by filehandle without being able
> to check or trust the remote caller's access. Therefore additional
> protection against this attack is needed for NFS case.  We propose to sign
> filehandles by appending an 8-byte MAC which is the siphash of the
> filehandle from a key set from the nfs-utilities.  NFS server can then
> ensure that guessing a valid filehandle+MAC is practically impossible
> without knowledge of the MAC's key.  The NFS server performs optional
> signing by possessing a key set from userspace and having the "sign_fh"
> export option.
> 
> Because filehandles are long-lived, and there's no method for expiring them,
> the server's key should be set once and not changed.  It also should be
> persisted across restarts.  The methods to set the key allow only setting it

Right, otherwise you break a ton of stuff that relies on stable file
handles across reboots.

> once, afterward it cannot be changed.  A separate patchset for nfs-utils
> contains the userspace changes required to set the server's key.
> 
> I had a previous attempt to solve this problem by encrypting filehandles,
> which turned out to be a problematic, poor solution.  The discussion on
> that previous attempt is here:
> https://lore.kernel.org/linux-nfs/510E10A4-11BE-412D-93AF-C4CC969954E7@hammerspace.com/T/#t
> 
> There are some changes from that version:
> 	- sign filehandles instead of encrypt them (Eric Biggers)
> 	- fix the NFSEXP_ macros, specifically NFSEXP_ALLFLAGS (NeilBrown)
> 	- rebase onto cel/nfsd-next (Chuck Lever)
> 	- condensed/clarified problem explantion (thanks Chuck Lever)
> 	- add nfsctl file "fh_key" for rpc.nfsd to also set the key
> 
> I plan on adding additional work to enable the server to check whether the
> 8-byte MAC will overflow maximum filehandle length for the protocol at
> export time.  There could be some filesystems with 40-byte fileid and
> 24-byte fsid which would break NFSv3's 64-byte filehandle maximum with an
> 8-byte MAC appended.  The server should refuse to export those filesystems
> when "sign_fh" is requested.
> 
> Thanks for any comments and critique.
> 
> Benjamin Coddington (4):
>   nfsd: Convert export flags to use BIT() macro
>   nfsd: Add a key for signing filehandles
>   NFSD/export: Add sign_fh export option
>   NFSD: Sign filehandles
> 
>  Documentation/netlink/specs/nfsd.yaml | 12 ++++
>  fs/nfsd/export.c                      |  5 +-
>  fs/nfsd/netlink.c                     | 15 +++++
>  fs/nfsd/netlink.h                     |  1 +
>  fs/nfsd/netns.h                       |  2 +
>  fs/nfsd/nfs3xdr.c                     | 20 +++---
>  fs/nfsd/nfs4xdr.c                     | 12 ++--
>  fs/nfsd/nfsctl.c                      | 87 ++++++++++++++++++++++++++-
>  fs/nfsd/nfsfh.c                       | 72 +++++++++++++++++++++-
>  fs/nfsd/nfsfh.h                       | 22 +++++++
>  fs/nfsd/trace.h                       | 19 ++++++
>  include/linux/sunrpc/svc.h            |  1 +
>  include/uapi/linux/nfsd/export.h      | 38 ++++++------
>  include/uapi/linux/nfsd_netlink.h     |  2 +
>  14 files changed, 272 insertions(+), 36 deletions(-)
> 
> 
> base-commit: bfd453acb5637b5df881cef4b21803344aa9e7ac
> -- 
> 2.50.1
> 

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

[Benjamin Coddington]

On 16 Jan 2026, at 9:59, 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.

Ok - will do on v2.

Ben

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[NeilBrown]

On Mon, 19 Jan 2026, Christian Brauner wrote:
> On Fri, Jan 16, 2026 at 09:32:10AM -0500, Benjamin Coddington wrote:
> > The following series enables the linux NFS server to add a Message
> > Authentication Code (MAC) to the filehandles it gives to clients.  This
> > provides additional protection to the exported filesystem against filehandle
> > guessing attacks.
> > 
> > Filesystems generate their own filehandles through the export_operation
> > "encode_fh" and a filehandle provides sufficient access to open a file
> > without needing to perform a lookup.  An NFS client holding a valid
> > filehandle can remotely open and read the contents of the file referred to
> > by the filehandle.
> > 
> > In order to acquire a filehandle, you must perform lookup operations on the
> > parent directory(ies), and the permissions on those directories may
> > prohibit you from walking into them to find the files within.  This would
> > normally be considered sufficient protection on a local filesystem to
> > prohibit users from accessing those files, however when the filesystem is
> > exported via NFS those files can still be accessed by guessing the correct,
> > valid filehandles.
> > 
> 
> I really like this concept (I think Lennart has talked about support for
> this before?).
> 
> Fwiw, I would really like if nsfs and pidfs file handles were signed
> specifically because they're not supposed to persist across reboot. But
> we can't do that in a backward compatbile way because userspace accesses
> the file handle directly to get e.g., the inode number out of it.

You don't need signing to ensure a filehandle doesn't persist across
reboot.  For that you just need a generation number.  Storing a random
number generated at boot time in the filehandle would be a good solution.

The only reason we need signing is because filesystems only provide
32bits of generation number.  If a filesystem stored 64 bits, and used a
crypto-safe random number for the generation number, then we wouldn't
need signing or a key.

We need a key, effectively, to turn a 32bit number that can be iterated
into a 64bit number which cannot, in a non-reversible way.

Does userspace refuse the extract the inode number if the filehandle
size changes?  It it can cope with size change, then adding a random
number to the end of the filehandle should not be a problem.

> 
> But for new types of file handles for such pseudo-fses I think this
> would be very lovely to have. It would probably mean generating a
> per-boot key that is used to sign such file handles.

For new fses I recommend including a 64bit random number.  No signing.

> 
> For nfs I understand that you probably outsource the problem to
> userspace how to generate and share the key.
> 
> > Filehandles are easy to guess because they are well-formed.  The
> > open_by_handle_at(2) man page contains an example C program
> > (t_name_to_handle_at.c) that can display a filehandle given a path.  Here's
> > an example filehandle from a fairly modern XFS:
> > 
> > # ./t_name_to_handle_at /exports/foo 
> > 57
> > 12 129    99 00 00 00 00 00 00 00 b4 10 0b 8c
> > 
> >           ^---------  filehandle  ----------^
> >           ^------- inode -------^ ^-- gen --^
> > 
> > This filehandle consists of a 64-bit inode number and 32-bit generation
> > number.  Because the handle is well-formed, its easy to fabricate
> > filehandles that match other files within the same filesystem.  You can
> > simply insert inode numbers and iterate on the generation number.
> > Eventually you'll be able to access the file using open_by_handle_at(2).
> > For a local system, open_by_handle_at(2) requires CAP_DAC_READ_SEARCH, which
> > protects against guessing attacks by unprivileged users.
> 
> Fyi, it's not so simple. For some filesystems like pidfs and nsfs they
> have custom permission checks and are available to unprivileged users.
> But they also don't do any path lookup ofc.

I didn't know that.....
Oh, there is a "permission" operation now:

 * permission:
 *    Allow filesystems to specify a custom permission function.

Not the most useful documentation I have ever read.
Not documented in Documentation/filesystems/exporting.rst

Not used in fs/exportfs/.
Ahhh.. used in fs/fhandle.c to bypass may_decode_fh()

Fair enough - seems sane for a special-purpose filesystem to give away
different access.

Thanks for the info.

I wonder if nfsd should refuse to export filesystems which have a
.permission function, as they clearly are something special.

NeilBrown

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Jeff Layton]

On Tue, 2026-01-20 at 08:06 +1100, NeilBrown wrote:
> 
> 
> Thanks for the info.
> 
> I wonder if nfsd should refuse to export filesystems which have a
> .permission function, as they clearly are something special.
> 
> 

That would exclude a lot of filesystems that are currently exportable
(including NFS). I don't think we want to do that.

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[NeilBrown]

On Tue, 20 Jan 2026, Jeff Layton wrote:
> On Tue, 2026-01-20 at 08:06 +1100, NeilBrown wrote:
> > 
> > 
> > Thanks for the info.
> > 
> > I wonder if nfsd should refuse to export filesystems which have a
> > .permission function, as they clearly are something special.
> > 
> > 
> 
> That would exclude a lot of filesystems that are currently exportable
> (including NFS). I don't think we want to do that.

I was referring to the .permission function in export_operations, not
the one in inode_operations (and no, it isn't confusing at all that
both structs have the ops with the same name but different
function....).

NFS doesn't define .permission in export_operations.  Only pidfs and
nsfs do.

NeilBrown

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

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Christian Brauner]

> You don't need signing to ensure a filehandle doesn't persist across
> reboot.  For that you just need a generation number.  Storing a random
> number generated at boot time in the filehandle would be a good solution.

For pidfs I went with the 64-bit inode number. But I dislike the
generation number thing. If I would have to freedom to completely redo
it I would probably assign a uuid to the pidfs sb and then use that in
the file handles alongside the inode number. That would be enough for
sure as the uuid would change on each boot.

> The only reason we need signing is because filesystems only provide
> 32bits of generation number.  If a filesystem stored 64 bits, and used a
> crypto-safe random number for the generation number, then we wouldn't
> need signing or a key.
> 
> We need a key, effectively, to turn a 32bit number that can be iterated
> into a 64bit number which cannot, in a non-reversible way.
> 
> Does userspace refuse the extract the inode number if the filehandle
> size changes?  It it can cope with size change, then adding a random
> number to the end of the filehandle should not be a problem.

At least nsfs file handles are public api and may grow in size.

> I didn't know that.....
> Oh, there is a "permission" operation now:
> 
>  * permission:
>  *    Allow filesystems to specify a custom permission function.
> 
> Not the most useful documentation I have ever read.
> Not documented in Documentation/filesystems/exporting.rst
> 
> Not used in fs/exportfs/.
> Ahhh.. used in fs/fhandle.c to bypass may_decode_fh()
> 
> Fair enough - seems sane for a special-purpose filesystem to give away
> different access.
> 
> Thanks for the info.
> 
> I wonder if nfsd should refuse to export filesystems which have a
> .permission function, as they clearly are something special.

No problem. pidfs and nsfs have custom permission and open because they
both don't need to reconstruct any paths and aren't subject to the same
permission checking.

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[NeilBrown]

On Tue, 20 Jan 2026, Christian Brauner wrote:
> > You don't need signing to ensure a filehandle doesn't persist across
> > reboot.  For that you just need a generation number.  Storing a random
> > number generated at boot time in the filehandle would be a good solution.
> 
> For pidfs I went with the 64-bit inode number. But I dislike the
> generation number thing. If I would have to freedom to completely redo
> it I would probably assign a uuid to the pidfs sb and then use that in
> the file handles alongside the inode number. That would be enough for
> sure as the uuid would change on each boot.

What you are calling a "uuid" in "the pidfs sb" is exactly what I am
calling a "generation number" - for pidfs it would be a "generation
number" for the whole filesystem, while for ext4 etc it is a generation
number of the inode number.

So we are substantially in agreement.

Why do you not have freedom to add a uuid to the pidfs sb and to the
filehandles now?

Thanks,
NeilBrown

> 
> > The only reason we need signing is because filesystems only provide
> > 32bits of generation number.  If a filesystem stored 64 bits, and used a
> > crypto-safe random number for the generation number, then we wouldn't
> > need signing or a key.
> > 
> > We need a key, effectively, to turn a 32bit number that can be iterated
> > into a 64bit number which cannot, in a non-reversible way.
> > 
> > Does userspace refuse the extract the inode number if the filehandle
> > size changes?  It it can cope with size change, then adding a random
> > number to the end of the filehandle should not be a problem.
> 
> At least nsfs file handles are public api and may grow in size.
> 
> > I didn't know that.....
> > Oh, there is a "permission" operation now:
> > 
> >  * permission:
> >  *    Allow filesystems to specify a custom permission function.
> > 
> > Not the most useful documentation I have ever read.
> > Not documented in Documentation/filesystems/exporting.rst
> > 
> > Not used in fs/exportfs/.
> > Ahhh.. used in fs/fhandle.c to bypass may_decode_fh()
> > 
> > Fair enough - seems sane for a special-purpose filesystem to give away
> > different access.
> > 
> > Thanks for the info.
> > 
> > I wonder if nfsd should refuse to export filesystems which have a
> > .permission function, as they clearly are something special.
> 
> No problem. pidfs and nsfs have custom permission and open because they
> both don't need to reconstruct any paths and aren't subject to the same
> permission checking.
> 

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[NeilBrown]

On Sat, 17 Jan 2026, Benjamin Coddington wrote:
> The following series enables the linux NFS server to add a Message
> Authentication Code (MAC) to the filehandles it gives to clients.  This
> provides additional protection to the exported filesystem against filehandle
> guessing attacks.

I've pondering this some more and I think we can get the same result
without any API change, though it would require internal changes to each
filesystem of interest.

The problem can be rephrased as saying that 32 bits is too small for a
generation number as it allows guessing in a modest time.  A 64bit
generation number would provide sufficient protection (though admittedly
not quite as much as a 32bit gen number with a 64 bit MAC).

If a filesystem had 64 bits or more of random bits in the superblock
which are stable and not visible over NFS, it could combine those with
the 32bit gen number stored in the inode and present a 64 bit generation
number in the filehandle which would, in practice, be unguessable.

ext4 has s_journal_uuid which is stable and (I think) is not visible.
xfs has sb_meta_uuid which seems to fill a vaguely similar role.
I haven't looked further afield but it is credible that filesystems
either have some suitable bits, or could add them.

We could add a generic_encode_ino32_gen64_fh() which takes a uuid or
similar, and some corresponding support for verifying the gen64 on
decode.

This would remove the need to change nfs-utils at all.

This would mean the file handles change when you upgrade to a new kernel
(though a CONFIG option would could give distros control of when to make
the change), but nfsd it able to handle that if we are careful.
Lookups from an "old" style filehandle will always produce an "old"
style filehandle.  But when the client remounts they get a new style for
everything.

This is a lot more work in the kernel and requires buy-in from each
relevant fs maintainer, but saves us any API change and so is largely
transparent.

Is it worth it?  I don't know.

NeilBrown

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Christian Brauner]

On Tue, Jan 20, 2026 at 08:46:01PM +1100, NeilBrown wrote:
> On Tue, 20 Jan 2026, Christian Brauner wrote:
> > > You don't need signing to ensure a filehandle doesn't persist across
> > > reboot.  For that you just need a generation number.  Storing a random
> > > number generated at boot time in the filehandle would be a good solution.
> > 
> > For pidfs I went with the 64-bit inode number. But I dislike the
> > generation number thing. If I would have to freedom to completely redo
> > it I would probably assign a uuid to the pidfs sb and then use that in
> > the file handles alongside the inode number. That would be enough for
> > sure as the uuid would change on each boot.
> 
> What you are calling a "uuid" in "the pidfs sb" is exactly what I am
> calling a "generation number" - for pidfs it would be a "generation

"generation number" just evokes the 32-bit identifier in struct inode
that's overall somewhat useless. And a UUID has much stronger
guarantees.

> number" for the whole filesystem, while for ext4 etc it is a generation
> number of the inode number.
> 
> So we are substantially in agreement.

Great!

> 
> Why do you not have freedom to add a uuid to the pidfs sb and to the
> filehandles now?

Userspace relies on the current format to get the inode number from the
file handle:
https://github.com/systemd/systemd/blob/main/src/basic/pidfd-util.c#L233-L281

And they often also construct them in userspace. That needs to continue
to work. I also don't think it's that critical.

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[NeilBrown]

On Tue, 20 Jan 2026, Christian Brauner wrote:
> On Tue, Jan 20, 2026 at 08:46:01PM +1100, NeilBrown wrote:
> > On Tue, 20 Jan 2026, Christian Brauner wrote:
> > > > You don't need signing to ensure a filehandle doesn't persist across
> > > > reboot.  For that you just need a generation number.  Storing a random
> > > > number generated at boot time in the filehandle would be a good solution.
> > > 
> > > For pidfs I went with the 64-bit inode number. But I dislike the
> > > generation number thing. If I would have to freedom to completely redo
> > > it I would probably assign a uuid to the pidfs sb and then use that in
> > > the file handles alongside the inode number. That would be enough for
> > > sure as the uuid would change on each boot.
> > 
> > What you are calling a "uuid" in "the pidfs sb" is exactly what I am
> > calling a "generation number" - for pidfs it would be a "generation
> 
> "generation number" just evokes the 32-bit identifier in struct inode
> that's overall somewhat useless. And a UUID has much stronger
> guarantees.
> 
> > number" for the whole filesystem, while for ext4 etc it is a generation
> > number of the inode number.
> > 
> > So we are substantially in agreement.
> 
> Great!
> 
> > 
> > Why do you not have freedom to add a uuid to the pidfs sb and to the
> > filehandles now?
> 
> Userspace relies on the current format to get the inode number from the
> file handle:
> https://github.com/systemd/systemd/blob/main/src/basic/pidfd-util.c#L233-L281

The
  assert(r != -EOVERFLOW);
means you cannot extend it

> 
> And they often also construct them in userspace. That needs to continue
> to work. I also don't think it's that critical.
> 

as does this.

OK, thanks.

NeilBrown

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Christian Brauner]

On Tue, Jan 20, 2026 at 09:28:13PM +1100, NeilBrown wrote:
> On Tue, 20 Jan 2026, Christian Brauner wrote:
> > On Tue, Jan 20, 2026 at 08:46:01PM +1100, NeilBrown wrote:
> > > On Tue, 20 Jan 2026, Christian Brauner wrote:
> > > > > You don't need signing to ensure a filehandle doesn't persist across
> > > > > reboot.  For that you just need a generation number.  Storing a random
> > > > > number generated at boot time in the filehandle would be a good solution.
> > > > 
> > > > For pidfs I went with the 64-bit inode number. But I dislike the
> > > > generation number thing. If I would have to freedom to completely redo
> > > > it I would probably assign a uuid to the pidfs sb and then use that in
> > > > the file handles alongside the inode number. That would be enough for
> > > > sure as the uuid would change on each boot.
> > > 
> > > What you are calling a "uuid" in "the pidfs sb" is exactly what I am
> > > calling a "generation number" - for pidfs it would be a "generation
> > 
> > "generation number" just evokes the 32-bit identifier in struct inode
> > that's overall somewhat useless. And a UUID has much stronger
> > guarantees.
> > 
> > > number" for the whole filesystem, while for ext4 etc it is a generation
> > > number of the inode number.
> > > 
> > > So we are substantially in agreement.
> > 
> > Great!
> > 
> > > 
> > > Why do you not have freedom to add a uuid to the pidfs sb and to the
> > > filehandles now?
> > 
> > Userspace relies on the current format to get the inode number from the
> > file handle:
> > https://github.com/systemd/systemd/blob/main/src/basic/pidfd-util.c#L233-L281
> 
> The
>   assert(r != -EOVERFLOW);
> means you cannot extend it
> 
> > 
> > And they often also construct them in userspace. That needs to continue
> > to work. I also don't think it's that critical.
> > 
> 
> as does this.

Well, we could add a new version and we could make this work. I just
don't think we need to right now.

Btw, the manpage for name_to_handle_at() and friends mandates that file
handles are to be treated as opaque and should not be messed with
directly. That ship has sailed a long time ago. Userspace very much
relies on file handle layout. That's at least true for cgroupfs and
pidfs.

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Miklos Szeredi]

On Fri, 16 Jan 2026 at 15:36, Benjamin Coddington
<bcodding@hammerspace.com> wrote:

>  Documentation/netlink/specs/nfsd.yaml | 12 ++++
>  fs/nfsd/export.c                      |  5 +-

Would this make sense as a generic utility (i.e. in fs/exportfs/)?

The ultimate use case for me would be unprivileged open_by_handle_at(2).

Thanks,
Miklos

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Benjamin Coddington]

On 20 Jan 2026, at 5:13, NeilBrown wrote:

> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>> The following series enables the linux NFS server to add a Message
>> Authentication Code (MAC) to the filehandles it gives to clients.  This
>> provides additional protection to the exported filesystem against filehandle
>> guessing attacks.
>
> I've pondering this some more and I think we can get the same result
> without any API change, though it would require internal changes to each
> filesystem of interest.
>
> The problem can be rephrased as saying that 32 bits is too small for a
> generation number as it allows guessing in a modest time.  A 64bit
> generation number would provide sufficient protection (though admittedly
> not quite as much as a 32bit gen number with a 64 bit MAC).
>
> If a filesystem had 64 bits or more of random bits in the superblock
> which are stable and not visible over NFS, it could combine those with
> the 32bit gen number stored in the inode and present a 64 bit generation
> number in the filehandle which would, in practice, be unguessable.
>
> ext4 has s_journal_uuid which is stable and (I think) is not visible.
> xfs has sb_meta_uuid which seems to fill a vaguely similar role.
> I haven't looked further afield but it is credible that filesystems
> either have some suitable bits, or could add them.
>
> We could add a generic_encode_ino32_gen64_fh() which takes a uuid or
> similar, and some corresponding support for verifying the gen64 on
> decode.
>
> This would remove the need to change nfs-utils at all.

If so, it would be nice to find a way to unify setting/changing the per-fs
unique value because cloning and "golden image" approaches are often used in
deployments.  I think there are steps taken to "uniquify" cloned systems,
and setting/resetting this key would need to be one of those.

> This would mean the file handles change when you upgrade to a new kernel
> (though a CONFIG option would could give distros control of when to make
> the change), but nfsd it able to handle that if we are careful.
> Lookups from an "old" style filehandle will always produce an "old"
> style filehandle.  But when the client remounts they get a new style for
> everything.
>
> This is a lot more work in the kernel and requires buy-in from each
> relevant fs maintainer, but saves us any API change and so is largely
> transparent.
>
> Is it worth it?  I don't know.

Good pondering, I'm hoping this discussion continues a bit and we can figure
it out - I'll still iterate on this design in the meantime.

Ben

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Benjamin Coddington]

On 20 Jan 2026, at 5:55, Miklos Szeredi wrote:

> On Fri, 16 Jan 2026 at 15:36, Benjamin Coddington
> <bcodding@hammerspace.com> wrote:
>
>>  Documentation/netlink/specs/nfsd.yaml | 12 ++++
>>  fs/nfsd/export.c                      |  5 +-
>
> Would this make sense as a generic utility (i.e. in fs/exportfs/)?
>
> The ultimate use case for me would be unprivileged open_by_handle_at(2).

It might - I admit I don't know if signed filehandles would sufficiently
protect everything that CAP_DAC_READ_SEARCH has.  I've been focused on the
NFS (and pNFS/flexfiles) cases.

Would open_by_handle_at(2) need the ability to set/change the confidential
key used, and how can it be persisted?  Neil has some ideas about using a
per-fs unique value.

Ben

Re: [PATCH v1 0/4] kNFSD Signed Filehandles

[Miklos Szeredi]

On Tue, 20 Jan 2026 at 14:03, Benjamin Coddington
<bcodding@hammerspace.com> wrote:
>
> On 20 Jan 2026, at 5:55, Miklos Szeredi wrote:
>
> > On Fri, 16 Jan 2026 at 15:36, Benjamin Coddington
> > <bcodding@hammerspace.com> wrote:
> >
> >>  Documentation/netlink/specs/nfsd.yaml | 12 ++++
> >>  fs/nfsd/export.c                      |  5 +-
> >
> > Would this make sense as a generic utility (i.e. in fs/exportfs/)?
> >
> > The ultimate use case for me would be unprivileged open_by_handle_at(2).
>
> It might - I admit I don't know if signed filehandles would sufficiently
> protect everything that CAP_DAC_READ_SEARCH has.  I've been focused on the
> NFS (and pNFS/flexfiles) cases.

Problem is that keeping access to a file through an open file
descriptor can be better controlled than through a file handle, which
is just a cookie in memory.  Otherwise the two are equivalent, AFAICS.

I guess this is something that can be decided by the admin by weighing
the pros and cons.

> Would open_by_handle_at(2) need the ability to set/change the confidential
> key used, and how can it be persisted?  Neil has some ideas about using a
> per-fs unique value.

I think an automatically generated per-userns key would work in some
situations, for example.

An unprivileged userspace NFS server that can survive a reboot would
need a persistent key, but that would have to be managed by a
privileged entity.

Thanks,
Miklos