clean up Monitor code, improve debugging

Author: Crote

src/consts.rs

@@ -81,5 +81,5 @@ pub const SW_WRONG_LENGTH: [u8; 2] = [0x67, 0x00];
 pub const U2F_AID: [u8; 8] = [0xA0, 0x00, 0x00, 0x06, 0x47, 0x2F, 0x00, 0x01];
 pub const U2F_SELECT_FILE: u8 = 0xA4;
 pub const U2F_SELECT_DIRECT: u8 = 0x04;
-pub const U2F_CONTINUE: u8 = 0xC0;
+pub const U2F_GET_RESPONSE: u8 = 0xC0;
 pub const U2F_MORE_DATA: u8 = 0x61;

src/nfc/device.rs

@@ -4,7 +4,6 @@
 
 use pcsc::*;
 use std::io;
-use log;
 
 use u2ftypes::*;
 use u2fprotocol::status_word_to_result;
@@ -21,9 +20,7 @@ fn sendrecv(card: &mut Card, send: &[u8]) -> io::Result<Vec<u8>>
             Ok(rapdu.to_vec())
         },
         Err(err) => {
-            if log_enabled!(log::Level::Trace) {
-                trace!("NFC error: {}", err);
-            }
+            trace!("NFC error: {}", err);
             let s = format!("{}", err);
             Err(io_err(&s))
         }
@@ -55,7 +52,7 @@ impl APDUDevice for Card {
         }
 
         loop {
-            let out = APDU::serialize_short(U2F_CONTINUE, 0x00, &[])?;
+            let out = APDU::serialize_short(U2F_GET_RESPONSE, 0x00, &[])?;
             let ret = sendrecv(self, &out)?;
             let (mut more, [s1, s2]) = APDU::deserialize(ret)?;
             data.append(&mut more);

src/nfc/monitor.rs

@@ -7,13 +7,18 @@ use pcsc::*;
 
 use std::thread;
 use std::time::Duration;
+use std::ffi::CString;
+
+use runloop::RunLoop;
+use std::collections::HashMap;
 
 use u2ftypes::{APDUDevice};
 
 pub struct Monitor<F>
 where
     F: Fn(&mut dyn APDUDevice, &dyn Fn() -> bool) + Sync,
 {
+    runloops: HashMap<CString, RunLoop>,
     new_device_cb: Arc<F>,
 }
 
@@ -23,6 +28,7 @@ where
 {
     pub fn new(new_device_cb: F) -> Self {
         Self {
+            runloops: HashMap::new(),
             new_device_cb: Arc::new(new_device_cb),
         }
     }
@@ -31,40 +37,108 @@ where
         let ctx = Context::establish(Scope::User)?;
 
         let mut readers_buf = [0; 2048];
-        let mut reader_states: Vec<ReaderState> = vec![
-            // Listen for reader insertions/removals, if supported.
-            //ReaderState::new(PNP_NOTIFICATION(), State::UNAWARE),
-        ];
+        // We _could_ insert `ReaderState::new(PNP_NOTIFICATION(), State::UNAWARE)`
+        // to be reminded of reader insertion/removal,
+        // but this is not guaranteed to be supported
+        // and we need to poll anyways.
+        let mut reader_states: Vec<ReaderState> = Vec::new();
 
         while alive() {
-            // Remove dead readers.
-            reader_states.retain(|rs| !rs.event_state().intersects(State::UNKNOWN | State::IGNORE));
-
             // Add new readers.
             let names = ctx.list_readers(&mut readers_buf)?;
             for name in names {
-                if !reader_states.iter().any(|rs| rs.name() == name) {
+                if !reader_states.iter().any(|reader| reader.name() == name) {
+                    debug!("Adding reader {:?}", name);
                     reader_states.push(ReaderState::new(name, State::UNAWARE));
                 }
             }
 
-            // TODO:
-            // This REALLY spams the reader, perhaps check whether a card is present first??
+            // Remove dead readers.
+            fn is_dead(reader: &ReaderState) -> bool {
+                reader.event_state().intersects(State::UNKNOWN | State::IGNORE)
+            }
             for reader in &reader_states {
-                let mut card = match ctx.connect(reader.name(), ShareMode::Shared, Protocols::ANY) {
-                    Ok(card) => card,
-                    _ => continue
-                };
-
-                let f = self.new_device_cb.clone();
-                if alive() {
-                    f(&mut card, alive);
+                if is_dead(reader) {
+                    debug!("Removing reader {:?}", reader.name());
+                }
+            }
+            reader_states.retain(|reader| !is_dead(reader));
+
+            // Let backend know that we know about the reader state.
+            // Otherwise it will keep trying to update us.
+            for rs in &mut reader_states {
+                rs.sync_current_state();
+            }
+
+            if reader_states.len() == 0 {
+                // No readers available. This means that `get_status_change` will return
+                // immediately without any work, causing a busy-loop.
+                // Let's wait for a bit and look for a new reader.
+                thread::sleep(Duration::from_millis(500));
+                continue;
+            }
+
+            // This call is blocking, so we must give it _some_ timeout in order for
+            // the `alive()` check to work.
+            let timeout = Duration::from_millis(100);
+            if let Err(e) = ctx.get_status_change(timeout, &mut reader_states) {
+                if e == Error::Timeout {
+                    continue;
                 }
+                return Err(e);
             }
 
-            // TODO: sleep here to prevent infinite loop?
-            //thread::sleep(Duration::from_millis(1000));
+            for reader in &mut reader_states {
+                let state = reader.event_state();
+                let name = reader.name();
+
+                trace!("Reader {:?}: state {:?}", name, state);
+
+                if state.contains(State::PRESENT) && !state.contains(State::EXCLUSIVE) {
+                    self.add_card(&ctx, CString::from(name));
+                } else {
+                    self.remove_card(CString::from(name));
+                }
+            }
         }
+
+        self.remove_all_cards();
+
         Ok(())
     }
+
+    fn add_card(&mut self, ctx: &Context, reader: CString) {
+        let key = reader.clone();
+        let f = self.new_device_cb.clone();
+
+        let mut card = match ctx.connect(&reader, ShareMode::Shared, Protocols::ANY) {
+            Ok(card) => card,
+            _ => return
+        };
+
+        let runloop = RunLoop::new(move |alive| {
+            if alive() {
+                debug!("Connected to card on reader {:?}", reader);
+                f(&mut card, alive);
+            }
+        });
+
+        if let Ok(runloop) = runloop {
+            self.runloops.insert(key, runloop);
+        }
+    }
+
+    fn remove_card(&mut self, reader: CString) {
+        if let Some(runloop) = self.runloops.remove(&reader) {
+            runloop.cancel();
+            debug!("Disconnected from card on reader {:?}", reader);
+        }
+    }
+
+    fn remove_all_cards(&mut self) {
+        while !self.runloops.is_empty() {
+            let reader = self.runloops.keys().next().unwrap().clone();
+            self.remove_card(reader);
+        }
+    }
 }

src/u2fprotocol.rs

@@ -20,7 +20,15 @@ use util::io_err;
 pub fn u2f_init_device(dev: &mut dyn APDUDevice) -> bool
 {
     // Initialize the device and check its version.
-    dev.init_apdu().is_ok() && is_v2_device(dev).unwrap_or(false)
+    if let Err(_) = dev.init_apdu() {
+        debug!("Device could not be initialized");
+        return false;
+    }
+    if !is_v2_device(dev).unwrap_or(false) {
+        debug!("Device is not v2");
+        return false;
+    }
+    true
 }
 
 pub fn u2f_register(dev: &mut dyn APDUDevice, challenge: &[u8], application: &[u8]) -> io::Result<Vec<u8>>

src/util.rs

@@ -103,9 +103,7 @@ impl<T> Clone for OnceCallback<T> {
 }
 
 pub fn trace_hex(label: &str, data: &[u8]) {
-    if log_enabled!(log::Level::Trace) {
-        let parts: Vec<String> = data.iter().map(|byte| format!("{:02x} ", byte)).collect();
-        let s = parts.join("");
-        trace!("{} ({}): {}", label, data.len(), s);
-    }
+    let parts: Vec<String> = data.iter().map(|byte| format!("{:02x} ", byte)).collect();
+    let s = parts.join("");
+    trace!("{} ({}): {}", label, data.len(), s);
 }