diff --git a/call/rtp_demuxer.cc b/call/rtp_demuxer.cc index 0b74f2ac0a..841d7e3b94 100644 --- a/call/rtp_demuxer.cc +++ b/call/rtp_demuxer.cc @@ -255,6 +255,19 @@ bool RtpDemuxer::RemoveSink(const RtpPacketSinkInterface* sink) { return num_removed > 0; } +flat_set RtpDemuxer::GetSsrcsForSink( + const RtpPacketSinkInterface* sink) const { + flat_set 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) { diff --git a/call/rtp_demuxer.h b/call/rtp_demuxer.h index 53eeb0b6b6..80427b82a7 100644 --- a/call/rtp_demuxer.h +++ b/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 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); diff --git a/experiments/field_trials.py b/experiments/field_trials.py index e39b53eb47..5b2dd7fb34 100755 --- a/experiments/field_trials.py +++ b/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)), diff --git a/pc/rtp_transport.cc b/pc/rtp_transport.cc index 42828a7ab1..2ffb53f477 100644 --- a/pc/rtp_transport.cc +++ b/pc/rtp_transport.cc @@ -180,6 +180,10 @@ bool RtpTransport::UnregisterRtpDemuxerSink(RtpPacketSinkInterface* sink) { return true; } +flat_set RtpTransport::GetSsrcsForSink(RtpPacketSinkInterface* sink) { + return rtp_demuxer_.GetSsrcsForSink(sink); +} + void RtpTransport::DemuxPacket(rtc::CopyOnWriteBuffer packet, int64_t packet_time_us) { webrtc::RtpPacketReceived parsed_packet( diff --git a/pc/rtp_transport.h b/pc/rtp_transport.h index 490a9ef171..6d5d4bff57 100644 --- a/pc/rtp_transport.h +++ b/pc/rtp_transport.h @@ -96,6 +96,7 @@ class RtpTransport : public RtpTransportInternal { rtc::CopyOnWriteBuffer* packet, const rtc::PacketOptions& options, int flags); + flat_set GetSsrcsForSink(RtpPacketSinkInterface* sink); // Overridden by SrtpTransport. virtual void OnNetworkRouteChanged( diff --git a/pc/srtp_session.cc b/pc/srtp_session.cc index 5408d3e0da..f16679cef1 100644 --- a/pc/srtp_session.cc +++ b/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) { diff --git a/pc/srtp_session.h b/pc/srtp_session.h index 60f1860ada..f8fd3e3123 100644 --- a/pc/srtp_session.h +++ b/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, diff --git a/pc/srtp_session_unittest.cc b/pc/srtp_session_unittest.cc index 16a840a307..7adfee86fd 100644 --- a/pc/srtp_session_unittest.cc +++ b/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 diff --git a/pc/srtp_transport.cc b/pc/srtp_transport.cc index cc20216672..033cca75bb 100644 --- a/pc/srtp_transport.cc +++ b/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 diff --git a/pc/srtp_transport.h b/pc/srtp_transport.h index 46c11ed56d..cb443d5f1c 100644 --- a/pc/srtp_transport.h +++ b/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(); diff --git a/pc/srtp_transport_unittest.cc b/pc/srtp_transport_unittest.cc index ac8be8762b..fead5d6a8b 100644 --- a/pc/srtp_transport_unittest.cc +++ b/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(/*rtcp_mux_enabled=*/true, field_trials); + auto rtp_packet_transport = std::make_unique( + "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 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(); + 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