From 94bd319b264a64f73202a82bdd2d0bcf23722246 Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallemand@haproxy.com>
Date: Fri, 14 Aug 2020 14:43:35 +0200
Subject: [PATCH] BUG/MEDIUM: ssl: does not look for all SNIs before chosing a
 certificate

In bug #810, the SNI are not matched correctly, indeed when trying to
match a certificate type in ssl_sock_switchctx_cbk() all SNIs were not
looked up correctly.

In the case you have in a crt-list:

wildcard.subdomain.domain.tld.pem.rsa *.subdomain.domain.tld record.subdomain.domain.tld
record.subdomain.domain.tld.pem.ecdsa record.subdomain.domain.tld another-record.subdomain.domain.tld

If the client only supports RSA and requests
"another-record.subdomain.domain.tld", HAProxy will find the single
ECDSA certificate and won't try to look up for a wildcard RSA
certificate.

This patch fixes the code so we look for all single and
wildcard before chosing the certificate type.

This bug was introduced by commit 3777e3a ("BUG/MINOR: ssl: certificate
choice can be unexpected with openssl >= 1.1.1").

It must be backported as far as 1.8 once it is heavily tested.
---
 src/ssl_sock.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index bb28f1815..aabd861c5 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -2352,6 +2352,8 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *arg)
 
 	HA_RWLOCK_RDLOCK(SNI_LOCK, &s->sni_lock);
 
+	/* Look for an ECDSA, RSA and DSA certificate, first in the single
+	 * name and if not found in the wildcard  */
 	for (i = 0; i < 2; i++) {
 		if (i == 0) 	/* lookup in full qualified names */
 			node = ebst_lookup(&s->sni_ctx, trash.area);
@@ -2378,26 +2380,28 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *arg)
 				}
 			}
 		}
-		/* select by key_signature priority order */
-		node = (has_ecdsa_sig && node_ecdsa) ? node_ecdsa
-			: ((has_rsa_sig && node_rsa) ? node_rsa
-			   : (node_anonymous ? node_anonymous
-			      : (node_ecdsa ? node_ecdsa      /* no ecdsa signature case (< TLSv1.2) */
-				 : node_rsa                   /* no rsa signature case (far far away) */
-				 )));
-		if (node) {
-			/* switch ctx */
-			struct ssl_bind_conf *conf = container_of(node, struct sni_ctx, name)->conf;
-			ssl_sock_switchctx_set(ssl, container_of(node, struct sni_ctx, name)->ctx);
-			if (conf) {
-				methodVersions[conf->ssl_methods.min].ssl_set_version(ssl, SET_MIN);
-				methodVersions[conf->ssl_methods.max].ssl_set_version(ssl, SET_MAX);
-				if (conf->early_data)
-					allow_early = 1;
-			}
-			HA_RWLOCK_RDUNLOCK(SNI_LOCK, &s->sni_lock);
-			goto allow_early;
+	}
+	/* Once the certificates are found, select them depending on what is
+	 * supported in the client and by key_signature priority order: EDSA >
+	 * RSA > DSA */
+	node = (has_ecdsa_sig && node_ecdsa) ? node_ecdsa
+		: ((has_rsa_sig && node_rsa) ? node_rsa
+		   : (node_anonymous ? node_anonymous
+		      : (node_ecdsa ? node_ecdsa      /* no ecdsa signature case (< TLSv1.2) */
+			 : node_rsa                   /* no rsa signature case (far far away) */
+			)));
+	if (node) {
+		/* switch ctx */
+		struct ssl_bind_conf *conf = container_of(node, struct sni_ctx, name)->conf;
+		ssl_sock_switchctx_set(ssl, container_of(node, struct sni_ctx, name)->ctx);
+		if (conf) {
+			methodVersions[conf->ssl_methods.min].ssl_set_version(ssl, SET_MIN);
+			methodVersions[conf->ssl_methods.max].ssl_set_version(ssl, SET_MAX);
+			if (conf->early_data)
+				allow_early = 1;
 		}
+		HA_RWLOCK_RDUNLOCK(SNI_LOCK, &s->sni_lock);
+		goto allow_early;
 	}
 
 	HA_RWLOCK_RDUNLOCK(SNI_LOCK, &s->sni_lock);