From b6502cb2023f66e3c4ab1864b0298cfc159abb09 Mon Sep 17 00:00:00 2001 From: Jamil Bou Kheir Date: Wed, 17 Jun 2026 13:20:37 -0700 Subject: [PATCH] fix(tls): allow no preceding ccs --- lib/ssl/src/tls_client_connection_1_3.erl | 14 +++++-- lib/ssl/test/openssl_server_cert_SUITE.erl | 16 ++++++++ lib/ssl/test/ssl_cert_tests.erl | 48 ++++++++++++++++++++++ lib/ssl/test/ssl_test_lib.erl | 21 ++++++++-- 4 files changed, 92 insertions(+), 7 deletions(-) diff --git a/lib/ssl/src/tls_client_connection_1_3.erl b/lib/ssl/src/tls_client_connection_1_3.erl index 89f19daa2d1f..63b1fae70331 100644 --- a/lib/ssl/src/tls_client_connection_1_3.erl +++ b/lib/ssl/src/tls_client_connection_1_3.erl @@ -318,11 +318,14 @@ hello_middlebox_assert(enter, _, State) -> {keep_state, State}; hello_middlebox_assert(internal, #change_cipher_spec{}, State) -> tls_gen_connection:next_event(wait_ee, no_record, State); -hello_middlebox_assert(internal = Type, #encrypted_extensions{} = Msg, +hello_middlebox_assert(internal, #encrypted_extensions{} = Msg, #state{ssl_options = #{log_level := Level}} = State) -> ssl_logger:log(warning, Level, #{description => "Failed to assert middlebox server message", reason => [{missing, #change_cipher_spec{}}]}, ?LOCATION), - ssl_gen_statem:handle_common_event(Type, Msg, hello_middlebox_assert, State); + %% D.4. Middlebox Compatibility Mode: the server's dummy change_cipher_spec + %% is optional (a server in non-middlebox mode never sends it), so its + %% absence must be tolerated. Process the EncryptedExtensions in 'wait_ee'. + {next_state, wait_ee, State, [{next_event, internal, Msg}]}; hello_middlebox_assert(info, Msg, State) -> tls_gen_connection:gen_info(Msg, ?STATE(hello_middlebox_assert), State); hello_middlebox_assert(Type, Msg, State) -> @@ -338,11 +341,14 @@ hello_retry_middlebox_assert(enter, _, State) -> {keep_state, State}; hello_retry_middlebox_assert(internal, #change_cipher_spec{}, State) -> tls_gen_connection:next_event(wait_sh, no_record, State); -hello_retry_middlebox_assert(internal = Type, #server_hello{} = Msg, +hello_retry_middlebox_assert(internal, #server_hello{} = Msg, #state{ssl_options = #{log_level := Level}} = State) -> ssl_logger:log(warning, Level, #{description => "Failed to assert middlebox server message", reason => [{missing, #change_cipher_spec{}}]}, ?LOCATION), - ssl_gen_statem:handle_common_event(Type, Msg, ?STATE(hello_retry_middlebox_assert), State); + %% D.4. Middlebox Compatibility Mode: the server's dummy change_cipher_spec + %% is optional (a server in non-middlebox mode never sends it), so its + %% absence must be tolerated. Process the ServerHello in 'wait_sh'. + {next_state, wait_sh, State, [{next_event, internal, Msg}]}; hello_retry_middlebox_assert(info, Msg, State) -> tls_gen_connection:gen_info(Msg, ?STATE(hello_retry_middlebox_assert), State); hello_retry_middlebox_assert(Type, Msg, State) -> diff --git a/lib/ssl/test/openssl_server_cert_SUITE.erl b/lib/ssl/test/openssl_server_cert_SUITE.erl index a7fa9c444aa6..f216e1162bb3 100644 --- a/lib/ssl/test/openssl_server_cert_SUITE.erl +++ b/lib/ssl/test/openssl_server_cert_SUITE.erl @@ -57,6 +57,10 @@ unsupported_sign_algo_cert_client_auth/1, hello_retry_request/0, hello_retry_request/1, + hello_retry_request_no_middlebox/0, + hello_retry_request_no_middlebox/1, + no_middlebox_server/0, + no_middlebox_server/1, custom_groups/0, custom_groups/1, mlkem_groups/0, @@ -138,6 +142,8 @@ tls_1_3_protocol_groups() -> tls_1_3_tests() -> [ hello_retry_request, + hello_retry_request_no_middlebox, + no_middlebox_server, custom_groups, mlkem_groups, mlkem_hybrid_groups, @@ -274,6 +280,16 @@ hello_retry_request() -> hello_retry_request(Config) -> ssl_cert_tests:hello_retry_request(Config). %%-------------------------------------------------------------------- +hello_retry_request_no_middlebox() -> + ssl_cert_tests:hello_retry_request_no_middlebox(). +hello_retry_request_no_middlebox(Config) -> + ssl_cert_tests:hello_retry_request_no_middlebox(Config). +%%-------------------------------------------------------------------- +no_middlebox_server() -> + ssl_cert_tests:no_middlebox_server(). +no_middlebox_server(Config) -> + ssl_cert_tests:no_middlebox_server(Config). +%%-------------------------------------------------------------------- custom_groups() -> ssl_cert_tests:custom_groups(). custom_groups(Config) -> diff --git a/lib/ssl/test/ssl_cert_tests.erl b/lib/ssl/test/ssl_cert_tests.erl index 46876bef48f9..114b7d513a69 100644 --- a/lib/ssl/test/ssl_cert_tests.erl +++ b/lib/ssl/test/ssl_cert_tests.erl @@ -74,6 +74,10 @@ certs_keys_signature_algs_selection/1, hello_retry_request/0, hello_retry_request/1, + hello_retry_request_no_middlebox/0, + hello_retry_request_no_middlebox/1, + no_middlebox_server/0, + no_middlebox_server/1, custom_groups/0, custom_groups/1, mlkem_groups/0, @@ -751,6 +755,50 @@ hello_retry_request(Config) -> ssl_test_lib:basic_test(ClientOpts, ServerOpts, Config). %%-------------------------------------------------------------------- +hello_retry_request_no_middlebox() -> + [{doc, "Test that a TLS-1.3 client using middlebox compatibility mode can " + "complete a HelloRetryRequest handshake against a server that does not " + "send the optional dummy change_cipher_spec record (RFC 8446 D.4). " + "Regression test for GH-11237: the client must tolerate the missing " + "change_cipher_spec in the hello_retry_middlebox_assert state instead " + "of aborting with an unexpected_message alert."}]. + +hello_retry_request_no_middlebox(Config) -> + ClientOpts0 = ssl_test_lib:ssl_options(client_cert_opts, Config), + ServerOpts0 = ssl_test_lib:ssl_options(server_cert_opts, Config), + + {ServerOpts1, ClientOpts} = + group_config(Config, + [{versions, ['tlsv1.3']} | ServerOpts0], + [{versions, ['tlsv1.3']}, + {middlebox_comp_mode, true} | ClientOpts0]), + + %% Tell the OpenSSL server to omit the dummy change_cipher_spec records, + %% in particular the one normally sent after the HelloRetryRequest. + ServerOpts = [{middlebox, false} | ServerOpts1], + + ssl_test_lib:basic_test(ClientOpts, ServerOpts, Config). +%%-------------------------------------------------------------------- +no_middlebox_server() -> + [{doc, "Test that a TLS-1.3 client using middlebox compatibility mode can " + "complete an ordinary (non-HelloRetryRequest) handshake against a server " + "that does not send the optional dummy change_cipher_spec record " + "(RFC 8446 D.4). The client must tolerate the missing change_cipher_spec " + "in the hello_middlebox_assert state instead of aborting with an " + "unexpected_message alert."}]. + +no_middlebox_server(Config) -> + ClientOpts0 = ssl_test_lib:ssl_options(client_cert_opts, Config), + ServerOpts0 = ssl_test_lib:ssl_options(server_cert_opts, Config), + + ClientOpts = [{versions, ['tlsv1.3']}, + {middlebox_comp_mode, true} | ClientOpts0], + %% Tell the OpenSSL server to omit the dummy change_cipher_spec record it + %% would otherwise send after its ServerHello. + ServerOpts = [{versions, ['tlsv1.3']}, {middlebox, false} | ServerOpts0], + + ssl_test_lib:basic_test(ClientOpts, ServerOpts, Config). +%%-------------------------------------------------------------------- custom_groups() -> [{doc,"Test that ssl server can select a common group for key-exchange"}]. diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl index 7f4a92366550..0d912b000b0b 100644 --- a/lib/ssl/test/ssl_test_lib.erl +++ b/lib/ssl/test/ssl_test_lib.erl @@ -941,7 +941,16 @@ init_openssl_server(openssl, _, Options) -> Args0 ++ ["-early_data", "-no_anti_replay", "-max_early_data", integer_to_list(MaxSize)] end, - Args = maybe_force_ipv4(Args1), + Args2 = case proplists:get_value(middlebox, Options, true) of + false -> + %% Disable the dummy change_cipher_spec records that + %% s_server otherwise sends for middlebox compatibility + %% (RFC 8446 D.4). + Args1 ++ ["-no_middlebox"]; + _ -> + Args1 + end, + Args = maybe_force_ipv4(Args2), SslPort = portable_open_port(Exe, Args), wait_for_openssl_server(Port, proplists:get_value(protocol, Options, tls)), Pid ! {started, SslPort}, @@ -2545,17 +2554,23 @@ start_server(openssl, ClientOpts, ServerOpts, Config) -> SessionArgs = proplists:get_value(session_args, Config, []), DOpenssl = proplists:get_value(debug_openssl, ServerOpts, false), Debug = openssl_debug_options(DOpenssl), + %% Disable the dummy change_cipher_spec records that s_server otherwise + %% sends for middlebox compatibility (RFC 8446 D.4). + MiddleBox = case proplists:get_value(middlebox, ServerOpts, true) of + false -> ["-no_middlebox"]; + _ -> [] + end, Args = case Groups0 of undefined -> ["s_server", "-accept", integer_to_list(Port), cipher_flag(Version), ciphers(Ciphers, Version), - version_flag(Version)] ++ sig_algs(SigAlgs) ++ CertArgs ++ SessionArgs ++ Debug; + version_flag(Version)] ++ MiddleBox ++ sig_algs(SigAlgs) ++ CertArgs ++ SessionArgs ++ Debug; Group -> ["s_server", "-accept", integer_to_list(Port), cipher_flag(Version), ciphers(Ciphers, Version), "-groups", Group, version_flag(Version)] ++ - sig_algs(SigAlgs) ++ CertArgs ++ SessionArgs ++ Debug + MiddleBox ++ sig_algs(SigAlgs) ++ CertArgs ++ SessionArgs ++ Debug end, OpenSslPort = portable_open_port(Exe, Args), true = port_command(OpenSslPort, "Hello world"),