On Mon, Oct 20, 2025 at 4:09 PM Hannes Reinecke [off-list ref] wrote:

On 10/17/25 06:23, alistair23@gmail.com wrote:

quoted

From: Alistair Francis <redacted>

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 <redacted>
---
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                    |  8 +++
  include/uapi/linux/handshake.h             | 13 ++++
  net/handshake/tlshd.c                      | 83 +++++++++++++++++++++-
  6 files changed, 137 insertions(+), 8 deletions(-)

diff --git a/Documentation/netlink/specs/handshake.yaml b/Documentation/netlink/specs/handshake.yaml
index a273bc74d26f..c72ec8fa7d7a 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,
+              clientkeyupdaterequest, serverkeyupdate, serverkeyupdaterequest]
    -

Why do we need the 'keyupdate' and 'keyupdaterequest' types?

msg-type indicates if it's a client or server and hello or keyupdate,
the idea being

client:
 - Hello
 - KeyUpdate

server:
 - Hello
 - KeyUpdate

I'll drop clientkeyupdaterequest and serverkeyupdaterequest

Isn't the 'keyupdate' type enough, and can we specify anything
else via the update type?

Once we know if it's a client or server KeyUpdate we need to know if
we are receiving one, sending one or receiving one with the
request_update flag set, hence key-update-type

quoted

      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]

See above.

quoted

  attribute-sets:
    -

@@ -74,6 +80,13 @@ attribute-sets:
        -
          name: keyring
          type: u32
+      -
+        name: key-update-request
+        type: u32
+        enum: key-update-type
+      -
+        name: key-serial
+        type: u32

Not sure if I like key-serial. Yes, it is a key serial number,
but it's not the serial number of the updated key (rather the serial
number of the key holding the session information).
Maybe 'key-update-serial' ?

quoted

    -
      name: done
      attributes:

@@ -116,6 +129,7 @@ operations:
              - certificate
              - peername
              - keyring
+            - key-serial
      -
        name: done
        doc: Handler reports handshake completion

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 611be56f8013..2696bf97dfac 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,
+                           handshake_key_update_type keyupdate);

  static inline struct nvme_tcp_ctrl *to_tcp_ctrl(struct nvme_ctrl *ctrl)
  {

@@ -1726,7 +1731,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,
+                           handshake_key_update_type keyupdate)
  {
      int qid = nvme_tcp_queue_id(queue);
      int ret;

@@ -1748,7 +1754,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);

@@ -1898,7 +1907,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 4ef4dd140ada..8aeec4a7f136 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,
+     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 dc2222fd6d99..084c92a20b68 100644
--- a/include/net/handshake.h
+++ b/include/net/handshake.h

@@ -10,6 +10,10 @@
  #ifndef _NET_HANDSHAKE_H
  #define _NET_HANDSHAKE_H

+#include <uapi/linux/handshake.h>
+
+#define handshake_key_update_type u32
+

Huh?
You define it as 'u32' here

quoted

  enum {
      TLS_NO_KEYRING = 0,
      TLS_NO_PEERID = 0,

@@ -38,8 +42,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,
+                          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,
+                          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..b691530073c6 100644
--- a/include/uapi/linux/handshake.h
+++ b/include/uapi/linux/handshake.h

@@ -19,6 +19,10 @@ enum handshake_msg_type {
      HANDSHAKE_MSG_TYPE_UNSPEC,
      HANDSHAKE_MSG_TYPE_CLIENTHELLO,
      HANDSHAKE_MSG_TYPE_SERVERHELLO,
+     HANDSHAKE_MSG_TYPE_CLIENTKEYUPDATE,
+     HANDSHAKE_MSG_TYPE_CLIENTKEYUPDATEREQUEST,
+     HANDSHAKE_MSG_TYPE_SERVERKEYUPDATE,
+     HANDSHAKE_MSG_TYPE_SERVERKEYUPDATEREQUEST,
  };

  enum handshake_auth {

@@ -28,6 +32,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,
+};
+

and here it's an enum. Please kill the first declaration.

quoted

  enum {
      HANDSHAKE_A_X509_CERT = 1,
      HANDSHAKE_A_X509_PRIVKEY,

@@ -46,6 +57,8 @@ enum {
      HANDSHAKE_A_ACCEPT_CERTIFICATE,
      HANDSHAKE_A_ACCEPT_PEERNAME,
      HANDSHAKE_A_ACCEPT_KEYRING,
+     HANDSHAKE_A_ACCEPT_KEY_UPDATE_REQUEST,
+     HANDSHAKE_A_ACCEPT_KEY_SERIAL,

      __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 2549c5dbccd8..c40839977ab9 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];

+     int                     th_key_update_request;
      key_serial_t            user_session_id;
  };

Why 'int' ? Can it be negative?
If not please make it an 'unsigned int'

quoted

@@ -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->user_session_id = TLS_NO_PRIVKEY;
+     treq->user_session_id = args->user_session_id;
+
      return treq;
  }

@@ -265,6 +267,16 @@ static int tls_handshake_accept(struct handshake_req *req,
              break;
      }

+     ret = nla_put_u32(msg, HANDSHAKE_A_ACCEPT_KEY_SERIAL,
+                       treq->user_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,
+                          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];

Hmm?
Do we use the 'peerids'?

We don't, this is just copied from the
tls_client_hello_psk()/tls_server_hello_psk() to provide the same
information to keep things more consistent.

I can remove setting these

I thought that the information was encoded in the session, ie
the 'user_session_id' ?

quoted

+
+     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,
+                          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];

Same here. Why do we need to set 'peerid'?

quoted

+
+     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

Nit: we _could_ overload 'peerid' with the user_session_id,then we
wouldn't need to specify a new field in the handshake
request.
But that's arguably quite hackish.

Oh no! Let's not do that. That just seems prone to confusion

Alistair

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