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, }) }