diff --git a/net/dcsctp/public/dcsctp_handover_state.h b/net/dcsctp/public/dcsctp_handover_state.h index 253f4da939..f277ebc02c 100644 --- a/net/dcsctp/public/dcsctp_handover_state.h +++ b/net/dcsctp/public/dcsctp_handover_state.h @@ -40,6 +40,7 @@ struct DcSctpSocketHandoverState { bool partial_reliability = false; bool message_interleaving = false; bool reconfig = false; + bool zero_checksum = false; uint16_t negotiated_maximum_incoming_streams = 0; uint16_t negotiated_maximum_outgoing_streams = 0; }; diff --git a/net/dcsctp/public/dcsctp_socket.h b/net/dcsctp/public/dcsctp_socket.h index 2df6a2c009..35dc9c56e4 100644 --- a/net/dcsctp/public/dcsctp_socket.h +++ b/net/dcsctp/public/dcsctp_socket.h @@ -245,6 +245,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 diff --git a/net/dcsctp/socket/capabilities.h b/net/dcsctp/socket/capabilities.h index fa3be37d12..286509a40a 100644 --- a/net/dcsctp/socket/capabilities.h +++ b/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; diff --git a/net/dcsctp/socket/dcsctp_socket.cc b/net/dcsctp/socket/dcsctp_socket.cc index 139ec28055..1dc4ae1bc7 100644 --- a/net/dcsctp/socket/dcsctp_socket.cc +++ b/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().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 = @@ -1209,7 +1223,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( diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc index 0da893d9bd..c1901b202d 100644 --- a/net/dcsctp/socket/dcsctp_socket_test.cc +++ b/net/dcsctp/socket/dcsctp_socket_test.cc @@ -23,6 +23,7 @@ #include "api/array_view.h" #include "net/dcsctp/common/handover_testing.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" @@ -30,6 +31,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" @@ -58,8 +60,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; @@ -2864,5 +2868,156 @@ TEST(DcSctpSocketTest, ResetStreamsWithPausedSenderResumesWhenPerformed) { EXPECT_EQ(msg2->payload().size(), kSmallMessageSize); } +TEST_P(DcSctpSocketParametrizedTest, ZeroChecksumMetricsAreSet) { + std::vector> 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("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 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 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 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 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("Z", options); + + ConnectSockets(a, *z); + z = MaybeHandoverSocket(std::move(z)); + + std::vector payload(a.options.mtu - 100); + a.socket.Send(DcSctpMessage(StreamID(1), PPID(53), payload), {}); + + std::vector 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("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 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 msg1 = z->cb.ConsumeReceivedMessage(); + ASSERT_TRUE(msg1.has_value()); + EXPECT_THAT(msg1->payload(), SizeIs(kLargeMessageSize)); + + absl::optional msg2 = a.cb.ConsumeReceivedMessage(); + ASSERT_TRUE(msg2.has_value()); + EXPECT_THAT(msg2->payload(), SizeIs(kLargeMessageSize)); + + MaybeHandoverSocketAndSendMessage(a, std::move(z)); +} } // namespace } // namespace dcsctp diff --git a/net/dcsctp/socket/packet_sender.cc b/net/dcsctp/socket/packet_sender.cc index 85392e205d..f0134eea9b 100644 --- a/net/dcsctp/socket/packet_sender.cc +++ b/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 payload = builder.Build(); + std::vector payload = builder.Build(write_checksum); SendPacketStatus status = callbacks_.SendPacketWithStatus(payload); on_sent_packet_(payload, status); diff --git a/net/dcsctp/socket/packet_sender.h b/net/dcsctp/socket/packet_sender.h index 7af4d3c47b..395c2efcba 100644 --- a/net/dcsctp/socket/packet_sender.h +++ b/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_; diff --git a/net/dcsctp/socket/state_cookie.cc b/net/dcsctp/socket/state_cookie.cc index 86be77aa34..624d783a3b 100644 --- a/net/dcsctp/socket/state_cookie.cc +++ b/net/dcsctp/socket/state_cookie.cc @@ -42,6 +42,7 @@ std::vector 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::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); diff --git a/net/dcsctp/socket/state_cookie.h b/net/dcsctp/socket/state_cookie.h index a26dbf86f7..34cd6d3690 100644 --- a/net/dcsctp/socket/state_cookie.h +++ b/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, diff --git a/net/dcsctp/socket/state_cookie_test.cc b/net/dcsctp/socket/state_cookie_test.cc index 7d8e1339ee..19be71a1ca 100644 --- a/net/dcsctp/socket/state_cookie_test.cc +++ b/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, diff --git a/net/dcsctp/socket/transmission_control_block.cc b/net/dcsctp/socket/transmission_control_block.cc index 1dcf394813..8bb1e8b2fb 100644 --- a/net/dcsctp/socket/transmission_control_block.cc +++ b/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 = diff --git a/net/dcsctp/socket/transmission_control_block.h b/net/dcsctp/socket/transmission_control_block.h index fc66fcc857..46a39d5a7b 100644 --- a/net/dcsctp/socket/transmission_control_block.h +++ b/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 diff --git a/net/dcsctp/socket/transmission_control_block_test.cc b/net/dcsctp/socket/transmission_control_block_test.cc index 40aea58d4b..6106fbb309 100644 --- a/net/dcsctp/socket/transmission_control_block_test.cc +++ b/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(), - "verification_tag=000001c8, last_cumulative_ack=999, " - "capabilities=PR,IL,Reconfig, max_in=1000 max_out=2000"); + EXPECT_EQ( + tcb.ToString(), + "verification_tag=000001c8, last_cumulative_ack=999, " + "capabilities=PR,IL,Reconfig,ZeroChecksum, max_in=1000 max_out=2000"); } TEST_F(TransmissionControlBlockTest, IsInitiallyHandoverReady) {