diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index 464e22c3f5..407f33b237 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -1254,6 +1254,14 @@ rtc::scoped_refptr CreateReportFilteredBySelector( } // namespace +RTCStatsCollector::CertificateStatsPair +RTCStatsCollector::CertificateStatsPair::Copy() const { + CertificateStatsPair copy; + copy.local = local ? local->Copy() : nullptr; + copy.remote = remote ? remote->Copy() : nullptr; + return copy; +} + RTCStatsCollector::RequestInfo::RequestInfo( rtc::scoped_refptr callback) : RequestInfo(FilterMode::kAll, std::move(callback), nullptr, nullptr) {} @@ -1386,6 +1394,7 @@ void RTCStatsCollector::GetStatsReportInternal( void RTCStatsCollector::ClearCachedStatsReport() { RTC_DCHECK_RUN_ON(signaling_thread_); cached_report_ = nullptr; + cached_certificates_by_transport_.clear(); } void RTCStatsCollector::WaitForPendingRequest() { @@ -2309,29 +2318,44 @@ void RTCStatsCollector::ProduceTransportStats_n( std::map RTCStatsCollector::PrepareTransportCertificateStats_n( const std::map& - transport_stats_by_name) const { + transport_stats_by_name) { RTC_DCHECK_RUN_ON(network_thread_); rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; std::map transport_cert_stats; - for (const auto& entry : transport_stats_by_name) { - const std::string& transport_name = entry.first; - - CertificateStatsPair certificate_stats_pair; - rtc::scoped_refptr local_certificate; - if (pc_->GetLocalCertificate(transport_name, &local_certificate)) { - certificate_stats_pair.local = - local_certificate->GetSSLCertificateChain().GetStats(); + if (!cached_certificates_by_transport_.empty()) { + // Copy the certificate info from the cache, avoiding expensive + // rtc::SSLCertChain::GetStats() calls. + for (const auto& pair : cached_certificates_by_transport_) { + transport_cert_stats.insert( + std::make_pair(pair.first, pair.second.Copy())); } + } else { + // Collect certificate info. + for (const auto& entry : transport_stats_by_name) { + const std::string& transport_name = entry.first; - std::unique_ptr remote_cert_chain = - pc_->GetRemoteSSLCertChain(transport_name); - if (remote_cert_chain) { - certificate_stats_pair.remote = remote_cert_chain->GetStats(); + CertificateStatsPair certificate_stats_pair; + rtc::scoped_refptr local_certificate; + if (pc_->GetLocalCertificate(transport_name, &local_certificate)) { + certificate_stats_pair.local = + local_certificate->GetSSLCertificateChain().GetStats(); + } + + std::unique_ptr remote_cert_chain = + pc_->GetRemoteSSLCertChain(transport_name); + if (remote_cert_chain) { + certificate_stats_pair.remote = remote_cert_chain->GetStats(); + } + + transport_cert_stats.insert( + std::make_pair(transport_name, std::move(certificate_stats_pair))); + } + // Copy the result into the certificate cache for future reference. + for (const auto& pair : transport_cert_stats) { + cached_certificates_by_transport_.insert( + std::make_pair(pair.first, pair.second.Copy())); } - - transport_cert_stats.insert( - std::make_pair(transport_name, std::move(certificate_stats_pair))); } return transport_cert_stats; } diff --git a/pc/rtc_stats_collector.h b/pc/rtc_stats_collector.h index b1909cb4c4..e5c6aedd88 100644 --- a/pc/rtc_stats_collector.h +++ b/pc/rtc_stats_collector.h @@ -79,7 +79,11 @@ class RTCStatsCollector : public rtc::RefCountInterface, void GetStatsReport(rtc::scoped_refptr selector, rtc::scoped_refptr callback); // Clears the cache's reference to the most recent stats report. Subsequently - // calling `GetStatsReport` guarantees fresh stats. + // calling `GetStatsReport` guarantees fresh stats. This method must be called + // any time the PeerConnection visibly changes as a result of an API call as + // per + // https://w3c.github.io/webrtc-stats/#guidelines-for-getstats-results-caching-throttling + // and it must be called any time negotiation happens. void ClearCachedStatsReport(); // If there is a `GetStatsReport` requests in-flight, waits until it has been @@ -93,6 +97,8 @@ class RTCStatsCollector : public rtc::RefCountInterface, struct CertificateStatsPair { std::unique_ptr local; std::unique_ptr remote; + + CertificateStatsPair Copy() const; }; // Stats gathering on a particular thread. Virtual for the sake of testing. @@ -227,7 +233,7 @@ class RTCStatsCollector : public rtc::RefCountInterface, std::map PrepareTransportCertificateStats_n( const std::map& - transport_stats_by_name) const; + transport_stats_by_name); // The results are stored in `transceiver_stats_infos_` and `call_stats_`. void PrepareTransceiverStatsInfosAndCallStats_s_w_n(); @@ -279,6 +285,10 @@ class RTCStatsCollector : public rtc::RefCountInterface, // now get rid of the variable and keep the data scoped within a stats // collection sequence. std::vector transceiver_stats_infos_; + // This cache avoids having to call rtc::SSLCertChain::GetStats(), which can + // relatively expensive. ClearCachedStatsReport() needs to be called on + // negotiation to ensure the cache is not obsolete. + std::map cached_certificates_by_transport_; Call::Stats call_stats_; diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index 9d7ae249c1..88bb6aefa5 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -677,6 +677,18 @@ class RTCStatsCollectorTest : public ::testing::Test { } } + const RTCCertificateStats* GetCertificateStatsFromFingerprint( + const rtc::scoped_refptr& report, + const std::string& fingerprint) { + auto certificates = report->GetStatsOfType(); + for (const auto* certificate : certificates) { + if (*certificate->fingerprint == fingerprint) { + return certificate; + } + } + return nullptr; + } + struct ExampleStatsGraph { rtc::scoped_refptr sender; rtc::scoped_refptr receiver; @@ -1360,6 +1372,124 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsChain) { ExpectReportContainsCertificateInfo(report, *remote_certinfo); } +TEST_F(RTCStatsCollectorTest, CertificateStatsCache) { + const char kTransportName[] = "transport"; + rtc::ScopedFakeClock fake_clock; + + pc_->AddVoiceChannel("audio", kTransportName); + + // Set local and remote cerificates. + std::unique_ptr initial_local_certinfo = + CreateFakeCertificateAndInfoFromDers({"LocalCertA", "LocalCertB"}); + pc_->SetLocalCertificate(kTransportName, initial_local_certinfo->certificate); + std::unique_ptr initial_remote_certinfo = + CreateFakeCertificateAndInfoFromDers({"RemoteCertA", "RemoteCertB"}); + pc_->SetRemoteCertChain( + kTransportName, + initial_remote_certinfo->certificate->GetSSLCertificateChain().Clone()); + ASSERT_EQ(initial_local_certinfo->fingerprints.size(), 2u); + ASSERT_EQ(initial_remote_certinfo->fingerprints.size(), 2u); + + rtc::scoped_refptr first_report = + stats_->GetStatsReport(); + const auto* first_local_cert0 = GetCertificateStatsFromFingerprint( + first_report, initial_local_certinfo->fingerprints[0]); + const auto* first_local_cert1 = GetCertificateStatsFromFingerprint( + first_report, initial_local_certinfo->fingerprints[1]); + const auto* first_remote_cert0 = GetCertificateStatsFromFingerprint( + first_report, initial_remote_certinfo->fingerprints[0]); + const auto* first_remote_cert1 = GetCertificateStatsFromFingerprint( + first_report, initial_remote_certinfo->fingerprints[1]); + ASSERT_TRUE(first_local_cert0); + ASSERT_TRUE(first_local_cert1); + ASSERT_TRUE(first_remote_cert0); + ASSERT_TRUE(first_remote_cert1); + EXPECT_EQ(first_local_cert0->timestamp_us(), rtc::TimeMicros()); + EXPECT_EQ(first_local_cert1->timestamp_us(), rtc::TimeMicros()); + EXPECT_EQ(first_remote_cert0->timestamp_us(), rtc::TimeMicros()); + EXPECT_EQ(first_remote_cert1->timestamp_us(), rtc::TimeMicros()); + + // Replace all certificates. + std::unique_ptr updated_local_certinfo = + CreateFakeCertificateAndInfoFromDers( + {"UpdatedLocalCertA", "UpdatedLocalCertB"}); + pc_->SetLocalCertificate(kTransportName, updated_local_certinfo->certificate); + std::unique_ptr updated_remote_certinfo = + CreateFakeCertificateAndInfoFromDers( + {"UpdatedRemoteCertA", "UpdatedRemoteCertB"}); + pc_->SetRemoteCertChain( + kTransportName, + updated_remote_certinfo->certificate->GetSSLCertificateChain().Clone()); + // This test assumes fingerprints are different for the old and new + // certificates. + EXPECT_NE(initial_local_certinfo->fingerprints, + updated_local_certinfo->fingerprints); + EXPECT_NE(initial_remote_certinfo->fingerprints, + updated_remote_certinfo->fingerprints); + + // Advance time to ensure a fresh stats report, but don't clear the + // certificate stats cache. + fake_clock.AdvanceTime(TimeDelta::Seconds(1)); + rtc::scoped_refptr second_report = + stats_->GetStatsReport(); + // We expect to see the same certificates as before due to not clearing the + // certificate cache. + const auto* second_local_cert0 = + second_report->GetAs(first_local_cert0->id()); + const auto* second_local_cert1 = + second_report->GetAs(first_local_cert1->id()); + const auto* second_remote_cert0 = + second_report->GetAs(first_remote_cert0->id()); + const auto* second_remote_cert1 = + second_report->GetAs(first_remote_cert1->id()); + ASSERT_TRUE(second_local_cert0); + ASSERT_TRUE(second_local_cert1); + ASSERT_TRUE(second_remote_cert0); + ASSERT_TRUE(second_remote_cert1); + // The information in the certificate stats are obsolete. + EXPECT_EQ(*second_local_cert0->fingerprint, + initial_local_certinfo->fingerprints[0]); + EXPECT_EQ(*second_local_cert1->fingerprint, + initial_local_certinfo->fingerprints[1]); + EXPECT_EQ(*second_remote_cert0->fingerprint, + initial_remote_certinfo->fingerprints[0]); + EXPECT_EQ(*second_remote_cert1->fingerprint, + initial_remote_certinfo->fingerprints[1]); + // But timestamps are up-to-date, because this is a fresh stats report. + EXPECT_EQ(second_local_cert0->timestamp_us(), rtc::TimeMicros()); + EXPECT_EQ(second_local_cert1->timestamp_us(), rtc::TimeMicros()); + EXPECT_EQ(second_remote_cert0->timestamp_us(), rtc::TimeMicros()); + EXPECT_EQ(second_remote_cert1->timestamp_us(), rtc::TimeMicros()); + // The updated certificates are not part of the report yet. + EXPECT_FALSE(GetCertificateStatsFromFingerprint( + second_report, updated_local_certinfo->fingerprints[0])); + EXPECT_FALSE(GetCertificateStatsFromFingerprint( + second_report, updated_local_certinfo->fingerprints[1])); + EXPECT_FALSE(GetCertificateStatsFromFingerprint( + second_report, updated_remote_certinfo->fingerprints[0])); + EXPECT_FALSE(GetCertificateStatsFromFingerprint( + second_report, updated_remote_certinfo->fingerprints[1])); + + // Clear the cache, including the cached certificates. + stats_->stats_collector()->ClearCachedStatsReport(); + rtc::scoped_refptr third_report = + stats_->GetStatsReport(); + // Now the old certificates stats should be deleted. + EXPECT_FALSE(third_report->Get(first_local_cert0->id())); + EXPECT_FALSE(third_report->Get(first_local_cert1->id())); + EXPECT_FALSE(third_report->Get(first_remote_cert0->id())); + EXPECT_FALSE(third_report->Get(first_remote_cert1->id())); + // And updated certificates exist. + EXPECT_TRUE(GetCertificateStatsFromFingerprint( + third_report, updated_local_certinfo->fingerprints[0])); + EXPECT_TRUE(GetCertificateStatsFromFingerprint( + third_report, updated_local_certinfo->fingerprints[1])); + EXPECT_TRUE(GetCertificateStatsFromFingerprint( + third_report, updated_remote_certinfo->fingerprints[0])); + EXPECT_TRUE(GetCertificateStatsFromFingerprint( + third_report, updated_remote_certinfo->fingerprints[1])); +} + TEST_F(RTCStatsCollectorTest, CollectTwoRTCDataChannelStatsWithPendingId) { pc_->AddSctpDataChannel(rtc::make_ref_counted( /*id=*/-1, DataChannelInterface::kConnecting)); diff --git a/rtc_base/ssl_certificate.cc b/rtc_base/ssl_certificate.cc index ddb1524f76..d1fd57fca5 100644 --- a/rtc_base/ssl_certificate.cc +++ b/rtc_base/ssl_certificate.cc @@ -44,6 +44,12 @@ SSLCertificateStats::SSLCertificateStats( SSLCertificateStats::~SSLCertificateStats() {} +std::unique_ptr SSLCertificateStats::Copy() const { + return std::make_unique( + std::string(fingerprint), std::string(fingerprint_algorithm), + std::string(base64_certificate), issuer ? issuer->Copy() : nullptr); +} + ////////////////////////////////////////////////////////////////////// // SSLCertificate ////////////////////////////////////////////////////////////////////// diff --git a/rtc_base/ssl_certificate.h b/rtc_base/ssl_certificate.h index 77fbba3e9e..2e198800c4 100644 --- a/rtc_base/ssl_certificate.h +++ b/rtc_base/ssl_certificate.h @@ -38,6 +38,8 @@ struct RTC_EXPORT SSLCertificateStats { std::string fingerprint_algorithm; std::string base64_certificate; std::unique_ptr issuer; + + std::unique_ptr Copy() const; }; // Abstract interface overridden by SSL library specific