Bug 1381126: Resume requiring WebAuthn RP ID to be a Domain String r?keeler
In
Bug 1380421 we reverted some behavior that required Web Authentication's
RP ID to be domain string to permit it to be an origin, too, for interop
testing. That is no longer needed, so this patch resumes enforcement that
RP ID be a domain string.
It also adds a needed test that the RP ID hash is calculated correctly.
MozReview-Commit-ID: 8dDjzo5kQKP
--- a/dom/webauthn/WebAuthnManager.cpp
+++ b/dom/webauthn/WebAuthnManager.cpp
@@ -2,17 +2,16 @@
/* 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 "nsNetUtil.h" // Used by WD-05 compat support (Remove in Bug 1381126)
#include "nsThreadUtils.h"
#include "mozilla/ClearOnShutdown.h"
#include "mozilla/dom/AuthenticatorAttestationResponse.h"
#include "mozilla/dom/Promise.h"
#include "mozilla/dom/WebAuthnCBORUtil.h"
#include "mozilla/dom/WebAuthnManager.h"
#include "mozilla/dom/WebAuthnUtil.h"
#include "mozilla/dom/PWebAuthnTransaction.h"
@@ -186,35 +185,17 @@ RelaxSameOrigin(nsPIDOMWindowInner* aPar
if (!document || !document->IsHTMLDocument()) {
return NS_ERROR_FAILURE;
}
nsHTMLDocument* html = document->AsHTMLDocument();
if (NS_WARN_IF(!html)) {
return NS_ERROR_FAILURE;
}
- // WD-05 origin compatibility support - aInputRpId might be a URI/origin,
- // so catch that (Bug 1380421). Remove in Bug 1381126.
- nsAutoString inputRpId(aInputRpId);
- nsCOMPtr<nsIURI> inputUri;
- if (NS_SUCCEEDED(NS_NewURI(getter_AddRefs(inputUri), aInputRpId))) {
- // If we parsed the input as a URI, then pull out the host and use it as the
- // input
- nsAutoCString uriHost;
- if (NS_FAILED(inputUri->GetHost(uriHost))) {
- return NS_ERROR_FAILURE;
- }
- CopyUTF8toUTF16(uriHost, inputRpId);
- MOZ_LOG(gWebAuthnManagerLog, LogLevel::Debug,
- ("WD-05 Fallback: Parsed input %s URI into host %s",
- NS_ConvertUTF16toUTF8(aInputRpId).get(), uriHost.get()));
- }
- // End WD-05 origin compatibility support (Bug 1380421)
-
- if (!html->IsRegistrableDomainSuffixOfOrEqualTo(inputRpId, originHost)) {
+ if (!html->IsRegistrableDomainSuffixOfOrEqualTo(aInputRpId, originHost)) {
return NS_ERROR_DOM_SECURITY_ERR;
}
aRelaxedRpId.Assign(NS_ConvertUTF16toUTF8(aInputRpId));
return NS_OK;
}
static void
@@ -486,20 +467,18 @@ WebAuthnManager::MakeCredential(nsPIDOMW
// and compute the clientDataJSON and clientDataHash.
CryptoBuffer challenge;
if (!challenge.Assign(aOptions.mChallenge)) {
promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
return promise.forget();
}
- // WD-05 vs. WD-06: In WD-06, the first parameter should be "origin". Fix
- // this in Bug 1384776
nsAutoCString clientDataJSON;
- srv = AssembleClientData(NS_ConvertUTF8toUTF16(rpId), challenge, clientDataJSON);
+ srv = AssembleClientData(origin, challenge, clientDataJSON);
if (NS_WARN_IF(NS_FAILED(srv))) {
promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
return promise.forget();
}
CryptoBuffer clientDataHash;
if (!clientDataHash.SetLength(SHA256_LENGTH, fallible)) {
promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
@@ -650,19 +629,17 @@ WebAuthnManager::GetAssertion(nsPIDOMWin
// the clientDataJSON and clientDataHash.
CryptoBuffer challenge;
if (!challenge.Assign(aOptions.mChallenge)) {
promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
return promise.forget();
}
nsAutoCString clientDataJSON;
- // WD-05 vs. WD-06: In WD-06, the first parameter should be "origin". Fix
- // this in Bug 1384776
- srv = AssembleClientData(NS_ConvertUTF8toUTF16(rpId), challenge, clientDataJSON);
+ srv = AssembleClientData(origin, challenge, clientDataJSON);
if (NS_WARN_IF(NS_FAILED(srv))) {
promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
return promise.forget();
}
CryptoBuffer clientDataHash;
if (!clientDataHash.SetLength(SHA256_LENGTH, fallible)) {
promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
--- a/dom/webauthn/tests/test_webauthn_loopback.html
+++ b/dom/webauthn/tests/test_webauthn_loopback.html
@@ -58,24 +58,32 @@ function() {
ok(aCredInfo.rawId === aCredInfo.rawId, "PublicKeyCredential.RawID is SameObject");
ok(aCredInfo.response === aCredInfo.response, "PublicKeyCredential.Response is SameObject");
ok(aCredInfo.response.clientDataJSON === aCredInfo.response.clientDataJSON, "PublicKeyCredential.Response.ClientDataJSON is SameObject");
ok(aCredInfo.response.attestationObject === aCredInfo.response.attestationObject, "PublicKeyCredential.Response.AttestationObject is SameObject");
let clientData = JSON.parse(buffer2string(aCredInfo.response.clientDataJSON));
is(clientData.challenge, bytesToBase64UrlSafe(gCredentialChallenge), "Challenge is correct");
- // WD-05 vs. WD-06: In WD-06, the second parameter should be "window.location.origin". Fix
- // this in Bug 1384776
- is(clientData.origin, document.domain, "Origin is correct");
+ is(clientData.origin, window.location.origin, "Origin is correct");
is(clientData.hashAlg, "SHA-256", "Hash algorithm is correct");
return webAuthnDecodeCBORAttestation(aCredInfo.response.attestationObject.buffer)
.then(function(decodedResult) {
- ok(decodedResult.flags == (flag_TUP | flag_AT), "User presence and Attestation Object must both be set");
+ // Make sure the RP ID hash matches what we calculate.
+ return crypto.subtle.digest("SHA-256", string2buffer(document.domain))
+ .then(function(calculatedHash) {
+ is(bytesToBase64(new Uint8Array(calculatedHash)), bytesToBase64(decodedResult.rpIdHash),
+ "Calculated RP ID hash must match what the browser derived.");
+ return Promise.resolve(decodedResult);
+ });
+ })
+ .then(function(decodedResult) {
+ ok(decodedResult.flags == (flag_TUP | flag_AT),
+ "User presence and Attestation Object must both be set");
aCredInfo.clientDataObj = clientData;
aCredInfo.publicKeyHandle = decodedResult.publicKeyHandle;
aCredInfo.attestationObject = decodedResult.attestationObject;
return aCredInfo;
});
}
@@ -96,19 +104,17 @@ function() {
is(aAssertion.id, bytesToBase64UrlSafe(aAssertion.rawId), "Encoded Key ID and Raw Key ID match");
ok(aAssertion.response.authenticatorData === aAssertion.response.authenticatorData, "AuthenticatorAssertionResponse.AuthenticatorData is SameObject");
ok(aAssertion.response.signature === aAssertion.response.signature, "AuthenticatorAssertionResponse.Signature is SameObject");
ok(aAssertion.response.authenticatorData.length > 0, "Authenticator data exists");
let clientData = JSON.parse(buffer2string(aAssertion.response.clientDataJSON));
is(clientData.challenge, bytesToBase64UrlSafe(gAssertionChallenge), "Challenge is correct");
- // WD-05 vs. WD-06: In WD-06, the second parameter should be "window.location.origin". Fix
- // this in Bug 1384776
- is(clientData.origin, document.domain, "Origin is correct");
+ is(clientData.origin, window.location.origin, "Origin is correct");
is(clientData.hashAlg, "SHA-256", "Hash algorithm is correct");
return webAuthnDecodeAttestation(aAssertion.response.authenticatorData)
.then(function(decodedResult) {
ok(decodedResult.flags == flag_TUP, "User presence must be the only flag set");
is(decodedResult.counter.length, 4, "Counter must be 4 bytes");
return deriveAppAndChallengeParam(window.location.host, aAssertion.response.clientDataJSON, decodedResult)
})
--- a/dom/webauthn/tests/test_webauthn_sameorigin.html
+++ b/dom/webauthn/tests/test_webauthn_sameorigin.html
@@ -226,40 +226,37 @@
challenge: chall,
rpId: "alt.test",
allowList: [gTrackedCredential["basic"]]
};
return credm.get({publicKey: publicKeyCredentialRequestOptions})
.then(arrivingHereIsBad)
.catch(expectSecurityError);
},
- // These next two tests should be removed in Bug 1381126.
function () {
// Test basic good Create call but using an origin (Bug 1380421)
let rp = {id: window.origin};
let makeCredentialOptions = {
rp: rp, user: user, challenge: chall, parameters: [param]
};
return credm.create({publicKey: makeCredentialOptions})
- .then(keepThisPublicKeyCredential("origin"))
- .then(arrivingHereIsGood)
- .catch(arrivingHereIsBad);
+ .then(arrivingHereIsBad)
+ .catch(expectSecurityError);
},
function () {
// Test basic good Get call but using an origin (Bug 1380421)
let publicKeyCredentialRequestOptions = {
challenge: chall,
rpId: window.origin,
- allowList: [gTrackedCredential["origin"]]
+ allowList: [gTrackedCredential["basic"]]
};
return credm.get({publicKey: publicKeyCredentialRequestOptions})
- .then(arrivingHereIsGood)
- .catch(arrivingHereIsBad);
+ .then(arrivingHereIsBad)
+ .catch(expectSecurityError);
}
- // End remove in Bug 1381126.
];
var i = 0;
var runNextTest = () => {
if (i == testFuncs.length) {
SimpleTest.finish();
return;
}
console.log(i, testFuncs[i], testFuncs.length);