Bug 1375744 - Add U2FTokenTransport::Cancel() to abort requests on HW devices r=qDot
authorTim Taubert <ttaubert@mozilla.com>
Fri, 23 Jun 2017 21:04:38 +0200
changeset 365813 6910856b1ae9e86b2a3b116cfe1b5a8612ee9e5b
parent 365812 ae8b53ec3d68941e3edc5c25402a29b9e51d4aa2
child 365814 54e2ea6388dcb23d9eb3815df5c11128fcffbee9
push id91834
push userttaubert@mozilla.com
push dateFri, 23 Jun 2017 19:06:25 +0000
treeherdermozilla-inbound@6910856b1ae9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqDot
bugs1375744
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1375744 - Add U2FTokenTransport::Cancel() to abort requests on HW devices r=qDot This patch adds a Cancel() method to the U2FTokenTransport interface so that we can forward request cancellations to the actual token manager implementation. The current softtoken doesn't need that as it processes API calls synchronously, USB HID tokens however need a cancellation mechanism. The SendRequestCancel() call has been removed from WebAuthnManager::Cancel() as we're currently only calling this method either when the chrome process cancels the request (and then we don't need to send it back again) or the content process fails to process the data after a request was fulfilled and thus there's nothing to cancel. We will touch this again later when the UI cancels requests on tab switch and similar user actions.
dom/webauthn/U2FSoftTokenManager.cpp
dom/webauthn/U2FSoftTokenManager.h
dom/webauthn/U2FTokenManager.cpp
dom/webauthn/U2FTokenManager.h
dom/webauthn/U2FTokenTransport.h
dom/webauthn/WebAuthnManager.cpp
dom/webauthn/WebAuthnTransactionParent.cpp
--- a/dom/webauthn/U2FSoftTokenManager.cpp
+++ b/dom/webauthn/U2FSoftTokenManager.cpp
@@ -871,10 +871,16 @@ U2FSoftTokenManager::Sign(const nsTArray
   signatureBuf.AppendElement(0x01, mozilla::fallible);
   signatureBuf.AppendSECItem(counterItem);
   signatureBuf.AppendSECItem(signatureItem);
 
   aSignature = signatureBuf;
   return NS_OK;
 }
 
+void
+U2FSoftTokenManager::Cancel()
+{
+  // This implementation is sync, requests can't be aborted.
+}
+
 }
 }
--- a/dom/webauthn/U2FSoftTokenManager.h
+++ b/dom/webauthn/U2FSoftTokenManager.h
@@ -32,16 +32,18 @@ public:
                             /* out */ nsTArray<uint8_t>& aSignature) override;
 
   virtual nsresult Sign(const nsTArray<WebAuthnScopedCredentialDescriptor>& aDescriptors,
                         const nsTArray<uint8_t>& aApplication,
                         const nsTArray<uint8_t>& aChallenge,
                         /* out */ nsTArray<uint8_t>& aKeyHandle,
                         /* out */ nsTArray<uint8_t>& aSignature) override;
 
+  virtual void Cancel() override;
+
   // For nsNSSShutDownObject
   virtual void virtualDestroyNSSReference() override;
   void destructorSafeDestroyNSSReference();
 
 private:
   ~U2FSoftTokenManager();
   nsresult Init();
   bool IsCompatibleVersion(const nsAString& aVersion);
--- a/dom/webauthn/U2FTokenManager.cpp
+++ b/dom/webauthn/U2FTokenManager.cpp
@@ -140,123 +140,135 @@ U2FTokenManager::Get()
 {
   MOZ_ASSERT(XRE_IsParentProcess());
   // We should only be accessing this on the background thread
   MOZ_ASSERT(!NS_IsMainThread());
   return gU2FTokenManager;
 }
 
 void
+U2FTokenManager::AbortTransaction(const nsresult& aError)
+{
+  Unused << mTransactionParent->SendCancel(aError);
+  ClearTransaction();
+}
+
+void
 U2FTokenManager::MaybeClearTransaction(WebAuthnTransactionParent* aParent)
 {
   // Only clear if we've been requested to do so by our current transaction
   // parent.
-  if (mTransactionParent != aParent) {
-    return;
+  if (mTransactionParent == aParent) {
+    ClearTransaction();
   }
+}
+
+void
+U2FTokenManager::ClearTransaction()
+{
   mTransactionParent = nullptr;
   // Drop managers at the end of all transactions
   mTokenManagerImpl = nullptr;
 }
 
 void
-U2FTokenManager::Cancel(const nsresult& aError)
-{
-  if (mTransactionParent) {
-    Unused << mTransactionParent->SendCancel(aError);
-  }
-  MaybeClearTransaction(mTransactionParent);
-}
-
-void
 U2FTokenManager::Register(WebAuthnTransactionParent* aTransactionParent,
                           const WebAuthnTransactionInfo& aTransactionInfo)
 {
   MOZ_LOG(gU2FTokenManagerLog, LogLevel::Debug, ("U2FAuthRegister"));
   MOZ_ASSERT(U2FPrefManager::Get());
   mTransactionParent = aTransactionParent;
 
   // Since we only have soft token available at the moment, use that if the pref
   // is on.
   //
   // TODO Check all transports and use WebAuthnRequest to aggregate
   // replies
   if (!U2FPrefManager::Get()->GetSoftTokenEnabled()) {
-    Cancel(NS_ERROR_DOM_NOT_ALLOWED_ERR);
+    AbortTransaction(NS_ERROR_DOM_NOT_ALLOWED_ERR);
     return;
   }
 
   if (!mTokenManagerImpl) {
     mTokenManagerImpl = new U2FSoftTokenManager(U2FPrefManager::Get()->GetSoftTokenCounter());
   }
 
   // Check if all the supplied parameters are syntactically well-formed and
   // of the correct length. If not, return an error code equivalent to
   // UnknownError and terminate the operation.
 
   if ((aTransactionInfo.RpIdHash().Length() != SHA256_LENGTH) ||
       (aTransactionInfo.ClientDataHash().Length() != SHA256_LENGTH)) {
-    Cancel(NS_ERROR_DOM_UNKNOWN_ERR);
+    AbortTransaction(NS_ERROR_DOM_UNKNOWN_ERR);
     return;
   }
 
   nsTArray<uint8_t> reg;
   nsTArray<uint8_t> sig;
   nsresult rv = mTokenManagerImpl->Register(aTransactionInfo.Descriptors(),
                                             aTransactionInfo.RpIdHash(),
                                             aTransactionInfo.ClientDataHash(),
                                             reg,
                                             sig);
   if (NS_FAILED(rv)) {
-    Cancel(rv);
+    AbortTransaction(rv);
     return;
   }
 
   Unused << mTransactionParent->SendConfirmRegister(reg, sig);
-  MaybeClearTransaction(mTransactionParent);
+  ClearTransaction();
 }
 
 void
 U2FTokenManager::Sign(WebAuthnTransactionParent* aTransactionParent,
                       const WebAuthnTransactionInfo& aTransactionInfo)
 {
   MOZ_LOG(gU2FTokenManagerLog, LogLevel::Debug, ("U2FAuthSign"));
   MOZ_ASSERT(U2FPrefManager::Get());
   mTransactionParent = aTransactionParent;
 
   // Since we only have soft token available at the moment, use that if the pref
   // is on.
   //
   // TODO Check all transports and use WebAuthnRequest to aggregate
   // replies
   if (!U2FPrefManager::Get()->GetSoftTokenEnabled()) {
-    Cancel(NS_ERROR_DOM_NOT_ALLOWED_ERR);
+    AbortTransaction(NS_ERROR_DOM_NOT_ALLOWED_ERR);
     return;
   }
 
   if (!mTokenManagerImpl) {
     mTokenManagerImpl = new U2FSoftTokenManager(U2FPrefManager::Get()->GetSoftTokenCounter());
   }
 
   if ((aTransactionInfo.RpIdHash().Length() != SHA256_LENGTH) ||
       (aTransactionInfo.ClientDataHash().Length() != SHA256_LENGTH)) {
-    Cancel(NS_ERROR_DOM_UNKNOWN_ERR);
+    AbortTransaction(NS_ERROR_DOM_UNKNOWN_ERR);
     return;
   }
 
   nsTArray<uint8_t> id;
   nsTArray<uint8_t> sig;
   nsresult rv = mTokenManagerImpl->Sign(aTransactionInfo.Descriptors(),
                                         aTransactionInfo.RpIdHash(),
                                         aTransactionInfo.ClientDataHash(),
                                         id,
                                         sig);
   if (NS_FAILED(rv)) {
-    Cancel(rv);
+    AbortTransaction(rv);
     return;
   }
 
   Unused << mTransactionParent->SendConfirmSign(id, sig);
-  MaybeClearTransaction(mTransactionParent);
+  ClearTransaction();
+}
+
+void
+U2FTokenManager::Cancel(WebAuthnTransactionParent* aParent)
+{
+  if (mTransactionParent == aParent) {
+    mTokenManagerImpl->Cancel();
+    ClearTransaction();
+  }
 }
 
 }
 }
--- a/dom/webauthn/U2FTokenManager.h
+++ b/dom/webauthn/U2FTokenManager.h
@@ -42,22 +42,24 @@ public:
     NumTransactionTypes
   };
   NS_INLINE_DECL_REFCOUNTING(U2FTokenManager)
   static U2FTokenManager* Get();
   void Register(WebAuthnTransactionParent* aTransactionParent,
                 const WebAuthnTransactionInfo& aTransactionInfo);
   void Sign(WebAuthnTransactionParent* aTransactionParent,
             const WebAuthnTransactionInfo& aTransactionInfo);
+  void Cancel(WebAuthnTransactionParent* aTransactionParent);
   void MaybeClearTransaction(WebAuthnTransactionParent* aParent);
   static void Initialize();
 private:
   U2FTokenManager();
   ~U2FTokenManager();
-  void Cancel(const nsresult& aError);
+  void AbortTransaction(const nsresult& aError);
+  void ClearTransaction();
   // Using a raw pointer here, as the lifetime of the IPC object is managed by
   // the PBackground protocol code. This means we cannot be left holding an
   // invalid IPC protocol object after the transaction is finished.
   WebAuthnTransactionParent* mTransactionParent;
   RefPtr<U2FTokenTransport> mTokenManagerImpl;
 };
 
 } // namespace dom
--- a/dom/webauthn/U2FTokenTransport.h
+++ b/dom/webauthn/U2FTokenTransport.h
@@ -31,16 +31,18 @@ public:
                             /* out */ nsTArray<uint8_t>& aSignature) = 0;
 
   virtual nsresult Sign(const nsTArray<WebAuthnScopedCredentialDescriptor>& aDescriptors,
                         const nsTArray<uint8_t>& aApplication,
                         const nsTArray<uint8_t>& aChallenge,
                         /* out */ nsTArray<uint8_t>& aKeyHandle,
                         /* out */ nsTArray<uint8_t>& aSignature) = 0;
 
+  virtual void Cancel() = 0;
+
 protected:
   virtual ~U2FTokenTransport() = default;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_U2FTokenTransport_h
--- a/dom/webauthn/WebAuthnManager.cpp
+++ b/dom/webauthn/WebAuthnManager.cpp
@@ -734,19 +734,16 @@ WebAuthnManager::FinishGetAssertion(nsTA
 
   mTransactionPromise->MaybeResolve(credential);
   MaybeClearTransaction();
 }
 
 void
 WebAuthnManager::Cancel(const nsresult& aError)
 {
-  if (mChild) {
-    mChild->SendRequestCancel();
-  }
   if (mTransactionPromise) {
     mTransactionPromise->MaybeReject(aError);
   }
   MaybeClearTransaction();
 }
 
 void
 WebAuthnManager::ActorCreated(PBackgroundChild* aActor)
--- a/dom/webauthn/WebAuthnTransactionParent.cpp
+++ b/dom/webauthn/WebAuthnTransactionParent.cpp
@@ -25,17 +25,17 @@ WebAuthnTransactionParent::RecvRequestSi
   mgr->Sign(this, aTransactionInfo);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 WebAuthnTransactionParent::RecvRequestCancel()
 {
   U2FTokenManager* mgr = U2FTokenManager::Get();
-  mgr->MaybeClearTransaction(this);
+  mgr->Cancel(this);
   return IPC_OK();
 }
 
 void
 WebAuthnTransactionParent::ActorDestroy(ActorDestroyReason aWhy)
 {
   U2FTokenManager* mgr = U2FTokenManager::Get();
   mgr->MaybeClearTransaction(this);