Clean up/simplify more of the dtls1/ssl3 record writing code:

- Make the DTLS code much more consistent with the ssl3 code.

- Avoid assigning wr->input and wr->length just so they can be used as
  arguments to memcpy().

- Remove the arc4random_buf() call for the explicit IV, since tls1_enc()
  already does this for us.

ok tb@

2 files changed, 34 insertions, 73 deletions

diff --git a/src/lib/libssl/d1_pkt.c b/src/lib/libssl/d1_pkt.c
index d6b1506119..f888592223 100644
--- a/src/lib/libssl/d1_pkt.c
+++ b/src/lib/libssl/d1_pkt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: d1_pkt.c,v 1.74 2020/07/30 16:53:01 jsing Exp $ */
+/* $OpenBSD: d1_pkt.c,v 1.75 2020/08/01 16:50:16 jsing Exp $ */
 /*
  * DTLS implementation written by Nagendra Modadugu
  * (nagendra@cs.stanford.edu) for the OpenSSL project 2005.
@@ -1174,12 +1174,12 @@ dtls1_write_bytes(SSL *s, int type, const void *buf, int len)
 int
 do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len)
 {
+        SSL3_RECORD_INTERNAL *wr = &(S3I(s)->wrec);
+        SSL3_BUFFER_INTERNAL *wb = &(S3I(s)->wbuf);
+        SSL_SESSION *sess = s->session;
+        int eivlen = 0, mac_size = 0;
         unsigned char *p;
-        SSL3_RECORD_INTERNAL *wr;
-        SSL3_BUFFER_INTERNAL *wb;
-        SSL_SESSION *sess;
-        int mac_size = 0;
-        int bs, ret;
+        int ret;
         CBB cbb;
 
         memset(&cbb, 0, sizeof(cbb));
@@ -1188,7 +1188,7 @@ do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len)
          * First check if there is a SSL3_BUFFER_INTERNAL still being written
          * out.  This will happen with non blocking IO.
          */
-        if (S3I(s)->wbuf.left != 0) {
+        if (wb->left != 0) {
                 OPENSSL_assert(0); /* XDTLS:  want to see if we ever get here */
                 return (ssl3_write_pending(s, type, buf, len));
         }
@@ -1203,10 +1203,6 @@ do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len)
         if (len == 0)
                 return 0;
 
-        wr = &(S3I(s)->wrec);
-        wb = &(S3I(s)->wbuf);
-        sess = s->session;
-
         if (sess != NULL && s->internal->enc_write_ctx != NULL &&
             EVP_MD_CTX_md(s->internal->write_hash) != NULL) {
                 if ((mac_size = EVP_MD_CTX_size(s->internal->write_hash)) < 0)
@@ -1216,6 +1212,7 @@ do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len)
         /* DTLS implements explicit IV, so no need for empty fragments. */
 
         p = wb->buf;
+        wb->offset = 0;
 
         if (!CBB_init_fixed(&cbb, p, DTLS1_RT_HEADER_LENGTH))
                 goto err;
@@ -1232,50 +1229,30 @@ do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len)
 
         p += DTLS1_RT_HEADER_LENGTH;
 
-        /* lets setup the record stuff. */
-
-        /* Make space for the explicit IV in case of CBC.
+        /*
+         * Make space for the explicit IV in case of CBC.
          * (this is a bit of a boundary violation, but what the heck).
          */
         if (s->internal->enc_write_ctx &&
             (EVP_CIPHER_mode(s->internal->enc_write_ctx->cipher) & EVP_CIPH_CBC_MODE))
-                bs = EVP_CIPHER_block_size(s->internal->enc_write_ctx->cipher);
-        else
-                bs = 0;
+                eivlen = EVP_CIPHER_block_size(s->internal->enc_write_ctx->cipher);
 
         wr->type = type;
-        wr->data = p + bs;
-        /* make room for IV in case of CBC */
+        wr->data = p + eivlen;
         wr->length = (int)len;
-        wr->input = (unsigned char *)buf;
-
-        /* we now 'read' from wr->input, wr->length bytes into
-         * wr->data */
-
-        memcpy(wr->data, wr->input, wr->length);
         wr->input = wr->data;
 
-        /* we should still have the output to wr->data and the input
-         * from wr->input.  Length should be wr->length.
-         * wr->data still points in the wb->buf */
+        memcpy(wr->data, buf, len);
 
         if (mac_size != 0) {
-                if (tls1_mac(s, &(p[wr->length + bs]), 1) < 0)
+                if (tls1_mac(s, &(p[wr->length + eivlen]), 1) < 0)
                         goto err;
                 wr->length += mac_size;
         }
 
-        /* this is true regardless of mac size */
-        wr->input = p;
         wr->data = p;
-
-        /* bs != 0 in case of CBC */
-        if (bs) {
-                arc4random_buf(p, bs);
-                /* master IV and last CBC residue stand for
-                 * the rest of randomness */
-                wr->length += bs;
-        }
+        wr->input = p;
+        wr->length += eivlen;
 
         /* tls1_enc can only have an error on read */
         tls1_enc(s, 1);
@@ -1285,25 +1262,27 @@ do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len)
         if (!CBB_finish(&cbb, NULL, NULL))
                 goto err;
 
-        /* we should now have
-         * wr->data pointing to the encrypted data, which is
-         * wr->length long */
+        /*
+         * We should now have wr->data pointing to the encrypted data,
+         * which is wr->length long.
+         */
         wr->type = type; /* not needed but helps for debugging */
         wr->length += DTLS1_RT_HEADER_LENGTH;
 
         tls1_record_sequence_increment(S3I(s)->write_sequence);
 
-        /* now let's set up wb */
         wb->left = wr->length;
-        wb->offset = 0;
 
-        /* memorize arguments so that ssl3_write_pending can detect bad write retries later */
+        /*
+         * Memorize arguments so that ssl3_write_pending can detect
+         * bad write retries later.
+         */
         S3I(s)->wpend_tot = len;
         S3I(s)->wpend_buf = buf;
         S3I(s)->wpend_type = type;
         S3I(s)->wpend_ret = len;
 
-        /* we now just need to write the buffer */
+        /* We now just need to write the buffer. */
         return ssl3_write_pending(s, type, buf, len);
 
  err:
@@ -1312,8 +1291,6 @@ do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len)
         return -1;
 }
 
-
-
 static int
 dtls1_record_replay_check(SSL *s, DTLS1_BITMAP *bitmap)
 {
diff --git a/src/lib/libssl/ssl_pkt.c b/src/lib/libssl/ssl_pkt.c
index 5d12b40f28..6bb722098a 100644
--- a/src/lib/libssl/ssl_pkt.c
+++ b/src/lib/libssl/ssl_pkt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_pkt.c,v 1.26 2020/08/01 16:38:17 jsing Exp $ */
+/* $OpenBSD: ssl_pkt.c,v 1.27 2020/08/01 16:50:16 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -622,7 +622,7 @@ ssl3_create_record(SSL *s, unsigned char *p, uint16_t version, uint8_t type,
 {
         SSL3_RECORD_INTERNAL *wr = &(S3I(s)->wrec);
         SSL_SESSION *sess = s->session;
-        int eivlen, mac_size = 0;
+        int eivlen = 0, mac_size = 0;
         CBB cbb;
 
         memset(&cbb, 0, sizeof(cbb));
@@ -645,7 +645,6 @@ ssl3_create_record(SSL *s, unsigned char *p, uint16_t version, uint8_t type,
         p += SSL3_RT_HEADER_LENGTH;
 
         /* Explicit IV length. */
-        eivlen = 0;
         if (s->internal->enc_write_ctx && SSL_USE_EXPLICIT_IV(s)) {
                 int mode = EVP_CIPHER_CTX_mode(s->internal->enc_write_ctx);
                 if (mode == EVP_CIPH_CBC_MODE) {
@@ -658,22 +657,12 @@ ssl3_create_record(SSL *s, unsigned char *p, uint16_t version, uint8_t type,
                 eivlen = s->internal->aead_write_ctx->variable_nonce_len;
         }
 
-        /* lets setup the record stuff. */
         wr->type = type;
         wr->data = p + eivlen;
         wr->length = (int)len;
-        wr->input = (unsigned char *)buf;
-
-        /* we now 'read' from wr->input, wr->length bytes into wr->data */
-
-        memcpy(wr->data, wr->input, wr->length);
         wr->input = wr->data;
 
-        /*
-         * We should still have the output to wr->data and the input
-         * from wr->input.  Length should be wr->length.
-         * wr->data still points in the wb->buf.
-         */
+        memcpy(wr->data, buf, len);
 
         if (mac_size != 0) {
                 if (tls1_mac(s, &(p[wr->length + eivlen]), 1) < 0)
@@ -681,15 +670,9 @@ ssl3_create_record(SSL *s, unsigned char *p, uint16_t version, uint8_t type,
                 wr->length += mac_size;
         }
 
-        wr->input = p;
         wr->data = p;
-
-        if (eivlen) {
-                /* if (RAND_pseudo_bytes(p, eivlen) <= 0)
-                        goto err;
-                */
-                wr->length += eivlen;
-        }
+        wr->input = p;
+        wr->length += eivlen;
 
         /* tls1_enc can only have an error on read */
         tls1_enc(s, 1);
@@ -700,9 +683,10 @@ ssl3_create_record(SSL *s, unsigned char *p, uint16_t version, uint8_t type,
         if (!CBB_finish(&cbb, NULL, NULL))
                 goto err;
 
-        /* we should now have
-         * wr->data pointing to the encrypted data, which is
-         * wr->length long */
+        /*
+         * We should now have wr->data pointing to the encrypted data,
+         * which is wr->length long.
+         */
         wr->type = type; /* not needed but helps for debugging */
         wr->length += SSL3_RT_HEADER_LENGTH;