Bug 1407093 - Web Authentication - WD-07 updates for user handles r=jcj,smaug
authorTim Taubert <ttaubert@mozilla.com>
Wed, 13 Dec 2017 11:15:16 +0100
changeset 396202 1343829c156774a621b2164be5462eed31475a79
parent 396201 286873a5be9b2ee3d2ddcf587b3c06af49e8da29
child 396203 26cc9645024af9153a63f6aa914c2d81c49f3bd4
push id33085
push userncsoregi@mozilla.com
push dateWed, 13 Dec 2017 22:02:06 +0000
treeherdermozilla-central@22d2831cc1f4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjcj, smaug
bugs1407093
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
dom/webauthn/AuthenticatorAssertionResponse.h
dom/webauthn/WebAuthnManager.cpp
dom/webauthn/tests/test_webauthn_loopback.html
dom/webauthn/tests/test_webauthn_make_credential.html
dom/webauthn/tests/test_webauthn_no_token.html
dom/webauthn/tests/test_webauthn_sameorigin.html
dom/webidl/WebAuthentication.webidl
--- a/dom/webauthn/AuthenticatorAssertionResponse.cpp
+++ b/dom/webauthn/AuthenticatorAssertionResponse.cpp
@@ -10,39 +10,42 @@
 namespace mozilla {
 namespace dom {
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(AuthenticatorAssertionResponse)
 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,
                                                AuthenticatorResponse)
   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,
                                                   AuthenticatorResponse)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_ADDREF_INHERITED(AuthenticatorAssertionResponse, AuthenticatorResponse)
 NS_IMPL_RELEASE_INHERITED(AuthenticatorAssertionResponse, AuthenticatorResponse)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(AuthenticatorAssertionResponse)
 NS_INTERFACE_MAP_END_INHERITING(AuthenticatorResponse)
 
 AuthenticatorAssertionResponse::AuthenticatorAssertionResponse(nsPIDOMWindowInner* aParent)
   : AuthenticatorResponse(aParent)
   , mAuthenticatorDataCachedObj(nullptr)
   , mSignatureCachedObj(nullptr)
+  , mUserHandleCachedObj(nullptr)
 {
   mozilla::HoldJSObjects(this);
 }
 
 AuthenticatorAssertionResponse::~AuthenticatorAssertionResponse()
 {
   mozilla::DropJSObjects(this);
 }
@@ -88,24 +91,28 @@ AuthenticatorAssertionResponse::SetSigna
 {
   if (NS_WARN_IF(!mSignature.Assign(aBuffer))) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
   return NS_OK;
 }
 
 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;
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/webauthn/AuthenticatorAssertionResponse.h
+++ b/dom/webauthn/AuthenticatorAssertionResponse.h
@@ -43,25 +43,26 @@ public:
 
   void
   GetSignature(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal);
 
   nsresult
   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
 } // namespace mozilla
 
 #endif // mozilla_dom_AuthenticatorAssertionResponse_h
--- a/dom/webauthn/WebAuthnManager.cpp
+++ b/dom/webauthn/WebAuthnManager.cpp
@@ -220,22 +220,22 @@ WebAuthnManager::MakeCredential(const Ma
   nsString origin;
   nsCString rpId;
   rv = GetOrigin(mParent, origin, rpId);
   if (NS_WARN_IF(rv.Failed())) {
     promise->MaybeReject(rv);
     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();
     }
   }
 
   // If timeoutSeconds was specified, check if its value lies within a
   // reasonable range as defined by the platform and if not, correct it to the
--- a/dom/webauthn/tests/test_webauthn_loopback.html
+++ b/dom/webauthn/tests/test_webauthn_loopback.html
@@ -102,18 +102,22 @@ function() {
     */
 
     is(aAssertion.type, "public-key", "Credential type must be public-key")
 
     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");
-    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));
     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");
 
     return webAuthnDecodeAuthDataArray(aAssertion.response.authenticatorData)
@@ -132,17 +136,17 @@ function() {
     .then(function(aSignedData) {
       console.log(aPublicKey, aSignedData, aAssertion.response.signature);
       return verifySignature(aPublicKey, aSignedData, aAssertion.response.signature);
     })
   }
 
   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: gCredentialChallenge,
       pubKeyCredParams: [param]
     };
     credm.create({publicKey: makeCredentialOptions})
@@ -151,17 +155,17 @@ function() {
     .catch(function(aReason) {
       ok(false, aReason);
       SimpleTest.finish();
     });
   }
 
   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,
       user: user,
       challenge: gCredentialChallenge,
       pubKeyCredParams: [param],
       excludeCredentials: [{type: "public-key", id: new Uint8Array(aCredInfo.rawId),
                      transports: ["usb"]}]
--- a/dom/webauthn/tests/test_webauthn_make_credential.html
+++ b/dom/webauthn/tests/test_webauthn_make_credential.html
@@ -82,16 +82,71 @@
           let makeCredentialOptions = {
             challenge: gCredentialChallenge, pubKeyCredParams: [param]
           };
           return credm.create({publicKey: makeCredentialOptions})
                       .then(arrivingHereIsBad)
                       .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() {
           let makeCredentialOptions = {
             rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: []
           };
           return credm.create({publicKey: makeCredentialOptions})
                       .then(arrivingHereIsGood)
--- a/dom/webauthn/tests/test_webauthn_no_token.html
+++ b/dom/webauthn/tests/test_webauthn_no_token.html
@@ -39,17 +39,17 @@ function() {
   window.crypto.getRandomValues(assertionChallenge);
   let credentialId = new Uint8Array(128);
   window.crypto.getRandomValues(credentialId);
 
   testMakeCredential();
 
   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]
     };
     credm.create({publicKey: makeCredentialOptions})
     .then(function(aResult) {
       ok(false, "Should have failed.");
       testAssertion();
--- a/dom/webauthn/tests/test_webauthn_sameorigin.html
+++ b/dom/webauthn/tests/test_webauthn_sameorigin.html
@@ -20,27 +20,28 @@
 
     // Execute the full-scope test
     SimpleTest.waitForExplicitFinish();
 
     var gTrackedCredential = {};
 
     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) {
       return function(aPublicKeyCredential) {
         gTrackedCredential[aIdentifier] = {
           type: "public-key",
           id: new Uint8Array(aPublicKeyCredential.rawId),
           transports: [ "usb" ],
@@ -61,87 +62,96 @@
       window.crypto.getRandomValues(chall);
 
       let user = {id: new Uint8Array(16), name: "none", icon: "none", displayName: "none"};
       let param = {type: "public-key", alg: cose_alg_ECDSA_w_SHA256};
 
       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]
           };
           return credm.create({publicKey: makeCredentialOptions})
                       .then(keepThisPublicKeyCredential("basic"))
                       .then(arrivingHereIsGood)
                       .catch(arrivingHereIsBad);
         },
         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]
           };
           return credm.create({publicKey: makeCredentialOptions})
                       .then(arrivingHereIsBad)
                       .catch(expectSecurityError);
         },
         function() {
           // Test blank rp.id
-          let rp = {id: ""};
+          let rp = {id: "", name: "none"};
           let makeCredentialOptions = {
             rp: rp, user: user, challenge: chall, pubKeyCredParams: [param]
           };
           return credm.create({publicKey: makeCredentialOptions})
                       .then(arrivingHereIsBad)
                       .catch(expectSecurityError);
         },
         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]
           };
           return credm.create({publicKey: makeCredentialOptions})
                       .then(arrivingHereIsBad)
                       .catch(expectSecurityError);
         },
         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]
           };
           return credm.create({publicKey: makeCredentialOptions})
                       .then(arrivingHereIsGood)
                       .catch(arrivingHereIsBad);
         },
         function() {
           // Test the eTLD
-          let rp = {id: "com"};
+          let rp = {id: "com", name: "none"};
           let makeCredentialOptions = {
             rp: rp, user: user, challenge: chall, pubKeyCredParams: [param]
           };
           return credm.create({publicKey: makeCredentialOptions})
                       .then(arrivingHereIsBad)
                       .catch(expectSecurityError);
         },
         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]
           };
           return credm.create({publicKey: makeCredentialOptions})
                       .then(arrivingHereIsBad)
                       .catch(expectSecurityError);
         },
         function () {
@@ -228,17 +238,17 @@
             allowCredentials: [gTrackedCredential["basic"]]
           };
           return credm.get({publicKey: publicKeyCredentialRequestOptions})
                       .then(arrivingHereIsBad)
                       .catch(expectSecurityError);
         },
         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]
           };
           return credm.create({publicKey: makeCredentialOptions})
                       .then(arrivingHereIsBad)
                       .catch(expectSecurityError);
         },
         function () {
--- a/dom/webidl/WebAuthentication.webidl
+++ b/dom/webidl/WebAuthentication.webidl
@@ -31,17 +31,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;
-    readonly attribute DOMString                     userId;
+    [SameObject] readonly attribute ArrayBuffer      userHandle;
 };
 
 dictionary PublicKeyCredentialParameters {
     required PublicKeyCredentialType  type;
     required COSEAlgorithmIdentifier  alg;
 };
 
 dictionary MakePublicKeyCredentialOptions {
@@ -54,27 +54,27 @@ dictionary MakePublicKeyCredentialOption
     unsigned long                                timeout;
     sequence<PublicKeyCredentialDescriptor>      excludeCredentials = [];
     AuthenticatorSelectionCriteria               authenticatorSelection;
     // Extensions are not supported yet.
     // AuthenticationExtensions                  extensions; // Add in Bug 1406458
 };
 
 dictionary PublicKeyCredentialEntity {
-    DOMString      name;
-    USVString      icon;
+    required DOMString    name;
+    USVString             icon;
 };
 
 dictionary PublicKeyCredentialRpEntity : PublicKeyCredentialEntity {
     DOMString      id;
 };
 
 dictionary PublicKeyCredentialUserEntity : PublicKeyCredentialEntity {
-    BufferSource   id;
-    DOMString      displayName;
+    required BufferSource   id;
+    required DOMString      displayName;
 };
 
 dictionary AuthenticatorSelectionCriteria {
     AuthenticatorAttachment      authenticatorAttachment;
     boolean                      requireResidentKey = false;
     UserVerificationRequirement  userVerification = "preferred";
 };
 
@@ -110,19 +110,19 @@ dictionary CollectedClientData {
     // AuthenticationExtensions     authenticatorExtensions; // Add in Bug 1406458
 };
 
 enum PublicKeyCredentialType {
     "public-key"
 };
 
 dictionary PublicKeyCredentialDescriptor {
-    required PublicKeyCredentialType type;
-    required BufferSource id;
-    sequence<AuthenticatorTransport>   transports;
+    required PublicKeyCredentialType      type;
+    required BufferSource                 id;
+    sequence<AuthenticatorTransport>      transports;
 };
 
 enum AuthenticatorTransport {
     "usb",
     "nfc",
     "ble"
 };