From b2f12ee4afbba1b83909ac7eb784f469b03c0f94 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 | 6 ++ examples/interactive_management.rs | 3 + examples/prf.rs | 3 + examples/reset.rs | 3 + examples/set_pin.rs | 3 + examples/test_exclude_list.rs | 3 + 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, 153 insertions(+), 28 deletions(-) diff --git a/examples/ctap2.rs b/examples/ctap2.rs index 78701bdc..27f1c31b 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 49660960..b246ad95 100644 --- a/examples/ctap2_discoverable_creds.rs +++ b/examples/ctap2_discoverable_creds.rs @@ -98,6 +98,9 @@ fn register_user( 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."); } @@ -355,6 +358,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/interactive_management.rs b/examples/interactive_management.rs index c0dbe4bd..8bd26673 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/prf.rs b/examples/prf.rs index 4a118529..3e7e32c9 100644 --- a/examples/prf.rs +++ b/examples/prf.rs @@ -100,6 +100,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/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 98cf8343..60c8569c 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 fa7716b9..da62c31f 100644 --- a/examples/test_exclude_list.rs +++ b/examples/test_exclude_list.rs @@ -80,6 +80,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/status_update.rs b/src/status_update.rs index 14312c8e..a1a01f60 100644 --- a/src/status_update.rs +++ b/src/status_update.rs @@ -111,6 +111,8 @@ pub enum StatusUpdate { /// After MakeCredential, supply the user with the large blob key and let /// them calculate the payload, to send back to us. LargeBlobData(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 fc8c1f1b..f5c0a7db 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; @@ -43,7 +45,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| { @@ -75,6 +77,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); @@ -97,9 +102,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 @@ -113,6 +124,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); @@ -185,6 +199,7 @@ pub mod tests { transport::FidoDevice, u2ftypes::U2FDeviceInfo, }; + use std::sync::mpsc::TryRecvError; pub(crate) fn gen_info(id: String) -> U2FDeviceInfo { U2FDeviceInfo { @@ -268,9 +283,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); @@ -279,6 +295,7 @@ pub mod tests { send_no_token(d, &selector); } }); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); send_i_am_token(&devices[2], &selector); @@ -293,18 +310,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(); } @@ -324,7 +347,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); @@ -356,6 +380,7 @@ pub mod tests { devices[5].receiver.as_ref().unwrap().recv().unwrap(), DeviceCommand::Blink ); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); } #[test] @@ -376,7 +401,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); @@ -418,6 +444,7 @@ pub mod tests { devices[6].receiver.as_ref().unwrap().recv().unwrap(), DeviceCommand::Blink ); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); } #[test] @@ -438,7 +465,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); @@ -457,11 +485,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); @@ -472,4 +505,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| {