Merge branch 'ia/pr/340' into maint

* ia/pr/340:
  ssl: Refactor so that there is only one source for the default hashsign values
  ssl: always pass negotiated version when selecting hashsign
  ssl: TLSv1.2: proper default sign algo for RSA

5 files changed, 90 insertions, 84 deletions

diff --git a/lib/ssl/src/dtls_connection.erl b/lib/ssl/src/dtls_connection.erl
index 57f8dd86d3..508983ddac 100644
--- a/lib/ssl/src/dtls_connection.erl
+++ b/lib/ssl/src/dtls_connection.erl
@@ -202,13 +202,14 @@ hello(Hello = #client_hello{client_version = ClientVersion,
 		     session_cache = Cache,
 		     session_cache_cb = CacheCb,
 		     ssl_options = SslOpts}) ->
-    HashSign = ssl_handshake:select_hashsign(HashSigns, Cert),
     case dtls_handshake:hello(Hello, SslOpts, {Port, Session0, Cache, CacheCb,
 					      ConnectionStates0, Cert}, Renegotiation) of
         {Version, {Type, Session},
 	 ConnectionStates,
 	 #hello_extensions{ec_point_formats = EcPointFormats,
 			   elliptic_curves = EllipticCurves} = ServerHelloExt} ->
+            HashSign = ssl_handshake:select_hashsign(HashSigns, Cert, 
+						     dtls_v1:corresponding_tls_version(Version)),
             ssl_connection:hello({common_client_hello, Type, ServerHelloExt, HashSign},
 				 State#state{connection_states  = ConnectionStates,
 					     negotiated_version = Version,
diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl
index edf49a340b..1eda926bcb 100644
--- a/lib/ssl/src/ssl_connection.erl
+++ b/lib/ssl/src/ssl_connection.erl
@@ -290,12 +290,11 @@ hello(#hello_request{}, #state{role = client} = State0, Connection) ->
     {Record, State} = Connection:next_record(State0),
     Connection:next_state(hello, hello, Record, State);
 
-hello({common_client_hello, Type, ServerHelloExt, HashSign},
-      #state{session = #session{cipher_suite = CipherSuite},
-	     negotiated_version = Version} = State, Connection) ->
-    {KeyAlg, _, _, _} = ssl_cipher:suite_definition(CipherSuite),
-    NegotiatedHashSign = negotiated_hashsign(HashSign, KeyAlg, Version),
+hello({common_client_hello, Type, ServerHelloExt, NegotiatedHashSign},
+      State, Connection) ->
     do_server_hello(Type, ServerHelloExt,
+		    %% Note NegotiatedHashSign is only negotiated for real if
+		    %% if TLS version is at least TLS-1.2 
 		    State#state{hashsign_algorithm = NegotiatedHashSign}, Connection);
 
 hello(timeout, State, _) ->
@@ -432,7 +431,8 @@ certify(#server_key_exchange{exchange_keys = Keys},
 		    calculate_secret(Params#server_key_params.params,
 				     State#state{hashsign_algorithm = HashSign}, Connection);
 		false ->
-		    ?ALERT_REC(?FATAL, ?DECRYPT_ERROR)
+		    Connection:handle_own_alert(?ALERT_REC(?FATAL, ?DECRYPT_ERROR),
+						Version, certify, State)
 	    end
     end;
 
@@ -441,8 +441,9 @@ certify(#server_key_exchange{} = Msg,
     Connection:handle_unexpected_message(Msg, certify_server_keyexchange, State);
 
 certify(#certificate_request{hashsign_algorithms = HashSigns},
-	#state{session = #session{own_certificate = Cert}} = State0, Connection) ->
-    HashSign = ssl_handshake:select_hashsign(HashSigns, Cert),
+	#state{session = #session{own_certificate = Cert},
+        negotiated_version = Version} = State0, Connection) ->
+    HashSign = ssl_handshake:select_hashsign(HashSigns, Cert, Version),
     {Record, State} = Connection:next_record(State0#state{client_certificate_requested = true}),
     Connection:next_state(certify, certify, Record,
 			  State#state{cert_hashsign_algorithm = HashSign});
@@ -559,7 +560,7 @@ cipher(#certificate_verify{signature = Signature, hashsign_algorithm = CertHashS
 	      tls_handshake_history = Handshake
 	     } = State0, Connection) ->
 
-    HashSign = ssl_handshake:select_cert_hashsign(CertHashSign, Algo, Version),
+    HashSign = ssl_handshake:select_hashsign_algs(CertHashSign, Algo, Version),
     case ssl_handshake:certificate_verify(Signature, PublicKeyInfo,
 					  Version, HashSign, MasterSecret, Handshake) of
 	valid ->
@@ -1563,60 +1564,6 @@ cipher_role(server, Data, Session,  #state{connection_states = ConnectionStates0
 					session = Session}, cipher, Connection),
     Connection:next_state_connection(cipher, ack_connection(State#state{session = Session})).
 
-negotiated_hashsign(undefined, Algo, Version) ->
-    default_hashsign(Version, Algo);
-negotiated_hashsign(HashSign = {_, _}, _, _) ->
-    HashSign.
-
-%% RFC 5246, Sect. 7.4.1.4.1.  Signature Algorithms
-%% If the client does not send the signature_algorithms extension, the
-%% server MUST do the following:
-%%
-%% -  If the negotiated key exchange algorithm is one of (RSA, DHE_RSA,
-%%    DH_RSA, RSA_PSK, ECDH_RSA, ECDHE_RSA), behave as if client had
-%%    sent the value {sha1,rsa}.
-%%
-%% -  If the negotiated key exchange algorithm is one of (DHE_DSS,
-%%    DH_DSS), behave as if the client had sent the value {sha1,dsa}.
-%%
-%% -  If the negotiated key exchange algorithm is one of (ECDH_ECDSA,
-%%    ECDHE_ECDSA), behave as if the client had sent value {sha1,ecdsa}.
-
-default_hashsign(_Version = {Major, Minor}, KeyExchange)
-  when Major >= 3 andalso Minor >= 3 andalso
-       (KeyExchange == rsa orelse
-	KeyExchange == dhe_rsa orelse
-	KeyExchange == dh_rsa orelse
-	KeyExchange == ecdhe_rsa orelse
-	KeyExchange == ecdh_rsa orelse
-	KeyExchange == srp_rsa) ->
-    {sha, rsa};
-default_hashsign(_Version, KeyExchange)
-  when KeyExchange == rsa;
-       KeyExchange == dhe_rsa;
-       KeyExchange == dh_rsa;
-       KeyExchange == ecdhe_rsa;
-       KeyExchange == ecdh_rsa;
-       KeyExchange == srp_rsa ->
-    {md5sha, rsa};
-default_hashsign(_Version, KeyExchange)
-  when KeyExchange == ecdhe_ecdsa;
-       KeyExchange == ecdh_ecdsa ->
-    {sha, ecdsa};
-default_hashsign(_Version, KeyExchange)
-  when KeyExchange == dhe_dss;
-       KeyExchange == dh_dss;
-       KeyExchange == srp_dss ->
-    {sha, dsa};
-default_hashsign(_Version, KeyExchange)
-  when KeyExchange == dh_anon;
-       KeyExchange == ecdh_anon;
-       KeyExchange == psk;
-       KeyExchange == dhe_psk;
-       KeyExchange == rsa_psk;
-       KeyExchange == srp_anon ->
-    {null, anon}.
-
 select_curve(#state{client_ecc = {[Curve|_], _}}) ->
     {namedCurve, Curve};
 select_curve(_) ->
@@ -1888,3 +1835,15 @@ new_ssl_options([undefined | Rest0], [Head1| Rest1], Acc) ->
     new_ssl_options(Rest0, Rest1, [Head1 | Acc]);
 new_ssl_options([Head0 | Rest0], [_| Rest1], Acc) ->
     new_ssl_options(Rest0, Rest1, [Head0 | Acc]).
+
+negotiated_hashsign(undefined, Alg, Version) ->
+    %% Not negotiated choose default 
+    case is_anonymous(Alg) of
+	true ->
+	    {null, anon};
+	false ->
+	    ssl_handshake:select_hashsign_algs(Alg, Version)
+    end;
+negotiated_hashsign(HashSign = {_, _}, _, _) ->
+    HashSign.
+
diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index 10dd830baf..fc67d2c28d 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -73,7 +73,8 @@
 	]).
 
 %% MISC
--export([select_version/3, prf/5, select_hashsign/2, select_cert_hashsign/3,
+-export([select_version/3, prf/5, select_hashsign/3, 
+	 select_hashsign_algs/2, select_hashsign_algs/3,
 	 premaster_secret/2, premaster_secret/3, premaster_secret/4]).
 
 %%====================================================================
@@ -590,23 +591,25 @@ prf({3,1}, Secret, Label, Seed, WantedLength) ->
     {ok, tls_v1:prf(?MD5SHA, Secret, Label, Seed, WantedLength)};
 prf({3,_N}, Secret, Label, Seed, WantedLength) ->
     {ok, tls_v1:prf(?SHA256, Secret, Label, Seed, WantedLength)}.
+
+
 %%--------------------------------------------------------------------
--spec select_hashsign(#hash_sign_algos{}| undefined,  undefined | binary()) ->
-			      [{atom(), atom()}] | undefined.
+-spec select_hashsign(#hash_sign_algos{}| undefined,  undefined | binary(), ssl_record:ssl_version()) ->
+			      {atom(), atom()} | undefined.
 
 %%
 %% Description:
 %%--------------------------------------------------------------------
-select_hashsign(_, undefined) ->
+select_hashsign(_, undefined, _Version) ->
     {null, anon};
-select_hashsign(undefined,  Cert) ->
+select_hashsign(undefined,  Cert, Version) ->
     #'OTPCertificate'{tbsCertificate = TBSCert} = public_key:pkix_decode_cert(Cert, otp),
     #'OTPSubjectPublicKeyInfo'{algorithm = {_,Algo, _}} = TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo,
-    select_cert_hashsign(undefined, Algo, {undefined, undefined});
-select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert) ->
+    select_hashsign_algs(undefined, Algo, Version);
+select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert, Version) ->
     #'OTPCertificate'{tbsCertificate = TBSCert} =public_key:pkix_decode_cert(Cert, otp),
     #'OTPSubjectPublicKeyInfo'{algorithm = {_,Algo, _}} = TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo,
-    DefaultHashSign = {_, Sign} = select_cert_hashsign(undefined, Algo, {undefined, undefined}),
+    DefaultHashSign = {_, Sign} = select_hashsign_algs(undefined, Algo, Version),
     case lists:filter(fun({sha, dsa}) ->
 			      true;
 			 ({_, dsa}) ->
@@ -622,26 +625,59 @@ select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert) ->
 	[HashSign| _] ->
 	    HashSign
     end.
+
 %%--------------------------------------------------------------------
--spec select_cert_hashsign(#hash_sign_algos{}| undefined, oid(), ssl_record:ssl_version() | {undefined, undefined}) ->
+-spec select_hashsign_algs(#hash_sign_algos{}| undefined, oid(), ssl_record:ssl_version()) ->
 				  {atom(), atom()}.
 
+%% Description: For TLS 1.2 hash function and signature algorithm pairs can be
+%% negotiated with the signature_algorithms extension,
+%% for previous versions always use appropriate defaults.
+%% RFC 5246, Sect. 7.4.1.4.1.  Signature Algorithms
+%% If the client does not send the signature_algorithms extension, the
+%% server MUST do the following: (e.i defaults for TLS 1.2)
+%%
+%% -  If the negotiated key exchange algorithm is one of (RSA, DHE_RSA,
+%%    DH_RSA, RSA_PSK, ECDH_RSA, ECDHE_RSA), behave as if client had
+%%    sent the value {sha1,rsa}.
 %%
-%% Description: For TLS 1.2 selected cert_hash_sign will be recived
-%% in the handshake message, for previous versions use appropriate defaults.
-%% This function is also used by select_hashsign to extract
-%% the alogrithm of the server cert key.
+%% -  If the negotiated key exchange algorithm is one of (DHE_DSS,
+%%    DH_DSS), behave as if the client had sent the value {sha1,dsa}.
+%%
+%% -  If the negotiated key exchange algorithm is one of (ECDH_ECDSA,
+%%    ECDHE_ECDSA), behave as if the client had sent value {sha1,ecdsa}.
+
 %%--------------------------------------------------------------------
-select_cert_hashsign(HashSign, _, {Major, Minor}) when HashSign =/= undefined andalso
+select_hashsign_algs(HashSign, _, {Major, Minor}) when HashSign =/= undefined andalso
 						       Major >= 3 andalso Minor >= 3 ->
     HashSign;
-select_cert_hashsign(undefined,?'id-ecPublicKey', _) ->
+select_hashsign_algs(undefined, ?rsaEncryption, {Major, Minor}) when Major >= 3 andalso Minor >= 3 ->
+    {sha, rsa};
+select_hashsign_algs(undefined,?'id-ecPublicKey', _) ->
     {sha, ecdsa};
-select_cert_hashsign(undefined, ?rsaEncryption, _) ->
+select_hashsign_algs(undefined, ?rsaEncryption, _) -> 
     {md5sha, rsa};
-select_cert_hashsign(undefined, ?'id-dsa', _) ->
+select_hashsign_algs(undefined, ?'id-dsa', _) ->
     {sha, dsa}.
 
+-spec select_hashsign_algs(atom(),  ssl_record:ssl_version()) -> {atom(), atom()}.
+%% Wrap function to keep the knowledge of the default values in
+%% one place only 
+select_hashsign_algs(Alg, Version) when (Alg == rsa orelse
+				    Alg == dhe_rsa orelse
+				    Alg == dh_rsa orelse
+				    Alg == ecdhe_rsa orelse
+				    Alg == ecdh_rsa orelse
+				    Alg == srp_rsa) ->    
+    select_hashsign_algs(undefined, ?rsaEncryption, Version);
+select_hashsign_algs(Alg, Version) when (Alg == dhe_dss orelse
+				    Alg == dh_dss orelse
+				    Alg == srp_dss) ->
+    select_hashsign_algs(undefined, ?'id-dsa', Version);
+select_hashsign_algs(Alg, Version) when (Alg == ecdhe_ecdsa orelse
+					 Alg == ecdh_ecdsa) ->
+    select_hashsign_algs(undefined, ?'id-ecPublicKey', Version).
+
 %%--------------------------------------------------------------------
 -spec master_secret(atom(), ssl_record:ssl_version(), #session{} | binary(), #connection_states{},
 		   client | server) -> {binary(), #connection_states{}} | #alert{}.
diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl
index c3171da566..8142a18c37 100644
--- a/lib/ssl/src/tls_connection.erl
+++ b/lib/ssl/src/tls_connection.erl
@@ -208,11 +208,11 @@ hello(Hello = #client_hello{client_version = ClientVersion,
 		     session_cache = Cache,
 		     session_cache_cb = CacheCb,
 		     ssl_options = SslOpts}) ->
-    HashSign = ssl_handshake:select_hashsign(HashSigns, Cert),
     case tls_handshake:hello(Hello, SslOpts, {Port, Session0, Cache, CacheCb,
 					      ConnectionStates0, Cert}, Renegotiation) of
         {Version, {Type, Session},
 	 ConnectionStates, ServerHelloExt} ->
+            HashSign = ssl_handshake:select_hashsign(HashSigns, Cert, Version),
             ssl_connection:hello({common_client_hello, Type, ServerHelloExt, HashSign},
 				 State#state{connection_states  = ConnectionStates,
 					     negotiated_version = Version,
diff --git a/lib/ssl/test/ssl_handshake_SUITE.erl b/lib/ssl/test/ssl_handshake_SUITE.erl
index 6d020c472b..5f36842f9e 100644
--- a/lib/ssl/test/ssl_handshake_SUITE.erl
+++ b/lib/ssl/test/ssl_handshake_SUITE.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2008-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2008-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
@@ -26,6 +26,7 @@
 -include_lib("common_test/include/ct.hrl").
 -include("ssl_internal.hrl").
 -include("tls_handshake.hrl").
+-include_lib("public_key/include/public_key.hrl").
 
 %%--------------------------------------------------------------------
 %% Common Test interface functions -----------------------------------
@@ -36,7 +37,8 @@ all() -> [decode_hello_handshake,
 	  decode_single_hello_extension_correctly,
 	  decode_supported_elliptic_curves_hello_extension_correctly,
 	  decode_unknown_hello_extension_correctly,
-	  encode_single_hello_sni_extension_correctly].
+	  encode_single_hello_sni_extension_correctly,
+	  select_proper_tls_1_2_rsa_default_hashsign].
 
 %%--------------------------------------------------------------------
 %% Test Cases --------------------------------------------------------
@@ -95,3 +97,11 @@ encode_single_hello_sni_extension_correctly(_Config) ->
     HelloExt = <<ExtSize:16/unsigned-big-integer, SNI/binary>>,
     Encoded = ssl_handshake:encode_hello_extensions(Exts),
     HelloExt = Encoded.
+
+select_proper_tls_1_2_rsa_default_hashsign(_Config) ->
+    % RFC 5246 section 7.4.1.4.1 tells to use {sha1,rsa} as default signature_algorithm for RSA key exchanges
+    {sha, rsa} = ssl_handshake:select_hashsign_algs(undefined, ?rsaEncryption, {3,3}),
+    % Older versions use MD5/SHA1 combination
+    {md5sha, rsa} = ssl_handshake:select_hashsign_algs(undefined, ?rsaEncryption, {3,2}),
+    {md5sha, rsa} = ssl_handshake:select_hashsign_algs(undefined, ?rsaEncryption, {3,0}).
+