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.
This commit is contained in:
Willy Tarreau 2022-05-04 16:11:50 +02:00
parent 4cf3ef8007
commit 4fd9b4ddf0
2 changed files with 19 additions and 12 deletions

View File

@ -179,6 +179,7 @@ struct appctx {
struct ckch_store *old_ckchs; struct ckch_store *old_ckchs;
struct ckch_store *new_ckchs; struct ckch_store *new_ckchs;
struct ckch_inst *next_ckchi; struct ckch_inst *next_ckchi;
struct ckch_store *cur_ckchs;
struct ckch_inst_link *next_ckchi_link; struct ckch_inst_link *next_ckchi_link;
struct cafile_entry *old_cafile_entry; struct cafile_entry *old_cafile_entry;

View File

@ -1231,7 +1231,9 @@ static void cli_release_show_cert(struct appctx *appctx)
HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock); 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) static int cli_io_handler_show_cert(struct appctx *appctx)
{ {
struct buffer *trash = alloc_trash_chunk(); 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"); chunk_appendf(trash, "# filename\n");
node = ebmb_first(&ckchs_tree); node = ebmb_first(&ckchs_tree);
} else { } else {
node = &((struct ckch_store *)appctx->ctx.cli.p0)->node; node = &((struct ckch_store *)appctx->ctx.ssl.cur_ckchs)->node;
} }
while (node) { while (node) {
ckchs = ebmb_entry(node, struct ckch_store, 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); free_trash_chunk(trash);
return 1; return 1;
yield: yield:
free_trash_chunk(trash); free_trash_chunk(trash);
appctx->ctx.cli.p0 = ckchs; appctx->ctx.ssl.cur_ckchs = ckchs;
return 0; /* should come back */ 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) static int cli_io_handler_show_cert_detail(struct appctx *appctx)
{ {
struct conn_stream *cs = appctx->owner; 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(); struct buffer *out = alloc_trash_chunk();
int retval = 0; 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) 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) #if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP) && !defined OPENSSL_IS_BORINGSSL)
struct conn_stream *cs = appctx->owner; 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(); struct buffer *out = alloc_trash_chunk();
int from_transaction = appctx->ctx.cli.i0; int from_transaction = appctx->ctx.ssl.index;
if (!out) if (!out)
goto end_no_putchk; 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 */ /* use the IO handler that shows details */
if (show_ocsp_detail) { 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; appctx->io_handler = cli_io_handler_show_cert_ocsp_detail;
} }
else else