Make the HID channel safer (fix #103) (#109)

Fixes #103.

Also protect against mutex poisoning.

Author: AlfioEmanueleFresta

libwebauthn/src/transport/ble/channel.rs

@@ -7,9 +7,7 @@ use crate::proto::ctap1::apdu::{ApduRequest, ApduResponse};
 use crate::proto::ctap2::cbor::{CborRequest, CborResponse};
 use crate::proto::CtapError;
 use crate::transport::ble::btleplug;
-use crate::transport::channel::{
-    AuthTokenData, Channel, ChannelStatus, Ctap2AuthTokenStore,
-};
+use crate::transport::channel::{AuthTokenData, Channel, ChannelStatus, Ctap2AuthTokenStore};
 use crate::transport::device::SupportedProtocols;
 use crate::transport::error::{Error, TransportError};
 use crate::UxUpdate;
@@ -129,7 +127,9 @@ impl<'a> Channel for BleChannel<'a> {
         debug!("Sending CBOR request");
         trace!(?request);
 
-        let cbor_request = request.raw_long().or(Err(TransportError::InvalidFraming))?;
+        let cbor_request = request
+            .raw_long()
+            .map_err(|e| TransportError::IoError(e.kind()))?;
         let request_frame = BleFrame::new(BleCommand::Msg, &cbor_request);
         self.connection
             .frame_send(&request_frame)

libwebauthn/src/transport/cable/tunnel.rs

@@ -453,7 +453,9 @@ async fn connection_send(
     debug!("Sending CBOR request");
     trace!(?request);
 
-    let cbor_request = request.raw_long().or(Err(TransportError::InvalidFraming))?;
+    let cbor_request = request
+        .raw_long()
+        .map_err(|e| TransportError::IoError(e.kind()))?;
     if cbor_request.len() > MAX_CBOR_SIZE {
         error!(
             cbor_request_len = cbor_request.len(),

libwebauthn/src/transport/error.rs

@@ -40,6 +40,8 @@ pub enum TransportError {
     InvalidKey,
     #[error("invalid signature")]
     InvalidSignature,
+    #[error("input/output error: {0}")]
+    IoError(std::io::ErrorKind),
 }
 
 #[derive(thiserror::Error, Debug, Copy, Clone, PartialEq)]

libwebauthn/src/transport/hid/channel.rs

@@ -246,7 +246,10 @@ impl<'d> HidChannel<'d> {
     pub async fn hid_send(&self, msg: &HidMessage) -> Result<(), Error> {
         match &self.open_device {
             OpenHidDevice::HidApiDevice(hidapi_device) => {
-                let guard = hidapi_device.lock().unwrap();
+                let Ok(guard) = hidapi_device.lock() else {
+                    warn!("Poisoned lock on HID API device");
+                    return Err(Error::Transport(TransportError::ConnectionLost));
+                };
                 Self::hid_send_hidapi(guard.deref(), msg)
             }
             #[cfg(feature = "virtual-hid-device")]
@@ -264,7 +267,9 @@ impl<'d> HidChannel<'d> {
             report.extend(vec![0; PACKET_SIZE - packet.len()]);
             debug!({ packet = i, len = report.len() }, "Sending packet as HID report",);
             trace!(?report);
-            device.write(&report).unwrap();
+            device
+                .write(&report)
+                .or(Err(Error::Transport(TransportError::ConnectionLost)))?;
         }
         Ok(())
     }
@@ -307,7 +312,10 @@ impl<'d> HidChannel<'d> {
         loop {
             let response = match &self.open_device {
                 OpenHidDevice::HidApiDevice(hidapi_device) => {
-                    let guard = hidapi_device.lock().unwrap();
+                    let Ok(guard) = hidapi_device.lock() else {
+                        warn!("Poisoned lock on HID API device");
+                        return Err(Error::Transport(TransportError::ConnectionLost));
+                    };
                     Self::hid_recv_hidapi(guard.deref(), timeout)
                 }
                 #[cfg(feature = "virtual-hid-device")]
@@ -440,7 +448,9 @@ impl Channel for HidChannel<'_> {
         let cid = self.init.cid;
         debug!({ cid }, "Sending APDU request");
         trace!(?request);
-        let apdu_raw = request.raw_long().unwrap();
+        let apdu_raw = request
+            .raw_long()
+            .map_err(|e| TransportError::IoError(e.kind()))?;
         self.hid_send(&HidMessage::new(cid, HidCommand::Msg, &apdu_raw))
             .await?;
         Ok(())