From: sftcd Date: Tue, 17 Mar 2026 21:08:36 +0000 (+0000) Subject: ECH: Conformance test changes in response to AISLE review X-Git-Tag: openssl-4.0.0~36 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5bb9dc1f128f5c4fbdc56b16aca4ecbeab8b318b;p=thirdparty%2Fopenssl.git ECH: Conformance test changes in response to AISLE review Reviewed-by: Matt Caswell Reviewed-by: Tomas Mraz MergeDate: Wed Apr 8 08:59:19 2026 (Merged from https://github.com/openssl/openssl/pull/30419) (cherry picked from commit 7952bc4b8a31ba35d12f9109da0ab82a7ce3318a) --- diff --git a/doc/man3/SSL_set1_echstore.pod b/doc/man3/SSL_set1_echstore.pod index ada6712c6a4..cd983c4ae97 100644 --- a/doc/man3/SSL_set1_echstore.pod +++ b/doc/man3/SSL_set1_echstore.pod @@ -327,7 +327,7 @@ Return codes from SSL_ech_get_status =item B -101, ECH wasn't attempted -=item B -102, ECH ok but server cert bad +=item B -102, ECH ok but server or client cert bad =item B -103, ECH wasn't configured diff --git a/ssl/ech/ech_internal.c b/ssl/ech/ech_internal.c index 338c111bfd0..78e78bf8d13 100644 --- a/ssl/ech/ech_internal.c +++ b/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) { diff --git a/ssl/ech/ech_local.h b/ssl/ech/ech_local.h index b42196e210c..1b31023cbab 100644 --- a/ssl/ech/ech_local.h +++ b/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 */ diff --git a/ssl/ech/ech_store.c b/ssl/ech/ech_store.c index fb0b72bde8d..1f5d79e0212 100644 --- a/ssl/ech/ech_store.c +++ b/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(); diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 16467f782fc..3f32eca3c4a 100644 --- a/ssl/statem/extensions.c +++ b/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, @@ -1217,6 +1218,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) diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 29be982383d..3036812af9e 100644 --- a/ssl/statem/extensions_srvr.c +++ b/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; diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 7d0818e7ab2..e4ba06567f1 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1876,8 +1876,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 @@ -1885,12 +1883,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) { diff --git a/test/ech_corrupt_test.c b/test/ech_corrupt_test.c index ee1edeff5b6..7f11a3f7faf 100644 --- a/test/ech_corrupt_test.c +++ b/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 }, diff --git a/test/ech_test.c b/test/ech_test.c index c26a33dc129..49bc26ad47d 100644 --- a/test/ech_test.c +++ b/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);