From bdaf1583af1de60bbad2f5dfc8aa17126cb23563 Mon Sep 17 00:00:00 2001 From: jsing Date: Sat, 19 Jun 2021 17:21:39 +0000 Subject: [PATCH] Correctly handle epoch wrapping in dtls1_get_bitmap(). Due to a type bug that has been present in DTLS since the code was first committed in 2005, dtls1_get_bitmap() fails to handle next epoch correctly when the epoch is currently 0xffff (and wraps to zero). For various reasons unknown, the epoch field in the SSL3_RECORD_INTERNAL (formerly SSL3_RECORD) was added as unsigned long (even though the value is an unsigned 16 bit value on the wire, hence cannot exceed 0xffff), however was added to other code as unsigned short. Due to integer promotion, the r_epoch value is incremented by one to become 0x10000, before being cast to an unsigned long and compared to the value pulled from the DTLS record header (which is zero). Strangely 0x10000 != 0, meaning that we drop the DTLS record, instead of queueing it for the next epoch. Fix this issue by using more appropriate types and pulling up the calculation of the next epoch value for improved readability. ok inoguchi@ tb@ --- lib/libssl/d1_pkt.c | 5 +++-- lib/libssl/ssl_locl.h | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/libssl/d1_pkt.c b/lib/libssl/d1_pkt.c index 11735f0d2c7..17be6dd182d 100644 --- a/lib/libssl/d1_pkt.c +++ b/lib/libssl/d1_pkt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: d1_pkt.c,v 1.98 2021/06/15 19:09:03 jsing Exp $ */ +/* $OpenBSD: d1_pkt.c,v 1.99 2021/06/19 17:21:39 jsing Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -1191,6 +1191,7 @@ dtls1_dispatch_alert(SSL *s) static DTLS1_BITMAP * dtls1_get_bitmap(SSL *s, SSL3_RECORD_INTERNAL *rr, unsigned int *is_next_epoch) { + uint16_t next_epoch = D1I(s)->r_epoch + 1; *is_next_epoch = 0; @@ -1199,7 +1200,7 @@ dtls1_get_bitmap(SSL *s, SSL3_RECORD_INTERNAL *rr, unsigned int *is_next_epoch) return &D1I(s)->bitmap; /* Only HM and ALERT messages can be from the next epoch */ - else if (rr->epoch == (unsigned long)(D1I(s)->r_epoch + 1) && + else if (rr->epoch == next_epoch && (rr->type == SSL3_RT_HANDSHAKE || rr->type == SSL3_RT_ALERT)) { *is_next_epoch = 1; return &D1I(s)->next_bitmap; diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h index 18509438aed..d171ef0984a 100644 --- a/lib/libssl/ssl_locl.h +++ b/lib/libssl/ssl_locl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_locl.h,v 1.349 2021/06/19 16:52:47 jsing Exp $ */ +/* $OpenBSD: ssl_locl.h,v 1.350 2021/06/19 17:21:40 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -886,7 +886,7 @@ typedef struct ssl3_record_internal_st { unsigned int off; /* read/write offset into 'buf' */ unsigned char *data; /* pointer to the record data */ unsigned char *input; /* where the decode bytes are */ - unsigned long epoch; /* epoch number, needed by DTLS1 */ + uint16_t epoch; /* epoch number, needed by DTLS1 */ unsigned char seq_num[8]; /* sequence number, needed by DTLS1 */ } SSL3_RECORD_INTERNAL; -- 2.20.1