PlatformKeysService functions for built-in certificate provisioning

This CL extends PlatformKeysService by introducing the following new
functions:
- RemoveKey
This function will be used to remove the key pair corresponding to
a given DER-encoded SubjectPublicKeyInfo.

- GetAttributeForKey
This function retrieves the value of an attribute for a private key
corresponding to a provided DER-encoded public key.

- SetAttributeForKey
This function sets the value of an attribute for a private key
corresponding to a provided DER-encoded public key.

This is not planned to be exposed as an actual extension API. Instead,
the functions are implemented mainly to be used by another component
(built-in cert_provisioning).

Bug: 1045895
Test: browser_tests --gtest_filter= *PlatformKeys*
Change-Id: I8797e690487b866c80377aafce615546afd8f54d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151868Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Omar Morsi <omorsi@google.com>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Michael Ershov <miersh@google.com>
Commit-Queue: Omar Morsi <omorsi@google.com>
Cr-Commit-Position: refs/heads/master@{#762796}

chrome/browser/chromeos/platform_keys/mock_platform_keys_service.h

@@ -92,6 +92,13 @@ class MockPlatformKeysService : public PlatformKeysService {
                const RemoveCertificateCallback& callback),
               (override));
 
+  MOCK_METHOD(void,
+              RemoveKey,
+              (const std::string& token_id,
+               const std::string& public_key_spki_der,
+               RemoveKeyCallback callback),
+              (override));
+
   MOCK_METHOD(void, GetTokens, (const GetTokensCallback& callback), (override));
 
   MOCK_METHOD(void,
@@ -99,6 +106,28 @@ class MockPlatformKeysService : public PlatformKeysService {
               (const std::string& public_key_spki_der,
                const GetKeyLocationsCallback& callback),
               (override));
+
+  MOCK_METHOD(void,
+              SetAttributeForKey,
+              (const std::string& token_id,
+               const std::string& public_key_spki_der,
+               KeyAttributeType attribute_type,
+               const std::string& attribute_value,
+               SetAttributeForKeyCallback callback),
+              (override));
+
+  MOCK_METHOD(void,
+              GetAttributeForKey,
+              (const std::string& token_id,
+               const std::string& public_key_spki_der,
+               KeyAttributeType attribute_type,
+               GetAttributeForKeyCallback callback),
+              (override));
+
+  MOCK_METHOD(void,
+              SetMapToSoftokenAttrsForTesting,
+              (bool map_to_softoken_attrs_for_testing),
+              (override));
 };
 
 std::unique_ptr<KeyedService> BuildMockPlatformKeysService(

chrome/browser/chromeos/platform_keys/platform_keys_service.h

@@ -36,6 +36,9 @@ extern const char kTokenIdSystem[];
 // Supported key types.
 enum class KeyType { kRsassaPkcs1V15, kEcdsa };
 
+// Supported key attribute types.
+enum class KeyAttributeType { CertificateProvisioningId };
+
 // Supported hash algorithms.
 enum HashAlgorithm {
   HASH_ALGORITHM_NONE,  // The value if no hash function is selected.
@@ -126,6 +129,11 @@ using ImportCertificateCallback =
 using RemoveCertificateCallback =
     base::Callback<void(const std::string& error_message)>;
 
+// If the the key pair has been successfully removed, |error_message| will be
+// empty. If an error occurs, |error_message| will be set to the error message.
+using RemoveKeyCallback =
+    base::OnceCallback<void(const std::string& error_message)>;
+
 // If the list of available tokens could be successfully retrieved, |token_ids|
 // will contain the token ids. If an error occurs, |token_ids| will be nullptr
 // and |error_message| will be set to an error message.
@@ -149,9 +157,24 @@ using GetKeyLocationsCallback =
     base::RepeatingCallback<void(const std::vector<std::string>& token_ids,
                                  const std::string& error_message)>;
 
+// If the attribute value has been successfully set, |error_message| will be
+// empty.
+// If an error occurs, |error_message| will be set to the error message.
+using SetAttributeForKeyCallback =
+    base::OnceCallback<void(const std::string& error_message)>;
+
+// If the attribute value has been successfully retrieved, |attribute_value|
+// will contain the result and |error_message| will be empty.
+// If an error occurs, |attribute_value| will be empty and |error_message| will
+// be set to the error message.
+using GetAttributeForKeyCallback =
+    base::OnceCallback<void(const std::string& attribute_value,
+                            const std::string& error_message)>;
+
 // Functions of this class shouldn't be called directly from the context of
 // an extension. Instead use ExtensionPlatformKeysService which enforces
 // restrictions upon extensions.
+// All public methods of this class should be called on the UI thread.
 class PlatformKeysService : public KeyedService {
  public:
   PlatformKeysService() = default;
@@ -225,7 +248,7 @@ class PlatformKeysService : public KeyedService {
   // Returns the list of all keys available from the given |token_id| as a list
   // of der-encoded SubjectPublicKeyInfo strings. |callback| will be invoked on
   // the UI thread with the list of available public keys, possibly with an
-  // error message. Must be called on the UI thread.
+  // error message.
   virtual void GetAllKeys(const std::string& token_id,
                           GetAllKeysCallback callback) = 0;
 
@@ -250,17 +273,54 @@ class PlatformKeysService : public KeyedService {
       const scoped_refptr<net::X509Certificate>& certificate,
       const RemoveCertificateCallback& callback) = 0;
 
+  // Removes the key pair if no matching certificates exist. Only keys in the
+  // given |token_id| are considered. |callback| will be invoked on the UI
+  // thread when the removal is finished, possibly with an error message.
+  virtual void RemoveKey(const std::string& token_id,
+                         const std::string& public_key_spki_der,
+                         RemoveKeyCallback callback) = 0;
+
   // Gets the list of available tokens. |callback| will be invoked when the list
   // of available tokens is determined, possibly with an error message.
-  // Must be called and calls |callback| on the UI thread.
+  // Calls |callback| on the UI thread.
   virtual void GetTokens(const GetTokensCallback& callback) = 0;
 
   // Determines the token(s) on which the private key corresponding to
   // |public_key_spki_der| is stored. |callback| will be invoked when the token
-  // ids are determined, possibly with an error message. Must be called and
-  // calls |callback| on the UI thread.
+  // ids are determined, possibly with an error message. Calls |callback| on the
+  // UI thread.
   virtual void GetKeyLocations(const std::string& public_key_spki_der,
                                const GetKeyLocationsCallback& callback) = 0;
+
+  // Sets |attribute_type| for the private key corresponding to
+  // |public_key_spki_der| to |attribute_value| only if the key is in
+  // |token_id|. |callback| will be invoked on the UI thread when setting the
+  // attribute is done, possibly with an error message.
+  virtual void SetAttributeForKey(const std::string& token_id,
+                                  const std::string& public_key_spki_der,
+                                  KeyAttributeType attribute_type,
+                                  const std::string& attribute_value,
+                                  SetAttributeForKeyCallback callback) = 0;
+
+  // Gets |attribute_type| for the private key corresponding to
+  // |public_key_spki_der| only if the key is in |token_id|.
+  // |callback| will be invoked on the UI thread when getting the attribute
+  // is done, possibly with an error message.
+  virtual void GetAttributeForKey(const std::string& token_id,
+                                  const std::string& public_key_spki_der,
+                                  KeyAttributeType attribute_type,
+                                  GetAttributeForKeyCallback callback) = 0;
+
+  // Softoken NSS PKCS11 module (used for testing) allows only predefined key
+  // attributes to be set and retrieved. Chaps supports setting and retrieving
+  // custom attributes.
+  // If |map_to_softoken_attrs_for_testing| is true, the service will use
+  // fake KeyAttribute mappings predefined in softoken module for testing.
+  // Otherwise, the real mappings to constants in
+  // third_party/cros_system_api/constants/pkcs11_custom_attributes.h will be
+  // used.
+  virtual void SetMapToSoftokenAttrsForTesting(
+      const bool map_to_softoken_attrs_for_testing) = 0;
 };
 
 class PlatformKeysServiceImpl final : public PlatformKeysService {
@@ -302,12 +362,28 @@ class PlatformKeysServiceImpl final : public PlatformKeysService {
   void RemoveCertificate(const std::string& token_id,
                          const scoped_refptr<net::X509Certificate>& certificate,
                          const RemoveCertificateCallback& callback) override;
+  void RemoveKey(const std::string& token_id,
+                 const std::string& public_key_spki_der,
+                 RemoveKeyCallback callback) override;
   void GetTokens(const GetTokensCallback& callback) override;
   void GetKeyLocations(const std::string& public_key_spki_der,
                        const GetKeyLocationsCallback& callback) override;
+  void SetAttributeForKey(const std::string& token_id,
+                          const std::string& public_key_spki_der,
+                          KeyAttributeType attribute_type,
+                          const std::string& attribute_value,
+                          SetAttributeForKeyCallback callback) override;
+  void GetAttributeForKey(const std::string& token_id,
+                          const std::string& public_key_spki_der,
+                          KeyAttributeType attribute_type,
+                          GetAttributeForKeyCallback callback) override;
+  void SetMapToSoftokenAttrsForTesting(
+      bool map_to_softoken_attrs_for_testing) override;
+  bool IsSetMapToSoftokenAttrsForTesting();
 
  private:
   content::BrowserContext* const browser_context_;
+  bool map_to_softoken_attrs_for_testing_ = false;
   base::WeakPtrFactory<PlatformKeysServiceImpl> weak_factory_{this};
 };
 

chrome/browser/chromeos/platform_keys/platform_keys_service_browsertest.cc

@@ -3,8 +3,10 @@
 // found in the LICENSE file.
 
 #include <memory>
+#include <string>
 #include <type_traits>
 #include <utility>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ -14,6 +16,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
 #include "base/task/post_task.h"
+#include "base/threading/thread_restrictions.h"
 #include "chrome/browser/chromeos/login/test/device_state_mixin.h"
 #include "chrome/browser/chromeos/login/test/login_manager_mixin.h"
 #include "chrome/browser/chromeos/login/test/scoped_policy_update.h"
@@ -22,6 +25,7 @@
 #include "chrome/browser/chromeos/platform_keys/platform_keys_service_factory.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/scoped_test_system_nss_key_slot_mixin.h"
+#include "chrome/browser/net/nss_context.h"
 #include "chrome/browser/policy/policy_test_utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -35,9 +39,17 @@
 #include "components/signin/public/identity_manager/identity_test_utils.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "crypto/nss_key_util.h"
+#include "crypto/scoped_nss_types.h"
 #include "crypto/signature_verifier.h"
+#include "net/cert/nss_cert_database.h"
+#include "net/cert/x509_certificate.h"
+#include "net/cert/x509_util_nss.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/cros_system_api/constants/pkcs11_custom_attributes.h"
 
 namespace chromeos {
 namespace platform_keys {
@@ -45,6 +57,8 @@ namespace {
 
 constexpr char kTestUserEmail[] = "test@example.com";
 constexpr char kTestAffiliationId[] = "test_affiliation_id";
+constexpr char kSystemToken[] = "system";
+constexpr char kUserToken[] = "user";
 
 enum class ProfileToUse {
   // A Profile that belongs to a user that is not affiliated with the device (no
@@ -68,6 +82,37 @@ struct TestConfig {
   std::vector<std::string> token_ids;
 };
 
+// Softoken NSS PKCS11 module (used for testing) allows only predefined key
+// attributes to be set and retrieved. Chaps supports setting and retrieving
+// custom attributes.
+// This helper is created to allow setting and retrieving attributes
+// supported by PlatformKeysService. For the lifetime of an instance of this
+// helper, PlatformKeysService will be configured to use softoken key attribute
+// mapping for the lifetime of the helper.
+class ScopedSoftokenAttrsMapping {
+ public:
+  explicit ScopedSoftokenAttrsMapping(
+      PlatformKeysService* platform_keys_service);
+  ScopedSoftokenAttrsMapping(const ScopedSoftokenAttrsMapping& other) = delete;
+  ScopedSoftokenAttrsMapping& operator=(
+      const ScopedSoftokenAttrsMapping& other) = delete;
+  ~ScopedSoftokenAttrsMapping();
+
+ private:
+  PlatformKeysService* const platform_keys_service_;
+};
+
+ScopedSoftokenAttrsMapping::ScopedSoftokenAttrsMapping(
+    PlatformKeysService* platform_keys_service)
+    : platform_keys_service_(platform_keys_service) {
+  platform_keys_service_->SetMapToSoftokenAttrsForTesting(true);
+}
+
+ScopedSoftokenAttrsMapping::~ScopedSoftokenAttrsMapping() {
+  DCHECK(platform_keys_service_);
+  platform_keys_service_->SetMapToSoftokenAttrsForTesting(false);
+}
+
 // A helper that waits until execution of an asynchronous PlatformKeysService
 // operation has finished and provides access to the results.
 // Note: all PlatformKeysService operations have a trailing const std::string&
@@ -165,13 +210,53 @@ class SignExecutionWaiter : public ExecutionWaiter<const std::string&> {
   }
 };
 
+// Supports waiting for the result of the PlatformKeysService::GetCertificates.
+class GetCertificatesExecutionWaiter
+    : public ExecutionWaiter<std::unique_ptr<net::CertificateList>> {
+ public:
+  GetCertificatesExecutionWaiter() = default;
+  ~GetCertificatesExecutionWaiter() = default;
+
+  const net::CertificateList& matches() const {
+    return *std::get<0>(result_callback_args());
+  }
+};
+
+// Supports waiting for the result of the
+// PlatformKeysService::SetAttributeForKey.
+class SetAttributeForKeyExecutionWaiter : public ExecutionWaiter<> {
+ public:
+  SetAttributeForKeyExecutionWaiter() = default;
+  ~SetAttributeForKeyExecutionWaiter() = default;
+};
+
+// Supports waiting for the result of the
+// PlatformKeysService::GetAttributeForKey.
+class GetAttributeForKeyExecutionWaiter
+    : public ExecutionWaiter<const std::string&> {
+ public:
+  GetAttributeForKeyExecutionWaiter() = default;
+  ~GetAttributeForKeyExecutionWaiter() = default;
+
+  const std::string& attribute_value() const {
+    return std::get<0>(result_callback_args());
+  }
+};
+
+// Supports waiting for the result of the PlatformKeysService::RemoveKey.
+class RemoveKeyExecutionWaiter : public ExecutionWaiter<> {
+ public:
+  RemoveKeyExecutionWaiter() = default;
+  ~RemoveKeyExecutionWaiter() = default;
+};
+
 class GetAllKeysExecutionWaiter
     : public ExecutionWaiter<std::vector<std::string>> {
  public:
   GetAllKeysExecutionWaiter() = default;
   ~GetAllKeysExecutionWaiter() = default;
 
-  const std::vector<std::string> public_keys() const {
+  const std::vector<std::string>& public_keys() const {
     return std::get<0>(result_callback_args());
   }
 };
@@ -212,12 +297,30 @@ class PlatformKeysServiceBrowserTest
       ASSERT_TRUE(login_manager_mixin_.LoginAndWaitForActiveSession(
           LoginManagerMixin::CreateDefaultUserContext(test_user_info_)));
       profile_ = ProfileManager::GetActiveUserProfile();
+
+      base::RunLoop loop;
+      GetNSSCertDatabaseForProfile(
+          profile_,
+          base::BindRepeating(&PlatformKeysServiceBrowserTest::SetUserSlot,
+                              base::Unretained(this), loop.QuitClosure()));
+      loop.Run();
     }
     ASSERT_TRUE(profile_);
 
     platform_keys_service_ =
         PlatformKeysServiceFactory::GetForBrowserContext(profile_);
     ASSERT_TRUE(platform_keys_service_);
+
+    scoped_softoken_attrs_mapping_ =
+        std::make_unique<ScopedSoftokenAttrsMapping>(platform_keys_service_);
+  }
+
+  void TearDownOnMainThread() override {
+    MixinBasedInProcessBrowserTest::TearDownOnMainThread();
+
+    // Destroy |scoped_softoken_attrs_mapping_| before |platform_keys_service_|
+    // is destroyed.
+    scoped_softoken_attrs_mapping_.reset();
   }
 
  protected:
@@ -225,6 +328,15 @@ class PlatformKeysServiceBrowserTest
     return platform_keys_service_;
   }
 
+  // Returns the slot to be used depending on |token_id|.
+  PK11SlotInfo* GetSlot(const std::string& token_id) {
+    if (token_id == kSystemToken) {
+      return system_nss_key_slot_mixin_.slot();
+    }
+    DCHECK_EQ(token_id, kUserToken);
+    return user_slot_.get();
+  }
+
   // Generates a key pair in the given |token_id| using platform keys service
   // and returns the SubjectPublicKeyInfo string encoded in DER format.
   std::string GenerateKeyPair(const std::string& token_id) {
@@ -239,6 +351,12 @@ class PlatformKeysServiceBrowserTest
   }
 
  private:
+  void SetUserSlot(const base::Closure& done_callback,
+                   net::NSSCertDatabase* db) {
+    user_slot_ = db->GetPrivateSlot();
+    done_callback.Run();
+  }
+
   const AccountId test_user_account_id_ = AccountId::FromUserEmailGaiaId(
       kTestUserEmail,
       signin::GetTestGaiaIdForEmail(kTestUserEmail));
@@ -257,6 +375,12 @@ class PlatformKeysServiceBrowserTest
   // Unowned pointer to the PlatformKeysService for |profile_|. Valid after
   // SetUpOnMainThread().
   PlatformKeysService* platform_keys_service_ = nullptr;
+  // The private slot for the profile under test. This should be null if the
+  // test parameter mandates testing with the sign-in profile.
+  crypto::ScopedPK11Slot user_slot_;
+  // Owned pointer to a ScopedSoftokenAttrsMapping object that is created after
+  // |platform_keys_service_| is valid.
+  std::unique_ptr<ScopedSoftokenAttrsMapping> scoped_softoken_attrs_mapping_;
 };
 
 // Tests that GetTokens() is callable and returns the expected tokens.
@@ -274,14 +398,14 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, GetTokens) {
 // Generates a Rsa key pair and tests signing using that key pair.
 IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, GenerateRsaAndSign) {
   const std::string data_to_sign = "test";
-  const unsigned int key_size = 2048;
+  const unsigned int kKeySize = 2048;
   const HashAlgorithm hash_algorithm = HASH_ALGORITHM_SHA256;
   const crypto::SignatureVerifier::SignatureAlgorithm signature_algorithm =
       crypto::SignatureVerifier::RSA_PKCS1_SHA256;
 
   for (const std::string& token_id : GetParam().token_ids) {
     GenerateKeyExecutionWaiter generate_key_waiter;
-    platform_keys_service()->GenerateRSAKey(token_id, key_size,
+    platform_keys_service()->GenerateRSAKey(token_id, kKeySize,
                                             generate_key_waiter.GetCallback());
     generate_key_waiter.Wait();
     EXPECT_TRUE(generate_key_waiter.error_message().empty());
@@ -308,6 +432,185 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, GenerateRsaAndSign) {
   }
 }
 
+IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, SetAndGetKeyAttribute) {
+  // The attribute type to be set and retrieved using platform keys service.
+  const KeyAttributeType kAttributeType =
+      KeyAttributeType::CertificateProvisioningId;
+
+  for (const std::string& token_id : GetParam().token_ids) {
+    const std::string kAttributeValue = "test" + token_id;
+
+    // Generate key pair.
+    const std::string public_key_spki_der = GenerateKeyPair(token_id);
+    ASSERT_FALSE(public_key_spki_der.empty());
+
+    // Set key attribute.
+    SetAttributeForKeyExecutionWaiter set_attribute_for_key_execution_waiter;
+    platform_keys_service()->SetAttributeForKey(
+        token_id, public_key_spki_der, kAttributeType, kAttributeValue,
+        set_attribute_for_key_execution_waiter.GetCallback());
+    set_attribute_for_key_execution_waiter.Wait();
+
+    // Get key attribute.
+    GetAttributeForKeyExecutionWaiter get_attribute_for_key_execution_waiter;
+    platform_keys_service()->GetAttributeForKey(
+        token_id, public_key_spki_der, kAttributeType,
+        get_attribute_for_key_execution_waiter.GetCallback());
+    get_attribute_for_key_execution_waiter.Wait();
+
+    EXPECT_TRUE(get_attribute_for_key_execution_waiter.error_message().empty());
+    EXPECT_EQ(get_attribute_for_key_execution_waiter.attribute_value(),
+              kAttributeValue);
+  }
+}
+
+IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, GetUnsetKeyAttribute) {
+  const KeyAttributeType kAttributeType =
+      KeyAttributeType::CertificateProvisioningId;
+
+  for (const std::string& token_id : GetParam().token_ids) {
+    // Generate key pair.
+    const std::string public_key_spki_der = GenerateKeyPair(token_id);
+    ASSERT_FALSE(public_key_spki_der.empty());
+
+    // Get key attribute.
+    GetAttributeForKeyExecutionWaiter get_attribute_for_key_execution_waiter;
+    platform_keys_service()->GetAttributeForKey(
+        token_id, public_key_spki_der, kAttributeType,
+        get_attribute_for_key_execution_waiter.GetCallback());
+    get_attribute_for_key_execution_waiter.Wait();
+
+    EXPECT_TRUE(get_attribute_for_key_execution_waiter.error_message().empty());
+    EXPECT_TRUE(
+        get_attribute_for_key_execution_waiter.attribute_value().empty());
+  }
+}
+
+IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
+                       GetKeyAttributeForNonExistingKey) {
+  const KeyAttributeType kAttributeType =
+      KeyAttributeType::CertificateProvisioningId;
+  const std::string kPublicKey = "Non Existing public key";
+
+  for (const std::string& token_id : GetParam().token_ids) {
+    // Get key attribute.
+    GetAttributeForKeyExecutionWaiter get_attribute_for_key_execution_waiter;
+    platform_keys_service()->GetAttributeForKey(
+        token_id, kPublicKey, kAttributeType,
+        get_attribute_for_key_execution_waiter.GetCallback());
+    get_attribute_for_key_execution_waiter.Wait();
+
+    EXPECT_FALSE(
+        get_attribute_for_key_execution_waiter.error_message().empty());
+  }
+}
+
+IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
+                       SetKeyAttributeForNonExistingKey) {
+  const KeyAttributeType kAttributeType =
+      KeyAttributeType::CertificateProvisioningId;
+  const std::string kAttributeValue = "test";
+  const std::string kPublicKey = "Non Existing public key";
+
+  for (const std::string& token_id : GetParam().token_ids) {
+    // Set key attribute.
+    SetAttributeForKeyExecutionWaiter set_attribute_for_key_execution_waiter;
+    platform_keys_service()->SetAttributeForKey(
+        token_id, kPublicKey, kAttributeType, kAttributeValue,
+        set_attribute_for_key_execution_waiter.GetCallback());
+    set_attribute_for_key_execution_waiter.Wait();
+
+    EXPECT_FALSE(
+        set_attribute_for_key_execution_waiter.error_message().empty());
+  }
+}
+
+IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
+                       RemoveKeyWithNoMatchingCertificates) {
+  for (const std::string& token_id : GetParam().token_ids) {
+    // Generate first key pair.
+    const std::string public_key_1 = GenerateKeyPair(token_id);
+    ASSERT_FALSE(public_key_1.empty());
+
+    // Generate second key pair.
+    const std::string public_key_2 = GenerateKeyPair(token_id);
+    ASSERT_FALSE(public_key_2.empty());
+
+    auto public_key_bytes_1 = base::as_bytes(base::make_span(public_key_1));
+    auto public_key_bytes_2 = base::as_bytes(base::make_span(public_key_2));
+    EXPECT_TRUE(crypto::FindNSSKeyFromPublicKeyInfo(public_key_bytes_1));
+    EXPECT_TRUE(crypto::FindNSSKeyFromPublicKeyInfo(public_key_bytes_2));
+
+    RemoveKeyExecutionWaiter remove_key_waiter;
+    platform_keys_service()->RemoveKey(token_id, public_key_1,
+                                       remove_key_waiter.GetCallback());
+    remove_key_waiter.Wait();
+
+    EXPECT_TRUE(remove_key_waiter.error_message().empty());
+    EXPECT_FALSE(crypto::FindNSSKeyFromPublicKeyInfo(public_key_bytes_1));
+    EXPECT_TRUE(crypto::FindNSSKeyFromPublicKeyInfo(public_key_bytes_2));
+  }
+}
+
+IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
+                       RemoveKeyWithMatchingCertificate) {
+  for (const std::string& token_id : GetParam().token_ids) {
+    PK11SlotInfo* const slot = GetSlot(token_id);
+
+    // Assert that there are no certificates before importing.
+    GetCertificatesExecutionWaiter get_certificates_waiter;
+    platform_keys_service()->GetCertificates(
+        token_id, get_certificates_waiter.GetCallback());
+    get_certificates_waiter.Wait();
+    ASSERT_EQ(get_certificates_waiter.matches().size(), 0U);
+
+    // Import testing key pair and certificate.
+    net::ScopedCERTCertificate cert;
+    {
+      base::ScopedAllowBlockingForTesting allow_io;
+      net::ImportClientCertAndKeyFromFile(net::GetTestCertsDirectory(),
+                                          "client_1.pem", "client_1.pk8", slot,
+                                          &cert);
+    }
+
+    // Assert that the certificate is imported correctly.
+    ASSERT_TRUE(cert.get());
+    GetCertificatesExecutionWaiter get_certificates_waiter_2;
+    platform_keys_service()->GetCertificates(
+        token_id, get_certificates_waiter_2.GetCallback());
+    get_certificates_waiter_2.Wait();
+    ASSERT_EQ(get_certificates_waiter_2.matches().size(), 1U);
+
+    ASSERT_GT(cert->derPublicKey.len, 0U);
+    std::string public_key(
+        reinterpret_cast<const char*>(cert->derPublicKey.data),
+        cert->derPublicKey.len);
+    auto public_key_bytes = base::as_bytes(base::make_span(public_key));
+    EXPECT_TRUE(crypto::FindNSSKeyFromPublicKeyInfo(public_key_bytes));
+
+    // Try Removing the key pair.
+    RemoveKeyExecutionWaiter remove_key_waiter;
+    platform_keys_service()->RemoveKey(token_id, public_key,
+                                       remove_key_waiter.GetCallback());
+    remove_key_waiter.Wait();
+    EXPECT_FALSE(remove_key_waiter.error_message().empty());
+
+    // Assert that the certificate is not removed.
+    GetCertificatesExecutionWaiter get_certificates_waiter_3;
+    platform_keys_service()->GetCertificates(
+        token_id, get_certificates_waiter_3.GetCallback());
+    get_certificates_waiter_3.Wait();
+
+    net::CertificateList found_certs = get_certificates_waiter_3.matches();
+    ASSERT_EQ(found_certs.size(), 1U);
+    EXPECT_TRUE(
+        net::x509_util::IsSameCertificate(found_certs[0].get(), cert.get()));
+
+    // Assert that the key pair is not deleted.
+    EXPECT_TRUE(crypto::FindNSSKeyFromPublicKeyInfo(public_key_bytes));
+  }
+}
+
 // Generates a key pair in tokens accessible from the profile under test and
 // retrieves them.
 IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, GetAllKeys) {
@@ -328,7 +631,7 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, GetAllKeys) {
 
     EXPECT_TRUE(get_all_keys_waiter.error_message().empty());
     std::vector<std::string> public_keys = get_all_keys_waiter.public_keys();
-    EXPECT_EQ(public_keys.size(), 1U);
+    ASSERT_EQ(public_keys.size(), 1U);
     EXPECT_EQ(public_keys[0], token_key_map[token_id]);
   }
 }
@@ -343,17 +646,18 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
 
     EXPECT_TRUE(get_all_keys_waiter.error_message().empty());
     std::vector<std::string> public_keys = get_all_keys_waiter.public_keys();
-    EXPECT_EQ(public_keys.size(), 0U);
+    EXPECT_TRUE(public_keys.empty());
   }
 }
 
 INSTANTIATE_TEST_SUITE_P(
     AllSupportedProfileTypes,
     PlatformKeysServiceBrowserTest,
-    ::testing::Values(
-        TestConfig{ProfileToUse::kSigninProfile, {"system"}},
-        TestConfig{ProfileToUse::kUnaffiliatedUserProfile, {"user"}},
-        TestConfig{ProfileToUse::kAffiliatedUserProfile, {"user", "system"}}));
+    ::testing::Values(TestConfig{ProfileToUse::kSigninProfile, {kSystemToken}},
+                      TestConfig{ProfileToUse::kUnaffiliatedUserProfile,
+                                 {kUserToken}},
+                      TestConfig{ProfileToUse::kAffiliatedUserProfile,
+                                 {kSystemToken, kUserToken}}));
 
 }  // namespace platform_keys
 }  // namespace chromeos

chrome/browser/chromeos/platform_keys/platform_keys_service_nss.cc

@@ -5,9 +5,11 @@
 #include "chrome/browser/chromeos/platform_keys/platform_keys_service.h"
 
 #include <cert.h>
+#include <certdb.h>
 #include <cryptohi.h>
 #include <keyhi.h>
 #include <pk11pub.h>
+#include <pkcs11t.h>
 #include <secder.h>
 #include <stddef.h>
 #include <stdint.h>
@@ -55,6 +57,7 @@
 #include "third_party/boringssl/src/include/openssl/bytestring.h"
 #include "third_party/boringssl/src/include/openssl/evp.h"
 #include "third_party/boringssl/src/include/openssl/rsa.h"
+#include "third_party/cros_system_api/constants/pkcs11_custom_attributes.h"
 
 using content::BrowserContext;
 using content::BrowserThread;
@@ -281,16 +284,6 @@ class SignState : public NSSOperationState {
                              std::move(bound_callback), service_weak_ptr_));
   }
 
-  crypto::ScopedSECKEYPrivateKey GetPrivateKey() {
-    auto public_key_bytes =
-        base::as_bytes(base::make_span(public_key_spki_der_));
-    if (slot_) {
-      return crypto::FindNSSKeyFromPublicKeyInfoInSlot(public_key_bytes,
-                                                       slot_.get());
-    }
-    return crypto::FindNSSKeyFromPublicKeyInfo(public_key_bytes);
-  }
-
   // The data that will be signed.
   const std::string data_;
 
@@ -459,6 +452,33 @@ class RemoveCertificateState : public NSSOperationState {
   RemoveCertificateCallback callback_;
 };
 
+class RemoveKeyState : public NSSOperationState {
+ public:
+  RemoveKeyState(ServiceWeakPtr weak_ptr,
+                 const std::string& public_key_spki_der,
+                 RemoveKeyCallback callback);
+  ~RemoveKeyState() override = default;
+
+  void OnError(const base::Location& from,
+               const std::string& error_message) override {
+    CallBack(from, error_message);
+  }
+
+  void CallBack(const base::Location& from, const std::string& error_message) {
+    auto bound_callback = base::BindOnce(std::move(callback_), error_message);
+    origin_task_runner_->PostTask(
+        from, base::BindOnce(&NSSOperationState::RunCallback,
+                             std::move(bound_callback), service_weak_ptr_));
+  }
+
+  // Must be a DER encoding of a SubjectPublicKeyInfo.
+  const std::string public_key_spki_der_;
+
+ private:
+  // Must be called on origin thread, therefore use CallBack().
+  RemoveKeyCallback callback_;
+};
+
 class GetTokensState : public NSSOperationState {
  public:
   explicit GetTokensState(ServiceWeakPtr weak_ptr,
@@ -516,6 +536,69 @@ class GetKeyLocationsState : public NSSOperationState {
   GetKeyLocationsCallback callback_;
 };
 
+class SetAttributeForKeyState : public NSSOperationState {
+ public:
+  SetAttributeForKeyState(ServiceWeakPtr weak_ptr,
+                          const std::string& public_key_spki_der,
+                          CK_ATTRIBUTE_TYPE attribute_type,
+                          const std::string& attribute_value,
+                          SetAttributeForKeyCallback callback);
+  ~SetAttributeForKeyState() override = default;
+
+  void OnError(const base::Location& from,
+               const std::string& error_message) override {
+    CallBack(from, error_message);
+  }
+
+  void CallBack(const base::Location& from, const std::string& error_message) {
+    auto bound_callback = base::BindOnce(std::move(callback_), error_message);
+    origin_task_runner_->PostTask(
+        from, base::BindOnce(&NSSOperationState::RunCallback,
+                             std::move(bound_callback), service_weak_ptr_));
+  }
+
+  // Must be a DER encoding of a SubjectPublicKeyInfo.
+  const std::string public_key_spki_der_;
+  const CK_ATTRIBUTE_TYPE attribute_type_;
+  const std::string attribute_value_;
+
+ private:
+  // Must be called on origin thread, therefore use CallBack().
+  SetAttributeForKeyCallback callback_;
+};
+
+class GetAttributeForKeyState : public NSSOperationState {
+ public:
+  GetAttributeForKeyState(ServiceWeakPtr weak_ptr,
+                          const std::string& public_key_spki_der,
+                          CK_ATTRIBUTE_TYPE attribute_type,
+                          GetAttributeForKeyCallback callback);
+  ~GetAttributeForKeyState() override = default;
+
+  void OnError(const base::Location& from,
+               const std::string& error_message) override {
+    CallBack(from, std::string() /* no attribute value */, error_message);
+  }
+
+  void CallBack(const base::Location& from,
+                const std::string& attribute_value,
+                const std::string& error_message) {
+    auto bound_callback =
+        base::BindOnce(std::move(callback_), attribute_value, error_message);
+    origin_task_runner_->PostTask(
+        from, base::BindOnce(&NSSOperationState::RunCallback,
+                             std::move(bound_callback), service_weak_ptr_));
+  }
+
+  // Must be a DER encoding of a SubjectPublicKeyInfo.
+  const std::string public_key_spki_der_;
+  const CK_ATTRIBUTE_TYPE attribute_type_;
+
+ private:
+  // Must be called on origin thread, therefore use CallBack().
+  GetAttributeForKeyCallback callback_;
+};
+
 NSSOperationState::NSSOperationState(ServiceWeakPtr weak_ptr)
     : service_weak_ptr_(weak_ptr),
       origin_task_runner_(base::ThreadTaskRunnerHandle::Get()) {}
@@ -582,6 +665,13 @@ RemoveCertificateState::RemoveCertificateState(
       certificate_(certificate),
       callback_(callback) {}
 
+RemoveKeyState::RemoveKeyState(ServiceWeakPtr weak_ptr,
+                               const std::string& public_key_spki_der,
+                               RemoveKeyCallback callback)
+    : NSSOperationState(weak_ptr),
+      public_key_spki_der_(public_key_spki_der),
+      callback_(std::move(callback)) {}
+
 GetTokensState::GetTokensState(ServiceWeakPtr weak_ptr,
                                const GetTokensCallback& callback)
     : NSSOperationState(weak_ptr), callback_(callback) {}
@@ -594,6 +684,42 @@ GetKeyLocationsState::GetKeyLocationsState(
       public_key_spki_der_(public_key_spki_der),
       callback_(callback) {}
 
+SetAttributeForKeyState::SetAttributeForKeyState(
+    ServiceWeakPtr weak_ptr,
+    const std::string& public_key_spki_der,
+    CK_ATTRIBUTE_TYPE attribute_type,
+    const std::string& attribute_value,
+    SetAttributeForKeyCallback callback)
+    : NSSOperationState(weak_ptr),
+      public_key_spki_der_(public_key_spki_der),
+      attribute_type_(attribute_type),
+      attribute_value_(attribute_value),
+      callback_(std::move(callback)) {}
+
+GetAttributeForKeyState::GetAttributeForKeyState(
+    ServiceWeakPtr weak_ptr,
+    const std::string& public_key_spki_der,
+    CK_ATTRIBUTE_TYPE attribute_type,
+    GetAttributeForKeyCallback callback)
+    : NSSOperationState(weak_ptr),
+      public_key_spki_der_(public_key_spki_der),
+      attribute_type_(attribute_type),
+      callback_(std::move(callback)) {}
+
+// Returns the private key corresponding to the der-encoded
+// |public_key_spki_der| if found in |slot|. If |slot| is nullptr, the
+// private key will be searched in all slots.
+crypto::ScopedSECKEYPrivateKey GetPrivateKey(
+    const std::string& public_key_spki_der,
+    const crypto::ScopedPK11Slot& slot) {
+  auto public_key_bytes = base::as_bytes(base::make_span(public_key_spki_der));
+  if (slot) {
+    return crypto::FindNSSKeyFromPublicKeyInfoInSlot(public_key_bytes,
+                                                     slot.get());
+  }
+  return crypto::FindNSSKeyFromPublicKeyInfo(public_key_bytes);
+}
+
 // Does the actual RSA key generation on a worker thread. Used by
 // GenerateRSAKeyWithDB().
 void GenerateRSAKeyOnWorkerThread(std::unique_ptr<GenerateRSAKeyState> state) {
@@ -699,7 +825,8 @@ void GenerateECKeyWithDB(std::unique_ptr<GenerateECKeyState> state,
 
 // Does the actual RSA signing on a worker thread.
 void SignRSAOnWorkerThread(std::unique_ptr<SignState> state) {
-  crypto::ScopedSECKEYPrivateKey rsa_key = state->GetPrivateKey();
+  crypto::ScopedSECKEYPrivateKey rsa_key =
+      GetPrivateKey(state->public_key_spki_der_, state->slot_);
 
   // Fail if the key was not found or is of the wrong type.
   if (!rsa_key || SECKEY_GetPrivateKeyType(rsa_key.get()) != rsaKey) {
@@ -769,7 +896,8 @@ void SignRSAOnWorkerThread(std::unique_ptr<SignState> state) {
 
 // Does the actual EC Signing on a worker thread.
 void SignECOnWorkerThread(std::unique_ptr<SignState> state) {
-  crypto::ScopedSECKEYPrivateKey ec_key = state->GetPrivateKey();
+  crypto::ScopedSECKEYPrivateKey ec_key =
+      GetPrivateKey(state->public_key_spki_der_, state->slot_);
 
   // Fail if the key was not found or is of the wrong type.
   if (!ec_key || SECKEY_GetPrivateKeyType(ec_key.get()) != ecKey) {
@@ -815,7 +943,8 @@ void SignECOnWorkerThread(std::unique_ptr<SignState> state) {
 
 // Decides which signing algorithm will be used. Used by SignWithDB().
 void SignOnWorkerThread(std::unique_ptr<SignState> state) {
-  crypto::ScopedSECKEYPrivateKey key = state->GetPrivateKey();
+  crypto::ScopedSECKEYPrivateKey key =
+      GetPrivateKey(state->public_key_spki_der_, state->slot_);
 
   if (!key) {
     state->OnError(FROM_HERE, kErrorKeyNotFound);
@@ -953,9 +1082,12 @@ void GetAllKeysOnWorkerThread(std::unique_ptr<GetAllKeysState> state) {
       LOG(WARNING) << "Could not encode subject public key info.";
       continue;
     }
-    public_key_spki_der_list.push_back(std::string(
-        subject_public_key_info->data,
-        subject_public_key_info->data + subject_public_key_info->len));
+
+    if (subject_public_key_info->len > 0) {
+      public_key_spki_der_list.push_back(std::string(
+          subject_public_key_info->data,
+          subject_public_key_info->data + subject_public_key_info->len));
+    }
   }
 
   SECKEY_DestroyPublicKeyList(public_keys);
@@ -1057,6 +1189,58 @@ void RemoveCertificateWithDB(std::unique_ptr<RemoveCertificateState> state,
                      certificate_found));
 }
 
+// Does the actual key pair removal on a worker thread. Used by
+// RemoveKeyWithDb().
+void RemoveKeyOnWorkerThread(std::unique_ptr<RemoveKeyState> state) {
+  DCHECK(state->slot_.get());
+
+  crypto::ScopedSECKEYPrivateKey private_key =
+      GetPrivateKey(state->public_key_spki_der_, state->slot_);
+
+  if (!private_key) {
+    state->OnError(FROM_HERE, kErrorKeyNotFound);
+    return;
+  }
+
+  crypto::ScopedSECKEYPublicKey public_key(
+      SECKEY_ConvertToPublicKey(private_key.get()));
+
+  // PK11_DeleteTokenPrivateKey function frees the privKey structure
+  // unconditionally, and thus releasing the ownership of the passed private
+  // key.
+  // |force| is set to false so as not to delete the key if there are any
+  // matching certificates.
+  if (PK11_DeleteTokenPrivateKey(/*privKey=*/private_key.release(),
+                                 /*force=*/false) != SECSuccess) {
+    LOG(ERROR) << "Cannot delete private key";
+    state->OnError(FROM_HERE, kErrorInternal);
+    return;
+  }
+
+  // PK11_DeleteTokenPublicKey function frees the pubKey structure
+  // unconditionally, and thus releasing the ownership of the passed private
+  // key.
+  if (PK11_DeleteTokenPublicKey(/*pubKey=*/public_key.release()) !=
+      SECSuccess) {
+    LOG(WARNING) << "Cannot delete public key";
+  }
+
+  state->CallBack(FROM_HERE, std::string() /* no error */);
+}
+
+// Continues removing the key pair with the obtained |cert_db|. Called by
+// RemoveKey().
+void RemoveKeyWithDb(std::unique_ptr<RemoveKeyState> state,
+                     net::NSSCertDatabase* cert_db) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  base::ThreadPool::PostTask(
+      FROM_HERE,
+      {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
+       base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
+      base::BindOnce(&RemoveKeyOnWorkerThread, std::move(state)));
+}
+
 // Does the actual work to determine which tokens are available.
 void GetTokensWithDB(std::unique_ptr<GetTokensState> state,
                      net::NSSCertDatabase* cert_db) {
@@ -1117,6 +1301,98 @@ void GetKeyLocationsWithDB(std::unique_ptr<GetKeyLocationsState> state,
                   std::string() /* no error */);
 }
 
+// Translates |type| to one of the NSS softoken module's predefined key
+// attributes which are used in tests.
+CK_ATTRIBUTE_TYPE TranslateKeyAttributeTypeForSoftoken(KeyAttributeType type) {
+  switch (type) {
+    case KeyAttributeType::CertificateProvisioningId:
+      return CKA_START_DATE;
+  }
+}
+
+// If |map_to_softoken_attrs| is true, translates |type| to one of the softoken
+// module predefined key attributes. Otherwise, applies normal translation.
+CK_ATTRIBUTE_TYPE TranslateKeyAttributeType(KeyAttributeType type,
+                                            bool map_to_softoken_attrs) {
+  if (map_to_softoken_attrs) {
+    return TranslateKeyAttributeTypeForSoftoken(type);
+  }
+
+  switch (type) {
+    case KeyAttributeType::CertificateProvisioningId:
+      return pkcs11_custom_attributes::kCkaChromeOsBuiltinProvisioningProfileId;
+  }
+}
+
+// Does the actual attribute value setting with the obtained |cert_db|.
+// Called by SetAttributeForKey().
+void SetAttributeForKeyWithDb(std::unique_ptr<SetAttributeForKeyState> state,
+                              net::NSSCertDatabase* cert_db) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(state->slot_.get());
+
+  crypto::ScopedSECKEYPrivateKey private_key =
+      GetPrivateKey(state->public_key_spki_der_, state->slot_);
+
+  if (!private_key) {
+    state->OnError(FROM_HERE, kErrorKeyNotFound);
+    return;
+  }
+
+  // This SECItem will point to data owned by |state| so it is not necessary to
+  // use ScopedSECItem.
+  SECItem attribute_value;
+
+  attribute_value.data = reinterpret_cast<unsigned char*>(
+      const_cast<char*>(state->attribute_value_.data()));
+  attribute_value.len = state->attribute_value_.size();
+
+  if (PK11_WriteRawAttribute(
+          /*objType=*/PK11_TypePrivKey, private_key.get(),
+          state->attribute_type_, &attribute_value) != SECSuccess) {
+    state->OnError(FROM_HERE, kErrorInternal);
+    return;
+  }
+
+  state->CallBack(FROM_HERE, std::string() /* no error */);
+}
+
+// Does the actual attribute value retrieval with the obtained |cert_db|.
+// Called by GetAttributeForKey().
+void GetAttributeForKeyWithDb(std::unique_ptr<GetAttributeForKeyState> state,
+                              net::NSSCertDatabase* cert_db) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(state->slot_.get());
+
+  crypto::ScopedSECKEYPrivateKey private_key =
+      GetPrivateKey(state->public_key_spki_der_, state->slot_);
+
+  if (!private_key) {
+    state->OnError(FROM_HERE, kErrorKeyNotFound);
+    return;
+  }
+
+  crypto::ScopedSECItem attribute_value(SECITEM_AllocItem(/*arena=*/nullptr,
+                                                          /*item=*/nullptr,
+                                                          /*len=*/0));
+  DCHECK(attribute_value.get());
+
+  if (PK11_ReadRawAttribute(
+          /*objType=*/PK11_TypePrivKey, private_key.get(),
+          state->attribute_type_, attribute_value.get()) != SECSuccess) {
+    state->OnError(FROM_HERE, kErrorInternal);
+    return;
+  }
+
+  std::string attribute_value_str;
+  if (attribute_value->len > 0) {
+    attribute_value_str.assign(attribute_value->data,
+                               attribute_value->data + attribute_value->len);
+  }
+
+  state->CallBack(FROM_HERE, attribute_value_str, std::string() /* no error */);
+}
+
 }  // namespace
 
 void PlatformKeysServiceImpl::GenerateRSAKey(
@@ -1371,6 +1647,23 @@ void PlatformKeysServiceImpl::RemoveCertificate(
                   browser_context_, state_ptr);
 }
 
+void PlatformKeysServiceImpl::RemoveKey(const std::string& token_id,
+                                        const std::string& public_key_spki_der,
+                                        RemoveKeyCallback callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  auto state = std::make_unique<RemoveKeyState>(
+      weak_factory_.GetWeakPtr(), public_key_spki_der, std::move(callback));
+
+  // Get the pointer to |state| before base::Passed releases |state|.
+  NSSOperationState* state_ptr = state.get();
+
+  // The NSSCertDatabase object is not required. But in case it's not available
+  // we would get more informative error messages.
+  GetCertDatabase(token_id, base::Bind(&RemoveKeyWithDb, base::Passed(&state)),
+                  browser_context_, state_ptr);
+}
+
 void PlatformKeysServiceImpl::GetTokens(const GetTokensCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   auto state =
@@ -1396,5 +1689,67 @@ void PlatformKeysServiceImpl::GetKeyLocations(
       browser_context_, state_ptr);
 }
 
+void PlatformKeysServiceImpl::SetAttributeForKey(
+    const std::string& token_id,
+    const std::string& public_key_spki_der,
+    KeyAttributeType attribute_type,
+    const std::string& attribute_value,
+    SetAttributeForKeyCallback callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  CK_ATTRIBUTE_TYPE ck_attribute_type = TranslateKeyAttributeType(
+      attribute_type,
+      /*map_to_softoken_attrs=*/IsSetMapToSoftokenAttrsForTesting());
+
+  auto state = std::make_unique<SetAttributeForKeyState>(
+      weak_factory_.GetWeakPtr(), public_key_spki_der, ck_attribute_type,
+      attribute_value, std::move(callback));
+
+  // Get the pointer to |state| before base::Passed releases |state|.
+  NSSOperationState* state_ptr = state.get();
+
+  // The NSSCertDatabase object is not required. Only setting the state slot is
+  // required.
+  GetCertDatabase(
+      token_id,
+      base::BindRepeating(&SetAttributeForKeyWithDb, base::Passed(&state)),
+      browser_context_, state_ptr);
+}
+
+void PlatformKeysServiceImpl::GetAttributeForKey(
+    const std::string& token_id,
+    const std::string& public_key_spki_der,
+    KeyAttributeType attribute_type,
+    GetAttributeForKeyCallback callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  CK_ATTRIBUTE_TYPE ck_attribute_type = TranslateKeyAttributeType(
+      attribute_type,
+      /*map_to_softoken_attrs=*/IsSetMapToSoftokenAttrsForTesting());
+
+  auto state = std::make_unique<GetAttributeForKeyState>(
+      weak_factory_.GetWeakPtr(), public_key_spki_der, ck_attribute_type,
+      std::move(callback));
+
+  // Get the pointer to |state| before base::Passed releases |state|.
+  NSSOperationState* state_ptr = state.get();
+
+  // The NSSCertDatabase object is not required. Only setting the state slot is
+  // required.
+  GetCertDatabase(
+      token_id,
+      base::BindRepeating(&GetAttributeForKeyWithDb, base::Passed(&state)),
+      browser_context_, state_ptr);
+}
+
+void PlatformKeysServiceImpl::SetMapToSoftokenAttrsForTesting(
+    bool map_to_softoken_attrs_for_testing) {
+  map_to_softoken_attrs_for_testing_ = map_to_softoken_attrs_for_testing;
+}
+
+bool PlatformKeysServiceImpl::IsSetMapToSoftokenAttrsForTesting() {
+  return map_to_softoken_attrs_for_testing_;
+}
+
 }  // namespace platform_keys
 }  // namespace chromeos