ClientCertStoreChromeOS: support additional non-platform certs.

chrome/browser/chromeos/net/client_cert_store_chromeos.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/net/client_cert_store_chromeos.h"
 
 #include <cert.h>
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/location.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/cert/x509_certificate.h"
+#include "net/ssl/ssl_cert_request_info.h"
 
 namespace chromeos {
 
 namespace {
 
 class CertNotAllowedPredicate {
  public:
   explicit CertNotAllowedPredicate(
       const ClientCertStoreChromeOS::CertFilter& filter)
       : filter_(filter) {}
   bool operator()(const scoped_refptr<net::X509Certificate>& cert) const {
     return !filter_.IsCertAllowed(cert);
   }
 
  private:
   const ClientCertStoreChromeOS::CertFilter& filter_;

[davidben, 2015/08/20 00:40:34]

Nit: I'd probably have made this a pointer I think? I don't know if we save
const references into structures much.

[pneubeck (no reviews), 2015/08/20 07:18:23]

Done.

 };
 
 }  // namespace
 
 ClientCertStoreChromeOS::ClientCertStoreChromeOS(
+    scoped_ptr<CertificateProvider> cert_provider,
     scoped_ptr<CertFilter> cert_filter,
-    const PasswordDelegateFactory& password_delegate_factory)
-    : ClientCertStoreNSS(password_delegate_factory),
-      cert_filter_(cert_filter.Pass()) {}
+    const net::ClientCertStoreNSS::PasswordDelegateFactory&
+        password_delegate_factory)
+    : cert_provider_(cert_provider.Pass()), cert_filter_(cert_filter.Pass()) {}
 
 ClientCertStoreChromeOS::~ClientCertStoreChromeOS() {}
 
 void ClientCertStoreChromeOS::GetClientCerts(
     const net::SSLCertRequestInfo& cert_request_info,
     net::CertificateList* selected_certs,
     const base::Closure& callback) {
-  base::Closure bound_callback = base::Bind(
-      &ClientCertStoreChromeOS::CertFilterInitialized,
-      // Caller is responsible for keeping the ClientCertStore alive
-      // until the callback is run.
-      base::Unretained(this), &cert_request_info, selected_certs, callback);
+  // Caller is responsible for keeping the ClientCertStore alive until the
+  // callback is run.
+  base::Callback<void(const net::CertificateList&)>
+      get_platform_certs_and_filter = base::Bind(
+          &ClientCertStoreChromeOS::GotAdditionalCerts, base::Unretained(this),
+          &cert_request_info, selected_certs, callback);
 
-  if (cert_filter_->Init(bound_callback))
-    bound_callback.Run();
+  base::Closure get_additional_certs_and_continue;
+  if (cert_provider_) {
+    get_additional_certs_and_continue = base::Bind(
+        &CertificateProvider::GetCertificates,
+        base::Unretained(cert_provider_.get()), get_platform_certs_and_filter);
+  } else {
+    get_additional_certs_and_continue =
+        base::Bind(get_platform_certs_and_filter, net::CertificateList());
+  }
+
+  if (cert_filter_->Init(get_additional_certs_and_continue))
+    get_additional_certs_and_continue.Run();
 }
 
-void ClientCertStoreChromeOS::GetClientCertsImpl(
-    CERTCertList* cert_list,
-    const net::SSLCertRequestInfo& request,
-    bool query_nssdb,
-    net::CertificateList* selected_certs) {
-  net::ClientCertStoreNSS::GetClientCertsImpl(cert_list, request, query_nssdb,
-                                              selected_certs);
-
-  size_t pre_size = selected_certs->size();
-  selected_certs->erase(
-      std::remove_if(selected_certs->begin(), selected_certs->end(),
-                     CertNotAllowedPredicate(*cert_filter_)),
-      selected_certs->end());
-  DVLOG(1) << "filtered " << pre_size - selected_certs->size() << " of "
-           << pre_size << " certs";
+void ClientCertStoreChromeOS::GotAdditionalCerts(
+    const net::SSLCertRequestInfo* request,
+    net::CertificateList* selected_certs,
+    const base::Closure& callback,
+    const net::CertificateList& additional_certs) {
+  scoped_ptr<crypto::CryptoModuleBlockingPasswordDelegate> password_delegate;
+  if (!password_delegate_factory_.is_null()) {
+    password_delegate.reset(
+        password_delegate_factory_.Run(request->host_and_port));
+  }
+  if (!base::WorkerPool::PostTaskAndReply(
+          FROM_HERE,
+          base::Bind(&ClientCertStoreChromeOS::GetAndFilterCertsOnWorkerThread,
+                     base::Unretained(this), base::Passed(&password_delegate),
+                     request, additional_certs, selected_certs),
+          callback, true)) {
+    selected_certs->clear();
+    callback.Run();
+  }
 }
 
-void ClientCertStoreChromeOS::CertFilterInitialized(
+void ClientCertStoreChromeOS::GetAndFilterCertsOnWorkerThread(
+    scoped_ptr<crypto::CryptoModuleBlockingPasswordDelegate> password_delegate,
     const net::SSLCertRequestInfo* request,
-    net::CertificateList* selected_certs,
-    const base::Closure& callback) {
-  net::ClientCertStoreNSS::GetClientCerts(*request, selected_certs, callback);
+    const net::CertificateList& additional_certs,
+    net::CertificateList* selected_certs) {
+  net::CertificateList unfiltered_certs;
+  net::ClientCertStoreNSS::GetPlatformCerts(password_delegate.Pass(),
+                                            &unfiltered_certs);
+
+  size_t pre_size = unfiltered_certs.size();
+  unfiltered_certs.erase(
+      std::remove_if(unfiltered_certs.begin(), unfiltered_certs.end(),
+                     CertNotAllowedPredicate(*cert_filter_)),
+      unfiltered_certs.end());
+  DVLOG(1) << "skipped " << pre_size - unfiltered_certs.size() << " of "
+           << pre_size << " platform certs";

[davidben, 2015/08/20 00:40:34]

Optional: Has this and the corresponding NSS DVLOG ever been useful? Seems we
may as well not bother with either I think. I dunno, the only time I've ever
debugged an issue with existing logging is when we don't preserve all details of
some error (notably Windows error codes when some CNG or CAPI operation fails).

[pneubeck (no reviews), 2015/08/20 07:18:23]

Removed.

+
+  unfiltered_certs.insert(unfiltered_certs.end(), additional_certs.begin(),
+                          additional_certs.end());
+
+  net::ClientCertStoreNSS::FilterCertsOnWorkerThread(unfiltered_certs, *request,
+                                                     true, selected_certs);
 }
 
 }  // namespace chromeos