Merge pull request #4595 from esl/tomasz/mim-2512-tls-cert-validation

Add TLS keyfile validation

Author: chrzaszcz

src/config/mongoose_config_spec.erl

@@ -655,7 +655,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 +1097,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.

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/cert.pem",
+                                                    keyfile => "priv/dc1.pem"})
                                    })
                       }),
       mod_pubsub =>

test/config_parser_SUITE.erl

@@ -1173,20 +1173,25 @@ 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">>})),
     ?cfg(P ++ [password], "secret", T(M#{<<"password">> => <<"secret">>})),
     ?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,20 +1452,23 @@ 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">>})),
     ?cfgh(P ++ [password], "secret", T(#{<<"password">> => <<"secret">>})),
     ?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
 

test/config_parser_SUITE_data/modules.toml

@@ -120,7 +120,8 @@
   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
   cache.domain_lifetime_seconds = 60

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