Merge pull request #10672 from zhongwencool/fix-listener-default

fix: bad listeners default ssl_options

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 => D("cacertfile"),
+                    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 => D("certfile"),
+                    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 => D("keyfile"),
+                    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(
@@ -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">>, server),
+        <<"certfile">> => cert_file(<<"cert.pem">>, server),
+        <<"keyfile">> => cert_file(<<"key.pem">>, server)
     },
     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, client) -> undefined;
+cert_file(File, server) -> iolist_to_binary(filename:join(["${EMQX_ETC_DIR}", "certs", File])).

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) ->

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 = <<BaseConf/binary, ?LISTENERS>>,
+    {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.
     {

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.