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}
diff --git a/net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter.cc b/net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter.cc
index 75f7d3c..a846d6d 100644
--- a/net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter.cc
+++ b/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
diff --git a/net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter.h b/net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter.h
index 9ae2ec8..18c98c9 100644
--- a/net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter.h
+++ b/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 @@
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
diff --git a/net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter_test.cc b/net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter_test.cc
index 8a004e1..861fa4d 100644
--- a/net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter_test.cc
+++ b/net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter_test.cc
@@ -24,18 +24,28 @@
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, HasToString) {
- ZeroChecksumAcceptableChunkParameter parameter;
+ ZeroChecksumAcceptableChunkParameter parameter(
+ ZeroChecksumAlternateErrorDetectionMethod::LowerLayerDtls());
- EXPECT_EQ(parameter.ToString(), "Zero Checksum Acceptable");
+ EXPECT_EQ(parameter.ToString(), "Zero Checksum Acceptable (1)");
}
} // namespace
diff --git a/net/dcsctp/packet/sctp_packet.cc b/net/dcsctp/packet/sctp_packet.cc
index de407bb..978181f 100644
--- a/net/dcsctp/packet/sctp_packet.cc
+++ b/net/dcsctp/packet/sctp_packet.cc
@@ -126,7 +126,9 @@
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
diff --git a/net/dcsctp/packet/sctp_packet_test.cc b/net/dcsctp/packet/sctp_packet_test.cc
index fcdd161..aab01df 100644
--- a/net/dcsctp/packet/sctp_packet_test.cc
+++ b/net/dcsctp/packet/sctp_packet_test.cc
@@ -38,7 +38,8 @@
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 @@
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 @@
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 @@
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) {
diff --git a/net/dcsctp/public/dcsctp_options.h b/net/dcsctp/public/dcsctp_options.h
index d197980..600e8a3 100644
--- a/net/dcsctp/public/dcsctp_options.h
+++ b/net/dcsctp/public/dcsctp_options.h
@@ -196,14 +196,13 @@
// 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
diff --git a/net/dcsctp/public/types.h b/net/dcsctp/public/types.h
index 02e2ce1..1565cd7 100644
--- a/net/dcsctp/public/types.h
+++ b/net/dcsctp/public/types.h
@@ -151,6 +151,27 @@
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_
diff --git a/net/dcsctp/socket/capabilities.h b/net/dcsctp/socket/capabilities.h
index 286509a..9b1bff0 100644
--- a/net/dcsctp/socket/capabilities.h
+++ b/net/dcsctp/socket/capabilities.h
@@ -21,7 +21,7 @@
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;
diff --git a/net/dcsctp/socket/dcsctp_socket.cc b/net/dcsctp/socket/dcsctp_socket.cc
index 98cd34a..0667e6f 100644
--- a/net/dcsctp/socket/dcsctp_socket.cc
+++ b/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.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 @@
}
void AddCapabilityParameters(const DcSctpOptions& options,
+ bool support_zero_checksum,
Parameters::Builder& builder) {
std::vector<uint8_t> chunk_types = {ReConfigChunk::kType};
@@ -145,8 +149,11 @@
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::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 @@
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,
diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc
index 413516b..dfe8ba6 100644
--- a/net/dcsctp/socket/dcsctp_socket_test.cc
+++ b/net/dcsctp/socket/dcsctp_socket_test.cc
@@ -2887,8 +2887,16 @@
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(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, 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, 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, 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_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, 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);
diff --git a/net/dcsctp/socket/heartbeat_handler_test.cc b/net/dcsctp/socket/heartbeat_handler_test.cc
index 4475527..b7f3a60 100644
--- a/net/dcsctp/socket/heartbeat_handler_test.cc
+++ b/net/dcsctp/socket/heartbeat_handler_test.cc
@@ -38,7 +38,8 @@
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;
}
