* [PATCH v1 1/7] nfsd: Convert export flags to use BIT() macro
2025-12-27 17:04 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Benjamin Coddington
@ 2025-12-27 17:04 ` Benjamin Coddington
2025-12-27 17:04 ` [PATCH v1 2/7] nfsd: Add a symmetric-key cipher for encrypted filehandles Benjamin Coddington
` (8 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-27 17:04 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, NeilBrown, Trond Myklebust,
Anna Schumaker, Benjamin Coddington
Cc: linux-nfs
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v1 2/7] nfsd: Add a symmetric-key cipher for encrypted filehandles
2025-12-27 17:04 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Benjamin Coddington
2025-12-27 17:04 ` [PATCH v1 1/7] nfsd: Convert export flags to use BIT() macro Benjamin Coddington
@ 2025-12-27 17:04 ` Benjamin Coddington
2025-12-27 17:04 ` [PATCH v1 3/7] nfsd/sunrpc: add per-thread crypto context pointer Benjamin Coddington
` (7 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-27 17:04 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, NeilBrown, Trond Myklebust,
Anna Schumaker, Benjamin Coddington
Cc: linux-nfs
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v1 3/7] nfsd/sunrpc: add per-thread crypto context pointer
2025-12-27 17:04 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Benjamin Coddington
2025-12-27 17:04 ` [PATCH v1 1/7] nfsd: Convert export flags to use BIT() macro Benjamin Coddington
2025-12-27 17:04 ` [PATCH v1 2/7] nfsd: Add a symmetric-key cipher for encrypted filehandles Benjamin Coddington
@ 2025-12-27 17:04 ` Benjamin Coddington
2025-12-27 17:04 ` [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf Benjamin Coddington
` (6 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-27 17:04 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, NeilBrown, Trond Myklebust,
Anna Schumaker, Benjamin Coddington
Cc: linux-nfs
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf
2025-12-27 17:04 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Benjamin Coddington
` (2 preceding siblings ...)
2025-12-27 17:04 ` [PATCH v1 3/7] nfsd/sunrpc: add per-thread crypto context pointer Benjamin Coddington
@ 2025-12-27 17:04 ` Benjamin Coddington
2025-12-28 17:52 ` kernel test robot
2025-12-29 0:33 ` kernel test robot
2025-12-27 17:04 ` [PATCH v1 5/7] NFSD/export: Add encrypt_fh export option Benjamin Coddington
` (5 subsequent siblings)
9 siblings, 2 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-27 17:04 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, NeilBrown, Trond Myklebust,
Anna Schumaker, Benjamin Coddington
Cc: linux-nfs
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf
2025-12-27 17:04 ` [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf Benjamin Coddington
@ 2025-12-28 17:52 ` kernel test robot
2025-12-29 0:33 ` kernel test robot
1 sibling, 0 replies; 28+ messages in thread
From: kernel test robot @ 2025-12-28 17:52 UTC (permalink / raw)
To: Benjamin Coddington, Chuck Lever, Jeff Layton, NeilBrown,
Trond Myklebust, Anna Schumaker
Cc: oe-kbuild-all, linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf
2025-12-27 17:04 ` [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf Benjamin Coddington
2025-12-28 17:52 ` kernel test robot
@ 2025-12-29 0:33 ` kernel test robot
1 sibling, 0 replies; 28+ messages in thread
From: kernel test robot @ 2025-12-29 0:33 UTC (permalink / raw)
To: Benjamin Coddington, Chuck Lever, Jeff Layton, NeilBrown,
Trond Myklebust, Anna Schumaker
Cc: oe-kbuild-all, linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 5/7] NFSD/export: Add encrypt_fh export option
2025-12-27 17:04 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Benjamin Coddington
` (3 preceding siblings ...)
2025-12-27 17:04 ` [PATCH v1 4/7] NFSD: Add a per-knfsd reusable encfh_buf Benjamin Coddington
@ 2025-12-27 17:04 ` Benjamin Coddington
2025-12-27 17:04 ` [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers Benjamin Coddington
` (4 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-27 17:04 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, NeilBrown, Trond Myklebust,
Anna Schumaker, Benjamin Coddington
Cc: linux-nfs
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers
2025-12-27 17:04 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Benjamin Coddington
` (4 preceding siblings ...)
2025-12-27 17:04 ` [PATCH v1 5/7] NFSD/export: Add encrypt_fh export option Benjamin Coddington
@ 2025-12-27 17:04 ` Benjamin Coddington
2025-12-27 17:14 ` Benjamin Coddington
` (2 more replies)
2025-12-27 17:04 ` [PATCH v1 7/7] NFSD: Enable filehandle encryption Benjamin Coddington
` (3 subsequent siblings)
9 siblings, 3 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-27 17:04 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, NeilBrown, Trond Myklebust,
Anna Schumaker, Benjamin Coddington
Cc: linux-nfs
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers
2025-12-27 17:04 ` [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers Benjamin Coddington
@ 2025-12-27 17:14 ` Benjamin Coddington
2025-12-28 1:34 ` Chuck Lever
2025-12-28 5:17 ` kernel test robot
2 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-27 17:14 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, NeilBrown, Trond Myklebust,
Anna Schumaker, Benjamin Coddington
Cc: linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers
2025-12-27 17:04 ` [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers Benjamin Coddington
2025-12-27 17:14 ` Benjamin Coddington
@ 2025-12-28 1:34 ` Chuck Lever
2025-12-28 20:45 ` Eric Biggers
2025-12-28 5:17 ` kernel test robot
2 siblings, 1 reply; 28+ messages in thread
From: Chuck Lever @ 2025-12-28 1:34 UTC (permalink / raw)
To: Benjamin Coddington, Chuck Lever, Jeff Layton, NeilBrown,
Trond Myklebust, Anna Schumaker
Cc: linux-nfs, linux-crypto
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers
2025-12-28 1:34 ` Chuck Lever
@ 2025-12-28 20:45 ` Eric Biggers
2025-12-29 13:39 ` Benjamin Coddington
0 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2025-12-28 20:45 UTC (permalink / raw)
To: Chuck Lever
Cc: Benjamin Coddington, Chuck Lever, Jeff Layton, NeilBrown,
Trond Myklebust, Anna Schumaker, linux-nfs, linux-crypto
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers
2025-12-28 20:45 ` Eric Biggers
@ 2025-12-29 13:39 ` Benjamin Coddington
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-29 13:39 UTC (permalink / raw)
To: Eric Biggers
Cc: Chuck Lever, Chuck Lever, Jeff Layton, NeilBrown, Trond Myklebust,
Anna Schumaker, linux-nfs, linux-crypto
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers
2025-12-27 17:04 ` [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers Benjamin Coddington
2025-12-27 17:14 ` Benjamin Coddington
2025-12-28 1:34 ` Chuck Lever
@ 2025-12-28 5:17 ` kernel test robot
2 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2025-12-28 5:17 UTC (permalink / raw)
To: Benjamin Coddington, Chuck Lever, Jeff Layton, NeilBrown,
Trond Myklebust, Anna Schumaker
Cc: oe-kbuild-all, linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 7/7] NFSD: Enable filehandle encryption
2025-12-27 17:04 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Benjamin Coddington
` (5 preceding siblings ...)
2025-12-27 17:04 ` [PATCH v1 6/7] NFSD: Add filehandle crypto functions and helpers Benjamin Coddington
@ 2025-12-27 17:04 ` Benjamin Coddington
2025-12-27 23:06 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles NeilBrown
` (2 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-27 17:04 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, NeilBrown, Trond Myklebust,
Anna Schumaker, Benjamin Coddington
Cc: linux-nfs
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles
2025-12-27 17:04 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Benjamin Coddington
` (6 preceding siblings ...)
2025-12-27 17:04 ` [PATCH v1 7/7] NFSD: Enable filehandle encryption Benjamin Coddington
@ 2025-12-27 23:06 ` NeilBrown
2025-12-27 23:26 ` Benjamin Coddington
2025-12-28 5:33 ` [PATCH v1 1/7] nfsd: Convert export flags to use BIT() macro NeilBrown
2025-12-28 17:09 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Chuck Lever
9 siblings, 1 reply; 28+ messages in thread
From: NeilBrown @ 2025-12-27 23:06 UTC (permalink / raw)
To: Benjamin Coddington
Cc: Chuck Lever, Jeff Layton, Trond Myklebust, Anna Schumaker,
Benjamin Coddington, linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles
2025-12-27 23:06 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles NeilBrown
@ 2025-12-27 23:26 ` Benjamin Coddington
2025-12-28 5:49 ` NeilBrown
0 siblings, 1 reply; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-27 23:26 UTC (permalink / raw)
To: NeilBrown
Cc: Chuck Lever, Jeff Layton, Trond Myklebust, Anna Schumaker,
linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles
2025-12-27 23:26 ` Benjamin Coddington
@ 2025-12-28 5:49 ` NeilBrown
2025-12-28 17:05 ` Rick Macklem
0 siblings, 1 reply; 28+ messages in thread
From: NeilBrown @ 2025-12-28 5:49 UTC (permalink / raw)
To: Benjamin Coddington
Cc: Chuck Lever, Jeff Layton, Trond Myklebust, Anna Schumaker,
linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles
2025-12-28 5:49 ` NeilBrown
@ 2025-12-28 17:05 ` Rick Macklem
2025-12-29 12:52 ` Benjamin Coddington
0 siblings, 1 reply; 28+ messages in thread
From: Rick Macklem @ 2025-12-28 17:05 UTC (permalink / raw)
To: NeilBrown
Cc: Benjamin Coddington, Chuck Lever, Jeff Layton, Trond Myklebust,
Anna Schumaker, linux-nfs
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles
2025-12-28 17:05 ` Rick Macklem
@ 2025-12-29 12:52 ` Benjamin Coddington
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-29 12:52 UTC (permalink / raw)
To: Rick Macklem, NeilBrown
Cc: Chuck Lever, Jeff Layton, Trond Myklebust, Anna Schumaker,
linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 1/7] nfsd: Convert export flags to use BIT() macro
2025-12-27 17:04 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Benjamin Coddington
` (7 preceding siblings ...)
2025-12-27 23:06 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles NeilBrown
@ 2025-12-28 5:33 ` NeilBrown
2025-12-29 12:11 ` Benjamin Coddington
2025-12-28 17:09 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Chuck Lever
9 siblings, 1 reply; 28+ messages in thread
From: NeilBrown @ 2025-12-28 5:33 UTC (permalink / raw)
To: Benjamin Coddington
Cc: Chuck Lever, Jeff Layton, Trond Myklebust, Anna Schumaker,
Benjamin Coddington, linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 1/7] nfsd: Convert export flags to use BIT() macro
2025-12-28 5:33 ` [PATCH v1 1/7] nfsd: Convert export flags to use BIT() macro NeilBrown
@ 2025-12-29 12:11 ` Benjamin Coddington
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-29 12:11 UTC (permalink / raw)
To: NeilBrown
Cc: Chuck Lever, Jeff Layton, Trond Myklebust, Anna Schumaker,
linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles
2025-12-27 17:04 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Benjamin Coddington
` (8 preceding siblings ...)
2025-12-28 5:33 ` [PATCH v1 1/7] nfsd: Convert export flags to use BIT() macro NeilBrown
@ 2025-12-28 17:09 ` Chuck Lever
2025-12-29 13:23 ` Benjamin Coddington
9 siblings, 1 reply; 28+ messages in thread
From: Chuck Lever @ 2025-12-28 17:09 UTC (permalink / raw)
To: Benjamin Coddington, Chuck Lever, Jeff Layton, NeilBrown,
Trond Myklebust, Anna Schumaker
Cc: linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 0/7] kNFSD Encrypted Filehandles
2025-12-28 17:09 ` [PATCH v1 0/7] kNFSD Encrypted Filehandles Chuck Lever
@ 2025-12-29 13:23 ` Benjamin Coddington
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2025-12-29 13:23 UTC (permalink / raw)
To: Chuck Lever
Cc: Chuck Lever, Jeff Layton, NeilBrown, Trond Myklebust,
Anna Schumaker, linux-nfs
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
^ permalink raw reply [flat|nested] 28+ messages in thread