Bug 1329764 - WebAuthn's RP IDs must be domain strings r?keeler draft
authorJ.C. Jones <jjones@mozilla.com>
Fri, 07 Jul 2017 13:30:57 -0700
changeset 605499 8f77d007cd42eed3a43bd9308f60ced20cda9de3
parent 605055 20f32734df750bddada9d1edca665c2ea53946f0
child 605500 3477552c6b98ecd70a59439f4b898422370f0cc1
push id67424
push userbmo:jjones@mozilla.com
push dateFri, 07 Jul 2017 20:34:04 +0000
reviewerskeeler
bugs1329764
milestone56.0a1
Bug 1329764 - WebAuthn's RP IDs must be domain strings r?keeler The spec for WebAuthn defines "RP ID" as a "valid domain string" [1], whereas we were using an origin string (with the scheme and whatnot). This patch corrects the default rpId strings (when not overriden) to be domain strings. [1] https://w3c.github.io/webauthn/#rp-id MozReview-Commit-ID: 2p1cEQDa2FV
dom/webauthn/WebAuthnManager.cpp
dom/webauthn/tests/test_webauthn_loopback.html
--- a/dom/webauthn/WebAuthnManager.cpp
+++ b/dom/webauthn/WebAuthnManager.cpp
@@ -1,28 +1,28 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* 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 "hasht.h"
+#include "nsICryptoHash.h"
 #include "nsNetCID.h"
-#include "nsICryptoHash.h"
+#include "nsThreadUtils.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/dom/AuthenticatorAttestationResponse.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/WebAuthnManager.h"
 #include "mozilla/dom/WebAuthnUtil.h"
 #include "mozilla/dom/PWebAuthnTransaction.h"
 #include "mozilla/dom/WebAuthnTransactionChild.h"
 #include "mozilla/dom/WebCryptoCommon.h"
 #include "mozilla/ipc/PBackgroundChild.h"
 #include "mozilla/ipc/BackgroundChild.h"
-#include "nsThreadUtils.h"
 
 using namespace mozilla::ipc;
 
 namespace mozilla {
 namespace dom {
 
 /***********************************************************************
  * Statics
@@ -112,36 +112,44 @@ AssembleClientData(const nsAString& aOri
   }
 
   aJsonOut.Assign(NS_ConvertUTF16toUTF8(temp));
   return NS_OK;
 }
 
 nsresult
 GetOrigin(nsPIDOMWindowInner* aParent,
-          /*out*/ nsAString& aOrigin)
+          /*out*/ nsAString& aOrigin, /*out*/ nsACString& aHost)
 {
   nsCOMPtr<nsIDocument> doc = aParent->GetDoc();
   MOZ_ASSERT(doc);
 
-  nsIPrincipal* principal = doc->NodePrincipal();
+  nsCOMPtr<nsIPrincipal> principal = doc->NodePrincipal();
   nsresult rv = nsContentUtils::GetUTFOrigin(principal, aOrigin);
   if (NS_WARN_IF(NS_FAILED(rv)) ||
       NS_WARN_IF(aOrigin.IsEmpty())) {
     return NS_ERROR_FAILURE;
   }
 
   if (aOrigin.EqualsLiteral("null")) {
     // 4.1.1.3 If callerOrigin is an opaque origin, reject promise with a
     // DOMException whose name is "NotAllowedError", and terminate this
     // algorithm
     MOZ_LOG(gWebAuthnManagerLog, LogLevel::Debug, ("Rejecting due to opaque origin"));
     return NS_ERROR_DOM_NOT_ALLOWED_ERR;
   }
 
+  nsCOMPtr<nsIURI> originUri;
+  if (NS_FAILED(principal->GetURI(getter_AddRefs(originUri)))) {
+    return NS_ERROR_FAILURE;
+  }
+  if (NS_FAILED(originUri->GetAsciiHost(aHost))) {
+    return NS_ERROR_FAILURE;
+  }
+
   return NS_OK;
 }
 
 nsresult
 RelaxSameOrigin(nsPIDOMWindowInner* aParent,
                 const nsAString& aInputRpId,
                 /* out */ nsACString& aRelaxedRpId)
 {
@@ -227,39 +235,35 @@ WebAuthnManager::MakeCredential(nsPIDOMW
 
   ErrorResult rv;
   RefPtr<Promise> promise = Promise::Create(global, rv);
   if(rv.Failed()) {
     return nullptr;
   }
 
   nsString origin;
-  rv = GetOrigin(aParent, origin);
+  nsCString rpId;
+  rv = GetOrigin(aParent, origin, rpId);
   if (NS_WARN_IF(rv.Failed())) {
     promise->MaybeReject(rv);
     return promise.forget();
   }
 
   // If timeoutSeconds was specified, check if its value lies within a
   // reasonable range as defined by the platform and if not, correct it to the
   // closest value lying within that range.
 
   double adjustedTimeout = 30.0;
   if (aOptions.mTimeout.WasPassed()) {
     adjustedTimeout = aOptions.mTimeout.Value();
     adjustedTimeout = std::max(15.0, adjustedTimeout);
     adjustedTimeout = std::min(120.0, adjustedTimeout);
   }
 
-  nsCString rpId;
-  if (!aOptions.mRp.mId.WasPassed()) {
-    // If rp.id is not specified, then set rpId to callerOrigin, and rpIdHash to
-    // the SHA-256 hash of rpId.
-    rpId.Assign(NS_ConvertUTF16toUTF8(origin));
-  } else {
+  if (aOptions.mRp.mId.WasPassed()) {
     // If rpId is specified, then invoke the procedure used for relaxing the
     // same-origin restriction by setting the document.domain attribute, using
     // rpId as the given value but without changing the current document’s
     // domain. If no errors are thrown, set rpId to the value of host as
     // computed by this procedure, and rpIdHash to the SHA-256 hash of rpId.
     // Otherwise, reject promise with a DOMException whose name is
     // "SecurityError", and terminate this algorithm.
 
@@ -467,39 +471,35 @@ WebAuthnManager::GetAssertion(nsPIDOMWin
 
   ErrorResult rv;
   RefPtr<Promise> promise = Promise::Create(global, rv);
   if(rv.Failed()) {
     return nullptr;
   }
 
   nsString origin;
-  rv = GetOrigin(aParent, origin);
+  nsCString rpId;
+  rv = GetOrigin(aParent, origin, rpId);
   if (NS_WARN_IF(rv.Failed())) {
     promise->MaybeReject(rv);
     return promise.forget();
   }
 
   // If timeoutSeconds was specified, check if its value lies within a
   // reasonable range as defined by the platform and if not, correct it to the
   // closest value lying within that range.
 
   uint32_t adjustedTimeout = 30000;
   if (aOptions.mTimeout.WasPassed()) {
     adjustedTimeout = aOptions.mTimeout.Value();
     adjustedTimeout = std::max(15000u, adjustedTimeout);
     adjustedTimeout = std::min(120000u, adjustedTimeout);
   }
 
-  nsCString rpId;
-  if (!aOptions.mRpId.WasPassed()) {
-    // If rpId is not specified, then set rpId to callerOrigin, and rpIdHash to
-    // the SHA-256 hash of rpId.
-    rpId.Assign(NS_ConvertUTF16toUTF8(origin));
-  } else {
+  if (aOptions.mRpId.WasPassed()) {
     // If rpId is specified, then invoke the procedure used for relaxing the
     // same-origin restriction by setting the document.domain attribute, using
     // rpId as the given value but without changing the current document’s
     // domain. If no errors are thrown, set rpId to the value of host as
     // computed by this procedure, and rpIdHash to the SHA-256 hash of rpId.
     // Otherwise, reject promise with a DOMException whose name is
     // "SecurityError", and terminate this algorithm.
 
--- a/dom/webauthn/tests/test_webauthn_loopback.html
+++ b/dom/webauthn/tests/test_webauthn_loopback.html
@@ -85,17 +85,17 @@ function() {
       throw "User presence byte not set";
     }
     let presenceAndCounter = aAssertion.response.signature.slice(0,5);
     let signatureValue = aAssertion.response.signature.slice(5);
 
     let rpIdHash = aAssertion.response.authenticatorData.slice(0,32);
 
     // Assemble the signed data and verify the signature
-    return deriveAppAndChallengeParam(clientData.origin, aAssertion.response.clientDataJSON)
+    return deriveAppAndChallengeParam(window.location.host, aAssertion.response.clientDataJSON)
     .then(function(aParams) {
       console.log(aParams.appParam, rpIdHash, presenceAndCounter, aParams.challengeParam);
       console.log("ClientData buffer: ", hexEncode(aAssertion.response.clientDataJSON));
       console.log("ClientDataHash: ", hexEncode(aParams.challengeParam));
       return assembleSignedData(aParams.appParam, presenceAndCounter, aParams.challengeParam);
     })
     .then(function(aSignedData) {
       console.log(aPublicKey, aSignedData, signatureValue);