Remove SSRCs from libSRTP when removing them from the rtp_demuxer

This uses libSRTPs srtp_remove_stream()
  https://github.com/cisco/libsrtp/blob/main/include/srtp.h#L597
method to remove SSRCs from the libSRTP session when they are removed
from the RTP demuxer. This works even when the stream was added
automatically via the ssrc_any_inbound mechanism.

Only streams for inbound SSRCs that were added explicitly via SDP negotiation are removed.

Guarded by WebRTC-SrtpRemoveReceiveStream field trial.

BUG=webrtc:15604

Change-Id: I655bde5f8ddf26ac91395ef54bd1b3c598813380
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324720
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#41105}

call/rtp_demuxer.cc

@@ -255,6 +255,19 @@ bool RtpDemuxer::RemoveSink(const RtpPacketSinkInterface* sink) {
   return num_removed > 0;
 }
 
+flat_set<uint32_t> RtpDemuxer::GetSsrcsForSink(
+    const RtpPacketSinkInterface* sink) const {
+  flat_set<uint32_t> ssrcs;
+  if (sink) {
+    for (const auto& it : sink_by_ssrc_) {
+      if (it.second == sink) {
+        ssrcs.insert(it.first);
+      }
+    }
+  }
+  return ssrcs;
+}
+
 bool RtpDemuxer::OnRtpPacket(const RtpPacketReceived& packet) {
   RtpPacketSinkInterface* sink = ResolveSink(packet);
   if (sink != nullptr) {

call/rtp_demuxer.h

@@ -151,6 +151,9 @@ class RtpDemuxer {
   // Null pointer is not allowed.
   bool RemoveSink(const RtpPacketSinkInterface* sink);
 
+  // Returns the set of SSRCs associated with a sink.
+  flat_set<uint32_t> GetSsrcsForSink(const RtpPacketSinkInterface* sink) const;
+
   // Demuxes the given packet and forwards it to the chosen sink. Returns true
   // if the packet was forwarded and false if the packet was dropped.
   bool OnRtpPacket(const RtpPacketReceived& packet);

experiments/field_trials.py

@@ -107,6 +107,9 @@ ACTIVE_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([
     FieldTrial('WebRTC-SendPacketsOnWorkerThread',
                'webrtc:14502',
                date(2024, 4, 1)),
+    FieldTrial('WebRTC-SrtpRemoveReceiveStream',
+               'webrtc:15604',
+               date(2024, 10, 1)),
     FieldTrial('WebRTC-Stats-RtxReceiveStats',
                'webrtc:15096',
                date(2024, 4, 1)),

pc/rtp_transport.cc

@@ -180,6 +180,10 @@ bool RtpTransport::UnregisterRtpDemuxerSink(RtpPacketSinkInterface* sink) {
   return true;
 }
 
+flat_set<uint32_t> RtpTransport::GetSsrcsForSink(RtpPacketSinkInterface* sink) {
+  return rtp_demuxer_.GetSsrcsForSink(sink);
+}
+
 void RtpTransport::DemuxPacket(rtc::CopyOnWriteBuffer packet,
                                int64_t packet_time_us) {
   webrtc::RtpPacketReceived parsed_packet(

pc/rtp_transport.h

@@ -96,6 +96,7 @@ class RtpTransport : public RtpTransportInternal {
                   rtc::CopyOnWriteBuffer* packet,
                   const rtc::PacketOptions& options,
                   int flags);
+  flat_set<uint32_t> GetSsrcsForSink(RtpPacketSinkInterface* sink);
 
   // Overridden by SrtpTransport.
   virtual void OnNetworkRouteChanged(

pc/srtp_session.cc

@@ -332,6 +332,12 @@ bool SrtpSession::IsExternalAuthActive() const {
   return external_auth_active_;
 }
 
+bool SrtpSession::RemoveSsrcFromSession(uint32_t ssrc) {
+  RTC_DCHECK(session_);
+  // libSRTP expects the SSRC to be in network byte order.
+  return srtp_remove_stream(session_, htonl(ssrc)) == srtp_err_status_ok;
+}
+
 bool SrtpSession::GetSendStreamPacketIndex(void* p,
                                            int in_len,
                                            int64_t* index) {

pc/srtp_session.h

@@ -97,6 +97,14 @@ class SrtpSession {
   // been set.
   bool IsExternalAuthActive() const;
 
+  // Removes a SSRC from the underlying libSRTP session.
+  // Note: this should only be done for SSRCs that are received.
+  // Removing SSRCs that were sent and then reusing them leads to
+  // cryptographic weaknesses described in
+  // https://www.rfc-editor.org/rfc/rfc3711#section-8
+  // https://www.rfc-editor.org/rfc/rfc7714#section-8.4
+  bool RemoveSsrcFromSession(uint32_t ssrc);
+
  private:
   bool DoSetKey(int type,
                 int crypto_suite,

pc/srtp_session_unittest.cc

@@ -251,4 +251,36 @@ TEST_F(SrtpSessionTest, TestReplay) {
       s1_.ProtectRtp(rtp_packet_, rtp_len_, sizeof(rtp_packet_), &out_len));
 }
 
+TEST_F(SrtpSessionTest, RemoveSsrc) {
+  EXPECT_TRUE(s1_.SetSend(kSrtpAes128CmSha1_80, kTestKey1, kTestKeyLen,
+                          kEncryptedHeaderExtensionIds));
+  EXPECT_TRUE(s2_.SetRecv(kSrtpAes128CmSha1_80, kTestKey1, kTestKeyLen,
+                          kEncryptedHeaderExtensionIds));
+  int out_len;
+  // Encrypt and decrypt the packet once.
+  EXPECT_TRUE(
+      s1_.ProtectRtp(rtp_packet_, rtp_len_, sizeof(rtp_packet_), &out_len));
+  EXPECT_TRUE(s2_.UnprotectRtp(rtp_packet_, out_len, &out_len));
+  EXPECT_EQ(rtp_len_, out_len);
+  EXPECT_EQ(0, memcmp(rtp_packet_, kPcmuFrame, out_len));
+
+  // Recreate the original packet and encrypt again.
+  memcpy(rtp_packet_, kPcmuFrame, rtp_len_);
+  EXPECT_TRUE(
+      s1_.ProtectRtp(rtp_packet_, rtp_len_, sizeof(rtp_packet_), &out_len));
+  // Attempting to decrypt will fail as a replay attack.
+  // (srtp_err_status_replay_fail) since the sequence number was already seen.
+  EXPECT_FALSE(s2_.UnprotectRtp(rtp_packet_, out_len, &out_len));
+
+  // Remove the fake packet SSRC 1 from the session.
+  EXPECT_TRUE(s2_.RemoveSsrcFromSession(1));
+  EXPECT_FALSE(s2_.RemoveSsrcFromSession(1));
+
+  // Since the SRTP state was discarded, this is no longer a replay attack.
+  EXPECT_TRUE(s2_.UnprotectRtp(rtp_packet_, out_len, &out_len));
+  EXPECT_EQ(rtp_len_, out_len);
+  EXPECT_EQ(0, memcmp(rtp_packet_, kPcmuFrame, out_len));
+  EXPECT_TRUE(s2_.RemoveSsrcFromSession(1));
+}
+
 }  // namespace rtc

pc/srtp_transport.cc

@@ -519,4 +519,19 @@ void SrtpTransport::MaybeUpdateWritableState() {
   }
 }
 
+bool SrtpTransport::UnregisterRtpDemuxerSink(RtpPacketSinkInterface* sink) {
+  if (recv_session_ &&
+      field_trials_.IsEnabled("WebRTC-SrtpRemoveReceiveStream")) {
+    // Remove the SSRCs explicitly registered with the demuxer
+    // (via SDP negotiation) from the SRTP session.
+    for (const auto ssrc : GetSsrcsForSink(sink)) {
+      if (!recv_session_->RemoveSsrcFromSession(ssrc)) {
+        RTC_LOG(LS_WARNING)
+            << "Could not remove SSRC " << ssrc << " from SRTP session.";
+      }
+    }
+  }
+  return RtpTransport::UnregisterRtpDemuxerSink(sink);
+}
+
 }  // namespace webrtc

pc/srtp_transport.h

@@ -109,6 +109,10 @@ class SrtpTransport : public RtpTransport {
     rtp_abs_sendtime_extn_id_ = rtp_abs_sendtime_extn_id;
   }
 
+  // In addition to unregistering the sink, the SRTP transport
+  // disassociates all SSRCs of the sink from libSRTP.
+  bool UnregisterRtpDemuxerSink(RtpPacketSinkInterface* sink) override;
+
  protected:
   // If the writable state changed, fire the SignalWritableState.
   void MaybeUpdateWritableState();

pc/srtp_transport_unittest.cc

@@ -425,4 +425,66 @@ TEST_F(SrtpTransportTest, TestSetParamsKeyTooShort) {
       rtc::kSrtpAes128CmSha1_80, kTestKey1, kTestKeyLen - 1, extension_ids));
 }
 
+TEST_F(SrtpTransportTest, RemoveSrtpReceiveStream) {
+  test::ScopedKeyValueConfig field_trials(
+      "WebRTC-SrtpRemoveReceiveStream/Enabled/");
+  auto srtp_transport =
+      std::make_unique<SrtpTransport>(/*rtcp_mux_enabled=*/true, field_trials);
+  auto rtp_packet_transport = std::make_unique<rtc::FakePacketTransport>(
+      "fake_packet_transport_loopback");
+
+  bool asymmetric = false;
+  rtp_packet_transport->SetDestination(rtp_packet_transport.get(), asymmetric);
+  srtp_transport->SetRtpPacketTransport(rtp_packet_transport.get());
+
+  TransportObserver rtp_sink;
+
+  std::vector<int> extension_ids;
+  EXPECT_TRUE(srtp_transport->SetRtpParams(
+      rtc::kSrtpAeadAes128Gcm, kTestKeyGcm128_1, kTestKeyGcm128Len,
+      extension_ids, rtc::kSrtpAeadAes128Gcm, kTestKeyGcm128_1,
+      kTestKeyGcm128Len, extension_ids));
+
+  RtpDemuxerCriteria demuxer_criteria;
+  uint32_t ssrc = 0x1;  // SSRC of kPcmuFrame
+  demuxer_criteria.ssrcs().insert(ssrc);
+  EXPECT_TRUE(
+      srtp_transport->RegisterRtpDemuxerSink(demuxer_criteria, &rtp_sink));
+
+  // Create a packet and try to send it three times.
+  size_t rtp_len = sizeof(kPcmuFrame);
+  size_t packet_size = rtp_len + rtc::rtp_auth_tag_len(rtc::kCsAeadAes128Gcm);
+  rtc::Buffer rtp_packet_buffer(packet_size);
+  char* rtp_packet_data = rtp_packet_buffer.data<char>();
+  memcpy(rtp_packet_data, kPcmuFrame, rtp_len);
+
+  // First attempt will succeed.
+  rtc::CopyOnWriteBuffer first_try(rtp_packet_data, rtp_len, packet_size);
+  EXPECT_TRUE(srtp_transport->SendRtpPacket(&first_try, rtc::PacketOptions(),
+                                            cricket::PF_SRTP_BYPASS));
+  EXPECT_EQ(rtp_sink.rtp_count(), 1);
+
+  // Second attempt will be rejected by libSRTP as a replay attack
+  // (srtp_err_status_replay_fail) since the sequence number was already seen.
+  // Hence the packet never reaches the sink.
+  rtc::CopyOnWriteBuffer second_try(rtp_packet_data, rtp_len, packet_size);
+  EXPECT_TRUE(srtp_transport->SendRtpPacket(&second_try, rtc::PacketOptions(),
+                                            cricket::PF_SRTP_BYPASS));
+  EXPECT_EQ(rtp_sink.rtp_count(), 1);
+
+  // Reset the sink.
+  EXPECT_TRUE(srtp_transport->UnregisterRtpDemuxerSink(&rtp_sink));
+  EXPECT_TRUE(
+      srtp_transport->RegisterRtpDemuxerSink(demuxer_criteria, &rtp_sink));
+
+  // Third attempt will succeed again since libSRTP does not remember seeing
+  // the sequence number after the reset.
+  rtc::CopyOnWriteBuffer third_try(rtp_packet_data, rtp_len, packet_size);
+  EXPECT_TRUE(srtp_transport->SendRtpPacket(&third_try, rtc::PacketOptions(),
+                                            cricket::PF_SRTP_BYPASS));
+  EXPECT_EQ(rtp_sink.rtp_count(), 2);
+  // Clear the sink to clean up.
+  srtp_transport->UnregisterRtpDemuxerSink(&rtp_sink);
+}
+
 }  // namespace webrtc