Reapply "dcsctp: Negotiate zero checksum"

The handover state has been added with
commit daaa6ab5a8c74b87d9d6ded07342d8a2c50c73f7.

This reverts commit 014cbed9d2377ec0a0b15f2c48b21a562f770366.

Bug: webrtc:14997
Change-Id: Ie84f3184f3ea67aaa6438481634046ba18b497a6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/320941
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40794}

net/dcsctp/public/dcsctp_socket.h

@@ -255,6 +255,10 @@ struct Metrics {
   // peers.
   bool uses_message_interleaving = false;
 
+  // Indicates if draft-tuexen-tsvwg-sctp-zero-checksum-00 has been negotiated
+  // by both peers.
+  bool uses_zero_checksum = false;
+
   // The number of negotiated incoming and outgoing streams, which is configured
   // locally as `DcSctpOptions::announced_maximum_incoming_streams` and
   // `DcSctpOptions::announced_maximum_outgoing_streams`, and which will be

net/dcsctp/socket/capabilities.h

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

net/dcsctp/socket/dcsctp_socket.cc

@@ -56,6 +56,7 @@
 #include "net/dcsctp/packet/parameter/parameter.h"
 #include "net/dcsctp/packet/parameter/state_cookie_parameter.h"
 #include "net/dcsctp/packet/parameter/supported_extensions_parameter.h"
+#include "net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter.h"
 #include "net/dcsctp/packet/sctp_packet.h"
 #include "net/dcsctp/packet/tlv_trait.h"
 #include "net/dcsctp/public/dcsctp_message.h"
@@ -117,6 +118,11 @@ Capabilities ComputeCapabilities(const DcSctpOptions& options,
     capabilities.reconfig = true;
   }
 
+  if (options.enable_zero_checksum &&
+      parameters.get<ZeroChecksumAcceptableChunkParameter>().has_value()) {
+    capabilities.zero_checksum = true;
+  }
+
   capabilities.negotiated_maximum_incoming_streams = std::min(
       options.announced_maximum_incoming_streams, peer_nbr_outbound_streams);
   capabilities.negotiated_maximum_outgoing_streams = std::min(
@@ -137,6 +143,9 @@ 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());
+  }
   builder.Add(SupportedExtensionsParameter(std::move(chunk_types)));
 }
 
@@ -279,7 +288,10 @@ void DcSctpSocket::SendInit() {
                  connect_params_.initial_tsn, params_builder.Build());
   SctpPacket::Builder b(VerificationTag(0), options_);
   b.Add(init);
-  packet_sender_.Send(b);
+  // https://www.ietf.org/archive/id/draft-tuexen-tsvwg-sctp-zero-checksum-01.html#section-4.2
+  // "When an end point sends a packet containing an INIT chunk, it MUST include
+  // a correct CRC32c checksum in the packet containing the INIT chunk."
+  packet_sender_.Send(b, /*write_checksum=*/true);
 }
 
 void DcSctpSocket::MakeConnectionParameters() {
@@ -320,6 +332,7 @@ void DcSctpSocket::CreateTransmissionControlBlock(
     size_t a_rwnd,
     TieTag tie_tag) {
   metrics_.uses_message_interleaving = capabilities.message_interleaving;
+  metrics_.uses_zero_checksum = capabilities.zero_checksum;
   metrics_.negotiated_maximum_incoming_streams =
       capabilities.negotiated_maximum_incoming_streams;
   metrics_.negotiated_maximum_outgoing_streams =
@@ -351,6 +364,7 @@ void DcSctpSocket::RestoreFromState(const DcSctpSocketHandoverState& state) {
       capabilities.message_interleaving =
           state.capabilities.message_interleaving;
       capabilities.reconfig = state.capabilities.reconfig;
+      capabilities.zero_checksum = state.capabilities.zero_checksum;
       capabilities.negotiated_maximum_incoming_streams =
           state.capabilities.negotiated_maximum_incoming_streams;
       capabilities.negotiated_maximum_outgoing_streams =
@@ -1211,7 +1225,9 @@ void DcSctpSocket::HandleInit(const CommonHeader& header,
                         options_.announced_maximum_incoming_streams,
                         connect_params_.initial_tsn, params_builder.Build());
   b.Add(init_ack);
-  packet_sender_.Send(b);
+  // If the peer has signaled that it supports zero checksum, INIT-ACK can then
+  // have its checksum as zero.
+  packet_sender_.Send(b, /*write_checksum=*/!capabilities.zero_checksum);
 }
 
 void DcSctpSocket::HandleInitAck(

net/dcsctp/socket/dcsctp_socket_test.cc

@@ -24,6 +24,7 @@
 #include "net/dcsctp/common/handover_testing.h"
 #include "net/dcsctp/common/math.h"
 #include "net/dcsctp/packet/chunk/chunk.h"
+#include "net/dcsctp/packet/chunk/cookie_ack_chunk.h"
 #include "net/dcsctp/packet/chunk/cookie_echo_chunk.h"
 #include "net/dcsctp/packet/chunk/data_chunk.h"
 #include "net/dcsctp/packet/chunk/data_common.h"
@@ -31,6 +32,7 @@
 #include "net/dcsctp/packet/chunk/heartbeat_ack_chunk.h"
 #include "net/dcsctp/packet/chunk/heartbeat_request_chunk.h"
 #include "net/dcsctp/packet/chunk/idata_chunk.h"
+#include "net/dcsctp/packet/chunk/init_ack_chunk.h"
 #include "net/dcsctp/packet/chunk/init_chunk.h"
 #include "net/dcsctp/packet/chunk/sack_chunk.h"
 #include "net/dcsctp/packet/chunk/shutdown_chunk.h"
@@ -59,8 +61,10 @@ namespace {
 using ::testing::_;
 using ::testing::AllOf;
 using ::testing::ElementsAre;
+using ::testing::Eq;
 using ::testing::HasSubstr;
 using ::testing::IsEmpty;
+using ::testing::Not;
 using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 
@@ -2903,5 +2907,156 @@ TEST(DcSctpSocketTest, ResetStreamsWithPausedSenderResumesWhenPerformed) {
   EXPECT_EQ(msg2->payload().size(), kSmallMessageSize);
 }
 
+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};
+
+    SocketUnderTest a("A", a_options);
+    auto z = std::make_unique<SocketUnderTest>("Z", z_options);
+
+    ConnectSockets(a, *z);
+    z = MaybeHandoverSocket(std::move(z));
+
+    EXPECT_EQ(a.socket.GetMetrics()->uses_zero_checksum, a_enable && z_enable);
+    EXPECT_EQ(z->socket.GetMetrics()->uses_zero_checksum, a_enable && z_enable);
+  }
+}
+
+TEST(DcSctpSocketTest, AlwaysSendsInitWithNonZeroChecksum) {
+  DcSctpOptions options = {.enable_zero_checksum = true};
+  SocketUnderTest a("A", options);
+
+  a.socket.Connect();
+  std::vector<uint8_t> data = a.cb.ConsumeSentPacket();
+  ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
+                              SctpPacket::Parse(data, options));
+  EXPECT_THAT(packet.descriptors(),
+              ElementsAre(testing::Field(&SctpPacket::ChunkDescriptor::type,
+                                         InitChunk::kType)));
+  EXPECT_THAT(packet.common_header().checksum, Not(Eq(0u)));
+}
+
+TEST(DcSctpSocketTest, MaySendInitAckWithZeroChecksum) {
+  DcSctpOptions options = {.enable_zero_checksum = true};
+  SocketUnderTest a("A", options);
+  SocketUnderTest z("Z", options);
+
+  a.socket.Connect();
+  z.socket.ReceivePacket(a.cb.ConsumeSentPacket());  // INIT
+
+  std::vector<uint8_t> data = z.cb.ConsumeSentPacket();
+  ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
+                              SctpPacket::Parse(data, options));
+  EXPECT_THAT(packet.descriptors(),
+              ElementsAre(testing::Field(&SctpPacket::ChunkDescriptor::type,
+                                         InitAckChunk::kType)));
+  EXPECT_THAT(packet.common_header().checksum, 0u);
+}
+
+TEST(DcSctpSocketTest, AlwaysSendsCookieEchoWithNonZeroChecksum) {
+  DcSctpOptions options = {.enable_zero_checksum = true};
+  SocketUnderTest a("A", options);
+  SocketUnderTest z("Z", options);
+
+  a.socket.Connect();
+  z.socket.ReceivePacket(a.cb.ConsumeSentPacket());  // INIT
+  a.socket.ReceivePacket(z.cb.ConsumeSentPacket());  // INIT-ACK
+
+  std::vector<uint8_t> data = a.cb.ConsumeSentPacket();
+  ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
+                              SctpPacket::Parse(data, options));
+  EXPECT_THAT(packet.descriptors(),
+              ElementsAre(testing::Field(&SctpPacket::ChunkDescriptor::type,
+                                         CookieEchoChunk::kType)));
+  EXPECT_THAT(packet.common_header().checksum, Not(Eq(0u)));
+}
+
+TEST(DcSctpSocketTest, SendsCookieAckWithZeroChecksum) {
+  DcSctpOptions options = {.enable_zero_checksum = true};
+  SocketUnderTest a("A", options);
+  SocketUnderTest z("Z", options);
+
+  a.socket.Connect();
+  z.socket.ReceivePacket(a.cb.ConsumeSentPacket());  // INIT
+  a.socket.ReceivePacket(z.cb.ConsumeSentPacket());  // INIT-ACK
+  z.socket.ReceivePacket(a.cb.ConsumeSentPacket());  // COOKIE-ECHO
+
+  std::vector<uint8_t> data = z.cb.ConsumeSentPacket();
+  ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
+                              SctpPacket::Parse(data, options));
+  EXPECT_THAT(packet.descriptors(),
+              ElementsAre(testing::Field(&SctpPacket::ChunkDescriptor::type,
+                                         CookieAckChunk::kType)));
+  EXPECT_THAT(packet.common_header().checksum, 0u);
+}
+
+TEST_P(DcSctpSocketParametrizedTest, SendsDataWithZeroChecksum) {
+  DcSctpOptions options = {.enable_zero_checksum = true};
+  SocketUnderTest a("A", options);
+  auto z = std::make_unique<SocketUnderTest>("Z", options);
+
+  ConnectSockets(a, *z);
+  z = MaybeHandoverSocket(std::move(z));
+
+  std::vector<uint8_t> payload(a.options.mtu - 100);
+  a.socket.Send(DcSctpMessage(StreamID(1), PPID(53), payload), {});
+
+  std::vector<uint8_t> data = a.cb.ConsumeSentPacket();
+  z->socket.ReceivePacket(data);
+  ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
+                              SctpPacket::Parse(data, options));
+  EXPECT_THAT(packet.descriptors(),
+              ElementsAre(testing::Field(&SctpPacket::ChunkDescriptor::type,
+                                         DataChunk::kType)));
+  EXPECT_THAT(packet.common_header().checksum, 0u);
+
+  MaybeHandoverSocketAndSendMessage(a, std::move(z));
+}
+
+TEST_P(DcSctpSocketParametrizedTest, AllPacketsAfterConnectHaveZeroChecksum) {
+  DcSctpOptions options = {.enable_zero_checksum = true};
+  SocketUnderTest a("A", options);
+  auto z = std::make_unique<SocketUnderTest>("Z", options);
+
+  ConnectSockets(a, *z);
+  z = MaybeHandoverSocket(std::move(z));
+
+  // Send large messages in both directions, and verify that they arrive and
+  // that every packet has zero checksum.
+  std::vector<uint8_t> payload(kLargeMessageSize);
+  a.socket.Send(DcSctpMessage(StreamID(1), PPID(53), payload), kSendOptions);
+  z->socket.Send(DcSctpMessage(StreamID(1), PPID(53), payload), kSendOptions);
+
+  for (;;) {
+    if (auto data = a.cb.ConsumeSentPacket(); !data.empty()) {
+      ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
+                                  SctpPacket::Parse(data, options));
+      EXPECT_THAT(packet.common_header().checksum, 0u);
+      z->socket.ReceivePacket(std::move(data));
+
+    } else if (auto data = z->cb.ConsumeSentPacket(); !data.empty()) {
+      ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
+                                  SctpPacket::Parse(data, options));
+      EXPECT_THAT(packet.common_header().checksum, 0u);
+      a.socket.ReceivePacket(std::move(data));
+
+    } else {
+      break;
+    }
+  }
+
+  absl::optional<DcSctpMessage> msg1 = z->cb.ConsumeReceivedMessage();
+  ASSERT_TRUE(msg1.has_value());
+  EXPECT_THAT(msg1->payload(), SizeIs(kLargeMessageSize));
+
+  absl::optional<DcSctpMessage> msg2 = a.cb.ConsumeReceivedMessage();
+  ASSERT_TRUE(msg2.has_value());
+  EXPECT_THAT(msg2->payload(), SizeIs(kLargeMessageSize));
+
+  MaybeHandoverSocketAndSendMessage(a, std::move(z));
+}
 }  // namespace
 }  // namespace dcsctp

net/dcsctp/socket/packet_sender.cc

@@ -21,12 +21,12 @@ PacketSender::PacketSender(DcSctpSocketCallbacks& callbacks,
                                               SendPacketStatus)> on_sent_packet)
     : callbacks_(callbacks), on_sent_packet_(std::move(on_sent_packet)) {}
 
-bool PacketSender::Send(SctpPacket::Builder& builder) {
+bool PacketSender::Send(SctpPacket::Builder& builder, bool write_checksum) {
   if (builder.empty()) {
     return false;
   }
 
-  std::vector<uint8_t> payload = builder.Build();
+  std::vector<uint8_t> payload = builder.Build(write_checksum);
 
   SendPacketStatus status = callbacks_.SendPacketWithStatus(payload);
   on_sent_packet_(payload, status);

net/dcsctp/socket/packet_sender.h

@@ -25,7 +25,7 @@ class PacketSender {
                                   SendPacketStatus)> on_sent_packet);
 
   // Sends the packet, and returns true if it was sent successfully.
-  bool Send(SctpPacket::Builder& builder);
+  bool Send(SctpPacket::Builder& builder, bool write_checksum = true);
 
  private:
   DcSctpSocketCallbacks& callbacks_;

net/dcsctp/socket/state_cookie.cc

@@ -42,6 +42,7 @@ std::vector<uint8_t> StateCookie::Serialize() {
   buffer.Store8<30>(capabilities_.reconfig);
   buffer.Store16<32>(capabilities_.negotiated_maximum_incoming_streams);
   buffer.Store16<34>(capabilities_.negotiated_maximum_outgoing_streams);
+  buffer.Store8<36>(capabilities_.zero_checksum);
   return cookie;
 }
 
@@ -74,6 +75,7 @@ absl::optional<StateCookie> StateCookie::Deserialize(
   capabilities.reconfig = buffer.Load8<30>() != 0;
   capabilities.negotiated_maximum_incoming_streams = buffer.Load16<32>();
   capabilities.negotiated_maximum_outgoing_streams = buffer.Load16<34>();
+  capabilities.zero_checksum = buffer.Load8<36>() != 0;
 
   return StateCookie(verification_tag, initial_tsn, a_rwnd, tie_tag,
                      capabilities);

net/dcsctp/socket/state_cookie.h

@@ -27,7 +27,7 @@ namespace dcsctp {
 // Do not trust anything in it; no pointers or anything like that.
 class StateCookie {
  public:
-  static constexpr size_t kCookieSize = 36;
+  static constexpr size_t kCookieSize = 37;
 
   StateCookie(VerificationTag initiate_tag,
               TSN initial_tsn,

net/dcsctp/socket/state_cookie_test.cc

@@ -21,6 +21,7 @@ TEST(StateCookieTest, SerializeAndDeserialize) {
   Capabilities capabilities = {.partial_reliability = true,
                                .message_interleaving = false,
                                .reconfig = true,
+                               .zero_checksum = true,
                                .negotiated_maximum_incoming_streams = 123,
                                .negotiated_maximum_outgoing_streams = 234};
   StateCookie cookie(VerificationTag(123), TSN(456),
@@ -36,6 +37,7 @@ TEST(StateCookieTest, SerializeAndDeserialize) {
   EXPECT_TRUE(deserialized.capabilities().partial_reliability);
   EXPECT_FALSE(deserialized.capabilities().message_interleaving);
   EXPECT_TRUE(deserialized.capabilities().reconfig);
+  EXPECT_TRUE(deserialized.capabilities().zero_checksum);
   EXPECT_EQ(deserialized.capabilities().negotiated_maximum_incoming_streams,
             123);
   EXPECT_EQ(deserialized.capabilities().negotiated_maximum_outgoing_streams,

net/dcsctp/socket/transmission_control_block.cc

@@ -163,7 +163,7 @@ void TransmissionControlBlock::MaybeSendForwardTsn(SctpPacket::Builder& builder,
     } else {
       builder.Add(retransmission_queue_.CreateForwardTsn());
     }
-    packet_sender_.Send(builder);
+    Send(builder);
     // https://datatracker.ietf.org/doc/html/rfc3758
     // "IMPLEMENTATION NOTE: An implementation may wish to limit the number of
     // duplicate FORWARD TSN chunks it sends by ... waiting a full RTT before
@@ -198,7 +198,7 @@ void TransmissionControlBlock::MaybeSendFastRetransmit() {
       builder.Add(DataChunk(tsn, std::move(data), false));
     }
   }
-  packet_sender_.Send(builder);
+  Send(builder);
 }
 
 void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& builder,
@@ -245,7 +245,13 @@ void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& builder,
       }
     }
 
-    if (!packet_sender_.Send(builder)) {
+    // https://www.ietf.org/archive/id/draft-tuexen-tsvwg-sctp-zero-checksum-02.html#section-4.2
+    // "When an end point sends a packet containing a COOKIE ECHO chunk, it MUST
+    // include a correct CRC32c checksum in the packet containing the COOKIE
+    // ECHO chunk."
+    bool write_checksum =
+        !capabilities_.zero_checksum || cookie_echo_chunk_.has_value();
+    if (!packet_sender_.Send(builder, write_checksum)) {
       break;
     }
 
@@ -274,6 +280,9 @@ std::string TransmissionControlBlock::ToString() const {
   if (capabilities_.reconfig) {
     sb << "Reconfig,";
   }
+  if (capabilities_.zero_checksum) {
+    sb << "ZeroChecksum,";
+  }
   sb << " max_in=" << capabilities_.negotiated_maximum_incoming_streams;
   sb << " max_out=" << capabilities_.negotiated_maximum_outgoing_streams;
 
@@ -294,6 +303,7 @@ void TransmissionControlBlock::AddHandoverState(
   state.capabilities.partial_reliability = capabilities_.partial_reliability;
   state.capabilities.message_interleaving = capabilities_.message_interleaving;
   state.capabilities.reconfig = capabilities_.reconfig;
+  state.capabilities.zero_checksum = capabilities_.zero_checksum;
   state.capabilities.negotiated_maximum_incoming_streams =
       capabilities_.negotiated_maximum_incoming_streams;
   state.capabilities.negotiated_maximum_outgoing_streams =

net/dcsctp/socket/transmission_control_block.h

@@ -80,7 +80,8 @@ class TransmissionControlBlock : public Context {
     return tx_error_counter_.IsExhausted();
   }
   void Send(SctpPacket::Builder& builder) override {
-    packet_sender_.Send(builder);
+    packet_sender_.Send(builder,
+                        /*write_checksum=*/!capabilities_.zero_checksum);
   }
 
   // Other accessors

net/dcsctp/socket/transmission_control_block_test.cc

@@ -92,6 +92,7 @@ TEST_F(TransmissionControlBlockTest, LogsAllCapabilitiesInToSring) {
   capabilities_.negotiated_maximum_outgoing_streams = 2000;
   capabilities_.message_interleaving = true;
   capabilities_.partial_reliability = true;
+  capabilities_.zero_checksum = true;
   capabilities_.reconfig = true;
 
   TransmissionControlBlock tcb(
@@ -99,9 +100,10 @@ TEST_F(TransmissionControlBlockTest, LogsAllCapabilitiesInToSring) {
       kMyVerificationTag, kMyInitialTsn, kPeerVerificationTag, kPeerInitialTsn,
       kArwnd, kTieTag, sender_, on_connection_established.AsStdFunction());
 
-  EXPECT_EQ(tcb.ToString(),
+  EXPECT_EQ(
+      tcb.ToString(),
       "verification_tag=000001c8, last_cumulative_ack=999, "
-            "capabilities=PR,IL,Reconfig, max_in=1000 max_out=2000");
+      "capabilities=PR,IL,Reconfig,ZeroChecksum, max_in=1000 max_out=2000");
 }
 
 TEST_F(TransmissionControlBlockTest, IsInitiallyHandoverReady) {