use AttestationClient to preparations in PlatformVerificationFlow

We are deprecating attestation methods by CryptohomeClient.

BUG=b:158955123
TEST=unit_tests and browser_tests.

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

chrome/browser/chromeos/BUILD.gn

@@ -3853,6 +3853,8 @@ source_set("unit_tests") {
     "//chromeos/cryptohome:test_support",
     "//chromeos/dbus:lorgnette_proto",
     "//chromeos/dbus:test_support",
+    "//chromeos/dbus/attestation",
+    "//chromeos/dbus/attestation:attestation_proto",
     "//chromeos/dbus/authpolicy",
     "//chromeos/dbus/cryptohome",
     "//chromeos/dbus/cryptohome:attestation_proto",

chrome/browser/chromeos/attestation/attestation_policy_browsertest.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chromeos/dbus/attestation/fake_attestation_client.h"
 #include "chromeos/dbus/cryptohome/fake_cryptohome_client.h"
 #include "components/policy/proto/chrome_device_policy.pb.h"
 #include "content/public/test/browser_test.h"
@@ -64,7 +65,8 @@ class AttestationDevicePolicyTest
   PlatformVerificationFlow::Result SyncContentProtectionAttestation() {
     scoped_refptr<PlatformVerificationFlow> verifier(
         new PlatformVerificationFlow(
-            nullptr, nullptr, chromeos::FakeCryptohomeClient::Get(), nullptr));
+            nullptr, nullptr, chromeos::FakeCryptohomeClient::Get(),
+            chromeos::FakeAttestationClient::Get(), nullptr));
     verifier->ChallengePlatformKey(
         browser()->tab_strip_model()->GetActiveWebContents(), "fake_service_id",
         "fake_challenge",

chrome/browser/chromeos/attestation/platform_verification_flow.cc

@@ -23,6 +23,8 @@
 #include "chromeos/cryptohome/async_method_caller.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
 #include "chromeos/dbus/attestation/attestation.pb.h"
+#include "chromeos/dbus/attestation/attestation_client.h"
+#include "chromeos/dbus/attestation/interface.pb.h"
 #include "chromeos/dbus/cryptohome/cryptohome_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
@@ -52,22 +54,6 @@ const char kAttestationAvailableHistogram[] =
     "ChromeOS.PlatformVerification.Available";
 const int kOpportunisticRenewalThresholdInDays = 30;
 
-// A callback method to handle DBus errors.
-// `on_success` and `on_failure` are called mutally exclusively,
-// and `context` is moved into the chosen callback.
-template <typename ContextT>
-void DBusCallback(base::OnceCallback<void(ContextT, bool)> on_success,
-                  base::OnceCallback<void(ContextT)> on_failure,
-                  ContextT context,
-                  base::Optional<bool> result) {
-  if (result.has_value()) {
-    std::move(on_success).Run(std::move(context), result.value());
-  } else {
-    LOG(ERROR) << "PlatformVerificationFlow: DBus call failed!";
-    std::move(on_failure).Run(std::move(context));
-  }
-}
-
 // A helper to call a ChallengeCallback with an error result.
 void ReportError(
     PlatformVerificationFlow::ChallengeCallback callback,
@@ -153,6 +139,7 @@ PlatformVerificationFlow::PlatformVerificationFlow()
     : attestation_flow_(NULL),
       async_caller_(cryptohome::AsyncMethodCaller::GetInstance()),
       cryptohome_client_(CryptohomeClient::Get()),
+      attestation_client_(AttestationClient::Get()),
       delegate_(NULL),
       timeout_delay_(base::TimeDelta::FromSeconds(kTimeoutInSeconds)) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -168,10 +155,12 @@ PlatformVerificationFlow::PlatformVerificationFlow(
     AttestationFlow* attestation_flow,
     cryptohome::AsyncMethodCaller* async_caller,
     CryptohomeClient* cryptohome_client,
+    AttestationClient* attestation_client,
     Delegate* delegate)
     : attestation_flow_(attestation_flow),
       async_caller_(async_caller),
       cryptohome_client_(cryptohome_client),
+      attestation_client_(attestation_client),
       delegate_(delegate),
       timeout_delay_(base::TimeDelta::FromSeconds(kTimeoutInSeconds)) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -227,18 +216,23 @@ void PlatformVerificationFlow::ChallengePlatformKey(
                            std::move(callback));
 
   // Check if the device has been prepared to use attestation.
-  cryptohome_client_->TpmAttestationIsPrepared(base::BindOnce(
-      &DBusCallback<ChallengeContext>,
-      base::Bind(&PlatformVerificationFlow::OnAttestationPrepared, this),
-      base::Bind([](ChallengeContext context) {
-        ReportError(std::move(context).callback, INTERNAL_ERROR);
-      }),
-      std::move(context)));
+  ::attestation::GetEnrollmentPreparationsRequest request;
+  attestation_client_->GetEnrollmentPreparations(
+      request, base::BindOnce(&PlatformVerificationFlow::OnAttestationPrepared,
+                              this, std::move(context)));
 }
 
 void PlatformVerificationFlow::OnAttestationPrepared(
     ChallengeContext context,
-    bool attestation_prepared) {
+    const ::attestation::GetEnrollmentPreparationsReply& reply) {
+  if (reply.status() != ::attestation::STATUS_SUCCESS) {
+    LOG(ERROR)
+        << "Platform verification failed to check if attestation is prepared.";
+    ReportError(std::move(context).callback, INTERNAL_ERROR);
+    return;
+  }
+  const bool attestation_prepared =
+      AttestationClient::IsAttestationPrepared(reply);
   UMA_HISTOGRAM_BOOLEAN(kAttestationAvailableHistogram, attestation_prepared);
 
   if (!attestation_prepared) {

chrome/browser/chromeos/attestation/platform_verification_flow.h

@@ -14,6 +14,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "chromeos/dbus/attestation/interface.pb.h"
 #include "chromeos/dbus/constants/attestation_constants.h"
 #include "url/gurl.h"
 
@@ -33,6 +34,7 @@ class User;
 
 namespace chromeos {
 
+class AttestationClient;
 class CryptohomeClient;
 
 namespace attestation {
@@ -130,6 +132,7 @@ class PlatformVerificationFlow
   PlatformVerificationFlow(AttestationFlow* attestation_flow,
                            cryptohome::AsyncMethodCaller* async_caller,
                            CryptohomeClient* cryptohome_client,
+                           AttestationClient* attestation_client,
                            Delegate* delegate);
 
   // Invokes an asynchronous operation to challenge a platform key.  Any user
@@ -174,10 +177,10 @@ class PlatformVerificationFlow
   ~PlatformVerificationFlow();
 
   // Callback for attestation preparation. The arguments to ChallengePlatformKey
-  // are in |context|, and |attestation_prepared| specifies whether attestation
-  // has been prepared on this device.
-  void OnAttestationPrepared(ChallengeContext context,
-                             bool attestation_prepared);
+  // are in |context|, and |reply| is the result of |GetEnrollmentPreparations|.
+  void OnAttestationPrepared(
+      ChallengeContext context,
+      const ::attestation::GetEnrollmentPreparationsReply& reply);
 
   // Initiates the flow to get a platform key certificate.  The arguments to
   // ChallengePlatformKey are in |context|.  |account_id| identifies the user
@@ -193,11 +196,11 @@ class PlatformVerificationFlow
   // completes.  The arguments to ChallengePlatformKey are in |context|.
   // |account_id| identifies the user for which the certificate was requested.
   // |operation_success| is true iff the certificate request operation
-  // succeeded.  |certificate_chain| holds the certificate for the platform key
-  // on success.  If the certificate request was successful, this method invokes
-  // a request to sign the challenge.  If the operation timed out prior to this
-  // method being called, this method does nothing - notably, the callback is
-  // not invoked.
+  // succeeded.  |certificate_chain| holds the certificate for the platform
+  // key on success.  If the certificate request was successful, this method
+  // invokes a request to sign the challenge.  If the operation timed out
+  // prior to this method being called, this method does nothing - notably,
+  // the callback is not invoked.
   void OnCertificateReady(
       scoped_refptr<base::RefCountedData<ChallengeContext>> context,
       const AccountId& account_id,
@@ -243,7 +246,8 @@ class PlatformVerificationFlow
   AttestationFlow* attestation_flow_;
   std::unique_ptr<AttestationFlow> default_attestation_flow_;
   cryptohome::AsyncMethodCaller* async_caller_;
-  CryptohomeClient* cryptohome_client_;
+  CryptohomeClient* const cryptohome_client_;
+  AttestationClient* const attestation_client_;
   Delegate* delegate_;
   std::unique_ptr<Delegate> default_delegate_;
   base::TimeDelta timeout_delay_;

chrome/browser/chromeos/attestation/platform_verification_flow_unittest.cc

@@ -20,7 +20,8 @@
 #include "chromeos/attestation/mock_attestation_flow.h"
 #include "chromeos/cryptohome/mock_async_method_caller.h"
 #include "chromeos/dbus/attestation/attestation.pb.h"
-#include "chromeos/dbus/cryptohome/fake_cryptohome_client.h"
+#include "chromeos/dbus/attestation/fake_attestation_client.h"
+#include "chromeos/dbus/attestation/interface.pb.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -102,14 +103,21 @@ class PlatformVerificationFlowTest : public ::testing::Test {
       : certificate_status_(ATTESTATION_SUCCESS),
         fake_certificate_index_(0),
         sign_challenge_success_(true),
-        result_(PlatformVerificationFlow::INTERNAL_ERROR) {}
+        result_(PlatformVerificationFlow::INTERNAL_ERROR) {
+    ::chromeos::AttestationClient::InitializeFake();
+  }
+  ~PlatformVerificationFlowTest() override {
+    ::chromeos::AttestationClient::Shutdown();
+  }
 
   void SetUp() {
     // Create a verifier for tests to call.
-    verifier_ = new PlatformVerificationFlow(&mock_attestation_flow_,
-                                             &mock_async_caller_,
-                                             &fake_cryptohome_client_,
-                                             &fake_delegate_);
+    // We don't need cryptohome_client in unittests because it is only needed
+    // for real AttestationFlow and in unittests we uses mocked one.
+    verifier_ = new PlatformVerificationFlow(
+        &mock_attestation_flow_, &mock_async_caller_,
+        /*cryptohome_client=*/nullptr, AttestationClient::Get(),
+        &fake_delegate_);
 
     // Create callbacks for tests to use with verifier_.
     settings_helper_.ReplaceDeviceSettingsProviderWithStub();
@@ -184,7 +192,6 @@ class PlatformVerificationFlowTest : public ::testing::Test {
   content::BrowserTaskEnvironment task_environment_;
   StrictMock<MockAttestationFlow> mock_attestation_flow_;
   cryptohome::MockAsyncMethodCaller mock_async_caller_;
-  chromeos::FakeCryptohomeClient fake_cryptohome_client_;
   FakeDelegate fake_delegate_;
   ScopedCrosSettingsTestHelper settings_helper_;
   scoped_refptr<PlatformVerificationFlow> verifier_;
@@ -259,7 +266,20 @@ TEST_F(PlatformVerificationFlowTest, ChallengeSigningError) {
 }
 
 TEST_F(PlatformVerificationFlowTest, DBusFailure) {
-  fake_cryptohome_client_.SetServiceIsAvailable(false);
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->ConfigureEnrollmentPreparationsStatus(::attestation::STATUS_DBUS_ERROR);
+  verifier_->ChallengePlatformKey(nullptr, kTestID, kTestChallenge,
+                                  CreateChallengeCallback());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(PlatformVerificationFlow::INTERNAL_ERROR, result_);
+}
+
+TEST_F(PlatformVerificationFlowTest, AttestationServiceInternalError) {
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->ConfigureEnrollmentPreparationsStatus(
+          ::attestation::STATUS_UNEXPECTED_DEVICE_ERROR);
   verifier_->ChallengePlatformKey(nullptr, kTestID, kTestChallenge,
                                   CreateChallengeCallback());
   base::RunLoop().RunUntilIdle();
@@ -380,8 +400,9 @@ TEST_F(PlatformVerificationFlowTest, UnsupportedMode) {
 }
 
 TEST_F(PlatformVerificationFlowTest, AttestationNotPrepared) {
-  fake_cryptohome_client_.set_tpm_attestation_is_enrolled(false);
-  fake_cryptohome_client_.set_tpm_attestation_is_prepared(false);
+  chromeos::AttestationClient::Get()
+      ->GetTestInterface()
+      ->ConfigureEnrollmentPreparations(false);
   verifier_->ChallengePlatformKey(nullptr, kTestID, kTestChallenge,
                                   CreateChallengeCallback());
   base::RunLoop().RunUntilIdle();