From 5702009c8c2e24005026d098ef261574efed80d0 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 5 Dec 2025 11:16:46 +0100 Subject: [PATCH] BUG/MEDIUM: ssl: Don't reuse TLS session if the connection's SNI differs When a new SSL server connection is created, if no SNI is set, it is possible to inherit from the one of the reused TLS session. The bug was introduced by the commit 95ac5fe4a ("MEDIUM: ssl_sock: always use the SSL's server name, not the one from the tid"). The mixup is possible between regular connections but also with health-checks connections. But it is only the visible part of the bug. If the SNI of the cached TLS session does not match the one of the new connection, no reuse must be performed at all. To fix the bug, hash of the SNI of the reused session is compared with the one of the new connection. The TLS session is reused only if the hashes are the same. This patch should fix the issue #3195. It must be slowly backported as far as 3.0. it relies on the following series: * MEDIUM: tcpcheck/backend: Get the connection SNI before initializing SSL ctx * MINOR: connection/ssl: Store the SNI hash value in the connection itself * MEDIUM: ssl: Store hash of the SNI for cached TLS sessions * MINOR: ssl: Add a function to hash SNIs * MEDIUM: quic: Add connection as argument when qc_new_conn() is called * BUG/MINOR: ssl: Don't allow to set NULL sni --- src/ssl_sock.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 880bb2bd9..7a9a2da24 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -5689,21 +5689,32 @@ int ssl_sock_srv_try_reuse_sess(struct ssl_sock_ctx *ctx, struct server *srv) * (transport parameters and ALPN) are successfully reused. */ int ret = qc && (srv->ssl_ctx.options & SRV_SSL_O_EARLY_DATA) ? 0 : 1; + struct connection *conn = qc ? qc->conn : ctx->conn; #else /* Always succeeds for TCP sockets. */ int ret = 1; + struct connection *conn = ctx->conn; #endif HA_RWLOCK_RDLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.lock); if (srv->ssl_ctx.reused_sess[tid].ptr) { + const unsigned char *ptr; + SSL_SESSION *sess; + + /* No connection or the sni of the cached SSL session does not + * match the one of the new connection, don't reuse the SSL session + */ + if (!conn || srv->ssl_ctx.reused_sess[tid].sni_hash != conn->sni_hash) + goto out; + /* let's recreate a session from (ptr,size) and assign * it to ctx->ssl. Its refcount will be updated by the * creation and by the assignment, so after assigning * it or failing to, we must always free it to decrement * the refcount. */ - const unsigned char *ptr = srv->ssl_ctx.reused_sess[tid].ptr; - SSL_SESSION *sess = d2i_SSL_SESSION(NULL, &ptr, srv->ssl_ctx.reused_sess[tid].size); + ptr = srv->ssl_ctx.reused_sess[tid].ptr; + sess = d2i_SSL_SESSION(NULL, &ptr, srv->ssl_ctx.reused_sess[tid].size); if (sess && !SSL_set_session(ctx->ssl, sess)) { uint old_tid = HA_ATOMIC_LOAD(&srv->ssl_ctx.last_ssl_sess_tid); // 0=none, >0 = tid + 1 @@ -5748,6 +5759,14 @@ int ssl_sock_srv_try_reuse_sess(struct ssl_sock_ctx *ctx, struct server *srv) if (old_tid) { HA_RWLOCK_RDLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[old_tid-1].sess_lock); + /* No connection or the sni of the cached SSL session does not + * match the one of the new connection, don't reuse the SSL session + */ + if (!conn || srv->ssl_ctx.reused_sess[old_tid-1].sni_hash != conn->sni_hash) { + HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[old_tid-1].sess_lock); + goto out; + } + ptr = srv->ssl_ctx.reused_sess[old_tid-1].ptr; if (ptr) { sess = d2i_SSL_SESSION(NULL, &ptr, srv->ssl_ctx.reused_sess[old_tid-1].size); @@ -5774,6 +5793,7 @@ int ssl_sock_srv_try_reuse_sess(struct ssl_sock_ctx *ctx, struct server *srv) HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[old_tid-1].sess_lock); } } + out: HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.lock); return ret;