public/openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

This change fixes an issue where a DTLS 1.3 would calculate a wrong transcript hash.

Browse source 
A wrong transcript hash was calculated when the client received a HRR which caused interop failures with WolfSSL. This change also refactors the internal calls to ssl3_finish_mac() that no longer requires the "incl_hdr" argument.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26465)
This commit is contained in:
Frederik Wedel-Heinen 2025-01-17 09:05:59 +01:00 • committed by Tomas Mraz
parent 1ee54c646b
commit 718f5b4cfb
7 changed files with 98 additions and 78 deletions
Download patch file Download diff file Expand all files Collapse all files

2
include/openssl/prov_ssl.h
View file

@ -30,6 +30,8 @@ extern "C" {
# define DTLS1_3_VERSION 0xFEFC
# define DTLS1_BAD_VER 0x0100
# define PROTO_VERSION_UNSET 0
/* QUIC uses a 4 byte unsigned version number */
# define OSSL_QUIC1_VERSION 0x0000001

5
ssl/record/rec_layer_s3.c
View file

@ -1476,11 +1476,14 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
int ssl_set_record_protocol_version(SSL_CONNECTION *s, int vers)
{
if (!ossl_assert(s->rlayer.rrlmethod != NULL)
if ((s->negotiated_version != PROTO_VERSION_UNSET && s->negotiated_version != vers)
|| !ossl_assert(s->rlayer.rrlmethod != NULL)
|| !ossl_assert(s->rlayer.wrlmethod != NULL)
|| !s->rlayer.rrlmethod->set_protocol_version(s->rlayer.rrl, vers)
|| !s->rlayer.wrlmethod->set_protocol_version(s->rlayer.wrl, vers))
return 0;
s->negotiated_version = vers;
return 1;
}

94
ssl/s3_enc.c
View file

@ -244,7 +244,7 @@ void ssl3_free_digest_list(SSL_CONNECTION *s)
s->s3.handshake_dgst = NULL;
}
int ssl3_finish_mac(SSL_CONNECTION *s, const unsigned char *buf, size_t len, int hmhdr_incl)
int ssl3_finish_mac(SSL_CONNECTION *s, const unsigned char *buf, size_t len)
{
int ret;
@ -273,20 +273,58 @@ int ssl3_finish_mac(SSL_CONNECTION *s, const unsigned char *buf, size_t len, int
* we calculate the digest when initiating s->s3.handshake_dgst at which
* point we know what the protocol version is.
*/
if (s->negotiated_version == DTLS1_3_VERSION) {
/*
* In DTLS 1.3 we need to parse the messages that are buffered to
* be able to remove message_sequence, fragment_size and fragment_offset
* from the Transcript Hash calculation.
*/
while (len > 0) {
PACKET hmhdr;
unsigned long hmbodylen;
unsigned int msgtype;
size_t hmhdrlen;
if (SSL_CONNECTION_IS_DTLS13(s) && hmhdr_incl
&& ossl_assert(len >= DTLS1_HM_HEADER_LENGTH)) {
ret = EVP_DigestUpdate(s->s3.handshake_dgst, buf, SSL3_HM_HEADER_LENGTH);
ret &= EVP_DigestUpdate(s->s3.handshake_dgst,
buf + DTLS1_HM_HEADER_LENGTH,
len - DTLS1_HM_HEADER_LENGTH);
if (!ossl_assert(len >= SSL3_HM_HEADER_LENGTH)
|| !PACKET_buf_init(&hmhdr, buf, SSL3_HM_HEADER_LENGTH)
|| !PACKET_get_1(&hmhdr, &msgtype)
|| !PACKET_get_net_3(&hmhdr, &hmbodylen)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
/*
* SSL3_MT_MESSAGE_HASH is a dummy message type only used when
* calculating the transcript hash of the synthetic message in
* (D)TLS 1.3.
*/
if (msgtype == SSL3_MT_MESSAGE_HASH)
hmhdrlen = SSL3_HM_HEADER_LENGTH;
else
hmhdrlen = DTLS1_HM_HEADER_LENGTH;
/*
* In DTLS 1.3 the transcript hash is calculated excluding the
* message_sequence, fragment_size and fragment_offset header
* fields which are carried in the last
* DTLS1_HM_HEADER_LENGTH - SSL3_HM_HEADER_LENGTH header bytes
* of the DTLS handshake message header.
*/
if (!ossl_assert(hmhdrlen + hmbodylen <= len)
|| !EVP_DigestUpdate(s->s3.handshake_dgst, buf, SSL3_HM_HEADER_LENGTH)
|| !EVP_DigestUpdate(s->s3.handshake_dgst, buf + hmhdrlen, hmbodylen)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
buf += hmhdrlen + hmbodylen;
len -= hmhdrlen + hmbodylen;
}
} else {
ret = EVP_DigestUpdate(s->s3.handshake_dgst, buf, len);
}
if (!ret) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
if (!EVP_DigestUpdate(s->s3.handshake_dgst, buf, len)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
}
}
return 1;
@ -321,33 +359,9 @@ int ssl3_digest_cached_records(SSL_CONNECTION *s, int keep)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
if (SSL_CONNECTION_IS_DTLS13(s)) {
unsigned char *hm = hdata;
size_t remlen = hdatalen;
while (remlen > 0) {
PACKET hmhdr;
unsigned long msglen;
if (remlen < DTLS1_HM_HEADER_LENGTH
|| !PACKET_buf_init(&hmhdr, hm, DTLS1_HM_HEADER_LENGTH)
|| !PACKET_forward(&hmhdr, 1)
|| !PACKET_get_net_3(&hmhdr, &msglen)
|| (msglen + DTLS1_HM_HEADER_LENGTH) > remlen
|| !ssl3_finish_mac(s, hm, msglen + DTLS1_HM_HEADER_LENGTH, 1)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
hm += msglen + DTLS1_HM_HEADER_LENGTH;
remlen -= msglen + DTLS1_HM_HEADER_LENGTH;
}
} else {
if (!ssl3_finish_mac(s, hdata, hdatalen, 1)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
if (!ssl3_finish_mac(s, hdata, hdatalen)) {
/* SSLfatal() already called */
return 0;
}
}
if (keep == 0) {

9
ssl/ssl_local.h
View file

@ -1276,6 +1276,13 @@ struct ssl_connection_st {
* DTLS1_3_VERSION)
*/
int version;
/*
* The negotiated version for the connection. Initially PROTO_VERSION_UNSET.
* Set by ssl_set_negotiated_protocol_version().
*/
int negotiated_version;
/*
* There are 2 BIO's even though they are normally both the same. This
* is so data can be read and written to different handlers
@ -2710,7 +2717,7 @@ __owur int ssl3_dispatch_alert(SSL *s);
__owur size_t ssl3_final_finish_mac(SSL_CONNECTION *s, const char *sender,
size_t slen, unsigned char *p);
__owur int ssl3_finish_mac(SSL_CONNECTION *s, const unsigned char *buf,
size_t len, int hmhdr_incl);
size_t len);
void ssl3_free_digest_list(SSL_CONNECTION *s);
__owur unsigned long ssl3_output_cert_chain(SSL_CONNECTION *s, WPACKET *pkt,
CERT_PKEY *cpk, int for_comp);

4
ssl/statem/statem_clnt.c
View file

@ -710,7 +710,7 @@ WORK_STATE ossl_statem_client_pre_work(SSL_CONNECTION *s, WORK_STATE wst)
case TLS_ST_CW_CLNT_HELLO:
s->shutdown = 0;
if (SSL_CONNECTION_IS_DTLS(s)) {
if (SSL_CONNECTION_IS_DTLS(s) && s->hello_retry_request != SSL_HRR_PENDING) {
/* every DTLS ClientHello resets Finished MAC */
if (!ssl3_init_finished_mac(s)) {
/* SSLfatal() already called */
@ -1900,7 +1900,7 @@ static MSG_PROCESS_RETURN tls_process_as_hello_retry_request(SSL_CONNECTION *s,
* for HRR messages.
*/
if (!ssl3_finish_mac(s, (unsigned char *)s->init_buf->data,
s->init_num + msghdrlen, 1)) {
s->init_num + msghdrlen)) {
/* SSLfatal() already called */
goto err;
}

29
ssl/statem/statem_dtls.c
View file

@ -291,15 +291,14 @@ int dtls1_do_write(SSL_CONNECTION *s, uint8_t recordtype)
assert(s->s3.tmp.new_compression != NULL
|| BIO_wpending(s->wbio) <= (int)s->d1->mtu);
if (recordtype == SSL3_RT_HANDSHAKE && !s->d1->retransmitting) {
/*
* should not be done for 'Hello Request's, but in that case
* we'll ignore the result anyway
*/
size_t xlen;
int hmhdr_incl;
if (recordtype == SSL3_RT_HANDSHAKE && !s->d1->retransmitting
&& s->init_off == 0) {
size_t xlen = s->init_num;
if (s->init_off == 0 && s->version != DTLS1_BAD_VER) {
if (s->version == DTLS1_BAD_VER) {
msgstart += DTLS1_HM_HEADER_LENGTH;
xlen -= DTLS1_HM_HEADER_LENGTH;
} else {
/*
* Now prepare to calculate the transcript hash. For
* versions prior to DTLSv1.3 this means:
@ -318,14 +317,8 @@ int dtls1_do_write(SSL_CONNECTION *s, uint8_t recordtype)
if (!dtls1_write_hm_header(msgstart, msg_type, msg_len,
msg_seq, 0, msg_len))
return -1;
xlen = written;
hmhdr_incl = 1;
} else {
msgstart += DTLS1_HM_HEADER_LENGTH;
xlen = written - DTLS1_HM_HEADER_LENGTH;
hmhdr_incl = 0;
}
/*
* should not be done for 'Hello Request's, but in that case we'll
* ignore the result anyway
@ -334,12 +327,10 @@ int dtls1_do_write(SSL_CONNECTION *s, uint8_t recordtype)
if (!SSL_CONNECTION_IS_DTLS13(s)
|| (s->statem.hand_state != TLS_ST_SW_SESSION_TICKET
&& s->statem.hand_state != TLS_ST_CW_KEY_UPDATE
&& s->statem.hand_state != TLS_ST_SW_KEY_UPDATE)) {
if (!ssl3_finish_mac(s, msgstart, xlen, hmhdr_incl))
&& s->statem.hand_state != TLS_ST_SW_KEY_UPDATE))
if (!ssl3_finish_mac(s, msgstart, xlen))
return -1;
}
}
if (written == s->init_num) {
if (s->msg_callback)
s->msg_callback(1, s->version, recordtype, s->init_buf->data,

33
ssl/statem/statem_lib.c
View file

@ -107,7 +107,7 @@ int ssl3_do_write(SSL_CONNECTION *s, uint8_t type)
&& s->statem.hand_state != TLS_ST_SW_KEY_UPDATE))
if (!ssl3_finish_mac(s,
(unsigned char *)&s->init_buf->data[s->init_off],
written, 1))
written))
return -1;
if (written == s->init_num) {
s->statem.write_in_progress = 0;
@ -1657,7 +1657,7 @@ int tls_common_finish_mac(SSL_CONNECTION *s) {
/* Feed this message into MAC computation. */
if (RECORD_LAYER_is_sslv2_record(&s->rlayer)
&& !ssl3_finish_mac(s, msg, msg_len, 1)) {
&& !ssl3_finish_mac(s, msg, msg_len)) {
/* SSLfatal() already called */
return 0;
} else {
@ -1678,7 +1678,7 @@ int tls_common_finish_mac(SSL_CONNECTION *s) {
|| memcmp(hrrrandom,
s->init_buf->data + srvhellorandom_offs,
SSL3_RANDOM_SIZE) != 0) {
if (!ssl3_finish_mac(s, msg, msg_len, 1))
if (!ssl3_finish_mac(s, msg, msg_len))
/* SSLfatal() already called */
return 0;
}
@ -2654,23 +2654,27 @@ int create_synthetic_message_hash(SSL_CONNECTION *s,
size_t hashlen, const unsigned char *hrr,
size_t hrrlen)
{
unsigned char hashvaltmp[EVP_MAX_MD_SIZE];
unsigned char synmsghdr[SSL3_HM_HEADER_LENGTH];
unsigned char *hashvaltmp;
unsigned char synmsg[SSL3_HM_HEADER_LENGTH + EVP_MAX_MD_SIZE];
size_t currmsghdr_len = SSL_CONNECTION_IS_DTLS(s) ? DTLS1_HM_HEADER_LENGTH
: SSL3_HM_HEADER_LENGTH;
memset(synmsghdr, 0, sizeof(synmsghdr));
memset(synmsg, 0, SSL3_HM_HEADER_LENGTH);
hashvaltmp = synmsg + SSL3_HM_HEADER_LENGTH;
if (hashval == NULL) {
hashval = hashvaltmp;
hashlen = 0;
/* Get the hash of the initial ClientHello */
if (!ssl3_digest_cached_records(s, 0)
|| !ssl_handshake_hash(s, hashvaltmp, sizeof(hashvaltmp),
|| !ssl_handshake_hash(s, hashvaltmp, EVP_MAX_MD_SIZE,
&hashlen)) {
/* SSLfatal() already called */
return 0;
}
} else {
if (!ossl_assert(hashlen <= EVP_MAX_MD_SIZE))
return 0;
memcpy(hashvaltmp, hashval, hashlen);
}
/* Reinitialise the transcript hash */
@ -2680,10 +2684,9 @@ int create_synthetic_message_hash(SSL_CONNECTION *s,
}
/* Inject the synthetic message_hash message */
synmsghdr[0] = SSL3_MT_MESSAGE_HASH;
synmsghdr[SSL3_HM_HEADER_LENGTH - 1] = (unsigned char)hashlen;
if (!ssl3_finish_mac(s, synmsghdr, SSL3_HM_HEADER_LENGTH, 0)
|| !ssl3_finish_mac(s, hashval, hashlen, 0)) {
synmsg[0] = SSL3_MT_MESSAGE_HASH;
synmsg[SSL3_HM_HEADER_LENGTH - 1] = (unsigned char)hashlen;
if (!ssl3_finish_mac(s, synmsg, SSL3_HM_HEADER_LENGTH + hashlen)) {
/* SSLfatal() already called */
return 0;
}
@ -2694,9 +2697,9 @@ int create_synthetic_message_hash(SSL_CONNECTION *s,
* receiving a ClientHello2 with a cookie.
*/
if (hrr != NULL
&& (!ssl3_finish_mac(s, hrr, hrrlen, 1)
&& (!ssl3_finish_mac(s, hrr, hrrlen)
|| !ssl3_finish_mac(s, (unsigned char *)s->init_buf->data,
s->s3.tmp.message_size + currmsghdr_len, 1))) {
s->s3.tmp.message_size + currmsghdr_len))) {
/* SSLfatal() already called */
return 0;
}