Bug 1244959 - Use IsRegistrableDomainSuffixOfOrEqualTo for U2F Facets r?ttaubert
In Comment 8 of
Bug 1244959 [1], Brad Hill argues that instead of leaving our
U2F Facet support completely half-way, that we could use the Public Suffix logic
introduced into HTML for W3C Web Authentication (the method named
IsRegistrableDomainSuffixOfOrEqualTo) to scope the FIDO AppID to an eTLD+1
hierarchy. This is a deviation from the FIDO specification, but doesn't break
anything that currently works with our U2F implementation, and theoretically
enables sites that otherwise need an external FacetID fetch which we aren't
implementing.
The downside to this is that it's then Firefox-specific behavior. But since this
isn't a shipped feature, we have more room to experiment. As an additional
bonus, it encourages U2F sites to use the upcoming Web Authentication security
model, which will help them prepare to adopt the newer standard.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1244959#c8
MozReview-Commit-ID: DzNVhHT9qRL
--- a/dom/u2f/U2F.cpp
+++ b/dom/u2f/U2F.cpp
@@ -2,21 +2,29 @@
/* vim:set ts=2 sw=2 sts=2 et cindent: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "mozilla/dom/U2F.h"
#include "mozilla/dom/WebCryptoCommon.h"
#include "nsContentUtils.h"
+#include "nsIEffectiveTLDService.h"
#include "nsNetCID.h"
#include "nsNetUtil.h"
#include "nsURLParsers.h"
#include "U2FManager.h"
+// Forward decl because of nsHTMLDocument.h's complex dependency on /layout/style
+class nsHTMLDocument {
+public:
+ bool IsRegistrableDomainSuffixOfOrEqualTo(const nsAString& aHostSuffixString,
+ const nsACString& aOrigHost);
+};
+
namespace mozilla {
namespace dom {
static mozilla::LazyLogModule gU2FLog("u2fmanager");
NS_NAMED_LITERAL_STRING(kFinishEnrollment, "navigator.id.finishEnrollment");
NS_NAMED_LITERAL_STRING(kGetAssertion, "navigator.id.getAssertion");
@@ -85,17 +93,18 @@ RegisteredKeysToScopedCredentialList(con
WebAuthnScopedCredentialDescriptor c;
c.id() = keyHandle;
aList.AppendElement(c);
}
}
static ErrorCode
-EvaluateAppID(const nsString& aOrigin, /* in/out */ nsString& aAppId)
+EvaluateAppID(nsPIDOMWindowInner* aParent, const nsString& aOrigin,
+ /* in/out */ nsString& aAppId)
{
// Facet is the specification's way of referring to the web origin.
nsAutoCString facetString = NS_ConvertUTF16toUTF8(aOrigin);
nsCOMPtr<nsIURI> facetUri;
if (NS_FAILED(NS_NewURI(getter_AddRefs(facetUri), facetString))) {
return ErrorCode::BAD_REQUEST;
}
@@ -119,30 +128,53 @@ EvaluateAppID(const nsString& aOrigin, /
}
// if the appId URL is not HTTPS, reject.
bool appIdIsHttps = false;
if (NS_FAILED(appIdUri->SchemeIs("https", &appIdIsHttps)) || !appIdIsHttps) {
return ErrorCode::BAD_REQUEST;
}
- // If the facetId and the appId hosts match, accept
- nsAutoCString facetHost;
- if (NS_FAILED(facetUri->GetHost(facetHost))) {
+ // Run the HTML5 algorithm to relax the same-origin policy, copied from W3C
+ // Web Authentication. See Bug 1244959 comment #8 for context on why we are
+ // doing this instead of implementing the external-fetch FacetID logic.
+ nsCOMPtr<nsIDocument> document = aParent->GetDoc();
+ if (!document || !document->IsHTMLDocument()) {
+ return ErrorCode::BAD_REQUEST;
+ }
+ nsHTMLDocument* html = document->AsHTMLDocument();
+ if (NS_WARN_IF(!html)) {
+ return ErrorCode::BAD_REQUEST;
+ }
+
+ // Use the base domain as the facet for evaluation. This lets this algorithm
+ // relax the whole eTLD+1.
+ nsCOMPtr<nsIEffectiveTLDService> tldService =
+ do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID);
+ if (!tldService) {
+ return ErrorCode::BAD_REQUEST;
+ }
+
+ nsAutoCString lowestFacetHost;
+ if (NS_FAILED(tldService->GetBaseDomain(facetUri, 0, lowestFacetHost))) {
return ErrorCode::BAD_REQUEST;
}
nsAutoCString appIdHost;
- if (NS_FAILED(appIdUri->GetHost(appIdHost))) {
+ if (NS_FAILED(appIdUri->GetAsciiHost(appIdHost))) {
return ErrorCode::BAD_REQUEST;
}
- if (facetHost.Equals(appIdHost)) {
+
+ MOZ_LOG(gU2FLog, LogLevel::Debug,
+ ("AppId %s Facet %s", appIdHost.get(), lowestFacetHost.get()));
+
+ if (html->IsRegistrableDomainSuffixOfOrEqualTo(NS_ConvertUTF8toUTF16(lowestFacetHost),
+ appIdHost)) {
return ErrorCode::OK;
}
- // TODO(Bug 1244959) Implement the remaining algorithm.
return ErrorCode::BAD_REQUEST;
}
template<typename T, typename C>
static void
ExecuteCallback(T& aResp, Maybe<nsMainThreadPtrHandle<C>>& aCb)
{
MOZ_ASSERT(NS_IsMainThread());
@@ -228,17 +260,17 @@ U2F::Register(const nsAString& aAppId,
new nsMainThreadPtrHolder<U2FRegisterCallback>(
"U2F::Register::callback", &aCallback)));
uint32_t adjustedTimeoutMillis = AdjustedTimeoutMillis(opt_aTimeoutSeconds);
// Evaluate the AppID
nsString adjustedAppId;
adjustedAppId.Assign(aAppId);
- ErrorCode appIdResult = EvaluateAppID(mOrigin, adjustedAppId);
+ ErrorCode appIdResult = EvaluateAppID(mParent, mOrigin, adjustedAppId);
if (appIdResult != ErrorCode::OK) {
RegisterResponse response;
response.mErrorCode.Construct(static_cast<uint32_t>(appIdResult));
ExecuteCallback(response, mRegisterCallback);
return;
}
// Produce the AppParam from the current AppID
@@ -325,17 +357,17 @@ U2F::Sign(const nsAString& aAppId,
new nsMainThreadPtrHolder<U2FSignCallback>(
"U2F::Sign::callback", &aCallback)));
uint32_t adjustedTimeoutMillis = AdjustedTimeoutMillis(opt_aTimeoutSeconds);
// Evaluate the AppID
nsString adjustedAppId;
adjustedAppId.Assign(aAppId);
- ErrorCode appIdResult = EvaluateAppID(mOrigin, adjustedAppId);
+ ErrorCode appIdResult = EvaluateAppID(mParent, mOrigin, adjustedAppId);
if (appIdResult != ErrorCode::OK) {
SignResponse response;
response.mErrorCode.Construct(static_cast<uint32_t>(appIdResult));
ExecuteCallback(response, mSignCallback);
return;
}
// Produce the AppParam from the current AppID
--- a/dom/u2f/U2F.h
+++ b/dom/u2f/U2F.h
@@ -9,17 +9,16 @@
#include "js/TypeDecls.h"
#include "mozilla/Attributes.h"
#include "mozilla/dom/BindingDeclarations.h"
#include "mozilla/dom/Nullable.h"
#include "mozilla/dom/U2FBinding.h"
#include "mozilla/ErrorResult.h"
#include "mozilla/MozPromise.h"
-#include "nsPIDOMWindow.h"
#include "nsProxyRelease.h"
#include "nsWrapperCache.h"
#include "U2FAuthenticator.h"
class nsISerialEventTarget;
namespace mozilla {
namespace dom {
--- a/dom/u2f/tests/frame_appid_facet.html
+++ b/dom/u2f/tests/frame_appid_facet.html
@@ -49,25 +49,33 @@ async function doTests() {
local_is(res.errorCode, 0, "HTTPS origin for example.com should work");
});
// Test: Sub-domain
await promiseU2FRegister("https://test2.example.com/appId", [{
version: version,
challenge: bytesToBase64UrlSafe(challenge),
}], [], function(res){
- local_is(res.errorCode, 2, "HTTPS origin for test2.example.com shouldn't work");
+ local_is(res.errorCode, 0, "HTTPS origin for test2.example.com should work");
});
// Test: Sub-sub-domain
await promiseU2FRegister("https://sub.test2.example.com/appId", [{
version: version,
challenge: bytesToBase64UrlSafe(challenge),
}], [], function(res){
- local_is(res.errorCode, 2, "HTTPS origin for sub.test2.example.com shouldn't work");
+ local_is(res.errorCode, 0, "HTTPS origin for sub.test2.example.com should work");
+ });
+
+ // Test: TLD
+ await promiseU2FRegister("https://com/crazyAppID", [{
+ version: version,
+ challenge: bytesToBase64UrlSafe(challenge),
+ }], [], function(res){
+ local_is(res.errorCode, 2, "HTTPS origin of the TLD should not work");
});
// Test: Dynamic origin
await promiseU2FRegister(window.location.origin + "/otherAppId", [{
version: version,
challenge: bytesToBase64UrlSafe(challenge),
}], [], function(res){
local_is(res.errorCode, 0, "Direct window origin should work");
--- a/dom/u2f/tests/frame_appid_facet_subdomain.html
+++ b/dom/u2f/tests/frame_appid_facet_subdomain.html
@@ -31,27 +31,54 @@ async function doTests() {
local_isnot(res.errorCode, 0, "AppID should not work when using a different scheme");
});
// eTLD+1 subdomain check
await promiseU2FRegister("https://example.com/appId", [{
version: version,
challenge: bytesToBase64UrlSafe(challenge),
}], [], function(res){
- local_isnot(res.errorCode, 0, "AppID should not work from another subdomain in this registered domain");
+ // Changed in Bug 1244959 to behave like W3C Web Authentication
+ local_is(res.errorCode, 0, "AppID should work from another subdomain in this registered domain");
+ });
+
+ // sub-subdomain check
+ await promiseU2FRegister("https://sub.test1.example.com/appId", [{
+ version: version,
+ challenge: bytesToBase64UrlSafe(challenge),
+ }], [], function(res){
+ // Changed in Bug 1244959 to behave like W3C Web Authentication
+ local_is(res.errorCode, 0, "AppID should work from a sub-subdomain");
+ });
+
+ // sub-subdomain check
+ await promiseU2FRegister("https://test2.example.com/appId", [{
+ version: version,
+ challenge: bytesToBase64UrlSafe(challenge),
+ }], [], function(res){
+ // Changed in Bug 1244959 to behave like W3C Web Authentication
+ local_is(res.errorCode, 0, "AppID should work from another subdomain of the eTLD+1");
});
// other domain check
await promiseU2FRegister("https://mochi.test:8888/appId", [{
version: version,
challenge: bytesToBase64UrlSafe(challenge),
}], [], function(res){
local_isnot(res.errorCode, 0, "AppID should not work from other domains");
});
+ // TLD check
+ await promiseU2FRegister("https://com:8888/appId", [{
+ version: version,
+ challenge: bytesToBase64UrlSafe(challenge),
+ }], [], function(res){
+ local_isnot(res.errorCode, 0, "AppID should not work from the eTLD itself");
+ });
+
local_finished();
};
doTests();
</script>
</body>
</html>
--- a/dom/u2f/tests/u2futil.js
+++ b/dom/u2f/tests/u2futil.js
@@ -52,17 +52,17 @@ function base64ToBytes(b64encoded) {
function bytesToBase64UrlSafe(buf) {
return bytesToBase64(buf)
.replace(/\+/g, "-")
.replace(/\//g, "_")
.replace(/=/g, "");
}
function base64ToBytesUrlSafe(str) {
- if (str.length % 4 == 1) {
+ if (!str || str.length % 4 == 1) {
throw "Improper b64 string";
}
var b64 = str.replace(/\-/g, "+").replace(/\_/g, "/");
while (b64.length % 4 != 0) {
b64 += "=";
}
return base64ToBytes(b64);