From f0eed56f7346a1146c4acb12bf28ef392a383f33 Mon Sep 17 00:00:00 2001
From: Ingela Anderton Andin <ingela@erlang.org>
Date: Wed, 10 Jul 2019 11:35:27 +0200
Subject: ssl: Correct RSP/PSK and ALPN handling
Extention handling need some fixes to work
correctly for ALPN and SSL-3.0 only client/servers
do not support extensions
---
lib/ssl/src/dtls_handshake.erl | 3 ++-
lib/ssl/src/ssl_cipher.erl | 30 ++++++++++++++----------
lib/ssl/src/ssl_handshake.erl | 30 ++++++++++++++----------
lib/ssl/src/tls_handshake.erl | 4 ++--
lib/ssl/test/property_test/ssl_eqc_handshake.erl | 2 +-
lib/ssl/test/ssl_handshake_SUITE.erl | 4 ++--
lib/ssl/test/ssl_test_lib.erl | 10 +++-----
7 files changed, 46 insertions(+), 37 deletions(-)
(limited to 'lib')
diff --git a/lib/ssl/src/dtls_handshake.erl b/lib/ssl/src/dtls_handshake.erl
index d8c0e30973..4a381745d4 100644
--- a/lib/ssl/src/dtls_handshake.erl
+++ b/lib/ssl/src/dtls_handshake.erl
@@ -255,7 +255,8 @@ enc_handshake(#client_hello{client_version = {Major, Minor},
CmLength = byte_size(BinCompMethods),
BinCipherSuites = list_to_binary(CipherSuites),
CsLength = byte_size(BinCipherSuites),
- ExtensionsBin = ssl_handshake:encode_hello_extensions(HelloExtensions),
+ ExtensionsBin = ssl_handshake:encode_hello_extensions(HelloExtensions,
+ dtls_v1:corresponding_tls_version({Major, Minor})),
{?CLIENT_HELLO, <<?BYTE(Major), ?BYTE(Minor), Random:32/binary,
?BYTE(SIDLength), SessionID/binary,
diff --git a/lib/ssl/src/ssl_cipher.erl b/lib/ssl/src/ssl_cipher.erl
index f4a91cac52..c16e2331ff 100644
--- a/lib/ssl/src/ssl_cipher.erl
+++ b/lib/ssl/src/ssl_cipher.erl
@@ -38,7 +38,7 @@
cipher_init/3, nonce_seed/2, decipher/6, cipher/5, aead_encrypt/6, aead_decrypt/6,
suites/1, all_suites/1, crypto_support_filters/0,
chacha_suites/1, anonymous_suites/1, psk_suites/1, psk_suites_anon/1,
- srp_suites/0, srp_suites_anon/0,
+ srp_suites/1, srp_suites_anon/1,
rc4_suites/1, des_suites/1, rsa_suites/1,
filter/3, filter_suites/1, filter_suites/2,
hash_algorithm/1, sign_algorithm/1, is_acceptable_hash/2, is_fallback/1,
@@ -284,7 +284,7 @@ all_suites({3, _} = Version) ->
suites(Version)
++ chacha_suites(Version)
++ psk_suites(Version)
- ++ srp_suites()
+ ++ srp_suites(Version)
++ rc4_suites(Version)
++ des_suites(Version)
++ rsa_suites(Version);
@@ -313,8 +313,8 @@ chacha_suites(_) ->
%% Description: Returns a list of the anonymous cipher suites, only supported
%% if explicitly set by user. Intended only for testing.
%%--------------------------------------------------------------------
-anonymous_suites({3, N}) ->
- srp_suites_anon() ++ anonymous_suites(N);
+anonymous_suites({3, N} = Version) ->
+ srp_suites_anon(Version) ++ anonymous_suites(N);
anonymous_suites({254, _} = Version) ->
dtls_v1:anonymous_suites(Version);
anonymous_suites(4) ->
@@ -375,7 +375,7 @@ psk_suites(_) ->
%%--------------------------------------------------------------------
psk_suites_anon({3, N}) ->
psk_suites_anon(N);
-psk_suites_anon(3) ->
+psk_suites_anon(3 = N) ->
[
?TLS_DHE_PSK_WITH_AES_256_GCM_SHA384,
?TLS_PSK_WITH_AES_256_GCM_SHA384,
@@ -401,8 +401,8 @@ psk_suites_anon(3) ->
?TLS_PSK_WITH_AES_128_CCM,
?TLS_PSK_WITH_AES_128_CCM_8,
?TLS_ECDHE_PSK_WITH_RC4_128_SHA
- ] ++ psk_suites_anon(0);
-psk_suites_anon(_) ->
+ ] ++ psk_suites_anon(N-1);
+psk_suites_anon(N) when N > 0 ->
[?TLS_DHE_PSK_WITH_AES_256_CBC_SHA,
?TLS_PSK_WITH_AES_256_CBC_SHA,
?TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA,
@@ -413,14 +413,18 @@ psk_suites_anon(_) ->
?TLS_PSK_WITH_3DES_EDE_CBC_SHA,
?TLS_ECDHE_PSK_WITH_RC4_128_SHA,
?TLS_DHE_PSK_WITH_RC4_128_SHA,
- ?TLS_PSK_WITH_RC4_128_SHA].
+ ?TLS_PSK_WITH_RC4_128_SHA];
+psk_suites_anon(0) ->
+ [].
%%--------------------------------------------------------------------
--spec srp_suites() -> [ssl_cipher_format:cipher_suite()].
+-spec srp_suites(tls_record:tls_version()) -> [ssl_cipher_format:cipher_suite()].
%%
%% Description: Returns a list of the SRP cipher suites, only supported
%% if explicitly set by user.
%%--------------------------------------------------------------------
-srp_suites() ->
+srp_suites({3,0}) ->
+ [];
+srp_suites(_) ->
[?TLS_SRP_SHA_RSA_WITH_3DES_EDE_CBC_SHA,
?TLS_SRP_SHA_DSS_WITH_3DES_EDE_CBC_SHA,
?TLS_SRP_SHA_RSA_WITH_AES_128_CBC_SHA,
@@ -429,12 +433,14 @@ srp_suites() ->
?TLS_SRP_SHA_DSS_WITH_AES_256_CBC_SHA].
%%--------------------------------------------------------------------
--spec srp_suites_anon() -> [ssl_cipher_format:cipher_suite()].
+-spec srp_suites_anon(tls_record:tls_version()) -> [ssl_cipher_format:cipher_suite()].
%%
%% Description: Returns a list of the SRP anonymous cipher suites, only supported
%% if explicitly set by user.
%%--------------------------------------------------------------------
-srp_suites_anon() ->
+srp_suites_anon({3,0}) ->
+ [];
+srp_suites_anon(_) ->
[?TLS_SRP_SHA_WITH_3DES_EDE_CBC_SHA,
?TLS_SRP_SHA_WITH_AES_128_CBC_SHA,
?TLS_SRP_SHA_WITH_AES_256_CBC_SHA].
diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index f2fd049ee3..488e4bb72a 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -58,7 +58,7 @@
]).
%% Encode
--export([encode_handshake/2, encode_hello_extensions/1, encode_extensions/1, encode_extensions/2,
+-export([encode_handshake/2, encode_hello_extensions/2, encode_extensions/1, encode_extensions/2,
encode_client_protocol_negotiation/2, encode_protocols_advertised_on_server/1]).
%% Decode
-export([decode_handshake/3, decode_vector/1, decode_hello_extensions/4, decode_extensions/3,
@@ -534,14 +534,14 @@ encode_handshake(#next_protocol{selected_protocol = SelectedProtocol}, _Version)
PaddingLength = 32 - ((byte_size(SelectedProtocol) + 2) rem 32),
{?NEXT_PROTOCOL, <<?BYTE((byte_size(SelectedProtocol))), SelectedProtocol/binary,
?BYTE(PaddingLength), 0:(PaddingLength * 8)>>};
-encode_handshake(#server_hello{server_version = {Major, Minor},
+encode_handshake(#server_hello{server_version = {Major, Minor} = Version,
random = Random,
session_id = Session_ID,
cipher_suite = CipherSuite,
compression_method = Comp_method,
extensions = Extensions}, _Version) ->
SID_length = byte_size(Session_ID),
- ExtensionsBin = encode_hello_extensions(Extensions),
+ ExtensionsBin = encode_hello_extensions(Extensions, Version),
{?SERVER_HELLO, <<?BYTE(Major), ?BYTE(Minor), Random:32/binary,
?BYTE(SID_length), Session_ID/binary,
CipherSuite/binary, ?BYTE(Comp_method), ExtensionsBin/binary>>};
@@ -589,7 +589,9 @@ encode_handshake(#certificate_verify{signature = BinSig, hashsign_algorithm = Ha
encode_handshake(#finished{verify_data = VerifyData}, _Version) ->
{?FINISHED, VerifyData}.
-encode_hello_extensions(Extensions) ->
+encode_hello_extensions(_, {3, 0}) ->
+ <<>>;
+encode_hello_extensions(Extensions, _) ->
encode_extensions(hello_extensions_list(Extensions), <<>>).
encode_extensions(Exts) ->
@@ -1256,6 +1258,8 @@ handle_server_hello_extensions(RecordCB, Random, CipherSuite, Compression,
%% We also ignore the ALPN extension during renegotiation (see encode_alpn/2).
[Protocol] when not Renegotiation ->
{ConnectionStates, alpn, Protocol};
+ [_] when Renegotiation ->
+ {ConnectionStates, alpn, undefined};
undefined ->
NextProtocolNegotiation = maps:get(next_protocol_negotiation, Exts, undefined),
Protocol = handle_next_protocol(NextProtocolNegotiation, NextProtoSelector, Renegotiation),
@@ -2666,7 +2670,7 @@ filter_unavailable_ecc_suites(_, Suites) ->
handle_renegotiation_extension(Role, RecordCB, Version, Info, Random, NegotiatedCipherSuite,
ClientCipherSuites, Compression,
ConnectionStates0, Renegotiation, SecureRenegotation) ->
- {ok, ConnectionStates} = handle_renegotiation_info(RecordCB, Role, Info, ConnectionStates0,
+ {ok, ConnectionStates} = handle_renegotiation_info(Version, RecordCB, Role, Info, ConnectionStates0,
Renegotiation, SecureRenegotation,
ClientCipherSuites),
hello_pending_connection_states(RecordCB, Role,
@@ -2936,11 +2940,11 @@ renegotiation_info(_RecordCB, server, ConnectionStates, true) ->
#renegotiation_info{renegotiated_connection = undefined}
end.
-handle_renegotiation_info(_RecordCB, _, #renegotiation_info{renegotiated_connection = ?byte(0)},
+handle_renegotiation_info(_, _RecordCB, _, #renegotiation_info{renegotiated_connection = ?byte(0)},
ConnectionStates, false, _, _) ->
{ok, ssl_record:set_renegotiation_flag(true, ConnectionStates)};
-handle_renegotiation_info(_RecordCB, server, undefined, ConnectionStates, _, _, CipherSuites) ->
+handle_renegotiation_info(_, _RecordCB, server, undefined, ConnectionStates, _, _, CipherSuites) ->
case is_member(?TLS_EMPTY_RENEGOTIATION_INFO_SCSV, CipherSuites) of
true ->
{ok, ssl_record:set_renegotiation_flag(true, ConnectionStates)};
@@ -2948,10 +2952,10 @@ handle_renegotiation_info(_RecordCB, server, undefined, ConnectionStates, _, _,
{ok, ssl_record:set_renegotiation_flag(false, ConnectionStates)}
end;
-handle_renegotiation_info(_RecordCB, _, undefined, ConnectionStates, false, _, _) ->
+handle_renegotiation_info(_, _RecordCB, _, undefined, ConnectionStates, false, _, _) ->
{ok, ssl_record:set_renegotiation_flag(false, ConnectionStates)};
-handle_renegotiation_info(_RecordCB, client, #renegotiation_info{renegotiated_connection = ClientServerVerify},
+handle_renegotiation_info(_, _RecordCB, client, #renegotiation_info{renegotiated_connection = ClientServerVerify},
ConnectionStates, true, _, _) ->
ConnectionState = ssl_record:current_connection_state(ConnectionStates, read),
CData = maps:get(client_verify_data, ConnectionState),
@@ -2962,7 +2966,7 @@ handle_renegotiation_info(_RecordCB, client, #renegotiation_info{renegotiated_co
false ->
throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, client_renegotiation))
end;
-handle_renegotiation_info(_RecordCB, server, #renegotiation_info{renegotiated_connection = ClientVerify},
+handle_renegotiation_info(_, _RecordCB, server, #renegotiation_info{renegotiated_connection = ClientVerify},
ConnectionStates, true, _, CipherSuites) ->
case is_member(?TLS_EMPTY_RENEGOTIATION_INFO_SCSV, CipherSuites) of
@@ -2978,11 +2982,13 @@ handle_renegotiation_info(_RecordCB, server, #renegotiation_info{renegotiated_co
throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, server_renegotiation))
end
end;
+handle_renegotiation_info({3,0}, _RecordCB, client, undefined, ConnectionStates, true, _SecureRenegotation, _) ->
+ {ok, ssl_record:set_renegotiation_flag(true, ConnectionStates)};
-handle_renegotiation_info(RecordCB, client, undefined, ConnectionStates, true, SecureRenegotation, _) ->
+handle_renegotiation_info(_, RecordCB, client, undefined, ConnectionStates, true, SecureRenegotation, _) ->
handle_renegotiation_info(RecordCB, ConnectionStates, SecureRenegotation);
-handle_renegotiation_info(RecordCB, server, undefined, ConnectionStates, true, SecureRenegotation, CipherSuites) ->
+handle_renegotiation_info(_, RecordCB, server, undefined, ConnectionStates, true, SecureRenegotation, CipherSuites) ->
case is_member(?TLS_EMPTY_RENEGOTIATION_INFO_SCSV, CipherSuites) of
true ->
throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, {server_renegotiation, empty_renegotiation_info_scsv}));
diff --git a/lib/ssl/src/tls_handshake.erl b/lib/ssl/src/tls_handshake.erl
index c132f75eae..37265e0759 100644
--- a/lib/ssl/src/tls_handshake.erl
+++ b/lib/ssl/src/tls_handshake.erl
@@ -379,7 +379,7 @@ do_hello(Version, Versions, CipherSuites, Hello, SslOpts, Info, Renegotiation) -
%%--------------------------------------------------------------------
enc_handshake(#hello_request{}, {3, N}) when N < 4 ->
{?HELLO_REQUEST, <<>>};
-enc_handshake(#client_hello{client_version = {Major, Minor},
+enc_handshake(#client_hello{client_version = {Major, Minor} = Version,
random = Random,
session_id = SessionID,
cipher_suites = CipherSuites,
@@ -390,7 +390,7 @@ enc_handshake(#client_hello{client_version = {Major, Minor},
CmLength = byte_size(BinCompMethods),
BinCipherSuites = list_to_binary(CipherSuites),
CsLength = byte_size(BinCipherSuites),
- ExtensionsBin = ssl_handshake:encode_hello_extensions(HelloExtensions),
+ ExtensionsBin = ssl_handshake:encode_hello_extensions(HelloExtensions, Version),
{?CLIENT_HELLO, <<?BYTE(Major), ?BYTE(Minor), Random:32/binary,
?BYTE(SIDLength), SessionID/binary,
diff --git a/lib/ssl/test/property_test/ssl_eqc_handshake.erl b/lib/ssl/test/property_test/ssl_eqc_handshake.erl
index 31934ada2b..21aad26425 100644
--- a/lib/ssl/test/property_test/ssl_eqc_handshake.erl
+++ b/lib/ssl/test/property_test/ssl_eqc_handshake.erl
@@ -132,7 +132,7 @@ client_hello(Version) ->
compression_methods = compressions(Version),
random = client_random(Version),
extensions = client_hello_extensions(Version)
- }.
+ };
client_hello(?'SSL_v3' = Version) ->
#client_hello{session_id = session_id(),
client_version = Version,
diff --git a/lib/ssl/test/ssl_handshake_SUITE.erl b/lib/ssl/test/ssl_handshake_SUITE.erl
index 1e6adf3e86..2750a4a9dc 100644
--- a/lib/ssl/test/ssl_handshake_SUITE.erl
+++ b/lib/ssl/test/ssl_handshake_SUITE.erl
@@ -141,7 +141,7 @@ encode_single_hello_sni_extension_correctly(_Config) ->
$t, $e, $s, $t, $., $c, $o, $m>>,
ExtSize = byte_size(SNI),
HelloExt = <<ExtSize:16/unsigned-big-integer, SNI/binary>>,
- Encoded = ssl_handshake:encode_extensions([#sni{hostname = "test.com"}], {3,3}),
+ Encoded = ssl_handshake:encode_extensions([#sni{hostname = "test.com"}]),
HelloExt = Encoded.
decode_single_hello_sni_extension_correctly(_Config) ->
@@ -214,7 +214,7 @@ encode_decode_srp(_Config) ->
0,3, % HostNameLength
98,97,114>>, % hostname = "bar"
EncodedExts0 = <<?UINT16(_),EncodedExts/binary>> =
- ssl_handshake:encode_hello_extensions(Exts),
+ ssl_handshake:encode_hello_extensions(Exts, {3,3}),
Exts = ssl_handshake:decode_hello_extensions(EncodedExts, {3,3}, {3,3}, client).
signature_algorithms(Config) ->
diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl
index bf2c0e7edf..49e7b50b1b 100644
--- a/lib/ssl/test/ssl_test_lib.erl
+++ b/lib/ssl/test/ssl_test_lib.erl
@@ -703,11 +703,7 @@ make_dsa_cert(Config) ->
[{server_dsa_opts, ServerConf},
{server_dsa_verify_opts, [{verify, verify_peer} | ServerConf]},
- {client_dsa_opts, ClientConf},
- {server_srp_dsa, [{user_lookup_fun, {fun user_lookup/3, undefined}},
- {ciphers, srp_dss_suites()} | ServerConf]},
- {client_srp_dsa, [{srp_identity, {"Test-User", "secret"}}
- | ClientConf]}
+ {client_dsa_opts, ClientConf}
| Config];
false ->
Config
@@ -2216,8 +2212,8 @@ filter_suites(Ciphers0, AtomVersion) ->
++ ssl_cipher:anonymous_suites(Version)
++ ssl_cipher:psk_suites(Version)
++ ssl_cipher:psk_suites_anon(Version)
- ++ ssl_cipher:srp_suites()
- ++ ssl_cipher:srp_suites_anon()
+ ++ ssl_cipher:srp_suites(Version)
+ ++ ssl_cipher:srp_suites_anon(Version)
++ ssl_cipher:rc4_suites(Version),
Supported1 = ssl_cipher:filter_suites(Supported0),
Supported2 = [ssl_cipher_format:suite_bin_to_map(S) || S <- Supported1],
--
cgit v1.2.3