aboutsummaryrefslogtreecommitdiffstats
path: root/lib
diff options
context:
space:
mode:
authorIngela Anderton Andin <[email protected]>2018-12-12 15:39:24 +0100
committerIngela Anderton Andin <[email protected]>2018-12-20 11:56:56 +0100
commit389762a796c255fe6a14d39da7b17911af65d253 (patch)
treea35470388d04874f21c2dcb8b121df249d1de55e /lib
parentc4594c28af456b009068a461017e893d1714576a (diff)
downloadotp-389762a796c255fe6a14d39da7b17911af65d253.tar.gz
otp-389762a796c255fe6a14d39da7b17911af65d253.tar.bz2
otp-389762a796c255fe6a14d39da7b17911af65d253.zip
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.
Diffstat (limited to 'lib')
-rw-r--r--lib/ssl/src/ssl_connection.erl40
-rw-r--r--lib/ssl/src/ssl_connection.hrl2
-rw-r--r--lib/ssl/src/tls_connection.erl44
-rw-r--r--lib/ssl/src/tls_sender.erl20
-rw-r--r--lib/ssl/test/ssl_basic_SUITE.erl39
5 files changed, 94 insertions, 51 deletions
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);
%%
diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl
index a00fa86b61..37fe83192e 100644
--- a/lib/ssl/test/ssl_basic_SUITE.erl
+++ b/lib/ssl/test/ssl_basic_SUITE.erl
@@ -2118,15 +2118,21 @@ tls_downgrade(Config) when is_list(Config) ->
Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
{from, self()},
- {mfa, {?MODULE, tls_downgrade_result, []}},
+ {mfa, {?MODULE, tls_downgrade_result, [self()]}},
{options, [{active, false} | ServerOpts]}]),
Port = ssl_test_lib:inet_port(Server),
Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port},
{host, Hostname},
{from, self()},
- {mfa, {?MODULE, tls_downgrade_result, []}},
+ {mfa, {?MODULE, tls_downgrade_result, [self()]}},
{options, [{active, false} |ClientOpts]}]),
+
+ ssl_test_lib:check_result(Server, ready, Client, ready),
+
+ Server ! go,
+ Client ! go,
+
ssl_test_lib:check_result(Server, ok, Client, ok),
ssl_test_lib:close(Server),
ssl_test_lib:close(Client).
@@ -5176,23 +5182,28 @@ connect_dist_c(S) ->
{ok, Test} = ssl:recv(S, 0, 10000),
ok.
-tls_downgrade_result(Socket) ->
+tls_downgrade_result(Socket, Pid) ->
ok = ssl_test_lib:send_recv_result(Socket),
+ Pid ! {self(), ready},
+ receive
+ go ->
+ ok
+ end,
case ssl:close(Socket, {self(), 10000}) of
{ok, TCPSocket} ->
- inet:setopts(TCPSocket, [{active, true}]),
+ inet:setopts(TCPSocket, [{active, true}]),
gen_tcp:send(TCPSocket, "Downgraded"),
- receive
- {tcp, TCPSocket, <<"Downgraded">>} ->
- ok;
- {tcp_closed, TCPSocket} ->
- ct:pal("Peer timed out, downgrade aborted"),
- ok;
- Other ->
- {error, Other}
- end;
+ receive
+ {tcp, TCPSocket, <<"Downgraded">>} ->
+ ok;
+ {tcp_closed, TCPSocket} ->
+ ct:fail("Peer timed out, downgrade aborted"),
+ ok;
+ Other ->
+ {error, Other}
+ end;
{error, timeout} ->
- ct:pal("Timed out, downgrade aborted"),
+ ct:fail("Timed out, downgrade aborted"),
ok;
Fail ->
{error, Fail}