Bug 1463170 - Set AuthenticatorAssertionResponse.userHandle to null. r=ttaubert, r=smaug, a=RyanVM
authorJ.C. Jones <jjones@mozilla.com>
Mon, 21 May 2018 09:04:50 -0700
changeset 802273 69ca265a87712c4192be6f84f12df2daee5ab5c8
parent 802272 12bf945bd10987f92e8c57d0911895ab0cdb26c8
child 802274 7f9fbb6a71d25fd88202440ed96915f8df868a49
push id111850
push userbmo:tom@mozilla.com
push dateThu, 31 May 2018 16:41:37 +0000
reviewersttaubert, smaug, RyanVM
bugs1463170
milestone60.0.2
Bug 1463170 - Set AuthenticatorAssertionResponse.userHandle to null. r=ttaubert, r=smaug, a=RyanVM Summary: The WebAuthn spec says to set `AuthenticatorAssertionResponse.userHandle` to null when the authenticator returns no user handle (e.g., when allowList is set), but we return an empty ArrayBuffer. This is because of the defaults in AuthenticatorAssertionResponse.h, as the field is itself unset. We missed this change to the spec that happened in December [2], so this also has a corresponding WebIDL update. I don't see any other instances of WebIDL differences. [1] https://w3c.github.io/webauthn/#ref-for-dom-authenticatorassertionresponse-userhandle%E2%91%A0 [2] https://github.com/w3c/webauthn/commit/3b2a1d141cbd8f2954f073a6b6598d954398a986 Test Plan: https://treeherder.mozilla.org/#/jobs?repo=try&revision=59a2ab255ef14e935c1aa9f457276f8e61e5d779 Reviewers: smaug, ttaubert Bug #: 1463170 Differential Revision: https://phabricator.services.mozilla.com/D1337
dom/webauthn/AuthenticatorAssertionResponse.cpp
dom/webauthn/tests/test_webauthn_loopback.html
dom/webidl/WebAuthentication.webidl
--- a/dom/webauthn/AuthenticatorAssertionResponse.cpp
+++ b/dom/webauthn/AuthenticatorAssertionResponse.cpp
@@ -94,20 +94,26 @@ AuthenticatorAssertionResponse::SetSigna
   }
   return NS_OK;
 }
 
 void
 AuthenticatorAssertionResponse::GetUserHandle(JSContext* aCx,
                                               JS::MutableHandle<JSObject*> aRetVal)
 {
-  if (!mUserHandleCachedObj) {
-    mUserHandleCachedObj = mUserHandle.ToArrayBuffer(aCx);
+  // Per https://w3c.github.io/webauthn/#ref-for-dom-authenticatorassertionresponse-userhandle%E2%91%A0
+  // this should return null if the handle is unset.
+  if (mUserHandle.IsEmpty()) {
+    aRetVal.set(nullptr);
+  } else {
+    if (!mUserHandleCachedObj) {
+      mUserHandleCachedObj = mUserHandle.ToArrayBuffer(aCx);
+    }
+    aRetVal.set(mUserHandleCachedObj);
   }
-  aRetVal.set(mUserHandleCachedObj);
 }
 
 nsresult
 AuthenticatorAssertionResponse::SetUserHandle(CryptoBuffer& aBuffer)
 {
   if (NS_WARN_IF(!mUserHandle.Assign(aBuffer))) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
--- a/dom/webauthn/tests/test_webauthn_loopback.html
+++ b/dom/webauthn/tests/test_webauthn_loopback.html
@@ -111,19 +111,17 @@ function() {
 
     ok(aAssertion.rawId.byteLength > 0, "Key ID exists");
     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");
     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.userHandle === null, "AuthenticatorAssertionResponse.UserHandle is null for u2f authenticators");
 
     ok(aAssertion.response.authenticatorData.byteLength > 0, "Authenticator data exists");
     let clientData = JSON.parse(buffer2string(aAssertion.response.clientDataJSON));
     is(clientData.challenge, bytesToBase64UrlSafe(gAssertionChallenge), "Challenge is correct");
     is(clientData.origin, window.location.origin, "Origin is correct");
     is(clientData.hashAlgorithm, "SHA-256", "Hash algorithm is correct");
     is(clientData.type, "webauthn.get", "Type is correct");
 
--- a/dom/webidl/WebAuthentication.webidl
+++ b/dom/webidl/WebAuthentication.webidl
@@ -30,17 +30,17 @@ interface AuthenticatorResponse {
 interface AuthenticatorAttestationResponse : AuthenticatorResponse {
     [SameObject] readonly attribute ArrayBuffer attestationObject;
 };
 
 [SecureContext, Pref="security.webauth.webauthn"]
 interface AuthenticatorAssertionResponse : AuthenticatorResponse {
     [SameObject] readonly attribute ArrayBuffer      authenticatorData;
     [SameObject] readonly attribute ArrayBuffer      signature;
-    [SameObject] readonly attribute ArrayBuffer      userHandle;
+    [SameObject] readonly attribute ArrayBuffer?     userHandle;
 };
 
 dictionary PublicKeyCredentialParameters {
     required PublicKeyCredentialType  type;
     required COSEAlgorithmIdentifier  alg;
 };
 
 dictionary PublicKeyCredentialCreationOptions {