public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Benjamin Coddington <bcodding@hammerspace.com>,
	Chuck Lever <chuck.lever@oracle.com>, NeilBrown <neil@brown.name>,
	Trond Myklebust <trondmy@kernel.org>,
	Anna Schumaker <anna@kernel.org>,
	Eric Biggers <ebiggers@kernel.org>,
	Rick Macklem <rick.macklem@gmail.com>
Cc: linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	 linux-crypto@vger.kernel.org
Subject: Re: [PATCH v7 3/3] NFSD: Sign filehandles
Date: Wed, 25 Feb 2026 07:10:45 -0500	[thread overview]
Message-ID: <c64ca6ae0f2d948f42b454c87ebcc58edee8bc3c.camel@kernel.org> (raw)
In-Reply-To: <6ca1559957e3ebe3a96ac9553df621305a4b33ea.1771961922.git.bcodding@hammerspace.com>

On Tue, 2026-02-24 at 14:41 -0500, Benjamin Coddington wrote:
> NFS clients may bypass restrictive directory permissions by using
> open_by_handle() (or other available OS system call) to guess the
> filehandles for files below that directory.
> 
> In order to harden knfsd servers against this attack, create a method to
> sign and verify filehandles using SipHash-2-4 as a MAC (Message
> Authentication Code).  According to
> https://cr.yp.to/siphash/siphash-20120918.pdf, SipHash can be used as a
> MAC, and our use of SipHash-2-4 provides a low 1 in 2^64 chance of forgery.
> 
> Filehandles that have been signed cannot be tampered with, nor can
> clients reasonably guess correct filehandles and hashes that may exist in
> parts of the filesystem they cannot access due to directory permissions.
> 
> Append the 8 byte SipHash to encoded filehandles for exports that have set
> the "sign_fh" export option.  Filehandles received from clients are
> verified by comparing the appended hash to the expected hash.  If the MAC
> does not match the server responds with NFS error _STALE.  If unsigned
> filehandles are received for an export with "sign_fh" they are rejected
> with NFS error _STALE.
> 
> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> ---
>  Documentation/filesystems/nfs/exporting.rst |  85 +++++++++++++
>  fs/nfsd/Kconfig                             |   2 +-
>  fs/nfsd/nfsfh.c                             | 127 +++++++++++++++++++-
>  fs/nfsd/trace.h                             |   1 +
>  4 files changed, 210 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/filesystems/nfs/exporting.rst b/Documentation/filesystems/nfs/exporting.rst
> index a01d9b9b5bc3..4aa59b0bf253 100644
> --- a/Documentation/filesystems/nfs/exporting.rst
> +++ b/Documentation/filesystems/nfs/exporting.rst
> @@ -206,3 +206,88 @@ following flags are defined:
>      all of an inode's dirty data on last close. Exports that behave this
>      way should set EXPORT_OP_FLUSH_ON_CLOSE so that NFSD knows to skip
>      waiting for writeback when closing such files.
> +
> +Signed Filehandles
> +------------------
> +
> +To protect against filehandle guessing attacks, the Linux NFS server can be
> +configured to sign filehandles with a Message Authentication Code (MAC).
> +
> +Standard NFS filehandles are often predictable. If an attacker can guess
> +a valid filehandle for a file they do not have permission to access via
> +directory traversal, they may be able to bypass path-based permissions
> +(though they still remain subject to inode-level permissions).
> +
> +Signed filehandles prevent this by appending a MAC to the filehandle
> +before it is sent to the client. Upon receiving a filehandle back from a
> +client, the server re-calculates the MAC using its internal key and
> +verifies it against the one provided. If the signatures do not match,
> +the server treats the filehandle as invalid (returning NFS[34]ERR_STALE).
> +
> +Note that signing filehandles provides integrity and authenticity but
> +not confidentiality. The contents of the filehandle remain visible to
> +the client; they simply cannot be forged or modified.
> +
> +Configuration
> +~~~~~~~~~~~~~
> +
> +To enable signed filehandles, the administrator must provide a signing
> +key to the kernel and enable the "sign_fh" export option.
> +
> +1. Providing a Key
> +   The signing key is managed via the nfsd netlink interface. This key
> +   is per-network-namespace and must be set before any exports using
> +   "sign_fh" become active.
> +
> +2. Export Options
> +   The feature is controlled on a per-export basis in /etc/exports:
> +
> +   sign_fh
> +     Enables signing for all filehandles generated under this export.
> +
> +   no_sign_fh
> +     (Default) Disables signing.
> +
> +Key Management and Rotation
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The security of this mechanism relies entirely on the secrecy of the
> +signing key.
> +
> +Initial Setup:
> +  The key should be generated using a high-quality random source and
> +  loaded early in the boot process or during the nfs-server startup
> +  sequence.
> +
> +Changing Keys:
> +  If a key is changed while clients have active mounts, existing
> +  filehandles held by those clients will become invalid, resulting in
> +  "Stale file handle" errors on the client side.
> +
> +Safe Rotation:
> +  Currently, there is no mechanism for "graceful" key rotation
> +  (maintaining multiple valid keys). Changing the key is an atomic
> +  operation that immediately invalidates all previous signatures.
> +
> +Transitioning Exports
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +When adding or removing the "sign_fh" flag from an active export, the
> +following behaviors should be expected:
> +
> ++-------------------+---------------------------------------------------+
> +| Change            | Result for Existing Clients                       |
> ++===================+===================================================+
> +| Adding sign_fh    | Clients holding unsigned filehandles will find    |
> +|                   | them rejected, as the server now expects a        |
> +|                   | signature.                                        |
> ++-------------------+---------------------------------------------------+
> +| Removing sign_fh  | Clients holding signed filehandles will find them |
> +|                   | rejected, as the server now expects the           |
> +|                   | filehandle to end at its traditional boundary     |
> +|                   | without a MAC.                                    |
> ++-------------------+---------------------------------------------------+
> +
> +Because filehandles are often cached persistently by clients, adding or
> +removing this option should generally be done during a scheduled maintenance
> +window involving a NFS client unmount/remount.
> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> index fc0e87eaa257..ffb76761d6a8 100644
> --- a/fs/nfsd/Kconfig
> +++ b/fs/nfsd/Kconfig
> @@ -7,6 +7,7 @@ config NFSD
>  	select CRC32
>  	select CRYPTO_LIB_MD5 if NFSD_LEGACY_CLIENT_TRACKING
>  	select CRYPTO_LIB_SHA256 if NFSD_V4
> +	select CRYPTO # required by RPCSEC_GSS_KRB5 and signed filehandles
>  	select LOCKD
>  	select SUNRPC
>  	select EXPORTFS
> @@ -78,7 +79,6 @@ config NFSD_V4
>  	depends on NFSD && PROC_FS
>  	select FS_POSIX_ACL
>  	select RPCSEC_GSS_KRB5
> -	select CRYPTO # required by RPCSEC_GSS_KRB5
>  	select GRACE_PERIOD
>  	select NFS_V4_2_SSC_HELPER if NFS_V4_2
>  	help
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index 68b629fbaaeb..383d04596627 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/utils.h>
>  #include "nfsd.h"
>  #include "vfs.h"
>  #include "auth.h"
> @@ -140,6 +141,110 @@ static inline __be32 check_pseudo_root(struct dentry *dentry,
>  	return nfs_ok;
>  }
>  
> +/* Size of a file handle MAC, in 4-octet words */
> +#define FH_MAC_WORDS (sizeof(__le64) / 4)
> +
> +static bool fh_append_mac(struct svc_fh *fhp, struct net *net)

I get build failures with this patch in place. This function is defined
here....

> +{
> +	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> +	struct knfsd_fh *fh = &fhp->fh_handle;
> +	siphash_key_t *fh_key = nn->fh_key;
> +	__le64 hash;
> +
> +	if (!fh_key)
> +		goto out_no_key;
> +	if (fh->fh_size + sizeof(hash) > fhp->fh_maxsize)
> +		goto out_no_space;
> +
> +	hash = cpu_to_le64(siphash(&fh->fh_raw, fh->fh_size, fh_key));
> +	memcpy(&fh->fh_raw[fh->fh_size], &hash, sizeof(hash));
> +	fh->fh_size += sizeof(hash);
> +	return true;
> +
> +out_no_key:
> +	pr_warn_ratelimited("NFSD: unable to sign filehandles, fh_key not set.\n");
> +	return false;
> +
> +out_no_space:
> +	pr_warn_ratelimited("NFSD: unable to sign filehandles, fh_size %zu would be greater than fh_maxsize %d.\n",
> +			    fh->fh_size + sizeof(hash), fhp->fh_maxsize);
> +	return false;
> +}
> +
> +/*
> + * Verify that the filehandle's MAC was hashed from this filehandle
> + * given the server's fh_key:
> + */
> +static bool fh_verify_mac(struct svc_fh *fhp, struct net *net)
> +{
> +	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> +	struct knfsd_fh *fh = &fhp->fh_handle;
> +	siphash_key_t *fh_key = nn->fh_key;
> +	__le64 hash;
> +
> +	if (!fh_key) {
> +		pr_warn_ratelimited("NFSD: unable to verify signed filehandles, fh_key not set.\n");
> +		return false;
> +	}
> +
> +	hash = cpu_to_le64(siphash(&fh->fh_raw, fh->fh_size - sizeof(hash),  fh_key));
> +	return crypto_memneq(&fh->fh_raw[fh->fh_size - sizeof(hash)],
> +					&hash, sizeof(hash)) == 0;
> +}
> +
> +/*
> + * Append an 8-byte MAC to the filehandle hashed from the server's fh_key:
> + */
> +#define FH_MAC_WORDS sizeof(__le64)/4
> +static bool fh_append_mac(struct svc_fh *fhp, struct net *net)


...and here, and the compiler doesn't seem to like that.

> +{
> +	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> +	struct knfsd_fh *fh = &fhp->fh_handle;
> +	siphash_key_t *fh_key = nn->fh_key;
> +	__le64 hash;
> +
> +	if (!(fhp->fh_export->ex_flags & NFSEXP_SIGN_FH))
> +		return true;
> +
> +	if (!fh_key) {
> +		pr_warn_ratelimited("NFSD: unable to sign filehandles, fh_key not set.\n");
> +		return false;
> +	}
> +
> +	if (fh->fh_size + sizeof(hash) > fhp->fh_maxsize) {
> +		pr_warn_ratelimited("NFSD: unable to sign filehandles, fh_size %d would be greater"
> +			" than fh_maxsize %d.\n", (int)(fh->fh_size + sizeof(hash)), fhp->fh_maxsize);
> +		return false;
> +	}
> +
> +	hash = cpu_to_le64(siphash(&fh->fh_raw, fh->fh_size, fh_key));
> +	memcpy(&fh->fh_raw[fh->fh_size], &hash, sizeof(hash));
> +	fh->fh_size += sizeof(hash);
> +
> +	return true;
> +}
> +
> +/*
> + * Verify that the filehandle's MAC was hashed from this filehandle
> + * given the server's fh_key:
> + */
> +static bool fh_verify_mac(struct svc_fh *fhp, struct net *net)
> +{
> +	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> +	struct knfsd_fh *fh = &fhp->fh_handle;
> +	siphash_key_t *fh_key = nn->fh_key;
> +	__le64 hash;
> +
> +	if (!fh_key) {
> +		pr_warn_ratelimited("NFSD: unable to verify signed filehandles, fh_key not set.\n");
> +		return false;
> +	}
> +
> +	hash = cpu_to_le64(siphash(&fh->fh_raw, fh->fh_size - sizeof(hash),  fh_key));
> +	return crypto_memneq(&fh->fh_raw[fh->fh_size - sizeof(hash)],
> +					&hash, sizeof(hash)) == 0;
> +}
> +
>  /*
>   * Use the given filehandle to look up the corresponding export and
>   * dentry.  On success, the results are used to set fh_export and
> @@ -236,13 +341,21 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct net *net,
>  	/*
>  	 * Look up the dentry using the NFS file handle.
>  	 */
> -	error = nfserr_badhandle;
> -
>  	fileid_type = fh->fh_fileid_type;
> +	error = nfserr_stale;
>  
> -	if (fileid_type == FILEID_ROOT)
> +	if (fileid_type == FILEID_ROOT) {
> +		/* We don't sign or verify the root, no per-file identity */
>  		dentry = dget(exp->ex_path.dentry);
> -	else {
> +	} else {
> +		if (exp->ex_flags & NFSEXP_SIGN_FH) {
> +			if (!fh_verify_mac(fhp, net)) {
> +				trace_nfsd_set_fh_dentry_badmac(rqstp, fhp, -ESTALE);
> +				goto out;
> +			}
> +			data_left -= FH_MAC_WORDS;
> +		}
> +
>  		dentry = exportfs_decode_fh_raw(exp->ex_path.mnt, fid,
>  						data_left, fileid_type, 0,
>  						nfsd_acceptable, exp);
> @@ -258,6 +371,8 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct net *net,
>  			}
>  		}
>  	}
> +
> +	error = nfserr_badhandle;
>  	if (dentry == NULL)
>  		goto out;
>  	if (IS_ERR(dentry)) {
> @@ -498,6 +613,10 @@ static void _fh_update(struct svc_fh *fhp, struct svc_export *exp,
>  		fhp->fh_handle.fh_fileid_type =
>  			fileid_type > 0 ? fileid_type : FILEID_INVALID;
>  		fhp->fh_handle.fh_size += maxsize * 4;
> +
> +		if (exp->ex_flags & NFSEXP_SIGN_FH)
> +			if (!fh_append_mac(fhp, exp->cd->net))
> +				fhp->fh_handle.fh_fileid_type = FILEID_INVALID;
>  	} else {
>  		fhp->fh_handle.fh_fileid_type = FILEID_ROOT;
>  	}
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index 185a998996a0..5ad38f50836d 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -373,6 +373,7 @@ DEFINE_EVENT_CONDITION(nfsd_fh_err_class, nfsd_##name,	\
>  
>  DEFINE_NFSD_FH_ERR_EVENT(set_fh_dentry_badexport);
>  DEFINE_NFSD_FH_ERR_EVENT(set_fh_dentry_badhandle);
> +DEFINE_NFSD_FH_ERR_EVENT(set_fh_dentry_badmac);
>  
>  TRACE_EVENT(nfsd_exp_find_key,
>  	TP_PROTO(const struct svc_expkey *key,

-- 
Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2026-02-25 12:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-24 19:41 [PATCH v7 0/3] kNFSD Signed Filehandles Benjamin Coddington
2026-02-24 19:41 ` [PATCH v7 1/3] NFSD: Add a key for signing filehandles Benjamin Coddington
2026-02-24 19:41 ` [PATCH v7 2/3] NFSD/export: Add sign_fh export option Benjamin Coddington
2026-02-24 19:41 ` [PATCH v7 3/3] NFSD: Sign filehandles Benjamin Coddington
2026-02-25 12:10   ` Jeff Layton [this message]
2026-02-25 12:23     ` Benjamin Coddington
2026-02-24 22:14 ` [PATCH v7 0/3] kNFSD Signed Filehandles Chuck Lever
  -- strict thread matches above, loose matches on Subject: below --
2026-02-25 12:51 [RESEND PATCH " Benjamin Coddington
2026-02-25 12:51 ` [PATCH v7 3/3] NFSD: Sign filehandles Benjamin Coddington

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c64ca6ae0f2d948f42b454c87ebcc58edee8bc3c.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=anna@kernel.org \
    --cc=bcodding@hammerspace.com \
    --cc=chuck.lever@oracle.com \
    --cc=ebiggers@kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=rick.macklem@gmail.com \
    --cc=trondmy@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox