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

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 880bb2bd9f398523701d899e2f4406e473d66930..7a9a2da245e42fde7fbb365e4d295faec20edda5 100644 (file)
--- 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;