Refactor client cert private key handling.

chrome/browser/chromeos/net/client_cert_store_chromeos.cc

Index: chrome/browser/chromeos/net/client_cert_store_chromeos.cc
diff --git a/chrome/browser/chromeos/net/client_cert_store_chromeos.cc b/chrome/browser/chromeos/net/client_cert_store_chromeos.cc
index 48c3cb7a8818ca792a1df39b8232a08c74c23982..675c2306f96b521f653bd6240eb16d8f4fae7663 100644
--- a/chrome/browser/chromeos/net/client_cert_store_chromeos.cc
+++ b/chrome/browser/chromeos/net/client_cert_store_chromeos.cc
@@ -6,29 +6,66 @@
 
 #include <cert.h>
 #include <algorithm>
+#include <iterator>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/location.h"
+#include "base/memory/ptr_util.h"
 #include "base/task_runner_util.h"
 #include "base/threading/worker_pool.h"
 #include "chrome/browser/chromeos/certificate_provider/certificate_provider.h"
 #include "crypto/nss_crypto_module_delegate.h"
+#include "net/ssl/client_key_store.h"
 #include "net/ssl/ssl_cert_request_info.h"
+#include "net/ssl/ssl_private_key.h"
 
 namespace chromeos {
 
 namespace {
 
+scoped_refptr<net::SSLPrivateKey> FetchClientCertPrivateKeyCros(
+    const net::X509Certificate* certificate) {
+  return net::ClientKeyStore::GetInstance()->FetchClientCertPrivateKey(
+      *certificate);
+}
+
+class ClientCertIdentityCros : public net::ClientCertIdentity {
+ public:
+  explicit ClientCertIdentityCros(scoped_refptr<net::X509Certificate> cert)
+      : net::ClientCertIdentity(std::move(cert)) {}
+  ~ClientCertIdentityCros() override = default;
+
+  void AcquirePrivateKey(
+      const base::Callback<void(scoped_refptr<net::SSLPrivateKey>)>&
+          private_key_callback) override;
+};
+
+void ClientCertIdentityCros::AcquirePrivateKey(

[davidben, 2017/06/07 23:06:16]

Optional: Any reason not to inline this one into the declaration? Not sure what
the style is.

[mattm, 2017/06/08 21:47:55]

Done.

+    const base::Callback<void(scoped_refptr<net::SSLPrivateKey>)>&
+        private_key_callback) {
+  if (base::PostTaskAndReplyWithResult(
+          base::WorkerPool::GetTaskRunner(true /* task_is_slow */).get(),

[davidben, 2017/06/07 23:06:16]

This doesn't actually need to be on a worker thread, does it? It doesn't really
do anything. :-)

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/certificate_prov...

[mattm, 2017/06/08 21:47:55]

Hm, yeah, guess so.

It feels a little weird to depend on that when using a general interface like
clientkeystore, but since it's really just a leftover, ok.

+          FROM_HERE,
+          base::Bind(&FetchClientCertPrivateKeyCros,
+                     base::RetainedRef(certificate())),
+          private_key_callback)) {
+    return;
+  }
+  // If the task could not be posted, behave as if there was no key.
+  private_key_callback.Run(nullptr);
+}
+
 class CertNotAllowedPredicate {
  public:
   explicit CertNotAllowedPredicate(
       const ClientCertStoreChromeOS::CertFilter* filter)
       : filter_(filter) {}
-  bool operator()(const scoped_refptr<net::X509Certificate>& cert) const {
-    return !filter_->IsCertAllowed(cert);
+  bool operator()(
+      const std::unique_ptr<net::ClientCertIdentity>& identity) const {
+    return !filter_->IsCertAllowed(identity->certificate());
   }
 
  private:
@@ -74,46 +111,41 @@ void ClientCertStoreChromeOS::GotAdditionalCerts(
     const net::SSLCertRequestInfo* request,
     const ClientCertListCallback& callback,
     const net::CertificateList& additional_certs) {
-  std::unique_ptr<crypto::CryptoModuleBlockingPasswordDelegate>
-      password_delegate;
-  if (!password_delegate_factory_.is_null()) {
-    password_delegate.reset(
-        password_delegate_factory_.Run(request->host_and_port));
-  }
+  scoped_refptr<crypto::CryptoModuleBlockingPasswordDelegate> password_delegate;
+  if (!password_delegate_factory_.is_null())
+    password_delegate = password_delegate_factory_.Run(request->host_and_port);
   if (base::PostTaskAndReplyWithResult(
           base::WorkerPool::GetTaskRunner(true /* task_is_slow */).get(),
           FROM_HERE,
           base::Bind(&ClientCertStoreChromeOS::GetAndFilterCertsOnWorkerThread,
-                     base::Unretained(this), base::Passed(&password_delegate),
-                     request, additional_certs),
+                     base::Unretained(this), password_delegate, request,
+                     additional_certs),
           callback)) {
     return;
   }
   // If the task could not be posted, behave as if there were no certificates.
-  callback.Run(net::CertificateList());
+  callback.Run(net::ClientCertIdentityList());
 }
 
-net::CertificateList ClientCertStoreChromeOS::GetAndFilterCertsOnWorkerThread(
-    std::unique_ptr<crypto::CryptoModuleBlockingPasswordDelegate>
+net::ClientCertIdentityList
+ClientCertStoreChromeOS::GetAndFilterCertsOnWorkerThread(
+    scoped_refptr<crypto::CryptoModuleBlockingPasswordDelegate>
         password_delegate,
     const net::SSLCertRequestInfo* request,
     const net::CertificateList& additional_certs) {
-  net::CertificateList unfiltered_certs;
+  net::ClientCertIdentityList client_certs;
   net::ClientCertStoreNSS::GetPlatformCertsOnWorkerThread(
-      std::move(password_delegate), &unfiltered_certs);
+      std::move(password_delegate), &client_certs);
 
-  unfiltered_certs.erase(
-      std::remove_if(unfiltered_certs.begin(), unfiltered_certs.end(),
+  client_certs.erase(
+      std::remove_if(client_certs.begin(), client_certs.end(),
                      CertNotAllowedPredicate(cert_filter_.get())),
-      unfiltered_certs.end());
-
-  unfiltered_certs.insert(unfiltered_certs.end(), additional_certs.begin(),
-                          additional_certs.end());
+      client_certs.end());
 
-  net::CertificateList selected_certs;
-  net::ClientCertStoreNSS::FilterCertsOnWorkerThread(unfiltered_certs, *request,
-                                                     &selected_certs);
-  return selected_certs;
+  for (const scoped_refptr<net::X509Certificate>& cert : additional_certs)
+    client_certs.push_back(base::MakeUnique<ClientCertIdentityCros>(cert));
+  net::ClientCertStoreNSS::FilterCertsOnWorkerThread(&client_certs, *request);
+  return client_certs;
 }
 
 }  // namespace chromeos