Bug 1244959 - Use IsRegistrableDomainSuffixOfOrEqualTo for U2F Facets r?ttaubert draft
authorJ.C. Jones <jjones@mozilla.com>
Thu, 28 Sep 2017 16:45:28 -0700
changeset 674864 de54cf782b9a646deb51069759580f6eb7a11049
parent 674178 11fe0a2895aab26c57bcfe61b3041d7837e954cd
child 734447 fc31141576493234866257529442f765fe116bde
push id82963
push userbmo:jjones@mozilla.com
push dateWed, 04 Oct 2017 14:09:30 +0000
reviewersttaubert
bugs1244959
milestone58.0a1
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
dom/u2f/U2F.cpp
dom/u2f/U2F.h
dom/u2f/tests/frame_appid_facet.html
dom/u2f/tests/frame_appid_facet_subdomain.html
dom/u2f/tests/u2futil.js
--- 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);