Skip to content

Conversation

@AlfioEmanueleFresta
Copy link
Member

@AlfioEmanueleFresta AlfioEmanueleFresta commented May 11, 2025

Changes

  • Updates internals of CableKnownDeviceInfoStore, to make it easier to use and thread-safe.
  • Adds parsing of caBLE update messages. This includes a Google-specific key (999). Due to a limitation in serde-indexed, parsing of this structure has to be done field-by-field with serde_cbor. To be fixed properly with Ignore unknown CTAP response fields #66.
  • Connect to known device from saved state.
  • Verifies handshake signature. Note this currently only works for linking information received on QR-initiated transactions. The protocol doesn't seen to support subsequent updates, as the ephemeral QR-code identity key is needed for verification.

Copy link
Collaborator

@msirringhaus msirringhaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only small comments so far

@AlfioEmanueleFresta
Copy link
Member Author

AlfioEmanueleFresta commented May 14, 2025

Connecting to a known device is now working on Android.

I'll add handshake signature verification next, as part of this PR.

@AlfioEmanueleFresta AlfioEmanueleFresta changed the title Hybrid transport: Add state-assisted transactions (WiP) Hybrid transport: Add state-assisted transactions May 18, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for state-assisted caBLE transactions by refactoring the tunnel API, making device storage thread-safe, and parsing/verifying update messages.

  • Introduce CableTunnelConnectionType, separate handshake (do_handshake) from channel setup (channel), and add connection_recv_update parsing with HMAC verification.
  • Make CableKnownDeviceInfoStore thread-safe (using Arc<Mutex<_>>), define CableKnownDeviceInfo and CableKnownDevice flows, and update examples to use a persistent store.
  • Factor out BLE advertisement handling into advertisement.rs and update QR code device logic to consume it.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
libwebauthn/src/transport/error.rs Added UnknownDevice, InvalidKey, and InvalidSignature error variants
libwebauthn/src/transport/cable/tunnel.rs Refactored tunnel connection, added state-assisted flow, parsing of update messages
libwebauthn/src/transport/cable/qr_code_device.rs Switched to Arc store, removed embedded BLE logic, now uses advertisement module
libwebauthn/src/transport/cable/known_devices.rs Thread-safe store implementation, new CableKnownDeviceInfo and CableKnownDevice types
libwebauthn/src/transport/cable/advertisement.rs New BLE advertisement module, extracts and decrypts service data
libwebauthn/src/transport/cable/crypto.rs Changed derive to return a fixed-size [u8; 64] instead of Vec<u8>
libwebauthn/src/transport/cable/channel.rs Simplified CableChannel, removed device enum, added Drop impl to abort tasks
libwebauthn/src/transport/ble/btleplug/manager.rs Updated discovery API to include Adapter in stream items
libwebauthn/src/transport/cable/mod.rs Exposed the new advertisement submodule
libwebauthn/examples/webauthn_cable.rs Example updated to demonstrate persistent known-device flow over QR and state-assisted paths
Comments suppressed due to low confidence (2)

libwebauthn/src/transport/ble/btleplug/manager.rs:84

  • The return type trait bound '+ use<'>' is invalid syntax; it should likely be '+ '', or a correct lifetime specifier.
)-> Result<impl Stream<Item = (Adapter, Peripheral, Vec<u8>)> + use<'_>, Error> {

libwebauthn/src/transport/cable/tunnel.rs:590

  • The new function connection_recv_update contains non-trivial parsing logic for CAble update messages but lacks corresponding unit tests. Consider adding tests that cover valid, missing-field, and error cases.
async fn connection_recv_update(message: &[u8]) -> Result<Option<CableLinkingInfo>, Error> {

@AlfioEmanueleFresta AlfioEmanueleFresta marked this pull request as ready for review May 19, 2025 06:10
Copy link
Collaborator

@msirringhaus msirringhaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nitpicks. I currently can't judge the overall spec compliance, but I think you know what you are doing :)

@AlfioEmanueleFresta AlfioEmanueleFresta force-pushed the hybrid-transport-state-assisted branch from 821e2aa to 3cff871 Compare May 23, 2025 15:28
@AlfioEmanueleFresta AlfioEmanueleFresta merged commit e2789ad into master May 23, 2025
4 checks passed
@AlfioEmanueleFresta AlfioEmanueleFresta deleted the hybrid-transport-state-assisted branch May 23, 2025 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants