use AttestationClient to delete attested keys for cert provisioning.

We are deprecating attestation methods by CryptohomeClient.

BUG=b:158955123
TEST=unit_tests.

Change-Id: I901403863794308619bec161354884808988b2f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2497386Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Michael Ershov <miersh@google.com>
Commit-Queue: Leo Lai <cylai@google.com>
Cr-Commit-Position: refs/heads/master@{#821186}

chrome/browser/chromeos/cert_provisioning/cert_provisioning_common.cc

@@ -15,7 +15,8 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
-#include "chromeos/dbus/cryptohome/cryptohome_client.h"
+#include "chromeos/dbus/attestation/attestation_client.h"
+#include "chromeos/dbus/attestation/interface.pb.h"
 #include "components/account_id/account_id.h"
 #include "components/prefs/pref_registry_simple.h"
 
@@ -41,6 +42,35 @@ base::Optional<AccountId> GetAccountId(CertScope scope, Profile* profile) {
 
   NOTREACHED();
 }
+
+// This function implements `DeleteVaKey()` and `DeleteVaKeysByPrefix()`, both
+// of which call this function with a proper match behavior.
+void DeleteVaKeysWithMatchBehavior(
+    CertScope scope,
+    Profile* profile,
+    ::attestation::DeleteKeysRequest::MatchBehavior match_behavior,
+    const std::string& label_match,
+    DeleteVaKeyCallback callback) {
+  auto account_id = GetAccountId(scope, profile);
+  if (!account_id.has_value()) {
+    std::move(callback).Run(false);
+    return;
+  }
+  ::attestation::DeleteKeysRequest request;
+  request.set_username(
+      cryptohome::CreateAccountIdentifierFromAccountId(account_id.value())
+          .account_id());
+  request.set_key_label_match(label_match);
+  request.set_match_behavior(match_behavior);
+
+  auto wrapped_callback = [](DeleteVaKeyCallback cb,
+                             const ::attestation::DeleteKeysReply& reply) {
+    std::move(cb).Run(reply.status() == ::attestation::STATUS_SUCCESS);
+  };
+  AttestationClient::Get()->DeleteKeys(
+      request, base::BindOnce(wrapped_callback, std::move(callback)));
+}
+
 }  // namespace
 
 bool IsFinalState(CertProvisioningWorkerState state) {
@@ -170,14 +200,8 @@ void DeleteVaKey(CertScope scope,
                  Profile* profile,
                  const std::string& key_name,
                  DeleteVaKeyCallback callback) {
-  auto account_id = GetAccountId(scope, profile);
-  if (!account_id.has_value()) {
-    return;
-  }
-
-  CryptohomeClient::Get()->TpmAttestationDeleteKey(
-      GetVaKeyType(scope),
-      cryptohome::CreateAccountIdentifierFromAccountId(account_id.value()),
+  DeleteVaKeysWithMatchBehavior(
+      scope, profile, ::attestation::DeleteKeysRequest::MATCH_BEHAVIOR_EXACT,
       key_name, std::move(callback));
 }
 
@@ -185,14 +209,8 @@ void DeleteVaKeysByPrefix(CertScope scope,
                           Profile* profile,
                           const std::string& key_prefix,
                           DeleteVaKeyCallback callback) {
-  auto account_id = GetAccountId(scope, profile);
-  if (!account_id.has_value()) {
-    return;
-  }
-
-  CryptohomeClient::Get()->TpmAttestationDeleteKeysByPrefix(
-      GetVaKeyType(scope),
-      cryptohome::CreateAccountIdentifierFromAccountId(account_id.value()),
+  DeleteVaKeysWithMatchBehavior(
+      scope, profile, ::attestation::DeleteKeysRequest::MATCH_BEHAVIOR_PREFIX,
       key_prefix, std::move(callback));
 }
 

chrome/browser/chromeos/cert_provisioning/cert_provisioning_common.h

@@ -14,6 +14,7 @@
 #include "base/values.h"
 #include "chrome/browser/chromeos/platform_keys/platform_keys.h"
 #include "chrome/browser/chromeos/platform_keys/platform_keys_service.h"
+#include "chromeos/dbus/attestation/interface.pb.h"
 #include "chromeos/dbus/constants/attestation_constants.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "net/cert/x509_certificate.h"
@@ -30,7 +31,7 @@ class PlatformKeysService;
 namespace cert_provisioning {
 
 // Used for both DeleteVaKey and DeleteVaKeysByPrefix
-using DeleteVaKeyCallback = base::OnceCallback<void(base::Optional<bool>)>;
+using DeleteVaKeyCallback = base::OnceCallback<void(bool)>;
 
 const char kKeyNamePrefix[] = "cert-provis-";
 

chrome/browser/chromeos/cert_provisioning/cert_provisioning_scheduler.cc

@@ -291,10 +291,10 @@ void CertProvisioningSchedulerImpl::CleanVaKeysIfIdle() {
 }
 
 void CertProvisioningSchedulerImpl::OnCleanVaKeysIfIdleDone(
-    base::Optional<bool> delete_result) {
+    bool delete_result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  if (!delete_result.has_value() || !delete_result.value()) {
+  if (!delete_result) {
     LOG(ERROR) << "Failed to delete keys while idle";
   }
 

chrome/browser/chromeos/cert_provisioning/cert_provisioning_scheduler.h

@@ -156,7 +156,7 @@ class CertProvisioningSchedulerImpl
   void OnDeleteCertsWithoutPolicyDone(platform_keys::Status status);
   void CancelWorkersWithoutPolicy(const std::vector<CertProfile>& profiles);
   void CleanVaKeysIfIdle();
-  void OnCleanVaKeysIfIdleDone(base::Optional<bool> delete_result);
+  void OnCleanVaKeysIfIdleDone(bool delete_result);
   void RegisterForPrefsChanges();
 
   void InitiateRenewal(const CertProfileId& cert_profile_id);

chrome/browser/chromeos/cert_provisioning/cert_provisioning_scheduler_unittest.cc

@@ -16,6 +16,8 @@
 #include "chrome/browser/chromeos/platform_keys/platform_keys.h"
 #include "chrome/browser/chromeos/platform_keys/platform_keys_service.h"
 #include "chrome/common/pref_names.h"
+#include "chromeos/dbus/attestation/fake_attestation_client.h"
+#include "chromeos/dbus/attestation/interface.pb.h"
 #include "chromeos/network/network_state_test_helper.h"
 #include "components/policy/core/common/cloud/mock_cloud_policy_client.h"
 #include "components/prefs/testing_pref_service.h"
@@ -45,6 +47,28 @@ constexpr char kCertProfileId[] = "cert_profile_id_1";
 constexpr char kCertProfileVersion[] = "cert_profile_version_1";
 constexpr TimeDelta kCertProfileRenewalPeriod = TimeDelta::FromSeconds(0);
 
+void VerifyDeleteKeysByPrefixCalledOnce(CertScope cert_scope) {
+  const std::vector<::attestation::DeleteKeysRequest> delete_keys_history =
+      chromeos::AttestationClient::Get()
+          ->GetTestInterface()
+          ->delete_keys_history();
+  // Use `ASSERT_EQ()` so the checks that follows don't crash.
+  ASSERT_EQ(delete_keys_history.size(), 1);
+  EXPECT_EQ(delete_keys_history[0].username().empty(),
+            cert_scope != CertScope::kUser);
+  EXPECT_EQ(delete_keys_history[0].key_label_match(), kKeyNamePrefix);
+  EXPECT_EQ(delete_keys_history[0].match_behavior(),
+            ::attestation::DeleteKeysRequest::MATCH_BEHAVIOR_PREFIX);
+}
+
+void ExpectDeleteKeysByPrefixNeverCalled() {
+  const std::vector<::attestation::DeleteKeysRequest> delete_keys_history =
+      chromeos::AttestationClient::Get()
+          ->GetTestInterface()
+          ->delete_keys_history();
+  EXPECT_TRUE(delete_keys_history.empty());
+}
+
 //=============== TestCertProvisioningSchedulerObserver ========================
 
 class TestCertProvisioningSchedulerObserver
@@ -103,11 +127,13 @@ class CertProvisioningSchedulerTest : public testing::Test {
   }
 
   void SetUp() override {
+    chromeos::AttestationClient::InitializeFake();
     CertProvisioningWorkerFactory::SetFactoryForTesting(&mock_factory_);
   }
 
   void TearDown() override {
     CertProvisioningWorkerFactory::SetFactoryForTesting(nullptr);
+    chromeos::AttestationClient::Shutdown();
   }
 
   void AddOnlineWifiNetwork() {
@@ -148,7 +174,6 @@ class CertProvisioningSchedulerTest : public testing::Test {
   ProfileHelperForTesting profile_helper_for_testing_;
   platform_keys::MockPlatformKeysService platform_keys_service_;
   std::unique_ptr<CertificateHelperForTesting> certificate_helper_;
-  StrictMock<SpyingFakeCryptohomeClient> fake_cryptohome_client_;
   TestingPrefServiceSimple pref_service_;
   policy::MockCloudPolicyClient cloud_policy_client_;
   // Only expected creations are allowed.
@@ -171,12 +196,6 @@ TEST_F(CertProvisioningSchedulerTest, Success) {
       network_state_test_helper_.network_state_handler(),
       std::move(mock_invalidation_factory_obj));
 
-  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
-  EXPECT_CALL(fake_cryptohome_client_,
-              OnTpmAttestationDeleteKeysByPrefix(
-                  attestation::AttestationKeyType::KEY_USER, kKeyNamePrefix))
-      .Times(1);
-
   // The policy is empty, so no workers should be created yet.
   FastForwardBy(TimeDelta::FromSeconds(1));
   EXPECT_EQ(scheduler.GetWorkers().size(), 0U);
@@ -186,6 +205,9 @@ TEST_F(CertProvisioningSchedulerTest, Success) {
       .WillOnce(
           Return(ByMove(nullptr)));  // nullptr is good enough for mock worker.
 
+  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
+  VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
+
   // One worker will be created on prefs update.
   CertProfile cert_profile(kCertProfileId, kCertProfileVersion,
                            /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
@@ -232,16 +254,14 @@ TEST_F(CertProvisioningSchedulerTest, WorkerFailed) {
       network_state_test_helper_.network_state_handler(),
       MakeFakeInvalidationFactory());
 
-  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
-  EXPECT_CALL(fake_cryptohome_client_,
-              OnTpmAttestationDeleteKeysByPrefix(
-                  attestation::AttestationKeyType::KEY_DEVICE, kKeyNamePrefix))
-      .Times(1);
 
   // The policy is empty, so no workers should be created yet.
   FastForwardBy(TimeDelta::FromSeconds(1));
   EXPECT_EQ(scheduler.GetWorkers().size(), 0U);
 
+  // From CertProvisioningScheduler::CleanVaKeysIfIdle.
+  VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
+
   // One worker will be created on prefs update.
   CertProfile cert_profile(kCertProfileId, kCertProfileVersion,
                            /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
@@ -300,12 +320,6 @@ TEST_F(CertProvisioningSchedulerTest, InitialAndDailyUpdates) {
       network_state_test_helper_.network_state_handler(),
       MakeFakeInvalidationFactory());
 
-  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
-  EXPECT_CALL(fake_cryptohome_client_,
-              OnTpmAttestationDeleteKeysByPrefix(
-                  attestation::AttestationKeyType::KEY_USER, kKeyNamePrefix))
-      .Times(1);
-
   // Now one worker should be created.
   MockCertProvisioningWorker* worker =
       mock_factory_.ExpectCreateReturnMock(kCertScope, cert_profile);
@@ -326,6 +340,9 @@ TEST_F(CertProvisioningSchedulerTest, InitialAndDailyUpdates) {
   FastForwardBy(TimeDelta::FromHours(20));
   ASSERT_EQ(scheduler.GetWorkers().size(), 0U);
 
+  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
+  VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
+
   // Now list of failed profiles should be cleared that will cause a new attempt
   // to provision certificate.
   MockCertProvisioningWorker* worker2 =
@@ -352,16 +369,13 @@ TEST_F(CertProvisioningSchedulerTest, MultipleWorkers) {
       network_state_test_helper_.network_state_handler(),
       MakeFakeInvalidationFactory());
 
-  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
-  EXPECT_CALL(fake_cryptohome_client_,
-              OnTpmAttestationDeleteKeysByPrefix(
-                  attestation::AttestationKeyType::KEY_DEVICE, kKeyNamePrefix))
-      .Times(1);
-
   // The policy is empty, so no workers should be created yet.
   FastForwardBy(TimeDelta::FromSeconds(1));
   ASSERT_EQ(scheduler.GetWorkers().size(), 0U);
 
+  // From CertProvisioningScheduler::CleanVaKeysIfIdle.
+  VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
+
   // New workers will be created on prefs update.
   const char kCertProfileId0[] = "cert_profile_id_0";
   const char kCertProfileVersion0[] = "cert_profile_version_0";
@@ -527,16 +541,13 @@ TEST_F(CertProvisioningSchedulerTest, InconsistentDataErrorHandling) {
       network_state_test_helper_.network_state_handler(),
       MakeFakeInvalidationFactory());
 
-  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
-  EXPECT_CALL(fake_cryptohome_client_,
-              OnTpmAttestationDeleteKeysByPrefix(
-                  attestation::AttestationKeyType::KEY_DEVICE, kKeyNamePrefix))
-      .Times(1);
-
   // The policy is empty, so no workers should be created yet.
   FastForwardBy(TimeDelta::FromSeconds(1));
   EXPECT_EQ(scheduler.GetWorkers().size(), 0U);
 
+  // From CertProvisioningScheduler::CleanVaKeysIfIdle.
+  VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
+
   CertProfile cert_profile_v1(kCertProfileId, kCertProfileVersion1,
                               /*is_va_enabled=*/true,
                               kCertProfileRenewalPeriod);
@@ -650,15 +661,12 @@ TEST_F(CertProvisioningSchedulerTest, RetryAfterNoInternetConnection) {
       network_state_test_helper_.network_state_handler(),
       MakeFakeInvalidationFactory());
 
-  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
-  EXPECT_CALL(fake_cryptohome_client_,
-              OnTpmAttestationDeleteKeysByPrefix(
-                  attestation::AttestationKeyType::KEY_DEVICE, kKeyNamePrefix))
-      .Times(1);
-
   FastForwardBy(TimeDelta::FromHours(72));
   ASSERT_EQ(scheduler.GetWorkers().size(), 0U);
 
+  // From CertProvisioningScheduler::CleanVaKeysIfIdle.
+  VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
+
   // Add a new worker to the factory.
   MockCertProvisioningWorker* worker =
       mock_factory_.ExpectCreateReturnMock(kCertScope, cert_profile);
@@ -689,12 +697,6 @@ TEST_F(CertProvisioningSchedulerTest, DeleteWorkerWithoutPolicy) {
       network_state_test_helper_.network_state_handler(),
       MakeFakeInvalidationFactory());
 
-  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
-  EXPECT_CALL(fake_cryptohome_client_,
-              OnTpmAttestationDeleteKeysByPrefix(
-                  attestation::AttestationKeyType::KEY_DEVICE, kKeyNamePrefix))
-      .Times(1);
-
   // Add a new worker to the factory.
   MockCertProvisioningWorker* worker =
       mock_factory_.ExpectCreateReturnMock(kCertScope, cert_profile);
@@ -720,6 +722,9 @@ TEST_F(CertProvisioningSchedulerTest, DeleteWorkerWithoutPolicy) {
                               CertProvisioningWorkerState::kCanceled);
 
   ASSERT_EQ(scheduler.GetWorkers().size(), 0U);
+
+  // From CertProvisioningScheduler::CleanVaKeysIfIdle.
+  VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
 }
 
 TEST_F(CertProvisioningSchedulerTest, DeleteVaKeysOnIdle) {
@@ -732,16 +737,16 @@ TEST_F(CertProvisioningSchedulerTest, DeleteVaKeysOnIdle) {
         network_state_test_helper_.network_state_handler(),
         MakeFakeInvalidationFactory());
 
-    // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
-    EXPECT_CALL(
-        fake_cryptohome_client_,
-        OnTpmAttestationDeleteKeysByPrefix(
-            attestation::AttestationKeyType::KEY_DEVICE, kKeyNamePrefix))
-        .Times(1);
-
     FastForwardBy(TimeDelta::FromSeconds(1));
+
+    // From CertProvisioningScheduler::CleanVaKeysIfIdle.
+    VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
   }
 
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->ClearDeleteKeysHistory();
+
   {
     CertProfile cert_profile(kCertProfileId, kCertProfileVersion,
                              /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
@@ -778,10 +783,10 @@ TEST_F(CertProvisioningSchedulerTest, DeleteVaKeysOnIdle) {
         network_state_test_helper_.network_state_handler(),
         MakeFakeInvalidationFactory());
 
-    EXPECT_CALL(fake_cryptohome_client_, OnTpmAttestationDeleteKeysByPrefix)
-        .Times(0);
 
     FastForwardBy(TimeDelta::FromSeconds(1));
+
+    ExpectDeleteKeysByPrefixNeverCalled();
   }
 }
 
@@ -797,10 +802,11 @@ TEST_F(CertProvisioningSchedulerTest, UpdateOneCert) {
   CertProfile cert_profile(kCertProfileId, kCertProfileVersion,
                            /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
 
-  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
-  EXPECT_CALL(fake_cryptohome_client_, OnTpmAttestationDeleteKeysByPrefix);
   FastForwardBy(TimeDelta::FromSeconds(1));
 
+  // From CertProvisioningScheduler::CleanVaKeysIfIdle.
+  VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
+
   // There is no policies yet, |kCertProfileId| will not be found.
   scheduler.UpdateOneCert(kCertProfileId);
   FastForwardBy(TimeDelta::FromSeconds(1));
@@ -905,17 +911,14 @@ TEST_F(CertProvisioningSchedulerTest, CertRenewal) {
       network_state_test_helper_.network_state_handler(),
       MakeFakeInvalidationFactory());
 
-  // From CertProvisioningScheduler::CleanVaKeysIfIdle.
-  EXPECT_CALL(fake_cryptohome_client_,
-              OnTpmAttestationDeleteKeysByPrefix(
-                  attestation::AttestationKeyType::KEY_USER, kKeyNamePrefix))
-      .Times(1);
-
   // The certificate already exists, nothing should happen on scheduler
   // creation.
   FastForwardBy(TimeDelta::FromSeconds(1));
   ASSERT_EQ(scheduler.GetWorkers().size(), 0U);
 
+  // From CertProvisioningScheduler::CleanVaKeysIfIdle.
+  VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
+
   // Also nothing should happen in the next ~6 days.
   FastForwardBy(TimeDelta::FromDays(5) + TimeDelta::FromHours(23));
   ASSERT_EQ(scheduler.GetWorkers().size(), 0U);
@@ -993,16 +996,13 @@ TEST_F(CertProvisioningSchedulerTest, StateChangeNotifications) {
   TestCertProvisioningSchedulerObserver observer;
   scheduler.AddObserver(&observer);
 
-  // From CertProvisioningSchedulerImpl::CleanVaKeysIfIdle.
-  EXPECT_CALL(fake_cryptohome_client_,
-              OnTpmAttestationDeleteKeysByPrefix(
-                  attestation::AttestationKeyType::KEY_DEVICE, kKeyNamePrefix))
-      .Times(1);
-
   // The policy is empty, so no workers should be created yet.
   FastForwardBy(TimeDelta::FromSeconds(1));
   ASSERT_EQ(scheduler.GetWorkers().size(), 0U);
 
+  // From CertProvisioningScheduler::CleanVaKeysIfIdle.
+  VerifyDeleteKeysByPrefixCalledOnce(kCertScope);
+
   // Two new workers will be created on prefs update.
   // Expect a state change notification for this.
   const char kCertProfileId0[] = "cert_profile_id_0";

chrome/browser/chromeos/cert_provisioning/cert_provisioning_worker.cc

@@ -817,11 +817,10 @@ void CertProvisioningWorkerImpl::CleanUpAndRunCallback() {
   OnCleanUpDone();
 }
 
-void CertProvisioningWorkerImpl::OnDeleteVaKeyDone(
-    base::Optional<bool> delete_result) {
+void CertProvisioningWorkerImpl::OnDeleteVaKeyDone(bool delete_result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (!delete_result.has_value() || !delete_result.value()) {
+  if (!delete_result) {
     LOG(ERROR) << "Failed to delete a va key";
   }
   OnCleanUpDone();

chrome/browser/chromeos/cert_provisioning/cert_provisioning_worker.h

@@ -211,7 +211,7 @@ class CertProvisioningWorkerImpl : public CertProvisioningWorker {
   void InitAfterDeserialization();
 
   void CleanUpAndRunCallback();
-  void OnDeleteVaKeyDone(base::Optional<bool> delete_result);
+  void OnDeleteVaKeyDone(bool delete_result);
   void OnRemoveKeyDone(platform_keys::Status status);
   void OnCleanUpDone();
 

chrome/browser/chromeos/cert_provisioning/cert_provisioning_worker_unittest.cc

@@ -29,6 +29,7 @@
 #include "chrome/browser/chromeos/platform_keys/platform_keys.h"
 #include "chrome/browser/chromeos/platform_keys/platform_keys_service.h"
 #include "chrome/browser/chromeos/platform_keys/platform_keys_service_factory.h"
+#include "chromeos/dbus/attestation/fake_attestation_client.h"
 #include "components/policy/core/common/cloud/mock_cloud_policy_client.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/prefs/pref_service.h"
@@ -110,6 +111,20 @@ const std::string& GetPublicKey() {
   return public_key;
 }
 
+void VerifyDeleteKeyCalledOnce(CertScope cert_scope) {
+  const std::vector<::attestation::DeleteKeysRequest> delete_keys_history =
+      chromeos::AttestationClient::Get()
+          ->GetTestInterface()
+          ->delete_keys_history();
+  EXPECT_EQ(delete_keys_history.size(), 1);
+  EXPECT_EQ(delete_keys_history[0].username().empty(),
+            cert_scope != CertScope::kUser);
+  EXPECT_EQ(delete_keys_history[0].key_label_match(),
+            GetKeyName(kCertProfileId));
+  EXPECT_EQ(delete_keys_history[0].match_behavior(),
+            ::attestation::DeleteKeysRequest::MATCH_BEHAVIOR_EXACT);
+}
+
 // Using macros to reduce boilerplate code, but keep real line numbers in
 // error messages in case of expectation failure. They use some of protected
 // fields of CertProvisioningWorkerTest class and may be considered as extra
@@ -337,6 +352,7 @@ class CertProvisioningWorkerTest : public ::testing::Test {
   ~CertProvisioningWorkerTest() override = default;
 
   void SetUp() override {
+    ::chromeos::AttestationClient::InitializeFake();
     // There should not be any calls to callback before this expect is
     // overridden.
     EXPECT_CALL(callback_observer_, Callback).Times(0);
@@ -348,6 +364,7 @@ class CertProvisioningWorkerTest : public ::testing::Test {
   void TearDown() override {
     EXPECT_FALSE(
         attestation::TpmChallengeKeySubtleFactory::WillReturnTestingInstance());
+    ::chromeos::AttestationClient::Shutdown();
   }
 
  protected:
@@ -427,7 +444,6 @@ class CertProvisioningWorkerTest : public ::testing::Test {
 
   StrictMock<StateChangeCallbackObserver> state_change_callback_observer_;
   StrictMock<CallbackObserver> callback_observer_;
-  StrictMock<SpyingFakeCryptohomeClient> fake_cryptohome_client_;
   ProfileHelperForTesting profile_helper_for_testing_;
   TestingPrefServiceSimple testing_pref_service_;
 
@@ -815,12 +831,13 @@ TEST_F(CertProvisioningWorkerTest, TryLaterWait) {
 // Checks that when the server returns error status, the worker will enter an
 // error state and stop the provisioning.
 TEST_F(CertProvisioningWorkerTest, StatusErrorHandling) {
+  const CertScope kCertScope = CertScope::kUser;
   CertProfile cert_profile(kCertProfileId, kCertProfileVersion,
                            /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
 
   MockTpmChallengeKeySubtle* mock_tpm_challenge_key = PrepareTpmChallengeKey();
   CertProvisioningWorkerImpl worker(
-      CertScope::kUser, GetProfile(), &testing_pref_service_, cert_profile,
+      kCertScope, GetProfile(), &testing_pref_service_, cert_profile,
       &cloud_policy_client_, MakeInvalidator(), GetStateChangeCallback(),
       GetResultCallback());
 
@@ -841,12 +858,6 @@ TEST_F(CertProvisioningWorkerTest, StatusErrorHandling) {
         kCertScopeStrUser, kCertProfileId, kCertProfileVersion, GetPublicKey(),
         /*callback=*/_));
 
-    EXPECT_CALL(
-        fake_cryptohome_client_,
-        OnTpmAttestationDeleteKey(attestation::AttestationKeyType::KEY_USER,
-                                  GetKeyName(kCertProfileId)))
-        .Times(1);
-
     EXPECT_CALL(callback_observer_,
                 Callback(cert_profile, CertProvisioningWorkerState::kFailed))
         .Times(1);
@@ -854,11 +865,14 @@ TEST_F(CertProvisioningWorkerTest, StatusErrorHandling) {
 
   worker.DoStep();
   FastForwardBy(TimeDelta::FromSeconds(1));
+
+  VerifyDeleteKeyCalledOnce(kCertScope);
 }
 
 // Checks that when the server returns response error, the worker will enter an
 // error state and stop the provisioning. Also check factory.
 TEST_F(CertProvisioningWorkerTest, ResponseErrorHandling) {
+  const CertScope kCertScope = CertScope::kUser;
   base::HistogramTester histogram_tester;
 
   CertProfile cert_profile(kCertProfileId, kCertProfileVersion,
@@ -866,7 +880,7 @@ TEST_F(CertProvisioningWorkerTest, ResponseErrorHandling) {
 
   MockTpmChallengeKeySubtle* mock_tpm_challenge_key = PrepareTpmChallengeKey();
   auto worker = CertProvisioningWorkerFactory::Get()->Create(
-      CertScope::kUser, GetProfile(), &testing_pref_service_, cert_profile,
+      kCertScope, GetProfile(), &testing_pref_service_, cert_profile,
       &cloud_policy_client_, MakeInvalidator(), GetStateChangeCallback(),
       GetResultCallback());
 
@@ -885,12 +899,6 @@ TEST_F(CertProvisioningWorkerTest, ResponseErrorHandling) {
 
     EXPECT_START_CSR_CA_ERROR(ClientCertProvisioningStartCsr);
 
-    EXPECT_CALL(
-        fake_cryptohome_client_,
-        OnTpmAttestationDeleteKey(attestation::AttestationKeyType::KEY_USER,
-                                  GetKeyName(kCertProfileId)))
-        .Times(1);
-
     EXPECT_CALL(callback_observer_,
                 Callback(cert_profile, CertProvisioningWorkerState::kFailed))
         .Times(1);
@@ -899,6 +907,8 @@ TEST_F(CertProvisioningWorkerTest, ResponseErrorHandling) {
   worker->DoStep();
   FastForwardBy(TimeDelta::FromSeconds(1));
 
+  VerifyDeleteKeyCalledOnce(kCertScope);
+
   histogram_tester.ExpectBucketCount("ChromeOS.CertProvisioning.Result.User",
                                      CertProvisioningWorkerState::kFailed, 1);
   histogram_tester.ExpectBucketCount(
@@ -908,12 +918,13 @@ TEST_F(CertProvisioningWorkerTest, ResponseErrorHandling) {
 }
 
 TEST_F(CertProvisioningWorkerTest, InconsistentDataErrorHandling) {
+  const CertScope kCertScope = CertScope::kUser;
   CertProfile cert_profile(kCertProfileId, kCertProfileVersion,
                            /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
 
   MockTpmChallengeKeySubtle* mock_tpm_challenge_key = PrepareTpmChallengeKey();
   auto worker = CertProvisioningWorkerFactory::Get()->Create(
-      CertScope::kUser, GetProfile(), &testing_pref_service_, cert_profile,
+      kCertScope, GetProfile(), &testing_pref_service_, cert_profile,
       &cloud_policy_client_, MakeInvalidator(), GetStateChangeCallback(),
       GetResultCallback());
 
@@ -932,12 +943,6 @@ TEST_F(CertProvisioningWorkerTest, InconsistentDataErrorHandling) {
 
     EXPECT_START_CSR_INCONSISTENT_DATA(ClientCertProvisioningStartCsr);
 
-    EXPECT_CALL(
-        fake_cryptohome_client_,
-        OnTpmAttestationDeleteKey(attestation::AttestationKeyType::KEY_USER,
-                                  GetKeyName(kCertProfileId)))
-        .Times(1);
-
     EXPECT_CALL(callback_observer_,
                 Callback(cert_profile,
                          CertProvisioningWorkerState::kInconsistentDataError))
@@ -946,6 +951,8 @@ TEST_F(CertProvisioningWorkerTest, InconsistentDataErrorHandling) {
 
   worker->DoStep();
   FastForwardBy(TimeDelta::FromSeconds(1));
+
+  VerifyDeleteKeyCalledOnce(kCertScope);
 }
 
 // Checks that when the server returns TEMPORARY_UNAVAILABLE status code, the
@@ -1128,7 +1135,7 @@ TEST_F(CertProvisioningWorkerTest, SerializationSuccess) {
           GetResultCallback());
 
   StrictMock<PrefServiceObserver> pref_observer(
-      &testing_pref_service_, GetPrefNameForSerialization(CertScope::kUser));
+      &testing_pref_service_, GetPrefNameForSerialization(kCertScope));
   base::Value pref_val;
 
   EXPECT_CALL(state_change_callback_observer_, StateChangeCallback)
@@ -1297,17 +1304,18 @@ TEST_F(CertProvisioningWorkerTest, SerializationSuccess) {
 }
 
 TEST_F(CertProvisioningWorkerTest, SerializationOnFailure) {
+  const CertScope kCertScope = CertScope::kUser;
   CertProfile cert_profile(kCertProfileId, kCertProfileVersion,
                            /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
 
   MockTpmChallengeKeySubtle* mock_tpm_challenge_key = PrepareTpmChallengeKey();
   auto worker = CertProvisioningWorkerFactory::Get()->Create(
-      CertScope::kUser, GetProfile(), &testing_pref_service_, cert_profile,
+      kCertScope, GetProfile(), &testing_pref_service_, cert_profile,
       &cloud_policy_client_, MakeInvalidator(), GetStateChangeCallback(),
       GetResultCallback());
 
-  PrefServiceObserver pref_observer(
-      &testing_pref_service_, GetPrefNameForSerialization(CertScope::kUser));
+  PrefServiceObserver pref_observer(&testing_pref_service_,
+                                    GetPrefNameForSerialization(kCertScope));
   base::Value pref_val;
 
   EXPECT_CALL(state_change_callback_observer_, StateChangeCallback)
@@ -1345,12 +1353,6 @@ TEST_F(CertProvisioningWorkerTest, SerializationOnFailure) {
     pref_val = ParseJson("{}");
     EXPECT_CALL(pref_observer, OnPrefValueUpdated(IsJson(pref_val))).Times(1);
 
-    EXPECT_CALL(
-        fake_cryptohome_client_,
-        OnTpmAttestationDeleteKey(attestation::AttestationKeyType::KEY_USER,
-                                  GetKeyName(kCertProfileId)))
-        .Times(1);
-
     EXPECT_CALL(callback_observer_,
                 Callback(cert_profile, CertProvisioningWorkerState::kFailed))
         .Times(1);
@@ -1358,15 +1360,18 @@ TEST_F(CertProvisioningWorkerTest, SerializationOnFailure) {
 
   worker->DoStep();
   FastForwardBy(TimeDelta::FromSeconds(1));
+
+  VerifyDeleteKeyCalledOnce(kCertScope);
 }
 
 TEST_F(CertProvisioningWorkerTest, InformationalGetters) {
+  const CertScope kCertScope = CertScope::kUser;
   CertProfile cert_profile(kCertProfileId, kCertProfileVersion,
                            /*is_va_enabled=*/true, kCertProfileRenewalPeriod);
 
   MockTpmChallengeKeySubtle* mock_tpm_challenge_key = PrepareTpmChallengeKey();
   CertProvisioningWorkerImpl worker(
-      CertScope::kUser, GetProfile(), &testing_pref_service_, cert_profile,
+      kCertScope, GetProfile(), &testing_pref_service_, cert_profile,
       &cloud_policy_client_, MakeInvalidator(), GetStateChangeCallback(),
       GetResultCallback());
 
@@ -1394,12 +1399,6 @@ TEST_F(CertProvisioningWorkerTest, InformationalGetters) {
 
     EXPECT_START_CSR_CA_ERROR(ClientCertProvisioningStartCsr);
 
-    EXPECT_CALL(
-        fake_cryptohome_client_,
-        OnTpmAttestationDeleteKey(attestation::AttestationKeyType::KEY_USER,
-                                  GetKeyName(kCertProfileId)))
-        .Times(1);
-
     EXPECT_CALL(callback_observer_,
                 Callback(cert_profile, CertProvisioningWorkerState::kFailed))
         .Times(1);
@@ -1407,6 +1406,8 @@ TEST_F(CertProvisioningWorkerTest, InformationalGetters) {
     worker.DoStep();
     FastForwardBy(TimeDelta::FromSeconds(1));
 
+    VerifyDeleteKeyCalledOnce(kCertScope);
+
     EXPECT_EQ(worker.GetState(), CertProvisioningWorkerState::kFailed);
     EXPECT_EQ(worker.GetPreviousState(),
               CertProvisioningWorkerState::kKeypairGenerated);
@@ -1432,8 +1433,8 @@ TEST_F(CertProvisioningWorkerTest, CancelDeviceWorker) {
 
   EXPECT_CALL(callback_observer_, Callback).Times(0);
 
-  PrefServiceObserver pref_observer(
-      &testing_pref_service_, GetPrefNameForSerialization(CertScope::kDevice));
+  PrefServiceObserver pref_observer(&testing_pref_service_,
+                                    GetPrefNameForSerialization(kCertScope));
   base::Value pref_val;
 
   {
@@ -1470,12 +1471,6 @@ TEST_F(CertProvisioningWorkerTest, CancelDeviceWorker) {
   }
 
   {
-    EXPECT_CALL(
-        fake_cryptohome_client_,
-        OnTpmAttestationDeleteKey(attestation::AttestationKeyType::KEY_DEVICE,
-                                  GetKeyName(kCertProfileId)))
-        .Times(1);
-
     pref_val = ParseJson("{}");
     EXPECT_CALL(pref_observer, OnPrefValueUpdated(IsJson(pref_val))).Times(1);
 
@@ -1485,6 +1480,8 @@ TEST_F(CertProvisioningWorkerTest, CancelDeviceWorker) {
                 Callback(cert_profile, CertProvisioningWorkerState::kCanceled))
         .Times(1);
     FastForwardBy(TimeDelta::FromSeconds(1));
+
+    VerifyDeleteKeyCalledOnce(kCertScope);
   }
 
   histogram_tester.ExpectUniqueSample("ChromeOS.CertProvisioning.Result.Device",

chromeos/dbus/attestation/attestation_client.h

@@ -6,6 +6,7 @@
 #define CHROMEOS_DBUS_ATTESTATION_ATTESTATION_CLIENT_H_
 
 #include <deque>
+#include <vector>
 
 #include "base/callback.h"
 #include "base/component_export.h"
@@ -110,6 +111,13 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_ATTESTATION) AttestationClient {
     // queried.
     virtual ::attestation::CreateCertificateRequestReply*
     mutable_certificate_request_reply() = 0;
+
+    // Gets the history of `DeleteKeys()` requests.
+    virtual const std::vector<::attestation::DeleteKeysRequest>&
+    delete_keys_history() const = 0;
+
+    // Clears the request history of `DeleteKeys()`.
+    virtual void ClearDeleteKeysHistory() = 0;
   };
 
   // Not copyable or movable.

chromeos/dbus/attestation/fake_attestation_client.cc

@@ -234,7 +234,10 @@ void FakeAttestationClient::SetKeyPayload(
 void FakeAttestationClient::DeleteKeys(
     const ::attestation::DeleteKeysRequest& request,
     DeleteKeysCallback callback) {
-  NOTIMPLEMENTED();
+  delete_keys_history_.push_back(request);
+  ::attestation::DeleteKeysReply reply;
+  reply.set_status(::attestation::STATUS_SUCCESS);
+  PostProtoResponse(std::move(callback), reply);
 }
 
 void FakeAttestationClient::ResetIdentity(
@@ -296,6 +299,15 @@ void FakeAttestationClient::AllowlistLegacyCreateCertificateRequest(
   allowlisted_create_requests_.push_back(request);
 }
 
+const std::vector<::attestation::DeleteKeysRequest>&
+FakeAttestationClient::delete_keys_history() const {
+  return delete_keys_history_;
+}
+
+void FakeAttestationClient::ClearDeleteKeysHistory() {
+  delete_keys_history_.clear();
+}
+
 AttestationClient::TestInterface* FakeAttestationClient::GetTestInterface() {
   return this;
 }

chromeos/dbus/attestation/fake_attestation_client.h

@@ -106,6 +106,9 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_ATTESTATION) FakeAttestationClient
       ::attestation::KeyType key_type) override;
   ::attestation::CreateCertificateRequestReply*
   mutable_certificate_request_reply() override;
+  const std::vector<::attestation::DeleteKeysRequest>& delete_keys_history()
+      const override;
+  void ClearDeleteKeysHistory() override;
 
   AttestationClient::TestInterface* GetTestInterface() override;
 
@@ -130,6 +133,9 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_ATTESTATION) FakeAttestationClient
   std::vector<int> certificate_indices_;
   // The count of certificates that has been issued.
   int certificate_count_ = 0;
+
+  // Maintains the input request history of `DeleteKeys()`.
+  std::vector<::attestation::DeleteKeysRequest> delete_keys_history_;
 };
 
 }  // namespace chromeos