commit 5edfd244fed9a339f6279cdccb4d57d0c6749b21
author Philipp Hancke <phancke@meta.com> Sun May 04 19:12:31 2025
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon May 05 17:33:08 2025
tree 1504f2fcb0cacef599c16c10899982f4f6edf7c2
parent 3d09240cd078eb2ab45dccc1553c84ad5ba59a7b

sdp munging: forbid removing a content with SDP munging

This is now disabled by default, guarded by
  WebRTC-NoSdpMangleNumberOfContents
which can be disabled if necessary.

BUG=webrtc:40567530

Change-Id: I02a3d4d21678f41e0910144be1aa159828c40757
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/387560
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#44517}

experiments/field_trials.py

diff --git a/experiments/field_trials.py b/experiments/field_trials.py
index dff2fe0..bf6b833 100755
--- a/experiments/field_trials.py
+++ b/experiments/field_trials.py
@@ -122,6 +122,9 @@
     FieldTrial('WebRTC-NoSdpMangleUfragRestrictedAddresses',
                409713509,
                date(2025, 10, 11)),
+    FieldTrial('WebRTC-NoSdpMangleNumberOfContents',
+               40567530,
+               date(2025, 10, 11)),
     FieldTrial('WebRTC-Pacer-FastRetransmissions',
                40235589,
                date(2024, 4, 1)),

pc/sdp_offer_answer.cc

diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 31808c5..6f19d1b 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -2473,11 +2473,20 @@
   SdpMungingType sdp_munging_type =
       DetermineSdpMungingType(desc.get(), last_created_desc);
 
-  if (!disable_sdp_munging_checks_ &&
-      HasUfragSdpMunging(desc.get(), last_created_desc)) {
-    has_sdp_munged_ufrag_ = true;
-    if (pc_->trials().IsEnabled("WebRTC-NoSdpMangleUfrag")) {
-      RTC_LOG(LS_ERROR) << "Rejecting SDP because of ufrag modification";
+  if (!disable_sdp_munging_checks_) {
+    bool reject_error = false;
+    if (HasUfragSdpMunging(desc.get(), last_created_desc)) {
+      has_sdp_munged_ufrag_ = true;
+      if (pc_->trials().IsEnabled("WebRTC-NoSdpMangleUfrag")) {
+        RTC_LOG(LS_ERROR) << "Rejecting SDP because of ufrag modification";
+        reject_error = true;
+      }
+    } else if (sdp_munging_type == kNumberOfContents &&
+               !pc_->trials().IsDisabled(
+                   "WebRTC-NoSdpMangleNumberOfContents")) {
+      reject_error = true;
+    }
+    if (reject_error) {
       observer->OnSetLocalDescriptionComplete(
           RTCError(RTCErrorType::INVALID_MODIFICATION,
                    "SDP is modified in a non-acceptable way"));

pc/sdp_offer_answer_unittest.cc

diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index c47f086..3fe6e19 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -2089,7 +2089,7 @@
       ElementsAre(Pair(SdpMungingType::kDtlsSetup, 1)));
 }
 
-TEST_F(SdpOfferAnswerMungingTest, RemoveContent) {
+TEST_F(SdpOfferAnswerMungingTest, RemoveContentDefault) {
   auto pc = CreatePeerConnection();
   pc->AddAudioTrack("audio_track", {});
 
@@ -2105,6 +2105,29 @@
       absl::StrReplaceAll(sdp, {{"a=group:BUNDLE " + name, "a=group:BUNDLE"}}));
 
   RTCError error;
+  EXPECT_FALSE(pc->SetLocalDescription(std::move(modified_offer), &error));
+  EXPECT_THAT(
+      metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),
+      ElementsAre(Pair(SdpMungingType::kNumberOfContents, 1)));
+}
+
+TEST_F(SdpOfferAnswerMungingTest, RemoveContentKillswitch) {
+  auto pc = CreatePeerConnection(FieldTrials::CreateNoGlobal(
+      "WebRTC-NoSdpMangleNumberOfContents/Disabled/"));
+  pc->AddAudioTrack("audio_track", {});
+
+  auto offer = pc->CreateOffer();
+  auto& contents = offer->description()->contents();
+  ASSERT_EQ(contents.size(), 1u);
+  auto name = contents[0].mid();
+  EXPECT_TRUE(offer->description()->RemoveContentByName(contents[0].mid()));
+  std::string sdp;
+  offer->ToString(&sdp);
+  auto modified_offer = CreateSessionDescription(
+      SdpType::kOffer,
+      absl::StrReplaceAll(sdp, {{"a=group:BUNDLE " + name, "a=group:BUNDLE"}}));
+
+  RTCError error;
   EXPECT_TRUE(pc->SetLocalDescription(std::move(modified_offer), &error));
   EXPECT_THAT(
       metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),