From d41dba75ec1a905d72078a5e5980e4e769dd5699 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Fri, 10 Oct 2025 09:46:31 +0200 Subject: [PATCH] ssl: Avoid sending internal message to client When key_update is initiated by the TLS sender process, and not by the peer, no internal ack is needed for post_handshake_data event to the reciver process. Previously an ack message was incorrectly sent to socket user process instead of just doing nothing when that happened. closes #10273 --- lib/ssl/src/ssl_trace.erl | 2 +- lib/ssl/src/tls_sender.erl | 16 +++++++++------- lib/ssl/test/ssl_key_update_SUITE.erl | 26 ++++++++++++++++++++++++-- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/lib/ssl/src/ssl_trace.erl b/lib/ssl/src/ssl_trace.erl index 8f5a483bf702..79ed0d97649e 100644 --- a/lib/ssl/src/ssl_trace.erl +++ b/lib/ssl/src/ssl_trace.erl @@ -491,7 +491,7 @@ trace_profiles() -> fun(M, F, A) -> dbg:ctpl(M, F, A) end, [{tls_gen_connection_1_3, [{handle_key_update, 2}]}, {tls_sender, [{init, 3}, {time_to_rekey, 5}, - {send_post_handshake_data, 4}]}, + {send_post_handshake_data, 5}]}, {tls_v1, [{update_traffic_secret, 2}]}]}, {rle, %% role fun(M, F, A) -> dbg:tpl(M, F, A, x) end, diff --git a/lib/ssl/src/tls_sender.erl b/lib/ssl/src/tls_sender.erl index 32f1c3200da0..d8c34ebc77b1 100644 --- a/lib/ssl/src/tls_sender.erl +++ b/lib/ssl/src/tls_sender.erl @@ -284,7 +284,7 @@ connection({call, From}, {application_data, Data}, StateData) -> connection({call, From}, {post_handshake_data, HSData}, #data{buff = Buff} = StateData) -> case Buff of undefined -> - send_post_handshake_data(HSData, From, connection, StateData); + send_post_handshake_data(HSData, From, connection, StateData, [{reply, From, ok}]); Async -> {next_state, async_wait, StateData#data{buff = Async#async{low = 0}}, [postpone]} end; @@ -328,7 +328,7 @@ connection(internal, {application_packets, From, Data}, StateData) -> connection(internal, {post_handshake_data, From, HSData}, #data{buff = Buff} = StateData) -> case Buff of undefined -> - send_post_handshake_data(HSData, From, connection, StateData); + send_post_handshake_data(HSData, From, connection, StateData, []); Async -> {next_state, async_wait, StateData#data{buff = Async#async{low = 0}}, [postpone]} end; @@ -554,13 +554,13 @@ send_application_data(Data, From, StateName, KeyUpdate = tls_handshake_1_3:key_update(update_requested), case DataSz > KeyUpdateAt of false -> - {keep_state_and_data, [{next_event, internal, {post_handshake_data, From, KeyUpdate}}, + {keep_state_and_data, [{next_event, internal, {post_handshake_data, undefined, KeyUpdate}}, {next_event, internal, {application_packets, From, Data}}]}; true -> %% Prevent infinite loop of key updates {Chunk, Rest} = split_binary(iolist_to_binary(Data), KeyUpdateAt), - {keep_state_and_data, [{next_event, internal, {post_handshake_data, From, KeyUpdate}}, - {next_event, internal, {application_packets, From, [Chunk]}}, + {keep_state_and_data, [{next_event, internal, {post_handshake_data, undefined, KeyUpdate}}, + {next_event, internal, {application_packets, undefined, [Chunk]}}, {next_event, internal, {application_packets, From, [Rest]}}]} end; {renegotiate, _} -> @@ -730,7 +730,7 @@ send_post_handshake_data(Handshake, From, StateName, #data{env = #env{socket = Socket, negotiated_version = Version, transport_cb = Transport}, - connection_states = ConnStates0} = StateData0) -> + connection_states = ConnStates0} = StateData0, AckAction) -> BinHandshake = tls_handshake:encode_handshake(Handshake, Version), {Encoded, ConnStates} = tls_record:encode_handshake(BinHandshake, Version, ConnStates0), @@ -745,7 +745,9 @@ send_post_handshake_data(Handshake, From, StateName, ok -> ssl_logger:debug(LogLevel, outbound, 'record', Encoded), StateData = maybe_update_cipher_key(StateData1, Handshake), - {next_state, StateName, StateData, [{reply, From, ok}]}; + %% AckAction will send sync message if post_handshake_data + %% was initiated by peer via the receiver process. + {next_state, StateName, StateData, AckAction}; {error, Reason} -> death_row_shutdown(Reason, StateData1) end. diff --git a/lib/ssl/test/ssl_key_update_SUITE.erl b/lib/ssl/test/ssl_key_update_SUITE.erl index 95ea110306cc..2b16e4e41de7 100644 --- a/lib/ssl/test/ssl_key_update_SUITE.erl +++ b/lib/ssl/test/ssl_key_update_SUITE.erl @@ -43,7 +43,9 @@ keylog_client_cb/0, keylog_client_cb/1, keylog_server_cb/0, - keylog_server_cb/1 + keylog_server_cb/1, + key_update_unexpected_msg/0, + key_update_unexpected_msg/1 ]). -include("ssl_test_lib.hrl"). @@ -62,7 +64,8 @@ tls_1_3_tests() -> key_update_at_server, explicit_key_update, keylog_client_cb, - keylog_server_cb]. + keylog_server_cb, + key_update_unexpected_msg]. init_per_suite(Config0) -> case application:ensure_started(crypto) of @@ -201,6 +204,25 @@ keylog_server_cb(Config) -> end, ok = traffic_secret_1_and_2([{client,1}, {client, 2}, {server,1}, {server, 2}]). + +key_update_unexpected_msg() -> + [{doc,"Test that internla sync messages are not sent to socket user"}]. +key_update_unexpected_msg(Config) -> + Data = "123456789012345", %% 15 bytes + Server = ssl_test_lib:start_server(erlang,[], Config), + Port = ssl_test_lib:inet_port(Server), + + {ok, Socket} = ssl:connect(net_adm:localhost(), Port, [{verify, verify_none}, {key_update_at, 9}]), + + ok = ssl:send(Socket, Data), + + receive + {_, ok} = Msg -> + ct:fail({unexpected_message, Msg}) + after 500 -> + ok + end. + %%-------------------------------------------------------------------- %% Internal functions ----------------------------------------------- %%--------------------------------------------------------------------