diff --git a/net/dcsctp/public/dcsctp_handover_state.h b/net/dcsctp/public/dcsctp_handover_state.h index f277ebc02c..253f4da939 100644 --- a/net/dcsctp/public/dcsctp_handover_state.h +++ b/net/dcsctp/public/dcsctp_handover_state.h @@ -40,7 +40,6 @@ 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 35dc9c56e4..2df6a2c009 100644 --- a/net/dcsctp/public/dcsctp_socket.h +++ b/net/dcsctp/public/dcsctp_socket.h @@ -245,10 +245,6 @@ 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 286509a40a..fa3be37d12 100644 --- a/net/dcsctp/socket/capabilities.h +++ b/net/dcsctp/socket/capabilities.h @@ -21,8 +21,6 @@ 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 1dc4ae1bc7..139ec28055 100644 --- a/net/dcsctp/socket/dcsctp_socket.cc +++ b/net/dcsctp/socket/dcsctp_socket.cc @@ -56,7 +56,6 @@ #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" @@ -118,11 +117,6 @@ 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( @@ -143,9 +137,6 @@ 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))); } @@ -288,10 +279,7 @@ void DcSctpSocket::SendInit() { connect_params_.initial_tsn, params_builder.Build()); SctpPacket::Builder b(VerificationTag(0), options_); b.Add(init); - // 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); + packet_sender_.Send(b); } void DcSctpSocket::MakeConnectionParameters() { @@ -332,7 +320,6 @@ 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 = @@ -364,7 +351,6 @@ 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 = @@ -1223,9 +1209,7 @@ void DcSctpSocket::HandleInit(const CommonHeader& header, options_.announced_maximum_incoming_streams, connect_params_.initial_tsn, params_builder.Build()); b.Add(init_ack); - // 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); + packet_sender_.Send(b); } void DcSctpSocket::HandleInitAck( diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc index c1901b202d..0da893d9bd 100644 --- a/net/dcsctp/socket/dcsctp_socket_test.cc +++ b/net/dcsctp/socket/dcsctp_socket_test.cc @@ -23,7 +23,6 @@ #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" @@ -31,7 +30,6 @@ #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" @@ -60,10 +58,8 @@ 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; @@ -2868,156 +2864,5 @@ 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 f0134eea9b..85392e205d 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 write_checksum) { +bool PacketSender::Send(SctpPacket::Builder& builder) { if (builder.empty()) { return false; } - std::vector payload = builder.Build(write_checksum); + std::vector payload = builder.Build(); 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 395c2efcba..7af4d3c47b 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 write_checksum = true); + bool Send(SctpPacket::Builder& builder); private: DcSctpSocketCallbacks& callbacks_; diff --git a/net/dcsctp/socket/state_cookie.cc b/net/dcsctp/socket/state_cookie.cc index 624d783a3b..86be77aa34 100644 --- a/net/dcsctp/socket/state_cookie.cc +++ b/net/dcsctp/socket/state_cookie.cc @@ -42,7 +42,6 @@ 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; } @@ -75,7 +74,6 @@ 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 34cd6d3690..a26dbf86f7 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 = 37; + static constexpr size_t kCookieSize = 36; 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 19be71a1ca..7d8e1339ee 100644 --- a/net/dcsctp/socket/state_cookie_test.cc +++ b/net/dcsctp/socket/state_cookie_test.cc @@ -21,7 +21,6 @@ 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), @@ -37,7 +36,6 @@ 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 8bb1e8b2fb..1dcf394813 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()); } - Send(builder); + packet_sender_.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)); } } - Send(builder); + packet_sender_.Send(builder); } void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& builder, @@ -245,13 +245,7 @@ void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& 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)) { + if (!packet_sender_.Send(builder)) { break; } @@ -280,9 +274,6 @@ 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; @@ -303,7 +294,6 @@ 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 46a39d5a7b..fc66fcc857 100644 --- a/net/dcsctp/socket/transmission_control_block.h +++ b/net/dcsctp/socket/transmission_control_block.h @@ -80,8 +80,7 @@ class TransmissionControlBlock : public Context { return tx_error_counter_.IsExhausted(); } void Send(SctpPacket::Builder& builder) override { - packet_sender_.Send(builder, - /*write_checksum=*/!capabilities_.zero_checksum); + packet_sender_.Send(builder); } // Other accessors diff --git a/net/dcsctp/socket/transmission_control_block_test.cc b/net/dcsctp/socket/transmission_control_block_test.cc index 6106fbb309..40aea58d4b 100644 --- a/net/dcsctp/socket/transmission_control_block_test.cc +++ b/net/dcsctp/socket/transmission_control_block_test.cc @@ -92,7 +92,6 @@ 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( @@ -100,10 +99,9 @@ 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,ZeroChecksum, max_in=1000 max_out=2000"); + EXPECT_EQ(tcb.ToString(), + "verification_tag=000001c8, last_cumulative_ack=999, " + "capabilities=PR,IL,Reconfig, max_in=1000 max_out=2000"); } TEST_F(TransmissionControlBlockTest, IsInitiallyHandoverReady) {