Bug 1854016 - move webauthn signature selection logic to authrs_bridge. r=keeler

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

Cargo.lock

@@ -333,6 +333,7 @@ dependencies = [
  "nsstring",
  "rand",
  "serde_cbor",
+ "serde_json",
  "static_prefs",
  "thin-vec",
  "xpcom",

browser/base/content/browser.js

@@ -7655,7 +7655,7 @@ var WebAuthnPromptHelper = {
     let secondaryActions = [];
     for (let i = 0; i < usernames.length; i++) {
       secondaryActions.push({
-        label: unescape(decodeURIComponent(usernames[i])),
+        label: usernames[i],
         accessKey: i.toString(),
         callback(aState) {
           mgr.signatureSelectionCallback(tid, i);

dom/webauthn/WebAuthnController.cpp

@@ -51,9 +51,6 @@ static const char16_t kRegisterDirectPromptNotification[] =
     u"\"origin\":\"%s\",\"browsingContextId\":%llu}";
 static const char16_t kCancelPromptNotification[] =
     u"{\"action\":\"cancel\",\"tid\":%llu}";
-static const char16_t kSelectSignResultNotification[] =
-    u"{\"action\":\"select-sign-result\",\"tid\":%llu,"
-    u"\"origin\":\"%s\",\"browsingContextId\":%llu,\"usernames\":[%s]}";
 
 /***********************************************************************
  * U2FManager Implementation
@@ -123,7 +120,6 @@ void WebAuthnController::ClearTransaction(bool cancel_prompt) {
   // Forget any pending registration.
   mPendingRegisterInfo.reset();
   mPendingSignInfo.reset();
-  mPendingSignResults.Clear();
   mTransaction.reset();
 }
 
@@ -509,17 +505,13 @@ void WebAuthnController::Sign(PWebAuthnTransactionParent* aTransactionParent,
 }
 
 NS_IMETHODIMP
-WebAuthnController::FinishSign(
+WebAuthnController::FinishSign(uint64_t aTransactionId,
-    uint64_t aTransactionId,
+                               nsICtapSignResult* aResult) {
-    const nsTArray<RefPtr<nsICtapSignResult>>& aResult) {
   MOZ_ASSERT(XRE_IsParentProcess());
-  nsTArray<RefPtr<nsICtapSignResult>> ownedResult = aResult.Clone();
-
   nsCOMPtr<nsIRunnable> r(
-      NewRunnableMethod<uint64_t, nsTArray<RefPtr<nsICtapSignResult>>>(
+      NewRunnableMethod<uint64_t, RefPtr<nsICtapSignResult>>(
           "WebAuthnController::RunFinishSign", this,
-          &WebAuthnController::RunFinishSign, aTransactionId,
+          &WebAuthnController::RunFinishSign, aTransactionId, aResult));
-          std::move(ownedResult)));
 
   if (!gWebAuthnBackgroundThread) {
     return NS_ERROR_FAILURE;
@@ -531,82 +523,78 @@ WebAuthnController::FinishSign(
 }
 
 void WebAuthnController::RunFinishSign(
-    uint64_t aTransactionId,
+    uint64_t aTransactionId, const RefPtr<nsICtapSignResult>& aResult) {
-    const nsTArray<RefPtr<nsICtapSignResult>>& aResult) {
   mozilla::ipc::AssertIsOnBackgroundThread();
   if (mTransaction.isNothing() ||
       aTransactionId != mTransaction.ref().mTransactionId) {
     return;
   }
 
-  if (aResult.Length() == 0) {
+  nsresult status;
+  nsresult rv = aResult->GetStatus(&status);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
+    return;
+  }
+  if (NS_FAILED(status)) {
+    bool shouldCancelActiveDialog = true;
+    if (status == NS_ERROR_DOM_INVALID_STATE_ERR) {
+      // PIN-related errors, e.g. blocked token. Let the dialog show to inform
+      // the user
+      shouldCancelActiveDialog = false;
+    }
     Telemetry::ScalarAdd(Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
                          u"CTAPSignAbort"_ns, 1);
+    AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR,
+                     shouldCancelActiveDialog);
+    return;
+  }
+
+  nsTArray<uint8_t> credentialId;
+  rv = aResult->GetCredentialId(credentialId);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
     AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
     return;
   }
 
-  if (aResult.Length() == 1) {
+  nsTArray<uint8_t> signature;
-    nsresult status;
+  rv = aResult->GetSignature(signature);
-    nsresult rv = aResult[0]->GetStatus(&status);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
-    if (NS_WARN_IF(NS_FAILED(rv))) {
+    AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
-      AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
-      return;
-    }
-    if (NS_FAILED(status)) {
-      bool shouldCancelActiveDialog = true;
-      if (status == NS_ERROR_DOM_INVALID_STATE_ERR) {
-        // PIN-related errors, e.g. blocked token. Let the dialog show to inform
-        // the user
-        shouldCancelActiveDialog = false;
-      }
-      Telemetry::ScalarAdd(Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
-                           u"CTAPSignAbort"_ns, 1);
-      AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR,
-                       shouldCancelActiveDialog);
-      return;
-    }
-    mPendingSignResults = aResult.Clone();
-    RunResumeWithSelectedSignResult(aTransactionId, 0);
     return;
   }
 
-  // If we more than one assertion, all of them should have OK status.
+  nsTArray<uint8_t> authenticatorData;
-  for (const auto& assertion : aResult) {
+  rv = aResult->GetAuthenticatorData(authenticatorData);
-    nsresult status;
+  if (NS_WARN_IF(NS_FAILED(rv))) {
-    nsresult rv = assertion->GetStatus(&status);
+    AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
-      AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
-      return;
-    }
-    if (NS_WARN_IF(NS_FAILED(status))) {
-      Telemetry::ScalarAdd(Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
-                           u"CTAPSignAbort"_ns, 1);
-      AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
-      return;
-    }
   }
 
-  nsCString usernames;
+  nsTArray<uint8_t> rpIdHash;
-  StringJoinAppend(
+  rv = aResult->GetRpIdHash(rpIdHash);
-      usernames, ","_ns, aResult,
+  if (NS_WARN_IF(NS_FAILED(rv))) {
-      [](nsACString& dst, const RefPtr<nsICtapSignResult>& assertion) {
+    AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
-        nsCString username;
+    return;
-        nsresult rv = assertion->GetUserName(username);
+  }
-        if (NS_FAILED(rv)) {
-          username.Assign("<Unknown username>");
-        }
-        nsCString escaped_username;
-        NS_Escape(username, escaped_username, url_XAlphas);
-        dst.Append("\""_ns + escaped_username + "\""_ns);
-      });
 
-  mPendingSignResults = aResult.Clone();
+  nsTArray<uint8_t> userHandle;
-  NS_ConvertUTF16toUTF8 origin(mPendingSignInfo.ref().Origin());
+  Unused << aResult->GetUserHandle(userHandle);  // optional
-  SendPromptNotification(kSelectSignResultNotification,
+
-                         mTransaction.ref().mTransactionId, origin.get(),
+  nsTArray<WebAuthnExtensionResult> extensions;
-                         mPendingSignInfo.ref().BrowsingContextId(),
+  if (mTransaction.ref().mAppIdHash.isSome()) {
-                         usernames.get());
+    bool usedAppId = (rpIdHash == mTransaction.ref().mAppIdHash.ref());
+    extensions.AppendElement(WebAuthnExtensionResultAppId(usedAppId));
+  }
+
+  WebAuthnGetAssertionResult result(mTransaction.ref().mClientDataJSON,
+                                    credentialId, signature, authenticatorData,
+                                    extensions, userHandle);
+
+  Telemetry::ScalarAdd(Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
+                       u"CTAPSignFinish"_ns, 1);
+  Unused << mTransactionParent->SendConfirmSign(aTransactionId, result);
+  ClearTransaction(true);
 }
 
 NS_IMETHODIMP
@@ -630,64 +618,12 @@ WebAuthnController::SignatureSelectionCallback(uint64_t aTransactionId,
 }
 
 void WebAuthnController::RunResumeWithSelectedSignResult(
-    uint64_t aTransactionId, uint64_t idx) {
+    uint64_t aTransactionId, uint64_t aIndex) {
   mozilla::ipc::AssertIsOnBackgroundThread();
-  if (mTransaction.isNothing() ||
+
-      mTransaction.ref().mTransactionId != aTransactionId) {
+  if (mTransportImpl) {
-    return;
+    mTransportImpl->SelectionCallback(aTransactionId, aIndex);
   }
-
-  if (NS_WARN_IF(mPendingSignResults.Length() <= idx)) {
-    return;
-  }
-
-  RefPtr<nsICtapSignResult>& selected = mPendingSignResults[idx];
-
-  nsTArray<uint8_t> credentialId;
-  nsresult rv = selected->GetCredentialId(credentialId);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
-    return;
-  }
-
-  nsTArray<uint8_t> signature;
-  rv = selected->GetSignature(signature);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
-    return;
-  }
-
-  nsTArray<uint8_t> authenticatorData;
-  rv = selected->GetAuthenticatorData(authenticatorData);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
-    return;
-  }
-
-  nsTArray<uint8_t> rpIdHash;
-  rv = selected->GetRpIdHash(rpIdHash);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
-    return;
-  }
-
-  nsTArray<uint8_t> userHandle;
-  Unused << selected->GetUserHandle(userHandle);  // optional
-
-  nsTArray<WebAuthnExtensionResult> extensions;
-  if (mTransaction.ref().mAppIdHash.isSome()) {
-    bool usedAppId = (rpIdHash == mTransaction.ref().mAppIdHash.ref());
-    extensions.AppendElement(WebAuthnExtensionResultAppId(usedAppId));
-  }
-
-  WebAuthnGetAssertionResult result(mTransaction.ref().mClientDataJSON,
-                                    credentialId, signature, authenticatorData,
-                                    extensions, userHandle);
-
-  Telemetry::ScalarAdd(Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
-                       u"CTAPSignFinish"_ns, 1);
-  Unused << mTransactionParent->SendConfirmSign(aTransactionId, result);
-  ClearTransaction(true);
 }
 
 NS_IMETHODIMP

dom/webauthn/WebAuthnController.h

@@ -74,7 +74,7 @@ class WebAuthnController final : public nsIWebAuthnController {
   void RunFinishRegister(uint64_t aTransactionId,
                          const RefPtr<nsICtapRegisterResult>& aResult);
   void RunFinishSign(uint64_t aTransactionId,
-                     const nsTArray<RefPtr<nsICtapSignResult>>& aResult);
+                     const RefPtr<nsICtapSignResult>& aResult);
 
   // The main thread runnable function for "nsIU2FTokenManager.ResumeRegister".
   void RunResumeRegister(uint64_t aTransactionId, bool aForceNoneAttestation);
@@ -98,8 +98,6 @@ class WebAuthnController final : public nsIWebAuthnController {
   // Pending registration info while we wait for user input.
   Maybe<WebAuthnGetAssertionInfo> mPendingSignInfo;
 
-  nsTArray<RefPtr<nsICtapSignResult>> mPendingSignResults;
-
   class Transaction {
    public:
     Transaction(uint64_t aTransactionId, const nsTArray<uint8_t>& aRpIdHash,

dom/webauthn/authrs_bridge/Cargo.toml

@@ -13,6 +13,7 @@ nserror = { path = "../../../xpcom/rust/nserror" }
 nsstring = { path = "../../../xpcom/rust/nsstring" }
 rand = "0.8"
 serde_cbor = "0.11"
+serde_json = "1.0"
 static_prefs = { path = "../../../modules/libpref/init/static_prefs" }
 thin-vec = { version = "0.2.1", features = ["gecko-ffi"] }
 xpcom = { path = "../../../xpcom/rust/xpcom" }

dom/webauthn/authrs_bridge/src/lib.rs

@@ -19,7 +19,7 @@ use authenticator::{
     },
     errors::{AuthenticatorError, PinError, U2FTokenError},
     statecallback::StateCallback,
-    Assertion, Pin, RegisterResult, SignResult, StateMachine, StatusPinUv, StatusUpdate,
+    Pin, RegisterResult, SignResult, StateMachine, StatusPinUv, StatusUpdate,
 };
 use base64::Engine;
 use moz_task::RunnableBuilder;
@@ -31,6 +31,7 @@ use nserror::{
 };
 use nsstring::{nsACString, nsCString, nsString};
 use serde_cbor;
+use serde_json::json;
 use std::cell::RefCell;
 use std::sync::mpsc::{channel, Receiver, RecvError, Sender};
 use std::sync::{Arc, Mutex};
@@ -73,6 +74,29 @@ fn make_pin_required_prompt(
     )
 }
 
+fn make_user_selection_prompt(
+    tid: u64,
+    origin: &str,
+    browsing_context_id: u64,
+    user_entities: &[PublicKeyCredentialUserEntity],
+) -> String {
+    // Bug 1854280: "Unknown username" should be a localized string here.
+    let usernames: Vec<String> = user_entities
+        .iter()
+        .map(|entity| {
+            entity
+                .name
+                .clone()
+                .unwrap_or("<Unknown username>".to_string())
+        })
+        .collect();
+    let usernames_json = json!(usernames);
+    let out = format!(
+        r#"{{"action":"select-sign-result","tid":{tid},"origin":"{origin}","browsingContextId":{browsing_context_id},"usernames":{usernames_json}}}"#,
+    );
+    out
+}
+
 fn authrs_to_nserror(e: &AuthenticatorError) -> nsresult {
     match e {
         AuthenticatorError::U2FToken(U2FTokenError::NotSupported) => NS_ERROR_DOM_NOT_SUPPORTED_ERR,
@@ -190,73 +214,51 @@ impl WebAuthnAttObj {
 
 #[xpcom(implement(nsICtapSignResult), atomic)]
 pub struct CtapSignResult {
-    result: Result<Assertion, AuthenticatorError>,
+    result: Result<SignResult, AuthenticatorError>,
 }
 
 impl CtapSignResult {
     xpcom_method!(get_credential_id => GetCredentialId() -> ThinVec<u8>);
     fn get_credential_id(&self) -> Result<ThinVec<u8>, nsresult> {
-        let mut out = ThinVec::new();
+        let rv = NS_ERROR_FAILURE;
-        if let Ok(assertion) = &self.result {
+        let inner = self.result.as_ref().or(Err(rv))?;
-            if let Some(cred) = &assertion.credentials {
+        let cred = inner.assertion.credentials.as_ref().ok_or(rv)?;
-                out.extend_from_slice(&cred.id);
+        Ok(cred.id.as_slice().into())
-                return Ok(out);
-            }
-        }
-        Err(NS_ERROR_FAILURE)
     }
 
     xpcom_method!(get_signature => GetSignature() -> ThinVec<u8>);
     fn get_signature(&self) -> Result<ThinVec<u8>, nsresult> {
-        let mut out = ThinVec::new();
+        let inner = self.result.as_ref().or(Err(NS_ERROR_FAILURE))?;
-        if let Ok(assertion) = &self.result {
+        Ok(inner.assertion.signature.as_slice().into())
-            out.extend_from_slice(&assertion.signature);
-            return Ok(out);
-        }
-        Err(NS_ERROR_FAILURE)
     }
 
     xpcom_method!(get_authenticator_data => GetAuthenticatorData() -> ThinVec<u8>);
     fn get_authenticator_data(&self) -> Result<ThinVec<u8>, nsresult> {
-        self.result
+        let inner = self.result.as_ref().or(Err(NS_ERROR_FAILURE))?;
-            .as_ref()
+        Ok(inner.assertion.auth_data.to_vec().into())
-            .map(|assertion| assertion.auth_data.to_vec().into())
-            .or(Err(NS_ERROR_FAILURE))
     }
 
     xpcom_method!(get_user_handle => GetUserHandle() -> ThinVec<u8>);
     fn get_user_handle(&self) -> Result<ThinVec<u8>, nsresult> {
-        let mut out = ThinVec::new();
+        let rv = NS_ERROR_NOT_AVAILABLE;
-        if let Ok(assertion) = &self.result {
+        let inner = self.result.as_ref().or(Err(rv))?;
-            if let Some(user) = &assertion.user {
+        let user = &inner.assertion.user.as_ref().ok_or(rv)?;
-                out.extend_from_slice(&user.id);
+        Ok(user.id.as_slice().into())
-                return Ok(out);
-            }
-        }
-        Err(NS_ERROR_FAILURE)
     }
 
     xpcom_method!(get_user_name => GetUserName() -> nsACString);
     fn get_user_name(&self) -> Result<nsCString, nsresult> {
-        if let Ok(assertion) = &self.result {
+        let rv = NS_ERROR_NOT_AVAILABLE;
-            if let Some(user) = &assertion.user {
+        let inner = self.result.as_ref().or(Err(rv))?;
-                if let Some(name) = &user.name {
+        let user = inner.assertion.user.as_ref().ok_or(rv)?;
-                    return Ok(nsCString::from(name));
+        let name = user.name.as_ref().ok_or(rv)?;
-                }
+        Ok(nsCString::from(name))
-            }
-        }
-        Err(NS_ERROR_NOT_AVAILABLE)
     }
 
     xpcom_method!(get_rp_id_hash => GetRpIdHash() -> ThinVec<u8>);
     fn get_rp_id_hash(&self) -> Result<ThinVec<u8>, nsresult> {
-        // assertion.auth_data.rp_id_hash
+        let inner = self.result.as_ref().or(Err(NS_ERROR_FAILURE))?;
-        let mut out = ThinVec::new();
+        Ok(inner.assertion.auth_data.rp_id_hash.0.into())
-        if let Ok(assertion) = &self.result {
-            out.extend_from_slice(&assertion.auth_data.rp_id_hash.0);
-            return Ok(out);
-        }
-        Err(NS_ERROR_FAILURE)
     }
 
     xpcom_method!(get_status => GetStatus() -> nsresult);
@@ -321,38 +323,22 @@ impl Controller {
         if (*self.0.borrow()).is_null() {
             return Err(NS_ERROR_FAILURE);
         }
-
+        let wrapped_result = CtapSignResult::allocate(InitCtapSignResult { result })
-        // If result is an error, we return a single CtapSignResult that has its status field set
+            .query_interface::<nsICtapSignResult>()
-        // to an error. Otherwise we convert the entries of SignResult (= Vec<Assertion>) into
+            .ok_or(NS_ERROR_FAILURE)?;
-        // CtapSignResults with OK statuses.
-        let mut assertions: ThinVec<Option<RefPtr<nsICtapSignResult>>> = ThinVec::new();
-        match result {
-            Err(e) => assertions.push(
-                CtapSignResult::allocate(InitCtapSignResult { result: Err(e) })
-                    .query_interface::<nsICtapSignResult>(),
-            ),
-            Ok(result) => {
-                assertions.push(
-                    CtapSignResult::allocate(InitCtapSignResult {
-                        result: Ok(result.assertion),
-                    })
-                    .query_interface::<nsICtapSignResult>(),
-                );
-            }
-        }
-
         unsafe {
-            (**(self.0.borrow())).FinishSign(tid, &mut assertions);
+            (**(self.0.borrow())).FinishSign(tid, wrapped_result.coerce());
         }
         Ok(())
     }
 }
 
-// The state machine creates a Sender<Pin>/Receiver<Pin> channel in ask_user_for_pin. It passes the
+// A transaction may create a channel to ask a user for additional input, e.g. a PIN. The Sender
-// Sender through status_callback, which stores the Sender in the pin_receiver field of an
+// component of this channel is sent to an AuthrsTransport in a StatusUpdate. AuthrsTransport
-// AuthrsTransport. The u64 in PinReceiver is a transaction ID, which the AuthrsTransport uses the
+// caches the sender along with the expected (u64) transaction ID, which is used as a consistency
-// transaction ID as a consistency check.
+// check in callbacks.
 type PinReceiver = Option<(u64, Sender<Pin>)>;
+type SelectionReceiver = Option<(u64, Sender<Option<usize>>)>;
 
 fn status_callback(
     status_rx: Receiver<StatusUpdate>,
@@ -361,6 +347,7 @@ fn status_callback(
     browsing_context_id: u64,
     controller: Controller,
     pin_receiver: Arc<Mutex<PinReceiver>>, /* Shared with an AuthrsTransport */
+    selection_receiver: Arc<Mutex<SelectionReceiver>>, /* Shared with an AuthrsTransport */
 ) {
     loop {
         match status_rx.recv() {
@@ -376,23 +363,13 @@ fn status_callback(
                 controller.send_prompt(tid, &notification_str);
             }
             Ok(StatusUpdate::PinUvError(StatusPinUv::PinRequired(sender))) => {
-                let guard = pin_receiver.lock();
+                pin_receiver.lock().unwrap().replace((tid, sender));
-                if let Ok(mut entry) = guard {
-                    entry.replace((tid, sender));
-                } else {
-                    return;
-                }
                 let notification_str =
                     make_pin_required_prompt(tid, origin, browsing_context_id, false, -1);
                 controller.send_prompt(tid, &notification_str);
             }
             Ok(StatusUpdate::PinUvError(StatusPinUv::InvalidPin(sender, attempts))) => {
-                let guard = pin_receiver.lock();
+                pin_receiver.lock().unwrap().replace((tid, sender));
-                if let Ok(mut entry) = guard {
-                    entry.replace((tid, sender));
-                } else {
-                    return;
-                }
                 let notification_str = make_pin_required_prompt(
                     tid,
                     origin,
@@ -437,8 +414,12 @@ fn status_callback(
             Ok(StatusUpdate::InteractiveManagement(_)) => {
                 debug!("STATUS: interactive management");
             }
-            Ok(StatusUpdate::SelectResultNotice(_, _)) => {
+            Ok(StatusUpdate::SelectResultNotice(sender, choices)) => {
-                // The selection prompt will be added in Bug 1854016
+                debug!("STATUS: select result notice");
+                selection_receiver.lock().unwrap().replace((tid, sender));
+                let notification_str =
+                    make_user_selection_prompt(tid, origin, browsing_context_id, &choices);
+                controller.send_prompt(tid, &notification_str);
             }
             Err(RecvError) => {
                 debug!("STATUS: end");
@@ -459,6 +440,7 @@ pub struct AuthrsTransport {
     test_token_manager: TestTokenManager,
     controller: Controller,
     pin_receiver: Arc<Mutex<PinReceiver>>,
+    selection_receiver: Arc<Mutex<SelectionReceiver>>,
 }
 
 impl AuthrsTransport {
@@ -480,7 +462,6 @@ impl AuthrsTransport {
     fn pin_callback(&self, transaction_id: u64, pin: &nsACString) -> Result<(), nsresult> {
         let mut guard = self.pin_receiver.lock().or(Err(NS_ERROR_FAILURE))?;
         match guard.take() {
-            // The pin_receiver is single-use.
             Some((tid, channel)) if tid == transaction_id => channel
                 .send(Pin::new(&pin.to_string()))
                 .or(Err(NS_ERROR_FAILURE)),
@@ -491,6 +472,20 @@ impl AuthrsTransport {
         }
     }
 
+    xpcom_method!(selection_callback => SelectionCallback(aTransactionId: u64, aSelection: u64));
+    fn selection_callback(&self, transaction_id: u64, selection: u64) -> Result<(), nsresult> {
+        let mut guard = self.selection_receiver.lock().or(Err(NS_ERROR_FAILURE))?;
+        match guard.take() {
+            Some((tid, channel)) if tid == transaction_id => channel
+                .send(Some(selection as usize))
+                .or(Err(NS_ERROR_FAILURE)),
+            // Either we weren't expecting a selection, or the controller is confused
+            // about which transaction is active. Neither is recoverable, so it's
+            // OK to drop the SelectionReceiver here.
+            _ => Err(NS_ERROR_FAILURE),
+        }
+    }
+
     // # Safety
     //
     // This will mutably borrow usb_token_manager through a RefCell. The caller must ensure that at
@@ -624,6 +619,7 @@ impl AuthrsTransport {
 
         let (status_tx, status_rx) = channel::<StatusUpdate>();
         let pin_receiver = self.pin_receiver.clone();
+        let selection_receiver = self.selection_receiver.clone();
         let controller = self.controller.clone();
         let status_origin = origin.to_string();
         RunnableBuilder::new(
@@ -636,6 +632,7 @@ impl AuthrsTransport {
                     browsing_context_id,
                     controller,
                     pin_receiver,
+                    selection_receiver,
                 )
             },
         )
@@ -755,6 +752,7 @@ impl AuthrsTransport {
 
         let (status_tx, status_rx) = channel::<StatusUpdate>();
         let pin_receiver = self.pin_receiver.clone();
+        let selection_receiver = self.selection_receiver.clone();
         let controller = self.controller.clone();
         let status_origin = origin.to_string();
         RunnableBuilder::new("AuthrsTransport::GetAssertion::StatusReceiver", move || {
@@ -765,6 +763,7 @@ impl AuthrsTransport {
                 browsing_context_id,
                 controller,
                 pin_receiver,
+                selection_receiver,
             )
         })
         .may_block(true)
@@ -830,9 +829,15 @@ impl AuthrsTransport {
     // most one WebAuthn transaction is active at any given time.
     xpcom_method!(cancel => Cancel());
     fn cancel(&self) -> Result<(), nsresult> {
-        // We may be waiting for a pin. Drop the channel to release the
+        // The transaction thread may be waiting for user input. Dropping the associated channel
-        // state machine from `ask_user_for_pin`.
+        // will cause the transaction to error out with a "CancelledByUser" result.
         drop(self.pin_receiver.lock().or(Err(NS_ERROR_FAILURE))?.take());
+        drop(
+            self.selection_receiver
+                .lock()
+                .or(Err(NS_ERROR_FAILURE))?
+                .take(),
+        );
 
         self.usb_token_manager.borrow_mut().cancel();
 
@@ -969,6 +974,7 @@ pub extern "C" fn authrs_transport_constructor(
         test_token_manager: TestTokenManager::new(),
         controller: Controller(RefCell::new(std::ptr::null())),
         pin_receiver: Arc::new(Mutex::new(None)),
+        selection_receiver: Arc::new(Mutex::new(None)),
     });
 
     #[cfg(feature = "fuzzing")]

dom/webauthn/authrs_bridge/src/test_token.rs

@@ -31,6 +31,7 @@ use authenticator::{ctap2, statecallback::StateCallback};
 use authenticator::{FidoDevice, FidoDeviceIO, FidoProtocol, VirtualFidoDevice};
 use authenticator::{RegisterResult, SignResult, StatusUpdate};
 use base64::Engine;
+use moz_task::RunnableBuilder;
 use nserror::{nsresult, NS_ERROR_FAILURE, NS_ERROR_INVALID_ARG, NS_ERROR_NOT_IMPLEMENTED, NS_OK};
 use nsstring::{nsACString, nsCString};
 use rand::{thread_rng, RngCore};
@@ -39,7 +40,7 @@ use std::collections::{hash_map::Entry, HashMap};
 use std::ops::{Deref, DerefMut};
 use std::sync::atomic::{AtomicU32, Ordering};
 use std::sync::mpsc::Sender;
-use std::sync::Mutex;
+use std::sync::{Arc, Mutex};
 use thin_vec::ThinVec;
 use xpcom::interfaces::nsICredentialParameters;
 use xpcom::{xpcom_method, RefPtr};
@@ -626,7 +627,7 @@ impl CredentialParameters {
 
 #[derive(Default)]
 pub(crate) struct TestTokenManager {
-    state: Mutex<HashMap<u64, TestToken>>,
+    state: Arc<Mutex<HashMap<u64, TestToken>>>,
 }
 
 impl TestTokenManager {
@@ -772,7 +773,7 @@ impl TestTokenManager {
 
     pub fn register(
         &self,
-        _timeout: u64,
+        _timeout_ms: u64,
         ctap_args: RegisterArgs,
         status: Sender<StatusUpdate>,
         callback: StateCallback<Result<RegisterResult, AuthenticatorError>>,
@@ -781,30 +782,37 @@ impl TestTokenManager {
             return;
         }
 
-        let mut state_obj = self.state.lock().unwrap();
+        let state_obj = self.state.clone();
 
-        // We query the tokens sequentially since the register operation will not block.
+        // Registration doesn't currently block, but it might in a future version, so we run it on
-        for token in state_obj.values_mut() {
+        // a background thread.
-            let _ = token.init();
+        let _ = RunnableBuilder::new("TestTokenManager::register", move || {
-            if ctap2::register(
+            // TODO(Bug 1854278) We should actually run one thread per token here
-                token,
+            // and attempt to fulfill this request in parallel.
-                ctap_args.clone(),
+            for token in state_obj.lock().unwrap().values_mut() {
-                status.clone(),
+                let _ = token.init();
-                callback.clone(),
+                if ctap2::register(
-                &|| true,
+                    token,
-            ) {
+                    ctap_args.clone(),
-                // callback was called
+                    status.clone(),
-                return;
+                    callback.clone(),
+                    &|| true,
+                ) {
+                    // callback was called
+                    return;
+                }
             }
-        }
 
-        // Send an error, if the callback wasn't called already.
+            // Send an error, if the callback wasn't called already.
-        callback.call(Err(AuthenticatorError::U2FToken(U2FTokenError::NotAllowed)));
+            callback.call(Err(AuthenticatorError::U2FToken(U2FTokenError::NotAllowed)));
+        })
+        .may_block(true)
+        .dispatch_background_task();
     }
 
     pub fn sign(
         &self,
-        _timeout: u64,
+        _timeout_ms: u64,
         ctap_args: SignArgs,
         status: Sender<StatusUpdate>,
         callback: StateCallback<Result<SignResult, AuthenticatorError>>,
@@ -813,23 +821,30 @@ impl TestTokenManager {
             return;
         }
 
-        let mut state_obj = self.state.lock().unwrap();
+        let state_obj = self.state.clone();
 
-        // We query the tokens sequentially since the sign operation will not block.
+        // Signing can block during signature selection, so we need to run it on a background thread.
-        for token in state_obj.values_mut() {
+        let _ = RunnableBuilder::new("TestTokenManager::sign", move || {
-            let _ = token.init();
+            // TODO(Bug 1854278) We should actually run one thread per token here
-            if ctap2::sign(
+            // and attempt to fulfill this request in parallel.
-                token,
+            for token in state_obj.lock().unwrap().values_mut() {
-                ctap_args.clone(),
+                let _ = token.init();
-                status.clone(),
+                if ctap2::sign(
-                callback.clone(),
+                    token,
-                &|| true,
+                    ctap_args.clone(),
-            ) {
+                    status.clone(),
-                // callback was called
+                    callback.clone(),
-                return;
+                    &|| true,
+                ) {
+                    // callback was called
+                    return;
+                }
             }
-        }
+
-        // Send an error, if the callback wasn't called already.
+            // Send an error, if the callback wasn't called already.
-        callback.call(Err(AuthenticatorError::U2FToken(U2FTokenError::NotAllowed)));
+            callback.call(Err(AuthenticatorError::U2FToken(U2FTokenError::NotAllowed)));
+        })
+        .may_block(true)
+        .dispatch_background_task();
     }
 }

dom/webauthn/nsIWebAuthnController.idl

@@ -194,7 +194,7 @@ interface nsIWebAuthnController : nsISupports
     // Authenticator callbacks
     [noscript] void sendPromptNotificationPreformatted(in uint64_t aTransactionId, in ACString aJSON);
     [noscript] void finishRegister(in uint64_t aTransactionId, in nsICtapRegisterResult aResult);
-    [noscript] void finishSign(in uint64_t aTransactionId, in Array<nsICtapSignResult> aResult);
+    [noscript] void finishSign(in uint64_t aTransactionId, in nsICtapSignResult aResult);
 };
 
 [scriptable, uuid(6c4ecd9f-57c0-4d7d-8080-bf6e4d499f8f)]
@@ -263,6 +263,7 @@ interface nsIWebAuthnTransport : nsISupports
 
     // These are prompt callbacks but they're not intended to be called directly from
     // JavaScript---they are proxied through the nsIWebAuthnController first.
+    [noscript] void selectionCallback(in uint64_t aTransactionId, in uint64_t aIndex);
     [noscript] void pinCallback(in uint64_t aTransactionId, in ACString aPin);
     [noscript] void cancel();
 };

dom/webauthn/tests/browser/browser_fido_appid_extension.js

@@ -19,7 +19,9 @@ add_task(async function test_appid() {
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
 
   // The FIDO AppId extension can't be used for MakeCredential.
-  await promiseWebAuthnMakeCredential(tab, "none", { appid: gAppId })
+  await promiseWebAuthnMakeCredential(tab, "none", "discouraged", {
+    appid: gAppId,
+  })
     .then(arrivingHereIsBad)
     .catch(expectNotSupportedError);
 

dom/webauthn/tests/browser/browser_webauthn_ipaddress.js

@@ -21,7 +21,7 @@ add_task(async function test_appid() {
   // Open a new tab.
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
 
-  await promiseWebAuthnMakeCredential(tab, "none", {})
+  await promiseWebAuthnMakeCredential(tab)
     .then(arrivingHereIsBad)
     .catch(expectSecurityError);
 

dom/webauthn/tests/browser/browser_webauthn_prompts.js

@@ -11,6 +11,7 @@ XPCOMUtils.defineLazyScriptGetter(
 );
 
 const TEST_URL = "https://example.com/";
+var gAuthenticatorId;
 
 add_task(async function test_setup_usbtoken() {
   return SpecialPowers.pushPrefEnv({
@@ -39,7 +40,7 @@ add_task(async function test_setup_fullscreen() {
 add_task(test_fullscreen_show_nav_toolbar);
 add_task(test_no_fullscreen_dom);
 add_task(async function test_setup_softtoken() {
-  add_virtual_authenticator();
+  gAuthenticatorId = add_virtual_authenticator();
   return SpecialPowers.pushPrefEnv({
     set: [
       ["security.webauth.webauthn_enable_softtoken", true],
@@ -49,6 +50,7 @@ add_task(async function test_setup_softtoken() {
 });
 add_task(test_register_direct_proceed);
 add_task(test_register_direct_proceed_anon);
+add_task(test_select_sign_result);
 
 function promiseNotification(id) {
   return new Promise(resolve => {
@@ -123,7 +125,7 @@ async function test_register() {
 
   // Request a new credential and wait for the prompt.
   let active = true;
-  let request = promiseWebAuthnMakeCredential(tab, "none", {})
+  let request = promiseWebAuthnMakeCredential(tab)
     .then(arrivingHereIsBad)
     .catch(expectNotAllowedError)
     .then(() => (active = false));
@@ -144,7 +146,7 @@ async function test_register_escape() {
 
   // Request a new credential and wait for the prompt.
   let active = true;
-  let request = promiseWebAuthnMakeCredential(tab, "none", {})
+  let request = promiseWebAuthnMakeCredential(tab)
     .then(arrivingHereIsBad)
     .catch(expectNotAllowedError)
     .then(() => (active = false));
@@ -207,7 +209,7 @@ async function test_register_direct_cancel() {
 
   // Request a new credential with direct attestation and wait for the prompt.
   let active = true;
-  let promise = promiseWebAuthnMakeCredential(tab, "direct", {})
+  let promise = promiseWebAuthnMakeCredential(tab, "direct")
     .then(arrivingHereIsBad)
     .catch(expectNotAllowedError)
     .then(() => (active = false));
@@ -230,7 +232,7 @@ async function test_tab_switching() {
 
   // Request a new credential and wait for the prompt.
   let active = true;
-  let request = promiseWebAuthnMakeCredential(tab_one, "none", {})
+  let request = promiseWebAuthnMakeCredential(tab_one)
     .then(arrivingHereIsBad)
     .catch(expectNotAllowedError)
     .then(() => (active = false));
@@ -276,7 +278,7 @@ async function test_window_switching() {
 
   // Request a new credential and wait for the prompt.
   let active = true;
-  let request = promiseWebAuthnMakeCredential(tab, "none", {})
+  let request = promiseWebAuthnMakeCredential(tab)
     .then(arrivingHereIsBad)
     .catch(expectNotAllowedError)
     .then(() => (active = false));
@@ -324,7 +326,7 @@ async function test_register_direct_proceed() {
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
 
   // Request a new credential with direct attestation and wait for the prompt.
-  let request = promiseWebAuthnMakeCredential(tab, "direct", {});
+  let request = promiseWebAuthnMakeCredential(tab, "direct");
   await promiseNotification("webauthn-prompt-register-direct");
 
   // Proceed.
@@ -342,7 +344,7 @@ async function test_register_direct_proceed_anon() {
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
 
   // Request a new credential with direct attestation and wait for the prompt.
-  let request = promiseWebAuthnMakeCredential(tab, "direct", {});
+  let request = promiseWebAuthnMakeCredential(tab, "direct");
   await promiseNotification("webauthn-prompt-register-direct");
 
   // Check "anonymize anyway" and proceed.
@@ -356,6 +358,35 @@ async function test_register_direct_proceed_anon() {
   await BrowserTestUtils.removeTab(tab);
 }
 
+async function test_select_sign_result() {
+  // Open a new tab.
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
+
+  // Make two discoverable credentials for the same RP ID so that
+  // the user has to select one to return.
+  let cred1 = await addCredential(gAuthenticatorId, "example.com");
+  let cred2 = await addCredential(gAuthenticatorId, "example.com");
+
+  let active = true;
+  let request = promiseWebAuthnGetAssertionDiscoverable(tab)
+    .then(arrivingHereIsBad)
+    .catch(expectNotAllowedError)
+    .then(() => (active = false));
+
+  // Ensure the selection prompt is shown
+  await promiseNotification("webauthn-prompt-select-sign-result");
+
+  ok(active, "request is active");
+
+  // Cancel the request
+  PopupNotifications.panel.firstElementChild.button.click();
+  await request;
+
+  await removeCredential(gAuthenticatorId, cred1);
+  await removeCredential(gAuthenticatorId, cred2);
+  await BrowserTestUtils.removeTab(tab);
+}
+
 async function test_fullscreen_show_nav_toolbar() {
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
 
@@ -375,7 +406,7 @@ async function test_fullscreen_show_nav_toolbar() {
   let navToolboxShownPromise = promiseNavToolboxStatus("shown");
 
   let active = true;
-  let requestPromise = promiseWebAuthnMakeCredential(tab, "direct", {})
+  let requestPromise = promiseWebAuthnMakeCredential(tab, "direct")
     .then(arrivingHereIsBad)
     .catch(expectNotAllowedError)
     .then(() => (active = false));
@@ -412,7 +443,7 @@ async function test_no_fullscreen_dom() {
   fullScreenPaintPromise = promiseFullScreenPaint();
 
   let active = true;
-  let requestPromise = promiseWebAuthnMakeCredential(tab, "direct", {})
+  let requestPromise = promiseWebAuthnMakeCredential(tab, "direct")
     .then(arrivingHereIsBad)
     .catch(expectNotAllowedError)
     .then(() => (active = false));

dom/webauthn/tests/browser/head.js

@@ -121,12 +121,13 @@ function expectError(aType) {
 function promiseWebAuthnMakeCredential(
   tab,
   attestation = "none",
+  residentKey = "discouraged",
   extensions = {}
 ) {
   return ContentTask.spawn(
     tab.linkedBrowser,
-    [attestation, extensions],
+    [attestation, residentKey, extensions],
-    ([attestation, extensions]) => {
+    ([attestation, residentKey, extensions]) => {
       const cose_alg_ECDSA_w_SHA256 = -7;
 
       let challenge = content.crypto.getRandomValues(new Uint8Array(16));
@@ -146,6 +147,10 @@ function promiseWebAuthnMakeCredential(
           displayName: "none",
         },
         pubKeyCredParams,
+        authenticatorSelection: {
+          authenticatorAttachment: "cross-platform",
+          residentKey,
+        },
         extensions,
         attestation,
         challenge,
@@ -201,6 +206,21 @@ function promiseWebAuthnGetAssertion(tab, key_handle = null, extensions = {}) {
   );
 }
 
+function promiseWebAuthnGetAssertionDiscoverable(tab, extensions = {}) {
+  return ContentTask.spawn(tab.linkedBrowser, [extensions], ([extensions]) => {
+    let challenge = content.crypto.getRandomValues(new Uint8Array(16));
+
+    let publicKey = {
+      challenge,
+      extensions,
+      rpId: content.document.domain,
+      allowCredentials: [],
+    };
+
+    return content.navigator.credentials.get({ publicKey });
+  });
+}
+
 function checkRpIdHash(rpIdHash, hostname) {
   return crypto.subtle
     .digest("SHA-256", string2buffer(hostname))