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

- Consume fidoStatus notifications instead of GATT Read
- Use Write Request for spec-declared Write characteristics
- Refuse unbonded LE links per CTAP 2.2 §11.4.3
- Enumerate undiscovered peripherals
- Add webauthn_ble example

Author: AlfioEmanueleFresta

README.md

@@ -70,7 +70,7 @@ WebAuthn examples consume and emit JSON per the [WebAuthn IDL][webauthn].
 | Transport             | FIDO U2F                                                                                                                 | WebAuthn (FIDO2)                                                                                                                   |
 | --------------------- | ------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------- |
 | **USB (HID)**         | `cargo run --example u2f_hid`                                                                                            | `cargo run --example webauthn_hid`                                                                                                 |
-| **Bluetooth (BLE)**   | `cargo run --example u2f_ble`                                                                                            | —                                                                                                                                  |
+| **Bluetooth (BLE)**   | `cargo run --example u2f_ble`                                                                                            | `cargo run --example webauthn_ble`                                                                                                 |
 | **NFC** [^nfc]        | `cargo run --features nfc-backend-pcsc --example u2f_nfc`<br>`cargo run --features nfc-backend-libnfc --example u2f_nfc` | `cargo run --features nfc-backend-pcsc --example webauthn_nfc`<br>`cargo run --features nfc-backend-libnfc --example webauthn_nfc` |
 | **Hybrid (caBLE v2)** | —                                                                                                                        | `cargo run --example webauthn_cable`                                                                                               |
 

libwebauthn/Cargo.toml

@@ -116,6 +116,10 @@ required-features = ["nfc"]
 name = "webauthn_hid"
 path = "examples/ceremony/webauthn_hid.rs"
 
+[[example]]
+name = "webauthn_ble"
+path = "examples/ceremony/webauthn_ble.rs"
+
 [[example]]
 name = "webauthn_nfc"
 path = "examples/ceremony/webauthn_nfc.rs"

libwebauthn/examples/ceremony/webauthn_ble.rs

@@ -0,0 +1,120 @@
+use std::error::Error;
+
+use libwebauthn::ops::webauthn::{
+    DatFilePublicSuffixList, GetAssertionRequest, JsonFormat, MakeCredentialRequest, RequestOrigin,
+    WebAuthnIDL as _, WebAuthnIDLResponse as _,
+};
+use libwebauthn::proto::ctap2::Ctap2PublicKeyCredentialDescriptor;
+use libwebauthn::transport::ble::list_devices;
+use libwebauthn::transport::{Channel as _, Device};
+use libwebauthn::webauthn::WebAuthn;
+
+#[path = "../common/mod.rs"]
+mod common;
+
+#[tokio::main]
+pub async fn main() -> Result<(), Box<dyn Error>> {
+    common::setup_logging();
+
+    let devices = list_devices().await?;
+    println!("Devices found: {:?}", devices);
+
+    for mut device in devices {
+        println!("Selected BLE authenticator: {}", &device);
+        let mut channel = device.channel().await?;
+
+        let request_origin: RequestOrigin =
+            "https://example.org".try_into().expect("Invalid origin");
+        let psl = DatFilePublicSuffixList::from_system_file().expect(
+            "PSL not available; install the publicsuffix-list package or pass an explicit path",
+        );
+        let request_json = r#"
+                {
+                    "rp": {
+                        "id": "example.org",
+                        "name": "Example Relying Party"
+                    },
+                    "user": {
+                        "id": "MTIzNDU2NzgxMjM0NTY3ODEyMzQ1Njc4MTIzNDU2Nzg",
+                        "name": "Mario Rossi",
+                        "displayName": "Mario Rossi"
+                    },
+                    "challenge": "MTIzNDU2NzgxMjM0NTY3ODEyMzQ1Njc4MTIzNDU2Nzg",
+                    "pubKeyCredParams": [
+                        {"type": "public-key", "alg": -7}
+                    ],
+                    "timeout": 60000,
+                    "excludeCredentials": [],
+                    "authenticatorSelection": {
+                        "residentKey": "discouraged",
+                        "userVerification": "preferred"
+                    },
+                    "attestation": "none"
+                }
+                "#;
+        let make_credentials_request: MakeCredentialRequest =
+            MakeCredentialRequest::from_json(&request_origin, &psl, request_json)
+                .expect("Failed to parse request JSON");
+        println!(
+            "WebAuthn MakeCredential request: {:?}",
+            make_credentials_request
+        );
+
+        let state_recv = channel.get_ux_update_receiver();
+        tokio::spawn(common::handle_uv_updates(state_recv));
+
+        let response =
+            retry_user_errors!(channel.webauthn_make_credential(&make_credentials_request))
+                .unwrap();
+        println!("WebAuthn MakeCredential response: {:?}", response);
+
+        match response.to_json_string(&make_credentials_request, JsonFormat::Prettified) {
+            Ok(response_json) => {
+                println!(
+                    "WebAuthn MakeCredential response (JSON):\n{}",
+                    response_json
+                );
+            }
+            Err(e) => {
+                eprintln!("Failed to serialize MakeCredential response: {:?}", e);
+            }
+        }
+
+        let cred: Ctap2PublicKeyCredentialDescriptor =
+            (&response.authenticator_data).try_into().unwrap();
+        let cred_id_b64 = base64_url::encode(cred.id.as_ref());
+        let request_json = format!(
+            r#"
+                {{
+                    "challenge": "Y3JlZGVudGlhbHMtZm9yLWxpbnV4L2xpYndlYmF1dGhu",
+                    "timeout": 30000,
+                    "rpId": "example.org",
+                    "userVerification": "discouraged",
+                    "allowCredentials": [
+                        {{"id": "{cred_id_b64}", "type": "public-key", "transports": ["ble"]}}
+                    ]
+                }}
+                "#
+        );
+        let get_assertion: GetAssertionRequest =
+            GetAssertionRequest::from_json(&request_origin, &psl, &request_json)
+                .expect("Failed to parse request JSON");
+        println!("WebAuthn GetAssertion request: {:?}", get_assertion);
+
+        let response = retry_user_errors!(channel.webauthn_get_assertion(&get_assertion)).unwrap();
+        println!("WebAuthn GetAssertion response: {:?}", response);
+
+        for assertion in &response.assertions {
+            match assertion.to_json_string(&get_assertion, JsonFormat::Prettified) {
+                Ok(assertion_json) => {
+                    println!("WebAuthn GetAssertion response (JSON):\n{}", assertion_json);
+                }
+                Err(e) => {
+                    eprintln!("Failed to serialize GetAssertion response: {:?}", e);
+                }
+            }
+        }
+    }
+
+    Ok(())
+}

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