Skip to content

Commit

Permalink
Merge pull request #10983 from id/0608-handle-incompatible-tls-options
Browse files Browse the repository at this point in the history
fix(tls): issue when ssl listner is configured to use tls v1.3 only
  • Loading branch information
id committed Jun 8, 2023
2 parents b4d081f + 0434e6c commit 3a83328
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 22 deletions.
65 changes: 47 additions & 18 deletions apps/emqx/src/emqx_tls_lib.erl
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,8 @@ to_server_opts(Type, Opts) ->
cacertfile => Path(cacertfile),
ciphers => Ciphers,
versions => Versions
})
}),
Versions
).

%% @doc Convert hocon-checked tls client options (map()) to
Expand All @@ -510,19 +511,22 @@ to_client_opts(Type, Opts) ->
SNI = ensure_sni(Get(server_name_indication)),
Versions = integral_versions(Type, Get(versions)),
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 ->
[]
end.
Expand Down Expand Up @@ -552,10 +556,35 @@ resolve_cert_path_for_read_strict(Path) ->
resolve_cert_path_for_read(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().
fuzzy_map_get(Key, Options, Default) ->
Expand Down
65 changes: 65 additions & 0 deletions apps/emqx/test/emqx_tls_lib_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,71 @@ ssl_file_deterministic_names_test() ->
),
_ = 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).

test_key() ->
Expand Down
3 changes: 3 additions & 0 deletions changes/ce/fix-10983.en.md
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 8 additions & 4 deletions rel/i18n/emqx_schema.hocon
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ mqtt_max_topic_alias.label:
"""Max Topic Alias"""

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:
"""SSL PSK user lookup fun"""
Expand Down Expand Up @@ -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.
The default value is true. Note that disabling renegotiation can result in
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:
"""SSL client renegotiation"""
Expand Down Expand Up @@ -1326,7 +1328,8 @@ common_ssl_opts_schema_secure_renegotiate.desc:
"""SSL parameter renegotiation is a feature that allows a client and a server
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,
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:
"""SSL renegotiate"""
Expand Down Expand Up @@ -1361,7 +1364,8 @@ mqtt_max_packet_size.label:
"""Max Packet Size"""

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:
"""TLS session reuse"""
Expand Down

0 comments on commit 3a83328

Please sign in to comment.