From a0df7d07a55a2de04d1239551f549a3d4e57d9d2 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Wed, 17 Jan 2024 09:22:50 +0100 Subject: [PATCH 01/20] Bench: make client use active once Probably a good idea, since that is probably what users will use. --- lib/ssl/test/ssl_bench_SUITE.erl | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/ssl/test/ssl_bench_SUITE.erl b/lib/ssl/test/ssl_bench_SUITE.erl index 41970ba66bf9..95957879d3c4 100644 --- a/lib/ssl/test/ssl_bench_SUITE.erl +++ b/lib/ssl/test/ssl_bench_SUITE.erl @@ -380,12 +380,22 @@ setup_connection(_, _, _) -> ok. payload(Loop, ssl, D = {Socket, Size}) when Loop > 0 -> + ssl:setopts(Socket, [{active, once}]), ok = ssl:send(Socket, msg()), - {ok, _} = ssl:recv(Socket, Size), + fetch_data(Socket, Size), payload(Loop-1, ssl, D); payload(_, _, {Socket, _}) -> ssl:close(Socket). +fetch_data(Socket, Size) -> + receive + {ssl, Socket, Bin} -> + case Size - size(Bin) of + 0 -> ok; + N -> fetch_data(Socket, N) + end + end. + msg() -> <<"Hello", 0:(512*8), From 6d6a0f3f6e6b29d9ae8189d3f69a9b7a5aeb9e0c Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Mon, 22 Jan 2024 12:56:38 +0100 Subject: [PATCH 02/20] Bench: Use tprof for memory profiling --- lib/ssl/test/ssl_bench_SUITE.erl | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/ssl/test/ssl_bench_SUITE.erl b/lib/ssl/test/ssl_bench_SUITE.erl index 95957879d3c4..4403fb066f3a 100644 --- a/lib/ssl/test/ssl_bench_SUITE.erl +++ b/lib/ssl/test/ssl_bench_SUITE.erl @@ -155,6 +155,9 @@ count(Config) -> -define(EPROF_CLIENT, false). -define(EPROF_SERVER, false). +-define(TPROF_CLIENT, false). +-define(TPROF_SERVER, false). %% untested + %% Current numbers gives roughly a testcase per minute on todays hardware.. setup_sequential(Config) -> @@ -276,11 +279,15 @@ do_test(Type, {Func, _}=TC, Loop, ParallellConnections, Server) -> start_profile(fprof, [self(),new]), ?EPROF_CLIENT andalso Id =:= 1 andalso start_profile(eprof, [ssl_connection_sup, ssl_manager]), + ?TPROF_CLIENT andalso Id =:= 1 andalso + start_profile(tprof, []), ok = ?MODULE:Func(Loop, Type, CData), ?FPROF_CLIENT andalso Id =:= 1 andalso stop_profile(fprof, "test_connection_client_res.fprof"), ?EPROF_CLIENT andalso Id =:= 1 andalso stop_profile(eprof, "test_connection_client_res.eprof"), + ?TPROF_CLIENT andalso Id =:= 1 andalso + stop_profile(tprof, "test_connection_client_res.tprof"), Me ! self() end end, @@ -313,6 +320,7 @@ server_init(ssl, {setup_connection, Opts}, _, _, Server, Certs) -> ?FPROF_SERVER andalso start_profile(fprof, [whereis(ssl_manager), new]), %%?EPROF_SERVER andalso start_profile(eprof, [ssl_connection_sup, ssl_manager]), ?EPROF_SERVER andalso start_profile(eprof, [ssl_manager]), + ?TPROF_SERVER andalso start_profile(tprof, [ssl_manager]), Server ! {self(), {init, Host, Port}}, Test = fun(TSocket) -> {ok, Socket} = ssl:handshake(TSocket), @@ -352,6 +360,7 @@ setup_server_connection(LSocket, Test) -> receive quit -> ?FPROF_SERVER andalso stop_profile(fprof, "test_server_res.fprof"), ?EPROF_SERVER andalso stop_profile(eprof, "test_server_res.eprof"), + ?TPROF_SERVER andalso stop_profile(tprof, "test_server_res.tprof"), ok after 0 -> case ssl:transport_accept(LSocket, 2000) of @@ -447,7 +456,12 @@ start_profile(eprof, Procs) -> io:format("(E)Profiling ...",[]); start_profile(fprof, Procs) -> fprof:trace([start, {procs, Procs}]), - io:format("(F)Profiling ...",[]). + io:format("(F)Profiling ...",[]); +start_profile(tprof, _) -> + tprof:start(#{type => call_memory}), + tprof:enable_trace({all_children, ssl_sup}), + io:format("(T)Profiling ...",[]), + tprof:set_pattern('_', '_' , '_'). stop_profile(eprof, File) -> profiling_stopped = eprof:stop_profiling(), @@ -462,7 +476,11 @@ stop_profile(fprof, File) -> fprof:analyse([{dest, File},{totals, true}]), io:format(".analysed => ~s ~n",[File]), fprof:stop(), - ok. + ok; +stop_profile(tprof, _File) -> + Sample = tprof:collect(), + tprof:stop(), + tprof:format(tprof:inspect(Sample)). ssl_opts(listen, Opts, Certs) -> [{backlog, 500} | ssl_opts(server_config, Opts, Certs)]; From 1d5b2ffb4b4f0e8a0fa15680d37fb0190e8499e7 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Wed, 17 Jan 2024 09:19:51 +0100 Subject: [PATCH 03/20] Garbage collect after conn setup Be sure to remove cleanup memory usage after connection setup. --- lib/ssl/src/dtls_gen_connection.erl | 7 +++++-- lib/ssl/src/tls_gen_connection.erl | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/ssl/src/dtls_gen_connection.erl b/lib/ssl/src/dtls_gen_connection.erl index 100dff6bce70..f2ae085b1ca7 100644 --- a/lib/ssl/src/dtls_gen_connection.erl +++ b/lib/ssl/src/dtls_gen_connection.erl @@ -558,9 +558,12 @@ queue_change_cipher(ChangeCipher, #state{flight_buffer = Flight, State#state{flight_buffer = Flight#{change_cipher_spec => ChangeCipher}, connection_states = ConnectionStates}. -reinit(State) -> +reinit(State0) -> %% To be API compatible with TLS NOOP here - reinit_handshake_data(State). + State = reinit_handshake_data(State0), + garbage_collect(), + State. + reinit_handshake_data(#state{static_env = #static_env{data_tag = DataTag}, protocol_buffers = Buffers, protocol_specific = PS, diff --git a/lib/ssl/src/tls_gen_connection.erl b/lib/ssl/src/tls_gen_connection.erl index dcf100a370ac..4300241053e0 100644 --- a/lib/ssl/src/tls_gen_connection.erl +++ b/lib/ssl/src/tls_gen_connection.erl @@ -222,9 +222,11 @@ queue_change_cipher(Msg, #state{connection_env = #connection_env{negotiated_vers reinit(#state{protocol_specific = #{sender := Sender}, connection_env = #connection_env{negotiated_version = Version}, - connection_states = #{current_write := Write}} = State) -> + connection_states = #{current_write := Write}} = State0) -> tls_sender:update_connection_state(Sender, Write, Version), - reinit_handshake_data(State). + State = reinit_handshake_data(State0), + garbage_collect(), + State. reinit_handshake_data(#state{handshake_env = HsEnv} =State) -> %% premaster_secret, public_key_info and tls_handshake_info From d4bf7d1b00d325bd3fdb5ebf06b96eff7e64bde1 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Tue, 16 May 2023 09:51:45 +0200 Subject: [PATCH 04/20] Remove unused log_level field --- lib/ssl/src/ssl_connection.hrl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ssl/src/ssl_connection.hrl b/lib/ssl/src/ssl_connection.hrl index f7486e31c1e6..a5a4f7f3644e 100644 --- a/lib/ssl/src/ssl_connection.hrl +++ b/lib/ssl/src/ssl_connection.hrl @@ -133,8 +133,7 @@ user_data_buffer :: undefined | {[binary()],non_neg_integer(),[binary()]} | secret_printout(), bytes_to_read :: undefined | integer(), %% bytes to read in passive mode %% recv and start handling - start_or_recv_from :: term(), - log_level + start_or_recv_from :: term() }). -define(DEFAULT_DIFFIE_HELLMAN_PARAMS, From 28f7e80233718f9f42fe8863370f7e99ee14d22c Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Thu, 25 May 2023 12:12:02 +0200 Subject: [PATCH 05/20] Move max_fragment_size Move it from connection_state to connection_stats map and make it optional. Slight memory saving. --- lib/ssl/src/dtls_gen_connection.erl | 8 +++---- lib/ssl/src/dtls_record.erl | 12 +++------- lib/ssl/src/ssl_gen_statem.erl | 4 ++-- lib/ssl/src/ssl_handshake.erl | 2 +- lib/ssl/src/ssl_record.erl | 22 +++--------------- lib/ssl/src/tls_gen_connection.erl | 2 +- lib/ssl/src/tls_record.erl | 36 +++++++++++------------------ lib/ssl/src/tls_record_1_3.erl | 21 ++++------------- 8 files changed, 31 insertions(+), 76 deletions(-) diff --git a/lib/ssl/src/dtls_gen_connection.erl b/lib/ssl/src/dtls_gen_connection.erl index f2ae085b1ca7..81dd1a05cb68 100644 --- a/lib/ssl/src/dtls_gen_connection.erl +++ b/lib/ssl/src/dtls_gen_connection.erl @@ -320,7 +320,7 @@ send_handshake_flight(#state{static_env = #static_env{socket = Socket, connection_states = ConnectionStates0, ssl_options = #{log_level := LogLevel}} = State0, Epoch) -> - #{current_write := #{max_fragment_length := MaxFragmentLength}} = ConnectionStates0, + MaxFragmentLength = maps:get(max_fragment_length, ConnectionStates0, undefined), MaxSize = mtu(MaxFragmentLength, DataTag), {Encoded, ConnectionStates} = encode_handshake_flight(lists:reverse(Flight), Version, MaxSize, Epoch, ConnectionStates0), @@ -338,7 +338,7 @@ send_handshake_flight(#state{static_env = #static_env{socket = Socket, connection_states = ConnectionStates0, ssl_options = #{log_level := LogLevel}} = State0, Epoch) -> - #{current_write := #{max_fragment_length := MaxFragmentLength}} = ConnectionStates0, + MaxFragmentLength = maps:get(max_fragment_length, ConnectionStates0, undefined), MaxSize = mtu(MaxFragmentLength, DataTag), {HsBefore, ConnectionStates1} = encode_handshake_flight(lists:reverse(Flight0), Version, MaxSize, Epoch, ConnectionStates0), @@ -358,7 +358,7 @@ send_handshake_flight(#state{static_env = #static_env{socket = Socket, connection_states = ConnectionStates0, ssl_options = #{log_level := LogLevel}} = State0, Epoch) -> - #{current_write := #{max_fragment_length := MaxFragmentLength}} = ConnectionStates0, + MaxFragmentLength = maps:get(max_fragment_length, ConnectionStates0, undefined), MaxSize = mtu(MaxFragmentLength, DataTag), {HsBefore, ConnectionStates1} = encode_handshake_flight(lists:reverse(Flight0), Version, MaxSize, Epoch-1, ConnectionStates0), @@ -382,7 +382,7 @@ send_handshake_flight(#state{static_env = #static_env{socket = Socket, connection_states = ConnectionStates0, ssl_options = #{log_level := LogLevel}} = State0, Epoch) -> - #{current_write := #{max_fragment_length := MaxFragmentLength}} = ConnectionStates0, + MaxFragmentLength = maps:get(max_fragment_length, ConnectionStates0, undefined), MaxSize = mtu(MaxFragmentLength, DataTag), {EncChangeCipher, ConnectionStates1} = encode_change_cipher(ChangeCipher, Version, Epoch-1, ConnectionStates0), diff --git a/lib/ssl/src/dtls_record.erl b/lib/ssl/src/dtls_record.erl index 4a4fa5cf7f92..100dafae19cb 100644 --- a/lib/ssl/src/dtls_record.erl +++ b/lib/ssl/src/dtls_record.erl @@ -223,13 +223,8 @@ encode_change_cipher_spec(Version, Epoch, ConnectionStates) -> %% Description: Encodes data to send on the ssl-socket. %%-------------------------------------------------------------------- encode_data(Data, Version, ConnectionStates) -> - #{epoch := Epoch, max_fragment_length := MaxFragmentLength} - = ssl_record:current_connection_state(ConnectionStates, write), - MaxLength = if is_integer(MaxFragmentLength) -> - MaxFragmentLength; - true -> - ?MAX_PLAIN_TEXT_LENGTH - end, + #{epoch := Epoch} = ssl_record:current_connection_state(ConnectionStates, write), + MaxLength = maps:get(max_fragment_length, ConnectionStates, ?MAX_PLAIN_TEXT_LENGTH), case iolist_size(Data) of N when N > MaxLength -> Frags = tls_record:split_iovec(erlang:iolist_to_iovec(Data), MaxLength), @@ -417,8 +412,7 @@ initial_connection_state(ConnectionEnd, BeastMitigation) -> mac_secret => undefined, secure_renegotiation => undefined, client_verify_data => undefined, - server_verify_data => undefined, - max_fragment_length => undefined + server_verify_data => undefined }. get_dtls_records_aux({DataTag, StateName, _, Versions} = Vinfo, diff --git a/lib/ssl/src/ssl_gen_statem.erl b/lib/ssl/src/ssl_gen_statem.erl index ea6f356a32fb..f38b390ac92c 100644 --- a/lib/ssl/src/ssl_gen_statem.erl +++ b/lib/ssl/src/ssl_gen_statem.erl @@ -1840,7 +1840,7 @@ connection_info(#state{handshake_env = #handshake_env{sni_hostname = SNIHostname cipher_suite = CipherSuite, srp_username = SrpUsername, ecc = ECCCurve} = Session, - connection_states = #{current_write := CurrentWrite}, + connection_states = ConnectionStates, connection_env = #connection_env{negotiated_version = {_,_} = Version}, ssl_options = #{protocol := Protocol} = Opts}) -> RecordCB = record_cb(Protocol), @@ -1853,7 +1853,7 @@ connection_info(#state{handshake_env = #handshake_env{sni_hostname = SNIHostname _ -> [] end, - MFLInfo = case maps:get(max_fragment_length, CurrentWrite, undefined) of + MFLInfo = case maps:get(max_fragment_length, ConnectionStates, undefined) of MaxFragmentLength when is_integer(MaxFragmentLength) -> [{max_fragment_length, MaxFragmentLength}]; _ -> diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index e1195e3fba71..60e48266cfc1 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -1505,7 +1505,7 @@ handle_server_hello_extensions(RecordCB, Random, CipherSuite, %% RFC 6066: handle received/expected maximum fragment length if IsNew -> ServerMaxFragEnum = maps:get(max_frag_enum, Exts, undefined), - #{current_write := #{max_fragment_length := ConnMaxFragLen}} = ConnectionStates, + ConnMaxFragLen = maps:get(max_fragment_length, ConnectionStates0, undefined), ClientMaxFragEnum = max_frag_enum(ConnMaxFragLen), if ServerMaxFragEnum == ClientMaxFragEnum -> diff --git a/lib/ssl/src/ssl_record.erl b/lib/ssl/src/ssl_record.erl index fcb6b10afb7f..8433610de5fa 100644 --- a/lib/ssl/src/ssl_record.erl +++ b/lib/ssl/src/ssl_record.erl @@ -222,29 +222,14 @@ set_renegotiation_flag(Flag, #{current_read := CurrentRead0, %% %% Description: Set maximum fragment length in all connection states %%-------------------------------------------------------------------- -set_max_fragment_length(#max_frag_enum{enum = MaxFragEnum}, - #{current_read := CurrentRead0, - current_write := CurrentWrite0, - pending_read := PendingRead0, - pending_write := PendingWrite0} - = ConnectionStates) when (MaxFragEnum == 1) orelse - (MaxFragEnum == 2) orelse - (MaxFragEnum == 3) orelse - (MaxFragEnum == 4) - -> +set_max_fragment_length(#max_frag_enum{enum = MaxFragEnum}, ConnectionStates) + when is_integer(MaxFragEnum), 1 =< MaxFragEnum, MaxFragEnum =< 4 -> MaxFragmentLength = if MaxFragEnum == 1 -> ?MAX_FRAGMENT_LENGTH_BYTES_1; MaxFragEnum == 2 -> ?MAX_FRAGMENT_LENGTH_BYTES_2; MaxFragEnum == 3 -> ?MAX_FRAGMENT_LENGTH_BYTES_3; MaxFragEnum == 4 -> ?MAX_FRAGMENT_LENGTH_BYTES_4 end, - CurrentRead = CurrentRead0#{max_fragment_length => MaxFragmentLength}, - CurrentWrite = CurrentWrite0#{max_fragment_length => MaxFragmentLength}, - PendingRead = PendingRead0#{max_fragment_length => MaxFragmentLength}, - PendingWrite = PendingWrite0#{max_fragment_length => MaxFragmentLength}, - ConnectionStates#{current_read => CurrentRead, - current_write => CurrentWrite, - pending_read => PendingRead, - pending_write => PendingWrite}; + ConnectionStates#{max_fragment_length => MaxFragmentLength}; set_max_fragment_length(_,ConnectionStates) -> ConnectionStates. @@ -462,7 +447,6 @@ empty_connection_state(ConnectionEnd, Version, client_verify_data => undefined, server_verify_data => undefined, pending_early_data_size => MaxEarlyDataSize, - max_fragment_length => undefined, trial_decryption => false, early_data_accepted => false }. diff --git a/lib/ssl/src/tls_gen_connection.erl b/lib/ssl/src/tls_gen_connection.erl index 4300241053e0..864703f8e84b 100644 --- a/lib/ssl/src/tls_gen_connection.erl +++ b/lib/ssl/src/tls_gen_connection.erl @@ -658,7 +658,7 @@ next_tls_record(Data, StateName, _ -> State0#state.connection_env#connection_env.negotiated_version end, - #{current_write := #{max_fragment_length := MaxFragLen}} = State0#state.connection_states, + MaxFragLen = maps:get(max_fragment_length, State0#state.connection_states, undefined), SslOpts = maps:put(downgrade, Downgrade, SslOpts0), case tls_record:get_tls_records(Data, Versions, Buf0, MaxFragLen, SslOpts) of {Records, Buf1} -> diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl index cd6e2503739f..8af13e04dd4c 100644 --- a/lib/ssl/src/tls_record.erl +++ b/lib/ssl/src/tls_record.erl @@ -108,10 +108,10 @@ init_connection_states(Role, Version, BeastMitigation, MaxEarlyDataSize) -> Buffer0 :: binary() | {'undefined' | #ssl_tls{}, {[binary()],non_neg_integer(),[binary()]}}, tls_max_frag_len(), ssl_options()) -> - {Records :: [#ssl_tls{}], - Buffer :: {'undefined' | #ssl_tls{}, {[binary()],non_neg_integer(),[binary()]}}} | - #alert{}. -%% + {Records :: [#ssl_tls{}], + Buffer :: {'undefined' | #ssl_tls{}, {[binary()],non_neg_integer(),[binary()]}}} | + #alert{}. +%% %% and returns it as a list of binaries also returns leftover %% Description: Given old buffer and new data from TCP, packs up a records %% data @@ -133,18 +133,13 @@ get_tls_records(Data, Versions, {Hdr, {Front,Size,Rear}}, MaxFragLen, SslOpts) - %%-------------------------------------------------------------------- encode_handshake(Frag, ?TLS_1_3, ConnectionStates) -> tls_record_1_3:encode_handshake(Frag, ConnectionStates); -encode_handshake(Frag, Version, +encode_handshake(Frag, Version, #{current_write := #{beast_mitigation := BeastMitigation, - max_fragment_length := MaxFragmentLength, - security_parameters := - #security_parameters{bulk_cipher_algorithm = BCA}}} = + security_parameters := + #security_parameters{bulk_cipher_algorithm = BCA}}} = ConnectionStates) -> - MaxLength = if is_integer(MaxFragmentLength) -> - MaxFragmentLength; - true -> - ?MAX_PLAIN_TEXT_LENGTH - end, + MaxLength = maps:get(max_fragment_length, ConnectionStates, ?MAX_PLAIN_TEXT_LENGTH), case iolist_size(Frag) of N when N > MaxLength -> Data = split_iovec(erlang:iolist_to_iovec(Frag), Version, BCA, BeastMitigation, MaxLength), @@ -184,16 +179,12 @@ encode_change_cipher_spec(Version, ConnectionStates) -> encode_data(Data, ?TLS_1_3, ConnectionStates) -> tls_record_1_3:encode_data(Data, ConnectionStates); encode_data(Data, Version, - #{current_write := #{beast_mitigation := BeastMitigation, - max_fragment_length := MaxFragmentLength, - security_parameters := - #security_parameters{bulk_cipher_algorithm = BCA}}} = + #{current_write := + #{beast_mitigation := BeastMitigation, + security_parameters := + #security_parameters{bulk_cipher_algorithm = BCA}}} = ConnectionStates) -> - MaxLength = if is_integer(MaxFragmentLength) -> - MaxFragmentLength; - true -> - ?MAX_PLAIN_TEXT_LENGTH - end, + MaxLength = maps:get(max_fragment_length, ConnectionStates, ?MAX_PLAIN_TEXT_LENGTH), Fragments = split_iovec(Data, Version, BCA, BeastMitigation, MaxLength), encode_fragments(?APPLICATION_DATA, Version, Fragments, ConnectionStates). @@ -479,7 +470,6 @@ initial_connection_state(ConnectionEnd, BeastMitigation, MaxEarlyDataSize) -> client_verify_data => undefined, server_verify_data => undefined, pending_early_data_size => MaxEarlyDataSize, - max_fragment_length => undefined, trial_decryption => false, early_data_expected => false }. diff --git a/lib/ssl/src/tls_record_1_3.erl b/lib/ssl/src/tls_record_1_3.erl index eed02111a2ff..9d86bbf31af6 100644 --- a/lib/ssl/src/tls_record_1_3.erl +++ b/lib/ssl/src/tls_record_1_3.erl @@ -46,15 +46,8 @@ % %% Description: Encodes a handshake message to send on the tls-1.3-socket. %%-------------------------------------------------------------------- -encode_handshake(Frag, #{current_write := - #{max_fragment_length := MaxFragmentLength}} = - ConnectionStates) -> - MaxLength = if is_integer(MaxFragmentLength) -> - MaxFragmentLength; - true -> - %% TODO: Consider padding here - ?MAX_PLAIN_TEXT_LENGTH - end, +encode_handshake(Frag,ConnectionStates) -> + MaxLength = maps:get(max_fragment_length, ConnectionStates, ?MAX_PLAIN_TEXT_LENGTH), case iolist_size(Frag) of N when N > MaxLength -> Data = tls_record:split_iovec(erlang:iolist_to_iovec(Frag), MaxLength), @@ -79,14 +72,8 @@ encode_alert_record(#alert{level = Level, description = Description}, %% %% Description: Encodes data to send on the ssl-socket. %%-------------------------------------------------------------------- -encode_data(Frag, #{current_write := - #{max_fragment_length := MaxFragmentLength}} = - ConnectionStates) -> - MaxLength = if is_integer(MaxFragmentLength) -> - MaxFragmentLength; - true -> - ?MAX_PLAIN_TEXT_LENGTH - end, +encode_data(Frag, ConnectionStates) -> + MaxLength = maps:get(max_fragment_length, ConnectionStates, ?MAX_PLAIN_TEXT_LENGTH), Data = tls_record:split_iovec(Frag, MaxLength), encode_iolist(?APPLICATION_DATA, Data, ConnectionStates). From 4e662c3f1f731e61cf23ee0f2e7c270687e2f75d Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Fri, 12 Jan 2024 15:18:31 +0100 Subject: [PATCH 06/20] Remove unecessary record creation Reducing memory allocation. --- lib/ssl/src/tls_record_1_3.erl | 100 ++++++++++++++------------------- lib/ssl/src/tls_record_1_3.hrl | 5 -- 2 files changed, 42 insertions(+), 63 deletions(-) diff --git a/lib/ssl/src/tls_record_1_3.erl b/lib/ssl/src/tls_record_1_3.erl index 9d86bbf31af6..5bb6cad8d731 100644 --- a/lib/ssl/src/tls_record_1_3.erl +++ b/lib/ssl/src/tls_record_1_3.erl @@ -77,22 +77,18 @@ encode_data(Frag, ConnectionStates) -> Data = tls_record:split_iovec(Frag, MaxLength), encode_iolist(?APPLICATION_DATA, Data, ConnectionStates). -encode_plain_text(Type, Data0, #{current_write := Write0} = - ConnectionStates) -> +encode_plain_text(Type, Data, ConnectionStates) -> PadLen = 0, %% TODO where to specify PadLen? - Data = inner_plaintext(Type, Data0, PadLen), - CipherFragment = encode_plain_text(Data, Write0), - {CipherText, Write} = encode_tls_cipher_text(CipherFragment, Write0), - {CipherText, ConnectionStates#{current_write => Write}}. + encode_plain_text(Type, Data, PadLen, ConnectionStates). -encode_iolist(Type, Data, ConnectionStates0) -> - {ConnectionStates, EncodedMsg} = - lists:foldl(fun(Text, {CS0, Encoded}) -> - {Enc, CS1} = - encode_plain_text(Type, Text, CS0), - {CS1, [Enc | Encoded]} - end, {ConnectionStates0, []}, Data), - {lists:reverse(EncodedMsg), ConnectionStates}. +encode_iolist(Type, Data, ConnectionStates) -> + encode_iolist(Type, Data, ConnectionStates, []). + +encode_iolist(Type, [Text|Rest], CS0, Encoded) -> + {Enc, CS1} = encode_plain_text(Type, Text, CS0), + encode_iolist(Type, Rest, CS1, [Enc|Encoded]); +encode_iolist(_Type, [], CS, Encoded) -> + {lists:reverse(Encoded), CS}. %%==================================================================== %% Decoding @@ -250,47 +246,40 @@ process_early_data(ConnectionStates0, ReadState0, PendingMaxEarlyDataSize0, Seq, end end. -inner_plaintext(Type, Data, Length) -> - #inner_plaintext{ - content = Data, - type = Type, - zeros = zero_padding(Length) - }. -zero_padding(Length)-> - binary:copy(<>, Length). - -encode_plain_text(#inner_plaintext{ - content = Data, - type = Type, - zeros = Zeros - }, #{cipher_state := #cipher_state{key= Key, - iv = IV, - tag_len = TagLen}, - sequence_number := Seq, - security_parameters := - #security_parameters{ - cipher_type = ?AEAD, - bulk_cipher_algorithm = BulkCipherAlgo} - }) -> - PlainText = [Data, Type, Zeros], - Encoded = cipher_aead(PlainText, BulkCipherAlgo, Key, Seq, IV, TagLen), +encode_plain_text(Type, Data, 0, + #{current_write := + #{cipher_state := + #cipher_state{key= Key, + iv = IV, + tag_len = TagLen}, + sequence_number := Seq, + security_parameters := + #security_parameters{ + cipher_type = ?AEAD, + bulk_cipher_algorithm = BulkCipherAlgo} + } = Write} = CS) -> + %% Pad = <<0:(Length*8)>>, + TLSInnerPlainText = [Data, Type], %% ++ Pad (currently always zero) + Encoded = cipher_aead(TLSInnerPlainText, BulkCipherAlgo, Key, Seq, IV, TagLen), %% 23 (application_data) for outward compatibility - #tls_cipher_text{opaque_type = ?OPAQUE_TYPE, - legacy_version = ?LEGACY_VERSION, - encoded_record = Encoded}; -encode_plain_text(#inner_plaintext{ - content = Data, - type = Type - }, #{security_parameters := - #security_parameters{ - cipher_suite = ?TLS_NULL_WITH_NULL_NULL} - }) -> + { + encode_tls_cipher_text(?OPAQUE_TYPE, ?LEGACY_VERSION, Encoded), + CS#{current_write := Write#{sequence_number := Seq+1}} + }; +encode_plain_text(Type, Data, 0, + #{current_write := + #{sequence_number := Seq, + security_parameters := + #security_parameters{ + cipher_suite = ?TLS_NULL_WITH_NULL_NULL} + } = Write} = CS) -> %% RFC8446 - 5.1. Record Layer %% When record protection has not yet been engaged, TLSPlaintext %% structures are written directly onto the wire. - #tls_cipher_text{opaque_type = Type, - legacy_version = ?TLS_1_2, - encoded_record = Data}. + { + encode_tls_cipher_text(Type, ?TLS_1_2, Data), + CS#{current_write := Write#{sequence_number := Seq+1}} + }. additional_data(Length) -> <>. @@ -317,14 +306,9 @@ cipher_aead(Fragment, BulkCipherAlgo, Key, Seq, IV, TagLen) -> ssl_cipher:aead_encrypt(BulkCipherAlgo, Key, Nonce, Fragment, AAD, TagLen), <>. -encode_tls_cipher_text(#tls_cipher_text{opaque_type = Type, - legacy_version = Version, - encoded_record = Encoded}, - #{sequence_number := Seq} = Write) -> +encode_tls_cipher_text(Type, {MajVer,MinVer}, Encoded) -> Length = erlang:iolist_size(Encoded), - {MajVer,MinVer} = Version, - {[<>, Encoded], - Write#{sequence_number => Seq +1}}. + [<>, Encoded]. decipher_aead(CipherFragment, BulkCipherAlgo, Key, Seq, IV, TagLen) -> try diff --git a/lib/ssl/src/tls_record_1_3.hrl b/lib/ssl/src/tls_record_1_3.hrl index c2624e8fde15..99a5d82461e7 100644 --- a/lib/ssl/src/tls_record_1_3.hrl +++ b/lib/ssl/src/tls_record_1_3.hrl @@ -46,11 +46,6 @@ -define(LEGACY_VERSION, ?TLS_1_2). -define(OPAQUE_TYPE, 23). --record(inner_plaintext, { - content, %% data - type, %% Contentype - zeros %% padding "uint8 zeros[length_of_padding]" - }). -record(tls_cipher_text, { %% Equivalent of encrypted version of #ssl_tls from previous versions %% decrypted version will still use #ssl_tls for code reuse purposes %% with real values for content type and version From 293db3c1a268c08d133eb614c73cd2729b7606a9 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Wed, 17 Jan 2024 09:22:03 +0100 Subject: [PATCH 07/20] Optimize some binary handling --- lib/ssl/src/ssl_handshake.erl | 14 ++-- lib/ssl/src/tls_record_1_3.erl | 67 ++++++------------- lib/ssl/test/openssl_session_ticket_SUITE.erl | 2 +- 3 files changed, 28 insertions(+), 55 deletions(-) diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index 60e48266cfc1..3c436a126849 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -260,8 +260,8 @@ key_exchange(client, _Version, {srp, PublicKey}) -> key_exchange(server, Version, {dh, {PublicKey, _}, #'DHParameter'{prime = P, base = G}, HashSign, ClientRandom, ServerRandom, PrivateKey}) -> - ServerDHParams = #server_dh_params{dh_p = int_to_bin(P), - dh_g = int_to_bin(G), dh_y = PublicKey}, + ServerDHParams = #server_dh_params{dh_p = binary:encode_unsigned(P), + dh_g = binary:encode_unsigned(G), dh_y = PublicKey}, enc_server_key_exchange(Version, ServerDHParams, HashSign, ClientRandom, ServerRandom, PrivateKey); @@ -283,8 +283,8 @@ key_exchange(server, Version, {dhe_psk, PskIdentityHint, {PublicKey, _}, HashSign, ClientRandom, ServerRandom, PrivateKey}) -> ServerEDHPSKParams = #server_dhe_psk_params{ hint = PskIdentityHint, - dh_params = #server_dh_params{dh_p = int_to_bin(P), - dh_g = int_to_bin(G), dh_y = PublicKey} + dh_params = #server_dh_params{dh_p = binary:encode_unsigned(P), + dh_g = binary:encode_unsigned(G), dh_y = PublicKey} }, enc_server_key_exchange(Version, ServerEDHPSKParams, HashSign, ClientRandom, ServerRandom, PrivateKey); @@ -1972,10 +1972,6 @@ certificate_authorities(CertDbHandle, CertDbRef) -> %%-------------------------------------------------------------------- %%------------- Create handshake messages ---------------------------- -int_to_bin(I) -> - L = (length(integer_to_list(I, 16)) + 1) div 2, - <>. - %% The end-entity certificate provided by the client MUST contain a %% key that is compatible with certificate_types. certificate_types(Version) when ?TLS_LTE(Version, ?TLS_1_2) -> @@ -3298,7 +3294,7 @@ psk_secret(PSKIdentity, PSKLookup) -> case handle_psk_identity(PSKIdentity, PSKLookup) of {ok, PSK} when is_binary(PSK) -> Len = erlang:byte_size(PSK), - <>; + <>; #alert{} = Alert -> Alert; _ -> diff --git a/lib/ssl/src/tls_record_1_3.erl b/lib/ssl/src/tls_record_1_3.erl index 5bb6cad8d731..1ed91d458d09 100644 --- a/lib/ssl/src/tls_record_1_3.erl +++ b/lib/ssl/src/tls_record_1_3.erl @@ -129,14 +129,12 @@ decode_cipher_text(#ssl_tls{type = ?OPAQUE_TYPE, BulkCipherAlgo, CipherFragment); #alert{} = Alert -> Alert; - PlainFragment0 when EarlyDataAccepted =:= true andalso - PendingMaxEarlyDataSize0 > 0 -> - PlainFragment = remove_padding(PlainFragment0), + PlainFragment when EarlyDataAccepted =:= true andalso + PendingMaxEarlyDataSize0 > 0 -> process_early_data(ConnectionStates0, ReadState0, PendingMaxEarlyDataSize0, Seq, PlainFragment); - PlainFragment0 -> - PlainFragment = remove_padding(PlainFragment0), + PlainFragment -> ConnectionStates = ConnectionStates0#{current_read => ReadState0#{sequence_number => Seq + 1}}, @@ -221,14 +219,13 @@ process_early_data(ConnectionStates0, ReadState0, PendingMaxEarlyDataSize0, Seq, PlainFragment) -> %% First packet is deciphered anyway so we must check if more early data is received %% than the configured limit (max_early_data_size). - Record = decode_inner_plaintext(PlainFragment), - case {Record#ssl_tls.type, remove_padding(Record#ssl_tls.fragment)} of - {?HANDSHAKE, <>} -> + case Record = decode_inner_plaintext(PlainFragment) of + #ssl_tls{type = ?HANDSHAKE, fragment = <>} -> ConnectionStates = ConnectionStates0#{current_read => ReadState0#{sequence_number => Seq + 1}}, {Record, ConnectionStates}; - {?APPLICATION_DATA, Data} -> + #ssl_tls{type=?APPLICATION_DATA, fragment=Data} -> PendingMaxEarlyDataSize = pending_early_data_size(PendingMaxEarlyDataSize0, Data), if PendingMaxEarlyDataSize < 0 -> @@ -296,8 +293,7 @@ additional_data(Length) -> %% The resulting quantity (of length iv_length) is used as the %% per-record nonce. nonce(Seq, IV) -> - Padding = binary:copy(<<0>>, byte_size(IV) - 8), - crypto:exor(<>, IV). + crypto:exor(<<0:(bit_size(IV)-64),?UINT64(Seq)>>, IV). cipher_aead(Fragment, BulkCipherAlgo, Key, Seq, IV, TagLen) -> AAD = additional_data(erlang:iolist_size(Fragment) + TagLen), @@ -310,11 +306,14 @@ encode_tls_cipher_text(Type, {MajVer,MinVer}, Encoded) -> Length = erlang:iolist_size(Encoded), [<>, Encoded]. -decipher_aead(CipherFragment, BulkCipherAlgo, Key, Seq, IV, TagLen) -> +decipher_aead(CipherFragment0, BulkCipherAlgo, Key, Seq, IV, TagLen) -> try - AAD = additional_data(erlang:iolist_size(CipherFragment)), + CipherFragment = iolist_to_binary(CipherFragment0), + FragLen = byte_size(CipherFragment), + AAD = additional_data(FragLen), Nonce = nonce(Seq, IV), - {CipherText, CipherTag} = aead_ciphertext_split(CipherFragment, TagLen), + CipherLen = FragLen - TagLen, + <> = CipherFragment, case ssl_cipher:aead_decrypt(BulkCipherAlgo, Key, Nonce, CipherText, CipherTag, AAD) of Content when is_binary(Content) -> Content; @@ -329,44 +328,22 @@ decipher_aead(CipherFragment, BulkCipherAlgo, Key, Seq, IV, TagLen) -> ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC, decryption_failed) end. - -aead_ciphertext_split(CipherTextFragment, TagLen) - when is_binary(CipherTextFragment) -> - CipherLen = erlang:byte_size(CipherTextFragment) - TagLen, - <> = CipherTextFragment, - {CipherText, CipherTag}; -aead_ciphertext_split(CipherTextFragment, TagLen) - when is_list(CipherTextFragment) -> - CipherLen = erlang:iolist_size(CipherTextFragment) - TagLen, - <> = - erlang:iolist_to_binary(CipherTextFragment), - {CipherText, CipherTag}. - decode_inner_plaintext(PlainText) -> - case binary:last(PlainText) of - Type when Type =:= ?APPLICATION_DATA orelse - Type =:= ?HANDSHAKE orelse - Type =:= ?ALERT -> + Sz = byte_size(PlainText) - 1, + case PlainText of + <> -> %% Remove padding + decode_inner_plaintext(Bin); + <> when + Type =:= ?APPLICATION_DATA orelse + Type =:= ?HANDSHAKE orelse + Type =:= ?ALERT -> #ssl_tls{type = Type, version = ?TLS_1_3, %% Internally use real version - fragment = init_binary(PlainText)}; + fragment = Bin}; _Else -> ?ALERT_REC(?FATAL, ?UNEXPECTED_MESSAGE, empty_alert) end. -init_binary(B) -> - {Init, _} = - split_binary(B, byte_size(B) - 1), - Init. - -remove_padding(InnerPlainText) -> - case binary:last(InnerPlainText) of - 0 -> - remove_padding(init_binary(InnerPlainText)); - _ -> - InnerPlainText - end. - pending_early_data_size(PendingMaxEarlyDataSize, PlainFragment) -> %% The maximum amount of 0-RTT data that the client is allowed to %% send when using this ticket, in bytes. Only Application Data diff --git a/lib/ssl/test/openssl_session_ticket_SUITE.erl b/lib/ssl/test/openssl_session_ticket_SUITE.erl index 63c8d7424b9e..2986a2844a6f 100644 --- a/lib/ssl/test/openssl_session_ticket_SUITE.erl +++ b/lib/ssl/test/openssl_session_ticket_SUITE.erl @@ -658,7 +658,7 @@ openssl_client_early_data_basic(Config) when is_list(Config) -> RequestFile = filename:join([proplists:get_value(priv_dir, Config), "request"]), ServerTicketMode = proplists:get_value(server_ticket_mode, Config), %% Create request file to be used with early data - EarlyData = <<"HEAD / HTTP/1.1\nHost: \nConnection: close\n\n">>, + EarlyData = <<"HEAD / HTTP/1.1\nHost: \nConnection: close\n\n", 0, 0>>, create_request(RequestFile, EarlyData), %% Configure session tickets From ecb9164c1b35eae20a9fe2c7418f44b2a8f1091f Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Wed, 17 Jan 2024 16:36:19 +0100 Subject: [PATCH 08/20] Move beast_mitigation out of connection_state And make it optional since it is only used in tlsv-1.0. --- lib/ssl/src/dtls_gen_connection.erl | 6 ++-- lib/ssl/src/dtls_record.erl | 23 +++++++-------- lib/ssl/src/ssl_record.erl | 30 +++++++------------ lib/ssl/src/tls_gen_connection.erl | 9 +++--- lib/ssl/src/tls_gen_connection_1_3.erl | 1 + lib/ssl/src/tls_record.erl | 41 +++++++++++--------------- lib/ssl/src/tls_sender.erl | 14 +++++++-- 7 files changed, 60 insertions(+), 64 deletions(-) diff --git a/lib/ssl/src/dtls_gen_connection.erl b/lib/ssl/src/dtls_gen_connection.erl index 81dd1a05cb68..4a43aa3ef83f 100644 --- a/lib/ssl/src/dtls_gen_connection.erl +++ b/lib/ssl/src/dtls_gen_connection.erl @@ -48,7 +48,7 @@ reinit/1, reinit_handshake_data/1, select_sni_extension/1, - empty_connection_state/2, + empty_connection_state/1, gen_info/3, prepare_flight/1, next_flight/1, @@ -585,8 +585,8 @@ select_sni_extension(#client_hello{extensions = #{sni := SNI}}) -> select_sni_extension(_) -> undefined. -empty_connection_state(ConnectionEnd, BeastMitigation) -> - Empty = ssl_record:empty_connection_state(ConnectionEnd, BeastMitigation), +empty_connection_state(ConnectionEnd) -> + Empty = ssl_record:empty_connection_state(ConnectionEnd), dtls_record:empty_connection_state(Empty). %%==================================================================== diff --git a/lib/ssl/src/dtls_record.erl b/lib/ssl/src/dtls_record.erl index 100dafae19cb..aaadc8760c41 100644 --- a/lib/ssl/src/dtls_record.erl +++ b/lib/ssl/src/dtls_record.erl @@ -74,19 +74,20 @@ %%-------------------------------------------------------------------- init_connection_states(Role, BeastMitigation) -> ConnectionEnd = ssl_record:record_protocol_role(Role), - Initial = initial_connection_state(ConnectionEnd, BeastMitigation), + Initial = initial_connection_state(ConnectionEnd), Current = Initial#{epoch := 0}, %% No need to pass Version to ssl_record:empty_connection_state since %% random nonce is generated with same algorithm for DTLS version %% Might require a change for DTLS-1.3 - InitialPending = ssl_record:empty_connection_state(ConnectionEnd, BeastMitigation), + InitialPending = ssl_record:empty_connection_state(ConnectionEnd), Pending = empty_connection_state(InitialPending), - #{saved_read => Current, - current_read => Current, - pending_read => Pending, - saved_write => Current, - current_write => Current, - pending_write => Pending}. + CS = #{saved_read => Current, current_read => Current, + pending_read => Pending, saved_write => Current, + current_write => Current, pending_write => Pending}, + case BeastMitigation of + disabled -> CS; + _NotDisabled -> CS#{beast_mitigation => BeastMitigation} + end. empty_connection_state(Empty) -> Empty#{epoch => undefined, replay_window => init_replay_window()}. @@ -401,13 +402,11 @@ hello_version(Version, Versions) -> %%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- -initial_connection_state(ConnectionEnd, BeastMitigation) -> - #{security_parameters => - ssl_record:initial_security_params(ConnectionEnd), +initial_connection_state(ConnectionEnd) -> + #{security_parameters => ssl_record:initial_security_params(ConnectionEnd), epoch => undefined, sequence_number => 0, replay_window => init_replay_window(), - beast_mitigation => BeastMitigation, cipher_state => undefined, mac_secret => undefined, secure_renegotiation => undefined, diff --git a/lib/ssl/src/ssl_record.erl b/lib/ssl/src/ssl_record.erl index 8433610de5fa..dc5cf0458322 100644 --- a/lib/ssl/src/ssl_record.erl +++ b/lib/ssl/src/ssl_record.erl @@ -42,8 +42,8 @@ set_client_verify_data/3, set_server_verify_data/3, set_max_fragment_length/2, - empty_connection_state/2, - empty_connection_state/4, + empty_connection_state/1, + empty_connection_state/3, record_protocol_role/1, step_encryption_state/1, step_encryption_state_read/1, @@ -101,29 +101,24 @@ activate_pending_connection_state(#{current_read := Current, pending_read := Pending} = States, read, Connection) -> #{secure_renegotiation := SecureRenegotation} = Current, - #{beast_mitigation := BeastMitigation, - security_parameters := SecParams} = Pending, + #{security_parameters := SecParams} = Pending, NewCurrent = Pending#{sequence_number => 0}, ConnectionEnd = SecParams#security_parameters.connection_end, - EmptyPending = Connection:empty_connection_state(ConnectionEnd, BeastMitigation), + EmptyPending = Connection:empty_connection_state(ConnectionEnd), NewPending = EmptyPending#{secure_renegotiation => SecureRenegotation}, States#{current_read => NewCurrent, - pending_read => NewPending - }; - + pending_read => NewPending}; activate_pending_connection_state(#{current_write := Current, pending_write := Pending} = States, write, Connection) -> NewCurrent = Pending#{sequence_number => 0}, #{secure_renegotiation := SecureRenegotation} = Current, - #{beast_mitigation := BeastMitigation, - security_parameters := SecParams} = Pending, + #{security_parameters := SecParams} = Pending, ConnectionEnd = SecParams#security_parameters.connection_end, - EmptyPending = Connection:empty_connection_state(ConnectionEnd, BeastMitigation), + EmptyPending = Connection:empty_connection_state(ConnectionEnd), NewPending = EmptyPending#{secure_renegotiation => SecureRenegotation}, States#{current_write => NewCurrent, - pending_write => NewPending - }. + pending_write => NewPending}. %%-------------------------------------------------------------------- -spec step_encryption_state(#state{}) -> #state{}. @@ -431,16 +426,13 @@ nonce_seed(_,_, CipherState) -> %%% Internal functions %%-------------------------------------------------------------------- -empty_connection_state(ConnectionEnd, BeastMitigation) -> +empty_connection_state(ConnectionEnd) -> MaxEarlyDataSize = ssl_config:get_max_early_data_size(), - empty_connection_state(ConnectionEnd, _Version = undefined, - BeastMitigation, MaxEarlyDataSize). + empty_connection_state(ConnectionEnd, _Version = undefined, MaxEarlyDataSize). %% -empty_connection_state(ConnectionEnd, Version, - BeastMitigation, MaxEarlyDataSize) -> +empty_connection_state(ConnectionEnd, Version, MaxEarlyDataSize) -> SecParams = init_security_parameters(ConnectionEnd, Version), #{security_parameters => SecParams, - beast_mitigation => BeastMitigation, cipher_state => undefined, mac_secret => undefined, secure_renegotiation => undefined, diff --git a/lib/ssl/src/tls_gen_connection.erl b/lib/ssl/src/tls_gen_connection.erl index 864703f8e84b..27560f2ba83e 100644 --- a/lib/ssl/src/tls_gen_connection.erl +++ b/lib/ssl/src/tls_gen_connection.erl @@ -49,7 +49,7 @@ reinit/1, reinit_handshake_data/1, select_sni_extension/1, - empty_connection_state/2, + empty_connection_state/1, encode_handshake/4]). %% State transition handling @@ -161,10 +161,11 @@ initialize_tls_sender(#state{static_env = #static_env{ key_update_at := KeyUpdateAt, log_level := LogLevel } = SSLOpts, - connection_states = #{current_write := ConnectionWriteState}, + connection_states = #{current_write := ConnectionWriteState} = CS, protocol_specific = #{sender := Sender}}) -> HibernateAfter = maps:get(hibernate_after, SSLOpts, infinity), Init = #{current_write => ConnectionWriteState, + beast_mitigation => maps:get(beast_mitigation, CS, disabled), role => Role, socket => Socket, socket_options => SockOpts, @@ -244,8 +245,8 @@ select_sni_extension(#client_hello{extensions = #{sni := SNI}}) -> select_sni_extension(_) -> undefined. -empty_connection_state(ConnectionEnd, BeastMitigation) -> - ssl_record:empty_connection_state(ConnectionEnd, BeastMitigation). +empty_connection_state(ConnectionEnd) -> + ssl_record:empty_connection_state(ConnectionEnd). %%==================================================================== %% Data handling diff --git a/lib/ssl/src/tls_gen_connection_1_3.erl b/lib/ssl/src/tls_gen_connection_1_3.erl index f4584aac69c2..7af57c25c236 100644 --- a/lib/ssl/src/tls_gen_connection_1_3.erl +++ b/lib/ssl/src/tls_gen_connection_1_3.erl @@ -100,6 +100,7 @@ initial_state(Role, Sender, Host, Port, Socket, active_n_toggle => true } }. + %%-------------------------------------------------------------------- %% generic state functions %%-------------------------------------------------------------------- diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl index 8af13e04dd4c..71566f91c1ee 100644 --- a/lib/ssl/src/tls_record.erl +++ b/lib/ssl/src/tls_record.erl @@ -91,15 +91,14 @@ init_connection_states(Role, Version, BeastMitigation) -> init_connection_states(Role, Version, BeastMitigation, MaxEarlyDataSize) -> ConnectionEnd = ssl_record:record_protocol_role(Role), - Current = initial_connection_state(ConnectionEnd, BeastMitigation, MaxEarlyDataSize), - Pending = ssl_record:empty_connection_state(ConnectionEnd, - Version, - BeastMitigation, - MaxEarlyDataSize), - #{current_read => Current, - pending_read => Pending, - current_write => Current, - pending_write => Pending}. + Current = initial_connection_state(ConnectionEnd, MaxEarlyDataSize), + Pending = ssl_record:empty_connection_state(ConnectionEnd, Version, MaxEarlyDataSize), + CS = #{current_read => Current, pending_read => Pending, + current_write => Current, pending_write => Pending}, + case BeastMitigation of + disabled -> CS; + _NotDisabled -> CS#{beast_mitigation => BeastMitigation} + end. %%-------------------------------------------------------------------- -spec get_tls_records( @@ -135,14 +134,13 @@ encode_handshake(Frag, ?TLS_1_3, ConnectionStates) -> tls_record_1_3:encode_handshake(Frag, ConnectionStates); encode_handshake(Frag, Version, #{current_write := - #{beast_mitigation := BeastMitigation, - security_parameters := - #security_parameters{bulk_cipher_algorithm = BCA}}} = - ConnectionStates) -> + #{security_parameters := #security_parameters{bulk_cipher_algorithm = BCA}} + } = ConnectionStates) -> MaxLength = maps:get(max_fragment_length, ConnectionStates, ?MAX_PLAIN_TEXT_LENGTH), + BeastM = maps:get(beast_mitigation, ConnectionStates, disabled), case iolist_size(Frag) of N when N > MaxLength -> - Data = split_iovec(erlang:iolist_to_iovec(Frag), Version, BCA, BeastMitigation, MaxLength), + Data = split_iovec(erlang:iolist_to_iovec(Frag), Version, BCA, BeastM, MaxLength), encode_fragments(?HANDSHAKE, Version, Data, ConnectionStates); _ -> encode_plain_text(?HANDSHAKE, Version, Frag, ConnectionStates) @@ -180,12 +178,11 @@ encode_data(Data, ?TLS_1_3, ConnectionStates) -> tls_record_1_3:encode_data(Data, ConnectionStates); encode_data(Data, Version, #{current_write := - #{beast_mitigation := BeastMitigation, - security_parameters := - #security_parameters{bulk_cipher_algorithm = BCA}}} = - ConnectionStates) -> + #{security_parameters := #security_parameters{bulk_cipher_algorithm = BCA}} + } = ConnectionStates) -> MaxLength = maps:get(max_fragment_length, ConnectionStates, ?MAX_PLAIN_TEXT_LENGTH), - Fragments = split_iovec(Data, Version, BCA, BeastMitigation, MaxLength), + BeastM = maps:get(beast_mitigation, ConnectionStates, disabled), + Fragments = split_iovec(Data, Version, BCA, BeastM, MaxLength), encode_fragments(?APPLICATION_DATA, Version, Fragments, ConnectionStates). %%==================================================================== @@ -459,11 +456,9 @@ split_iovec(Data, MaximumFragmentLength) -> %%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- -initial_connection_state(ConnectionEnd, BeastMitigation, MaxEarlyDataSize) -> - #{security_parameters => - ssl_record:initial_security_params(ConnectionEnd), +initial_connection_state(ConnectionEnd, MaxEarlyDataSize) -> + #{security_parameters => ssl_record:initial_security_params(ConnectionEnd), sequence_number => 0, - beast_mitigation => BeastMitigation, cipher_state => undefined, mac_secret => undefined, secure_renegotiation => undefined, diff --git a/lib/ssl/src/tls_sender.erl b/lib/ssl/src/tls_sender.erl index d504c18d3971..608eb553bb45 100644 --- a/lib/ssl/src/tls_sender.erl +++ b/lib/ssl/src/tls_sender.erl @@ -225,6 +225,7 @@ init(_) -> gen_statem:event_handler_result(atom()). %%-------------------------------------------------------------------- init({call, From}, {Pid, #{current_write := WriteState, + beast_mitigation := BeastMitigation, role := Role, socket := Socket, socket_options := SockOpts, @@ -236,9 +237,16 @@ init({call, From}, {Pid, #{current_write := WriteState, key_update_at := KeyUpdateAt, log_level := LogLevel, hibernate_after := HibernateAfter}}, - #data{connection_states = ConnectionStates, static = Static0} = StateData0) -> - StateData = - StateData0#data{connection_states = ConnectionStates#{current_write => WriteState}, + #data{connection_states = ConnectionStates0, static = Static0} = StateData0) -> + ConnectionStates = case BeastMitigation of + disabled -> + ConnectionStates0#{current_write => WriteState}; + _ -> + ConnectionStates0#{current_write => WriteState, + beast_mitigation => BeastMitigation} + end, + StateData = + StateData0#data{connection_states = ConnectionStates, static = Static0#static{connection_pid = Pid, role = Role, socket = Socket, From 7c1e0f87999b534ce3206da98fcb4cdcc390e7d9 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Thu, 18 Jan 2024 15:03:22 +0100 Subject: [PATCH 09/20] Special case for {active, X} Most common use of ssl:setopts/2. --- lib/ssl/src/ssl.erl | 2 ++ lib/ssl/src/ssl_gen_statem.erl | 63 +++++++++++++++++----------------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl index c3e6d9137b3c..1e061668c3a0 100644 --- a/lib/ssl/src/ssl.erl +++ b/lib/ssl/src/ssl.erl @@ -2783,6 +2783,8 @@ getopts(#sslsocket{}, OptionTags) -> %% %% Description: Sets options %%-------------------------------------------------------------------- +setopts(#sslsocket{pid = [Pid|_]}, [{active, _}] = Active) when is_pid(Pid) -> + ssl_gen_statem:set_opts(Pid, Active); setopts(#sslsocket{pid = [Pid, Sender]}, Options0) when is_pid(Pid), is_list(Options0) -> try proplists:expand([{binary, [{mode, binary}]}, {list, [{mode, list}]}], Options0) of diff --git a/lib/ssl/src/ssl_gen_statem.erl b/lib/ssl/src/ssl_gen_statem.erl index f38b390ac92c..208dbf08b5bd 100644 --- a/lib/ssl/src/ssl_gen_statem.erl +++ b/lib/ssl/src/ssl_gen_statem.erl @@ -1974,6 +1974,37 @@ set_socket_opts(ConnectionCb, Transport, Socket, [], SockOpts, Other) -> {{error, {options, {socket_options, Other, Error}}}, SockOpts} end; +set_socket_opts(ConnectionCb, Transport, Socket, [{active, Active}| Opts], SockOpts, Other) + when Active == once; Active == true; Active == false -> + set_socket_opts(ConnectionCb, Transport, Socket, Opts, + SockOpts#socket_options{active = Active}, Other); +set_socket_opts(ConnectionCb, Transport, Socket, [{active, Active1} = Opt| Opts], + SockOpts=#socket_options{active = Active0}, Other) + when Active1 >= -32768, Active1 =< 32767 -> + Active = if + is_integer(Active0), Active0 + Active1 < -32768 -> + error; + is_integer(Active0), Active0 + Active1 =< 0 -> + false; + is_integer(Active0), Active0 + Active1 > 32767 -> + error; + Active1 =< 0 -> + false; + is_integer(Active0) -> + Active0 + Active1; + true -> + Active1 + end, + case Active of + error -> + {{error, {options, {socket_options, Opt}} }, SockOpts}; + _ -> + set_socket_opts(ConnectionCb, Transport, Socket, Opts, + SockOpts#socket_options{active = Active}, Other) + end; +set_socket_opts(_,_, _, [{active, _} = Opt| _], SockOpts, _) -> + {{error, {options, {socket_options, Opt}} }, SockOpts}; + set_socket_opts(ConnectionCb, Transport,Socket, [{mode, Mode}| Opts], SockOpts, Other) when Mode == list; Mode == binary -> set_socket_opts(ConnectionCb, Transport, Socket, Opts, @@ -2006,38 +2037,6 @@ set_socket_opts(ConnectionCb, Transport, Socket, [{header, Header}| Opts], SockO SockOpts#socket_options{header = Header}, Other); set_socket_opts(_, _, _, [{header, _} = Opt| _], SockOpts, _) -> {{error,{options, {socket_options, Opt}}}, SockOpts}; -set_socket_opts(ConnectionCb, Transport, Socket, [{active, Active}| Opts], SockOpts, Other) - when Active == once; - Active == true; - Active == false -> - set_socket_opts(ConnectionCb, Transport, Socket, Opts, - SockOpts#socket_options{active = Active}, Other); -set_socket_opts(ConnectionCb, Transport, Socket, [{active, Active1} = Opt| Opts], - SockOpts=#socket_options{active = Active0}, Other) - when Active1 >= -32768, Active1 =< 32767 -> - Active = if - is_integer(Active0), Active0 + Active1 < -32768 -> - error; - is_integer(Active0), Active0 + Active1 =< 0 -> - false; - is_integer(Active0), Active0 + Active1 > 32767 -> - error; - Active1 =< 0 -> - false; - is_integer(Active0) -> - Active0 + Active1; - true -> - Active1 - end, - case Active of - error -> - {{error, {options, {socket_options, Opt}} }, SockOpts}; - _ -> - set_socket_opts(ConnectionCb, Transport, Socket, Opts, - SockOpts#socket_options{active = Active}, Other) - end; -set_socket_opts(_,_, _, [{active, _} = Opt| _], SockOpts, _) -> - {{error, {options, {socket_options, Opt}} }, SockOpts}; set_socket_opts(ConnectionCb, Transport,Socket, [{packet_size, Size}| Opts], SockOpts, Other) when is_integer(Size) -> set_socket_opts(ConnectionCb, Transport, Socket, Opts, SockOpts#socket_options{packet_size = Size}, Other); From 4b3cc7033d5be52d08d9c2d3342472fa222c1a02 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Thu, 18 Jan 2024 15:33:51 +0100 Subject: [PATCH 10/20] Remove SSLOpts from tls_record:get_tls_records/5 Use Downgrade as an argument instead of putting in a map in the most used function. --- lib/ssl/src/ssl_logger.erl | 4 +- lib/ssl/src/tls_gen_connection.erl | 18 +++---- lib/ssl/src/tls_record.erl | 87 +++++++++++++++--------------- 3 files changed, 53 insertions(+), 56 deletions(-) diff --git a/lib/ssl/src/ssl_logger.erl b/lib/ssl/src/ssl_logger.erl index ccb0f394657c..03ebf78946fd 100644 --- a/lib/ssl/src/ssl_logger.erl +++ b/lib/ssl/src/ssl_logger.erl @@ -35,8 +35,6 @@ -include("ssl_internal.hrl"). -include("tls_record.hrl"). --include("ssl_cipher.hrl"). --include("ssl_internal.hrl"). -include("tls_handshake.hrl"). -include("dtls_handshake.hrl"). -include("tls_handshake_1_3.hrl"). @@ -58,6 +56,8 @@ log(Level, LogLevel, ReportMap, Meta) -> ok end. +debug(undefined, _Direction, _Protocol, _Message) -> + ok; debug(LogLevel, Direction, Protocol, Message) when (Direction =:= inbound orelse Direction =:= outbound) andalso (Protocol =:= 'record' orelse Protocol =:= 'handshake') -> diff --git a/lib/ssl/src/tls_gen_connection.erl b/lib/ssl/src/tls_gen_connection.erl index 27560f2ba83e..fab8fc347372 100644 --- a/lib/ssl/src/tls_gen_connection.erl +++ b/lib/ssl/src/tls_gen_connection.erl @@ -634,9 +634,10 @@ next_tls_record(Data, StateName, #state{protocol_buffers = #protocol_buffers{tls_record_buffer = Buf0, tls_cipher_texts = CT0} = Buffers, - connection_env = #connection_env{ - downgrade = Downgrade}, - ssl_options = SslOpts0} = State0) -> + connection_env = + #connection_env{downgrade = Downgrade, + negotiated_version = Vsns0} + } = State0) -> Versions = %% TLSPlaintext.legacy_record_version is ignored in TLS 1.3 and thus all %% record version are accepted when receiving initial ClientHello and @@ -647,21 +648,18 @@ next_tls_record(Data, StateName, %% Note: TLS record version {3,4} is used internally in TLS 1.3 and at this %% point it is the same as the negotiated protocol version. TLS-1.3 %% uses TLS-1.2 as record version. - case StateName of - State when State =:= hello orelse - State =:= start -> + if StateName =:= hello orelse StateName =:= start -> %% Allow any {03,XX} TLS record version for the hello message %% for maximum interopability and compliance with TLS-1.2 spec. %% This does not allow SSL-3.0 connections, that we do not support %% or interfere with TLS-1.3 extensions to handle version negotiation. AllHelloVersions = [ 'sslv3' | ?ALL_AVAILABLE_VERSIONS], [tls_record:protocol_version_name(Vsn) || Vsn <- AllHelloVersions]; - _ -> - State0#state.connection_env#connection_env.negotiated_version + true -> + Vsns0 end, MaxFragLen = maps:get(max_fragment_length, State0#state.connection_states, undefined), - SslOpts = maps:put(downgrade, Downgrade, SslOpts0), - case tls_record:get_tls_records(Data, Versions, Buf0, MaxFragLen, SslOpts) of + case tls_record:get_tls_records(Data, Versions, Buf0, MaxFragLen, Downgrade) of {Records, Buf1} -> CT1 = CT0 ++ Records, next_record(StateName, State0#state{protocol_buffers = diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl index 71566f91c1ee..de0076efa699 100644 --- a/lib/ssl/src/tls_record.erl +++ b/lib/ssl/src/tls_record.erl @@ -106,7 +106,7 @@ init_connection_states(Role, Version, BeastMitigation, MaxEarlyDataSize) -> [tls_version()] | tls_version(), Buffer0 :: binary() | {'undefined' | #ssl_tls{}, {[binary()],non_neg_integer(),[binary()]}}, tls_max_frag_len(), - ssl_options()) -> + Downgrade :: {NewController::pid(), From::gen_statem:from()} | 'undefined') -> {Records :: [#ssl_tls{}], Buffer :: {'undefined' | #ssl_tls{}, {[binary()],non_neg_integer(),[binary()]}}} | #alert{}. @@ -115,10 +115,10 @@ init_connection_states(Role, Version, BeastMitigation, MaxEarlyDataSize) -> %% Description: Given old buffer and new data from TCP, packs up a records %% data %%-------------------------------------------------------------------- -get_tls_records(Data, Versions, Buffer, MaxFragLen, SslOpts) when is_binary(Buffer) -> - parse_tls_records(Versions, {[Data],byte_size(Data),[]}, MaxFragLen, SslOpts, undefined); -get_tls_records(Data, Versions, {Hdr, {Front,Size,Rear}}, MaxFragLen, SslOpts) -> - parse_tls_records(Versions, {Front,Size + byte_size(Data),[Data|Rear]}, MaxFragLen, SslOpts, Hdr). +get_tls_records(Data, Versions, Buffer, MaxFragLen, Downgrade) when is_binary(Buffer) -> + parse_tls_records(Versions, {[Data],byte_size(Data),[]}, MaxFragLen, Downgrade, undefined); +get_tls_records(Data, Versions, {Hdr, {Front,Size,Rear}}, MaxFragLen, Downgrade) -> + parse_tls_records(Versions, {Front,Size + byte_size(Data),[Data|Rear]}, MaxFragLen, Downgrade, Hdr). %%==================================================================== %% Encoding @@ -476,95 +476,94 @@ build_tls_record(#ssl_tls{type = Type, version = Version, fragment = Fragment}) <>. -parse_tls_records(Versions, Q, MaxFragLen, SslOpts, undefined) -> - decode_tls_records(Versions, Q, MaxFragLen, SslOpts, [], undefined, undefined, undefined); -parse_tls_records(Versions, Q, MaxFragLen, SslOpts, #ssl_tls{type = Type, version = Version, fragment = Length}) -> - decode_tls_records(Versions, Q, MaxFragLen, SslOpts, [], Type, Version, Length). +parse_tls_records(Versions, Q, MaxFragLen, Downgrade, undefined) -> + decode_tls_records(Versions, Q, MaxFragLen, Downgrade, [], undefined, undefined, undefined); +parse_tls_records(Versions, Q, MaxFragLen, Downgrade, #ssl_tls{type = Type, version = Version, fragment = Length}) -> + decode_tls_records(Versions, Q, MaxFragLen, Downgrade, [], Type, Version, Length). %% Generic code path -decode_tls_records(Versions, {_,Size,_} = Q0, MaxFragLen, SslOpts, Acc, undefined, _Version, _Length) -> +decode_tls_records(Versions, {_,Size,_} = Q0, MaxFragLen, Downgrade, Acc, undefined, _Version, _Length) -> if 5 =< Size -> {<>, Q} = binary_from_front(5, Q0), %% TODO: convert the MajVer and MinVer to corresponding macro Version = {MajVer,MinVer}, - validate_tls_records_type(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length); + validate_tls_records_type(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length); 3 =< Size -> {<>, Q} = binary_from_front(3, Q0), Version = {MajVer,MinVer}, - validate_tls_records_type(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, undefined); + validate_tls_records_type(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, undefined); 1 =< Size -> {<>, Q} = binary_from_front(1, Q0), - validate_tls_records_type(Versions, Q, MaxFragLen, SslOpts, Acc, Type, undefined, undefined); + validate_tls_records_type(Versions, Q, MaxFragLen, Downgrade, Acc, Type, undefined, undefined); true -> - validate_tls_records_type(Versions, Q0, MaxFragLen, SslOpts, Acc, undefined, undefined, undefined) + validate_tls_records_type(Versions, Q0, MaxFragLen, Downgrade, Acc, undefined, undefined, undefined) end; -decode_tls_records(Versions, {_,Size,_} = Q0, MaxFragLen, SslOpts, Acc, Type, undefined, _Length) -> +decode_tls_records(Versions, {_,Size,_} = Q0, MaxFragLen, Downgrade, Acc, Type, undefined, _Length) -> if 4 =< Size -> {<>, Q} = binary_from_front(4, Q0), Version = {MajVer,MinVer}, - validate_tls_record_version(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length); + validate_tls_record_version(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length); 2 =< Size -> {<>, Q} = binary_from_front(2, Q0), Version = {MajVer,MinVer}, - validate_tls_record_version(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, undefined); + validate_tls_record_version(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, undefined); true -> - validate_tls_record_version(Versions, Q0, MaxFragLen, SslOpts, Acc, Type, undefined, undefined) + validate_tls_record_version(Versions, Q0, MaxFragLen, Downgrade, Acc, Type, undefined, undefined) end; -decode_tls_records(Versions, {_,Size,_} = Q0, MaxFragLen, SslOpts, Acc, Type, Version, undefined) -> +decode_tls_records(Versions, {_,Size,_} = Q0, MaxFragLen, Downgrade, Acc, Type, Version, undefined) -> if 2 =< Size -> {<>, Q} = binary_from_front(2, Q0), - validate_tls_record_length(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length); + validate_tls_record_length(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length); true -> - validate_tls_record_length(Versions, Q0, MaxFragLen, SslOpts, Acc, Type, Version, undefined) + validate_tls_record_length(Versions, Q0, MaxFragLen, Downgrade, Acc, Type, Version, undefined) end; -decode_tls_records(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length) -> - validate_tls_record_length(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length). +decode_tls_records(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length) -> + validate_tls_record_length(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length). %% TODO: separate validation logic from modification of the record -validate_tls_records_type(_Versions, Q, _MaxFragLen, _SslOpts, Acc, undefined, _Version, _Length) -> +validate_tls_records_type(_Versions, Q, _MaxFragLen, _Downgrade, Acc, undefined, _Version, _Length) -> {lists:reverse(Acc), {undefined, Q}}; -validate_tls_records_type(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length) when ?KNOWN_RECORD_TYPE(Type) -> - validate_tls_record_version(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length); -validate_tls_records_type(_Versions, _Q, _MaxFragLen, _SslOpts, _Acc, Type, _Version, _Length) -> +validate_tls_records_type(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length) when ?KNOWN_RECORD_TYPE(Type) -> + validate_tls_record_version(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length); +validate_tls_records_type(_Versions, _Q, _MaxFragLen, _Downgrade, _Acc, Type, _Version, _Length) -> %% Not ?KNOWN_RECORD_TYPE(Type) ?ALERT_REC(?FATAL, ?UNEXPECTED_MESSAGE, {unsupported_record_type, Type}). -validate_tls_record_version(_Versions, Q, _MaxFragLen, _SslOpts, Acc, Type, undefined, _Length) -> +validate_tls_record_version(_Versions, Q, _MaxFragLen, _Downgrade, Acc, Type, undefined, _Length) -> {lists:reverse(Acc), {#ssl_tls{type = Type, version = undefined, fragment = undefined}, Q}}; -validate_tls_record_version(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length) when is_list(Versions) -> +validate_tls_record_version(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length) when is_list(Versions) -> case is_acceptable_version(Version, Versions) of true -> - validate_tls_record_length(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length); + validate_tls_record_length(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length); false -> ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC, {unsupported_version, Version}) end; -validate_tls_record_version(?TLS_1_3=Versions, Q, MaxFragLen, SslOpts, Acc, Type, ?TLS_1_2=Version, Length) -> - validate_tls_record_length(Versions, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length); -validate_tls_record_version(Version, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length) -> +validate_tls_record_version(?TLS_1_3=Versions, Q, MaxFragLen, Downgrade, Acc, Type, ?TLS_1_2=Version, Length) -> + validate_tls_record_length(Versions, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length); +validate_tls_record_version(Version, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length) -> %% Exact version match - validate_tls_record_length(Version, Q, MaxFragLen, SslOpts, Acc, Type, Version, Length); -validate_tls_record_version(_Versions, _Q, _MaxFragLen, _SslOpts, _Acc, _Type, Version, _Length) -> + validate_tls_record_length(Version, Q, MaxFragLen, Downgrade, Acc, Type, Version, Length); +validate_tls_record_version(_Versions, _Q, _MaxFragLen, _Downgrade, _Acc, _Type, Version, _Length) -> ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC, {unsupported_version, Version}). -validate_tls_record_length(_Versions, Q, _MaxFragLen, _SslOpts, Acc, Type, Version, undefined) -> +validate_tls_record_length(_Versions, Q, _MaxFragLen, _Downgrade, Acc, Type, Version, undefined) -> {lists:reverse(Acc), {#ssl_tls{type = Type, version = Version, fragment = undefined}, Q}}; validate_tls_record_length(Versions, {_,Size0,_} = Q0, MaxFragLen, - #{log_level := LogLevel, downgrade := Downgrade} = SslOpts, - Acc, Type, Version, Length) -> + Downgrade, Acc, Type, Version, Length) -> Max = if is_integer(MaxFragLen) -> - MaxFragLen + ?MAX_PADDING_LENGTH + ?MAX_MAC_LENGTH; - true -> - max_len(Versions) - end, + MaxFragLen + ?MAX_PADDING_LENGTH + ?MAX_MAC_LENGTH; + true -> + max_len(Versions) + end, if Length =< Max -> if @@ -572,13 +571,13 @@ validate_tls_record_length(Versions, {_,Size0,_} = Q0, MaxFragLen, %% Complete record {Fragment, Q} = binary_from_front(Length, Q0), Record = #ssl_tls{type = Type, version = Version, fragment = Fragment}, - ssl_logger:debug(LogLevel, inbound, 'record', Record), + ssl_logger:debug(get(log_level), inbound, 'record', Record), case Downgrade of {_Pid, _From} -> %% parse only single record for downgrade scenario, buffer remaining data {[Record], {undefined, Q}}; _ -> - decode_tls_records(Versions, Q, MaxFragLen, SslOpts, [Record|Acc], undefined, undefined, undefined) + decode_tls_records(Versions, Q, MaxFragLen, Downgrade, [Record|Acc], undefined, undefined, undefined) end; true -> {lists:reverse(Acc), From 01649058750520dae1685b172d45c28fb758fc5f Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Thu, 18 Jan 2024 16:56:27 +0100 Subject: [PATCH 11/20] Reduce memory usage tls_sender --- lib/ssl/src/tls_sender.erl | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/ssl/src/tls_sender.erl b/lib/ssl/src/tls_sender.erl index 608eb553bb45..b7a2bde8671e 100644 --- a/lib/ssl/src/tls_sender.erl +++ b/lib/ssl/src/tls_sender.erl @@ -69,15 +69,16 @@ negotiated_version, renegotiate_at, key_update_at, %% TLS 1.3 - bytes_sent, %% TLS 1.3 dist_handle, log_level, hibernate_after }). -record(data, - {static = #static{}, - connection_states = #{} + { + static = #static{}, + connection_states = #{}, + bytes_sent %% TLS 1.3 }). %%%=================================================================== @@ -247,6 +248,7 @@ init({call, From}, {Pid, #{current_write := WriteState, end, StateData = StateData0#data{connection_states = ConnectionStates, + bytes_sent = 0, static = Static0#static{connection_pid = Pid, role = Role, socket = Socket, @@ -257,7 +259,6 @@ init({call, From}, {Pid, #{current_write := WriteState, negotiated_version = Version, renegotiate_at = RenegotiateAt, key_update_at = KeyUpdateAt, - bytes_sent = 0, log_level = LogLevel, hibernate_after = HibernateAfter}}, proc_lib:set_label({tls_sender, Role, {connection, Pid}}), @@ -510,8 +511,8 @@ send_application_data(Data, From, StateName, transport_cb = Transport, renegotiate_at = RenegotiateAt, key_update_at = KeyUpdateAt, - bytes_sent = BytesSent, log_level = LogLevel}, + bytes_sent = BytesSent, connection_states = ConnectionStates0} = StateData0) -> case time_to_rekey(Version, Data, ConnectionStates0, RenegotiateAt, KeyUpdateAt, BytesSent) of key_update -> @@ -576,12 +577,10 @@ send_post_handshake_data(Handshake, From, StateName, {next_state, StateName, StateData1, [{reply, From, Result}]} end. -maybe_update_cipher_key(#data{connection_states = ConnectionStates0, - static = Static0} = StateData, #key_update{}) -> +maybe_update_cipher_key(#data{connection_states = ConnectionStates0}= StateData, #key_update{}) -> ConnectionStates = tls_gen_connection_1_3:update_cipher_key(current_write, ConnectionStates0), - Static = Static0#static{bytes_sent = 0}, StateData#data{connection_states = ConnectionStates, - static = Static}; + bytes_sent = 0}; maybe_update_cipher_key(StateData, _) -> StateData. @@ -590,8 +589,8 @@ update_bytes_sent(Version, StateData, _) when ?TLS_LT(Version, ?TLS_1_3) -> %% Count bytes sent in TLS 1.3 for AES-GCM update_bytes_sent(_, #data{static = #static{key_update_at = seq_num_wrap}} = StateData, _) -> StateData; %% Chacha20-Poly1305 -update_bytes_sent(_, #data{static = #static{bytes_sent = Sent} = Static} = StateData, Data) -> - StateData#data{static = Static#static{bytes_sent = Sent + iolist_size(Data)}}. %% AES-GCM +update_bytes_sent(_, #data{bytes_sent = Sent} = StateData, Data) -> + StateData#data{bytes_sent = Sent + iolist_size(Data)}. %% AES-GCM %% For AES-GCM, up to 2^24.5 full-size records (about 24 million) may be %% encrypted on a given connection while keeping a safety margin of @@ -711,15 +710,15 @@ dist_data(DHandle, CurBytes) -> %% Empty the inbox from distribution ticks - do not let them accumulate consume_ticks() -> - receive tick -> + receive tick -> consume_ticks() - after 0 -> + after 0 -> ok end. hibernate_after(connection = StateName, #data{static=#static{hibernate_after = HibernateAfter}} = State, - Actions) -> + Actions) when HibernateAfter =/= infinity -> {next_state, StateName, State, [{timeout, HibernateAfter, hibernate} | Actions]}; hibernate_after(StateName, State, Actions) -> {next_state, StateName, State, Actions}. From 6b8c10018bb1a084261a6f5b5e1b804a8b208190 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Sun, 21 Jan 2024 09:19:12 +0100 Subject: [PATCH 12/20] Reduce memory usage in gen modules Do not set timeout infinity. Try to update the #state{} as few times as possible in the loop. --- lib/ssl/src/ssl_gen_statem.erl | 11 +- lib/ssl/src/tls_gen_connection.erl | 217 +++++++++++++++-------------- 2 files changed, 121 insertions(+), 107 deletions(-) diff --git a/lib/ssl/src/ssl_gen_statem.erl b/lib/ssl/src/ssl_gen_statem.erl index 208dbf08b5bd..72f67c5ac671 100644 --- a/lib/ssl/src/ssl_gen_statem.erl +++ b/lib/ssl/src/ssl_gen_statem.erl @@ -967,13 +967,16 @@ passive_receive(#state{user_data_buffer = {Front,BufferSize,Rear}, %% Hibernation %%==================================================================== -hibernate_after(connection = StateName, +hibernate_after(StateName, #state{ssl_options= SslOpts} = State, Actions) -> HibernateAfter = maps:get(hibernate_after, SslOpts, infinity), - {next_state, StateName, State, [{timeout, HibernateAfter, hibernate} | Actions]}; -hibernate_after(StateName, State, Actions) -> - {next_state, StateName, State, Actions}. + case HibernateAfter == infinity orelse StateName =/= connection of + true -> {next_state, StateName, State, Actions}; + false -> + {next_state, StateName, State, + [{timeout, HibernateAfter, hibernate} | Actions]} + end. %%==================================================================== %% Alert and close handling diff --git a/lib/ssl/src/tls_gen_connection.erl b/lib/ssl/src/tls_gen_connection.erl index fab8fc347372..10908081cb70 100644 --- a/lib/ssl/src/tls_gen_connection.erl +++ b/lib/ssl/src/tls_gen_connection.erl @@ -291,21 +291,20 @@ handle_info({Protocol, _, Data}, StateName, #alert{} = Alert -> ssl_gen_statem:handle_own_alert(Alert, StateName, State0) end; -handle_info({PassiveTag, Socket}, StateName, - #state{static_env = #static_env{socket = Socket, - passive_tag = PassiveTag}, +handle_info({PassiveTag, Socket}, StateName, + #state{static_env = #static_env{socket = Socket, passive_tag = PassiveTag} = StatEnv, start_or_recv_from = From, protocol_buffers = #protocol_buffers{tls_cipher_texts = CTs}, protocol_specific = PS } = State0) -> case (From =/= undefined) andalso (CTs == []) of true -> - {Record, State} = activate_socket(State0#state{protocol_specific = - PS#{active_n_toggle => true}}), - next_event(StateName, Record, State); + do_activate_socket(PS, StatEnv), + State = State0#state{protocol_specific = PS#{active_n_toggle => false}}, + next_event(StateName, no_record, State); false -> - next_event(StateName, no_record, - State0#state{protocol_specific = PS#{active_n_toggle => true}}) + State = State0#state{protocol_specific = PS#{active_n_toggle => true}}, + next_event(StateName, no_record, State) end; handle_info({CloseTag, Socket}, StateName, #state{static_env = #static_env{ @@ -637,7 +636,7 @@ next_tls_record(Data, StateName, connection_env = #connection_env{downgrade = Downgrade, negotiated_version = Vsns0} - } = State0) -> + } = State) -> Versions = %% TLSPlaintext.legacy_record_version is ignored in TLS 1.3 and thus all %% record version are accepted when receiving initial ClientHello and @@ -658,175 +657,190 @@ next_tls_record(Data, StateName, true -> Vsns0 end, - MaxFragLen = maps:get(max_fragment_length, State0#state.connection_states, undefined), + MaxFragLen = maps:get(max_fragment_length, State#state.connection_states, undefined), case tls_record:get_tls_records(Data, Versions, Buf0, MaxFragLen, Downgrade) of {Records, Buf1} -> CT1 = CT0 ++ Records, - next_record(StateName, State0#state{protocol_buffers = - Buffers#protocol_buffers{tls_record_buffer = Buf1, - tls_cipher_texts = CT1}}); + next_record(StateName, Buffers#protocol_buffers{tls_record_buffer = Buf1, + tls_cipher_texts = CT1}, State); #alert{} = Alert -> - handle_record_alert(Alert, State0) + handle_record_alert(Alert, State) end. -next_record(_, #state{handshake_env = - #handshake_env{unprocessed_handshake_events = N} = HsEnv} +next_record(StateName, #state{protocol_buffers = PBuffers} = State) -> + next_record(StateName, PBuffers, State). + +next_record(_, PBuffers, #state{handshake_env = + #handshake_env{unprocessed_handshake_events = N} = HsEnv} = State) when N > 0 -> - {no_record, State#state{handshake_env = - HsEnv#handshake_env{unprocessed_handshake_events = N-1}}}; -next_record(_, #state{protocol_buffers = - #protocol_buffers{tls_cipher_texts = [_|_] = CipherTexts}, - connection_states = ConnectionStates, - ssl_options = SslOpts} = State) -> + {no_record, State#state{handshake_env = HsEnv#handshake_env{unprocessed_handshake_events = N-1}, + protocol_buffers = PBuffers}}; +next_record(_, + #protocol_buffers{tls_cipher_texts = [_|_] = CipherTexts} = PBuffers, + #state{connection_states = ConnectionStates, ssl_options = SslOpts} = State) -> %% Do not match this option as it is relevant for TLS-1.0 only %% and will not be present otherwise that is we regard it to always be true Check = maps:get(padding_check, SslOpts, true), - next_record(State, CipherTexts, ConnectionStates, Check); -next_record(connection, #state{protocol_buffers = #protocol_buffers{tls_cipher_texts = []}, - protocol_specific = #{active_n_toggle := true} - } = State) -> + next_record(State, CipherTexts, ConnectionStates, PBuffers, Check); +next_record(connection, #protocol_buffers{tls_cipher_texts = []} = PBuffers, + #state{protocol_specific = #{active_n_toggle := true}} = State) -> %% If ssl application user is not reading data wait to activate socket - flow_ctrl(State); - -next_record(_, #state{protocol_buffers = #protocol_buffers{tls_cipher_texts = []}, - protocol_specific = #{active_n_toggle := true} - } = State) -> - activate_socket(State); -next_record(_, State) -> - {no_record, State}. - -flow_ctrl(#state{ssl_options = #{ktls := true}} = State) -> - {no_record, State}; + flow_ctrl(State, PBuffers); + +next_record(_, #protocol_buffers{tls_cipher_texts = []} = PBuffers, + #state{protocol_specific = #{active_n_toggle := true}} = State) -> + activate_socket(State, PBuffers); +next_record(_, PBuffers, State) -> + {no_record, State#state{protocol_buffers = PBuffers}}. + +flow_ctrl(#state{ssl_options = #{ktls := true}} = State, PBuffers) -> + {no_record, State#state{protocol_buffers = PBuffers}}; %%% bytes_to_read equals the integer Length arg of ssl:recv %%% the actual value is only relevant for packet = raw | 0 %%% bytes_to_read = undefined means no recv call is ongoing flow_ctrl(#state{user_data_buffer = {_,Size,_}, socket_options = #socket_options{active = false}, - bytes_to_read = undefined} = State) when Size =/= 0 -> + bytes_to_read = undefined} = State, + PBuffers) + when Size =/= 0 -> %% Passive mode wait for new recv request or socket activation %% that is preserve some tcp back pressure by waiting to activate %% socket - {no_record, State}; + {no_record, State#state{protocol_buffers = PBuffers}}; %%%%%%%%%% A packet mode is set and socket is passive %%%%%%%%%% flow_ctrl(#state{socket_options = #socket_options{active = false, - packet = Packet}} = State) + packet = Packet}} = State, + PBuffers) when ((Packet =/= 0) andalso (Packet =/= raw)) -> %% We need more data to complete the packet. - activate_socket(State); + activate_socket(State, PBuffers); %%%%%%%%% No packet mode set and socket is passive %%%%%%%%%%%% flow_ctrl(#state{user_data_buffer = {_,Size,_}, socket_options = #socket_options{active = false}, - bytes_to_read = 0} = State) when Size == 0 -> - %% Passive mode no available bytes, get some - activate_socket(State); + bytes_to_read = 0} = State, + PBuffers) + when Size == 0 -> + %% Passive mode no available bytes, get some + activate_socket(State, PBuffers); flow_ctrl(#state{user_data_buffer = {_,Size,_}, socket_options = #socket_options{active = false}, - bytes_to_read = 0} = State) when Size =/= 0 -> + bytes_to_read = 0} = State, + PBuffers) + when Size =/= 0 -> %% There is data in the buffer to deliver - {no_record, State}; + {no_record, State#state{protocol_buffers = PBuffers}}; flow_ctrl(#state{user_data_buffer = {_,Size,_}, socket_options = #socket_options{active = false}, - bytes_to_read = BytesToRead} = State) when (BytesToRead > 0) -> + bytes_to_read = BytesToRead} = State, + PBuffers) + when (BytesToRead > 0) -> case (Size >= BytesToRead) of true -> %% There is enough data bufferd - {no_record, State}; + {no_record, State#state{protocol_buffers = PBuffers}}; false -> %% We need more data to complete the delivery of size - activate_socket(State) + activate_socket(State, PBuffers) end; %%%%%%%%%%% Active mode or more data needed %%%%%%%%%% -flow_ctrl(State) -> - activate_socket(State). +flow_ctrl(State, PBuffers) -> + activate_socket(State, PBuffers). -activate_socket(#state{protocol_specific = - #{active_n_toggle := true, active_n := N} = ProtocolSpec, - static_env = #static_env{socket = Socket, - close_tag = CloseTag, - transport_cb = Transport} - } = State) -> +activate_socket(#state{protocol_specific = #{active_n_toggle := true} = ProtocolSpec, + static_env = StatEnv + } = State, + PBuffers) -> + do_activate_socket(ProtocolSpec, StatEnv), + {no_record, State#state{protocol_specific = ProtocolSpec#{active_n_toggle => false}, + protocol_buffers = PBuffers}}. + +do_activate_socket(#{active_n := N}, + #static_env{socket = Socket, close_tag = CloseTag, transport_cb = Transport}) -> case tls_socket:setopts(Transport, Socket, [{active, N}]) of - ok -> - {no_record, State#state{protocol_specific = ProtocolSpec#{active_n_toggle => false}}}; - _ -> - self() ! {CloseTag, Socket}, - {no_record, State} + ok -> ok; + _ -> self() ! {CloseTag, Socket} end. %% Decipher next record and concatenate consecutive ?APPLICATION_DATA records into one %% -next_record(State, CipherTexts, ConnectionStates, Check) -> - next_record(State, CipherTexts, ConnectionStates, Check, [], false). +next_record(State, CipherTexts, ConnectionStates, PBuffers, Check) -> + next_record(State, CipherTexts, ConnectionStates, Check, PBuffers, false, []). %% -next_record(#state{connection_env = #connection_env{negotiated_version = ?TLS_1_3 = Version}} = - State, [CT|CipherTexts], ConnectionStates0, Check, Acc, IsEarlyData) -> - case tls_record:decode_cipher_text(Version, CT, ConnectionStates0, Check) of - {Record = #ssl_tls{type = ?APPLICATION_DATA, fragment = Fragment}, ConnectionStates} -> +next_record(#state{connection_env = #connection_env{negotiated_version = ?TLS_1_3 = Vsn}} = State, + [CT|CipherTexts], ConnectionStates0, Check, PBuffers0, IsEarlyData, Acc) -> + case tls_record:decode_cipher_text(Vsn, CT, ConnectionStates0, Check) of + {Record0 = #ssl_tls{type = ?APPLICATION_DATA, fragment = Fragment0}, ConnectionStates} -> case CipherTexts of [] -> %% End of cipher texts - build and deliver an ?APPLICATION_DATA record %% from the accumulated fragments - NewFragment = iolist_to_binary(lists:reverse(Acc, [Fragment])), - next_record_done(State, [], ConnectionStates, - Record#ssl_tls{type = ?APPLICATION_DATA, - fragment = NewFragment}); + PBuffers = PBuffers0#protocol_buffers{tls_cipher_texts = []}, + Fragment = iolist_to_binary(lists:reverse(Acc, [Fragment0])), + Record = Record0#ssl_tls{type = ?APPLICATION_DATA, fragment = Fragment}, + next_record_done(State, PBuffers, ConnectionStates, Record); [_|_] -> - next_record(State, CipherTexts, ConnectionStates, - Check, [Fragment|Acc], Record#ssl_tls.early_data) + next_record(State, CipherTexts, ConnectionStates, Check, PBuffers0, + Record0#ssl_tls.early_data, [Fragment0|Acc]) end; {no_record, ConnectionStates} -> case CipherTexts of [] -> Record = accumulated_app_record(Acc, IsEarlyData), - next_record_done(State, [], ConnectionStates, Record); + PBuffers = PBuffers0#protocol_buffers{tls_cipher_texts = []}, + next_record_done(State, PBuffers, ConnectionStates, Record); [_|_] -> - next_record(State, CipherTexts, ConnectionStates, Check, Acc, IsEarlyData) + next_record(State, CipherTexts, ConnectionStates, Check, + PBuffers0, IsEarlyData, Acc) end; {Record, ConnectionStates} when Acc =:= [] -> %% Singleton non-?APPLICATION_DATA record - deliver - next_record_done(State, CipherTexts, ConnectionStates, Record); + PBuffers = PBuffers0#protocol_buffers{tls_cipher_texts = CipherTexts}, + next_record_done(State, PBuffers, ConnectionStates, Record); {_Record, _ConnectionStates_to_forget} -> %% Not ?APPLICATION_DATA but we have accumulated fragments %% -> build an ?APPLICATION_DATA record with concatenated fragments %% and forget about decrypting this record - we'll decrypt it again next time %% Will not work for stream ciphers - next_record_done(State, [CT|CipherTexts], ConnectionStates0, - #ssl_tls{type = ?APPLICATION_DATA, - early_data = IsEarlyData, - fragment = iolist_to_binary(lists:reverse(Acc))}); + PBuffers = PBuffers0#protocol_buffers{tls_cipher_texts = [CT|CipherTexts]}, + Fragment = iolist_to_binary(lists:reverse(Acc)), + Record = #ssl_tls{type = ?APPLICATION_DATA, + early_data = IsEarlyData, + fragment = Fragment}, + next_record_done(State, PBuffers, ConnectionStates0, Record); #alert{} = Alert -> Alert end; next_record(#state{connection_env = #connection_env{negotiated_version = Version}} = State, - [#ssl_tls{type = ?APPLICATION_DATA} = CT |CipherTexts], ConnectionStates0, - Check, Acc, NotRelevant) -> + [#ssl_tls{type = ?APPLICATION_DATA} = CT |CipherTexts], + ConnectionStates0, Check, PBuffers0, NotRelevant, Acc) -> case tls_record:decode_cipher_text(Version, CT, ConnectionStates0, Check) of - {Record = #ssl_tls{type = ?APPLICATION_DATA, fragment = Fragment}, ConnectionStates} -> + {Record0 = #ssl_tls{type = ?APPLICATION_DATA, fragment = Fragment0}, ConnectionStates} -> case CipherTexts of [] -> %% End of cipher texts - build and deliver an ?APPLICATION_DATA record %% from the accumulated fragments - NewFragment = iolist_to_binary(lists:reverse(Acc, [Fragment])), - next_record_done(State, [], ConnectionStates, - Record#ssl_tls{type = ?APPLICATION_DATA, - fragment = NewFragment}); + PBuffers = PBuffers0#protocol_buffers{tls_cipher_texts = []}, + Fragment = iolist_to_binary(lists:reverse(Acc, [Fragment0])), + Record = Record0#ssl_tls{type = ?APPLICATION_DATA,fragment = Fragment}, + next_record_done(State, PBuffers, ConnectionStates, Record); [_|_] -> - next_record(State, CipherTexts, ConnectionStates, Check, [Fragment|Acc], - NotRelevant) + next_record(State, CipherTexts, ConnectionStates, Check, + PBuffers0, NotRelevant, [Fragment0|Acc]) end; #alert{} = Alert -> Alert end; -next_record(State, CipherTexts, ConnectionStates, _, [_|_] = Acc, IsEarlyData) -> - next_record_done(State, CipherTexts, ConnectionStates, - #ssl_tls{type = ?APPLICATION_DATA, - early_data = IsEarlyData, - fragment = iolist_to_binary(lists:reverse(Acc))}); +next_record(State, CipherTexts, ConnectionStates, _, PBuffers0, IsEarlyData, [_|_] = Acc) -> + PBuffers = PBuffers0#protocol_buffers{tls_cipher_texts = CipherTexts}, + Fragment = iolist_to_binary(lists:reverse(Acc)), + Record = #ssl_tls{type = ?APPLICATION_DATA, early_data = IsEarlyData, fragment = Fragment}, + next_record_done(State, PBuffers, ConnectionStates, Record); next_record(#state{connection_env = #connection_env{negotiated_version = Version}} = State, - [CT|CipherTexts], ConnectionStates0, Check, [], _) -> - case tls_record:decode_cipher_text(Version, CT, ConnectionStates0, Check) of + [CT|CipherTexts], ConnectionStates0, Check, PBuffers0, _, []) -> + case tls_record:decode_cipher_text(Version, CT, ConnectionStates0, Check) of {Record, ConnectionStates} -> %% Singleton non-?APPLICATION_DATA record - deliver - next_record_done(State, CipherTexts, ConnectionStates, Record); + PBuffers = PBuffers0#protocol_buffers{tls_cipher_texts = CipherTexts}, + next_record_done(State, PBuffers, ConnectionStates, Record); #alert{} = Alert -> Alert end. @@ -838,11 +852,8 @@ accumulated_app_record([_|_] = Acc, IsEarlyData) -> early_data = IsEarlyData, fragment = iolist_to_binary(lists:reverse(Acc))}. -next_record_done(#state{protocol_buffers = Buffers} = State, CipherTexts, - ConnectionStates, Record) -> - {Record, - State#state{protocol_buffers = Buffers#protocol_buffers{tls_cipher_texts = CipherTexts}, - connection_states = ConnectionStates}}. +next_record_done(State, #protocol_buffers{} = PBuffers, ConnectionStates, Record) -> + {Record, State#state{protocol_buffers = PBuffers, connection_states = ConnectionStates}}. %% Pre TLS-1.3, on the client side, the connection state variable From 6b38ca77a94fcd0c16dade6efce67b6f5242ad15 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Mon, 22 Jan 2024 11:53:58 +0100 Subject: [PATCH 13/20] Use gen_statem reply So we don't need to allocate memory and process Action inside gen_statem. --- lib/ssl/src/dtls_gen_connection.erl | 29 +++++++++------ lib/ssl/src/ssl_gen_statem.erl | 38 +++++++++++-------- lib/ssl/src/tls_sender.erl | 58 ++++++++++++++++------------- 3 files changed, 72 insertions(+), 53 deletions(-) diff --git a/lib/ssl/src/dtls_gen_connection.erl b/lib/ssl/src/dtls_gen_connection.erl index 4a43aa3ef83f..23c1bac29575 100644 --- a/lib/ssl/src/dtls_gen_connection.erl +++ b/lib/ssl/src/dtls_gen_connection.erl @@ -406,8 +406,12 @@ handle_protocol_record(#ssl_tls{type = ?APPLICATION_DATA, fragment = Data}, Stat {stop, _, _} = Stop-> Stop; {Record, State1} -> - {next_state, StateName, State, Actions} = next_event(StateName0, Record, State1), - ssl_gen_statem:hibernate_after(StateName, State, Actions) + case next_event(StateName0, Record, State1) of + {next_state, StateName, State} -> + ssl_gen_statem:hibernate_after(StateName, State, []); + {next_state, StateName, State, Actions} -> + ssl_gen_statem:hibernate_after(StateName, State, Actions) + end end; %%% DTLS record protocol level handshake messages handle_protocol_record(#ssl_tls{type = ?HANDSHAKE, epoch = Epoch, fragment = Data}, @@ -510,15 +514,18 @@ new_timeout(_) -> 60000. handle_state_timeout(flight_retransmission_timeout, StateName, - #state{protocol_specific = - #{flight_state := {retransmit, CurrentTimeout}}} = State0) -> - {State1, Actions0} = send_handshake_flight(State0, - retransmit_epoch(StateName, State0)), - {next_state, StateName, #state{protocol_specific = PS} = State2, Actions} = - next_event(StateName, no_record, State1, Actions0), - State = State2#state{protocol_specific = PS#{flight_state => {retransmit, new_timeout(CurrentTimeout)}}}, - %% This will reset the retransmission timer by repeating the enter state event - {repeat_state, State, Actions}. + #state{protocol_specific = #{flight_state := {retransmit, CurrentTimeout}}} + = State0) -> + {State1, Actions0} = send_handshake_flight(State0, retransmit_epoch(StateName, State0)), + case next_event(StateName, no_record, State1, Actions0) of + %% This will reset the retransmission timer by repeating the enter state event + {next_state, StateName, #state{protocol_specific = PS0} = State, Actions} -> + PS = PS0#{flight_state => {retransmit, new_timeout(CurrentTimeout)}}, + {repeat_state, State#state{protocol_specific = PS}, Actions}; + {next_state, StateName, #state{protocol_specific = PS0} = State} -> + PS = PS0#{flight_state => {retransmit, new_timeout(CurrentTimeout)}}, + {repeat_state, State#state{protocol_specific = PS}} + end. send_handshake(Handshake, #state{connection_states = ConnectionStates} = State) -> #{epoch := Epoch} = ssl_record:current_connection_state(ConnectionStates, write), diff --git a/lib/ssl/src/ssl_gen_statem.erl b/lib/ssl/src/ssl_gen_statem.erl index 72f67c5ac671..46bf7d6c2287 100644 --- a/lib/ssl/src/ssl_gen_statem.erl +++ b/lib/ssl/src/ssl_gen_statem.erl @@ -940,6 +940,7 @@ read_application_data(Data, user_data_buffer = {Front,BufferSize,Rear}}} end end. + passive_receive(#state{user_data_buffer = {Front,BufferSize,Rear}, %% Assert! Erl distribution uses active sockets connection_env = #connection_env{erl_dist_handle = undefined}} @@ -967,16 +968,15 @@ passive_receive(#state{user_data_buffer = {Front,BufferSize,Rear}, %% Hibernation %%==================================================================== -hibernate_after(StateName, - #state{ssl_options= SslOpts} = State, - Actions) -> - HibernateAfter = maps:get(hibernate_after, SslOpts, infinity), - case HibernateAfter == infinity orelse StateName =/= connection of - true -> {next_state, StateName, State, Actions}; - false -> - {next_state, StateName, State, - [{timeout, HibernateAfter, hibernate} | Actions]} - end. +hibernate_after(connection = StateName, + #state{ssl_options= #{hibernate_after := HibernateAfter}} = State, + Actions) when HibernateAfter =/= infinity -> + {next_state, StateName, State, + [{timeout, HibernateAfter, hibernate} | Actions]}; +hibernate_after(StateName, State, []) -> + {next_state, StateName, State}; +hibernate_after(StateName, State, Actions) -> + {next_state, StateName, State, Actions}. %%==================================================================== %% Alert and close handling @@ -1371,7 +1371,8 @@ no_records(Extensions) -> end, Extensions). handle_active_option(false, connection = StateName, To, Reply, State) -> - hibernate_after(StateName, State, [{reply, To, Reply}]); + gen_statem:reply(To, Reply), + hibernate_after(StateName, State, []); handle_active_option(_, connection = StateName, To, Reply, #state{static_env = #static_env{role = Role}, @@ -1384,15 +1385,18 @@ handle_active_option(_, connection = StateName0, To, Reply, #state{static_env = user_data_buffer = {_,0,_}} = State0) -> case Connection:next_event(StateName0, no_record, State0) of {next_state, StateName, State} -> - hibernate_after(StateName, State, [{reply, To, Reply}]); + gen_statem:reply(To, Reply), + hibernate_after(StateName, State, []); {next_state, StateName, State, Actions} -> - hibernate_after(StateName, State, [{reply, To, Reply} | Actions]); + gen_statem:reply(To, Reply), + hibernate_after(StateName, State, Actions); {stop, _, _} = Stop -> Stop end; handle_active_option(_, StateName, To, Reply, #state{user_data_buffer = {_,0,_}} = State) -> %% Active once already set - {next_state, StateName, State, [{reply, To, Reply}]}; + gen_statem:reply(To, Reply), + {next_state, StateName, State}; %% user_data_buffer nonempty handle_active_option(_, StateName0, To, Reply, @@ -1404,9 +1408,11 @@ handle_active_option(_, StateName0, To, Reply, %% Note: Renogotiation may cause StateName0 =/= StateName case Connection:next_event(StateName0, Record, State1) of {next_state, StateName, State} -> - hibernate_after(StateName, State, [{reply, To, Reply}]); + gen_statem:reply(To, Reply), + hibernate_after(StateName, State, []); {next_state, StateName, State, Actions} -> - hibernate_after(StateName, State, [{reply, To, Reply} | Actions]); + gen_statem:reply(To, Reply), + hibernate_after(StateName, State, Actions); {stop, _, _} = Stop -> Stop end diff --git a/lib/ssl/src/tls_sender.erl b/lib/ssl/src/tls_sender.erl index b7a2bde8671e..505facf85deb 100644 --- a/lib/ssl/src/tls_sender.erl +++ b/lib/ssl/src/tls_sender.erl @@ -504,17 +504,18 @@ send_tls_alert(#alert{} = Alert, StateData0#data{connection_states = ConnectionStates}. send_application_data(Data, From, StateName, - #data{static = #static{connection_pid = Pid, - socket = Socket, - dist_handle = DistHandle, - negotiated_version = Version, - transport_cb = Transport, - renegotiate_at = RenegotiateAt, - key_update_at = KeyUpdateAt, - log_level = LogLevel}, - bytes_sent = BytesSent, - connection_states = ConnectionStates0} = StateData0) -> - case time_to_rekey(Version, Data, ConnectionStates0, RenegotiateAt, KeyUpdateAt, BytesSent) of + #data{static = #static{connection_pid = Pid, + socket = Socket, + dist_handle = DistHandle, + negotiated_version = Version, + transport_cb = Transport, + renegotiate_at = RenegotiateAt, + key_update_at = KeyUpdateAt, + log_level = LogLevel}, + bytes_sent = BytesSent, + connection_states = ConnectionStates0} = StateData0) -> + DataSz = iolist_size(Data), + 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}}, @@ -532,20 +533,23 @@ send_application_data(Data, From, StateName, {next_event, internal, {application_packets, From, Rest}}]}; false -> {Msgs, ConnectionStates} = tls_record:encode_data(Data, Version, ConnectionStates0), - StateData = StateData0#data{connection_states = ConnectionStates}, case tls_socket:send(Transport, Socket, Msgs) of ok when DistHandle =/= undefined -> ssl_logger:debug(LogLevel, outbound, 'record', Msgs), - StateData1 = update_bytes_sent(Version, StateData, Data), + 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), - StateData1 = update_bytes_sent(Version, StateData, Data), - hibernate_after(StateName, StateData1, [{reply, From, ok}]); + StateData = update_bytes_sent(Version, ConnectionStates, StateData0, DataSz), + gen_statem:reply(From, ok), + hibernate_after(StateName, StateData, []); Result -> - hibernate_after(StateName, StateData, [{reply, From, Result}]) + gen_statem:reply(From, Result), + StateData = StateData0#data{connection_states = ConnectionStates}, + hibernate_after(StateName, StateData, []) end end. @@ -584,13 +588,14 @@ maybe_update_cipher_key(#data{connection_states = ConnectionStates0}= StateData, maybe_update_cipher_key(StateData, _) -> StateData. -update_bytes_sent(Version, StateData, _) when ?TLS_LT(Version, ?TLS_1_3) -> - StateData; +update_bytes_sent(Version, CS, StateData, _) when ?TLS_LT(Version, ?TLS_1_3) -> + StateData#data{connection_states = CS}; %% Count bytes sent in TLS 1.3 for AES-GCM -update_bytes_sent(_, #data{static = #static{key_update_at = seq_num_wrap}} = StateData, _) -> - StateData; %% Chacha20-Poly1305 -update_bytes_sent(_, #data{bytes_sent = Sent} = StateData, Data) -> - StateData#data{bytes_sent = Sent + iolist_size(Data)}. %% AES-GCM +update_bytes_sent(_, CS, #data{static = #static{key_update_at = seq_num_wrap}} = StateData, _) -> + StateData#data{connection_states = CS}; %% Chacha20-Poly1305 +update_bytes_sent(_, CS, #data{bytes_sent = Sent} = StateData, DataSz) -> + StateData#data{connection_states = CS, + bytes_sent = Sent + DataSz}. %% AES-GCM %% For AES-GCM, up to 2^24.5 full-size records (about 24 million) may be %% encrypted on a given connection while keeping a safety margin of @@ -623,14 +628,13 @@ encode_packet(Packet, Data) -> set_opts(SocketOptions, [{packet, N}]) -> SocketOptions#socket_options{packet = N}. -time_to_rekey(Version, _Data, +time_to_rekey(Version, _DataSz, #{current_write := #{sequence_number := ?MAX_SEQUENCE_NUMBER}}, _, _, _) when ?TLS_GTE(Version, ?TLS_1_3) -> key_update; -time_to_rekey(Version, _Data, _, _, seq_num_wrap, _) when ?TLS_GTE(Version, ?TLS_1_3) -> +time_to_rekey(Version, _DataSz, _, _, seq_num_wrap, _) when ?TLS_GTE(Version, ?TLS_1_3) -> false; -time_to_rekey(Version, Data, _, _, KeyUpdateAt, BytesSent) when ?TLS_GTE(Version, ?TLS_1_3) -> - DataSize = iolist_size(Data), +time_to_rekey(Version, DataSize, _, _, KeyUpdateAt, BytesSent) when ?TLS_GTE(Version, ?TLS_1_3) -> case (BytesSent + DataSize) > KeyUpdateAt of true -> %% Handle special case that causes an invite loop of key updates. @@ -720,6 +724,8 @@ hibernate_after(connection = StateName, #data{static=#static{hibernate_after = HibernateAfter}} = State, Actions) when HibernateAfter =/= infinity -> {next_state, StateName, State, [{timeout, HibernateAfter, hibernate} | Actions]}; +hibernate_after(StateName, State, []) -> + {next_state, StateName, State}; hibernate_after(StateName, State, Actions) -> {next_state, StateName, State, Actions}. From b938309de845623f7221638db0f4c5e839a6100b Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Tue, 23 Jan 2024 09:13:47 +0100 Subject: [PATCH 14/20] Move client_certificate_status from state to handshake env To reduce the state record. --- lib/ssl/src/ssl_connection.hrl | 6 +- lib/ssl/src/tls_client_connection_1_3.erl | 2 +- lib/ssl/src/tls_dtls_client_connection.erl | 25 ++++--- lib/ssl/src/tls_dtls_server_connection.erl | 78 ++++++++++++---------- lib/ssl/src/tls_handshake_1_3.erl | 4 +- lib/ssl/src/tls_server_connection.erl | 7 +- 6 files changed, 72 insertions(+), 50 deletions(-) diff --git a/lib/ssl/src/ssl_connection.hrl b/lib/ssl/src/ssl_connection.hrl index a5a4f7f3644e..b1437bf742f0 100644 --- a/lib/ssl/src/ssl_connection.hrl +++ b/lib/ssl/src/ssl_connection.hrl @@ -88,6 +88,9 @@ server_psk_identity :: binary() | 'undefined', % server psk identity hint cookie_iv_shard :: {binary(), binary()} %% IV, Shard | 'undefined', + client_certificate_status = not_requested :: not_requested | requested | + empty | needs_verifying | + verified, stapling_state = #{configured => false, status => not_negotiated} }). @@ -122,8 +125,7 @@ %% mechanism is also useful in TLS although we do not %% need to worry about packet loss in TLS. In DTLS we %% need to track DTLS handshake seqnr - flight_buffer = [] :: list() | map(), - client_certificate_status = not_requested :: not_requested | requested | empty | needs_verifying | verified, + flight_buffer = [] :: list() | map(), protocol_specific = #{} :: map(), session :: #session{} | secret_printout(), key_share, diff --git a/lib/ssl/src/tls_client_connection_1_3.erl b/lib/ssl/src/tls_client_connection_1_3.erl index 093c89874c4b..ef3b14cc7672 100644 --- a/lib/ssl/src/tls_client_connection_1_3.erl +++ b/lib/ssl/src/tls_client_connection_1_3.erl @@ -933,7 +933,7 @@ cipher_hash_algos(Ciphers) -> end, lists:map(Fun, Ciphers). -maybe_queue_cert_cert_cv(#state{client_certificate_status = not_requested} +maybe_queue_cert_cert_cv(#state{handshake_env = #handshake_env{client_certificate_status = not_requested}} = State) -> {ok, State}; maybe_queue_cert_cert_cv(#state{connection_states = _ConnectionStates0, diff --git a/lib/ssl/src/tls_dtls_client_connection.erl b/lib/ssl/src/tls_dtls_client_connection.erl index 5496ea9a9342..bf9d8a50dfda 100644 --- a/lib/ssl/src/tls_dtls_client_connection.erl +++ b/lib/ssl/src/tls_dtls_client_connection.erl @@ -253,12 +253,14 @@ certify(internal, #certificate_request{}, certify(internal, #certificate_request{}, #state{static_env = #static_env{protocol_cb = Connection}, session = Session0, + handshake_env = HsEnv0, connection_env = #connection_env{cert_key_alts = [#{certs := [[]]}]}} = State) -> %% The client does not have a certificate and will send an empty reply, the server may fail %% or accept the connection by its own preference. No signature algorithms needed as there is %% no certificate to verify. + HsEnv = HsEnv0#handshake_env{client_certificate_status = requested}, Connection:next_event(?STATE(certify), no_record, - State#state{client_certificate_status = requested, + State#state{handshake_env = HsEnv, session = Session0#session{own_certificates = [[]], private_key = #{}}}); certify(internal, #certificate_request{} = CertRequest, @@ -268,6 +270,7 @@ certify(internal, #certificate_request{} = CertRequest, connection_env = #connection_env{negotiated_version = Version, cert_key_alts = CertKeyAlts }, + handshake_env = HsEnv0, session = Session0, ssl_options = #{signature_algs := SupportedHashSigns}} = State) -> TLSVersion = ssl:tls_version(Version), @@ -275,9 +278,9 @@ certify(internal, #certificate_request{} = CertRequest, Session = select_client_cert_key_pair(Session0, CertRequest, CertKeyPairs, SupportedHashSigns, TLSVersion, CertDbHandle, CertDbRef), + HsEnv = HsEnv0#handshake_env{client_certificate_status = requested}, Connection:next_event(?STATE(certify), no_record, - State#state{client_certificate_status = requested, - session = Session}); + State#state{handshake_env = HsEnv, session = Session}); %% PSK and RSA_PSK might bypass the Server-Key-Exchange certify(internal, #server_hello_done{}, #state{static_env = #static_env{protocol_cb = Connection}, @@ -626,7 +629,9 @@ calculate_secret(#server_srp_params{srp_n = Prime, srp_g = Generator} = ServerKe client_certify_and_key_exchange(State0, Connection) -> State1 = do_client_certify_and_key_exchange(State0, Connection), {State2, Actions} = tls_dtls_gen_connection:finalize_handshake(State1, certify, Connection), - State = State2#state{client_certificate_status = not_requested}, %% Reinitialize + #state{handshake_env = HsEnv0} = State2, + HsEnv = HsEnv0#handshake_env{client_certificate_status = not_requested}, + State = State2#state{handshake_env = HsEnv}, %% Reinitialize Connection:next_event(cipher, no_record, State, Actions). do_client_certify_and_key_exchange(State0, Connection) -> @@ -636,12 +641,13 @@ do_client_certify_and_key_exchange(State0, Connection) -> certify_client(#state{static_env = #static_env{cert_db = CertDbHandle, cert_db_ref = CertDbRef}, - client_certificate_status = requested, + handshake_env = #handshake_env{client_certificate_status = requested}, session = #session{own_certificates = OwnCerts}} = State, Connection) -> Certificate = ssl_handshake:certificate(OwnCerts, CertDbHandle, CertDbRef, client), Connection:queue_handshake(Certificate, State); -certify_client(#state{client_certificate_status = not_requested} = State, _) -> +certify_client(#state{handshake_env = #handshake_env{client_certificate_status = not_requested} + } = State, _) -> State. key_exchange(#state{handshake_env = #handshake_env{kex_algorithm = rsa, @@ -759,9 +765,9 @@ generate_srp_client_keys(Generator, Prime, N) -> generate_srp_client_keys(Generator, Prime, N+1) end. -verify_client_cert(#state{handshake_env = #handshake_env{tls_handshake_history = Hist}, +verify_client_cert(#state{handshake_env = #handshake_env{tls_handshake_history = Hist, + client_certificate_status = requested}, connection_env = #connection_env{negotiated_version = Version}, - client_certificate_status = requested, session = #session{sign_alg = HashSign, master_secret = MasterSecret, private_key = PrivateKey, @@ -775,7 +781,8 @@ verify_client_cert(#state{handshake_env = #handshake_env{tls_handshake_history = #alert{} = Alert -> throw(Alert) end; -verify_client_cert(#state{client_certificate_status = not_requested} = State, _) -> +verify_client_cert(#state{handshake_env = #handshake_env{client_certificate_status = not_requested} + } = State, _) -> State. handle_peer_cert(Role, PeerCert, PublicKeyInfo, diff --git a/lib/ssl/src/tls_dtls_server_connection.erl b/lib/ssl/src/tls_dtls_server_connection.erl index 3be78c80145d..d318f86fa497 100644 --- a/lib/ssl/src/tls_dtls_server_connection.erl +++ b/lib/ssl/src/tls_dtls_server_connection.erl @@ -145,11 +145,12 @@ certify(internal, #certificate{asn1_certificates = []}, certify(internal, #certificate{asn1_certificates = []}, #state{static_env = #static_env{role = server, protocol_cb = Connection}, + handshake_env = HsEnv0, ssl_options = #{verify := verify_peer, fail_if_no_peer_cert := false}} = State0) -> - Connection:next_event(?STATE(certify), no_record, - State0#state{client_certificate_status = empty}); + HsEnv = HsEnv0#handshake_env{client_certificate_status = empty}, + Connection:next_event(?STATE(certify), no_record, State0#state{handshake_env = HsEnv}); certify(internal, #certificate{}, #state{ssl_options = #{verify := verify_none}}) -> throw(?ALERT_REC(?FATAL,?UNEXPECTED_MESSAGE, unrequested_certificate)); @@ -161,7 +162,7 @@ certify(internal, #certificate{asn1_certificates = [Peer|_]} = Cert, cert_db = CertDbHandle, cert_db_ref = CertDbRef, crl_db = CRLDbInfo}, - handshake_env = #handshake_env{stapling_state = StaplingState}, + handshake_env = #handshake_env{stapling_state = StaplingState} = HsEnv0, connection_env = #connection_env{ negotiated_version = Version}, ssl_options = Opts} = State0) -> @@ -172,15 +173,16 @@ certify(internal, #certificate{asn1_certificates = [Peer|_]} = Cert, ssl:tls_version(Version), ExtInfo) of {PeerCert, PublicKeyInfo} -> - State = State0#state{client_certificate_status = needs_verifying}, - tls_dtls_gen_connection:handle_peer_cert(Role, PeerCert, PublicKeyInfo, State, - Connection, []); + HsEnv = HsEnv0#handshake_env{client_certificate_status = needs_verifying}, + State = State0#state{handshake_env = HsEnv}, + tls_dtls_gen_connection:handle_peer_cert(Role, PeerCert, PublicKeyInfo, State, + Connection, []); #alert{} = Alert -> throw(Alert) end; certify(internal = Type, #client_key_exchange{} = Msg, - #state{client_certificate_status = requested, - ssl_options = #{fail_if_no_peer_cert := true}}) -> + #state{handshake_env = #handshake_env{client_certificate_status = requested}, + ssl_options = #{fail_if_no_peer_cert := true}}) -> %% We expect a certificate here throw(?ALERT_REC(?FATAL,?UNEXPECTED_MESSAGE, {unexpected_msg, {Type, Msg}})); certify(internal, #client_key_exchange{exchange_keys = Keys}, @@ -207,10 +209,10 @@ certify(Type, Event, State) -> wait_cert_verify(internal, #certificate_verify{signature = Signature, hashsign_algorithm = CertHashSign}, #state{static_env = #static_env{protocol_cb = Connection}, - client_certificate_status = needs_verifying, handshake_env = #handshake_env{tls_handshake_history = Hist, kex_algorithm = KexAlg, - public_key_info = PubKeyInfo}, + client_certificate_status = needs_verifying, + public_key_info = PubKeyInfo} = HsEnv0, connection_env = #connection_env{negotiated_version = Version}, session = #session{master_secret = MasterSecret} = Session0 } = State) -> @@ -222,8 +224,9 @@ wait_cert_verify(internal, #certificate_verify{signature = Signature, case ssl_handshake:certificate_verify(Signature, PubKeyInfo, TLSVersion, HashSign, MasterSecret, Hist) of valid -> + HsEnv = HsEnv0#handshake_env{client_certificate_status = verified}, Connection:next_event(cipher, no_record, - State#state{client_certificate_status = verified, + State#state{handshake_env = HsEnv, session = Session0#session{sign_alg = HashSign}}); #alert{} = Alert -> throw(Alert) @@ -648,8 +651,9 @@ request_client_cert(#state{static_env = #static_env{cert_db = CertDbHandle, TLSVersion), Msg = ssl_handshake:certificate_request(CertDbHandle, CertDbRef, HashSigns, TLSVersion), - State = Connection:queue_handshake(Msg, State0), - State#state{client_certificate_status = requested}; + #state{handshake_env = HsEnv0} = State = Connection:queue_handshake(Msg, State0), + HsEnv = HsEnv0#handshake_env{client_certificate_status = requested}, + State#state{handshake_env = HsEnv}; request_client_cert(#state{ssl_options = #{verify := verify_none}} = State, _) -> @@ -657,11 +661,12 @@ request_client_cert(#state{ssl_options = #{verify := verify_none}} = certify_client_key_exchange(#encrypted_premaster_secret{premaster_secret= EncPMS}, #state{session = #session{private_key = PrivateKey}, - handshake_env = - #handshake_env{client_hello_version = {Major, Minor} - = Version}, - client_certificate_status = CCStatus} - = State, Connection) -> + handshake_env = #handshake_env{ + client_hello_version = Version, + client_certificate_status = CCStatus + } + } = State, Connection) -> + {Major, Minor} = Version, FakeSecret = tls_dtls_gen_connection:make_premaster_secret(Version, rsa), %% Countermeasure for Bleichenbacher attack always provide some kind of premaster secret %% and fail handshake later.RFC 5246 section 7.4.7.1. @@ -686,8 +691,8 @@ certify_client_key_exchange(#client_diffie_hellman_public{dh_public = ClientPubl #state{handshake_env = #handshake_env{diffie_hellman_params = #'DHParameter'{} = Params, - kex_keys = {_, ServerDhPrivateKey}}, - client_certificate_status = CCStatus + kex_keys = {_, ServerDhPrivateKey}, + client_certificate_status = CCStatus} } = State, Connection) -> PremasterSecret = ssl_handshake:premaster_secret(ClientPublicDhKey, @@ -697,8 +702,9 @@ certify_client_key_exchange(#client_diffie_hellman_public{dh_public = ClientPubl client_kex_next_state(CCStatus)); certify_client_key_exchange(#client_ec_diffie_hellman_public{dh_public = ClientPublicEcDhPoint}, - #state{handshake_env = #handshake_env{kex_keys = ECDHKey}, - client_certificate_status = CCStatus + #state{handshake_env = + #handshake_env{kex_keys = ECDHKey, + client_certificate_status = CCStatus} } = State, Connection) -> PremasterSecret = ssl_handshake:premaster_secret(#'ECPoint'{point = ClientPublicEcDhPoint}, ECDHKey), @@ -708,7 +714,8 @@ certify_client_key_exchange(#client_ec_diffie_hellman_public{dh_public = ClientP certify_client_key_exchange(#client_psk_identity{} = ClientKey, #state{ssl_options = #{user_lookup_fun := PSKLookup}, - client_certificate_status = CCStatus + handshake_env = + #handshake_env{client_certificate_status = CCStatus} } = State0, Connection) -> PremasterSecret = ssl_handshake:premaster_secret(ClientKey, PSKLookup), @@ -719,10 +726,11 @@ certify_client_key_exchange(#client_dhe_psk_identity{} = ClientKey, #state{handshake_env = #handshake_env{diffie_hellman_params = #'DHParameter'{} = Params, - kex_keys = {_, ServerDhPrivateKey}}, + kex_keys = {_, ServerDhPrivateKey}, + client_certificate_status = CCStatus + }, ssl_options = - #{user_lookup_fun := PSKLookup}, - client_certificate_status = CCStatus + #{user_lookup_fun := PSKLookup} } = State0, Connection) -> PremasterSecret = @@ -731,10 +739,10 @@ certify_client_key_exchange(#client_dhe_psk_identity{} = ClientKey, Connection, certify, client_kex_next_state(CCStatus)); certify_client_key_exchange(#client_ecdhe_psk_identity{} = ClientKey, - #state{handshake_env = #handshake_env{kex_keys = ServerEcDhPrivateKey}, - ssl_options = - #{user_lookup_fun := PSKLookup}, - client_certificate_status = CCStatus + #state{handshake_env = + #handshake_env{kex_keys = ServerEcDhPrivateKey, + client_certificate_status = CCStatus}, + ssl_options = #{user_lookup_fun := PSKLookup} } = State, Connection) -> PremasterSecret = @@ -746,7 +754,8 @@ certify_client_key_exchange(#client_rsa_psk_identity{} = ClientKey, #state{session = #session{private_key = PrivateKey}, ssl_options = #{user_lookup_fun := PSKLookup}, - client_certificate_status = CCStatus + handshake_env = + #handshake_env{client_certificate_status = CCStatus} } = State0, Connection) -> PremasterSecret = ssl_handshake:premaster_secret(ClientKey, PrivateKey, PSKLookup), @@ -754,9 +763,10 @@ certify_client_key_exchange(#client_rsa_psk_identity{} = ClientKey, Connection, certify, client_kex_next_state(CCStatus)); certify_client_key_exchange(#client_srp_public{} = ClientKey, - #state{handshake_env = #handshake_env{srp_params = Params, - kex_keys = Key}, - client_certificate_status = CCStatus + #state{handshake_env = + #handshake_env{srp_params = Params, + kex_keys = Key, + client_certificate_status = CCStatus} } = State0, Connection) -> PremasterSecret = ssl_handshake:premaster_secret(ClientKey, Key, Params), tls_dtls_gen_connection:calculate_master_secret(PremasterSecret, State0, diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl index b5884ad2c5ba..fd02efbeabb6 100644 --- a/lib/ssl/src/tls_handshake_1_3.erl +++ b/lib/ssl/src/tls_handshake_1_3.erl @@ -408,6 +408,7 @@ process_certificate_request(#certificate_request_1_3{ connection_env = #connection_env{cert_key_alts = CertKeyAlts, negotiated_version = Version}, static_env = #static_env{cert_db = CertDbHandle, cert_db_ref = CertDbRef}, + handshake_env = HsEnv, session = Session0} = State) -> ServerSignAlgs = get_signature_scheme_list( @@ -420,7 +421,8 @@ process_certificate_request(#certificate_request_1_3{ Session = select_client_cert_key_pair(Session0, CertKeyPairs, ServerSignAlgs, ServerSignAlgsCert, ClientSignAlgs, CertDbHandle, CertDbRef, CertAuths, undefined), - {ok, {State#state{client_certificate_status = requested, session = Session}, wait_cert}}. + {ok, {State#state{handshake_env = HsEnv#handshake_env{client_certificate_status = requested}, + session = Session}, wait_cert}}. process_certificate(#certificate_1_3{ certificate_request_context = <<>>, diff --git a/lib/ssl/src/tls_server_connection.erl b/lib/ssl/src/tls_server_connection.erl index 4d541b856c77..c7a8b94b8b49 100644 --- a/lib/ssl/src/tls_server_connection.erl +++ b/lib/ssl/src/tls_server_connection.erl @@ -258,10 +258,10 @@ wait_cert_verify(info, Event, State) -> wait_cert_verify(internal, #certificate_verify{signature = Signature, hashsign_algorithm = CertHashSign}, #state{static_env = #static_env{protocol_cb = Connection}, - client_certificate_status = needs_verifying, handshake_env = #handshake_env{tls_handshake_history = Hist, kex_algorithm = KexAlg, - public_key_info = PubKeyInfo}, + client_certificate_status = needs_verifying, + public_key_info = PubKeyInfo} = HsEnv0, connection_env = #connection_env{negotiated_version = Version}, session = #session{master_secret = MasterSecret} = Session0 } = State) -> @@ -273,8 +273,9 @@ wait_cert_verify(internal, #certificate_verify{signature = Signature, case ssl_handshake:certificate_verify(Signature, PubKeyInfo, TLSVersion, HashSign, MasterSecret, Hist) of valid -> + HsEnv = HsEnv0#handshake_env{client_certificate_status = verified}, Connection:next_event(cipher, no_record, - State#state{client_certificate_status = verified, + State#state{handshake_env = HsEnv, session = Session0#session{sign_alg = HashSign}}); #alert{} = Alert -> throw(Alert) From 2120ac8c02498ce556f84f588f5919e6a5d21c4a Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Tue, 23 Jan 2024 15:07:08 +0100 Subject: [PATCH 15/20] Move flight_buffer to handshake env --- lib/ssl/src/dtls_gen_connection.erl | 86 ++++++++++++++----------- lib/ssl/src/dtls_server_connection.erl | 2 +- lib/ssl/src/ssl_connection.hrl | 19 +++--- lib/ssl/src/ssl_gen_statem.erl | 3 +- lib/ssl/src/tls_dtls_gen_connection.erl | 4 +- lib/ssl/src/tls_gen_connection.erl | 21 +++--- lib/ssl/src/tls_gen_connection_1_3.erl | 18 +++--- lib/ssl/src/tls_handshake_1_3.erl | 10 +-- 8 files changed, 89 insertions(+), 74 deletions(-) diff --git a/lib/ssl/src/dtls_gen_connection.erl b/lib/ssl/src/dtls_gen_connection.erl index 23c1bac29575..67573f4a6f7d 100644 --- a/lib/ssl/src/dtls_gen_connection.erl +++ b/lib/ssl/src/dtls_gen_connection.erl @@ -119,7 +119,8 @@ initial_state(Role, Host, Port, Socket, handshake_env = #handshake_env{ tls_handshake_history = ssl_handshake:init_handshake_history(), renegotiation = {false, first}, - allow_renegotiate = maps:get(client_renegotiation, SSLOptions, undefined) + allow_renegotiate = maps:get(client_renegotiation, SSLOptions, undefined), + flight_buffer = new_flight() }, connection_env = #connection_env{user_application = {Monitor, User}}, socket_options = SocketOptions, @@ -129,7 +130,6 @@ initial_state(Role, Host, Port, Socket, protocol_buffers = #protocol_buffers{}, user_data_buffer = {[],0,[]}, start_or_recv_from = undefined, - flight_buffer = new_flight(), protocol_specific = #{active_n => InternalActiveN, active_n_toggle => true, flight_state => initial_flight_state(DataTag), @@ -315,8 +315,9 @@ send_handshake_flight(#state{static_env = #static_env{socket = Socket, transport_cb = Transport, data_tag = DataTag}, connection_env = #connection_env{negotiated_version = Version}, - flight_buffer = #{handshakes := Flight, - change_cipher_spec := undefined}, + handshake_env = #handshake_env{ + flight_buffer = #{handshakes := Flight, + change_cipher_spec := undefined}}, connection_states = ConnectionStates0, ssl_options = #{log_level := LogLevel}} = State0, Epoch) -> @@ -332,9 +333,11 @@ send_handshake_flight(#state{static_env = #static_env{socket = Socket, transport_cb = Transport, data_tag = DataTag}, connection_env = #connection_env{negotiated_version = Version}, - flight_buffer = #{handshakes := [_|_] = Flight0, - change_cipher_spec := ChangeCipher, - handshakes_after_change_cipher_spec := []}, + handshake_env = #handshake_env{ + flight_buffer = + #{handshakes := [_|_] = Flight0, + change_cipher_spec := ChangeCipher, + handshakes_after_change_cipher_spec := []}}, connection_states = ConnectionStates0, ssl_options = #{log_level := LogLevel}} = State0, Epoch) -> @@ -352,9 +355,11 @@ send_handshake_flight(#state{static_env = #static_env{socket = Socket, transport_cb = Transport, data_tag = DataTag}, connection_env = #connection_env{negotiated_version = Version}, - flight_buffer = #{handshakes := [_|_] = Flight0, - change_cipher_spec := ChangeCipher, - handshakes_after_change_cipher_spec := Flight1}, + handshake_env = #handshake_env{ + flight_buffer = + #{handshakes := [_|_] = Flight0, + change_cipher_spec := ChangeCipher, + handshakes_after_change_cipher_spec := Flight1}}, connection_states = ConnectionStates0, ssl_options = #{log_level := LogLevel}} = State0, Epoch) -> @@ -376,9 +381,11 @@ send_handshake_flight(#state{static_env = #static_env{socket = Socket, transport_cb = Transport, data_tag = DataTag}, connection_env = #connection_env{negotiated_version = Version}, - flight_buffer = #{handshakes := [], - change_cipher_spec := ChangeCipher, - handshakes_after_change_cipher_spec := Flight1}, + handshake_env = #handshake_env{ + flight_buffer = + #{handshakes := [], + change_cipher_spec := ChangeCipher, + handshakes_after_change_cipher_spec := Flight1}}, connection_states = ConnectionStates0, ssl_options = #{log_level := LogLevel}} = State0, Epoch) -> @@ -473,12 +480,12 @@ gen_info(Event, StateName, State) -> alert_or_reset_connection(Alert, StateName, State) end. -prepare_flight(#state{flight_buffer = Flight, +prepare_flight(#state{handshake_env = #handshake_env{flight_buffer = Flight} = HsEnv, connection_states = ConnectionStates0, protocol_buffers = #protocol_buffers{} = Buffers} = State) -> ConnectionStates = dtls_record:save_current_connection_state(ConnectionStates0, write), - State#state{flight_buffer = next_flight(Flight), + State#state{handshake_env = HsEnv#handshake_env{flight_buffer = next_flight(Flight)}, connection_states = ConnectionStates, protocol_buffers = Buffers#protocol_buffers{ dtls_handshake_next_fragments = [], @@ -531,39 +538,46 @@ send_handshake(Handshake, #state{connection_states = ConnectionStates} = State) #{epoch := Epoch} = ssl_record:current_connection_state(ConnectionStates, write), send_handshake_flight(queue_handshake(Handshake, State), Epoch). -queue_handshake(Handshake0, #state{handshake_env = #handshake_env{tls_handshake_history = Hist0} = HsEnv, +queue_handshake(Handshake0, #state{handshake_env = + #handshake_env{tls_handshake_history = Hist0, + flight_buffer = + #{handshakes := HsBuffer0, + change_cipher_spec := undefined, + next_sequence := Seq} = Flight0 + } = HsEnv0, connection_env = #connection_env{negotiated_version = Version}, - flight_buffer = #{handshakes := HsBuffer0, - change_cipher_spec := undefined, - next_sequence := Seq} = Flight0, ssl_options = #{log_level := LogLevel}} = State) -> Handshake = dtls_handshake:encode_handshake(Handshake0, Version, Seq), Hist = update_handshake_history(Handshake0, Handshake, Hist0), ssl_logger:debug(LogLevel, outbound, 'handshake', Handshake0), - State#state{flight_buffer = Flight0#{handshakes => [Handshake | HsBuffer0], - next_sequence => Seq +1}, - handshake_env = HsEnv#handshake_env{tls_handshake_history = Hist}}; + Flight = Flight0#{handshakes => [Handshake | HsBuffer0], next_sequence => Seq +1}, + HsEnv = HsEnv0#handshake_env{tls_handshake_history = Hist, flight_buffer = Flight}, + State#state{handshake_env = HsEnv}; -queue_handshake(Handshake0, #state{handshake_env = #handshake_env{tls_handshake_history = Hist0} = HsEnv, +queue_handshake(Handshake0, #state{handshake_env = + #handshake_env{ + tls_handshake_history = Hist0, + flight_buffer = + #{handshakes_after_change_cipher_spec := Buffer0, + next_sequence := Seq} = Flight0} = HsEnv0, connection_env = #connection_env{negotiated_version = Version}, - flight_buffer = #{handshakes_after_change_cipher_spec := Buffer0, - next_sequence := Seq} = Flight0, ssl_options = #{log_level := LogLevel}} = State) -> Handshake = dtls_handshake:encode_handshake(Handshake0, Version, Seq), Hist = update_handshake_history(Handshake0, Handshake, Hist0), ssl_logger:debug(LogLevel, outbound, 'handshake', Handshake0), - State#state{flight_buffer = Flight0#{handshakes_after_change_cipher_spec => [Handshake | Buffer0], - next_sequence => Seq +1}, - handshake_env = HsEnv#handshake_env{tls_handshake_history = Hist}}. + Flight = Flight0#{handshakes_after_change_cipher_spec => [Handshake | Buffer0], + next_sequence => Seq +1}, + HsEnv = HsEnv0#handshake_env{tls_handshake_history = Hist, flight_buffer = Flight}, + State#state{handshake_env = HsEnv}. -queue_change_cipher(ChangeCipher, #state{flight_buffer = Flight, - connection_states = ConnectionStates0} = State) -> - ConnectionStates = - dtls_record:next_epoch(ConnectionStates0, write), - State#state{flight_buffer = Flight#{change_cipher_spec => ChangeCipher}, - connection_states = ConnectionStates}. +queue_change_cipher(ChangeCipher, #state{handshake_env = + #handshake_env{flight_buffer = Flight} = HsEnv0, + connection_states = ConnectionStates0} = State) -> + ConnectionStates = dtls_record:next_epoch(ConnectionStates0, write), + HsEnv = HsEnv0#handshake_env{flight_buffer = Flight#{change_cipher_spec => ChangeCipher}}, + State#state{handshake_env = HsEnv, connection_states = ConnectionStates}. reinit(State0) -> %% To be API compatible with TLS NOOP here @@ -577,9 +591,9 @@ reinit_handshake_data(#state{static_env = #static_env{data_tag = DataTag}, handshake_env = HsEnv} = State) -> State#state{handshake_env = HsEnv#handshake_env{tls_handshake_history = ssl_handshake:init_handshake_history(), public_key_info = undefined, - premaster_secret = undefined}, + premaster_secret = undefined, + flight_buffer = new_flight()}, protocol_specific = PS#{flight_state => initial_flight_state(DataTag)}, - flight_buffer = new_flight(), protocol_buffers = Buffers#protocol_buffers{ dtls_handshake_next_seq = 0, diff --git a/lib/ssl/src/dtls_server_connection.erl b/lib/ssl/src/dtls_server_connection.erl index c23a4bbc32e6..3ba104802a90 100644 --- a/lib/ssl/src/dtls_server_connection.erl +++ b/lib/ssl/src/dtls_server_connection.erl @@ -226,7 +226,6 @@ hello(internal, #client_hello{cookie = <<>>, client_version = Version} = Hello, #state{static_env = #static_env{transport_cb = Transport, socket = Socket}, - handshake_env = HsEnv, connection_env = CEnv, protocol_specific = #{current_cookie_secret := Secret}} = State0) -> try tls_dtls_server_connection:handle_sni_extension(State0, Hello) of @@ -245,6 +244,7 @@ hello(internal, #client_hello{cookie = <<>>, State1#state{connection_env = CEnv#connection_env{negotiated_version = Version}}), {State, Actions} = dtls_gen_connection:send_handshake(VerifyRequest, State2), + #state{handshake_env = HsEnv} = State, NewHSEnv = HsEnv#handshake_env{tls_handshake_history = ssl_handshake:init_handshake_history()}, dtls_gen_connection:next_event(hello, no_record, diff --git a/lib/ssl/src/ssl_connection.hrl b/lib/ssl/src/ssl_connection.hrl index b1437bf742f0..a225080aba29 100644 --- a/lib/ssl/src/ssl_connection.hrl +++ b/lib/ssl/src/ssl_connection.hrl @@ -89,8 +89,15 @@ cookie_iv_shard :: {binary(), binary()} %% IV, Shard | 'undefined', client_certificate_status = not_requested :: not_requested | requested | - empty | needs_verifying | - verified, + empty | needs_verifying | verified, + %% Buffer of TLS/DTLS records, used during the TLS + %% handshake to when possible pack more than one TLS + %% record into the underlying packet + %% format. Introduced by DTLS - RFC 4347. The + %% mechanism is also useful in TLS although we do not + %% need to worry about packet loss in TLS. In DTLS we + %% need to track DTLS handshake seqnr + flight_buffer = [] :: list() | map(), stapling_state = #{configured => false, status => not_negotiated} }). @@ -118,14 +125,6 @@ %% Handshake %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% handshake_env :: #handshake_env{} | secret_printout(), - %% Buffer of TLS/DTLS records, used during the TLS - %% handshake to when possible pack more than one TLS - %% record into the underlying packet - %% format. Introduced by DTLS - RFC 4347. The - %% mechanism is also useful in TLS although we do not - %% need to worry about packet loss in TLS. In DTLS we - %% need to track DTLS handshake seqnr - flight_buffer = [] :: list() | map(), protocol_specific = #{} :: map(), session :: #session{} | secret_printout(), key_share, diff --git a/lib/ssl/src/ssl_gen_statem.erl b/lib/ssl/src/ssl_gen_statem.erl index 46bf7d6c2287..16db5efe81bd 100644 --- a/lib/ssl/src/ssl_gen_statem.erl +++ b/lib/ssl/src/ssl_gen_statem.erl @@ -1243,8 +1243,7 @@ format_status(terminate, [_, StateName, State]) -> handshake_env = ?SECRET_PRINTOUT, connection_env = ?SECRET_PRINTOUT, session = ?SECRET_PRINTOUT, - ssl_options = NewOptions, - flight_buffer = ?SECRET_PRINTOUT} + ssl_options = NewOptions} }}]}]. %%-------------------------------------------------------------------- %%% Internal functions diff --git a/lib/ssl/src/tls_dtls_gen_connection.erl b/lib/ssl/src/tls_dtls_gen_connection.erl index 31c51d9dd74f..10426cc5912a 100644 --- a/lib/ssl/src/tls_dtls_gen_connection.erl +++ b/lib/ssl/src/tls_dtls_gen_connection.erl @@ -165,7 +165,8 @@ initial_state(Role, Sender, Host, Port, Socket, {SSLOptions, SocketOptions, Trac handshake_env = #handshake_env{ tls_handshake_history = ssl_handshake:init_handshake_history(), renegotiation = {false, first}, - allow_renegotiate = maps:get(client_renegotiation, SSLOptions, undefined) + allow_renegotiate = maps:get(client_renegotiation, SSLOptions, undefined), + flight_buffer = [] }, connection_env = #connection_env{user_application = {UserMonitor, User}}, socket_options = SocketOptions, @@ -175,7 +176,6 @@ initial_state(Role, Sender, Host, Port, Socket, {SSLOptions, SocketOptions, Trac protocol_buffers = #protocol_buffers{}, user_data_buffer = {[],0,[]}, start_or_recv_from = undefined, - flight_buffer = [], protocol_specific = #{sender => Sender, active_n => ssl_config:get_internal_active_n( maps:get(erl_dist, SSLOptions, false)), diff --git a/lib/ssl/src/tls_gen_connection.erl b/lib/ssl/src/tls_gen_connection.erl index 10908081cb70..2e0791fb1ef4 100644 --- a/lib/ssl/src/tls_gen_connection.erl +++ b/lib/ssl/src/tls_gen_connection.erl @@ -186,9 +186,9 @@ send_handshake(Handshake, State) -> send_handshake_flight(queue_handshake(Handshake, State)). queue_handshake(Handshake, #state{handshake_env = - #handshake_env{tls_handshake_history = Hist0} = HsEnv, + #handshake_env{tls_handshake_history = Hist0, + flight_buffer = Flight0} = HsEnv0, connection_env = #connection_env{negotiated_version = Version}, - flight_buffer = Flight0, ssl_options = #{log_level := LogLevel}, connection_states = ConnectionStates0} = State0) -> {BinHandshake, ConnectionStates, Hist} = @@ -196,9 +196,9 @@ queue_handshake(Handshake, #state{handshake_env = ssl_logger:debug(LogLevel, outbound, 'handshake', Handshake), ssl_logger:debug(LogLevel, outbound, 'record', BinHandshake), - State0#state{connection_states = ConnectionStates, - handshake_env = HsEnv#handshake_env{tls_handshake_history = Hist}, - flight_buffer = Flight0 ++ [BinHandshake]}. + HsEnv = HsEnv0#handshake_env{tls_handshake_history = Hist, + flight_buffer = [Flight0, BinHandshake]}, + State0#state{connection_states = ConnectionStates, handshake_env = HsEnv}. -spec send_handshake_flight(StateIn) -> {StateOut, FlightBuffer} when StateIn :: #state{}, @@ -206,20 +206,21 @@ queue_handshake(Handshake, #state{handshake_env = FlightBuffer :: list(). send_handshake_flight(#state{static_env = #static_env{socket = Socket, transport_cb = Transport}, - flight_buffer = Flight} = State0) -> + handshake_env = #handshake_env{flight_buffer = Flight} = HsEnv} + = State0) -> tls_socket:send(Transport, Socket, Flight), - {State0#state{flight_buffer = []}, []}. + {State0#state{handshake_env = HsEnv#handshake_env{flight_buffer = []}}, []}. queue_change_cipher(Msg, #state{connection_env = #connection_env{negotiated_version = Version}, - flight_buffer = Flight0, + handshake_env = #handshake_env{flight_buffer = Flight0} = HsEnv0, ssl_options = #{log_level := LogLevel}, connection_states = ConnectionStates0} = State0) -> {BinChangeCipher, ConnectionStates} = encode_change_cipher(Msg, Version, ConnectionStates0), ssl_logger:debug(LogLevel, outbound, 'record', BinChangeCipher), - State0#state{connection_states = ConnectionStates, - flight_buffer = Flight0 ++ [BinChangeCipher]}. + HsEnv = HsEnv0#handshake_env{flight_buffer = [Flight0, BinChangeCipher]}, + State0#state{connection_states = ConnectionStates, handshake_env = HsEnv}. reinit(#state{protocol_specific = #{sender := Sender}, connection_env = #connection_env{negotiated_version = Version}, diff --git a/lib/ssl/src/tls_gen_connection_1_3.erl b/lib/ssl/src/tls_gen_connection_1_3.erl index 7af57c25c236..a3fe527845e4 100644 --- a/lib/ssl/src/tls_gen_connection_1_3.erl +++ b/lib/ssl/src/tls_gen_connection_1_3.erl @@ -82,6 +82,7 @@ initial_state(Role, Sender, Host, Port, Socket, handshake_env = #handshake_env{ tls_handshake_history = ssl_handshake:init_handshake_history(), + flight_buffer = [], renegotiation = {false, first} }, connection_env = #connection_env{user_application = {UserMonitor, User}}, @@ -94,7 +95,6 @@ initial_state(Role, Sender, Host, Port, Socket, protocol_buffers = #protocol_buffers{}, user_data_buffer = {[],0,[]}, start_or_recv_from = undefined, - flight_buffer = [], protocol_specific = #{sender => Sender, active_n => internal_active_n(SSLOptions, Socket), active_n_toggle => true @@ -204,16 +204,18 @@ downgrade(Type, Event, State) -> %% Description: Enqueues a change_cipher_spec record as the first/last %% message of the current flight buffer -maybe_queue_change_cipher_spec(#state{flight_buffer = FlightBuffer0} = State0, +maybe_queue_change_cipher_spec(#state{handshake_env = #handshake_env{flight_buffer = FlightBuffer0} + } = State0, first) -> - {State, FlightBuffer} = + {State = #state{handshake_env = HsEnv}, FlightBuffer} = maybe_prepend_change_cipher_spec(State0, FlightBuffer0), - State#state{flight_buffer = FlightBuffer}; -maybe_queue_change_cipher_spec(#state{flight_buffer = FlightBuffer0} = State0, + State#state{handshake_env = HsEnv#handshake_env{flight_buffer = FlightBuffer}}; +maybe_queue_change_cipher_spec(#state{handshake_env = #handshake_env{flight_buffer = FlightBuffer0} + } = State0, last) -> - {State, FlightBuffer} = maybe_append_change_cipher_spec(State0, - FlightBuffer0), - State#state{flight_buffer = FlightBuffer}. + {State = #state{handshake_env = HsEnv}, FlightBuffer} = + maybe_append_change_cipher_spec(State0, FlightBuffer0), + State#state{handshake_env = HsEnv#handshake_env{flight_buffer = FlightBuffer}}. handle_change_cipher_spec(Type, Msg, StateName, #state{protocol_specific = PS0} = State) -> diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl index fd02efbeabb6..f37a5aef1681 100644 --- a/lib/ssl/src/tls_handshake_1_3.erl +++ b/lib/ssl/src/tls_handshake_1_3.erl @@ -578,14 +578,14 @@ encode_handshake(HandshakeMsg) -> encode_early_data(Cipher, #state{ - flight_buffer = Flight0, + handshake_env = #handshake_env{ + flight_buffer = Flight0} = HsEnv, protocol_specific = #{sender := _Sender}, + connection_states = ConnectionStates0, ssl_options = #{versions := [Version|_], early_data := EarlyData} = _SslOpts0 } = State0) -> - #state{connection_states = - #{current_write := - #{security_parameters := SecurityParameters0} = Write0} = ConnectionStates0} = State0, + #{current_write := #{security_parameters := SecurityParameters0} = Write0} = ConnectionStates0, BulkCipherAlgo = ssl_cipher:bulk_cipher_algorithm(Cipher), SecurityParameters = SecurityParameters0#security_parameters{ cipher_type = ?AEAD, @@ -594,7 +594,7 @@ encode_early_data(Cipher, ConnectionStates1 = ConnectionStates0#{current_write => Write}, {BinEarlyData, ConnectionStates} = tls_record:encode_data([EarlyData], Version, ConnectionStates1), State0#state{connection_states = ConnectionStates, - flight_buffer = Flight0 ++ [BinEarlyData]}. + handshake_env = HsEnv#handshake_env{flight_buffer = Flight0 ++ [BinEarlyData]}}. %%==================================================================== From ba26240448bd389e909349cdc4892698c2d50353 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Wed, 24 Jan 2024 14:37:32 +0100 Subject: [PATCH 16/20] Move start_recv_from and bytes_to_read to own record Reduces state record and keeps related info together. --- lib/ssl/src/dtls_client_connection.erl | 6 +- lib/ssl/src/dtls_gen_connection.erl | 2 +- lib/ssl/src/dtls_server_connection.erl | 11 ++- lib/ssl/src/ssl_connection.hrl | 11 ++- lib/ssl/src/ssl_gen_statem.erl | 108 ++++++++++++---------- lib/ssl/src/tls_client_connection.erl | 14 +-- lib/ssl/src/tls_client_connection_1_3.erl | 11 ++- lib/ssl/src/tls_dtls_gen_connection.erl | 4 +- lib/ssl/src/tls_gen_connection.erl | 63 +++++-------- lib/ssl/src/tls_gen_connection_1_3.erl | 2 +- lib/ssl/src/tls_server_connection.erl | 9 +- lib/ssl/src/tls_server_connection_1_3.erl | 21 ++--- 12 files changed, 131 insertions(+), 131 deletions(-) diff --git a/lib/ssl/src/dtls_client_connection.erl b/lib/ssl/src/dtls_client_connection.erl index 087584d286ac..36b26adb44c8 100644 --- a/lib/ssl/src/dtls_client_connection.erl +++ b/lib/ssl/src/dtls_client_connection.erl @@ -217,7 +217,7 @@ initial_hello({call, From}, {start, Timeout}, State = State2#state{connection_env = CEnv#connection_env{negotiated_version = Version}, %% RequestedVersion session = Session, - start_or_recv_from = From, + recv = State2#state.recv#recv{from = From}, protocol_specific = PS#{active_n_toggle := false} }, dtls_gen_connection:next_event(hello, no_record, State, @@ -288,8 +288,8 @@ hello(internal, #hello_verify_request{cookie = Cookie}, dtls_gen_connection:next_event(?STATE(hello), no_record, State, Actions); hello(internal, #server_hello{extensions = Extensions}, #state{handshake_env = #handshake_env{continue_status = pause}, - start_or_recv_from = From} = State) -> - {next_state, user_hello, State#state{start_or_recv_from = undefined}, + recv = #recv{from = From} = Recv} = State) -> + {next_state, user_hello, State#state{recv = Recv#recv{from = undefined}}, [{postpone, true},{reply, From, {ok, Extensions}}]}; hello(internal, #server_hello{} = Hello, #state{handshake_env = #handshake_env{ diff --git a/lib/ssl/src/dtls_gen_connection.erl b/lib/ssl/src/dtls_gen_connection.erl index 67573f4a6f7d..9ea37e8230db 100644 --- a/lib/ssl/src/dtls_gen_connection.erl +++ b/lib/ssl/src/dtls_gen_connection.erl @@ -129,7 +129,7 @@ initial_state(Role, Host, Port, Socket, connection_states = ConnectionStates, protocol_buffers = #protocol_buffers{}, user_data_buffer = {[],0,[]}, - start_or_recv_from = undefined, + recv = #recv{}, protocol_specific = #{active_n => InternalActiveN, active_n_toggle => true, flight_state => initial_flight_state(DataTag), diff --git a/lib/ssl/src/dtls_server_connection.erl b/lib/ssl/src/dtls_server_connection.erl index 3ba104802a90..cf9cdb440f27 100644 --- a/lib/ssl/src/dtls_server_connection.erl +++ b/lib/ssl/src/dtls_server_connection.erl @@ -179,12 +179,13 @@ init([Role, Host, Port, Socket, Options, User, CbInfo]) -> %%-------------------------------------------------------------------- initial_hello(enter, _, State) -> {keep_state, State}; -initial_hello({call, From}, {start, Timeout}, #state{protocol_specific = PS0} = State) -> +initial_hello({call, From}, {start, Timeout}, + #state{protocol_specific = PS0, recv = Recv} = State) -> PS = PS0#{current_cookie_secret => dtls_v1:cookie_secret(), previous_cookie_secret => <<>>}, erlang:send_after(dtls_v1:cookie_timeout(), self(), new_cookie_secret), dtls_gen_connection:next_event(hello, no_record, - State#state{start_or_recv_from = From, + State#state{recv = Recv#recv{from = From}, protocol_specific = PS}, [{{timeout, handshake}, Timeout, close}]); initial_hello({call, From}, {start, {Opts, EmOpts}, Timeout}, @@ -254,10 +255,10 @@ hello(internal, #client_hello{cookie = <<>>, end; hello(internal, #client_hello{extensions = Extensions} = Hello, #state{handshake_env = #handshake_env{continue_status = pause}, - start_or_recv_from = From} = State0) -> + recv = #recv{from = From}} = State0) -> try tls_dtls_server_connection:handle_sni_extension(State0, Hello) of - #state{} = State -> - {next_state, user_hello, State#state{start_or_recv_from = undefined}, + #state{recv = Recv} = State -> + {next_state, user_hello, State#state{recv = Recv#recv{from = undefined}}, [{postpone, true}, {reply, From, {ok, Extensions}}]} catch throw:#alert{} = Alert -> dtls_gen_connection:alert_or_reset_connection(Alert, hello, State0) diff --git a/lib/ssl/src/ssl_connection.hrl b/lib/ssl/src/ssl_connection.hrl index a225080aba29..b9aae40688be 100644 --- a/lib/ssl/src/ssl_connection.hrl +++ b/lib/ssl/src/ssl_connection.hrl @@ -117,6 +117,11 @@ } | secret_printout() | 'undefined' }). +-record(recv, { + from :: term(), %% start or recv from + bytes_to_read :: undefined | integer() %% bytes to read in passive mode + }). + -record(state, { static_env :: #static_env{}, connection_env :: #connection_env{} | secret_printout(), @@ -129,12 +134,10 @@ session :: #session{} | secret_printout(), key_share, %% Data shuffling %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% + recv = #recv{} :: #recv{}, connection_states :: ssl_record:connection_states() | secret_printout(), protocol_buffers :: term() | secret_printout() , %% #protocol_buffers{} from tls_record.hrl or dtls_recor.hr - user_data_buffer :: undefined | {[binary()],non_neg_integer(),[binary()]} | secret_printout(), - bytes_to_read :: undefined | integer(), %% bytes to read in passive mode - %% recv and start handling - start_or_recv_from :: term() + user_data_buffer :: undefined | {[binary()],non_neg_integer(),[binary()]} | secret_printout() }). -define(DEFAULT_DIFFIE_HELLMAN_PARAMS, diff --git a/lib/ssl/src/ssl_gen_statem.erl b/lib/ssl/src/ssl_gen_statem.erl index 16db5efe81bd..9c157d6bc77c 100644 --- a/lib/ssl/src/ssl_gen_statem.erl +++ b/lib/ssl/src/ssl_gen_statem.erl @@ -386,7 +386,7 @@ socket_control(dtls_gen_connection, {PeerAddrPort, Socket}, end. prepare_connection(#state{handshake_env = #handshake_env{renegotiation = Renegotiate}, - start_or_recv_from = RecvFrom} = State0, Connection) + recv = #recv{from = RecvFrom}} = State0, Connection) when Renegotiate =/= {false, first}, RecvFrom =/= undefined -> State = Connection:reinit(State0), @@ -558,10 +558,12 @@ config_error(_Type, _Event, _State) -> %%-------------------------------------------------------------------- connection({call, RecvFrom}, {recv, N, Timeout}, #state{static_env = #static_env{protocol_cb = Connection}, - socket_options = - #socket_options{active = false}} = State0) -> - passive_receive(State0#state{bytes_to_read = N, - start_or_recv_from = RecvFrom}, connection, Connection, + recv = Recv, + socket_options = #socket_options{active = false} + } = State0) -> + passive_receive(State0#state{recv = Recv#recv{from = RecvFrom, + bytes_to_read = N}}, + ?STATE(connection), Connection, [{{timeout, recv}, Timeout, timeout}]); connection({call, From}, peer_certificate, #state{session = #session{peer_certificate = Cert}} = State) -> @@ -664,19 +666,22 @@ connection(cast, {dist_handshake_complete, DHandle}, #state{ssl_options = #{erl_dist := true}, static_env = #static_env{protocol_cb = Connection}, connection_env = CEnv, + recv = Recv, socket_options = SockOpts} = State0) -> process_flag(priority, normal), State1 = State0#state{ socket_options = SockOpts#socket_options{active = true}, connection_env = CEnv#connection_env{erl_dist_handle = DHandle}, - bytes_to_read = undefined}, + recv = Recv#recv{bytes_to_read = undefined} + }, {Record, State} = read_application_data(<<>>, State1), Connection:next_event(?STATE(connection), Record, State); connection(info, Msg, #state{static_env = #static_env{protocol_cb = Connection}} = State) -> Connection:handle_info(Msg, ?STATE(connection), State); -connection(internal, {recv, RecvFrom}, #state{start_or_recv_from = RecvFrom, - static_env = #static_env{protocol_cb = Connection}} = State) -> +connection(internal, {recv, RecvFrom}, + #state{recv = #recv{from=RecvFrom}, + static_env = #static_env{protocol_cb = Connection}} = State) -> passive_receive(State, ?STATE(connection), Connection, []); connection(Type, Msg, State) -> handle_common_event(Type, Msg, ?STATE(connection), State). @@ -734,15 +739,19 @@ handle_common_event(internal, {protocol_record, TLSorDTLSRecord}, StateName, Connection:handle_protocol_record(TLSorDTLSRecord, StateName, State); handle_common_event(timeout, hibernate, _, _) -> {keep_state_and_data, [hibernate]}; -handle_common_event({timeout, handshake}, close, _StateName, #state{start_or_recv_from = StartFrom} = State) -> +handle_common_event({timeout, handshake}, close, _StateName, + #state{recv = #recv{from = StartFrom} = Recv} = State) -> {stop_and_reply, {shutdown, user_timeout}, - {reply, StartFrom, {error, timeout}}, State#state{start_or_recv_from = undefined}}; -handle_common_event({timeout, recv}, timeout, StateName, #state{start_or_recv_from = RecvFrom} = State) -> - {next_state, StateName, State#state{start_or_recv_from = undefined, - bytes_to_read = undefined}, [{reply, RecvFrom, {error, timeout}}]}; -handle_common_event(internal, {recv, RecvFrom}, StateName, #state{start_or_recv_from = RecvFrom}) when - StateName =/= connection -> + {reply, StartFrom, {error, timeout}}, State#state{recv = Recv#recv{from = undefined}}}; +handle_common_event({timeout, recv}, timeout, StateName, + #state{recv = #recv{from = RecvFrom} = Recv} = State) -> + {next_state, StateName, + State#state{recv = Recv#recv{from = undefined, bytes_to_read = undefined}}, + [{reply, RecvFrom, {error, timeout}}]}; +handle_common_event(internal, {recv, RecvFrom}, StateName, + #state{recv = #recv{from = RecvFrom}}) + when StateName =/= connection -> {keep_state_and_data, [postpone]}; handle_common_event(internal, new_connection, StateName, State) -> {next_state, StateName, State}; @@ -796,10 +805,10 @@ handle_call({recv, _N, _Timeout}, From, _, #state{socket_options = #socket_options{active = Active}}) when Active =/= false -> {keep_state_and_data, [{reply, From, {error, einval}}]}; -handle_call({recv, N, Timeout}, RecvFrom, StateName, State) -> +handle_call({recv, N, Timeout}, RecvFrom, StateName, #state{recv = Recv} = State) -> %% Doing renegotiate wait with handling request until renegotiate is %% finished. - {next_state, StateName, State#state{bytes_to_read = N, start_or_recv_from = RecvFrom}, + {next_state, StateName, State#state{recv = Recv#recv{from=RecvFrom, bytes_to_read = N}}, [{next_event, internal, {recv, RecvFrom}} , {{timeout, recv}, Timeout, timeout}]}; handle_call({new_user, User}, From, StateName, State = #state{connection_env = #connection_env{user_application = {OldMon, _}} = CEnv}) -> @@ -854,9 +863,8 @@ handle_info({ErrorTag, Socket, econnaborted}, StateName, handshake_env = #handshake_env{renegotiation = Type}, connection_env = #connection_env{negotiated_version = Version}, session = Session, - start_or_recv_from = StartFrom + recv = #recv{from = StartFrom} } = State) when StateName =/= connection -> - maybe_invalidate_session(Version, Type, Role, Host, Port, Session), Pids = Connection:pids(State), alert_user(Pids, Transport, Trackers,Socket, @@ -952,15 +960,11 @@ passive_receive(#state{user_data_buffer = {Front,BufferSize,Rear}, case read_application_data(State0, Front, BufferSize, Rear) of {stop, _, _} = ShutdownError -> ShutdownError; + {Record, #state{recv = #recv{from = undefined}} = State} -> + Connection:next_event(StateName, Record, State, + [{{timeout, recv}, infinity, timeout}]); {Record, State} -> - case State#state.start_or_recv_from of - undefined -> - %% Cancel recv timeout as data has been delivered - Connection:next_event(StateName, Record, State, - [{{timeout, recv}, infinity, timeout}]); - _ -> - Connection:next_event(StateName, Record, State, StartTimerAction) - end + Connection:next_event(StateName, Record, State, StartTimerAction) end end. @@ -1010,7 +1014,8 @@ handle_normal_shutdown(Alert, StateName, #state{static_env = #static_env{role = protocol_cb = Connection, trackers = Trackers}, handshake_env = #handshake_env{renegotiation = {false, first}}, - start_or_recv_from = StartFrom} = State) -> + recv = #recv{from = StartFrom} + } = State) -> Pids = Connection:pids(State), alert_user(Pids, Transport, Trackers, Socket, StartFrom, Alert, Role, StateName, Connection); @@ -1022,7 +1027,8 @@ handle_normal_shutdown(Alert, StateName, #state{static_env = #static_env{role = connection_env = #connection_env{user_application = {_Mon, Pid}}, handshake_env = #handshake_env{renegotiation = Type}, socket_options = Opts, - start_or_recv_from = RecvFrom} = State) -> + recv = #recv{from = RecvFrom} + } = State) -> Pids = Connection:pids(State), alert_user(Pids, Transport, Trackers, Socket, Type, Opts, Pid, RecvFrom, Alert, Role, StateName, Connection). @@ -1131,7 +1137,7 @@ handle_fatal_alert(Alert0, StateName, protocol_cb = Connection}, connection_env = #connection_env{user_application = {_Mon, Pid}}, ssl_options = #{log_level := LogLevel}, - start_or_recv_from = From, + recv = #recv{from = From}, session = Session, socket_options = Opts} = State) -> invalidate_session(Role, Host, Port, Session), @@ -1357,10 +1363,11 @@ ack_connection(#state{handshake_env = #handshake_env{renegotiation = {true, From gen_statem:reply(From, ok), State#state{handshake_env = HsEnv#handshake_env{renegotiation = undefined}}; ack_connection(#state{handshake_env = #handshake_env{renegotiation = {false, first}} = HsEnv, - start_or_recv_from = StartFrom} = State) when StartFrom =/= undefined -> + recv = #recv{from = StartFrom} = Recv} = State) + when StartFrom =/= undefined -> gen_statem:reply(StartFrom, connected), State#state{handshake_env = HsEnv#handshake_env{renegotiation = undefined}, - start_or_recv_from = undefined}; + recv = Recv#recv{from = undefined}}; ack_connection(State) -> State. @@ -1417,10 +1424,10 @@ handle_active_option(_, StateName0, To, Reply, end end. -read_application_data(#state{ - socket_options = SocketOpts, - bytes_to_read = BytesToRead, - start_or_recv_from = RecvFrom} = State, Front, BufferSize, Rear) -> +read_application_data(#state{socket_options = SocketOpts, + recv = #recv{from = RecvFrom, + bytes_to_read = BytesToRead}} = State, + Front, BufferSize, Rear) -> read_application_data(State, Front, BufferSize, Rear, SocketOpts, RecvFrom, BytesToRead). %% Pick binary from queue front, if empty wait for more data @@ -1429,8 +1436,9 @@ read_application_data(State, [Bin|Front], BufferSize, Rear, SocketOpts, RecvFrom read_application_data(State, [] = Front, BufferSize, [] = Rear, SocketOpts, RecvFrom, BytesToRead) -> 0 = BufferSize, % Assert {no_record, State#state{socket_options = SocketOpts, - bytes_to_read = BytesToRead, - start_or_recv_from = RecvFrom, + recv = State#state.recv#recv{from = RecvFrom, + bytes_to_read = BytesToRead + }, user_data_buffer = {Front,BufferSize,Rear}}}; read_application_data(State, [], BufferSize, Rear, SocketOpts, RecvFrom, BytesToRead) -> [Bin|Front] = lists:reverse(Rear), @@ -1457,8 +1465,8 @@ read_application_data_bin(State, Front0, BufferSize0, Rear0, SocketOpts0, RecvFr true -> %% All data is in the first binary, no use to retry - wait for more {no_record, State#state{socket_options = SocketOpts0, - bytes_to_read = BytesToRead, - start_or_recv_from = RecvFrom, + recv = State#state.recv#recv{from = RecvFrom, + bytes_to_read = BytesToRead}, user_data_buffer = {[Bin0|Front0],BufferSize0,Rear0}}} end; {more, Size} when Size =< BufferSize0 -> @@ -1470,13 +1478,13 @@ read_application_data_bin(State, Front0, BufferSize0, Rear0, SocketOpts0, RecvFr {more, _Size} -> %% We do not have a packet in the buffer - wait for more {no_record, State#state{socket_options = SocketOpts0, - bytes_to_read = BytesToRead, - start_or_recv_from = RecvFrom, + recv = State#state.recv#recv{from = RecvFrom, + bytes_to_read = BytesToRead}, user_data_buffer = {[Bin0|Front0],BufferSize0,Rear0}}}; passive -> {no_record, State#state{socket_options = SocketOpts0, - bytes_to_read = BytesToRead, - start_or_recv_from = RecvFrom, + recv = State#state.recv#recv{from = RecvFrom, + bytes_to_read = BytesToRead}, user_data_buffer = {[Bin0|Front0],BufferSize0,Rear0}}}; {error,_Reason} -> %% Invalid packet in packet mode @@ -1493,10 +1501,11 @@ read_application_data_bin(State, Front0, BufferSize0, Rear0, SocketOpts0, RecvFr deliver_packet_error( Connection:pids(State), Transport, Socket, SocketOpts0, Buffer, Pid, RecvFrom, Trackers, Connection), - {stop, {shutdown, normal}, State#state{socket_options = SocketOpts0, - bytes_to_read = BytesToRead, - start_or_recv_from = RecvFrom, - user_data_buffer = {[Buffer],BufferSize0,[]}}} + {stop, {shutdown, normal}, + State#state{socket_options = SocketOpts0, + recv = State#state.recv#recv{from = RecvFrom, + bytes_to_read = BytesToRead}, + user_data_buffer = {[Buffer],BufferSize0,[]}}} end. read_application_data_deliver(State, Front, BufferSize, Rear, SocketOpts0, RecvFrom, Data) -> @@ -1518,8 +1527,7 @@ read_application_data_deliver(State, Front, BufferSize, Rear, SocketOpts0, RecvF {no_record, State#state{ user_data_buffer = {Front,BufferSize,Rear}, - start_or_recv_from = undefined, - bytes_to_read = undefined, + recv = State#state.recv#recv{from = undefined, bytes_to_read = undefined}, socket_options = SocketOpts }}; true -> %% Try to deliver more data diff --git a/lib/ssl/src/tls_client_connection.erl b/lib/ssl/src/tls_client_connection.erl index e713799501cc..239429d7d074 100644 --- a/lib/ssl/src/tls_client_connection.erl +++ b/lib/ssl/src/tls_client_connection.erl @@ -183,7 +183,8 @@ initial_hello({call, From}, {start, Timeout}, session_tickets := SessionTickets, early_data := EarlyData} = SslOpts, session = Session, - connection_states = ConnectionStates0 + connection_states = ConnectionStates0, + recv = Recv0 } = State0) -> KeyShare = tls_client_connection_1_3:maybe_generate_client_shares(SslOpts), @@ -244,14 +245,15 @@ initial_hello({call, From}, {start, Timeout}, stapling_state = StaplingState0#{ocsp_nonce => OcspNonce, configured => StaplingKeyPresent}}, - start_or_recv_from = From, + recv = State5#state.recv#recv{from = From} key_share = KeyShare}, NextState = next_statem_state(Versions), Connection:next_event(NextState, no_record, State, [{{timeout, handshake}, Timeout, close}]) catch {Ref, #alert{} = Alert} -> - ssl_gen_statem:handle_own_alert(Alert, init, State0#state{start_or_recv_from = From}) + NewState = State0#state{recv = Recv0#recv{from = From}}, + ssl_gen_statem:handle_own_alert(Alert, init, NewState) end; initial_hello({call, From}, {start, {Opts, EmOpts}, Timeout}, #state{static_env = #static_env{role = Role}, @@ -286,10 +288,10 @@ config_error(Type, Event, State) -> %%-------------------------------------------------------------------- hello(internal, #server_hello{extensions = Extensions}, #state{handshake_env = #handshake_env{continue_status = pause}, - start_or_recv_from = From} = State) -> + recv = #recv{from = From}=Recv} = State) -> {next_state, user_hello, - State#state{start_or_recv_from = undefined}, [{postpone, true}, - {reply, From, {ok, Extensions}}]}; + State#state{recv = Recv#recv{from = undefined}}, + [{postpone, true},{reply, From, {ok, Extensions}}]}; hello(internal, #server_hello{} = Hello, #state{connection_states = ConnectionStates0, connection_env = CEnv, diff --git a/lib/ssl/src/tls_client_connection_1_3.erl b/lib/ssl/src/tls_client_connection_1_3.erl index ef3b14cc7672..218308fccbe8 100644 --- a/lib/ssl/src/tls_client_connection_1_3.erl +++ b/lib/ssl/src/tls_client_connection_1_3.erl @@ -178,7 +178,7 @@ user_hello({call, From}, {handshake_continue, NewOptions, Timeout}, try ssl:update_options(NewOptions, ?CLIENT_ROLE, Options0) of Options -> State = ssl_gen_statem:ssl_config(Options, ?CLIENT_ROLE, State0), - {next_state, wait_sh, State#state{start_or_recv_from = From, + {next_state, wait_sh, State#state{recv = State#state.recv#recv{from = From}, handshake_env = HSEnv#handshake_env{continue_status = continue} @@ -225,13 +225,13 @@ start(internal, #server_hello{extensions = selected_version = Version}} = Extensions}, #state{ssl_options = #{versions := SupportedVersions}, - start_or_recv_from = From, + recv = #recv{from = From} = Recv, handshake_env = #handshake_env{continue_status = pause}} = State) -> case tls_record:is_acceptable_version(Version, SupportedVersions) of true -> {next_state, user_hello, - State#state{start_or_recv_from = undefined}, + State#state{recv=Recv#recv{from = undefined}}, [{postpone, true}, {reply, From, {ok, Extensions}}]}; false -> @@ -265,9 +265,10 @@ wait_sh(internal = Type, #change_cipher_spec{} = Msg, State)-> ?STATE(wait_sh), State); wait_sh(internal, #server_hello{extensions = Extensions}, #state{handshake_env = #handshake_env{continue_status = pause}, - start_or_recv_from = From} = State) -> + recv = #recv{from = From} = Recv + } = State) -> {next_state, user_hello, - State#state{start_or_recv_from = undefined}, + State#state{recv = Recv#recv{from = undefined}}, [{postpone, true},{reply, From, {ok, Extensions}}]}; wait_sh(internal, #server_hello{session_id = ?EMPTY_ID} = Hello, #state{session = #session{session_id = ?EMPTY_ID}, diff --git a/lib/ssl/src/tls_dtls_gen_connection.erl b/lib/ssl/src/tls_dtls_gen_connection.erl index 10426cc5912a..e6a22c379a4e 100644 --- a/lib/ssl/src/tls_dtls_gen_connection.erl +++ b/lib/ssl/src/tls_dtls_gen_connection.erl @@ -175,7 +175,7 @@ initial_state(Role, Sender, Host, Port, Socket, {SSLOptions, SocketOptions, Trac connection_states = ConnectionStates, protocol_buffers = #protocol_buffers{}, user_data_buffer = {[],0,[]}, - start_or_recv_from = undefined, + recv = #recv{}, protocol_specific = #{sender => Sender, active_n => ssl_config:get_internal_active_n( maps:get(erl_dist, SSLOptions, false)), @@ -261,7 +261,7 @@ user_hello({call, From}, {handshake_continue, NewOptions, Timeout}, Options -> State = ssl_gen_statem:ssl_config(Options, Role, State0), {next_state, hello, - State#state{start_or_recv_from = From, + State#state{recv = State#state.recv#recv{from = From}, handshake_env = HSEnv#handshake_env{continue_status = continue} }, diff --git a/lib/ssl/src/tls_gen_connection.erl b/lib/ssl/src/tls_gen_connection.erl index 2e0791fb1ef4..04e2afeba165 100644 --- a/lib/ssl/src/tls_gen_connection.erl +++ b/lib/ssl/src/tls_gen_connection.erl @@ -294,7 +294,7 @@ handle_info({Protocol, _, Data}, StateName, end; handle_info({PassiveTag, Socket}, StateName, #state{static_env = #static_env{socket = Socket, passive_tag = PassiveTag} = StatEnv, - start_or_recv_from = From, + recv = #recv{from = From}, protocol_buffers = #protocol_buffers{tls_cipher_texts = CTs}, protocol_specific = PS } = State0) -> @@ -325,10 +325,9 @@ handle_info({CloseTag, Socket}, StateName, role = Role, socket = Socket, close_tag = CloseTag}, - start_or_recv_from = From, + recv = #recv{from = From}, socket_options = #socket_options{active = Active}, protocol_specific = PS} = State) -> - %% Note that as of TLS 1.1, %% failure to properly close a connection no longer requires that a %% session not be resumed. This is a change from TLS 1.0 to conform @@ -426,20 +425,17 @@ handle_protocol_record(#ssl_tls{type = ?APPLICATION_DATA}, StateName, application_data_before_handshake_or_intervened_in_post_handshake_auth), ssl_gen_statem:handle_own_alert(Alert, StateName, State); handle_protocol_record(#ssl_tls{type = ?APPLICATION_DATA, fragment = Data}, StateName, - #state{start_or_recv_from = From, - socket_options = #socket_options{active = false}} - = State0) when From =/= undefined -> + #state{recv = #recv{from = From}, + socket_options = #socket_options{active = false}} = State0) + when From =/= undefined -> case ssl_gen_statem:read_application_data(Data, State0) of {stop, _, _} = Stop-> Stop; - {Record, #state{start_or_recv_from = Caller} = State} -> - TimerAction = case Caller of - undefined -> %% Passive recv complete cancel timer - [{{timeout, recv}, infinity, timeout}]; - _ -> - [] - end, - next_event(StateName, Record, State, TimerAction) + {Record, #state{recv = #recv{from = undefined}} = State} -> + TimerAction = [{{timeout, recv}, infinity, timeout}], + next_event(StateName, Record, State, TimerAction); + {Record, State} -> + next_event(StateName, Record, State, []) end; handle_protocol_record(#ssl_tls{type = ?APPLICATION_DATA, fragment = Data}, StateName, State0) -> case ssl_gen_statem:read_application_data(Data, State0) of @@ -701,7 +697,7 @@ flow_ctrl(#state{ssl_options = #{ktls := true}} = State, PBuffers) -> %%% bytes_to_read = undefined means no recv call is ongoing flow_ctrl(#state{user_data_buffer = {_,Size,_}, socket_options = #socket_options{active = false}, - bytes_to_read = undefined} = State, + recv = #recv{bytes_to_read = undefined}} = State, PBuffers) when Size =/= 0 -> %% Passive mode wait for new recv request or socket activation @@ -718,27 +714,18 @@ flow_ctrl(#state{socket_options = #socket_options{active = false, %%%%%%%%% No packet mode set and socket is passive %%%%%%%%%%%% flow_ctrl(#state{user_data_buffer = {_,Size,_}, socket_options = #socket_options{active = false}, - bytes_to_read = 0} = State, - PBuffers) - when Size == 0 -> - %% Passive mode no available bytes, get some - activate_socket(State, PBuffers); -flow_ctrl(#state{user_data_buffer = {_,Size,_}, - socket_options = #socket_options{active = false}, - bytes_to_read = 0} = State, - PBuffers) - when Size =/= 0 -> - %% There is data in the buffer to deliver - {no_record, State#state{protocol_buffers = PBuffers}}; -flow_ctrl(#state{user_data_buffer = {_,Size,_}, - socket_options = #socket_options{active = false}, - bytes_to_read = BytesToRead} = State, - PBuffers) - when (BytesToRead > 0) -> - case (Size >= BytesToRead) of - true -> %% There is enough data bufferd + recv = #recv{bytes_to_read = BytesToRead}} = State, + PBuffers) -> + if BytesToRead =:= 0, Size =:= 0 -> + %% Passive mode no available bytes, get some + activate_socket(State, PBuffers); + BytesToRead =:= 0, Size =/= 0 -> + %% There is data in the buffer to deliver + {no_record, State#state{protocol_buffers = PBuffers}}; + Size >= BytesToRead -> + %% There is enough data bufferd {no_record, State#state{protocol_buffers = PBuffers}}; - false -> %% We need more data to complete the delivery of size + true -> %% We need more data to complete the delivery of size activate_socket(State, PBuffers) end; %%%%%%%%%%% Active mode or more data needed %%%%%%%%%% @@ -924,9 +911,9 @@ handle_alerts([#alert{level = ?WARNING, description = ?CLOSE_NOTIFY} | _Alerts], {next_state, connection = StateName, #state{connection_env = CEnv, socket_options = #socket_options{active = false}, - start_or_recv_from = From} = State}) when From == undefined -> - {next_state, StateName, State#state{connection_env = - CEnv#connection_env{socket_tls_closed = true}}}; + recv = #recv{from = From}} = State}) when From == undefined -> + {next_state, StateName, + State#state{connection_env = CEnv#connection_env{socket_tls_closed = true}}}; handle_alerts([Alert | Alerts], {next_state, StateName, State}) -> handle_alerts(Alerts, ssl_gen_statem:handle_alert(Alert, StateName, State)); handle_alerts([Alert | Alerts], {next_state, StateName, State, _Actions}) -> diff --git a/lib/ssl/src/tls_gen_connection_1_3.erl b/lib/ssl/src/tls_gen_connection_1_3.erl index a3fe527845e4..334d7e162958 100644 --- a/lib/ssl/src/tls_gen_connection_1_3.erl +++ b/lib/ssl/src/tls_gen_connection_1_3.erl @@ -94,7 +94,7 @@ initial_state(Role, Sender, Host, Port, Socket, connection_states = ConnectionStates, protocol_buffers = #protocol_buffers{}, user_data_buffer = {[],0,[]}, - start_or_recv_from = undefined, + recv = #recv{from = undefined}, protocol_specific = #{sender => Sender, active_n => internal_active_n(SSLOptions, Socket), active_n_toggle => true diff --git a/lib/ssl/src/tls_server_connection.erl b/lib/ssl/src/tls_server_connection.erl index c7a8b94b8b49..cfb9608aebc8 100644 --- a/lib/ssl/src/tls_server_connection.erl +++ b/lib/ssl/src/tls_server_connection.erl @@ -159,9 +159,10 @@ init([Role, Sender, Host, Port, Socket, Options, User, CbInfo]) -> initial_hello({call, From}, {start, Timeout}, #state{static_env = #static_env{ protocol_cb = Connection}, - ssl_options = #{versions := Versions}} = State0) -> + ssl_options = #{versions := Versions}, + recv = Recv} = State0) -> NextState = next_statem_state(Versions), - Connection:next_event(NextState, no_record, State0#state{start_or_recv_from = From}, + Connection:next_event(NextState, no_record, State0#state{recv = Recv#recv{from = From}}, [{{timeout, handshake}, Timeout, close}]); initial_hello({call, From}, {start, {Opts, EmOpts}, Timeout}, #state{static_env = #static_env{role = Role}, @@ -195,8 +196,8 @@ config_error(Type, Event, State) -> %%-------------------------------------------------------------------- hello(internal, #client_hello{extensions = Extensions}, #state{handshake_env = #handshake_env{continue_status = pause}, - start_or_recv_from = From} = State) -> - {next_state, user_hello, State#state{start_or_recv_from = undefined}, + recv = #recv{from = From} = Recv} = State) -> + {next_state, user_hello, State#state{recv = Recv#recv{from = undefined}}, [{postpone, true}, {reply, From, {ok, Extensions}}]}; hello(internal, #client_hello{client_version = ClientVersion} = Hello, #state{connection_env = CEnv} = State0) -> diff --git a/lib/ssl/src/tls_server_connection_1_3.erl b/lib/ssl/src/tls_server_connection_1_3.erl index e65f0b464d84..02746b2d738c 100644 --- a/lib/ssl/src/tls_server_connection_1_3.erl +++ b/lib/ssl/src/tls_server_connection_1_3.erl @@ -163,25 +163,22 @@ user_hello({call, From}, cancel, State) -> ssl_gen_statem:handle_own_alert(?ALERT_REC(?FATAL, ?USER_CANCELED, user_canceled), user_hello, State); user_hello({call, From}, {handshake_continue, NewOptions, Timeout}, - #state{handshake_env = - #handshake_env{continue_status = {pause, ClientVersions}} = HSEnv, + #state{handshake_env = #handshake_env{continue_status = {pause, ClientVersions}}, ssl_options = Options0} = State0) -> try ssl:update_options(NewOptions, ?SERVER_ROLE, Options0) of Options = #{versions := Versions} -> - State = ssl_gen_statem:ssl_config(Options, ?SERVER_ROLE, State0), + State1 = ssl_gen_statem:ssl_config(Options, ?SERVER_ROLE, State0), + #state{handshake_env = HsEnv0} = State1, + HsEnv = HsEnv0#handshake_env{continue_status = continue}, + State = State1#state{recv = State1#state.recv#recv{from = From}, handshake_env = HsEnv}, case ssl_handshake:select_supported_version(ClientVersions, Versions) of ?TLS_1_3 -> - {next_state, start, - State#state{start_or_recv_from = From, - handshake_env = HSEnv#handshake_env{continue_status = continue}}, - [{{timeout, handshake}, Timeout, close}]}; + {next_state, start, State, [{{timeout, handshake}, Timeout, close}]}; undefined -> ssl_gen_statem:handle_own_alert(?ALERT_REC(?FATAL, ?PROTOCOL_VERSION), ?STATE(user_hello), State); _Else -> - {next_state, hello, - State#state{start_or_recv_from = From, - handshake_env = HSEnv#handshake_env{continue_status = continue}}, + {next_state, hello, State, [{change_callback_module, tls_server_connection}, {{timeout, handshake}, Timeout, close}]} end @@ -226,10 +223,10 @@ start(internal, #client_hello{extensions = #{client_hello_versions := start(internal, #client_hello{extensions = #{client_hello_versions := #client_hello_versions{versions = ClientVersions} }= Extensions}, - #state{start_or_recv_from = From, + #state{recv = #recv{from = From} = Recv, handshake_env = #handshake_env{continue_status = pause} = HSEnv} = State) -> {next_state, user_hello, - State#state{start_or_recv_from = undefined, + State#state{recv = Recv#recv{from = undefined}, handshake_env = HSEnv#handshake_env{continue_status = {pause, ClientVersions}}}, [{postpone, true}, {reply, From, {ok, Extensions}}]}; start(internal, #client_hello{} = Hello, From 17d139b9a8e8b701503d6fb21e6caf068344f426 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Wed, 24 Jan 2024 14:43:05 +0100 Subject: [PATCH 17/20] Move key_share to handshake_env --- lib/ssl/src/ssl_connection.hrl | 2 +- lib/ssl/src/tls_client_connection.erl | 3 ++- lib/ssl/src/tls_client_connection_1_3.erl | 20 +++++++++----------- lib/ssl/src/tls_handshake_1_3.erl | 4 ++-- lib/ssl/src/tls_server_connection_1_3.erl | 5 +++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/ssl/src/ssl_connection.hrl b/lib/ssl/src/ssl_connection.hrl index b9aae40688be..b7107e0ecc26 100644 --- a/lib/ssl/src/ssl_connection.hrl +++ b/lib/ssl/src/ssl_connection.hrl @@ -90,6 +90,7 @@ | 'undefined', client_certificate_status = not_requested :: not_requested | requested | empty | needs_verifying | verified, + key_share, %% Buffer of TLS/DTLS records, used during the TLS %% handshake to when possible pack more than one TLS %% record into the underlying packet @@ -132,7 +133,6 @@ handshake_env :: #handshake_env{} | secret_printout(), protocol_specific = #{} :: map(), session :: #session{} | secret_printout(), - key_share, %% Data shuffling %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% recv = #recv{} :: #recv{}, connection_states :: ssl_record:connection_states() | secret_printout(), diff --git a/lib/ssl/src/tls_client_connection.erl b/lib/ssl/src/tls_client_connection.erl index 239429d7d074..c773237df6f4 100644 --- a/lib/ssl/src/tls_client_connection.erl +++ b/lib/ssl/src/tls_client_connection.erl @@ -242,11 +242,12 @@ initial_hello({call, From}, {start, Timeout}, session = Session, handshake_env = HsEnv1#handshake_env{ + key_share = KeyShare, stapling_state = StaplingState0#{ocsp_nonce => OcspNonce, configured => StaplingKeyPresent}}, recv = State5#state.recv#recv{from = From} - key_share = KeyShare}, + }, NextState = next_statem_state(Versions), Connection:next_event(NextState, no_record, State, [{{timeout, handshake}, Timeout, close}]) diff --git a/lib/ssl/src/tls_client_connection_1_3.erl b/lib/ssl/src/tls_client_connection_1_3.erl index 218308fccbe8..98babd58ce5c 100644 --- a/lib/ssl/src/tls_client_connection_1_3.erl +++ b/lib/ssl/src/tls_client_connection_1_3.erl @@ -712,11 +712,10 @@ do_handle_exlusive_1_3_hello_or_hello_retry_request( State = State3#state{ connection_states = ConnectionStates, - session = Session0#session{session_id = - Hello#client_hello.session_id}, - handshake_env = - HsEnv#handshake_env{tls_handshake_history = HHistory}, - key_share = ClientKeyShare}, + session = Session0#session{session_id = Hello#client_hello.session_id}, + handshake_env = HsEnv#handshake_env{tls_handshake_history = HHistory, + key_share = ClientKeyShare} + }, %% If it is a hello_retry and middlebox mode is %% used assert the change_cipher_spec message @@ -738,11 +737,11 @@ handle_server_hello(#server_hello{cipher_suite = SelectedCipherSuite, random = Random, session_id = SessionId, extensions = Extensions} = ServerHello, - #state{key_share = ClientKeyShare, - ssl_options = #{ciphers := ClientCiphers, - supported_groups := ClientGroups0, - session_tickets := SessionTickets, - use_ticket := UseTicket}} = State0) -> + #state{handshake_env = #handshake_env{key_share = ClientKeyShare}, + ssl_options = #{ciphers := ClientCiphers, + supported_groups := ClientGroups0, + session_tickets := SessionTickets, + use_ticket := UseTicket}} = State0) -> {Ref,Maybe} = tls_gen_connection_1_3:do_maybe(), try ClientGroups = @@ -941,7 +940,6 @@ maybe_queue_cert_cert_cv(#state{connection_states = _ConnectionStates0, session = #session{session_id = _SessionId, own_certificates = OwnCerts}, ssl_options = #{} = _SslOpts, - key_share = _KeyShare, handshake_env = #handshake_env{tls_handshake_history = _HHistory0}, diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl index f37a5aef1681..ea509f78ffd7 100644 --- a/lib/ssl/src/tls_handshake_1_3.erl +++ b/lib/ssl/src/tls_handshake_1_3.erl @@ -1261,8 +1261,8 @@ update_start_state(#state{connection_states = ConnectionStates0, ConnectionStates0#{pending_read => PendingRead#{security_parameters => SecParamsR}, pending_write => PendingWrite#{security_parameters => SecParamsW}}, State#state{connection_states = ConnectionStates, - handshake_env = HsEnv#handshake_env{alpn = ALPNProtocol}, - key_share = KeyShare, + handshake_env = HsEnv#handshake_env{alpn = ALPNProtocol, + key_share = KeyShare}, session = Session#session{session_id = SessionId, ecc = Group, sign_alg = SelectedSignAlg, diff --git a/lib/ssl/src/tls_server_connection_1_3.erl b/lib/ssl/src/tls_server_connection_1_3.erl index 02746b2d738c..d5a4c1f217f4 100644 --- a/lib/ssl/src/tls_server_connection_1_3.erl +++ b/lib/ssl/src/tls_server_connection_1_3.erl @@ -511,13 +511,14 @@ send_hello_flight({start_handshake, PSK0}, #state{connection_states = ConnectionStates0, handshake_env = #handshake_env{ + key_share = KeyShare, early_data_accepted = EarlyDataAccepted}, static_env = #static_env{protocol_cb = Connection}, session = #session{session_id = SessionId, ecc = SelectedGroup, dh_public_value = ClientPublicKey}, - ssl_options = #{} = SslOpts, - key_share = KeyShare} = State0) -> + ssl_options = #{} = SslOpts + } = State0) -> ServerPrivateKey = select_server_private_key(KeyShare), #{security_parameters := SecParamsR} = From 7b7c155041825935e1434a7363a32511be6d5ece Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Mon, 29 Jan 2024 15:50:40 +0100 Subject: [PATCH 18/20] Remove some unnecessary record updates --- lib/ssl/src/tls_dtls_client_connection.erl | 6 ++---- lib/ssl/src/tls_sender.erl | 10 +++------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/ssl/src/tls_dtls_client_connection.erl b/lib/ssl/src/tls_dtls_client_connection.erl index bf9d8a50dfda..d022dc7117a7 100644 --- a/lib/ssl/src/tls_dtls_client_connection.erl +++ b/lib/ssl/src/tls_dtls_client_connection.erl @@ -471,11 +471,9 @@ handle_session(#server_hello{cipher_suite = CipherSuite}, case ssl_session:is_new(Session, NewId) of true -> - handle_new_session(NewId, CipherSuite, - State#state{connection_states = ConnectionStates}); + handle_new_session(NewId, CipherSuite, State); false -> - handle_resumed_session(NewId, - State#state{connection_states = ConnectionStates}) + handle_resumed_session(NewId, State) end. diff --git a/lib/ssl/src/tls_sender.erl b/lib/ssl/src/tls_sender.erl index 505facf85deb..6ece50951a1a 100644 --- a/lib/ssl/src/tls_sender.erl +++ b/lib/ssl/src/tls_sender.erl @@ -527,10 +527,10 @@ send_application_data(Data, From, StateName, chunk_and_key_update -> KeyUpdate = tls_handshake_1_3:key_update(update_requested), %% Prevent infinite loop of key updates - {Chunk, Rest} = chunk_data(Data, KeyUpdateAt), + {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}}, - {next_event, internal, {application_packets, From, Rest}}]}; + {next_event, internal, {application_packets, From, [Chunk]}}, + {next_event, internal, {application_packets, From, [Rest]}}]}; false -> {Msgs, ConnectionStates} = tls_record:encode_data(Data, Version, ConnectionStates0), case tls_socket:send(Transport, Socket, Msgs) of @@ -657,10 +657,6 @@ time_to_rekey(_, _Data, %% have a some what lower renegotiateAt and a much cheaper test is_time_to_renegotiate(Num, RenegotiateAt). -chunk_data(Data, Size) -> - {Chunk, Rest} = split_binary(iolist_to_binary(Data), Size), - {[Chunk], [Rest]}. - is_time_to_renegotiate(N, M) when N < M-> false; is_time_to_renegotiate(_,_) -> From be419661ed5d35918b8d71ca0d7363b3ca00b4b0 Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Wed, 31 Jan 2024 11:15:33 +0100 Subject: [PATCH 19/20] Refactor renegotiation data in connection states Reduce memory of the state record. --- lib/ssl/src/dtls_record.erl | 6 +- lib/ssl/src/ssl_handshake.erl | 46 +++---- lib/ssl/src/ssl_record.erl | 168 ++++++++++++-------------- lib/ssl/src/ssl_record.hrl | 8 +- lib/ssl/src/tls_record.erl | 8 +- lib/ssl/test/ssl_npn_hello_SUITE.erl | 2 +- lib/ssl/test/ssl_session_SUITE.erl | 13 +- lib/ssl/test/tls_1_3_record_SUITE.erl | 16 ++- 8 files changed, 125 insertions(+), 142 deletions(-) diff --git a/lib/ssl/src/dtls_record.erl b/lib/ssl/src/dtls_record.erl index aaadc8760c41..b821ed4bff7b 100644 --- a/lib/ssl/src/dtls_record.erl +++ b/lib/ssl/src/dtls_record.erl @@ -409,9 +409,9 @@ initial_connection_state(ConnectionEnd) -> replay_window => init_replay_window(), cipher_state => undefined, mac_secret => undefined, - secure_renegotiation => undefined, - client_verify_data => undefined, - server_verify_data => undefined + reneg => #{secure_renegotiation => undefined, + client_verify_data => undefined, + server_verify_data => undefined} }. get_dtls_records_aux({DataTag, StateName, _, Versions} = Vinfo, diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index 3c436a126849..83d59867ebda 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -3609,31 +3609,26 @@ max_frag_enum(undefined) -> renegotiation_info(_, client, _, false) -> #renegotiation_info{renegotiated_connection = undefined}; renegotiation_info(_RecordCB, server, ConnectionStates, false) -> - ConnectionState = ssl_record:current_connection_state(ConnectionStates, read), - case maps:get(secure_renegotiation, ConnectionState) of - true -> + case ssl_record:current_connection_state(ConnectionStates, read) of + #{reneg := #{secure_renegotiation := true}} -> #renegotiation_info{renegotiated_connection = ?byte(0)}; - false -> + _ -> #renegotiation_info{renegotiated_connection = undefined} end; renegotiation_info(_RecordCB, client, ConnectionStates, true) -> - ConnectionState = ssl_record:current_connection_state(ConnectionStates, read), - case maps:get(secure_renegotiation, ConnectionState) of - true -> - Data = maps:get(client_verify_data, ConnectionState), + case ssl_record:current_connection_state(ConnectionStates, read) of + #{reneg := #{secure_renegotiation := true, client_verify_data := Data}} -> #renegotiation_info{renegotiated_connection = Data}; - false -> + _ -> #renegotiation_info{renegotiated_connection = undefined} end; - renegotiation_info(_RecordCB, server, ConnectionStates, true) -> - ConnectionState = ssl_record:current_connection_state(ConnectionStates, read), - case maps:get(secure_renegotiation, ConnectionState) of - true -> - CData = maps:get(client_verify_data, ConnectionState), - SData = maps:get(server_verify_data, ConnectionState), - #renegotiation_info{renegotiated_connection = <>}; - false -> + case ssl_record:current_connection_state(ConnectionStates, read) of + #{reneg := #{secure_renegotiation := true, + client_verify_data := CData, + server_verify_data := SData}} -> + #renegotiation_info{renegotiated_connection = <>}; + _ -> #renegotiation_info{renegotiated_connection = undefined} end. @@ -3654,9 +3649,8 @@ handle_renegotiation_info(_, _RecordCB, _, undefined, ConnectionStates, false, _ handle_renegotiation_info(_, _RecordCB, client, #renegotiation_info{renegotiated_connection = ClientServerVerify}, ConnectionStates, true, _, _) -> - ConnectionState = ssl_record:current_connection_state(ConnectionStates, read), - CData = maps:get(client_verify_data, ConnectionState), - SData = maps:get(server_verify_data, ConnectionState), + #{reneg := ReNeg} = ssl_record:current_connection_state(ConnectionStates, read), + #{client_verify_data := CData, server_verify_data := SData} = ReNeg, case <> == ClientServerVerify of true -> {ok, ConnectionStates}; @@ -3670,12 +3664,10 @@ handle_renegotiation_info(_, _RecordCB, server, #renegotiation_info{renegotiated true -> throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, {server_renegotiation, empty_renegotiation_info_scsv})); false -> - ConnectionState = ssl_record:current_connection_state(ConnectionStates, read), - Data = maps:get(client_verify_data, ConnectionState), - case Data == ClientVerify of - true -> + case ssl_record:current_connection_state(ConnectionStates, read) of + #{reneg := #{client_verify_data := ClientVerify}} -> {ok, ConnectionStates}; - false -> + _ -> throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, server_renegotiation)) end end; @@ -3691,8 +3683,8 @@ handle_renegotiation_info(_, RecordCB, server, undefined, ConnectionStates, true end. handle_renegotiation_info(_RecordCB, ConnectionStates, SecureRenegotation) -> - ConnectionState = ssl_record:current_connection_state(ConnectionStates, read), - case {SecureRenegotation, maps:get(secure_renegotiation, ConnectionState)} of + #{reneg := #{secure_renegotiation := SR}} = ssl_record:current_connection_state(ConnectionStates, read), + case {SecureRenegotation, SR} of {_, true} -> throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, already_secure)); {true, false} -> diff --git a/lib/ssl/src/ssl_record.erl b/lib/ssl/src/ssl_record.erl index dc5cf0458322..da4db79c671c 100644 --- a/lib/ssl/src/ssl_record.erl +++ b/lib/ssl/src/ssl_record.erl @@ -100,25 +100,24 @@ pending_connection_state(ConnectionStates, write) -> activate_pending_connection_state(#{current_read := Current, pending_read := Pending} = States, read, Connection) -> - #{secure_renegotiation := SecureRenegotation} = Current, - #{security_parameters := SecParams} = Pending, - NewCurrent = Pending#{sequence_number => 0}, - ConnectionEnd = SecParams#security_parameters.connection_end, - EmptyPending = Connection:empty_connection_state(ConnectionEnd), - NewPending = EmptyPending#{secure_renegotiation => SecureRenegotation}, - States#{current_read => NewCurrent, - pending_read => NewPending}; + {NewCurrent, NewPending} = activate_pending_connection_state_1(Current, Pending, Connection), + States#{current_read => NewCurrent, pending_read => NewPending}; activate_pending_connection_state(#{current_write := Current, pending_write := Pending} = States, write, Connection) -> - NewCurrent = Pending#{sequence_number => 0}, - #{secure_renegotiation := SecureRenegotation} = Current, + {NewCurrent, NewPending} = activate_pending_connection_state_1(Current, Pending, Connection), + States#{current_write => NewCurrent, pending_write => NewPending}. + +activate_pending_connection_state_1(Current, Pending, Connection) -> + #{reneg := #{secure_renegotiation := SecReNeg}} = Current, + Update = fun(#{reneg := ReNeg} = Cs) -> + Cs#{reneg := ReNeg#{secure_renegotiation := SecReNeg}} + end, #{security_parameters := SecParams} = Pending, + NewCurrent = Pending#{sequence_number => 0}, ConnectionEnd = SecParams#security_parameters.connection_end, EmptyPending = Connection:empty_connection_state(ConnectionEnd), - NewPending = EmptyPending#{secure_renegotiation => SecureRenegotation}, - States#{current_write => NewCurrent, - pending_write => NewPending}. + {NewCurrent, Update(EmptyPending)}. %%-------------------------------------------------------------------- -spec step_encryption_state(#state{}) -> #state{}. @@ -198,19 +197,19 @@ set_master_secret(MasterSecret, %% %% Description: Set secure_renegotiation in pending connection states %%-------------------------------------------------------------------- -set_renegotiation_flag(Flag, #{current_read := CurrentRead0, - current_write := CurrentWrite0, - pending_read := PendingRead0, - pending_write := PendingWrite0} - = ConnectionStates) -> - CurrentRead = CurrentRead0#{secure_renegotiation => Flag}, - CurrentWrite = CurrentWrite0#{secure_renegotiation => Flag}, - PendingRead = PendingRead0#{secure_renegotiation => Flag}, - PendingWrite = PendingWrite0#{secure_renegotiation => Flag}, - ConnectionStates#{current_read => CurrentRead, - current_write => CurrentWrite, - pending_read => PendingRead, - pending_write => PendingWrite}. +set_renegotiation_flag(Flag, #{current_read := CurrentRead, + current_write := CurrentWrite, + pending_read := PendingRead, + pending_write := PendingWrite} + = ConnectionStates) + when is_boolean(Flag) -> + Update = fun(#{reneg := ReNeg} = CS) -> + CS#{reneg := ReNeg#{secure_renegotiation := Flag}} + end, + ConnectionStates#{current_read => Update(CurrentRead), + current_write => Update(CurrentWrite), + pending_read => Update(PendingRead), + pending_write => Update(PendingWrite)}. %%-------------------------------------------------------------------- -spec set_max_fragment_length(term(), connection_states()) -> connection_states(). @@ -235,30 +234,24 @@ set_max_fragment_length(_,ConnectionStates) -> %% %% Description: Set verify data in connection states. %%-------------------------------------------------------------------- -set_client_verify_data(current_read, Data, - #{current_read := CurrentRead0, - pending_write := PendingWrite0} - = ConnectionStates) -> - CurrentRead = CurrentRead0#{client_verify_data => Data}, - PendingWrite = PendingWrite0#{client_verify_data => Data}, - ConnectionStates#{current_read => CurrentRead, - pending_write => PendingWrite}; -set_client_verify_data(current_write, Data, - #{pending_read := PendingRead0, - current_write := CurrentWrite0} - = ConnectionStates) -> - PendingRead = PendingRead0#{client_verify_data => Data}, - CurrentWrite = CurrentWrite0#{client_verify_data => Data}, - ConnectionStates#{pending_read => PendingRead, - current_write => CurrentWrite}; -set_client_verify_data(current_both, Data, - #{current_read := CurrentRead0, - current_write := CurrentWrite0} - = ConnectionStates) -> - CurrentRead = CurrentRead0#{client_verify_data => Data}, - CurrentWrite = CurrentWrite0#{client_verify_data => Data}, - ConnectionStates#{current_read => CurrentRead, - current_write => CurrentWrite}. +set_client_verify_data(What, Data, ConnectionStates) -> + Update = fun(#{reneg := ReNeg} = CS) -> + CS#{reneg := ReNeg#{client_verify_data := Data}} + end, + case What of + current_read -> + #{current_read := CurrentRead, pending_write := PendingWrite} = ConnectionStates, + ConnectionStates#{current_read => Update(CurrentRead), + pending_write => Update(PendingWrite)}; + current_write -> + #{pending_read := PendingRead, current_write := CurrentWrite} = ConnectionStates, + ConnectionStates#{pending_read => Update(PendingRead), + current_write => Update(CurrentWrite)}; + current_both -> + #{current_read := CurrentRead, current_write := CurrentWrite} = ConnectionStates, + ConnectionStates#{current_read => Update(CurrentRead), + current_write => Update(CurrentWrite)} + end. %%-------------------------------------------------------------------- -spec set_server_verify_data(current_read | current_write | current_both, binary(), connection_states())-> @@ -266,32 +259,24 @@ set_client_verify_data(current_both, Data, %% %% Description: Set verify data in pending connection states. %%-------------------------------------------------------------------- -set_server_verify_data(current_write, Data, - #{pending_read := PendingRead0, - current_write := CurrentWrite0} - = ConnectionStates) -> - PendingRead = PendingRead0#{server_verify_data => Data}, - CurrentWrite = CurrentWrite0#{server_verify_data => Data}, - ConnectionStates#{pending_read => PendingRead, - current_write => CurrentWrite}; - -set_server_verify_data(current_read, Data, - #{current_read := CurrentRead0, - pending_write := PendingWrite0} - = ConnectionStates) -> - CurrentRead = CurrentRead0#{server_verify_data => Data}, - PendingWrite = PendingWrite0#{server_verify_data => Data}, - ConnectionStates#{current_read => CurrentRead, - pending_write => PendingWrite}; - -set_server_verify_data(current_both, Data, - #{current_read := CurrentRead0, - current_write := CurrentWrite0} - = ConnectionStates) -> - CurrentRead = CurrentRead0#{server_verify_data => Data}, - CurrentWrite = CurrentWrite0#{server_verify_data => Data}, - ConnectionStates#{current_read => CurrentRead, - current_write => CurrentWrite}. +set_server_verify_data(What, Data, ConnectionStates) -> + Update = fun(#{reneg := ReNeg} = CS) -> + CS#{reneg := ReNeg#{server_verify_data := Data}} + end, + case What of + current_write -> + #{pending_read := PendingRead, current_write := CurrentWrite} = ConnectionStates, + ConnectionStates#{pending_read => Update(PendingRead), + current_write => Update(CurrentWrite)}; + current_read -> + #{current_read := CurrentRead, pending_write := PendingWrite} = ConnectionStates, + ConnectionStates#{current_read => Update(CurrentRead), + pending_write => Update(PendingWrite)}; + current_both -> + #{current_read := CurrentRead, current_write := CurrentWrite} = ConnectionStates, + ConnectionStates#{current_read => Update(CurrentRead), + current_write => Update(CurrentWrite)} + end. %%-------------------------------------------------------------------- -spec set_pending_cipher_state(connection_states(), #cipher_state{}, #cipher_state{}, client | server) -> @@ -299,19 +284,16 @@ set_server_verify_data(current_both, Data, %% %% Description: Set the cipher state in the specified pending connection state. %%-------------------------------------------------------------------- -set_pending_cipher_state(#{pending_read := Read, - pending_write := Write} = States, - ClientState, ServerState, server) -> - States#{ - pending_read => Read#{cipher_state => ClientState}, - pending_write => Write#{cipher_state => ServerState}}; - -set_pending_cipher_state(#{pending_read := Read, - pending_write := Write} = States, - ClientState, ServerState, client) -> - States#{ - pending_read => Read#{cipher_state => ServerState}, - pending_write => Write#{cipher_state => ClientState}}. +set_pending_cipher_state(#{pending_read := Read, pending_write := Write} = States, + ClientState, ServerState, Role) -> + case Role of + server -> + States#{pending_read => Read#{cipher_state => ClientState}, + pending_write => Write#{cipher_state => ServerState}}; + client -> + States#{pending_read => Read#{cipher_state => ServerState}, + pending_write => Write#{cipher_state => ClientState}} + end. %%==================================================================== %% Payload encryption/decryption @@ -435,12 +417,12 @@ empty_connection_state(ConnectionEnd, Version, MaxEarlyDataSize) -> #{security_parameters => SecParams, cipher_state => undefined, mac_secret => undefined, - secure_renegotiation => undefined, - client_verify_data => undefined, - server_verify_data => undefined, pending_early_data_size => MaxEarlyDataSize, trial_decryption => false, - early_data_accepted => false + early_data_accepted => false, + reneg => #{secure_renegotiation => undefined, + client_verify_data => undefined, + server_verify_data => undefined} }. init_security_parameters(?CLIENT, Version) -> diff --git a/lib/ssl/src/ssl_record.hrl b/lib/ssl/src/ssl_record.hrl index e5a741323bba..763d99e74efc 100644 --- a/lib/ssl/src/ssl_record.hrl +++ b/lib/ssl/src/ssl_record.hrl @@ -36,10 +36,10 @@ %% cipher_state, %% mac_secret, %% sequence_number, -%% %% RFC 5746 -%% secure_renegotiation, -%% client_verify_data, -%% server_verify_data, +%% reneg = %% RFC 5746 +%% #{secure_renegotiation, +%% client_verify_data, +%% server_verify_data}, %% %% How to do BEAST mitigation? %% beast_mitigation %% }). diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl index de0076efa699..9ecdc893abe6 100644 --- a/lib/ssl/src/tls_record.erl +++ b/lib/ssl/src/tls_record.erl @@ -461,12 +461,12 @@ initial_connection_state(ConnectionEnd, MaxEarlyDataSize) -> sequence_number => 0, cipher_state => undefined, mac_secret => undefined, - secure_renegotiation => undefined, - client_verify_data => undefined, - server_verify_data => undefined, pending_early_data_size => MaxEarlyDataSize, trial_decryption => false, - early_data_expected => false + early_data_expected => false, + reneg => #{secure_renegotiation => undefined, + client_verify_data => undefined, + server_verify_data => undefined} }. %% Used by logging to recreate the received bytes diff --git a/lib/ssl/test/ssl_npn_hello_SUITE.erl b/lib/ssl/test/ssl_npn_hello_SUITE.erl index 946c931ce400..12516e29d742 100644 --- a/lib/ssl/test/ssl_npn_hello_SUITE.erl +++ b/lib/ssl/test/ssl_npn_hello_SUITE.erl @@ -164,7 +164,7 @@ create_connection_states() -> cipher_suite = ?TLS_DHE_DSS_WITH_DES_CBC_SHA } }, - current_read => #{secure_renegotiation => false + current_read => #{reneg => #{secure_renegotiation => false} } }. diff --git a/lib/ssl/test/ssl_session_SUITE.erl b/lib/ssl/test/ssl_session_SUITE.erl index e9c350399f7a..0901539b9c0d 100644 --- a/lib/ssl/test/ssl_session_SUITE.erl +++ b/lib/ssl/test/ssl_session_SUITE.erl @@ -760,9 +760,13 @@ client_hello(Random) -> connection_states(Random) -> #{current_write => - #{beast_mitigation => one_n_minus_one,cipher_state => undefined, - client_verify_data => undefined, - mac_secret => undefined,secure_renegotiation => undefined, + #{beast_mitigation => one_n_minus_one, + cipher_state => undefined, + mac_secret => undefined, + reneg => #{secure_renegotiation => undefined, + client_verify_data => undefined, + server_verify_data => undefined + }, security_parameters => #security_parameters{ cipher_suite = <<0,0>>, @@ -778,7 +782,8 @@ connection_states(Random) -> resumption_master_secret = undefined, client_random = Random, server_random = undefined}, - sequence_number => 0,server_verify_data => undefined,max_fragment_length => undefined}}. + sequence_number => 0, + max_fragment_length => undefined}}. diff --git a/lib/ssl/test/tls_1_3_record_SUITE.erl b/lib/ssl/test/tls_1_3_record_SUITE.erl index 78df8eceb6f3..763a0201c03e 100644 --- a/lib/ssl/test/tls_1_3_record_SUITE.erl +++ b/lib/ssl/test/tls_1_3_record_SUITE.erl @@ -88,8 +88,10 @@ encode_decode(_Config) -> <<197,54,168,218,54,91,157,58,30,201,197,142,51,58,53,231,228, 131,57,122,170,78,82,196,30,48,23,16,95,255,185,236>>, undefined,undefined,undefined,16}, - client_verify_data => undefined, - mac_secret => undefined,secure_renegotiation => undefined, + mac_secret => undefined, + reneg => #{secure_renegotiation => undefined, + client_verify_data => undefined, + server_verify_data => undefined}, security_parameters => #security_parameters{ cipher_suite = <<19,2>>, @@ -106,7 +108,7 @@ encode_decode(_Config) -> server_random = <<92,24,205,75,244,60,136,212,250,32,214,20,37,3,213,87,61,207, 147,61,168,145,177,118,160,153,33,53,48,108,191,174>>}, - sequence_number => 0,server_verify_data => undefined, + sequence_number => 0, pending_early_data_size => 0, trial_decryption => false, early_data_accepted => false}, @@ -119,8 +121,10 @@ encode_decode(_Config) -> <<197,54,168,218,54,91,157,58,30,201,197,142,51,58,53,231,228, 131,57,122,170,78,82,196,30,48,23,16,95,255,185,236>>, undefined,undefined,undefined,16}, - client_verify_data => undefined, - mac_secret => undefined,secure_renegotiation => undefined, + mac_secret => undefined, + reneg => #{secure_renegotiation => undefined, + client_verify_data => undefined, + server_verify_data => undefined}, security_parameters => #security_parameters{ cipher_suite = <<19,2>>, @@ -137,7 +141,7 @@ encode_decode(_Config) -> server_random = <<92,24,205,75,244,60,136,212,250,32,214,20,37,3,213,87,61,207, 147,61,168,145,177,118,160,153,33,53,48,108,191,174>>}, - sequence_number => 0,server_verify_data => undefined},max_fragment_length => undefined}, + sequence_number => 0},max_fragment_length => undefined}, PlainText = [11, <<0,2,175>>, From 6004d27eda492cf32e9ec31e87f354198c91c7ee Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Thu, 1 Feb 2024 18:45:31 +0100 Subject: [PATCH 20/20] Move early_data meta data to its own map To reduce memory when updating record states. --- lib/ssl/src/ssl_record.erl | 7 +-- lib/ssl/src/tls_record.erl | 7 +-- lib/ssl/src/tls_record_1_3.erl | 54 +++++++++++------------ lib/ssl/src/tls_server_connection_1_3.erl | 7 +-- lib/ssl/test/tls_1_3_record_SUITE.erl | 8 ++-- 5 files changed, 44 insertions(+), 39 deletions(-) diff --git a/lib/ssl/src/ssl_record.erl b/lib/ssl/src/ssl_record.erl index da4db79c671c..14cd800664ce 100644 --- a/lib/ssl/src/ssl_record.erl +++ b/lib/ssl/src/ssl_record.erl @@ -417,9 +417,10 @@ empty_connection_state(ConnectionEnd, Version, MaxEarlyDataSize) -> #{security_parameters => SecParams, cipher_state => undefined, mac_secret => undefined, - pending_early_data_size => MaxEarlyDataSize, - trial_decryption => false, - early_data_accepted => false, + early_data => #{pending_early_data_size => MaxEarlyDataSize, + trial_decryption => false, + early_data_accepted => false + }, reneg => #{secure_renegotiation => undefined, client_verify_data => undefined, server_verify_data => undefined} diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl index 9ecdc893abe6..759689c0df0b 100644 --- a/lib/ssl/src/tls_record.erl +++ b/lib/ssl/src/tls_record.erl @@ -461,9 +461,10 @@ initial_connection_state(ConnectionEnd, MaxEarlyDataSize) -> sequence_number => 0, cipher_state => undefined, mac_secret => undefined, - pending_early_data_size => MaxEarlyDataSize, - trial_decryption => false, - early_data_expected => false, + early_data => #{pending_early_data_size => MaxEarlyDataSize, + trial_decryption => false, + early_data_expected => false + }, reneg => #{secure_renegotiation => undefined, client_verify_data => undefined, server_verify_data => undefined} diff --git a/lib/ssl/src/tls_record_1_3.erl b/lib/ssl/src/tls_record_1_3.erl index 1ed91d458d09..1f443f847038 100644 --- a/lib/ssl/src/tls_record_1_3.erl +++ b/lib/ssl/src/tls_record_1_3.erl @@ -116,9 +116,11 @@ decode_cipher_text(#ssl_tls{type = ?OPAQUE_TYPE, cipher_type = ?AEAD, bulk_cipher_algorithm = BulkCipherAlgo}, - pending_early_data_size := PendingMaxEarlyDataSize0, - trial_decryption := TrialDecryption, - early_data_accepted := EarlyDataAccepted + early_data := + #{pending_early_data_size := PendingMaxEarlyDataSize0, + trial_decryption := TrialDecryption, + early_data_accepted := EarlyDataAccepted + } } = ReadState0} = ConnectionStates0) -> case decipher_aead(CipherFragment, BulkCipherAlgo, Key, Seq, IV, TagLen) of #alert{} when TrialDecryption =:= true andalso @@ -199,24 +201,24 @@ decode_cipher_text(#ssl_tls{type = Type}, _) -> %%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- -ignore_early_data(ConnectionStates0, ReadState0, PendingMaxEarlyDataSize0, - BulkCipherAlgo, CipherFragment) -> - PendingMaxEarlyDataSize = - approximate_pending_early_data_size(PendingMaxEarlyDataSize0, - BulkCipherAlgo, CipherFragment), - ConnectionStates = - ConnectionStates0#{current_read => - ReadState0#{pending_early_data_size => PendingMaxEarlyDataSize}}, - if PendingMaxEarlyDataSize < 0 -> - %% More early data is trial decrypted as the configured limit - ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC, {decryption_failed, - {max_early_data_threshold_exceeded, - PendingMaxEarlyDataSize}}); - true -> - {no_record, ConnectionStates} - end. -process_early_data(ConnectionStates0, ReadState0, PendingMaxEarlyDataSize0, Seq, - PlainFragment) -> +ignore_early_data(ConnectionStates0, #{early_data:=EarlyData0} = ReadState0, + PendingMaxEarlyDataSize0, + BulkCipherAlgo, CipherFragment) -> + PendingMaxEarlyDataSize = approximate_pending_early_data_size(PendingMaxEarlyDataSize0, + BulkCipherAlgo, CipherFragment), + EarlyData = EarlyData0#{pending_early_data_size => PendingMaxEarlyDataSize}, + ConnectionStates = ConnectionStates0#{current_read => ReadState0#{early_data := EarlyData}}, + if PendingMaxEarlyDataSize < 0 -> + %% More early data is trial decrypted as the configured limit + ?ALERT_REC(?FATAL, ?BAD_RECORD_MAC, {decryption_failed, + {max_early_data_threshold_exceeded, + PendingMaxEarlyDataSize}}); + true -> + {no_record, ConnectionStates} + end. + +process_early_data(ConnectionStates0, #{early_data:=EarlyData0} = ReadState0, + PendingMaxEarlyDataSize0, Seq, PlainFragment) -> %% First packet is deciphered anyway so we must check if more early data is received %% than the configured limit (max_early_data_size). case Record = decode_inner_plaintext(PlainFragment) of @@ -226,8 +228,7 @@ process_early_data(ConnectionStates0, ReadState0, PendingMaxEarlyDataSize0, Seq, ReadState0#{sequence_number => Seq + 1}}, {Record, ConnectionStates}; #ssl_tls{type=?APPLICATION_DATA, fragment=Data} -> - PendingMaxEarlyDataSize = - pending_early_data_size(PendingMaxEarlyDataSize0, Data), + PendingMaxEarlyDataSize = pending_early_data_size(PendingMaxEarlyDataSize0, Data), if PendingMaxEarlyDataSize < 0 -> %% Too much early data received, send alert unexpected_message ?ALERT_REC(?FATAL, ?UNEXPECTED_MESSAGE, @@ -235,10 +236,9 @@ process_early_data(ConnectionStates0, ReadState0, PendingMaxEarlyDataSize0, Seq, {max_early_data_threshold_exceeded, PendingMaxEarlyDataSize}}); true -> - ConnectionStates = - ConnectionStates0#{current_read => - ReadState0#{sequence_number => Seq + 1, - pending_early_data_size => PendingMaxEarlyDataSize}}, + EarlyData = EarlyData0#{pending_early_data_size => PendingMaxEarlyDataSize}, + ReadState = ReadState0#{sequence_number => Seq + 1, early_data => EarlyData}, + ConnectionStates = ConnectionStates0#{current_read => ReadState}, {Record#ssl_tls{early_data = true}, ConnectionStates} end end. diff --git a/lib/ssl/src/tls_server_connection_1_3.erl b/lib/ssl/src/tls_server_connection_1_3.erl index d5a4c1f217f4..ef1fb7cb2b29 100644 --- a/lib/ssl/src/tls_server_connection_1_3.erl +++ b/lib/ssl/src/tls_server_connection_1_3.erl @@ -799,9 +799,10 @@ send_hello_retry_request(State0, _, _, _) -> {ok, {State0, negotiated}}. update_current_read(#state{connection_states = CS} = State, TrialDecryption, EarlyDataExpected) -> - Read0 = ssl_record:current_connection_state(CS, read), - Read = Read0#{trial_decryption => TrialDecryption, - early_data_accepted => EarlyDataExpected}, + #{early_data := EarlyData0} = Read0 = ssl_record:current_connection_state(CS, read), + EarlyData = EarlyData0#{trial_decryption => TrialDecryption, + early_data_accepted => EarlyDataExpected}, + Read = Read0#{early_data := EarlyData}, State#state{connection_states = CS#{current_read => Read}}. handle_early_data(State, enabled, #early_data_indication{}) -> diff --git a/lib/ssl/test/tls_1_3_record_SUITE.erl b/lib/ssl/test/tls_1_3_record_SUITE.erl index 763a0201c03e..aa02594ed5fa 100644 --- a/lib/ssl/test/tls_1_3_record_SUITE.erl +++ b/lib/ssl/test/tls_1_3_record_SUITE.erl @@ -109,9 +109,11 @@ encode_decode(_Config) -> <<92,24,205,75,244,60,136,212,250,32,214,20,37,3,213,87,61,207, 147,61,168,145,177,118,160,153,33,53,48,108,191,174>>}, sequence_number => 0, - pending_early_data_size => 0, - trial_decryption => false, - early_data_accepted => false}, + early_data => #{ + pending_early_data_size => 0, + trial_decryption => false, + early_data_accepted => false} + }, current_write => #{beast_mitigation => one_n_minus_one, cipher_state =>