Add RTCCertificateStats cache to avoid rtc::SSLCertChain::GetStats.
Unlike the cache of the entire stats report which is time limited, this
certificate cache is valid for an unlimited amount of time, but is
cleared at ClearCachedStatsReport() which is already called on each
SLD/SRD call. Since certificates can only change by negotiation, this
cache is ensured to always be invalidated when certificates change.
Since ClearCachedStatsReport() can happen for other reasons than
certificates changing we may clear the cache more often then is
necessary, but arguably this is seldom enough that we don't have to
create a separate "ClearCertificateStats()" method. Keep it simple?
The cache specifically avoids rtc::SSLCertChain::GetStats which
trigger rtc::SSLCertificate::GetStats and rtc::Base64::EncodeFromArray.
Bug: webrtc:14458
Change-Id: I5f95a4a5eb51cc4462147270fdae7bb9fb7bc822
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276602
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38205}
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index 464e22c..407f33b 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -1254,6 +1254,14 @@
} // 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<RTCStatsCollectorCallback> callback)
: RequestInfo(FilterMode::kAll, std::move(callback), nullptr, nullptr) {}
@@ -1386,6 +1394,7 @@
void RTCStatsCollector::ClearCachedStatsReport() {
RTC_DCHECK_RUN_ON(signaling_thread_);
cached_report_ = nullptr;
+ cached_certificates_by_transport_.clear();
}
void RTCStatsCollector::WaitForPendingRequest() {
@@ -2309,29 +2318,44 @@
std::map<std::string, RTCStatsCollector::CertificateStatsPair>
RTCStatsCollector::PrepareTransportCertificateStats_n(
const std::map<std::string, cricket::TransportStats>&
- transport_stats_by_name) const {
+ transport_stats_by_name) {
RTC_DCHECK_RUN_ON(network_thread_);
rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
std::map<std::string, CertificateStatsPair> 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<rtc::RTCCertificate> 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<rtc::SSLCertChain> 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<rtc::RTCCertificate> local_certificate;
+ if (pc_->GetLocalCertificate(transport_name, &local_certificate)) {
+ certificate_stats_pair.local =
+ local_certificate->GetSSLCertificateChain().GetStats();
+ }
+
+ std::unique_ptr<rtc::SSLCertChain> 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)));
}
-
- 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()));
+ }
}
return transport_cert_stats;
}
diff --git a/pc/rtc_stats_collector.h b/pc/rtc_stats_collector.h
index b1909cb..e5c6aed 100644
--- a/pc/rtc_stats_collector.h
+++ b/pc/rtc_stats_collector.h
@@ -79,7 +79,11 @@
void GetStatsReport(rtc::scoped_refptr<RtpReceiverInternal> selector,
rtc::scoped_refptr<RTCStatsCollectorCallback> 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 @@
struct CertificateStatsPair {
std::unique_ptr<rtc::SSLCertificateStats> local;
std::unique_ptr<rtc::SSLCertificateStats> remote;
+
+ CertificateStatsPair Copy() const;
};
// Stats gathering on a particular thread. Virtual for the sake of testing.
@@ -227,7 +233,7 @@
std::map<std::string, CertificateStatsPair>
PrepareTransportCertificateStats_n(
const std::map<std::string, cricket::TransportStats>&
- 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 @@
// now get rid of the variable and keep the data scoped within a stats
// collection sequence.
std::vector<RtpTransceiverStatsInfo> 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<std::string, CertificateStatsPair> 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 9d7ae24..88bb6ae 100644
--- a/pc/rtc_stats_collector_unittest.cc
+++ b/pc/rtc_stats_collector_unittest.cc
@@ -677,6 +677,18 @@
}
}
+ const RTCCertificateStats* GetCertificateStatsFromFingerprint(
+ const rtc::scoped_refptr<const RTCStatsReport>& report,
+ const std::string& fingerprint) {
+ auto certificates = report->GetStatsOfType<RTCCertificateStats>();
+ for (const auto* certificate : certificates) {
+ if (*certificate->fingerprint == fingerprint) {
+ return certificate;
+ }
+ }
+ return nullptr;
+ }
+
struct ExampleStatsGraph {
rtc::scoped_refptr<RtpSenderInternal> sender;
rtc::scoped_refptr<RtpReceiverInternal> receiver;
@@ -1360,6 +1372,124 @@
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<CertificateInfo> initial_local_certinfo =
+ CreateFakeCertificateAndInfoFromDers({"LocalCertA", "LocalCertB"});
+ pc_->SetLocalCertificate(kTransportName, initial_local_certinfo->certificate);
+ std::unique_ptr<CertificateInfo> 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<const RTCStatsReport> 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<CertificateInfo> updated_local_certinfo =
+ CreateFakeCertificateAndInfoFromDers(
+ {"UpdatedLocalCertA", "UpdatedLocalCertB"});
+ pc_->SetLocalCertificate(kTransportName, updated_local_certinfo->certificate);
+ std::unique_ptr<CertificateInfo> 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<const RTCStatsReport> 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<RTCCertificateStats>(first_local_cert0->id());
+ const auto* second_local_cert1 =
+ second_report->GetAs<RTCCertificateStats>(first_local_cert1->id());
+ const auto* second_remote_cert0 =
+ second_report->GetAs<RTCCertificateStats>(first_remote_cert0->id());
+ const auto* second_remote_cert1 =
+ second_report->GetAs<RTCCertificateStats>(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<const RTCStatsReport> 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<MockSctpDataChannel>(
/*id=*/-1, DataChannelInterface::kConnecting));
diff --git a/rtc_base/ssl_certificate.cc b/rtc_base/ssl_certificate.cc
index ddb1524f..d1fd57f 100644
--- a/rtc_base/ssl_certificate.cc
+++ b/rtc_base/ssl_certificate.cc
@@ -44,6 +44,12 @@
SSLCertificateStats::~SSLCertificateStats() {}
+std::unique_ptr<SSLCertificateStats> SSLCertificateStats::Copy() const {
+ return std::make_unique<SSLCertificateStats>(
+ 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 77fbba3..2e19880 100644
--- a/rtc_base/ssl_certificate.h
+++ b/rtc_base/ssl_certificate.h
@@ -38,6 +38,8 @@
std::string fingerprint_algorithm;
std::string base64_certificate;
std::unique_ptr<SSLCertificateStats> issuer;
+
+ std::unique_ptr<SSLCertificateStats> Copy() const;
};
// Abstract interface overridden by SSL library specific
