Merge pull request #7826 from zhongwencool/dashboard-https

fix: dashboard https without deafult pem/keyfile
2022-04-29 13:21:28 +08:00 · 2022-04-29 13:21:28 +08:00 · 6bdd625b21
parent 725c96dc2f 591b7c4fdb
commit 6bdd625b21
7 changed files with 182 additions and 54 deletions
--- a/apps/emqx/src/emqx_tls_lib.erl
+++ b/apps/emqx/src/emqx_tls_lib.erl
@ -30,6 +30,7 @@
 %% SSL files
 -export([
    ensure_ssl_files/2,
+    ensure_ssl_files/3,
    delete_ssl_files/3,
    drop_invalid_certs/1,
    is_valid_pem_file/1,
@ -275,34 +276,41 @@ drop_tls13(SslOpts0) ->
 %% sub-dir in emqx's data_dir, and replace saved file paths for SSL options.
 -spec ensure_ssl_files(file:name_all(), undefined | map()) ->
    {ok, undefined | map()} | {error, map()}.
-ensure_ssl_files(Dir, Opts) ->
-    ensure_ssl_files(Dir, Opts, _DryRun = false).
+ensure_ssl_files(Dir, SSL) ->
+    ensure_ssl_files(Dir, SSL, #{dry_run => false, required_keys => []}).

-ensure_ssl_files(_Dir, undefined, _DryRun) ->
+ensure_ssl_files(_Dir, undefined, _Opts) ->
    {ok, undefined};
-ensure_ssl_files(_Dir, #{<<"enable">> := False} = Opts, _DryRun) when ?IS_FALSE(False) ->
-    {ok, Opts};
-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).
+ensure_ssl_files(_Dir, #{<<"enable">> := False} = SSL, _Opts) when ?IS_FALSE(False) ->
+    {ok, SSL};
+ensure_ssl_files(_Dir, #{enable := False} = SSL, _Opts) when ?IS_FALSE(False) ->
+    {ok, SSL};
+ensure_ssl_files(Dir, SSL, Opts) ->
+    RequiredKeys = maps:get(required_keys, Opts, []),
+    case ensure_ssl_file_key(SSL, RequiredKeys) of
+        ok ->
+            Keys = ?SSL_FILE_OPT_NAMES ++ ?SSL_FILE_OPT_NAMES,
+            ensure_ssl_files(Dir, SSL, Keys, Opts);
+        {error, _} = Error ->
+            Error
+    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
-        {ok, NewOpts} ->
-            ensure_ssl_files(Dir, NewOpts, Keys, DryRun);
+ensure_ssl_files(_Dir, SSL, [], _Opts) ->
+    {ok, SSL};
+ensure_ssl_files(Dir, SSL, [Key | Keys], Opts) ->
+    case ensure_ssl_file(Dir, Key, SSL, maps:get(Key, SSL, undefined), Opts) of
+        {ok, NewSSL} ->
+            ensure_ssl_files(Dir, NewSSL, Keys, Opts);
        {error, Reason} ->
-            {error, Reason#{which_option => Key}}
+            {error, Reason#{which_options => [Key]}}
    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, DryRun),
-    {ok, OldOpts} = ensure_ssl_files(Dir, OldOpts0, DryRun),
+    {ok, NewOpts} = ensure_ssl_files(Dir, NewOpts0, #{dry_run => DryRun}),
+    {ok, OldOpts} = ensure_ssl_files(Dir, OldOpts0, #{dry_run => DryRun}),
    Get = fun
        (_K, undefined) -> undefined;
        (K, Opts) -> maps:get(K, Opts, undefined)
@ -329,28 +337,29 @@ 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, SSL, undefined, _Opts) ->
+    {ok, SSL};
+ensure_ssl_file(Dir, Key, SSL, MaybePem, Opts) ->
    case is_valid_string(MaybePem) of
        true ->
-            do_ensure_ssl_file(Dir, Key, Opts, MaybePem, DryRun);
+            DryRun = maps:get(dry_run, Opts, false),
+            do_ensure_ssl_file(Dir, Key, SSL, MaybePem, DryRun);
        false ->
            {error, #{reason => invalid_file_path_or_pem_string}}
    end.

-do_ensure_ssl_file(Dir, Key, Opts, MaybePem, DryRun) ->
+do_ensure_ssl_file(Dir, Key, SSL, MaybePem, DryRun) ->
    case is_pem(MaybePem) of
        true ->
            case save_pem_file(Dir, Key, MaybePem, DryRun) of
-                {ok, Path} -> {ok, Opts#{Key => Path}};
+                {ok, Path} -> {ok, SSL#{Key => Path}};
                {error, Reason} -> {error, Reason}
            end;
        false ->
            case is_valid_pem_file(MaybePem) of
                true ->
-                    {ok, Opts};
-                {error, enoent} when DryRun -> {ok, Opts};
+                    {ok, SSL};
+                {error, enoent} when DryRun -> {ok, SSL};
                {error, Reason} ->
                    {error, #{
                        pem_check => invalid_pem,
@ -517,6 +526,15 @@ ensure_str(B) when is_binary(B) -> unicode:characters_to_list(B, utf8).
 ensure_bin(B) when is_binary(B) -> B;
 ensure_bin(A) when is_atom(A) -> atom_to_binary(A, utf8).

+ensure_ssl_file_key(_SSL, []) ->
+    ok;
+ensure_ssl_file_key(SSL, RequiredKeys) ->
+    Filter = fun(Key) -> not maps:is_key(Key, SSL) end,
+    case lists:filter(Filter, RequiredKeys) of
+        [] -> ok;
+        Miss -> {error, #{reason => ssl_file_option_not_found, which_options => Miss}}
+    end.
+
 -if(?OTP_RELEASE > 22).
 -ifdef(TEST).
 -include_lib("eunit/include/eunit.hrl").
--- a/apps/emqx/test/emqx_tls_lib_tests.erl
+++ b/apps/emqx/test/emqx_tls_lib_tests.erl
@ -96,26 +96,47 @@ ssl_files_failure_test_() ->
            ),
            ?assertMatch(
                {error, #{file_read := enoent, pem_check := invalid_pem}},
-                emqx_tls_lib:ensure_ssl_files("/tmp", #{<<"keyfile">> => NonExistingFile})
+                emqx_tls_lib:ensure_ssl_files("/tmp", #{
+                    <<"keyfile">> => NonExistingFile,
+                    <<"certfile">> => bin(test_key()),
+                    <<"cacertfile">> => bin(test_key())
+                })
            )
        end},
        {"bad_pem_string", fun() ->
            %% not valid unicode
            ?assertMatch(
-                {error, #{reason := invalid_file_path_or_pem_string, which_option := <<"keyfile">>}},
-                emqx_tls_lib:ensure_ssl_files("/tmp", #{<<"keyfile">> => <<255, 255>>})
+                {error, #{
+                    reason := invalid_file_path_or_pem_string, which_options := [<<"keyfile">>]
+                }},
+                emqx_tls_lib:ensure_ssl_files("/tmp", #{
+                    <<"keyfile">> => <<255, 255>>,
+                    <<"certfile">> => bin(test_key()),
+                    <<"cacertfile">> => bin(test_key())
+                })
            ),
            %% not printable
            ?assertMatch(
                {error, #{reason := invalid_file_path_or_pem_string}},
-                emqx_tls_lib:ensure_ssl_files("/tmp", #{<<"keyfile">> => <<33, 22>>})
+                emqx_tls_lib:ensure_ssl_files("/tmp", #{
+                    <<"keyfile">> => <<33, 22>>,
+                    <<"certfile">> => bin(test_key()),
+                    <<"cacertfile">> => bin(test_key())
+                })
            ),
            TmpFile = filename:join("/tmp", integer_to_list(erlang:system_time(microsecond))),
            try
                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 +145,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 +174,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 +190,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_conf/i18n/emqx_conf_schema.conf
+++ b/apps/emqx_conf/i18n/emqx_conf_schema.conf
@ -944,7 +944,7 @@ until the RPC connection is considered lost."""
      zh: """需要持久化到文件的日志处理进程列表。默认只有 default 一个处理进程。"""
    }
    label {
-      en: "Log Handlers Key Val List"
+      en: "Log Handlers List"
      zh: "日志 Handler 列表"
    }
  }
--- 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,38 @@ 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),
+    _ = 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),
+    Opts = #{required_keys => [<<"keyfile">>, <<"certfile">>, <<"cacertfile">>]},
+    case emqx_tls_lib:ensure_ssl_files(?DIR, Https, Opts) 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/src/emqx_mgmt_api_configs.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl
@ -250,7 +250,7 @@ global_zone_configs(put, #{body := Body}, _Req) ->
 config_reset(post, _Params, Req) ->
    %% reset the config specified by the query string param 'conf_path'
    Path = conf_path_reset(Req) ++ conf_path_from_querystr(Req),
-    case emqx:reset_config(Path, #{}) of
+    case emqx_conf:reset(Path, ?OPTS) of
        {ok, _} ->
            {200};
        {error, no_default_value} ->
--- 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