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

--HG--
extra : rebase_source : ad1665b8140f74b1291f17994285e6146c4ec468
  • Loading branch information
jcjones committed Aug 4, 2017
1 parent fc6de5f commit b491193
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 2 deletions.
87 changes: 86 additions & 1 deletion 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);

/***********************************************************************
* Utility Functions
Expand Down Expand Up @@ -131,6 +134,7 @@ nsresult
GetOrigin(nsPIDOMWindowInner* aParent,
/*out*/ nsAString& aOrigin, /*out*/ 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,
/* use capture */ true,
/* wants untrusted */ 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,
/* use capture */ true);
Unused << NS_WARN_IF(NS_FAILED(rv));
}

/***********************************************************************
* WebAuthnManager Implementation
**********************************************************************/
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 @@ -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();

// nsIIPCbackgroundChildCreateCallback methods
void ActorCreated(PBackgroundChild* aActor) override;
Expand Down

0 comments on commit b491193

Please sign in to comment.