From 26aa7664ce65da81b4980475c0ee530bd2019d78 Mon Sep 17 00:00:00 2001 From: Martin Sirringhaus <> Date: Mon, 23 Oct 2023 11:50:47 +0200 Subject: [PATCH] Detect if no devices are connected and send a StatusUpdate accordingly. --- examples/ctap2.rs | 3 + examples/ctap2_discoverable_creds.rs | 21 ++++-- examples/interactive_management.rs | 3 + examples/reset.rs | 3 + examples/set_pin.rs | 3 + examples/test_exclude_list.rs | 3 + src/ctap2/commands/mod.rs | 4 +- src/status_update.rs | 2 + src/transport/device_selector.rs | 97 ++++++++++++++++++++++++++-- src/transport/freebsd/transaction.rs | 2 +- src/transport/linux/transaction.rs | 2 +- src/transport/macos/transaction.rs | 2 +- src/transport/netbsd/monitor.rs | 17 +++-- src/transport/netbsd/transaction.rs | 2 +- src/transport/openbsd/monitor.rs | 12 ++-- src/transport/openbsd/transaction.rs | 2 +- src/transport/stub/transaction.rs | 4 +- src/transport/windows/monitor.rs | 13 ++-- src/transport/windows/transaction.rs | 2 +- 19 files changed, 160 insertions(+), 37 deletions(-) diff --git a/examples/ctap2.rs b/examples/ctap2.rs index be742a5e..9c5e161f 100644 --- a/examples/ctap2.rs +++ b/examples/ctap2.rs @@ -84,6 +84,9 @@ fn main() { let (status_tx, status_rx) = channel::(); thread::spawn(move || loop { match status_rx.recv() { + Ok(StatusUpdate::NoDevicesFound) => { + println!("STATUS: No device found. Please connect one!"); + } Ok(StatusUpdate::InteractiveManagement(..)) => { panic!("STATUS: This can't happen when doing non-interactive usage"); } diff --git a/examples/ctap2_discoverable_creds.rs b/examples/ctap2_discoverable_creds.rs index d19ccc6f..f9893e91 100644 --- a/examples/ctap2_discoverable_creds.rs +++ b/examples/ctap2_discoverable_creds.rs @@ -15,9 +15,9 @@ use authenticator::{ }; use getopts::Options; use sha2::{Digest, Sha256}; +use std::io::Write; use std::sync::mpsc::{channel, RecvError}; use std::{env, io, thread}; -use std::io::Write; fn print_usage(program: &str, opts: Options) { println!("------------------------------------------------------------------------"); @@ -84,6 +84,9 @@ fn register_user(manager: &mut AuthenticatorService, username: &str, timeout_ms: Ok(StatusUpdate::InteractiveManagement(..)) => { panic!("STATUS: This can't happen when doing non-interactive usage"); } + Ok(StatusUpdate::NoDevicesFound) => { + println!("STATUS: No device found. Please connect one!"); + } Ok(StatusUpdate::SelectDeviceNotice) => { println!("STATUS: Please select a device by touching one of them."); } @@ -216,10 +219,7 @@ fn main() { "timeout in seconds", "SEC", ); - opts.optflag( - "s", - "skip_reg", - "Skip registration"); + opts.optflag("s", "skip_reg", "Skip registration"); opts.optflag("h", "help", "print this help menu"); let matches = match opts.parse(&args[1..]) { @@ -273,6 +273,9 @@ fn main() { Ok(StatusUpdate::InteractiveManagement(..)) => { panic!("STATUS: This can't happen when doing non-interactive usage"); } + Ok(StatusUpdate::NoDevicesFound) => { + println!("STATUS: No device found. Please connect one!"); + } Ok(StatusUpdate::SelectDeviceNotice) => { println!("STATUS: Please select a device by touching one of them."); } @@ -368,7 +371,13 @@ fn main() { println!("Found credentials:"); println!( "{:?}", - assertion_object.assertion.user.clone().unwrap().name.unwrap() // Unwrapping here, as these shouldn't fail + assertion_object + .assertion + .user + .clone() + .unwrap() + .name + .unwrap() // Unwrapping here, as these shouldn't fail ); println!("-----------------------------------------------------------------"); println!("Done."); diff --git a/examples/interactive_management.rs b/examples/interactive_management.rs index 6ef0e9dc..bc53945c 100644 --- a/examples/interactive_management.rs +++ b/examples/interactive_management.rs @@ -680,6 +680,9 @@ fn interactive_status_callback(status_rx: Receiver) { ); continue; } + Ok(StatusUpdate::NoDevicesFound) => { + println!("STATUS: No device found. Please connect one!"); + } Ok(StatusUpdate::SelectDeviceNotice) => { println!("STATUS: Please select a device by touching one of them."); } diff --git a/examples/reset.rs b/examples/reset.rs index 867ab544..0e7506e7 100644 --- a/examples/reset.rs +++ b/examples/reset.rs @@ -93,6 +93,9 @@ fn main() { loop { match status_rx.recv() { + Ok(StatusUpdate::NoDevicesFound) => { + println!("STATUS: No device found. Please connect one!"); + } Ok(StatusUpdate::SelectDeviceNotice) => { println!("ERROR: Please unplug all other tokens that should not be reset!"); // Needed to give the tokens enough time to start blinking diff --git a/examples/set_pin.rs b/examples/set_pin.rs index 5534ca08..3e94cddf 100644 --- a/examples/set_pin.rs +++ b/examples/set_pin.rs @@ -68,6 +68,9 @@ fn main() { Ok(StatusUpdate::InteractiveManagement(..)) => { panic!("STATUS: This can't happen when doing non-interactive usage"); } + Ok(StatusUpdate::NoDevicesFound) => { + println!("STATUS: No device found. Please connect one!"); + } Ok(StatusUpdate::SelectDeviceNotice) => { println!("STATUS: Please select a device by touching one of them."); } diff --git a/examples/test_exclude_list.rs b/examples/test_exclude_list.rs index e24c49d0..0b827a71 100644 --- a/examples/test_exclude_list.rs +++ b/examples/test_exclude_list.rs @@ -82,6 +82,9 @@ fn main() { Ok(StatusUpdate::InteractiveManagement(..)) => { panic!("STATUS: This can't happen when doing non-interactive usage"); } + Ok(StatusUpdate::NoDevicesFound) => { + println!("STATUS: No device found. Please connect one!"); + } Ok(StatusUpdate::SelectDeviceNotice) => { println!("STATUS: Please select a device by touching one of them."); } diff --git a/src/ctap2/commands/mod.rs b/src/ctap2/commands/mod.rs index 926a7ea2..fa270c4b 100644 --- a/src/ctap2/commands/mod.rs +++ b/src/ctap2/commands/mod.rs @@ -1,7 +1,5 @@ use crate::crypto::{CryptoError, PinUvAuthParam, PinUvAuthToken}; -use crate::ctap2::commands::client_pin::{ - GetPinRetries, GetUvRetries, PinError, -}; +use crate::ctap2::commands::client_pin::{GetPinRetries, GetUvRetries, PinError}; use crate::ctap2::commands::get_info::AuthenticatorInfo; use crate::ctap2::server::UserVerificationRequirement; use crate::errors::AuthenticatorError; diff --git a/src/status_update.rs b/src/status_update.rs index c4a7fee7..f0147b23 100644 --- a/src/status_update.rs +++ b/src/status_update.rs @@ -107,6 +107,8 @@ pub enum StatusUpdate { InteractiveManagement(InteractiveUpdate), /// Sent when a token returns multiple results for a getAssertion request SelectResultNotice(Sender>, Vec), + /// Inform user that no devices are plugged in + NoDevicesFound, } pub(crate) fn send_status(status: &Sender, msg: StatusUpdate) { diff --git a/src/transport/device_selector.rs b/src/transport/device_selector.rs index 80bca404..40a68e4d 100644 --- a/src/transport/device_selector.rs +++ b/src/transport/device_selector.rs @@ -1,4 +1,6 @@ +use crate::status_update::send_status; use crate::transport::hid::HIDDevice; +use crate::StatusUpdate; pub use crate::transport::platform::device::Device; @@ -45,7 +47,7 @@ pub struct DeviceSelector { } impl DeviceSelector { - pub fn run() -> Self { + pub fn run(status: Sender) -> Self { let (selector_send, selector_rec) = channel(); // let new_device_callback = Arc::new(new_device_cb); let runloop = RunLoop::new(move |alive| { @@ -77,6 +79,9 @@ impl DeviceSelector { break; // We are done here. The selected device continues without us. } DeviceSelectorEvent::DevicesAdded(ids) => { + if ids.is_empty() && waiting_for_response.is_empty() && tokens.is_empty() { + send_status(&status, StatusUpdate::NoDevicesFound); + } for id in ids { debug!("Device added event: {:?}", id); waiting_for_response.insert(id); @@ -99,9 +104,15 @@ impl DeviceSelector { tokens.retain(|dev_id, _| dev_id != id); if tokens.is_empty() { blinking = false; + if waiting_for_response.is_empty() { + send_status(&status, StatusUpdate::NoDevicesFound); + } continue; } } + if waiting_for_response.is_empty() && tokens.is_empty() { + send_status(&status, StatusUpdate::NoDevicesFound); + } // We are already blinking, so no need to run the code below this match // that figures out if we should blink or not. In fact, currently, we do // NOT want to run this code again, because if you have 2 blinking tokens @@ -115,6 +126,9 @@ impl DeviceSelector { DeviceSelectorEvent::NotAToken(ref id) => { debug!("Device not a token event: {:?}", id); waiting_for_response.remove(id); + if waiting_for_response.is_empty() && tokens.is_empty() { + send_status(&status, StatusUpdate::NoDevicesFound); + } } DeviceSelectorEvent::ImAToken((id, tx)) => { let _ = waiting_for_response.remove(&id); @@ -187,6 +201,7 @@ pub mod tests { transport::FidoDevice, u2ftypes::U2FDeviceInfo, }; + use std::sync::mpsc::TryRecvError; fn gen_info(id: String) -> U2FDeviceInfo { U2FDeviceInfo { @@ -267,9 +282,10 @@ pub mod tests { Device::new("device selector 4").unwrap(), ]; + let (tx, rx) = channel(); // Make those actual tokens. The rest is interpreted as non-u2f-devices make_device_with_pin(&mut devices[2]); - let selector = DeviceSelector::run(); + let selector = DeviceSelector::run(tx); // Adding all add_devices(devices.iter(), &selector); @@ -278,6 +294,7 @@ pub mod tests { send_no_token(d, &selector); } }); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); send_i_am_token(&devices[2], &selector); @@ -292,18 +309,24 @@ pub mod tests { fn test_device_selector_stop() { let device = Device::new("device selector 1").unwrap(); - let mut selector = DeviceSelector::run(); + let (tx, rx) = channel(); + let mut selector = DeviceSelector::run(tx); // Adding all selector .clone_sender() .send(DeviceSelectorEvent::DevicesAdded(vec![device.id()])) .unwrap(); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); selector .clone_sender() .send(DeviceSelectorEvent::NotAToken(device.id())) .unwrap(); + assert_matches!( + rx.recv_timeout(Duration::from_millis(500)), + Ok(StatusUpdate::NoDevicesFound) + ); selector.stop(); } @@ -323,7 +346,8 @@ pub mod tests { make_device_with_pin(&mut devices[4]); make_device_with_pin(&mut devices[5]); - let selector = DeviceSelector::run(); + let (tx, rx) = channel(); + let selector = DeviceSelector::run(tx); // Adding all, except the last one (we simulate that this one is not yet plugged in) add_devices(devices.iter().take(5), &selector); @@ -355,6 +379,7 @@ pub mod tests { devices[5].receiver.as_ref().unwrap().recv().unwrap(), DeviceCommand::Blink ); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); } #[test] @@ -375,7 +400,8 @@ pub mod tests { make_device_simple_u2f(&mut devices[4]); make_device_simple_u2f(&mut devices[5]); - let selector = DeviceSelector::run(); + let (tx, rx) = channel(); + let selector = DeviceSelector::run(tx); // Adding all, except the last one (we simulate that this one is not yet plugged in) add_devices(devices.iter().take(5), &selector); @@ -417,6 +443,7 @@ pub mod tests { devices[6].receiver.as_ref().unwrap().recv().unwrap(), DeviceCommand::Blink ); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); } #[test] @@ -437,7 +464,8 @@ pub mod tests { make_device_with_pin(&mut devices[4]); make_device_with_pin(&mut devices[5]); - let selector = DeviceSelector::run(); + let (tx, rx) = channel(); + let selector = DeviceSelector::run(tx); // Adding all, except the last one (we simulate that this one is not yet plugged in) add_devices(devices.iter(), &selector); @@ -456,11 +484,16 @@ pub mod tests { DeviceCommand::Blink ); } + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); // Remove all tokens for idx in [2, 4, 5] { remove_device(&devices[idx], &selector); } + assert_matches!( + rx.recv_timeout(Duration::from_millis(500)), + Ok(StatusUpdate::NoDevicesFound) + ); // Adding one again send_i_am_token(&devices[4], &selector); @@ -471,4 +504,56 @@ pub mod tests { DeviceCommand::Continue ); } + + #[test] + fn test_device_selector_no_devices() { + let mut devices = vec![ + Device::new("device selector 1").unwrap(), + Device::new("device selector 2").unwrap(), + Device::new("device selector 3").unwrap(), + Device::new("device selector 4").unwrap(), + ]; + + let (tx, rx) = channel(); + // Make those actual tokens. The rest is interpreted as non-u2f-devices + make_device_with_pin(&mut devices[2]); + make_device_with_pin(&mut devices[3]); + let selector = DeviceSelector::run(tx); + + // Adding no devices first (none are plugged in when we start) + add_devices(std::iter::empty(), &selector); + assert_matches!( + rx.recv_timeout(Duration::from_millis(500)), + Ok(StatusUpdate::NoDevicesFound) + ); + + // Adding the devices + add_devices(devices.iter(), &selector); + devices.iter_mut().for_each(|d| { + if !d.is_u2f() { + send_no_token(d, &selector); + } + }); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); + + send_i_am_token(&devices[2], &selector); + send_i_am_token(&devices[3], &selector); + + assert_eq!( + devices[2].receiver.as_ref().unwrap().recv().unwrap(), + DeviceCommand::Blink + ); + assert_eq!( + devices[3].receiver.as_ref().unwrap().recv().unwrap(), + DeviceCommand::Blink + ); + + // Removing all blinking devices + remove_device(&devices[2], &selector); + remove_device(&devices[3], &selector); + assert_matches!( + rx.recv_timeout(Duration::from_millis(500)), + Ok(StatusUpdate::NoDevicesFound) + ); + } } diff --git a/src/transport/freebsd/transaction.rs b/src/transport/freebsd/transaction.rs index 6b15f675..b7bd0632 100644 --- a/src/transport/freebsd/transaction.rs +++ b/src/transport/freebsd/transaction.rs @@ -35,7 +35,7 @@ impl Transaction { + 'static, T: 'static, { - let device_selector = DeviceSelector::run(); + let device_selector = DeviceSelector::run(status.clone()); let selector_sender = device_selector.clone_sender(); let thread = RunLoop::new_with_timeout( move |alive| { diff --git a/src/transport/linux/transaction.rs b/src/transport/linux/transaction.rs index 6b15f675..b7bd0632 100644 --- a/src/transport/linux/transaction.rs +++ b/src/transport/linux/transaction.rs @@ -35,7 +35,7 @@ impl Transaction { + 'static, T: 'static, { - let device_selector = DeviceSelector::run(); + let device_selector = DeviceSelector::run(status.clone()); let selector_sender = device_selector.clone_sender(); let thread = RunLoop::new_with_timeout( move |alive| { diff --git a/src/transport/macos/transaction.rs b/src/transport/macos/transaction.rs index d9709e73..0385d7bb 100644 --- a/src/transport/macos/transaction.rs +++ b/src/transport/macos/transaction.rs @@ -46,7 +46,7 @@ impl Transaction { { let (tx, rx) = channel(); let timeout = (timeout as f64) / 1000.0; - let device_selector = DeviceSelector::run(); + let device_selector = DeviceSelector::run(status.clone()); let selector_sender = device_selector.clone_sender(); let builder = thread::Builder::new(); let thread = builder diff --git a/src/transport/netbsd/monitor.rs b/src/transport/netbsd/monitor.rs index c521bdea..ea366fd0 100644 --- a/src/transport/netbsd/monitor.rs +++ b/src/transport/netbsd/monitor.rs @@ -65,15 +65,13 @@ where pub fn run(&mut self, alive: &dyn Fn() -> bool) -> Result<(), Box> { // Loop until we're stopped by the controlling thread, or fail. while alive() { + let mut added = Vec::new(); for n in 0..100 { let uhidpath = OsString::from(format!("/dev/uhid{n}")); match Fd::open(&uhidpath, libc::O_RDWR | libc::O_CLOEXEC) { Ok(uhid) => { // The device is available if it can be opened. - let _ = self - .selector_sender - .send(DeviceSelectorEvent::DevicesAdded(vec![uhidpath.clone()])); - self.add_device(WrappedOpenDevice { + added.push(WrappedOpenDevice { fd: uhid, os_path: uhidpath, }); @@ -85,6 +83,17 @@ where }, } } + + // We have to notify additions in batches to avoid + // arbitrarily selecting the first added device and + // to know when there are no devices present (then + // we send an empty vec here). + let _ = self.selector_sender.send(DeviceSelectorEvent::DevicesAdded( + added.iter().map(|e| e.os_path.clone()).collect(), + )); + for device in added { + self.add_device(device); + } thread::sleep(Duration::from_millis(POLL_TIMEOUT)); } diff --git a/src/transport/netbsd/transaction.rs b/src/transport/netbsd/transaction.rs index 6b15f675..b7bd0632 100644 --- a/src/transport/netbsd/transaction.rs +++ b/src/transport/netbsd/transaction.rs @@ -35,7 +35,7 @@ impl Transaction { + 'static, T: 'static, { - let device_selector = DeviceSelector::run(); + let device_selector = DeviceSelector::run(status.clone()); let selector_sender = device_selector.clone_sender(); let thread = RunLoop::new_with_timeout( move |alive| { diff --git a/src/transport/openbsd/monitor.rs b/src/transport/openbsd/monitor.rs index 0ea5f3d0..38545acb 100644 --- a/src/transport/openbsd/monitor.rs +++ b/src/transport/openbsd/monitor.rs @@ -65,6 +65,7 @@ where pub fn run(&mut self, alive: &dyn Fn() -> bool) -> Result<(), Box> { // Loop until we're stopped by the controlling thread, or fail. while alive() { + let mut added = Vec::new(); // Iterate the first 10 fido(4) devices. for path in (0..10) .map(|unit| PathBuf::from(&format!("/dev/fido/{}", unit))) @@ -78,10 +79,7 @@ where match from_unix_result(fd) { Ok(fd) => { // The device is available if it can be opened. - let _ = self - .selector_sender - .send(DeviceSelectorEvent::DevicesAdded(vec![os_path.clone()])); - self.add_device(WrappedOpenDevice { fd, os_path }); + added.push(WrappedOpenDevice { fd, os_path }); } Err(ref err) if err.raw_os_error() == Some(libc::EBUSY) => { // The device is available but currently in use. @@ -92,6 +90,12 @@ where } } } + let _ = self.selector_sender.send(DeviceSelectorEvent::DevicesAdded( + added.iter().map(|e| e.os_path.clone()).collect(), + )); + for device in added { + self.add_device(device); + } thread::sleep(Duration::from_millis(POLL_TIMEOUT)); } diff --git a/src/transport/openbsd/transaction.rs b/src/transport/openbsd/transaction.rs index 6b15f675..b7bd0632 100644 --- a/src/transport/openbsd/transaction.rs +++ b/src/transport/openbsd/transaction.rs @@ -35,7 +35,7 @@ impl Transaction { + 'static, T: 'static, { - let device_selector = DeviceSelector::run(); + let device_selector = DeviceSelector::run(status.clone()); let selector_sender = device_selector.clone_sender(); let thread = RunLoop::new_with_timeout( move |alive| { diff --git a/src/transport/stub/transaction.rs b/src/transport/stub/transaction.rs index d471c94d..21a9687e 100644 --- a/src/transport/stub/transaction.rs +++ b/src/transport/stub/transaction.rs @@ -16,7 +16,7 @@ impl Transaction { pub fn new( timeout: u64, callback: StateCallback>, - _status: Sender, + status: Sender, new_device_cb: F, ) -> crate::Result where @@ -31,7 +31,7 @@ impl Transaction { T: 'static, { // Just to silence "unused"-warnings - let mut device_selector = DeviceSelector::run(); + let mut device_selector = DeviceSelector::run(status); let _ = DeviceSelectorEvent::DevicesAdded(vec![]); let _ = DeviceSelectorEvent::DeviceRemoved(PathBuf::new()); let _ = device_selector.clone_sender(); diff --git a/src/transport/windows/monitor.rs b/src/transport/windows/monitor.rs index c73c012b..a3c92582 100644 --- a/src/transport/windows/monitor.rs +++ b/src/transport/windows/monitor.rs @@ -60,12 +60,13 @@ where let added: Vec = current.difference(&previous).cloned().collect(); // We have to notify additions in batches to avoid - // arbitrarily selecting the first added device. - if !added.is_empty() - && self - .selector_sender - .send(DeviceSelectorEvent::DevicesAdded(added.clone())) - .is_err() + // arbitrarily selecting the first added device and + // to know when there are no devices present (then + // we send an empty vec here). + if self + .selector_sender + .send(DeviceSelectorEvent::DevicesAdded(added.clone())) + .is_err() { // Send only fails if the receiver hung up. We should exit the loop. break; diff --git a/src/transport/windows/transaction.rs b/src/transport/windows/transaction.rs index 6b15f675..b7bd0632 100644 --- a/src/transport/windows/transaction.rs +++ b/src/transport/windows/transaction.rs @@ -35,7 +35,7 @@ impl Transaction { + 'static, T: 'static, { - let device_selector = DeviceSelector::run(); + let device_selector = DeviceSelector::run(status.clone()); let selector_sender = device_selector.clone_sender(); let thread = RunLoop::new_with_timeout( move |alive| {