From b2c0bef3cff85784b2ccb3b91bbea997acab6187 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Fri, 4 Sep 2015 16:13:08 +0200 Subject: ssl: Correct return value of default session callback module ssl_session_cache:select_session/2 returned [sesionid(), #session{}] instead of #session{} as the API demands. This was wrongly compensated for in the code in one place making it look like everything was good. But the client check for unique session would always fail, potentially making the client session table grow a lot and causing long setup times. --- lib/ssl/src/ssl_session.erl | 4 +- lib/ssl/src/ssl_session_cache.erl | 2 +- lib/ssl/test/ssl_session_cache_SUITE.erl | 71 +++++++++++++++++++++++++++----- lib/ssl/test/ssl_test_lib.erl | 18 +++++++- 4 files changed, 80 insertions(+), 15 deletions(-) diff --git a/lib/ssl/src/ssl_session.erl b/lib/ssl/src/ssl_session.erl index 1770faf1ff..0d6cc93a20 100644 --- a/lib/ssl/src/ssl_session.erl +++ b/lib/ssl/src/ssl_session.erl @@ -100,14 +100,14 @@ select_session([], _, _) -> no_session; select_session(Sessions, #ssl_options{ciphers = Ciphers}, OwnCert) -> IsNotResumable = - fun([_Id, Session]) -> + fun(Session) -> not (resumable(Session#session.is_resumable) andalso lists:member(Session#session.cipher_suite, Ciphers) andalso (OwnCert == Session#session.own_certificate)) end, case lists:dropwhile(IsNotResumable, Sessions) of [] -> no_session; - [[Id, _]|_] -> Id + [Session | _] -> Session#session.session_id end. is_resumable(_, _, #ssl_options{reuse_sessions = false}, _, _, _, _) -> diff --git a/lib/ssl/src/ssl_session_cache.erl b/lib/ssl/src/ssl_session_cache.erl index 11ed310477..cfc48cd935 100644 --- a/lib/ssl/src/ssl_session_cache.erl +++ b/lib/ssl/src/ssl_session_cache.erl @@ -83,7 +83,7 @@ foldl(Fun, Acc0, Cache) -> %%-------------------------------------------------------------------- select_session(Cache, PartialKey) -> ets:select(Cache, - [{{{PartialKey,'$1'}, '$2'},[],['$$']}]). + [{{{PartialKey,'_'}, '$1'},[],['$1']}]). %%-------------------------------------------------------------------- %%% Internal functions diff --git a/lib/ssl/test/ssl_session_cache_SUITE.erl b/lib/ssl/test/ssl_session_cache_SUITE.erl index 8ddc5db4b2..0738869f2b 100644 --- a/lib/ssl/test/ssl_session_cache_SUITE.erl +++ b/lib/ssl/test/ssl_session_cache_SUITE.erl @@ -45,7 +45,8 @@ all() -> [session_cleanup, session_cache_process_list, - session_cache_process_mnesia]. + session_cache_process_mnesia, + client_unique_session]. groups() -> []. @@ -90,8 +91,8 @@ init_per_testcase(session_cleanup, Config) -> ct:timetrap({seconds, 20}), Config; -init_per_testcase(_TestCase, Config) -> - ct:timetrap({seconds, 5}), +init_per_testcase(client_unique_session, Config) -> + ct:timetrap({seconds, 20}), Config. init_customized_session_cache(Type, Config) -> @@ -131,10 +132,40 @@ end_per_testcase(_, Config) -> %%-------------------------------------------------------------------- %% Test Cases -------------------------------------------------------- %%-------------------------------------------------------------------- +client_unique_session() -> + [{doc, "Test session table does not grow when client " + "sets up many connections"}]. +client_unique_session(Config) when is_list(Config) -> + process_flag(trap_exit, true), + ClientOpts = ?config(client_opts, Config), + ServerOpts = ?config(server_opts, Config), + {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), + Server = + ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, + {from, self()}, + {from, self()}, + {mfa, {ssl_test_lib, no_result, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + LastClient = clients_start(Server, + ClientNode, Hostname, Port, ClientOpts, 20), + receive + {LastClient, {ok, _}} -> + ok + end, + {status, _, _, StatusInfo} = sys:get_status(whereis(ssl_manager)), + [_, _,_, _, Prop] = StatusInfo, + State = ssl_test_lib:state(Prop), + ClientCache = element(2, State), + 1 = ets:info(ClientCache, size), + + ssl_test_lib:close(Server, 500), + ssl_test_lib:close(LastClient). + session_cleanup() -> [{doc, "Test that sessions are cleand up eventually, so that the session table " "does not grow and grow ..."}]. -session_cleanup(Config)when is_list(Config) -> +session_cleanup(Config) when is_list(Config) -> process_flag(trap_exit, true), ClientOpts = ?config(client_opts, Config), ServerOpts = ?config(server_opts, Config), @@ -148,9 +179,9 @@ session_cleanup(Config)when is_list(Config) -> Port = ssl_test_lib:inet_port(Server), Client = ssl_test_lib:start_client([{node, ClientNode}, - {port, Port}, {host, Hostname}, + {port, Port}, {host, Hostname}, {mfa, {ssl_test_lib, no_result, []}}, - {from, self()}, {options, ClientOpts}]), + {from, self()}, {options, ClientOpts}]), SessionInfo = receive {Server, Info} -> @@ -325,8 +356,8 @@ select_session(Cache, PartialKey) -> mnesia -> Sel = fun() -> mnesia:select(Cache, - [{{Cache,{PartialKey,'$1'}, '$2'}, - [],['$$']}]) + [{{Cache,{PartialKey,'_'}, '$1'}, + [],['$1']}]) end, {atomic, Res} = mnesia:transaction(Sel), Res @@ -354,8 +385,8 @@ session_loop(Sess) -> Pid ! {self(), Res}, session_loop(Sess); {Pid,select_session,PKey} -> - Sel = fun({{PKey0, Id},Session}, Acc) when PKey == PKey0 -> - [[Id, Session]|Acc]; + Sel = fun({{PKey0, _Id},Session}, Acc) when PKey == PKey0 -> + [Session | Acc]; (_,Acc) -> Acc end, @@ -370,3 +401,23 @@ session_loop(Sess) -> session_cache_process(_Type,Config) when is_list(Config) -> ssl_basic_SUITE:reuse_session(Config). + + +clients_start(_Server, ClientNode, Hostname, Port, ClientOpts, 0) -> + %% Make sure session is registered + ct:sleep(?SLEEP * 2), + ssl_test_lib:start_client([{node, ClientNode}, + {port, Port}, {host, Hostname}, + {mfa, {?MODULE, connection_info_result, []}}, + {from, self()}, {options, ClientOpts}]); +clients_start(Server, ClientNode, Hostname, Port, ClientOpts, N) -> + spawn_link(ssl_test_lib, start_client, + [[{node, ClientNode}, + {port, Port}, {host, Hostname}, + {mfa, {ssl_test_lib, no_result, []}}, + {from, self()}, {options, ClientOpts}]]), + Server ! listen, + clients_start(Server, ClientNode, Hostname, Port, ClientOpts, N-1). + +connection_info_result(Socket) -> + ssl:connection_information(Socket, [protocol, cipher_suite]). diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl index 8317148aa5..ba8588f2f9 100644 --- a/lib/ssl/test/ssl_test_lib.erl +++ b/lib/ssl/test/ssl_test_lib.erl @@ -241,7 +241,21 @@ close(Pid) -> receive {'DOWN', Monitor, process, Pid, Reason} -> erlang:demonitor(Monitor), - ct:log("~p:~p~nPid: ~p down due to:~p ~n", [?MODULE,?LINE, Pid, Reason]) + ct:log("~p:~p~nPid: ~p down due to:~p ~n", [?MODULE,?LINE, Pid, Reason]) + + end. + +close(Pid, Timeout) -> + ct:log("~p:~p~n Close ~p ~n", [?MODULE,?LINE, Pid]), + Monitor = erlang:monitor(process, Pid), + Pid ! close, + receive + {'DOWN', Monitor, process, Pid, Reason} -> + erlang:demonitor(Monitor), + ct:log("~p:~p~nPid: ~p down due to:~p ~n", [?MODULE,?LINE, Pid, Reason]) + after + Timeout -> + exit(Pid, kill) end. check_result(Server, ServerMsg, Client, ClientMsg) -> @@ -360,7 +374,7 @@ cert_options(Config) -> SNIServerAKeyFile = filename:join([?config(priv_dir, Config), "a.server", "key.pem"]), SNIServerBCertFile = filename:join([?config(priv_dir, Config), "b.server", "cert.pem"]), SNIServerBKeyFile = filename:join([?config(priv_dir, Config), "b.server", "key.pem"]), - [{client_opts, [{ssl_imp, new},{reuseaddr, true}]}, + [{client_opts, []}, {client_verification_opts, [{cacertfile, ClientCaCertFile}, {certfile, ClientCertFile}, {keyfile, ClientKeyFile}, -- cgit v1.2.3 From 1b06210c16465bcb995b0a54ba1b24ef1de3c5a4 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Tue, 25 Aug 2015 18:19:38 +0200 Subject: ssl: Improve shutdown logic Add possibility to downgrade an SSL/TLS connection to a tcp connection, and give back the socket control to a user process. Add application setting to be able to change fatal alert shutdown timeout, also shorten the default timeout. The fatal alert timeout is the number of milliseconds between sending of a fatal alert and closing the connection. Waiting a little while improves the peers chances to properly receiving the alert so it may shutdown gracefully. --- lib/ssl/doc/src/ssl.xml | 15 +++++++ lib/ssl/doc/src/ssl_app.xml | 11 +++++ lib/ssl/src/ssl.erl | 20 ++++++++- lib/ssl/src/ssl_connection.erl | 95 +++++++++++++++++++--------------------- lib/ssl/src/tls_connection.erl | 55 ++++++++++++++++++----- lib/ssl/test/ssl_basic_SUITE.erl | 76 +++++++++++++++++++++++++++++++- 6 files changed, 208 insertions(+), 64 deletions(-) diff --git a/lib/ssl/doc/src/ssl.xml b/lib/ssl/doc/src/ssl.xml index 6c977bdb74..22ac98c24e 100644 --- a/lib/ssl/doc/src/ssl.xml +++ b/lib/ssl/doc/src/ssl.xml @@ -765,6 +765,21 @@ fun(srp, Username :: string(), UserState :: term()) -> + + close(SslSocket, How) -> ok | {ok, port()} | {error, Reason} + Closes an SSL connection. + + SslSocket = sslsocket() + How = timeout() | {NewController::pid(), timeout()} + Reason = term() + +

Closes or downgrades an SSL connection, in the later case the transport + connection will be handed over to the NewController process after reciving + the TLS close alert from the peer. The retuned transport socket will have + the following options set [{active, false}, {packet, 0}, {mode, binary}].

+ + + connection_info(SslSocket) -> {ok, {ProtocolVersion, CipherSuite}} | {error, Reason} diff --git a/lib/ssl/doc/src/ssl_app.xml b/lib/ssl/doc/src/ssl_app.xml index 2b6dc7e8be..51ce0cedf1 100644 --- a/lib/ssl/doc/src/ssl_app.xml +++ b/lib/ssl/doc/src/ssl_app.xml @@ -87,6 +87,17 @@ marker="ssl#clear_pem_cache-0">ssl:clear_pem_cache/0 + ]]> + +

+ Number of milliseconds between sending of a fatal alert and + closing the connection. Waiting a little while improves the + peers chances to properly receiving the alert so it may + shutdown gracefully. Defaults to 5000 milliseconds. +

+ + + diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl index 120e8b59ed..f611079912 100644 --- a/lib/ssl/src/ssl.erl +++ b/lib/ssl/src/ssl.erl @@ -34,7 +34,7 @@ listen/2, transport_accept/1, transport_accept/2, ssl_accept/1, ssl_accept/2, ssl_accept/3, controlling_process/2, peername/1, peercert/1, sockname/1, - close/1, shutdown/2, recv/2, recv/3, send/2, getopts/2, setopts/2 + close/1, close/2, shutdown/2, recv/2, recv/3, send/2, getopts/2, setopts/2 ]). %% SSL/TLS protocol handling -export([cipher_suites/0, cipher_suites/1, suite_definition/1, @@ -247,10 +247,26 @@ ssl_accept(Socket, SslOptions, Timeout) when is_port(Socket) -> %% Description: Close an ssl connection %%-------------------------------------------------------------------- close(#sslsocket{pid = Pid}) when is_pid(Pid) -> - ssl_connection:close(Pid); + ssl_connection:close(Pid, {close, ?DEFAULT_TIMEOUT}); close(#sslsocket{pid = {ListenSocket, #config{transport_info={Transport,_, _, _}}}}) -> Transport:close(ListenSocket). +%%-------------------------------------------------------------------- +-spec close(#sslsocket{}, integer() | {pid(), integer()}) -> term(). +%% +%% Description: Close an ssl connection +%%-------------------------------------------------------------------- +close(#sslsocket{pid = TLSPid}, + {Pid, Timeout} = DownGrade) when is_pid(TLSPid), + is_pid(Pid), + (is_integer(Timeout) andalso Timeout > 0) or (Timeout == infinity) -> + ssl_connection:close(TLSPid, {close, DownGrade}); +close(#sslsocket{pid = TLSPid}, Timeout) when is_pid(TLSPid), + (is_integer(Timeout) andalso Timeout > 0) or (Timeout == infinity) -> + ssl_connection:close(TLSPid, {close, Timeout}); +close(#sslsocket{pid = {ListenSocket, #config{transport_info={Transport,_, _, _}}}}, _) -> + Transport:close(ListenSocket). + %%-------------------------------------------------------------------- -spec send(#sslsocket{}, iodata()) -> ok | {error, reason()}. %% diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index 5b754c16bc..f8afbdb41d 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -41,7 +41,7 @@ socket_control/4, socket_control/5]). %% User Events --export([send/2, recv/3, close/1, shutdown/2, +-export([send/2, recv/3, close/2, shutdown/2, new_user/2, get_opts/2, set_opts/2, session_info/1, peer_certificate/1, renegotiation/1, negotiated_protocol/1, prf/5, connection_information/1 @@ -171,18 +171,19 @@ connection_information(Pid) when is_pid(Pid) -> sync_send_all_state_event(Pid, connection_information). %%-------------------------------------------------------------------- --spec close(pid()) -> ok | {error, reason()}. +-spec close(pid(), {close, Timeout::integer() | + {NewController::pid(), Timeout::integer()}}) -> + ok | {ok, port()} | {error, reason()}. %% %% Description: Close an ssl connection %%-------------------------------------------------------------------- -close(ConnectionPid) -> - case sync_send_all_state_event(ConnectionPid, close) of +close(ConnectionPid, How) -> + case sync_send_all_state_event(ConnectionPid, How) of {error, closed} -> ok; Other -> Other end. - %%-------------------------------------------------------------------- -spec shutdown(pid(), atom()) -> ok | {error, reason()}. %% @@ -706,12 +707,12 @@ handle_sync_event({start, Timeout}, StartFrom, StateName, #state{role = Role, s {stop, normal, {error, Error}, State0} end; -handle_sync_event(close, _, StateName, #state{protocol_cb = Connection} = State) -> - %% Run terminate before returning - %% so that the reuseaddr inet-option will work - %% as intended. - (catch Connection:terminate(user_close, StateName, State)), - {stop, normal, ok, State#state{terminated = true}}; +handle_sync_event({close, _} = Close, _, StateName, #state{protocol_cb = Connection} = State) -> + %% Run terminate before returning so that the reuseaddr + %% inet-option and possible downgrade will work as intended. + Result = Connection:terminate(Close, StateName, State), + {stop, normal, Result, State#state{terminated = true}}; + handle_sync_event({shutdown, How0}, _, StateName, #state{transport_cb = Transport, negotiated_version = Version, @@ -901,41 +902,46 @@ terminate(_, _, #state{terminated = true}) -> %% we want to guarantee that Transport:close has been called %% when ssl:close/1 returns. ok; -terminate({shutdown, transport_closed}, StateName, #state{send_queue = SendQueue, - renegotiation = Renegotiate} = State) -> - handle_unrecv_data(StateName, State), +terminate({shutdown, transport_closed} = Reason, + _StateName, #state{send_queue = SendQueue, protocol_cb = Connection, + socket = Socket, transport_cb = Transport, + renegotiation = Renegotiate} = State) -> handle_trusted_certs_db(State), notify_senders(SendQueue), - notify_renegotiater(Renegotiate); - -terminate({shutdown, own_alert}, _StateName, #state{send_queue = SendQueue, - renegotiation = Renegotiate} = State) -> + notify_renegotiater(Renegotiate), + Connection:close(Reason, Socket, Transport, undefined, undefined); +terminate({shutdown, own_alert}, _StateName, #state{send_queue = SendQueue, protocol_cb = Connection, + socket = Socket, transport_cb = Transport, + renegotiation = Renegotiate} = State) -> handle_trusted_certs_db(State), notify_senders(SendQueue), - notify_renegotiater(Renegotiate); + notify_renegotiater(Renegotiate), + case application:get_env(ssl, alert_timeout) of + {ok, Timeout} when is_integer(Timeout) -> + Connection:close({timeout, Timeout}, Socket, Transport, undefined, undefined); + _ -> + Connection:close({timeout, ?DEFAULT_TIMEOUT}, Socket, Transport, undefined, undefined) + end; terminate(Reason, connection, #state{negotiated_version = Version, protocol_cb = Connection, - connection_states = ConnectionStates, + connection_states = ConnectionStates0, + ssl_options = #ssl_options{padding_check = Check}, transport_cb = Transport, socket = Socket, send_queue = SendQueue, renegotiation = Renegotiate} = State) -> handle_trusted_certs_db(State), notify_senders(SendQueue), notify_renegotiater(Renegotiate), - BinAlert = terminate_alert(Reason, Version, ConnectionStates), + {BinAlert, ConnectionStates} = terminate_alert(Reason, Version, ConnectionStates0), Transport:send(Socket, BinAlert), - case Connection of - tls_connection -> - tls_connection:workaround_transport_delivery_problems(Socket, Transport); - _ -> - ok - end; -terminate(_Reason, _StateName, #state{transport_cb = Transport, + Connection:close(Reason, Socket, Transport, ConnectionStates, Check); + +terminate(Reason, _StateName, #state{transport_cb = Transport, protocol_cb = Connection, socket = Socket, send_queue = SendQueue, renegotiation = Renegotiate} = State) -> handle_trusted_certs_db(State), notify_senders(SendQueue), notify_renegotiater(Renegotiate), - Transport:close(Socket). + Connection:close(Reason, Socket, Transport, undefined, undefined). format_status(normal, [_, State]) -> [{data, [{"StateData", State}]}]; @@ -1758,30 +1764,17 @@ get_timeout(#state{ssl_options=#ssl_options{hibernate_after = undefined}}) -> get_timeout(#state{ssl_options=#ssl_options{hibernate_after = HibernateAfter}}) -> HibernateAfter. -terminate_alert(Reason, Version, ConnectionStates) when Reason == normal; - Reason == user_close -> - {BinAlert, _} = ssl_alert:encode(?ALERT_REC(?WARNING, ?CLOSE_NOTIFY), - Version, ConnectionStates), - BinAlert; -terminate_alert({shutdown, _}, Version, ConnectionStates) -> - {BinAlert, _} = ssl_alert:encode(?ALERT_REC(?WARNING, ?CLOSE_NOTIFY), - Version, ConnectionStates), - BinAlert; +terminate_alert(normal, Version, ConnectionStates) -> + ssl_alert:encode(?ALERT_REC(?WARNING, ?CLOSE_NOTIFY), + Version, ConnectionStates); +terminate_alert({Reason, _}, Version, ConnectionStates) when Reason == close; + Reason == shutdown -> + ssl_alert:encode(?ALERT_REC(?WARNING, ?CLOSE_NOTIFY), + Version, ConnectionStates); terminate_alert(_, Version, ConnectionStates) -> - {BinAlert, _} = ssl_alert:encode(?ALERT_REC(?FATAL, ?INTERNAL_ERROR), - Version, ConnectionStates), - BinAlert. - -handle_unrecv_data(StateName, #state{socket = Socket, transport_cb = Transport, - protocol_cb = Connection} = State) -> - ssl_socket:setopts(Transport, Socket, [{active, false}]), - case Transport:recv(Socket, 0, 0) of - {error, closed} -> - ok; - {ok, Data} -> - Connection:handle_close_alert(Data, StateName, State) - end. + ssl_alert:encode(?ALERT_REC(?FATAL, ?INTERNAL_ERROR), + Version, ConnectionStates). handle_trusted_certs_db(#state{ssl_options = #ssl_options{cacertfile = <<>>, cacerts = []}}) -> %% No trusted certs specified diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl index 7fda2377ee..3093508f61 100644 --- a/lib/ssl/src/tls_connection.erl +++ b/lib/ssl/src/tls_connection.erl @@ -54,7 +54,7 @@ %% Alert and close handling -export([send_alert/2, handle_own_alert/4, handle_close_alert/3, handle_normal_shutdown/3, handle_unexpected_message/3, - workaround_transport_delivery_problems/2, alert_user/6, alert_user/9 + close/5, alert_user/6, alert_user/9 ]). %% Data handling @@ -924,8 +924,7 @@ handle_own_alert(Alert, Version, StateName, try %% Try to tell the other side {BinMsg, _} = ssl_alert:encode(Alert, Version, ConnectionStates), - Transport:send(Socket, BinMsg), - workaround_transport_delivery_problems(Socket, Transport) + Transport:send(Socket, BinMsg) catch _:_ -> %% Can crash if we are in a uninitialized state ignore end, @@ -977,21 +976,57 @@ invalidate_session(client, Host, Port, Session) -> invalidate_session(server, _, Port, Session) -> ssl_manager:invalidate_session(Port, Session). -workaround_transport_delivery_problems(Socket, gen_tcp = Transport) -> +%% User downgrades connection +%% When downgrading an TLS connection to a transport connection +%% we must recive the close message before releasing the +%% transport socket. +close({close, {Pid, Timeout}}, Socket, Transport, ConnectionStates, Check) when is_pid(Pid) -> + ssl_socket:setopts(Transport, Socket, [{active, false}, {packet, ssl_tls}]), + case Transport:recv(Socket, 0, Timeout) of + {ok, {ssl_tls, Socket, ?ALERT, Version, Fragment}} -> + case tls_record:decode_cipher_text(#ssl_tls{type = ?ALERT, + version = Version, + fragment = Fragment + }, ConnectionStates, Check) of + {#ssl_tls{fragment = Plain}, _} -> + [Alert| _] = decode_alerts(Plain), + downgrade(Alert, Transport, Socket, Pid) + end; + {error, timeout} -> + {error, timeout}; + _ -> + {error, no_tls_close} + end; +%% User closes or recursive call! +close({close, Timeout}, Socket, Transport = gen_tcp, _,_) -> + ssl_socket:setopts(Transport, Socket, [{active, false}]), + Transport:shutdown(Socket, write), + _ = Transport:recv(Socket, 0, Timeout), + ok; +%% Peer closed socket +close({shutdown, transport_closed}, Socket, Transport = gen_tcp, ConnectionStates, Check) -> + close({close, 0}, Socket, Transport, ConnectionStates, Check); +%% We generate fatal alert +close({shutdown, own_alert}, Socket, Transport = gen_tcp, ConnectionStates, Check) -> %% Standard trick to try to make sure all %% data sent to the tcp port is really delivered to the %% peer application before tcp port is closed so that the peer will %% get the correct TLS alert message and not only a transport close. - ssl_socket:setopts(Transport, Socket, [{active, false}]), - Transport:shutdown(Socket, write), - %% Will return when other side has closed or after 30 s + %% Will return when other side has closed or after timout millisec %% e.g. we do not want to hang if something goes wrong %% with the network but we want to maximise the odds that %% peer application gets all data sent on the tcp connection. - Transport:recv(Socket, 0, 30000); -workaround_transport_delivery_problems(Socket, Transport) -> + close({close, ?DEFAULT_TIMEOUT}, Socket, Transport, ConnectionStates, Check); +%% Other +close(_, Socket, Transport, _,_) -> Transport:close(Socket). - +downgrade(#alert{description = ?CLOSE_NOTIFY}, Transport, Socket, Pid) -> + ssl_socket:setopts(Transport, Socket, [{active, false}, {packet, 0}, {mode, binary}]), + Transport:controlling_process(Socket, Pid), + {ok, Socket}; +downgrade(_, _,_,_) -> + {error, no_tls_close}. + convert_state(#state{ssl_options = Options} = State, up, "5.3.5", "5.3.6") -> State#state{ssl_options = convert_options_partial_chain(Options, up)}; convert_state(#state{ssl_options = Options} = State, down, "5.3.6", "5.3.5") -> diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index 378f42c2ee..82c3c4ac74 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -129,6 +129,8 @@ api_tests() -> controlling_process, upgrade, upgrade_with_timeout, + downgrade, + close_with_timeout, shutdown, shutdown_write, shutdown_both, @@ -320,7 +322,8 @@ init_per_testcase(rizzo, Config) -> Config; init_per_testcase(TestCase, Config) when TestCase == ssl_accept_timeout; - TestCase == client_closes_socket -> + TestCase == client_closes_socket; + TestCase == downgrade -> ct:log("TLS/SSL version ~p~n ", [tls_record:supported_protocol_versions()]), ct:timetrap({seconds, 15}), Config; @@ -1407,6 +1410,53 @@ upgrade_with_timeout(Config) when is_list(Config) -> ssl_test_lib:close(Server), ssl_test_lib:close(Client). +%%-------------------------------------------------------------------- +downgrade() -> + [{doc,"Test that you can downgarde an ssl connection to an tcp connection"}]. +downgrade(Config) when is_list(Config) -> + ClientOpts = ?config(client_opts, Config), + ServerOpts = ?config(server_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, tls_downgrade, []}}, + {options, [{active, false} | ServerOpts]}]), + Port = ssl_test_lib:inet_port(Server), + Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port}, + {host, Hostname}, + {from, self()}, + {mfa, {?MODULE, tls_downgrade, []}}, + {options, [{active, false} |ClientOpts]}]), + + ssl_test_lib:check_result(Server, ok, Client, ok), + ssl_test_lib:close(Server), + ssl_test_lib:close(Client). + +%%-------------------------------------------------------------------- +close_with_timeout() -> + [{doc,"Test normal (not downgrade) ssl:close/2"}]. +close_with_timeout(Config) when is_list(Config) -> + ClientOpts = ?config(client_opts, Config), + ServerOpts = ?config(server_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, tls_close, []}}, + {options,[{active, false} | ServerOpts]}]), + Port = ssl_test_lib:inet_port(Server), + Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port}, + {host, Hostname}, + {from, self()}, + {mfa, {?MODULE, tls_close, []}}, + {options, [{active, false} |ClientOpts]}]), + + ssl_test_lib:check_result(Server, ok, Client, ok). + + %%-------------------------------------------------------------------- tcp_connect() -> [{doc,"Test what happens when a tcp tries to connect, i,e. a bad (ssl) packet is sent first"}]. @@ -3931,6 +3981,30 @@ connect_dist_c(S) -> {ok, Test} = ssl:recv(S, 0, 10000), ok. +tls_downgrade(Socket) -> + ok = ssl_test_lib:send_recv_result(Socket), + case ssl:close(Socket, {self(), 5000}) of + {ok, TCPSocket} -> + inet:setopts(TCPSocket, [{active, true}]), + gen_tcp:send(TCPSocket, "Downgraded"), + receive + {tcp, TCPSocket, <<"Downgraded">>} -> + ok; + Other -> + {error, Other} + end; + {error, timeout} -> + ct:pal("Timed out, downgrade aborted"), + ok; + Fail -> + {error, Fail} + end. + +tls_close(Socket) -> + ok = ssl_test_lib:send_recv_result(Socket), + ok = ssl:close(Socket, 5000). + + %% First two clauses handles 1/n-1 splitting countermeasure Rizzo/Duong-Beast treashold(N, {3,0}) -> (N div 2) + 1; -- cgit v1.2.3 From 82aafa295a4004bcbd549c5bb888907e3ffc0c97 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Mon, 31 Aug 2015 16:44:16 +0200 Subject: ssl: Better timeout gaurds --- lib/ssl/src/ssl.erl | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl index f611079912..03495cfd90 100644 --- a/lib/ssl/src/ssl.erl +++ b/lib/ssl/src/ssl.erl @@ -99,7 +99,8 @@ stop() -> connect(Socket, SslOptions) when is_port(Socket) -> connect(Socket, SslOptions, infinity). -connect(Socket, SslOptions0, Timeout) when is_port(Socket) -> +connect(Socket, SslOptions0, Timeout) when is_port(Socket), + (is_integer(Timeout) andalso Timeout > 0) or (Timeout == infinity) -> {Transport,_,_,_} = proplists:get_value(cb_info, SslOptions0, {gen_tcp, tcp, tcp_closed, tcp_error}), EmulatedOptions = ssl_socket:emulated_options(), @@ -125,7 +126,7 @@ connect(Socket, SslOptions0, Timeout) when is_port(Socket) -> connect(Host, Port, Options) -> connect(Host, Port, Options, infinity). -connect(Host, Port, Options, Timeout) -> +connect(Host, Port, Options, Timeout) when (is_integer(Timeout) andalso Timeout > 0) or (Timeout == infinity) -> try handle_options(Options) of {ok, Config} -> do_connect(Host,Port,Config,Timeout) @@ -175,7 +176,7 @@ transport_accept(#sslsocket{pid = {ListenSocket, #config{transport_info = {Transport,_,_, _} =CbInfo, connection_cb = ConnectionCb, ssl = SslOpts, - emulated = Tracker}}}, Timeout) -> + emulated = Tracker}}}, Timeout) when (is_integer(Timeout) andalso Timeout > 0) or (Timeout == infinity) -> case Transport:accept(ListenSocket, Timeout) of {ok, Socket} -> {ok, EmOpts} = ssl_socket:get_emulated_opts(Tracker), @@ -208,15 +209,16 @@ transport_accept(#sslsocket{pid = {ListenSocket, ssl_accept(ListenSocket) -> ssl_accept(ListenSocket, infinity). -ssl_accept(#sslsocket{} = Socket, Timeout) -> +ssl_accept(#sslsocket{} = Socket, Timeout) when (is_integer(Timeout) andalso Timeout > 0) or (Timeout == infinity) -> ssl_connection:handshake(Socket, Timeout); ssl_accept(ListenSocket, SslOptions) when is_port(ListenSocket) -> ssl_accept(ListenSocket, SslOptions, infinity). -ssl_accept(#sslsocket{} = Socket, [], Timeout) -> +ssl_accept(#sslsocket{} = Socket, [], Timeout) when (is_integer(Timeout) andalso Timeout > 0) or (Timeout == infinity)-> ssl_accept(#sslsocket{} = Socket, Timeout); -ssl_accept(#sslsocket{fd = {_, _, _, Tracker}} = Socket, SslOpts0, Timeout) -> +ssl_accept(#sslsocket{fd = {_, _, _, Tracker}} = Socket, SslOpts0, Timeout) when + (is_integer(Timeout) andalso Timeout > 0) or (Timeout == infinity)-> try {ok, EmOpts, InheritedSslOpts} = ssl_socket:get_all_opts(Tracker), SslOpts = handle_options(SslOpts0, InheritedSslOpts), @@ -224,7 +226,8 @@ ssl_accept(#sslsocket{fd = {_, _, _, Tracker}} = Socket, SslOpts0, Timeout) -> catch Error = {error, _Reason} -> Error end; -ssl_accept(Socket, SslOptions, Timeout) when is_port(Socket) -> +ssl_accept(Socket, SslOptions, Timeout) when is_port(Socket), + (is_integer(Timeout) andalso Timeout > 0) or (Timeout == infinity) -> {Transport,_,_,_} = proplists:get_value(cb_info, SslOptions, {gen_tcp, tcp, tcp_closed, tcp_error}), EmulatedOptions = ssl_socket:emulated_options(), @@ -285,7 +288,8 @@ send(#sslsocket{pid = {ListenSocket, #config{transport_info={Transport, _, _, _} %%-------------------------------------------------------------------- recv(Socket, Length) -> recv(Socket, Length, infinity). -recv(#sslsocket{pid = Pid}, Length, Timeout) when is_pid(Pid) -> +recv(#sslsocket{pid = Pid}, Length, Timeout) when is_pid(Pid), + (is_integer(Timeout) andalso Timeout > 0) or (Timeout == infinity)-> ssl_connection:recv(Pid, Length, Timeout); recv(#sslsocket{pid = {Listen, #config{transport_info = {Transport, _, _, _}}}}, _,_) when is_port(Listen)-> -- cgit v1.2.3