From 78e91a7f7cb29c045617072d32ec6d07b3c3b22a Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Mon, 2 Jun 2025 15:03:59 -0400 Subject: [PATCH 01/32] tlshd: Fix a minor race Parfait complains about using a pathname to perform an access(2) and then passing the same pathname to open(2). Between the access(2) and the open(2) calls, the permissions can change. I think this is harmless for tlshd, but all the same, let's clean this up. Signed-off-by: Chuck Lever --- src/tlshd/config.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/tlshd/config.c b/src/tlshd/config.c index be5d472..f4907eb 100644 --- a/src/tlshd/config.c +++ b/src/tlshd/config.c @@ -140,7 +140,11 @@ static bool tlshd_config_read_datum(const char *pathname, gnutls_datum_t *data, fd = open(pathname, O_RDONLY); if (fd == -1) { - tlshd_log_perror("open"); + if (access(pathname, F_OK)) + tlshd_log_debug("tlshd cannot access \"%s\"", + pathname); + else + tlshd_log_perror("open"); goto out; } if (fstat(fd, &statbuf)) { @@ -198,7 +202,7 @@ bool tlshd_config_get_client_truststore(char **bundle) g_error_free(error); return false; } else if (access(pathname, F_OK)) { - tlshd_log_debug("client x509.truststore pathname \"%s\" is not accessible", pathname); + tlshd_log_debug("tlshd cannot access \"%s\"", pathname); g_free(pathname); return false; } @@ -234,10 +238,6 @@ bool tlshd_config_get_client_certs(gnutls_pcert_st *certs, if (!pathname) { g_error_free(error); return false; - } else if (access(pathname, F_OK)) { - tlshd_log_debug("client x509.certificate pathname \"%s\" is not accessible", pathname); - g_free(pathname); - return false; } if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER, @@ -282,10 +282,6 @@ bool tlshd_config_get_client_privkey(gnutls_privkey_t *privkey) if (!pathname) { g_error_free(error); return false; - } else if (access(pathname, F_OK)) { - tlshd_log_debug("client x509.private_key pathname \"%s\" is not accessible", pathname); - g_free(pathname); - return false; } if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER, @@ -336,7 +332,7 @@ bool tlshd_config_get_server_truststore(char **bundle) g_error_free(error); return false; } else if (access(pathname, F_OK)) { - tlshd_log_debug("server x509.truststore pathname \"%s\" is not accessible", pathname); + tlshd_log_debug("tlshd cannot access \"%s\"", pathname); g_free(pathname); return false; } @@ -372,10 +368,6 @@ bool tlshd_config_get_server_certs(gnutls_pcert_st *certs, if (!pathname) { g_error_free(error); return false; - } else if (access(pathname, F_OK)) { - tlshd_log_debug("server x509.certificate pathname \"%s\" is not accessible", pathname); - g_free(pathname); - return false; } if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER, @@ -420,10 +412,6 @@ bool tlshd_config_get_server_privkey(gnutls_privkey_t *privkey) if (!pathname) { g_error_free(error); return false; - } else if (access(pathname, F_OK)) { - tlshd_log_debug("server x509.privkey pathname \"%s\" is not accessible", pathname); - g_free(pathname); - return false; } if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER, From dbdedccfd22e804623518eb82f7bc0f249a0b5ac Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Mon, 2 Jun 2025 15:04:08 -0400 Subject: [PATCH 02/32] tlshd: Remove unneeded variable "error" Clean up: Commit c380357cf8f1 ("tlshd: Demote warnings about missing options") removed some debug messages that used the local "error" variable. "error" is passed to glib, but the returned value is no longer checked or used. Fixes: c380357cf8f1 ("tlshd: Demote warnings about missing options") Signed-off-by: Chuck Lever --- src/tlshd/config.c | 44 ++++++++++++++------------------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/src/tlshd/config.c b/src/tlshd/config.c index f4907eb..e050d3d 100644 --- a/src/tlshd/config.c +++ b/src/tlshd/config.c @@ -193,15 +193,13 @@ static bool tlshd_config_read_datum(const char *pathname, gnutls_datum_t *data, */ bool tlshd_config_get_client_truststore(char **bundle) { - GError *error = NULL; gchar *pathname; pathname = g_key_file_get_string(tlshd_configuration, "authenticate.client", - "x509.truststore", &error); - if (!pathname) { - g_error_free(error); + "x509.truststore", NULL); + if (!pathname) return false; - } else if (access(pathname, F_OK)) { + if (access(pathname, F_OK)) { tlshd_log_debug("tlshd cannot access \"%s\"", pathname); g_free(pathname); return false; @@ -228,17 +226,14 @@ bool tlshd_config_get_client_truststore(char **bundle) bool tlshd_config_get_client_certs(gnutls_pcert_st *certs, unsigned int *certs_len) { - GError *error = NULL; gnutls_datum_t data; gchar *pathname; int ret; pathname = g_key_file_get_string(tlshd_configuration, "authenticate.client", - "x509.certificate", &error); - if (!pathname) { - g_error_free(error); + "x509.certificate", NULL); + if (!pathname) return false; - } if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER, TLSHD_CERT_MODE)) { @@ -272,17 +267,14 @@ bool tlshd_config_get_client_certs(gnutls_pcert_st *certs, */ bool tlshd_config_get_client_privkey(gnutls_privkey_t *privkey) { - GError *error = NULL; gnutls_datum_t data; gchar *pathname; int ret; pathname = g_key_file_get_string(tlshd_configuration, "authenticate.client", - "x509.private_key", &error); - if (!pathname) { - g_error_free(error); + "x509.private_key", NULL); + if (!pathname) return false; - } if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER, TLSHD_PRIVKEY_MODE)) { @@ -323,15 +315,13 @@ bool tlshd_config_get_client_privkey(gnutls_privkey_t *privkey) */ bool tlshd_config_get_server_truststore(char **bundle) { - GError *error = NULL; gchar *pathname; pathname = g_key_file_get_string(tlshd_configuration, "authenticate.server", - "x509.truststore", &error); - if (!pathname) { - g_error_free(error); + "x509.truststore", NULL); + if (!pathname) return false; - } else if (access(pathname, F_OK)) { + if (access(pathname, F_OK)) { tlshd_log_debug("tlshd cannot access \"%s\"", pathname); g_free(pathname); return false; @@ -358,17 +348,14 @@ bool tlshd_config_get_server_truststore(char **bundle) bool tlshd_config_get_server_certs(gnutls_pcert_st *certs, unsigned int *certs_len) { - GError *error = NULL; gnutls_datum_t data; gchar *pathname; int ret; pathname = g_key_file_get_string(tlshd_configuration, "authenticate.server", - "x509.certificate", &error); - if (!pathname) { - g_error_free(error); + "x509.certificate", NULL); + if (!pathname) return false; - } if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER, TLSHD_CERT_MODE)) { @@ -402,17 +389,14 @@ bool tlshd_config_get_server_certs(gnutls_pcert_st *certs, */ bool tlshd_config_get_server_privkey(gnutls_privkey_t *privkey) { - GError *error = NULL; gnutls_datum_t data; gchar *pathname; int ret; pathname = g_key_file_get_string(tlshd_configuration, "authenticate.server", - "x509.private_key", &error); - if (!pathname) { - g_error_free(error); + "x509.private_key", NULL); + if (!pathname) return false; - } if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER, TLSHD_PRIVKEY_MODE)) { From 3ff8a366a84a1b0ac006436ee27d716324ce19ff Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Mon, 2 Jun 2025 15:52:47 -0400 Subject: [PATCH 03/32] workflows: Limit permission of the makefile.yml action Suggested by CodeQL. Signed-off-by: Chuck Lever --- .github/workflows/makefile.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/makefile.yml b/.github/workflows/makefile.yml index 5d16bd9..3a0ee4f 100644 --- a/.github/workflows/makefile.yml +++ b/.github/workflows/makefile.yml @@ -6,6 +6,7 @@ jobs: build: runs-on: ubuntu-latest + permissions: read-all strategy: fail-fast: false matrix: From 96340c1a47bea623c693c0191ea7cee86e5a1ac9 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 5 Jun 2025 12:22:08 -0400 Subject: [PATCH 04/32] tlshd: Add default keyrings for NFS The NFS mount command is to add keys to the .nfs keyring. Also add a keyring for NFSD configuration. Signed-off-by: Chuck Lever --- src/tlshd/config.c | 11 +++++++---- src/tlshd/tlshd.conf.man | 8 +++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/tlshd/config.c b/src/tlshd/config.c index e050d3d..b41051e 100644 --- a/src/tlshd/config.c +++ b/src/tlshd/config.c @@ -99,15 +99,18 @@ bool tlshd_config_init(const gchar *pathname) for (i = 0; i < length; i++) { if (!strcmp(keyrings[i], ".nvme")) continue; + if (!strcmp(keyrings[i], ".nfs")) + continue; + if (!strcmp(keyrings[i], ".nfsd")) + continue; tlshd_keyring_link_session(keyrings[i]); } g_strfreev(keyrings); } - /* - * Always link the default nvme subsystem keyring into the - * session. - */ + /* The ".nvme", ".nfs", and ".nfsd" keyrings cannot be disabled. */ tlshd_keyring_link_session(".nvme"); + tlshd_keyring_link_session(".nfs"); + tlshd_keyring_link_session(".nfsd"); return true; } diff --git a/src/tlshd/tlshd.conf.man b/src/tlshd/tlshd.conf.man index 9d6d92f..abb2f99 100644 --- a/src/tlshd/tlshd.conf.man +++ b/src/tlshd/tlshd.conf.man @@ -79,7 +79,13 @@ that contain handshake authentication tokens. .B tlshd links these keyrings into its session keyring. The configuration file may specify either a keyring's name or serial number. -The default is to provide no keyring. +.B tlshd +always includes the +.IR .nvme , +.IR .nfs , +and +.I .nfsd +keyrings on its session keyring. .P And, in this section, there are two subsections: .I [client] From 69b6a5402d6fae375b005e88b22b31ef10d9bcf3 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 6 Jun 2025 20:41:10 -0400 Subject: [PATCH 05/32] tlshd: Relocate TLSHD_ALLPERMS Clean up: Move it next to the other related macro definitions. Signed-off-by: Chuck Lever --- src/tlshd/config.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tlshd/config.c b/src/tlshd/config.c index b41051e..df26663 100644 --- a/src/tlshd/config.c +++ b/src/tlshd/config.c @@ -46,12 +46,6 @@ static GKeyFile *tlshd_configuration; -/** - * ALLPERMS exists in glibc, but not on musl, so we - * manually define TLSHD_ACCESSPERMS instead of using ALLPERMS. - */ -#define TLSHD_ACCESSPERMS (S_IRWXU|S_IRWXG|S_IRWXO) - /** * tlshd_config_init - Read tlshd's config file * @pathname: Pathname to config file @@ -120,6 +114,12 @@ void tlshd_config_shutdown(void) g_key_file_free(tlshd_configuration); } +/** + * ALLPERMS exists in glibc, but not on musl, so we manually + * define TLSHD_ACCESSPERMS instead of using ALLPERMS. + */ +#define TLSHD_ACCESSPERMS (S_IRWXU|S_IRWXG|S_IRWXO) + /* * Expected file attributes */ From 731d1b2bf8d6427226becf3b4c237ea65f41134c Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sun, 15 Jun 2025 21:08:36 -0400 Subject: [PATCH 06/32] tlshd: Handshake needs to check for CERTIFICATE_ERROR tlshd's certificate verification functions return GNUTLS_E_CERTIFICATE_ERROR, not GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR, when certification verification is unsuccessful. This gets rid of an unexpected journal message: tlshd_start_tls_handshake unhandled error -43, returning EACCES Fixes: ecaf6608da74 ("tlshd: Report peer certificate verification failures") Signed-off-by: Chuck Lever --- src/tlshd/handshake.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tlshd/handshake.c b/src/tlshd/handshake.c index b9de6b3..672b7a6 100644 --- a/src/tlshd/handshake.c +++ b/src/tlshd/handshake.c @@ -93,7 +93,7 @@ void tlshd_start_tls_handshake(gnutls_session_t session, /* Any errors here should default to blocking access: */ parms->session_status = EACCES; switch (ret) { - case GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR: + case GNUTLS_E_CERTIFICATE_ERROR: tlshd_log_cert_verification_error(session); break; case -ETIMEDOUT: From 67ef5a5d62dcbb7ab0b196223976b673ee2fdc6d Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 14 Jun 2025 11:35:22 -0400 Subject: [PATCH 07/32] tlshd: Fix silent tlshd_keyring_link_session() failures The error flow for find_key_by_type_and_desc() is wrong -- that API returns -1 when it fails, not zero. Fixes: a70f14354543 ("keyring: merge keyring lookup in tlshd_keyring_link_session()") Signed-off-by: Chuck Lever --- src/tlshd/keyring.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/tlshd/keyring.c b/src/tlshd/keyring.c index f499a76..e3c3166 100644 --- a/src/tlshd/keyring.c +++ b/src/tlshd/keyring.c @@ -263,9 +263,8 @@ int tlshd_keyring_link_session(const char *keyring) } serial = find_key_by_type_and_desc("keyring", keyring, 0); - if (!serial) { - tlshd_log_debug("Failed to lookup keyring '%s'\n", - keyring); + if (serial == -1) { + tlshd_log_debug("Failed to find keyring '%s'\n", keyring); errno = -ENOKEY; return -1; } From 46d9e84928d558a8d7dbb9da018f7aa46973b18e Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 14 Jun 2025 11:58:02 -0400 Subject: [PATCH 08/32] tlshd: Don't set errno in tlshd_keyring_link_session() - No caller ever examines the return value for tlshd_keyring_link_session(), thus the errno set here is always ignored. - The errno values set here are not documented. Clean up by removing set-but-unexamined variables. Signed-off-by: Chuck Lever --- src/tlshd/keyring.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tlshd/keyring.c b/src/tlshd/keyring.c index e3c3166..01c0c30 100644 --- a/src/tlshd/keyring.c +++ b/src/tlshd/keyring.c @@ -258,14 +258,12 @@ int tlshd_keyring_link_session(const char *keyring) if (!keyring) { tlshd_log_error("No keyring specified"); - errno = -EINVAL; return -1; } serial = find_key_by_type_and_desc("keyring", keyring, 0); if (serial == -1) { tlshd_log_debug("Failed to find keyring '%s'\n", keyring); - errno = -ENOKEY; return -1; } From cc49670781b2736a539a085ad6c66cdd20ca390a Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 14 Jun 2025 12:03:00 -0400 Subject: [PATCH 09/32] tlshd: Display errno message Usually we use tlshd_log_perror() in these case to convert the errno value to an error message. However, here there is more to display than just the name of the failing function. Signed-off-by: Chuck Lever --- src/tlshd/keyring.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tlshd/keyring.c b/src/tlshd/keyring.c index 01c0c30..38f34d1 100644 --- a/src/tlshd/keyring.c +++ b/src/tlshd/keyring.c @@ -269,8 +269,8 @@ int tlshd_keyring_link_session(const char *keyring) ret = keyctl_link(serial, KEY_SPEC_SESSION_KEYRING); if (ret < 0) { - tlshd_log_debug("Failed to link keyring %s (%lx) error %d\n", - keyring, serial, errno); + tlshd_log_debug("Failed to link keyring '%s' (%lx): %s\n", + keyring, serial, strerror(errno)); return -1; } From d3d5c3a5b517bf7790729268c48390e4601707c2 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 14 Jun 2025 12:05:09 -0400 Subject: [PATCH 10/32] tlshd: Check for an empty string For completeness, the sanity check at the top of tlshd_keyring_link_session() should check for an empty string in addition to a NULL string pointer. Signed-off-by: Chuck Lever --- src/tlshd/keyring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tlshd/keyring.c b/src/tlshd/keyring.c index 38f34d1..6aac02e 100644 --- a/src/tlshd/keyring.c +++ b/src/tlshd/keyring.c @@ -256,7 +256,7 @@ int tlshd_keyring_link_session(const char *keyring) key_serial_t serial; long ret; - if (!keyring) { + if (!keyring || keyring[0] == '\0') { tlshd_log_error("No keyring specified"); return -1; } From 058f25770486b97c0b685e31fc6b60d6e4e575ad Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 10 Jun 2025 20:04:53 -0400 Subject: [PATCH 11/32] tlshd: Show ingress certificate on successful handshake As a debugging aid and for auditing, emit a one-line summary of incoming certificate(s) after a successful handshake. Server-side only. Signed-off-by: Chuck Lever --- src/tlshd/server.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/tlshd/server.c b/src/tlshd/server.c index 72ff6f5..7f225d6 100644 --- a/src/tlshd/server.c +++ b/src/tlshd/server.c @@ -273,6 +273,28 @@ static void tlshd_tls13_server_x509_handshake(struct tlshd_handshake_parms *parm tlshd_start_tls_handshake(session, parms); + if (tlshd_debug && + gnutls_certificate_type_get(session) == GNUTLS_CRT_X509) { + const gnutls_datum_t *peercerts; + unsigned int i, num_certs = 0; + + peercerts = gnutls_certificate_get_peers(session, &num_certs); + for (i = 0; i < num_certs; i++) { + gnutls_x509_crt_t cert; + gnutls_datum_t cinfo; + + gnutls_x509_crt_init(&cert); + gnutls_x509_crt_import(cert, &peercerts[i], + GNUTLS_X509_FMT_DER); + if (gnutls_x509_crt_print(cert, GNUTLS_CRT_PRINT_ONELINE, + &cinfo) == 0) { + tlshd_log_debug("Peer certificate: %s", cinfo.data); + gnutls_free(cinfo.data); + } + gnutls_x509_crt_deinit(cert); + } + } + gnutls_deinit(session); out_free_creds: From 1177c26ef4717fa5d115183c87005436a6b15972 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sun, 15 Jun 2025 21:30:20 -0400 Subject: [PATCH 12/32] tlshd: Remove useless verification status report The verification routine has already succeeded. The verification status report shows only: The certificate is trusted. If the handshake is successful, that is obvious. Signed-off-by: Chuck Lever --- src/tlshd/client.c | 8 -------- src/tlshd/server.c | 6 ------ 2 files changed, 14 deletions(-) diff --git a/src/tlshd/client.c b/src/tlshd/client.c index 9c8f512..85699b5 100644 --- a/src/tlshd/client.c +++ b/src/tlshd/client.c @@ -207,9 +207,7 @@ static int tlshd_client_x509_verify_function(gnutls_session_t session, struct tlshd_handshake_parms *parms) { const gnutls_datum_t *peercerts; - gnutls_certificate_type_t type; unsigned int i, status; - gnutls_datum_t out; int ret; ret = gnutls_certificate_verify_peers3(session, parms->peername, @@ -218,12 +216,6 @@ static int tlshd_client_x509_verify_function(gnutls_session_t session, tlshd_log_gnutls_error(ret); return GNUTLS_E_CERTIFICATE_ERROR; } - - type = gnutls_certificate_type_get(session); - gnutls_certificate_verification_status_print(status, type, &out, 0); - tlshd_log_debug("%s", out.data); - gnutls_free(out.data); - if (status) return GNUTLS_E_CERTIFICATE_ERROR; diff --git a/src/tlshd/server.c b/src/tlshd/server.c index 7f225d6..311cdd1 100644 --- a/src/tlshd/server.c +++ b/src/tlshd/server.c @@ -145,9 +145,7 @@ static int tlshd_server_x509_verify_function(gnutls_session_t session, struct tlshd_handshake_parms *parms) { const gnutls_datum_t *peercerts; - gnutls_certificate_type_t type; unsigned int i, status; - gnutls_datum_t out; int ret; ret = gnutls_certificate_verify_peers3(session, NULL, &status); @@ -161,10 +159,6 @@ static int tlshd_server_x509_verify_function(gnutls_session_t session, tlshd_log_gnutls_error(ret); goto certificate_error; } - type = gnutls_certificate_type_get(session); - gnutls_certificate_verification_status_print(status, type, &out, 0); - tlshd_log_debug("%s", out.data); - gnutls_free(out.data); if (status) goto certificate_error; From a6eec63c0b420465919c595339269831d57eaed1 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Mon, 16 Jun 2025 13:40:26 -0400 Subject: [PATCH 13/32] tlshd: Do not return remote peer IDs for x.509 handshakes These were never adopted by the kernel x.509-based TLS consumers, and are to be replaced with handshake tags. Signed-off-by: Chuck Lever --- src/tlshd/client.c | 31 +------------------------------ src/tlshd/server.c | 33 ++------------------------------- 2 files changed, 3 insertions(+), 61 deletions(-) diff --git a/src/tlshd/client.c b/src/tlshd/client.c index 85699b5..0b80cd0 100644 --- a/src/tlshd/client.c +++ b/src/tlshd/client.c @@ -206,8 +206,7 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session, static int tlshd_client_x509_verify_function(gnutls_session_t session, struct tlshd_handshake_parms *parms) { - const gnutls_datum_t *peercerts; - unsigned int i, status; + unsigned int status; int ret; ret = gnutls_certificate_verify_peers3(session, parms->peername, @@ -223,34 +222,6 @@ static int tlshd_client_x509_verify_function(gnutls_session_t session, * to get picky. Kernel would have to tell us what to look for * via a netlink attribute. */ - peercerts = gnutls_certificate_get_peers(session, - &parms->num_remote_peerids); - if (!peercerts || parms->num_remote_peerids == 0) { - tlshd_log_debug("The peer cert list is empty.\n"); - return GNUTLS_E_CERTIFICATE_ERROR; - } - - tlshd_log_debug("The peer offered %u certificate(s).\n", - parms->num_remote_peerids); - - if (parms->num_remote_peerids > ARRAY_SIZE(parms->remote_peerid)) - parms->num_remote_peerids = ARRAY_SIZE(parms->remote_peerid); - for (i = 0; i < parms->num_remote_peerids; i++) { - gnutls_x509_crt_t cert; - - gnutls_x509_crt_init(&cert); - ret = gnutls_x509_crt_import(cert, &peercerts[i], - GNUTLS_X509_FMT_DER); - if (ret != GNUTLS_E_SUCCESS) { - tlshd_log_gnutls_error(ret); - gnutls_x509_crt_deinit(cert); - return GNUTLS_E_CERTIFICATE_ERROR; - } - parms->remote_peerid[i] = - tlshd_keyring_create_cert(cert, parms->peername); - gnutls_x509_crt_deinit(cert); - } - return GNUTLS_E_SUCCESS; } diff --git a/src/tlshd/server.c b/src/tlshd/server.c index 311cdd1..7ad776c 100644 --- a/src/tlshd/server.c +++ b/src/tlshd/server.c @@ -142,10 +142,9 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session, * verification failed. The server sends an ALERT to the client. */ static int tlshd_server_x509_verify_function(gnutls_session_t session, - struct tlshd_handshake_parms *parms) + __attribute__ ((unused)) struct tlshd_handshake_parms *parms) { - const gnutls_datum_t *peercerts; - unsigned int i, status; + unsigned int status; int ret; ret = gnutls_certificate_verify_peers3(session, NULL, &status); @@ -166,34 +165,6 @@ static int tlshd_server_x509_verify_function(gnutls_session_t session, * to get picky. Kernel would have to tell us what to look for * via a netlink attribute. */ - peercerts = gnutls_certificate_get_peers(session, - &parms->num_remote_peerids); - if (!peercerts || parms->num_remote_peerids == 0) { - tlshd_log_debug("The peer cert list is empty."); - goto certificate_error; - } - - tlshd_log_debug("The peer offered %u certificate(s).", - parms->num_remote_peerids); - - if (parms->num_remote_peerids > ARRAY_SIZE(parms->remote_peerid)) - parms->num_remote_peerids = ARRAY_SIZE(parms->remote_peerid); - for (i = 0; i < parms->num_remote_peerids; i++) { - gnutls_x509_crt_t cert; - - gnutls_x509_crt_init(&cert); - ret = gnutls_x509_crt_import(cert, &peercerts[i], - GNUTLS_X509_FMT_DER); - if (ret != GNUTLS_E_SUCCESS) { - tlshd_log_gnutls_error(ret); - gnutls_x509_crt_deinit(cert); - return GNUTLS_E_CERTIFICATE_ERROR; - } - parms->remote_peerid[i] = - tlshd_keyring_create_cert(cert, parms->peername); - gnutls_x509_crt_deinit(cert); - } - return GNUTLS_E_SUCCESS; certificate_error: From 9e85767b0332baaad8af27aa51ca37cf711e26e6 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 13 Jun 2025 21:05:33 -0400 Subject: [PATCH 14/32] tlshd: Add tlshd_log_completion() Refactor the code that emits the handshake completion notice. Simplify and de-duplicate. One functional change: Emit the completion notice only when debugging is enabled. It's a best practice to avoid giving remote attackers a mechanism to flood the system journal. Signed-off-by: Chuck Lever --- src/tlshd/handshake.c | 8 +------- src/tlshd/log.c | 43 +++++++++++++++++-------------------------- src/tlshd/tlshd.h | 6 +----- 3 files changed, 19 insertions(+), 38 deletions(-) diff --git a/src/tlshd/handshake.c b/src/tlshd/handshake.c index 672b7a6..16da7e6 100644 --- a/src/tlshd/handshake.c +++ b/src/tlshd/handshake.c @@ -181,16 +181,10 @@ void tlshd_service_socket(void) out: tlshd_genl_done(&parms); + tlshd_log_completion(&parms); if (parms.keyring) keyctl_unlink(parms.keyring, KEY_SPEC_SESSION_KEYRING); free(parms.peerids); - - if (parms.session_status) { - tlshd_log_failure(parms.peername, parms.peeraddr, - parms.peeraddr_len); - return; - } - tlshd_log_success(parms.peername, parms.peeraddr, parms.peeraddr_len); } diff --git a/src/tlshd/log.c b/src/tlshd/log.c index 3594d52..89337cb 100644 --- a/src/tlshd/log.c +++ b/src/tlshd/log.c @@ -45,40 +45,31 @@ int tlshd_tls_debug; int tlshd_stderr; /** - * tlshd_log_success - Emit "handshake successful" notification - * @hostname: peer's DNS name - * @sap: peer's IP address - * @salen: length of IP address + * tlshd_log_completion - Emit completion notification + * @parms: handshake parameters * */ -void tlshd_log_success(const char *hostname, const struct sockaddr *sap, - socklen_t salen) +void tlshd_log_completion(struct tlshd_handshake_parms *parms) { - char buf[NI_MAXHOST]; + const char *status = "succeeded"; + int priority = LOG_INFO; - getnameinfo(sap, salen, buf, sizeof(buf), NULL, 0, NI_NUMERICHOST); - syslog(LOG_INFO, "Handshake with %s (%s) was successful\n", - hostname, buf); -} + if (!tlshd_debug) + return; -/** - * tlshd_log_failure - Emit "handshake failed" notification - * @hostname: peer's DNS name - * @sap: peer's IP address - * @salen: length of IP address - * - */ -void tlshd_log_failure(const char *hostname, const struct sockaddr *sap, - socklen_t salen) -{ - if (salen) { + if (parms->session_status) { + status = "failed"; + priority = LOG_ERR; + } + if (parms->peeraddr_len) { char buf[NI_MAXHOST]; - getnameinfo(sap, salen, buf, sizeof(buf), NULL, 0, NI_NUMERICHOST); - syslog(LOG_ERR, "Handshake with '%s' (%s) failed\n", - hostname, buf); + getnameinfo(parms->peeraddr, parms->peeraddr_len, buf, + sizeof(buf), NULL, 0, NI_NUMERICHOST); + syslog(priority, "Handshake with '%s' (%s) %s\n", + parms->peername, buf, status); } else - syslog(LOG_ERR, "Handshake request failed\n"); + syslog(priority, "Handshake request %s\n", status); } /** diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h index 135e1e0..cbf1903 100644 --- a/src/tlshd/tlshd.h +++ b/src/tlshd/tlshd.h @@ -96,11 +96,7 @@ extern void tlshd_log_init(const char *progname); extern void tlshd_log_shutdown(void); extern void tlshd_log_close(void); -extern void tlshd_log_success(const char *hostname, - const struct sockaddr *sap, socklen_t salen); -extern void tlshd_log_failure(const char *hostname, - const struct sockaddr *sap, socklen_t salen); - +extern void tlshd_log_completion(struct tlshd_handshake_parms *parms); extern void tlshd_log_debug(const char *fmt, ...); extern void tlshd_log_notice(const char *fmt, ...); extern void tlshd_log_error(const char *fmt, ...); From 5c5b73076db06fe873c2f8f6d4fc2cb979fe3338 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 21 Jun 2025 12:15:47 -0400 Subject: [PATCH 15/32] tlshd: Restore GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR Client handshakes return CERTIFICATE_VERIFICATION_ERROR. Fixes: 731d1b2bf8d6 ("tlshd: Handshake needs to check for CERTIFICATE_ERROR") Signed-off-by: Chuck Lever --- src/tlshd/handshake.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tlshd/handshake.c b/src/tlshd/handshake.c index 16da7e6..1581869 100644 --- a/src/tlshd/handshake.c +++ b/src/tlshd/handshake.c @@ -94,6 +94,7 @@ void tlshd_start_tls_handshake(gnutls_session_t session, parms->session_status = EACCES; switch (ret) { case GNUTLS_E_CERTIFICATE_ERROR: + case GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR: tlshd_log_cert_verification_error(session); break; case -ETIMEDOUT: From ccd22a5877b915dbffc08b56375d3495d3a34eb2 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 27 Jun 2025 10:10:00 -0400 Subject: [PATCH 16/32] tlshd: Preserve pcache during tlshd_gnutls_priority_init() Valgrind reports: ==110975== Invalid read of size 4 ==110975== at 0x405B01: tlshd_gnutls_priority_init (ktls.c:422) ==110975== by 0x4039D1: main (main.c:109) ==110975== Address 0x534b1ec is 1,548 bytes inside a block of size 8,304 free'd ==110975== at 0x4847B4C: free (vg_replace_malloc.c:989) ==110975== by 0x405A02: tlshd_gnutls_priority_init (ktls.c:373) ==110975== by 0x4039D1: main (main.c:109) ==110975== Block was alloc'd at ==110975== at 0x484C214: calloc (vg_replace_malloc.c:1675) ==110975== by 0x48CBD1F: gnutls_priority_init (in /usr/lib64/libgnutls.so.30.37.1) ==110975== by 0x4059DE: tlshd_gnutls_priority_init (ktls.c:366) ==110975== by 0x4039D1: main (main.c:109) The cipher list is allocated as part of the pcache. But tlshd_gnutls_priority_init() releases the pcache immediately after gnutls_priority_cipher_list() returns, then continues to access the returned cipher list, which is now free memory. Refactor tlshd_gnutls_priority_init() so that the lifetime of the pcache is extended until tlshd is done with the cipher list array. Refresh of fix-certificate-verification-error --- src/tlshd/ktls.c | 71 ++++++++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/src/tlshd/ktls.c b/src/tlshd/ktls.c index 1615a40..40a26a4 100644 --- a/src/tlshd/ktls.c +++ b/src/tlshd/ktls.c @@ -350,32 +350,13 @@ static gnutls_priority_t tlshd_gnutls_priority_psk; static gnutls_priority_t tlshd_gnutls_priority_psk_sha256; static gnutls_priority_t tlshd_gnutls_priority_psk_sha384; -/** - * tlshd_gnutls_priority_init - Initialize GnuTLS priority caches - * - */ -int tlshd_gnutls_priority_init(void) +static int tlshd_gnutls_priority_init_list(const unsigned int *ciphers, + int cipher_count) { - const unsigned int *ciphers; - gnutls_priority_t pcache; - const char *errpos; char *pstring, *pstring_sha256, *pstring_sha384; + const char *errpos; int ret, i; - /* Retrieve the system default priority settings */ - ret = gnutls_priority_init(&pcache, NULL, &errpos); - if (ret != GNUTLS_E_SUCCESS) { - tlshd_log_gnutls_error(ret); - return -EIO; - } - - ret = gnutls_priority_cipher_list(pcache, &ciphers); - gnutls_priority_deinit(pcache); - if (ret < 0) { - tlshd_log_gnutls_error(ret); - return -EIO; - } - pstring = strdup("SECURE256:+SECURE128:-COMP-ALL"); if (!pstring) return -ENOMEM; @@ -407,7 +388,7 @@ int tlshd_gnutls_priority_init(void) return -ENOMEM; } - for (i = 0; i < ret; ++i) { + for (i = 0; i < cipher_count; ++i) { bool skip_sha256 = false; bool skip_sha384 = false; @@ -444,7 +425,6 @@ int tlshd_gnutls_priority_init(void) } tlshd_log_debug("x.509 priority string: %s\n", pstring); - ret = gnutls_priority_init(&tlshd_gnutls_priority_x509, pstring, &errpos); if (ret != GNUTLS_E_SUCCESS) { free(pstring_sha256); @@ -453,7 +433,6 @@ int tlshd_gnutls_priority_init(void) tlshd_log_gnutls_error(ret); return -EIO; } - pstring = tlshd_string_concat(pstring, ":+PSK:+DHE-PSK:+ECDHE-PSK"); if (!pstring) { free(pstring_sha256); @@ -463,7 +442,6 @@ int tlshd_gnutls_priority_init(void) } tlshd_log_debug("PSK priority string: %s\n", pstring); - ret = gnutls_priority_init(&tlshd_gnutls_priority_psk, pstring, &errpos); if (ret != GNUTLS_E_SUCCESS) { free(pstring_sha256); @@ -473,9 +451,7 @@ int tlshd_gnutls_priority_init(void) tlshd_log_gnutls_error(ret); return -EIO; } - free(pstring); - pstring = tlshd_string_concat(pstring_sha256, ":+DHE-PSK:+ECDHE-PSK"); if (!pstring) { free(pstring_sha384); @@ -485,7 +461,6 @@ int tlshd_gnutls_priority_init(void) } tlshd_log_debug("PSK SHA256 priority string: %s\n", pstring); - ret = gnutls_priority_init(&tlshd_gnutls_priority_psk_sha256, pstring, &errpos); if (ret != GNUTLS_E_SUCCESS) { @@ -496,9 +471,7 @@ int tlshd_gnutls_priority_init(void) tlshd_log_gnutls_error(ret); return -EIO; } - free(pstring); - pstring = tlshd_string_concat(pstring_sha384, ":+DHE-PSK:+ECDHE-PSK"); if (!pstring) { gnutls_priority_deinit(tlshd_gnutls_priority_psk_sha256); @@ -509,7 +482,6 @@ int tlshd_gnutls_priority_init(void) } tlshd_log_debug("PSK SHA384 priority string: %s\n", pstring); - ret = gnutls_priority_init(&tlshd_gnutls_priority_psk_sha384, pstring, &errpos); if (ret != GNUTLS_E_SUCCESS) { @@ -520,12 +492,45 @@ int tlshd_gnutls_priority_init(void) tlshd_log_gnutls_error(ret); return -EIO; } - free(pstring); return 0; } +/** + * tlshd_gnutls_priority_init - Initialize GnuTLS priority caches + * + * Returns zero on success, or a negative errno value if a failure + * occurred. + */ +int tlshd_gnutls_priority_init(void) +{ + const unsigned int *ciphers; + gnutls_priority_t pcache; + const char *errpos; + int ret; + + /* Retrieve the system default priority settings */ + ret = gnutls_priority_init(&pcache, NULL, &errpos); + if (ret != GNUTLS_E_SUCCESS) { + tlshd_log_gnutls_error(ret); + return -EIO; + } + + ret = gnutls_priority_cipher_list(pcache, &ciphers); + if (ret < 0) { + tlshd_log_gnutls_error(ret); + ret = -EIO; + goto out; + } + + ret = tlshd_gnutls_priority_init_list(ciphers, ret); + +out: + gnutls_priority_deinit(pcache); + return ret; +} + /** * tlshd_gnutls_priority_set - Initialize priorities per-session * @session: session to initialize From 9844627a750b19f6b89891a39dd69b03e1dd172f Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 27 Jun 2025 11:10:54 -0400 Subject: [PATCH 17/32] tlshd: Release server certs and priv key Clean up valgrind output. Signed-off-by: Chuck Lever --- src/tlshd/server.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/src/tlshd/server.c b/src/tlshd/server.c index 7ad776c..fb6dd52 100644 --- a/src/tlshd/server.c +++ b/src/tlshd/server.c @@ -46,9 +46,6 @@ static gnutls_privkey_t tlshd_server_privkey; static unsigned int tlshd_server_certs_len = TLSHD_MAX_CERTS; static gnutls_pcert_st tlshd_server_certs[TLSHD_MAX_CERTS]; -/* - * XXX: After this point, tlshd_server_certs should be deinited on error. - */ static bool tlshd_x509_server_get_certs(struct tlshd_handshake_parms *parms) { if (parms->x509_cert != TLS_NO_CERT) @@ -59,9 +56,14 @@ static bool tlshd_x509_server_get_certs(struct tlshd_handshake_parms *parms) &tlshd_server_certs_len); } -/* - * XXX: After this point, tlshd_server_privkey should be deinited on error. - */ +static void tlshd_x509_server_put_certs(void) +{ + unsigned int i; + + for (i = 0; i < tlshd_server_certs_len; i++) + gnutls_pcert_deinit(&tlshd_server_certs[i]); +} + static bool tlshd_x509_server_get_privkey(struct tlshd_handshake_parms *parms) { if (parms->x509_privkey != TLS_NO_PRIVKEY) @@ -70,6 +72,11 @@ static bool tlshd_x509_server_get_privkey(struct tlshd_handshake_parms *parms) return tlshd_config_get_server_privkey(&tlshd_server_privkey); } +static void tlshd_x509_server_put_privkey(void) +{ + gnutls_privkey_deinit(tlshd_server_privkey); +} + static void tlshd_x509_log_issuers(const gnutls_datum_t *req_ca_rdn, int nreqs) { char issuer_dn[256]; @@ -204,19 +211,17 @@ static void tlshd_tls13_server_x509_handshake(struct tlshd_handshake_parms *parm } tlshd_log_debug("System trust: Loaded %d certificate(s).", ret); - if (!tlshd_x509_server_get_certs(parms)) { + if (!tlshd_x509_server_get_certs(parms)) goto out_free_creds; - } - if (!tlshd_x509_server_get_privkey(parms)) { - goto out_free_creds; - } + if (!tlshd_x509_server_get_privkey(parms)) + goto out_free_certs; gnutls_certificate_set_retrieve_function2(xcred, tlshd_x509_retrieve_key_cb); ret = gnutls_init(&session, GNUTLS_SERVER); if (ret != GNUTLS_E_SUCCESS) { tlshd_log_gnutls_error(ret); - goto out_free_creds; + goto out_free_certs; } gnutls_transport_set_int(session, parms->sockfd); gnutls_session_set_ptr(session, parms); @@ -224,7 +229,7 @@ static void tlshd_tls13_server_x509_handshake(struct tlshd_handshake_parms *parm ret = gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, xcred); if (ret != GNUTLS_E_SUCCESS) { tlshd_log_gnutls_error(ret); - goto out_free_creds; + goto out_free_certs; } gnutls_certificate_set_verify_function(xcred, tlshd_tls13_server_x509_verify_function); @@ -233,7 +238,7 @@ static void tlshd_tls13_server_x509_handshake(struct tlshd_handshake_parms *parm ret = tlshd_gnutls_priority_set(session, parms, 0); if (ret) { tlshd_log_gnutls_error(ret); - goto out_free_creds; + goto out_free_certs; } tlshd_start_tls_handshake(session, parms); @@ -262,6 +267,10 @@ static void tlshd_tls13_server_x509_handshake(struct tlshd_handshake_parms *parm gnutls_deinit(session); +out_free_certs: + tlshd_x509_server_put_privkey(); + tlshd_x509_server_put_certs(); + out_free_creds: gnutls_certificate_free_credentials(xcred); } @@ -515,6 +524,8 @@ static int tlshd_quic_server_set_x509_session(struct tlshd_quic_conn *conn) err_cred: gnutls_certificate_free_credentials(cred); err: + tlshd_x509_server_put_privkey(); + tlshd_x509_server_put_certs(); tlshd_log_gnutls_error(ret); return ret; } From 93ed4658e0475cec9fcb9b5489a659bea3a1bc96 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 28 Jun 2025 12:59:41 -0400 Subject: [PATCH 18/32] tlshd: Release client cert and priv key Reduce false leak reports in valgrind output. Signed-off-by: Chuck Lever --- src/tlshd/client.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/tlshd/client.c b/src/tlshd/client.c index 0b80cd0..26f0885 100644 --- a/src/tlshd/client.c +++ b/src/tlshd/client.c @@ -116,9 +116,6 @@ static gnutls_privkey_t tlshd_privkey; static unsigned int tlshd_certs_len = TLSHD_MAX_CERTS; static gnutls_pcert_st tlshd_certs[TLSHD_MAX_CERTS]; -/* - * XXX: After this point, tlshd_certs should be deinited on error. - */ static bool tlshd_x509_client_get_certs(struct tlshd_handshake_parms *parms) { if (parms->x509_cert != TLS_NO_CERT) @@ -127,9 +124,14 @@ static bool tlshd_x509_client_get_certs(struct tlshd_handshake_parms *parms) return tlshd_config_get_client_certs(tlshd_certs, &tlshd_certs_len); } -/* - * XXX: After this point, tlshd_privkey should be deinited on error. - */ +static void tlshd_x509_client_put_certs(void) +{ + unsigned int i; + + for (i = 0; i < tlshd_certs_len; i++) + gnutls_pcert_deinit(&tlshd_certs[i]); +} + static bool tlshd_x509_client_get_privkey(struct tlshd_handshake_parms *parms) { if (parms->x509_privkey != TLS_NO_PRIVKEY) @@ -138,6 +140,11 @@ static bool tlshd_x509_client_get_privkey(struct tlshd_handshake_parms *parms) return tlshd_config_get_client_privkey(&tlshd_privkey); } +static void tlshd_x509_client_put_privkey(void) +{ + gnutls_privkey_deinit(tlshd_privkey); +} + static void tlshd_x509_log_issuers(const gnutls_datum_t *req_ca_rdn, int nreqs) { char issuer_dn[256]; @@ -261,7 +268,7 @@ static void tlshd_tls13_client_x509_handshake(struct tlshd_handshake_parms *parm if (!tlshd_x509_client_get_certs(parms)) goto out_free_creds; if (!tlshd_x509_client_get_privkey(parms)) - goto out_free_creds; + goto out_free_certs; gnutls_certificate_set_retrieve_function2(xcred, tlshd_x509_retrieve_key_cb); @@ -269,7 +276,7 @@ static void tlshd_tls13_client_x509_handshake(struct tlshd_handshake_parms *parm ret = gnutls_init(&session, flags); if (ret != GNUTLS_E_SUCCESS) { tlshd_log_gnutls_error(ret); - goto out_free_creds; + goto out_free_certs; } gnutls_transport_set_int(session, parms->sockfd); gnutls_session_set_ptr(session, parms); @@ -278,17 +285,17 @@ static void tlshd_tls13_client_x509_handshake(struct tlshd_handshake_parms *parm parms->peername, strlen(parms->peername)); if (ret != GNUTLS_E_SUCCESS) { tlshd_log_gnutls_error(ret); - goto out_free_creds; + goto out_free_certs; } ret = gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, xcred); if (ret != GNUTLS_E_SUCCESS) { tlshd_log_gnutls_error(ret); - goto out_free_creds; + goto out_free_certs; } ret = tlshd_gnutls_priority_set(session, parms, 0); if (ret != GNUTLS_E_SUCCESS) { tlshd_log_gnutls_error(ret); - goto out_free_creds; + goto out_free_certs; } gnutls_certificate_set_verify_function(xcred, tlshd_tls13_client_x509_verify_function); @@ -297,6 +304,10 @@ static void tlshd_tls13_client_x509_handshake(struct tlshd_handshake_parms *parm gnutls_deinit(session); +out_free_certs: + tlshd_x509_client_put_privkey(); + tlshd_x509_client_put_certs(); + out_free_creds: gnutls_certificate_free_credentials(xcred); } @@ -509,6 +520,8 @@ static int tlshd_quic_client_set_x509_session(struct tlshd_quic_conn *conn) err_cred: gnutls_certificate_free_credentials(cred); err: + tlshd_x509_client_put_privkey(); + tlshd_x509_client_put_certs(); tlshd_log_gnutls_error(ret); return ret; } From b910855f37eb128c425f8eaff58f828b11f058b3 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 27 Jun 2025 11:28:05 -0400 Subject: [PATCH 19/32] tlshd: Free netlink messages after fork(3) returns Since the child doesn't return NL_SKIP, the incoming message is never freed. Signed-off-by: Chuck Lever --- src/tlshd/netlink.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index ff9e35a..58e7311 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -125,6 +125,7 @@ static int tlshd_genl_event_handler(struct nl_msg *msg, if (!fork()) { /* child */ + nlmsg_free(msg); tlshd_service_socket(); exit(EXIT_SUCCESS); } From c3f3e2db974724371638ad4c190cf74020e8642e Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 28 Jun 2025 11:38:42 -0400 Subject: [PATCH 20/32] tlshd: Child process should shut down before exiting These resources aren't really leaked because the child process exits immediately, but they do show up on valgrind. Release them before exiting so the real leaks are more obvious. Signed-off-by: Chuck Lever --- src/tlshd/netlink.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index 58e7311..d41e5d4 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -126,7 +126,11 @@ static int tlshd_genl_event_handler(struct nl_msg *msg, if (!fork()) { /* child */ nlmsg_free(msg); + tlshd_service_socket(); + + tlshd_gnutls_priority_deinit(); + tlshd_config_shutdown(); exit(EXIT_SUCCESS); } From 1a95de58b74232cbf951c16d7c9917f889bcc025 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 28 Jun 2025 11:35:20 -0400 Subject: [PATCH 21/32] tlshd: Child should close the notification socket Since the spawned processes do not handle netlink notifications, they should not leave the notification socket open. Signed-off-by: Chuck Lever --- src/tlshd/netlink.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index d41e5d4..80eddcc 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -104,6 +104,8 @@ tlshd_accept_nl_policy[HANDSHAKE_A_ACCEPT_MAX + 1] = { [HANDSHAKE_A_ACCEPT_KEYRING] = { .type = NLA_U32, }, }; +static struct nl_sock *tlshd_notification_nls; + static int tlshd_genl_event_handler(struct nl_msg *msg, __attribute__ ((unused)) void *arg) { @@ -126,6 +128,7 @@ static int tlshd_genl_event_handler(struct nl_msg *msg, if (!fork()) { /* child */ nlmsg_free(msg); + tlshd_genl_sock_close(tlshd_notification_nls); tlshd_service_socket(); @@ -143,24 +146,23 @@ static int tlshd_genl_event_handler(struct nl_msg *msg, */ void tlshd_genl_dispatch(void) { - struct nl_sock *nls; int err, mcgrp; - err = tlshd_genl_sock_open(&nls); + err = tlshd_genl_sock_open(&tlshd_notification_nls); if (err) return; - nl_socket_modify_cb(nls, NL_CB_VALID, NL_CB_CUSTOM, + nl_socket_modify_cb(tlshd_notification_nls, NL_CB_VALID, NL_CB_CUSTOM, tlshd_genl_event_handler, NULL); /* Let the kernel know we are running */ - mcgrp = genl_ctrl_resolve_grp(nls, HANDSHAKE_FAMILY_NAME, + mcgrp = genl_ctrl_resolve_grp(tlshd_notification_nls, HANDSHAKE_FAMILY_NAME, HANDSHAKE_MCGRP_TLSHD); if (mcgrp < 0) { tlshd_log_error("Kernel handshake service is not available"); goto out_close; } - err = nl_socket_add_membership(nls, mcgrp); + err = nl_socket_add_membership(tlshd_notification_nls, mcgrp); if (err != NLE_SUCCESS) { tlshd_log_nl_error("nl_socket_add_membership", err); goto out_close; @@ -171,9 +173,9 @@ void tlshd_genl_dispatch(void) goto out_close; } - nl_socket_disable_seq_check(nls); + nl_socket_disable_seq_check(tlshd_notification_nls); while (true) { - err = nl_recvmsgs_default(nls); + err = nl_recvmsgs_default(tlshd_notification_nls); if (err < 0) { tlshd_log_nl_error("nl_recvmsgs", err); break; @@ -181,7 +183,7 @@ void tlshd_genl_dispatch(void) }; out_close: - tlshd_genl_sock_close(nls); + tlshd_genl_sock_close(tlshd_notification_nls); } static void tlshd_parse_peer_identity(struct tlshd_handshake_parms *parms, From 65dffd59609479d21324bb5ee7623fc69ff7f49f Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 28 Jun 2025 12:58:25 -0400 Subject: [PATCH 22/32] tlshd: Add a SIGINT handler Clean up dynamically allocated resources after a shutdown signal to reduce the number of false valgrind leak reports. Signed-off-by: Chuck Lever --- src/tlshd/main.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/tlshd/main.c b/src/tlshd/main.c index 649fe3f..f0f2aa7 100644 --- a/src/tlshd/main.c +++ b/src/tlshd/main.c @@ -61,6 +61,17 @@ static void usage(char *progname) fprintf(stderr, "usage: %s [-chsv]\n", progname); } +static void tlshd_sigint(int signum) +{ + if (signum == SIGINT) { + tlshd_gnutls_priority_deinit(); + tlshd_config_shutdown(); + tlshd_log_shutdown(); + tlshd_log_close(); + exit(EXIT_SUCCESS); + } +} + int main(int argc, char **argv) { static gchar config_file[PATH_MAX + 1] = "/etc/tlshd.conf"; @@ -113,6 +124,8 @@ int main(int argc, char **argv) return EXIT_FAILURE; } + signal(SIGINT, tlshd_sigint); + tlshd_genl_dispatch(); tlshd_gnutls_priority_deinit(); From 2fbaf345fa969e05596ae61eecffca2c822850f6 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Wed, 18 Jun 2025 10:57:09 -0400 Subject: [PATCH 23/32] tlshd: Refactor trust store management Prepare for implementing support for certificate revocation lists by de-duplicating the code that reads in the trust store. Suggested-by: Long Xin Signed-off-by: Chuck Lever --- src/tlshd/client.c | 56 +++++++++++++++++++++------------------------- src/tlshd/server.c | 44 +++++++++++++++++++----------------- 2 files changed, 49 insertions(+), 51 deletions(-) diff --git a/src/tlshd/client.c b/src/tlshd/client.c index 26f0885..fe688d4 100644 --- a/src/tlshd/client.c +++ b/src/tlshd/client.c @@ -43,12 +43,31 @@ #include "tlshd.h" #include "netlink.h" +static int tlshd_client_get_truststore(gnutls_certificate_credentials_t cred) +{ + char *pathname; + int ret; + + if (tlshd_config_get_client_truststore(&pathname)) { + ret = gnutls_certificate_set_x509_trust_file(cred, pathname, + GNUTLS_X509_FMT_PEM); + free(pathname); + } else + ret = gnutls_certificate_set_x509_system_trust(cred); + if (ret < 0) { + tlshd_log_gnutls_error(ret); + return ret; + } + tlshd_log_debug("System trust: Loaded %d certificate(s).", ret); + + return GNUTLS_E_SUCCESS; +} + static void tlshd_tls13_client_anon_handshake(struct tlshd_handshake_parms *parms) { gnutls_certificate_credentials_t xcred; gnutls_session_t session; unsigned int flags; - char *cafile; int ret; ret = gnutls_certificate_allocate_credentials(&xcred); @@ -65,17 +84,9 @@ static void tlshd_tls13_client_anon_handshake(struct tlshd_handshake_parms *parm gnutls_certificate_set_flags(xcred, GNUTLS_CERTIFICATE_SKIP_KEY_CERT_MATCH | GNUTLS_CERTIFICATE_SKIP_OCSP_RESPONSE_CHECK); - if (tlshd_config_get_client_truststore(&cafile)) { - ret = gnutls_certificate_set_x509_trust_file(xcred, cafile, - GNUTLS_X509_FMT_PEM); - free(cafile); - } else - ret = gnutls_certificate_set_x509_system_trust(xcred); - if (ret < 0) { - tlshd_log_gnutls_error(ret); + ret = tlshd_client_get_truststore(xcred); + if (ret != GNUTLS_E_SUCCESS) goto out_free_creds; - } - tlshd_log_debug("System trust: Loaded %d certificate(s).", ret); flags = GNUTLS_CLIENT; ret = gnutls_init(&session, flags); @@ -244,7 +255,6 @@ static void tlshd_tls13_client_x509_handshake(struct tlshd_handshake_parms *parm gnutls_certificate_credentials_t xcred; gnutls_session_t session; unsigned int flags; - char *cafile; int ret; ret = gnutls_certificate_allocate_credentials(&xcred); @@ -253,17 +263,9 @@ static void tlshd_tls13_client_x509_handshake(struct tlshd_handshake_parms *parm return; } - if (tlshd_config_get_client_truststore(&cafile)) { - ret = gnutls_certificate_set_x509_trust_file(xcred, cafile, - GNUTLS_X509_FMT_PEM); - free(cafile); - } else - ret = gnutls_certificate_set_x509_system_trust(xcred); - if (ret < 0) { - tlshd_log_gnutls_error(ret); + ret = tlshd_client_get_truststore(xcred); + if (ret != GNUTLS_E_SUCCESS) goto out_free_creds; - } - tlshd_log_debug("System trust: Loaded %d certificate(s).", ret); if (!tlshd_x509_client_get_certs(parms)) goto out_free_creds; @@ -463,7 +465,6 @@ static int tlshd_quic_client_set_x509_session(struct tlshd_quic_conn *conn) gnutls_certificate_credentials_t cred; gnutls_session_t session; int ret = -EINVAL; - char *cafile; if (conn->cert_req != TLSHD_QUIC_NO_CERT_AUTH) { if (!tlshd_x509_client_get_certs(parms) || !tlshd_x509_client_get_privkey(parms)) { @@ -474,14 +475,9 @@ static int tlshd_quic_client_set_x509_session(struct tlshd_quic_conn *conn) ret = gnutls_certificate_allocate_credentials(&cred); if (ret) goto err; - if (tlshd_config_get_client_truststore(&cafile)) { - ret = gnutls_certificate_set_x509_trust_file(cred, cafile, GNUTLS_X509_FMT_PEM); - free(cafile); - } else - ret = gnutls_certificate_set_x509_system_trust(cred); - if (ret < 0) + ret = tlshd_client_get_truststore(xcred); + if (ret != GNUTLS_E_SUCCESS) goto err_cred; - tlshd_log_debug("System trust: Loaded %d certificate(s).", ret); if (conn->cert_req == TLSHD_QUIC_NO_CERT_AUTH) { gnutls_certificate_set_verify_flags(cred, GNUTLS_VERIFY_ALLOW_SIGN_RSA_MD2 | diff --git a/src/tlshd/server.c b/src/tlshd/server.c index fb6dd52..8b9357e 100644 --- a/src/tlshd/server.c +++ b/src/tlshd/server.c @@ -133,6 +133,24 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session, return 0; } +static int tlshd_server_get_truststore(gnutls_certificate_credentials_t cred) +{ + char *pathname; + int ret; + + if (tlshd_config_get_server_truststore(&pathname)) { + ret = gnutls_certificate_set_x509_trust_file(cred, pathname, + GNUTLS_X509_FMT_PEM); + free(pathname); + } else + ret = gnutls_certificate_set_x509_system_trust(cred); + if (ret < 0) + return ret; + tlshd_log_debug("System trust: Loaded %d certificate(s).", ret); + + return GNUTLS_E_SUCCESS; +} + /** * tlshd_server_x509_verify_function - Verify remote's x.509 certificate * @session: session in the midst of a handshake @@ -190,7 +208,6 @@ static void tlshd_tls13_server_x509_handshake(struct tlshd_handshake_parms *parm { gnutls_certificate_credentials_t xcred; gnutls_session_t session; - char *cafile; int ret; ret = gnutls_certificate_allocate_credentials(&xcred); @@ -198,18 +215,9 @@ static void tlshd_tls13_server_x509_handshake(struct tlshd_handshake_parms *parm tlshd_log_gnutls_error(ret); return; } - - if (tlshd_config_get_server_truststore(&cafile)) { - ret = gnutls_certificate_set_x509_trust_file(xcred, cafile, - GNUTLS_X509_FMT_PEM); - free(cafile); - } else - ret = gnutls_certificate_set_x509_system_trust(xcred); - if (ret < 0) { - tlshd_log_gnutls_error(ret); + ret = tlshd_server_get_truststore(xcred); + if (ret != GNUTLS_E_SUCCESS) goto out_free_creds; - } - tlshd_log_debug("System trust: Loaded %d certificate(s).", ret); if (!tlshd_x509_server_get_certs(parms)) goto out_free_creds; @@ -471,15 +479,9 @@ static int tlshd_quic_server_set_x509_session(struct tlshd_quic_conn *conn) ret = gnutls_certificate_allocate_credentials(&cred); if (ret) goto err; - if (tlshd_config_get_server_truststore(&cafile)) { - ret = gnutls_certificate_set_x509_trust_file(cred, cafile, - GNUTLS_X509_FMT_PEM); - free(cafile); - } else - ret = gnutls_certificate_set_x509_system_trust(cred); - if (ret < 0) - goto err_cred; - tlshd_log_debug("System trust: Loaded %d certificate(s).", ret); + ret = tlshd_server_get_truststore(cred); + if (ret) + goto err; gnutls_certificate_set_retrieve_function2(cred, tlshd_x509_retrieve_key_cb); From cb8470df5d1fca960e4d452182237e763e019ea8 Mon Sep 17 00:00:00 2001 From: Rik Theys Date: Wed, 18 Jun 2025 11:00:36 +0200 Subject: [PATCH 24/32] tlshd: Add server-side CRL checking Enable server administrators to configure a certificate revocation list. This follows good security practice: The trust store acts as an "allow" list and the CRL acts as a "deny" list. Signed-off-by: Rik Theys Signed-off-by: Chuck Lever --- src/tlshd/config.c | 31 +++++++++++++++++++++++++++++++ src/tlshd/server.c | 11 +++++++++++ src/tlshd/tlshd.h | 1 + 3 files changed, 43 insertions(+) diff --git a/src/tlshd/config.c b/src/tlshd/config.c index df26663..6a5321a 100644 --- a/src/tlshd/config.c +++ b/src/tlshd/config.c @@ -339,6 +339,37 @@ bool tlshd_config_get_server_truststore(char **bundle) return true; } +/** + * tlshd_config_get_server_crl - Get CRL for ServerHello from .conf + * @result: OUT: pathname to CRL + * + * Return values: + * %false: pathname not retrieved + * %true: pathname retrieved successfully; caller must free @result using free(3) + */ +bool tlshd_config_get_server_crl(char **result) +{ + gchar *pathname; + + pathname = g_key_file_get_string(tlshd_configuration, "authenticate.server", + "x509.crl", NULL); + if (!pathname) + return false; + if (access(pathname, F_OK)) { + tlshd_log_debug("tlshd cannot access \"%s\"", pathname); + g_free(pathname); + return false; + } + + *result = strdup(pathname); + g_free(pathname); + if (!*result) + return false; + + tlshd_log_debug("Server x.509 crl is %s", *result); + return true; +} + /** * tlshd_config_get_server_certs - Get certs for ServerHello from .conf * @certs: OUT: in-memory certificates diff --git a/src/tlshd/server.c b/src/tlshd/server.c index 8b9357e..174db60 100644 --- a/src/tlshd/server.c +++ b/src/tlshd/server.c @@ -148,6 +148,17 @@ static int tlshd_server_get_truststore(gnutls_certificate_credentials_t cred) return ret; tlshd_log_debug("System trust: Loaded %d certificate(s).", ret); + if (tlshd_config_get_server_crl(&pathname)) { + ret = gnutls_certificate_set_x509_crl_file(cred, pathname, + GNUTLS_X509_FMT_PEM); + free(pathname); + if (ret < 0 ) { + tlshd_log_gnutls_error(ret); + return ret; + } + tlshd_log_debug("System CRL: Loaded %d CRL(s).", ret); + } + return GNUTLS_E_SUCCESS; } diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h index cbf1903..17e207d 100644 --- a/src/tlshd/tlshd.h +++ b/src/tlshd/tlshd.h @@ -61,6 +61,7 @@ bool tlshd_config_get_client_certs(gnutls_pcert_st *certs, unsigned int *certs_len); bool tlshd_config_get_client_privkey(gnutls_privkey_t *privkey); bool tlshd_config_get_server_truststore(char **bundle); +bool tlshd_config_get_server_crl(char **result); bool tlshd_config_get_server_certs(gnutls_pcert_st *certs, unsigned int *certs_len); bool tlshd_config_get_server_privkey(gnutls_privkey_t *privkey); From 75198609e0398ae7adf7302744d9e00bb02808b6 Mon Sep 17 00:00:00 2001 From: Rik Theys Date: Wed, 18 Jun 2025 11:00:37 +0200 Subject: [PATCH 25/32] Add client-side CRL checking If an x509.crl option is specifiedin the authenticate.client section of the configuration file, use it as a certificate revocation list. This commit only adds the check for tcp based TLS sessions. Support for QUIC still needs to be added. Signed-off-by: Rik Theys Signed-off-by: Chuck Lever --- src/tlshd/client.c | 11 +++++++++++ src/tlshd/config.c | 31 +++++++++++++++++++++++++++++++ src/tlshd/tlshd.h | 1 + 3 files changed, 43 insertions(+) diff --git a/src/tlshd/client.c b/src/tlshd/client.c index fe688d4..c81e26c 100644 --- a/src/tlshd/client.c +++ b/src/tlshd/client.c @@ -60,6 +60,17 @@ static int tlshd_client_get_truststore(gnutls_certificate_credentials_t cred) } tlshd_log_debug("System trust: Loaded %d certificate(s).", ret); + if (tlshd_config_get_client_crl(&pathname)) { + ret = gnutls_certificate_set_x509_crl_file(cred, pathname, + GNUTLS_X509_FMT_PEM); + free(pathname); + if (ret < 0 ) { + tlshd_log_gnutls_error(ret); + return ret; + } + tlshd_log_debug("System CRL: Loaded %d CRL(s).", ret); + } + return GNUTLS_E_SUCCESS; } diff --git a/src/tlshd/config.c b/src/tlshd/config.c index 6a5321a..4c54d37 100644 --- a/src/tlshd/config.c +++ b/src/tlshd/config.c @@ -217,6 +217,37 @@ bool tlshd_config_get_client_truststore(char **bundle) return true; } +/** + * tlshd_config_get_client_crl - Get CRL for ClientHello from .conf + * @result: OUT: pathname to CRL + * + * Return values: + * %false: pathname not retrieved + * %true: pathname retrieved successfully; caller must free @result using free(3) + */ +bool tlshd_config_get_client_crl(char **result) +{ + gchar *pathname; + + pathname = g_key_file_get_string(tlshd_configuration, "authenticate.client", + "x509.crl", NULL); + if (!pathname) + return false; + if (access(pathname, F_OK)) { + tlshd_log_debug("tlshd cannot access \"%s\"", pathname); + g_free(pathname); + return false; + } + + *result = strdup(pathname); + g_free(pathname); + if (!*result) + return false; + + tlshd_log_debug("Client x.509 crl is %s", *result); + return true; +} + /** * tlshd_config_get_client_certs - Get certs for ClientHello from .conf * @certs: OUT: in-memory certificates diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h index 17e207d..246f28a 100644 --- a/src/tlshd/tlshd.h +++ b/src/tlshd/tlshd.h @@ -57,6 +57,7 @@ extern void tlshd_quic_clienthello_handshake(struct tlshd_handshake_parms *parms bool tlshd_config_init(const gchar *pathname); void tlshd_config_shutdown(void); bool tlshd_config_get_client_truststore(char **bundle); +bool tlshd_config_get_client_crl(char **result); bool tlshd_config_get_client_certs(gnutls_pcert_st *certs, unsigned int *certs_len); bool tlshd_config_get_client_privkey(gnutls_privkey_t *privkey); From d0ffb5fe36ffb7ed4630a54fa3fa446327bb8e2c Mon Sep 17 00:00:00 2001 From: Rik Theys Date: Wed, 18 Jun 2025 11:00:38 +0200 Subject: [PATCH 26/32] tlshd: Add x509.crl option to man page. Update the tlshd.conf man page to include the x509.crl option available in the authenticate.server and authenticate.client sections. Signed-off-by: Rik Theys Signed-off-by: Chuck Lever --- src/tlshd/tlshd.conf | 2 ++ src/tlshd/tlshd.conf.man | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/tlshd/tlshd.conf b/src/tlshd/tlshd.conf index 6ac13c8..620bd17 100644 --- a/src/tlshd/tlshd.conf +++ b/src/tlshd/tlshd.conf @@ -30,10 +30,12 @@ nl=0 [authenticate.client] #x509.truststore= +#x509.crl= #x509.certificate= #x509.private_key= [authenticate.server] #x509.truststore= +#x509.crl= #x509.certificate= #x509.private_key= diff --git a/src/tlshd/tlshd.conf.man b/src/tlshd/tlshd.conf.man index abb2f99..914261e 100644 --- a/src/tlshd/tlshd.conf.man +++ b/src/tlshd/tlshd.conf.man @@ -100,7 +100,7 @@ and it consults the settings in the .I [server] subsection when handling the server end of a handshake. .P -In each of these two subsections, there are three available options: +In each of these two subsections, there are four available options: .TP .B x509.truststore This option specifies the pathname of a file containing a @@ -110,6 +110,13 @@ If this option is not specified, .B tlshd uses the system's trust store. .TP +.B x509.crl +This option specifies the pathname of a file containing +PEM-encoded certificate revocation lists (CRL) that are to be +used to verify the revocation status of certificates during +each handshake. +If this option is not specified, CRL checking is skipped. +.TP .B x509.certificate This option specifies the pathname of a file containing a PEM-encoded x.509 certificate that is to be presented during From b967c3236e5dc9c9b21bdda25d35aa85b121a764 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 13 Jun 2025 16:36:25 -0400 Subject: [PATCH 27/32] tlshd: Add tlshd_genl_put_handshake_parms() API Manage dynamically-allocated resources in struct tlshd_handshake_parms by adding a function to release these resources before the handshake child process exits. This improves the ability of tlshd to dynamically allocate and manage memory containing variably-sized arguments such as hostnames, arrays, and socket addresses. The parms.peername and parms.peeraddr fields will be included in the set of dynamically-allocated resources, so add the new "release" API call /after/ the handshake completion notice. Signed-off-by: Chuck Lever --- src/tlshd/handshake.c | 6 +----- src/tlshd/netlink.c | 14 +++++++++++++- src/tlshd/tlshd.h | 1 + 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/tlshd/handshake.c b/src/tlshd/handshake.c index 1581869..ed5c13b 100644 --- a/src/tlshd/handshake.c +++ b/src/tlshd/handshake.c @@ -183,9 +183,5 @@ void tlshd_service_socket(void) out: tlshd_genl_done(&parms); tlshd_log_completion(&parms); - - if (parms.keyring) - keyctl_unlink(parms.keyring, KEY_SPEC_SESSION_KEYRING); - - free(parms.peerids); + tlshd_genl_put_handshake_parms(&parms); } diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index 80eddcc..1ce0b59 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -405,6 +405,18 @@ int tlshd_genl_get_handshake_parms(struct tlshd_handshake_parms *parms) return ret; } +/** + * tlshd_genl_put_handshake_parms - Release handshake resources + * @parms: handshake parameters to be released + * + */ +void tlshd_genl_put_handshake_parms(struct tlshd_handshake_parms *parms) +{ + if (parms->keyring) + keyctl_unlink(parms->keyring, KEY_SPEC_SESSION_KEYRING); + free(parms->peerids); +} + static int tlshd_genl_put_remote_peerids(struct nl_msg *msg, struct tlshd_handshake_parms *parms) { @@ -423,7 +435,7 @@ static int tlshd_genl_put_remote_peerids(struct nl_msg *msg, } /** - * tlshd_genl_done - Indicate anon handshake has completed successfully + * tlshd_genl_done - Indicate handshake has completed successfully * @parms: buffer filled in with parameters * */ diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h index 246f28a..cacb200 100644 --- a/src/tlshd/tlshd.h +++ b/src/tlshd/tlshd.h @@ -116,6 +116,7 @@ void tlshd_log_nl_error(const char *msg, int err); /* netlink.c */ extern void tlshd_genl_dispatch(void); extern int tlshd_genl_get_handshake_parms(struct tlshd_handshake_parms *parms); +extern void tlshd_genl_put_handshake_parms(struct tlshd_handshake_parms *parms); extern void tlshd_genl_done(struct tlshd_handshake_parms *parms); /* server.c */ From 1e03c0ee9240467dd076a1bad2a5944b606a1c67 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 14 Jun 2025 11:53:33 -0400 Subject: [PATCH 28/32] tlshd: Store remote peerids in a GArray The number of remote peer IDs passed back from the TLS library is not known in advance. Use a flexible array to handle one-at-a-time storage of the peerids as they are retrieved. Signed-off-by: Chuck Lever --- src/tlshd/client.c | 6 ++---- src/tlshd/netlink.c | 15 +++++++++------ src/tlshd/server.c | 6 ++---- src/tlshd/tlshd.h | 4 +--- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/tlshd/client.c b/src/tlshd/client.c index c81e26c..0b32e89 100644 --- a/src/tlshd/client.c +++ b/src/tlshd/client.c @@ -404,11 +404,9 @@ static void tlshd_tls13_client_psk_handshake_one(struct tlshd_handshake_parms *p tlshd_log_debug("start ClientHello handshake"); tlshd_start_tls_handshake(session, parms); - if (!parms->session_status) { + if (!parms->session_status) /* PSK uses the same identity for both client and server */ - parms->num_remote_peerids = 1; - parms->remote_peerid[0] = peerid; - } + g_array_append_val(parms->remote_peerids, peerid); gnutls_deinit(session); diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index 1ce0b59..ee180a4 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -321,10 +321,9 @@ static const struct tlshd_handshake_parms tlshd_default_handshake_parms = { .x509_privkey = TLS_NO_PRIVKEY, .peerids = NULL, .num_peerids = 0, + .remote_peerids = NULL, .msg_status = 0, .session_status = EIO, - - .num_remote_peerids = 0, }; /** @@ -347,6 +346,8 @@ int tlshd_genl_get_handshake_parms(struct tlshd_handshake_parms *parms) *parms = tlshd_default_handshake_parms; + parms->remote_peerids = g_array_new(FALSE, FALSE, sizeof(key_serial_t)); + ret = tlshd_genl_sock_open(&nls); if (ret) return ret; @@ -415,17 +416,19 @@ void tlshd_genl_put_handshake_parms(struct tlshd_handshake_parms *parms) if (parms->keyring) keyctl_unlink(parms->keyring, KEY_SPEC_SESSION_KEYRING); free(parms->peerids); + g_array_free(parms->remote_peerids, TRUE); } static int tlshd_genl_put_remote_peerids(struct nl_msg *msg, struct tlshd_handshake_parms *parms) { - unsigned int i; + key_serial_t peerid; + guint i; int err; - for (i = 0; i < parms->num_remote_peerids; i++) { - err = nla_put_s32(msg, HANDSHAKE_A_DONE_REMOTE_AUTH, - parms->remote_peerid[i]); + for (i = 0; i < parms->remote_peerids->len; i++) { + peerid = g_array_index(parms->remote_peerids, key_serial_t, i); + err = nla_put_s32(msg, HANDSHAKE_A_DONE_REMOTE_AUTH, peerid); if (err < 0) { tlshd_log_nl_error("nla_put peer id", err); return -1; diff --git a/src/tlshd/server.c b/src/tlshd/server.c index 174db60..db5db31 100644 --- a/src/tlshd/server.c +++ b/src/tlshd/server.c @@ -331,8 +331,7 @@ static int tlshd_server_psk_cb(gnutls_session_t session, } /* PSK uses the same identity for both client and server */ - parms->remote_peerid[0] = psk; - parms->num_remote_peerids = 1; + g_array_append_val(parms->remote_peerids, psk); return 0; } @@ -468,8 +467,7 @@ static int tlshd_quic_server_psk_cb(gnutls_session_t session, const char *userna } /* PSK uses the same identity for both client and server */ - parms->remote_peerid[0] = psk; - parms->num_remote_peerids = 1; + g_array_append_val(parms->remote_peerids, psk); return 0; } diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h index cacb200..46abcda 100644 --- a/src/tlshd/tlshd.h +++ b/src/tlshd/tlshd.h @@ -41,12 +41,10 @@ struct tlshd_handshake_parms { key_serial_t x509_privkey; key_serial_t *peerids; unsigned int num_peerids; + GArray *remote_peerids; int msg_status; unsigned int session_status; - - unsigned int num_remote_peerids; - key_serial_t remote_peerid[10]; }; /* client.c */ From 86607af3b464043cf37ec9d1e76a59a49657d517 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 14 Jun 2025 10:29:36 -0400 Subject: [PATCH 29/32] tlshd: Store peer IDs in a GArray The number of peer IDs passed up from the kernel is not known in advance. Use a flexible array to handle one-at-a-time storage of the peerids as they are retrieved from the netlink arguments. Signed-off-by: Chuck Lever --- src/tlshd/client.c | 10 ++++++---- src/tlshd/netlink.c | 17 ++++++----------- src/tlshd/tlshd.h | 3 +-- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/tlshd/client.c b/src/tlshd/client.c index 0b32e89..0e648ed 100644 --- a/src/tlshd/client.c +++ b/src/tlshd/client.c @@ -417,9 +417,10 @@ static void tlshd_tls13_client_psk_handshake_one(struct tlshd_handshake_parms *p static void tlshd_tls13_client_psk_handshake(struct tlshd_handshake_parms *parms) { - unsigned int i; + key_serial_t peerid; + guint i; - if (!parms->peerids) { + if (parms->peerids->len == 0) { tlshd_log_error("No key identities"); return; } @@ -428,8 +429,9 @@ static void tlshd_tls13_client_psk_handshake(struct tlshd_handshake_parms *parms * GnuTLS does not yet support multiple offered PskIdentities. * Retry ClientHello with each identity on the kernel's list. */ - for (i = 0; i < parms->num_peerids; i++) { - tlshd_tls13_client_psk_handshake_one(parms, parms->peerids[i]); + for (i = 0; i < parms->peerids->len; i++) { + peerid = g_array_index(parms->peerids, key_serial_t, i); + tlshd_tls13_client_psk_handshake_one(parms, peerid); if (parms->session_status != EACCES) break; } diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index ee180a4..4e03d6a 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -189,20 +189,15 @@ void tlshd_genl_dispatch(void) static void tlshd_parse_peer_identity(struct tlshd_handshake_parms *parms, struct nlattr *head) { + key_serial_t peerid; + if (!head) { tlshd_log_debug("No peer identities found\n"); return; } - parms->num_peerids = 1; - - parms->peerids = calloc(parms->num_peerids, sizeof(key_serial_t)); - if (!parms->peerids) { - parms->num_peerids = 0; - return; - } - - parms->peerids[0] = nla_get_s32(head); + peerid = nla_get_s32(head); + g_array_append_val(parms->peerids, peerid); } #if LIBNL_VER_NUM >= LIBNL_VER(3,5) @@ -320,7 +315,6 @@ static const struct tlshd_handshake_parms tlshd_default_handshake_parms = { .x509_cert = TLS_NO_CERT, .x509_privkey = TLS_NO_PRIVKEY, .peerids = NULL, - .num_peerids = 0, .remote_peerids = NULL, .msg_status = 0, .session_status = EIO, @@ -346,6 +340,7 @@ int tlshd_genl_get_handshake_parms(struct tlshd_handshake_parms *parms) *parms = tlshd_default_handshake_parms; + parms->peerids = g_array_new(FALSE, FALSE, sizeof(key_serial_t)); parms->remote_peerids = g_array_new(FALSE, FALSE, sizeof(key_serial_t)); ret = tlshd_genl_sock_open(&nls); @@ -415,7 +410,7 @@ void tlshd_genl_put_handshake_parms(struct tlshd_handshake_parms *parms) { if (parms->keyring) keyctl_unlink(parms->keyring, KEY_SPEC_SESSION_KEYRING); - free(parms->peerids); + g_array_free(parms->peerids, TRUE); g_array_free(parms->remote_peerids, TRUE); } diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h index 46abcda..6a623c2 100644 --- a/src/tlshd/tlshd.h +++ b/src/tlshd/tlshd.h @@ -39,8 +39,7 @@ struct tlshd_handshake_parms { key_serial_t keyring; key_serial_t x509_cert; key_serial_t x509_privkey; - key_serial_t *peerids; - unsigned int num_peerids; + GArray *peerids; GArray *remote_peerids; int msg_status; From 484b63c095d3bc7bf5c27e050336ecdc7e8a849f Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Mon, 16 Jun 2025 13:55:05 -0400 Subject: [PATCH 30/32] tlshd: Convert parms->peeraddr to a presentation address There are no consumers that use peeraddr as a sockaddr. Render it as a presentation address as soon as getpeername(3) gets the socket address. Signed-off-by: Chuck Lever --- src/tlshd/log.c | 10 +++------- src/tlshd/netlink.c | 35 +++++++++++++++++++++++++---------- src/tlshd/tlshd.h | 3 +-- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/tlshd/log.c b/src/tlshd/log.c index 89337cb..243bdba 100644 --- a/src/tlshd/log.c +++ b/src/tlshd/log.c @@ -61,14 +61,10 @@ void tlshd_log_completion(struct tlshd_handshake_parms *parms) status = "failed"; priority = LOG_ERR; } - if (parms->peeraddr_len) { - char buf[NI_MAXHOST]; - - getnameinfo(parms->peeraddr, parms->peeraddr_len, buf, - sizeof(buf), NULL, 0, NI_NUMERICHOST); + if (parms->peeraddr) syslog(priority, "Handshake with '%s' (%s) %s\n", - parms->peername, buf, status); - } else + parms->peername, parms->peeraddr, status); + else syslog(priority, "Handshake request %s\n", status); } diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index 4e03d6a..5dfcc57 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -233,14 +233,15 @@ static void tlshd_parse_certificate(struct tlshd_handshake_parms *parms, } static char tlshd_peername[NI_MAXHOST] = "unknown"; -static struct sockaddr_storage tlshd_peeraddr = { 0 }; static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) { struct nlattr *tb[HANDSHAKE_A_ACCEPT_MAX + 1]; struct tlshd_handshake_parms *parms = arg; + struct sockaddr_storage addr; + struct sockaddr *sap = NULL; + socklen_t salen, optlen; char *peername = NULL; - socklen_t optlen; int err; tlshd_log_debug("Parsing a valid netlink message\n"); @@ -253,18 +254,32 @@ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) } if (tb[HANDSHAKE_A_ACCEPT_SOCKFD]) { + char buf[NI_MAXHOST]; + int proto; + parms->sockfd = nla_get_s32(tb[HANDSHAKE_A_ACCEPT_SOCKFD]); - if (getpeername(parms->sockfd, parms->peeraddr, - &parms->peeraddr_len) == -1) { + + salen = sizeof(addr); + sap = (struct sockaddr *)&addr; + if (getpeername(parms->sockfd, sap, &salen) == -1) { tlshd_log_perror("getpeername"); return NL_STOP; } - optlen = sizeof(parms->ip_proto); + err = getnameinfo(sap, salen, buf, sizeof(buf), + NULL, 0, NI_NUMERICHOST); + if (err) { + tlshd_log_gai_error(err); + return NL_STOP; + } + parms->peeraddr = strdup(buf); + + optlen = sizeof(proto); if (getsockopt(parms->sockfd, SOL_SOCKET, SO_PROTOCOL, - &parms->ip_proto, &optlen) == -1) { + &proto, &optlen) == -1) { tlshd_log_perror("getsockopt (SO_PROTOCOL)"); return NL_STOP; } + parms->ip_proto = proto; } if (tb[HANDSHAKE_A_ACCEPT_MESSAGE_TYPE]) parms->handshake_type = nla_get_u32(tb[HANDSHAKE_A_ACCEPT_MESSAGE_TYPE]); @@ -290,8 +305,8 @@ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) if (peername) strncpy(tlshd_peername, peername, sizeof(tlshd_peername) - 1); - else { - err = getnameinfo(parms->peeraddr, parms->peeraddr_len, + else if (sap) { + err = getnameinfo(sap, salen, tlshd_peername, sizeof(tlshd_peername), NULL, 0, NI_NAMEREQD); if (err) { @@ -305,8 +320,7 @@ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) static const struct tlshd_handshake_parms tlshd_default_handshake_parms = { .peername = tlshd_peername, - .peeraddr = (struct sockaddr *)&tlshd_peeraddr, - .peeraddr_len = sizeof(tlshd_peeraddr), + .peeraddr = NULL, .sockfd = -1, .ip_proto = -1, .handshake_type = HANDSHAKE_MSG_TYPE_UNSPEC, @@ -412,6 +426,7 @@ void tlshd_genl_put_handshake_parms(struct tlshd_handshake_parms *parms) keyctl_unlink(parms->keyring, KEY_SPEC_SESSION_KEYRING); g_array_free(parms->peerids, TRUE); g_array_free(parms->remote_peerids, TRUE); + free(parms->peeraddr); } static int tlshd_genl_put_remote_peerids(struct nl_msg *msg, diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h index 6a623c2..a0dd47e 100644 --- a/src/tlshd/tlshd.h +++ b/src/tlshd/tlshd.h @@ -29,8 +29,7 @@ struct nl_sock; struct tlshd_handshake_parms { char *peername; - struct sockaddr *peeraddr; - socklen_t peeraddr_len; + char *peeraddr; int sockfd; int ip_proto; uint32_t handshake_type; From 808a57f8b9c01b393dbc34cd129c46a09edd288a Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Mon, 16 Jun 2025 14:19:59 -0400 Subject: [PATCH 31/32] tlshd: Dynamically allocate hostname Ensure that, when allocation or DNS query fails, parms->peername always remains NULL. tlshd should not assume that the buffer returned from getnameinfo(3) will remain untouched if that call is unsuccessful. Fixes: 7655d96c7ace ("tlshd: Move peername/peeraddr preparation") Reported-by: Ken Milmore Signed-off-by: Chuck Lever --- src/tlshd/log.c | 2 +- src/tlshd/netlink.c | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/tlshd/log.c b/src/tlshd/log.c index 243bdba..849027e 100644 --- a/src/tlshd/log.c +++ b/src/tlshd/log.c @@ -61,7 +61,7 @@ void tlshd_log_completion(struct tlshd_handshake_parms *parms) status = "failed"; priority = LOG_ERR; } - if (parms->peeraddr) + if (parms->peername && parms->peeraddr) syslog(priority, "Handshake with '%s' (%s) %s\n", parms->peername, parms->peeraddr, status); else diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index 5dfcc57..f716232 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -232,8 +232,6 @@ static void tlshd_parse_certificate(struct tlshd_handshake_parms *parms, parms->x509_privkey = nla_get_s32(tb[HANDSHAKE_A_X509_PRIVKEY]); } -static char tlshd_peername[NI_MAXHOST] = "unknown"; - static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) { struct nlattr *tb[HANDSHAKE_A_ACCEPT_MAX + 1]; @@ -304,22 +302,24 @@ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) tlshd_parse_certificate(parms, tb[HANDSHAKE_A_ACCEPT_CERTIFICATE]); if (peername) - strncpy(tlshd_peername, peername, sizeof(tlshd_peername) - 1); + parms->peername = strdup(peername); else if (sap) { - err = getnameinfo(sap, salen, - tlshd_peername, sizeof(tlshd_peername), + char buf[NI_MAXHOST]; + + err = getnameinfo(sap, salen, buf, sizeof(buf), NULL, 0, NI_NAMEREQD); if (err) { tlshd_log_gai_error(err); return NL_STOP; } + parms->peername = strdup(buf); } return NL_SKIP; } static const struct tlshd_handshake_parms tlshd_default_handshake_parms = { - .peername = tlshd_peername, + .peername = NULL, .peeraddr = NULL, .sockfd = -1, .ip_proto = -1, @@ -426,6 +426,7 @@ void tlshd_genl_put_handshake_parms(struct tlshd_handshake_parms *parms) keyctl_unlink(parms->keyring, KEY_SPEC_SESSION_KEYRING); g_array_free(parms->peerids, TRUE); g_array_free(parms->remote_peerids, TRUE); + free(parms->peername); free(parms->peeraddr); } From 964aaf0aaef9ab024ddaa5cdce5241591d03bace Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 8 Jul 2025 13:31:37 -0400 Subject: [PATCH 32/32] Release ktls-utils 1.2.0 Signed-off-by: Chuck Lever --- ChangeLog | 6 ++++++ NEWS | 10 +++++----- README | 2 +- README.md | 2 +- configure.ac | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index a46076e..a5640b5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ Change Log - In newest-release-first order +ktls-utils 1.2.0 2025-07-11 + * Implement Certificate Revocation Lists + * Add a default keyring for NFS consumers + * Improvements to error reporting and logging + * Manage per-session resources more effectively + ktls-utils 1.1.0 2025-06-02 * Return to the old release process * Update the contribution process diff --git a/NEWS b/NEWS index f6e6d92..60bd06e 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,5 @@ -ktls-utils 1.1.0 2025-06-02 - * Return to the old release process - * Update the contribution process - * Accept alternate keyrings during handshake upcall - * Initial support for building ktls-utils with MUSL +ktls-utils 1.2.0 2025-07-11 + * Implement Certificate Revocation Lists + * Add a default keyring for NFS consumers + * Improvements to error reporting and logging + * Manage per-session resources more effectively diff --git a/README b/README index a26e010..a7b731a 100644 --- a/README +++ b/README @@ -1,4 +1,4 @@ -# Release Notes for ktls-utils 1.1.0 +# Release Notes for ktls-utils 1.2.0 In-kernel TLS consumers need a mechanism to perform TLS handshakes on a connected socket to negotiate TLS session parameters that can diff --git a/README.md b/README.md index a26e010..a7b731a 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Release Notes for ktls-utils 1.1.0 +# Release Notes for ktls-utils 1.2.0 In-kernel TLS consumers need a mechanism to perform TLS handshakes on a connected socket to negotiate TLS session parameters that can diff --git a/configure.ac b/configure.ac index 818b30b..a6d9d09 100644 --- a/configure.ac +++ b/configure.ac @@ -18,7 +18,7 @@ dnl 02110-1301, USA. dnl AC_PREREQ([2.69]) -AC_INIT([ktls-utils],[1.1.0],[kernel-tls-handshake@lists.linux.dev]) +AC_INIT([ktls-utils],[1.2.0],[kernel-tls-handshake@lists.linux.dev]) AM_INIT_AUTOMAKE AM_SILENT_RULES([yes]) AC_CONFIG_SRCDIR([config.h.in])