From 345fb574b81931fb2d31008f43d2b40eddd2e702 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 10 May 2026 21:00:06 +0100 Subject: [PATCH 1/3] fix(ble): use WriteType::WithResponse for spec-declared Write characteristics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per CTAP 2.2 §11.4, both fidoControlPoint and fidoServiceRevisionBitfield expose the standard Write property (GATT Write Request, with response). The previous code always issued WriteType::WithoutResponse, which is spec-incorrect and may silently drop bytes on conforming authenticators. Introduces a write_type_for() helper that inspects the characteristic's declared GATT properties and picks WithResponse when WRITE is set, falling back to WithoutResponse only when the authenticator explicitly advertises just that property. Unit tests cover the property-detection logic. --- .../src/transport/ble/btleplug/connection.rs | 18 ++--- .../src/transport/ble/btleplug/gatt.rs | 66 ++++++++++++++++++- 2 files changed, 72 insertions(+), 12 deletions(-) diff --git a/libwebauthn/src/transport/ble/btleplug/connection.rs b/libwebauthn/src/transport/ble/btleplug/connection.rs index 8941689a..51315068 100644 --- a/libwebauthn/src/transport/ble/btleplug/connection.rs +++ b/libwebauthn/src/transport/ble/btleplug/connection.rs @@ -1,11 +1,12 @@ use std::io::Cursor as IOCursor; -use btleplug::api::{Peripheral as _, WriteType}; +use btleplug::api::Peripheral as _; use btleplug::platform::Peripheral; use byteorder::{BigEndian, ReadBytesExt}; use tracing::{debug, info, instrument, trace, warn}; use super::device::FidoEndpoints; +use super::gatt::write_type_for; use super::Error; use crate::fido::FidoRevision; use crate::transport::ble::framing::{ @@ -60,16 +61,14 @@ impl Connection { .fragments(max_fragment_size) .or(Err(Error::InvalidFraming))?; + let write_type = write_type_for(&self.services.control_point); + for (i, fragment) in fragments.iter().enumerate() { debug!({ fragment = i, len = fragment.len() }, "Sending fragment"); trace!(?fragment); self.peripheral - .write( - &self.services.control_point, - fragment, - WriteType::WithoutResponse, - ) + .write(&self.services.control_point, fragment, write_type) .await .or(Err(Error::OperationFailed))?; } @@ -79,12 +78,9 @@ impl Connection { pub(crate) async fn select_fido_revision(&self, revision: &FidoRevision) -> Result<(), Error> { let ack: u8 = *revision as u8; + let write_type = write_type_for(&self.services.service_revision_bitfield); self.peripheral - .write( - &self.services.service_revision_bitfield, - &[ack], - WriteType::WithoutResponse, - ) + .write(&self.services.service_revision_bitfield, &[ack], write_type) .await .or(Err(Error::OperationFailed))?; diff --git a/libwebauthn/src/transport/ble/btleplug/gatt.rs b/libwebauthn/src/transport/ble/btleplug/gatt.rs index 8ed92f3e..a169cee4 100644 --- a/libwebauthn/src/transport/ble/btleplug/gatt.rs +++ b/libwebauthn/src/transport/ble/btleplug/gatt.rs @@ -1,4 +1,4 @@ -use btleplug::api::{Characteristic, Peripheral as _}; +use btleplug::api::{CharPropFlags, Characteristic, Peripheral as _, WriteType}; use btleplug::platform::Peripheral; use uuid::Uuid; @@ -15,3 +15,67 @@ pub fn get_gatt_characteristic( .map(ToOwned::to_owned) .ok_or(Error::ConnectionFailed) } + +/// Picks a `WriteType` from a characteristic's advertised GATT properties. +/// +/// `fidoControlPoint` and `fidoServiceRevisionBitfield` are Write +/// characteristics per CTAP 2.2 §11.4; only downgrade to WithoutResponse +/// when that is the sole property advertised. +pub fn write_type_for(characteristic: &Characteristic) -> WriteType { + if characteristic.properties.contains(CharPropFlags::WRITE) { + WriteType::WithResponse + } else if characteristic + .properties + .contains(CharPropFlags::WRITE_WITHOUT_RESPONSE) + { + WriteType::WithoutResponse + } else { + WriteType::WithResponse + } +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeSet; + + use super::*; + + fn make_characteristic(properties: CharPropFlags) -> Characteristic { + Characteristic { + uuid: Uuid::nil(), + service_uuid: Uuid::nil(), + properties, + descriptors: BTreeSet::new(), + } + } + + #[test] + fn write_type_prefers_with_response_when_write_property_set() { + let c = make_characteristic(CharPropFlags::WRITE); + assert_eq!(write_type_for(&c), WriteType::WithResponse); + } + + #[test] + fn write_type_uses_without_response_when_only_write_without_response() { + let c = make_characteristic(CharPropFlags::WRITE_WITHOUT_RESPONSE); + assert_eq!(write_type_for(&c), WriteType::WithoutResponse); + } + + #[test] + fn write_type_prefers_with_response_when_both_properties_set() { + let c = make_characteristic(CharPropFlags::WRITE | CharPropFlags::WRITE_WITHOUT_RESPONSE); + assert_eq!(write_type_for(&c), WriteType::WithResponse); + } + + #[test] + fn write_type_defaults_to_with_response_when_no_property_set() { + let c = make_characteristic(CharPropFlags::empty()); + assert_eq!(write_type_for(&c), WriteType::WithResponse); + } + + #[test] + fn write_type_ignores_unrelated_properties() { + let c = make_characteristic(CharPropFlags::READ | CharPropFlags::NOTIFY); + assert_eq!(write_type_for(&c), WriteType::WithResponse); + } +} From 063336451c2741e5aadb96e524a1053764fa82a0 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 10 May 2026 21:02:07 +0100 Subject: [PATCH 2/3] fix(ble): consume fidoStatus notifications instead of GATT Read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per CTAP 2.2 §11.4 the fidoStatus characteristic is Notify-only. The previous receive path called peripheral.read() against it, which bluez rejects with NotPermitted and makes FIDO2-over-BLE non-functional on Linux. Other backends may tolerate the Read by returning stale cached data, which is not real notification-driven framing either way. Connection::new now subscribes to fidoStatus, obtains the peripheral's notification stream, filters it to the fidoStatus UUID, and stores it on the connection. frame_recv awaits the next notification with the caller-supplied operation timeout; on expiry it sends a BleCommand::Cancel on fidoControlPoint (best-effort, WithoutResponse) and returns Timeout. The channel's apdu_recv and cbor_recv now forward their timeout to frame_recv and map Error::Timeout to TransportError::Timeout. --- .../src/transport/ble/btleplug/connection.rs | 94 ++++++++++++++++--- libwebauthn/src/transport/ble/channel.rs | 19 ++-- 2 files changed, 96 insertions(+), 17 deletions(-) diff --git a/libwebauthn/src/transport/ble/btleplug/connection.rs b/libwebauthn/src/transport/ble/btleplug/connection.rs index 51315068..f8d6b3bc 100644 --- a/libwebauthn/src/transport/ble/btleplug/connection.rs +++ b/libwebauthn/src/transport/ble/btleplug/connection.rs @@ -1,8 +1,14 @@ use std::io::Cursor as IOCursor; +use std::pin::Pin; +use std::sync::Arc; +use std::time::Duration; -use btleplug::api::Peripheral as _; +use btleplug::api::{Peripheral as _, ValueNotification, WriteType}; use btleplug::platform::Peripheral; use byteorder::{BigEndian, ReadBytesExt}; +use futures::stream::{Stream, StreamExt}; +use tokio::sync::Mutex; +use tokio::time::timeout; use tracing::{debug, info, instrument, trace, warn}; use super::device::FidoEndpoints; @@ -13,10 +19,24 @@ use crate::transport::ble::framing::{ BleCommand, BleFrame as Frame, BleFrameParser, BleFrameParserResult, }; -#[derive(Debug, Clone)] +type NotificationStream = Pin + Send>>; + +#[derive(Clone)] pub struct Connection { pub peripheral: Peripheral, pub services: FidoEndpoints, + /// `fidoStatus` is Notify-only (CTAP 2.2 §11.4); we consume notifications + /// rather than issue GATT Read. + notifications: Arc>, +} + +impl std::fmt::Debug for Connection { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Connection") + .field("peripheral", &self.peripheral) + .field("services", &self.services) + .finish_non_exhaustive() + } } impl Connection { @@ -25,9 +45,26 @@ impl Connection { services: &FidoEndpoints, revision: &FidoRevision, ) -> Result { + // Subscribe before opening the stream so early frames aren't dropped. + peripheral + .subscribe(&services.status) + .await + .or(Err(Error::OperationFailed))?; + + let status_uuid = services.status.uuid; + let raw_stream = peripheral + .notifications() + .await + .or(Err(Error::OperationFailed))?; + let notifications: NotificationStream = Box::pin(raw_stream.filter(move |n| { + let matches = n.uuid == status_uuid; + async move { matches } + })); + let connection = Self { peripheral: peripheral.to_owned(), services: services.clone(), + notifications: Arc::new(Mutex::new(notifications)), }; connection.select_fido_revision(revision).await?; Ok(connection) @@ -88,12 +125,53 @@ impl Connection { Ok(()) } + /// Sends a best-effort Cancel on `fidoControlPoint` using + /// `WriteType::WithoutResponse` so cancellation never blocks. + async fn send_cancel(&self) -> Result<(), Error> { + let cancel_frame = Frame::new(BleCommand::Cancel, &[]); + let max_fragment_size = self.control_point_length().await.unwrap_or(20); + let fragments = cancel_frame + .fragments(max_fragment_size) + .or(Err(Error::InvalidFraming))?; + for fragment in fragments { + self.peripheral + .write( + &self.services.control_point, + &fragment, + WriteType::WithoutResponse, + ) + .await + .or(Err(Error::OperationFailed))?; + } + Ok(()) + } + #[instrument(skip_all)] - pub async fn frame_recv(&self) -> Result { + pub async fn frame_recv(&self, op_timeout: Duration) -> Result { let mut parser = BleFrameParser::new(); + let mut stream = self.notifications.lock().await; loop { - let fragment = self.receive_fragment().await?; + let fragment = match timeout(op_timeout, stream.next()).await { + Ok(Some(notification)) => notification.value, + Ok(None) => { + warn!("Notification stream ended unexpectedly"); + return Err(Error::ConnectionFailed); + } + Err(_) => { + warn!( + ?op_timeout, + "Timed out waiting for fidoStatus notification; sending Cancel" + ); + // Drop the lock so a late notification doesn't deadlock the cancel. + drop(stream); + if let Err(e) = self.send_cancel().await { + warn!(?e, "Failed to send Cancel after timeout"); + } + return Err(Error::Timeout); + } + }; + debug!("Received fragment"); trace!(?fragment); @@ -129,13 +207,7 @@ impl Connection { } } - async fn receive_fragment(&self) -> Result, Error> { - self.peripheral - .read(&self.services.status) - .await - .or(Err(Error::OperationFailed)) - } - + /// Enables notifications on `fidoStatus`. Idempotent. pub async fn subscribe(&self) -> Result<(), Error> { self.peripheral .subscribe(&self.services.status) diff --git a/libwebauthn/src/transport/ble/channel.rs b/libwebauthn/src/transport/ble/channel.rs index 8928b077..8243d1be 100644 --- a/libwebauthn/src/transport/ble/channel.rs +++ b/libwebauthn/src/transport/ble/channel.rs @@ -100,12 +100,16 @@ impl<'a> Channel for BleChannel<'a> { } #[instrument(level = Level::DEBUG, skip_all)] - async fn apdu_recv(&mut self, _timeout: Duration) -> Result { + async fn apdu_recv(&mut self, timeout: Duration) -> Result { let response_frame = self .connection - .frame_recv() + .frame_recv(timeout) .await - .or(Err(Error::Transport(TransportError::ConnectionFailed)))?; + .map_err(|e| match e { + btleplug::Error::Timeout => Error::Transport(TransportError::Timeout), + _ => Error::Transport(TransportError::ConnectionFailed), + })?; + match response_frame.cmd { BleCommand::Error => return Err(Error::Transport(TransportError::InvalidFraming)), // Encapsulation layer error BleCommand::Cancel => return Err(Error::Ctap(CtapError::KeepAliveCancel)), @@ -143,12 +147,15 @@ impl<'a> Channel for BleChannel<'a> { } #[instrument(level = Level::DEBUG, skip_all)] - async fn cbor_recv(&mut self, _timeout: std::time::Duration) -> Result { + async fn cbor_recv(&mut self, timeout: std::time::Duration) -> Result { let response_frame = self .connection - .frame_recv() + .frame_recv(timeout) .await - .or(Err(Error::Transport(TransportError::ConnectionFailed)))?; + .map_err(|e| match e { + btleplug::Error::Timeout => Error::Transport(TransportError::Timeout), + _ => Error::Transport(TransportError::ConnectionFailed), + })?; match response_frame.cmd { BleCommand::Error => return Err(Error::Transport(TransportError::InvalidFraming)), // Encapsulation layer error BleCommand::Cancel => return Err(Error::Ctap(CtapError::KeepAliveCancel)), From d8142f11a7c6b458581b1abf1457ea9561266dff Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 10 May 2026 21:05:17 +0100 Subject: [PATCH 3/3] fix(ble): refuse to operate on unbonded LE links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per CTAP 2.2 §11.4, BLE FIDO authenticator traffic MUST run on a bonded LE Secure Connections link. The previous connect path issued FIDO operations without verifying bonding state, so a session that fell back to an unauthenticated link would proceed in violation of the spec. Adds a pairing module that, on Linux, queries org.bluez.Device1.Paired and org.bluez.Device1.Bonded over DBus and refuses to proceed if either is false. The DBus call is dispatched on spawn_blocking so it doesn't stall the tokio runtime. On non-bluez backends or when DBus is unreachable the check falls back gracefully: macOS and Windows enforce bonding at the OS level for authenticated GATT characteristics, so deferring there is acceptable. The library itself cannot trigger pairing; the user is expected to pair the device beforehand (e.g. via bluetoothctl). manager::connect calls enforce_bonded after establishing the link and before discovering services. --- .../src/transport/ble/btleplug/manager.rs | 6 +- libwebauthn/src/transport/ble/btleplug/mod.rs | 1 + .../src/transport/ble/btleplug/pairing.rs | 111 ++++++++++++++++++ 3 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 libwebauthn/src/transport/ble/btleplug/pairing.rs diff --git a/libwebauthn/src/transport/ble/btleplug/manager.rs b/libwebauthn/src/transport/ble/btleplug/manager.rs index 832c7c6b..bfba8bd3 100644 --- a/libwebauthn/src/transport/ble/btleplug/manager.rs +++ b/libwebauthn/src/transport/ble/btleplug/manager.rs @@ -11,6 +11,7 @@ use uuid::Uuid; use super::device::FidoEndpoints; use super::gatt::get_gatt_characteristic; +use super::pairing::enforce_bonded; use super::{Connection, Error, FidoDevice}; use crate::fido::{FidoProtocol, FidoRevision}; @@ -207,8 +208,8 @@ pub async fn supported_fido_revisions( Ok(supported) } -/// Connect, discover FIDO services on this device, and -/// select the FIDO revision to be used. +/// Connect, discover FIDO services on this device, and select the FIDO +/// revision to be used. Refuses unbonded LE links (CTAP 2.2 §11.4). pub async fn connect( peripheral: &Peripheral, revision: &FidoRevision, @@ -217,6 +218,7 @@ pub async fn connect( .connect() .await .or(Err(Error::ConnectionFailed))?; + enforce_bonded(peripheral).await?; peripheral .discover_services() .await diff --git a/libwebauthn/src/transport/ble/btleplug/mod.rs b/libwebauthn/src/transport/ble/btleplug/mod.rs index 28aab4a6..a1ebb3a7 100644 --- a/libwebauthn/src/transport/ble/btleplug/mod.rs +++ b/libwebauthn/src/transport/ble/btleplug/mod.rs @@ -3,6 +3,7 @@ pub mod device; pub mod error; pub mod gatt; pub mod manager; +pub(crate) mod pairing; pub use connection::Connection; pub use device::FidoDevice; diff --git a/libwebauthn/src/transport/ble/btleplug/pairing.rs b/libwebauthn/src/transport/ble/btleplug/pairing.rs new file mode 100644 index 00000000..59b1a5c5 --- /dev/null +++ b/libwebauthn/src/transport/ble/btleplug/pairing.rs @@ -0,0 +1,111 @@ +//! Bonding enforcement for BLE FIDO authenticators. +//! +//! CTAP 2.2 §11.4 requires the platform-authenticator BLE link to be +//! bonded with LE Secure Connections. btleplug doesn't surface bonding +//! state, so on Linux we query bluez's `org.bluez.Device1.{Paired,Bonded}` +//! directly. Pairing itself is the OS's responsibility (e.g. +//! `bluetoothctl pair `); this module only verifies the link. + +use btleplug::api::{BDAddr, Peripheral as _}; +use btleplug::platform::Peripheral; +use tracing::{debug, info, warn}; + +use super::Error; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum BondingState { + Bonded, + NotBonded, + /// Bonding state could not be determined (non-bluez backend or DBus + /// unreachable); the caller decides whether to proceed. + Unknown, +} + +/// Reads `Paired` and `Bonded` from bluez DBus for `peripheral`. +pub(crate) async fn check_bonded(peripheral: &Peripheral) -> BondingState { + let address = peripheral.address(); + debug!(?address, "Checking bonded state via bluez DBus"); + + let result = tokio::task::spawn_blocking(move || query_bluez_bonded(address)).await; + + match result { + Ok(Ok((paired, bonded))) => { + info!(?address, paired, bonded, "bluez bonding state"); + if paired && bonded { + BondingState::Bonded + } else { + BondingState::NotBonded + } + } + Ok(Err(e)) => { + warn!(?address, error = ?e, "Could not query bluez bonding state"); + BondingState::Unknown + } + Err(e) => { + warn!(error = ?e, "bluez bonding query task panicked"); + BondingState::Unknown + } + } +} + +/// Returns `Err(ConnectionFailed)` when the device is reachable but +/// explicitly not bonded; falls through on `Unknown`. +pub(crate) async fn enforce_bonded(peripheral: &Peripheral) -> Result<(), Error> { + match check_bonded(peripheral).await { + BondingState::Bonded => Ok(()), + BondingState::Unknown => { + warn!( + "Could not verify LE Secure Connections bonding via bluez; \ + proceeding under OS pairing enforcement" + ); + Ok(()) + } + BondingState::NotBonded => { + warn!( + "BLE FIDO authenticator is not bonded with LE Secure Connections; \ + CTAP 2.2 §11.4 requires bonding. Pair the device via the OS \ + (e.g. `bluetoothctl pair `) before retrying." + ); + Err(Error::ConnectionFailed) + } + } +} + +/// btleplug doesn't expose the adapter index, so we walk the bluez +/// ObjectManager tree and match the first device with this address. +fn query_bluez_bonded(address: BDAddr) -> Result<(bool, bool), String> { + use dbus::arg::{PropMap, RefArg}; + use dbus::blocking::stdintf::org_freedesktop_dbus::ObjectManager; + use dbus::blocking::{Connection, Proxy}; + use std::time::Duration as StdDuration; + + let conn = Connection::new_system().map_err(|e| format!("dbus connect: {e}"))?; + let manager = Proxy::new("org.bluez", "/", StdDuration::from_secs(2), &conn); + let objects = manager + .get_managed_objects() + .map_err(|e| format!("GetManagedObjects: {e}"))?; + + let mac_lower = format!("{:x}", address); + let dev_segment = format!("dev_{}", mac_lower.replace(':', "_").to_uppercase()); + + for (path, interfaces) in objects { + let path_str = path.to_string(); + if !path_str.starts_with("/org/bluez/") || !path_str.ends_with(&dev_segment) { + continue; + } + let Some(device_props): Option<&PropMap> = interfaces.get("org.bluez.Device1") else { + continue; + }; + let paired = device_props + .get("Paired") + .and_then(|v| v.0.as_any().downcast_ref::().copied()) + .unwrap_or(false); + let bonded = device_props + .get("Bonded") + .and_then(|v| v.0.as_any().downcast_ref::().copied()) + .unwrap_or(false); + return Ok((paired, bonded)); + } + + Err(format!("device {address} not found in bluez ObjectManager")) +}