certificateProvider: allow referring to extension-provided keys by SPKI

Allow referring to extension-provided keys/client certificates by
Subject Public Key Info (SPKI).

Bug: 826417, 531351
Test: unit_tests --gtest_filter=CertificateProviderServiceTest*
Change-Id: I0a6ee71e4a2d19957a2d675750cb86afda483eb3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1667345Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671563}

chrome/browser/chromeos/certificate_provider/certificate_provider_service.cc

@@ -409,6 +409,44 @@ void CertificateProviderService::OnExtensionUnloaded(
   pin_dialog_manager_.ExtensionUnloaded(extension_id);
 }
 
+void CertificateProviderService::RequestSignatureBySpki(
+    const std::string& subject_public_key_info,
+    uint16_t algorithm,
+    base::span<const uint8_t> digest,
+    net::SSLPrivateKey::SignCallback callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  bool is_currently_provided = false;
+  CertificateInfo info;
+  std::string extension_id;
+  certificate_map_.LookUpCertificateBySpki(
+      subject_public_key_info, &is_currently_provided, &info, &extension_id);
+  if (!is_currently_provided) {
+    LOG(ERROR) << "no certificate with the specified spki was found";
+    std::move(callback).Run(net::ERR_FAILED, std::vector<uint8_t>());
+    return;
+  }
+
+  RequestSignatureFromExtension(extension_id, info.certificate, algorithm,
+                                digest, std::move(callback));
+}
+
+bool CertificateProviderService::GetSupportedAlgorithmsBySpki(
+    const std::string& subject_public_key_info,
+    std::vector<uint16_t>* supported_algorithms) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  bool is_currently_provided = false;
+  CertificateInfo info;
+  std::string extension_id;
+  certificate_map_.LookUpCertificateBySpki(
+      subject_public_key_info, &is_currently_provided, &info, &extension_id);
+  if (!is_currently_provided) {
+    LOG(ERROR) << "no certificate with the specified spki was found";
+    return false;
+  }
+  *supported_algorithms = info.supported_algorithms;
+  return true;
+}
+
 void CertificateProviderService::GetCertificatesFromExtensions(
     base::OnceCallback<void(net::ClientCertIdentityList)> callback) {
   DCHECK(thread_checker_.CalledOnValidThread());

chrome/browser/chromeos/certificate_provider/certificate_provider_service.h

@@ -154,6 +154,26 @@ class CertificateProviderService : public KeyedService {
   // corresponding notification of the ExtensionRegistry is triggered.
   void OnExtensionUnloaded(const std::string& extension_id);
 
+  // Requests the extension which provided the certificate identified by
+  // |subject_public_key_info| to sign |digest| with the corresponding private
+  // key. |algorithm| is a TLS 1.3 SignatureScheme value. See net::SSLPrivateKey
+  // for details. |callback| will be run with the reply of the extension or an
+  // error.
+  void RequestSignatureBySpki(const std::string& subject_public_key_info,
+                              uint16_t algorithm,
+                              base::span<const uint8_t> digest,
+                              net::SSLPrivateKey::SignCallback callback);
+
+  // Looks up the certificate identified by |subject_public_key_info|. If any
+  // extension is currently providing such a certificate, fills
+  // *|supported_algorithms| with the algorithms supported for that certificate
+  // and returns true. Values used for |supported_algorithms| are TLS 1.3
+  // SignatureSchemes. See net::SSLPrivateKey for details. If no extension is
+  // currently providing such a certificate, returns false.
+  bool GetSupportedAlgorithmsBySpki(
+      const std::string& subject_public_key_info,
+      std::vector<uint16_t>* supported_algorithms);
+
   PinDialogManager* pin_dialog_manager() { return &pin_dialog_manager_; }
 
  private:

chrome/browser/chromeos/certificate_provider/certificate_provider_service_unittest.cc

@@ -8,15 +8,20 @@
 #include <set>
 #include <utility>
 
+#include "base/base64.h"
 #include "base/bind.h"
+#include "base/containers/span.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/cert/asn1_util.h"
+#include "net/cert/x509_util.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/test_data_directory.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/boringssl/src/include/openssl/ssl.h"
 
@@ -528,4 +533,53 @@ TEST_F(CertificateProviderServiceTest, UnloadExtensionDuringSign) {
   EXPECT_EQ(net::ERR_FAILED, error);
 }
 
+// Try to sign data using key; using the Subject Public Key Info (SPKI) to
+// identify the key.
+TEST_F(CertificateProviderServiceTest, SignUsingSpkiAsIdentification) {
+  base::StringPiece client1_spki_piece;
+  ASSERT_TRUE(net::asn1::ExtractSPKIFromDERCert(
+      net::x509_util::CryptoBufferAsStringPiece(
+          cert_info1_.certificate->cert_buffer()),
+      &client1_spki_piece));
+  std::string client1_spki = client1_spki_piece.as_string();
+
+  std::unique_ptr<net::ClientCertIdentity> cert(ProvideDefaultCert());
+  ASSERT_TRUE(cert);
+
+  std::vector<uint16_t> supported_algorithms;
+  // If this fails, try to regenerate kClient1SpkiBase64 using the command shown
+  // above.
+  EXPECT_TRUE(service_->GetSupportedAlgorithmsBySpki(client1_spki,
+                                                     &supported_algorithms));
+  EXPECT_THAT(supported_algorithms,
+              testing::UnorderedElementsAre(SSL_SIGN_RSA_PKCS1_SHA256));
+
+  test_delegate_->ClearAndExpectRequest(TestDelegate::RequestType::SIGN);
+  std::vector<uint8_t> input{'d', 'a', 't', 'a'};
+  std::vector<uint8_t> received_signature;
+  service_->RequestSignatureBySpki(
+      client1_spki, SSL_SIGN_RSA_PKCS1_SHA256, input,
+      base::BindOnce(&ExpectOKAndStoreSignature, &received_signature));
+
+  task_runner_->RunUntilIdle();
+
+  const int sign_request_id = test_delegate_->last_sign_request_id_;
+  EXPECT_EQ(TestDelegate::RequestType::NONE,
+            test_delegate_->expected_request_type_);
+  EXPECT_TRUE(cert_info1_.certificate->EqualsExcludingChain(
+      test_delegate_->last_certificate_.get()));
+
+  // No signature received until the extension replied to the service.
+  EXPECT_TRUE(received_signature.empty());
+
+  std::vector<uint8_t> signature_reply;
+  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);
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/certificate_provider/thread_safe_certificate_map.cc

@@ -5,12 +5,24 @@
 #include "chrome/browser/chromeos/certificate_provider/thread_safe_certificate_map.h"
 
 #include "net/base/hash_value.h"
+#include "net/cert/asn1_util.h"
 #include "net/cert/x509_certificate.h"
+#include "net/cert/x509_util.h"
 
 namespace chromeos {
 namespace certificate_provider {
 namespace {
 
+std::string GetSubjectPublicKeyInfo(const net::X509Certificate& certificate) {
+  base::StringPiece spki_bytes;
+  if (!net::asn1::ExtractSPKIFromDERCert(
+          net::x509_util::CryptoBufferAsStringPiece(certificate.cert_buffer()),
+          &spki_bytes)) {
+    return {};
+  }
+  return spki_bytes.as_string();
+}
+
 void BuildFingerprintsMap(
     const std::map<std::string, certificate_provider::CertificateInfoList>&
         extension_to_certificates,
@@ -23,19 +35,39 @@ void BuildFingerprintsMap(
           net::X509Certificate::CalculateFingerprint256(
               cert_info.certificate->cert_buffer());
       fingerprint_to_cert->insert(std::make_pair(
-          fingerprint, std::make_unique<ThreadSafeCertificateMap::MapValue>(
-                           cert_info, extension_id)));
+          fingerprint,
+          std::make_unique<ThreadSafeCertificateMap::CertAndExtension>(
+              cert_info, extension_id)));
+    }
+  }
+}
+
+void BuildSpkiMap(
+    const std::map<std::string, certificate_provider::CertificateInfoList>&
+        extension_to_certificates,
+    ThreadSafeCertificateMap::SpkiToCertAndExtensionMap* spki_to_cert) {
+  for (const auto& entry : extension_to_certificates) {
+    const std::string& extension_id = entry.first;
+    for (const CertificateInfo& cert_info : entry.second) {
+      const std::string spki = GetSubjectPublicKeyInfo(*cert_info.certificate);
+      // If the same public key appears in the |extension_to_certificates| input
+      // multiple times, it is unspecified which (cert_info, extension_id) will
+      // end up in the output map.
+      spki_to_cert->insert(std::make_pair(
+          spki, std::make_unique<ThreadSafeCertificateMap::CertAndExtension>(
+                    cert_info, extension_id)));
     }
   }
 }
 
 }  // namespace
 
-ThreadSafeCertificateMap::MapValue::MapValue(const CertificateInfo& cert_info,
-                                             const std::string& extension_id)
+ThreadSafeCertificateMap::CertAndExtension::CertAndExtension(
+    const CertificateInfo& cert_info,
+    const std::string& extension_id)
     : cert_info(cert_info), extension_id(extension_id) {}
 
-ThreadSafeCertificateMap::MapValue::~MapValue() {}
+ThreadSafeCertificateMap::CertAndExtension::~CertAndExtension() {}
 
 ThreadSafeCertificateMap::ThreadSafeCertificateMap() {}
 
@@ -45,17 +77,27 @@ void ThreadSafeCertificateMap::Update(
     const std::map<std::string, certificate_provider::CertificateInfoList>&
         extension_to_certificates) {
   FingerprintToCertAndExtensionMap new_fingerprint_map;
+  SpkiToCertAndExtensionMap new_spki_map;
   BuildFingerprintsMap(extension_to_certificates, &new_fingerprint_map);
+  BuildSpkiMap(extension_to_certificates, &new_spki_map);
 
   base::AutoLock auto_lock(lock_);
-  // Keep all old fingerprints from |fingerprint_to_cert_and_extension_| but
-  // remove the association to any extension.
+  // Keep all old keys from the old maps (|fingerprint_to_cert_and_extension_|
+  // and |spki_to_cert_and_extension_|), but remove the association to any
+  // extension.
   for (const auto& entry : fingerprint_to_cert_and_extension_) {
     const net::SHA256HashValue& fingerprint = entry.first;
     // This doesn't modify the map if it already contains the key |fingerprint|.
     new_fingerprint_map.insert(std::make_pair(fingerprint, nullptr));
   }
   fingerprint_to_cert_and_extension_.swap(new_fingerprint_map);
+
+  for (const auto& entry : spki_to_cert_and_extension_) {
+    const std::string& spki = entry.first;
+    // This doesn't modify the map if it already contains the key |spki|.
+    new_spki_map.insert(std::make_pair(spki, nullptr));
+  }
+  spki_to_cert_and_extension_.swap(new_spki_map);
 }
 
 bool ThreadSafeCertificateMap::LookUpCertificate(
@@ -72,7 +114,27 @@ bool ThreadSafeCertificateMap::LookUpCertificate(
   if (it == fingerprint_to_cert_and_extension_.end())
     return false;
 
-  MapValue* const value = it->second.get();
+  CertAndExtension* const value = it->second.get();
+  if (value) {
+    *is_currently_provided = true;
+    *info = value->cert_info;
+    *extension_id = value->extension_id;
+  }
+  return true;
+}
+
+bool ThreadSafeCertificateMap::LookUpCertificateBySpki(
+    const std::string& subject_public_key_info,
+    bool* is_currently_provided,
+    CertificateInfo* info,
+    std::string* extension_id) {
+  *is_currently_provided = false;
+  base::AutoLock auto_lock(lock_);
+  const auto it = spki_to_cert_and_extension_.find(subject_public_key_info);
+  if (it == spki_to_cert_and_extension_.end())
+    return false;
+
+  CertAndExtension* const value = it->second.get();
   if (value) {
     *is_currently_provided = true;
     *info = value->cert_info;
@@ -85,11 +147,19 @@ void ThreadSafeCertificateMap::RemoveExtension(
     const std::string& extension_id) {
   base::AutoLock auto_lock(lock_);
   for (auto& entry : fingerprint_to_cert_and_extension_) {
-    MapValue* const value = entry.second.get();
+    CertAndExtension* const value = entry.second.get();
     // Only remove the association of the fingerprint to the extension, but keep
     // the fingerprint.
     if (value && value->extension_id == extension_id)
-      fingerprint_to_cert_and_extension_[entry.first] = nullptr;
+      entry.second.reset();
+  }
+
+  for (auto& entry : spki_to_cert_and_extension_) {
+    CertAndExtension* const value = entry.second.get();
+    // Only remove the association of the SPKI to the extension, but keep the
+    // SPKI.
+    if (value && value->extension_id == extension_id)
+      entry.second.reset();
   }
 }
 

chrome/browser/chromeos/certificate_provider/thread_safe_certificate_map.h

@@ -23,15 +23,19 @@ namespace certificate_provider {
 
 class ThreadSafeCertificateMap {
  public:
-  struct MapValue {
-    MapValue(const CertificateInfo& cert_info, const std::string& extension_id);
-    ~MapValue();
+  struct CertAndExtension {
+    CertAndExtension(const CertificateInfo& cert_info,
+                     const std::string& extension_id);
+    ~CertAndExtension();
 
     CertificateInfo cert_info;
     std::string extension_id;
   };
   using FingerprintToCertAndExtensionMap =
-      std::map<net::SHA256HashValue, std::unique_ptr<MapValue>>;
+      std::map<net::SHA256HashValue, std::unique_ptr<CertAndExtension>>;
+  // A map that has a DER-encoded X.509 Subject Public Key Info as keys.
+  using SpkiToCertAndExtensionMap =
+      std::map<std::string, std::unique_ptr<CertAndExtension>>;
 
   ThreadSafeCertificateMap();
   ~ThreadSafeCertificateMap();
@@ -54,6 +58,21 @@ class ThreadSafeCertificateMap {
                          CertificateInfo* info,
                          std::string* extension_id);
 
+  // Looks up for certificate and extension_id based on
+  // |subject_public_key_info|, which is a DER-encoded X.509 Subject Public Key
+  // Info. If the certificate was added by previous Update() call, returns true.
+  // If this certificate was provided in the most recent Update() call,
+  // |is_currently_provided| will be set to true and |info| and |extension_id|
+  // will be populated according to the data that have been mapped to this
+  // |subject_public_key_info|. Otherwise, if this certificate was not provided
+  // in the most recent Update() call, sets |is_currently_provided| to false and
+  // doesn't modify |info| and |extension_id|. If multiple entries are found, it
+  // is unspecified which one will be returned.
+  bool LookUpCertificateBySpki(const std::string& subject_public_key_info,
+                               bool* is_currently_provided,
+                               CertificateInfo* info,
+                               std::string* extension_id);
+
   // Remove every association of stored certificates to the given extension.
   // The certificates themselves will be remembered.
   void RemoveExtension(const std::string& extension_id);
@@ -61,6 +80,7 @@ class ThreadSafeCertificateMap {
  private:
   base::Lock lock_;
   FingerprintToCertAndExtensionMap fingerprint_to_cert_and_extension_;
+  SpkiToCertAndExtensionMap spki_to_cert_and_extension_;
 
   DISALLOW_COPY_AND_ASSIGN(ThreadSafeCertificateMap);
 };