From b83e3d1e33eb39ed346661a361a1a9ec5747ff10 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Thu, 26 Aug 2010 16:32:11 +0200 Subject: Fix handshake problem with multiple messages in one packet If hello and client_key_exchange message is sent together in the same packet, ssl can't handle it and closes the connection. Also fixed compiler warning. --- lib/ssl/src/ssl_connection.erl | 101 ++++++++++++++++++++--------------------- lib/ssl/src/ssl_handshake.erl | 89 ++++++++++++++++++------------------ 2 files changed, 94 insertions(+), 96 deletions(-) diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index 76422155a5..960282b588 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -334,14 +334,12 @@ hello(start, #state{host = Host, port = Port, role = client, ssl_options = SslOpts, transport_cb = Transport, socket = Socket, connection_states = ConnectionStates, - own_cert = Cert, renegotiation = {Renegotiation, _}} = State0) -> Hello = ssl_handshake:client_hello(Host, Port, ConnectionStates, - SslOpts, Cert, - Renegotiation), + SslOpts, Renegotiation), Version = Hello#client_hello.client_version, Hashes0 = ssl_handshake:init_hashes(), @@ -353,7 +351,7 @@ hello(start, #state{host = Host, port = Port, role = client, session = #session{session_id = Hello#client_hello.session_id, is_resumable = false}, - tls_handshake_hashes = Hashes1}, + tls_handshake_hashes = Hashes1}, {Record, State} = next_record(State1), next_state(hello, Record, State); @@ -579,58 +577,61 @@ certify(#client_key_exchange{} = Msg, %% We expect a certificate here handle_unexpected_message(Msg, certify_client_key_exchange, State); -certify(#client_key_exchange{exchange_keys - = #encrypted_premaster_secret{premaster_secret - = EncPMS}}, - #state{negotiated_version = Version, - connection_states = ConnectionStates0, - session = Session0, - private_key = Key} = State0) -> - try ssl_handshake:decrypt_premaster_secret(EncPMS, Key) of - PremasterSecret -> - case ssl_handshake:master_secret(Version, PremasterSecret, - ConnectionStates0, server) of - {MasterSecret, ConnectionStates} -> - Session = Session0#session{master_secret = MasterSecret}, - State1 = State0#state{connection_states = ConnectionStates, - session = Session}, - {Record, State} = next_record(State1), - next_state(cipher, Record, State); - #alert{} = Alert -> - handle_own_alert(Alert, Version, - certify_client_key_exchange, State0), - {stop, normal, State0} - end +certify(#client_key_exchange{exchange_keys = Keys}, + State = #state{key_algorithm = KeyAlg, negotiated_version = Version}) -> + try + certify_client_key_exchange(ssl_handshake:decode_client_key(Keys, KeyAlg, Version), State) catch #alert{} = Alert -> - handle_own_alert(Alert, Version, certify_client_key_exchange, - State0), + handle_own_alert(Alert, Version, certify_client_key_exchange, State), + {stop, normal, State} + end; + +certify(Msg, State) -> + handle_unexpected_message(Msg, certify, State). + +certify_client_key_exchange(#encrypted_premaster_secret{premaster_secret= EncPMS}, + #state{negotiated_version = Version, + connection_states = ConnectionStates0, + session = Session0, + private_key = Key} = State0) -> + PremasterSecret = ssl_handshake:decrypt_premaster_secret(EncPMS, Key), + case ssl_handshake:master_secret(Version, PremasterSecret, + ConnectionStates0, server) of + {MasterSecret, ConnectionStates} -> + Session = Session0#session{master_secret = MasterSecret}, + State1 = State0#state{connection_states = ConnectionStates, + session = Session}, + {Record, State} = next_record(State1), + next_state(cipher, Record, State); + #alert{} = Alert -> + handle_own_alert(Alert, Version, + certify_client_key_exchange, State0), {stop, normal, State0} end; -certify(#client_key_exchange{exchange_keys = #client_diffie_hellman_public{ - dh_public = ClientPublicDhKey}}, - #state{negotiated_version = Version, - diffie_hellman_params = #'DHParameter'{prime = P, - base = G}, - diffie_hellman_keys = {_, ServerDhPrivateKey}, - role = Role, - session = Session, - connection_states = ConnectionStates0} = State0) -> - +certify_client_key_exchange(#client_diffie_hellman_public{dh_public = ClientPublicDhKey}, + #state{negotiated_version = Version, + diffie_hellman_params = #'DHParameter'{prime = P, + base = G}, + diffie_hellman_keys = {_, ServerDhPrivateKey}, + role = Role, + session = Session, + connection_states = ConnectionStates0} = State0) -> + PMpint = crypto:mpint(P), GMpint = crypto:mpint(G), PremasterSecret = crypto:dh_compute_key(mpint_binary(ClientPublicDhKey), ServerDhPrivateKey, [PMpint, GMpint]), - + case ssl_handshake:master_secret(Version, PremasterSecret, ConnectionStates0, Role) of {MasterSecret, ConnectionStates} -> State1 = State0#state{session = - Session#session{master_secret - = MasterSecret}, - connection_states = ConnectionStates}, + Session#session{master_secret + = MasterSecret}, + connection_states = ConnectionStates}, {Record, State} = next_record(State1), next_state(cipher, Record, State); @@ -638,10 +639,7 @@ certify(#client_key_exchange{exchange_keys = #client_diffie_hellman_public{ handle_own_alert(Alert, Version, certify_client_key_exchange, State0), {stop, normal, State0} - end; - -certify(Msg, State) -> - handle_unexpected_message(Msg, certify, State). + end. %%-------------------------------------------------------------------- -spec cipher(#hello_request{} | #certificate_verify{} | #finished{} | term(), @@ -701,14 +699,13 @@ connection(#hello_request{}, #state{host = Host, port = Port, socket = Socket, ssl_options = SslOpts, negotiated_version = Version, - own_cert = Cert, transport_cb = Transport, connection_states = ConnectionStates0, renegotiation = {Renegotiation, _}, tls_handshake_hashes = Hashes0} = State0) -> - Hello = ssl_handshake:client_hello(Host, Port, - ConnectionStates0, SslOpts, Cert, Renegotiation), + Hello = ssl_handshake:client_hello(Host, Port, ConnectionStates0, + SslOpts, Renegotiation), {BinMsg, ConnectionStates1, Hashes1} = encode_handshake(Hello, Version, ConnectionStates0, Hashes0), @@ -1794,9 +1791,7 @@ next_state(Next, #ssl_tls{type = ?ALERT, fragment = EncAlerts}, State) -> handle_alerts(Alerts, {next_state, Next, State}); next_state(StateName, #ssl_tls{type = ?HANDSHAKE, fragment = Data}, - State0 = #state{key_algorithm = KeyAlg, - tls_handshake_buffer = Buf0, - negotiated_version = Version}) -> + State0 = #state{tls_handshake_buffer = Buf0, negotiated_version = Version}) -> Handle = fun({#hello_request{} = Packet, _}, {next_state, connection = SName, State}) -> %% This message should not be included in handshake @@ -1819,7 +1814,7 @@ next_state(StateName, #ssl_tls{type = ?HANDSHAKE, fragment = Data}, (_, StopState) -> StopState end, try - {Packets, Buf} = ssl_handshake:get_tls_handshake(Data,Buf0, KeyAlg,Version), + {Packets, Buf} = ssl_handshake:get_tls_handshake(Data,Buf0), State = State0#state{tls_packets = Packets, tls_handshake_buffer = Buf}, handle_tls_handshake(Handle, StateName, State) catch throw:#alert{} = Alert -> diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index 3d831eae02..ee725997a4 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -31,13 +31,13 @@ -include("ssl_debug.hrl"). -include_lib("public_key/include/public_key.hrl"). --export([master_secret/4, client_hello/6, server_hello/4, hello/4, +-export([master_secret/4, client_hello/5, server_hello/4, hello/4, hello_request/0, certify/7, certificate/3, client_certificate_verify/6, certificate_verify/6, certificate_request/2, key_exchange/2, server_key_exchange_hash/2, finished/4, verify_connection/5, - get_tls_handshake/4, + get_tls_handshake/2, decode_client_key/3, server_hello_done/0, sig_alg/1, encode_handshake/3, init_hashes/0, update_hashes/2, decrypt_premaster_secret/2]). @@ -52,13 +52,13 @@ %%==================================================================== %%-------------------------------------------------------------------- -spec client_hello(host(), port_num(), #connection_states{}, - #ssl_options{}, binary(), boolean()) -> #client_hello{}. + #ssl_options{}, boolean()) -> #client_hello{}. %% %% Description: Creates a client hello message. %%-------------------------------------------------------------------- client_hello(Host, Port, ConnectionStates, #ssl_options{versions = Versions, ciphers = UserSuites} - = SslOpts, Cert, Renegotiation) -> + = SslOpts, Renegotiation) -> Fun = fun(Version) -> ssl_record:protocol_version(Version) @@ -461,29 +461,36 @@ encode_handshake(Package, Version, KeyAlg) -> [MsgType, ?uint24(Len), Bin]. %%-------------------------------------------------------------------- --spec get_tls_handshake(binary(), binary() | iolist(), key_algo(), tls_version()) -> +-spec get_tls_handshake(binary(), binary() | iolist()) -> {[tls_handshake()], binary()}. %% %% Description: Given buffered and new data from ssl_record, collects %% and returns it as a list of handshake messages, also returns leftover %% data. %%-------------------------------------------------------------------- -get_tls_handshake(Data, <<>>, KeyAlg, Version) -> - get_tls_handshake_aux(Data, KeyAlg, Version, []); -get_tls_handshake(Data, Buffer, KeyAlg, Version) -> - get_tls_handshake_aux(list_to_binary([Buffer, Data]), - KeyAlg, Version, []). +get_tls_handshake(Data, <<>>) -> + get_tls_handshake_aux(Data, []); +get_tls_handshake(Data, Buffer) -> + get_tls_handshake_aux(list_to_binary([Buffer, Data]), []). + +%%-------------------------------------------------------------------- +-spec dec_client_key(binary(), key_algo(), tls_version()) -> + #encrypted_premaster_secret{} | #client_diffie_hellman_public{}. +%% +%% Description: Decode client_key data and return appropriate type +%%-------------------------------------------------------------------- +decode_client_key(ClientKey, Type, Version) -> + dec_client_key(ClientKey, key_exchange_alg(Type), Version). %%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- get_tls_handshake_aux(<>, KeyAlg, - Version, Acc) -> + Body:Length/binary,Rest/binary>>, Acc) -> Raw = <>, - H = dec_hs(Type, Body, key_exchange_alg(KeyAlg), Version), - get_tls_handshake_aux(Rest, KeyAlg, Version, [{H,Raw} | Acc]); -get_tls_handshake_aux(Data, _KeyAlg, _Version, Acc) -> + H = dec_hs(Type, Body), + get_tls_handshake_aux(Rest, [{H,Raw} | Acc]); +get_tls_handshake_aux(Data, Acc) -> {lists:reverse(Acc), Data}. verify_bool(verify_peer) -> @@ -729,7 +736,7 @@ master_secret(Version, MasterSecret, #security_parameters{ ServerCipherState, Role)}. -dec_hs(?HELLO_REQUEST, <<>>, _, _) -> +dec_hs(?HELLO_REQUEST, <<>>) -> #hello_request{}; %% Client hello v2. @@ -739,8 +746,7 @@ dec_hs(?CLIENT_HELLO, <>, - _, _) -> + ChallengeData:CDLength/binary>>) -> ?DBG_HEX(CipherSuites), ?DBG_HEX(CipherSuites), #client_hello{client_version = {Major, Minor}, @@ -754,8 +760,7 @@ dec_hs(?CLIENT_HELLO, <>, - _, _) -> + Extensions/binary>>) -> RenegotiationInfo = proplists:get_value(renegotiation_info, dec_hello_extensions(Extensions), undefined), @@ -770,7 +775,7 @@ dec_hs(?CLIENT_HELLO, <>, _, _) -> + Cipher_suite:2/binary, ?BYTE(Comp_method)>>) -> #server_hello{ server_version = {Major,Minor}, random = Random, @@ -782,7 +787,7 @@ dec_hs(?SERVER_HELLO, <>, _, _) -> + ?UINT16(ExtLen), Extensions:ExtLen/binary>>) -> RenegotiationInfo = proplists:get_value(renegotiation_info, dec_hello_extensions(Extensions, []), undefined), @@ -793,44 +798,42 @@ dec_hs(?SERVER_HELLO, <>, _, _) -> +dec_hs(?CERTIFICATE, <>) -> #certificate{asn1_certificates = certs_to_list(ASN1Certs)}; dec_hs(?SERVER_KEY_EXCHANGE, <>, - ?KEY_EXCHANGE_DIFFIE_HELLMAN, _) -> + ?UINT16(Len), Sig:Len/binary>>) -> #server_key_exchange{params = #server_dh_params{dh_p = P,dh_g = G, dh_y = Y}, signed_params = Sig}; dec_hs(?CERTIFICATE_REQUEST, <>, _, _) -> + ?UINT16(CertAuthsLen), CertAuths:CertAuthsLen/binary>>) -> #certificate_request{certificate_types = CertTypes, certificate_authorities = CertAuths}; -dec_hs(?SERVER_HELLO_DONE, <<>>, _, _) -> +dec_hs(?SERVER_HELLO_DONE, <<>>) -> #server_hello_done{}; -dec_hs(?CERTIFICATE_VERIFY,<>, _, _)-> +dec_hs(?CERTIFICATE_VERIFY,<>)-> #certificate_verify{signature = Signature}; -dec_hs(?CLIENT_KEY_EXCHANGE, PKEPMS, ?KEY_EXCHANGE_RSA, {3, 0}) -> - PreSecret = #encrypted_premaster_secret{premaster_secret = PKEPMS}, - #client_key_exchange{exchange_keys = PreSecret}; -dec_hs(?CLIENT_KEY_EXCHANGE, <>, - ?KEY_EXCHANGE_RSA, _) -> - PreSecret = #encrypted_premaster_secret{premaster_secret = PKEPMS}, - #client_key_exchange{exchange_keys = PreSecret}; -dec_hs(?CLIENT_KEY_EXCHANGE, <<>>, ?KEY_EXCHANGE_DIFFIE_HELLMAN, _) -> - throw(?ALERT_REC(?FATAL, ?UNSUPPORTED_CERTIFICATE)); -dec_hs(?CLIENT_KEY_EXCHANGE, <>, - ?KEY_EXCHANGE_DIFFIE_HELLMAN, _) -> - #client_key_exchange{exchange_keys = - #client_diffie_hellman_public{dh_public = DH_Y}}; -dec_hs(?FINISHED, VerifyData, _, _) -> +dec_hs(?CLIENT_KEY_EXCHANGE, PKEPMS) -> + #client_key_exchange{exchange_keys = PKEPMS}; +dec_hs(?FINISHED, VerifyData) -> #finished{verify_data = VerifyData}; -dec_hs(_, _, _, _) -> +dec_hs(_, _) -> throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE)). +dec_client_key(PKEPMS, ?KEY_EXCHANGE_RSA, {3, 0}) -> + #encrypted_premaster_secret{premaster_secret = PKEPMS}; +dec_client_key(<>, ?KEY_EXCHANGE_RSA, _) -> + #encrypted_premaster_secret{premaster_secret = PKEPMS}; +dec_client_key(<<>>, ?KEY_EXCHANGE_DIFFIE_HELLMAN, _) -> + throw(?ALERT_REC(?FATAL, ?UNSUPPORTED_CERTIFICATE)); +dec_client_key(<>, + ?KEY_EXCHANGE_DIFFIE_HELLMAN, _) -> + #client_diffie_hellman_public{dh_public = DH_Y}. + dec_hello_extensions(<<>>) -> []; dec_hello_extensions(<>) -> -- cgit v1.2.3