fix(ble): notifications, write-with-response, LE secure connections check

libwebauthn/src/transport/ble/btleplug/connection.rs

@@ -1,21 +1,42 @@
 use std::io::Cursor as IOCursor;
+use std::pin::Pin;
+use std::sync::Arc;
+use std::time::Duration;
 
-use btleplug::api::{Peripheral as _, WriteType};
+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;
+use super::gatt::write_type_for;
 use super::Error;
 use crate::fido::FidoRevision;
 use crate::transport::ble::framing::{
     BleCommand, BleFrame as Frame, BleFrameParser, BleFrameParserResult,
 };
 
-#[derive(Debug, Clone)]
+type NotificationStream = Pin<Box<dyn Stream<Item = ValueNotification> + 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<Mutex<NotificationStream>>,
+}
+
+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 {
@@ -24,9 +45,26 @@ impl Connection {
         services: &FidoEndpoints,
         revision: &FidoRevision,
     ) -> Result<Self, Error> {
+        // 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)
@@ -60,16 +98,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,25 +115,63 @@ 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))?;
 
         info!(?revision, "Successfully selected FIDO revision");
         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<Frame, Error> {
+    pub async fn frame_recv(&self, op_timeout: Duration) -> Result<Frame, Error> {
         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);
 
@@ -133,13 +207,7 @@ impl Connection {
         }
     }
 
-    async fn receive_fragment(&self) -> Result<Vec<u8>, 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)

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);
+    }
+}

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

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;

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 <ADDR>`); 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 <ADDR>`) 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::<bool>().copied())
+            .unwrap_or(false);
+        let bonded = device_props
+            .get("Bonded")
+            .and_then(|v| v.0.as_any().downcast_ref::<bool>().copied())
+            .unwrap_or(false);
+        return Ok((paired, bonded));
+    }
+
+    Err(format!("device {address} not found in bluez ObjectManager"))
+}