From e3ae3cd6c9c09e2489c5d8b6149d643e1ead7c07 Mon Sep 17 00:00:00 2001 From: jsing Date: Mon, 29 Mar 2021 16:19:15 +0000 Subject: [PATCH] Move the TLSv1.2 record number increment into the new record layer. This adds checks (based on the TLSv1.3 implementation) to ensure that the TLS/DTLS sequence numbers do not wrap, as required by the respective RFCs. ok inoguchi@ tb@ --- lib/libssl/ssl_locl.h | 3 +-- lib/libssl/t1_enc.c | 13 +-------- lib/libssl/tls12_record_layer.c | 47 +++++++++++++++++++++++++++++---- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h index 7f197bbcdff..4b2f98f84d2 100644 --- a/lib/libssl/ssl_locl.h +++ b/lib/libssl/ssl_locl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_locl.h,v 1.331 2021/03/27 17:56:28 tb Exp $ */ +/* $OpenBSD: ssl_locl.h,v 1.332 2021/03/29 16:19:15 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1261,7 +1261,6 @@ int ssl3_handshake_msg_finish(SSL *s, CBB *handshake); int ssl3_handshake_write(SSL *s); int ssl3_record_write(SSL *s, int type); -void tls1_record_sequence_increment(unsigned char *seq); int ssl3_do_change_cipher_spec(SSL *ssl); int dtls1_do_write(SSL *s, int type); diff --git a/lib/libssl/t1_enc.c b/lib/libssl/t1_enc.c index b9dcbac6614..0ddd52b5301 100644 --- a/lib/libssl/t1_enc.c +++ b/lib/libssl/t1_enc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: t1_enc.c,v 1.135 2021/03/24 18:44:00 jsing Exp $ */ +/* $OpenBSD: t1_enc.c,v 1.136 2021/03/29 16:19:15 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -157,17 +157,6 @@ tls1_cleanup_key_block(SSL *s) S3I(s)->hs.tls12.key_block_len = 0; } -void -tls1_record_sequence_increment(unsigned char *seq) -{ - int i; - - for (i = SSL3_SEQUENCE_SIZE - 1; i >= 0; i--) { - if (++seq[i] != 0) - break; - } -} - /* * TLS P_hash() data expansion function - see RFC 5246, section 5. */ diff --git a/lib/libssl/tls12_record_layer.c b/lib/libssl/tls12_record_layer.c index ba3c3dfb2bd..6cf8b31c63e 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.24 2021/03/21 19:08:22 tb Exp $ */ +/* $OpenBSD: tls12_record_layer.c,v 1.25 2021/03/29 16:19:15 jsing Exp $ */ /* * Copyright (c) 2020 Joel Sing * @@ -22,9 +22,11 @@ #include "ssl_locl.h" +#define TLS12_RECORD_SEQ_NUM_LEN 8 + struct tls12_record_protection { uint16_t epoch; - uint8_t seq_num[SSL3_SEQUENCE_SIZE]; + uint8_t seq_num[TLS12_RECORD_SEQ_NUM_LEN]; SSL_AEAD_CTX *aead_ctx; @@ -342,6 +344,38 @@ tls12_record_layer_reflect_seq_num(struct tls12_record_layer *rl) sizeof(rl->write->seq_num)); } +static const uint8_t tls12_max_seq_num[TLS12_RECORD_SEQ_NUM_LEN] = { + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, +}; + +int +tls12_record_layer_inc_seq_num(struct tls12_record_layer *rl, uint8_t *seq_num) +{ + CBS max_seq_num; + int i; + + /* + * RFC 5246 section 6.1 and RFC 6347 section 4.1 - both TLS and DTLS + * sequence numbers must not wrap. Note that for DTLS the first two + * bytes are used as an "epoch" and not part of the sequence number. + */ + CBS_init(&max_seq_num, seq_num, TLS12_RECORD_SEQ_NUM_LEN); + if (rl->dtls) { + if (!CBS_skip(&max_seq_num, 2)) + return 0; + } + if (CBS_mem_equal(&max_seq_num, tls12_max_seq_num, + CBS_len(&max_seq_num))) + return 0; + + for (i = TLS12_RECORD_SEQ_NUM_LEN - 1; i >= 0; i--) { + if (++seq_num[i] != 0) + break; + } + + return 1; +} + static int tls12_record_layer_set_mac_key(struct tls12_record_protection *rp, const uint8_t *mac_key, size_t mac_key_len) @@ -1074,8 +1108,10 @@ tls12_record_layer_open_record(struct tls12_record_layer *rl, uint8_t *buf, return 0; } - if (!rl->dtls) - tls1_record_sequence_increment(rl->read->seq_num); + if (!rl->dtls) { + if (!tls12_record_layer_inc_seq_num(rl, rl->read->seq_num)) + return 0; + } return 1; } @@ -1274,7 +1310,8 @@ tls12_record_layer_seal_record(struct tls12_record_layer *rl, if (!CBB_flush(cbb)) goto err; - tls1_record_sequence_increment(rl->write->seq_num); + if (!tls12_record_layer_inc_seq_num(rl, rl->write->seq_num)) + goto err; ret = 1; -- 2.20.1