Revert "Change default secure SCTP protocol to UDP/DTLS/SCTP"
This reverts commit c3f4820e129d44471b366b8885a67b5392918d5a.
Reason for revert: Will temporarily revert to fix an issue and reland afterwards.
Original change's description:
> Change default secure SCTP protocol to UDP/DTLS/SCTP
>
> The old value - DTLS/SCTP - is not standards conformant,
> and the new value should be parsable since Chrome M61.
>
> Bug: webrtc:7706
> Change-Id: I7468cc9597dec4ef4b102fccddc4e981fed7e8d8
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/136804
> Reviewed-by: Steve Anton <steveanton@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#27940}
TBR=steveanton@webrtc.org,mbonadei@webrtc.org,hbos@webrtc.org,hta@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:7706
Change-Id: Ida8ae20767485c75edc44dff8a3fa1af2006f207
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139244
Reviewed-by: Guido Urdaneta <guidou@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Guido Urdaneta <guidou@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28121}
diff --git a/pc/media_session.cc b/pc/media_session.cc
index 1b3d015..de96930 100644
--- a/pc/media_session.cc
+++ b/pc/media_session.cc
@@ -2236,7 +2236,10 @@
// before we call CreateMediaContentOffer. Otherwise,
// CreateMediaContentOffer won't know this is SCTP and will
// generate SSRCs rather than SIDs.
- data->set_protocol(secure_transport ? kMediaProtocolUdpDtlsSctp
+ // TODO(deadbeef): Offer kMediaProtocolUdpDtlsSctp (or TcpDtlsSctp), once
+ // it's safe to do so. Older versions of webrtc would reject these
+ // protocols; see https://bugs.chromium.org/p/webrtc/issues/detail?id=7706.
+ data->set_protocol(secure_transport ? kMediaProtocolDtlsSctp
: kMediaProtocolSctp);
data->set_use_sctpmap(session_options.use_obsolete_sctp_sdp);
data->set_max_message_size(kSctpSendBufferSize);
diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc
index b14e14c..17cc2a7 100644
--- a/pc/media_session_unittest.cc
+++ b/pc/media_session_unittest.cc
@@ -916,26 +916,6 @@
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, NULL);
EXPECT_TRUE(offer.get() != NULL);
EXPECT_TRUE(offer->GetContentByName("data") != NULL);
- auto dcd = GetFirstSctpDataContentDescription(offer.get());
- ASSERT_TRUE(dcd);
- // Since this transport is insecure, the protocol should be "SCTP".
- EXPECT_EQ(cricket::kMediaProtocolSctp, dcd->protocol());
-}
-
-// Create an SCTP data offer with bundle without error.
-TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSecureSctpDataOffer) {
- MediaSessionOptions opts;
- opts.bundle_enabled = true;
- AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
- f1_.set_secure(SEC_ENABLED);
- tdf1_.set_secure(SEC_ENABLED);
- std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, NULL);
- EXPECT_TRUE(offer.get() != NULL);
- EXPECT_TRUE(offer->GetContentByName("data") != NULL);
- auto dcd = GetFirstSctpDataContentDescription(offer.get());
- ASSERT_TRUE(dcd);
- // The protocol should now be "UDP/DTLS/SCTP"
- EXPECT_EQ(cricket::kMediaProtocolUdpDtlsSctp, dcd->protocol());
}
// Test creating an sctp data channel from an already generated offer.
diff --git a/pc/peer_connection_data_channel_unittest.cc b/pc/peer_connection_data_channel_unittest.cc
index 435d1eb..97305ed 100644
--- a/pc/peer_connection_data_channel_unittest.cc
+++ b/pc/peer_connection_data_channel_unittest.cc
@@ -414,7 +414,7 @@
EXPECT_EQ(nullptr, caller);
}
-TEST_P(PeerConnectionDataChannelTest, ModernSdpSyntaxByDefault) {
+TEST_P(PeerConnectionDataChannelTest, DISABLED_ModernSdpSyntaxByDefault) {
PeerConnectionInterface::RTCOfferAnswerOptions options;
auto caller = CreatePeerConnectionWithDataChannel();
auto offer = caller->CreateOffer(options);
diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index 9a99b6d..cae06e8 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -278,7 +278,7 @@
// draft-ietf-mmusic-sctp-sdp-03
static const char kSdpSctpDataChannelString[] =
- "m=application 9 UDP/DTLS/SCTP 5000\r\n"
+ "m=application 9 DTLS/SCTP 5000\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=ice-ufrag:ufrag_data\r\n"
"a=ice-pwd:pwd_data\r\n"
@@ -289,7 +289,7 @@
// Note - this is invalid per draft-ietf-mmusic-sctp-sdp-26,
// since the separator after "sctp-port" needs to be a colon.
static const char kSdpSctpDataChannelStringWithSctpPort[] =
- "m=application 9 UDP/DTLS/SCTP webrtc-datachannel\r\n"
+ "m=application 9 DTLS/SCTP webrtc-datachannel\r\n"
"a=sctp-port 5000\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=ice-ufrag:ufrag_data\r\n"
@@ -298,7 +298,7 @@
// draft-ietf-mmusic-sctp-sdp-26
static const char kSdpSctpDataChannelStringWithSctpColonPort[] =
- "m=application 9 UDP/DTLS/SCTP webrtc-datachannel\r\n"
+ "m=application 9 DTLS/SCTP webrtc-datachannel\r\n"
"a=sctp-port:5000\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=ice-ufrag:ufrag_data\r\n"
@@ -306,7 +306,7 @@
"a=mid:data_content_name\r\n";
static const char kSdpSctpDataChannelWithCandidatesString[] =
- "m=application 2345 UDP/DTLS/SCTP 5000\r\n"
+ "m=application 2345 DTLS/SCTP 5000\r\n"
"c=IN IP4 74.125.127.126\r\n"
"a=candidate:a0+B/1 1 udp 2130706432 192.168.1.5 1234 typ host "
"generation 2\r\n"
@@ -1784,7 +1784,7 @@
new SctpDataContentDescription());
sctp_desc_ = data.get();
sctp_desc_->set_use_sctpmap(use_sctpmap);
- sctp_desc_->set_protocol(cricket::kMediaProtocolUdpDtlsSctp);
+ sctp_desc_->set_protocol(cricket::kMediaProtocolDtlsSctp);
sctp_desc_->set_port(kDefaultSctpPort);
desc_.AddContent(kDataContentName, MediaProtocolType::kSctp,
data.release());
@@ -2822,18 +2822,18 @@
sdp_with_data.append(kSdpSctpDataChannelString);
JsepSessionDescription jdesc_output(kDummyType);
- // Verify with UDP/DTLS/SCTP (already in kSdpSctpDataChannelString).
+ // Verify with DTLS/SCTP (already in kSdpSctpDataChannelString).
EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
- // Verify with DTLS/SCTP.
- sdp_with_data.replace(sdp_with_data.find(kUdpDtlsSctp), strlen(kUdpDtlsSctp),
- kDtlsSctp);
+ // Verify with UDP/DTLS/SCTP.
+ sdp_with_data.replace(sdp_with_data.find(kDtlsSctp), strlen(kDtlsSctp),
+ kUdpDtlsSctp);
EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
// Verify with TCP/DTLS/SCTP.
- sdp_with_data.replace(sdp_with_data.find(kDtlsSctp), strlen(kDtlsSctp),
+ sdp_with_data.replace(sdp_with_data.find(kUdpDtlsSctp), strlen(kUdpDtlsSctp),
kTcpDtlsSctp);
EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
@@ -2849,6 +2849,19 @@
sdp_with_data.append(kSdpSctpDataChannelStringWithSctpPort);
JsepSessionDescription jdesc_output(kDummyType);
+ // Verify with DTLS/SCTP (already in kSdpSctpDataChannelStringWithSctpPort).
+ EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
+ EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
+
+ // Verify with UDP/DTLS/SCTP.
+ sdp_with_data.replace(sdp_with_data.find(kDtlsSctp), strlen(kDtlsSctp),
+ kUdpDtlsSctp);
+ EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
+ EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
+
+ // Verify with TCP/DTLS/SCTP.
+ sdp_with_data.replace(sdp_with_data.find(kUdpDtlsSctp), strlen(kUdpDtlsSctp),
+ kTcpDtlsSctp);
EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
}
@@ -2863,6 +2876,19 @@
sdp_with_data.append(kSdpSctpDataChannelStringWithSctpColonPort);
JsepSessionDescription jdesc_output(kDummyType);
+ // Verify with DTLS/SCTP.
+ EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
+ EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
+
+ // Verify with UDP/DTLS/SCTP.
+ sdp_with_data.replace(sdp_with_data.find(kDtlsSctp), strlen(kDtlsSctp),
+ kUdpDtlsSctp);
+ EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
+ EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
+
+ // Verify with TCP/DTLS/SCTP.
+ sdp_with_data.replace(sdp_with_data.find(kUdpDtlsSctp), strlen(kUdpDtlsSctp),
+ kTcpDtlsSctp);
EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
}
@@ -2890,6 +2916,19 @@
MutateJsepSctpMaxMessageSize(desc_, 12345, &jdesc);
JsepSessionDescription jdesc_output(kDummyType);
+ // Verify with DTLS/SCTP.
+ EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
+ EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
+
+ // Verify with UDP/DTLS/SCTP.
+ sdp_with_data.replace(sdp_with_data.find(kDtlsSctp), strlen(kDtlsSctp),
+ kUdpDtlsSctp);
+ EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
+ EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
+
+ // Verify with TCP/DTLS/SCTP.
+ sdp_with_data.replace(sdp_with_data.find(kUdpDtlsSctp), strlen(kUdpDtlsSctp),
+ kTcpDtlsSctp);
EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
}
@@ -4520,29 +4559,3 @@
EXPECT_EQ("name", copy->MediaTransportSettings()[0].transport_name);
EXPECT_EQ("setting", copy->MediaTransportSettings()[0].transport_setting);
}
-
-TEST_F(WebRtcSdpTest, SerializeWithDefaultSctpProtocol) {
- AddSctpDataChannel(false); // Don't use sctpmap
- JsepSessionDescription jsep_desc(kDummyType);
- MakeDescriptionWithoutCandidates(&jsep_desc);
- std::string message = webrtc::SdpSerialize(jsep_desc);
- EXPECT_NE(std::string::npos,
- message.find(cricket::kMediaProtocolUdpDtlsSctp));
-}
-
-TEST_F(WebRtcSdpTest, DeserializeWithAllSctpProtocols) {
- AddSctpDataChannel(false);
- std::string protocols[] = {cricket::kMediaProtocolDtlsSctp,
- cricket::kMediaProtocolUdpDtlsSctp,
- cricket::kMediaProtocolTcpDtlsSctp};
- for (const auto& protocol : protocols) {
- sctp_desc_->set_protocol(protocol);
- JsepSessionDescription jsep_desc(kDummyType);
- MakeDescriptionWithoutCandidates(&jsep_desc);
- std::string message = webrtc::SdpSerialize(jsep_desc);
- EXPECT_NE(std::string::npos, message.find(protocol));
- JsepSessionDescription jsep_output(kDummyType);
- SdpParseError error;
- EXPECT_TRUE(webrtc::SdpDeserialize(message, &jsep_output, &error));
- }
-}
