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.

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