Skip to content

Commit 4b7a558

Browse files
committed
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
1 parent 3431636 commit 4b7a558

File tree

3 files changed

+34
-10
lines changed

3 files changed

+34
-10
lines changed

lib/ssl/src/ssl_trace.erl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,7 @@ trace_profiles() ->
491491
fun(M, F, A) -> dbg:ctpl(M, F, A) end,
492492
[{tls_gen_connection_1_3, [{handle_key_update, 2}]},
493493
{tls_sender, [{init, 3}, {time_to_rekey, 5},
494-
{send_post_handshake_data, 4}]},
494+
{send_post_handshake_data, 5}]},
495495
{tls_v1, [{update_traffic_secret, 2}]}]},
496496
{rle, %% role
497497
fun(M, F, A) -> dbg:tpl(M, F, A, x) end,

lib/ssl/src/tls_sender.erl

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ connection({call, From}, {application_data, Data}, StateData) ->
282282
connection({call, From}, {post_handshake_data, HSData}, #data{buff = Buff} = StateData) ->
283283
case Buff of
284284
undefined ->
285-
send_post_handshake_data(HSData, From, connection, StateData);
285+
send_post_handshake_data(HSData, From, connection, StateData, [{reply, From, ok}]);
286286
Async ->
287287
{next_state, async_wait, StateData#data{buff = Async#async{low = 0}}, [postpone]}
288288
end;
@@ -326,7 +326,7 @@ connection(internal, {application_packets, From, Data}, StateData) ->
326326
connection(internal, {post_handshake_data, From, HSData}, #data{buff = Buff} = StateData) ->
327327
case Buff of
328328
undefined ->
329-
send_post_handshake_data(HSData, From, connection, StateData);
329+
send_post_handshake_data(HSData, From, connection, StateData, []);
330330
Async ->
331331
{next_state, async_wait, StateData#data{buff = Async#async{low = 0}}, [postpone]}
332332
end;
@@ -546,13 +546,13 @@ send_application_data(Data, From, StateName,
546546
KeyUpdate = tls_handshake_1_3:key_update(update_requested),
547547
case DataSz > KeyUpdateAt of
548548
false ->
549-
{keep_state_and_data, [{next_event, internal, {post_handshake_data, From, KeyUpdate}},
549+
{keep_state_and_data, [{next_event, internal, {post_handshake_data, undefined, KeyUpdate}},
550550
{next_event, internal, {application_packets, From, Data}}]};
551551
true ->
552552
%% Prevent infinite loop of key updates
553553
{Chunk, Rest} = split_binary(iolist_to_binary(Data), KeyUpdateAt),
554-
{keep_state_and_data, [{next_event, internal, {post_handshake_data, From, KeyUpdate}},
555-
{next_event, internal, {application_packets, From, [Chunk]}},
554+
{keep_state_and_data, [{next_event, internal, {post_handshake_data, undefined, KeyUpdate}},
555+
{next_event, internal, {application_packets, undefined, [Chunk]}},
556556
{next_event, internal, {application_packets, From, [Rest]}}]}
557557
end;
558558
{renegotiate, _} ->
@@ -722,7 +722,7 @@ send_post_handshake_data(Handshake, From, StateName,
722722
#data{env = #env{socket = Socket,
723723
negotiated_version = Version,
724724
transport_cb = Transport},
725-
connection_states = ConnStates0} = StateData0) ->
725+
connection_states = ConnStates0} = StateData0, AckAction) ->
726726
BinHandshake = tls_handshake:encode_handshake(Handshake, Version),
727727
{Encoded, ConnStates} =
728728
tls_record:encode_handshake(BinHandshake, Version, ConnStates0),
@@ -737,7 +737,9 @@ send_post_handshake_data(Handshake, From, StateName,
737737
ok ->
738738
ssl_logger:debug(LogLevel, outbound, 'record', Encoded),
739739
StateData = maybe_update_cipher_key(StateData1, Handshake),
740-
{next_state, StateName, StateData, [{reply, From, ok}]};
740+
%% AckAction will send sync message if post_handshake_data
741+
%% was initiated by peer via the receiver process.
742+
{next_state, StateName, StateData, AckAction};
741743
{error, Reason} ->
742744
death_row_shutdown(Reason, StateData1)
743745
end.

lib/ssl/test/ssl_key_update_SUITE.erl

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@
4343
keylog_client_cb/0,
4444
keylog_client_cb/1,
4545
keylog_server_cb/0,
46-
keylog_server_cb/1
46+
keylog_server_cb/1,
47+
key_update_unexpected_msg/0,
48+
key_update_unexpected_msg/1
4749
]).
4850

4951
-include("ssl_test_lib.hrl").
@@ -62,7 +64,8 @@ tls_1_3_tests() ->
6264
key_update_at_server,
6365
explicit_key_update,
6466
keylog_client_cb,
65-
keylog_server_cb].
67+
keylog_server_cb,
68+
key_update_unexpected_msg].
6669

6770
init_per_suite(Config0) ->
6871
case application:ensure_started(crypto) of
@@ -201,6 +204,25 @@ keylog_server_cb(Config) ->
201204
end,
202205
ok = traffic_secret_1_and_2([{client,1}, {client, 2}, {server,1}, {server, 2}]).
203206

207+
208+
key_update_unexpected_msg() ->
209+
[{doc,"Test that internla sync messages are not sent to socket user"}].
210+
key_update_unexpected_msg(Config) ->
211+
Data = "123456789012345", %% 15 bytes
212+
Server = ssl_test_lib:start_server(erlang,[], Config),
213+
Port = ssl_test_lib:inet_port(Server),
214+
215+
{ok, Socket} = ssl:connect(net_adm:localhost(), Port, [{verify, verify_none}, {key_update_at, 9}]),
216+
217+
ok = ssl:send(Socket, Data),
218+
219+
receive
220+
{_, ok} = Msg ->
221+
ct:fail({unexpected_message, Msg})
222+
after 500 ->
223+
ok
224+
end.
225+
204226
%%--------------------------------------------------------------------
205227
%% Internal functions -----------------------------------------------
206228
%%--------------------------------------------------------------------

0 commit comments

Comments
 (0)