admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Treat non DTLS/SCTP Protocol Based Data Channels as Unsupported Media

Browse Source 
In current state, the SDP parser in webrtc is not backward compatible with clients that might still be using RTP data channels.
Obviously, this isn't there is no such usecase in webrtc since the code is deleted, but in Meta we still use it and would like
to be able to negotiate between clients that offer RTP data channels.
Instead of erroring the parsing procedure, we can parse it as unsupported media in the client that no longer supports RTP data channels.

Replaced the existing test that expects parsing failures with a test that validates that the content was parsed as unsupported media.

Bug: webrtc:14872
Change-Id: I4c105cf55e33b8c19b2849e16148b8175053c40c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291190
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39252}
This commit is contained in:
Dor Hen 2023-02-02 17:59:28 -08:00 committed by WebRTC LUCI CQ
parent f0be3bee1f
commit d1831cb4f8
2 changed files with 57 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
pc/webrtc_sdp.cc
View File

@ -2772,36 +2772,31 @@ bool ParseMediaDescription(
message, cricket::MEDIA_TYPE_AUDIO, mline_index, protocol,
payload_types, pos, &content_name, &bundle_only,
&section_msid_signaling, &transport, candidates, error);
} else if (media_type == kMediaTypeData) {
if (cricket::IsDtlsSctp(protocol)) {
// The draft-03 format is:
// m=application <port> DTLS/SCTP <sctp-port>...
// use_sctpmap should be false.
// The draft-26 format is:
// m=application <port> UDP/DTLS/SCTP webrtc-datachannel
// use_sctpmap should be false.
auto data_desc = std::make_unique<SctpDataContentDescription>();
// Default max message size is 64K
// according to draft-ietf-mmusic-sctp-sdp-26
data_desc->set_max_message_size(kDefaultSctpMaxMessageSize);
int p;
if (rtc::FromString(fields[3], &p)) {
data_desc->set_port(p);
} else if (fields[3] == kDefaultSctpmapProtocol) {
data_desc->set_use_sctpmap(false);
}
if (!ParseContent(message, cricket::MEDIA_TYPE_DATA, mline_index,
protocol, payload_types, pos, &content_name,
&bundle_only, &section_msid_signaling,
data_desc.get(), &transport, candidates, error)) {
return false;
}
data_desc->set_protocol(protocol);
content = std::move(data_desc);
} else {
return ParseFailed(*mline, "Unsupported protocol for media type",
error);
} else if (media_type == kMediaTypeData && cricket::IsDtlsSctp(protocol)) {
// The draft-03 format is:
// m=application <port> DTLS/SCTP <sctp-port>...
// use_sctpmap should be false.
// The draft-26 format is:
// m=application <port> UDP/DTLS/SCTP webrtc-datachannel
// use_sctpmap should be false.
auto data_desc = std::make_unique<SctpDataContentDescription>();
// Default max message size is 64K
// according to draft-ietf-mmusic-sctp-sdp-26
data_desc->set_max_message_size(kDefaultSctpMaxMessageSize);
int p;
if (rtc::FromString(fields[3], &p)) {
data_desc->set_port(p);
} else if (fields[3] == kDefaultSctpmapProtocol) {
data_desc->set_use_sctpmap(false);
}
if (!ParseContent(message, cricket::MEDIA_TYPE_DATA, mline_index,
protocol, payload_types, pos, &content_name,
&bundle_only, &section_msid_signaling, data_desc.get(),
&transport, candidates, error)) {
return false;
}
data_desc->set_protocol(protocol);
content = std::move(data_desc);
} else {
RTC_LOG(LS_WARNING) << "Unsupported media type: " << *mline;
auto unsupported_desc =

50
pc/webrtc_sdp_unittest.cc
View File

@ -2943,21 +2943,6 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithRtpmapAttribute) {
EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithStrangeApplicationProtocolNames) {
static const char* bad_strings[] = {
"DTLS/SCTPRTP/", "obviously-bogus", "UDP/TL/RTSP/SAVPF",
"UDP/TL/RTSP/S", "DTLS/SCTP/RTP/FOO", "obviously-bogus/RTP/"};
for (auto proto : bad_strings) {
std::string sdp_with_data = kSdpString;
sdp_with_data.append("m=application 9 ");
sdp_with_data.append(proto);
sdp_with_data.append(" 47\r\n");
JsepSessionDescription jdesc_output(kDummyType);
EXPECT_FALSE(SdpDeserialize(sdp_with_data, &jdesc_output))
<< "Parsing should have failed on " << proto;
}
}
// For crbug/344475.
TEST_F(WebRtcSdpTest, DeserializeSdpWithCorruptedSctpDataChannels) {
std::string sdp_with_data = kSdpString;
@ -4866,6 +4851,39 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutCname) {
EXPECT_TRUE(CompareSessionDescription(jdesc_, new_jdesc));
}
TEST_F(WebRtcSdpTest,
DeserializeSdpWithUnrecognizedApplicationProtocolRejectsSection) {
const char* unsupported_application_protocols[] = {
"bogus/RTP/", "RTP/SAVPF", "DTLS/SCTP/RTP/", "DTLS/SCTPRTP/",
"obviously-bogus", "UDP/TL/RTSP/SAVPF", "UDP/TL/RTSP/S"};
for (auto proto : unsupported_application_protocols) {
JsepSessionDescription jdesc_output(kDummyType);
std::string sdp = kSdpSessionString;
sdp.append("m=application 9 ");
sdp.append(proto);
sdp.append(" 101\r\n");
EXPECT_TRUE(SdpDeserialize(sdp, &jdesc_output));
// Make sure we actually parsed a single media section
ASSERT_EQ(1u, jdesc_output.description()->contents().size());
// Content is not getting parsed as sctp but instead unsupported.
EXPECT_EQ(nullptr, jdesc_output.description()
->contents()[0]
.media_description()
->as_sctp());
EXPECT_NE(nullptr, jdesc_output.description()
->contents()[0]
.media_description()
->as_unsupported());
// Reject the content
EXPECT_TRUE(jdesc_output.description()->contents()[0].rejected);
}
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithUnsupportedMediaType) {
std::string sdp = kSdpSessionString;
sdp +=
@ -4910,8 +4928,6 @@ TEST_F(WebRtcSdpTest, MediaTypeProtocolMismatch) {
"m=video");
ExpectParseFailure(std::string(sdp + "m=video 9 SOMETHING 120\r\n"),
"m=video");
ExpectParseFailure(std::string(sdp + "m=application 9 SOMETHING 120\r\n"),
"m=application");
}
// Regression test for: