Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(985)

Unified Diff: net/ssl/server_bound_cert_service.cc

Issue 13130004: Don't expire certs used as channel IDs. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 7 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..144bf92fed3792bc95e2f9687b081336db82a742 100644
--- a/net/ssl/server_bound_cert_service.cc
+++ b/net/ssl/server_bound_cert_service.cc
@@ -37,36 +37,16 @@ namespace {
const int kKeySizeInBits = 1024;
const int kValidityPeriodInDays = 365;
-// When we check the system time, we add this many days to the end of the check
-// so the result will still hold even after chrome has been running for a
-// while.
-const int kSystemTimeValidityBufferInDays = 90;
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 {
@@ -413,10 +393,6 @@ ServerBoundCertService::ServerBoundCertService(
requests_(0),
cert_store_hits_(0),
inflight_joins_(0) {
- base::Time start = base::Time::Now();
- base::Time end = start + base::TimeDelta::FromDays(
- kValidityPeriodInDays + kSystemTimeValidityBufferInDays);
- is_system_time_valid_ = x509_util::IsSupportedValidityRange(start, end);
}
ServerBoundCertService::~ServerBoundCertService() {
@@ -507,32 +483,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)) {
+ // 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 +551,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(),

Powered by Google App Engine
This is Rietveld 408576698