diff --git a/dom/webauthn/AuthenticatorAssertionResponse.cpp b/dom/webauthn/AuthenticatorAssertionResponse.cpp index e3b91a19f57a..a3e63e2539e0 100644 --- a/dom/webauthn/AuthenticatorAssertionResponse.cpp +++ b/dom/webauthn/AuthenticatorAssertionResponse.cpp @@ -15,6 +15,7 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(AuthenticatorAssertionResponse, AuthenticatorResponse) tmp->mAuthenticatorDataCachedObj = nullptr; tmp->mSignatureCachedObj = nullptr; + tmp->mUserHandleCachedObj = nullptr; NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(AuthenticatorAssertionResponse, @@ -22,6 +23,7 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(AuthenticatorAssertionResponse, NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mAuthenticatorDataCachedObj) NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mSignatureCachedObj) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mUserHandleCachedObj) NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(AuthenticatorAssertionResponse, @@ -38,6 +40,7 @@ AuthenticatorAssertionResponse::AuthenticatorAssertionResponse(nsPIDOMWindowInne : AuthenticatorResponse(aParent) , mAuthenticatorDataCachedObj(nullptr) , mSignatureCachedObj(nullptr) + , mUserHandleCachedObj(nullptr) { mozilla::HoldJSObjects(this); } @@ -93,17 +96,21 @@ AuthenticatorAssertionResponse::SetSignature(CryptoBuffer& aBuffer) } void -AuthenticatorAssertionResponse::GetUserId(DOMString& aRetVal) +AuthenticatorAssertionResponse::GetUserHandle(JSContext* aCx, + JS::MutableHandle aRetVal) { - // This requires mUserId to not be re-set for the life of the caller's in-var. - aRetVal.SetOwnedString(mUserId); + if (!mUserHandleCachedObj) { + mUserHandleCachedObj = mUserHandle.ToArrayBuffer(aCx); + } + aRetVal.set(mUserHandleCachedObj); } nsresult -AuthenticatorAssertionResponse::SetUserId(const nsAString& aUserId) +AuthenticatorAssertionResponse::SetUserHandle(CryptoBuffer& aBuffer) { - MOZ_ASSERT(mUserId.IsEmpty(), "We already have a UserID?"); - mUserId.Assign(aUserId); + if (NS_WARN_IF(!mUserHandle.Assign(aBuffer))) { + return NS_ERROR_OUT_OF_MEMORY; + } return NS_OK; } diff --git a/dom/webauthn/AuthenticatorAssertionResponse.h b/dom/webauthn/AuthenticatorAssertionResponse.h index 1bd2ddef3637..c6fe2641a4f1 100644 --- a/dom/webauthn/AuthenticatorAssertionResponse.h +++ b/dom/webauthn/AuthenticatorAssertionResponse.h @@ -48,17 +48,18 @@ public: SetSignature(CryptoBuffer& aBuffer); void - GetUserId(DOMString& aRetVal); + GetUserHandle(JSContext* aCx, JS::MutableHandle aRetVal); nsresult - SetUserId(const nsAString& aUserId); + SetUserHandle(CryptoBuffer& aUserHandle); private: CryptoBuffer mAuthenticatorData; JS::Heap mAuthenticatorDataCachedObj; CryptoBuffer mSignature; JS::Heap mSignatureCachedObj; - nsString mUserId; + CryptoBuffer mUserHandle; + JS::Heap mUserHandleCachedObj; }; } // namespace dom diff --git a/dom/webauthn/WebAuthnManager.cpp b/dom/webauthn/WebAuthnManager.cpp index 41717a626600..7e0cb41bd06c 100644 --- a/dom/webauthn/WebAuthnManager.cpp +++ b/dom/webauthn/WebAuthnManager.cpp @@ -225,12 +225,12 @@ WebAuthnManager::MakeCredential(const MakePublicKeyCredentialOptions& aOptions, return promise.forget(); } - // Enforce 4.4.3 User Account Parameters for Credential Generation - if (aOptions.mUser.mId.WasPassed()) { - // When we add UX, we'll want to do more with this value, but for now - // we just have to verify its correctness. + // Enforce 5.4.3 User Account Parameters for Credential Generation + // When we add UX, we'll want to do more with this value, but for now + // we just have to verify its correctness. + { CryptoBuffer userId; - userId.Assign(aOptions.mUser.mId.Value()); + userId.Assign(aOptions.mUser.mId); if (userId.Length() > 64) { promise->MaybeReject(NS_ERROR_DOM_TYPE_ERR); return promise.forget(); diff --git a/dom/webauthn/tests/test_webauthn_loopback.html b/dom/webauthn/tests/test_webauthn_loopback.html index 0fb99f90cabe..fa0166bac4d4 100644 --- a/dom/webauthn/tests/test_webauthn_loopback.html +++ b/dom/webauthn/tests/test_webauthn_loopback.html @@ -107,8 +107,12 @@ function() { is(aAssertion.id, bytesToBase64UrlSafe(new Uint8Array(aAssertion.rawId)), "Encoded Key ID and Raw Key ID match"); ok(aAssertion.response.authenticatorData === aAssertion.response.authenticatorData, "AuthenticatorAssertionResponse.AuthenticatorData is SameObject"); + ok(aAssertion.response.authenticatorData instanceof ArrayBuffer, "AuthenticatorAssertionResponse.AuthenticatorData is an ArrayBuffer"); ok(aAssertion.response.signature === aAssertion.response.signature, "AuthenticatorAssertionResponse.Signature is SameObject"); - isnot(aAssertion.response.userId, undefined, "AuthenticatorAssertionResponse.UserId is defined") + ok(aAssertion.response.signature instanceof ArrayBuffer, "AuthenticatorAssertionResponse.Signature is an ArrayBuffer"); + ok(aAssertion.response.userHandle === aAssertion.response.userHandle, "AuthenticatorAssertionResponse.UserHandle is SameObject"); + ok(aAssertion.response.userHandle instanceof ArrayBuffer, "AuthenticatorAssertionResponse.UserHandle is an ArrayBuffer"); + is(aAssertion.response.userHandle.byteLength, 0, "AuthenticatorAssertionResponse.UserHandle is emtpy"); ok(aAssertion.response.authenticatorData.byteLength > 0, "Authenticator data exists"); let clientData = JSON.parse(buffer2string(aAssertion.response.clientDataJSON)); @@ -137,7 +141,7 @@ function() { function testMakeCredential() { let rp = {id: document.domain, name: "none", icon: "none"}; - let user = {name: "none", icon: "none", displayName: "none"}; + let user = {id: new Uint8Array(), name: "none", icon: "none", displayName: "none"}; let param = {type: "public-key", alg: cose_alg_ECDSA_w_SHA256}; let makeCredentialOptions = { rp: rp, @@ -156,7 +160,7 @@ function() { function testMakeDuplicate(aCredInfo) { let rp = {id: document.domain, name: "none", icon: "none"}; - let user = {name: "none", icon: "none", displayName: "none"}; + let user = {id: new Uint8Array(), name: "none", icon: "none", displayName: "none"}; let param = {type: "public-key", alg: cose_alg_ECDSA_w_SHA256}; let makeCredentialOptions = { rp: rp, diff --git a/dom/webauthn/tests/test_webauthn_make_credential.html b/dom/webauthn/tests/test_webauthn_make_credential.html index e399dbaa18ed..a64dfa4fe84a 100644 --- a/dom/webauthn/tests/test_webauthn_make_credential.html +++ b/dom/webauthn/tests/test_webauthn_make_credential.html @@ -87,6 +87,61 @@ .catch(expectTypeError); }, + // Test without rp.name + function() { + let rp = {id: document.domain, icon: "none"}; + let makeCredentialOptions = { + rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [param] + }; + return credm.create({publicKey: makeCredentialOptions}) + .then(arrivingHereIsBad) + .catch(expectTypeError); + }, + + // Test without user.id + function() { + let user = {name: "none", icon: "none", displayName: "none"}; + let makeCredentialOptions = { + rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [param] + }; + return credm.create({publicKey: makeCredentialOptions}) + .then(arrivingHereIsBad) + .catch(expectTypeError); + }, + + // Test without user.name + function() { + let user = {id: new Uint8Array(64), icon: "none", displayName: "none"}; + let makeCredentialOptions = { + rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [param] + }; + return credm.create({publicKey: makeCredentialOptions}) + .then(arrivingHereIsBad) + .catch(expectTypeError); + }, + + // Test without user.displayName + function() { + let user = {id: new Uint8Array(64), name: "none", icon: "none"}; + let makeCredentialOptions = { + rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [param] + }; + return credm.create({publicKey: makeCredentialOptions}) + .then(arrivingHereIsBad) + .catch(expectTypeError); + }, + + // Test with a user handle that exceeds the max length + function() { + let user = {id: new Uint8Array(65), name: "none", icon: "none", displayName: "none"}; + let makeCredentialOptions = { + rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [param] + }; + return credm.create({publicKey: makeCredentialOptions}) + .then(arrivingHereIsBad) + .catch(expectTypeError); + }, + // Test without any parameters; this is acceptable meaning the RP ID is // happy to take any credential type function() { diff --git a/dom/webauthn/tests/test_webauthn_no_token.html b/dom/webauthn/tests/test_webauthn_no_token.html index 34c7402fba7a..2b162c00a08a 100644 --- a/dom/webauthn/tests/test_webauthn_no_token.html +++ b/dom/webauthn/tests/test_webauthn_no_token.html @@ -44,7 +44,7 @@ function() { function testMakeCredential() { let rp = {id: document.domain, name: "none", icon: "none"}; - let user = {name: "none", icon: "none", displayName: "none"}; + let user = {id: new Uint8Array(), name: "none", icon: "none", displayName: "none"}; let param = {type: "public-key", alg: cose_alg_ECDSA_w_SHA256}; let makeCredentialOptions = { rp: rp, user: user, challenge: credentialChallenge, pubKeyCredParams: [param] diff --git a/dom/webauthn/tests/test_webauthn_sameorigin.html b/dom/webauthn/tests/test_webauthn_sameorigin.html index 007814f2fd92..75a4f8b4b131 100644 --- a/dom/webauthn/tests/test_webauthn_sameorigin.html +++ b/dom/webauthn/tests/test_webauthn_sameorigin.html @@ -25,17 +25,18 @@ function arrivingHereIsGood(aResult) { ok(true, "Good result! Received a: " + aResult); - return Promise.resolve(); } function arrivingHereIsBad(aResult) { ok(false, "Bad result! Received a: " + aResult); - return Promise.resolve(); } function expectSecurityError(aResult) { ok(aResult.toString().startsWith("SecurityError"), "Expecting a SecurityError"); - return Promise.resolve(); + } + + function expectTypeError(aResult) { + ok(aResult.toString().startsWith("TypeError"), "Expecting a TypeError"); } function keepThisPublicKeyCredential(aIdentifier) { @@ -66,7 +67,7 @@ var testFuncs = [ function() { // Test basic good call - let rp = {id: document.domain}; + let rp = {id: document.domain, name: "none"}; let makeCredentialOptions = { rp: rp, user: user, challenge: chall, pubKeyCredParams: [param] }; @@ -78,15 +79,24 @@ function() { // Test rp.id being unset let makeCredentialOptions = { - rp: {}, user: user, challenge: chall, pubKeyCredParams: [param] + rp: {name: "none"}, user: user, challenge: chall, pubKeyCredParams: [param] }; return credm.create({publicKey: makeCredentialOptions}) .then(arrivingHereIsGood) .catch(arrivingHereIsBad); }, + function() { + // Test rp.name being unset + let makeCredentialOptions = { + rp: {id: document.domain}, user: user, challenge: chall, pubKeyCredParams: [param] + }; + return credm.create({publicKey: makeCredentialOptions}) + .then(arrivingHereIsBad) + .catch(expectTypeError); + }, function() { // Test this origin with optional fields - let rp = {id: "user:pass@" + document.domain + ":8888"}; + let rp = {id: "user:pass@" + document.domain + ":8888", name: "none"}; let makeCredentialOptions = { rp: rp, user: user, challenge: chall, pubKeyCredParams: [param] }; @@ -96,7 +106,7 @@ }, function() { // Test blank rp.id - let rp = {id: ""}; + let rp = {id: "", name: "none"}; let makeCredentialOptions = { rp: rp, user: user, challenge: chall, pubKeyCredParams: [param] }; @@ -106,7 +116,7 @@ }, function() { // Test subdomain of this origin - let rp = {id: "subdomain." + document.domain}; + let rp = {id: "subdomain." + document.domain, name: "none"}; let makeCredentialOptions = { rp: rp, user: user, challenge: chall, pubKeyCredParams: [param] }; @@ -116,7 +126,7 @@ }, function() { // Test the same origin - let rp = {id: "example.com"}; + let rp = {id: "example.com", name: "none"}; let makeCredentialOptions = { rp: rp, user: user, challenge: chall, pubKeyCredParams: [param] }; @@ -126,7 +136,7 @@ }, function() { // Test the eTLD - let rp = {id: "com"}; + let rp = {id: "com", name: "none"}; let makeCredentialOptions = { rp: rp, user: user, challenge: chall, pubKeyCredParams: [param] }; @@ -136,7 +146,7 @@ }, function () { // Test a different domain within the same TLD - let rp = {id: "alt.test"}; + let rp = {id: "alt.test", name: "none"}; let makeCredentialOptions = { rp: rp, user: user, challenge: chall, pubKeyCredParams: [param] }; @@ -233,7 +243,7 @@ }, function () { // Test basic good Create call but using an origin (Bug 1380421) - let rp = {id: window.origin}; + let rp = {id: window.origin, name: "none"}; let makeCredentialOptions = { rp: rp, user: user, challenge: chall, pubKeyCredParams: [param] }; diff --git a/dom/webidl/WebAuthentication.webidl b/dom/webidl/WebAuthentication.webidl index 0f6f6c9b4319..8bf7f49c25b5 100644 --- a/dom/webidl/WebAuthentication.webidl +++ b/dom/webidl/WebAuthentication.webidl @@ -36,7 +36,7 @@ interface AuthenticatorAttestationResponse : AuthenticatorResponse { interface AuthenticatorAssertionResponse : AuthenticatorResponse { [SameObject] readonly attribute ArrayBuffer authenticatorData; [SameObject] readonly attribute ArrayBuffer signature; - readonly attribute DOMString userId; + [SameObject] readonly attribute ArrayBuffer userHandle; }; dictionary PublicKeyCredentialParameters { @@ -59,8 +59,8 @@ dictionary MakePublicKeyCredentialOptions { }; dictionary PublicKeyCredentialEntity { - DOMString name; - USVString icon; + required DOMString name; + USVString icon; }; dictionary PublicKeyCredentialRpEntity : PublicKeyCredentialEntity { @@ -68,8 +68,8 @@ dictionary PublicKeyCredentialRpEntity : PublicKeyCredentialEntity { }; dictionary PublicKeyCredentialUserEntity : PublicKeyCredentialEntity { - BufferSource id; - DOMString displayName; + required BufferSource id; + required DOMString displayName; }; dictionary AuthenticatorSelectionCriteria { @@ -115,9 +115,9 @@ enum PublicKeyCredentialType { }; dictionary PublicKeyCredentialDescriptor { - required PublicKeyCredentialType type; - required BufferSource id; - sequence transports; + required PublicKeyCredentialType type; + required BufferSource id; + sequence transports; }; enum AuthenticatorTransport {