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>
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 @@
// |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 @@
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 @@
// 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 @@
config: Config{
Bugs: ProtocolBugs{
SequenceNumberMapping: func(in uint64) uint64 {
- return in * 127
+ return in * 1023
},
},
},
@@ -9912,11 +9912,15 @@
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,
})
}