Bug 1694336 - Web Authentication: Only send "none" attestation if it was requested (by server or user) r=dveditz

The old behavior (only send attestation, if attestation-type was "direct" and "none" otherwise) broke the spec.
Only send "none", if directly requested by RP or the user.

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

dom/webauthn/U2FTokenManager.cpp

@@ -321,7 +321,7 @@ void U2FTokenManager::Register(
   mLastTransactionId = aTransactionId;
 
   // Determine whether direct attestation was requested.
-  bool directAttestationRequested = false;
+  bool noneAttestationRequested = true;
 
 // On Android, let's always reject direct attestations until we have a
 // mechanism to solicit user consent, from Bug 1550164
@@ -332,17 +332,16 @@ void U2FTokenManager::Register(
     AttestationConveyancePreference attestation =
         extra.attestationConveyancePreference();
 
-    directAttestationRequested =
-        attestation == AttestationConveyancePreference::Direct;
+    noneAttestationRequested =
+        attestation == AttestationConveyancePreference::None;
   }
 #endif  // not MOZ_WIDGET_ANDROID
 
   // Start a register request immediately if direct attestation
   // wasn't requested or the test pref is set.
-  if (!directAttestationRequested ||
+  if (noneAttestationRequested ||
       U2FPrefManager::Get()->GetAllowDirectAttestationForTesting()) {
-    // Force "none" attestation when "direct" attestation wasn't requested.
-    DoRegister(aTransactionInfo, !directAttestationRequested);
+    DoRegister(aTransactionInfo, noneAttestationRequested);
     return;
   }
 

dom/webauthn/tests/browser/browser_webauthn_prompts.js

@@ -68,7 +68,7 @@ add_task(async function test_register() {
 
   // Request a new credential and wait for the prompt.
   let active = true;
-  let request = promiseWebAuthnMakeCredential(tab, "indirect", {})
+  let request = promiseWebAuthnMakeCredential(tab, "none", {})
     .then(arrivingHereIsBad)
     .catch(expectAbortError)
     .then(() => (active = false));
@@ -133,7 +133,7 @@ add_task(async function test_tab_switching() {
 
   // Request a new credential and wait for the prompt.
   let active = true;
-  let request = promiseWebAuthnMakeCredential(tab_one, "indirect", {})
+  let request = promiseWebAuthnMakeCredential(tab_one, "none", {})
     .then(arrivingHereIsBad)
     .catch(expectAbortError)
     .then(() => (active = false));
@@ -179,7 +179,7 @@ add_task(async function test_window_switching() {
 
   // Request a new credential and wait for the prompt.
   let active = true;
-  let request = promiseWebAuthnMakeCredential(tab, "indirect", {})
+  let request = promiseWebAuthnMakeCredential(tab, "none", {})
     .then(arrivingHereIsBad)
     .catch(expectAbortError)
     .then(() => (active = false));

dom/webauthn/tests/test_webauthn_attestation_conveyance.html

@@ -93,9 +93,16 @@
         .then(verifyAnonymizedCertificate)
         .catch(arrivingHereIsBad);
 
-      // Request indirect attestation, which is the same as none.
+      // Request indirect attestation, which is the same as direct.
       await requestMakeCredential("indirect")
-        .then(verifyAnonymizedCertificate)
+        .then((x) => {
+          if (AppConstants.platform === "android") {
+            // If this is Android, the result will be anonymized (Bug 1551229)
+            return verifyAnonymizedCertificate(x);
+          } else {
+            return verifyDirectCertificate(x);
+          }
+        })
         .catch(arrivingHereIsBad);
 
       // Request direct attestation, which will prompt for user intervention.