diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c index 9a82e3ffa2..866ef18381 100644 --- a/ssl/record/rec_layer_d1.c +++ b/ssl/record/rec_layer_d1.c @@ -939,7 +939,7 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf, if (eivlen) SSL3_RECORD_add_length(&wr, eivlen); - if (s->method->ssl3_enc->enc(s, &wr, 1, 1) < 1) { + if (s->method->ssl3_enc->enc(s, &wr, 1, 1, NULL, mac_size) < 1) { if (!ossl_statem_in_error(s)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DO_DTLS1_WRITE, ERR_R_INTERNAL_ERROR); diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index fac3506b19..8ea16672b6 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -1044,7 +1044,7 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, * We haven't actually negotiated the version yet, but we're trying to * send early data - so we need to use the tls13enc function. */ - if (tls13_enc(s, wr, numpipes, 1) < 1) { + if (tls13_enc(s, wr, numpipes, 1, NULL, mac_size) < 1) { if (!ossl_statem_in_error(s)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); @@ -1053,7 +1053,8 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, } } else { if (!BIO_get_ktls_send(s->wbio)) { - if (s->method->ssl3_enc->enc(s, wr, numpipes, 1) < 1) { + if (s->method->ssl3_enc->enc(s, wr, numpipes, 1, NULL, + mac_size) < 1) { if (!ossl_statem_in_error(s)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); diff --git a/ssl/record/record.h b/ssl/record/record.h index 0504a6f959..234656bf93 100644 --- a/ssl/record/record.h +++ b/ssl/record/record.h @@ -178,6 +178,12 @@ typedef struct record_layer_st { * * *****************************************************************************/ +struct ssl_mac_buf_st { + unsigned char *mac; + int alloced; +}; +typedef struct ssl_mac_buf_st SSL_MAC_BUF; + #define MIN_SSL2_RECORD_LEN 9 #define RECORD_LAYER_set_read_ahead(rl, ra) ((rl)->read_ahead = (ra)) @@ -213,13 +219,16 @@ __owur int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, size_t len, int peek, size_t *readbytes); __owur int ssl3_setup_buffers(SSL *s); -__owur int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int send); +__owur int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int send, + SSL_MAC_BUF *mac, size_t macsize); __owur int n_ssl3_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int send); __owur int ssl3_write_pending(SSL *s, int type, const unsigned char *buf, size_t len, size_t *written); -__owur int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int send); +__owur int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending, + SSL_MAC_BUF *mac, size_t macsize); __owur int tls1_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int send); -__owur int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int send); +__owur int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int send, + SSL_MAC_BUF *mac, size_t macsize); int DTLS_RECORD_LAYER_new(RECORD_LAYER *rl); void DTLS_RECORD_LAYER_free(RECORD_LAYER *rl); void DTLS_RECORD_LAYER_clear(RECORD_LAYER *rl); diff --git a/ssl/record/record_local.h b/ssl/record/record_local.h index f7734d832b..dc92732243 100644 --- a/ssl/record/record_local.h +++ b/ssl/record/record_local.h @@ -107,13 +107,16 @@ void SSL3_RECORD_set_seq_num(SSL3_RECORD *r, const unsigned char *seq_num); int ssl3_get_record(SSL *s); __owur int ssl3_do_compress(SSL *ssl, SSL3_RECORD *wr); __owur int ssl3_do_uncompress(SSL *ssl, SSL3_RECORD *rr); -int ssl3_cbc_copy_mac(unsigned char *out, - const SSL3_RECORD *rec, size_t md_size); -__owur int ssl3_cbc_remove_padding(SSL3_RECORD *rec, - size_t block_size, size_t mac_size); -__owur int tls1_cbc_remove_padding(const SSL *s, - SSL3_RECORD *rec, - size_t block_size, size_t mac_size); +__owur int ssl3_cbc_remove_padding_and_mac(SSL *s, + SSL3_RECORD *rec, + unsigned char **mac, + int *alloced, + size_t block_size, size_t mac_size); +__owur int tls1_cbc_remove_padding_and_mac(const SSL *s, + SSL3_RECORD *rec, + unsigned char **mac, + int *alloced, + size_t block_size, size_t mac_size); int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap); __owur int dtls1_get_record(SSL *s); int early_data_count_ok(SSL *s, size_t length, size_t overhead, int send); diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c index a2f7f848d1..3b1007f574 100644 --- a/ssl/record/ssl3_record.c +++ b/ssl/record/ssl3_record.c @@ -32,6 +32,14 @@ static const unsigned char ssl3_pad_2[48] = { 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c }; +static int ssl3_cbc_copy_mac(const SSL *s, + SSL3_RECORD *rec, + unsigned char **mac, + int *alloced, + size_t block_size, + size_t mac_size, + size_t good); + /* * Clear the contents of an SSL3_RECORD but retain any memory allocated */ @@ -182,12 +190,13 @@ int ssl3_get_record(SSL *s) unsigned char *p; unsigned char md[EVP_MAX_MD_SIZE]; unsigned int version; - size_t mac_size; + size_t mac_size = 0; int imac_size; size_t num_recs = 0, max_recs, j; PACKET pkt, sslv2pkt; - size_t first_rec_len; int is_ktls_left; + SSL_MAC_BUF *macbufs = NULL; + int ret = -1; rr = RECORD_LAYER_get_rrec(&s->rlayer); rbuf = RECORD_LAYER_get_rbuf(&s->rlayer); @@ -526,20 +535,28 @@ int ssl3_get_record(SSL *s) if (BIO_get_ktls_recv(s->rbio) && !is_ktls_left) goto skip_decryption; + /* TODO(size_t): convert this to do size_t properly */ + if (s->read_hash != NULL) { + const EVP_MD *tmpmd = EVP_MD_CTX_md(s->read_hash); + + if (tmpmd != NULL) { + imac_size = EVP_MD_size(tmpmd); + if (!ossl_assert(imac_size >= 0 && imac_size <= EVP_MAX_MD_SIZE)) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD, + ERR_LIB_EVP); + return -1; + } + mac_size = (size_t)imac_size; + } + } + /* * If in encrypt-then-mac mode calculate mac from encrypted record. All * the details below are public so no timing details can leak. */ if (SSL_READ_ETM(s) && s->read_hash) { unsigned char *mac; - /* TODO(size_t): convert this to do size_t properly */ - imac_size = EVP_MD_CTX_size(s->read_hash); - if (!ossl_assert(imac_size >= 0 && imac_size <= EVP_MAX_MD_SIZE)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD, - ERR_LIB_EVP); - return -1; - } - mac_size = (size_t)imac_size; + for (j = 0; j < num_recs; j++) { thisrr = &rr[j]; @@ -557,27 +574,39 @@ int ssl3_get_record(SSL *s) return -1; } } + /* + * We've handled the mac now - there is no MAC inside the encrypted + * record + */ + mac_size = 0; } - first_rec_len = rr[0].length; + if (mac_size > 0) { + macbufs = OPENSSL_zalloc(sizeof(*macbufs) * num_recs); + if (macbufs == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD, + ERR_R_MALLOC_FAILURE); + return -1; + } + } - enc_err = s->method->ssl3_enc->enc(s, rr, num_recs, 0); + enc_err = s->method->ssl3_enc->enc(s, rr, num_recs, 0, macbufs, mac_size); /*- * enc_err is: - * 0: (in non-constant time) if the record is publicly invalid. - * 1: if the padding is valid - * -1: if the padding is invalid + * 0: if the record is publicly invalid, or an internal error, or AEAD + * decryption failed, or ETM decryption failed. + * 1: Success or MTE decryption failed (MAC will be randomised) */ if (enc_err == 0) { if (ossl_statem_in_error(s)) { /* SSLfatal() already got called */ - return -1; + goto end; } if (num_recs == 1 && ossl_statem_skip_early_data(s)) { /* - * Valid early_data that we cannot decrypt might fail here as - * publicly invalid. We treat it like an empty record. + * Valid early_data that we cannot decrypt will fail here. We treat + * it like an empty record. */ thisrr = &rr[0]; @@ -585,18 +614,19 @@ int ssl3_get_record(SSL *s) if (!early_data_count_ok(s, thisrr->length, EARLY_DATA_CIPHERTEXT_OVERHEAD, 0)) { /* SSLfatal() already called */ - return -1; + goto end; } thisrr->length = 0; thisrr->read = 1; RECORD_LAYER_set_numrpipes(&s->rlayer, 1); RECORD_LAYER_reset_read_sequence(&s->rlayer); - return 1; + ret = 1; + goto end; } SSLfatal(s, SSL_AD_BAD_RECORD_MAC, SSL_F_SSL3_GET_RECORD, - SSL_R_BLOCK_CIPHER_PAD_IS_WRONG); - return -1; + SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); + goto end; } OSSL_TRACE_BEGIN(TLS) { BIO_printf(trc_out, "dec %lu\n", (unsigned long)rr[0].length); @@ -608,93 +638,24 @@ int ssl3_get_record(SSL *s) (s->enc_read_ctx != NULL) && (!SSL_READ_ETM(s) && EVP_MD_CTX_md(s->read_hash) != NULL)) { /* s->read_hash != NULL => mac_size != -1 */ - unsigned char *mac = NULL; - unsigned char mac_tmp[EVP_MAX_MD_SIZE]; - - mac_size = EVP_MD_CTX_size(s->read_hash); - if (!ossl_assert(mac_size <= EVP_MAX_MD_SIZE)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD, - ERR_R_INTERNAL_ERROR); - return -1; - } for (j = 0; j < num_recs; j++) { + SSL_MAC_BUF *thismb = &macbufs[j]; thisrr = &rr[j]; - /* - * orig_len is the length of the record before any padding was - * removed. This is public information, as is the MAC in use, - * therefore we can safely process the record in a different amount - * of time if it's too short to possibly contain a MAC. - */ - if (thisrr->orig_len < mac_size || - /* CBC records must have a padding length byte too. */ - (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE && - thisrr->orig_len < mac_size + 1)) { - SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_SSL3_GET_RECORD, - SSL_R_LENGTH_TOO_SHORT); - return -1; - } - - if (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE) { - /* - * We update the length so that the TLS header bytes can be - * constructed correctly but we need to extract the MAC in - * constant time from within the record, without leaking the - * contents of the padding bytes. - */ - mac = mac_tmp; - if (!ssl3_cbc_copy_mac(mac_tmp, thisrr, mac_size)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD, - ERR_R_INTERNAL_ERROR); - return -1; - } - thisrr->length -= mac_size; - } else { - /* - * In this case there's no padding, so |rec->orig_len| equals - * |rec->length| and we checked that there's enough bytes for - * |mac_size| above. - */ - thisrr->length -= mac_size; - mac = &thisrr->data[thisrr->length]; - } i = s->method->ssl3_enc->mac(s, thisrr, md, 0 /* not send */ ); - if (i == 0 || mac == NULL - || CRYPTO_memcmp(md, mac, (size_t)mac_size) != 0) - enc_err = -1; + if (i == 0 || thismb == NULL || thismb->mac == NULL + || CRYPTO_memcmp(md, thismb->mac, (size_t)mac_size) != 0) + enc_err = 0; if (thisrr->length > SSL3_RT_MAX_COMPRESSED_LENGTH + mac_size) - enc_err = -1; + enc_err = 0; } } - if (enc_err < 0) { + if (enc_err == 0) { if (ossl_statem_in_error(s)) { /* We already called SSLfatal() */ - return -1; - } - if (num_recs == 1 && ossl_statem_skip_early_data(s)) { - /* - * We assume this is unreadable early_data - we treat it like an - * empty record - */ - - /* - * The record length may have been modified by the mac check above - * so we use the previously saved value - */ - if (!early_data_count_ok(s, first_rec_len, - EARLY_DATA_CIPHERTEXT_OVERHEAD, 0)) { - /* SSLfatal() already called */ - return -1; - } - - thisrr = &rr[0]; - thisrr->length = 0; - thisrr->read = 1; - RECORD_LAYER_set_numrpipes(&s->rlayer, 1); - RECORD_LAYER_reset_read_sequence(&s->rlayer); - return 1; + goto end; } /* * A separate 'decryption_failed' alert was introduced with TLS 1.0, @@ -705,7 +666,7 @@ int ssl3_get_record(SSL *s) */ SSLfatal(s, SSL_AD_BAD_RECORD_MAC, SSL_F_SSL3_GET_RECORD, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); - return -1; + goto end; } skip_decryption: @@ -718,12 +679,12 @@ int ssl3_get_record(SSL *s) if (thisrr->length > SSL3_RT_MAX_COMPRESSED_LENGTH) { SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_SSL3_GET_RECORD, SSL_R_COMPRESSED_LENGTH_TOO_LONG); - return -1; + goto end; } if (!ssl3_do_uncompress(s, thisrr)) { SSLfatal(s, SSL_AD_DECOMPRESSION_FAILURE, SSL_F_SSL3_GET_RECORD, SSL_R_BAD_DECOMPRESSION); - return -1; + goto end; } } @@ -736,7 +697,7 @@ int ssl3_get_record(SSL *s) || thisrr->type != SSL3_RT_APPLICATION_DATA) { SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_GET_RECORD, SSL_R_BAD_RECORD_TYPE); - return -1; + goto end; } /* Strip trailing padding */ @@ -751,7 +712,7 @@ int ssl3_get_record(SSL *s) && thisrr->type != SSL3_RT_HANDSHAKE) { SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_GET_RECORD, SSL_R_BAD_RECORD_TYPE); - return -1; + goto end; } if (s->msg_callback) s->msg_callback(0, s->version, SSL3_RT_INNER_CONTENT_TYPE, @@ -768,13 +729,13 @@ int ssl3_get_record(SSL *s) && thisrr->length == 0) { SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_GET_RECORD, SSL_R_BAD_LENGTH); - return -1; + goto end; } if (thisrr->length > SSL3_RT_MAX_PLAIN_LENGTH && !BIO_get_ktls_recv(s->rbio)) { SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_SSL3_GET_RECORD, SSL_R_DATA_LENGTH_TOO_LONG); - return -1; + goto end; } /* If received packet overflows current Max Fragment Length setting */ @@ -783,7 +744,7 @@ int ssl3_get_record(SSL *s) && !BIO_get_ktls_recv(s->rbio)) { SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_SSL3_GET_RECORD, SSL_R_DATA_LENGTH_TOO_LONG); - return -1; + goto end; } thisrr->off = 0; @@ -802,7 +763,7 @@ int ssl3_get_record(SSL *s) > MAX_EMPTY_RECORDS) { SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_GET_RECORD, SSL_R_RECORD_TOO_SMALL); - return -1; + goto end; } } else { RECORD_LAYER_reset_empty_record_count(&s->rlayer); @@ -814,12 +775,21 @@ int ssl3_get_record(SSL *s) if (thisrr->type == SSL3_RT_APPLICATION_DATA && !early_data_count_ok(s, thisrr->length, 0, 0)) { /* SSLfatal already called */ - return -1; + goto end; } } RECORD_LAYER_set_numrpipes(&s->rlayer, num_recs); - return 1; + ret = 1; + end: + if (macbufs != NULL) { + for (j = 0; j < num_recs; j++) { + if (macbufs[j].alloced) + OPENSSL_free(macbufs[j].mac); + } + OPENSSL_free(macbufs); + } + return ret; } int ssl3_do_uncompress(SSL *ssl, SSL3_RECORD *rr) @@ -866,23 +836,21 @@ int ssl3_do_compress(SSL *ssl, SSL3_RECORD *wr) } /*- - * ssl3_enc encrypts/decrypts |n_recs| records in |inrecs|. Will call - * SSLfatal() for internal errors, but not otherwise. + * ssl3_enc encrypts/decrypts |n_recs| records in |inrecs|. Calls SSLfatal on + * internal error, but not otherwise. It is the responsibility of the caller to + * report a bad_record_mac * * Returns: - * 0: (in non-constant time) if the record is publicly invalid (i.e. too - * short etc). - * 1: if the record's padding is valid / the encryption was successful. - * -1: if the record's padding is invalid or, if sending, an internal error - * occurred. + * 0: if the record is publicly invalid, or an internal error + * 1: Success or Mac-then-encrypt decryption failed (MAC will be randomised) */ -int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int sending) +int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int sending, + SSL_MAC_BUF *mac, size_t macsize) { SSL3_RECORD *rec; EVP_CIPHER_CTX *ds; size_t l, i; - size_t bs, mac_size = 0; - int imac_size; + size_t bs; const EVP_CIPHER *enc; rec = inrecs; @@ -930,52 +898,50 @@ int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int sending) } if (!sending) { - if (l == 0 || l % bs != 0) + if (l == 0 || l % bs != 0) { + /* Publicly invalid */ return 0; + } /* otherwise, rec->length >= bs */ } /* TODO(size_t): Convert this call */ - if (EVP_Cipher(ds, rec->data, rec->input, (unsigned int)l) < 1) - return -1; - - if (EVP_MD_CTX_md(s->read_hash) != NULL) { - /* TODO(size_t): convert me */ - imac_size = EVP_MD_CTX_size(s->read_hash); - if (imac_size < 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_ENC, - ERR_R_INTERNAL_ERROR); - return -1; - } - mac_size = (size_t)imac_size; + if (EVP_Cipher(ds, rec->data, rec->input, (unsigned int)l) < 1) { + /* Shouldn't happen */ + SSLfatal(s, SSL_AD_BAD_RECORD_MAC, 0, ERR_R_INTERNAL_ERROR); + return 0; } - if ((bs != 1) && !sending) - return ssl3_cbc_remove_padding(rec, bs, mac_size); + + if (!sending) + return !ssl3_cbc_remove_padding_and_mac(s, rec, + (mac != NULL) ? &mac->mac : NULL, + (mac != NULL) ? &mac->alloced : NULL, + bs, + macsize); } return 1; } #define MAX_PADDING 256 /*- - * tls1_enc encrypts/decrypts |n_recs| in |recs|. Will call SSLfatal() for - * internal errors, but not otherwise. + * tls1_enc encrypts/decrypts |n_recs| in |recs|. Calls SSLfatal on internal + * error, but not otherwise. It is the responsibility of the caller to report + * a bad_record_mac - if appropriate (DTLS just drops the record). * * Returns: - * 0: (in non-constant time) if the record is publicly invalid (i.e. too - * short etc). - * 1: if the record's padding is valid / the encryption was successful. - * -1: if the record's padding/AEAD-authenticator is invalid or, if sending, - * an internal error occurred. + * 0: if the record is publicly invalid, or an internal error, or AEAD + * decryption failed, or Encrypt-then-mac decryption failed. + * 1: Success or Mac-then-encrypt decryption failed (MAC will be randomised) */ -int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) +int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending, + SSL_MAC_BUF *macs, size_t macsize) { EVP_CIPHER_CTX *ds; size_t reclen[SSL_MAX_PIPELINES]; unsigned char buf[SSL_MAX_PIPELINES][EVP_AEAD_TLS1_AAD_LEN]; - int i, pad = 0, ret, tmpr; - size_t bs, mac_size = 0, ctr, padnum, loop; + int i, pad = 0, tmpr; + size_t bs, ctr, padnum, loop; unsigned char padval; - int imac_size; const EVP_CIPHER *enc; int tlstree_enc = sending ? (s->mac_flags & SSL_MAC_FLAG_WRITE_MAC_TLSTREE) : (s->mac_flags & SSL_MAC_FLAG_READ_MAC_TLSTREE); @@ -992,7 +958,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) if (!ossl_assert(n >= 0)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } } ds = s->enc_write_ctx; @@ -1016,12 +982,12 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) */ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } else if (RAND_bytes_ex(s->ctx->libctx, recs[ctr].input, ivlen) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } } } @@ -1032,7 +998,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) if (!ossl_assert(n >= 0)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } } ds = s->enc_read_ctx; @@ -1047,7 +1013,6 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) memmove(recs[ctr].data, recs[ctr].input, recs[ctr].length); recs[ctr].input = recs[ctr].data; } - ret = 1; } else { bs = EVP_CIPHER_block_size(EVP_CIPHER_CTX_cipher(ds)); @@ -1060,7 +1025,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) */ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, SSL_R_PIPELINE_FAILURE); - return -1; + return 0; } } for (ctr = 0; ctr < n_recs; ctr++) { @@ -1100,7 +1065,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) if (pad <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } if (sending) { @@ -1116,7 +1081,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) if (padnum > MAX_PADDING) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } /* we need to add 'padnum' padding bytes of value padval */ padval = (unsigned char)(padnum - 1); @@ -1127,8 +1092,10 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) } if (!sending) { - if (reclen[ctr] == 0 || reclen[ctr] % bs != 0) + if (reclen[ctr] == 0 || reclen[ctr] % bs != 0) { + /* Publicly invalid */ return 0; + } } } if (n_recs > 1) { @@ -1142,7 +1109,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) (int)n_recs, data) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, SSL_R_PIPELINE_FAILURE); - return -1; + return 0; } /* Set the input buffers */ for (ctr = 0; ctr < n_recs; ctr++) { @@ -1154,7 +1121,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) (int)n_recs, reclen) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, SSL_R_PIPELINE_FAILURE); - return -1; + return 0; } } @@ -1175,7 +1142,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) if (EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_TLSTREE, decrement_seq, seq) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } } @@ -1185,8 +1152,10 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) if ((EVP_CIPHER_flags(EVP_CIPHER_CTX_cipher(ds)) & EVP_CIPH_FLAG_CUSTOM_CIPHER) ? (tmpr < 0) - : (tmpr == 0)) - return -1; /* AEAD can fail to verify MAC */ + : (tmpr == 0)) { + /* AEAD can fail to verify MAC */ + return 0; + } if (sending == 0) { if (EVP_CIPHER_mode(enc) == EVP_CIPH_GCM_MODE) { @@ -1204,29 +1173,18 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) } } - ret = 1; - if (!SSL_READ_ETM(s) && EVP_MD_CTX_md(s->read_hash) != NULL) { - imac_size = EVP_MD_CTX_size(s->read_hash); - if (imac_size < 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC, - ERR_R_INTERNAL_ERROR); - return -1; - } - mac_size = (size_t)imac_size; - } - if ((bs != 1) && !sending) { - int tmpret; + if (!sending) { for (ctr = 0; ctr < n_recs; ctr++) { - tmpret = tls1_cbc_remove_padding(s, &recs[ctr], bs, mac_size); /* - * If tmpret == 0 then this means publicly invalid so we can - * short circuit things here. Otherwise we must respect constant - * time behaviour. + * If using Mac-then-encrypt, then this will succeed but with a + * random MAC if padding is invalid */ - if (tmpret == 0) + if (!tls1_cbc_remove_padding_and_mac(s, &recs[ctr], + (macs != NULL) ? &macs[ctr].mac : NULL, + (macs != NULL) ? &macs[ctr].alloced : NULL, + bs, + macsize)) return 0; - ret = constant_time_select_int(constant_time_eq_int(tmpret, 1), - ret, -1); } } if (pad && !sending) { @@ -1235,7 +1193,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) } } } - return ret; + return 1; } int n_ssl3_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int sending) @@ -1448,16 +1406,20 @@ int tls1_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int sending) /*- * ssl3_cbc_remove_padding removes padding from the decrypted, SSLv3, CBC - * record in |rec| by updating |rec->length| in constant time. + * record in |rec| by updating |rec->length| in constant time. It also extracts + * the MAC from the underlying record. * * block_size: the block size of the cipher used to encrypt the record. * returns: - * 0: (in non-constant time) if the record is publicly invalid. - * 1: if the padding was valid - * -1: otherwise. + * 0: if the record is publicly invalid. + * 1: if the record is publicly valid. If the padding removal fails then the + * MAC returned is random. */ -int ssl3_cbc_remove_padding(SSL3_RECORD *rec, - size_t block_size, size_t mac_size) +int ssl3_cbc_remove_padding_and_mac(SSL *s, + SSL3_RECORD *rec, + unsigned char **mac, + int *alloced, + size_t block_size, size_t mac_size) { size_t padding_length; size_t good; @@ -1474,86 +1436,95 @@ int ssl3_cbc_remove_padding(SSL3_RECORD *rec, /* SSLv3 requires that the padding is minimal. */ good &= constant_time_ge_s(block_size, padding_length + 1); rec->length -= good & (padding_length + 1); - return constant_time_select_int_s(good, 1, -1); + + return ssl3_cbc_copy_mac(s, rec, mac, alloced, block_size, mac_size, good); } /*- * tls1_cbc_remove_padding removes the CBC padding from the decrypted, TLS, CBC - * record in |rec| in constant time and returns 1 if the padding is valid and - * -1 otherwise. It also removes any explicit IV from the start of the record - * without leaking any timing about whether there was enough space after the - * padding was removed. + * record in |rec| in constant time. It also removes any explicit IV from the + * start of the record without leaking any timing about whether there was enough + * space after the padding was removed, as well as extracting the embedded MAC + * (also in constant time). For Mac-then-encrypt, if the padding is invalid then + * a success result will occur and a randomised MAC will be returned. * * block_size: the block size of the cipher used to encrypt the record. * returns: - * 0: (in non-constant time) if the record is publicly invalid. - * 1: if the padding was valid - * -1: otherwise. + * 0: if the record is publicly invalid, or an internal error + * 1: Success or Mac-then-encrypt decryption failed (MAC will be randomised) */ -int tls1_cbc_remove_padding(const SSL *s, - SSL3_RECORD *rec, - size_t block_size, size_t mac_size) +int tls1_cbc_remove_padding_and_mac(const SSL *s, + SSL3_RECORD *rec, + unsigned char **mac, + int *alloced, + size_t block_size, size_t mac_size) { size_t good; size_t padding_length, to_check, i; - const size_t overhead = 1 /* padding length byte */ + mac_size; - /* Check if version requires explicit IV */ - if (SSL_USE_EXPLICIT_IV(s)) { - /* - * These lengths are all public so we can test them in non-constant - * time. - */ - if (overhead + block_size > rec->length) - return 0; - /* We can now safely skip explicit IV */ - rec->data += block_size; - rec->input += block_size; - rec->length -= block_size; - rec->orig_len -= block_size; - } else if (overhead > rec->length) + size_t overhead = ((block_size == 1) ? 0 : 1) /* padding length byte */ + + (SSL_USE_EXPLICIT_IV(s) ? block_size : 0) + + mac_size; + + /* + * These lengths are all public so we can test them in non-constant + * time. + */ + if (overhead > rec->length) return 0; - padding_length = rec->data[rec->length - 1]; + if (block_size != 1) { + if (SSL_USE_EXPLICIT_IV(s)) { + rec->data += block_size; + rec->input += block_size; + rec->length -= block_size; + rec->orig_len -= block_size; + overhead -= block_size; + } - if (EVP_CIPHER_flags(EVP_CIPHER_CTX_cipher(s->enc_read_ctx)) & - EVP_CIPH_FLAG_AEAD_CIPHER) { - /* padding is already verified */ - rec->length -= padding_length + 1; - return 1; - } + padding_length = rec->data[rec->length - 1]; - good = constant_time_ge_s(rec->length, overhead + padding_length); - /* - * The padding consists of a length byte at the end of the record and - * then that many bytes of padding, all with the same value as the length - * byte. Thus, with the length byte included, there are i+1 bytes of - * padding. We can't check just |padding_length+1| bytes because that - * leaks decrypted information. Therefore we always have to check the - * maximum amount of padding possible. (Again, the length of the record - * is public information so we can use it.) - */ - to_check = 256; /* maximum amount of padding, inc length byte. */ - if (to_check > rec->length) - to_check = rec->length; + if (EVP_CIPHER_flags(EVP_CIPHER_CTX_cipher(s->enc_read_ctx)) & + EVP_CIPH_FLAG_AEAD_CIPHER) { + /* padding is already verified and we don't need to check the MAC */ + rec->length -= padding_length + 1 + mac_size; + *mac = NULL; + *alloced = 0; + return 1; + } - for (i = 0; i < to_check; i++) { - unsigned char mask = constant_time_ge_8_s(padding_length, i); - unsigned char b = rec->data[rec->length - 1 - i]; + good = constant_time_ge_s(rec->length, overhead + padding_length); /* - * The final |padding_length+1| bytes should all have the value - * |padding_length|. Therefore the XOR should be zero. + * The padding consists of a length byte at the end of the record and + * then that many bytes of padding, all with the same value as the + * length byte. Thus, with the length byte included, there are i+1 bytes + * of padding. We can't check just |padding_length+1| bytes because that + * leaks decrypted information. Therefore we always have to check the + * maximum amount of padding possible. (Again, the length of the record + * is public information so we can use it.) */ - good &= ~(mask & (padding_length ^ b)); + to_check = 256; /* maximum amount of padding, inc length byte. */ + if (to_check > rec->length) + to_check = rec->length; + + for (i = 0; i < to_check; i++) { + unsigned char mask = constant_time_ge_8_s(padding_length, i); + unsigned char b = rec->data[rec->length - 1 - i]; + /* + * The final |padding_length+1| bytes should all have the value + * |padding_length|. Therefore the XOR should be zero. + */ + good &= ~(mask & (padding_length ^ b)); + } + + /* + * If any of the final |padding_length+1| bytes had the wrong value, one + * or more of the lower eight bits of |good| will be cleared. + */ + good = constant_time_eq_s(0xff, good & 0xff); + rec->length -= good & (padding_length + 1); } - /* - * If any of the final |padding_length+1| bytes had the wrong value, one - * or more of the lower eight bits of |good| will be cleared. - */ - good = constant_time_eq_s(0xff, good & 0xff); - rec->length -= good & (padding_length + 1); - - return constant_time_select_int_s(good, 1, -1); + return ssl3_cbc_copy_mac(s, rec, mac, alloced, block_size, mac_size, good); } /*- @@ -1561,9 +1532,6 @@ int tls1_cbc_remove_padding(const SSL *s, * constant time (independent of the concrete value of rec->length, which may * vary within a 256-byte window). * - * ssl3_cbc_remove_padding or tls1_cbc_remove_padding must be called prior to - * this function. - * * On entry: * rec->orig_len >= md_size * md_size <= EVP_MAX_MD_SIZE @@ -1576,8 +1544,13 @@ int tls1_cbc_remove_padding(const SSL *s, */ #define CBC_MAC_ROTATE_IN_PLACE -int ssl3_cbc_copy_mac(unsigned char *out, - const SSL3_RECORD *rec, size_t md_size) +static int ssl3_cbc_copy_mac(const SSL *s, + SSL3_RECORD *rec, + unsigned char **mac, + int *alloced, + size_t block_size, + size_t mac_size, + size_t good) { #if defined(CBC_MAC_ROTATE_IN_PLACE) unsigned char rotated_mac_buf[64 + EVP_MAX_MD_SIZE]; @@ -1585,12 +1558,14 @@ int ssl3_cbc_copy_mac(unsigned char *out, #else unsigned char rotated_mac[EVP_MAX_MD_SIZE]; #endif + unsigned char randmac[EVP_MAX_MD_SIZE]; + unsigned char *out; /* * mac_end is the index of |rec->data| just after the end of the MAC. */ size_t mac_end = rec->length; - size_t mac_start = mac_end - md_size; + size_t mac_start = mac_end - mac_size; size_t in_mac; /* * scan_start contains the number of bytes that we can ignore because the @@ -1600,21 +1575,51 @@ int ssl3_cbc_copy_mac(unsigned char *out, size_t i, j; size_t rotate_offset; - if (!ossl_assert(rec->orig_len >= md_size - && md_size <= EVP_MAX_MD_SIZE)) + if (!ossl_assert(rec->orig_len >= mac_size + && mac_size <= EVP_MAX_MD_SIZE)) return 0; + /* If no MAC then nothing to be done */ + if (mac_size == 0) { + /* No MAC so we can do this in non-constant time */ + if (good == 0) + return 0; + return 1; + } + + rec->length -= mac_size; + + if (block_size == 1) { + /* There's no padding so the position of the MAC is fixed */ + if (mac != NULL) + *mac = &rec->data[rec->length]; + if (alloced != NULL) + *alloced = 0; + return 1; + } + + /* Create the random MAC we will emit if padding is bad */ + if (!RAND_bytes_ex(s->ctx->libctx, randmac, mac_size)) + return 0; + + if (!ossl_assert(mac != NULL && alloced != NULL)) + return 0; + *mac = out = OPENSSL_malloc(mac_size); + if (*mac == NULL) + return 0; + *alloced = 1; + #if defined(CBC_MAC_ROTATE_IN_PLACE) rotated_mac = rotated_mac_buf + ((0 - (size_t)rotated_mac_buf) & 63); #endif /* This information is public so it's safe to branch based on it. */ - if (rec->orig_len > md_size + 255 + 1) - scan_start = rec->orig_len - (md_size + 255 + 1); + if (rec->orig_len > mac_size + 255 + 1) + scan_start = rec->orig_len - (mac_size + 255 + 1); in_mac = 0; rotate_offset = 0; - memset(rotated_mac, 0, md_size); + memset(rotated_mac, 0, mac_size); for (i = scan_start, j = 0; i < rec->orig_len; i++) { size_t mac_started = constant_time_eq_s(i, mac_start); size_t mac_ended = constant_time_lt_s(i, mac_end); @@ -1624,27 +1629,35 @@ int ssl3_cbc_copy_mac(unsigned char *out, in_mac &= mac_ended; rotate_offset |= j & mac_started; rotated_mac[j++] |= b & in_mac; - j &= constant_time_lt_s(j, md_size); + j &= constant_time_lt_s(j, mac_size); } /* Now rotate the MAC */ #if defined(CBC_MAC_ROTATE_IN_PLACE) j = 0; - for (i = 0; i < md_size; i++) { + for (i = 0; i < mac_size; i++) { /* in case cache-line is 32 bytes, touch second line */ ((volatile unsigned char *)rotated_mac)[rotate_offset ^ 32]; - out[j++] = rotated_mac[rotate_offset++]; - rotate_offset &= constant_time_lt_s(rotate_offset, md_size); + + /* If the padding wasn't good we emit a random MAC */ + out[j++] = constant_time_select_8((unsigned char)(good & 0xff), + rotated_mac[rotate_offset++], + randmac[i]); + rotate_offset &= constant_time_lt_s(rotate_offset, mac_size); } #else - memset(out, 0, md_size); - rotate_offset = md_size - rotate_offset; - rotate_offset &= constant_time_lt_s(rotate_offset, md_size); - for (i = 0; i < md_size; i++) { - for (j = 0; j < md_size; j++) + memset(out, 0, mac_size); + rotate_offset = mac_size - rotate_offset; + rotate_offset &= constant_time_lt_s(rotate_offset, mac_size); + for (i = 0; i < mac_size; i++) { + for (j = 0; j < mac_size; j++) out[j] |= rotated_mac[i] & constant_time_eq_8_s(j, rotate_offset); rotate_offset++; - rotate_offset &= constant_time_lt_s(rotate_offset, md_size); + rotate_offset &= constant_time_lt_s(rotate_offset, mac_size); + + /* If the padding wasn't good we emit a random MAC */ + out[i] = constant_time_select_8((unsigned char)(good & 0xff), out[i], + randmac[i]); } #endif @@ -1658,9 +1671,11 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap) SSL_SESSION *sess; SSL3_RECORD *rr; int imac_size; - size_t mac_size; + size_t mac_size = 0; unsigned char md[EVP_MAX_MD_SIZE]; size_t max_plain_length = SSL3_RT_MAX_PLAIN_LENGTH; + SSL_MAC_BUF macbuf = { NULL, 0 }; + int ret = 0; rr = RECORD_LAYER_get_rrec(&s->rlayer); sess = s->session; @@ -1694,14 +1709,24 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap) rr->data = rr->input; rr->orig_len = rr->length; + /* TODO(size_t): convert this to do size_t properly */ + if (s->read_hash != NULL) { + const EVP_MD *tmpmd = EVP_MD_CTX_md(s->read_hash); + + if (tmpmd != NULL) { + imac_size = EVP_MD_size(tmpmd); + if (!ossl_assert(imac_size >= 0 && imac_size <= EVP_MAX_MD_SIZE)) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD, + ERR_LIB_EVP); + return -1; + } + mac_size = (size_t)imac_size; + } + } + if (SSL_READ_ETM(s) && s->read_hash) { unsigned char *mac; - mac_size = EVP_MD_CTX_size(s->read_hash); - if (!ossl_assert(mac_size <= EVP_MAX_MD_SIZE)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DTLS1_PROCESS_RECORD, - ERR_R_INTERNAL_ERROR); - return 0; - } + if (rr->orig_len < mac_size) { SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_DTLS1_PROCESS_RECORD, SSL_R_LENGTH_TOO_SHORT); @@ -1715,24 +1740,30 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap) SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); return 0; } + /* + * We've handled the mac now - there is no MAC inside the encrypted + * record + */ + mac_size = 0; } - enc_err = s->method->ssl3_enc->enc(s, rr, 1, 0); + enc_err = s->method->ssl3_enc->enc(s, rr, 1, 0, &macbuf, mac_size); + /*- * enc_err is: - * 0: (in non-constant time) if the record is publicly invalid. - * 1: if the padding is valid - * -1: if the padding is invalid + * 0: if the record is publicly invalid, or an internal error, or AEAD + * decryption failed, or ETM decryption failed. + * 1: Success or MTE decryption failed (MAC will be randomised) */ if (enc_err == 0) { if (ossl_statem_in_error(s)) { /* SSLfatal() got called */ - return 0; + goto end; } /* For DTLS we simply ignore bad packets. */ rr->length = 0; RECORD_LAYER_reset_packet_length(&s->rlayer); - return 0; + goto end; } OSSL_TRACE_BEGIN(TLS) { BIO_printf(trc_out, "dec %zd\n", rr->length); @@ -1743,75 +1774,20 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap) if ((sess != NULL) && !SSL_READ_ETM(s) && (s->enc_read_ctx != NULL) && (EVP_MD_CTX_md(s->read_hash) != NULL)) { /* s->read_hash != NULL => mac_size != -1 */ - unsigned char *mac = NULL; - unsigned char mac_tmp[EVP_MAX_MD_SIZE]; - - /* TODO(size_t): Convert this to do size_t properly */ - imac_size = EVP_MD_CTX_size(s->read_hash); - if (imac_size < 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DTLS1_PROCESS_RECORD, - ERR_LIB_EVP); - return 0; - } - mac_size = (size_t)imac_size; - if (!ossl_assert(mac_size <= EVP_MAX_MD_SIZE)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DTLS1_PROCESS_RECORD, - ERR_R_INTERNAL_ERROR); - return 0; - } - - /* - * orig_len is the length of the record before any padding was - * removed. This is public information, as is the MAC in use, - * therefore we can safely process the record in a different amount - * of time if it's too short to possibly contain a MAC. - */ - if (rr->orig_len < mac_size || - /* CBC records must have a padding length byte too. */ - (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE && - rr->orig_len < mac_size + 1)) { - SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_DTLS1_PROCESS_RECORD, - SSL_R_LENGTH_TOO_SHORT); - return 0; - } - - if (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE) { - /* - * We update the length so that the TLS header bytes can be - * constructed correctly but we need to extract the MAC in - * constant time from within the record, without leaking the - * contents of the padding bytes. - */ - mac = mac_tmp; - if (!ssl3_cbc_copy_mac(mac_tmp, rr, mac_size)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DTLS1_PROCESS_RECORD, - ERR_R_INTERNAL_ERROR); - return 0; - } - rr->length -= mac_size; - } else { - /* - * In this case there's no padding, so |rec->orig_len| equals - * |rec->length| and we checked that there's enough bytes for - * |mac_size| above. - */ - rr->length -= mac_size; - mac = &rr->data[rr->length]; - } i = s->method->ssl3_enc->mac(s, rr, md, 0 /* not send */ ); - if (i == 0 || mac == NULL - || CRYPTO_memcmp(md, mac, mac_size) != 0) - enc_err = -1; + if (i == 0 || macbuf.mac == NULL + || CRYPTO_memcmp(md, macbuf.mac, mac_size) != 0) + enc_err = 0; if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH + mac_size) - enc_err = -1; + enc_err = 0; } - if (enc_err < 0) { + if (enc_err == 0) { /* decryption failed, silently discard message */ rr->length = 0; RECORD_LAYER_reset_packet_length(&s->rlayer); - return 0; + goto end; } /* r->length is now just compressed */ @@ -1819,12 +1795,12 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap) if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH) { SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_DTLS1_PROCESS_RECORD, SSL_R_COMPRESSED_LENGTH_TOO_LONG); - return 0; + goto end; } if (!ssl3_do_uncompress(s, rr)) { SSLfatal(s, SSL_AD_DECOMPRESSION_FAILURE, SSL_F_DTLS1_PROCESS_RECORD, SSL_R_BAD_DECOMPRESSION); - return 0; + goto end; } } @@ -1836,7 +1812,7 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap) if (rr->length > max_plain_length) { SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_DTLS1_PROCESS_RECORD, SSL_R_DATA_LENGTH_TOO_LONG); - return 0; + goto end; } rr->off = 0; @@ -1855,7 +1831,11 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap) /* Mark receipt of record. */ dtls1_record_bitmap_update(s, bitmap); - return 1; + ret = 1; + end: + if (macbuf.alloced) + OPENSSL_free(macbuf.mac); + return ret; } /* diff --git a/ssl/record/ssl3_record_tls13.c b/ssl/record/ssl3_record_tls13.c index f18da2db74..910b6a5862 100644 --- a/ssl/record/ssl3_record_tls13.c +++ b/ssl/record/ssl3_record_tls13.c @@ -12,17 +12,16 @@ #include "internal/cryptlib.h" /*- - * tls13_enc encrypts/decrypts |n_recs| in |recs|. Will call SSLfatal() for - * internal errors, but not otherwise. + * tls13_enc encrypts/decrypts |n_recs| in |recs|. Calls SSLfatal on internal + * error, but not otherwise. It is the responsibility of the caller to report + * a bad_record_mac. * * Returns: - * 0: (in non-constant time) if the record is publicly invalid (i.e. too - * short etc). - * 1: if the record encryption was successful. - * -1: if the record's AEAD-authenticator is invalid or, if sending, - * an internal error occurred. + * 0: On failure + * 1: if the record encryption/decryption was successful. */ -int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) +int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending, + SSL_MAC_BUF *mac, size_t macsize) { EVP_CIPHER_CTX *ctx; unsigned char iv[EVP_MAX_IV_LENGTH], recheader[SSL3_RT_HEADER_LENGTH]; @@ -39,7 +38,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) /* TODO(TLS1.3): Support pipelining */ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } if (sending) { @@ -75,7 +74,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) && s->psksession->ext.max_early_data > 0)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } alg_enc = s->psksession->cipher->algorithm_enc; } @@ -87,7 +86,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) if (!ossl_assert(s->s3.tmp.new_cipher != NULL)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } alg_enc = s->s3.tmp.new_cipher->algorithm_enc; } @@ -101,7 +100,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) NULL) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } } else if (alg_enc & SSL_AESGCM) { taglen = EVP_GCM_TLS_TAG_LEN; @@ -110,7 +109,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) } else { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } if (!sending) { @@ -128,7 +127,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) /* Should not happen */ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } offset = ivlen - SEQ_NUM_SIZE; memcpy(iv, staticiv, offset); @@ -143,7 +142,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) } if (loop == 0) { /* Sequence has wrapped */ - return -1; + return 0; } /* TODO(size_t): lenu/lenf should be a size_t but EVP doesn't support it */ @@ -151,7 +150,9 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) || (!sending && EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG, taglen, rec->data + rec->length) <= 0)) { - return -1; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC, + ERR_R_INTERNAL_ERROR); + return 0; } /* Set up the AAD */ @@ -162,8 +163,10 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) || !WPACKET_get_total_written(&wpkt, &hdrlen) || hdrlen != SSL3_RT_HEADER_LENGTH || !WPACKET_finish(&wpkt)) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC, + ERR_R_INTERNAL_ERROR); WPACKET_cleanup(&wpkt); - return -1; + return 0; } /* @@ -179,7 +182,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) (unsigned int)rec->length) <= 0 || EVP_CipherFinal_ex(ctx, rec->data + lenu, &lenf) <= 0 || (size_t)(lenu + lenf) != rec->length) { - return -1; + return 0; } if (sending) { /* Add the tag */ @@ -187,7 +190,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending) rec->data + rec->length) <= 0) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } rec->length += taglen; } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index a252761ca4..c3174a7c91 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -34,51 +34,37 @@ DEFINE_STACK_OF(OCSP_RESPID) DEFINE_STACK_OF(SRTP_PROTECTION_PROFILE) DEFINE_STACK_OF(SCT) -static int ssl_undefined_function_1(SSL *ssl, SSL3_RECORD *r, size_t s, int t) +static int ssl_undefined_function_1(SSL *ssl, SSL3_RECORD *r, size_t s, int t, + SSL_MAC_BUF *mac, size_t macsize) { - (void)r; - (void)s; - (void)t; return ssl_undefined_function(ssl); } static int ssl_undefined_function_2(SSL *ssl, SSL3_RECORD *r, unsigned char *s, int t) { - (void)r; - (void)s; - (void)t; return ssl_undefined_function(ssl); } static int ssl_undefined_function_3(SSL *ssl, unsigned char *r, unsigned char *s, size_t t, size_t *u) { - (void)r; - (void)s; - (void)t; - (void)u; return ssl_undefined_function(ssl); } static int ssl_undefined_function_4(SSL *ssl, int r) { - (void)r; return ssl_undefined_function(ssl); } static size_t ssl_undefined_function_5(SSL *ssl, const char *r, size_t s, unsigned char *t) { - (void)r; - (void)s; - (void)t; return ssl_undefined_function(ssl); } static int ssl_undefined_function_6(int r) { - (void)r; return ssl_undefined_function(NULL); } @@ -86,13 +72,6 @@ static int ssl_undefined_function_7(SSL *ssl, unsigned char *r, size_t s, const char *t, size_t u, const unsigned char *v, size_t w, int x) { - (void)r; - (void)s; - (void)t; - (void)u; - (void)v; - (void)w; - (void)x; return ssl_undefined_function(ssl); } diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h index 58bc1f99c4..17b856fabc 100644 --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -2069,7 +2069,7 @@ typedef struct cert_st { * of a mess of functions, but hell, think of it as an opaque structure :-) */ typedef struct ssl3_enc_method { - int (*enc) (SSL *, SSL3_RECORD *, size_t, int); + int (*enc) (SSL *, SSL3_RECORD *, size_t, int, SSL_MAC_BUF *, size_t); int (*mac) (SSL *, SSL3_RECORD *, unsigned char *, int); int (*setup_key_block) (SSL *); int (*generate_master_secret) (SSL *, unsigned char *, unsigned char *, diff --git a/test/tls13encryptiontest.c b/test/tls13encryptiontest.c index a49fbc0013..3bb189f0b5 100644 --- a/test/tls13encryptiontest.c +++ b/test/tls13encryptiontest.c @@ -368,7 +368,7 @@ static int test_tls13_encryption(void) } /* Encrypt it */ - if (!TEST_size_t_eq(tls13_enc(s, &rec, 1, 1), 1)) { + if (!TEST_size_t_eq(tls13_enc(s, &rec, 1, 1, NULL, 0), 1)) { TEST_info("Failed to encrypt record %zu", ctr); goto err; } @@ -378,7 +378,7 @@ static int test_tls13_encryption(void) } /* Decrypt it */ - if (!TEST_int_eq(tls13_enc(s, &rec, 1, 0), 1)) { + if (!TEST_int_eq(tls13_enc(s, &rec, 1, 0, NULL, 0), 1)) { TEST_info("Failed to decrypt record %zu", ctr); goto err; }