Don't expire certs used as channel IDs.

net/ssl/server_bound_cert_service.cc

Index: net/ssl/server_bound_cert_service.cc
diff --git a/net/ssl/server_bound_cert_service.cc b/net/ssl/server_bound_cert_service.cc
index ece1a2c8a461ffb3d9b86d3711b6a3bbdf536797..6163d0a33f5ef01779ee13ef32ec1e96515b3c5c 100644
--- a/net/ssl/server_bound_cert_service.cc
+++ b/net/ssl/server_bound_cert_service.cc
@@ -46,27 +46,11 @@ bool IsSupportedCertType(uint8 type) {
   switch(type) {
     case CLIENT_CERT_ECDSA_SIGN:
       return true;
-    // If we add any more supported types, CertIsValid will need to be updated
-    // to check that the returned type matches one of the requested types.
     default:
       return false;
   }
 }
 
-bool CertIsValid(const std::string& domain,
-                 SSLClientCertType type,
-                 base::Time expiration_time) {
-  if (expiration_time < base::Time::Now()) {
-    DVLOG(1) << "Cert store had expired cert for " << domain;
-    return false;
-  } else if (!IsSupportedCertType(type)) {
-    DVLOG(1) << "Cert store had cert of wrong type " << type << " for "
-             << domain;
-    return false;
-  }
-  return true;
-}
-
 // Used by the GetDomainBoundCertResult histogram to record the final
 // outcome of each GetDomainBoundCert call.  Do not re-use values.
 enum GetCertResult {
@@ -415,7 +399,7 @@ ServerBoundCertService::ServerBoundCertService(
       inflight_joins_(0) {
   base::Time start = base::Time::Now();
   base::Time end = start + base::TimeDelta::FromDays(
-          kValidityPeriodInDays + kSystemTimeValidityBufferInDays);
+      kValidityPeriodInDays + kSystemTimeValidityBufferInDays);
   is_system_time_valid_ = x509_util::IsSupportedValidityRange(start, end);
 }
 
@@ -507,32 +491,30 @@ int ServerBoundCertService::GetDomainBoundCert(
   }
 
   // Check if a domain bound cert of an acceptable type already exists for this
-  // domain, and that it has not expired.
+  // domain. Note that |expiration_time| is ignored, and expired certs are
+  // considered valid.
   base::Time expiration_time;
   if (server_bound_cert_store_->GetServerBoundCert(
       domain,
       type,
-      &expiration_time,
+      &expiration_time /* ignored */,
       private_key,
       cert,
       base::Bind(&ServerBoundCertService::GotServerBoundCert,
                  weak_ptr_factory_.GetWeakPtr()))) {
-    if (*type != CLIENT_CERT_INVALID_TYPE) {
-      // Sync lookup found a cert.
-      if (CertIsValid(domain, *type, expiration_time)) {
-        DVLOG(1) << "Cert store had valid cert for " << domain
-                 << " of type " << *type;
-        cert_store_hits_++;
-        RecordGetDomainBoundCertResult(SYNC_SUCCESS);
-        base::TimeDelta request_time = base::TimeTicks::Now() - request_start;
-        UMA_HISTOGRAM_TIMES("DomainBoundCerts.GetCertTimeSync", request_time);
-        RecordGetCertTime(request_time);
-        return OK;
-      }
+    if (type && IsSupportedCertType(*type)) {

[mattm, 2013/04/01 23:03:12]

Can remove the "type && " check, since it is already assumed valid when we
passed it to GetServerBoundCert.  (If you really wanted to, you could add it to
the invalid argument check at the start of the function)

[thaidn_google, 2013/04/01 23:56:14]

Done.

+      // Sync lookup found a valid cert.
+      DVLOG(1) << "Cert store had valid cert for " << domain
+               << " of type " << *type;
+      cert_store_hits_++;
+      RecordGetDomainBoundCertResult(SYNC_SUCCESS);
+      base::TimeDelta request_time = base::TimeTicks::Now() - request_start;
+      UMA_HISTOGRAM_TIMES("DomainBoundCerts.GetCertTimeSync", request_time);
+      RecordGetCertTime(request_time);
+      return OK;
     }
 
-    // Sync lookup did not find a cert, or it found an expired one.  Start
-    // generating a new one.
+    // Sync lookup did not find a valid cert.  Start generating a new one.
     ServerBoundCertServiceWorker* worker = new ServerBoundCertServiceWorker(
         domain,
         preferred_type,
@@ -577,20 +559,17 @@ void ServerBoundCertService::GotServerBoundCert(
   }
   ServerBoundCertServiceJob* job = j->second;
 
-  if (type != CLIENT_CERT_INVALID_TYPE) {
-    // Async DB lookup found a cert.
-    if (CertIsValid(server_identifier, type, expiration_time)) {
-      DVLOG(1) << "Cert store had valid cert for " << server_identifier
-               << " of type " << type;
-      cert_store_hits_++;
-      // ServerBoundCertServiceRequest::Post will do the histograms and stuff.
-      HandleResult(OK, server_identifier, type, key, cert);
-      return;
-    }
+  if (IsSupportedCertType(type)) {
+    // Async DB lookup found a valid cert.
+    DVLOG(1) << "Cert store had valid cert for " << server_identifier
+             << " of type " << type;
+    cert_store_hits_++;
+    // ServerBoundCertServiceRequest::Post will do the histograms and stuff.
+    HandleResult(OK, server_identifier, type, key, cert);
+    return;
   }
 
-  // Async lookup did not find a cert, or it found an expired one.  Start
-  // generating a new one.
+  // Async lookup did not find a valid cert. Start generating a new one.
   ServerBoundCertServiceWorker* worker = new ServerBoundCertServiceWorker(
       server_identifier,
       job->type(),