Get ClientCertStore through ResourceContext.

net/ssl/client_cert_store_impl_nss.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

[wtc, 2013/10/28 19:41:12]

My comments in this file also apply to the _mac.cc and _win.cc files.

[mattm, 2013/10/28 23:56:16]

Done.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/ssl/client_cert_store_impl.h"
 
 #include <nss.h>
 #include <ssl.h>
 
+#include "base/callback.h"
 #include "base/logging.h"
 #include "net/cert/x509_util.h"
 
 namespace net {
 
 namespace {
 
 // Examines the certificates in |cert_list| to find all certificates that match
 // the client certificate request in |request|, storing the matching
 // certificates in |selected_certs|.
 // If |query_nssdb| is true, NSS will be queried to construct full certificate
 // chains. If it is false, only the certificate will be considered.
 bool GetClientCertsImpl(CERTCertList* cert_list,

[wtc, 2013/10/28 19:41:12]

This function always returns true. It should be changed to return void. This
will avoid the question of why GetClientCerts can ignore the return value of
this function.

[mattm, 2013/10/28 23:56:16]

Well, the return value is still checked by the unittest which calls
SelectClientCertsForTesting instead of GetClientCerts.

[wtc, 2013/10/29 23:03:12]

I checked all three implementations of GetClientCertsImpl. All of them always
return true. So the return value isn't useful, even if some caller checks it.

[mattm, 2013/10/29 23:59:00]

Ah, good point.  Done.

                         const SSLCertRequestInfo& request,
                         bool query_nssdb,
                         CertificateList* selected_certs) {
   DCHECK(cert_list);
   DCHECK(selected_certs);
 
   selected_certs->clear();
 
   // Create a "fake" CERTDistNames structure. No public API exists to create
   // one from a list of issuers.
@@ skipping 37 matching lines @@
     }
   }
 
   std::sort(selected_certs->begin(), selected_certs->end(),
             x509_util::ClientCertSorter());
   return true;
 }
 
 }  // namespace
 
-bool ClientCertStoreImpl::GetClientCerts(const SSLCertRequestInfo& request,
-                                         CertificateList* selected_certs) {
+void ClientCertStoreImpl::GetClientCerts(const SSLCertRequestInfo& request,
+                                         CertificateList* selected_certs,
+                                         const base::Closure& callback) {
   CERTCertList* client_certs = CERT_FindUserCertsByUsage(
       CERT_GetDefaultCertDB(), certUsageSSLClient,
       PR_FALSE, PR_FALSE, NULL);
   // It is ok for a user not to have any client certs.
-  if (!client_certs)
-    return true;
+  if (!client_certs) {
+    callback.Run();

[wtc, 2013/10/28 19:41:12]

Nit: it may be a good idea to clear selected_certs explicitly in this case. I
believe we are relying on the caller passing an empty selected_certs to us.

[mattm, 2013/10/28 23:56:16]

Done.

+    return;
+  }
 
-  bool rv = GetClientCertsImpl(client_certs, request, true, selected_certs);
+  GetClientCertsImpl(client_certs, request, true, selected_certs);
   CERT_DestroyCertList(client_certs);
-  return rv;
+  callback.Run();
 }
 
 bool ClientCertStoreImpl::SelectClientCertsForTesting(
     const CertificateList& input_certs,
     const SSLCertRequestInfo& request,
     CertificateList* selected_certs) {
   CERTCertList* cert_list = CERT_NewCertList();
   if (!cert_list)
     return false;
   for (size_t i = 0; i < input_certs.size(); ++i) {
     CERT_AddCertToListTail(
         cert_list, CERT_DupCertificate(input_certs[i]->os_cert_handle()));
   }
 
   bool rv = GetClientCertsImpl(cert_list, request, false, selected_certs);
   CERT_DestroyCertList(cert_list);
   return rv;
 }
 
 }  // namespace net