Commit 3f22aab4f9 for openssl.org
commit 3f22aab4f9a0676db23e645abb3c5bdbdc84e464
Author: sftcd <stephen.farrell@cs.tcd.ie>
Date: Tue Mar 17 21:10:34 2026 +0000
ECH: conformance test changes for echspec test tool
Reviewed-by: Matt Caswell <matt@openssl.foundation>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
MergeDate: Wed Apr 8 08:59:20 2026
(Merged from https://github.com/openssl/openssl/pull/30419)
diff --git a/ssl/ech/ech_helper.c b/ssl/ech/ech_helper.c
index ee71c3443e..86cb9dbb88 100644
--- a/ssl/ech/ech_helper.c
+++ b/ssl/ech/ech_helper.c
@@ -85,9 +85,11 @@ int ossl_ech_helper_get_ch_offsets(const unsigned char *ch, size_t ch_len,
if (ch == NULL || ch_len == 0 || sessid_off == NULL || exts_off == NULL
|| ech_off == NULL || echtype == NULL || ech_len == NULL
- || sni_off == NULL || inner == NULL)
+ || sni_off == NULL || inner == NULL || exts_len == NULL)
return 0;
*sessid_off = *exts_off = *ech_off = *sni_off = *sni_len = *ech_len = 0;
+ *exts_len = 0;
+ *inner = OSSL_ECH_UNKNOWN_CH_TYPE;
*echtype = 0xffff;
if (!PACKET_buf_init(&pkt, ch, ch_len))
return 0;
diff --git a/ssl/ech/ech_internal.c b/ssl/ech/ech_internal.c
index 78e78bf8d1..72e927ad27 100644
--- a/ssl/ech/ech_internal.c
+++ b/ssl/ech/ech_internal.c
@@ -1136,7 +1136,7 @@ int ossl_ech_get_ch_offsets(SSL_CONNECTION *s, PACKET *pkt, size_t *sessid_off,
if (pkt == NULL || sessid_off == NULL || exts_off == NULL
|| ech_off == NULL || echtype == NULL || inner == NULL
|| sni_off == NULL) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
return 0;
}
/* check if we've already done the work */
@@ -1157,14 +1157,14 @@ int ossl_ech_get_ch_offsets(SSL_CONNECTION *s, PACKET *pkt, size_t *sessid_off,
/* do the work */
ch_len = PACKET_remaining(pkt);
if (PACKET_peek_bytes(pkt, &ch, ch_len) != 1) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_EXTENSION);
return 0;
}
if (ossl_ech_helper_get_ch_offsets(ch, ch_len, sessid_off, exts_off,
&exts_len, ech_off, echtype, &ech_len,
sni_off, &sni_len, inner)
!= 1) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
return 0;
}
#ifdef OSSL_ECH_SUPERVERBOSE
@@ -1218,8 +1218,12 @@ static int ech_get_outer_sni(SSL_CONNECTION *s, char **osni_str,
|| !PACKET_get_net_2(&wrap, &type)
|| type != 0
|| !PACKET_get_net_2(&wrap, &osnilen)
- || !PACKET_get_sub_packet(&wrap, &osni, osnilen)
- || tls_parse_ctos_server_name(s, &osni, 0, NULL, 0) != 1)
+ || !PACKET_get_sub_packet(&wrap, &osni, osnilen)) {
+ SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ return 0;
+ }
+ if (tls_parse_ctos_server_name(s, &osni, 0, NULL, 0) != 1)
+ /* SSLfatal called already */
return 0;
OPENSSL_free(s->ext.ech.outer_hostname);
*osni_str = s->ext.ech.outer_hostname = s->ext.hostname;
@@ -1272,7 +1276,7 @@ static int ech_decode_inbound_ech(SSL_CONNECTION *s, PACKET *pkt,
goto err;
}
if (innerorouter != OSSL_ECH_OUTER_CH_TYPE) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
if (!PACKET_get_net_2(pkt, &pval_tmp)) {
@@ -1300,7 +1304,7 @@ static int ech_decode_inbound_ech(SSL_CONNECTION *s, PACKET *pkt,
goto err;
}
if (pval_tmp > OSSL_ECH_MAX_GREASE_PUB) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
if (pval_tmp > PACKET_remaining(pkt)) {
@@ -1308,7 +1312,7 @@ static int ech_decode_inbound_ech(SSL_CONNECTION *s, PACKET *pkt,
goto err;
}
if (pval_tmp == 0 && s->hello_retry_request != SSL_HRR_PENDING) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
} else if (pval_tmp > 0 && s->hello_retry_request == SSL_HRR_PENDING) {
unsigned char *tmpenc = NULL;
@@ -1318,12 +1322,17 @@ static int ech_decode_inbound_ech(SSL_CONNECTION *s, PACKET *pkt,
* and it should be the same value as 1st time, so we'll check
* that
*/
+ if (s->ext.ech.success == 1) {
+ /* first decrypt worked, so enc should be empty */
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
+ goto err;
+ }
if (s->ext.ech.pub == NULL || s->ext.ech.pub_len == 0) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
if (pval_tmp != s->ext.ech.pub_len) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
tmpenc = OPENSSL_malloc(pval_tmp);
@@ -1336,13 +1345,13 @@ static int ech_decode_inbound_ech(SSL_CONNECTION *s, PACKET *pkt,
}
if (memcmp(tmpenc, s->ext.ech.pub, pval_tmp) != 0) {
OPENSSL_free(tmpenc);
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
OPENSSL_free(tmpenc);
} else if (pval_tmp == 0 && s->hello_retry_request == SSL_HRR_PENDING) {
if (s->ext.ech.pub == NULL || s->ext.ech.pub_len == 0) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
extval->enc_len = s->ext.ech.pub_len;
@@ -1374,11 +1383,11 @@ static int ech_decode_inbound_ech(SSL_CONNECTION *s, PACKET *pkt,
goto err;
}
if (pval_tmp > OSSL_ECH_MAX_PAYLOAD_LEN) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
if (pval_tmp == 0 || pval_tmp > PACKET_remaining(pkt)) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
extval->payload_len = pval_tmp;
@@ -1469,7 +1478,7 @@ static int ech_find_outers(SSL_CONNECTION *s, PACKET *pkt,
if (!PACKET_get_1(&op, &olen)
|| olen % 2 == 1
|| olen / 2 > OSSL_ECH_OUTERS_MAX) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
*n_outers = olen / 2;
@@ -1513,14 +1522,14 @@ static int ech_copy_ext(SSL_CONNECTION *s, WPACKET *di, uint16_t type2copy,
if (!WPACKET_put_bytes_u16(di, etype)
|| !WPACKET_put_bytes_u16(di, elen)
|| !WPACKET_memcpy(di, eval, elen)) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_EXTENSION);
goto err;
}
return 1;
}
}
/* we didn't find such an extension - that's an error */
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
err:
return 0;
}
@@ -1554,6 +1563,7 @@ static int ech_reconstitute_inner(SSL_CONNECTION *s, WPACKET *di, PACKET *ei,
!PACKET_get_net_2(&outer, &pi_tmp)
|| !PACKET_get_net_2(ei, &pi_tmp)
|| !WPACKET_put_bytes_u16(di, pi_tmp)
+ || pi_tmp != TLS1_2_VERSION
/* client random */
|| !PACKET_get_bytes(&outer, &pp_tmp, SSL3_RANDOM_SIZE)
@@ -1636,7 +1646,7 @@ static int ech_reconstitute_inner(SSL_CONNECTION *s, WPACKET *di, PACKET *ei,
if (!WPACKET_put_bytes_u16(di, etype)
|| !WPACKET_put_bytes_u16(di, elen)
|| !WPACKET_memcpy(di, eval, elen)) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_EXTENSION);
goto err;
}
}
@@ -1844,7 +1854,7 @@ end:
/* we need to remove possible (actually, v. likely) padding */
*innerlen = clearlen;
if (ee->version == OSSL_ECH_RFC9849_VERSION) {
- /* draft-13 pads after the encoded CH with zeros */
+ /* RFC 9849 pads after the encoded CH with zeros */
size_t extsoffset = 0;
size_t extslen = 0;
size_t ch_len = 0;
@@ -1852,11 +1862,11 @@ end:
size_t echoffset = 0; /* offset of start of ECH within CH */
uint16_t echtype = OSSL_ECH_type_unknown; /* type of ECH seen */
size_t outersnioffset = 0; /* offset to SNI in outer */
- int innerflag = -1;
+ int innerflag = OSSL_ECH_UNKNOWN_CH_TYPE;
PACKET innerchpkt;
if (PACKET_buf_init(&innerchpkt, clear, clearlen) != 1) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_EXTENSION);
goto paderr;
}
/* reset the offsets, as we move from outer to inner CH */
@@ -1865,7 +1875,7 @@ end:
&extsoffset, &echoffset, &echtype,
&innerflag, &outersnioffset);
if (rv != 1) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ /* SSLfatal called already */
goto paderr;
}
/* odd form of check below just for emphasis */
@@ -1884,7 +1894,7 @@ end:
/* The RFC calls for that padding to be all zeros */
if (*innerlen < ch_len) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto paderr;
}
for (zind = ch_len; zind != *innerlen; zind++) {
@@ -1898,6 +1908,8 @@ end:
ossl_ech_pbuf("unpadded clear", clear, *innerlen);
#endif
return clear;
+ } else {
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
}
paderr:
OPENSSL_free(clear);
@@ -1947,7 +1959,7 @@ int ossl_ech_early_decrypt(SSL_CONNECTION *s, PACKET *outerpkt, PACKET *newpkt)
if (s == NULL)
return 0;
if (outerpkt == NULL || newpkt == NULL) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_EXTENSION);
return 0;
}
/* find offsets - on success, outputs are safe to use */
@@ -1955,13 +1967,13 @@ int ossl_ech_early_decrypt(SSL_CONNECTION *s, PACKET *outerpkt, PACKET *newpkt)
&echoffset, &echtype, &innerflag,
&outersnioffset)
!= 1) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ /* SSLfatal called already */
return 0;
}
if (echoffset == 0 || echtype != TLSEXT_TYPE_ech)
return 1; /* ECH not present or wrong version */
if (innerflag == 1) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
return 0;
}
s->ext.ech.attempted = 1; /* Remember that we got an ECH */
@@ -1973,14 +1985,16 @@ int ossl_ech_early_decrypt(SSL_CONNECTION *s, PACKET *outerpkt, PACKET *newpkt)
s->tmp_session_id_len = opd[startofsessid]; /* grab the session id */
if (s->tmp_session_id_len > SSL_MAX_SSL_SESSION_ID_LENGTH
|| startofsessid + 1 + s->tmp_session_id_len > opl) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
memcpy(s->tmp_session_id, &opd[startofsessid + 1], s->tmp_session_id_len);
if (outersnioffset > 0) { /* Grab the outer SNI for tracing */
- if (ech_get_outer_sni(s, &osni_str, opd, opl, outersnioffset) != 1
- || osni_str == NULL) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ if (ech_get_outer_sni(s, &osni_str, opd, opl, outersnioffset) != 1)
+ /* SSLfatal called already */
+ goto err;
+ if (osni_str == NULL) {
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
OSSL_TRACE1(TLS, "EARLY: outer SNI of %s\n", osni_str);
@@ -1998,7 +2012,7 @@ int ossl_ech_early_decrypt(SSL_CONNECTION *s, PACKET *outerpkt, PACKET *newpkt)
goto err;
}
if (PACKET_buf_init(&echpkt, startofech, echlen) != 1) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_EXTENSION);
goto err;
}
if (ech_decode_inbound_ech(s, &echpkt, &extval, &startofciphertext) != 1)
@@ -2027,7 +2041,7 @@ int ossl_ech_early_decrypt(SSL_CONNECTION *s, PACKET *outerpkt, PACKET *newpkt)
#endif
s->ext.ech.grease = OSSL_ECH_GREASE_UNKNOWN;
if (s->ext.ech.es == NULL) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
goto err;
}
es = s->ext.ech.es;
diff --git a/ssl/ech/ech_local.h b/ssl/ech/ech_local.h
index 1b31023cba..143742354a 100644
--- a/ssl/ech/ech_local.h
+++ b/ssl/ech/ech_local.h
@@ -43,6 +43,7 @@
#define OSSL_ECH_OUTER_CH_TYPE 0 /* outer ECHClientHello enum */
#define OSSL_ECH_INNER_CH_TYPE 1 /* inner ECHClientHello enum */
+#define OSSL_ECH_UNKNOWN_CH_TYPE -1 /* we don't know yet */
#define OSSL_ECH_CIPHER_LEN 4 /* ECHCipher length (2 for kdf, 2 for aead) */
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 3036812af9..679b5de92c 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -2465,7 +2465,7 @@ int tls_parse_ctos_ech(SSL_CONNECTION *s, PACKET *pkt, unsigned int context,
}
if (s->ext.ech.attempted_type != TLSEXT_TYPE_ech) {
/* if/when new versions of ECH are added we'll update here */
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
return 0;
}
/*
@@ -2473,14 +2473,17 @@ int tls_parse_ctos_ech(SSL_CONNECTION *s, PACKET *pkt, unsigned int context,
* and only if we decrypted ok or are a backend
*/
if (PACKET_get_1(pkt, &echtype) != 1
- || echtype != OSSL_ECH_INNER_CH_TYPE
|| PACKET_remaining(pkt) != 0) {
SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
return 0;
}
+ if (echtype != OSSL_ECH_INNER_CH_TYPE) {
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, 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);
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
return 0;
}
/* yay - we're ok with this */
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 84e8b92a1e..8197b60530 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1634,7 +1634,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL_CONNECTION *s, PACKET *pkt)
* that ECH "worked."
*/
if (s->server && PACKET_remaining(pkt) != 0) {
- int rv = 0, innerflag = -1;
+ int rv = 0, innerflag = OSSL_ECH_UNKNOWN_CH_TYPE;
size_t startofsessid = 0, startofexts = 0, echoffset = 0;
size_t outersnioffset = 0; /* offset to SNI in outer */
uint16_t echtype = OSSL_ECH_type_unknown; /* type of ECH seen */
@@ -1646,7 +1646,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL_CONNECTION *s, PACKET *pkt)
&echoffset, &echtype, &innerflag,
&outersnioffset);
if (rv != 1) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
+ /* SSLfatal already called */
goto err;
}
if (innerflag == OSSL_ECH_INNER_CH_TYPE) {
@@ -1689,9 +1689,19 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL_CONNECTION *s, PACKET *pkt)
}
} else if (s->ext.ech.es != NULL) {
PACKET newpkt;
+ int secondtime = s->ext.ech.success;
+ /*
+ * if ECH decrypt worked first time (success == 1) then fail
+ * if there's no ECH extension at all 2nd time
+ */
+ if (secondtime == 1 && echoffset == 0) {
+ SSLfatal(s, SSL_AD_MISSING_EXTENSION,
+ SSL_R_TLSV13_ALERT_MISSING_EXTENSION);
+ goto err;
+ }
if (ossl_ech_early_decrypt(s, pkt, &newpkt) != 1) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+ /* SSLfatal() already called */
goto err;
}
if (s->ext.ech.success == 1) {
@@ -1705,6 +1715,10 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL_CONNECTION *s, PACKET *pkt)
goto err;
}
*pkt = newpkt;
+ } else if (secondtime == 1 && s->ext.ech.success == 0) {
+ /* 2nd time decrypt failed */
+ SSLfatal(s, SSL_AD_DECRYPT_ERROR, ERR_R_INTERNAL_ERROR);
+ goto err;
}
}
}
@@ -1872,6 +1886,11 @@ static int tls_early_post_process_client_hello(SSL_CONNECTION *s)
/* Choose the server SSL/TLS/DTLS version. */
protverr = ssl_choose_server_version(s, clienthello, &dgrd);
+#ifndef OPENSSL_NO_ECH
+ if (protverr && s->ext.ech.success == 1) {
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, protverr);
+ }
+#endif
if (protverr) {
if (SSL_IS_FIRST_HANDSHAKE(s)) {
/* like ssl3_get_record, send alert using remote version number */