Increase DTLS window size from 64 to 256

Bug: chromium:40925630
Change-Id: Ide72960600747f5ce9a9213a9103510fee3e3806
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67527
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

3 files changed, 16 insertions, 11 deletions

diff --git a/ssl/dtls_record.cc b/ssl/dtls_record.cc
index eb3df69..068864f 100644
--- a/ssl/dtls_record.cc
+++ b/ssl/dtls_record.cc
@@ -127,26 +127,26 @@ BSSL_NAMESPACE_BEGIN
 // |bitmap| or is stale. Otherwise it returns zero.
 static bool dtls1_bitmap_should_discard(DTLS1_BITMAP *bitmap,
                                         uint64_t seq_num) {
-  const unsigned kWindowSize = sizeof(bitmap->map) * 8;
+  const size_t kWindowSize = bitmap->map.size();
 
   if (seq_num > bitmap->max_seq_num) {
     return false;
   }
   uint64_t idx = bitmap->max_seq_num - seq_num;
-  return idx >= kWindowSize || (bitmap->map & (((uint64_t)1) << idx));
+  return idx >= kWindowSize || bitmap->map[idx];
 }
 
 // dtls1_bitmap_record updates |bitmap| to record receipt of sequence number
 // |seq_num|. It slides the window forward if needed. It is an error to call
 // this function on a stale sequence number.
 static void dtls1_bitmap_record(DTLS1_BITMAP *bitmap, uint64_t seq_num) {
-  const unsigned kWindowSize = sizeof(bitmap->map) * 8;
+  const size_t kWindowSize = bitmap->map.size();
 
   // Shift the window if necessary.
   if (seq_num > bitmap->max_seq_num) {
     uint64_t shift = seq_num - bitmap->max_seq_num;
     if (shift >= kWindowSize) {
-      bitmap->map = 0;
+      bitmap->map.reset();
     } else {
       bitmap->map <<= shift;
     }
@@ -155,7 +155,7 @@ static void dtls1_bitmap_record(DTLS1_BITMAP *bitmap, uint64_t seq_num) {
 
   uint64_t idx = bitmap->max_seq_num - seq_num;
   if (idx < kWindowSize) {
-    bitmap->map |= ((uint64_t)1) << idx;
+    bitmap->map[idx] = true;
   }
 }
 
diff --git a/ssl/internal.h b/ssl/internal.h
index 0c2c2f8..669cb77 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -147,6 +147,7 @@
 #include <stdlib.h>
 
 #include <algorithm>
+#include <bitset>
 #include <initializer_list>
 #include <limits>
 #include <new>
@@ -961,9 +962,9 @@ class SSLAEADContext {
 // DTLS1_BITMAP maintains a sliding window of 64 sequence numbers to detect
 // replayed packets. It should be initialized by zeroing every field.
 struct DTLS1_BITMAP {
-  // map is a bit mask of the last 64 sequence numbers. Bit
-  // |1<<i| corresponds to |max_seq_num - i|.
-  uint64_t map = 0;
+  // map is a bitset of sequence numbers that have been seen. Bit i corresponds
+  // to |max_seq_num - i|.
+  std::bitset<256> map;
   // max_seq_num is the largest sequence number seen so far as a 64-bit
   // integer.
   uint64_t max_seq_num = 0;
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 70cb520..6f2befd 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -9897,7 +9897,7 @@ func addDTLSReplayTests() {
 		config: Config{
 			Bugs: ProtocolBugs{
 				SequenceNumberMapping: func(in uint64) uint64 {
-					return in * 127
+					return in * 1023
 				},
 			},
 		},
@@ -9912,11 +9912,15 @@ func addDTLSReplayTests() {
 		config: Config{
 			Bugs: ProtocolBugs{
 				SequenceNumberMapping: func(in uint64) uint64 {
-					return in ^ 31
+					// This mapping has numbers counting backwards in groups
+					// of 256, and then jumping forwards 511 numbers.
+					return in ^ 255
 				},
 			},
 		},
-		messageCount: 200,
+		// This messageCount is large enough to make sure that the SequenceNumberMapping
+		// will reach the point where it jumps forwards after stepping backwards.
+		messageCount: 500,
 		replayWrites: true,
 	})
 }