From d9f9e951ec6ee47835f37ecb718516b49e93a9de Mon Sep 17 00:00:00 2001 From: Zhongwen Deng Date: Thu, 11 May 2023 16:24:01 +0800 Subject: [PATCH 1/4] fix: bad listeners default ssl_options --- apps/emqx/src/emqx_schema.erl | 18 ++--- .../emqx_conf/test/emqx_conf_schema_tests.erl | 81 +++++++++++++++++++ 2 files changed, 90 insertions(+), 9 deletions(-) diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index cba67aca4..80fc6f2ad 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -2200,7 +2200,7 @@ common_ssl_opts_schema(Defaults) -> sc( binary(), #{ - default => D("cacertfile"), + default => cert_file("cacert.pem"), required => false, desc => ?DESC(common_ssl_opts_schema_cacertfile) } @@ -2209,7 +2209,7 @@ common_ssl_opts_schema(Defaults) -> sc( binary(), #{ - default => D("certfile"), + default => cert_file("cert.pem"), required => false, desc => ?DESC(common_ssl_opts_schema_certfile) } @@ -2218,7 +2218,7 @@ common_ssl_opts_schema(Defaults) -> sc( binary(), #{ - default => D("keyfile"), + default => cert_file("key.pem"), required => false, desc => ?DESC(common_ssl_opts_schema_keyfile) } @@ -3251,13 +3251,10 @@ default_listener(ws) -> }; default_listener(SSLListener) -> %% The env variable is resolved in emqx_tls_lib by calling naive_env_interpolate - CertFile = fun(Name) -> - iolist_to_binary("${EMQX_ETC_DIR}/" ++ filename:join(["certs", Name])) - end, SslOptions = #{ - <<"cacertfile">> => CertFile(<<"cacert.pem">>), - <<"certfile">> => CertFile(<<"cert.pem">>), - <<"keyfile">> => CertFile(<<"key.pem">>) + <<"cacertfile">> => cert_file(<<"cacert.pem">>), + <<"certfile">> => cert_file(<<"cert.pem">>), + <<"keyfile">> => cert_file(<<"key.pem">>) }, case SSLListener of ssl -> @@ -3374,3 +3371,6 @@ ensure_default_listener(#{<<"default">> := _} = Map, _ListenerType) -> ensure_default_listener(Map, ListenerType) -> NewMap = Map#{<<"default">> => default_listener(ListenerType)}, keep_default_tombstone(NewMap, #{}). + +cert_file(File) -> + iolist_to_binary(filename:join(["${EMQX_ETC_DIR}", "certs", File])). diff --git a/apps/emqx_conf/test/emqx_conf_schema_tests.erl b/apps/emqx_conf/test/emqx_conf_schema_tests.erl index 667d1766f..79fe30293 100644 --- a/apps/emqx_conf/test/emqx_conf_schema_tests.erl +++ b/apps/emqx_conf/test/emqx_conf_schema_tests.erl @@ -116,6 +116,87 @@ authn_validations_test() -> ), ok. +%% erlfmt-ignore +-define(LISTENERS, + """ + listeners.ssl.default.bind = 9999 + listeners.wss.default.bind = 9998 + listeners.wss.default.ssl_options.cacertfile = \"mytest/certs/cacert.pem\" + listeners.wss.new.bind = 9997 + listeners.wss.new.websocket.mqtt_path = \"/my-mqtt\" + """ +). + +listeners_test() -> + BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]), + + Conf = <>, + {ok, ConfMap0} = hocon:binary(Conf, #{format => richmap}), + {_, ConfMap} = hocon_tconf:map_translate(emqx_conf_schema, ConfMap0, #{format => richmap}), + #{<<"listeners">> := Listeners} = hocon_util:richmap_to_map(ConfMap), + #{ + <<"tcp">> := #{<<"default">> := Tcp}, + <<"ws">> := #{<<"default">> := Ws}, + <<"wss">> := #{<<"default">> := DefaultWss, <<"new">> := NewWss}, + <<"ssl">> := #{<<"default">> := Ssl} + } = Listeners, + DefaultCacertFile = <<"${EMQX_ETC_DIR}/certs/cacert.pem">>, + DefaultCertFile = <<"${EMQX_ETC_DIR}/certs/cert.pem">>, + DefaultKeyFile = <<"${EMQX_ETC_DIR}/certs/key.pem">>, + ?assertMatch( + #{ + <<"bind">> := {{0, 0, 0, 0}, 1883}, + <<"enabled">> := true + }, + Tcp + ), + ?assertMatch( + #{ + <<"bind">> := {{0, 0, 0, 0}, 8083}, + <<"enabled">> := true, + <<"websocket">> := #{<<"mqtt_path">> := "/mqtt"} + }, + Ws + ), + ?assertMatch( + #{ + <<"bind">> := 9999, + <<"ssl_options">> := #{ + <<"cacertfile">> := DefaultCacertFile, + <<"certfile">> := DefaultCertFile, + <<"keyfile">> := DefaultKeyFile + } + }, + Ssl + ), + ?assertMatch( + #{ + <<"bind">> := 9998, + <<"websocket">> := #{<<"mqtt_path">> := "/mqtt"}, + <<"ssl_options">> := + #{ + <<"cacertfile">> := <<"mytest/certs/cacert.pem">>, + <<"certfile">> := DefaultCertFile, + <<"keyfile">> := DefaultKeyFile + } + }, + DefaultWss + ), + ?assertMatch( + #{ + <<"bind">> := 9997, + <<"websocket">> := #{<<"mqtt_path">> := "/my-mqtt"}, + <<"ssl_options">> := + #{ + <<"cacertfile">> := DefaultCacertFile, + <<"certfile">> := DefaultCertFile, + <<"keyfile">> := DefaultKeyFile + } + }, + NewWss + ), + ok. + doc_gen_test() -> %% the json file too large to encode. { From 3d41449fde6df7d7a92ea23bf0f3128470c5ee9c Mon Sep 17 00:00:00 2001 From: Zhongwen Deng Date: Thu, 11 May 2023 17:33:18 +0800 Subject: [PATCH 2/4] fix: only fill cerf_file default in server side --- apps/emqx/src/emqx_schema.erl | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 80fc6f2ad..1779457e1 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -2189,8 +2189,8 @@ filter(Opts) -> %% @private This function defines the SSL opts which are commonly used by %% SSL listener and client. --spec common_ssl_opts_schema(map()) -> hocon_schema:field_schema(). -common_ssl_opts_schema(Defaults) -> +-spec common_ssl_opts_schema(map(), server | client) -> hocon_schema:field_schema(). +common_ssl_opts_schema(Defaults, Type) -> D = fun(Field) -> maps:get(to_atom(Field), Defaults, undefined) end, Df = fun(Field, Default) -> maps:get(to_atom(Field), Defaults, Default) end, Collection = maps:get(versions, Defaults, tls_all_available), @@ -2200,7 +2200,7 @@ common_ssl_opts_schema(Defaults) -> sc( binary(), #{ - default => cert_file("cacert.pem"), + default => cert_file("cacert.pem", Type), required => false, desc => ?DESC(common_ssl_opts_schema_cacertfile) } @@ -2209,7 +2209,7 @@ common_ssl_opts_schema(Defaults) -> sc( binary(), #{ - default => cert_file("cert.pem"), + default => cert_file("cert.pem", Type), required => false, desc => ?DESC(common_ssl_opts_schema_certfile) } @@ -2218,7 +2218,7 @@ common_ssl_opts_schema(Defaults) -> sc( binary(), #{ - default => cert_file("key.pem"), + default => cert_file("key.pem", Type), required => false, desc => ?DESC(common_ssl_opts_schema_keyfile) } @@ -2305,7 +2305,7 @@ common_ssl_opts_schema(Defaults) -> server_ssl_opts_schema(Defaults, IsRanchListener) -> D = fun(Field) -> maps:get(to_atom(Field), Defaults, undefined) end, Df = fun(Field, Default) -> maps:get(to_atom(Field), Defaults, Default) end, - common_ssl_opts_schema(Defaults) ++ + common_ssl_opts_schema(Defaults, server) ++ [ {"dhfile", sc( @@ -2431,7 +2431,7 @@ crl_outer_validator(_SSLOpts) -> %% @doc Make schema for SSL client. -spec client_ssl_opts_schema(map()) -> hocon_schema:field_schema(). client_ssl_opts_schema(Defaults) -> - common_ssl_opts_schema(Defaults) ++ + common_ssl_opts_schema(Defaults, client) ++ [ {"enable", sc( @@ -3252,9 +3252,9 @@ default_listener(ws) -> default_listener(SSLListener) -> %% The env variable is resolved in emqx_tls_lib by calling naive_env_interpolate SslOptions = #{ - <<"cacertfile">> => cert_file(<<"cacert.pem">>), - <<"certfile">> => cert_file(<<"cert.pem">>), - <<"keyfile">> => cert_file(<<"key.pem">>) + <<"cacertfile">> => cert_file(<<"cacert.pem">>, server), + <<"certfile">> => cert_file(<<"cert.pem">>, server), + <<"keyfile">> => cert_file(<<"key.pem">>, server) }, case SSLListener of ssl -> @@ -3372,5 +3372,5 @@ ensure_default_listener(Map, ListenerType) -> NewMap = Map#{<<"default">> => default_listener(ListenerType)}, keep_default_tombstone(NewMap, #{}). -cert_file(File) -> - iolist_to_binary(filename:join(["${EMQX_ETC_DIR}", "certs", File])). +cert_file(_File, client) -> undefined; +cert_file(File, server) -> iolist_to_binary(filename:join(["${EMQX_ETC_DIR}", "certs", File])). From 91f97f6c29a3e7e490cc1197f8d6f7b0ffd7fa92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9F=90=E6=96=87?= Date: Thu, 11 May 2023 21:26:23 +0800 Subject: [PATCH 3/4] fix: ocsp cache SUITE failed --- apps/emqx/test/emqx_ocsp_cache_SUITE.erl | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/apps/emqx/test/emqx_ocsp_cache_SUITE.erl b/apps/emqx/test/emqx_ocsp_cache_SUITE.erl index 75c41b9fb..b0ba4f0e2 100644 --- a/apps/emqx/test/emqx_ocsp_cache_SUITE.erl +++ b/apps/emqx/test/emqx_ocsp_cache_SUITE.erl @@ -967,20 +967,11 @@ do_t_validations(_Config) -> {error, {_, _, ResRaw3}} = update_listener_via_api(ListenerId, ListenerData3), #{<<"code">> := <<"BAD_REQUEST">>, <<"message">> := MsgRaw3} = emqx_utils_json:decode(ResRaw3, [return_maps]), + %% we can't remove certfile now, because it has default value. ?assertMatch( - #{ - <<"mismatches">> := - #{ - <<"listeners:ssl_not_required_bind">> := - #{ - <<"reason">> := - <<"Server certificate must be defined when using OCSP stapling">> - } - } - }, - emqx_utils_json:decode(MsgRaw3, [return_maps]) + <<"{bad_ssl_config,#{file_read => enoent,pem_check => invalid_pem", _/binary>>, + MsgRaw3 ), - ok. t_unknown_error_fetching_ocsp_response(_Config) -> From 44212f6e0667468f29d86051c1cdcb727eb21a93 Mon Sep 17 00:00:00 2001 From: Zhongwen Deng Date: Fri, 12 May 2023 11:18:15 +0800 Subject: [PATCH 4/4] chore: add listener default changelog --- changes/ee/fix-10672.en.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changes/ee/fix-10672.en.md diff --git a/changes/ee/fix-10672.en.md b/changes/ee/fix-10672.en.md new file mode 100644 index 000000000..cfd622701 --- /dev/null +++ b/changes/ee/fix-10672.en.md @@ -0,0 +1,2 @@ +Fix the issue where the lack of a default value for ssl_options in listeners results in startup failure. +For example, such command(`EMQX_LISTENERS__WSS__DEFAULT__BIND='0.0.0.0:8089' ./bin/emqx console`) would have caused a crash before.