Bug 1820886 - vendor authenticator-rs v0.4.0-alpha10. r=keeler,supply…

…-chain-reviewers Differential Revision: https://phabricator.services.mozilla.com/D171913
iaxat · Mar 7, 2023 · b1e9c77 · b1e9c77
1 parent 8dba904
commit b1e9c77
Show file tree

Hide file tree

Showing 11 changed files with 147 additions and 93 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/supply-chain/audits.toml b/supply-chain/audits.toml
@@ -202,7 +202,7 @@ delta = "0.1.8 -> 0.1.9"
 [[audits.authenticator]]
 who = "John M. Schanck <[email protected]>"
 criteria = "safe-to-deploy"
-version = "0.4.0-alpha.9"
+version = "0.4.0-alpha.10"
 notes = "Maintained by the CryptoEng team at Mozilla."
 
 [[audits.autocfg]]

diff --git a/third_party/rust/authenticator/.cargo-checksum.json b/third_party/rust/authenticator/.cargo-checksum.json
diff --git a/third_party/rust/authenticator/Cargo.lock b/third_party/rust/authenticator/Cargo.lock
diff --git a/third_party/rust/authenticator/Cargo.toml b/third_party/rust/authenticator/Cargo.toml
@@ -12,7 +12,7 @@
 [package]
 edition = "2018"
 name = "authenticator"
-version = "0.4.0-alpha.9"
+version = "0.4.0-alpha.10"
 authors = [
     "J.C. Jones <[email protected]>",
     "Tim Taubert <[email protected]>",

diff --git a/third_party/rust/authenticator/src/consts.rs b/third_party/rust/authenticator/src/consts.rs
@@ -130,16 +130,6 @@ bitflags! {
     }
 }
 
-impl Capability {
-    pub fn has_fido1(self) -> bool {
-        !self.contains(Capability::NMSG)
-    }
-
-    pub fn has_fido2(self) -> bool {
-        self.contains(Capability::CBOR)
-    }
-}
-
 // Low-level error codes. Return as negatives.
 
 pub const ERR_NONE: u8 = 0x00; // No error

diff --git a/third_party/rust/authenticator/src/statemachine.rs b/third_party/rust/authenticator/src/statemachine.rs
@@ -308,6 +308,7 @@ impl StateMachineCtap2 {
         info: DeviceBuildParameters,
         selector: &Sender<DeviceSelectorEvent>,
         ctap2_only: bool,
+        keep_alive: &dyn Fn() -> bool,
     ) -> Option<Device> {
         // Create a new device.
         let mut dev = match Device::new(info) {
@@ -349,9 +350,16 @@ impl StateMachineCtap2 {
             .send(DeviceSelectorEvent::ImAToken((write_only_clone, tx)))
             .ok()?;
 
+        // We can be cancelled from the user (through keep_alive()) or from the device selector
+        // (through a DeviceCommand::Cancel on rx).  We'll combine those signals into a single
+        // predicate to pass to Device::block_and_blink.
+        let keep_blinking = || {
+            keep_alive() && !matches!(rx.try_recv(), Ok(DeviceCommand::Cancel))
+        };
+
         // Blocking recv. DeviceSelector will tell us what to do
         match rx.recv() {
-            Ok(DeviceCommand::Blink) => match dev.block_and_blink() {
+            Ok(DeviceCommand::Blink) => match dev.block_and_blink(&keep_blinking) {
                 BlinkResult::DeviceSelected => {
                     // User selected us. Let DeviceSelector know, so it can cancel all other
                     // outstanding open blink-requests.
@@ -364,6 +372,10 @@ impl StateMachineCtap2 {
                     return None;
                 }
             },
+            Ok(DeviceCommand::Cancel) => {
+                info!("Device {:?} was not selected", dev.id());
+                return None;
+            }
             Ok(DeviceCommand::Removed) => {
                 info!("Device {:?} was removed", dev.id());
                 return None;
@@ -448,8 +460,8 @@ impl StateMachineCtap2 {
             timeout,
             cbc.clone(),
             status,
-            move |info, selector, status, _alive| {
-                let mut dev = match Self::init_and_select(info, &selector, false) {
+            move |info, selector, status, alive| {
+                let mut dev = match Self::init_and_select(info, &selector, false, alive) {
                     None => {
                         return;
                     }
@@ -505,7 +517,7 @@ impl StateMachineCtap2 {
                 debug!("------------------------------------------------------------------");
                 debug!("{:?}", makecred);
                 debug!("------------------------------------------------------------------");
-                let resp = dev.send_msg(&makecred);
+                let resp = dev.send_msg_cancellable(&makecred, alive);
                 if resp.is_ok() {
                     send_status(
                         &status,
@@ -557,8 +569,8 @@ impl StateMachineCtap2 {
             timeout,
             callback.clone(),
             status,
-            move |info, selector, status, _alive| {
-                let mut dev = match Self::init_and_select(info, &selector, false) {
+            move |info, selector, status, alive| {
+                let mut dev = match Self::init_and_select(info, &selector, false, alive) {
                     None => {
                         return;
                     }
@@ -607,14 +619,17 @@ impl StateMachineCtap2 {
                 debug!("{:?}", getassertion);
                 debug!("------------------------------------------------------------------");
 
-                let mut resp = dev.send_msg(&getassertion);
+                let mut resp = dev.send_msg_cancellable(&getassertion, alive);
                 if resp.is_err() {
                     // Retry with a different RP ID if one was supplied. This is intended to be
                     // used with the AppID provided in the WebAuthn FIDO AppID extension.
                     if let Some(alternate_rp_id) = getassertion.alternate_rp_id {
-                        getassertion.rp = RelyingPartyWrapper::Data(RelyingParty{id: alternate_rp_id, ..Default::default()});
+                        getassertion.rp = RelyingPartyWrapper::Data(RelyingParty {
+                            id: alternate_rp_id,
+                            ..Default::default()
+                        });
                         getassertion.alternate_rp_id = None;
-                        resp = dev.send_msg(&getassertion);
+                        resp = dev.send_msg_cancellable(&getassertion, alive);
                     }
                 }
                 if resp.is_ok() {
@@ -682,9 +697,9 @@ impl StateMachineCtap2 {
             timeout,
             callback.clone(),
             status,
-            move |info, selector, status, _alive| {
+            move |info, selector, status, alive| {
                 let reset = Reset {};
-                let mut dev = match Self::init_and_select(info, &selector, true) {
+                let mut dev = match Self::init_and_select(info, &selector, true, alive) {
                     None => {
                         return;
                     }
@@ -696,7 +711,7 @@ impl StateMachineCtap2 {
                 debug!("{:?}", reset);
                 debug!("------------------------------------------------------------------");
 
-                let resp = dev.send_cbor(&reset);
+                let resp = dev.send_cbor_cancellable(&reset, alive);
                 if resp.is_ok() {
                     send_status(
                         &status,
@@ -745,8 +760,8 @@ impl StateMachineCtap2 {
             timeout,
             callback.clone(),
             status,
-            move |info, selector, status, _alive| {
-                let mut dev = match Self::init_and_select(info, &selector, true) {
+            move |info, selector, status, alive| {
+                let mut dev = match Self::init_and_select(info, &selector, true, alive) {
                     None => {
                         return;
                     }
@@ -793,7 +808,7 @@ impl StateMachineCtap2 {
                             &new_pin,
                         )
                         .map_err(HIDError::Command)
-                        .and_then(|msg| dev.send_cbor(&msg))
+                        .and_then(|msg| dev.send_cbor_cancellable(&msg, alive))
                         .map_err(AuthenticatorError::HIDError)
                         .map_err(|e| repackage_pin_errors(&mut dev, e));
 
@@ -819,7 +834,7 @@ impl StateMachineCtap2 {
                 } else {
                     SetNewPin::new(&authinfo, &shared_secret, &new_pin)
                         .map_err(HIDError::Command)
-                        .and_then(|msg| dev.send_cbor(&msg))
+                        .and_then(|msg| dev.send_cbor_cancellable(&msg, alive))
                         .map_err(AuthenticatorError::HIDError)
                 };
                 callback.call(res);

diff --git a/third_party/rust/authenticator/src/transport/device_selector.rs b/third_party/rust/authenticator/src/transport/device_selector.rs
@@ -21,6 +21,7 @@ pub enum BlinkResult {
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum DeviceCommand {
     Blink,
+    Cancel,
     Continue,
     Removed,
 }
@@ -190,10 +191,11 @@ impl DeviceSelector {
     }
 
     fn cancel_all(tokens: HashMap<Device, Sender<DeviceCommand>>, exclude: Option<&DeviceID>) {
-        tokens
-            .into_keys()
-            .filter(|x| exclude.map_or(true, |y| y != &x.id()))
-            .for_each(|mut dev| dev.cancel().unwrap()); // TODO
+        for (dev, tx) in tokens.iter() {
+            if Some(&dev.id()) != exclude {
+                let _ = tx.send(DeviceCommand::Cancel);
+            }
+        }
     }
 
     pub fn stop(&mut self) {