Added a timeout for platform verification key generation.

chrome/browser/chromeos/attestation/platform_verification_flow.cc

Index: chrome/browser/chromeos/attestation/platform_verification_flow.cc
diff --git a/chrome/browser/chromeos/attestation/platform_verification_flow.cc b/chrome/browser/chromeos/attestation/platform_verification_flow.cc
index d6ef3ace1ee2847e7048b4176e3ec1fea8cf49fb..f848a48765030fe69e4d070b15e125f95d1caa63 100644
--- a/chrome/browser/chromeos/attestation/platform_verification_flow.cc
+++ b/chrome/browser/chromeos/attestation/platform_verification_flow.cc
@@ -6,7 +6,9 @@
 
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_service.h"
+#include "base/time/time.h"
 #include "chrome/browser/chromeos/attestation/attestation_ca_client.h"
 #include "chrome/browser/chromeos/attestation/attestation_signed_data.pb.h"
 #include "chrome/browser/chromeos/attestation/platform_verification_dialog.h"
@@ -29,6 +31,8 @@
 
 namespace {
 
+const int kTimeoutInSeconds = 8;
+
 // A callback method to handle DBus errors.
 void DBusCallback(const base::Callback<void(bool)>& on_success,
                   const base::Closure& on_failure,
@@ -79,7 +83,7 @@ PlatformVerificationFlow::PlatformVerificationFlow()
       user_manager_(UserManager::Get()),
       delegate_(NULL),
       testing_prefs_(NULL),
-      weak_factory_(this) {
+      timeout_delay_(kTimeoutInSeconds) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   scoped_ptr<ServerProxy> attestation_ca_client(new AttestationCAClient());
   default_attestation_flow_.reset(new AttestationFlow(
@@ -103,7 +107,7 @@ PlatformVerificationFlow::PlatformVerificationFlow(
       user_manager_(user_manager),
       delegate_(delegate),
       testing_prefs_(NULL),
-      weak_factory_(this) {
+      timeout_delay_(kTimeoutInSeconds) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 }
 
@@ -121,42 +125,32 @@ void PlatformVerificationFlow::ChallengePlatformKey(
     ReportError(callback, POLICY_REJECTED);
     return;
   }
+  ChallengeContext context = {web_contents, service_id, challenge, callback};
   BoolDBusMethodCallback dbus_callback = base::Bind(
       &DBusCallback,
-      base::Bind(&PlatformVerificationFlow::CheckConsent,
-                 weak_factory_.GetWeakPtr(),
-                 web_contents,
-                 service_id,
-                 challenge,
-                 callback),
+      base::Bind(&PlatformVerificationFlow::CheckConsent, this, context),
       base::Bind(&ReportError, callback, INTERNAL_ERROR));
   cryptohome_client_->TpmAttestationIsEnrolled(dbus_callback);
 }
 
-void PlatformVerificationFlow::CheckConsent(content::WebContents* web_contents,
-                                            const std::string& service_id,
-                                            const std::string& challenge,
-                                            const ChallengeCallback& callback,
+void PlatformVerificationFlow::CheckConsent(const ChallengeContext& context,
                                             bool attestation_enrolled) {
   ConsentType consent_type = CONSENT_TYPE_NONE;
-  if (!attestation_enrolled || IsFirstUse(web_contents)) {
+  if (!attestation_enrolled || IsFirstUse(context.web_contents)) {
     consent_type = CONSENT_TYPE_ATTESTATION;
-  } else if (IsAlwaysAskRequired(web_contents)) {
+  } else if (IsAlwaysAskRequired(context.web_contents)) {
     consent_type = CONSENT_TYPE_ALWAYS;
   }
   Delegate::ConsentCallback consent_callback = base::Bind(
       &PlatformVerificationFlow::OnConsentResponse,
-      weak_factory_.GetWeakPtr(),
-      web_contents,
-      service_id,
-      challenge,
-      callback,
+      this,
+      context,
       consent_type);
   if (consent_type == CONSENT_TYPE_NONE) {
     consent_callback.Run(CONSENT_RESPONSE_NONE);
   } else {
     delegate_->ShowConsentPrompt(consent_type,
-                                 web_contents,
+                                 context.web_contents,
                                  consent_callback);
   }
 }
@@ -175,28 +169,25 @@ void PlatformVerificationFlow::RegisterProfilePrefs(
 }
 
 void PlatformVerificationFlow::OnConsentResponse(
-    content::WebContents* web_contents,
-    const std::string& service_id,
-    const std::string& challenge,
-    const ChallengeCallback& callback,
+    const ChallengeContext& context,
     ConsentType consent_type,
     ConsentResponse consent_response) {
   if (consent_type != CONSENT_TYPE_NONE) {
     if (consent_response == CONSENT_RESPONSE_NONE) {
       // No user response - do not proceed and do not modify any settings.
       LOG(WARNING) << "PlatformVerificationFlow: No response from user.";
-      ReportError(callback, USER_REJECTED);
+      ReportError(context.callback, USER_REJECTED);
       return;
     }
-    if (!UpdateSettings(web_contents, consent_type, consent_response)) {
-      ReportError(callback, INTERNAL_ERROR);
+    if (!UpdateSettings(context.web_contents, consent_type, consent_response)) {
+      ReportError(context.callback, INTERNAL_ERROR);
       return;
     }
     if (consent_response == CONSENT_RESPONSE_DENY) {
       LOG(INFO) << "PlatformVerificationFlow: User rejected request.";
       content::RecordAction(
           content::UserMetricsAction("PlatformVerificationRejected"));
-      ReportError(callback, USER_REJECTED);
+      ReportError(context.callback, USER_REJECTED);
       return;
     } else if (consent_response == CONSENT_RESPONSE_ALLOW) {
       content::RecordAction(
@@ -206,75 +197,95 @@ void PlatformVerificationFlow::OnConsentResponse(
 
   // At this point all user interaction is complete and we can proceed with the
   // certificate request.
-  chromeos::User* user = GetUser(web_contents);
+  chromeos::User* user = GetUser(context.web_contents);
   if (!user) {
-    ReportError(callback, INTERNAL_ERROR);
+    ReportError(context.callback, INTERNAL_ERROR);
     LOG(ERROR) << "Profile does not map to a valid user.";
     return;
   }
+  scoped_refptr<TimeoutStatus> timeout_status(new TimeoutStatus);

[DaleCurtis, 2013/10/30 20:11:52]

TimeoutStatus()

+  base::MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&PlatformVerificationFlow::OnCertificateTimeout, this,
+                 context, timeout_status),
+      base::TimeDelta::FromSeconds(timeout_delay_));
   AttestationFlow::CertificateCallback certificate_callback = base::Bind(
       &PlatformVerificationFlow::OnCertificateReady,
-      weak_factory_.GetWeakPtr(),
+      this,
+      context,
       user->email(),
-      service_id,
-      challenge,
-      callback);
+      timeout_status);
   attestation_flow_->GetCertificate(
       PROFILE_CONTENT_PROTECTION_CERTIFICATE,
       user->email(),
-      service_id,
+      context.service_id,
       false,  // Don't force a new key.
       certificate_callback);
 }
 
 void PlatformVerificationFlow::OnCertificateReady(
+    const ChallengeContext& context,
     const std::string& user_id,
-    const std::string& service_id,
-    const std::string& challenge,
-    const ChallengeCallback& callback,
+    TimeoutStatus* timeout_status,

[DaleCurtis, 2013/10/30 20:11:52]

Usage of raw pointers with scoped_refptr seems questionable.  scoped_refptr<>
instead?

[Darren Krahn, 2013/10/30 22:24:53]

Now using scoped_ptr but this can be done with scoped_refptr to save the extra
AddRef/Release.  See
http://www.chromium.org/developers/design-documents/threading

     bool operation_success,
     const std::string& certificate) {
   if (!operation_success) {
     LOG(WARNING) << "PlatformVerificationFlow: Failed to certify platform.";
-    ReportError(callback, PLATFORM_NOT_VERIFIED);
+  }
+  if (timeout_status->timed_out) {
+    LOG(WARNING) << "PlatformVerificationFlow: Certificate ready but call has "
+                 << "already timed out.";
+    return;
+  }
+  timeout_status->finished = true;
+  if (!operation_success) {
+    ReportError(context.callback, PLATFORM_NOT_VERIFIED);
     return;
   }
   cryptohome::AsyncMethodCaller::DataCallback cryptohome_callback = base::Bind(
       &PlatformVerificationFlow::OnChallengeReady,
-      weak_factory_.GetWeakPtr(),
-      certificate,
-      challenge,
-      callback);
+      this,
+      context,
+      certificate);
   std::string key_name = kContentProtectionKeyPrefix;
-  key_name += service_id;
+  key_name += context.service_id;
   async_caller_->TpmAttestationSignSimpleChallenge(KEY_USER,
                                                    user_id,
                                                    key_name,
-                                                   challenge,
+                                                   context.challenge,
                                                    cryptohome_callback);
 }
 
+void PlatformVerificationFlow::OnCertificateTimeout(
+    const ChallengeContext& context,
+    TimeoutStatus* timeout_status) {
+  if (!timeout_status->finished) {
+    LOG(WARNING) << "PlatformVerificationFlow: Timing out.";
+    timeout_status->timed_out = true;
+    ReportError(context.callback, TIMEOUT);
+  }
+}
+
 void PlatformVerificationFlow::OnChallengeReady(
+    const ChallengeContext& context,
     const std::string& certificate,
-    const std::string& challenge,
-    const ChallengeCallback& callback,
     bool operation_success,
     const std::string& response_data) {
   if (!operation_success) {
     LOG(ERROR) << "PlatformVerificationFlow: Failed to sign challenge.";
-    ReportError(callback, INTERNAL_ERROR);
+    ReportError(context.callback, INTERNAL_ERROR);
     return;
   }
   SignedData signed_data_pb;
   if (response_data.empty() || !signed_data_pb.ParseFromString(response_data)) {
     LOG(ERROR) << "PlatformVerificationFlow: Failed to parse response data.";
-    ReportError(callback, INTERNAL_ERROR);
+    ReportError(context.callback, INTERNAL_ERROR);
     return;
   }
-  callback.Run(SUCCESS,
-               signed_data_pb.data(),
-               signed_data_pb.signature(),
-               certificate);
+  context.callback.Run(SUCCESS,
+                       signed_data_pb.data(),
+                       signed_data_pb.signature(),
+                       certificate);
   LOG(INFO) << "PlatformVerificationFlow: Platform successfully verified.";
 }