From 203968f3f78d91e7238d0d942c3b4f6c2f37938b Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Tue, 1 Oct 2019 16:24:15 +0000 Subject: [PATCH] Bug 1383799 - Cancel WebAuthn operations on tab-switch r=ttaubert WebAuthn operations that are in-flight with authenticators must be cancelled when switching tabs. There's an Issue [1] opened with the WebAuthn spec for this already, but the language is _not_ in spec. Still, it's necessary for security, spec or not. This also matches how Chromium handles U2F operations during a tab switch. [1] https://github.com/w3c/webauthn/issues/316 MozReview-Commit-ID: 6Qh9oC4pqys UltraBlame original commit: f7a53ff2f8cb312eb6a65b127207e04d2bd1c79c --- dom/webauthn/WebAuthnManager.cpp | 93 ++++++++++++++++++++++++++++++-- dom/webauthn/WebAuthnManager.h | 7 ++- 2 files changed, 95 insertions(+), 5 deletions(-) diff --git a/dom/webauthn/WebAuthnManager.cpp b/dom/webauthn/WebAuthnManager.cpp index 68c199d6214a..b03959077ced 100644 --- a/dom/webauthn/WebAuthnManager.cpp +++ b/dom/webauthn/WebAuthnManager.cpp @@ -42,7 +42,10 @@ StaticRefPtr gWebAuthnManager; static mozilla::LazyLogModule gWebAuthnManagerLog("webauthnmanager"); } -NS_IMPL_ISUPPORTS(WebAuthnManager, nsIIPCBackgroundChildCreateCallback); +NS_NAMED_LITERAL_STRING(kVisibilityChange, "visibilitychange"); + +NS_IMPL_ISUPPORTS(WebAuthnManager, nsIIPCBackgroundChildCreateCallback, + nsIDOMEventListener); @@ -131,6 +134,7 @@ nsresult GetOrigin(nsPIDOMWindowInner* aParent, nsAString& aOrigin, nsACString& aHost) { + MOZ_ASSERT(aParent); nsCOMPtr doc = aParent->GetDoc(); MOZ_ASSERT(doc); @@ -168,6 +172,7 @@ RelaxSameOrigin(nsPIDOMWindowInner* aParent, MOZ_ASSERT(aParent); nsCOMPtr doc = aParent->GetDoc(); MOZ_ASSERT(doc); + nsCOMPtr principal = doc->NodePrincipal(); nsCOMPtr uri; if (NS_FAILED(principal->GetURI(getter_AddRefs(uri)))) { @@ -212,6 +217,41 @@ RelaxSameOrigin(nsPIDOMWindowInner* aParent, return NS_OK; } +static void +ListenForVisibilityEvents(nsPIDOMWindowInner* aParent, + WebAuthnManager* aListener) +{ + MOZ_ASSERT(aParent); + MOZ_ASSERT(aListener); + + nsCOMPtr doc = aParent->GetExtantDoc(); + if (NS_WARN_IF(!doc)) { + return; + } + + nsresult rv = doc->AddSystemEventListener(kVisibilityChange, aListener, + true, + false); + Unused << NS_WARN_IF(NS_FAILED(rv)); +} + +static void +StopListeningForVisibilityEvents(nsPIDOMWindowInner* aParent, + WebAuthnManager* aListener) +{ + MOZ_ASSERT(aParent); + MOZ_ASSERT(aListener); + + nsCOMPtr doc = aParent->GetExtantDoc(); + if (NS_WARN_IF(!doc)) { + return; + } + + nsresult rv = doc->RemoveSystemEventListener(kVisibilityChange, aListener, + true); + Unused << NS_WARN_IF(NS_FAILED(rv)); +} + @@ -227,6 +267,11 @@ WebAuthnManager::MaybeClearTransaction() mClientData.reset(); mInfo.reset(); mTransactionPromise = nullptr; + if (mCurrentParent) { + StopListeningForVisibilityEvents(mCurrentParent, this); + mCurrentParent = nullptr; + } + if (mChild) { RefPtr c; mChild.swap(c); @@ -310,11 +355,11 @@ WebAuthnManager::MakeCredential(nsPIDOMWindowInner* aParent, - uint32_t adjustedTimeout = 30000; + double adjustedTimeout = 30.0; if (aOptions.mTimeout.WasPassed()) { adjustedTimeout = aOptions.mTimeout.Value(); - adjustedTimeout = std::max(15000u, adjustedTimeout); - adjustedTimeout = std::min(120000u, adjustedTimeout); + adjustedTimeout = std::max(15.0, adjustedTimeout); + adjustedTimeout = std::min(120.0, adjustedTimeout); } if (aOptions.mRp.mId.WasPassed()) { @@ -503,6 +548,8 @@ WebAuthnManager::MakeCredential(nsPIDOMWindowInner* aParent, mClientData = Some(clientDataJSON); mCurrentParent = aParent; mInfo = Some(info); + ListenForVisibilityEvents(aParent, this); + return promise.forget(); } @@ -520,6 +567,13 @@ WebAuthnManager::StartSign() { } } +void +WebAuthnManager::StartCancel() { + if (mChild) { + mChild->SendRequestCancel(); + } +} + already_AddRefed WebAuthnManager::GetAssertion(nsPIDOMWindowInner* aParent, const PublicKeyCredentialRequestOptions& aOptions) @@ -669,6 +723,8 @@ WebAuthnManager::GetAssertion(nsPIDOMWindowInner* aParent, mClientData = Some(clientDataJSON); mCurrentParent = aParent; mInfo = Some(info); + ListenForVisibilityEvents(aParent, this); + return promise.forget(); } @@ -877,12 +933,41 @@ WebAuthnManager::FinishGetAssertion(nsTArray& aCredentialId, void WebAuthnManager::Cancel(const nsresult& aError) { + MOZ_ASSERT(NS_IsMainThread()); + if (mTransactionPromise) { mTransactionPromise->MaybeReject(aError); } + MaybeClearTransaction(); } +NS_IMETHODIMP +WebAuthnManager::HandleEvent(nsIDOMEvent* aEvent) +{ + MOZ_ASSERT(aEvent); + + nsAutoString type; + aEvent->GetType(type); + if (!type.Equals(kVisibilityChange)) { + return NS_ERROR_FAILURE; + } + + nsCOMPtr doc = + do_QueryInterface(aEvent->InternalDOMEvent()->GetTarget()); + MOZ_ASSERT(doc); + + if (doc && doc->Hidden()) { + MOZ_LOG(gWebAuthnManagerLog, LogLevel::Debug, + ("Visibility change: WebAuthn window is hidden, cancelling job.")); + + StartCancel(); + Cancel(NS_ERROR_ABORT); + } + + return NS_OK; +} + void WebAuthnManager::ActorCreated(PBackgroundChild* aActor) { diff --git a/dom/webauthn/WebAuthnManager.h b/dom/webauthn/WebAuthnManager.h index 1eb3195634b5..44270c1c6a26 100644 --- a/dom/webauthn/WebAuthnManager.h +++ b/dom/webauthn/WebAuthnManager.h @@ -8,7 +8,9 @@ #define mozilla_dom_WebAuthnManager_h #include "mozilla/MozPromise.h" +#include "mozilla/dom/Event.h" #include "mozilla/dom/PWebAuthnTransaction.h" +#include "nsIDOMEventListener.h" #include "nsIIPCBackgroundChildCreateCallback.h" @@ -61,10 +63,12 @@ class Promise; class WebAuthnTransactionChild; class WebAuthnTransactionInfo; -class WebAuthnManager final : public nsIIPCBackgroundChildCreateCallback +class WebAuthnManager final : public nsIIPCBackgroundChildCreateCallback, + public nsIDOMEventListener { public: NS_DECL_ISUPPORTS + NS_DECL_NSIDOMEVENTLISTENER static WebAuthnManager* GetOrCreate(); static WebAuthnManager* Get(); @@ -88,6 +92,7 @@ class WebAuthnManager final : public nsIIPCBackgroundChildCreateCallback void StartRegister(); void StartSign(); + void StartCancel(); void ActorCreated(PBackgroundChild* aActor) override;