Merge pull request #13733 from zmstone/0827-report-file-path-in-error-message

cacertfile should be optional for dashboard https listener

apps/emqx/src/emqx_tls_lib.erl

@@ -353,7 +353,7 @@ ensure_ssl_files_per_key(Dir, SSL, [KeyPath | KeyPaths], Opts) ->
         {ok, NewSSL} ->
             ensure_ssl_files_per_key(Dir, NewSSL, KeyPaths, Opts);
         {error, Reason} ->
-            {error, Reason#{which_options => [KeyPath]}}
+            {error, Reason#{which_option => format_key_path(KeyPath)}}
     end.
 
 ensure_ssl_file(_Dir, _KeyPath, SSL, undefined, _Opts) ->
@@ -392,13 +392,10 @@ do_ensure_ssl_file(Dir, RawDir, KeyPath, SSL, MaybePem, DryRun) ->
             case is_valid_pem_file(MaybePem) of
                 true ->
                     {ok, SSL};
-                {error, enoent} when DryRun ->
+                {error, #{pem_check := enoent}} when DryRun ->
                     {ok, SSL};
                 {error, Reason} ->
-                    {error, #{
-                        pem_check => invalid_pem,
-                        file_read => Reason
-                    }}
+                    {error, Reason}
             end
     end.
 
@@ -476,11 +473,29 @@ is_hex_str(Str) ->
 %% @doc Returns 'true' when the file is a valid pem, otherwise {error, Reason}.
 is_valid_pem_file(Path0) ->
     Path = resolve_cert_path_for_read(Path0),
-    case file:read_file(Path) of
-        {ok, Pem} -> is_pem(Pem) orelse {error, not_pem};
-        {error, Reason} -> {error, Reason}
+    case is_valid_filename(Path) of
+        true ->
+            case file:read_file(Path) of
+                {ok, Pem} ->
+                    case is_pem(Pem) of
+                        true ->
+                            true;
+                        false ->
+                            {error, #{pem_check => not_pem, file_path => Path}}
+                    end;
+                {error, Reason} ->
+                    {error, #{pem_check => Reason, file_path => Path}}
+            end;
+        false ->
+            %% do not report path because the content can be huge
+            {error, #{pem_check => not_pem, file_path => not_file_path}}
     end.
 
+%% no controle chars 0-31
+%% the input is always string for this function
+is_valid_filename(Path) ->
+    lists:all(fun(C) -> C >= 32 end, Path).
+
 %% @doc Input and output are both HOCON-checked maps, with invalid SSL
 %% file options dropped.
 %% This is to give a feedback to the front-end or management API caller
@@ -711,10 +726,21 @@ ensure_ssl_file_key(SSL, RequiredKeyPaths) ->
         end
     end,
     case lists:filter(Filter, RequiredKeyPaths) of
-        [] -> ok;
-        Miss -> {error, #{reason => ssl_file_option_not_found, which_options => Miss}}
+        [] ->
+            ok;
+        MissingL ->
+            {error, #{
+                reason => ssl_file_option_not_found,
+                missing_options => format_key_paths(MissingL)
+            }}
     end.
 
+format_key_paths(Paths) ->
+    lists:map(fun format_key_path/1, Paths).
+
+format_key_path(Path) ->
+    iolist_to_binary(lists:join(".", [ensure_bin(S) || S <- Path])).
+
 -spec maybe_inject_ssl_fun(root_fun | verify_fun, map()) -> map().
 maybe_inject_ssl_fun(FunName, SslOpts) ->
     case persistent_term:get(?EMQX_SSL_FUN_MFA(FunName), undefined) of

apps/emqx/test/emqx_listeners_update_SUITE.erl

@@ -337,7 +337,7 @@ t_update_empty_ssl_options_conf(_Conf) ->
         {error,
             {bad_ssl_config, #{
                 reason := pem_file_path_or_string_is_required,
-                which_options := [[<<"keyfile">>]]
+                which_option := <<"keyfile">>
             }}},
         emqx:update_config(?LISTENERS, BadRaw)
     ),

apps/emqx/test/emqx_ocsp_cache_SUITE.erl

@@ -916,7 +916,7 @@ do_t_validations(_Config) ->
         emqx_utils_json:decode(ResRaw3, [return_maps]),
     %% we can't remove certfile now, because it has default value.
     ?assertMatch({match, _}, re:run(MsgRaw3, <<"enoent">>)),
-    ?assertMatch({match, _}, re:run(MsgRaw3, <<"invalid_pem">>)),
+    ?assertMatch({match, _}, re:run(MsgRaw3, <<"ocsp\\.issuer_pem">>)),
     ok.
 
 t_unknown_error_fetching_ocsp_response(_Config) ->

apps/emqx/test/emqx_tls_lib_tests.erl

@@ -100,12 +100,40 @@ ssl_files_failure_test_() ->
                 emqx_tls_lib:ensure_ssl_files_in_mutable_certs_dir("dir", Disabled)
             )
         end},
+        {"mandatory_keys", fun() ->
+            ?assertMatch(
+                {error, #{missing_options := [<<"keyfile">>]}},
+                emqx_tls_lib:ensure_ssl_files_in_mutable_certs_dir("dir", #{}, #{
+                    required_keys => [[<<"keyfile">>]]
+                })
+            ),
+            ?assertMatch(
+                {error, #{missing_options := [<<"keyfile">>]}},
+                emqx_tls_lib:ensure_ssl_files_in_mutable_certs_dir("dir", #{}, #{
+                    required_keys => [[keyfile]]
+                })
+            )
+        end},
+        {"invalid_file_path", fun() ->
+            ?assertMatch(
+                {error, #{
+                    pem_check := not_pem,
+                    file_path := not_file_path,
+                    which_option := <<"keyfile">>
+                }},
+                emqx_tls_lib:ensure_ssl_files_in_mutable_certs_dir("/tmp", #{
+                    keyfile => <<"abc", $\n, "123">>,
+                    certfile => test_key(),
+                    cacertfile => test_key()
+                })
+            )
+        end},
         {"enoent_key_file", fun() ->
             NonExistingFile = filename:join(
                 "/tmp", integer_to_list(erlang:system_time(microsecond))
             ),
             ?assertMatch(
-                {error, #{file_read := enoent, pem_check := invalid_pem}},
+                {error, #{pem_check := enoent, file_path := _}},
                 emqx_tls_lib:ensure_ssl_files_in_mutable_certs_dir("/tmp", #{
                     <<"keyfile">> => NonExistingFile,
                     <<"certfile">> => test_key(),
@@ -128,7 +156,7 @@ ssl_files_failure_test_() ->
             ?assertMatch(
                 {error, #{
                     reason := pem_file_path_or_string_is_required,
-                    which_options := [[<<"keyfile">>]]
+                    which_option := <<"keyfile">>
                 }},
                 emqx_tls_lib:ensure_ssl_files_in_mutable_certs_dir("/tmp", #{
                     <<"keyfile">> => <<>>,
@@ -139,7 +167,7 @@ ssl_files_failure_test_() ->
             %% not valid unicode
             ?assertMatch(
                 {error, #{
-                    reason := invalid_file_path_or_pem_string, which_options := [[<<"keyfile">>]]
+                    reason := invalid_file_path_or_pem_string, which_option := <<"keyfile">>
                 }},
                 emqx_tls_lib:ensure_ssl_files_in_mutable_certs_dir("/tmp", #{
                     <<"keyfile">> => <<255, 255>>,
@@ -150,7 +178,7 @@ ssl_files_failure_test_() ->
             ?assertMatch(
                 {error, #{
                     reason := invalid_file_path_or_pem_string,
-                    which_options := [[<<"ocsp">>, <<"issuer_pem">>]]
+                    which_option := <<"ocsp.issuer_pem">>
                 }},
                 emqx_tls_lib:ensure_ssl_files_in_mutable_certs_dir("/tmp", #{
                     <<"keyfile">> => test_key(),
@@ -172,7 +200,7 @@ ssl_files_failure_test_() ->
             try
                 ok = file:write_file(TmpFile, <<"not a valid pem">>),
                 ?assertMatch(
-                    {error, #{file_read := not_pem}},
+                    {error, #{pem_check := not_pem}},
                     emqx_tls_lib:ensure_ssl_files_in_mutable_certs_dir(
                         "/tmp",
                         #{

apps/emqx_auth/src/emqx_auth.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_auth, [
     {description, "EMQX Authentication and authorization"},
-    {vsn, "0.4.0"},
+    {vsn, "0.4.1"},
     {modules, []},
     {registered, [emqx_auth_sup]},
     {applications, [

apps/emqx_auth/src/emqx_authn/emqx_authn_api.erl

@@ -1296,7 +1296,7 @@ serialize_error(unsupported_operation) ->
 serialize_error({bad_ssl_config, Details}) ->
     {400, #{
         code => <<"BAD_REQUEST">>,
-        message => binfmt("bad_ssl_config ~p", [Details])
+        message => binfmt("bad_ssl_config ~0p", [Details])
     }};
 serialize_error({missing_parameter, Detail}) ->
     {400, #{

apps/emqx_connector/src/emqx_connector_ssl.erl

@@ -49,22 +49,26 @@ new_ssl_config(#{<<"ssl">> := _} = Config, NewSSL) ->
 new_ssl_config(Config, _NewSSL) ->
     Config.
 
-map_bad_ssl_error(#{pem_check := invalid_pem} = TLSLibError) ->
-    #{which_options := Paths, file_read := Reason} = TLSLibError,
+map_bad_ssl_error(#{
+    pem_check := NotPem,
+    file_path := FilePath,
+    which_option := Field
+}) ->
     #{
         kind => validation_error,
         reason => <<"bad_ssl_config">>,
-        bad_fields => Paths,
+        bad_field => Field,
+        file_path => FilePath,
         details => emqx_utils:format(
             "Failed to access certificate / key file: ~s",
-            [emqx_utils:explain_posix(Reason)]
+            [emqx_utils:explain_posix(NotPem)]
         )
     };
-map_bad_ssl_error(#{which_options := Paths, reason := Reason}) ->
+map_bad_ssl_error(#{which_option := Field, reason := Reason}) ->
     #{
         kind => validation_error,
         reason => <<"bad_ssl_config">>,
-        bad_fields => Paths,
+        bad_field => Field,
         details => Reason
     };
 map_bad_ssl_error(TLSLibError) ->

apps/emqx_connector/test/emqx_connector_api_SUITE.erl

@@ -742,7 +742,7 @@ t_create_with_bad_tls_files(Config) ->
                     <<"reason">> := <<"bad_ssl_config">>,
                     <<"details">> :=
                         <<"Failed to access certificate / key file: No such file or directory">>,
-                    <<"bad_fields">> := [[<<"cacertfile">>]]
+                    <<"bad_field">> := <<"cacertfile">>
                 },
                 json(Msg0)
             ),

apps/emqx_dashboard/src/emqx_dashboard.app.src

@@ -2,7 +2,7 @@
 {application, emqx_dashboard, [
     {description, "EMQX Web Dashboard"},
     % strict semver, bump manually!
-    {vsn, "5.1.4"},
+    {vsn, "5.1.5"},
     {modules, []},
     {registered, [emqx_dashboard_sup]},
     {applications, [

apps/emqx_dashboard/src/emqx_dashboard_listener.erl

@@ -188,7 +188,7 @@ ensure_ssl_cert(#{<<"listeners">> := #{<<"https">> := #{<<"bind">> := Bind} = Ht
     Https1 = emqx_dashboard_schema:https_converter(Https0, #{}),
     Conf1 = emqx_utils_maps:deep_put([<<"listeners">>, <<"https">>], Conf0, Https1),
     Ssl = maps:get(<<"ssl_options">>, Https1, undefined),
-    Opts = #{required_keys => [[<<"keyfile">>], [<<"certfile">>], [<<"cacertfile">>]]},
+    Opts = #{required_keys => [[<<"keyfile">>], [<<"certfile">>]]},
     case emqx_tls_lib:ensure_ssl_files_in_mutable_certs_dir(?DIR, Ssl, Opts) of
         {ok, undefined} ->
             {error, <<"ssl_cert_not_found">>};

apps/emqx_gateway/src/emqx_gateway_http.erl

@@ -490,13 +490,8 @@ reason2msg(
         "The authentication already exist on ~s",
         [listener_id(GwName, LType, LName)]
     );
-reason2msg(
-    {bad_ssl_config, #{
-        reason := Reason,
-        which_options := Options
-    }}
-) ->
-    fmtstr("Bad TLS configuration for ~p, reason: ~s", [Options, Reason]);
+reason2msg({bad_ssl_config, Reason}) ->
+    fmtstr("Bad TLS configuration: ~0p", [Reason]);
 reason2msg(
     {#{roots := [{gateway, _}]}, [_ | _]} = Error
 ) ->

changes/ce/fix-13733.en.md

@@ -0,0 +1 @@
+Made `cacertfile` optional when configuring https listener from `emqx ctl conf load` command.

scripts/test/start-two-nodes-in-docker.sh

@@ -10,7 +10,7 @@ set -euo pipefail
 # ensure dir
 cd -P -- "$(dirname -- "$0")/../../"
 
-HAPROXY_PORTS=(-p 18083:18083 -p 8883:8883 -p 8084:8084)
+HAPROXY_PORTS=(-p 18083:18083 -p 1883:1883 -p 8883:8883 -p 8084:8084)
 
 NET='emqx.io'
 NODE1="node1.$NET"
@@ -50,7 +50,7 @@ do
     case $opt in
         # -P option is treated similarly to docker run -P:
         # publish ports to random available host ports
-        P) HAPROXY_PORTS=(-p 18083 -p 8883 -p 8084);;
+        P) HAPROXY_PORTS=(-p 18083 -p 1883 -p 8883 -p 8084);;
         c) cleanup; exit 0;;
         h) show_help; exit 0;;
         6) IPV6=1;;
@@ -129,7 +129,7 @@ cat <<EOF > tmp/haproxy.cfg
 global
     log stdout format raw daemon debug
     # Replace 1024000 with deployment connections
-    maxconn 1000
+    maxconn 102400
     nbproc 1
     nbthread 2
     cpu-map auto:1/1-2 0-1
@@ -147,7 +147,7 @@ defaults
     mode tcp
     option tcplog
     # Replace 1024000 with deployment connections
-    maxconn 1000
+    maxconn 102400
     timeout connect 30000
     timeout client 600s
     timeout server 600s
@@ -171,27 +171,33 @@ backend emqx_dashboard_back
     ${DASHBOARD_BACKEND2}
 
 ##----------------------------------------------------------------
-## TLS
+## MQTT Listeners
 ##----------------------------------------------------------------
+frontend emqx_tcp
+    mode tcp
+    option tcplog
+    bind *:1883
+    default_backend emqx_backend_tcp
+
 frontend emqx_ssl
     mode tcp
     option tcplog
     bind *:8883 ssl crt /tmp/emqx.pem ca-file /usr/local/etc/haproxy/certs/cacert.pem verify required no-sslv3
-    default_backend emqx_ssl_back
+    default_backend emqx_backend_tcp
 
 frontend emqx_wss
     mode tcp
     option tcplog
     bind *:8084 ssl crt /tmp/emqx.pem ca-file /usr/local/etc/haproxy/certs/cacert.pem verify required no-sslv3
-    default_backend emqx_wss_back
+    default_backend emqx_backend_ws
 
-backend emqx_ssl_back
+backend emqx_backend_tcp
     mode tcp
     balance static-rr
     server emqx-1 $NODE1:1883 check-send-proxy send-proxy-v2-ssl-cn
     server emqx-2 $NODE2:1883 check-send-proxy send-proxy-v2-ssl-cn
 
-backend emqx_wss_back
+backend emqx_backend_ws
     mode tcp
     balance static-rr
     server emqx-1 $NODE1:8083 check-send-proxy send-proxy-v2-ssl-cn