Reland <Wait for extensions in ChallengeResponse>

ChallengeResponseAuthKeysLoader now waits until force-installed login
screen extensions are installed before requesting a certificate from
these extensions, avoiding an incorrect error message.

In order to know which extensions to wait for, stored
ChallengeResponseKeys now contain the id of the extension that signed
them.

ChallengeResponseAuthKeysLoader will only wait for 5 seconds, then the
error is displayed anyway (e.g. if the extensions failed to load).

This is a reland of
https://chromium-review.googlesource.com/c/chromium/src/+/2078584
which was reverted in
https://chromium-review.googlesource.com/c/chromium/src/+/2101749
since a test added in this CL was crashing in debug mode:
A ExtensionLoadObserver object which waits on the loading of extensions
would, when all extensions had already loaded, indirectly destroy
itself in the middle of the function StartWaiting(). This was fixed by
ensuring that the function returns after any call that might cause
immediate destruction of the object.

Test: Compile with is_debug=true and run the browser_tests in this CL.

Bug: 1052404
Change-Id: I5b564d05c5da0a075d94772e4698673dcd418cd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2102651Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Commit-Queue: Fabian Sommer <fabiansommer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750935}

chrome/browser/chromeos/certificate_provider/certificate_provider_service.cc

@@ -6,7 +6,9 @@
 
 #include <stddef.h>
 
+#include <string>
 #include <utility>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -275,7 +277,7 @@ void CertificateProviderService::ReplyToSignRequest(
 
   if (!signature.empty()) {
     for (auto& observer : observers_)
-      observer.OnSignCompleted(certificate);
+      observer.OnSignCompleted(certificate, extension_id);
   }
 }
 
@@ -341,15 +343,15 @@ void CertificateProviderService::RequestSignatureBySpki(
                                 std::move(callback));
 }
 
-bool CertificateProviderService::GetSupportedAlgorithmsBySpki(
+bool CertificateProviderService::LookUpSpki(
     const std::string& subject_public_key_info,
-    std::vector<uint16_t>* supported_algorithms) {
+    std::vector<uint16_t>* supported_algorithms,
+    std::string* extension_id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   bool is_currently_provided = false;
   CertificateInfo info;
-  std::string extension_id;
   certificate_map_.LookUpCertificateBySpki(
-      subject_public_key_info, &is_currently_provided, &info, &extension_id);
+      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;

chrome/browser/chromeos/certificate_provider/certificate_provider_service.h

@@ -102,7 +102,8 @@ class CertificateProviderService : public KeyedService {
    public:
     // Called when a sign request gets successfully completed.
     virtual void OnSignCompleted(
-        const scoped_refptr<net::X509Certificate>& certificate) {}
+        const scoped_refptr<net::X509Certificate>& certificate,
+        const std::string& extension_id) {}
   };
 
   // |SetDelegate| must be called exactly once directly after construction.
@@ -179,14 +180,14 @@ class CertificateProviderService : public KeyedService {
       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);
+  // extension is currently providing such a certificate, fills |extension_id|,
+  // 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 LookUpSpki(const std::string& subject_public_key_info,
+                  std::vector<uint16_t>* supported_algorithms,
+                  std::string* extension_id);
 
   // Aborts all signature requests and related PIN dialogs that are associated
   // with the authentication of the given user.

chrome/browser/chromeos/certificate_provider/certificate_provider_service_unittest.cc

@@ -139,8 +139,9 @@ class TestDelegate : public CertificateProviderService::Delegate {
 
 class MockObserver : public CertificateProviderService::Observer {
  public:
-  MOCK_METHOD1(OnSignCompleted,
-               void(const scoped_refptr<net::X509Certificate>& certificate));
+  MOCK_METHOD2(OnSignCompleted,
+               void(const scoped_refptr<net::X509Certificate>& certificate,
+                    const std::string& extension_id));
 };
 
 }  // namespace
@@ -505,7 +506,7 @@ TEST_F(CertificateProviderServiceTest, SignRequest) {
   // No signature received until the extension replied to the service.
   EXPECT_TRUE(received_signature.empty());
 
-  EXPECT_CALL(observer_, OnSignCompleted(cert_info1_.certificate));
+  EXPECT_CALL(observer_, OnSignCompleted(cert_info1_.certificate, kExtension1));
 
   std::vector<uint8_t> signature_reply;
   signature_reply.push_back(5);
@@ -560,10 +561,12 @@ TEST_F(CertificateProviderServiceTest, SignUsingSpkiAsIdentification) {
   ASSERT_TRUE(cert);
 
   std::vector<uint16_t> supported_algorithms;
+  std::string extension_id;
   // If this fails, try to regenerate kClient1SpkiBase64 using the command shown
   // above.
-  EXPECT_TRUE(service_->GetSupportedAlgorithmsBySpki(client1_spki,
-                                                     &supported_algorithms));
+  EXPECT_TRUE(
+      service_->LookUpSpki(client1_spki, &supported_algorithms, &extension_id));
+  EXPECT_EQ(extension_id, kExtension1);
   EXPECT_THAT(supported_algorithms,
               testing::UnorderedElementsAre(SSL_SIGN_RSA_PKCS1_SHA256));
 
@@ -586,7 +589,7 @@ TEST_F(CertificateProviderServiceTest, SignUsingSpkiAsIdentification) {
   // No signature received until the extension replied to the service.
   EXPECT_TRUE(received_signature.empty());
 
-  EXPECT_CALL(observer_, OnSignCompleted(cert_info1_.certificate));
+  EXPECT_CALL(observer_, OnSignCompleted(cert_info1_.certificate, kExtension1));
 
   std::vector<uint8_t> signature_reply;
   signature_reply.push_back(5);

chrome/browser/chromeos/certificate_provider/test_certificate_provider_extension.cc

@@ -24,6 +24,7 @@
 #include "crypto/rsa_private_key.h"
 #include "extensions/browser/api/test/test_api.h"
 #include "extensions/browser/notification_types.h"
+#include "net/cert/asn1_util.h"
 #include "net/cert/x509_certificate.h"
 #include "net/cert/x509_util.h"
 #include "net/test/cert_test_util.h"
@@ -54,6 +55,10 @@ std::vector<uint8_t> ExtractBytesFromValue(const base::Value& value) {
   return bytes;
 }
 
+scoped_refptr<net::X509Certificate> GetCertificate() {
+  return net::ImportCertFromFile(net::GetTestCertsDirectory(), "client_1.pem");
+}
+
 base::span<const uint8_t> GetCertDer(const net::X509Certificate& certificate) {
   return base::as_bytes(base::make_span(
       net::x509_util::CryptoBufferAsStringPiece(certificate.cert_buffer())));
@@ -109,13 +114,24 @@ void SendReplyToJs(extensions::TestSendMessageFunction* function,
 
 }  // namespace
 
+// Returns the Spki of the certificate provided by the extension.
+std::string TestCertificateProviderExtension::GetCertificateSpki() {
+  const scoped_refptr<net::X509Certificate> certificate = GetCertificate();
+  base::StringPiece spki_bytes;
+  if (!net::asn1::ExtractSPKIFromDERCert(
+          net::x509_util::CryptoBufferAsStringPiece(certificate->cert_buffer()),
+          &spki_bytes)) {
+    return {};
+  }
+  return spki_bytes.as_string();
+}
+
 TestCertificateProviderExtension::TestCertificateProviderExtension(
     content::BrowserContext* browser_context,
     const std::string& extension_id)
     : browser_context_(browser_context),
       extension_id_(extension_id),
-      certificate_(net::ImportCertFromFile(net::GetTestCertsDirectory(),
-                                           "client_1.pem")),
+      certificate_(GetCertificate()),
       private_key_(net::key_util::LoadEVP_PKEYFromPEM(
           net::GetTestCertsDirectory().Append(
               FILE_PATH_LITERAL("client_1.key")))) {

chrome/browser/chromeos/certificate_provider/test_certificate_provider_extension.h

@@ -15,6 +15,7 @@
 #include "base/values.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "net/cert/x509_certificate.h"
 #include "third_party/boringssl/src/include/openssl/base.h"
 #include "third_party/boringssl/src/include/openssl/evp.h"
 
@@ -26,10 +27,6 @@ namespace content {
 class BrowserContext;
 }
 
-namespace net {
-class X509Certificate;
-}
-
 // This class provides the C++ side of the test certificate provider extension's
 // implementation (the JavaScript side is in
 // chrome/test/data/extensions/test_certificate_provider).
@@ -42,14 +39,13 @@ class X509Certificate;
 class TestCertificateProviderExtension final
     : public content::NotificationObserver {
  public:
+  // Returns the Spki of the certificate provided by the extension.
+  static std::string GetCertificateSpki();
+
   TestCertificateProviderExtension(content::BrowserContext* browser_context,
                                    const std::string& extension_id);
   ~TestCertificateProviderExtension() override;
 
-  const scoped_refptr<net::X509Certificate>& certificate() const {
-    return certificate_;
-  }
-
   // Sets the PIN that will be required when doing every signature request.
   // (By default, no PIN is requested.)
   void set_require_pin(const std::string& pin) { required_pin_ = pin; }

chrome/browser/chromeos/certificate_provider/test_certificate_provider_extension_login_screen_mixin.cc

@@ -30,13 +30,25 @@ constexpr char kTestCertProviderExtensionId[] =
 constexpr char kTestCertProviderExtensionUpdateManifestPath[] =
     "/extensions/test_certificate_provider/update_manifest.xml";
 
+Profile* GetProfile() {
+  return chromeos::ProfileHelper::GetSigninProfile()->GetOriginalProfile();
+}
+
 }  // namespace
 
+// static
+std::string TestCertificateProviderExtensionLoginScreenMixin::GetExtensionId() {
+  return kTestCertProviderExtensionId;
+}
+
 TestCertificateProviderExtensionLoginScreenMixin::
     TestCertificateProviderExtensionLoginScreenMixin(
         InProcessBrowserTestMixinHost* host,
-        chromeos::DeviceStateMixin* device_state_mixin)
-    : InProcessBrowserTestMixin(host), device_state_mixin_(device_state_mixin) {
+        chromeos::DeviceStateMixin* device_state_mixin,
+        bool load_extension_immediately)
+    : InProcessBrowserTestMixin(host),
+      device_state_mixin_(device_state_mixin),
+      load_extension_immediately_(load_extension_immediately) {
   base::FilePath test_data_dir;
   base::PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir);
   embedded_test_server_.ServeFilesFromDirectory(test_data_dir);
@@ -51,30 +63,35 @@ TestCertificateProviderExtensionLoginScreenMixin::
     ~TestCertificateProviderExtensionLoginScreenMixin() = default;
 
 void TestCertificateProviderExtensionLoginScreenMixin::SetUpOnMainThread() {
-  ASSERT_TRUE(embedded_test_server_.Start());
-
-  Profile* const profile =
-      chromeos::ProfileHelper::GetSigninProfile()->GetOriginalProfile();
   test_certificate_provider_extension_ =
-      std::make_unique<TestCertificateProviderExtension>(
-          profile, kTestCertProviderExtensionId);
+      std::make_unique<TestCertificateProviderExtension>(GetProfile(),
+                                                         GetExtensionId());
+  ASSERT_TRUE(embedded_test_server_.Start());
+  if (load_extension_immediately_) {
+    AddExtensionForForceInstallation();
+    WaitUntilExtensionLoaded();
+  }
+}
 
-  extensions::TestBackgroundPageFirstLoadObserver bg_page_first_load_observer(
-      profile, kTestCertProviderExtensionId);
+void TestCertificateProviderExtensionLoginScreenMixin::TearDownOnMainThread() {
+  test_certificate_provider_extension_.reset();
+}
 
+void TestCertificateProviderExtensionLoginScreenMixin::
+    AddExtensionForForceInstallation() {
   const GURL update_manifest_url = embedded_test_server_.GetURL(
       kTestCertProviderExtensionUpdateManifestPath);
   const std::string policy_item_value = base::ReplaceStringPlaceholders(
-      "$1;$2", {kTestCertProviderExtensionId, update_manifest_url.spec()},
-      nullptr);
+      "$1;$2", {GetExtensionId(), update_manifest_url.spec()}, nullptr);
   device_state_mixin_->RequestDevicePolicyUpdate()
       ->policy_payload()
       ->mutable_device_login_screen_extensions()
       ->add_device_login_screen_extensions(policy_item_value);
-
-  bg_page_first_load_observer.Wait();
 }
 
-void TestCertificateProviderExtensionLoginScreenMixin::TearDownOnMainThread() {
-  test_certificate_provider_extension_.reset();
+void TestCertificateProviderExtensionLoginScreenMixin::
+    WaitUntilExtensionLoaded() {
+  extensions::TestBackgroundPageFirstLoadObserver bg_page_first_load_observer(
+      GetProfile(), GetExtensionId());
+  bg_page_first_load_observer.Wait();
 }

chrome/browser/chromeos/certificate_provider/test_certificate_provider_extension_login_screen_mixin.h

@@ -24,9 +24,15 @@ class TestCertificateProviderExtension;
 class TestCertificateProviderExtensionLoginScreenMixin final
     : public InProcessBrowserTestMixin {
  public:
+  static std::string GetExtensionId();
+
+  // If |load_extension_immediately| is false,
+  // |AddExtensionForForceInstallation()| needs to be called by the test.
+  // Otherwise, the extension will be installed during setup.
   TestCertificateProviderExtensionLoginScreenMixin(
       InProcessBrowserTestMixinHost* host,
-      chromeos::DeviceStateMixin* device_state_mixin);
+      chromeos::DeviceStateMixin* device_state_mixin,
+      bool load_extension_immediately);
   TestCertificateProviderExtensionLoginScreenMixin(
       const TestCertificateProviderExtensionLoginScreenMixin&) = delete;
   TestCertificateProviderExtensionLoginScreenMixin& operator=(
@@ -45,9 +51,20 @@ class TestCertificateProviderExtensionLoginScreenMixin final
   void SetUpOnMainThread() override;
   void TearDownOnMainThread() override;
 
+  // Triggers async installation of the extension. Will be called during setup
+  // if |load_extension_immediately_| is set in ctor. In that case, should not
+  // be called again in tests.
+  void AddExtensionForForceInstallation();
+
+  // Waits until the extension is installed and loaded. Will be called during
+  // setup if |load_extension_immediately_| is set in ctor. In that case, should
+  // not be called again in tests.
+  void WaitUntilExtensionLoaded();
+
  private:
   chromeos::DeviceStateMixin* const device_state_mixin_;
   net::EmbeddedTestServer embedded_test_server_;
+  bool load_extension_immediately_;
   std::unique_ptr<TestCertificateProviderExtension>
       test_certificate_provider_extension_;
 };

chrome/browser/chromeos/dbus/cryptohome_key_delegate_service_provider.cc

@@ -156,9 +156,10 @@ void HandleSignatureKeyChallenge(
   }
 
   std::vector<uint16_t> supported_ssl_algorithms;
-  if (!certificate_provider_service->GetSupportedAlgorithmsBySpki(
+  std::string extension_id_ignored;
+  if (!certificate_provider_service->LookUpSpki(
           challenge_request_data.public_key_spki_der(),
-          &supported_ssl_algorithms)) {
+          &supported_ssl_algorithms, &extension_id_ignored)) {
     std::move(response_sender)
         .Run(dbus::ErrorResponse::FromMethodCall(method_call, DBUS_ERROR_FAILED,
                                                  "Key is unavailable"));

chrome/browser/chromeos/dbus/cryptohome_key_delegate_service_provider_browsertest.cc

@@ -32,30 +32,13 @@
 #include "components/account_id/account_id.h"
 #include "components/user_manager/user_names.h"
 #include "content/public/browser/browser_context.h"
+#include "crypto/signature_verifier.h"
 #include "dbus/message.h"
 #include "dbus/object_path.h"
-#include "net/cert/asn1_util.h"
-#include "net/cert/x509_certificate.h"
-#include "net/cert/x509_util.h"
 #include "net/ssl/client_cert_identity.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/cryptohome/dbus-constants.h"
 
-namespace {
-
-// Extracts the SubjectPublicKeyInfo from the given certificate.
-std::string GetCertSpki(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();
-}
-
-}  // namespace
-
 // Tests for the CryptohomeKeyDelegateServiceProvider class.
 class CryptohomeKeyDelegateServiceProviderTest
     : public MixinBasedInProcessBrowserTest {
@@ -132,7 +115,7 @@ class CryptohomeKeyDelegateServiceProviderTest
         cryptohome::KeyChallengeRequest::CHALLENGE_TYPE_SIGNATURE);
     request.mutable_signature_request_data()->set_data_to_sign(kDataToSign);
     request.mutable_signature_request_data()->set_public_key_spki_der(
-        GetCertSpki(*cert_provider_extension()->certificate()));
+        cert_provider_extension()->GetCertificateSpki());
     request.mutable_signature_request_data()->set_signature_algorithm(
         signature_algorithm);
 
@@ -162,8 +145,7 @@ class CryptohomeKeyDelegateServiceProviderTest
   // data.
   bool IsSignatureValid(crypto::SignatureVerifier::SignatureAlgorithm algorithm,
                         const std::vector<uint8_t>& signature) const {
-    const std::string spki =
-        GetCertSpki(*cert_provider_extension()->certificate());
+    const std::string spki = cert_provider_extension()->GetCertificateSpki();
     crypto::SignatureVerifier verifier;
     if (!verifier.VerifyInit(algorithm, signature,
                              base::as_bytes(base::make_span(spki)))) {
@@ -188,7 +170,8 @@ class CryptohomeKeyDelegateServiceProviderTest
       &mixin_host_,
       chromeos::DeviceStateMixin::State::OOBE_COMPLETED_CLOUD_ENROLLED};
   TestCertificateProviderExtensionLoginScreenMixin
-      cert_provider_extension_mixin_{&mixin_host_, &device_state_mixin_};
+      cert_provider_extension_mixin_{&mixin_host_, &device_state_mixin_,
+                                     /*load_extension_immediately=*/true};
 
   chromeos::CryptohomeKeyDelegateServiceProvider service_provider_;
   std::unique_ptr<chromeos::ServiceProviderTestHelper>

chrome/browser/chromeos/login/challenge_response_auth_keys_loader.h

@@ -8,8 +8,6 @@
 #include <string>
 #include <vector>
 
-#include "base/callback_forward.h"
-#include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "chromeos/login/auth/challenge_response_key.h"
 #include "net/ssl/client_cert_identity.h"
@@ -38,30 +36,48 @@ class ChallengeResponseAuthKeysLoader final {
   static bool CanAuthenticateUser(const AccountId& account_id);
 
   ChallengeResponseAuthKeysLoader();
+  ChallengeResponseAuthKeysLoader(const ChallengeResponseAuthKeysLoader&) =
+      delete;
+  ChallengeResponseAuthKeysLoader& operator=(
+      const ChallengeResponseAuthKeysLoader&) = delete;
   ~ChallengeResponseAuthKeysLoader();
 
   // Prepares the ChallengeResponseKey values containing the currently available
-  // cryptographic keys that can be used to authenticate the given user.
+  // cryptographic keys that can be used to authenticate the given user. If
+  // there should be force-installed extensions that provide a certificate for
+  // the given user, waits until these are installed and loaded (default: up to
+  // 5 seconds, configured by |maximum_extension_load_waiting_time_|).
   //
   // The callback is run with an empty |challenge_response_keys| in the cases
   // when the user's profile doesn't support challenge-response authentication
-  // or when there's no currently available suitable cryptographic key.
+  // or when there is no suitable cryptographic key available.
   void LoadAvailableKeys(const AccountId& account_id,
                          LoadAvailableKeysCallback callback);
 
+  void SetMaxWaitTimeForTesting(base::TimeDelta time) {
+    maximum_extension_load_waiting_time_ = time;
+  }
+
  private:
-  // Asynchronous job which is scheduled by LoadAvailableKeys after the list of
-  // currently available cryptographic keys is refreshed from certificate
-  // providers.
+  // Asynchronous job which is scheduled by LoadAvailableKeys after all
+  // necessary extensions are loaded.
+  void ContinueLoadAvailableKeysExtensionsLoaded(
+      const AccountId& account_id,
+      const std::vector<std::string>& suitable_public_key_spki_items,
+      LoadAvailableKeysCallback callback);
+
+  // Asynchronous job which is scheduled by
+  // ContinueLoadAvailableKeysExtensionsLoaded after the list of currently
+  // available cryptographic keys is refreshed from certificate providers.
   void ContinueLoadAvailableKeysWithCerts(
       const AccountId& account_id,
       const std::vector<std::string>& suitable_public_key_spki_items,
       LoadAvailableKeysCallback callback,
       net::ClientCertIdentityList cert_identities);
 
-  base::WeakPtrFactory<ChallengeResponseAuthKeysLoader> weak_ptr_factory_{this};
+  base::TimeDelta maximum_extension_load_waiting_time_;
 
-  DISALLOW_COPY_AND_ASSIGN(ChallengeResponseAuthKeysLoader);
+  base::WeakPtrFactory<ChallengeResponseAuthKeysLoader> weak_ptr_factory_{this};
 };
 
 }  // namespace chromeos

chrome/browser/chromeos/login/login_client_cert_usage_observer.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/login/login_client_cert_usage_observer.h"
 
 #include <cstdint>
+#include <string>
 
 #include "base/logging.h"
 #include "base/optional.h"
@@ -39,8 +40,9 @@ bool ObtainSignatureAlgorithms(
     return false;
   }
   std::vector<uint16_t> ssl_algorithms;
-  if (!certificate_provider_service->GetSupportedAlgorithmsBySpki(
-          spki.as_string(), &ssl_algorithms)) {
+  std::string extension_id;
+  if (!certificate_provider_service->LookUpSpki(
+          spki.as_string(), &ssl_algorithms, &extension_id)) {
     return false;
   }
   signature_algorithms->clear();
@@ -69,8 +71,8 @@ bool LoginClientCertUsageObserver::ClientCertsWereUsed() const {
 
 bool LoginClientCertUsageObserver::GetOnlyUsedClientCert(
     scoped_refptr<net::X509Certificate>* cert,
-    std::vector<ChallengeResponseKey::SignatureAlgorithm>* signature_algorithms)
-    const {
+    std::vector<ChallengeResponseKey::SignatureAlgorithm>* signature_algorithms,
+    std::string* extension_id) const {
   if (!used_cert_count_)
     return false;
   if (used_cert_count_ > 1) {
@@ -83,14 +85,17 @@ bool LoginClientCertUsageObserver::GetOnlyUsedClientCert(
   if (!ObtainSignatureAlgorithms(*used_cert_, signature_algorithms))
     return false;
   *cert = used_cert_;
+  *extension_id = used_extension_id_;
   return true;
 }
 
 void LoginClientCertUsageObserver::OnSignCompleted(
-    const scoped_refptr<net::X509Certificate>& certificate) {
+    const scoped_refptr<net::X509Certificate>& certificate,
+    const std::string& extension_id) {
   if (!used_cert_ || !used_cert_->EqualsExcludingChain(certificate.get()))
     ++used_cert_count_;
   used_cert_ = certificate;
+  used_extension_id_ = extension_id;
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/login/login_client_cert_usage_observer.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_CHROMEOS_LOGIN_LOGIN_CLIENT_CERT_USAGE_OBSERVER_H_
 #define CHROME_BROWSER_CHROMEOS_LOGIN_LOGIN_CLIENT_CERT_USAGE_OBSERVER_H_
 
+#include <string>
 #include <vector>
 
 #include "base/macros.h"
@@ -28,21 +29,25 @@ class LoginClientCertUsageObserver final
   // Returns whether exactly one unique certificate was used, and, if so,
   // writes this certificate to |cert| and appends the signature algorithms
   // supported by its provider into |signature_algorithms|.
+  // The certificate was signed by the extension |extension_id|.
   bool GetOnlyUsedClientCert(
       scoped_refptr<net::X509Certificate>* cert,
       std::vector<ChallengeResponseKey::SignatureAlgorithm>*
-          signature_algorithms) const;
+          signature_algorithms,
+      std::string* extension_id) const;
 
  private:
   // CertificateProviderService::Observer:
-  void OnSignCompleted(
-      const scoped_refptr<net::X509Certificate>& certificate) override;
+  void OnSignCompleted(const scoped_refptr<net::X509Certificate>& certificate,
+                       const std::string& extension_id) override;
 
   // How many times the client certificate, used on the login screen, has
   // changed.
   int used_cert_count_ = 0;
   // One of the client certificates that was used on the login screen.
   scoped_refptr<net::X509Certificate> used_cert_;
+  // The extension that signed |used_cert_|.
+  std::string used_extension_id_;
 
   DISALLOW_COPY_AND_ASSIGN(LoginClientCertUsageObserver);
 };

chrome/browser/chromeos/login/saml/security_token_saml_browsertest.cc

@@ -205,7 +205,8 @@ class SecurityTokenSamlTest : public OobeBaseTest {
       &mixin_host_, &gaia_mixin_,
       /*client_cert_authorities=*/{GetClientCertCaName()}};
   TestCertificateProviderExtensionLoginScreenMixin
-      cert_provider_extension_mixin_{&mixin_host_, &device_state_mixin_};
+      cert_provider_extension_mixin_{&mixin_host_, &device_state_mixin_,
+                                     /*load_extension_immediately=*/true};
   int pin_dialog_shown_count_ = 0;
   base::RunLoop* pin_dialog_shown_run_loop_ = nullptr;
   base::WeakPtrFactory<SecurityTokenSamlTest> weak_factory_{this};

chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc

@@ -1543,8 +1543,9 @@ bool GaiaScreenHandler::BuildUserContextForGaiaSignIn(
       extension_provided_client_cert_usage_observer_->ClientCertsWereUsed()) {
     scoped_refptr<net::X509Certificate> saml_client_cert;
     std::vector<ChallengeResponseKey::SignatureAlgorithm> signature_algorithms;
+    std::string extension_id;
     if (!extension_provided_client_cert_usage_observer_->GetOnlyUsedClientCert(
-            &saml_client_cert, &signature_algorithms)) {
+            &saml_client_cert, &signature_algorithms, &extension_id)) {
       *error_message = l10n_util::GetStringUTF8(
           IDS_CHALLENGE_RESPONSE_AUTH_MULTIPLE_CLIENT_CERTS_ERROR);
       return false;
@@ -1556,6 +1557,7 @@ bool GaiaScreenHandler::BuildUserContextForGaiaSignIn(
           IDS_CHALLENGE_RESPONSE_AUTH_INVALID_CLIENT_CERT_ERROR);
       return false;
     }
+    challenge_response_key.set_extension_id(extension_id);
     user_context->GetMutableChallengeResponseKeys()->push_back(
         challenge_response_key);
   } else {

chrome/test/BUILD.gn

@@ -2175,6 +2175,7 @@ if (!is_android) {
         "../browser/chromeos/login/active_directory_login_browsertest.cc",
         "../browser/chromeos/login/arc_terms_of_service_browsertest.cc",
         "../browser/chromeos/login/auto_launched_kiosk_browsertest.cc",
+        "../browser/chromeos/login/challenge_response_auth_keys_loader_browsertest.cc",
         "../browser/chromeos/login/configuration_based_oobe_browsertest.cc",
         "../browser/chromeos/login/crash_restore_browsertest.cc",
         "../browser/chromeos/login/demo_mode/demo_app_launcher_browsertest.cc",

chromeos/login/auth/challenge_response/known_user_pref_utils.cc

@@ -15,6 +15,40 @@ namespace chromeos {
 namespace {
 
 constexpr char kPublicKeySpkiKey[] = "public_key_spki";
+constexpr char kExtensionId[] = "extension_id";
+
+void DeserializeSpkiKey(const base::Value& key_representation,
+                        std::string* spki_key) {
+  const base::Value* spki_base64 = key_representation.FindKeyOfType(
+      kPublicKeySpkiKey, base::Value::Type::STRING);
+  if (!spki_base64) {
+    LOG(WARNING)
+        << "Ignoring challenge-response key info: missing SPKI property";
+    return;
+  }
+  if (!base::Base64Decode(spki_base64->GetString(), spki_key)) {
+    LOG(WARNING) << "Ignoring challenge-response key info: invalid SPKI base64";
+    return;
+  }
+  if (spki_key->empty()) {
+    LOG(WARNING) << "Ignoring challenge-response key info: empty SPKI";
+  }
+}
+
+void DeserializeExtensionId(const base::Value& key_representation,
+                            std::string* extension_id) {
+  const base::Value* extension_id_value =
+      key_representation.FindKeyOfType(kExtensionId, base::Value::Type::STRING);
+  if (!extension_id_value) {
+    LOG(WARNING)
+        << "Missing extension id property in challenge-response key info.";
+    return;
+  }
+  *extension_id = extension_id_value->GetString();
+  if (extension_id->empty()) {
+    LOG(WARNING) << "Empty extension id in challenge-response key info.";
+  }
+}
 
 }  // namespace
 
@@ -26,15 +60,17 @@ base::Value SerializeChallengeResponseKeysForKnownUser(
     base::Base64Encode(key.public_key_spki_der(), &spki_base64);
     base::Value key_representation(base::Value::Type::DICTIONARY);
     key_representation.SetKey(kPublicKeySpkiKey, base::Value(spki_base64));
+    key_representation.SetKey(kExtensionId, base::Value(key.extension_id()));
     pref_value.Append(std::move(key_representation));
   }
   return pref_value;
 }
 
-bool DeserializeChallengeResponseKeysFromKnownUser(
+bool DeserializeChallengeResponseKeyFromKnownUser(
     const base::Value& pref_value,
-    std::vector<std::string>* public_key_spki_list) {
-  public_key_spki_list->clear();
+    std::vector<DeserializedChallengeResponseKey>*
+        deserialized_challenge_response_keys) {
+  deserialized_challenge_response_keys->clear();
   if (!pref_value.is_list())
     return false;
   for (const base::Value& key_representation : pref_value.GetList()) {
@@ -42,26 +78,18 @@ bool DeserializeChallengeResponseKeysFromKnownUser(
       LOG(WARNING) << "Ignoring challenge-response key info: not a dictionary";
       continue;
     }
-    const base::Value* spki_base64 = key_representation.FindKeyOfType(
-        kPublicKeySpkiKey, base::Value::Type::STRING);
-    if (!spki_base64) {
-      LOG(WARNING)
-          << "Ignoring challenge-response key info: missing SPKI property";
-      continue;
-    }
-    std::string spki;
-    if (!base::Base64Decode(spki_base64->GetString(), &spki)) {
-      LOG(WARNING)
-          << "Ignoring challenge-response key info: invalid SPKI base64";
-      continue;
-    }
-    if (spki.empty()) {
-      LOG(WARNING) << "Ignoring challenge-response key info: empty SPKI";
-      continue;
+    DeserializedChallengeResponseKey deserialized_challenge_response_key;
+    DeserializeSpkiKey(
+        key_representation,
+        &deserialized_challenge_response_key.public_key_spki_der);
+    DeserializeExtensionId(key_representation,
+                           &deserialized_challenge_response_key.extension_id);
+    if (!deserialized_challenge_response_key.public_key_spki_der.empty()) {
+      deserialized_challenge_response_keys->push_back(
+          deserialized_challenge_response_key);
     }
-    public_key_spki_list->emplace_back(std::move(spki));
   }
-  return !public_key_spki_list->empty();
+  return !deserialized_challenge_response_keys->empty();
 }
 
 }  // namespace chromeos

chromeos/login/auth/challenge_response/known_user_pref_utils.h

@@ -23,14 +23,17 @@ namespace chromeos {
 // The format currently is a list of dictionaries, each with the following keys:
 // * "public_key_spki" - contains the base64-encoded DER blob of the X.509
 //   Subject Public Key Info.
+// * "extension_id" - contains the base64-encoded id of the extension that is
+//   used to sign the key.
 base::Value COMPONENT_EXPORT(CHROMEOS_LOGIN_AUTH)
     SerializeChallengeResponseKeysForKnownUser(
         const std::vector<ChallengeResponseKey>& challenge_response_keys);
 
 bool COMPONENT_EXPORT(CHROMEOS_LOGIN_AUTH)
-    DeserializeChallengeResponseKeysFromKnownUser(
+    DeserializeChallengeResponseKeyFromKnownUser(
         const base::Value& pref_value,
-        std::vector<std::string>* public_key_spki_list);
+        std::vector<DeserializedChallengeResponseKey>*
+            deserialized_challenge_response_keys);
 
 }  // namespace chromeos
 

chromeos/login/auth/challenge_response_key.h

@@ -14,8 +14,9 @@ namespace chromeos {
 
 // This class contains information about a challenge-response key for user
 // authentication. This includes information about the public key of the
-// cryptographic key to be challenged, as well as the signature algorithms
-// supported for the challenge.
+// cryptographic key to be challenged, the signature algorithms supported for
+// the challenge, and the id of the extension that handles the
+// challenge-response.
 class COMPONENT_EXPORT(CHROMEOS_LOGIN_AUTH) ChallengeResponseKey {
  public:
   // Cryptographic signature algorithm type for challenge requests.
@@ -53,9 +54,22 @@ class COMPONENT_EXPORT(CHROMEOS_LOGIN_AUTH) ChallengeResponseKey {
     signature_algorithms_ = signature_algorithms;
   }
 
+  // Getter and setter for the id of the extension that is used to sign the key.
+  const std::string& extension_id() const { return extension_id_; }
+  void set_extension_id(const std::string& extension_id) {
+    extension_id_ = extension_id;
+  }
+
  private:
   std::string public_key_spki_der_;
   std::vector<SignatureAlgorithm> signature_algorithms_;
+  std::string extension_id_;
+};
+
+// Contains the parts of a ChallengeResponseKey that can be persisted to disk.
+struct COMPONENT_EXPORT(CHROMEOS_LOGIN_AUTH) DeserializedChallengeResponseKey {
+  std::string public_key_spki_der;
+  std::string extension_id;
 };
 
 }  // namespace chromeos