Simplify nonce handling in the TLSv1.2 record layer.

Pass the CBS for the sequence number through, which also allows us to do
more sensible length checks. Also, add a missing length check while here.

ok inoguchi@ tb@

1 files changed, 16 insertions, 13 deletions

diff --git a/lib/libssl/tls12_record_layer.c b/lib/libssl/tls12_record_layer.c
index 06d1af9def8..481680d9cc9 100644
--- a/lib/libssl/tls12_record_layer.c
+++ b/lib/libssl/tls12_record_layer.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls12_record_layer.c,v 1.30 2021/05/16 15:49:01 jsing Exp $ */
+/* $OpenBSD: tls12_record_layer.c,v 1.31 2021/06/14 14:22:52 jsing Exp $ */
 /*
  * Copyright (c) 2020 Joel Sing <jsing@openbsd.org>
  *
@@ -780,12 +780,12 @@ tls12_record_layer_write_mac(struct tls12_record_layer *rl, CBB *cbb,
 
 static int
 tls12_record_layer_aead_concat_nonce(struct tls12_record_layer *rl,
-    struct tls12_record_protection *rp, const uint8_t *seq_num,
+    struct tls12_record_protection *rp, CBS *seq_num,
     uint8_t **out, size_t *out_len)
 {
 	CBB cbb;
 
-	if (rp->aead_variable_nonce_len > SSL3_SEQUENCE_SIZE)
+	if (rp->aead_variable_nonce_len > CBS_len(seq_num))
 		return 0;
 
 	/* Fixed nonce and variable nonce (sequence number) are concatenated. */
@@ -794,7 +794,8 @@ tls12_record_layer_aead_concat_nonce(struct tls12_record_layer *rl,
 	if (!CBB_add_bytes(&cbb, rp->aead_fixed_nonce,
 	    rp->aead_fixed_nonce_len))
 		goto err;
-	if (!CBB_add_bytes(&cbb, seq_num, rp->aead_variable_nonce_len))
+	if (!CBB_add_bytes(&cbb, CBS_data(seq_num),
+	    rp->aead_variable_nonce_len))
 		goto err;
 	if (!CBB_finish(&cbb, out, out_len))
 		goto err;
@@ -809,7 +810,7 @@ tls12_record_layer_aead_concat_nonce(struct tls12_record_layer *rl,
 
 static int
 tls12_record_layer_aead_xored_nonce(struct tls12_record_layer *rl,
-    struct tls12_record_protection *rp, const uint8_t *seq_num,
+    struct tls12_record_protection *rp, CBS *seq_num,
     uint8_t **out, size_t *out_len)
 {
 	uint8_t *nonce = NULL;
@@ -818,7 +819,7 @@ tls12_record_layer_aead_xored_nonce(struct tls12_record_layer *rl,
 	CBB cbb;
 	int i;
 
-	if (rp->aead_variable_nonce_len > SSL3_SEQUENCE_SIZE)
+	if (rp->aead_variable_nonce_len > CBS_len(seq_num))
 		return 0;
 	if (rp->aead_fixed_nonce_len < rp->aead_variable_nonce_len)
 		return 0;
@@ -832,7 +833,8 @@ tls12_record_layer_aead_xored_nonce(struct tls12_record_layer *rl,
 	if (!CBB_add_space(&cbb, &pad,
 	    rp->aead_fixed_nonce_len - rp->aead_variable_nonce_len))
 		goto err;
-	if (!CBB_add_bytes(&cbb, seq_num, rp->aead_variable_nonce_len))
+	if (!CBB_add_bytes(&cbb, CBS_data(seq_num),
+	    rp->aead_variable_nonce_len))
 		goto err;
 	if (!CBB_finish(&cbb, &nonce, &nonce_len))
 		goto err;
@@ -882,18 +884,18 @@ tls12_record_layer_open_record_protected_aead(struct tls12_record_layer *rl,
 	/* XXX - move to nonce allocated in record layer, matching TLSv1.3 */
 	if (rp->aead_xor_nonces) {
 		if (!tls12_record_layer_aead_xored_nonce(rl, rp,
-		    CBS_data(seq_num), &nonce, &nonce_len))
+		    seq_num, &nonce, &nonce_len))
 			goto err;
 	} else if (rp->aead_variable_nonce_in_record) {
 		if (!CBS_get_bytes(fragment, &var_nonce,
 		    rp->aead_variable_nonce_len))
 			goto err;
 		if (!tls12_record_layer_aead_concat_nonce(rl, rp,
-		    CBS_data(&var_nonce), &nonce, &nonce_len))
+		    &var_nonce, &nonce, &nonce_len))
 			goto err;
 	} else {
 		if (!tls12_record_layer_aead_concat_nonce(rl, rp,
-		    CBS_data(seq_num), &nonce, &nonce_len))
+		    seq_num, &nonce, &nonce_len))
 			goto err;
 	}
 
@@ -1145,16 +1147,17 @@ tls12_record_layer_seal_record_protected_aead(struct tls12_record_layer *rl,
 	/* XXX - move to nonce allocated in record layer, matching TLSv1.3 */
 	if (rp->aead_xor_nonces) {
 		if (!tls12_record_layer_aead_xored_nonce(rl, rp,
-		    CBS_data(seq_num), &nonce, &nonce_len))
+		    seq_num, &nonce, &nonce_len))
 			goto err;
 	} else {
 		if (!tls12_record_layer_aead_concat_nonce(rl, rp,
-		    CBS_data(seq_num), &nonce, &nonce_len))
+		    seq_num, &nonce, &nonce_len))
 			goto err;
 	}
 
 	if (rp->aead_variable_nonce_in_record) {
-		/* XXX - length check? */
+		if (rp->aead_variable_nonce_len > CBS_len(seq_num))
+			goto err;
 		if (!CBB_add_bytes(out, CBS_data(seq_num),
 		    rp->aead_variable_nonce_len))
 			goto err;