From 19051f639bf31e60f859359b21e708ccb8d3f443 Mon Sep 17 00:00:00 2001
From: "Zaiming (Stone) Shi" <zmstone@gmail.com>
Date: Tue, 12 Dec 2023 23:36:54 +0100
Subject: [PATCH] test: ensure verify_none to ssl client opts as default value

---
 apps/emqx/test/emqx_crl_cache_SUITE.erl       |  3 +-
 .../test/emqx_dashboard_SUITE.erl             |  8 +++-
 .../test/emqx_dashboard_https_SUITE.erl       | 38 ++++++++++++++-----
 .../src/emqx_otel_config.erl                  |  4 +-
 4 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/apps/emqx/test/emqx_crl_cache_SUITE.erl b/apps/emqx/test/emqx_crl_cache_SUITE.erl
index cf317eadb..36883b182 100644
--- a/apps/emqx/test/emqx_crl_cache_SUITE.erl
+++ b/apps/emqx/test/emqx_crl_cache_SUITE.erl
@@ -1008,7 +1008,8 @@ do_t_update_listener(Config) ->
         {ssl, true},
         {ssl_opts, [
             {certfile, ClientCert},
-            {keyfile, ClientKey}
+            {keyfile, ClientKey},
+            {verify, verify_none}
         ]},
         {port, 8883}
     ]),
diff --git a/apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl b/apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl
index 7f0dd02de..a11b537d1 100644
--- a/apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl
+++ b/apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl
@@ -261,9 +261,10 @@ request_dashboard(Method, Url, Auth) ->
 request_dashboard(Method, Url, QueryParams, Auth) ->
     Request = {Url ++ "?" ++ QueryParams, [Auth]},
     do_request_dashboard(Method, Request).
-do_request_dashboard(Method, Request) ->
+
+do_request_dashboard(Method, {Url, _} = Request) ->
     ct:pal("Method: ~p, Request: ~p", [Method, Request]),
-    case httpc:request(Method, Request, [], []) of
+    case httpc:request(Method, Request, maybe_ssl(Url), []) of
         {error, socket_closed_remotely} ->
             {error, socket_closed_remotely};
         {ok, {{"HTTP/1.1", Code, _}, _Headers, Return}} when
@@ -276,6 +277,9 @@ do_request_dashboard(Method, Request) ->
             {error, Reason}
     end.
 
+maybe_ssl("http://" ++ _) -> [];
+maybe_ssl("https://" ++ _) -> [{ssl, [{verify, verify_none}]}].
+
 auth_header_() ->
     auth_header_(<<"admin">>, <<"public">>).
 
diff --git a/apps/emqx_dashboard/test/emqx_dashboard_https_SUITE.erl b/apps/emqx_dashboard/test/emqx_dashboard_https_SUITE.erl
index a09f8fa3b..c46c0ddbd 100644
--- a/apps/emqx_dashboard/test/emqx_dashboard_https_SUITE.erl
+++ b/apps/emqx_dashboard/test/emqx_dashboard_https_SUITE.erl
@@ -198,8 +198,25 @@ t_verify_cacertfile(_Config) ->
         VerifyPeerConf1,
         naive_env_interpolation(<<"${EMQX_ETC_DIR}/certs/cacert.pem">>)
     ),
-    validate_https(VerifyPeerConf2, MaxConnection, DefaultSSLCert, verify_peer),
-    ok.
+    %% we always test client with verify_none and no client cert is sent
+    %% since the server is configured with verify_peer
+    %% hence the expected observation on the client side is an error
+    ErrorReason =
+        try
+            validate_https(VerifyPeerConf2, MaxConnection, DefaultSSLCert, verify_peer)
+        catch
+            error:{https_client_error, Reason} ->
+                Reason
+        end,
+    %% There seems to be a race-condition causing the return value to vary a bit
+    case ErrorReason of
+        socket_closed_remotely ->
+            ok;
+        {ssl_error, _SslSock, {tls_alert, {certificate_required, _}}} ->
+            ok;
+        Other ->
+            throw({unexpected, Other})
+    end.
 
 t_bad_certfile(_Config) ->
     Conf = #{
@@ -219,9 +236,12 @@ t_bad_certfile(_Config) ->
 validate_https(Conf, MaxConnection, SSLCert, Verify) ->
     emqx_common_test_helpers:load_config(emqx_dashboard_schema, Conf),
     emqx_mgmt_api_test_util:init_suite([emqx_management], fun(X) -> X end),
-    assert_ranch_options(MaxConnection, SSLCert, Verify),
-    assert_https_request(),
-    emqx_mgmt_api_test_util:end_suite([emqx_management]).
+    try
+        assert_ranch_options(MaxConnection, SSLCert, Verify),
+        assert_https_request()
+    after
+        emqx_mgmt_api_test_util:end_suite([emqx_management])
+    end.
 
 assert_ranch_options(MaxConnections0, SSLCert, Verify) ->
     Middlewares = [emqx_dashboard_middleware, cowboy_router, cowboy_handler],
@@ -286,10 +306,10 @@ assert_https_request() ->
     lists:foreach(
         fun(Path) ->
             ApiPath = https_api_path([Path]),
-            ?assertMatch(
-                {ok, _},
-                emqx_dashboard_SUITE:request_dashboard(get, ApiPath, Headers)
-            )
+            case emqx_dashboard_SUITE:request_dashboard(get, ApiPath, Headers) of
+                {ok, _} -> ok;
+                {error, Reason} -> error({https_client_error, Reason})
+            end
         end,
         ?OVERVIEWS
     ).
diff --git a/apps/emqx_opentelemetry/src/emqx_otel_config.erl b/apps/emqx_opentelemetry/src/emqx_otel_config.erl
index c16b3385a..72b15aa04 100644
--- a/apps/emqx_opentelemetry/src/emqx_otel_config.erl
+++ b/apps/emqx_opentelemetry/src/emqx_otel_config.erl
@@ -155,7 +155,7 @@ ssl_opts(Endpoint, SSLOpts) ->
             []
     end.
 
-is_ssl(<<"https://", _/binary>> = _Endpoint) ->
+is_ssl(<<"https://", _/binary>>) ->
     true;
-is_ssl(_Endpoint) ->
+is_ssl(<<"http://", _/binary>>) ->
     false.