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

Author: IngelaAndin

lib/ssl/src/ssl_trace.erl

@@ -489,7 +489,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, 6},
-                     {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,

lib/ssl/src/tls_sender.erl

@@ -289,7 +289,7 @@ connection({call, From}, {application_data, AppData},
             send_application_data(Data, From, connection, StateData)
     end;
 connection({call, From}, {post_handshake_data, HSData}, StateData) ->
-    send_post_handshake_data(HSData, From, connection, StateData);
+    send_post_handshake_data(HSData, From, connection, StateData, [{reply, From, ok}]);
 connection({call, From}, {ack_alert, #alert{} = Alert}, StateData0) ->
     StateData = send_tls_alert(Alert, StateData0),
     {next_state, connection, StateData,
@@ -340,7 +340,7 @@ connection({call, From}, get_application_traffic_secret, #data{env = #env{num_ke
 connection(internal, {application_packets, From, Data}, StateData) ->
     send_application_data(Data, From, connection, StateData);
 connection(internal, {post_handshake_data, From, HSData}, StateData) ->
-    send_post_handshake_data(HSData, From, connection, StateData);
+    send_post_handshake_data(HSData, From, connection, StateData, []);
 connection(cast, #alert{} = Alert, StateData0) ->
     StateData = send_tls_alert(Alert, StateData0),
     {next_state, connection, StateData};
@@ -439,6 +439,9 @@ death_row(_Type, _Msg, _StateData) ->
     keep_state_and_data.
 
 %% State entry function that starts shutdown state_timeout
+%% distribution otherwise shuts down the sender
+death_row_shutdown(Reason, #data{env = #env{dist_handle = false}} = StateData) ->
+    {stop, {shutdown, Reason}, StateData};
 death_row_shutdown(Reason, StateData) ->
     {next_state, death_row, StateData, [{state_timeout, 5000, Reason}]}.
 
@@ -532,7 +535,7 @@ send_application_data(Data, From, StateName,
     case time_to_rekey(Version, DataSz, ConnectionStates0, RenegotiateAt, KeyUpdateAt, BytesSent) of
         key_update ->
             KeyUpdate = tls_handshake_1_3:key_update(update_requested),
-            {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}}]};
 	renegotiate ->
 	    tls_dtls_gen_connection:internal_renegotiation(Pid, ConnectionStates0),
@@ -542,8 +545,8 @@ send_application_data(Data, From, StateName,
             KeyUpdate = tls_handshake_1_3:key_update(update_requested),
             %% 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]}}]};
 	false ->
 	    {Msgs, ConnectionStates} = tls_record:encode_data(Data, Version, ConnectionStates0),
@@ -552,29 +555,29 @@ send_application_data(Data, From, StateName,
                     ssl_logger:debug(LogLevel, outbound, 'record', Msgs),
                     StateData1 = update_bytes_sent(Version, ConnectionStates, StateData0, DataSz),
                     hibernate_after(StateName, StateData1, []);
-                Reason when DistHandle =/= undefined ->
-                    StateData = StateData0#data{connection_states = ConnectionStates},
-                    death_row_shutdown(Reason, StateData);
                 ok ->
                     ssl_logger:debug(LogLevel, outbound, 'record', Msgs),
                     StateData = update_bytes_sent(Version, ConnectionStates, StateData0, DataSz),
-                    gen_statem:reply(From, ok),
+                    send_reply(From, ok),
                     hibernate_after(StateName, StateData, []);
                 Result ->
-                    gen_statem:reply(From, Result),
+                    send_reply(From, Result),
                     StateData = StateData0#data{connection_states = ConnectionStates},
                     hibernate_after(StateName, StateData, [])
             end
     end.
 
+send_reply(undefined, _Msg) -> ok;
+send_reply(From, Msg) -> gen_statem:reply(From, Msg).
+
 %% TLS 1.3 Post Handshake Data
-send_post_handshake_data(Handshake, From, StateName,
+send_post_handshake_data(Handshake, _From, StateName,
                          #data{env = #env{socket = Socket,
-                                                dist_handle = DistHandle,
-                                                negotiated_version = Version,
-                                                transport_cb = Transport,
-                                                log_level = LogLevel},
-                               connection_states = ConnectionStates0} = StateData0) ->
+                                          dist_handle = DistHandle,
+                                          negotiated_version = Version,
+                                          transport_cb = Transport,
+                                          log_level = LogLevel},
+                               connection_states = ConnectionStates0} = StateData0, AckAction) ->
     BinHandshake = tls_handshake:encode_handshake(Handshake, Version),
     {Encoded, ConnectionStates} =
         tls_record:encode_handshake(BinHandshake, Version, ConnectionStates0),
@@ -584,15 +587,13 @@ send_post_handshake_data(Handshake, From, StateName,
         ok when DistHandle =/=  undefined ->
             ssl_logger:debug(LogLevel, outbound, 'record', Encoded),
             StateData = maybe_update_cipher_key(StateData1, Handshake),
-            {next_state, StateName, StateData, []};
-        Reason when DistHandle =/= undefined ->
-            death_row_shutdown(Reason, StateData1);
+            {next_state, StateName, StateData, AckAction};
         ok ->
             ssl_logger:debug(LogLevel, outbound, 'record', Encoded),
             StateData = maybe_update_cipher_key(StateData1, Handshake),
-            {next_state, StateName, StateData,  [{reply, From, ok}]};
-        Result ->
-            {next_state, StateName, StateData1,  [{reply, From, Result}]}
+            {next_state, StateName, StateData,  AckAction};
+        {error, Reason} ->
+            death_row_shutdown(Reason, StateData1)
     end.
 
 maybe_update_cipher_key(#data{connection_states = ConnectionStates0,

lib/ssl/test/ssl_key_update_SUITE.erl

@@ -41,7 +41,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").
@@ -59,7 +61,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
@@ -198,6 +201,27 @@ 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  -----------------------------------------------
+%%--------------------------------------------------------------------
 traffic_secret_1_and_2([]) ->
     ok;
 traffic_secret_1_and_2([_|_] = List) ->