From 4961aca3d07fecf7c3315df3bc2d682012006434 Mon Sep 17 00:00:00 2001 From: William Yang Date: Fri, 2 Jun 2023 16:19:58 +0200 Subject: [PATCH 01/16] perf(mqtt-caps): save some map builds --- apps/emqx/src/emqx_mqtt_caps.erl | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/apps/emqx/src/emqx_mqtt_caps.erl b/apps/emqx/src/emqx_mqtt_caps.erl index bf544280f..9f8b40d14 100644 --- a/apps/emqx/src/emqx_mqtt_caps.erl +++ b/apps/emqx/src/emqx_mqtt_caps.erl @@ -108,23 +108,12 @@ do_check_pub(_Flags, _Caps) -> ok_or_error(emqx_types:reason_code()). check_sub(ClientInfo = #{zone := Zone}, Topic, SubOpts) -> Caps = get_caps(?SUBCAP_KEYS, Zone), - Flags = lists:foldl( - fun - (max_topic_levels, Map) -> - Map#{topic_levels => emqx_topic:levels(Topic)}; - (wildcard_subscription, Map) -> - Map#{is_wildcard => emqx_topic:wildcard(Topic)}; - (shared_subscription, Map) -> - Map#{is_shared => maps:is_key(share, SubOpts)}; - (exclusive_subscription, Map) -> - Map#{is_exclusive => maps:get(is_exclusive, SubOpts, false)}; - %% Ignore - (_Key, Map) -> - Map - end, - #{}, - maps:keys(Caps) - ), + Flags = #{ + topic_levels => emqx_topic:levels(Topic), + is_wildcard => emqx_topic:wildcard(Topic), + is_shared => maps:is_key(share, SubOpts), + is_exclusive => maps:get(is_exclusive, SubOpts, false) + }, do_check_sub(Flags, Caps, ClientInfo, Topic). do_check_sub(#{topic_levels := Levels}, #{max_topic_levels := Limit}, _, _) when From b1c188f9b1f9eda6b7b2cf5e7c405881c69524be Mon Sep 17 00:00:00 2001 From: William Yang Date: Fri, 2 Jun 2023 16:24:52 +0200 Subject: [PATCH 02/16] perf(mqtt-caps): dont filter cap keys save map builds --- apps/emqx/src/emqx_mqtt_caps.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx/src/emqx_mqtt_caps.erl b/apps/emqx/src/emqx_mqtt_caps.erl index 9f8b40d14..11f495dbd 100644 --- a/apps/emqx/src/emqx_mqtt_caps.erl +++ b/apps/emqx/src/emqx_mqtt_caps.erl @@ -84,7 +84,7 @@ check_pub(Zone, Flags) when is_map(Flags) -> error -> Flags end, - get_caps(?PUBCAP_KEYS, Zone) + emqx_config:get_zone_conf(Zone, [mqtt]) ). do_check_pub(#{topic_levels := Levels}, #{max_topic_levels := Limit}) when @@ -107,7 +107,7 @@ do_check_pub(_Flags, _Caps) -> ) -> ok_or_error(emqx_types:reason_code()). check_sub(ClientInfo = #{zone := Zone}, Topic, SubOpts) -> - Caps = get_caps(?SUBCAP_KEYS, Zone), + Caps = emqx_config:get_zone_conf(Zone, [mqtt]), Flags = #{ topic_levels => emqx_topic:levels(Topic), is_wildcard => emqx_topic:wildcard(Topic), From 95f706bb9e0e58d55f0635a9a643bd5faf0ff2c7 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Wed, 31 May 2023 22:50:16 +0300 Subject: [PATCH 03/16] fix(ssl): avoid explicit deletion of managed certs / keys This logic was incorrect because it didn't take into account certfiles / keyfiles "refcounts". --- apps/emqx/src/emqx_authentication_config.erl | 38 +----- apps/emqx/src/emqx_listeners.erl | 13 +- apps/emqx/src/emqx_tls_lib.erl | 57 +-------- apps/emqx/test/emqx_tls_lib_tests.erl | 118 +++++------------ apps/emqx_authz/src/emqx_authz.erl | 28 +--- apps/emqx_bridge/src/emqx_bridge_app.erl | 9 +- apps/emqx_bridge/src/emqx_bridge_resource.erl | 29 +---- .../emqx_connector/src/emqx_connector_ssl.erl | 34 +---- apps/emqx_exhook/src/emqx_exhook_mgr.erl | 34 +---- apps/emqx_gateway/src/emqx_gateway_conf.erl | 120 +++--------------- 10 files changed, 77 insertions(+), 403 deletions(-) diff --git a/apps/emqx/src/emqx_authentication_config.erl b/apps/emqx/src/emqx_authentication_config.erl index a1b55ea43..92041095b 100644 --- a/apps/emqx/src/emqx_authentication_config.erl +++ b/apps/emqx/src/emqx_authentication_config.erl @@ -32,9 +32,7 @@ %% Used in emqx_gateway -export([ certs_dir/2, - convert_certs/2, - convert_certs/3, - clear_certs/2 + convert_certs/2 ]). -export_type([config/0]). @@ -97,7 +95,7 @@ do_pre_config_update(_, {update_authenticator, ChainName, AuthenticatorID, Confi NewConfig = lists:map( fun(OldConfig0) -> case AuthenticatorID =:= authenticator_id(OldConfig0) of - true -> convert_certs(CertsDir, Config, OldConfig0); + true -> convert_certs(CertsDir, Config); false -> OldConfig0 end end, @@ -162,17 +160,10 @@ do_post_config_update( _, {delete_authenticator, ChainName, AuthenticatorID}, _NewConfig, - OldConfig, + _OldConfig, _AppEnvs ) -> - case emqx_authentication:delete_authenticator(ChainName, AuthenticatorID) of - ok -> - Config = get_authenticator_config(AuthenticatorID, to_list(OldConfig)), - CertsDir = certs_dir(ChainName, AuthenticatorID), - ok = clear_certs(CertsDir, Config); - {error, Reason} -> - {error, Reason} - end; + emqx_authentication:delete_authenticator(ChainName, AuthenticatorID); do_post_config_update( _, {update_authenticator, ChainName, AuthenticatorID, Config}, @@ -231,9 +222,7 @@ delete_authenticators(NewIds, ChainName, OldConfig) -> true -> ok; false -> - _ = emqx_authentication:delete_authenticator(ChainName, Id), - CertsDir = certs_dir(ChainName, Conf), - ok = clear_certs(CertsDir, Conf) + emqx_authentication:delete_authenticator(ChainName, Id) end end, OldConfig @@ -244,21 +233,10 @@ to_list(M) when M =:= #{} -> []; to_list(M) when is_map(M) -> [M]; to_list(L) when is_list(L) -> L. -convert_certs(CertsDir, Config) -> - case emqx_tls_lib:ensure_ssl_files(CertsDir, maps:get(<<"ssl">>, Config, undefined)) of - {ok, SSL} -> - new_ssl_config(Config, SSL); - {error, Reason} -> - ?SLOG(error, Reason#{msg => "bad_ssl_config"}), - throw({bad_ssl_config, Reason}) - end. - -convert_certs(CertsDir, NewConfig, OldConfig) -> - OldSSL = maps:get(<<"ssl">>, OldConfig, undefined), +convert_certs(CertsDir, NewConfig) -> NewSSL = maps:get(<<"ssl">>, NewConfig, undefined), case emqx_tls_lib:ensure_ssl_files(CertsDir, NewSSL) of {ok, NewSSL1} -> - ok = emqx_tls_lib:delete_ssl_files(CertsDir, NewSSL1, OldSSL), new_ssl_config(NewConfig, NewSSL1); {error, Reason} -> ?SLOG(error, Reason#{msg => "bad_ssl_config"}), @@ -268,10 +246,6 @@ convert_certs(CertsDir, NewConfig, OldConfig) -> new_ssl_config(Config, undefined) -> Config; new_ssl_config(Config, SSL) -> Config#{<<"ssl">> => SSL}. -clear_certs(CertsDir, Config) -> - OldSSL = maps:get(<<"ssl">>, Config, undefined), - ok = emqx_tls_lib:delete_ssl_files(CertsDir, undefined, OldSSL). - get_authenticator_config(AuthenticatorID, AuthenticatorsConfig) -> case filter_authenticator(AuthenticatorID, AuthenticatorsConfig) of [C] -> C; diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl index acdb9ff96..6e35cb602 100644 --- a/apps/emqx/src/emqx_listeners.erl +++ b/apps/emqx/src/emqx_listeners.erl @@ -488,7 +488,6 @@ pre_config_update(_Path, _Request, RawConf) -> post_config_update([listeners, Type, Name], {create, _Request}, NewConf, undefined, _AppEnvs) -> start_listener(Type, Name, NewConf); post_config_update([listeners, Type, Name], {update, _Request}, NewConf, OldConf, _AppEnvs) -> - try_clear_ssl_files(certs_dir(Type, Name), NewConf, OldConf), ok = maybe_unregister_ocsp_stapling_refresh(Type, Name, NewConf), case NewConf of #{enabled := true} -> restart_listener(Type, Name, {OldConf, NewConf}); @@ -501,8 +500,7 @@ post_config_update([listeners, Type, Name], Op, _, OldConf, _AppEnvs) when case stop_listener(Type, Name, OldConf) of ok -> _ = emqx_authentication:delete_chain(listener_id(Type, Name)), - CertsDir = certs_dir(Type, Name), - clear_certs(CertsDir, OldConf); + ok; Err -> Err end; @@ -773,10 +771,6 @@ convert_certs(CertsDir, Conf) -> throw({bad_ssl_config, Reason}) end. -clear_certs(CertsDir, Conf) -> - OldSSL = get_ssl_options(Conf), - emqx_tls_lib:delete_ssl_files(CertsDir, undefined, OldSSL). - filter_stacktrace({Reason, _Stacktrace}) -> Reason; filter_stacktrace(Reason) -> Reason. @@ -786,11 +780,6 @@ ensure_override_limiter_conf(Conf, #{<<"limiter">> := Limiter}) -> ensure_override_limiter_conf(Conf, _) -> Conf. -try_clear_ssl_files(CertsDir, NewConf, OldConf) -> - NewSSL = get_ssl_options(NewConf), - OldSSL = get_ssl_options(OldConf), - emqx_tls_lib:delete_ssl_files(CertsDir, NewSSL, OldSSL). - get_ssl_options(Conf) -> case maps:find(ssl_options, Conf) of {ok, SSL} -> diff --git a/apps/emqx/src/emqx_tls_lib.erl b/apps/emqx/src/emqx_tls_lib.erl index db0996e56..a7b63382c 100644 --- a/apps/emqx/src/emqx_tls_lib.erl +++ b/apps/emqx/src/emqx_tls_lib.erl @@ -30,8 +30,8 @@ -export([ ensure_ssl_files/2, ensure_ssl_files/3, - delete_ssl_files/3, drop_invalid_certs/1, + pem_dir/1, is_valid_pem_file/1, is_pem/1 ]). @@ -326,38 +326,6 @@ ensure_ssl_files_per_key(Dir, SSL, [KeyPath | KeyPaths], Opts) -> {error, Reason#{which_options => [KeyPath]}} 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, #{dry_run => DryRun}), - {ok, OldOpts} = ensure_ssl_files(Dir, OldOpts0, #{dry_run => DryRun}), - Get = fun - (_KP, undefined) -> undefined; - (KP, Opts) -> emqx_utils_maps:deep_get(KP, Opts, undefined) - end, - lists:foreach( - fun(KeyPath) -> delete_old_file(Get(KeyPath, NewOpts), Get(KeyPath, OldOpts)) end, - ?SSL_FILE_OPT_PATHS ++ ?SSL_FILE_OPT_PATHS_A - ), - %% try to delete the dir if it is empty - _ = file:del_dir(pem_dir(Dir)), - ok. - -delete_old_file(New, Old) when New =:= Old -> ok; -delete_old_file(_New, _Old = undefined) -> - ok; -delete_old_file(_New, Old) -> - case is_generated_file(Old) andalso filelib:is_regular(Old) andalso file:delete(Old) of - ok -> - ok; - %% the file is not generated by us, or it is already deleted - false -> - ok; - {error, Reason} -> - ?SLOG(error, #{msg => "failed_to_delete_ssl_file", file_path => Old, reason => Reason}) - end. - ensure_ssl_file(_Dir, _KeyPath, SSL, undefined, _Opts) -> {ok, SSL}; ensure_ssl_file(Dir, KeyPath, SSL, MaybePem, Opts) -> @@ -440,7 +408,7 @@ is_generated_file(Filename) -> pem_file_name(Dir, KeyPath, Pem) -> <> = crypto:hash(md5, Pem), - Suffix = hex_str(CK), + Suffix = binary:encode_hex(CK), Segments = lists:map(fun ensure_bin/1, KeyPath), Filename0 = iolist_to_binary(lists:join(<<"_">>, Segments)), Filename1 = binary:replace(Filename0, <<"file">>, <<>>), @@ -450,27 +418,14 @@ pem_file_name(Dir, KeyPath, Pem) -> pem_dir(Dir) -> filename:join([emqx:mutable_certs_dir(), Dir]). -is_hex_str(HexStr) -> +is_hex_str(Str) -> try - is_hex_str2(ensure_str(HexStr)) + _ = binary:decode_hex(iolist_to_binary(Str)), + true catch - throw:not_hex -> false + error:badarg -> false end. -is_hex_str2(HexStr) -> - _ = [ - case S of - S when S >= $0, S =< $9 -> S; - S when S >= $a, S =< $f -> S; - _ -> throw(not_hex) - end - || S <- HexStr - ], - true. - -hex_str(Bin) -> - iolist_to_binary([io_lib:format("~2.16.0b", [X]) || <> <= Bin]). - %% @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), diff --git a/apps/emqx/test/emqx_tls_lib_tests.erl b/apps/emqx/test/emqx_tls_lib_tests.erl index 0f5883b10..8eea21596 100644 --- a/apps/emqx/test/emqx_tls_lib_tests.erl +++ b/apps/emqx/test/emqx_tls_lib_tests.erl @@ -108,8 +108,8 @@ ssl_files_failure_test_() -> {error, #{file_read := enoent, pem_check := invalid_pem}}, emqx_tls_lib:ensure_ssl_files("/tmp", #{ <<"keyfile">> => NonExistingFile, - <<"certfile">> => bin(test_key()), - <<"cacertfile">> => bin(test_key()) + <<"certfile">> => test_key(), + <<"cacertfile">> => test_key() }) ) end}, @@ -121,8 +121,8 @@ ssl_files_failure_test_() -> }}, emqx_tls_lib:ensure_ssl_files("/tmp", #{ <<"keyfile">> => <<>>, - <<"certfile">> => bin(test_key()), - <<"cacertfile">> => bin(test_key()) + <<"certfile">> => test_key(), + <<"cacertfile">> => test_key() }) ), %% not valid unicode @@ -132,8 +132,8 @@ ssl_files_failure_test_() -> }}, emqx_tls_lib:ensure_ssl_files("/tmp", #{ <<"keyfile">> => <<255, 255>>, - <<"certfile">> => bin(test_key()), - <<"cacertfile">> => bin(test_key()) + <<"certfile">> => test_key(), + <<"cacertfile">> => test_key() }) ), ?assertMatch( @@ -142,9 +142,9 @@ ssl_files_failure_test_() -> which_options := [[<<"ocsp">>, <<"issuer_pem">>]] }}, emqx_tls_lib:ensure_ssl_files("/tmp", #{ - <<"keyfile">> => bin(test_key()), - <<"certfile">> => bin(test_key()), - <<"cacertfile">> => bin(test_key()), + <<"keyfile">> => test_key(), + <<"certfile">> => test_key(), + <<"cacertfile">> => test_key(), <<"ocsp">> => #{<<"issuer_pem">> => <<255, 255>>} }) ), @@ -153,8 +153,8 @@ ssl_files_failure_test_() -> {error, #{reason := invalid_file_path_or_pem_string}}, emqx_tls_lib:ensure_ssl_files("/tmp", #{ <<"keyfile">> => <<33, 22>>, - <<"certfile">> => bin(test_key()), - <<"cacertfile">> => bin(test_key()) + <<"certfile">> => test_key(), + <<"cacertfile">> => test_key() }) ), TmpFile = filename:join("/tmp", integer_to_list(erlang:system_time(microsecond))), @@ -178,63 +178,9 @@ ssl_files_failure_test_() -> end} ]. -ssl_files_save_delete_test() -> - Key = bin(test_key()), - SSL0 = #{ - <<"keyfile">> => Key, - <<"certfile">> => Key, - <<"cacertfile">> => Key, - <<"ocsp">> => #{<<"issuer_pem">> => Key} - }, - Dir = filename:join(["/tmp", "ssl-test-dir"]), - {ok, SSL} = emqx_tls_lib:ensure_ssl_files(Dir, SSL0), - FileKey = maps:get(<<"keyfile">>, SSL), - ?assertMatch(<<"/tmp/ssl-test-dir/key-", _:16/binary>>, FileKey), - ?assertEqual({ok, bin(test_key())}, file:read_file(FileKey)), - FileIssuerPem = emqx_utils_maps:deep_get([<<"ocsp">>, <<"issuer_pem">>], SSL), - ?assertMatch(<<"/tmp/ssl-test-dir/ocsp_issuer_pem-", _:16/binary>>, FileIssuerPem), - ?assertEqual({ok, bin(test_key())}, file:read_file(FileIssuerPem)), - %% no old file to delete - ok = emqx_tls_lib:delete_ssl_files(Dir, SSL, undefined), - ?assertEqual({ok, bin(test_key())}, file:read_file(FileKey)), - ?assertEqual({ok, bin(test_key())}, file:read_file(FileIssuerPem)), - %% old and new identical, no delete - ok = emqx_tls_lib:delete_ssl_files(Dir, SSL, SSL), - ?assertEqual({ok, bin(test_key())}, file:read_file(FileKey)), - ?assertEqual({ok, bin(test_key())}, file:read_file(FileIssuerPem)), - %% new is gone, delete old - ok = emqx_tls_lib:delete_ssl_files(Dir, undefined, SSL), - ?assertEqual({error, enoent}, file:read_file(FileKey)), - ?assertEqual({error, enoent}, file:read_file(FileIssuerPem)), - %% test idempotence - ok = emqx_tls_lib:delete_ssl_files(Dir, undefined, SSL), - ok. - -ssl_files_handle_non_generated_file_test() -> - TmpKeyFile = <<"my-key-file.pem">>, - KeyFileContent = bin(test_key()), - ok = file:write_file(TmpKeyFile, KeyFileContent), - ?assert(filelib:is_regular(TmpKeyFile)), - SSL0 = #{ - <<"keyfile">> => TmpKeyFile, - <<"certfile">> => TmpKeyFile, - <<"cacertfile">> => TmpKeyFile, - <<"ocsp">> => #{<<"issuer_pem">> => TmpKeyFile} - }, - Dir = filename:join(["/tmp", "ssl-test-dir-00"]), - {ok, SSL2} = emqx_tls_lib:ensure_ssl_files(Dir, SSL0), - File1 = maps:get(<<"keyfile">>, SSL2), - %% verify the filename and path is not changed by the emqx_tls_lib - ?assertEqual(TmpKeyFile, File1), - ok = emqx_tls_lib:delete_ssl_files(Dir, undefined, SSL2), - %% verify the file is not delete and not changed, because it is not generated by - %% emqx_tls_lib - ?assertEqual({ok, KeyFileContent}, file:read_file(TmpKeyFile)), - ok = file:delete(TmpKeyFile). - ssl_file_replace_test() -> - Key1 = bin(test_key()), - Key2 = bin(test_key2()), + Key1 = test_key(), + Key2 = test_key2(), SSL0 = #{ <<"keyfile">> => Key1, <<"certfile">> => Key1, @@ -258,32 +204,26 @@ ssl_file_replace_test() -> ?assert(filelib:is_regular(File2)), ?assert(filelib:is_regular(IssuerPem1)), ?assert(filelib:is_regular(IssuerPem2)), - %% 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)), - ?assertNot(filelib:is_regular(IssuerPem1)), - ?assert(filelib:is_regular(IssuerPem2)), ok. bin(X) -> iolist_to_binary(X). test_key() -> - "" - "\n" - "-----BEGIN EC PRIVATE KEY-----\n" - "MHQCAQEEICKTbbathzvD8zvgjL7qRHhW4alS0+j0Loo7WeYX9AxaoAcGBSuBBAAK\n" - "oUQDQgAEJBdF7MIdam5T4YF3JkEyaPKdG64TVWCHwr/plC0QzNVJ67efXwxlVGTo\n" - "ju0VBj6tOX1y6C0U+85VOM0UU5xqvw==\n" - "-----END EC PRIVATE KEY-----\n" - "". + << + "\n" + "-----BEGIN EC PRIVATE KEY-----\n" + "MHQCAQEEICKTbbathzvD8zvgjL7qRHhW4alS0+j0Loo7WeYX9AxaoAcGBSuBBAAK\n" + "oUQDQgAEJBdF7MIdam5T4YF3JkEyaPKdG64TVWCHwr/plC0QzNVJ67efXwxlVGTo\n" + "ju0VBj6tOX1y6C0U+85VOM0UU5xqvw==\n" + "-----END EC PRIVATE KEY-----\n" + >>. test_key2() -> - "" - "\n" - "-----BEGIN EC PRIVATE KEY-----\n" - "MHQCAQEEID9UlIyAlLFw0irkRHX29N+ZGivGtDjlVJvATY3B0TTmoAcGBSuBBAAK\n" - "oUQDQgAEUwiarudRNAT25X11js8gE9G+q0GdsT53QJQjRtBO+rTwuCW1vhLzN0Ve\n" - "AbToUD4JmV9m/XwcSVH06ZaWqNuC5w==\n" - "-----END EC PRIVATE KEY-----\n" - "". + << + "\n" + "-----BEGIN EC PRIVATE KEY-----\n" + "MHQCAQEEID9UlIyAlLFw0irkRHX29N+ZGivGtDjlVJvATY3B0TTmoAcGBSuBBAAK\n" + "oUQDQgAEUwiarudRNAT25X11js8gE9G+q0GdsT53QJQjRtBO+rTwuCW1vhLzN0Ve\n" + "AbToUD4JmV9m/XwcSVH06ZaWqNuC5w==\n" + "-----END EC PRIVATE KEY-----\n" + >>. diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index a8c678be1..278b70d6d 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/apps/emqx_authz/src/emqx_authz.erl @@ -62,8 +62,6 @@ -define(METRICS, [?METRIC_SUPERUSER, ?METRIC_ALLOW, ?METRIC_DENY, ?METRIC_NOMATCH]). --define(IS_ENABLED(Enable), ((Enable =:= true) or (Enable =:= <<"true">>))). - %% Initialize authz backend. %% Populate the passed configuration map with necessary data, %% like `ResourceID`s @@ -266,7 +264,6 @@ ensure_deleted(#{enable := false}, _) -> ensure_deleted(Source, #{clear_metric := ClearMetric}) -> TypeName = type(Source), ensure_resource_deleted(Source), - clear_certs(Source), ClearMetric andalso emqx_metrics_worker:clear_metrics(authz_metrics, TypeName). ensure_resource_deleted(#{type := Type} = Source) -> @@ -530,22 +527,16 @@ write_acl_file(Source) -> acl_conf_file() -> filename:join([emqx:data_dir(), "authz", "acl.conf"]). -maybe_write_certs(#{<<"type">> := Type} = Source) -> - case - emqx_tls_lib:ensure_ssl_files( - ssl_file_path(Type), maps:get(<<"ssl">>, Source, undefined) - ) - of - {ok, SSL} -> - new_ssl_source(Source, SSL); +maybe_write_certs(#{<<"type">> := Type, <<"ssl">> := SSL = #{}} = Source) -> + case emqx_tls_lib:ensure_ssl_files(ssl_file_path(Type), SSL) of + {ok, NSSL} -> + Source#{<<"ssl">> => NSSL}; {error, Reason} -> ?SLOG(error, Reason#{msg => "bad_ssl_config"}), throw({bad_ssl_config, Reason}) - end. - -clear_certs(OldSource) -> - OldSSL = maps:get(ssl, OldSource, undefined), - ok = emqx_tls_lib:delete_ssl_files(ssl_file_path(type(OldSource)), undefined, OldSSL). + end; +maybe_write_certs(#{} = Source) -> + Source. write_file(Filename, Bytes) -> ok = filelib:ensure_dir(Filename), @@ -560,11 +551,6 @@ write_file(Filename, Bytes) -> ssl_file_path(Type) -> filename:join(["authz", Type]). -new_ssl_source(Source, undefined) -> - Source; -new_ssl_source(Source, SSL) -> - Source#{<<"ssl">> => SSL}. - get_source_by_type(Type, Sources) -> {Source, _Front, _Rear} = take(Type, Sources), Source. diff --git a/apps/emqx_bridge/src/emqx_bridge_app.erl b/apps/emqx_bridge/src/emqx_bridge_app.erl index b226b3b32..59c94cef7 100644 --- a/apps/emqx_bridge/src/emqx_bridge_app.erl +++ b/apps/emqx_bridge/src/emqx_bridge_app.erl @@ -69,15 +69,13 @@ pre_config_update(Path, Conf, _OldConfig) when is_map(Conf) -> {ok, ConfNew} end. -post_config_update([bridges, BridgeType, BridgeName] = Path, '$remove', _, OldConf, _AppEnvs) -> - _ = emqx_connector_ssl:clear_certs(filename:join(Path), OldConf), +post_config_update([bridges, BridgeType, BridgeName], '$remove', _, _OldConf, _AppEnvs) -> ok = emqx_bridge_resource:remove(BridgeType, BridgeName), Bridges = emqx_utils_maps:deep_remove([BridgeType, BridgeName], emqx:get_config([bridges])), emqx_bridge:reload_hook(Bridges), ?tp(bridge_post_config_update_done, #{}), ok; -post_config_update([bridges, BridgeType, BridgeName] = Path, _Req, NewConf, undefined, _AppEnvs) -> - _ = emqx_connector_ssl:try_clear_certs(filename:join(Path), NewConf, undefined), +post_config_update([bridges, BridgeType, BridgeName], _Req, NewConf, undefined, _AppEnvs) -> ResOpts = emqx_resource:fetch_creation_opts(NewConf), ok = emqx_bridge_resource:create(BridgeType, BridgeName, NewConf, ResOpts), Bridges = emqx_utils_maps:deep_put( @@ -86,8 +84,7 @@ post_config_update([bridges, BridgeType, BridgeName] = Path, _Req, NewConf, unde emqx_bridge:reload_hook(Bridges), ?tp(bridge_post_config_update_done, #{}), ok; -post_config_update([bridges, BridgeType, BridgeName] = Path, _Req, NewConf, OldConf, _AppEnvs) -> - _ = emqx_connector_ssl:try_clear_certs(filename:join(Path), NewConf, OldConf), +post_config_update([bridges, BridgeType, BridgeName], _Req, NewConf, OldConf, _AppEnvs) -> ResOpts = emqx_resource:fetch_creation_opts(NewConf), ok = emqx_bridge_resource:update(BridgeType, BridgeName, {OldConf, NewConf}, ResOpts), Bridges = emqx_utils_maps:deep_put( diff --git a/apps/emqx_bridge/src/emqx_bridge_resource.erl b/apps/emqx_bridge/src/emqx_bridge_resource.erl index d7234a6bf..f927ef750 100644 --- a/apps/emqx_bridge/src/emqx_bridge_resource.erl +++ b/apps/emqx_bridge/src/emqx_bridge_resource.erl @@ -247,8 +247,7 @@ recreate(Type, Name, Conf, Opts) -> ). create_dry_run(Type, Conf0) -> - TmpPath0 = iolist_to_binary([?TEST_ID_PREFIX, emqx_utils:gen_id(8)]), - TmpPath = emqx_utils:safe_filename(TmpPath0), + TmpPath = emqx_utils:safe_filename(?TEST_ID_PREFIX ++ emqx_utils:gen_id(8)), Conf = emqx_utils_maps:safe_atom_key_map(Conf0), case emqx_connector_ssl:convert_certs(TmpPath, Conf) of {error, Reason} -> @@ -265,7 +264,7 @@ create_dry_run(Type, Conf0) -> throw:Reason -> {error, Reason} after - _ = maybe_clear_certs(TmpPath, ConfNew) + _ = file:del_dir_r(emqx_tls_lib:pem_dir(TmpPath)) end end. @@ -285,27 +284,6 @@ remove(Type, Name, _Conf, _Opts) -> {error, Reason} -> {error, Reason} end. -maybe_clear_certs(TmpPath, #{ssl := SslConf} = Conf) -> - %% don't remove the cert files if they are in use - case is_tmp_path_conf(TmpPath, SslConf) of - true -> emqx_connector_ssl:clear_certs(TmpPath, Conf); - false -> ok - end; -maybe_clear_certs(_TmpPath, _ConfWithoutSsl) -> - ok. - -is_tmp_path_conf(TmpPath, #{certfile := Certfile}) -> - is_tmp_path(TmpPath, Certfile); -is_tmp_path_conf(TmpPath, #{keyfile := Keyfile}) -> - is_tmp_path(TmpPath, Keyfile); -is_tmp_path_conf(TmpPath, #{cacertfile := CaCertfile}) -> - is_tmp_path(TmpPath, CaCertfile); -is_tmp_path_conf(_TmpPath, _Conf) -> - false. - -is_tmp_path(TmpPath, File) -> - string:str(str(File), str(TmpPath)) > 0. - %% convert bridge configs to what the connector modules want parse_confs( <<"webhook">>, @@ -412,9 +390,6 @@ parse_url(Url) -> invalid_data(<<"Missing scheme in URL: ", Url/binary>>) end. -str(Bin) when is_binary(Bin) -> binary_to_list(Bin); -str(Str) when is_list(Str) -> Str. - bin(Bin) when is_binary(Bin) -> Bin; bin(Str) when is_list(Str) -> list_to_binary(Str); bin(Atom) when is_atom(Atom) -> atom_to_binary(Atom, utf8). diff --git a/apps/emqx_connector/src/emqx_connector_ssl.erl b/apps/emqx_connector/src/emqx_connector_ssl.erl index e07d95d51..44c0dbc27 100644 --- a/apps/emqx_connector/src/emqx_connector_ssl.erl +++ b/apps/emqx_connector/src/emqx_connector_ssl.erl @@ -19,9 +19,7 @@ -include_lib("emqx/include/logger.hrl"). -export([ - convert_certs/2, - clear_certs/2, - try_clear_certs/3 + convert_certs/2 ]). convert_certs(RltvDir, #{<<"ssl">> := SSL} = Config) -> @@ -32,26 +30,6 @@ convert_certs(RltvDir, #{ssl := SSL} = Config) -> convert_certs(_RltvDir, Config) -> {ok, Config}. -clear_certs(RltvDir, Config) -> - clear_certs2(RltvDir, normalize_key_to_bin(Config)). - -clear_certs2(RltvDir, #{<<"ssl">> := OldSSL} = _Config) -> - ok = emqx_tls_lib:delete_ssl_files(RltvDir, undefined, OldSSL); -clear_certs2(_RltvDir, _) -> - ok. - -try_clear_certs(RltvDir, NewConf, OldConf) -> - try_clear_certs2( - RltvDir, - normalize_key_to_bin(NewConf), - normalize_key_to_bin(OldConf) - ). - -try_clear_certs2(RltvDir, NewConf, OldConf) -> - NewSSL = try_map_get(<<"ssl">>, NewConf, undefined), - OldSSL = try_map_get(<<"ssl">>, OldConf, undefined), - ok = emqx_tls_lib:delete_ssl_files(RltvDir, NewSSL, OldSSL). - new_ssl_config(RltvDir, Config, SSL) -> case emqx_tls_lib:ensure_ssl_files(RltvDir, SSL) of {ok, NewSSL} -> @@ -70,13 +48,3 @@ new_ssl_config(#{<<"ssl">> := _} = Config, NewSSL) -> Config#{<<"ssl">> => NewSSL}; new_ssl_config(Config, _NewSSL) -> Config. - -normalize_key_to_bin(undefined) -> - undefined; -normalize_key_to_bin(Map) when is_map(Map) -> - emqx_utils_maps:binary_key_map(Map). - -try_map_get(Key, Map, Default) when is_map(Map) -> - maps:get(Key, Map, Default); -try_map_get(_Key, undefined, Default) -> - Default. diff --git a/apps/emqx_exhook/src/emqx_exhook_mgr.erl b/apps/emqx_exhook/src/emqx_exhook_mgr.erl index 0647c80ea..22e03d967 100644 --- a/apps/emqx_exhook/src/emqx_exhook_mgr.erl +++ b/apps/emqx_exhook/src/emqx_exhook_mgr.erl @@ -183,9 +183,8 @@ pre_config_update(_, {enable, Name, Enable}, OldConf) -> NewConf -> {ok, lists:map(fun maybe_write_certs/1, NewConf)} end. -post_config_update(_KeyPath, UpdateReq, NewConf, OldConf, _AppEnvs) -> +post_config_update(_KeyPath, UpdateReq, NewConf, _OldConf, _AppEnvs) -> Result = call({update_config, UpdateReq, NewConf}), - try_clear_ssl_files(UpdateReq, NewConf, OldConf), {ok, Result}. %%===================================================================== @@ -602,34 +601,3 @@ new_ssl_source(Source, undefined) -> Source; new_ssl_source(Source, SSL) -> Source#{<<"ssl">> => SSL}. - -try_clear_ssl_files({delete, Name}, _NewConf, OldConfs) -> - OldSSL = find_server_ssl_cfg(Name, OldConfs), - emqx_tls_lib:delete_ssl_files(ssl_file_path(Name), undefined, OldSSL); -try_clear_ssl_files({Op, Name, _}, NewConfs, OldConfs) when - Op =:= update; Op =:= enable --> - NewSSL = find_server_ssl_cfg(Name, NewConfs), - OldSSL = find_server_ssl_cfg(Name, OldConfs), - emqx_tls_lib:delete_ssl_files(ssl_file_path(Name), NewSSL, OldSSL); -try_clear_ssl_files(_Req, _NewConf, _OldConf) -> - ok. - -search_server_cfg(Name, Confs) -> - lists:search( - fun - (#{name := SvrName}) when SvrName =:= Name -> - true; - (_) -> - false - end, - Confs - ). - -find_server_ssl_cfg(Name, Confs) -> - case search_server_cfg(Name, Confs) of - {value, Value} -> - maps:get(ssl, Value, undefined); - false -> - undefined - end. diff --git a/apps/emqx_gateway/src/emqx_gateway_conf.erl b/apps/emqx_gateway/src/emqx_gateway_conf.erl index da86d6a58..b5d3d44b0 100644 --- a/apps/emqx_gateway/src/emqx_gateway_conf.erl +++ b/apps/emqx_gateway/src/emqx_gateway_conf.erl @@ -392,11 +392,6 @@ pre_config_update(_, {update_gateway, GwName, Conf}, RawConf) -> {ok, emqx_utils_maps:deep_put([GwName], RawConf, NConf1)} end; pre_config_update(_, {unload_gateway, GwName}, RawConf) -> - _ = tune_gw_certs( - fun clear_certs/2, - GwName, - maps:get(GwName, RawConf, #{}) - ), {ok, maps:remove(GwName, RawConf)}; pre_config_update(_, {add_listener, GwName, {LType, LName}, Conf}, RawConf) -> case @@ -423,8 +418,8 @@ pre_config_update(_, {update_listener, GwName, {LType, LName}, Conf}, RawConf) - of undefined -> badres_listener(not_found, GwName, LType, LName); - OldConf -> - NConf = convert_certs(certs_dir(GwName), Conf, OldConf), + _OldConf -> + NConf = convert_certs(certs_dir(GwName), Conf), NRawConf = emqx_utils_maps:deep_put( [GwName, <<"listeners">>, LType, LName], RawConf, @@ -437,8 +432,7 @@ pre_config_update(_, {remove_listener, GwName, {LType, LName}}, RawConf) -> case emqx_utils_maps:deep_get(Path, RawConf, undefined) of undefined -> {ok, RawConf}; - OldConf -> - clear_certs(certs_dir(GwName), OldConf), + _OldConf -> {ok, emqx_utils_maps:deep_remove(Path, RawConf)} end; pre_config_update(_, {add_authn, GwName, Conf}, RawConf) -> @@ -487,26 +481,18 @@ pre_config_update(_, {add_authn, GwName, {LType, LName}, Conf}, RawConf) -> end end; pre_config_update(_, {update_authn, GwName, Conf}, RawConf) -> - case - emqx_utils_maps:deep_get( - [GwName, ?AUTHN_BIN], RawConf, undefined - ) - of + Path = [GwName, ?AUTHN_BIN], + case emqx_utils_maps:deep_get(Path, RawConf, undefined) of undefined -> badres_authn(not_found, GwName); - OldAuthnConf -> + _OldConf -> CertsDir = authn_certs_dir(GwName, Conf), - Conf1 = emqx_authentication_config:convert_certs(CertsDir, Conf, OldAuthnConf), - {ok, emqx_utils_maps:deep_put([GwName, ?AUTHN_BIN], RawConf, Conf1)} + Conf1 = emqx_authentication_config:convert_certs(CertsDir, Conf), + {ok, emqx_utils_maps:deep_put(Path, RawConf, Conf1)} end; pre_config_update(_, {update_authn, GwName, {LType, LName}, Conf}, RawConf) -> - case - emqx_utils_maps:deep_get( - [GwName, <<"listeners">>, LType, LName], - RawConf, - undefined - ) - of + Path = [GwName, <<"listeners">>, LType, LName], + case emqx_utils_maps:deep_get(Path, RawConf, undefined) of undefined -> badres_listener(not_found, GwName, LType, LName); Listener -> @@ -515,55 +501,20 @@ pre_config_update(_, {update_authn, GwName, {LType, LName}, Conf}, RawConf) -> badres_listener_authn(not_found, GwName, LType, LName); OldAuthnConf -> CertsDir = authn_certs_dir(GwName, LType, LName, OldAuthnConf), - Conf1 = emqx_authentication_config:convert_certs( - CertsDir, - Conf, - OldAuthnConf - ), + Conf1 = emqx_authentication_config:convert_certs(CertsDir, Conf), NListener = maps:put( ?AUTHN_BIN, Conf1, Listener ), - {ok, - emqx_utils_maps:deep_put( - [GwName, <<"listeners">>, LType, LName], - RawConf, - NListener - )} + {ok, emqx_utils_maps:deep_put(Path, RawConf, NListener)} end end; pre_config_update(_, {remove_authn, GwName}, RawConf) -> - case - emqx_utils_maps:deep_get( - [GwName, ?AUTHN_BIN], RawConf, undefined - ) - of - undefined -> - ok; - OldAuthnConf -> - CertsDir = authn_certs_dir(GwName, OldAuthnConf), - emqx_authentication_config:clear_certs(CertsDir, OldAuthnConf) - end, - {ok, - emqx_utils_maps:deep_remove( - [GwName, ?AUTHN_BIN], RawConf - )}; + Path = [GwName, ?AUTHN_BIN], + {ok, emqx_utils_maps:deep_remove(Path, RawConf)}; pre_config_update(_, {remove_authn, GwName, {LType, LName}}, RawConf) -> Path = [GwName, <<"listeners">>, LType, LName, ?AUTHN_BIN], - case - emqx_utils_maps:deep_get( - Path, - RawConf, - undefined - ) - of - undefined -> - ok; - OldAuthnConf -> - CertsDir = authn_certs_dir(GwName, LType, LName, OldAuthnConf), - emqx_authentication_config:clear_certs(CertsDir, OldAuthnConf) - end, {ok, emqx_utils_maps:deep_remove(Path, RawConf)}; pre_config_update(_, UnknownReq, _RawConf) -> logger:error("Unknown configuration update request: ~0p", [UnknownReq]), @@ -729,43 +680,14 @@ authn_certs_dir(GwName, AuthnConf) -> convert_certs(SubDir, Conf) -> convert_certs(<<"dtls_options">>, SubDir, convert_certs(<<"ssl_options">>, SubDir, Conf)). -convert_certs(Type, SubDir, Conf) when ?IS_SSL(Type) -> - case - emqx_tls_lib:ensure_ssl_files( - SubDir, - maps:get(Type, Conf, undefined) - ) - of - {ok, SSL} -> - new_ssl_config(Type, Conf, SSL); - {error, Reason} -> - ?SLOG(error, Reason#{msg => bad_ssl_config}), - throw({bad_ssl_config, Reason}) - end; -convert_certs(SubDir, NConf, OConf) when is_map(NConf); is_map(OConf) -> - convert_certs( - <<"dtls_options">>, SubDir, convert_certs(<<"ssl_options">>, SubDir, NConf, OConf), OConf - ). - -convert_certs(Type, SubDir, NConf, OConf) when ?IS_SSL(Type) -> - OSSL = maps:get(Type, OConf, undefined), - NSSL = maps:get(Type, NConf, undefined), - case emqx_tls_lib:ensure_ssl_files(SubDir, NSSL) of - {ok, NSSL1} -> - ok = emqx_tls_lib:delete_ssl_files(SubDir, NSSL1, OSSL), - new_ssl_config(Type, NConf, NSSL1); +convert_certs(Type, SubDir, Conf) -> + SSL = maps:get(Type, Conf, undefined), + case is_map(SSL) andalso emqx_tls_lib:ensure_ssl_files(SubDir, SSL) of + false -> + Conf; + {ok, NSSL = #{}} -> + Conf#{Type => NSSL}; {error, Reason} -> ?SLOG(error, Reason#{msg => bad_ssl_config}), throw({bad_ssl_config, Reason}) end. - -new_ssl_config(_Type, Conf, undefined) -> Conf; -new_ssl_config(Type, Conf, SSL) when ?IS_SSL(Type) -> Conf#{Type => SSL}. - -clear_certs(SubDir, Conf) -> - clear_certs(<<"ssl_options">>, SubDir, Conf), - clear_certs(<<"dtls_options">>, SubDir, Conf). - -clear_certs(Type, SubDir, Conf) when ?IS_SSL(Type) -> - SSL = maps:get(Type, Conf, undefined), - ok = emqx_tls_lib:delete_ssl_files(SubDir, undefined, SSL). From 99ea9b86c2161a58282f5f05ab386b1718dede4b Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Fri, 2 Jun 2023 16:25:52 +0300 Subject: [PATCH 04/16] feat(tlslib): add separate managed certfiles GC process Which periodically inpects managed certificates directory and tries to collect "orphans" here, in other words files that aren't referenced anywhere in the current emqx config. --- apps/emqx/src/emqx_kernel_sup.erl | 3 +- apps/emqx/src/emqx_tls_certfile_gc.erl | 335 ++++++++++++++ apps/emqx/src/emqx_tls_lib.erl | 3 +- apps/emqx/src/emqx_tls_lib_sup.erl | 49 ++ apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl | 430 ++++++++++++++++++ apps/emqx_bridge/src/emqx_bridge_resource.erl | 10 +- apps/emqx_bridge/test/emqx_bridge_SUITE.erl | 16 +- apps/emqx_exhook/test/emqx_exhook_SUITE.erl | 19 +- .../test/emqx_gateway_conf_SUITE.erl | 1 + .../test/emqx_mgmt_api_listeners_SUITE.erl | 11 +- apps/emqx_utils/src/emqx_utils.erl | 5 + apps/emqx_utils/src/emqx_utils_fs.erl | 80 ++++ apps/emqx_utils/test/emqx_utils_fs_SUITE.erl | 98 ++++ .../emqx_utils_fs_SUITE_data/nonempty/d1/1 | 0 .../emqx_utils_fs_SUITE_data/nonempty/d1/2 | 0 .../nonempty/d1/mutrec | 1 + .../emqx_utils_fs_SUITE_data/nonempty/d1/файл | 0 .../nonempty/d2/deep/down/here | 0 .../nonempty/d2/deep/mutrec | 1 + .../test/emqx_utils_fs_SUITE_data/nonempty/д3 | 1 + 20 files changed, 1023 insertions(+), 40 deletions(-) create mode 100644 apps/emqx/src/emqx_tls_certfile_gc.erl create mode 100644 apps/emqx/src/emqx_tls_lib_sup.erl create mode 100644 apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl create mode 100644 apps/emqx_utils/src/emqx_utils_fs.erl create mode 100644 apps/emqx_utils/test/emqx_utils_fs_SUITE.erl create mode 100644 apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/1 create mode 100644 apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/2 create mode 120000 apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/mutrec create mode 100644 apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/файл create mode 100644 apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d2/deep/down/here create mode 120000 apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d2/deep/mutrec create mode 120000 apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/д3 diff --git a/apps/emqx/src/emqx_kernel_sup.erl b/apps/emqx/src/emqx_kernel_sup.erl index 1027ef639..45451084a 100644 --- a/apps/emqx/src/emqx_kernel_sup.erl +++ b/apps/emqx/src/emqx_kernel_sup.erl @@ -37,7 +37,8 @@ init([]) -> child_spec(emqx_metrics, worker), child_spec(emqx_authn_authz_metrics_sup, supervisor), child_spec(emqx_ocsp_cache, worker), - child_spec(emqx_crl_cache, worker) + child_spec(emqx_crl_cache, worker), + child_spec(emqx_tls_lib_sup, supervisor) ] }}. diff --git a/apps/emqx/src/emqx_tls_certfile_gc.erl b/apps/emqx/src/emqx_tls_certfile_gc.erl new file mode 100644 index 000000000..ed0c2c083 --- /dev/null +++ b/apps/emqx/src/emqx_tls_certfile_gc.erl @@ -0,0 +1,335 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +%% @doc Orphaned TLS certificates / keyfiles garbage collector +%% +%% This module is a worker process that periodically scans the mutable +%% certificates directory and removes any files that are not referenced +%% by _any_ TLS configuration in _any_ of the config roots. Such files +%% are called "orphans". +%% +%% In order to ensure safety, GC considers a file to be a candidate for +%% deletion (a "convict") only if it was considered an orphan twice in +%% a row. This should help avoid deleting files that are not yet in the +%% config but was already materialized on disk (e.g. during +%% `pre_config_update/3`). + +-module(emqx_tls_certfile_gc). + +-include_lib("kernel/include/file.hrl"). +-include_lib("emqx/include/logger.hrl"). +-include_lib("snabbkaffe/include/trace.hrl"). + +%% API +-export([ + start_link/0, + start_link/1, + run/0, + force/0 +]). + +-behaviour(gen_server). +-export([ + init/1, + handle_call/3, + handle_cast/2, + handle_info/2 +]). + +%% Testing & maintenance +-export([ + orphans/0, + orphans/1, + convicts/2, + collect_files/2 +]). + +-define(GC_INVERVAL, 5 * 60 * 1000). + +-define(HAS_OWNER_READ(Mode), ((Mode band 8#00400) > 0)). +-define(HAS_OWNER_WRITE(Mode), ((Mode band 8#00200) > 0)). + +-type filename() :: string(). +-type fileinfo() :: #file_info{}. + +-type st() :: #{ + orphans => #{filename() => fileinfo()}, + gc_interval => pos_integer(), + next_gc_timer => reference() +}. + +-type event() :: + {collect, filename(), ok | {error, file:posix()}}. + +%%-------------------------------------------------------------------- +%% API +%%-------------------------------------------------------------------- + +-spec start_link() -> + {ok, pid()}. +start_link() -> + start_link(?GC_INVERVAL). + +-spec start_link(_Interval :: pos_integer()) -> + {ok, pid()}. +start_link(Interval) -> + gen_server:start_link({local, ?MODULE}, ?MODULE, Interval, []). + +-spec run() -> + {ok, _Events :: [event()]}. +run() -> + gen_server:call(?MODULE, collect). + +-spec force() -> + {ok, _Events :: [event()]}. +force() -> + % NOTE + % Simulate a complete GC cycle by running it twice. Mostly useful in tests. + {ok, Events1} = run(), + {ok, Events2} = run(), + {ok, Events1 ++ Events2}. + +%%-------------------------------------------------------------------- +%% Supervisor callbacks +%%-------------------------------------------------------------------- + +-spec init(_) -> + {ok, st()}. +init(Interval) -> + {ok, start_timer(#{gc_interval => Interval})}. + +-spec handle_call(collect | _Call, gen_server:from(), st()) -> + {reply, {ok, [event()]}, st()} | {noreply, st()}. +handle_call(collect, From, St) -> + {ok, Events, StNext} = ?tp_span( + tls_certfile_gc_manual, + #{caller => From}, + collect(St, #{evhandler => {fun emqx_utils:cons/2, []}}) + ), + {reply, {ok, Events}, restart_timer(StNext)}; +handle_call(Call, From, St) -> + ?SLOG(error, #{msg => "unexpected_call", call => Call, from => From}), + {noreply, St}. + +-spec handle_cast(_Cast, st()) -> + {noreply, st()}. +handle_cast(Cast, State) -> + ?SLOG(error, #{msg => "unexpected_cast", cast => Cast}), + {noreply, State}. + +-spec handle_info({timeout, reference(), collect}, st()) -> + {noreply, st()}. +handle_info({timeout, TRef, collect}, St = #{next_gc_timer := TRef}) -> + {ok, _, StNext} = ?tp_span( + tls_certfile_gc_periodic, + #{}, + collect(St, #{evhandler => {fun log_event/2, []}}) + ), + {noreply, start_timer(StNext)}. + +start_timer(St = #{gc_interval := Interval}) -> + TRef = erlang:start_timer(Interval, self(), collect), + St#{next_gc_timer => TRef}. + +restart_timer(St = #{next_gc_timer := TRef}) -> + ok = emqx_utils:cancel_timer(TRef), + start_timer(St). + +log_event({collect, Filename, ok}, _) -> + ?tp(debug, "tls_certfile_gc_collected", #{filename => Filename}); +log_event({collect, Filename, {error, Reason}}, _) -> + ?tp(warning, "tls_certfile_gc_collect_error", #{filename => Filename, reason => Reason}). + +%%-------------------------------------------------------------------- +%% Internal functions +%% ------------------------------------------------------------------- + +collect(St, Opts) -> + RootDir = emqx_utils_fs:canonicalize(emqx:mutable_certs_dir()), + Orphans = orphans(RootDir), + OrphansLast = maps:get(orphans, St, #{}), + Convicts = convicts(Orphans, OrphansLast), + Result = collect_files(Convicts, RootDir, Opts), + {ok, Result, St#{orphans => maps:without(Convicts, Orphans)}}. + +orphans() -> + Dir = emqx_utils_fs:canonicalize(emqx:mutable_certs_dir()), + orphans(Dir). + +orphans(Dir) -> + Certfiles = find_managed_files(fun is_managed_file/2, Dir), + lists:foldl( + fun(Root, Acc) -> + References = find_references(Root), + maps:without(References, Acc) + end, + Certfiles, + emqx_config:get_root_names() + ). + +convicts(Orphans, OrphansLast) -> + maps:fold( + fun(AbsPath, #file_info{mtime = MTime, inode = Inode}, Acc) -> + case maps:get(AbsPath, Orphans, undefined) of + #file_info{mtime = MTime, inode = Inode} -> + % Certfile was not changed / recreated in the meantime + [AbsPath | Acc]; + _ -> + % Certfile was changed / created / recreated in the meantime + Acc + end + end, + [], + OrphansLast + ). + +find_managed_files(Filter, Dir) -> + emqx_utils_fs:traverse_dir( + fun + (AbsPath, Info = #file_info{}, Acc) -> + case Filter(AbsPath, Info) of + true -> Acc#{AbsPath => Info}; + false -> Acc + end; + (AbsPath, {error, Reason}, Acc) -> + ?SLOG(notice, "filesystem_object_inaccessible", #{ + abspath => AbsPath, + reason => Reason + }), + Acc + end, + #{}, + Dir + ). + +is_managed_file(AbsPath, #file_info{type = Type, mode = Mode}) -> + % NOTE + % We consider a certfile is managed if: this is a regular file, owner has RW permission + % and the filename looks like a managed filename. + Type == regular andalso + ?HAS_OWNER_READ(Mode) andalso + ?HAS_OWNER_WRITE(Mode) andalso + emqx_tls_lib:is_managed_ssl_file(AbsPath). + +find_references(Root) -> + Config = emqx_config:get_raw([Root]), + fold_config( + fun(Stack, Value, Acc) -> + case is_file_reference(Stack) andalso is_string(Value) of + true -> + Filename = emqx_schema:naive_env_interpolation(Value), + {stop, [emqx_utils_fs:canonicalize(Filename) | Acc]}; + false -> + {cont, Acc} + end + end, + [], + Config + ). + +is_file_reference([<<"keyfile">> | _]) -> true; +is_file_reference([<<"certfile">> | _]) -> true; +is_file_reference([<<"cacertfile">> | _]) -> true; +is_file_reference([<<"issuer_pem">>, <<"ocsp">> | _]) -> true; +is_file_reference(_) -> false. + +is_string(Value) -> + is_list(Value) orelse is_binary(Value). + +%% + +fold_config(FoldFun, AccIn, Config) -> + fold_config(FoldFun, AccIn, [], Config). + +fold_config(FoldFun, AccIn, Stack, Config) when is_map(Config) -> + maps:fold( + fun(K, SubConfig, Acc) -> + fold_subconf(FoldFun, Acc, [K | Stack], SubConfig) + end, + AccIn, + Config + ); +fold_config(FoldFun, Acc, Stack, []) -> + fold_confval(FoldFun, Acc, Stack, []); +fold_config(FoldFun, Acc, Stack, String = [C | _]) when is_integer(C), C >= 0, C < 16#10FFFF -> + fold_confval(FoldFun, Acc, Stack, String); +fold_config(FoldFun, Acc, Stack, Config) when is_list(Config) -> + fold_confarray(FoldFun, Acc, Stack, 1, Config); +fold_config(FoldFun, Acc, Stack, Config) -> + fold_confval(FoldFun, Acc, Stack, Config). + +fold_confarray(FoldFun, AccIn, StackIn, I, [H | T]) -> + Stack = [I | StackIn], + case FoldFun(Stack, H, AccIn) of + {cont, Acc} -> + AccOut = fold_config(FoldFun, Acc, Stack, H), + fold_confarray(FoldFun, AccOut, StackIn, I + 1, T); + {stop, Acc} -> + fold_confarray(FoldFun, Acc, StackIn, I + 1, T) + end; +fold_confarray(_FoldFun, Acc, _Stack, _, []) -> + Acc. + +fold_subconf(FoldFun, AccIn, Stack, SubConfig) -> + case FoldFun(Stack, SubConfig, AccIn) of + {cont, Acc} -> + fold_config(FoldFun, Acc, Stack, SubConfig); + {stop, Acc} -> + Acc + end. + +fold_confval(FoldFun, AccIn, Stack, ConfVal) -> + case FoldFun(Stack, ConfVal, AccIn) of + {_, Acc} -> + Acc + end. + +%% + +-spec collect_files([filename()], filename()) -> + [event()]. +collect_files(Filenames, RootDir) -> + collect_files(Filenames, RootDir, #{evhandler => {fun emqx_utils:cons/2, []}}). + +collect_files(Filenames, RootDir, Opts) -> + {Handler, AccIn} = maps:get(evhandler, Opts), + lists:foldl( + fun(Filename, Acc) -> collect_file(Filename, RootDir, Handler, Acc) end, + AccIn, + Filenames + ). + +collect_file(Filename, RootDir, Handler, AccIn) -> + case file:delete(Filename) of + ok -> + Acc = Handler({collect, Filename, ok}, AccIn), + collect_parents(filename:dirname(Filename), RootDir, Handler, Acc); + {error, _} = Error -> + Handler({collect, Filename, Error}, AccIn) + end. + +collect_parents(RootDir, RootDir, _Handler, Acc) -> + Acc; +collect_parents(ParentDir, RootDir, Handler, AccIn) -> + case file:del_dir(ParentDir) of + ok -> + Acc = Handler({collect, ParentDir, ok}, AccIn), + collect_parents(filename:dirname(ParentDir), RootDir, Handler, Acc); + {error, eexist} -> + AccIn; + {error, _} = Error -> + Handler({collect, ParentDir, Error}, AccIn) + end. diff --git a/apps/emqx/src/emqx_tls_lib.erl b/apps/emqx/src/emqx_tls_lib.erl index a7b63382c..c05e0d092 100644 --- a/apps/emqx/src/emqx_tls_lib.erl +++ b/apps/emqx/src/emqx_tls_lib.erl @@ -32,6 +32,7 @@ ensure_ssl_files/3, drop_invalid_certs/1, pem_dir/1, + is_managed_ssl_file/1, is_valid_pem_file/1, is_pem/1 ]). @@ -400,7 +401,7 @@ save_pem_file(Dir, KeyPath, Pem, DryRun) -> %% 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 -is_generated_file(Filename) -> +is_managed_ssl_file(Filename) -> case string:split(filename:basename(Filename), "-") of [_Name, Suffix] -> is_hex_str(Suffix); _ -> false diff --git a/apps/emqx/src/emqx_tls_lib_sup.erl b/apps/emqx/src/emqx_tls_lib_sup.erl new file mode 100644 index 000000000..db64d218e --- /dev/null +++ b/apps/emqx/src/emqx_tls_lib_sup.erl @@ -0,0 +1,49 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +-module(emqx_tls_lib_sup). + +-behaviour(supervisor). + +-export([start_link/0]). + +-export([init/1]). + +%%-------------------------------------------------------------------- +%% API +%%-------------------------------------------------------------------- + +start_link() -> + supervisor:start_link({local, ?MODULE}, ?MODULE, []). + +%%-------------------------------------------------------------------- +%% Supervisor callbacks +%%-------------------------------------------------------------------- + +init([]) -> + SupFlags = #{ + strategy => one_for_one, + intensity => 100, + period => 10 + }, + GC = #{ + id => emqx_tls_certfile_gc, + start => {emqx_tls_certfile_gc, start_link, []}, + restart => permanent, + shutdown => brutal_kill, + type => worker + }, + {ok, {SupFlags, [GC]}}. diff --git a/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl b/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl new file mode 100644 index 000000000..3b0d0cf0c --- /dev/null +++ b/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl @@ -0,0 +1,430 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2021-2023 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +-module(emqx_tls_certfile_gc_SUITE). + +-compile([export_all, nowarn_export_all]). + +-include_lib("typerefl/include/types.hrl"). +-include_lib("eunit/include/eunit.hrl"). +-include_lib("common_test/include/ct.hrl"). + +-include_lib("snabbkaffe/include/test_macros.hrl"). + +-define(of_pid(PID, EVENTS), [E || #{?snk_meta := #{pid := __Pid}} = E <- EVENTS, __Pid == PID]). + +-import(hoconsc, [mk/1, mk/2, ref/2]). + +all() -> + emqx_common_test_helpers:all(?MODULE). + +init_per_suite(Config) -> + _ = application:load(emqx), + ok = application:set_env(emqx, data_dir, ?config(priv_dir, Config)), + ok = emqx_config:save_schema_mod_and_names(?MODULE), + Config. + +end_per_suite(_Config) -> + emqx_config:erase_all(). + +init_per_testcase(TC, Config) -> + TCAbsDir = filename:join(?config(priv_dir, Config), TC), + ok = application:set_env(emqx, data_dir, TCAbsDir), + ok = snabbkaffe:start_trace(), + [{tc_name, TC}, {tc_absdir, TCAbsDir} | Config]. + +end_per_testcase(_TC, Config) -> + ok = snabbkaffe:stop(), + ok = application:set_env(emqx, data_dir, ?config(priv_dir, Config)), + ok. + +t_no_orphans(Config) -> + SSL0 = #{ + <<"keyfile">> => key(), + <<"certfile">> => cert(), + <<"cacertfile">> => cert() + }, + {ok, SSL} = emqx_tls_lib:ensure_ssl_files("ssl", SSL0), + {ok, SSLUnused} = emqx_tls_lib:ensure_ssl_files("unused", SSL0), + SSLKeyfile = maps:get(<<"keyfile">>, SSL), + ok = load_config(#{ + <<"clients">> => [ + #{<<"transport">> => #{<<"ssl">> => SSL}} + ], + <<"servers">> => #{ + <<"noname">> => #{<<"ssl">> => SSL} + } + }), + % Should not be considered an orphan: it's a symlink. + ok = file:make_symlink( + SSLKeyfile, + filename:join(?config(tc_absdir, Config), filename:basename(SSLKeyfile)) + ), + % Should not be considered orphans: files are now read-only. + ok = file:change_mode(maps:get(<<"keyfile">>, SSLUnused), 8#400), + ok = file:change_mode(maps:get(<<"certfile">>, SSLUnused), 8#400), + ok = file:change_mode(maps:get(<<"cacertfile">>, SSLUnused), 8#400), + % Verify there are no orphans + ?assertEqual( + #{}, + orphans() + ), + % Verify there are no orphans, since SSL config is still in use + ok = put_config([<<"servers">>, <<"noname">>, <<"ssl">>], #{<<"enable">> => false}), + ?assertEqual( + #{}, + orphans() + ). + +t_collect_orphans(_Config) -> + % 0. Set up a client and two servers (each with the same set of certfiles). + SSL0 = #{ + <<"keyfile">> => key(), + <<"certfile">> => cert(), + <<"cacertfile">> => cert() + }, + SSL1 = SSL0#{ + <<"ocsp">> => #{<<"issuer_pem">> => cert()} + }, + {ok, SSL2} = emqx_tls_lib:ensure_ssl_files("client", SSL0), + {ok, SSL3} = emqx_tls_lib:ensure_ssl_files("server", SSL1), + ok = load_config(#{ + <<"clients">> => [ + #{<<"transport">> => #{<<"ssl">> => SSL2}} + ], + <<"servers">> => #{ + <<"name">> => #{<<"ssl">> => SSL3}, + <<"noname">> => #{<<"ssl">> => SSL3} + } + }), + Orphans1 = orphans(), + ?assertEqual( + #{}, + Orphans1 + ), + % 1. Remove clients from the config + ok = put_config([<<"clients">>], []), + Orphans2 = orphans(), + ?assertMatch( + M = #{} when map_size(M) == 3, + Orphans2 + ), + % All orphans are newly observed, nothing to collect + ?assertEqual( + [], + collect(convicts(Orphans2, Orphans1)) + ), + % Same orphans are "observed", should be collected + ?assertMatch( + [ + {collect, _DirClient, ok}, + {collect, _CACert, ok}, + {collect, _Cert, ok}, + {collect, _Key, ok} + ], + collect(convicts(Orphans2, Orphans2)) + ), + % 2. Remove server from the config + ok = put_config([<<"servers">>, <<"name">>, <<"ssl">>], #{}), + Orphans3 = orphans(), + % Files are still referenced by the "noname" server + ?assertEqual( + #{}, + Orphans3 + ), + % 3. Remove another server from the config + ok = put_config([<<"servers">>, <<"noname">>, <<"ssl">>], #{}), + Orphans4 = orphans(), + ?assertMatch( + M = #{} when map_size(M) == 4, + Orphans4 + ), + ?assertMatch( + [ + {collect, _DirServer, ok}, + {collect, _IssuerPEM, ok}, + {collect, _CACert, ok}, + {collect, _Cert, ok}, + {collect, _Key, ok} + ], + collect(convicts(Orphans4, Orphans4)) + ), + % No more orphans left + ?assertEqual( + #{}, + orphans() + ). + +t_gc_runs_periodically(_Config) -> + {ok, Pid} = emqx_tls_certfile_gc:start_link(500), + + % Set up two servers in the config, each with its own set of certfiles + SSL = #{ + <<"keyfile">> => key(), + <<"certfile">> => cert() + }, + {ok, SSL1} = emqx_tls_lib:ensure_ssl_files("s1", SSL), + SSL1Keyfile = emqx_utils_fs:canonicalize(maps:get(<<"keyfile">>, SSL1)), + SSL1Certfile = emqx_utils_fs:canonicalize(maps:get(<<"certfile">>, SSL1)), + {ok, SSL2} = emqx_tls_lib:ensure_ssl_files("s2", SSL#{ + <<"ocsp">> => #{<<"issuer_pem">> => cert()} + }), + SSL2Keyfile = emqx_utils_fs:canonicalize(maps:get(<<"keyfile">>, SSL2)), + SSL2Certfile = emqx_utils_fs:canonicalize(maps:get(<<"certfile">>, SSL2)), + SSL2IssPEM = emqx_utils_fs:canonicalize( + emqx_utils_maps:deep_get([<<"ocsp">>, <<"issuer_pem">>], SSL2) + ), + ok = load_config(#{ + <<"servers">> => #{ + <<"name">> => #{<<"ssl">> => SSL1}, + <<"noname">> => #{<<"ssl">> => SSL2} + } + }), + + % Wait for a periodic collection event + ?check_trace( + ?block_until(#{?snk_kind := tls_certfile_gc_periodic, ?snk_span := {complete, _}}), + fun(Events) -> + % Nothing should have been collected yet + ?assertMatch( + [ + #{?snk_kind := tls_certfile_gc_periodic, ?snk_span := start}, + #{?snk_kind := tls_certfile_gc_periodic, ?snk_span := {complete, _}} + ], + ?of_pid(Pid, Events) + ) + end + ), + + % Delete the server from the config + ok = put_config([<<"servers">>, <<"noname">>, <<"ssl">>], #{}), + + % Wait for a periodic collection event + ?check_trace( + ?block_until(#{?snk_kind := tls_certfile_gc_periodic, ?snk_span := {complete, _}}), + fun(Events) -> + % Nothing should have been collected yet, certfiles considered orphans for the first time + ?assertMatch( + [ + #{?snk_kind := tls_certfile_gc_periodic, ?snk_span := start}, + #{?snk_kind := tls_certfile_gc_periodic, ?snk_span := {complete, _}} + ], + ?of_pid(Pid, Events) + ) + end + ), + + % Delete another server from the config + ok = put_config([<<"servers">>, <<"name">>, <<"ssl">>], #{}), + + % Wait for next periodic collection event + ?check_trace( + ?block_until(#{?snk_kind := tls_certfile_gc_periodic, ?snk_span := {complete, _}}), + fun(Events) -> + % SSL2 certfiles should have been collected now, but not SSL1 + ?assertMatch( + [ + #{?snk_kind := tls_certfile_gc_periodic, ?snk_span := start}, + #{?snk_kind := "tls_certfile_gc_collected", filename := SSL2IssPEM}, + #{?snk_kind := "tls_certfile_gc_collected", filename := SSL2Keyfile}, + #{?snk_kind := "tls_certfile_gc_collected", filename := SSL2Certfile}, + #{?snk_kind := "tls_certfile_gc_collected", filename := _SSL2Dir}, + #{?snk_kind := tls_certfile_gc_periodic, ?snk_span := {complete, _}} + ], + ?of_pid(Pid, Events) + ) + end + ), + + % Wait for next periodic collection event + ?check_trace( + ?block_until(#{?snk_kind := tls_certfile_gc_periodic, ?snk_span := {complete, _}}), + fun(Events) -> + % SSL1 certfiles should have been collected finally, they were considered orphans before + ?assertMatch( + [ + #{?snk_kind := tls_certfile_gc_periodic, ?snk_span := start}, + #{?snk_kind := "tls_certfile_gc_collected", filename := SSL1Keyfile}, + #{?snk_kind := "tls_certfile_gc_collected", filename := SSL1Certfile}, + #{?snk_kind := "tls_certfile_gc_collected", filename := _SSL1Dir}, + #{?snk_kind := tls_certfile_gc_periodic, ?snk_span := {complete, _}} + ], + ?of_pid(Pid, Events) + ) + end + ), + + ok = proc_lib:stop(Pid). + +t_gc_spares_recreated_certfiles(_Config) -> + {ok, Pid} = emqx_tls_certfile_gc:start_link(), + + % Create two sets of certfiles, with no references to them + SSL = #{ + <<"keyfile">> => key(), + <<"certfile">> => cert() + }, + {ok, SSL1} = emqx_tls_lib:ensure_ssl_files("s1", SSL), + SSL1Keyfile = emqx_utils_fs:canonicalize(maps:get(<<"keyfile">>, SSL1)), + SSL1Certfile = emqx_utils_fs:canonicalize(maps:get(<<"certfile">>, SSL1)), + {ok, SSL2} = emqx_tls_lib:ensure_ssl_files("s2", SSL), + SSL2Keyfile = emqx_utils_fs:canonicalize(maps:get(<<"keyfile">>, SSL2)), + SSL2Certfile = emqx_utils_fs:canonicalize(maps:get(<<"certfile">>, SSL2)), + ok = load_config(#{}), + + % Nothing should have been collected yet + ?assertMatch( + {ok, []}, + emqx_tls_certfile_gc:run() + ), + + % At least one second should pass, since mtime has second resolution + ok = timer:sleep(1000), + ok = file:change_time(SSL2Keyfile, erlang:localtime()), + ok = file:change_time(SSL2Certfile, erlang:localtime()), + % Only SSL1 certfiles should have been collected + ?assertMatch( + {ok, [ + {collect, _SSL1Dir, ok}, + {collect, SSL1Certfile, ok}, + {collect, SSL1Keyfile, ok} + ]}, + emqx_tls_certfile_gc:run() + ), + + % Recreate the SSL2 certfiles + ok = file:delete(SSL2Keyfile), + ok = file:delete(SSL2Certfile), + {ok, _} = emqx_tls_lib:ensure_ssl_files("s2", SSL), + % Nothing should have been collected + ?assertMatch( + {ok, []}, + emqx_tls_certfile_gc:run() + ), + + ok = proc_lib:stop(Pid). + +t_gc_active(_Config) -> + ok = emqx_common_test_helpers:boot_modules([]), + ok = emqx_common_test_helpers:start_apps([]), + try + ?assertEqual( + {ok, []}, + emqx_tls_certfile_gc:run() + ) + after + emqx_common_test_helpers:stop_apps([]), + emqx_common_test_helpers:boot_modules(all) + end. + +orphans() -> + emqx_tls_certfile_gc:orphans(emqx:mutable_certs_dir()). + +convicts(Orphans, OrphansLast) -> + emqx_tls_certfile_gc:convicts(Orphans, OrphansLast). + +collect(Convicts) -> + emqx_tls_certfile_gc:collect_files(Convicts, emqx:mutable_certs_dir()). + +load_config(Config) -> + emqx_config:init_load( + ?MODULE, + emqx_utils_json:encode(#{<> => Config}) + ). + +put_config(Path, SubConfig) -> + emqx_config:put_raw([<> | Path], SubConfig). + +cert() -> + << + "-----BEGIN CERTIFICATE-----\n" + "MIIFljCCA36gAwIBAgICEAEwDQYJKoZIhvcNAQELBQAwazELMAkGA1UEBhMCU0Ux\n" + "EjAQBgNVBAgMCVN0b2NraG9sbTESMBAGA1UECgwJTXlPcmdOYW1lMRkwFwYDVQQL\n" + "DBBNeUludGVybWVkaWF0ZUNBMRkwFwYDVQQDDBBNeUludGVybWVkaWF0ZUNBMB4X\n" + "DTIzMDExMjEzMDgxNloXDTMzMDQxOTEzMDgxNlowdzELMAkGA1UEBhMCU0UxEjAQ\n" + "BgNVBAgMCVN0b2NraG9sbTESMBAGA1UEBwwJU3RvY2tob2xtMRIwEAYDVQQKDAlN\n" + "eU9yZ05hbWUxGTAXBgNVBAsMEE15SW50ZXJtZWRpYXRlQ0ExETAPBgNVBAMMCE15\n" + "Q2xpZW50MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvGuAShewEo8V\n" + "/+aWVO/MuUt92m8K0Ut4nC2gOvpjMjf8mhSSf6KfnxPklsFwP4fdyPOjOiXwCsf3\n" + "1QO5fjVr8to3iGTHhEyZpzRcRqmw1eYJC7iDh3BqtYLAT30R+Kq6Mk+f4tXB5Lp/\n" + "2jXgdi0wshWagCPgJO3CtiwGyE8XSa+Q6EBYwzgh3NFbgYdJma4x+S86Y/5WfmXP\n" + "zF//UipsFp4gFUqwGuj6kJrN9NnA1xCiuOxCyN4JuFNMfM/tkeh26jAp0OHhJGsT\n" + "s3YiUm9Dpt7Rs7o0so9ov9K+hgDFuQw9HZW3WIJI99M5a9QZ4ZEQqKpABtYBl/Nb\n" + "VPXcr+T3fQIDAQABo4IBNjCCATIwCQYDVR0TBAIwADARBglghkgBhvhCAQEEBAMC\n" + "BaAwMwYJYIZIAYb4QgENBCYWJE9wZW5TU0wgR2VuZXJhdGVkIENsaWVudCBDZXJ0\n" + "aWZpY2F0ZTAdBgNVHQ4EFgQUOIChBA5aZB0dPWEtALfMIfSopIIwHwYDVR0jBBgw\n" + "FoAUTHCGOxVSibq1D5KqrrExQRO+c/MwDgYDVR0PAQH/BAQDAgXgMB0GA1UdJQQW\n" + "MBQGCCsGAQUFBwMCBggrBgEFBQcDBDA7BgNVHR8ENDAyMDCgLqAshipodHRwOi8v\n" + "bG9jYWxob3N0Ojk4NzgvaW50ZXJtZWRpYXRlLmNybC5wZW0wMQYIKwYBBQUHAQEE\n" + "JTAjMCEGCCsGAQUFBzABhhVodHRwOi8vbG9jYWxob3N0Ojk4NzcwDQYJKoZIhvcN\n" + "AQELBQADggIBAE0qTL5WIWcxRPU9oTrzJ+oxMTp1JZ7oQdS+ZekLkQ8mP7T6C/Ew\n" + "6YftjvkopnHUvn842+PTRXSoEtlFiTccmA60eMAai2tn5asxWBsLIRC9FH3LzOgV\n" + "/jgyY7HXuh8XyDBCDD+Sj9QityO+accTHijYAbHPAVBwmZU8nO5D/HsxLjRrCfQf\n" + "qf4OQpX3l1ryOi19lqoRXRGwcoZ95dqq3YgTMlLiEqmerQZSR6iSPELw3bcwnAV1\n" + "hoYYzeKps3xhwszCTz2+WaSsUO2sQlcFEsZ9oHex/02UiM4a8W6hGFJl5eojErxH\n" + "7MqaSyhwwyX6yt8c75RlNcUThv+4+TLkUTbTnWgC9sFjYfd5KSfAdIMp3jYzw3zw\n" + "XEMTX5FaLaOCAfUDttPzn+oNezWZ2UyFTQXQE2CazpRdJoDd04qVg9WLpQxLYRP7\n" + "xSFEHulOPccdAYF2C45yNtJAZyWKfGaAZIxrgEXbMkcdDMlYphpRwpjS8SIBNZ31\n" + "KFE8BczKrg2qO0ywIjanPaRgrFVmeSvBKeU/YLQVx6fZMgOk6vtidLGZLyDXy0Ff\n" + "yaZSoj+on++RDz1IXb96Y8scuNlfcYI8QeoNjwiLtf80BV8SRJiG4e/jTvMf/z9L\n" + "kWrnDWvx4xkUmxFg4TK42dkNp7sEYBTlVVq9fjKE92ha7FGZRqsxOLNQ\n" + "-----END CERTIFICATE-----\n" + >>. + +key() -> + << + "-----BEGIN EC PRIVATE KEY-----\n" + "MHQCAQEEICKTbbathzvD8zvgjL7qRHhW4alS0+j0Loo7WeYX9AxaoAcGBSuBBAAK\n" + "oUQDQgAEJBdF7MIdam5T4YF3JkEyaPKdG64TVWCHwr/plC0QzNVJ67efXwxlVGTo\n" + "ju0VBj6tOX1y6C0U+85VOM0UU5xqvw==\n" + "-----END EC PRIVATE KEY-----\n" + >>. + +%%-------------------------------------------------------------------- +%% Schema +%% ------------------------------------------------------------------- + +roots() -> + [?MODULE]. + +namespace() -> + "ct". + +fields(?MODULE) -> + [ + {servers, mk(hoconsc:map(string(), ref(?MODULE, server)))}, + {clients, mk(hoconsc:array(ref(?MODULE, client)))} + ]; +fields(server) -> + [ + {ssl, mk(ref(emqx_schema, "listener_ssl_opts"))} + ]; +fields(client) -> + [ + {transport, + mk( + ref(?MODULE, transport), + #{required => false} + )} + ]; +fields(transport) -> + [ + {ssl, + mk( + ref(emqx_schema, "ssl_client_opts"), + #{default => #{<<"enable">> => false}} + )} + ]. diff --git a/apps/emqx_bridge/src/emqx_bridge_resource.erl b/apps/emqx_bridge/src/emqx_bridge_resource.erl index f927ef750..b19b7139f 100644 --- a/apps/emqx_bridge/src/emqx_bridge_resource.erl +++ b/apps/emqx_bridge/src/emqx_bridge_resource.erl @@ -247,18 +247,16 @@ recreate(Type, Name, Conf, Opts) -> ). create_dry_run(Type, Conf0) -> - TmpPath = emqx_utils:safe_filename(?TEST_ID_PREFIX ++ emqx_utils:gen_id(8)), + TmpName = iolist_to_binary([?TEST_ID_PREFIX, emqx_utils:gen_id(8)]), + TmpPath = emqx_utils:safe_filename(TmpName), Conf = emqx_utils_maps:safe_atom_key_map(Conf0), case emqx_connector_ssl:convert_certs(TmpPath, Conf) of {error, Reason} -> {error, Reason}; {ok, ConfNew} -> try - ParseConf = parse_confs(bin(Type), TmpPath, ConfNew), - Res = emqx_resource:create_dry_run_local( - bridge_to_resource_type(Type), ParseConf - ), - Res + ParseConf = parse_confs(bin(Type), TmpName, ConfNew), + emqx_resource:create_dry_run_local(bridge_to_resource_type(Type), ParseConf) catch %% validation errors throw:Reason -> diff --git a/apps/emqx_bridge/test/emqx_bridge_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_SUITE.erl index fe68e610d..1cbc94d24 100644 --- a/apps/emqx_bridge/test/emqx_bridge_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_SUITE.erl @@ -156,6 +156,7 @@ setup_fake_telemetry_data() -> t_update_ssl_conf(Config) -> Path = proplists:get_value(config_path, Config), + CertDir = filename:join([emqx:mutable_certs_dir() | Path]), EnableSSLConf = #{ <<"bridge_mode">> => false, <<"clean_start">> => true, @@ -172,22 +173,13 @@ t_update_ssl_conf(Config) -> } }, {ok, _} = emqx:update_config(Path, EnableSSLConf), - {ok, Certs} = list_pem_dir(Path), - ?assertMatch([_, _, _], Certs), + ?assertMatch({ok, [_, _, _]}, file:list_dir(CertDir)), NoSSLConf = EnableSSLConf#{<<"ssl">> := #{<<"enable">> => false}}, {ok, _} = emqx:update_config(Path, NoSSLConf), - ?assertMatch({error, not_dir}, list_pem_dir(Path)), + {ok, _} = emqx_tls_certfile_gc:force(), + ?assertMatch({error, enoent}, file:list_dir(CertDir)), ok. -list_pem_dir(Path) -> - Dir = filename:join([emqx:mutable_certs_dir() | Path]), - case filelib:is_dir(Dir) of - true -> - file:list_dir(Dir); - _ -> - {error, not_dir} - end. - data_file(Name) -> Dir = code:lib_dir(emqx_bridge, test), {ok, Bin} = file:read_file(filename:join([Dir, "data", Name])), diff --git a/apps/emqx_exhook/test/emqx_exhook_SUITE.erl b/apps/emqx_exhook/test/emqx_exhook_SUITE.erl index ff313c8c8..907d22b6c 100644 --- a/apps/emqx_exhook/test/emqx_exhook_SUITE.erl +++ b/apps/emqx_exhook/test/emqx_exhook_SUITE.erl @@ -360,13 +360,10 @@ t_stop_timeout(_) -> t_ssl_clear(_) -> SvrName = <<"ssl_test">>, SSLConf = #{ - <<"cacertfile">> => cert_file("cafile"), - - <<"certfile">> => cert_file("certfile"), - <<"enable">> => true, + <<"cacertfile">> => cert_file("cafile"), + <<"certfile">> => cert_file("certfile"), <<"keyfile">> => cert_file("keyfile"), - <<"verify">> => <<"verify_peer">> }, AddConf = #{ @@ -377,7 +374,6 @@ t_ssl_clear(_) -> <<"pool_size">> => 16, <<"request_timeout">> => <<"5s">>, <<"ssl">> => SSLConf, - <<"url">> => <<"http://127.0.0.1:9000">> }, emqx_exhook_mgr:update_config([exhook, servers], {add, AddConf}), @@ -387,6 +383,7 @@ t_ssl_clear(_) -> UpdateConf = AddConf#{<<"ssl">> => SSLConf#{<<"keyfile">> => cert_file("keyfile2")}}, emqx_exhook_mgr:update_config([exhook, servers], {update, SvrName, UpdateConf}), + {ok, _} = emqx_tls_certfile_gc:force(), ListResult2 = list_pem_dir(SvrName), ?assertMatch({ok, [_, _, _]}, ListResult2), {ok, ResultList2} = ListResult2, @@ -403,7 +400,8 @@ t_ssl_clear(_) -> ?assertNotEqual(FindKeyFile(ResultList1), FindKeyFile(ResultList2)), emqx_exhook_mgr:update_config([exhook, servers], {delete, SvrName}), - ?assertMatch({error, not_dir}, list_pem_dir(SvrName)), + {ok, _} = emqx_tls_certfile_gc:force(), + ?assertMatch({error, enoent}, list_pem_dir(SvrName)), ok. %%-------------------------------------------------------------------- @@ -475,12 +473,7 @@ is_exhook_callback(Cb) -> list_pem_dir(Name) -> Dir = filename:join([emqx:mutable_certs_dir(), "exhook", Name]), - case filelib:is_dir(Dir) of - true -> - file:list_dir(Dir); - _ -> - {error, not_dir} - end. + file:list_dir(Dir). data_file(Name) -> Dir = code:lib_dir(emqx_exhook, test), diff --git a/apps/emqx_gateway/test/emqx_gateway_conf_SUITE.erl b/apps/emqx_gateway/test/emqx_gateway_conf_SUITE.erl index ce709efc3..89a35879e 100644 --- a/apps/emqx_gateway/test/emqx_gateway_conf_SUITE.erl +++ b/apps/emqx_gateway/test/emqx_gateway_conf_SUITE.erl @@ -495,6 +495,7 @@ t_add_listener_with_certs_content(_) -> ok. assert_ssl_confs_files_deleted(SslConf) when is_map(SslConf) -> + {ok, _} = emqx_tls_certfile_gc:force(), Ks = [<<"cacertfile">>, <<"certfile">>, <<"keyfile">>], lists:foreach( fun(K) -> diff --git a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl index 977c81c2b..b038863a8 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl @@ -237,6 +237,7 @@ t_clear_certs(Config) when is_list(Config) -> [<<"ssl_options">>, <<"keyfile">>], NewConf2, cert_file("keyfile2") ), _ = request(put, NewPath, [], UpdateConf), + _ = emqx_tls_certfile_gc:force(), ListResult2 = list_pem_dir("ssl", "clear"), %% make sure the old cret file is deleted @@ -259,7 +260,8 @@ t_clear_certs(Config) when is_list(Config) -> %% remove, check all cert files are deleted _ = delete(NewPath), - ?assertMatch({error, not_dir}, list_pem_dir("ssl", "clear")), + _ = emqx_tls_certfile_gc:force(), + ?assertMatch({error, enoent}, list_pem_dir("ssl", "clear")), ok. get_tcp_listeners(Node) -> @@ -431,12 +433,7 @@ is_running(Id) -> list_pem_dir(Type, Name) -> ListenerDir = emqx_listeners:certs_dir(Type, Name), Dir = filename:join([emqx:mutable_certs_dir(), ListenerDir]), - case filelib:is_dir(Dir) of - true -> - file:list_dir(Dir); - _ -> - {error, not_dir} - end. + file:list_dir(Dir). data_file(Name) -> Dir = code:lib_dir(emqx, test), diff --git a/apps/emqx_utils/src/emqx_utils.erl b/apps/emqx_utils/src/emqx_utils.erl index 2c6ddd9c1..5070a7523 100644 --- a/apps/emqx_utils/src/emqx_utils.erl +++ b/apps/emqx_utils/src/emqx_utils.erl @@ -25,6 +25,7 @@ maybe_apply/2, compose/1, compose/2, + cons/2, run_fold/3, pipeline/3, start_timer/2, @@ -135,6 +136,10 @@ compose(F, G) when is_function(G) -> fun(X) -> G(F(X)) end; compose(F, [G]) -> compose(F, G); compose(F, [G | More]) -> compose(compose(F, G), More). +-spec cons(X, [X]) -> [X, ...]. +cons(Head, Tail) -> + [Head | Tail]. + %% @doc RunFold run_fold([], Acc, _State) -> Acc; diff --git a/apps/emqx_utils/src/emqx_utils_fs.erl b/apps/emqx_utils/src/emqx_utils_fs.erl new file mode 100644 index 000000000..57d9d4329 --- /dev/null +++ b/apps/emqx_utils/src/emqx_utils_fs.erl @@ -0,0 +1,80 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +-module(emqx_utils_fs). + +-include_lib("kernel/include/file.hrl"). + +-export([traverse_dir/3]). +-export([canonicalize/1]). + +-type fileinfo() :: #file_info{}. +-type foldfun(Acc) :: + fun((_Filepath :: file:name(), fileinfo() | {error, file:posix()}, Acc) -> Acc). + +%% @doc Traverse a directory recursively and apply a fold function to each file. +%% +%% This is a safer version of `filelib:fold_files/5` which does not follow symlinks +%% and reports errors to the fold function, giving the user more control over the +%% traversal. +%% It's not an error if `Dirpath` is not a directory, in which case the fold function +%% will be called once with the file info of `Dirpath`. +-spec traverse_dir(foldfun(Acc), Acc, _Dirpath :: file:name()) -> + Acc. +traverse_dir(FoldFun, Acc, Dirpath) -> + traverse_dir(FoldFun, Acc, Dirpath, read_info(Dirpath)). + +traverse_dir(FoldFun, AccIn, DirPath, {ok, #file_info{type = directory}}) -> + case file:list_dir(DirPath) of + {ok, Filenames} -> + lists:foldl( + fun(Filename, Acc) -> + AbsPath = filename:join(DirPath, Filename), + traverse_dir(FoldFun, Acc, AbsPath) + end, + AccIn, + Filenames + ); + {error, Reason} -> + FoldFun(DirPath, {error, Reason}, AccIn) + end; +traverse_dir(FoldFun, Acc, AbsPath, {ok, Info}) -> + FoldFun(AbsPath, Info, Acc); +traverse_dir(FoldFun, Acc, AbsPath, {error, Reason}) -> + FoldFun(AbsPath, {error, Reason}, Acc). + +read_info(AbsPath) -> + file:read_link_info(AbsPath, [{time, posix}, raw]). + +%% @doc Canonicalize a file path. +%% Removes stray slashes and converts to a string. +-spec canonicalize(file:name()) -> + string(). +canonicalize(Filename) -> + case filename:split(str(Filename)) of + Components = [_ | _] -> + filename:join(Components); + [] -> + "" + end. + +str(Value) -> + case unicode:characters_to_list(Value, unicode) of + Str when is_list(Str) -> + Str; + {error, _, _} -> + erlang:error(badarg, [Value]) + end. diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl b/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl new file mode 100644 index 000000000..9018a6189 --- /dev/null +++ b/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl @@ -0,0 +1,98 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +-module(emqx_utils_fs_SUITE). + +-compile(export_all). +-compile(nowarn_export_all). + +-include_lib("common_test/include/ct.hrl"). +-include_lib("eunit/include/eunit.hrl"). +-include_lib("kernel/include/file.hrl"). + +all() -> + emqx_common_test_helpers:all(?MODULE). + +%% + +t_traverse_dir(Config) -> + Dir = ?config(data_dir, Config), + Traversal = lists:sort(emqx_utils_fs:traverse_dir(fun cons_fileinfo/3, [], Dir)), + ?assertMatch( + [ + {"nonempty/d1/1", #file_info{type = regular, mode = ORWAR}}, + {"nonempty/d1/2", #file_info{type = regular, mode = ORWAR}}, + {"nonempty/d1/mutrec", #file_info{type = symlink, mode = ARWX}}, + {"nonempty/d1/файл", #file_info{type = regular}}, + {"nonempty/d2/deep/down/here", #file_info{type = regular, mode = ORW}}, + {"nonempty/d2/deep/mutrec", #file_info{type = symlink, mode = ARWX}}, + {"nonempty/д3", #file_info{type = symlink, mode = ARWX}} + ] when + ((ORWAR band 8#00644 =:= 8#00644) and + (ORW band 8#00600 =:= 8#00600) and + (ARWX band 8#00777 =:= 8#00777)), + + [{string:prefix(Filename, Dir), Info} || {Filename, Info} <- Traversal] + ). + +t_traverse_empty(Config) -> + Dir = filename:join(?config(data_dir, Config), "empty"), + ok = file:make_dir(Dir), + ?assertEqual( + [], + emqx_utils_fs:traverse_dir(fun cons_fileinfo/3, [], Dir) + ). + +t_traverse_nonexisting(_) -> + ?assertEqual( + [{"this/should/not/exist", {error, enoent}}], + emqx_utils_fs:traverse_dir(fun cons_fileinfo/3, [], "this/should/not/exist") + ). + +cons_fileinfo(Filename, Info, Acc) -> + [{Filename, Info} | Acc]. + +%% + +t_canonicalize_empty(_) -> + ?assertEqual( + "", + emqx_utils_fs:canonicalize(<<>>) + ). + +t_canonicalize_relative(_) -> + ?assertEqual( + "rel", + emqx_utils_fs:canonicalize(<<"rel/">>) + ). + +t_canonicalize_trailing_slash(_) -> + ?assertEqual( + "/usr/local", + emqx_utils_fs:canonicalize("/usr/local/") + ). + +t_canonicalize_double_slashes(_) -> + ?assertEqual( + "/usr/local/.", + emqx_utils_fs:canonicalize("//usr//local//.//") + ). + +t_canonicalize_non_utf8(_) -> + ?assertError( + badarg, + emqx_utils_fs:canonicalize(<<128, 128, 128>>) + ). diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/1 b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/1 new file mode 100644 index 000000000..e69de29bb diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/2 b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/2 new file mode 100644 index 000000000..e69de29bb diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/mutrec b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/mutrec new file mode 120000 index 000000000..ca385d3e7 --- /dev/null +++ b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/mutrec @@ -0,0 +1 @@ +../../empty \ No newline at end of file diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/файл b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/файл new file mode 100644 index 000000000..e69de29bb diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d2/deep/down/here b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d2/deep/down/here new file mode 100644 index 000000000..e69de29bb diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d2/deep/mutrec b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d2/deep/mutrec new file mode 120000 index 000000000..354c7c5c3 --- /dev/null +++ b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d2/deep/mutrec @@ -0,0 +1 @@ +../../d1 \ No newline at end of file diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/д3 b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/д3 new file mode 120000 index 000000000..8ade2da06 --- /dev/null +++ b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/д3 @@ -0,0 +1 @@ +../dx \ No newline at end of file From afbf8cb32f8d6e24a5b0c865740bffb742e83b94 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Fri, 2 Jun 2023 16:32:42 +0300 Subject: [PATCH 05/16] chore(util): drop obsolete stuff from `emqx_utils_maps` --- apps/emqx_utils/src/emqx_utils_maps.erl | 52 +------------------------ 1 file changed, 1 insertion(+), 51 deletions(-) diff --git a/apps/emqx_utils/src/emqx_utils_maps.erl b/apps/emqx_utils/src/emqx_utils_maps.erl index d1c3ed649..d1caf1f1e 100644 --- a/apps/emqx_utils/src/emqx_utils_maps.erl +++ b/apps/emqx_utils/src/emqx_utils_maps.erl @@ -31,7 +31,6 @@ binary_string/1, deep_convert/3, diff_maps/2, - merge_with/3, best_effort_recursive_sum/3, if_only_to_toggle_enable/2 ]). @@ -231,55 +230,6 @@ convert_keys_to_atom(BinKeyMap, Conv) -> [] ). -%% copy from maps.erl OTP24.0 -merge_with(Combiner, Map1, Map2) when - is_map(Map1), - is_map(Map2), - is_function(Combiner, 3) --> - case map_size(Map1) > map_size(Map2) of - true -> - Iterator = maps:iterator(Map2), - merge_with_t( - maps:next(Iterator), - Map1, - Map2, - Combiner - ); - false -> - Iterator = maps:iterator(Map1), - merge_with_t( - maps:next(Iterator), - Map2, - Map1, - fun(K, V1, V2) -> Combiner(K, V2, V1) end - ) - end; -merge_with(Combiner, Map1, Map2) -> - ErrorType = error_type_merge_intersect(Map1, Map2, Combiner), - throw(#{maps_merge_error => ErrorType, args => [Map1, Map2]}). - -merge_with_t({K, V2, Iterator}, Map1, Map2, Combiner) -> - case Map1 of - #{K := V1} -> - NewMap1 = Map1#{K := Combiner(K, V1, V2)}, - merge_with_t(maps:next(Iterator), NewMap1, Map2, Combiner); - #{} -> - merge_with_t(maps:next(Iterator), maps:put(K, V2, Map1), Map2, Combiner) - end; -merge_with_t(none, Result, _, _) -> - Result. - -error_type_merge_intersect(M1, M2, Combiner) when is_function(Combiner, 3) -> - error_type_two_maps(M1, M2); -error_type_merge_intersect(_M1, _M2, _Combiner) -> - badarg_combiner_function. - -error_type_two_maps(M1, M2) when is_map(M1) -> - {badmap, M2}; -error_type_two_maps(M1, _M2) -> - {badmap, M1}. - %% @doc Sum-merge map values. %% For bad merges, ErrorLogger is called to log the key, and value in M2 is ignored. best_effort_recursive_sum(M10, M20, ErrorLogger) -> @@ -314,7 +264,7 @@ do_best_effort_recursive_sum(M1, M2, ErrorLogger) -> V1 + V2 end end, - merge_with(F, M1, M2). + maps:merge_with(F, M1, M2). deep_filter(M, F) when is_map(M) -> %% maps:filtermap is not available before OTP 24 From 1cb226dffb0c81702d4834d8e778f49c7d7bdf36 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Fri, 2 Jun 2023 16:45:05 +0300 Subject: [PATCH 06/16] chore: bump applications versions * emqx_authz 0.1.22 * emqx_exhook 5.0.13 * emqx_utils 5.0.3 --- apps/emqx_utils/src/emqx_utils.app.src | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx_utils/src/emqx_utils.app.src b/apps/emqx_utils/src/emqx_utils.app.src index 605093875..0b172565a 100644 --- a/apps/emqx_utils/src/emqx_utils.app.src +++ b/apps/emqx_utils/src/emqx_utils.app.src @@ -2,7 +2,7 @@ {application, emqx_utils, [ {description, "Miscellaneous utilities for EMQX apps"}, % strict semver, bump manually! - {vsn, "5.0.2"}, + {vsn, "5.0.3"}, {modules, [ emqx_utils, emqx_utils_api, From 8a31e5639b9af39cb01e1ba0dc3422e02ce5b2c3 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Fri, 2 Jun 2023 20:03:59 +0300 Subject: [PATCH 07/16] fix(emqx): ensure that standalone build works MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns out rebar3 `path` resource has troubles with dangling symlinks and non-ASCII filenames. 🥲 --- apps/emqx/rebar.config | 14 +++++++------- apps/emqx_utils/test/emqx_utils_fs_SUITE.erl | 11 ++++------- .../emqx_utils_fs_SUITE_data/nonempty/d1/mutrec | 2 +- .../test/emqx_utils_fs_SUITE_data/nonempty/d1/файл | 0 .../test/emqx_utils_fs_SUITE_data/nonempty/д3 | 1 - 5 files changed, 12 insertions(+), 16 deletions(-) delete mode 100644 apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/файл delete mode 120000 apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/д3 diff --git a/apps/emqx/rebar.config b/apps/emqx/rebar.config index fd7855004..9dedf3644 100644 --- a/apps/emqx/rebar.config +++ b/apps/emqx/rebar.config @@ -59,12 +59,12 @@ {statistics, true} ]}. -{project_plugins, [ - {erlfmt, [ - {files, [ - "{src,include,test}/*.{hrl,erl,app.src}", - "rebar.config", - "rebar.config.script" - ]} +{project_plugins, [erlfmt]}. + +{erlfmt, [ + {files, [ + "{src,include,test}/*.{hrl,erl,app.src}", + "rebar.config", + "rebar.config.script" ]} ]}. diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl b/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl index 9018a6189..32147c3b8 100644 --- a/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl +++ b/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl @@ -33,16 +33,13 @@ t_traverse_dir(Config) -> Traversal = lists:sort(emqx_utils_fs:traverse_dir(fun cons_fileinfo/3, [], Dir)), ?assertMatch( [ - {"nonempty/d1/1", #file_info{type = regular, mode = ORWAR}}, - {"nonempty/d1/2", #file_info{type = regular, mode = ORWAR}}, + {"nonempty/d1/1", #file_info{type = regular}}, + {"nonempty/d1/2", #file_info{type = regular}}, {"nonempty/d1/mutrec", #file_info{type = symlink, mode = ARWX}}, - {"nonempty/d1/файл", #file_info{type = regular}}, {"nonempty/d2/deep/down/here", #file_info{type = regular, mode = ORW}}, - {"nonempty/d2/deep/mutrec", #file_info{type = symlink, mode = ARWX}}, - {"nonempty/д3", #file_info{type = symlink, mode = ARWX}} + {"nonempty/d2/deep/mutrec", #file_info{type = symlink, mode = ARWX}} ] when - ((ORWAR band 8#00644 =:= 8#00644) and - (ORW band 8#00600 =:= 8#00600) and + ((ORW band 8#00600 =:= 8#00600) and (ARWX band 8#00777 =:= 8#00777)), [{string:prefix(Filename, Dir), Info} || {Filename, Info} <- Traversal] diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/mutrec b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/mutrec index ca385d3e7..d378eb1b6 120000 --- a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/mutrec +++ b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/mutrec @@ -1 +1 @@ -../../empty \ No newline at end of file +../d2/deep/down \ No newline at end of file diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/файл b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/d1/файл deleted file mode 100644 index e69de29bb..000000000 diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/д3 b/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/д3 deleted file mode 120000 index 8ade2da06..000000000 --- a/apps/emqx_utils/test/emqx_utils_fs_SUITE_data/nonempty/д3 +++ /dev/null @@ -1 +0,0 @@ -../dx \ No newline at end of file From 206d0472e0e27b4a6ae6b1ffeea8793e3fd05579 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Fri, 2 Jun 2023 22:21:33 +0300 Subject: [PATCH 08/16] fix(emqx): avoid interference in testsuites Also remove that `?wait_async_action` in `emqx_trace_SUITE` which consistently becomes stuck for 100 seconds. It's not clear why it was there in the first place because listeners should start synchronously, so `emqx_common_test_helpers:start_apps/1` won't return until required listeners are started. --- apps/emqx/test/emqx_common_test_helpers.erl | 1 + apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl | 3 +-- apps/emqx/test/emqx_trace_SUITE.erl | 21 ++++++------------- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/apps/emqx/test/emqx_common_test_helpers.erl b/apps/emqx/test/emqx_common_test_helpers.erl index 54b3b3ca9..e545bb624 100644 --- a/apps/emqx/test/emqx_common_test_helpers.erl +++ b/apps/emqx/test/emqx_common_test_helpers.erl @@ -347,6 +347,7 @@ stop_apps(Apps, Opts) -> ok = mria_mnesia:delete_schema(), %% to avoid inter-suite flakiness application:unset_env(emqx, init_config_load_done), + application:unset_env(emqx, boot_modules), persistent_term:erase(?EMQX_AUTHENTICATION_SCHEMA_MODULE_PT_KEY), case Opts of #{erase_all_configs := false} -> diff --git a/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl b/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl index 3b0d0cf0c..854319b03 100644 --- a/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl +++ b/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl @@ -326,8 +326,7 @@ t_gc_active(_Config) -> emqx_tls_certfile_gc:run() ) after - emqx_common_test_helpers:stop_apps([]), - emqx_common_test_helpers:boot_modules(all) + emqx_common_test_helpers:stop_apps([]) end. orphans() -> diff --git a/apps/emqx/test/emqx_trace_SUITE.erl b/apps/emqx/test/emqx_trace_SUITE.erl index 140ec79ff..0166613a4 100644 --- a/apps/emqx/test/emqx_trace_SUITE.erl +++ b/apps/emqx/test/emqx_trace_SUITE.erl @@ -33,21 +33,12 @@ all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Config) -> - %% ensure dependent apps stopped - emqx_common_test_helpers:stop_apps([]), - ?check_trace( - ?wait_async_action( - emqx_common_test_helpers:start_apps([]), - #{?snk_kind := listener_started, bind := 1883}, - timer:seconds(100) - ), - fun(Trace) -> - ct:pal("listener start statuses: ~p", [ - ?of_kind([listener_started, listener_not_started], Trace) - ]), - %% more than one listener - ?assertMatch([_ | _], ?of_kind(listener_started, Trace)) - end + ok = emqx_common_test_helpers:start_apps([]), + Listeners = emqx_listeners:list(), + ct:pal("emqx_listeners:list() = ~p~n", [Listeners]), + ?assertMatch( + [_ | _], + [ID || {ID, #{running := true}} <- Listeners] ), Config. From ec06850bd054ba47c5cd40503ca0dfd63c4949de Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Fri, 2 Jun 2023 22:59:26 +0300 Subject: [PATCH 09/16] test(auth): drop obsolete testcase altogether Most of the stuff that was tested by this test case is now covered by `emqx_tls_certfile_gc_SUITE`. --- apps/emqx/test/emqx_authentication_SUITE.erl | 62 -------------------- 1 file changed, 62 deletions(-) diff --git a/apps/emqx/test/emqx_authentication_SUITE.erl b/apps/emqx/test/emqx_authentication_SUITE.erl index 5be3521d9..fb73a3fc1 100644 --- a/apps/emqx/test/emqx_authentication_SUITE.erl +++ b/apps/emqx/test/emqx_authentication_SUITE.erl @@ -26,7 +26,6 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). --include_lib("typerefl/include/types.hrl"). -include("emqx_authentication.hrl"). -define(AUTHN, emqx_authentication). @@ -474,52 +473,6 @@ t_restart({'end', _Config}) -> ?AUTHN:deregister_providers([{password_based, built_in_database}]), ok. -t_convert_certs({_, Config}) -> - Config; -t_convert_certs(Config) when is_list(Config) -> - Global = <<"mqtt:global">>, - Certs = certs([ - {<<"keyfile">>, "key.pem"}, - {<<"certfile">>, "cert.pem"}, - {<<"cacertfile">>, "cacert.pem"} - ]), - - CertsDir = certs_dir(Config, [Global, <<"password_based:built_in_database">>]), - #{<<"ssl">> := NCerts} = convert_certs(CertsDir, #{<<"ssl">> => Certs}), - - Certs2 = certs([ - {<<"keyfile">>, "key.pem"}, - {<<"certfile">>, "cert.pem"} - ]), - - #{<<"ssl">> := NCerts2} = convert_certs( - CertsDir, - #{<<"ssl">> => Certs2}, - #{<<"ssl">> => NCerts} - ), - - ?assertEqual(maps:get(<<"keyfile">>, NCerts), maps:get(<<"keyfile">>, NCerts2)), - ?assertEqual(maps:get(<<"certfile">>, NCerts), maps:get(<<"certfile">>, NCerts2)), - - Certs3 = certs([ - {<<"keyfile">>, "client-key.pem"}, - {<<"certfile">>, "client-cert.pem"}, - {<<"cacertfile">>, "cacert.pem"} - ]), - - #{<<"ssl">> := NCerts3} = convert_certs( - CertsDir, - #{<<"ssl">> => Certs3}, - #{<<"ssl">> => NCerts2} - ), - - ?assertNotEqual(maps:get(<<"keyfile">>, NCerts2), maps:get(<<"keyfile">>, NCerts3)), - ?assertNotEqual(maps:get(<<"certfile">>, NCerts2), maps:get(<<"certfile">>, NCerts3)), - - ?assertEqual(true, filelib:is_regular(maps:get(<<"keyfile">>, NCerts3))), - clear_certs(CertsDir, #{<<"ssl">> => NCerts3}), - ?assertEqual(false, filelib:is_regular(maps:get(<<"keyfile">>, NCerts3))). - t_combine_authn_and_callback({init, Config}) -> [ {listener_id, 'tcp:default'}, @@ -627,18 +580,3 @@ certs(Certs) -> register_provider(Type, Module) -> ok = ?AUTHN:register_providers([{Type, Module}]). - -certs_dir(CtConfig, Path) -> - DataDir = proplists:get_value(data_dir, CtConfig), - Dir = filename:join([DataDir | Path]), - filelib:ensure_dir(Dir), - Dir. - -convert_certs(CertsDir, SslConfig) -> - emqx_authentication_config:convert_certs(CertsDir, SslConfig). - -convert_certs(CertsDir, New, Old) -> - emqx_authentication_config:convert_certs(CertsDir, New, Old). - -clear_certs(CertsDir, SslConfig) -> - emqx_authentication_config:clear_certs(CertsDir, SslConfig). From 3c2a7dbadc1064ca86a097987980c57dde6ae23e Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Sat, 3 Jun 2023 00:12:10 +0300 Subject: [PATCH 10/16] fix(tlsgc): consolidate conf keypaths knowledge in `emqx_tls_lib` So that this GC mechanism will be easier to maintain. --- apps/emqx/src/emqx_tls_certfile_gc.erl | 10 +++++----- apps/emqx/src/emqx_tls_lib.erl | 5 +++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/apps/emqx/src/emqx_tls_certfile_gc.erl b/apps/emqx/src/emqx_tls_certfile_gc.erl index ed0c2c083..1d6f4c41b 100644 --- a/apps/emqx/src/emqx_tls_certfile_gc.erl +++ b/apps/emqx/src/emqx_tls_certfile_gc.erl @@ -240,11 +240,11 @@ find_references(Root) -> Config ). -is_file_reference([<<"keyfile">> | _]) -> true; -is_file_reference([<<"certfile">> | _]) -> true; -is_file_reference([<<"cacertfile">> | _]) -> true; -is_file_reference([<<"issuer_pem">>, <<"ocsp">> | _]) -> true; -is_file_reference(_) -> false. +is_file_reference(Stack) -> + lists:any( + fun(KP) -> lists:prefix(lists:reverse(KP), Stack) end, + emqx_tls_lib:ssl_file_conf_keypaths() + ). is_string(Value) -> is_list(Value) orelse is_binary(Value). diff --git a/apps/emqx/src/emqx_tls_lib.erl b/apps/emqx/src/emqx_tls_lib.erl index c05e0d092..157040c30 100644 --- a/apps/emqx/src/emqx_tls_lib.erl +++ b/apps/emqx/src/emqx_tls_lib.erl @@ -31,6 +31,7 @@ ensure_ssl_files/2, ensure_ssl_files/3, drop_invalid_certs/1, + ssl_file_conf_keypaths/0, pem_dir/1, is_managed_ssl_file/1, is_valid_pem_file/1, @@ -371,6 +372,10 @@ is_valid_string(Binary) when is_binary(Binary) -> _Otherwise -> false end. +-spec ssl_file_conf_keypaths() -> [_ConfKeypath :: [binary()]]. +ssl_file_conf_keypaths() -> + ?SSL_FILE_OPT_PATHS. + %% Check if it is a valid PEM formatted key. is_pem(MaybePem) -> try From 468fd981732e955b8e8537d7f60f72c0298d8718 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Mon, 5 Jun 2023 17:02:34 +0300 Subject: [PATCH 11/16] test(utilfs): add few more `traverse_dir/3` testcases --- apps/emqx_utils/test/emqx_utils_fs_SUITE.erl | 24 +++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl b/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl index 32147c3b8..243db98cd 100644 --- a/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl +++ b/apps/emqx_utils/test/emqx_utils_fs_SUITE.erl @@ -45,9 +45,31 @@ t_traverse_dir(Config) -> [{string:prefix(Filename, Dir), Info} || {Filename, Info} <- Traversal] ). +t_traverse_symlink(Config) -> + Dir = filename:join([?config(data_dir, Config), "nonempty", "d1", "mutrec"]), + ?assertMatch( + [{Dir, #file_info{type = symlink}}], + emqx_utils_fs:traverse_dir(fun cons_fileinfo/3, [], Dir) + ). + +t_traverse_symlink_subdir(Config) -> + Dir = filename:join([?config(data_dir, Config), "nonempty", "d2", "deep", "mutrec", "."]), + Traversal = lists:sort(emqx_utils_fs:traverse_dir(fun cons_fileinfo/3, [], Dir)), + ?assertMatch( + [ + {"nonempty/d2/deep/mutrec/1", #file_info{type = regular}}, + {"nonempty/d2/deep/mutrec/2", #file_info{type = regular}}, + {"nonempty/d2/deep/mutrec/mutrec", #file_info{type = symlink}} + ], + [ + {string:prefix(Filename, ?config(data_dir, Config)), Info} + || {Filename, Info} <- Traversal + ] + ). + t_traverse_empty(Config) -> Dir = filename:join(?config(data_dir, Config), "empty"), - ok = file:make_dir(Dir), + _ = file:make_dir(Dir), ?assertEqual( [], emqx_utils_fs:traverse_dir(fun cons_fileinfo/3, [], Dir) From f8e8b7993b3e92792dbcbf9b36630330f72cd041 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Mon, 5 Jun 2023 18:25:36 +0300 Subject: [PATCH 12/16] fix(tlsgc): anticipate only binaries as string in raw config Co-authored-by: Zaiming (Stone) Shi --- apps/emqx/src/emqx_tls_certfile_gc.erl | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/apps/emqx/src/emqx_tls_certfile_gc.erl b/apps/emqx/src/emqx_tls_certfile_gc.erl index 1d6f4c41b..ec345be70 100644 --- a/apps/emqx/src/emqx_tls_certfile_gc.erl +++ b/apps/emqx/src/emqx_tls_certfile_gc.erl @@ -138,7 +138,7 @@ handle_info({timeout, TRef, collect}, St = #{next_gc_timer := TRef}) -> #{}, collect(St, #{evhandler => {fun log_event/2, []}}) ), - {noreply, start_timer(StNext)}. + {noreply, restart_timer(StNext)}. start_timer(St = #{gc_interval := Interval}) -> TRef = erlang:start_timer(Interval, self(), collect), @@ -228,7 +228,7 @@ find_references(Root) -> Config = emqx_config:get_raw([Root]), fold_config( fun(Stack, Value, Acc) -> - case is_file_reference(Stack) andalso is_string(Value) of + case is_file_reference(Stack) andalso is_binary(Value) of true -> Filename = emqx_schema:naive_env_interpolation(Value), {stop, [emqx_utils_fs:canonicalize(Filename) | Acc]}; @@ -246,9 +246,6 @@ is_file_reference(Stack) -> emqx_tls_lib:ssl_file_conf_keypaths() ). -is_string(Value) -> - is_list(Value) orelse is_binary(Value). - %% fold_config(FoldFun, AccIn, Config) -> @@ -264,8 +261,6 @@ fold_config(FoldFun, AccIn, Stack, Config) when is_map(Config) -> ); fold_config(FoldFun, Acc, Stack, []) -> fold_confval(FoldFun, Acc, Stack, []); -fold_config(FoldFun, Acc, Stack, String = [C | _]) when is_integer(C), C >= 0, C < 16#10FFFF -> - fold_confval(FoldFun, Acc, Stack, String); fold_config(FoldFun, Acc, Stack, Config) when is_list(Config) -> fold_confarray(FoldFun, Acc, Stack, 1, Config); fold_config(FoldFun, Acc, Stack, Config) -> From 31d38e18c9b578cee2bf9fb174beb0379b3bb603 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Mon, 5 Jun 2023 18:27:09 +0300 Subject: [PATCH 13/16] refactor(tlsgc): simplify `fold_config/4` --- apps/emqx/src/emqx_tls_certfile_gc.erl | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/apps/emqx/src/emqx_tls_certfile_gc.erl b/apps/emqx/src/emqx_tls_certfile_gc.erl index ec345be70..5e617a272 100644 --- a/apps/emqx/src/emqx_tls_certfile_gc.erl +++ b/apps/emqx/src/emqx_tls_certfile_gc.erl @@ -267,14 +267,8 @@ fold_config(FoldFun, Acc, Stack, Config) -> fold_confval(FoldFun, Acc, Stack, Config). fold_confarray(FoldFun, AccIn, StackIn, I, [H | T]) -> - Stack = [I | StackIn], - case FoldFun(Stack, H, AccIn) of - {cont, Acc} -> - AccOut = fold_config(FoldFun, Acc, Stack, H), - fold_confarray(FoldFun, AccOut, StackIn, I + 1, T); - {stop, Acc} -> - fold_confarray(FoldFun, Acc, StackIn, I + 1, T) - end; + Acc = fold_subconf(FoldFun, AccIn, [I | StackIn], H), + fold_confarray(FoldFun, Acc, StackIn, I + 1, T); fold_confarray(_FoldFun, Acc, _Stack, _, []) -> Acc. From 750b7158d4ccf3e25b01909ac70a793b5c9cef90 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Mon, 5 Jun 2023 22:22:53 +0300 Subject: [PATCH 14/16] fix(tlsgc): make computing orphans safe against symlinking Before this commit, `orphans/1` considered managed file an orphan based on its absolute filename, which is not safe when, for example, emqx has a symlink configured as `data_dir` while config references certfiles through realpaths. --- apps/emqx/src/emqx_tls_certfile_gc.erl | 70 +++++++++++++++---- apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl | 56 ++++++++++++++- apps/emqx_utils/src/emqx_utils_fs.erl | 3 + 3 files changed, 116 insertions(+), 13 deletions(-) diff --git a/apps/emqx/src/emqx_tls_certfile_gc.erl b/apps/emqx/src/emqx_tls_certfile_gc.erl index 5e617a272..78dfdbaca 100644 --- a/apps/emqx/src/emqx_tls_certfile_gc.erl +++ b/apps/emqx/src/emqx_tls_certfile_gc.erl @@ -63,10 +63,13 @@ -define(HAS_OWNER_WRITE(Mode), ((Mode band 8#00200) > 0)). -type filename() :: string(). +-type basename() :: string(). -type fileinfo() :: #file_info{}. +-type fileref() :: {basename(), fileinfo()}. +-type orphans() :: #{fileref() => [filename()]}. -type st() :: #{ - orphans => #{filename() => fileinfo()}, + orphans => orphans(), gc_interval => pos_integer(), next_gc_timer => reference() }. @@ -149,7 +152,7 @@ restart_timer(St = #{next_gc_timer := TRef}) -> start_timer(St). log_event({collect, Filename, ok}, _) -> - ?tp(debug, "tls_certfile_gc_collected", #{filename => Filename}); + ?tp(info, "tls_certfile_gc_collected", #{filename => Filename}); log_event({collect, Filename, {error, Reason}}, _) -> ?tp(warning, "tls_certfile_gc_collect_error", #{filename => Filename, reason => Reason}). @@ -163,30 +166,45 @@ collect(St, Opts) -> OrphansLast = maps:get(orphans, St, #{}), Convicts = convicts(Orphans, OrphansLast), Result = collect_files(Convicts, RootDir, Opts), - {ok, Result, St#{orphans => maps:without(Convicts, Orphans)}}. + {ok, Result, St#{orphans => Orphans}}. +-spec orphans() -> + orphans(). orphans() -> Dir = emqx_utils_fs:canonicalize(emqx:mutable_certs_dir()), orphans(Dir). +-spec orphans(_Dir :: filename()) -> + orphans(). orphans(Dir) -> + % NOTE + % Orphans are files located under directory `Dir` that are not referenced by any + % configuration fragment defining TLS-related options. Certfiles = find_managed_files(fun is_managed_file/2, Dir), lists:foldl( fun(Root, Acc) -> - References = find_references(Root), + % NOTE + % In situations where there's an ambiguity in `Certfiles` (e.g. a fileref + % pointing to more than one file), we'll spare _all of them_ from marking + % as orphans. + References = find_config_references(Root), maps:without(References, Acc) end, Certfiles, emqx_config:get_root_names() ). +-spec convicts(orphans(), orphans()) -> + [filename()]. convicts(Orphans, OrphansLast) -> + % NOTE + % Convicts are files that are orphans both in `Orphans` and `OrphansLast`. maps:fold( - fun(AbsPath, #file_info{mtime = MTime, inode = Inode}, Acc) -> - case maps:get(AbsPath, Orphans, undefined) of - #file_info{mtime = MTime, inode = Inode} -> + fun(FileRef, Filenames, Acc) -> + case maps:get(FileRef, Orphans, []) of + Filenames -> % Certfile was not changed / recreated in the meantime - [AbsPath | Acc]; + Filenames ++ Acc; _ -> % Certfile was changed / created / recreated in the meantime Acc @@ -196,13 +214,18 @@ convicts(Orphans, OrphansLast) -> OrphansLast ). +-spec find_managed_files(Filter, _Dir :: filename()) -> orphans() when + Filter :: fun((_Abs :: filename(), fileinfo()) -> boolean()). find_managed_files(Filter, Dir) -> emqx_utils_fs:traverse_dir( fun (AbsPath, Info = #file_info{}, Acc) -> case Filter(AbsPath, Info) of - true -> Acc#{AbsPath => Info}; - false -> Acc + true -> + FileRef = mk_fileref(AbsPath, Info), + Acc#{FileRef => [AbsPath | maps:get(FileRef, Acc, [])]}; + false -> + Acc end; (AbsPath, {error, Reason}, Acc) -> ?SLOG(notice, "filesystem_object_inaccessible", #{ @@ -224,14 +247,16 @@ is_managed_file(AbsPath, #file_info{type = Type, mode = Mode}) -> ?HAS_OWNER_WRITE(Mode) andalso emqx_tls_lib:is_managed_ssl_file(AbsPath). -find_references(Root) -> +-spec find_config_references(Root :: binary()) -> + [fileref() | {basename(), {error, _}}]. +find_config_references(Root) -> Config = emqx_config:get_raw([Root]), fold_config( fun(Stack, Value, Acc) -> case is_file_reference(Stack) andalso is_binary(Value) of true -> Filename = emqx_schema:naive_env_interpolation(Value), - {stop, [emqx_utils_fs:canonicalize(Filename) | Acc]}; + {stop, [mk_fileref(Filename) | Acc]}; false -> {cont, Acc} end @@ -246,6 +271,27 @@ is_file_reference(Stack) -> emqx_tls_lib:ssl_file_conf_keypaths() ). +mk_fileref(AbsPath) -> + case emqx_utils_fs:read_info(AbsPath) of + {ok, Info} -> + mk_fileref(AbsPath, Info); + {error, _} = Error -> + % NOTE + % Such broken fileref will not be regarded as live reference. However, there + % are some edge cases where this _might be wrong_, e.g. one of the `AbsPath` + % components is a symlink w/o read permission. + {filename:basename(AbsPath), Error} + end. + +mk_fileref(AbsPath, Info = #file_info{}) -> + % NOTE + % This structure helps to tell if two file paths refer to the same file, without the + % need to reimplement `realpath` or something similar. It is not perfect because false + % positives are always possible, due to: + % * On Windows, inode is always 0. + % * On Unix, files can be hardlinked and have the same basename. + {filename:basename(AbsPath), Info#file_info{atime = undefined}}. + %% fold_config(FoldFun, AccIn, Config) -> diff --git a/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl b/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl index 854319b03..4d53a9413 100644 --- a/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl +++ b/apps/emqx/test/emqx_tls_certfile_gc_SUITE.erl @@ -44,7 +44,7 @@ init_per_testcase(TC, Config) -> TCAbsDir = filename:join(?config(priv_dir, Config), TC), ok = application:set_env(emqx, data_dir, TCAbsDir), ok = snabbkaffe:start_trace(), - [{tc_name, TC}, {tc_absdir, TCAbsDir} | Config]. + [{tc_name, atom_to_list(TC)}, {tc_absdir, TCAbsDir} | Config]. end_per_testcase(_TC, Config) -> ok = snabbkaffe:stop(), @@ -317,6 +317,60 @@ t_gc_spares_recreated_certfiles(_Config) -> ok = proc_lib:stop(Pid). +t_gc_spares_symlinked_datadir(Config) -> + {ok, Pid} = emqx_tls_certfile_gc:start_link(), + + % Create a certfiles set and a server that references it + SSL = #{ + <<"keyfile">> => key(), + <<"certfile">> => cert(), + <<"ocsp">> => #{<<"issuer_pem">> => cert()} + }, + {ok, SSL1} = emqx_tls_lib:ensure_ssl_files("srv", SSL), + SSL1Keyfile = emqx_utils_fs:canonicalize(maps:get(<<"keyfile">>, SSL1)), + + ok = load_config(#{ + <<"servers">> => #{<<"srv">> => #{<<"ssl">> => SSL1}} + }), + + % Change the emqx data_dir to a symlink + TCAbsDir = ?config(tc_absdir, Config), + TCAbsLink = filename:join(?config(priv_dir, Config), ?config(tc_name, Config) ++ ".symlink"), + ok = file:make_symlink(TCAbsDir, TCAbsLink), + ok = application:set_env(emqx, data_dir, TCAbsLink), + + % Make a hardlink to the SSL1 keyfile, that looks like a managed SSL file + SSL1KeyfileHardlink = filename:join([ + filename:dirname(SSL1Keyfile), + "hardlink", + filename:basename(SSL1Keyfile) + ]), + ok = filelib:ensure_dir(SSL1KeyfileHardlink), + ok = file:make_link(SSL1Keyfile, SSL1KeyfileHardlink), + + % Nothing should have been collected + ?assertMatch( + {ok, []}, + emqx_tls_certfile_gc:force() + ), + + ok = put_config([<<"servers">>, <<"srv">>, <<"ssl">>], #{}), + + % Everything should have been collected, including the hardlink + ?assertMatch( + {ok, [ + {collect, _SSL1Dir, ok}, + {collect, _SSL1Certfile, ok}, + {collect, _SSL1KeyfileHardlinkDir, ok}, + {collect, _SSL1KeyfileHardlink, ok}, + {collect, _SSL1Keyfile, ok}, + {collect, _SSL1IssuerPEM, ok} + ]}, + emqx_tls_certfile_gc:force() + ), + + ok = proc_lib:stop(Pid). + t_gc_active(_Config) -> ok = emqx_common_test_helpers:boot_modules([]), ok = emqx_common_test_helpers:start_apps([]), diff --git a/apps/emqx_utils/src/emqx_utils_fs.erl b/apps/emqx_utils/src/emqx_utils_fs.erl index 57d9d4329..ad50a409e 100644 --- a/apps/emqx_utils/src/emqx_utils_fs.erl +++ b/apps/emqx_utils/src/emqx_utils_fs.erl @@ -19,6 +19,7 @@ -include_lib("kernel/include/file.hrl"). -export([traverse_dir/3]). +-export([read_info/1]). -export([canonicalize/1]). -type fileinfo() :: #file_info{}. @@ -56,6 +57,8 @@ traverse_dir(FoldFun, Acc, AbsPath, {ok, Info}) -> traverse_dir(FoldFun, Acc, AbsPath, {error, Reason}) -> FoldFun(AbsPath, {error, Reason}, Acc). +-spec read_info(file:name()) -> + {ok, fileinfo()} | {error, file:posix() | badarg}. read_info(AbsPath) -> file:read_link_info(AbsPath, [{time, posix}, raw]). From d5aedaac7daa9237f3af71f3fe6532feac7c829d Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Tue, 6 Jun 2023 11:21:03 +0300 Subject: [PATCH 15/16] fix(tlslib): append random suffix to managed certfiles In order to lessen the chance of ambiguity in determining collectable certfiles during GC. --- apps/emqx/src/emqx_tls_lib.erl | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/emqx/src/emqx_tls_lib.erl b/apps/emqx/src/emqx_tls_lib.erl index 157040c30..3540bddd5 100644 --- a/apps/emqx/src/emqx_tls_lib.erl +++ b/apps/emqx/src/emqx_tls_lib.erl @@ -389,7 +389,7 @@ is_pem(MaybePem) -> %% Also a potentially half-written PEM file (e.g. due to power outage) %% can be corrected with an overwrite. save_pem_file(Dir, KeyPath, Pem, DryRun) -> - Path = pem_file_name(Dir, KeyPath, Pem), + Path = pem_file_name(Dir, KeyPath), case filelib:ensure_dir(Path) of ok when DryRun -> {ok, Path}; @@ -412,9 +412,8 @@ is_managed_ssl_file(Filename) -> _ -> false end. -pem_file_name(Dir, KeyPath, Pem) -> - <> = crypto:hash(md5, Pem), - Suffix = binary:encode_hex(CK), +pem_file_name(Dir, KeyPath) -> + Suffix = binary:encode_hex(crypto:strong_rand_bytes(8)), Segments = lists:map(fun ensure_bin/1, KeyPath), Filename0 = iolist_to_binary(lists:join(<<"_">>, Segments)), Filename1 = binary:replace(Filename0, <<"file">>, <<>>), From c05f462d0afd59ed2cd9d28a861b84af20cc97c1 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Wed, 7 Jun 2023 13:32:35 +0300 Subject: [PATCH 16/16] fix(tlslib): bring back deterministic filenames for managed certs --- apps/emqx/src/emqx_tls_lib.erl | 9 ++++++--- apps/emqx/test/emqx_tls_lib_tests.erl | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/apps/emqx/src/emqx_tls_lib.erl b/apps/emqx/src/emqx_tls_lib.erl index 3540bddd5..653f26708 100644 --- a/apps/emqx/src/emqx_tls_lib.erl +++ b/apps/emqx/src/emqx_tls_lib.erl @@ -389,7 +389,7 @@ is_pem(MaybePem) -> %% Also a potentially half-written PEM file (e.g. due to power outage) %% can be corrected with an overwrite. save_pem_file(Dir, KeyPath, Pem, DryRun) -> - Path = pem_file_name(Dir, KeyPath), + Path = pem_file_name(Dir, KeyPath, Pem), case filelib:ensure_dir(Path) of ok when DryRun -> {ok, Path}; @@ -412,11 +412,14 @@ is_managed_ssl_file(Filename) -> _ -> false end. -pem_file_name(Dir, KeyPath) -> - Suffix = binary:encode_hex(crypto:strong_rand_bytes(8)), +pem_file_name(Dir, KeyPath, Pem) -> + % NOTE + % Wee need to have the same filename on every cluster node. Segments = lists:map(fun ensure_bin/1, KeyPath), Filename0 = iolist_to_binary(lists:join(<<"_">>, Segments)), Filename1 = binary:replace(Filename0, <<"file">>, <<>>), + Fingerprint = crypto:hash(md5, [Dir, Filename1, Pem]), + Suffix = binary:encode_hex(binary:part(Fingerprint, 0, 8)), Filename = <>, filename:join([pem_dir(Dir), Filename]). diff --git a/apps/emqx/test/emqx_tls_lib_tests.erl b/apps/emqx/test/emqx_tls_lib_tests.erl index 8eea21596..4e8435484 100644 --- a/apps/emqx/test/emqx_tls_lib_tests.erl +++ b/apps/emqx/test/emqx_tls_lib_tests.erl @@ -206,6 +206,24 @@ ssl_file_replace_test() -> ?assert(filelib:is_regular(IssuerPem2)), ok. +ssl_file_deterministic_names_test() -> + SSL0 = #{ + <<"keyfile">> => test_key(), + <<"certfile">> => test_key() + }, + Dir0 = filename:join(["/tmp", ?FUNCTION_NAME, "ssl0"]), + Dir1 = filename:join(["/tmp", ?FUNCTION_NAME, "ssl1"]), + {ok, SSLFiles0} = emqx_tls_lib:ensure_ssl_files(Dir0, SSL0), + ?assertEqual( + {ok, SSLFiles0}, + emqx_tls_lib:ensure_ssl_files(Dir0, SSL0) + ), + ?assertNotEqual( + {ok, SSLFiles0}, + emqx_tls_lib:ensure_ssl_files(Dir1, SSL0) + ), + _ = file:del_dir_r(filename:join(["/tmp", ?FUNCTION_NAME])). + bin(X) -> iolist_to_binary(X). test_key() ->