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.

diff --git a/include/haproxy/applet-t.h b/include/haproxy/applet-t.h
index 15a8736aa447bb4132bc52cbfefd797e1d2bd725..ea730c7b7aa7218f20da331ecc27ac7d82b2c81f 100644 (file)
--- a/include/haproxy/applet-t.h
+++ b/include/haproxy/applet-t.h
@@ -179,6 +179,7 @@ struct appctx {
                        struct ckch_store *old_ckchs;
                        struct ckch_store *new_ckchs;
                        struct ckch_inst *next_ckchi;
+                       struct ckch_store *cur_ckchs;
 
                        struct ckch_inst_link *next_ckchi_link;
                        struct cafile_entry *old_cafile_entry;

diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index cb0ed5f0eef3bb162ec2a17044cc9afa2bd90bc2..1e4909c8cc0a50896710eddd96fb0def7fd891d6 100644 (file)
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -1231,7 +1231,9 @@ static void cli_release_show_cert(struct appctx *appctx)
        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)
 {
        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");
                node = ebmb_first(&ckchs_tree);
        } else {
-               node = &((struct ckch_store *)appctx->ctx.cli.p0)->node;
+               node = &((struct ckch_store *)appctx->ctx.ssl.cur_ckchs)->node;
        }
        while (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);
        return 1;
 yield:
 
        free_trash_chunk(trash);
-       appctx->ctx.cli.p0 = ckchs;
+       appctx->ctx.ssl.cur_ckchs = ckchs;
        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)
 {
        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();
        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)
 {
 #if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP) && !defined OPENSSL_IS_BORINGSSL)
        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();
-       int from_transaction = appctx->ctx.cli.i0;
+       int from_transaction = appctx->ctx.ssl.index;
 
        if (!out)
                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 */
                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;
                }
                else