From 6e0853421cb470cb876004d81389575b2d52a2a4 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Fri, 13 Sep 2024 14:22:06 +0200 Subject: [PATCH] ssl: Convey alert information to passive socket operations recv and setopts If a TLS-1.3 server fails client certification the alert might arrive in the connection state and even after data has been sent. Make sure the alert information will be available in error reason returned from passive socket API functions recv and setopt. --- lib/ssl/src/ssl_connection.hrl | 3 +- lib/ssl/src/ssl_gen_statem.erl | 56 ++++++++++++---- lib/ssl/src/tls_gen_connection.erl | 17 +++-- lib/ssl/test/tls_1_3_version_SUITE.erl | 89 +++++++++++++++++++++++++- 4 files changed, 145 insertions(+), 20 deletions(-) diff --git a/lib/ssl/src/ssl_connection.hrl b/lib/ssl/src/ssl_connection.hrl index e55e920e4336..3e431dcc6043 100644 --- a/lib/ssl/src/ssl_connection.hrl +++ b/lib/ssl/src/ssl_connection.hrl @@ -31,6 +31,7 @@ -include("ssl_handshake.hrl"). -include("ssl_srp.hrl"). -include("ssl_cipher.hrl"). +-include("ssl_alert.hrl"). -include_lib("public_key/include/public_key.hrl"). -record(static_env, { @@ -108,7 +109,7 @@ user_application :: {Monitor::reference(), User::pid()}, downgrade :: {NewController::pid(), From::gen_statem:from()} | 'undefined', socket_terminated = false ::boolean(), - socket_tls_closed = false ::boolean(), + socket_tls_closed = false ::boolean() | #alert{}, negotiated_version :: ssl_record:ssl_version() | 'undefined', erl_dist_handle = undefined :: erlang:dist_handle() | 'undefined', cert_key_alts = undefined :: #{eddsa => list(), diff --git a/lib/ssl/src/ssl_gen_statem.erl b/lib/ssl/src/ssl_gen_statem.erl index 73cb87d66452..1e31356c1696 100644 --- a/lib/ssl/src/ssl_gen_statem.erl +++ b/lib/ssl/src/ssl_gen_statem.erl @@ -566,14 +566,12 @@ config_error(_Type, _Event, _State) -> gen_statem:state_function_result(). %%-------------------------------------------------------------------- connection({call, RecvFrom}, {recv, N, Timeout}, - #state{static_env = #static_env{protocol_cb = Connection}, - recv = Recv, + #state{recv = Recv, socket_options = #socket_options{active = false} } = State0) -> passive_receive(State0#state{recv = Recv#recv{from = RecvFrom, bytes_to_read = N}}, - ?STATE(connection), Connection, - [{{timeout, recv}, Timeout, timeout}]); + ?STATE(connection), [{{timeout, recv}, Timeout, timeout}]); connection({call, From}, peer_certificate, #state{session = #session{peer_certificate = Cert}} = State) -> hibernate_after(?STATE(connection), State, [{reply, From, {ok, Cert}}]); @@ -597,6 +595,18 @@ connection({call, From}, negotiated_protocol, negotiated_protocol = undefined}} = State) -> hibernate_after(?STATE(connection), State, [{reply, From, {ok, SelectedProtocol}}]); +connection({call, From}, + {close,{_NewController, _Timeout}}, + #state{static_env = #static_env{role = Role, + socket = Socket, + trackers = Trackers, + transport_cb = Transport, + protocol_cb = Connection}, + connection_env = #connection_env{socket_tls_closed = #alert{} = Alert} + } = State) -> + Pids = Connection:pids(State), + alert_user(Pids, Transport, Trackers, Socket, From, Alert, Role, ?STATE(connection), Connection), + {stop, {shutdown, normal}, State}; connection({call, From}, {close,{NewController, Timeout}}, #state{connection_states = ConnectionStates, @@ -689,9 +699,8 @@ connection(cast, {dist_handshake_complete, DHandle}, connection(info, Msg, #state{static_env = #static_env{protocol_cb = Connection}} = State) -> Connection:handle_info(Msg, ?STATE(connection), State); connection(internal, {recv, RecvFrom}, - #state{recv = #recv{from=RecvFrom}, - static_env = #static_env{protocol_cb = Connection}} = State) -> - passive_receive(State, ?STATE(connection), Connection, []); + #state{recv = #recv{from=RecvFrom}} = State) -> + passive_receive(State, ?STATE(connection), []); connection(Type, Msg, State) -> handle_common_event(Type, Msg, ?STATE(connection), State). @@ -876,7 +885,7 @@ handle_info({ErrorTag, Socket, econnaborted}, StateName, } = State) when StateName =/= connection -> maybe_invalidate_session(Version, Type, Role, Host, Port, Session), Pids = Connection:pids(State), - alert_user(Pids, Transport, Trackers,Socket, + alert_user(Pids, Transport, Trackers, Socket, StartFrom, ?ALERT_REC(?FATAL, ?CLOSE_NOTIFY), Role, StateName, Connection), {stop, {shutdown, normal}, State}; @@ -958,10 +967,22 @@ read_application_data(Data, end end. +passive_receive(#state{static_env = #static_env{role = Role, + socket = Socket, + trackers = Trackers, + transport_cb = Transport, + protocol_cb = Connection}, + recv = #recv{from = RecvFrom}, + connection_env = #connection_env{socket_tls_closed = #alert{} = Alert}} = State, + StateName, _) -> + Pids = Connection:pids(State), + alert_user(Pids, Transport, Trackers, Socket, RecvFrom, Alert, Role, StateName, Connection), + {stop, {shutdown, normal}, State}; passive_receive(#state{user_data_buffer = {Front,BufferSize,Rear}, %% Assert! Erl distribution uses active sockets + static_env = #static_env{protocol_cb = Connection}, connection_env = #connection_env{erl_dist_handle = undefined}} - = State0, StateName, Connection, StartTimerAction) -> + = State0, StateName, StartTimerAction) -> case BufferSize of 0 -> Connection:next_event(StateName, no_record, State0, StartTimerAction); @@ -1396,8 +1417,21 @@ handle_active_option(_, connection = StateName, To, Reply, Alert = ?ALERT_REC(?FATAL, ?CLOSE_NOTIFY, all_data_delivered), handle_normal_shutdown(Alert#alert{role = Role}, StateName, State), {stop_and_reply,{shutdown, peer_close}, [{reply, To, Reply}]}; -handle_active_option(_, connection = StateName0, To, Reply, #state{static_env = #static_env{protocol_cb = Connection}, - user_data_buffer = {_,0,_}} = State0) -> +handle_active_option(_, connection = StateName, To, _Reply, + #state{static_env = #static_env{role = Role, + socket = Socket, + trackers = Trackers, + transport_cb = Transport, + protocol_cb = Connection}, + connection_env = + #connection_env{socket_tls_closed = Alert = #alert{}}, + user_data_buffer = {_,0,_}} = State) -> + Pids = Connection:pids(State), + alert_user(Pids, Transport, Trackers, Socket, To, Alert, Role, StateName, Connection), + {stop, {shutdown, normal}, State}; +handle_active_option(_, connection = StateName0, To, Reply, + #state{static_env = #static_env{protocol_cb = Connection}, + user_data_buffer = {_,0,_}} = State0) -> case Connection:next_event(StateName0, no_record, State0) of {next_state, StateName, State} -> gen_statem:reply(To, Reply), diff --git a/lib/ssl/src/tls_gen_connection.erl b/lib/ssl/src/tls_gen_connection.erl index f5e453a61643..6344bdc86f64 100644 --- a/lib/ssl/src/tls_gen_connection.erl +++ b/lib/ssl/src/tls_gen_connection.erl @@ -908,12 +908,17 @@ handle_alerts([], Result) -> handle_alerts(_, {stop, _, _} = Stop) -> Stop; handle_alerts([#alert{level = ?WARNING, description = ?CLOSE_NOTIFY} | _Alerts], - {next_state, connection = StateName, - #state{connection_env = CEnv, - socket_options = #socket_options{active = false}, - recv = #recv{from = From}} = State}) when From == undefined -> - {next_state, StateName, - State#state{connection_env = CEnv#connection_env{socket_tls_closed = true}}}; + {next_state, connection = StateName, #state{connection_env = CEnv, + socket_options = #socket_options{active = false}, + recv = #recv{from = From}} = State}) when From == undefined -> + %% Linger to allow recv and setopts to possibly fetch data not yet delivered to user to be fetched + {next_state, StateName, State#state{connection_env = CEnv#connection_env{socket_tls_closed = true}}}; +handle_alerts([#alert{level = ?FATAL} = Alert | _Alerts], + {next_state, connection = StateName, #state{connection_env = CEnv, + socket_options = #socket_options{active = false}, + recv = #recv{from = From}} = State}) when From == undefined -> + %% Linger to allow recv and setopts to retrieve alert reason + {next_state, StateName, State#state{connection_env = CEnv#connection_env{socket_tls_closed = Alert}}}; handle_alerts([Alert | Alerts], {next_state, StateName, State}) -> handle_alerts(Alerts, ssl_gen_statem:handle_alert(Alert, StateName, State)); handle_alerts([Alert | Alerts], {next_state, StateName, State, _Actions}) -> diff --git a/lib/ssl/test/tls_1_3_version_SUITE.erl b/lib/ssl/test/tls_1_3_version_SUITE.erl index 2ba02d006ebb..19856b44b674 100644 --- a/lib/ssl/test/tls_1_3_version_SUITE.erl +++ b/lib/ssl/test/tls_1_3_version_SUITE.erl @@ -63,7 +63,11 @@ middle_box_client_tls_v2_session_reused/0, middle_box_client_tls_v2_session_reused/1, renegotiate_error/0, - renegotiate_error/1 + renegotiate_error/1, + client_cert_fail_alert_active/0, + client_cert_fail_alert_active/1, + client_cert_fail_alert_passive/0, + client_cert_fail_alert_passive/1 ]). @@ -99,7 +103,9 @@ tls_1_3_1_2_tests() -> middle_box_tls13_client, middle_box_tls12_enabled_client, middle_box_client_tls_v2_session_reused, - renegotiate_error + renegotiate_error, + client_cert_fail_alert_active, + client_cert_fail_alert_passive ]. legacy_tests() -> [tls_client_tls10_server, @@ -402,6 +408,59 @@ renegotiate_error(Config) when is_list(Config) -> ct:fail(Reason) end. +client_cert_fail_alert_active() -> + [{doc, "Check that we receive alert message"}]. +client_cert_fail_alert_active(Config) when is_list(Config) -> + ssl:clear_pem_cache(), + {_ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), + ClientOpts0 = ssl_test_lib:ssl_options(extra_client, client_cert_opts, Config), + ServerOpts0 = ssl_test_lib:ssl_options(extra_server, server_cert_opts, Config), + PrivDir = proplists:get_value(priv_dir, Config), + + NewClientCertFile = filename:join(PrivDir, "clinet_invalid_cert.pem"), + create_bad_client_certfile(NewClientCertFile, ClientOpts0), + + ClientOpts = [{active, true}, + {verify, verify_peer}, + {certfile, NewClientCertFile} | proplists:delete(certfile, ClientOpts0)], + ServerOpts = [{verify, verify_peer}, {fail_if_no_peer_cert, true}| ServerOpts0], + 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), + {ok, Socket} = ssl:connect(Hostname, Port, ClientOpts), + receive + {Server, {error, {tls_alert, {unknown_ca, _}}}} -> + receive + {ssl_error, Socket, {tls_alert, {unknown_ca, _}}} -> + ok + after 500 -> + ct:fail(no_acticv_msg) + end + end. + +client_cert_fail_alert_passive() -> + [{doc, "Check that recv or setopts return alert"}]. +client_cert_fail_alert_passive(Config) when is_list(Config) -> + ssl:clear_pem_cache(), + {_, ServerNode, Hostname} = ssl_test_lib:run_where(Config), + ClientOpts0 = ssl_test_lib:ssl_options(extra_client, client_cert_opts, Config), + ServerOpts0 = ssl_test_lib:ssl_options(extra_server, server_cert_opts, Config), + PrivDir = proplists:get_value(priv_dir, Config), + + NewClientCertFile = filename:join(PrivDir, "client_invalid_cert.pem"), + create_bad_client_certfile(NewClientCertFile, ClientOpts0), + + ClientOpts = [{active, false}, + {verify, verify_peer}, + {certfile, NewClientCertFile} | proplists:delete(certfile, ClientOpts0)], + ServerOpts = [{verify, verify_peer}, {fail_if_no_peer_cert, true}| ServerOpts0], + alert_passive(ServerOpts, ClientOpts, recv, + ServerNode, Hostname), + alert_passive(ServerOpts, ClientOpts, setopts, + ServerNode, Hostname). + %%-------------------------------------------------------------------- %% Internal functions and callbacks ----------------------------------- %%-------------------------------------------------------------------- @@ -432,4 +491,30 @@ check_session_id(Socket, Expected) -> {nok, {{expected, Expected}, {got, SessionId}}} end. +alert_passive(ServerOpts, ClientOpts, Function, + ServerNode, Hostname) -> + 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), + {ok, Socket} = ssl:connect(Hostname, Port, ClientOpts), + ct:sleep(500), + case Function of + recv -> + {error, {tls_alert, {unknown_ca,_}}} = ssl:recv(Socket, 0); + setopts -> + {error, {tls_alert, {unknown_ca,_}}} = ssl:setopts(Socket, [{active, once}]) + end. +create_bad_client_certfile(NewClientCertFile, ClientOpts) -> + KeyFile = proplists:get_value(keyfile, ClientOpts), + [KeyEntry] = ssl_test_lib:pem_to_der(KeyFile), + Key = ssl_test_lib:public_key(public_key:pem_entry_decode(KeyEntry)), + ClientCertFile = proplists:get_value(certfile, ClientOpts), + + [{'Certificate', ClientDerCert, _}] = ssl_test_lib:pem_to_der(ClientCertFile), + ClientOTPCert = public_key:pkix_decode_cert(ClientDerCert, otp), + ClientOTPTbsCert = ClientOTPCert#'OTPCertificate'.tbsCertificate, + NewClientDerCert = public_key:pkix_sign(ClientOTPTbsCert, Key), + ssl_test_lib:der_to_pem(NewClientCertFile, [{'Certificate', NewClientDerCert, not_encrypted}]).