Bug 1416056 - Web Authentication - Default to "None Attestation" r=jcj

Summary:
Always replace attestation statements with a "none" attestation.

Bug 1430150 will introduce a prompt that asks the user for permission whenever
the RP requests "direct" attestation. Only if the user opts in we will forward
the attestation statement with the token's certificate and signature.

Reviewers: jcj

Reviewed By: jcj

Bug #: 1416056

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

dom/webauthn/WebAuthnCBORUtil.cpp

@@ -47,10 +47,10 @@ CBOREncodePublicKeyObj(const CryptoBuffer& aPubKeyBuf,
 }
 
 nsresult
-CBOREncodeAttestationObj(const CryptoBuffer& aAuthDataBuf,
-                         const CryptoBuffer& aAttestationCertBuf,
-                         const CryptoBuffer& aSignatureBuf,
-                         /* out */ CryptoBuffer& aAttestationObj)
+CBOREncodeFidoU2FAttestationObj(const CryptoBuffer& aAuthDataBuf,
+                                const CryptoBuffer& aAttestationCertBuf,
+                                const CryptoBuffer& aSignatureBuf,
+                                /* out */ CryptoBuffer& aAttestationObj)
 {
   /*
   Attestation Object, encoded in CBOR (description is CDDL)
@@ -97,5 +97,39 @@ CBOREncodeAttestationObj(const CryptoBuffer& aAuthDataBuf,
   return NS_OK;
 }
 
+nsresult
+CBOREncodeNoneAttestationObj(const CryptoBuffer& aAuthDataBuf,
+                             /* out */ CryptoBuffer& aAttestationObj)
+{
+  /*
+  Attestation Object, encoded in CBOR (description is CDDL)
+
+  $$attStmtType //= (
+                          fmt: "none",
+                          attStmt: emptyMap
+                      )
+
+  emptyMap = {}
+  */
+  cbor::output_dynamic cborAttOut;
+  cbor::encoder encoder(cborAttOut);
+  encoder.write_map(3);
+  {
+    encoder.write_string("fmt");
+    encoder.write_string("none");
+
+    encoder.write_string("attStmt");
+    encoder.write_map(0);
+
+    encoder.write_string("authData");
+    encoder.write_bytes(aAuthDataBuf.Elements(), aAuthDataBuf.Length());
+  }
+
+  if (!aAttestationObj.Assign(cborAttOut.data(), cborAttOut.size())) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+  return NS_OK;
+}
+
 }
 }

dom/webauthn/WebAuthnCBORUtil.h

@@ -21,10 +21,14 @@ CBOREncodePublicKeyObj(const CryptoBuffer& aPubKeyBuf,
                        /* out */ CryptoBuffer& aPubKeyObj);
 
 nsresult
-CBOREncodeAttestationObj(const CryptoBuffer& aAuthDataBuf,
-                         const CryptoBuffer& aAttestationCertBuf,
-                         const CryptoBuffer& aSignatureBuf,
-                         /* out */ CryptoBuffer& aAttestationObj);
+CBOREncodeFidoU2FAttestationObj(const CryptoBuffer& aAuthDataBuf,
+                                const CryptoBuffer& aAttestationCertBuf,
+                                const CryptoBuffer& aSignatureBuf,
+                                /* out */ CryptoBuffer& aAttestationObj);
+
+nsresult
+CBOREncodeNoneAttestationObj(const CryptoBuffer& aAuthDataBuf,
+                             /* out */ CryptoBuffer& aAttestationObj);
 
 } // namespace dom
 } // namespace mozilla

dom/webauthn/WebAuthnManager.cpp

@@ -397,9 +397,24 @@ WebAuthnManager::MakeCredential(const PublicKeyCredentialCreationOptions& aOptio
   bool requestDirectAttestation =
     attestation == AttestationConveyancePreference::Direct;
 
-  // In Bug 1430150, if requestDirectAttestation is true, we will need to prompt
-  // the user for permission to proceed. For now, we ignore it.
-  Unused << requestDirectAttestation;
+  // XXX Bug 1430150. Need something that allows direct attestation
+  // for tests until we implement a permission dialog we can click.
+  if (requestDirectAttestation) {
+    nsresult rv;
+    nsCOMPtr<nsIPrefService> prefService = do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);
+
+    if (NS_SUCCEEDED(rv)) {
+      nsCOMPtr<nsIPrefBranch> branch;
+      rv = prefService->GetBranch("security.webauth.", getter_AddRefs(branch));
+
+      if (NS_SUCCEEDED(rv)) {
+        rv = branch->GetBoolPref("webauthn_testing_allow_direct_attestation",
+                                 &requestDirectAttestation);
+      }
+    }
+
+    requestDirectAttestation &= NS_SUCCEEDED(rv);
+  }
 
   // Create and forward authenticator selection criteria.
   WebAuthnAuthenticatorSelection authSelection(selection.mRequireResidentKey,
@@ -425,6 +440,7 @@ WebAuthnManager::MakeCredential(const PublicKeyCredentialCreationOptions& aOptio
   mTransaction = Some(WebAuthnTransaction(promise,
                                           rpIdHash,
                                           clientDataJSON,
+                                          requestDirectAttestation,
                                           signal));
 
   mChild->SendRequestRegister(mTransaction.ref().mId, info);
@@ -604,6 +620,7 @@ WebAuthnManager::GetAssertion(const PublicKeyCredentialRequestOptions& aOptions,
   mTransaction = Some(WebAuthnTransaction(promise,
                                           rpIdHash,
                                           clientDataJSON,
+                                          false /* aDirectAttestation */,
                                           signal));
 
   mChild->SendRequestSign(mTransaction.ref().mId, info);
@@ -653,8 +670,8 @@ WebAuthnManager::FinishMakeCredential(const uint64_t& aTransactionId,
     RejectTransaction(NS_ERROR_OUT_OF_MEMORY);
     return;
   }
-  // TODO: Adjust the AAGUID from all zeroes in Bug 1381575 (if needed)
-  // See https://github.com/w3c/webauthn/issues/506
+  // FIDO U2F devices have no AAGUIDs, so they'll be all zeros until we add
+  // support for CTAP2 devices.
   for (int i=0; i<16; i++) {
     aaguidBuf.AppendElement(0x00, mozilla::fallible);
   }
@@ -730,11 +747,16 @@ WebAuthnManager::FinishMakeCredential(const uint64_t& aTransactionId,
     return;
   }
 
-  // The Authentication Data buffer gets CBOR-encoded with the Cert and
-  // Signature to build the Attestation Object.
+  // Direct attestation might have been requested by the RP. mDirectAttestation
+  // will be true only if the user consented via the permission UI.
   CryptoBuffer attObj;
-  rv = CBOREncodeAttestationObj(authDataBuf, attestationCertBuf, signatureBuf,
-                                attObj);
+  if (mTransaction.ref().mDirectAttestation) {
+    rv = CBOREncodeFidoU2FAttestationObj(authDataBuf, attestationCertBuf,
+                                         signatureBuf, attObj);
+  } else {
+    rv = CBOREncodeNoneAttestationObj(authDataBuf, attObj);
+  }
+
   if (NS_FAILED(rv)) {
     RejectTransaction(rv);
     return;

dom/webauthn/WebAuthnManager.h

@@ -57,10 +57,12 @@ public:
   WebAuthnTransaction(const RefPtr<Promise>& aPromise,
                       const nsTArray<uint8_t>& aRpIdHash,
                       const nsCString& aClientData,
+                      bool aDirectAttestation,
                       AbortSignal* aSignal)
     : mPromise(aPromise)
     , mRpIdHash(aRpIdHash)
     , mClientData(aClientData)
+    , mDirectAttestation(aDirectAttestation)
     , mSignal(aSignal)
     , mId(NextId())
   {
@@ -76,6 +78,10 @@ public:
   // Client data used to assemble reply objects.
   nsCString mClientData;
 
+  // The RP might request direct attestation.
+  // Only used by the MakeCredential operation.
+  bool mDirectAttestation;
+
   // An optional AbortSignal instance.
   RefPtr<AbortSignal> mSignal;
 

dom/webauthn/tests/browser/browser_webauthn_telemetry.js

@@ -68,6 +68,7 @@ add_task(async function test_loopback() {
     Services.prefs.setBoolPref("security.webauth.webauthn", true);
     Services.prefs.setBoolPref("security.webauth.webauthn_enable_softtoken", true);
     Services.prefs.setBoolPref("security.webauth.webauthn_enable_usbtoken", false);
+    Services.prefs.setBoolPref("security.webauth.webauthn_testing_allow_direct_attestation", true);
 
     await executeTestPage(testPage);
 

dom/webauthn/tests/browser/tab_webauthn_success.html

@@ -39,6 +39,7 @@ let makeCredentialOptions = {
   challenge: gCredentialChallenge,
   timeout: 5000, // the minimum timeout is actually 15 seconds
   pubKeyCredParams: [{type: "public-key", alg: cose_alg_ECDSA_w_SHA256}],
+  attestation: "direct"
 };
 
 navigator.credentials.create({publicKey: makeCredentialOptions})
@@ -113,4 +114,4 @@ navigator.credentials.create({publicKey: makeCredentialOptions})
 </script>
 
 </body>
-</html>
+</html>

dom/webauthn/tests/test_webauthn_attestation_conveyance.html

@@ -24,6 +24,7 @@
     function getAttestationCertFromAttestationBuffer(aAttestationBuffer) {
       return webAuthnDecodeCBORAttestation(aAttestationBuffer)
       .then((aAttestationObj) => {
+        is("fido-u2f", aAttestationObj.fmt, "Is a FIDO U2F Attestation");
         let attestationCertDER = aAttestationObj.attStmt.x5c[0];
         let certDERBuffer = attestationCertDER.slice(0, attestationCertDER.byteLength).buffer;
         let certAsn1 = org.pkijs.fromBER(certDERBuffer);
@@ -32,9 +33,12 @@
     }
 
     function verifyAnonymizedCertificate(aResult) {
-      // TODO: Update this logic with Bug 1430150.
-      // Until then, all certificates are direct.
-      return verifyDirectCertificate(aResult);
+      return webAuthnDecodeCBORAttestation(aResult.response.attestationObject)
+      .then(({fmt, attStmt}) => {
+        is("none", fmt, "Is a None Attestation");
+        is("object", typeof(attStmt), "attStmt is a map");
+        is(0, Object.keys(attStmt).length, "attStmt is empty");
+      });
     }
 
     function verifyDirectCertificate(aResult) {
@@ -59,6 +63,7 @@
         ["security.webauth.webauthn", true],
         ["security.webauth.webauthn_enable_softtoken", true],
         ["security.webauth.webauthn_enable_usbtoken", false],
+        ["security.webauth.webauthn_testing_allow_direct_attestation", true],
       ]});
     });
 

dom/webauthn/tests/test_webauthn_loopback.html

@@ -24,7 +24,8 @@ SimpleTest.waitForExplicitFinish();
 
 SpecialPowers.pushPrefEnv({"set": [["security.webauth.webauthn", true],
                                    ["security.webauth.webauthn_enable_softtoken", true],
-                                   ["security.webauth.webauthn_enable_usbtoken", false]]},
+                                   ["security.webauth.webauthn_enable_usbtoken", false],
+                                   ["security.webauth.webauthn_testing_allow_direct_attestation", true]]},
 function() {
   is(navigator.authentication, undefined, "navigator.authentication does not exist any longer");
   isnot(navigator.credentials, undefined, "Credential Management API endpoint must exist");
@@ -149,7 +150,8 @@ function() {
       rp: rp,
       user: user,
       challenge: gCredentialChallenge,
-      pubKeyCredParams: [param]
+      pubKeyCredParams: [param],
+      attestation: "direct"
     };
     credm.create({publicKey: makeCredentialOptions})
     .then(decodeCreatedCredential)

dom/webauthn/tests/u2futil.js

@@ -146,13 +146,22 @@ function hexDecode(str) {
   return new Uint8Array(str.match(/../g).map(x => parseInt(x, 16)));
 }
 
+function hasOnlyKeys(obj, ...keys) {
+  let okeys = new Set(Object.keys(obj));
+  return keys.length == okeys.size &&
+         keys.every(k => okeys.has(k));
+}
+
 function webAuthnDecodeCBORAttestation(aCborAttBuf) {
   let attObj = CBOR.decode(aCborAttBuf);
   console.log(":: Attestation CBOR Object ::");
-  if (!("authData" in attObj && "fmt" in attObj && "attStmt" in attObj)) {
+  if (!hasOnlyKeys(attObj, "authData", "fmt", "attStmt")) {
     return Promise.reject("Invalid CBOR Attestation Object");
   }
-  if (!("sig" in attObj.attStmt && "x5c" in attObj.attStmt)) {
+  if (attObj.fmt == "fido-u2f" && !hasOnlyKeys(attObj.attStmt, "sig", "x5c")) {
+    return Promise.reject("Invalid CBOR Attestation Statement");
+  }
+  if (attObj.fmt == "none" && Object.keys(attObj.attStmt).length > 0) {
     return Promise.reject("Invalid CBOR Attestation Statement");
   }