fixeria submitted this change.

View Change

tls: fix broken certificate hostname verification

verify_cert_cb() retrieved the gnutls session pointer and passed it to
gnutls_certificate_verify_peers3() as the expected hostname.  But the
session pointer is set to the osmo_tls_session struct (it is needed by
cert_callback()), not a hostname string.  Hostname matching was
therefore performed against raw struct bytes, rendering verification
meaningless and potentially reading out of bounds, even when
"tls verify-cert" was enabled.

Store the configured hostname in struct osmo_tls_session and have
verify_cert_cb() read it from there.  Also drop the stray
gnutls_certificate_verify_peers3() call in the client setup: it ran
before any handshake (so there were no peer certificates yet) and its
result was ignored; the real verification happens via the registered
callback during the handshake.

Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
AI-Assisted: yes (Claude)
---
M include/osmo-pcap/osmo_tls.h
M src/osmo_tls.c
2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/osmo-pcap/osmo_tls.h b/include/osmo-pcap/osmo_tls.h
index 716604e..6afbe71 100644
--- a/include/osmo-pcap/osmo_tls.h
+++ b/include/osmo-pcap/osmo_tls.h
@@ -38,6 +38,9 @@
 	bool need_resend;
 	gnutls_session_t session;
 
+	/* expected peer hostname for certificate verification (may be NULL) */
+	const char *tls_hostname;
+
 	/* any credentials */
 	bool anon_alloc;
 	gnutls_anon_client_credentials_t anon_cred;
diff --git a/src/osmo_tls.c b/src/osmo_tls.c
index f06f50e..d3ed45e 100644
--- a/src/osmo_tls.c
+++ b/src/osmo_tls.c
@@ -139,11 +139,15 @@
 
 static int verify_cert_cb(gnutls_session_t session)
 {
+	const struct osmo_tls_session *sess;
 	const char *hostname;
 	unsigned int status;
 	int ret;
 
-	hostname = gnutls_session_get_ptr(session);
+	/* The session ptr is the osmo_tls_session (see gnutls_session_set_ptr());
+	 * the expected hostname is stored inside it. */
+	sess = gnutls_session_get_ptr(session);
+	hostname = sess ? sess->tls_hostname : NULL;
 	ret = gnutls_certificate_verify_peers3(session,
 				hostname, &status);
 	if (ret != 0)
@@ -448,7 +452,6 @@
 {
 	struct osmo_tls_session *sess = &client->tls_session;
 	struct osmo_wqueue *wq = &client->wqueue;
-	unsigned int status;
 	int rc;
 
 	gnutls_global_set_log_level(client->tls_log_level);
@@ -518,14 +521,19 @@
 	gnutls_certificate_set_retrieve_function2(sess->cert_cred, cert_callback);
 
 	/* set the hostname if we have one */
-	if (client->tls_hostname)
+	if (client->tls_hostname) {
 		gnutls_server_name_set(sess->session, GNUTLS_NAME_DNS,
 				client->tls_hostname, strlen(client->tls_hostname));
+		/* Remember it so verify_cert_cb() can match it against the peer
+		 * certificate during the handshake. */
+		sess->tls_hostname = talloc_strdup(client, client->tls_hostname);
+	}
 
 	/* do the verification */
 	if (client->tls_verify) {
+		/* The actual verification happens during the handshake via the
+		 * registered callback; there are no peer certificates yet here. */
 		gnutls_certificate_set_verify_function(sess->cert_cred, verify_cert_cb);
-		gnutls_certificate_verify_peers3(sess->session, client->tls_hostname, &status);
 	} else
 		LOGP(DTLS, LOGL_NOTICE, "Not going to validate certs as configured\n");
 
@@ -545,6 +553,9 @@
 
 	gnutls_deinit(session->session);
 
+	talloc_free((void *)session->tls_hostname);
+	session->tls_hostname = NULL;
+
 	release_keys(session);
 
 	if (session->anon_alloc)

To view, visit change 42849. To unsubscribe, or for help writing mail filters, visit settings.