On Thu, May 15, 2025 at 01:50:55PM +0200, Christoph Hellwig wrote:
> Allow tlshd to use a per-mount key from the kernel keyring similar
> to NVMe over TCP.
>
> Note that tlshd expects keys and certificates stored in the kernel
> keyring to be in DER format, not the PEM format used for file based keys
> and certificates, so they need to be converted before they are added
> to the keyring, which is a bit unexpected.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/nfs/fs_context.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> index 13f71ca8c974..9e94d18448ff 100644
> --- a/fs/nfs/fs_context.c
> +++ b/fs/nfs/fs_context.c
> @@ -96,6 +96,8 @@ enum nfs_param {
> Opt_wsize,
> Opt_write,
> Opt_xprtsec,
> + Opt_cert_serial,
> + Opt_privkey_serial,
> };
>
> enum {
> @@ -221,6 +223,8 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
> fsparam_enum ("write", Opt_write, nfs_param_enums_write),
> fsparam_u32 ("wsize", Opt_wsize),
> fsparam_string("xprtsec", Opt_xprtsec),
> + fsparam_s32("cert_serial", Opt_cert_serial),
> + fsparam_s32("privkey_serial", Opt_privkey_serial),
> {}
> };
>
> @@ -551,6 +555,32 @@ static int nfs_parse_version_string(struct fs_context *fc,
> return 0;
> }
>
> +#ifdef CONFIG_KEYS
> +static int nfs_tls_key_verify(key_serial_t key_id)
> +{
> + struct key *key = key_lookup(key_id);
> + int error = 0;
> +
> + if (IS_ERR(key)) {
> + pr_err("key id %08x not found\n", key_id);
> + return PTR_ERR(key);
> + }
> + if (test_bit(KEY_FLAG_REVOKED, &key->flags) ||
> + test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
> + pr_err("key id %08x revoked\n", key_id);
> + error = -EKEYREVOKED;
> + }
> +
> + key_put(key);
> + return error;
> +}
This is equivalent nvme_tls_key_lookup() so would it be more senseful
to call it nfs_tls_key_lookup()? I'm also a bit puzzled how the code
will associate nfs_keyring to all this (e.g., with keyring_search as
done in nvme_tls_psk_lookup())?
> +#else
> +static inline int nfs_tls_key_verify(key_serial_t key_id)
> +{
> + return -ENOENT;
> +}
> +#endif /* CONFIG_KEYS */
> +
> /*
> * Parse a single mount parameter.
> */
> @@ -807,6 +837,18 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
> if (ret < 0)
> return ret;
> break;
> + case Opt_cert_serial:
> + ret = nfs_tls_key_verify(result.int_32);
> + if (ret < 0)
> + return ret;
> + ctx->xprtsec.cert_serial = result.int_32;
> + break;
> + case Opt_privkey_serial:
> + ret = nfs_tls_key_verify(result.int_32);
> + if (ret < 0)
> + return ret;
> + ctx->xprtsec.privkey_serial = result.int_32;
> + break;
>
> case Opt_proto:
> if (!param->string)
> --
> 2.47.2
>
I get the change i.e., keep keys opaque, and it is a reasonable goal.
However, the keyring-key association is where I get lost, so if you
could help me out with that a bit, we could make progress :-)
BR, Jarkko
