diff options
author | Ingela Anderton Andin <ingela@erlang.org> | 2020-02-12 14:50:52 +0100 |
---|---|---|
committer | Péter Dimitrov <peterdmv@erlang.org> | 2020-02-24 09:58:53 +0100 |
commit | 6aebaa11abb36a09f2b800c59b35ed59cff333b0 (patch) | |
tree | e335332fa47155b2c2f8403b8120f46914bd8430 | |
parent | 60d3c955652cbd44e9d1e300b4834e16db565ce1 (diff) | |
download | erlang-6aebaa11abb36a09f2b800c59b35ed59cff333b0.tar.gz |
ssl: Invalidate session on abrupt close during initial handshake
-rw-r--r-- | lib/ssl/src/ssl_connection.erl | 29 | ||||
-rw-r--r-- | lib/ssl/src/tls_connection.erl | 35 | ||||
-rw-r--r-- | lib/ssl/test/ssl_session_SUITE.erl | 184 |
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}}. |