diff --git a/dom/webauthn/WebAuthnManager.cpp b/dom/webauthn/WebAuthnManager.cpp index 4c5ebaa580d3..cc80276eef06 100644 --- a/dom/webauthn/WebAuthnManager.cpp +++ b/dom/webauthn/WebAuthnManager.cpp @@ -26,6 +26,8 @@ #include "mozilla/dom/WindowGlobalChild.h" #include "mozilla/ipc/BackgroundChild.h" #include "mozilla/ipc/PBackgroundChild.h" +#include "mozilla/JSONStringWriteFuncs.h" +#include "mozilla/JSONWriter.h" #ifdef XP_WIN # include "WinWebAuthnService.h" @@ -65,28 +67,35 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END static nsresult AssembleClientData( const nsAString& aOrigin, const CryptoBuffer& aChallenge, - const nsAString& aType, + const nsACString& aType, const AuthenticationExtensionsClientInputs& aExtensions, /* out */ nsACString& aJsonOut) { MOZ_ASSERT(NS_IsMainThread()); - nsString challengeBase64; - nsresult rv = aChallenge.ToJwkBase64(challengeBase64); + nsAutoCString challengeBase64; + nsresult rv = + Base64URLEncode(aChallenge.Length(), aChallenge.Elements(), + Base64URLEncodePaddingPolicy::Omit, challengeBase64); if (NS_WARN_IF(NS_FAILED(rv))) { return NS_ERROR_FAILURE; } - CollectedClientData clientDataObject; - clientDataObject.mType.Assign(aType); - clientDataObject.mChallenge.Assign(challengeBase64); - clientDataObject.mOrigin.Assign(aOrigin); + // Serialize the collected client data using the algorithm from + // https://www.w3.org/TR/webauthn-3/#clientdatajson-serialization. + // Please update the definition of CollectedClientData in + // dom/webidl/WebAuthentication.webidl when changes are made here. + JSONStringRefWriteFunc f(aJsonOut); + JSONWriter w(f, JSONWriter::CollectionStyle::SingleLineStyle); + w.Start(); + // Steps 2 and 3 + w.StringProperty("type", aType); + // Steps 4 and 5 + w.StringProperty("challenge", challengeBase64); + // Steps 6 and 7 + w.StringProperty("origin", NS_ConvertUTF16toUTF8(aOrigin)); + // Steps 8 through 11 will be implemented in Bug 1901809. + w.End(); - nsAutoString temp; - if (NS_WARN_IF(!clientDataObject.ToJSON(temp))) { - return NS_ERROR_FAILURE; - } - - aJsonOut.Assign(NS_ConvertUTF16toUTF8(temp)); return NS_OK; } @@ -347,7 +356,7 @@ already_AddRefed WebAuthnManager::MakeCredential( } nsAutoCString clientDataJSON; - nsresult srv = AssembleClientData(origin, challenge, u"webauthn.create"_ns, + nsresult srv = AssembleClientData(origin, challenge, "webauthn.create"_ns, aOptions.mExtensions, clientDataJSON); if (NS_WARN_IF(NS_FAILED(srv))) { promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR); @@ -557,7 +566,7 @@ already_AddRefed WebAuthnManager::GetAssertion( } nsAutoCString clientDataJSON; - rv = AssembleClientData(origin, challenge, u"webauthn.get"_ns, + rv = AssembleClientData(origin, challenge, "webauthn.get"_ns, aOptions.mExtensions, clientDataJSON); if (NS_WARN_IF(NS_FAILED(rv))) { promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR); diff --git a/dom/webauthn/tests/test_webauthn_loopback.html b/dom/webauthn/tests/test_webauthn_loopback.html index 70905739a093..b918b4922f54 100644 --- a/dom/webauthn/tests/test_webauthn_loopback.html +++ b/dom/webauthn/tests/test_webauthn_loopback.html @@ -58,10 +58,14 @@ add_task(async function() { 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"); + let clientDataJSON = buffer2string(aCredInfo.response.clientDataJSON); + let clientData = JSON.parse(clientDataJSON); + let challengeB64 = bytesToBase64UrlSafe(gCredentialChallenge); + let expectedClientDataJSON = '{"type":"webauthn.create","challenge":"'+challengeB64+'","origin":"'+window.location.origin+'"}'; + is(clientData.challenge, challengeB64, "Challenge is correct"); is(clientData.origin, window.location.origin, "Origin is correct"); is(clientData.type, "webauthn.create", "Type is correct"); + is(clientDataJSON, expectedClientDataJSON, "clientData is in the correct order for limited verification"); let attestationObj = await webAuthnDecodeCBORAttestation(aCredInfo.response.attestationObject); // Make sure the RP ID hash matches what we calculate. diff --git a/dom/webidl/WebAuthentication.webidl b/dom/webidl/WebAuthentication.webidl index ca20b387cdbd..a446c25b43b3 100644 --- a/dom/webidl/WebAuthentication.webidl +++ b/dom/webidl/WebAuthentication.webidl @@ -212,18 +212,18 @@ dictionary AuthenticationExtensionsClientOutputs { typedef record AuthenticationExtensionsAuthenticatorInputs; -[GenerateToJSON] -dictionary CollectedClientData { - required DOMString type; - required DOMString challenge; - required DOMString origin; - TokenBinding tokenBinding; -}; - -dictionary TokenBinding { - required DOMString status; - DOMString id; -}; +// The CollectedClientData dictionary must be serialized using the algorithm +// from https://w3c.github.io/webauthn/#clientdatajson-serialization. Because +// CollectedClientData is only consumed by the relying party, and because +// [GenerateToJSON] does not produce the correct serialization algorithm, the +// definition below is commented out. Please keep this definition in sync with +// in AssembleClientData in dom/webauthn/WebAuthnManager.cpp. +// +// dictionary CollectedClientData { +// required DOMString type; +// required DOMString challenge; +// required DOMString origin; +// }; dictionary PublicKeyCredentialDescriptor { required DOMString type;