Bug 1848821 - decode base64url encoded arguments in add/remove credential. r=keeler

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

Cargo.lock

@@ -326,6 +326,7 @@ name = "authrs_bridge"
 version = "0.1.0"
 dependencies = [
  "authenticator",
+ "base64 0.21.0",
  "log",
  "moz_task",
  "nserror",

dom/webauthn/authrs_bridge/Cargo.toml

@@ -6,6 +6,7 @@ authors = ["Martin Sirringhaus", "John Schanck"]
 
 [dependencies]
 authenticator = { version = "0.4.0-alpha.18", features = ["gecko"] }
+base64 = "^0.21"
 log = "0.4"
 moz_task = { path = "../../../xpcom/rust/moz_task" }
 nserror = { path = "../../../xpcom/rust/nserror" }

dom/webauthn/authrs_bridge/src/lib.rs

@@ -20,6 +20,7 @@ use authenticator::{
     statecallback::StateCallback,
     Assertion, Pin, RegisterResult, SignResult, StateMachine, StatusPinUv, StatusUpdate,
 };
+use base64::Engine;
 use moz_task::RunnableBuilder;
 use nserror::{
     nsresult, NS_ERROR_DOM_INVALID_STATE_ERR, NS_ERROR_DOM_NOT_ALLOWED_ERR,
@@ -858,11 +859,20 @@ impl AuthrsTransport {
         user_handle: &nsACString,
         sign_count: u32,
     ) -> Result<(), nsresult> {
+        let credential_id = base64::engine::general_purpose::URL_SAFE_NO_PAD
+            .decode(credential_id)
+            .or(Err(NS_ERROR_INVALID_ARG))?;
+        let private_key = base64::engine::general_purpose::URL_SAFE_NO_PAD
+            .decode(private_key)
+            .or(Err(NS_ERROR_INVALID_ARG))?;
+        let user_handle = base64::engine::general_purpose::URL_SAFE_NO_PAD
+            .decode(user_handle)
+            .or(Err(NS_ERROR_INVALID_ARG))?;
         self.test_token_manager.add_credential(
             authenticator_id,
-            credential_id.as_ref(),
-            private_key.as_ref(),
-            user_handle.as_ref(),
+            &credential_id,
+            &private_key,
+            &user_handle,
             sign_count,
             rp_id.to_string(),
             is_resident_credential,
@@ -883,6 +893,9 @@ impl AuthrsTransport {
         authenticator_id: u64,
         credential_id: &nsACString,
     ) -> Result<(), nsresult> {
+        let credential_id = base64::engine::general_purpose::URL_SAFE_NO_PAD
+            .decode(credential_id)
+            .or(Err(NS_ERROR_INVALID_ARG))?;
         self.test_token_manager
             .remove_credential(authenticator_id, credential_id.as_ref())
     }

dom/webauthn/authrs_bridge/src/test_token.rs

@@ -60,7 +60,7 @@ impl TestTokenCredential {
         &self,
         client_data_hash: &ClientDataHash,
         flags: AuthenticatorDataFlags,
-    ) -> GetAssertionResponse {
+    ) -> Result<GetAssertionResponse, HIDError> {
         let credentials = Some(PublicKeyCredentialDescriptor {
             id: self.id.clone(),
             transports: vec![],
@@ -79,16 +79,17 @@ impl TestTokenCredential {
             ..Default::default()
         });
 
-        let mut data = auth_data.to_vec().unwrap();
+        let mut data = auth_data.to_vec().or(Err(HIDError::DeviceError))?;
         data.extend_from_slice(client_data_hash.as_ref());
-        let signature = ecdsa_p256_sha256_sign_raw(&self.privkey, &data).unwrap();
-        GetAssertionResponse {
+        let signature =
+            ecdsa_p256_sha256_sign_raw(&self.privkey, &data).or(Err(HIDError::DeviceError))?;
+        Ok(GetAssertionResponse {
             credentials,
             auth_data,
             signature,
             user,
             number_of_credentials: Some(1),
-        }
+        })
     }
 }
 
@@ -363,7 +364,7 @@ impl VirtualFidoDevice for TestToken {
             // return at most one assertion matching an allowed credential ID
             for credential in eligible_cred_iter {
                 if req.allow_list.iter().any(|x| x.id == credential.id) {
-                    let assertion = credential.assert(&req.client_data_hash, flags).into();
+                    let assertion = credential.assert(&req.client_data_hash, flags)?.into();
                     assertions.push(assertion);
                     break;
                 }
@@ -375,7 +376,7 @@ impl VirtualFidoDevice for TestToken {
             // return a list of credentials here. The UI to select one of the results blocks
             // testing.
             for credential in eligible_cred_iter.filter(|x| x.is_discoverable_credential) {
-                let assertion = credential.assert(&req.client_data_hash, flags).into();
+                let assertion = credential.assert(&req.client_data_hash, flags)?.into();
                 assertions.push(assertion);
                 break;
             }

dom/webauthn/tests/mochitest.ini

@@ -90,3 +90,5 @@ skip-if =
   win11_2009 # Bug 1718296 (Windows 10 1903+ has its own window and U2F that we cannot control with tests.)
 [test_webauthn_isplatformauthenticatoravailable.html]
 [test_webauthn_isexternalctap2securitykeysupported.html]
+[test_webauthn_webdriver_virtual_authenticator.html]
+fail-if = xorigin # Cross-origin use of WebAuthn requires a feature policy.

dom/webauthn/tests/test_webauthn_webdriver_virtual_authenticator.html

@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<head>
+  <title>Tests for WebDriver Virtual Authenticator Extension for W3C Web Authentication</title>
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="u2futil.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+
+  <h1>Tests for WebDriver Virtual Authenticator Extension for W3C Web Authentication</h1>
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1460986">Mozilla Bug 1460986</a>
+
+  <script class="testbody" type="text/javascript">
+    "use strict";
+
+    function arrivingHereIsGood(aResult) {
+      ok(true, "Good result! Received a: " + aResult);
+    }
+
+    function arrivingHereIsBad(aResult) {
+      ok(false, "Bad result! Received a: " + aResult);
+    }
+
+    function expectNotAllowedError(aResult) {
+      ok(aResult == "NotAllowedError", "Expecting a NotAllowedError, got " + aResult);
+    }
+
+    function getAssertion(id) {
+        let chall = new Uint8Array(16);
+        crypto.getRandomValues(chall);
+
+        let options = {
+          challenge: chall,
+          allowCredentials: [ { type: "public-key", id } ],
+        };
+
+        return navigator.credentials.get({publicKey: options});
+    }
+
+    add_task(async function test_add_and_remove_credential() {
+      let authenticatorId = await addVirtualAuthenticator();
+      let credIdB64 = await addCredential(authenticatorId, document.domain);
+      let credId = base64ToBytesUrlSafe(credIdB64);
+
+      await getAssertion(credId)
+          .then(arrivingHereIsGood)
+          .catch(arrivingHereIsBad);
+
+      await removeCredential(authenticatorId, credIdB64);
+      await getAssertion(credId)
+          .then(arrivingHereIsBad)
+          .catch(e => expectNotAllowedError(e.name));
+    });
+  </script>
+
+</body>
+</html>

dom/webauthn/tests/u2futil.js

@@ -44,6 +44,8 @@ async function addVirtualAuthenticator() {
       webauthnTransport.removeVirtualAuthenticator(id);
     });
   });
+
+  return id;
 }
 
 function handleEventMessage(event) {
@@ -430,3 +432,56 @@ function verifySignature(key, data, derSig) {
   let alg = { name: "ECDSA", hash: "SHA-256" };
   return crypto.subtle.verify(alg, key, sigData, data);
 }
+
+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));
+
+  await SpecialPowers.spawnChrome(
+    [authenticatorId, credId, rpId, privateKey],
+    (authenticatorId, credId, rpId, 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) {
+  await SpecialPowers.spawnChrome(
+    [authenticatorId, credId],
+    (authenticatorId, credId) => {
+      let webauthnTransport = Cc[
+        "@mozilla.org/webauthn/transport;1"
+      ].getService(Ci.nsIWebAuthnTransport);
+
+      webauthnTransport.removeCredential(authenticatorId, credId);
+    }
+  );
+}