Просмотр исходного кода

fix(ssl): avoid explicit deletion of managed certs / keys

This logic was incorrect because it didn't take into account
certfiles / keyfiles "refcounts".
Andrew Mayorov 2 лет назад
Родитель
Сommit
95f706bb9e

+ 6 - 32
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;

+ 1 - 12
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} ->

+ 6 - 51
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) ->
     <<CK:8/binary, _/binary>> = 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]) || <<X:8>> <= 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),

+ 29 - 89
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"
+    >>.

+ 7 - 21
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.

+ 3 - 6
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(

+ 2 - 27
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).

+ 1 - 33
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.

+ 1 - 33
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.

+ 21 - 99
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).