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 {
+const char kCanCheckForInclusionHistogramName[] =
+    "Net.CertificateTransparency.CanInclusionCheckSCT";

[Alexei Svitkine (slow), 2016/07/18 15:03:57]

If you're only going to use it from the same file once, you might as well inline
it into the macro invocation.

However, in this case it looks like you're actually interested in this name from
the test, so suggest declaring this in the header.

In can be in an "internal" namespace or a static class member.

[Eran Messeri, 2016/07/20 11:02:17]

Done.

+}
+
 namespace certificate_transparency {
 
 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())
+      UMA_HISTOGRAM_BOOLEAN(kCanCheckForInclusionHistogramName, false);
     return;
   }
 
+  UMA_HISTOGRAM_BOOLEAN(kCanCheckForInclusionHistogramName, true);

[Alexei Svitkine (slow), 2016/07/18 15:03:57]

Each UMA macro expansion results in a bunch of code. So please refactor to only
have this macro once for this histogram. (e.g. either make a helper function
that takes the bool param or change the structure of this function.

[Eran Messeri, 2016/07/20 11:02:17]

Done.

   // 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