Merge pull request #11137 from zhongwencool/dashboard-https-ssl-options

feat: refactor dashboard https ssl_options
2023-06-30 09:57:07 +08:00 · 2023-06-30 09:57:07 +08:00 · 8b679cf358
parent 4506eff3ff 73487ecdd5
commit 8b679cf358
10 changed files with 139 additions and 31 deletions
--- a/apps/emqx/src/emqx_schema.erl
+++ b/apps/emqx/src/emqx_schema.erl
@ -2302,6 +2302,8 @@ ciphers_schema(Default) ->
        #{
            default => default_ciphers(Default),
            converter => fun
+                (undefined) ->
+                    [];
                (<<>>) ->
                    [];
                ("") ->
@ -2649,6 +2651,8 @@ parse_ka_int(Bin, Name, Min, Max) ->
            throw(#{reason => lists:flatten(Msg), value => I})
    end.

+user_lookup_fun_tr(undefined, Opts) ->
+    user_lookup_fun_tr(<<"emqx_tls_psk:lookup">>, Opts);
 user_lookup_fun_tr(Lookup, #{make_serializable := true}) ->
    fmt_user_lookup_fun(Lookup);
 user_lookup_fun_tr(Lookup, _) ->
--- a/apps/emqx_dashboard/src/emqx_dashboard.erl
+++ b/apps/emqx_dashboard/src/emqx_dashboard.erl
@ -248,9 +248,10 @@ api_key_authorize(Req, Key, Secret) ->
            )
    end.

-ensure_ssl_cert(Listeners = #{https := Https0}) ->
-    Https1 = emqx_tls_lib:to_server_opts(tls, Https0),
-    Listeners#{https => maps:from_list(Https1)};
+ensure_ssl_cert(Listeners = #{https := Https0 = #{ssl_options := SslOpts}}) ->
+    SslOpt1 = maps:from_list(emqx_tls_lib:to_server_opts(tls, SslOpts)),
+    Https1 = maps:remove(ssl_options, Https0),
+    Listeners#{https => maps:merge(Https1, SslOpt1)};
 ensure_ssl_cert(Listeners) ->
    Listeners.

--- a/apps/emqx_dashboard/src/emqx_dashboard_listener.erl
+++ b/apps/emqx_dashboard/src/emqx_dashboard_listener.erl
@ -174,17 +174,19 @@ diff_listeners(Type, Stop, Start) -> {#{Type => Stop}, #{Type => Start}}.

 -define(DIR, <<"dashboard">>).

-ensure_ssl_cert(#{<<"listeners">> := #{<<"https">> := #{<<"bind">> := Bind}}} = Conf) when
+ensure_ssl_cert(#{<<"listeners">> := #{<<"https">> := #{<<"bind">> := Bind} = Https0}} = Conf0) when
    Bind =/= 0
 ->
-    Https = emqx_utils_maps:deep_get([<<"listeners">>, <<"https">>], Conf, undefined),
+    Https1 = emqx_dashboard_schema:https_converter(Https0, #{}),
+    Conf1 = emqx_utils_maps:deep_put([<<"listeners">>, <<"https">>], Conf0, Https1),
+    Ssl = maps:get(<<"ssl_options">>, Https1, undefined),
    Opts = #{required_keys => [[<<"keyfile">>], [<<"certfile">>], [<<"cacertfile">>]]},
-    case emqx_tls_lib:ensure_ssl_files(?DIR, Https, Opts) of
+    case emqx_tls_lib:ensure_ssl_files(?DIR, Ssl, Opts) of
        {ok, undefined} ->
            {error, <<"ssl_cert_not_found">>};
-        {ok, NewHttps} ->
-            {ok,
-                emqx_utils_maps:deep_merge(Conf, #{<<"listeners">> => #{<<"https">> => NewHttps}})};
+        {ok, NewSsl} ->
+            Keys = [<<"listeners">>, <<"https">>, <<"ssl_options">>],
+            {ok, emqx_utils_maps:deep_put(Keys, Conf1, NewSsl)};
        {error, Reason} ->
            ?SLOG(error, Reason#{msg => "bad_ssl_config"}),
            {error, Reason}
--- a/apps/emqx_dashboard/src/emqx_dashboard_schema.erl
+++ b/apps/emqx_dashboard/src/emqx_dashboard_schema.erl
@ -21,7 +21,8 @@
    roots/0,
    fields/1,
    namespace/0,
-    desc/1
+    desc/1,
+    https_converter/2
 ]).

 namespace() -> dashboard.
@ -63,7 +64,8 @@ fields("dashboard") ->
                    desc => ?DESC(bootstrap_users_file),
                    required => false,
                    default => <<>>,
-                    deprecated => {since, "5.1.0"}
+                    deprecated => {since, "5.1.0"},
+                    importance => ?IMPORTANCE_HIDDEN
                }
            )}
    ];
@ -82,7 +84,8 @@ fields("listeners") ->
                ?R_REF("https"),
                #{
                    desc => "SSL listeners",
-                    required => {false, recursively}
+                    required => {false, recursively},
+                    converter => fun ?MODULE:https_converter/2
                }
            )}
    ];
@ -95,11 +98,38 @@ fields("http") ->
 fields("https") ->
    [
        enable(false),
-        bind(18084)
-        | common_listener_fields() ++ server_ssl_opts()
-    ].
+        bind(18084),
+        ssl_options()
+        | common_listener_fields() ++
+            hidden_server_ssl_options()
+    ];
+fields("ssl_options") ->
+    server_ssl_options().

-server_ssl_opts() ->
+ssl_options() ->
+    {"ssl_options",
+        ?HOCON(
+            ?R_REF("ssl_options"),
+            #{
+                required => true,
+                desc => ?DESC(ssl_options),
+                importance => ?IMPORTANCE_HIGH
+            }
+        )}.
+
+hidden_server_ssl_options() ->
+    lists:map(
+        fun({K, V}) ->
+            {K, V#{
+                importance => ?IMPORTANCE_HIDDEN,
+                default => undefined,
+                required => false
+            }}
+        end,
+        server_ssl_options()
+    ).
+
+server_ssl_options() ->
    Opts0 = emqx_schema:server_ssl_opts_schema(#{}, true),
    exclude_fields(["fail_if_no_peer_cert"], Opts0).

@ -213,6 +243,8 @@ desc("http") ->
    ?DESC(desc_http);
 desc("https") ->
    ?DESC(desc_https);
+desc("ssl_options") ->
+    ?DESC(ssl_options);
 desc(_) ->
    undefined.

@ -241,7 +273,7 @@ cors(desc) -> ?DESC(cors);
 cors(_) -> undefined.

 %% TODO: change it to string type
-%% It will be up to the dashboard package which languagues to support
+%% It will be up to the dashboard package which languages to support
 i18n_lang(type) -> ?ENUM([en, zh]);
 i18n_lang(default) -> en;
 i18n_lang('readOnly') -> true;
@ -257,3 +289,13 @@ validate_sample_interval(Second) ->
            Msg = "must be between 1 and 60 and be a divisor of 60.",
            {error, Msg}
    end.
+
+https_converter(Conf = #{<<"ssl_options">> := _}, _Opts) ->
+    Conf;
+https_converter(Conf = #{}, _Opts) ->
+    Keys = lists:map(fun({K, _}) -> list_to_binary(K) end, server_ssl_options()),
+    SslOpts = maps:with(Keys, Conf),
+    Conf1 = maps:without(Keys, Conf),
+    Conf1#{<<"ssl_options">> => SslOpts};
+https_converter(Conf, _Opts) ->
+    Conf.
--- a/apps/emqx_dashboard/test/emqx_dashboard_https_SUITE.erl
+++ b/apps/emqx_dashboard/test/emqx_dashboard_https_SUITE.erl
@ -49,7 +49,7 @@ t_update_conf(_Config) ->
    Conf = #{
        dashboard => #{
            listeners => #{
-                https => #{bind => 18084},
+                https => #{bind => 18084, ssl_options => #{depth => 5}},
                http => #{bind => 18083}
            }
        }
@ -64,6 +64,12 @@ t_update_conf(_Config) ->
        get, http_api_path(["clients"]), Headers
    ),
    Raw = emqx:get_raw_config([<<"dashboard">>]),
+    ?assertEqual(
+        5,
+        emqx_utils_maps:deep_get(
+            [<<"listeners">>, <<"https">>, <<"ssl_options">>, <<"depth">>], Raw
+        )
+    ),
    ?assertEqual(Client1, Client2),
    ?check_trace(
        begin
@ -120,7 +126,7 @@ t_default_ssl_cert(_Config) ->
    validate_https(Conf, 512, default_ssl_cert(), verify_none),
    ok.

-t_normal_ssl_cert(_Config) ->
+t_compatibility_ssl_cert(_Config) ->
    MaxConnection = 1000,
    Conf = #{
        dashboard => #{
@ -138,6 +144,29 @@ t_normal_ssl_cert(_Config) ->
    validate_https(Conf, MaxConnection, default_ssl_cert(), verify_none),
    ok.

+t_normal_ssl_cert(_Config) ->
+    MaxConnection = 1024,
+    Conf = #{
+        dashboard => #{
+            listeners => #{
+                https => #{
+                    bind => 18084,
+                    ssl_options => #{
+                        cacertfile => naive_env_interpolation(
+                            <<"${EMQX_ETC_DIR}/certs/cacert.pem">>
+                        ),
+                        certfile => naive_env_interpolation(<<"${EMQX_ETC_DIR}/certs/cert.pem">>),
+                        keyfile => naive_env_interpolation(<<"${EMQX_ETC_DIR}/certs/key.pem">>),
+                        depth => 5
+                    },
+                    max_connections => MaxConnection
+                }
+            }
+        }
+    },
+    validate_https(Conf, MaxConnection, default_ssl_cert(), verify_none),
+    ok.
+
 t_verify_cacertfile(_Config) ->
    MaxConnection = 1024,
    DefaultSSLCert = default_ssl_cert(),
--- a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl
+++ b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl
@ -222,11 +222,13 @@ t_dashboard(_Config) ->
    ),

    Https2 = #{
-        enable => true,
-        bind => 18084,
-        keyfile => "etc/certs/badkey.pem",
-        cacertfile => "etc/certs/badcacert.pem",
-        certfile => "etc/certs/badcert.pem"
+        <<"bind">> => 18084,
+        <<"ssl_options">> =>
+            #{
+                <<"keyfile">> => "etc/certs/badkey.pem",
+                <<"cacertfile">> => "etc/certs/badcacert.pem",
+                <<"certfile">> => "etc/certs/badcert.pem"
+            }
    },
    Dashboard2 = Dashboard#{<<"listeners">> => Listeners#{<<"https">> => Https2}},
    ?assertMatch(
@ -240,20 +242,21 @@ t_dashboard(_Config) ->
        emqx, filename:join(["etc", "certs", "cacert.pem"])
    ),
    Https3 = #{
-        <<"enable">> => true,
        <<"bind">> => 18084,
-        <<"keyfile">> => list_to_binary(KeyFile),
-        <<"cacertfile">> => list_to_binary(CacertFile),
-        <<"certfile">> => list_to_binary(CertFile)
+        <<"ssl_options">> => #{
+            <<"keyfile">> => list_to_binary(KeyFile),
+            <<"cacertfile">> => list_to_binary(CacertFile),
+            <<"certfile">> => list_to_binary(CertFile)
+        }
    },
    Dashboard3 = Dashboard#{<<"listeners">> => Listeners#{<<"https">> => Https3}},
    ?assertMatch({ok, _}, update_config("dashboard", Dashboard3)),

-    Dashboard4 = Dashboard#{<<"listeners">> => Listeners#{<<"https">> => #{<<"enable">> => false}}},
+    Dashboard4 = Dashboard#{<<"listeners">> => Listeners#{<<"https">> => #{<<"bind">> => 0}}},
    ?assertMatch({ok, _}, update_config("dashboard", Dashboard4)),
    {ok, Dashboard41} = get_config("dashboard"),
    ?assertEqual(
-        Https3#{<<"enable">> => false},
+        Https3#{<<"bind">> => 0},
        read_conf([<<"dashboard">>, <<"listeners">>, <<"https">>]),
        Dashboard41
    ),
--- a/changes/ce/feat-11137.en.md
+++ b/changes/ce/feat-11137.en.md
@ -0,0 +1 @@
+Refactors the dashboard listener configuration to use a nested `ssl_options` field for ssl settings.
--- a/rel/i18n/emqx_dashboard_schema.hocon
+++ b/rel/i18n/emqx_dashboard_schema.hocon
@ -7,7 +7,9 @@ backlog.label:
 """Backlog"""

 bind.desc:
-"""Port without IP(18083) or port with specified IP(127.0.0.1:18083)."""
+"""Port without IP(18083) or port with specified IP(127.0.0.1:18083).
+Disabled when setting bind to `0`.
+"""

 bind.label:
 """Bind"""
@ -136,4 +138,10 @@ token_expired_time.desc:
 token_expired_time.label:
 """Token expired time"""

+ssl_options.desc:
+"""SSL/TLS options for the dashboard listener."""
+
+ssl_options.label:
+"""SSL options"""
+
 }
--- a/scripts/conf-test/old-confs/v5.0.25.conf
+++ b/scripts/conf-test/old-confs/v5.0.25.conf
@ -14,6 +14,10 @@ dashboard {
    listeners.http {
        bind = 18083
    }
+    listeners.https {
+      bind = 18084
+      depth = 5
+    }
 }

 authentication = [
--- a/scripts/conf-test/run.sh
+++ b/scripts/conf-test/run.sh
@ -7,9 +7,23 @@ EMQX_ROOT="${EMQX_ROOT:-_build/$PROFILE/rel/emqx}"
 EMQX_WAIT_FOR_START="${EMQX_WAIT_FOR_START:-30}"
 export EMQX_WAIT_FOR_START

+function check_dashboard_https_ssl_options_depth() {
+  if [[ $1 =~ v5\.0\.25 ]]; then
+    EXPECT_DEPTH=5
+  else
+    EXPECT_DEPTH=10
+  fi
+  DEPTH=$("$EMQX_ROOT"/bin/emqx eval "emqx:get_config([dashboard,listeners,https,ssl_options,depth],10)")
+  if [[ "$DEPTH" != "$EXPECT_DEPTH" ]]; then
+    echo "Bad Https depth $DEPTH, expect $EXPECT_DEPTH"
+    exit 1
+  fi
+}
+
 start_emqx_with_conf() {
    echo "Starting $PROFILE with $1"
    "$EMQX_ROOT"/bin/emqx start
+    check_dashboard_https_ssl_options_depth "$1"
    "$EMQX_ROOT"/bin/emqx stop
 }
				`@ -0,0 +1 @@`
				Refactors the dashboard listener configuration to use a nested `ssl_options` field for ssl settings.