From 6aeeadc093f7ec020b4ef2061417c049c6efc800 Mon Sep 17 00:00:00 2001 From: Jakub Witczak Date: Wed, 22 Feb 2023 17:31:31 +0100 Subject: ssl: fix keylog mechanism - fetch application traffic secret from TLS sender process --- lib/ssl/src/ssl_gen_statem.erl | 32 ++++++++-- lib/ssl/src/tls_sender.erl | 7 +++ lib/ssl/test/ssl_key_update_SUITE.erl | 111 +++++++++++++++++++--------------- 3 files changed, 94 insertions(+), 56 deletions(-) diff --git a/lib/ssl/src/ssl_gen_statem.erl b/lib/ssl/src/ssl_gen_statem.erl index 36768ab6c7..62c09c60ae 100644 --- a/lib/ssl/src/ssl_gen_statem.erl +++ b/lib/ssl/src/ssl_gen_statem.erl @@ -1897,7 +1897,8 @@ connection_info(#state{handshake_env = #handshake_env{sni_hostname = SNIHostname security_info(#state{connection_states = ConnectionStates, static_env = #static_env{role = Role}, - ssl_options = #{keep_secrets := KeepSecrets}}) -> + ssl_options = #{keep_secrets := KeepSecrets}, + protocol_specific = ProtocolSpecific}) -> ReadState = ssl_record:current_connection_state(ConnectionStates, read), #{security_parameters := #security_parameters{client_random = ClientRand, @@ -1911,10 +1912,12 @@ security_info(#state{connection_states = ConnectionStates, BaseSecurityInfo; true -> #{security_parameters := - #security_parameters{application_traffic_secret = AppTrafSecretWrite, - client_early_data_secret = ClientEarlyData - }} = + #security_parameters{ + application_traffic_secret = AppTrafSecretWrite0, + client_early_data_secret = ClientEarlyData}} = ssl_record:current_connection_state(ConnectionStates, write), + Sender = maps:get(sender, ProtocolSpecific, undefined), + AppTrafSecretWrite = {Sender, AppTrafSecretWrite0}, if Role == server -> if ServerEarlyData =/= undefined -> [{server_traffic_secret_0, AppTrafSecretWrite}, @@ -2118,8 +2121,25 @@ maybe_add_keylog({_, 'tlsv1.2'}, Info) -> maybe_add_keylog({_, 'tlsv1.3'}, Info) -> try {client_random, ClientRandomBin} = lists:keyfind(client_random, 1, Info), - {client_traffic_secret_0, ClientTrafficSecret0Bin} = lists:keyfind(client_traffic_secret_0, 1, Info), - {server_traffic_secret_0, ServerTrafficSecret0Bin} = lists:keyfind(server_traffic_secret_0, 1, Info), + %% after traffic key update current traffic secret + %% is stored in tls_sender process state + MaybeUpdateTrafficSecret = + fun({Direction, {Sender, TrafficSecret0}}) -> + TrafficSecret = + case call(Sender, get_application_traffic_secret) of + {ok, SenderAppTrafSecretWrite} -> + SenderAppTrafSecretWrite; + _ -> + TrafficSecret0 + end, + {Direction, TrafficSecret}; + (TrafficSecret0) -> + TrafficSecret0 + end, + {client_traffic_secret_0, ClientTrafficSecret0Bin} = + MaybeUpdateTrafficSecret(lists:keyfind(client_traffic_secret_0, 1, Info)), + {server_traffic_secret_0, ServerTrafficSecret0Bin} = + MaybeUpdateTrafficSecret(lists:keyfind(server_traffic_secret_0, 1, Info)), {client_handshake_traffic_secret, ClientHSecretBin} = lists:keyfind(client_handshake_traffic_secret, 1, Info), {server_handshake_traffic_secret, ServerHSecretBin} = lists:keyfind(server_handshake_traffic_secret, 1, Info), {selected_cipher_suite, #{prf := Prf}} = lists:keyfind(selected_cipher_suite, 1, Info), diff --git a/lib/ssl/src/tls_sender.erl b/lib/ssl/src/tls_sender.erl index b53a801b90..b531b51819 100644 --- a/lib/ssl/src/tls_sender.erl +++ b/lib/ssl/src/tls_sender.erl @@ -313,6 +313,13 @@ connection({call, From}, {dist_handshake_complete, _Node, DHandle}, {next_event, internal, {application_packets, {self(),undefined}, Data}}]} end; +connection({call, From}, get_application_traffic_secret, State) -> + CurrentWrite = maps:get(current_write, State#data.connection_states), + SecurityParams = maps:get(security_parameters, CurrentWrite), + ApplicationTrafficSecret = + SecurityParams#security_parameters.application_traffic_secret, + hibernate_after(?FUNCTION_NAME, State, + [{reply, From, {ok, ApplicationTrafficSecret}}]); connection(internal, {application_packets, From, Data}, StateData) -> send_application_data(Data, From, ?FUNCTION_NAME, StateData); connection(internal, {post_handshake_data, From, HSData}, StateData) -> diff --git a/lib/ssl/test/ssl_key_update_SUITE.erl b/lib/ssl/test/ssl_key_update_SUITE.erl index baa408dbc5..0ac3228f85 100644 --- a/lib/ssl/test/ssl_key_update_SUITE.erl +++ b/lib/ssl/test/ssl_key_update_SUITE.erl @@ -42,7 +42,6 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("ssl/src/ssl_api.hrl"). -include_lib("ssl/src/ssl_connection.hrl"). - all() -> [{group, 'tlsv1.3'}]. @@ -107,24 +106,22 @@ key_update_at_server(Config) -> key_update_at(Config, Role) -> Data = "123456789012345", %% 15 bytes Server = ssl_test_lib:start_server(erlang, - [{options, [{key_update_at, 14}]}], + [{options, [{keep_secrets, true}, + {key_update_at, 14}]}], Config), Port = ssl_test_lib:inet_port(Server), - {Client, - #sslsocket{pid = - [ClientReceiverPid, ClientSenderPid]}} = - ssl_test_lib:start_client(erlang, - [return_socket, {port, Port}, - {options, [{key_update_at, 14}]}], - Config), + ClientResult = ssl_test_lib:start_client(erlang, + [return_socket, {port, Port}, + {options, [{keep_secrets, true}, + {key_update_at, 14}]}], + Config), + {Client, ClientSocket} = ClientResult, Server ! get_socket, - #sslsocket{pid = - [ServerReceiverPid, ServerSenderPid]} = - receive - {Server, {socket, S}} -> S - end, - Keys0 = get_keys(ClientReceiverPid, ClientSenderPid, - ServerReceiverPid, ServerSenderPid), + ServerSocket = receive + {Server, {socket, S}} -> S + end, + Keys0 = get_traffic_secrets(ClientSocket, ServerSocket), + ct:log("connected", []), {Sender, Receiver} = case Role of client -> {Client, Server}; server -> {Server, Client} @@ -134,53 +131,67 @@ key_update_at(Config, Role) -> Data = ssl_test_lib:check_active_receive(Receiver, Data), %% TODO check if key has been updated (needs debug logging of secrets) ct:sleep(500), - Keys1 = get_keys(ClientReceiverPid, ClientSenderPid, - ServerReceiverPid, ServerSenderPid), + ct:log("sent and waited", []), + Keys1 = get_traffic_secrets(ClientSocket, ServerSocket), verify_key_update(Keys0, Keys1), %% Test mechanism to prevent infinite loop of key updates BigData = binary:copy(<<"1234567890">>, 10), %% 100 bytes ok = ssl_test_lib:send(Sender, BigData), ct:sleep(500), - Keys2 = get_keys(ClientReceiverPid, ClientSenderPid, - ServerReceiverPid, ServerSenderPid), + ct:log("sent and waited 2", []), + Keys2 = get_traffic_secrets(ClientSocket, ServerSocket), verify_key_update(Keys1, Keys2), ssl_test_lib:close(Server), ssl_test_lib:close(Client). -get_keys(ClientReceiverPid, ClientSenderPid, - ServerReceiverPid, ServerSenderPid) -> - F = fun(Pid) -> - {connection, D} = sys:get_state(Pid), - M0 = element(3, D), - Cr = maps:get(current_write, M0), - {Pid, {maps:get(security_parameters, Cr), - maps:get(cipher_state, Cr)}} +get_traffic_secrets(ClientSocket, ServerSocket) -> + ProcessSocket = + fun(Socket, Role) -> + {ok, [{keylog, KeyLog}]} = ssl:connection_information(Socket, [keylog]), + Interesting = + fun(S) -> + Patterns = ["CLIENT_TRAFFIC_SECRET", "SERVER_TRAFFIC_SECRET"], + SearchResults = [string:find(S, P) || P <- Patterns], + lists:any(fun(I) -> I /= nomatch end, SearchResults) + end, + TrafficSecrets = lists:filter(Interesting, KeyLog), + Print = fun(Secret) -> + [Name, _A, B] = string:lexemes(Secret, " "), + [Key] = io_lib:format("~s", [B]), + {Name, {Role, Key}} + end, + [Print(Scr) || Scr <- TrafficSecrets] end, - SendersKeys = [F(P) || P <- [ClientSenderPid, ServerSenderPid]], - - G = fun(Pid) -> - {connection, D} = sys:get_state(Pid), - #state{connection_states = Cs} = D, - Cr = maps:get(current_read, Cs), - {Pid, {maps:get(security_parameters,Cr), - maps:get(cipher_state, Cr)}} + Secrets = lists:flatten( + [ProcessSocket(S, R) || + {S, R} <- + [{ClientSocket, client}, {ServerSocket, server}]]), + P = fun(Direction) -> + Vals = proplists:get_all_values(Direction, Secrets), + ct:log("~30s ~10s(c) ~10s(s)", + [Direction, proplists:get_value(client, Vals), + proplists:get_value(server, Vals)]), + {Direction, [proplists:get_value(client, Vals), + proplists:get_value(server, Vals)]} end, - ReceiversKeys = [G(P) || P <- [ClientReceiverPid, ServerReceiverPid]], - maps:from_list(SendersKeys ++ ReceiversKeys). + [P(Direction) || + Direction <- + ["CLIENT_TRAFFIC_SECRET_0", "SERVER_TRAFFIC_SECRET_0"]]. verify_key_update(Keys0, Keys1) -> - V = fun(Pid, CurrentKeys) -> - BaseKeys = maps:get(Pid, Keys0), - ct:log("Pid = ~p~nBaseKeys = ~p~nCurrentKeys = ~p", - [Pid, BaseKeys, CurrentKeys], [esc_chars]), - case BaseKeys == CurrentKeys of - true -> - ct:fail("Keys don't differ for ~w", [Pid]); - false -> - ok - end - end, - maps:foreach(V, Keys1). + CTS0 = proplists:get_value("CLIENT_TRAFFIC_SECRET_0", Keys0), + CTS1 = proplists:get_value("CLIENT_TRAFFIC_SECRET_0", Keys1), + STS0 = proplists:get_value("SERVER_TRAFFIC_SECRET_0", Keys0), + STS1 = proplists:get_value("SERVER_TRAFFIC_SECRET_0", Keys1), + CTS = lists:zip(CTS0, CTS1), + STS = lists:zip(STS0, STS1), + Pred = fun({A, B}) when A == B -> + ct:fail(no_key_update), + false; + (_) -> + true + end, + [true = lists:all(Pred, X) || X <- [CTS, STS]]. explicit_key_update() -> [{doc,"Test ssl:update_key/2 between erlang client and erlang server."}]. -- cgit v1.2.1