CLEANUP: ssl/cli: use a local context for "show cafile"

Saying that the layout and usage of the various variables in the ssl
applet context is a mess would be an understatement. It's very hard
to know what command uses what fields, even after having moved away
from the mix of cli and ssl.

Let's extract the parts used by "show cafile" into their own structure.
Only the "show_all" field would be removed from the ssl ctx, the other
fields are still shared with other commands.
This commit is contained in:
Willy Tarreau 2022-05-04 19:26:59 +02:00
parent bcda5f6bcd
commit 50c2f1e0cd
2 changed files with 31 additions and 21 deletions

View File

@ -154,7 +154,6 @@ struct appctx {
struct cafile_entry *new_crlfile_entry; struct cafile_entry *new_crlfile_entry;
int cafile_type; /* either CA or CRL, depending on the current command */ int cafile_type; /* either CA or CRL, depending on the current command */
int index; int index;
int show_all;
} ssl; } ssl;
struct { struct {
void *ptr; void *ptr;

View File

@ -26,6 +26,7 @@
#include <import/ebpttree.h> #include <import/ebpttree.h>
#include <import/ebsttree.h> #include <import/ebsttree.h>
#include <haproxy/applet.h>
#include <haproxy/base64.h> #include <haproxy/base64.h>
#include <haproxy/channel.h> #include <haproxy/channel.h>
#include <haproxy/cli.h> #include <haproxy/cli.h>
@ -61,6 +62,13 @@ static struct {
char *path; char *path;
} crlfile_transaction; } crlfile_transaction;
/* CLI context used by "show cafile" */
struct show_cafile_ctx {
struct cafile_entry *cur_cafile_entry;
struct cafile_entry *old_cafile_entry;
int ca_index;
int show_all;
};
/******************** cert_key_and_chain functions ************************* /******************** cert_key_and_chain functions *************************
@ -2931,20 +2939,21 @@ 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.ssl.index, ctx.ssl.show_all, * It uses a show_cafile_ctx context, and the global
* and the global cafile_transaction.new_cafile_entry in read-only. * 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 show_cafile_ctx *ctx = appctx->svcctx;
struct conn_stream *cs = appctx->owner; struct conn_stream *cs = appctx->owner;
struct cafile_entry *cafile_entry = appctx->ctx.ssl.cur_cafile_entry; struct cafile_entry *cafile_entry = ctx->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;
STACK_OF(X509_OBJECT) *objs; STACK_OF(X509_OBJECT) *objs;
int retval = 0; int retval = 0;
int ca_index = appctx->ctx.ssl.index; int ca_index = ctx->ca_index;
int show_all = appctx->ctx.ssl.show_all; int show_all = ctx->show_all;
if (!out) if (!out)
goto end_no_putchk; goto end_no_putchk;
@ -2998,18 +3007,19 @@ end_no_putchk:
return 1; return 1;
yield: yield:
/* save the current state */ /* save the current state */
appctx->ctx.ssl.index = i; ctx->ca_index = i;
free_trash_chunk(out); free_trash_chunk(out);
return 0; /* should come back */ return 0; /* should come back */
} }
/* parsing function for 'show ssl ca-file [cafile[:index]]'. /* parsing function for 'show ssl ca-file [cafile[:index]]'.
* It sets ctx.ssl.cur_cafile_entry, ctx.ssl.show_all and ctx.ssl.index * It prepares a show_cafile_ctx context, and checks the global
* and checks the global cafile_transaction under the ckch_lock (read only). * cafile_transaction under the ckch_lock (read only).
*/ */
static int cli_parse_show_cafile(char **args, char *payload, struct appctx *appctx, void *private) static int cli_parse_show_cafile(char **args, char *payload, struct appctx *appctx, void *private)
{ {
struct show_cafile_ctx *ctx = applet_reserve_svcctx(appctx, sizeof(*ctx));
struct cafile_entry *cafile_entry; struct cafile_entry *cafile_entry;
int ca_index = 0; int ca_index = 0;
char *colons; char *colons;
@ -3023,8 +3033,8 @@ static int cli_parse_show_cafile(char **args, char *payload, struct appctx *appc
if (HA_SPIN_TRYLOCK(CKCH_LOCK, &ckch_lock)) if (HA_SPIN_TRYLOCK(CKCH_LOCK, &ckch_lock))
return cli_err(appctx, "Can't show!\nOperations on certificates are currently locked!\n"); return cli_err(appctx, "Can't show!\nOperations on certificates are currently locked!\n");
appctx->ctx.ssl.show_all = 1; /* show all certificates */ ctx->show_all = 1; /* show all certificates */
appctx->ctx.ssl.index = 0; ctx->ca_index = 0;
/* check if there is a certificate to lookup */ /* check if there is a certificate to lookup */
if (*args[3]) { if (*args[3]) {
@ -3040,8 +3050,8 @@ static int cli_parse_show_cafile(char **args, char *payload, struct appctx *appc
goto error; goto error;
} }
*colons = '\0'; *colons = '\0';
appctx->ctx.ssl.index = ca_index - 1; /* we start counting at 0 in the ca_store, but at 1 on the CLI */ ctx->ca_index = ca_index - 1; /* we start counting at 0 in the ca_store, but at 1 on the CLI */
appctx->ctx.ssl.show_all = 0; /* show only one certificate */ ctx->show_all = 0; /* show only one certificate */
} }
if (*args[3] == '*') { if (*args[3] == '*') {
@ -3060,7 +3070,7 @@ static int cli_parse_show_cafile(char **args, char *payload, struct appctx *appc
goto error; goto error;
} }
appctx->ctx.ssl.cur_cafile_entry = cafile_entry; ctx->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;
} }
@ -3098,11 +3108,12 @@ 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 * It uses a show_cafile_ctx and the global cafile_transaction.new_cafile_entry
* cafile_transaction.new_cafile_entry in read-only. * in read-only.
*/ */
static int cli_io_handler_show_cafile(struct appctx *appctx) static int cli_io_handler_show_cafile(struct appctx *appctx)
{ {
struct show_cafile_ctx *ctx = appctx->svcctx;
struct buffer *trash = alloc_trash_chunk(); struct buffer *trash = alloc_trash_chunk();
struct ebmb_node *node; struct ebmb_node *node;
struct conn_stream *cs = appctx->owner; struct conn_stream *cs = appctx->owner;
@ -3111,7 +3122,7 @@ static int cli_io_handler_show_cafile(struct appctx *appctx)
if (trash == NULL) if (trash == NULL)
return 1; return 1;
if (!appctx->ctx.ssl.old_cafile_entry) { if (!ctx->old_cafile_entry) {
if (cafile_transaction.old_cafile_entry) { if (cafile_transaction.old_cafile_entry) {
chunk_appendf(trash, "# transaction\n"); chunk_appendf(trash, "# transaction\n");
chunk_appendf(trash, "*%s", cafile_transaction.old_cafile_entry->path); chunk_appendf(trash, "*%s", cafile_transaction.old_cafile_entry->path);
@ -3121,12 +3132,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.ssl.cur_cafile_entry) { if (!ctx->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 = &appctx->ctx.ssl.cur_cafile_entry->node; node = &ctx->cur_cafile_entry->node;
} }
while (node) { while (node) {
@ -3144,13 +3155,13 @@ static int cli_io_handler_show_cafile(struct appctx *appctx)
} }
} }
appctx->ctx.ssl.cur_cafile_entry = NULL; ctx->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.ssl.cur_cafile_entry = cafile_entry; ctx->cur_cafile_entry = cafile_entry;
return 0; /* should come back */ return 0; /* should come back */
} }