From 38b07caa2a1c6cd3537eadd36770afa54f067562 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Tue, 7 Nov 2017 18:34:34 +0100 Subject: ssl: Countermeasurements for Bleichenbacher attack --- lib/ssl/src/ssl_connection.erl | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) (limited to 'lib/ssl/src/ssl_connection.erl') diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index b031d3d47b..b33ce63c32 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -1367,8 +1367,25 @@ server_certify_and_key_exchange(State0, Connection) -> request_client_cert(State2, Connection). certify_client_key_exchange(#encrypted_premaster_secret{premaster_secret= EncPMS}, - #state{private_key = Key} = State, Connection) -> - PremasterSecret = ssl_handshake:premaster_secret(EncPMS, Key), + #state{private_key = Key, client_hello_version = {Major, Minor} = Version} = State, Connection) -> + + %% Countermeasure for Bleichenbacher attack always provide some kind of premaster secret + %% and fail handshake later.RFC 5246 section 7.4.7.1. + PremasterSecret = + try ssl_handshake:premaster_secret(EncPMS, Key) of + Secret when erlang:byte_size(Secret) == ?NUM_OF_PREMASTERSECRET_BYTES -> + case Secret of + <> -> %% Correct + Secret; + <> -> %% Version mismatch + <> + end; + _ -> %% erlang:byte_size(Secret) =/= ?NUM_OF_PREMASTERSECRET_BYTES + make_premaster_secret(Version, rsa) + catch + #alert{description = ?DECRYPT_ERROR} -> + make_premaster_secret(Version, rsa) + end, calculate_master_secret(PremasterSecret, State, Connection, certify, cipher); certify_client_key_exchange(#client_diffie_hellman_public{dh_public = ClientPublicDhKey}, -- cgit v1.2.3 From 4147cfeb7c068cc65831d3fc249f6d4a83830a58 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Fri, 13 Oct 2017 13:29:05 +0200 Subject: ssl: Use ?FUNCTION_NAME Use ?FUNCTION_NAME macro to enhance code as we will not back-port this version of the ssl application to versions pre OTP 19. --- lib/ssl/src/ssl_connection.erl | 98 +++++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 49 deletions(-) (limited to 'lib/ssl/src/ssl_connection.erl') diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index b031d3d47b..c6f67eb696 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -366,7 +366,7 @@ init({call, From}, {start, {Opts, EmOpts}, Timeout}, {stop_and_reply, normal, {reply, From, {error, Error}}} end; init({call, From}, Msg, State, Connection) -> - handle_call(Msg, From, init, State, Connection); + handle_call(Msg, From, ?FUNCTION_NAME, State, Connection); init(_Type, _Event, _State, _Connection) -> {keep_state_and_data, [postpone]}. @@ -377,13 +377,13 @@ init(_Type, _Event, _State, _Connection) -> gen_statem:state_function_result(). %%-------------------------------------------------------------------- hello({call, From}, Msg, State, Connection) -> - handle_call(Msg, From, hello, State, Connection); + handle_call(Msg, From, ?FUNCTION_NAME, State, Connection); hello(internal, {common_client_hello, Type, ServerHelloExt}, State, Connection) -> do_server_hello(Type, ServerHelloExt, State, Connection); hello(info, Msg, State, _) -> - handle_info(Msg, hello, State); + handle_info(Msg, ?FUNCTION_NAME, State); hello(Type, Msg, State, Connection) -> - handle_common_event(Type, Msg, hello, State, Connection). + handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection). %%-------------------------------------------------------------------- -spec abbreviated(gen_statem:event_type(), @@ -392,7 +392,7 @@ hello(Type, Msg, State, Connection) -> gen_statem:state_function_result(). %%-------------------------------------------------------------------- abbreviated({call, From}, Msg, State, Connection) -> - handle_call(Msg, From, abbreviated, State, Connection); + handle_call(Msg, From, ?FUNCTION_NAME, State, Connection); abbreviated(internal, #finished{verify_data = Data} = Finished, #state{role = server, @@ -412,7 +412,7 @@ abbreviated(internal, #finished{verify_data = Data} = Finished, expecting_finished = false}, Connection), Connection:next_event(connection, Record, State); #alert{} = Alert -> - handle_own_alert(Alert, Version, abbreviated, State0) + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State0) end; abbreviated(internal, #finished{verify_data = Data} = Finished, @@ -428,11 +428,11 @@ abbreviated(internal, #finished{verify_data = Data} = Finished, ssl_record:set_server_verify_data(current_read, Data, ConnectionStates0), {State1, Actions} = finalize_handshake(State0#state{connection_states = ConnectionStates1}, - abbreviated, Connection), + ?FUNCTION_NAME, Connection), {Record, State} = prepare_connection(State1#state{expecting_finished = false}, Connection), Connection:next_event(connection, Record, State, Actions); #alert{} = Alert -> - handle_own_alert(Alert, Version, abbreviated, State0) + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State0) end; %% only allowed to send next_protocol message after change cipher spec @@ -442,7 +442,7 @@ abbreviated(internal, #next_protocol{selected_protocol = SelectedProtocol}, Connection) -> {Record, State} = Connection:next_record(State0#state{negotiated_protocol = SelectedProtocol}), - Connection:next_event(abbreviated, Record, + Connection:next_event(?FUNCTION_NAME, Record, State#state{expecting_next_protocol_negotiation = false}); abbreviated(internal, #change_cipher_spec{type = <<1>>}, #state{connection_states = ConnectionStates0} = @@ -451,11 +451,11 @@ abbreviated(internal, ssl_record:activate_pending_connection_state(ConnectionStates0, read), {Record, State} = Connection:next_record(State0#state{connection_states = ConnectionStates1}), - Connection:next_event(abbreviated, Record, State#state{expecting_finished = true}); + Connection:next_event(?FUNCTION_NAME, Record, State#state{expecting_finished = true}); abbreviated(info, Msg, State, _) -> - handle_info(Msg, abbreviated, State); + handle_info(Msg, ?FUNCTION_NAME, State); abbreviated(Type, Msg, State, Connection) -> - handle_common_event(Type, Msg, abbreviated, State, Connection). + handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection). %%-------------------------------------------------------------------- -spec certify(gen_statem:event_type(), @@ -465,16 +465,16 @@ abbreviated(Type, Msg, State, Connection) -> gen_statem:state_function_result(). %%-------------------------------------------------------------------- certify({call, From}, Msg, State, Connection) -> - handle_call(Msg, From, certify, State, Connection); + handle_call(Msg, From, ?FUNCTION_NAME, State, Connection); certify(info, Msg, State, _) -> - handle_info(Msg, certify, State); + handle_info(Msg, ?FUNCTION_NAME, State); certify(internal, #certificate{asn1_certificates = []}, #state{role = server, negotiated_version = Version, ssl_options = #ssl_options{verify = verify_peer, fail_if_no_peer_cert = true}} = State, _) -> Alert = ?ALERT_REC(?FATAL,?HANDSHAKE_FAILURE), - handle_own_alert(Alert, Version, certify, State); + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State); certify(internal, #certificate{asn1_certificates = []}, #state{role = server, @@ -483,7 +483,7 @@ certify(internal, #certificate{asn1_certificates = []}, State0, Connection) -> {Record, State} = Connection:next_record(State0#state{client_certificate_requested = false}), - Connection:next_event(certify, Record, State); + Connection:next_event(?FUNCTION_NAME, Record, State); certify(internal, #certificate{}, #state{role = server, @@ -491,7 +491,7 @@ certify(internal, #certificate{}, ssl_options = #ssl_options{verify = verify_none}} = State, _) -> Alert = ?ALERT_REC(?FATAL,?UNEXPECTED_MESSAGE, unrequested_certificate), - handle_own_alert(Alert, Version, certify, State); + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State); certify(internal, #certificate{} = Cert, #state{negotiated_version = Version, @@ -506,7 +506,7 @@ certify(internal, #certificate{} = Cert, handle_peer_cert(Role, PeerCert, PublicKeyInfo, State#state{client_certificate_requested = false}, Connection); #alert{} = Alert -> - handle_own_alert(Alert, Version, certify, State) + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State) end; certify(internal, #server_key_exchange{exchange_keys = Keys}, @@ -538,7 +538,7 @@ certify(internal, #server_key_exchange{exchange_keys = Keys}, Connection); false -> handle_own_alert(?ALERT_REC(?FATAL, ?DECRYPT_ERROR), - Version, certify, State) + Version, ?FUNCTION_NAME, State) end end; @@ -549,10 +549,10 @@ certify(internal, #certificate_request{} = CertRequest, negotiated_version = Version} = State0, Connection) -> case ssl_handshake:select_hashsign(CertRequest, Cert, SupportedHashSigns, ssl:tls_version(Version)) of #alert {} = Alert -> - handle_own_alert(Alert, Version, certify, State0); + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State0); NegotiatedHashSign -> {Record, State} = Connection:next_record(State0#state{client_certificate_requested = true}), - Connection:next_event(certify, Record, + Connection:next_event(?FUNCTION_NAME, Record, State#state{cert_hashsign_algorithm = NegotiatedHashSign}) end; @@ -568,7 +568,7 @@ certify(internal, #server_hello_done{}, when Alg == psk -> case ssl_handshake:premaster_secret({Alg, PSKIdentity}, PSKLookup) of #alert{} = Alert -> - handle_own_alert(Alert, Version, certify, State0); + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State0); PremasterSecret -> State = master_secret(PremasterSecret, State0#state{premaster_secret = PremasterSecret}), @@ -589,7 +589,7 @@ certify(internal, #server_hello_done{}, case ssl_handshake:premaster_secret({Alg, PSKIdentity}, PSKLookup, RSAPremasterSecret) of #alert{} = Alert -> - handle_own_alert(Alert, Version, certify, State0); + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State0); PremasterSecret -> State = master_secret(PremasterSecret, State0#state{premaster_secret = RSAPremasterSecret}), @@ -609,7 +609,7 @@ certify(internal, #server_hello_done{}, State = State0#state{connection_states = ConnectionStates}, client_certify_and_key_exchange(State, Connection); #alert{} = Alert -> - handle_own_alert(Alert, Version, certify, State0) + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State0) end; %% Master secret is calculated from premaster_secret @@ -627,7 +627,7 @@ certify(internal, #server_hello_done{}, session = Session}, client_certify_and_key_exchange(State, Connection); #alert{} = Alert -> - handle_own_alert(Alert, Version, certify, State0) + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State0) end; certify(internal = Type, #client_key_exchange{} = Msg, @@ -636,7 +636,7 @@ certify(internal = Type, #client_key_exchange{} = Msg, ssl_options = #ssl_options{fail_if_no_peer_cert = true}} = State, Connection) -> %% We expect a certificate here - handle_common_event(Type, Msg, certify, State, Connection); + handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection); certify(internal, #client_key_exchange{exchange_keys = Keys}, State = #state{key_algorithm = KeyAlg, negotiated_version = Version}, Connection) -> @@ -645,11 +645,11 @@ certify(internal, #client_key_exchange{exchange_keys = Keys}, State, Connection) catch #alert{} = Alert -> - handle_own_alert(Alert, Version, certify, State) + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State) end; certify(Type, Msg, State, Connection) -> - handle_common_event(Type, Msg, certify, State, Connection). + handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection). %%-------------------------------------------------------------------- -spec cipher(gen_statem:event_type(), @@ -658,10 +658,10 @@ certify(Type, Msg, State, Connection) -> gen_statem:state_function_result(). %%-------------------------------------------------------------------- cipher({call, From}, Msg, State, Connection) -> - handle_call(Msg, From, cipher, State, Connection); + handle_call(Msg, From, ?FUNCTION_NAME, State, Connection); cipher(info, Msg, State, _) -> - handle_info(Msg, cipher, State); + handle_info(Msg, ?FUNCTION_NAME, State); cipher(internal, #certificate_verify{signature = Signature, hashsign_algorithm = CertHashSign}, @@ -680,10 +680,10 @@ cipher(internal, #certificate_verify{signature = Signature, TLSVersion, HashSign, MasterSecret, Handshake) of valid -> {Record, State} = Connection:next_record(State0), - Connection:next_event(cipher, Record, + Connection:next_event(?FUNCTION_NAME, Record, State#state{cert_hashsign_algorithm = HashSign}); #alert{} = Alert -> - handle_own_alert(Alert, Version, cipher, State0) + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State0) end; %% client must send a next protocol message if we are expecting it @@ -691,7 +691,7 @@ cipher(internal, #finished{}, #state{role = server, expecting_next_protocol_negotiation = true, negotiated_protocol = undefined, negotiated_version = Version} = State0, _Connection) -> - handle_own_alert(?ALERT_REC(?FATAL,?UNEXPECTED_MESSAGE), Version, cipher, State0); + handle_own_alert(?ALERT_REC(?FATAL,?UNEXPECTED_MESSAGE), Version, ?FUNCTION_NAME, State0); cipher(internal, #finished{verify_data = Data} = Finished, #state{negotiated_version = Version, @@ -712,7 +712,7 @@ cipher(internal, #finished{verify_data = Data} = Finished, cipher_role(Role, Data, Session, State#state{expecting_finished = false}, Connection); #alert{} = Alert -> - handle_own_alert(Alert, Version, cipher, State) + handle_own_alert(Alert, Version, ?FUNCTION_NAME, State) end; %% only allowed to send next_protocol message after change cipher spec @@ -722,7 +722,7 @@ cipher(internal, #next_protocol{selected_protocol = SelectedProtocol}, expecting_finished = true} = State0, Connection) -> {Record, State} = Connection:next_record(State0#state{negotiated_protocol = SelectedProtocol}), - Connection:next_event(cipher, Record, + Connection:next_event(?FUNCTION_NAME, Record, State#state{expecting_next_protocol_negotiation = false}); cipher(internal, #change_cipher_spec{type = <<1>>}, #state{connection_states = ConnectionStates0} = State0, Connection) -> @@ -730,9 +730,9 @@ cipher(internal, #change_cipher_spec{type = <<1>>}, #state{connection_states = ssl_record:activate_pending_connection_state(ConnectionStates0, read), {Record, State} = Connection:next_record(State0#state{connection_states = ConnectionStates1}), - Connection:next_event(cipher, Record, State#state{expecting_finished = true}); + Connection:next_event(?FUNCTION_NAME, Record, State#state{expecting_finished = true}); cipher(Type, Msg, State, Connection) -> - handle_common_event(Type, Msg, cipher, State, Connection). + handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection). %%-------------------------------------------------------------------- -spec connection(gen_statem:event_type(), term(), @@ -747,7 +747,7 @@ connection({call, From}, {application_data, Data}, try write_application_data(Data, From, State) catch throw:Error -> - hibernate_after(connection, State, [{reply, From, Error}]) + hibernate_after(?FUNCTION_NAME, State, [{reply, From, Error}]) end; connection({call, RecvFrom}, {recv, N, Timeout}, #state{protocol_cb = Connection, socket_options = @@ -755,34 +755,34 @@ connection({call, RecvFrom}, {recv, N, Timeout}, Timer = start_or_recv_cancel_timer(Timeout, RecvFrom), Connection:passive_receive(State0#state{bytes_to_read = N, start_or_recv_from = RecvFrom, - timer = Timer}, connection); + timer = Timer}, ?FUNCTION_NAME); connection({call, From}, renegotiate, #state{protocol_cb = Connection} = State, Connection) -> Connection:renegotiate(State#state{renegotiation = {true, From}}, []); connection({call, From}, peer_certificate, #state{session = #session{peer_certificate = Cert}} = State, _) -> - hibernate_after(connection, State, [{reply, From, {ok, Cert}}]); + hibernate_after(?FUNCTION_NAME, State, [{reply, From, {ok, Cert}}]); connection({call, From}, {connection_information, true}, State, _) -> Info = connection_info(State) ++ security_info(State), - hibernate_after(connection, State, [{reply, From, {ok, Info}}]); + hibernate_after(?FUNCTION_NAME, State, [{reply, From, {ok, Info}}]); connection({call, From}, {connection_information, false}, State, _) -> Info = connection_info(State), - hibernate_after(connection, State, [{reply, From, {ok, Info}}]); + hibernate_after(?FUNCTION_NAME, State, [{reply, From, {ok, Info}}]); connection({call, From}, negotiated_protocol, #state{negotiated_protocol = undefined} = State, _) -> - hibernate_after(connection, State, [{reply, From, {error, protocol_not_negotiated}}]); + hibernate_after(?FUNCTION_NAME, State, [{reply, From, {error, protocol_not_negotiated}}]); connection({call, From}, negotiated_protocol, #state{negotiated_protocol = SelectedProtocol} = State, _) -> - hibernate_after(connection, State, + hibernate_after(?FUNCTION_NAME, State, [{reply, From, {ok, SelectedProtocol}}]); connection({call, From}, Msg, State, Connection) -> - handle_call(Msg, From, connection, State, Connection); + handle_call(Msg, From, ?FUNCTION_NAME, State, Connection); connection(info, Msg, State, _) -> - handle_info(Msg, connection, State); + handle_info(Msg, ?FUNCTION_NAME, State); connection(internal, {recv, _}, State, Connection) -> - Connection:passive_receive(State, connection); + Connection:passive_receive(State, ?FUNCTION_NAME); connection(Type, Msg, State, Connection) -> - handle_common_event(Type, Msg, connection, State, Connection). + handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection). %%-------------------------------------------------------------------- -spec downgrade(gen_statem:event_type(), term(), @@ -800,7 +800,7 @@ downgrade(timeout, downgrade, #state{downgrade = {_, From}} = State, _) -> gen_statem:reply(From, {error, timeout}), {stop, normal, State}; downgrade(Type, Event, State, Connection) -> - handle_common_event(Type, Event, downgrade, State, Connection). + handle_common_event(Type, Event, ?FUNCTION_NAME, State, Connection). %%-------------------------------------------------------------------- %% Event handling functions called by state functions to handle -- cgit v1.2.3 From 3da1637b5ec4f24787d473fa3031bed44958136e Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Fri, 6 Oct 2017 17:24:16 +0200 Subject: ssl: Extend hostname check to fallback to checking IP-address If no SNI is available and the hostname is an IP-address also check for IP-address match. This check is not as good as a DNS hostname check and certificates using IP-address are not recommended. --- lib/ssl/src/ssl_connection.erl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib/ssl/src/ssl_connection.erl') diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index c6f67eb696..099b5f8ffa 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -496,12 +496,13 @@ certify(internal, #certificate{}, certify(internal, #certificate{} = Cert, #state{negotiated_version = Version, role = Role, + host = Host, cert_db = CertDbHandle, cert_db_ref = CertDbRef, crl_db = CRLDbInfo, ssl_options = Opts} = State, Connection) -> case ssl_handshake:certify(Cert, CertDbHandle, CertDbRef, - Opts, CRLDbInfo, Role) of + Opts, CRLDbInfo, Role, Host) of {PeerCert, PublicKeyInfo} -> handle_peer_cert(Role, PeerCert, PublicKeyInfo, State#state{client_certificate_requested = false}, Connection); -- cgit v1.2.3 From 138e63eb7eb2248ff3821d6dda4a64b0479b480c Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Fri, 6 Oct 2017 17:26:43 +0200 Subject: ssl: Sessions must be registered with SNI if exists --- lib/ssl/src/ssl_connection.erl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'lib/ssl/src/ssl_connection.erl') diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index 099b5f8ffa..31e7738b4d 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -702,6 +702,7 @@ cipher(internal, #finished{verify_data = Data} = Finished, expecting_finished = true, session = #session{master_secret = MasterSecret} = Session0, + ssl_options = SslOpts, connection_states = ConnectionStates0, tls_handshake_history = Handshake0} = State, Connection) -> case ssl_handshake:verify_connection(ssl:tls_version(Version), Finished, @@ -709,7 +710,7 @@ cipher(internal, #finished{verify_data = Data} = Finished, get_current_prf(ConnectionStates0, read), MasterSecret, Handshake0) of verified -> - Session = register_session(Role, Host, Port, Session0), + Session = register_session(Role, host_id(Role, Host, SslOpts), Port, Session0), cipher_role(Role, Data, Session, State#state{expecting_finished = false}, Connection); #alert{} = Alert -> @@ -2097,6 +2098,11 @@ register_session(server, _, Port, #session{is_resumable = new} = Session0) -> register_session(_, _, _, Session) -> Session. %% Already registered +host_id(client, _Host, #ssl_options{server_name_indication = Hostname}) when is_list(Hostname) -> + Hostname; +host_id(_, Host, _) -> + Host. + handle_new_session(NewId, CipherSuite, Compression, #state{session = Session0, protocol_cb = Connection} = State0) -> -- cgit v1.2.3