Bug 1846836 - allow App ID extension when security.webauthn.ctap2 is true. r=keeler

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

dom/webauthn/authrs_bridge/src/lib.rs

@@ -555,7 +555,7 @@ impl AuthrsTransport {
             resident_key_req,
             extensions: Default::default(),
             pin: None,
-            use_ctap1_fallback: static_prefs::pref!("security.webauthn.ctap2") == false,
+            use_ctap1_fallback: !static_prefs::pref!("security.webauthn.ctap2"),
         };
 
         let (status_tx, status_rx) = channel::<StatusUpdate>();
@@ -734,12 +734,6 @@ impl AuthrsTransport {
                 let _ = controller.finish_sign(tid, result);
             }));
 
-        // Bug 1834771 - Pre-filtering allowlists broke AppID support. As a temporary
-        // workaround, we will fallback to CTAP1 when the request includes the AppID
-        // extension and the allowlist is non-empty.
-        let use_ctap1_fallback = static_prefs::pref!("security.webauthn.ctap2") == false
-            || (alternate_rp_id.is_some() && !allow_list.is_empty());
-
         let info = SignArgs {
             client_data_hash: client_data_hash_arr,
             relying_party_id: relying_party_id.to_string(),
@@ -750,7 +744,7 @@ impl AuthrsTransport {
             extensions: Default::default(),
             pin: None,
             alternate_rp_id,
-            use_ctap1_fallback,
+            use_ctap1_fallback: !static_prefs::pref!("security.webauthn.ctap2"),
         };
 
         // As in `register`, we are intentionally avoiding `AuthenticatorService` here.

dom/webauthn/tests/browser/browser_fido_appid_extension.js

@@ -7,10 +7,70 @@
 const TEST_URL = "https://example.com/";
 
 let expectNotSupportedError = expectError("NotSupported");
-let expectInvalidStateError = expectError("InvalidState");
+let expectNotAllowedError = expectError("NotAllowed");
 let expectSecurityError = expectError("Security");
 
-add_virtual_authenticator();
+let gAppId = "https://example.com/appId";
+let gCrossOriginAppId = "https://example.org/appId";
+let gAuthenticatorId = add_virtual_authenticator();
+
+add_task(async function test_appid() {
+  // Open a new tab.
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
+
+  // The FIDO AppId extension can't be used for MakeCredential.
+  await promiseWebAuthnMakeCredential(tab, "none", { appid: gAppId })
+    .then(arrivingHereIsBad)
+    .catch(expectNotSupportedError);
+
+  // Side-load a credential with an RP ID matching the App ID.
+  let credIdB64 = await addCredential(gAuthenticatorId, gAppId);
+  let credId = base64ToBytesUrlSafe(credIdB64);
+
+  // And another for a different origin
+  let crossOriginCredIdB64 = await addCredential(
+    gAuthenticatorId,
+    gCrossOriginAppId
+  );
+  let crossOriginCredId = base64ToBytesUrlSafe(crossOriginCredIdB64);
+
+  // The App ID extension is required
+  await promiseWebAuthnGetAssertion(tab, credId)
+    .then(arrivingHereIsBad)
+    .catch(expectNotAllowedError);
+
+  // The value in the App ID extension must match the origin.
+  await promiseWebAuthnGetAssertion(tab, crossOriginCredId, {
+    appid: gCrossOriginAppId,
+  })
+    .then(arrivingHereIsBad)
+    .catch(expectSecurityError);
+
+  // The value in the App ID extension must match the credential's RP ID.
+  await promiseWebAuthnGetAssertion(tab, credId, { appid: gAppId + "2" })
+    .then(arrivingHereIsBad)
+    .catch(expectNotAllowedError);
+
+  // Succeed with the right App ID.
+  let rpIdHash = await promiseWebAuthnGetAssertion(tab, credId, {
+    appid: gAppId,
+  })
+    .then(({ authenticatorData, extensions }) => {
+      is(extensions.appid, true, "appid extension was acted upon");
+      return authenticatorData.slice(0, 32);
+    })
+    .then(rpIdHash => {
+      // Make sure the returned RP ID hash matches the hash of the App ID.
+      checkRpIdHash(rpIdHash, gAppId);
+    })
+    .catch(arrivingHereIsBad);
+
+  removeCredential(gAuthenticatorId, credIdB64);
+  removeCredential(gAuthenticatorId, crossOriginCredIdB64);
+
+  // Close tab.
+  BrowserTestUtils.removeTab(tab);
+});
 
 add_task(async function test_appid_unused() {
   // Open a new tab.

dom/webauthn/tests/browser/head.js

@@ -42,6 +42,49 @@ function add_virtual_authenticator(autoremove = true) {
   return id;
 }
 
+async function addCredential(authenticatorId, rpId) {
+  let keyPair = await crypto.subtle.generateKey(
+    {
+      name: "ECDSA",
+      namedCurve: "P-256",
+    },
+    true,
+    ["sign"]
+  );
+
+  let credId = new Uint8Array(32);
+  crypto.getRandomValues(credId);
+  credId = bytesToBase64UrlSafe(credId);
+
+  let privateKey = await crypto.subtle
+    .exportKey("pkcs8", keyPair.privateKey)
+    .then(privateKey => bytesToBase64UrlSafe(privateKey));
+
+  let webauthnTransport = Cc["@mozilla.org/webauthn/transport;1"].getService(
+    Ci.nsIWebAuthnTransport
+  );
+
+  webauthnTransport.addCredential(
+    authenticatorId,
+    credId,
+    true, // resident key
+    rpId,
+    privateKey,
+    "VGVzdCBVc2Vy", // "Test User"
+    0 // sign count
+  );
+
+  return credId;
+}
+
+async function removeCredential(authenticatorId, credId) {
+  let webauthnTransport = Cc["@mozilla.org/webauthn/transport;1"].getService(
+    Ci.nsIWebAuthnTransport
+  );
+
+  webauthnTransport.removeCredential(authenticatorId, credId);
+}
+
 function memcmp(x, y) {
   let xb = new Uint8Array(x);
   let yb = new Uint8Array(y);