From 8695ce0bae21238eba660438c819797a245be71e Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallemand@haproxy.org>
Date: Mon, 1 Feb 2021 15:31:00 +0100
Subject: [PATCH] BUG/MEDIUM: ssl/cli: abort ssl cert is freeing the old store

The "abort ssl cert" command is buggy and removes the current ckch store,
and instances, leading to SNI removal. It must only removes the new one.

This patch also adds a check in set_ssl_cert.vtc and
set_ssl_server_cert.vtc.

Must be backported as far as 2.2.
---
 reg-tests/ssl/set_ssl_cert.vtc        |   11 +++++++++++
 reg-tests/ssl/set_ssl_server_cert.vtc |   20 ++++++++++++++++++++
 src/ssl_ckch.c                        |    1 -
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/reg-tests/ssl/set_ssl_cert.vtc b/reg-tests/ssl/set_ssl_cert.vtc
index 0e84058..d6d4526 100644
--- a/reg-tests/ssl/set_ssl_cert.vtc
+++ b/reg-tests/ssl/set_ssl_cert.vtc
@@ -86,3 +86,14 @@ client c1 -connect ${h1_clearlst_sock} {
     rxresp
     expect resp.status == 200
 } -run
+
+shell {
+    printf "set ssl cert ${testdir}/common.pem <<\n$(cat ${testdir}/common.pem)\n\n" | socat "${tmpdir}/h1/stats" -
+    echo "abort ssl cert ${testdir}/common.pem" | socat "${tmpdir}/h1/stats" -
+}
+
+haproxy h1 -cli {
+    send "show ssl cert ${testdir}/common.pem"
+    expect ~ ".*SHA1 FingerPrint: A490D069DBAFBEE66DE434BEC34030ADE8BCCBF1"
+}
+
diff --git a/reg-tests/ssl/set_ssl_server_cert.vtc b/reg-tests/ssl/set_ssl_server_cert.vtc
index cab2d04..412e9f0 100644
--- a/reg-tests/ssl/set_ssl_server_cert.vtc
+++ b/reg-tests/ssl/set_ssl_server_cert.vtc
@@ -108,3 +108,23 @@ client c1 -connect ${h1_clearlst_sock} {
     expect resp.http.x-ssl == "Revoked"
 } -run
 
+# Abort a transaction
+shell {
+    printf "set ssl cert ${testdir}/client1.pem <<\n$(cat ${testdir}/client3_revoked.pem)\n\n" | socat "${tmpdir}/h1/stats" -
+    echo "abort ssl cert ${testdir}/client1.pem" | socat "${tmpdir}/h1/stats" -
+}
+
+haproxy h1 -cli {
+    send "show ssl cert ${testdir}/client1.pem"
+    expect ~ ".*SHA1 FingerPrint: 992386628A40C9D49C89BAC0058B5D45D8575151"
+}
+
+# The certificate was not updated so it should still be revoked
+client c1 -connect ${h1_clearlst_sock} {
+    txreq
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-ssl == "Revoked"
+} -run
+
+
diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 62e7b44..e8a20c3 100644
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -1742,7 +1742,6 @@ static int cli_parse_abort_cert(char **args, char *payload, struct appctx *appct
 	/* Only free the ckchs there, because the SNI and instances were not generated yet */
 	ckch_store_free(ckchs_transaction.new_ckchs);
 	ckchs_transaction.new_ckchs = NULL;
-	ckch_store_free(ckchs_transaction.old_ckchs);
 	ckchs_transaction.old_ckchs = NULL;
 	free(ckchs_transaction.path);
 	ckchs_transaction.path = NULL;
-- 
1.7.10.4