File 0657-ssl-Properly-handle-default-session-data-s... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 0657-ssl-Properly-handle-default-session-data-storage.patch of Package erlang

From 142694507b0e6148dc21ed63ecd734b3ba8c6c26 Mon Sep 17 00:00:00 2001
From: Ingela Anderton Andin <ingela@erlang.org>
Date: Mon, 13 Sep 2021 17:12:31 +0200
Subject: [PATCH] ssl: Properly handle default session data storage

When a client tries to reuse an expired session the default
server storage handling would crash loosing other session data.
This would cause a error report and possible loss of abbreviated handshakes.

Solves #5192
---
 lib/ssl/src/ssl_server_session_cache.erl |  10 +-
 lib/ssl/test/ssl_session_SUITE.erl       | 126 ++++++++++++++++++++++-
 2 files changed, 126 insertions(+), 10 deletions(-)

diff --git a/lib/ssl/src/ssl_server_session_cache.erl b/lib/ssl/src/ssl_server_session_cache.erl
index 4c77fa951b..a89959413c 100644
--- a/lib/ssl/src/ssl_server_session_cache.erl
+++ b/lib/ssl/src/ssl_server_session_cache.erl
@@ -143,8 +143,8 @@ handle_call({reuse_session, SessionId}, _From,  #state{store_cb = Cb,
                 true ->
                     {reply, Session, State0};
                 false ->
-                    Order = invalidate_session(Cb, Store0, Order0, SessionId, InId),
-                    {reply, not_reusable, State0#state{session_order = Order}}
+                    {Store, Order} = invalidate_session(Cb, Store0, Order0, SessionId, InId),
+                    {reply, not_reusable, State0#state{db = Store, session_order = Order}}
             end
     end.
 
@@ -220,9 +220,9 @@ session_id(Key) ->
     Bin2 = crypto:crypto_one_time(aes_128_ecb, Key, <<Unique2:128>>, true),
     <<Bin1/binary, Bin2/binary>>.
 
-invalidate_session(Cb, Store, Order, SessionId, InternalId) ->
-    Cb:delete(Store, SessionId),
-    gb_trees:delete(InternalId, Order).
+invalidate_session(Cb, Store0, Order, SessionId, InternalId) ->
+    Store = delete(Cb, Store0, SessionId),
+    {Store, gb_trees:delete(InternalId, Order)}.
 
 init(Cb, Options) ->
     Cb:init(Options).
diff --git a/lib/ssl/test/ssl_session_SUITE.erl b/lib/ssl/test/ssl_session_SUITE.erl
index 33a455bf77..8f2be84a84 100644
--- a/lib/ssl/test/ssl_session_SUITE.erl
+++ b/lib/ssl/test/ssl_session_SUITE.erl
@@ -49,6 +49,8 @@
          server_does_not_want_to_reuse_session/1,
          explicit_session_reuse/0,
          explicit_session_reuse/1,
+         explicit_session_reuse_expired/0,
+         explicit_session_reuse_expired/1,
          no_reuses_session_server_restart_new_cert/0,
          no_reuses_session_server_restart_new_cert/1,
          no_reuses_session_server_restart_new_cert_file/0,
@@ -62,7 +64,7 @@
         ]).
 
 -define(SLEEP, 500).
--define(EXPIRE, 10).
+-define(EXPIRE, 2).
 -define(CLIENT_CB, ssl_client_session_cache_db).
 
 %%--------------------------------------------------------------------
@@ -91,6 +93,7 @@ session_tests() ->
      reuse_session_expired,
      server_does_not_want_to_reuse_session,
      explicit_session_reuse,
+     explicit_session_reuse_expired,
      no_reuses_session_server_restart_new_cert,
      no_reuses_session_server_restart_new_cert_file,
      client_max_session_table,
@@ -121,7 +124,8 @@ init_per_group(GroupName, Config) ->
 end_per_group(GroupName, Config) ->
   ssl_test_lib:end_per_group(GroupName, Config).
 
-init_per_testcase(reuse_session_expired, Config)  ->
+init_per_testcase(TestCase, Config)  when TestCase == reuse_session_expired;
+                                          TestCase == explicit_session_reuse_expired ->
     Versions = ssl_test_lib:protocol_version(Config),
     ssl:stop(),
     application:load(ssl),    
@@ -216,7 +220,7 @@ reuse_session_expired(Config) when is_list(Config) ->
     Server0 ! listen,
     
     %% Make sure session is unregistered due to expiration
-    ct:sleep((?EXPIRE*2)),
+    ct:sleep({seconds, ?EXPIRE*2}),
 
     make_sure_expired(Hostname, Port0, SID),
     
@@ -341,6 +345,118 @@ explicit_session_reuse(Config) when is_list(Config) ->
     {ok, [{session_id, ID}]} = ssl:connection_information(Client1Sock, [session_id]).
 
 
+%%--------------------------------------------------------------------
+
+explicit_session_reuse_expired() ->
+    [{doc,"Test to reuse a session that has expired and make sure server session table is correctly handled"}].
+explicit_session_reuse_expired(Config) when is_list(Config) ->
+    ClientOpts = ssl_test_lib:ssl_options(client_rsa_verify_opts, Config),
+    ServerOpts = ssl_test_lib:ssl_options(server_rsa_verify_opts, Config),
+    {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+    Server =
+	ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+                                   {from, self()},
+                                   {mfa, {ssl_test_lib, no_result, []}},
+                                   {options, ServerOpts}]),
+    Port = ssl_test_lib:inet_port(Server),
+    {Client0, Client0Sock} =
+	ssl_test_lib:start_client([{node, ClientNode},
+                                   {port, Port}, {host, Hostname},
+                                   {mfa, {ssl_test_lib, no_result, []}},
+                                   {from, self()}, {options, [{reuse_sessions, false} | ClientOpts]},
+                                   return_socket
+                                  ]),
+    %% Retrieve session data
+    {ok, [{session_id, ID0}, {session_data, SessData}]} = ssl:connection_information(Client0Sock, [session_id, session_data]),
+
+    ssl_test_lib:close(Client0),
+
+    Server ! listen,
+
+    SupName = sup_name(ServerOpts),
+    Sup = whereis(SupName),
+    %% Will only be one process, that is one server, in our test scenario
+    [{_, SessionCachePid, worker,[ssl_server_session_cache]}] = supervisor:which_children(Sup),
+
+    %% Start a new connections so there are three sessions
+    {Client1, Client1Sock}  = ssl_test_lib:start_client([{node, ClientNode},
+                                          {port, Port}, {host, Hostname},
+                                                         {mfa, {ssl_test_lib, no_result, []}},
+                                          {from, self()},  {options, [{reuse_sessions, save} | ClientOpts]},
+                                                         return_socket]),
+
+    Server ! listen,
+
+    {Client2, Client2Sock}  = ssl_test_lib:start_client([{node, ClientNode},
+                                                         {port, Port}, {host, Hostname},
+                                                         {mfa, {ssl_test_lib, no_result, []}},
+                                                         {from, self()},  {options, [{reuse_sessions, save} | ClientOpts]},
+                                                         return_socket]),
+
+    {ok, [{session_id, ID1}]} = ssl:connection_information(Client1Sock, [session_id]),
+
+    %% Assert three sessions in server table
+    {SessionCacheCb, SessionCacheDb} = session_cachce_info(SessionCachePid),
+    3 = SessionCacheCb:size(SessionCacheDb),
+
+    Server ! listen,
+
+    {ok, [{session_id, ID2}]} = ssl:connection_information(Client2Sock, [session_id]),
+
+    ssl_test_lib:close(Client1),
+    ssl_test_lib:close(Client2),
+
+    %% Make sure session expired
+    ct:sleep({seconds, ?EXPIRE*2}),
+
+    %% Try to reuse session one after expiration
+    {_, Client3Sock} =
+	ssl_test_lib:start_client([{node, ClientNode},
+                                   {port, Port}, {host, Hostname},
+                                   {mfa, {ssl_test_lib, no_result, []}},
+                                   {from, self()}, {options, [{reuse_session, {ID0, SessData}} | ClientOpts]},
+                                   return_socket]),
+
+    %% Verify that we got a new session
+    {ok, [{session_id, ID3}]} = ssl:connection_information(Client3Sock, [session_id]),
+
+    %% We tried reusing ID0. So ID1 and ID2 should not be possible but assert anyway
+    true = (ID3=/= ID0) andalso (ID3 =/= ID1) andalso (ID3 =/= ID2),
+
+    %% Server table should have removed the expired session that we tried to reuse.
+    %% and replaced the second one that expired. Leaving the third and the new
+    {SessionCacheCb1, SessionCacheDb1} = session_cachce_info(SessionCachePid),
+    2 = SessionCacheCb1:size(SessionCacheDb1),
+
+
+    Server ! listen,
+
+    %% Make sure session expired
+    ct:sleep({seconds, ?EXPIRE*2}),
+
+
+    %% Nothing is removed yet
+    {SessionCacheCb2, SessionCacheDb2} = session_cachce_info(SessionCachePid),
+    2 = SessionCacheCb2:size(SessionCacheDb2),
+
+
+    {_, Client4Sock} =
+	ssl_test_lib:start_client([{node, ClientNode},
+                                   {port, Port}, {host, Hostname},
+                                   {mfa, {ssl_test_lib, no_result, []}},
+                                   {from, self()}, {options, ClientOpts},
+                                   return_socket]),
+
+    %% Expired session is not reused
+    {ok, [{session_id, ID4}]} = ssl:connection_information(Client4Sock, [session_id]),
+    true = (ID4 =/= ID3),
+
+    %% One expired session is replaced
+    {SessionCacheCb3, SessionCacheDb3} = session_cachce_info(SessionCachePid),
+    2 = SessionCacheCb3:size(SessionCacheDb3).
+
+
 %%--------------------------------------------------------------------
 no_reuses_session_server_restart_new_cert() ->
     [{doc,"Check that a session is not reused if the server is restarted with a new cert."}].
@@ -500,7 +616,7 @@ session_table_stable_size_on_tcp_close(Config) when is_list(Config)->
 
     Sup = whereis(ssl_server_session_cache_sup),
 
-    %% Will only be one process, that is one server, in our test senario
+    %% Will only be one process, that is one server, in our test scenario
     [{_, SessionCachePid, worker,[ssl_server_session_cache]}] = supervisor:which_children(Sup),
 
 
@@ -538,7 +654,7 @@ faulty_client(Host, Port) ->
     CH = client_hello(Random),
     CHBin = encode_client_hello(CH, Random),
     gen_tcp:send(Sock, CHBin),
-    ct:sleep(100),
+    ct:sleep(?SLEEP),
     gen_tcp:close(Sock).
 
 
-- 
2.31.1

Places

File 0657-ssl-Properly-handle-default-session-data-storage.patch of Package erlang

Places