Certificate Transparency: Require SCTs for EV certificates.

net/cert/cert_policy_enforcer.cc

Index: net/cert/cert_policy_enforcer.cc
diff --git a/net/cert/cert_policy_enforcer.cc b/net/cert/cert_policy_enforcer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..45255e7d2df79bbf04dfee12de084b881114fc6f
--- /dev/null
+++ b/net/cert/cert_policy_enforcer.cc
@@ -0,0 +1,134 @@
+// Copyright 2014 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 "net/cert/cert_policy_enforcer.h"
+
+#include <algorithm>
+
+#include "base/build_time.h"
+#include "base/metrics/histogram.h"
+#include "base/numerics/safe_conversions.h"
+#include "base/strings/string_number_conversions.h"
+#include "net/cert/ct_ev_whitelist.h"
+#include "net/cert/ct_verify_result.h"
+#include "net/cert/signed_certificate_timestamp.h"
+#include "net/cert/x509_certificate.h"
+
+namespace net {
+
+namespace {
+
+bool IsEmbeddedSCT(const scoped_refptr<ct::SignedCertificateTimestamp>& sct) {
+  return sct->origin == ct::SignedCertificateTimestamp::SCT_EMBEDDED;
+}
+
+// Taken from TransportSecurityState::IsBuildTimely

[Ryan Sleevi, 2014/11/28 15:27:44]

Don't describe where this is from, describe why it exists and is relevant.

[Eran Messeri, 2014/12/01 13:59:02]

Done.

+// TODO(eranm): Move to base or net/base
+bool IsBuildTimely() {
+#if defined(DONT_EMBED_BUILD_METADATA) && !defined(OFFICIAL_BUILD)
+  return true;
+#else
+  const base::Time build_time = base::GetBuildTime();
+  // We consider built-in information to be timely for 10 weeks.
+  return (base::Time::Now() - build_time).InDays() < 70 /* 10 weeks */;
+#endif
+}
+
+uint32_t ApproximateMonthDifference(const base::Time& start,
+                                    const base::Time& end) {
+  base::Time::Exploded exploded_start;
+  base::Time::Exploded exploded_expiry;
+  start.UTCExplode(&exploded_start);
+  end.UTCExplode(&exploded_expiry);

[Ryan Sleevi, 2014/11/28 15:27:44]

There are base::Time values that may not be represented as UTC time values (due
to limitations of time_t)

You want to check whether either of these times is == Max()

[Note, for certs, it's messier; the X509Certificate won't parse on Windows and
the OSCertHandle will == NULL. On Linux, because of our use of NSS, the PRTime
can represent a greater range than time_t, so it can be < Max() but still >
TimeT::Max(); on OS X/Android/iOS it will == Max()}

[Eran Messeri, 2014/12/01 13:59:02]

As discussed offline, added checks for is_max below; Can't easily  add a test
because it depends on whether it's running on a 32-bit or a 64-bit machine
(it'll only fail on a 32-bit machine)

[Ryan Sleevi, 2014/12/01 15:27:55]

Yeah, we'll come back to this as a follow-on (we can look at
std::numeric_limits<time_t>::max() on the POSIX-ish systems, for example, and
use that as a gauge for EXPECT_TRUE or EXPECT_FALSE, etc)

+  uint32_t month_diff = (exploded_expiry.year - exploded_start.year) * 12 +
+                        (exploded_expiry.month - exploded_start.month);
+
+  // Add any remainder as a full month.
+  if (exploded_expiry.day_of_month > exploded_start.day_of_month)
+    ++month_diff;
+
+  return month_diff;
+}
+}
+
+CertPolicyEnforcer::CertPolicyEnforcer(size_t num_ct_logs,
+                                       bool require_ct_for_ev)
+    : num_ct_logs_(num_ct_logs), enforce_ct_requirement_(require_ct_for_ev) {
+}
+
+CertPolicyEnforcer::~CertPolicyEnforcer() {
+}
+
+bool CertPolicyEnforcer::DoesConformToCTEVPolicy(
+    X509Certificate* cert,
+    const ct::EVCertsWhitelist* ev_whitelist,
+    const ct::CTVerifyResult& ct_result) {
+  bool cert_in_ev_whitelist = false;
+  if (ev_whitelist && ev_whitelist->IsValid()) {
+    const SHA256HashValue fingerprint(
+        X509Certificate::CalculateFingerprint256(cert->os_cert_handle()));
+
+    std::string truncated_fp =
+        std::string(reinterpret_cast<const char*>(fingerprint.data), 8);

[Ryan Sleevi, 2014/11/28 15:27:44]

Note: If we make ContainsCertificateHash to take a base::StringPiece instead of
an std::string, we could truncate just using a base::StringPiece here and avoid
a copy.

Then again, this will trigger std::string's small string optimization and copy 8
bytes onto the stack, so perhaps its no big deal. Just calling it out.

[Eran Messeri, 2014/12/01 13:59:02]

Acknowledged, since it's changing the interface (which is implemented in
//chrome/browser/net) it'll be neater to address in a follow-up optimization CL.

+    cert_in_ev_whitelist = ev_whitelist->ContainsCertificateHash(truncated_fp);
+
+    VLOG(1) << "Have a valid whitelist. Is cert with fingerprint "
+            << base::HexEncode(
+                   reinterpret_cast<const char*>(truncated_fp.data()),
+                   truncated_fp.length()) << " in the whitelist? "
+            << (cert_in_ev_whitelist ? "true" : "false");

[Ryan Sleevi, 2014/11/28 15:27:44]

I'm not keen on this VLOG. If the intent is to aid debugging in the field, it
should go into the NetLog instead (or likely, a BoundNetLog which can be bound
to the certificate verification event). Alternatively, removing it.

[Eran Messeri, 2014/12/01 13:59:02]

Removed, for now. I'll can NetLog logging in a follow-up change (although I'm
not sure it's necessary because it should be able to tell from the histograms
what happened with each cert).

[Ryan Sleevi, 2014/12/01 15:27:55]

The histograms won't tell us (or the net-internals user) which cert it was that
failed or why. We'll know that some certs aren't in, but if the user doesn't see
EV, they won't know if it's for this reason or some other reason.

So I'm still keen on NetLogs for this.

+
+    UMA_HISTOGRAM_BOOLEAN("Net.SSL_EVCertificateInWhitelist",
+                          cert_in_ev_whitelist);
+  }
+
+  if (!enforce_ct_requirement_)
+    return true;
+
+  if (!IsBuildTimely())
+    return false;
+
+  if (cert_in_ev_whitelist)
+    return true;
+
+  size_t num_valid_scts = ct_result.verified_scts.size();
+  size_t num_embedded_scts =
+      std::count_if(ct_result.verified_scts.begin(),
+                    ct_result.verified_scts.end(), IsEmbeddedSCT);
+
+  // TODO(eranm): Count the number of *independent* SCTs once the information
+  // about log operators is available, crbug.com/425174
+  size_t num_non_embedded_scts = num_valid_scts - num_embedded_scts;
+  if (num_non_embedded_scts >= 2)
+    return true;

[Ryan Sleevi, 2014/11/28 15:27:44]

I find this conditional hard to reason about (as with those on 106/107), so you
should consider explaining why / what portion of this policy it applies to and
how this logically works.

// If at least two valid SCTs were delivered by means other than embedding (i.e.
in a TLS extension or OCSP),
// then the certificate conforms to Sections 2/3 of the CT/EV policy.

[Eran Messeri, 2014/12/01 13:59:02]

Done.

+
+  if ((num_non_embedded_scts == 1) && (num_embedded_scts > 0))
+    return true;

[Ryan Sleevi, 2014/11/28 15:27:44]

This doesn't seem spelled out in our policy at all. I think this should be
removed?

[Eran Messeri, 2014/12/01 13:59:02]

Done.

+
+  if (cert->valid_start().is_null() || cert->valid_expiry().is_null()) {
+    // Will not be able to calculate the certificate's validity period.
+    return false;
+  }
+
+  uint32_t expiry_in_months_approx =
+      ApproximateMonthDifference(cert->valid_start(), cert->valid_expiry());
+  // At most 5 SCTs are required - for certificate with lifetime of over
+  // 39 months.
+  size_t num_required_embedded_scts;
+  if (expiry_in_months_approx > 39) {
+    num_required_embedded_scts = 5;
+  } else if (expiry_in_months_approx > 27) {
+    num_required_embedded_scts = 4;
+  } else if (expiry_in_months_approx >= 15) {
+    num_required_embedded_scts = 3;
+  } else {
+    num_required_embedded_scts = 2;
+  }
+
+  size_t min_acceptable_logs = std::max(num_ct_logs_, static_cast<size_t>(2u));
+  return num_embedded_scts >=
+         std::min(num_required_embedded_scts, min_acceptable_logs);

[Ryan Sleevi, 2014/11/28 15:27:44]

BUG/FUTURE: This doesn't consider when we need to distrust a log. Can you file a
bug and add a TODO here to indicate that the number of embedded SCTs that are
signature valid are X, but the number of embedded SCTs required to be valid
(recognized by Chrome) is 1.

+}
+
+}  // namespace net