Move client certificates retrieval logic out of the SSL sockets.

CL 11879048 introduces ClientCertStore API providing client certificate
lookup/filtering logic currently being done at the SSL socket level. This patch
removes this logic from the sockets, plugging the new API in the upper layers instead.

BUG=170374


Review URL: https://chromiumcodereview.appspot.com/12035105

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@181104 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 359 insertions, 376 deletions

diff --git a/content/browser/loader/resource_loader.cc b/content/browser/loader/resource_loader.cc
index a45229e..e4716dd 100644
--- a/content/browser/loader/resource_loader.cc
+++ b/content/browser/loader/resource_loader.cc
@@ -21,6 +21,8 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/resource_response.h"
 #include "content/public/common/url_constants.h"
+#include "net/base/client_cert_store.h"
+#include "net/base/client_cert_store_impl.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "webkit/appcache/appcache_interceptor.h"
@@ -58,16 +60,12 @@ void PopulateResourceResponse(net::URLRequest* request,
 ResourceLoader::ResourceLoader(scoped_ptr<net::URLRequest> request,
                                scoped_ptr<ResourceHandler> handler,
                                ResourceLoaderDelegate* delegate)
-    : deferred_stage_(DEFERRED_NONE),
-      request_(request.Pass()),
-      handler_(handler.Pass()),
-      delegate_(delegate),
-      last_upload_position_(0),
-      waiting_for_upload_progress_ack_(false),
-      is_transferring_(false),
-      weak_ptr_factory_(this) {
-  request_->set_delegate(this);
-  handler_->SetController(this);
+    : weak_ptr_factory_(this) {
+  scoped_ptr<net::ClientCertStore> client_cert_store;
+#if !defined(USE_OPENSSL)
+  client_cert_store.reset(new net::ClientCertStoreImpl());
+#endif
+  Init(request.Pass(), handler.Pass(), delegate, client_cert_store.Pass());
 }
 
 ResourceLoader::~ResourceLoader() {
@@ -195,6 +193,32 @@ void ResourceLoader::OnUploadProgressACK() {
   waiting_for_upload_progress_ack_ = false;
 }
 
+ResourceLoader::ResourceLoader(
+    scoped_ptr<net::URLRequest> request,
+    scoped_ptr<ResourceHandler> handler,
+    ResourceLoaderDelegate* delegate,
+    scoped_ptr<net::ClientCertStore> client_cert_store)
+    : weak_ptr_factory_(this) {
+  Init(request.Pass(), handler.Pass(), delegate, client_cert_store.Pass());
+}
+
+void ResourceLoader::Init(scoped_ptr<net::URLRequest> request,
+                          scoped_ptr<ResourceHandler> handler,
+                          ResourceLoaderDelegate* delegate,
+                          scoped_ptr<net::ClientCertStore> client_cert_store) {
+  deferred_stage_ = DEFERRED_NONE;
+  request_ = request.Pass();
+  handler_ = handler.Pass();
+  delegate_ = delegate;
+  last_upload_position_ = 0;
+  waiting_for_upload_progress_ack_ = false;
+  is_transferring_ = false;
+  client_cert_store_ = client_cert_store.Pass();
+
+  request_->set_delegate(this);
+  handler_->SetController(this);
+}
+
 void ResourceLoader::OnReceivedRedirect(net::URLRequest* unused,
                                         const GURL& new_url,
                                         bool* defer) {
@@ -269,11 +293,14 @@ void ResourceLoader::OnCertificateRequested(
     return;
   }
 
+#if !defined(USE_OPENSSL)
+  client_cert_store_->GetClientCerts(*cert_info, &cert_info->client_certs);
   if (cert_info->client_certs.empty()) {
     // No need to query the user if there are no certs to choose from.
     request_->ContinueWithCertificate(NULL);
     return;
   }
+#endif
 
   DCHECK(!ssl_client_auth_handler_) <<
       "OnCertificateRequested called with ssl_client_auth_handler pending";
diff --git a/content/browser/loader/resource_loader.h b/content/browser/loader/resource_loader.h
index 14a67ec..03667e3 100644
--- a/content/browser/loader/resource_loader.h
+++ b/content/browser/loader/resource_loader.h
@@ -5,13 +5,19 @@
 #ifndef CONTENT_BROWSER_LOADER_RESOURCE_LOADER_H_
 #define CONTENT_BROWSER_LOADER_RESOURCE_LOADER_H_
 
+#include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "content/browser/loader/resource_handler.h"
 #include "content/browser/ssl/ssl_error_handler.h"
+#include "content/common/content_export.h"
 #include "content/public/browser/resource_controller.h"
 #include "net/url_request/url_request.h"
 
+namespace net {
+class ClientCertStore;
+}
+
 namespace content {
 class ResourceDispatcherHostLoginDelegate;
 class ResourceLoaderDelegate;
@@ -21,9 +27,9 @@ class SSLClientAuthHandler;
 // This class is responsible for driving the URLRequest (i.e., calling Start,
 // Read, and servicing events).  It has a ResourceHandler, which is typically a
 // chain of ResourceHandlers, and is the ResourceController for its handler.
-class ResourceLoader : public net::URLRequest::Delegate,
-                       public SSLErrorHandler::Delegate,
-                       public ResourceController {
+class CONTENT_EXPORT ResourceLoader : public net::URLRequest::Delegate,
+                                      public SSLErrorHandler::Delegate,
+                                      public ResourceController {
  public:
   ResourceLoader(scoped_ptr<net::URLRequest> request,
                  scoped_ptr<ResourceHandler> handler,
@@ -50,6 +56,19 @@ class ResourceLoader : public net::URLRequest::Delegate,
   void OnUploadProgressACK();
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(ResourceLoaderTest, ClientCertStoreLookup);
+
+  ResourceLoader(scoped_ptr<net::URLRequest> request,
+                 scoped_ptr<ResourceHandler> handler,
+                 ResourceLoaderDelegate* delegate,
+                 scoped_ptr<net::ClientCertStore> client_cert_store);
+
+  // Initialization logic shared between the public and private constructor.
+  void Init(scoped_ptr<net::URLRequest> request,
+            scoped_ptr<ResourceHandler> handler,
+            ResourceLoaderDelegate* delegate,
+            scoped_ptr<net::ClientCertStore> client_cert_store);
+
   // net::URLRequest::Delegate implementation:
   virtual void OnReceivedRedirect(net::URLRequest* request,
                                   const GURL& new_url,
@@ -114,6 +133,8 @@ class ResourceLoader : public net::URLRequest::Delegate,
   // which point we'll receive a new ResourceHandler.
   bool is_transferring_;
 
+  scoped_ptr<net::ClientCertStore> client_cert_store_;
+
   base::WeakPtrFactory<ResourceLoader> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourceLoader);
diff --git a/content/browser/loader/resource_loader_unittest.cc b/content/browser/loader/resource_loader_unittest.cc
new file mode 100644
index 0000000..135f41c
--- /dev/null
+++ b/content/browser/loader/resource_loader_unittest.cc
@@ -0,0 +1,256 @@
+// Copyright (c) 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 "content/browser/loader/resource_loader.h"
+
+#include "base/message_loop.h"
+#include "content/browser/browser_thread_impl.h"
+#include "content/browser/loader/resource_loader_delegate.h"
+#include "content/public/browser/resource_request_info.h"
+#include "content/public/test/mock_resource_context.h"
+#include "content/test/test_content_browser_client.h"
+#include "net/base/client_cert_store.h"
+#include "net/base/ssl_cert_request_info.h"
+#include "net/base/x509_certificate.h"
+#include "net/url_request/url_request.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+namespace {
+
+// Stub client certificate store that returns a preset list of certificates for
+// each request and records the arguments of the most recent request for later
+// inspection.
+class ClientCertStoreStub : public net::ClientCertStore {
+ public:
+  ClientCertStoreStub(const net::CertificateList& certs)
+      : response_(certs),
+        request_count_(0) {}
+
+  virtual ~ClientCertStoreStub() {}
+
+  // Returns |cert_authorities| field of the certificate request passed in the
+  // most recent call to GetClientCerts().
+  // TODO(ppi): Make the stub independent from the internal representation of
+  // SSLCertRequestInfo. For now it seems that we cannot neither save the
+  // scoped_refptr<> (since it is never passed to us) nor copy the entire
+  // CertificateRequestInfo (since there is no copy constructor).
+  std::vector<std::string> requested_authorities() {
+    return requested_authorities_;
+  }
+
+  // Returns the number of calls to GetClientCerts().
+  int request_count() {
+    return request_count_;
+  }
+
+  // net::ClientCertStore:
+  virtual bool GetClientCerts(const net::SSLCertRequestInfo& cert_request_info,
+                              net::CertificateList* selected_certs) OVERRIDE {
+    ++request_count_;
+    requested_authorities_ = cert_request_info.cert_authorities;
+    *selected_certs = response_;
+    return true;
+  }
+
+ private:
+  const net::CertificateList response_;
+  int request_count_;
+  std::vector<std::string> requested_authorities_;
+};
+
+// Dummy implementation of ResourceHandler, instance of which is needed to
+// initialize ResourceLoader.
+class ResourceHandlerStub : public ResourceHandler {
+ public:
+  virtual bool OnUploadProgress(int request_id,
+                                uint64 position,
+                                uint64 size) OVERRIDE {
+    return true;
+  }
+
+  virtual bool OnRequestRedirected(int request_id,
+                                   const GURL& url,
+                                   ResourceResponse* response,
+                                   bool* defer) OVERRIDE {
+    return true;
+  }
+
+  virtual bool OnResponseStarted(int request_id,
+                                 ResourceResponse* response,
+                                 bool* defer) OVERRIDE { return true; }
+
+  virtual bool OnWillStart(int request_id,
+                           const GURL& url,
+                           bool* defer) OVERRIDE {
+    return true;
+  }
+
+  virtual bool OnWillRead(int request_id,
+                          net::IOBuffer** buf,
+                          int* buf_size,
+                          int min_size) OVERRIDE {
+    return true;
+  }
+
+  virtual bool OnReadCompleted(int request_id,
+                               int bytes_read,
+                               bool* defer) OVERRIDE {
+    return true;
+  }
+
+  virtual bool OnResponseCompleted(int request_id,
+                                   const net::URLRequestStatus& status,
+                                   const std::string& security_info) OVERRIDE {
+    return true;
+  }
+
+  virtual void OnDataDownloaded(int request_id,
+                                int bytes_downloaded) OVERRIDE {}
+};
+
+// Test browser client that captures calls to SelectClientCertificates and
+// records the arguments of the most recent call for later inspection.
+class SelectCertificateBrowserClient : public TestContentBrowserClient {
+ public:
+  SelectCertificateBrowserClient() : call_count_(0) {}
+
+  virtual void SelectClientCertificate(
+      int render_process_id,
+      int render_view_id,
+      const net::HttpNetworkSession* network_session,
+      net::SSLCertRequestInfo* cert_request_info,
+      const base::Callback<void(net::X509Certificate*)>& callback) OVERRIDE {
+    ++call_count_;
+    passed_certs_ = cert_request_info->client_certs;
+  }
+
+  int call_count() {
+    return call_count_;
+  }
+
+  net::CertificateList passed_certs() {
+    return passed_certs_;
+  }
+
+ private:
+  net::CertificateList passed_certs_;
+  int call_count_;
+};
+
+}  // namespace
+
+class ResourceLoaderTest : public testing::Test,
+                           public ResourceLoaderDelegate {
+ protected:
+  // testing::Test:
+  virtual void SetUp() OVERRIDE {
+    message_loop_.reset(new MessageLoop(MessageLoop::TYPE_IO));
+    ui_thread_.reset(new BrowserThreadImpl(BrowserThread::UI,
+                                           message_loop_.get()));
+    io_thread_.reset(new BrowserThreadImpl(BrowserThread::IO,
+                                           message_loop_.get()));
+  }
+
+  // ResourceLoaderDelegate:
+  virtual ResourceDispatcherHostLoginDelegate* CreateLoginDelegate(
+      ResourceLoader* loader,
+      net::AuthChallengeInfo* auth_info) OVERRIDE {
+    return NULL;
+  }
+  virtual bool AcceptAuthRequest(
+      ResourceLoader* loader,
+      net::AuthChallengeInfo* auth_info) OVERRIDE {
+    return false;
+  };
+  virtual bool AcceptSSLClientCertificateRequest(
+      ResourceLoader* loader,
+      net::SSLCertRequestInfo* cert_info) OVERRIDE {
+    return true;
+  }
+  virtual bool HandleExternalProtocol(ResourceLoader* loader,
+                                      const GURL& url) OVERRIDE {
+    return false;
+  }
+  virtual void DidStartRequest(ResourceLoader* loader) OVERRIDE {}
+  virtual void DidReceiveRedirect(ResourceLoader* loader,
+                                  const GURL& new_url) OVERRIDE {}
+  virtual void DidReceiveResponse(ResourceLoader* loader) OVERRIDE {}
+  virtual void DidFinishLoading(ResourceLoader* loader) OVERRIDE {}
+
+  scoped_ptr<MessageLoop> message_loop_;
+  scoped_ptr<BrowserThreadImpl> ui_thread_;
+  scoped_ptr<BrowserThreadImpl> io_thread_;
+
+  content::MockResourceContext resource_context_;
+};
+
+// When OpenSSL is used, client cert store is not being queried in
+// ResourceLoader.
+#if !defined(USE_OPENSSL)
+// Verifies if a call to net::UrlRequest::Delegate::OnCertificateRequested()
+// causes client cert store to be queried for certificates and if the returned
+// certificates are correctly passed to the content browser client for
+// selection.
+TEST_F(ResourceLoaderTest, ClientCertStoreLookup) {
+  const int kRenderProcessId = 1;
+  const int kRenderViewId = 2;
+
+  scoped_ptr<net::URLRequest> request(new net::URLRequest(
+      GURL("dummy"), NULL,
+      resource_context_.GetRequestContext()));
+  ResourceRequestInfo::AllocateForTesting(request.get(),
+                                          ResourceType::MAIN_FRAME,
+                                          &resource_context_,
+                                          kRenderProcessId,
+                                          kRenderViewId);
+
+  // Set up the test client cert store.
+  net::CertificateList dummy_certs(1, scoped_refptr<net::X509Certificate>(
+      new net::X509Certificate("test", "test", base::Time(), base::Time())));
+  scoped_ptr<ClientCertStoreStub> test_store(
+      new ClientCertStoreStub(dummy_certs));
+  EXPECT_EQ(0, test_store->request_count());
+
+  // Ownership of the |request| and |test_store| is about to be turned over to
+  // ResourceLoader. We need to keep raw pointer copies to access these objects
+  // later.
+  net::URLRequest* raw_ptr_to_request = request.get();
+  ClientCertStoreStub* raw_ptr_to_store = test_store.get();
+
+  scoped_ptr<ResourceHandler> resource_handler(new ResourceHandlerStub());
+  ResourceLoader loader(request.Pass(), resource_handler.Pass(), this,
+                        test_store.PassAs<net::ClientCertStore>());
+
+  // Prepare a dummy certificate request.
+  scoped_refptr<net::SSLCertRequestInfo> cert_request_info(
+      new net::SSLCertRequestInfo());
+  std::vector<std::string> dummy_authority(1, "dummy");
+  cert_request_info->cert_authorities = dummy_authority;
+
+  // Plug in test content browser client.
+  ContentBrowserClient* old_client = GetContentClient()->browser();
+  SelectCertificateBrowserClient test_client;
+  GetContentClient()->set_browser_for_testing(&test_client);
+
+  // Everything is set up. Trigger the resource loader certificate request event
+  // and run the message loop.
+  loader.OnCertificateRequested(raw_ptr_to_request, cert_request_info.get());
+  message_loop_->RunUntilIdle();
+
+  // Restore the original content browser client.
+  GetContentClient()->set_browser_for_testing(old_client);
+
+  // Check if the test store was queried against correct |cert_authorities|.
+  EXPECT_EQ(1, raw_ptr_to_store->request_count());
+  EXPECT_EQ(dummy_authority, raw_ptr_to_store->requested_authorities());
+
+  // Check if the retrieved certificates were passed to the content browser
+  // client.
+  EXPECT_EQ(1, test_client.call_count());
+  EXPECT_EQ(dummy_certs, test_client.passed_certs());
+}
+#endif  // !defined(OPENSSL)
+
+}  // namespace content
diff --git a/content/browser/ssl/ssl_error_handler.h b/content/browser/ssl/ssl_error_handler.h
index 8fcfeae..aa301ae 100644
--- a/content/browser/ssl/ssl_error_handler.h
+++ b/content/browser/ssl/ssl_error_handler.h
@@ -47,7 +47,7 @@ class SSLErrorHandler : public base::RefCountedThreadSafe<SSLErrorHandler> {
   // SSLManager::OnSSLCertificateError() and represents the request.
   // Finally, CancelSSLRequest() or ContinueSSLRequest() will be called after
   // SSLErrorHandler makes a decision on the SSL error.
-  class Delegate {
+  class CONTENT_EXPORT Delegate {
    public:
     // Called when SSLErrorHandler decides to cancel the request because of
     // the SSL error.
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index 9b8bb88..715fcec 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -297,6 +297,7 @@
         'browser/intents/internal_web_intents_dispatcher_unittest.cc',
         'browser/loader/resource_buffer_unittest.cc',
         'browser/loader/resource_dispatcher_host_unittest.cc',
+        'browser/loader/resource_loader_unittest.cc',
         'browser/mach_broker_mac_unittest.cc',
         'browser/media/media_internals_unittest.cc',
         'browser/media/webrtc_internals_unittest.cc',
diff --git a/content/public/browser/resource_controller.h b/content/public/browser/resource_controller.h
index db4b234..e18d0ae 100644
--- a/content/public/browser/resource_controller.h
+++ b/content/public/browser/resource_controller.h
@@ -5,6 +5,8 @@
 #ifndef CONTENT_PUBLIC_BROWSER_RESOURCE_CONTROLLER_H_
 #define CONTENT_PUBLIC_BROWSER_RESOURCE_CONTROLLER_H_
 
+#include "content/common/content_export.h"
+
 namespace content {
 
 // Used to either resume a deferred resource load or cancel a resource load at
@@ -12,7 +14,7 @@ namespace content {
 // requester of the resource to act like the request was never made.  By
 // default, load is cancelled with ERR_ABORTED code. CancelWithError can be used
 // to cancel load with any other error code.
-class ResourceController {
+class CONTENT_EXPORT ResourceController {
  public:
   virtual void Cancel() = 0;
   virtual void CancelAndIgnore() = 0;
diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index 1bd6a4b..ade9655 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -279,20 +279,6 @@ class NET_EXPORT X509Certificate
   // Does this certificate's usage allow SSL client authentication?
   bool SupportsSSLClientAuth() const;
 
-  // Do any of the given issuer names appear in this cert's chain of trust?
-  bool IsIssuedBy(const std::vector<CertPrincipal>& valid_issuers);
-
-  // Adds all available SSL client identity certs to the given vector.
-  // |server_domain| is a hint for which domain the cert is to be sent to
-  // (a cert previously specified as the default for that domain will be given
-  // precedence and returned first in the output vector.)
-  // If valid_issuers is non-empty, only certs that were transitively issued
-  // by one of the given names will be included in the list.
-  static bool GetSSLClientCertificates(
-      const std::string& server_domain,
-      const std::vector<CertPrincipal>& valid_issuers,
-      CertificateList* certs);
-
   // Creates the chain of certs to use for this client identity cert.
   CFArrayRef CreateClientCertificateChain() const;
 
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index f4aaf92..9e2565a 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -712,117 +712,6 @@ bool X509Certificate::SupportsSSLClientAuth() const {
   return true;
 }
 
-bool X509Certificate::IsIssuedBy(
-    const std::vector<CertPrincipal>& valid_issuers) {
-  // Get the cert's issuer chain.
-  CFArrayRef cert_chain = NULL;
-  OSStatus result = CopyCertChain(os_cert_handle(), &cert_chain);
-  if (result)
-    return false;
-  ScopedCFTypeRef<CFArrayRef> scoped_cert_chain(cert_chain);
-
-  // Check all the certs in the chain for a match.
-  int n = CFArrayGetCount(cert_chain);
-  for (int i = 0; i < n; ++i) {
-    SecCertificateRef cert_handle = reinterpret_cast<SecCertificateRef>(
-        const_cast<void*>(CFArrayGetValueAtIndex(cert_chain, i)));
-    scoped_refptr<X509Certificate> cert(X509Certificate::CreateFromHandle(
-        cert_handle, X509Certificate::OSCertHandles()));
-    for (unsigned j = 0; j < valid_issuers.size(); j++) {
-      if (cert->issuer().Matches(valid_issuers[j]))
-        return true;
-    }
-  }
-  return false;
-}
-
-// static
-bool X509Certificate::GetSSLClientCertificates(
-    const std::string& server_domain,
-    const std::vector<CertPrincipal>& valid_issuers,
-    CertificateList* certs) {
-  ScopedCFTypeRef<SecIdentityRef> preferred_identity;
-  if (!server_domain.empty()) {
-    // See if there's an identity preference for this domain:
-    ScopedCFTypeRef<CFStringRef> domain_str(
-        base::SysUTF8ToCFStringRef("https://" + server_domain));
-    SecIdentityRef identity = NULL;
-    // While SecIdentityCopyPreferences appears to take a list of CA issuers
-    // to restrict the identity search to, within Security.framework the
-    // argument is ignored and filtering unimplemented. See
-    // SecIdentity.cpp in libsecurity_keychain, specifically
-    // _SecIdentityCopyPreferenceMatchingName().
-    {
-      base::AutoLock lock(crypto::GetMacSecurityServicesLock());
-      if (SecIdentityCopyPreference(domain_str, 0, NULL, &identity) == noErr)
-        preferred_identity.reset(identity);
-    }
-  }
-
-  // Now enumerate the identities in the available keychains.
-  SecIdentitySearchRef search = NULL;
-  OSStatus err;
-  {
-    base::AutoLock lock(crypto::GetMacSecurityServicesLock());
-    err = SecIdentitySearchCreate(NULL, CSSM_KEYUSE_SIGN, &search);
-  }
-  if (err)
-    return false;
-  ScopedCFTypeRef<SecIdentitySearchRef> scoped_search(search);
-  while (!err) {
-    SecIdentityRef identity = NULL;
-    {
-      base::AutoLock lock(crypto::GetMacSecurityServicesLock());
-      err = SecIdentitySearchCopyNext(search, &identity);
-    }
-    if (err)
-      break;
-    ScopedCFTypeRef<SecIdentityRef> scoped_identity(identity);
-
-    SecCertificateRef cert_handle;
-    err = SecIdentityCopyCertificate(identity, &cert_handle);
-    if (err != noErr)
-      continue;
-    ScopedCFTypeRef<SecCertificateRef> scoped_cert_handle(cert_handle);
-
-    scoped_refptr<X509Certificate> cert(
-        CreateFromHandle(cert_handle, OSCertHandles()));
-    if (cert->HasExpired() || !cert->SupportsSSLClientAuth())
-      continue;
-
-    // Skip duplicates (a cert may be in multiple keychains).
-    const SHA1HashValue& fingerprint = cert->fingerprint();
-    unsigned i;
-    for (i = 0; i < certs->size(); ++i) {
-      if ((*certs)[i]->fingerprint().Equals(fingerprint))
-        break;
-    }
-    if (i < certs->size())
-      continue;
-
-    bool is_preferred = preferred_identity &&
-        CFEqual(preferred_identity, identity);
-
-    // Make sure the issuer matches valid_issuers, if given.
-    if (!valid_issuers.empty() && !cert->IsIssuedBy(valid_issuers))
-      continue;
-
-    // The cert passes, so add it to the vector.
-    // If it's the preferred identity, add it at the start (so it'll be
-    // selected by default in the UI.)
-    if (is_preferred)
-      certs->insert(certs->begin(), cert);
-    else
-      certs->push_back(cert);
-  }
-
-  if (err != errSecItemNotFound) {
-    OSSTATUS_LOG(ERROR, err) << "SecIdentitySearch error";
-    return false;
-  }
-  return true;
-}
-
 CFArrayRef X509Certificate::CreateClientCertificateChain() const {
   // Initialize the result array with just the IdentityRef of the receiver:
   SecIdentityRef identity;
diff --git a/net/base/x509_certificate_unittest.cc b/net/base/x509_certificate_unittest.cc
index 4e6c83a..5fc0bb3 100644
--- a/net/base/x509_certificate_unittest.cc
+++ b/net/base/x509_certificate_unittest.cc
@@ -681,56 +681,6 @@ TEST(X509CertificateTest, IntermediateCertificates) {
   X509Certificate::FreeOSCertHandle(google_handle);
 }
 
-#if !defined(OS_IOS)
-// TODO(ios): Not yet implemented on iOS.
-#if defined(OS_MACOSX)
-TEST(X509CertificateTest, IsIssuedBy) {
-  FilePath certs_dir = GetTestCertsDirectory();
-
-  // Test a client certificate from MIT.
-  scoped_refptr<X509Certificate> mit_davidben_cert(
-      ImportCertFromFile(certs_dir, "mit.davidben.der"));
-  ASSERT_NE(static_cast<X509Certificate*>(NULL), mit_davidben_cert);
-
-  CertPrincipal mit_issuer;
-  mit_issuer.country_name = "US";
-  mit_issuer.state_or_province_name = "Massachusetts";
-  mit_issuer.organization_names.push_back(
-      "Massachusetts Institute of Technology");
-  mit_issuer.organization_unit_names.push_back("Client CA v1");
-
-  // IsIssuedBy should return true even if it cannot build a chain
-  // with that principal.
-  std::vector<CertPrincipal> mit_issuers(1, mit_issuer);
-  EXPECT_TRUE(mit_davidben_cert->IsIssuedBy(mit_issuers));
-
-  // Test a client certificate from FOAF.ME.
-  scoped_refptr<X509Certificate> foaf_me_chromium_test_cert(
-      ImportCertFromFile(certs_dir, "foaf.me.chromium-test-cert.der"));
-  ASSERT_NE(static_cast<X509Certificate*>(NULL), foaf_me_chromium_test_cert);
-
-  CertPrincipal foaf_issuer;
-  foaf_issuer.common_name = "FOAF.ME";
-  foaf_issuer.locality_name = "Wimbledon";
-  foaf_issuer.state_or_province_name = "LONDON";
-  foaf_issuer.country_name = "GB";
-  foaf_issuer.organization_names.push_back("FOAF.ME");
-
-  std::vector<CertPrincipal> foaf_issuers(1, foaf_issuer);
-  EXPECT_TRUE(foaf_me_chromium_test_cert->IsIssuedBy(foaf_issuers));
-
-  // And test some combinations and mismatches.
-  std::vector<CertPrincipal> both_issuers;
-  both_issuers.push_back(mit_issuer);
-  both_issuers.push_back(foaf_issuer);
-  EXPECT_TRUE(foaf_me_chromium_test_cert->IsIssuedBy(both_issuers));
-  EXPECT_TRUE(mit_davidben_cert->IsIssuedBy(both_issuers));
-  EXPECT_FALSE(foaf_me_chromium_test_cert->IsIssuedBy(mit_issuers));
-  EXPECT_FALSE(mit_davidben_cert->IsIssuedBy(foaf_issuers));
-}
-#endif  // defined(OS_MACOSX)
-#endif  // !defined(OS_IOS)
-
 TEST(X509CertificateTest, IsIssuedByEncoded) {
   FilePath certs_dir = GetTestCertsDirectory();
 
diff --git a/net/data/ssl/scripts/client_authentication/generate-client-certificates.sh b/net/data/ssl/scripts/client_authentication/generate-client-certificates.sh
index 0337389..f6daddf 100755
--- a/net/data/ssl/scripts/client_authentication/generate-client-certificates.sh
+++ b/net/data/ssl/scripts/client_authentication/generate-client-certificates.sh
@@ -68,4 +68,12 @@ do
     -keyfile out/root_$id.key \
     -out out/client_$id.pem \
     -config client_authentication.cnf
+
+  # Package the client cert and private key into a pkcs12 file.
+  try openssl pkcs12 \
+    -inkey out/client_$id.key \
+    -in out/client_$id.pem \
+    -out out/client_$id.p12 \
+    -export \
+    -passout pass:
 done
diff --git a/net/data/ssl/scripts/client_authentication/run-test-server.sh b/net/data/ssl/scripts/client_authentication/run-test-server.sh
new file mode 100755
index 0000000..b5b64f4
--- /dev/null
+++ b/net/data/ssl/scripts/client_authentication/run-test-server.sh
@@ -0,0 +1,16 @@
+#!/bin/bash
+
+# Copyright (c) 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.
+
+# Helper script for local manual testing. Runs an openssl test server at
+# localhost:4433. The server will be accessible for www connections and will
+# require a client certificate issued by Client Auth Test Root 1.
+openssl s_server \
+  -accept 4433 \
+  -cert out/root_1.pem \
+  -key out/root_1.key \
+  -www \
+  -Verify 5 \
+  -CAfile out/root_1.pem
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 7c9c930..5b120f7 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -1174,16 +1174,11 @@ int HttpNetworkTransaction::HandleCertificateRequest(int error) {
   // is likely to accept, based on the criteria supplied in the
   // CertificateRequest message.
   if (client_cert) {
-    const std::vector<scoped_refptr<X509Certificate> >& client_certs =
-        response_.cert_request_info->client_certs;
-    bool cert_still_valid = false;
-    for (size_t i = 0; i < client_certs.size(); ++i) {
-      if (client_cert->Equals(client_certs[i])) {
-        cert_still_valid = true;
-        break;
-      }
-    }
+    const std::vector<std::string>& cert_authorities =
+        response_.cert_request_info->cert_authorities;
 
+    bool cert_still_valid = cert_authorities.empty() ||
+        client_cert->IsIssuedByEncoded(cert_authorities);
     if (!cert_still_valid)
       return error;
   }
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 1b33032..6ef621b 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -411,7 +411,6 @@ struct HandshakeState {
     next_proto.clear();
     server_protos.clear();
     channel_id_sent = false;
-    client_certs.clear();
     server_cert_chain.Reset(NULL);
     server_cert = NULL;
     resumed_handshake = false;
@@ -428,11 +427,6 @@ struct HandshakeState {
   // True if a channel ID was sent.
   bool channel_id_sent;
 
-  // If the peer requests client certificate authentication, the set of
-  // certificates that matched the peer's criteria. This should be soon removed
-  // as being tracked in http://crbug.com/166642.
-  CertificateList client_certs;
-
   // List of DER-encoded X.509 DistinguishedName of certificate authorities
   // allowed by the server.
   std::vector<std::string> cert_authorities;
@@ -1358,7 +1352,6 @@ SECStatus SSLClientSocketNSS::Core::PlatformClientAuthHandler(
     return SECFailure;
   }
 
-  core->nss_handshake_state_.client_certs.clear();
   core->nss_handshake_state_.cert_authorities.clear();
 
   std::vector<CERT_NAME_BLOB> issuer_list(ca_names->nnames);
@@ -1370,98 +1363,8 @@ SECStatus SSLClientSocketNSS::Core::PlatformClientAuthHandler(
         static_cast<size_t>(ca_names->names[i].len)));
   }
 
-  // Retrieve the list of matching client certificates. This is to be moved out
-  // of here as a part of refactoring effort being tracked in
-  // http://crbug.com/166642.
-
-  // Client certificates of the user are in the "MY" system certificate store.
-  HCERTSTORE my_cert_store = CertOpenSystemStore(NULL, L"MY");
-  if (!my_cert_store) {
-    PLOG(ERROR) << "Could not open the \"MY\" system certificate store";
-
-    core->AddCertProvidedEvent(0);
-    return SECFailure;
-  }
-
-  // Enumerate the client certificates.
-  CERT_CHAIN_FIND_BY_ISSUER_PARA find_by_issuer_para;
-  memset(&find_by_issuer_para, 0, sizeof(find_by_issuer_para));
-  find_by_issuer_para.cbSize = sizeof(find_by_issuer_para);
-  find_by_issuer_para.pszUsageIdentifier = szOID_PKIX_KP_CLIENT_AUTH;
-  find_by_issuer_para.cIssuer = ca_names->nnames;
-  find_by_issuer_para.rgIssuer = ca_names->nnames ? &issuer_list[0] : NULL;
-  find_by_issuer_para.pfnFindCallback = ClientCertFindCallback;
-
-  PCCERT_CHAIN_CONTEXT chain_context = NULL;
-  DWORD find_flags = CERT_CHAIN_FIND_BY_ISSUER_CACHE_ONLY_FLAG |
-                     CERT_CHAIN_FIND_BY_ISSUER_CACHE_ONLY_URL_FLAG;
-
-  for (;;) {
-    // Find a certificate chain.
-    chain_context = CertFindChainInStore(my_cert_store,
-                                         X509_ASN_ENCODING,
-                                         find_flags,
-                                         CERT_CHAIN_FIND_BY_ISSUER,
-                                         &find_by_issuer_para,
-                                         chain_context);
-    if (!chain_context) {
-      DWORD err = GetLastError();
-      if (err != CRYPT_E_NOT_FOUND)
-        DLOG(ERROR) << "CertFindChainInStore failed: " << err;
-      break;
-    }
-
-    // Get the leaf certificate.
-    PCCERT_CONTEXT cert_context =
-        chain_context->rgpChain[0]->rgpElement[0]->pCertContext;
-    // Create a copy the handle, so that we can close the "MY" certificate store
-    // before returning from this function.
-    PCCERT_CONTEXT cert_context2;
-    BOOL ok = CertAddCertificateContextToStore(NULL, cert_context,
-                                               CERT_STORE_ADD_USE_EXISTING,
-                                               &cert_context2);
-    if (!ok) {
-      NOTREACHED();
-      continue;
-    }
-
-    // Copy the rest of the chain. Copying the chain stops gracefully if an
-    // error is encountered, with the partial chain being used as the
-    // intermediates, as opposed to failing to consider the client certificate
-    // at all.
-    net::X509Certificate::OSCertHandles intermediates;
-    for (DWORD i = 1; i < chain_context->rgpChain[0]->cElement; i++) {
-      PCCERT_CONTEXT intermediate_copy;
-      ok = CertAddCertificateContextToStore(
-          NULL, chain_context->rgpChain[0]->rgpElement[i]->pCertContext,
-          CERT_STORE_ADD_USE_EXISTING, &intermediate_copy);
-      if (!ok) {
-        NOTREACHED();
-        break;
-      }
-      intermediates.push_back(intermediate_copy);
-    }
-
-    scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromHandle(
-        cert_context2, intermediates);
-    core->nss_handshake_state_.client_certs.push_back(cert);
-
-    X509Certificate::FreeOSCertHandle(cert_context2);
-    for (net::X509Certificate::OSCertHandles::iterator it =
-        intermediates.begin(); it != intermediates.end(); ++it) {
-      net::X509Certificate::FreeOSCertHandle(*it);
-    }
-  }
-
-  std::sort(core->nss_handshake_state_.client_certs.begin(),
-            core->nss_handshake_state_.client_certs.end(),
-            x509_util::ClientCertSorter());
-
-  BOOL ok = CertCloseStore(my_cert_store, CERT_CLOSE_STORE_CHECK_FLAG);
-  DCHECK(ok);
-
   // Update the network task runner's view of the handshake state now that
-  // client certs have been detected.
+  // server certificate request has been recorded.
   core->PostOrRunCallback(
       FROM_HERE, base::Bind(&Core::OnHandshakeStateUpdated, core,
                             core->nss_handshake_state_));
@@ -1545,40 +1448,19 @@ SECStatus SSLClientSocketNSS::Core::PlatformClientAuthHandler(
     return SECFailure;
   }
 
-  core->nss_handshake_state_.client_certs.clear();
   core->nss_handshake_state_.cert_authorities.clear();
 
-  // Retrieve the cert issuers accepted by the server. This information is
-  // currently (temporarily) being saved both in |valid_issuers| and
-  // |cert_authorities|, the latter being the target solution. The refactoring
-  // effort is being tracked in http://crbug.com/166642.
+  // Retrieve the cert issuers accepted by the server.
   std::vector<CertPrincipal> valid_issuers;
   int n = ca_names->nnames;
   for (int i = 0; i < n; i++) {
-    // Add the DER-encoded issuer DistinguishedName to |cert_authorities|.
     core->nss_handshake_state_.cert_authorities.push_back(std::string(
         reinterpret_cast<const char*>(ca_names->names[i].data),
         static_cast<size_t>(ca_names->names[i].len)));
-    // Add the CertPrincipal object representing the issuer to
-    // |valid_issuers|.
-    CertPrincipal p;
-    if (p.ParseDistinguishedName(ca_names->names[i].data,
-                                 ca_names->names[i].len)) {
-      valid_issuers.push_back(p);
-    }
   }
 
-  // Now get the available client certs whose issuers are allowed by the server.
-  X509Certificate::GetSSLClientCertificates(
-      core->host_and_port_.host(), valid_issuers,
-      &core->nss_handshake_state_.client_certs);
-
-  std::sort(core->nss_handshake_state_.client_certs.begin(),
-            core->nss_handshake_state_.client_certs.end(),
-            x509_util::ClientCertSorter());
-
   // Update the network task runner's view of the handshake state now that
-  // client certs have been detected.
+  // server certificate request has been recorded.
   core->PostOrRunCallback(
       FROM_HERE, base::Bind(&Core::OnHandshakeStateUpdated, core,
                             core->nss_handshake_state_));
@@ -1663,7 +1545,6 @@ SECStatus SSLClientSocketNSS::Core::ClientAuthHandler(
   }
 
   // First pass: client certificate is needed.
-  core->nss_handshake_state_.client_certs.clear();
   core->nss_handshake_state_.cert_authorities.clear();
 
   // Retrieve the DER-encoded DistinguishedName of the cert issuers accepted by
@@ -1674,45 +1555,8 @@ SECStatus SSLClientSocketNSS::Core::ClientAuthHandler(
         static_cast<size_t>(ca_names->names[i].len)));
   }
 
-  // Iterate over all client certificates and put the ones matching the server
-  // criteria in |nss_handshake_state_.client_certs|. This is to be moved out of
-  // here as a part of refactoring effort being tracked in
-  // http://crbug.com/166642.
-  CERTCertList* client_certs = CERT_FindUserCertsByUsage(
-      CERT_GetDefaultCertDB(), certUsageSSLClient,
-      PR_FALSE, PR_FALSE, wincx);
-  if (client_certs) {
-    for (CERTCertListNode* node = CERT_LIST_HEAD(client_certs);
-         !CERT_LIST_END(node, client_certs);
-         node = CERT_LIST_NEXT(node)) {
-      // Only offer unexpired certificates.
-      if (CERT_CheckCertValidTimes(node->cert, PR_Now(), PR_TRUE) !=
-          secCertTimeValid) {
-        continue;
-      }
-      // Filter by issuer.
-      //
-      // TODO(davidben): This does a binary comparison of the DER-encoded
-      // issuers. We should match according to RFC 5280 sec. 7.1. We should find
-      // an appropriate NSS function or add one if needbe.
-      if (ca_names->nnames &&
-          NSS_CmpCertChainWCANames(node->cert, ca_names) != SECSuccess) {
-        continue;
-      }
-
-      X509Certificate* x509_cert = X509Certificate::CreateFromHandle(
-          node->cert, net::X509Certificate::OSCertHandles());
-      core->nss_handshake_state_.client_certs.push_back(x509_cert);
-    }
-    CERT_DestroyCertList(client_certs);
-  }
-
-  std::sort(core->nss_handshake_state_.client_certs.begin(),
-            core->nss_handshake_state_.client_certs.end(),
-            x509_util::ClientCertSorter());
-
   // Update the network task runner's view of the handshake state now that
-  // client certs have been detected.
+  // server certificate request has been recorded.
   core->PostOrRunCallback(
       FROM_HERE, base::Bind(&Core::OnHandshakeStateUpdated, core,
                             core->nss_handshake_state_));
@@ -2924,9 +2768,7 @@ void SSLClientSocketNSS::GetSSLCertRequestInfo(
   // TODO(rch): switch SSLCertRequestInfo.host_and_port to a HostPortPair
   cert_request_info->host_and_port = host_and_port_.ToString();
   cert_request_info->cert_authorities = core_->state().cert_authorities;
-  // This should be removed as being tracked in http://crbug.com/166642.
-  cert_request_info->client_certs = core_->state().client_certs;
-  LeaveFunction(cert_request_info->client_certs.size());
+  LeaveFunction("");
 }
 
 int SSLClientSocketNSS::ExportKeyingMaterial(const base::StringPiece& label,
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index 47e85c9..e14527c 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -652,7 +652,6 @@ void SSLClientSocketOpenSSL::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
   cert_request_info->host_and_port = host_and_port_.ToString();
   cert_request_info->cert_authorities = cert_authorities_;
-  cert_request_info->client_certs = client_certs_;
 }
 
 int SSLClientSocketOpenSSL::ExportKeyingMaterial(
@@ -771,7 +770,6 @@ void SSLClientSocketOpenSSL::Disconnect() {
   completed_handshake_ = false;
 
   cert_authorities_.clear();
-  client_certs_.clear();
   client_auth_cert_needed_ = false;
 }
 
diff --git a/net/socket/ssl_client_socket_openssl.h b/net/socket/ssl_client_socket_openssl.h
index b0f61c7..12d9229 100644
--- a/net/socket/ssl_client_socket_openssl.h
+++ b/net/socket/ssl_client_socket_openssl.h
@@ -151,9 +151,6 @@ class SSLClientSocketOpenSSL : public SSLClientSocket {
   // List of DER-encoded X.509 DistinguishedName of certificate authorities
   // allowed by the server.
   std::vector<std::string> cert_authorities_;
-  // Set of certificates that matches the server criteria. This should be
-  // removed soon as being tracked in http://crbug.com/166642.
-  std::vector<scoped_refptr<X509Certificate> > client_certs_;
 
   CertVerifier* const cert_verifier_;
   scoped_ptr<SingleRequestCertVerifier> verifier_;
diff --git a/net/socket_stream/socket_stream.cc b/net/socket_stream/socket_stream.cc
index bf53d17..158cff3 100644
--- a/net/socket_stream/socket_stream.cc
+++ b/net/socket_stream/socket_stream.cc
@@ -1168,15 +1168,10 @@ int SocketStream::HandleCertificateRequest(int result, SSLConfig* ssl_config) {
   if (!client_cert)
     return result;
 
-  const std::vector<scoped_refptr<X509Certificate> >& client_certs =
-      cert_request_info->client_certs;
-  bool cert_still_valid = false;
-  for (size_t i = 0; i < client_certs.size(); ++i) {
-    if (client_cert->Equals(client_certs[i])) {
-      cert_still_valid = true;
-      break;
-    }
-  }
+  const std::vector<std::string>& cert_authorities =
+      cert_request_info->cert_authorities;
+  bool cert_still_valid = cert_authorities.empty() ||
+      client_cert->IsIssuedByEncoded(cert_authorities);
   if (!cert_still_valid)
     return result;