Skip to content

Commit

Permalink
Merge pull request #4391 from esl/MIM-2314-reject-verify_peer-without…
Browse files Browse the repository at this point in the history
…-cacertfile

Mim 2314 reject verify peer without cacertfile
  • Loading branch information
chrzaszcz authored Nov 20, 2024
2 parents de22c49 + 9eef160 commit 1d43a17
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 14 deletions.
1 change: 1 addition & 0 deletions rel/mim1.vars-toml.config
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
shaper = \"c2s_shaper\"
max_stanza_size = 65536
tls.certfile = \"priv/ssl/fake_server.pem\"
tls.cacertfile = \"priv/ssl/cacert.pem\"
tls.mode = \"tls\""}.
{listen_service,
"[[listen.service]]
Expand Down
64 changes: 60 additions & 4 deletions src/config/mongoose_config_spec.erl
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
tls/2]).

%% callbacks for the 'process' step
-export([process_root/1,
-export([process_dynamic_domains/1,
process_s2s/1,
process_host/1,
process_general/1,
process_listener/2,
process_c2s_tls/1,
process_c2s_just_tls/1,
process_c2s_fast_tls/1,
process_just_tls/1,
process_fast_tls/1,
process_sasl_external/1,
Expand Down Expand Up @@ -106,7 +108,7 @@ root() ->
},
defaults = #{<<"internal_databases">> => default_internal_databases()},
required = [<<"general">>],
process = fun ?MODULE:process_root/1,
process = [fun ?MODULE:process_dynamic_domains/1, fun ?MODULE:process_s2s/1],
wrap = none,
format_items = list
}.
Expand Down Expand Up @@ -701,7 +703,7 @@ tls(c2s, just_tls) ->
validate = filename}}},
process = fun ?MODULE:process_c2s_just_tls/1};
tls(c2s, fast_tls) ->
#section{}.
#section{process = fun ?MODULE:process_c2s_fast_tls/1}.

server_name_indication() ->
#section{items = #{<<"enabled">> => #option{type = boolean},
Expand Down Expand Up @@ -938,8 +940,9 @@ s2s_address() ->

%% Callbacks for 'process'

%% Callback for root level `process` function
%% Check that all auth methods and modules enabled for any host type support dynamic domains
process_root(Items) ->
process_dynamic_domains(Items) ->
case proplists:lookup(host_types, Items) of
{_, [_|_] = HostTypes} ->
HTItems = lists:filter(fun(Item) -> is_host_type_item(Item, HostTypes) end, Items),
Expand Down Expand Up @@ -975,6 +978,48 @@ extract_modules(KVs) ->
(_) -> []
end, KVs)).

%% Callback for root level `process` function
%% Ensure that CA Certificate file (`cacertfile` option) is provided for s2s listeners
%% if user configured s2s `use_starttls` as `required` or `required_trusted`
%% for at least one host, host_type or globally.
process_s2s(Items) ->
UseStartTlsValues = lists:flatmap(fun({{s2s, _}, S2S}) -> [maps:get(use_starttls, S2S)];
(_) -> [] end, Items),
case lists:any(fun is_starttls_required/1, UseStartTlsValues) of
true ->
check_s2s_verify_mode_cacertfile(Items);
_ ->
Items
end.

is_starttls_required(required) ->
true;
is_starttls_required(required_trusted) ->
true;
is_starttls_required(_) ->
false.

check_s2s_verify_mode_cacertfile(Items) ->
case lists:keyfind(listen, 1, Items) of
false ->
Items;
{_, ListenItems} ->
S2S = [Item || Item <- ListenItems, maps:get(module, Item) == ejabberd_s2s_in],
lists:foreach(fun verify_s2s_tls/1, S2S),
Items
end.

verify_s2s_tls(#{tls := #{cacertfile := _}} = Item) ->
Item;
verify_s2s_tls(#{tls := #{verify_mode := none}} = Item) ->
Item;
verify_s2s_tls(_) ->
error(#{what => missing_cacertfile,
text => <<"You need to provide CA certificate (cacertfile) "
"or disable peer verification (set `verify_mode` to `none`) "
"in the `s2s.listen` sections when any of the s2s sections "
"contains use_starttls as `required` or `required_trusted`.">>}).

is_host_type_item({{_, HostType}, _}, HostTypes) ->
HostType =:= global orelse lists:member(HostType, HostTypes);
is_host_type_item(_, _) ->
Expand Down Expand Up @@ -1045,6 +1090,17 @@ fast_tls_defaults() ->
#{ciphers => mongoose_tls:default_ciphers(),
protocol_options => ["no_sslv2", "no_sslv3", "no_tlsv1", "no_tlsv1_1"]}.

process_c2s_fast_tls(M = #{module := just_tls}) ->
M;
process_c2s_fast_tls(M = #{cacertfile := _}) ->
M;
process_c2s_fast_tls(M = #{verify_mode := none}) ->
M;
process_c2s_fast_tls(_) ->
error(#{what => missing_cacertfile,
text => <<"You need to provide CA certificate (cacertfile) "
"or disable peer verification (verify_mode)">>}).

process_listener([item, Type | _], Opts) ->
mongoose_listener_config:ensure_ip_options(Opts#{module => listener_module(Type),
connection_type => connection_type(Type)}).
Expand Down
3 changes: 2 additions & 1 deletion test/common/config_parser_helper.erl
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ options("mongooseim-pgsql") ->
access => c2s,
shaper => c2s_shaper,
max_stanza_size => 65536,
tls => #{certfile => "priv/dc1.pem", dhfile => "priv/dh.pem"}
tls => #{certfile => "priv/dc1.pem", dhfile => "priv/dh.pem",
cacertfile => "priv/ca.pem"}
}),
config([listen, c2s],
#{port => 5223,
Expand Down
49 changes: 40 additions & 9 deletions test/config_parser_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ groups() ->
listen_c2s_just_tls,
listen_s2s,
listen_s2s_tls,
listen_s2s_cacertfile_verify,
listen_service,
listen_http,
listen_http_tls,
Expand Down Expand Up @@ -508,11 +509,18 @@ listen_c2s_fast_tls(_Config) ->
T = fun(Opts) -> listen_raw(c2s, #{<<"port">> => 5222,
<<"tls">> => Opts}) end,
P = [listen, 1, tls],
?cfg(P, default_c2s_tls(fast_tls), T(#{})),
M = tls_ca_raw(),
?cfg(P, maps:merge(default_c2s_tls(fast_tls), tls_ca()), T(M)),
test_fast_tls_server(P, T),
?cfg(P ++ [mode], tls, T(#{<<"mode">> => <<"tls">>})),
?err(T(#{<<"mode">> => <<"stopttls">>})),
?err(T(#{<<"module">> => <<"slow_tls">>})).
%% we do not require `cacertfile` when `verify_mode` is `none`
?cfg(P ++ [verify_mode], none, T(#{<<"verify_mode">> => <<"none">>})),
%% we require `cacertfile` when `verify_mode` is `peer` (which is the default)
?cfg(P ++ [cacertfile], "priv/ca.pem", T(M#{<<"verify_mode">> => <<"peer">>})),
?err([#{reason := missing_cacertfile}], T(#{})),
?err([#{reason := missing_cacertfile}], T(#{<<"verify_mode">> => <<"peer">>})),
?cfg(P ++ [mode], tls, T(M#{<<"mode">> => <<"tls">>})),
?err(T(M#{<<"mode">> => <<"stopttls">>})),
?err(T(M#{<<"module">> => <<"slow_tls">>})).

listen_c2s_just_tls(_Config) ->
T = fun(Opts) -> listen_raw(c2s, #{<<"port">> => 5222,
Expand Down Expand Up @@ -545,6 +553,28 @@ listen_s2s_tls(_Config) ->
?cfg(P, default_config([listen, s2s, tls]), T(#{})),
test_fast_tls_server(P, T).

listen_s2s_cacertfile_verify(_Config) ->
T = fun(UseStartTLS, Opts) ->
maps:merge(#{<<"s2s">> => #{<<"use_starttls">> => UseStartTLS}},
listen_raw(s2s, #{<<"port">> => 5269, <<"tls">> => Opts})) end,
P = [listen, 1, tls],
ConfigWithCA = maps:merge(default_config([listen, s2s, tls]), tls_ca()),
%% no checking of `cacertfile` when `use_starttls` is `false` or `optional`
?cfg(P, default_config([listen, s2s, tls]), T(<<"false">>, #{})),
?cfg(P, default_config([listen, s2s, tls]), T(<<"optional">>, #{})),
%% `cacertfile` is required when `use_starttls` is `required` or `optional`
?cfg(P, ConfigWithCA, T(<<"required">>, tls_ca_raw())),
?cfg(P, ConfigWithCA, T(<<"required_trusted">>, tls_ca_raw())),
?err([#{reason := missing_cacertfile}], T(<<"required">>, #{})),
?err([#{reason := missing_cacertfile}], T(<<"required_trusted">>, #{})),
%% setting `verify_mode` to `none` turns off `cacertfile` validation
VerifyModeNone = #{verify_mode => none},
VerifyModeNoneRaw = #{<<"verify_mode">> => <<"none">>},
ConfigWithVerifyModeNone = maps:merge(default_config([listen, s2s, tls]),
#{verify_mode => none}),
?cfg(P, ConfigWithVerifyModeNone, T(<<"required">>, VerifyModeNoneRaw)),
?cfg(P, ConfigWithVerifyModeNone, T(<<"required_trusted">>, VerifyModeNoneRaw)).

listen_service(_Config) ->
T = fun(Opts) -> listen_raw(service, maps:merge(#{<<"port">> => 8888,
<<"password">> => <<"secret">>}, Opts))
Expand Down Expand Up @@ -1197,12 +1227,13 @@ test_just_tls_client_sni(ParentP, ParentT) ->

test_fast_tls_server(P, T) ->
?cfg(P ++ [verify_mode], none, T(#{<<"verify_mode">> => <<"none">>})),
?cfg(P ++ [certfile], "priv/cert.pem", T(#{<<"certfile">> => <<"priv/cert.pem">>})),
?cfg(P ++ [cacertfile], "priv/ca.pem", T(tls_ca_raw())),
M = tls_ca_raw(),
?cfg(P ++ [certfile], "priv/cert.pem", T(M#{<<"certfile">> => <<"priv/cert.pem">>})),
?cfg(P ++ [cacertfile], "priv/ca.pem", T(M)),
?cfg(P ++ [ciphers], "TLS_AES_256_GCM_SHA384",
T(#{<<"ciphers">> => <<"TLS_AES_256_GCM_SHA384">>})),
?cfg(P ++ [dhfile], "priv/dh.pem", T(#{<<"dhfile">> => <<"priv/dh.pem">>})),
?cfg(P ++ [protocol_options], ["nosslv2"], T(#{<<"protocol_options">> => [<<"nosslv2">>]})),
T(M#{<<"ciphers">> => <<"TLS_AES_256_GCM_SHA384">>})),
?cfg(P ++ [dhfile], "priv/dh.pem", T(M#{<<"dhfile">> => <<"priv/dh.pem">>})),
?cfg(P ++ [protocol_options], ["nosslv2"], T(M#{<<"protocol_options">> => [<<"nosslv2">>]})),
?err(T(#{<<"verify_mode">> => <<"selfsigned_peer">>})), % value only for just_tls
?err(T(#{<<"crl_files">> => [<<"priv/cert.pem">>]})), % option only for just_tls
?err(T(#{<<"certfile">> => <<"no_such_file.pem">>})),
Expand Down
1 change: 1 addition & 0 deletions test/config_parser_SUITE_data/mongooseim-pgsql.toml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
tls.mode = "starttls"
tls.certfile = "priv/dc1.pem"
tls.dhfile = "priv/dh.pem"
tls.cacertfile = "priv/ca.pem"

[[listen.c2s]]
port = 5223
Expand Down

0 comments on commit 1d43a17

Please sign in to comment.