linux-kernel - Re: [PATCH v5 6/6] nvmet-tcp: Support KeyUpdate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251112070138.GG4873@lst.de>
Date: Wed, 12 Nov 2025 08:01:38 +0100
From: Christoph Hellwig <hch@....de>
To: alistair23@...il.com
Cc: 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,
	kbusch@...nel.org, axboe@...nel.dk, hch@....de, sagi@...mberg.me,
	kch@...dia.com, hare@...e.de,
	Alistair Francis <alistair.francis@....com>
Subject: Re: [PATCH v5 6/6] nvmet-tcp: Support KeyUpdate

On Wed, Nov 12, 2025 at 02:27:20PM +1000, alistair23@...il.com wrote:
> From: Alistair Francis <alistair.francis@....com>
> 
> If the nvmet_tcp_try_recv() function return EKEYEXPIRED or if we receive
> a KeyUpdate handshake type then the underlying TLS keys need to be
> updated.
> 
> If the NVMe Host (TLS client) initiates a KeyUpdate this patch will
> allow the NVMe layer to process the KeyUpdate request and forward the
> request to userspace. Userspace must then update the key to keep the
> connection alive.
> 
> This patch allows us to handle the NVMe host sending a KeyUpdate
> request without aborting the connection. At this time we don't support
> initiating a KeyUpdate.
> 
> Link: https://datatracker.ietf.org/doc/html/rfc8446#section-4.6.3
> Signed-off-by: Alistair Francis <alistair.francis@....com>
> Reviewed-by: Hannes Reinecke <hare@...e.de>
> ---
> v5:
>  - No change
> v4:
>  - Restructure code to avoid #ifdefs and forward declarations
>  - Use a helper function for checking -EKEYEXPIRED
>  - Remove all support for initiating KeyUpdate
>  - Use helper function for restoring callbacks
> v3:
>  - Use a write lock for sk_user_data
>  - Fix build with CONFIG_NVME_TARGET_TCP_TLS disabled
>  - Remove unused variable
> v2:
>  - Use a helper function for KeyUpdates
>  - Ensure keep alive timer is stopped
>  - Wait for TLS KeyUpdate to complete
> 
>  drivers/nvme/target/tcp.c | 203 ++++++++++++++++++++++++++------------
>  1 file changed, 142 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index 818efdeccef1..486ea7bb0056 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -175,6 +175,7 @@ struct nvmet_tcp_queue {
>  
>  	/* TLS state */
>  	key_serial_t		tls_pskid;
> +	key_serial_t		handshake_session_id;
>  	struct delayed_work	tls_handshake_tmo_work;
>  
>  	unsigned long           poll_end;
> @@ -186,6 +187,8 @@ struct nvmet_tcp_queue {
>  	struct sockaddr_storage	sockaddr_peer;
>  	struct work_struct	release_work;
>  
> +	struct completion       tls_complete;
> +
>  	int			idx;
>  	struct list_head	queue_list;
>  
> @@ -214,6 +217,10 @@ static struct workqueue_struct *nvmet_tcp_wq;
>  static const struct nvmet_fabrics_ops nvmet_tcp_ops;
>  static void nvmet_tcp_free_cmd(struct nvmet_tcp_cmd *c);
>  static void nvmet_tcp_free_cmd_buffers(struct nvmet_tcp_cmd *cmd);
> +#ifdef CONFIG_NVME_TARGET_TCP_TLS
> +static int nvmet_tcp_tls_handshake(struct nvmet_tcp_queue *queue,
> +				   enum handshake_key_update_type keyupdate);
> +#endif
>  
>  static inline u16 nvmet_tcp_cmd_tag(struct nvmet_tcp_queue *queue,
>  		struct nvmet_tcp_cmd *cmd)
> @@ -832,6 +839,23 @@ static int nvmet_tcp_try_send_one(struct nvmet_tcp_queue *queue,
>  	return 1;
>  }
>  
> +#ifdef CONFIG_NVME_TARGET_TCP_TLS
> +static bool nvmet_tls_key_expired(struct nvmet_tcp_queue *queue, int ret)
> +{
> +	if (ret == -EKEYEXPIRED &&
> +	    queue->state != NVMET_TCP_Q_DISCONNECTING &&
> +	    queue->state != NVMET_TCP_Q_TLS_HANDSHAKE)
> +					return true;
> +
> +	return false;

Extra indentation before the return true.  This could also be simplified
down to

	return ret == -EKEYEXPIRED &&
		queue->state != NVMET_TCP_Q_DISCONNECTING &&
		queue->state != NVMET_TCP_Q_TLS_HANDSHAKE;

or if you want to do away with the ifdef entirely:

	return IS_ENABLED(CONFIG_NVME_TARGET_TCP_TLS) &&
		ret == -EKEYEXPIRED &&
		queue->state != NVMET_TCP_Q_DISCONNECTING &&
		queue->state != NVMET_TCP_Q_TLS_HANDSHAKE;

Othwise looks good.