Refactor client cert private key handling.

net/ssl/client_cert_store_nss_unittest.cc

Index: net/ssl/client_cert_store_nss_unittest.cc
diff --git a/net/ssl/client_cert_store_nss_unittest.cc b/net/ssl/client_cert_store_nss_unittest.cc
index 8bfae5313f81bdb3d5e3b47baf89a6224f73d49b..aa77182861a15cc82536f57f4bf91bbb429b0fc7 100644
--- a/net/ssl/client_cert_store_nss_unittest.cc
+++ b/net/ssl/client_cert_store_nss_unittest.cc
@@ -12,12 +12,17 @@
 #include <string>
 
 #include "base/bind.h"
+#include "base/files/file_util.h"
+#include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/run_loop.h"
 #include "crypto/scoped_test_nss_db.h"
 #include "net/cert/x509_certificate.h"
+#include "net/ssl/client_cert_identity_test_util.h"
 #include "net/ssl/client_cert_store_unittest-inl.h"
 #include "net/ssl/ssl_cert_request_info.h"
+#include "net/ssl/ssl_private_key.h"
+#include "net/ssl/ssl_private_key_test_util.h"
 #include "net/test/cert_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -25,10 +30,17 @@ namespace net {
 
 namespace {
 
-void SaveCertsAndQuitCallback(CertificateList* out_certs,
-                              base::Closure quit_closure,
-                              CertificateList in_certs) {
-  *out_certs = std::move(in_certs);
+void SaveIdentitiesAndQuitCallback(ClientCertIdentityList* out_identities,
+                                   base::Closure quit_closure,
+                                   ClientCertIdentityList in_identities) {
+  *out_identities = std::move(in_identities);
+  quit_closure.Run();
+}
+
+void SavePrivateKeyAndQuitCallback(scoped_refptr<net::SSLPrivateKey>* out_key,
+                                   base::Closure quit_closure,
+                                   scoped_refptr<net::SSLPrivateKey> in_key) {
+  *out_key = std::move(in_key);
   quit_closure.Run();
 }
 
@@ -40,11 +52,14 @@ class ClientCertStoreNSSTestDelegate {
 
   bool SelectClientCerts(const CertificateList& input_certs,
                          const SSLCertRequestInfo& cert_request_info,
-                         CertificateList* selected_certs) {
-    // Filters |input_certs| using the logic being used to filter the system
-    // store when GetClientCerts() is called.
-    ClientCertStoreNSS::FilterCertsOnWorkerThread(
-        input_certs, cert_request_info, selected_certs);
+                         ClientCertIdentityList* selected_identities) {
+    *selected_identities =
+        FakeClientCertIdentityListFromCertificateList(input_certs);
+
+    // Filters |selected_identities| using the logic being used to filter the
+    // system store when GetClientCerts() is called.
+    ClientCertStoreNSS::FilterCertsOnWorkerThread(selected_identities,
+                                                  cert_request_info);
     return true;
   }
 };
@@ -68,6 +83,9 @@ TEST(ClientCertStoreNSSTest, BuildsCertificateChain) {
             PK11_ImportCert(test_db.slot(), client_1_ca->os_cert_handle(),
                             CK_INVALID_HANDLE, "client_1_ca",
                             PR_FALSE /* includeTrust (unused) */));
+  std::string pkcs8_key;
+  ASSERT_TRUE(base::ReadFileToString(
+      GetTestCertsDirectory().AppendASCII("client_1.pk8"), &pkcs8_key));
 
   std::unique_ptr<ClientCertStoreNSS> store(
       new ClientCertStoreNSS(ClientCertStoreNSS::PasswordDelegateFactory()));
@@ -78,19 +96,29 @@ TEST(ClientCertStoreNSSTest, BuildsCertificateChain) {
     request->cert_authorities.push_back(std::string(
         reinterpret_cast<const char*>(kAuthority1DN), sizeof(kAuthority1DN)));
 
-    CertificateList selected_certs;
+    ClientCertIdentityList selected_identities;
     base::RunLoop loop;
     store->GetClientCerts(*request.get(),
-                          base::Bind(SaveCertsAndQuitCallback, &selected_certs,
-                                     loop.QuitClosure()));
+                          base::Bind(SaveIdentitiesAndQuitCallback,
+                                     &selected_identities, loop.QuitClosure()));
     loop.Run();
 
     // The result be |client_1| with no intermediates.
-    ASSERT_EQ(1u, selected_certs.size());
-    scoped_refptr<X509Certificate> selected_cert = selected_certs[0];
+    ASSERT_EQ(1u, selected_identities.size());
+    scoped_refptr<X509Certificate> selected_cert =
+        selected_identities[0]->certificate();
     EXPECT_TRUE(X509Certificate::IsSameOSCert(client_1->os_cert_handle(),
                                               selected_cert->os_cert_handle()));
     ASSERT_EQ(0u, selected_cert->GetIntermediateCertificates().size());
+
+    scoped_refptr<SSLPrivateKey> ssl_private_key;
+    base::RunLoop key_loop;
+    selected_identities[0]->AcquirePrivateKey(
+        base::Bind(SavePrivateKeyAndQuitCallback, &ssl_private_key,
+                   key_loop.QuitClosure()));
+    key_loop.Run();
+    ASSERT_TRUE(ssl_private_key);
+    TestSSLPrivateKeyMatches(ssl_private_key.get(), pkcs8_key);

[davidben, 2017/06/01 23:41:25]

Nice!

(In the ssl_platform_key_* tests, this function is paired with a supported hash
functions check since TestSSLPrivateKeyMatches only checks whatever the key
advertises, which might be empty. But perhaps that's not worth the trouble.)

[mattm, 2017/06/02 04:04:20]

Done.

   }
 
   {
@@ -100,23 +128,35 @@ TEST(ClientCertStoreNSSTest, BuildsCertificateChain) {
         std::string(reinterpret_cast<const char*>(kAuthorityRootDN),
                     sizeof(kAuthorityRootDN)));
 
-    CertificateList selected_certs;
+    ClientCertIdentityList selected_identities;
     base::RunLoop loop;
     store->GetClientCerts(*request.get(),
-                          base::Bind(SaveCertsAndQuitCallback, &selected_certs,
-                                     loop.QuitClosure()));
+                          base::Bind(SaveIdentitiesAndQuitCallback,
+                                     &selected_identities, loop.QuitClosure()));
     loop.Run();
 
     // The result be |client_1| with |client_1_ca| as an intermediate.
-    ASSERT_EQ(1u, selected_certs.size());
-    scoped_refptr<X509Certificate> selected_cert = selected_certs[0];
+    ASSERT_EQ(1u, selected_identities.size());
+    scoped_refptr<X509Certificate> selected_cert =
+        selected_identities[0]->certificate();
     EXPECT_TRUE(X509Certificate::IsSameOSCert(client_1->os_cert_handle(),
                                               selected_cert->os_cert_handle()));
     ASSERT_EQ(1u, selected_cert->GetIntermediateCertificates().size());
     EXPECT_TRUE(X509Certificate::IsSameOSCert(
         client_1_ca->os_cert_handle(),
         selected_cert->GetIntermediateCertificates()[0]));
+
+    scoped_refptr<SSLPrivateKey> ssl_private_key;
+    base::RunLoop key_loop;
+    selected_identities[0]->AcquirePrivateKey(
+        base::Bind(SavePrivateKeyAndQuitCallback, &ssl_private_key,
+                   key_loop.QuitClosure()));
+    key_loop.Run();
+    ASSERT_TRUE(ssl_private_key);
+    TestSSLPrivateKeyMatches(ssl_private_key.get(), pkcs8_key);
   }
 }
 
+// TODO(mattm): is it possible to unittest slot unlocking?
+
 }  // namespace net