Consider the verification time as well as the expiration time when caching certificate verification…

net/base/multi_threaded_cert_verifier.cc

Index: net/base/multi_threaded_cert_verifier.cc
diff --git a/net/base/multi_threaded_cert_verifier.cc b/net/base/multi_threaded_cert_verifier.cc
index 18122dd6627a09748f86707a623eb9456efff4f4..701dc1038148b307cb06a44e9a61faf27baf871b 100644
--- a/net/base/multi_threaded_cert_verifier.cc
+++ b/net/base/multi_threaded_cert_verifier.cc
@@ -83,6 +83,55 @@ MultiThreadedCertVerifier::CachedResult::CachedResult() : error(ERR_FAILED) {}
 
 MultiThreadedCertVerifier::CachedResult::~CachedResult() {}
 
+MultiThreadedCertVerifier::CacheValidityPeriod::CacheValidityPeriod(
+    const base::Time& now)
+    : verification_time(now),
+      expiration_time(now) {
+}
+
+MultiThreadedCertVerifier::CacheValidityPeriod::CacheValidityPeriod(
+    const base::Time& now,
+    const base::Time& expiration)
+    : verification_time(now),
+      expiration_time(expiration) {
+}
+
+bool MultiThreadedCertVerifier::CacheExpirationFunctor::operator()(
+    const CacheValidityPeriod& now,
+    const CacheValidityPeriod& expiration) const {
+  // Ensure this functor is being used for expiration only, and not strict
+  // weak ordering/sorting. |now| should only ever contain a single
+  // base::Time.
+  // Note: DCHECK_EQ is not used due to operator<< overloading requirements.
+  DCHECK(now.verification_time == now.expiration_time);
+
+  // |now| contains only a single time (verification_time), while |expiration|
+  // contains the validity range - both when the certificate was verified and
+  // when the verification result should expire.
+  //
+  // If the user receives a "not yet valid" message, and adjusts their clock
+  // foward to the correct time, this will (typically) cause
+  // now.verification_time to advance past expiration.expiration_time, thus
+  // treating the cached result as an expired entry and re-verifying.
+  // If the user receives a "expired" message, and adjusts their clock
+  // backwards to the correct time, this will cause now.verification_time to
+  // be less than expiration_verification_time, thus treating the cached
+  // result as an expired entry and re-verifying.
+  // If the user receives either of those messages, and does not adjust their
+  // clock, then the result will be (typically) be cached until the expiration
+  // TTL.
+  //
+  // This algorithm is only problematic if the user consistently keeps
+  // adjusting their clock backwards in increments smaller than the expiration
+  // TTL, in which case, cached elements continue to be added. However,
+  // because the cache has a fixed upper bound, if no entries are expired, a
+  // 'random' entry will be, thus keeping the memory constraints bounded over
+  // time.
+  return now.verification_time >= expiration.verification_time &&
+         now.verification_time < expiration.expiration_time;
+};
+
+
 // Represents the output and result callback of a request.
 class CertVerifierRequest {
  public:
@@ -362,7 +411,7 @@ int MultiThreadedCertVerifier::Verify(X509Certificate* cert,
   const RequestParams key(cert->fingerprint(), cert->ca_fingerprint(),
                           hostname, flags);
   const CertVerifierCache::value_type* cached_entry =
-      cache_.Get(key, base::TimeTicks::Now());
+      cache_.Get(key, CacheValidityPeriod(base::Time::Now()));
   if (cached_entry) {
     ++cache_hits_;
     *out_req = NULL;
@@ -427,8 +476,10 @@ void MultiThreadedCertVerifier::HandleResult(
   CachedResult cached_result;
   cached_result.error = error;
   cached_result.result = verify_result;
-  cache_.Put(key, cached_result, base::TimeTicks::Now(),
-             base::TimeDelta::FromSeconds(kTTLSecs));
+  base::Time now = base::Time::Now();
+  cache_.Put(
+      key, cached_result, CacheValidityPeriod(now),
+      CacheValidityPeriod(now, now + base::TimeDelta::FromSeconds(kTTLSecs)));
 
   std::map<RequestParams, CertVerifierJob*>::iterator j;
   j = inflight_.find(key);