If a passive receive was ongoing during a renegotiation the process

evaluating ssl:recv could be left hanging for ever.

2 files changed, 64 insertions, 34 deletions

diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl
index b187d038a1..d4807704b2 100644
--- a/lib/ssl/src/ssl_connection.erl
+++ b/lib/ssl/src/ssl_connection.erl
@@ -87,10 +87,10 @@
           bytes_to_read,       % integer(), # bytes to read in passive mode
           user_data_buffer,    % binary()
 	  log_alert,           % boolean() 
-	  renegotiation,        % {boolean(), From | internal | peer}
-	  recv_during_renegotiation,  %boolean() 
-	  send_queue,           % queue()
-	  terminated = false,   %
+	  renegotiation,       % {boolean(), From | internal | peer}
+	  recv_from,           %
+	  send_queue,          % queue()
+	  terminated = false,  %
 	  allow_renegotiate = true
 	 }).
 
@@ -720,9 +720,7 @@ connection(#client_hello{}, #state{role = server, allow_renegotiate = false,
     {BinMsg, ConnectionStates} =
 	encode_alert(Alert, Version, ConnectionStates0),
     Transport:send(Socket, BinMsg),
-    {Record, State} = next_record(State0#state{connection_states = 
-     						   ConnectionStates}),
-    next_state(connection, connection, Record, State);
+    next_state_connection(connection, State0#state{connection_states = ConnectionStates});
   
 connection(timeout, State) ->
     {next_state, connection, State, hibernate};
@@ -814,14 +812,12 @@ handle_sync_event({shutdown, How0}, _, StateName,
     end;
     
 handle_sync_event({recv, N}, From, connection = StateName, State0) ->
-    passive_receive(State0#state{bytes_to_read = N, from = From}, StateName);
+    passive_receive(State0#state{bytes_to_read = N, recv_from = From}, StateName);
 
 %% Doing renegotiate wait with handling request until renegotiate is
-%% finished. Will be handled by next_state_connection/2.
+%% finished. Will be handled by next_state_is_connection/2.
 handle_sync_event({recv, N}, From, StateName, State) ->
-    {next_state, StateName,
-     State#state{bytes_to_read = N, from = From,
-                 recv_during_renegotiation = true},
+    {next_state, StateName, State#state{bytes_to_read = N, recv_from = From},
      get_timeout(State)};
 
 handle_sync_event({new_user, User}, _From, StateName, 
@@ -1689,7 +1685,7 @@ passive_receive(State0 = #state{user_data_buffer = Buffer}, StateName) ->
 read_application_data(Data, #state{user_application = {_Mon, Pid},
                               socket_options = SOpts,
                               bytes_to_read = BytesToRead,
-                              from = From,
+                              recv_from = From,
                               user_data_buffer = Buffer0} = State0) ->
     Buffer1 = if 
 		  Buffer0 =:= <<>> -> Data;
@@ -1700,7 +1696,7 @@ read_application_data(Data, #state{user_application = {_Mon, Pid},
 	{ok, ClientData, Buffer} -> % Send data
 	    SocketOpt = deliver_app_data(SOpts, ClientData, Pid, From),
 	    State = State0#state{user_data_buffer = Buffer,
-				 from = undefined,
+				 recv_from = undefined,
 				 bytes_to_read = 0,
 				 socket_options = SocketOpt 
 				},
@@ -2000,10 +1996,10 @@ next_state_connection(StateName, #state{send_queue = Queue0,
 %% premaster_secret and public_key_info (only needed during handshake)
 %% to reduce memory foot print of a connection.
 next_state_is_connection(_, State = 
-		      #state{recv_during_renegotiation = true, socket_options =     
-			     #socket_options{active = false}}) ->
-    passive_receive(State#state{recv_during_renegotiation = false,
-				premaster_secret = undefined,
+		      #state{recv_from = From,
+			     socket_options =
+			     #socket_options{active = false}}) when From =/= undefined ->
+    passive_receive(State#state{premaster_secret = undefined,
 				public_key_info = undefined,
 				tls_handshake_hashes = {<<>>, <<>>}}, connection);
 
@@ -2065,7 +2061,7 @@ initial_state(Role, Host, Port, Socket, {SSLOptions, SocketOptions}, User,
 	   log_alert = true,
 	   session_cache_cb = SessionCacheCb,
 	   renegotiation = {false, first},
-	   recv_during_renegotiation = false,
+	   recv_from = undefined,
 	   send_queue = queue:new()
 	  }.
 
diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl
index fc56ceb17e..228ec9e294 100644
--- a/lib/ssl/test/ssl_basic_SUITE.erl
+++ b/lib/ssl/test/ssl_basic_SUITE.erl
@@ -257,7 +257,8 @@ all() ->
      %%different_ca_peer_sign,
      no_reuses_session_server_restart_new_cert,
      no_reuses_session_server_restart_new_cert_file, reuseaddr,
-     hibernate, connect_twice, renegotiate_dos_mitigate,
+     hibernate, connect_twice, renegotiate_dos_mitigate_active,
+     renegotiate_dos_mitigate_passive,
      tcp_error_propagation_in_active_mode
     ].
 
@@ -1565,14 +1566,14 @@ eoptions(Config) when is_list(Config) ->
 		{cacertfile, ""}, 
 		{dhfile,'dh.pem' },
 		{ciphers, [{foo, bar, sha, ignore}]},
-		{reuse_session, foo}, 
-		{reuse_sessions, 0}, 
+		{reuse_session, foo},
+		{reuse_sessions, 0},
 		{renegotiate_at, "10"},
-		{debug, 1}, 
+		{debug, 1},
 		{mode, depech},
-		{packet, 8.0}, 
-		{packet_size, "2"}, 
-		{header, a}, 
+		{packet, 8.0},
+		{packet_size, "2"},
+		{header, a},
 		{active, trice},
 		{key, 'key.pem' }],
 
@@ -3692,25 +3693,57 @@ connect_twice(Config) when is_list(Config) ->
     ssl_test_lib:close(Client1).
 
 %%--------------------------------------------------------------------
-renegotiate_dos_mitigate(doc) -> 
+renegotiate_dos_mitigate_active(doc) ->
     ["Mitigate DOS computational attack by not allowing client to renegotiate many times in a row",
     "immediately after each other"];
 
-renegotiate_dos_mitigate(suite) -> 
+renegotiate_dos_mitigate_active(suite) ->
     [];
 
-renegotiate_dos_mitigate(Config) when is_list(Config) -> 
-    ServerOpts = ?config(server_opts, Config),  
-    ClientOpts = ?config(client_opts, Config),  
+renegotiate_dos_mitigate_active(Config) when is_list(Config) ->
+    ServerOpts = ?config(server_opts, Config),
+    ClientOpts = ?config(client_opts, Config),
 
     {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
-    
-    Server = 
-	ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, 
+
+    Server =
+	ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
 				   {from, self()},
 				   {mfa, {?MODULE, send_recv_result_active, []}},
 				   {options, [ServerOpts]}]),
     Port = ssl_test_lib:inet_port(Server),
+
+    Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port},
+					{host, Hostname},
+					{from, self()},
+					{mfa, {?MODULE,
+					       renegotiate_immediately, []}},
+					{options, ClientOpts}]),
+
+    ssl_test_lib:check_result(Client, ok, Server, ok),
+    ssl_test_lib:close(Server),
+    ssl_test_lib:close(Client).
+
+%%--------------------------------------------------------------------
+renegotiate_dos_mitigate_passive(doc) ->
+    ["Mitigate DOS computational attack by not allowing client to renegotiate many times in a row",
+    "immediately after each other"];
+
+renegotiate_dos_mitigate_passive(suite) ->
+    [];
+
+renegotiate_dos_mitigate_passive(Config) when is_list(Config) ->
+    ServerOpts = ?config(server_opts, Config),
+    ClientOpts = ?config(client_opts, Config),
+
+    {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+    Server =
+	ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+				   {from, self()},
+				   {mfa, {?MODULE, send_recv_result, []}},
+				   {options, [{active, false} | ServerOpts]}]),
+    Port = ssl_test_lib:inet_port(Server),
  
     Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port},
 					{host, Hostname},
@@ -3723,6 +3756,7 @@ renegotiate_dos_mitigate(Config) when is_list(Config) ->
     ssl_test_lib:close(Server),
     ssl_test_lib:close(Client).
 
+%%--------------------------------------------------------------------
 tcp_error_propagation_in_active_mode(doc) ->
     ["Test that process recives {ssl_error, Socket, closed} when tcp error ocurres"];
 tcp_error_propagation_in_active_mode(Config) when is_list(Config) ->