fixeria has uploaded this change for review.
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");
To view, visit change 42849. To unsubscribe, or for help writing mail filters, visit settings.