Re: [PATCH v5 1/6] net/handshake: Store the key serial number on completion

Chuck Lever
2025-11-12T15:02:30Z

On 11/11/25 11:27 PM, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Allow userspace to include a key serial number when completing a
> handshake with the HANDSHAKE_CMD_DONE command.
>
> We then store this serial number and will provide it back to userspace
> in the future. This allows userspace to save data to the keyring and
> then restore that data later.
>
> This will be used to support the TLS KeyUpdate operation, as now
> userspace can resume information about a established session.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Hannes Reincke <hare@suse.de>
> ---
> v5:
> - Change name to "handshake session ID"
> v4:
> - No change
> v3:
> - No change
> v2:
> - Change "key-serial" to "session-id"
>
> Documentation/netlink/specs/handshake.yaml | 4 ++++
> Documentation/networking/tls-handshake.rst | 1 +
> drivers/nvme/host/tcp.c | 3 ++-
> drivers/nvme/target/tcp.c | 3 ++-
> include/net/handshake.h | 4 +++-
> include/uapi/linux/handshake.h | 1 +
> net/handshake/genl.c | 5 +++--
> net/handshake/tlshd.c | 15 +++++++++++++--
> net/sunrpc/svcsock.c | 4 +++-
> net/sunrpc/xprtsock.c | 4 +++-
> 10 files changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/netlink/specs/handshake.yaml b/Documentation/netlink/specs/handshake.yaml
> index 95c3fade7a8d..a273bc74d26f 100644
> --- a/Documentation/netlink/specs/handshake.yaml
> +++ b/Documentation/netlink/specs/handshake.yaml
> @@ -87,6 +87,9 @@ attribute-sets:
> name: remote-auth
> type: u32
> multi-attr: true
> + -
> + name: session-id
> + type: u32
>
> operations:
> list:
> @@ -123,6 +126,7 @@ operations:
> - status
> - sockfd
> - remote-auth
> + - session-id
>
> mcast-groups:
> list:
> diff --git a/Documentation/networking/tls-handshake.rst b/Documentation/networking/tls-handshake.rst
> index 6f5ea1646a47..0941b81dde5c 100644
> --- a/Documentation/networking/tls-handshake.rst
> +++ b/Documentation/networking/tls-handshake.rst
> @@ -60,6 +60,7 @@ fills in a structure that contains the parameters of the request:
> key_serial_t ta_my_privkey;
> unsigned int ta_num_peerids;
> key_serial_t ta_my_peerids[5];
> + key_serial_t handshake_session_id;
> };
>
> The @ta_sock field references an open and connected socket. The consumer
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 9058ea64b89c..024d02248831 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -1694,7 +1694,8 @@ static void nvme_tcp_set_queue_io_cpu(struct nvme_tcp_queue *queue)
> qid, queue->io_cpu);
> }
>
> -static void nvme_tcp_tls_done(void *data, int status, key_serial_t pskid)
> +static void nvme_tcp_tls_done(void *data, int status, key_serial_t pskid,
> + key_serial_t handshake_session_id)
> {
> struct nvme_tcp_queue *queue = data;
> struct nvme_tcp_ctrl *ctrl = queue->ctrl;
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index 470bf37e5a63..7f8516892359 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -1780,7 +1780,8 @@ static int nvmet_tcp_tls_key_lookup(struct nvmet_tcp_queue *queue,
> }
>
> static void nvmet_tcp_tls_handshake_done(void *data, int status,
> - key_serial_t peerid)
> + key_serial_t peerid,
> + key_serial_t handshake_session_id)
> {
> struct nvmet_tcp_queue *queue = data;
>
> diff --git a/include/net/handshake.h b/include/net/handshake.h
> index 8ebd4f9ed26e..68d7f89e431a 100644
> --- a/include/net/handshake.h
> +++ b/include/net/handshake.h
> @@ -18,7 +18,8 @@ enum {
> };
>
> typedef void (*tls_done_func_t)(void *data, int status,
> - key_serial_t peerid);
> + key_serial_t peerid,
> + key_serial_t handshake_session_id);
>
> struct tls_handshake_args {
> struct socket *ta_sock;
> @@ -31,6 +32,7 @@ struct tls_handshake_args {
> key_serial_t ta_my_privkey;
> unsigned int ta_num_peerids;
> key_serial_t ta_my_peerids[5];
> + key_serial_t handshake_session_id;

Please follow the existing field name convention and add a "ta_"
prefix.

> };
>
> int tls_client_hello_anon(const struct tls_handshake_args *args, gfp_t flags);
> diff --git a/include/uapi/linux/handshake.h b/include/uapi/linux/handshake.h
> index 662e7de46c54..b68ffbaa5f31 100644
> --- a/include/uapi/linux/handshake.h
> +++ b/include/uapi/linux/handshake.h
> @@ -55,6 +55,7 @@ enum {
> HANDSHAKE_A_DONE_STATUS = 1,
> HANDSHAKE_A_DONE_SOCKFD,
> HANDSHAKE_A_DONE_REMOTE_AUTH,
> + HANDSHAKE_A_DONE_SESSION_ID,
>
> __HANDSHAKE_A_DONE_MAX,
> HANDSHAKE_A_DONE_MAX = (__HANDSHAKE_A_DONE_MAX - 1)
> diff --git a/net/handshake/genl.c b/net/handshake/genl.c
> index f55d14d7b726..6cdce7e5dbc0 100644
> --- a/net/handshake/genl.c
> +++ b/net/handshake/genl.c
> @@ -16,10 +16,11 @@ static const struct nla_policy handshake_accept_nl_policy[HANDSHAKE_A_ACCEPT_HAN
> };
>
> /* HANDSHAKE_CMD_DONE - do */
> -static const struct nla_policy handshake_done_nl_policy[HANDSHAKE_A_DONE_REMOTE_AUTH + 1] = {
> +static const struct nla_policy handshake_done_nl_policy[HANDSHAKE_A_DONE_SESSION_ID + 1] = {
> [HANDSHAKE_A_DONE_STATUS] = { .type = NLA_U32, },
> [HANDSHAKE_A_DONE_SOCKFD] = { .type = NLA_S32, },
> [HANDSHAKE_A_DONE_REMOTE_AUTH] = { .type = NLA_U32, },
> + [HANDSHAKE_A_DONE_SESSION_ID] = { .type = NLA_U32, },
> };
>
> /* Ops table for handshake */
> @@ -35,7 +36,7 @@ static const struct genl_split_ops handshake_nl_ops[] = {
> .cmd = HANDSHAKE_CMD_DONE,
> .doit = handshake_nl_done_doit,
> .policy = handshake_done_nl_policy,
> - .maxattr = HANDSHAKE_A_DONE_REMOTE_AUTH,
> + .maxattr = HANDSHAKE_A_DONE_SESSION_ID,
> .flags = GENL_CMD_CAP_DO,
> },
> };
> diff --git a/net/handshake/tlshd.c b/net/handshake/tlshd.c
> index 081093dfd553..85c5fed690c0 100644
> --- a/net/handshake/tlshd.c
> +++ b/net/handshake/tlshd.c
> @@ -26,7 +26,8 @@
>
> struct tls_handshake_req {
> void (*th_consumer_done)(void *data, int status,
> - key_serial_t peerid);
> + key_serial_t peerid,
> + key_serial_t handshake_session_id);
> void *th_consumer_data;
>
> int th_type;
> @@ -39,6 +40,8 @@ struct tls_handshake_req {
>
> unsigned int th_num_peerids;
> key_serial_t th_peerid[5];
> +
> + key_serial_t handshake_session_id;

Please follow the existing field name convention and add a "th_"
prefix.

> };
>
> static struct tls_handshake_req *
> @@ -55,6 +58,7 @@ 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;

Please define a TLS_NO_SESSION_ID symbolic constant for initializing
this field, which is not a private key.

> return treq;
> }
>
> @@ -83,6 +87,13 @@ static void tls_handshake_remote_peerids(struct tls_handshake_req *treq,
> if (i >= treq->th_num_peerids)
> break;
> }
> +
> + nla_for_each_attr(nla, head, len, rem) {
> + if (nla_type(nla) == HANDSHAKE_A_DONE_SESSION_ID) {
> + treq->handshake_session_id = nla_get_u32(nla);
> + break;
> + }
> + }
> }
>
> /**
> @@ -105,7 +116,7 @@ static void tls_handshake_done(struct handshake_req *req,
> set_bit(HANDSHAKE_F_REQ_SESSION, &req->hr_flags);
>
> treq->th_consumer_done(treq->th_consumer_data, -status,
> - treq->th_peerid[0]);
> + treq->th_peerid[0], treq->handshake_session_id);
> }
>
> #if IS_ENABLED(CONFIG_KEYS)
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 7b90abc5cf0e..2401b4c757f6 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -444,13 +444,15 @@ static void svc_tcp_kill_temp_xprt(struct svc_xprt *xprt)
> * @data: address of xprt to wake
> * @status: status of handshake
> * @peerid: serial number of key containing the remote peer's identity
> + * @handshake_session_id: serial number of the userspace session ID
> *
> * If a security policy is specified as an export option, we don't
> * have a specific export here to check. So we set a "TLS session
> * is present" flag on the xprt and let an upper layer enforce local
> * security policy.
> */
> -static void svc_tcp_handshake_done(void *data, int status, key_serial_t peerid)
> +static void svc_tcp_handshake_done(void *data, int status, key_serial_t peerid,
> + key_serial_t handshake_session_id)
> {
> struct svc_xprt *xprt = data;
> struct svc_sock *svsk = container_of(xprt, struct svc_sock, sk_xprt);
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 3aa987e7f072..5c6e7543f293 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -2589,9 +2589,11 @@ static int xs_tcp_tls_finish_connecting(struct rpc_xprt *lower_xprt,
> * @data: address of xprt to wake
> * @status: status of handshake
> * @peerid: serial number of key containing the remote's identity
> + * @handshake_session_id: serial number of the userspace session ID
> *
> */
> -static void xs_tls_handshake_done(void *data, int status, key_serial_t peerid)
> +static void xs_tls_handshake_done(void *data, int status, key_serial_t peerid,
> + key_serial_t handshake_session_id)
> {
> struct rpc_xprt *lower_xprt = data;
> struct sock_xprt *lower_transport =

Aside from a few nits:

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

--
Chuck Lever