mirror of https://github.com/openssl/openssl.git
				
				
				
			This change fixes an issue where a DTLS 1.3 would calculate a wrong transcript hash.
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:
		
							parent
							
								
									9eb40ddded
								
							
						
					
					
						commit
						a0634d5e98
					
				|  | @ -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 | ||||
| 
 | ||||
|  |  | |||
|  | @ -1482,11 +1482,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
								
								
								
								
							
							
						
						
									
										94
									
								
								ssl/s3_enc.c
								
								
								
								
							|  | @ -245,7 +245,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; | ||||
| 
 | ||||
|  | @ -274,20 +274,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; | ||||
|  | @ -322,33 +360,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) { | ||||
|  |  | |||
|  | @ -1311,6 +1311,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 | ||||
|  | @ -2758,7 +2765,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); | ||||
|  |  | |||
|  | @ -713,7 +713,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 */ | ||||
|  | @ -1920,7 +1920,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; | ||||
|     } | ||||
|  |  | |||
|  | @ -302,15 +302,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: | ||||
|  | @ -329,14 +328,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 | ||||
|  | @ -345,12 +338,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, | ||||
|  |  | |||
|  | @ -108,7 +108,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; | ||||
|  | @ -1673,7 +1673,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 { | ||||
|  | @ -1694,7 +1694,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; | ||||
|             } | ||||
|  | @ -2673,23 +2673,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 */ | ||||
|  | @ -2699,10 +2703,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; | ||||
|     } | ||||
|  | @ -2713,9 +2716,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; | ||||
|     } | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue