From: sftcd Date: Mon, 15 Sep 2025 20:10:33 +0000 (+0100) Subject: Fix a client-auth bug introduced by ECH code X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3daefc47907d5a7b0064af59d2ae8a23043dc9f9;p=thirdparty%2Fopenssl.git Fix a client-auth bug introduced by ECH code Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/28555) --- diff --git a/ssl/ech/ech_local.h b/ssl/ech/ech_local.h index 9928d1a2844..9edf349de1c 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,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 20b29f999ac..ba78bcedeab 100644 --- 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 f777c22ff3f..a77943ace01 100644 --- 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)