From f0eed56f7346a1146c4acb12bf28ef392a383f33 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin 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(-) 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, < 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, <>}; -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, <>}; @@ -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, < 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 = <>, - 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 = <> = - 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