Commit 3e4f7e7720 for openssl.org
commit 3e4f7e77208138174ce50f0403ce6b947cb1d509
Author: Mounir IDRASSI <mounir.idrassi@idrix.fr>
Date: Tue May 5 19:30:49 2026 +0900
Reject unknown TLS 1.3 ServerHello extensions
TLS 1.3 does not permit clients to ignore unknown extensions in ServerHello
or HelloRetryRequest. Add a validation pass for these messages so fully
unknown extension types fail early with unsupported_extension.
Keep the existing ignore behavior for other extension contexts and leave
TLS 1.2 ServerHello processing unchanged. Add TLSProxy coverage for the
TLS 1.3 ServerHello case.
Correct the unknown-extension path in tls_collect_extensions(). Collection
still runs before the new TLS 1.3 validation, and verify_extension()
returns success with thisex == NULL for fully unknown extension types.
The previous idx calculation used invalid pointer arithmetic on NULL in
that path. Map unknown extensions explicitly outside the built-in extension
range instead, preserving the existing ignore semantics while avoiding
undefined behavior.
Update the truncated HRR ECH corruption vector to keep testing malformed
ECH length after the new unknown-extension validation. The old 0xdddd bytes
were only padding to preserve the replacement length, but they now trigger
the unknown-extension check before the ECH parser. Use a known HRR cookie
extension as padding so the test still reaches the intended
SSL_R_LENGTH_MISMATCH path.
Reviewed-by: Andrew Dinh <andrewd@openssl.org>
Reviewed-by: Daniel Kubec <kubec@openssl.foundation>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
MergeDate: Thu Jul 23 15:55:09 2026
(Merged from https://github.com/openssl/openssl/pull/31086)
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index c3ef683d75..753402b84f 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -798,6 +798,62 @@ static int tls_parse_ec_pt_formats(SSL_CONNECTION *s, PACKET *pkt,
return 1;
}
+/*
+ * Verify that all extensions in |packet| are known built-in or custom
+ * extension types. This is used for TLS 1.3 server extension responses where
+ * unknown extensions are not ignored.
+ */
+int tls_validate_no_unknown_extensions(SSL_CONNECTION *s, PACKET *packet,
+ unsigned int context)
+{
+ PACKET extensions = *packet;
+ custom_ext_methods *exts = &s->cert->custext;
+ ENDPOINT role = ENDPOINT_BOTH;
+
+ if ((context & SSL_EXT_CLIENT_HELLO) != 0) {
+#ifndef OPENSSL_NO_ECH
+ if (s->ext.ech.attempted == 1 && s->ext.ech.ch_depth == 1)
+ role = ENDPOINT_CLIENT;
+ else
+ role = ENDPOINT_SERVER;
+#else
+ role = ENDPOINT_SERVER;
+#endif
+ } else if ((context & SSL_EXT_TLS1_2_SERVER_HELLO) != 0) {
+ role = ENDPOINT_CLIENT;
+ }
+
+ while (PACKET_remaining(&extensions) > 0) {
+ unsigned int type;
+ size_t i;
+ PACKET extension;
+ const EXTENSION_DEFINITION *thisext;
+
+ if (!PACKET_get_net_2(&extensions, &type)
+ || !PACKET_get_length_prefixed_2(&extensions, &extension)) {
+ SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ return 0;
+ }
+
+ for (i = 0, thisext = ext_defs; i < OSSL_NELEM(ext_defs);
+ i++, thisext++) {
+ if (type == thisext->type)
+ break;
+ }
+ if (i < OSSL_NELEM(ext_defs))
+ continue;
+
+ if (exts != NULL && custom_ext_find(exts, role, type, NULL) != NULL)
+ continue;
+
+ SSLfatal(s, SSL_AD_UNSUPPORTED_EXTENSION,
+ SSL_R_UNSOLICITED_EXTENSION);
+ return 0;
+ }
+
+ return 1;
+}
+
/*
* Check whether the context defined for an extension |extctx| means whether
* the extension is relevant for the current context |thisctx| or not. Returns
@@ -907,7 +963,6 @@ int tls_collect_extensions(SSL_CONNECTION *s, PACKET *packet,
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
-
/* The server must tolerate the unknown extension and complete. */
if (thisex == NULL)
continue;
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 15ed8ef8d5..d719ef8a84 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1799,6 +1799,8 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)
if (hrr) {
if (!tls_collect_extensions(s, &extpkt, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST,
&extensions, NULL, 1)
+ || !tls_validate_no_unknown_extensions(s, &extpkt,
+ SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST)
|| !tls_parse_extension(s, TLSEXT_IDX_ech,
SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST,
extensions, NULL, 0)) {
@@ -1959,6 +1961,10 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
+ if (SSL_CONNECTION_IS_TLS13(s)
+ && !tls_validate_no_unknown_extensions(s, &extpkt, context))
+ /* SSLfatal() already called */
+ goto err;
s->hit = 0;
diff --git a/ssl/statem/statem_local.h b/ssl/statem/statem_local.h
index b73c1e100f..de9ff299be 100644
--- a/ssl/statem/statem_local.h
+++ b/ssl/statem/statem_local.h
@@ -269,6 +269,8 @@ __owur int tls_validate_all_contexts(SSL_CONNECTION *s, unsigned int thisctx,
RAW_EXTENSION *exts);
__owur int extension_is_relevant(SSL_CONNECTION *s, unsigned int extctx,
unsigned int thisctx);
+__owur int tls_validate_no_unknown_extensions(SSL_CONNECTION *s,
+ PACKET *packet, unsigned int context);
__owur int tls_collect_extensions(SSL_CONNECTION *s, PACKET *packet,
unsigned int context,
RAW_EXTENSION **res, size_t *len, int init);
diff --git a/test/ech_corrupt_test.c b/test/ech_corrupt_test.c
index 7070f4ef37..79d59844fd 100644
--- a/test/ech_corrupt_test.c
+++ b/test/ech_corrupt_test.c
@@ -710,10 +710,10 @@ typedef struct {
#define OSSL_ECH_BORK_GREASE (1 << 4)
#define OSSL_ECH_BORK_REPLACE (1 << 5)
-/* a truncated ECH, with another bogus ext to match overall length */
+/* a truncated ECH, padded with a known HRR ext to match overall length */
static unsigned char shortech[] = {
0xfe, 0x0d, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00,
- 0xdd, 0xdd, 0x00, 0x00
+ 0x00, 0x2c, 0x00, 0x00
};
/* a too-long ECH internal length */
diff --git a/test/recipes/70-test_sslextension.t b/test/recipes/70-test_sslextension.t
index 3327df474e..89b241efee 100644
--- a/test/recipes/70-test_sslextension.t
+++ b/test/recipes/70-test_sslextension.t
@@ -38,7 +38,8 @@ use constant {
UNSOLICITED_SERVER_NAME => 0,
UNSOLICITED_SERVER_NAME_TLS13 => 1,
UNSOLICITED_SCT => 2,
- NONCOMPLIANT_SUPPORTED_GROUPS => 3
+ NONCOMPLIANT_SUPPORTED_GROUPS => 3,
+ UNKNOWN_SERVER_HELLO_TLS13 => 4
};
my $testtype;
@@ -146,7 +147,8 @@ sub inject_unsolicited_extension
if ($proxy->flight != 1) {
if ($sent_unsolisited_extension) {
my $last_record = @{$proxy->record_list}[-1];
- $fatal_alert = 1 if $last_record->is_fatal_alert(0);
+ my $alert = $last_record->is_fatal_alert(0);
+ $fatal_alert = $alert if $alert;
}
return;
}
@@ -172,6 +174,8 @@ sub inject_unsolicited_extension
$type = TLSProxy::Message::EXT_SCT;
} elsif ($testtype == NONCOMPLIANT_SUPPORTED_GROUPS) {
$type = TLSProxy::Message::EXT_SUPPORTED_GROUPS;
+ } elsif ($testtype == UNKNOWN_SERVER_HELLO_TLS13) {
+ $type = TLSProxy::Message::EXT_UNKNOWN;
}
$message->set_extension($type, $ext);
$message->repack();
@@ -194,7 +198,7 @@ sub inject_cryptopro_extension
# Test 1-2: Sending a duplicate extension should fail.
$proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 8;
+plan tests => 9;
ok($fatal_alert, "Duplicate ClientHello extension");
SKIP: {
@@ -261,7 +265,7 @@ SKIP: {
}
SKIP: {
- skip "TLS 1.3 disabled", 1
+ skip "TLS 1.3 disabled", 2
if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
#Test 8: Inject an unsolicited extension (TLSv1.3)
$fatal_alert = 0;
@@ -271,4 +275,14 @@ SKIP: {
$proxy->clientflags("-noservername");
$proxy->start();
ok($fatal_alert, "Unsolicited server name extension (TLSv1.3)");
+
+ #Test 9: Inject an unknown extension in ServerHello (TLSv1.3)
+ $fatal_alert = 0;
+ $proxy->clear();
+ $proxy->filter(\&inject_unsolicited_extension);
+ $testtype = UNKNOWN_SERVER_HELLO_TLS13;
+ $proxy->clientflags("");
+ $proxy->start();
+ ok($fatal_alert == TLSProxy::Message::AL_DESC_UNSUPPORTED_EXTENSION,
+ "Unknown ServerHello extension (TLSv1.3)");
}
diff --git a/util/perl/TLSProxy/Message.pm b/util/perl/TLSProxy/Message.pm
index 4c1ba50de4..8c4f9564fa 100644
--- a/util/perl/TLSProxy/Message.pm
+++ b/util/perl/TLSProxy/Message.pm
@@ -51,7 +51,8 @@ use constant {
AL_DESC_DECRYPT_ERROR => 51,
AL_DESC_PROTOCOL_VERSION => 70,
AL_DESC_NO_RENEGOTIATION => 100,
- AL_DESC_MISSING_EXTENSION => 109
+ AL_DESC_MISSING_EXTENSION => 109,
+ AL_DESC_UNSUPPORTED_EXTENSION => 110
};
my %message_type = (