Skip to content

Commit b1c2b69

Browse files
IngelaAndinIngelaAndin
committed
Merge branch 'ingela/ssl/alert-encrypt-tls-1.3/OTP-19759' into maint
* ingela/ssl/alert-encrypt-tls-1.3/OTP-19759: ssl: Improve maintainability and correct keylogging ssl: Move lists:reverse call into get_handshake_context ssl: Move main logic to to top function ssl: Handle encryption state stepping properly
2 parents 422f3bd + 9a1a89e commit b1c2b69

7 files changed

Lines changed: 457 additions & 338 deletions

‎lib/ssl/src/ssl_gen_statem.erl‎

Lines changed: 102 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1929,16 +1929,16 @@ connection_info(#state{handshake_env = #handshake_env{sni_hostname = SNIHostname
19291929
{sni_hostname, SNIHostname},
19301930
{srp_username, SrpUsername} | CurveInfo] ++ MFLInfo ++ ssl_options_list(Opts).
19311931

1932-
security_info(#state{connection_states = ConnectionStates,
1932+
security_info(#state{connection_states = #{current_read := Read,
1933+
current_write := Write},
19331934
static_env = #static_env{role = Role},
19341935
connection_env = #connection_env{negotiated_version = Version},
19351936
ssl_options = Opts,
19361937
protocol_specific = ProtocolSpecific}) ->
1937-
ReadState = ssl_record:current_connection_state(ConnectionStates, read),
19381938
#{security_parameters :=
19391939
#security_parameters{client_random = ClientRand,
19401940
server_random = ServerRand,
1941-
master_secret = MasterSecret}} = ReadState,
1941+
master_secret = MasterSecret}} = Read,
19421942
SecInfo = [{client_random, ClientRand},
19431943
{server_random, ServerRand}, {master_secret, MasterSecret}],
19441944
KeepSecrets = maps:get(keep_secrets, Opts, false),
@@ -1948,30 +1948,31 @@ security_info(#state{connection_states = ConnectionStates,
19481948
%% to be able to run in user process context
19491949
[{keep_secrets, false} | SecInfo];
19501950
_ -> %% true or keylog fun tuple
1951-
maybe_security_info_1_3(Version, ReadState, ProtocolSpecific, Role)
1951+
maybe_security_info_1_3(Version, Read, Write, ProtocolSpecific, Role)
19521952
++ SecInfo
19531953
end.
19541954

1955-
maybe_security_info_1_3(Version, #{security_parameters :=
1956-
#security_parameters{application_traffic_secret = AppTrafSecretRead,
1957-
client_early_data_secret = EarlyData
1958-
}} = ReadState,
1959-
ProtocolSpecific, Role) when ?TLS_GTE(Version, ?TLS_1_3)->
1955+
maybe_security_info_1_3(Version,
1956+
#{security_parameters :=
1957+
#security_parameters{application_traffic_secret = AppTrafSecretRead,
1958+
client_early_data_secret = EarlyData
1959+
}} = Read, Write,
1960+
ProtocolSpecific, Role) when ?TLS_GTE(Version, ?TLS_1_3)->
19601961
N = maps:get(num_key_updates, ProtocolSpecific, 0),
19611962
Sender = maps:get(sender, ProtocolSpecific, undefined),
19621963
case Role of
19631964
server ->
19641965
tls_handshake_1_3:early_data_secret(EarlyData) ++
1965-
tls_handshake_1_3:hs_traffic_secrets(ReadState) ++
1966+
tls_handshake_1_3:hs_traffic_secrets(Read, Write) ++
19661967
[{client_traffic_secret, AppTrafSecretRead, N},
19671968
{Role, Sender}];
19681969
client ->
19691970
tls_handshake_1_3:early_data_secret(EarlyData) ++
1970-
tls_handshake_1_3:hs_traffic_secrets(ReadState) ++
1971+
tls_handshake_1_3:hs_traffic_secrets(Read, Write) ++
19711972
[{server_traffic_secret, AppTrafSecretRead, N},
19721973
{Role, Sender}]
19731974
end;
1974-
maybe_security_info_1_3(_,_,_,_) ->
1975+
maybe_security_info_1_3(_,_,_,_,_) ->
19751976
[].
19761977

19771978
record_cb(tls) ->
@@ -2243,56 +2244,111 @@ maybe_keylog_hs_callback(#alert{level = ?FATAL}, StateName,
22432244
end;
22442245
maybe_keylog_hs_callback(_, _, _) ->
22452246
ok.
2246-
2247+
22472248
keylog_hs_alert(start, _) -> %% TLS 1.3: No secrets yet established
22482249
{[], undefined};
22492250
keylog_hs_alert(wait_sh, _) -> %% TLS 1.3: No secrets yet established
22502251
{[], undefined};
2251-
%% Server alert for certificate validation can happen when client is in connection state already.
2252-
keylog_hs_alert(connection, #state{static_env = #static_env{role = client},
2253-
connection_env =
2254-
#connection_env{negotiated_version = TlsVersion},
2255-
connection_states = ConnectionStates,
2256-
protocol_specific = #{sender := Sender} = PS})
2252+
keylog_hs_alert(negotiated, #state{static_env = #static_env{role = server},
2253+
connection_env =
2254+
#connection_env{negotiated_version = TlsVersion},
2255+
connection_states = #{current_read := Read,
2256+
current_write := Write
2257+
}})
22572258
when ?TLS_GTE(TlsVersion, ?TLS_1_3) ->
2258-
CSRead = #{security_parameters :=
2259-
#security_parameters{application_traffic_secret = SecretRead,
2260-
client_random = ClientRandom,
2261-
prf_algorithm = Prf}}
2262-
= ssl_record:current_connection_state(ConnectionStates, read),
2263-
NRead = maps:get(num_key_updates, PS, 0),
2264-
{ok, SecretWrite, NWrite} = call(Sender, get_application_traffic_secret),
2265-
{keylog_hs_1_3(CSRead) ++
2266-
ssl_logger:keylog_traffic_1_3(client, ClientRandom, Prf, SecretWrite, NWrite) ++
2267-
ssl_logger:keylog_traffic_1_3(server, ClientRandom, Prf, SecretRead, NRead), ClientRandom};
2268-
keylog_hs_alert(_, #state{connection_env =
2259+
#{server_handshake_traffic_secret := ServerHSSecret,
2260+
security_parameters :=
2261+
#security_parameters{client_random = ClientRandomBin,
2262+
client_early_data_secret = EarlySecret,
2263+
prf_algorithm = Prf
2264+
}} = Write,
2265+
#{client_handshake_traffic_secret := ClientHSSecret} = Read,
2266+
{keylog_hs_1_3(ClientRandomBin, Prf, EarlySecret, ServerHSSecret, ClientHSSecret),
2267+
ClientRandomBin};
2268+
keylog_hs_alert(wait_eoed, #state{static_env = #static_env{role = server},
2269+
connection_env =
2270+
#connection_env{negotiated_version = TlsVersion},
2271+
connection_states = #{pending_read := Read,
2272+
current_write := Write
2273+
}})
2274+
when ?TLS_GTE(TlsVersion, ?TLS_1_3) ->
2275+
keylog_1_3_client_finished(Read, Write);
2276+
keylog_hs_alert(StateName, #state{static_env = #static_env{role = server},
2277+
connection_states =
2278+
#{current_read := Read,
2279+
current_write := Write
2280+
}}) when StateName == wait_cert;
2281+
StateName == wait_cv;
2282+
StateName == wait_finished ->
2283+
keylog_1_3_client_finished(Read, Write);
2284+
keylog_hs_alert(connection, #state{static_env = #static_env{role = client},
2285+
connection_env =
2286+
#connection_env{negotiated_version = TlsVersion},
2287+
connection_states = #{current_read := Read,
2288+
current_write := Write
2289+
}})
2290+
when ?TLS_GTE(TlsVersion, ?TLS_1_3) ->
2291+
#{server_handshake_traffic_secret := ServerHSSecret,
2292+
security_parameters :=
2293+
#security_parameters{client_random = ClientRandomBin,
2294+
client_early_data_secret = EarlySecret,
2295+
prf_algorithm = Prf,
2296+
master_secret = {master_secret, STrafficSecret}
2297+
}}
2298+
= Read,
2299+
#{client_handshake_traffic_secret := ClientHSSecret,
2300+
security_parameters :=
2301+
#security_parameters{master_secret = {master_secret, CTrafficSecret}
2302+
}} = Write,
2303+
2304+
{keylog_hs_1_3(ClientRandomBin, Prf, EarlySecret, ClientHSSecret, ServerHSSecret) ++
2305+
ssl_logger:keylog_traffic_1_3(client, ClientRandomBin, Prf, CTrafficSecret, 0) ++
2306+
ssl_logger:keylog_traffic_1_3(server, ClientRandomBin, Prf, STrafficSecret, 0),
2307+
ClientRandomBin};
2308+
keylog_hs_alert(_, #state{static_env = #static_env{role = client},
2309+
connection_env =
22692310
#connection_env{negotiated_version = TlsVersion},
2270-
connection_states = ConnectionStates})
2311+
connection_states = #{current_read := Read,
2312+
current_write := Write
2313+
}})
22712314
when ?TLS_GTE(TlsVersion, ?TLS_1_3) ->
2272-
CSRead = #{security_parameters :=
2273-
#security_parameters{client_random = ClientRandom}}
2274-
= ssl_record:pending_connection_state(ConnectionStates, read),
2275-
{keylog_hs_1_3(CSRead), ClientRandom};
2276-
keylog_hs_alert(_,_) -> % NOT Relevant pre TLS-1.3
2315+
#{server_handshake_traffic_secret := ServerHSSecret,
2316+
security_parameters :=
2317+
#security_parameters{client_random = ClientRandomBin,
2318+
client_early_data_secret = EarlySecret,
2319+
prf_algorithm = Prf
2320+
}}
2321+
= Write,
2322+
#{client_handshake_traffic_secret := ClientHSSecret} = Read,
2323+
{keylog_hs_1_3(ClientRandomBin, Prf, EarlySecret, ClientHSSecret, ServerHSSecret),
2324+
ClientRandomBin};
2325+
keylog_hs_alert(_, _) -> % NOT Relevant pre TLS-1.3
22772326
{[], undefined}.
22782327

2279-
keylog_hs_1_3(#{client_handshake_traffic_secret := ClientHSTrafficSecret,
2280-
server_handshake_traffic_secret := ServerHSTrafficSecret,
2281-
security_parameters := #security_parameters{client_random = ClientRandomBin,
2282-
prf_algorithm = Prf,
2283-
client_early_data_secret = EarlySecret}}) ->
2328+
keylog_hs_1_3(ClientRandomBin, Prf, EarlySecret, ClientSecret, ServerSecret) ->
22842329
HSItems =
2285-
ssl_logger:keylog_hs(ClientRandomBin, Prf, ClientHSTrafficSecret, ServerHSTrafficSecret),
2330+
ssl_logger:keylog_hs(ClientRandomBin, Prf, ClientSecret, ServerSecret),
22862331
case EarlySecret of
22872332
undefined ->
22882333
HSItems;
22892334
_ ->
22902335
[ssl_logger:keylog_early_data(ClientRandomBin, Prf, EarlySecret) | HSItems]
2291-
end;
2292-
keylog_hs_1_3(_) ->
2293-
[].
2336+
end.
2337+
2338+
keylog_1_3_client_finished(Read, Write) ->
2339+
#{server_handshake_traffic_secret := ServerHSSecret,
2340+
security_parameters :=
2341+
#security_parameters{client_random = ClientRandomBin,
2342+
client_early_data_secret = EarlySecret,
2343+
prf_algorithm = Prf,
2344+
master_secret = {master_secret, TrafficSecret}
2345+
}}
2346+
= Write,
2347+
#{client_handshake_traffic_secret := ClientHSSecret} = Read,
2348+
{keylog_hs_1_3(ClientRandomBin, Prf, EarlySecret, ServerHSSecret, ClientHSSecret) ++
2349+
ssl_logger:keylog_traffic_1_3(server, ClientRandomBin, Prf, TrafficSecret, 0),
2350+
ClientRandomBin}.
22942351

2295-
22962352
%%%################################################################
22972353
%%%#
22982354
%%%# Tracing

‎lib/ssl/src/tls_client_connection_1_3.erl‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -463,11 +463,13 @@ wait_finished(internal,
463463
State4 = Connection:queue_handshake(Finished, State3),
464464
%% Send first flight
465465
{State5, _} = Connection:send_handshake_flight(State4),
466-
State6 = tls_handshake_1_3:handle_secrets(State5),
467-
%% Configure traffic keys
468-
State7 = ssl_record:step_encryption_state(State6),
466+
State6 = tls_handshake_1_3:calculate_write_traffic_secrets(State5),
467+
State7 = tls_handshake_1_3:calculate_read_traffic_secrets(State6),
468+
State8 = tls_handshake_1_3:maybe_calculate_resumption_master_secret(State7),
469+
State9 = ssl_record:step_encryption_state(State8),
470+
State10 = tls_handshake_1_3:prepare_connection(State9),
469471
{Record, State} =
470-
ssl_gen_statem:prepare_connection(State7, tls_gen_connection),
472+
ssl_gen_statem:prepare_connection(State10, tls_gen_connection),
471473
KeepSecrets = maps:get(keep_secrets, SSLOpts, false),
472474
tls_gen_connection_1_3:maybe_traffic_keylog_1_3(KeepSecrets, Role,
473475
State#state.connection_states, 0),
@@ -486,6 +488,19 @@ wait_finished(Type, Msg, State) ->
486488
term(), #state{}) ->
487489
gen_statem:state_function_result().
488490
%%--------------------------------------------------------------------
491+
connection(enter, _, #state{ssl_options = Opts} = State) ->
492+
case maps:get(keep_secrets, Opts, undefined) of
493+
{keylog_hs, _} ->
494+
%% Mitigate consequences of keylog_hs being activated, as
495+
%% this forces the client to remember secrets longer, that
496+
%% is the client certification can fail after the client
497+
%% reached connection state.
498+
{next_state, ?STATE(connection), State, [{timeout, 1000, forget}]};
499+
_ ->
500+
{keep_state, State}
501+
end;
502+
connection(timeout, forget, State) ->
503+
{next_state, ?STATE(connection), tls_handshake_1_3:forget_master_secret(State)};
489504
connection(info, Msg, State) ->
490505
tls_gen_connection:gen_info(Msg, connection, State);
491506
connection(Type, Msg, State) ->

‎lib/ssl/src/tls_gen_connection_1_3.erl‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
do_maybe/0]).
5555

5656
%%--------------------------------------------------------------------
57-
%% Internal API
57+
%% Internal API
5858
%%--------------------------------------------------------------------
5959
initial_state(Role, Sender, Tab, Host, Port, Socket,
6060
{SSLOptions, SocketOptions, Trackers}, User,
@@ -255,13 +255,17 @@ handle_resumption(#state{handshake_env = HSEnv0} = State, _) ->
255255
HSEnv = HSEnv0#handshake_env{resumption = true},
256256
State#state{handshake_env = HSEnv}.
257257

258-
maybe_forget_hs_secrets({keylog_hs, _}, #state{connection_states = CS} = State) ->
258+
maybe_forget_hs_secrets({keylog_hs, _}, #state{connection_states =
259+
#{current_read := Read0,
260+
current_write := Write0} = CS} = State) ->
259261
%% Server finishes the handshake last and is able to immediately forget
260262
%% hs secrets used for handshake alert logging.
261-
Read0 = #{security_parameters := SecParams} = ssl_record:current_connection_state(CS, read),
262-
Read1 = Read0#{security_parameters => SecParams#security_parameters{client_early_data_secret = undefined}},
263-
Read = maps:without([client_handshake_traffic_secret, server_handshake_traffic_secret], Read1),
264-
State#state{connection_states = CS#{current_read => Read}};
263+
#{security_parameters := SecParams} = Read0,
264+
Read1 = Read0#{security_parameters =>
265+
SecParams#security_parameters{client_early_data_secret = undefined}},
266+
Read = maps:without([client_handshake_traffic_secret], Read1),
267+
Write = maps:without([server_handshake_traffic_secret], Write0),
268+
State#state{connection_states = CS#{current_read => Read, current_write => Write}};
265269
maybe_forget_hs_secrets(_, State) ->
266270
State.
267271

0 commit comments

Comments
 (0)