Merge pull request #4189 from esl/validate-cacertfile

Require 'cacertfile' for just_tls when verify_mode = 'peer'

doc/configuration/listen.md

@@ -193,7 +193,7 @@ By default the TLS library used for C2S connections is `fast_tls`, which uses Op
 Specifies the way client certificate verification works:
 
 * `peer` - makes sure the client certificate is valid and signed by a trusted CA. Requires a valid `cacertfile`.
-* `selfsigned_peer` - makes sure the client certificate is valid, but allows self-signed certificates; supported only by `just_tls`.
+* `selfsigned_peer` - makes sure the client certificate is valid, but allows self-signed certificates; supported only by `just_tls`. Requires a valid `cacertfile`.
 * `none` - client certificate is not checked.
 
 ### `listen.c2s.tls.certfile`

doc/configuration/outgoing-connections.md

@@ -373,8 +373,11 @@ TLS options for a given pool type/tag pair are defined in a subsection starting
 * **Default:** `"peer"`
 * **Example:** `tls.verify_mode = "none"`
 
-The default value, `"peer"`, enforces verification of the server certificate, and requires a valid `cacertfile` to do so.
-You can set it to `"selfsigned_peer"` to accept self-signed certificates or to `"none"` to skip certificate verification altogether.
+Specifies the way server certificate verification works:
+
+* `peer` - makes sure the server certificate is valid and signed by a trusted CA. Requires a valid `cacertfile`.
+* `selfsigned_peer` - makes sure the server certificate is valid, but allows self-signed certificates. Requires a valid `cacertfile`.
+* `none` - server certificate is not checked.
 
 ### `outgoing_pools.*.*.connection.tls.certfile`
 * **Syntax:** string, path in the file system

doc/migrations/6.1.0_6.2.0.md

@@ -5,6 +5,20 @@ Now there is an option to use [CETS](https://github.com/esl/cets/) instead.
 Mnesia is still used by default, so you don't need to change your configuration file.
 If you want to switch to CETS, see [`internal_databases`](../configuration/internal-databases.md).
 
-# Transition to New CLI Commands
+## Validation of TLS options
+
+Erlang/OTP 26 has more strict checking of the TLS options, as described in [release highlights](https://www.erlang.org/blog/otp-26-highlights/#ssl-improved-checking-of-options).
+MongooseIM follows the same rules now, preventing runtime crashes if TLS is misconfigured.
+
+By default `verify_mode` is set to `"peer"` for each `tls` section in the configuration, and this requires `cacertfile` - otherwise the server will refuse to start. This was already documented, but not enforced. The option `"selfsigned_peer"` also requires `cacertfile` now.
+
+This change affects the following configuration sections:
+
+* [Listeners](../configuration/listen.md). Currently it only affects [`http`](../configuration/listen.md#http-based-services-listenhttp) and [`c2s`](../configuration/listen.md#client-to-server-c2s-listenc2s) with [`tls.module`](../configuration/listen.md#listenc2stlsmodule) set to `"just_tls"`, but we recommend fixing it for all listeners already, because in future releases all listeners would have this validation.
+* [Outgoing connections](../configuration/outgoing-connections.md).
+
+For each of the affected sections, if there is any `tls` option present, **make sure** that either `tls.cacertfile` is provided or `tls.verify_mode` is set to `"none"`.
+
+## Transition to New CLI Commands
 
 Legacy CLI commands previously marked as deprecated have now been removed. The users are encouraged to explore the new GraphQL-based CLI. It is recommended to transition to the new CLI commands **prior to the next system upgrade**. The configuration options `general.mongooseimctl_access_commands` and `services.service_admin_extra` related to the legacy CLI were also removed. **You need to remove them** from your configuration file unless you have already done so.

doc/tutorials/client-certificate.md

@@ -64,6 +64,7 @@ In order to tell MongooseIM to accept self-signed certs, the `listen.c2s.tls.ver
 [listen.c2s]
   tls.verify_mode = "selfsigned_peer"
   tls.disconnect_on_failure = false
+  tls.cacertfile = "ca.pem"
 ```
 
 where the `tls.disconnect_on_failure` is a boolean with the following meaning only for `just_tls`:
@@ -81,6 +82,7 @@ In order to accept self-signed certs for WS or BOSH connections, the `tls` optio
 ```toml
 [listen.http]
   tls.verify_mode = "selfsigned_peer"
+  tls.cacertfile = "ca.pem"
 ```
 
 ### Examples

src/config/mongoose_config_spec.erl

@@ -14,6 +14,8 @@
          process_general/1,
          process_listener/2,
          process_c2s_tls/1,
+         process_c2s_just_tls/1,
+         process_just_tls/1,
          process_fast_tls/1,
          process_sasl_external/1,
          process_sasl_mechanism/1,
@@ -263,10 +265,8 @@ listener_common() ->
             }.
 
 listener_extra(http) ->
-    %% options listed here are passed to ranch_ssl (with verify_mode translated to verify_fun)
-    TLSKeys = [verify_mode, certfile, cacertfile, ciphers, keyfile, password, versions, dhfile],
-    TLSSection = mongoose_config_utils:section_with_keys(TLSKeys, tls([server], [just_tls])),
-    #section{items = #{<<"tls">> => TLSSection,
+    %% tls options passed to ranch_ssl (with verify_mode translated to verify_fun)
+    #section{items = #{<<"tls">> => tls([server], [just_tls]),
                        <<"transport">> => http_transport(),
                        <<"protocol">> => http_protocol(),
                        <<"handlers">> => mongoose_http_handler:config_spec()}};
@@ -307,7 +307,7 @@ xmpp_listener_extra(c2s) ->
                            #list{items = #option{type = atom,
                                                  validate = {module, ejabberd_auth}},
                                  validate = unique},
-                       <<"tls">> => c2s_tls()},
+                       <<"tls">> => tls([server, c2s], [fast_tls, just_tls])},
              defaults = #{<<"access">> => all,
                           <<"shaper">> => none,
                           <<"max_connections">> => infinity,
@@ -319,8 +319,7 @@ xmpp_listener_extra(s2s) ->
     TLSSection = tls([server], [fast_tls]),
     #section{items = #{<<"shaper">> => #option{type = atom,
                                                validate = non_empty},
-                       <<"tls">> => TLSSection#section{include = always,
-                                                       process = fun ?MODULE:process_fast_tls/1}},
+                       <<"tls">> => TLSSection#section{include = always}},
              defaults = #{<<"shaper">> => none}
             };
 xmpp_listener_extra(service) ->
@@ -345,20 +344,6 @@ xmpp_listener_extra(service) ->
                           <<"conflict_behaviour">> => disconnect}
             }.
 
-%% path: listen.c2s[].tls
-c2s_tls() ->
-    mongoose_config_utils:merge_sections(tls([server], [fast_tls, just_tls]), c2s_tls_extra()).
-
-c2s_tls_extra() ->
-    #section{items = #{<<"module">> => #option{type = atom,
-                                               validate = {enum, [fast_tls, just_tls]}},
-                       <<"mode">> => #option{type = atom,
-                                             validate = {enum, [tls, starttls, starttls_required]}}
-                      },
-             defaults = #{<<"module">> => fast_tls,
-                          <<"mode">> => starttls},
-             process = fun ?MODULE:process_c2s_tls/1}.
-
 %% path: listen.http[].transport
 http_transport() ->
     #section{
@@ -674,27 +659,40 @@ tls(common, common) ->
              defaults = #{<<"verify_mode">> => peer}};
 tls(common, fast_tls) ->
     #section{items = #{<<"protocol_options">> => #list{items = #option{type = string,
-                                                                       validate = non_empty}}}};
+                                                                       validate = non_empty}}},
+             process = fun ?MODULE:process_fast_tls/1};
 tls(common, just_tls) ->
     #section{items = #{<<"keyfile">> => #option{type = string,
                                                 validate = filename},
                        <<"password">> => #option{type = string},
-                       <<"versions">> => #list{items = #option{type = atom}}}};
+                       <<"versions">> => #list{items = #option{type = atom}}},
+             process = fun ?MODULE:process_just_tls/1};
 tls(server, common) ->
     #section{items = #{<<"dhfile">> => #option{type = string,
                                                validate = filename}}};
-tls(server, fast_tls) ->
+tls(server, _) ->
     #section{};
-tls(server, just_tls) ->
-    #section{items = #{<<"disconnect_on_failure">> => #option{type = boolean},
-                       <<"crl_files">> => #list{items = #option{type = string,
-                                                                validate = filename}}}};
 tls(client, common) ->
     #section{};
 tls(client, fast_tls) ->
     #section{};
 tls(client, just_tls) ->
-    #section{items = #{<<"server_name_indication">> => server_name_indication()}}.
+    #section{items = #{<<"server_name_indication">> => server_name_indication()}};
+tls(c2s, common) ->
+    #section{items = #{<<"module">> => #option{type = atom,
+                                               validate = {enum, [fast_tls, just_tls]}},
+                       <<"mode">> => #option{type = atom,
+                                             validate = {enum, [tls, starttls, starttls_required]}}},
+             defaults = #{<<"module">> => fast_tls,
+                          <<"mode">> => starttls},
+             process = fun ?MODULE:process_c2s_tls/1};
+tls(c2s, just_tls) ->
+    #section{items = #{<<"disconnect_on_failure">> => #option{type = boolean},
+                       <<"crl_files">> => #list{items = #option{type = string,
+                                                                validate = filename}}},
+             process = fun ?MODULE:process_c2s_just_tls/1};
+tls(c2s, fast_tls) ->
+    #section{}.
 
 server_name_indication() ->
     #section{items = #{<<"enabled">> => #option{type = boolean},
@@ -995,40 +993,48 @@ get_all_hosts_and_host_types(General) ->
                           []
                   end, General).
 
+%% User chooses just_tls or fast_tls, and this choice limits the allowed keys
 process_c2s_tls(M = #{module := Module}) ->
-    check_tls_keys(M, [module, mode]),
-    process_tls(Module, M).
-
-process_fast_tls(M) ->
-    process_tls(fast_tls, M).
-
-process_tls(Module, M) ->
-    check_tls_verify_mode(Module, M),
-    maps:merge(tls_defaults(Module), M).
-
-%% The user chooses just_tls or fast_tls, and this choice limits the allowed keys
-check_tls_keys(M = #{module := Module}, ExtraKeys) ->
-    AllowedItems = (tls([server], [Module]))#section.items,
-    AllowedKeys = [binary_to_atom(Key) || Key <- maps:keys(AllowedItems)] ++ ExtraKeys,
+    AllowedItems = (tls([server, c2s], [Module]))#section.items,
+    AllowedKeys = [binary_to_atom(Key) || Key <- maps:keys(AllowedItems)] ++ [module, mode],
     case maps:keys(M) -- AllowedKeys of
-        [] -> ok;
+        [] -> M;
         UnexpectedKeys -> error(#{what => unexpected_tls_options,
                                   tls_module => Module,
                                   unexpected_keys => UnexpectedKeys})
     end.
 
-tls_defaults(just_tls) ->
-    #{crl_files => [],
-      disconnect_on_failure => true};
-tls_defaults(fast_tls) ->
-    #{ciphers => mongoose_tls:default_ciphers(),
-      protocol_options => ["no_sslv2", "no_sslv3", "no_tlsv1", "no_tlsv1_1"]}.
+process_c2s_just_tls(#{module := just_tls} = M) ->
+    maps:merge(just_tls_c2s_defaults(), M);
+process_c2s_just_tls(M) ->
+    M.
 
-check_tls_verify_mode(fast_tls, #{verify_mode := selfsigned_peer}) ->
+just_tls_c2s_defaults() ->
+    #{crl_files => [],
+      disconnect_on_failure => true}.
+
+process_just_tls(M = #{module := fast_tls}) ->
+    M;
+process_just_tls(M = #{cacertfile := _}) ->
+    M;
+process_just_tls(M = #{verify_mode := none}) ->
+    M;
+process_just_tls(_) ->
+    error(#{what => missing_cacertfile,
+            text => <<"You need to provide CA certificate (cacertfile) "
+                      "or disable peer verification (verify_mode)">>}).
+
+process_fast_tls(M = #{module := just_tls}) ->
+    M;
+process_fast_tls(#{verify_mode := selfsigned_peer}) ->
     error(#{what => invalid_tls_verify_mode,
             text => <<"fast_tls does not support self-signed certificate verification">>});
-check_tls_verify_mode(_Module, #{}) ->
-    ok.
+process_fast_tls(M) ->
+    maps:merge(fast_tls_defaults(), M).
+
+fast_tls_defaults() ->
+    #{ciphers => mongoose_tls:default_ciphers(),
+      protocol_options => ["no_sslv2", "no_sslv3", "no_tlsv1", "no_tlsv1_1"]}.
 
 process_listener([item, Type | _], Opts) ->
     mongoose_listener_config:ensure_ip_options(Opts#{module => listener_module(Type),

src/config/mongoose_config_utils.erl

@@ -2,7 +2,7 @@
 %% This stuff can be pure, but most likely not.
 %% It's for generic functions.
 -module(mongoose_config_utils).
--export([exit_or_halt/1, section_to_defaults/1, merge_sections/2, section_with_keys/2]).
+-export([exit_or_halt/1, section_to_defaults/1, merge_sections/2]).
 
 -ignore_xref([section_to_defaults/1]).
 
@@ -40,10 +40,3 @@ merge_process_functions(Process1, Process2) ->
             V1 = mongoose_config_parser_toml:process(Path, V, Process1),
             mongoose_config_parser_toml:process(Path, V1, Process2)
     end.
-
-section_with_keys(Keys, Section) ->
-    BinKeys = [atom_to_binary(Key) || Key <- Keys],
-    #section{items = Items, defaults = Defaults, required = Required} = Section,
-    Section#section{items = maps:with(BinKeys, Items),
-                    defaults = maps:with(BinKeys, Defaults),
-                    required = Required -- (Required -- BinKeys)}.

src/global_distrib/mod_global_distrib.erl

@@ -128,8 +128,7 @@ endpoint_spec() ->
     }.
 
 tls_spec() ->
-    TLSSection = mongoose_config_spec:tls([client, server], [fast_tls]),
-    TLSSection#section{process = fun mongoose_config_spec:process_fast_tls/1}.
+    mongoose_config_spec:tls([client, server], [fast_tls]).
 
 redis_spec() ->
     #section{

test/common/config_parser_helper.erl

@@ -180,7 +180,8 @@ options("mongooseim-pgsql") ->
                               username => <<"ala">>, password => <<"makotaipsa">>})
                     ],
                 transport => #{num_acceptors => 10, max_connections => 1024},
-                tls => #{certfile => "priv/cert.pem", keyfile => "priv/dc1.pem", password => ""}
+                tls => #{certfile => "priv/cert.pem", keyfile => "priv/dc1.pem", password => "",
+                         verify_mode => none}
                }),
        config([listen, http],
               #{ip_address => "127.0.0.1",
@@ -198,7 +199,8 @@ options("mongooseim-pgsql") ->
                             #{host => '_', path => "/api"})],
                 protocol => #{compress => true},
                 transport => #{num_acceptors => 10, max_connections => 1024},
-                tls => #{certfile => "priv/cert.pem", keyfile => "priv/dc1.pem", password => ""}
+                tls => #{certfile => "priv/cert.pem", keyfile => "priv/dc1.pem", password => "",
+                         verify_mode => none}
                }),
        config([listen, s2s],
               #{port => 5269,