[RFC/v1] kNFSD Encrypted Filehandles

[RFC/v1] kNFSD Encrypted Filehandles

[Benjamin Coddington]

Following are patches for nfs-utils and linux kernel to implement kNFSD
encrypted filehandles.  Currently, kNFSD's filehandles expose a lot of
information to anyone able to view them.  On some systems, filehandle
guesses can be used to subvert security measures.

This is a working implementation, but it is still in rough shape - there are
various comments left over and I would like to introduce a few more
tracepoints and refine man pages and/or add kernel documentation.  I plan on
doing this work in parallel with accepting critique and refining the
approach.

That said, I'm posting this before traveling for a few weeks and will be
slow to respond in that timeframe.  I'd expect to have another version out
later in the month of January.

All comments and critique welcome - thanks for looking!
Ben

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

[Benjamin Coddington]

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

The secret key is only implemented via nfs.conf and "nfsdctl autostart", as
further interfaces to set it into the kernel have not yet been built.  I
expect we may need a way for rpc.nfsd to set the key - but perhaps the
community would like to continue to transition to nfsdctl for future
features.

Comments and critique welcomed.

Benjamin Coddington (2):
  nfsdctl: Add support for passing encrypted filehandle key
  exportfs: Add support for export option encrypt_fh

 configure.ac                 |   4 +-
 nfs.conf                     |   1 +
 support/include/nfs/export.h |   2 +-
 support/nfs/exports.c        |   4 ++
 systemd/nfs.conf.man         |   1 +
 utils/exportfs/exportfs.c    |   2 +
 utils/exportfs/exports.man   |   6 ++
 utils/nfsdctl/Makefile.am    |   2 +-
 utils/nfsdctl/nfsd_netlink.h |   2 +
 utils/nfsdctl/nfsdctl.c      | 132 ++++++++++++++++++++++++++++++++++-
 utils/nfsdctl/nfsdctl.h      |  10 ++-
 11 files changed, 158 insertions(+), 8 deletions(-)

-- 
2.50.1

[PATCH v1 1/2] nfsdctl: 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.

A compatible kernel can use this key to encrypt filehandles.

Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
 configure.ac                 |   4 +-
 nfs.conf                     |   1 +
 systemd/nfs.conf.man         |   1 +
 utils/nfsdctl/Makefile.am    |   2 +-
 utils/nfsdctl/nfsd_netlink.h |   2 +
 utils/nfsdctl/nfsdctl.c      | 132 ++++++++++++++++++++++++++++++++++-
 utils/nfsdctl/nfsdctl.h      |  10 ++-
 7 files changed, 145 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6da23915836d..91122d09c4ce 100644
--- a/configure.ac
+++ b/configure.ac
@@ -263,9 +263,9 @@ AC_ARG_ENABLE(nfsdctl,
 		PKG_CHECK_MODULES(LIBREADLINE, readline)
 		AC_CHECK_HEADERS(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/systemd/nfs.conf.man b/systemd/nfs.conf.man
index e6a84a9725da..cc4894d2b00e 100644
--- a/systemd/nfs.conf.man
+++ b/systemd/nfs.conf.man
@@ -175,6 +175,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/nfsdctl/Makefile.am b/utils/nfsdctl/Makefile.am
index 89c7ecd6f30b..25af60b438cc 100644
--- a/utils/nfsdctl/Makefile.am
+++ b/utils/nfsdctl/Makefile.am
@@ -8,6 +8,6 @@ sbin_PROGRAMS	= nfsdctl
 noinst_HEADERS = nfsdctl.h
 nfsdctl_SOURCES = nfsdctl.c
 nfsdctl_CFLAGS = $(LIBNL3_CFLAGS) $(LIBNLGENL3_CFLAGS) $(LIBREADLINE_CFLAGS)
-nfsdctl_LDADD = ../../support/nfs/libnfs.la $(LIBNL3_LIBS) $(LIBNLGENL3_LIBS) $(LIBREADLINE_LIBS)
+nfsdctl_LDADD = ../../support/nfs/libnfs.la $(LIBNL3_LIBS) $(LIBNLGENL3_LIBS) $(LIBREADLINE_LIBS) -luuid
 
 MAINTAINERCLEANFILES = Makefile.in
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 7d2d4fd93211..0039a07bb003 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -28,6 +28,7 @@
 
 #include <readline/readline.h>
 #include <readline/history.h>
+#include <uuid/uuid.h>
 
 #ifdef USE_SYSTEM_NFSD_NETLINK_H
 #include <linux/nfsd_netlink.h>
@@ -1567,6 +1568,128 @@ static int configure_listeners(void)
 	return ret;
 }
 
+#define HASH_BLOCKSIZE  256
+static int hash_fh_key_file(const char *fh_key_file, uuid_t uuid)
+{
+	const char seed_s[] = "8fc57f1b-1a6f-482f-af92-d2e007c1ae58";
+	FILE *sfile;
+	char *buf = malloc(HASH_BLOCKSIZE);
+	size_t pos;
+	int ret;
+
+	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;
+}
+
+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));
+		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);
@@ -1581,7 +1704,7 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
 	int *threads, grace, lease, idx, ret, opt, pools;
 	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;
@@ -1653,6 +1776,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");
 
diff --git a/utils/nfsdctl/nfsdctl.h b/utils/nfsdctl/nfsdctl.h
index f0aa3ab862c2..12491b833f56 100644
--- a/utils/nfsdctl/nfsdctl.h
+++ b/utils/nfsdctl/nfsdctl.h
@@ -1,11 +1,15 @@
 /* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
-/* Do not edit directly, auto-generated from: */
-/*	Documentation/netlink/specs/nfsd.yaml */
-/* YNL-GEN uapi header */
+/*
+ * Copyright (c) 2023 Lorenzo Bianconi <lorenzo@kernel.org>
+ * Copyright (c) 2025 Benjamin Coddington <bcodding@hammerspace.com>
+ */
 
 #ifndef _UTILS_NFSDCTL_NFSDCTL_H
 #define _UTILS_NFSDCTL_NFSDCTL_H
 
+#include <stdint.h>
+#include <stddef.h> /* For size_t */
+
 enum nfs_opnum4 {
 	OP_ACCESS = 3,
 	OP_CLOSE = 4,
-- 
2.50.1

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

[Benjamin Coddington]

If configured with "encrypt_fh", exports will be flagged to signal that
filehandles should be encrypted.

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   | 6 ++++++
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/support/include/nfs/export.h b/support/include/nfs/export.h
index be5867cffc3c..d893f49dd696 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_ENCRYPT_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..52467a99e7fb 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_ENCRYPT_FH)
+		fprintf(fp, "encrypt_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, "encrypt_fh"))
+			setflags(NFSEXP_ENCRYPT_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..908a99f48516 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_ENCRYPT_FH)
+				c = dumpopt(c, "encrypt_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..610b603aa859 100644
--- a/utils/exportfs/exports.man
+++ b/utils/exportfs/exports.man
@@ -351,6 +351,12 @@ file.  If you put neither option,
 .B exportfs
 will warn you that the change has occurred.
 
+.TP
+.IR encrypt_fh
+This option enforces the encryption of filehandles on the export.  If the
+server has been configured with a secret key for such purpose, filehandles
+will be reversibly hashed to impede filehandle reverse-engineering attacks.
+
 .TP
 .IR insecure_locks
 .TP
-- 
2.50.1

[PATCH v1 0/7] kNFSD Encrypted Filehandles

[Benjamin Coddington]

In order to harden kNFSD against various filehandle manipulation techniques
the following patches implement a method of reversibly encrypting filehandle
contents.

Using the kernel's skcipher AES-CBC, filehandles are encrypted by firstly
hashing the fileid using the fsid as a salt, then using the hashed fileid as
the first block to finally hash the fsid.

The first attempts at this used stack-allocated buffers, but I ran into many
memory alignment problems on my arm64 machine that sent me back to using
GFP_KERNEL allocations (here's to you /include/linux/scatterlist.h:210).  In
order to avoid constant allocation/freeing, the buffers are allocated once
for every knfsd thread.  If anyone has suggestions for reducing the number
of buffers required and their memcpy() operations, I am all ears.

Currently the code overloads filehandle's auth_type byte.  This seems
appropriate for this purpose, but this implementation does not actually
reject unencrypted filehandles on an export that is giving out encrypted
ones.  I expect we'll want to tighten this up in a future version.

Comments and critique welcome.

Benjamin Coddington (7):
  nfsd: Convert export flags to use BIT() macro
  nfsd: Add a symmetric-key cipher for encrypted filehandles
  nfsd/sunrpc: add per-thread crypto context pointer
  NFSD: Add a per-knfsd reusable encfh_buf
  NFSD/export: Add encrypt_fh export option
  NFSD: Add filehandle crypto functions and helpers
  NFSD: Enable filehandle encryption

 Documentation/netlink/specs/nfsd.yaml |  12 ++
 fs/nfsd/export.c                      |   7 +-
 fs/nfsd/localio.c                     |   2 +-
 fs/nfsd/lockd.c                       |   2 +-
 fs/nfsd/netlink.c                     |  15 +++
 fs/nfsd/netlink.h                     |   1 +
 fs/nfsd/netns.h                       |   1 +
 fs/nfsd/nfs3proc.c                    |  10 +-
 fs/nfsd/nfs3xdr.c                     |  14 +-
 fs/nfsd/nfs4proc.c                    |  10 +-
 fs/nfsd/nfs4xdr.c                     |  14 +-
 fs/nfsd/nfsctl.c                      |  40 +++++-
 fs/nfsd/nfsfh.c                       | 179 +++++++++++++++++++++++++-
 fs/nfsd/nfsfh.h                       |  26 +++-
 fs/nfsd/nfsproc.c                     |   8 +-
 fs/nfsd/trace.h                       |  19 +++
 include/linux/sunrpc/svc.h            |  12 +-
 include/uapi/linux/nfsd/export.h      |  36 +++---
 include/uapi/linux/nfsd_netlink.h     |   2 +
 net/sunrpc/svc.c                      |   1 +
 20 files changed, 356 insertions(+), 55 deletions(-)

-- 
2.50.1

[PATCH v1 1/7] 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 | 36 ++++++++++++++++----------------
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 5ce9a49e76ba..ad1f3af8d682 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -166,7 +166,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%x 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..aac57180c5c4 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,8 +51,8 @@
  * 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
-- 
2.50.1

[PATCH v1 2/7] nfsd: Add a symmetric-key cipher for encrypted filehandles

[Benjamin Coddington]

Expand the nfsd_net to hold a reference to a crypto_sync_skcipher.

Expand the netlink server interface to allow the setting of 128-bit
fh_key value to be used as an encryption key for filehandles.

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                       |  1 +
 fs/nfsd/nfsctl.c                      | 38 +++++++++++++++++++++++++++
 fs/nfsd/trace.h                       | 19 ++++++++++++++
 include/uapi/linux/nfsd_netlink.h     |  2 ++
 7 files changed, 88 insertions(+)

diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index 100363029e82..1e4b2d1a61e8 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -78,6 +78,9 @@ attribute-sets:
       -
         name: scope
         type: string
+      -
+        name: fh-key
+        type: u64
   -
     name: version
     attributes:
@@ -222,3 +225,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 ac51a44e1065..f932d9b16e0c 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -46,6 +46,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[] = {
 	{
@@ -101,6 +109,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 3e2d0fde80a7..b914b34ac0f7 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -217,6 +217,7 @@ struct nfsd_net {
 	spinlock_t              local_clients_lock;
 	struct list_head	local_clients;
 #endif
+	struct crypto_sync_skcipher *encfh_tfm;
 };
 
 /* 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 ad1f3af8d682..ed08ca63f139 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"
@@ -2129,6 +2130,33 @@ 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)
+{
+	struct crypto_sync_skcipher *encfh_tfm;
+	struct nfsd_net *nn;
+	int fh_key_len;
+	u8 fh_key[16];
+	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 != 16)
+		return -EINVAL;
+
+	memcpy(fh_key, nla_data(info->attrs[NFSD_A_SERVER_FH_KEY]), 16);
+	nn = net_generic(genl_info_net(info), nfsd_net_id);
+	encfh_tfm = nn->encfh_tfm;
+	ret = crypto_sync_skcipher_setkey(encfh_tfm, fh_key, 16);
+
+	trace_nfsd_ctl_fh_key_set(fh_key, ret);
+	return ret;
+}
+
+
+
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace
@@ -2171,6 +2199,13 @@ static __net_init int nfsd_net_init(struct net *net)
 	nn->nfsd_serv = NULL;
 	nfsd4_init_leases_net(nn);
 	get_random_bytes(&nn->siphash_key, sizeof(nn->siphash_key));
+
+	nn->encfh_tfm = crypto_alloc_sync_skcipher("cbc(aes)", 0, 0);
+	if (IS_ERR(nn->encfh_tfm)) {
+		retval = PTR_ERR(nn->encfh_tfm);
+		goto out_encfh_error;
+	}
+
 	seqlock_init(&nn->writeverf_lock);
 #if IS_ENABLED(CONFIG_NFS_LOCALIO)
 	spin_lock_init(&nn->local_clients_lock);
@@ -2178,6 +2213,8 @@ static __net_init int nfsd_net_init(struct net *net)
 #endif
 	return 0;
 
+out_encfh_error:
+	nfsd_proc_stat_shutdown(net);
 out_proc_error:
 	percpu_counter_destroy_many(nn->counter, NFSD_STATS_COUNTERS_NUM);
 out_repcache_error:
@@ -2214,6 +2251,7 @@ static __net_exit void nfsd_net_exit(struct net *net)
 {
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
 
+	crypto_free_sync_skcipher(nn->encfh_tfm);
 	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 5ae2a611e57f..8b1330bf8c36 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -2186,6 +2186,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 e157e2009ea8..952c98fca3f8 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -35,6 +35,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)
@@ -89,6 +90,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/7] nfsd/sunrpc: add per-thread crypto context pointer

[Benjamin Coddington]

Move rq_err up to fill a 4-byte hole (arm64), and add a pointer to store
per-thread encryption objects for encrypting filehandles.  This allows nfsd
to only perform the memory allocations and skcipher request setup once
per-knfsd thread rather than using stack memory or doing multiple
allocations for every encrypt/decrypt cycle.

Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
 include/linux/sunrpc/svc.h | 9 +++++----
 net/sunrpc/svc.c           | 1 +
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 5506d20857c3..479a52a755af 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -228,6 +228,10 @@ struct svc_rqst {
 	int			rq_reserved;	/* space on socket outq
 						 * reserved for this request
 						 */
+	int			rq_err;		/* Thread sets this to inidicate
+						 * initialisation success.
+						 */
+
 	ktime_t			rq_stime;	/* start time */
 
 	struct cache_req	rq_chandle;	/* handle passed to caches for 
@@ -241,14 +245,11 @@ struct svc_rqst {
 						 * net namespace
 						 */
 
-	int			rq_err;		/* Thread sets this to inidicate
-						 * initialisation success.
-						 */
-
 	unsigned long		bc_to_initval;
 	unsigned int		bc_to_retries;
 	unsigned int		rq_status_counter; /* RPC processing counter */
 	void			**rq_lease_breaker; /* The v4 client breaking a lease */
+	void *			rq_crypto; /* handle for per-thread cryptography context */
 };
 
 /* bits for rq_flags */
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 4704dce7284e..eade597c9aae 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -674,6 +674,7 @@ svc_rqst_free(struct svc_rqst *rqstp)
 	kfree(rqstp->rq_resp);
 	kfree(rqstp->rq_argp);
 	kfree(rqstp->rq_auth_data);
+	kfree(rqstp->rq_crypto);
 	kfree_rcu(rqstp, rq_rcu_head);
 }
 
-- 
2.50.1

[PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf

[Benjamin Coddington]

The encfh_buf contains two NFS4_FHSIZE buffers, used interchangeably as
source or destination during filehandle encryption.  It also holds a
reusable skcipher_request.

Assign a pointer to this buffer into filehandles when calling fh_init().
Once allocated, nfsd can access the per-knfsd encfh_buf via the filehandle
itself.

Note - there's probably another approach: rather than assigning the buffer
pointer during fh_init(), it is probably just as easy to plumb ths svc_rqst
into the call paths for filehandle encryption/decryption.  I will probably
refactor this work in that direction.

Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
 fs/nfsd/export.c           |  2 +-
 fs/nfsd/localio.c          |  2 +-
 fs/nfsd/lockd.c            |  2 +-
 fs/nfsd/nfs3proc.c         | 10 +++++-----
 fs/nfsd/nfs3xdr.c          |  4 ++--
 fs/nfsd/nfs4proc.c         | 10 +++++-----
 fs/nfsd/nfs4xdr.c          |  2 +-
 fs/nfsd/nfsfh.h            | 12 +++++++++++-
 fs/nfsd/nfsproc.c          |  8 ++++----
 include/linux/sunrpc/svc.h |  3 +++
 10 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 9d55512d0cc9..5a53e1af88d2 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1050,7 +1050,7 @@ exp_rootfh(struct net *net, struct auth_domain *clp, char *name,
 	/*
 	 * fh must be initialized before calling fh_compose
 	 */
-	fh_init(&fh, maxsize);
+	fh_init(&fh, maxsize, NULL);
 	if (fh_compose(&fh, exp, path.dentry, NULL))
 		err = -EINVAL;
 	else
diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
index be710d809a3b..6e1a0e5e96c4 100644
--- a/fs/nfsd/localio.c
+++ b/fs/nfsd/localio.c
@@ -68,7 +68,7 @@ nfsd_open_local_fh(struct net *net, struct auth_domain *dom,
 		return localio;
 
 	/* nfs_fh -> svc_fh */
-	fh_init(&fh, NFS4_FHSIZE);
+	fh_init(&fh, NFS4_FHSIZE, NULL);
 	fh.fh_handle.fh_size = nfs_fh->size;
 	memcpy(fh.fh_handle.fh_raw, nfs_fh->data, nfs_fh->size);
 
diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
index c774ce9aa296..e6f1c4be5cdb 100644
--- a/fs/nfsd/lockd.c
+++ b/fs/nfsd/lockd.c
@@ -33,7 +33,7 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
 	struct svc_fh	fh;
 
 	/* must initialize before using! but maxsize doesn't matter */
-	fh_init(&fh,0);
+	fh_init(&fh, 0, NULL);
 	fh.fh_handle.fh_size = f->size;
 	memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
 	fh.fh_export = NULL;
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 42adc5461db0..3cdeb9e1bb20 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -123,7 +123,7 @@ nfsd3_proc_lookup(struct svc_rqst *rqstp)
 				argp->name);
 
 	fh_copy(&resp->dirfh, &argp->fh);
-	fh_init(&resp->fh, NFS3_FHSIZE);
+	fh_init(&resp->fh, NFS3_FHSIZE, rqstp);
 
 	resp->status = nfsd_lookup(rqstp, &resp->dirfh,
 				   argp->name, argp->len,
@@ -378,7 +378,7 @@ nfsd3_proc_create(struct svc_rqst *rqstp)
 	svc_fh *dirfhp, *newfhp;
 
 	dirfhp = fh_copy(&resp->dirfh, &argp->fh);
-	newfhp = fh_init(&resp->fh, NFS3_FHSIZE);
+	newfhp = fh_init(&resp->fh, NFS3_FHSIZE, rqstp);
 
 	resp->status = nfsd3_create_file(rqstp, dirfhp, newfhp, argp);
 	resp->status = nfsd3_map_status(resp->status);
@@ -399,7 +399,7 @@ nfsd3_proc_mkdir(struct svc_rqst *rqstp)
 
 	argp->attrs.ia_valid &= ~ATTR_SIZE;
 	fh_copy(&resp->dirfh, &argp->fh);
-	fh_init(&resp->fh, NFS3_FHSIZE);
+	fh_init(&resp->fh, NFS3_FHSIZE, rqstp);
 	resp->status = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len,
 				   &attrs, S_IFDIR, 0, &resp->fh);
 	resp->status = nfsd3_map_status(resp->status);
@@ -433,7 +433,7 @@ nfsd3_proc_symlink(struct svc_rqst *rqstp)
 	}
 
 	fh_copy(&resp->dirfh, &argp->ffh);
-	fh_init(&resp->fh, NFS3_FHSIZE);
+	fh_init(&resp->fh, NFS3_FHSIZE, rqstp);
 	resp->status = nfsd_symlink(rqstp, &resp->dirfh, argp->fname,
 				    argp->flen, argp->tname, &attrs, &resp->fh);
 	kfree(argp->tname);
@@ -457,7 +457,7 @@ nfsd3_proc_mknod(struct svc_rqst *rqstp)
 	dev_t	rdev = 0;
 
 	fh_copy(&resp->dirfh, &argp->fh);
-	fh_init(&resp->fh, NFS3_FHSIZE);
+	fh_init(&resp->fh, NFS3_FHSIZE, rqstp);
 
 	if (argp->ftype == NF3CHR || argp->ftype == NF3BLK) {
 		rdev = MKDEV(argp->major, argp->minor);
diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index ef4971d71ac4..854ee536e338 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -90,7 +90,7 @@ svcxdr_decode_nfs_fh3(struct xdr_stream *xdr, struct svc_fh *fhp)
 	p = xdr_inline_decode(xdr, size);
 	if (!p)
 		return false;
-	fh_init(fhp, NFS3_FHSIZE);
+	fh_init(fhp, NFS3_FHSIZE, ARGSTRM_RQST(xdr));
 	fhp->fh_handle.fh_size = size;
 	memcpy(&fhp->fh_handle.fh_raw, p, size);
 
@@ -1111,7 +1111,7 @@ svcxdr_encode_entry3_plus(struct nfsd3_readdirres *resp, const char *name,
 	bool result;
 
 	result = false;
-	fh_init(fhp, NFS3_FHSIZE);
+	fh_init(fhp, NFS3_FHSIZE, resp->rqstp);
 	if (compose_entry_fh(resp, fhp, name, namlen, ino) != nfs_ok)
 		goto out_noattrs;
 
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index b74800917583..18526542b542 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -425,7 +425,7 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru
 	*resfh = kmalloc(sizeof(struct svc_fh), GFP_KERNEL);
 	if (!*resfh)
 		return nfserr_jukebox;
-	fh_init(*resfh, NFS4_FHSIZE);
+	fh_init(*resfh, NFS4_FHSIZE, rqstp);
 	open->op_truncate = false;
 
 	if (open->op_create) {
@@ -785,7 +785,7 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	__be32 status;
 	dev_t rdev;
 
-	fh_init(&resfh, NFS4_FHSIZE);
+	fh_init(&resfh, NFS4_FHSIZE, rqstp);
 
 	status = fh_verify(rqstp, &cstate->current_fh, S_IFDIR, NFSD_MAY_NOP);
 	if (status)
@@ -910,7 +910,7 @@ static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
 	struct svc_fh tmp_fh;
 	__be32 ret;
 
-	fh_init(&tmp_fh, NFS4_FHSIZE);
+	fh_init(&tmp_fh, NFS4_FHSIZE, rqstp);
 	ret = exp_pseudoroot(rqstp, &tmp_fh);
 	if (ret)
 		return ret;
@@ -2883,8 +2883,8 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 	resp->tag = args->tag;
 	resp->rqstp = rqstp;
 	cstate->minorversion = args->minorversion;
-	fh_init(current_fh, NFS4_FHSIZE);
-	fh_init(save_fh, NFS4_FHSIZE);
+	fh_init(current_fh, NFS4_FHSIZE, rqstp);
+	fh_init(save_fh, NFS4_FHSIZE, rqstp);
 	/*
 	 * Don't use the deferral mechanism for NFSv4; compounds make it
 	 * too hard to avoid non-idempotency problems.
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 30ce5851fe4c..a45ab840ecd4 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3675,7 +3675,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 		status = nfserr_jukebox;
 		if (!tempfh)
 			goto out;
-		fh_init(tempfh, NFS4_FHSIZE);
+		fh_init(tempfh, NFS4_FHSIZE, rqstp);
 		status = fh_compose(tempfh, exp, dentry, NULL);
 		if (status)
 			goto out;
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 5ef7191f8ad8..f29bb09af242 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -13,6 +13,7 @@
 #include <linux/iversion.h>
 #include <linux/exportfs.h>
 #include <linux/nfs4.h>
+#include <crypto/skcipher.h>
 
 #include "export.h"
 
@@ -74,6 +75,13 @@ static inline ino_t u32_to_ino_t(__u32 uino)
 	return (ino_t) uino;
 }
 
+/* filehandle crypto buckets allocated per-knfsd */
+struct encfh_buf {
+	u8 a_buf[NFS4_FHSIZE];
+	u8 b_buf[NFS4_FHSIZE];
+	struct skcipher_request req;
+};
+
 /*
  * This is the internal representation of an NFS handle used in knfsd.
  * pre_mtime/post_version will be used to support wcc_attr's in NFSv3.
@@ -83,6 +91,7 @@ typedef struct svc_fh {
 	int			fh_maxsize;	/* max size for fh_handle */
 	struct dentry *		fh_dentry;	/* validated dentry */
 	struct svc_export *	fh_export;	/* export pointer */
+	struct svc_rqst	*	fh_rqstp;	/* access per-knfsd buffer for encfh_buf */
 
 	bool			fh_want_write;	/* remount protection taken */
 	bool			fh_no_wcc;	/* no wcc data needed */
@@ -244,10 +253,11 @@ fh_copy_shallow(struct knfsd_fh *dst, const struct knfsd_fh *src)
 }
 
 static __inline__ struct svc_fh *
-fh_init(struct svc_fh *fhp, int maxsize)
+fh_init(struct svc_fh *fhp, int maxsize, struct svc_rqst *rqstp)
 {
 	memset(fhp, 0, sizeof(*fhp));
 	fhp->fh_maxsize = maxsize;
+	fhp->fh_rqstp = rqstp;
 	return fhp;
 }
 
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 481e789a7697..83e1bb2a9a7e 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -162,7 +162,7 @@ nfsd_proc_lookup(struct svc_rqst *rqstp)
 	dprintk("nfsd: LOOKUP   %s %.*s\n",
 		SVCFH_fmt(&argp->fh), argp->len, argp->name);
 
-	fh_init(&resp->fh, NFS_FHSIZE);
+	fh_init(&resp->fh, NFS_FHSIZE, rqstp);
 	resp->status = nfsd_lookup(rqstp, &argp->fh, argp->name, argp->len,
 				   &resp->fh);
 	fh_put(&argp->fh);
@@ -312,7 +312,7 @@ nfsd_proc_create(struct svc_rqst *rqstp)
 		resp->status = nfserrno(PTR_ERR(dchild));
 		goto out_write;
 	}
-	fh_init(newfhp, NFS_FHSIZE);
+	fh_init(newfhp, NFS_FHSIZE, rqstp);
 	resp->status = fh_compose(newfhp, dirfhp->fh_export, dchild, dirfhp);
 	if (!resp->status && d_really_is_negative(dchild))
 		resp->status = nfserr_noent;
@@ -502,7 +502,7 @@ nfsd_proc_symlink(struct svc_rqst *rqstp)
 		goto out;
 	}
 
-	fh_init(&newfh, NFS_FHSIZE);
+	fh_init(&newfh, NFS_FHSIZE, rqstp);
 	resp->status = nfsd_symlink(rqstp, &argp->ffh, argp->fname, argp->flen,
 				    argp->tname, &attrs, &newfh);
 
@@ -533,7 +533,7 @@ nfsd_proc_mkdir(struct svc_rqst *rqstp)
 	}
 
 	argp->attrs.ia_valid &= ~ATTR_SIZE;
-	fh_init(&resp->fh, NFS_FHSIZE);
+	fh_init(&resp->fh, NFS_FHSIZE, rqstp);
 	resp->status = nfsd_create(rqstp, &argp->fh, argp->name, argp->len,
 				   &attrs, S_IFDIR, 0, &resp->fh);
 	fh_put(&argp->fh);
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 479a52a755af..109840dda93c 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -264,6 +264,9 @@ enum {
 
 #define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : rqst->rq_bc_net)
 
+#define ARGSTRM_RQST(xdr_stream) (container_of(xdr_stream, struct svc_rqst, rq_arg_stream))
+#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

[PATCH v1 5/7] NFSD/export: Add encrypt_fh export option

[Benjamin Coddington]

Setting the "encrypt_fh" export option sets NFSEXP_ENCRYPT_FH.  In a future
patch NFSD uses this signal to encrypt filehandles for that export.

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

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 5a53e1af88d2..acc20d389376 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_ENCRYPT_FH, {"encrypt_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 aac57180c5c4..8bb971f68558 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_ENCRYPT_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 */
-- 
2.50.1

[PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers

[Benjamin Coddington]

In order to improve the security of knfsd servers, create a method to
encrypt and decrypt filehandles.

Filehandle encryption begins by checking for an allocated encfh_buf for
each knfsd thread.  It not yet allocated, nfsd performs JIT alloation and
proceeds to encrypt or decrypt.

In order to increase entropy, filehandles are encrypted in two passes.  In
the first pass, the fileid is expanded to the AES block size and encrypted
with the server's key and a salt from the fsid.  In the second pass, the
entirety of the filehandle is encrypted starting with the block containing
the results of the first pass.  Decryption reverses this operation.

This approach ensures that the same fileid values are encrypted differently
for differing fsid values.  This protects against comparisons between the
same fileids across different exports that may not be encrypted, which
could ease the discovery of the server's private key.  Additionally, it
allows the fsid to be encrypted uniquely for each filehandle.

The filehandle's auth_type is used to indicate that a filehandle has been
encrypted.

Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
 fs/nfsd/nfsfh.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfsfh.h |  13 ++++
 2 files changed, 178 insertions(+)

diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index ed85dd43da18..86bdced0f905 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,170 @@ static inline __be32 check_pseudo_root(struct dentry *dentry,
 	return nfs_ok;
 }
 
+static int fh_crypto_init(struct svc_rqst *rqstp)
+{
+	struct encfh_buf *fh_encfh = (struct encfh_buf *)rqstp->rq_crypto;
+
+	/* This knfsd has not allocated buffers and reqest yet: */
+	if (!fh_encfh) {
+		struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+
+		fh_encfh = kmalloc(sizeof(struct encfh_buf), GFP_KERNEL);
+		if (!fh_encfh)
+			return -ENOMEM;
+
+		skcipher_request_set_sync_tfm(&fh_encfh->req, nn->encfh_tfm);
+		rqstp->rq_crypto = fh_encfh;
+	}
+	memset(fh_encfh->a_buf, 0, NFS4_FHSIZE);
+	memset(fh_encfh->b_buf, 0, NFS4_FHSIZE);
+	return 0;
+}
+
+static int fh_crypto(struct svc_fh *fhp, bool encrypting)
+{
+	struct encfh_buf *encfh = (struct encfh_buf *)fhp->fh_rqstp->rq_crypto;
+	int err, pad, hash_size, fileid_offset;
+	struct knfsd_fh *fh = &fhp->fh_handle;
+	struct scatterlist fh_sgl[2];
+	struct scatterlist hash_sg;
+	u8 *a_buf = encfh->a_buf;
+	u8 *b_buf = encfh->b_buf;
+	u8 iv[16];
+
+	/* blocksize */
+	int bs = crypto_sync_skcipher_blocksize(
+				crypto_sync_skcipher_reqtfm(&encfh->req));
+
+	/* always renew as it gets transformed: */
+	memset(iv, 0, sizeof(iv));
+
+	fileid_offset = fh_fileid_offset(fh);
+	sg_init_table(fh_sgl, 2);
+
+	if (encrypting) {
+		/* encryption */
+		memcpy(&a_buf[fileid_offset], &fh->fh_raw[fileid_offset],
+				fh->fh_size - fileid_offset);
+		memcpy(b_buf, fh->fh_raw, fileid_offset);
+
+		/* encrypt the fileid using the fsid as iv: */
+		memcpy(iv, fh_fsid(fh), min(sizeof(iv), key_len(fh->fh_fsid_type)));
+
+		/* pad out the fileid to block size */
+		hash_size = fh_fileid_len(fh);
+		pad = (bs - (hash_size & (bs - 1))) & (bs - 1);
+		hash_size += pad;
+
+		sg_set_buf(&fh_sgl[0], &a_buf[fileid_offset], hash_size);
+		sg_mark_end(&fh_sgl[1]);  /* don't need sg1 yet */
+		sg_init_one(&hash_sg, &b_buf[fileid_offset], hash_size);
+
+		skcipher_request_set_crypt(&encfh->req, fh_sgl, &hash_sg, hash_size, iv);
+		err = crypto_skcipher_encrypt(&encfh->req);
+		if (err)
+			goto out;
+
+		/* encrypt the fsid + fileid with zero iv, starting with the last
+		 * block of the hashed fileid */
+		memset(iv, 0, sizeof(iv));
+
+		/* calculate the new padding: */
+		hash_size += key_len(fh->fh_fsid_type) + 4;
+		pad = (bs - (hash_size & (bs - 1))) & (bs - 1);
+		hash_size += pad;
+
+		sg_unmark_end(&fh_sgl[1]); /* now we use it */
+		sg_set_buf(&fh_sgl[0], &b_buf[hash_size-bs], bs);
+		sg_set_buf(&fh_sgl[1], b_buf, hash_size-bs);
+		sg_init_one(&hash_sg, a_buf, hash_size);
+
+		skcipher_request_set_crypt(&encfh->req, fh_sgl, &hash_sg, hash_size, iv);
+		err = crypto_skcipher_encrypt(&encfh->req);
+
+		if (!err) {
+			memcpy(&fh->fh_raw[4], a_buf, hash_size);
+			fh->fh_auth_type = FH_AT_ENCRYPTED;
+			fh->fh_fileid_type = fh->fh_size; /* we'll use this in decryption */
+			fh->fh_size = hash_size + 4;
+		}
+	} else {
+		/* decryption */
+		int fh_size = fh->fh_size - 4;
+		memcpy(b_buf, &fh->fh_raw[4], fh_size);
+
+		/* first, we decode starting with the last hashed block and zero iv */
+		hash_size = fh_size;
+		sg_set_buf(&fh_sgl[0], &a_buf[fh_size - bs], bs);
+		sg_set_buf(&fh_sgl[1], a_buf, fh_size - bs);
+		sg_init_one(&hash_sg, b_buf, fh_size);
+
+		skcipher_request_set_crypt(&encfh->req, &hash_sg, fh_sgl, hash_size, iv);
+		err = crypto_skcipher_decrypt(&encfh->req);
+		if (err)
+			goto out;
+
+		/* Now we're dealing with the original fh_size: */
+		fh_size = fh->fh_fileid_type;
+
+		/* a_buf now has the decrypted fsid and header: */
+		memcpy(fh->fh_raw, a_buf, fileid_offset);
+
+		/* now we set the iv to the decrypted fsid value */
+		memset(iv, 0, sizeof(iv));;
+		memcpy(iv, &a_buf[4], min(sizeof(iv), key_len(fh->fh_fsid_type)));
+
+		/* align to block size */
+		hash_size = fh_size - fileid_offset;
+		pad = (bs - (hash_size & (bs - 1))) & (bs - 1);
+		hash_size += pad;
+
+		/* decrypt only the fileid: */
+		sg_set_buf(&fh_sgl[0], &b_buf[fileid_offset], hash_size);
+		sg_mark_end(&fh_sgl[1]);
+		sg_init_one(&hash_sg, &a_buf[fileid_offset], hash_size);
+
+		skcipher_request_set_crypt(&encfh->req, &hash_sg, fh_sgl, hash_size, iv);
+		err = crypto_skcipher_decrypt(&encfh->req);
+
+		if (!err) {
+			fh->fh_size = fh_size;
+			/* copy in the fileid */
+			memcpy(&fh->fh_raw[fileid_offset], &b_buf[fileid_offset], hash_size);
+			/* trim the leftover hash padding */
+			memset(&fh->fh_raw[fh->fh_size], 0, NFS4_FHSIZE - fh->fh_size);
+		}
+	}
+	// add a tracepoint to show the error;
+	// if decrypting, we want nfserr_badhandle
+out:
+	return err;
+}
+
+/* we should never get here without calling fh_init first */
+int fh_encrypt(struct svc_fh *fhp)
+{
+	if (!(fhp->fh_export->ex_flags & NFSEXP_ENCRYPT_FH))
+		return 0;
+
+	if (fh_crypto_init(fhp->fh_rqstp))
+		return -ENOMEM;
+
+	return fh_crypto(fhp, true);
+}
+
+/* Lets try to decrypt, no matter the export setting */
+static int fh_decrypt(struct svc_fh *fhp)
+{
+	if (fhp->fh_handle.fh_auth_type != FH_AT_ENCRYPTED)
+		return 0;
+
+	if (fh_crypto_init(fhp->fh_rqstp))
+		return -ENOMEM;
+
+	return fh_crypto(fhp, false);
+}
+
 /*
  * Use the given filehandle to look up the corresponding export and
  * dentry.  On success, the results are used to set fh_export and
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index f29bb09af242..786f34e72304 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -60,6 +60,9 @@ struct knfsd_fh {
 #define fh_fsid_type		fh_raw[2]
 #define fh_fileid_type		fh_raw[3]
 
+#define FH_AT_PLAIN		0
+#define FH_AT_ENCRYPTED	1
+
 static inline u32 *fh_fsid(const struct knfsd_fh *fh)
 {
 	return (u32 *)&fh->fh_raw[4];
@@ -284,6 +287,16 @@ 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)
+{
+	return fh->fh_size - fh_fileid_offset(fh);
+}
+
 /**
  * fh_want_write - Get write access to an export
  * @fhp: File handle of file to be written
-- 
2.50.1

[PATCH v1 7/7] NFSD: Enable filehandle encryption

[Benjamin Coddington]

Add calls to fh_encrypt() and fh_decrypt() to allow the server to encrypt
and decrypt filehandles.  The fh_encrypt() calls are from the xdr encoders
instead of within fh_compose() because callers of fh_compose() may later
call fh_update() after file creation.  Doing encryption during fh_compose()
would scramble the raw filehandle information fh_update() requires.

NFSD can call fh_decrypt() from a single location - when it needs to
resolve a dentry from the filehandle: nfsd_set_fh_dentry().

Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
 fs/nfsd/nfs3xdr.c | 10 +++++++---
 fs/nfsd/nfs4xdr.c | 12 ++++++++----
 fs/nfsd/nfsfh.c   | 14 ++++++++++++--
 fs/nfsd/nfsfh.h   |  1 +
 4 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index 854ee536e338..b2d3c0e95e86 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -120,11 +120,15 @@ 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 xdr_stream *xdr, struct svc_fh *fhp)
 {
-	u32 size = fhp->fh_handle.fh_size;
+	u32 size;
 	__be32 *p;
 
+	if (fh_encrypt(fhp))
+		return false;
+	size = fhp->fh_handle.fh_size;
+
 	p = xdr_reserve_space(xdr, XDR_UNIT + size);
 	if (!p)
 		return false;
@@ -137,7 +141,7 @@ 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 xdr_stream *xdr, struct svc_fh *fhp)
 {
 	if (xdr_stream_encode_item_present(xdr) < 0)
 		return false;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index a45ab840ecd4..7af8cbaf401b 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2563,9 +2563,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_encrypt(fhp))
+		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 */
@@ -3134,7 +3138,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,
@@ -4119,7 +4123,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 86bdced0f905..c0657b378434 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -319,7 +319,7 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct net *net,
 	struct dentry *dentry;
 	int fileid_type;
 	int data_left = fh->fh_size/4;
-	int len;
+	int len, ret;
 	__be32 error;
 
 	error = nfserr_badhandle;
@@ -331,8 +331,18 @@ 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_PLAIN or FH_AT_ENCRYPTED: */
+	if (fh->fh_auth_type > 1)
+		return error;
+
+	ret = fh_decrypt(fhp);
+	if (ret) {
+		/* probably should modify this tracepoint */
+		trace_nfsd_set_fh_dentry_badhandle(rqstp, fhp, ret);;
 		return error;
+	}
+
 	len = key_len(fh->fh_fsid_type) / 4;
 	if (len == 0)
 		return error;
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 786f34e72304..e94bf96c3340 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -238,6 +238,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_encrypt(struct svc_fh *);
 
 static __inline__ struct svc_fh *
 fh_copy(struct svc_fh *dst, const struct svc_fh *src)
-- 
2.50.1

Re: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers

[Benjamin Coddington]

On 27 Dec 2025, at 12:04, Benjamin Coddington wrote:
> +	if (encrypting) {
> +		/* encryption */
> +		memcpy(&a_buf[fileid_offset], &fh->fh_raw[fileid_offset],
> +				fh->fh_size - fileid_offset);
> +		memcpy(b_buf, fh->fh_raw, fileid_offset);
> +
> +		/* encrypt the fileid using the fsid as iv: */
> +		memcpy(iv, fh_fsid(fh), min(sizeof(iv), key_len(fh->fh_fsid_type)));

^^ this needs to use min_t(size_t, .., ..) else we compare signed/unsigned..

Thank you -Wall x86_64 build..

> +
> +		/* pad out the fileid to block size */
> +		hash_size = fh_fileid_len(fh);
> +		pad = (bs - (hash_size & (bs - 1))) & (bs - 1);
> +		hash_size += pad;
> +
> +		sg_set_buf(&fh_sgl[0], &a_buf[fileid_offset], hash_size);
> +		sg_mark_end(&fh_sgl[1]);  /* don't need sg1 yet */
> +		sg_init_one(&hash_sg, &b_buf[fileid_offset], hash_size);
> +
> +		skcipher_request_set_crypt(&encfh->req, fh_sgl, &hash_sg, hash_size, iv);
> +		err = crypto_skcipher_encrypt(&encfh->req);
> +		if (err)
> +			goto out;
> +
> +		/* encrypt the fsid + fileid with zero iv, starting with the last
> +		 * block of the hashed fileid */
> +		memset(iv, 0, sizeof(iv));
> +
> +		/* calculate the new padding: */
> +		hash_size += key_len(fh->fh_fsid_type) + 4;
> +		pad = (bs - (hash_size & (bs - 1))) & (bs - 1);
> +		hash_size += pad;
> +
> +		sg_unmark_end(&fh_sgl[1]); /* now we use it */
> +		sg_set_buf(&fh_sgl[0], &b_buf[hash_size-bs], bs);
> +		sg_set_buf(&fh_sgl[1], b_buf, hash_size-bs);
> +		sg_init_one(&hash_sg, a_buf, hash_size);
> +
> +		skcipher_request_set_crypt(&encfh->req, fh_sgl, &hash_sg, hash_size, iv);
> +		err = crypto_skcipher_encrypt(&encfh->req);
> +
> +		if (!err) {
> +			memcpy(&fh->fh_raw[4], a_buf, hash_size);
> +			fh->fh_auth_type = FH_AT_ENCRYPTED;
> +			fh->fh_fileid_type = fh->fh_size; /* we'll use this in decryption */
> +			fh->fh_size = hash_size + 4;
> +		}
> +	} else {
> +		/* decryption */
> +		int fh_size = fh->fh_size - 4;
> +		memcpy(b_buf, &fh->fh_raw[4], fh_size);
> +
> +		/* first, we decode starting with the last hashed block and zero iv */
> +		hash_size = fh_size;
> +		sg_set_buf(&fh_sgl[0], &a_buf[fh_size - bs], bs);
> +		sg_set_buf(&fh_sgl[1], a_buf, fh_size - bs);
> +		sg_init_one(&hash_sg, b_buf, fh_size);
> +
> +		skcipher_request_set_crypt(&encfh->req, &hash_sg, fh_sgl, hash_size, iv);
> +		err = crypto_skcipher_decrypt(&encfh->req);
> +		if (err)
> +			goto out;
> +
> +		/* Now we're dealing with the original fh_size: */
> +		fh_size = fh->fh_fileid_type;
> +
> +		/* a_buf now has the decrypted fsid and header: */
> +		memcpy(fh->fh_raw, a_buf, fileid_offset);
> +
> +		/* now we set the iv to the decrypted fsid value */
> +		memset(iv, 0, sizeof(iv));;
> +		memcpy(iv, &a_buf[4], min(sizeof(iv), key_len(fh->fh_fsid_type)));

^^ ditto..

Ben

Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles

[NeilBrown]

On Sun, 28 Dec 2025, Benjamin Coddington wrote:
> In order to harden kNFSD against various filehandle manipulation techniques
> the following patches implement a method of reversibly encrypting filehandle
> contents.
> 
> Using the kernel's skcipher AES-CBC, filehandles are encrypted by firstly
> hashing the fileid using the fsid as a salt, then using the hashed fileid as
> the first block to finally hash the fsid.
> 
> The first attempts at this used stack-allocated buffers, but I ran into many
> memory alignment problems on my arm64 machine that sent me back to using
> GFP_KERNEL allocations (here's to you /include/linux/scatterlist.h:210).  In
> order to avoid constant allocation/freeing, the buffers are allocated once
> for every knfsd thread.  If anyone has suggestions for reducing the number
> of buffers required and their memcpy() operations, I am all ears.
> 
> Currently the code overloads filehandle's auth_type byte.  This seems
> appropriate for this purpose, but this implementation does not actually
> reject unencrypted filehandles on an export that is giving out encrypted
> ones.  I expect we'll want to tighten this up in a future version.
> 
> Comments and critique welcome.

Can you say more about the threat-model you are trying to address?

I never pursued this idea (despite adding the auth_type byte to the
filehandle) because I couldn't think of a scenario where it made a
useful difference.

If an attacker can inject arbitrary RPC packets into the network in a
way that the server will trust them, then it is very likely to be able
to snoop filehandles and do a similar amount of damage...  though I'm
having trouble remembering that damage that would be?

Thanks,
NeilBrown

Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles

[Benjamin Coddington]

On 27 Dec 2025, at 18:06, NeilBrown wrote:

> On Sun, 28 Dec 2025, Benjamin Coddington wrote:
>> In order to harden kNFSD against various filehandle manipulation techniques
>> the following patches implement a method of reversibly encrypting filehandle
>> contents.
>>
>> Using the kernel's skcipher AES-CBC, filehandles are encrypted by firstly
>> hashing the fileid using the fsid as a salt, then using the hashed fileid as
>> the first block to finally hash the fsid.
>>
>> The first attempts at this used stack-allocated buffers, but I ran into many
>> memory alignment problems on my arm64 machine that sent me back to using
>> GFP_KERNEL allocations (here's to you /include/linux/scatterlist.h:210).  In
>> order to avoid constant allocation/freeing, the buffers are allocated once
>> for every knfsd thread.  If anyone has suggestions for reducing the number
>> of buffers required and their memcpy() operations, I am all ears.
>>
>> Currently the code overloads filehandle's auth_type byte.  This seems
>> appropriate for this purpose, but this implementation does not actually
>> reject unencrypted filehandles on an export that is giving out encrypted
>> ones.  I expect we'll want to tighten this up in a future version.
>>
>> Comments and critique welcome.
>
> Can you say more about the threat-model you are trying to address?
>
> I never pursued this idea (despite adding the auth_type byte to the
> filehandle) because I couldn't think of a scenario where it made a
> useful difference.
>
> If an attacker can inject arbitrary RPC packets into the network in a
> way that the server will trust them, then it is very likely to be able
> to snoop filehandles and do a similar amount of damage...  though I'm
> having trouble remembering that damage that would be?

Filehandles are usually pretty easy to reverse engineer.  Once you've seen a
few, the number of bits you need to manipulate to find new things on the
filesystem is pretty small.  That means that (forget about MITM - though
that is still a real threat) even a trusted client might be able to access
objects outside the export root on the same filesystem.

This problem is further exacerbated when using kNFSD as a DS for a flexfiles
setup - the MDS may be performing access checks for objects that the DS does
not.  Manipulating filehandles to a DS can circumvent those access checks.

I can absolutely add more information on this for subsequent postings.

Ben

Re: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers

[Chuck Lever]

On Sat, Dec 27, 2025, at 12:04 PM, Benjamin Coddington wrote:
> In order to improve the security of knfsd servers, create a method to
> encrypt and decrypt filehandles.
>
> Filehandle encryption begins by checking for an allocated encfh_buf for
> each knfsd thread.  It not yet allocated, nfsd performs JIT alloation and
> proceeds to encrypt or decrypt.
>
> In order to increase entropy, filehandles are encrypted in two passes.  In
> the first pass, the fileid is expanded to the AES block size and encrypted
> with the server's key and a salt from the fsid.  In the second pass, the
> entirety of the filehandle is encrypted starting with the block containing
> the results of the first pass.  Decryption reverses this operation.
>
> This approach ensures that the same fileid values are encrypted differently
> for differing fsid values.  This protects against comparisons between the
> same fileids across different exports that may not be encrypted, which
> could ease the discovery of the server's private key.  Additionally, it
> allows the fsid to be encrypted uniquely for each filehandle.
>
> The filehandle's auth_type is used to indicate that a filehandle has been
> encrypted.
>
> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
> ---
>  fs/nfsd/nfsfh.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/nfsfh.h |  13 ++++
>  2 files changed, 178 insertions(+)
>
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index ed85dd43da18..86bdced0f905 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,170 @@ static inline __be32 check_pseudo_root(struct 
> dentry *dentry,
>  	return nfs_ok;
>  }
> 
> +static int fh_crypto_init(struct svc_rqst *rqstp)
> +{
> +	struct encfh_buf *fh_encfh = (struct encfh_buf *)rqstp->rq_crypto;
> +
> +	/* This knfsd has not allocated buffers and reqest yet: */
> +	if (!fh_encfh) {
> +		struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> +
> +		fh_encfh = kmalloc(sizeof(struct encfh_buf), GFP_KERNEL);
> +		if (!fh_encfh)
> +			return -ENOMEM;
> +
> +		skcipher_request_set_sync_tfm(&fh_encfh->req, nn->encfh_tfm);
> +		rqstp->rq_crypto = fh_encfh;
> +	}
> +	memset(fh_encfh->a_buf, 0, NFS4_FHSIZE);
> +	memset(fh_encfh->b_buf, 0, NFS4_FHSIZE);
> +	return 0;
> +}
> +
> +static int fh_crypto(struct svc_fh *fhp, bool encrypting)
> +{
> +	struct encfh_buf *encfh = (struct encfh_buf *)fhp->fh_rqstp->rq_crypto;
> +	int err, pad, hash_size, fileid_offset;
> +	struct knfsd_fh *fh = &fhp->fh_handle;
> +	struct scatterlist fh_sgl[2];
> +	struct scatterlist hash_sg;
> +	u8 *a_buf = encfh->a_buf;
> +	u8 *b_buf = encfh->b_buf;
> +	u8 iv[16];
> +
> +	/* blocksize */
> +	int bs = crypto_sync_skcipher_blocksize(
> +				crypto_sync_skcipher_reqtfm(&encfh->req));
> +
> +	/* always renew as it gets transformed: */
> +	memset(iv, 0, sizeof(iv));
> +
> +	fileid_offset = fh_fileid_offset(fh);
> +	sg_init_table(fh_sgl, 2);
> +
> +	if (encrypting) {
> +		/* encryption */
> +		memcpy(&a_buf[fileid_offset], &fh->fh_raw[fileid_offset],
> +				fh->fh_size - fileid_offset);
> +		memcpy(b_buf, fh->fh_raw, fileid_offset);
> +
> +		/* encrypt the fileid using the fsid as iv: */
> +		memcpy(iv, fh_fsid(fh), min(sizeof(iv), key_len(fh->fh_fsid_type)));
> +
> +		/* pad out the fileid to block size */
> +		hash_size = fh_fileid_len(fh);
> +		pad = (bs - (hash_size & (bs - 1))) & (bs - 1);
> +		hash_size += pad;
> +
> +		sg_set_buf(&fh_sgl[0], &a_buf[fileid_offset], hash_size);
> +		sg_mark_end(&fh_sgl[1]);  /* don't need sg1 yet */
> +		sg_init_one(&hash_sg, &b_buf[fileid_offset], hash_size);
> +
> +		skcipher_request_set_crypt(&encfh->req, fh_sgl, &hash_sg, hash_size, iv);
> +		err = crypto_skcipher_encrypt(&encfh->req);
> +		if (err)
> +			goto out;
> +
> +		/* encrypt the fsid + fileid with zero iv, starting with the last
> +		 * block of the hashed fileid */
> +		memset(iv, 0, sizeof(iv));
> +
> +		/* calculate the new padding: */
> +		hash_size += key_len(fh->fh_fsid_type) + 4;
> +		pad = (bs - (hash_size & (bs - 1))) & (bs - 1);
> +		hash_size += pad;
> +
> +		sg_unmark_end(&fh_sgl[1]); /* now we use it */
> +		sg_set_buf(&fh_sgl[0], &b_buf[hash_size-bs], bs);
> +		sg_set_buf(&fh_sgl[1], b_buf, hash_size-bs);
> +		sg_init_one(&hash_sg, a_buf, hash_size);
> +
> +		skcipher_request_set_crypt(&encfh->req, fh_sgl, &hash_sg, hash_size, iv);
> +		err = crypto_skcipher_encrypt(&encfh->req);
> +
> +		if (!err) {
> +			memcpy(&fh->fh_raw[4], a_buf, hash_size);
> +			fh->fh_auth_type = FH_AT_ENCRYPTED;
> +			fh->fh_fileid_type = fh->fh_size; /* we'll use this in decryption */
> +			fh->fh_size = hash_size + 4;
> +		}
> +	} else {
> +		/* decryption */
> +		int fh_size = fh->fh_size - 4;
> +		memcpy(b_buf, &fh->fh_raw[4], fh_size);
> +
> +		/* first, we decode starting with the last hashed block and zero iv */
> +		hash_size = fh_size;
> +		sg_set_buf(&fh_sgl[0], &a_buf[fh_size - bs], bs);
> +		sg_set_buf(&fh_sgl[1], a_buf, fh_size - bs);
> +		sg_init_one(&hash_sg, b_buf, fh_size);
> +
> +		skcipher_request_set_crypt(&encfh->req, &hash_sg, fh_sgl, hash_size, iv);
> +		err = crypto_skcipher_decrypt(&encfh->req);
> +		if (err)
> +			goto out;
> +
> +		/* Now we're dealing with the original fh_size: */
> +		fh_size = fh->fh_fileid_type;
> +
> +		/* a_buf now has the decrypted fsid and header: */
> +		memcpy(fh->fh_raw, a_buf, fileid_offset);
> +
> +		/* now we set the iv to the decrypted fsid value */
> +		memset(iv, 0, sizeof(iv));;
> +		memcpy(iv, &a_buf[4], min(sizeof(iv), key_len(fh->fh_fsid_type)));
> +
> +		/* align to block size */
> +		hash_size = fh_size - fileid_offset;
> +		pad = (bs - (hash_size & (bs - 1))) & (bs - 1);
> +		hash_size += pad;
> +
> +		/* decrypt only the fileid: */
> +		sg_set_buf(&fh_sgl[0], &b_buf[fileid_offset], hash_size);
> +		sg_mark_end(&fh_sgl[1]);
> +		sg_init_one(&hash_sg, &a_buf[fileid_offset], hash_size);
> +
> +		skcipher_request_set_crypt(&encfh->req, &hash_sg, fh_sgl, hash_size, iv);
> +		err = crypto_skcipher_decrypt(&encfh->req);
> +
> +		if (!err) {
> +			fh->fh_size = fh_size;
> +			/* copy in the fileid */
> +			memcpy(&fh->fh_raw[fileid_offset], &b_buf[fileid_offset], hash_size);
> +			/* trim the leftover hash padding */
> +			memset(&fh->fh_raw[fh->fh_size], 0, NFS4_FHSIZE - fh->fh_size);
> +		}
> +	}
> +	// add a tracepoint to show the error;
> +	// if decrypting, we want nfserr_badhandle
> +out:
> +	return err;
> +}
> +
> +/* we should never get here without calling fh_init first */
> +int fh_encrypt(struct svc_fh *fhp)
> +{
> +	if (!(fhp->fh_export->ex_flags & NFSEXP_ENCRYPT_FH))
> +		return 0;
> +
> +	if (fh_crypto_init(fhp->fh_rqstp))
> +		return -ENOMEM;
> +
> +	return fh_crypto(fhp, true);
> +}
> +
> +/* Lets try to decrypt, no matter the export setting */
> +static int fh_decrypt(struct svc_fh *fhp)
> +{
> +	if (fhp->fh_handle.fh_auth_type != FH_AT_ENCRYPTED)
> +		return 0;
> +
> +	if (fh_crypto_init(fhp->fh_rqstp))
> +		return -ENOMEM;
> +
> +	return fh_crypto(fhp, false);
> +}
> +
>  /*
>   * Use the given filehandle to look up the corresponding export and
>   * dentry.  On success, the results are used to set fh_export and
> diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> index f29bb09af242..786f34e72304 100644
> --- a/fs/nfsd/nfsfh.h
> +++ b/fs/nfsd/nfsfh.h
> @@ -60,6 +60,9 @@ struct knfsd_fh {
>  #define fh_fsid_type		fh_raw[2]
>  #define fh_fileid_type		fh_raw[3]
> 
> +#define FH_AT_PLAIN		0
> +#define FH_AT_ENCRYPTED	1
> +
>  static inline u32 *fh_fsid(const struct knfsd_fh *fh)
>  {
>  	return (u32 *)&fh->fh_raw[4];
> @@ -284,6 +287,16 @@ 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)
> +{
> +	return fh->fh_size - fh_fileid_offset(fh);
> +}
> +
>  /**
>   * fh_want_write - Get write access to an export
>   * @fhp: File handle of file to be written
> -- 
> 2.50.1

I'd feel more comfortable if the crypto community had a look
to ensure that we're utilizing the APIs in the most efficient
way possible. Adding linux-crypto ...


-- 
Chuck Lever

Re: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers

[kernel test robot]

Hi Benjamin,

kernel test robot noticed the following build errors:

[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on trondmy-nfs/linux-next linus/master v6.19-rc2 next-20251219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Coddington/nfsd-Convert-export-flags-to-use-BIT-macro/20251228-010753
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/0688787cf4764d5add06c8ef1fecc9ea549573d7.1766848778.git.bcodding%40hammerspace.com
patch subject: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers
config: arm-randconfig-004-20251228 (https://download.01.org/0day-ci/archive/20251228/202512281330.lPQZAymu-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251228/202512281330.lPQZAymu-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/202512281330.lPQZAymu-lkp@intel.com/

All errors (new ones prefixed by >>):

   fs/nfsd/nfsfh.c:282:5: warning: no previous prototype for 'fh_encrypt' [-Wmissing-prototypes]
     282 | int fh_encrypt(struct svc_fh *fhp)
         |     ^~~~~~~~~~
   fs/nfsd/nfsfh.c:294:12: warning: 'fh_decrypt' defined but not used [-Wunused-function]
     294 | static int fh_decrypt(struct svc_fh *fhp)
         |            ^~~~~~~~~~
   In file included from include/linux/string.h:386,
                    from include/linux/bitmap.h:13,
                    from include/linux/cpumask.h:11,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/spinlock.h:63,
                    from include/linux/sched.h:37,
                    from include/linux/sunrpc/svcauth_gss.h:12,
                    from fs/nfsd/nfsfh.c:13:
   fs/nfsd/nfsfh.c: In function 'fh_crypto.constprop':
>> include/linux/compiler_types.h:630:45: error: call to '__compiletime_assert_1035' declared with attribute error: min(sizeof(iv), key_len(fh->fh_raw[2])) signedness error
     630 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |                                             ^
   include/linux/fortify-string.h:627:48: note: in definition of macro '__fortify_memcpy_chk'
     627 |         const size_t __fortify_size = (size_t)(size);                   \
         |                                                ^~~~
   fs/nfsd/nfsfh.c:189:17: note: in expansion of macro 'memcpy'
     189 |                 memcpy(iv, fh_fsid(fh), min(sizeof(iv), key_len(fh->fh_fsid_type)));
         |                 ^~~~~~
   include/linux/compiler_types.h:618:9: note: in expansion of macro '__compiletime_assert'
     618 |         __compiletime_assert(condition, msg, prefix, suffix)
         |         ^~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:630:9: note: in expansion of macro '_compiletime_assert'
     630 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:93:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
      93 |         BUILD_BUG_ON_MSG(!__types_ok(ux, uy),           \
         |         ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:98:9: note: in expansion of macro '__careful_cmp_once'
      98 |         __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ^~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:105:25: note: in expansion of macro '__careful_cmp'
     105 | #define min(x, y)       __careful_cmp(min, x, y)
         |                         ^~~~~~~~~~~~~
   fs/nfsd/nfsfh.c:189:41: note: in expansion of macro 'min'
     189 |                 memcpy(iv, fh_fsid(fh), min(sizeof(iv), key_len(fh->fh_fsid_type)));
         |                                         ^~~


vim +/__compiletime_assert_1035 +630 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  616  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  617  #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  618  	__compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  619  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  620  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  621   * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  622   * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  623   * @msg:       a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  624   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  625   * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  626   * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  627   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  628   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  629  #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @630  	_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  631  

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

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

[NeilBrown]

On Sun, 28 Dec 2025, 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 | 36 ++++++++++++++++----------------
>  2 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 5ce9a49e76ba..ad1f3af8d682 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -166,7 +166,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%x 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..aac57180c5c4 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,8 +51,8 @@
>   * 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

Could we make this:

  #define NFSEXP_ALLFLAGS ((BIT(18)-1) - NFSEXP_NOACL)

Or something like that?

Thanks,
NeilBrown

Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles

[NeilBrown]

On Sun, 28 Dec 2025, Benjamin Coddington wrote:
> On 27 Dec 2025, at 18:06, NeilBrown wrote:
> 
> > On Sun, 28 Dec 2025, Benjamin Coddington wrote:
> >> In order to harden kNFSD against various filehandle manipulation techniques
> >> the following patches implement a method of reversibly encrypting filehandle
> >> contents.
> >>
> >> Using the kernel's skcipher AES-CBC, filehandles are encrypted by firstly
> >> hashing the fileid using the fsid as a salt, then using the hashed fileid as
> >> the first block to finally hash the fsid.
> >>
> >> The first attempts at this used stack-allocated buffers, but I ran into many
> >> memory alignment problems on my arm64 machine that sent me back to using
> >> GFP_KERNEL allocations (here's to you /include/linux/scatterlist.h:210).  In
> >> order to avoid constant allocation/freeing, the buffers are allocated once
> >> for every knfsd thread.  If anyone has suggestions for reducing the number
> >> of buffers required and their memcpy() operations, I am all ears.
> >>
> >> Currently the code overloads filehandle's auth_type byte.  This seems
> >> appropriate for this purpose, but this implementation does not actually
> >> reject unencrypted filehandles on an export that is giving out encrypted
> >> ones.  I expect we'll want to tighten this up in a future version.
> >>
> >> Comments and critique welcome.
> >
> > Can you say more about the threat-model you are trying to address?
> >
> > I never pursued this idea (despite adding the auth_type byte to the
> > filehandle) because I couldn't think of a scenario where it made a
> > useful difference.
> >
> > If an attacker can inject arbitrary RPC packets into the network in a
> > way that the server will trust them, then it is very likely to be able
> > to snoop filehandles and do a similar amount of damage...  though I'm
> > having trouble remembering that damage that would be?
> 
> Filehandles are usually pretty easy to reverse engineer.  Once you've seen a
> few, the number of bits you need to manipulate to find new things on the
> filesystem is pretty small.  That means that (forget about MITM - though
> that is still a real threat) even a trusted client might be able to access
> objects outside the export root on the same filesystem.

So this is only seen to be useful when for sub-directory export?

> 
> This problem is further exacerbated when using kNFSD as a DS for a flexfiles
> setup - the MDS may be performing access checks for objects that the DS does
> not.  Manipulating filehandles to a DS can circumvent those access checks.

Not being familiar with flexfiles and don't know what that means -
though I can imagine that pNFS could add extra complications.

> 
> I can absolutely add more information on this for subsequent postings.

That would be helpful - thank.

Next question: why are you encrypting the filehandle?  Is there
something you want to hide?

Normally encryption is for privacy and a MAC (message authentication
code) is used for integrity.  Why are you not simply adding a MAC?

With pure encryption you are relying on the fact that changing (or
synthesising) a filehandle will probably produce a badly formatted
handle  - except that you are only encrypting the bytes after the fsid. 
So there is less redundancy....  Maybe the generation number is enough
to ensure decrypted garbage will never be valid - by it doesn't feel
clean. 

If we are using encryption it would be nice to encrypt the whole fh.
Then you would have a single key for the server rather than
per-export....

As Chuck suggested: getting review from someone with crypto design
expertise would be a good thing.

Thanks,
NeilBrown

Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles

[Rick Macklem]

On Sat, Dec 27, 2025 at 9:49 PM NeilBrown <neilb@ownmail.net> wrote:
>
> On Sun, 28 Dec 2025, Benjamin Coddington wrote:
> > On 27 Dec 2025, at 18:06, NeilBrown wrote:
> >
> > > On Sun, 28 Dec 2025, Benjamin Coddington wrote:
> > >> In order to harden kNFSD against various filehandle manipulation techniques
> > >> the following patches implement a method of reversibly encrypting filehandle
> > >> contents.
> > >>
> > >> Using the kernel's skcipher AES-CBC, filehandles are encrypted by firstly
> > >> hashing the fileid using the fsid as a salt, then using the hashed fileid as
> > >> the first block to finally hash the fsid.
> > >>
> > >> The first attempts at this used stack-allocated buffers, but I ran into many
> > >> memory alignment problems on my arm64 machine that sent me back to using
> > >> GFP_KERNEL allocations (here's to you /include/linux/scatterlist.h:210).  In
> > >> order to avoid constant allocation/freeing, the buffers are allocated once
> > >> for every knfsd thread.  If anyone has suggestions for reducing the number
> > >> of buffers required and their memcpy() operations, I am all ears.
> > >>
> > >> Currently the code overloads filehandle's auth_type byte.  This seems
> > >> appropriate for this purpose, but this implementation does not actually
> > >> reject unencrypted filehandles on an export that is giving out encrypted
> > >> ones.  I expect we'll want to tighten this up in a future version.
> > >>
> > >> Comments and critique welcome.
> > >
> > > Can you say more about the threat-model you are trying to address?
> > >
> > > I never pursued this idea (despite adding the auth_type byte to the
> > > filehandle) because I couldn't think of a scenario where it made a
> > > useful difference.
> > >
> > > If an attacker can inject arbitrary RPC packets into the network in a
> > > way that the server will trust them, then it is very likely to be able
> > > to snoop filehandles and do a similar amount of damage...  though I'm
> > > having trouble remembering that damage that would be?
> >
> > Filehandles are usually pretty easy to reverse engineer.  Once you've seen a
> > few, the number of bits you need to manipulate to find new things on the
> > filesystem is pretty small.  That means that (forget about MITM - though
> > that is still a real threat) even a trusted client might be able to access
> > objects outside the export root on the same filesystem.
>
> So this is only seen to be useful when for sub-directory export?
If this is the case, I'll ask..

If a malicious entity can perform RPCs on the server with faked file
handles, what stops that malicious entity from doing a
LOOKUP ".."/LOOKUPP at the root of the subdirectory mount
to get out of the subtree?

>
> >
> > This problem is further exacerbated when using kNFSD as a DS for a flexfiles
> > setup - the MDS may be performing access checks for objects that the DS does
> > not.  Manipulating filehandles to a DS can circumvent those access checks.
I'm not sure why a DS is more vulnerable that any other NFSv3 server.
(Either the client can be "trusted" to access the DS or it is not. That's what
exports do.)

An additional concern I'll mention (not knowing how Linux handles this)
is that file handles (NFSv3 and NFSv4 persistent) are expected to be T-stable,
which implies that the key cannot change for a very long time, including
after a server reboot (or even a server reboot after a software upgrade).

rick

>
> Not being familiar with flexfiles and don't know what that means -
> though I can imagine that pNFS could add extra complications.
>
> >
> > I can absolutely add more information on this for subsequent postings.
>
> That would be helpful - thank.
>
> Next question: why are you encrypting the filehandle?  Is there
> something you want to hide?
>
> Normally encryption is for privacy and a MAC (message authentication
> code) is used for integrity.  Why are you not simply adding a MAC?
>
> With pure encryption you are relying on the fact that changing (or
> synthesising) a filehandle will probably produce a badly formatted
> handle  - except that you are only encrypting the bytes after the fsid.
> So there is less redundancy....  Maybe the generation number is enough
> to ensure decrypted garbage will never be valid - by it doesn't feel
> clean.
>
> If we are using encryption it would be nice to encrypt the whole fh.
> Then you would have a single key for the server rather than
> per-export....
>
> As Chuck suggested: getting review from someone with crypto design
> expertise would be a good thing.
>
> Thanks,
> NeilBrown
>

Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles

[Chuck Lever]

Hi Ben -

Thanks for getting this started.

I tried to pull in the kernel patches as follows:

cel@morisot:~/src/linux/for-korg$ b4 am https://lore.kernel.org/linux-nfs/176690096534.16766.12693781635285919555@noble.neil.brown.name/T/#u
Grabbing thread from lore.kernel.org/all/176690096534.16766.12693781635285919555@noble.neil.brown.name/t.mbox.gz
Analyzing 19 messages in the thread
WARNING: duplicate messages found at index 2
   Subject 1: exportfs: Add support for export option encrypt_fh
   Subject 2: nfsd: Add a symmetric-key cipher for encrypted filehandles
  2 is not a reply... assume additional patch
WARNING: duplicate messages found at index 1
   Subject 1: nfsdctl: Add support for passing encrypted filehandle key
   Subject 2: nfsd: Convert export flags to use BIT() macro
  2 is not a reply... assume additional patch
Analyzing 0 code-review messages
Checking attestation on all messages, may take a moment...
---
  ✓ [PATCH v1 1/2] nfsdctl: Add support for passing encrypted filehandle key
    ✓ Signed: DKIM/hammerspace.com
  ✓ [PATCH v1 1/7] nfsd: Convert export flags to use BIT() macro
    ✓ Signed: DKIM/hammerspace.com
  ✓ [PATCH v1 2/7] nfsd: Add a symmetric-key cipher for encrypted filehandles
    ✓ Signed: DKIM/hammerspace.com
  ✓ [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf
    ✓ Signed: DKIM/hammerspace.com
  ✓ [PATCH v1 5/7] NFSD/export: Add encrypt_fh export option
    ✓ Signed: DKIM/hammerspace.com
  ✓ [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers
    ✓ Signed: DKIM/hammerspace.com
  ✓ [PATCH v1 7/7] NFSD: Enable filehandle encryption
    ✓ Signed: DKIM/hammerspace.com
  ERROR: missing [8/2]!
  ERROR: missing [9/2]!


Whatever you did to post these seems to badly confuse b4. I recommend
posting nfs-utils and kernel patches as entirely separate threads.

Also, the cover letters for both series should mention the base commit
for your series. Typically for NFSD patches, base your series on the cel
nfsd-testing branch. But any base is workable as long as you mention the
base commit in the cover letter.

More below.


On Sat, Dec 27, 2025, at 12:04 PM, Benjamin Coddington wrote:
> In order to harden kNFSD against various filehandle manipulation techniques
> the following patches implement a method of reversibly encrypting filehandle
> contents.

"various filehandle manipulation techniques" is pretty vague. Reviewers
will need to know which specific attack vectors you are guarding against
in order to evaluate whether your proposal addresses those attacks.

Also, next posting should copy both linux-crypto and probably linux-fsdevel
too, as the FUSE and exportfs folks hang out there and might be interested
in this work. There are other consumers of filehandles in the kernel.


> Using the kernel's skcipher AES-CBC, filehandles are encrypted by firstly
> hashing the fileid using the fsid as a salt, then using the hashed fileid as
> the first block to finally hash the fsid.

Is the FSID possibly exposed on the wire via NFSv3 FSINFO and certain
NFSv4 GETATTR operations? It's not clear to me from this description
whether these values are otherwise unknown to network systems.

Can you elaborate on why you selected AES-CBC? An enumeration of the
cryptography requirements would be great to see, either in the cover
letter or as a new file under Documentation/fs/nfs/ .

The use of a symmetric cipher is surprising. I thought there was going
to be a cache of file handles so that file handle decryption operations
for each I/O would not be necessary.


> The first attempts at this used stack-allocated buffers, but I ran into many
> memory alignment problems on my arm64 machine that sent me back to using
> GFP_KERNEL allocations (here's to you /include/linux/scatterlist.h:210).  In
> order to avoid constant allocation/freeing, the buffers are allocated once
> for every knfsd thread.  If anyone has suggestions for reducing the number
> of buffers required and their memcpy() operations, I am all ears.

The required use of dynamically allocated buffers is a well-known
constraint of the crypto API. That would be another reason to consider
not using one of the kernel's crypto APIs.

I'd rather avoid hanging anything NFSD-related off of svc_rqst, which
is really an RPC layer object. I know we still have some NFSD-specific
fields in there, but those are really technical debt.


> Currently the code overloads filehandle's auth_type byte.  This seems
> appropriate for this purpose, but this implementation does not actually
> reject unencrypted filehandles on an export that is giving out encrypted
> ones.  I expect we'll want to tighten this up in a future version.

I recall one purported reason to encrypt file handles on the wire is to
mitigate file handle guessing attacks... so operations on an export that
uses encrypted file handles really should return NFSERR_STALE when a
non-encrypted FH is presented, from day-zero, unless I misunderstand.

Can you elaborate more on the size of an encrypted file handle? I assume
these are fixed in size. An NFSv3 on-the-wire file handle can be up to
64 octets, but NFSv4 file handles can be up to 128. Are both going to be
encrypted to the same size?

Can the cover letter or other documentation include a bit diagram that
graphically shows the proposed layout of an encrypted file handle on the
wire?


> Comments and critique welcome.
>
> Benjamin Coddington (7):
>   nfsd: Convert export flags to use BIT() macro
>   nfsd: Add a symmetric-key cipher for encrypted filehandles
>   nfsd/sunrpc: add per-thread crypto context pointer
>   NFSD: Add a per-knfsd reusable encfh_buf
>   NFSD/export: Add encrypt_fh export option
>   NFSD: Add filehandle crypto functions and helpers
>   NFSD: Enable filehandle encryption
>
>  Documentation/netlink/specs/nfsd.yaml |  12 ++
>  fs/nfsd/export.c                      |   7 +-
>  fs/nfsd/localio.c                     |   2 +-
>  fs/nfsd/lockd.c                       |   2 +-
>  fs/nfsd/netlink.c                     |  15 +++
>  fs/nfsd/netlink.h                     |   1 +
>  fs/nfsd/netns.h                       |   1 +
>  fs/nfsd/nfs3proc.c                    |  10 +-
>  fs/nfsd/nfs3xdr.c                     |  14 +-
>  fs/nfsd/nfs4proc.c                    |  10 +-
>  fs/nfsd/nfs4xdr.c                     |  14 +-
>  fs/nfsd/nfsctl.c                      |  40 +++++-
>  fs/nfsd/nfsfh.c                       | 179 +++++++++++++++++++++++++-
>  fs/nfsd/nfsfh.h                       |  26 +++-
>  fs/nfsd/nfsproc.c                     |   8 +-
>  fs/nfsd/trace.h                       |  19 +++
>  include/linux/sunrpc/svc.h            |  12 +-
>  include/uapi/linux/nfsd/export.h      |  36 +++---
>  include/uapi/linux/nfsd_netlink.h     |   2 +
>  net/sunrpc/svc.c                      |   1 +
>  20 files changed, 356 insertions(+), 55 deletions(-)
>
> -- 
> 2.50.1

-- 
Chuck Lever

Re: [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf

[kernel test robot]

Hi Benjamin,

kernel test robot noticed the following build errors:

[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on trondmy-nfs/linux-next linus/master v6.19-rc2 next-20251219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Coddington/nfsd-Convert-export-flags-to-use-BIT-macro/20251228-010753
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/5b7bf494b1ec816111ab34416dcde85c0bc01a0a.1766848778.git.bcodding%40hammerspace.com
patch subject: [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf
config: um-allyesconfig (https://download.01.org/0day-ci/archive/20251229/202512290151.LBKUBvUx-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251229/202512290151.LBKUBvUx-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/202512290151.LBKUBvUx-lkp@intel.com/

All errors (new ones prefixed by >>):

   fs/nfsd/nfsxdr.c: In function 'svcxdr_decode_fhandle':
>> fs/nfsd/nfsxdr.c:66:9: error: too few arguments to function 'fh_init'
      66 |         fh_init(fhp, NFS_FHSIZE);
         |         ^~~~~~~
   In file included from fs/nfsd/vfs.h:11,
                    from fs/nfsd/nfsxdr.c:8:
   fs/nfsd/nfsfh.h:256:1: note: declared here
     256 | fh_init(struct svc_fh *fhp, int maxsize, struct svc_rqst *rqstp)
         | ^~~~~~~


vim +/fh_init +66 fs/nfsd/nfsxdr.c

a887eaed2a96475 Chuck Lever 2020-10-23  48  
635a45d34706400 Chuck Lever 2020-11-17  49  /**
635a45d34706400 Chuck Lever 2020-11-17  50   * svcxdr_decode_fhandle - Decode an NFSv2 file handle
635a45d34706400 Chuck Lever 2020-11-17  51   * @xdr: XDR stream positioned at an encoded NFSv2 FH
635a45d34706400 Chuck Lever 2020-11-17  52   * @fhp: OUT: filled-in server file handle
635a45d34706400 Chuck Lever 2020-11-17  53   *
635a45d34706400 Chuck Lever 2020-11-17  54   * Return values:
635a45d34706400 Chuck Lever 2020-11-17  55   *  %false: The encoded file handle was not valid
635a45d34706400 Chuck Lever 2020-11-17  56   *  %true: @fhp has been initialized
635a45d34706400 Chuck Lever 2020-11-17  57   */
635a45d34706400 Chuck Lever 2020-11-17  58  bool
ebcd8e8b28535b6 Chuck Lever 2020-10-21  59  svcxdr_decode_fhandle(struct xdr_stream *xdr, struct svc_fh *fhp)
ebcd8e8b28535b6 Chuck Lever 2020-10-21  60  {
ebcd8e8b28535b6 Chuck Lever 2020-10-21  61  	__be32 *p;
ebcd8e8b28535b6 Chuck Lever 2020-10-21  62  
ebcd8e8b28535b6 Chuck Lever 2020-10-21  63  	p = xdr_inline_decode(xdr, NFS_FHSIZE);
ebcd8e8b28535b6 Chuck Lever 2020-10-21  64  	if (!p)
ebcd8e8b28535b6 Chuck Lever 2020-10-21  65  		return false;
ebcd8e8b28535b6 Chuck Lever 2020-10-21 @66  	fh_init(fhp, NFS_FHSIZE);
d8b26071e65e80a NeilBrown   2021-09-02  67  	memcpy(&fhp->fh_handle.fh_raw, p, NFS_FHSIZE);
ebcd8e8b28535b6 Chuck Lever 2020-10-21  68  	fhp->fh_handle.fh_size = NFS_FHSIZE;
ebcd8e8b28535b6 Chuck Lever 2020-10-21  69  
ebcd8e8b28535b6 Chuck Lever 2020-10-21  70  	return true;
ebcd8e8b28535b6 Chuck Lever 2020-10-21  71  }
ebcd8e8b28535b6 Chuck Lever 2020-10-21  72  

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

Re: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers

[Eric Biggers]

On Sat, Dec 27, 2025 at 08:34:18PM -0500, Chuck Lever wrote:
> I'd feel more comfortable if the crypto community had a look
> to ensure that we're utilizing the APIs in the most efficient
> way possible. Adding linux-crypto ...

Many crypto algorithms (especially hash algorithms and MACs) have
library APIs now.  They're much easier to use than the traditional APIs.

But it's too soon to be discussing which API to use.  Looking at the
whole series in lore, there doesn't seem to be any explanation of what
problem this series is trying to solve and how cryptography is being
used to solve that problem.

The choice of AES-CBC encryption is unusual.  It's unlikely to be an
appropriate choice for the problem.

I suspect you're really looking to protect the authenticity of the file
handles, not their confidentiality; i.e., you'd like to prevent clients
from constructing their own file handles.  In that case you'd probably
need a MAC, such as SipHash or HMAC-SHA256.  This would be similar to
the kernel's existing implementations of TCP SYN and SCTP cookies: the
system sends out cookies that encode some information, and it uses a MAC
to verify that any received cookie is a previously sent one.

But that's just what I suspect.  I can't know for sure since this series
doesn't provide any context about what it's trying to achieve.

- Eric

Re: [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf

[kernel test robot]

Hi Benjamin,

kernel test robot noticed the following build errors:

[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on trondmy-nfs/linux-next linus/master v6.19-rc2 next-20251219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Coddington/nfsd-Convert-export-flags-to-use-BIT-macro/20251228-010753
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/5b7bf494b1ec816111ab34416dcde85c0bc01a0a.1766848778.git.bcodding%40hammerspace.com
patch subject: [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20251229/202512290829.D6hFwVeD-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251229/202512290829.D6hFwVeD-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/202512290829.D6hFwVeD-lkp@intel.com/

All errors (new ones prefixed by >>):

   fs/nfsd/nfsxdr.c: In function 'svcxdr_decode_fhandle':
>> fs/nfsd/nfsxdr.c:66:9: error: too few arguments to function 'fh_init'; expected 3, have 2
      66 |         fh_init(fhp, NFS_FHSIZE);
         |         ^~~~~~~
   In file included from fs/nfsd/vfs.h:11,
                    from fs/nfsd/nfsxdr.c:8:
   fs/nfsd/nfsfh.h:256:1: note: declared here
     256 | fh_init(struct svc_fh *fhp, int maxsize, struct svc_rqst *rqstp)
         | ^~~~~~~


vim +/fh_init +66 fs/nfsd/nfsxdr.c

a887eaed2a9647 Chuck Lever 2020-10-23  48  
635a45d3470640 Chuck Lever 2020-11-17  49  /**
635a45d3470640 Chuck Lever 2020-11-17  50   * svcxdr_decode_fhandle - Decode an NFSv2 file handle
635a45d3470640 Chuck Lever 2020-11-17  51   * @xdr: XDR stream positioned at an encoded NFSv2 FH
635a45d3470640 Chuck Lever 2020-11-17  52   * @fhp: OUT: filled-in server file handle
635a45d3470640 Chuck Lever 2020-11-17  53   *
635a45d3470640 Chuck Lever 2020-11-17  54   * Return values:
635a45d3470640 Chuck Lever 2020-11-17  55   *  %false: The encoded file handle was not valid
635a45d3470640 Chuck Lever 2020-11-17  56   *  %true: @fhp has been initialized
635a45d3470640 Chuck Lever 2020-11-17  57   */
635a45d3470640 Chuck Lever 2020-11-17  58  bool
ebcd8e8b28535b Chuck Lever 2020-10-21  59  svcxdr_decode_fhandle(struct xdr_stream *xdr, struct svc_fh *fhp)
ebcd8e8b28535b Chuck Lever 2020-10-21  60  {
ebcd8e8b28535b Chuck Lever 2020-10-21  61  	__be32 *p;
ebcd8e8b28535b Chuck Lever 2020-10-21  62  
ebcd8e8b28535b Chuck Lever 2020-10-21  63  	p = xdr_inline_decode(xdr, NFS_FHSIZE);
ebcd8e8b28535b Chuck Lever 2020-10-21  64  	if (!p)
ebcd8e8b28535b Chuck Lever 2020-10-21  65  		return false;
ebcd8e8b28535b Chuck Lever 2020-10-21 @66  	fh_init(fhp, NFS_FHSIZE);
d8b26071e65e80 NeilBrown   2021-09-02  67  	memcpy(&fhp->fh_handle.fh_raw, p, NFS_FHSIZE);
ebcd8e8b28535b Chuck Lever 2020-10-21  68  	fhp->fh_handle.fh_size = NFS_FHSIZE;
ebcd8e8b28535b Chuck Lever 2020-10-21  69  
ebcd8e8b28535b Chuck Lever 2020-10-21  70  	return true;
ebcd8e8b28535b Chuck Lever 2020-10-21  71  }
ebcd8e8b28535b Chuck Lever 2020-10-21  72  

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

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

[Benjamin Coddington]

On 28 Dec 2025, at 0:33, NeilBrown wrote:

> On Sun, 28 Dec 2025, Benjamin Coddington wrote:
>>
...
>>  /* All flags that we claim to support.  (Note we don't support NOACL.) */
>>  #define NFSEXP_ALLFLAGS		0x3FEFF
>
> Could we make this:
>
>   #define NFSEXP_ALLFLAGS ((BIT(18)-1) - NFSEXP_NOACL)
>
> Or something like that?

Sure can, will do.

Ben

Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles

[Benjamin Coddington]

Hi Neil, hi Rick,

I'll try to answer both your responses here:

On 28 Dec 2025, at 12:05, Rick Macklem wrote:
> On Sat, Dec 27, 2025 at 9:49 PM NeilBrown <neilb@ownmail.net> wrote:
>> On Sun, 28 Dec 2025, Benjamin Coddington wrote:
>>> On 27 Dec 2025, at 18:06, NeilBrown wrote:
>>>
>>>> On Sun, 28 Dec 2025, Benjamin Coddington wrote:
...

>>> Filehandles are usually pretty easy to reverse engineer.  Once you've seen a
>>> few, the number of bits you need to manipulate to find new things on the
>>> filesystem is pretty small.  That means that (forget about MITM - though
>>> that is still a real threat) even a trusted client might be able to access
>>> objects outside the export root on the same filesystem.
>>
>> So this is only seen to be useful when for sub-directory export?

Well, pretty much yes - let me explain more..

> If this is the case, I'll ask..
>
> If a malicious entity can perform RPCs on the server with faked file
> handles, what stops that malicious entity from doing a
> LOOKUP ".."/LOOKUPP at the root of the subdirectory mount
> to get out of the subtree?

I am targeting a very specific problem, but I've been a little too general
in my cover letter explaining how this work benefits everyone.  That's my
mistake - you guys are too sharp to let it go by.  :)

In a flexfiles setup with a knfsd v3 DS, the MDS can give filehandles to a
client in its ff_data_server4 that the client can't normally discover on its
own because it cannot walk the tree to those files.  The tree will have a
directory with search-only perms: rwx--x--x and root ownership, and root is
squashed for that client.  Files that are linked below this directory can't
be looked up by the client while the MDS (by not having root squashed) can
look them up and selectively give out filehandles for them.

In this setup, the MDS can have control over which clients access which
files on the DS.

Exposing information about the fsid and fileid within the filehandles
themselves and then allowing clients to construct their own acceptable
filehandles circumvents this arrangement.

I do hope (and not!) that you experts can think of another way this
arrangement can be bypassed.

>>> This problem is further exacerbated when using kNFSD as a DS for a flexfiles
>>> setup - the MDS may be performing access checks for objects that the DS does
>>> not.  Manipulating filehandles to a DS can circumvent those access checks.
> I'm not sure why a DS is more vulnerable that any other NFSv3 server.
> (Either the client can be "trusted" to access the DS or it is not. That's what
> exports do.)
>
> An additional concern I'll mention (not knowing how Linux handles this)
> is that file handles (NFSv3 and NFSv4 persistent) are expected to be T-stable,
> which implies that the key cannot change for a very long time, including
> after a server reboot (or even a server reboot after a software upgrade).

Yes indeed - users of this feature will either need to know or have the
unfortunate discovery that key changes will be disastrous.  One important
current commission is documentation about the dangers of arbitrary key
changes.

That being said, it should be possible to create a future feature to allow
the server to switch modes on key change - instead of tightly enforcing one
key, you could arrange to have a list of keys and retire old ones once
clients have stopped using filehandles for them.

> rick

Rick, thanks for reading and thinking about this work.
(more for Neil below):

>> Not being familiar with flexfiles and don't know what that means -
>> though I can imagine that pNFS could add extra complications.

I hope its slightly clearer from the above explanation.

>>>
>>> I can absolutely add more information on this for subsequent postings.
>>
>> That would be helpful - thank.
>>
>> Next question: why are you encrypting the filehandle?  Is there
>> something you want to hide?

Yes!   We want to hide the method the underlying filesystem uses to
identify its inodes, so that it can't be used to identify inodes the client
might not normally be allowed to access through the arrangement I described
above.

>> Normally encryption is for privacy and a MAC (message authentication
>> code) is used for integrity.  Why are you not simply adding a MAC?

Good question - I'll have to think more about why a MAC wouldn't do the job.
So far, I've been thinking about this as I want to give clients an absolute
minimum amount of information about the filesystem on the DS.  Less is more
here - but I can see how a MAC could do the same job and possibly be less
work for the server.

>> With pure encryption you are relying on the fact that changing (or
>> synthesising) a filehandle will probably produce a badly formatted
>> handle  - except that you are only encrypting the bytes after the fsid.
>> So there is less redundancy....  Maybe the generation number is enough
>> to ensure decrypted garbage will never be valid - by it doesn't feel
>> clean.

I agree - that's why..

>> If we are using encryption it would be nice to encrypt the whole fh.
>> Then you would have a single key for the server rather than
>> per-export....

..we do encrypt the whole filehandle here, we just start by hashing the
fileid first because it creates a better hash of the fsid.  If we do AES-CBC
on the filehandle in order (fsid then fileid), then first 16 bytes for each
fsid are identical.  Depending, of course, on fsid size - as you well know
both fsid and fileid can be different sizes - so the encryption uses the
/last/ 16 byte block of the filehandle for the first hash and then uses that
(hashed again) as first block to re-hash the whole filehandle.

>> As Chuck suggested: getting review from someone with crypto design
>> expertise would be a good thing.

Yes - Eric Biggers has responded, I think I need to explain the use-case
more specifically to him and I hope he has good suggestions about the best
way.

>> Thanks,
>> NeilBrown

Thanks very much for looking at this Neil,

Ben

Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles

[Benjamin Coddington]

On 28 Dec 2025, at 12:09, Chuck Lever wrote:

> Hi Ben -
>
> Thanks for getting this started.

Hi Chuck,

Thanks for all the advice here - I'll do my best to fix things up in the
next version, and I'll respond to a few things inline here:

>
> I tried to pull in the kernel patches as follows:
>
> cel@morisot:~/src/linux/for-korg$ b4 am https://lore.kernel.org/linux-nfs/176690096534.16766.12693781635285919555@noble.neil.brown.name/T/#u
> Grabbing thread from lore.kernel.org/all/176690096534.16766.12693781635285919555@noble.neil.brown.name/t.mbox.gz
> Analyzing 19 messages in the thread
> WARNING: duplicate messages found at index 2
>    Subject 1: exportfs: Add support for export option encrypt_fh
>    Subject 2: nfsd: Add a symmetric-key cipher for encrypted filehandles
>   2 is not a reply... assume additional patch
> WARNING: duplicate messages found at index 1
>    Subject 1: nfsdctl: Add support for passing encrypted filehandle key
>    Subject 2: nfsd: Convert export flags to use BIT() macro
>   2 is not a reply... assume additional patch
> Analyzing 0 code-review messages
> Checking attestation on all messages, may take a moment...
> ---
>   ✓ [PATCH v1 1/2] nfsdctl: Add support for passing encrypted filehandle key
>     ✓ Signed: DKIM/hammerspace.com
>   ✓ [PATCH v1 1/7] nfsd: Convert export flags to use BIT() macro
>     ✓ Signed: DKIM/hammerspace.com
>   ✓ [PATCH v1 2/7] nfsd: Add a symmetric-key cipher for encrypted filehandles
>     ✓ Signed: DKIM/hammerspace.com
>   ✓ [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf
>     ✓ Signed: DKIM/hammerspace.com
>   ✓ [PATCH v1 5/7] NFSD/export: Add encrypt_fh export option
>     ✓ Signed: DKIM/hammerspace.com
>   ✓ [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers
>     ✓ Signed: DKIM/hammerspace.com
>   ✓ [PATCH v1 7/7] NFSD: Enable filehandle encryption
>     ✓ Signed: DKIM/hammerspace.com
>   ERROR: missing [8/2]!
>   ERROR: missing [9/2]!
>
>
> Whatever you did to post these seems to badly confuse b4. I recommend
> posting nfs-utils and kernel patches as entirely separate threads.

I only told git-send-email to add "In-Reply-To" and "References" headers to
the first message.  I guess b4 uses those headers to climb up the email chain
rather than pay attention to the "[PATCH vn n/n]" subjects.

> Also, the cover letters for both series should mention the base commit
> for your series. Typically for NFSD patches, base your series on the cel
> nfsd-testing branch. But any base is workable as long as you mention the
> base commit in the cover letter.

Oh yeah - I will do so next time.  The kernel thread is on v6.19-rc1
(8f0b4cce4481), and nfs-utils is on version 2.8.4 (612e407c46b8).

> More below.
>
>
> On Sat, Dec 27, 2025, at 12:04 PM, Benjamin Coddington wrote:
>> In order to harden kNFSD against various filehandle manipulation techniques
>> the following patches implement a method of reversibly encrypting filehandle
>> contents.
>
> "various filehandle manipulation techniques" is pretty vague. Reviewers
> will need to know which specific attack vectors you are guarding against
> in order to evaluate whether your proposal addresses those attacks.
>
> Also, next posting should copy both linux-crypto and probably linux-fsdevel
> too, as the FUSE and exportfs folks hang out there and might be interested
> in this work. There are other consumers of filehandles in the kernel.

Roger, I can do, though I think the chance this provides value to them is
small.

>> Using the kernel's skcipher AES-CBC, filehandles are encrypted by firstly
>> hashing the fileid using the fsid as a salt, then using the hashed fileid as
>> the first block to finally hash the fsid.
>
> Is the FSID possibly exposed on the wire via NFSv3 FSINFO and certain
> NFSv4 GETATTR operations? It's not clear to me from this description
> whether these values are otherwise unknown to network systems.

Yes, the FSID and inode numbers for file can absolutely still be derived for a
given file.

> Can you elaborate on why you selected AES-CBC? An enumeration of the
> cryptography requirements would be great to see, either in the cover
> letter or as a new file under Documentation/fs/nfs/ .

I chose AES because many CPUs have native instructions for them and I wanted
to minimize the performance impact.  I chose CBC because I know that most
filehandles will fit into just a couple 16-byte blocks, and I can use the
standard way that filehandles are composed to arrange to have the most
entropy for a given complete filehandle.  By having each block depend on the
hash output of the previous block, a complete filehandle can be have a
unique hashed result by starting with the fileid.  The actual implementation
is a little more nuanced, but that was my original thinking when I chose the
CBC method.

> The use of a symmetric cipher is surprising. I thought there was going
> to be a cache of file handles so that file handle decryption operations
> for each I/O would not be necessary.

Oh, I never thought about this - but it sounds like it would have all the
problems that cache invalidation does today.  I can't imagine having to make
the server persistenly store that cache or figure out how to trim it.

>> The first attempts at this used stack-allocated buffers, but I ran into many
>> memory alignment problems on my arm64 machine that sent me back to using
>> GFP_KERNEL allocations (here's to you /include/linux/scatterlist.h:210).  In
>> order to avoid constant allocation/freeing, the buffers are allocated once
>> for every knfsd thread.  If anyone has suggestions for reducing the number
>> of buffers required and their memcpy() operations, I am all ears.
>
> The required use of dynamically allocated buffers is a well-known
> constraint of the crypto API. That would be another reason to consider
> not using one of the kernel's crypto APIs.

I'm not sure that we'd benefit spinning our own thing.. I'm much more
comfortable using the crypto API.  I'm pretty sure you'd agree, so I think
I'm not understanding you here.

> I'd rather avoid hanging anything NFSD-related off of svc_rqst, which
> is really an RPC layer object. I know we still have some NFSD-specific
> fields in there, but those are really technical debt.

Doh, ok - good to know.  How would you recommend I approach creating
per-thread objects?

>> Currently the code overloads filehandle's auth_type byte.  This seems
>> appropriate for this purpose, but this implementation does not actually
>> reject unencrypted filehandles on an export that is giving out encrypted
>> ones.  I expect we'll want to tighten this up in a future version.
>
> I recall one purported reason to encrypt file handles on the wire is to
> mitigate file handle guessing attacks... so operations on an export that
> uses encrypted file handles really should return NFSERR_STALE when a
> non-encrypted FH is presented, from day-zero, unless I misunderstand.

Yes I agree completely.  What I didn't want to happen is for someone to just
"kick tyres" on this and break an existing setup, there's some missing
necessary BIG WARNINGS that will exist in future versions.  I also think
that the community can discuss what those warnings should look like.

> Can you elaborate more on the size of an encrypted file handle? I assume
> these are fixed in size. An NFSv3 on-the-wire file handle can be up to
> 64 octets, but NFSv4 file handles can be up to 128. Are both going to be
> encrypted to the same size?

No.  As you know, filehandles can be variable based on the fsid len and the
fileid len.  Also, AES-CBC wants 16-byte boundaries, so currently some
padding gets added in encryption.  I've tested max/max, max/min, min/min,
and min/max for each.  Depending on size for each, the filehandles typically
grow a bit from their original sizes.

The NFS3_FH size might be a problem - I admit I haven't made sure we don't
blow that 64 bytes for the larger cases.  I will do that.  Now that I think
about it, it might be possible to avoid having to pad out the blocks.. more
investigation is needed.

One thing I can do is produce a table for the resulting crypted FH sizes for
each given fsid/fileid length..

> Can the cover letter or other documentation include a bit diagram that
> graphically shows the proposed layout of an encrypted file handle on the
> wire?

Haha - yes, but the result is going to look pretty basic: something like:

16 bytes block 1 | 16 bytes block 2 | ...

Thanks for the look and your time to comment Chuck.

Ben

Re: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers

[Benjamin Coddington]

On 28 Dec 2025, at 15:45, Eric Biggers wrote:

> On Sat, Dec 27, 2025 at 08:34:18PM -0500, Chuck Lever wrote:
>> I'd feel more comfortable if the crypto community had a look
>> to ensure that we're utilizing the APIs in the most efficient
>> way possible. Adding linux-crypto ...
>
> Many crypto algorithms (especially hash algorithms and MACs) have
> library APIs now.  They're much easier to use than the traditional APIs.
>
> But it's too soon to be discussing which API to use.  Looking at the
> whole series in lore, there doesn't seem to be any explanation of what
> problem this series is trying to solve and how cryptography is being
> used to solve that problem.

Hi Eric, thanks for the look.  Agree I could have done a better job
explaining the problem.

I've done a bit more explaining here:
https://lore.kernel.org/linux-nfs/706F9EDB-D98E-41D9-92DD-5172A34A278F@hammerspace.com/

.. summarized:

    I am targeting a very specific problem, but I've been a little too general
    in my cover letter explaining how this work benefits everyone.  That's my
    mistake - you guys are too sharp to let it go by.  :)

    In a flexfiles setup with a knfsd v3 DS, the MDS can give filehandles to a
    client in its ff_data_server4 that the client can't normally discover on its
    own because it cannot walk the tree to those files.  The tree will have a
    directory with search-only perms: rwx--x--x and root ownership, and root is
    squashed for that client.  Files that are linked below this directory can't
    be looked up by the client while the MDS (by not having root squashed) can
    look them up and selectively give out filehandles for them.

    In this setup, the MDS can have control over which clients access which
    files on the DS.

    Exposing information about the fsid and fileid within the filehandles
    themselves and then allowing clients to construct their own acceptable
    filehandles circumvents this arrangement.


> The choice of AES-CBC encryption is unusual.  It's unlikely to be an
> appropriate choice for the problem.

Good to know - I'll do my best to help you make a better recommendation.  In
a different thread responding to Chuck Lever:
https://lore.kernel.org/linux-nfs/2DB9B1FF-B740-48E4-9528-630D10E21613@hammerspace.com/
.. I wrote:

    > Can you elaborate on why you selected AES-CBC? An enumeration of the
    > cryptography requirements would be great to see, either in the cover
    > letter or as a new file under Documentation/fs/nfs/ .

    I chose AES because many CPUs have native instructions for them and I wanted
    to minimize the performance impact.  I chose CBC because I know that most
    filehandles will fit into just a couple 16-byte blocks, and I can use the
    standard way that filehandles are composed to arrange to have the most
    entropy for a given complete filehandle.  By having each block depend on the
    hash output of the previous block, a complete filehandle can be have a
    unique hashed result by starting with the fileid.  The actual implementation
    is a little more nuanced, but that was my original thinking when I chose the
    CBC method.

> I suspect you're really looking to protect the authenticity of the file
> handles, not their confidentiality; i.e., you'd like to prevent clients
> from constructing their own file handles.  In that case you'd probably
> need a MAC, such as SipHash or HMAC-SHA256.  This would be similar to
> the kernel's existing implementations of TCP SYN and SCTP cookies: the
> system sends out cookies that encode some information, and it uses a MAC
> to verify that any received cookie is a previously sent one.
>
> But that's just what I suspect.  I can't know for sure since this series
> doesn't provide any context about what it's trying to achieve.

In another excerpt from that first thread linked above, I responded to
Neil's inquiry about why not MAC:

    >> Normally encryption is for privacy and a MAC (message authentication
    >> code) is used for integrity.  Why are you not simply adding a MAC?

    Good question - I'll have to think more about why a MAC wouldn't do the job.
    So far, I've been thinking about this as I want to give clients an absolute
    minimum amount of information about the filesystem on the DS.  Less is more
    here - but I can see how a MAC could do the same job and possibly be less
    work for the server.

So (trying not to be too wordy), I wonder if that's still too vague.

Thanks for your response here, and sorry for the chopped up reply.  If this
doesn't make sense, I'll try again without the reference noise.

Ben