Commit a732ff7aa2 for openssl.org
commit a732ff7aa20c965c21a3d6c5ee74c47aaf04dde5
Author: sftcd <stephen.farrell@cs.tcd.ie>
Date: Mon Sep 15 21:10:33 2025 +0100
Fix a client-auth bug introduced by ECH code
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/28555)
diff --git a/ssl/ech/ech_local.h b/ssl/ech/ech_local.h
index 047aa8873b..c82869fa0e 100644
--- a/ssl/ech/ech_local.h
+++ b/ssl/ech/ech_local.h
@@ -282,7 +282,9 @@ typedef struct ossl_ech_conn_st {
* be the same as in the inner.
*
* This macro should be called in each _ctos_ function that doesn't explicitly
- * have special ECH handling.
+ * have special ECH handling. There are some _ctos_ functions that are called
+ * from a server, but we don't want to do anything in such cases. We also
+ * screen out cases where the context is not handling the ClientHello.
*
* Note that the placement of this macro needs a bit of thought - it has to go
* after declarations (to keep the ansi-c compile happy) and also after any
@@ -290,15 +292,16 @@ typedef struct ossl_ech_conn_st {
* state changes that would affect a possible 2nd call to the constructor.
* Luckily, that's usually not too hard, but it's not mechanical.
*/
-#define ECH_SAME_EXT(s, pkt) \
- if (s->ext.ech.es != NULL && s->ext.ech.grease == 0) { \
- int ech_iosame_rv = ossl_ech_same_ext(s, pkt); \
- \
- if (ech_iosame_rv == OSSL_ECH_SAME_EXT_ERR) \
- return EXT_RETURN_FAIL; \
- if (ech_iosame_rv == OSSL_ECH_SAME_EXT_DONE) \
- return EXT_RETURN_SENT; \
- /* otherwise continue as normal */ \
+#define ECH_SAME_EXT(s, context, pkt) \
+ if (context == SSL_EXT_CLIENT_HELLO && !s->server \
+ && s->ext.ech.es != NULL && s->ext.ech.grease == 0) { \
+ int ech_iosame_rv = ossl_ech_same_ext(s, pkt); \
+ \
+ if (ech_iosame_rv == OSSL_ECH_SAME_EXT_ERR) \
+ return EXT_RETURN_FAIL; \
+ if (ech_iosame_rv == OSSL_ECH_SAME_EXT_DONE) \
+ return EXT_RETURN_SENT; \
+ /* otherwise continue as normal */ \
}
/* Internal ECH APIs */
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 153fecb224..0b596e98de 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1120,6 +1120,7 @@ int tls_construct_extensions(SSL_CONNECTION *s, WPACKET *pkt,
* the real ECH extension value
*/
if (s->server
+ || context != SSL_EXT_CLIENT_HELLO
|| s->ext.ech.attempted == 0
|| s->ext.ech.ch_depth == 1
|| s->ext.ech.grease == OSSL_ECH_IS_GREASE) {
@@ -2060,7 +2061,7 @@ static EXT_RETURN tls_construct_compress_certificate(SSL_CONNECTION *sc, WPACKET
if (sc->cert_comp_prefs[0] == TLSEXT_comp_cert_none)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(sc, pkt);
+ ECH_SAME_EXT(sc, context, pkt);
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_compress_certificate)
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 23eac642b6..1be65b600c 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -48,7 +48,7 @@ EXT_RETURN tls_construct_ctos_renegotiate(SSL_CONNECTION *s, WPACKET *pkt,
}
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_renegotiate)
@@ -63,7 +63,7 @@ EXT_RETURN tls_construct_ctos_renegotiate(SSL_CONNECTION *s, WPACKET *pkt,
}
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
/* Add a complete RI extension if renegotiating */
@@ -132,7 +132,7 @@ EXT_RETURN tls_construct_ctos_maxfragmentlen(SSL_CONNECTION *s, WPACKET *pkt,
if (s->ext.max_fragment_len_mode == TLSEXT_max_fragment_length_DISABLED)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
/* Add Max Fragment Length extension if client enabled it. */
@@ -161,7 +161,7 @@ EXT_RETURN tls_construct_ctos_srp(SSL_CONNECTION *s, WPACKET *pkt,
if (s->srp_ctx.login == NULL)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_srp)
@@ -277,7 +277,7 @@ EXT_RETURN tls_construct_ctos_ec_pt_formats(SSL_CONNECTION *s, WPACKET *pkt,
if (num_formats == 0)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
/* Add TLS extension ECPointFormats to the ClientHello message */
@@ -319,7 +319,7 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL_CONNECTION *s, WPACKET *pkt,
: (max_version <= TLS1_2_VERSION)))
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
/*
@@ -384,7 +384,7 @@ EXT_RETURN tls_construct_ctos_session_ticket(SSL_CONNECTION *s, WPACKET *pkt,
if (!tls_use_ticket(s))
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!s->new_session && s->session != NULL
@@ -443,7 +443,7 @@ EXT_RETURN tls_construct_ctos_sig_algs(SSL_CONNECTION *s, WPACKET *pkt,
}
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
salglen = tls12_get_psigalgs(s, 1, &salg);
@@ -476,7 +476,7 @@ EXT_RETURN tls_construct_ctos_status_request(SSL_CONNECTION *s, WPACKET *pkt,
if (s->ext.status_type != TLSEXT_STATUSTYPE_ocsp)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_status_request)
@@ -539,7 +539,7 @@ EXT_RETURN tls_construct_ctos_npn(SSL_CONNECTION *s, WPACKET *pkt,
|| !SSL_IS_FIRST_HANDSHAKE(s))
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
/*
@@ -615,7 +615,7 @@ EXT_RETURN tls_construct_ctos_use_srtp(SSL_CONNECTION *s, WPACKET *pkt,
if (clnt == NULL)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_use_srtp)
@@ -655,7 +655,7 @@ EXT_RETURN tls_construct_ctos_etm(SSL_CONNECTION *s, WPACKET *pkt,
if (s->options & SSL_OP_NO_ENCRYPT_THEN_MAC)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_encrypt_then_mac)
@@ -679,7 +679,7 @@ EXT_RETURN tls_construct_ctos_sct(SSL_CONNECTION *s, WPACKET *pkt,
if (x != NULL)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_signed_certificate_timestamp)
@@ -699,7 +699,7 @@ EXT_RETURN tls_construct_ctos_ems(SSL_CONNECTION *s, WPACKET *pkt,
if (s->options & SSL_OP_NO_EXTENDED_MASTER_SECRET)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_extended_master_secret)
@@ -730,7 +730,7 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL_CONNECTION *s, WPACKET *pkt
if (max_version < TLS1_3_VERSION)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions)
@@ -765,7 +765,7 @@ EXT_RETURN tls_construct_ctos_psk_kex_modes(SSL_CONNECTION *s, WPACKET *pkt,
int nodhe = s->options & SSL_OP_ALLOW_NO_DHE_KEX;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_psk_kex_modes)
@@ -860,7 +860,7 @@ EXT_RETURN tls_construct_ctos_key_share(SSL_CONNECTION *s, WPACKET *pkt,
size_t valid_keyshare = 0;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
/* key_share extension */
@@ -957,7 +957,7 @@ EXT_RETURN tls_construct_ctos_cookie(SSL_CONNECTION *s, WPACKET *pkt,
if (s->ext.tls13_cookie_len == 0)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_cookie)
@@ -1188,7 +1188,7 @@ EXT_RETURN tls_construct_ctos_padding(SSL_CONNECTION *s, WPACKET *pkt,
if ((s->options & SSL_OP_TLSEXT_PADDING) == 0)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt);
+ ECH_SAME_EXT(s, context, pkt);
#endif
/*
@@ -1564,7 +1564,7 @@ EXT_RETURN tls_construct_ctos_post_handshake_auth(SSL_CONNECTION *s, WPACKET *pk
if (!s->pha_enabled)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(s, pkt)
+ ECH_SAME_EXT(s, context, pkt)
#endif
/* construct extension - 0 length, no contents */
@@ -2461,7 +2461,7 @@ EXT_RETURN tls_construct_ctos_client_cert_type(SSL_CONNECTION *sc, WPACKET *pkt,
if (sc->client_cert_type == NULL)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(sc, pkt)
+ ECH_SAME_EXT(sc, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_client_cert_type)
@@ -2516,7 +2516,7 @@ EXT_RETURN tls_construct_ctos_server_cert_type(SSL_CONNECTION *sc, WPACKET *pkt,
if (sc->server_cert_type == NULL)
return EXT_RETURN_NOT_SENT;
#ifndef OPENSSL_NO_ECH
- ECH_SAME_EXT(sc, pkt)
+ ECH_SAME_EXT(sc, context, pkt)
#endif
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_server_cert_type)