Skip to content

Commit

Permalink
Bug 1539541 - Enable FIDO U2F API, and permit registrations for Googl…
Browse files Browse the repository at this point in the history
…e Accounts r=keeler,qdot

Per the thread "Intent-to-Ship: Backward-Compatibility FIDO U2F support for
Google Accounts" on dev-platform [0], this bug is to:

  1. Enable the security.webauth.u2f by default, to ride the trains

  2. Remove the aOp == U2FOperation::Sign check from EvaluateAppID in
     WebAuthnUtil.cpp, permitting the Google override to work for Register as
     well as Sign.

This would enable Firefox users to use FIDO U2F API on most all sites, subject
to the algorithm limitations discussed in the section "Thorny issues in
enabling our FIDO U2F API implementation" of that post.

[0] https://groups.google.com/d/msg/mozilla.dev.platform/q5cj38hGTEA/lC834665BQAJ

Differential Revision: https://phabricator.services.mozilla.com/D25241

--HG--
extra : moz-landing-system : lando
  • Loading branch information
jcjones committed Mar 29, 2019
1 parent 6f2f0f6 commit 1746417
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 9 deletions.
2 changes: 1 addition & 1 deletion dom/tests/mochitest/general/test_interfaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ var interfaceNamesInGlobalScope =
// IMPORTANT: Do not change this list without review from a DOM peer!
{name: "TreeWalker", insecureContext: true},
// IMPORTANT: Do not change this list without review from a DOM peer!
{name: "U2F", insecureContext: true, disabled: true},
{name: "U2F", insecureContext: false},
// IMPORTANT: Do not change this list without review from a DOM peer!
{name: "UIEvent", insecureContext: true},
// IMPORTANT: Do not change this list without review from a DOM peer!
Expand Down
4 changes: 2 additions & 2 deletions dom/u2f/U2F.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ void U2F::Register(const nsAString& aAppId,

// Evaluate the AppID
nsString adjustedAppId(aAppId);
if (!EvaluateAppID(mParent, mOrigin, U2FOperation::Register, adjustedAppId)) {
if (!EvaluateAppID(mParent, mOrigin, adjustedAppId)) {
RegisterResponse response;
response.mErrorCode.Construct(
static_cast<uint32_t>(ErrorCode::BAD_REQUEST));
Expand Down Expand Up @@ -357,7 +357,7 @@ void U2F::Sign(const nsAString& aAppId, const nsAString& aChallenge,

// Evaluate the AppID
nsString adjustedAppId(aAppId);
if (!EvaluateAppID(mParent, mOrigin, U2FOperation::Sign, adjustedAppId)) {
if (!EvaluateAppID(mParent, mOrigin, adjustedAppId)) {
SignResponse response;
response.mErrorCode.Construct(
static_cast<uint32_t>(ErrorCode::BAD_REQUEST));
Expand Down
2 changes: 1 addition & 1 deletion dom/webauthn/WebAuthnManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ already_AddRefed<Promise> WebAuthnManager::GetAssertion(
nsString appId(aOptions.mExtensions.mAppid.Value());

// Check that the appId value is allowed.
if (!EvaluateAppID(mParent, origin, U2FOperation::Sign, appId)) {
if (!EvaluateAppID(mParent, origin, appId)) {
promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
return promise.forget();
}
Expand Down
5 changes: 2 additions & 3 deletions dom/webauthn/WebAuthnUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const uint8_t FLAG_TUP = 0x01; // Test of User Presence required
const uint8_t FLAG_AT = 0x40; // Authenticator Data is provided

bool EvaluateAppID(nsPIDOMWindowInner* aParent, const nsString& aOrigin,
const U2FOperation& aOp, /* in/out */ nsString& aAppId) {
/* in/out */ nsString& aAppId) {
// Facet is the specification's way of referring to the web origin.
nsAutoCString facetString = NS_ConvertUTF16toUTF8(aOrigin);
nsCOMPtr<nsIURI> facetUri;
Expand Down Expand Up @@ -101,8 +101,7 @@ bool EvaluateAppID(nsPIDOMWindowInner* aParent, const nsString& aOrigin,
}

// Bug #1436078 - Permit Google Accounts. Remove in Bug #1436085 in Jan 2023.
if (aOp == U2FOperation::Sign &&
lowestFacetHost.EqualsLiteral("google.com") &&
if (lowestFacetHost.EqualsLiteral("google.com") &&
(aAppId.Equals(kGoogleAccountsAppId1) ||
aAppId.Equals(kGoogleAccountsAppId2))) {
return true;
Expand Down
2 changes: 1 addition & 1 deletion dom/webauthn/WebAuthnUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace dom {
enum class U2FOperation { Register, Sign };

bool EvaluateAppID(nsPIDOMWindowInner* aParent, const nsString& aOrigin,
const U2FOperation& aOp, /* in/out */ nsString& aAppId);
/* in/out */ nsString& aAppId);

nsresult AssembleAuthenticatorData(const CryptoBuffer& rpIdHashBuf,
const uint8_t flags,
Expand Down
2 changes: 1 addition & 1 deletion security/manager/ssl/security-prefs.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ pref("security.pki.netscape_step_up_policy", 2);
pref("security.pki.certificate_transparency.mode", 0);

// Hardware Origin-bound Second Factor Support
pref("security.webauth.u2f", false);
pref("security.webauth.u2f", true);
pref("security.webauth.webauthn", true);
// Only one of "enable_softtoken" and "enable_usbtoken" can be true
// at a time.
Expand Down

0 comments on commit 1746417

Please sign in to comment.