Convert TLS record construction to use WPACKET

Reviewed-by: NRich Salz <rsalz@openssl.org>

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;
-        }
+        } else if (mode == EVP_CIPH_GCM_MODE) {
             /* Need explicit part of IV for GCM mode */
-        else if (mode == EVP_CIPH_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);
+            }
         }
 
-        SSL3_RECORD_set_data(&wr[j], outbuf[j]);
-        SSL3_RECORD_reset_input(&wr[j]);
-
-        if (eivlen) {
         /*
-             * if (RAND_pseudo_bytes(p, eivlen) <= 0) goto err;
+         * 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
                     */
-            SSL3_RECORD_add_length(&wr[j], eivlen);
+                || !WPACKET_get_length(&pkt[j], &len)) {
+            SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR);
+            goto err;
         }
+
+        /* 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 (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;
 }
 

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);