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}
This commit is contained in:
Harald Alvestrand
2019-05-14 17:27:11 +02:00
committed by Commit Bot
parent 83afeebe94
commit c3f4820e12
3 changed files with 57 additions and 53 deletions

View File

@ -2235,10 +2235,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);
if (!CreateContentOffer(media_description_options, session_options,

View File

@ -915,6 +915,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.

View File

@ -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());
@ -2819,18 +2819,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));
@ -2846,19 +2846,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));
}
@ -2873,19 +2860,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));
}
@ -2913,19 +2887,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));
}
@ -4528,3 +4489,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));
}
}