Index: chromeos/attestation/attestation_flow.cc |
diff --git a/chromeos/attestation/attestation_flow.cc b/chromeos/attestation/attestation_flow.cc |
index c82cbe554008bf6f81cde6267570743de5e8c3b5..643dfee7e95006bf26067f7d58e80e23c30b6bd5 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,9 @@ namespace attestation { |
namespace { |
+// Delay before checking whether attestation has been prepared again. |
+constexpr base::TimeDelta kRetryDelay = base::TimeDelta::FromSeconds(7); |
+ |
// Redirects to one of three callbacks based on a boolean value and dbus call |
// status. |
// |
@@ -29,10 +36,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; |
@@ -59,6 +67,18 @@ void DBusDataMethodCallback( |
} // namespace |
+struct AttestationFlow::RetryData { |
+ RetryData(base::TimeDelta timeout, base::TickClock* tick_clock) |
+ : timeout(timeout), |
+ tick_clock(tick_clock), |
+ retry_timer(tick_clock), |
+ begin(tick_clock->NowTicks()) {} |
+ base::TimeDelta timeout; |
+ base::TickClock* tick_clock; |
+ base::OneShotTimer retry_timer; |
+ base::TimeTicks begin; |
+}; |
+ |
AttestationKeyType AttestationFlow::GetKeyTypeForProfile( |
AttestationCertificateProfile certificate_profile) { |
switch (certificate_profile) { |
@@ -91,10 +111,12 @@ std::string AttestationFlow::GetKeyNameForProfile( |
AttestationFlow::AttestationFlow(cryptohome::AsyncMethodCaller* async_caller, |
CryptohomeClient* cryptohome_client, |
- std::unique_ptr<ServerProxy> server_proxy) |
+ std::unique_ptr<ServerProxy> server_proxy, |
+ base::TimeDelta preparedness_timeout) |
: async_caller_(async_caller), |
cryptohome_client_(cryptohome_client), |
server_proxy_(std::move(server_proxy)), |
+ preparedness_timeout_(preparedness_timeout), |
weak_factory_(this) {} |
AttestationFlow::~AttestationFlow() { |
@@ -112,15 +134,46 @@ void AttestationFlow::GetCertificate( |
&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 = |
achuithb
2016/12/05 19:53:59
const.
on_enroll_failure too
|
+ 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; |
+} |
+ |
+void AttestationFlow::InitiateEnroll(const base::Closure& on_failure, |
+ const base::Closure& next_task) { |
+ TryInitiateEnroll(new RetryData(preparedness_timeout_, tick_clock_), |
achuithb
2016/12/05 19:53:59
YOu should pass ownership with a unique_ptr.
Shou
|
+ on_failure, next_task); |
+} |
+ |
+void AttestationFlow::TryInitiateEnroll(RetryData* retry_data, |
+ const base::Closure& on_failure, |
+ const base::Closure& next_task) { |
+ base::Closure do_enroll = base::Bind( |
achuithb
2016/12/05 19:53:59
const here and below
|
+ &AttestationFlow::DoneRetrying, weak_factory_.GetWeakPtr(), retry_data, |
+ base::Bind(&AttestationFlow::StartEnroll, weak_factory_.GetWeakPtr(), |
+ on_failure, next_task)); |
+ base::Closure retry_initiate_enroll = base::Bind( |
+ &AttestationFlow::StillRetrying, weak_factory_.GetWeakPtr(), retry_data, |
+ base::Bind(&AttestationFlow::DoneRetrying, weak_factory_.GetWeakPtr(), |
+ retry_data, on_failure), |
+ base::Bind(&AttestationFlow::TryInitiateEnroll, |
+ weak_factory_.GetWeakPtr(), retry_data, on_failure, |
+ next_task)); |
+ base::Closure do_fail = |
+ base::Bind(&AttestationFlow::DoneRetrying, weak_factory_.GetWeakPtr(), |
+ retry_data, 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, |
@@ -222,7 +275,8 @@ void AttestationFlow::StartCertificateRequest( |
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 +331,31 @@ void AttestationFlow::GetExistingCertificate( |
base::Bind(&DBusDataMethodCallback, callback)); |
} |
+void AttestationFlow::StillRetrying(RetryData* retry_data, |
+ const base::Closure& on_giving_up, |
+ const base::Closure& on_retrying) { |
+ base::TimeTicks now = retry_data->tick_clock->NowTicks(); |
achuithb
2016/12/05 19:53:59
const
|
+ base::TimeDelta elapsed = now - retry_data->begin; |
achuithb
2016/12/05 19:53:59
const
|
+ 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." |
achuithb
2016/12/05 19:53:59
Should this be a LOG(WARNING)? Maybe VLOG(1)?
|
+ << " Retrying in " << kRetryDelay << "."; |
+ retry_data->retry_timer.Start(FROM_HERE, kRetryDelay, on_retrying); |
+ } |
+} |
+ |
+void AttestationFlow::DoneRetrying(RetryData* retry_data, |
achuithb
2016/12/05 19:53:59
We don's pass naked pointers around like this with
|
+ const base::Closure& continuation) { |
+ retry_data->retry_timer.Stop(); |
+ delete retry_data; |
apronin1
2016/12/05 19:25:50
Not a big fan of deleting using the pointer passed
|
+ if (!continuation.is_null()) |
+ continuation.Run(); |
+} |
+ |
ServerProxy::~ServerProxy() {} |
PrivacyCAType ServerProxy::GetType() { |