fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcap/+/42849?usp=email )
Change subject: tls: fix broken certificate hostname verification ......................................................................
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.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384 --- M include/osmo-pcap/osmo_tls.h M src/osmo_tls.c 2 files changed, 15 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcap refs/changes/49/42849/1
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..9502dbc 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 = 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");
-
fixeria