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/lib/libssl/d1_pkt.c b/lib/libssl/d1_pkt.c
index d6b15061196..f888592223c 100644
--- a/lib/libssl/d1_pkt.c
+++ b/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/lib/libssl/ssl_pkt.c b/lib/libssl/ssl_pkt.c
index 5d12b40f28e..6bb722098a1 100644
--- a/lib/libssl/ssl_pkt.c
+++ b/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;