From 4fd9b4ddf04bfddaa46dd720da91c7b025f963c4 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 4 May 2022 16:11:50 +0200 Subject: [PATCH] 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. --- include/haproxy/applet-t.h | 1 + src/ssl_ckch.c | 30 ++++++++++++++++++------------ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/include/haproxy/applet-t.h b/include/haproxy/applet-t.h index 15a8736aa..ea730c7b7 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 cb0ed5f0e..1e4909c8c 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 " */ +/* IO handler of "show ssl cert ". + * 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 " */ +/* IO handler of the details "show ssl cert ". + * 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 " */ +/* IO handler of the details "show ssl cert ". + * 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