use AttestationClient for enrollment status

We are deprecating attestation methods by CryptohomeClient.

BUG=b:158955123
TEST=unit_tests and browser_tests.

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

chromeos/attestation/attestation_flow.cc

@@ -123,25 +123,29 @@ void AttestationFlow::GetCertificate(
 
   // If this device has not enrolled with the Privacy CA, we need to do that
   // first.  Once enrolled we can proceed with the certificate request.
-  cryptohome_client_->TpmAttestationIsEnrolled(base::BindOnce(
-      &AttestationFlow::OnEnrollmentCheckComplete, weak_factory_.GetWeakPtr(),
-      std::move(start_certificate_request)));
+  attestation_client_->GetStatus(
+      ::attestation::GetStatusRequest(),
+      base::BindOnce(&AttestationFlow::OnEnrollmentCheckComplete,
+                     weak_factory_.GetWeakPtr(),
+                     std::move(start_certificate_request)));
 }
 
 void AttestationFlow::OnEnrollmentCheckComplete(
     base::OnceCallback<void(bool)> callback,
-    base::Optional<bool> result) {
-  if (!result) {
-    LOG(ERROR) << "Attestation: Failed to check enrollment state.";
+    const ::attestation::GetStatusReply& reply) {
+  if (reply.status() != ::attestation::STATUS_SUCCESS) {
+    LOG(ERROR) << "Attestation: Failed to check enrollment state. Status: "
+               << reply.status();
     std::move(callback).Run(false);
     return;
   }
 
-  if (*result) {
+  if (reply.enrolled()) {
     std::move(callback).Run(true);
     return;
   }
 
+  // The device is not enrolled; check if it's enrollment prepared.
   base::TimeTicks end_time = base::TimeTicks::Now() + ready_timeout_;
   WaitForAttestationPrepared(end_time, std::move(callback));
 }

chromeos/attestation/attestation_flow.h

@@ -126,14 +126,14 @@ class COMPONENT_EXPORT(CHROMEOS_ATTESTATION) AttestationFlow {
                               CertificateCallback callback);
 
  private:
-  // Handles the result of a call to TpmAttestationIsEnrolled. Reports success
-  // if enrollment is complete and otherwise starts the process.
+  // Handles the result of a call to `GetStatus()` for enrollment status.
+  // Reports success if enrollment is complete and otherwise starts the process.
   //
   // Parameters
   //   callback - Called with the success or failure of the enrollment.
-  //   result - Result of TpmAttestationIsEnrolled().
+  //   result - Result of `GetStatus()`, which contains `enrolled` field.
   void OnEnrollmentCheckComplete(base::OnceCallback<void(bool)> callback,
-                                 base::Optional<bool> result);
+                                 const ::attestation::GetStatusReply& reply);
 
   // Asynchronously waits for attestation to be ready and start enrollment once
   // it is. If attestation is not ready by the time the flow's timeout is

chromeos/attestation/attestation_flow_unittest.cc

@@ -78,9 +78,13 @@ TEST_F(AttestationFlowTest, GetCertificate) {
   // Verify the order of calls in a sequence.
   Sequence flow_order;
 
-  // Use DBusCallbackFalse so the full enrollment flow is triggered.
-  chromeos::FakeCryptohomeClient client;
-  client.set_tpm_attestation_is_enrolled(false);
+  FakeCryptohomeClient client;
+  // Set the enrollment status as `false` so the full enrollment flow is
+  // triggered.
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(false);
   chromeos::AttestationClient::Get()
       ->GetTestInterface()
       ->ConfigureEnrollmentPreparations(true);
@@ -166,9 +170,12 @@ TEST_F(AttestationFlowTest, GetCertificate_Ecc) {
   Sequence flow_order;
 
   FakeCryptohomeClient client;
-  // Set the enrollment status as |false| so the full enrollment flow is
+  // Set the enrollment status as `false` so the full enrollment flow is
   // triggered.
-  client.set_tpm_attestation_is_enrolled(false);
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(false);
   chromeos::AttestationClient::Get()
       ->GetTestInterface()
       ->ConfigureEnrollmentPreparations(true);
@@ -253,7 +260,10 @@ TEST_F(AttestationFlowTest, GetCertificate_Attestation_Not_Prepared) {
   Sequence flow_order;
 
   FakeCryptohomeClient client;
-  client.set_tpm_attestation_is_enrolled(false);
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(false);
   chromeos::AttestationClient::Get()
       ->GetTestInterface()
       ->ConfigureEnrollmentPreparationsSequence({false, true});
@@ -340,7 +350,10 @@ TEST_F(AttestationFlowTest, GetCertificate_Attestation_Never_Prepared) {
   async_caller.SetUp(false, cryptohome::MOUNT_ERROR_NONE);
 
   chromeos::FakeCryptohomeClient client;
-  client.set_tpm_attestation_is_enrolled(false);
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(false);
   chromeos::AttestationClient::Get()
       ->GetTestInterface()
       ->ConfigureEnrollmentPreparations(false);
@@ -375,7 +388,10 @@ TEST_F(AttestationFlowTest, GetCertificate_Attestation_Never_Confirm_Prepared) {
   async_caller.SetUp(false, cryptohome::MOUNT_ERROR_NONE);
 
   chromeos::FakeCryptohomeClient client;
-  client.set_tpm_attestation_is_enrolled(false);
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(false);
   chromeos::AttestationClient::Get()
       ->GetTestInterface()
       ->ConfigureEnrollmentPreparationsStatus(
@@ -413,7 +429,10 @@ TEST_F(AttestationFlowTest, GetCertificate_NoEK) {
       .Times(1);
 
   chromeos::FakeCryptohomeClient client;
-  client.set_tpm_attestation_is_enrolled(false);
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(false);
   chromeos::AttestationClient::Get()
       ->GetTestInterface()
       ->ConfigureEnrollmentPreparations(true);
@@ -444,7 +463,10 @@ TEST_F(AttestationFlowTest, GetCertificate_EKRejected) {
       .Times(1);
 
   chromeos::FakeCryptohomeClient client;
-  client.set_tpm_attestation_is_enrolled(false);
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(false);
   chromeos::AttestationClient::Get()
       ->GetTestInterface()
       ->ConfigureEnrollmentPreparations(true);
@@ -486,7 +508,10 @@ TEST_F(AttestationFlowTest, GetCertificate_FailEnroll) {
       .WillOnce(WithArgs<2>(Invoke(AsyncCallbackFalse)));
 
   chromeos::FakeCryptohomeClient client;
-  client.set_tpm_attestation_is_enrolled(false);
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(false);
   chromeos::AttestationClient::Get()
       ->GetTestInterface()
       ->ConfigureEnrollmentPreparations(true);
@@ -516,6 +541,10 @@ TEST_F(AttestationFlowTest, GetCertificate_FailEnroll) {
 }
 
 TEST_F(AttestationFlowTest, GetMachineCertificateAlreadyEnrolled) {
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(true);
   StrictMock<cryptohome::MockAsyncMethodCaller> async_caller;
   async_caller.SetUp(true, cryptohome::MOUNT_ERROR_NONE);
 
@@ -569,6 +598,10 @@ TEST_F(AttestationFlowTest, GetMachineCertificateAlreadyEnrolled) {
 }
 
 TEST_F(AttestationFlowTest, GetEnrollmentCertificateAlreadyEnrolled) {
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(true);
   StrictMock<cryptohome::MockAsyncMethodCaller> async_caller;
   async_caller.SetUp(true, cryptohome::MOUNT_ERROR_NONE);
   chromeos::AttestationClient::Get()
@@ -621,6 +654,10 @@ TEST_F(AttestationFlowTest, GetEnrollmentCertificateAlreadyEnrolled) {
 }
 
 TEST_F(AttestationFlowTest, GetCertificate_FailCreateCertRequest) {
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(true);
   StrictMock<cryptohome::MockAsyncMethodCaller> async_caller;
   async_caller.SetUp(false, cryptohome::MOUNT_ERROR_NONE);
   chromeos::AttestationClient::Get()
@@ -656,6 +693,10 @@ TEST_F(AttestationFlowTest, GetCertificate_FailCreateCertRequest) {
 }
 
 TEST_F(AttestationFlowTest, GetCertificate_CertRequestRejected) {
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(true);
   StrictMock<cryptohome::MockAsyncMethodCaller> async_caller;
   async_caller.SetUp(true, cryptohome::MOUNT_ERROR_NONE);
   chromeos::FakeCryptohomeClient client;
@@ -696,6 +737,10 @@ TEST_F(AttestationFlowTest, GetCertificate_CertRequestRejected) {
 }
 
 TEST_F(AttestationFlowTest, GetCertificate_CertRequestBadRequest) {
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(true);
   StrictMock<cryptohome::MockAsyncMethodCaller> async_caller;
   async_caller.SetUp(true, cryptohome::MOUNT_ERROR_NONE);
 
@@ -748,7 +793,10 @@ TEST_F(AttestationFlowTest, GetCertificate_FailIsEnrolled) {
   StrictMock<cryptohome::MockAsyncMethodCaller> async_caller;
 
   chromeos::FakeCryptohomeClient client;
-  client.SetServiceIsAvailable(false);
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_status(::attestation::STATUS_DBUS_ERROR);
 
   // We're not expecting any server calls in this case; StrictMock will verify.
   std::unique_ptr<MockServerProxy> proxy(new StrictMock<MockServerProxy>());
@@ -770,6 +818,10 @@ TEST_F(AttestationFlowTest, GetCertificate_FailIsEnrolled) {
 }
 
 TEST_F(AttestationFlowTest, GetCertificate_CheckExisting) {
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(true);
   StrictMock<cryptohome::MockAsyncMethodCaller> async_caller;
   async_caller.SetUp(true, cryptohome::MOUNT_ERROR_NONE);
   chromeos::AttestationClient::Get()
@@ -821,6 +873,10 @@ TEST_F(AttestationFlowTest, GetCertificate_CheckExisting) {
 }
 
 TEST_F(AttestationFlowTest, GetCertificate_AlreadyExists) {
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->mutable_status_reply()
+      ->set_enrolled(true);
   // We're not expecting any async calls in this case; StrictMock will verify.
   StrictMock<cryptohome::MockAsyncMethodCaller> async_caller;
 

chromeos/dbus/attestation/attestation_client.h

@@ -92,6 +92,9 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_ATTESTATION) AttestationClient {
     virtual void ConfigureEnrollmentPreparationsStatus(
         ::attestation::AttestationStatus status) = 0;
 
+    // Gets the mutable |GetStatusReply| that is returned when queried.
+    virtual ::attestation::GetStatusReply* mutable_status_reply() = 0;
+
     // Allowlists |request| so the certificate requests that comes in afterwards
     // will get a fake certificate. If any alias of |request| has been
     // allowlisted this functions performs no-ops.

chromeos/dbus/attestation/fake_attestation_client.cc

@@ -45,7 +45,9 @@ bool GetCertificateRequestEqual(::attestation::GetCertificateRequest r1,
 
 }  // namespace
 
-FakeAttestationClient::FakeAttestationClient() = default;
+FakeAttestationClient::FakeAttestationClient() {
+  status_reply_.set_enrolled(true);
+}
 
 FakeAttestationClient::~FakeAttestationClient() = default;
 
@@ -143,7 +145,7 @@ void FakeAttestationClient::GetEnrollmentPreparations(
 void FakeAttestationClient::GetStatus(
     const ::attestation::GetStatusRequest& request,
     GetStatusCallback callback) {
-  NOTIMPLEMENTED();
+  PostProtoResponse(std::move(callback), status_reply_);
 }
 
 void FakeAttestationClient::Verify(const ::attestation::VerifyRequest& request,
@@ -198,8 +200,10 @@ void FakeAttestationClient::GetCertificate(
   reply.set_status(
       ::attestation::AttestationStatus::STATUS_UNEXPECTED_DEVICE_ERROR);
 
-  is_enrolled_ |= request.shall_trigger_enrollment();
-  if (!is_enrolled_) {
+  if (request.shall_trigger_enrollment()) {
+    status_reply_.set_enrolled(true);
+  }
+  if (!status_reply_.enrolled()) {
     PostProtoResponse(std::move(callback), reply);
     return;
   }
@@ -302,6 +306,10 @@ void FakeAttestationClient::ConfigureEnrollmentPreparationsStatus(
   preparations_status_ = status;
 }
 
+::attestation::GetStatusReply* FakeAttestationClient::mutable_status_reply() {
+  return &status_reply_;
+}
+
 void FakeAttestationClient::AllowlistCertificateRequest(
     const ::attestation::GetCertificateRequest& request) {
   for (const auto& req : allowlisted_requests_) {

chromeos/dbus/attestation/fake_attestation_client.h

@@ -99,6 +99,7 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_ATTESTATION) FakeAttestationClient
       std::deque<bool> sequence) override;
   void ConfigureEnrollmentPreparationsStatus(
       ::attestation::AttestationStatus status) override;
+  ::attestation::GetStatusReply* mutable_status_reply() override;
   void AllowlistCertificateRequest(
       const ::attestation::GetCertificateRequest& request) override;
   void AllowlistLegacyCreateCertificateRequest(
@@ -126,7 +127,7 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_ATTESTATION) FakeAttestationClient
   bool is_prepared_ = true;
   std::deque<bool> preparation_sequences_;
 
-  bool is_enrolled_ = false;
+  ::attestation::GetStatusReply status_reply_;
 
   ::attestation::CreateCertificateRequestReply certificate_request_reply_;