From: sftcd Date: Sat, 28 Dec 2024 02:49:12 +0000 (+0000) Subject: ECH client side transcript refactor X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=66039b9e760f82fe9ab5e3938be678f9c566a11a;p=thirdparty%2Fopenssl.git ECH client side transcript refactor Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/26011) --- diff --git a/include/internal/ech_helpers.h b/include/internal/ech_helpers.h index b71029857b1..3e13936e2e9 100644 --- a/include/internal/ech_helpers.h +++ b/include/internal/ech_helpers.h @@ -17,10 +17,8 @@ # ifndef OPENSSL_NO_ECH -int ossl_ech_get_sh_offsets(const unsigned char *sh, size_t sh_len, - size_t *exts, size_t *echoffset, - uint16_t *echtype); -int ossl_ech_make_enc_info(unsigned char *encoding, size_t encoding_length, +int ossl_ech_make_enc_info(const unsigned char *encoding, + size_t encoding_length, unsigned char *info, size_t *info_len); # endif diff --git a/ssl/ech/ech_helper.c b/ssl/ech/ech_helper.c index 6f900408ef5..4303a1b6eb0 100644 --- a/ssl/ech/ech_helper.c +++ b/ssl/ech/ech_helper.c @@ -20,116 +20,22 @@ static const char OSSL_ECH_CONTEXT_STRING[] = "\x74\x6c\x73\x20\x65\x63\x68"; /* - * Given a SH (or HRR) find the offsets of the ECH (if any) - * sh is the SH buffer - * sh_len is the length of the SH - * exts points to offset of extensions - * echoffset points to offset of ECH - * echtype points to the ext type of the ECH - * return 1 for success, zero otherwise - * - * Offsets are returned to the type or length field in question. - * Offsets are set to zero if relevant thing not found. - * - * Note: input here is untrusted! - */ -int ossl_ech_get_sh_offsets(const unsigned char *sh, size_t sh_len, - size_t *exts, size_t *echoffset, - uint16_t *echtype) -{ - unsigned int etype = 0, pi_tmp = 0; - const unsigned char *pp_tmp = NULL, *shstart = NULL; - PACKET pkt, session_id, extpkt, oneext; - size_t extlens = 0; - int done = 0; -#ifdef OSSL_ECH_SUPERVERBOSE - size_t echlen = 0; /* length of ECH, including type & ECH-internal length */ - size_t sessid_offset = 0, sessid_len = 0; -#endif - - if (sh == NULL || sh_len == 0 || exts == NULL || echoffset == NULL - || echtype == NULL) - return 0; - *exts = *echoffset = *echtype = 0; - if (!PACKET_buf_init(&pkt, sh, sh_len)) - return 0; - shstart = PACKET_data(&pkt); - if (!PACKET_get_net_2(&pkt, &pi_tmp)) - return 0; - /* - * TODO(ECH): we've had a TLSv1.2 test in the past where we add an - * ECH to a TLSv1.2 CH to ensure server code ignores that properly. - * We might or might not keep that, if we don't then the test below - * should allow TLSv1.3 only. - */ - /* if we're not TLSv1.2+ then we can bail, but it's not an error */ - if (pi_tmp != TLS1_2_VERSION && pi_tmp != TLS1_3_VERSION) - return 1; - if (!PACKET_get_bytes(&pkt, &pp_tmp, SSL3_RANDOM_SIZE) -#ifdef OSSL_ECH_SUPERVERBOSE - || (sessid_offset = PACKET_data(&pkt) - shstart) == 0 -#endif - || !PACKET_get_length_prefixed_1(&pkt, &session_id) -#ifdef OSSL_ECH_SUPERVERBOSE - || (sessid_len = PACKET_remaining(&session_id)) == 0 -#endif - || !PACKET_get_net_2(&pkt, &pi_tmp) /* ciphersuite */ - || !PACKET_get_1(&pkt, &pi_tmp) /* compression */ - || (*exts = PACKET_data(&pkt) - shstart) == 0 - || !PACKET_as_length_prefixed_2(&pkt, &extpkt) - || PACKET_remaining(&pkt) != 0) - return 0; - extlens = PACKET_remaining(&extpkt); - if (extlens == 0) /* not an error, in theory */ - return 1; - while (PACKET_remaining(&extpkt) > 0 && done < 1) { - if (!PACKET_get_net_2(&extpkt, &etype) - || !PACKET_get_length_prefixed_2(&extpkt, &oneext)) - return 0; - if (etype == TLSEXT_TYPE_ech) { - if (PACKET_remaining(&oneext) != 8) - return 0; - *echoffset = PACKET_data(&oneext) - shstart - 4; - *echtype = etype; -#ifdef OSSL_ECH_SUPERVERBOSE - echlen = PACKET_remaining(&oneext) + 4; /* type/length included */ -#endif - done++; - } - } -#ifdef OSSL_ECH_SUPERVERBOSE - OSSL_TRACE_BEGIN(TLS) { - BIO_printf(trc_out, "orig SH/ECH type: %4x\n", *echtype); - } OSSL_TRACE_END(TLS); - ossl_ech_pbuf("orig SH", (unsigned char *)sh, sh_len); - ossl_ech_pbuf("orig SH session_id", (unsigned char *)sh + sessid_offset, - sessid_len); - ossl_ech_pbuf("orig SH exts", (unsigned char *)sh + *exts, extlens); - ossl_ech_pbuf("orig SH/ECH ", (unsigned char *)sh + *echoffset, echlen); -#endif - return 1; -} - -/* - * make up HPKE "info" input as per spec + * Construct HPKE "info" input as per spec * encoding is the ECHconfig being used - * encodinglen is the length of ECHconfig being used + * encoding_length is the length of ECHconfig being used * info is a caller-allocated buffer for results * info_len is the buffer size on input, used-length on output * return 1 for success, zero otherwise */ -int ossl_ech_make_enc_info(unsigned char *encoding, size_t encoding_length, +int ossl_ech_make_enc_info(const unsigned char *encoding, + size_t encoding_length, unsigned char *info, size_t *info_len) { WPACKET ipkt = { 0 }; - BUF_MEM *ipkt_mem = NULL; if (encoding == NULL || info == NULL || info_len == NULL) return 0; - if (*info_len < (sizeof(OSSL_ECH_CONTEXT_STRING) + encoding_length)) - return 0; - if ((ipkt_mem = BUF_MEM_new()) == NULL - || !WPACKET_init(&ipkt, ipkt_mem) + if (!WPACKET_init_static_len(&ipkt, info, *info_len, 0) || !WPACKET_memcpy(&ipkt, OSSL_ECH_CONTEXT_STRING, sizeof(OSSL_ECH_CONTEXT_STRING) - 1) /* @@ -138,14 +44,11 @@ int ossl_ech_make_enc_info(unsigned char *encoding, size_t encoding_length, * the context string being NUL terminated */ || !WPACKET_put_bytes_u8(&ipkt, 0) - || !WPACKET_memcpy(&ipkt, encoding, encoding_length)) { + || !WPACKET_memcpy(&ipkt, encoding, encoding_length) + || !WPACKET_get_total_written(&ipkt, info_len)) { WPACKET_cleanup(&ipkt); - BUF_MEM_free(ipkt_mem); return 0; } - *info_len = sizeof(OSSL_ECH_CONTEXT_STRING) + encoding_length; - memcpy(info, WPACKET_get_curr(&ipkt) - *info_len, *info_len); WPACKET_cleanup(&ipkt); - BUF_MEM_free(ipkt_mem); return 1; } diff --git a/ssl/ech/ech_internal.c b/ssl/ech/ech_internal.c index a8d60102046..ae71656546e 100644 --- a/ssl/ech/ech_internal.c +++ b/ssl/ech/ech_internal.c @@ -12,7 +12,8 @@ #include "../ssl_local.h" #include "ech_local.h" #include -#include +#include "../statem/statem_local.h" +#include "internal/ech_helpers.h" #include #ifndef OPENSSL_NO_ECH @@ -46,26 +47,29 @@ void ossl_ech_pbuf(const char *msg, const unsigned char *buf, const size_t blen) } /* trace out transcript */ -void ossl_ech_ptranscript(SSL_CONNECTION *s, const char *msg) +static void ossl_ech_ptranscript(SSL_CONNECTION *s, const char *msg) { - OSSL_TRACE_BEGIN(TLS) { - size_t hdatalen = 0; - unsigned char *hdata = NULL; - unsigned char ddata[EVP_MAX_MD_SIZE]; - size_t ddatalen; + size_t hdatalen = 0; + unsigned char *hdata = NULL; + unsigned char ddata[EVP_MAX_MD_SIZE]; + size_t ddatalen; - if (s == NULL) - return; - hdatalen = BIO_get_mem_data(s->s3.handshake_buffer, &hdata); - ossl_ech_pbuf(msg, hdata, hdatalen); - if (s->s3.handshake_dgst != NULL) { - if (ssl_handshake_hash(s, ddata, sizeof(ddata), &ddatalen) == 0) + if (s == NULL) + return; + hdatalen = BIO_get_mem_data(s->s3.handshake_buffer, &hdata); + ossl_ech_pbuf(msg, hdata, hdatalen); + if (s->s3.handshake_dgst != NULL) { + if (ssl_handshake_hash(s, ddata, sizeof(ddata), &ddatalen) == 0) { + OSSL_TRACE_BEGIN(TLS) { BIO_printf(trc_out, "ssl_handshake_hash failed\n"); + } OSSL_TRACE_END(TLS); ossl_ech_pbuf(msg, ddata, ddatalen); - } else { - BIO_printf(trc_out, "handshake_dgst is NULL\n"); } + } + OSSL_TRACE_BEGIN(TLS) { + BIO_printf(trc_out, "new transbuf:\n"); } OSSL_TRACE_END(TLS); + ossl_ech_pbuf(msg, s->ext.ech.transbuf, s->ext.ech.transbuf_len); return; } # endif @@ -168,16 +172,15 @@ void ossl_ech_conn_clear(OSSL_ECH_CONN *ec) OPENSSL_free(ec->outer_hostname); OPENSSL_free(ec->alpn_outer); OPENSSL_free(ec->former_inner); + OPENSSL_free(ec->transbuf); OPENSSL_free(ec->innerch); - OPENSSL_free(ec->encoded_innerch); - OPENSSL_free(ec->innerch1); - OPENSSL_free(ec->kepthrr); OPENSSL_free(ec->grease_suite); OPENSSL_free(ec->sent); OPENSSL_free(ec->returned); OPENSSL_free(ec->pub); OSSL_HPKE_CTX_free(ec->hpke_ctx); EVP_PKEY_free(ec->tmp_pkey); + OPENSSL_free(ec->encoded_inner); return; } @@ -226,8 +229,8 @@ int ossl_ech_get_retry_configs(SSL_CONNECTION *s, unsigned char **rcfgs, OSSL_ECHSTORE *es = NULL; OSSL_ECHSTORE_ENTRY *ee = NULL; int i, num = 0; - size_t retslen = 0, encilen = 0; - unsigned char *tmp = NULL, *enci = NULL, *rets = NULL; + size_t retslen = 0; + unsigned char *tmp = NULL, *rets = NULL; if (s == NULL || rcfgs == NULL || rcfgslen == NULL) return 0; @@ -237,17 +240,13 @@ int ossl_ech_get_retry_configs(SSL_CONNECTION *s, unsigned char **rcfgs, for (i = 0; i != num; i++) { ee = sk_OSSL_ECHSTORE_ENTRY_value(es->entries, i); if (ee != NULL && ee->for_retry == OSSL_ECH_FOR_RETRY) { - encilen = ee->encoded_len; - if (encilen < 2) - goto err; - encilen -= 2; - enci = ee->encoded + 2; - tmp = (unsigned char *)OPENSSL_realloc(rets, retslen + encilen); + tmp = (unsigned char *)OPENSSL_realloc(rets, + retslen + ee->encoded_len); if (tmp == NULL) goto err; rets = tmp; - memcpy(rets + retslen, enci, encilen); - retslen += encilen; + memcpy(rets + retslen, ee->encoded, ee->encoded_len); + retslen += ee->encoded_len; } } *rcfgs = rets; @@ -285,7 +284,6 @@ int ossl_ech_send_grease(SSL_CONNECTION *s, WPACKET *pkt) size_t cipher_len = 0, cipher_len_jitter = 0; unsigned char cid, senderpub[OSSL_ECH_MAX_GREASE_PUB]; unsigned char cipher[OSSL_ECH_MAX_GREASE_CT]; - unsigned char *pp = WPACKET_get_curr(pkt); if (s == NULL) return 0; @@ -295,8 +293,7 @@ int ossl_ech_send_grease(SSL_CONNECTION *s, WPACKET *pkt) } WPACKET_get_total_written(pkt, &pp_at_start); /* randomly select cipher_len to be one of 144, 176, 208, 244 */ - if (RAND_bytes_ex(s->ssl.ctx->libctx, &cid, 1, - RAND_DRBG_STRENGTH) <= 0) { + if (RAND_bytes_ex(s->ssl.ctx->libctx, &cid, 1, 0) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } @@ -304,8 +301,7 @@ int ossl_ech_send_grease(SSL_CONNECTION *s, WPACKET *pkt) cipher_len = 144; cipher_len += 32 * cipher_len_jitter; /* generate a random (1 octet) client id */ - if (RAND_bytes_ex(s->ssl.ctx->libctx, &cid, 1, - RAND_DRBG_STRENGTH) <= 0) { + if (RAND_bytes_ex(s->ssl.ctx->libctx, &cid, 1, 0) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } @@ -348,7 +344,8 @@ int ossl_ech_send_grease(SSL_CONNECTION *s, WPACKET *pkt) SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } - memcpy(s->ext.ech.sent, pp, s->ext.ech.sent_len); + memcpy(s->ext.ech.sent, WPACKET_get_curr(pkt) - s->ext.ech.sent_len, + s->ext.ech.sent_len); s->ext.ech.grease = OSSL_ECH_IS_GREASE; OSSL_TRACE_BEGIN(TLS) { BIO_printf(trc_out, "ECH - sending GREASE\n"); @@ -373,6 +370,7 @@ int ossl_ech_pick_matching_cfg(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY **ee, if (s == NULL || s->ext.ech.es == NULL || ee == NULL || suite == NULL) return 0; + *ee = NULL; es = s->ext.ech.es; if (es->entries == NULL) return 0; @@ -387,7 +385,7 @@ int ossl_ech_pick_matching_cfg(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY **ee, hnlen = 0; nameoverride = 1; } - for (cind = 0; cind != num && (suitematch == 0 || namematch == 0); cind++) { + for (cind = 0; cind < num && (suitematch == 0 || namematch == 0); cind++) { lee = sk_OSSL_ECHSTORE_ENTRY_value(es->entries, cind); if (lee == NULL || lee->version != OSSL_ECH_RFCXXXX_VERSION) continue; @@ -398,8 +396,8 @@ int ossl_ech_pick_matching_cfg(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY **ee, if (hnlen == 0 || (lee->public_name != NULL && strlen(lee->public_name) == hnlen - && !OPENSSL_strncasecmp(hn, (char *)lee->public_name, - hnlen))) + && OPENSSL_strncasecmp(hn, (char *)lee->public_name, + hnlen) == 0)) namematch = 1; } suitematch = 0; @@ -418,7 +416,8 @@ int ossl_ech_pick_matching_cfg(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY **ee, } } } - if (nameoverride == 1 && (namematch == 0 || suitematch == 0)) { + if (tee != NULL && nameoverride == 1 + && (namematch == 0 || suitematch == 0)) { *suite = tee->suites[tsuite]; *ee = tee; } else if (namematch == 0 || suitematch == 0) { @@ -431,11 +430,11 @@ int ossl_ech_pick_matching_cfg(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY **ee, } /* Make up the ClientHelloInner and EncodedClientHelloInner buffers */ -int ossl_ech_encode_inner(SSL_CONNECTION *s) +int ossl_ech_encode_inner(SSL_CONNECTION *s, unsigned char **encoded, + size_t *encoded_len) { int rv = 0; size_t nraws = 0, ind = 0, innerlen = 0; - unsigned char *innerch_full = NULL; WPACKET inner = { 0 }; /* "fake" pkt for inner */ BUF_MEM *inner_mem = NULL; RAW_EXTENSION *raws = NULL; @@ -517,15 +516,9 @@ int ossl_ech_encode_inner(SSL_CONNECTION *s) SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } - innerch_full = OPENSSL_malloc(innerlen); - if (innerch_full == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - memcpy(innerch_full, inner_mem->data, innerlen); - OPENSSL_free(s->ext.ech.encoded_innerch); - s->ext.ech.encoded_innerch = innerch_full; - s->ext.ech.encoded_innerch_len = innerlen; + *encoded = (unsigned char *)inner_mem->data; + inner_mem->data = NULL; /* keep BUF_MEM_free happy */ + *encoded_len = innerlen; /* and clean up */ rv = 1; err: @@ -543,192 +536,19 @@ err: * return: 1 for success, 0 otherwise */ int ossl_ech_find_confirm(SSL_CONNECTION *s, int hrr, - unsigned char acbuf[OSSL_ECH_SIGNAL_LEN], - const unsigned char *shbuf, const size_t shlen) + unsigned char acbuf[OSSL_ECH_SIGNAL_LEN]) { - PACKET pkt; - const unsigned char *acp = NULL, *pp_tmp; - unsigned int pi_tmp, etype, elen; - int done = 0; + unsigned char *acp = NULL; if (hrr == 0) { acp = s->s3.server_random + SSL3_RANDOM_SIZE - OSSL_ECH_SIGNAL_LEN; - memcpy(acbuf, acp, OSSL_ECH_SIGNAL_LEN); - return 1; - } else { - if (!PACKET_buf_init(&pkt, shbuf, shlen) - || !PACKET_get_net_2(&pkt, &pi_tmp) - || !PACKET_get_bytes(&pkt, &pp_tmp, SSL3_RANDOM_SIZE) - || !PACKET_get_1(&pkt, &pi_tmp) /* sessid len */ - || !PACKET_get_bytes(&pkt, &pp_tmp, pi_tmp) /* sessid */ - || !PACKET_get_net_2(&pkt, &pi_tmp) /* ciphersuite */ - || !PACKET_get_1(&pkt, &pi_tmp) /* compression */ - || !PACKET_get_net_2(&pkt, &pi_tmp)) /* len(extensions) */ + } else { /* was set in extension handler */ + if (s->ext.ech.hrrsignal_p == NULL) return 0; - while (PACKET_remaining(&pkt) > 0 && done < 1) { - if (!PACKET_get_net_2(&pkt, &etype) - || !PACKET_get_net_2(&pkt, &elen)) - return 0; - if (etype == TLSEXT_TYPE_ech) { - if (elen != OSSL_ECH_SIGNAL_LEN - || !PACKET_get_bytes(&pkt, &acp, elen)) - return 0; - memcpy(acbuf, acp, elen); - done++; - } else { - if (!PACKET_get_bytes(&pkt, &pp_tmp, elen)) - return 0; - } - } - return done; - } - return 0; -} - -/* - * make up a buffer to use to reset transcript - * for_hrr is 1 if we've just seen HRR, 0 otherwise - * shbuf is the output buffer - * shlen is the length of that buffer - * tbuf is the output buffer - * tlen is the length of that buffer - * chend returns the offset of the end of the last CH in the buffer - * fixedshbuf_len returns the fixed up length of the SH - * return 1 for good, 0 otherwise - */ -int ossl_ech_make_transcript_buffer(SSL_CONNECTION *s, int for_hrr, - const unsigned char *shbuf, size_t shlen, - unsigned char **tbuf, size_t *tlen, - size_t *chend, size_t *fixedshbuf_len) -{ - unsigned char *fixedshbuf = NULL, *hashin = NULL, hashval[EVP_MAX_MD_SIZE]; - unsigned int hashlen = 0, hashin_len = 0; - EVP_MD_CTX *ctx = NULL; - EVP_MD *md = NULL; - WPACKET tpkt = { 0 }, shpkt = { 0 }; - BUF_MEM *tpkt_mem = NULL, *shpkt_mem = NULL; - - /* - * SH preamble has bad length at this point on server - * and is missing on client so we'll fix - */ - if ((shpkt_mem = BUF_MEM_new()) == NULL - || !WPACKET_init(&shpkt, shpkt_mem)) { - BUF_MEM_free(shpkt_mem); - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - if (!WPACKET_put_bytes_u8(&shpkt, SSL3_MT_SERVER_HELLO) - || (s->server == 1 && !WPACKET_put_bytes_u24(&shpkt, shlen - 4)) - || (s->server == 1 && !WPACKET_memcpy(&shpkt, shbuf + 4, shlen -4)) - || (s->server == 0 && !WPACKET_put_bytes_u24(&shpkt, shlen)) - || (s->server == 0 && !WPACKET_memcpy(&shpkt, shbuf, shlen)) - || !WPACKET_get_length(&shpkt, fixedshbuf_len)) { - BUF_MEM_free(shpkt_mem); - WPACKET_cleanup(&shpkt); - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - fixedshbuf = (unsigned char *)shpkt_mem->data; - WPACKET_cleanup(&shpkt); -# ifdef OSSL_ECH_SUPERVERBOSE - ossl_ech_pbuf("cx: fixed sh buf", fixedshbuf, *fixedshbuf_len); -# endif - if ((tpkt_mem = BUF_MEM_new()) == NULL - || !WPACKET_init(&tpkt, tpkt_mem)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - if (s->hello_retry_request == SSL_HRR_NONE) { - if (!WPACKET_memcpy(&tpkt, s->ext.ech.innerch, - s->ext.ech.innerch_len) - || !WPACKET_get_length(&tpkt, chend) - || !WPACKET_memcpy(&tpkt, fixedshbuf, *fixedshbuf_len) - || !WPACKET_get_length(&tpkt, tlen)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - *tbuf = OPENSSL_malloc(*tlen); - if (*tbuf == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - memcpy(*tbuf, WPACKET_get_curr(&tpkt) - *tlen, *tlen); - BUF_MEM_free(shpkt_mem); - WPACKET_cleanup(&tpkt); - BUF_MEM_free(tpkt_mem); - return 1; - } - /* everything below only applies if we're at some stage in doing HRR */ - if ((md = (EVP_MD *)ssl_handshake_md(s)) == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - if (for_hrr == 0) { /* after 2nd SH rx'd */ - hashin = s->ext.ech.innerch1; - hashin_len = s->ext.ech.innerch1_len; - } else { /* after HRR rx'd */ - hashin = s->ext.ech.innerch; - hashin_len = s->ext.ech.innerch_len; - OPENSSL_free(s->ext.ech.kepthrr); - /* stash this SH/HRR for later */ - s->ext.ech.kepthrr = OPENSSL_malloc(*fixedshbuf_len); - if (s->ext.ech.kepthrr == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - memcpy(s->ext.ech.kepthrr, fixedshbuf, *fixedshbuf_len); - s->ext.ech.kepthrr_len = *fixedshbuf_len; - } -# ifdef OSSL_ECH_SUPERVERBOSE - ossl_ech_pbuf("cx: ch2hash", hashin, hashin_len); -# endif - if ((ctx = EVP_MD_CTX_new()) == NULL - || EVP_DigestInit_ex(ctx, md, NULL) <= 0 - || EVP_DigestUpdate(ctx, hashin, hashin_len) <= 0 - || EVP_DigestFinal_ex(ctx, hashval, &hashlen) <= 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - EVP_MD_CTX_free(ctx); - ctx = NULL; - if (!WPACKET_put_bytes_u8(&tpkt, SSL3_MT_MESSAGE_HASH) - || !WPACKET_put_bytes_u24(&tpkt, hashlen) - || !WPACKET_memcpy(&tpkt, hashval, hashlen)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - if (for_hrr == 0) { /* after 2nd SH */ - if (!WPACKET_memcpy(&tpkt, s->ext.ech.kepthrr, - s->ext.ech.kepthrr_len) - || !WPACKET_memcpy(&tpkt, s->ext.ech.innerch, - s->ext.ech.innerch_len)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - } - if (!WPACKET_get_length(&tpkt, chend) - || !WPACKET_memcpy(&tpkt, fixedshbuf, *fixedshbuf_len) - || !WPACKET_get_length(&tpkt, tlen)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; + acp = s->ext.ech.hrrsignal; } - *tbuf = OPENSSL_malloc(*tlen); - if (*tbuf == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - memcpy(*tbuf, WPACKET_get_curr(&tpkt) - *tlen, *tlen); - BUF_MEM_free(shpkt_mem); - WPACKET_cleanup(&tpkt); - BUF_MEM_free(tpkt_mem); + memcpy(acbuf, acp, OSSL_ECH_SIGNAL_LEN); return 1; -err: - BUF_MEM_free(shpkt_mem); - BUF_MEM_free(tpkt_mem); - WPACKET_cleanup(&tpkt); - EVP_MD_CTX_free(ctx); - return 0; } /* @@ -744,15 +564,16 @@ int ossl_ech_reset_hs_buffer(SSL_CONNECTION *s, const unsigned char *buf, ossl_ech_pbuf("RESET transcript to", buf, blen); # endif if (s->s3.handshake_buffer != NULL) { + if (BIO_reset(s->s3.handshake_buffer) < 0) + return 0; + } else { + s->s3.handshake_buffer = BIO_new(BIO_s_mem()); + if (s->s3.handshake_buffer == NULL) + return 0; (void)BIO_set_close(s->s3.handshake_buffer, BIO_CLOSE); - BIO_free(s->s3.handshake_buffer); - s->s3.handshake_buffer = NULL; } EVP_MD_CTX_free(s->s3.handshake_dgst); s->s3.handshake_dgst = NULL; - s->s3.handshake_buffer = BIO_new(BIO_s_mem()); - if (s->s3.handshake_buffer == NULL) - return 0; /* providing nothing at all is a real use (mid-HRR) */ if (buf != NULL && blen > 0) BIO_write(s->s3.handshake_buffer, (void *)buf, (int)blen); @@ -778,7 +599,7 @@ int ossl_ech_reset_hs_buffer(SSL_CONNECTION *s, const unsigned char *buf, * better really, BUT... we might want to keep this if others (e.g. * browsers) do it so as to not stand out compared to them. * - * The "+ 9" constant below is from the specifiation and is the + * The "+ 9" constant below is from the specification and is the * expansion comparing a string length to an encoded SNI extension. * Same is true of the 31/32 formula below. * @@ -786,11 +607,12 @@ int ossl_ech_reset_hs_buffer(SSL_CONNECTION *s, const unsigned char *buf, * a padded cleartext of 128 octets, the ciphertext will be 144 * octets. */ -static size_t ech_calc_padding(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY *ee) +size_t ossl_ech_calc_padding(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY *ee, + size_t encoded_len) { int length_of_padding = 0, length_with_snipadding = 0; int innersnipadding = 0, length_with_padding = 0; - size_t mnl = 0, clear_len = 0, isnilen = 0; + size_t mnl = 0, isnilen = 0; if (s == NULL || ee == NULL) return 0; @@ -805,10 +627,9 @@ static size_t ech_calc_padding(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY *ee) } } /* padding is after the inner client hello has been encoded */ - length_with_snipadding = innersnipadding + s->ext.ech.encoded_innerch_len; + length_with_snipadding = innersnipadding + encoded_len; length_of_padding = 31 - ((length_with_snipadding - 1) % 32); - length_with_padding = s->ext.ech.encoded_innerch_len - + length_of_padding + innersnipadding; + length_with_padding = encoded_len + length_of_padding + innersnipadding; /* * Finally - make sure final result is longer than padding target * and a multiple of our padding increment. @@ -816,20 +637,18 @@ static size_t ech_calc_padding(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY *ee) * it makes us stick out; or if we take out the above more (uselessly:-) * complicated scheme, we may only need this in the end. */ - if (length_with_padding % OSSL_ECH_PADDING_INCREMENT) + if ((length_with_padding % OSSL_ECH_PADDING_INCREMENT) != 0) length_with_padding += OSSL_ECH_PADDING_INCREMENT - (length_with_padding % OSSL_ECH_PADDING_INCREMENT); while (length_with_padding < OSSL_ECH_PADDING_TARGET) length_with_padding += OSSL_ECH_PADDING_INCREMENT; - clear_len = length_with_padding; OSSL_TRACE_BEGIN(TLS) { BIO_printf(trc_out, "EAAE: padding: mnl: %zu, lws: %d " - "lop: %d, lwp: %d, clear_len: %zu, orig: %zu\n", + "lop: %d, clear_len (len with padding): %d, orig: %zu\n", mnl, length_with_snipadding, length_of_padding, - length_with_padding, clear_len, - s->ext.ech.encoded_innerch_len); + length_with_padding, encoded_len); } OSSL_TRACE_END(TLS); - return clear_len; + return (size_t)length_with_padding; } /* @@ -844,13 +663,11 @@ static size_t ech_calc_padding(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY *ee) */ int ossl_ech_aad_and_encrypt(SSL_CONNECTION *s, WPACKET *pkt) { - int rv = 0, hpke_mode = OSSL_HPKE_MODE_BASE; - OSSL_ECHSTORE_ENTRY *ee = NULL; - OSSL_HPKE_SUITE hpke_suite = OSSL_HPKE_SUITE_DEFAULT; - unsigned char config_id_to_use = 0x00, info[SSL3_RT_MAX_PLAIN_LENGTH]; - unsigned char *clear = NULL, *cipher = NULL, *aad = NULL, *mypub = NULL; - size_t cipherlen = 0, aad_len = 0, lenclen = 0, mypub_len = 0; - size_t info_len = SSL3_RT_MAX_PLAIN_LENGTH, clear_len = 0; + int rv = 0; + size_t cipherlen = 0, aad_len = 0, mypub_len = 0, clear_len = 0; + size_t encoded_inner_len = 0; + unsigned char *clear = NULL, *aad = NULL, *mypub = NULL; + unsigned char *encoded_inner = NULL, *cipher_loc = NULL; if (s == NULL) return 0; @@ -859,124 +676,19 @@ int ossl_ech_aad_and_encrypt(SSL_CONNECTION *s, WPACKET *pkt) SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } - rv = ossl_ech_pick_matching_cfg(s, &ee, &hpke_suite); - if (rv != 1 || ee == NULL) { + /* values calculated in tls_construct_ctos_ech */ + encoded_inner = s->ext.ech.encoded_inner; + encoded_inner_len = s->ext.ech.encoded_inner_len; + clear_len = s->ext.ech.clearlen; + cipherlen = s->ext.ech.cipherlen; + if (!WPACKET_get_total_written(pkt, &aad_len) || aad_len < 4) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } - s->ext.ech.attempted_type = ee->version; - OSSL_TRACE_BEGIN(TLS) { - BIO_printf(trc_out, "EAAE: selected: version: %4x, config %2x\n", - ee->version, ee->config_id); - } OSSL_TRACE_END(TLS); - config_id_to_use = ee->config_id; /* if requested, use a random config_id instead */ - if ((s->ssl.ctx->options & SSL_OP_ECH_IGNORE_CID) != 0 - || (s->options & SSL_OP_ECH_IGNORE_CID) != 0) { - if (RAND_bytes_ex(s->ssl.ctx->libctx, &config_id_to_use, 1, - RAND_DRBG_STRENGTH) <= 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; - } -# ifdef OSSL_ECH_SUPERVERBOSE - ossl_ech_pbuf("EAAE: random config_id", &config_id_to_use, 1); -# endif - } - s->ext.ech.attempted_cid = config_id_to_use; -# ifdef OSSL_ECH_SUPERVERBOSE - ossl_ech_pbuf("EAAE: peer pub", ee->pub, ee->pub_len); - ossl_ech_pbuf("EAAE: clear", s->ext.ech.encoded_innerch, - s->ext.ech.encoded_innerch_len); - ossl_ech_pbuf("EAAE: ECHConfig", ee->encoded, ee->encoded_len); -# endif - /* - * The AAD is the full outer client hello but with the correct number of - * zeros for where the ECH ciphertext octets will later be placed. So we - * add the ECH extension to the |pkt| but with zeros for ciphertext, that - * forms up the AAD, then after we've encrypted, we'll splice in the actual - * ciphertext. - * Watch out for the "4" offsets that remove the type and 3-octet length - * from the encoded CH as per the spec. - */ - clear_len = ech_calc_padding(s, ee); - if (clear_len == 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - lenclen = OSSL_HPKE_get_public_encap_size(hpke_suite); - if (s->ext.ech.hpke_ctx == NULL) { /* 1st CH */ - if (ossl_ech_make_enc_info(ee->encoded, ee->encoded_len, - info, &info_len) != 1) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } -# ifdef OSSL_ECH_SUPERVERBOSE - ossl_ech_pbuf("EAAE info", info, info_len); -# endif - s->ext.ech.hpke_ctx = OSSL_HPKE_CTX_new(hpke_mode, hpke_suite, - OSSL_HPKE_ROLE_SENDER, - NULL, NULL); - if (s->ext.ech.hpke_ctx == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - mypub = OPENSSL_malloc(lenclen); - if (mypub == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - mypub_len = lenclen; - rv = OSSL_HPKE_encap(s->ext.ech.hpke_ctx, mypub, &mypub_len, - ee->pub, ee->pub_len, info, info_len); - if (rv != 1) { - OPENSSL_free(mypub); - mypub = NULL; - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - s->ext.ech.pub = mypub; - s->ext.ech.pub_len = mypub_len; - } else { /* HRR - retrieve public */ - mypub = s->ext.ech.pub; - mypub_len = s->ext.ech.pub_len; - if (mypub == NULL || mypub_len == 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - } -# ifdef OSSL_ECH_SUPERVERBOSE - ossl_ech_pbuf("EAAE: mypub", mypub, mypub_len); - WPACKET_get_total_written(pkt, &aad_len); /* use aad_len for tracing */ - ossl_ech_pbuf("EAAE pkt b4", WPACKET_get_curr(pkt) - aad_len, aad_len); -# endif - cipherlen = OSSL_HPKE_get_ciphertext_size(hpke_suite, clear_len); - if (cipherlen <= clear_len || cipherlen > OSSL_ECH_MAX_PAYLOAD_LEN) { - SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION); - goto err; - } - cipher = OPENSSL_zalloc(cipherlen); - if (cipher == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_ech) - || !WPACKET_start_sub_packet_u16(pkt) - || !WPACKET_put_bytes_u8(pkt, OSSL_ECH_OUTER_CH_TYPE) - || !WPACKET_put_bytes_u16(pkt, hpke_suite.kdf_id) - || !WPACKET_put_bytes_u16(pkt, hpke_suite.aead_id) - || !WPACKET_put_bytes_u8(pkt, config_id_to_use) - || (s->hello_retry_request == SSL_HRR_PENDING - && !WPACKET_put_bytes_u16(pkt, 0x00)) /* no pub */ - || (s->hello_retry_request != SSL_HRR_PENDING - && !WPACKET_sub_memcpy_u16(pkt, mypub, mypub_len)) - || !WPACKET_sub_memcpy_u16(pkt, cipher, cipherlen) - || !WPACKET_close(pkt) - || !WPACKET_get_total_written(pkt, &aad_len) - || aad_len < 4) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - aad_len -= 4; /* aad starts after type + 3-octet len */ + aad_len -= 4; /* ECH/HPKE aad starts after type + 3-octet len */ aad = WPACKET_get_curr(pkt) - aad_len; + /* where we'll replace zeros with ciphertext */ + cipher_loc = aad + s->ext.ech.cipher_offset; /* * close the extensions of the CH - we skipped doing this * earlier when encoding extensions, to allow for adding the @@ -995,32 +707,29 @@ int ossl_ech_aad_and_encrypt(SSL_CONNECTION *s, WPACKET *pkt) SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } - memcpy(clear, s->ext.ech.encoded_innerch, s->ext.ech.encoded_innerch_len); + memcpy(clear, encoded_inner, encoded_inner_len); # ifdef OSSL_ECH_SUPERVERBOSE ossl_ech_pbuf("EAAE: padded clear", clear, clear_len); # endif - rv = OSSL_HPKE_seal(s->ext.ech.hpke_ctx, cipher, &cipherlen, - aad, aad_len, clear, clear_len); + /* we're done with this now */ + OPENSSL_free(s->ext.ech.encoded_inner); + s->ext.ech.encoded_inner = NULL; + rv = OSSL_HPKE_seal(s->ext.ech.hpke_ctx, cipher_loc, + &cipherlen, aad, aad_len, clear, clear_len); OPENSSL_free(clear); if (rv != 1) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } # ifdef OSSL_ECH_SUPERVERBOSE - ossl_ech_pbuf("EAAE: cipher", cipher, cipherlen); + ossl_ech_pbuf("EAAE: cipher", cipher_loc, cipherlen); ossl_ech_pbuf("EAAE: hpke mypub", mypub, mypub_len); -# endif - /* splice real ciphertext back in now */ - memcpy(aad + aad_len - cipherlen, cipher, cipherlen); -# ifdef OSSL_ECH_SUPERVERBOSE /* re-use aad_len for tracing */ WPACKET_get_total_written(pkt, &aad_len); ossl_ech_pbuf("EAAE pkt aftr", WPACKET_get_curr(pkt) - aad_len, aad_len); # endif - OPENSSL_free(cipher); return 1; err: - OPENSSL_free(cipher); return 0; } @@ -1029,7 +738,7 @@ err: * out is the BIO to use (e.g. stdout/whatever) * selector OSSL_ECH_SELECT_ALL or just one of the SSL_ECH values */ -static void ech_status_print(BIO *out, SSL_CONNECTION *s, int selector) +void ossl_ech_status_print(BIO *out, SSL_CONNECTION *s, int selector) { int num = 0, i, has_priv, for_retry; size_t j; @@ -1091,17 +800,14 @@ static void ech_status_print(BIO *out, SSL_CONNECTION *s, int selector) return; } -/* size of string buffer returned via ECH callback */ -# define OSSL_ECH_PBUF_SIZE 8 * 1024 - /* * Swap the inner and outer after ECH success on the client * return 0 for error, 1 for success */ int ossl_ech_swaperoo(SSL_CONNECTION *s) { - unsigned char *curr_buf = NULL, *new_buf = NULL; - size_t curr_buflen = 0, new_buflen = 0, outer_chlen = 0, other_octets = 0; + unsigned char *curr_buf = NULL; + size_t curr_buflen = 0; if (s == NULL) return 0; @@ -1118,69 +824,49 @@ int ossl_ech_swaperoo(SSL_CONNECTION *s) s->s3.group_id = s->ext.ech.group_id; s->ext.ech.tmp_pkey = NULL; /* - * TODO(ECH): I suggest re-factoring transcript handling (which - * is probably needed) after/with the PR that includes the server- - * side ECH code. That should be much easier as at that point the - * full set of tests can be run, whereas for now, we're limited - * to testing the client side really works via bodged s_client - * scripts, so there'd be a bigger risk of breaking something - * subtly if we try re-factor now. - * * When not doing HRR... fix up the transcript to reflect the inner CH. * If there's a client hello at the start of the buffer, then that's * the outer CH and we want to replace that with the inner. We need to - * be careful that there could be a server hello following and can't - * lose that. + * be careful that there could be early data or a server hello following + * and we can't lose that. * * For HRR... HRR processing code has already done the necessary. */ if (s->hello_retry_request == SSL_HRR_NONE) { - curr_buflen = BIO_get_mem_data(s->s3.handshake_buffer, - &curr_buf); - if (curr_buflen > 4 && curr_buf[0] == SSL3_MT_CLIENT_HELLO) { - outer_chlen = 1 + curr_buf[1] * 256 * 256 - + curr_buf[2] * 256 + curr_buf[3]; - if (outer_chlen > curr_buflen) { + BIO *handbuf = s->s3.handshake_buffer; + PACKET pkt, subpkt; + unsigned int mt; + + s->s3.handshake_buffer = NULL; + if (ssl3_init_finished_mac(s) == 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + BIO_free(handbuf); + return 0; + } + if (ssl3_finish_mac(s, s->ext.ech.innerch, s->ext.ech.innerch_len) == 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + BIO_free(handbuf); + return 0; + } + curr_buflen = BIO_get_mem_data(handbuf, &curr_buf); + if (PACKET_buf_init(&pkt, curr_buf, curr_buflen) + && PACKET_get_1(&pkt, &mt) + && mt == SSL3_MT_CLIENT_HELLO + && PACKET_remaining(&pkt) >= 3) { + if (!PACKET_get_length_prefixed_3(&pkt, &subpkt)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + BIO_free(handbuf); return 0; } - other_octets = curr_buflen - outer_chlen; - if (other_octets > 0) { - new_buflen = s->ext.ech.innerch_len + other_octets; - new_buf = OPENSSL_malloc(new_buflen); - if (new_buf == NULL) { + if (PACKET_remaining(&pkt) > 0) { + if (ssl3_finish_mac(s, PACKET_data(&pkt), PACKET_remaining(&pkt)) == 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + BIO_free(handbuf); return 0; } - if (s->ext.ech.innerch != NULL) /* asan check added */ - memcpy(new_buf, s->ext.ech.innerch, - s->ext.ech.innerch_len); - memcpy(new_buf + s->ext.ech.innerch_len, - &curr_buf[outer_chlen], other_octets); - } else { - new_buf = s->ext.ech.innerch; - new_buflen = s->ext.ech.innerch_len; } - } else { - new_buf = s->ext.ech.innerch; - new_buflen = s->ext.ech.innerch_len; + BIO_free(handbuf); } - /* - * And now reset the handshake transcript to our buffer - * Note ssl3_finish_mac isn't that great a name - that one just - * adds to the transcript but doesn't actually "finish" anything - */ - if (ssl3_init_finished_mac(s) == 0) { - OPENSSL_free(new_buf); - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; - } - if (ssl3_finish_mac(s, new_buf, new_buflen) == 0) { - OPENSSL_free(new_buf); - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; - } - OPENSSL_free(new_buf); } # ifdef OSSL_ECH_SUPERVERBOSE ossl_ech_ptranscript(s, "ech_swaperoo, after"); @@ -1202,7 +888,7 @@ int ossl_ech_swaperoo(SSL_CONNECTION *s) SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } - ech_status_print(biom, s, OSSL_ECHSTORE_ALL); + ossl_ech_status_print(biom, s, OSSL_ECHSTORE_ALL); BIO_read(biom, pstr, OSSL_ECH_PBUF_SIZE); cbrv = s->ext.ech.cb(&s->ssl, pstr); BIO_free(biom); @@ -1235,10 +921,11 @@ static int ech_hkdf_extract_wrap(SSL_CONNECTION *s, EVP_MD *md, int for_hrr, if (for_hrr == 1) { label = OSSL_ECH_HRR_CONFIRM_STRING; + labellen = sizeof(OSSL_ECH_HRR_CONFIRM_STRING) - 1; } else { label = OSSL_ECH_ACCEPT_CONFIRM_STRING; + labellen = sizeof(OSSL_ECH_ACCEPT_CONFIRM_STRING) - 1; } - labellen = strlen(label); # ifdef OSSL_ECH_SUPERVERBOSE ossl_ech_pbuf("cc: label", (unsigned char *)label, labellen); # endif @@ -1247,8 +934,6 @@ static int ech_hkdf_extract_wrap(SSL_CONNECTION *s, EVP_MD *md, int for_hrr, pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, NULL); if (pctx == NULL || EVP_PKEY_derive_init(pctx) != 1 - || EVP_PKEY_CTX_hkdf_mode(pctx, - EVP_PKEY_HKDEF_MODE_EXTRACT_ONLY) != 1 || EVP_PKEY_CTX_hkdf_mode(pctx, EVP_PKEY_HKDEF_MODE_EXTRACT_ONLY) != 1 || EVP_PKEY_CTX_set_hkdf_md(pctx, md) != 1) { @@ -1291,9 +976,8 @@ err: /* * ECH accept_confirmation calculation * for_hrr is 1 if this is for an HRR, otherwise for SH - * ac is (a caller allocated) 8 octet buffer - * shbuf is a pointer to the SH buffer (incl. the type+3-octet length) - * shlen is the length of the SH buf + * acbuf is an 8 octet buffer for the confirmation value + * shlen is the server hello length * return: 1 for success, 0 otherwise * * This is a magic value in the ServerHello.random lower 8 octets @@ -1314,7 +998,7 @@ err: */ int ossl_ech_calc_confirm(SSL_CONNECTION *s, int for_hrr, unsigned char acbuf[OSSL_ECH_SIGNAL_LEN], - const unsigned char *shbuf, const size_t shlen) + const size_t shlen) { int rv = 0; EVP_MD_CTX *ctx = NULL; @@ -1322,18 +1006,28 @@ int ossl_ech_calc_confirm(SSL_CONNECTION *s, int for_hrr, unsigned char *tbuf = NULL, *conf_loc = NULL; unsigned char *fixedshbuf = NULL; size_t fixedshbuf_len = 0, tlen = 0, chend = 0; - size_t shoffset = 6 + 24, extoffset = 0, echoffset = 0; - uint16_t echtype; + /* shoffset is: 4 + 2 + 32 - 8 */ + size_t shoffset = SSL3_HM_HEADER_LENGTH + sizeof(uint16_t) + + SSL3_RANDOM_SIZE - OSSL_ECH_SIGNAL_LEN; unsigned int hashlen = 0; - unsigned char hashval[EVP_MAX_MD_SIZE], hoval[EVP_MAX_MD_SIZE]; + unsigned char hashval[EVP_MAX_MD_SIZE]; - if ((md = (EVP_MD *)ssl_handshake_md(s)) == NULL) - goto err; - if (ossl_ech_make_transcript_buffer(s, for_hrr, shbuf, shlen, &tbuf, &tlen, - &chend, &fixedshbuf_len) != 1) - goto err; /* SSLfatal called already */ + if ((md = (EVP_MD *)ssl_handshake_md(s)) == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_ECH_REQUIRED); + goto end; + } + if (ossl_ech_intbuf_fetch(s, &tbuf, &tlen) != 1) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_ECH_REQUIRED); + goto end; + } + chend = tlen - shlen - 4; + fixedshbuf_len = shlen + 4; + if (s->server) { + chend = tlen - shlen; + fixedshbuf_len = shlen; + } # ifdef OSSL_ECH_SUPERVERBOSE - ossl_ech_pbuf("cx: tbuf b4", tbuf, tlen); + ossl_ech_pbuf("cx: tbuf b4-b4", tbuf, tlen); # endif /* put zeros in correct place */ if (for_hrr == 0) { /* zap magic octets at fixed place for SH */ @@ -1342,24 +1036,17 @@ int ossl_ech_calc_confirm(SSL_CONNECTION *s, int for_hrr, if (s->server == 1) { /* we get to say where we put ECH:-) */ conf_loc = tbuf + tlen - OSSL_ECH_SIGNAL_LEN; } else { - if (ossl_ech_get_sh_offsets(shbuf, shlen, &extoffset, - &echoffset, &echtype) != 1) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_ECH_REQUIRED); - goto err; - } - if (echoffset == 0 || extoffset == 0 || echtype == 0 - || tlen < (chend + 4 + echoffset + 4 + OSSL_ECH_SIGNAL_LEN)) { + if (s->ext.ech.hrrsignal_p == NULL) { /* No ECH found so we'll exit, but set random output */ if (RAND_bytes_ex(s->ssl.ctx->libctx, acbuf, - OSSL_ECH_SIGNAL_LEN, - RAND_DRBG_STRENGTH) <= 0) { + OSSL_ECH_SIGNAL_LEN, 0) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_ECH_REQUIRED); - goto err; + goto end; } rv = 1; - goto err; + goto end; } - conf_loc = tbuf + chend + 4 + echoffset + 4; + conf_loc = s->ext.ech.hrrsignal_p; } } memset(conf_loc, 0, OSSL_ECH_SIGNAL_LEN); @@ -1371,37 +1058,95 @@ int ossl_ech_calc_confirm(SSL_CONNECTION *s, int for_hrr, || EVP_DigestUpdate(ctx, tbuf, tlen) <= 0 || EVP_DigestFinal_ex(ctx, hashval, &hashlen) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; + goto end; } EVP_MD_CTX_free(ctx); ctx = NULL; # ifdef OSSL_ECH_SUPERVERBOSE ossl_ech_pbuf("cx: hashval", hashval, hashlen); # endif - if (ech_hkdf_extract_wrap(s, md, for_hrr, hashval, hashlen, hoval) != 1) { + /* calculate and set the final output */ + if (ech_hkdf_extract_wrap(s, md, for_hrr, hashval, hashlen, acbuf) != 1) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; + goto end; } - memcpy(acbuf, hoval, OSSL_ECH_SIGNAL_LEN); /* Finally, set the output */ # ifdef OSSL_ECH_SUPERVERBOSE ossl_ech_pbuf("cx: result", acbuf, OSSL_ECH_SIGNAL_LEN); # endif - /* put confirm value back into transcript vars */ - if (s->hello_retry_request != SSL_HRR_NONE && s->ext.ech.kepthrr != NULL - && for_hrr == 1 && s->server == 1) - memcpy(s->ext.ech.kepthrr + s->ext.ech.kepthrr_len - - OSSL_ECH_SIGNAL_LEN, acbuf, OSSL_ECH_SIGNAL_LEN); - memcpy(conf_loc, acbuf, OSSL_ECH_SIGNAL_LEN); + /* put confirm value back into transcript */ + if (s->ext.ech.hrrsignal_p == NULL) + memcpy(conf_loc, acbuf, OSSL_ECH_SIGNAL_LEN); + else + memcpy(conf_loc, s->ext.ech.hrrsignal, 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); if (s->server && s->hello_retry_request == SSL_HRR_COMPLETE) ossl_ech_reset_hs_buffer(s, tbuf, tlen - fixedshbuf_len); rv = 1; -err: +end: OPENSSL_free(fixedshbuf); - OPENSSL_free(tbuf); EVP_MD_CTX_free(ctx); return rv; } + +int ossl_ech_intbuf_add(SSL_CONNECTION *s, const unsigned char *buf, + size_t blen, int hash_existing) +{ + EVP_MD_CTX *ctx = NULL; + EVP_MD *md = NULL; + unsigned int rv = 0, hashlen = 0; + unsigned char hashval[EVP_MAX_MD_SIZE], *t1; + size_t tlen; + WPACKET tpkt = { 0 }; + BUF_MEM *tpkt_mem = NULL; + + if (s == NULL || buf == NULL || blen == 0) + goto err; + if (hash_existing == 1) { + /* hash existing buffer, needed during HRR */ + if (s->ext.ech.transbuf == NULL + || (md = (EVP_MD *)ssl_handshake_md(s)) == NULL + || (ctx = EVP_MD_CTX_new()) == NULL + || EVP_DigestInit_ex(ctx, md, NULL) <= 0 + || EVP_DigestUpdate(ctx, s->ext.ech.transbuf, + s->ext.ech.transbuf_len) <= 0 + || EVP_DigestFinal_ex(ctx, hashval, &hashlen) <= 0 + || (tpkt_mem = BUF_MEM_new()) == NULL + || !WPACKET_init(&tpkt, tpkt_mem) + || !WPACKET_put_bytes_u8(&tpkt, SSL3_MT_MESSAGE_HASH) + || !WPACKET_put_bytes_u24(&tpkt, hashlen) + || !WPACKET_memcpy(&tpkt, hashval, hashlen) + || !WPACKET_get_length(&tpkt, &tlen) + || (t1 = OPENSSL_realloc(s->ext.ech.transbuf, tlen + blen)) == NULL) + goto err; + s->ext.ech.transbuf = t1; + memcpy(s->ext.ech.transbuf, tpkt_mem->data, tlen); + memcpy(s->ext.ech.transbuf + tlen, buf, blen); + s->ext.ech.transbuf_len = tlen + blen; + } else { + /* just add new octets */ + if ((t1 = OPENSSL_realloc(s->ext.ech.transbuf, + s->ext.ech.transbuf_len + blen)) == NULL) + goto err; + s->ext.ech.transbuf = t1; + memcpy(s->ext.ech.transbuf + s->ext.ech.transbuf_len, buf, blen); + s->ext.ech.transbuf_len += blen; + } + rv = 1; +err: + BUF_MEM_free(tpkt_mem); + WPACKET_cleanup(&tpkt); + EVP_MD_CTX_free(ctx); + return rv; +} + +int ossl_ech_intbuf_fetch(SSL_CONNECTION *s, unsigned char **buf, size_t *blen) +{ + if (s == NULL || buf == NULL || blen == NULL || s->ext.ech.transbuf == NULL) + return 0; + *buf = s->ext.ech.transbuf; + *blen = s->ext.ech.transbuf_len; + return 1; +} #endif diff --git a/ssl/ech/ech_local.h b/ssl/ech/ech_local.h index fdf7d45b0f6..f2ecb4aee20 100644 --- a/ssl/ech/ech_local.h +++ b/ssl/ech/ech_local.h @@ -48,6 +48,9 @@ # define OSSL_ECH_SIGNAL_LEN 8 /* length of ECH acceptance signal */ +/* size of string buffer returned via ECH callback */ +# define OSSL_ECH_PBUF_SIZE 8 * 1024 + # ifndef CLIENT_VERSION_LEN /* * This is the legacy version length, i.e. len(0x0303). The same @@ -55,6 +58,7 @@ * defined in a header file I could find. */ # define CLIENT_VERSION_LEN 2 + # endif /* @@ -147,37 +151,20 @@ typedef struct ossl_ech_conn_st { * the value we tried as the inner SNI for debug purposes */ char *former_inner; - /* - * TODO(ECH): The next 4 buffers (and lengths) may change if a - * better way to handle the mutiple transcripts needed is - * suggested/invented. I suggest re-factoring transcript handling - * (which is probably needed) after/with the PR that includes the - * server-side ECH code. That should be much easier as at that point - * the full set of tests can be run, whereas for now, we're limited - * to testing the client side really works via bodged s_client - * scripts, so there'd be a bigger risk of breaking something - * subtly if we try re-factor now. - */ - /* - * encoded inner ClientHello before/after ECH compression, which` - * is nitty/complex (to avoid repeating the same extension value - * in outer and inner, thus saving bandwidth) but (re-)calculating - * the compression is a pain, so we'll store those as we make them - */ - unsigned char *innerch; /* before compression */ + /* inner CH transcript buffer */ + unsigned char *transbuf; + size_t transbuf_len; + /* inner ClientHello before ECH compression */ + unsigned char *innerch; size_t innerch_len; - unsigned char *encoded_innerch; /* after compression */ - size_t encoded_innerch_len; - /* - * in case of HRR, we need to record the 1st inner client hello, and - * the first server hello (aka the HRR) so we can independently - * generate the transcript and accept confirmation when making the - * 2nd server hello - */ - unsigned char *innerch1; - size_t innerch1_len; - unsigned char *kepthrr; - size_t kepthrr_len; + /* encoded inner CH */ + unsigned char *encoded_inner; + size_t encoded_inner_len; + /* lengths calculated early, used when encrypting at end of processing */ + size_t clearlen; + size_t cipherlen; + /* location to put ciphertext, initially filled with zeros */ + size_t cipher_offset; /* * Extensions are "outer-only" if the value is only sent in the * outer CH and only the type is sent in the inner CH. @@ -202,6 +189,8 @@ typedef struct ossl_ech_conn_st { uint16_t attempted_type; /* ECH version used */ int attempted_cid; /* ECH config id sent/rx'd */ int backend; /* 1 if we're a server backend in split-mode, 0 otherwise */ + /* When using a PSK stash the tick_identity from inner, for outer */ + int tick_identity; /* * success is 1 if ECH succeeded, 0 otherwise, on the server this * is known early, on the client we need to wait for the ECH confirm @@ -217,6 +206,14 @@ typedef struct ossl_ech_conn_st { unsigned char *pub; /* client ephemeral public kept by server in case HRR */ size_t pub_len; OSSL_HPKE_CTX *hpke_ctx; /* HPKE context, needed for HRR */ + /* + * A pointer to, and copy of, the hrrsignal from an HRR message. + * We need both, as we zero-out the octets when re-calculating and + * may need to put back what the server included so the transcript + * is correct when ECH acceptance failed. + */ + unsigned char *hrrsignal_p; + unsigned char hrrsignal[OSSL_ECH_SIGNAL_LEN]; /* * Fields that differ on client between inner and outer that we need to * keep and swap over IFF ECH has succeeded. Same names chosen as are @@ -233,7 +230,7 @@ typedef struct ossl_ech_conn_st { # define OSSL_ECH_SAME_EXT_CONTINUE 2 /* generate a new value for outer CH */ /* - * During extension construction (in extensions_clnt.c and surprisingly also in + * During extension construction (in extensions_clnt.c, and surprisingly also in * extensions.c), we need to handle inner/outer CH cloning - ossl_ech_same_ext * will (depending on compile time handling options) copy the value from * CH.inner to CH.outer or else processing will continue, for a 2nd call, @@ -273,33 +270,42 @@ OSSL_ECHEXT *ossl_echext_dup(const OSSL_ECHEXT *src); # ifdef OSSL_ECH_SUPERVERBOSE void ossl_ech_pbuf(const char *msg, const unsigned char *buf, const size_t blen); -void ossl_ech_ptranscript(SSL_CONNECTION *s, const char *msg); # endif int ossl_ech_get_retry_configs(SSL_CONNECTION *s, unsigned char **rcfgs, size_t *rcfgslen); int ossl_ech_send_grease(SSL_CONNECTION *s, WPACKET *pkt); int ossl_ech_pick_matching_cfg(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY **ee, OSSL_HPKE_SUITE *suite); -int ossl_ech_encode_inner(SSL_CONNECTION *s); +int ossl_ech_encode_inner(SSL_CONNECTION *s, unsigned char **encoded, + size_t *encoded_len); int ossl_ech_find_confirm(SSL_CONNECTION *s, int hrr, - unsigned char acbuf[OSSL_ECH_SIGNAL_LEN], - const unsigned char *shbuf, const size_t shlen); -int ossl_ech_make_transcript_buffer(SSL_CONNECTION *s, int for_hrr, - const unsigned char *shbuf, size_t shlen, - unsigned char **tbuf, size_t *tlen, - size_t *chend, size_t *fixedshbuf_len); + unsigned char acbuf[OSSL_ECH_SIGNAL_LEN]); int ossl_ech_reset_hs_buffer(SSL_CONNECTION *s, const unsigned char *buf, size_t blen); int ossl_ech_aad_and_encrypt(SSL_CONNECTION *s, WPACKET *pkt); int ossl_ech_swaperoo(SSL_CONNECTION *s); int ossl_ech_calc_confirm(SSL_CONNECTION *s, int for_hrr, unsigned char acbuf[OSSL_ECH_SIGNAL_LEN], - const unsigned char *shbuf, const size_t shlen); + const size_t shlen); /* these are internal but located in ssl/statem/extensions.c */ int ossl_ech_same_ext(SSL_CONNECTION *s, WPACKET *pkt); int ossl_ech_same_key_share(void); int ossl_ech_2bcompressed(int ind); +int ossl_ech_copy_inner2outer(SSL_CONNECTION *s, uint16_t ext_type, int ind, + WPACKET *pkt); + +int ossl_ech_get_ch_offsets(SSL_CONNECTION *s, PACKET *pkt, size_t *sessid, + size_t *exts, size_t *echoffset, uint16_t *echtype, + int *inner, size_t *snioffset); +int ossl_ech_early_decrypt(SSL_CONNECTION *s, PACKET *outerpkt, PACKET *newpkt); +void ossl_ech_status_print(BIO *out, SSL_CONNECTION *s, int selector); + +int ossl_ech_intbuf_add(SSL_CONNECTION *s, const unsigned char *buf, + size_t blen, int hash_existing); +int ossl_ech_intbuf_fetch(SSL_CONNECTION *s, unsigned char **buf, size_t *blen); +size_t ossl_ech_calc_padding(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY *ee, + size_t encoded_len); # endif #endif diff --git a/ssl/ech/ech_ssl_apis.c b/ssl/ech/ech_ssl_apis.c index 7e38ac0e036..601a8ad1621 100644 --- a/ssl/ech/ech_ssl_apis.c +++ b/ssl/ech/ech_ssl_apis.c @@ -185,7 +185,7 @@ int SSL_ech_get1_status(SSL *ssl, char **inner_sni, char **outer_sni) return SSL_ECH_STATUS_GREASE_ECH; return SSL_ECH_STATUS_GREASE; } - if (s->options & SSL_OP_ECH_GREASE) + if ((s->options & SSL_OP_ECH_GREASE) !=0 && s->ext.ech.attempted != 1) return SSL_ECH_STATUS_GREASE; if (s->ext.ech.backend == 1) { if (s->ext.hostname != NULL @@ -292,6 +292,8 @@ int SSL_ech_get1_retry_config(SSL *ssl, unsigned char **ec, size_t *eclen) OSSL_ECHSTORE *ve = NULL; BIO *in = NULL; int rv = 0; + OSSL_LIB_CTX *libctx = NULL; + const char *propq = NULL; s = SSL_CONNECTION_FROM_SSL(ssl); if (s == NULL || ec == NULL || eclen == NULL) @@ -309,10 +311,13 @@ int SSL_ech_get1_retry_config(SSL *ssl, unsigned char **ec, size_t *eclen) * and letting the application see that might cause someone to do an * upgrade. */ + if (s->ext.ech.es != NULL) { + libctx = s->ext.ech.es->libctx; + propq = s->ext.ech.es->propq; + } if ((in = BIO_new(BIO_s_mem())) == NULL || BIO_write(in, s->ext.ech.returned, s->ext.ech.returned_len) <= 0 - || (ve = OSSL_ECHSTORE_new(s->ext.ech.es->libctx, - s->ext.ech.es->propq)) == NULL) { + || (ve = OSSL_ECHSTORE_new(libctx, propq)) == NULL) { ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); goto err; } diff --git a/ssl/ech/ech_store.c b/ssl/ech/ech_store.c index 5d2781fbf4b..c52c124568d 100644 --- a/ssl/ech/ech_store.c +++ b/ssl/ech/ech_store.c @@ -703,8 +703,7 @@ int OSSL_ECHSTORE_new_config(OSSL_ECHSTORE *es, goto err; } /* random config_id */ - if (RAND_bytes_ex(es->libctx, (unsigned char *)&config_id, 1, - RAND_DRBG_STRENGTH) <= 0) { + if (RAND_bytes_ex(es->libctx, (unsigned char *)&config_id, 1, 0) <= 0) { ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); goto err; } @@ -1124,7 +1123,7 @@ int OSSL_ECHSTORE_flush_keys(OSSL_ECHSTORE *es, time_t age) ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT); return 0; } - if (ee->keyshare != NULL && ee->loadtime + age >= now) { + if (ee->keyshare != NULL && ee->loadtime + age <= now) { ossl_echstore_entry_free(ee); sk_OSSL_ECHSTORE_ENTRY_delete(es->entries, i); } diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 87f3dccb987..f0b9cc7033a 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -22,7 +22,7 @@ /* * values for ext_defs ech_handling field * exceptionally, we don't conditionally compile that field to avoid a pile of - * fndefs all over the ext_defs values + * ifndefs all over the ext_defs values */ #define OSSL_ECH_HANDLING_CALL_BOTH 1 /* call constructor both times */ #define OSSL_ECH_HANDLING_COMPRESS 2 /* compress outer value into inner */ @@ -119,8 +119,8 @@ typedef struct extensions_definition_st { */ unsigned int context; /* - * exceptionally, we don't conditionally compile this field to avoid a pile of - * fndefs all over the ext_defs values + * exceptionally, we don't conditionally compile this field to avoid a + * pile of ifndefs all over the ext_defs values */ int ech_handling; /* how to handle ECH for this extension type */ /* @@ -545,8 +545,8 @@ static const EXTENSION_DEFINITION ext_defs[] = { * inner CH must have been pre-decoded into s->clienthello->pre_proc_exts * already. */ -static int ech_copy_inner2outer(SSL_CONNECTION *s, uint16_t ext_type, - int ind, WPACKET *pkt) +int ossl_ech_copy_inner2outer(SSL_CONNECTION *s, uint16_t ext_type, + int ind, WPACKET *pkt) { RAW_EXTENSION *myext = NULL, *raws = NULL; @@ -629,6 +629,7 @@ int ossl_ech_same_ext(SSL_CONNECTION *s, WPACKET *pkt) # endif if (s == NULL || s->ext.ech.es == NULL) return OSSL_ECH_SAME_EXT_CONTINUE; /* nothing to do */ + /* TODO(ECH): we need a better way to handle indexing exts */ tind = s->ext.ech.ext_ind; /* If this index'd extension won't be compressed, we're done */ if (tind < 0 || tind >= nexts) @@ -656,7 +657,7 @@ int ossl_ech_same_ext(SSL_CONNECTION *s, WPACKET *pkt) if (ext_defs[tind].ech_handling == OSSL_ECH_HANDLING_CALL_BOTH) return OSSL_ECH_SAME_EXT_CONTINUE; else - return ech_copy_inner2outer(s, type, tind, pkt); + return ossl_ech_copy_inner2outer(s, type, tind, pkt); } /* just in case - shouldn't happen */ return OSSL_ECH_SAME_EXT_ERR; @@ -1127,12 +1128,12 @@ int tls_construct_extensions(SSL_CONNECTION *s, WPACKET *pkt, #ifndef OPENSSL_NO_ECH /* - * Two passes - we first construct the to-be-ECH-compressed - * extensions, and then go around again constructing those that - * aren't to be ECH-compressed. We need to ensure this ordering - * so that all the ECH-compressed extensions are contiguous - * in the encoding. The actual compression happens later in - * ech_encode_inner(). + * Two passes if doing real ECH - we first construct the + * to-be-ECH-compressed extensions, and then go around again + * constructing those that aren't to be ECH-compressed. We + * need to ensure this ordering so that all the ECH-compressed + * extensions are contiguous in the encoding. The actual + * compression happens later in ech_encode_inner(). */ for (pass = 0; pass <= 1; pass++) #endif @@ -1823,13 +1824,6 @@ int tls_psk_do_binder(SSL_CONNECTION *s, const EVP_MD *md, int ret = -1; int usepskfored = 0; SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(s); -#ifndef OPENSSL_NO_ECH - unsigned char hashval[EVP_MAX_MD_SIZE]; - unsigned int hashlen = 0; - EVP_MD_CTX *ctx = NULL; - WPACKET tpkt; - BUF_MEM *tpkt_mem = NULL; -#endif /* Ensure cast to size_t is safe */ if (!ossl_assert(hashsizei > 0)) { @@ -1914,33 +1908,10 @@ int tls_psk_do_binder(SSL_CONNECTION *s, const EVP_MD *md, #ifndef OPENSSL_NO_ECH /* handle the hashing as per ECH needs (on client) */ if (s->ext.ech.attempted == 1 && s->ext.ech.ch_depth == 1) { - if ((tpkt_mem = BUF_MEM_new()) == NULL - || !BUF_MEM_grow(tpkt_mem, SSL3_RT_MAX_PLAIN_LENGTH) - || !WPACKET_init(&tpkt, tpkt_mem)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - hashlen = EVP_MD_size(md); - if ((ctx = EVP_MD_CTX_new()) == NULL - || EVP_DigestInit_ex(ctx, md, NULL) <= 0 - || EVP_DigestUpdate(ctx, s->ext.ech.innerch1, - s->ext.ech.innerch1_len) <= 0 - || EVP_DigestFinal_ex(ctx, hashval, &hashlen) <= 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } - EVP_MD_CTX_free(ctx); - ctx = NULL; - if (!WPACKET_put_bytes_u8(&tpkt, SSL3_MT_MESSAGE_HASH) - || !WPACKET_put_bytes_u24(&tpkt, hashlen) - || !WPACKET_memcpy(&tpkt, hashval, hashlen) - || !WPACKET_memcpy(&tpkt, s->ext.ech.kepthrr, - s->ext.ech.kepthrr_len) - || !WPACKET_get_length(&tpkt, &hdatalen)) { + if (ossl_ech_intbuf_fetch(s, (unsigned char **)&hdata, &hdatalen) != 1) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } - hdata = WPACKET_get_curr(&tpkt) - hdatalen; } else { #endif hdatalen = hdatalen_l = @@ -2019,14 +1990,6 @@ int tls_psk_do_binder(SSL_CONNECTION *s, const EVP_MD *md, OPENSSL_cleanse(finishedkey, sizeof(finishedkey)); EVP_PKEY_free(mackey); EVP_MD_CTX_free(mctx); -#ifndef OPENSSL_NO_ECH - EVP_MD_CTX_free(ctx); - if (tpkt_mem != NULL) { - WPACKET_cleanup(&tpkt); - BUF_MEM_free(tpkt_mem); - } -#endif - return ret; } diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index 5866fa73216..7251ba80e35 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -14,6 +14,12 @@ #include "statem_local.h" #ifndef OPENSSL_NO_ECH # include +#include "internal/ech_helpers.h" +/* + * the max HPKE 'info' we'll process is the max ECHConfig size + * (OSSL_ECH_MAX_ECHCONFIG_LEN) plus OSSL_ECH_CONTEXT_STRING(len=7) + 1 + */ +#define OSSL_ECH_MAX_INFO_LEN (OSSL_ECH_MAX_ECHCONFIG_LEN + 8) #endif EXT_RETURN tls_construct_ctos_renegotiate(SSL_CONNECTION *s, WPACKET *pkt, @@ -74,8 +80,8 @@ EXT_RETURN tls_construct_ctos_server_name(SSL_CONNECTION *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx) { -#ifndef OPENSSL_NO_ECH char *chosen = s->ext.hostname; +#ifndef OPENSSL_NO_ECH OSSL_HPKE_SUITE suite; OSSL_ECHSTORE_ENTRY *ee = NULL; @@ -96,41 +102,23 @@ EXT_RETURN tls_construct_ctos_server_name(SSL_CONNECTION *s, WPACKET *pkt, chosen = ee->public_name; } } +#endif if (chosen == NULL) return EXT_RETURN_NOT_SENT; /* Add TLS extension servername to the Client Hello message */ - if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_server_name) - || !WPACKET_start_sub_packet_u16(pkt) - || !WPACKET_start_sub_packet_u16(pkt) - || !WPACKET_put_bytes_u8(pkt, TLSEXT_NAMETYPE_host_name) - || !WPACKET_sub_memcpy_u16(pkt, chosen, strlen(chosen)) - || !WPACKET_close(pkt) - || !WPACKET_close(pkt)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return EXT_RETURN_FAIL; - } - return EXT_RETURN_SENT; -#else - if (s->ext.hostname == NULL) - return EXT_RETURN_NOT_SENT; - - /* Add TLS extension servername to the Client Hello message */ if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_server_name) /* Sub-packet for server_name extension */ || !WPACKET_start_sub_packet_u16(pkt) /* Sub-packet for servername list (always 1 hostname)*/ || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_put_bytes_u8(pkt, TLSEXT_NAMETYPE_host_name) - || !WPACKET_sub_memcpy_u16(pkt, s->ext.hostname, - strlen(s->ext.hostname)) + || !WPACKET_sub_memcpy_u16(pkt, chosen, strlen(chosen)) || !WPACKET_close(pkt) || !WPACKET_close(pkt)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; } - return EXT_RETURN_SENT; -#endif } /* Push a Max Fragment Len extension into ClientHello */ @@ -527,12 +515,12 @@ EXT_RETURN tls_construct_ctos_alpn(SSL_CONNECTION *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx) { -#ifndef OPENSSL_NO_ECH - unsigned char *aval = NULL; - size_t alen = 0; -#endif + unsigned char *aval = s->ext.alpn; + size_t alen = s->ext.alpn_len; s->s3.alpn_sent = 0; + if (!SSL_IS_FIRST_HANDSHAKE(s)) + return EXT_RETURN_NOT_SENT; #ifndef OPENSSL_NO_ECH /* * If we have different alpn and alpn_outer values, then we set @@ -545,10 +533,6 @@ EXT_RETURN tls_construct_ctos_alpn(SSL_CONNECTION *s, WPACKET *pkt, * If you don't want the inner value in the outer, you have to * pick what to send in the outer and send that. */ - if (!SSL_IS_FIRST_HANDSHAKE(s)) - return EXT_RETURN_NOT_SENT; - aval = s->ext.alpn; - alen = s->ext.alpn_len; if (s->ext.ech.ch_depth == 1 && s->ext.alpn == NULL) /* inner */ return EXT_RETURN_NOT_SENT; if (s->ext.ech.ch_depth == 0 && s->ext.alpn == NULL @@ -558,30 +542,19 @@ EXT_RETURN tls_construct_ctos_alpn(SSL_CONNECTION *s, WPACKET *pkt, aval = s->ext.ech.alpn_outer; alen = s->ext.ech.alpn_outer_len; } +#endif + if (aval == NULL) + return EXT_RETURN_NOT_SENT; if (!WPACKET_put_bytes_u16(pkt, - TLSEXT_TYPE_application_layer_protocol_negotiation) + TLSEXT_TYPE_application_layer_protocol_negotiation) + /* Sub-packet ALPN extension */ || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_sub_memcpy_u16(pkt, aval, alen) || !WPACKET_close(pkt)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; } -#else - if (s->ext.alpn == NULL || !SSL_IS_FIRST_HANDSHAKE(s)) - return EXT_RETURN_NOT_SENT; - - if (!WPACKET_put_bytes_u16(pkt, - TLSEXT_TYPE_application_layer_protocol_negotiation) - /* Sub-packet ALPN extension */ - || !WPACKET_start_sub_packet_u16(pkt) - || !WPACKET_sub_memcpy_u16(pkt, s->ext.alpn, s->ext.alpn_len) - || !WPACKET_close(pkt)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return EXT_RETURN_FAIL; - } -#endif s->s3.alpn_sent = 1; - return EXT_RETURN_SENT; } @@ -813,7 +786,10 @@ static int add_key_share(SSL_CONNECTION *s, WPACKET *pkt, unsigned int group_id, # ifndef OPENSSL_NO_ECH if (s->ext.ech.ch_depth == 1) { /* stash inner */ - EVP_PKEY_up_ref(key_share_key); + if (EVP_PKEY_up_ref(key_share_key) != 1) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } EVP_PKEY_free(s->ext.ech.tmp_pkey); s->ext.ech.tmp_pkey = key_share_key; s->ext.ech.group_id = group_id; @@ -1167,6 +1143,9 @@ 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); +#endif /* * Add padding to workaround bugs in F5 terminators. See RFC7685. @@ -1288,6 +1267,19 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt, goto dopsksess; } +#ifndef OPENSSL_NO_ECH + /* + * When doing ECH, we get here twice (for inner then outer). The + * 2nd time (for outer) we can skip some checks as we know how + * those went last time. + */ + if (s->ext.ech.es != NULL && s->ext.ech.ch_depth == 0) { + s->ext.tick_identity = s->ext.ech.tick_identity; + dores = (s->ext.tick_identity > 0); + goto dopsksess; + } +#endif + /* * Technically the C standard just says time() returns a time_t and says * nothing about the encoding of that type. In practice most @@ -1298,6 +1290,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt, */ t = ossl_time_subtract(ossl_time_now(), s->session->time); agesec = (uint32_t)ossl_time2seconds(t); + /* * We calculate the age in seconds but the server may work in ms. Due to * rounding errors we could overestimate the age by up to 1s. It is @@ -1338,6 +1331,11 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt, if (reshashsize <= 0) goto dopsksess; s->ext.tick_identity++; +#ifndef OPENSSL_NO_ECH + /* stash this for re-use in outer CH */ + if (s->ext.ech.es != NULL && s->ext.ech.ch_depth == 1) + s->ext.ech.tick_identity = s->ext.tick_identity; +#endif dores = 1; } @@ -1388,7 +1386,8 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt, * with random values of the same length. */ if (s->ext.ech.es != NULL && s->ext.ech.ch_depth == 0) { - unsigned char *rndbuf = NULL; + /* TODO(ECH): changes here need testing with server-side code PR */ + unsigned char *rndbuf = NULL, *rndbufp = NULL; size_t totalrndsize = 0; if (s->session == NULL) { @@ -1396,7 +1395,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt, return EXT_RETURN_FAIL; } totalrndsize = s->session->ext.ticklen - + 4 /* agems */ + + sizeof(agems) + s->psksession_id_len + reshashsize + pskhashsize; @@ -1405,56 +1404,43 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt, SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; } - /* outer CH allocate a similar sized random value */ - if (RAND_bytes_ex(s->ssl.ctx->libctx, rndbuf, totalrndsize, - RAND_DRBG_STRENGTH) <= 0) { + /* for outer CH allocate a similar sized random value */ + if (RAND_bytes_ex(s->ssl.ctx->libctx, rndbuf, totalrndsize, 0) <= 0) { OPENSSL_free(rndbuf); SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; } /* set agems from random buffer */ - agems = *((uint32_t *)(rndbuf + s->session->ext.ticklen)); + rndbufp = rndbuf; + agems = *((uint32_t *)(rndbufp)); + rndbufp += sizeof(agems); if (dores != 0) { - if (!WPACKET_sub_memcpy_u16(pkt, rndbuf, + if (!WPACKET_sub_memcpy_u16(pkt, rndbufp, s->session->ext.ticklen) || !WPACKET_put_bytes_u32(pkt, agems)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); OPENSSL_free(rndbuf); return EXT_RETURN_FAIL; } + rndbufp += s->session->ext.ticklen; } if (s->psksession != NULL) { - if (!WPACKET_sub_memcpy_u16(pkt, - rndbuf + s->session->ext.ticklen + 4, - s->psksession_id_len) + if (!WPACKET_sub_memcpy_u16(pkt, rndbufp, s->psksession_id_len) || !WPACKET_put_bytes_u32(pkt, 0)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); OPENSSL_free(rndbuf); return EXT_RETURN_FAIL; } + rndbufp += s->psksession_id_len; } if (!WPACKET_close(pkt) - || !WPACKET_get_total_written(pkt, &binderoffset) || !WPACKET_start_sub_packet_u16(pkt) || (dores == 1 - && !WPACKET_sub_memcpy_u8(pkt, - rndbuf + s->session->ext.ticklen - + 4 + s->psksession_id_len, - reshashsize)) + && !WPACKET_sub_memcpy_u8(pkt, rndbufp, reshashsize)) || (s->psksession != NULL - && !WPACKET_sub_memcpy_u8(pkt, - rndbuf + s->session->ext.ticklen - + 4 + s->psksession_id_len - + reshashsize, - pskhashsize)) + && !WPACKET_sub_memcpy_u8(pkt, rndbufp, pskhashsize)) || !WPACKET_close(pkt) - || !WPACKET_close(pkt) - || !WPACKET_get_total_written(pkt, &msglen) - /* - * We need to fill in all the sub-packet lengths now so we can - * calculate the HMAC of the message up to the binders - */ - || !WPACKET_fill_lengths(pkt)) { + || !WPACKET_close(pkt)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); OPENSSL_free(rndbuf); return EXT_RETURN_FAIL; @@ -1653,12 +1639,13 @@ int tls_parse_stoc_server_name(SSL_CONNECTION *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx) { -#ifndef OPENSSL_NO_ECH char *eff_sni = s->ext.hostname; +#ifndef OPENSSL_NO_ECH /* if we tried ECH and failed, the outer is what's expected */ if (s->ext.ech.es != NULL && s->ext.ech.success == 0) eff_sni = s->ext.ech.outer_hostname; +#endif if (eff_sni == NULL) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; @@ -1678,29 +1665,6 @@ int tls_parse_stoc_server_name(SSL_CONNECTION *s, PACKET *pkt, return 0; } } -#else - if (s->ext.hostname == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; - } - - if (PACKET_remaining(pkt) > 0) { - SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION); - return 0; - } - - if (!s->hit) { - if (s->session->ext.hostname != NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; - } - s->session->ext.hostname = OPENSSL_strdup(s->ext.hostname); - if (s->session->ext.hostname == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; - } - } -#endif return 1; } @@ -2564,14 +2528,23 @@ EXT_RETURN tls_construct_ctos_ech(SSL_CONNECTION *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx) { - if (s->ext.ech.attempted_type != TLSEXT_TYPE_ech - && s->ext.ech.grease != OSSL_ECH_IS_GREASE - && !(s->options & SSL_OP_ECH_GREASE)) + int rv = 0, hpke_mode = OSSL_HPKE_MODE_BASE; + OSSL_ECHSTORE_ENTRY *ee = NULL; + OSSL_HPKE_SUITE hpke_suite = OSSL_HPKE_SUITE_DEFAULT; + unsigned char config_id_to_use = 0x00, info[OSSL_ECH_MAX_INFO_LEN]; + unsigned char *encoded = NULL, *mypub = NULL; + size_t cipherlen = 0, aad_len = 0, lenclen = 0, mypub_len = 0; + size_t info_len = OSSL_ECH_MAX_INFO_LEN, clear_len = 0, encoded_len = 0; + + /* whether or not we've been asked to GREASE, one way or another */ + int grease_opt_set = (s->ext.ech.grease == OSSL_ECH_IS_GREASE + || ((s->options & SSL_OP_ECH_GREASE) != 0)); + + /* if we're not doing real ECH and not GREASEing then exit */ + if (s->ext.ech.attempted_type != TLSEXT_TYPE_ech && grease_opt_set == 0) return EXT_RETURN_NOT_SENT; /* send grease if not really attempting ECH */ - if (s->ext.ech.attempted == 0 - && (s->ext.ech.grease == OSSL_ECH_IS_GREASE - || (s->options & SSL_OP_ECH_GREASE))) { + if (s->ext.ech.attempted == 0 && grease_opt_set == 1) { if (s->hello_retry_request == SSL_HRR_PENDING && s->ext.ech.sent != NULL) { /* re-tx already sent GREASEy ECH */ @@ -2591,14 +2564,7 @@ EXT_RETURN tls_construct_ctos_ech(SSL_CONNECTION *s, WPACKET *pkt, } return EXT_RETURN_SENT; } - /* - * If not GREASEing we fake sending the outer value - after the - * entire thing has been constructed we only then finally encode - * and encrypt - need to do it that way as we need the rest of - * the outer CH as AAD input to the encryption. - */ - if (s->ext.ech.ch_depth == 0) - return EXT_RETURN_NOT_SENT; + /* For the inner CH - we simply include one of these saying "inner" */ if (s->ext.ech.ch_depth == 1) { if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_ech) @@ -2610,6 +2576,141 @@ EXT_RETURN tls_construct_ctos_ech(SSL_CONNECTION *s, WPACKET *pkt, } return EXT_RETURN_SENT; } + + /* + * If not GREASEing we prepare sending the outer value - after the + * entire thing has been constructed, putting in zeros for now where + * we'd otherwise include ECH ciphertext, we later encode and encrypt. + * We need to do it that way as we need the rest of the outer CH to + * be known and used as AAD input before we do encryption. + */ + if (s->ext.ech.ch_depth != 0) + return EXT_RETURN_NOT_SENT; + /* Make ClientHelloInner and EncodedClientHelloInner as per spec. */ + if (ossl_ech_encode_inner(s, &encoded, &encoded_len) != 1) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } + s->ext.ech.encoded_inner = encoded; + s->ext.ech.encoded_inner_len = encoded_len; +# ifdef OSSL_ECH_SUPERVERBOSE + ossl_ech_pbuf("encoded inner CH", encoded, encoded_len); +# endif + rv = ossl_ech_pick_matching_cfg(s, &ee, &hpke_suite); + if (rv != 1 || ee == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } + s->ext.ech.attempted_type = ee->version; + OSSL_TRACE_BEGIN(TLS) { + BIO_printf(trc_out, "EAAE: selected: version: %4x, config %2x\n", + ee->version, ee->config_id); + } OSSL_TRACE_END(TLS); + config_id_to_use = ee->config_id; /* if requested, use a random config_id instead */ + if ((s->options & SSL_OP_ECH_IGNORE_CID) != 0) { + if (RAND_bytes_ex(s->ssl.ctx->libctx, &config_id_to_use, 1, 0) <= 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + return 0; + } +# ifdef OSSL_ECH_SUPERVERBOSE + ossl_ech_pbuf("EAAE: random config_id", &config_id_to_use, 1); +# endif + } + s->ext.ech.attempted_cid = config_id_to_use; +# ifdef OSSL_ECH_SUPERVERBOSE + ossl_ech_pbuf("EAAE: peer pub", ee->pub, ee->pub_len); + ossl_ech_pbuf("EAAE: clear", encoded, encoded_len); + ossl_ech_pbuf("EAAE: ECHConfig", ee->encoded, ee->encoded_len); +# endif + /* + * The AAD is the full outer client hello but with the correct number of + * zeros for where the ECH ciphertext octets will later be placed. So we + * add the ECH extension to the |pkt| but with zeros for ciphertext, that + * forms up the AAD, then after we've encrypted, we'll splice in the actual + * ciphertext. + * Watch out for the "4" offsets that remove the type and 3-octet length + * from the encoded CH as per the spec. + */ + clear_len = ossl_ech_calc_padding(s, ee, encoded_len); + if (clear_len == 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } + lenclen = OSSL_HPKE_get_public_encap_size(hpke_suite); + if (s->ext.ech.hpke_ctx == NULL) { /* 1st CH */ + if (ossl_ech_make_enc_info(ee->encoded, ee->encoded_len, + info, &info_len) != 1) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } +# ifdef OSSL_ECH_SUPERVERBOSE + ossl_ech_pbuf("EAAE info", info, info_len); +# endif + s->ext.ech.hpke_ctx = OSSL_HPKE_CTX_new(hpke_mode, hpke_suite, + OSSL_HPKE_ROLE_SENDER, + NULL, NULL); + if (s->ext.ech.hpke_ctx == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } + mypub = OPENSSL_malloc(lenclen); + if (mypub == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } + mypub_len = lenclen; + rv = OSSL_HPKE_encap(s->ext.ech.hpke_ctx, mypub, &mypub_len, + ee->pub, ee->pub_len, info, info_len); + if (rv != 1) { + OPENSSL_free(mypub); + mypub = NULL; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } + s->ext.ech.pub = mypub; + s->ext.ech.pub_len = mypub_len; + } else { /* HRR - retrieve public */ + mypub = s->ext.ech.pub; + mypub_len = s->ext.ech.pub_len; + if (mypub == NULL || mypub_len == 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } + } +# ifdef OSSL_ECH_SUPERVERBOSE + ossl_ech_pbuf("EAAE: mypub", mypub, mypub_len); + WPACKET_get_total_written(pkt, &aad_len); /* use aad_len for tracing */ + ossl_ech_pbuf("EAAE pkt b4", WPACKET_get_curr(pkt) - aad_len, aad_len); +# endif + cipherlen = OSSL_HPKE_get_ciphertext_size(hpke_suite, clear_len); + if (cipherlen <= clear_len || cipherlen > OSSL_ECH_MAX_PAYLOAD_LEN) { + SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION); + goto err; + } + s->ext.ech.clearlen = clear_len; + s->ext.ech.cipherlen = cipherlen; + if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_ech) + || !WPACKET_start_sub_packet_u16(pkt) + || !WPACKET_put_bytes_u8(pkt, OSSL_ECH_OUTER_CH_TYPE) + || !WPACKET_put_bytes_u16(pkt, hpke_suite.kdf_id) + || !WPACKET_put_bytes_u16(pkt, hpke_suite.aead_id) + || !WPACKET_put_bytes_u8(pkt, config_id_to_use) + || (s->hello_retry_request == SSL_HRR_PENDING + && !WPACKET_put_bytes_u16(pkt, 0x00)) /* no pub */ + || (s->hello_retry_request != SSL_HRR_PENDING + && !WPACKET_sub_memcpy_u16(pkt, mypub, mypub_len)) + || !WPACKET_start_sub_packet_u16(pkt) + || !WPACKET_get_total_written(pkt, &s->ext.ech.cipher_offset) + || !WPACKET_memset(pkt, 0, cipherlen) + || !WPACKET_close(pkt) + || !WPACKET_close(pkt)) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } + /* don't count the type + 3-octet length */ + s->ext.ech.cipher_offset -= 4; + return EXT_RETURN_SENT; +err: return EXT_RETURN_FAIL; } @@ -2620,22 +2721,29 @@ int tls_parse_stoc_ech(SSL_CONNECTION *s, PACKET *pkt, unsigned int context, unsigned int rlen = 0; const unsigned char *rval = NULL; unsigned char *srval = NULL; + PACKET rcfgs_pkt; /* - * An HRR will have an ECH extension with the - * 8-octet confirmation value, already handled + * An HRR will have an ECH extension with the 8-octet confirmation value. + * Store it away for when we check it later */ - if (context == SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) + if (context == SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) { + if (PACKET_remaining(pkt) != OSSL_ECH_SIGNAL_LEN) { + SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_LENGTH_MISMATCH); + return 0; + } + s->ext.ech.hrrsignal_p = (unsigned char *)PACKET_data(pkt); + memcpy(s->ext.ech.hrrsignal, s->ext.ech.hrrsignal_p, + OSSL_ECH_SIGNAL_LEN); return 1; + } /* othewise we expect retry-configs */ - if (!PACKET_get_net_2(pkt, &rlen)) { + if (!PACKET_get_length_prefixed_2(pkt, &rcfgs_pkt)) { SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_LENGTH_MISMATCH); return 0; } - if (!PACKET_get_bytes(pkt, &rval, rlen)) { - SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_LENGTH_MISMATCH); - return 0; - } + rval = PACKET_data(&rcfgs_pkt); + rlen = PACKET_remaining(&rcfgs_pkt); OPENSSL_free(s->ext.ech.returned); s->ext.ech.returned = NULL; srval = OPENSSL_malloc(rlen + 2); diff --git a/ssl/statem/extensions_cust.c b/ssl/statem/extensions_cust.c index b9d08da33f9..41988c26aea 100644 --- a/ssl/statem/extensions_cust.c +++ b/ssl/statem/extensions_cust.c @@ -200,6 +200,65 @@ int custom_ext_add(SSL_CONNECTION *s, int context, WPACKET *pkt, X509 *x, if (!(meth->ext_flags & SSL_EXT_FLAG_RECEIVED)) continue; } + +#ifndef OPENSSL_NO_ECH + if ((context & SSL_EXT_CLIENT_HELLO) != 0 + && s->ext.ech.attempted == 1) { + if (s->ext.ech.ch_depth == 1) { + /* mark custom CH ext for ECH compression, if doing ECH */ + if (s->ext.ech.n_outer_only >= OSSL_ECH_OUTERS_MAX) { + OSSL_TRACE_BEGIN(TLS) { + BIO_printf(trc_out, + "Too many outers to compress (max=%d)\n", + OSSL_ECH_OUTERS_MAX); + } OSSL_TRACE_END(TLS); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_EXTENSION); + return 0; + } + s->ext.ech.outer_only[s->ext.ech.n_outer_only] = meth->ext_type; + s->ext.ech.n_outer_only++; + OSSL_TRACE_BEGIN(TLS) { + BIO_printf(trc_out, "ECH compressing type " + "0x%04x (tot: %d)\n", + (int) meth->ext_type, + (int) s->ext.ech.n_outer_only); + } OSSL_TRACE_END(TLS); + } + if (s->ext.ech.ch_depth == 0) { + /* TODO(ECH): we need a better way to handle indexing exts */ + /* copy over the extension octets (if any) to outer */ + int j, tind = -1; + RAW_EXTENSION *raws = NULL; + + /* we gotta find the relevant index to copy over this ext */ + if (s->clienthello == NULL + || s->clienthello->pre_proc_exts == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_EXTENSION); + return 0; + } + raws = s->clienthello->pre_proc_exts; + for (j = 0; j != (int) s->clienthello->pre_proc_exts_len; j++) { + if (raws[j].type == meth->ext_type) { + tind = j; + break; + } + } + if (tind == -1) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_EXTENSION); + return 0; + } + if (ossl_ech_copy_inner2outer(s, meth->ext_type, tind, pkt) + != OSSL_ECH_SAME_EXT_DONE) { + /* for custom exts, we really should have found it */ + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_EXTENSION); + return 0; + } + /* we're done with that one now */ + continue; + } + } +#endif + /* * We skip it if the callback is absent - except for a ClientHello where * we add an empty extension. @@ -393,7 +452,19 @@ int ossl_tls_add_custom_ext_intern(SSL_CTX *ctx, custom_ext_methods *exts, * for extension types that previously were not supported, but now are. */ if (SSL_extension_supported(ext_type) +#if !defined(OPENSSL_NO_ECH) && defined(OPENSSL_ECH_ALLOW_CUST_INJECT) + /* + * Do this conditionally so we can test an ECH in TLSv1.2 + * via the custom extensions API. + * OPENSSL_ECH_ALLOW_CUST_INJECT is defined (or not) in + * include/openssl/ech.h and if defined enables a test in + * test/ech_test.c + */ + && ext_type != TLSEXT_TYPE_ech && ext_type != TLSEXT_TYPE_signed_certificate_timestamp) +#else + && ext_type != TLSEXT_TYPE_signed_certificate_timestamp) +#endif return 0; /* Extension type must fit in 16 bits */ @@ -546,6 +617,10 @@ int SSL_extension_supported(unsigned int ext_type) case TLSEXT_TYPE_compress_certificate: case TLSEXT_TYPE_client_cert_type: case TLSEXT_TYPE_server_cert_type: +#ifndef OPENSSL_NO_ECH + case TLSEXT_TYPE_ech: + case TLSEXT_TYPE_outer_extensions: +#endif return 1; default: return 0; diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 7239211298e..4a372945c6a 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -31,7 +31,7 @@ #include "internal/ssl_unwrap.h" static MSG_PROCESS_RETURN tls_process_as_hello_retry_request(SSL_CONNECTION *s, - PACKET *pkt); + RAW_EXTENSION *extensions); static MSG_PROCESS_RETURN tls_process_encrypted_extensions(SSL_CONNECTION *s, PACKET *pkt); @@ -1163,13 +1163,13 @@ WORK_STATE ossl_statem_client_post_process_message(SSL_CONNECTION *s, #ifndef OPENSSL_NO_ECH /* - * Wrap the existing ClientHello construction with ECH code. + * Wrap ClientHello construction with ECH code. * - * As needed, we'll call the existing CH constructor twice, - * first for inner, and then for outer. + * As needed, we'll call the CH constructor twice, first for + * inner, and then for outer. * - * So the old tls_construct_client_hello is renamed to the _aux - * variant, and the new tls_construct_client_hello just calls + * `tls_construct_client_hello_aux` is the pre-ECH code + * and the ECH-aware tls_construct_client_hello just calls * that if there's no ECH involved, but otherwise does ECH * things around calls to the _aux variant. * @@ -1204,7 +1204,6 @@ static int tls_construct_client_hello_aux(SSL_CONNECTION *s, WPACKET *pkt); __owur CON_FUNC_RETURN tls_construct_client_hello(SSL_CONNECTION *s, WPACKET *pkt) { - unsigned char *innerch_full = NULL, *innerch_end = NULL; WPACKET inner; /* "fake" pkt for inner */ BUF_MEM *inner_mem = NULL; PACKET rpkt; /* we'll decode back the inner ch to help make the outer */ @@ -1226,12 +1225,12 @@ __owur CON_FUNC_RETURN tls_construct_client_hello(SSL_CONNECTION *s, /* note version we're attempting and that an attempt is being made */ if (s->ext.ech.es->entries != NULL) { if (ossl_ech_pick_matching_cfg(s, &ee, &suite) != 1 || ee == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, protverr); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_UNSUPPORTED); return 0; } if (ee->version != OSSL_ECH_RFCXXXX_VERSION) { /* we only support that version for now */ - SSLfatal(s, SSL_AD_INTERNAL_ERROR, protverr); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_UNSUPPORTED); return 0; } s->ext.ech.attempted_type = TLSEXT_TYPE_ech; @@ -1267,7 +1266,7 @@ __owur CON_FUNC_RETURN tls_construct_client_hello(SSL_CONNECTION *s, s->tmp_session_id_len = sess_id_len; if (s->hello_retry_request == SSL_HRR_NONE && RAND_bytes_ex(s->ssl.ctx->libctx, s->tmp_session_id, - sess_id_len, RAND_DRBG_STRENGTH) <= 0) { + sess_id_len, 0) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } @@ -1284,19 +1283,8 @@ __owur CON_FUNC_RETURN tls_construct_client_hello(SSL_CONNECTION *s, memcpy(s->tmp_session_id, s->session->session_id, sess_id_len); } } - if (s->hello_retry_request != SSL_HRR_NONE) { + if (s->hello_retry_request != SSL_HRR_NONE) s->ext.ech.n_outer_only = 0; /* reset count of "compressed" exts */ - OPENSSL_free(s->ext.ech.encoded_innerch); - s->ext.ech.encoded_innerch = NULL; - s->ext.ech.encoded_innerch_len = 0; - if (s->ext.ech.innerch != NULL) { - OPENSSL_free(s->ext.ech.innerch1); - s->ext.ech.innerch1 = s->ext.ech.innerch; - s->ext.ech.innerch1_len = s->ext.ech.innerch_len; - s->ext.ech.innerch_len = 0; - s->ext.ech.innerch = NULL; - } - } /* * Set CH depth flag so that other code (e.g. extension handlers) * know where we're at: 1 is "inner CH", 0 is "outer CH" @@ -1308,19 +1296,18 @@ __owur CON_FUNC_RETURN tls_construct_client_hello(SSL_CONNECTION *s, || tls_construct_client_hello_aux(s, &inner) != 1 || !WPACKET_close(&inner) || !WPACKET_get_length(&inner, &innerlen)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, protverr); - goto err; - } - innerch_full = OPENSSL_malloc(innerlen); - if (innerch_full == NULL) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } - innerch_end = WPACKET_get_curr(&inner); - memcpy(innerch_full, innerch_end - innerlen, innerlen); OPENSSL_free(s->ext.ech.innerch); - s->ext.ech.innerch = innerch_full; + s->ext.ech.innerch = (unsigned char*)inner_mem->data; + inner_mem->data = NULL; s->ext.ech.innerch_len = innerlen; + /* add inner to transcript */ + if (ossl_ech_intbuf_add(s, s->ext.ech.innerch, innerlen, 0) != 1) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } WPACKET_cleanup(&inner); BUF_MEM_free(inner_mem); inner_mem = NULL; @@ -1346,15 +1333,7 @@ __owur CON_FUNC_RETURN tls_construct_client_hello(SSL_CONNECTION *s, SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } - /* Make ClientHelloInner and EncodedClientHelloInner as per spec. */ - if (ossl_ech_encode_inner(s) != 1) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - goto err; - } -# ifdef OSSL_ECH_SUPERVERBOSE - ossl_ech_pbuf("encoded inner CH", s->ext.ech.encoded_innerch, - s->ext.ech.encoded_innerch_len); -# endif + s->ext.ech.ch_depth = 0; /* set depth for outer CH */ /* * If we want different key shares for inner and outer, then @@ -1368,7 +1347,7 @@ __owur CON_FUNC_RETURN tls_construct_client_hello(SSL_CONNECTION *s, /* Make second call into CH construction for outer CH. */ rv = tls_construct_client_hello_aux(s, pkt); if (rv != 1) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, protverr); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } # ifdef OSSL_ECH_SUPERVERBOSE @@ -1445,10 +1424,8 @@ __owur CON_FUNC_RETURN tls_construct_client_hello(SSL_CONNECTION *s, WPACKET *pk if (s->ext.ech.es != NULL && s->ext.ech.ch_depth == 1) p = s->ext.ech.client_random; else - p = s->s3.client_random; -#else - p = s->s3.client_random; #endif + p = s->s3.client_random; /* * for DTLS if client_random is initialized, reuse it, we are @@ -1506,19 +1483,11 @@ __owur CON_FUNC_RETURN tls_construct_client_hello(SSL_CONNECTION *s, WPACKET *pk * For TLS 1.3 we always set the ClientHello version to 1.2 and rely on the * supported_versions extension for the real supported versions. */ -#ifndef OPENSSL_NO_ECH if (!WPACKET_put_bytes_u16(pkt, s->client_version) || !WPACKET_memcpy(pkt, p, SSL3_RANDOM_SIZE)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return CON_FUNC_ERROR; } -#else - if (!WPACKET_put_bytes_u16(pkt, s->client_version) - || !WPACKET_memcpy(pkt, s->s3.client_random, SSL3_RANDOM_SIZE)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return CON_FUNC_ERROR; - } -#endif /* Session ID */ session_id = s->session->session_id; @@ -1745,13 +1714,13 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt) #endif #ifndef OPENSSL_NO_ECH const unsigned char *shbuf = NULL; - size_t shlen, chend, fixedshbuf_len, alen; + size_t shlen, alen; /* * client and server accept signal buffers, initialise in case of * e.g. memory fail when calculating, only really applies when * SUPERVERBOSE is defined and we trace these. */ - unsigned char c_signal[OSSL_ECH_SIGNAL_LEN] = { 0 }; + unsigned char c_signal[OSSL_ECH_SIGNAL_LEN] = { 0 }; unsigned char s_signal[OSSL_ECH_SIGNAL_LEN] = { 0xff }; unsigned char *abuf = NULL; @@ -1816,6 +1785,34 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt) goto err; } + /* TLS extensions */ + if (PACKET_remaining(pkt) == 0 && !hrr) { + PACKET_null_init(&extpkt); + } else if (!PACKET_as_length_prefixed_2(pkt, &extpkt) + || PACKET_remaining(pkt) != 0) { + SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_LENGTH); + goto err; + } + + if (hrr) { + if (!tls_collect_extensions(s, &extpkt, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST, + &extensions, NULL, 1) + || !tls_parse_extension(s, TLSEXT_IDX_ech, + SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST, + extensions, NULL, 0)) { + /* SSLfatal() already called */ + goto err; + } + } else { + if (!tls_collect_extensions(s, &extpkt, + SSL_EXT_TLS1_2_SERVER_HELLO + | SSL_EXT_TLS1_3_SERVER_HELLO, + &extensions, NULL, 1)) { + /* SSLfatal() already called */ + goto err; + } + } + #ifndef OPENSSL_NO_ECH /* * If we sent an ECH then check if that worked based on the @@ -1828,14 +1825,31 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt) && s->ext.ech.done != 1 && s->ext.ech.ch_depth == 0 && s->ext.ech.grease == OSSL_ECH_NOT_GREASE && s->ext.ech.attempted_type == TLSEXT_TYPE_ech) { - /* try set this earlier see what happens */ if (!set_client_ciphersuite(s, cipherchars)) { /* SSLfatal() already called */ goto err; } + /* add any SH/HRR to inner transcript if we tried ECH */ + if (s->ext.ech.attempted == 1) { + unsigned char prelude[4]; + + prelude[0] = SSL3_MT_SERVER_HELLO; + prelude[1] = (shlen >> 16) & 0xff; + prelude[2] = (shlen >> 8) & 0xff; + prelude[3] = shlen & 0xff; + if (ossl_ech_intbuf_add(s, prelude, sizeof(prelude), hrr) != 1 + || ossl_ech_intbuf_add(s, shbuf, shlen, 0) != 1) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } + } /* check the ECH accept signal */ - if (ossl_ech_calc_confirm(s, hrr, c_signal, shbuf, shlen) != 1 - || ossl_ech_find_confirm(s, hrr, s_signal, shbuf, shlen) != 1 + if (ossl_ech_calc_confirm(s, hrr, c_signal, shlen) != 1) { + /* SSLfatal() already called */ + OSSL_TRACE(TLS, "ECH calc confim failed\n"); + goto err; + } + if (ossl_ech_find_confirm(s, hrr, s_signal) != 1 || memcmp(s_signal, c_signal, sizeof(c_signal)) != 0) { OSSL_TRACE(TLS, "ECH accept check failed\n"); # ifdef OSSL_ECH_SUPERVERBOSE @@ -1851,17 +1865,15 @@ 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_make_transcript_buffer(s, hrr, shbuf, shlen, - &abuf, &alen, - &chend, &fixedshbuf_len) != 1 + || ossl_ech_intbuf_fetch(s, &abuf, &alen) != 1 || ossl_ech_reset_hs_buffer(s, abuf, alen) != 1) { - OPENSSL_free(abuf); SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } - OPENSSL_free(abuf); } else if (!hrr) { /* * If we got retry_configs then we should be validating @@ -1881,24 +1893,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt) } #endif - /* TLS extensions */ - if (PACKET_remaining(pkt) == 0 && !hrr) { - PACKET_null_init(&extpkt); - } else if (!PACKET_as_length_prefixed_2(pkt, &extpkt) - || PACKET_remaining(pkt) != 0) { - SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_LENGTH); - goto err; - } - if (!hrr) { - if (!tls_collect_extensions(s, &extpkt, - SSL_EXT_TLS1_2_SERVER_HELLO - | SSL_EXT_TLS1_3_SERVER_HELLO, - &extensions, NULL, 1)) { - /* SSLfatal() already called */ - goto err; - } - if (!ssl_choose_client_version(s, sversion, extensions)) { /* SSLfatal() already called */ goto err; @@ -1921,12 +1916,17 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt) } if (hrr) { + int ret; + if (!set_client_ciphersuite(s, cipherchars)) { /* SSLfatal() already called */ goto err; } - return tls_process_as_hello_retry_request(s, &extpkt); + ret = tls_process_as_hello_retry_request(s, extensions); + OPENSSL_free(extensions); + + return ret; } /* @@ -2175,10 +2175,8 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt) } static MSG_PROCESS_RETURN tls_process_as_hello_retry_request(SSL_CONNECTION *s, - PACKET *extpkt) + RAW_EXTENSION *extensions) { - RAW_EXTENSION *extensions = NULL; - /* * If we were sending early_data then any alerts should not be sent using * the old wrlmethod. @@ -2196,17 +2194,12 @@ static MSG_PROCESS_RETURN tls_process_as_hello_retry_request(SSL_CONNECTION *s, /* We are definitely going to be using TLSv1.3 */ s->rlayer.wrlmethod->set_protocol_version(s->rlayer.wrl, TLS1_3_VERSION); - if (!tls_collect_extensions(s, extpkt, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST, - &extensions, NULL, 1) - || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST, - extensions, NULL, 0, 1)) { + if (!tls_parse_all_extensions(s, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST, + extensions, NULL, 0, 1)) { /* SSLfatal() already called */ goto err; } - OPENSSL_free(extensions); - extensions = NULL; - if (s->ext.tls13_cookie_len == 0 && s->s3.tmp.pkey != NULL) { /* * We didn't receive a cookie or a new key_share so the next @@ -2239,7 +2232,6 @@ static MSG_PROCESS_RETURN tls_process_as_hello_retry_request(SSL_CONNECTION *s, return MSG_PROCESS_FINISHED_READING; err: - OPENSSL_free(extensions); return MSG_PROCESS_ERROR; } @@ -3332,8 +3324,12 @@ int tls_process_initial_server_flight(SSL_CONNECTION *s) #ifndef OPENSSL_NO_ECH /* check result of ech and return error if needed */ - if (!s->server - && s->ext.ech.es != NULL + /* + * TODO(ECH): check that we never get here in a server + * during split-mode or test cases - there used be a + * check of !s->server added to the below. + */ + if (s->ext.ech.es != NULL && s->ext.ech.attempted == 1 && s->ext.ech.success != 1 && s->ext.ech.grease != OSSL_ECH_IS_GREASE) { diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index 6bddc9b51c4..9ed7e1633db 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -500,10 +500,25 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which) labellen = sizeof(client_early_traffic) - 1; log_label = CLIENT_EARLY_LABEL; - handlen = BIO_get_mem_data(s->s3.handshake_buffer, &hdata); - if (handlen <= 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_HANDSHAKE_LENGTH); - goto err; +#ifndef OPENSSL_NO_ECH + /* if ECH worked then use the innerch and not the h/s buffer here */ + if (((which & SSL3_CC_SERVER) && s->ext.ech.success == 1) + || ((which & SSL3_CC_CLIENT) && s->ext.ech.attempted == 1)) { + if (s->ext.ech.innerch == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } + handlen = s->ext.ech.innerch_len; + hdata = s->ext.ech.innerch; + } else +#endif + { + handlen = BIO_get_mem_data(s->s3.handshake_buffer, &hdata); + if (handlen <= 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_R_BAD_HANDSHAKE_LENGTH); + goto err; + } } if (s->early_data_state == SSL_EARLY_DATA_CONNECTING diff --git a/test/ech_test.c b/test/ech_test.c index 743ab90208e..968fca896d1 100644 --- a/test/ech_test.c +++ b/test/ech_test.c @@ -24,6 +24,8 @@ static char *cert = NULL; static char *privkey = NULL; static char *rootcert = NULL; +/* TODO(ECH): add some testing of SSL_OP_ECH_IGNORE_CID */ + /* callback */ static unsigned int test_cb(SSL *s, const char *str) { @@ -887,10 +889,19 @@ static int ech_ingest_test(int run) || !TEST_false(OSSL_ECHSTORE_write_pem(es, 100, out))) goto end; flush_time = time(0); + /* + * Occasionally, flush_time will be 1 more than add_time. We'll + * check for that as that should catch a few more code paths + * in the flush_keys API. + */ if (!TEST_true(OSSL_ECHSTORE_flush_keys(es, flush_time - add_time)) || !TEST_int_eq(OSSL_ECHSTORE_num_keys(es, &keysaftr), 1) - || !TEST_int_eq(keysaftr, 0)) + || ((flush_time <= add_time) && !TEST_int_eq(keysaftr, 0)) + || ((flush_time > add_time) && !TEST_int_eq(keysaftr, 1))) { + TEST_info("Flush time: %lld, add_time: %lld", (long long)flush_time, + (long long)add_time); goto end; + } rv = 1; end: OPENSSL_free(pn);