Merge pull request #4167 from zmstone/fix-tls-ensure-tls-config-integrity

fix(tls): Ensure tls config integrity

apps/emqx_auth_http/src/emqx_auth_http_app.erl

@@ -74,11 +74,10 @@ translate_env(EnvName) ->
                                                         (_) ->
                                                         true
                                                     end, [{keyfile, KeyFile}, {certfile, CertFile}, {cacertfile, CACertFile}]),
-                            TlsVers = ['tlsv1.2','tlsv1.1',tlsv1],
+                            NTLSOpts = [ {versions, emqx_tls_lib:default_versions()}
-                            NTLSOpts = [{versions, TlsVers},
+                                       , {ciphers, emqx_tls_lib:default_ciphers()}
-                                        {ciphers, lists:foldl(fun(TlsVer, Ciphers) ->
+                                       | TLSOpts
-                                                                    Ciphers ++ ssl:cipher_suites(all, TlsVer)
+                                       ],
-                                                                end, [], TlsVers)} | TLSOpts],
                             [{transport, ssl}, {transport_opts, [Inet | NTLSOpts]}]
                         end,
             PoolOpts = [{host, Host},

apps/emqx_auth_pgsql/etc/emqx_auth_pgsql.conf

@@ -43,7 +43,7 @@ auth.pgsql.ssl = off
 ## You can configure multi-version use "," split,
 ## default value is :tlsv1.2
 ## Example:
-##    tlsv1.1,tlsv1.2,tlsv1.3
+##    tlsv1.2,tlsv1.1
 ##
 #auth.pgsql.ssl.tls_versions = tlsv1.2
 

apps/emqx_bridge_mqtt/README.md

@@ -126,7 +126,7 @@ bridge.mqtt.emqx2.ciphers = ECDHE-ECDSA-AES256-GCM-SHA384,ECDHE-RSA-AES256-GCM-S
 bridge.mqtt.emqx2.keepalive = 60s
 
 ## Supported TLS version
-bridge.mqtt.emqx2.tls_versions = tlsv1.2,tlsv1.1,tlsv1
+bridge.mqtt.emqx2.tls_versions = tlsv1.3,tlsv1.2,tlsv1.1,tlsv1
 
 ## Forwarding topics of the message
 bridge.mqtt.emqx2.forwards = sensor1/#,sensor2/#

apps/emqx_bridge_mqtt/docs/guide.rst

@@ -133,9 +133,6 @@ EMQ X MQTT bridging principle: Create an MQTT client on the EMQ X broker, and co
    ## Key file of Client SSL connection 
    bridge.mqtt.emqx2.keyfile = etc/certs/client-key.pem
 
-   ## SSL encryption
-   bridge.mqtt.emqx2.ciphers = ECDHE-ECDSA-AES256-GCM-SHA384,ECDHE-RSA-AES256-GCM-SHA384
-
    ## TTLS PSK password
    ## Note 'listener.ssl.external.ciphers' and 'listener.ssl.external.psk_ciphers' cannot be configured at the same time
    ##
@@ -146,7 +143,10 @@ EMQ X MQTT bridging principle: Create an MQTT client on the EMQ X broker, and co
    bridge.mqtt.emqx2.keepalive = 60s
 
    ## Supported TLS version
-   bridge.mqtt.emqx2.tls_versions = tlsv1.2,tlsv1.1,tlsv1
+   bridge.mqtt.emqx2.tls_versions = tlsv1.2
+
+   ## SSL encryption
+   bridge.mqtt.emqx2.ciphers = ECDHE-ECDSA-AES256-GCM-SHA384,ECDHE-RSA-AES256-GCM-SHA384
 
    ## Forwarding topics of the message
    bridge.mqtt.emqx2.forwards = sensor1/#,sensor2/#

apps/emqx_bridge_mqtt/etc/emqx_bridge_mqtt.conf

@@ -128,6 +128,7 @@ bridge.mqtt.aws.keepalive = 60s
 
 ## TLS versions used by the bridge.
 ##
+## NOTE: Do not use tlsv1.3 if emqx is running on OTP-22 or earlier
 ## Value: String
 bridge.mqtt.aws.tls_versions = tlsv1.3,tlsv1.2,tlsv1.1,tlsv1
 

apps/emqx_bridge_mqtt/priv/emqx_bridge_mqtt.schema

@@ -90,7 +90,7 @@
 
 {mapping, "bridge.mqtt.$name.tls_versions", "emqx_bridge_mqtt.bridges", [
   {datatype, string},
-  {default, "tlsv1,tlsv1.1,tlsv1.2"}
+  {default, "tlsv1.3,tlsv1.2,tlsv1.1,tlsv1"}
 ]}.
 
 {mapping, "bridge.mqtt.$name.reconnect_interval", "emqx_bridge_mqtt.bridges", [

apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt_actions.erl

@@ -671,12 +671,6 @@ format_data([], Msg) ->
 format_data(Tokens, Msg) ->
     emqx_rule_utils:proc_tmpl(Tokens, Msg).
 
-tls_versions() ->
-    ['tlsv1.2','tlsv1.1', tlsv1].
-
-ciphers(Ciphers) ->
-    string:tokens(str(Ciphers), ", ").
-
 subscriptions(Subscriptions) ->
     scan_binary(<<"[", Subscriptions/binary, "].">>).
 
@@ -749,6 +743,8 @@ options(Options, PoolName) ->
                      Topic ->
                          [{subscriptions, [{Topic, Get(<<"qos">>)}]} | Subscriptions]
                  end,
+                 %% TODO check why only ciphers are configurable but not versions
+                 TlsVersions = emqx_tls_lib:default_versions(),
                  [{address, binary_to_list(Address)},
                   {bridge_mode, GetD(<<"bridge_mode">>, true)},
                   {clean_start, true},
@@ -761,12 +757,13 @@ options(Options, PoolName) ->
                   {proto_ver, mqtt_ver(Get(<<"proto_ver">>))},
                   {retry_interval, cuttlefish_duration:parse(str(GetD(<<"retry_interval">>, "30s")), s)},
                   {ssl, cuttlefish_flag:parse(str(Get(<<"ssl">>)))},
-                  {ssl_opts, [{versions, tls_versions()},
+                  {ssl_opts, [ {keyfile, str(Get(<<"keyfile">>))}
-                              {ciphers, ciphers(Get(<<"ciphers">>))},
+                             , {certfile, str(Get(<<"certfile">>))}
-                              {keyfile, str(Get(<<"keyfile">>))},
+                             , {cacertfile, str(Get(<<"cacertfile">>))}
-                              {certfile, str(Get(<<"certfile">>))},
+                             , {versions, TlsVersions}
-                              {cacertfile, str(Get(<<"cacertfile">>))}
+                             , {ciphers, emqx_tls_lib:integral_ciphers(TlsVersions, Get(<<"ciphers">>))}
-                             ]}] ++ Subscriptions1
+                             ]}
+                 ] ++ Subscriptions1
          end.
 
 

apps/emqx_coap/priv/emqx_coap.schema

@@ -75,10 +75,7 @@ end}.
   Ciphers =
       case cuttlefish:conf_get("coap.dtls.ciphers", Conf, undefined) of
           undefined ->
-              lists:foldl(
+              lists:append([ssl:cipher_suites(all, V, openssl) || V <- ['dtlsv1.2', 'dtlsv1']]);
-                  fun(TlsVer, Ciphers) ->
-                      Ciphers ++ ssl:cipher_suites(all, TlsVer)
-                  end, [], ['dtlsv1', 'dtlsv1.2']);
           C ->
               SplitFun(C)
       end,

apps/emqx_dashboard/etc/emqx_dashboard.conf

@@ -105,7 +105,8 @@ dashboard.listener.http.ipv6_v6only = false
 ## TLS versions only to protect from POODLE attack.
 ##
 ## Value: String, seperated by ','
-## dashboard.listener.https.tls_versions = tlsv1.2,tlsv1.1,tlsv1
+## NOTE: Do not use tlsv1.3 if emqx is running on OTP-22 or earlier
+## dashboard.listener.https.tls_versions = tlsv1.3,tlsv1.2,tlsv1.1,tlsv1
 
 ## See: 'listener.ssl.<name>.ciphers' in emq.conf
 ##

apps/emqx_exproto/test/emqx_exproto_SUITE.erl

@@ -425,8 +425,8 @@ udp_opts() ->
 
 ssl_opts() ->
     Certs = certs("key.pem", "cert.pem", "cacert.pem"),
-    [{versions, ['tlsv1.2','tlsv1.1',tlsv1]},
+    [{versions, emqx_tls_lib:default_versions()},
-     {ciphers, ciphers('tlsv1.2')},
+     {ciphers, emqx_tls_lib:default_ciphers()},
      {verify, verify_peer},
      {fail_if_no_peer_cert, true},
      {secure_renegotiate, false},
@@ -437,9 +437,6 @@ dtls_opts() ->
     Opts = ssl_opts(),
     lists:keyreplace(versions, 1, Opts, {versions, ['dtlsv1.2', 'dtlsv1']}).
 
-ciphers(Version) ->
-    proplists:get_value(ciphers, emqx_ct_helpers:client_ssl(Version)).
-
 %%--------------------------------------------------------------------
 %% Client-Opts
 

apps/emqx_management/etc/emqx_management.conf

@@ -45,7 +45,8 @@ management.listener.http.ipv6_v6only = false
 ## management.listener.https.keyfile = etc/certs/key.pem
 ## management.listener.https.cacertfile = etc/certs/cacert.pem
 ## management.listener.https.verify = verify_peer
-## management.listener.https.tls_versions = tlsv1.2,tlsv1.1,tlsv1
+## NOTE: Do not use tlsv1.3 if emqx is running on OTP-22 or earlier
+## management.listener.https.tls_versions = tlsv1.3,tlsv1.2,tlsv1.1,tlsv1
 ## management.listener.https.ciphers = TLS_AES_256_GCM_SHA384,TLS_AES_128_GCM_SHA256,TLS_CHACHA20_POLY1305_SHA256,TLS_AES_128_CCM_SHA256,TLS_AES_128_CCM_8_SHA256,ECDHE-ECDSA-AES256-GCM-SHA384,ECDHE-RSA-AES256-GCM-SHA384,ECDHE-ECDSA-AES256-SHA384,ECDHE-RSA-AES256-SHA384,ECDHE-ECDSA-DES-CBC3-SHA,ECDH-ECDSA-AES256-GCM-SHA384,ECDH-RSA-AES256-GCM-SHA384,ECDH-ECDSA-AES256-SHA384,ECDH-RSA-AES256-SHA384,DHE-DSS-AES256-GCM-SHA384,DHE-DSS-AES256-SHA256,AES256-GCM-SHA384,AES256-SHA256,ECDHE-ECDSA-AES128-GCM-SHA256,ECDHE-RSA-AES128-GCM-SHA256,ECDHE-ECDSA-AES128-SHA256,ECDHE-RSA-AES128-SHA256,ECDH-ECDSA-AES128-GCM-SHA256,ECDH-RSA-AES128-GCM-SHA256,ECDH-ECDSA-AES128-SHA256,ECDH-RSA-AES128-SHA256,DHE-DSS-AES128-GCM-SHA256,DHE-DSS-AES128-SHA256,AES128-GCM-SHA256,AES128-SHA256,ECDHE-ECDSA-AES256-SHA,ECDHE-RSA-AES256-SHA,DHE-DSS-AES256-SHA,ECDH-ECDSA-AES256-SHA,ECDH-RSA-AES256-SHA,AES256-SHA,ECDHE-ECDSA-AES128-SHA,ECDHE-RSA-AES128-SHA,DHE-DSS-AES128-SHA,ECDH-ECDSA-AES128-SHA,ECDH-RSA-AES128-SHA,AES128-SHA
 ## management.listener.https.fail_if_no_peer_cert = true
 ## management.listener.https.inet6 = false

apps/emqx_stomp/etc/emqx_stomp.conf

@@ -58,7 +58,8 @@ stomp.listener.max_connections = 512
 ## TLS versions only to protect from POODLE attack.
 ##
 ## Value: String, seperated by ','
-## stomp.listener.tls_versions = tlsv1.2,tlsv1.1,tlsv1
+## NOTE: Do not use tlsv1.3 if emqx is running on OTP-22 or earlier
+## stomp.listener.tls_versions = tlsv1.3,tlsv1.2,tlsv1.1,tlsv1
 
 ## SSL Handshake timeout.
 ##

apps/emqx_web_hook/src/emqx_web_hook_actions.erl

@@ -354,12 +354,11 @@ pool_opts(Params = #{<<"url">> := URL}) ->
                                                  (_) ->
                                                   true
                                               end, [{keyfile, KeyFile}, {certfile, CertFile}, {cacertfile, CACertFile}]),
-                       TlsVers = ['tlsv1.2', 'tlsv1.1', tlsv1],
+                       NTLSOpts = [ {verify, VerifyType}
-                       NTLSOpts = [{verify, VerifyType},
+                                  , {versions, emqx_tls_lib:default_versions()}
-                                   {versions, TlsVers},
+                                  , {ciphers, emqx_tls_lib:default_ciphers()}
-                                   {ciphers, lists:foldl(fun(TlsVer, Ciphers) ->
+                                  | TLSOpts
-                                                             Ciphers ++ ssl:cipher_suites(all, TlsVer)
+                                  ],
-                                                         end, [], TlsVers)} | TLSOpts],
                        [{transport, ssl}, {transport_opts, [Inet | NTLSOpts]}]
               end,
     [{host, Host},
@@ -397,4 +396,4 @@ test_http_connect(Conf) ->
         Err:Reason:ST ->
            ?LOG(error, "check http_connectivity failed: ~p, ~0p", [Conf, {Err, Reason, ST}]),
            false
-    end.
+    end.

apps/emqx_web_hook/src/emqx_web_hook_app.erl

@@ -75,12 +75,11 @@ translate_env() ->
                        TLSOpts = lists:filter(fun({_K, V}) ->
                                                 V /= <<>> andalso V /= undefined andalso V /= "" andalso true
                                               end, [{keyfile, KeyFile}, {certfile, CertFile}, {cacertfile, CACertFile}]),
-                       TlsVers = ['tlsv1.2','tlsv1.1',tlsv1],
+                       NTLSOpts = [ {verify, VerifyType}
-                       NTLSOpts = [{verify, VerifyType},
+                                  , {versions, emqx_tls_lib:default_versions()}
-                                   {versions, TlsVers},
+                                  , {ciphers, emqx_tls_lib:default_ciphers()}
-                                   {ciphers, lists:foldl(fun(TlsVer, Ciphers) ->
+                                  | TLSOpts
-                                                               Ciphers ++ ssl:cipher_suites(all, TlsVer)
+                                  ],
-                                                           end, [], TlsVers)} | TLSOpts],
                        [{transport, ssl}, {transport_opts, [Inet | NTLSOpts]}]
                 end,
     PoolOpts = [{host, Host},
@@ -114,4 +113,4 @@ parse_host(Host) ->
                 {ok, _} -> {inet6, Host};
                 {error, _} -> {inet, Host}
             end
-    end.
+    end.

etc/emqx.conf

@@ -1317,7 +1317,8 @@ listener.ssl.external.access.1 = allow all
 ## See: http://erlang.org/doc/man/ssl.html
 ##
 ## Value: String, seperated by ','
-## listener.ssl.external.tls_versions = tlsv1.2,tlsv1.1,tlsv1
+## NOTE: Do not use tlsv1.3 if emqx is running on OTP-22 or earlier
+## listener.ssl.external.tls_versions = tlsv1.3,tlsv1.2,tlsv1.1,tlsv1
 
 ## TLS Handshake timeout.
 ##
@@ -1785,7 +1786,7 @@ listener.wss.external.access.1 = allow all
 ## Supported subprotocols
 ##
 ## Default: mqtt, mqtt-v3, mqtt-v3.1.1, mqtt-v5
-## listener.ws.external.supported_protocols = mqtt, mqtt-v3, mqtt-v3.1.1, mqtt-v5
+## listener.wss.external.supported_protocols = mqtt, mqtt-v3, mqtt-v3.1.1, mqtt-v5
 
 ## Enable the Proxy Protocol V1/2 support.
 ##
@@ -1806,7 +1807,8 @@ listener.wss.external.access.1 = allow all
 ## See: listener.ssl.$name.tls_versions
 ##
 ## Value: String, seperated by ','
-## listener.wss.external.tls_versions = tlsv1.2,tlsv1.1,tlsv1
+## NOTE: Do not use tlsv1.3 if emqx is running on OTP-22 or earlier
+## listener.wss.external.tls_versions = tlsv1.3,tlsv1.2,tlsv1.1,tlsv1
 
 ## Path to the file containing the user's private PEM-encoded key.
 ##

src/emqx_tls_lib.erl

@@ -0,0 +1,98 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2021 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+-module(emqx_tls_lib).
+
+-export([ default_versions/0
+        , integral_versions/1
+        , default_ciphers/0
+        , default_ciphers/1
+        , integral_ciphers/2
+        ]).
+
+-define(IS_STRING_LIST(L), (is_list(L) andalso L =/= [] andalso is_list(hd(L)))).
+
+%% @doc Returns the default supported tls versions.
+-spec default_versions() -> [atom()].
+default_versions() ->
+    OtpRelease = list_to_integer(erlang:system_info(otp_release)),
+    integral_versions(default_versions(OtpRelease)).
+
+%% @doc Validate a given list of desired tls versions.
+%% raise an error exception if non of them are available.
+-spec integral_versions([ssl:tls_version()]) -> [ssl:tls_version()].
+integral_versions(Desired) ->
+    {_, Available} = lists:keyfind(available, 1, ssl:versions()),
+    case lists:filter(fun(V) -> lists:member(V, Available) end, Desired) of
+        [] -> erlang:error(#{ reason => no_available_tls_version
+                            , desired => Desired
+                            , available => Available
+                            });
+        Filtered ->
+            Filtered
+    end.
+
+%% @doc Return a list of default (openssl string format) cipher suites.
+-spec default_ciphers() -> [string()].
+default_ciphers() -> default_ciphers(default_versions()).
+
+%% @doc Return a list of (openssl string format) cipher suites.
+-spec default_ciphers([ssl:tls_version()]) -> [string()].
+default_ciphers(['tlsv1.3']) ->
+    %% When it's only tlsv1.3 wanted, use 'exclusive' here
+    %% because 'all' returns legacy cipher suites too,
+    %% which does not make sense since tlsv1.3 can not use
+    %% legacy cipher suites.
+    ssl:cipher_suites(exclusive, 'tlsv1.3', openssl);
+default_ciphers(Versions) ->
+    %% assert non-empty
+    [_ | _] = dedup(lists:append([ssl:cipher_suites(all, V, openssl) || V <- Versions])).
+
+%% @doc Ensure version & cipher-suites integrity.
+-spec integral_ciphers([ssl:tls_version()], binary() | string() | [string()]) -> [string()].
+integral_ciphers(Versions, Ciphers) when Ciphers =:= [] orelse Ciphers =:= undefined ->
+    %% not configured
+    integral_ciphers(Versions, default_ciphers(Versions));
+integral_ciphers(Versions, Ciphers) when ?IS_STRING_LIST(Ciphers) ->
+    %% ensure tlsv1.3 ciphers if none of them is found in Ciphers
+    dedup(ensure_tls13_cipher(lists:member('tlsv1.3', Versions), Ciphers));
+integral_ciphers(Versions, Ciphers) when is_binary(Ciphers) ->
+    %% parse binary
+    integral_ciphers(Versions, binary_to_list(Ciphers));
+integral_ciphers(Versions, Ciphers) ->
+    %% parse comma separated cipher suite names
+    integral_ciphers(Versions, string:tokens(Ciphers, ", ")).
+
+%% In case tlsv1.3 is present, ensure tlsv1.3 cipher is added if user
+%% did not provide it from config --- which is a common mistake
+ensure_tls13_cipher(true, Ciphers) ->
+    Tls13Ciphers = default_ciphers(['tlsv1.3']),
+    case lists:any(fun(C) -> lists:member(C, Tls13Ciphers) end, Ciphers) of
+        true  -> Ciphers;
+        false -> Tls13Ciphers ++ Ciphers
+    end;
+ensure_tls13_cipher(false, Ciphers) ->
+    Ciphers.
+
+%% tlsv1.3 is available from OTP-22 but we do not want to use until 23.
+default_versions(OtpRelease) when OtpRelease >= 23 ->
+    ['tlsv1.3' | default_versions(22)];
+default_versions(_) ->
+    ['tlsv1.2', 'tlsv1.1', tlsv1].
+
+%% Deduplicate a list without re-ordering the elements.
+dedup([]) -> [];
+dedup([H | T]) -> [H | dedup([I || I <- T, I =/= H])].

test/emqx_tls_lib_tests.erl

@@ -0,0 +1,64 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2021 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+-module(emqx_tls_lib_tests).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% one of the cipher suite from tlsv1.2 and tlsv1.3 each
+-define(TLS_12_CIPHER, "ECDHE-ECDSA-AES256-GCM-SHA384").
+-define(TLS_13_CIPHER, "TLS_AES_256_GCM_SHA384").
+
+ensure_tls13_ciphers_added_test() ->
+    Ciphers = emqx_tls_lib:integral_ciphers(['tlsv1.3'], [?TLS_12_CIPHER]),
+    ?assert(lists:member(?TLS_12_CIPHER, Ciphers)),
+    ?assert(lists:member(?TLS_13_CIPHER, Ciphers)).
+
+legacy_cipher_suites_test() ->
+    Ciphers = emqx_tls_lib:integral_ciphers(['tlsv1.2'], [?TLS_12_CIPHER]),
+    ?assertEqual([?TLS_12_CIPHER], Ciphers).
+
+use_default_ciphers_test() ->
+    Ciphers = emqx_tls_lib:integral_ciphers(['tlsv1.3', 'tlsv1.2'], ""),
+    ?assert(lists:member(?TLS_12_CIPHER, Ciphers)),
+    ?assert(lists:member(?TLS_13_CIPHER, Ciphers)).
+
+ciphers_format_test_() ->
+    String = ?TLS_13_CIPHER ++ "," ++ ?TLS_12_CIPHER,
+    Binary = iolist_to_binary(String),
+    List = [?TLS_13_CIPHER, ?TLS_12_CIPHER],
+    [ {"string", fun() -> test_cipher_format(String) end}
+    , {"binary", fun() -> test_cipher_format(Binary) end}
+    , {"string-list", fun() -> test_cipher_format(List) end}
+    ].
+
+test_cipher_format(Input) ->
+    Ciphers = emqx_tls_lib:integral_ciphers(['tlsv1.3', 'tlsv1.2'], Input),
+    ?assertEqual([?TLS_13_CIPHER, ?TLS_12_CIPHER], Ciphers).
+
+tls_versions_test() ->
+    ?assert(lists:member('tlsv1.3', emqx_tls_lib:default_versions())).
+
+tls_version_unknown_test() ->
+    ?assertError(#{reason := no_available_tls_version},
+                 emqx_tls_lib:integral_versions([])),
+    ?assertError(#{reason := no_available_tls_version},
+                 emqx_tls_lib:integral_versions([foo])).
+
+cipher_suites_no_duplication_test() ->
+    AllCiphers = emqx_tls_lib:default_ciphers(),
+    ?assertEqual(length(AllCiphers), length(lists:usort(AllCiphers))).
+