From cb6c5f468341e1902fae7527bfe76921d9d2aea6 Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallemand@haproxy.org>
Date: Mon, 20 Jun 2022 16:51:53 +0200
Subject: [PATCH] BUG/MEDIUM: ssl/cli: crash when crt inserted into a crt-list

The crash occures when the same certificate which is used on both a
server line and a bind line is inserted in a crt-list over the CLI.

This is quite uncommon as using the same file for a client and a server
certificate does not make sense in a lot of environments.

This patch fixes the issue by skipping the insertion of the SNI when no
bind_conf is available in the ckch_inst.

Change the reg-test to reproduce this corner case.

Should fix issue #1748.

Must be backported as far as 2.2. (it was previously in ssl_sock.c)
---
 reg-tests/ssl/add_ssl_crt-list.vtc | 2 ++
 src/ssl_crtlist.c                  | 9 ++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/reg-tests/ssl/add_ssl_crt-list.vtc b/reg-tests/ssl/add_ssl_crt-list.vtc
index 5ed72bfe2..e62ac4892 100644
--- a/reg-tests/ssl/add_ssl_crt-list.vtc
+++ b/reg-tests/ssl/add_ssl_crt-list.vtc
@@ -50,6 +50,7 @@ haproxy h1 -conf {
         bind "${tmpdir}/ssl.sock" ssl strict-sni crt-list ${testdir}/localhost.crt-list
 
         server s1 ${s1_addr}:${s1_port}
+        server s2 ${s1_addr}:${s1_port} ssl crt "${testdir}/common.pem" weight 0 verify none
 } -start
 
 
@@ -68,6 +69,7 @@ shell {
     echo "new ssl cert ${testdir}/ecdsa.pem" | socat "${tmpdir}/h1/stats" -
     printf "set ssl cert ${testdir}/ecdsa.pem <<\n$(cat ${testdir}/ecdsa.pem)\n\n" | socat "${tmpdir}/h1/stats" -
     echo "commit ssl cert ${testdir}/ecdsa.pem" | socat "${tmpdir}/h1/stats" -
+    printf "add ssl crt-list ${testdir}/localhost.crt-list/ <<\n${testdir}/common.pem [ssl-min-ver SSLv3 verify none allow-0rtt] !*\n\n" | socat "${tmpdir}/h1/stats" -
     printf "add ssl crt-list ${testdir}/localhost.crt-list/ <<\n${testdir}/ecdsa.pem [ssl-min-ver SSLv3 verify none allow-0rtt] localhost !www.test1.com\n\n" | socat "${tmpdir}/h1/stats" -
     printf "add ssl crt-list ${testdir}/localhost.crt-list <<\n${testdir}/ecdsa.pem [verify none allow-0rtt]\n\n" | socat "${tmpdir}/h1/stats" -
     printf "add ssl crt-list ${testdir}/localhost.crt-list/// <<\n${testdir}/ecdsa.pem localhost !www.test1.com\n\n" | socat "${tmpdir}/h1/stats" -
diff --git a/src/ssl_crtlist.c b/src/ssl_crtlist.c
index 8aa4eae2a..a8cd24044 100644
--- a/src/ssl_crtlist.c
+++ b/src/ssl_crtlist.c
@@ -1138,8 +1138,15 @@ static int cli_io_handler_add_crtlist(struct appctx *appctx)
 		ctx->state = ADDCRT_ST_INSERT;
 		/* fallthrough */
 	case ADDCRT_ST_INSERT:
-		/* insert SNIs in bind_conf */
+		/* the insertion is called for every instance of the store, not
+		 * only the one we generated.
+		 * But the ssl_sock_load_cert_sni() skip the sni already
+		 * inserted. Not every instance has a bind_conf, it could be
+		 * the store of a server so we should be careful */
+
 		list_for_each_entry(new_inst, &store->ckch_inst, by_ckchs) {
+			if (!new_inst->bind_conf) /* this is a server instance */
+				continue;
 			HA_RWLOCK_WRLOCK(SNI_LOCK, &new_inst->bind_conf->sni_lock);
 			ssl_sock_load_cert_sni(new_inst, new_inst->bind_conf);
 			HA_RWLOCK_WRUNLOCK(SNI_LOCK, &new_inst->bind_conf->sni_lock);