linux-credentials / linux-credentials/libwebauthn
fix(cable): close hybrid transport spec gaps in tunnel and L2CAP
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 579
- Forks
- 27
- Avg merge
- 4d 6h
- Merged PRs (30d)
- 1
Description
## Problem
The hybrid (caBLE) transport in `transport/cable` handles the common QR-initiated flow, but four behaviours required by the CTAP hybrid / caBLE v2 spec are missing or broken, across tunnel connect handling and L2CAP framing.
## Why it matters
These break interop with real authenticators and platforms. Redirecting tunnel servers are unreachable, dead linking records are retried forever, clean shutdown and late linking updates are lost, and some direct-BLE messages get corrupted.
## What to do
### 1. Follow HTTP redirects on tunnel connect
`tunnel::connect()` (`libwebauthn/src/transport/cable/tunnel.rs`) calls `tokio_tungstenite::connect_async()`, which doesn't follow redirects, so any 3xx becomes `ConnectionFailed`. The spec requires following them.
- [ ] Follow 3xx redirects (e.g. 307/308) to the `Location` target before the WebSocket upgrade, with a redirect cap.
- [ ] Re-attach `Sec-WebSocket-Protocol: fido.cable`, and `X-caBLE-Client-Payload` for known-device connections, on the redirected request.
### 2. Handle HTTP 410 Gone on state-assisted contact
`connect()` collapses every non-101 status into `ConnectionFailed`. For `CableTunnelConnectionType::KnownDevice` (`/cable/contact/{contact_id}`), a 410 means the linking record is permanently gone, but nothing deletes it, so the dead link is retried forever.
- [ ] Surface 410 distinctly. No `TransportError` variant exists yet (`transport/error.rs`).
- [ ] On 410, delete the record via `CableKnownDeviceInfoStore::delete_known_device` and stop retrying. `connect_data_channel()` in `connection_stages.rs` needs store access (today it is only in the post-handshake `connection()` path).
### 3. Send Shutdown on close and linger for late linking data
`CableChannel::close()` (`channel.rs`) is an empty TODO and `Drop` just aborts. We handle inbound `CableTunnelMessageType::Shutdown` (`protocol.rs`, `RecvOutcome::PeerShutdown`) but never send one, and tearing down right after the CTAP response drops any post-transaction `Update` linking message.
- [ ] On `close()`, send a `Shutdown` frame over the encrypted channel before dropping it.
- [ ] After the CTAP response, linger briefly (~2 minutes) to capture late `Update` linking data.
### 4. Fix ambiguous CRLF framing over L2CAP
`l2cap.rs` ends each message with `EOM = [0x0D, 0x0A]` and `split_next_message()` splits at the first CRLF, but binary Noise/AES-GCM payloads can contain `0x0D 0x0A`. For a 700-1400 byte ciphertext that is a ~1-2% chance of an internal CRLF corrupting the frame. The `split_handles_binary_payload` test only covers payloads with no internal CRLF.
- [ ] Make L2CAP message boundaries unambiguous for arbitrary binary payloads.
- [ ] Add a regression test for a payload containing an internal `0x0D 0x0A`.
- [ ] Raise the framing ambiguity with the CTAP hybrid spec editors. Delimiter framing without escaping or a length prefix is unsafe and likely needs a spec clarification.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with transport/cable/tunnel.rs and connection_stages.rs, then inspect transport/error.rs and CableKnownDeviceInfoStore::delete_known_device for connect status handling. Read channel.rs, protocol.rs, and l2cap.rs, and run the existing split_handles_binary_payload test before changing framing or shutdown behavior. Done means the four listed transport and framing cases are covered, including an internal-CRLF regression test and the required spec clarification is raised.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking, security, testing-qa
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100