Commit 981f976bb0 for openssl.org
commit 981f976bb072e928a869c630af55506514d3d62b
Author: Martin Stecher <martin@cloudfellows.de>
Date: Sun Oct 18 16:16:11 2026 +0200
Fix ECH client with custom extensions after HelloRetryRequest
OpenSSL intentionally preserves custom extension state from an ECH
inner ClientHello to its corresponding outer ClientHello. However,
tls_construct_extensions() also skipped custom_ext_init() when
constructing a second inner ClientHello after a HelloRetryRequest.
This left SSL_EXT_FLAG_SENT set from the first ClientHello, causing
custom_ext_add() to fail with an internal error while constructing
the second one. It also breaks ECH over QUIC whenever the server sends
an HRR, both for OpenSSL's own QUIC implementation and for external
QUIC stacks using the QUIC TLS API, because QUIC transport parameters
are registered as a custom extension.
Fix it by resetting the custom extension flags at the start of each
inner ClientHello while retaining them for the corresponding outer
ClientHello. Added regression tests that exercise ECH, a custom
extension, and HRR both with plain TLS and with OpenSSL's QUIC client.
Resolves: https://github.com/openssl/openssl/issues/33231
Complements: 96c76dc72bb2 "curl ECH+QUIC fix"
Assisted-by: Claude:claude-opus-5-5
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
Reviewed-by: Mounir Idrassi <mounir.idrassi@idrix.fr>
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Merge-date: Fri Nov 13 07:00:21 2026
Merged-from: https://github.com/openssl/openssl/pull/33246
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index be2d52bb08..87779b3eb3 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1187,7 +1187,14 @@ int tls_construct_extensions(SSL_CONNECTION *s, WPACKET *pkt,
/* Add custom extensions first */
#ifndef OPENSSL_NO_ECH
- if ((context & SSL_EXT_CLIENT_HELLO) != 0 && s->ext.ech.attempted == 0)
+ /*
+ * With ECH, the outer ClientHello reuses the custom extensions added
+ * to the inner one, so only reset their flags for the inner. That has
+ * to happen for every inner, as after a HelloRetryRequest the flags
+ * would still be set from the first ClientHello.
+ */
+ if ((context & SSL_EXT_CLIENT_HELLO) != 0
+ && (s->ext.ech.attempted == 0 || s->ext.ech.ch_depth == 1))
/* On the server side with initialise during ClientHello parsing */
custom_ext_init(&s->cert->custext);
#else
diff --git a/test/ech_test.c b/test/ech_test.c
index ad38ae2a7f..adbea656d0 100644
--- a/test/ech_test.c
+++ b/test/ech_test.c
@@ -25,6 +25,7 @@ static char *cert = NULL;
static char *privkey = NULL;
static char *rootcert = NULL;
static int ch_test_cb_ok = 0;
+static int client_ext1_adds = 0;
/* ECH callback */
static unsigned int ech_test_cb(SSL *s, const char *str)
@@ -119,6 +120,8 @@ static int new_add_cb(SSL *s, unsigned int ext_type, unsigned int context,
if (*server != SSL_is_server(s))
return -1;
if (ext_type == TEST_EXT_TYPE1) {
+ if (!SSL_is_server(s))
+ client_ext1_adds++;
if ((data = OPENSSL_malloc(sizeof(*data))) == NULL)
return -1;
*data = 1;
@@ -1278,6 +1281,7 @@ end:
#define OSSL_ECH_TEST_CBS 5 /* test callbacks */
#define OSSL_ECH_TEST_V12 6 /* test TLSv1.2 */
#define OSSL_ECH_TEST_NO_INNER 7 /* test no inner SNI */
+#define OSSL_ECH_TEST_CUSTOM_HRR 8 /* custom CH ext + HRR */
/* note: early-data is prohibited after HRR so no tests for that */
/*
@@ -1354,7 +1358,7 @@ static int test_ech_roundtrip_helper(int idx, int combo)
SSL3_RT_MAX_PLAIN_LENGTH)))
goto end;
}
- if (combo == OSSL_ECH_TEST_CUSTOM) {
+ if (combo == OSSL_ECH_TEST_CUSTOM || combo == OSSL_ECH_TEST_CUSTOM_HRR) {
context = SSL_EXT_CLIENT_HELLO; /* add custom CH ext to client/server */
if (!TEST_true(SSL_CTX_add_custom_ext(cctx, TEST_EXT_TYPE1, context,
new_add_cb, new_free_cb,
@@ -1383,7 +1387,7 @@ static int test_ech_roundtrip_helper(int idx, int combo)
|| !TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
&clientssl, NULL, NULL)))
goto end;
- if (combo == OSSL_ECH_TEST_HRR
+ if ((combo == OSSL_ECH_TEST_HRR || combo == OSSL_ECH_TEST_CUSTOM_HRR)
&& !TEST_true(SSL_set1_groups_list(serverssl, "P-384")))
goto end;
if (combo != OSSL_ECH_TEST_NO_INNER
@@ -1422,10 +1426,15 @@ static int test_ech_roundtrip_helper(int idx, int combo)
goto end;
if (combo == OSSL_ECH_TEST_CBS && !TEST_int_eq(ch_test_cb_ok, 1))
goto end;
+ /* client must have added TEST_EXT_TYPE1 to both inner ClientHellos */
+ if (combo == OSSL_ECH_TEST_CUSTOM_HRR
+ && !TEST_int_eq(client_ext1_adds, 2))
+ goto end;
/* all good */
if (combo == OSSL_ECH_TEST_BASIC || combo == OSSL_ECH_TEST_HRR
|| combo == OSSL_ECH_TEST_CUSTOM || combo == OSSL_ECH_TEST_CBS
- || combo == OSSL_ECH_TEST_NO_INNER) {
+ || combo == OSSL_ECH_TEST_NO_INNER
+ || combo == OSSL_ECH_TEST_CUSTOM_HRR) {
res = 1;
goto end;
}
@@ -1499,6 +1508,7 @@ end:
SSL_CTX_free(cctx);
SSL_CTX_free(sctx);
ch_test_cb_ok = 0;
+ client_ext1_adds = 0;
return res;
}
@@ -1542,6 +1552,14 @@ static int ech_custom_test(int idx)
return test_ech_roundtrip_helper(idx, OSSL_ECH_TEST_CUSTOM);
}
+/* Test a roundtrip with ECH, custom CH extensions and HRR */
+static int ech_custom_hrr_test(int idx)
+{
+ if (verbose)
+ TEST_info("Doing: ech_custom_hrr_test");
+ return test_ech_roundtrip_helper(idx, OSSL_ECH_TEST_CUSTOM_HRR);
+}
+
/* Test a roundtrip with No ECH, and early data */
static int ech_enoe_test(int idx)
{
@@ -2715,6 +2733,7 @@ int setup_tests(void)
ADD_ALL_TESTS(test_ech_hrr, suite_combos);
ADD_ALL_TESTS(test_ech_early, suite_combos);
ADD_ALL_TESTS(ech_custom_test, suite_combos);
+ ADD_ALL_TESTS(ech_custom_hrr_test, suite_combos);
ADD_ALL_TESTS(ech_enoe_test, suite_combos);
ADD_ALL_TESTS(ech_cb_test, suite_combos);
ADD_ALL_TESTS(ech_v12_test, suite_combos);
diff --git a/test/quicapitest.c b/test/quicapitest.c
index 535b7a107c..74e6497907 100644
--- a/test/quicapitest.c
+++ b/test/quicapitest.c
@@ -5316,8 +5316,12 @@ err:
return ret;
}
-/* Test ECH with quic */
-static int test_ech(void)
+/*
+ * Test ECH with quic. QUIC carries its transport parameters as a custom
+ * extension, so idx 1 also forces a HelloRetryRequest to check that the
+ * custom extension is added again to the second inner ClientHello.
+ */
+static int test_ech(int idx)
{
/*
* Don't try this test if various ECC things are set of unavailable
@@ -5372,6 +5376,13 @@ static int test_ech(void)
if (!TEST_ptr(sctx) || !TEST_true(SSL_CTX_set1_echstore(sctx, es)))
goto err;
+ /* client key share is X25519 only, server accepts P-256 only: HRR */
+ if (idx == 1
+ && (!TEST_ptr(cctx)
+ || !TEST_true(SSL_CTX_set1_groups_list(cctx, "X25519:P-256"))
+ || !TEST_true(SSL_CTX_set1_groups_list(sctx, "P-256"))))
+ goto err;
+
if (!TEST_ptr(cctx)
|| !TEST_true(qtest_create_quic_objects(libctx, cctx, sctx, cert,
privkey,
@@ -5743,7 +5754,7 @@ int setup_tests(void)
#endif
ADD_TEST(test_quic_peer_addr_v4);
ADD_MFAIL_NO_CHECK_TEST(test_quic_handshake_multipkt_mfail);
- ADD_TEST(test_ech);
+ ADD_ALL_TESTS(test_ech, 2);
ADD_TEST(test_quic_resize_txe);
ADD_MFAIL_TEST(test_ssl_new_mfail);
ADD_TEST(test_pending_limit);