Factor FidoDeviceIO trait out of FidoDevice

This allows us to provide transport-specific default implementations of
`send_msg_cancellable`. The previous approach exposed some HID-specific
abstractions through the abstract FidoDevice trait.

src/ctap2/commands/get_assertion.rs

@@ -603,7 +603,7 @@ pub mod test {
     };
     use crate::transport::device_selector::Device;
     use crate::transport::hid::HIDDevice;
-    use crate::transport::{FidoDevice, FidoProtocol};
+    use crate::transport::{FidoDevice, FidoDeviceIO, FidoProtocol};
     use crate::u2ftypes::U2FDeviceInfo;
     use rand::{thread_rng, RngCore};
 

src/ctap2/commands/reset.rs

@@ -47,7 +47,7 @@ pub mod tests {
     use super::*;
     use crate::consts::HIDCmd;
     use crate::transport::device_selector::Device;
-    use crate::transport::{hid::HIDDevice, FidoDevice, FidoProtocol};
+    use crate::transport::{hid::HIDDevice, FidoDevice, FidoDeviceIO, FidoProtocol};
     use rand::{thread_rng, RngCore};
     use serde_cbor::{de::from_slice, Value};
 

src/ctap2/commands/selection.rs

@@ -47,7 +47,7 @@ pub mod tests {
     use super::*;
     use crate::consts::HIDCmd;
     use crate::transport::device_selector::Device;
-    use crate::transport::{hid::HIDDevice, FidoDevice, FidoProtocol};
+    use crate::transport::{hid::HIDDevice, FidoDevice, FidoDeviceIO, FidoProtocol};
     use rand::{thread_rng, RngCore};
     use serde_cbor::{de::from_slice, Value};
 

src/ctap2/mod.rs

@@ -35,7 +35,7 @@ use crate::errors::{AuthenticatorError, UnsupportedOption};
 use crate::statecallback::StateCallback;
 use crate::transport::device_selector::{Device, DeviceSelectorEvent};
 
-use crate::transport::{errors::HIDError, hid::HIDDevice, FidoDevice, FidoProtocol};
+use crate::transport::{errors::HIDError, hid::HIDDevice, FidoDevice, FidoDeviceIO, FidoProtocol};
 
 use crate::{send_status, RegisterResult, SignResult, StatusPinUv, StatusUpdate};
 use std::sync::mpsc::{channel, RecvError, Sender};

src/transport/freebsd/device.rs

@@ -8,7 +8,7 @@ use crate::consts::{Capability, CID_BROADCAST, MAX_HID_RPT_SIZE};
 use crate::ctap2::commands::get_info::AuthenticatorInfo;
 use crate::transport::hid::HIDDevice;
 use crate::transport::platform::uhid;
-use crate::transport::{FidoDevice, FidoProtocol, HIDCmd, HIDError, Nonce, SharedSecret};
+use crate::transport::{FidoDevice, FidoProtocol, HIDError, Nonce, SharedSecret};
 use crate::u2ftypes::U2FDeviceInfo;
 use crate::util::from_unix_result;
 use crate::util::io_err;
@@ -188,15 +188,6 @@ impl FidoDevice for Device {
         HIDDevice::pre_init(self, noncecmd)
     }
 
-    fn sendrecv(
-        &mut self,
-        cmd: HIDCmd,
-        send: &[u8],
-        keep_alive: &dyn Fn() -> bool,
-    ) -> io::Result<(HIDCmd, Vec<u8>)> {
-        HIDDevice::sendrecv(self, cmd, send, keep_alive)
-    }
-
     fn should_try_ctap2(&self) -> bool {
         HIDDevice::get_device_info(self)
             .cap_flags

src/transport/hid.rs

@@ -1,14 +1,20 @@
 use crate::consts::{HIDCmd, CID_BROADCAST};
 
-use crate::transport::{errors::HIDError, Nonce};
-use crate::transport::{FidoDevice, FidoProtocol};
+use crate::ctap2::commands::{
+    CommandError, Request, RequestCtap1, RequestCtap2, Retryable, StatusCode,
+};
+use crate::transport::errors::{ApduErrorStatus, HIDError};
+use crate::transport::{FidoDevice, FidoDeviceIO, FidoProtocol, Nonce};
 use crate::u2ftypes::{U2FDeviceInfo, U2FHIDCont, U2FHIDInit, U2FHIDInitResp};
+use crate::util::io_err;
 use rand::{thread_rng, RngCore};
 use std::cmp::Eq;
 use std::fmt;
 use std::hash::Hash;
 use std::io;
 use std::io::{Read, Write};
+use std::thread;
+use std::time::Duration;
 
 pub trait HIDDevice: FidoDevice + Read + Write {
     type BuildParameters: Sized;
@@ -157,3 +163,83 @@ pub trait HIDDevice: FidoDevice + Read + Write {
         Ok((cmd, data))
     }
 }
+
+impl<T: HIDDevice> FidoDeviceIO for T {
+    fn send_msg_cancellable<Out, Req: Request<Out>>(
+        &mut self,
+        msg: &Req,
+        keep_alive: &dyn Fn() -> bool,
+    ) -> Result<Out, HIDError> {
+        if !self.initialized() {
+            return Err(HIDError::DeviceNotInitialized);
+        }
+
+        match self.get_protocol() {
+            FidoProtocol::CTAP1 => self.send_ctap1_cancellable(msg, keep_alive),
+            FidoProtocol::CTAP2 => self.send_cbor_cancellable(msg, keep_alive),
+        }
+    }
+
+    fn send_cbor_cancellable<Req: RequestCtap2>(
+        &mut self,
+        msg: &Req,
+        keep_alive: &dyn Fn() -> bool,
+    ) -> Result<Req::Output, HIDError> {
+        debug!("sending {:?} to {:?}", msg, self);
+
+        let mut data = msg.wire_format()?;
+        let mut buf: Vec<u8> = Vec::with_capacity(data.len() + 1);
+        // CTAP2 command
+        buf.push(Req::command() as u8);
+        // payload
+        buf.append(&mut data);
+        let buf = buf;
+
+        let (cmd, resp) = self.sendrecv(HIDCmd::Cbor, &buf, keep_alive)?;
+        if cmd == HIDCmd::Cbor {
+            Ok(msg.handle_response_ctap2(self, &resp)?)
+        } else {
+            Err(HIDError::UnexpectedCmd(cmd.into()))
+        }
+    }
+
+    fn send_ctap1_cancellable<Req: RequestCtap1>(
+        &mut self,
+        msg: &Req,
+        keep_alive: &dyn Fn() -> bool,
+    ) -> Result<Req::Output, HIDError> {
+        debug!("sending {:?} to {:?}", msg, self);
+        let (data, add_info) = msg.ctap1_format()?;
+
+        while keep_alive() {
+            // sendrecv will not block with a CTAP1 device
+            let (cmd, mut data) = self.sendrecv(HIDCmd::Msg, &data, &|| true)?;
+            if cmd == HIDCmd::Msg {
+                if data.len() < 2 {
+                    return Err(io_err("Unexpected Response: shorter than expected").into());
+                }
+                let split_at = data.len() - 2;
+                let status = data.split_off(split_at);
+                // This will bubble up error if status != no error
+                let status = ApduErrorStatus::from([status[0], status[1]]);
+
+                match msg.handle_response_ctap1(status, &data, &add_info) {
+                    Ok(out) => return Ok(out),
+                    Err(Retryable::Retry) => {
+                        // sleep 100ms then loop again
+                        // TODO(baloo): meh, use tokio instead?
+                        thread::sleep(Duration::from_millis(100));
+                    }
+                    Err(Retryable::Error(e)) => return Err(e),
+                }
+            } else {
+                return Err(HIDError::UnexpectedCmd(cmd.into()));
+            }
+        }
+
+        Err(HIDError::Command(CommandError::StatusCode(
+            StatusCode::KeepaliveCancel,
+            None,
+        )))
+    }
+}

src/transport/linux/device.rs

@@ -7,7 +7,7 @@ use crate::consts::{Capability, CID_BROADCAST};
 use crate::ctap2::commands::get_info::AuthenticatorInfo;
 use crate::transport::hid::HIDDevice;
 use crate::transport::platform::{hidraw, monitor};
-use crate::transport::{FidoDevice, FidoProtocol, HIDCmd, HIDError, Nonce, SharedSecret};
+use crate::transport::{FidoDevice, FidoProtocol, HIDError, Nonce, SharedSecret};
 use crate::u2ftypes::U2FDeviceInfo;
 use crate::util::from_unix_result;
 use std::fs::OpenOptions;
@@ -140,15 +140,6 @@ impl FidoDevice for Device {
         HIDDevice::pre_init(self, noncecmd)
     }
 
-    fn sendrecv(
-        &mut self,
-        cmd: HIDCmd,
-        send: &[u8],
-        keep_alive: &dyn Fn() -> bool,
-    ) -> io::Result<(HIDCmd, Vec<u8>)> {
-        HIDDevice::sendrecv(self, cmd, send, keep_alive)
-    }
-
     fn should_try_ctap2(&self) -> bool {
         HIDDevice::get_device_info(self)
             .cap_flags

src/transport/macos/device.rs

@@ -8,7 +8,7 @@ use crate::consts::{Capability, CID_BROADCAST, MAX_HID_RPT_SIZE};
 use crate::ctap2::commands::get_info::AuthenticatorInfo;
 use crate::transport::hid::HIDDevice;
 use crate::transport::platform::iokit::*;
-use crate::transport::{FidoDevice, FidoProtocol, HIDCmd, HIDError, Nonce, SharedSecret};
+use crate::transport::{FidoDevice, FidoProtocol, HIDError, Nonce, SharedSecret};
 use crate::u2ftypes::U2FDeviceInfo;
 use core_foundation::base::*;
 use core_foundation::string::*;
@@ -188,15 +188,6 @@ impl FidoDevice for Device {
         HIDDevice::pre_init(self, noncecmd)
     }
 
-    fn sendrecv(
-        &mut self,
-        cmd: HIDCmd,
-        send: &[u8],
-        keep_alive: &dyn Fn() -> bool,
-    ) -> io::Result<(HIDCmd, Vec<u8>)> {
-        HIDDevice::sendrecv(self, cmd, send, keep_alive)
-    }
-
     fn should_try_ctap2(&self) -> bool {
         HIDDevice::get_device_info(self)
             .cap_flags

src/transport/mock/device.rs

@@ -1,7 +1,7 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-use crate::consts::{Capability, HIDCmd, CID_BROADCAST};
+use crate::consts::{Capability, CID_BROADCAST};
 use crate::crypto::SharedSecret;
 use crate::ctap2::commands::get_info::AuthenticatorInfo;
 use crate::transport::device_selector::DeviceCommand;
@@ -161,15 +161,6 @@ impl FidoDevice for Device {
         HIDDevice::pre_init(self, noncecmd)
     }
 
-    fn sendrecv(
-        &mut self,
-        cmd: HIDCmd,
-        send: &[u8],
-        keep_alive: &dyn Fn() -> bool,
-    ) -> io::Result<(HIDCmd, Vec<u8>)> {
-        HIDDevice::sendrecv(self, cmd, send, keep_alive)
-    }
-
     fn should_try_ctap2(&self) -> bool {
         HIDDevice::get_device_info(self)
             .cap_flags

src/transport/mod.rs

@@ -1,4 +1,3 @@
-use crate::consts::HIDCmd;
 use crate::crypto::{PinUvAuthProtocol, PinUvAuthToken, SharedSecret};
 use crate::ctap2::commands::client_pin::{
     GetKeyAgreement, GetPinToken, GetPinUvAuthTokenUsingPinWithPermissions,
@@ -8,20 +7,14 @@ use crate::ctap2::commands::get_info::{AuthenticatorInfo, AuthenticatorVersion,
 use crate::ctap2::commands::get_version::GetVersion;
 use crate::ctap2::commands::make_credentials::dummy_make_credentials_cmd;
 use crate::ctap2::commands::selection::Selection;
-use crate::ctap2::commands::{
-    CommandError, Request, RequestCtap1, RequestCtap2, Retryable, StatusCode,
-};
+use crate::ctap2::commands::{CommandError, Request, RequestCtap1, RequestCtap2, StatusCode};
 use crate::transport::device_selector::BlinkResult;
-use crate::transport::errors::{ApduErrorStatus, HIDError};
-use crate::util::io_err;
+use crate::transport::errors::HIDError;
+
 use crate::Pin;
 use std::convert::TryFrom;
 use std::fmt;
 
-use std::io;
-use std::thread;
-use std::time::Duration;
-
 pub mod device_selector;
 pub mod errors;
 pub mod hid;
@@ -84,7 +77,39 @@ pub enum FidoProtocol {
     CTAP2,
 }
 
-pub trait FidoDevice
+pub trait FidoDeviceIO {
+    fn send_msg<Out, Req: Request<Out>>(&mut self, msg: &Req) -> Result<Out, HIDError> {
+        self.send_msg_cancellable(msg, &|| true)
+    }
+
+    fn send_cbor<Req: RequestCtap2>(&mut self, msg: &Req) -> Result<Req::Output, HIDError> {
+        self.send_cbor_cancellable(msg, &|| true)
+    }
+
+    fn send_ctap1<Req: RequestCtap1>(&mut self, msg: &Req) -> Result<Req::Output, HIDError> {
+        self.send_ctap1_cancellable(msg, &|| true)
+    }
+
+    fn send_msg_cancellable<Out, Req: Request<Out>>(
+        &mut self,
+        msg: &Req,
+        keep_alive: &dyn Fn() -> bool,
+    ) -> Result<Out, HIDError>;
+
+    fn send_cbor_cancellable<Req: RequestCtap2>(
+        &mut self,
+        msg: &Req,
+        keep_alive: &dyn Fn() -> bool,
+    ) -> Result<Req::Output, HIDError>;
+
+    fn send_ctap1_cancellable<Req: RequestCtap1>(
+        &mut self,
+        msg: &Req,
+        keep_alive: &dyn Fn() -> bool,
+    ) -> Result<Req::Output, HIDError>;
+}
+
+pub trait FidoDevice: FidoDeviceIO
 where
     Self: Sized,
     Self: fmt::Debug,
@@ -92,13 +117,6 @@ where
     fn pre_init(&mut self, noncecmd: Nonce) -> Result<(), HIDError>;
     fn initialized(&self) -> bool;
 
-    fn sendrecv(
-        &mut self,
-        cmd: HIDCmd,
-        send: &[u8],
-        keep_alive: &dyn Fn() -> bool,
-    ) -> io::Result<(HIDCmd, Vec<u8>)>;
-
     // Check if the device is actually a token
     fn is_u2f(&mut self) -> bool;
     fn should_try_ctap2(&self) -> bool;
@@ -118,97 +136,6 @@ where
     fn get_shared_secret(&self) -> Option<&SharedSecret>;
     fn set_shared_secret(&mut self, secret: SharedSecret);
 
-    fn send_msg<Out, Req: Request<Out>>(&mut self, msg: &Req) -> Result<Out, HIDError> {
-        self.send_msg_cancellable(msg, &|| true)
-    }
-
-    fn send_cbor<Req: RequestCtap2>(&mut self, msg: &Req) -> Result<Req::Output, HIDError> {
-        self.send_cbor_cancellable(msg, &|| true)
-    }
-
-    fn send_ctap1<Req: RequestCtap1>(&mut self, msg: &Req) -> Result<Req::Output, HIDError> {
-        self.send_ctap1_cancellable(msg, &|| true)
-    }
-
-    fn send_msg_cancellable<Out, Req: Request<Out>>(
-        &mut self,
-        msg: &Req,
-        keep_alive: &dyn Fn() -> bool,
-    ) -> Result<Out, HIDError> {
-        if !self.initialized() {
-            return Err(HIDError::DeviceNotInitialized);
-        }
-
-        if self.get_protocol() == FidoProtocol::CTAP2 {
-            self.send_cbor_cancellable(msg, keep_alive)
-        } else {
-            self.send_ctap1_cancellable(msg, keep_alive)
-        }
-    }
-
-    fn send_cbor_cancellable<Req: RequestCtap2>(
-        &mut self,
-        msg: &Req,
-        keep_alive: &dyn Fn() -> bool,
-    ) -> Result<Req::Output, HIDError> {
-        debug!("sending {:?} to {:?}", msg, self);
-
-        let mut data = msg.wire_format()?;
-        let mut buf: Vec<u8> = Vec::with_capacity(data.len() + 1);
-        // CTAP2 command
-        buf.push(Req::command() as u8);
-        // payload
-        buf.append(&mut data);
-        let buf = buf;
-
-        let (cmd, resp) = self.sendrecv(HIDCmd::Cbor, &buf, keep_alive)?;
-        if cmd == HIDCmd::Cbor {
-            Ok(msg.handle_response_ctap2(self, &resp)?)
-        } else {
-            Err(HIDError::UnexpectedCmd(cmd.into()))
-        }
-    }
-
-    fn send_ctap1_cancellable<Req: RequestCtap1>(
-        &mut self,
-        msg: &Req,
-        keep_alive: &dyn Fn() -> bool,
-    ) -> Result<Req::Output, HIDError> {
-        debug!("sending {:?} to {:?}", msg, self);
-        let (data, add_info) = msg.ctap1_format()?;
-
-        while keep_alive() {
-            // sendrecv will not block with a CTAP1 device
-            let (cmd, mut data) = self.sendrecv(HIDCmd::Msg, &data, &|| true)?;
-            if cmd == HIDCmd::Msg {
-                if data.len() < 2 {
-                    return Err(io_err("Unexpected Response: shorter than expected").into());
-                }
-                let split_at = data.len() - 2;
-                let status = data.split_off(split_at);
-                // This will bubble up error if status != no error
-                let status = ApduErrorStatus::from([status[0], status[1]]);
-
-                match msg.handle_response_ctap1(status, &data, &add_info) {
-                    Ok(out) => return Ok(out),
-                    Err(Retryable::Retry) => {
-                        // sleep 100ms then loop again
-                        // TODO(baloo): meh, use tokio instead?
-                        thread::sleep(Duration::from_millis(100));
-                    }
-                    Err(Retryable::Error(e)) => return Err(e),
-                }
-            } else {
-                return Err(HIDError::UnexpectedCmd(cmd.into()));
-            }
-        }
-
-        Err(HIDError::Command(CommandError::StatusCode(
-            StatusCode::KeepaliveCancel,
-            None,
-        )))
-    }
-
     fn init(&mut self, nonce: Nonce) -> Result<(), HIDError> {
         self.pre_init(nonce)?;
 

src/transport/netbsd/device.rs

@@ -9,7 +9,7 @@ use crate::transport::hid::HIDDevice;
 use crate::transport::platform::fd::Fd;
 use crate::transport::platform::monitor::WrappedOpenDevice;
 use crate::transport::platform::uhid;
-use crate::transport::{FidoDevice, FidoProtocol, HIDCmd, HIDError, Nonce, SharedSecret};
+use crate::transport::{FidoDevice, FidoProtocol, HIDError, Nonce, SharedSecret};
 use crate::u2ftypes::U2FDeviceInfo;
 use crate::util::io_err;
 use std::ffi::OsString;
@@ -190,15 +190,6 @@ impl FidoDevice for Device {
         HIDDevice::pre_init(self, noncecmd)
     }
 
-    fn sendrecv(
-        &mut self,
-        cmd: HIDCmd,
-        send: &[u8],
-        keep_alive: &dyn Fn() -> bool,
-    ) -> io::Result<(HIDCmd, Vec<u8>)> {
-        HIDDevice::sendrecv(self, cmd, send, keep_alive)
-    }
-
     fn should_try_ctap2(&self) -> bool {
         HIDDevice::get_device_info(self)
             .cap_flags

src/transport/openbsd/device.rs

@@ -7,7 +7,7 @@ use crate::consts::{Capability, CID_BROADCAST, MAX_HID_RPT_SIZE};
 use crate::ctap2::commands::get_info::AuthenticatorInfo;
 use crate::transport::hid::HIDDevice;
 use crate::transport::platform::monitor::WrappedOpenDevice;
-use crate::transport::{FidoDevice, FidoProtocol, HIDCmd, HIDError, Nonce, SharedSecret};
+use crate::transport::{FidoDevice, FidoProtocol, HIDError, Nonce, SharedSecret};
 use crate::u2ftypes::U2FDeviceInfo;
 use crate::util::{from_unix_result, io_err};
 use std::ffi::{CString, OsString};
@@ -171,15 +171,6 @@ impl FidoDevice for Device {
         HIDDevice::pre_init(self, noncecmd)
     }
 
-    fn sendrecv(
-        &mut self,
-        cmd: HIDCmd,
-        send: &[u8],
-        keep_alive: &dyn Fn() -> bool,
-    ) -> io::Result<(HIDCmd, Vec<u8>)> {
-        HIDDevice::sendrecv(self, cmd, send, keep_alive)
-    }
-
     fn should_try_ctap2(&self) -> bool {
         HIDDevice::get_device_info(self)
             .cap_flags

src/transport/stub/device.rs

@@ -4,7 +4,7 @@
 
 use crate::ctap2::commands::get_info::AuthenticatorInfo;
 use crate::transport::hid::HIDDevice;
-use crate::transport::{FidoDevice, FidoProtocol, HIDCmd};
+use crate::transport::{FidoDevice, FidoProtocol};
 use crate::transport::{HIDError, Nonce, SharedSecret};
 use crate::u2ftypes::U2FDeviceInfo;
 use std::hash::Hash;
@@ -77,15 +77,6 @@ impl FidoDevice for Device {
         unimplemented!();
     }
 
-    fn sendrecv(
-        &mut self,
-        cmd: HIDCmd,
-        send: &[u8],
-        keep_alive: &dyn Fn() -> bool,
-    ) -> io::Result<(HIDCmd, Vec<u8>)> {
-        unimplemented!();
-    }
-
     fn should_try_ctap2(&self) -> bool {
         unimplemented!();
     }

src/transport/windows/device.rs

@@ -8,7 +8,7 @@ use crate::consts::{
 };
 use crate::ctap2::commands::get_info::AuthenticatorInfo;
 use crate::transport::hid::HIDDevice;
-use crate::transport::{FidoDevice, FidoProtocol, HIDCmd, HIDError, Nonce, SharedSecret};
+use crate::transport::{FidoDevice, FidoProtocol, HIDError, Nonce, SharedSecret};
 use crate::u2ftypes::U2FDeviceInfo;
 use std::fs::{File, OpenOptions};
 use std::hash::{Hash, Hasher};
@@ -129,15 +129,6 @@ impl FidoDevice for Device {
         HIDDevice::pre_init(self, noncecmd)
     }
 
-    fn sendrecv(
-        &mut self,
-        cmd: HIDCmd,
-        send: &[u8],
-        keep_alive: &dyn Fn() -> bool,
-    ) -> io::Result<(HIDCmd, Vec<u8>)> {
-        HIDDevice::sendrecv(self, cmd, send, keep_alive)
-    }
-
     fn should_try_ctap2(&self) -> bool {
         HIDDevice::get_device_info(self)
             .cap_flags