From 11de6dc88ee69521cba384fcb62ac7bfd75d9edb Mon Sep 17 00:00:00 2001 From: Tomasz Wojcikowski Date: Tue, 9 Dec 2025 16:59:03 +0100 Subject: [PATCH 1/4] Add TLS keyfile validation --- src/config/mongoose_config_spec.erl | 20 +++++++++++++++++-- test/common/config_parser_helper.erl | 4 +++- test/config_parser_SUITE.erl | 20 +++++++++++++------ test/config_parser_SUITE_data/modules.toml | 1 + .../mongooseim-pgsql.toml | 1 + 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/config/mongoose_config_spec.erl b/src/config/mongoose_config_spec.erl index ce3d502afe1..7c0cd5935b9 100644 --- a/src/config/mongoose_config_spec.erl +++ b/src/config/mongoose_config_spec.erl @@ -24,7 +24,8 @@ process_acl_condition/1, process_s2s_host_policy/1, process_s2s_address/1, - process_infinity_as_zero/1]). + process_infinity_as_zero/1, + validate_tls_options/1]). %% For tests -export([configurable_modules/0]). @@ -655,7 +656,8 @@ tls(common) -> }, defaults = #{<<"verify_mode">> => peer, <<"disconnect_on_failure">> => true, - <<"crl_files">> => []}}; + <<"crl_files">> => []}, + process = fun validate_tls_options/1}; tls(server) -> #section{items = #{<<"dhfile">> => #option{type = string, validate = filename}, <<"early_data">> => #option{type = boolean}, @@ -1096,3 +1098,17 @@ ip_version(T) when tuple_size(T) =:= 8 -> inet6. process_infinity_as_zero(infinity) -> 0; process_infinity_as_zero(Num) -> Num. + +%% Validate that certfile and keyfile are separate files +%% OTP 28.1+ doesn't allow concatenated certificate with private key +validate_tls_options(TLSConfig) -> + case maps:is_key(certfile, TLSConfig) andalso not maps:is_key(keyfile, TLSConfig) of + true -> + error(#{what => tls_certfile_without_keyfile, + text => <<"TLS certfile provided without a separate keyfile. " + "Since OTP 28.1, concatenated certificate and private key files " + "are not supported. Please provide a separate keyfile option.">>, + certfile => maps:get(certfile, TLSConfig)}); + false -> + TLSConfig + end. diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index 584a1ae123a..1e3fe0b5902 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -245,6 +245,7 @@ options("mongooseim-pgsql") -> max_stanza_size => 131072, tls => #{cacertfile => "priv/ca.pem", certfile => "priv/cert.pem", + keyfile => "priv/dc1.pem", dhfile => "priv/dh.pem"} }) ]}, @@ -477,7 +478,8 @@ all_modules() -> connections_per_endpoint => 30, tls => config([modules, mod_global_distrib, connections, tls], #{cacertfile => "priv/ca.pem", - certfile => "priv/dc1.pem"}) + certfile => "priv/dc1.pem", + keyfile => "priv/dc1.pem"}) }) }), mod_pubsub => diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index b506b962a07..d9bffcd561f 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -1173,7 +1173,9 @@ test_just_tls_common(P, T) -> ?cfg(P ++ [verify_mode], none, T(#{<<"verify_mode">> => <<"none">>})), M = tls_ca_raw(), ?cfg(P ++ [cacertfile], "priv/ca.pem", T(M)), - ?cfg(P ++ [certfile], "priv/cert.pem", T(M#{<<"certfile">> => <<"priv/cert.pem">>})), + %% Certfile with keyfile should work + ?cfg(P ++ [certfile], "priv/cert.pem", T(M#{<<"certfile">> => <<"priv/cert.pem">>, + <<"keyfile">> => <<"priv/dc1.pem">>})), ?cfg(P ++ [ciphers], "TLS_AES_256_GCM_SHA384", T(M#{<<"ciphers">> => <<"TLS_AES_256_GCM_SHA384">>})), ?cfg(P ++ [keyfile], "priv/dc1.pem", T(M#{<<"keyfile">> => <<"priv/dc1.pem">>})), @@ -1181,12 +1183,15 @@ test_just_tls_common(P, T) -> ?cfg(P ++ [versions], ['tlsv1.2', 'tlsv1.3'], T(M#{<<"versions">> => [<<"tlsv1.2">>, <<"tlsv1.3">>]})), ?err(T(#{<<"verify_mode">> => <<"whatever">>})), - ?err(T(M#{<<"certfile">> => <<"no_such_file.pem">>})), ?err(T(M#{<<"cacertfile">> => <<"no_such_file.pem">>})), ?err(T(M#{<<"ciphers">> => [<<"TLS_AES_256_GCM_SHA384">>]})), ?err(T(M#{<<"keyfile">> => <<"no_such_file.pem">>})), ?err(T(M#{<<"password">> => false})), - ?err(T(M#{<<"versions">> => <<"tlsv1.2">>})). + ?err(T(M#{<<"versions">> => <<"tlsv1.2">>})), + %% Certfile without keyfile should fail (OTP 28.1+ requirement) + ?err(T(#{<<"certfile">> => <<"priv/cert.pem">>})), + ?err(T(M#{<<"certfile">> => <<"priv/cert.pem">>})), + ?err(T(M#{<<"certfile">> => <<"no_such_file.pem">>})). test_just_tls_client_sni(ParentP, ParentT) -> P = ParentP ++ [server_name_indication], @@ -1447,7 +1452,8 @@ s2s_outgoing_tls(_Config) -> ?cfgh(P, default_config(P), T(#{})), % default options if tls section is present ?cfgh(P ++ [verify_mode], none, T(#{<<"verify_mode">> => <<"none">>})), ?cfgh(P ++ [cacertfile], "priv/ca.pem", T(tls_ca_raw())), - ?cfgh(P ++ [certfile], "priv/cert.pem", T(#{<<"certfile">> => <<"priv/cert.pem">>})), + ?cfgh(P ++ [certfile], "priv/cert.pem", T(#{<<"certfile">> => <<"priv/cert.pem">>, + <<"keyfile">> => <<"priv/dc1.pem">>})), ?cfgh(P ++ [ciphers], "TLS_AES_256_GCM_SHA384", T(#{<<"ciphers">> => <<"TLS_AES_256_GCM_SHA384">>})), ?cfgh(P ++ [keyfile], "priv/dc1.pem", T(#{<<"keyfile">> => <<"priv/dc1.pem">>})), @@ -1455,12 +1461,14 @@ s2s_outgoing_tls(_Config) -> ?cfgh(P ++ [versions], ['tlsv1.2', 'tlsv1.3'], T(#{<<"versions">> => [<<"tlsv1.2">>, <<"tlsv1.3">>]})), ?err(T(#{<<"verify_mode">> => <<"whatever">>})), - ?err(T(#{<<"certfile">> => <<"no_such_file.pem">>})), ?err(T(#{<<"cacertfile">> => <<"no_such_file.pem">>})), ?err(T(#{<<"ciphers">> => [<<"TLS_AES_256_GCM_SHA384">>]})), ?err(T(#{<<"keyfile">> => <<"no_such_file.pem">>})), ?err(T(#{<<"password">> => false})), - ?err(T(#{<<"versions">> => <<"tlsv1.2">>})). + ?err(T(#{<<"versions">> => <<"tlsv1.2">>})), + %% Certfile without keyfile should fail (OTP 28.1+ requirement) + ?err(T(#{<<"certfile">> => <<"priv/cert.pem">>})), + ?err(T(#{<<"certfile">> => <<"no_such_file.pem">>})). %% modules diff --git a/test/config_parser_SUITE_data/modules.toml b/test/config_parser_SUITE_data/modules.toml index a57dfa076dc..5f02f38e1c8 100644 --- a/test/config_parser_SUITE_data/modules.toml +++ b/test/config_parser_SUITE_data/modules.toml @@ -121,6 +121,7 @@ connections.endpoints = [{host = "172.16.0.2", port = 5555}] connections.advertised_endpoints = [{host = "172.16.0.2", port = 5555}] connections.tls.certfile = "priv/dc1.pem" + connections.tls.keyfile = "priv/dc1.pem" connections.tls.cacertfile = "priv/ca.pem" connections.connections_per_endpoint = 30 cache.domain_lifetime_seconds = 60 diff --git a/test/config_parser_SUITE_data/mongooseim-pgsql.toml b/test/config_parser_SUITE_data/mongooseim-pgsql.toml index 98565b53015..b6047df4971 100644 --- a/test/config_parser_SUITE_data/mongooseim-pgsql.toml +++ b/test/config_parser_SUITE_data/mongooseim-pgsql.toml @@ -99,6 +99,7 @@ max_stanza_size = 131072 tls.dhfile = "priv/dh.pem" tls.certfile = "priv/cert.pem" + tls.keyfile = "priv/dc1.pem" tls.cacertfile = "priv/ca.pem" [[listen.component]] From de67c58d990ab8b88cbc431a0bd162ef2da1f310 Mon Sep 17 00:00:00 2001 From: Tomasz Wojcikowski Date: Tue, 9 Dec 2025 18:02:08 +0100 Subject: [PATCH 2/4] Remove unused export of validate_tls_options/1 from mongoose_config_spec.erl --- src/config/mongoose_config_spec.erl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/config/mongoose_config_spec.erl b/src/config/mongoose_config_spec.erl index 7c0cd5935b9..a0e9ff57dc4 100644 --- a/src/config/mongoose_config_spec.erl +++ b/src/config/mongoose_config_spec.erl @@ -24,8 +24,7 @@ process_acl_condition/1, process_s2s_host_policy/1, process_s2s_address/1, - process_infinity_as_zero/1, - validate_tls_options/1]). + process_infinity_as_zero/1]). %% For tests -export([configurable_modules/0]). From 130ebc2b365f341f09eedc88818d46425d93f825 Mon Sep 17 00:00:00 2001 From: Tomasz Wojcikowski Date: Wed, 10 Dec 2025 17:35:54 +0100 Subject: [PATCH 3/4] Update TLS certfile path in modules.toml for connection configuration --- test/config_parser_SUITE_data/modules.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/config_parser_SUITE_data/modules.toml b/test/config_parser_SUITE_data/modules.toml index 5f02f38e1c8..e8fd291331b 100644 --- a/test/config_parser_SUITE_data/modules.toml +++ b/test/config_parser_SUITE_data/modules.toml @@ -120,7 +120,7 @@ local_host = "datacenter1.example.com" connections.endpoints = [{host = "172.16.0.2", port = 5555}] connections.advertised_endpoints = [{host = "172.16.0.2", port = 5555}] - connections.tls.certfile = "priv/dc1.pem" + connections.tls.certfile = "priv/cert.pem" connections.tls.keyfile = "priv/dc1.pem" connections.tls.cacertfile = "priv/ca.pem" connections.connections_per_endpoint = 30 From b89bdb9f463be8bd7ae0c7a9b11b8252901b9268 Mon Sep 17 00:00:00 2001 From: Tomasz Wojcikowski Date: Thu, 11 Dec 2025 09:23:24 +0100 Subject: [PATCH 4/4] Update TLS certfile path in config_parser_helper.erl --- test/common/config_parser_helper.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index 1e3fe0b5902..1f26fce3981 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -478,7 +478,7 @@ all_modules() -> connections_per_endpoint => 30, tls => config([modules, mod_global_distrib, connections, tls], #{cacertfile => "priv/ca.pem", - certfile => "priv/dc1.pem", + certfile => "priv/cert.pem", keyfile => "priv/dc1.pem"}) }) }),