From: Willy Tarreau <w@1wt.eu>
Date: Wed, 4 May 2022 14:11:50 +0000 (+0200)
Subject: BUG/MINOR: ssl/cli: fix "show ssl cert" not to mix cli+ssl contexts
X-Git-Tag: v2.6-dev9~76
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4fd9b4ddf04bfddaa46dd720da91c7b025f963c4;p=thirdparty%2Fhaproxy.git

BUG/MINOR: ssl/cli: fix "show ssl cert" not to mix cli+ssl contexts

The "show ssl cert" command mixes some generic pointers from the
"ctx.cli" struct with context-specific ones from "ctx.ssl" while both
are in a union. Amazingly, despite the use of both p0 and i0 to store
respectively a pointer to the current ckchs and a transaction id, there
was no overlap with the other pointers used during these operations,
but should these fields be reordered or slightly updated this will break.
Comments were added above the faulty functions to indicate which fields
they are using.

This needs to be backported to 2.5.
---

diff --git a/include/haproxy/applet-t.h b/include/haproxy/applet-t.h
index 15a8736aa4..ea730c7b7a 100644
--- a/include/haproxy/applet-t.h
+++ b/include/haproxy/applet-t.h
@@ -179,6 +179,7 @@ struct appctx {
 			struct ckch_store *old_ckchs;
 			struct ckch_store *new_ckchs;
 			struct ckch_inst *next_ckchi;
+			struct ckch_store *cur_ckchs;
 
 			struct ckch_inst_link *next_ckchi_link;
 			struct cafile_entry *old_cafile_entry;
diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index cb0ed5f0ee..1e4909c8cc 100644
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -1231,7 +1231,9 @@ static void cli_release_show_cert(struct appctx *appctx)
 	HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock);
 }
 
-/* IO handler of "show ssl cert <filename>" */
+/* IO handler of "show ssl cert <filename>".
+ * It makes use of ctx.ssl.cur_ckchs, ctx.ssl.old_ckchs.
+ */
 static int cli_io_handler_show_cert(struct appctx *appctx)
 {
 	struct buffer *trash = alloc_trash_chunk();
@@ -1250,11 +1252,11 @@ static int cli_io_handler_show_cert(struct appctx *appctx)
 		}
 	}
 
-	if (!appctx->ctx.cli.p0) {
+	if (!appctx->ctx.ssl.cur_ckchs) {
 		chunk_appendf(trash, "# filename\n");
 		node = ebmb_first(&ckchs_tree);
 	} else {
-		node = &((struct ckch_store *)appctx->ctx.cli.p0)->node;
+		node = &((struct ckch_store *)appctx->ctx.ssl.cur_ckchs)->node;
 	}
 	while (node) {
 		ckchs = ebmb_entry(node, struct ckch_store, node);
@@ -1267,13 +1269,13 @@ static int cli_io_handler_show_cert(struct appctx *appctx)
 		}
 	}
 
-	appctx->ctx.cli.p0 = NULL;
+	appctx->ctx.ssl.cur_ckchs = NULL;
 	free_trash_chunk(trash);
 	return 1;
 yield:
 
 	free_trash_chunk(trash);
-	appctx->ctx.cli.p0 = ckchs;
+	appctx->ctx.ssl.cur_ckchs = ckchs;
 	return 0; /* should come back */
 }
 
@@ -1632,11 +1634,13 @@ static int ckch_store_show_ocsp_certid(struct ckch_store *ckch_store, struct buf
 }
 
 
-/* IO handler of the details "show ssl cert <filename>" */
+/* IO handler of the details "show ssl cert <filename>".
+ * It uses ctx.ssl.cur_ckchs.
+ */
 static int cli_io_handler_show_cert_detail(struct appctx *appctx)
 {
 	struct conn_stream *cs = appctx->owner;
-	struct ckch_store *ckchs = appctx->ctx.cli.p0;
+	struct ckch_store *ckchs = appctx->ctx.ssl.cur_ckchs;
 	struct buffer *out = alloc_trash_chunk();
 	int retval = 0;
 
@@ -1679,14 +1683,16 @@ yield:
 }
 
 
-/* IO handler of the details "show ssl cert <filename.ocsp>" */
+/* IO handler of the details "show ssl cert <filename.ocsp>".
+ * It uses ctx.ssl.cur_ckchs and ctx.ssl.index.
+ */
 static int cli_io_handler_show_cert_ocsp_detail(struct appctx *appctx)
 {
 #if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP) && !defined OPENSSL_IS_BORINGSSL)
 	struct conn_stream *cs = appctx->owner;
-	struct ckch_store *ckchs = appctx->ctx.cli.p0;
+	struct ckch_store *ckchs = appctx->ctx.ssl.cur_ckchs;
 	struct buffer *out = alloc_trash_chunk();
-	int from_transaction = appctx->ctx.cli.i0;
+	int from_transaction = appctx->ctx.ssl.index;
 
 	if (!out)
 		goto end_no_putchk;
@@ -1769,10 +1775,10 @@ static int cli_parse_show_cert(char **args, char *payload, struct appctx *appctx
 
 		}
 
-		appctx->ctx.cli.p0 = ckchs;
+		appctx->ctx.ssl.cur_ckchs = ckchs;
 		/* use the IO handler that shows details */
 		if (show_ocsp_detail) {
-			appctx->ctx.cli.i0 = from_transaction;
+			appctx->ctx.ssl.index = from_transaction;
 			appctx->io_handler = cli_io_handler_show_cert_ocsp_detail;
 		}
 		else