Merge branch 'ia/ssl-decryption-error2/OTP-8930' into dev

* ia/ssl-decryption-error2/OTP-8930:
  Added alert in stream cipher case.

6 files changed, 40 insertions, 75 deletions

diff --git a/lib/ssl/src/ssl_cipher.erl b/lib/ssl/src/ssl_cipher.erl
index 175d589931..1c8bbbaf06 100644
--- a/lib/ssl/src/ssl_cipher.erl
+++ b/lib/ssl/src/ssl_cipher.erl
@@ -75,20 +75,12 @@ cipher(?RC4, CipherState, Mac, Fragment) ->
                  S -> S
              end,
     GenStreamCipherList = [Fragment, Mac],
-
-    ?DBG_HEX(GenStreamCipherList),
-    ?DBG_HEX(State0),
     {State1, T} = crypto:rc4_encrypt_with_state(State0, GenStreamCipherList),
-    ?DBG_HEX(T),
     {T, CipherState#cipher_state{state = State1}};
 cipher(?DES, CipherState, Mac, Fragment) ->
     block_cipher(fun(Key, IV, T) ->
 			 crypto:des_cbc_encrypt(Key, IV, T)
 		 end, block_size(des_cbc), CipherState, Mac, Fragment);
-%% cipher(?DES40, CipherState, Mac, Fragment) ->
-%%     block_cipher(fun(Key, IV, T) ->
-%% 			 crypto:des_cbc_encrypt(Key, IV, T)
-%% 		 end, block_size(des_cbc), CipherState, Mac, Fragment);
 cipher(?'3DES', CipherState, Mac, Fragment) ->
     block_cipher(fun(<<K1:8/binary, K2:8/binary, K3:8/binary>>, IV, T) ->
 			 crypto:des3_cbc_encrypt(K1, K2, K3, IV, T)
@@ -109,11 +101,7 @@ block_cipher(Fun, BlockSz, #cipher_state{key=Key, iv=IV} = CS0,
     TotSz = byte_size(Mac) + erlang:iolist_size(Fragment) + 1,
     {PaddingLength, Padding} = get_padding(TotSz, BlockSz),
     L = [Fragment, Mac, PaddingLength, Padding],
-    ?DBG_HEX(Key),
-    ?DBG_HEX(IV),
-    ?DBG_HEX(L),
     T = Fun(Key, IV, L),
-    ?DBG_HEX(T),
     NextIV = next_iv(T, IV),
     {T, CS0#cipher_state{iv=NextIV}}.
 
@@ -127,26 +115,29 @@ block_cipher(Fun, BlockSz, #cipher_state{key=Key, iv=IV} = CS0,
 decipher(?NULL, _HashSz, CipherState, Fragment, _) ->
     {Fragment, <<>>, CipherState};
 decipher(?RC4, HashSz, CipherState, Fragment, _) ->
-    ?DBG_TERM(CipherState#cipher_state.key),
     State0 = case CipherState#cipher_state.state of
                  undefined -> crypto:rc4_set_key(CipherState#cipher_state.key);
                  S -> S
              end,
-    ?DBG_HEX(State0),
-    ?DBG_HEX(Fragment),
-    {State1, T} = crypto:rc4_encrypt_with_state(State0, Fragment),
-    ?DBG_HEX(T),
-    GSC = generic_stream_cipher_from_bin(T, HashSz),
-    #generic_stream_cipher{content=Content, mac=Mac} = GSC,
-    {Content, Mac, CipherState#cipher_state{state=State1}};
+    try crypto:rc4_encrypt_with_state(State0, Fragment) of
+	{State, Text} ->
+	    GSC = generic_stream_cipher_from_bin(Text, HashSz),
+	    #generic_stream_cipher{content = Content, mac = Mac} = GSC,
+	    {Content, Mac, CipherState#cipher_state{state = State}}
+    catch
+	_:_ ->
+	    %% This is a DECRYPTION_FAILED but
+	    %% "differentiating between bad_record_mac and decryption_failed
+	    %% alerts may permit certain attacks against CBC mode as used in
+	    %% TLS [CBCATT].  It is preferable to uniformly use the
+	    %% bad_record_mac alert to hide the specific type of the error."
+	    ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC)
+    end;
+
 decipher(?DES, HashSz, CipherState, Fragment, Version) ->
     block_decipher(fun(Key, IV, T) ->
 			   crypto:des_cbc_decrypt(Key, IV, T)
 		   end, CipherState, HashSz, Fragment, Version);
-%% decipher(?DES40, HashSz, CipherState, Fragment, Version) ->
-%%     block_decipher(fun(Key, IV, T) ->
-%% 			   crypto:des_cbc_decrypt(Key, IV, T)
-%% 		   end, CipherState, HashSz, Fragment, Version);
 decipher(?'3DES', HashSz, CipherState, Fragment, Version) ->
     block_decipher(fun(<<K1:8/binary, K2:8/binary, K3:8/binary>>, IV, T) ->
 			   crypto:des3_cbc_decrypt(K1, K2, K3, IV, T)
@@ -178,7 +169,12 @@ block_decipher(Fun, #cipher_state{key=Key, iv=IV} = CipherState0,
 	    end
     catch
 	_:_ ->
-	    ?ALERT_REC(?FATAL, ?DECRYPTION_FAILED)
+	    %% This is a DECRYPTION_FAILED but
+	    %% "differentiating between bad_record_mac and decryption_failed
+	    %% alerts may permit certain attacks against CBC mode as used in
+	    %% TLS [CBCATT].  It is preferable to uniformly use the
+	    %% bad_record_mac alert to hide the specific type of the error."
+	    ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC)
     end.
 %%--------------------------------------------------------------------
 -spec suites(tls_version()) -> [cipher_suite()].
@@ -416,8 +412,6 @@ bulk_cipher_algorithm(null) ->
 %%     ?IDEA;
 bulk_cipher_algorithm(rc4_128) ->
     ?RC4;
-%% bulk_cipher_algorithm(des40_cbc) ->
-%%     ?DES40;
 bulk_cipher_algorithm(des_cbc) ->
     ?DES;
 bulk_cipher_algorithm('3des_ede_cbc') ->
@@ -431,7 +425,6 @@ type(Cipher) when Cipher == null;
     ?STREAM;
 
 type(Cipher) when Cipher == idea_cbc;
-		  Cipher == des40_cbc;
 		  Cipher == des_cbc;
 		  Cipher == '3des_ede_cbc';
 		  Cipher == aes_128_cbc;
@@ -443,8 +436,6 @@ key_material(null) ->
 key_material(Cipher) when Cipher == idea_cbc;
  			  Cipher == rc4_128 ->
     16;
-%%key_material(des40_cbc) ->	
-%%   5;
 key_material(des_cbc) ->
     8;
 key_material('3des_ede_cbc') ->
@@ -459,8 +450,7 @@ expanded_key_material(null) ->
 expanded_key_material(Cipher) when Cipher == idea_cbc;
  				   Cipher == rc4_128 ->
     16;
-expanded_key_material(Cipher) when Cipher == des_cbc;
- 				   Cipher == des40_cbc ->
+expanded_key_material(Cipher) when Cipher == des_cbc ->
     8;
 expanded_key_material('3des_ede_cbc') ->
     24;
@@ -471,8 +461,6 @@ expanded_key_material(Cipher) when Cipher == aes_128_cbc;
 
 effective_key_bits(null) ->
     0;
-%%effective_key_bits(des40_cbc) -> 
-%%    40;
 effective_key_bits(des_cbc) ->
     56;
 effective_key_bits(Cipher) when Cipher == idea_cbc;
@@ -491,7 +479,6 @@ iv_size(Cipher) ->
     block_size(Cipher).
 
 block_size(Cipher) when Cipher == idea_cbc;
-			Cipher == des40_cbc;
 			Cipher == des_cbc;
 			Cipher == '3des_ede_cbc' -> 
     8;
diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl
index 3a9cada81e..ce90d22c09 100644
--- a/lib/ssl/src/ssl_connection.erl
+++ b/lib/ssl/src/ssl_connection.erl
@@ -967,15 +967,14 @@ handle_info(Msg, StateName, State) ->
 %% necessary cleaning up. When it returns, the gen_fsm terminates with
 %% Reason. The return value is ignored.
 %%--------------------------------------------------------------------
-terminate(_Reason, connection, #state{negotiated_version = Version,
+terminate(Reason, connection, #state{negotiated_version = Version,
 				      connection_states = ConnectionStates,
 				      transport_cb = Transport,
 				      socket = Socket, send_queue = SendQueue,
 				      renegotiation = Renegotiate}) ->
     notify_senders(SendQueue),
     notify_renegotiater(Renegotiate),
-    {BinAlert, _} = encode_alert(?ALERT_REC(?WARNING,?CLOSE_NOTIFY),
-				 Version, ConnectionStates),
+    BinAlert = terminate_alert(Reason, Version, ConnectionStates),
     Transport:send(Socket, BinAlert),
     workaround_transport_delivery_problems(Socket, Transport),
     Transport:close(Socket);
@@ -1519,7 +1518,7 @@ handle_server_key(
 	true ->
 	    dh_master_secret(P, G, ServerPublicDhKey, undefined, State);
 	false ->
-	    ?ALERT_REC(?FATAL,?HANDSHAKE_FAILURE)
+	    ?ALERT_REC(?FATAL, ?DECRYPT_ERROR)
     end.
 
 verify_dh_params(Signed, Hashes, {?rsaEncryption, PubKey, _PubKeyParams}) ->
@@ -1574,15 +1573,12 @@ cipher_role(server, Data, Session,  #state{connection_states = ConnectionStates0
 							     tls_handshake_hashes =
 							     Hashes})).
 encode_alert(#alert{} = Alert, Version, ConnectionStates) ->
-    ?DBG_TERM(Alert),
     ssl_record:encode_alert_record(Alert, Version, ConnectionStates).
 
 encode_change_cipher(#change_cipher_spec{}, Version, ConnectionStates) ->
-    ?DBG_TERM(#change_cipher_spec{}),
     ssl_record:encode_change_cipher_spec(Version, ConnectionStates).
 
 encode_handshake(HandshakeRec, Version, ConnectionStates0, Hashes0) ->
-    ?DBG_TERM(HandshakeRec),
     Frag = ssl_handshake:encode_handshake(HandshakeRec, Version),
     Hashes1 = ssl_handshake:update_hashes(Hashes0, Frag),
     {E, ConnectionStates1} =
@@ -1840,7 +1836,6 @@ next_state(StateName, #ssl_tls{type = ?APPLICATION_DATA, fragment = Data}, State
 next_state(StateName, #ssl_tls{type = ?CHANGE_CIPHER_SPEC, fragment = <<1>>} = 
  	   _ChangeCipher, 
  	   #state{connection_states = ConnectionStates0} = State0) ->
-    ?DBG_TERM(_ChangeCipher),
     ConnectionStates1 =
  	ssl_record:activate_pending_connection_state(ConnectionStates0, read),
     {Record, State} = next_record(State0#state{connection_states = ConnectionStates1}),
@@ -2191,6 +2186,15 @@ notify_renegotiater({true, From}) when not is_atom(From)  ->
 notify_renegotiater(_) ->
     ok.
 
+terminate_alert(Reason, Version, ConnectionStates) when Reason == normal; Reason == shutdown ->
+    {BinAlert, _} = encode_alert(?ALERT_REC(?WARNING, ?CLOSE_NOTIFY),
+				 Version, ConnectionStates),
+    BinAlert;
+terminate_alert(_, Version, ConnectionStates) ->
+    {BinAlert, _} = encode_alert(?ALERT_REC(?FATAL, ?INTERNAL_ERROR),
+				 Version, ConnectionStates),
+    BinAlert.
+
 workaround_transport_delivery_problems(Socket, Transport) ->
     %% Standard trick to try to make sure all
     %% data sent to to tcp port is really sent
diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index f8e5d585e7..7082a23fd0 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -428,13 +428,11 @@ finished(Version, Role, MasterSecret, {Hashes, _}) -> % use the current hashes
 verify_connection(Version, #finished{verify_data = Data}, 
 		  Role, MasterSecret, {_, {MD5, SHA}}) -> 
     %% use the previous hashes
-    ?DBG_HEX(crypto:md5_final(MD5)),
-    ?DBG_HEX(crypto:sha_final(SHA)),
     case calc_finished(Version, Role, MasterSecret, {MD5, SHA}) of
 	Data ->
 	    verified;
-	_E ->
- 	    ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE)
+	_ ->
+	    ?ALERT_REC(?FATAL, ?DECRYPT_ERROR)
     end.
 %%--------------------------------------------------------------------
 -spec server_hello_done() ->  #server_hello_done{}.
@@ -507,11 +505,8 @@ update_hashes(Hashes, % special-case SSL2 client hello
 		   CipherSuites:CSLength/binary,
 		   ChallengeData:CDLength/binary>>);
 update_hashes({{MD50, SHA0}, _Prev}, Data) ->
-    ?DBG_HEX(Data),
     {MD51, SHA1} = {crypto:md5_update(MD50, Data),
 		    crypto:sha_update(SHA0, Data)},
-    ?DBG_HEX(crypto:md5_final(MD51)),
-    ?DBG_HEX(crypto:sha_final(SHA1)),
     {{MD51, SHA1}, {MD50, SHA0}}.
 
 %%--------------------------------------------------------------------
@@ -525,7 +520,7 @@ decrypt_premaster_secret(Secret, RSAPrivateKey) ->
 				   [{rsa_pad, rsa_pkcs1_padding}])
     catch
 	_:_ ->
-	    throw(?ALERT_REC(?FATAL, ?DECRYPTION_FAILED))
+	    throw(?ALERT_REC(?FATAL, ?DECRYPT_ERROR))
     end.
 
 %%--------------------------------------------------------------------
@@ -782,8 +777,7 @@ master_secret(Version, MasterSecret, #security_parameters{
      ServerWriteKey, ClientIV, ServerIV} =
 	setup_keys(Version, MasterSecret, ServerRandom, 
 		   ClientRandom, HashSize, KML, EKML, IVS),
-    ?DBG_HEX(ClientWriteKey),
-    ?DBG_HEX(ClientIV),
+
     ConnStates1 = ssl_record:set_master_secret(MasterSecret, ConnectionStates),
     ConnStates2 =
 	ssl_record:set_mac_secret(ClientWriteMacSecret, ServerWriteMacSecret,
@@ -807,8 +801,6 @@ dec_hs(?CLIENT_HELLO, <<?BYTE(Major), ?BYTE(Minor),
 		       ?UINT16(CDLength), 
 		       CipherSuites:CSLength/binary, 
 		       ChallengeData:CDLength/binary>>) ->
-    ?DBG_HEX(CipherSuites),
-    ?DBG_HEX(CipherSuites),
     #client_hello{client_version = {Major, Minor},
 		  random = ssl_ssl2:client_random(ChallengeData, CDLength),
 		  session_id = 0,
diff --git a/lib/ssl/src/ssl_record.erl b/lib/ssl/src/ssl_record.erl
index 803baeb09c..e7a2d8ecf1 100644
--- a/lib/ssl/src/ssl_record.erl
+++ b/lib/ssl/src/ssl_record.erl
@@ -649,9 +649,7 @@ cipher(Type, Version, Fragment, CS0) ->
 						      BCA}
 				}} = 
 	hash_and_bump_seqno(CS0, Type, Version, Length, Fragment),
-    ?DBG_HEX(Fragment),
     {Ciphered, CipherS1} = ssl_cipher:cipher(BCA, CipherS0, MacHash, Fragment),
-    ?DBG_HEX(Ciphered),
     CS2 = CS1#connection_state{cipher_state=CipherS1},
     {Ciphered, CS2}.
 
diff --git a/lib/ssl/src/ssl_ssl3.erl b/lib/ssl/src/ssl_ssl3.erl
index f3cb6ad66e..18c3f4ed3d 100644
--- a/lib/ssl/src/ssl_ssl3.erl
+++ b/lib/ssl/src/ssl_ssl3.erl
@@ -41,9 +41,6 @@
 -spec master_secret(binary(), binary(), binary()) -> binary().
 
 master_secret(PremasterSecret, ClientRandom, ServerRandom) ->
-    ?DBG_HEX(PremasterSecret),
-    ?DBG_HEX(ClientRandom),
-    ?DBG_HEX(ServerRandom),
     %%  draft-ietf-tls-ssl-version3-00 - 6.2.2 
     %% key_block =
     %%   MD5(master_secret + SHA(`A' + master_secret +
@@ -55,9 +52,8 @@ master_secret(PremasterSecret, ClientRandom, ServerRandom) ->
     %%   MD5(master_secret + SHA(`CCC' + master_secret +
     %%                           ServerHello.random +
     %%                           ClientHello.random)) + [...];
-    B = generate_keyblock(PremasterSecret, ClientRandom, ServerRandom, 48),
-    ?DBG_HEX(B),
-    B.
+    Block = generate_keyblock(PremasterSecret, ClientRandom, ServerRandom, 48),
+    Block.
 
 -spec finished(client | server, binary(), {binary(), binary()}) -> binary().
 
@@ -110,14 +106,11 @@ mac_hash(Method, Mac_write_secret, Seq_num, Type, Length, Fragment) ->
     case Method of
         ?NULL -> ok;
         _ ->
-	    ?DBG_HEX(Mac_write_secret),
-	    ?DBG_HEX(hash(Method, Fragment)),
             ok
     end,
     Mac = mac_hash(Method, Mac_write_secret, 
 		   [<<?UINT64(Seq_num), ?BYTE(Type), 
 		     ?UINT16(Length)>>, Fragment]),
-    ?DBG_HEX(Mac),
     Mac.
 
 -spec setup_keys(binary(), binary(), binary(),  
@@ -139,12 +132,6 @@ setup_keys(MasterSecret, ServerRandom, ClientRandom, HS, KML, _EKML, IVS) ->
     <<ClientWriteMacSecret:HS/binary, ServerWriteMacSecret:HS/binary,
      ClientWriteKey:KML/binary, ServerWriteKey:KML/binary,
      ClientIV:IVS/binary, ServerIV:IVS/binary>> = KeyBlock,
-    ?DBG_HEX(ClientWriteMacSecret),
-    ?DBG_HEX(ServerWriteMacSecret),
-    ?DBG_HEX(ClientWriteKey),
-    ?DBG_HEX(ServerWriteKey),
-    ?DBG_HEX(ClientIV),
-    ?DBG_HEX(ServerIV),
     {ClientWriteMacSecret, ServerWriteMacSecret, ClientWriteKey,
      ServerWriteKey, ClientIV, ServerIV}.
 
diff --git a/lib/ssl/src/ssl_tls1.erl b/lib/ssl/src/ssl_tls1.erl
index dd66418dd8..88394b23ba 100644
--- a/lib/ssl/src/ssl_tls1.erl
+++ b/lib/ssl/src/ssl_tls1.erl
@@ -132,15 +132,12 @@ mac_hash(Method, Mac_write_secret, Seq_num, Type, {Major, Minor},
     case Method of
         ?NULL -> ok;
         _ ->
-	    ?DBG_HEX(Mac_write_secret),
-	    ?DBG_HEX(hash(Method, Fragment)),
             ok
     end,
     Mac = hmac_hash(Method, Mac_write_secret, 
 		    [<<?UINT64(Seq_num), ?BYTE(Type), 
 		      ?BYTE(Major), ?BYTE(Minor), ?UINT16(Length)>>, 
 		     Fragment]),
-    ?DBG_HEX(Mac),
     Mac.
 
 -spec suites() -> [cipher_suite()].