Linux NFS development
 help / color / mirror / Atom feed
* Breakage in ktls-utils with nfs keyring?
@ 2026-04-30 13:32 Sagi Grimberg
  2026-04-30 13:38 ` Chuck Lever
  0 siblings, 1 reply; 14+ messages in thread
From: Sagi Grimberg @ 2026-04-30 13:32 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing List

Hey Chuck,

Upstream ktls-utils fails passing client certificate and private key 
using the .nfs keyring.
Bisecting leads commit facd084e43fc ("tlshd: Client-side dual 
certificate support").

I manually apply this (probably wrong) change and keyring works:
--
diff --git a/src/tlshd/client.c b/src/tlshd/client.c
index 2664ffb..a946797 100644
--- a/src/tlshd/client.c
+++ b/src/tlshd/client.c
@@ -327,7 +327,7 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session,
         } else {
                 tlshd_log_debug("%s: Selecting x509.certificate from 
conf file", __func__);
                 *pcert_length = tlshd_certs_len;
-               *pcert = tlshd_certs + tlshd_pq_certs_len;
+               *pcert = tlshd_certs;
                 *privkey = tlshd_privkey;
         }
         return 0;
--

But, I have a feeling its not the correct change...

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-04-30 13:32 Breakage in ktls-utils with nfs keyring? Sagi Grimberg
@ 2026-04-30 13:38 ` Chuck Lever
  2026-05-01 19:58   ` [PATCH] tlshd: fix keyring cert retrieval Scott Mayhew
  2026-05-01 20:19   ` Breakage in ktls-utils with nfs keyring? Scott Mayhew
  0 siblings, 2 replies; 14+ messages in thread
From: Chuck Lever @ 2026-04-30 13:38 UTC (permalink / raw)
  To: Sagi Grimberg, Chuck Lever, Scott Mayhew
  Cc: Linux NFS Mailing List, kernel-tls-handshake

Cc'ing the ktls-utils development list.

On Thu, Apr 30, 2026, at 9:32 AM, Sagi Grimberg wrote:
> Hey Chuck,
>
> Upstream ktls-utils fails passing client certificate and private key 
> using the .nfs keyring.
> Bisecting leads commit facd084e43fc ("tlshd: Client-side dual 
> certificate support").
>
> I manually apply this (probably wrong) change and keyring works:
> --
> diff --git a/src/tlshd/client.c b/src/tlshd/client.c
> index 2664ffb..a946797 100644
> --- a/src/tlshd/client.c
> +++ b/src/tlshd/client.c
> @@ -327,7 +327,7 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session,
>          } else {
>                  tlshd_log_debug("%s: Selecting x509.certificate from 
> conf file", __func__);
>                  *pcert_length = tlshd_certs_len;
> -               *pcert = tlshd_certs + tlshd_pq_certs_len;
> +               *pcert = tlshd_certs;
>                  *privkey = tlshd_privkey;
>          }
>          return 0;
> --
>
> But, I have a feeling its not the correct change...


Scott, can you triage this?


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] tlshd: fix keyring cert retrieval
  2026-04-30 13:38 ` Chuck Lever
@ 2026-05-01 19:58   ` Scott Mayhew
  2026-05-03  7:30     ` Sagi Grimberg
  2026-05-01 20:19   ` Breakage in ktls-utils with nfs keyring? Scott Mayhew
  1 sibling, 1 reply; 14+ messages in thread
From: Scott Mayhew @ 2026-05-01 19:58 UTC (permalink / raw)
  To: cel, sagi; +Cc: linux-nfs, kernel-tls-handshake

The code that gets certs from keyrings currently only gets RSA certs, so
we need to zero out the PQ certs length fields when a keyring is used.
Otherwise the retrieval callback will look in the wrong offset in the
tlshd_certs list.

Reported-by: Sagi Grimberg <sagi@grimberg.me>
Fixes: facd084 ("tlshd: Client-side dual certificate support")
Fixes: 14f5349 ("tlshd: Server-side dual certificate support")
Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 src/tlshd/client.c | 4 +++-
 src/tlshd/server.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/tlshd/client.c b/src/tlshd/client.c
index 2664ffb..f291ee5 100644
--- a/src/tlshd/client.c
+++ b/src/tlshd/client.c
@@ -195,9 +195,11 @@ static gnutls_pk_algorithm_t tlshd_pq_pkalg = GNUTLS_PK_UNKNOWN;
  */
 static bool tlshd_x509_client_get_certs(struct tlshd_handshake_parms *parms)
 {
-	if (parms->x509_cert != TLS_NO_CERT)
+	if (parms->x509_cert != TLS_NO_CERT) {
+		tlshd_pq_certs_len = 0;
 		return tlshd_keyring_get_certs(parms->x509_cert, tlshd_certs,
 					       &tlshd_certs_len);
+	}
 	return tlshd_config_get_certs(PEER_TYPE_CLIENT, tlshd_certs,
 				      &tlshd_pq_certs_len, &tlshd_certs_len,
 				      &tlshd_pq_pkalg);
diff --git a/src/tlshd/server.c b/src/tlshd/server.c
index 8e0f192..e8fe5c8 100644
--- a/src/tlshd/server.c
+++ b/src/tlshd/server.c
@@ -92,10 +92,12 @@ static gnutls_pk_algorithm_t tlshd_server_pq_pkalg = GNUTLS_PK_UNKNOWN;
  */
 static bool tlshd_x509_server_get_certs(struct tlshd_handshake_parms *parms)
 {
-	if (parms->x509_cert != TLS_NO_CERT)
+	if (parms->x509_cert != TLS_NO_CERT) {
+		tlshd_server_pq_certs_len = 0;
 		return tlshd_keyring_get_certs(parms->x509_cert,
 					       tlshd_server_certs,
 					       &tlshd_server_certs_len);
+	}
 	return tlshd_config_get_certs(PEER_TYPE_SERVER, tlshd_server_certs,
 				      &tlshd_server_pq_certs_len,
 				      &tlshd_server_certs_len,
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-04-30 13:38 ` Chuck Lever
  2026-05-01 19:58   ` [PATCH] tlshd: fix keyring cert retrieval Scott Mayhew
@ 2026-05-01 20:19   ` Scott Mayhew
  2026-05-02  3:08     ` Chuck Lever
  1 sibling, 1 reply; 14+ messages in thread
From: Scott Mayhew @ 2026-05-01 20:19 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Sagi Grimberg, Chuck Lever, Linux NFS Mailing List,
	kernel-tls-handshake

On Thu, 30 Apr 2026, Chuck Lever wrote:

> Cc'ing the ktls-utils development list.
> 
> On Thu, Apr 30, 2026, at 9:32 AM, Sagi Grimberg wrote:
> > Hey Chuck,
> >
> > Upstream ktls-utils fails passing client certificate and private key 
> > using the .nfs keyring.
> > Bisecting leads commit facd084e43fc ("tlshd: Client-side dual 
> > certificate support").
> >
> > I manually apply this (probably wrong) change and keyring works:
> > --
> > diff --git a/src/tlshd/client.c b/src/tlshd/client.c
> > index 2664ffb..a946797 100644
> > --- a/src/tlshd/client.c
> > +++ b/src/tlshd/client.c
> > @@ -327,7 +327,7 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session,
> >          } else {
> >                  tlshd_log_debug("%s: Selecting x509.certificate from 
> > conf file", __func__);
> >                  *pcert_length = tlshd_certs_len;
> > -               *pcert = tlshd_certs + tlshd_pq_certs_len;
> > +               *pcert = tlshd_certs;
> >                  *privkey = tlshd_privkey;
> >          }
> >          return 0;
> > --
> >
> > But, I have a feeling its not the correct change...
> 
> 
> Scott, can you triage this?

So when I added the dual certificate support, I didn't touch any of the
keyring code.  Frankly, I'm not entirely sure what is the right way to
set it up and the docs are pretty much nonexistent.  As far as I can
tell:

- you need to load nfs.ko first so that the .nfs keyring gets created
  via nfs_init_keyring()
- you need to restart tlshd so that it links the .nfs keyring into its
  session keyring (I tried loading nfs.ko at boot via modules-load.d,
  but tlshd still reported an error saying it couldn't find the .nfs
  keyring)
- you need to convert the cert and key to DER format
- you need to add the cert and key to the .nfs keyring, e.g.

  keyctl padd user "nfs_cert" %:.nfs < smayhew-rawhide.crt.der
  keyctl padd user "nfs_key" %:.nfs < smayhew-rawhide.key.der

- then you mount w/ '-o xprtsec=mtls,cert_serial=...,privkey_serial=...'

Is that somewhat accurate?  Is there a better way to do it?  It seems
like a lot more work than just using the config file.

At any rate, I was able to reproduce the reported bug and the patch I
just sent fixes it, but I think we probably want to make dual
certificate support work with keyrings too.  What's the right way to go
about that?  Add PQ cert and PQ key parameters to the upcall?  Or add
lists of both PQ and RSA certs and private keys to the existing keys
and teach tlshd to parse both out of the existing keys (which I'm not
sure is possible)?

Also, is nfsd supposed to work with keyrings?  I see that tlshd looks
for a .nfsd keyring, but svc_tcp_handshake() doesn't populate ta_my_cert
and ta_my_privkey...

-Scott
> 
> 
> -- 
> Chuck Lever
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-05-01 20:19   ` Breakage in ktls-utils with nfs keyring? Scott Mayhew
@ 2026-05-02  3:08     ` Chuck Lever
  2026-05-03  7:48       ` Sagi Grimberg
  0 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2026-05-02  3:08 UTC (permalink / raw)
  To: Scott Mayhew
  Cc: Sagi Grimberg, Chuck Lever, Linux NFS Mailing List,
	kernel-tls-handshake



On Fri, May 1, 2026, at 4:19 PM, Scott Mayhew wrote:
> On Thu, 30 Apr 2026, Chuck Lever wrote:
>
>> Cc'ing the ktls-utils development list.
>> 
>> On Thu, Apr 30, 2026, at 9:32 AM, Sagi Grimberg wrote:
>> > Hey Chuck,
>> >
>> > Upstream ktls-utils fails passing client certificate and private key 
>> > using the .nfs keyring.
>> > Bisecting leads commit facd084e43fc ("tlshd: Client-side dual 
>> > certificate support").
>> >
>> > I manually apply this (probably wrong) change and keyring works:
>> > --
>> > diff --git a/src/tlshd/client.c b/src/tlshd/client.c
>> > index 2664ffb..a946797 100644
>> > --- a/src/tlshd/client.c
>> > +++ b/src/tlshd/client.c
>> > @@ -327,7 +327,7 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session,
>> >          } else {
>> >                  tlshd_log_debug("%s: Selecting x509.certificate from 
>> > conf file", __func__);
>> >                  *pcert_length = tlshd_certs_len;
>> > -               *pcert = tlshd_certs + tlshd_pq_certs_len;
>> > +               *pcert = tlshd_certs;
>> >                  *privkey = tlshd_privkey;
>> >          }
>> >          return 0;
>> > --
>> >
>> > But, I have a feeling its not the correct change...
>> 
>> 
>> Scott, can you triage this?
>
> So when I added the dual certificate support, I didn't touch any of the
> keyring code.  Frankly, I'm not entirely sure what is the right way to
> set it up and the docs are pretty much nonexistent.  As far as I can
> tell:
>
> - you need to load nfs.ko first so that the .nfs keyring gets created
>   via nfs_init_keyring()
> - you need to restart tlshd so that it links the .nfs keyring into its
>   session keyring (I tried loading nfs.ko at boot via modules-load.d,
>   but tlshd still reported an error saying it couldn't find the .nfs
>   keyring)
> - you need to convert the cert and key to DER format
> - you need to add the cert and key to the .nfs keyring, e.g.
>
>   keyctl padd user "nfs_cert" %:.nfs < smayhew-rawhide.crt.der
>   keyctl padd user "nfs_key" %:.nfs < smayhew-rawhide.key.der
>
> - then you mount w/ '-o xprtsec=mtls,cert_serial=...,privkey_serial=...'
>
> Is that somewhat accurate?  Is there a better way to do it?  It seems
> like a lot more work than just using the config file.

It is more work because keyring support for the NFS consumers is still
aspirational/experimental.

I've pushed your patch to a "fixes" branch for folks to try out. I'm not
sure yet whether we want a 1.4.1 release with this fix, since keyring
support for NFS is "not finished".


> At any rate, I was able to reproduce the reported bug and the patch I
> just sent fixes it, but I think we probably want to make dual
> certificate support work with keyrings too.

Yes, and clearly there are some questions to be answered.


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tlshd: fix keyring cert retrieval
  2026-05-01 19:58   ` [PATCH] tlshd: fix keyring cert retrieval Scott Mayhew
@ 2026-05-03  7:30     ` Sagi Grimberg
  0 siblings, 0 replies; 14+ messages in thread
From: Sagi Grimberg @ 2026-05-03  7:30 UTC (permalink / raw)
  To: Scott Mayhew, cel; +Cc: linux-nfs, kernel-tls-handshake



On 01/05/2026 22:58, Scott Mayhew wrote:
> The code that gets certs from keyrings currently only gets RSA certs, so
> we need to zero out the PQ certs length fields when a keyring is used.
> Otherwise the retrieval callback will look in the wrong offset in the
> tlshd_certs list.
>
> Reported-by: Sagi Grimberg <sagi@grimberg.me>
> Fixes: facd084 ("tlshd: Client-side dual certificate support")
> Fixes: 14f5349 ("tlshd: Server-side dual certificate support")
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-05-02  3:08     ` Chuck Lever
@ 2026-05-03  7:48       ` Sagi Grimberg
  2026-05-03 19:11         ` Chuck Lever
  0 siblings, 1 reply; 14+ messages in thread
From: Sagi Grimberg @ 2026-05-03  7:48 UTC (permalink / raw)
  To: Chuck Lever, Scott Mayhew
  Cc: Chuck Lever, Linux NFS Mailing List, kernel-tls-handshake



On 02/05/2026 6:08, Chuck Lever wrote:
>
> On Fri, May 1, 2026, at 4:19 PM, Scott Mayhew wrote:
>> On Thu, 30 Apr 2026, Chuck Lever wrote:
>>
>>> Cc'ing the ktls-utils development list.
>>>
>>> On Thu, Apr 30, 2026, at 9:32 AM, Sagi Grimberg wrote:
>>>> Hey Chuck,
>>>>
>>>> Upstream ktls-utils fails passing client certificate and private key
>>>> using the .nfs keyring.
>>>> Bisecting leads commit facd084e43fc ("tlshd: Client-side dual
>>>> certificate support").
>>>>
>>>> I manually apply this (probably wrong) change and keyring works:
>>>> --
>>>> diff --git a/src/tlshd/client.c b/src/tlshd/client.c
>>>> index 2664ffb..a946797 100644
>>>> --- a/src/tlshd/client.c
>>>> +++ b/src/tlshd/client.c
>>>> @@ -327,7 +327,7 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session,
>>>>           } else {
>>>>                   tlshd_log_debug("%s: Selecting x509.certificate from
>>>> conf file", __func__);
>>>>                   *pcert_length = tlshd_certs_len;
>>>> -               *pcert = tlshd_certs + tlshd_pq_certs_len;
>>>> +               *pcert = tlshd_certs;
>>>>                   *privkey = tlshd_privkey;
>>>>           }
>>>>           return 0;
>>>> --
>>>>
>>>> But, I have a feeling its not the correct change...
>>>
>>> Scott, can you triage this?
>> So when I added the dual certificate support, I didn't touch any of the
>> keyring code.  Frankly, I'm not entirely sure what is the right way to
>> set it up and the docs are pretty much nonexistent.  As far as I can
>> tell:
>>
>> - you need to load nfs.ko first so that the .nfs keyring gets created
>>    via nfs_init_keyring()
>> - you need to restart tlshd so that it links the .nfs keyring into its
>>    session keyring (I tried loading nfs.ko at boot via modules-load.d,
>>    but tlshd still reported an error saying it couldn't find the .nfs
>>    keyring)
>> - you need to convert the cert and key to DER format
>> - you need to add the cert and key to the .nfs keyring, e.g.
>>
>>    keyctl padd user "nfs_cert" %:.nfs < smayhew-rawhide.crt.der
>>    keyctl padd user "nfs_key" %:.nfs < smayhew-rawhide.key.der
>>
>> - then you mount w/ '-o xprtsec=mtls,cert_serial=...,privkey_serial=...'
>>
>> Is that somewhat accurate?

It is.

>>    Is there a better way to do it?

Have a script/automation SW.

>>   It seems
>> like a lot more work than just using the config file.

Well in some cases, storing credentials on a persistent file is not a 
viable option.
For nvme there is a userspace utility that helps with this to some extent.

> It is more work because keyring support for the NFS consumers is still
> aspirational/experimental.

Can you elaborate? I think people expect to be able to pass certs/keys to
tlshd the .nfs keyring. Also I expected it to work (as it used to).

>
> I've pushed your patch to a "fixes" branch for folks to try out. I'm not
> sure yet whether we want a 1.4.1 release with this fix, since keyring
> support for NFS is "not finished".

I understand that there are features that are not supported via the
keyring interface. But I think that users expect things that used to work to
continue working. My personal opinion is that releasing this fix is 
appropriate
given that this is a regression.

Is keyring support for NFS marked as "experimental" or "not finished" 
anywhere?
[I should register to kernel-tls-handshake mailing list]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-05-03  7:48       ` Sagi Grimberg
@ 2026-05-03 19:11         ` Chuck Lever
  2026-05-03 20:37           ` Sagi Grimberg
  0 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2026-05-03 19:11 UTC (permalink / raw)
  To: Sagi Grimberg, Scott Mayhew
  Cc: Chuck Lever, Linux NFS Mailing List, kernel-tls-handshake



On Sun, May 3, 2026, at 9:48 AM, Sagi Grimberg wrote:
> On 02/05/2026 6:08, Chuck Lever wrote:
>>
>> On Fri, May 1, 2026, at 4:19 PM, Scott Mayhew wrote:
>>> On Thu, 30 Apr 2026, Chuck Lever wrote:
>>>
>>>> Cc'ing the ktls-utils development list.
>>>>
>>>> On Thu, Apr 30, 2026, at 9:32 AM, Sagi Grimberg wrote:
>>>>> Hey Chuck,
>>>>>
>>>>> Upstream ktls-utils fails passing client certificate and private key
>>>>> using the .nfs keyring.
>>>>> Bisecting leads commit facd084e43fc ("tlshd: Client-side dual
>>>>> certificate support").
>>>>>
>>>>> I manually apply this (probably wrong) change and keyring works:
>>>>> --
>>>>> diff --git a/src/tlshd/client.c b/src/tlshd/client.c
>>>>> index 2664ffb..a946797 100644
>>>>> --- a/src/tlshd/client.c
>>>>> +++ b/src/tlshd/client.c
>>>>> @@ -327,7 +327,7 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session,
>>>>>           } else {
>>>>>                   tlshd_log_debug("%s: Selecting x509.certificate from
>>>>> conf file", __func__);
>>>>>                   *pcert_length = tlshd_certs_len;
>>>>> -               *pcert = tlshd_certs + tlshd_pq_certs_len;
>>>>> +               *pcert = tlshd_certs;
>>>>>                   *privkey = tlshd_privkey;
>>>>>           }
>>>>>           return 0;
>>>>> --
>>>>>
>>>>> But, I have a feeling its not the correct change...
>>>>
>>>> Scott, can you triage this?
>>> So when I added the dual certificate support, I didn't touch any of the
>>> keyring code.  Frankly, I'm not entirely sure what is the right way to
>>> set it up and the docs are pretty much nonexistent.  As far as I can
>>> tell:
>>>
>>> - you need to load nfs.ko first so that the .nfs keyring gets created
>>>    via nfs_init_keyring()
>>> - you need to restart tlshd so that it links the .nfs keyring into its
>>>    session keyring (I tried loading nfs.ko at boot via modules-load.d,
>>>    but tlshd still reported an error saying it couldn't find the .nfs
>>>    keyring)
>>> - you need to convert the cert and key to DER format
>>> - you need to add the cert and key to the .nfs keyring, e.g.
>>>
>>>    keyctl padd user "nfs_cert" %:.nfs < smayhew-rawhide.crt.der
>>>    keyctl padd user "nfs_key" %:.nfs < smayhew-rawhide.key.der
>>>
>>> - then you mount w/ '-o xprtsec=mtls,cert_serial=...,privkey_serial=...'
>>>
>>> Is that somewhat accurate?
>
> It is.
>
>>>    Is there a better way to do it?
>
> Have a script/automation SW.

Our intention is to have mount.nfs pick up some of this work.

We need a solution that can pick up a different certificate for each
network namespace, for instance. And we want to enable certificate
storage in the system's TPM someday.

And this needs to be made reliable relative to module load order.


>>>   It seems
>>> like a lot more work than just using the config file.
>
> Well in some cases, storing credentials on a persistent file is not a 
> viable option.
> For nvme there is a userspace utility that helps with this to some extent.

This is because handling an NVMe PSK in the keyring is a first-class,
supported mechanism. Handling the x.509 certificate this way hasn't
really been thought through.

>> It is more work because keyring support for the NFS consumers is still
>> aspirational/experimental.
>
> Can you elaborate? I think people expect to be able to pass certs/keys to
> tlshd the .nfs keyring. Also I expected it to work (as it used to).

The "cert_serial" and "privkey_serial" mount options are not documented
at all in nfs(5). They are intended to be a way for the mount.nfs command
to pass key serial numbers to the kernel NFS client, not as an
administrative interface. Because, yuck.

This doesn't mean we don't want to support using a keyring for x.509
certificates on NFS mounts. It means the capability isn't finished yet.


>> I've pushed your patch to a "fixes" branch for folks to try out. I'm not
>> sure yet whether we want a 1.4.1 release with this fix, since keyring
>> support for NFS is "not finished".
>
> I understand that there are features that are not supported via the
> keyring interface. But I think that users expect things that used to work to
> continue working. My personal opinion is that releasing this fix is 
> appropriate
> given that this is a regression.

You are the first user I know of for this capability.

Yes, technically it's a regression, but it's not really a feature that
is supposed to be ready for users at this stage.

You are also building tlshd from scratch rather than using a distro-
packaged version of it. That's rare enough, but it also means you can
apply the patch that fixes the issue and build it yourself.

I'm open to considering a dot-release, but you haven't convinced me yet.


> Is keyring support for NFS marked as "experimental" or "not finished" 
> anywhere?

Where would we add such a marking? nfs(5) doesn't document either of
those mount options.

Patches and architecture are welcome. As I said, we want this to work
eventually.


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-05-03 19:11         ` Chuck Lever
@ 2026-05-03 20:37           ` Sagi Grimberg
  2026-05-04  6:44             ` Chuck Lever
  0 siblings, 1 reply; 14+ messages in thread
From: Sagi Grimberg @ 2026-05-03 20:37 UTC (permalink / raw)
  To: Chuck Lever, Scott Mayhew
  Cc: Chuck Lever, Linux NFS Mailing List, kernel-tls-handshake



On 03/05/2026 22:11, Chuck Lever wrote:
>
> On Sun, May 3, 2026, at 9:48 AM, Sagi Grimberg wrote:
>> On 02/05/2026 6:08, Chuck Lever wrote:
>>> On Fri, May 1, 2026, at 4:19 PM, Scott Mayhew wrote:
>>>> On Thu, 30 Apr 2026, Chuck Lever wrote:
>>>>
>>>>> Cc'ing the ktls-utils development list.
>>>>>
>>>>> On Thu, Apr 30, 2026, at 9:32 AM, Sagi Grimberg wrote:
>>>>>> Hey Chuck,
>>>>>>
>>>>>> Upstream ktls-utils fails passing client certificate and private key
>>>>>> using the .nfs keyring.
>>>>>> Bisecting leads commit facd084e43fc ("tlshd: Client-side dual
>>>>>> certificate support").
>>>>>>
>>>>>> I manually apply this (probably wrong) change and keyring works:
>>>>>> --
>>>>>> diff --git a/src/tlshd/client.c b/src/tlshd/client.c
>>>>>> index 2664ffb..a946797 100644
>>>>>> --- a/src/tlshd/client.c
>>>>>> +++ b/src/tlshd/client.c
>>>>>> @@ -327,7 +327,7 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session,
>>>>>>            } else {
>>>>>>                    tlshd_log_debug("%s: Selecting x509.certificate from
>>>>>> conf file", __func__);
>>>>>>                    *pcert_length = tlshd_certs_len;
>>>>>> -               *pcert = tlshd_certs + tlshd_pq_certs_len;
>>>>>> +               *pcert = tlshd_certs;
>>>>>>                    *privkey = tlshd_privkey;
>>>>>>            }
>>>>>>            return 0;
>>>>>> --
>>>>>>
>>>>>> But, I have a feeling its not the correct change...
>>>>> Scott, can you triage this?
>>>> So when I added the dual certificate support, I didn't touch any of the
>>>> keyring code.  Frankly, I'm not entirely sure what is the right way to
>>>> set it up and the docs are pretty much nonexistent.  As far as I can
>>>> tell:
>>>>
>>>> - you need to load nfs.ko first so that the .nfs keyring gets created
>>>>     via nfs_init_keyring()
>>>> - you need to restart tlshd so that it links the .nfs keyring into its
>>>>     session keyring (I tried loading nfs.ko at boot via modules-load.d,
>>>>     but tlshd still reported an error saying it couldn't find the .nfs
>>>>     keyring)
>>>> - you need to convert the cert and key to DER format
>>>> - you need to add the cert and key to the .nfs keyring, e.g.
>>>>
>>>>     keyctl padd user "nfs_cert" %:.nfs < smayhew-rawhide.crt.der
>>>>     keyctl padd user "nfs_key" %:.nfs < smayhew-rawhide.key.der
>>>>
>>>> - then you mount w/ '-o xprtsec=mtls,cert_serial=...,privkey_serial=...'
>>>>
>>>> Is that somewhat accurate?
>> It is.
>>
>>>>     Is there a better way to do it?
>> Have a script/automation SW.
> Our intention is to have mount.nfs pick up some of this work.

That would be a nice addition.

> We need a solution that can pick up a different certificate for each
> network namespace, for instance. And we want to enable certificate
> storage in the system's TPM someday.
>
> And this needs to be made reliable relative to module load order.

I support everything you said.

>>>>    It seems
>>>> like a lot more work than just using the config file.
>> Well in some cases, storing credentials on a persistent file is not a
>> viable option.
>> For nvme there is a userspace utility that helps with this to some extent.
> This is because handling an NVMe PSK in the keyring is a first-class,
> supported mechanism. Handling the x.509 certificate this way hasn't
> really been thought through.

What makes NVMe PSK more "supported" than x.509? Both rely on userspace
to create keys using a keyring (either a well-known keyring, or some 
user created keyring)
and pass it to the kernel as parameters (either comma-separate-string to 
/dev/nvme-fabrics
or as a comma-separated-string to mount.nfs). These are not different IMO.

Unlike NFS, the NVMe spec defines standard identities (host and 
subsystem qualified names) and
shared secret format as well as naming conventions. Hence nvme-cli 
provides a nice interface that
does not force the user to open and read the specification. nvme-cli TLS 
helpers primary purpose
is not really to save the user adding a key to a keyring and adding it 
as a parameter to the driver
connection string, that is just a nice by-product afaik.

>
>>> It is more work because keyring support for the NFS consumers is still
>>> aspirational/experimental.
>> Can you elaborate? I think people expect to be able to pass certs/keys to
>> tlshd the .nfs keyring. Also I expected it to work (as it used to).
> The "cert_serial" and "privkey_serial" mount options are not documented
> at all in nfs(5).

I thought this was an oversight?

>   They are intended to be a way for the mount.nfs command
> to pass key serial numbers to the kernel NFS client, not as an
> administrative interface. Because, yuck.

I agree this can be made nicer with passing a key identity perhaps (although
it can create also some annoyance with how flexible key identities can be).
In NVMe both options are supported.

The way I see it, use of a keyring most likely mean users rely on some
automation software to populate it anyways.

> This doesn't mean we don't want to support using a keyring for x.509
> certificates on NFS mounts. It means the capability isn't finished yet.

I understand. But it does exist and I know of multiple users of it.

>>> I've pushed your patch to a "fixes" branch for folks to try out. I'm not
>>> sure yet whether we want a 1.4.1 release with this fix, since keyring
>>> support for NFS is "not finished".
>> I understand that there are features that are not supported via the
>> keyring interface. But I think that users expect things that used to work to
>> continue working. My personal opinion is that releasing this fix is
>> appropriate
>> given that this is a regression.
> You are the first user I know of for this capability.

The first that you know :)
Definitely not the only one.

> Yes, technically it's a regression, but it's not really a feature that
> is supposed to be ready for users at this stage.
>
> You are also building tlshd from scratch rather than using a distro-
> packaged version of it. That's rare enough, but it also means you can
> apply the patch that fixes the issue and build it yourself.

This breakage was brought to my attention by a user working on
Ubuntu24.04 ktls-utils (1.3.0). It'd be better if we'd caught it sooner...

> I'm open to considering a dot-release, but you haven't convinced me yet.

Ultimately it's your call Chuck. But IMO we shouldn't hold out a fix
for this until we are happy with a nicer mount.nfs interface.

>> Is keyring support for NFS marked as "experimental" or "not finished"
>> anywhere?
> Where would we add such a marking? nfs(5) doesn't document either of
> those mount options.

Yea, I agree.

> Patches and architecture are welcome. As I said, we want this to work
> eventually.

 From my perspective (up until this conversation at least) it used to work.
I was not aware of any dissatisfaction with the interface.

Anyways, would be happy to contribute to this (don't know anything
about the pq stuff though)...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-05-03 20:37           ` Sagi Grimberg
@ 2026-05-04  6:44             ` Chuck Lever
  2026-05-04  8:02               ` Sagi Grimberg
  2026-05-05  8:15               ` Chuck Lever
  0 siblings, 2 replies; 14+ messages in thread
From: Chuck Lever @ 2026-05-04  6:44 UTC (permalink / raw)
  To: Sagi Grimberg, Scott Mayhew
  Cc: Chuck Lever, Linux NFS Mailing List, kernel-tls-handshake



On Sun, May 3, 2026, at 10:37 PM, Sagi Grimberg wrote:
> On 03/05/2026 22:11, Chuck Lever wrote:
>>
>> On Sun, May 3, 2026, at 9:48 AM, Sagi Grimberg wrote:
>>> On 02/05/2026 6:08, Chuck Lever wrote:
>>>> On Fri, May 1, 2026, at 4:19 PM, Scott Mayhew wrote:
>>>>> On Thu, 30 Apr 2026, Chuck Lever wrote:

>>>>>    It seems
>>>>> like a lot more work than just using the config file.
>>> Well in some cases, storing credentials on a persistent file is not a
>>> viable option.
>>> For nvme there is a userspace utility that helps with this to some extent.
>> This is because handling an NVMe PSK in the keyring is a first-class,
>> supported mechanism. Handling the x.509 certificate this way hasn't
>> really been thought through.
>
> What makes NVMe PSK more "supported" than x.509?

Hannes contributed NVMe PSK in the beginning. IIUC PSK was the first
authentication mode available for the NVMe/TCP protocol. I'm not sure
we can say that x.509 is supported for our NVMe/TCP implementation,
though that is something that should be made to work someday.

Likewise for NFS and x.509 -- that was the easier authentication
mode to implement for RPC-with-TLS. Eventually we want to support
both.

It's simply a matter of development resources and priorities, there
is really no spec reason it cannot be done.


> The way I see it, use of a keyring most likely mean users rely on some
> automation software to populate it anyways.

Sure, but that software does not exist right now for NFS.

And with containery deployments, everyone likes to write their own
special scripts. Hard to say what exactly the nfs-utils-provided
pieces will need to implement.


>> You are also building tlshd from scratch rather than using a distro-
>> packaged version of it. That's rare enough, but it also means you can
>> apply the patch that fixes the issue and build it yourself.
>
> This breakage was brought to my attention by a user working on
> Ubuntu24.04 ktls-utils (1.3.0). It'd be better if we'd caught it sooner...

Full CI is something that is still in the works.


>> I'm open to considering a dot-release, but you haven't convinced me yet.
>
> Ultimately it's your call Chuck. But IMO we shouldn't hold out a fix
> for this until we are happy with a nicer mount.nfs interface.

I have to stop you there: That's completely not what I'm saying. No
one is holding back a fix -- it will be merged into the main branch
in a few days.

The question is whether this issue merits fresh upstream releases. As
I said, the capability isn't advertised, so at this time anyone who is
using this capability is doing so at their own risk. Whoever told you
this was a production-ready feature of the NFS client was mistaken. Can
you provide a key serial number on the mount command line? Yes. Is it
something that is tested and is the interface unchanging for all time?
No.

It wasn't clear that 1.3.0 was the problem. 1.4.0 was released just
last week, so that's where my attention was focused.

What you are asking for, then, is a 1.3.0 dot release for this fix. I
still don't feel there is a strong requirement for that, given that
distributions apply fixes to packages all the time. But I haven't made
a final call on that.


> Anyways, would be happy to contribute to this (don't know anything
> about the pq stuff though)...

Patches are absolutely welcome: post to kernel-tls-handshake, or
open PRs on github.


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-05-04  6:44             ` Chuck Lever
@ 2026-05-04  8:02               ` Sagi Grimberg
  2026-05-04  8:21                 ` Hannes Reinecke
  2026-05-05  8:15               ` Chuck Lever
  1 sibling, 1 reply; 14+ messages in thread
From: Sagi Grimberg @ 2026-05-04  8:02 UTC (permalink / raw)
  To: Chuck Lever, Scott Mayhew
  Cc: Chuck Lever, Linux NFS Mailing List, kernel-tls-handshake

>>> This is because handling an NVMe PSK in the keyring is a first-class,
>>> supported mechanism. Handling the x.509 certificate this way hasn't
>>> really been thought through.
>> What makes NVMe PSK more "supported" than x.509?
> Hannes contributed NVMe PSK in the beginning. IIUC PSK was the first
> authentication mode available for the NVMe/TCP protocol. I'm not sure
> we can say that x.509 is supported for our NVMe/TCP implementation,
> though that is something that should be made to work someday.

That depends if NVMe standardizes x.509, I am no longer a member of
the TWG so I don't know, but I agree that it would be very nice to have.

>
> Likewise for NFS and x.509 -- that was the easier authentication
> mode to implement for RPC-with-TLS. Eventually we want to support
> both.

OK.

>
> It's simply a matter of development resources and priorities, there
> is really no spec reason it cannot be done.

Agree. Although I am not aware of a need to use PSK over x.509 for NFS...

>> The way I see it, use of a keyring most likely mean users rely on some
>> automation software to populate it anyways.
> Sure, but that software does not exist right now for NFS.
>
> And with containery deployments, everyone likes to write their own
> special scripts. Hard to say what exactly the nfs-utils-provided
> pieces will need to implement.

OK. So just so I understand, what interface would you expect mount.nfs
to have for this? Or you expect rpcctl to provide an interface to create 
keys
with nice human-friendly identities which would in turn be referenced by the
nfs mount command?

>>> You are also building tlshd from scratch rather than using a distro-
>>> packaged version of it. That's rare enough, but it also means you can
>>> apply the patch that fixes the issue and build it yourself.
>> This breakage was brought to my attention by a user working on
>> Ubuntu24.04 ktls-utils (1.3.0). It'd be better if we'd caught it sooner...
> Full CI is something that is still in the works.
>
>
>>> I'm open to considering a dot-release, but you haven't convinced me yet.
>> Ultimately it's your call Chuck. But IMO we shouldn't hold out a fix
>> for this until we are happy with a nicer mount.nfs interface.
> I have to stop you there: That's completely not what I'm saying. No
> one is holding back a fix -- it will be merged into the main branch
> in a few days.

Bad choice of words :) didn't mean to say that you are against fixing it.

> The question is whether this issue merits fresh upstream releases. As
> I said, the capability isn't advertised, so at this time anyone who is
> using this capability is doing so at their own risk. Whoever told you
> this was a production-ready feature of the NFS client was mistaken.

I don't think anyone told me, The mount params exist in the kernel, and it
was working, so I just assumed that this is supported.

>   Can
> you provide a key serial number on the mount command line? Yes. Is it
> something that is tested and is the interface unchanging for all time?
> No.

Yea. The person leading the project was not aware that it is used by anyone
so of course it is not guaranteed to always work.

Perhaps we can discuss what do you think is needed to make it something
that is expected to work. For sure it needs to be documented in nfs(5), that
is a no-brainer. What else though? (my assumption is that TPM support
and per-namespace certs are not a hard requirement for the keyring feature?)

> It wasn't clear that 1.3.0 was the problem. 1.4.0 was released just
> last week, so that's where my attention was focused.
>
> What you are asking for, then, is a 1.3.0 dot release for this fix. I
> still don't feel there is a strong requirement for that, given that
> distributions apply fixes to packages all the time. But I haven't made
> a final call on that.

I suppose we could have users open bugs to their different distributions
which are exposed to this and have them fix it in their pkg. Or, tell users
to build from upstream/main. The question is what is the downside/effort of
making a dot release that contains this fix? That I do not know.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-05-04  8:02               ` Sagi Grimberg
@ 2026-05-04  8:21                 ` Hannes Reinecke
  0 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2026-05-04  8:21 UTC (permalink / raw)
  To: Sagi Grimberg, Chuck Lever, Scott Mayhew
  Cc: Chuck Lever, Linux NFS Mailing List, kernel-tls-handshake

On 5/4/26 10:02, Sagi Grimberg wrote:
>>>> This is because handling an NVMe PSK in the keyring is a first-class,
>>>> supported mechanism. Handling the x.509 certificate this way hasn't
>>>> really been thought through.
>>> What makes NVMe PSK more "supported" than x.509?
>> Hannes contributed NVMe PSK in the beginning. IIUC PSK was the first
>> authentication mode available for the NVMe/TCP protocol. I'm not sure
>> we can say that x.509 is supported for our NVMe/TCP implementation,
>> though that is something that should be made to work someday.
> 
> That depends if NVMe standardizes x.509, I am no longer a member of
> the TWG so I don't know, but I agree that it would be very nice to have.
> 
Sadly, this has been shelved for now.
There are no takers to move things forward.
So we're stuck with PSK for the time being.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-05-04  6:44             ` Chuck Lever
  2026-05-04  8:02               ` Sagi Grimberg
@ 2026-05-05  8:15               ` Chuck Lever
  2026-05-05  8:32                 ` Sagi Grimberg
  1 sibling, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2026-05-05  8:15 UTC (permalink / raw)
  To: Sagi Grimberg, Scott Mayhew
  Cc: Chuck Lever, Linux NFS Mailing List, kernel-tls-handshake


On Mon, May 4, 2026, at 8:44 AM, Chuck Lever wrote:
> What you are asking for, then, is a 1.3.0 dot release for this fix. I
> still don't feel there is a strong requirement for that, given that
> distributions apply fixes to packages all the time. But I haven't made
> a final call on that.

After auditing the commit history between ktls-utils-1.3.0 and 1.4.0,
there are enough fixes to bundle together and release a 1.3.1 that
includes the fix for the keyring-based NFS mount failure. I've created
a branch ktls-utils-1.3-fixes that can be tagged and released once the
keyring fix is merged into main.


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Breakage in ktls-utils with nfs keyring?
  2026-05-05  8:15               ` Chuck Lever
@ 2026-05-05  8:32                 ` Sagi Grimberg
  0 siblings, 0 replies; 14+ messages in thread
From: Sagi Grimberg @ 2026-05-05  8:32 UTC (permalink / raw)
  To: Chuck Lever, Scott Mayhew
  Cc: Chuck Lever, Linux NFS Mailing List, kernel-tls-handshake



On 05/05/2026 11:15, Chuck Lever wrote:
> On Mon, May 4, 2026, at 8:44 AM, Chuck Lever wrote:
>> What you are asking for, then, is a 1.3.0 dot release for this fix. I
>> still don't feel there is a strong requirement for that, given that
>> distributions apply fixes to packages all the time. But I haven't made
>> a final call on that.
> After auditing the commit history between ktls-utils-1.3.0 and 1.4.0,
> there are enough fixes to bundle together and release a 1.3.1 that
> includes the fix for the keyring-based NFS mount failure. I've created
> a branch ktls-utils-1.3-fixes that can be tagged and released once the
> keyring fix is merged into main.

Thanks Chuck.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-05-05  8:32 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-30 13:32 Breakage in ktls-utils with nfs keyring? Sagi Grimberg
2026-04-30 13:38 ` Chuck Lever
2026-05-01 19:58   ` [PATCH] tlshd: fix keyring cert retrieval Scott Mayhew
2026-05-03  7:30     ` Sagi Grimberg
2026-05-01 20:19   ` Breakage in ktls-utils with nfs keyring? Scott Mayhew
2026-05-02  3:08     ` Chuck Lever
2026-05-03  7:48       ` Sagi Grimberg
2026-05-03 19:11         ` Chuck Lever
2026-05-03 20:37           ` Sagi Grimberg
2026-05-04  6:44             ` Chuck Lever
2026-05-04  8:02               ` Sagi Grimberg
2026-05-04  8:21                 ` Hannes Reinecke
2026-05-05  8:15               ` Chuck Lever
2026-05-05  8:32                 ` Sagi Grimberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox