support keyrings for NFS TLS mounts v2

support keyrings for NFS TLS mounts v2

[Christoph Hellwig]

Hi all,

this series allows storing the key and certificate for NFS over
TLS mounts in the keyring and be specified using a mount option.
This way they don't need to be hardcoded in the global tlshd.conf
configuration file and can even be different per-mount.

Note that for now the .nfs keyring still needs to be added to
tlshd.conf, but that should go away with the handshake enhacement
from Hannes.

Changes since v1:
 - don't depend on nfsv4 for the keyring
 - fix compile when the kernel keyring is disabled

[PATCH 1/2] NFS: support the kernel keyring for TLS

[Christoph Hellwig]

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@lst.de>
---
 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;
+}
+#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

[PATCH 2/2] nfs: create a kernel keyring

[Christoph Hellwig]

Create a kernel .nfs keyring similar to the nvme .nvme one.  Unlike for
a userspace-created keyrind, tlshd is a possesor of the keys with this
and thus the keys don't need user read permissions.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/inode.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 119e447758b9..e7a519f5b6bc 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -2571,6 +2571,35 @@ static struct pernet_operations nfs_net_ops = {
 	.size = sizeof(struct nfs_net),
 };
 
+#ifdef CONFIG_KEYS
+static struct key *nfs_keyring;
+
+static int __init nfs_init_keyring(void)
+{
+	nfs_keyring = keyring_alloc(".nfs",
+			     GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+			     current_cred(),
+			     (KEY_POS_ALL & ~KEY_POS_SETATTR) |
+			     (KEY_USR_ALL & ~KEY_USR_SETATTR),
+			     KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
+	return PTR_ERR_OR_ZERO(nfs_keyring);
+}
+
+static void __exit nfs_exit_keyring(void)
+{
+	key_put(nfs_keyring);
+}
+#else
+static inline int nfs_init_keyring(void)
+{
+	return 0;
+}
+
+static inline void nfs_exit_keyring(void)
+{
+}
+#endif /* CONFIG_KEYS */
+
 /*
  * Initialize NFS
  */
@@ -2578,6 +2607,10 @@ static int __init init_nfs_fs(void)
 {
 	int err;
 
+	err = nfs_init_keyring();
+	if (err)
+		return err;
+
 	err = nfs_sysfs_init();
 	if (err < 0)
 		goto out10;
@@ -2638,6 +2671,7 @@ static int __init init_nfs_fs(void)
 out9:
 	nfs_sysfs_exit();
 out10:
+	nfs_exit_keyring();
 	return err;
 }
 
@@ -2653,6 +2687,7 @@ static void __exit exit_nfs_fs(void)
 	nfs_fs_proc_exit();
 	nfsiod_stop();
 	nfs_sysfs_exit();
+	nfs_exit_keyring();
 }
 
 /* Not quite true; I just maintain it */
-- 
2.47.2

Re: support keyrings for NFS TLS mounts v2

[Chuck Lever]

On 5/15/25 7:50 AM, Christoph Hellwig wrote:
> Hi all,
> 
> this series allows storing the key and certificate for NFS over
> TLS mounts in the keyring and be specified using a mount option.
> This way they don't need to be hardcoded in the global tlshd.conf
> configuration file and can even be different per-mount.
> 
> Note that for now the .nfs keyring still needs to be added to
> tlshd.conf, but that should go away with the handshake enhacement
> from Hannes.

Just curious: Is there a downside to shipping a default /etc/tlshd.conf
with the NVMe and NFS keyrings already added?


> Changes since v1:
>  - don't depend on nfsv4 for the keyring
>  - fix compile when the kernel keyring is disabled


-- 
Chuck Lever

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Jarkko Sakkinen]

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@lst.de>
> ---
>  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

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Hannes Reinecke]

On 5/15/25 14:51, Jarkko Sakkinen wrote:
> 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@lst.de>
>> ---
>>   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())?
> 
With this patch the keyring is pretty much immaterial; the interface
is passing in a serial number which is unique across all keyrings.
Where the keyring comes in when looking up keys on the TLS server,
as there the TLS client hello only transports the key description
(which are not required to be unique across all keyrings).
So there we'll need the keyring to be specified.
But for the client we really don't.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: support keyrings for NFS TLS mounts v2

[Christoph Hellwig]

On Thu, May 15, 2025 at 08:31:23AM -0400, Chuck Lever wrote:
> > Note that for now the .nfs keyring still needs to be added to
> > tlshd.conf, but that should go away with the handshake enhacement
> > from Hannes.
> 
> Just curious: Is there a downside to shipping a default /etc/tlshd.conf
> with the NVMe and NFS keyrings already added?

That's probably a good idea for the current situation, but doesn't
resolve solve the overall problem of having to add every new service
there.

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Christoph Hellwig]

On Thu, May 15, 2025 at 04:46:43PM +0200, Hannes Reinecke wrote:
> With this patch the keyring is pretty much immaterial; the interface
> is passing in a serial number which is unique across all keyrings.
> Where the keyring comes in when looking up keys on the TLS server,
> as there the TLS client hello only transports the key description
> (which are not required to be unique across all keyrings).
> So there we'll need the keyring to be specified.
> But for the client we really don't.

Yes.  Patch 1 on it's own actually works fine-ish.  The big difference
is that the keys would have to be made user-readable as without the
keyring, tlshd would not be the possesor of the key.

Re: support keyrings for NFS TLS mounts v2

[Sagi Grimberg]

On 16/05/2025 8:16, Christoph Hellwig wrote:
> On Thu, May 15, 2025 at 08:31:23AM -0400, Chuck Lever wrote:
>>> Note that for now the .nfs keyring still needs to be added to
>>> tlshd.conf, but that should go away with the handshake enhacement
>>> from Hannes.
>> Just curious: Is there a downside to shipping a default /etc/tlshd.conf
>> with the NVMe and NFS keyrings already added?
> That's probably a good idea for the current situation, but doesn't
> resolve solve the overall problem of having to add every new service
> there.

IIRC Hannes told me that latest tlshd already does this for .nvme keyring

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 2/2] nfs: create a kernel keyring

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 1/2] NFS: support the kernel keyring for TLS

[Jarkko Sakkinen]

On Thu, May 15, 2025 at 04:46:43PM +0200, Hannes Reinecke wrote:
> > 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())?
> > 
> With this patch the keyring is pretty much immaterial; the interface
> is passing in a serial number which is unique across all keyrings.
> Where the keyring comes in when looking up keys on the TLS server,
> as there the TLS client hello only transports the key description
> (which are not required to be unique across all keyrings).
> So there we'll need the keyring to be specified.
> But for the client we really don't.

I did not see anything that would depend on having 2/2 at all, or
it getting populated.

> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke                  Kernel Storage Architect
> hare@suse.de                                +49 911 74053 688
> SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
> HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

BR, Jarkko

Re: [PATCH 2/2] nfs: create a kernel keyring

[Jarkko Sakkinen]

On Fri, May 16, 2025 at 02:47:18PM +0300, Sagi Grimberg wrote:
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Based on?

BR, Jarkko

Re: [PATCH 2/2] nfs: create a kernel keyring

[Sagi Grimberg]

On 16/05/2025 20:03, Jarkko Sakkinen wrote:
> On Fri, May 16, 2025 at 02:47:18PM +0300, Sagi Grimberg wrote:
>> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> Based on?

Based on the same that nvme is doing. The only reason I see to have it
is to avoid having the user explicitly set perms on the key for tlshd to be
able to load it. nvme creates its own keyring that possessors can use, 
so makes
sense that nfs has this keyring as well.

Re: [PATCH 2/2] nfs: create a kernel keyring

[kernel test robot]

Hi Christoph,

kernel test robot noticed the following build warnings:

[auto build test WARNING on trondmy-nfs/linux-next]
[also build test WARNING on linus/master v6.15-rc6 next-20250516]
[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/Christoph-Hellwig/nfs-create-a-kernel-keyring/20250515-205131
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
patch link:    https://lore.kernel.org/r/20250515115107.33052-3-hch%40lst.de
patch subject: [PATCH 2/2] nfs: create a kernel keyring
config: sh-sh03_defconfig (https://download.01.org/0day-ci/archive/20250518/202505180251.Qt2Rlaed-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250518/202505180251.Qt2Rlaed-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/202505180251.Qt2Rlaed-lkp@intel.com/

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> WARNING: modpost: vmlinux: section mismatch in reference: init_nfs_fs+0x18c (section: .init.text) -> exit_misc_binfmt (section: .exit.text)

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

Re: [PATCH 2/2] nfs: create a kernel keyring

[Christoph Hellwig]

On Sat, May 17, 2025 at 12:45:02PM +0300, Sagi Grimberg wrote:
>
>
> On 16/05/2025 20:03, Jarkko Sakkinen wrote:
>> On Fri, May 16, 2025 at 02:47:18PM +0300, Sagi Grimberg wrote:
>>> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
>> Based on?
>
> Based on the same that nvme is doing. The only reason I see to have it
> is to avoid having the user explicitly set perms on the key for tlshd to be
> able to load it. nvme creates its own keyring that possessors can use, so 
> makes
> sense that nfs has this keyring as well.

Jarkoo, can you please state your objections clearly?  You've only
asked this one liner question in response to Sagi's question but not
even commented the original patch.

Re: [PATCH 2/2] nfs: create a kernel keyring

[Jarkko Sakkinen]

On Mon, Jun 02, 2025 at 05:25:25PM +0200, Christoph Hellwig wrote:
> On Sat, May 17, 2025 at 12:45:02PM +0300, Sagi Grimberg wrote:
> >
> >
> > On 16/05/2025 20:03, Jarkko Sakkinen wrote:
> >> On Fri, May 16, 2025 at 02:47:18PM +0300, Sagi Grimberg wrote:
> >>> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> >> Based on?
> >
> > Based on the same that nvme is doing. The only reason I see to have it
> > is to avoid having the user explicitly set perms on the key for tlshd to be
> > able to load it. nvme creates its own keyring that possessors can use, so 
> > makes
> > sense that nfs has this keyring as well.
> 
> Jarkoo, can you please state your objections clearly?  You've only
> asked this one liner question in response to Sagi's question but not
> even commented the original patch.

OK, I put this in simple terms, so perhaps I learn something from
nvme and nfs code:

1. The code change itself, if this keyring is needed, it looks
   reasonable.
2. However, I don't see any callers within the scope of patch set
   for this keyring.

I could quite quickly grab the idea how NVME uses nvme_keyring in TLS
handshake code from drivers/nvme/target/{configfs.c,tcp.c}. I guess
similar idea will be used in nfs code but I don't see any use for it
in the patch set.

Thus, it is hard to grasp the idea of having this patch applied without
any supplemental patch set.

BR, Jarkko

Re: [PATCH 2/2] nfs: create a kernel keyring

[Christoph Hellwig]

On Wed, Jun 04, 2025 at 07:42:52PM +0300, Jarkko Sakkinen wrote:
> OK, I put this in simple terms, so perhaps I learn something from
> nvme and nfs code:
> 
> 1. The code change itself, if this keyring is needed, it looks
>    reasonable.
> 2. However, I don't see any callers within the scope of patch set
>    for this keyring.
> 
> I could quite quickly grab the idea how NVME uses nvme_keyring in TLS
> handshake code from drivers/nvme/target/{configfs.c,tcp.c}. I guess
> similar idea will be used in nfs code but I don't see any use for it
> in the patch set.
> 
> Thus, it is hard to grasp the idea of having this patch applied without
> any supplemental patch set.

Maybe I'm missing something.  The reason I added the keyring was that
without it, tlshd is not the possesor of the keys and can't read them.

I guess you refer to the fact that nvme_tls_psk_lookup does a
keyring_search and nothing in the NFS code does?  nvme_tls_psk_lookup is
only used for the default key based on the server side identification in
NVMe, a concept that doesn't exist in NFS.  But the fact that the keys
aren't otherwise readable exists for both nvme and NFS.

Re: [PATCH 2/2] nfs: create a kernel keyring

[Jarkko Sakkinen]

On Thu, Jun 05, 2025 at 06:28:02AM +0200, Christoph Hellwig wrote:
> On Wed, Jun 04, 2025 at 07:42:52PM +0300, Jarkko Sakkinen wrote:
> > OK, I put this in simple terms, so perhaps I learn something from
> > nvme and nfs code:
> > 
> > 1. The code change itself, if this keyring is needed, it looks
> >    reasonable.
> > 2. However, I don't see any callers within the scope of patch set
> >    for this keyring.
> > 
> > I could quite quickly grab the idea how NVME uses nvme_keyring in TLS
> > handshake code from drivers/nvme/target/{configfs.c,tcp.c}. I guess
> > similar idea will be used in nfs code but I don't see any use for it
> > in the patch set.
> > 
> > Thus, it is hard to grasp the idea of having this patch applied without
> > any supplemental patch set.
> 
> Maybe I'm missing something.  The reason I added the keyring was that
> without it, tlshd is not the possesor of the keys and can't read them.
> 
> I guess you refer to the fact that nvme_tls_psk_lookup does a
> keyring_search and nothing in the NFS code does?  nvme_tls_psk_lookup is
> only used for the default key based on the server side identification in
> NVMe, a concept that doesn't exist in NFS.  But the fact that the keys
> aren't otherwise readable exists for both nvme and NFS.

Ah, ok this cleared it up, thanks! Just learning these subsystem,
appreciate the patience with this one :-)

BR, Jarkko

Re: [PATCH 2/2] nfs: create a kernel keyring

[Christoph Hellwig]

On Fri, Jun 06, 2025 at 07:47:57PM +0300, Jarkko Sakkinen wrote:
> Ah, ok this cleared it up, thanks! Just learning these subsystem,
> appreciate the patience with this one :-)

I'm also just learning the keyring, so double checking from that
perspective is also always welcome..

Re: [PATCH 2/2] nfs: create a kernel keyring

[Jarkko Sakkinen]

On Mon, 2025-06-09 at 06:01 +0200, Christoph Hellwig wrote:
> On Fri, Jun 06, 2025 at 07:47:57PM +0300, Jarkko Sakkinen wrote:
> > Ah, ok this cleared it up, thanks! Just learning these subsystem,
> > appreciate the patience with this one :-)
> 
> I'm also just learning the keyring, so double checking from that
> perspective is also always welcome..

After quickly studying tlshd, my understanding is that the ".nfs" is a
"vault for transient stuff" passed to "keyrings" configuration option.
After that serials within that vault are passed to mount-options
defined in 1/2.

Stating the obvious, just checking that I understand the user story...

BR, Jarkko

Re: [PATCH 2/2] nfs: create a kernel keyring

[Christoph Hellwig]

On Tue, Jun 10, 2025 at 12:28:49AM +0300, Jarkko Sakkinen wrote:
> On Mon, 2025-06-09 at 06:01 +0200, Christoph Hellwig wrote:
> > On Fri, Jun 06, 2025 at 07:47:57PM +0300, Jarkko Sakkinen wrote:
> > > Ah, ok this cleared it up, thanks! Just learning these subsystem,
> > > appreciate the patience with this one :-)
> > 
> > I'm also just learning the keyring, so double checking from that
> > perspective is also always welcome..
> 
> After quickly studying tlshd, my understanding is that the ".nfs" is a
> "vault for transient stuff" passed to "keyrings" configuration option.
> After that serials within that vault are passed to mount-options
> defined in 1/2.

Yes.  I'll try to make it more clear for a resend, and I also plan
to write documents for using TLS and thus the keyrings with NFS and
nvme.

Re: support keyrings for NFS TLS mounts v2

[Christoph Hellwig]

Trond/Anna:

can you queue this up?  If not can you tell me what is missing?

Re: support keyrings for NFS TLS mounts v2

[Trond Myklebust]

On Thu, 2025-07-10 at 09:25 +0200, Christoph Hellwig wrote:
> Trond/Anna:
> 
> can you queue this up?  If not can you tell me what is missing?
> 

Sorry. That series managed to fly under the radar while I was on
vacation. It looks good to me, so I'm picking it into my 'testing'
branch now.

Thanks for the reminder!

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com