From 758675c1975bdeac8644dbb24f0b9d529e7d062c Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 7 Jun 2026 22:09:39 +0100 Subject: [PATCH 01/13] fix(cable): send Shutdown on close On hybrid connections the client never sent a Shutdown frame and tore the connection down immediately after the response. This sends a Shutdown control frame when the channel is closed and adds the encrypted test harness for the tunnel protocol. Part of #257. --- libwebauthn/Cargo.toml | 2 + libwebauthn/src/transport/cable/channel.rs | 10 +- .../src/transport/cable/connection_stages.rs | 3 + .../src/transport/cable/known_devices.rs | 3 + libwebauthn/src/transport/cable/protocol.rs | 197 +++++++++++++++++- .../src/transport/cable/qr_code_device.rs | 3 + 6 files changed, 211 insertions(+), 7 deletions(-) diff --git a/libwebauthn/Cargo.toml b/libwebauthn/Cargo.toml index a59328b2..79bb67d7 100644 --- a/libwebauthn/Cargo.toml +++ b/libwebauthn/Cargo.toml @@ -120,6 +120,8 @@ reqwest = { version = "0.12", default-features = false, features = [ [dev-dependencies] tracing-subscriber = { version = "0.3.3", features = ["env-filter"] } qrcode = "0.14.1" +# test-util enables paused time for deterministic timeout/linger tests +tokio = { version = "1.45", features = ["test-util"] } # For turning on logging in unittests test-log = { version = "0.2" } diff --git a/libwebauthn/src/transport/cable/channel.rs b/libwebauthn/src/transport/cable/channel.rs index 5bc1cacb..3ecde18a 100644 --- a/libwebauthn/src/transport/cable/channel.rs +++ b/libwebauthn/src/transport/cable/channel.rs @@ -48,6 +48,7 @@ pub struct CableChannel { pub(crate) ux_update_sender: broadcast::Sender, pub(crate) connection_state_receiver: watch::Receiver, pub(crate) persistent_token_store: Option>, + pub(crate) close_sender: Option>, } impl CableChannel { @@ -141,7 +142,14 @@ impl Channel for CableChannel { } async fn close(&mut self) { - // TODO Send CableTunnelMessageType#Shutdown and drop the connection + // Signal the loop to send Shutdown, then wait for it to flush and terminate. + if let Some(close_sender) = self.close_sender.take() { + let _ = close_sender.send(()).await; + } + let mut connection_state = self.connection_state_receiver.clone(); + let _ = connection_state + .wait_for(|state| *state == ConnectionState::Terminated) + .await; } async fn apdu_send( diff --git a/libwebauthn/src/transport/cable/connection_stages.rs b/libwebauthn/src/transport/cable/connection_stages.rs index 740c64bc..8c560e40 100644 --- a/libwebauthn/src/transport/cable/connection_stages.rs +++ b/libwebauthn/src/transport/cable/connection_stages.rs @@ -201,6 +201,7 @@ pub(crate) struct TunnelConnectionInput { pub noise_state: TunnelNoiseState, pub cbor_tx_recv: mpsc::Receiver, pub cbor_rx_send: mpsc::Sender, + pub close_rx: mpsc::Receiver<()>, } impl TunnelConnectionInput { @@ -209,6 +210,7 @@ impl TunnelConnectionInput { known_device_store: Option>, cbor_tx_recv: mpsc::Receiver, cbor_rx_send: mpsc::Sender, + close_rx: mpsc::Receiver<()>, ) -> Self { Self { connection_type: handshake_output.connection_type, @@ -218,6 +220,7 @@ impl TunnelConnectionInput { noise_state: handshake_output.noise_state, cbor_tx_recv, cbor_rx_send, + close_rx, } } } diff --git a/libwebauthn/src/transport/cable/known_devices.rs b/libwebauthn/src/transport/cable/known_devices.rs index 01d42cdc..b8132458 100644 --- a/libwebauthn/src/transport/cable/known_devices.rs +++ b/libwebauthn/src/transport/cable/known_devices.rs @@ -201,6 +201,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { let (ux_update_sender, _) = broadcast::channel(16); let (cbor_tx_send, cbor_tx_recv) = mpsc::channel(16); let (cbor_rx_send, cbor_rx_recv) = mpsc::channel(16); + let (close_sender, close_rx) = mpsc::channel(1); let (connection_state_sender, connection_state_receiver) = watch::channel(ConnectionState::Connecting); @@ -224,6 +225,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { Some(known_device.store), cbor_tx_recv, cbor_rx_send, + close_rx, ); match protocol::connection(tunnel_input).await { @@ -246,6 +248,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { ux_update_sender, connection_state_receiver, persistent_token_store: settings.persistent_token_store, + close_sender: Some(close_sender), }) } } diff --git a/libwebauthn/src/transport/cable/protocol.rs b/libwebauthn/src/transport/cable/protocol.rs index 73f68bb8..aeb8eaf2 100644 --- a/libwebauthn/src/transport/cable/protocol.rs +++ b/libwebauthn/src/transport/cable/protocol.rs @@ -330,6 +330,15 @@ pub(crate) async fn connection(mut input: TunnelConnectionInput) -> Result<(), C } } } + _ = input.close_rx.recv() => { + debug!("Channel close requested, sending Shutdown control frame"); + if let Err(e) = + connection_send_shutdown(&mut *input.data_channel, &mut input.noise_state).await + { + warn!(?e, "Failed to send Shutdown control frame on close"); + } + return Ok(()); + } Some(request) = input.cbor_tx_recv.recv() => { match request.command { // Optimisation: respond to GetInfo requests immediately with the cached response @@ -378,16 +387,47 @@ async fn connection_send( } trace!(?cbor_request, cbor_request_len = cbor_request.len()); - let extra_bytes = PADDING_GRANULARITY - (cbor_request.len() % PADDING_GRANULARITY); - let padded_len = cbor_request.len() + extra_bytes; + send_tunnel_frame( + CableTunnelMessageType::Ctap, + &cbor_request, + data_channel, + noise_state, + ) + .await +} + +/// Sends an empty `Shutdown` control frame over the encrypted channel. +async fn connection_send_shutdown( + data_channel: &mut dyn CableDataChannel, + noise_state: &mut TunnelNoiseState, +) -> Result<(), CableError> { + debug!("Sending Shutdown control frame"); + send_tunnel_frame( + CableTunnelMessageType::Shutdown, + &[], + data_channel, + noise_state, + ) + .await +} - let mut padded_cbor_request = cbor_request.clone(); - padded_cbor_request.resize(padded_len, 0u8); - if let Some(last) = padded_cbor_request.last_mut() { +/// Pads `payload`, wraps it in a `CableTunnelMessage`, encrypts it, and sends it. +async fn send_tunnel_frame( + message_type: CableTunnelMessageType, + payload: &[u8], + data_channel: &mut dyn CableDataChannel, + noise_state: &mut TunnelNoiseState, +) -> Result<(), CableError> { + let extra_bytes = PADDING_GRANULARITY - (payload.len() % PADDING_GRANULARITY); + let padded_len = payload.len() + extra_bytes; + + let mut padded_payload = payload.to_vec(); + padded_payload.resize(padded_len, 0u8); + if let Some(last) = padded_payload.last_mut() { *last = (extra_bytes - 1) as u8; } - let frame = CableTunnelMessage::new(CableTunnelMessageType::Ctap, &padded_cbor_request); + let frame = CableTunnelMessage::new(message_type, &padded_payload); let frame_serialized = frame.to_vec(); trace!(?frame_serialized); @@ -794,4 +834,149 @@ mod tests { let stripped = strip_frame_padding(frame).unwrap(); assert_eq!(stripped, vec![0xAA, 0xBB, 0xCC, 0xDD]); } + + use serde_indexed::SerializeIndexed; + use tokio::sync::mpsc; + + /// In-memory data channel: records outbound frames and replays queued inbound ones. + struct TestDataChannel { + inbound: mpsc::UnboundedReceiver>, + outbound: mpsc::UnboundedSender>, + } + + #[async_trait] + impl CableDataChannel for TestDataChannel { + async fn send(&mut self, message: &[u8]) -> Result<(), CableError> { + let _ = self.outbound.send(message.to_vec()); + Ok(()) + } + + async fn recv(&mut self) -> Result>, CableError> { + Ok(self.inbound.recv().await) + } + } + + /// Two Noise transport states that can encrypt/decrypt to each other. + fn paired_transport_states() -> (TransportState, TransportState) { + let mut initiator = Builder::new("Noise_NN_P256_AESGCM_SHA256".parse().unwrap()) + .build_initiator() + .unwrap(); + let mut responder = Builder::new("Noise_NN_P256_AESGCM_SHA256".parse().unwrap()) + .build_responder() + .unwrap(); + let mut a = [0u8; 1024]; + let mut b = [0u8; 1024]; + let n = initiator.write_message(&[], &mut a).unwrap(); + responder.read_message(&a[..n], &mut b).unwrap(); + let n = responder.write_message(&[], &mut a).unwrap(); + initiator.read_message(&a[..n], &mut b).unwrap(); + ( + initiator.into_transport_mode().unwrap(), + responder.into_transport_mode().unwrap(), + ) + } + + fn pad(mut payload: Vec) -> Vec { + let extra = PADDING_GRANULARITY - (payload.len() % PADDING_GRANULARITY); + let new_len = payload.len() + extra; + payload.resize(new_len, 0u8); + *payload.last_mut().unwrap() = (extra - 1) as u8; + payload + } + + fn encrypt(state: &mut TransportState, plaintext: &[u8]) -> Vec { + let mut out = vec![0u8; plaintext.len() + 64]; + let n = state.write_message(plaintext, &mut out).unwrap(); + out.truncate(n); + out + } + + fn decrypt(state: &mut TransportState, ciphertext: &[u8]) -> Vec { + let mut out = vec![0u8; ciphertext.len() + 64]; + let n = state.read_message(ciphertext, &mut out).unwrap(); + out.truncate(n); + out + } + + #[derive(SerializeIndexed)] + struct TestInitialMessage { + #[serde(index = 0x01)] + info: ByteBuf, + } + + /// Encrypted initial post-handshake message carrying a minimal GetInfo. + fn encrypted_initial_message(responder: &mut TransportState) -> Vec { + let get_info = Ctap2GetInfoResponse { + versions: vec!["FIDO_2_0".to_string()], + aaguid: ByteBuf::from(vec![0u8; 16]), + ..Default::default() + }; + let initial = TestInitialMessage { + info: ByteBuf::from(cbor::to_vec(&get_info).unwrap()), + }; + encrypt(responder, &pad(cbor::to_vec(&initial).unwrap())) + } + + fn qr_connection_type() -> CableTunnelConnectionType { + CableTunnelConnectionType::QrCode { + routing_id: "000000".to_string(), + tunnel_id: "00000000000000000000000000000000".to_string(), + private_key: NonZeroScalar::random(&mut OsRng), + } + } + + /// Decrypts an outbound frame and returns its tunnel message type byte. + fn outbound_message_type(frame: &[u8], responder: &mut TransportState) -> u8 { + let stripped = strip_frame_padding(decrypt(responder, frame)).unwrap(); + *stripped.first().unwrap() + } + + #[tokio::test] + async fn connection_sends_shutdown_on_close() { + let (initiator, mut responder) = paired_transport_states(); + let (inbound_tx, inbound_rx) = mpsc::unbounded_channel::>(); + let (outbound_tx, mut outbound_rx) = mpsc::unbounded_channel::>(); + + inbound_tx + .send(encrypted_initial_message(&mut responder)) + .unwrap(); + + let (cbor_tx_send, cbor_tx_recv) = mpsc::channel::(4); + let (cbor_rx_send, cbor_rx_recv) = mpsc::channel::(4); + let (close_tx, close_rx) = mpsc::channel::<()>(1); + + let input = TunnelConnectionInput { + connection_type: qr_connection_type(), + tunnel_domain: "cable.example.com".to_string(), + known_device_store: None, + data_channel: Box::new(TestDataChannel { + inbound: inbound_rx, + outbound: outbound_tx, + }), + noise_state: TunnelNoiseState { + transport_state: initiator, + handshake_hash: vec![0u8; 32], + }, + cbor_tx_recv, + cbor_rx_send, + close_rx, + }; + + let handle = tokio::spawn(connection(input)); + + close_tx.send(()).await.unwrap(); + + let frame = outbound_rx + .recv() + .await + .expect("a frame on the outbound path"); + assert_eq!( + outbound_message_type(&frame, &mut responder), + CableTunnelMessageType::Shutdown as u8 + ); + assert!(handle.await.unwrap().is_ok()); + + // Keep the channel ends alive until the loop has shut down. + drop((inbound_tx, cbor_tx_send, cbor_rx_recv, close_tx)); + } } diff --git a/libwebauthn/src/transport/cable/qr_code_device.rs b/libwebauthn/src/transport/cable/qr_code_device.rs index 5f85e8a3..bc523d09 100644 --- a/libwebauthn/src/transport/cable/qr_code_device.rs +++ b/libwebauthn/src/transport/cable/qr_code_device.rs @@ -247,6 +247,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { let (ux_update_sender, _) = broadcast::channel(16); let (cbor_tx_send, cbor_tx_recv) = mpsc::channel(16); let (cbor_rx_send, cbor_rx_recv) = mpsc::channel(16); + let (close_sender, close_rx) = mpsc::channel(1); let (connection_state_sender, connection_state_receiver) = watch::channel(ConnectionState::Connecting); @@ -270,6 +271,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { qr_device.store, cbor_tx_recv, cbor_rx_send, + close_rx, ); match protocol::connection(tunnel_input).await { Ok(()) => { @@ -291,6 +293,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { ux_update_sender, connection_state_receiver, persistent_token_store: settings.persistent_token_store, + close_sender: Some(close_sender), }) } From 5768c4349fc40285634e667ffc9d9ee06e64a2af Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 20:33:00 +0100 Subject: [PATCH 02/13] fix(cable): decode a type-only Shutdown frame as a peer close A Shutdown control frame carries no payload, so the empty-payload check rejected it as invalid framing before the message type was inspected and a peer shutdown surfaced as a UX error instead of a clean close. --- libwebauthn/src/transport/cable/protocol.rs | 85 ++++++++++++++++++++- 1 file changed, 81 insertions(+), 4 deletions(-) diff --git a/libwebauthn/src/transport/cable/protocol.rs b/libwebauthn/src/transport/cable/protocol.rs index aeb8eaf2..5b763477 100644 --- a/libwebauthn/src/transport/cable/protocol.rs +++ b/libwebauthn/src/transport/cable/protocol.rs @@ -46,9 +46,6 @@ impl CableTunnelMessage { } pub fn from_slice(slice: &[u8]) -> Result { let (type_byte, payload) = slice.split_first().ok_or(CableError::InvalidFraming)?; - if payload.is_empty() { - return Err(CableError::InvalidFraming); - } let message_type = match *type_byte { 0 => CableTunnelMessageType::Shutdown, @@ -59,6 +56,11 @@ impl CableTunnelMessage { } }; + // Shutdown is the type byte alone. Ctap and Update must carry a payload. + if payload.is_empty() && message_type != CableTunnelMessageType::Shutdown { + return Err(CableError::InvalidFraming); + } + Ok(Self { message_type, payload: ByteBuf::from(payload.to_vec()), @@ -107,7 +109,7 @@ pub(crate) struct CableLinkingInfo { } #[repr(u8)] -#[derive(Debug, Clone, Copy, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] enum CableTunnelMessageType { Shutdown = 0, Ctap = 1, @@ -813,6 +815,37 @@ mod tests { ); } + #[test] + fn from_slice_accepts_type_only_shutdown() { + let message = CableTunnelMessage::from_slice(&[0]).unwrap(); + assert_eq!(message.message_type, CableTunnelMessageType::Shutdown); + assert!(message.payload.is_empty()); + } + + #[test] + fn from_slice_rejects_empty_ctap_and_update() { + assert!(matches!( + CableTunnelMessage::from_slice(&[1]), + Err(CableError::InvalidFraming) + )); + assert!(matches!( + CableTunnelMessage::from_slice(&[2]), + Err(CableError::InvalidFraming) + )); + } + + #[test] + fn from_slice_rejects_empty_frame_and_unknown_type() { + assert!(matches!( + CableTunnelMessage::from_slice(&[]), + Err(CableError::InvalidFraming) + )); + assert!(matches!( + CableTunnelMessage::from_slice(&[3, 0]), + Err(CableError::InvalidFraming) + )); + } + #[test] fn strip_frame_padding_rejects_empty() { let result = strip_frame_padding(Vec::new()); @@ -979,4 +1012,48 @@ mod tests { // Keep the channel ends alive until the loop has shut down. drop((inbound_tx, cbor_tx_send, cbor_rx_recv, close_tx)); } + + #[tokio::test] + async fn peer_shutdown_ends_connection_cleanly() { + let (initiator, mut responder) = paired_transport_states(); + let (inbound_tx, inbound_rx) = mpsc::unbounded_channel::>(); + let (outbound_tx, mut outbound_rx) = mpsc::unbounded_channel::>(); + + inbound_tx + .send(encrypted_initial_message(&mut responder)) + .unwrap(); + // A type-only Shutdown frame from the peer, padded like any other frame. + inbound_tx + .send(encrypt( + &mut responder, + &pad(vec![CableTunnelMessageType::Shutdown as u8]), + )) + .unwrap(); + + let (cbor_tx_send, cbor_tx_recv) = mpsc::channel::(4); + let (cbor_rx_send, cbor_rx_recv) = mpsc::channel::(4); + let (close_tx, close_rx) = mpsc::channel::<()>(1); + + let input = TunnelConnectionInput { + connection_type: qr_connection_type(), + tunnel_domain: "cable.example.com".to_string(), + known_device_store: None, + data_channel: Box::new(TestDataChannel { + inbound: inbound_rx, + outbound: outbound_tx, + }), + noise_state: TunnelNoiseState { + transport_state: initiator, + handshake_hash: vec![0u8; 32], + }, + cbor_tx_recv, + cbor_rx_send, + close_rx, + }; + + assert!(connection(input).await.is_ok()); + assert!(outbound_rx.try_recv().is_err(), "no frame is sent in reply"); + + drop((inbound_tx, cbor_tx_send, cbor_rx_recv, close_tx)); + } } From 3b30efb527a6beaab44e014c3efaccca4870bf85 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 20:33:47 +0100 Subject: [PATCH 03/13] feat(cable): add the Lingering connection state Nothing publishes it yet. A lingering connection admits no further operations and reports as closed. --- libwebauthn/src/transport/cable/channel.rs | 75 ++++++++++++++++++++-- 1 file changed, 70 insertions(+), 5 deletions(-) diff --git a/libwebauthn/src/transport/cable/channel.rs b/libwebauthn/src/transport/cable/channel.rs index 3ecde18a..a04ec4e7 100644 --- a/libwebauthn/src/transport/cable/channel.rs +++ b/libwebauthn/src/transport/cable/channel.rs @@ -25,11 +25,15 @@ use super::known_devices::CableKnownDevice; use super::qr_code_device::CableQrCodeDevice; #[derive(Debug, Clone, PartialEq)] +#[non_exhaustive] pub enum ConnectionState { /// Connection is being established (proximity check, connecting, authenticating) Connecting, /// Connection is fully established and ready for operations Connected, + /// Shutdown has been sent and the connection is only receiving a late + /// linking update. No further operations are admitted. + Lingering, /// Connection has terminated Terminated, } @@ -65,8 +69,10 @@ impl CableChannel { // surfaces the same variant as one that terminates while we wait; // the caller can't observe the timing difference and the asymmetry // was accidental. - if *rx.borrow() == ConnectionState::Terminated { - return Err(CableError::ConnectionFailed); + match *rx.borrow() { + ConnectionState::Terminated => return Err(CableError::ConnectionFailed), + ConnectionState::Lingering => return Err(CableError::ConnectionLost), + _ => {} } // Wait for state change @@ -74,6 +80,7 @@ impl CableChannel { match *rx.borrow() { ConnectionState::Connected => return Ok(()), ConnectionState::Terminated => return Err(CableError::ConnectionFailed), + ConnectionState::Lingering => return Err(CableError::ConnectionLost), ConnectionState::Connecting => continue, } } @@ -135,9 +142,12 @@ impl Channel for CableChannel { } async fn status(&self) -> ChannelStatus { - match self.handle_connection.is_finished() { - true => ChannelStatus::Closed, - false => ChannelStatus::Ready, + if self.handle_connection.is_finished() { + return ChannelStatus::Closed; + } + match *self.connection_state_receiver.borrow() { + ConnectionState::Lingering | ConnectionState::Terminated => ChannelStatus::Closed, + _ => ChannelStatus::Ready, } } @@ -232,3 +242,58 @@ impl Ctap2AuthTokenStore for CableChannel { self.persistent_token_store.clone() } } + +#[cfg(test)] +mod tests { + use super::*; + + fn channel_in_state(state: ConnectionState) -> (CableChannel, watch::Sender) { + let (ux_update_sender, _) = broadcast::channel(1); + let (cbor_sender, _cbor_tx_recv) = mpsc::channel(1); + let (_cbor_rx_send, cbor_receiver) = mpsc::channel(1); + let (close_sender, _close_rx) = mpsc::channel(1); + let (state_tx, connection_state_receiver) = watch::channel(state); + let channel = CableChannel { + handle_connection: task::spawn(std::future::pending()), + cbor_sender, + cbor_receiver, + ux_update_sender, + connection_state_receiver, + persistent_token_store: None, + close_sender: Some(close_sender), + }; + (channel, state_tx) + } + + #[tokio::test] + async fn wait_for_connection_rejects_lingering() { + let (channel, _state_tx) = channel_in_state(ConnectionState::Lingering); + assert!(matches!( + channel.wait_for_connection().await, + Err(CableError::ConnectionLost) + )); + } + + #[tokio::test] + async fn wait_for_connection_rejects_transition_to_lingering() { + let (channel, state_tx) = channel_in_state(ConnectionState::Connecting); + let waiter = tokio::spawn(async move { channel.wait_for_connection().await }); + state_tx.send(ConnectionState::Lingering).unwrap(); + assert!(matches!( + waiter.await.unwrap(), + Err(CableError::ConnectionLost) + )); + } + + #[tokio::test] + async fn status_maps_lingering_to_closed() { + let (channel, _state_tx) = channel_in_state(ConnectionState::Lingering); + assert!(matches!(channel.status().await, ChannelStatus::Closed)); + } + + #[tokio::test] + async fn status_maps_connected_to_ready() { + let (channel, _state_tx) = channel_in_state(ConnectionState::Connected); + assert!(matches!(channel.status().await, ChannelStatus::Ready)); + } +} From 511f1d949d1b2d126755665433bc216acdc2e0c3 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 20:38:53 +0100 Subject: [PATCH 04/13] feat(cable): caller-driven teardown with close, cancel and drop semantics Replace the one-shot close signal with a teardown watch shared between the channel and the connection task. close() sends Shutdown and waits for termination, cancel() drops the connection without a goodbye, and an unattended drop is a hard cancel. The connect and handshake stages and the initial receive are interruptible, so a close during connecting no longer hangs, and every outbound send is bounded. --- libwebauthn/src/transport/cable/channel.rs | 160 +++++++- .../src/transport/cable/connection_stages.rs | 31 +- .../src/transport/cable/known_devices.rs | 32 +- libwebauthn/src/transport/cable/linger.rs | 14 + libwebauthn/src/transport/cable/mod.rs | 1 + libwebauthn/src/transport/cable/protocol.rs | 378 +++++++++++++----- .../src/transport/cable/qr_code_device.rs | 32 +- libwebauthn/src/transport/channel.rs | 8 + 8 files changed, 518 insertions(+), 138 deletions(-) create mode 100644 libwebauthn/src/transport/cable/linger.rs diff --git a/libwebauthn/src/transport/cable/channel.rs b/libwebauthn/src/transport/cable/channel.rs index a04ec4e7..2269a76d 100644 --- a/libwebauthn/src/transport/cable/channel.rs +++ b/libwebauthn/src/transport/cable/channel.rs @@ -5,7 +5,7 @@ use std::time::Duration; use async_trait::async_trait; use tokio::sync::{broadcast, mpsc, watch}; use tokio::{task, time}; -use tracing::error; +use tracing::{debug, error, warn}; use crate::pin::persistent_token::PersistentTokenStore; use crate::proto::{ @@ -22,8 +22,14 @@ use crate::Transport; use crate::UvUpdate; use super::known_devices::CableKnownDevice; +use super::linger::Teardown; use super::qr_code_device::CableQrCodeDevice; +/// Bounds `close()`: the Shutdown send plus the task's return. +const CLOSE_FLUSH_TIMEOUT: Duration = Duration::from_secs(5); +/// Bounds `cancel()`: one select hop plus a socket drop. Aborts on expiry. +const CANCEL_TIMEOUT: Duration = Duration::from_secs(2); + #[derive(Debug, Clone, PartialEq)] #[non_exhaustive] pub enum ConnectionState { @@ -52,10 +58,33 @@ pub struct CableChannel { pub(crate) ux_update_sender: broadcast::Sender, pub(crate) connection_state_receiver: watch::Receiver, pub(crate) persistent_token_store: Option>, - pub(crate) close_sender: Option>, + pub(crate) teardown: Arc>, } impl CableChannel { + /// Sets the teardown intent if nobody has set one yet. Returns whether it did. + fn request_teardown(&self, intent: Teardown) -> bool { + self.teardown.send_if_modified(|current| { + if *current == Teardown::Active { + *current = intent; + true + } else { + false + } + }) + } + + /// Waits until the connection reaches a state matching `done`, bounded by `timeout`. + async fn wait_for_state( + &self, + timeout: Duration, + done: impl FnMut(&ConnectionState) -> bool, + ) -> bool { + let mut rx = self.connection_state_receiver.clone(); + let reached = time::timeout(timeout, rx.wait_for(done)).await.is_ok(); + reached + } + async fn wait_for_connection(&self) -> Result<(), CableError> { let mut rx = self.connection_state_receiver.clone(); @@ -98,7 +127,11 @@ impl Display for CableChannel { impl Drop for CableChannel { fn drop(&mut self) { - self.handle_connection.abort(); + // An unattended drop is a hard cancel. A teardown already under way + // (close, linger, cancel) is left to run its course. + if self.request_teardown(Teardown::Cancel) { + self.handle_connection.abort(); + } } } @@ -151,15 +184,32 @@ impl Channel for CableChannel { } } + /// Sends Shutdown, then waits for the connection to terminate. Never lingers. async fn close(&mut self) { - // Signal the loop to send Shutdown, then wait for it to flush and terminate. - if let Some(close_sender) = self.close_sender.take() { - let _ = close_sender.send(()).await; + self.request_teardown(Teardown::Close); + if !self + .wait_for_state(CLOSE_FLUSH_TIMEOUT, |state| { + *state == ConnectionState::Terminated + }) + .await + { + warn!("Timed out waiting for the hybrid connection to close"); + } + } + + /// Drops the connection without sending Shutdown. Always wins over a + /// graceful teardown already under way. + async fn cancel(&mut self) { + self.teardown.send_replace(Teardown::Cancel); + if !self + .wait_for_state(CANCEL_TIMEOUT, |state| { + *state == ConnectionState::Terminated + }) + .await + { + debug!("Aborting the hybrid connection task after cancel timeout"); + self.handle_connection.abort(); } - let mut connection_state = self.connection_state_receiver.clone(); - let _ = connection_state - .wait_for(|state| *state == ConnectionState::Terminated) - .await; } async fn apdu_send( @@ -251,7 +301,7 @@ mod tests { let (ux_update_sender, _) = broadcast::channel(1); let (cbor_sender, _cbor_tx_recv) = mpsc::channel(1); let (_cbor_rx_send, cbor_receiver) = mpsc::channel(1); - let (close_sender, _close_rx) = mpsc::channel(1); + let (teardown, _teardown_rx) = watch::channel(Teardown::Active); let (state_tx, connection_state_receiver) = watch::channel(state); let channel = CableChannel { handle_connection: task::spawn(std::future::pending()), @@ -260,11 +310,97 @@ mod tests { ux_update_sender, connection_state_receiver, persistent_token_store: None, - close_sender: Some(close_sender), + teardown: Arc::new(teardown), }; (channel, state_tx) } + /// A channel whose task mimics the connection loop's teardown handling: + /// it publishes `Terminated` on any intent and reports the intent seen. + fn channel_with_teardown_task() -> ( + CableChannel, + tokio::sync::oneshot::Receiver, + Arc>, + ) { + let (ux_update_sender, _) = broadcast::channel(1); + let (cbor_sender, _cbor_tx_recv) = mpsc::channel(1); + let (_cbor_rx_send, cbor_receiver) = mpsc::channel(1); + let (teardown, mut teardown_rx) = watch::channel(Teardown::Active); + let teardown = Arc::new(teardown); + let (state_tx, connection_state_receiver) = watch::channel(ConnectionState::Connected); + let (seen_tx, seen_rx) = tokio::sync::oneshot::channel(); + let handle_connection = task::spawn(async move { + let intent = super::super::connection_stages::next_teardown(&mut teardown_rx).await; + let _ = seen_tx.send(intent); + let _ = state_tx.send(ConnectionState::Terminated); + }); + let channel = CableChannel { + handle_connection, + cbor_sender, + cbor_receiver, + ux_update_sender, + connection_state_receiver, + persistent_token_store: None, + teardown: teardown.clone(), + }; + (channel, seen_rx, teardown) + } + + #[tokio::test] + async fn close_requests_graceful_close_and_waits_for_termination() { + let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); + channel.close().await; + assert_eq!(seen_rx.await.unwrap(), Teardown::Close); + assert_eq!( + *channel.connection_state_receiver.borrow(), + ConnectionState::Terminated + ); + assert_eq!(*teardown.borrow(), Teardown::Close); + assert!(matches!(channel.status().await, ChannelStatus::Closed)); + } + + #[tokio::test] + async fn cancel_requests_hard_cancel() { + let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); + channel.cancel().await; + assert_eq!(seen_rx.await.unwrap(), Teardown::Cancel); + assert_eq!(*teardown.borrow(), Teardown::Cancel); + } + + #[tokio::test] + async fn cancel_overrides_a_close_in_progress() { + let (mut channel, _seen_rx, teardown) = channel_with_teardown_task(); + assert!(channel.request_teardown(Teardown::Close)); + channel.cancel().await; + assert_eq!(*teardown.borrow(), Teardown::Cancel); + } + + #[tokio::test] + async fn unattended_drop_cancels_and_aborts() { + let (channel, seen_rx, teardown) = channel_with_teardown_task(); + drop(channel); + assert_eq!(*teardown.borrow(), Teardown::Cancel); + // The task was aborted, so it never reported the intent it saw. + assert!(seen_rx.await.is_err()); + } + + #[tokio::test] + async fn drop_after_close_does_not_downgrade_the_intent() { + let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); + channel.close().await; + drop(channel); + assert_eq!(*teardown.borrow(), Teardown::Close); + assert_eq!(seen_rx.await.unwrap(), Teardown::Close); + } + + #[tokio::test(start_paused = true)] + async fn cancel_aborts_a_task_that_ignores_the_intent() { + let (mut channel, _state_tx) = channel_in_state(ConnectionState::Connected); + channel.cancel().await; + let joined = (&mut channel.handle_connection).await; + assert!(joined.unwrap_err().is_cancelled()); + } + #[tokio::test] async fn wait_for_connection_rejects_lingering() { let (channel, _state_tx) = channel_in_state(ConnectionState::Lingering); diff --git a/libwebauthn/src/transport/cable/connection_stages.rs b/libwebauthn/src/transport/cable/connection_stages.rs index 8c560e40..e2eb8016 100644 --- a/libwebauthn/src/transport/cable/connection_stages.rs +++ b/libwebauthn/src/transport/cable/connection_stages.rs @@ -9,12 +9,14 @@ use super::crypto::{derive, KeyPurpose}; use super::data_channel::{CableDataChannel, WebSocketDataChannel}; use super::known_devices::{CableKnownDevice, CableKnownDeviceInfoStore, ClientNonce}; use super::l2cap::L2capDataChannel; +use super::linger::Teardown; use super::protocol::{self, CableTunnelConnectionType, TunnelNoiseState}; use super::qr_code_device::CableQrCodeDevice; use super::tunnel; use crate::proto::ctap2::cbor::{CborRequest, CborResponse}; use crate::transport::ble::btleplug::FidoDevice; use crate::transport::cable::error::CableError; +use std::future::Future; use std::sync::Arc; #[derive(Debug)] @@ -201,7 +203,7 @@ pub(crate) struct TunnelConnectionInput { pub noise_state: TunnelNoiseState, pub cbor_tx_recv: mpsc::Receiver, pub cbor_rx_send: mpsc::Sender, - pub close_rx: mpsc::Receiver<()>, + pub teardown_rx: watch::Receiver, } impl TunnelConnectionInput { @@ -210,7 +212,7 @@ impl TunnelConnectionInput { known_device_store: Option>, cbor_tx_recv: mpsc::Receiver, cbor_rx_send: mpsc::Sender, - close_rx: mpsc::Receiver<()>, + teardown_rx: watch::Receiver, ) -> Self { Self { connection_type: handshake_output.connection_type, @@ -220,11 +222,34 @@ impl TunnelConnectionInput { noise_state: handshake_output.noise_state, cbor_tx_recv, cbor_rx_send, - close_rx, + teardown_rx, } } } +/// Waits for the next teardown intent. Every sender gone counts as a cancel, +/// since nobody is left to ask for a graceful close. +pub(crate) async fn next_teardown(teardown_rx: &mut watch::Receiver) -> Teardown { + match teardown_rx.changed().await { + Ok(()) => *teardown_rx.borrow_and_update(), + Err(_) => Teardown::Cancel, + } +} + +/// Drives the connect and handshake stages until they complete or the caller +/// tears the channel down. There is no secure channel yet, so any intent +/// simply drops the in-flight future. +pub(crate) async fn until_teardown( + fut: F, + teardown_rx: &mut watch::Receiver, +) -> Option { + tokio::select! { + biased; + _ = next_teardown(teardown_rx) => None, + output = fut => Some(output), + } +} + #[async_trait] pub(crate) trait UxUpdateSender: Send + Sync { async fn send_update(&self, update: CableUxUpdate); diff --git a/libwebauthn/src/transport/cable/known_devices.rs b/libwebauthn/src/transport/cable/known_devices.rs index b8132458..dc8e4473 100644 --- a/libwebauthn/src/transport/cable/known_devices.rs +++ b/libwebauthn/src/transport/cable/known_devices.rs @@ -4,9 +4,9 @@ use std::sync::Arc; use crate::transport::cable::channel::ConnectionState; use crate::transport::cable::connection_stages::{ - connection_stage, handshake_stage, proximity_check_stage, ConnectionInput, HandshakeInput, - HandshakeOutput, MpscUxUpdateSender, ProximityCheckInput, TunnelConnectionInput, - UxUpdateSender, + connection_stage, handshake_stage, proximity_check_stage, until_teardown, ConnectionInput, + HandshakeInput, HandshakeOutput, MpscUxUpdateSender, ProximityCheckInput, + TunnelConnectionInput, UxUpdateSender, }; use crate::transport::cable::error::CableError; @@ -24,6 +24,7 @@ use tokio::task; use tracing::{debug, instrument, trace}; use super::channel::CableChannel; +use super::linger::Teardown; use super::protocol::{self, CableLinkingInfo}; use super::Cable; @@ -201,9 +202,11 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { let (ux_update_sender, _) = broadcast::channel(16); let (cbor_tx_send, cbor_tx_recv) = mpsc::channel(16); let (cbor_rx_send, cbor_rx_recv) = mpsc::channel(16); - let (close_sender, close_rx) = mpsc::channel(1); let (connection_state_sender, connection_state_receiver) = watch::channel(ConnectionState::Connecting); + let (teardown_tx, teardown_rx) = watch::channel(Teardown::Active); + let teardown_tx = Arc::new(teardown_tx); + let mut teardown_rx_connect = teardown_rx.clone(); let ux_update_sender_clone = ux_update_sender.clone(); let known_device: CableKnownDevice = self.clone(); @@ -212,12 +215,21 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { let ux_sender = MpscUxUpdateSender::new(ux_update_sender_clone, connection_state_sender); - let handshake_output = match Self::connection(&known_device, &ux_sender).await { - Ok(handshake_output) => handshake_output, - Err(e) => { + let connecting = Self::connection(&known_device, &ux_sender); + let handshake_output = match until_teardown(connecting, &mut teardown_rx_connect).await + { + Some(Ok(handshake_output)) => handshake_output, + Some(Err(e)) => { ux_sender.send_error(e).await; return; } + None => { + debug!("Hybrid connection torn down before the handshake completed"); + ux_sender + .set_connection_state(ConnectionState::Terminated) + .await; + return; + } }; let tunnel_input = TunnelConnectionInput::from_handshake_output( @@ -225,10 +237,10 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { Some(known_device.store), cbor_tx_recv, cbor_rx_send, - close_rx, + teardown_rx, ); - match protocol::connection(tunnel_input).await { + match protocol::connection(tunnel_input, &ux_sender).await { Ok(()) => { ux_sender .set_connection_state(ConnectionState::Terminated) @@ -248,7 +260,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { ux_update_sender, connection_state_receiver, persistent_token_store: settings.persistent_token_store, - close_sender: Some(close_sender), + teardown: teardown_tx, }) } } diff --git a/libwebauthn/src/transport/cable/linger.rs b/libwebauthn/src/transport/cable/linger.rs new file mode 100644 index 00000000..cfed2c2b --- /dev/null +++ b/libwebauthn/src/transport/cable/linger.rs @@ -0,0 +1,14 @@ +//! Caller-driven teardown of hybrid connections. + +/// Caller to task teardown intent. One watch per connection, distinct from +/// [`ConnectionState`](super::channel::ConnectionState), which is the task to +/// caller phase. Cancel always wins and no intent is ever downgraded. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Teardown { + /// Running normally: connecting, or in the active loop. + Active, + /// Graceful: send Shutdown, then terminate. + Close, + /// Hard: no Shutdown, terminate now. + Cancel, +} diff --git a/libwebauthn/src/transport/cable/mod.rs b/libwebauthn/src/transport/cable/mod.rs index 5258344e..ed480bdb 100644 --- a/libwebauthn/src/transport/cable/mod.rs +++ b/libwebauthn/src/transport/cable/mod.rs @@ -4,6 +4,7 @@ mod crypto; mod data_channel; mod digit_encode; mod l2cap; +mod linger; mod protocol; pub mod advertisement; diff --git a/libwebauthn/src/transport/cable/protocol.rs b/libwebauthn/src/transport/cable/protocol.rs index 5b763477..49a51cb3 100644 --- a/libwebauthn/src/transport/cable/protocol.rs +++ b/libwebauthn/src/transport/cable/protocol.rs @@ -2,6 +2,7 @@ //! hybrid transport. Runs over any [`CableDataChannel`]. use std::collections::BTreeMap; use std::sync::Arc; +use std::time::Duration; use hmac::{Hmac, Mac}; use p256::{ecdh, NonZeroScalar}; @@ -20,14 +21,20 @@ use super::known_devices::ClientPayload; use super::known_devices::{CableKnownDeviceInfo, CableKnownDeviceInfoStore}; use crate::proto::ctap2::cbor::{self, CborRequest, CborResponse, Value}; use crate::proto::ctap2::{Ctap2CommandCode, Ctap2GetInfoResponse}; -use crate::transport::cable::connection_stages::TunnelConnectionInput; +use crate::transport::cable::connection_stages::{ + next_teardown, TunnelConnectionInput, UxUpdateSender, +}; use crate::transport::cable::error::CableError; use crate::transport::cable::known_devices::CableKnownDeviceId; +use crate::transport::cable::linger::Teardown; const P256_X962_LENGTH: usize = 65; const MAX_CBOR_SIZE: usize = 1024 * 1024; const PADDING_GRANULARITY: usize = 32; +/// Bounds every outbound send, so a dead socket cannot stall teardown. +const SEND_TIMEOUT: Duration = Duration::from_secs(5); + const CABLE_PROLOGUE_STATE_ASSISTED: &[u8] = &[0u8]; const CABLE_PROLOGUE_QR_INITIATED: &[u8] = &[1u8]; @@ -277,28 +284,61 @@ pub(crate) async fn do_handshake( /// Returns `Ok(())` on a clean close and `Err(_)` on any fault that leaves /// the encrypted channel unusable; callers surface `Err(_)` via `send_error`. -pub(crate) async fn connection(mut input: TunnelConnectionInput) -> Result<(), CableError> { - let get_info_response_serialized: Vec = match input.data_channel.recv().await { - Ok(Some(message)) => match connection_recv_initial(message, &mut input.noise_state).await { - Ok(initial) => initial, - Err(e) => { - error!(?e, "Failed to process initial message"); - return Err(e); - } - }, - Ok(None) => { - error!("Connection closed before initial message was received"); - return Err(CableError::ConnectionLost); - } - Err(e) => { - error!(?e, "Failed to read initial message"); - return Err(e); +pub(crate) async fn connection( + mut input: TunnelConnectionInput, + _ux_sender: &dyn UxUpdateSender, +) -> Result<(), CableError> { + // The secure channel exists, so a graceful teardown before the initial + // message still gets a courtesy Shutdown. + let get_info_response_serialized: Vec = loop { + tokio::select! { + biased; + intent = next_teardown(&mut input.teardown_rx) => match intent { + Teardown::Close => { + send_shutdown_bounded(&mut *input.data_channel, &mut input.noise_state).await; + return Ok(()); + } + Teardown::Cancel => return Ok(()), + Teardown::Active => continue, + }, + result = input.data_channel.recv() => match result { + Ok(Some(message)) => { + match connection_recv_initial(message, &mut input.noise_state).await { + Ok(initial) => break initial, + Err(e) => { + error!(?e, "Failed to process initial message"); + return Err(e); + } + } + } + Ok(None) => { + error!("Connection closed before initial message was received"); + return Err(CableError::ConnectionLost); + } + Err(e) => { + error!(?e, "Failed to read initial message"); + return Err(e); + } + }, } }; debug!(?get_info_response_serialized, "Received initial message"); loop { tokio::select! { + biased; + intent = next_teardown(&mut input.teardown_rx) => match intent { + Teardown::Close => { + debug!("Channel close requested, sending Shutdown control frame"); + send_shutdown_bounded(&mut *input.data_channel, &mut input.noise_state).await; + return Ok(()); + } + Teardown::Cancel => { + debug!("Channel cancelled, dropping the connection"); + return Ok(()); + } + Teardown::Active => {} + }, result = input.data_channel.recv() => { match result { Ok(Some(message)) => { @@ -332,15 +372,6 @@ pub(crate) async fn connection(mut input: TunnelConnectionInput) -> Result<(), C } } } - _ = input.close_rx.recv() => { - debug!("Channel close requested, sending Shutdown control frame"); - if let Err(e) = - connection_send_shutdown(&mut *input.data_channel, &mut input.noise_state).await - { - warn!(?e, "Failed to send Shutdown control frame on close"); - } - return Ok(()); - } Some(request) = input.cbor_tx_recv.recv() => { match request.command { // Optimisation: respond to GetInfo requests immediately with the cached response @@ -354,15 +385,21 @@ pub(crate) async fn connection(mut input: TunnelConnectionInput) -> Result<(), C } _ => { debug!(?request.command, "Sending CBOR request"); - if let Err(e) = connection_send( + let send = connection_send( request, &mut *input.data_channel, &mut input.noise_state, - ) - .await - { - error!(?e, "Fatal error sending CBOR request"); - return Err(e); + ); + match tokio::time::timeout(SEND_TIMEOUT, send).await { + Ok(Ok(())) => {} + Ok(Err(e)) => { + error!(?e, "Fatal error sending CBOR request"); + return Err(e); + } + Err(_) => { + error!("Timed out sending CBOR request"); + return Err(CableError::Timeout); + } } } } @@ -371,6 +408,20 @@ pub(crate) async fn connection(mut input: TunnelConnectionInput) -> Result<(), C } } +/// Best-effort Shutdown on a graceful teardown. A failure or timeout is +/// logged and otherwise ignored, since the connection is going away anyway. +async fn send_shutdown_bounded( + data_channel: &mut dyn CableDataChannel, + noise_state: &mut TunnelNoiseState, +) { + let send = connection_send_shutdown(data_channel, noise_state); + match tokio::time::timeout(SEND_TIMEOUT, send).await { + Ok(Ok(())) => {} + Ok(Err(e)) => warn!(?e, "Failed to send Shutdown control frame"), + Err(_) => warn!("Timed out sending Shutdown control frame"), + } +} + async fn connection_send( request: CborRequest, data_channel: &mut dyn CableDataChannel, @@ -869,7 +920,9 @@ mod tests { } use serde_indexed::SerializeIndexed; - use tokio::sync::mpsc; + use tokio::sync::{mpsc, watch}; + + use crate::transport::cable::channel::{CableUxUpdate, ConnectionState}; /// In-memory data channel: records outbound frames and replays queued inbound ones. struct TestDataChannel { @@ -889,6 +942,22 @@ mod tests { } } + /// Publishes connection states on a watch so tests can observe phases. + struct TestUxSender { + state_tx: watch::Sender, + } + + #[async_trait] + impl UxUpdateSender for TestUxSender { + async fn send_update(&self, _update: CableUxUpdate) {} + async fn send_error(&self, _error: CableError) { + let _ = self.state_tx.send(ConnectionState::Terminated); + } + async fn set_connection_state(&self, state: ConnectionState) { + let _ = self.state_tx.send(state); + } + } + /// Two Noise transport states that can encrypt/decrypt to each other. fn paired_transport_states() -> (TransportState, TransportState) { let mut initiator = Builder::new("Noise_NN_P256_AESGCM_SHA256".parse().unwrap()) @@ -964,96 +1033,199 @@ mod tests { *stripped.first().unwrap() } + /// The peer side of a post-handshake connection plus every caller-side handle. + struct Harness { + responder: TransportState, + inbound_tx: mpsc::UnboundedSender>, + outbound_rx: mpsc::UnboundedReceiver>, + cbor_tx_send: mpsc::Sender, + cbor_rx_recv: mpsc::Receiver, + teardown_tx: watch::Sender, + input: Option, + ux_sender: Option, + } + + impl Harness { + fn new(connection_type: CableTunnelConnectionType) -> Self { + let (initiator, responder) = paired_transport_states(); + let (inbound_tx, inbound_rx) = mpsc::unbounded_channel::>(); + let (outbound_tx, outbound_rx) = mpsc::unbounded_channel::>(); + let (cbor_tx_send, cbor_tx_recv) = mpsc::channel::(4); + let (cbor_rx_send, cbor_rx_recv) = mpsc::channel::(4); + let (teardown_tx, teardown_rx) = watch::channel(Teardown::Active); + let (state_tx, _state_rx) = watch::channel(ConnectionState::Connected); + let input = TunnelConnectionInput { + connection_type, + tunnel_domain: "cable.example.com".to_string(), + known_device_store: None, + data_channel: Box::new(TestDataChannel { + inbound: inbound_rx, + outbound: outbound_tx, + }), + noise_state: TunnelNoiseState { + transport_state: initiator, + handshake_hash: vec![0u8; 32], + }, + cbor_tx_recv, + cbor_rx_send, + teardown_rx, + }; + Self { + responder, + inbound_tx, + outbound_rx, + cbor_tx_send, + cbor_rx_recv, + teardown_tx, + input: Some(input), + ux_sender: Some(TestUxSender { state_tx }), + } + } + + fn qr() -> Self { + Self::new(qr_connection_type()) + } + + /// Queues the peer's initial message, as sent right after the handshake. + fn send_initial_message(&mut self) { + let frame = encrypted_initial_message(&mut self.responder); + self.inbound_tx.send(frame).unwrap(); + } + + fn send_peer_frame(&mut self, plaintext: Vec) { + let frame = encrypt(&mut self.responder, &pad(plaintext)); + self.inbound_tx.send(frame).unwrap(); + } + + /// Runs the connection loop on its own task. + fn spawn(&mut self) -> tokio::task::JoinHandle> { + let input = self.input.take().expect("spawned once"); + let ux_sender = self.ux_sender.take().expect("spawned once"); + tokio::spawn(async move { connection(input, &ux_sender).await }) + } + + fn teardown(&self, intent: Teardown) { + self.teardown_tx.send_replace(intent); + } + + async fn next_outbound_type(&mut self) -> u8 { + let frame = self.outbound_rx.recv().await.expect("an outbound frame"); + outbound_message_type(&frame, &mut self.responder) + } + } + #[tokio::test] async fn connection_sends_shutdown_on_close() { - let (initiator, mut responder) = paired_transport_states(); - let (inbound_tx, inbound_rx) = mpsc::unbounded_channel::>(); - let (outbound_tx, mut outbound_rx) = mpsc::unbounded_channel::>(); + let mut h = Harness::qr(); + h.send_initial_message(); + let handle = h.spawn(); + + h.teardown(Teardown::Close); + + assert_eq!( + h.next_outbound_type().await, + CableTunnelMessageType::Shutdown as u8 + ); + assert!(handle.await.unwrap().is_ok()); + assert!(h.outbound_rx.try_recv().is_err(), "exactly one Shutdown"); + } - inbound_tx - .send(encrypted_initial_message(&mut responder)) + #[tokio::test] + async fn cancel_sends_nothing() { + let mut h = Harness::qr(); + h.send_initial_message(); + let handle = h.spawn(); + + // Let the loop consume the initial message before cancelling. + h.cbor_tx_send + .send(CborRequest::new(Ctap2CommandCode::AuthenticatorGetInfo)) + .await .unwrap(); + h.cbor_rx_recv + .recv() + .await + .expect("cached GetInfo response"); - let (cbor_tx_send, cbor_tx_recv) = mpsc::channel::(4); - let (cbor_rx_send, cbor_rx_recv) = mpsc::channel::(4); - let (close_tx, close_rx) = mpsc::channel::<()>(1); - - let input = TunnelConnectionInput { - connection_type: qr_connection_type(), - tunnel_domain: "cable.example.com".to_string(), - known_device_store: None, - data_channel: Box::new(TestDataChannel { - inbound: inbound_rx, - outbound: outbound_tx, - }), - noise_state: TunnelNoiseState { - transport_state: initiator, - handshake_hash: vec![0u8; 32], - }, - cbor_tx_recv, - cbor_rx_send, - close_rx, - }; + h.teardown(Teardown::Cancel); - let handle = tokio::spawn(connection(input)); + assert!(handle.await.unwrap().is_ok()); + assert!(h.outbound_rx.try_recv().is_err(), "no Shutdown on cancel"); + } - close_tx.send(()).await.unwrap(); + #[tokio::test] + async fn close_before_initial_message_sends_shutdown() { + let mut h = Harness::qr(); + let handle = h.spawn(); + + // The peer never sends its initial message; the loop must still + // honour the close promptly and say goodbye. + h.teardown(Teardown::Close); - let frame = outbound_rx - .recv() - .await - .expect("a frame on the outbound path"); assert_eq!( - outbound_message_type(&frame, &mut responder), + h.next_outbound_type().await, CableTunnelMessageType::Shutdown as u8 ); assert!(handle.await.unwrap().is_ok()); + } - // Keep the channel ends alive until the loop has shut down. - drop((inbound_tx, cbor_tx_send, cbor_rx_recv, close_tx)); + #[tokio::test] + async fn cancel_before_initial_message_terminates_silently() { + let mut h = Harness::qr(); + let handle = h.spawn(); + + h.teardown(Teardown::Cancel); + + assert!(handle.await.unwrap().is_ok()); + assert!(h.outbound_rx.try_recv().is_err()); } #[tokio::test] - async fn peer_shutdown_ends_connection_cleanly() { - let (initiator, mut responder) = paired_transport_states(); - let (inbound_tx, inbound_rx) = mpsc::unbounded_channel::>(); - let (outbound_tx, mut outbound_rx) = mpsc::unbounded_channel::>(); + async fn every_sender_gone_counts_as_cancel() { + let mut h = Harness::qr(); + h.send_initial_message(); + let handle = h.spawn(); - inbound_tx - .send(encrypted_initial_message(&mut responder)) - .unwrap(); + drop(h.teardown_tx); + + assert!(handle.await.unwrap().is_ok()); + assert!(h.outbound_rx.try_recv().is_err()); + } + + #[tokio::test] + async fn peer_shutdown_ends_connection_cleanly() { + let mut h = Harness::qr(); + h.send_initial_message(); // A type-only Shutdown frame from the peer, padded like any other frame. - inbound_tx - .send(encrypt( - &mut responder, - &pad(vec![CableTunnelMessageType::Shutdown as u8]), - )) - .unwrap(); + h.send_peer_frame(vec![CableTunnelMessageType::Shutdown as u8]); + let handle = h.spawn(); - let (cbor_tx_send, cbor_tx_recv) = mpsc::channel::(4); - let (cbor_rx_send, cbor_rx_recv) = mpsc::channel::(4); - let (close_tx, close_rx) = mpsc::channel::<()>(1); - - let input = TunnelConnectionInput { - connection_type: qr_connection_type(), - tunnel_domain: "cable.example.com".to_string(), - known_device_store: None, - data_channel: Box::new(TestDataChannel { - inbound: inbound_rx, - outbound: outbound_tx, - }), - noise_state: TunnelNoiseState { - transport_state: initiator, - handshake_hash: vec![0u8; 32], - }, - cbor_tx_recv, - cbor_rx_send, - close_rx, - }; + assert!(handle.await.unwrap().is_ok()); + assert!( + h.outbound_rx.try_recv().is_err(), + "no frame is sent in reply" + ); + } - assert!(connection(input).await.is_ok()); - assert!(outbound_rx.try_recv().is_err(), "no frame is sent in reply"); + #[tokio::test] + async fn ctap_request_is_forwarded_and_response_delivered() { + let mut h = Harness::qr(); + h.send_initial_message(); + let handle = h.spawn(); - drop((inbound_tx, cbor_tx_send, cbor_rx_recv, close_tx)); + h.cbor_tx_send + .send(CborRequest::new(Ctap2CommandCode::AuthenticatorClientPin)) + .await + .unwrap(); + assert_eq!( + h.next_outbound_type().await, + CableTunnelMessageType::Ctap as u8 + ); + + // A CTAP response frame: [Ctap type byte][CTAP status OK]. + h.send_peer_frame(vec![CableTunnelMessageType::Ctap as u8, 0x00]); + h.cbor_rx_recv.recv().await.expect("a CTAP response"); + + h.teardown(Teardown::Cancel); + assert!(handle.await.unwrap().is_ok()); } } diff --git a/libwebauthn/src/transport/cable/qr_code_device.rs b/libwebauthn/src/transport/cable/qr_code_device.rs index bc523d09..ed5364f7 100644 --- a/libwebauthn/src/transport/cable/qr_code_device.rs +++ b/libwebauthn/src/transport/cable/qr_code_device.rs @@ -13,13 +13,14 @@ use serde_indexed::SerializeIndexed; use serde_repr::Serialize_repr; use tokio::sync::{broadcast, mpsc, watch}; use tokio::task; -use tracing::instrument; +use tracing::{debug, instrument}; use super::connection_stages::{ - connection_stage, handshake_stage, proximity_check_stage, ConnectionInput, HandshakeInput, - MpscUxUpdateSender, ProximityCheckInput, TunnelConnectionInput, UxUpdateSender, + connection_stage, handshake_stage, proximity_check_stage, until_teardown, ConnectionInput, + HandshakeInput, MpscUxUpdateSender, ProximityCheckInput, TunnelConnectionInput, UxUpdateSender, }; use super::known_devices::CableKnownDeviceInfoStore; +use super::linger::Teardown; use super::protocol; use super::tunnel::KNOWN_TUNNEL_DOMAINS; use super::{channel::CableChannel, channel::ConnectionState, Cable}; @@ -247,9 +248,11 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { let (ux_update_sender, _) = broadcast::channel(16); let (cbor_tx_send, cbor_tx_recv) = mpsc::channel(16); let (cbor_rx_send, cbor_rx_recv) = mpsc::channel(16); - let (close_sender, close_rx) = mpsc::channel(1); let (connection_state_sender, connection_state_receiver) = watch::channel(ConnectionState::Connecting); + let (teardown_tx, teardown_rx) = watch::channel(Teardown::Active); + let teardown_tx = Arc::new(teardown_tx); + let mut teardown_rx_connect = teardown_rx.clone(); let ux_update_sender_clone = ux_update_sender.clone(); let qr_device = self.clone(); @@ -258,12 +261,21 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { let ux_sender = MpscUxUpdateSender::new(ux_update_sender_clone.clone(), connection_state_sender); - let handshake_output = match Self::connection(&qr_device, &ux_sender).await { - Ok(handshake_output) => handshake_output, - Err(e) => { + let connecting = Self::connection(&qr_device, &ux_sender); + let handshake_output = match until_teardown(connecting, &mut teardown_rx_connect).await + { + Some(Ok(handshake_output)) => handshake_output, + Some(Err(e)) => { ux_sender.send_error(e).await; return; } + None => { + debug!("Hybrid connection torn down before the handshake completed"); + ux_sender + .set_connection_state(ConnectionState::Terminated) + .await; + return; + } }; let tunnel_input = TunnelConnectionInput::from_handshake_output( @@ -271,9 +283,9 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { qr_device.store, cbor_tx_recv, cbor_rx_send, - close_rx, + teardown_rx, ); - match protocol::connection(tunnel_input).await { + match protocol::connection(tunnel_input, &ux_sender).await { Ok(()) => { ux_sender .set_connection_state(ConnectionState::Terminated) @@ -293,7 +305,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { ux_update_sender, connection_state_receiver, persistent_token_store: settings.persistent_token_store, - close_sender: Some(close_sender), + teardown: teardown_tx, }) } diff --git a/libwebauthn/src/transport/channel.rs b/libwebauthn/src/transport/channel.rs index 461db570..a6367c13 100644 --- a/libwebauthn/src/transport/channel.rs +++ b/libwebauthn/src/transport/channel.rs @@ -71,8 +71,16 @@ pub trait Channel: Send + Sync + Display + Ctap2AuthTokenStore { &self, ) -> Result>; async fn status(&self) -> ChannelStatus; + + /// Graceful close. Returns once the channel has been torn down. async fn close(&mut self); + /// Hard abort without a protocol-level goodbye. Falls back to + /// [`close`](Self::close) on transports without a distinct hard path. + async fn cancel(&mut self) { + self.close().await + } + /// The transport this channel speaks over. Drives the registration response /// `transports` member and the `authenticatorAttachment` of both registration /// and assertion responses. From b425e7cf95b32e02efc9a9174f6687fbaf16a7eb Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 20:47:35 +0100 Subject: [PATCH 05/13] feat(cable): linger after Shutdown to capture a late linking update A state-assisted QR connection can keep receiving after the ceremony so the authenticator's linking update is stored for a later state-assisted reconnect. The caller opts in with a CableLingerConfig on ChannelSettings and calls linger() on the channel. The linger is detached from the channel, bounded by a configurable window under a hard cap, and tracked in a caller-owned CableLingerRegistry so a new connection evicts it and the caller can drain it on suspend. --- .../persistent_cred_management_hid.rs | 1 + libwebauthn/src/transport/cable/channel.rs | 54 ++ .../src/transport/cable/connection_stages.rs | 6 +- .../src/transport/cable/known_devices.rs | 9 + libwebauthn/src/transport/cable/linger.rs | 301 ++++++++- libwebauthn/src/transport/cable/mod.rs | 1 + libwebauthn/src/transport/cable/protocol.rs | 598 ++++++++++++++++-- .../src/transport/cable/qr_code_device.rs | 42 +- libwebauthn/src/transport/channel.rs | 4 + libwebauthn/src/transport/mod.rs | 1 + 10 files changed, 961 insertions(+), 56 deletions(-) diff --git a/libwebauthn/examples/management/persistent_cred_management_hid.rs b/libwebauthn/examples/management/persistent_cred_management_hid.rs index 3505721a..49b41983 100644 --- a/libwebauthn/examples/management/persistent_cred_management_hid.rs +++ b/libwebauthn/examples/management/persistent_cred_management_hid.rs @@ -42,6 +42,7 @@ pub async fn main() -> Result<(), WebAuthnError> { // token through it. The same settings apply to any transport. let settings = ChannelSettings { persistent_token_store: Some(store.clone()), + ..Default::default() }; let mut channel = device.channel(settings).await?; let state_recv = channel.get_ux_update_receiver(); diff --git a/libwebauthn/src/transport/cable/channel.rs b/libwebauthn/src/transport/cable/channel.rs index 2269a76d..a4465b8b 100644 --- a/libwebauthn/src/transport/cable/channel.rs +++ b/libwebauthn/src/transport/cable/channel.rs @@ -59,9 +59,35 @@ pub struct CableChannel { pub(crate) connection_state_receiver: watch::Receiver, pub(crate) persistent_token_store: Option>, pub(crate) teardown: Arc>, + pub(crate) linger_eligible: bool, } impl CableChannel { + /// Sends Shutdown, then keeps the connection open in the background to + /// capture a late linking update. Returns once the connection is + /// lingering, not when the window ends, and the channel can be dropped. + /// + /// Only a state-assisted QR connection opened with a + /// [`CableLingerConfig`](super::CableLingerConfig) can linger. Anything + /// else behaves like [`close`](Channel::close). + pub async fn linger(&mut self) { + if !self.linger_eligible { + return self.close().await; + } + self.request_teardown(Teardown::Linger); + if !self + .wait_for_state(CLOSE_FLUSH_TIMEOUT, |state| { + matches!( + state, + ConnectionState::Lingering | ConnectionState::Terminated + ) + }) + .await + { + warn!("Timed out waiting for the hybrid connection to start lingering"); + } + } + /// Sets the teardown intent if nobody has set one yet. Returns whether it did. fn request_teardown(&self, intent: Teardown) -> bool { self.teardown.send_if_modified(|current| { @@ -311,6 +337,7 @@ mod tests { connection_state_receiver, persistent_token_store: None, teardown: Arc::new(teardown), + linger_eligible: true, }; (channel, state_tx) } @@ -342,10 +369,37 @@ mod tests { connection_state_receiver, persistent_token_store: None, teardown: teardown.clone(), + linger_eligible: true, }; (channel, seen_rx, teardown) } + #[tokio::test] + async fn linger_requests_linger_on_an_eligible_channel() { + let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); + channel.linger().await; + assert_eq!(seen_rx.await.unwrap(), Teardown::Linger); + assert_eq!(*teardown.borrow(), Teardown::Linger); + } + + #[tokio::test] + async fn linger_degrades_to_close_on_an_ineligible_channel() { + let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); + channel.linger_eligible = false; + channel.linger().await; + assert_eq!(seen_rx.await.unwrap(), Teardown::Close); + assert_eq!(*teardown.borrow(), Teardown::Close); + } + + #[tokio::test] + async fn drop_after_linger_does_not_cancel() { + let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); + channel.linger().await; + drop(channel); + assert_eq!(*teardown.borrow(), Teardown::Linger); + assert_eq!(seen_rx.await.unwrap(), Teardown::Linger); + } + #[tokio::test] async fn close_requests_graceful_close_and_waits_for_termination() { let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); diff --git a/libwebauthn/src/transport/cable/connection_stages.rs b/libwebauthn/src/transport/cable/connection_stages.rs index e2eb8016..338bea09 100644 --- a/libwebauthn/src/transport/cable/connection_stages.rs +++ b/libwebauthn/src/transport/cable/connection_stages.rs @@ -9,7 +9,7 @@ use super::crypto::{derive, KeyPurpose}; use super::data_channel::{CableDataChannel, WebSocketDataChannel}; use super::known_devices::{CableKnownDevice, CableKnownDeviceInfoStore, ClientNonce}; use super::l2cap::L2capDataChannel; -use super::linger::Teardown; +use super::linger::{LingerParams, Teardown}; use super::protocol::{self, CableTunnelConnectionType, TunnelNoiseState}; use super::qr_code_device::CableQrCodeDevice; use super::tunnel; @@ -204,6 +204,8 @@ pub(crate) struct TunnelConnectionInput { pub cbor_tx_recv: mpsc::Receiver, pub cbor_rx_send: mpsc::Sender, pub teardown_rx: watch::Receiver, + /// Present only when this connection may linger after Shutdown. + pub linger: Option, } impl TunnelConnectionInput { @@ -213,6 +215,7 @@ impl TunnelConnectionInput { cbor_tx_recv: mpsc::Receiver, cbor_rx_send: mpsc::Sender, teardown_rx: watch::Receiver, + linger: Option, ) -> Self { Self { connection_type: handshake_output.connection_type, @@ -223,6 +226,7 @@ impl TunnelConnectionInput { cbor_tx_recv, cbor_rx_send, teardown_rx, + linger, } } } diff --git a/libwebauthn/src/transport/cable/known_devices.rs b/libwebauthn/src/transport/cable/known_devices.rs index dc8e4473..c27cf981 100644 --- a/libwebauthn/src/transport/cable/known_devices.rs +++ b/libwebauthn/src/transport/cable/known_devices.rs @@ -208,6 +208,13 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { let teardown_tx = Arc::new(teardown_tx); let mut teardown_rx_connect = teardown_rx.clone(); + // A new connection supersedes any connection still lingering. Known + // device connections never linger themselves: their linking update + // cannot be verified and is discarded. + if let Some(config) = &settings.cable_linger { + config.registry.close_lingering(); + } + let ux_update_sender_clone = ux_update_sender.clone(); let known_device: CableKnownDevice = self.clone(); @@ -238,6 +245,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { cbor_tx_recv, cbor_rx_send, teardown_rx, + None, ); match protocol::connection(tunnel_input, &ux_sender).await { @@ -261,6 +269,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableKnownDevice { connection_state_receiver, persistent_token_store: settings.persistent_token_store, teardown: teardown_tx, + linger_eligible: false, }) } } diff --git a/libwebauthn/src/transport/cable/linger.rs b/libwebauthn/src/transport/cable/linger.rs index cfed2c2b..138f8563 100644 --- a/libwebauthn/src/transport/cable/linger.rs +++ b/libwebauthn/src/transport/cable/linger.rs @@ -1,4 +1,30 @@ -//! Caller-driven teardown of hybrid connections. +//! Caller-driven teardown of hybrid connections, and the registry that tracks +//! connections left lingering for a late linking update. +//! +//! After a QR-initiated ceremony the authenticator may send its linking +//! information a while after the CTAP response. Capturing it needs the +//! connection to stay open after the caller is done with the channel. A +//! caller opts in by carrying a [`CableLingerConfig`] on its +//! [`ChannelSettings`](crate::transport::ChannelSettings) and calling +//! [`CableChannel::linger`](super::channel::CableChannel::linger) once the +//! ceremony has completed. The same [`CableLingerRegistry`] instance must be +//! threaded through every hybrid `channel()` call of one logical client: a +//! new connection evicts any connection still lingering, and the registry is +//! the only handle left once the channel has been dropped. + +use std::collections::BTreeMap; +use std::fmt; +use std::sync::{Arc, Mutex, Weak}; +use std::time::Duration; + +use tokio::sync::watch; + +/// Default linger window. The spec asks for at least two minutes after Shutdown. +pub const DEFAULT_LINGER: Duration = Duration::from_secs(120); +/// Absolute ceiling on a linger, whatever the configured window. Matches Chromium. +pub const HARD_CAP: Duration = Duration::from_secs(180); +/// Concurrent detached lingerers per registry. The oldest is evicted on overflow. +pub(crate) const MAX_LINGERING: usize = 8; /// Caller to task teardown intent. One watch per connection, distinct from /// [`ConnectionState`](super::channel::ConnectionState), which is the task to @@ -9,6 +35,279 @@ pub(crate) enum Teardown { Active, /// Graceful: send Shutdown, then terminate. Close, + /// Graceful: send Shutdown, then linger for a late linking update if eligible. + Linger, /// Hard: no Shutdown, terminate now. Cancel, } + +/// Opt-in to lingering, carried on +/// [`ChannelSettings::cable_linger`](crate::transport::ChannelSettings::cable_linger). +#[derive(Debug, Clone)] +pub struct CableLingerConfig { + /// Tracks lingering connections across ceremonies. Share one instance per client. + pub registry: CableLingerRegistry, + /// How long to keep receiving after Shutdown. Clamped to [`HARD_CAP`]. + pub linger_duration: Duration, +} + +impl CableLingerConfig { + pub fn new(registry: CableLingerRegistry) -> Self { + Self { + registry, + linger_duration: DEFAULT_LINGER, + } + } +} + +#[derive(Default)] +struct RegistryInner { + next_id: u64, + entries: BTreeMap>>, +} + +impl RegistryInner { + fn is_lingering(tx: &watch::Sender) -> bool { + *tx.borrow() == Teardown::Linger + } +} + +impl Drop for RegistryInner { + fn drop(&mut self) { + for tx in self.entries.values() { + tx.send_replace(Teardown::Cancel); + } + } +} + +/// Tracks hybrid connections from creation so that a lingering one can be +/// evicted after its channel is gone. Cheap to clone. The caller holds the +/// only strong reference: dropping the last clone cancels every lingerer. +#[derive(Clone, Default)] +pub struct CableLingerRegistry { + inner: Arc>, +} + +impl CableLingerRegistry { + pub fn new() -> Self { + Self::default() + } + + fn lock(&self) -> std::sync::MutexGuard<'_, RegistryInner> { + self.inner.lock().unwrap_or_else(|e| e.into_inner()) + } + + /// Connections currently lingering. Eventually consistent with the + /// natural expiry of a linger window. + pub fn lingering_count(&self) -> usize { + self.lock() + .entries + .values() + .filter(|tx| RegistryInner::is_lingering(tx)) + .count() + } + + /// Cancels every lingering connection. Returns how many were signalled. + /// Connections still connecting or in use are left alone. + pub fn close_lingering(&self) -> usize { + let inner = self.lock(); + let lingering: Vec<_> = inner + .entries + .values() + .filter(|tx| RegistryInner::is_lingering(tx)) + .collect(); + for tx in &lingering { + tx.send_replace(Teardown::Cancel); + } + lingering.len() + } + + /// Registers a connection at creation time. Over [`MAX_LINGERING`], the + /// oldest lingering connection is cancelled to make room. + pub(crate) fn register(&self, tx: Arc>) -> RegistryGuard { + let mut inner = self.lock(); + if inner.entries.len() >= MAX_LINGERING { + let oldest = inner + .entries + .iter() + .find(|(_, tx)| RegistryInner::is_lingering(tx)) + .map(|(id, _)| *id); + if let Some(id) = oldest { + if let Some(evicted) = inner.entries.remove(&id) { + evicted.send_replace(Teardown::Cancel); + } + } + } + let id = inner.next_id; + inner.next_id += 1; + inner.entries.insert(id, tx); + RegistryGuard { + inner: Arc::downgrade(&self.inner), + id, + } + } +} + +impl fmt::Debug for CableLingerRegistry { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("CableLingerRegistry") + .field("lingering_count", &self.lingering_count()) + .finish() + } +} + +/// Removes the connection from its registry when the connection task ends, +/// however it ends. Holds a weak reference so it never keeps the registry alive. +pub(crate) struct RegistryGuard { + inner: Weak>, + id: u64, +} + +impl Drop for RegistryGuard { + fn drop(&mut self) { + if let Some(inner) = self.inner.upgrade() { + inner + .lock() + .unwrap_or_else(|e| e.into_inner()) + .entries + .remove(&self.id); + } + } +} + +/// What the connection task needs to linger. Present only for connections +/// that are eligible and tracked by a registry. +pub(crate) struct LingerParams { + pub linger_duration: Duration, + #[allow(dead_code)] + pub guard: RegistryGuard, +} + +impl LingerParams { + /// Builds the linger parameters for a connection, registering it. `None` + /// when the caller did not opt in or the connection is not eligible. + pub(crate) fn new( + config: Option<&CableLingerConfig>, + eligible: bool, + tx: &Arc>, + ) -> Option { + let config = config?; + if !eligible { + return None; + } + Some(Self { + linger_duration: config.linger_duration.min(HARD_CAP), + guard: config.registry.register(tx.clone()), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn entry(registry: &CableLingerRegistry) -> (Arc>, RegistryGuard) { + let (tx, _rx) = watch::channel(Teardown::Active); + let tx = Arc::new(tx); + let guard = registry.register(tx.clone()); + (tx, guard) + } + + #[test] + fn close_lingering_cancels_only_lingerers() { + let registry = CableLingerRegistry::new(); + let (lingering, _g1) = entry(®istry); + let (active, _g2) = entry(®istry); + lingering.send_replace(Teardown::Linger); + + assert_eq!(registry.lingering_count(), 1); + assert_eq!(registry.close_lingering(), 1); + assert_eq!(*lingering.borrow(), Teardown::Cancel); + assert_eq!(*active.borrow(), Teardown::Active); + assert_eq!(registry.lingering_count(), 0); + } + + #[test] + fn guard_drop_deregisters() { + let registry = CableLingerRegistry::new(); + let (tx, guard) = entry(®istry); + tx.send_replace(Teardown::Linger); + assert_eq!(registry.lingering_count(), 1); + drop(guard); + assert_eq!(registry.lingering_count(), 0); + assert_eq!(registry.close_lingering(), 0); + } + + #[test] + fn overflow_evicts_the_oldest_lingerer() { + let registry = CableLingerRegistry::new(); + let mut entries = Vec::new(); + for _ in 0..MAX_LINGERING { + let (tx, guard) = entry(®istry); + tx.send_replace(Teardown::Linger); + entries.push((tx, guard)); + } + let (newest, _guard) = entry(®istry); + + assert_eq!(*entries[0].0.borrow(), Teardown::Cancel); + assert_eq!(*entries[1].0.borrow(), Teardown::Linger); + assert_eq!(*newest.borrow(), Teardown::Active); + assert_eq!(registry.lingering_count(), MAX_LINGERING - 1); + } + + #[test] + fn overflow_never_evicts_an_active_connection() { + let registry = CableLingerRegistry::new(); + let mut entries = Vec::new(); + for _ in 0..MAX_LINGERING { + entries.push(entry(®istry)); + } + let _newest = entry(®istry); + assert!(entries + .iter() + .all(|(tx, _)| *tx.borrow() == Teardown::Active)); + } + + #[test] + fn dropping_the_registry_cancels_everything() { + let registry = CableLingerRegistry::new(); + let (lingering, _g1) = entry(®istry); + let (active, _g2) = entry(®istry); + lingering.send_replace(Teardown::Linger); + + let clone = registry.clone(); + drop(registry); + assert_eq!( + *lingering.borrow(), + Teardown::Linger, + "a clone keeps it alive" + ); + drop(clone); + assert_eq!(*lingering.borrow(), Teardown::Cancel); + assert_eq!(*active.borrow(), Teardown::Cancel); + } + + #[test] + fn linger_params_require_opt_in_and_eligibility() { + let registry = CableLingerRegistry::new(); + let config = CableLingerConfig::new(registry.clone()); + let (tx, _rx) = watch::channel(Teardown::Active); + let tx = Arc::new(tx); + + assert!(LingerParams::new(None, true, &tx).is_none()); + assert!(LingerParams::new(Some(&config), false, &tx).is_none()); + let params = LingerParams::new(Some(&config), true, &tx).expect("eligible"); + assert_eq!(params.linger_duration, DEFAULT_LINGER); + tx.send_replace(Teardown::Linger); + assert_eq!(registry.lingering_count(), 1); + } + + #[test] + fn linger_duration_is_clamped_to_the_hard_cap() { + let mut config = CableLingerConfig::new(CableLingerRegistry::new()); + config.linger_duration = HARD_CAP * 2; + let (tx, _rx) = watch::channel(Teardown::Active); + let params = LingerParams::new(Some(&config), true, &Arc::new(tx)).expect("eligible"); + assert_eq!(params.linger_duration, HARD_CAP); + } +} diff --git a/libwebauthn/src/transport/cable/mod.rs b/libwebauthn/src/transport/cable/mod.rs index ed480bdb..c9fbe1bc 100644 --- a/libwebauthn/src/transport/cable/mod.rs +++ b/libwebauthn/src/transport/cable/mod.rs @@ -17,6 +17,7 @@ pub mod tunnel; use super::Transport; pub use digit_encode::digit_encode; +pub use linger::{CableLingerConfig, CableLingerRegistry, DEFAULT_LINGER, HARD_CAP}; /// Checks if the Cable/Hybrid transport is available on the system. /// Cable depends on a Bluetooth adapter for BLE advertisement discovery. diff --git a/libwebauthn/src/transport/cable/protocol.rs b/libwebauthn/src/transport/cable/protocol.rs index 49a51cb3..3de663ac 100644 --- a/libwebauthn/src/transport/cable/protocol.rs +++ b/libwebauthn/src/transport/cable/protocol.rs @@ -21,12 +21,13 @@ use super::known_devices::ClientPayload; use super::known_devices::{CableKnownDeviceInfo, CableKnownDeviceInfoStore}; use crate::proto::ctap2::cbor::{self, CborRequest, CborResponse, Value}; use crate::proto::ctap2::{Ctap2CommandCode, Ctap2GetInfoResponse}; +use crate::transport::cable::channel::ConnectionState; use crate::transport::cable::connection_stages::{ next_teardown, TunnelConnectionInput, UxUpdateSender, }; use crate::transport::cable::error::CableError; use crate::transport::cable::known_devices::CableKnownDeviceId; -use crate::transport::cable::linger::Teardown; +use crate::transport::cable::linger::{LingerParams, Teardown, HARD_CAP}; const P256_X962_LENGTH: usize = 65; const MAX_CBOR_SIZE: usize = 1024 * 1024; @@ -34,6 +35,12 @@ const PADDING_GRANULARITY: usize = 32; /// Bounds every outbound send, so a dead socket cannot stall teardown. const SEND_TIMEOUT: Duration = Duration::from_secs(5); +/// Poll granularity of the linger receive, so the deadline and teardown are re-checked. +const LINGER_RECV_POLL: Duration = Duration::from_secs(30); +/// Bounds the processing of one linger frame, including the caller's store write. +const STORE_WRITE_TIMEOUT: Duration = Duration::from_secs(5); +/// Consecutive undecryptable frames before a lingering connection gives up. +const DECRYPT_FAILURE_BUDGET: u32 = 3; const CABLE_PROLOGUE_STATE_ASSISTED: &[u8] = &[0u8]; const CABLE_PROLOGUE_QR_INITIATED: &[u8] = &[1u8]; @@ -286,7 +293,7 @@ pub(crate) async fn do_handshake( /// the encrypted channel unusable; callers surface `Err(_)` via `send_error`. pub(crate) async fn connection( mut input: TunnelConnectionInput, - _ux_sender: &dyn UxUpdateSender, + ux_sender: &dyn UxUpdateSender, ) -> Result<(), CableError> { // The secure channel exists, so a graceful teardown before the initial // message still gets a courtesy Shutdown. @@ -294,7 +301,7 @@ pub(crate) async fn connection( tokio::select! { biased; intent = next_teardown(&mut input.teardown_rx) => match intent { - Teardown::Close => { + Teardown::Close | Teardown::Linger => { send_shutdown_bounded(&mut *input.data_channel, &mut input.noise_state).await; return Ok(()); } @@ -333,6 +340,11 @@ pub(crate) async fn connection( send_shutdown_bounded(&mut *input.data_channel, &mut input.noise_state).await; return Ok(()); } + Teardown::Linger => { + debug!("Channel linger requested, sending Shutdown control frame"); + send_shutdown_bounded(&mut *input.data_channel, &mut input.noise_state).await; + break; + } Teardown::Cancel => { debug!("Channel cancelled, dropping the connection"); return Ok(()); @@ -406,6 +418,126 @@ pub(crate) async fn connection( } }; } + + // Only a QR-initiated connection with a store can use a linking update. + let eligible = matches!( + input.connection_type, + CableTunnelConnectionType::QrCode { .. } + ) && input.known_device_store.is_some(); + match input.linger.take() { + Some(params) if eligible => linger(input, params, ux_sender).await, + _ => {} + } + Ok(()) +} + +/// Outcome of one frame received while lingering. +enum LingerStep { + Keep, + PeerClosed, +} + +/// Keeps receiving after Shutdown to capture a late linking update. Detached +/// from the channel, so every await is bounded and the whole phase sits under +/// an absolute ceiling of [`HARD_CAP`]. +async fn linger( + mut input: TunnelConnectionInput, + params: LingerParams, + ux_sender: &dyn UxUpdateSender, +) { + ux_sender + .set_connection_state(ConnectionState::Lingering) + .await; + debug!(linger_duration = ?params.linger_duration, "Lingering for a late linking update"); + + let now = tokio::time::Instant::now(); + let deadline = now + params.linger_duration; + let hard_cap = now + HARD_CAP; + let mut decrypt_failures = 0u32; + + let run = async { + loop { + tokio::select! { + biased; + _ = next_teardown(&mut input.teardown_rx) => { + debug!("Linger cancelled"); + break; + } + _ = tokio::time::sleep_until(deadline) => { + debug!("Linger window elapsed"); + break; + } + received = tokio::time::timeout(LINGER_RECV_POLL, input.data_channel.recv()) => { + let frame = match received { + Err(_elapsed) => continue, + Ok(Ok(Some(frame))) => frame, + Ok(Ok(None)) | Ok(Err(_)) => { + debug!("Peer closed the connection while lingering"); + break; + } + }; + let step = linger_recv( + &input.connection_type, + &input.tunnel_domain, + &input.known_device_store, + frame, + &mut input.noise_state, + ); + match tokio::time::timeout(STORE_WRITE_TIMEOUT, step).await { + Ok(Ok(LingerStep::Keep)) => decrypt_failures = 0, + Ok(Ok(LingerStep::PeerClosed)) => break, + Ok(Err(e)) => { + decrypt_failures += 1; + warn!({ ?e, decrypt_failures }, "Undecodable frame while lingering"); + if decrypt_failures >= DECRYPT_FAILURE_BUDGET { + break; + } + } + Err(_elapsed) => { + warn!("Timed out processing a frame while lingering"); + break; + } + } + } + } + } + }; + if tokio::time::timeout_at(hard_cap, run).await.is_err() { + warn!("Linger hit the hard cap"); + } + // The registry guard drops with `params` here, deregistering the connection. + drop(params); +} + +/// Processes one frame received while lingering. Only a linking update has +/// any effect. Nothing is ever forwarded to the CBOR receiver. +async fn linger_recv( + connection_type: &CableTunnelConnectionType, + tunnel_domain: &str, + known_device_store: &Option>, + encrypted_frame: Vec, + noise_state: &mut TunnelNoiseState, +) -> Result { + let decrypted_frame = decrypt_frame(encrypted_frame, noise_state).await?; + let cable_message = CableTunnelMessage::from_slice(&decrypted_frame)?; + match cable_message.message_type { + CableTunnelMessageType::Shutdown => Ok(LingerStep::PeerClosed), + CableTunnelMessageType::Ctap => { + debug!("Ignoring CTAP frame while lingering"); + Ok(LingerStep::Keep) + } + CableTunnelMessageType::Update => { + handle_update_message( + connection_type, + tunnel_domain, + known_device_store, + &cable_message.payload, + &noise_state.handshake_hash, + ) + .await; + Ok(LingerStep::Keep) + } + } } /// Best-effort Shutdown on a graceful teardown. A failure or timeout is @@ -681,46 +813,64 @@ async fn connection_recv( Ok(RecvOutcome::Continue) } CableTunnelMessageType::Update => { - // Malformed or unsigned update: log, drop the update, keep the channel. - let maybe_update_message = match connection_recv_update(&cable_message.payload).await { - Ok(m) => m, - Err(e) => { - warn!(?e, "Malformed update message; ignoring"); - return Ok(RecvOutcome::Continue); - } - }; + handle_update_message( + connection_type, + tunnel_domain, + known_device_store, + &cable_message.payload, + &noise_state.handshake_hash, + ) + .await; + Ok(RecvOutcome::Continue) + } + } +} - let Some(linking_info) = maybe_update_message else { - warn!("Ignoring update message without linking info"); - return Ok(RecvOutcome::Continue); - }; +/// Applies a linking update to the store. Malformed, unsigned, non-QR or +/// store-less updates are logged and dropped without affecting the channel. +async fn handle_update_message( + connection_type: &CableTunnelConnectionType, + tunnel_domain: &str, + known_device_store: &Option>, + payload: &[u8], + handshake_hash: &[u8], +) { + let maybe_update_message = match connection_recv_update(payload).await { + Ok(m) => m, + Err(e) => { + warn!(?e, "Malformed update message; ignoring"); + return; + } + }; - let CableTunnelConnectionType::QrCode { private_key, .. } = connection_type else { - warn!("Ignoring update message for non-QR code connection"); - return Ok(RecvOutcome::Continue); - }; + let Some(linking_info) = maybe_update_message else { + warn!("Ignoring update message without linking info"); + return; + }; - debug!("Received update message with linking info"); - trace!(?linking_info); - - match known_device_store { - Some(store) => { - apply_linking_update( - store, - private_key, - tunnel_domain, - &linking_info, - &noise_state.handshake_hash, - ) - .await; - } - None => { - warn!("Ignoring update message without a device store"); - } - }; - Ok(RecvOutcome::Continue) + let CableTunnelConnectionType::QrCode { private_key, .. } = connection_type else { + warn!("Ignoring update message for non-QR code connection"); + return; + }; + + debug!("Received update message with linking info"); + trace!(?linking_info); + + match known_device_store { + Some(store) => { + apply_linking_update( + store, + private_key, + tunnel_domain, + &linking_info, + handshake_hash, + ) + .await; } - } + None => { + warn!("Ignoring update message without a device store"); + } + }; } /// Stores the update only on a valid signature; invalid updates are dropped without evicting. @@ -923,6 +1073,7 @@ mod tests { use tokio::sync::{mpsc, watch}; use crate::transport::cable::channel::{CableUxUpdate, ConnectionState}; + use crate::transport::cable::linger::{CableLingerRegistry, DEFAULT_LINGER}; /// In-memory data channel: records outbound frames and replays queued inbound ones. struct TestDataChannel { @@ -1020,11 +1171,102 @@ mod tests { } fn qr_connection_type() -> CableTunnelConnectionType { + qr_connection_type_with(NonZeroScalar::random(&mut OsRng)) + } + + fn qr_connection_type_with(private_key: NonZeroScalar) -> CableTunnelConnectionType { CableTunnelConnectionType::QrCode { routing_id: "000000".to_string(), tunnel_id: "00000000000000000000000000000000".to_string(), - private_key: NonZeroScalar::random(&mut OsRng), + private_key, + } + } + + fn known_device_connection_type() -> CableTunnelConnectionType { + CableTunnelConnectionType::KnownDevice { + contact_id: "contact".to_string(), + authenticator_public_key: vec![0u8; 65], + client_payload: ClientPayload { + link_id: ByteBuf::from(vec![0u8; 8]), + client_nonce: ByteBuf::from(vec![0u8; 16]), + hint: crate::transport::cable::known_devices::ClientPayloadHint::GetAssertion, + }, + } + } + + /// A linking update signed by a fresh authenticator key for the QR + /// private key of the connection. Returns the plaintext tunnel frame and + /// the known device id the store will see. + fn signed_update_payload( + qr_private_key: &NonZeroScalar, + handshake_hash: &[u8], + ) -> (Vec, CableKnownDeviceId) { + let authenticator_secret = SecretKey::random(&mut OsRng); + let authenticator_public_key = authenticator_secret + .public_key() + .to_encoded_point(false) + .as_bytes() + .to_vec(); + let shared_secret = ecdh::diffie_hellman( + qr_private_key, + authenticator_secret.public_key().as_affine(), + ) + .raw_secret_bytes() + .to_vec(); + let mut hmac = Hmac::::new_from_slice(&shared_secret).unwrap(); + hmac.update(handshake_hash); + let signature = hmac.finalize().into_bytes().to_vec(); + + let mut info = BTreeMap::new(); + info.insert(Value::Integer(1), Value::Bytes(vec![0u8; 4])); + info.insert(Value::Integer(2), Value::Bytes(vec![0u8; 8])); + info.insert(Value::Integer(3), Value::Bytes(vec![0u8; 32])); + info.insert( + Value::Integer(4), + Value::Bytes(authenticator_public_key.clone()), + ); + info.insert(Value::Integer(5), Value::Text("alice's phone".to_string())); + info.insert(Value::Integer(6), Value::Bytes(signature)); + let mut update = BTreeMap::new(); + update.insert(Value::Integer(1), Value::Map(info)); + + let mut payload = vec![CableTunnelMessageType::Update as u8]; + payload.extend(serde_cbor::to_vec(&Value::Map(update)).unwrap()); + (payload, hex::encode(&authenticator_public_key)) + } + + /// Reports every stored device on a channel so tests can await the write. + #[derive(Debug)] + struct NotifyingStore { + puts: mpsc::UnboundedSender, + } + + #[async_trait] + impl CableKnownDeviceInfoStore for NotifyingStore { + async fn put_known_device( + &self, + device_id: &CableKnownDeviceId, + _device: &CableKnownDeviceInfo, + ) { + let _ = self.puts.send(device_id.clone()); + } + async fn delete_known_device(&self, _device_id: &CableKnownDeviceId) {} + } + + /// A store whose writes never complete. + #[derive(Debug)] + struct WedgedStore; + + #[async_trait] + impl CableKnownDeviceInfoStore for WedgedStore { + async fn put_known_device( + &self, + _device_id: &CableKnownDeviceId, + _device: &CableKnownDeviceInfo, + ) { + std::future::pending::<()>().await; } + async fn delete_known_device(&self, _device_id: &CableKnownDeviceId) {} } /// Decrypts an outbound frame and returns its tunnel message type byte. @@ -1040,7 +1282,8 @@ mod tests { outbound_rx: mpsc::UnboundedReceiver>, cbor_tx_send: mpsc::Sender, cbor_rx_recv: mpsc::Receiver, - teardown_tx: watch::Sender, + teardown_tx: Arc>, + state_rx: watch::Receiver, input: Option, ux_sender: Option, } @@ -1053,7 +1296,7 @@ mod tests { let (cbor_tx_send, cbor_tx_recv) = mpsc::channel::(4); let (cbor_rx_send, cbor_rx_recv) = mpsc::channel::(4); let (teardown_tx, teardown_rx) = watch::channel(Teardown::Active); - let (state_tx, _state_rx) = watch::channel(ConnectionState::Connected); + let (state_tx, state_rx) = watch::channel(ConnectionState::Connected); let input = TunnelConnectionInput { connection_type, tunnel_domain: "cable.example.com".to_string(), @@ -1069,6 +1312,7 @@ mod tests { cbor_tx_recv, cbor_rx_send, teardown_rx, + linger: None, }; Self { responder, @@ -1076,7 +1320,8 @@ mod tests { outbound_rx, cbor_tx_send, cbor_rx_recv, - teardown_tx, + teardown_tx: Arc::new(teardown_tx), + state_rx, input: Some(input), ux_sender: Some(TestUxSender { state_tx }), } @@ -1086,6 +1331,60 @@ mod tests { Self::new(qr_connection_type()) } + fn input_mut(&mut self) -> &mut TunnelConnectionInput { + self.input.as_mut().expect("not spawned yet") + } + + fn with_store(mut self, store: Arc) -> Self { + self.input_mut().known_device_store = Some(store); + self + } + + /// Registers the connection with `registry` as an eligible lingerer. + fn with_linger( + mut self, + registry: &CableLingerRegistry, + linger_duration: Duration, + ) -> Self { + let guard = registry.register(self.teardown_tx.clone()); + self.input_mut().linger = Some(LingerParams { + linger_duration, + guard, + }); + self + } + + async fn wait_for_state(&mut self, state: ConnectionState) { + self.state_rx + .wait_for(|current| *current == state) + .await + .expect("state sender alive"); + } + + /// Round-trips a cached GetInfo, proving the loop has consumed the + /// initial message and is in its active phase. + async fn await_active(&mut self) { + self.cbor_tx_send + .send(CborRequest::new(Ctap2CommandCode::AuthenticatorGetInfo)) + .await + .unwrap(); + self.cbor_rx_recv + .recv() + .await + .expect("cached GetInfo response"); + } + + /// Sends the Linger intent from the active phase and waits for the + /// Shutdown that precedes the linger. + async fn start_linger(&mut self) { + self.await_active().await; + self.teardown(Teardown::Linger); + assert_eq!( + self.next_outbound_type().await, + CableTunnelMessageType::Shutdown as u8 + ); + } + /// Queues the peer's initial message, as sent right after the handshake. fn send_initial_message(&mut self) { let frame = encrypted_initial_message(&mut self.responder); @@ -1136,16 +1435,7 @@ mod tests { h.send_initial_message(); let handle = h.spawn(); - // Let the loop consume the initial message before cancelling. - h.cbor_tx_send - .send(CborRequest::new(Ctap2CommandCode::AuthenticatorGetInfo)) - .await - .unwrap(); - h.cbor_rx_recv - .recv() - .await - .expect("cached GetInfo response"); - + h.await_active().await; h.teardown(Teardown::Cancel); assert!(handle.await.unwrap().is_ok()); @@ -1228,4 +1518,206 @@ mod tests { h.teardown(Teardown::Cancel); assert!(handle.await.unwrap().is_ok()); } + + #[tokio::test(start_paused = true)] + async fn linger_captures_a_late_linking_update() { + let qr_private_key = NonZeroScalar::random(&mut OsRng); + let (puts_tx, mut puts_rx) = mpsc::unbounded_channel(); + let registry = CableLingerRegistry::new(); + let mut h = Harness::new(qr_connection_type_with(qr_private_key)) + .with_store(Arc::new(NotifyingStore { puts: puts_tx })) + .with_linger(®istry, DEFAULT_LINGER); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + h.wait_for_state(ConnectionState::Lingering).await; + + let (payload, device_id) = signed_update_payload(&qr_private_key, &[0u8; 32]); + h.send_peer_frame(payload); + assert_eq!(puts_rx.recv().await.unwrap(), device_id); + assert!( + h.cbor_rx_recv.try_recv().is_err(), + "nothing reaches the CBOR receiver" + ); + + h.teardown(Teardown::Cancel); + assert!(handle.await.unwrap().is_ok()); + assert_eq!(registry.lingering_count(), 0); + } + + #[tokio::test(start_paused = true)] + async fn linger_window_elapses() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::qr() + .with_store(Arc::new(RecordingStore::default())) + .with_linger(®istry, Duration::from_secs(10)); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + h.wait_for_state(ConnectionState::Lingering).await; + let started = tokio::time::Instant::now(); + + assert!(handle.await.unwrap().is_ok()); + assert_eq!(started.elapsed(), Duration::from_secs(10)); + assert_eq!(registry.lingering_count(), 0); + assert!(h.outbound_rx.try_recv().is_err(), "no second Shutdown"); + } + + #[tokio::test(start_paused = true)] + async fn hard_cap_bounds_an_overlong_window() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::qr() + .with_store(Arc::new(RecordingStore::default())) + .with_linger(®istry, HARD_CAP * 2); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + h.wait_for_state(ConnectionState::Lingering).await; + let started = tokio::time::Instant::now(); + + assert!(handle.await.unwrap().is_ok()); + assert_eq!(started.elapsed(), HARD_CAP); + } + + #[tokio::test(start_paused = true)] + async fn close_lingering_evicts_a_lingerer() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::qr() + .with_store(Arc::new(RecordingStore::default())) + .with_linger(®istry, DEFAULT_LINGER); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + h.wait_for_state(ConnectionState::Lingering).await; + assert_eq!(registry.lingering_count(), 1); + + let started = tokio::time::Instant::now(); + assert_eq!(registry.close_lingering(), 1); + assert!(handle.await.unwrap().is_ok()); + assert!(started.elapsed() < Duration::from_secs(1)); + assert_eq!(registry.lingering_count(), 0); + } + + #[tokio::test(start_paused = true)] + async fn wedged_store_write_is_preempted() { + let qr_private_key = NonZeroScalar::random(&mut OsRng); + let registry = CableLingerRegistry::new(); + let mut h = Harness::new(qr_connection_type_with(qr_private_key)) + .with_store(Arc::new(WedgedStore)) + .with_linger(®istry, DEFAULT_LINGER); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + h.wait_for_state(ConnectionState::Lingering).await; + let started = tokio::time::Instant::now(); + + let (payload, _) = signed_update_payload(&qr_private_key, &[0u8; 32]); + h.send_peer_frame(payload); + + assert!(handle.await.unwrap().is_ok()); + assert_eq!(started.elapsed(), STORE_WRITE_TIMEOUT); + } + + #[tokio::test(start_paused = true)] + async fn decrypt_failure_budget_ends_the_linger() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::qr() + .with_store(Arc::new(RecordingStore::default())) + .with_linger(®istry, DEFAULT_LINGER); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + h.wait_for_state(ConnectionState::Lingering).await; + + for _ in 0..DECRYPT_FAILURE_BUDGET { + h.inbound_tx.send(vec![0xFFu8; 48]).unwrap(); + } + let started = tokio::time::Instant::now(); + assert!(handle.await.unwrap().is_ok()); + assert!(started.elapsed() < Duration::from_secs(1)); + } + + #[tokio::test(start_paused = true)] + async fn peer_shutdown_ends_the_linger() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::qr() + .with_store(Arc::new(RecordingStore::default())) + .with_linger(®istry, DEFAULT_LINGER); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + h.wait_for_state(ConnectionState::Lingering).await; + + h.send_peer_frame(vec![CableTunnelMessageType::Shutdown as u8]); + let started = tokio::time::Instant::now(); + assert!(handle.await.unwrap().is_ok()); + assert!(started.elapsed() < Duration::from_secs(1)); + } + + #[tokio::test(start_paused = true)] + async fn ctap_frames_are_ignored_while_lingering() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::qr() + .with_store(Arc::new(RecordingStore::default())) + .with_linger(®istry, Duration::from_secs(10)); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + h.wait_for_state(ConnectionState::Lingering).await; + + h.send_peer_frame(vec![CableTunnelMessageType::Ctap as u8, 0x00]); + assert!(handle.await.unwrap().is_ok()); + assert!(h.cbor_rx_recv.try_recv().is_err()); + } + + #[tokio::test(start_paused = true)] + async fn linger_without_a_store_is_a_close() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::qr().with_linger(®istry, DEFAULT_LINGER); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + assert!(handle.await.unwrap().is_ok()); + assert_ne!(*h.state_rx.borrow(), ConnectionState::Lingering); + } + + #[tokio::test(start_paused = true)] + async fn known_device_connection_never_lingers() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::new(known_device_connection_type()) + .with_store(Arc::new(RecordingStore::default())) + .with_linger(®istry, DEFAULT_LINGER); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + assert!(handle.await.unwrap().is_ok()); + assert_ne!(*h.state_rx.borrow(), ConnectionState::Lingering); + } + + #[tokio::test] + async fn linger_before_the_initial_message_is_a_close() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::qr() + .with_store(Arc::new(RecordingStore::default())) + .with_linger(®istry, DEFAULT_LINGER); + let handle = h.spawn(); + + h.teardown(Teardown::Linger); + assert_eq!( + h.next_outbound_type().await, + CableTunnelMessageType::Shutdown as u8 + ); + assert!(handle.await.unwrap().is_ok()); + assert_ne!(*h.state_rx.borrow(), ConnectionState::Lingering); + } } diff --git a/libwebauthn/src/transport/cable/qr_code_device.rs b/libwebauthn/src/transport/cable/qr_code_device.rs index ed5364f7..15d10b17 100644 --- a/libwebauthn/src/transport/cable/qr_code_device.rs +++ b/libwebauthn/src/transport/cable/qr_code_device.rs @@ -20,7 +20,7 @@ use super::connection_stages::{ HandshakeInput, MpscUxUpdateSender, ProximityCheckInput, TunnelConnectionInput, UxUpdateSender, }; use super::known_devices::CableKnownDeviceInfoStore; -use super::linger::Teardown; +use super::linger::{LingerParams, Teardown}; use super::protocol; use super::tunnel::KNOWN_TUNNEL_DOMAINS; use super::{channel::CableChannel, channel::ConnectionState, Cable}; @@ -211,6 +211,11 @@ impl CableQrCodeDevice { Self::new(hint, false, None, transports) } + /// Only a state-assisted QR connection with a store can use a late linking update. + fn linger_eligible(&self) -> bool { + self.qr_code.state_assisted == Some(true) && self.store.is_some() + } + #[instrument(skip_all, err)] async fn connection( qr_device: &CableQrCodeDevice, @@ -254,6 +259,17 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { let teardown_tx = Arc::new(teardown_tx); let mut teardown_rx_connect = teardown_rx.clone(); + // A new connection supersedes any connection still lingering. + if let Some(config) = &settings.cable_linger { + config.registry.close_lingering(); + } + let linger = LingerParams::new( + settings.cable_linger.as_ref(), + self.linger_eligible(), + &teardown_tx, + ); + let linger_eligible = linger.is_some(); + let ux_update_sender_clone = ux_update_sender.clone(); let qr_device = self.clone(); @@ -284,6 +300,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { cbor_tx_recv, cbor_rx_send, teardown_rx, + linger, ); match protocol::connection(tunnel_input, &ux_sender).await { Ok(()) => { @@ -306,6 +323,7 @@ impl<'d> Device<'d, Cable, CableChannel> for CableQrCodeDevice { connection_state_receiver, persistent_token_store: settings.persistent_token_store, teardown: teardown_tx, + linger_eligible, }) } @@ -327,6 +345,28 @@ mod tests { assert_send_sync::(); }; + #[test] + fn transient_qr_code_is_not_linger_eligible() { + let device = CableQrCodeDevice::new_transient( + QrCodeOperationHint::GetAssertionRequest, + CableTransports::CloudAssistedOnly, + ) + .unwrap(); + assert!(!device.linger_eligible()); + } + + #[test] + fn persistent_qr_code_is_linger_eligible() { + let store = Arc::new(super::super::known_devices::EphemeralDeviceInfoStore::new()); + let device = CableQrCodeDevice::new_persistent( + QrCodeOperationHint::GetAssertionRequest, + store, + CableTransports::CloudAssistedOnly, + ) + .unwrap(); + assert!(device.linger_eligible()); + } + #[test] fn qr_code_omits_key_6_for_cloud_assisted_only() { let device = CableQrCodeDevice::new_transient( diff --git a/libwebauthn/src/transport/channel.rs b/libwebauthn/src/transport/channel.rs index a6367c13..697448bf 100644 --- a/libwebauthn/src/transport/channel.rs +++ b/libwebauthn/src/transport/channel.rs @@ -10,6 +10,7 @@ use crate::proto::{ ctap1::apdu::{ApduRequest, ApduResponse}, ctap2::cbor::{CborRequest, CborResponse}, }; +use crate::transport::cable::CableLingerConfig; use crate::webauthn::error::WebAuthnError; use crate::Transport; use crate::UvUpdate; @@ -37,6 +38,9 @@ pub struct ChannelSettings { /// credential management reuses a stored token across sessions instead of /// re-prompting for the PIN. See [`PersistentTokenStore`]. pub persistent_token_store: Option>, + /// Opt-in to keeping a hybrid connection open after the ceremony to capture + /// a late linking update. `None` disables it. See [`CableLingerConfig`]. + pub cable_linger: Option, } #[async_trait] diff --git a/libwebauthn/src/transport/mod.rs b/libwebauthn/src/transport/mod.rs index b4cc7014..e12d9633 100644 --- a/libwebauthn/src/transport/mod.rs +++ b/libwebauthn/src/transport/mod.rs @@ -36,6 +36,7 @@ mod channel; #[allow(clippy::module_inception)] mod transport; +pub use cable::{CableLingerConfig, CableLingerRegistry}; pub(crate) use channel::{AuthTokenData, Ctap2AuthTokenPermission}; pub use channel::{Channel, ChannelSettings, Ctap2AuthTokenStore}; From e10367db5d59c7ea3430e2e99c1c0cd3f68c3ca8 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 20:48:48 +0100 Subject: [PATCH 06/13] fix(cable): bound tunnel WebSocket message size The tunnel accepted messages up to the tungstenite default of 64 MiB before the protocol layer could reject them. Cap them at the CBOR bound plus framing overhead. --- libwebauthn/src/transport/cable/protocol.rs | 2 +- libwebauthn/src/transport/cable/tunnel.rs | 26 +++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/libwebauthn/src/transport/cable/protocol.rs b/libwebauthn/src/transport/cable/protocol.rs index 3de663ac..09c83ae6 100644 --- a/libwebauthn/src/transport/cable/protocol.rs +++ b/libwebauthn/src/transport/cable/protocol.rs @@ -30,7 +30,7 @@ use crate::transport::cable::known_devices::CableKnownDeviceId; use crate::transport::cable::linger::{LingerParams, Teardown, HARD_CAP}; const P256_X962_LENGTH: usize = 65; -const MAX_CBOR_SIZE: usize = 1024 * 1024; +pub(crate) const MAX_CBOR_SIZE: usize = 1024 * 1024; const PADDING_GRANULARITY: usize = 32; /// Bounds every outbound send, so a dead socket cannot stall teardown. diff --git a/libwebauthn/src/transport/cable/tunnel.rs b/libwebauthn/src/transport/cable/tunnel.rs index 528bcc66..2f135683 100644 --- a/libwebauthn/src/transport/cable/tunnel.rs +++ b/libwebauthn/src/transport/cable/tunnel.rs @@ -3,8 +3,9 @@ use sha2::{Digest, Sha256}; use tokio::net::TcpStream; use tokio_tungstenite::tungstenite::handshake::client::Request; use tokio_tungstenite::tungstenite::http::{header::LOCATION, StatusCode}; +use tokio_tungstenite::tungstenite::protocol::WebSocketConfig; use tokio_tungstenite::tungstenite::Error as TungsteniteError; -use tokio_tungstenite::{connect_async, MaybeTlsStream, WebSocketStream}; +use tokio_tungstenite::{connect_async_with_config, MaybeTlsStream, WebSocketStream}; use tracing::{debug, error, trace}; use tungstenite::client::IntoClientRequest; use url::Url; @@ -16,6 +17,16 @@ use crate::proto::ctap2::cbor; use crate::transport::cable::error::CableError; const MAX_TUNNEL_REDIRECTS: usize = 5; +/// Largest tunnel message accepted from the server: the CBOR bound plus the +/// type byte, padding and AEAD tag. Everything larger is rejected before it +/// is buffered. +const MAX_WS_MESSAGE_SIZE: usize = super::protocol::MAX_CBOR_SIZE + 64; + +fn websocket_config() -> WebSocketConfig { + WebSocketConfig::default() + .max_message_size(Some(MAX_WS_MESSAGE_SIZE)) + .max_frame_size(Some(MAX_WS_MESSAGE_SIZE)) +} fn ensure_rustls_crypto_provider() { use std::sync::Once; @@ -139,7 +150,8 @@ pub(crate) async fn connect( let request = build_tunnel_request(&connect_url, connection_type)?; trace!(?request); - let error = match connect_async(request).await { + let error = match connect_async_with_config(request, Some(websocket_config()), false).await + { Ok((ws_stream, response)) => { debug!(?response, "Connected to tunnel server"); if response.status() != StatusCode::SWITCHING_PROTOCOLS { @@ -188,6 +200,16 @@ mod tests { use super::*; use crate::transport::cable::known_devices::{ClientPayload, ClientPayloadHint}; use p256::NonZeroScalar; + + #[test] + fn websocket_config_bounds_message_and_frame_size() { + let config = websocket_config(); + assert_eq!(config.max_message_size, Some(MAX_WS_MESSAGE_SIZE)); + assert_eq!(config.max_frame_size, Some(MAX_WS_MESSAGE_SIZE)); + let default = WebSocketConfig::default(); + assert!(Some(MAX_WS_MESSAGE_SIZE) < default.max_message_size); + assert!(Some(MAX_WS_MESSAGE_SIZE) < default.max_frame_size); + } use rand::rngs::OsRng; use serde_bytes::ByteBuf; From 2d6c6a52ad514283d15be805286089936edbfbac Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 20:50:49 +0100 Subject: [PATCH 07/13] docs(cable): show linger and close-on-new in the examples --- README.md | 1 + .../examples/ceremony/webauthn_cable.rs | 2 ++ .../examples/ceremony/webauthn_cable_wss.rs | 34 ++++++++++++++----- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 7fbb3863..c1230d38 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,7 @@ _Looking for the D-Bus API proposal?_ Check out [credentialsd][credentialsd]. - 🟢 Discoverable credentials (resident keys) - 🟢 Hybrid transport (caBLE v2): QR-initiated transactions - 🟢 Hybrid transport (caBLE v2): State-assisted transactions (remember this phone) + - 🟢 Hybrid transport (caBLE v2): Linger after the ceremony to capture the linking update - 🟢 Hybrid transport (CTAP 2.3): direct BLE L2CAP data channel, QR-initiated, no tunnel server ## Runtime requirements diff --git a/libwebauthn/examples/ceremony/webauthn_cable.rs b/libwebauthn/examples/ceremony/webauthn_cable.rs index 975c3a67..48337f55 100644 --- a/libwebauthn/examples/ceremony/webauthn_cable.rs +++ b/libwebauthn/examples/ceremony/webauthn_cable.rs @@ -96,5 +96,7 @@ pub async fn main() -> Result<(), Box> { .expect("Failed to serialize MakeCredential response"); println!("WebAuthn MakeCredential response (JSON):\n{response_json}"); + // A transient QR code never lingers, so this is a plain graceful close. + channel.close().await; Ok(()) } diff --git a/libwebauthn/examples/ceremony/webauthn_cable_wss.rs b/libwebauthn/examples/ceremony/webauthn_cable_wss.rs index e9555583..50a689fd 100644 --- a/libwebauthn/examples/ceremony/webauthn_cable_wss.rs +++ b/libwebauthn/examples/ceremony/webauthn_cable_wss.rs @@ -2,13 +2,13 @@ use std::error::Error; use std::sync::Arc; use std::time::Duration; -use libwebauthn::transport::cable::is_available; use libwebauthn::transport::cable::known_devices::{ CableKnownDevice, ClientPayloadHint, EphemeralDeviceInfoStore, }; use libwebauthn::transport::cable::qr_code_device::{ CableQrCodeDevice, CableTransports, QrCodeOperationHint, }; +use libwebauthn::transport::cable::{is_available, CableLingerConfig, CableLingerRegistry}; use qrcode::render::unicode; use qrcode::QrCode; use tokio::time::sleep; @@ -68,6 +68,14 @@ pub async fn main() -> Result<(), Box> { } let device_info_store = Arc::new(EphemeralDeviceInfoStore::default()); + // One registry per client, threaded through every hybrid channel. It lets + // a connection keep receiving the linking update after the ceremony, and + // a new connection evict the one still lingering. + let linger_registry = CableLingerRegistry::new(); + let settings = || ChannelSettings { + cable_linger: Some(CableLingerConfig::new(linger_registry.clone())), + ..Default::default() + }; let request_origin: RequestOrigin = "https://example.org".try_into().expect("Invalid origin"); let psl = SystemPublicSuffixList::auto().expect( "PSL not available; install the publicsuffix-list (or publicsuffix-list-dafsa) package, or pass an explicit path", @@ -89,7 +97,7 @@ pub async fn main() -> Result<(), Box> { .build(); println!("{}", image); - let mut channel = device.channel(ChannelSettings::default()).await.unwrap(); + let mut channel = device.channel(settings()).await.unwrap(); println!("Channel established {:?}", channel); let state_recv = channel.get_ux_update_receiver(); @@ -113,10 +121,18 @@ pub async fn main() -> Result<(), Box> { .to_json_string(&request, JsonFormat::Prettified) .expect("Failed to serialize MakeCredential response"); println!("WebAuthn MakeCredential response (JSON):\n{response_json}"); + + // Say goodbye, then keep receiving in the background: the phone may + // send its linking information a while after the response. + channel.linger().await; } - println!("Waiting for 5 seconds before contacting the device..."); + println!("Waiting for 5 seconds for a linking update..."); sleep(Duration::from_secs(5)).await; + println!( + "Connections still lingering: {}", + linger_registry.lingering_count() + ); // Second leg: prefer state-assisted reconnection if the peer offered // linking info, otherwise fall back to a fresh QR. Many authenticators @@ -131,12 +147,11 @@ pub async fn main() -> Result<(), Box> { ) .await .unwrap(); - let mut channel = known_device - .channel(ChannelSettings::default()) - .await - .unwrap(); + // Opening this channel evicts the lingering QR connection. + let mut channel = known_device.channel(settings()).await.unwrap(); println!("Channel established {:?}", channel); run_get_assertion(&mut channel, &request_origin, &psl).await?; + channel.close().await; } else { println!("No known devices (peer did not offer linking). Falling back to QR."); let mut device: CableQrCodeDevice = CableQrCodeDevice::new_persistent( @@ -151,11 +166,14 @@ pub async fn main() -> Result<(), Box> { .light_color(unicode::Dense1x2::Dark) .build(); println!("{}", image); - let mut channel = device.channel(ChannelSettings::default()).await.unwrap(); + let mut channel = device.channel(settings()).await.unwrap(); println!("Channel established {:?}", channel); run_get_assertion(&mut channel, &request_origin, &psl).await?; + channel.linger().await; } + // Drain anything still lingering before the runtime goes away. + linger_registry.close_lingering(); Ok(()) } From 469b1a87812df60010fd6ab0870f8a460d8afb76 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 21:46:51 +0100 Subject: [PATCH 08/13] fix(cable): keep connections in use out of registry teardown Dropping the last registry clone cancelled every tracked connection, including one still connecting or mid-ceremony, so a registry built inline in the settings was gone by the time channel() returned and the connection died silently. Registry drop now cancels lingerers only, and a connection whose registry is gone by the time it would linger closes instead. Also bound the linking update store write in the active phase so a wedged store cannot hold off a close, and count only undecryptable frames toward the linger budget so unknown frame types are ignored as intended. --- libwebauthn/src/transport/cable/linger.rs | 78 +++++--- libwebauthn/src/transport/cable/mod.rs | 2 +- libwebauthn/src/transport/cable/protocol.rs | 189 ++++++++++++++++++-- 3 files changed, 228 insertions(+), 41 deletions(-) diff --git a/libwebauthn/src/transport/cable/linger.rs b/libwebauthn/src/transport/cable/linger.rs index 138f8563..89b43535 100644 --- a/libwebauthn/src/transport/cable/linger.rs +++ b/libwebauthn/src/transport/cable/linger.rs @@ -1,16 +1,5 @@ //! Caller-driven teardown of hybrid connections, and the registry that tracks //! connections left lingering for a late linking update. -//! -//! After a QR-initiated ceremony the authenticator may send its linking -//! information a while after the CTAP response. Capturing it needs the -//! connection to stay open after the caller is done with the channel. A -//! caller opts in by carrying a [`CableLingerConfig`] on its -//! [`ChannelSettings`](crate::transport::ChannelSettings) and calling -//! [`CableChannel::linger`](super::channel::CableChannel::linger) once the -//! ceremony has completed. The same [`CableLingerRegistry`] instance must be -//! threaded through every hybrid `channel()` call of one logical client: a -//! new connection evicts any connection still lingering, and the registry is -//! the only handle left once the channel has been dropped. use std::collections::BTreeMap; use std::fmt; @@ -19,10 +8,6 @@ use std::time::Duration; use tokio::sync::watch; -/// Default linger window. The spec asks for at least two minutes after Shutdown. -pub const DEFAULT_LINGER: Duration = Duration::from_secs(120); -/// Absolute ceiling on a linger, whatever the configured window. Matches Chromium. -pub const HARD_CAP: Duration = Duration::from_secs(180); /// Concurrent detached lingerers per registry. The oldest is evicted on overflow. pub(crate) const MAX_LINGERING: usize = 8; @@ -43,19 +28,35 @@ pub(crate) enum Teardown { /// Opt-in to lingering, carried on /// [`ChannelSettings::cable_linger`](crate::transport::ChannelSettings::cable_linger). +/// +/// After a QR-initiated ceremony the authenticator may send its linking +/// information a while after the CTAP response. Capturing it needs the +/// connection to stay open after the caller is done with the channel, which +/// only happens when the caller calls +/// [`CableChannel::linger`](super::channel::CableChannel::linger) once the +/// ceremony has completed. Closing or dropping the channel captures nothing. +/// +/// Carrying a config also makes opening a new hybrid channel evict any +/// connection still lingering in the same [`CableLingerRegistry`]. #[derive(Debug, Clone)] pub struct CableLingerConfig { - /// Tracks lingering connections across ceremonies. Share one instance per client. + /// Tracks lingering connections across ceremonies. Thread the same instance + /// through every hybrid `channel()` call of one logical client. pub registry: CableLingerRegistry, - /// How long to keep receiving after Shutdown. Clamped to [`HARD_CAP`]. + /// How long to keep receiving after Shutdown. Clamped to [`Self::HARD_CAP`]. pub linger_duration: Duration, } impl CableLingerConfig { + /// Default linger window. The spec asks for at least two minutes after Shutdown. + pub const DEFAULT_DURATION: Duration = Duration::from_secs(120); + /// Absolute ceiling on a linger, whatever the configured window. Matches Chromium. + pub const HARD_CAP: Duration = Duration::from_secs(180); + pub fn new(registry: CableLingerRegistry) -> Self { Self { registry, - linger_duration: DEFAULT_LINGER, + linger_duration: Self::DEFAULT_DURATION, } } } @@ -74,15 +75,23 @@ impl RegistryInner { impl Drop for RegistryInner { fn drop(&mut self) { - for tx in self.entries.values() { + // Connections still in use stay owned by their channel. + for tx in self.entries.values().filter(|tx| Self::is_lingering(tx)) { tx.send_replace(Teardown::Cancel); } } } /// Tracks hybrid connections from creation so that a lingering one can be -/// evicted after its channel is gone. Cheap to clone. The caller holds the -/// only strong reference: dropping the last clone cancels every lingerer. +/// evicted after its channel is gone. Cheap to clone. +/// +/// Close-on-new and eviction only apply to channels opened with the same +/// registry instance. A channel opened without it neither evicts nor can be +/// evicted, so use one registry per logical client, and independent registries +/// for independent concurrent clients. The caller holds the only strong +/// references: dropping the last clone cancels every connection that is +/// lingering, and a connection whose registry is gone by the time it would +/// linger closes instead. Connections still in use are never affected. #[derive(Clone, Default)] pub struct CableLingerRegistry { inner: Arc>, @@ -163,6 +172,14 @@ pub(crate) struct RegistryGuard { id: u64, } +impl RegistryGuard { + /// Whether the registry still exists. Without it a linger would be + /// untracked, so the connection closes instead. + pub(crate) fn is_live(&self) -> bool { + self.inner.strong_count() > 0 + } +} + impl Drop for RegistryGuard { fn drop(&mut self) { if let Some(inner) = self.inner.upgrade() { @@ -196,7 +213,7 @@ impl LingerParams { return None; } Some(Self { - linger_duration: config.linger_duration.min(HARD_CAP), + linger_duration: config.linger_duration.min(CableLingerConfig::HARD_CAP), guard: config.registry.register(tx.clone()), }) } @@ -269,10 +286,10 @@ mod tests { } #[test] - fn dropping_the_registry_cancels_everything() { + fn dropping_the_registry_cancels_lingerers_only() { let registry = CableLingerRegistry::new(); - let (lingering, _g1) = entry(®istry); - let (active, _g2) = entry(®istry); + let (lingering, g1) = entry(®istry); + let (active, g2) = entry(®istry); lingering.send_replace(Teardown::Linger); let clone = registry.clone(); @@ -282,9 +299,12 @@ mod tests { Teardown::Linger, "a clone keeps it alive" ); + assert!(g1.is_live()); drop(clone); assert_eq!(*lingering.borrow(), Teardown::Cancel); - assert_eq!(*active.borrow(), Teardown::Cancel); + assert_eq!(*active.borrow(), Teardown::Active); + assert!(!g1.is_live()); + assert!(!g2.is_live()); } #[test] @@ -297,7 +317,7 @@ mod tests { assert!(LingerParams::new(None, true, &tx).is_none()); assert!(LingerParams::new(Some(&config), false, &tx).is_none()); let params = LingerParams::new(Some(&config), true, &tx).expect("eligible"); - assert_eq!(params.linger_duration, DEFAULT_LINGER); + assert_eq!(params.linger_duration, CableLingerConfig::DEFAULT_DURATION); tx.send_replace(Teardown::Linger); assert_eq!(registry.lingering_count(), 1); } @@ -305,9 +325,9 @@ mod tests { #[test] fn linger_duration_is_clamped_to_the_hard_cap() { let mut config = CableLingerConfig::new(CableLingerRegistry::new()); - config.linger_duration = HARD_CAP * 2; + config.linger_duration = CableLingerConfig::HARD_CAP * 2; let (tx, _rx) = watch::channel(Teardown::Active); let params = LingerParams::new(Some(&config), true, &Arc::new(tx)).expect("eligible"); - assert_eq!(params.linger_duration, HARD_CAP); + assert_eq!(params.linger_duration, CableLingerConfig::HARD_CAP); } } diff --git a/libwebauthn/src/transport/cable/mod.rs b/libwebauthn/src/transport/cable/mod.rs index c9fbe1bc..7dc3bd96 100644 --- a/libwebauthn/src/transport/cable/mod.rs +++ b/libwebauthn/src/transport/cable/mod.rs @@ -17,7 +17,7 @@ pub mod tunnel; use super::Transport; pub use digit_encode::digit_encode; -pub use linger::{CableLingerConfig, CableLingerRegistry, DEFAULT_LINGER, HARD_CAP}; +pub use linger::{CableLingerConfig, CableLingerRegistry}; /// Checks if the Cable/Hybrid transport is available on the system. /// Cable depends on a Bluetooth adapter for BLE advertisement discovery. diff --git a/libwebauthn/src/transport/cable/protocol.rs b/libwebauthn/src/transport/cable/protocol.rs index 09c83ae6..a7b40d3b 100644 --- a/libwebauthn/src/transport/cable/protocol.rs +++ b/libwebauthn/src/transport/cable/protocol.rs @@ -27,10 +27,10 @@ use crate::transport::cable::connection_stages::{ }; use crate::transport::cable::error::CableError; use crate::transport::cable::known_devices::CableKnownDeviceId; -use crate::transport::cable::linger::{LingerParams, Teardown, HARD_CAP}; +use crate::transport::cable::linger::{CableLingerConfig, LingerParams, Teardown}; const P256_X962_LENGTH: usize = 65; -pub(crate) const MAX_CBOR_SIZE: usize = 1024 * 1024; +const MAX_CBOR_SIZE: usize = 1024 * 1024; const PADDING_GRANULARITY: usize = 32; /// Bounds every outbound send, so a dead socket cannot stall teardown. @@ -425,7 +425,9 @@ pub(crate) async fn connection( CableTunnelConnectionType::QrCode { .. } ) && input.known_device_store.is_some(); match input.linger.take() { - Some(params) if eligible => linger(input, params, ux_sender).await, + Some(params) if eligible && params.guard.is_live() => { + linger(input, params, ux_sender).await + } _ => {} } Ok(()) @@ -439,7 +441,7 @@ enum LingerStep { /// Keeps receiving after Shutdown to capture a late linking update. Detached /// from the channel, so every await is bounded and the whole phase sits under -/// an absolute ceiling of [`HARD_CAP`]. +/// an absolute ceiling of [`CableLingerConfig::HARD_CAP`]. async fn linger( mut input: TunnelConnectionInput, params: LingerParams, @@ -452,7 +454,7 @@ async fn linger( let now = tokio::time::Instant::now(); let deadline = now + params.linger_duration; - let hard_cap = now + HARD_CAP; + let hard_cap = now + CableLingerConfig::HARD_CAP; let mut decrypt_failures = 0u32; let run = async { @@ -486,13 +488,16 @@ async fn linger( match tokio::time::timeout(STORE_WRITE_TIMEOUT, step).await { Ok(Ok(LingerStep::Keep)) => decrypt_failures = 0, Ok(Ok(LingerStep::PeerClosed)) => break, - Ok(Err(e)) => { + // A desynced peer fails every following frame. Anything + // else that decrypts is merely ignored. + Ok(Err(CableError::EncryptionFailed)) => { decrypt_failures += 1; - warn!({ ?e, decrypt_failures }, "Undecodable frame while lingering"); + warn!(decrypt_failures, "Undecryptable frame while lingering"); if decrypt_failures >= DECRYPT_FAILURE_BUDGET { break; } } + Ok(Err(e)) => debug!(?e, "Ignoring undecodable frame while lingering"), Err(_elapsed) => { warn!("Timed out processing a frame while lingering"); break; @@ -813,14 +818,19 @@ async fn connection_recv( Ok(RecvOutcome::Continue) } CableTunnelMessageType::Update => { - handle_update_message( + let update = handle_update_message( connection_type, tunnel_domain, known_device_store, &cable_message.payload, &noise_state.handshake_hash, - ) - .await; + ); + if tokio::time::timeout(STORE_WRITE_TIMEOUT, update) + .await + .is_err() + { + warn!("Timed out storing a linking update; ignoring it"); + } Ok(RecvOutcome::Continue) } } @@ -1073,7 +1083,10 @@ mod tests { use tokio::sync::{mpsc, watch}; use crate::transport::cable::channel::{CableUxUpdate, ConnectionState}; - use crate::transport::cable::linger::{CableLingerRegistry, DEFAULT_LINGER}; + use crate::transport::cable::linger::CableLingerRegistry; + + const DEFAULT_LINGER: Duration = CableLingerConfig::DEFAULT_DURATION; + const HARD_CAP: Duration = CableLingerConfig::HARD_CAP; /// In-memory data channel: records outbound frames and replays queued inbound ones. struct TestDataChannel { @@ -1093,6 +1106,22 @@ mod tests { } } + /// A data channel whose sends never complete, like a stalled socket. + struct WedgedSendChannel { + inbound: mpsc::UnboundedReceiver>, + } + + #[async_trait] + impl CableDataChannel for WedgedSendChannel { + async fn send(&mut self, _message: &[u8]) -> Result<(), CableError> { + std::future::pending().await + } + + async fn recv(&mut self) -> Result>, CableError> { + Ok(self.inbound.recv().await) + } + } + /// Publishes connection states on a watch so tests can observe phases. struct TestUxSender { state_tx: watch::Sender, @@ -1340,6 +1369,16 @@ mod tests { self } + /// Replaces the data channel with one whose sends stall forever. + fn with_wedged_send(mut self) -> Self { + let (inbound_tx, inbound_rx) = mpsc::unbounded_channel::>(); + self.inbound_tx = inbound_tx; + self.input_mut().data_channel = Box::new(WedgedSendChannel { + inbound: inbound_rx, + }); + self + } + /// Registers the connection with `registry` as an eligible lingerer. fn with_linger( mut self, @@ -1419,6 +1458,7 @@ mod tests { h.send_initial_message(); let handle = h.spawn(); + h.await_active().await; h.teardown(Teardown::Close); assert_eq!( @@ -1673,8 +1713,10 @@ mod tests { h.start_linger().await; h.wait_for_state(ConnectionState::Lingering).await; + let started = tokio::time::Instant::now(); h.send_peer_frame(vec![CableTunnelMessageType::Ctap as u8, 0x00]); assert!(handle.await.unwrap().is_ok()); + assert_eq!(started.elapsed(), Duration::from_secs(10)); assert!(h.cbor_rx_recv.try_recv().is_err()); } @@ -1688,6 +1730,7 @@ mod tests { h.start_linger().await; assert!(handle.await.unwrap().is_ok()); assert_ne!(*h.state_rx.borrow(), ConnectionState::Lingering); + assert_eq!(registry.lingering_count(), 0); } #[tokio::test(start_paused = true)] @@ -1702,6 +1745,7 @@ mod tests { h.start_linger().await; assert!(handle.await.unwrap().is_ok()); assert_ne!(*h.state_rx.borrow(), ConnectionState::Lingering); + assert_eq!(registry.lingering_count(), 0); } #[tokio::test] @@ -1719,5 +1763,128 @@ mod tests { ); assert!(handle.await.unwrap().is_ok()); assert_ne!(*h.state_rx.borrow(), ConnectionState::Lingering); + assert_eq!(registry.lingering_count(), 0); + } + + #[tokio::test(start_paused = true)] + async fn wedged_store_write_in_the_active_phase_does_not_block_close() { + let qr_private_key = NonZeroScalar::random(&mut OsRng); + let mut h = + Harness::new(qr_connection_type_with(qr_private_key)).with_store(Arc::new(WedgedStore)); + h.send_initial_message(); + let handle = h.spawn(); + h.await_active().await; + + let (payload, _) = signed_update_payload(&qr_private_key, &[0u8; 32]); + h.send_peer_frame(payload); + tokio::task::yield_now().await; + let started = tokio::time::Instant::now(); + h.teardown(Teardown::Close); + + assert_eq!( + h.next_outbound_type().await, + CableTunnelMessageType::Shutdown as u8 + ); + assert!(handle.await.unwrap().is_ok()); + assert_eq!(started.elapsed(), STORE_WRITE_TIMEOUT); + } + + #[tokio::test(start_paused = true)] + async fn close_with_a_wedged_socket_terminates_at_send_timeout() { + let mut h = Harness::qr().with_wedged_send(); + h.send_initial_message(); + let handle = h.spawn(); + h.await_active().await; + + let started = tokio::time::Instant::now(); + h.teardown(Teardown::Close); + assert!(handle.await.unwrap().is_ok()); + assert_eq!(started.elapsed(), SEND_TIMEOUT); + } + + #[tokio::test(start_paused = true)] + async fn cbor_send_on_a_wedged_socket_fails_at_send_timeout() { + let mut h = Harness::qr().with_wedged_send(); + h.send_initial_message(); + let handle = h.spawn(); + h.await_active().await; + + let started = tokio::time::Instant::now(); + h.cbor_tx_send + .send(CborRequest::new(Ctap2CommandCode::AuthenticatorClientPin)) + .await + .unwrap(); + assert!(matches!(handle.await.unwrap(), Err(CableError::Timeout))); + assert_eq!(started.elapsed(), SEND_TIMEOUT); + } + + #[tokio::test(start_paused = true)] + async fn decrypt_failures_below_the_budget_keep_the_linger_alive() { + let qr_private_key = NonZeroScalar::random(&mut OsRng); + let (puts_tx, mut puts_rx) = mpsc::unbounded_channel(); + let registry = CableLingerRegistry::new(); + let mut h = Harness::new(qr_connection_type_with(qr_private_key)) + .with_store(Arc::new(NotifyingStore { puts: puts_tx })) + .with_linger(®istry, DEFAULT_LINGER); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + h.wait_for_state(ConnectionState::Lingering).await; + + for _ in 0..DECRYPT_FAILURE_BUDGET - 1 { + h.inbound_tx.send(vec![0xFFu8; 48]).unwrap(); + } + // A good frame resets the count and is still applied. + let (payload, device_id) = signed_update_payload(&qr_private_key, &[0u8; 32]); + h.send_peer_frame(payload); + assert_eq!(puts_rx.recv().await.unwrap(), device_id); + assert_eq!(registry.lingering_count(), 1); + + for _ in 0..DECRYPT_FAILURE_BUDGET - 1 { + h.inbound_tx.send(vec![0xFFu8; 48]).unwrap(); + } + tokio::task::yield_now().await; + assert_eq!(registry.lingering_count(), 1, "still under the budget"); + + h.inbound_tx.send(vec![0xFFu8; 48]).unwrap(); + assert!(handle.await.unwrap().is_ok()); + } + + #[tokio::test(start_paused = true)] + async fn unknown_frame_types_are_ignored_while_lingering() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::qr() + .with_store(Arc::new(RecordingStore::default())) + .with_linger(®istry, Duration::from_secs(10)); + h.send_initial_message(); + let handle = h.spawn(); + + h.start_linger().await; + h.wait_for_state(ConnectionState::Lingering).await; + + let started = tokio::time::Instant::now(); + for _ in 0..DECRYPT_FAILURE_BUDGET + 1 { + // Decrypts fine, unknown type byte (e.g. a CTAP 2.3 JSON frame). + h.send_peer_frame(vec![3, 0x00]); + } + assert!(handle.await.unwrap().is_ok()); + assert_eq!(started.elapsed(), Duration::from_secs(10)); + } + + #[tokio::test(start_paused = true)] + async fn linger_with_a_dropped_registry_is_a_close() { + let registry = CableLingerRegistry::new(); + let mut h = Harness::qr() + .with_store(Arc::new(RecordingStore::default())) + .with_linger(®istry, DEFAULT_LINGER); + h.send_initial_message(); + let handle = h.spawn(); + h.await_active().await; + + drop(registry); + h.start_linger().await; + assert!(handle.await.unwrap().is_ok()); + assert_ne!(*h.state_rx.borrow(), ConnectionState::Lingering); } } From 4e533ed439c8b2d8c011997859400ebfafd5a419 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 21:46:51 +0100 Subject: [PATCH 09/13] fix(cable): close() escalates on timeout and respects a linger A close that does not complete within the flush timeout now cancels the connection so close means closed. A close after linger() no longer waits out the flush timeout for a termination that will not come. --- libwebauthn/src/transport/cable/channel.rs | 103 ++++++++++++++++++++- libwebauthn/src/transport/channel.rs | 12 ++- 2 files changed, 108 insertions(+), 7 deletions(-) diff --git a/libwebauthn/src/transport/cable/channel.rs b/libwebauthn/src/transport/cable/channel.rs index a4465b8b..8f2f704a 100644 --- a/libwebauthn/src/transport/cable/channel.rs +++ b/libwebauthn/src/transport/cable/channel.rs @@ -69,7 +69,10 @@ impl CableChannel { /// /// Only a state-assisted QR connection opened with a /// [`CableLingerConfig`](super::CableLingerConfig) can linger. Anything - /// else behaves like [`close`](Channel::close). + /// else behaves like [`close`](Channel::close). The linger runs on the + /// tokio runtime that opened the channel, which must outlive the window. + /// Call [`CableLingerRegistry::close_lingering`](super::CableLingerRegistry::close_lingering) + /// before shutting that runtime down. pub async fn linger(&mut self) { if !self.linger_eligible { return self.close().await; @@ -210,16 +213,29 @@ impl Channel for CableChannel { } } - /// Sends Shutdown, then waits for the connection to terminate. Never lingers. + /// Sends Shutdown, then waits for the connection to terminate, cancelling + /// it if that takes too long. Never lingers, and never cuts short a + /// linger already requested. async fn close(&mut self) { self.request_teardown(Teardown::Close); + if *self.teardown.borrow() == Teardown::Linger { + self.wait_for_state(CLOSE_FLUSH_TIMEOUT, |state| { + matches!( + state, + ConnectionState::Lingering | ConnectionState::Terminated + ) + }) + .await; + return; + } if !self .wait_for_state(CLOSE_FLUSH_TIMEOUT, |state| { *state == ConnectionState::Terminated }) .await { - warn!("Timed out waiting for the hybrid connection to close"); + warn!("Timed out waiting for the hybrid connection to close, cancelling it"); + self.cancel().await; } } @@ -404,11 +420,11 @@ mod tests { async fn close_requests_graceful_close_and_waits_for_termination() { let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); channel.close().await; - assert_eq!(seen_rx.await.unwrap(), Teardown::Close); assert_eq!( *channel.connection_state_receiver.borrow(), ConnectionState::Terminated ); + assert_eq!(seen_rx.await.unwrap(), Teardown::Close); assert_eq!(*teardown.borrow(), Teardown::Close); assert!(matches!(channel.status().await, ChannelStatus::Closed)); } @@ -450,11 +466,90 @@ mod tests { #[tokio::test(start_paused = true)] async fn cancel_aborts_a_task_that_ignores_the_intent() { let (mut channel, _state_tx) = channel_in_state(ConnectionState::Connected); + let started = time::Instant::now(); channel.cancel().await; + assert_eq!(started.elapsed(), CANCEL_TIMEOUT); let joined = (&mut channel.handle_connection).await; assert!(joined.unwrap_err().is_cancelled()); } + #[tokio::test(start_paused = true)] + async fn close_escalates_to_cancel_after_the_flush_timeout() { + let (mut channel, _state_tx) = channel_in_state(ConnectionState::Connected); + let started = time::Instant::now(); + channel.close().await; + assert_eq!(started.elapsed(), CLOSE_FLUSH_TIMEOUT + CANCEL_TIMEOUT); + assert_eq!(*channel.teardown.borrow(), Teardown::Cancel); + let joined = (&mut channel.handle_connection).await; + assert!(joined.unwrap_err().is_cancelled()); + } + + #[tokio::test(start_paused = true)] + async fn linger_gives_up_after_the_flush_timeout_without_aborting() { + let (mut channel, _state_tx) = channel_in_state(ConnectionState::Connected); + let started = time::Instant::now(); + channel.linger().await; + assert_eq!(started.elapsed(), CLOSE_FLUSH_TIMEOUT); + assert_eq!(*channel.teardown.borrow(), Teardown::Linger); + assert!(!channel.handle_connection.is_finished()); + } + + /// A channel whose task publishes `Lingering` on the linger intent and + /// then stays alive, like the real linger phase. + fn channel_with_lingering_task() -> (CableChannel, task::AbortHandle) { + let (ux_update_sender, _) = broadcast::channel(1); + let (cbor_sender, _cbor_tx_recv) = mpsc::channel(1); + let (_cbor_rx_send, cbor_receiver) = mpsc::channel(1); + let (teardown, mut teardown_rx) = watch::channel(Teardown::Active); + let (state_tx, connection_state_receiver) = watch::channel(ConnectionState::Connected); + let handle_connection = task::spawn(async move { + let intent = super::super::connection_stages::next_teardown(&mut teardown_rx).await; + if intent == Teardown::Linger { + let _ = state_tx.send(ConnectionState::Lingering); + std::future::pending::<()>().await; + } + let _ = state_tx.send(ConnectionState::Terminated); + }); + let abort = handle_connection.abort_handle(); + let channel = CableChannel { + handle_connection, + cbor_sender, + cbor_receiver, + ux_update_sender, + connection_state_receiver, + persistent_token_store: None, + teardown: Arc::new(teardown), + linger_eligible: true, + }; + (channel, abort) + } + + #[tokio::test(start_paused = true)] + async fn linger_returns_once_lingering_and_survives_drop() { + let (mut channel, abort) = channel_with_lingering_task(); + let started = time::Instant::now(); + channel.linger().await; + assert_eq!(started.elapsed(), Duration::ZERO); + assert!(matches!(channel.status().await, ChannelStatus::Closed)); + + drop(channel); + task::yield_now().await; + assert!(!abort.is_finished(), "the linger outlives the channel"); + abort.abort(); + } + + #[tokio::test(start_paused = true)] + async fn close_after_linger_does_not_cut_the_linger_short() { + let (mut channel, abort) = channel_with_lingering_task(); + channel.linger().await; + let started = time::Instant::now(); + channel.close().await; + assert_eq!(started.elapsed(), Duration::ZERO); + assert_eq!(*channel.teardown.borrow(), Teardown::Linger); + assert!(!abort.is_finished()); + abort.abort(); + } + #[tokio::test] async fn wait_for_connection_rejects_lingering() { let (channel, _state_tx) = channel_in_state(ConnectionState::Lingering); diff --git a/libwebauthn/src/transport/channel.rs b/libwebauthn/src/transport/channel.rs index 697448bf..c871cf8d 100644 --- a/libwebauthn/src/transport/channel.rs +++ b/libwebauthn/src/transport/channel.rs @@ -39,7 +39,11 @@ pub struct ChannelSettings { /// re-prompting for the PIN. See [`PersistentTokenStore`]. pub persistent_token_store: Option>, /// Opt-in to keeping a hybrid connection open after the ceremony to capture - /// a late linking update. `None` disables it. See [`CableLingerConfig`]. + /// a late linking update. Enables close-on-new for this channel and lets + /// it linger when the caller calls + /// [`CableChannel::linger`](crate::transport::cable::channel::CableChannel::linger) + /// afterwards. Closing or dropping the channel captures nothing. `None` + /// disables it. Ignored by the other transports. pub cable_linger: Option, } @@ -76,11 +80,13 @@ pub trait Channel: Send + Sync + Display + Ctap2AuthTokenStore { ) -> Result>; async fn status(&self) -> ChannelStatus; - /// Graceful close. Returns once the channel has been torn down. + /// Graceful close. Hybrid sends its protocol-level goodbye and returns + /// once the connection has been torn down. HID, BLE and NFC release the + /// link when the channel is dropped, so this is a no-op there. async fn close(&mut self); /// Hard abort without a protocol-level goodbye. Falls back to - /// [`close`](Self::close) on transports without a distinct hard path. + /// [`close`](Self::close), so it is a no-op on HID, BLE and NFC. async fn cancel(&mut self) { self.close().await } From 98771b6de20bd406357592b9e562d193ba76e37c Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 21:46:51 +0100 Subject: [PATCH 10/13] fix(cable): bound tunnel messages at the Noise ceiling A Noise transport message is at most 65535 bytes, so anything larger could never be decrypted. Bound WebSocket messages there instead of at the CBOR size. --- libwebauthn/src/transport/cable/tunnel.rs | 41 +++++++++++++++++++---- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/libwebauthn/src/transport/cable/tunnel.rs b/libwebauthn/src/transport/cable/tunnel.rs index 2f135683..a64284e8 100644 --- a/libwebauthn/src/transport/cable/tunnel.rs +++ b/libwebauthn/src/transport/cable/tunnel.rs @@ -17,10 +17,10 @@ use crate::proto::ctap2::cbor; use crate::transport::cable::error::CableError; const MAX_TUNNEL_REDIRECTS: usize = 5; -/// Largest tunnel message accepted from the server: the CBOR bound plus the -/// type byte, padding and AEAD tag. Everything larger is rejected before it -/// is buffered. -const MAX_WS_MESSAGE_SIZE: usize = super::protocol::MAX_CBOR_SIZE + 64; +/// Largest tunnel message accepted from the server. A Noise transport message +/// is at most 65535 bytes, so nothing larger could be decrypted anyway. +/// Everything above is rejected before it is buffered. +const MAX_WS_MESSAGE_SIZE: usize = 65535; fn websocket_config() -> WebSocketConfig { WebSocketConfig::default() @@ -201,6 +201,9 @@ mod tests { use crate::transport::cable::known_devices::{ClientPayload, ClientPayloadHint}; use p256::NonZeroScalar; + use rand::rngs::OsRng; + use serde_bytes::ByteBuf; + #[test] fn websocket_config_bounds_message_and_frame_size() { let config = websocket_config(); @@ -210,8 +213,34 @@ mod tests { assert!(Some(MAX_WS_MESSAGE_SIZE) < default.max_message_size); assert!(Some(MAX_WS_MESSAGE_SIZE) < default.max_frame_size); } - use rand::rngs::OsRng; - use serde_bytes::ByteBuf; + + #[test] + fn websocket_bound_matches_the_noise_message_ceiling() { + // snow rejects transport messages above 65535 bytes, so the bound + // admits every decryptable frame and nothing more. + let mut initiator = snow::Builder::new("Noise_NN_P256_AESGCM_SHA256".parse().unwrap()) + .build_initiator() + .unwrap(); + let mut responder = snow::Builder::new("Noise_NN_P256_AESGCM_SHA256".parse().unwrap()) + .build_responder() + .unwrap(); + let mut a = [0u8; 1024]; + let mut b = [0u8; 1024]; + let n = initiator.write_message(&[], &mut a).unwrap(); + responder.read_message(&a[..n], &mut b).unwrap(); + let n = responder.write_message(&[], &mut a).unwrap(); + initiator.read_message(&a[..n], &mut b).unwrap(); + let mut responder = responder.into_transport_mode().unwrap(); + let mut out = vec![0u8; MAX_WS_MESSAGE_SIZE + 1]; + assert!(matches!( + responder.read_message(&vec![0u8; MAX_WS_MESSAGE_SIZE + 1], &mut out), + Err(snow::Error::Input) + )); + assert!(matches!( + responder.read_message(&vec![0u8; MAX_WS_MESSAGE_SIZE], &mut out), + Err(snow::Error::Decrypt) + )); + } fn known_device_connection_type(public_key: Vec) -> CableTunnelConnectionType { CableTunnelConnectionType::KnownDevice { From f1adc1721aa0c677219d132e6faccdaaf928fcaf Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 21:46:51 +0100 Subject: [PATCH 11/13] test(cable): cover the interrupted connect and the teardown fallback --- .../src/transport/cable/connection_stages.rs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/libwebauthn/src/transport/cable/connection_stages.rs b/libwebauthn/src/transport/cable/connection_stages.rs index 338bea09..3bbcbeb3 100644 --- a/libwebauthn/src/transport/cable/connection_stages.rs +++ b/libwebauthn/src/transport/cable/connection_stages.rs @@ -427,3 +427,38 @@ pub(crate) fn decode_tunnel_domain_from_advert( CableError::InvalidFraming }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test(start_paused = true)] + async fn until_teardown_drops_a_pending_connect_on_any_intent() { + let (tx, mut rx) = watch::channel(Teardown::Active); + let started = tokio::time::Instant::now(); + let connect = until_teardown(std::future::pending::<()>(), &mut rx); + tx.send_replace(Teardown::Close); + assert!(connect.await.is_none()); + assert_eq!(started.elapsed(), std::time::Duration::ZERO); + } + + #[tokio::test] + async fn until_teardown_yields_the_output_when_undisturbed() { + let (_tx, mut rx) = watch::channel(Teardown::Active); + assert_eq!(until_teardown(async { 7 }, &mut rx).await, Some(7)); + } + + #[tokio::test] + async fn until_teardown_prefers_an_intent_over_a_ready_connect() { + let (tx, mut rx) = watch::channel(Teardown::Active); + tx.send_replace(Teardown::Cancel); + assert_eq!(until_teardown(async { 7 }, &mut rx).await, None); + } + + #[tokio::test] + async fn next_teardown_treats_a_dropped_sender_as_cancel() { + let (tx, mut rx) = watch::channel(Teardown::Active); + drop(tx); + assert_eq!(next_teardown(&mut rx).await, Teardown::Cancel); + } +} From 64f4aa606249deb6230c9c9120c270701a3afa65 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 13 Sep 2026 21:46:51 +0100 Subject: [PATCH 12/13] docs(cable): close instead of linger in the example's fallback leg --- libwebauthn/examples/ceremony/webauthn_cable_wss.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libwebauthn/examples/ceremony/webauthn_cable_wss.rs b/libwebauthn/examples/ceremony/webauthn_cable_wss.rs index 50a689fd..b2348caf 100644 --- a/libwebauthn/examples/ceremony/webauthn_cable_wss.rs +++ b/libwebauthn/examples/ceremony/webauthn_cable_wss.rs @@ -169,10 +169,11 @@ pub async fn main() -> Result<(), Box> { let mut channel = device.channel(settings()).await.unwrap(); println!("Channel established {:?}", channel); run_get_assertion(&mut channel, &request_origin, &psl).await?; - channel.linger().await; + // Nothing follows that could use a linking update, so just close. + channel.close().await; } - // Drain anything still lingering before the runtime goes away. + // Signal any lingering connection to stop before the runtime goes away. linger_registry.close_lingering(); Ok(()) } From f746e4dc7897c6310e0b2997c75aedbcc18f5121 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Mon, 14 Sep 2026 09:07:53 +0100 Subject: [PATCH 13/13] refactor(cable): close takes a mode instead of a separate linger method CableChannel::close(CableClose::Immediate) sends Shutdown and waits for termination, CableClose::Linger keeps receiving for a late linking update. The trait close() maps to Immediate. --- .../examples/ceremony/webauthn_cable.rs | 4 +- .../examples/ceremony/webauthn_cable_wss.rs | 10 +- libwebauthn/src/transport/cable/channel.rs | 108 +++++++++++------- libwebauthn/src/transport/cable/linger.rs | 6 +- libwebauthn/src/transport/cable/mod.rs | 1 + libwebauthn/src/transport/channel.rs | 12 +- libwebauthn/src/transport/mod.rs | 2 +- 7 files changed, 84 insertions(+), 59 deletions(-) diff --git a/libwebauthn/examples/ceremony/webauthn_cable.rs b/libwebauthn/examples/ceremony/webauthn_cable.rs index 48337f55..8b499c0e 100644 --- a/libwebauthn/examples/ceremony/webauthn_cable.rs +++ b/libwebauthn/examples/ceremony/webauthn_cable.rs @@ -3,10 +3,10 @@ //! MakeCredential only. use std::error::Error; -use libwebauthn::transport::cable::is_available; use libwebauthn::transport::cable::qr_code_device::{ CableQrCodeDevice, CableTransports, QrCodeOperationHint, }; +use libwebauthn::transport::cable::{is_available, CableClose}; use qrcode::render::unicode; use qrcode::QrCode; @@ -97,6 +97,6 @@ pub async fn main() -> Result<(), Box> { println!("WebAuthn MakeCredential response (JSON):\n{response_json}"); // A transient QR code never lingers, so this is a plain graceful close. - channel.close().await; + channel.close(CableClose::Immediate).await; Ok(()) } diff --git a/libwebauthn/examples/ceremony/webauthn_cable_wss.rs b/libwebauthn/examples/ceremony/webauthn_cable_wss.rs index b2348caf..458ba3be 100644 --- a/libwebauthn/examples/ceremony/webauthn_cable_wss.rs +++ b/libwebauthn/examples/ceremony/webauthn_cable_wss.rs @@ -8,7 +8,9 @@ use libwebauthn::transport::cable::known_devices::{ use libwebauthn::transport::cable::qr_code_device::{ CableQrCodeDevice, CableTransports, QrCodeOperationHint, }; -use libwebauthn::transport::cable::{is_available, CableLingerConfig, CableLingerRegistry}; +use libwebauthn::transport::cable::{ + is_available, CableClose, CableLingerConfig, CableLingerRegistry, +}; use qrcode::render::unicode; use qrcode::QrCode; use tokio::time::sleep; @@ -124,7 +126,7 @@ pub async fn main() -> Result<(), Box> { // Say goodbye, then keep receiving in the background: the phone may // send its linking information a while after the response. - channel.linger().await; + channel.close(CableClose::Linger).await; } println!("Waiting for 5 seconds for a linking update..."); @@ -151,7 +153,7 @@ pub async fn main() -> Result<(), Box> { let mut channel = known_device.channel(settings()).await.unwrap(); println!("Channel established {:?}", channel); run_get_assertion(&mut channel, &request_origin, &psl).await?; - channel.close().await; + channel.close(CableClose::Immediate).await; } else { println!("No known devices (peer did not offer linking). Falling back to QR."); let mut device: CableQrCodeDevice = CableQrCodeDevice::new_persistent( @@ -170,7 +172,7 @@ pub async fn main() -> Result<(), Box> { println!("Channel established {:?}", channel); run_get_assertion(&mut channel, &request_origin, &psl).await?; // Nothing follows that could use a linking update, so just close. - channel.close().await; + channel.close(CableClose::Immediate).await; } // Signal any lingering connection to stop before the runtime goes away. diff --git a/libwebauthn/src/transport/cable/channel.rs b/libwebauthn/src/transport/cable/channel.rs index 8f2f704a..d3f488b6 100644 --- a/libwebauthn/src/transport/cable/channel.rs +++ b/libwebauthn/src/transport/cable/channel.rs @@ -44,6 +44,15 @@ pub enum ConnectionState { Terminated, } +/// How [`CableChannel::close`] ends the connection. Both send Shutdown first. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CableClose { + /// Terminate as soon as Shutdown has been sent. + Immediate, + /// Keep receiving in the background to capture a late linking update. + Linger, +} + #[derive(Debug)] pub enum CableChannelDevice<'d> { QrCode(&'d CableQrCodeDevice), @@ -63,20 +72,53 @@ pub struct CableChannel { } impl CableChannel { - /// Sends Shutdown, then keeps the connection open in the background to - /// capture a late linking update. Returns once the connection is - /// lingering, not when the window ends, and the channel can be dropped. + /// Sends Shutdown, then ends the connection as `mode` says. The + /// [`Channel::close`] of this channel is [`CableClose::Immediate`]. + /// + /// [`CableClose::Immediate`] waits for the connection to terminate, + /// cancelling it if that takes too long. A linger already under way is + /// left alone. /// - /// Only a state-assisted QR connection opened with a - /// [`CableLingerConfig`](super::CableLingerConfig) can linger. Anything - /// else behaves like [`close`](Channel::close). The linger runs on the - /// tokio runtime that opened the channel, which must outlive the window. - /// Call [`CableLingerRegistry::close_lingering`](super::CableLingerRegistry::close_lingering) + /// [`CableClose::Linger`] returns once the connection is lingering, not + /// when the window ends, and the channel can then be dropped. Only a + /// state-assisted QR connection opened with a + /// [`CableLingerConfig`](super::CableLingerConfig) can linger, anything + /// else closes immediately. The linger runs on the tokio runtime that + /// opened the channel, which must outlive the window. Call + /// [`CableLingerRegistry::close_lingering`](super::CableLingerRegistry::close_lingering) /// before shutting that runtime down. - pub async fn linger(&mut self) { - if !self.linger_eligible { - return self.close().await; + pub async fn close(&mut self, mode: CableClose) { + match mode { + CableClose::Immediate => self.close_immediately().await, + CableClose::Linger if self.linger_eligible => self.linger().await, + CableClose::Linger => self.close_immediately().await, + } + } + + async fn close_immediately(&mut self) { + self.request_teardown(Teardown::Close); + if *self.teardown.borrow() == Teardown::Linger { + self.wait_for_state(CLOSE_FLUSH_TIMEOUT, |state| { + matches!( + state, + ConnectionState::Lingering | ConnectionState::Terminated + ) + }) + .await; + return; } + if !self + .wait_for_state(CLOSE_FLUSH_TIMEOUT, |state| { + *state == ConnectionState::Terminated + }) + .await + { + warn!("Timed out waiting for the hybrid connection to close, cancelling it"); + self.cancel().await; + } + } + + async fn linger(&mut self) { self.request_teardown(Teardown::Linger); if !self .wait_for_state(CLOSE_FLUSH_TIMEOUT, |state| { @@ -213,30 +255,8 @@ impl Channel for CableChannel { } } - /// Sends Shutdown, then waits for the connection to terminate, cancelling - /// it if that takes too long. Never lingers, and never cuts short a - /// linger already requested. async fn close(&mut self) { - self.request_teardown(Teardown::Close); - if *self.teardown.borrow() == Teardown::Linger { - self.wait_for_state(CLOSE_FLUSH_TIMEOUT, |state| { - matches!( - state, - ConnectionState::Lingering | ConnectionState::Terminated - ) - }) - .await; - return; - } - if !self - .wait_for_state(CLOSE_FLUSH_TIMEOUT, |state| { - *state == ConnectionState::Terminated - }) - .await - { - warn!("Timed out waiting for the hybrid connection to close, cancelling it"); - self.cancel().await; - } + CableChannel::close(self, CableClose::Immediate).await } /// Drops the connection without sending Shutdown. Always wins over a @@ -393,7 +413,7 @@ mod tests { #[tokio::test] async fn linger_requests_linger_on_an_eligible_channel() { let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); - channel.linger().await; + channel.close(CableClose::Linger).await; assert_eq!(seen_rx.await.unwrap(), Teardown::Linger); assert_eq!(*teardown.borrow(), Teardown::Linger); } @@ -402,7 +422,7 @@ mod tests { async fn linger_degrades_to_close_on_an_ineligible_channel() { let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); channel.linger_eligible = false; - channel.linger().await; + channel.close(CableClose::Linger).await; assert_eq!(seen_rx.await.unwrap(), Teardown::Close); assert_eq!(*teardown.borrow(), Teardown::Close); } @@ -410,7 +430,7 @@ mod tests { #[tokio::test] async fn drop_after_linger_does_not_cancel() { let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); - channel.linger().await; + channel.close(CableClose::Linger).await; drop(channel); assert_eq!(*teardown.borrow(), Teardown::Linger); assert_eq!(seen_rx.await.unwrap(), Teardown::Linger); @@ -419,7 +439,7 @@ mod tests { #[tokio::test] async fn close_requests_graceful_close_and_waits_for_termination() { let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); - channel.close().await; + channel.close(CableClose::Immediate).await; assert_eq!( *channel.connection_state_receiver.borrow(), ConnectionState::Terminated @@ -457,7 +477,7 @@ mod tests { #[tokio::test] async fn drop_after_close_does_not_downgrade_the_intent() { let (mut channel, seen_rx, teardown) = channel_with_teardown_task(); - channel.close().await; + channel.close(CableClose::Immediate).await; drop(channel); assert_eq!(*teardown.borrow(), Teardown::Close); assert_eq!(seen_rx.await.unwrap(), Teardown::Close); @@ -477,7 +497,7 @@ mod tests { async fn close_escalates_to_cancel_after_the_flush_timeout() { let (mut channel, _state_tx) = channel_in_state(ConnectionState::Connected); let started = time::Instant::now(); - channel.close().await; + channel.close(CableClose::Immediate).await; assert_eq!(started.elapsed(), CLOSE_FLUSH_TIMEOUT + CANCEL_TIMEOUT); assert_eq!(*channel.teardown.borrow(), Teardown::Cancel); let joined = (&mut channel.handle_connection).await; @@ -488,7 +508,7 @@ mod tests { async fn linger_gives_up_after_the_flush_timeout_without_aborting() { let (mut channel, _state_tx) = channel_in_state(ConnectionState::Connected); let started = time::Instant::now(); - channel.linger().await; + channel.close(CableClose::Linger).await; assert_eq!(started.elapsed(), CLOSE_FLUSH_TIMEOUT); assert_eq!(*channel.teardown.borrow(), Teardown::Linger); assert!(!channel.handle_connection.is_finished()); @@ -528,7 +548,7 @@ mod tests { async fn linger_returns_once_lingering_and_survives_drop() { let (mut channel, abort) = channel_with_lingering_task(); let started = time::Instant::now(); - channel.linger().await; + channel.close(CableClose::Linger).await; assert_eq!(started.elapsed(), Duration::ZERO); assert!(matches!(channel.status().await, ChannelStatus::Closed)); @@ -541,9 +561,9 @@ mod tests { #[tokio::test(start_paused = true)] async fn close_after_linger_does_not_cut_the_linger_short() { let (mut channel, abort) = channel_with_lingering_task(); - channel.linger().await; + channel.close(CableClose::Linger).await; let started = time::Instant::now(); - channel.close().await; + channel.close(CableClose::Immediate).await; assert_eq!(started.elapsed(), Duration::ZERO); assert_eq!(*channel.teardown.borrow(), Teardown::Linger); assert!(!abort.is_finished()); diff --git a/libwebauthn/src/transport/cable/linger.rs b/libwebauthn/src/transport/cable/linger.rs index 89b43535..5acea7f3 100644 --- a/libwebauthn/src/transport/cable/linger.rs +++ b/libwebauthn/src/transport/cable/linger.rs @@ -32,9 +32,9 @@ pub(crate) enum Teardown { /// After a QR-initiated ceremony the authenticator may send its linking /// information a while after the CTAP response. Capturing it needs the /// connection to stay open after the caller is done with the channel, which -/// only happens when the caller calls -/// [`CableChannel::linger`](super::channel::CableChannel::linger) once the -/// ceremony has completed. Closing or dropping the channel captures nothing. +/// only happens when the caller closes the channel with +/// [`CableClose::Linger`](super::channel::CableClose::Linger) once the +/// ceremony has completed. An immediate close or a drop captures nothing. /// /// Carrying a config also makes opening a new hybrid channel evict any /// connection still lingering in the same [`CableLingerRegistry`]. diff --git a/libwebauthn/src/transport/cable/mod.rs b/libwebauthn/src/transport/cable/mod.rs index 7dc3bd96..3b916d39 100644 --- a/libwebauthn/src/transport/cable/mod.rs +++ b/libwebauthn/src/transport/cable/mod.rs @@ -16,6 +16,7 @@ pub mod qr_code_device; pub mod tunnel; use super::Transport; +pub use channel::CableClose; pub use digit_encode::digit_encode; pub use linger::{CableLingerConfig, CableLingerRegistry}; diff --git a/libwebauthn/src/transport/channel.rs b/libwebauthn/src/transport/channel.rs index c871cf8d..14996453 100644 --- a/libwebauthn/src/transport/channel.rs +++ b/libwebauthn/src/transport/channel.rs @@ -40,9 +40,9 @@ pub struct ChannelSettings { pub persistent_token_store: Option>, /// Opt-in to keeping a hybrid connection open after the ceremony to capture /// a late linking update. Enables close-on-new for this channel and lets - /// it linger when the caller calls - /// [`CableChannel::linger`](crate::transport::cable::channel::CableChannel::linger) - /// afterwards. Closing or dropping the channel captures nothing. `None` + /// it linger when the caller closes it with + /// [`CableClose::Linger`](crate::transport::cable::CableClose::Linger) + /// afterwards. An immediate close or a drop captures nothing. `None` /// disables it. Ignored by the other transports. pub cable_linger: Option, } @@ -81,8 +81,10 @@ pub trait Channel: Send + Sync + Display + Ctap2AuthTokenStore { async fn status(&self) -> ChannelStatus; /// Graceful close. Hybrid sends its protocol-level goodbye and returns - /// once the connection has been torn down. HID, BLE and NFC release the - /// link when the channel is dropped, so this is a no-op there. + /// once the connection has been torn down (see + /// [`CableChannel::close`](crate::transport::cable::channel::CableChannel::close) + /// for the lingering variant). HID, BLE and NFC release the link when the + /// channel is dropped, so this is a no-op there. async fn close(&mut self); /// Hard abort without a protocol-level goodbye. Falls back to diff --git a/libwebauthn/src/transport/mod.rs b/libwebauthn/src/transport/mod.rs index e12d9633..c4d816df 100644 --- a/libwebauthn/src/transport/mod.rs +++ b/libwebauthn/src/transport/mod.rs @@ -36,7 +36,7 @@ mod channel; #[allow(clippy::module_inception)] mod transport; -pub use cable::{CableLingerConfig, CableLingerRegistry}; +pub use cable::{CableClose, CableLingerConfig, CableLingerRegistry}; pub(crate) use channel::{AuthTokenData, Ctap2AuthTokenPermission}; pub use channel::{Channel, ChannelSettings, Ctap2AuthTokenStore};