From 121047232026505d0e23de1cc15137d986a4b4b4 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Fri, 12 Nov 2010 17:28:11 +0100 Subject: Added alert in stream cipher case. Also changed alert to BAD_RECORD_MAC as: "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." Also cleaned up the code and changed a few other alert reasons in according to alert descriptions in the TLS RFC 4346. And added function terminate_alert/3 so that we can differentiate between a crash in ssl (a bug in our code) and a crash in the application using ssl. --- lib/ssl/src/ssl_cipher.erl | 57 ++++++++++++++++-------------------------- lib/ssl/src/ssl_connection.erl | 20 +++++++++------ lib/ssl/src/ssl_handshake.erl | 16 +++--------- lib/ssl/src/ssl_record.erl | 2 -- lib/ssl/src/ssl_ssl3.erl | 17 ++----------- lib/ssl/src/ssl_tls1.erl | 3 --- 6 files changed, 40 insertions(+), 75 deletions(-) (limited to 'lib/ssl') 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(<>, 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(<>, 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, <>) -> - ?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, [<>, Fragment]), - ?DBG_HEX(Mac), Mac. -spec setup_keys(binary(), binary(), binary(), @@ -139,12 +132,6 @@ setup_keys(MasterSecret, ServerRandom, ClientRandom, HS, KML, _EKML, IVS) -> <> = 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, [<>, Fragment]), - ?DBG_HEX(Mac), Mac. -spec suites() -> [cipher_suite()]. -- cgit v1.2.3