Skip to content

Commit ec46123

Browse files
michaelklishinmichaelklishin
authored
Merge pull request #16201 from rabbitmq/mk-rabbitmq-server-16199
Trust store: respect user-provided `fail_if_no_peer_cert` values
2 parents 223b2ce + f637f9c commit ec46123

3 files changed

Lines changed: 264 additions & 11 deletions

File tree

‎deps/rabbitmq_trust_store/src/rabbit_trust_store_app.erl‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
-behaviour(application).
1313
-export([change_SSL_options/0]).
1414
-export([revert_SSL_options/0]).
15+
-export([merge_tls_options/1]).
1516
-export([start/2, stop/1]).
1617

1718
-rabbit_boot_step({rabbit_trust_store, [
@@ -56,21 +57,29 @@ edit(Options) ->
5657
"It will be overwritten by the plugin.", [Val]),
5758
ok
5859
end,
60+
merge_tls_options(Options).
61+
62+
merge_tls_options(Options) ->
5963
CaCerts =
6064
case lists:keymember(cacerts, 1, Options) orelse
6165
lists:keymember(cacertfile, 1, Options) of
6266
true ->
6367
[];
6468
false ->
65-
%% if verify_peer is enabled, then the ssl app
66-
%% requires a cacerts option to be set even if it is
67-
%% not used, as we rely on verify_fun instead.
69+
%% Required by the `ssl` app when `verify_peer` is set,
70+
%% even though we rely on `verify_fun`.
6871
[{cacerts, []}]
6972
end,
70-
%% Only enter those options neccessary for this application.
73+
FailIfNoPeerCert =
74+
case lists:keymember(fail_if_no_peer_cert, 1, Options) of
75+
true ->
76+
[];
77+
false ->
78+
[{fail_if_no_peer_cert, true}]
79+
end,
7180
lists:ukeymerge(
7281
1,
73-
lists:sort(CaCerts ++
82+
lists:sort(CaCerts ++ FailIfNoPeerCert ++
7483
[{verify_fun, {delegate(), continue}},
7584
{partial_chain, fun partial_chain/1}
7685
|required_options()]),
@@ -90,4 +99,4 @@ partial_chain(Chain) ->
9099
end.
91100

92101
required_options() ->
93-
[{verify, verify_peer}, {fail_if_no_peer_cert, true}].
102+
[{verify, verify_peer}].

‎deps/rabbitmq_trust_store/test/system_SUITE.erl‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,16 @@ library(_) ->
198198
ok.
199199

200200
invasive_SSL_option_change(Config) ->
201+
GroupProps = ?config(tc_group_properties, Config),
202+
GroupName = proplists:get_value(name, GroupProps),
203+
ExpectedFailIfNoPeerCert = case GroupName of
204+
required_options -> false;
205+
_ -> true
206+
end,
201207
ok = rabbit_ct_broker_helpers:rpc(Config, 0,
202-
?MODULE, invasive_SSL_option_change1, []).
208+
?MODULE, invasive_SSL_option_change1, [ExpectedFailIfNoPeerCert]).
203209

204-
invasive_SSL_option_change1() ->
210+
invasive_SSL_option_change1(ExpectedFailIfNoPeerCert) ->
205211
%% Given: Rabbit is started with the boot-steps in the
206212
%% Trust-Store's OTP Application file.
207213

@@ -210,9 +216,9 @@ invasive_SSL_option_change1() ->
210216

211217
%% Then: all necessary settings are correct.
212218
OptionsMap = proplists:to_map(lists:reverse(Options)),
213-
verify_peer = maps:get(verify, OptionsMap),
214-
true = maps:get(fail_if_no_peer_cert, OptionsMap),
215-
{Verifyfun, _UserState} = maps:get(verify_fun, OptionsMap),
219+
verify_peer = maps:get(verify, OptionsMap),
220+
ExpectedFailIfNoPeerCert = maps:get(fail_if_no_peer_cert, OptionsMap),
221+
{Verifyfun, _UserState} = maps:get(verify_fun, OptionsMap),
216222

217223
{module, rabbit_trust_store} = erlang:fun_info(Verifyfun, module),
218224
ok.

‎deps/rabbitmq_trust_store/test/tls_options_SUITE.erl‎

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
%% This Source Code Form is subject to the terms of the Mozilla Public
2+
%% License, v. 2.0. If a copy of the MPL was not distributed with this
3+
%% file, You can obtain one at https://mozilla.org/MPL/2.0/.
4+
%%
5+
%% Copyright (c) 2007-2026 Broadcom. All Rights Reserved. The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
6+
%%
7+
8+
-module(tls_options_SUITE).
9+
10+
-compile([export_all, nowarn_export_all]).
11+
12+
-include_lib("proper/include/proper.hrl").
13+
-include_lib("common_test/include/ct.hrl").
14+
-include_lib("eunit/include/eunit.hrl").
15+
16+
all() ->
17+
[{group, unit},
18+
{group, prop}].
19+
20+
groups() ->
21+
[{unit, [parallel],
22+
[fail_if_no_peer_cert_defaults_to_true,
23+
fail_if_no_peer_cert_false_is_respected,
24+
fail_if_no_peer_cert_true_is_respected,
25+
verify_always_set_to_verify_peer,
26+
verify_fun_always_set,
27+
verify_fun_overrides_user_value,
28+
partial_chain_always_set,
29+
cacerts_added_when_missing,
30+
cacerts_not_added_when_present,
31+
cacertfile_prevents_cacerts_default,
32+
both_cacerts_and_cacertfile_preserved,
33+
user_options_are_preserved]},
34+
{prop, [parallel],
35+
[prop_verify_always_verify_peer,
36+
prop_fail_if_no_peer_cert_default,
37+
prop_fail_if_no_peer_cert_respected,
38+
prop_cacerts_default_when_no_ca_opts,
39+
prop_user_options_preserved]}].
40+
41+
suite() ->
42+
[{timetrap, {seconds, 60}}].
43+
44+
init_per_suite(Config) ->
45+
Config.
46+
47+
end_per_suite(_Config) ->
48+
ok.
49+
50+
init_per_group(_GroupName, Config) ->
51+
Config.
52+
53+
end_per_group(_GroupName, Config) ->
54+
Config.
55+
56+
init_per_testcase(_Case, Config) ->
57+
Config.
58+
59+
end_per_testcase(_Case, _Config) ->
60+
ok.
61+
62+
%% -------------------------------------------------------------------
63+
%% Unit tests
64+
%% -------------------------------------------------------------------
65+
66+
fail_if_no_peer_cert_defaults_to_true(_Config) ->
67+
Opts = merge([]),
68+
true = opt(fail_if_no_peer_cert, Opts).
69+
70+
fail_if_no_peer_cert_false_is_respected(_Config) ->
71+
Opts = merge([{fail_if_no_peer_cert, false}]),
72+
false = opt(fail_if_no_peer_cert, Opts).
73+
74+
fail_if_no_peer_cert_true_is_respected(_Config) ->
75+
Opts = merge([{fail_if_no_peer_cert, true}]),
76+
true = opt(fail_if_no_peer_cert, Opts).
77+
78+
verify_always_set_to_verify_peer(_Config) ->
79+
verify_peer = opt(verify, merge([])),
80+
verify_peer = opt(verify, merge([{verify, verify_none}])),
81+
verify_peer = opt(verify, merge([{verify, verify_peer}])).
82+
83+
verify_fun_always_set(_Config) ->
84+
Opts = merge([]),
85+
{Fun, continue} = opt(verify_fun, Opts),
86+
true = is_function(Fun, 3).
87+
88+
verify_fun_overrides_user_value(_Config) ->
89+
UserFun = fun(_Cert, _Event, State) -> {valid, State} end,
90+
Opts = merge([{verify_fun, {UserFun, some_state}}]),
91+
{Fun, continue} = opt(verify_fun, Opts),
92+
true = (Fun =/= UserFun).
93+
94+
partial_chain_always_set(_Config) ->
95+
Opts = merge([]),
96+
Fun = opt(partial_chain, Opts),
97+
true = is_function(Fun, 1).
98+
99+
cacerts_added_when_missing(_Config) ->
100+
Opts = merge([]),
101+
[] = opt(cacerts, Opts).
102+
103+
cacerts_not_added_when_present(_Config) ->
104+
MyCaCerts = [<<1, 2, 3>>],
105+
Opts = merge([{cacerts, MyCaCerts}]),
106+
MyCaCerts = opt(cacerts, Opts).
107+
108+
cacertfile_prevents_cacerts_default(_Config) ->
109+
Opts = merge([{cacertfile, "/path/to/ca.pem"}]),
110+
"/path/to/ca.pem" = opt(cacertfile, Opts),
111+
undefined = opt(cacerts, Opts).
112+
113+
both_cacerts_and_cacertfile_preserved(_Config) ->
114+
MyCaCerts = [<<1, 2, 3>>],
115+
Opts = merge([{cacerts, MyCaCerts}, {cacertfile, "/path/to/ca.pem"}]),
116+
MyCaCerts = opt(cacerts, Opts),
117+
"/path/to/ca.pem" = opt(cacertfile, Opts).
118+
119+
user_options_are_preserved(_Config) ->
120+
Input = [{certfile, "/path/to/cert.pem"},
121+
{keyfile, "/path/to/key.pem"},
122+
{versions, ['tlsv1.2', 'tlsv1.3']}],
123+
Opts = merge(Input),
124+
"/path/to/cert.pem" = opt(certfile, Opts),
125+
"/path/to/key.pem" = opt(keyfile, Opts),
126+
['tlsv1.2', 'tlsv1.3'] = opt(versions, Opts).
127+
128+
%% -------------------------------------------------------------------
129+
%% Property-based tests
130+
%% -------------------------------------------------------------------
131+
132+
prop_verify_always_verify_peer(_Config) ->
133+
run_proper(
134+
fun() ->
135+
?FORALL(
136+
UserOpts, tls_options(),
137+
verify_peer =:= opt(verify, merge(UserOpts)))
138+
end).
139+
140+
prop_fail_if_no_peer_cert_default(_Config) ->
141+
run_proper(
142+
fun() ->
143+
?FORALL(
144+
UserOpts, tls_options_without(fail_if_no_peer_cert),
145+
true =:= opt(fail_if_no_peer_cert, merge(UserOpts)))
146+
end).
147+
148+
prop_fail_if_no_peer_cert_respected(_Config) ->
149+
run_proper(
150+
fun() ->
151+
?FORALL(
152+
{Val, BaseOpts}, {boolean(), tls_options_without(fail_if_no_peer_cert)},
153+
Val =:= opt(fail_if_no_peer_cert,
154+
merge([{fail_if_no_peer_cert, Val} | BaseOpts])))
155+
end).
156+
157+
prop_cacerts_default_when_no_ca_opts(_Config) ->
158+
run_proper(
159+
fun() ->
160+
?FORALL(
161+
UserOpts, tls_options_without_ca(),
162+
[] =:= opt(cacerts, merge(UserOpts)))
163+
end).
164+
165+
prop_user_options_preserved(_Config) ->
166+
run_proper(
167+
fun() ->
168+
?FORALL(
169+
UserOpts, tls_options(),
170+
begin
171+
Merged = merge(UserOpts),
172+
%% Every user option that is not overridden by the
173+
%% plugin must appear in the result.
174+
Overridden = [verify, verify_fun, partial_chain],
175+
lists:all(
176+
fun({K, V}) ->
177+
lists:member(K, Overridden) orelse
178+
opt(K, Merged) =:= V
179+
end, UserOpts)
180+
end)
181+
end).
182+
183+
%% -------------------------------------------------------------------
184+
%% Generators
185+
%% -------------------------------------------------------------------
186+
187+
tls_options() ->
188+
?LET(Opts, list(tls_option()),
189+
unique_keys(Opts)).
190+
191+
tls_options_without(Key) ->
192+
?LET(Opts, tls_options(),
193+
lists:keydelete(Key, 1, Opts)).
194+
195+
tls_options_without_ca() ->
196+
?LET(Opts, tls_options(),
197+
lists:filter(fun({K, _}) ->
198+
K =/= cacerts andalso K =/= cacertfile
199+
end, Opts)).
200+
201+
tls_option() ->
202+
oneof([
203+
{verify, oneof([verify_peer, verify_none])},
204+
{fail_if_no_peer_cert, boolean()},
205+
{depth, range(0, 10)},
206+
{certfile, binary()},
207+
{keyfile, binary()},
208+
{cacerts, list(binary())},
209+
{cacertfile, binary()},
210+
{versions, list(oneof(['tlsv1.2', 'tlsv1.3']))},
211+
{ciphers, list(binary())}
212+
]).
213+
214+
unique_keys(Opts) ->
215+
lists:foldl(fun({K, _} = Opt, Acc) ->
216+
case lists:keymember(K, 1, Acc) of
217+
true -> Acc;
218+
false -> [Opt | Acc]
219+
end
220+
end, [], Opts).
221+
222+
%% -------------------------------------------------------------------
223+
%% Helpers
224+
%% -------------------------------------------------------------------
225+
226+
merge(Options) ->
227+
rabbit_trust_store_app:merge_tls_options(Options).
228+
229+
opt(Key, Options) ->
230+
proplists:get_value(Key, Options).
231+
232+
run_proper(Fun) ->
233+
?assert(proper:counterexample(
234+
Fun(),
235+
[{numtests, 100},
236+
{on_output, fun(".", _) -> ok;
237+
(F, A) -> ct:pal(?LOW_IMPORTANCE, F, A)
238+
end}])).

0 commit comments

Comments
 (0)
⚡