From ded17621f18739c41463918caf96762af3efc20e Mon Sep 17 00:00:00 2001
From: sftcd <stephen.farrell@cs.tcd.ie>
Date: Fri, 4 Jul 2025 20:04:26 +0100
Subject: [PATCH] fixup! ECH both sides now

---
 doc/man3/SSL_CTX_set_client_hello_cb.pod |  2 ++
 include/internal/packet.h                | 17 -----------------
 ssl/ech/ech_internal.c                   | 18 ++++++------------
 ssl/statem/extensions_srvr.c             |  2 +-
 ssl/statem/statem_srvr.c                 |  3 ---
 5 files changed, 9 insertions(+), 33 deletions(-)

diff --git a/doc/man3/SSL_CTX_set_client_hello_cb.pod b/doc/man3/SSL_CTX_set_client_hello_cb.pod
index 74468ab8ac..2cf5c4e573 100644
--- a/doc/man3/SSL_CTX_set_client_hello_cb.pod
+++ b/doc/man3/SSL_CTX_set_client_hello_cb.pod
@@ -104,6 +104,8 @@ resumption and the historical servername callback.
 The SSL_client_hello_* family of functions may only be called from code executing
 within a ClientHello callback.
 
+TODO(ECH): How ECH is handled here needs to be documented.
+
 =head1 RETURN VALUES
 
 The application's supplied ClientHello callback returns
diff --git a/include/internal/packet.h b/include/internal/packet.h
index 9a959e1b11..fc09c3f176 100644
--- a/include/internal/packet.h
+++ b/include/internal/packet.h
@@ -647,23 +647,6 @@ __owur static ossl_inline int PACKET_get_length_prefixed_3(PACKET *pkt,
     return 1;
 }
 
-# ifndef OPENSSL_NO_ECH
-/*
- * New ECH API allowing replacement of outer client hello on server
- * with inner. If newpkt is bigger, the caller has to fix that first
- * (because pkt may be a subpacket so we don't here have a pointer to
- * the allocated buffer)
- */
-__owur static ossl_inline int PACKET_replace(PACKET *pkt, const PACKET *newpkt)
-{
-    if (pkt == NULL || newpkt == NULL)
-        return 0;
-    memcpy((unsigned char *)pkt->curr, newpkt->curr, newpkt->remaining);
-    pkt->remaining = newpkt->remaining;
-    return 1;
-}
-# endif
-
 /* Writable packets */
 
 typedef struct wpacket_sub WPACKET_SUB;
diff --git a/ssl/ech/ech_internal.c b/ssl/ech/ech_internal.c
index a4526758b0..97d9c0c566 100644
--- a/ssl/ech/ech_internal.c
+++ b/ssl/ech/ech_internal.c
@@ -1219,7 +1219,7 @@ static int ech_get_outer_sni(SSL_CONNECTION *s, char **osni_str,
 /*
  * decode EncryptedClientHello extension value
  * pkt contains the ECH value as a PACKET
- * extval is the returned decoded structure
+ * retext is the returned decoded structure
  * payload_offset is the offset to the ciphertext
  * return 1 for good, 0 for bad
  *
@@ -1320,7 +1320,7 @@ static int ech_decode_inbound_ech(SSL_CONNECTION *s, PACKET *pkt,
             SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
             goto err;
         }
-        if (memcmp(tmpenc, s->ext.ech.pub, pval_tmp)) {
+        if (memcmp(tmpenc, s->ext.ech.pub, pval_tmp) != 0) {
             OPENSSL_free(tmpenc);
             SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
             goto err;
@@ -1448,7 +1448,7 @@ static int ech_find_outers(SSL_CONNECTION *s, PACKET *pkt,
         return 1;
     }
     /*
-     * outers has a silly internal length as well and that betterk
+     * outers has a silly internal length as well and that better
      * be one less than the extension length and an even number
      * and we only support a certain max of outers
      */
@@ -1779,7 +1779,7 @@ static unsigned char *hpke_decrypt_encch(SSL_CONNECTION *s,
      * We may generate externally visible OpenSSL errors
      * if decryption fails (which is normal) but we'll
      * ignore those as we might be dealing with a GREASEd
-     * ECH. To do that we need to know ingore some errors
+     * ECH. To do that we need to now ingore some errors
      * so we use ERR_set_mark() then later ERR_pop_to_mark().
      */
     if (ERR_set_mark() != 0) {
@@ -1799,6 +1799,7 @@ static unsigned char *hpke_decrypt_encch(SSL_CONNECTION *s,
         rv = OSSL_HPKE_CTX_set_seq(hctx, 1);
         if (rv != 1) {
             /* don't clear this error - GREASE can't cause it */
+            ERR_clear_last_mark();
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
             goto end;
         }
@@ -1871,24 +1872,17 @@ end:
 # ifdef CHECKZEROS
         {
             size_t zind = 0;
-            size_t zeros = 0;
 
             if (*innerlen < ch_len) {
                 OPENSSL_free(clear);
                 return NULL;
             }
             for (zind = ch_len; zind != *innerlen; zind++) {
-                if (clear[zind] == 0x00) {
-                    zeros++;
-                } else {
+                if (clear[zind] != 0x00) {
                     OPENSSL_free(clear);
                     return NULL;
                 }
             }
-            if (zeros != (*innerlen - ch_len)) {
-                OPENSSL_free(clear);
-                return NULL;
-            }
         }
 # endif
         *innerlen = ch_len;
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 2804447b36..18589468e4 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -2477,7 +2477,7 @@ int tls_parse_ctos_ech(SSL_CONNECTION *s, PACKET *pkt, unsigned int context,
 }
 
 /*
- * answer a draft-13 ECH, as needed
+ * Answer an ECH, as needed
  * return 1 for good, 0 otherwise
  *
  * Return most-recent ECH config for retry, as needed.
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 947131c41b..0aadcff5f4 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -2091,9 +2091,6 @@ static int tls_early_post_process_client_hello(SSL_CONNECTION *s)
             && !SSL_CONNECTION_IS_DTLS(s)
             && s->ext.session_secret_cb != NULL)) {
         const SSL_CIPHER *pref_cipher = NULL;
-#elif 0
-        /* silence the style checker */
-    }
 #else
 
     if (!s->hit