BUG/MEDIUM: ssl/cli: fix yielding in show_cafile_detail
HAProxy crashes when "show ssl ca-file" is being called on a ca-file
which contains a lot of certificates. (127 in our test with
/etc/ssl/certs/ca-certificates.crt).
The root cause is the fonction does not yield when there is no available
space when writing the details, and we could write a lot.
To fix the issue, we try to put the data with ci_putchk() after every
show_cert_detail() and we yield if the ci_putchk() fails.
This patch also cleans up a little bit the code:
- the end label is now a real end with a return 1;
- i0 is used instead of (long)p1
- the ID is stored upon yield
diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 447fdbb..aff35eb 100644
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -2928,11 +2928,12 @@
struct conn_stream *cs = appctx->owner;
struct cafile_entry *cafile_entry = appctx->ctx.cli.p0;
struct buffer *out = alloc_trash_chunk();
- int i;
+ int i = 0;
X509 *cert;
STACK_OF(X509_OBJECT) *objs;
int retval = 0;
- long ca_index = (long)appctx->ctx.cli.p1;
+ int ca_index = appctx->ctx.cli.i0;
+ int show_all = appctx->ctx.cli.i1;
if (!out)
goto end_no_putchk;
@@ -2954,33 +2955,39 @@
goto end;
objs = X509_STORE_get0_objects(cafile_entry->ca_store);
- for (i = 0; i < sk_X509_OBJECT_num(objs); i++) {
+ for (i = ca_index; i < sk_X509_OBJECT_num(objs); i++) {
+
cert = X509_OBJECT_get0_X509(sk_X509_OBJECT_value(objs, i));
if (!cert)
continue;
- /* Certificate indexes start at 1 on the CLI output. */
- if (ca_index && ca_index-1 != i)
- continue;
-
+ /* file starts at line 1 */
chunk_appendf(out, " \nCertificate #%d:\n", i+1);
retval = show_cert_detail(cert, NULL, out);
if (retval < 0)
goto end_no_putchk;
- else if (retval || ca_index)
+ else if (retval)
+ goto yield;
+
+ if (ci_putchk(cs_ic(cs), out) == -1) {
+ cs_rx_room_blk(cs);
+ goto yield;
+ }
+
+ if (!show_all) /* only need to dump one certificate */
goto end;
}
end:
- if (ci_putchk(cs_ic(cs), out) == -1) {
- cs_rx_room_blk(cs);
- goto yield;
- }
+ free_trash_chunk(out);
+ return 1; /* end, don't come back */
end_no_putchk:
free_trash_chunk(out);
return 1;
yield:
+ /* save the current state */
+ appctx->ctx.cli.i0 = i;
free_trash_chunk(out);
return 0; /* should come back */
}
@@ -2990,7 +2997,7 @@
static int cli_parse_show_cafile(char **args, char *payload, struct appctx *appctx, void *private)
{
struct cafile_entry *cafile_entry;
- long ca_index = 0;
+ int ca_index = 0;
char *colons;
char *err = NULL;
@@ -3002,6 +3009,8 @@
if (HA_SPIN_TRYLOCK(CKCH_LOCK, &ckch_lock))
return cli_err(appctx, "Can't show!\nOperations on certificates are currently locked!\n");
+ appctx->ctx.cli.i1 = 1; /* show all certificates */
+ appctx->ctx.cli.i0 = 0;
/* check if there is a certificate to lookup */
if (*args[3]) {
@@ -3017,6 +3026,8 @@
goto error;
}
*colons = '\0';
+ appctx->ctx.cli.i0 = ca_index - 1; /* we start counting at 0 in the ca_store, but at 1 on the CLI */
+ appctx->ctx.cli.i1 = 0; /* show only one certificate */
}
if (*args[3] == '*') {
@@ -3036,7 +3047,6 @@
}
appctx->ctx.cli.p0 = cafile_entry;
- appctx->ctx.cli.p1 = (void*)ca_index;
/* use the IO handler that shows details */
appctx->io_handler = cli_io_handler_show_cafile_detail;
}