Make CertificateProviderService vend ClientCertIdentities directly.

chrome/browser/chromeos/certificate_provider/certificate_provider_service_unittest.cc

 // Copyright 2015 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/certificate_provider/certificate_provider_service.h"
 
 #include <stdint.h>
 #include <set>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/stl_util.h"
 #include "base/test/test_mock_time_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/chromeos/certificate_provider/certificate_provider.h"
 #include "net/base/net_errors.h"
-#include "net/ssl/client_key_store.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/test_data_directory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
 
 namespace {
 
 const char kExtension1[] = "extension1";
 const char kExtension2[] = "extension2";
 
 void ExpectEmptySignatureAndStoreError(net::Error* out_error,
                                        net::Error error,
                                        const std::vector<uint8_t>& signature) {
   EXPECT_TRUE(signature.empty());
   *out_error = error;
 }
 
 void ExpectOKAndStoreSignature(std::vector<uint8_t>* out_signature,
                                net::Error error,
                                const std::vector<uint8_t>& signature) {
   EXPECT_EQ(net::OK, error);
   *out_signature = signature;
 }
 
-void StoreCertificates(net::CertificateList* out_certs,
-                       const net::CertificateList& certs) {
+void StoreCertificates(net::ClientCertIdentityList* out_certs,
+                       net::ClientCertIdentityList certs) {
   if (out_certs)
-    *out_certs = certs;
+    *out_certs = std::move(certs);
+}
+
+void SavePrivateKey(scoped_refptr<net::SSLPrivateKey>* out_key,

[davidben, 2017/06/16 00:28:25]

Nit: vaguely odd that this and the above function do exactly the same thing but
are named differently.

[mattm, 2017/06/16 22:34:55]

Done.

+                    scoped_refptr<net::SSLPrivateKey> in_key) {
+  *out_key = std::move(in_key);
 }
 
 certificate_provider::CertificateInfo CreateCertInfo(
     const std::string& cert_filename) {
   certificate_provider::CertificateInfo cert_info;
   cert_info.certificate =
       net::ImportCertFromFile(net::GetTestCertsDirectory(), cert_filename);
   EXPECT_TRUE(cert_info.certificate) << "Could not load " << cert_filename;
   cert_info.supported_hashes.push_back(net::SSLPrivateKey::Hash::SHA256);
 
   return cert_info;
 }
 
 bool IsKeyEqualToCertInfo(const certificate_provider::CertificateInfo& info,
                           net::SSLPrivateKey* key) {
   return info.supported_hashes == key->GetDigestPreferences();
 }
 
+bool ClientCertIdentityAlphabeticSorter(
+    const std::unique_ptr<net::ClientCertIdentity>& a_identity,
+    const std::unique_ptr<net::ClientCertIdentity>& b_identity) {
+  return a_identity->certificate()->subject().GetDisplayName() <
+         b_identity->certificate()->subject().GetDisplayName();
+}
+
 class TestDelegate : public CertificateProviderService::Delegate {
  public:
   enum class RequestType { NONE, SIGN, GET_CERTIFICATES };
 
   TestDelegate() {}
 
   std::vector<std::string> CertificateProviderExtensions() override {
     return std::vector<std::string>(provider_extensions_.begin(),
                                     provider_extensions_.end());
   }
@@ skipping 42 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(TestDelegate);
 };
 
 }  // namespace
 
 class CertificateProviderServiceTest : public testing::Test {
  public:
   CertificateProviderServiceTest()
       : task_runner_(new base::TestMockTimeTaskRunner()),
         task_runner_handle_(task_runner_),
-        client_key_store_(net::ClientKeyStore::GetInstance()),
         service_(new CertificateProviderService()),
         cert_info1_(CreateCertInfo("client_1.pem")),
         cert_info2_(CreateCertInfo("client_2.pem")) {
     std::unique_ptr<TestDelegate> test_delegate(new TestDelegate);
     test_delegate_ = test_delegate.get();
     service_->SetDelegate(std::move(test_delegate));
 
     certificate_provider_ = service_->CreateCertificateProvider();
     EXPECT_TRUE(certificate_provider_);
 
     test_delegate_->provider_extensions_.insert(kExtension1);
   }
 
   // Triggers a GetCertificates request and returns the request id. Assumes that
   // at least one extension is registered as a certificate provider.
-  int RequestCertificatesFromExtensions(net::CertificateList* certs) {
+  int RequestCertificatesFromExtensions(net::ClientCertIdentityList* certs) {
     test_delegate_->ClearAndExpectRequest(
         TestDelegate::RequestType::GET_CERTIFICATES);
 
     certificate_provider_->GetCertificates(
         base::Bind(&StoreCertificates, certs));
 
     task_runner_->RunUntilIdle();
     EXPECT_EQ(TestDelegate::RequestType::NONE,
               test_delegate_->expected_request_type_);
     return test_delegate_->last_cert_request_id_;
   }
 
+  scoped_refptr<net::SSLPrivateKey> FetchIdentityPrivateKey(
+      net::ClientCertIdentity* identity) {
+    scoped_refptr<net::SSLPrivateKey> ssl_private_key;
+    identity->AcquirePrivateKey(base::Bind(SavePrivateKey, &ssl_private_key));
+    task_runner_->RunUntilIdle();
+    return ssl_private_key;
+  }
+
   // Provides |cert_info1_| through kExtension1.
-  void ProvideDefaultCert() {
-    const int cert_request_id = RequestCertificatesFromExtensions(nullptr);
+  std::unique_ptr<net::ClientCertIdentity> ProvideDefaultCert() {
+    net::ClientCertIdentityList certs;
+    const int cert_request_id = RequestCertificatesFromExtensions(&certs);
     SetCertificateProvidedByExtension(kExtension1, cert_request_id,
                                       cert_info1_);
     task_runner_->RunUntilIdle();
+    if (certs.empty())
+      return nullptr;
+    return std::move(certs[0]);
   }
 
   // Like service_->SetCertificatesProvidedByExtension but taking a single
   // CertificateInfo instead of a list.
   void SetCertificateProvidedByExtension(
       const std::string& extension_id,
       int cert_request_id,
       const certificate_provider::CertificateInfo& cert_info) {
     certificate_provider::CertificateInfoList infos;
     infos.push_back(cert_info);
@@ skipping 21 matching lines @@
     if (expected_extension_id != extension_id) {
       LOG(ERROR) << "Wrong extension id. Got " << extension_id;
       return false;
     }
     return true;
   }
 
   scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
   base::ThreadTaskRunnerHandle task_runner_handle_;
   TestDelegate* test_delegate_ = nullptr;
-  net::ClientKeyStore* const client_key_store_;
   std::unique_ptr<CertificateProvider> certificate_provider_;
   std::unique_ptr<CertificateProviderService> service_;
   const certificate_provider::CertificateInfo cert_info1_;
   const certificate_provider::CertificateInfo cert_info2_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(CertificateProviderServiceTest);
 };
 
 TEST_F(CertificateProviderServiceTest, GetCertificates) {
   test_delegate_->provider_extensions_.insert(kExtension2);
 
-  net::CertificateList certs;
+  net::ClientCertIdentityList certs;
   const int cert_request_id = RequestCertificatesFromExtensions(&certs);
 
   task_runner_->RunUntilIdle();
   // No certificates set until all registered extensions replied.
   EXPECT_TRUE(certs.empty());
 
   SetCertificateProvidedByExtension(kExtension1, cert_request_id, cert_info1_);
 
   task_runner_->RunUntilIdle();
   // No certificates set until all registered extensions replied.
   EXPECT_TRUE(certs.empty());
 
   SetCertificateProvidedByExtension(kExtension2, cert_request_id, cert_info2_);
 
   task_runner_->RunUntilIdle();
-  EXPECT_EQ(2u, certs.size());
+  ASSERT_EQ(2u, certs.size());
 
-  // Verify that the ClientKeyStore returns key handles for the provide certs.
-  EXPECT_TRUE(
-      client_key_store_->FetchClientCertPrivateKey(*cert_info1_.certificate));
-  EXPECT_TRUE(
-      client_key_store_->FetchClientCertPrivateKey(*cert_info2_.certificate));
+  // Verify that the ClientCertIdentity returns key handles for the provided
+  // certs.
+  EXPECT_TRUE(FetchIdentityPrivateKey(certs[0].get()));
+  EXPECT_TRUE(FetchIdentityPrivateKey(certs[1].get()));
 
   // Deregister the extensions as certificate providers. The next
   // GetCertificates call must report an empty list of certs.
   test_delegate_->provider_extensions_.clear();
 
   // No request expected.
   test_delegate_->ClearAndExpectRequest(TestDelegate::RequestType::NONE);
 
   certificate_provider_->GetCertificates(
       base::Bind(&StoreCertificates, &certs));
@@ skipping 92 matching lines @@
                                      std::string()));
 
   EXPECT_TRUE(CheckLookUpCertificate(cert_info2_, true /* is known */,
                                      true /* is currently provided */,
                                      kExtension1));
 }
 
 TEST_F(CertificateProviderServiceTest, GetCertificatesTimeout) {
   test_delegate_->provider_extensions_.insert(kExtension2);
 
-  net::CertificateList certs;
+  net::ClientCertIdentityList certs;
   const int cert_request_id = RequestCertificatesFromExtensions(&certs);
 
   certificate_provider::CertificateInfoList infos;
   SetCertificateProvidedByExtension(kExtension1, cert_request_id, cert_info1_);
 
   task_runner_->RunUntilIdle();
   // No certificates set until all registered extensions replied or a timeout
   // occurred.
   EXPECT_TRUE(certs.empty());
 
   task_runner_->FastForwardUntilNoTasksRemain();
   // After the timeout, only extension1_'s certificates are returned.
   // This verifies that the timeout delay is > 0 but not how long the delay is.
-  EXPECT_EQ(1u, certs.size());
+  ASSERT_EQ(1u, certs.size());
 
-  EXPECT_TRUE(
-      client_key_store_->FetchClientCertPrivateKey(*cert_info1_.certificate));
+  EXPECT_TRUE(FetchIdentityPrivateKey(certs[0].get()));
 }
 
 TEST_F(CertificateProviderServiceTest, UnloadExtensionAfterGetCertificates) {
   test_delegate_->provider_extensions_.insert(kExtension2);
 
-  const int cert_request_id = RequestCertificatesFromExtensions(nullptr);
+  net::ClientCertIdentityList certs;
+  const int cert_request_id = RequestCertificatesFromExtensions(&certs);
 
   SetCertificateProvidedByExtension(kExtension1, cert_request_id, cert_info1_);
   SetCertificateProvidedByExtension(kExtension2, cert_request_id, cert_info2_);
   task_runner_->RunUntilIdle();
 
+  ASSERT_EQ(2u, certs.size());
+
+  // Sort the returned certs to ensure that the test results are stable.
+  std::sort(certs.begin(), certs.end(), ClientCertIdentityAlphabeticSorter);
+
   // Private key handles for both certificates must be available now.
-  EXPECT_TRUE(
-      client_key_store_->FetchClientCertPrivateKey(*cert_info1_.certificate));
-  EXPECT_TRUE(
-      client_key_store_->FetchClientCertPrivateKey(*cert_info2_.certificate));
+  EXPECT_TRUE(FetchIdentityPrivateKey(certs[0].get()));
+  EXPECT_TRUE(FetchIdentityPrivateKey(certs[1].get()));
 
   // Unload one of the extensions.
   service_->OnExtensionUnloaded(kExtension2);
 
   // extension1 isn't affected by the uninstall.
-  EXPECT_TRUE(
-      client_key_store_->FetchClientCertPrivateKey(*cert_info1_.certificate));
+  EXPECT_TRUE(FetchIdentityPrivateKey(certs[0].get()));
   // No key handles that were backed by the uninstalled extension must be
   // returned.
-  EXPECT_FALSE(
-      client_key_store_->FetchClientCertPrivateKey(*cert_info2_.certificate));
+  EXPECT_FALSE(FetchIdentityPrivateKey(certs[1].get()));
+}
+
+TEST_F(CertificateProviderServiceTest, DestroyServiceAfterGetCertificates) {
+  test_delegate_->provider_extensions_.insert(kExtension2);
+
+  net::ClientCertIdentityList certs;
+  const int cert_request_id = RequestCertificatesFromExtensions(&certs);
+
+  SetCertificateProvidedByExtension(kExtension1, cert_request_id, cert_info1_);
+  SetCertificateProvidedByExtension(kExtension2, cert_request_id, cert_info2_);
+  task_runner_->RunUntilIdle();
+
+  ASSERT_EQ(2u, certs.size());
+
+  // Destroy the service.
+  service_.reset();
+
+  // Private key handles for both certificates should return nullptr now.
+  EXPECT_FALSE(FetchIdentityPrivateKey(certs[0].get()));
+  EXPECT_FALSE(FetchIdentityPrivateKey(certs[1].get()));
 }
 
 TEST_F(CertificateProviderServiceTest, UnloadExtensionDuringGetCertificates) {
   test_delegate_->provider_extensions_.insert(kExtension2);
 
-  net::CertificateList certs;
+  net::ClientCertIdentityList certs;
   const int cert_request_id = RequestCertificatesFromExtensions(&certs);
 
   SetCertificateProvidedByExtension(kExtension1, cert_request_id, cert_info1_);
 
   // The pending certificate request is only waiting for kExtension2. Unloading
   // that extension must cause the request to be finished.
   service_->OnExtensionUnloaded(kExtension2);
 
   task_runner_->RunUntilIdle();
   EXPECT_EQ(1u, certs.size());
 }
 
 // Trying to sign data using the exposed SSLPrivateKey must cause a sign
 // request. The reply must be correctly routed back to the private key.
 TEST_F(CertificateProviderServiceTest, SignRequest) {
-  ProvideDefaultCert();
+  std::unique_ptr<net::ClientCertIdentity> cert(ProvideDefaultCert());

[davidben, 2017/06/16 00:28:25]

ASSERT_TRUE(cert)?

[mattm, 2017/06/16 22:34:55]

Done.

 
   scoped_refptr<net::SSLPrivateKey> private_key(
-      client_key_store_->FetchClientCertPrivateKey(*cert_info1_.certificate));
+      FetchIdentityPrivateKey(cert.get()));
 
   ASSERT_TRUE(private_key);
   EXPECT_TRUE(IsKeyEqualToCertInfo(cert_info1_, private_key.get()));
 
   test_delegate_->ClearAndExpectRequest(TestDelegate::RequestType::SIGN);
 
   std::vector<uint8_t> received_signature;
   private_key->SignDigest(
       net::SSLPrivateKey::Hash::SHA256, std::string("any input data"),
       base::Bind(&ExpectOKAndStoreSignature, &received_signature));
@@ skipping 13 matching lines @@
   signature_reply.push_back(5);
   signature_reply.push_back(7);
   signature_reply.push_back(8);
   service_->ReplyToSignRequest(kExtension1, sign_request_id, signature_reply);
 
   task_runner_->RunUntilIdle();
   EXPECT_EQ(signature_reply, received_signature);
 }
 
 TEST_F(CertificateProviderServiceTest, UnloadExtensionDuringSign) {
-  ProvideDefaultCert();
+  std::unique_ptr<net::ClientCertIdentity> cert(ProvideDefaultCert());

[davidben, 2017/06/16 00:28:25]

Ditto.

[mattm, 2017/06/16 22:34:55]

Done.

 
   scoped_refptr<net::SSLPrivateKey> private_key(
-      client_key_store_->FetchClientCertPrivateKey(*cert_info1_.certificate));
+      FetchIdentityPrivateKey(cert.get()));
   ASSERT_TRUE(private_key);
 
   test_delegate_->ClearAndExpectRequest(TestDelegate::RequestType::SIGN);
 
   net::Error error = net::OK;
   private_key->SignDigest(
       net::SSLPrivateKey::Hash::SHA256, std::string("any input data"),
       base::Bind(&ExpectEmptySignatureAndStoreError, &error));
 
   task_runner_->RunUntilIdle();
 
   // No signature received until the extension replied to the service or is
   // unloaded.
   EXPECT_EQ(net::OK, error);
 
   // Unload the extension.
   service_->OnExtensionUnloaded(kExtension1);
 
   task_runner_->RunUntilIdle();
   EXPECT_EQ(net::ERR_FAILED, error);
 }
 
 }  // namespace chromeos