fix(authn): save certificates to certs dir

apps/emqx/src/emqx.erl

@@ -66,6 +66,7 @@
         , remove_config/2
         , reset_config/2
         , data_dir/0
+        , certs_dir/0
         ]).
 
 -define(APP, ?MODULE).
@@ -250,3 +251,6 @@ reset_config([RootName | _] = KeyPath, Opts) ->
 
 data_dir() ->
     application:get_env(emqx, data_dir, "data").
+
+certs_dir() ->
+    filename:join([data_dir(), certs]).

apps/emqx/src/emqx_authentication_config.erl

@@ -56,44 +56,33 @@
 -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),
-        NConfig = convert_certs(CertsDir, Config),
-        {ok, OldConfig ++ [NConfig]}
-    catch
-        error:{save_cert_to_file, _} = Reason ->
-            {error, Reason};
-        error:{missing_parameter, _} = Reason ->
-            {error, Reason}
-    end;
+    CertsDir = certs_dir(ChainName, Config),
+    NConfig = convert_certs(CertsDir, Config),
+    {ok, OldConfig ++ [NConfig]};
 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),
-        NewConfig = lists:map(
-                        fun(OldConfig0) ->
-                            case AuthenticatorID =:= authenticator_id(OldConfig0) of
-                                true -> convert_certs(CertsDir, Config, OldConfig0);
-                                false -> OldConfig0
-                            end
-                        end, OldConfig),
-        {ok, NewConfig}
-    catch
-        error:{save_cert_to_file, _} = Reason ->
-            {error, Reason};
-        error:{missing_parameter, _} = Reason ->
-            {error, Reason}
-    end;
+    CertsDir = certs_dir(ChainName, AuthenticatorID),
+    NewConfig = lists:map(
+                    fun(OldConfig0) ->
+                        case AuthenticatorID =:= authenticator_id(OldConfig0) of
+                            true -> convert_certs(CertsDir, Config, OldConfig0);
+                            false -> OldConfig0
+                        end
+                    end, OldConfig),
+    {ok, NewConfig};
 do_pre_config_update({move_authenticator, _ChainName, AuthenticatorID, Position}, OldConfig) ->
     case split_by_id(AuthenticatorID, OldConfig) of
         {error, Reason} -> {error, Reason};
@@ -152,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.
@@ -180,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;
@@ -197,7 +186,7 @@ convert_certs(CertsDir, Config) ->
             new_ssl_config(Config, SSL);
         {error, Reason} ->
             ?SLOG(error, Reason#{msg => bad_ssl_config}),
-            throw(bad_ssl_config)
+            throw({bad_ssl_config, Reason})
     end.
 
 convert_certs(CertsDir, NewConfig, OldConfig) ->
@@ -209,7 +198,7 @@ convert_certs(CertsDir, NewConfig, OldConfig) ->
             new_ssl_config(NewConfig, NewSSL1);
         {error, Reason} ->
             ?SLOG(error, Reason#{msg => bad_ssl_config}),
-            throw(bad_ssl_config)
+            throw({bad_ssl_config, Reason})
     end.
 
 new_ssl_config(Config, undefined) -> Config;
@@ -257,8 +246,8 @@ authenticator_id(#{<<"mechanism">> := Mechanism, <<"backend">> := Backend}) ->
     <<Mechanism/binary, ":", Backend/binary>>;
 authenticator_id(#{<<"mechanism">> := Mechanism}) ->
     Mechanism;
-authenticator_id(C) ->
-    error({missing_parameter, mechanism, C}).
+authenticator_id(_C) ->
+    throw({missing_parameter, #{name => mechanism}}).
 
 %% @doc Make the authentication type.
 authn_type(#{mechanism := M, backend :=  B}) -> {atom(M), atom(B)};
@@ -270,7 +259,13 @@ atom(Bin) ->
     binary_to_existing_atom(Bin, utf8).
 
 %% The relative dir for ssl files.
-certs_dir(ChainName, ID) when is_binary(ID) ->
-    filename:join([to_bin(ChainName), ID]);
-certs_dir(ChainName, Config) when is_map(Config) ->
-    certs_dir(ChainName, authenticator_id(Config)).
+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_tls_lib.erl

@@ -292,8 +292,8 @@ do_ensure_ssl_file(Dir, Key, Opts, MaybePem, DryRun) ->
                 true -> {ok, Opts};
                 {error, enoent} when DryRun -> {ok, Opts};
                 {error, Reason} ->
-                    {error, #{file_path => MaybePem,
-                              reason => Reason
+                    {error, #{pem_check => invalid_pem,
+                              file_read => Reason
                             }}
             end
     end.
@@ -333,12 +333,16 @@ save_pem_file(Dir, Key, Pem, DryRun) ->
 
 %% 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 of base64 encode result of the PEM content's
-%% md5 checksum.  e.g. key-EKjjO9um, cert-TwuCW1vh, and cacert-6ZaWqNuC
+%% 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>> = base64:encode(crypto:hash(md5, Pem)),
-    FileName = binary:replace(Key, <<"file">>, <<"-", CK/binary>>),
-    filename:join([emqx:data_dir(), Dir, FileName]).
+    <<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

apps/emqx/test/emqx_tls_lib_tests.erl

@@ -79,7 +79,7 @@ ssl_files_failure_test_() ->
      {"enoent_key_file",
       fun() ->
               NonExistingFile = filename:join("/tmp", integer_to_list(erlang:system_time(microsecond))),
-              ?assertMatch({error, #{reason := enoent}},
+              ?assertMatch({error, #{file_read := enoent, pem_check := invalid_pem}},
                            emqx_tls_lib:ensure_ssl_files("/tmp", #{<<"keyfile">> => NonExistingFile}))
       end},
      {"bad_pem_string",
@@ -93,7 +93,7 @@ ssl_files_failure_test_() ->
               TmpFile = filename:join("/tmp", integer_to_list(erlang:system_time(microsecond))),
               try
                   ok = file:write_file(TmpFile, <<"not a valid pem">>),
-                  ?assertMatch({error, #{file_path := _, reason := not_pem}},
+                  ?assertMatch({error, #{file_read := not_pem}},
                                emqx_tls_lib:ensure_ssl_files("/tmp", #{<<"cacertfile">> => bin(TmpFile)}))
               after
                   file:delete(TmpFile)
@@ -106,7 +106,7 @@ ssl_files_save_delete_test() ->
     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-", _:8/binary>>, File),
+    ?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),

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(
-                io_lib:format("Authenticator '~ts' does not exist", [ID])
-            )}};
-
+            message => binfmt("Authenticator '~ts' does not exist", [ID]) }};
 serialize_error({not_found, {listener, ID}}) ->
     {404, #{code => <<"NOT_FOUND">>,
-            message => list_to_binary(
-                io_lib:format("Listener '~ts' does not exist", [ID])
-            )}};
-
+            message => binfmt("Listener '~ts' does not exist", [ID])}};
 serialize_error({not_found, {chain, ?GLOBAL}}) ->
-    {500, #{code => <<"INTERNAL_SERVER_ERROR">>,
-            message => <<"Authentication status is abnormal">>}};
-
+    {404, #{code => <<"NOT_FOUND">>,
+            message => <<"Authenticator not found in the 'global' scope">>}};
 serialize_error({not_found, {chain, Name}}) ->
     {400, #{code => <<"BAD_REQUEST">>,
-            message => list_to_binary(
-                io_lib:format("No authentication has been create for listener '~ts'", [Name])
-            )}};
-
+            message => binfmt("No authentication has been create for listener '~ts'", [Name])}};
 serialize_error({already_exists, {authenticator, ID}}) ->
     {409, #{code => <<"ALREADY_EXISTS">>,
-            message => list_to_binary(
-                io_lib:format("Authenticator '~ts' already exist", [ID])
-            )}};
-
+            message => binfmt("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({save_cert_to_file, invalid_certificate}) ->
+serialize_error({bad_ssl_config, Details}) ->
     {400, #{code => <<"BAD_REQUEST">>,
-            message => <<"Invalid certificate">>}};
-
-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}) ->
+            message => binfmt("bad_ssl_config ~p", [Details])}};
+serialize_error({missing_parameter, Detail}) ->
     {400, #{code => <<"MISSING_PARAMETER">>,
-            message => list_to_binary(
-                io_lib:format("The input parameter '~p' that is mandatory for processing this request is not supplied", [Name])
-            )}};
-
+            message => binfmt("Missing required parameter", [Detail])}};
 serialize_error({invalid_parameter, Name}) ->
     {400, #{code => <<"INVALID_PARAMETER">>,
-            message => list_to_binary(
-                io_lib:format("The value of input parameter '~p' is invalid", [Name])
-            )}};
-
+            message => binfmt("Invalid value for '~p'", [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_authz/src/emqx_authz_api_sources.erl

@@ -438,7 +438,7 @@ read_certs(Source) -> Source.
 
 maybe_write_certs(#{<<"ssl">> := #{<<"enable">> := true} = SSL} = Source) ->
     Type = maps:get(<<"type">>, Source),
-    emqx_tls_lib:ensure_ssl_files(filename:join(["authz", "certs", Type]), SSL);
+    emqx_tls_lib:ensure_ssl_files(filename:join(["authz", Type]), SSL);
 maybe_write_certs(Source) -> Source.
 
 write_file(Filename, Bytes0) ->

apps/emqx_authz/src/emqx_authz_schema.erl

@@ -76,7 +76,7 @@ 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 `authz` sub directory inside EMQ X's `data_dir`,
+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.
 """
               }}