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
