ssl: Incorrect inputed cipherlist lead server to think that the client

did not support secure renegotiation

3 files changed, 65 insertions, 27 deletions

diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index f5c0034f1b..2b9bae6e80 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2013-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2013-2014. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -56,7 +56,7 @@
 
 %% Extensions handling
 -export([client_hello_extensions/6,
-	 handle_client_hello_extensions/8, %% Returns server hello extensions
+	 handle_client_hello_extensions/9, %% Returns server hello extensions
 	 handle_server_hello_extensions/9, select_curve/2
 	]).
 
@@ -1088,17 +1088,19 @@ certificate_authorities_from_db(CertDbHandle, CertDbRef) ->
 
 %%-------------Extension handling --------------------------------
 
-handle_client_hello_extensions(RecordCB, Random,
-			#hello_extensions{renegotiation_info = Info,
-					  srp = SRP,
-					  ec_point_formats = ECCFormat,
-					  next_protocol_negotiation = NextProtocolNegotiation}, Version,
-			#ssl_options{secure_renegotiate = SecureRenegotation} = Opts,
-			#session{cipher_suite = CipherSuite, compression_method = Compression} = Session0,
-			ConnectionStates0, Renegotiation) ->
+handle_client_hello_extensions(RecordCB, Random, ClientCipherSuites,
+			       #hello_extensions{renegotiation_info = Info,
+						 srp = SRP,
+						 ec_point_formats = ECCFormat,
+						 next_protocol_negotiation = NextProtocolNegotiation}, Version,
+			       #ssl_options{secure_renegotiate = SecureRenegotation} = Opts,
+			       #session{cipher_suite = NegotiatedCipherSuite,
+					compression_method = Compression} = Session0,
+			       ConnectionStates0, Renegotiation) ->
     Session = handle_srp_extension(SRP, Session0),
     ConnectionStates = handle_renegotiation_extension(server, RecordCB, Version, Info,
-						      Random, CipherSuite, Compression,
+						      Random, NegotiatedCipherSuite, 
+						      ClientCipherSuites, Compression,
 						      ConnectionStates0, Renegotiation, SecureRenegotation),
     ProtocolsToAdvertise = handle_next_protocol_extension(NextProtocolNegotiation, Renegotiation, Opts),
    
@@ -1117,7 +1119,8 @@ handle_server_hello_extensions(RecordCB, Random, CipherSuite, Compression,
 			       #ssl_options{secure_renegotiate = SecureRenegotation,
 					    next_protocol_selector = NextProtoSelector},
 			       ConnectionStates0, Renegotiation) ->
-    ConnectionStates = handle_renegotiation_extension(client, RecordCB, Version, Info, Random, CipherSuite,
+    ConnectionStates = handle_renegotiation_extension(client, RecordCB, Version, Info, Random, 
+						      CipherSuite, undefined,
 						      Compression, ConnectionStates0,
 						      Renegotiation, SecureRenegotation),
     case handle_next_protocol(NextProtocolNegotiation, NextProtoSelector, Renegotiation) of
@@ -1415,15 +1418,16 @@ calc_master_secret({3,0}, _PrfAlgo, PremasterSecret, ClientRandom, ServerRandom)
 calc_master_secret({3,_}, PrfAlgo, PremasterSecret, ClientRandom, ServerRandom) ->
     tls_v1:master_secret(PrfAlgo, PremasterSecret, ClientRandom, ServerRandom).
 
-handle_renegotiation_extension(Role, RecordCB, Version, Info, Random, CipherSuite, Compression,
+handle_renegotiation_extension(Role, RecordCB, Version, Info, Random, NegotiatedCipherSuite, 
+			       ClientCipherSuites, Compression,
 			       ConnectionStates0, Renegotiation, SecureRenegotation) ->
     case handle_renegotiation_info(RecordCB, Role, Info, ConnectionStates0,
 				   Renegotiation, SecureRenegotation,
-				   [CipherSuite]) of
+				   ClientCipherSuites) of
 	{ok, ConnectionStates} ->
 	    hello_pending_connection_states(RecordCB, Role,
 					    Version,
-					    CipherSuite,
+					    NegotiatedCipherSuite,
 					    Random,
 					    Compression,
 					    ConnectionStates);
diff --git a/lib/ssl/src/tls_handshake.erl b/lib/ssl/src/tls_handshake.erl
index 003614b448..01abefca46 100644
--- a/lib/ssl/src/tls_handshake.erl
+++ b/lib/ssl/src/tls_handshake.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2007-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2007-2014. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -52,9 +52,9 @@ client_hello(Host, Port, ConnectionStates,
     Pending = ssl_record:pending_connection_state(ConnectionStates, read),
     SecParams = Pending#connection_state.security_parameters,
     CipherSuites = ssl_handshake:available_suites(UserSuites, Version),
-
-    Extensions = ssl_handshake:client_hello_extensions(Host, Version, CipherSuites,
-						SslOpts, ConnectionStates, Renegotiation),
+    Extensions = ssl_handshake:client_hello_extensions(Host, Version, 
+						       CipherSuites,
+						       SslOpts, ConnectionStates, Renegotiation),
 
     Id = ssl_session:client_id({Host, Port, SslOpts}, Cache, CacheCb, OwnCert),
 
@@ -87,8 +87,8 @@ hello(#server_hello{server_version = Version, random = Random,
       ConnectionStates0, Renegotiation) ->
     case tls_record:is_acceptable_version(Version, SupportedVersions) of
 	true ->
-	    handle_hello_extensions(Version, SessionId, Random, CipherSuite,
-				    Compression, HelloExt, SslOpt, ConnectionStates0, Renegotiation);
+	    handle_server_hello_extensions(Version, SessionId, Random, CipherSuite,
+					   Compression, HelloExt, SslOpt, ConnectionStates0, Renegotiation);
 	false ->
 	    ?ALERT_REC(?FATAL, ?PROTOCOL_VERSION)
     end;
@@ -113,9 +113,9 @@ hello(#client_hello{client_version = ClientVersion,
 		no_suite ->
 		    ?ALERT_REC(?FATAL, ?INSUFFICIENT_SECURITY);
 		_ ->
-		    handle_hello_extensions(Version, Type, Random, HelloExt,
-					    SslOpts, Session1, ConnectionStates0,
-					    Renegotiation)
+		    handle_client_hello_extensions(Version, Type, Random, CipherSuites, HelloExt,
+						   SslOpts, Session1, ConnectionStates0,
+						   Renegotiation)
 	    end;
 	false ->
 	    ?ALERT_REC(?FATAL, ?PROTOCOL_VERSION)
@@ -217,8 +217,10 @@ enc_handshake(HandshakeMsg, Version) ->
     ssl_handshake:encode_handshake(HandshakeMsg, Version).
 
 
-handle_hello_extensions(Version, Type, Random, HelloExt, SslOpts, Session0, ConnectionStates0, Renegotiation) ->
-    try ssl_handshake:handle_client_hello_extensions(tls_record, Random, HelloExt, Version, SslOpts,
+handle_client_hello_extensions(Version, Type, Random, CipherSuites,
+			HelloExt, SslOpts, Session0, ConnectionStates0, Renegotiation) ->
+    try ssl_handshake:handle_client_hello_extensions(tls_record, Random, CipherSuites,
+						     HelloExt, Version, SslOpts,
 						     Session0, ConnectionStates0, Renegotiation) of
 	{Session, ConnectionStates, ServerHelloExt} ->
 	    {Version, {Type, Session}, ConnectionStates, ServerHelloExt}
@@ -227,7 +229,7 @@ handle_hello_extensions(Version, Type, Random, HelloExt, SslOpts, Session0, Conn
     end.
 
 
-handle_hello_extensions(Version, SessionId, Random, CipherSuite,
+handle_server_hello_extensions(Version, SessionId, Random, CipherSuite,
 			Compression, HelloExt, SslOpt, ConnectionStates0, Renegotiation) ->
     case ssl_handshake:handle_server_hello_extensions(tls_record, Random, CipherSuite,
 						      Compression, HelloExt, Version,
diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl
index bc7e68a86c..1006b23a30 100644
--- a/lib/ssl/test/ssl_basic_SUITE.erl
+++ b/lib/ssl/test/ssl_basic_SUITE.erl
@@ -144,6 +144,7 @@ session_tests() ->
 renegotiate_tests() ->
     [client_renegotiate,
      server_renegotiate,
+     client_secure_renegotiate,
      client_renegotiate_reused_session,
      server_renegotiate_reused_session,
      client_no_wrap_sequence_number,
@@ -1979,6 +1980,37 @@ client_renegotiate(Config) when is_list(Config) ->
     ssl_test_lib:close(Client).
 
 %%--------------------------------------------------------------------
+client_secure_renegotiate() ->
+    [{doc,"Test ssl:renegotiate/1 on client."}].
+client_secure_renegotiate(Config) when is_list(Config) ->
+    ServerOpts = ?config(server_opts, Config),
+    ClientOpts = ?config(client_opts, Config),
+
+    {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+    Data = "From erlang to erlang",
+
+    Server =
+	ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+				   {from, self()},
+				   {mfa, {?MODULE, erlang_ssl_receive, [Data]}},
+				   {options, [{secure_renegotiate, true} | ServerOpts]}]),
+    Port = ssl_test_lib:inet_port(Server),
+
+    Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port},
+					{host, Hostname},
+					{from, self()},
+					{mfa, {?MODULE,
+					       renegotiate, [Data]}},
+					{options, [{reuse_sessions, false},
+						   {secure_renegotiate, true}| ClientOpts]}]),
+    
+    ssl_test_lib:check_result(Client, ok, Server, ok),
+    ssl_test_lib:close(Server),
+    ssl_test_lib:close(Client).
+
+
+%%--------------------------------------------------------------------
 server_renegotiate() ->
     [{doc,"Test ssl:renegotiate/1 on server."}].
 server_renegotiate(Config) when is_list(Config) ->