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

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

[Chuck Lever]

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

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


-- 
Chuck Lever

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

[Eric Biggers]

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

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

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

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

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

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

- Eric

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

[Benjamin Coddington]

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

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

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

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

.. summarized:

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

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

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

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


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

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

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

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

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

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

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

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

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

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

Ben