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 9928d1a2844d410814315da664247b1c38bb8f01..9edf349de1c733109e0b962574037eca28cbadb2 100644 (file)
--- 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,8 +292,9 @@ 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) { \
+#  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) \

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 20b29f999ac1ff5f0168c76a5b0511e09a33f32a..ba78bcedeabc7627d65b65525c0ec95c9f22a2ad 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1178,6 +1178,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) {
@@ -2121,7 +2122,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 f777c22ff3f56eb2b95e9fa332314235eafb8319..a77943ace01ace4a5ea2aed3b828128270b912f6 100644 (file)
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -40,7 +40,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)
@@ -55,7 +55,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 */
@@ -124,7 +124,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. */
@@ -153,7 +153,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)
@@ -234,7 +234,7 @@ EXT_RETURN tls_construct_ctos_ec_pt_formats(SSL_CONNECTION *s, WPACKET *pkt,
     if (!use_ecc(s, min_version, max_version))
         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 */
@@ -274,7 +274,7 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL_CONNECTION *s, WPACKET *pkt,
             && (SSL_CONNECTION_IS_DTLS(s) || 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
 
     /*
@@ -333,7 +333,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
@@ -393,7 +393,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);
@@ -426,7 +426,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)
@@ -489,7 +489,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
 
     /*
@@ -566,7 +566,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)
@@ -607,7 +607,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)
@@ -631,7 +631,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)
@@ -651,7 +651,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)
@@ -682,7 +682,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)
@@ -717,7 +717,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)
@@ -812,7 +812,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 */
@@ -909,7 +909,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)
@@ -1140,7 +1140,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
 
     /*
@@ -1514,7 +1514,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 */
@@ -2411,7 +2411,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)
@@ -2466,7 +2466,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)