From 4ab26b955e1283c258ce8a854a6a33a6bbce514c Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 18 Aug 2026 15:19:35 -0400 Subject: [PATCH 1/4] tlshd: Fail the handshake request when message parsing fails tlshd_genl_valid_handler() returns NL_STOP when it cannot parse an accept message. libnl reports NL_STOP to its caller as success. nl_recvmsgs_default() returns zero, and tlshd_genl_get_handshake_parms() hands back handshake parameters it never finished filling in. tlshd then services the socket with a sockfd it could not read a peer address from, or with ip_proto left at -1. Return a negative libnl error code instead. recvmsgs() propagates it to nl_recvmsgs_default(), and tlshd_genl_get_handshake_parms() already converts a negative return to EINVAL. tlshd_service_socket() skips the handshake and reports the failure to the kernel. tlshd_genl_event_handler() returns NL_SKIP and nothing else. Drop the NL_OK and NL_STOP retvals its documentation still lists. Signed-off-by: Chuck Lever --- src/tlshd/netlink.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index 0f8de30..682f39a 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -355,9 +355,7 @@ static int tlshd_sig_poll_fd; * @param[in] msg A netlink event to be handled * @param[in] arg Additional arguments * - * @retval NL_OK Proceed with the next message * @retval NL_SKIP Skip this message. - * @retval NL_STOP Stop and discard remaining messages. */ static int tlshd_genl_event_handler(struct nl_msg *msg, __attribute__ ((unused)) void *arg) @@ -574,9 +572,11 @@ static void tlshd_parse_certificate(struct tlshd_handshake_parms *parms, * @param[in] msg Message to be processed * @param[out] arg Handshake parms to be filled in * - * @retval NL_OK Proceed with the next message + * libnl reports NL_STOP to its caller as success, so a failure to + * parse the message has to return a negative libnl error code. + * * @retval NL_SKIP Skip this message. - * @retval NL_STOP Stop and discard remaining messages. + * @retval <0 Negative libnl error code */ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) { @@ -594,7 +594,7 @@ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) tlshd_accept_nl_policy); if (err < 0) { tlshd_log_nl_error("genlmsg_parse", err); - return NL_STOP; + return -NLE_FAILURE; } if (tb[HANDSHAKE_A_ACCEPT_SOCKFD]) { @@ -607,13 +607,13 @@ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) sap = (struct sockaddr *)&addr; if (getpeername(parms->sockfd, sap, &salen) == -1) { tlshd_log_perror("getpeername"); - return NL_STOP; + return -NLE_FAILURE; } err = getnameinfo(sap, salen, buf, sizeof(buf), NULL, 0, NI_NUMERICHOST); if (err) { tlshd_log_gai_error(err); - return NL_STOP; + return -NLE_FAILURE; } parms->peeraddr = strdup(buf); @@ -621,7 +621,7 @@ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) if (getsockopt(parms->sockfd, SOL_SOCKET, SO_PROTOCOL, &proto, &optlen) == -1) { tlshd_log_perror("getsockopt (SO_PROTOCOL)"); - return NL_STOP; + return -NLE_FAILURE; } parms->ip_proto = proto; } @@ -656,7 +656,7 @@ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) NULL, 0, NI_NAMEREQD); if (err) { tlshd_log_gai_error(err); - return NL_STOP; + return -NLE_FAILURE; } parms->peername = strdup(buf); } From c9276380af3bd05f0731c5961ba39625160bf7cf Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 18 Aug 2026 15:56:13 -0400 Subject: [PATCH 2/4] tlshd: Fail client handshakes that have no peer name HANDSHAKE_A_ACCEPT_PEERNAME is optional. The kernel emits it only for a consumer that set ta_peername. Both TLS 1.3 client handshake paths pass parms->peername to strlen() without checking. Only the SUNRPC client requests a client-side handshake today, and it always sets ta_peername, so the dereference is unreachable. Nothing in the protocol keeps it that way. Guarding the strlen() and continuing would trade the dereference for a weaker session. The peer name also reaches gnutls_session_set_verify_cert() and gnutls_certificate_verify_peers3(). Given a NULL hostname, GnuTLS checks the certificate chain but not who presented it. Nothing on the kernel side makes up the difference, because xs_tls_handshake_done() ignores the peerid. The QUIC client path guards its strlen() already, and completes exactly that unverified handshake. Reject the request instead when the peer name is missing. session_status is already EIO, so the early return fails the kernel's request rather than stranding the socket. The QUIC path's test for a peer name is then redundant and goes away. Signed-off-by: Chuck Lever --- src/tlshd/client.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/tlshd/client.c b/src/tlshd/client.c index 4ed30ca..30bfa57 100644 --- a/src/tlshd/client.c +++ b/src/tlshd/client.c @@ -96,6 +96,16 @@ static void tlshd_tls13_client_anon_handshake(struct tlshd_handshake_parms *parm unsigned int flags; int ret; + /* + * Without a peer name, GnuTLS verifies the certificate chain + * but not who presented it. session_status is already EIO, so + * the early return fails the kernel's request. + */ + if (!parms->peername) { + tlshd_log_error("No peer name: cannot verify the server's identity"); + return; + } + ret = gnutls_certificate_allocate_credentials(&xcred); if (ret != GNUTLS_E_SUCCESS) { tlshd_log_gnutls_error(ret); @@ -417,6 +427,11 @@ static void tlshd_tls13_client_x509_handshake(struct tlshd_handshake_parms *parm unsigned int flags; int ret; + if (!parms->peername) { + tlshd_log_error("No peer name: cannot verify the server's identity"); + return; + } + ret = gnutls_certificate_allocate_credentials(&xcred); if (ret != GNUTLS_E_SUCCESS) { tlshd_log_gnutls_error(ret); @@ -646,6 +661,11 @@ static void tlshd_quic_client_set_x509_session(struct tlshd_quic_conn *conn) gnutls_session_t session; int ret; + if (!parms->peername) { + tlshd_log_error("No peer name: cannot verify the server's identity"); + return; + } + if (conn->cert_req != TLSHD_QUIC_NO_CERT_AUTH) { if (!tlshd_x509_client_get_certs(parms) || !tlshd_x509_client_get_privkey(parms)) { tlshd_log_error("Failed to get cert or privkey"); @@ -683,12 +703,10 @@ static void tlshd_quic_client_set_x509_session(struct tlshd_quic_conn *conn) ret = gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, cred); if (ret) goto err_session; - if (parms->peername) { - ret = gnutls_server_name_set(session, GNUTLS_NAME_DNS, - parms->peername, strlen(parms->peername)); - if (ret) - goto err_session; - } + ret = gnutls_server_name_set(session, GNUTLS_NAME_DNS, + parms->peername, strlen(parms->peername)); + if (ret) + goto err_session; conn->session = session; return; From 5c2db4b1a6432a9014d458dd00c5ac2d40b870af Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 18 Aug 2026 16:01:48 -0400 Subject: [PATCH 3/4] tlshd: Keep a failed reverse lookup from failing the request tlshd_genl_valid_handler() fails the handshake request when getnameinfo() cannot map the peer address to a name. That lookup is a fallback for a request that carries no HANDSHAKE_A_ACCEPT_PEERNAME. Only the SUNRPC client sets ta_peername, so the fallback runs for the NFSD and NVMe handshakes, and none of them read the result. A resolver that cannot answer must not fail those handshakes. Leave the name unset and proceed. The client handshake paths are the only ones that need a peer name, and they already reject a request that arrives without one. Signed-off-by: Chuck Lever --- src/tlshd/netlink.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index 682f39a..cc9f150 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -652,13 +652,13 @@ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) else if (sap) { char buf[NI_MAXHOST]; + /* A peer name is optional: leave it unset and proceed. */ err = getnameinfo(sap, salen, buf, sizeof(buf), NULL, 0, NI_NAMEREQD); - if (err) { + if (err) tlshd_log_gai_error(err); - return -NLE_FAILURE; - } - parms->peername = strdup(buf); + else + parms->peername = strdup(buf); } return NL_SKIP; From 75d0affa6222758ac343e077510fa64bf00e5739 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 18 Aug 2026 15:32:14 -0400 Subject: [PATCH 4/4] tlshd: Detect kernel capabilities from the netlink policy tlshd_probe_attr() sends a HANDSHAKE_CMD_DONE message carrying the attribute under test and a sockfd of -1, then reads nl_send_auto()'s return to decide whether the kernel accepts that attribute. That return is the count of bytes written to the socket. The kernel has not seen the message yet, so every attribute reads as supported. An administrator who configures session tags for a kernel whose HANDSHAKE_CMD_DONE policy has no HANDSHAKE_A_DONE_TAG entry gets a DONE message the kernel rejects. The handshake completion is lost. Reading the reply does not rescue the probe. The sockfd of -1 that makes the probe harmless is the one sockfd_lookup() rejects, so the reply carries an error whether or not the kernel knows the attribute. Ask the generic netlink control family instead. CTRL_CMD_GETPOLICY dumps the attribute policy for one command, and an attribute the policy rejects is left out of the dump. Per-command policy dumping arrived in v5.10 and the handshake family in v6.5, so every kernel that offers the family can answer. Give the dump its own socket rather than borrowing the notification socket, which by then has joined the tlshd multicast group and has the event handler installed. These control attributes come from the build host's UAPI headers. Nothing else in tlshd requires headers that recent, so a host with pre-v5.10 headers builds the daemon today. Test for them at configure time and record the requirement in README, so that host gets one clear diagnostic instead of a compile failure in netlink.c. Signed-off-by: Chuck Lever --- README | 4 + README.md | 4 + configure.ac | 11 +++ src/tlshd/netlink.c | 233 +++++++++++++++++++++++++++++++------------- 4 files changed, 183 insertions(+), 69 deletions(-) diff --git a/README b/README index 9392ef5..a35e7ef 100644 --- a/README +++ b/README @@ -35,6 +35,10 @@ following libraries to be installed: * libnl3 * libyaml +The Linux UAPI headers must be v5.10 or later. tlshd reads the +kernel's generic netlink attribute policy to detect optional +handshake features, and the definitions for that arrived in v5.10. + ## Installation See [NEWS](NEWS) to see what has changed in the latest release, diff --git a/README.md b/README.md index 9392ef5..a35e7ef 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,10 @@ following libraries to be installed: * libnl3 * libyaml +The Linux UAPI headers must be v5.10 or later. tlshd reads the +kernel's generic netlink attribute policy to detect optional +handshake features, and the definitions for that arrived in v5.10. + ## Installation See [NEWS](NEWS) to see what has changed in the latest release, diff --git a/configure.ac b/configure.ac index a17cef8..7db41bc 100644 --- a/configure.ac +++ b/configure.ac @@ -136,6 +136,17 @@ if test "x$have_tls_tx_max_payload_len" = xyes ; then AC_DEFINE([HAVE_TLS_TX_MAX_PAYLOAD_LEN], [1], [Define to 1 if linux/tls.h defines TLS_TX_MAX_PAYLOAD_LEN]) fi +AC_MSG_CHECKING(for CTRL_ATTR_OP_POLICY in linux/genetlink.h) +AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([[ #include ]], + [[ (void) CTRL_ATTR_OP_POLICY; ]])], + [ have_ctrl_attr_op_policy=yes ], + [ have_ctrl_attr_op_policy=no ]) +AC_MSG_RESULT([$have_ctrl_attr_op_policy]) +if test "x$have_ctrl_attr_op_policy" = xno ; then + AC_MSG_ERROR([Linux UAPI headers v5.10 or later are required]) +fi + AC_SUBST([AM_CPPFLAGS]) AC_CONFIG_FILES([Makefile \ diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index cc9f150..52984c5 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -42,6 +42,7 @@ #include #include #include +#include #include #include @@ -213,97 +214,191 @@ static void tlshd_genl_sock_close(struct nl_sock *nls) } /** - * @brief Probe whether the kernel supports a specific netlink attribute - * @param[in] nls Netlink socket - * @param[in] cmd Netlink command (e.g., HANDSHAKE_CMD_DONE) - * @param[in] attr_type Attribute type to test - * - * Sends a test message with the specified attribute and minimal - * required fields. The kernel rejects the message for having invalid - * required fields, but this determines whether it parsed the optional - * attribute without error. - * - * @retval true Kernel accepts this attribute type - * @retval false Kernel rejected the attribute as unsupported + * @struct tlshd_op_policy + * @brief The attribute types a kernel accepts for one netlink command */ -static bool tlshd_probe_attr(struct nl_sock *nls, int cmd, int attr_type) +struct tlshd_op_policy { + int cmd; /**< Command being probed */ + uint32_t policy_id; /**< Policy index the kernel assigned */ + bool have_id; /**< policy_id has been read */ + uint32_t attrs; /**< Accepted attribute types */ +}; + +/** + * @def TLSHD_OP_POLICY_ATTRS + * Number of attribute types that fit in tlshd_op_policy::attrs + */ +#define TLSHD_OP_POLICY_ATTRS (32) + +/* + * A DONE attribute numbered past the width of that bitmask gives this + * typedef a negative size. The build then fails where the attribute is + * added, rather than the daemon quietly reporting it unsupported. + */ +typedef char tlshd_done_attrs_fit + [HANDSHAKE_A_DONE_MAX < TLSHD_OP_POLICY_ATTRS ? 1 : -1]; + +/** + * @var struct nla_policy tlshd_ctrl_op_policy + * Netlink policy for the per-command nests in CTRL_ATTR_OP_POLICY + */ +#if LIBNL_VER_NUM >= LIBNL_VER(3,5) +static const struct nla_policy +#else +static struct nla_policy +#endif +tlshd_ctrl_op_policy[CTRL_ATTR_POLICY_MAX + 1] = { + [CTRL_ATTR_POLICY_DO] = { .type = NLA_U32, }, + [CTRL_ATTR_POLICY_DUMP] = { .type = NLA_U32, }, +}; + +/** + * @brief Collect one command's attribute policy from a dump message + * @param[in] msg Message to be processed + * @param[in,out] arg struct tlshd_op_policy to be filled in + * + * The kernel reports a policy in two parts. A CTRL_ATTR_OP_POLICY + * message maps the command to a policy index, and the CTRL_ATTR_POLICY + * messages that follow carry one attribute apiece, nested under that + * index. An attribute the policy rejects is left out of the dump, so + * the presence of a nest is the answer this probe wants. + * + * @retval NL_SKIP Skip this message. + */ +static int tlshd_policy_valid_handler(struct nl_msg *msg, void *arg) { + struct tlshd_op_policy *policy = arg; + struct nlattr *tb[CTRL_ATTR_MAX + 1]; + struct nlattr *pol, *attr; + int rem, rem2; + + if (genlmsg_parse(nlmsg_hdr(msg), 0, tb, CTRL_ATTR_MAX, NULL) < 0) + return NL_SKIP; + + if (tb[CTRL_ATTR_OP_POLICY]) { + nla_for_each_nested(pol, tb[CTRL_ATTR_OP_POLICY], rem) { + struct nlattr *op[CTRL_ATTR_POLICY_MAX + 1]; + + if (nla_type(pol) != policy->cmd) + continue; + if (nla_parse_nested(op, CTRL_ATTR_POLICY_MAX, pol, + tlshd_ctrl_op_policy) < 0) + continue; + if (op[CTRL_ATTR_POLICY_DO]) { + policy->policy_id = + nla_get_u32(op[CTRL_ATTR_POLICY_DO]); + policy->have_id = true; + } + } + } + + if (tb[CTRL_ATTR_POLICY] && policy->have_id) { + nla_for_each_nested(pol, tb[CTRL_ATTR_POLICY], rem) { + if ((uint32_t)nla_type(pol) != policy->policy_id) + continue; + nla_for_each_nested(attr, pol, rem2) { + int type = nla_type(attr); + + if (type > 0 && type < TLSHD_OP_POLICY_ATTRS) + policy->attrs |= 1U << type; + } + } + } + + return NL_SKIP; +} + +/** + * @brief Retrieve the attribute policy the kernel applies to a command + * @param[in] cmd Netlink command (e.g., HANDSHAKE_CMD_DONE) + * @param[out] policy Filled in with the accepted attribute types + * + * @retval true The kernel reported a policy for this command + * @retval false No policy could be retrieved + */ +static bool tlshd_get_op_policy(int cmd, struct tlshd_op_policy *policy) +{ + struct nl_sock *nls; struct nl_msg *msg; - int family_id, err; - bool supported; + bool ret = false; + int err; - family_id = genl_ctrl_resolve(nls, HANDSHAKE_FAMILY_NAME); - if (family_id < 0) - return false; + memset(policy, 0, sizeof(*policy)); + policy->cmd = cmd; - msg = nlmsg_alloc(); - if (!msg) + if (tlshd_genl_sock_open(&nls)) return false; - genlmsg_put(msg, NL_AUTO_PID, NL_AUTO_SEQ, family_id, 0, - NLM_F_REQUEST, cmd, HANDSHAKE_FAMILY_VERSION); + nl_socket_modify_cb(nls, NL_CB_VALID, NL_CB_CUSTOM, + tlshd_policy_valid_handler, policy); - switch (cmd) { - case HANDSHAKE_CMD_DONE: - nla_put_u32(msg, HANDSHAKE_A_DONE_STATUS, 0); - nla_put_u32(msg, HANDSHAKE_A_DONE_SOCKFD, -1); - break; - default: - nlmsg_free(msg); - return false; + msg = nlmsg_alloc(); + if (!msg) { + tlshd_log_error("Failed to allocate message buffer."); + goto out_close; } - switch (attr_type) { - case HANDSHAKE_A_DONE_TAG: - nla_put_string(msg, attr_type, "__probe__"); - break; - case HANDSHAKE_A_DONE_REMOTE_AUTH: - nla_put_s32(msg, attr_type, 0); - break; - default: - tlshd_log_error("Attribute %d not supported", attr_type); - nlmsg_free(msg); - return false; + if (!genlmsg_put(msg, NL_AUTO_PORT, NL_AUTO_SEQ, GENL_ID_CTRL, 0, + NLM_F_DUMP, CTRL_CMD_GETPOLICY, 1)) { + tlshd_log_error("Failed to set up message header."); + goto out_msgfree; + } + + err = nla_put_string(msg, CTRL_ATTR_FAMILY_NAME, + HANDSHAKE_FAMILY_NAME); + if (err < 0) { + tlshd_log_nl_error("nla_put family name", err); + goto out_msgfree; + } + err = nla_put_u32(msg, CTRL_ATTR_OP, cmd); + if (err < 0) { + tlshd_log_nl_error("nla_put op", err); + goto out_msgfree; } err = nl_send_auto(nls, msg); - nlmsg_free(msg); + if (err < 0) { + tlshd_log_nl_error("nl_send_auto", err); + goto out_msgfree; + } - /* - * nl_send_auto() returns the number of bytes sent on success, - * or a negative error code on failure. Treat any failure as - * the attribute being unsupported; a positive return indicates - * the kernel accepted the message containing this attribute. - */ - supported = (err >= 0); - /* Drain kernel response to prevent stale data on socket reuse */ - nl_recvmsgs_default(nls); - - return supported; + err = nl_recvmsgs_default(nls); + if (err < 0) { + tlshd_log_nl_error("CTRL_CMD_GETPOLICY", err); + goto out_msgfree; + } + + ret = policy->have_id; + +out_msgfree: + nlmsg_free(msg); +out_close: + tlshd_genl_sock_close(nls); + return ret; } /** * @brief Detect which optional netlink attributes the kernel supports - * @param[in] nls Netlink socket * - * Probes the kernel to determine which optional handshake netlink - * attributes are supported. Results are cached in tlshd_kernel_caps - * for use throughout the daemon lifetime. Unsupported attributes are - * not included in subsequent netlink messages to avoid rejection. + * Reads the kernel's attribute policy for HANDSHAKE_CMD_DONE. Results + * are cached in tlshd_kernel_caps for use throughout the daemon + * lifetime. Unsupported attributes are not included in subsequent + * netlink messages to avoid rejection. A kernel that reports no policy + * leaves every capability off, which is the conservative choice. * - * This function should be called once during initialization, after - * connecting to the handshake netlink family but before processing - * any handshake requests. + * This function should be called once during initialization, before + * processing any handshake requests. */ -static void tlshd_detect_kernel_caps(struct nl_sock *nls) +static void tlshd_detect_kernel_caps(void) { - tlshd_kernel_caps.done_tag = - tlshd_probe_attr(nls, HANDSHAKE_CMD_DONE, - HANDSHAKE_A_DONE_TAG); + struct tlshd_op_policy policy; - tlshd_kernel_caps.done_remote_auth = - tlshd_probe_attr(nls, HANDSHAKE_CMD_DONE, - HANDSHAKE_A_DONE_REMOTE_AUTH); + if (tlshd_get_op_policy(HANDSHAKE_CMD_DONE, &policy)) { + tlshd_kernel_caps.done_tag = + policy.attrs & (1U << HANDSHAKE_A_DONE_TAG); + tlshd_kernel_caps.done_remote_auth = + policy.attrs & (1U << HANDSHAKE_A_DONE_REMOTE_AUTH); + } tlshd_log_notice("Kernel capabilities: " "session_tags=%s remote_peerids=%s", @@ -430,7 +525,7 @@ void tlshd_genl_dispatch(void) } /* Detect which optional netlink attributes the kernel supports */ - tlshd_detect_kernel_caps(tlshd_notification_nls); + tlshd_detect_kernel_caps(); if (signal(SIGCHLD, SIG_IGN) == SIG_ERR) { tlshd_log_perror("signal");