Added a timeout for platform verification key generation.

chrome/browser/chromeos/attestation/platform_verification_flow.h

 // Copyright 2013 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_ATTESTATION_PLATFORM_VERIFICATION_FLOW_H_
 #define CHROME_BROWSER_CHROMEOS_ATTESTATION_PLATFORM_VERIFICATION_FLOW_H_
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/memory/weak_ptr.h"
 #include "url/gurl.h"
 
 class PrefService;
 
 namespace content {
 class WebContents;
 }
 
 namespace cryptohome {
 class AsyncMethodCaller;
@@ skipping 12 matching lines @@
 namespace attestation {
 
 class AttestationFlow;
 
 // This class allows platform verification for the content protection use case.
 // All methods must only be called on the UI thread.  Example:
 //   PlatformVerificationFlow verifier;
 //   PlatformVerificationFlow::Callback callback = base::Bind(&MyCallback);
 //   verifier.ChallengePlatformKey(my_web_contents, "my_id", "some_challenge",
 //                                 callback);
-class PlatformVerificationFlow {
+class PlatformVerificationFlow
+    : public base::RefCountedThreadSafe<PlatformVerificationFlow> {

[pastarmovj, 2013/10/29 09:24:29]

From looking at this CL it is not obvious to me why do you need this class to be
RefCountedThreadSafe please put this explnation in the comment above this class
since this is a rather important detail IMO.

[Darren Krahn, 2013/10/29 17:23:04]

Done.  It is important -- please let me know if have a better way to solve this.

[pastarmovj, 2013/10/30 10:20:21]

Thanks this looks reasonable to me. I think your choice to use ref-counted is
reasonable. Especially the point that having this a singleton is not a good
choice because it isn't actually needed all the time so it doesn't have to stay
around all the time and pollute the global namespace/memory.

  public:
   enum Result {
     SUCCESS,                // The operation succeeded.
     INTERNAL_ERROR,         // The operation failed unexpectedly.
     PLATFORM_NOT_VERIFIED,  // The platform cannot be verified.  For example:
                             // - It is not a Chrome device.
                             // - It is not running a verified OS image.
     USER_REJECTED,          // The user explicitly rejected the operation.
     POLICY_REJECTED,        // The operation is not allowed by policy/settings.
+    TIMEOUT,                // The operation timed out.
   };
 
   enum ConsentType {
     CONSENT_TYPE_NONE,         // No consent necessary.
     CONSENT_TYPE_ATTESTATION,  // Consent to use attestation.
     CONSENT_TYPE_ALWAYS,       // Consent because 'Always Ask' was requested.
   };
 
   enum ConsentResponse {
     CONSENT_RESPONSE_NONE,
@@ skipping 38 matching lines @@
   PlatformVerificationFlow();
 
   // An alternate constructor which specifies dependent objects explicitly.
   // This is useful in testing.  The caller retains ownership of all pointers.
   PlatformVerificationFlow(AttestationFlow* attestation_flow,
                            cryptohome::AsyncMethodCaller* async_caller,
                            CryptohomeClient* cryptohome_client,
                            UserManager* user_manager,
                            Delegate* delegate);
 
-  virtual ~PlatformVerificationFlow();
-
   // Invokes an asynchronous operation to challenge a platform key.  Any user
   // interaction will be associated with |web_contents|.  The |service_id| is an
   // arbitrary value but it should uniquely identify the origin of the request
   // and should not be determined by that origin; its purpose is to prevent
   // collusion between multiple services.  The |challenge| is also an arbitrary
   // value but it should be time sensitive or associated to some kind of session
   // because its purpose is to prevent certificate replay.  The |callback| will
   // be called when the operation completes.  The duration of the operation can
   // vary depending on system state, hardware capabilities, and interaction with
   // the user.
   void ChallengePlatformKey(content::WebContents* web_contents,
                             const std::string& service_id,
                             const std::string& challenge,
                             const ChallengeCallback& callback);
 
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* prefs);
 
   void set_testing_prefs(PrefService* testing_prefs) {
     testing_prefs_ = testing_prefs;
   }
 
   void set_testing_url(const GURL& testing_url) {
     testing_url_ = testing_url;
   }
 
+  void set_timeout_delay(int timeout_delay) {
+    timeout_delay_ = timeout_delay;
+  }
+
  private:
+  // Tracks timeout status for an asynchronous operation.
+  struct TimeoutStatus : public base::RefCountedThreadSafe<TimeoutStatus> {
+    bool finished;
+    bool timed_out;
+
+    TimeoutStatus() : finished(false), timed_out(false) {}
+
+   private:
+    friend class base::RefCountedThreadSafe<TimeoutStatus>;
+    ~TimeoutStatus() {}
+  };
+
+  // Holds the arguments of a ChallengePlatformKey call.  This is convenient for
+  // use with base::Bind so we don't get too many arguments.
+  struct ChallengeContext {
+    content::WebContents* web_contents;
+    std::string service_id;
+    std::string challenge;
+    ChallengeCallback callback;
+  };
+
+  friend class base::RefCountedThreadSafe<PlatformVerificationFlow>;
+  ~PlatformVerificationFlow();
+
   // Checks whether we need to prompt the user for consent before proceeding and
-  // invokes the consent UI if so.  All parameters are the same as in
-  // ChallengePlatformKey except for the additional |attestation_enrolled| which
-  // specifies whether attestation has been enrolled for this device.
-  void CheckConsent(content::WebContents* web_contents,
-                    const std::string& service_id,
-                    const std::string& challenge,
-                    const ChallengeCallback& callback,
+  // invokes the consent UI if so.  The arguments to ChallengePlatformKey are
+  // in |context| and |attestation_enrolled| specifies whether attestation has
+  // been enrolled for this device.
+  void CheckConsent(const ChallengeContext& context,
                     bool attestation_enrolled);
 
-  // A callback called when the user has given their consent response.  All
-  // parameters are the same as in ChallengePlatformKey except for the
-  // additional |consent_type| and |consent_response| which indicate the consent
-  // type and user response, respectively.  If the response indicates that the
-  // operation should proceed, this method invokes a certificate request.
-  void OnConsentResponse(content::WebContents* web_contents,
-                         const std::string& service_id,
-                         const std::string& challenge,
-                         const ChallengeCallback& callback,
+  // A callback called when the user has given their consent response.  The
+  // arguments to ChallengePlatformKey are in |context|.  |consent_type| and
+  // |consent_response| indicate the consent type and user response,
+  // respectively.  If the response indicates that the operation should proceed,
+  // this method invokes a certificate request.
+  void OnConsentResponse(const ChallengeContext& context,
                          ConsentType consent_type,
                          ConsentResponse consent_response);
 
   // A callback called when an attestation certificate request operation
-  // completes.  |service_id|, |challenge|, and |callback| are the same as in
-  // ChallengePlatformKey.  |user_id| identifies the user for which the
-  // certificate was requested.  |operation_success| is true iff the certificate
-  // request operation succeeded.  |certificate| holds the certificate for the
-  // platform key on success.  If the certificate request was successful, this
-  // method invokes a request to sign the challenge.
-  void OnCertificateReady(const std::string& user_id,
-                          const std::string& service_id,
-                          const std::string& challenge,
-                          const ChallengeCallback& callback,
+  // completes.  The arguments to ChallengePlatformKey are in |context|.
+  // |user_id| identifies the user for which the certificate was requested.
+  // |operation_success| is true iff the certificate request operation
+  // succeeded.  |certificate| 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(const ChallengeContext& context,
+                          const std::string& user_id,
+                          TimeoutStatus* timeout_status,
                           bool operation_success,
                           const std::string& certificate);
 
+  // A callback run after a constant delay to handle timeouts for lengthy
+  // certificate requests.  |context.callback| will be invoked with a TIMEOUT
+  // result if the certificate request has not already finished.
+  void OnCertificateTimeout(const ChallengeContext& context,
+                            TimeoutStatus* timeout_status);
+
   // A callback called when a challenge signing request has completed.  The
   // |certificate| is the platform certificate for the key which signed the
-  // |challenge|.  |callback| is the same as in ChallengePlatformKey.
+  // |challenge|.  The arguments to ChallengePlatformKey are in |context|.
   // |operation_success| is true iff the challenge signing operation was
   // successful.  If it was successful, |response_data| holds the challenge
-  // response and the method will invoke |callback|.
-  void OnChallengeReady(const std::string& certificate,
-                        const std::string& challenge,
-                        const ChallengeCallback& callback,
+  // response and the method will invoke |context.callback|.
+  void OnChallengeReady(const ChallengeContext& context,
+                        const std::string& certificate,
                         bool operation_success,
                         const std::string& response_data);
 
   // Gets prefs associated with the given |web_contents|.  If prefs have been
   // set explicitly using set_testing_prefs(), then these are always returned.
   // If no prefs are associated with |web_contents| then NULL is returned.
   PrefService* GetPrefs(content::WebContents* web_contents);
 
   // Gets the URL associated with the given |web_contents|.  If a URL as been
   // set explicitly using set_testing_url(), then this value is always returned.
@@ skipping 38 matching lines @@
 
   AttestationFlow* attestation_flow_;
   scoped_ptr<AttestationFlow> default_attestation_flow_;
   cryptohome::AsyncMethodCaller* async_caller_;
   CryptohomeClient* cryptohome_client_;
   UserManager* user_manager_;
   Delegate* delegate_;
   scoped_ptr<Delegate> default_delegate_;
   PrefService* testing_prefs_;
   GURL testing_url_;
-
-  // Note: This should remain the last member so it'll be destroyed and
-  // invalidate the weak pointers before any other members are destroyed.
-  base::WeakPtrFactory<PlatformVerificationFlow> weak_factory_;
+  int timeout_delay_;
 
   DISALLOW_COPY_AND_ASSIGN(PlatformVerificationFlow);
 };
 
 }  // namespace attestation
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_ATTESTATION_PLATFORM_VERIFICATION_FLOW_H_