aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorIngela Anderton Andin <[email protected]>2018-08-23 14:29:36 +0200
committerIngela Anderton Andin <[email protected]>2018-08-27 15:22:45 +0200
commita508428df8af14437f1a4766ec68294d1d736915 (patch)
tree4c13500b01060c8f52f7d0d32d7352f9f9ebb8ff
parent0078ebf6c5311edc1a07be71fb7a127a175a60fa (diff)
downloadotp-a508428df8af14437f1a4766ec68294d1d736915.tar.gz
otp-a508428df8af14437f1a4766ec68294d1d736915.tar.bz2
otp-a508428df8af14437f1a4766ec68294d1d736915.zip
ssl: Improve close handling
We want to make sure that the sender process that may get stuck in prim_inet:send will die if the tls_connection process is terminated. And we also like to make sure that it terminates as gracefully as possible. So when the tls_connection process dies it spawns a killer process that will brutaly kill the sender if it is unresponsive and does not terminate due to its monitor of the tls_connetion process triggering. When the sender process also acts as distribution controller it may also have other processess that it is linked with that it should bring down or that could bring the connection down.
-rw-r--r--lib/ssl/src/ssl_connection.erl47
-rw-r--r--lib/ssl/src/tls_connection.erl44
-rw-r--r--lib/ssl/src/tls_sender.erl50
3 files changed, 72 insertions, 69 deletions
diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl
index b17b7fbffe..5769939ee5 100644
--- a/lib/ssl/src/ssl_connection.erl
+++ b/lib/ssl/src/ssl_connection.erl
@@ -64,7 +64,7 @@
%% General gen_statem state functions with extra callback argument
%% to determine if it is an SSL/TLS or DTLS gen_statem machine
-export([init/4, error/4, hello/4, user_hello/4, abbreviated/4, certify/4, cipher/4,
- connection/4, death_row/4, death_row/2, downgrade/4]).
+ connection/4, downgrade/4]).
%% gen_statem callbacks
-export([terminate/3, format_status/2]).
@@ -511,7 +511,7 @@ dist_app_data(ClientData, #state{erl_dist_data = #{dist_handle := DHandle,
_ -> %% We have more data
read_application_data(<<>>, State)
catch error:_ ->
- death_row(State, disconnect)
+ stop(State, disconnect)
end.
merge_dist_data(<<>>, ClientData) ->
@@ -1081,29 +1081,6 @@ connection(Type, Msg, State, Connection) ->
handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection).
%%--------------------------------------------------------------------
--spec death_row(gen_statem:event_type(), term(),
- #state{}, tls_connection | dtls_connection) ->
- gen_statem:state_function_result().
-%%--------------------------------------------------------------------
-%% We just wait for the owner to die which triggers the monitor,
-%% or the socket may die too
-death_row(info, {'DOWN', MonitorRef, _, _, Reason},
- #state{user_application={MonitorRef,_Pid}},_) ->
- {stop, {shutdown, Reason}};
-death_row(info, {'EXIT', Socket, Reason}, #state{socket = Socket}, _) ->
- {stop, {shutdown, Reason}};
-death_row(state_timeout, Reason, _State, _Connection) ->
- {stop, {shutdown,Reason}};
-death_row(_Type, _Msg, _State, _Connection) ->
- %% Waste all other events
- keep_state_and_data.
-
-%% State entry function
-death_row(State, Reason) ->
- {next_state, death_row, State,
- [{state_timeout, 5000, Reason}]}.
-
-%%--------------------------------------------------------------------
-spec downgrade(gen_statem:event_type(), term(),
#state{}, tls_connection | dtls_connection) ->
gen_statem:state_function_result().
@@ -2727,26 +2704,12 @@ new_emulated([], EmOpts) ->
EmOpts;
new_emulated(NewEmOpts, _) ->
NewEmOpts.
-%%---------------Erlang distribution --------------------------------------
-%% When acting as distribution controller map the exit reason
-%% to follow the documented nodedown_reason for net_kernel
+
stop(Reason, State) ->
- {stop, erl_dist_stop_reason(Reason, State), State}.
+ {stop, Reason, State}.
stop_and_reply(Reason, Replies, State) ->
- {stop_and_reply, erl_dist_stop_reason(Reason, State), Replies, State}.
-
-erl_dist_stop_reason(
- Reason, #state{ssl_options = #ssl_options{erl_dist = true}}) ->
- case Reason of
- normal ->
- %% We can not exit with normal since that will not bring
- %% down the rest of the distribution processes
- {shutdown, normal};
- _ -> Reason
- end;
-erl_dist_stop_reason(Reason, _State) ->
- Reason.
+ {stop_and_reply, Reason, Replies, State}.
is_dist_up(#{dist_handle := Handle}) when Handle =/= undefined ->
true;
diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl
index 8277569281..6c7511d2b3 100644
--- a/lib/ssl/src/tls_connection.erl
+++ b/lib/ssl/src/tls_connection.erl
@@ -65,7 +65,7 @@
%% gen_statem state functions
-export([init/3, error/3, downgrade/3, %% Initiation and take down states
hello/3, user_hello/3, certify/3, cipher/3, abbreviated/3, %% Handshake states
- connection/3, death_row/3]).
+ connection/3]).
%% gen_statem callbacks
-export([callback_mode/0, terminate/3, code_change/4, format_status/2]).
@@ -135,7 +135,7 @@ init([Role, Sender, Host, Port, Socket, {SslOpts, _, _} = Options, User, CbInfo
gen_statem:enter_loop(?MODULE, [], error, EState)
end.
-pids(#state{protocol_specific = #{sender := {_, Sender}}}) ->
+pids(#state{protocol_specific = #{sender := Sender}}) ->
[self(), Sender].
%%====================================================================
@@ -307,7 +307,7 @@ queue_change_cipher(Msg, #state{negotiated_version = Version,
State0#state{connection_states = ConnectionStates,
flight_buffer = Flight0 ++ [BinChangeCipher]}.
-reinit(#state{protocol_specific = #{sender := {_,Sender}},
+reinit(#state{protocol_specific = #{sender := Sender},
negotiated_version = Version,
connection_states = #{current_write := Write}} = State) ->
tls_sender:update_connection_state(Sender, Write, Version),
@@ -353,7 +353,7 @@ send_alert(Alert, #state{negotiated_version = Version,
Connection:send(Transport, Socket, BinMsg),
StateData0#state{connection_states = ConnectionStates}.
-send_alert_in_connection(Alert, #state{protocol_specific = #{sender := {_, Sender}}}) ->
+send_alert_in_connection(Alert, #state{protocol_specific = #{sender := Sender}}) ->
tls_sender:send_alert(Sender, Alert).
%% User closes or recursive call!
@@ -597,7 +597,7 @@ connection(internal, #hello_request{},
connection(internal, #client_hello{} = Hello,
#state{role = server, allow_renegotiate = true, connection_states = CS,
%%protocol_cb = Connection,
- protocol_specific = #{sender := {_, Sender}}
+ protocol_specific = #{sender := Sender}
} = State0) ->
%% Mitigate Computational DoS attack
%% http://www.educatedguesswork.org/2011/10/ssltls_and_computational_dos.html
@@ -622,13 +622,6 @@ connection(Type, Event, State) ->
ssl_connection:?FUNCTION_NAME(Type, Event, State, ?MODULE).
%%--------------------------------------------------------------------
--spec death_row(gen_statem:event_type(), term(), #state{}) ->
- gen_statem:state_function_result().
-%%--------------------------------------------------------------------
-death_row(Type, Event, State) ->
- ssl_connection:death_row(Type, Event, State, ?MODULE).
-
-%%--------------------------------------------------------------------
-spec downgrade(gen_statem:event_type(), term(), #state{}) ->
gen_statem:state_function_result().
%%--------------------------------------------------------------------
@@ -642,6 +635,7 @@ callback_mode() ->
state_functions.
terminate(Reason, StateName, State) ->
+ ensure_sender_terminate(Reason, State),
catch ssl_connection:terminate(Reason, StateName, State).
format_status(Type, Data) ->
@@ -668,7 +662,6 @@ initial_state(Role, Sender, Host, Port, Socket, {SSLOptions, SocketOptions, Trac
end,
UserMonitor = erlang:monitor(process, User),
- SendMonitor = erlang:monitor(process, Sender),
#state{socket_options = SocketOptions,
ssl_options = SSLOptions,
@@ -693,7 +686,7 @@ initial_state(Role, Sender, Host, Port, Socket, {SSLOptions, SocketOptions, Trac
protocol_cb = ?MODULE,
tracker = Tracker,
flight_buffer = [],
- protocol_specific = #{sender => {SendMonitor, Sender}}
+ protocol_specific = #{sender => Sender}
}.
erl_dist_data(true) ->
@@ -711,7 +704,7 @@ initialize_tls_sender(#state{role = Role,
negotiated_version = Version,
ssl_options = #ssl_options{renegotiate_at = RenegotiateAt},
connection_states = #{current_write := ConnectionWriteState},
- protocol_specific = #{sender := {_, Sender}}}) ->
+ protocol_specific = #{sender := Sender}}) ->
Init = #{current_write => ConnectionWriteState,
role => Role,
socket => Socket,
@@ -794,11 +787,9 @@ handle_info({CloseTag, Socket}, StateName,
%% and then receive the final message.
next_event(StateName, no_record, State)
end;
-handle_info({'DOWN', Mon, _, _, _}, _, #state{ssl_options = #ssl_options{erl_dist = true},
- protocol_specific = #{sender:= {Mon, _}}} = State) ->
- ssl_connection:death_row(State, disconnect);
-handle_info({'DOWN', Mon, _, _, Reason}, _, #state{protocol_specific = #{sender:= {Mon, _}}} = State) ->
- {stop, {shudown, sender_died, Reason}, State};
+handle_info({'EXIT', Pid, Reason}, _,
+ #state{protocol_specific = Pid} = State) ->
+ {stop, {shutdown, sender_died, Reason}, State};
handle_info(Msg, StateName, State) ->
ssl_connection:StateName(info, Msg, State, ?MODULE).
@@ -888,3 +879,16 @@ assert_buffer_sanity(Bin, _) ->
throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE,
malformed_handshake_data))
end.
+
+ensure_sender_terminate(downgrade, _) ->
+ ok; %% Do not terminate sender during downgrade phase
+ensure_sender_terminate(_, #state{protocol_specific = #{sender := Sender}}) ->
+ %% Make sure TLS sender dies when connection process is terminated normally
+ %% This is needed if the tls_sender is blocked in prim_inet:send
+ Kill = fun() ->
+ receive
+ after 5000 ->
+ catch (exit(Sender, kill))
+ end
+ end,
+ spawn(Kill).
diff --git a/lib/ssl/src/tls_sender.erl b/lib/ssl/src/tls_sender.erl
index 2746d89048..007fd345dd 100644
--- a/lib/ssl/src/tls_sender.erl
+++ b/lib/ssl/src/tls_sender.erl
@@ -25,6 +25,7 @@
-include("ssl_internal.hrl").
-include("ssl_alert.hrl").
-include("ssl_handshake.hrl").
+-include("ssl_api.hrl").
%% API
-export([start/0, start/1, initialize/2, send_data/2, send_alert/2, renegotiate/1,
@@ -32,7 +33,7 @@
%% gen_statem callbacks
-export([callback_mode/0, init/1, terminate/3, code_change/4]).
--export([init/3, connection/3, handshake/3]).
+-export([init/3, connection/3, handshake/3, death_row/3]).
-define(SERVER, ?MODULE).
@@ -53,6 +54,7 @@
%%%===================================================================
%%% API
%%%===================================================================
+%%--------------------------------------------------------------------
-spec start() -> {ok, Pid :: pid()} |
ignore |
{error, Error :: term()}.
@@ -65,7 +67,7 @@
%% same process is sending and receiving
%%--------------------------------------------------------------------
start() ->
- gen_statem:start(?MODULE, [], []).
+ gen_statem:start_link(?MODULE, [], []).
start(SpawnOpts) ->
gen_statem:start_link(?MODULE, [], SpawnOpts).
@@ -77,10 +79,19 @@ start(SpawnOpts) ->
initialize(Pid, InitMsg) ->
gen_statem:call(Pid, {self(), InitMsg}).
+%%--------------------------------------------------------------------
+-spec send_data(pid(), iodata()) -> ok.
+%% Description: Send application data
+%%--------------------------------------------------------------------
send_data(Pid, AppData) ->
%% Needs error handling for external API
call(Pid, {application_data, AppData}).
+%%--------------------------------------------------------------------
+-spec send_alert(pid(), #alert{}) -> _.
+%% Description: TLS connection process wants to end an Alert
+%% in the connection state.
+%%--------------------------------------------------------------------
send_alert(Pid, Alert) ->
gen_statem:cast(Pid, Alert).
@@ -99,10 +110,16 @@ renegotiate(Pid) ->
%%--------------------------------------------------------------------
update_connection_state(Pid, NewState, Version) ->
gen_statem:cast(Pid, {new_write, NewState, Version}).
-
+%%--------------------------------------------------------------------
+-spec dist_handshake_complete(pid(), node(), term()) -> ok.
+%% Description: Erlang distribution callback
+%%--------------------------------------------------------------------
dist_handshake_complete(ConnectionPid, Node, DHandle) ->
gen_statem:call(ConnectionPid, {dist_handshake_complete, Node, DHandle}).
-
+%%--------------------------------------------------------------------
+-spec dist_tls_socket(pid()) -> {ok, #sslsocket{}}.
+%% Description: To enable distribution startup to get a proper "#sslsocket{}"
+%%--------------------------------------------------------------------
dist_tls_socket(Pid) ->
gen_statem:call(Pid, dist_get_tls_socket).
@@ -120,6 +137,9 @@ callback_mode() ->
gen_statem:init_result(atom()).
%%--------------------------------------------------------------------
init(_) ->
+ %% Note: Should not trap exits so that this process
+ %% will be terminated if tls_connection process is
+ %% killed brutally
{ok, init, #data{}}.
%%--------------------------------------------------------------------
@@ -233,6 +253,18 @@ handshake(info, Msg, StateData) ->
handle_info(Msg, ?FUNCTION_NAME, StateData).
%%--------------------------------------------------------------------
+-spec death_row(gen_statem:event_type(),
+ Msg :: term(),
+ StateData :: term()) ->
+ gen_statem:event_handler_result(atom()).
+%%--------------------------------------------------------------------
+death_row(state_timeout, Reason, _State) ->
+ {stop, {shutdown, Reason}};
+death_row(_Type, _Msg, _State) ->
+ %% Waste all other events
+ keep_state_and_data.
+
+%%--------------------------------------------------------------------
-spec terminate(Reason :: term(), State :: term(), Data :: term()) ->
any().
%%--------------------------------------------------------------------
@@ -254,8 +286,12 @@ code_change(_OldVsn, State, Data, _Extra) ->
%%% Internal functions
%%%===================================================================
handle_info({'DOWN', Monitor, _, _, Reason}, _,
+ #data{connection_monitor = Monitor,
+ dist_handle = Handle} = StateData) when Handle =/= undefined->
+ {next_state, death_row, StateData, [{state_timeout, 5000, Reason}]};
+handle_info({'DOWN', Monitor, _, _, _}, _,
#data{connection_monitor = Monitor} = StateData) ->
- {stop, {shutdown, Reason}, StateData};
+ {stop, normal, StateData};
handle_info(_,_,_) ->
{keep_state_and_data}.
@@ -290,8 +326,8 @@ send_application_data(Data, From, StateName,
case Connection:send(Transport, Socket, Msgs) of
ok when DistHandle =/= undefined ->
{next_state, StateName, StateData, []};
- Error when DistHandle =/= undefined ->
- ssl_connection:stop({shutdown, Error}, StateData);
+ Reason when DistHandle =/= undefined ->
+ {next_state, death_row, StateData, [{state_timeout, 5000, Reason}]};
ok ->
{next_state, StateName, StateData, [{reply, From, ok}]};
Result ->