From 1746417e717c8e71a93f6e457dbfe7cb82126a61 Mon Sep 17 00:00:00 2001 From: "J.C. Jones" Date: Fri, 29 Mar 2019 17:16:13 +0000 Subject: [PATCH] Bug 1539541 - Enable FIDO U2F API, and permit registrations for Google 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 --- dom/tests/mochitest/general/test_interfaces.js | 2 +- dom/u2f/U2F.cpp | 4 ++-- dom/webauthn/WebAuthnManager.cpp | 2 +- dom/webauthn/WebAuthnUtil.cpp | 5 ++--- dom/webauthn/WebAuthnUtil.h | 2 +- security/manager/ssl/security-prefs.js | 2 +- 6 files changed, 8 insertions(+), 9 deletions(-) diff --git a/dom/tests/mochitest/general/test_interfaces.js b/dom/tests/mochitest/general/test_interfaces.js index ee35a72ae903e..56fb3122a449b 100644 --- a/dom/tests/mochitest/general/test_interfaces.js +++ b/dom/tests/mochitest/general/test_interfaces.js @@ -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! diff --git a/dom/u2f/U2F.cpp b/dom/u2f/U2F.cpp index b20e1c191467e..4f3b8c77b8ca8 100644 --- a/dom/u2f/U2F.cpp +++ b/dom/u2f/U2F.cpp @@ -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(ErrorCode::BAD_REQUEST)); @@ -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(ErrorCode::BAD_REQUEST)); diff --git a/dom/webauthn/WebAuthnManager.cpp b/dom/webauthn/WebAuthnManager.cpp index 7510c61d219b0..cf26a5e0ea2ba 100644 --- a/dom/webauthn/WebAuthnManager.cpp +++ b/dom/webauthn/WebAuthnManager.cpp @@ -526,7 +526,7 @@ already_AddRefed 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(); } diff --git a/dom/webauthn/WebAuthnUtil.cpp b/dom/webauthn/WebAuthnUtil.cpp index 55550a5f2f829..7341430908509 100644 --- a/dom/webauthn/WebAuthnUtil.cpp +++ b/dom/webauthn/WebAuthnUtil.cpp @@ -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 facetUri; @@ -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; diff --git a/dom/webauthn/WebAuthnUtil.h b/dom/webauthn/WebAuthnUtil.h index 52b8c98bf0d56..4ca7d7e84b868 100644 --- a/dom/webauthn/WebAuthnUtil.h +++ b/dom/webauthn/WebAuthnUtil.h @@ -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, diff --git a/security/manager/ssl/security-prefs.js b/security/manager/ssl/security-prefs.js index f9bba1b3db7cb..9e7cef3f5b759 100644 --- a/security/manager/ssl/security-prefs.js +++ b/security/manager/ssl/security-prefs.js @@ -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.