dcsctp: Add Chunk Validators

The SCTP RFCs aren't very strict in specifying when a chunk or parameter
is invalid, so most chunks and/or parameters must be accepted but they
may need some cleaning to avoid a lot of error handling deeper in the
chunk handling code.

Bug: webrtc:12614
Change-Id: I723f08cbdc26e1a1b78463b6137340e638089037
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214966
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33788}

net/dcsctp/packet/BUILD.gn

@@ -210,6 +210,19 @@ rtc_library("chunk") {
   absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
 }
 
+rtc_library("chunk_validators") {
+  deps = [
+    ":chunk",
+    "../../../rtc_base",
+    "../../../rtc_base:checks",
+    "../../../rtc_base:rtc_base_approved",
+  ]
+  sources = [
+    "chunk_validators.cc",
+    "chunk_validators.h",
+  ]
+}
+
 rtc_library("sctp_packet") {
   deps = [
     ":bounded_io",
@@ -240,6 +253,7 @@ if (rtc_include_tests) {
     deps = [
       ":bounded_io",
       ":chunk",
+      ":chunk_validators",
       ":crc32c",
       ":error_cause",
       ":parameter",
@@ -271,6 +285,7 @@ if (rtc_include_tests) {
       "chunk/shutdown_ack_chunk_test.cc",
       "chunk/shutdown_chunk_test.cc",
       "chunk/shutdown_complete_chunk_test.cc",
+      "chunk_validators_test.cc",
       "crc32c_test.cc",
       "error_cause/cookie_received_while_shutting_down_cause_test.cc",
       "error_cause/invalid_mandatory_parameter_cause_test.cc",

net/dcsctp/packet/chunk_validators.cc

@@ -0,0 +1,90 @@
+/*
+ *  Copyright (c) 2021 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+#include "net/dcsctp/packet/chunk_validators.h"
+
+#include <algorithm>
+#include <utility>
+#include <vector>
+
+#include "net/dcsctp/packet/chunk/sack_chunk.h"
+#include "rtc_base/logging.h"
+
+namespace dcsctp {
+
+SackChunk ChunkValidators::Clean(SackChunk&& sack) {
+  if (Validate(sack)) {
+    return std::move(sack);
+  }
+
+  RTC_DLOG(LS_WARNING) << "Received SACK is malformed; cleaning it";
+
+  std::vector<SackChunk::GapAckBlock> gap_ack_blocks;
+  gap_ack_blocks.reserve(sack.gap_ack_blocks().size());
+
+  // First: Only keep blocks that are sane
+  for (const SackChunk::GapAckBlock& gap_ack_block : sack.gap_ack_blocks()) {
+    if (gap_ack_block.end > gap_ack_block.start) {
+      gap_ack_blocks.emplace_back(gap_ack_block);
+    }
+  }
+
+  // Not more than at most one remaining? Exit early.
+  if (gap_ack_blocks.size() <= 1) {
+    return SackChunk(sack.cumulative_tsn_ack(), sack.a_rwnd(),
+                     std::move(gap_ack_blocks),
+                     std::vector<TSN>(sack.duplicate_tsns().begin(),
+                                      sack.duplicate_tsns().end()));
+  }
+
+  // Sort the intervals by their start value, to aid in the merging below.
+  absl::c_sort(gap_ack_blocks, [&](const SackChunk::GapAckBlock& a,
+                                   const SackChunk::GapAckBlock& b) {
+    return a.start < b.start;
+  });
+
+  // Merge overlapping ranges.
+  std::vector<SackChunk::GapAckBlock> merged;
+  merged.reserve(gap_ack_blocks.size());
+  merged.push_back(gap_ack_blocks[0]);
+
+  for (size_t i = 1; i < gap_ack_blocks.size(); ++i) {
+    if (merged.back().end + 1 >= gap_ack_blocks[i].start) {
+      merged.back().end = std::max(merged.back().end, gap_ack_blocks[i].end);
+    } else {
+      merged.push_back(gap_ack_blocks[i]);
+    }
+  }
+
+  return SackChunk(sack.cumulative_tsn_ack(), sack.a_rwnd(), std::move(merged),
+                   std::vector<TSN>(sack.duplicate_tsns().begin(),
+                                    sack.duplicate_tsns().end()));
+}
+
+bool ChunkValidators::Validate(const SackChunk& sack) {
+  if (sack.gap_ack_blocks().empty()) {
+    return true;
+  }
+
+  // Ensure that gap-ack-blocks are sorted, has an "end" that is not before
+  // "start" and are non-overlapping and non-adjacent.
+  uint16_t prev_end = 0;
+  for (const SackChunk::GapAckBlock& gap_ack_block : sack.gap_ack_blocks()) {
+    if (gap_ack_block.end < gap_ack_block.start) {
+      return false;
+    }
+    if (gap_ack_block.start <= (prev_end + 1)) {
+      return false;
+    }
+    prev_end = gap_ack_block.end;
+  }
+  return true;
+}
+
+}  // namespace dcsctp

net/dcsctp/packet/chunk_validators.h

@@ -0,0 +1,33 @@
+/*
+ *  Copyright (c) 2021 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+#ifndef NET_DCSCTP_PACKET_CHUNK_VALIDATORS_H_
+#define NET_DCSCTP_PACKET_CHUNK_VALIDATORS_H_
+
+#include "net/dcsctp/packet/chunk/sack_chunk.h"
+
+namespace dcsctp {
+// Validates and cleans SCTP chunks.
+class ChunkValidators {
+ public:
+  // Given a SackChunk, will return `true` if it's valid, and `false` if not.
+  static bool Validate(const SackChunk& sack);
+
+  // Given a SackChunk, it will return a cleaned and validated variant of it.
+  // RFC4960 doesn't say anything about validity of SACKs or if the Gap ACK
+  // blocks must be sorted, and non-overlapping. While they always are in
+  // well-behaving implementations, this can't be relied on.
+  //
+  // This method internally calls `Validate`, which means that you can always
+  // pass a SackChunk to this method (valid or not), and use the results.
+  static SackChunk Clean(SackChunk&& sack);
+};
+}  // namespace dcsctp
+
+#endif  // NET_DCSCTP_PACKET_CHUNK_VALIDATORS_H_

net/dcsctp/packet/chunk_validators_test.cc

@@ -0,0 +1,161 @@
+/*
+ *  Copyright (c) 2021 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+#include "net/dcsctp/packet/chunk_validators.h"
+
+#include <utility>
+
+#include "rtc_base/gunit.h"
+#include "test/gmock.h"
+
+namespace dcsctp {
+namespace {
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+
+TEST(ChunkValidatorsTest, NoGapAckBlocksAreValid) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456,
+                 /*gap_ack_blocks=*/{}, {});
+
+  EXPECT_TRUE(ChunkValidators::Validate(sack));
+
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+  EXPECT_THAT(clean.gap_ack_blocks(), IsEmpty());
+}
+
+TEST(ChunkValidatorsTest, OneValidAckBlock) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456, {SackChunk::GapAckBlock(2, 3)}, {});
+
+  EXPECT_TRUE(ChunkValidators::Validate(sack));
+
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+  EXPECT_THAT(clean.gap_ack_blocks(),
+              ElementsAre(SackChunk::GapAckBlock(2, 3)));
+}
+
+TEST(ChunkValidatorsTest, TwoValidAckBlocks) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456,
+                 {SackChunk::GapAckBlock(2, 3), SackChunk::GapAckBlock(5, 6)},
+                 {});
+
+  EXPECT_TRUE(ChunkValidators::Validate(sack));
+
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+  EXPECT_THAT(
+      clean.gap_ack_blocks(),
+      ElementsAre(SackChunk::GapAckBlock(2, 3), SackChunk::GapAckBlock(5, 6)));
+}
+
+TEST(ChunkValidatorsTest, OneInvalidAckBlock) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456, {SackChunk::GapAckBlock(1, 2)}, {});
+
+  EXPECT_FALSE(ChunkValidators::Validate(sack));
+
+  // It's not strictly valid, but due to the renegable nature of gap ack blocks,
+  // the cum_ack_tsn can't simply be moved.
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+  EXPECT_THAT(clean.gap_ack_blocks(),
+              ElementsAre(SackChunk::GapAckBlock(1, 2)));
+}
+
+TEST(ChunkValidatorsTest, RemovesInvalidGapAckBlockFromSack) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456,
+                 {SackChunk::GapAckBlock(2, 3), SackChunk::GapAckBlock(6, 4)},
+                 {});
+
+  EXPECT_FALSE(ChunkValidators::Validate(sack));
+
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+
+  EXPECT_THAT(clean.gap_ack_blocks(),
+              ElementsAre(SackChunk::GapAckBlock(2, 3)));
+}
+
+TEST(ChunkValidatorsTest, SortsGapAckBlocksInOrder) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456,
+                 {SackChunk::GapAckBlock(6, 7), SackChunk::GapAckBlock(3, 4)},
+                 {});
+
+  EXPECT_FALSE(ChunkValidators::Validate(sack));
+
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+
+  EXPECT_THAT(
+      clean.gap_ack_blocks(),
+      ElementsAre(SackChunk::GapAckBlock(3, 4), SackChunk::GapAckBlock(6, 7)));
+}
+
+TEST(ChunkValidatorsTest, MergesAdjacentBlocks) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456,
+                 {SackChunk::GapAckBlock(3, 4), SackChunk::GapAckBlock(5, 6)},
+                 {});
+
+  EXPECT_FALSE(ChunkValidators::Validate(sack));
+
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+
+  EXPECT_THAT(clean.gap_ack_blocks(),
+              ElementsAre(SackChunk::GapAckBlock(3, 6)));
+}
+
+TEST(ChunkValidatorsTest, MergesOverlappingByOne) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456,
+                 {SackChunk::GapAckBlock(3, 4), SackChunk::GapAckBlock(4, 5)},
+                 {});
+
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+
+  EXPECT_FALSE(ChunkValidators::Validate(sack));
+
+  EXPECT_THAT(clean.gap_ack_blocks(),
+              ElementsAre(SackChunk::GapAckBlock(3, 5)));
+}
+
+TEST(ChunkValidatorsTest, MergesOverlappingByMore) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456,
+                 {SackChunk::GapAckBlock(3, 10), SackChunk::GapAckBlock(4, 5)},
+                 {});
+
+  EXPECT_FALSE(ChunkValidators::Validate(sack));
+
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+
+  EXPECT_THAT(clean.gap_ack_blocks(),
+              ElementsAre(SackChunk::GapAckBlock(3, 10)));
+}
+
+TEST(ChunkValidatorsTest, MergesBlocksStartingWithSameStartOffset) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456,
+                 {SackChunk::GapAckBlock(3, 7), SackChunk::GapAckBlock(3, 5),
+                  SackChunk::GapAckBlock(3, 9)},
+                 {});
+
+  EXPECT_FALSE(ChunkValidators::Validate(sack));
+
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+
+  EXPECT_THAT(clean.gap_ack_blocks(),
+              ElementsAre(SackChunk::GapAckBlock(3, 9)));
+}
+
+TEST(ChunkValidatorsTest, MergesBlocksPartiallyOverlapping) {
+  SackChunk sack(TSN(123), /*a_rwnd=*/456,
+                 {SackChunk::GapAckBlock(3, 7), SackChunk::GapAckBlock(5, 9)},
+                 {});
+
+  EXPECT_FALSE(ChunkValidators::Validate(sack));
+
+  SackChunk clean = ChunkValidators::Clean(std::move(sack));
+
+  EXPECT_THAT(clean.gap_ack_blocks(),
+              ElementsAre(SackChunk::GapAckBlock(3, 9)));
+}
+
+}  // namespace
+}  // namespace dcsctp