Reject HelloRequest in TLS 1.3

TLS 1.3 reserves handshake message type 0 and must not silently
ignore HelloRequest records. The legacy client-side HelloRequest skip
path in tls_get_message_header() could run before the TLS 1.3 state
machine had a chance to reject the message, so a zero-length
HelloRequest injected after ClientHello was discarded instead of
triggering unexpected_message.

Restrict the skip to cases where TLS 1.3 is no longer possible.
Before ServerHello selects a version, s->version is the configured
maximum; after ServerHello or during renegotiation, it is the
negotiated version. Skip only when that value is below TLS 1.3,
preserving the existing TLS 1.2-and-below behavior.

Add TLSProxy regression tests covering rejection while TLS 1.3 is
possible and the preserved TLS 1.2 skip after ServerHello.

Fixes #31531

Reviewed-by: Bob Beck <beck@openssl.org>
Reviewed-by: Norbert Pocs <norbertp@openssl.org>
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
MergeDate: Wed Jun 24 13:01:12 2026
(Merged from https://github.com/openssl/openssl/pull/31577)

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index bacbd58218a848db069dedab68c8705a641313bf..c9d76fe8a7296cc91333ad9a20a14b56d92a3250 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1535,6 +1535,23 @@ WORK_STATE tls_finish_handshake(SSL_CONNECTION *s, ossl_unused WORK_STATE wst,
     return WORK_FINISHED_STOP;
 }
 
+/*
+ * TLS 1.3 reserves handshake message type 0, so a HelloRequest must reach the
+ * state machine and be rejected there whenever TLS 1.3 is still possible.
+ *
+ * By the time a client reads a server handshake message, s->version is either
+ * the configured maximum for an initial pre-ServerHello handshake, or the
+ * already negotiated version after ServerHello or during renegotiation. Skip
+ * only when that version is below TLS 1.3.
+ */
+static int should_skip_hello_request(const SSL_CONNECTION *s)
+{
+    if (SSL_CONNECTION_IS_TLS13(s))
+        return 0;
+
+    return s->version > 0 && s->version < TLS1_3_VERSION;
+}
+
 int tls_get_message_header(SSL_CONNECTION *s, int *mt)
 {
     /* s->init_num < SSL3_HM_HEADER_LENGTH */
@@ -1594,7 +1611,8 @@ int tls_get_message_header(SSL_CONNECTION *s, int *mt)
         skip_message = 0;
         if (!s->server)
             if (s->statem.hand_state != TLS_ST_OK
-                && p[0] == SSL3_MT_HELLO_REQUEST)
+                && p[0] == SSL3_MT_HELLO_REQUEST
+                && should_skip_hello_request(s))
                 /*
                  * The server may always send 'Hello Request' messages --
                  * we are doing a handshake anyway now, so ignore them if

diff --git a/test/recipes/70-test_tls13messages.t b/test/recipes/70-test_tls13messages.t
index f3a3f4789f9c06b33482ad272d22fa896ba15fc1..b2e356763c0ab095929268b5cd944eee110fb57d 100644 (file)
--- a/test/recipes/70-test_tls13messages.t
+++ b/test/recipes/70-test_tls13messages.t
@@ -211,6 +211,9 @@ my $proxy = TLSProxy::Proxy->new(
     (!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE}),
     have_IPv6()
 );
+my $fatal_alert = 0;
+my $hello_request_added = 0;
+my $hello_request_after_server_hello = 0;
 
 #Test 1: Check we get all the right messages for a default handshake
 (undef, my $session) = tempfile();
@@ -219,7 +222,7 @@ $proxy->cipherc("DEFAULT:\@SECLEVEL=2");
 $proxy->clientflags("-no_rx_cert_comp -sess_out ".$session);
 $proxy->sessionfile($session);
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 17;
+plan tests => 19;
 checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
                checkhandshake::DEFAULT_EXTENSIONS,
                "Default handshake test");
@@ -419,4 +422,90 @@ checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
                | checkhandshake::SUPPORTED_GROUPS_SRV_EXTENSION,
                "Acceptable but non preferred key_share");
 
+#Test 18: HelloRequest is reserved in TLSv1.3
+$proxy->clear();
+$fatal_alert = 0;
+$hello_request_added = 0;
+$hello_request_after_server_hello = 0;
+$proxy->filter(\&inject_hello_request);
+$proxy->cipherc("DEFAULT:\@SECLEVEL=2");
+$proxy->clientflags("-no_rx_cert_comp");
+$proxy->start();
+ok($fatal_alert, "HelloRequest rejected in TLSv1.3");
+
+#Test 19: A HelloRequest received after selecting TLSv1.2 in the initial
+#         handshake is still ignored, confirming the legacy skip path is
+#         preserved even when TLSv1.3 was initially enabled.
+SKIP: {
+    skip "TLSv1.2 disabled", 1 if disabled("tls1_2");
+
+    $proxy->clear();
+    $fatal_alert = 0;
+    $hello_request_added = 0;
+    $hello_request_after_server_hello = 1;
+    $proxy->filter(\&inject_hello_request);
+    $proxy->cipherc("DEFAULT:\@SECLEVEL=2");
+    $proxy->clientflags("-no_rx_cert_comp");
+    $proxy->serverflags("-no_tls1_3");
+    $proxy->start();
+    ok(TLSProxy::Message->success() && !$fatal_alert,
+       "HelloRequest ignored in TLSv1.2");
+}
+
 unlink $session;
+
+sub inject_hello_request
+{
+    my $proxy = shift;
+    my $records = $proxy->record_list;
+    my $hello_request;
+    my $record;
+    my $server_hello_record;
+    my $i;
+
+    if ($hello_request_added) {
+        $fatal_alert = 1
+            if @{$records}[-1]->is_fatal_alert(0)
+               == TLSProxy::Message::AL_DESC_UNEXPECTED_MESSAGE;
+        return;
+    }
+
+    return if $proxy->flight != 1;
+
+    $hello_request = pack("C4", TLSProxy::Message::MT_HELLO_REQUEST,
+                          0, 0, 0);
+    $record = TLSProxy::Record->new(
+        1,
+        TLSProxy::Record::RT_HANDSHAKE,
+        TLSProxy::Record::VERS_TLS_1_2,
+        length($hello_request),
+        length($hello_request),
+        length($hello_request),
+        $hello_request,
+        $hello_request
+    );
+
+    if ($hello_request_after_server_hello) {
+        foreach my $message (@{$proxy->message_list}) {
+            next if $message->mt != TLSProxy::Message::MT_SERVER_HELLO
+                    || ${$message->records}[0]->flight != 1;
+
+            $server_hello_record = @{$message->records}[-1];
+            last;
+        }
+
+        return if !defined $server_hello_record;
+
+        for ($i = 0; $i < @{$records}; $i++) {
+            last if ${$records}[$i] == $server_hello_record;
+        }
+        $i++;
+    } else {
+        for ($i = 0; ${$records}[$i]->flight() < 1; $i++) {
+            next;
+        }
+    }
+
+    splice @{$records}, $i, 0, $record;
+    $hello_request_added = 1;
+}