From e9fc656241a52b1cf72a8d3cef47b07d8d2be551 Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Wed, 4 Jun 2025 12:24:45 +0400 Subject: [PATCH 1/3] Wrap TLS options password into a function in more places A follow-up to #13958 #13999. Pair: @dcorbacho. --- deps/rabbit/src/rabbit_ssl.erl | 13 +--------- deps/rabbit/test/unit_rabbit_ssl_SUITE.erl | 4 +-- deps/rabbit_common/src/rabbit_ssl_options.erl | 25 ++++++++++++++++--- .../src/rabbit_mgmt_app.erl | 5 ++-- .../src/rabbit_prometheus_app.erl | 20 +++++++++++---- .../src/rabbit_web_dispatch_sup.erl | 8 +++--- 6 files changed, 48 insertions(+), 27 deletions(-) diff --git a/deps/rabbit/src/rabbit_ssl.erl b/deps/rabbit/src/rabbit_ssl.erl index ebc133b0d5d3..6eafe2022951 100644 --- a/deps/rabbit/src/rabbit_ssl.erl +++ b/deps/rabbit/src/rabbit_ssl.erl @@ -39,18 +39,7 @@ -spec wrap_password_opt(tls_opts()) -> tls_opts(). wrap_password_opt(Opts0) -> - case proplists:get_value(password, Opts0) of - undefined -> - Opts0; - Fun when is_function(Fun) -> - Opts0; - Password -> - %% A password can be a value or a function returning that value. - %% See the key_pem_password/0 type in https://github.com/erlang/otp/pull/5843/files. - NewOpts = proplists:delete(password, Opts0), - Fun = fun() -> Password end, - [{password, Fun} | NewOpts] - end. + rabbit_ssl_options:wrap_password_opt(Opts0). -spec cipher_suites(cipher_suites_mode()) -> ssl:ciphers(). cipher_suites(Mode) -> diff --git a/deps/rabbit/test/unit_rabbit_ssl_SUITE.erl b/deps/rabbit/test/unit_rabbit_ssl_SUITE.erl index 1c7bd90d20ea..0bf8643fb22d 100644 --- a/deps/rabbit/test/unit_rabbit_ssl_SUITE.erl +++ b/deps/rabbit/test/unit_rabbit_ssl_SUITE.erl @@ -33,7 +33,7 @@ wrap_tls_opts_with_binary_password(_Config) -> {password, Bin} ], - Opts = rabbit_ssl:wrap_password_opt(Opts0), + Opts = rabbit_ssl_options:wrap_password_opt(Opts0), M = maps:from_list(Opts), ?assertEqual(Path, maps:get(keyfile, M)), @@ -53,7 +53,7 @@ wrap_tls_opts_with_function_password(_Config) -> {password, Fun} ], - Opts = rabbit_ssl:wrap_password_opt(Opts0), + Opts = rabbit_ssl_options:wrap_password_opt(Opts0), M = maps:from_list(Opts), ?assertEqual(Path, maps:get(keyfile, M)), diff --git a/deps/rabbit_common/src/rabbit_ssl_options.erl b/deps/rabbit_common/src/rabbit_ssl_options.erl index 823a9467fddf..2916e92d3d8d 100644 --- a/deps/rabbit_common/src/rabbit_ssl_options.erl +++ b/deps/rabbit_common/src/rabbit_ssl_options.erl @@ -7,15 +7,34 @@ -module(rabbit_ssl_options). --export([fix/1]). --export([fix_client/1]). - +-export([ + fix/1, + fix_client/1, + wrap_password_opt/1 +]). -define(BAD_SSL_PROTOCOL_VERSIONS, [ %% POODLE sslv3 ]). +-type tls_opts() :: [ssl:tls_server_option()] | [ssl:tls_client_option()]. + +-spec wrap_password_opt(tls_opts()) -> tls_opts(). +wrap_password_opt(Opts0) -> + case proplists:get_value(password, Opts0) of + undefined -> + Opts0; + Fun when is_function(Fun) -> + Opts0; + Password -> + %% A password can be a value or a function returning that value. + %% See the key_pem_password/0 type in https://github.com/erlang/otp/pull/5843/files. + NewOpts = proplists:delete(password, Opts0), + Fun = fun() -> Password end, + [{password, Fun} | NewOpts] + end. + -spec fix(rabbit_types:infos()) -> rabbit_types:infos(). fix(Config) -> diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_app.erl b/deps/rabbitmq_management/src/rabbit_mgmt_app.erl index d10b645c760d..e6423ce426c5 100644 --- a/deps/rabbitmq_management/src/rabbit_mgmt_app.erl +++ b/deps/rabbitmq_management/src/rabbit_mgmt_app.erl @@ -128,16 +128,17 @@ get_legacy_listener() -> get_tls_listener() -> {ok, Listener0} = application:get_env(rabbitmq_management, ssl_config), {ok, Listener1} = ensure_port(tls, Listener0), + Listener2 = rabbit_ssl:wrap_password_opt(Listener1), Port = proplists:get_value(port, Listener1), case proplists:get_value(cowboy_opts, Listener0) of undefined -> [ {port, Port}, {ssl, true}, - {ssl_opts, Listener0} + {ssl_opts, Listener2} ]; CowboyOpts -> - WithoutCowboyOpts = lists:keydelete(cowboy_opts, 1, Listener0), + WithoutCowboyOpts = lists:keydelete(cowboy_opts, 1, Listener2), [ {port, Port}, {ssl, true}, diff --git a/deps/rabbitmq_prometheus/src/rabbit_prometheus_app.erl b/deps/rabbitmq_prometheus/src/rabbit_prometheus_app.erl index ae5d7c550b56..0a0436ef4918 100644 --- a/deps/rabbitmq_prometheus/src/rabbit_prometheus_app.erl +++ b/deps/rabbitmq_prometheus/src/rabbit_prometheus_app.erl @@ -34,7 +34,16 @@ init(_) -> -spec start_configured_listener() -> ok. start_configured_listener() -> TCPListenerConf = get_env(tcp_config, []), - TLSListenerConf = get_env(ssl_config, []), + TLSListenerConf0 = get_env(ssl_config, []), + TLSListenerConf = + case proplists:get_value(ssl_opts, TLSListenerConf0, undef) of + undef -> + TLSListenerConf0; + Opts0 -> + Opts = rabbit_ssl:wrap_password_opt(Opts0), + Tmp = proplists:delete(ssl_opts, TLSListenerConf0), + [{ssl_opts, Opts} | Tmp] + end, case {TCPListenerConf, TLSListenerConf} of %% nothing is configured @@ -64,10 +73,11 @@ start_configured_tcp_listener(Conf) -> start_configured_tls_listener(Conf) -> case Conf of [] -> ok; - SSLCon -> - SSLListener0 = [{ssl, true} | SSLCon], - SSLListener1 = maybe_disable_sendfile(SSLListener0), - start_listener(SSLListener1) + TLSConf -> + TLSListener0 = [{ssl, true} | TLSConf], + TLSListener1 = maybe_disable_sendfile(TLSListener0), + TLSListener2 = rabbit_ssl:wrap_password_opt(TLSListener1), + start_listener(TLSListener2) end. maybe_disable_sendfile(Listener) -> diff --git a/deps/rabbitmq_web_dispatch/src/rabbit_web_dispatch_sup.erl b/deps/rabbitmq_web_dispatch/src/rabbit_web_dispatch_sup.erl index 2fae65b13de3..534f4a884dec 100644 --- a/deps/rabbitmq_web_dispatch/src/rabbit_web_dispatch_sup.erl +++ b/deps/rabbitmq_web_dispatch/src/rabbit_web_dispatch_sup.erl @@ -27,7 +27,8 @@ ensure_listener(Listener) -> undefined -> {error, {no_port_given, Listener}}; _ -> - {Transport, TransportOpts, ProtoOpts} = preprocess_config(Listener), + {Transport, TransportOpts0, ProtoOpts} = preprocess_config(Listener), + TransportOpts = rabbit_ssl_options:wrap_password_opt(TransportOpts0), ProtoOptsMap = maps:from_list(ProtoOpts), StreamHandlers = stream_handlers_config(ProtoOpts), rabbit_log:debug("Starting HTTP[S] listener with transport ~ts", [Transport]), @@ -86,9 +87,10 @@ auto_ssl(Options) -> fix_ssl([{ssl_opts, SSLOpts} | Options]). fix_ssl(Options) -> - SSLOpts = proplists:get_value(ssl_opts, Options), + TLSOpts0 = proplists:get_value(ssl_opts, Options), + TLSOpts = rabbit_ssl_options:wrap_password_opt(TLSOpts0), {ranch_ssl, - transport_config(Options ++ rabbit_networking:fix_ssl_options(SSLOpts)), + transport_config(Options ++ rabbit_networking:fix_ssl_options(TLSOpts)), protocol_config(Options)}. transport_config(Options0) -> From 61dcfd5fa6ca366be21f0811dcc2b4b1fde7f6be Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Wed, 4 Jun 2025 12:31:27 +0400 Subject: [PATCH 2/3] Use the standard 'undefined' here --- deps/rabbitmq_prometheus/src/rabbit_prometheus_app.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deps/rabbitmq_prometheus/src/rabbit_prometheus_app.erl b/deps/rabbitmq_prometheus/src/rabbit_prometheus_app.erl index 0a0436ef4918..4de0b36cb8a1 100644 --- a/deps/rabbitmq_prometheus/src/rabbit_prometheus_app.erl +++ b/deps/rabbitmq_prometheus/src/rabbit_prometheus_app.erl @@ -36,8 +36,8 @@ start_configured_listener() -> TCPListenerConf = get_env(tcp_config, []), TLSListenerConf0 = get_env(ssl_config, []), TLSListenerConf = - case proplists:get_value(ssl_opts, TLSListenerConf0, undef) of - undef -> + case proplists:get_value(ssl_opts, TLSListenerConf0, undefined) of + undefined -> TLSListenerConf0; Opts0 -> Opts = rabbit_ssl:wrap_password_opt(Opts0), From 081dee8883fdc53d5c15c1fa00b954ccf4f7609d Mon Sep 17 00:00:00 2001 From: Diana Parra Corbacho Date: Wed, 4 Jun 2025 11:12:14 +0200 Subject: [PATCH 3/3] Tests: sort nested proplists --- .../test/listener_config_SUITE.erl | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/deps/rabbitmq_management/test/listener_config_SUITE.erl b/deps/rabbitmq_management/test/listener_config_SUITE.erl index 4def1fafdb04..35ba13bc6a4b 100644 --- a/deps/rabbitmq_management/test/listener_config_SUITE.erl +++ b/deps/rabbitmq_management/test/listener_config_SUITE.erl @@ -73,7 +73,7 @@ tcp_config_only(_Config) -> ]}, {port, 999} ], - ?assertEqual(lists:usort(Expected), get_single_listener_config()). + ?assertEqual(sort_nested(Expected), sort_nested(get_single_listener_config())). ssl_config_only(_Config) -> application:set_env(rabbitmq_management, ssl_config, [ @@ -92,7 +92,7 @@ ssl_config_only(_Config) -> {idle_timeout, 10000} ]} ], - ?assertEqual(lists:usort(Expected), get_single_listener_config()). + ?assertEqual(sort_nested(Expected), sort_nested(get_single_listener_config())). multiple_listeners(_Config) -> application:set_env(rabbitmq_management, tcp_config, [ @@ -126,9 +126,18 @@ multiple_listeners(_Config) -> ]} ] ], - ?assertEqual(lists:usort(Expected), rabbit_mgmt_app:get_listeners_config()). + ?assertEqual(sort_nested(Expected), sort_nested(rabbit_mgmt_app:get_listeners_config())). get_single_listener_config() -> [Config] = rabbit_mgmt_app:get_listeners_config(), lists:usort(Config). + +sort_nested(Proplist) when is_list(Proplist) -> + lists:usort(lists:map(fun({K, V}) when is_list(V) -> + {K, lists:usort(V)}; + (Any) -> + sort_nested(Any) + end, Proplist)); +sort_nested(Value) -> + Value.