Bug 1407093 - Web Authentication - WD-07 updates for user handles r=jcj,smaug

Reviewers: jcj, smaug

Reviewed By: jcj, smaug

Bug #: 1407093

Differential Revision: https://phabricator.services.mozilla.com/D328

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<JSObject*> 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;
 }
 

dom/webauthn/AuthenticatorAssertionResponse.h

@@ -48,17 +48,18 @@ public:
   SetSignature(CryptoBuffer& aBuffer);
 
   void
-  GetUserId(DOMString& aRetVal);
+  GetUserHandle(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal);
 
   nsresult
-  SetUserId(const nsAString& aUserId);
+  SetUserHandle(CryptoBuffer& aUserHandle);
 
 private:
   CryptoBuffer mAuthenticatorData;
   JS::Heap<JSObject*> mAuthenticatorDataCachedObj;
   CryptoBuffer mSignature;
   JS::Heap<JSObject*> mSignatureCachedObj;
-  nsString mUserId;
+  CryptoBuffer mUserHandle;
+  JS::Heap<JSObject*> mUserHandleCachedObj;
 };
 
 } // namespace dom

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()) {
+  // 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();

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,

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() {

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]

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]
           };

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,7 +59,7 @@ dictionary MakePublicKeyCredentialOptions {
 };
 
 dictionary PublicKeyCredentialEntity {
-    DOMString      name;
+    required DOMString    name;
     USVString             icon;
 };
 
@@ -68,8 +68,8 @@ dictionary PublicKeyCredentialRpEntity : PublicKeyCredentialEntity {
 };
 
 dictionary PublicKeyCredentialUserEntity : PublicKeyCredentialEntity {
-    BufferSource   id;
-    DOMString      displayName;
+    required BufferSource   id;
+    required DOMString      displayName;
 };
 
 dictionary AuthenticatorSelectionCriteria {