ssl: Invalidate session on abrupt close during initial handshake

3 files changed, 228 insertions, 20 deletions

diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl
index 955841a7a4..6630e417f9 100644
--- a/lib/ssl/src/ssl_connection.erl
+++ b/lib/ssl/src/ssl_connection.erl
@@ -53,7 +53,8 @@
 %% Alert and close handling
 -export([handle_own_alert/4, handle_alert/3, 
 	 handle_normal_shutdown/3, 
-         handle_trusted_certs_db/1]).
+         handle_trusted_certs_db/1,
+         maybe_invalidate_session/6]).
 
 %% Data handling
 -export([read_application_data/2, internal_renegotiation/2]).
@@ -325,6 +326,7 @@ dist_handshake_complete(ConnectionPid, DHandle) ->
 prf(ConnectionPid, Secret, Label, Seed, WantedLength) ->
     call(ConnectionPid, {prf, Secret, Label, Seed, WantedLength}).
 
+
 %%====================================================================
 %% Alert and close handling
 %%====================================================================
@@ -442,6 +444,13 @@ handle_alert(#alert{level = ?WARNING} = Alert, StateName,
               Alert#alert{role = opposite_role(Role)}),
     Connection:next_event(StateName, no_record, State).
 
+maybe_invalidate_session(undefined,_, _, _, _, _) ->
+    ok;
+maybe_invalidate_session({3, 4},_, _, _, _, _) ->
+    ok;
+maybe_invalidate_session({3, N}, Type, Role, Host, Port, Session) when N < 4 ->
+    maybe_invalidate_session(Type, Role, Host, Port, Session).
+
 %%====================================================================
 %% Data handling
 %%====================================================================
@@ -1492,16 +1501,23 @@ handle_call(_,_,_,_,_) ->
 
 handle_info({ErrorTag, Socket, econnaborted}, StateName,  
 	    #state{static_env = #static_env{role = Role,
+                                            host = Host,
+                                            port = Port,
                                             socket = Socket,
                                             transport_cb = Transport,
                                             error_tag = ErrorTag,
                                             trackers = Trackers,
                                             protocol_cb = Connection},
-		   start_or_recv_from = StartFrom
-		  } = State)  when StateName =/= connection ->
+                   handshake_env = #handshake_env{renegotiation = Type},
+                   connection_env = #connection_env{negotiated_version = Version},
+                   session = Session,
+                   start_or_recv_from = StartFrom
+                  } = State)  when StateName =/= connection ->
+
+    maybe_invalidate_session(Version, Type, Role, Host, Port, Session),
     Pids = Connection:pids(State),
     alert_user(Pids, Transport, Trackers,Socket, 
-	       StartFrom, ?ALERT_REC(?FATAL, ?CLOSE_NOTIFY), Role, StateName, Connection),
+               StartFrom, ?ALERT_REC(?FATAL, ?CLOSE_NOTIFY), Role, StateName, Connection),
     {stop, {shutdown, normal}, State};
 
 handle_info({ErrorTag, Socket, Reason}, StateName, #state{static_env = #static_env{
@@ -3009,6 +3025,11 @@ log_alert(Level, Role, ProtocolName, StateName,  Alert) ->
                                     alert => Alert,
                                     alerter => peer}, Alert#alert.where).
 
+maybe_invalidate_session({false, first}, server = Role, Host, Port, Session) ->
+    invalidate_session(Role, Host, Port, Session);
+maybe_invalidate_session(_, _, _, _, _) ->
+    ok.
+
 invalidate_session(client, Host, Port, Session) ->
     ssl_manager:invalidate_session(Host, Port, Session);
 invalidate_session(server, _, Port, Session) ->
diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl
index 877c6629c2..b396eb1355 100644
--- a/lib/ssl/src/tls_connection.erl
+++ b/lib/ssl/src/tls_connection.erl
@@ -736,7 +736,7 @@ hello(internal, #server_hello{} = Hello,
              [{next_event, internal, Hello}]}
     end;
 hello(info, Event, State) ->
-    gen_info(Event, ?FUNCTION_NAME, State);
+    handle_info(Event, ?FUNCTION_NAME, State);
 hello(Type, Event, State) ->
     gen_handshake(?FUNCTION_NAME, Type, Event, State).
 
@@ -1183,9 +1183,22 @@ handle_info({PassiveTag, Socket},  StateName,
 handle_info({CloseTag, Socket}, StateName,
             #state{static_env = #static_env{
                                    role = Role,
+                                   host = Host,
+                                   port = Port,
                                    socket = Socket, 
                                    close_tag = CloseTag},
+                   handshake_env = #handshake_env{renegotiation = Type},
                    connection_env = #connection_env{negotiated_version = Version},
+                   session = Session} = State) when  StateName =/= connection ->
+    ssl_connection:maybe_invalidate_session(Version, Type, Role, Host, Port, Session),
+    Alert = ?ALERT_REC(?FATAL, ?CLOSE_NOTIFY, transport_closed),
+    ssl_connection:handle_normal_shutdown(Alert#alert{role = Role}, StateName, State),
+    {stop, {shutdown, transport_closed}, State};
+handle_info({CloseTag, Socket}, StateName,
+            #state{static_env = #static_env{
+                                   role = Role,
+                                   socket = Socket,
+                                   close_tag = CloseTag},
                    socket_options = #socket_options{active = Active},
                    protocol_buffers = #protocol_buffers{tls_cipher_texts = CTs},
                    user_data_buffer = {_,BufferSize,_},
@@ -1198,16 +1211,16 @@ handle_info({CloseTag, Socket}, StateName,
 
     case (Active == false) andalso ((CTs =/= []) or (BufferSize =/= 0)) of
         false ->
-            case Version of
-                {1, N} when N >= 1 ->
-                    ok;
-                _ ->
-                    %% As invalidate_sessions here causes performance issues,
-                    %% we will conform to the widespread implementation
-                    %% practice and go aginst the spec
-                    %%invalidate_session(Role, Host, Port, Session)
-                    ok
-            end,
+            %% As invalidate_sessions here causes performance issues,
+            %% we will conform to the widespread implementation
+            %% practice and go aginst the spec
+            %% case Version of
+            %%     {3, N} when N >= 1 ->
+            %%         ok;
+            %%     _ ->
+            %%         invalidate_session(Role, Host, Port, Session)
+            %%         ok
+            %% end,
             Alert = ?ALERT_REC(?FATAL, ?CLOSE_NOTIFY, transport_closed),
             ssl_connection:handle_normal_shutdown(Alert#alert{role = Role}, StateName, State),
             {stop, {shutdown, transport_closed}, State};
diff --git a/lib/ssl/test/ssl_session_SUITE.erl b/lib/ssl/test/ssl_session_SUITE.erl
index aa79698a72..f8dd633ed4 100644
--- a/lib/ssl/test/ssl_session_SUITE.erl
+++ b/lib/ssl/test/ssl_session_SUITE.erl
@@ -25,6 +25,7 @@
 -compile(export_all).
 
 -include("tls_handshake.hrl").
+-include("ssl_record.hrl").
 
 -include_lib("common_test/include/ct.hrl").
 -include_lib("public_key/include/public_key.hrl").
@@ -48,11 +49,11 @@ all() ->
 groups() ->
     [{'dtlsv1.2', [], session_tests()},
      {'dtlsv1', [], session_tests()},
-     {'tlsv1.3', [], session_tests()},
-     {'tlsv1.2', [], session_tests()},
-     {'tlsv1.1', [], session_tests()},
-     {'tlsv1', [], session_tests()},
-     {'sslv3', [], session_tests()}
+     {'tlsv1.3', [], session_tests() ++ tls_session_tests()},
+     {'tlsv1.2', [], session_tests() ++ tls_session_tests()},
+     {'tlsv1.1', [], session_tests() ++ tls_session_tests()},
+     {'tlsv1', [], session_tests() ++ tls_session_tests()},
+     {'sslv3', [], session_tests() ++ tls_session_tests()}
     ].
 
 session_tests() ->
@@ -62,6 +63,8 @@ session_tests() ->
      no_reuses_session_server_restart_new_cert,
      no_reuses_session_server_restart_new_cert_file].
 
+tls_session_tests() ->
+       [session_table_stable_size_on_tcp_close].
 
 init_per_suite(Config0) ->
     catch crypto:stop(),
@@ -372,6 +375,177 @@ no_reuses_session_server_restart_new_cert_file(Config) when is_list(Config) ->
     ssl_test_lib:close(Server1),
     ssl_test_lib:close(Client1).
 
+session_table_stable_size_on_tcp_close() ->
+      [{doc, "Check that new sessions are cleanup when connection is closed abruptly during first handshake"}].
+
+session_table_stable_size_on_tcp_close(Config) when is_list(Config)->
+    ServerOpts = ssl_test_lib:ssl_options(server_rsa_verify_opts, Config),
+    {_, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+    {status, _, _, StatusInfo} = sys:get_status(whereis(ssl_manager)),
+    [_, _,_, _, Prop] = StatusInfo,
+    State = ssl_test_lib:state(Prop),
+    ServerCache = element(3, State),
+
+    N = ets:info(ServerCache, size),
+
+    Server = ssl_test_lib:start_server_error([{node, ServerNode}, {port, 0},
+                                              {from, self()},
+                                              {options,  [{reuseaddr, true} | ServerOpts]}]),
+    Port = ssl_test_lib:inet_port(Server),
+    faulty_client(Hostname, Port),
+    check_table_did_not_grow(ServerCache, N).
+
+
 %%--------------------------------------------------------------------
 %% Internal functions ------------------------------------------------
 %%--------------------------------------------------------------------
+check_table_did_not_grow(ServerCache, N) ->
+    ct:sleep(500),
+    check_table_did_not_grow(ServerCache, N, 10).
+
+check_table_did_not_grow(_, _, 0) ->
+    ct:fail(table_grew);
+check_table_did_not_grow(ServerCache, N, Tries) ->
+    case ets:info(ServerCache, size) of
+        N ->
+            ok;
+        _ ->
+            ct:sleep(500),
+            check_table_did_not_grow(ServerCache, N, Tries -1)
+    end.
+
+faulty_client(Host, Port) ->
+    {ok, Sock} = gen_tcp:connect(Host, Port, [], 10000),
+    Random = crypto:strong_rand_bytes(32),
+    CH = client_hello(Random),
+    CHBin = encode_client_hello(CH, Random),
+    gen_tcp:send(Sock, CHBin),
+    ct:sleep(100),
+    gen_tcp:close(Sock).
+
+
+server(LOpts, Port) ->
+    {ok, LSock} = ssl:listen(Port, LOpts),
+    Pid = spawn_link(?MODULE, accept_loop, [LSock]),
+    ssl:controlling_process(LSock, Pid),
+    Pid.
+
+accept_loop(Sock) ->
+    {ok, CSock} = ssl:transport_accept(Sock),
+    _ = ssl:handshake(CSock),
+    accept_loop(Sock).
+
+
+encode_client_hello(CH, Random) ->
+    HSBin = tls_handshake:encode_handshake(CH, {3,3}),
+    CS = connection_states(Random),
+    {Encoded, _} = tls_record:encode_handshake(HSBin, {3,3}, CS),
+    Encoded.
+
+client_hello(Random) ->
+    CipherSuites = [<<0,255>>, <<"À,">>, <<"À0">>, <<"À$">>, <<"À(">>,
+		    <<"À.">>, <<"À2">>, <<"À&">>, <<"À*">>, <<0,159>>,
+		    <<0,163>>, <<0,107>>, <<0,106>>, <<"À+">>, <<"À/">>,
+		    <<"À#">>, <<"À'">>, <<"À-">>, <<"À1">>, <<"À%">>,
+		    <<"À)">>, <<0,158>>, <<0,162>>, <<0,103>>, <<0,64>>,
+		    <<"À\n">>, <<192,20>>, <<0,57>>, <<0,56>>, <<192,5>>,
+		    <<192,15>>, <<"À\t">>, <<192,19>>, <<0,51>>, <<0,50>>,
+		    <<192,4>>, <<192,14>>],
+    Extensions = #{alpn => undefined,
+		   ec_point_formats =>
+		       {ec_point_formats,
+			[0]},
+		   elliptic_curves =>
+		       {elliptic_curves,
+			[{1,3,132,0,39},
+			 {1,3,132,0,38},
+			 {1,3,132,0,35},
+			 {1,3,36,3,3,2,
+			  8,1,1,13},
+			 {1,3,132,0,36},
+			 {1,3,132,0,37},
+			 {1,3,36,3,3,2,
+			  8,1,1,11},
+			 {1,3,132,0,34},
+			 {1,3,132,0,16},
+			 {1,3,132,0,17},
+			 {1,3,36,3,3,2,
+			  8,1,1,7},
+			 {1,3,132,0,10},
+			 {1,2,840,
+			  10045,3,1,7},
+			 {1,3,132,0,3},
+			 {1,3,132,0,26},
+			 {1,3,132,0,27},
+			 {1,3,132,0,32},
+			 {1,3,132,0,33},
+			 {1,3,132,0,24},
+			 {1,3,132,0,25},
+			 {1,3,132,0,31},
+			 {1,2,840,
+			  10045,3,1,1},
+			 {1,3,132,0,1},
+			 {1,3,132,0,2},
+			 {1,3,132,0,15},
+			 {1,3,132,0,9},
+			 {1,3,132,0,8},
+			 {1,3,132,0,
+			  30}]},
+		   next_protocol_negotiation =>
+		       undefined,
+		   renegotiation_info =>
+		       {renegotiation_info,
+			undefined},
+		   signature_algs =>
+		       {hash_sign_algos,
+			[{sha512,ecdsa},
+			 {sha512,rsa},
+			 {sha384,ecdsa},
+			 {sha384,rsa},
+			 {sha256,ecdsa},
+			 {sha256,rsa},
+			 {sha224,ecdsa},
+			 {sha224,rsa},
+			 {sha,ecdsa},
+			 {sha,rsa},
+			 {sha,dsa}]},
+		   sni =>
+		       {sni,
+			"localhost"},
+		   srp =>
+		       undefined},
+
+    #client_hello{client_version = {3,3},
+		  random = Random,
+		  session_id = crypto:strong_rand_bytes(32),
+		  cipher_suites = CipherSuites,
+		  compression_methods = [0],
+		  extensions = Extensions
+		 }.
+
+connection_states(Random) ->
+    #{current_write =>
+          #{beast_mitigation => one_n_minus_one,cipher_state => undefined,
+		 client_verify_data => undefined,compression_state => undefined,
+		 mac_secret => undefined,secure_renegotiation => undefined,
+            security_parameters =>
+                #security_parameters{
+                  cipher_suite = <<0,0>>,
+                   connection_end = 1,
+                   bulk_cipher_algorithm = 0,
+                   cipher_type = 0,
+                   iv_size = 0,
+                   key_size = 0,
+                   key_material_length = 0,
+                   expanded_key_material_length = 0,
+                   mac_algorithm = 0,
+                   prf_algorithm = 0,
+                   hash_size = 0,
+                   compression_algorithm = 0,
+                   master_secret = undefined,
+                   resumption_master_secret = undefined,
+                   client_random = Random,
+                   server_random = undefined,
+                   exportable = undefined},
+            sequence_number => 0,server_verify_data => undefined}}.