From 0d6fb3f9c0740c74b0b9381d38f0360b9c81160d Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Wed, 12 Dec 2018 15:39:24 +0100 Subject: ssl: Fix downgrade Both test case and code needed updates to work as intended. Code needed update due to new tls_sender process and the test case gave false positive reusult erarlier probably due to beeing to sloopy in order to avoid timeouts. --- lib/ssl/src/ssl_connection.erl | 40 ++++++++------------------------------ lib/ssl/src/ssl_connection.hrl | 2 +- lib/ssl/src/tls_connection.erl | 44 +++++++++++++++++++++++++++++++++++++++--- lib/ssl/src/tls_sender.erl | 20 ++++++++++++++++++- 4 files changed, 69 insertions(+), 37 deletions(-) (limited to 'lib/ssl/src') diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index 12e5b001b0..14162ab07a 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -392,10 +392,12 @@ handle_alert(#alert{level = ?FATAL} = Alert, StateName, stop(normal, State); handle_alert(#alert{level = ?WARNING, description = ?CLOSE_NOTIFY} = Alert, - StateName, State) -> + downgrade= StateName, State) -> + {next_state, StateName, State, [{next_event, internal, Alert}]}; +handle_alert(#alert{level = ?WARNING, description = ?CLOSE_NOTIFY} = Alert, + StateName, State) -> handle_normal_shutdown(Alert, StateName, State), stop({shutdown, peer_close}, State); - handle_alert(#alert{level = ?WARNING, description = ?NO_RENEGOTIATION} = Alert, StateName, #state{static_env = #static_env{role = Role, protocol_cb = Connection}, @@ -1115,23 +1117,6 @@ connection(Type, Msg, State, Connection) -> #state{}, tls_connection | dtls_connection) -> gen_statem:state_function_result(). %%-------------------------------------------------------------------- -downgrade(internal, #alert{description = ?CLOSE_NOTIFY}, - #state{static_env = #static_env{transport_cb = Transport, - socket = Socket}, - downgrade = {Pid, From}} = State, _) -> - tls_socket:setopts(Transport, Socket, [{active, false}, {packet, 0}, {mode, binary}]), - Transport:controlling_process(Socket, Pid), - gen_statem:reply(From, {ok, Socket}), - stop(normal, State); -downgrade(timeout, downgrade, #state{downgrade = {_, From}} = State, _) -> - gen_statem:reply(From, {error, timeout}), - stop(normal, State); -downgrade(info, {CloseTag, Socket}, - #state{static_env = #static_env{socket = Socket, - close_tag = CloseTag}, downgrade = {_, From}} = - State, _) -> - gen_statem:reply(From, {error, CloseTag}), - stop(normal, State); downgrade(Type, Event, State, Connection) -> handle_common_event(Type, Event, ?FUNCTION_NAME, State, Connection). @@ -1175,15 +1160,6 @@ handle_common_event(_Type, Msg, StateName, #state{negotiated_version = Version} handle_call({application_data, _Data}, _, _, _, _) -> %% In renegotiation priorities handshake, send data when handshake is finished {keep_state_and_data, [postpone]}; -handle_call({close, {Pid, Timeout}}, From, StateName, State0, Connection) when is_pid(Pid) -> - %% terminate will send close alert to peer - State = State0#state{downgrade = {Pid, From}}, - Connection:terminate(downgrade, StateName, State), - %% User downgrades connection - %% When downgrading an TLS connection to a transport connection - %% we must recive the close alert from the peer before releasing the - %% transport socket. - {next_state, downgrade, State#state{terminated = true}, [{timeout, Timeout, downgrade}]}; handle_call({close, _} = Close, From, StateName, State, _Connection) -> %% Run terminate before returning so that the reuseaddr %% inet-option works properly @@ -1375,10 +1351,10 @@ terminate({shutdown, own_alert}, _StateName, #state{ _ -> Connection:close({timeout, ?DEFAULT_TIMEOUT}, Socket, Transport, undefined, undefined) end; -terminate(downgrade = Reason, connection, #state{static_env = #static_env{protocol_cb = Connection, - transport_cb = Transport, - socket = Socket} - } = State) -> +terminate({shutdown, downgrade = Reason}, downgrade, #state{static_env = #static_env{protocol_cb = Connection, + transport_cb = Transport, + socket = Socket} + } = State) -> handle_trusted_certs_db(State), Connection:close(Reason, Socket, Transport, undefined, undefined); terminate(Reason, connection, #state{static_env = #static_env{ diff --git a/lib/ssl/src/ssl_connection.hrl b/lib/ssl/src/ssl_connection.hrl index 8ddc3199e1..2f4dfefdda 100644 --- a/lib/ssl/src/ssl_connection.hrl +++ b/lib/ssl/src/ssl_connection.hrl @@ -59,7 +59,7 @@ socket_options :: #socket_options{}, session :: #session{} | secret_printout(), allow_renegotiate = true ::boolean(), - terminated = false ::boolean(), + terminated = false ::boolean() | closed, negotiated_version :: ssl_record:ssl_version() | 'undefined', bytes_to_read :: undefined | integer(), %% bytes to read in passive mode downgrade, diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl index 5864b86cbc..9d0a9085e5 100644 --- a/lib/ssl/src/tls_connection.erl +++ b/lib/ssl/src/tls_connection.erl @@ -17,7 +17,6 @@ %% %% %CopyrightEnd% %% - %% %%---------------------------------------------------------------------- %% Purpose: Handles an ssl connection, e.i. both the setup @@ -590,6 +589,30 @@ connection({call, From}, {user_renegotiate, WriteState}, #state{connection_states = ConnectionStates} = State) -> {next_state, ?FUNCTION_NAME, State#state{connection_states = ConnectionStates#{current_write => WriteState}}, [{next_event,{call, From}, renegotiate}]}; +connection({call, From}, + {close, {Pid, _Timeout}}, + #state{terminated = closed} = State) -> + {next_state, downgrade, State#state{terminated = true, downgrade = {Pid, From}}, + [{next_event, internal, ?ALERT_REC(?WARNING, ?CLOSE_NOTIFY)}]}; +connection({call, From}, + {close,{Pid, Timeout}}, + #state{connection_states = ConnectionStates, + protocol_specific = #{sender := Sender} + } = State0) -> + case tls_sender:downgrade(Sender, Timeout) of + {ok, Write} -> + %% User downgrades connection + %% When downgrading an TLS connection to a transport connection + %% we must recive the close alert from the peer before releasing the + %% transport socket. + State = send_alert(?ALERT_REC(?WARNING, ?CLOSE_NOTIFY), + State0#state{connection_states = + ConnectionStates#{current_write => Write}}), + {next_state, downgrade, State#state{downgrade = {Pid, From}, + terminated = true}, [{timeout, Timeout, downgrade}]}; + {error, timeout} -> + {stop_and_reply, {shutdown, downgrade_fail}, [{reply, From, {error, timeout}}]} + end; connection(internal, #hello_request{}, #state{static_env = #static_env{role = client, host = Host, @@ -638,9 +661,24 @@ connection(Type, Event, State) -> -spec downgrade(gen_statem:event_type(), term(), #state{}) -> gen_statem:state_function_result(). %%-------------------------------------------------------------------- +downgrade(internal, #alert{description = ?CLOSE_NOTIFY}, + #state{static_env = #static_env{transport_cb = Transport, + socket = Socket}, + downgrade = {Pid, From}} = State) -> + tls_socket:setopts(Transport, Socket, [{active, false}, {packet, 0}, {mode, binary}]), + Transport:controlling_process(Socket, Pid), + {stop_and_reply, {shutdown, downgrade},[{reply, From, {ok, Socket}}], State}; +downgrade(timeout, downgrade, #state{downgrade = {_, From}} = State) -> + {stop_and_reply, {shutdown, normal},[{reply, From, {error, timeout}}], State}; +downgrade(info, {CloseTag, Socket}, + #state{static_env = #static_env{socket = Socket, + close_tag = CloseTag}, downgrade = {_, From}} = + State) -> + {stop_and_reply, {shutdown, normal},[{reply, From, {error, CloseTag}}], State}; +downgrade(info, Info, State) -> + handle_info(Info, ?FUNCTION_NAME, State); downgrade(Type, Event, State) -> - ssl_connection:?FUNCTION_NAME(Type, Event, State, ?MODULE). - + ssl_connection:?FUNCTION_NAME(Type, Event, State, ?MODULE). %-------------------------------------------------------------------- %% gen_statem callbacks %%-------------------------------------------------------------------- diff --git a/lib/ssl/src/tls_sender.erl b/lib/ssl/src/tls_sender.erl index ee2b7be4f4..7520832f39 100644 --- a/lib/ssl/src/tls_sender.erl +++ b/lib/ssl/src/tls_sender.erl @@ -29,7 +29,7 @@ %% API -export([start/0, start/1, initialize/2, send_data/2, send_alert/2, - send_and_ack_alert/2, setopts/2, renegotiate/1, + send_and_ack_alert/2, setopts/2, renegotiate/1, downgrade/2, update_connection_state/3, dist_tls_socket/1, dist_handshake_complete/3]). %% gen_statem callbacks @@ -125,6 +125,21 @@ renegotiate(Pid) -> %%-------------------------------------------------------------------- update_connection_state(Pid, NewState, Version) -> gen_statem:cast(Pid, {new_write, NewState, Version}). + +%%-------------------------------------------------------------------- +-spec downgrade(pid(), integer()) -> {ok, ssl_record:connection_state()} + | {error, timeout}. +%% Description: So TLS connection process can synchronize the +%% encryption state to be used when sending application data. +%%-------------------------------------------------------------------- +downgrade(Pid, Timeout) -> + try gen_statem:call(Pid, downgrade, Timeout) of + Result -> + Result + catch + _:_ -> + {error, timeout} + end. %%-------------------------------------------------------------------- -spec dist_handshake_complete(pid(), node(), term()) -> ok. %% Description: Erlang distribution callback @@ -239,6 +254,9 @@ connection({call, From}, {ack_alert, #alert{} = Alert}, StateData0) -> StateData = send_tls_alert(Alert, StateData0), {next_state, ?FUNCTION_NAME, StateData, [{reply,From,ok}]}; +connection({call, From}, downgrade, #data{connection_states = + #{current_write := Write}} = StateData) -> + {next_state, death_row, StateData, [{reply,From, {ok, Write}}]}; connection(internal, {application_packets, From, Data}, StateData) -> send_application_data(Data, From, ?FUNCTION_NAME, StateData); %% -- cgit v1.2.3