mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2026-05-04 20:46:11 +02:00
Code Issues Projects Releases Wiki Activity

MEDIUM: ssl/server: No longer store the SNI of cached TLS sessions

Browse Source 
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.
This commit is contained in:
Christopher Faulet 2025-12-05 17:35:53 +01:00
parent 5702009c8c
commit be998b590e
3 changed files with 5 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/haproxy/server-t.h
View File

@ -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;

4
src/ssl_ckch.c
View File

@ -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 {

42
src/ssl_sock.c
View File

@ -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
}