Explorar o código

Merge pull request #10983 from id/0608-handle-incompatible-tls-options

fix(tls): issue when ssl listner is configured to use tls v1.3 only
Ivan Dyachkov %!s(int64=2) %!d(string=hai) anos
pai
achega
3a8332811a

+ 47 - 18
apps/emqx/src/emqx_tls_lib.erl

@@ -485,7 +485,8 @@ to_server_opts(Type, Opts) ->
             cacertfile => Path(cacertfile),
             cacertfile => Path(cacertfile),
             ciphers => Ciphers,
             ciphers => Ciphers,
             versions => Versions
             versions => Versions
-        })
+        }),
+        Versions
     ).
     ).
 
 
 %% @doc Convert hocon-checked tls client options (map()) to
 %% @doc Convert hocon-checked tls client options (map()) to
@@ -510,19 +511,22 @@ to_client_opts(Type, Opts) ->
             SNI = ensure_sni(Get(server_name_indication)),
             SNI = ensure_sni(Get(server_name_indication)),
             Versions = integral_versions(Type, Get(versions)),
             Versions = integral_versions(Type, Get(versions)),
             Ciphers = integral_ciphers(Versions, Get(ciphers)),
             Ciphers = integral_ciphers(Versions, Get(ciphers)),
-            filter([
-                {keyfile, KeyFile},
-                {certfile, CertFile},
-                {cacertfile, CAFile},
-                {verify, Verify},
-                {server_name_indication, SNI},
-                {versions, Versions},
-                {ciphers, Ciphers},
-                {reuse_sessions, Get(reuse_sessions)},
-                {depth, Get(depth)},
-                {password, ensure_str(Get(password))},
-                {secure_renegotiate, Get(secure_renegotiate)}
-            ]);
+            filter(
+                [
+                    {keyfile, KeyFile},
+                    {certfile, CertFile},
+                    {cacertfile, CAFile},
+                    {verify, Verify},
+                    {server_name_indication, SNI},
+                    {versions, Versions},
+                    {ciphers, Ciphers},
+                    {reuse_sessions, Get(reuse_sessions)},
+                    {depth, Get(depth)},
+                    {password, ensure_str(Get(password))},
+                    {secure_renegotiate, Get(secure_renegotiate)}
+                ],
+                Versions
+            );
         false ->
         false ->
             []
             []
     end.
     end.
@@ -552,10 +556,35 @@ resolve_cert_path_for_read_strict(Path) ->
 resolve_cert_path_for_read(Path) ->
 resolve_cert_path_for_read(Path) ->
     emqx_schema:naive_env_interpolation(Path).
     emqx_schema:naive_env_interpolation(Path).
 
 
-filter([]) -> [];
-filter([{_, undefined} | T]) -> filter(T);
-filter([{_, ""} | T]) -> filter(T);
-filter([H | T]) -> [H | filter(T)].
+filter([], _) ->
+    [];
+filter([{_, undefined} | T], Versions) ->
+    filter(T, Versions);
+filter([{_, ""} | T], Versions) ->
+    filter(T, Versions);
+filter([{K, V} | T], Versions) ->
+    case tls_option_compatible_versions(K) of
+        all ->
+            [{K, V} | filter(T, Versions)];
+        CompatibleVersions ->
+            case CompatibleVersions -- (CompatibleVersions -- Versions) of
+                [] ->
+                    filter(T, Versions);
+                _ ->
+                    [{K, V} | filter(T, Versions)]
+            end
+    end.
+
+tls_option_compatible_versions(reuse_sessions) ->
+    [dtlsv1, 'dtlsv1.2', 'tlsv1', 'tlsv1.1', 'tlsv1.2'];
+tls_option_compatible_versions(secure_renegotiate) ->
+    [dtlsv1, 'dtlsv1.2', 'tlsv1', 'tlsv1.1', 'tlsv1.2'];
+tls_option_compatible_versions(user_lookup_fun) ->
+    [dtlsv1, 'dtlsv1.2', 'tlsv1', 'tlsv1.1', 'tlsv1.2'];
+tls_option_compatible_versions(client_renegotiation) ->
+    [dtlsv1, 'dtlsv1.2', 'tlsv1', 'tlsv1.1', 'tlsv1.2'];
+tls_option_compatible_versions(_) ->
+    all.
 
 
 -spec fuzzy_map_get(atom() | binary(), map(), any()) -> any().
 -spec fuzzy_map_get(atom() | binary(), map(), any()) -> any().
 fuzzy_map_get(Key, Options, Default) ->
 fuzzy_map_get(Key, Options, Default) ->

+ 65 - 0
apps/emqx/test/emqx_tls_lib_tests.erl

@@ -224,6 +224,71 @@ ssl_file_deterministic_names_test() ->
     ),
     ),
     _ = file:del_dir_r(filename:join(["/tmp", ?FUNCTION_NAME])).
     _ = file:del_dir_r(filename:join(["/tmp", ?FUNCTION_NAME])).
 
 
+to_client_opts_test() ->
+    VersionsAll = [tlsv1, 'tlsv1.1', 'tlsv1.2', 'tlsv1.3'],
+    Versions13Only = ['tlsv1.3'],
+    Options = #{
+        enable => true,
+        verify => "Verify",
+        server_name_indication => "SNI",
+        ciphers => "Ciphers",
+        depth => "depth",
+        password => "password",
+        versions => VersionsAll,
+        secure_renegotiate => "secure_renegotiate",
+        reuse_sessions => "reuse_sessions"
+    },
+    Expected1 = lists:usort(maps:keys(Options) -- [enable]),
+    ?assertEqual(
+        Expected1, lists:usort(proplists:get_keys(emqx_tls_lib:to_client_opts(tls, Options)))
+    ),
+    Expected2 =
+        lists:usort(
+            maps:keys(Options) --
+                [enable, reuse_sessions, secure_renegotiate]
+        ),
+    ?assertEqual(
+        Expected2,
+        lists:usort(
+            proplists:get_keys(
+                emqx_tls_lib:to_client_opts(tls, Options#{versions := Versions13Only})
+            )
+        )
+    ),
+    Expected3 = lists:usort(maps:keys(Options) -- [enable, depth, password]),
+    ?assertEqual(
+        Expected3,
+        lists:usort(
+            proplists:get_keys(
+                emqx_tls_lib:to_client_opts(tls, Options#{depth := undefined, password := ""})
+            )
+        )
+    ).
+
+to_server_opts_test() ->
+    VersionsAll = [tlsv1, 'tlsv1.1', 'tlsv1.2', 'tlsv1.3'],
+    Versions13Only = ['tlsv1.3'],
+    Options = #{
+        verify => "Verify",
+        ciphers => "Ciphers",
+        versions => VersionsAll,
+        user_lookup_fun => "funfunfun",
+        client_renegotiation => "client_renegotiation"
+    },
+    Expected1 = lists:usort(maps:keys(Options)),
+    ?assertEqual(
+        Expected1, lists:usort(proplists:get_keys(emqx_tls_lib:to_server_opts(tls, Options)))
+    ),
+    Expected2 = lists:usort(maps:keys(Options) -- [user_lookup_fun, client_renegotiation]),
+    ?assertEqual(
+        Expected2,
+        lists:usort(
+            proplists:get_keys(
+                emqx_tls_lib:to_server_opts(tls, Options#{versions := Versions13Only})
+            )
+        )
+    ).
+
 bin(X) -> iolist_to_binary(X).
 bin(X) -> iolist_to_binary(X).
 
 
 test_key() ->
 test_key() ->

+ 3 - 0
changes/ce/fix-10983.en.md

@@ -0,0 +1,3 @@
+Fix issue when mqtt clients could not connect over TLS if the listener was configured to use TLS v1.3 only.
+
+The problem was that TLS connection was trying to use options incompatible with TLS v1.3.

+ 8 - 4
rel/i18n/emqx_schema.hocon

@@ -92,7 +92,8 @@ mqtt_max_topic_alias.label:
 """Max Topic Alias"""
 """Max Topic Alias"""
 
 
 common_ssl_opts_schema_user_lookup_fun.desc:
 common_ssl_opts_schema_user_lookup_fun.desc:
-"""EMQX-internal callback that is used to lookup pre-shared key (PSK) identity."""
+"""EMQX-internal callback that is used to lookup pre-shared key (PSK) identity.</br>
+Has no effect when TLS version is configured (or negotiated) to 1.3"""
 
 
 common_ssl_opts_schema_user_lookup_fun.label:
 common_ssl_opts_schema_user_lookup_fun.label:
 """SSL PSK user lookup fun"""
 """SSL PSK user lookup fun"""
@@ -1240,7 +1241,8 @@ The SSL application already takes measures to counter-act such attempts,
 but client-initiated renegotiation can be strictly disabled by setting this option to false.
 but client-initiated renegotiation can be strictly disabled by setting this option to false.
 The default value is true. Note that disabling renegotiation can result in
 The default value is true. Note that disabling renegotiation can result in
 long-lived connections becoming unusable due to limits on
 long-lived connections becoming unusable due to limits on
-the number of messages the underlying cipher suite can encipher."""
+the number of messages the underlying cipher suite can encipher.</br>
+Has no effect when TLS version is configured (or negotiated) to 1.3"""
 
 
 server_ssl_opts_schema_client_renegotiation.label:
 server_ssl_opts_schema_client_renegotiation.label:
 """SSL client renegotiation"""
 """SSL client renegotiation"""
@@ -1326,7 +1328,8 @@ common_ssl_opts_schema_secure_renegotiate.desc:
 """SSL parameter renegotiation is a feature that allows a client and a server
 """SSL parameter renegotiation is a feature that allows a client and a server
 to renegotiate the parameters of the SSL connection on the fly.
 to renegotiate the parameters of the SSL connection on the fly.
 RFC 5746 defines a more secure way of doing this. By enabling secure renegotiation,
 RFC 5746 defines a more secure way of doing this. By enabling secure renegotiation,
-you drop support for the insecure renegotiation, prone to MitM attacks."""
+you drop support for the insecure renegotiation, prone to MitM attacks.</br>
+Has no effect when TLS version is configured (or negotiated) to 1.3"""
 
 
 common_ssl_opts_schema_secure_renegotiate.label:
 common_ssl_opts_schema_secure_renegotiate.label:
 """SSL renegotiate"""
 """SSL renegotiate"""
@@ -1361,7 +1364,8 @@ mqtt_max_packet_size.label:
 """Max Packet Size"""
 """Max Packet Size"""
 
 
 common_ssl_opts_schema_reuse_sessions.desc:
 common_ssl_opts_schema_reuse_sessions.desc:
-"""Enable TLS session reuse."""
+"""Enable TLS session reuse.</br>
+Has no effect when TLS version is configured (or negotiated) to 1.3"""
 
 
 common_ssl_opts_schema_reuse_sessions.label:
 common_ssl_opts_schema_reuse_sessions.label:
 """TLS session reuse"""
 """TLS session reuse"""