Certificate Transparency: Collect metrics on age of SCT vs STH

components/certificate_transparency/single_tree_tracker.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/certificate_transparency/single_tree_tracker.h"
 
 #include <utility>
 
+#include "base/metrics/histogram_macros.h"
 #include "net/cert/ct_log_verifier.h"
 #include "net/cert/signed_certificate_timestamp.h"
 #include "net/cert/x509_certificate.h"
 
 using net::ct::SignedTreeHead;
 
 namespace certificate_transparency {
 
+namespace internal {
+
+const char kCanCheckForInclusionHistogramName[] =
+    "Net.CertificateTransparency.CanInclusionCheckSCT";
+
+}  // namespace internal
+
+namespace {
+
+void LogCanBeCheckedForInclusionToUMA(bool can_be_checked) {

[Eran Messeri, 2016/07/21 17:53:43]

FYI I've documented, in this function, what we're measuring and why.

+  UMA_HISTOGRAM_BOOLEAN(internal::kCanCheckForInclusionHistogramName,
+                        can_be_checked);
+}
+
+}  // namespace

[Ryan Sleevi, 2016/07/20 20:34:00]

Generally we try to have the unnamed namespace at the top. It's unclear if you
put internal first because of kCanCheckForInclusionHistogramName, but if so,
that wouldn't be necessary, because the .h declares that symbol so you'd be fine
referencing it before the definition.

[Eran Messeri, 2016/07/21 17:53:43]

Moved the unnamed namespace to the top, will keep that in mind for future
changes.

+
 SingleTreeTracker::SingleTreeTracker(
     scoped_refptr<const net::CTLogVerifier> ct_log)
     : ct_log_(std::move(ct_log)) {}
 
 SingleTreeTracker::~SingleTreeTracker() {}
 
 void SingleTreeTracker::OnSCTVerified(
     net::X509Certificate* cert,
     const net::ct::SignedCertificateTimestamp* sct) {
   DCHECK_EQ(ct_log_->key_id(), sct->log_id);
 
   // SCT was previously observed, so its status should not be changed.
   if (entries_status_.find(sct->timestamp) != entries_status_.end())
     return;
 
   // If there isn't a valid STH or the STH is not fresh enough to check
   // inclusion against, store the SCT for future checking and return.
   if (verified_sth_.timestamp.is_null() ||
       (verified_sth_.timestamp <
        (sct->timestamp + base::TimeDelta::FromHours(24)))) {
-    // TODO(eranm): UMA - how often SCTs have to wait for a newer STH for
-    // inclusion check.
     entries_status_.insert(
         std::make_pair(sct->timestamp, SCT_PENDING_NEWER_STH));
+
+    // Do not log histogram if there's no STH for this log yet, as it does
+    // not provide any meaningful data on how fresh SCTs usually are.
+    if (!verified_sth_.timestamp.is_null())
+      LogCanBeCheckedForInclusionToUMA(false);
     return;
   }
 
+  LogCanBeCheckedForInclusionToUMA(true);
   // TODO(eranm): Check inclusion here.
-  // TODO(eranm): UMA - how often inclusion can be checked immediately.
   entries_status_.insert(
       std::make_pair(sct->timestamp, SCT_PENDING_INCLUSION_CHECK));
 }
 
 void SingleTreeTracker::NewSTHObserved(const SignedTreeHead& sth) {
   DCHECK_EQ(ct_log_->key_id(), sth.log_id);
 
   if (!ct_log_->VerifySignedTreeHead(sth)) {
     // Sanity check the STH; the caller should have done this
     // already, but being paranoid here.
@@ skipping 35 matching lines @@
 SingleTreeTracker::SCTInclusionStatus
 SingleTreeTracker::GetLogEntryInclusionStatus(
     net::X509Certificate* cert,
     const net::ct::SignedCertificateTimestamp* sct) {
   auto it = entries_status_.find(sct->timestamp);
 
   return it == entries_status_.end() ? SCT_NOT_OBSERVED : it->second;
 }
 
 }  // namespace certificate_transparency