ClientCertStoreChromeOS: support additional non-platform certs.

net/ssl/client_cert_store_chromeos.cc

Index: net/ssl/client_cert_store_chromeos.cc
diff --git a/net/ssl/client_cert_store_chromeos.cc b/net/ssl/client_cert_store_chromeos.cc
index a4abef0244e9e7211a9e49c76e38c6dabb6c03f1..03d5b1d5465df424c792e742c05209ed988f6099 100644
--- a/net/ssl/client_cert_store_chromeos.cc
+++ b/net/ssl/client_cert_store_chromeos.cc
@@ -18,24 +18,32 @@ namespace {
 class CertNotAllowedPredicate {
  public:
   explicit CertNotAllowedPredicate(
+      const CertificateList& additional_certs,
       const ClientCertStoreChromeOS::CertFilter& filter)
-      : filter_(filter) {}
+      : additional_certs_(additional_certs), filter_(filter) {}
+
   bool operator()(const scoped_refptr<X509Certificate>& cert) const {
+    for (const auto& additional_cert : additional_certs_) {
+      if (additional_cert->Equals(cert.get()))
+        return false;
+    }
     return !filter_.IsCertAllowed(cert);
   }
 
  private:
+  const CertificateList& additional_certs_;
   const ClientCertStoreChromeOS::CertFilter& filter_;
 };
 
 }  // namespace
 
 ClientCertStoreChromeOS::ClientCertStoreChromeOS(
+    const CertificateList& additional_certs,
     scoped_ptr<CertFilter> cert_filter,
     const PasswordDelegateFactory& password_delegate_factory)
     : ClientCertStoreNSS(password_delegate_factory),
-      cert_filter_(cert_filter.Pass()) {
-}
+      additional_certs_(additional_certs),
+      cert_filter_(cert_filter.Pass()) {}
 
 ClientCertStoreChromeOS::~ClientCertStoreChromeOS() {}
 
@@ -61,14 +69,22 @@ void ClientCertStoreChromeOS::GetClientCertsImpl(
     const SSLCertRequestInfo& request,
     bool query_nssdb,
     CertificateList* selected_certs) {
+  for (const auto& cert : additional_certs_) {
+    CERT_AddCertToListTail(cert_list,
+                           CERT_DupCertificate(cert->os_cert_handle()));
+  }
+
   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());
+
+  // Remove certificates that are not allowed by |cert_filter_| but always keep
+  // certificates from |additional_certs_|.
+  selected_certs->erase(
+      std::remove_if(selected_certs->begin(), selected_certs->end(),
+                     CertNotAllowedPredicate(additional_certs_, *cert_filter_)),
+      selected_certs->end());

[Ryan Sleevi, 2015/08/07 23:51:09]

This seems highly inefficient - the operator() operation goes from O(N) [linear
scan] to O(N*M), where M is the number of certs, since you iterate through the
entirity of |additional_certs| every single pass.

It seems you really should do two passes - create a temp CERT_CertList, call
ClientCertStoreNSS::GetClientCertsImpl() on it, get filtered_additional_certs.
Then call GetClientCertsImpl on cert_list, get selected_certs. Run
selected_certs through the predicate. Then

selected_certs->insert(selected_certs.end(), filtered_additional_certs.begin(),
filtered_additional_certs.end());

This gives you O(N) + O(M) performance.

[pneubeck (no reviews), 2015/08/10 12:09:56]

Done.

   DVLOG(1) << "filtered " << pre_size - selected_certs->size() << " of "
            << pre_size << " certs";
 }