sdp: measure rtp header extension collisions

since extension ids are required to be unique in a BUNDLE group:
  https://www.rfc-editor.org/rfc/rfc8843#name-rtp-header-extensions-consi

Measure how much enforcing this would break in UMA first.

BUG=webrtc:14782

Change-Id: Ieaf7a436feea677032499e11ca14973eebda322e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288362
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38914}
This commit is contained in:
Philipp Hancke 2022-12-16 12:45:57 +01:00 committed by WebRTC LUCI CQ
parent 3ccec2675c
commit 6c27d56a2a
2 changed files with 177 additions and 1 deletions

View File

@ -491,6 +491,52 @@ RTCError ValidateBundledPayloadTypes(
return RTCError::OK();
}
RTCError FindDuplicateHeaderExtensionIds(
const RtpExtension extension,
std::map<int, RtpExtension>& id_to_extension) {
auto existing_extension = id_to_extension.find(extension.id);
if (existing_extension != id_to_extension.end() &&
extension != existing_extension->second) {
return RTCError(
RTCErrorType::INVALID_PARAMETER,
"A BUNDLE group contains a codec collision for "
"header extension id='" +
rtc::ToString(extension.id) +
". The id must be the same across all bundled media descriptions");
}
id_to_extension.insert(std::make_pair(extension.id, extension));
return RTCError::OK();
}
RTCError ValidateBundledRtpHeaderExtensions(
const cricket::SessionDescription& description) {
// https://www.rfc-editor.org/rfc/rfc8843#name-rtp-header-extensions-consi
// ... the identifier used for a given extension MUST identify the same
// extension across all the bundled media descriptions.
std::vector<const cricket::ContentGroup*> bundle_groups =
description.GetGroupsByName(cricket::GROUP_TYPE_BUNDLE);
for (const cricket::ContentGroup* bundle_group : bundle_groups) {
std::map<int, RtpExtension> id_to_extension;
for (const std::string& content_name : bundle_group->content_names()) {
const cricket::MediaContentDescription* media_description =
description.GetContentDescriptionByName(content_name);
if (!media_description) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
"A BUNDLE group contains a MID='" + content_name +
"' matching no m= section.");
}
for (const auto& extension : media_description->rtp_header_extensions()) {
auto error =
FindDuplicateHeaderExtensionIds(extension, id_to_extension);
if (!error.ok()) {
return error;
}
}
}
}
return RTCError::OK();
}
bool IsValidOfferToReceiveMedia(int value) {
typedef PeerConnectionInterface::RTCOfferAnswerOptions Options;
return (value >= Options::kUndefined) &&
@ -3371,12 +3417,18 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription(
return RTCError(RTCErrorType::INVALID_PARAMETER, kSdpWithoutIceUfragPwd);
}
// Validate bundle, payload types and that there are no collisions.
// Validate that there are no collisions of bundled payload types.
error = ValidateBundledPayloadTypes(*sdesc->description());
// TODO(bugs.webrtc.org/14420): actually reject.
RTC_HISTOGRAM_BOOLEAN("WebRTC.PeerConnection.ValidBundledPayloadTypes",
error.ok());
// Validate that there are no collisions of bundled header extensions ids.
error = ValidateBundledRtpHeaderExtensions(*sdesc->description());
// TODO(bugs.webrtc.org/14782): actually reject.
RTC_HISTOGRAM_BOOLEAN("WebRTC.PeerConnection.ValidBundledExtensionIds",
error.ok());
if (!pc_->ValidateBundleSettings(sdesc->description(),
bundle_groups_by_mid)) {
return RTCError(RTCErrorType::INVALID_PARAMETER, kBundleWithoutRtcpMux);

View File

@ -238,6 +238,130 @@ TEST_F(SdpOfferAnswerTest, BundleCodecCollisionInDifferentBundlesAllowed) {
"WebRTC.PeerConnection.ValidBundledPayloadTypes", false));
}
TEST_F(SdpOfferAnswerTest, BundleMeasuresHeaderExtensionIdCollision) {
auto pc = CreatePeerConnection();
std::string sdp =
"v=0\r\n"
"o=- 0 3 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
"a=group:BUNDLE 0 1\r\n"
"a=fingerprint:sha-1 "
"4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
"a=setup:actpass\r\n"
"a=ice-ufrag:ETEn\r\n"
"a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
"m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=rtcp-mux\r\n"
"a=sendonly\r\n"
"a=mid:0\r\n"
"a=rtpmap:111 opus/48000/2\r\n"
"a=extmap:3 "
"http://www.ietf.org/id/"
"draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n"
"m=video 9 UDP/TLS/RTP/SAVPF 112\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=rtcp-mux\r\n"
"a=sendonly\r\n"
"a=mid:1\r\n"
"a=rtpmap:112 VP8/90000\r\n"
"a=extmap:3 "
"http://www.ietf.org/id/"
"draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n";
auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
ASSERT_NE(desc, nullptr);
RTCError error;
pc->SetRemoteDescription(std::move(desc), &error);
EXPECT_TRUE(error.ok());
EXPECT_METRIC_EQ(1,
webrtc::metrics::NumEvents(
"WebRTC.PeerConnection.ValidBundledExtensionIds", true));
}
// extmap:3 is used with two different URIs which is not allowed.
TEST_F(SdpOfferAnswerTest, BundleRejectsHeaderExtensionIdCollision) {
auto pc = CreatePeerConnection();
std::string sdp =
"v=0\r\n"
"o=- 0 3 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
"a=group:BUNDLE 0 1\r\n"
"a=fingerprint:sha-1 "
"4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
"a=setup:actpass\r\n"
"a=ice-ufrag:ETEn\r\n"
"a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
"m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=rtcp-mux\r\n"
"a=sendonly\r\n"
"a=mid:0\r\n"
"a=rtpmap:111 opus/48000/2\r\n"
"a=extmap:3 "
"http://www.ietf.org/id/"
"draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n"
"m=video 9 UDP/TLS/RTP/SAVPF 112\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=rtcp-mux\r\n"
"a=sendonly\r\n"
"a=mid:1\r\n"
"a=rtpmap:112 VP8/90000\r\n"
"a=extmap:3 urn:3gpp:video-orientation\r\n";
auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
ASSERT_NE(desc, nullptr);
RTCError error;
pc->SetRemoteDescription(std::move(desc), &error);
EXPECT_TRUE(error.ok());
EXPECT_METRIC_EQ(
1, webrtc::metrics::NumEvents(
"WebRTC.PeerConnection.ValidBundledExtensionIds", false));
}
// transport-wide cc is negotiated with two different ids 3 and 4.
// This is not a good idea but tolerable.
TEST_F(SdpOfferAnswerTest, BundleAcceptsDifferentIdsForSameExtension) {
auto pc = CreatePeerConnection();
std::string sdp =
"v=0\r\n"
"o=- 0 3 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
"a=group:BUNDLE 0 1\r\n"
"a=fingerprint:sha-1 "
"4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
"a=setup:actpass\r\n"
"a=ice-ufrag:ETEn\r\n"
"a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
"m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=rtcp-mux\r\n"
"a=sendonly\r\n"
"a=mid:0\r\n"
"a=rtpmap:111 opus/48000/2\r\n"
"a=extmap:3 "
"http://www.ietf.org/id/"
"draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n"
"m=video 9 UDP/TLS/RTP/SAVPF 112\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=rtcp-mux\r\n"
"a=sendonly\r\n"
"a=mid:1\r\n"
"a=rtpmap:112 VP8/90000\r\n"
"a=extmap:4 "
"http://www.ietf.org/id/"
"draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n";
auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
ASSERT_NE(desc, nullptr);
RTCError error;
pc->SetRemoteDescription(std::move(desc), &error);
EXPECT_TRUE(error.ok());
EXPECT_METRIC_EQ(1,
webrtc::metrics::NumEvents(
"WebRTC.PeerConnection.ValidBundledExtensionIds", true));
}
TEST_F(SdpOfferAnswerTest, LargeMidsAreRejected) {
auto pc = CreatePeerConnection();
std::string sdp =