ECH: Conformance test changes in response to AISLE review

Reviewed-by: Matt Caswell <matt@openssl.foundation>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
MergeDate: Wed Apr  8 08:59:19 2026
(Merged from https://github.com/openssl/openssl/pull/30419)

doc/man3/SSL_set1_echstore.pod

@@ -327,7 +327,7 @@ Return codes from SSL_ech_get_status
 
 =item B<SSL_ECH_STATUS_NOT_TRIED>  -101, ECH wasn't attempted
 
-=item B<SSL_ECH_STATUS_BAD_NAME>   -102, ECH ok but server cert bad
+=item B<SSL_ECH_STATUS_BAD_NAME>   -102, ECH ok but server or client cert bad
 
 =item B<SSL_ECH_STATUS_NOT_CONFIGURED> -103, ECH wasn't configured
 

ssl/ech/ech_internal.c

@@ -1054,22 +1054,17 @@ int ossl_ech_calc_confirm(SSL_CONNECTION *s, int for_hrr,
     if (for_hrr == 0) { /* zap magic octets at fixed place for SH */
         conf_loc = tbuf + chend + shoffset;
     } else {
-        if (s->server == 1) { /* we get to say where we put ECH:-) */
-            conf_loc = tbuf + tlen - OSSL_ECH_SIGNAL_LEN;
-        } else {
-            if (s->ext.ech.hrrsignal_p == NULL) {
-                /* No ECH found so we'll exit, but set random output */
-                if (RAND_bytes_ex(sctx->libctx, acbuf,
-                        OSSL_ECH_SIGNAL_LEN, 0)
-                    <= 0) {
-                    SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_ECH_REQUIRED);
-                    goto end;
-                }
-                rv = 1;
+        if (s->server == 0 && s->ext.ech.hrrsignal_p == NULL) {
+            /* No ECH found so we'll exit, but set random output */
+            if (RAND_bytes_ex(sctx->libctx, acbuf, OSSL_ECH_SIGNAL_LEN, 0)
+                <= 0) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_ECH_REQUIRED);
                 goto end;
             }
-            conf_loc = s->ext.ech.hrrsignal_p;
+            rv = 1;
+            goto end;
         }
+        conf_loc = tbuf + tlen - OSSL_ECH_SIGNAL_LEN;
     }
     memset(conf_loc, 0, OSSL_ECH_SIGNAL_LEN);
 #ifdef OSSL_ECH_SUPERVERBOSE
@@ -1096,7 +1091,11 @@ int ossl_ech_calc_confirm(SSL_CONNECTION *s, int for_hrr,
     ossl_ech_pbuf("cx: result", acbuf, OSSL_ECH_SIGNAL_LEN);
 #endif
     /* put confirm value back into transcript */
-    memcpy(conf_loc, acbuf, OSSL_ECH_SIGNAL_LEN);
+    if (s->server == 0 && for_hrr == 1) { /* put back the one we got */
+        memcpy(conf_loc, s->ext.ech.hrrsignal, OSSL_ECH_SIGNAL_LEN);
+    } else {
+        memcpy(conf_loc, acbuf, OSSL_ECH_SIGNAL_LEN);
+    }
     /* on a server, we need to reset the hs buffer now */
     if (s->server && s->hello_retry_request == SSL_HRR_NONE)
         ossl_ech_reset_hs_buffer(s, s->ext.ech.innerch, s->ext.ech.innerch_len);
@@ -1475,9 +1474,11 @@ static int ech_find_outers(SSL_CONNECTION *s, PACKET *pkt,
     }
     *n_outers = olen / 2;
     for (i = 0; i != *n_outers; i++) {
+        /* check for ones that are not allowed */
         if (!PACKET_get_net_2(&op, &pi_tmp)
-            || pi_tmp == TLSEXT_TYPE_outer_extensions) {
-            SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+            || pi_tmp == TLSEXT_TYPE_outer_extensions
+            || pi_tmp == TLSEXT_TYPE_ech) {
+            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
             goto err;
         }
         outers[i] = (uint16_t)pi_tmp;
@@ -1491,25 +1492,20 @@ err:
  * copy one extension from outer to inner
  * di is the reconstituted inner CH
  * type2copy is the outer type to copy
- * extsbuf is the outer extensions buffer
- * extslen is the outer extensions buffer length
+ * exts is the outer extensions packet (changing as we go)
  * return 1 for good 0 for error
  */
 static int ech_copy_ext(SSL_CONNECTION *s, WPACKET *di, uint16_t type2copy,
-    const unsigned char *extsbuf, size_t extslen)
+    PACKET *exts)
 {
-    PACKET exts;
     unsigned int etype, elen;
     const unsigned char *eval;
 
-    if (PACKET_buf_init(&exts, extsbuf, extslen) != 1) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-        goto err;
-    }
-    while (PACKET_remaining(&exts) > 0) {
-        if (!PACKET_get_net_2(&exts, &etype)
-            || !PACKET_get_net_2(&exts, &elen)
-            || !PACKET_get_bytes(&exts, &eval, elen)) {
+    /* Skip until we find the thing to copy */
+    while (PACKET_remaining(exts) > 0) {
+        if (!PACKET_get_net_2(exts, &etype)
+            || !PACKET_get_net_2(exts, &elen)
+            || !PACKET_get_bytes(exts, &eval, elen)) {
             SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
             goto err;
         }
@@ -1547,6 +1543,7 @@ static int ech_reconstitute_inner(SSL_CONNECTION *s, WPACKET *di, PACKET *ei,
     unsigned int pi_tmp, etype, elen, outer_extslen;
     PACKET outer, session_id;
     size_t i;
+    int outers_done = 0;
 
     if (PACKET_buf_init(&outer, ob, ob_len) != 1) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
@@ -1620,10 +1617,18 @@ static int ech_reconstitute_inner(SSL_CONNECTION *s, WPACKET *di, PACKET *ei,
             goto err;
         }
         if (etype == TLSEXT_TYPE_outer_extensions) {
+            PACKET exts;
+
+            if (outers_done++) { /* just do this once */
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+                goto err;
+            }
+            if (PACKET_buf_init(&exts, outer_exts, outer_extslen) != 1) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+                goto err;
+            }
             for (i = 0; i != n_outers; i++) {
-                if (ech_copy_ext(s, di, outers[i],
-                        outer_exts, outer_extslen)
-                    != 1)
+                if (ech_copy_ext(s, di, outers[i], &exts) != 1)
                     /* SSLfatal called already */
                     goto err;
             }
@@ -1739,7 +1744,7 @@ static unsigned char *hpke_decrypt_encch(SSL_CONNECTION *s,
     size_t aad_len, unsigned char *aad,
     int forhrr, size_t *innerlen)
 {
-    size_t cipherlen = 0;
+    size_t cipherlen = 0, zind = 0;
     unsigned char *cipher = NULL;
     size_t senderpublen = 0;
     unsigned char *senderpub = NULL;
@@ -1876,24 +1881,18 @@ end:
             SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
             goto paderr;
         }
-        /*
-         * The RFC calls for that padding to be all zeros. I'm not so
-         * keen on that being a good idea to enforce, so we'll make it
-         * easy to not do so (but check by default)
-         */
-#define CHECKZEROS
-#ifdef CHECKZEROS
-        {
-            size_t zind = 0;
+        /* The RFC calls for that padding to be all zeros */
 
-            if (*innerlen < ch_len)
+        if (*innerlen < ch_len) {
+            SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+            goto paderr;
+        }
+        for (zind = ch_len; zind != *innerlen; zind++) {
+            if (clear[zind] != 0x00) {
+                SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
                 goto paderr;
-            for (zind = ch_len; zind != *innerlen; zind++) {
-                if (clear[zind] != 0x00)
-                    goto paderr;
             }
         }
-#endif
         *innerlen = ch_len;
 #ifdef OSSL_ECH_SUPERVERBOSE
         ossl_ech_pbuf("unpadded clear", clear, *innerlen);
@@ -2034,7 +2033,7 @@ int ossl_ech_early_decrypt(SSL_CONNECTION *s, PACKET *outerpkt, PACKET *newpkt)
     es = s->ext.ech.es;
     num = (es == NULL || es->entries == NULL ? 0
                                              : sk_OSSL_ECHSTORE_ENTRY_num(es->entries));
-    for (cfgind = 0; cfgind != num; cfgind++) {
+    for (cfgind = 0; cfgind != num && foundcfg == 0; cfgind++) {
         ee = sk_OSSL_ECHSTORE_ENTRY_value(es->entries, cfgind);
         OSSL_TRACE_BEGIN(TLS)
         {
@@ -2044,8 +2043,15 @@ int ossl_ech_early_decrypt(SSL_CONNECTION *s, PACKET *outerpkt, PACKET *newpkt)
         }
         OSSL_TRACE_END(TLS);
         if (extval->config_id == ee->config_id) {
-            foundcfg = 1;
-            break;
+            unsigned int suite_id;
+
+            /* check aead and kdf match a loaded suite for the config_id */
+            for (suite_id = 0; suite_id != ee->nsuites && foundcfg == 0; suite_id++) {
+                if (ee->suites[suite_id].kdf_id == extval->kdf_id
+                    && ee->suites[suite_id].aead_id == extval->aead_id) {
+                    foundcfg = 1;
+                }
+            }
         }
     }
     if (foundcfg == 1) {

ssl/ech/ech_local.h

@@ -242,6 +242,7 @@ typedef struct ossl_ech_conn_st {
      * not.
      */
     int retry_configs_ok;
+    int inner_ech_seen_ok; /* set if we see inner ECH as expected */
     int grease; /* 1 if we're GREASEing, 0 otherwise */
     char *grease_suite; /* HPKE suite string for GREASEing */
     unsigned char *sent; /* GREASEy ECH value sent, in case needed for re-tx */

ssl/ech/ech_store.c

@@ -249,8 +249,10 @@ err:
 static int ech_final_config_checks(OSSL_ECHSTORE_ENTRY *ee)
 {
     OSSL_HPKE_SUITE hpke_suite;
-    int ind, num;
-    int goodsuitefound = 0;
+    int ind, num, rv = 0, goodsuitefound = 0;
+    X509_VERIFY_PARAM *vpm = X509_VERIFY_PARAM_new();
+    char *lastlabel = NULL;
+    size_t lllen;
 
     /* check local support for some suite */
     for (ind = 0; ind != (int)ee->nsuites; ind++) {
@@ -267,7 +269,7 @@ static int ech_final_config_checks(OSSL_ECHSTORE_ENTRY *ee)
     }
     if (goodsuitefound == 0) {
         ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);
-        return 0;
+        goto err;
     }
     /* check no mandatory exts (with high bit set in type) */
     num = (ee->exts == NULL ? 0 : sk_OSSL_ECHEXT_num(ee->exts));
@@ -276,16 +278,48 @@ static int ech_final_config_checks(OSSL_ECHSTORE_ENTRY *ee)
 
         if (oe->type & 0x8000) {
             ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);
-            return 0;
+            goto err;
         }
     }
-    /* check public_name rules, as per spec section 4 */
+    /* check public_name rules, as per spec section 6.1.7 */
     if (ee->public_name == NULL
         || ee->public_name[0] == '\0'
         || ee->public_name[0] == '.'
-        || ee->public_name[strlen(ee->public_name) - 1] == '.')
-        return 0;
-    return 1;
+        || ee->public_name[strlen(ee->public_name) - 1] == '.'
+        || strlen(ee->public_name) > 255)
+        goto err;
+    /*
+     * Use X509_VERIFY_PARAM_add1_host to avoid coding same checks twice.
+     * This checks max 63 octets per label, overall length and some other
+     * DNS label checks.
+     */
+    if (X509_VERIFY_PARAM_add1_host(vpm, ee->public_name, 0) == 0)
+        goto err;
+    /*
+     * but we still have to check the last label restrictions, which
+     * are intended to avoid confusion with IP address literals in
+     * encodings browsers support, as per WHATWG (convincing, eh:-)
+     */
+    lastlabel = strrchr(ee->public_name, '.');
+    if (lastlabel == NULL) /* if there are no dots */
+        lastlabel = ee->public_name;
+    lllen = strlen(lastlabel);
+    if (lllen < 2)
+        goto err;
+    if (lastlabel[0] == '.') {
+        lastlabel++;
+        lllen--;
+    }
+    if (strspn(lastlabel, "0123456789") == lllen)
+        goto err;
+    if (lastlabel[0] == '0' && lllen > 2
+        && (lastlabel[1] == 'x' || lastlabel[1] == 'X')
+        && strspn(lastlabel + 2, "0123456789abcdefABCDEF") == (lllen - 2))
+        goto err;
+    rv = 1;
+err:
+    X509_VERIFY_PARAM_free(vpm);
+    return rv;
 }
 
 /**
@@ -393,6 +427,13 @@ static int ech_decode_one_entry(OSSL_ECHSTORE_ENTRY **rent, PACKET *pkt,
         ERR_raise(ERR_LIB_SSL, SSL_R_ECH_DECODE_ERROR);
         goto err;
     }
+    /*
+     * We don't really handle ECHConfig extensions as of now,
+     * (none are well-defined), so we're only skipping over
+     * whatever we find here. If/when adding real extensions
+     * then it may be necessary to also check that the set of
+     * extensions loaded contain no duplicate types.
+     */
     if (!PACKET_get_length_prefixed_2(&ver_pkt, &exts)) {
         ERR_raise(ERR_LIB_SSL, SSL_R_ECH_DECODE_ERROR);
         goto err;
@@ -774,6 +815,8 @@ int OSSL_ECHSTORE_new_config(OSSL_ECHSTORE *es,
     epkt_mem->data = NULL;
     epkt_mem->length = 0;
     ee->loadtime = time(0);
+    if (ech_final_config_checks(ee) != 1) /* check our work */
+        goto err;
     /* push entry into store */
     if (es->entries == NULL)
         es->entries = sk_OSSL_ECHSTORE_ENTRY_new_null();

ssl/statem/extensions.c

@@ -53,6 +53,7 @@
  */
 #ifndef OPENSSL_NO_ECH
 static int init_ech(SSL_CONNECTION *s, unsigned int context);
+static int final_ech(SSL_CONNECTION *s, unsigned int context, int sent);
 #endif /* OPENSSL_NO_ECH */
 
 static int final_renegotiate(SSL_CONNECTION *s, unsigned int context, int sent);
@@ -452,7 +453,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         init_ech,
         tls_parse_ctos_ech, tls_parse_stoc_ech,
         tls_construct_stoc_ech, tls_construct_ctos_ech,
-        NULL },
+        final_ech },
     { TLSEXT_TYPE_outer_extensions,
         SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_ONLY,
         OSSL_ECH_HANDLING_CALL_BOTH,
@@ -1229,6 +1230,16 @@ static int init_ech(SSL_CONNECTION *s, unsigned int context)
         s->ext.ech.done = 0;
     return 1;
 }
+
+static int final_ech(SSL_CONNECTION *s, unsigned int context, int sent)
+{
+    if (s->server && s->ext.ech.success == 1
+        && s->ext.ech.inner_ech_seen_ok != 1) {
+        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_ECH_REQUIRED);
+        return 0;
+    }
+    return 1;
+}
 #endif /* OPENSSL_NO_ECH */
 
 static int final_server_name(SSL_CONNECTION *s, unsigned int context, int sent)

ssl/statem/extensions_srvr.c

@@ -2478,6 +2478,7 @@ int tls_parse_ctos_ech(SSL_CONNECTION *s, PACKET *pkt, unsigned int context,
         SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
         return 0;
     }
+    s->ext.ech.inner_ech_seen_ok = 1;
     if (s->ext.ech.success != 1 && s->ext.ech.backend != 1) {
         SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
         return 0;

ssl/statem/statem_clnt.c

@@ -1862,8 +1862,6 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)
             OSSL_TRACE_END(TLS);
             s->ext.ech.success = 1;
         }
-        /* we're done with that hrrsignal (if we got one) */
-        s->ext.ech.hrrsignal_p = NULL;
         if (!hrr && s->ext.ech.success == 1) {
             if (ossl_ech_swaperoo(s) != 1
                 || ossl_ech_intbuf_fetch(s, &abuf, &alen) != 1
@@ -1871,12 +1869,25 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)
                 SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
                 goto err;
             }
-        } else if (!hrr) {
+        } else if (hrr == 1 && s->ext.ech.success == 1) {
+            OSSL_TRACE(TLS, "ECH HRR accept ok, continuing.\n");
             /*
              * If we got retry_configs then we should be validating
              * the outer CH, so we better set the hostname for the
              * connection accordingly.
              */
+        } else if (!hrr && s->ext.ech.success == 0
+            && s->ext.ech.hrrsignal_p != NULL) {
+            /*
+             * we previously saw a good HRR ECH acceptance but now
+             * the SH.random ECH acceptance signal is bad so that's an
+             * illegal protocol error
+             */
+            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_ECH_REQUIRED);
+            goto err;
+        } else {
+            OSSL_TRACE1(TLS, "ECH falling back to public_name: %s\n",
+                s->ext.ech.outer_hostname != NULL ? s->ext.ech.outer_hostname : "NONE");
             s->ext.ech.former_inner = s->ext.hostname;
             s->ext.hostname = NULL;
             if (s->ext.ech.outer_hostname != NULL) {

test/ech_corrupt_test.c

@@ -384,6 +384,38 @@ static const unsigned char too_many_outers[] = {
     0x00, 0x00, 0x00
 };
 
+static const unsigned char tlsv12_like_inner[] = {
+    0x03, 0x03, /* version, then client-random */
+    0x23, 0xc3, 0xa0, 0x49, 0xea, 0x17, 0x9e, 0x30,
+    0x6f, 0x0e, 0xc9, 0x79, 0xd0, 0xd1, 0xfd, 0xea,
+    0x63, 0xfd, 0x20, 0x04, 0xaa, 0xb3, 0x2a, 0x29,
+    0xf5, 0x96, 0x60, 0x29, 0x42, 0x7e, 0x5c, 0x7b,
+    0x00, /* zero'd session ID */
+    0x00, 0x02, /* ciphersuite len, just one */
+    0xc0, 0x2c, /* a TLSv1.2 ciphersuite */
+    0x01, 0x00, /* no compression */
+    0x00, 0x2c, /* extslen */
+    0xfd, 0x00, /* outers */
+    0x00, 0x0b, /* len of outers */
+    0x0a, /* above minus one (10) 5 outers */
+    0x00, 0x0a, /* the 'normal' outers, minus supported_versions */
+    0x00, 0x23,
+    0x00, 0x16,
+    0x00, 0x17,
+    0x00, 0x0d,
+    /* and now the inner SNI, inner ECH and padding octets */
+    0x00, 0x00, 0x00, 0x14, 0x00, 0x12, 0x00, 0x00,
+    0x0f, 0x66, 0x6f, 0x6f, 0x2e, 0x65, 0x78, 0x61,
+    0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d,
+    0xfe, 0x0d, 0x00, 0x01, 0x01,
+    0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00
+};
+
 /*
  * a full padded, encoded inner client hello, but
  * without an inner supported extensions, (take
@@ -443,6 +475,30 @@ static const unsigned char tlsv12_inner[] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
 };
 
+/*
+ * a full padded, encoded inner with no inner ECH
+ * we change 0xfe 0x0d to 0xFF 0xFD in the ext type
+ */
+static const unsigned char encoded_inner_no_ech[] = {
+    0x03, 0x03, 0x7b, 0xe8, 0xc1, 0x18, 0xd7, 0xd1,
+    0x9c, 0x39, 0xa4, 0xfa, 0xce, 0x75, 0x72, 0x40,
+    0xcf, 0x37, 0xbb, 0x4c, 0xcd, 0xa7, 0x62, 0xda,
+    0x04, 0xd2, 0xdb, 0xe2, 0x89, 0x33, 0x36, 0x15,
+    0x96, 0xc9, 0x00, 0x00, 0x08, 0x13, 0x02, 0x13,
+    0x03, 0x13, 0x01, 0x00, 0xff, 0x01, 0x00, 0x00,
+    0x32, 0xfd, 0x00, 0x00, 0x11, 0x10,
+    0x00, 0x0a, 0x00, 0x23, 0x00, 0x16, 0x00, 0x17,
+    0x00, 0x0d, 0x00, 0x2b, 0x00, 0x2d, 0x00, 0x33,
+    0x00, 0x00, 0x00, 0x14, 0x00, 0x12, 0x00, 0x00,
+    0x0f, 0x66, 0x6f, 0x6f, 0x2e, 0x65, 0x78, 0x61,
+    0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d,
+    0xFF, 0xFD, 0x00, 0x01, 0x01, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00
+};
+
 /* A set of test vectors */
 static TEST_ECHINNER test_inners[] = {
     /* 1. basic case - copy to show test code works with no change */
@@ -471,8 +527,7 @@ static TEST_ECHINNER test_inners[] = {
         encoded_inner_outers, sizeof(encoded_inner_outers),
         bad_pad_encoded_inner_post, sizeof(bad_pad_encoded_inner_post),
         0, /* expected result */
-        SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC },
-
+        SSL_R_TLS_ALERT_ILLEGAL_PARAMETER },
     /*
      * 6. unsupported extension instead of outers - resulting decoded
      * inner missing so much it seems to be the wrong protocol
@@ -597,7 +652,7 @@ static TEST_ECHINNER test_inners[] = {
      * allow max tlsv1.3
      */
     { NULL, 0,
-        no_supported_exts, sizeof(no_supported_exts),
+        tlsv12_like_inner, sizeof(tlsv12_like_inner),
         NULL, 0,
         0, /* expected result */ SSL_R_UNSUPPORTED_PROTOCOL },
     /*
@@ -614,7 +669,7 @@ static TEST_ECHINNER test_inners[] = {
      * allow min tlsv1.2
      */
     { NULL, 0,
-        no_supported_exts, sizeof(no_supported_exts),
+        tlsv12_like_inner, sizeof(tlsv12_like_inner),
         NULL, 0,
         0, /* expected result */ SSL_R_UNSUPPORTED_PROTOCOL },
     /* 25. smuggled TLSv1.2 CH */
@@ -622,6 +677,11 @@ static TEST_ECHINNER test_inners[] = {
         tlsv12_inner, sizeof(tlsv12_inner),
         NULL, 0,
         0, /* expected result */ SSL_R_BAD_EXTENSION },
+    /* 26. otherwise-correct case that fails due to lack of inner ECH */
+    { NULL, 0,
+        encoded_inner_no_ech, sizeof(encoded_inner_no_ech),
+        NULL, 0,
+        0, /* expected result */ SSL_R_ECH_REQUIRED },
 };
 
 /*
@@ -677,8 +737,7 @@ static TEST_SH test_shs[] = {
         SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC },
     /* 5. flip bits in HRR.exts ECH confirmation value */
     { OSSL_ECH_BORK_HRR | OSSL_ECH_BORK_FLIP,
-        NULL, 0, 0,
-        SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC },
+        NULL, 0, 0, SSL_R_ECH_REQUIRED },
     /* 6. truncate HRR.exts ECH confirmation value */
     { OSSL_ECH_BORK_HRR | OSSL_ECH_BORK_REPLACE,
         shortech, sizeof(shortech), 0, SSL_R_LENGTH_MISMATCH },

test/ech_test.c

@@ -1208,6 +1208,67 @@ end:
     return rv;
 }
 
+/*
+ * Check whether various public_name values are good or bad according to
+ * our RFC 9849 checker, which imposes some oddball restrictions on those.
+ * Read section 6.1.7 of RFC 9849 for details.
+ */
+static int ech_bad_public_names(void)
+{
+    int rv = 0, i;
+    OSSL_ECHSTORE *es = NULL;
+    OSSL_HPKE_SUITE hpke_suite = OSSL_HPKE_SUITE_DEFAULT;
+    const char *bad_names[] = {
+        ".dot.", /* leading dot */
+        "dot.", /* trailing dot */
+        ".dot", /* check both, why not */
+        /* a label > 62 chars (70 in this case) */
+        "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij.org",
+        /* last label numeric */
+        "last.one.is.numeric.456",
+        "456",
+        /* last label ascii-hex */
+        "last.ah.0x123",
+        "0x123"
+    };
+    const char *good_names[] = {
+        "example.com",
+        "0x123.stuff",
+        "0x",
+        "a-b.c",
+        "example.com.c",
+        "a.b.0x1234567890abcdefX",
+        "a.b.1234567890Y"
+    };
+
+    if (!TEST_ptr(es = OSSL_ECHSTORE_new(libctx, propq)))
+        goto end;
+    for (i = 0; i != OSSL_NELEM(bad_names); i++) {
+        if (verbose)
+            TEST_info("checking bad name |%s|", bad_names[i]);
+        if (!TEST_false(OSSL_ECHSTORE_new_config(es, 0xfe0d, 0, bad_names[i],
+                hpke_suite))) {
+            if (verbose)
+                TEST_info("bad name |%s| erroneously accepted", bad_names[i]);
+            goto end;
+        }
+    }
+    for (i = 0; i != OSSL_NELEM(good_names); i++) {
+        if (verbose)
+            TEST_info("checking good name |%s|", good_names[i]);
+        if (!TEST_true(OSSL_ECHSTORE_new_config(es, 0xfe0d, 0, good_names[i],
+                hpke_suite))) {
+            if (verbose)
+                TEST_info("good name |%s| erroneously rejected", good_names[i]);
+            goto end;
+        }
+    }
+    rv = 1;
+end:
+    OSSL_ECHSTORE_free(es);
+    return rv;
+}
+
 /* values that can be used in helper below */
 #define OSSL_ECH_TEST_BASIC 0
 #define OSSL_ECH_TEST_HRR 1
@@ -2000,6 +2061,7 @@ int setup_tests(void)
     ADD_ALL_TESTS(ech_test_file_read, OSSL_NELEM(fnames));
     ADD_TEST(ech_api_basic_calls);
     ADD_TEST(ech_boring_compat);
+    ADD_TEST(ech_bad_public_names);
     suite_combos = OSSL_NELEM(kem_str_list) * OSSL_NELEM(kdf_str_list)
         * OSSL_NELEM(aead_str_list);
     ADD_ALL_TESTS(test_ech_suites, suite_combos);