dcsctp: Update zero checksum option to v-06 draft

https://datatracker.ietf.org/doc/draft-ietf-tsvwg-sctp-zero-checksum/06/

The previous implementation was for version 00, and since then changes
have been made. The chunk that is used to negotiate this capability has
now grown to include an additional property - the sender's alternate
error detection method.

Bug: webrtc:14997
Change-Id: I78043d187b79f40bbadbcba02eae6eedf54f30f9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/336380
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41759}

net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter.cc

@@ -15,32 +15,44 @@
 
 #include "absl/types/optional.h"
 #include "api/array_view.h"
+#include "rtc_base/strings/string_builder.h"
 
 namespace dcsctp {
 
 // https://www.ietf.org/archive/id/draft-tuexen-tsvwg-sctp-zero-checksum-00.html#section-3
 
-//   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
-//  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-//  |         Type = 0x8001         |          Length = 4           |
-//  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+//  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// |   Type = 0x8001 (suggested)   |          Length = 8           |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// |           Error Detection Method Identifier (EDMID)           |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 constexpr int ZeroChecksumAcceptableChunkParameter::kType;
 
 absl::optional<ZeroChecksumAcceptableChunkParameter>
 ZeroChecksumAcceptableChunkParameter::Parse(
     rtc::ArrayView<const uint8_t> data) {
-  if (!ParseTLV(data).has_value()) {
+  absl::optional<BoundedByteReader<kHeaderSize>> reader = ParseTLV(data);
+  if (!reader.has_value()) {
     return absl::nullopt;
   }
-  return ZeroChecksumAcceptableChunkParameter();
+
+  ZeroChecksumAlternateErrorDetectionMethod method(reader->Load32<4>());
+  if (method == ZeroChecksumAlternateErrorDetectionMethod::None()) {
+    return absl::nullopt;
+  }
+  return ZeroChecksumAcceptableChunkParameter(method);
 }
 
 void ZeroChecksumAcceptableChunkParameter::SerializeTo(
     std::vector<uint8_t>& out) const {
-  AllocateTLV(out);
+  BoundedByteWriter<kHeaderSize> writer = AllocateTLV(out);
+  writer.Store32<4>(*error_detection_method_);
 }
 
 std::string ZeroChecksumAcceptableChunkParameter::ToString() const {
-  return "Zero Checksum Acceptable";
+  rtc::StringBuilder sb;
+  sb << "Zero Checksum Acceptable (" << *error_detection_method_ << ")";
+  return sb.Release();
 }
 }  // namespace dcsctp

net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter.h

@@ -19,13 +19,14 @@
 #include "api/array_view.h"
 #include "net/dcsctp/packet/parameter/parameter.h"
 #include "net/dcsctp/packet/tlv_trait.h"
+#include "net/dcsctp/public/types.h"
 
 namespace dcsctp {
 
-// https://datatracker.ietf.org/doc/draft-tuexen-tsvwg-sctp-zero-checksum/
+// https://datatracker.ietf.org/doc/draft-ietf-tsvwg-sctp-zero-checksum/
 struct ZeroChecksumAcceptableChunkParameterConfig : ParameterConfig {
   static constexpr int kType = 0x8001;
-  static constexpr size_t kHeaderSize = 4;
+  static constexpr size_t kHeaderSize = 8;
   static constexpr size_t kVariableLengthAlignment = 0;
 };
 
@@ -36,13 +37,22 @@ class ZeroChecksumAcceptableChunkParameter
   static constexpr int kType =
       ZeroChecksumAcceptableChunkParameterConfig::kType;
 
-  ZeroChecksumAcceptableChunkParameter() {}
+  explicit ZeroChecksumAcceptableChunkParameter(
+      ZeroChecksumAlternateErrorDetectionMethod error_detection_method)
+      : error_detection_method_(error_detection_method) {}
 
   static absl::optional<ZeroChecksumAcceptableChunkParameter> Parse(
       rtc::ArrayView<const uint8_t> data);
 
   void SerializeTo(std::vector<uint8_t>& out) const override;
   std::string ToString() const override;
+
+  ZeroChecksumAlternateErrorDetectionMethod error_detection_method() const {
+    return error_detection_method_;
+  }
+
+ private:
+  ZeroChecksumAlternateErrorDetectionMethod error_detection_method_;
 };
 
 }  // namespace dcsctp

net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter_test.cc

@@ -24,18 +24,28 @@ namespace {
 using ::testing::ElementsAre;
 
 TEST(ZeroChecksumAcceptableChunkParameterTest, SerializeAndDeserialize) {
-  ZeroChecksumAcceptableChunkParameter parameter;
+  ZeroChecksumAcceptableChunkParameter parameter(
+      ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls());
 
   std::vector<uint8_t> serialized;
   parameter.SerializeTo(serialized);
 
-  EXPECT_THAT(serialized, ElementsAre(0x80, 0x01, 0x00, 0x04));
+  EXPECT_THAT(serialized,
+              ElementsAre(0x80, 0x01, 0x00, 0x08, 0x00, 0x00, 0x00, 0x01));
 
   ASSERT_HAS_VALUE_AND_ASSIGN(
       ZeroChecksumAcceptableChunkParameter deserialized,
       ZeroChecksumAcceptableChunkParameter::Parse(serialized));
 }
 
+TEST(ZeroChecksumAcceptableChunkParameterTest, FailToDeserializePrevVersion) {
+  // This is how the draft described the chunk as, in version 00.
+  std::vector<uint8_t> invalid = {0x80, 0x01, 0x00, 0x04};
+
+  EXPECT_FALSE(
+      ZeroChecksumAcceptableChunkParameter::Parse(invalid).has_value());
+}
+
 TEST(ZeroChecksumAcceptableChunkParameterTest, FailToDeserialize) {
   std::vector<uint8_t> invalid = {0x00, 0x00, 0x00, 0x00};
 
@@ -44,9 +54,10 @@ TEST(ZeroChecksumAcceptableChunkParameterTest, FailToDeserialize) {
 }
 
 TEST(ZeroChecksumAcceptableChunkParameterTest, HasToString) {
-  ZeroChecksumAcceptableChunkParameter parameter;
+  ZeroChecksumAcceptableChunkParameter parameter(
+      ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls());
 
-  EXPECT_EQ(parameter.ToString(), "Zero Checksum Acceptable");
+  EXPECT_EQ(parameter.ToString(), "Zero Checksum Acceptable (1)");
 }
 
 }  // namespace

net/dcsctp/packet/sctp_packet.cc

@@ -126,7 +126,9 @@ absl::optional<SctpPacket> SctpPacket::Parse(rtc::ArrayView<const uint8_t> data,
       std::vector<uint8_t>(data.begin(), data.end());
 
   if (options.disable_checksum_verification ||
-      (options.enable_zero_checksum && common_header.checksum == 0u)) {
+      (options.zero_checksum_alternate_error_detection_method !=
+           ZeroChecksumAlternateErrorDetectionMethod::None() &&
+       common_header.checksum == 0u)) {
     // https://www.ietf.org/archive/id/draft-tuexen-tsvwg-sctp-zero-checksum-01.html#section-4.3:
     // If an end point has sent the Zero Checksum Acceptable Chunk Parameter in
     // an INIT or INIT ACK chunk, it MUST accept SCTP packets using an incorrect

net/dcsctp/packet/sctp_packet_test.cc

@@ -38,7 +38,8 @@ using ::testing::SizeIs;
 constexpr VerificationTag kVerificationTag = VerificationTag(0x12345678);
 constexpr DcSctpOptions kVerifyChecksumOptions =
     DcSctpOptions{.disable_checksum_verification = false,
-                  .enable_zero_checksum = false};
+                  .zero_checksum_alternate_error_detection_method =
+                      ZeroChecksumAlternateErrorDetectionMethod::None()};
 
 TEST(SctpPacketTest, DeserializeSimplePacketFromCapture) {
   /*
@@ -208,8 +209,10 @@ TEST(SctpPacketTest, DeserializePacketDontValidateChecksum) {
 
   ASSERT_HAS_VALUE_AND_ASSIGN(
       SctpPacket packet,
-      SctpPacket::Parse(data, {.disable_checksum_verification = true,
-                               .enable_zero_checksum = false}));
+      SctpPacket::Parse(
+          data, {.disable_checksum_verification = true,
+                 .zero_checksum_alternate_error_detection_method =
+                     ZeroChecksumAlternateErrorDetectionMethod::None()}));
   EXPECT_EQ(packet.common_header().source_port, 5000);
   EXPECT_EQ(packet.common_header().destination_port, 5000);
   EXPECT_EQ(packet.common_header().verification_tag,
@@ -375,8 +378,11 @@ TEST(SctpPacketTest, AcceptsZeroSetZeroChecksum) {
 
   ASSERT_HAS_VALUE_AND_ASSIGN(
       SctpPacket packet,
-      SctpPacket::Parse(data, {.disable_checksum_verification = false,
-                               .enable_zero_checksum = true}));
+      SctpPacket::Parse(
+          data,
+          {.disable_checksum_verification = false,
+           .zero_checksum_alternate_error_detection_method =
+               ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls()}));
   EXPECT_EQ(packet.common_header().source_port, 5000);
   EXPECT_EQ(packet.common_header().destination_port, 5000);
   EXPECT_EQ(packet.common_header().verification_tag,
@@ -409,9 +415,13 @@ TEST(SctpPacketTest, RejectsNonZeroIncorrectChecksumWhenZeroChecksumIsActive) {
                     0x00, 0x00, 0x03, 0x00, 0x00, 0x10, 0x55, 0x08, 0x36, 0x40,
                     0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
 
-  EXPECT_FALSE(SctpPacket::Parse(data, {.disable_checksum_verification = false,
-                                        .enable_zero_checksum = true})
-                   .has_value());
+  EXPECT_FALSE(
+      SctpPacket::Parse(
+          data,
+          {.disable_checksum_verification = false,
+           .zero_checksum_alternate_error_detection_method =
+               ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls()})
+          .has_value());
 }
 
 TEST(SctpPacketTest, WritePacketWithCalculatedChecksum) {

net/dcsctp/public/dcsctp_options.h

@@ -196,14 +196,13 @@ struct DcSctpOptions {
   // Disables SCTP packet crc32 verification. For fuzzers only!
   bool disable_checksum_verification = false;
 
-  // Controls the acceptance of zero checksum, as defined in
-  // https://datatracker.ietf.org/doc/draft-tuexen-tsvwg-sctp-zero-checksum/
-  // This should only be enabled if the packet integrity can be ensured by lower
-  // layers, which DTLS will do in WebRTC, as defined by RFC8261.
-  //
-  // This will also enable sending packets without a checksum value (set to 0)
-  // once both peers have negotiated this feature.
-  bool enable_zero_checksum = false;
+  // Controls the "zero checksum option" feature, as defined in
+  // https://www.ietf.org/archive/id/draft-ietf-tsvwg-sctp-zero-checksum-06.html.
+  // To have this feature enabled, both peers must be configured to use the
+  // same (defined, not "none") alternate error detection method.
+  ZeroChecksumAlternateErrorDetectionMethod
+      zero_checksum_alternate_error_detection_method =
+          ZeroChecksumAlternateErrorDetectionMethod::None();
 };
 }  // namespace dcsctp
 

net/dcsctp/public/types.h

@@ -151,6 +151,27 @@ class LifecycleId : public webrtc::StrongAlias<class LifecycleIdTag, uint64_t> {
 
   static constexpr LifecycleId NotSet() { return LifecycleId(0); }
 };
+
+// To enable zero checksum feature, both peers must agree on which alternate
+// error detection method that is used. See
+// https://www.ietf.org/archive/id/draft-ietf-tsvwg-sctp-zero-checksum-06.html.
+class ZeroChecksumAlternateErrorDetectionMethod
+    : public webrtc::StrongAlias<
+          class ZeroChecksumAlternateErrorDetectionMethodTag,
+          uint32_t> {
+ public:
+  constexpr explicit ZeroChecksumAlternateErrorDetectionMethod(
+      const UnderlyingType& v)
+      : webrtc::StrongAlias<class ZeroChecksumAlternateErrorDetectionMethodTag,
+                            uint32_t>(v) {}
+
+  static constexpr ZeroChecksumAlternateErrorDetectionMethod None() {
+    return ZeroChecksumAlternateErrorDetectionMethod(0);
+  }
+  static constexpr ZeroChecksumAlternateErrorDetectionMethod LowerLayerDtls() {
+    return ZeroChecksumAlternateErrorDetectionMethod(1);
+  }
+};
 }  // namespace dcsctp
 
 #endif  // NET_DCSCTP_PUBLIC_TYPES_H_

net/dcsctp/socket/capabilities.h

@@ -21,7 +21,7 @@ struct Capabilities {
   bool message_interleaving = false;
   // RFC6525 Stream Reconfiguration
   bool reconfig = false;
-  // https://datatracker.ietf.org/doc/draft-tuexen-tsvwg-sctp-zero-checksum/
+  // https://datatracker.ietf.org/doc/draft-ietf-tsvwg-sctp-zero-checksum/
   bool zero_checksum = false;
   // Negotiated maximum incoming and outgoing stream count.
   uint16_t negotiated_maximum_incoming_streams = 0;

net/dcsctp/socket/dcsctp_socket.cc

@@ -22,7 +22,6 @@
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
 #include "api/array_view.h"
-#include "api/task_queue/task_queue_base.h"
 #include "net/dcsctp/packet/chunk/abort_chunk.h"
 #include "net/dcsctp/packet/chunk/chunk.h"
 #include "net/dcsctp/packet/chunk/cookie_ack_chunk.h"
@@ -120,8 +119,12 @@ Capabilities ComputeCapabilities(const DcSctpOptions& options,
     capabilities.reconfig = true;
   }
 
-  if (options.enable_zero_checksum &&
-      parameters.get<ZeroChecksumAcceptableChunkParameter>().has_value()) {
+  if (options.zero_checksum_alternate_error_detection_method !=
+          ZeroChecksumAlternateErrorDetectionMethod::None() &&
+      parameters.get<ZeroChecksumAcceptableChunkParameter>().has_value() &&
+      parameters.get<ZeroChecksumAcceptableChunkParameter>()
+              ->error_detection_method() ==
+          options.zero_checksum_alternate_error_detection_method) {
     capabilities.zero_checksum = true;
   }
 
@@ -134,6 +137,7 @@ Capabilities ComputeCapabilities(const DcSctpOptions& options,
 }
 
 void AddCapabilityParameters(const DcSctpOptions& options,
+                             bool support_zero_checksum,
                              Parameters::Builder& builder) {
   std::vector<uint8_t> chunk_types = {ReConfigChunk::kType};
 
@@ -145,8 +149,11 @@ void AddCapabilityParameters(const DcSctpOptions& options,
     chunk_types.push_back(IDataChunk::kType);
     chunk_types.push_back(IForwardTsnChunk::kType);
   }
-  if (options.enable_zero_checksum) {
-    builder.Add(ZeroChecksumAcceptableChunkParameter());
+  if (support_zero_checksum) {
+    RTC_DCHECK(options.zero_checksum_alternate_error_detection_method !=
+               ZeroChecksumAlternateErrorDetectionMethod::None());
+    builder.Add(ZeroChecksumAcceptableChunkParameter(
+        options.zero_checksum_alternate_error_detection_method));
   }
   builder.Add(SupportedExtensionsParameter(std::move(chunk_types)));
 }
@@ -282,7 +289,11 @@ void DcSctpSocket::SetState(State state, absl::string_view reason) {
 
 void DcSctpSocket::SendInit() {
   Parameters::Builder params_builder;
-  AddCapabilityParameters(options_, params_builder);
+  AddCapabilityParameters(
+      options_, /*support_zero_checksum=*/
+      options_.zero_checksum_alternate_error_detection_method !=
+          ZeroChecksumAlternateErrorDetectionMethod::None(),
+      params_builder);
   InitChunk init(/*initiate_tag=*/connect_params_.verification_tag,
                  /*a_rwnd=*/options_.max_receiver_window_buffer_size,
                  options_.announced_maximum_outgoing_streams,
@@ -1227,7 +1238,7 @@ void DcSctpSocket::HandleInit(const CommonHeader& header,
                       chunk->initial_tsn(), my_initial_tsn, chunk->a_rwnd(),
                       tie_tag, capabilities)
               .Serialize()));
-  AddCapabilityParameters(options_, params_builder);
+  AddCapabilityParameters(options_, capabilities.zero_checksum, params_builder);
 
   InitAckChunk init_ack(/*initiate_tag=*/my_verification_tag,
                         options_.max_receiver_window_buffer_size,

net/dcsctp/socket/dcsctp_socket_test.cc

@@ -2887,8 +2887,16 @@ TEST_P(DcSctpSocketParametrizedTest, ZeroChecksumMetricsAreSet) {
   std::vector<std::pair<bool, bool>> combinations = {
       {false, false}, {false, true}, {true, false}, {true, true}};
   for (const auto& [a_enable, z_enable] : combinations) {
-    DcSctpOptions a_options = {.enable_zero_checksum = a_enable};
-    DcSctpOptions z_options = {.enable_zero_checksum = z_enable};
+    DcSctpOptions a_options = {
+        .zero_checksum_alternate_error_detection_method =
+            a_enable
+                ? ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls()
+                : ZeroChecksumAlternateErrorDetectionMethod::None()};
+    DcSctpOptions z_options = {
+        .zero_checksum_alternate_error_detection_method =
+            z_enable
+                ? ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls()
+                : ZeroChecksumAlternateErrorDetectionMethod::None()};
 
     SocketUnderTest a("A", a_options);
     auto z = std::make_unique<SocketUnderTest>("Z", z_options);
@@ -2902,7 +2910,9 @@ TEST_P(DcSctpSocketParametrizedTest, ZeroChecksumMetricsAreSet) {
 }
 
 TEST(DcSctpSocketTest, AlwaysSendsInitWithNonZeroChecksum) {
-  DcSctpOptions options = {.enable_zero_checksum = true};
+  DcSctpOptions options = {
+      .zero_checksum_alternate_error_detection_method =
+          ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls()};
   SocketUnderTest a("A", options);
 
   a.socket.Connect();
@@ -2916,7 +2926,9 @@ TEST(DcSctpSocketTest, AlwaysSendsInitWithNonZeroChecksum) {
 }
 
 TEST(DcSctpSocketTest, MaySendInitAckWithZeroChecksum) {
-  DcSctpOptions options = {.enable_zero_checksum = true};
+  DcSctpOptions options = {
+      .zero_checksum_alternate_error_detection_method =
+          ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls()};
   SocketUnderTest a("A", options);
   SocketUnderTest z("Z", options);
 
@@ -2933,7 +2945,9 @@ TEST(DcSctpSocketTest, MaySendInitAckWithZeroChecksum) {
 }
 
 TEST(DcSctpSocketTest, AlwaysSendsCookieEchoWithNonZeroChecksum) {
-  DcSctpOptions options = {.enable_zero_checksum = true};
+  DcSctpOptions options = {
+      .zero_checksum_alternate_error_detection_method =
+          ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls()};
   SocketUnderTest a("A", options);
   SocketUnderTest z("Z", options);
 
@@ -2951,7 +2965,9 @@ TEST(DcSctpSocketTest, AlwaysSendsCookieEchoWithNonZeroChecksum) {
 }
 
 TEST(DcSctpSocketTest, SendsCookieAckWithZeroChecksum) {
-  DcSctpOptions options = {.enable_zero_checksum = true};
+  DcSctpOptions options = {
+      .zero_checksum_alternate_error_detection_method =
+          ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls()};
   SocketUnderTest a("A", options);
   SocketUnderTest z("Z", options);
 
@@ -2970,7 +2986,9 @@ TEST(DcSctpSocketTest, SendsCookieAckWithZeroChecksum) {
 }
 
 TEST_P(DcSctpSocketParametrizedTest, SendsDataWithZeroChecksum) {
-  DcSctpOptions options = {.enable_zero_checksum = true};
+  DcSctpOptions options = {
+      .zero_checksum_alternate_error_detection_method =
+          ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls()};
   SocketUnderTest a("A", options);
   auto z = std::make_unique<SocketUnderTest>("Z", options);
 
@@ -2993,7 +3011,9 @@ TEST_P(DcSctpSocketParametrizedTest, SendsDataWithZeroChecksum) {
 }
 
 TEST_P(DcSctpSocketParametrizedTest, AllPacketsAfterConnectHaveZeroChecksum) {
-  DcSctpOptions options = {.enable_zero_checksum = true};
+  DcSctpOptions options = {
+      .zero_checksum_alternate_error_detection_method =
+          ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls()};
   SocketUnderTest a("A", options);
   auto z = std::make_unique<SocketUnderTest>("Z", options);
 

net/dcsctp/socket/heartbeat_handler_test.cc

@@ -38,7 +38,8 @@ DcSctpOptions MakeOptions(DurationMs heartbeat_interval) {
   DcSctpOptions options;
   options.heartbeat_interval_include_rtt = false;
   options.heartbeat_interval = heartbeat_interval;
-  options.enable_zero_checksum = false;
+  options.zero_checksum_alternate_error_detection_method =
+      ZeroChecksumAlternateErrorDetectionMethod::None();
   return options;
 }