Re: Question regarding TLS Key update for NVMe-TCP protocol

Re: Question regarding TLS Key update for NVMe-TCP protocol

[Chuck Lever III]

Copying the ktls utils mailing list.

> On Dec 2, 2024, at 1:02 PM, Narayan Ayalasomayajula <Narayan.Ayalasomayajula@wdc.com> wrote:
> 
>  Sabrina, Frantisek, Chuck,
>  I am looking for guidance regarding the TLS PSK KeyUpdate functionality in the context of NVMe-TCP transport. I work with a team in Western digital that develops NVMe-oF target adapters that support NVMe-TCP as well as RoCev2 transport protocols  TLS 1.3 species the requirements for KeyUpdate to occur under some events (bytes-transferred or time-elapsed events). When either event occurs, say at a sender node, the sender transmits a KeyUpdate message to indicate that the cryptographic key needs to be updated.  For NVMe-TCP transport protocol, for a host to establish a Fabric Secure Channel, the following sequence occurs between the host and the target:
> ·      3-way TCP handshake
> ·      TLS session (this establishes a secure channel)
> ·      Fabric Connect for Admin queue ·      Fabric Connect for associated IO queues  I am not able to find a way for the Linux host to perform the TLS KeyUpdate when the transport protocol is NVMe-TCP. It appears that the only way to update the TLS keys for this scenario is by disconnecting the transport as well as TCP connections and re-establishing them using the new TLS keys.
>  I see that Sabrina and Frantisek were working on a patchset for the TLS KeyUpdate functionality over a year ago (and Chuck, Christoph Hellwig mentioned to me that you may have been involved in this functional area as well). I am trying to understand if Linux supports TLS KeyUpdate for NVMe-TCP. My understanding is that the only way for the TLS KeyUpdate to occur is by disconnecting the transport as well as TCP connections and re-establishing them using the new TLS keys.
>  https://patchwork.kernel.org/project/netdevbpf/cover/cover.1691584074.git.sd@queasysnail.net/
>  Is my understanding correct?
>  Thanks,
> Narayan


IMO your understanding is correct. Currently a transport disconnect
is the only way to handle this scenario. We recognize that key
update needs to work without incurring a transport disconnect.


--
Chuck Lever

Re: Question regarding TLS Key update for NVMe-TCP protocol

[Hannes Reinecke]

On 12/2/24 19:50, Chuck Lever III wrote:
> Copying the ktls utils mailing list.
> 
>> On Dec 2, 2024, at 1:02 PM, Narayan Ayalasomayajula <Narayan.Ayalasomayajula@wdc.com> wrote:
>>
>>   Sabrina, Frantisek, Chuck,
>>   I am looking for guidance regarding the TLS PSK KeyUpdate functionality in the context of
 >>   NVMe-TCP transport. I work with a team in Western digital that
 >>   develops NVMe-oF target adapters that support NVMe-TCP as well as
 >>   RoCev2 transport protocols  TLS 1.3 species the requirements for
 >>   KeyUpdate to occur under some events (bytes-transferred or time-
 >>   elapsed events). When either event occurs, say at a sender node,
 >>   the sender transmits a KeyUpdate message to indicate that the
 >>   cryptographic key needs to be updated.  For NVMe-TCP transport
 >>   protocol, for a host to establish a Fabric Secure Channel, the
 >>   following sequence occurs between the host and the target:>> · 
  3-way TCP handshake
>> ·      TLS session (this establishes a secure channel)
>> ·      Fabric Connect for Admin queue ·      Fabric Connect for associated IO queues
 >>   I am not able to find a way for the Linux host to perform the TLS
 >> KeyUpdate when the transport protocol is NVMe-TCP. It appears that
 >> the only way to update the TLS keys for this scenario is by
 >> disconnecting the transport as well as TCP connections and re-
 >> establishing them using the new TLS keys.>>   I see that Sabrina and 
Frantisek were working on a patchset for the TLS KeyUpdate
 >>   functionality over a year ago (and Chuck, Christoph Hellwig
 >>   mentioned to me that you may have been involved in this functional
 >>   area as well). I am trying to understand if Linux supports TLS
 >>   KeyUpdate for NVMe-TCP. My understanding is that the only way for
 >>   the TLS KeyUpdate to occur is by disconnecting the transport as
 >>   well as TCP connections and re-establishing them using the new TLS
 >>   keys.>> 
https://patchwork.kernel.org/project/netdevbpf/cover/cover.1691584074.git.sd@queasysnail.net/
>>   Is my understanding correct?
>>   Thanks,
>> Narayan
> 
> 
> IMO your understanding is correct. Currently a transport disconnect
> is the only way to handle this scenario. We recognize that key
> update needs to work without incurring a transport disconnect.
> 
> 
Well, actually, not.
It was a design choice quite early on when developing NVMe-over-TLS that
we will _not_ do transparent key updates.
Currently the only error-recovery mechanism NVMe-over-Fabrics has is a 
controller reset, which for fabrics means a complete teardown of all 
connections, including a TLS key-renegotiation.
A transparent TLS key update would only improve the situation if we 
could make it completely transparent, ie that the actual connection
would stay alive and would just be blocked for the time of the key
update itself.
But the current in-kernel TLS implementation doesn't allow to change
the cipher ( cf net/tls/tls_main.c: do_tls_setsockopt_conf() ), so
the only chance here is to close the connection and start from scratch.
And that means the a key update currently requires a connection reset,
so we might as well just do a controller reset for a TLS key rotation.

And also note for secure concatenation a connection reset is built into
the protocol, so again a key rotation requires a complete connection
reset.

Please don't hestitate to contact me for further questions.

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: Question regarding TLS Key update for NVMe-TCP protocol

[Chuck Lever III]

> On Dec 3, 2024, at 2:17 AM, Hannes Reinecke <hare@suse.de> wrote:
> 
> On 12/2/24 19:50, Chuck Lever III wrote:
>> Copying the ktls utils mailing list.
>>> On Dec 2, 2024, at 1:02 PM, Narayan Ayalasomayajula <Narayan.Ayalasomayajula@wdc.com> wrote:
>>> 
>>>  Sabrina, Frantisek, Chuck,
>>>  I am looking for guidance regarding the TLS PSK KeyUpdate functionality in the context of
> >>   NVMe-TCP transport. I work with a team in Western digital that
> >>   develops NVMe-oF target adapters that support NVMe-TCP as well as
> >>   RoCev2 transport protocols  TLS 1.3 species the requirements for
> >>   KeyUpdate to occur under some events (bytes-transferred or time-
> >>   elapsed events). When either event occurs, say at a sender node,
> >>   the sender transmits a KeyUpdate message to indicate that the
> >>   cryptographic key needs to be updated.  For NVMe-TCP transport
> >>   protocol, for a host to establish a Fabric Secure Channel, the
> >>   following sequence occurs between the host and the target:>> ·  3-way TCP handshake
>>> ·      TLS session (this establishes a secure channel)
>>> ·      Fabric Connect for Admin queue ·      Fabric Connect for associated IO queues
> >>   I am not able to find a way for the Linux host to perform the TLS
> >> KeyUpdate when the transport protocol is NVMe-TCP. It appears that
> >> the only way to update the TLS keys for this scenario is by
> >> disconnecting the transport as well as TCP connections and re-
> >> establishing them using the new TLS keys.>>   I see that Sabrina and Frantisek were working on a patchset for the TLS KeyUpdate
> >>   functionality over a year ago (and Chuck, Christoph Hellwig
> >>   mentioned to me that you may have been involved in this functional
> >>   area as well). I am trying to understand if Linux supports TLS
> >>   KeyUpdate for NVMe-TCP. My understanding is that the only way for
> >>   the TLS KeyUpdate to occur is by disconnecting the transport as
> >>   well as TCP connections and re-establishing them using the new TLS
> >>   keys.>> https://patchwork.kernel.org/project/netdevbpf/cover/cover.1691584074.git.sd@queasysnail.net/
>>>  Is my understanding correct?
>>>  Thanks,
>>> Narayan
>> IMO your understanding is correct. Currently a transport disconnect
>> is the only way to handle this scenario. We recognize that key
>> update needs to work without incurring a transport disconnect.
> Well, actually, not.
> It was a design choice quite early on when developing NVMe-over-TLS that
> we will _not_ do transparent key updates.

OK. I was speaking only of KeyUpdate support in tlshd, which
manages the TLS handshake. I didn't realize that NVMe-o-TLS
was more restrictive.


> Currently the only error-recovery mechanism NVMe-over-Fabrics has is a controller reset, which for fabrics means a complete teardown of all connections, including a TLS key-renegotiation.
> A transparent TLS key update would only improve the situation if we could make it completely transparent, ie that the actual connection
> would stay alive and would just be blocked for the time of the key
> update itself.
> But the current in-kernel TLS implementation doesn't allow to change
> the cipher ( cf net/tls/tls_main.c: do_tls_setsockopt_conf() ), so
> the only chance here is to close the connection and start from scratch.

Have we asked Boris and friends if it would be possible to
implement KeyUpdate via ktls? Just curious.


> And that means the a key update currently requires a connection reset,
> so we might as well just do a controller reset for a TLS key rotation.
> 
> And also note for secure concatenation a connection reset is built into
> the protocol, so again a key rotation requires a complete connection
> reset.
> 
> Please don't hestitate to contact me for further questions.
> 
> 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

--
Chuck Lever

Re: Question regarding TLS Key update for NVMe-TCP protocol

[Hannes Reinecke]

On 12/3/24 15:33, Chuck Lever III wrote:
> 
> 
>> On Dec 3, 2024, at 2:17 AM, Hannes Reinecke <hare@suse.de> wrote:
>>
>> On 12/2/24 19:50, Chuck Lever III wrote:
>>> Copying the ktls utils mailing list.
>>>> On Dec 2, 2024, at 1:02 PM, Narayan Ayalasomayajula <Narayan.Ayalasomayajula@wdc.com> wrote:
>>>>
>>>>   Sabrina, Frantisek, Chuck,
>>>>   I am looking for guidance regarding the TLS PSK KeyUpdate functionality in the context of
>>>>    NVMe-TCP transport. I work with a team in Western digital that
>>>>    develops NVMe-oF target adapters that support NVMe-TCP as well as
>>>>    RoCev2 transport protocols  TLS 1.3 species the requirements for
>>>>    KeyUpdate to occur under some events (bytes-transferred or time-
>>>>    elapsed events). When either event occurs, say at a sender node,
>>>>    the sender transmits a KeyUpdate message to indicate that the
>>>>    cryptographic key needs to be updated.  For NVMe-TCP transport
>>>>    protocol, for a host to establish a Fabric Secure Channel, the
>>>>    following sequence occurs between the host and the target:>> ·  3-way TCP handshake
>>>> ·      TLS session (this establishes a secure channel)
>>>> ·      Fabric Connect for Admin queue ·      Fabric Connect for associated IO queues
>>>>    I am not able to find a way for the Linux host to perform the TLS
>>>> KeyUpdate when the transport protocol is NVMe-TCP. It appears that
>>>> the only way to update the TLS keys for this scenario is by
>>>> disconnecting the transport as well as TCP connections and re-
>>>> establishing them using the new TLS keys.>>   I see that Sabrina and Frantisek were working on a patchset for the TLS KeyUpdate
>>>>    functionality over a year ago (and Chuck, Christoph Hellwig
>>>>    mentioned to me that you may have been involved in this functional
>>>>    area as well). I am trying to understand if Linux supports TLS
>>>>    KeyUpdate for NVMe-TCP. My understanding is that the only way for
>>>>    the TLS KeyUpdate to occur is by disconnecting the transport as
>>>>    well as TCP connections and re-establishing them using the new TLS
>>>>    keys.>> https://patchwork.kernel.org/project/netdevbpf/cover/cover.1691584074.git.sd@queasysnail.net/
>>>>   Is my understanding correct?
>>>>   Thanks,
>>>> Narayan
>>> IMO your understanding is correct. Currently a transport disconnect
>>> is the only way to handle this scenario. We recognize that key
>>> update needs to work without incurring a transport disconnect.
>> Well, actually, not.
>> It was a design choice quite early on when developing NVMe-over-TLS that
>> we will _not_ do transparent key updates.
> 
> OK. I was speaking only of KeyUpdate support in tlshd, which
> manages the TLS handshake. I didn't realize that NVMe-o-TLS
> was more restrictive.
> 
> 
>> Currently the only error-recovery mechanism NVMe-over-Fabrics has
 >> is a controller reset, which for fabrics means a complete teardown
 >> of all connections, including a TLS key-renegotiation.>> A 
transparent TLS key update would only improve the situation if we
 >> could make it completely transparent, ie that the actual 
connection>> would stay alive and would just be blocked for the time of 
the key
>> update itself.
>> But the current in-kernel TLS implementation doesn't allow to change
>> the cipher ( cf net/tls/tls_main.c: do_tls_setsockopt_conf() ), so
>> the only chance here is to close the connection and start from scratch.
> 
> Have we asked Boris and friends if it would be possible to
> implement KeyUpdate via ktls? Just curious.
> 
The immediate problem with KeyUpdate is here:

net/tls/tls_main.c:tls_setsockopt_conf()

         /* Currently we don't support set crypto info more than one time */
         if (TLS_CRYPTO_INFO_READY(crypto_info))
                 return -EBUSY;


so we need to close the socket whenever we want to change the TLS key.
And that results in a call to the socket callbacks, which informs the 
NVMe-TCP driver, and the connect is reset.

KeyUpdate is a single message in the packet stream, transferring
the new key values encrypted with the original key.

But: The new key is derived from the application secret, which is _not_ 
the currently used Key/IV values used by the in-kernel TLS 
implementation (cf section 7.3 'Traffic Key Calculation' in RFC 8446).
So we cannot compute the next set of application traffic secrets; the 
only one who might have had this is the tlshd session, but that is 
destroyed once the handshake is done (or, rather, I hope it is ...).

So the original handshake traffic secrets are gone, and we cannot
derive the next set of traffic secrets from there. Plus I'm not sure
if we should keep that around in tlshd; that definitely would be prone
to leak secret material, inviting $RANDOM_HACKER to attack tlshd ...

And that's before even checking whether we need to modify gnutls
for deriving updated traffic secrets ...

So really, not sure if it's worth it.

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: Question regarding TLS Key update for NVMe-TCP protocol

[Sabrina Dubroca]

2024-12-03, 16:20:12 +0100, Hannes Reinecke wrote:
> On 12/3/24 15:33, Chuck Lever III wrote:
> > > On Dec 3, 2024, at 2:17 AM, Hannes Reinecke <hare@suse.de> wrote:
> > > But the current in-kernel TLS implementation doesn't allow to change
> > > the cipher ( cf net/tls/tls_main.c: do_tls_setsockopt_conf() ), so
> > > the only chance here is to close the connection and start from scratch.
> > 
> > Have we asked Boris and friends if it would be possible to
> > implement KeyUpdate via ktls? Just curious.
> > 
> The immediate problem with KeyUpdate is here:
> 
> net/tls/tls_main.c:tls_setsockopt_conf()
> 
>         /* Currently we don't support set crypto info more than one time */
>         if (TLS_CRYPTO_INFO_READY(crypto_info))
>                 return -EBUSY;

And my patches (mentioned in the first email of this thread, but
there's another version that I sent for review more recently [1])
address that.

[1] https://lore.kernel.org/netdev/cover.1731597571.git.sd@queasysnail.net/

> so we need to close the socket whenever we want to change the TLS key.
> And that results in a call to the socket callbacks, which informs the
> NVMe-TCP driver, and the connect is reset.
> 
> KeyUpdate is a single message in the packet stream, transferring
> the new key values encrypted with the original key.

nit: the KeyUpdate message doesn't contain the new key, it just says
that the new key will be used from that point on (otherwise we
wouldn't have to worry about keeping secrets around?).

> But: The new key is derived from the application secret, which is _not_ the
> currently used Key/IV values used by the in-kernel TLS implementation (cf
> section 7.3 'Traffic Key Calculation' in RFC 8446).
> So we cannot compute the next set of application traffic secrets; the only
> one who might have had this is the tlshd session, but that is destroyed once
> the handshake is done (or, rather, I hope it is ...).
> 
> So the original handshake traffic secrets are gone, and we cannot
> derive the next set of traffic secrets from there. Plus I'm not sure
> if we should keep that around in tlshd; that definitely would be prone
> to leak secret material, inviting $RANDOM_HACKER to attack tlshd ...
> 
> And that's before even checking whether we need to modify gnutls
> for deriving updated traffic secrets ...

I would expect all TLS libraries keep those secrets around to be able
to implement KeyUpdate regardless of ktls (but I only work on the
kernel, not gnutls/openssl).

-- 
Sabrina

Re: Question regarding TLS Key update for NVMe-TCP protocol

[Hannes Reinecke]

On 12/6/24 12:26, Sabrina Dubroca wrote:
> 2024-12-03, 16:20:12 +0100, Hannes Reinecke wrote:
>> On 12/3/24 15:33, Chuck Lever III wrote:
>>>> On Dec 3, 2024, at 2:17 AM, Hannes Reinecke <hare@suse.de> wrote:
>>>> But the current in-kernel TLS implementation doesn't allow to change
>>>> the cipher ( cf net/tls/tls_main.c: do_tls_setsockopt_conf() ), so
>>>> the only chance here is to close the connection and start from scratch.
>>>
>>> Have we asked Boris and friends if it would be possible to
>>> implement KeyUpdate via ktls? Just curious.
>>>
>> The immediate problem with KeyUpdate is here:
>>
>> net/tls/tls_main.c:tls_setsockopt_conf()
>>
>>          /* Currently we don't support set crypto info more than one time */
>>          if (TLS_CRYPTO_INFO_READY(crypto_info))
>>                  return -EBUSY;
> 
> And my patches (mentioned in the first email of this thread, but
> there's another version that I sent for review more recently [1])
> address that.
> 
> [1] https://lore.kernel.org/netdev/cover.1731597571.git.sd@queasysnail.net/
> 
Ah-ha.
So that worry is solved. Great work!

>> so we need to close the socket whenever we want to change the TLS key.
>> And that results in a call to the socket callbacks, which informs the
>> NVMe-TCP driver, and the connect is reset.
>>
>> KeyUpdate is a single message in the packet stream, transferring
>> the new key values encrypted with the original key.
> 
> nit: the KeyUpdate message doesn't contain the new key, it just says
> that the new key will be used from that point on (otherwise we
> wouldn't have to worry about keeping secrets around?).
> 
Yeah ... but that doesn't impact this issue, no?

>> But: The new key is derived from the application secret, which is _not_ the
>> currently used Key/IV values used by the in-kernel TLS implementation (cf
>> section 7.3 'Traffic Key Calculation' in RFC 8446).
>> So we cannot compute the next set of application traffic secrets; the only
>> one who might have had this is the tlshd session, but that is destroyed once
>> the handshake is done (or, rather, I hope it is ...).
>>
>> So the original handshake traffic secrets are gone, and we cannot
>> derive the next set of traffic secrets from there. Plus I'm not sure
>> if we should keep that around in tlshd; that definitely would be prone
>> to leak secret material, inviting $RANDOM_HACKER to attack tlshd ...
>>
>> And that's before even checking whether we need to modify gnutls
>> for deriving updated traffic secrets ...
> 
> I would expect all TLS libraries keep those secrets around to be able
> to implement KeyUpdate regardless of ktls (but I only work on the
> kernel, not gnutls/openssl).
> 
They will keep secrets around as long as the program using these 
libraries is running. So yeah, technically tlshd would be alive, and 
gnutls should keep the secrets somehow.
There is a resolved issue for gnutls ('kTLS key update support'), which 
seems to indicate that gnutls supports keyupdate.
And looking closer it seems to be transparent to the caller.

But how does userspace get the KeyUpdate message in the first place?
I _think_ we should get something like TLS_ALERT from the ktls stack.
How is that implemented?
Have you tested your implementation with gnutls?

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: Question regarding TLS Key update for NVMe-TCP protocol

[Sabrina Dubroca]

2024-12-06, 13:09:23 +0100, Hannes Reinecke wrote:
> On 12/6/24 12:26, Sabrina Dubroca wrote:
> > 2024-12-03, 16:20:12 +0100, Hannes Reinecke wrote:
> > > On 12/3/24 15:33, Chuck Lever III wrote:
> > > > > On Dec 3, 2024, at 2:17 AM, Hannes Reinecke <hare@suse.de> wrote:
> > > > > But the current in-kernel TLS implementation doesn't allow to change
> > > > > the cipher ( cf net/tls/tls_main.c: do_tls_setsockopt_conf() ), so
> > > > > the only chance here is to close the connection and start from scratch.
> > > > 
> > > > Have we asked Boris and friends if it would be possible to
> > > > implement KeyUpdate via ktls? Just curious.
> > > > 
> > > The immediate problem with KeyUpdate is here:
> > > 
> > > net/tls/tls_main.c:tls_setsockopt_conf()
> > > 
> > >          /* Currently we don't support set crypto info more than one time */
> > >          if (TLS_CRYPTO_INFO_READY(crypto_info))
> > >                  return -EBUSY;
> > 
> > And my patches (mentioned in the first email of this thread, but
> > there's another version that I sent for review more recently [1])
> > address that.
> > 
> > [1] https://lore.kernel.org/netdev/cover.1731597571.git.sd@queasysnail.net/
> > 
> Ah-ha.
> So that worry is solved. Great work!
> 
> > > so we need to close the socket whenever we want to change the TLS key.
> > > And that results in a call to the socket callbacks, which informs the
> > > NVMe-TCP driver, and the connect is reset.
> > > 
> > > KeyUpdate is a single message in the packet stream, transferring
> > > the new key values encrypted with the original key.
> > 
> > nit: the KeyUpdate message doesn't contain the new key, it just says
> > that the new key will be used from that point on (otherwise we
> > wouldn't have to worry about keeping secrets around?).
> > 
> Yeah ... but that doesn't impact this issue, no?

At least on RX we wouldn't have to worry about recomputing a key, it
would be provided. On TX we'd still have to generate the new key
either way.

> > > But: The new key is derived from the application secret, which is _not_ the
> > > currently used Key/IV values used by the in-kernel TLS implementation (cf
> > > section 7.3 'Traffic Key Calculation' in RFC 8446).
> > > So we cannot compute the next set of application traffic secrets; the only
> > > one who might have had this is the tlshd session, but that is destroyed once
> > > the handshake is done (or, rather, I hope it is ...).
> > > 
> > > So the original handshake traffic secrets are gone, and we cannot
> > > derive the next set of traffic secrets from there. Plus I'm not sure
> > > if we should keep that around in tlshd; that definitely would be prone
> > > to leak secret material, inviting $RANDOM_HACKER to attack tlshd ...
> > > 
> > > And that's before even checking whether we need to modify gnutls
> > > for deriving updated traffic secrets ...
> > 
> > I would expect all TLS libraries keep those secrets around to be able
> > to implement KeyUpdate regardless of ktls (but I only work on the
> > kernel, not gnutls/openssl).
> > 
> They will keep secrets around as long as the program using these libraries
> is running. So yeah, technically tlshd would be alive, and gnutls should
> keep the secrets somehow.
> There is a resolved issue for gnutls ('kTLS key update support'), which
> seems to indicate that gnutls supports keyupdate.
> And looking closer it seems to be transparent to the caller.

It makes sense to me. Which key is being used is an implementation
detail that an application using TLS shouldn't have to care about.

> But how does userspace get the KeyUpdate message in the first place?
> I _think_ we should get something like TLS_ALERT from the ktls stack.
> How is that implemented?

(talking strictly about "pure" ktls, not tls-handshake/tlshd)

Data records are passed to userspace via a simple recv(). Non-data
records (for example a KeyUpdate) need cmsg: the record type is
stuffed into a cmsg, the payload goes in the usual buffer. If the next
available record is non-data and no cmsg is provided, the read will
fail. See Documentation/networking/tls.rst (or
https://docs.kernel.org/networking/tls.html) for more details on how
cmsg is used.

Part of my patchset is pausing decryption when the kernel parses a
KeyUpdate message, until userspace has provided a new key (because
trying to decrypt the next record after the KeyUpdate with the old key
will just fail).

Quoting from the doc update in that patchset (updated for v6, so a bit
different from what you'll find on netdev):

---------- 8< ----------

TLS 1.3 Key Updates
-------------------

In TLS 1.3, KeyUpdate handshake messages signal that the sender is
updating its TX key. Any message sent after a KeyUpdate will be
encrypted using the new key. The userspace library can pass the new
key to the kernel using the TLS_TX and TLS_RX socket options, as for
the initial keys. TLS version and cipher cannot be changed.

To prevent attempting to decrypt incoming records using the wrong key,
decryption will be paused when a KeyUpdate message is received by the
kernel, until the new key has been provided using the TLS_RX socket
option. Any read occurring after the KeyUpdate has been read and
before the new key is provided will fail with EKEYEXPIRED. poll() will
not report any read events from the socket until the new key is
provided. There is no pausing on the transmit side.

---------- 8< ----------

> Have you tested your implementation with gnutls?

Frantisek has (a while back, I keep getting sidetracked from finishing
this patchset :/)

-- 
Sabrina

Re: Question regarding TLS Key update for NVMe-TCP protocol

[Hannes Reinecke]

On 12/6/24 14:05, Sabrina Dubroca wrote:
> 2024-12-06, 13:09:23 +0100, Hannes Reinecke wrote:
>> On 12/6/24 12:26, Sabrina Dubroca wrote:
>>> 2024-12-03, 16:20:12 +0100, Hannes Reinecke wrote:
[ .. ]
>>>> But: The new key is derived from the application secret, which is _not_ the
>>>> currently used Key/IV values used by the in-kernel TLS implementation (cf
>>>> section 7.3 'Traffic Key Calculation' in RFC 8446).
>>>> So we cannot compute the next set of application traffic secrets; the only
>>>> one who might have had this is the tlshd session, but that is destroyed once
>>>> the handshake is done (or, rather, I hope it is ...).
>>>>
>>>> So the original handshake traffic secrets are gone, and we cannot
>>>> derive the next set of traffic secrets from there. Plus I'm not sure
>>>> if we should keep that around in tlshd; that definitely would be prone
>>>> to leak secret material, inviting $RANDOM_HACKER to attack tlshd ...
>>>>
>>>> And that's before even checking whether we need to modify gnutls
>>>> for deriving updated traffic secrets ...
>>>
>>> I would expect all TLS libraries keep those secrets around to be able
>>> to implement KeyUpdate regardless of ktls (but I only work on the
>>> kernel, not gnutls/openssl).
>>>
>> They will keep secrets around as long as the program using these libraries
>> is running. So yeah, technically tlshd would be alive, and gnutls should
>> keep the secrets somehow.
>> There is a resolved issue for gnutls ('kTLS key update support'), which
>> seems to indicate that gnutls supports keyupdate.
>> And looking closer it seems to be transparent to the caller.
> 
> It makes sense to me. Which key is being used is an implementation
> detail that an application using TLS shouldn't have to care about.
> 
>> But how does userspace get the KeyUpdate message in the first place?
>> I _think_ we should get something like TLS_ALERT from the ktls stack.
>> How is that implemented?
> 
> (talking strictly about "pure" ktls, not tls-handshake/tlshd)
> 
> Data records are passed to userspace via a simple recv(). Non-data
> records (for example a KeyUpdate) need cmsg: the record type is
> stuffed into a cmsg, the payload goes in the usual buffer. If the next
> available record is non-data and no cmsg is provided, the read will
> fail. See Documentation/networking/tls.rst (or
> https://docs.kernel.org/networking/tls.html) for more details on how
> cmsg is used.
> 

Ah, right. And that's the issue.
NVMe-TLS is using the 'read_sock()' interface, which doesn't do control 
messages. And changing it over to recvmsg() would not only be a major 
churn, but we also lose the ability to calculate data digests.

So if you have ideas how to handle that ...

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: Question regarding TLS Key update for NVMe-TCP protocol

[Sabrina Dubroca]

2024-12-06, 15:26:08 +0100, Hannes Reinecke wrote:
> On 12/6/24 14:05, Sabrina Dubroca wrote:
> > 2024-12-06, 13:09:23 +0100, Hannes Reinecke wrote:
> > > But how does userspace get the KeyUpdate message in the first place?
> > > I _think_ we should get something like TLS_ALERT from the ktls stack.
> > > How is that implemented?
> > 
> > (talking strictly about "pure" ktls, not tls-handshake/tlshd)
> > 
> > Data records are passed to userspace via a simple recv(). Non-data
> > records (for example a KeyUpdate) need cmsg: the record type is
> > stuffed into a cmsg, the payload goes in the usual buffer. If the next
> > available record is non-data and no cmsg is provided, the read will
> > fail. See Documentation/networking/tls.rst (or
> > https://docs.kernel.org/networking/tls.html) for more details on how
> > cmsg is used.
> > 
> 
> Ah, right. And that's the issue.
> NVMe-TLS is using the 'read_sock()' interface, which doesn't do control
> messages. And changing it over to recvmsg() would not only be a major churn,
> but we also lose the ability to calculate data digests.
> 
> So if you have ideas how to handle that ...

But you have to use recvmsg for the control record if read_sock can't
read anything, which will happen when you hit a control record in the
stream. At this point you have to call recvmsg() to dequeue the
non-data record, process it, and then you can go back to read_sock
until the next non-data record arrives. Like a splice-based userspace
application, I guess. As long as you don't pop that control record
from the socket, you can't read the rest of the stream.

On the TX side, if you have to initiate a KeyUpdate because you've hit
the limit on the current key (or the peer sent its own KeyUpdate and
requested you to also update your key), you'll have to send that
KeyUpdate with sendmsg and the correct cmsg (see the selftest patch
for basic examples from a userspace program [1]).

[1] mainly TEST_F(tls, rekey), and maybe TEST_F(tls, splice_rekey) if
read_sock is similar enough to splice
https://lore.kernel.org/netdev/e00a6aed157b20573afbfc6c2353af2afa54f48e.1731597571.git.sd@queasysnail.net/

-- 
Sabrina

Re: Question regarding TLS Key update for NVMe-TCP protocol

[Hannes Reinecke]

On 12/6/24 16:40, Sabrina Dubroca wrote:
> 2024-12-06, 15:26:08 +0100, Hannes Reinecke wrote:
>> On 12/6/24 14:05, Sabrina Dubroca wrote:
>>> 2024-12-06, 13:09:23 +0100, Hannes Reinecke wrote:
>>>> But how does userspace get the KeyUpdate message in the first place?
>>>> I _think_ we should get something like TLS_ALERT from the ktls stack.
>>>> How is that implemented?
>>>
>>> (talking strictly about "pure" ktls, not tls-handshake/tlshd)
>>>
>>> Data records are passed to userspace via a simple recv(). Non-data
>>> records (for example a KeyUpdate) need cmsg: the record type is
>>> stuffed into a cmsg, the payload goes in the usual buffer. If the next
>>> available record is non-data and no cmsg is provided, the read will
>>> fail. See Documentation/networking/tls.rst (or
>>> https://docs.kernel.org/networking/tls.html) for more details on how
>>> cmsg is used.
>>>
>>
>> Ah, right. And that's the issue.
>> NVMe-TLS is using the 'read_sock()' interface, which doesn't do control
>> messages. And changing it over to recvmsg() would not only be a major churn,
>> but we also lose the ability to calculate data digests.
>>
>> So if you have ideas how to handle that ...
> 
> But you have to use recvmsg for the control record if read_sock can't
> read anything, which will happen when you hit a control record in the
> stream. At this point you have to call recvmsg() to dequeue the
> non-data record, process it, and then you can go back to read_sock
> until the next non-data record arrives. Like a splice-based userspace
> application, I guess. As long as you don't pop that control record
> from the socket, you can't read the rest of the stream.
> 
Ah. Right. Now I just need to find a way to forward that to nvme-tcp;
currently the read_sock() is implemented in net/tls, where I wasn't sure
how to deal with that.
But I guess I could return something like '-ENODATA', and then call 
recvmsg() from the receive loop in nvme-tcp.
Hmm.
But then I have that message in the kernel, and I guess I need to 
forward it to tlshd such that it can calculate the new key.
Chuck, I _think_ there is a bit of infrastructure missing...

> On the TX side, if you have to initiate a KeyUpdate because you've hit
> the limit on the current key (or the peer sent its own KeyUpdate and
> requested you to also update your key), you'll have to send that
> KeyUpdate with sendmsg and the correct cmsg (see the selftest patch
> for basic examples from a userspace program [1]).
> 
> [1] mainly TEST_F(tls, rekey), and maybe TEST_F(tls, splice_rekey) if
> read_sock is similar enough to splice
> https://lore.kernel.org/netdev/e00a6aed157b20573afbfc6c2353af2afa54f48e.1731597571.git.sd@queasysnail.net/
> 
Oh, _that_ we can easily tie into the initiator and/or target.
Formatting yet another cmsg is easy :-)

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: Question regarding TLS Key update for NVMe-TCP protocol

[Chuck Lever]

On 12/6/24 11:23 AM, Hannes Reinecke wrote:
> On 12/6/24 16:40, Sabrina Dubroca wrote:
>> 2024-12-06, 15:26:08 +0100, Hannes Reinecke wrote:
>>> On 12/6/24 14:05, Sabrina Dubroca wrote:
>>>> 2024-12-06, 13:09:23 +0100, Hannes Reinecke wrote:
>>>>> But how does userspace get the KeyUpdate message in the first place?
>>>>> I _think_ we should get something like TLS_ALERT from the ktls stack.
>>>>> How is that implemented?
>>>>
>>>> (talking strictly about "pure" ktls, not tls-handshake/tlshd)
>>>>
>>>> Data records are passed to userspace via a simple recv(). Non-data
>>>> records (for example a KeyUpdate) need cmsg: the record type is
>>>> stuffed into a cmsg, the payload goes in the usual buffer. If the next
>>>> available record is non-data and no cmsg is provided, the read will
>>>> fail. See Documentation/networking/tls.rst (or
>>>> https://docs.kernel.org/networking/tls.html) for more details on how
>>>> cmsg is used.
>>>>
>>>
>>> Ah, right. And that's the issue.
>>> NVMe-TLS is using the 'read_sock()' interface, which doesn't do control
>>> messages. And changing it over to recvmsg() would not only be a major 
>>> churn,
>>> but we also lose the ability to calculate data digests.
>>>
>>> So if you have ideas how to handle that ...
>>
>> But you have to use recvmsg for the control record if read_sock can't
>> read anything, which will happen when you hit a control record in the
>> stream. At this point you have to call recvmsg() to dequeue the
>> non-data record, process it, and then you can go back to read_sock
>> until the next non-data record arrives. Like a splice-based userspace
>> application, I guess. As long as you don't pop that control record
>> from the socket, you can't read the rest of the stream.
>>
> Ah. Right. Now I just need to find a way to forward that to nvme-tcp;
> currently the read_sock() is implemented in net/tls, where I wasn't sure
> how to deal with that.
> But I guess I could return something like '-ENODATA', and then call 
> recvmsg() from the receive loop in nvme-tcp.
> Hmm.
> But then I have that message in the kernel, and I guess I need to 
> forward it to tlshd such that it can calculate the new key.
> Chuck, I _think_ there is a bit of infrastructure missing...

Probably, but you'll have to be more specific.

The SunRPC code implements recvmsg so it can sort incoming control
messages. That's something that each ULP needs to handle in its own
way -- I don't think that can be done reasonably with common code.

But if you mean a KeyUpdate mechanism in the tlshd netlink upcall,
yes agreed. We've known that might be needed for a while.


>> On the TX side, if you have to initiate a KeyUpdate because you've hit
>> the limit on the current key (or the peer sent its own KeyUpdate and
>> requested you to also update your key), you'll have to send that
>> KeyUpdate with sendmsg and the correct cmsg (see the selftest patch
>> for basic examples from a userspace program [1]).
>>
>> [1] mainly TEST_F(tls, rekey), and maybe TEST_F(tls, splice_rekey) if
>> read_sock is similar enough to splice
>> https://lore.kernel.org/netdev/ 
>> e00a6aed157b20573afbfc6c2353af2afa54f48e.1731597571.git.sd@queasysnail.net/
>>
> Oh, _that_ we can easily tie into the initiator and/or target.
> Formatting yet another cmsg is easy :-)
> 
> Cheers,
> 
> Hannes


-- 
Chuck Lever

Re: Question regarding TLS Key update for NVMe-TCP protocol

[Chuck Lever]

On 12/6/24 6:26 AM, Sabrina Dubroca wrote:
> 2024-12-03, 16:20:12 +0100, Hannes Reinecke wrote:
>> On 12/3/24 15:33, Chuck Lever III wrote:
>>>> On Dec 3, 2024, at 2:17 AM, Hannes Reinecke <hare@suse.de> wrote:
>>>> But the current in-kernel TLS implementation doesn't allow to change
>>>> the cipher ( cf net/tls/tls_main.c: do_tls_setsockopt_conf() ), so
>>>> the only chance here is to close the connection and start from scratch.
>>>
>>> Have we asked Boris and friends if it would be possible to
>>> implement KeyUpdate via ktls? Just curious.
>>>
>> The immediate problem with KeyUpdate is here:
>>
>> net/tls/tls_main.c:tls_setsockopt_conf()
>>
>>          /* Currently we don't support set crypto info more than one time */
>>          if (TLS_CRYPTO_INFO_READY(crypto_info))
>>                  return -EBUSY;
> 
> And my patches (mentioned in the first email of this thread, but
> there's another version that I sent for review more recently [1])
> address that.
> 
> [1] https://lore.kernel.org/netdev/cover.1731597571.git.sd@queasysnail.net/
> 
>> so we need to close the socket whenever we want to change the TLS key.
>> And that results in a call to the socket callbacks, which informs the
>> NVMe-TCP driver, and the connect is reset.
>>
>> KeyUpdate is a single message in the packet stream, transferring
>> the new key values encrypted with the original key.
> 
> nit: the KeyUpdate message doesn't contain the new key, it just says
> that the new key will be used from that point on (otherwise we
> wouldn't have to worry about keeping secrets around?).
> 
>> But: The new key is derived from the application secret, which is _not_ the
>> currently used Key/IV values used by the in-kernel TLS implementation (cf
>> section 7.3 'Traffic Key Calculation' in RFC 8446).
>> So we cannot compute the next set of application traffic secrets; the only
>> one who might have had this is the tlshd session, but that is destroyed once
>> the handshake is done (or, rather, I hope it is ...).
>>
>> So the original handshake traffic secrets are gone, and we cannot
>> derive the next set of traffic secrets from there. Plus I'm not sure
>> if we should keep that around in tlshd; that definitely would be prone
>> to leak secret material, inviting $RANDOM_HACKER to attack tlshd ...
>>
>> And that's before even checking whether we need to modify gnutls
>> for deriving updated traffic secrets ...
> 
> I would expect all TLS libraries keep those secrets around to be able
> to implement KeyUpdate regardless of ktls (but I only work on the
> kernel, not gnutls/openssl).

For each handshake request, tlshd forks a child and instantiates
gnutls in that child. When the handshake is complete, the child
exits. No secrets are preserved -- the handshake service is shared
by every user in that network namespace.

So if KeyUpdate requires something to remember the previous key, we
will need to think about where and how that will be done.


-- 
Chuck Lever