BUG/MINOR: ssl/cli: fix frontend-not-found detection in 'show ssl sni -f'

Coverity (CID 1664029, GH #3466) points out that `px == NULL` after the
list_for_each_entry loop in cli_parse_show_sni() can never be true:
list_for_each_entry's loop variable, when the list is exhausted without
a break, is a container_of-derived pointer computed from the list
head's address, never NULL. This left the "-f <bogus-frontend>" case
silently falling through instead of erroring: the bug pre-dates
9993f29007, which fixed the found case (adding the missing break) but
kept comparing the wrong thing for the not-found case. Track "found"
explicitly instead.

Add reg-tests/ssl/show_ssl_sni_filter.vtc to cover the -f option,
including the not-found error case.

This must be backported to 3.2.

diff --git a/reg-tests/ssl/show_ssl_sni_filter.vtc b/reg-tests/ssl/show_ssl_sni_filter.vtc
new file mode 100644 (file)
index 0000000..b1d9342
--- /dev/null
+++ b/reg-tests/ssl/show_ssl_sni_filter.vtc
@@ -0,0 +1,76 @@
+#REGTEST_TYPE=bug
+
+# This reg-test checks the "-f <frontend>" filter of the "show ssl sni" CLI
+# command:
+#  - with no filter, every frontend's SNI entries are listed
+#  - "-f <existing frontend>" restricts the output to that frontend only
+#  - "-f <unknown frontend>" must report an error
+#
+# See GitHub issue #3466 (CID 1660133 / CID 1664029): the "not found" case
+# was never actually detected (the loop variable used for the check can't be
+# NULL once the search loop over "main_proxies" completes, matched or not),
+# so an unknown "-f" name silently fell back to displaying the first proxy
+# in the list instead of returning "Couldn't find the specified frontend!".
+
+varnishtest "Test the 'show ssl sni -f <frontend>' CLI option"
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(3.2-dev2)'"
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+    .if !ssllib_name_startswith(AWS-LC)
+        tune.ssl.default-dh-param 2048
+    .endif
+        stats socket "${tmpdir}/h1/stats" level admin
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    # frontend's default SNI entry comes from the certificate's CN/SAN:
+    # www.test1.com
+    frontend fe1
+        bind "${tmpdir}/fe1.sock" ssl crt ${testdir}/certs/common.pem
+
+    # frontend's default SNI entry comes from the certificate's CN/SAN:
+    # localhost
+    frontend fe2
+        bind "${tmpdir}/fe2.sock" ssl crt ${testdir}/certs/ecdsa.pem
+} -start
+
+# no filter: both frontends must be listed
+haproxy h1 -cli {
+    send "show ssl sni"
+    expect ~ ".*www.test1.com"
+    send "show ssl sni"
+    expect ~ ".*localhost"
+}
+
+# "-f fe1": only fe1's entry must be listed
+haproxy h1 -cli {
+    send "show ssl sni -f fe1"
+    expect ~ ".*www.test1.com"
+    send "show ssl sni -f fe1"
+    expect !~ ".*localhost"
+}
+
+# "-f fe2": only fe2's entry must be listed
+haproxy h1 -cli {
+    send "show ssl sni -f fe2"
+    expect ~ ".*localhost"
+    send "show ssl sni -f fe2"
+    expect !~ ".*www.test1.com"
+}
+
+# "-f <unknown frontend>" must report an error instead of silently falling
+# back to the first frontend of the list (GH #3466 / CID 1664029)
+haproxy h1 -cli {
+    send "show ssl sni -f does-not-exist"
+    expect ~ "Couldn't find the specified frontend!"
+}

diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 3c4c9d8e7f39e9b42b1577f3d38a24743b0a9d43..a4928e1086cc2d3c40fbdccc79fcf65fc6dd6071 100644 (file)
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -1857,6 +1857,7 @@ static int cli_parse_show_sni(char **args, char *payload, struct appctx *appctx,
 
        while (*args[cur_arg]) {
                struct proxy *px;
+               int found = 0;
 
                if (strcmp(args[cur_arg], "-f") == 0) {
 
@@ -1876,11 +1877,12 @@ static int cli_parse_show_sni(char **args, char *payload, struct appctx *appctx,
                                if (strcmp(px->id, args[cur_arg+1]) == 0) {
                                        ctx->px = px;
                                        ctx->options |= SHOW_SNI_OPT_1FRONTEND;
+                                       found = 1;
                                        break;
                                }
                        }
                        cur_arg++; /* skip the argument */
-                       if (px == NULL)
+                       if (!found)
                                return cli_err(appctx, "Couldn't find the specified frontend!\n");
 
                } else if (strcmp(args[cur_arg], "-A") == 0) {