fix(cable): send the tunnel Shutdown message on close

libwebauthn/src/transport/cable/channel.rs

@@ -3,9 +3,9 @@ use std::sync::Arc;
 use std::time::Duration;
 
 use async_trait::async_trait;
-use tokio::sync::{broadcast, mpsc, watch};
+use tokio::sync::{broadcast, mpsc, oneshot, watch};
 use tokio::{task, time};
-use tracing::error;
+use tracing::{debug, error};
 
 use crate::pin::persistent_token::PersistentTokenStore;
 use crate::proto::{
@@ -48,8 +48,17 @@ pub struct CableChannel {
     pub(crate) ux_update_sender: broadcast::Sender<CableUxUpdate>,
     pub(crate) connection_state_receiver: watch::Receiver<ConnectionState>,
     pub(crate) persistent_token_store: Option<Arc<dyn PersistentTokenStore>>,
+    /// Asks the connection task to send the tunnel Shutdown message and
+    /// close. Taken by [`Channel::close`].
+    pub(crate) shutdown_sender: Option<oneshot::Sender<()>>,
 }
 
+/// How long [`Channel::close`] waits for the connection task to deliver the
+/// Shutdown message and see the peer close. Chromium waits up to three minutes
+/// for the peer; a short bound keeps `close()` from stalling a caller that is
+/// about to exit, and phones close within a round trip of receiving Shutdown.
+pub(crate) const CLOSE_TIMEOUT: Duration = Duration::from_secs(3);
+
 impl CableChannel {
     async fn wait_for_connection(&self) -> Result<(), CableError> {
         let mut rx = self.connection_state_receiver.clone();
@@ -140,8 +149,26 @@ impl Channel for CableChannel {
         }
     }
 
+    /// Send the caBLE tunnel Shutdown message and close the connection, the
+    /// way Chromium's `FidoTunnelDevice` does. Without it the phone sees the
+    /// tunnel drop and shows an error even after a successful ceremony.
     async fn close(&mut self) {
-        // TODO Send CableTunnelMessageType#Shutdown and drop the connection
+        // Nothing to say Shutdown on before the tunnel is up: the task is
+        // still in the handshake, and dropping the channel aborts it.
+        if *self.connection_state_receiver.borrow() != ConnectionState::Connected {
+            self.shutdown_sender.take();
+            return;
+        }
+        let Some(shutdown) = self.shutdown_sender.take() else {
+            return;
+        };
+        if shutdown.send(()).is_ok()
+            && time::timeout(CLOSE_TIMEOUT, &mut self.handle_connection)
+                .await
+                .is_err()
+        {
+            debug!("caBLE connection did not finish closing in time");
+        }
     }
 
     async fn apdu_send(

libwebauthn/src/transport/cable/connection_stages.rs

@@ -1,6 +1,6 @@
 use ::btleplug::api::{AddressType, BDAddr};
 use async_trait::async_trait;
-use tokio::sync::{broadcast, mpsc, watch};
+use tokio::sync::{broadcast, mpsc, oneshot, watch};
 use tracing::{debug, error, info, instrument, trace, warn};
 
 use super::advertisement::{await_advertisement, DecryptedAdvert};
@@ -201,6 +201,8 @@ pub(crate) struct TunnelConnectionInput {
     pub noise_state: TunnelNoiseState,
     pub cbor_tx_recv: mpsc::Receiver<CborRequest>,
     pub cbor_rx_send: mpsc::Sender<CborResponse>,
+    /// Fires when the channel is closed.
+    pub shutdown_recv: oneshot::Receiver<()>,
 }
 
 impl TunnelConnectionInput {
@@ -209,6 +211,7 @@ impl TunnelConnectionInput {
         known_device_store: Option<Arc<dyn CableKnownDeviceInfoStore>>,
         cbor_tx_recv: mpsc::Receiver<CborRequest>,
         cbor_rx_send: mpsc::Sender<CborResponse>,
+        shutdown_recv: oneshot::Receiver<()>,
     ) -> Self {
         Self {
             connection_type: handshake_output.connection_type,
@@ -218,6 +221,7 @@ impl TunnelConnectionInput {
             noise_state: handshake_output.noise_state,
             cbor_tx_recv,
             cbor_rx_send,
+            shutdown_recv,
         }
     }
 }

libwebauthn/src/transport/cable/known_devices.rs

@@ -19,7 +19,7 @@ use futures::lock::Mutex;
 use serde::Serialize;
 use serde_bytes::ByteBuf;
 use serde_indexed::SerializeIndexed;
-use tokio::sync::{broadcast, mpsc, watch};
+use tokio::sync::{broadcast, mpsc, oneshot, watch};
 use tokio::task;
 use tracing::{debug, instrument, trace};
 
@@ -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 (shutdown_sender, shutdown_recv) = oneshot::channel();
         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,
+                shutdown_recv,
             );
 
             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,
+            shutdown_sender: Some(shutdown_sender),
         })
     }
 }