Bug 1406468 - Web Authentication - Implement isUserVerifyingPlatformAuthenticatorAvailable() method r=jcj,smaug
authorTim Taubert <ttaubert@mozilla.com>
Tue, 14 Nov 2017 11:44:46 +0100
changeset 436229 1ddfda4ef74f39e2558220e901ed51a665a66733
parent 436228 734949a8f6d1c77b36c8a455346a32afa0610af3
child 436230 6fab343d63b66b26b0f42f2c317a3279257f31d9
push id117
push userfmarier@mozilla.com
push dateTue, 28 Nov 2017 20:17:16 +0000
reviewersjcj, smaug
bugs1406468
milestone59.0a1
Bug 1406468 - Web Authentication - Implement isUserVerifyingPlatformAuthenticatorAvailable() method r=jcj,smaug Summary: We currently implement no platform authenticators, so this would always resolve to false. For those cases, the spec recommends a resolve timeout on the order of 10 minutes to avoid fingerprinting. A simple solution is thus to never resolve the promise, otherwise we'd have to track every single call to this method along with a promise and timer to resolve it after exactly X minutes. A Relying Party has to deal with a non-response in a timely fashion, so we can keep this as-is (and not resolve) even when we support platform authenticators but they're not available, or a user rejects a website's request to use them. Reviewers: jcj, smaug Reviewed By: jcj, smaug Bug #: 1406468 Differential Revision: https://phabricator.services.mozilla.com/D217
dom/webauthn/PublicKeyCredential.cpp
dom/webauthn/PublicKeyCredential.h
dom/webauthn/tests/test_webauthn_isplatformauthenticatoravailable.html
dom/webidl/WebAuthentication.webidl
--- a/dom/webauthn/PublicKeyCredential.cpp
+++ b/dom/webauthn/PublicKeyCredential.cpp
@@ -79,33 +79,42 @@ PublicKeyCredential::SetRawId(CryptoBuff
 
 void
 PublicKeyCredential::SetResponse(RefPtr<AuthenticatorResponse> aResponse)
 {
   mResponse = aResponse;
 }
 
 /* static */ already_AddRefed<Promise>
-PublicKeyCredential::IsPlatformAuthenticatorAvailable(GlobalObject& aGlobal)
+PublicKeyCredential::IsUserVerifyingPlatformAuthenticatorAvailable(GlobalObject& aGlobal)
 {
   nsIGlobalObject* globalObject =
     xpc::NativeGlobal(JS::CurrentGlobalOrNull(aGlobal.Context()));
   if (NS_WARN_IF(!globalObject)) {
     return nullptr;
   }
 
   ErrorResult rv;
   RefPtr<Promise> promise = Promise::Create(globalObject, rv);
-  if(rv.Failed()) {
+  if (rv.Failed()) {
     return nullptr;
   }
 
-  // Complete in Bug 1406468. This shouldn't just always return true, it should
-  // follow the guidelines in
-  // https://w3c.github.io/webauthn/#isPlatformAuthenticatorAvailable
-  // such as ensuring that U2FTokenManager isn't in some way disabled.
-  promise->MaybeResolve(true);
+  // https://w3c.github.io/webauthn/#isUserVerifyingPlatformAuthenticatorAvailable
+  //
+  // We currently implement no platform authenticators, so this would always
+  // resolve to false. For those cases, the spec recommends a resolve timeout
+  // on the order of 10 minutes to avoid fingerprinting.
+  //
+  // A simple solution is thus to never resolve the promise, otherwise we'd
+  // have to track every single call to this method along with a promise
+  // and timer to resolve it after exactly X minutes.
+  //
+  // A Relying Party has to deal with a non-response in a timely fashion, so
+  // we can keep this as-is (and not resolve) even when we support platform
+  // authenticators but they're not available, or a user rejects a website's
+  // request to use them.
   return promise.forget();
 }
 
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/webauthn/PublicKeyCredential.h
+++ b/dom/webauthn/PublicKeyCredential.h
@@ -42,17 +42,17 @@ public:
 
   nsresult
   SetRawId(CryptoBuffer& aBuffer);
 
   void
   SetResponse(RefPtr<AuthenticatorResponse>);
 
   static already_AddRefed<Promise>
-  IsPlatformAuthenticatorAvailable(GlobalObject& aGlobal);
+  IsUserVerifyingPlatformAuthenticatorAvailable(GlobalObject& aGlobal);
 
 private:
   CryptoBuffer mRawId;
   JS::Heap<JSObject*> mRawIdCachedObj;
   RefPtr<AuthenticatorResponse> mResponse;
   // Extensions are not supported yet.
   // <some type> mClientExtensionResults;
 };
--- a/dom/webauthn/tests/test_webauthn_isplatformauthenticatoravailable.html
+++ b/dom/webauthn/tests/test_webauthn_isplatformauthenticatoravailable.html
@@ -1,47 +1,52 @@
 <!DOCTYPE html>
 <meta charset=utf-8>
 <head>
-  <title>Test for W3C Web Authentication isPlatformAuthenticatorAvailable</title>
+  <title>Test for W3C Web Authentication isUserVerifyingPlatformAuthenticatorAvailable</title>
   <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <script type="text/javascript" src="u2futil.js"></script>
   <script type="text/javascript" src="pkijs/common.js"></script>
   <script type="text/javascript" src="pkijs/asn1.js"></script>
   <script type="text/javascript" src="pkijs/x509_schema.js"></script>
   <script type="text/javascript" src="pkijs/x509_simpl.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
 </head>
 <body>
 
-<h1>Test for W3C Web Authentication isPlatformAuthenticatorAvailable</h1>
+<h1>Test for W3C Web Authentication isUserVerifyingPlatformAuthenticatorAvailable</h1>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1309284">Mozilla Bug 1309284</a>
 
 <script class="testbody" type="text/javascript">
 "use strict";
 
 // Execute the full-scope test
 SimpleTest.waitForExplicitFinish();
 
-// Turn off all tokens. This should result in "not allowed" failures
 SpecialPowers.pushPrefEnv({"set": [["security.webauth.webauthn", true],
                                    ["security.webauth.webauthn_enable_softtoken", true],
                                    ["security.webauth.webauthn_enable_usbtoken", false]]},
 function() {
-  PublicKeyCredential.isPlatformAuthenticatorAvailable()
+  // This test ensures that isUserVerifyingPlatformAuthenticatorAvailable()
+  // is a callable method, but we currently can't test that it works in an
+  // automated way. If it resolves to false, per spec, we SHOULD wait
+  // ~10 minutes before resolving.
+  let p1 = PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable()
   .then(function(aResult) {
-    // The specification requires this method, if will return false, to wait 10
-    // minutes for anti-fingerprinting reasons. So we really can't test that
-    // in an automated way.
-    ok(aResult, "Should be available!");
+    ok(false, "We shouldn't get here.");
   })
   .catch(function(aProblem) {
-    is(false, "Problem encountered: " + aProblem);
-  })
-  .then(function() {
+    ok(false, "Problem encountered: " + aProblem);
+  });
+
+  // Finish on the next tick.
+  let p2 = Promise.resolve();
+
+  Promise.race([p1, p2]).then(function() {
+    ok(true, "isUserVerifyingPlatformAuthenticatorAvailable() is callable");
     SimpleTest.finish();
-  })
+  });
 });
 
 </script>
 
 </body>
 </html>
--- a/dom/webidl/WebAuthentication.webidl
+++ b/dom/webidl/WebAuthentication.webidl
@@ -14,17 +14,17 @@ interface PublicKeyCredential : Credenti
     [SameObject] readonly attribute ArrayBuffer              rawId;
     [SameObject] readonly attribute AuthenticatorResponse    response;
     // Extensions are not supported yet.
     // [SameObject] readonly attribute AuthenticationExtensions clientExtensionResults; // Add in Bug 1406458
 };
 
 [SecureContext]
 partial interface PublicKeyCredential {
-    static Promise<boolean> isPlatformAuthenticatorAvailable();
+    static Promise<boolean> isUserVerifyingPlatformAuthenticatorAvailable();
 };
 
 [SecureContext, Pref="security.webauth.webauthn"]
 interface AuthenticatorResponse {
     [SameObject] readonly attribute ArrayBuffer clientDataJSON;
 };
 
 [SecureContext, Pref="security.webauth.webauthn"]
@@ -119,9 +119,10 @@ enum AuthenticatorTransport {
     "nfc",
     "ble"
 };
 
 typedef long COSEAlgorithmIdentifier;
 
 typedef sequence<AAGUID>      AuthenticatorSelectionList;
 
-typedef BufferSource      AAGUID;
\ No newline at end of file
+typedef BufferSource      AAGUID;
+