Wait for the attestation to be ready (TPM being prepared for attestation) before trying to enroll.

chromeos/attestation/attestation_flow.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/attestation/attestation_flow.h"
 
 #include <utility>
 
 #include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/timer/timer.h"
 #include "chromeos/cryptohome/async_method_caller.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
 #include "chromeos/dbus/cryptohome_client.h"
 #include "components/signin/core/account_id/account_id.h"
 
 namespace chromeos {
 namespace attestation {
 
 namespace {
 
+// Default backoff policy.
+const net::BackoffEntry::Policy kDefaultBackoffPolicy = {
+    // Number of initial errors (in sequence) to ignore before applying
+    // exponential back-off rules.
+    0,
+
+    // Initial delay for exponential back-off in ms.
+    2000,
+
+    // Factor by which the waiting time will be multiplied.
+    2,
+
+    // Fuzzing percentage. ex: 10% will spread requests randomly
+    // between 90%-100% of the calculated time.
+    0,
+
+    // Maximum amount of time we are willing to delay our request in ms.
+    -1,
+
+    // Time to keep an entry from being discarded even when it
+    // has no significant state, -1 to never discard.
+    -1,
+
+    // Don't use initial delay unless the last request was an error.
+    false,
+};
+
 // Redirects to one of three callbacks based on a boolean value and dbus call
 // status.
 //
 // Parameters
 //   on_true - Called when status=succes and value=true.
 //   on_false - Called when status=success and value=false.
 //   on_fail - Called when status=failure.
 //   status - The D-Bus operation status.
 //   value - The value returned by the D-Bus operation.
 void DBusBoolRedirectCallback(const base::Closure& on_true,
                               const base::Closure& on_false,
                               const base::Closure& on_fail,
+                              const std::string& on_fail_message,
                               DBusMethodCallStatus status,
                               bool value) {
   if (status != DBUS_METHOD_CALL_SUCCESS) {
-    LOG(ERROR) << "Attestation: Failed to query enrollment state.";
+    LOG(ERROR) << "Attestation: Failed to " << on_fail_message << ".";
     if (!on_fail.is_null())
       on_fail.Run();
     return;
   }
   const base::Closure& task = value ? on_true : on_false;
   if (!task.is_null())
     task.Run();
 }
 
 void DBusDataMethodCallback(
@@ skipping 42 matching lines @@
   NOTREACHED();
   return "";
 }
 
 AttestationFlow::AttestationFlow(cryptohome::AsyncMethodCaller* async_caller,
                                  CryptohomeClient* cryptohome_client,
                                  std::unique_ptr<ServerProxy> server_proxy)
     : async_caller_(async_caller),
       cryptohome_client_(cryptohome_client),
       server_proxy_(std::move(server_proxy)),
+      retry_timer_(new base::OneShotTimer()),
+      retry_backoff_(&kDefaultBackoffPolicy),
       weak_factory_(this) {}
 
 AttestationFlow::~AttestationFlow() {
 }
 
 void AttestationFlow::GetCertificate(
     AttestationCertificateProfile certificate_profile,
     const AccountId& account_id,
     const std::string& request_origin,
     bool force_new_key,
     const CertificateCallback& callback) {
+  // Fail requests if we are backing off right now.
+  if (retry_backoff_.ShouldRejectRequest()) {

[The one and only Dr. Crash, 2016/12/02 05:57:49]

I actually think that the behavior I want is to stack the waiting callbacks.
There is no reason to go try ensure the first certificate request will succeed
by retrying, but then when we back off simply drop the certificate requests on
the floor. Also I am not sure that ShouldRejectRequest() will return true after
a first failure only (that doesn't seem right), so a second call to
GetCertificate() while a call is pending would mean the first call would never
have its callback called.

So instead I am thinking of managing a stack (vector) of the callbacks given to
GetCertificate(), push callbacks onto that stack and then process them in order
once we decide what to do.

What do you think?

+    if (!callback.is_null())
+      callback.Run(false, "");
+    return;
+  }
   // 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.
   base::Closure do_cert_request = base::Bind(
       &AttestationFlow::StartCertificateRequest, weak_factory_.GetWeakPtr(),
       certificate_profile, account_id, request_origin, force_new_key, callback);
   base::Closure on_enroll_failure = base::Bind(callback, false, "");
-  base::Closure do_enroll = base::Bind(&AttestationFlow::StartEnroll,
-                                       weak_factory_.GetWeakPtr(),
-                                       on_enroll_failure,
-                                       do_cert_request);
-  cryptohome_client_->TpmAttestationIsEnrolled(base::Bind(
-      &DBusBoolRedirectCallback,
-      do_cert_request,  // If enrolled, proceed with cert request.
-      do_enroll,        // If not enrolled, initiate enrollment.
-      on_enroll_failure));
+  base::Closure initiate_enroll =
+      base::Bind(&AttestationFlow::InitiateEnroll, weak_factory_.GetWeakPtr(),
+                 on_enroll_failure, do_cert_request);
+  cryptohome_client_->TpmAttestationIsEnrolled(
+      base::Bind(&DBusBoolRedirectCallback,
+                 do_cert_request,  // If enrolled, proceed with cert request.
+                 initiate_enroll,  // If not enrolled, initiate enrollment.
+                 on_enroll_failure, "check enrollment state"));
+}
+
+void AttestationFlow::SetRetryTimerForTest(
+    std::unique_ptr<base::OneShotTimer> retry_timer) {
+  retry_timer_ = std::move(retry_timer);
+}
+
+void AttestationFlow::InitiateEnroll(const base::Closure& on_failure,
+                                     const base::Closure& next_task) {
+  base::Closure do_enroll =
+      base::Bind(&AttestationFlow::StartEnroll, weak_factory_.GetWeakPtr(),
+                 on_failure, next_task);
+  base::Closure retry_initiate_enroll =
+      base::Bind(&AttestationFlow::RetryInitiateEnroll,
+                 weak_factory_.GetWeakPtr(), on_failure, next_task);
+  cryptohome_client_->TpmAttestationIsPrepared(
+      base::Bind(&DBusBoolRedirectCallback, do_enroll, retry_initiate_enroll,
+                 on_failure, "check for attestation readiness"));
+}
+
+void AttestationFlow::RetryInitiateEnroll(const base::Closure& on_failure,
+                                          const base::Closure& next_task) {
+  if (initiate_enroll_retries_ == 0) {
+    LOG(ERROR) << "Attestation: not ready for attestation."
+               << " Giving up on retrying.";
+    if (!on_failure.is_null())
+      on_failure.Run();
+  } else {
+    if (initiate_enroll_retries_ > 0)
+      --initiate_enroll_retries_;
+    retry_backoff_.InformOfRequest(false);
+    auto retry_delay = retry_backoff_.GetTimeUntilRelease().InMilliseconds();
+    LOG(WARNING) << "Attestation: not ready for attestation yet."
+                 << " Retrying in " << retry_delay << " ms.";
+    retry_timer_->Start(
+        FROM_HERE, retry_backoff_.GetTimeUntilRelease(),
+        base::Bind(&AttestationFlow::InitiateEnroll, weak_factory_.GetWeakPtr(),
+                   on_failure, next_task));
+  }
 }
 
 void AttestationFlow::StartEnroll(const base::Closure& on_failure,
                                   const base::Closure& next_task) {
+  // We are done retrying the enrollment initiation (i.e. TPM is ready).
+  retry_backoff_.Reset();
   // Get the attestation service to create a Privacy CA enrollment request.
   async_caller_->AsyncTpmAttestationCreateEnrollRequest(
       server_proxy_->GetType(),
       base::Bind(&AttestationFlow::SendEnrollRequestToPCA,
                  weak_factory_.GetWeakPtr(),
                  on_failure,
                  next_task));
 }
 
 void AttestationFlow::SendEnrollRequestToPCA(const base::Closure& on_failure,
@@ skipping 77 matching lines @@
         &AttestationFlow::GetExistingCertificate, weak_factory_.GetWeakPtr(),
         key_type, account_id, key_name, callback);
     // If the key does not exist, call this method back with |generate_new_key|
     // set to true.
     base::Closure on_key_not_exists = base::Bind(
         &AttestationFlow::StartCertificateRequest, weak_factory_.GetWeakPtr(),
         certificate_profile, account_id, request_origin, true, callback);
     cryptohome_client_->TpmAttestationDoesKeyExist(
         key_type, cryptohome::Identification(account_id), key_name,
         base::Bind(&DBusBoolRedirectCallback, on_key_exists, on_key_not_exists,
-                   base::Bind(callback, false, "")));
+                   base::Bind(callback, false, ""),
+                   "check for existence of attestation key"));
   }
 }
 
 void AttestationFlow::SendCertificateRequestToPCA(
     AttestationKeyType key_type,
     const AccountId& account_id,
     const std::string& key_name,
     const CertificateCallback& callback,
     bool success,
     const std::string& data) {
@@ skipping 42 matching lines @@
 }
 
 ServerProxy::~ServerProxy() {}
 
 PrivacyCAType ServerProxy::GetType() {
   return DEFAULT_PCA;
 }
 
 }  // namespace attestation
 }  // namespace chromeos