Index: net/ssl/channel_id_service.cc |
diff --git a/net/ssl/channel_id_service.cc b/net/ssl/channel_id_service.cc |
index 9bc21794a9c11b70f066fe60ad09aefb3a183587..9ac7c3249a35b4c429304ccc92368f06dc7bc6a9 100644 |
--- a/net/ssl/channel_id_service.cc |
+++ b/net/ssl/channel_id_service.cc |
@@ -88,45 +88,27 @@ void RecordGetChannelIDTime(base::TimeDelta request_time) { |
// unjoined thread, due to relying on a non-leaked LazyInstance |
scoped_ptr<ChannelIDStore::ChannelID> GenerateChannelID( |
const std::string& server_identifier, |
- uint32 serial_number, |
int* error) { |
scoped_ptr<ChannelIDStore::ChannelID> result; |
base::TimeTicks start = base::TimeTicks::Now(); |
base::Time not_valid_before = base::Time::Now(); |
- base::Time not_valid_after = |
- not_valid_before + base::TimeDelta::FromDays(kValidityPeriodInDays); |
- std::string der_cert; |
- std::vector<uint8> private_key_info; |
- scoped_ptr<crypto::ECPrivateKey> key; |
- if (!x509_util::CreateKeyAndChannelIDEC(server_identifier, |
- serial_number, |
- not_valid_before, |
- not_valid_after, |
- &key, |
- &der_cert)) { |
- DLOG(ERROR) << "Unable to create x509 cert for client"; |
- *error = ERR_ORIGIN_BOUND_CERT_GENERATION_FAILED; |
- return result.Pass(); |
- } |
+ scoped_ptr<crypto::ECPrivateKey> key(crypto::ECPrivateKey::Create()); |
- if (!key->ExportEncryptedPrivateKey(ChannelIDService::kEPKIPassword, |
- 1, &private_key_info)) { |
- DLOG(ERROR) << "Unable to export private key"; |
- *error = ERR_PRIVATE_KEY_EXPORT_FAILED; |
+ if (!key) { |
+ DLOG(ERROR) << "Unable to create channel ID key pair"; |
+ *error = ERR_KEY_GENERATION_FAILED; |
return result.Pass(); |
} |
- // TODO(rkn): Perhaps ExportPrivateKey should be changed to output a |
- // std::string* to prevent this copying. |
- std::string key_out(private_key_info.begin(), private_key_info.end()); |
+ std::string private_key_out; |
+ std::string public_key_out; |
+ *error = ExportKeypair(key, &public_key_out, &private_key_out); |
+ if (*error != OK) |
+ return result.Pass(); |
result.reset(new ChannelIDStore::ChannelID( |
- server_identifier, |
- not_valid_before, |
- not_valid_after, |
- key_out, |
- der_cert)); |
+ server_identifier, not_valid_before, private_key_out, public_key_out)); |
UMA_HISTOGRAM_CUSTOM_TIMES("DomainBoundCerts.GenerateCertTime", |
base::TimeTicks::Now() - start, |
base::TimeDelta::FromMilliseconds(1), |
@@ -138,32 +120,71 @@ scoped_ptr<ChannelIDStore::ChannelID> GenerateChannelID( |
} // namespace |
+int CreateECPrivateKeyFromSerializedKey( |
+ const std::string& public_key, |
+ const std::string& private_key, |
+ scoped_ptr<crypto::ECPrivateKey>* key_out) { |
+ if (public_key.empty() || private_key.empty()) |
+ return ERR_UNEXPECTED; |
+ |
+ std::vector<uint8> public_key_vector(public_key.size()); |
+ memcpy(&public_key_vector[0], public_key.data(), public_key.size()); |
Ryan Sleevi
2015/04/15 22:50:02
DANGER: What if |public_key| is .empty()? This wou
nharper
2015/04/25 02:59:19
If public_key or private_key is .empty(), then thi
|
+ std::vector<uint8> private_key_vector(private_key.size()); |
+ memcpy(&private_key_vector[0], private_key.data(), private_key.size()); |
+ crypto::ECPrivateKey* key = |
+ crypto::ECPrivateKey::CreateFromEncryptedPrivateKeyInfo( |
+ ChannelIDService::kEPKIPassword, private_key_vector, |
+ public_key_vector); |
+ if (key == nullptr) |
+ return ERR_UNEXPECTED; |
+ key_out->reset(key); |
+ return OK; |
+} |
+ |
+int ExportKeypair(scoped_ptr<crypto::ECPrivateKey>& key, |
Ryan Sleevi
2015/04/15 22:50:02
pass as const-ref
But really, you should be sendi
nharper
2015/04/25 02:59:19
Changed to const-ref. Does the style guide have in
|
+ std::string* public_key, |
+ std::string* private_key) { |
+ std::vector<uint8> public_key_vector; |
+ std::vector<uint8> private_key_vector; |
+ |
+ if (!key) |
+ return ERR_UNEXPECTED; |
+ |
+ if (!key->ExportEncryptedPrivateKey(ChannelIDService::kEPKIPassword, 1, |
+ &private_key_vector) || |
+ !key->ExportPublicKey(&public_key_vector)) { |
+ DLOG(ERROR) << "Unable to export key"; |
+ return ERR_PRIVATE_KEY_EXPORT_FAILED; |
+ } |
+ |
+ // TODO(rkn): Perhaps ExportPrivateKey should be changed to output a |
+ // std::string* to prevent this copying. |
+ *public_key = std::string(public_key_vector.begin(), public_key_vector.end()); |
Ryan Sleevi
2015/04/15 22:50:02
Does this work across platforms without throwing c
nharper
2015/04/25 02:59:20
This code was copied from the lhs of this diff (se
|
+ *private_key = |
+ std::string(private_key_vector.begin(), private_key_vector.end()); |
+ return OK; |
+} |
Ryan Sleevi
2015/04/15 22:50:02
Should match order in .h
nharper
2015/04/25 02:59:19
Done.
|
+ |
// Represents the output and result callback of a request. |
class ChannelIDServiceRequest { |
public: |
ChannelIDServiceRequest(base::TimeTicks request_start, |
const CompletionCallback& callback, |
- std::string* private_key, |
- std::string* cert) |
- : request_start_(request_start), |
- callback_(callback), |
- private_key_(private_key), |
- cert_(cert) { |
- } |
+ scoped_ptr<crypto::ECPrivateKey>* key) |
+ : request_start_(request_start), callback_(callback), key_(key) {} |
// Ensures that the result callback will never be made. |
void Cancel() { |
RecordGetChannelIDResult(ASYNC_CANCELLED); |
callback_.Reset(); |
- private_key_ = NULL; |
- cert_ = NULL; |
+ key_->reset(); |
} |
// Copies the contents of |private_key| and |cert| to the caller's output |
// arguments and calls the callback. |
void Post(int error, |
const std::string& private_key, |
- const std::string& cert) { |
+ const std::string& public_key) { |
switch (error) { |
case OK: { |
base::TimeDelta request_time = base::TimeTicks::Now() - request_start_; |
@@ -193,8 +214,10 @@ class ChannelIDServiceRequest { |
break; |
} |
if (!callback_.is_null()) { |
- *private_key_ = private_key; |
- *cert_ = cert; |
+ int err = |
+ CreateECPrivateKeyFromSerializedKey(public_key, private_key, key_); |
+ if (error == OK && err != OK) |
+ error = err; |
callback_.Run(error); |
} |
delete this; |
@@ -205,8 +228,7 @@ class ChannelIDServiceRequest { |
private: |
base::TimeTicks request_start_; |
CompletionCallback callback_; |
- std::string* private_key_; |
- std::string* cert_; |
+ scoped_ptr<crypto::ECPrivateKey>* key_; |
}; |
// ChannelIDServiceWorker runs on a worker thread and takes care of the |
@@ -223,7 +245,6 @@ class ChannelIDServiceWorker { |
const std::string& server_identifier, |
const WorkerDoneCallback& callback) |
: server_identifier_(server_identifier), |
- serial_number_(base::RandInt(0, std::numeric_limits<int>::max())), |
origin_loop_(base::MessageLoopProxy::current()), |
callback_(callback) { |
} |
@@ -244,7 +265,7 @@ class ChannelIDServiceWorker { |
// Runs on a worker thread. |
int error = ERR_FAILED; |
scoped_ptr<ChannelIDStore::ChannelID> cert = |
- GenerateChannelID(server_identifier_, serial_number_, &error); |
+ GenerateChannelID(server_identifier_, &error); |
DVLOG(1) << "GenerateCert " << server_identifier_ << " returned " << error; |
#if defined(USE_NSS) |
// Detach the thread from NSPR. |
@@ -262,9 +283,6 @@ class ChannelIDServiceWorker { |
} |
const std::string server_identifier_; |
- // Note that serial_number_ must be initialized on a non-worker thread |
- // (see documentation for GenerateCert). |
- uint32 serial_number_; |
scoped_refptr<base::SequencedTaskRunner> origin_loop_; |
WorkerDoneCallback callback_; |
@@ -293,8 +311,8 @@ class ChannelIDServiceJob { |
void HandleResult(int error, |
const std::string& private_key, |
- const std::string& cert) { |
- PostAll(error, private_key, cert); |
+ const std::string& public_key) { |
+ PostAll(error, private_key, public_key); |
} |
bool CreateIfMissing() const { return create_if_missing_; } |
@@ -302,13 +320,13 @@ class ChannelIDServiceJob { |
private: |
void PostAll(int error, |
const std::string& private_key, |
- const std::string& cert) { |
+ const std::string& public_key) { |
std::vector<ChannelIDServiceRequest*> requests; |
requests_.swap(requests); |
for (std::vector<ChannelIDServiceRequest*>::iterator |
i = requests.begin(); i != requests.end(); i++) { |
- (*i)->Post(error, private_key, cert); |
+ (*i)->Post(error, private_key, public_key); |
// Post() causes the ChannelIDServiceRequest to delete itself. |
} |
} |
@@ -370,7 +388,7 @@ ChannelIDService::ChannelIDService( |
: channel_id_store_(channel_id_store), |
task_runner_(task_runner), |
requests_(0), |
- cert_store_hits_(0), |
+ key_store_hits_(0), |
inflight_joins_(0), |
workers_created_(0), |
weak_ptr_factory_(this) { |
@@ -396,15 +414,14 @@ std::string ChannelIDService::GetDomainForHost(const std::string& host) { |
int ChannelIDService::GetOrCreateChannelID( |
const std::string& host, |
- std::string* private_key, |
- std::string* cert, |
+ scoped_ptr<crypto::ECPrivateKey>* key, |
const CompletionCallback& callback, |
RequestHandle* out_req) { |
DVLOG(1) << __FUNCTION__ << " " << host; |
DCHECK(CalledOnValidThread()); |
base::TimeTicks request_start = base::TimeTicks::Now(); |
- if (callback.is_null() || !private_key || !cert || host.empty()) { |
+ if (callback.is_null() || !key || host.empty()) { |
RecordGetChannelIDResult(INVALID_ARGUMENT); |
return ERR_INVALID_ARGUMENT; |
} |
@@ -417,15 +434,18 @@ int ChannelIDService::GetOrCreateChannelID( |
requests_++; |
+ std::string public_key; |
+ std::string private_key; |
Ryan Sleevi
2015/04/15 22:50:02
These two are unused, right?
nharper
2015/04/25 02:59:19
Correct. Removed.
|
+ |
// See if a request for the same domain is currently in flight. |
bool create_if_missing = true; |
- if (JoinToInFlightRequest(request_start, domain, private_key, cert, |
- create_if_missing, callback, out_req)) { |
+ if (JoinToInFlightRequest(request_start, domain, key, create_if_missing, |
+ callback, out_req)) { |
return ERR_IO_PENDING; |
} |
- int err = LookupChannelID(request_start, domain, private_key, cert, |
- create_if_missing, callback, out_req); |
+ int err = LookupChannelID(request_start, domain, key, create_if_missing, |
+ callback, out_req); |
if (err == ERR_FILE_NOT_FOUND) { |
// Sync lookup did not find a valid cert. Start generating a new one. |
workers_created_++; |
@@ -444,11 +464,9 @@ int ChannelIDService::GetOrCreateChannelID( |
inflight_[domain] = job; |
ChannelIDServiceRequest* request = new ChannelIDServiceRequest( |
- request_start, |
- base::Bind(&RequestHandle::OnRequestComplete, |
- base::Unretained(out_req)), |
- private_key, |
- cert); |
+ request_start, base::Bind(&RequestHandle::OnRequestComplete, |
+ base::Unretained(out_req)), |
+ key); |
job->AddRequest(request); |
out_req->RequestStarted(this, request, callback); |
return ERR_IO_PENDING; |
@@ -457,17 +475,15 @@ int ChannelIDService::GetOrCreateChannelID( |
return err; |
} |
-int ChannelIDService::GetChannelID( |
- const std::string& host, |
- std::string* private_key, |
- std::string* cert, |
- const CompletionCallback& callback, |
- RequestHandle* out_req) { |
+int ChannelIDService::GetChannelID(const std::string& host, |
+ scoped_ptr<crypto::ECPrivateKey>* key, |
+ const CompletionCallback& callback, |
+ RequestHandle* out_req) { |
DVLOG(1) << __FUNCTION__ << " " << host; |
DCHECK(CalledOnValidThread()); |
base::TimeTicks request_start = base::TimeTicks::Now(); |
- if (callback.is_null() || !private_key || !cert || host.empty()) { |
+ if (callback.is_null() || !key || host.empty()) { |
RecordGetChannelIDResult(INVALID_ARGUMENT); |
return ERR_INVALID_ARGUMENT; |
} |
@@ -482,22 +498,20 @@ int ChannelIDService::GetChannelID( |
// See if a request for the same domain currently in flight. |
bool create_if_missing = false; |
- if (JoinToInFlightRequest(request_start, domain, private_key, cert, |
- create_if_missing, callback, out_req)) { |
+ if (JoinToInFlightRequest(request_start, domain, key, create_if_missing, |
+ callback, out_req)) { |
return ERR_IO_PENDING; |
} |
- int err = LookupChannelID(request_start, domain, private_key, cert, |
- create_if_missing, callback, out_req); |
+ int err = LookupChannelID(request_start, domain, key, create_if_missing, |
+ callback, out_req); |
return err; |
} |
-void ChannelIDService::GotChannelID( |
- int err, |
- const std::string& server_identifier, |
- base::Time expiration_time, |
- const std::string& key, |
- const std::string& cert) { |
+void ChannelIDService::GotChannelID(int err, |
+ const std::string& server_identifier, |
+ const std::string& private_key, |
+ const std::string& public_key) { |
DCHECK(CalledOnValidThread()); |
std::map<std::string, ChannelIDServiceJob*>::iterator j; |
@@ -509,17 +523,17 @@ void ChannelIDService::GotChannelID( |
if (err == OK) { |
// Async DB lookup found a valid cert. |
- DVLOG(1) << "Cert store had valid cert for " << server_identifier; |
- cert_store_hits_++; |
+ DVLOG(1) << "Cert store had valid key for " << server_identifier; |
+ key_store_hits_++; |
// ChannelIDServiceRequest::Post will do the histograms and stuff. |
- HandleResult(OK, server_identifier, key, cert); |
+ HandleResult(OK, server_identifier, private_key, public_key); |
return; |
} |
// Async lookup failed or the certificate was missing. Return the error |
// directly, unless the certificate was missing and a request asked to create |
// one. |
if (err != ERR_FILE_NOT_FOUND || !j->second->CreateIfMissing()) { |
- HandleResult(err, server_identifier, key, cert); |
+ HandleResult(err, server_identifier, private_key, public_key); |
return; |
} |
// At least one request asked to create a cert => start generating a new one. |
@@ -556,24 +570,21 @@ void ChannelIDService::GeneratedChannelID( |
if (error == OK) { |
// TODO(mattm): we should just Pass() the cert object to |
// SetChannelID(). |
- channel_id_store_->SetChannelID( |
- cert->server_identifier(), |
- cert->creation_time(), |
- cert->expiration_time(), |
- cert->private_key(), |
- cert->cert()); |
- |
- HandleResult(error, server_identifier, cert->private_key(), cert->cert()); |
+ channel_id_store_->SetChannelID(cert->server_identifier(), |
+ cert->creation_time(), cert->private_key(), |
+ cert->public_key()); |
+ |
+ HandleResult(error, server_identifier, cert->private_key(), |
+ cert->public_key()); |
} else { |
HandleResult(error, server_identifier, std::string(), std::string()); |
} |
} |
-void ChannelIDService::HandleResult( |
- int error, |
- const std::string& server_identifier, |
- const std::string& private_key, |
- const std::string& cert) { |
+void ChannelIDService::HandleResult(int error, |
+ const std::string& server_identifier, |
+ const std::string& private_key, |
+ const std::string& public_key) { |
DCHECK(CalledOnValidThread()); |
std::map<std::string, ChannelIDServiceJob*>::iterator j; |
@@ -585,15 +596,14 @@ void ChannelIDService::HandleResult( |
ChannelIDServiceJob* job = j->second; |
inflight_.erase(j); |
- job->HandleResult(error, private_key, cert); |
+ job->HandleResult(error, private_key, public_key); |
delete job; |
} |
bool ChannelIDService::JoinToInFlightRequest( |
const base::TimeTicks& request_start, |
const std::string& domain, |
- std::string* private_key, |
- std::string* cert, |
+ scoped_ptr<crypto::ECPrivateKey>* key, |
bool create_if_missing, |
const CompletionCallback& callback, |
RequestHandle* out_req) { |
@@ -607,11 +617,9 @@ bool ChannelIDService::JoinToInFlightRequest( |
inflight_joins_++; |
ChannelIDServiceRequest* request = new ChannelIDServiceRequest( |
- request_start, |
- base::Bind(&RequestHandle::OnRequestComplete, |
- base::Unretained(out_req)), |
- private_key, |
- cert); |
+ request_start, base::Bind(&RequestHandle::OnRequestComplete, |
+ base::Unretained(out_req)), |
+ key); |
job->AddRequest(request, create_if_missing); |
out_req->RequestStarted(this, request, callback); |
return true; |
@@ -619,29 +627,29 @@ bool ChannelIDService::JoinToInFlightRequest( |
return false; |
} |
-int ChannelIDService::LookupChannelID( |
- const base::TimeTicks& request_start, |
- const std::string& domain, |
- std::string* private_key, |
- std::string* cert, |
- bool create_if_missing, |
- const CompletionCallback& callback, |
- RequestHandle* out_req) { |
+int ChannelIDService::LookupChannelID(const base::TimeTicks& request_start, |
+ const std::string& domain, |
+ scoped_ptr<crypto::ECPrivateKey>* key, |
+ bool create_if_missing, |
+ const CompletionCallback& callback, |
+ RequestHandle* out_req) { |
// Check if a domain bound cert already exists for this domain. Note that |
// |expiration_time| is ignored, and expired certs are considered valid. |
- base::Time expiration_time; |
+ std::string public_key; |
+ std::string private_key; |
int err = channel_id_store_->GetChannelID( |
- domain, |
- &expiration_time /* ignored */, |
- private_key, |
- cert, |
+ domain, &private_key, &public_key, |
base::Bind(&ChannelIDService::GotChannelID, |
weak_ptr_factory_.GetWeakPtr())); |
if (err == OK) { |
+ err = CreateECPrivateKeyFromSerializedKey(public_key, private_key, key); |
+ if (err != OK) { |
+ return err; |
+ } |
// Sync lookup found a valid cert. |
- DVLOG(1) << "Cert store had valid cert for " << domain; |
- cert_store_hits_++; |
+ DVLOG(1) << "Cert store had valid key for " << domain; |
+ key_store_hits_++; |
RecordGetChannelIDResult(SYNC_SUCCESS); |
base::TimeDelta request_time = base::TimeTicks::Now() - request_start; |
UMA_HISTOGRAM_TIMES("DomainBoundCerts.GetCertTimeSync", request_time); |
@@ -655,11 +663,9 @@ int ChannelIDService::LookupChannelID( |
inflight_[domain] = job; |
ChannelIDServiceRequest* request = new ChannelIDServiceRequest( |
- request_start, |
- base::Bind(&RequestHandle::OnRequestComplete, |
- base::Unretained(out_req)), |
- private_key, |
- cert); |
+ request_start, base::Bind(&RequestHandle::OnRequestComplete, |
+ base::Unretained(out_req)), |
+ key); |
job->AddRequest(request); |
out_req->RequestStarted(this, request, callback); |
return ERR_IO_PENDING; |