use AttestationClient to get key info in AttestationFlow.

We are deprecating attestation methods by CryptohomeClient.

This CL also includes a simplied flow by consolidating
TpmAttestationDoesKeyExist and TpmAttestationGetCertificate, of which
results come from the same methods of attestation service anyway.

BUG=b:158955123
TEST=chromeos_unittests.

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

chromeos/attestation/attestation_flow.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/optional.h"
@@ -50,21 +51,6 @@ constexpr uint16_t kReadyTimeoutInSeconds = 60;
 // attestation.
 constexpr uint16_t kRetryDelayInMilliseconds = 300;
 
-void DBusCertificateMethodCallback(
-    AttestationFlow::CertificateCallback callback,
-    base::Optional<CryptohomeClient::TpmAttestationDataResult> result) {
-  if (!result.has_value()) {
-    LOG(ERROR) << "Attestation: DBus data operation failed.";
-    std::move(callback).Run(ATTESTATION_UNSPECIFIED_FAILURE, "");
-    return;
-  }
-  if (callback) {
-    std::move(callback).Run(
-        result->success ? ATTESTATION_SUCCESS : ATTESTATION_UNSPECIFIED_FAILURE,
-        result->data);
-  }
-}
-
 }  // namespace
 
 AttestationKeyType AttestationFlow::GetKeyTypeForProfile(
@@ -92,7 +78,13 @@ AttestationFlow::AttestationFlow(cryptohome::AsyncMethodCaller* async_caller,
       crypto_key_type_(crypto_key_type),
       ready_timeout_(base::TimeDelta::FromSeconds(kReadyTimeoutInSeconds)),
       retry_delay_(
-          base::TimeDelta::FromMilliseconds(kRetryDelayInMilliseconds)) {}
+          base::TimeDelta::FromMilliseconds(kRetryDelayInMilliseconds)) {
+  // TODO(b/158955123): For now we only make the compiler happy because the
+  // removal of this involves changes to multiple consumers, but eventually we
+  // should get rid of them at one go once `AttestationFlow` doesn't use
+  // cryptohome client and its async method caller.
+  ALLOW_UNUSED_LOCAL(cryptohome_client_);
+}
 
 AttestationFlow::AttestationFlow(cryptohome::AsyncMethodCaller* async_caller,
                                  CryptohomeClient* cryptohome_client,
@@ -272,39 +264,45 @@ void AttestationFlow::StartCertificateRequest(
     return;
   }
 
-  cryptohome_client_->TpmAttestationDoesKeyExist(
-      key_type, cryptohome::CreateAccountIdentifierFromAccountId(account_id),
-      key_name,
-      base::BindOnce(&AttestationFlow::OnKeyExistCheckComplete,
+  ::attestation::GetKeyInfoRequest request;
+  request.set_username(
+      cryptohome::CreateAccountIdentifierFromAccountId(account_id)
+          .account_id());
+  request.set_key_label(key_name);
+  attestation_client_->GetKeyInfo(
+      request, base::BindOnce(&AttestationFlow::OnGetKeyInfoComplete,
                               weak_factory_.GetWeakPtr(), certificate_profile,
                               account_id, request_origin, key_name, key_type,
                               std::move(callback)));
 }
 
-void AttestationFlow::OnKeyExistCheckComplete(
+void AttestationFlow::OnGetKeyInfoComplete(
     AttestationCertificateProfile certificate_profile,
     const AccountId& account_id,
     const std::string& request_origin,
     const std::string& key_name,
     AttestationKeyType key_type,
     CertificateCallback callback,
-    base::Optional<bool> result) {
-  if (!result) {
-    LOG(ERROR) << "Attestation: Failed to check for existence of key.";
-    std::move(callback).Run(ATTESTATION_UNSPECIFIED_FAILURE, "");
+    const ::attestation::GetKeyInfoReply& reply) {
+  // If the key already exists, return the existing certificate.
+  if (reply.status() == ::attestation::STATUS_SUCCESS) {
+    std::move(callback).Run(ATTESTATION_SUCCESS, reply.certificate());
     return;
   }
 
-  // If the key already exists, query the existing certificate.
-  if (*result) {
-    GetExistingCertificate(key_type, account_id, key_name, std::move(callback));
+  // If the key does not exist, call this method back with |generate_new_key|
+  // set to true.
+  if (reply.status() == ::attestation::STATUS_INVALID_PARAMETER) {
+    StartCertificateRequest(certificate_profile, account_id, request_origin,
+                            /*generate_new_key=*/true, key_name,
+                            std::move(callback), /*enrolled=*/true);
     return;
   }
 
-  // If the key does not exist, call this method back with |generate_new_key|
-  // set to true.
-  StartCertificateRequest(certificate_profile, account_id, request_origin, true,
-                          key_name, std::move(callback), true);
+  // Otherwise the key info query fails.
+  LOG(ERROR) << "Attestation: Failed to check for existence of key; status: "
+             << reply.status() << ".";
+  std::move(callback).Run(ATTESTATION_UNSPECIFIED_FAILURE, "");
 }
 
 void AttestationFlow::SendCertificateRequestToPCA(
@@ -357,16 +355,6 @@ void AttestationFlow::OnCertRequestFinished(CertificateCallback callback,
     std::move(callback).Run(ATTESTATION_SERVER_BAD_REQUEST_FAILURE, data);
 }
 
-void AttestationFlow::GetExistingCertificate(AttestationKeyType key_type,
-                                             const AccountId& account_id,
-                                             const std::string& key_name,
-                                             CertificateCallback callback) {
-  cryptohome_client_->TpmAttestationGetCertificate(
-      key_type, cryptohome::CreateAccountIdentifierFromAccountId(account_id),
-      key_name,
-      base::BindOnce(&DBusCertificateMethodCallback, std::move(callback)));
-}
-
 ServerProxy::~ServerProxy() = default;
 
 PrivacyCAType ServerProxy::GetType() {

chromeos/attestation/attestation_flow.h

@@ -214,9 +214,8 @@ class COMPONENT_EXPORT(CHROMEOS_ATTESTATION) AttestationFlow {
       CertificateCallback callback,
       bool enrolled);
 
-  // Called with the result of TpmAttestationDoesKeyExist(). Will query the
-  // existing certificate if it exists and otherwise start a new certificate
-  // request.
+  // Called with the reply to `GetKeyInfo()`. Will query the existing
+  // certificate if it exists and otherwise start a new certificate request.
   //
   // Parameters
   //   certificate_profile - Specifies what kind of certificate should be
@@ -227,15 +226,14 @@ class COMPONENT_EXPORT(CHROMEOS_ATTESTATION) AttestationFlow {
   //   key_name - The name of the key. If left empty, a default name derived
   //              from the |certificate_profile| and |account_id| will be used.
   //   callback - Called when the operation completes.
-  //   result - Result of TpmAttestationDoesKeyExist().
-  void OnKeyExistCheckComplete(
-      AttestationCertificateProfile certificate_profile,
+  //   reply - The reply of `GetKeyInfo()`.
+  void OnGetKeyInfoComplete(AttestationCertificateProfile certificate_profile,
                             const AccountId& account_id,
                             const std::string& request_origin,
                             const std::string& key_name,
                             AttestationKeyType key_type,
                             CertificateCallback callback,
-      base::Optional<bool> result);
+                            const ::attestation::GetKeyInfoReply& reply);
 
   // Called when the attestation daemon has finished creating a certificate
   // request for the Privacy CA.  The request is asynchronously forwarded as-is
@@ -282,18 +280,6 @@ class COMPONENT_EXPORT(CHROMEOS_ATTESTATION) AttestationFlow {
                              bool success,
                              const std::string& data);
 
-  // Gets an existing certificate from the attestation daemon.
-  //
-  // Parameters
-  //   key_type - The type of the key for which a certificate is requested.
-  //   account_id - Identifies the active user.
-  //   key_name - The name of the key for which a certificate is requested.
-  //   callback - Called when the operation completes.
-  void GetExistingCertificate(AttestationKeyType key_type,
-                              const AccountId& account_id,
-                              const std::string& key_name,
-                              CertificateCallback callback);
-
   cryptohome::AsyncMethodCaller* async_caller_;
   CryptohomeClient* cryptohome_client_;
   AttestationClient* attestation_client_;

chromeos/attestation/attestation_flow_unittest.cc

@@ -881,8 +881,10 @@ TEST_F(AttestationFlowTest, GetCertificate_AlreadyExists) {
   StrictMock<cryptohome::MockAsyncMethodCaller> async_caller;
 
   chromeos::FakeCryptohomeClient client;
-  client.SetTpmAttestationUserCertificate(cryptohome::AccountIdentifier(),
-                                          kEnterpriseUserKey, "fake_cert");
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->GetMutableKeyInfoReply("", kEnterpriseUserKey)
+      ->set_certificate("fake_cert");
 
   // We're not expecting any server calls in this case; StrictMock will verify.
   std::unique_ptr<MockServerProxy> proxy(new StrictMock<MockServerProxy>());