arc: use fake private key for ARC command.

Install a fake RSA key mapped to the real subject public key info to
ARC certificate store via ARC command.
Later on the key source will be recognized by the dummy subject public
key info.

Bug: b:119914122
Test: ./unit_tests --gtest_filter=Arc*
Change-Id: I8e68bcee6c8a1c80545730e5fe2980d14021884d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1962246
Commit-Queue: Polina Bondarenko <pbond@chromium.org>
Auto-Submit: Polina Bondarenko <pbond@chromium.org>
Reviewed-by: Edman Anjos <edman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732352}

chrome/browser/chromeos/BUILD.gn

@@ -2617,7 +2617,6 @@ source_set("unit_tests") {
     "arc/bluetooth/arc_bluetooth_task_queue_unittest.cc",
     "arc/boot_phase_monitor/arc_boot_phase_monitor_bridge_unittest.cc",
     "arc/enterprise/cert_store/arc_cert_installer_unittest.cc",
-    "arc/enterprise/cert_store/arc_cert_installer_utils_unittest.cc",
     "arc/enterprise/cert_store/arc_smart_card_manager_bridge_unittest.cc",
     "arc/enterprise/cert_store/security_token_operation_bridge_unittest.cc",
     "arc/extensions/arc_support_message_host_unittest.cc",

chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_installer.cc

@@ -7,6 +7,7 @@
 #include <cert.h>
 
 #include <utility>
+#include <vector>
 
 #include "base/base64.h"
 #include "base/bind.h"
@@ -18,10 +19,27 @@
 #include "chrome/browser/net/nss_context.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/net/x509_certificate_model_nss.h"
+#include "crypto/rsa_private_key.h"
 #include "net/cert/x509_util_nss.h"
 
 namespace arc {
 
+namespace {
+
+// Exports subject public key info and encodes it in base64.
+std::string exportSpki(crypto::RSAPrivateKey* rsa) {
+  std::vector<uint8_t> spki;
+  if (!rsa->ExportPublicKey(&spki)) {
+    LOG(ERROR) << "Key export has failed.";
+    return "";
+  }
+  std::string encoded_spki;
+  base::Base64Encode(std::string(spki.begin(), spki.end()), &encoded_spki);
+  return encoded_spki;
+}
+
+}  // namespace
+
 ArcCertInstaller::ArcCertInstaller(content::BrowserContext* context)
     : ArcCertInstaller(Profile::FromBrowserContext(context),
                        std::make_unique<policy::RemoteCommandsQueue>()) {}
@@ -41,7 +59,7 @@ ArcCertInstaller::~ArcCertInstaller() {
   queue_->RemoveObserver(this);
 }
 
-std::set<std::string> ArcCertInstaller::InstallArcCerts(
+std::map<std::string, std::string> ArcCertInstaller::InstallArcCerts(
     const std::vector<net::ScopedCERTCertificate>& certificates,
     InstallArcCertsCallback callback) {
   VLOG(1) << "ArcCertInstaller::InstallArcCerts";
@@ -53,7 +71,8 @@ std::set<std::string> ArcCertInstaller::InstallArcCerts(
     pending_status_ = true;
   }
 
-  std::set<std::string> required_cert_names;
+  // Map the certificate name to the dummy RSA SPKI.
+  std::map<std::string, std::string> required_cert_names;
   callback_ = std::move(callback);
 
   for (const auto& nss_cert : certificates) {
@@ -65,9 +84,7 @@ std::set<std::string> ArcCertInstaller::InstallArcCerts(
 
     std::string cert_name =
         x509_certificate_model::GetCertNameOrNickname(nss_cert.get());
-    required_cert_names.insert(cert_name);
-
-    InstallArcCert(cert_name, nss_cert);
+    required_cert_names[cert_name] = InstallArcCert(cert_name, nss_cert);
   }
 
   // Cleanup |known_cert_names_| according to |required_cert_names|.
@@ -81,23 +98,24 @@ std::set<std::string> ArcCertInstaller::InstallArcCerts(
     std::move(callback_).Run(pending_status_);
     pending_status_ = true;
   }
-
   return required_cert_names;
 }
 
-void ArcCertInstaller::InstallArcCert(
+std::string ArcCertInstaller::InstallArcCert(
     const std::string& name,
     const net::ScopedCERTCertificate& nss_cert) {
   VLOG(1) << "ArcCertInstaller::InstallArcCert " << name;
 
-  // Do not install certificate if already exists.
-  if (known_cert_names_.count(name))
-    return;
+  // Do not install certificate if it is already installed or pending.
+  if (known_cert_names_.count(name)) {
+    VLOG(1) << "Certificate " << name << " is already installed or pending";
+    return "";
+  }
 
   std::string der_cert;
   if (!net::x509_util::GetDEREncoded(nss_cert.get(), &der_cert)) {
     LOG(ERROR) << "Certificate encoding error: " << name;
-    return;
+    return "";
   }
   known_cert_names_.insert(name);
 
@@ -113,23 +131,31 @@ void ArcCertInstaller::InstallArcCert(
 
   std::string der_cert64;
   base::Base64Encode(der_cert, &der_cert64);
-  command_proto.set_payload(base::StringPrintf(
-      "{\"type\":\"INSTALL_KEY_PAIR\","
-      "\"payload\":\"{"
-      "\\\"key\\\"=\\\"%s\\\","
-      "\\\"alias\\\":\\\"%s\\\","
-      "\\\"certs\\\":[\\\"%s\\\"]}\"}",
-      CreatePkcs12FromBlob(name).c_str(), name.c_str(), der_cert64.c_str()));
+
+  std::unique_ptr<crypto::RSAPrivateKey> rsa =
+      crypto::RSAPrivateKey::Create(2048);
+  std::string pkcs12 = CreatePkcs12ForKey(name, rsa->key());
+  command_proto.set_payload(
+      base::StringPrintf("{\"type\":\"INSTALL_KEY_PAIR\","
+                         "\"payload\":\"{"
+                         "\\\"key\\\"=\\\"%s\\\","
+                         "\\\"alias\\\":\\\"%s\\\","
+                         "\\\"certs\\\":[\\\"%s\\\"]}\"}",
+                         pkcs12.c_str(), name.c_str(), der_cert64.c_str()));
   if (!job || !job->Init(queue_->GetNowTicks(), command_proto, nullptr /* signed_command */)) {
     LOG(ERROR) << "Initialization of remote command failed";
     known_cert_names_.erase(name);
+    return "";
   } else {
     pending_commands_[next_id_++] = name;
     queue_->AddJob(std::move(job));
+
+    return exportSpki(rsa.get());
   }
 }
 
 void ArcCertInstaller::OnJobFinished(policy::RemoteCommandJob* command) {
+  VLOG(1) << "ArcCertInstaller::OnJobFinished";
   if (!pending_commands_.count(command->unique_id())) {
     LOG(ERROR) << "Received invalid ARC remote command with unrecognized "
                << "unique_id = " << command->unique_id();

chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_installer.h

@@ -42,17 +42,23 @@ class ArcCertInstaller : public policy::RemoteCommandsQueue::Observer {
 
   // Install missing certificates via ARC remote commands.
   //
-  // Return set of the names of certificates required being installed on ARC.
+  // Return map of certificate names required being installed on ARC to dummy
+  // SPKI.
+  // The dummy SPKI may be empty if the key is not installed during this call
+  // (either error or already installed key pair).
   // Return false via |callback| in case of any error, and true otherwise.
   // Made virtual for override in test.
-  virtual std::set<std::string> InstallArcCerts(
+  virtual std::map<std::string, std::string> InstallArcCerts(
       const std::vector<net::ScopedCERTCertificate>& certs,
       InstallArcCertsCallback callback);
 
  private:
   // Install ARC certificate if not installed yet.
-  void InstallArcCert(const std::string& name,
-                      const net::ScopedCERTCertificate& nss_cert);
+  // Return RSA public key material for the NSS cert encoded in base 64
+  // or an empty string if the key is not installed during this call
+  // (either error or already installed key pair).
+  std::string InstallArcCert(const std::string& name,
+                             const net::ScopedCERTCertificate& nss_cert);
 
   // RemoteCommandsQueue::Observer overrides:
   void OnJobStarted(policy::RemoteCommandJob* command) override {}

chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_installer_utils.cc

@@ -7,179 +7,38 @@
 #include "base/base64.h"
 #include "base/logging.h"
 #include "base/strings/string_piece.h"
-#include "third_party/boringssl/src/include/openssl/bn.h"
 #include "third_party/boringssl/src/include/openssl/evp.h"
 #include "third_party/boringssl/src/include/openssl/mem.h"
 #include "third_party/boringssl/src/include/openssl/pkcs8.h"
 
 namespace arc {
 
-// Creates a fake, but recognized as valid, RSA private key:
-// * q = |blob| + 3 + { 4, 3, 2 } so that q % 3 = 1
-// * p = 3
-// * n = p * q
-// * e = 1
-// * d = (q - 1) * (p - 1) + 1 so that d % (p - 1) * (q - 1) = 1
-// * dmp1 = 1
-// * dmq1 = 1
-// * iqmp = 1
-RSA* CreateRsaPrivateKeyFromBlob(const std::string& blob) {
-  const uint8_t* ptr = (const uint8_t*)blob.data();
-  BIGNUM* q = BN_bin2bn(ptr, blob.size(), nullptr);
-  int mod3;
-  if (!q || !BN_add_word(q, 3) || (mod3 = BN_mod_word(q, 3)) >= 3 ||
-      !BN_add_word(q, 4 - mod3)) {
-    BN_free(q);
-    VLOG(1) << "Failed to create q for " << blob;
-    return nullptr;
-  }
-
-  BIGNUM* p = BN_new();
-  if (!p || !BN_set_word(p, 3)) {
-    BN_free(q);
-    BN_free(p);
-    VLOG(1) << "Failed to create p for " << blob;
-    return nullptr;
-  }
-
-  BIGNUM* n = BN_new();
-  bssl::UniquePtr<BN_CTX> ctx(BN_CTX_new());
-  if (!n || !BN_mul(n, p, q, ctx.get())) {
-    BN_free(q);
-    BN_free(p);
-    BN_free(n);
-    VLOG(1) << "Failed to create n for " << blob;
-    return nullptr;
-  }
-
-  BIGNUM* e = BN_new();
-  if (!e || !BN_one(e)) {
-    BN_free(q);
-    BN_free(p);
-    BN_free(n);
-    BN_free(e);
-    VLOG(1) << "Failed to create e for " << blob;
-    return nullptr;
-  }
-
-  BIGNUM* d = BN_new();
-  if (!d || !BN_sub(d, q, BN_value_one()) || !BN_mul_word(d, 2) ||
-      !BN_add_word(d, 1)) {
-    BN_free(q);
-    BN_free(p);
-    BN_free(n);
-    BN_free(e);
-    BN_free(d);
-    VLOG(1) << "Failed to create d for " << blob;
-    return nullptr;
-  }
-
-  BIGNUM* dmp1 = BN_new();
-  BIGNUM* dmq1 = BN_new();
-  BIGNUM* iqmp = BN_new();
-  if (!dmp1 || !BN_one(dmp1) || !dmq1 || !BN_one(dmq1) || !iqmp ||
-      !BN_one(iqmp)) {
-    BN_free(q);
-    BN_free(p);
-    BN_free(n);
-    BN_free(e);
-    BN_free(d);
-    BN_free(dmp1);
-    BN_free(dmq1);
-    BN_free(iqmp);
-    VLOG(1) << "Failed to create dmp1 or dmq1 or iqmp for " << blob;
-    return nullptr;
-  }
-
-  RSA* rsa = RSA_new();
-  if (!rsa) {
-    BN_free(q);
-    BN_free(p);
-    BN_free(n);
-    BN_free(e);
-    BN_free(d);
-    BN_free(dmp1);
-    BN_free(dmq1);
-    BN_free(iqmp);
-    VLOG(1) << "Failed to create RSA key.";
-    return nullptr;
-  }
-
-  if (!RSA_set0_key(rsa, n, e, d)) {
-    BN_free(q);
-    BN_free(p);
-    BN_free(n);
-    BN_free(e);
-    BN_free(d);
-    BN_free(dmp1);
-    BN_free(dmq1);
-    BN_free(iqmp);
-    RSA_free(rsa);
-    VLOG(1) << "Failed to set n to RSA for " << blob;
-    return nullptr;
-  }
-
-  if (!RSA_set0_factors(rsa, p, q)) {
-    BN_free(q);
-    BN_free(p);
-    BN_free(dmp1);
-    BN_free(dmq1);
-    BN_free(iqmp);
-    RSA_free(rsa);
-    VLOG(1) << "Failed to set factors to RSA for " << blob;
-    return nullptr;
-  }
-
-  if (!RSA_set0_crt_params(rsa, dmp1, dmq1, iqmp)) {
-    BN_free(dmp1);
-    BN_free(dmq1);
-    BN_free(iqmp);
-    RSA_free(rsa);
-    VLOG(1) << "Failed to set crt params for " << blob;
-    return nullptr;
-  }
-  return rsa;
-}
-
-std::string CreatePkcs12FromBlob(const std::string& blob) {
-  bssl::UniquePtr<EVP_PKEY> pkey(EVP_PKEY_new());
-
-  if (!pkey) {
-    VLOG(1) << "Failed to generate RSA for " << blob;
-    return "";
-  }
-
-  RSA* rsa = CreateRsaPrivateKeyFromBlob(blob);
-  if (!rsa) {
-    return "";
-  }
-
-  if (!EVP_PKEY_assign_RSA(pkey.get(), rsa)) {
-    RSA_free(rsa);
-    VLOG(1) << "Failed to assign RSA for " << blob;
+std::string CreatePkcs12ForKey(const std::string& name, EVP_PKEY* key) {
+  if (!key) {
+    VLOG(1) << "Failed due to a nullptr key";
     return "";
   }
 
   // Make a PKCS#12 blob.
   bssl::UniquePtr<PKCS12> pkcs12(PKCS12_create(
-      nullptr, blob.c_str(), pkey.get(), nullptr, nullptr, 0, 0, 0, 0, 0));
+      nullptr, name.c_str(), key, nullptr, nullptr, 0, 0, 0, 0, 0));
   if (!pkcs12) {
-    VLOG(1) << "Failed to create PKCS12 object from pkey for " << blob;
+    VLOG(1) << "Failed to create PKCS12 object from |key| for " << name;
     return "";
   }
 
-  uint8_t* key = nullptr;
-  int key_len;
-  if (!(key_len = i2d_PKCS12(pkcs12.get(), &key))) {
-    VLOG(1) << "Failed to translate PKCS12 to byte array for " << blob;
+  uint8_t* pkcs12_key = nullptr;
+  int pkcs12_key_len;
+  if (!(pkcs12_key_len = i2d_PKCS12(pkcs12.get(), &pkcs12_key))) {
+    VLOG(1) << "Failed to translate PKCS12 to byte array for " << name;
     return "";
   }
 
-  bssl::UniquePtr<uint8_t> free_key(key);
-
-  std::string encoded_key;
-  base::Base64Encode(base::StringPiece((char*)key, key_len), &encoded_key);
-  return encoded_key;
+  bssl::UniquePtr<uint8_t> free_pkcs12_key(pkcs12_key);
+  std::string encoded_pkcs12_key;
+  base::Base64Encode(base::StringPiece((char*)pkcs12_key, pkcs12_key_len),
+                     &encoded_pkcs12_key);
+  return encoded_pkcs12_key;
 }
 
 }  // namespace arc

chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_installer_utils.h

@@ -7,20 +7,13 @@
 
 #include <string>
 
-#include "third_party/boringssl/src/include/openssl/rsa.h"
+#include "third_party/boringssl/src/include/openssl/base.h"
 
 namespace arc {
 
-// Creates a PKCS12 container with RSA private key, generated with p = |blob|,
-// to be able to extract |blob| value from a private key material later.
+// Creates a PKCS12 container named |name| with private key |key|.
 // Returns empty string in case of any error.
-std::string CreatePkcs12FromBlob(const std::string& blob);
-
-// Helper function that creates the RSA private key with p = |blob| to
-// be able to extract |blob| value from a private key material later.
-// Returns nullptr in case of any error.
-// Should be used only for testing.
-RSA* CreateRsaPrivateKeyFromBlob(const std::string& blob);
+std::string CreatePkcs12ForKey(const std::string& name, EVP_PKEY* key);
 
 }  // namespace arc
 

chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_installer_utils_unittest.cc

-// Copyright 2019 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 <string>
-
-#include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_installer_utils.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "third_party/boringssl/src/include/openssl/evp.h"
-#include "third_party/boringssl/src/include/openssl/mem.h"
-#include "third_party/boringssl/src/include/openssl/pkcs8.h"
-#include "third_party/boringssl/src/include/openssl/rsa.h"
-
-namespace arc {
-
-namespace {
-
-// Keep in sync with external/boringssl/src/crypto/fipsmodule/rsa/rsa.c
-int checkRsaKey(const RSA* key) {
-  BIGNUM n, pm1, qm1, lcm, gcd, de, dmp1, dmq1, iqmp_times_q;
-  BN_CTX* ctx;
-  int ok = 0, has_crt_values;
-
-  if (RSA_is_opaque(key)) {
-    // Opaque keys can't be checked.
-    return 1;
-  }
-
-  if ((key->p != nullptr) != (key->q != nullptr)) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_ONLY_ONE_OF_P_Q_GIVEN);
-    return 0;
-  }
-
-  if (!key->n || !key->e) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_VALUE_MISSING);
-    return 0;
-  }
-
-  if (!key->d || !key->p) {
-    // For a public key, or without p and q, there's nothing that can be
-    // checked.
-    return 1;
-  }
-
-  ctx = BN_CTX_new();
-  if (ctx == nullptr) {
-    OPENSSL_PUT_ERROR(RSA, ERR_R_MALLOC_FAILURE);
-    return 0;
-  }
-
-  BN_init(&n);
-  BN_init(&pm1);
-  BN_init(&qm1);
-  BN_init(&lcm);
-  BN_init(&gcd);
-  BN_init(&de);
-  BN_init(&dmp1);
-  BN_init(&dmq1);
-  BN_init(&iqmp_times_q);
-
-  if (!BN_mul(&n, key->p, key->q, ctx) ||
-      // lcm = lcm(p, q)
-      !BN_sub(&pm1, key->p, BN_value_one()) ||
-      !BN_sub(&qm1, key->q, BN_value_one()) || !BN_mul(&lcm, &pm1, &qm1, ctx) ||
-      !BN_gcd(&gcd, &pm1, &qm1, ctx)) {
-    OPENSSL_PUT_ERROR(RSA, ERR_LIB_BN);
-    goto out;
-  }
-
-  if (!BN_div(&lcm, nullptr, &lcm, &gcd, ctx) ||
-      !BN_gcd(&gcd, &pm1, &qm1, ctx) ||
-      // de = d*e mod lcm(p, q).
-      !BN_mod_mul(&de, key->d, key->e, &lcm, ctx)) {
-    OPENSSL_PUT_ERROR(RSA, ERR_LIB_BN);
-    goto out;
-  }
-
-  if (BN_cmp(&n, key->n) != 0) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_N_NOT_EQUAL_P_Q);
-    goto out;
-  }
-
-  if (!BN_is_one(&de)) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_D_E_NOT_CONGRUENT_TO_1);
-    goto out;
-  }
-
-  has_crt_values = key->dmp1 != nullptr;
-
-  if (has_crt_values != (key->dmq1 != nullptr) ||
-      has_crt_values != (key->iqmp != nullptr)) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_INCONSISTENT_SET_OF_CRT_VALUES);
-    goto out;
-  }
-
-  if (has_crt_values) {
-    if (  // dmp1 = d mod (p-1)
-        !BN_mod(&dmp1, key->d, &pm1, ctx) ||
-        // dmq1 = d mod (q-1)
-        !BN_mod(&dmq1, key->d, &qm1, ctx) ||
-        // iqmp = q^-1 mod p
-        !BN_mod_mul(&iqmp_times_q, key->iqmp, key->q, key->p, ctx)) {
-      OPENSSL_PUT_ERROR(RSA, ERR_LIB_BN);
-      goto out;
-    }
-
-    if (BN_cmp(&dmp1, key->dmp1) != 0 || BN_cmp(&dmq1, key->dmq1) != 0 ||
-        BN_cmp(key->iqmp, key->p) >= 0 || !BN_is_one(&iqmp_times_q)) {
-      OPENSSL_PUT_ERROR(RSA, RSA_R_CRT_VALUES_INCORRECT);
-      goto out;
-    }
-  }
-
-  ok = 1;
-
-out:
-  BN_free(&n);
-  BN_free(&pm1);
-  BN_free(&qm1);
-  BN_free(&lcm);
-  BN_free(&gcd);
-  BN_free(&de);
-  BN_free(&dmp1);
-  BN_free(&dmq1);
-  BN_free(&iqmp_times_q);
-  BN_CTX_free(ctx);
-
-  return ok;
-}
-
-}  // namespace
-
-class ArcCertInstallerUtilsTest
-    : public testing::Test,
-      public testing::WithParamInterface<std::string> {};
-
-// Test that CreatePkcs12FromBlob returns non-empty PKCS12 blob with a valid
-// RSA private key.
-TEST_P(ArcCertInstallerUtilsTest, Pkcs12) {
-  const std::string name = GetParam();
-  EXPECT_FALSE(CreatePkcs12FromBlob(name).empty());
-
-  RSA* rsa = CreateRsaPrivateKeyFromBlob(name);
-  ASSERT_TRUE(rsa);
-  EXPECT_TRUE(checkRsaKey(rsa));
-  RSA_free(rsa);
-}
-
-INSTANTIATE_TEST_SUITE_P(All,
-                         ArcCertInstallerUtilsTest,
-                         testing::Values("",
-                                         "name of the smart card",
-                                         std::string(2048, 'A')));
-
-}  // namespace arc

chrome/browser/chromeos/arc/enterprise/cert_store/arc_smart_card_manager_bridge.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_smart_card_manager_bridge.h"
 
+#include <algorithm>
+#include <iterator>
 #include <utility>
 
 #include "base/bind.h"
@@ -13,9 +15,12 @@
 #include "chrome/browser/chromeos/arc/policy/arc_policy_bridge.h"
 #include "chrome/browser/chromeos/certificate_provider/certificate_provider_service.h"
 #include "chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.h"
+#include "chrome/browser/chromeos/platform_keys/platform_keys.h"
+#include "chrome/common/net/x509_certificate_model_nss.h"
 #include "components/arc/arc_browser_context_keyed_service_factory_base.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_namespace.h"
+#include "crypto/rsa_private_key.h"
 #include "net/cert/x509_util_nss.h"
 
 namespace arc {
@@ -96,12 +101,46 @@ void ArcSmartCardManagerBridge::Refresh(RefreshCallback callback) {
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
+std::string ArcSmartCardManagerBridge::GetRealSpkiForDummySpki(
+    const std::string& dummy_spki) {
+  return certificate_cache_.GetRealSpkiForDummySpki(dummy_spki);
+}
+
+ArcSmartCardManagerBridge::CertificateCache::CertificateCache() = default;
+ArcSmartCardManagerBridge::CertificateCache::~CertificateCache() = default;
+
 void ArcSmartCardManagerBridge::DidGetCerts(
     RefreshCallback callback,
     net::ClientCertIdentityList cert_identities) {
   VLOG(1) << "ArcSmartCardManagerBridge::DidGetCerts";
 
+  certificate_cache_.clear_need_policy_update();
+  std::vector<net::ScopedCERTCertificate> certificates =
+      certificate_cache_.Update(std::move(cert_identities));
+
+  // Maps cert name to dummy SPKI.
+  std::map<std::string, std::string> installed_keys =
+      installer_->InstallArcCerts(std::move(certificates), std::move(callback));
+
+  certificate_cache_.Update(installed_keys);
+
+  if (certificate_cache_.need_policy_update()) {
+    ArcPolicyBridge* const policy_bridge =
+        ArcPolicyBridge::GetForBrowserContext(context_);
+    if (policy_bridge) {
+      policy_bridge->OnPolicyUpdated(policy::PolicyNamespace(),
+                                     policy::PolicyMap(), policy::PolicyMap());
+    }
+  }
+}
+
+std::vector<net::ScopedCERTCertificate>
+ArcSmartCardManagerBridge::CertificateCache::Update(
+    net::ClientCertIdentityList cert_identities) {
   std::vector<net::ScopedCERTCertificate> certificates;
+  // Map cert name to real SPKI.
+  real_spki_by_name_cache_.clear();
+  std::set<std::string> new_required_cert_names;
   for (const auto& identity : cert_identities) {
     net::ScopedCERTCertificate nss_cert(
         net::x509_util::CreateCERTCertificateFromX509Certificate(
@@ -111,20 +150,56 @@ void ArcSmartCardManagerBridge::DidGetCerts(
                  << "certificate.";
       continue;
     }
+    std::string cert_name =
+        x509_certificate_model::GetCertNameOrNickname(nss_cert.get());
+    real_spki_by_name_cache_[cert_name] =
+        chromeos::platform_keys::GetSubjectPublicKeyInfo(
+            identity->certificate());
+    new_required_cert_names.insert(cert_name);
 
     certificates.push_back(std::move(nss_cert));
   }
-  std::set<std::string> new_required_cert_names =
-      installer_->InstallArcCerts(std::move(certificates), std::move(callback));
-  if (required_cert_names_ != new_required_cert_names) {
-    required_cert_names_ = new_required_cert_names;
-    ArcPolicyBridge* const policy_bridge =
-        ArcPolicyBridge::GetForBrowserContext(context_);
-    if (policy_bridge) {
-      policy_bridge->OnPolicyUpdated(policy::PolicyNamespace(),
-                                     policy::PolicyMap(), policy::PolicyMap());
+  need_policy_update_ = (required_cert_names_ != new_required_cert_names);
+  for (auto cert_name : required_cert_names_) {
+    if (!new_required_cert_names.count(cert_name)) {
+      real_spki_by_dummy_spki_cache_.erase(
+          dummy_spki_by_name_cache_[cert_name]);
+      dummy_spki_by_name_cache_.erase(cert_name);
+    }
+  }
+  required_cert_names_ = new_required_cert_names;
+  return certificates;
+}
+
+void ArcSmartCardManagerBridge::CertificateCache::Update(
+    std::map<std::string, std::string> dummy_spki_by_name) {
+  if (required_cert_names_.size() != dummy_spki_by_name.size())
+    return;
+  for (const auto& cert : dummy_spki_by_name) {
+    const std::string& name = cert.first;
+    if (!required_cert_names_.count(name)) {
+      VLOG(1) << "ArcSmartCardManagerBridge::CertificateCache::Update error: "
+              << "An attempt to add a non-required key " << name;
+      continue;
+    }
+
+    std::string dummy_spki = cert.second;
+    if (dummy_spki.empty() && dummy_spki_by_name_cache_.count(name))
+      dummy_spki = dummy_spki_by_name_cache_[name];
+    if (!dummy_spki.empty()) {
+      dummy_spki_by_name_cache_[name] = dummy_spki;
+      real_spki_by_dummy_spki_cache_[dummy_spki] =
+          real_spki_by_name_cache_[name];
     }
   }
 }
 
+std::string
+ArcSmartCardManagerBridge::CertificateCache::GetRealSpkiForDummySpki(
+    const std::string& dummy_spki) {
+  if (real_spki_by_dummy_spki_cache_.count(dummy_spki))
+    return real_spki_by_dummy_spki_cache_[dummy_spki];
+  return "";
+}
+
 }  // namespace arc

chrome/browser/chromeos/arc/enterprise/cert_store/arc_smart_card_manager_bridge.h

@@ -55,18 +55,52 @@ class ArcSmartCardManagerBridge : public KeyedService,
   // SmartCardManagerHost overrides.
   void Refresh(RefreshCallback callback) override;
 
+  // Returns a real SPKI for a dummy SPKI |dummy_spki|.
+  // Returns empty string if the key is unknown.
+  std::string GetRealSpkiForDummySpki(const std::string& dummy_spki);
+
   std::vector<std::string> get_required_cert_names() const {
-    return std::vector<std::string>(required_cert_names_.begin(),
-                                    required_cert_names_.end());
+    return certificate_cache_.get_required_cert_names();
   }
 
   void set_required_cert_names_for_testing(
       const std::vector<std::string>& cert_names) {
-    required_cert_names_ =
-        std::set<std::string>(cert_names.begin(), cert_names.end());
+    certificate_cache_.set_required_cert_names_for_testing(
+        std::set<std::string>(cert_names.begin(), cert_names.end()));
   }
 
  private:
+  class CertificateCache {
+   public:
+    CertificateCache();
+    ~CertificateCache();
+
+    std::vector<net::ScopedCERTCertificate> Update(
+        net::ClientCertIdentityList cert_identities);
+    void Update(std::map<std::string, std::string> dummy_spki_by_name);
+    std::string GetRealSpkiForDummySpki(const std::string& dummy_spki);
+
+    bool need_policy_update() { return need_policy_update_; }
+    void clear_need_policy_update() { need_policy_update_ = false; }
+    std::vector<std::string> get_required_cert_names() const {
+      return std::vector<std::string>(required_cert_names_.begin(),
+                                      required_cert_names_.end());
+    }
+    void set_required_cert_names_for_testing(std::set<std::string> cert_names) {
+      required_cert_names_ = std::move(cert_names);
+    }
+
+   private:
+    bool need_policy_update_ = false;
+    std::set<std::string> required_cert_names_;
+    // Map dummy SPKI to real SPKI.
+    std::map<std::string, std::string> real_spki_by_dummy_spki_cache_;
+    // Map cert name to dummy SPKI.
+    std::map<std::string, std::string> dummy_spki_by_name_cache_;
+    // Intermediate map name to real SPKI.
+    std::map<std::string, std::string> real_spki_by_name_cache_;
+  };
+
   void DidGetCerts(RefreshCallback callback,
                    net::ClientCertIdentityList cert_identities);
 
@@ -75,8 +109,7 @@ class ArcSmartCardManagerBridge : public KeyedService,
 
   std::unique_ptr<chromeos::CertificateProvider> certificate_provider_;
   std::unique_ptr<ArcCertInstaller> installer_;
-
-  std::set<std::string> required_cert_names_;
+  CertificateCache certificate_cache_;
 
   base::WeakPtrFactory<ArcSmartCardManagerBridge> weak_ptr_factory_;
 

chrome/browser/chromeos/arc/enterprise/cert_store/arc_smart_card_manager_bridge_unittest.cc

@@ -108,7 +108,7 @@ class MockArcCertInstaller : public ArcCertInstaller {
                        std::unique_ptr<policy::RemoteCommandsQueue> queue)
       : ArcCertInstaller(profile, std::move(queue)) {}
   MOCK_METHOD2(InstallArcCerts,
-               std::set<std::string>(
+               std::map<std::string, std::string>(
                    const std::vector<net::ScopedCERTCertificate>& certs,
                    InstallArcCertsCallback callback));
 };
@@ -188,7 +188,7 @@ TEST_F(ArcSmartCardManagerBridgeTest, NoSmartCardTest) {
       .WillOnce(
           WithArg<1>(Invoke([](base::OnceCallback<void(bool result)> callback) {
             std::move(callback).Run(true);
-            return std::set<std::string>();
+            return std::map<std::string, std::string>();
           })));
   bridge()->Refresh(base::BindOnce([](bool result) { EXPECT_TRUE(result); }));
 }
@@ -204,7 +204,12 @@ TEST_F(ArcSmartCardManagerBridgeTest, BasicSmartCardTest) {
       .WillOnce(WithArg<1>(
           Invoke([&cert_names](base::OnceCallback<void(bool result)> callback) {
             std::move(callback).Run(true);
-            return std::set<std::string>(cert_names.begin(), cert_names.end());
+            std::map<std::string, std::string> cert_names_map;
+            std::transform(
+                cert_names.begin(), cert_names.end(),
+                std::inserter(cert_names_map, cert_names_map.end()),
+                [](const std::string& s) { return std::make_pair(s, ""); });
+            return cert_names_map;
           })));
   EXPECT_CALL(*policy_bridge(), OnPolicyUpdated(_, _, _));
   bridge()->Refresh(base::BindOnce([](bool result) { EXPECT_TRUE(result); }));