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>
---
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]
-
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: key-serial
+ type: u32
-
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
+
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,
+};
+
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;
};
@@ -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];
+
+ 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];
+
+ 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
--
2.51.0On 10/17/25 06:23, 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>
> ---
> 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?
Isn't the 'keyupdate' type enough, and can we specify anything
else via the update type?
> 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.
>
> 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' ?
> -
> 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
> 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.
> 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'
> @@ -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'?
I thought that the information was encoded in the session, ie
the 'user_session_id' ?
> +
> + 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'?
> +
> + 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.
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
On Mon, Oct 20, 2025 at 4:09 PM Hannes Reinecke <hare@suse.de> wrote:
>
> On 10/17/25 06:23, 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>
> > ---
> > 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
>
> > 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.
>
> >
> > 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' ?
>
> > -
> > 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
>
> > 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.
>
> > 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'
>
> > @@ -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' ?
>
> > +
> > + 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'?
>
> > +
> > + 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
On Tue, Oct 21, 2025 at 1:19 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Mon, Oct 20, 2025 at 4:09 PM Hannes Reinecke <hare@suse.de> wrote:
> >
> > On 10/17/25 06:23, 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>
> > > ---
> > > 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
>
> >
> > > 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.
> >
> > >
> > > 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' ?
> >
> > > -
> > > 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
> >
> > > 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.
> >
> > > 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'
> >
> > > @@ -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
Actually, ktls-utils (tlshd) expects these to be set, so I think we
should leave them as is
Alistair
>
> > I thought that the information was encoded in the session, ie
> > the 'user_session_id' ?
> >
> > > +
> > > + 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'?
> >
> > > +
> > > + 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
On 10/22/25 06:40, Alistair Francis wrote:
> On Tue, Oct 21, 2025 at 1:19 PM Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Mon, Oct 20, 2025 at 4:09 PM Hannes Reinecke <hare@suse.de> wrote:
>>>
>>> On 10/17/25 06:23, alistair23@gmail.com wrote:
>>>> From: Alistair Francis <alistair.francis@wdc.com>
>>>>
[ .. ]>>>> @@ -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
>
> Actually, ktls-utils (tlshd) expects these to be set, so I think we
> should leave them as is
>
Can't we rather fix up tlshd?
It feels really pointless, erroring out on values which are completely
irrelevant for the operation...
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
On Wed, Oct 22, 2025 at 5:03 PM Hannes Reinecke <hare@suse.de> wrote:
>
> On 10/22/25 06:40, Alistair Francis wrote:
> > On Tue, Oct 21, 2025 at 1:19 PM Alistair Francis <alistair23@gmail.com> wrote:
> >>
> >> On Mon, Oct 20, 2025 at 4:09 PM Hannes Reinecke <hare@suse.de> wrote:
> >>>
> >>> On 10/17/25 06:23, alistair23@gmail.com wrote:
> >>>> From: Alistair Francis <alistair.francis@wdc.com>
> >>>>
> [ .. ]>>>> @@ -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
> >
> > Actually, ktls-utils (tlshd) expects these to be set, so I think we
> > should leave them as is
> >
>
> Can't we rather fix up tlshd?
> It feels really pointless, erroring out on values which are completely
> irrelevant for the operation...
It's not that simple.
For example when we call "done" for a handshake or KeyUpdate we call
tls_handshake_done() in the kernel, which calls
tls_handshake_remote_peerids(). So the kernel expects the remote
peerids to be set.
I think there's a lot of value in re-using the existing flows (as a
KeyUpdate is similar to a handshake), but the existing flows expect
remote peerids to be set. We could duplicate everything just to remove
that requirement, but I don't think that's the right approach.
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
© 2016 - 2026 Red Hat, Inc.