From acbb7f73f5052c4ca78988cfd71dabe85e3c35c2 Mon Sep 17 00:00:00 2001 From: Frederik Wedel-Heinen Date: Fri, 25 Oct 2024 11:19:14 +0200 Subject: [PATCH] Fix DTLS 1.3 handshake transcript hash Reviewed-by: Matt Caswell Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/26035) --- doc/designs/dtlsv1_3/dtlsv1_3-main.md | 15 ++-- ssl/s3_enc.c | 58 +++++++++++++- ssl/ssl_local.h | 2 +- ssl/statem/extensions_srvr.c | 17 +++- ssl/statem/statem_clnt.c | 4 +- ssl/statem/statem_dtls.c | 33 ++++---- ssl/statem/statem_lib.c | 111 ++++++++++++++++---------- ssl/statem/statem_local.h | 1 + 8 files changed, 165 insertions(+), 76 deletions(-) diff --git a/doc/designs/dtlsv1_3/dtlsv1_3-main.md b/doc/designs/dtlsv1_3/dtlsv1_3-main.md index da280ea127e..88d985d5e8f 100644 --- a/doc/designs/dtlsv1_3/dtlsv1_3-main.md +++ b/doc/designs/dtlsv1_3/dtlsv1_3-main.md @@ -70,6 +70,11 @@ section 5.9. The DTLSv1.3 implementation modifies the epoch according to RFC9147 section 6.1 for DTLSv1.3 connections. +#### DTLS 1.3 Transcript Hash + +The DTLSv1.3 implementation does not include the message sequence number, +fragment offset and fragment length as is the case with previous versions of DTLS. + Implementation progress ----------------------- @@ -87,7 +92,6 @@ is not covered by these workitems and must be implemented separately. |-----------------------------------------------------|----------------| | ACK messages | - | | Use HelloRetryRequest instead of HelloVerifyRequest | #22985, #22400 | -| Message transcript | - | | ClientHello | #23320 | | EndOfEarlyData message | - | | Variable length header | - | @@ -101,15 +105,6 @@ separate work item. Here follows a collection of changes that need to be implemented. -#### Message Transcript - -The message transcript is computed differently from DTLS 1.2 and TLS 1.3: - -> In DTLSv1.3, the message transcript is computed over the original TLS 1.3-style -> Handshake messages without the message_seq, fragment_offset, and fragment_length -> values. Note that this is a change from DTLS 1.2 where those values were -> included in the transcript. - #### DTLSCipherText DTLSCipherText differs from DTLS 1.2 and TLS 1.3: diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c index 6cad3c07778..58adcb448b6 100644 --- a/ssl/s3_enc.c +++ b/ssl/s3_enc.c @@ -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 ssl3_finish_mac(SSL_CONNECTION *s, const unsigned char *buf, size_t len, int hmhdr_incl) { int ret; @@ -260,7 +260,30 @@ int ssl3_finish_mac(SSL_CONNECTION *s, const unsigned char *buf, size_t len) return 0; } } else { - ret = EVP_DigestUpdate(s->s3.handshake_dgst, buf, len); + /* + * rfc9147: + * In DTLS 1.3, the message transcript is computed over the + * original TLS 1.3-style Handshake messages without the + * message_seq, fragment_offset, and fragment_length values. Note + * that this is a change from DTLS 1.2 where those values were + * included in the transcript. + * + * So this means that we record the full handshake messages in + * s->s3.handshake_buffer while s->s3.handshake_dgst is not in use and then + * we calculate the digest when initiating s->s3.handshake_dgst at which + * point we know what the protocol version is. + */ + + 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); + } else { + ret = EVP_DigestUpdate(s->s3.handshake_dgst, buf, len); + } + if (!ret) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; @@ -294,11 +317,38 @@ int ssl3_digest_cached_records(SSL_CONNECTION *s, int keep) SSL_R_NO_SUITABLE_DIGEST_ALGORITHM); return 0; } - if (!EVP_DigestInit_ex(s->s3.handshake_dgst, md, NULL) - || !EVP_DigestUpdate(s->s3.handshake_dgst, hdata, hdatalen)) { + if (!EVP_DigestInit_ex(s->s3.handshake_dgst, md, NULL)) { 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 (keep == 0) { BIO_free(s->s3.handshake_buffer); diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h index ac4e516ac8e..6e2662e7716 100644 --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -2705,7 +2705,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); + size_t len, int hmhdr_incl); 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); diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 93f2ebd0868..1aa42a13bc0 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -31,7 +31,7 @@ * + 2 bytes for extension block length + 6 bytes for key_share extension * + 4 bytes for cookie extension header + the number of bytes in the cookie */ -#define MAX_HRR_SIZE (SSL3_HM_HEADER_LENGTH + 2 + SSL3_RANDOM_SIZE + 1 \ +#define MAX_HRR_SIZE (DTLS1_HM_HEADER_LENGTH + 2 + SSL3_RANDOM_SIZE + 1 \ + SSL_MAX_SSL_SESSION_ID_LENGTH + 2 + 1 + 2 + 6 + 4 \ + MAX_COOKIE_SIZE) @@ -739,7 +739,11 @@ int tls_parse_ctos_cookie(SSL_CONNECTION *s, PACKET *pkt, unsigned int context, uint64_t tm, now; SSL *ssl = SSL_CONNECTION_GET_SSL(s); SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(s); + const int version1_2 = SSL_CONNECTION_IS_DTLS(s) ? DTLS1_2_VERSION : TLS1_2_VERSION; const int version1_3 = SSL_CONNECTION_IS_DTLS(s) ? DTLS1_3_VERSION : TLS1_3_VERSION; + size_t msgbody_offs = SSL_CONNECTION_IS_DTLS(s) ? DTLS1_HM_HEADER_LENGTH + - SSL3_HM_HEADER_LENGTH + : 0; /* Ignore any cookie if we're not set up to verify it */ if (sctx->verify_stateless_cookie_cb == NULL @@ -873,8 +877,15 @@ int tls_parse_ctos_cookie(SSL_CONNECTION *s, PACKET *pkt, unsigned int context, return 0; } if (!WPACKET_put_bytes_u8(&hrrpkt, SSL3_MT_SERVER_HELLO) - || !WPACKET_start_sub_packet_u24(&hrrpkt) - || !WPACKET_put_bytes_u16(&hrrpkt, TLS1_2_VERSION) + || !WPACKET_start_sub_packet_u24_at_offset(&hrrpkt, msgbody_offs) + /* + * We are reconstructing the HRR to be able to calculate the + * transcript hash. + * Since HRR is only allowed for (D)TLSv1.3 and transcript hash does + * not include the values of message_seq, fragment_offset and + * fragment_length, setting these values is not required. + */ + || !WPACKET_put_bytes_u16(&hrrpkt, version1_2) || !WPACKET_memcpy(&hrrpkt, hrrrandom, SSL3_RANDOM_SIZE) || !WPACKET_sub_memcpy_u8(&hrrpkt, s->tmp_session_id, s->tmp_session_id_len) diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 5dd1cd6e408..a7648396f22 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1840,6 +1840,8 @@ static MSG_PROCESS_RETURN tls_process_as_hello_retry_request(SSL_CONNECTION *s, RAW_EXTENSION *extensions = NULL; const int version1_3 = SSL_CONNECTION_IS_DTLS(s) ? DTLS1_3_VERSION : TLS1_3_VERSION; const int versionany = SSL_CONNECTION_IS_DTLS(s) ? DTLS_ANY_VERSION : TLS_ANY_VERSION; + const size_t msghdrlen = SSL_CONNECTION_IS_DTLS(s) ? DTLS1_HM_HEADER_LENGTH + : SSL3_HM_HEADER_LENGTH; /* * If we were sending early_data then any alerts should not be sent using @@ -1894,7 +1896,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 + SSL3_HM_HEADER_LENGTH)) { + s->init_num + msghdrlen, 1)) { /* SSLfatal() already called */ goto err; } diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c index 5b7b0c4b456..a218e787a00 100644 --- a/ssl/statem/statem_dtls.c +++ b/ssl/statem/statem_dtls.c @@ -297,20 +297,34 @@ int dtls1_do_write(SSL_CONNECTION *s, uint8_t recordtype) * we'll ignore the result anyway */ size_t xlen; + int hmhdr_incl; if (s->init_off == 0 && s->version != DTLS1_BAD_VER) { /* - * reconstruct message header is if it is being sent in - * single fragment + * Now prepare to calculate the transcript hash. For + * versions prior to DTLSv1.3 this means: + * + * rfc6347: Hash calculations include entire handshake + * messages, including DTLS-specific fields: message_seq, + * fragment_offset, and fragment_length. However, the + * Finished MAC MUST be computed as if each handshake + * message had been sent as a single fragment. + * + * For DTLSv1.3 DTLS-specific fields: message_seq, + * fragment_offset, and fragment_length are not used in the + * calculation i.e. the MAC-calculation is the same as TLS + * even though DTLS handshake messages may be fragmented. */ if (!dtls1_write_hm_header(msgstart, msg_type, msg_len, - msg_seq, s->init_off, 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 @@ -321,9 +335,8 @@ int dtls1_do_write(SSL_CONNECTION *s, uint8_t recordtype) || (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)) { + if (!ssl3_finish_mac(s, msgstart, xlen, hmhdr_incl)) return -1; - } } } @@ -400,9 +413,6 @@ int dtls_get_message(SSL_CONNECTION *s, int *mt) */ int dtls_get_message_body(SSL_CONNECTION *s, size_t *len) { - unsigned char *msg = (unsigned char *)s->init_buf->data; - size_t msg_len = s->init_num + DTLS1_HM_HEADER_LENGTH; - if (s->s3.tmp.message_type == SSL3_MT_CHANGE_CIPHER_SPEC) { /* Nothing to be done */ goto end; @@ -416,12 +426,7 @@ int dtls_get_message_body(SSL_CONNECTION *s, size_t *len) return 0; } - if (s->version == DTLS1_BAD_VER) { - msg += DTLS1_HM_HEADER_LENGTH; - msg_len -= DTLS1_HM_HEADER_LENGTH; - } - - if (!ssl3_finish_mac(s, msg, msg_len)) + if (!tls_common_finish_mac(s)) return 0; if (s->msg_callback) diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 3eb37b09bf9..b738090d72c 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -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)) + written, 1)) return -1; if (written == s->init_num) { s->statem.write_in_progress = 0; @@ -1639,6 +1639,55 @@ int tls_get_message_header(SSL_CONNECTION *s, int *mt) return 1; } +int tls_common_finish_mac(SSL_CONNECTION *s) { + unsigned char *msg = (unsigned char *)s->init_buf->data; + size_t msg_len = s->init_num; + size_t hdr_len = 0; + + if (SSL_CONNECTION_IS_DTLS(s)) { + if (s->version != DTLS1_BAD_VER) + hdr_len = DTLS1_HM_HEADER_LENGTH; + else + msg += DTLS1_HM_HEADER_LENGTH; + } else if (!RECORD_LAYER_is_sslv2_record(&s->rlayer)) { + hdr_len = SSL3_HM_HEADER_LENGTH; + } + + msg_len += hdr_len; + + /* Feed this message into MAC computation. */ + if (RECORD_LAYER_is_sslv2_record(&s->rlayer) + && !ssl3_finish_mac(s, msg, msg_len, 1)) { + /* SSLfatal() already called */ + return 0; + } else { + /* + * We defer feeding in the HRR until later. We'll do it as part of + * the message processing. + * The (D)TLSv1.3 handshake transcript stops at the ClientFinished + * message. + */ + const size_t srvhellorandom_offs = hdr_len + 2; + + /* KeyUpdate and NewSessionTicket do not need to be added */ + if (!SSL_CONNECTION_IS_VERSION13(s) + || (s->s3.tmp.message_type != SSL3_MT_NEWSESSION_TICKET + && s->s3.tmp.message_type != SSL3_MT_KEY_UPDATE)) { + if (s->s3.tmp.message_type != SSL3_MT_SERVER_HELLO + || s->init_num < srvhellorandom_offs + SSL3_RANDOM_SIZE + || memcmp(hrrrandom, + s->init_buf->data + srvhellorandom_offs, + SSL3_RANDOM_SIZE) != 0) { + if (!ssl3_finish_mac(s, msg, msg_len, 1)) + /* SSLfatal() already called */ + return 0; + } + } + } + + return 1; +} + int tls_get_message_body(SSL_CONNECTION *s, size_t *len) { size_t n, readbytes; @@ -1677,45 +1726,20 @@ int tls_get_message_body(SSL_CONNECTION *s, size_t *len) return 0; } - /* Feed this message into MAC computation. */ + if (!tls_common_finish_mac(s)) { + /* SSLfatal() already called */ + *len = 0; + return 0; + } + if (RECORD_LAYER_is_sslv2_record(&s->rlayer)) { - if (!ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, - s->init_num)) { - /* SSLfatal() already called */ - *len = 0; - return 0; - } if (s->msg_callback) s->msg_callback(0, SSL2_VERSION, 0, s->init_buf->data, - (size_t)s->init_num, ussl, s->msg_callback_arg); + s->init_num, ussl, s->msg_callback_arg); } else { - /* - * We defer feeding in the HRR until later. We'll do it as part of - * processing the message - * The TLsv1.3 handshake transcript stops at the ClientFinished - * message. - */ -#define SERVER_HELLO_RANDOM_OFFSET (SSL3_HM_HEADER_LENGTH + 2) - /* KeyUpdate and NewSessionTicket do not need to be added */ - if (!SSL_CONNECTION_IS_VERSION13(s) - || (s->s3.tmp.message_type != SSL3_MT_NEWSESSION_TICKET - && s->s3.tmp.message_type != SSL3_MT_KEY_UPDATE)) { - if (s->s3.tmp.message_type != SSL3_MT_SERVER_HELLO - || s->init_num < SERVER_HELLO_RANDOM_OFFSET + SSL3_RANDOM_SIZE - || memcmp(hrrrandom, - s->init_buf->data + SERVER_HELLO_RANDOM_OFFSET, - SSL3_RANDOM_SIZE) != 0) { - if (!ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, - s->init_num + SSL3_HM_HEADER_LENGTH)) { - /* SSLfatal() already called */ - *len = 0; - return 0; - } - } - } if (s->msg_callback) s->msg_callback(0, s->version, SSL3_RT_HANDSHAKE, s->init_buf->data, - (size_t)s->init_num + SSL3_HM_HEADER_LENGTH, ussl, + s->init_num + SSL3_HM_HEADER_LENGTH, ussl, s->msg_callback_arg); } @@ -2631,9 +2655,11 @@ int create_synthetic_message_hash(SSL_CONNECTION *s, size_t hrrlen) { unsigned char hashvaltmp[EVP_MAX_MD_SIZE]; - unsigned char msghdr[SSL3_HM_HEADER_LENGTH]; + unsigned char synmsghdr[SSL3_HM_HEADER_LENGTH]; + size_t currmsghdr_len = SSL_CONNECTION_IS_DTLS(s) ? DTLS1_HM_HEADER_LENGTH + : SSL3_HM_HEADER_LENGTH; - memset(msghdr, 0, sizeof(msghdr)); + memset(synmsghdr, 0, sizeof(synmsghdr)); if (hashval == NULL) { hashval = hashvaltmp; @@ -2654,10 +2680,10 @@ int create_synthetic_message_hash(SSL_CONNECTION *s, } /* Inject the synthetic message_hash message */ - msghdr[0] = SSL3_MT_MESSAGE_HASH; - msghdr[SSL3_HM_HEADER_LENGTH - 1] = (unsigned char)hashlen; - if (!ssl3_finish_mac(s, msghdr, SSL3_HM_HEADER_LENGTH) - || !ssl3_finish_mac(s, hashval, hashlen)) { + 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)) { /* SSLfatal() already called */ return 0; } @@ -2668,10 +2694,9 @@ int create_synthetic_message_hash(SSL_CONNECTION *s, * receiving a ClientHello2 with a cookie. */ if (hrr != NULL - && (!ssl3_finish_mac(s, hrr, hrrlen) + && (!ssl3_finish_mac(s, hrr, hrrlen, 1) || !ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, - s->s3.tmp.message_size - + SSL3_HM_HEADER_LENGTH))) { + s->s3.tmp.message_size + currmsghdr_len, 1))) { /* SSLfatal() already called */ return 0; } diff --git a/ssl/statem/statem_local.h b/ssl/statem/statem_local.h index 04114b1e27d..8d105c4b9d3 100644 --- a/ssl/statem/statem_local.h +++ b/ssl/statem/statem_local.h @@ -113,6 +113,7 @@ __owur int tls_get_message_header(SSL_CONNECTION *s, int *mt); __owur int tls_get_message_body(SSL_CONNECTION *s, size_t *len); __owur int dtls_get_message(SSL_CONNECTION *s, int *mt); __owur int dtls_get_message_body(SSL_CONNECTION *s, size_t *len); +__owur int tls_common_finish_mac(SSL_CONNECTION *s); /* Message construction and processing functions */ __owur int tls_process_initial_server_flight(SSL_CONNECTION *s); -- 2.47.2