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

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, <<?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],