fix: report file path for invalid cert files

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_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_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_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
 ) ->