From c7c42022b9283d073d355b427ebb578f4ff15eb1 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 18 Nov 2016 16:35:46 +0000 Subject: [PATCH] Convert TLS record construction to use WPACKET Reviewed-by: Rich Salz --- ssl/record/rec_layer_s3.c | 163 ++++++++++++++++++++++++++++---------- ssl/record/ssl3_record.c | 2 +- 2 files changed, 122 insertions(+), 43 deletions(-) diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 317ee30aa9..925a835d9b 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -633,15 +633,16 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, size_t *pipelens, size_t numpipes, int create_empty_fragment, size_t *written) { - unsigned char *outbuf[SSL_MAX_PIPELINES], *plen[SSL_MAX_PIPELINES]; + WPACKET pkt[SSL_MAX_PIPELINES]; SSL3_RECORD wr[SSL_MAX_PIPELINES]; + unsigned char *recordstart; int i, mac_size, clear = 0; size_t prefix_len = 0; int eivlen; size_t align = 0; SSL3_BUFFER *wb; SSL_SESSION *sess; - size_t totlen = 0; + size_t totlen = 0, len, wpinited = 0; size_t j; for (j = 0; j < numpipes; j++) @@ -726,12 +727,24 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, align = (size_t)SSL3_BUFFER_get_buf(wb) + 2 * SSL3_RT_HEADER_LENGTH; align = SSL3_ALIGN_PAYLOAD - 1 - ((align - 1) % SSL3_ALIGN_PAYLOAD); #endif - outbuf[0] = SSL3_BUFFER_get_buf(wb) + align; SSL3_BUFFER_set_offset(wb, align); + if (!WPACKET_init_static_len(&pkt[0], SSL3_BUFFER_get_buf(wb), + SSL3_BUFFER_get_len(wb), 0) + || !WPACKET_allocate_bytes(&pkt[0], align, NULL)) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); + goto err; + } + wpinited = 1; } else if (prefix_len) { wb = &s->rlayer.wbuf[0]; - outbuf[0] = SSL3_BUFFER_get_buf(wb) + SSL3_BUFFER_get_offset(wb) - + prefix_len; + if (!WPACKET_init_static_len(&pkt[0], SSL3_BUFFER_get_buf(wb), + SSL3_BUFFER_get_len(wb), 0) + || !WPACKET_allocate_bytes(&pkt[0], SSL3_BUFFER_get_offset(wb) + + prefix_len, NULL)) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); + goto err; + } + wpinited = 1; } else { for (j = 0; j < numpipes; j++) { wb = &s->rlayer.wbuf[j]; @@ -739,8 +752,14 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, align = (size_t)SSL3_BUFFER_get_buf(wb) + SSL3_RT_HEADER_LENGTH; align = SSL3_ALIGN_PAYLOAD - 1 - ((align - 1) % SSL3_ALIGN_PAYLOAD); #endif - outbuf[j] = SSL3_BUFFER_get_buf(wb) + align; SSL3_BUFFER_set_offset(wb, align); + if (!WPACKET_init_static_len(&pkt[j], SSL3_BUFFER_get_buf(wb), + SSL3_BUFFER_get_len(wb), 0) + || !WPACKET_allocate_bytes(&pkt[j], align, NULL)) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); + goto err; + } + wpinited++; } } @@ -752,42 +771,52 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, eivlen = EVP_CIPHER_CTX_iv_length(s->enc_write_ctx); if (eivlen <= 1) eivlen = 0; - } - /* Need explicit part of IV for GCM mode */ - else if (mode == EVP_CIPH_GCM_MODE) + } else if (mode == EVP_CIPH_GCM_MODE) { + /* Need explicit part of IV for GCM mode */ eivlen = EVP_GCM_TLS_EXPLICIT_IV_LEN; - else if (mode == EVP_CIPH_CCM_MODE) + } else if (mode == EVP_CIPH_CCM_MODE) { eivlen = EVP_CCM_TLS_EXPLICIT_IV_LEN; - else + } else { eivlen = 0; - } else + } + } else { eivlen = 0; + } totlen = 0; /* Clear our SSL3_RECORD structures */ memset(wr, 0, sizeof wr); for (j = 0; j < numpipes; j++) { - /* write the header */ - *(outbuf[j]++) = type & 0xff; - SSL3_RECORD_set_type(&wr[j], type); + unsigned int version = s->version; + unsigned char *compressdata; + size_t maxcomplen; - *(outbuf[j]++) = (s->version >> 8); + SSL3_RECORD_set_type(&wr[j], type); /* * Some servers hang if initial client hello is larger than 256 bytes * and record version number > TLS 1.0 */ if (SSL_get_state(s) == TLS_ST_CW_CLNT_HELLO && !s->renegotiate && TLS1_get_version(s) > TLS1_VERSION) - *(outbuf[j]++) = 0x1; - else - *(outbuf[j]++) = s->version & 0xff; + version = TLS1_VERSION; - /* field where we are to write out packet length */ - plen[j] = outbuf[j]; - outbuf[j] += 2; + maxcomplen = pipelens[j] + (ssl_allow_compression(s) + ? SSL3_RT_MAX_COMPRESSED_OVERHEAD : 0); + /* write the header */ + if (!WPACKET_put_bytes_u8(&pkt[j], type) + || !WPACKET_put_bytes_u16(&pkt[j], version) + || !WPACKET_start_sub_packet_u16(&pkt[j]) + || (eivlen > 0 + && !WPACKET_allocate_bytes(&pkt[j], eivlen, NULL)) + || (maxcomplen > 0 + && !WPACKET_reserve_bytes(&pkt[j], maxcomplen, + &compressdata))) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); + goto err; + } /* lets setup the record stuff. */ - SSL3_RECORD_set_data(&wr[j], outbuf[j] + eivlen); + SSL3_RECORD_set_data(&wr[j], compressdata); SSL3_RECORD_set_length(&wr[j], pipelens[j]); SSL3_RECORD_set_input(&wr[j], (unsigned char *)&buf[totlen]); totlen += pipelens[j]; @@ -798,12 +827,16 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, /* first we compress */ if (s->compress != NULL) { - if (!ssl3_do_compress(s, &wr[j])) { + if (!ssl3_do_compress(s, &wr[j]) + || !WPACKET_allocate_bytes(&pkt[j], wr[j].length, NULL)) { SSLerr(SSL_F_DO_SSL3_WRITE, SSL_R_COMPRESSION_FAILURE); goto err; } } else { - memcpy(wr[j].data, wr[j].input, wr[j].length); + if (!WPACKET_memcpy(&pkt[j], wr[j].input, wr[j].length)) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); + goto err; + } SSL3_RECORD_reset_input(&wr[j]); } @@ -814,41 +847,85 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, */ if (!SSL_USE_ETM(s) && mac_size != 0) { - if (!s->method->ssl3_enc->mac(s, &wr[j], - &(outbuf[j][wr[j].length + eivlen]), - 1)) + unsigned char *mac; + + if (!WPACKET_allocate_bytes(&pkt[j], mac_size, &mac) + || !s->method->ssl3_enc->mac(s, &wr[j], mac, 1)) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); goto err; - SSL3_RECORD_add_length(&wr[j], mac_size); + } + } + + /* + * Reserve some bytes for any growth that may occur during encryption. + * This will be at most one cipher block or the tag length if using + * AEAD. SSL_RT_MAX_CIPHER_BLOCK_SIZE covers either case. + */ + if(!WPACKET_reserve_bytes(&pkt[j], SSL_RT_MAX_CIPHER_BLOCK_SIZE, + NULL) + /* + * We also need next the amount of bytes written to this + * sub-packet + */ + || !WPACKET_get_length(&pkt[j], &len)) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); + goto err; } - SSL3_RECORD_set_data(&wr[j], outbuf[j]); + /* Get a pointer to the start of this record excluding header */ + recordstart = WPACKET_get_curr(&pkt[j]) - len; + + SSL3_RECORD_set_data(&wr[j], recordstart); SSL3_RECORD_reset_input(&wr[j]); + SSL3_RECORD_set_length(&wr[j], len); - if (eivlen) { - /* - * if (RAND_pseudo_bytes(p, eivlen) <= 0) goto err; - */ - SSL3_RECORD_add_length(&wr[j], eivlen); - } } if (s->method->ssl3_enc->enc(s, wr, numpipes, 1) < 1) goto err; for (j = 0; j < numpipes; j++) { + size_t origlen; + + /* Allocate bytes for the encryption overhead */ + if (!WPACKET_get_length(&pkt[j], &origlen) + /* Encryption should never shrink the data! */ + || origlen > wr[j].length + || (wr[j].length > origlen + && !WPACKET_allocate_bytes(&pkt[j], + wr[j].length - origlen, NULL))) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); + goto err; + } if (SSL_USE_ETM(s) && mac_size != 0) { - if (!s->method->ssl3_enc->mac(s, &wr[j], - outbuf[j] + wr[j].length, 1)) + unsigned char *mac; + + if (!WPACKET_allocate_bytes(&pkt[j], mac_size, &mac) + || !s->method->ssl3_enc->mac(s, &wr[j], mac, 1)) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); goto err; + } SSL3_RECORD_add_length(&wr[j], mac_size); } - /* record length after mac and block padding */ - s2n(SSL3_RECORD_get_length(&wr[j]), plen[j]); + if (!WPACKET_get_length(&pkt[j], &len) + || !WPACKET_close(&pkt[j])) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); + goto err; + } - if (s->msg_callback) - s->msg_callback(1, 0, SSL3_RT_HEADER, plen[j] - 5, 5, s, + if (s->msg_callback) { + recordstart = WPACKET_get_curr(&pkt[j]) - len + - SSL3_RT_HEADER_LENGTH; + s->msg_callback(1, 0, SSL3_RT_HEADER, recordstart, + SSL3_RT_HEADER_LENGTH, s, s->msg_callback_arg); + } + + if (!WPACKET_finish(&pkt[j])) { + SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); + goto err; + } /* * we should now have wr->data pointing to the encrypted data, which is @@ -889,6 +966,8 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, /* we now just need to write the buffer */ return ssl3_write_pending(s, type, buf, totlen, written); err: + for (j = 0; j < wpinited; j++) + WPACKET_cleanup(&pkt[j]); return -1; } diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c index d106e3886e..b33dce23f0 100644 --- a/ssl/record/ssl3_record.c +++ b/ssl/record/ssl3_record.c @@ -553,7 +553,7 @@ int ssl3_do_compress(SSL *ssl, SSL3_RECORD *wr) /* TODO(size_t): Convert this call */ i = COMP_compress_block(ssl->compress, wr->data, - SSL3_RT_MAX_COMPRESSED_LENGTH, + (int)(wr->length + SSL3_RT_MAX_COMPRESSED_OVERHEAD), wr->input, (int)wr->length); if (i < 0) return (0); -- 2.25.1