From be998b590e4bb7c3f9272ca3574561f1e1294265 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 5 Dec 2025 17:35:53 +0100 Subject: [PATCH] MEDIUM: ssl/server: No longer store the SNI of cached TLS sessions Thanks to the previous patch, "BUG/MEDIUM: ssl: Don't reuse TLS session if the connection's SNI differs", it is no useless to store the SNI of cached TLS sessions. This SNI is no longer tested and new connections reusing a session must have the same SNI. The main change here is for the ssl_sock_set_servername() function. It is no longer possible to compare the SNI of the reused session with the one of the new connection. So, the SNI is always set, with no other processing. Mainly, the session is not destroyed when SNIs don't match. It means the commit 119a4084bf ("BUG/MEDIUM: ssl: for a handshake when server-side SNI changes") is implicitly reverted. It is good to note that it is unclear for me when and why the reused session should be destroyed. Because I'm unable to reproduce any issue fixed by the commit above. This patch could be backported as far as 3.0 with the commit above. --- include/haproxy/server-t.h | 1 - src/ssl_ckch.c | 4 +--- src/ssl_sock.c | 42 ++++---------------------------------- 3 files changed, 5 insertions(+), 42 deletions(-) diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h index 2dc27ecc4..7a2a95400 100644 --- a/include/haproxy/server-t.h +++ b/include/haproxy/server-t.h @@ -486,7 +486,6 @@ struct server { int size; int allocated_size; uint64_t sni_hash; /* Hash of the SNI used for the session */ - char *sni; /* SNI used for the session */ __decl_thread(HA_RWLOCK_T sess_lock); } * reused_sess; diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index 8ff6847c0..900e89de3 100644 --- a/src/ssl_ckch.c +++ b/src/ssl_ckch.c @@ -2770,10 +2770,8 @@ static void __ssl_sock_load_new_ckch_instance(struct ckch_inst *ckchi) ckchi->server->ssl_ctx.inst = ckchi; /* flush the session cache of the server */ - for (i = 0; i < global.nbthread; i++) { - ha_free(&ckchi->server->ssl_ctx.reused_sess[i].sni); + for (i = 0; i < global.nbthread; i++) ha_free(&ckchi->server->ssl_ctx.reused_sess[i].ptr); - } HA_RWLOCK_WRUNLOCK(SSL_SERVER_LOCK, &ckchi->server->ssl_ctx.lock); } else { diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 7a9a2da24..601c97613 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -4201,14 +4201,12 @@ static int ssl_sess_new_srv_cb(SSL *ssl, SSL_SESSION *sess) if (!(s->ssl_ctx.options & SRV_SSL_O_NO_REUSE)) { int len; unsigned char *ptr; - const char *sni; #ifdef USE_QUIC struct quic_conn *qc = SSL_get_ex_data(ssl, ssl_qc_app_data_index); #endif /* determine the required len to store this new session */ len = i2d_SSL_SESSION(sess, NULL); - sni = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name); HA_RWLOCK_RDLOCK(SSL_SERVER_LOCK, &s->ssl_ctx.lock); ptr = s->ssl_ctx.reused_sess[tid].ptr; @@ -4245,14 +4243,7 @@ static int ssl_sess_new_srv_cb(SSL *ssl, SSL_SESSION *sess) HA_ATOMIC_CAS(&s->ssl_ctx.last_ssl_sess_tid, &old_tid, 0); // no more valid else if (s->ssl_ctx.reused_sess[tid].ptr && !old_tid) HA_ATOMIC_CAS(&s->ssl_ctx.last_ssl_sess_tid, &old_tid, tid + 1); - - if (s->ssl_ctx.reused_sess[tid].sni_hash != conn->sni_hash) { - /* if the new sni hash or isn' t the same as the old one */ - ha_free(&s->ssl_ctx.reused_sess[tid].sni); - s->ssl_ctx.reused_sess[tid].sni_hash = conn->sni_hash; - if (sni) - s->ssl_ctx.reused_sess[tid].sni = strdup(sni); - } + s->ssl_ctx.reused_sess[tid].sni_hash = conn->sni_hash; #ifdef USE_QUIC /* The selected ALPN is not stored without SSL session. */ if (qc && (s->ssl_ctx.options & SRV_SSL_O_EARLY_DATA) && @@ -5477,10 +5468,8 @@ void ssl_sock_free_srv_ctx(struct server *srv) if (srv->ssl_ctx.reused_sess) { int i; - for (i = 0; i < global.nbthread; i++) { + for (i = 0; i < global.nbthread; i++) ha_free(&srv->ssl_ctx.reused_sess[i].ptr); - ha_free(&srv->ssl_ctx.reused_sess[i].sni); - } ha_free(&srv->ssl_ctx.reused_sess); } @@ -5723,16 +5712,10 @@ int ssl_sock_srv_try_reuse_sess(struct ssl_sock_ctx *ctx, struct server *srv) SSL_SESSION_free(sess); HA_RWLOCK_WRLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[tid].sess_lock); ha_free(&srv->ssl_ctx.reused_sess[tid].ptr); - HA_RWLOCK_WRTORD(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[tid].sess_lock); - if (srv->ssl_ctx.reused_sess[tid].sni) - SSL_set_tlsext_host_name(ctx->ssl, srv->ssl_ctx.reused_sess[tid].sni); - HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[tid].sess_lock); + HA_RWLOCK_WRUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[tid].sess_lock); } else if (sess) { /* already assigned, not needed anymore */ SSL_SESSION_free(sess); - HA_RWLOCK_RDLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[tid].sess_lock); - if (srv->ssl_ctx.reused_sess[tid].sni) - SSL_set_tlsext_host_name(ctx->ssl, srv->ssl_ctx.reused_sess[tid].sni); #ifdef USE_QUIC if (qc && srv->ssl_ctx.options & SRV_SSL_O_EARLY_DATA) { unsigned char *alpn = (unsigned char *)srv->path_params.nego_alpn; @@ -5743,7 +5726,6 @@ int ssl_sock_srv_try_reuse_sess(struct ssl_sock_ctx *ctx, struct server *srv) ret = 1; } #endif - HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[tid].sess_lock); } } else { /* No session available yet, let's see if we can pick one @@ -5787,9 +5769,6 @@ int ssl_sock_srv_try_reuse_sess(struct ssl_sock_ctx *ctx, struct server *srv) } } - if (srv->ssl_ctx.reused_sess[old_tid-1].sni) - SSL_set_tlsext_host_name(ctx->ssl, srv->ssl_ctx.reused_sess[old_tid-1].sni); - HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &srv->ssl_ctx.reused_sess[old_tid-1].sess_lock); } } @@ -7632,7 +7611,6 @@ void ssl_sock_set_servername(struct connection *conn, const char *hostname) { #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME struct ssl_sock_ctx *ctx = conn_get_ssl_sock_ctx(conn); - char *prev_name; if (!ctx || !hostname) return; @@ -7640,19 +7618,7 @@ void ssl_sock_set_servername(struct connection *conn, const char *hostname) BUG_ON(!(conn->flags & CO_FL_WAIT_L6_CONN)); BUG_ON(!(conn->flags & CO_FL_SSL_WAIT_HS)); - /* if the SNI changes, we must destroy the reusable context so that a - * new connection will present a new SNI. compare with the SNI - * previously stored in the reused_sess. If the session was reused, - * the associated SNI (if any) has already been assigned to the SSL - * during ssl_sock_init() so SSL_get_servername() will properly - * retrieve the currently known hostname for the SSL. - */ - - prev_name = (char *)SSL_get_servername(ctx->ssl, TLSEXT_NAMETYPE_host_name); - if (!prev_name || strcmp(hostname, prev_name) != 0) { - SSL_set_session(ctx->ssl, NULL); - SSL_set_tlsext_host_name(ctx->ssl, hostname); - } + SSL_set_tlsext_host_name(ctx->ssl, hostname); #endif } -- 2.47.3