Re: [PATCH v5 4/6] net/handshake: Support KeyUpdate message types

Chuck Lever
2025-11-13T14:42:14Z

On 11/11/25 11:27 PM, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> When reporting the msg-type to userspace let's also support reporting
> KeyUpdate events. This supports reporting a client/server event and if
> the other side requested a KeyUpdateRequest.
>
> Link: https://datatracker.ietf.org/doc/html/rfc8446#section-4.6.3
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> v5:
> - Drop clientkeyupdaterequest and serverkeyupdaterequest
> v4:
> - Don't overload existing functions, instead create new ones
> v3:
> - Fixup yamllint and kernel-doc failures
>
> Documentation/netlink/specs/handshake.yaml | 16 ++++-
> drivers/nvme/host/tcp.c | 15 +++-
> drivers/nvme/target/tcp.c | 10 ++-
> include/net/handshake.h | 6 ++
> include/uapi/linux/handshake.h | 11 +++
> net/handshake/tlshd.c | 83 +++++++++++++++++++++-
> 6 files changed, 133 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/netlink/specs/handshake.yaml b/Documentation/netlink/specs/handshake.yaml
> index a273bc74d26f..2f77216c8ddf 100644
> --- a/Documentation/netlink/specs/handshake.yaml
> +++ b/Documentation/netlink/specs/handshake.yaml
> @@ -21,12 +21,18 @@ definitions:
> type: enum
> name: msg-type
> value-start: 0
> - entries: [unspec, clienthello, serverhello]
> + entries: [unspec, clienthello, serverhello, clientkeyupdate,
> + serverkeyupdate]
> -
> type: enum
> name: auth
> value-start: 0
> entries: [unspec, unauth, psk, x509]
> + -
> + type: enum
> + name: key-update-type
> + value-start: 0
> + entries: [unspec, send, received, received_request_update]
>
> attribute-sets:
> -
> @@ -74,6 +80,13 @@ attribute-sets:
> -
> name: keyring
> type: u32
> + -
> + name: key-update-request
> + type: u32
> + enum: key-update-type
> + -
> + name: session-id
> + type: u32
> -
> name: done
> attributes:
> @@ -116,6 +129,7 @@ operations:
> - certificate
> - peername
> - keyring
> + - session-id
> -
> name: done
> doc: Handler reports handshake completion
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 024d02248831..4797a4532b0d 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -20,6 +20,7 @@
> #include <linux/iov_iter.h>
> #include <net/busy_poll.h>
> #include <trace/events/sock.h>
> +#include <uapi/linux/handshake.h>
>
> #include "nvme.h"
> #include "fabrics.h"
> @@ -206,6 +207,10 @@ static struct workqueue_struct *nvme_tcp_wq;
> static const struct blk_mq_ops nvme_tcp_mq_ops;
> static const struct blk_mq_ops nvme_tcp_admin_mq_ops;
> static int nvme_tcp_try_send(struct nvme_tcp_queue *queue);
> +static int nvme_tcp_start_tls(struct nvme_ctrl *nctrl,
> + struct nvme_tcp_queue *queue,
> + key_serial_t pskid,
> + enum handshake_key_update_type keyupdate);
>
> static inline struct nvme_tcp_ctrl *to_tcp_ctrl(struct nvme_ctrl *ctrl)
> {
> @@ -1729,7 +1734,8 @@ static void nvme_tcp_tls_done(void *data, int status, key_serial_t pskid,
>
> static int nvme_tcp_start_tls(struct nvme_ctrl *nctrl,
> struct nvme_tcp_queue *queue,
> - key_serial_t pskid)
> + key_serial_t pskid,
> + enum handshake_key_update_type keyupdate)
> {
> int qid = nvme_tcp_queue_id(queue);
> int ret;
> @@ -1751,7 +1757,10 @@ static int nvme_tcp_start_tls(struct nvme_ctrl *nctrl,
> args.ta_timeout_ms = tls_handshake_timeout * 1000;
> queue->tls_err = -EOPNOTSUPP;
> init_completion(&queue->tls_complete);
> - ret = tls_client_hello_psk(&args, GFP_KERNEL);
> + if (keyupdate == HANDSHAKE_KEY_UPDATE_TYPE_UNSPEC)
> + ret = tls_client_hello_psk(&args, GFP_KERNEL);
> + else
> + ret = tls_client_keyupdate_psk(&args, GFP_KERNEL, keyupdate);
> if (ret) {
> dev_err(nctrl->device, "queue %d: failed to start TLS: %d\n",
> qid, ret);
> @@ -1901,7 +1910,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
>
> /* If PSKs are configured try to start TLS */
> if (nvme_tcp_tls_configured(nctrl) && pskid) {
> - ret = nvme_tcp_start_tls(nctrl, queue, pskid);
> + ret = nvme_tcp_start_tls(nctrl, queue, pskid, HANDSHAKE_KEY_UPDATE_TYPE_UNSPEC);
> if (ret)
> goto err_init_connect;
> }
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index 7f8516892359..818efdeccef1 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -1833,7 +1833,8 @@ static void nvmet_tcp_tls_handshake_timeout(struct work_struct *w)
> kref_put(&queue->kref, nvmet_tcp_release_queue);
> }
>
> -static int nvmet_tcp_tls_handshake(struct nvmet_tcp_queue *queue)
> +static int nvmet_tcp_tls_handshake(struct nvmet_tcp_queue *queue,
> + enum handshake_key_update_type keyupdate)
> {
> int ret = -EOPNOTSUPP;
> struct tls_handshake_args args;
> @@ -1852,7 +1853,10 @@ static int nvmet_tcp_tls_handshake(struct nvmet_tcp_queue *queue)
> args.ta_keyring = key_serial(queue->port->nport->keyring);
> args.ta_timeout_ms = tls_handshake_timeout * 1000;
>
> - ret = tls_server_hello_psk(&args, GFP_KERNEL);
> + if (keyupdate == HANDSHAKE_KEY_UPDATE_TYPE_UNSPEC)
> + ret = tls_server_hello_psk(&args, GFP_KERNEL);
> + else
> + ret = tls_server_keyupdate_psk(&args, GFP_KERNEL, keyupdate);
> if (ret) {
> kref_put(&queue->kref, nvmet_tcp_release_queue);
> pr_err("failed to start TLS, err=%d\n", ret);
> @@ -1934,7 +1938,7 @@ static void nvmet_tcp_alloc_queue(struct nvmet_tcp_port *port,
> sk->sk_data_ready = port->data_ready;
> write_unlock_bh(&sk->sk_callback_lock);
> if (!nvmet_tcp_try_peek_pdu(queue)) {
> - if (!nvmet_tcp_tls_handshake(queue))
> + if (!nvmet_tcp_tls_handshake(queue, HANDSHAKE_KEY_UPDATE_TYPE_UNSPEC))
> return;
> /* TLS handshake failed, terminate the connection */
> goto out_destroy_sq;
> diff --git a/include/net/handshake.h b/include/net/handshake.h
> index 68d7f89e431a..f5a249327bf6 100644
> --- a/include/net/handshake.h
> +++ b/include/net/handshake.h
> @@ -10,6 +10,8 @@
> #ifndef _NET_HANDSHAKE_H
> #define _NET_HANDSHAKE_H
>
> +#include <uapi/linux/handshake.h>
> +
> enum {
> TLS_NO_KEYRING = 0,
> TLS_NO_PEERID = 0,
> @@ -38,8 +40,12 @@ struct tls_handshake_args {
> int tls_client_hello_anon(const struct tls_handshake_args *args, gfp_t flags);
> int tls_client_hello_x509(const struct tls_handshake_args *args, gfp_t flags);
> int tls_client_hello_psk(const struct tls_handshake_args *args, gfp_t flags);
> +int tls_client_keyupdate_psk(const struct tls_handshake_args *args, gfp_t flags,
> + enum handshake_key_update_type keyupdate);
> int tls_server_hello_x509(const struct tls_handshake_args *args, gfp_t flags);
> int tls_server_hello_psk(const struct tls_handshake_args *args, gfp_t flags);
> +int tls_server_keyupdate_psk(const struct tls_handshake_args *args, gfp_t flags,
> + enum handshake_key_update_type keyupdate);
>
> bool tls_handshake_cancel(struct sock *sk);
> void tls_handshake_close(struct socket *sock);
> diff --git a/include/uapi/linux/handshake.h b/include/uapi/linux/handshake.h
> index b68ffbaa5f31..483815a064f0 100644
> --- a/include/uapi/linux/handshake.h
> +++ b/include/uapi/linux/handshake.h
> @@ -19,6 +19,8 @@ enum handshake_msg_type {
> HANDSHAKE_MSG_TYPE_UNSPEC,
> HANDSHAKE_MSG_TYPE_CLIENTHELLO,
> HANDSHAKE_MSG_TYPE_SERVERHELLO,
> + HANDSHAKE_MSG_TYPE_CLIENTKEYUPDATE,
> + HANDSHAKE_MSG_TYPE_SERVERKEYUPDATE,
> };
>
> enum handshake_auth {
> @@ -28,6 +30,13 @@ enum handshake_auth {
> HANDSHAKE_AUTH_X509,
> };
>
> +enum handshake_key_update_type {
> + HANDSHAKE_KEY_UPDATE_TYPE_UNSPEC,
> + HANDSHAKE_KEY_UPDATE_TYPE_SEND,
> + HANDSHAKE_KEY_UPDATE_TYPE_RECEIVED,
> + HANDSHAKE_KEY_UPDATE_TYPE_RECEIVED_REQUEST_UPDATE,
> +};
> +
> enum {
> HANDSHAKE_A_X509_CERT = 1,
> HANDSHAKE_A_X509_PRIVKEY,
> @@ -46,6 +55,8 @@ enum {
> HANDSHAKE_A_ACCEPT_CERTIFICATE,
> HANDSHAKE_A_ACCEPT_PEERNAME,
> HANDSHAKE_A_ACCEPT_KEYRING,
> + HANDSHAKE_A_ACCEPT_KEY_UPDATE_REQUEST,
> + HANDSHAKE_A_ACCEPT_SESSION_ID,
>
> __HANDSHAKE_A_ACCEPT_MAX,
> HANDSHAKE_A_ACCEPT_MAX = (__HANDSHAKE_A_ACCEPT_MAX - 1)
> diff --git a/net/handshake/tlshd.c b/net/handshake/tlshd.c
> index 85c5fed690c0..91d2bb515b7d 100644
> --- a/net/handshake/tlshd.c
> +++ b/net/handshake/tlshd.c
> @@ -41,6 +41,7 @@ struct tls_handshake_req {
> unsigned int th_num_peerids;
> key_serial_t th_peerid[5];
>
> + unsigned int th_key_update_request;
> key_serial_t handshake_session_id;
> };
>
> @@ -58,7 +59,8 @@ tls_handshake_req_init(struct handshake_req *req,
> treq->th_num_peerids = 0;
> treq->th_certificate = TLS_NO_CERT;
> treq->th_privkey = TLS_NO_PRIVKEY;
> - treq->handshake_session_id = TLS_NO_PRIVKEY;
> + treq->handshake_session_id = args->handshake_session_id;

Should you initialize the new th_key_update_request field to 0 here?
Prevent leaking previous memory contents to user space...

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>

> +
> return treq;
> }
>
> @@ -265,6 +267,16 @@ static int tls_handshake_accept(struct handshake_req *req,
> break;
> }
>
> + ret = nla_put_u32(msg, HANDSHAKE_A_ACCEPT_SESSION_ID,
> + treq->handshake_session_id);
> + if (ret < 0)
> + goto out_cancel;
> +
> + ret = nla_put_u32(msg, HANDSHAKE_A_ACCEPT_KEY_UPDATE_REQUEST,
> + treq->th_key_update_request);
> + if (ret < 0)
> + goto out_cancel;
> +
> genlmsg_end(msg, hdr);
> return genlmsg_reply(msg, info);
>
> @@ -372,6 +384,44 @@ int tls_client_hello_psk(const struct tls_handshake_args *args, gfp_t flags)
> }
> EXPORT_SYMBOL(tls_client_hello_psk);
>
> +/**
> + * tls_client_keyupdate_psk - request a PSK-based TLS handshake on a socket
> + * @args: socket and handshake parameters for this request
> + * @flags: memory allocation control flags
> + * @keyupdate: specifies the type of KeyUpdate operation
> + *
> + * Return values:
> + * %0: Handshake request enqueue; ->done will be called when complete
> + * %-EINVAL: Wrong number of local peer IDs
> + * %-ESRCH: No user agent is available
> + * %-ENOMEM: Memory allocation failed
> + */
> +int tls_client_keyupdate_psk(const struct tls_handshake_args *args, gfp_t flags,
> + enum handshake_key_update_type keyupdate)
> +{
> + struct tls_handshake_req *treq;
> + struct handshake_req *req;
> + unsigned int i;
> +
> + if (!args->ta_num_peerids ||
> + args->ta_num_peerids > ARRAY_SIZE(treq->th_peerid))
> + return -EINVAL;
> +
> + req = handshake_req_alloc(&tls_handshake_proto, flags);
> + if (!req)
> + return -ENOMEM;
> + treq = tls_handshake_req_init(req, args);
> + treq->th_type = HANDSHAKE_MSG_TYPE_CLIENTKEYUPDATE;
> + treq->th_key_update_request = keyupdate;
> + treq->th_auth_mode = HANDSHAKE_AUTH_PSK;
> + treq->th_num_peerids = args->ta_num_peerids;
> + for (i = 0; i < args->ta_num_peerids; i++)
> + treq->th_peerid[i] = args->ta_my_peerids[i];
> +
> + return handshake_req_submit(args->ta_sock, req, flags);
> +}
> +EXPORT_SYMBOL(tls_client_keyupdate_psk);
> +
> /**
> * tls_server_hello_x509 - request a server TLS handshake on a socket
> * @args: socket and handshake parameters for this request
> @@ -428,6 +478,37 @@ int tls_server_hello_psk(const struct tls_handshake_args *args, gfp_t flags)
> }
> EXPORT_SYMBOL(tls_server_hello_psk);
>
> +/**
> + * tls_server_keyupdate_psk - request a server TLS KeyUpdate on a socket
> + * @args: socket and handshake parameters for this request
> + * @flags: memory allocation control flags
> + * @keyupdate: specifies the type of KeyUpdate operation
> + *
> + * Return values:
> + * %0: Handshake request enqueue; ->done will be called when complete
> + * %-ESRCH: No user agent is available
> + * %-ENOMEM: Memory allocation failed
> + */
> +int tls_server_keyupdate_psk(const struct tls_handshake_args *args, gfp_t flags,
> + enum handshake_key_update_type keyupdate)
> +{
> + struct tls_handshake_req *treq;
> + struct handshake_req *req;
> +
> + req = handshake_req_alloc(&tls_handshake_proto, flags);
> + if (!req)
> + return -ENOMEM;
> + treq = tls_handshake_req_init(req, args);
> + treq->th_type = HANDSHAKE_MSG_TYPE_SERVERKEYUPDATE;
> + treq->th_key_update_request = keyupdate;
> + treq->th_auth_mode = HANDSHAKE_AUTH_PSK;
> + treq->th_num_peerids = 1;
> + treq->th_peerid[0] = args->ta_my_peerids[0];
> +
> + return handshake_req_submit(args->ta_sock, req, flags);
> +}
> +EXPORT_SYMBOL(tls_server_keyupdate_psk);
> +
> /**
> * tls_handshake_cancel - cancel a pending handshake
> * @sk: socket on which there is an ongoing handshake

--
Chuck Lever