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

chromeos/attestation/attestation_flow.cc

Index: chromeos/attestation/attestation_flow.cc
diff --git a/chromeos/attestation/attestation_flow.cc b/chromeos/attestation/attestation_flow.cc
index c82cbe554008bf6f81cde6267570743de5e8c3b5..e11cbe9ce781de3e80533a66b8a0048057bda5f8 100644
--- a/chromeos/attestation/attestation_flow.cc
+++ b/chromeos/attestation/attestation_flow.cc
@@ -4,9 +4,13 @@
 
 #include "chromeos/attestation/attestation_flow.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/time/tick_clock.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"
@@ -17,6 +21,13 @@ namespace attestation {
 
 namespace {
 
+// A reasonable preparedness timeout that gives enough time for attestation
+// to be prepared, yet does not make the caller wait too long.
+base::TimeDelta kDefaultPreparednessTimeout = base::TimeDelta::FromMinutes(3);

[Darren Krahn, 2016/12/06 17:31:25]

POD-type globals only please

+
+// Delay before checking whether attestation has been prepared again.
+constexpr base::TimeDelta kRetryDelay = base::TimeDelta::FromSeconds(9);

[Darren Krahn, 2016/12/06 17:31:25]

same here, POD-type globals only

+
 // Redirects to one of three callbacks based on a boolean value and dbus call
 // status.
 //
@@ -29,10 +40,11 @@ namespace {
 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;
@@ -95,6 +107,7 @@ AttestationFlow::AttestationFlow(cryptohome::AsyncMethodCaller* async_caller,
     : async_caller_(async_caller),
       cryptohome_client_(cryptohome_client),
       server_proxy_(std::move(server_proxy)),
+      preparedness_timeout_(kDefaultPreparednessTimeout),
       weak_factory_(this) {}
 
 AttestationFlow::~AttestationFlow() {
@@ -108,19 +121,74 @@ void AttestationFlow::GetCertificate(
     const CertificateCallback& callback) {
   // 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(
+  const 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));
+  const base::Closure on_enroll_failure = base::Bind(callback, false, "");
+  const 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::SetTickClockForTest(base::TickClock* tick_clock) {
+  tick_clock_ = tick_clock;
+}
+
+struct AttestationFlow::RetryData {
+ public:
+  RetryData(base::TimeDelta timeout, base::TickClock* tick_clock)
+      : timeout(timeout), retry_timer(tick_clock), tick_clock_(tick_clock) {
+    begin = Now();
+  }
+  ~RetryData() { retry_timer.Stop(); }
+
+  base::TimeTicks Now() {
+    return tick_clock_ ? tick_clock_->NowTicks() : base::TimeTicks::Now();
+  }
+
+  base::TimeDelta timeout;
+  base::OneShotTimer retry_timer;
+  base::TimeTicks begin;
+
+ private:
+  base::TickClock* tick_clock_;
+};
+
+void AttestationFlow::InitiateEnroll(const base::Closure& on_failure,
+                                     const base::Closure& next_task) {
+  RetryId retry_id = CreateRetryData();
+  if (retry_id == kInvalidRetryId) {
+    if (!on_failure.is_null())
+      on_failure.Run();
+    return;
+  }
+  TryInitiateEnroll(retry_id, on_failure, next_task);
+}
+
+void AttestationFlow::TryInitiateEnroll(const RetryId retry_id,
+                                        const base::Closure& on_failure,
+                                        const base::Closure& next_task) {
+  const base::Closure do_enroll = base::Bind(
+      &AttestationFlow::DoneRetrying, weak_factory_.GetWeakPtr(), retry_id,
+      base::Bind(&AttestationFlow::StartEnroll, weak_factory_.GetWeakPtr(),
+                 on_failure, next_task));
+  const base::Closure retry_initiate_enroll = base::Bind(
+      &AttestationFlow::StillRetrying, weak_factory_.GetWeakPtr(), retry_id,
+      base::Bind(&AttestationFlow::DoneRetrying, weak_factory_.GetWeakPtr(),
+                 retry_id, on_failure),
+      base::Bind(&AttestationFlow::TryInitiateEnroll,
+                 weak_factory_.GetWeakPtr(), retry_id, on_failure, next_task));
+  base::Closure do_fail =
+      base::Bind(&AttestationFlow::DoneRetrying, weak_factory_.GetWeakPtr(),
+                 retry_id, on_failure);
+  cryptohome_client_->TpmAttestationIsPrepared(
+      base::Bind(&DBusBoolRedirectCallback, do_enroll, retry_initiate_enroll,
+                 do_fail, "check for attestation readiness"));
 }
 
 void AttestationFlow::StartEnroll(const base::Closure& on_failure,
@@ -211,18 +279,19 @@ void AttestationFlow::StartCertificateRequest(
                    callback));
   } else {
     // If the key already exists, query the existing certificate.
-    base::Closure on_key_exists = base::Bind(
+    const base::Closure on_key_exists = base::Bind(
         &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(
+    const 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"));
   }
 }
 
@@ -277,6 +346,51 @@ void AttestationFlow::GetExistingCertificate(
       base::Bind(&DBusDataMethodCallback, callback));
 }
 
+AttestationFlow::RetryId AttestationFlow::CreateRetryData() {
+  if (next_retry_id_ < 0)
+    next_retry_id_ = 0;
+  RetryId starting_retry_id = next_retry_id_;
+  while (retries_.find(next_retry_id_) != retries_.end()) {
+    if (++next_retry_id_ < 0)
+      next_retry_id_ = 0;
+    if (next_retry_id_ == starting_retry_id)
+      return kInvalidRetryId;
+  }
+  std::unique_ptr<RetryData> data =
+      base::MakeUnique<RetryData>(preparedness_timeout_, tick_clock_);
+  retries_.insert(std::make_pair(next_retry_id_, std::move(data)));
+  return next_retry_id_++;
+}
+
+AttestationFlow::RetryData& AttestationFlow::GetRetryData(RetryId retry_id) {
+  return *retries_.find(retry_id)->second;
+}
+
+void AttestationFlow::StillRetrying(const RetryId retry_id,
+                                    const base::Closure& on_giving_up,
+                                    const base::Closure& on_retrying) {
+  RetryData& retry_data = GetRetryData(retry_id);
+  base::TimeTicks now = retry_data.Now();
+  base::TimeDelta elapsed = now - retry_data.begin;
+  if (elapsed + kRetryDelay > retry_data.timeout) {
+    LOG(ERROR) << "Attestation: Not prepared."
+               << " Giving up on retrying after " << elapsed << ".";
+    if (!on_giving_up.is_null())
+      on_giving_up.Run();
+  } else {
+    LOG(WARNING) << "Attestation: Not prepared yet."
+                 << " Retrying in " << kRetryDelay << ".";
+    retry_data.retry_timer.Start(FROM_HERE, kRetryDelay, on_retrying);
+  }
+}
+
+void AttestationFlow::DoneRetrying(RetryId retry_id,
+                                   const base::Closure& continuation) {
+  retries_.erase(retry_id);
+  if (!continuation.is_null())
+    continuation.Run();
+}
+
 ServerProxy::~ServerProxy() {}
 
 PrivacyCAType ServerProxy::GetType() {