mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-23 06:41:32 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: ssl/cli: fix "show ssl ca-file/crl-file" not to mix cli+ssl contexts

Browse Source 
The "show ca-file" and "show crl-file" commands mix some generic pointers
from the "ctx.cli" struct and context-specific ones from "ctx.ssl" while
both are in a union. It's fortunate that the p0 pointer in use is located
immediately before the first one used (it overlaps with next_ckchi_link,
and old_cafile_entry is safe). But should these fields be reordered or
slightly updated this will break.

Comments were added on top of the affected functions to indicate what they
use.

This needs to be backported to 2.5.
This commit is contained in:
Willy Tarreau 2022-05-04 15:47:39 +02:00
parent 1ae0c43244
commit 821c3b0b5e
2 changed files with 32 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/haproxy/applet-t.h
View File

@ -183,6 +183,7 @@ struct appctx {
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;
struct cafile_entry *new_cafile_entry; struct cafile_entry *new_cafile_entry;
struct cafile_entry *cur_cafile_entry;
struct cafile_entry *old_crlfile_entry; struct cafile_entry *old_crlfile_entry;
struct cafile_entry *new_crlfile_entry; struct cafile_entry *new_crlfile_entry;

48
src/ssl_ckch.c
View File

@ -2908,7 +2908,9 @@ error:
return cli_dynerr(appctx, err); return cli_dynerr(appctx, err);
} }
/* release function of the `commit ssl ca-file' command, free things and unlock the spinlock */ /* release function of the `commit ssl ca-file' command, free things and unlock the spinlock.
* It uses ctx.ssl.new_cafile_entry.
*/
static void cli_release_commit_cafile(struct appctx *appctx) static void cli_release_commit_cafile(struct appctx *appctx)
{ {
if (appctx->st2 != SETCERT_ST_FIN) { if (appctx->st2 != SETCERT_ST_FIN) {
@ -2922,11 +2924,14 @@ static void cli_release_commit_cafile(struct appctx *appctx)
} }
/* IO handler of details "show ssl ca-file <filename[:index]>" */ /* IO handler of details "show ssl ca-file <filename[:index]>".
* It uses ctx.ssl.cur_cafile_entry, ctx.cli.i0, ctx.cli.i1, and
* the global cafile_transaction.new_cafile_entry in read-only.
*/
static int cli_io_handler_show_cafile_detail(struct appctx *appctx) static int cli_io_handler_show_cafile_detail(struct appctx *appctx)
{ {
struct conn_stream *cs = appctx->owner; struct conn_stream *cs = appctx->owner;
struct cafile_entry *cafile_entry = appctx->ctx.cli.p0; struct cafile_entry *cafile_entry = appctx->ctx.ssl.cur_cafile_entry;
struct buffer *out = alloc_trash_chunk(); struct buffer *out = alloc_trash_chunk();
int i = 0; int i = 0;
X509 *cert; X509 *cert;
@ -3046,7 +3051,7 @@ static int cli_parse_show_cafile(char **args, char *payload, struct appctx *appc
goto error; goto error;
} }
appctx->ctx.cli.p0 = cafile_entry; appctx->ctx.ssl.cur_cafile_entry = cafile_entry;
/* use the IO handler that shows details */ /* use the IO handler that shows details */
appctx->io_handler = cli_io_handler_show_cafile_detail; appctx->io_handler = cli_io_handler_show_cafile_detail;
} }
@ -3083,7 +3088,10 @@ static int get_certificate_count(struct cafile_entry *cafile_entry)
} }
/* IO handler of "show ssl ca-file". The command taking a specific CA file name /* IO handler of "show ssl ca-file". The command taking a specific CA file name
* is managed in cli_io_handler_show_cafile_detail. */ * is managed in cli_io_handler_show_cafile_detail.
* It uses ctx.ssl.cur_cafile_entry, and the global
* cafile_transaction.new_cafile_entry in read-only.
*/
static int cli_io_handler_show_cafile(struct appctx *appctx) static int cli_io_handler_show_cafile(struct appctx *appctx)
{ {
struct buffer *trash = alloc_trash_chunk(); struct buffer *trash = alloc_trash_chunk();
@ -3104,12 +3112,12 @@ static int cli_io_handler_show_cafile(struct appctx *appctx)
} }
/* First time in this io_handler. */ /* First time in this io_handler. */
if (!appctx->ctx.cli.p0) { if (!appctx->ctx.ssl.cur_cafile_entry) {
chunk_appendf(trash, "# filename\n"); chunk_appendf(trash, "# filename\n");
node = ebmb_first(&cafile_tree); node = ebmb_first(&cafile_tree);
} else { } else {
/* We yielded during a previous call. */ /* We yielded during a previous call. */
node = &((struct cafile_entry*)appctx->ctx.cli.p0)->node; node = &appctx->ctx.ssl.cur_cafile_entry->node;
} }
while (node) { while (node) {
@ -3127,13 +3135,13 @@ static int cli_io_handler_show_cafile(struct appctx *appctx)
} }
} }
appctx->ctx.cli.p0 = NULL; appctx->ctx.ssl.cur_cafile_entry = 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 = cafile_entry; appctx->ctx.ssl.cur_cafile_entry = cafile_entry;
return 0; /* should come back */ return 0; /* should come back */
} }
@ -3591,11 +3599,14 @@ end:
return 0; return 0;
} }
/* IO handler of details "show ssl crl-file <filename[:index]>" */ /* IO handler of details "show ssl crl-file <filename[:index]>".
* It uses ctx.ssl.cur_cafile_entry, ctx.cli.p1, ctx.cli.i1, and
* the global crlfile_transaction.new_cafile_entry in read-only.
*/
static int cli_io_handler_show_crlfile_detail(struct appctx *appctx) static int cli_io_handler_show_crlfile_detail(struct appctx *appctx)
{ {
struct conn_stream *cs = appctx->owner; struct conn_stream *cs = appctx->owner;
struct cafile_entry *cafile_entry = appctx->ctx.cli.p0; struct cafile_entry *cafile_entry = appctx->ctx.ssl.cur_cafile_entry;
struct buffer *out = alloc_trash_chunk(); struct buffer *out = alloc_trash_chunk();
int i; int i;
X509_CRL *crl; X509_CRL *crl;
@ -3654,7 +3665,10 @@ yield:
return 0; /* should come back */ return 0; /* should come back */
} }
/* parsing function for 'show ssl crl-file [crlfile[:index]]' */ /* parsing function for 'show ssl crl-file [crlfile[:index]]'.
* It sets ctx.ssl.cur_cafile_entry, ctx.cli.p1, and the global
* cafile_transaction.new_crlfile_entry under the ckch_lock.
*/
static int cli_parse_show_crlfile(char **args, char *payload, struct appctx *appctx, void *private) static int cli_parse_show_crlfile(char **args, char *payload, struct appctx *appctx, void *private)
{ {
struct cafile_entry *cafile_entry; struct cafile_entry *cafile_entry;
@ -3703,7 +3717,7 @@ static int cli_parse_show_crlfile(char **args, char *payload, struct appctx *app
goto error; goto error;
} }
appctx->ctx.cli.p0 = cafile_entry; appctx->ctx.ssl.cur_cafile_entry = cafile_entry;
appctx->ctx.cli.p1 = (void*)index; appctx->ctx.cli.p1 = (void*)index;
/* use the IO handler that shows details */ /* use the IO handler that shows details */
appctx->io_handler = cli_io_handler_show_crlfile_detail; appctx->io_handler = cli_io_handler_show_crlfile_detail;
@ -3738,12 +3752,12 @@ static int cli_io_handler_show_crlfile(struct appctx *appctx)
} }
/* First time in this io_handler. */ /* First time in this io_handler. */
if (!appctx->ctx.cli.p0) { if (!appctx->ctx.ssl.cur_cafile_entry) {
chunk_appendf(trash, "# filename\n"); chunk_appendf(trash, "# filename\n");
node = ebmb_first(&cafile_tree); node = ebmb_first(&cafile_tree);
} else { } else {
/* We yielded during a previous call. */ /* We yielded during a previous call. */
node = &((struct cafile_entry*)appctx->ctx.cli.p0)->node; node = &appctx->ctx.ssl.cur_cafile_entry->node;
} }
while (node) { while (node) {
@ -3759,13 +3773,13 @@ static int cli_io_handler_show_crlfile(struct appctx *appctx)
} }
} }
appctx->ctx.cli.p0 = NULL; appctx->ctx.ssl.cur_cafile_entry = 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 = cafile_entry; appctx->ctx.ssl.cur_cafile_entry = cafile_entry;
return 0; /* should come back */ return 0; /* should come back */
} }