From a7413bc11e07b5f43b5a658e7f4c1262e9744c40 Mon Sep 17 00:00:00 2001 From: Zaiming Shi Date: Sun, 24 Oct 2021 15:54:39 +0200 Subject: [PATCH] fix(authn): save certificates to certs dir --- apps/emqx/src/emqx.erl | 4 ++ apps/emqx/src/emqx_authentication_config.erl | 69 +++++++++---------- apps/emqx/src/emqx_tls_lib.erl | 18 +++-- apps/emqx/test/emqx_tls_lib_tests.erl | 6 +- apps/emqx_authn/src/emqx_authn_api.erl | 61 ++++++---------- .../emqx_authz/src/emqx_authz_api_sources.erl | 2 +- apps/emqx_authz/src/emqx_authz_schema.erl | 2 +- 7 files changed, 72 insertions(+), 90 deletions(-) diff --git a/apps/emqx/src/emqx.erl b/apps/emqx/src/emqx.erl index 3a1eda6c4..1df7f0149 100644 --- a/apps/emqx/src/emqx.erl +++ b/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]). diff --git a/apps/emqx/src/emqx_authentication_config.erl b/apps/emqx/src/emqx_authentication_config.erl index ab2124790..24abb951c 100644 --- a/apps/emqx/src/emqx_authentication_config.erl +++ b/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}) -> <>; 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)). + diff --git a/apps/emqx/src/emqx_tls_lib.erl b/apps/emqx/src/emqx_tls_lib.erl index f94fc1281..0d64f3003 100644 --- a/apps/emqx/src/emqx_tls_lib.erl +++ b/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) -> - <> = base64:encode(crypto:hash(md5, Pem)), - FileName = binary:replace(Key, <<"file">>, <<"-", CK/binary>>), - filename:join([emqx:data_dir(), Dir, FileName]). + <> = 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]) || <> <= Bin ]). is_valid_pem_file(Path) -> case file:read_file(Path) of diff --git a/apps/emqx/test/emqx_tls_lib_tests.erl b/apps/emqx/test/emqx_tls_lib_tests.erl index 514c365e7..8125154cc 100644 --- a/apps/emqx/test/emqx_tls_lib_tests.erl +++ b/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), diff --git a/apps/emqx_authn/src/emqx_authn_api.erl b/apps/emqx_authn/src/emqx_authn_api.erl index 4f02afc55..9f26e7f91 100644 --- a/apps/emqx_authn/src/emqx_authn_api.erl +++ b/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)). diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index efda82fc1..25135fa34 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/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) -> diff --git a/apps/emqx_authz/src/emqx_authz_schema.erl b/apps/emqx_authz/src/emqx_authz_schema.erl index 542a7445d..ab8ba220d 100644 --- a/apps/emqx_authz/src/emqx_authz_schema.erl +++ b/apps/emqx_authz/src/emqx_authz_schema.erl @@ -76,7 +76,7 @@ Path to the file which contains the ACL rules.
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. """ }}