From 145cbf26b41859a9ad12f80eb736748438ca761a Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 25 May 2025 17:38:09 +0200 Subject: [PATCH 1/2] Make the HID channel safer --- libwebauthn/src/transport/hid/channel.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/libwebauthn/src/transport/hid/channel.rs b/libwebauthn/src/transport/hid/channel.rs index fb16ac97..4a30af2d 100644 --- a/libwebauthn/src/transport/hid/channel.rs +++ b/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,10 @@ impl Channel for HidChannel<'_> { let cid = self.init.cid; debug!({ cid }, "Sending APDU request"); trace!(?request); - let apdu_raw = request.raw_long().unwrap(); + let Ok(apdu_raw) = request.raw_long() else { + warn!("Failed to serialize APDU request"); + return Err(Error::Transport(TransportError::InvalidFraming)); + }; self.hid_send(&HidMessage::new(cid, HidCommand::Msg, &apdu_raw)) .await?; Ok(()) From 66f607ee28aafe04a8f32c065e563f5fb63a55aa Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Mon, 26 May 2025 16:30:56 +0200 Subject: [PATCH 2/2] Add IoError to TransportError --- libwebauthn/src/transport/ble/channel.rs | 8 ++++---- libwebauthn/src/transport/cable/tunnel.rs | 4 +++- libwebauthn/src/transport/error.rs | 2 ++ libwebauthn/src/transport/hid/channel.rs | 7 +++---- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/libwebauthn/src/transport/ble/channel.rs b/libwebauthn/src/transport/ble/channel.rs index 9d3ab9dc..75bf58dd 100644 --- a/libwebauthn/src/transport/ble/channel.rs +++ b/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) diff --git a/libwebauthn/src/transport/cable/tunnel.rs b/libwebauthn/src/transport/cable/tunnel.rs index eff9c99f..a6481f16 100644 --- a/libwebauthn/src/transport/cable/tunnel.rs +++ b/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(), diff --git a/libwebauthn/src/transport/error.rs b/libwebauthn/src/transport/error.rs index 421f228a..aa19941b 100644 --- a/libwebauthn/src/transport/error.rs +++ b/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)] diff --git a/libwebauthn/src/transport/hid/channel.rs b/libwebauthn/src/transport/hid/channel.rs index 4a30af2d..38010bd6 100644 --- a/libwebauthn/src/transport/hid/channel.rs +++ b/libwebauthn/src/transport/hid/channel.rs @@ -448,10 +448,9 @@ impl Channel for HidChannel<'_> { let cid = self.init.cid; debug!({ cid }, "Sending APDU request"); trace!(?request); - let Ok(apdu_raw) = request.raw_long() else { - warn!("Failed to serialize APDU request"); - return Err(Error::Transport(TransportError::InvalidFraming)); - }; + 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(())