Skip to content

Commit c258607

Browse files
author
Erlang/OTP
committed
Merge branch 'ingela/ssl/maint-28/key-update-unexpected-message/OTP-19806' into maint-28
* ingela/ssl/maint-28/key-update-unexpected-message/OTP-19806: ssl: Avoid sending internal message to client
2 parents e8f0740 + 4b7a558 commit c258607

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
@@ -284,7 +284,7 @@ connection({call, From}, {application_data, Data}, StateData) ->
284284
connection({call, From}, {post_handshake_data, HSData}, #data{buff = Buff} = StateData) ->
285285
case Buff of
286286
undefined ->
287-
send_post_handshake_data(HSData, From, connection, StateData);
287+
send_post_handshake_data(HSData, From, connection, StateData, [{reply, From, ok}]);
288288
Async ->
289289
{next_state, async_wait, StateData#data{buff = Async#async{low = 0}}, [postpone]}
290290
end;
@@ -328,7 +328,7 @@ connection(internal, {application_packets, From, Data}, StateData) ->
328328
connection(internal, {post_handshake_data, From, HSData}, #data{buff = Buff} = StateData) ->
329329
case Buff of
330330
undefined ->
331-
send_post_handshake_data(HSData, From, connection, StateData);
331+
send_post_handshake_data(HSData, From, connection, StateData, []);
332332
Async ->
333333
{next_state, async_wait, StateData#data{buff = Async#async{low = 0}}, [postpone]}
334334
end;
@@ -554,13 +554,13 @@ send_application_data(Data, From, StateName,
554554
KeyUpdate = tls_handshake_1_3:key_update(update_requested),
555555
case DataSz > KeyUpdateAt of
556556
false ->
557-
{keep_state_and_data, [{next_event, internal, {post_handshake_data, From, KeyUpdate}},
557+
{keep_state_and_data, [{next_event, internal, {post_handshake_data, undefined, KeyUpdate}},
558558
{next_event, internal, {application_packets, From, Data}}]};
559559
true ->
560560
%% Prevent infinite loop of key updates
561561
{Chunk, Rest} = split_binary(iolist_to_binary(Data), KeyUpdateAt),
562-
{keep_state_and_data, [{next_event, internal, {post_handshake_data, From, KeyUpdate}},
563-
{next_event, internal, {application_packets, From, [Chunk]}},
562+
{keep_state_and_data, [{next_event, internal, {post_handshake_data, undefined, KeyUpdate}},
563+
{next_event, internal, {application_packets, undefined, [Chunk]}},
564564
{next_event, internal, {application_packets, From, [Rest]}}]}
565565
end;
566566
{renegotiate, _} ->
@@ -730,7 +730,7 @@ send_post_handshake_data(Handshake, From, StateName,
730730
#data{env = #env{socket = Socket,
731731
negotiated_version = Version,
732732
transport_cb = Transport},
733-
connection_states = ConnStates0} = StateData0) ->
733+
connection_states = ConnStates0} = StateData0, AckAction) ->
734734
BinHandshake = tls_handshake:encode_handshake(Handshake, Version),
735735
{Encoded, ConnStates} =
736736
tls_record:encode_handshake(BinHandshake, Version, ConnStates0),
@@ -745,7 +745,9 @@ send_post_handshake_data(Handshake, From, StateName,
745745
ok ->
746746
ssl_logger:debug(LogLevel, outbound, 'record', Encoded),
747747
StateData = maybe_update_cipher_key(StateData1, Handshake),
748-
{next_state, StateName, StateData, [{reply, From, ok}]};
748+
%% AckAction will send sync message if post_handshake_data
749+
%% was initiated by peer via the receiver process.
750+
{next_state, StateName, StateData, AckAction};
749751
{error, Reason} ->
750752
death_row_shutdown(Reason, StateData1)
751753
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)