Message-ID: <68e45231-8344-447d-95cc-4b95a13df353@suse.de>
Date: Mon, 15 Sep 2025 13:44:54 +0200
From: Hannes Reinecke <hare@...e.de>
To: alistair23@...il.com, chuck.lever@...cle.com, hare@...nel.org,
 kernel-tls-handshake@...ts.linux.dev, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
 linux-nvme@...ts.infradead.org, linux-nfs@...r.kernel.org
Cc: kbusch@...nel.org, axboe@...nel.dk, hch@....de, sagi@...mberg.me,
 kch@...dia.com, Alistair Francis <alistair.francis@....com>
Subject: Re: [PATCH v2 0/7] nvme-tcp: Support receiving KeyUpdate requests

On 9/5/25 04:46, alistair23@...il.com wrote:
> From: Alistair Francis <alistair.francis@....com>
> 
> The TLS 1.3 specification allows the TLS client or server to send a
> KeyUpdate. This is generally used when the sequence is about to
> overflow or after a certain amount of bytes have been encrypted.
> 
> The TLS spec doesn't mandate the conditions though, so a KeyUpdate
> can be sent by the TLS client or server at any time. This includes
> when running NVMe-OF over a TLS 1.3 connection.
> 
> As such Linux should be able to handle a KeyUpdate event, as the
> other NVMe side could initiate a KeyUpdate.
> 
> Upcoming WD NVMe-TCP hardware controllers implement TLS support
> and send KeyUpdate requests.
> 
> This series builds on top of the existing TLS EKEYEXPIRED work,
> which already detects a KeyUpdate request. We can now pass that
> information up to the NVMe layer (target and host) and then pass
> it up to userspace.
> 
> Userspace (ktls-utils) will need to save the connection state
> in the keyring during the initial handshake. The kernel then
> provides the key serial back to userspace when handling a
> KeyUpdate. Userspace can use this to restore the connection
> information and then update the keys, this final process
> is similar to the initial handshake.
> 
> Link: https://datatracker.ietf.org/doc/html/rfc8446#section-4.6.3
> 
> v2:
>   - Change "key-serial" to "session-id"
>   - Fix reported build failures
>   - Drop tls_clear_err() function
>   - Stop keep alive timer during KeyUpdate
>   - Drop handshake message decoding in the NVMe layer
> 
> Alistair Francis (7):
>    net/handshake: Store the key serial number on completion
>    net/handshake: Make handshake_req_cancel public
>    net/handshake: Expose handshake_sk_destruct_req publically
>    nvmet: Expose nvmet_stop_keep_alive_timer publically
>    net/handshake: Support KeyUpdate message types
>    nvme-tcp: Support KeyUpdate
>    nvmet-tcp: Support KeyUpdate
> 
>   Documentation/netlink/specs/handshake.yaml |  19 +++-
>   Documentation/networking/tls-handshake.rst |   4 +-
>   drivers/nvme/host/tcp.c                    |  88 +++++++++++++++--
>   drivers/nvme/target/core.c                 |   1 +
>   drivers/nvme/target/tcp.c                  | 104 +++++++++++++++++++--
>   include/net/handshake.h                    |  17 +++-
>   include/uapi/linux/handshake.h             |  14 +++
>   net/handshake/genl.c                       |   5 +-
>   net/handshake/handshake.h                  |   1 -
>   net/handshake/request.c                    |  18 ++++
>   net/handshake/tlshd.c                      |  46 +++++++--
>   net/sunrpc/svcsock.c                       |   3 +-
>   net/sunrpc/xprtsock.c                      |   3 +-
>   13 files changed, 289 insertions(+), 34 deletions(-)
> 

Hey Alistair,
thanks for doing this. While the patchset itself looks okay-ish, there
are some general ideas/concerns for it:

- I have posted a patch for replacing the current 'read_sock()'
interface with a recvmsg() base workflow. That should give us
access to the 'real' control message, so it would be good if you
could fold it in.
- Olga has send a patchset fixing a security issue with control
messages; the gist is that the network code expects a 'kvec' based
msg buffer when receiving a control message. So essentially one
has to receive a message _without_ a control buffer, check for
MSG_CTRUNC, and then read the control message via kvec.
Can you ensure that your patchset follows these guidelines?
- There is no method to trigger a KeyUpdate, making it really hard
to test this feature (eg by writin a blktest for it). Ideally we
should be able to trigger it from both directions, but having just
one (eg on the target side) should be enough for starters.
A possible interface would be to implement write support to the
'tls_key' debugfs attribute; when writing the same key ID as
the one currently in use the KeyUpdate mechanism could be started.

But thanks for doing the work!

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@...e.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

Powered by blists - more mailing lists