diff options
author | Guido Urdaneta <guidou@webrtc.org> | 2019-05-31 10:17:38 +0000 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2019-05-31 11:08:03 +0000 |
commit | cecf87ff29bf70477e74f5b50e1a14794153a0a8 (patch) | |
tree | 1620bc0371604795cba81311a642873c1cdfd619 | |
parent | 4436887ed2d3324279e0f2e091c9e9355392721a (diff) | |
download | webrtc-cecf87ff29bf70477e74f5b50e1a14794153a0a8.tar.gz |
Reland "Change default secure SCTP protocol to UDP/DTLS/SCTP"
This reverts commit 4436887ed2d3324279e0f2e091c9e9355392721a.
Reason for revert: The original revert was intended to be temporary.
Original change's description:
> 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}
TBR=steveanton@webrtc.org,mbonadei@webrtc.org,hbos@webrtc.org,hta@webrtc.org,guidou@webrtc.org
Change-Id: I381fa18b644874c20ddaa4cd13fec79a5fd9555a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:7706
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139246
Reviewed-by: Guido Urdaneta <guidou@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Guido Urdaneta <guidou@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28122}
-rw-r--r-- | pc/media_session.cc | 5 | ||||
-rw-r--r-- | pc/media_session_unittest.cc | 20 | ||||
-rw-r--r-- | pc/peer_connection_data_channel_unittest.cc | 2 | ||||
-rw-r--r-- | pc/webrtc_sdp_unittest.cc | 85 |
4 files changed, 58 insertions, 54 deletions
diff --git a/pc/media_session.cc b/pc/media_session.cc index de96930a0a..1b3d015a79 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -2236,10 +2236,7 @@ bool MediaSessionDescriptionFactory::AddSctpDataContentForOffer( // before we call CreateMediaContentOffer. Otherwise, // CreateMediaContentOffer won't know this is SCTP and will // generate SSRCs rather than SIDs. - // 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 + data->set_protocol(secure_transport ? kMediaProtocolUdpDtlsSctp : 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 17cc2a758e..b14e14c936 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -916,6 +916,26 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSctpDataOffer) { 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 97305edf66..435d1eb5dd 100644 --- a/pc/peer_connection_data_channel_unittest.cc +++ b/pc/peer_connection_data_channel_unittest.cc @@ -414,7 +414,7 @@ TEST_P(PeerConnectionDataChannelTest, MediaTransportWithoutSdesFails) { EXPECT_EQ(nullptr, caller); } -TEST_P(PeerConnectionDataChannelTest, DISABLED_ModernSdpSyntaxByDefault) { +TEST_P(PeerConnectionDataChannelTest, 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 cae06e8f4d..9a99b6dcc7 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -278,7 +278,7 @@ static const char kSdpRtpDataChannelString[] = // draft-ietf-mmusic-sctp-sdp-03 static const char kSdpSctpDataChannelString[] = - "m=application 9 DTLS/SCTP 5000\r\n" + "m=application 9 UDP/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 @@ static const char kSdpSctpDataChannelString[] = // 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 DTLS/SCTP webrtc-datachannel\r\n" + "m=application 9 UDP/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 @@ static const char kSdpSctpDataChannelStringWithSctpPort[] = // draft-ietf-mmusic-sctp-sdp-26 static const char kSdpSctpDataChannelStringWithSctpColonPort[] = - "m=application 9 DTLS/SCTP webrtc-datachannel\r\n" + "m=application 9 UDP/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 @@ static const char kSdpSctpDataChannelStringWithSctpColonPort[] = "a=mid:data_content_name\r\n"; static const char kSdpSctpDataChannelWithCandidatesString[] = - "m=application 2345 DTLS/SCTP 5000\r\n" + "m=application 2345 UDP/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 @@ class WebRtcSdpTest : public ::testing::Test { new SctpDataContentDescription()); sctp_desc_ = data.get(); sctp_desc_->set_use_sctpmap(use_sctpmap); - sctp_desc_->set_protocol(cricket::kMediaProtocolDtlsSctp); + sctp_desc_->set_protocol(cricket::kMediaProtocolUdpDtlsSctp); sctp_desc_->set_port(kDefaultSctpPort); desc_.AddContent(kDataContentName, MediaProtocolType::kSctp, data.release()); @@ -2822,18 +2822,18 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannels) { sdp_with_data.append(kSdpSctpDataChannelString); JsepSessionDescription jdesc_output(kDummyType); - // Verify with DTLS/SCTP (already in kSdpSctpDataChannelString). + // Verify with UDP/DTLS/SCTP (already in kSdpSctpDataChannelString). 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); + // Verify with DTLS/SCTP. + sdp_with_data.replace(sdp_with_data.find(kUdpDtlsSctp), strlen(kUdpDtlsSctp), + kDtlsSctp); 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), + sdp_with_data.replace(sdp_with_data.find(kDtlsSctp), strlen(kDtlsSctp), kTcpDtlsSctp); EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output)); EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output)); @@ -2849,19 +2849,6 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpPort) { 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)); } @@ -2876,19 +2863,6 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpColonPort) { 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)); } @@ -2916,19 +2890,6 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithMaxMessageSize) { 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)); } @@ -4559,3 +4520,29 @@ TEST_F(WebRtcSdpTest, SerializeMediaTransportSettingsTestCopy) { 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)); + } +} |