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.

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}