]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: ssl/cli: fix yielding in show_cafile_detail
authorWilliam Lallemand <wlallemand@haproxy.org>
Tue, 26 Apr 2022 16:17:15 +0000 (18:17 +0200)
committerWilliam Lallemand <wlallemand@haproxy.org>
Tue, 26 Apr 2022 17:35:43 +0000 (19:35 +0200)
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

src/ssl_ckch.c

index 447fdbb5685ca44d29ad5b7e412ddda21dc6ef49..aff35ebcd79fe736e852954b06f137c941c80f27 100644 (file)
@@ -2928,11 +2928,12 @@ static int cli_io_handler_show_cafile_detail(struct appctx *appctx)
        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 @@ static int cli_io_handler_show_cafile_detail(struct appctx *appctx)
                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 @@ yield:
 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 @@ static int cli_parse_show_cafile(char **args, char *payload, struct appctx *appc
        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 @@ static int cli_parse_show_cafile(char **args, char *payload, struct appctx *appc
                                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 @@ static int cli_parse_show_cafile(char **args, char *payload, struct appctx *appc
                }
 
                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;
        }