Prevent SDP munging of duplicate SSRCs

BUG=chromium:1459124

Change-Id: Ifa901955b79dc9ff40d198bc367e89a8a535c3e2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/311802
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40447}

pc/peer_connection_bundle_unittest.cc

@@ -730,16 +730,17 @@ TEST_P(PeerConnectionBundleTest, BundleOnFirstMidInAnswer) {
 }
 
 // This tests that applying description with conflicted RTP demuxing criteria
-// will fail.
-TEST_P(PeerConnectionBundleTest,
-       ApplyDescriptionWithConflictedDemuxCriteriaFail) {
+// will fail when using BUNDLE.
+TEST_P(PeerConnectionBundleTest, ApplyDescriptionWithSameSsrcsBundledFails) {
   auto caller = CreatePeerConnectionWithAudioVideo();
   auto callee = CreatePeerConnectionWithAudioVideo();
 
   RTCOfferAnswerOptions options;
-  options.use_rtp_mux = false;
+  options.use_rtp_mux = true;
   auto offer = caller->CreateOffer(options);
-  // Modified the SDP to make two m= sections have the same SSRC.
+  EXPECT_TRUE(
+      caller->SetLocalDescription(CloneSessionDescription(offer.get())));
+  // Modify the remote SDP to make two m= sections have the same SSRC.
   ASSERT_GE(offer->description()->contents().size(), 2U);
   offer->description()
       ->contents()[0]
@@ -751,20 +752,42 @@ TEST_P(PeerConnectionBundleTest,
       .media_description()
       ->mutable_streams()[0]
       .ssrcs[0] = 1111222;
+  EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer)));
+
+  // When BUNDLE is enabled, applying the description is expected to fail
+  // because the demuxing criteria can not be satisfied.
+  auto answer = callee->CreateAnswer(options);
+  EXPECT_FALSE(callee->SetLocalDescription(std::move(answer)));
+}
+
+// A variant of the above, without BUNDLE duplicate SSRCs are allowed.
+TEST_P(PeerConnectionBundleTest,
+       ApplyDescriptionWithSameSsrcsUnbundledSucceeds) {
+  auto caller = CreatePeerConnectionWithAudioVideo();
+  auto callee = CreatePeerConnectionWithAudioVideo();
+
+  RTCOfferAnswerOptions options;
+  options.use_rtp_mux = false;
+  auto offer = caller->CreateOffer(options);
   EXPECT_TRUE(
       caller->SetLocalDescription(CloneSessionDescription(offer.get())));
+  // Modify the remote SDP to make two m= sections have the same SSRC.
+  ASSERT_GE(offer->description()->contents().size(), 2U);
+  offer->description()
+      ->contents()[0]
+      .media_description()
+      ->mutable_streams()[0]
+      .ssrcs[0] = 1111222;
+  offer->description()
+      ->contents()[1]
+      .media_description()
+      ->mutable_streams()[0]
+      .ssrcs[0] = 1111222;
   EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer)));
-  EXPECT_TRUE(callee->CreateAnswerAndSetAsLocal(options));
 
-  // Enable BUNDLE in subsequent offer/answer exchange and two m= sections are
-  // expectd to use one RtpTransport underneath.
-  options.use_rtp_mux = true;
-  EXPECT_TRUE(
-      callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(options)));
+  // Without BUNDLE, demuxing is done per-transport.
   auto answer = callee->CreateAnswer(options);
-  // When BUNDLE is enabled, applying the description is expected to fail
-  // because the demuxing criteria is conflicted.
-  EXPECT_FALSE(callee->SetLocalDescription(std::move(answer)));
+  EXPECT_TRUE(callee->SetLocalDescription(std::move(answer)));
 }
 
 // This tests that changing the pre-negotiated BUNDLE tag is not supported.

pc/sdp_offer_answer.cc

@@ -1721,7 +1721,7 @@ RTCError SdpOfferAnswerHandler::ApplyLocalDescription(
     if (type == SdpType::kOffer) {
       // TODO(bugs.webrtc.org/4676) - Handle CreateChannel failure, as new local
       // description is applied. Restore back to old description.
-      RTCError error = CreateChannels(*local_description()->description());
+      error = CreateChannels(*local_description()->description());
       if (!error.ok()) {
         RTC_LOG(LS_ERROR) << error.message() << " (" << SdpTypeToString(type)
                           << ")";
@@ -1753,6 +1753,23 @@ RTCError SdpOfferAnswerHandler::ApplyLocalDescription(
   // SCTP sids.
   AllocateSctpSids();
 
+  // Validate SSRCs, we do not allow duplicates.
+  if (ConfiguredForMedia()) {
+    std::set<uint32_t> used_ssrcs;
+    for (const auto& content : local_description()->description()->contents()) {
+      for (const auto& stream : content.media_description()->streams()) {
+        for (uint32_t ssrc : stream.ssrcs) {
+          auto result = used_ssrcs.insert(ssrc);
+          if (!result.second) {
+            LOG_AND_RETURN_ERROR(
+                RTCErrorType::INVALID_PARAMETER,
+                "Duplicate ssrc " + rtc::ToString(ssrc) + " is not allowed");
+          }
+        }
+      }
+    }
+  }
+
   if (IsUnifiedPlan()) {
     if (ConfiguredForMedia()) {
       // We must use List and not ListInternal here because

pc/sdp_offer_answer_unittest.cc

@@ -12,6 +12,7 @@
 #include <utility>
 #include <vector>
 
+#include "absl/strings/str_replace.h"
 #include "api/audio/audio_mixer.h"
 #include "api/audio_codecs/builtin_audio_decoder_factory.h"
 #include "api/audio_codecs/builtin_audio_encoder_factory.h"
@@ -659,4 +660,21 @@ TEST_F(SdpOfferAnswerTest, ExpectAllSsrcsSpecifiedInSsrcGroupFecFr) {
   EXPECT_FALSE(pc->SetRemoteDescription(std::move(offer)));
 }
 
+TEST_F(SdpOfferAnswerTest, DuplicateSsrcsDisallowedInLocalDescription) {
+  auto pc = CreatePeerConnection();
+  pc->AddAudioTrack("audio_track", {});
+  pc->AddVideoTrack("video_track", {});
+  auto offer = pc->CreateOffer();
+  auto& offer_contents = offer->description()->contents();
+  ASSERT_EQ(offer_contents.size(), 2u);
+  uint32_t second_ssrc = offer_contents[1].media_description()->first_ssrc();
+
+  offer->description()
+      ->contents()[0]
+      .media_description()
+      ->mutable_streams()[0]
+      .ssrcs[0] = second_ssrc;
+  EXPECT_FALSE(pc->SetLocalDescription(std::move(offer)));
+}
+
 }  // namespace webrtc