Merge pull request #5989 from zmstone/refactor-ssl-certs-lib

refactor(tls): move ssl files handling to emqx_tls_lib

apps/emqx/src/emqx.erl

@@ -65,6 +65,8 @@
         , remove_config/1
         , remove_config/2
         , reset_config/2
+        , data_dir/0
+        , certs_dir/0
         ]).
 
 -define(APP, ?MODULE).
@@ -246,3 +248,9 @@ reset_config([RootName | _] = KeyPath, Opts) ->
         {error, _} = Error ->
             Error
     end.
+
+data_dir() ->
+    application:get_env(emqx, data_dir, "data").
+
+certs_dir() ->
+    filename:join([data_dir(), certs]).

apps/emqx/src/emqx_authentication_config.erl

@@ -27,9 +27,8 @@
         , authn_type/1
         ]).
 
-%% TODO: certs handling should be moved out of emqx app
 -ifdef(TEST).
--export([convert_certs/2, convert_certs/3, diff_cert/2, clear_certs/2]).
+-export([convert_certs/2, convert_certs/3, clear_certs/2]).
 -endif.
 
 -export_type([config/0]).
@@ -57,30 +56,25 @@
 -spec pre_config_update(update_request(), emqx_config:raw_config())
     -> {ok, map() | list()} | {error, term()}.
 pre_config_update(UpdateReq, OldConfig) ->
-    case do_pre_config_update(UpdateReq, to_list(OldConfig)) of
+    try do_pre_config_update(UpdateReq, to_list(OldConfig)) of
         {error, Reason} -> {error, Reason};
         {ok, NewConfig} -> {ok, return_map(NewConfig)}
+    catch
+        throw : Reason ->
+            {error, Reason}
     end.
 
 do_pre_config_update({create_authenticator, ChainName, Config}, OldConfig) ->
-    try
+    CertsDir = certs_dir(ChainName, Config),
-        CertsDir = certs_dir([to_bin(ChainName), authenticator_id(Config)]),
     NConfig = convert_certs(CertsDir, Config),
-        {ok, OldConfig ++ [NConfig]}
+    {ok, OldConfig ++ [NConfig]};
-    catch
-        error:{save_cert_to_file, _} = Reason ->
-            {error, Reason};
-        error:{missing_parameter, _} = Reason ->
-            {error, Reason}
-    end;
 do_pre_config_update({delete_authenticator, _ChainName, AuthenticatorID}, OldConfig) ->
     NewConfig = lists:filter(fun(OldConfig0) ->
                                 AuthenticatorID =/= authenticator_id(OldConfig0)
                              end, OldConfig),
     {ok, NewConfig};
 do_pre_config_update({update_authenticator, ChainName, AuthenticatorID, Config}, OldConfig) ->
-    try
+    CertsDir = certs_dir(ChainName, AuthenticatorID),
-        CertsDir = certs_dir([to_bin(ChainName), AuthenticatorID]),
     NewConfig = lists:map(
                     fun(OldConfig0) ->
                         case AuthenticatorID =:= authenticator_id(OldConfig0) of
@@ -88,13 +82,7 @@ do_pre_config_update({update_authenticator, ChainName, AuthenticatorID, Config},
                             false -> OldConfig0
                         end
                     end, OldConfig),
-        {ok, NewConfig}
+    {ok, NewConfig};
-    catch
-        error:{save_cert_to_file, _} = Reason ->
-            {error, Reason};
-        error:{missing_parameter, _} = Reason ->
-            {error, Reason}
-    end;
 do_pre_config_update({move_authenticator, _ChainName, AuthenticatorID, Position}, OldConfig) ->
     case split_by_id(AuthenticatorID, OldConfig) of
         {error, Reason} -> {error, Reason};
@@ -127,9 +115,8 @@ do_post_config_update({delete_authenticator, ChainName, AuthenticatorID}, _NewCo
     case emqx_authentication:delete_authenticator(ChainName, AuthenticatorID) of
         ok ->
             [Config] = [Config0 || Config0 <- to_list(OldConfig), AuthenticatorID == authenticator_id(Config0)],
-            CertsDir = certs_dir([to_bin(ChainName), AuthenticatorID]),
+            CertsDir = certs_dir(ChainName, AuthenticatorID),
-            clear_certs(CertsDir, Config),
+            ok = clear_certs(CertsDir, Config);
-            ok;
         {error, Reason} ->
             {error, Reason}
     end;
@@ -154,7 +141,7 @@ do_check_conifg(Config, Providers) ->
             ?SLOG(warning, #{msg => "unknown_authn_type",
                              type => Type,
                              providers => Providers}),
-            throw(unknown_authn_type);
+            throw({unknown_authn_type, Type});
         Module ->
             do_check_conifg(Type, Config, Module)
     end.
@@ -182,7 +169,7 @@ do_check_conifg(Type, Config, Module) ->
                              reason => E,
                              stacktrace => S
                             }),
-            throw(bad_authenticator_config)
+            throw({bad_authenticator_config, #{type => Type, reason => E}})
     end.
 
 return_map([L]) -> L;
@@ -193,105 +180,33 @@ to_list(M) when M =:= #{} -> [];
 to_list(M) when is_map(M) -> [M];
 to_list(L) when is_list(L) -> L.
 
-certs_dir(Dirs) when is_list(Dirs) ->
-    to_bin(filename:join([emqx:get_config([node, data_dir]), "certs", "authn"] ++ Dirs)).
-
 convert_certs(CertsDir, Config) ->
-    case maps:get(<<"ssl">>, Config, undefined) of
+    case emqx_tls_lib:ensure_ssl_files(CertsDir, maps:get(<<"ssl">>, Config, undefined)) of
-        undefined ->
+        {ok, SSL} ->
-            Config;
+            new_ssl_config(Config, SSL);
-        SSLOpts ->
+        {error, Reason} ->
-            NSSLOPts = lists:foldl(fun(K, Acc) ->
+            ?SLOG(error, Reason#{msg => bad_ssl_config}),
-                               case maps:get(K, Acc, undefined) of
+            throw({bad_ssl_config, Reason})
-                                   undefined -> Acc;
-                                   PemBin ->
-                                       CertFile = generate_filename(CertsDir, K),
-                                       ok = save_cert_to_file(CertFile, PemBin),
-                                       Acc#{K => CertFile}
-                               end
-                           end, SSLOpts, [<<"certfile">>, <<"keyfile">>, <<"cacertfile">>]),
-            Config#{<<"ssl">> => NSSLOPts}
     end.
 
 convert_certs(CertsDir, NewConfig, OldConfig) ->
-    case maps:get(<<"ssl">>, NewConfig, undefined) of
+    OldSSL = maps:get(<<"ssl">>, OldConfig, undefined),
-        undefined ->
+    NewSSL = maps:get(<<"ssl">>, NewConfig, undefined),
-            NewConfig;
+    case emqx_tls_lib:ensure_ssl_files(CertsDir, NewSSL) of
-        NewSSLOpts ->
+        {ok, NewSSL1} ->
-            OldSSLOpts = maps:get(<<"ssl">>, OldConfig, #{}),
+            ok = emqx_tls_lib:delete_ssl_files(CertsDir, NewSSL1, OldSSL),
-            Diff = diff_certs(NewSSLOpts, OldSSLOpts),
+            new_ssl_config(NewConfig, NewSSL1);
-            NSSLOpts = lists:foldl(fun({identical, K}, Acc) ->
+        {error, Reason} ->
-                                           Acc#{K => maps:get(K, OldSSLOpts)};
+            ?SLOG(error, Reason#{msg => bad_ssl_config}),
-                                      ({_, K}, Acc) ->
+            throw({bad_ssl_config, Reason})
-                                           CertFile = generate_filename(CertsDir, K),
-                                           ok = save_cert_to_file(CertFile, maps:get(K, NewSSLOpts)),
-                                           Acc#{K => CertFile}
-                                   end, NewSSLOpts, Diff),
-            NewConfig#{<<"ssl">> => NSSLOpts}
     end.
 
+new_ssl_config(Config, undefined) -> Config;
+new_ssl_config(Config, SSL) -> Config#{<<"ssl">> => SSL}.
+
 clear_certs(CertsDir, Config) ->
-    case maps:get(<<"ssl">>, Config, undefined) of
+    OldSSL = maps:get(<<"ssl">>, Config, undefined),
-        undefined ->
+    ok = emqx_tls_lib:delete_ssl_files(CertsDir, undefined, OldSSL).
-            ok;
-        SSLOpts ->
-            lists:foreach(
-                fun({_, Filename}) ->
-                    _ = file:delete(filename:join([CertsDir, Filename]))
-                end,
-                maps:to_list(maps:with([<<"certfile">>, <<"keyfile">>, <<"cacertfile">>], SSLOpts)))
-    end.
-
-save_cert_to_file(Filename, PemBin) ->
-    case public_key:pem_decode(PemBin) =/= [] of
-        true ->
-            case filelib:ensure_dir(Filename) of
-                ok ->
-                    case file:write_file(Filename, PemBin) of
-                        ok -> ok;
-                        {error, Reason} -> error({save_cert_to_file, {write_file, Reason}})
-                    end;
-                {error, Reason} ->
-                    error({save_cert_to_file, {ensure_dir, Reason}})
-            end;
-        false ->
-            error({save_cert_to_file, invalid_certificate})
-    end.
-
-generate_filename(CertsDir, Key) ->
-    Prefix = case Key of
-                 <<"keyfile">> -> "key-";
-                 <<"certfile">> -> "cert-";
-                 <<"cacertfile">> -> "cacert-"
-             end,
-    to_bin(filename:join([CertsDir, Prefix ++ emqx_misc:gen_id() ++ ".pem"])).
-
-diff_certs(NewSSLOpts, OldSSLOpts) ->
-    Keys = [<<"cacertfile">>, <<"certfile">>, <<"keyfile">>],
-    CertPems = maps:with(Keys, NewSSLOpts),
-    CertFiles = maps:with(Keys, OldSSLOpts),
-    Diff = lists:foldl(fun({K, CertFile}, Acc) ->
-                    case maps:find(K, CertPems) of
-                        error -> Acc;
-                        {ok, PemBin1} ->
-                            {ok, PemBin2} = file:read_file(CertFile),
-                            case diff_cert(PemBin1, PemBin2) of
-                                true ->
-                                    [{changed, K} | Acc];
-                                false ->
-                                    [{identical, K} | Acc]
-                            end
-                    end
-                end,
-                [], maps:to_list(CertFiles)),
-    Added = [{added, K} || K <- maps:keys(maps:without(maps:keys(CertFiles), CertPems))],
-    Diff ++ Added.
-
-diff_cert(Pem1, Pem2) ->
-    cal_md5_for_cert(Pem1) =/= cal_md5_for_cert(Pem2).
-
-cal_md5_for_cert(Pem) ->
-    crypto:hash(md5, term_to_binary(public_key:pem_decode(Pem))).
 
 split_by_id(ID, AuthenticatorsConfig) ->
     case lists:foldl(
@@ -331,8 +246,8 @@ authenticator_id(#{<<"mechanism">> := Mechanism, <<"backend">> := Backend}) ->
     <<Mechanism/binary, ":", Backend/binary>>;
 authenticator_id(#{<<"mechanism">> := Mechanism}) ->
     Mechanism;
-authenticator_id(C) ->
+authenticator_id(_C) ->
-    error({missing_parameter, mechanism, C}).
+    throw({missing_parameter, #{name => mechanism}}).
 
 %% @doc Make the authentication type.
 authn_type(#{mechanism := M, backend :=  B}) -> {atom(M), atom(B)};
@@ -342,3 +257,15 @@ authn_type(#{<<"mechanism">> := M}) -> atom(M).
 
 atom(Bin) ->
     binary_to_existing_atom(Bin, utf8).
+
+%% The relative dir for ssl files.
+certs_dir(ChainName, ConfigOrID) ->
+    DirName = dir(ChainName, ConfigOrID),
+    SubDir = iolist_to_binary(filename:join(["authn", DirName])),
+    binary:replace(SubDir, <<":">>, <<"-">>, [global]).
+
+dir(ChainName, ID) when is_binary(ID) ->
+    binary:replace(iolist_to_binary([to_bin(ChainName), "-", ID]), <<":">>, <<"-">>);
+dir(ChainName, Config) when is_map(Config) ->
+    dir(ChainName, authenticator_id(Config)).
+

apps/emqx/src/emqx_config.erl

@@ -277,7 +277,7 @@ init_load(SchemaMod, RawConf0) when is_map(RawConf0) ->
             maps:with(get_root_names(), RawConf0)).
 
 include_dirs() ->
-    [filename:join(application:get_env(emqx, data_dir, "data/"), "configs") ++ "/"].
+    [filename:join(emqx:data_dir(), "configs")].
 
 -spec check_config(module(), raw_config()) -> {AppEnvs, CheckedConf}
     when AppEnvs :: app_envs(), CheckedConf :: config().

apps/emqx/src/emqx_tls_lib.erl

@@ -16,6 +16,7 @@
 
 -module(emqx_tls_lib).
 
+%% version & cipher suites
 -export([ default_versions/0
         , integral_versions/1
         , default_ciphers/0
@@ -25,6 +26,16 @@
         , all_ciphers/0
         ]).
 
+%% SSL files
+-export([ ensure_ssl_files/2
+        , delete_ssl_files/3
+        , file_content_as_options/1
+        ]).
+
+-include("logger.hrl").
+
+-define(SSL_FILE_OPT_NAMES, [<<"keyfile">>, <<"certfile">>, <<"cacertfile">>]).
+
 %% non-empty string
 -define(IS_STRING(L), (is_list(L) andalso L =/= [] andalso is_integer(hd(L)))).
 %% non-empty list of strings
@@ -212,6 +223,155 @@ drop_tls13(SslOpts0) ->
             SslOpts1#{ciphers => Ciphers -- ?TLSV13_EXCLUSIVE_CIPHERS}
     end.
 
+%% @doc The input map is a HOCON decoded result of a struct defined as
+%% emqx_schema:server_ssl_opts_schema. (NOTE: before schema-checked).
+%% `keyfile', `certfile' and `cacertfile' can be either pem format key or certificates,
+%% or file path.
+%% When PEM format key or certificate is given, it tries to to save them in the given
+%% sub-dir in emqx's data_dir, and replace saved file paths for SSL options.
+-spec ensure_ssl_files(file:name_all(), undefined | map()) ->
+            {ok, undefined | map()} | {error, map()}.
+ensure_ssl_files(Dir, Opts) ->
+    ensure_ssl_files(Dir, Opts, _DryRun = false).
+
+ensure_ssl_files(_Dir, undefined, _DryRun) -> {ok, undefined};
+ensure_ssl_files(_Dir, #{<<"enable">> := false} = Opts, _DryRun) -> {ok, Opts};
+ensure_ssl_files(Dir, Opts, DryRun) ->
+    ensure_ssl_files(Dir, Opts, ?SSL_FILE_OPT_NAMES, DryRun).
+
+ensure_ssl_files(_Dir,Opts, [], _DryRun) -> {ok, Opts};
+ensure_ssl_files(Dir, Opts, [Key | Keys], DryRun) ->
+    case ensure_ssl_file(Dir, Key, Opts, maps:get(Key, Opts, undefined), DryRun) of
+        {ok, NewOpts} ->
+            ensure_ssl_files(Dir, NewOpts, Keys, DryRun);
+        {error, Reason} ->
+            {error, Reason#{which_option => Key}}
+    end.
+
+%% @doc Compare old and new config, delete the ones in old but not in new.
+-spec delete_ssl_files(file:name_all(), undefined | map(), undefined | map()) -> ok.
+delete_ssl_files(Dir, NewOpts0, OldOpts0) ->
+    DryRun = true,
+    {ok, NewOpts} = ensure_ssl_files(Dir, NewOpts0, DryRun),
+    {ok, OldOpts} = ensure_ssl_files(Dir, OldOpts0, DryRun),
+    Get = fun(_K, undefined) -> undefined;
+             (K, Opts) -> maps:get(K, Opts, undefined)
+          end,
+    lists:foreach(fun(Key) -> delete_old_file(Get(Key, NewOpts), Get(Key, OldOpts)) end,
+                  ?SSL_FILE_OPT_NAMES).
+
+delete_old_file(New, Old) when New =:= Old -> ok;
+delete_old_file(_New, _Old = undefined) -> ok;
+delete_old_file(_New, Old) ->
+    case filelib:is_regular(Old) andalso file:delete(Old) of
+        ok -> ok;
+        false -> ok; %% already deleted
+        {error, Reason} ->
+            ?SLOG(error, #{msg => "failed_to_delete_ssl_file", file_path => Old, reason => Reason})
+    end.
+
+ensure_ssl_file(_Dir, _Key, Opts, undefined, _DryRun) ->
+    {ok, Opts};
+ensure_ssl_file(Dir, Key, Opts, MaybePem, DryRun) ->
+    case is_valid_string(MaybePem) of
+        true ->
+            do_ensure_ssl_file(Dir, Key, Opts, MaybePem, DryRun);
+        false ->
+            {error, #{reason => invalid_file_path_or_pem_string}}
+    end.
+
+do_ensure_ssl_file(Dir, Key, Opts, MaybePem, DryRun) ->
+    case is_pem(MaybePem) of
+        true ->
+            case save_pem_file(Dir, Key, MaybePem, DryRun) of
+                {ok, Path} -> {ok, Opts#{Key => Path}};
+                {error, Reason} -> {error, Reason}
+            end;
+        false ->
+            case is_valid_pem_file(MaybePem) of
+                true -> {ok, Opts};
+                {error, enoent} when DryRun -> {ok, Opts};
+                {error, Reason} ->
+                    {error, #{pem_check => invalid_pem,
+                              file_read => Reason
+                            }}
+            end
+    end.
+
+is_valid_string(String) when is_list(String) ->
+    io_lib:printable_unicode_list(String);
+is_valid_string(Binary) when is_binary(Binary) ->
+    case unicode:characters_to_list(Binary, utf8) of
+        String when is_list(String) -> is_valid_string(String);
+        _Otherwise -> false
+    end.
+
+%% Check if it is a valid PEM formated key.
+is_pem(MaybePem) ->
+    try public_key:pem_decode(MaybePem) =/= []
+    catch _ : _ -> false
+    end.
+
+%% Write the pem file to the given dir.
+%% To make it simple, the file is always overwritten.
+%% Also a potentially half-written PEM file (e.g. due to power outage)
+%% can be corrected with an overwrite.
+save_pem_file(Dir, Key, Pem, DryRun) ->
+    Path = pem_file_name(Dir, Key, Pem),
+    case filelib:ensure_dir(Path) of
+        ok when DryRun ->
+            {ok, Path};
+        ok ->
+            case file:write_file(Path, Pem) of
+                ok -> {ok, Path};
+                {error, Reason} ->
+                    {error, #{failed_to_write_file => Reason, file_path => Path}}
+            end;
+        {error, Reason} ->
+            {error, #{failed_to_create_dir_for => Path, reason => Reason}}
+    end.
+
+%% compute the filename for a PEM format key/certificate
+%% the filename is prefixed by the option name without the 'file' part
+%% and suffixed with the first 8 byets the PEM content's md5 checksum.
+%% e.g. key-1234567890abcdef, cert-1234567890abcdef, and cacert-1234567890abcdef
+pem_file_name(Dir, Key, Pem) ->
+    <<CK:8/binary, _/binary>> = crypto:hash(md5, Pem),
+    Suffix = hex_str(CK),
+    FileName = binary:replace(Key, <<"file">>, <<"-", Suffix/binary>>),
+    filename:join([emqx:certs_dir(), Dir, FileName]).
+
+hex_str(Bin) ->
+    iolist_to_binary([io_lib:format("~2.16.0b",[X]) || <<X:8>> <= Bin ]).
+
+is_valid_pem_file(Path) ->
+    case file:read_file(Path) of
+        {ok, Pem} -> is_pem(Pem) orelse {error, not_pem};
+        {error, Reason} -> {error, Reason}
+    end.
+
+%% @doc This is to return SSL file content in management APIs.
+file_content_as_options(undefined) -> undefined;
+file_content_as_options(#{<<"enable">> := false} = SSL) ->
+    maps:without(?SSL_FILE_OPT_NAMES, SSL);
+file_content_as_options(#{<<"enable">> := true} = SSL) ->
+    file_content_as_options(?SSL_FILE_OPT_NAMES, SSL).
+
+file_content_as_options([], SSL) -> {ok, SSL};
+file_content_as_options([Key | Keys], SSL) ->
+    case maps:get(Key, SSL, undefined) of
+        undefined -> file_content_as_options(Keys, SSL);
+        Path ->
+            case file:read_file(Path) of
+                {ok, Bin} ->
+                    file_content_as_options(Keys, SSL#{Key => Bin});
+                {error, Reason} ->
+                    {error, #{file_path => Path,
+                              reason => Reason
+                             }}
+            end
+    end.
+
 -if(?OTP_RELEASE > 22).
 -ifdef(TEST).
 -include_lib("eunit/include/eunit.hrl").

apps/emqx/test/emqx_authentication_SUITE.erl

@@ -97,17 +97,10 @@ end_per_suite(_) ->
     ok.
 
 init_per_testcase(Case, Config) ->
-    meck:new(emqx, [non_strict, passthrough, no_history, no_link]),
-    meck:expect(emqx, get_config, fun([node, data_dir]) ->
-                                          {data_dir, Data} = lists:keyfind(data_dir, 1, Config),
-                                          Data;
-                                     (C) -> meck:passthrough([C])
-                                  end),
     ?MODULE:Case({'init', Config}).
 
 end_per_testcase(Case, Config) ->
     _ = ?MODULE:Case({'end', Config}),
-    meck:unload(emqx),
     ok.
 
 t_chain({_, Config}) -> Config;
@@ -119,7 +112,7 @@ t_chain(Config) when is_list(Config) ->
     ?assertEqual({error, {already_exists, {chain, ChainName}}}, ?AUTHN:create_chain(ChainName)),
     ?assertMatch({ok, #{name := ChainName, authenticators := []}}, ?AUTHN:lookup_chain(ChainName)),
     ?assertMatch({ok, [#{name := ChainName}]}, ?AUTHN:list_chains()),
-    ?assertEqual(ok, ?AUTHN:delete_chain(ChainName)),
+    ?assertEqual(ok, ?AUTHN:delete_chain(ChainName)),
     ?assertMatch({error, {not_found, {chain, ChainName}}}, ?AUTHN:lookup_chain(ChainName)),
     ok.
 
@@ -273,13 +266,11 @@ t_convert_certs(Config) when is_list(Config) ->
 
     CertsDir = certs_dir(Config, [Global, <<"password-based:built-in-database">>]),
     #{<<"ssl">> := NCerts} = convert_certs(CertsDir, #{<<"ssl">> => Certs}),
-    ?assertEqual(false, diff_cert(maps:get(<<"keyfile">>, NCerts), maps:get(<<"keyfile">>, Certs))),
 
     Certs2 = certs([ {<<"keyfile">>, "key.pem"}
                    , {<<"certfile">>, "cert.pem"}
                    ]),
     #{<<"ssl">> := NCerts2} = convert_certs(CertsDir, #{<<"ssl">> => Certs2}, #{<<"ssl">> => NCerts}),
-    ?assertEqual(false, diff_cert(maps:get(<<"keyfile">>, NCerts2), maps:get(<<"keyfile">>, Certs2))),
     ?assertEqual(maps:get(<<"keyfile">>, NCerts), maps:get(<<"keyfile">>, NCerts2)),
     ?assertEqual(maps:get(<<"certfile">>, NCerts), maps:get(<<"certfile">>, NCerts2)),
 
@@ -288,7 +279,6 @@ t_convert_certs(Config) when is_list(Config) ->
                    , {<<"cacertfile">>, "cacert.pem"}
                    ]),
     #{<<"ssl">> := NCerts3} = convert_certs(CertsDir, #{<<"ssl">> => Certs3}, #{<<"ssl">> => NCerts2}),
-    ?assertEqual(false, diff_cert(maps:get(<<"keyfile">>, NCerts3), maps:get(<<"keyfile">>, Certs3))),
     ?assertNotEqual(maps:get(<<"keyfile">>, NCerts2), maps:get(<<"keyfile">>, NCerts3)),
     ?assertNotEqual(maps:get(<<"certfile">>, NCerts2), maps:get(<<"certfile">>, NCerts3)),
 
@@ -306,10 +296,6 @@ certs(Certs) ->
                     Acc#{Key => Bin}
                 end, #{}, Certs).
 
-diff_cert(CertFile, CertPem2) ->
-    {ok, CertPem1} = file:read_file(CertFile),
-    emqx_authentication_config:diff_cert(CertPem1, CertPem2).
-
 register_provider(Type, Module) ->
     ok = ?AUTHN:register_providers([{Type, Module}]).
 

apps/emqx/test/emqx_tls_lib_tests.erl

@@ -38,7 +38,7 @@ use_default_ciphers_test() ->
 
 ciphers_format_test_() ->
     String = ?TLS_13_CIPHER ++ "," ++ ?TLS_12_CIPHER,
-    Binary = iolist_to_binary(String),
+    Binary = bin(String),
     List = [?TLS_13_CIPHER, ?TLS_12_CIPHER],
     [ {"string", fun() -> test_cipher_format(String) end}
     , {"binary", fun() -> test_cipher_format(Binary) end}
@@ -66,3 +66,93 @@ cipher_suites_no_duplication_test() ->
     AllCiphers = emqx_tls_lib:default_ciphers(),
     ?assertEqual(length(AllCiphers), length(lists:usort(AllCiphers))).
 
+ssl_files_failure_test_() ->
+    [{"undefined_is_undefined",
+      fun() ->
+              ?assertEqual({ok, undefined},
+                           emqx_tls_lib:ensure_ssl_files("dir", undefined)) end},
+     {"no_op_if_disabled",
+      fun() ->
+              Disabled = #{<<"enable">> => false, foo => bar},
+              ?assertEqual({ok, Disabled},
+                           emqx_tls_lib:ensure_ssl_files("dir", Disabled)) 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}},
+                           emqx_tls_lib:ensure_ssl_files("/tmp", #{<<"keyfile">> => NonExistingFile}))
+      end},
+     {"bad_pem_string",
+      fun() ->
+              %% not valid unicode
+              ?assertMatch({error, #{reason := invalid_file_path_or_pem_string, which_option := <<"keyfile">>}},
+                           emqx_tls_lib:ensure_ssl_files("/tmp", #{<<"keyfile">> => <<255, 255>>})),
+              %% not printable
+              ?assertMatch({error, #{reason := invalid_file_path_or_pem_string}},
+                           emqx_tls_lib:ensure_ssl_files("/tmp", #{<<"keyfile">> => <<33, 22>>})),
+              TmpFile = filename:join("/tmp", integer_to_list(erlang:system_time(microsecond))),
+              try
+                  ok = file:write_file(TmpFile, <<"not a valid pem">>),
+                  ?assertMatch({error, #{file_read := not_pem}},
+                               emqx_tls_lib:ensure_ssl_files("/tmp", #{<<"cacertfile">> => bin(TmpFile)}))
+              after
+                  file:delete(TmpFile)
+              end
+      end}
+    ].
+
+ssl_files_save_delete_test() ->
+    SSL0 = #{<<"keyfile">> => bin(test_key())},
+    Dir = filename:join(["/tmp", "ssl-test-dir"]),
+    {ok, SSL} = emqx_tls_lib:ensure_ssl_files(Dir, SSL0),
+    File = maps:get(<<"keyfile">>, SSL),
+    ?assertMatch(<<"/tmp/ssl-test-dir/key-", _:16/binary>>, File),
+    ?assertEqual({ok, bin(test_key())}, file:read_file(File)),
+    %% no old file to delete
+    ok = emqx_tls_lib:delete_ssl_files(Dir, SSL, undefined),
+    ?assertEqual({ok, bin(test_key())}, file:read_file(File)),
+    %% old and new identical, no delete
+    ok = emqx_tls_lib:delete_ssl_files(Dir, SSL, SSL),
+    ?assertEqual({ok, bin(test_key())}, file:read_file(File)),
+    %% new is gone, delete old
+    ok = emqx_tls_lib:delete_ssl_files(Dir, undefined, SSL),
+    ?assertEqual({error, enoent}, file:read_file(File)),
+    %% test idempotence
+    ok = emqx_tls_lib:delete_ssl_files(Dir, undefined, SSL),
+    ok.
+
+ssl_file_replace_test() ->
+    SSL0 = #{<<"keyfile">> => bin(test_key())},
+    SSL1 = #{<<"keyfile">> => bin(test_key2())},
+    Dir = filename:join(["/tmp", "ssl-test-dir2"]),
+    {ok, SSL2} = emqx_tls_lib:ensure_ssl_files(Dir, SSL0),
+    {ok, SSL3} = emqx_tls_lib:ensure_ssl_files(Dir, SSL1),
+    File1 = maps:get(<<"keyfile">>, SSL2),
+    File2 = maps:get(<<"keyfile">>, SSL3),
+    ?assert(filelib:is_regular(File1)),
+    ?assert(filelib:is_regular(File2)),
+    %% delete old file (File1, in SSL2)
+    ok = emqx_tls_lib:delete_ssl_files(Dir, SSL3, SSL2),
+    ?assertNot(filelib:is_regular(File1)),
+    ?assert(filelib:is_regular(File2)),
+    ok.
+
+bin(X) -> iolist_to_binary(X).
+
+test_key() ->
+"""
+-----BEGIN EC PRIVATE KEY-----
+MHQCAQEEICKTbbathzvD8zvgjL7qRHhW4alS0+j0Loo7WeYX9AxaoAcGBSuBBAAK
+oUQDQgAEJBdF7MIdam5T4YF3JkEyaPKdG64TVWCHwr/plC0QzNVJ67efXwxlVGTo
+ju0VBj6tOX1y6C0U+85VOM0UU5xqvw==
+-----END EC PRIVATE KEY-----
+""".
+
+test_key2() ->
+"""
+-----BEGIN EC PRIVATE KEY-----
+MHQCAQEEID9UlIyAlLFw0irkRHX29N+ZGivGtDjlVJvATY3B0TTmoAcGBSuBBAAK
+oUQDQgAEUwiarudRNAT25X11js8gE9G+q0GdsT53QJQjRtBO+rTwuCW1vhLzN0Ve
+AbToUD4JmV9m/XwcSVH06ZaWqNuC5w==
+-----END EC PRIVATE KEY-----
+""".

apps/emqx_authn/src/emqx_authn_api.erl

@@ -1989,69 +1989,46 @@ convert_certs(Config) ->
 
 serialize_error({not_found, {authenticator, ID}}) ->
     {404, #{code => <<"NOT_FOUND">>,
-            message => list_to_binary(
+            message => binfmt("Authenticator '~ts' does not exist", [ID]) }};
-                io_lib:format("Authenticator '~ts' does not exist", [ID])
-            )}};
-
 serialize_error({not_found, {listener, ID}}) ->
     {404, #{code => <<"NOT_FOUND">>,
-            message => list_to_binary(
+            message => binfmt("Listener '~ts' does not exist", [ID])}};
-                io_lib:format("Listener '~ts' does not exist", [ID])
-            )}};
-
 serialize_error({not_found, {chain, ?GLOBAL}}) ->
-    {500, #{code => <<"INTERNAL_SERVER_ERROR">>,
+    {404, #{code => <<"NOT_FOUND">>,
-            message => <<"Authentication status is abnormal">>}};
+            message => <<"Authenticator not found in the 'global' scope">>}};
-
 serialize_error({not_found, {chain, Name}}) ->
     {400, #{code => <<"BAD_REQUEST">>,
-            message => list_to_binary(
+            message => binfmt("No authentication has been create for listener '~ts'", [Name])}};
-                io_lib:format("No authentication has been create for listener '~ts'", [Name])
-            )}};
-
 serialize_error({already_exists, {authenticator, ID}}) ->
     {409, #{code => <<"ALREADY_EXISTS">>,
-            message => list_to_binary(
+            message => binfmt("Authenticator '~ts' already exist", [ID])}};
-                io_lib:format("Authenticator '~ts' already exist", [ID])
-            )}};
-
 serialize_error(no_available_provider) ->
     {400, #{code => <<"BAD_REQUEST">>,
             message => <<"Unsupported authentication type">>}};
-
 serialize_error(change_of_authentication_type_is_not_allowed) ->
     {400, #{code => <<"BAD_REQUEST">>,
             message => <<"Change of authentication type is not allowed">>}};
-
 serialize_error(unsupported_operation) ->
     {400, #{code => <<"BAD_REQUEST">>,
             message => <<"Operation not supported in this authentication type">>}};
-
+serialize_error({bad_ssl_config, Details}) ->
-serialize_error({save_cert_to_file, invalid_certificate}) ->
     {400, #{code => <<"BAD_REQUEST">>,
-            message => <<"Invalid certificate">>}};
+            message => binfmt("bad_ssl_config ~p", [Details])}};
-
+serialize_error({missing_parameter, Detail}) ->
-serialize_error({save_cert_to_file, {_, Reason}}) ->
-    {500, #{code => <<"INTERNAL_SERVER_ERROR">>,
-            message => list_to_binary(
-                io_lib:format("Cannot save certificate to file due to '~p'", [Reason])
-            )}};
-
-serialize_error({missing_parameter, Name}) ->
     {400, #{code => <<"MISSING_PARAMETER">>,
-            message => list_to_binary(
+            message => binfmt("Missing required parameter", [Detail])}};
-                io_lib:format("The input parameter '~p' that is mandatory for processing this request is not supplied", [Name])
-            )}};
-
 serialize_error({invalid_parameter, Name}) ->
     {400, #{code => <<"INVALID_PARAMETER">>,
-            message => list_to_binary(
+            message => binfmt("Invalid value for '~p'", [Name])}};
-                io_lib:format("The value of input parameter '~p' is invalid", [Name])
+serialize_error({unknown_authn_type, Type}) ->
-            )}};
+    {400, #{code => <<"BAD_REQUEST">>,
-
+            message => binfmt("Unknown type '~ts'", [Type])}};
+serialize_error({bad_authenticator_config, Reason}) ->
+    {400, #{code => <<"BAD_REQUEST">>,
+            message => binfmt("Bad authenticator config ~p", [Reason])}};
 serialize_error(Reason) ->
     {400, #{code => <<"BAD_REQUEST">>,
-            message => list_to_binary(io_lib:format("~p", [Reason]))}}.
+            message => binfmt("~p", [Reason])}}.
 
 parse_position(<<"top">>) ->
     {ok, top};
@@ -2069,3 +2046,5 @@ to_atom(B) when is_binary(B) ->
     binary_to_atom(B);
 to_atom(A) when is_atom(A) ->
     A.
+
+binfmt(Fmt, Args) -> iolist_to_binary(io_lib:format(Fmt, Args)).

apps/emqx_authn/test/emqx_authn_api_SUITE.erl

@@ -51,7 +51,6 @@ set_special_configs(emqx_dashboard) ->
         }]
     },
     emqx_config:put([emqx_dashboard], Config),
-    emqx_config:put([node, data_dir], "data"),
     ok;
 set_special_configs(_App) ->
     ok.

apps/emqx_authz/src/emqx_authz.erl

@@ -38,6 +38,7 @@
 
 -export([post_config_update/4, pre_config_update/2]).
 
+-export([acl_conf_file/0]).
 
 -spec(register_metrics() -> ok).
 register_metrics() ->
@@ -381,3 +382,7 @@ type(<<"postgresql">>) -> postgresql;
 type('built-in-database') -> 'built-in-database';
 type(<<"built-in-database">>) -> 'built-in-database';
 type(Unknown) -> error({unknown_authz_source_type, Unknown}). % should never happend if the input is type-checked by hocon schema
+
+%% @doc where the acl.conf file is stored.
+acl_conf_file() ->
+    filename:join([emqx:data_dir(), "authz", "acl.conf"]).

apps/emqx_authz/src/emqx_authz_api_sources.erl

@@ -340,11 +340,11 @@ sources(get, _) ->
                                                                 }])
                                   end;
                               (Source, AccIn) ->
-                                  lists:append(AccIn, [read_cert(Source)])
+                                  lists:append(AccIn, [read_certs(Source)])
                           end, [], get_raw_sources()),
     {200, #{sources => Sources}};
 sources(post, #{body := #{<<"type">> := <<"file">>, <<"rules">> := Rules}}) ->
-    {ok, Filename} = write_file(filename:join([emqx:get_config([node, data_dir]), "acl.conf"]), Rules),
+    {ok, Filename} = write_file(acl_conf_file(), Rules),
     update_config(?CMD_PREPEND, [#{<<"type">> => <<"file">>, <<"enable">> => true, <<"path">> => Filename}]);
 sources(post, #{body := Body}) when is_map(Body) ->
     update_config(?CMD_PREPEND, [maybe_write_certs(Body)]);
@@ -352,7 +352,7 @@ sources(put, #{body := Body}) when is_list(Body) ->
     NBody = [ begin
                 case Source of
                     #{<<"type">> := <<"file">>, <<"rules">> := Rules, <<"enable">> := Enable} ->
-                        {ok, Filename} = write_file(filename:join([emqx:get_config([node, data_dir]), "acl.conf"]), Rules),
+                        {ok, Filename} = write_file(acl_conf_file(), Rules),
                         #{<<"type">> => <<"file">>, <<"enable">> => Enable, <<"path">> => Filename};
                     _ -> maybe_write_certs(Source)
                 end
@@ -375,7 +375,7 @@ source(get, #{bindings := #{type := Type}}) ->
                             message => bin(Reason)}}
             end;
         [Source] ->
-            {200, read_cert(Source)}
+            {200, read_certs(Source)}
     end;
 source(put, #{bindings := #{type := <<"file">>}, body := #{<<"type">> := <<"file">>, <<"rules">> := Rules, <<"enable">> := Enable}}) ->
     {ok, Filename} = write_file(maps:get(path, emqx_authz:lookup(file), ""), Rules),
@@ -427,54 +427,18 @@ update_config(Cmd, Sources) ->
                     message => bin(Reason)}}
     end.
 
-read_cert(#{<<"ssl">> := #{<<"enable">> := true} = SSL} = Source) ->
+read_certs(#{<<"ssl">> := SSL} = Source) ->
-    CaCert = case file:read_file(maps:get(<<"cacertfile">>, SSL, "")) of
+    case emqx_tls_lib:file_content_as_options(SSL) of
-                 {ok, CaCert0} -> CaCert0;
+        {ok, NewSSL} -> Source#{<<"ssl">> => NewSSL};
-                 _ -> ""
+        {error, Reason} ->
-             end,
+            ?SLOG(error, Reason#{msg => failed_to_readd_ssl_file}),
-    Cert =   case file:read_file(maps:get(<<"certfile">>, SSL, "")) of
+            throw(failed_to_readd_ssl_file)
-                 {ok, Cert0} -> Cert0;
+    end;
-                 _ -> ""
+read_certs(Source) -> Source.
-             end,
-    Key =   case file:read_file(maps:get(<<"keyfile">>, SSL, "")) of
-                 {ok, Key0} -> Key0;
-                 _ -> ""
-             end,
-    Source#{<<"ssl">> => SSL#{<<"cacertfile">> => CaCert,
-                              <<"certfile">> => Cert,
-                              <<"keyfile">> => Key
-                             }
-           };
-read_cert(Source) -> Source.
 
 maybe_write_certs(#{<<"ssl">> := #{<<"enable">> := true} = SSL} = Source) ->
-    CertPath = filename:join([emqx:get_config([node, data_dir]), "certs"]),
+    Type = maps:get(<<"type">>, Source),
-    CaCert = case maps:is_key(<<"cacertfile">>, SSL) of
+    emqx_tls_lib:ensure_ssl_files(filename:join(["authz", Type]), SSL);
-                 true ->
-                     {ok, CaCertFile} = write_file(filename:join([CertPath, "cacert-" ++ emqx_misc:gen_id() ++".pem"]),
-                                                 maps:get(<<"cacertfile">>, SSL)),
-                     CaCertFile;
-                 false -> ""
-             end,
-    Cert =   case maps:is_key(<<"certfile">>, SSL) of
-                 true ->
-                     {ok, CertFile} = write_file(filename:join([CertPath, "cert-" ++ emqx_misc:gen_id() ++".pem"]),
-                                                 maps:get(<<"certfile">>, SSL)),
-                     CertFile;
-                 false -> ""
-             end,
-    Key =    case maps:is_key(<<"keyfile">>, SSL) of
-                 true ->
-                     {ok, KeyFile}  = write_file(filename:join([CertPath, "key-" ++ emqx_misc:gen_id() ++".pem"]),
-                                                 maps:get(<<"keyfile">>, SSL)),
-                     KeyFile;
-                 false -> ""
-             end,
-    Source#{<<"ssl">> => SSL#{<<"cacertfile">> => CaCert,
-                              <<"certfile">> => Cert,
-                              <<"keyfile">> => Key
-                             }
-           };
 maybe_write_certs(Source) -> Source.
 
 write_file(Filename, Bytes0) ->
@@ -482,7 +446,7 @@ write_file(Filename, Bytes0) ->
     case file:read_file(Filename) of
         {ok, Bytes1} ->
             case crypto:hash(md5, Bytes1) =:= crypto:hash(md5, Bytes0) of
-                true -> {ok,iolist_to_binary(Filename)};
+                true -> {ok, iolist_to_binary(Filename)};
                 false -> do_write_file(Filename, Bytes0)
             end;
         _ -> do_write_file(Filename, Bytes0)
@@ -498,3 +462,6 @@ do_write_file(Filename, Bytes) ->
 
 bin(Term) ->
    erlang:iolist_to_binary(io_lib:format("~p", [Term])).
+
+acl_conf_file() ->
+    emqx_authz:acl_conf_file().

apps/emqx_authz/src/emqx_authz_schema.erl

@@ -71,7 +71,14 @@ fields(file) ->
     , {enable, #{type => boolean(),
                  default => true}}
     , {path, #{type => string(),
-               desc => "Path to the file which contains the ACL rules."
+               desc => """
+Path to the file which contains the ACL rules.<br>
+If the file provisioned before starting EMQ X node, it can be placed anywhere
+as long as EMQ X has read access to it.
+In case rule set is created from EMQ X dashboard or management HTTP API,
+the file will be placed in `certs/authz` sub directory inside EMQ X's `data_dir`,
+and the new rules will override all rules from the old config file.
+"""
               }}
     ];
 fields(http_get) ->

apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl

@@ -147,11 +147,9 @@ init_per_testcase(t_api, Config) ->
     meck:expect(emqx_misc, gen_id, fun() -> "fake" end),
 
     meck:new(emqx, [non_strict, passthrough, no_history, no_link]),
-    meck:expect(emqx, get_config, fun([node, data_dir]) ->
+    meck:expect(emqx, data_dir, fun() ->
-                                          % emqx_common_test_helpers:deps_path(emqx_authz, "test");
                                         {data_dir, Data} = lists:keyfind(data_dir, 1, Config),
-                                          Data;
+                                        Data
-                                     (C) -> meck:passthrough([C])
                                 end),
     Config;
 init_per_testcase(_, Config) -> Config.
@@ -182,7 +180,7 @@ t_api(_) ->
                  , #{<<"type">> := <<"redis">>}
                  , #{<<"type">> := <<"file">>}
                  ], Sources),
-    ?assert(filelib:is_file(filename:join([emqx:get_config([node, data_dir]), "acl.conf"]))),
+    ?assert(filelib:is_file(emqx_authz:acl_conf_file())),
 
     {ok, 204, _} = request(put, uri(["authorization", "sources", "http"]),  ?SOURCE1#{<<"enable">> := false}),
     {ok, 200, Result3} = request(get, uri(["authorization", "sources", "http"]), []),
@@ -205,9 +203,9 @@ t_api(_) ->
                                   <<"verify">> := false
                                  }
                   }, jsx:decode(Result4)),
-    ?assert(filelib:is_file(filename:join([emqx:get_config([node, data_dir]), "certs", "cacert-fake.pem"]))),
+    ?assert(filelib:is_file(filename:join([data_dir(), "certs", "cacert-fake.pem"]))),
-    ?assert(filelib:is_file(filename:join([emqx:get_config([node, data_dir]), "certs", "cert-fake.pem"]))),
+    ?assert(filelib:is_file(filename:join([data_dir(), "certs", "cert-fake.pem"]))),
-    ?assert(filelib:is_file(filename:join([emqx:get_config([node, data_dir]), "certs", "key-fake.pem"]))),
+    ?assert(filelib:is_file(filename:join([data_dir(), "certs", "key-fake.pem"]))),
 
     {ok, 204, _} = request(put, uri(["authorization", "sources", "mysql"]),  ?SOURCE3#{<<"server">> := <<"192.168.1.100:3306">>}),
 
@@ -301,3 +299,5 @@ auth_header_() ->
     Password = <<"public">>,
     {ok, Token} = emqx_dashboard_admin:sign_token(Username, Password),
     {"Authorization", "Bearer " ++ binary_to_list(Token)}.
+
+data_dir() -> emqx:data_dir().

apps/emqx_plugin_libs/src/emqx_plugin_libs_ssl.erl

@@ -42,15 +42,27 @@
 %% @doc Parse ssl options input.
 %% If the input contains file content, save the files in the given dir.
 %% Returns ssl options for Erlang's ssl application.
+%%
+%% For SSL files in the input Option, it can either be a file path
+%% or a map like `#{filename := FileName, file := Content}`.
+%% In case it's a map, the file is saved in EMQ X's `data_dir'
+%% (unless `SubDir' is an absolute path).
+%% NOTE: This function is now deprecated, use emqx_tls_lib:ensure_ssl_files/2 instead.
 -spec save_files_return_opts(opts_input(), atom() | string() | binary(),
                              string() | binary()) -> opts().
 save_files_return_opts(Options, SubDir, ResId) ->
-    Dir = filename:join([emqx:get_config([node, data_dir]), SubDir, ResId]),
+    Dir = filename:join([emqx:data_dir(), SubDir, ResId]),
     save_files_return_opts(Options, Dir).
 
 %% @doc Parse ssl options input.
 %% If the input contains file content, save the files in the given dir.
 %% Returns ssl options for Erlang's ssl application.
+%%
+%% For SSL files in the input Option, it can either be a file path
+%% or a map like `#{filename := FileName, file := Content}`.
+%% In case it's a map, the file is saved in EMQ X's `data_dir'
+%% (unless `SubDir' is an absolute path).
+%% NOTE: This function is now deprecated, use emqx_tls_lib:ensure_ssl_files/2 instead.
 -spec save_files_return_opts(opts_input(), file:name_all()) -> opts().
 save_files_return_opts(Options, Dir) ->
     GetD = fun(Key, Default) -> fuzzy_map_get(Key, Options, Default) end,
@@ -76,7 +88,7 @@ save_files_return_opts(Options, Dir) ->
 %% empty string is returned if the input is empty.
 -spec save_file(file_input(), atom() | string() | binary()) -> string().
 save_file(Param, SubDir) ->
-   Dir = filename:join([emqx:get_config([node, data_dir]), SubDir]),
+   Dir = filename:join([emqx:data_dir(), SubDir]),
    do_save_file(Param, Dir).
 
 filter([]) -> [];