Skip to content

Commit

Permalink
Bug 1383799 - Cancel WebAuthn operations on tab-switch r=ttaubert
Browse files Browse the repository at this point in the history
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] w3c/webauthn#316

MozReview-Commit-ID: 6Qh9oC4pqys

UltraBlame original commit: f7a53ff2f8cb312eb6a65b127207e04d2bd1c79c
  • Loading branch information
marco-c committed Oct 1, 2019
1 parent 388886f commit 203968f
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 5 deletions.
93 changes: 89 additions & 4 deletions dom/webauthn/WebAuthnManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ StaticRefPtr<WebAuthnManager> gWebAuthnManager;
static mozilla::LazyLogModule gWebAuthnManagerLog("webauthnmanager");
}

NS_IMPL_ISUPPORTS(WebAuthnManager, nsIIPCBackgroundChildCreateCallback);
NS_NAMED_LITERAL_STRING(kVisibilityChange, "visibilitychange");

NS_IMPL_ISUPPORTS(WebAuthnManager, nsIIPCBackgroundChildCreateCallback,
nsIDOMEventListener);



Expand Down Expand Up @@ -131,6 +134,7 @@ nsresult
GetOrigin(nsPIDOMWindowInner* aParent,
nsAString& aOrigin, nsACString& aHost)
{
MOZ_ASSERT(aParent);
nsCOMPtr<nsIDocument> doc = aParent->GetDoc();
MOZ_ASSERT(doc);

Expand Down Expand Up @@ -168,6 +172,7 @@ RelaxSameOrigin(nsPIDOMWindowInner* aParent,
MOZ_ASSERT(aParent);
nsCOMPtr<nsIDocument> doc = aParent->GetDoc();
MOZ_ASSERT(doc);

nsCOMPtr<nsIPrincipal> principal = doc->NodePrincipal();
nsCOMPtr<nsIURI> uri;
if (NS_FAILED(principal->GetURI(getter_AddRefs(uri)))) {
Expand Down Expand Up @@ -212,6 +217,41 @@ RelaxSameOrigin(nsPIDOMWindowInner* aParent,
return NS_OK;
}

static void
ListenForVisibilityEvents(nsPIDOMWindowInner* aParent,
WebAuthnManager* aListener)
{
MOZ_ASSERT(aParent);
MOZ_ASSERT(aListener);

nsCOMPtr<nsIDocument> 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<nsIDocument> doc = aParent->GetExtantDoc();
if (NS_WARN_IF(!doc)) {
return;
}

nsresult rv = doc->RemoveSystemEventListener(kVisibilityChange, aListener,
true);
Unused << NS_WARN_IF(NS_FAILED(rv));
}




Expand All @@ -227,6 +267,11 @@ WebAuthnManager::MaybeClearTransaction()
mClientData.reset();
mInfo.reset();
mTransactionPromise = nullptr;
if (mCurrentParent) {
StopListeningForVisibilityEvents(mCurrentParent, this);
mCurrentParent = nullptr;
}

if (mChild) {
RefPtr<WebAuthnTransactionChild> c;
mChild.swap(c);
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -503,6 +548,8 @@ WebAuthnManager::MakeCredential(nsPIDOMWindowInner* aParent,
mClientData = Some(clientDataJSON);
mCurrentParent = aParent;
mInfo = Some(info);
ListenForVisibilityEvents(aParent, this);

return promise.forget();
}

Expand All @@ -520,6 +567,13 @@ WebAuthnManager::StartSign() {
}
}

void
WebAuthnManager::StartCancel() {
if (mChild) {
mChild->SendRequestCancel();
}
}

already_AddRefed<Promise>
WebAuthnManager::GetAssertion(nsPIDOMWindowInner* aParent,
const PublicKeyCredentialRequestOptions& aOptions)
Expand Down Expand Up @@ -669,6 +723,8 @@ WebAuthnManager::GetAssertion(nsPIDOMWindowInner* aParent,
mClientData = Some(clientDataJSON);
mCurrentParent = aParent;
mInfo = Some(info);
ListenForVisibilityEvents(aParent, this);

return promise.forget();
}

Expand Down Expand Up @@ -877,12 +933,41 @@ WebAuthnManager::FinishGetAssertion(nsTArray<uint8_t>& 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<nsIDocument> 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)
{
Expand Down
7 changes: 6 additions & 1 deletion dom/webauthn/WebAuthnManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"


Expand Down Expand Up @@ -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();

Expand All @@ -88,6 +92,7 @@ class WebAuthnManager final : public nsIIPCBackgroundChildCreateCallback

void StartRegister();
void StartSign();
void StartCancel();


void ActorCreated(PBackgroundChild* aActor) override;
Expand Down

0 comments on commit 203968f

Please sign in to comment.