[PATCH v1 0/2] nfs-utils: signed filehandle support

[PATCH v1 0/2] nfs-utils: signed filehandle support

[Benjamin Coddington]

Here are two patches allowing userspace to set a secret key for kNFSD to
sign filehandles, and also set the option to sign filehandles for an
export.

The secret key passed to the server is the first 128 bits of a sha1 hash of
the contents of a file configured via the nfs.conf server section
"fh_key_file".  Exports that have the option "sign_fh" set will cause the
server to use this key to append an 8-byte siphash of the filehandle onto
each filehandle.

This version of the userspace patches correspond with the v1 of the kernel
changes which have been posted here:
https://lore.kernel.org/linux-nfs/C69B1F13-7248-4CAF-977C-5F0236B0923A@hammerspace.com/T/#t

This work is based on a branch that includes Jeff Layton's patch for
min-threads:
https://lore.kernel.org/linux-nfs/20260112-minthreads-v1-1-30c5f4113720@kernel.org/

Comments and critique welcomed.

Benjamin Coddington (2):
  nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key
  exportfs: Add support for export option sign_fh

 configure.ac                 |  4 +-
 nfs.conf                     |  1 +
 support/include/nfs/export.h |  2 +-
 support/include/nfslib.h     |  2 +
 support/nfs/Makefile.am      |  4 +-
 support/nfs/exports.c        |  4 ++
 systemd/nfs.conf.man         |  1 +
 utils/exportfs/exportfs.c    |  2 +
 utils/exportfs/exports.man   |  9 ++++
 utils/nfsd/nfsd.c            | 16 ++++++-
 utils/nfsd/nfssvc.c          | 26 +++++++++++
 utils/nfsd/nfssvc.h          |  1 +
 utils/nfsdctl/nfsd_netlink.h |  2 +
 utils/nfsdctl/nfsdctl.c      | 86 +++++++++++++++++++++++++++++++++++-
 14 files changed, 153 insertions(+), 7 deletions(-)


base-commit: 612e407c46b848932c32be00b835a7b5317e3d08
prerequisite-patch-id: cc4d768b1f6935b3c94ae87bd0389270717bc5b0
prerequisite-patch-id: c1ef8324c84a18d3ba29a971cb43b16798d71166
-- 
2.50.1

[PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[Benjamin Coddington]

If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command
will hash the contents of the file with libuuid's uuid_generate_sha1 and
send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.

If fh-key-file=<path> is set in nfs.conf, rpc.nfsd will also hash the
contents of the file with libuuid's uuid_generate_sha1 and write the
resulting uuid into nfsdfs's ./fh_key_file entry.

A compatible kernel can use this key to sign filehandles.

Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
 configure.ac                 |  4 +-
 nfs.conf                     |  1 +
 support/include/nfslib.h     |  2 +
 support/nfs/Makefile.am      |  4 +-
 systemd/nfs.conf.man         |  1 +
 utils/nfsd/nfsd.c            | 16 ++++++-
 utils/nfsd/nfssvc.c          | 26 +++++++++++
 utils/nfsd/nfssvc.h          |  1 +
 utils/nfsdctl/nfsd_netlink.h |  2 +
 utils/nfsdctl/nfsdctl.c      | 86 +++++++++++++++++++++++++++++++++++-
 10 files changed, 137 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 33866e869666..db027ddbd995 100644
--- a/configure.ac
+++ b/configure.ac
@@ -265,9 +265,9 @@ AC_ARG_ENABLE(nfsdctl,
 		AC_CHECK_DECLS([NFSD_A_SERVER_MIN_THREADS], , ,
 			       [#include <linux/nfsd_netlink.h>])
 
-		# ensure we have the pool-mode commands
+		# ensure we have the fh-key commands
 		AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <linux/nfsd_netlink.h>]],
-				                   [[int foo = NFSD_CMD_POOL_MODE_GET;]])],
+				                   [[int foo = NFSD_CMD_FH_KEY_SET;]])],
 				   [AC_DEFINE([USE_SYSTEM_NFSD_NETLINK_H], 1,
 					      ["Use system's linux/nfsd_netlink.h"])])
 		AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <linux/lockd_netlink.h>]],
diff --git a/nfs.conf b/nfs.conf
index 3cca68c3530d..39068c19d7df 100644
--- a/nfs.conf
+++ b/nfs.conf
@@ -76,6 +76,7 @@
 # vers4.2=y
 rdma=y
 rdma-port=20049
+# fh-key-file=/etc/nfs_fh.key
 
 [statd]
 # debug=0
diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index eff2a486307f..c8601a156cba 100644
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -22,6 +22,7 @@
 #include <paths.h>
 #include <rpcsvc/nfs_prot.h>
 #include <nfs/nfs.h>
+#include <uuid/uuid.h>
 #include "xlog.h"
 
 #ifndef _PATH_EXPORTS
@@ -132,6 +133,7 @@ struct rmtabent *	fgetrmtabent(FILE *fp, int log, long *pos);
 void			fputrmtabent(FILE *fp, struct rmtabent *xep, long *pos);
 void			fendrmtabent(FILE *fp);
 void			frewindrmtabent(FILE *fp);
+int				hash_fh_key_file(const char *fh_key_file, uuid_t hash);
 
 _Bool state_setup_basedir(const char *, const char *);
 int setup_state_path_names(const char *, const char *, const char *, const char *, struct state_paths *);
diff --git a/support/nfs/Makefile.am b/support/nfs/Makefile.am
index 2e1577cc12df..775bccc6c5ea 100644
--- a/support/nfs/Makefile.am
+++ b/support/nfs/Makefile.am
@@ -7,8 +7,8 @@ libnfs_la_SOURCES = exports.c rmtab.c xio.c rpcmisc.c rpcdispatch.c \
 		   xcommon.c wildmat.c mydaemon.c \
 		   rpc_socket.c getport.c \
 		   svc_socket.c cacheio.c closeall.c nfs_mntent.c \
-		   svc_create.c atomicio.c strlcat.c strlcpy.c
-libnfs_la_LIBADD = libnfsconf.la
+		   svc_create.c atomicio.c strlcat.c strlcpy.c fh_key_file.c
+libnfs_la_LIBADD = libnfsconf.la -luuid
 libnfs_la_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) -I$(top_srcdir)/support/reexport
 
 libnfsconf_la_SOURCES = conffile.c xlog.c
diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
index 484de2c086db..1fb5653042d3 100644
--- a/systemd/nfs.conf.man
+++ b/systemd/nfs.conf.man
@@ -176,6 +176,7 @@ Recognized values:
 .BR vers4.1 ,
 .BR vers4.2 ,
 .BR rdma ,
+.BR fh-key-file ,
 
 Version and protocol values are Boolean values as described above,
 and are also used by
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index a405649976c2..08a7eaed4906 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -69,7 +69,7 @@ int
 main(int argc, char **argv)
 {
 	int	count = NFSD_NPROC, c, i, j, error = 0, portnum, fd, found_one;
-	char *p, *progname, *port, *rdma_port = NULL;
+	char *p, *progname, *port, *fh_key_file, *rdma_port = NULL;
 	char **haddr = NULL;
 	char *scope = NULL;
 	int hcounter = 0;
@@ -134,6 +134,8 @@ main(int argc, char **argv)
 		}
 	}
 
+	fh_key_file = conf_get_str("nfsd", "fh-key-file");
+
 	/* We assume the kernel will default all minor versions to 'on',
 	 * and allow the config file to disable some.
 	 */
@@ -380,6 +382,18 @@ main(int argc, char **argv)
 		goto set_threads;
 	}
 
+	if (fh_key_file) {
+		error = nfssvc_setfh_key(fh_key_file);
+		if (error) {
+			/* Common case: key is already set */
+			if (error != -EEXIST) {
+				xlog(L_ERROR, "Unable to set fh_key_file, error %d", error);
+				goto out;
+			}
+			xlog(L_NOTICE, "fh_key_file already set");
+		}
+	}
+
 	/*
 	 * Must set versions before the fd's so that the right versions get
 	 * registered with rpcbind. Note that on older kernels w/o the right
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 9650cecee986..4f3088b74285 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -34,6 +34,7 @@
 #define NFSD_PORTS_FILE   NFSD_FS_DIR "/portlist"
 #define NFSD_VERS_FILE    NFSD_FS_DIR "/versions"
 #define NFSD_THREAD_FILE  NFSD_FS_DIR "/threads"
+#define NFSD_FH_KEY_FILE  NFSD_FS_DIR "/fh_key"
 
 /*
  * declaring a common static scratch buffer here keeps us from having to
@@ -414,6 +415,31 @@ out:
 	return;
 }
 
+int
+nfssvc_setfh_key(const char *fh_key_file)
+{
+	char uuid_str[37];
+	uuid_t hash;
+	int fd, ret;
+
+	ret = hash_fh_key_file(fh_key_file, hash);
+	if (ret)
+		return ret;
+
+	uuid_unparse(hash, uuid_str);
+
+	fd = open(NFSD_FH_KEY_FILE, O_WRONLY);
+	if (fd < 0)
+		return fd;
+
+	ret = write(fd, uuid_str, 37);
+	close(fd);
+	if (ret < 0)
+		return -errno;
+
+	return 0;
+}
+
 int
 nfssvc_threads(const int nrservs)
 {
diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
index 4d53af1a8bc3..463110cac804 100644
--- a/utils/nfsd/nfssvc.h
+++ b/utils/nfsd/nfssvc.h
@@ -30,3 +30,4 @@ void	nfssvc_setvers(unsigned int ctlbits, unsigned int minorvers4,
 		       unsigned int minorvers4set, int force4dot0);
 int	nfssvc_threads(int nrservs);
 void	nfssvc_get_minormask(unsigned int *mask);
+int nfssvc_setfh_key(const char *fh_key_file);
diff --git a/utils/nfsdctl/nfsd_netlink.h b/utils/nfsdctl/nfsd_netlink.h
index 887cbd12b695..f7e7f5576774 100644
--- a/utils/nfsdctl/nfsd_netlink.h
+++ b/utils/nfsdctl/nfsd_netlink.h
@@ -34,6 +34,7 @@ enum {
 	NFSD_A_SERVER_GRACETIME,
 	NFSD_A_SERVER_LEASETIME,
 	NFSD_A_SERVER_SCOPE,
+	NFSD_A_SERVER_FH_KEY,
 
 	__NFSD_A_SERVER_MAX,
 	NFSD_A_SERVER_MAX = (__NFSD_A_SERVER_MAX - 1)
@@ -88,6 +89,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)
diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index 48b3ba0b276c..efd4c1cf6d8a 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -29,6 +29,7 @@
 
 #include <readline/readline.h>
 #include <readline/history.h>
+#include <uuid/uuid.h>
 
 #ifdef USE_SYSTEM_NFSD_NETLINK_H
 #include <linux/nfsd_netlink.h>
@@ -42,6 +43,7 @@
 #include "lockd_netlink.h"
 #endif
 
+#include "nfslib.h"
 #include "nfsdctl.h"
 #include "conffile.h"
 #include "xlog.h"
@@ -1597,6 +1599,81 @@ static int configure_listeners(void)
 	return ret;
 }
 
+static int fh_key_file_doit(struct nl_sock *sock, const char *fh_key_file)
+{
+	struct genlmsghdr *ghdr;
+	struct nlmsghdr *nlh;
+	struct nl_data *data;
+	struct nl_msg *msg;
+	struct nl_cb *cb;
+	int ret;
+	uuid_t hash;
+
+	ret = hash_fh_key_file(fh_key_file, hash);
+	if (ret)
+		return ret;
+
+	msg = netlink_msg_alloc(sock, NFSD_FAMILY_NAME);
+	if (!msg) {
+		xlog(L_ERROR, "failed to allocate netlink msg");
+		return 1;
+	}
+
+	data = nl_data_alloc(hash, sizeof(hash));
+	if (!data) {
+		xlog(L_ERROR, "failed to allocate netlink data");
+		ret = 1;
+		goto out;
+	}
+
+	nla_put_data(msg, NFSD_A_SERVER_FH_KEY, data);
+	nlh = nlmsg_hdr(msg);
+	ghdr = nlmsg_data(nlh);
+	ghdr->cmd = NFSD_CMD_FH_KEY_SET;
+	cb = nl_cb_alloc(NL_CB_CUSTOM);
+	if (!cb) {
+		xlog(L_ERROR, "failed to allocate netlink callbacks");
+		ret = 1;
+		goto out_data;
+	}
+
+	ret = nl_send_auto(sock, msg);
+	if (ret < 0) {
+		xlog(L_ERROR, "send failed (%d)!", ret);
+		goto out_cb;
+	}
+
+	ret = 1;
+	nl_cb_err(cb, NL_CB_CUSTOM, error_handler, &ret);
+	nl_cb_set(cb, NL_CB_FINISH, NL_CB_CUSTOM, finish_handler, &ret);
+	nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM, ack_handler, &ret);
+	nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, recv_handler, NULL);
+
+	while (ret > 0)
+		nl_recvmsgs(sock, cb);
+	if (ret < 0) {
+		if (ret == -EOPNOTSUPP) {
+			xlog(L_ERROR, "Kernel does not support encrypted filehandles: %s",
+						strerror(-ret));
+			ret = 0;
+		} else if (ret == -EEXIST) {
+			xlog(L_ERROR, "fh_key_file already set: %s", strerror(-ret));
+			ret = 0;
+		} else {
+			xlog(L_ERROR, "Error setting encrypted filehandle key: %s",
+						strerror(-ret));
+			ret = 1;
+		}
+	}
+out_cb:
+	nl_cb_put(cb);
+out_data:
+	nl_data_free(data);
+out:
+	nlmsg_free(msg);
+	return ret;
+}
+
 static void autostart_usage(void)
 {
 	printf("Usage: %s autostart\n", taskname);
@@ -1611,7 +1688,7 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
 	int *threads, grace, lease, idx, ret, opt, pools, minthreads;
 	struct conf_list *thread_str;
 	struct conf_list_node *n;
-	char *scope, *pool_mode;
+	char *scope, *pool_mode, *fh_key_file;
 	bool failed_listeners = false;
 
 	optind = 1;
@@ -1683,6 +1760,13 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
 		threads[0] = DEFAULT_AUTOSTART_THREADS;
 	}
 
+	fh_key_file = conf_get_str("nfsd", "fh-key-file");
+	if (fh_key_file) {
+		ret = fh_key_file_doit(sock, fh_key_file);
+		if (ret)
+			return ret;
+	}
+
 	lease = conf_get_num("nfsd", "lease-time", 0);
 	scope = conf_get_str("nfsd", "scope");
 	minthreads = conf_get_num("nfsd", "min-threads", 0);
-- 
2.50.1

[PATCH v1 2/2] exportfs: Add support for export option sign_fh

[Benjamin Coddington]

If configured with "sign_fh", exports will be flagged to signal that
filehandles should be signed with a Message Authentication Code (MAC).

Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
 support/include/nfs/export.h | 2 +-
 support/nfs/exports.c        | 4 ++++
 utils/exportfs/exportfs.c    | 2 ++
 utils/exportfs/exports.man   | 9 +++++++++
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/support/include/nfs/export.h b/support/include/nfs/export.h
index be5867cffc3c..ef3f3e7ea684 100644
--- a/support/include/nfs/export.h
+++ b/support/include/nfs/export.h
@@ -19,7 +19,7 @@
 #define NFSEXP_GATHERED_WRITES	0x0020
 #define NFSEXP_NOREADDIRPLUS	0x0040
 #define NFSEXP_SECURITY_LABEL	0x0080
-/* 0x100 unused */
+#define NFSEXP_SIGN_FH		0x0100
 #define NFSEXP_NOHIDE		0x0200
 #define NFSEXP_NOSUBTREECHECK	0x0400
 #define NFSEXP_NOAUTHNLM	0x0800
diff --git a/support/nfs/exports.c b/support/nfs/exports.c
index 21ec6486ba3d..6b4ca87ee957 100644
--- a/support/nfs/exports.c
+++ b/support/nfs/exports.c
@@ -310,6 +310,8 @@ putexportent(struct exportent *ep)
 		fprintf(fp, "nordirplus,");
 	if (ep->e_flags & NFSEXP_SECURITY_LABEL)
 		fprintf(fp, "security_label,");
+	if (ep->e_flags & NFSEXP_SIGN_FH)
+		fprintf(fp, "sign_fh,");
 	fprintf(fp, "%spnfs,", (ep->e_flags & NFSEXP_PNFS)? "" : "no_");
 	if (ep->e_flags & NFSEXP_FSID) {
 		fprintf(fp, "fsid=%d,", ep->e_fsid);
@@ -676,6 +678,8 @@ parseopts(char *cp, struct exportent *ep, int *had_subtree_opt_ptr)
 			setflags(NFSEXP_NOREADDIRPLUS, active, ep);
 		else if (!strcmp(opt, "security_label"))
 			setflags(NFSEXP_SECURITY_LABEL, active, ep);
+		else if (!strcmp(opt, "sign_fh"))
+			setflags(NFSEXP_SIGN_FH, active, ep);
 		else if (!strcmp(opt, "nohide"))
 			setflags(NFSEXP_NOHIDE, active, ep);
 		else if (!strcmp(opt, "hide"))
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index 748c38e3e966..54ce62c5ce9a 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -718,6 +718,8 @@ dump(int verbose, int export_format)
 				c = dumpopt(c, "nordirplus");
 			if (ep->e_flags & NFSEXP_SECURITY_LABEL)
 				c = dumpopt(c, "security_label");
+			if (ep->e_flags & NFSEXP_SIGN_FH)
+				c = dumpopt(c, "sign_fh");
 			if (ep->e_flags & NFSEXP_NOACL)
 				c = dumpopt(c, "no_acl");
 			if (ep->e_flags & NFSEXP_PNFS)
diff --git a/utils/exportfs/exports.man b/utils/exportfs/exports.man
index 39dc30fb8290..8549232dea74 100644
--- a/utils/exportfs/exports.man
+++ b/utils/exportfs/exports.man
@@ -351,6 +351,15 @@ file.  If you put neither option,
 .B exportfs
 will warn you that the change has occurred.
 
+.TP
+.IR sign_fh
+This option enforces signing filehandles on the export.  If the
+server has been configured with a secret key for such purpose, filehandles
+will include a hash to verify the filehandle was created by the server in
+order to guard against filehandle reverse-engineering attacks.  Note that
+for NFSv2 and NFSv3, some exported filesystems may exceed the maximum
+filehandle size when the signing hash is added.
+
 .TP
 .IR insecure_locks
 .TP
-- 
2.50.1

Re: [PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[NeilBrown]

On Sat, 17 Jan 2026, Benjamin Coddington wrote:
> If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command

... is set in THE NFSD SECTION OF nfs.conf


> will hash the contents of the file with libuuid's uuid_generate_sha1 and
> send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.

This patch adds no code that uses uuid_generate_sha1(), and doesn't
provide any code for hash_fh_key_file()...

> 
> If fh-key-file=<path> is set in nfs.conf, rpc.nfsd will also hash the
> contents of the file with libuuid's uuid_generate_sha1 and write the
> resulting uuid into nfsdfs's ./fh_key_file entry.
> 
> A compatible kernel can use this key to sign filehandles.
> 
> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
> ---
>  configure.ac                 |  4 +-
>  nfs.conf                     |  1 +
>  support/include/nfslib.h     |  2 +
>  support/nfs/Makefile.am      |  4 +-
>  systemd/nfs.conf.man         |  1 +
>  utils/nfsd/nfsd.c            | 16 ++++++-
>  utils/nfsd/nfssvc.c          | 26 +++++++++++
>  utils/nfsd/nfssvc.h          |  1 +
>  utils/nfsdctl/nfsd_netlink.h |  2 +
>  utils/nfsdctl/nfsdctl.c      | 86 +++++++++++++++++++++++++++++++++++-
>  10 files changed, 137 insertions(+), 6 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 33866e869666..db027ddbd995 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -265,9 +265,9 @@ AC_ARG_ENABLE(nfsdctl,
>  		AC_CHECK_DECLS([NFSD_A_SERVER_MIN_THREADS], , ,
>  			       [#include <linux/nfsd_netlink.h>])
>  
> -		# ensure we have the pool-mode commands
> +		# ensure we have the fh-key commands
>  		AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <linux/nfsd_netlink.h>]],
> -				                   [[int foo = NFSD_CMD_POOL_MODE_GET;]])],
> +				                   [[int foo = NFSD_CMD_FH_KEY_SET;]])],
>  				   [AC_DEFINE([USE_SYSTEM_NFSD_NETLINK_H], 1,
>  					      ["Use system's linux/nfsd_netlink.h"])])
>  		AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <linux/lockd_netlink.h>]],
> diff --git a/nfs.conf b/nfs.conf
> index 3cca68c3530d..39068c19d7df 100644
> --- a/nfs.conf
> +++ b/nfs.conf
> @@ -76,6 +76,7 @@
>  # vers4.2=y
>  rdma=y
>  rdma-port=20049
> +# fh-key-file=/etc/nfs_fh.key
>  
>  [statd]
>  # debug=0
> diff --git a/support/include/nfslib.h b/support/include/nfslib.h
> index eff2a486307f..c8601a156cba 100644
> --- a/support/include/nfslib.h
> +++ b/support/include/nfslib.h
> @@ -22,6 +22,7 @@
>  #include <paths.h>
>  #include <rpcsvc/nfs_prot.h>
>  #include <nfs/nfs.h>
> +#include <uuid/uuid.h>
>  #include "xlog.h"
>  
>  #ifndef _PATH_EXPORTS
> @@ -132,6 +133,7 @@ struct rmtabent *	fgetrmtabent(FILE *fp, int log, long *pos);
>  void			fputrmtabent(FILE *fp, struct rmtabent *xep, long *pos);
>  void			fendrmtabent(FILE *fp);
>  void			frewindrmtabent(FILE *fp);
> +int				hash_fh_key_file(const char *fh_key_file, uuid_t hash);
>  
>  _Bool state_setup_basedir(const char *, const char *);
>  int setup_state_path_names(const char *, const char *, const char *, const char *, struct state_paths *);
> diff --git a/support/nfs/Makefile.am b/support/nfs/Makefile.am
> index 2e1577cc12df..775bccc6c5ea 100644
> --- a/support/nfs/Makefile.am
> +++ b/support/nfs/Makefile.am
> @@ -7,8 +7,8 @@ libnfs_la_SOURCES = exports.c rmtab.c xio.c rpcmisc.c rpcdispatch.c \
>  		   xcommon.c wildmat.c mydaemon.c \
>  		   rpc_socket.c getport.c \
>  		   svc_socket.c cacheio.c closeall.c nfs_mntent.c \
> -		   svc_create.c atomicio.c strlcat.c strlcpy.c
> -libnfs_la_LIBADD = libnfsconf.la
> +		   svc_create.c atomicio.c strlcat.c strlcpy.c fh_key_file.c
> +libnfs_la_LIBADD = libnfsconf.la -luuid
>  libnfs_la_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) -I$(top_srcdir)/support/reexport
>  
>  libnfsconf_la_SOURCES = conffile.c xlog.c
> diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
> index 484de2c086db..1fb5653042d3 100644
> --- a/systemd/nfs.conf.man
> +++ b/systemd/nfs.conf.man
> @@ -176,6 +176,7 @@ Recognized values:
>  .BR vers4.1 ,
>  .BR vers4.2 ,
>  .BR rdma ,
> +.BR fh-key-file ,
>  
>  Version and protocol values are Boolean values as described above,
>  and are also used by
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index a405649976c2..08a7eaed4906 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -69,7 +69,7 @@ int
>  main(int argc, char **argv)
>  {
>  	int	count = NFSD_NPROC, c, i, j, error = 0, portnum, fd, found_one;
> -	char *p, *progname, *port, *rdma_port = NULL;
> +	char *p, *progname, *port, *fh_key_file, *rdma_port = NULL;
>  	char **haddr = NULL;
>  	char *scope = NULL;
>  	int hcounter = 0;
> @@ -134,6 +134,8 @@ main(int argc, char **argv)
>  		}
>  	}
>  
> +	fh_key_file = conf_get_str("nfsd", "fh-key-file");
> +
>  	/* We assume the kernel will default all minor versions to 'on',
>  	 * and allow the config file to disable some.
>  	 */
> @@ -380,6 +382,18 @@ main(int argc, char **argv)
>  		goto set_threads;
>  	}
>  
> +	if (fh_key_file) {
> +		error = nfssvc_setfh_key(fh_key_file);
> +		if (error) {
> +			/* Common case: key is already set */
> +			if (error != -EEXIST) {
> +				xlog(L_ERROR, "Unable to set fh_key_file, error %d", error);
> +				goto out;
> +			}
> +			xlog(L_NOTICE, "fh_key_file already set");
> +		}
> +	}
> +
>  	/*
>  	 * Must set versions before the fd's so that the right versions get
>  	 * registered with rpcbind. Note that on older kernels w/o the right
> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> index 9650cecee986..4f3088b74285 100644
> --- a/utils/nfsd/nfssvc.c
> +++ b/utils/nfsd/nfssvc.c
> @@ -34,6 +34,7 @@
>  #define NFSD_PORTS_FILE   NFSD_FS_DIR "/portlist"
>  #define NFSD_VERS_FILE    NFSD_FS_DIR "/versions"
>  #define NFSD_THREAD_FILE  NFSD_FS_DIR "/threads"
> +#define NFSD_FH_KEY_FILE  NFSD_FS_DIR "/fh_key"
>  
>  /*
>   * declaring a common static scratch buffer here keeps us from having to
> @@ -414,6 +415,31 @@ out:
>  	return;
>  }
>  
> +int
> +nfssvc_setfh_key(const char *fh_key_file)
> +{
> +	char uuid_str[37];
> +	uuid_t hash;
> +	int fd, ret;
> +
> +	ret = hash_fh_key_file(fh_key_file, hash);
> +	if (ret)
> +		return ret;
> +
> +	uuid_unparse(hash, uuid_str);
> +
> +	fd = open(NFSD_FH_KEY_FILE, O_WRONLY);
> +	if (fd < 0)
> +		return fd;

return -errno ??

> +
> +	ret = write(fd, uuid_str, 37);
> +	close(fd);
> +	if (ret < 0)
> +		return -errno;
> +
> +	return 0;
> +}
> +
>  int
>  nfssvc_threads(const int nrservs)
>  {
> diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
> index 4d53af1a8bc3..463110cac804 100644
> --- a/utils/nfsd/nfssvc.h
> +++ b/utils/nfsd/nfssvc.h
> @@ -30,3 +30,4 @@ void	nfssvc_setvers(unsigned int ctlbits, unsigned int minorvers4,
>  		       unsigned int minorvers4set, int force4dot0);
>  int	nfssvc_threads(int nrservs);
>  void	nfssvc_get_minormask(unsigned int *mask);
> +int nfssvc_setfh_key(const char *fh_key_file);
> diff --git a/utils/nfsdctl/nfsd_netlink.h b/utils/nfsdctl/nfsd_netlink.h
> index 887cbd12b695..f7e7f5576774 100644
> --- a/utils/nfsdctl/nfsd_netlink.h
> +++ b/utils/nfsdctl/nfsd_netlink.h
> @@ -34,6 +34,7 @@ enum {
>  	NFSD_A_SERVER_GRACETIME,
>  	NFSD_A_SERVER_LEASETIME,
>  	NFSD_A_SERVER_SCOPE,
> +	NFSD_A_SERVER_FH_KEY,
>  
>  	__NFSD_A_SERVER_MAX,
>  	NFSD_A_SERVER_MAX = (__NFSD_A_SERVER_MAX - 1)
> @@ -88,6 +89,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)
> diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
> index 48b3ba0b276c..efd4c1cf6d8a 100644
> --- a/utils/nfsdctl/nfsdctl.c
> +++ b/utils/nfsdctl/nfsdctl.c
> @@ -29,6 +29,7 @@
>  
>  #include <readline/readline.h>
>  #include <readline/history.h>
> +#include <uuid/uuid.h>
>  
>  #ifdef USE_SYSTEM_NFSD_NETLINK_H
>  #include <linux/nfsd_netlink.h>
> @@ -42,6 +43,7 @@
>  #include "lockd_netlink.h"
>  #endif
>  
> +#include "nfslib.h"
>  #include "nfsdctl.h"
>  #include "conffile.h"
>  #include "xlog.h"
> @@ -1597,6 +1599,81 @@ static int configure_listeners(void)
>  	return ret;
>  }
>  
> +static int fh_key_file_doit(struct nl_sock *sock, const char *fh_key_file)
> +{
> +	struct genlmsghdr *ghdr;
> +	struct nlmsghdr *nlh;
> +	struct nl_data *data;
> +	struct nl_msg *msg;
> +	struct nl_cb *cb;
> +	int ret;
> +	uuid_t hash;
> +
> +	ret = hash_fh_key_file(fh_key_file, hash);
> +	if (ret)
> +		return ret;
> +
> +	msg = netlink_msg_alloc(sock, NFSD_FAMILY_NAME);
> +	if (!msg) {
> +		xlog(L_ERROR, "failed to allocate netlink msg");
> +		return 1;
> +	}
> +
> +	data = nl_data_alloc(hash, sizeof(hash));
> +	if (!data) {
> +		xlog(L_ERROR, "failed to allocate netlink data");
> +		ret = 1;
> +		goto out;
> +	}
> +
> +	nla_put_data(msg, NFSD_A_SERVER_FH_KEY, data);
> +	nlh = nlmsg_hdr(msg);
> +	ghdr = nlmsg_data(nlh);
> +	ghdr->cmd = NFSD_CMD_FH_KEY_SET;
> +	cb = nl_cb_alloc(NL_CB_CUSTOM);
> +	if (!cb) {
> +		xlog(L_ERROR, "failed to allocate netlink callbacks");
> +		ret = 1;
> +		goto out_data;
> +	}
> +
> +	ret = nl_send_auto(sock, msg);
> +	if (ret < 0) {
> +		xlog(L_ERROR, "send failed (%d)!", ret);
> +		goto out_cb;
> +	}
> +
> +	ret = 1;
> +	nl_cb_err(cb, NL_CB_CUSTOM, error_handler, &ret);
> +	nl_cb_set(cb, NL_CB_FINISH, NL_CB_CUSTOM, finish_handler, &ret);
> +	nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM, ack_handler, &ret);
> +	nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, recv_handler, NULL);
> +
> +	while (ret > 0)
> +		nl_recvmsgs(sock, cb);
> +	if (ret < 0) {
> +		if (ret == -EOPNOTSUPP) {
> +			xlog(L_ERROR, "Kernel does not support encrypted filehandles: %s",
> +						strerror(-ret));
> +			ret = 0;
> +		} else if (ret == -EEXIST) {
> +			xlog(L_ERROR, "fh_key_file already set: %s", strerror(-ret));
> +			ret = 0;
> +		} else {
> +			xlog(L_ERROR, "Error setting encrypted filehandle key: %s",
> +						strerror(-ret));
> +			ret = 1;
> +		}
> +	}
> +out_cb:
> +	nl_cb_put(cb);
> +out_data:
> +	nl_data_free(data);
> +out:
> +	nlmsg_free(msg);
> +	return ret;
> +}
> +
>  static void autostart_usage(void)
>  {
>  	printf("Usage: %s autostart\n", taskname);
> @@ -1611,7 +1688,7 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
>  	int *threads, grace, lease, idx, ret, opt, pools, minthreads;
>  	struct conf_list *thread_str;
>  	struct conf_list_node *n;
> -	char *scope, *pool_mode;
> +	char *scope, *pool_mode, *fh_key_file;
>  	bool failed_listeners = false;
>  
>  	optind = 1;
> @@ -1683,6 +1760,13 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
>  		threads[0] = DEFAULT_AUTOSTART_THREADS;
>  	}
>  
> +	fh_key_file = conf_get_str("nfsd", "fh-key-file");
> +	if (fh_key_file) {
> +		ret = fh_key_file_doit(sock, fh_key_file);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	lease = conf_get_num("nfsd", "lease-time", 0);
>  	scope = conf_get_str("nfsd", "scope");
>  	minthreads = conf_get_num("nfsd", "min-threads", 0);
> -- 
> 2.50.1
> 
> 

I haven't reviewed the netlink parts because I don't know the API well,
but the rest seems ok, except for the missing code.

Thanks,
NeilBrown

Re: [PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[Benjamin Coddington]

On 17 Jan 2026, at 16:53, NeilBrown wrote:

> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>> If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command
>
> ... is set in THE NFSD SECTION OF nfs.conf
>
>
>> will hash the contents of the file with libuuid's uuid_generate_sha1 and
>> send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.
>
> This patch adds no code that uses uuid_generate_sha1(), and doesn't
> provide any code for hash_fh_key_file()...

I forgot to add the hash function after moving it into libnfs to make it
available to both rpc.nfsd and nfsdctl -- here it is, will fix on v2:

diff --git a/support/nfs/fh_key_file.c b/support/nfs/fh_key_file.c
new file mode 100644
index 000000000000..350d36bf8649
--- /dev/null
+++ b/support/nfs/fh_key_file.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2025 Benjamin Coddington <bcodding@hammerspace.com>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <errno.h>
+#include <uuid/uuid.h>
+
+#include "nfslib.h"
+
+#define HASH_BLOCKSIZE  256
+int hash_fh_key_file(const char *fh_key_file, uuid_t uuid)
+{
+	const char seed_s[] = "8fc57f1b-1a6f-482f-af92-d2e007c1ae58";
+	FILE *sfile = NULL;
+	char *buf = malloc(HASH_BLOCKSIZE);
+	size_t pos;
+	int ret = 0;
+
+	if (!buf)
+		goto out;
+
+	sfile = fopen(fh_key_file, "r");
+	if (!sfile) {
+		ret = errno;
+		xlog(L_ERROR, "Unable to read fh-key-file %s: %s", fh_key_file, strerror(errno));
+		goto out;
+	}
+
+	uuid_parse(seed_s, uuid);
+	while (1) {
+		size_t sread;
+		pos = 0;
+
+		while (1) {
+			if (feof(sfile))
+				goto finish_block;
+
+			sread = fread(buf + pos, 1, HASH_BLOCKSIZE - pos, sfile);
+			pos += sread;
+
+			if (pos == HASH_BLOCKSIZE)
+				break;
+
+			if (sread == 0) {
+				if (ferror(sfile))
+					goto out;
+				goto finish_block;
+			}
+		}
+		uuid_generate_sha1(uuid, uuid, buf, HASH_BLOCKSIZE);
+	}
+finish_block:
+	if (pos)
+		uuid_generate_sha1(uuid, uuid, buf, pos);
+out:
+	if (sfile)
+		fclose(sfile);
+	free(buf);
+	return ret;
+}

Re: [PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[Steve Dickson]

On 1/18/26 11:59 AM, Benjamin Coddington wrote:
> On 17 Jan 2026, at 16:53, NeilBrown wrote:
> 
>> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>>> If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command
>>
>> ... is set in THE NFSD SECTION OF nfs.conf
>>
>>
>>> will hash the contents of the file with libuuid's uuid_generate_sha1 and
>>> send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.
>>
>> This patch adds no code that uses uuid_generate_sha1(), and doesn't
>> provide any code for hash_fh_key_file()...
> 
> I forgot to add the hash function after moving it into libnfs to make it
> available to both rpc.nfsd and nfsdctl -- here it is, will fix on v2:
I'm a bit confused... This patch conflicts with the
"nfsdctl: Add support for passing encrypted filehandle key" patch.

So which patches should I take and which patches are tested ;-)

steved.

> 
> diff --git a/support/nfs/fh_key_file.c b/support/nfs/fh_key_file.c
> new file mode 100644
> index 000000000000..350d36bf8649
> --- /dev/null
> +++ b/support/nfs/fh_key_file.c
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (c) 2025 Benjamin Coddington <bcodding@hammerspace.com>
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <uuid/uuid.h>
> +
> +#include "nfslib.h"
> +
> +#define HASH_BLOCKSIZE  256
> +int hash_fh_key_file(const char *fh_key_file, uuid_t uuid)
> +{
> +	const char seed_s[] = "8fc57f1b-1a6f-482f-af92-d2e007c1ae58";
> +	FILE *sfile = NULL;
> +	char *buf = malloc(HASH_BLOCKSIZE);
> +	size_t pos;
> +	int ret = 0;
> +
> +	if (!buf)
> +		goto out;
> +
> +	sfile = fopen(fh_key_file, "r");
> +	if (!sfile) {
> +		ret = errno;
> +		xlog(L_ERROR, "Unable to read fh-key-file %s: %s", fh_key_file, strerror(errno));
> +		goto out;
> +	}
> +
> +	uuid_parse(seed_s, uuid);
> +	while (1) {
> +		size_t sread;
> +		pos = 0;
> +
> +		while (1) {
> +			if (feof(sfile))
> +				goto finish_block;
> +
> +			sread = fread(buf + pos, 1, HASH_BLOCKSIZE - pos, sfile);
> +			pos += sread;
> +
> +			if (pos == HASH_BLOCKSIZE)
> +				break;
> +
> +			if (sread == 0) {
> +				if (ferror(sfile))
> +					goto out;
> +				goto finish_block;
> +			}
> +		}
> +		uuid_generate_sha1(uuid, uuid, buf, HASH_BLOCKSIZE);
> +	}
> +finish_block:
> +	if (pos)
> +		uuid_generate_sha1(uuid, uuid, buf, pos);
> +out:
> +	if (sfile)
> +		fclose(sfile);
> +	free(buf);
> +	return ret;
> +}
> 

Re: [PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[Benjamin Coddington]

On 18 Jan 2026, at 15:09, Steve Dickson wrote:

> On 1/18/26 11:59 AM, Benjamin Coddington wrote:
>> On 17 Jan 2026, at 16:53, NeilBrown wrote:
>>
>>> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>>>> If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command
>>>
>>> ... is set in THE NFSD SECTION OF nfs.conf
>>>
>>>
>>>> will hash the contents of the file with libuuid's uuid_generate_sha1 and
>>>> send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.
>>>
>>> This patch adds no code that uses uuid_generate_sha1(), and doesn't
>>> provide any code for hash_fh_key_file()...
>>
>> I forgot to add the hash function after moving it into libnfs to make it
>> available to both rpc.nfsd and nfsdctl -- here it is, will fix on v2:
> I'm a bit confused... This patch conflicts with the
> "nfsdctl: Add support for passing encrypted filehandle key" patch.
>
> So which patches should I take and which patches are tested ;-)

Both are tested - but this series is more complete (but still missing a
hunk), because it has support for setting the filehandle signing key in both
nfsdctl and rpc.nfsd..  I am going to send a v2, probably tomorrow.

I think you can delay accepting this work until the kernel patches at least
make it into Chuck's nfsd-testing tree.  I can keep you appriased of the
that status.

Ben

Re: [PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[NeilBrown]

On Mon, 19 Jan 2026, Benjamin Coddington wrote:
> On 17 Jan 2026, at 16:53, NeilBrown wrote:
> 
> > On Sat, 17 Jan 2026, Benjamin Coddington wrote:
> >> If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command
> >
> > ... is set in THE NFSD SECTION OF nfs.conf
> >
> >
> >> will hash the contents of the file with libuuid's uuid_generate_sha1 and
> >> send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.
> >
> > This patch adds no code that uses uuid_generate_sha1(), and doesn't
> > provide any code for hash_fh_key_file()...
> 
> I forgot to add the hash function after moving it into libnfs to make it
> available to both rpc.nfsd and nfsdctl -- here it is, will fix on v2:
> 
> diff --git a/support/nfs/fh_key_file.c b/support/nfs/fh_key_file.c
> new file mode 100644
> index 000000000000..350d36bf8649
> --- /dev/null
> +++ b/support/nfs/fh_key_file.c
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (c) 2025 Benjamin Coddington <bcodding@hammerspace.com>
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */

I wonder if it is time to stop putting this boilerplate in nfs-utils and
start using SPDX like the kernel does.

> +
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <uuid/uuid.h>
> +
> +#include "nfslib.h"
> +
> +#define HASH_BLOCKSIZE  256
> +int hash_fh_key_file(const char *fh_key_file, uuid_t uuid)
> +{
> +	const char seed_s[] = "8fc57f1b-1a6f-482f-af92-d2e007c1ae58";
> +	FILE *sfile = NULL;
> +	char *buf = malloc(HASH_BLOCKSIZE);

Can this be 
   char buf[HASH_BLOCKSIZE];
??

> +	size_t pos;
> +	int ret = 0;
> +
> +	if (!buf)
> +		goto out;
> +
> +	sfile = fopen(fh_key_file, "r");
> +	if (!sfile) {
> +		ret = errno;
> +		xlog(L_ERROR, "Unable to read fh-key-file %s: %s", fh_key_file, strerror(errno));
> +		goto out;
> +	}
> +
> +	uuid_parse(seed_s, uuid);
> +	while (1) {
> +		size_t sread;
> +		pos = 0;
> +
> +		while (1) {
> +			if (feof(sfile))
> +				goto finish_block;
> +
> +			sread = fread(buf + pos, 1, HASH_BLOCKSIZE - pos, sfile);
> +			pos += sread;
> +
> +			if (pos == HASH_BLOCKSIZE)
> +				break;
> +
> +			if (sread == 0) {
> +				if (ferror(sfile))
> +					goto out;
> +				goto finish_block;
> +			}
> +		}

I think this inner look is not needed or wanted.
fread() will loop as needed until EOF or an error, and we don't want to
continue on an error.

> +		uuid_generate_sha1(uuid, uuid, buf, HASH_BLOCKSIZE);
> +	}
> +finish_block:
> +	if (pos)
> +		uuid_generate_sha1(uuid, uuid, buf, pos);
> +out:
> +	if (sfile)
> +		fclose(sfile);
> +	free(buf);
> +	return ret;
> +}
> 

Thanks,
NeilBrown

Re: [PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[Steve Dickson]

Hello,

On 1/18/26 6:05 PM, NeilBrown wrote:
> On Mon, 19 Jan 2026, Benjamin Coddington wrote:
>> On 17 Jan 2026, at 16:53, NeilBrown wrote:
>>
>>> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>>>> If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command
>>>
>>> ... is set in THE NFSD SECTION OF nfs.conf
>>>
>>>
>>>> will hash the contents of the file with libuuid's uuid_generate_sha1 and
>>>> send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.
>>>
>>> This patch adds no code that uses uuid_generate_sha1(), and doesn't
>>> provide any code for hash_fh_key_file()...
>>
>> I forgot to add the hash function after moving it into libnfs to make it
>> available to both rpc.nfsd and nfsdctl -- here it is, will fix on v2:
>>
>> diff --git a/support/nfs/fh_key_file.c b/support/nfs/fh_key_file.c
>> new file mode 100644
>> index 000000000000..350d36bf8649
>> --- /dev/null
>> +++ b/support/nfs/fh_key_file.c
>> @@ -0,0 +1,83 @@
>> +/*
>> + * Copyright (c) 2025 Benjamin Coddington <bcodding@hammerspace.com>
>> + * All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + *    notice, this list of conditions and the following disclaimer in the
>> + *    documentation and/or other materials provided with the distribution.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
>> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
>> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
>> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
>> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
>> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> + */
> 
> I wonder if it is time to stop putting this boilerplate in nfs-utils and
> start using SPDX like the kernel does.
Would there be legal ramifications to do this?

I remember a time when our legal department was
not happy with the copyright verbiage we were using.
>
>> +
>> +#include <sys/types.h>
>> +#include <unistd.h>
>> +#include <errno.h>
>> +#include <uuid/uuid.h>
>> +
>> +#include "nfslib.h"
>> +
>> +#define HASH_BLOCKSIZE  256
>> +int hash_fh_key_file(const char *fh_key_file, uuid_t uuid)
>> +{
>> +	const char seed_s[] = "8fc57f1b-1a6f-482f-af92-d2e007c1ae58";
>> +	FILE *sfile = NULL;
>> +	char *buf = malloc(HASH_BLOCKSIZE);
> 
> Can this be
>     char buf[HASH_BLOCKSIZE];
> ??
Isn't better to keep things off the stack? But is only
256... so it is probably not that big of a deal.

> 
>> +	size_t pos;
>> +	int ret = 0;
>> +
>> +	if (!buf)
>> +		goto out;
>> +
>> +	sfile = fopen(fh_key_file, "r");
>> +	if (!sfile) {
>> +		ret = errno;
>> +		xlog(L_ERROR, "Unable to read fh-key-file %s: %s", fh_key_file, strerror(errno));
>> +		goto out;
>> +	}
>> +
>> +	uuid_parse(seed_s, uuid);
>> +	while (1) {
>> +		size_t sread;
>> +		pos = 0;
>> +
>> +		while (1) {
>> +			if (feof(sfile))
>> +				goto finish_block;
>> +
>> +			sread = fread(buf + pos, 1, HASH_BLOCKSIZE - pos, sfile);
>> +			pos += sread;
>> +
>> +			if (pos == HASH_BLOCKSIZE)
>> +				break;
>> +
>> +			if (sread == 0) {
>> +				if (ferror(sfile))
>> +					goto out;
>> +				goto finish_block;
>> +			}
>> +		}
> 
> I think this inner look is not needed or wanted.
> fread() will loop as needed until EOF or an error, and we don't want to
> continue on an error.
Good call! Having nested infinite is a bit worrisome...

steved.

> 
>> +		uuid_generate_sha1(uuid, uuid, buf, HASH_BLOCKSIZE);
>> +	}
>> +finish_block:
>> +	if (pos)
>> +		uuid_generate_sha1(uuid, uuid, buf, pos);
>> +out:
>> +	if (sfile)
>> +		fclose(sfile);
>> +	free(buf);
>> +	return ret;
>> +}
>>
> 
> Thanks,
> NeilBrown
> 

Re: [PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[NeilBrown]

On Thu, 22 Jan 2026, Steve Dickson wrote:
> Hello,
> 
> On 1/18/26 6:05 PM, NeilBrown wrote:
> > On Mon, 19 Jan 2026, Benjamin Coddington wrote:
> >> On 17 Jan 2026, at 16:53, NeilBrown wrote:
> >>
> >>> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
> >>>> If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command
> >>>
> >>> ... is set in THE NFSD SECTION OF nfs.conf
> >>>
> >>>
> >>>> will hash the contents of the file with libuuid's uuid_generate_sha1 and
> >>>> send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.
> >>>
> >>> This patch adds no code that uses uuid_generate_sha1(), and doesn't
> >>> provide any code for hash_fh_key_file()...
> >>
> >> I forgot to add the hash function after moving it into libnfs to make it
> >> available to both rpc.nfsd and nfsdctl -- here it is, will fix on v2:
> >>
> >> diff --git a/support/nfs/fh_key_file.c b/support/nfs/fh_key_file.c
> >> new file mode 100644
> >> index 000000000000..350d36bf8649
> >> --- /dev/null
> >> +++ b/support/nfs/fh_key_file.c
> >> @@ -0,0 +1,83 @@
> >> +/*
> >> + * Copyright (c) 2025 Benjamin Coddington <bcodding@hammerspace.com>
> >> + * All rights reserved.
> >> + *
> >> + * Redistribution and use in source and binary forms, with or without
> >> + * modification, are permitted provided that the following conditions
> >> + * are met:
> >> + * 1. Redistributions of source code must retain the above copyright
> >> + *    notice, this list of conditions and the following disclaimer.
> >> + * 2. Redistributions in binary form must reproduce the above copyright
> >> + *    notice, this list of conditions and the following disclaimer in the
> >> + *    documentation and/or other materials provided with the distribution.
> >> + *
> >> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> >> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> >> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> >> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> >> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> >> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> >> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> >> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> >> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> >> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >> + */
> > 
> > I wonder if it is time to stop putting this boilerplate in nfs-utils and
> > start using SPDX like the kernel does.
> Would there be legal ramifications to do this?

Maybe.  Certainly it should be done with care.

As a first step we could consider requiring the SPDX line rather than
the verbose license text for new contributions.

> 
> I remember a time when our legal department was
> not happy with the copyright verbiage we were using.

I suspect it is the legal deparments job to not be happy :-)

> >
> >> +
> >> +#include <sys/types.h>
> >> +#include <unistd.h>
> >> +#include <errno.h>
> >> +#include <uuid/uuid.h>
> >> +
> >> +#include "nfslib.h"
> >> +
> >> +#define HASH_BLOCKSIZE  256
> >> +int hash_fh_key_file(const char *fh_key_file, uuid_t uuid)
> >> +{
> >> +	const char seed_s[] = "8fc57f1b-1a6f-482f-af92-d2e007c1ae58";
> >> +	FILE *sfile = NULL;
> >> +	char *buf = malloc(HASH_BLOCKSIZE);
> > 
> > Can this be
> >     char buf[HASH_BLOCKSIZE];
> > ??
> Isn't better to keep things off the stack? But is only
> 256... so it is probably not that big of a deal.

In the kernel, 256 bytes on the stack should raise questions.  In user
space I wouldn't blink below 1MB if the code made sense otherwise.

NeilBrown

Re: [PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[Benjamin Coddington]

On 18 Jan 2026, at 18:05, NeilBrown wrote:

> On Mon, 19 Jan 2026, Benjamin Coddington wrote:
>> On 17 Jan 2026, at 16:53, NeilBrown wrote:
>>
>>> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>>>> If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command
>>>
>>> ... is set in THE NFSD SECTION OF nfs.conf
>>>
>>>
>>>> will hash the contents of the file with libuuid's uuid_generate_sha1 and
>>>> send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.
>>>
>>> This patch adds no code that uses uuid_generate_sha1(), and doesn't
>>> provide any code for hash_fh_key_file()...
>>
>> I forgot to add the hash function after moving it into libnfs to make it
>> available to both rpc.nfsd and nfsdctl -- here it is, will fix on v2:
>>
>> diff --git a/support/nfs/fh_key_file.c b/support/nfs/fh_key_file.c
>> new file mode 100644
>> index 000000000000..350d36bf8649
>> --- /dev/null
>> +++ b/support/nfs/fh_key_file.c
>> @@ -0,0 +1,83 @@
>> +/*
>> + * Copyright (c) 2025 Benjamin Coddington <bcodding@hammerspace.com>
>> + * All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + *    notice, this list of conditions and the following disclaimer in the
>> + *    documentation and/or other materials provided with the distribution.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
>> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
>> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
>> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
>> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
>> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> + */
>
> I wonder if it is time to stop putting this boilerplate in nfs-utils and
> start using SPDX like the kernel does.
>
>> +
>> +#include <sys/types.h>
>> +#include <unistd.h>
>> +#include <errno.h>
>> +#include <uuid/uuid.h>
>> +
>> +#include "nfslib.h"
>> +
>> +#define HASH_BLOCKSIZE  256
>> +int hash_fh_key_file(const char *fh_key_file, uuid_t uuid)
>> +{
>> +	const char seed_s[] = "8fc57f1b-1a6f-482f-af92-d2e007c1ae58";
>> +	FILE *sfile = NULL;
>> +	char *buf = malloc(HASH_BLOCKSIZE);
>
> Can this be
>    char buf[HASH_BLOCKSIZE];
> ??
>
>> +	size_t pos;
>> +	int ret = 0;
>> +
>> +	if (!buf)
>> +		goto out;
>> +
>> +	sfile = fopen(fh_key_file, "r");
>> +	if (!sfile) {
>> +		ret = errno;
>> +		xlog(L_ERROR, "Unable to read fh-key-file %s: %s", fh_key_file, strerror(errno));
>> +		goto out;
>> +	}
>> +
>> +	uuid_parse(seed_s, uuid);
>> +	while (1) {
>> +		size_t sread;
>> +		pos = 0;
>> +
>> +		while (1) {
>> +			if (feof(sfile))
>> +				goto finish_block;
>> +
>> +			sread = fread(buf + pos, 1, HASH_BLOCKSIZE - pos, sfile);
>> +			pos += sread;
>> +
>> +			if (pos == HASH_BLOCKSIZE)
>> +				break;
>> +
>> +			if (sread == 0) {
>> +				if (ferror(sfile))
>> +					goto out;
>> +				goto finish_block;
>> +			}
>> +		}
>
> I think this inner look is not needed or wanted.
> fread() will loop as needed until EOF or an error, and we don't want to
> continue on an error.

The fh_key_file can be any length - and this function reads it in
HASH_BLOCKSIZE chunks.  Each chuck gets hashed against the previous result.

That said, there is a bug where ferror(sfile) never gets assigned to "ret",
fixing that..

Ben

Re: [PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[NeilBrown]

On Thu, 22 Jan 2026, Benjamin Coddington wrote:
> On 18 Jan 2026, at 18:05, NeilBrown wrote:
> 
> > On Mon, 19 Jan 2026, Benjamin Coddington wrote:
> >> On 17 Jan 2026, at 16:53, NeilBrown wrote:
> >>
> >>> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
> >>>> If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command
> >>>
> >>> ... is set in THE NFSD SECTION OF nfs.conf
> >>>
> >>>
> >>>> will hash the contents of the file with libuuid's uuid_generate_sha1 and
> >>>> send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.
> >>>
> >>> This patch adds no code that uses uuid_generate_sha1(), and doesn't
> >>> provide any code for hash_fh_key_file()...
> >>
> >> I forgot to add the hash function after moving it into libnfs to make it
> >> available to both rpc.nfsd and nfsdctl -- here it is, will fix on v2:
> >>
> >> diff --git a/support/nfs/fh_key_file.c b/support/nfs/fh_key_file.c
> >> new file mode 100644
> >> index 000000000000..350d36bf8649
> >> --- /dev/null
> >> +++ b/support/nfs/fh_key_file.c
> >> @@ -0,0 +1,83 @@
> >> +/*
> >> + * Copyright (c) 2025 Benjamin Coddington <bcodding@hammerspace.com>
> >> + * All rights reserved.
> >> + *
> >> + * Redistribution and use in source and binary forms, with or without
> >> + * modification, are permitted provided that the following conditions
> >> + * are met:
> >> + * 1. Redistributions of source code must retain the above copyright
> >> + *    notice, this list of conditions and the following disclaimer.
> >> + * 2. Redistributions in binary form must reproduce the above copyright
> >> + *    notice, this list of conditions and the following disclaimer in the
> >> + *    documentation and/or other materials provided with the distribution.
> >> + *
> >> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> >> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> >> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> >> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> >> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> >> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> >> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> >> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> >> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> >> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >> + */
> >
> > I wonder if it is time to stop putting this boilerplate in nfs-utils and
> > start using SPDX like the kernel does.
> >
> >> +
> >> +#include <sys/types.h>
> >> +#include <unistd.h>
> >> +#include <errno.h>
> >> +#include <uuid/uuid.h>
> >> +
> >> +#include "nfslib.h"
> >> +
> >> +#define HASH_BLOCKSIZE  256
> >> +int hash_fh_key_file(const char *fh_key_file, uuid_t uuid)
> >> +{
> >> +	const char seed_s[] = "8fc57f1b-1a6f-482f-af92-d2e007c1ae58";
> >> +	FILE *sfile = NULL;
> >> +	char *buf = malloc(HASH_BLOCKSIZE);
> >
> > Can this be
> >    char buf[HASH_BLOCKSIZE];
> > ??
> >
> >> +	size_t pos;
> >> +	int ret = 0;
> >> +
> >> +	if (!buf)
> >> +		goto out;
> >> +
> >> +	sfile = fopen(fh_key_file, "r");
> >> +	if (!sfile) {
> >> +		ret = errno;
> >> +		xlog(L_ERROR, "Unable to read fh-key-file %s: %s", fh_key_file, strerror(errno));
> >> +		goto out;
> >> +	}
> >> +
> >> +	uuid_parse(seed_s, uuid);
> >> +	while (1) {
> >> +		size_t sread;
> >> +		pos = 0;
> >> +
> >> +		while (1) {
> >> +			if (feof(sfile))
> >> +				goto finish_block;
> >> +
> >> +			sread = fread(buf + pos, 1, HASH_BLOCKSIZE - pos, sfile);
> >> +			pos += sread;
> >> +
> >> +			if (pos == HASH_BLOCKSIZE)
> >> +				break;
> >> +
> >> +			if (sread == 0) {
> >> +				if (ferror(sfile))
> >> +					goto out;
> >> +				goto finish_block;
> >> +			}
> >> +		}
> >
> > I think this inner look is not needed or wanted.
> > fread() will loop as needed until EOF or an error, and we don't want to
> > continue on an error.
> 
> The fh_key_file can be any length - and this function reads it in
> HASH_BLOCKSIZE chunks.  Each chuck gets hashed against the previous result.

Yes, so the outer loop is clearly needed.
The inner loop will never execute more than once as fread() only returns
short reads on EOF or error.

> 
> That said, there is a bug where ferror(sfile) never gets assigned to "ret",
> fixing that..

Thanks.

NeilBrown

> 
> Ben
> 

Re: [PATCH v1 1/2] nfsdctl/rpc.nfsd: Add support for passing encrypted filehandle key

[Benjamin Coddington]

On 21 Jan 2026, at 17:38, NeilBrown wrote:

> On Thu, 22 Jan 2026, Benjamin Coddington wrote:
>> On 18 Jan 2026, at 18:05, NeilBrown wrote:
>>
>>> On Mon, 19 Jan 2026, Benjamin Coddington wrote:
>>>> On 17 Jan 2026, at 16:53, NeilBrown wrote:
>>>>
>>>>> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>>>>>> If fh-key-file=<path> is set in nfs.conf, the "nfsdctl autostart" command
>>>>>
>>>>> ... is set in THE NFSD SECTION OF nfs.conf
>>>>>
>>>>>
>>>>>> will hash the contents of the file with libuuid's uuid_generate_sha1 and
>>>>>> send the first 16 bytes into the kernel via NFSD_CMD_FH_KEY_SET.
>>>>>
>>>>> This patch adds no code that uses uuid_generate_sha1(), and doesn't
>>>>> provide any code for hash_fh_key_file()...
>>>>
>>>> I forgot to add the hash function after moving it into libnfs to make it
>>>> available to both rpc.nfsd and nfsdctl -- here it is, will fix on v2:
>>>>
>>>> diff --git a/support/nfs/fh_key_file.c b/support/nfs/fh_key_file.c
>>>> new file mode 100644
>>>> index 000000000000..350d36bf8649
>>>> --- /dev/null
>>>> +++ b/support/nfs/fh_key_file.c
>>>> @@ -0,0 +1,83 @@
>>>> +/*
>>>> + * Copyright (c) 2025 Benjamin Coddington <bcodding@hammerspace.com>
>>>> + * All rights reserved.
>>>> + *
>>>> + * Redistribution and use in source and binary forms, with or without
>>>> + * modification, are permitted provided that the following conditions
>>>> + * are met:
>>>> + * 1. Redistributions of source code must retain the above copyright
>>>> + *    notice, this list of conditions and the following disclaimer.
>>>> + * 2. Redistributions in binary form must reproduce the above copyright
>>>> + *    notice, this list of conditions and the following disclaimer in the
>>>> + *    documentation and/or other materials provided with the distribution.
>>>> + *
>>>> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
>>>> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
>>>> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
>>>> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
>>>> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
>>>> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>>>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>>>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>>>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>>>> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>>> + */
>>>
>>> I wonder if it is time to stop putting this boilerplate in nfs-utils and
>>> start using SPDX like the kernel does.
>>>
>>>> +
>>>> +#include <sys/types.h>
>>>> +#include <unistd.h>
>>>> +#include <errno.h>
>>>> +#include <uuid/uuid.h>
>>>> +
>>>> +#include "nfslib.h"
>>>> +
>>>> +#define HASH_BLOCKSIZE  256
>>>> +int hash_fh_key_file(const char *fh_key_file, uuid_t uuid)
>>>> +{
>>>> +	const char seed_s[] = "8fc57f1b-1a6f-482f-af92-d2e007c1ae58";
>>>> +	FILE *sfile = NULL;
>>>> +	char *buf = malloc(HASH_BLOCKSIZE);
>>>
>>> Can this be
>>>    char buf[HASH_BLOCKSIZE];
>>> ??
>>>
>>>> +	size_t pos;
>>>> +	int ret = 0;
>>>> +
>>>> +	if (!buf)
>>>> +		goto out;
>>>> +
>>>> +	sfile = fopen(fh_key_file, "r");
>>>> +	if (!sfile) {
>>>> +		ret = errno;
>>>> +		xlog(L_ERROR, "Unable to read fh-key-file %s: %s", fh_key_file, strerror(errno));
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +	uuid_parse(seed_s, uuid);
>>>> +	while (1) {
>>>> +		size_t sread;
>>>> +		pos = 0;
>>>> +
>>>> +		while (1) {
>>>> +			if (feof(sfile))
>>>> +				goto finish_block;
>>>> +
>>>> +			sread = fread(buf + pos, 1, HASH_BLOCKSIZE - pos, sfile);
>>>> +			pos += sread;
>>>> +
>>>> +			if (pos == HASH_BLOCKSIZE)
>>>> +				break;
>>>> +
>>>> +			if (sread == 0) {
>>>> +				if (ferror(sfile))
>>>> +					goto out;
>>>> +				goto finish_block;
>>>> +			}
>>>> +		}
>>>
>>> I think this inner look is not needed or wanted.
>>> fread() will loop as needed until EOF or an error, and we don't want to
>>> continue on an error.
>>
>> The fh_key_file can be any length - and this function reads it in
>> HASH_BLOCKSIZE chunks.  Each chuck gets hashed against the previous result.
>
> Yes, so the outer loop is clearly needed.
> The inner loop will never execute more than once as fread() only returns
> short reads on EOF or error.

Oh yeah - you're quite right.  Crazy how a blind spot can stick like that.
Thanks!

Ben

Re: [PATCH v1 2/2] exportfs: Add support for export option sign_fh

[NeilBrown]

On Sat, 17 Jan 2026, Benjamin Coddington wrote:
> If configured with "sign_fh", exports will be flagged to signal that
> filehandles should be signed with a Message Authentication Code (MAC).
> 
> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
> ---
>  support/include/nfs/export.h | 2 +-
>  support/nfs/exports.c        | 4 ++++
>  utils/exportfs/exportfs.c    | 2 ++
>  utils/exportfs/exports.man   | 9 +++++++++
>  4 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/support/include/nfs/export.h b/support/include/nfs/export.h
> index be5867cffc3c..ef3f3e7ea684 100644
> --- a/support/include/nfs/export.h
> +++ b/support/include/nfs/export.h
> @@ -19,7 +19,7 @@
>  #define NFSEXP_GATHERED_WRITES	0x0020
>  #define NFSEXP_NOREADDIRPLUS	0x0040
>  #define NFSEXP_SECURITY_LABEL	0x0080
> -/* 0x100 unused */
> +#define NFSEXP_SIGN_FH		0x0100
>  #define NFSEXP_NOHIDE		0x0200
>  #define NFSEXP_NOSUBTREECHECK	0x0400
>  #define NFSEXP_NOAUTHNLM	0x0800
> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> index 21ec6486ba3d..6b4ca87ee957 100644
> --- a/support/nfs/exports.c
> +++ b/support/nfs/exports.c
> @@ -310,6 +310,8 @@ putexportent(struct exportent *ep)
>  		fprintf(fp, "nordirplus,");
>  	if (ep->e_flags & NFSEXP_SECURITY_LABEL)
>  		fprintf(fp, "security_label,");
> +	if (ep->e_flags & NFSEXP_SIGN_FH)
> +		fprintf(fp, "sign_fh,");
>  	fprintf(fp, "%spnfs,", (ep->e_flags & NFSEXP_PNFS)? "" : "no_");
>  	if (ep->e_flags & NFSEXP_FSID) {
>  		fprintf(fp, "fsid=%d,", ep->e_fsid);
> @@ -676,6 +678,8 @@ parseopts(char *cp, struct exportent *ep, int *had_subtree_opt_ptr)
>  			setflags(NFSEXP_NOREADDIRPLUS, active, ep);
>  		else if (!strcmp(opt, "security_label"))
>  			setflags(NFSEXP_SECURITY_LABEL, active, ep);
> +		else if (!strcmp(opt, "sign_fh"))
> +			setflags(NFSEXP_SIGN_FH, active, ep);
>  		else if (!strcmp(opt, "nohide"))
>  			setflags(NFSEXP_NOHIDE, active, ep);
>  		else if (!strcmp(opt, "hide"))
> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> index 748c38e3e966..54ce62c5ce9a 100644
> --- a/utils/exportfs/exportfs.c
> +++ b/utils/exportfs/exportfs.c
> @@ -718,6 +718,8 @@ dump(int verbose, int export_format)
>  				c = dumpopt(c, "nordirplus");
>  			if (ep->e_flags & NFSEXP_SECURITY_LABEL)
>  				c = dumpopt(c, "security_label");
> +			if (ep->e_flags & NFSEXP_SIGN_FH)
> +				c = dumpopt(c, "sign_fh");
>  			if (ep->e_flags & NFSEXP_NOACL)
>  				c = dumpopt(c, "no_acl");
>  			if (ep->e_flags & NFSEXP_PNFS)
> diff --git a/utils/exportfs/exports.man b/utils/exportfs/exports.man
> index 39dc30fb8290..8549232dea74 100644
> --- a/utils/exportfs/exports.man
> +++ b/utils/exportfs/exports.man
> @@ -351,6 +351,15 @@ file.  If you put neither option,
>  .B exportfs
>  will warn you that the change has occurred.
>  
> +.TP
> +.IR sign_fh
> +This option enforces signing filehandles on the export.  If the
> +server has been configured with a secret key for such purpose, filehandles
> +will include a hash to verify the filehandle was created by the server in
> +order to guard against filehandle reverse-engineering attacks.  Note that
> +for NFSv2 and NFSv3, some exported filesystems may exceed the maximum
> +filehandle size when the signing hash is added.

I don't think "reverse-engineering" tells the admin anything useful.

"... to guard against filehandle guessing attacks which can bypass
 path-name based access restrictions"

NeilBrown


> +
>  .TP
>  .IR insecure_locks
>  .TP
> -- 
> 2.50.1
> 
> 

Re: [PATCH v1 2/2] exportfs: Add support for export option sign_fh

[Benjamin Coddington]

On 17 Jan 2026, at 16:59, NeilBrown wrote:

> On Sat, 17 Jan 2026, Benjamin Coddington wrote:
>> If configured with "sign_fh", exports will be flagged to signal that
>> filehandles should be signed with a Message Authentication Code (MAC).
>>
>> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
>> ---
>>  support/include/nfs/export.h | 2 +-
>>  support/nfs/exports.c        | 4 ++++
>>  utils/exportfs/exportfs.c    | 2 ++
>>  utils/exportfs/exports.man   | 9 +++++++++
>>  4 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/support/include/nfs/export.h b/support/include/nfs/export.h
>> index be5867cffc3c..ef3f3e7ea684 100644
>> --- a/support/include/nfs/export.h
>> +++ b/support/include/nfs/export.h
>> @@ -19,7 +19,7 @@
>>  #define NFSEXP_GATHERED_WRITES	0x0020
>>  #define NFSEXP_NOREADDIRPLUS	0x0040
>>  #define NFSEXP_SECURITY_LABEL	0x0080
>> -/* 0x100 unused */
>> +#define NFSEXP_SIGN_FH		0x0100
>>  #define NFSEXP_NOHIDE		0x0200
>>  #define NFSEXP_NOSUBTREECHECK	0x0400
>>  #define NFSEXP_NOAUTHNLM	0x0800
>> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
>> index 21ec6486ba3d..6b4ca87ee957 100644
>> --- a/support/nfs/exports.c
>> +++ b/support/nfs/exports.c
>> @@ -310,6 +310,8 @@ putexportent(struct exportent *ep)
>>  		fprintf(fp, "nordirplus,");
>>  	if (ep->e_flags & NFSEXP_SECURITY_LABEL)
>>  		fprintf(fp, "security_label,");
>> +	if (ep->e_flags & NFSEXP_SIGN_FH)
>> +		fprintf(fp, "sign_fh,");
>>  	fprintf(fp, "%spnfs,", (ep->e_flags & NFSEXP_PNFS)? "" : "no_");
>>  	if (ep->e_flags & NFSEXP_FSID) {
>>  		fprintf(fp, "fsid=%d,", ep->e_fsid);
>> @@ -676,6 +678,8 @@ parseopts(char *cp, struct exportent *ep, int *had_subtree_opt_ptr)
>>  			setflags(NFSEXP_NOREADDIRPLUS, active, ep);
>>  		else if (!strcmp(opt, "security_label"))
>>  			setflags(NFSEXP_SECURITY_LABEL, active, ep);
>> +		else if (!strcmp(opt, "sign_fh"))
>> +			setflags(NFSEXP_SIGN_FH, active, ep);
>>  		else if (!strcmp(opt, "nohide"))
>>  			setflags(NFSEXP_NOHIDE, active, ep);
>>  		else if (!strcmp(opt, "hide"))
>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>> index 748c38e3e966..54ce62c5ce9a 100644
>> --- a/utils/exportfs/exportfs.c
>> +++ b/utils/exportfs/exportfs.c
>> @@ -718,6 +718,8 @@ dump(int verbose, int export_format)
>>  				c = dumpopt(c, "nordirplus");
>>  			if (ep->e_flags & NFSEXP_SECURITY_LABEL)
>>  				c = dumpopt(c, "security_label");
>> +			if (ep->e_flags & NFSEXP_SIGN_FH)
>> +				c = dumpopt(c, "sign_fh");
>>  			if (ep->e_flags & NFSEXP_NOACL)
>>  				c = dumpopt(c, "no_acl");
>>  			if (ep->e_flags & NFSEXP_PNFS)
>> diff --git a/utils/exportfs/exports.man b/utils/exportfs/exports.man
>> index 39dc30fb8290..8549232dea74 100644
>> --- a/utils/exportfs/exports.man
>> +++ b/utils/exportfs/exports.man
>> @@ -351,6 +351,15 @@ file.  If you put neither option,
>>  .B exportfs
>>  will warn you that the change has occurred.
>>
>> +.TP
>> +.IR sign_fh
>> +This option enforces signing filehandles on the export.  If the
>> +server has been configured with a secret key for such purpose, filehandles
>> +will include a hash to verify the filehandle was created by the server in
>> +order to guard against filehandle reverse-engineering attacks.  Note that
>> +for NFSv2 and NFSv3, some exported filesystems may exceed the maximum
>> +filehandle size when the signing hash is added.
>
> I don't think "reverse-engineering" tells the admin anything useful.
>
> "... to guard against filehandle guessing attacks which can bypass
>  path-name based access restrictions"

Your words are much better - thanks.

Ben