fix: dashboard https without deafult pem/keyfile

2022-04-29 00:01:31 +08:00 · 2022-04-29 00:01:31 +08:00 · 352984efe7
parent 7061d94cf9
commit 352984efe7
5 changed files with 131 additions and 27 deletions
--- a/apps/emqx/src/emqx_tls_lib.erl
+++ b/apps/emqx/src/emqx_tls_lib.erl
@ -285,12 +285,19 @@ ensure_ssl_files(_Dir, #{<<"enable">> := False} = Opts, _DryRun) when ?IS_FALSE(
 ensure_ssl_files(_Dir, #{enable := False} = Opts, _DryRun) when ?IS_FALSE(False) ->
    {ok, Opts};
 ensure_ssl_files(Dir, Opts, DryRun) ->
-    ensure_ssl_files(Dir, Opts, ?SSL_FILE_OPT_NAMES ++ ?SSL_FILE_OPT_NAMES_A, DryRun).
+    case ensure_ssl_files(Dir, Opts, ?SSL_FILE_OPT_NAMES_A, DryRun) of
+        {ok, NewOpts} ->
+            {ok, NewOpts};
+        {error, #{reason := file_path_or_pem_string_not_found}} ->
+            ensure_ssl_files(Dir, Opts, ?SSL_FILE_OPT_NAMES, DryRun);
+        {error, Reason} ->
+            {error, Reason}
+    end.

 ensure_ssl_files(_Dir, Opts, [], _DryRun) ->
    {ok, Opts};
 ensure_ssl_files(Dir, Opts, [Key | Keys], DryRun) ->
-    case ensure_ssl_file(Dir, Key, Opts, maps:get(Key, Opts, undefined), DryRun) of
+    case ensure_ssl_file(Dir, Key, Opts, maps:find(Key, Opts), DryRun) of
        {ok, NewOpts} ->
            ensure_ssl_files(Dir, NewOpts, Keys, DryRun);
        {error, Reason} ->
@ -329,9 +336,9 @@ delete_old_file(_New, Old) ->
            ?SLOG(error, #{msg => "failed_to_delete_ssl_file", file_path => Old, reason => Reason})
    end.

-ensure_ssl_file(_Dir, _Key, Opts, undefined, _DryRun) ->
-    {ok, Opts};
-ensure_ssl_file(Dir, Key, Opts, MaybePem, DryRun) ->
+ensure_ssl_file(_Dir, _Key, _Opts, error, _DryRun) ->
+    {error, #{reason => file_path_or_pem_string_not_found}};
+ensure_ssl_file(Dir, Key, Opts, {ok, MaybePem}, DryRun) ->
    case is_valid_string(MaybePem) of
        true ->
            do_ensure_ssl_file(Dir, Key, Opts, MaybePem, DryRun);
--- a/apps/emqx/test/emqx_tls_lib_tests.erl
+++ b/apps/emqx/test/emqx_tls_lib_tests.erl
@ -115,7 +115,14 @@ ssl_files_failure_test_() ->
                ok = file:write_file(TmpFile, <<"not a valid pem">>),
                ?assertMatch(
                    {error, #{file_read := not_pem}},
-                    emqx_tls_lib:ensure_ssl_files("/tmp", #{<<"cacertfile">> => bin(TmpFile)})
+                    emqx_tls_lib:ensure_ssl_files(
+                        "/tmp",
+                        #{
+                            <<"cacertfile">> => bin(TmpFile),
+                            <<"keyfile">> => bin(TmpFile),
+                            <<"certfile">> => bin(TmpFile)
+                        }
+                    )
                )
            after
                file:delete(TmpFile)
@ -124,7 +131,12 @@ ssl_files_failure_test_() ->
    ].

 ssl_files_save_delete_test() ->
-    SSL0 = #{<<"keyfile">> => bin(test_key())},
+    Key = bin(test_key()),
+    SSL0 = #{
+        <<"keyfile">> => Key,
+        <<"certfile">> => Key,
+        <<"cacertfile">> => Key
+    },
    Dir = filename:join(["/tmp", "ssl-test-dir"]),
    {ok, SSL} = emqx_tls_lib:ensure_ssl_files(Dir, SSL0),
    File = maps:get(<<"keyfile">>, SSL),
@ -148,7 +160,11 @@ ssl_files_handle_non_generated_file_test() ->
    KeyFileContent = bin(test_key()),
    ok = file:write_file(TmpKeyFile, KeyFileContent),
    ?assert(filelib:is_regular(TmpKeyFile)),
-    SSL0 = #{<<"keyfile">> => TmpKeyFile},
+    SSL0 = #{
+        <<"keyfile">> => TmpKeyFile,
+        <<"certfile">> => TmpKeyFile,
+        <<"cacertfile">> => TmpKeyFile
+    },
    Dir = filename:join(["/tmp", "ssl-test-dir-00"]),
    {ok, SSL2} = emqx_tls_lib:ensure_ssl_files(Dir, SSL0),
    File1 = maps:get(<<"keyfile">>, SSL2),
@ -160,8 +176,18 @@ ssl_files_handle_non_generated_file_test() ->
    ?assertEqual({ok, KeyFileContent}, file:read_file(TmpKeyFile)).

 ssl_file_replace_test() ->
-    SSL0 = #{<<"keyfile">> => bin(test_key())},
-    SSL1 = #{<<"keyfile">> => bin(test_key2())},
+    Key1 = bin(test_key()),
+    Key2 = bin(test_key2()),
+    SSL0 = #{
+        <<"keyfile">> => Key1,
+        <<"certfile">> => Key1,
+        <<"cacertfile">> => Key1
+    },
+    SSL1 = #{
+        <<"keyfile">> => Key2,
+        <<"certfile">> => Key2,
+        <<"cacertfile">> => Key2
+    },
    Dir = filename:join(["/tmp", "ssl-test-dir2"]),
    {ok, SSL2} = emqx_tls_lib:ensure_ssl_files(Dir, SSL0),
    {ok, SSL3} = emqx_tls_lib:ensure_ssl_files(Dir, SSL1),
--- a/apps/emqx_dashboard/src/emqx_dashboard_config.erl
+++ b/apps/emqx_dashboard/src/emqx_dashboard_config.erl
@ -15,6 +15,7 @@
 %%--------------------------------------------------------------------
 -module(emqx_dashboard_config).

+-include_lib("emqx/include/logger.hrl").
 -behaviour(emqx_config_handler).

 %% API
@ -65,7 +66,7 @@ remove_handler() ->
 pre_config_update(_Path, UpdateConf0, RawConf) ->
    UpdateConf = remove_sensitive_data(UpdateConf0),
    NewConf = emqx_map_lib:deep_merge(RawConf, UpdateConf),
-    {ok, NewConf}.
+    ensure_ssl_cert(NewConf).

 -define(SENSITIVE_PASSWORD, <<"******">>).

@ -85,20 +86,39 @@ remove_sensitive_data(Conf0) ->
    end.

 post_config_update(_, _Req, NewConf, OldConf, _AppEnvs) ->
-    #{listeners := #{http := NewHttp, https := NewHttps}} = NewConf,
-    #{listeners := #{http := OldHttp, https := OldHttps}} = OldConf,
-    _ =
-        case diff_listeners(OldHttp, NewHttp, OldHttps, NewHttps) of
-            identical -> ok;
-            {Stop, Start} -> erlang:send_after(500, ?MODULE, {update_listeners, Stop, Start})
-        end,
+    OldHttp = get_listener(http, OldConf),
+    OldHttps = get_listener(https, OldConf),
+    NewHttp = get_listener(http, NewConf),
+    NewHttps = get_listener(https, NewConf),
+    {StopHttp, StartHttp} = diff_listeners(http, OldHttp, NewHttp),
+    {StopHttps, StartHttps} = diff_listeners(https, OldHttps, NewHttps),
+    Stop = maps:merge(StopHttp, StopHttps),
+    Start = maps:merge(StartHttp, StartHttps),
+    ?SLOG(error, Stop#{action => stop}),
+    ?SLOG(error, Start#{acton => start}),
+    _ = erlang:send_after(500, ?MODULE, {update_listeners, Stop, Start}),
    ok.

-diff_listeners(Http, Http, Https, Https) ->
-    identical;
-diff_listeners(OldHttp, NewHttp, Https, Https) ->
-    {#{http => OldHttp}, #{http => NewHttp}};
-diff_listeners(Http, Http, OldHttps, NewHttps) ->
-    {#{https => OldHttps}, #{https => NewHttps}};
-diff_listeners(OldHttp, NewHttp, OldHttps, NewHttps) ->
-    {#{http => OldHttp, https => OldHttps}, #{http => NewHttp, https => NewHttps}}.
+get_listener(Type, Conf) ->
+    emqx_map_lib:deep_get([listeners, Type], Conf, undefined).
+
+diff_listeners(_, Listener, Listener) -> {#{}, #{}};
+diff_listeners(Type, undefined, Start) -> {#{}, #{Type => Start}};
+diff_listeners(Type, Stop, undefined) -> {#{Type => Stop}, #{}};
+diff_listeners(Type, Stop, Start) -> {#{Type => Stop}, #{Type => Start}}.
+
+-define(DIR, <<"dashboard">>).
+
+ensure_ssl_cert(#{<<"listeners">> := #{<<"https">> := #{<<"enable">> := true}}} = Conf) ->
+    Https = emqx_map_lib:deep_get([<<"listeners">>, <<"https">>], Conf, undefined),
+    case emqx_tls_lib:ensure_ssl_files(?DIR, Https) of
+        {ok, undefined} ->
+            {error, <<"ssl_cert_not_found">>};
+        {ok, NewHttps} ->
+            {ok, emqx_map_lib:deep_merge(Conf, #{<<"listeners">> => #{<<"https">> => NewHttps}})};
+        {error, Reason} ->
+            ?SLOG(error, Reason#{msg => "bad_ssl_config"}),
+            {error, Reason}
+    end;
+ensure_ssl_cert(Conf) ->
+    {ok, Conf}.
--- a/apps/emqx_dashboard/test/emqx_dashboard_api_test_helpers.erl
+++ b/apps/emqx_dashboard/test/emqx_dashboard_api_test_helpers.erl
@ -38,7 +38,13 @@ set_default_config(DefaultUsername) ->
        listeners => #{
            http => #{
                enable => true,
-                port => 18083
+                bind => 18083,
+                inet6 => false,
+                ipv6_v6only => false,
+                max_connections => 512,
+                num_acceptors => 4,
+                send_timeout => 5000,
+                backlog => 512
            }
        },
        default_username => DefaultUsername,
--- a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl
+++ b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl
@ -135,6 +135,51 @@ t_zones(_Config) ->
    ?assertEqual(undefined, emqx_config:get_raw([new_zone, mqtt], undefined)),
    ok.

+t_dashboard(_Config) ->
+    {ok, Dashboard = #{<<"listeners">> := Listeners}} = get_config("dashboard"),
+    Https1 = #{enable => true, bind => 18084},
+    ?assertMatch(
+        {error, {"HTTP/1.1", 400, _}},
+        update_config("dashboard", Dashboard#{<<"https">> => Https1})
+    ),
+
+    Https2 = #{
+        enable => true,
+        bind => 18084,
+        keyfile => "etc/certs/badkey.pem",
+        cacertfile => "etc/certs/badcacert.pem",
+        certfile => "etc/certs/badcert.pem"
+    },
+    Dashboard2 = Dashboard#{listeners => Listeners#{https => Https2}},
+    ?assertMatch(
+        {error, {"HTTP/1.1", 400, _}},
+        update_config("dashboard", Dashboard2)
+    ),
+
+    Keyfile = emqx_common_test_helpers:app_path(emqx, filename:join(["etc", "certs", "key.pem"])),
+    Certfile = emqx_common_test_helpers:app_path(emqx, filename:join(["etc", "certs", "cert.pem"])),
+    Cacertfile = emqx_common_test_helpers:app_path(
+        emqx, filename:join(["etc", "certs", "cacert.pem"])
+    ),
+    Https3 = #{
+        enable => true,
+        bind => 18084,
+        keyfile => Keyfile,
+        cacertfile => Cacertfile,
+        certfile => Certfile
+    },
+    Dashboard3 = Dashboard#{listeners => Listeners#{https => Https3}},
+    ?assertMatch({ok, _}, update_config("dashboard", Dashboard3)),
+
+    Dashboard4 = Dashboard#{listeners => Listeners#{https => #{enable => false}}},
+    ?assertMatch({ok, _}, update_config("dashboard", Dashboard4)),
+
+    ?assertMatch({ok, _}, update_config("dashboard", Dashboard)),
+
+    {ok, Dashboard1} = get_config("dashboard"),
+    ?assertNotEqual(Dashboard, Dashboard1),
+    ok.
+
 get_config(Name) ->
    Path = emqx_mgmt_api_test_util:api_path(["configs", Name]),
    case emqx_mgmt_api_test_util:request_api(get, Path) of