From 814ceef680c65ff9f2dd220964d5a75c3c49a021 Mon Sep 17 00:00:00 2001 From: Arjen <18398758+Origami74@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:51:58 +0100 Subject: [PATCH 1/9] fix(transport/ble): recover packet boundaries from the byte stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The BLE receive path assumes one `recv()` returns exactly one whole FIPS packet. That holds only for BlueZ's `SOCK_SEQPACKET`, which preserves SDU boundaries. It is not a property of L2CAP — it is a property of one backend's socket type. A stream-oriented L2CAP backend (Android's `BluetoothSocket` input stream, CoreBluetooth's `CBL2CAPChannel`) may return a fragment of a packet or several packets coalesced in a single read. Under the current loop a fragment ships up as a runt that FMP and Noise reject, and a coalesced tail is silently truncated and dropped — packets are lost and the transport thrashes with no error to show for it. Recover the boundaries from the bytes instead of trusting the OS to preserve them. FIPS packets are self-delimiting via the 4-byte FMP common prefix, and `transport::framing::read_fmp_packet` already parses exactly that, shared by every stream-oriented transport. All that was missing is an adapter: a new `BleStreamRead` turns the datagram-shaped `BleStream` into the `AsyncRead` the framer expects, buffering bytes left over from one read into the next. Its pending-read future owns its scratch buffer and yields an owned `Vec`, so it is `'static` and can be held across `poll_read` calls. One reader is threaded through both phases of a connection — the pre-handshake pubkey exchange and then the receive loop — so bytes a peer coalesces after the 33-byte pubkey stay buffered rather than being dropped at the hand-off. The exchange now reads via `read_exact`, which also reassembles a fragmented pubkey, instead of a single `recv()` with an exact-length check that a fragmenting backend can never satisfy. That ordering is load-bearing and worth stating plainly: the pubkey message's `0x00` prefix decodes as FMP version 0, phase 0 (established), with a payload length read out of the pubkey's own bytes. If the framer ever saw the exchange it would mis-frame badly. The exchange must be fully consumed before the framer starts, which is exactly what threading one reader guarantees. The constant now says so. On BlueZ this is a transparent pass-through — one `recv` already is one packet, so the adapter serves it whole and the framer takes it whole. On a stream backend it reassembles. Either way the layer above sees one complete packet per read, identically on every platform. Coverage: all of this is shared code exercised by `MockBleIo` on the host, so `cargo test` and `cargo clippy --all-targets` cover it in full on any platform where `transport::ble` compiles. Nothing here is inside `cfg(bluer_available)`, so no part of the change depends on a Bluetooth adapter to be checked. --- src/transport/ble/mod.rs | 335 ++++++++++++++++++++++++++----- src/transport/ble/stream_read.rs | 295 +++++++++++++++++++++++++++ 2 files changed, 584 insertions(+), 46 deletions(-) create mode 100644 src/transport/ble/stream_read.rs diff --git a/src/transport/ble/mod.rs b/src/transport/ble/mod.rs index 098868fd..92d57062 100644 --- a/src/transport/ble/mod.rs +++ b/src/transport/ble/mod.rs @@ -1,9 +1,18 @@ //! BLE L2CAP Transport Implementation //! -//! Provides BLE-based transport for FIPS peer communication using L2CAP -//! Connection-Oriented Channels (CoC) in SeqPacket mode. L2CAP CoC -//! preserves message boundaries (unlike TCP byte streams), so no FMP -//! framing is needed — each send/recv is one FIPS packet. +//! Provides BLE-based transport for FIPS peer communication over L2CAP +//! Connection-Oriented Channels. +//! +//! ## Packet boundaries +//! +//! Message-boundary preservation is a property of the *socket type* a +//! backend uses, not of L2CAP. BlueZ's `SOCK_SEQPACKET` preserves SDU +//! boundaries; other backends expose an L2CAP channel as a byte stream and +//! may return a fragment of a packet or several packets coalesced from one +//! read. The receive path therefore recovers boundaries from the FMP length +//! prefix via [`stream_read::BleStreamRead`] and +//! [`crate::transport::framing::read_fmp_packet`], which is a transparent +//! pass-through on a boundary-preserving backend. //! //! ## Architecture //! @@ -23,7 +32,9 @@ pub mod io; pub mod neighbor; pub mod pool; pub mod stats; +pub mod stream_read; +use super::framing::{StreamError, read_fmp_packet}; use super::{ ConnectionState, DiscoveredPeer, PacketTx, ReceivedPacket, Transport, TransportAddr, TransportError, TransportId, TransportState, TransportType, @@ -35,10 +46,12 @@ use io::{BleIo, BleScanner, BleStream}; use neighbor::NeighborBuffer; use pool::{BleConnection, ConnectionPool}; use stats::BleStats; +use stream_read::BleStreamRead; use secp256k1::XOnlyPublicKey; use std::collections::HashMap; use std::sync::Arc; +use tokio::io::AsyncReadExt; use tokio::sync::Mutex; use tokio::task::JoinHandle; use tracing::{debug, info, trace, warn}; @@ -364,9 +377,16 @@ impl BleTransport { } }; + // One reader for the life of the connection: the pubkey exchange and + // the receive loop must share it, or bytes the peer coalesced behind + // the exchange are dropped at the hand-off. + let stream = Arc::new(stream); + let recv_mtu = stream.recv_mtu(); + let mut reader = BleStreamRead::new(Arc::clone(&stream), recv_mtu); + // Pre-handshake pubkey exchange (temporary, pre-XX) if let Some(ref our_pubkey) = self.local_pubkey { - match pubkey_exchange(&stream, our_pubkey).await { + match pubkey_exchange(stream.as_ref(), &mut reader, our_pubkey).await { Ok(peer_pubkey) => { debug!(addr = %addr, "BLE outbound pubkey exchange complete"); self.neighbor_buffer @@ -379,7 +399,8 @@ impl BleTransport { } } - self.promote_connection(addr, &ble_addr, stream).await + self.promote_connection(addr, &ble_addr, stream, reader) + .await } /// Promote a newly established stream into the connection pool. @@ -389,14 +410,14 @@ impl BleTransport { &self, addr: &TransportAddr, ble_addr: &BleAddr, - stream: I::Stream, + stream: Arc, + reader: BleStreamRead, ) -> Result<(), TransportError> { let send_mtu = stream.send_mtu(); let recv_mtu = stream.recv_mtu(); - let stream = Arc::new(stream); let recv_task = tokio::spawn(receive_loop( - Arc::clone(&stream), + reader, addr.clone(), Arc::clone(&self.pool), self.packet_tx.clone(), @@ -484,9 +505,15 @@ impl BleTransport { match result { Ok(Ok(stream)) => { + let send_mtu = stream.send_mtu(); + let recv_mtu = stream.recv_mtu(); + let stream = Arc::new(stream); + // One reader across both phases — see `pubkey_exchange`. + let mut reader = BleStreamRead::new(Arc::clone(&stream), recv_mtu); + // Pre-handshake pubkey exchange (temporary, pre-XX) if let Some(ref our_pubkey) = local_pubkey { - match pubkey_exchange(&stream, our_pubkey).await { + match pubkey_exchange(stream.as_ref(), &mut reader, our_pubkey).await { Ok(peer_pubkey) => { debug!(addr = %addr_clone, "BLE outbound pubkey exchange complete"); neighbor_buffer.add_peer_with_pubkey(&ble_addr, peer_pubkey); @@ -501,12 +528,8 @@ impl BleTransport { } } - let send_mtu = stream.send_mtu(); - let recv_mtu = stream.recv_mtu(); - let stream = Arc::new(stream); - let recv_task = tokio::spawn(receive_loop( - Arc::clone(&stream), + reader, addr_clone.clone(), Arc::clone(&pool), packet_tx, @@ -663,6 +686,13 @@ impl Transport for BleTransport { /// /// Distinguishes the identity exchange from FMP packets (version ≥ 0x01). /// Temporary — removed when FMP switches from IK to XX handshake. +/// +/// Caution: this prefix is *not* distinguishable from an FMP packet by the +/// framer. `0x00` decodes as FMP version 0, phase 0 (established), with a +/// payload length read out of the pubkey's own bytes — i.e. arbitrary. Any +/// code that runs the framer over a connection before the exchange has been +/// fully consumed will mis-frame badly. Threading one reader through both +/// phases is what guarantees the ordering. const PUBKEY_EXCHANGE_PREFIX: u8 = 0x00; /// Pre-handshake pubkey exchange message size: `[0x00][pubkey:32]`. @@ -679,8 +709,16 @@ const PUBKEY_EXCHANGE_TIMEOUT_SECS: u64 = 5; /// /// Both sides send `[0x00][our_pubkey:32]` and receive the peer's. /// Returns the peer's XOnlyPublicKey on success. -async fn pubkey_exchange( +/// +/// Reads through the connection's `BleStreamRead` rather than calling +/// `recv` directly, for two reasons. It reassembles an exchange a +/// stream-oriented backend fragmented, which a single `recv` with an +/// exact-length check can never do. And anything the peer coalesced behind +/// the exchange stays buffered in the reader that the receive loop then +/// takes over, instead of being discarded at the hand-off. +async fn pubkey_exchange( stream: &S, + reader: &mut BleStreamRead, local_pubkey: &[u8; 32], ) -> Result { // Send our pubkey @@ -692,15 +730,15 @@ async fn pubkey_exchange( // Receive peer's pubkey (with timeout to prevent indefinite blocking) let mut buf = [0u8; PUBKEY_EXCHANGE_SIZE]; let timeout = std::time::Duration::from_secs(PUBKEY_EXCHANGE_TIMEOUT_SECS); - let n = match tokio::time::timeout(timeout, stream.recv(&mut buf)).await { - Ok(result) => result?, + match tokio::time::timeout(timeout, reader.read_exact(&mut buf)).await { + Ok(Ok(_)) => {} + Ok(Err(e)) => { + return Err(TransportError::RecvFailed(format!( + "pubkey exchange: {}", + e + ))); + } Err(_) => return Err(TransportError::Timeout), - }; - if n != PUBKEY_EXCHANGE_SIZE { - return Err(TransportError::RecvFailed(format!( - "pubkey exchange: expected {} bytes, got {}", - PUBKEY_EXCHANGE_SIZE, n - ))); } if buf[0] != PUBKEY_EXCHANGE_PREFIX { return Err(TransportError::RecvFailed(format!( @@ -751,10 +789,13 @@ async fn accept_loop( let send_mtu = stream.send_mtu(); let recv_mtu = stream.recv_mtu(); + let stream = Arc::new(stream); + // One reader across both phases — see `pubkey_exchange`. + let mut reader = BleStreamRead::new(Arc::clone(&stream), recv_mtu); // Pre-handshake pubkey exchange (temporary, pre-XX) if let Some(ref our_pubkey) = local_pubkey { - match pubkey_exchange(&stream, our_pubkey).await { + match pubkey_exchange(stream.as_ref(), &mut reader, our_pubkey).await { Ok(peer_pubkey) => { debug!(addr = %ta, "BLE inbound pubkey exchange complete"); neighbor_buffer.add_peer_with_pubkey(&addr, peer_pubkey); @@ -780,11 +821,9 @@ async fn accept_loop( } } - let stream = Arc::new(stream); - // Spawn receive loop let recv_task = tokio::spawn(receive_loop( - Arc::clone(&stream), + reader, ta.clone(), Arc::clone(&pool), packet_tx.clone(), @@ -829,8 +868,14 @@ async fn accept_loop( } /// Receive loop: reads packets from a BLE stream and delivers to node. -async fn receive_loop( - stream: Arc, +/// +/// Takes the connection's `BleStreamRead` — already positioned past the +/// pubkey exchange, and still holding anything the peer coalesced behind it +/// — and pulls whole FIPS packets out of it using the FMP length prefix. +/// Boundaries come from the bytes, not from the backend's socket type, so a +/// fragment is reassembled and a coalesced tail is not lost. +async fn receive_loop( + mut reader: BleStreamRead, addr: TransportAddr, pool: Arc>>>, packet_tx: PacketTx, @@ -838,21 +883,20 @@ async fn receive_loop( stats: Arc, recv_mtu: u16, ) { - let mut buf = vec![0u8; recv_mtu as usize]; loop { - match stream.recv(&mut buf).await { - Ok(0) => { - debug!(addr = %addr, "BLE connection closed by peer"); - break; - } - Ok(n) => { - stats.record_recv(n); - let packet = ReceivedPacket::new(transport_id, addr.clone(), buf[..n].to_vec()); + match read_fmp_packet(&mut reader, recv_mtu).await { + Ok(data) => { + stats.record_recv(data.len()); + let packet = ReceivedPacket::new(transport_id, addr.clone(), data); if packet_tx.send(packet).await.is_err() { trace!("BLE packet_tx closed, stopping receive loop"); break; } } + Err(StreamError::Io(e)) if e.kind() == std::io::ErrorKind::UnexpectedEof => { + debug!(addr = %addr, "BLE connection closed by peer"); + break; + } Err(e) => { debug!(addr = %addr, error = %e, "BLE receive error"); stats.record_recv_error(); @@ -986,7 +1030,12 @@ async fn scan_probe_loop( // Pubkey exchange, then promote connection to pool let ta = addr.to_transport_addr(); - match pubkey_exchange(&stream, &our_pubkey).await { + let send_mtu = stream.send_mtu(); + let recv_mtu = stream.recv_mtu(); + let stream = Arc::new(stream); + // One reader across both phases — see `pubkey_exchange`. + let mut reader = BleStreamRead::new(Arc::clone(&stream), recv_mtu); + match pubkey_exchange(stream.as_ref(), &mut reader, &our_pubkey).await { Ok(peer_pubkey) => { debug!(addr = %addr, "BLE probe complete"); @@ -1005,12 +1054,8 @@ async fn scan_probe_loop( } // Promote connection to pool — no second L2CAP connect needed - let send_mtu = stream.send_mtu(); - let recv_mtu = stream.recv_mtu(); - let stream = Arc::new(stream); - let recv_task = tokio::spawn(receive_loop( - Arc::clone(&stream), + reader, ta.clone(), Arc::clone(&pool), packet_tx.clone(), @@ -1064,7 +1109,42 @@ async fn scan_probe_loop( #[cfg(test)] mod tests { use super::*; - use io::MockBleIo; + use crate::transport::framing::build_established_frame; + use io::{MockBleIo, MockBleStream}; + use secp256k1::{Secp256k1, SecretKey}; + + /// Deterministic x-only pubkey for exchange tests. + fn test_pubkey(seed: u8) -> [u8; 32] { + let secp = Secp256k1::new(); + let sk = SecretKey::from_slice(&[seed; 32]).unwrap(); + sk.public_key(&secp).x_only_public_key().0.serialize() + } + + /// Handles a receive-loop test needs to observe: the task, the packets + /// it delivers, and the pool it reaps its entry from. + type ReceiveLoopHarness = ( + JoinHandle<()>, + tokio::sync::mpsc::Receiver, + Arc>>>, + ); + + /// Wire up a receive loop over one end of a mock stream pair. + fn spawn_receive_loop(local: MockBleStream) -> ReceiveLoopHarness { + let addr = test_addr(2).to_transport_addr(); + let pool = Arc::new(Mutex::new(ConnectionPool::new(7))); + let (tx, rx) = tokio::sync::mpsc::channel(16); + let reader = BleStreamRead::new(Arc::new(local), 2048); + let task = tokio::spawn(receive_loop( + reader, + addr, + Arc::clone(&pool), + tx, + TransportId::new(1), + Arc::new(BleStats::new()), + 2048, + )); + (task, rx, pool) + } fn test_addr(n: u8) -> BleAddr { BleAddr { @@ -1208,4 +1288,167 @@ mod tests { // Smaller node accepting from larger → drops inbound (outbound wins) // This means: smaller always uses outbound, larger always uses inbound } + + // ------------------------------------------------------------------ + // Packet boundary recovery + // ------------------------------------------------------------------ + + /// Two whole FMP packets delivered in one `recv` must both arrive. + /// Before reframing the tail was silently truncated and lost. + #[tokio::test] + async fn test_receive_loop_splits_coalesced_packets() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + let (task, mut rx, _pool) = spawn_receive_loop(local); + + let first = build_established_frame(16); + let second = build_established_frame(48); + let mut both = first.clone(); + both.extend_from_slice(&second); + peer.send(&both).await.unwrap(); + + assert_eq!(rx.recv().await.unwrap().data, first); + assert_eq!(rx.recv().await.unwrap().data, second); + task.abort(); + } + + /// One FMP packet split across three `recv`s arrives once, whole — + /// not as three runts that FMP and Noise would reject. + #[tokio::test] + async fn test_receive_loop_reassembles_fragmented_packet() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + let (task, mut rx, _pool) = spawn_receive_loop(local); + + let frame = build_established_frame(64); + let third = frame.len() / 3; + peer.send(&frame[..third]).await.unwrap(); + peer.send(&frame[third..2 * third]).await.unwrap(); + peer.send(&frame[2 * third..]).await.unwrap(); + + assert_eq!(rx.recv().await.unwrap().data, frame); + assert!(rx.try_recv().is_err(), "no runt packets"); + task.abort(); + } + + /// One `send` per packet still yields one packet per `send`, byte for + /// byte — the boundary-preserving backend regression. + #[tokio::test] + async fn test_receive_loop_passes_through_whole_packets() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + let (task, mut rx, _pool) = spawn_receive_loop(local); + + let frames: Vec> = [8u16, 0, 512] + .iter() + .map(|n| build_established_frame(*n)) + .collect(); + for f in &frames { + peer.send(f).await.unwrap(); + } + for f in &frames { + assert_eq!(&rx.recv().await.unwrap().data, f); + } + task.abort(); + } + + /// A malformed frame closes the connection and drops it from the pool + /// rather than spinning the loop. + #[tokio::test] + async fn test_receive_loop_drops_connection_on_bad_frame() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + let ta = test_addr(2).to_transport_addr(); + let (task, _rx, pool) = spawn_receive_loop(local); + + // Put a pool entry in place so its removal is observable. + let (parked, _other) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + pool.lock() + .await + .insert( + ta.clone(), + BleConnection { + stream: Arc::new(parked), + recv_task: None, + send_mtu: 2048, + recv_mtu: 2048, + established_at: tokio::time::Instant::now(), + is_static: false, + addr: test_addr(2), + }, + ) + .unwrap(); + assert!(pool.lock().await.contains(&ta)); + + // 0x16 is a TLS ClientHello record type; it parses as FMP version 1. + peer.send(&[0x16, 0x03, 0x01, 0x00]).await.unwrap(); + + // The loop exits and clears the pool entry. + for _ in 0..50 { + if !pool.lock().await.contains(&ta) { + break; + } + tokio::task::yield_now().await; + } + assert!(!pool.lock().await.contains(&ta)); + assert!(task.await.is_ok(), "loop exited cleanly"); + } + + /// A peer that coalesces its first data packet behind the 33-byte + /// pubkey exchange must not lose it at the hand-off to the framer. + #[tokio::test] + async fn test_pubkey_exchange_preserves_coalesced_data() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + let local = Arc::new(local); + let mut reader = BleStreamRead::new(Arc::clone(&local), 2048); + + let peer_pk = test_pubkey(2); + let frame = build_established_frame(24); + let mut wire = vec![PUBKEY_EXCHANGE_PREFIX]; + wire.extend_from_slice(&peer_pk); + wire.extend_from_slice(&frame); + peer.send(&wire).await.unwrap(); + + let got = pubkey_exchange(local.as_ref(), &mut reader, &test_pubkey(1)) + .await + .unwrap(); + assert_eq!(got.serialize(), peer_pk); + + let packet = read_fmp_packet(&mut reader, 2048).await.unwrap(); + assert_eq!(packet, frame); + } + + /// A fragmented pubkey exchange completes. The old exact-length `recv` + /// check could never satisfy this. + #[tokio::test] + async fn test_pubkey_exchange_reassembles_fragments() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + let local = Arc::new(local); + let mut reader = BleStreamRead::new(Arc::clone(&local), 2048); + + let peer_pk = test_pubkey(3); + let mut wire = vec![PUBKEY_EXCHANGE_PREFIX]; + wire.extend_from_slice(&peer_pk); + peer.send(&wire[..17]).await.unwrap(); + peer.send(&wire[17..]).await.unwrap(); + + let got = pubkey_exchange(local.as_ref(), &mut reader, &test_pubkey(1)) + .await + .unwrap(); + assert_eq!(got.serialize(), peer_pk); + } + + /// A peer that opens with something other than the exchange prefix is + /// rejected before the framer ever sees the bytes. + #[tokio::test] + async fn test_pubkey_exchange_rejects_bad_prefix() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + let local = Arc::new(local); + let mut reader = BleStreamRead::new(Arc::clone(&local), 2048); + + let mut wire = vec![0xFFu8]; + wire.extend_from_slice(&test_pubkey(4)); + peer.send(&wire).await.unwrap(); + + let err = pubkey_exchange(local.as_ref(), &mut reader, &test_pubkey(1)) + .await + .unwrap_err(); + assert!(matches!(err, TransportError::RecvFailed(_))); + } } diff --git a/src/transport/ble/stream_read.rs b/src/transport/ble/stream_read.rs new file mode 100644 index 00000000..59b12d66 --- /dev/null +++ b/src/transport/ble/stream_read.rs @@ -0,0 +1,295 @@ +//! `AsyncRead` adapter over a `BleStream`. +//! +//! The BLE receive path used to treat one `recv()` as one whole FIPS +//! packet. That is a property of a *SeqPacket* socket, not of L2CAP: a +//! stream-oriented backend may return a fragment of a packet, or several +//! packets coalesced, from a single read. This adapter turns the +//! datagram-shaped [`BleStream`] into the [`AsyncRead`] that +//! [`crate::transport::framing::read_fmp_packet`] expects, buffering bytes +//! left over from one read into the next so packet boundaries are recovered +//! from the FMP length prefix rather than trusted to the OS. +//! +//! Nothing here is backend-specific. On a boundary-preserving backend the +//! adapter is a transparent pass-through: one `recv` fills the buffer, the +//! framer consumes exactly it, and the next read hits the underlying stream +//! again. + +use std::future::Future; +use std::pin::Pin; +use std::sync::Arc; +use std::task::{Context, Poll}; + +use tokio::io::{AsyncRead, ReadBuf}; + +use crate::transport::TransportError; + +use super::io::BleStream; + +/// Smallest scratch buffer used for a single `recv`. +/// +/// Guards against a backend reporting a degenerate receive MTU, which would +/// otherwise make every `recv` return zero bytes and look like EOF. +const MIN_RECV_CHUNK: usize = 64; + +/// A pending `recv` that owns its scratch buffer and yields an owned `Vec`. +/// +/// Owning the buffer is what makes the future `'static`, which is what lets +/// it be held across `poll_read` calls when a read returns `Pending`. +type RecvFuture = Pin, TransportError>> + Send>>; + +/// Buffered [`AsyncRead`] view of a [`BleStream`]. +pub struct BleStreamRead { + stream: Arc, + /// Bytes received but not yet handed to the reader. + chunk: Vec, + /// Read cursor into `chunk`. + pos: usize, + /// Scratch size for one underlying `recv`. + capacity: usize, + /// In-flight `recv`, kept across polls. + pending: Option, + /// Set once the peer has closed the connection. + eof: bool, +} + +impl BleStreamRead { + /// Wrap a stream, sizing the scratch buffer from its receive MTU. + pub fn new(stream: Arc, recv_mtu: u16) -> Self { + Self { + stream, + chunk: Vec::new(), + pos: 0, + capacity: (recv_mtu as usize).max(MIN_RECV_CHUNK), + pending: None, + eof: false, + } + } + + /// Number of bytes already received but not yet consumed. + /// + /// Non-zero after a peer coalesces data behind an earlier message; the + /// hand-off from the pubkey exchange to the framer must preserve them. + #[cfg(test)] + pub fn buffered(&self) -> usize { + self.chunk.len() - self.pos + } + + fn start_recv(&self) -> RecvFuture { + let stream = Arc::clone(&self.stream); + let capacity = self.capacity; + Box::pin(async move { + let mut scratch = vec![0u8; capacity]; + let n = stream.recv(&mut scratch).await?; + scratch.truncate(n); + Ok(scratch) + }) + } +} + +/// Map a transport error onto the `io::Error` `AsyncRead` must report. +fn to_io(e: TransportError) -> std::io::Error { + match e { + TransportError::Io(e) => e, + other => std::io::Error::other(other.to_string()), + } +} + +impl AsyncRead for BleStreamRead { + fn poll_read( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + let this = self.get_mut(); + loop { + // Serve from the leftover buffer first. + if this.pos < this.chunk.len() { + let n = (this.chunk.len() - this.pos).min(buf.remaining()); + buf.put_slice(&this.chunk[this.pos..this.pos + n]); + this.pos += n; + if this.pos == this.chunk.len() { + this.chunk.clear(); + this.pos = 0; + } + return Poll::Ready(Ok(())); + } + + // A closed connection stays closed: report EOF (a filled length + // of zero) rather than re-polling a dead stream forever. + if this.eof { + return Poll::Ready(Ok(())); + } + + let mut fut = match this.pending.take() { + Some(f) => f, + None => this.start_recv(), + }; + match fut.as_mut().poll(cx) { + Poll::Pending => { + this.pending = Some(fut); + return Poll::Pending; + } + Poll::Ready(Ok(chunk)) => { + // `recv` returning zero bytes is the peer-closed signal, + // not an empty packet. + if chunk.is_empty() { + this.eof = true; + return Poll::Ready(Ok(())); + } + this.chunk = chunk; + this.pos = 0; + } + Poll::Ready(Err(e)) => return Poll::Ready(Err(to_io(e))), + } + } + } +} + +// ============================================================================ +// Tests +// ============================================================================ + +#[cfg(test)] +mod tests { + use super::*; + use crate::transport::ble::addr::BleAddr; + use crate::transport::ble::io::MockBleStream; + use tokio::io::AsyncReadExt; + use tokio::sync::Mutex as TokioMutex; + + fn test_addr(n: u8) -> BleAddr { + BleAddr { + adapter: "hci0".to_string(), + device: [0xAA, 0xBB, 0xCC, 0xDD, 0xEE, n], + } + } + + /// A stream that replays a fixed script of `recv` results and then + /// returns `Ok(0)` forever — the "peer closed but socket still open" + /// shape a channel-backed mock cannot produce. + struct ScriptedStream { + addr: BleAddr, + chunks: TokioMutex>>, + } + + impl ScriptedStream { + fn new(chunks: Vec>) -> Self { + Self { + addr: test_addr(9), + chunks: TokioMutex::new(chunks.into()), + } + } + } + + impl BleStream for ScriptedStream { + async fn send(&self, _data: &[u8]) -> Result<(), TransportError> { + Ok(()) + } + + async fn recv(&self, buf: &mut [u8]) -> Result { + match self.chunks.lock().await.pop_front() { + Some(chunk) => { + let n = chunk.len().min(buf.len()); + buf[..n].copy_from_slice(&chunk[..n]); + Ok(n) + } + None => Ok(0), + } + } + + fn send_mtu(&self) -> u16 { + 2048 + } + + fn recv_mtu(&self) -> u16 { + 2048 + } + + fn remote_addr(&self) -> &BleAddr { + &self.addr + } + } + + #[tokio::test] + async fn test_fragmented_delivery_is_reassembled() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + peer.send(b"abc").await.unwrap(); + peer.send(b"defg").await.unwrap(); + peer.send(b"hij").await.unwrap(); + + let mut reader = BleStreamRead::new(Arc::new(local), 2048); + let mut out = [0u8; 10]; + reader.read_exact(&mut out).await.unwrap(); + assert_eq!(&out, b"abcdefghij"); + } + + #[tokio::test] + async fn test_coalesced_delivery_keeps_the_tail() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + peer.send(b"0123456789").await.unwrap(); + + let mut reader = BleStreamRead::new(Arc::new(local), 2048); + let mut head = [0u8; 4]; + reader.read_exact(&mut head).await.unwrap(); + assert_eq!(&head, b"0123"); + assert_eq!(reader.buffered(), 6); + + let mut tail = [0u8; 6]; + reader.read_exact(&mut tail).await.unwrap(); + assert_eq!(&tail, b"456789"); + } + + #[tokio::test] + async fn test_peer_drop_surfaces_as_unexpected_eof() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + peer.send(b"ab").await.unwrap(); + drop(peer); + + let mut reader = BleStreamRead::new(Arc::new(local), 2048); + let mut out = [0u8; 4]; + let err = reader.read_exact(&mut out).await.unwrap_err(); + assert_eq!(err.kind(), std::io::ErrorKind::UnexpectedEof); + } + + #[tokio::test] + async fn test_zero_length_recv_is_eof_not_readiness() { + let stream = ScriptedStream::new(vec![b"xy".to_vec()]); + let mut reader = BleStreamRead::new(Arc::new(stream), 2048); + + let mut out = [0u8; 8]; + let n = reader.read(&mut out).await.unwrap(); + assert_eq!(&out[..n], b"xy"); + + // The scripted stream now returns Ok(0) forever. That must read as + // EOF once and stay EOF, not as a spurious zero-length packet. + assert_eq!(reader.read(&mut out).await.unwrap(), 0); + assert_eq!(reader.read(&mut out).await.unwrap(), 0); + } + + #[tokio::test] + async fn test_read_smaller_than_chunk_leaves_remainder() { + let stream = ScriptedStream::new(vec![b"abcdef".to_vec()]); + let mut reader = BleStreamRead::new(Arc::new(stream), 2048); + + let mut one = [0u8; 1]; + reader.read_exact(&mut one).await.unwrap(); + assert_eq!(&one, b"a"); + assert_eq!(reader.buffered(), 5); + + let mut rest = [0u8; 5]; + reader.read_exact(&mut rest).await.unwrap(); + assert_eq!(&rest, b"bcdef"); + assert_eq!(reader.buffered(), 0); + } + + #[tokio::test] + async fn test_degenerate_recv_mtu_still_reads() { + let (peer, local) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + peer.send(b"hello").await.unwrap(); + + let mut reader = BleStreamRead::new(Arc::new(local), 0); + let mut out = [0u8; 5]; + reader.read_exact(&mut out).await.unwrap(); + assert_eq!(&out, b"hello"); + } +} From 8ba8076dbb4dc83375014d592a6795a94389f1d2 Mon Sep 17 00:00:00 2001 From: Arjen <18398758+Origami74@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:51:58 +0100 Subject: [PATCH 2/9] fix(transport/ble): recognise a peer by node identity, not by its rotating link address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A peer using resolvable private addresses rotates its BLE address continually, and every rotation presents as a brand-new device. This is not an exotic case: RPA rotation is a BLE privacy feature that modern phones use by default, so a BlueZ node scanning one hits this today. Field capture from a two-node mesh recorded one peer dialling in 28 times from 28 distinct addresses in twenty minutes. Every identity check in this transport keys on the link address, so none of them could tell. `ConnectionPool` is a `HashMap`, and all three already-connected guards — one in `accept_loop`, two in `scan_probe_loop` — ask `pool.contains(&addr.to_transport_addr())`. For a rotated address the answer is always "not connected", so the caller opens another link, and another. Those 28 rotations were harmless only by luck. The cross-probe tie-breaker happened to reject every one of them, and which side it protects is decided by a byte comparison of two node addresses. Had they sorted the other way, the same 28 inbound dials would have been admitted into a pool that holds seven, evicting genuine peers roughly four times over. The tie-breaker is not the problem and is unchanged here — the problem is that link identity was being used as node identity. `BleConnection` now carries the peer's `NodeAddr` once the pubkey exchange has learned it, and `ConnectionPool::find_by_node` looks a peer up by the identity that does not rotate. Both admission points consult it after the exchange and decline a duplicate, keeping the incumbent link: it is known-good, and a genuinely dead one is already reaped by the send-error and receive-loop paths. Two consequences follow and are handled here rather than left as sequels. The scan loop remembers what a declined address resolved to and skips it while that node is still connected. Without that it re-dials every rotated alias of a live peer once per cooldown, forever — the duplicate is declined, so the alias never enters the pool, so the pool-keyed guard above never sees it. The mapping is dropped as soon as the node leaves the pool, so a peer that genuinely goes away is probed normally again. And a completed exchange is announced under the address the peer's link is actually on, not the rotated alias it happened on. Announcing the alias makes a consumer that compares addresses treat it as a new path to a peer it already holds a link to, and dial it; the duplicate is declined, so nothing upstream remembers the conclusion and the next round repeats it. `ConnectionPool::live_addr_of_node` answers with the `BleAddr` the link is on, where `find_by_node` answers with the pool key — the caller here has to name the address, not merely test for one. Canonicalising rather than withholding matters: suppressing the announcement would also stop the peer being offered at all, and consumers legitimately re-probe a peer whose link has gone idle to recover it. Declines are counted rather than silent, so absorption of a rotating peer shows up in `show_transports` as a climbing `duplicate_node_declines` instead of as an absence of log lines. Coverage: all shared code driven by `MockBleIo`, so `cargo test` and `cargo clippy --all-targets` cover every branch on any platform where `transport::ble` compiles. Seven new pool tests, including one pinning the regression — ten rotations of one peer leave the pool holding exactly one link — plus two integration tests driving the inbound decline and the scan loop's alias suppression end to end. Nothing here is inside `cfg(bluer_available)`. --- src/transport/ble/mod.rs | 347 ++++++++++++++++++++++++++++++++++--- src/transport/ble/pool.rs | 158 +++++++++++++++++ src/transport/ble/stats.rs | 16 ++ 3 files changed, 498 insertions(+), 23 deletions(-) diff --git a/src/transport/ble/mod.rs b/src/transport/ble/mod.rs index 92d57062..16ab99db 100644 --- a/src/transport/ble/mod.rs +++ b/src/transport/ble/mod.rs @@ -385,12 +385,16 @@ impl BleTransport { let mut reader = BleStreamRead::new(Arc::clone(&stream), recv_mtu); // Pre-handshake pubkey exchange (temporary, pre-XX) + let mut peer_node: Option = None; if let Some(ref our_pubkey) = self.local_pubkey { match pubkey_exchange(stream.as_ref(), &mut reader, our_pubkey).await { Ok(peer_pubkey) => { debug!(addr = %addr, "BLE outbound pubkey exchange complete"); + let node = NodeAddr::from_pubkey(&peer_pubkey); + peer_node = Some(node); + let announced = announced_addr(&self.pool, &node, &ble_addr).await; self.neighbor_buffer - .add_peer_with_pubkey(&ble_addr, peer_pubkey); + .add_peer_with_pubkey(&announced, peer_pubkey); } Err(e) => { warn!(addr = %addr, error = %e, "BLE outbound pubkey exchange failed"); @@ -399,7 +403,7 @@ impl BleTransport { } } - self.promote_connection(addr, &ble_addr, stream, reader) + self.promote_connection(addr, &ble_addr, stream, reader, peer_node) .await } @@ -412,6 +416,7 @@ impl BleTransport { ble_addr: &BleAddr, stream: Arc, reader: BleStreamRead, + node_addr: Option, ) -> Result<(), TransportError> { let send_mtu = stream.send_mtu(); let recv_mtu = stream.recv_mtu(); @@ -434,6 +439,7 @@ impl BleTransport { established_at: tokio::time::Instant::now(), is_static: false, addr: ble_addr.clone(), + node_addr, }; let mut pool = self.pool.lock().await; @@ -512,11 +518,15 @@ impl BleTransport { let mut reader = BleStreamRead::new(Arc::clone(&stream), recv_mtu); // Pre-handshake pubkey exchange (temporary, pre-XX) + let mut peer_node: Option = None; if let Some(ref our_pubkey) = local_pubkey { match pubkey_exchange(stream.as_ref(), &mut reader, our_pubkey).await { Ok(peer_pubkey) => { debug!(addr = %addr_clone, "BLE outbound pubkey exchange complete"); - neighbor_buffer.add_peer_with_pubkey(&ble_addr, peer_pubkey); + let node = NodeAddr::from_pubkey(&peer_pubkey); + peer_node = Some(node); + let announced = announced_addr(&pool, &node, &ble_addr).await; + neighbor_buffer.add_peer_with_pubkey(&announced, peer_pubkey); } Err(e) => { warn!( @@ -546,6 +556,7 @@ impl BleTransport { established_at: tokio::time::Instant::now(), is_static: false, addr: ble_addr, + node_addr: peer_node, }; let mut pool = pool.lock().await; @@ -705,6 +716,36 @@ const PUBKEY_EXCHANGE_SIZE: usize = 33; /// forever — killing scan_probe_loop, accept_loop, or the event loop. const PUBKEY_EXCHANGE_TIMEOUT_SECS: u64 = 5; +/// The link address a completed pubkey exchange should be announced under. +/// +/// A peer using resolvable private addresses presents a different link +/// address on every rotation, so the address an exchange happened on is a +/// transient alias for the peer, not a durable way to name it. Announcing the +/// alias makes a consumer that compares addresses treat it as a *new path* to +/// a peer it is already connected to and dial it; the duplicate is declined +/// here, so it never reaches the pool, so nothing upstream remembers the +/// conclusion and the next discovery round pays the same connect and exchange +/// again. `scan_probe_loop` breaks that cycle for its own probes, but callers +/// that reach `connect_async` directly never consult it. +/// +/// So when the peer is already connected, report the address its link is +/// actually on: same peer, named by the address that works. When it is not, +/// there is no incumbent and the observed address stands. +/// +/// Canonicalising rather than withholding matters: suppressing the +/// announcement would also stop the peer being offered at all, and consumers +/// legitimately re-probe a peer whose link has gone idle to recover it. +async fn announced_addr( + pool: &Mutex>, + node: &NodeAddr, + observed: &BleAddr, +) -> BleAddr { + pool.lock() + .await + .live_addr_of_node(node) + .unwrap_or_else(|| observed.clone()) +} + /// Exchange public keys over a newly established L2CAP connection. /// /// Both sides send `[0x00][our_pubkey:32]` and receive the peer's. @@ -794,24 +835,51 @@ async fn accept_loop( let mut reader = BleStreamRead::new(Arc::clone(&stream), recv_mtu); // Pre-handshake pubkey exchange (temporary, pre-XX) + let mut peer_node_addr: Option = None; if let Some(ref our_pubkey) = local_pubkey { match pubkey_exchange(stream.as_ref(), &mut reader, our_pubkey).await { Ok(peer_pubkey) => { debug!(addr = %ta, "BLE inbound pubkey exchange complete"); - neighbor_buffer.add_peer_with_pubkey(&addr, peer_pubkey); + let peer_node = NodeAddr::from_pubkey(&peer_pubkey); + peer_node_addr = Some(peer_node); + let announced = announced_addr(&pool, &peer_node, &addr).await; + neighbor_buffer.add_peer_with_pubkey(&announced, peer_pubkey); + + // Already linked to this peer on another address? + // A peer using resolvable private addresses rotates + // continually, and every rotation dials in looking + // like a new device. Admitting those would put one + // peer in several pool slots and evict real ones. + // The incumbent link is kept: it is known-good, and + // a genuinely dead one is already reaped by the + // send-error and receive-loop paths. + let dup = { + let pool_guard = pool.lock().await; + pool_guard.find_by_node(&peer_node) + }; + if let Some(existing) = dup + && existing != ta + { + debug!( + addr = %ta, + existing = %existing, + "BLE inbound: peer already connected on another address, dropping duplicate" + ); + stats.record_duplicate_node_decline(); + continue; + } // Cross-probe tie-breaker: smaller NodeAddr's // outbound wins. If we're smaller, our outbound // should win — drop this inbound. - if let Some(ref our_addr) = local_node_addr { - let peer_addr = NodeAddr::from_pubkey(&peer_pubkey); - if our_addr < &peer_addr { - debug!( - addr = %ta, - "BLE inbound tie-breaker: dropping (our addr < peer, outbound wins)" - ); - continue; - } + if let Some(ref our_addr) = local_node_addr + && our_addr < &peer_node + { + debug!( + addr = %ta, + "BLE inbound tie-breaker: dropping (our addr < peer, outbound wins)" + ); + continue; } } Err(e) => { @@ -840,6 +908,7 @@ async fn accept_loop( established_at: tokio::time::Instant::now(), is_static: false, addr, + node_addr: peer_node_addr, }; let mut pool_guard = pool.lock().await; @@ -942,6 +1011,13 @@ async fn scan_probe_loop( // Addresses discovered but not yet connected — retried after cooldown // even if the scanner doesn't fire again (BlueZ deduplicates). let mut pending_addrs: Vec = Vec::new(); + // Link addresses already resolved to a node identity by a completed pubkey + // exchange. Lets the loop skip an address it has *already* learned belongs + // to a peer it is connected to, instead of paying a full connect and + // exchange to rediscover that every cooldown. Rotation means this grows by + // one per rotation, so entries are dropped once their node is no longer in + // the pool — a peer that genuinely goes away is probed again normally. + let mut known_node_of: HashMap = HashMap::new(); let cooldown = std::time::Duration::from_secs(cooldown_secs); let retry_interval = tokio::time::interval(std::time::Duration::from_secs(cooldown_secs)); tokio::pin!(retry_interval); @@ -997,6 +1073,23 @@ async fn scan_probe_loop( continue; } + // Skip an address already known to belong to a peer we are connected + // to. Without this the loop re-dials every rotated address of a live + // peer once per cooldown, forever: the duplicate is declined so it + // never enters the pool, so the pool-keyed guard above never sees it. + if let Some(node) = known_node_of.get(&addr) { + let still_connected = { + let pool_guard = pool.lock().await; + pool_guard.find_by_node(node).is_some() + }; + if still_connected { + pending_addrs.retain(|a| a != &addr); + continue; + } + // That peer is gone — forget the mapping and probe normally. + known_node_of.remove(&addr); + } + // Record probe time (before attempt, so cooldown applies on failure too) last_probed.insert(addr.clone(), tokio::time::Instant::now()); @@ -1038,19 +1131,50 @@ async fn scan_probe_loop( match pubkey_exchange(stream.as_ref(), &mut reader, &our_pubkey).await { Ok(peer_pubkey) => { debug!(addr = %addr, "BLE probe complete"); + let peer_node = NodeAddr::from_pubkey(&peer_pubkey); // Cross-probe tie-breaker: smaller NodeAddr's outbound wins. // If we lose, drop connection — accept_loop handles inbound. - if let Some(ref our_addr) = local_node_addr { - let peer_addr = NodeAddr::from_pubkey(&peer_pubkey); - if our_addr >= &peer_addr { - debug!( - addr = %addr, - "BLE probe tie-breaker: yielding to peer's outbound" - ); - buffer.add_peer_with_pubkey(&addr, peer_pubkey); - continue; - } + if let Some(ref our_addr) = local_node_addr + && our_addr >= &peer_node + { + debug!( + addr = %addr, + "BLE probe tie-breaker: yielding to peer's outbound" + ); + let announced = announced_addr(&pool, &peer_node, &addr).await; + buffer.add_peer_with_pubkey(&announced, peer_pubkey); + continue; + } + + // Same duplicate guard as the inbound path: a rotated address + // for a peer we already hold a link to must not become a + // second pool entry. Checked after the tie-breaker so the two + // decisions stay independent. + let dup = { + let pool_guard = pool.lock().await; + pool_guard.find_by_node(&peer_node) + }; + if let Some(existing) = dup + && existing != ta + { + debug!( + addr = %ta, + existing = %existing, + "BLE probe: peer already connected on another address, dropping duplicate" + ); + stats.record_duplicate_node_decline(); + // Remember what this address resolved to, so the next + // cooldown skips it outright rather than paying another + // connect and exchange to reach the same conclusion. + known_node_of.insert(addr.clone(), peer_node); + // Report the peer under the address its live link is on, + // so the node layer is not handed an alias with no + // connection behind it. + let announced = announced_addr(&pool, &peer_node, &addr).await; + buffer.add_peer_with_pubkey(&announced, peer_pubkey); + pending_addrs.retain(|a| a != &addr); + continue; } // Promote connection to pool — no second L2CAP connect needed @@ -1072,6 +1196,7 @@ async fn scan_probe_loop( established_at: tokio::time::Instant::now(), is_static: false, addr: addr.clone(), + node_addr: Some(peer_node), }; let mut pool_guard = pool.lock().await; @@ -1371,6 +1496,7 @@ mod tests { established_at: tokio::time::Instant::now(), is_static: false, addr: test_addr(2), + node_addr: None, }, ) .unwrap(); @@ -1434,6 +1560,181 @@ mod tests { assert_eq!(got.serialize(), peer_pk); } + // ------------------------------------------------------------------ + // Node identity vs. rotating link address + // ------------------------------------------------------------------ + + /// Two pubkeys, returned as `(smaller_node_addr, larger_node_addr)`. + /// + /// The cross-probe tie-breaker is decided by `NodeAddr` ordering, so a + /// test that wants a connection admitted has to know which side it is. + fn pubkeys_ordered_by_node_addr() -> ([u8; 32], [u8; 32]) { + let a = test_pubkey(1); + let b = test_pubkey(2); + let na = NodeAddr::from_pubkey(&XOnlyPublicKey::from_slice(&a).unwrap()); + let nb = NodeAddr::from_pubkey(&XOnlyPublicKey::from_slice(&b).unwrap()); + if na < nb { (a, b) } else { (b, a) } + } + + /// Run the peer half of the pubkey exchange over a mock stream end. + async fn peer_side_exchange(peer: &MockBleStream, peer_pubkey: &[u8; 32]) { + let mut msg = [0u8; PUBKEY_EXCHANGE_SIZE]; + msg[0] = PUBKEY_EXCHANGE_PREFIX; + msg[1..].copy_from_slice(peer_pubkey); + peer.send(&msg).await.unwrap(); + let mut buf = [0u8; PUBKEY_EXCHANGE_SIZE]; + let n = peer.recv(&mut buf).await.unwrap(); + assert_eq!(n, PUBKEY_EXCHANGE_SIZE); + } + + fn identity_test_config() -> BleConfig { + BleConfig { + adapter: Some("hci0".to_string()), + scan: Some(false), + advertise: Some(false), + accept_connections: Some(true), + probe_cooldown_secs: Some(1), + ..Default::default() + } + } + + /// Let spawned loops make progress. + async fn settle() { + for _ in 0..64 { + tokio::task::yield_now().await; + } + } + + /// A second inbound connection from a rotated address for a peer already + /// in the pool is declined, the incumbent link is kept, and the peer is + /// still announced — under the address its live link is on. + #[tokio::test] + async fn test_inbound_rotation_is_declined_and_keeps_the_incumbent() { + let (smaller, larger) = pubkeys_ordered_by_node_addr(); + let io = MockBleIo::new("hci0", test_addr(1)); + let (tx, _rx) = tokio::sync::mpsc::channel(64); + let mut transport = + BleTransport::new(TransportId::new(1), None, identity_test_config(), io, tx); + // We take the larger node address, so the inbound tie-breaker admits + // rather than drops — the duplicate guard is what is under test. + transport.set_local_pubkey(larger); + transport.start_async().await.unwrap(); + + // First inbound, on link address 2. + let (ours, peer_a) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + transport.io.inject_inbound(ours).await; + peer_side_exchange(&peer_a, &smaller).await; + settle().await; + + assert_eq!(transport.pool.lock().await.len(), 1); + assert!( + transport + .pool + .lock() + .await + .contains(&test_addr(2).to_transport_addr()) + ); + + // The same node dials in again after rotating to link address 3. + let (ours2, peer_b) = MockBleStream::pair(test_addr(1), test_addr(3), 2048); + transport.io.inject_inbound(ours2).await; + peer_side_exchange(&peer_b, &smaller).await; + settle().await; + + let pool = transport.pool.lock().await; + assert_eq!(pool.len(), 1, "the rotation must not become a second link"); + assert!( + pool.contains(&test_addr(2).to_transport_addr()), + "the incumbent link is kept" + ); + assert!(!pool.contains(&test_addr(3).to_transport_addr())); + drop(pool); + + assert_eq!(transport.stats.snapshot().duplicate_node_declines, 1); + + // Discovery names the peer by the address its link is actually on, + // not by the alias the rotation arrived from — otherwise the node + // layer is handed an address with no connection behind it. + let peers = transport.neighbor_buffer.take(); + assert_eq!(peers.len(), 1); + assert_eq!(peers[0].addr, test_addr(2).to_transport_addr()); + + transport.stop_async().await.unwrap(); + drop((peer_a, peer_b)); + } + + /// Once a rotated alias has been resolved to a peer that holds a live + /// link, the scan loop stops paying a connect and exchange to reach that + /// same conclusion every cooldown. + #[tokio::test(start_paused = true)] + async fn test_scan_loop_stops_reprobing_a_resolved_alias() { + use std::sync::Mutex as StdMutex; + + let (smaller, larger) = pubkeys_ordered_by_node_addr(); + let io = MockBleIo::new("hci0", test_addr(1)); + + let connects: Arc>> = Arc::new(StdMutex::new(Vec::new())); + let (peer_tx, mut peer_rx) = tokio::sync::mpsc::unbounded_channel(); + { + let connects = Arc::clone(&connects); + io.set_connect_handler(move |addr, _psm| { + let (ours, theirs) = MockBleStream::pair(test_addr(1), addr.clone(), 2048); + connects.lock().unwrap().push(addr.clone()); + peer_tx + .send(theirs) + .map_err(|_| TransportError::ConnectionRefused)?; + Ok(ours) + }); + } + + // The remote answers every probe with one identity, whichever link + // address the probe went to. + tokio::spawn(async move { + let mut alive = Vec::new(); + while let Some(theirs) = peer_rx.recv().await { + peer_side_exchange(&theirs, &larger).await; + alive.push(theirs); + } + }); + + let config = BleConfig { + scan: Some(true), + accept_connections: Some(false), + ..identity_test_config() + }; + let (tx, _rx) = tokio::sync::mpsc::channel(64); + let mut transport = BleTransport::new(TransportId::new(1), None, config, io, tx); + // We take the smaller node address, so our outbound wins the + // tie-breaker and the probe is promoted. + transport.set_local_pubkey(smaller); + transport.start_async().await.unwrap(); + + transport.io.inject_scan_result(test_addr(2)).await; + settle().await; + assert_eq!(transport.pool.lock().await.len(), 1); + assert_eq!(connects.lock().unwrap().len(), 1); + + // The peer rotates to address 3. That probe is paid once and declined. + transport.io.inject_scan_result(test_addr(3)).await; + settle().await; + assert_eq!(connects.lock().unwrap().len(), 2); + assert_eq!(transport.stats.snapshot().duplicate_node_declines, 1); + assert_eq!(transport.pool.lock().await.len(), 1); + + // The alias is advertised again after the cooldown expires. It must + // not be dialled a third time: the loop already knows whose it is. + tokio::time::advance(std::time::Duration::from_secs(5)).await; + transport.io.inject_scan_result(test_addr(3)).await; + settle().await; + assert_eq!( + connects.lock().unwrap().len(), + 2, + "a resolved alias of a live peer must not be re-dialled" + ); + + transport.stop_async().await.unwrap(); + } + /// A peer that opens with something other than the exchange prefix is /// rejected before the framer ever sees the bytes. #[tokio::test] diff --git a/src/transport/ble/pool.rs b/src/transport/ble/pool.rs index ffdf45db..837b1c0c 100644 --- a/src/transport/ble/pool.rs +++ b/src/transport/ble/pool.rs @@ -8,6 +8,7 @@ use std::collections::HashMap; use tokio::task::JoinHandle; +use crate::identity::NodeAddr; use crate::transport::{TransportAddr, TransportError}; use super::addr::BleAddr; @@ -28,6 +29,16 @@ pub struct BleConnection { pub is_static: bool, /// Parsed remote address. pub addr: BleAddr, + /// The peer's node address, once the pubkey exchange has learned it. + /// + /// The pool is keyed by *link* address, but a BLE link address is not a + /// stable identity: peers using resolvable private addresses rotate theirs + /// continually, and each rotation looks like a brand-new device. This + /// field carries the identity that does not rotate, so + /// [`ConnectionPool::find_by_node`] can recognise a peer already connected + /// under an address never seen before. `None` for a connection whose peer + /// is not yet identified. + pub node_addr: Option, } impl BleConnection { @@ -95,6 +106,38 @@ impl ConnectionPool { self.connections.contains_key(addr) } + /// Find an existing connection to `node`, whatever link address it + /// arrived on. + /// + /// This is the identity check [`Self::contains`] cannot make. A peer using + /// resolvable private addresses presents a different link address every + /// rotation, so an address-keyed lookup reports "not connected" for a peer + /// that is very much connected — and the caller then opens a second link + /// to it, and a third. Callers that know the peer's node address should + /// ask this before admitting a connection. + /// + /// Only connections whose pubkey exchange has completed carry a node + /// address, so an unidentified connection is never matched. + pub fn find_by_node(&self, node: &NodeAddr) -> Option { + self.connections + .iter() + .find(|(_, c)| c.node_addr.as_ref() == Some(node)) + .map(|(addr, _)| addr.clone()) + } + + /// The live link address for `node`, if it is connected. + /// + /// [`Self::find_by_node`] answers with the pool key; this answers with the + /// `BleAddr` the link is actually on, which is what a caller needs when it + /// has to *name* the peer's current address rather than merely test for + /// one. + pub fn live_addr_of_node(&self, node: &NodeAddr) -> Option { + self.connections + .values() + .find(|c| c.node_addr.as_ref() == Some(node)) + .map(|c| c.addr.clone()) + } + /// Try to insert a connection, evicting if necessary. /// /// Returns `Ok(evicted_addr)` on success (with optional evicted peer), @@ -186,6 +229,13 @@ mod tests { } } + /// A distinct node identity per `n` — the identity that does NOT rotate. + fn test_node(n: u8) -> NodeAddr { + let mut bytes = [0u8; 16]; + bytes[0] = n; + NodeAddr::from_bytes(bytes) + } + fn test_conn(n: u8, is_static: bool) -> BleConnection<()> { BleConnection { stream: (), @@ -195,6 +245,7 @@ mod tests { established_at: tokio::time::Instant::now(), is_static, addr: test_ble_addr(n), + node_addr: None, } } @@ -287,4 +338,111 @@ mod tests { addrs.sort_by(|a, b| a.as_str().cmp(&b.as_str())); assert_eq!(addrs.len(), 2); } + + /// A node address is found regardless of which link address it arrived on + /// — the whole point of the lookup, since the link address rotates. + #[test] + fn test_find_by_node_matches_across_a_rotated_link_address() { + let mut pool: ConnectionPool<()> = ConnectionPool::new(7); + let node = test_node(1); + let mut conn = test_conn(1, false); + conn.node_addr = Some(node); + pool.insert(test_addr(1), conn).unwrap(); + + // Found under the address it was inserted with... + assert_eq!(pool.find_by_node(&node), Some(test_addr(1))); + assert!(pool.contains(&test_addr(1))); + // ...and a rotated address for the same peer is NOT found by + // `contains`, which is exactly the gap `find_by_node` closes. + assert!(!pool.contains(&test_addr(99))); + assert_eq!(pool.find_by_node(&node), Some(test_addr(1))); + } + + #[test] + fn test_find_by_node_ignores_unidentified_connections() { + let mut pool: ConnectionPool<()> = ConnectionPool::new(7); + // No pubkey exchange yet, so no node address. + pool.insert(test_addr(1), test_conn(1, false)).unwrap(); + assert_eq!(pool.find_by_node(&test_node(1)), None); + } + + #[test] + fn test_find_by_node_returns_none_for_an_unconnected_node() { + let mut pool: ConnectionPool<()> = ConnectionPool::new(7); + let mut conn = test_conn(1, false); + conn.node_addr = Some(test_node(1)); + pool.insert(test_addr(1), conn).unwrap(); + assert_eq!(pool.find_by_node(&test_node(2)), None); + } + + /// Distinct nodes do not alias: each resolves to its own link address. + #[test] + fn test_find_by_node_distinguishes_two_nodes() { + let mut pool: ConnectionPool<()> = ConnectionPool::new(7); + let mut a = test_conn(1, false); + a.node_addr = Some(test_node(1)); + let mut b = test_conn(2, false); + b.node_addr = Some(test_node(2)); + pool.insert(test_addr(1), a).unwrap(); + pool.insert(test_addr(2), b).unwrap(); + + assert_eq!(pool.find_by_node(&test_node(1)), Some(test_addr(1))); + assert_eq!(pool.find_by_node(&test_node(2)), Some(test_addr(2))); + } + + /// The live link address is reported for a peer found under any of its + /// rotated aliases — what a caller needs when it has to name the peer's + /// current address rather than merely test for one. + #[test] + fn test_live_addr_of_node_reports_the_incumbent_not_the_alias() { + let mut pool: ConnectionPool<()> = ConnectionPool::new(7); + let node = test_node(1); + let mut conn = test_conn(1, false); + conn.node_addr = Some(node); + pool.insert(test_addr(1), conn).unwrap(); + + assert_eq!(pool.live_addr_of_node(&node), Some(test_ble_addr(1))); + assert!(!pool.contains(&test_addr(99))); + // An unconnected node has no incumbent, so the caller keeps whatever + // address it observed. + assert_eq!(pool.live_addr_of_node(&test_node(2)), None); + } + + #[test] + fn test_live_addr_of_node_ignores_unidentified_connections() { + let mut pool: ConnectionPool<()> = ConnectionPool::new(7); + pool.insert(test_addr(1), test_conn(1, false)).unwrap(); + assert_eq!(pool.live_addr_of_node(&test_node(1)), None); + } + + /// The regression this guards: without a node-identity check, N rotated + /// addresses for ONE peer become N pool entries and evict real peers. + /// With it, the caller sees the peer is already present and declines. + #[test] + fn test_rotated_addresses_would_otherwise_fill_the_pool() { + let mut pool: ConnectionPool<()> = ConnectionPool::new(7); + let node = test_node(1); + + let mut first = test_conn(1, false); + first.node_addr = Some(node); + pool.insert(test_addr(1), first).unwrap(); + + // Ten rotations arrive. Each is a distinct link address, so `contains` + // says "new" every time — but `find_by_node` recognises all of them. + for n in 2..12u8 { + assert!( + !pool.contains(&test_addr(n)), + "rotation {n} looks new by address" + ); + assert_eq!( + pool.find_by_node(&node), + Some(test_addr(1)), + "rotation {n} is recognised as the peer already connected", + ); + } + // Nothing was admitted, so the pool still holds exactly one link, and + // it is the incumbent — the first one, not the newest. + assert_eq!(pool.len(), 1); + assert!(pool.contains(&test_addr(1))); + } } diff --git a/src/transport/ble/stats.rs b/src/transport/ble/stats.rs index f4fbccf5..9e43a82d 100644 --- a/src/transport/ble/stats.rs +++ b/src/transport/ble/stats.rs @@ -23,6 +23,9 @@ pub struct BleStats { pub pool_evictions: AtomicU64, pub advertisements_sent: AtomicU64, pub scan_results: AtomicU64, + /// Connections declined because the peer was already linked on another + /// link address (see `ConnectionPool::find_by_node`). + pub duplicate_node_declines: AtomicU64, } impl BleStats { @@ -43,6 +46,7 @@ impl BleStats { pool_evictions: AtomicU64::new(0), advertisements_sent: AtomicU64::new(0), scan_results: AtomicU64::new(0), + duplicate_node_declines: AtomicU64::new(0), } } @@ -108,6 +112,16 @@ impl BleStats { self.scan_results.fetch_add(1, Ordering::Relaxed); } + /// Record a connection declined as a duplicate of a peer already linked + /// under a different link address. + /// + /// A peer using resolvable private addresses rotates continually, so a + /// climbing count against a busy mesh is that rotation being absorbed — + /// not an error. + pub fn record_duplicate_node_decline(&self) { + self.duplicate_node_declines.fetch_add(1, Ordering::Relaxed); + } + /// Take a snapshot of all counters. pub fn snapshot(&self) -> BleStatsSnapshot { BleStatsSnapshot { @@ -125,6 +139,7 @@ impl BleStats { pool_evictions: self.pool_evictions.load(Ordering::Relaxed), advertisements_sent: self.advertisements_sent.load(Ordering::Relaxed), scan_results: self.scan_results.load(Ordering::Relaxed), + duplicate_node_declines: self.duplicate_node_declines.load(Ordering::Relaxed), } } } @@ -152,4 +167,5 @@ pub struct BleStatsSnapshot { pub pool_evictions: u64, pub advertisements_sent: u64, pub scan_results: u64, + pub duplicate_node_declines: u64, } From ae93c9090882bafa68f5adb8b61590d1d67f10f7 Mon Sep 17 00:00:00 2001 From: fr34aky <162515565+fr34aky@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:30:22 +0100 Subject: [PATCH 3/9] feat(transport/ble): put the L2CAP PSM in the seam, and implement it for BlueZ The transport dialled every peer on one configured PSM and bound its own listener to the same one. That works only because BlueZ lets an application choose the PSM it binds, and BlueZ is the exception: Android's listenUsingInsecureL2capChannel and macOS's CBPeripheralManager.publishL2CAPChannel both return an OS-assigned PSM the application cannot request. A dialer cannot guess it, and before a connection exists there is no channel to be told it on other than the advertisement. So the PSM becomes a property of the seam rather than a per-backend assumption. BleIo::listen reports the PSM it actually bound, start_advertising takes the PSM to advertise, and BleScanner yields a ScanAdvert -- address, plus PSM and RSSI when the backend can supply them -- instead of a bare address. The scan/probe loop keeps the learned PSM per address alongside the probe-cooldown map it already maintains and passes it into the existing connect(addr, psm), falling back to the configured PSM when a peer advertises none. The wire layout is a protocol decision and is documented in psm.rs with the byte budget that forces it. A legacy advertising PDU carries 31 bytes; flags take 3 and the 128-bit FIPS service UUID takes 18, which leaves too little for service data keyed on that same 128-bit UUID. The PSM is therefore keyed on the 16-bit UUID 0x9C90, the FIPS UUID's leading 16 bits through the Bluetooth base UUID, costing 6 bytes for a total of 27. The budget is a const assertion, so a change back to a 128-bit key fails the build rather than the radio. It rides the primary advertisement, never the scan response, because a scan response needs an active-scan round trip that drops asymmetrically across chipsets. The BlueZ backend now advertises and reads that service data, which is what lets a BlueZ node tell an Android peer where to dial and learn the peer's OS-assigned PSM in return. Emitting it costs the local_name, which no longer fits the budget. Nothing reads a peer's advertised name -- discovery keys on the service UUID alone, here and on maint -- so dropping it does not affect which nodes can find each other. Compatibility with deployed nodes is unchanged in both directions. BlueZ listeners still bind the configured PSM, so an existing node dialling that PSM still connects. A peer that advertises no service data yields psm: None and is dialled at the configured PSM exactly as before. BleConfig::psm keeps its type, default and meaning; only its doc comment changes to say it is now what to bind and what to dial when a peer advertises nothing. BlueZ shortens a base-range 128-bit UUID to its 16-bit form before building the AD structure, so the service data goes out as the 6-byte AD type 0x16 the layout requires rather than the 20-byte 0x21 form, and the advert stays inside the PDU. Read in BlueZ 5.72: bt_string_to_uuid tests is_base_uuid128 first (lib/uuid.c), and serialize_service_data emits BT_AD_SERVICE_DATA16 for a 2-byte UUID (src/shared/ad.c). This was the open question the change was held on. The BlueZ implementation moves out of io.rs into io_linux.rs at the same time. io.rs now holds only what is platform-neutral -- the traits, ScanAdvert and the mock -- so a new backend is a new io_.rs beside it rather than another arm inside the shared file. The move is content-preserving: the only changes to the relocated code are three import paths and two rustfmt reflows caused by the dedent. Co-authored-by: Arjen <18398758+Origami74@users.noreply.github.com> --- docs/reference/configuration.md | 11 +- src/config/transport.rs | 6 + src/node/mod.rs | 2 +- src/transport/ble/io.rs | 545 ++++++++----------------------- src/transport/ble/io_linux.rs | 456 ++++++++++++++++++++++++++ src/transport/ble/mod.rs | 216 +++++++++++- src/transport/ble/neighbor.rs | 7 +- src/transport/ble/psm.rs | 176 ++++++++++ src/transport/ble/stream_read.rs | 2 +- 9 files changed, 993 insertions(+), 428 deletions(-) create mode 100644 src/transport/ble/io_linux.rs create mode 100644 src/transport/ble/psm.rs diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 248e8cec..a61710d7 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -747,8 +747,15 @@ entries become no-ops. Communicates with BlueZ via D-Bus through the **Advertising and scanning.** When `advertise` is enabled, the transport advertises the FIPS service UUID continuously so that nearby nodes can -discover and connect via L2CAP. When `scan` is enabled, the transport -continuously scans for other FIPS nodes' advertisements. Discovered +discover and connect via L2CAP, plus the L2CAP PSM its listener actually +bound, as a service-data structure (see `src/transport/ble/psm.rs` for the +wire layout and why platforms with OS-assigned PSMs need it). The +advertisement carries no device name — alongside the PSM a name no longer +fits the 31-byte legacy PDU, so the node shows up in generic Bluetooth +scanners as an unnamed device with the FIPS UUID. When `scan` is enabled, +the transport continuously scans for other FIPS nodes' advertisements and +learns each peer's advertised PSM; a peer that advertises none is dialled +at the configured `psm`. Discovered peers are probed immediately (L2CAP connect + pubkey exchange) with a cooldown (`probe_cooldown_secs`) to prevent rapid re-probing of the same address. If two nodes probe each other at the same time (cross-probe), diff --git a/src/config/transport.rs b/src/config/transport.rs index dcb00459..6bc3d380 100644 --- a/src/config/transport.rs +++ b/src/config/transport.rs @@ -711,6 +711,12 @@ pub struct BleConfig { pub adapter: Option, /// L2CAP PSM for FIPS connections. Default: 0x0085 (133). + /// + /// This is the PSM to request for this node's listener, and the PSM to + /// dial a peer at when that peer advertises none. It is not always the + /// PSM finally used: platforms that assign listener PSMs themselves + /// report back what they bound and advertise that, and a peer that + /// advertises its own PSM is dialled there instead. #[serde(default, skip_serializing_if = "Option::is_none")] pub psm: Option, diff --git a/src/node/mod.rs b/src/node/mod.rs index 43641fa1..ab0c30f9 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -1164,7 +1164,7 @@ impl Node { let transport_id = self.allocate_transport_id(); let adapter = ble_config.adapter().to_string(); let mtu = ble_config.mtu(); - match crate::transport::ble::io::BluerIo::new(&adapter, mtu).await { + match crate::transport::ble::io_linux::BluerIo::new(&adapter, mtu).await { Ok(io) => { let mut ble = crate::transport::ble::BleTransport::new( transport_id, diff --git a/src/transport/ble/io.rs b/src/transport/ble/io.rs index 6b053492..c7d60408 100644 --- a/src/transport/ble/io.rs +++ b/src/transport/ble/io.rs @@ -1,8 +1,9 @@ //! BLE I/O abstraction layer. //! -//! Defines the `BleIo` trait that separates transport logic from the -//! BlueZ/bluer stack. `BluerIo` (behind `cfg(bluer_available)`) provides -//! the real implementation; `MockBleIo` provides an in-memory test double. +//! Defines the `BleIo` seam that separates transport logic from any one +//! radio stack, and the in-memory `MockBleIo` test double. Everything in +//! this file is platform-neutral; each concrete backend lives beside it in +//! its own `io_.rs` — [`super::io_linux`] for BlueZ. use crate::transport::TransportError; @@ -58,12 +59,53 @@ pub trait BleAcceptor: Send { ) -> impl std::future::Future> + Send; } +/// One advertisement observed by a scanner. +/// +/// Carries what the backend could read from the advert, not what it wishes +/// were there: a backend that cannot surface a field reports `None` for it, +/// the way `TransportHandle::local_addr` already does for transports that +/// have no address to give. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ScanAdvert { + /// The advertiser's link address. + pub addr: BleAddr, + /// The L2CAP listener PSM the peer advertised, if it advertised one. + /// + /// `None` for a legacy UUID-only advertiser, and for a backend that + /// cannot read advertised service data. The dialer then falls back to the + /// configured PSM. See [`super::psm`] for the wire layout. + pub psm: Option, + /// Received signal strength in dBm, if the backend reports it. + pub rssi: Option, +} + +impl ScanAdvert { + /// An advert carrying nothing but a link address — what a legacy + /// UUID-only advertiser produces. + pub fn new(addr: BleAddr) -> Self { + Self { + addr, + psm: None, + rssi: None, + } + } + + /// An advert carrying a listener PSM. + pub fn with_psm(addr: BleAddr, psm: u16) -> Self { + Self { + addr, + psm: Some(psm), + rssi: None, + } + } +} + /// A scanner that yields discovered BLE devices advertising the FIPS UUID. pub trait BleScanner: Send { - /// Wait for the next discovered device. + /// Wait for the next observed advertisement. /// /// Returns `None` when scanning is stopped. - fn next(&mut self) -> impl std::future::Future> + Send; + fn next(&mut self) -> impl std::future::Future> + Send; } /// Core BLE I/O operations. @@ -79,11 +121,19 @@ pub trait BleIo: Send + Sync + 'static { /// The concrete scanner type. type Scanner: BleScanner + 'static; - /// Start listening for inbound L2CAP connections on the given PSM. + /// Start listening for inbound L2CAP connections, and report the PSM + /// actually bound. + /// + /// `psm` is the PSM to request. Backends that let an application choose + /// one (BlueZ) bind it and report it back unchanged. Backends whose + /// platform assigns the PSM (Android, macOS) ignore the request and + /// report what the OS gave them — which is why this returns a value + /// rather than being assumed equal to the argument. The reported PSM is + /// what gets advertised. fn listen( &self, psm: u16, - ) -> impl std::future::Future> + Send; + ) -> impl std::future::Future> + Send; /// Connect to a remote BLE device on the given PSM. fn connect( @@ -92,9 +142,15 @@ pub trait BleIo: Send + Sync + 'static { psm: u16, ) -> impl std::future::Future> + Send; - /// Start advertising the FIPS service UUID. + /// Start advertising the FIPS service UUID, and the listener PSM. + /// + /// `psm` is the PSM this node's listener is bound to; see [`super::psm`] + /// for the wire layout it should be advertised in. A backend that cannot + /// put it in its advert ignores the argument, and peers dial it at their + /// configured PSM as before. fn start_advertising( &self, + psm: u16, ) -> impl std::future::Future> + Send; /// Stop advertising. @@ -114,390 +170,6 @@ pub trait BleIo: Send + Sync + 'static { fn adapter_name(&self) -> &str; } -// ============================================================================ -// BluerIo — Production BLE I/O via BlueZ D-Bus -// ============================================================================ - -#[cfg(bluer_available)] -mod bluer_impl { - use super::*; - use crate::transport::TransportError; - - use bluer::l2cap::{SeqPacket, SeqPacketListener, Socket, SocketAddr}; - use bluer::{ - AdapterEvent, AddressType, DiscoveryFilter, DiscoveryTransport, adv::Advertisement, - }; - use futures::StreamExt; - use std::collections::{BTreeSet, HashSet}; - use std::pin::Pin; - use tokio::sync::Mutex; - use tracing::{debug, trace}; - - /// FIPS BLE service UUID. - /// - /// Derived from SHA-256("FIPS: welcome to cryptoanarchy") with UUID v4 - /// version/variant bits applied. - pub const FIPS_SERVICE_UUID: bluer::Uuid = - bluer::Uuid::from_u128(0x9c90_b790_2cc5_42c0_9f87_c9cc_4064_8f4c); - - /// Map a bluer error to a TransportError. - fn map_err(context: &str, e: bluer::Error) -> TransportError { - TransportError::Io(std::io::Error::other(format!("{}: {}", context, e))) - } - - /// Map a std::io::Error to a TransportError. - fn map_io_err(context: &str, e: std::io::Error) -> TransportError { - TransportError::Io(std::io::Error::new(e.kind(), format!("{}: {}", context, e))) - } - - // ---------------------------------------------------------------- - // BluerStream - // ---------------------------------------------------------------- - - /// BLE stream wrapping a bluer L2CAP SeqPacket connection. - pub struct BluerStream { - conn: SeqPacket, - remote: BleAddr, - send_mtu: u16, - recv_mtu: u16, - } - - impl BluerStream { - /// Construct from a connected SeqPacket, querying MTU values. - pub fn new(conn: SeqPacket, remote: BleAddr) -> Result { - let send_mtu = conn.send_mtu().map_err(|e| map_io_err("send_mtu", e))? as u16; - let recv_mtu = conn.recv_mtu().map_err(|e| map_io_err("recv_mtu", e))? as u16; - - // Log negotiated PHY for diagnostics (2M vs 1M) - match conn.as_ref().phy() { - Ok(phy) => { - debug!(addr = %remote, phy, send_mtu, recv_mtu, "BLE connection established") - } - Err(_) => { - debug!(addr = %remote, send_mtu, recv_mtu, "BLE connection established (PHY query unsupported)") - } - } - - Ok(Self { - conn, - remote, - send_mtu, - recv_mtu, - }) - } - } - - impl BleStream for BluerStream { - async fn send(&self, data: &[u8]) -> Result<(), TransportError> { - self.conn - .send(data) - .await - .map(|_| ()) - .map_err(|e| TransportError::SendFailed(format!("{}", e))) - } - - async fn recv(&self, buf: &mut [u8]) -> Result { - self.conn - .recv(buf) - .await - .map_err(|e| TransportError::RecvFailed(format!("{}", e))) - } - - fn send_mtu(&self) -> u16 { - self.send_mtu - } - - fn recv_mtu(&self) -> u16 { - self.recv_mtu - } - - fn remote_addr(&self) -> &BleAddr { - &self.remote - } - } - - // ---------------------------------------------------------------- - // BluerAcceptor - // ---------------------------------------------------------------- - - /// Acceptor wrapping a bluer L2CAP SeqPacketListener. - pub struct BluerAcceptor { - listener: SeqPacketListener, - adapter_name: String, - } - - impl BleAcceptor for BluerAcceptor { - type Stream = BluerStream; - - async fn accept(&mut self) -> Result { - let (conn, peer_sa) = self - .listener - .accept() - .await - .map_err(|e| map_io_err("accept", e))?; - - let remote = BleAddr::from_bluer(peer_sa.addr, &self.adapter_name); - BluerStream::new(conn, remote) - } - } - - // ---------------------------------------------------------------- - // BluerScanner - // ---------------------------------------------------------------- - - /// Scanner wrapping a bluer discovery event stream. - pub struct BluerScanner { - events: Pin + Send>>, - adapter: bluer::Adapter, - adapter_name: String, - } - - impl BleScanner for BluerScanner { - async fn next(&mut self) -> Option { - loop { - match self.events.next().await { - Some(AdapterEvent::DeviceAdded(addr)) => { - // Check if device advertises FIPS UUID - if let Ok(device) = self.adapter.device(addr) { - match device.uuids().await { - Ok(Some(uuids)) if uuids.contains(&FIPS_SERVICE_UUID) => { - let ble_addr = BleAddr::from_bluer(addr, &self.adapter_name); - debug!(addr = %ble_addr, "BLE scanner: FIPS peer found"); - return Some(ble_addr); - } - Ok(_) => { - trace!(addr = %addr, "BLE scanner: device without FIPS UUID"); - } - Err(e) => { - trace!(addr = %addr, error = %e, "BLE scanner: failed to read UUIDs"); - } - } - } - } - Some(_) => continue, - None => return None, - } - } - } - } - - // ---------------------------------------------------------------- - // BluerIo - // ---------------------------------------------------------------- - - /// Production BLE I/O implementation via BlueZ D-Bus (bluer crate). - pub struct BluerIo { - #[allow(dead_code)] // Session must be kept alive for the adapter. - session: bluer::Session, - adapter: bluer::Adapter, - adapter_name: String, - adv_handle: Mutex>, - mtu: u16, - } - - impl BluerIo { - /// Create a new BluerIo for the given adapter. - /// - /// Connects to BlueZ via D-Bus and powers on the adapter. - pub async fn new(adapter_name: &str, mtu: u16) -> Result { - let session = bluer::Session::new() - .await - .map_err(|e| map_err("Session::new", e))?; - - let adapter = if adapter_name == "default" { - session - .default_adapter() - .await - .map_err(|e| map_err("default_adapter", e))? - } else { - session - .adapter(adapter_name) - .map_err(|e| map_err("adapter", e))? - }; - - adapter - .set_powered(true) - .await - .map_err(|e| map_err("set_powered", e))?; - - let name = adapter.name().to_string(); - debug!(adapter = %name, "BluerIo initialized"); - - Ok(Self { - session, - adapter, - adapter_name: name, - adv_handle: Mutex::new(None), - mtu, - }) - } - } - - impl BleIo for BluerIo { - type Stream = BluerStream; - type Acceptor = BluerAcceptor; - type Scanner = BluerScanner; - - async fn listen(&self, psm: u16) -> Result { - let local_addr = self - .adapter - .address() - .await - .map_err(|e| map_err("address", e))?; - - let sa = SocketAddr::new(local_addr, AddressType::LePublic, psm); - let listener = SeqPacketListener::bind(sa) - .await - .map_err(|e| map_io_err("bind", e))?; - - // Request high MTU for accepted connections - listener - .as_ref() - .set_recv_mtu(self.mtu) - .map_err(|e| map_io_err("set_recv_mtu", e))?; - - // Prevent sniff mode to reduce latency during data transfer - if let Err(e) = listener.as_ref().set_power_forced_active(true) { - debug!(error = %e, "BLE listener: set_power_forced_active not supported"); - } - - debug!(psm, mtu = self.mtu, "BLE listener bound"); - - Ok(BluerAcceptor { - listener, - adapter_name: self.adapter_name.clone(), - }) - } - - async fn connect(&self, addr: &BleAddr, psm: u16) -> Result { - let target_sa = addr.to_socket_addr(psm); - - let socket = Socket::::new_seq_packet() - .map_err(|e| map_io_err("new_seq_packet", e))?; - socket - .bind(SocketAddr::any_le()) - .map_err(|e| map_io_err("bind", e))?; - socket - .set_recv_mtu(self.mtu) - .map_err(|e| map_io_err("set_recv_mtu", e))?; - - // Prevent sniff mode to reduce latency during data transfer - if let Err(e) = socket.set_power_forced_active(true) { - debug!(error = %e, "BLE connect: set_power_forced_active not supported"); - } - - let conn = socket - .connect(target_sa) - .await - .map_err(|e| map_io_err("connect", e))?; - - let remote = addr.clone(); - BluerStream::new(conn, remote) - } - - async fn start_advertising(&self) -> Result<(), TransportError> { - let adv = Advertisement { - advertisement_type: bluer::adv::Type::Peripheral, - service_uuids: { - let mut s = BTreeSet::new(); - s.insert(FIPS_SERVICE_UUID); - s - }, - local_name: Some("fips".to_string()), - min_interval: Some(std::time::Duration::from_millis(400)), - max_interval: Some(std::time::Duration::from_millis(600)), - ..Default::default() - }; - - let handle = self - .adapter - .advertise(adv) - .await - .map_err(|e| map_err("advertise", e))?; - - *self.adv_handle.lock().await = Some(handle); - debug!("BLE advertising started"); - Ok(()) - } - - async fn stop_advertising(&self) -> Result<(), TransportError> { - let _ = self.adv_handle.lock().await.take(); - debug!("BLE advertising stopped"); - Ok(()) - } - - async fn start_scanning(&self) -> Result { - // Clear cached devices so BlueZ fires DeviceAdded for every - // advertisement. Without this, already-known devices only - // produce PropertyChanged events (which bluer doesn't expose - // at the device level), causing the scanner to miss peers - // after a daemon restart. - if let Ok(cached) = self.adapter.device_addresses().await { - let count = cached.len(); - for addr in cached { - let _ = self.adapter.remove_device(addr).await; - } - if count > 0 { - debug!(count, "BLE scanner: cleared cached devices"); - } - } - - // Set discovery filter for LE transport with FIPS UUID - let filter = DiscoveryFilter { - transport: DiscoveryTransport::Le, - uuids: { - let mut s = HashSet::new(); - s.insert(FIPS_SERVICE_UUID); - s - }, - ..Default::default() - }; - - self.adapter - .set_discovery_filter(filter) - .await - .map_err(|e| map_err("set_discovery_filter", e))?; - - let events = self - .adapter - .discover_devices() - .await - .map_err(|e| map_err("discover_devices", e))?; - - debug!("BLE scanning started"); - - Ok(BluerScanner { - events: Box::pin(events), - adapter: self.adapter.clone(), - adapter_name: self.adapter_name.clone(), - }) - } - - fn local_addr(&self) -> Result { - // Use futures::executor::block_on since this is a sync method - // but needs an async call. The adapter address is cached so - // the D-Bus call is fast. - let addr = futures::executor::block_on(self.adapter.address()) - .map_err(|e| map_err("address", e))?; - Ok(BleAddr::from_bluer(addr, &self.adapter_name)) - } - - fn adapter_name(&self) -> &str { - &self.adapter_name - } - } - - // Compile-time assertion that BluerIo satisfies Send + Sync. - #[allow(dead_code)] - fn _assert_bluer_io_send_sync() { - fn require() {} - require::(); - } -} - -#[cfg(bluer_available)] -pub use bluer_impl::{BluerAcceptor, BluerIo, BluerScanner, BluerStream, FIPS_SERVICE_UUID}; - // ============================================================================ // Mock BLE I/O (for testing without hardware) // ============================================================================ @@ -583,13 +255,13 @@ impl BleAcceptor for MockBleAcceptor { } } -/// Mock BLE scanner backed by a channel of discovered addresses. +/// Mock BLE scanner backed by a channel of observed adverts. pub struct MockBleScanner { - rx: tokio::sync::mpsc::Receiver, + rx: tokio::sync::mpsc::Receiver, } impl BleScanner for MockBleScanner { - async fn next(&mut self) -> Option { + async fn next(&mut self) -> Option { self.rx.recv().await } } @@ -607,9 +279,15 @@ pub struct MockBleIo { local_addr: BleAddr, accept_tx: tokio::sync::mpsc::Sender, accept_rx: std::sync::Mutex>>, - scan_tx: tokio::sync::mpsc::Sender, - scan_rx: std::sync::Mutex>>, + scan_tx: tokio::sync::mpsc::Sender, + scan_rx: std::sync::Mutex>>, connect_handler: std::sync::Mutex>, + /// PSM `listen` reports back, overriding the requested one. + /// + /// Simulates a platform that assigns the PSM itself. + bound_psm: std::sync::Mutex>, + /// PSM most recently passed to `start_advertising`. + advertised_psm: std::sync::Mutex>, } impl MockBleIo { @@ -625,6 +303,8 @@ impl MockBleIo { scan_tx, scan_rx: std::sync::Mutex::new(Some(scan_rx)), connect_handler: std::sync::Mutex::new(None), + bound_psm: std::sync::Mutex::new(None), + advertised_psm: std::sync::Mutex::new(None), } } @@ -633,9 +313,29 @@ impl MockBleIo { let _ = self.accept_tx.send(stream).await; } - /// Inject a scan result (simulates discovering a remote device). + /// Inject a scan result (simulates discovering a legacy UUID-only + /// advertiser, which carries no PSM). pub async fn inject_scan_result(&self, addr: BleAddr) { - let _ = self.scan_tx.send(addr).await; + self.inject_scan_advert(ScanAdvert::new(addr)).await; + } + + /// Inject an observed advertisement verbatim. + pub async fn inject_scan_advert(&self, advert: ScanAdvert) { + let _ = self.scan_tx.send(advert).await; + } + + /// Make `listen` report a PSM other than the one requested, the way a + /// platform that assigns PSMs itself would. + pub fn set_bound_psm(&self, psm: u16) { + *self.bound_psm.lock().unwrap_or_else(|e| e.into_inner()) = Some(psm); + } + + /// The PSM most recently handed to `start_advertising`. + pub fn advertised_psm(&self) -> Option { + *self + .advertised_psm + .lock() + .unwrap_or_else(|e| e.into_inner()) } /// Set a handler for outbound connect calls. @@ -655,14 +355,19 @@ impl BleIo for MockBleIo { type Acceptor = MockBleAcceptor; type Scanner = MockBleScanner; - async fn listen(&self, _psm: u16) -> Result { + async fn listen(&self, psm: u16) -> Result<(Self::Acceptor, u16), TransportError> { let rx = self .accept_rx .lock() .unwrap() .take() .ok_or_else(|| TransportError::NotSupported("acceptor already taken".into()))?; - Ok(MockBleAcceptor { rx }) + let bound = self + .bound_psm + .lock() + .unwrap_or_else(|e| e.into_inner()) + .unwrap_or(psm); + Ok((MockBleAcceptor { rx }, bound)) } async fn connect(&self, addr: &BleAddr, psm: u16) -> Result { @@ -676,7 +381,11 @@ impl BleIo for MockBleIo { } } - async fn start_advertising(&self) -> Result<(), TransportError> { + async fn start_advertising(&self, psm: u16) -> Result<(), TransportError> { + *self + .advertised_psm + .lock() + .unwrap_or_else(|e| e.into_inner()) = Some(psm); Ok(()) } @@ -751,7 +460,8 @@ mod tests { #[tokio::test] async fn test_mock_io_listen_accept() { let io = MockBleIo::new("hci0", test_addr(1)); - let mut acceptor = io.listen(0x0085).await.unwrap(); + let (mut acceptor, bound) = io.listen(0x0085).await.unwrap(); + assert_eq!(bound, 0x0085, "mock binds what it is asked for by default"); let (stream_a, _stream_b) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); io.inject_inbound(stream_a).await; @@ -789,8 +499,8 @@ mod tests { io.inject_scan_result(test_addr(2)).await; io.inject_scan_result(test_addr(3)).await; - assert_eq!(scanner.next().await, Some(test_addr(2))); - assert_eq!(scanner.next().await, Some(test_addr(3))); + assert_eq!(scanner.next().await, Some(ScanAdvert::new(test_addr(2)))); + assert_eq!(scanner.next().await, Some(ScanAdvert::new(test_addr(3)))); } #[tokio::test] @@ -803,10 +513,33 @@ mod tests { #[tokio::test] async fn test_mock_io_advertising_noop() { let io = MockBleIo::new("hci0", test_addr(1)); - io.start_advertising().await.unwrap(); + io.start_advertising(0x0085).await.unwrap(); + assert_eq!(io.advertised_psm(), Some(0x0085)); io.stop_advertising().await.unwrap(); } + /// A backend whose platform assigns the PSM reports back something other + /// than what was requested — the case the return value exists for. + #[tokio::test] + async fn test_mock_io_listen_reports_an_os_assigned_psm() { + let io = MockBleIo::new("hci0", test_addr(1)); + io.set_bound_psm(0x00C1); + let (_acceptor, bound) = io.listen(0x0085).await.unwrap(); + assert_eq!(bound, 0x00C1); + } + + #[tokio::test] + async fn test_mock_io_scan_advert_carries_a_psm() { + let io = MockBleIo::new("hci0", test_addr(1)); + let mut scanner = io.start_scanning().await.unwrap(); + io.inject_scan_advert(ScanAdvert::with_psm(test_addr(2), 0x00C1)) + .await; + let advert = scanner.next().await.unwrap(); + assert_eq!(advert.addr, test_addr(2)); + assert_eq!(advert.psm, Some(0x00C1)); + assert_eq!(advert.rssi, None); + } + #[tokio::test] async fn test_mock_io_listen_twice_fails() { let io = MockBleIo::new("hci0", test_addr(1)); diff --git a/src/transport/ble/io_linux.rs b/src/transport/ble/io_linux.rs new file mode 100644 index 00000000..60d7605e --- /dev/null +++ b/src/transport/ble/io_linux.rs @@ -0,0 +1,456 @@ +//! BlueZ backend for the BLE transport. +//! +//! The Linux implementation of the [`BleIo`](super::io::BleIo) seam, over the +//! `bluer` crate's D-Bus binding to BlueZ. Everything platform-neutral lives +//! in [`super::io`] and the modules beside it; this file holds only what +//! speaks to BlueZ. + +use super::io::*; +use crate::transport::TransportError; + +use bluer::l2cap::{SeqPacket, SeqPacketListener, Socket, SocketAddr}; +use bluer::{AdapterEvent, AddressType, DiscoveryFilter, DiscoveryTransport, adv::Advertisement}; +use futures::StreamExt; +use std::collections::{BTreeMap, BTreeSet, HashSet}; +use std::pin::Pin; +use tokio::sync::Mutex; +use tracing::{debug, trace}; + +use super::addr::BleAddr; +use super::psm; + +/// FIPS BLE service UUID. +/// +/// Derived from SHA-256("FIPS: welcome to cryptoanarchy") with UUID v4 +/// version/variant bits applied. +pub const FIPS_SERVICE_UUID: bluer::Uuid = + bluer::Uuid::from_u128(0x9c90_b790_2cc5_42c0_9f87_c9cc_4064_8f4c); + +/// The PSM service-data key as a whole UUID. +/// +/// BlueZ speaks in full UUIDs, so [`psm::PSM_SERVICE_DATA_UUID16`] is +/// expanded through the Bluetooth base UUID +/// (`00009C90-0000-1000-8000-00805F9B34FB`). The controller emits it +/// back on the air as the 16-bit Service Data AD structure the wire +/// layout in [`psm`] specifies. +pub const PSM_SERVICE_DATA_UUID: bluer::Uuid = bluer::Uuid::from_u128( + ((psm::PSM_SERVICE_DATA_UUID16 as u128) << 96) | 0x0000_0000_0000_1000_8000_0080_5F9B_34FB, +); + +/// Map a bluer error to a TransportError. +fn map_err(context: &str, e: bluer::Error) -> TransportError { + TransportError::Io(std::io::Error::other(format!("{}: {}", context, e))) +} + +/// Map a std::io::Error to a TransportError. +fn map_io_err(context: &str, e: std::io::Error) -> TransportError { + TransportError::Io(std::io::Error::new(e.kind(), format!("{}: {}", context, e))) +} + +// ---------------------------------------------------------------- +// BluerStream +// ---------------------------------------------------------------- + +/// BLE stream wrapping a bluer L2CAP SeqPacket connection. +pub struct BluerStream { + conn: SeqPacket, + remote: BleAddr, + send_mtu: u16, + recv_mtu: u16, +} + +impl BluerStream { + /// Construct from a connected SeqPacket, querying MTU values. + pub fn new(conn: SeqPacket, remote: BleAddr) -> Result { + let send_mtu = conn.send_mtu().map_err(|e| map_io_err("send_mtu", e))? as u16; + let recv_mtu = conn.recv_mtu().map_err(|e| map_io_err("recv_mtu", e))? as u16; + + // Log negotiated PHY for diagnostics (2M vs 1M) + match conn.as_ref().phy() { + Ok(phy) => { + debug!(addr = %remote, phy, send_mtu, recv_mtu, "BLE connection established") + } + Err(_) => { + debug!(addr = %remote, send_mtu, recv_mtu, "BLE connection established (PHY query unsupported)") + } + } + + Ok(Self { + conn, + remote, + send_mtu, + recv_mtu, + }) + } +} + +impl BleStream for BluerStream { + async fn send(&self, data: &[u8]) -> Result<(), TransportError> { + self.conn + .send(data) + .await + .map(|_| ()) + .map_err(|e| TransportError::SendFailed(format!("{}", e))) + } + + async fn recv(&self, buf: &mut [u8]) -> Result { + self.conn + .recv(buf) + .await + .map_err(|e| TransportError::RecvFailed(format!("{}", e))) + } + + fn send_mtu(&self) -> u16 { + self.send_mtu + } + + fn recv_mtu(&self) -> u16 { + self.recv_mtu + } + + fn remote_addr(&self) -> &BleAddr { + &self.remote + } +} + +// ---------------------------------------------------------------- +// BluerAcceptor +// ---------------------------------------------------------------- + +/// Acceptor wrapping a bluer L2CAP SeqPacketListener. +pub struct BluerAcceptor { + listener: SeqPacketListener, + adapter_name: String, +} + +impl BleAcceptor for BluerAcceptor { + type Stream = BluerStream; + + async fn accept(&mut self) -> Result { + let (conn, peer_sa) = self + .listener + .accept() + .await + .map_err(|e| map_io_err("accept", e))?; + + let remote = BleAddr::from_bluer(peer_sa.addr, &self.adapter_name); + BluerStream::new(conn, remote) + } +} + +// ---------------------------------------------------------------- +// BluerScanner +// ---------------------------------------------------------------- + +/// Scanner wrapping a bluer discovery event stream. +pub struct BluerScanner { + events: Pin + Send>>, + adapter: bluer::Adapter, + adapter_name: String, +} + +impl BleScanner for BluerScanner { + /// Yields adverts with the PSM and RSSI when BlueZ can supply them. + /// + /// The PSM comes out of the peer's Service Data AD structure (see + /// `super::super::psm`); a peer that advertises none — a legacy + /// UUID-only advertiser — yields `psm: None` and is dialled at the + /// configured PSM, exactly as before. + async fn next(&mut self) -> Option { + loop { + match self.events.next().await { + Some(AdapterEvent::DeviceAdded(addr)) => { + // Check if device advertises FIPS UUID + if let Ok(device) = self.adapter.device(addr) { + match device.uuids().await { + Ok(Some(uuids)) if uuids.contains(&FIPS_SERVICE_UUID) => { + let ble_addr = BleAddr::from_bluer(addr, &self.adapter_name); + let psm = + device.service_data().await.ok().flatten().and_then(|sd| { + sd.get(&PSM_SERVICE_DATA_UUID) + .and_then(|data| psm::decode_psm(data)) + }); + let rssi = device.rssi().await.ok().flatten(); + debug!(addr = %ble_addr, ?psm, ?rssi, "BLE scanner: FIPS peer found"); + return Some(ScanAdvert { + addr: ble_addr, + psm, + rssi, + }); + } + Ok(_) => { + trace!(addr = %addr, "BLE scanner: device without FIPS UUID"); + } + Err(e) => { + trace!(addr = %addr, error = %e, "BLE scanner: failed to read UUIDs"); + } + } + } + } + Some(_) => continue, + None => return None, + } + } + } +} + +// ---------------------------------------------------------------- +// BluerIo +// ---------------------------------------------------------------- + +/// Production BLE I/O implementation via BlueZ D-Bus (bluer crate). +pub struct BluerIo { + #[allow(dead_code)] // Session must be kept alive for the adapter. + session: bluer::Session, + adapter: bluer::Adapter, + adapter_name: String, + adv_handle: Mutex>, + mtu: u16, +} + +impl BluerIo { + /// Create a new BluerIo for the given adapter. + /// + /// Connects to BlueZ via D-Bus and powers on the adapter. + pub async fn new(adapter_name: &str, mtu: u16) -> Result { + let session = bluer::Session::new() + .await + .map_err(|e| map_err("Session::new", e))?; + + let adapter = if adapter_name == "default" { + session + .default_adapter() + .await + .map_err(|e| map_err("default_adapter", e))? + } else { + session + .adapter(adapter_name) + .map_err(|e| map_err("adapter", e))? + }; + + adapter + .set_powered(true) + .await + .map_err(|e| map_err("set_powered", e))?; + + let name = adapter.name().to_string(); + debug!(adapter = %name, "BluerIo initialized"); + + Ok(Self { + session, + adapter, + adapter_name: name, + adv_handle: Mutex::new(None), + mtu, + }) + } +} + +impl BleIo for BluerIo { + type Stream = BluerStream; + type Acceptor = BluerAcceptor; + type Scanner = BluerScanner; + + /// Binds the requested PSM and reports it back unchanged. + /// + /// BlueZ lets an application choose the PSM it binds, so the bound + /// PSM is always the requested one. Backends whose platform assigns + /// the PSM report something else; that is the reason for the return + /// value, not anything BlueZ does. + async fn listen(&self, psm: u16) -> Result<(Self::Acceptor, u16), TransportError> { + let local_addr = self + .adapter + .address() + .await + .map_err(|e| map_err("address", e))?; + + let sa = SocketAddr::new(local_addr, AddressType::LePublic, psm); + let listener = SeqPacketListener::bind(sa) + .await + .map_err(|e| map_io_err("bind", e))?; + + // Request high MTU for accepted connections + listener + .as_ref() + .set_recv_mtu(self.mtu) + .map_err(|e| map_io_err("set_recv_mtu", e))?; + + // Prevent sniff mode to reduce latency during data transfer + if let Err(e) = listener.as_ref().set_power_forced_active(true) { + debug!(error = %e, "BLE listener: set_power_forced_active not supported"); + } + + debug!(psm, mtu = self.mtu, "BLE listener bound"); + + Ok(( + BluerAcceptor { + listener, + adapter_name: self.adapter_name.clone(), + }, + psm, + )) + } + + async fn connect(&self, addr: &BleAddr, psm: u16) -> Result { + let target_sa = addr.to_socket_addr(psm); + + let socket = + Socket::::new_seq_packet().map_err(|e| map_io_err("new_seq_packet", e))?; + socket + .bind(SocketAddr::any_le()) + .map_err(|e| map_io_err("bind", e))?; + socket + .set_recv_mtu(self.mtu) + .map_err(|e| map_io_err("set_recv_mtu", e))?; + + // Prevent sniff mode to reduce latency during data transfer + if let Err(e) = socket.set_power_forced_active(true) { + debug!(error = %e, "BLE connect: set_power_forced_active not supported"); + } + + let conn = socket + .connect(target_sa) + .await + .map_err(|e| map_io_err("connect", e))?; + + let remote = addr.clone(); + BluerStream::new(conn, remote) + } + + /// Advertises the FIPS service UUID and the listener PSM. + /// + /// The PSM rides the Service Data AD structure specified in + /// `super::super::psm`. Emitting it costs the `local_name`: flags + /// (3) + 128-bit UUID list (18) + service data (6) fill 27 of the + /// 31-byte legacy PDU, and a name no longer fits. Peers that read + /// the service data dial the advertised PSM; legacy peers keep + /// dialling their configured one, which BlueZ listeners still bind. + /// + /// `super::super::psm` requires the PSM to ride the primary + /// advertisement, never the scan response, so a passive scanner + /// still sees it. Nothing here enforces that: BlueZ takes a set of + /// AD structures and chooses their placement itself. What keeps the + /// requirement holding is the arithmetic above — 27 of 31 bytes + /// used, so BlueZ has no reason to spill into the scan response — + /// and dropping the name is what makes it hold. + async fn start_advertising(&self, psm: u16) -> Result<(), TransportError> { + let adv = Advertisement { + advertisement_type: bluer::adv::Type::Peripheral, + service_uuids: { + let mut s = BTreeSet::new(); + s.insert(FIPS_SERVICE_UUID); + s + }, + service_data: { + let mut m = BTreeMap::new(); + m.insert(PSM_SERVICE_DATA_UUID, psm::encode_psm(psm).to_vec()); + m + }, + min_interval: Some(std::time::Duration::from_millis(400)), + max_interval: Some(std::time::Duration::from_millis(600)), + ..Default::default() + }; + + let handle = self + .adapter + .advertise(adv) + .await + .map_err(|e| map_err("advertise", e))?; + + *self.adv_handle.lock().await = Some(handle); + debug!(psm, "BLE advertising started"); + Ok(()) + } + + async fn stop_advertising(&self) -> Result<(), TransportError> { + let _ = self.adv_handle.lock().await.take(); + debug!("BLE advertising stopped"); + Ok(()) + } + + async fn start_scanning(&self) -> Result { + // Clear cached devices so BlueZ fires DeviceAdded for every + // advertisement. Without this, already-known devices only + // produce PropertyChanged events (which bluer doesn't expose + // at the device level), causing the scanner to miss peers + // after a daemon restart. + if let Ok(cached) = self.adapter.device_addresses().await { + let count = cached.len(); + for addr in cached { + let _ = self.adapter.remove_device(addr).await; + } + if count > 0 { + debug!(count, "BLE scanner: cleared cached devices"); + } + } + + // Set discovery filter for LE transport with FIPS UUID + let filter = DiscoveryFilter { + transport: DiscoveryTransport::Le, + uuids: { + let mut s = HashSet::new(); + s.insert(FIPS_SERVICE_UUID); + s + }, + ..Default::default() + }; + + self.adapter + .set_discovery_filter(filter) + .await + .map_err(|e| map_err("set_discovery_filter", e))?; + + let events = self + .adapter + .discover_devices() + .await + .map_err(|e| map_err("discover_devices", e))?; + + debug!("BLE scanning started"); + + Ok(BluerScanner { + events: Box::pin(events), + adapter: self.adapter.clone(), + adapter_name: self.adapter_name.clone(), + }) + } + + fn local_addr(&self) -> Result { + // Use futures::executor::block_on since this is a sync method + // but needs an async call. The adapter address is cached so + // the D-Bus call is fast. + let addr = futures::executor::block_on(self.adapter.address()) + .map_err(|e| map_err("address", e))?; + Ok(BleAddr::from_bluer(addr, &self.adapter_name)) + } + + fn adapter_name(&self) -> &str { + &self.adapter_name + } +} + +// Compile-time assertion that BluerIo satisfies Send + Sync. +#[allow(dead_code)] +fn _assert_bluer_io_send_sync() { + fn require() {} + require::(); +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A wrong shift in the base-UUID expansion would yield a plausible + /// UUID that simply never matches any peer — silent discovery + /// failure, not a build error. Companion to psm.rs's + /// `test_key_is_the_leading_16_bits_of_the_fips_uuid`. + #[test] + fn psm_service_data_uuid_expands_the_key_over_the_base_uuid() { + assert_eq!( + PSM_SERVICE_DATA_UUID, + "00009C90-0000-1000-8000-00805F9B34FB" + .parse::() + .unwrap() + ); + } +} diff --git a/src/transport/ble/mod.rs b/src/transport/ble/mod.rs index 16ab99db..61d6c916 100644 --- a/src/transport/ble/mod.rs +++ b/src/transport/ble/mod.rs @@ -11,7 +11,7 @@ //! may return a fragment of a packet or several packets coalesced from one //! read. The receive path therefore recovers boundaries from the FMP length //! prefix via [`stream_read::BleStreamRead`] and -//! [`crate::transport::framing::read_fmp_packet`], which is a transparent +//! `crate::transport::framing::read_fmp_packet`, which is a transparent //! pass-through on a boundary-preserving backend. //! //! ## Architecture @@ -29,8 +29,11 @@ pub mod addr; pub mod io; +#[cfg(bluer_available)] +pub mod io_linux; pub mod neighbor; pub mod pool; +pub mod psm; pub mod stats; pub mod stream_read; @@ -59,6 +62,12 @@ use tracing::{debug, info, trace, warn}; /// Default FIPS L2CAP PSM (Protocol Service Multiplexer). /// /// 0x0085 (133) is in the dynamic range (0x0080-0x00FF). +/// +/// This is a request and a fallback, not a guarantee. A backend whose +/// platform assigns the PSM reports back what it actually bound (see +/// [`io::BleIo::listen`]), and a peer that advertises its own PSM (see +/// [`psm`]) is dialled there instead. The configured value is what a peer is +/// dialled at when it advertises nothing. pub const DEFAULT_PSM: u16 = 0x0085; /// Concrete BLE transport type for use in TransportHandle. @@ -66,7 +75,7 @@ pub const DEFAULT_PSM: u16 = 0x0085; /// Production builds on glibc-linux use `BluerIo` (real BlueZ stack). /// Test builds, musl-linux, and non-Linux platforms use `MockBleIo`. #[cfg(all(bluer_available, not(test)))] -pub type DefaultBleTransport = BleTransport; +pub type DefaultBleTransport = BleTransport; #[cfg(any(not(bluer_available), test))] pub type DefaultBleTransport = BleTransport; @@ -177,9 +186,14 @@ impl BleTransport { } self.state = TransportState::Starting; - let psm = self.config.psm(); + let configured_psm = self.config.psm(); let adapter = self.io.adapter_name().to_string(); + // The PSM peers should dial us on. Only the listener knows it: a + // backend whose platform assigns PSMs reports back something other + // than what was requested, and that is what has to be advertised. + let mut listener_psm = configured_psm; + // Pre-compute local NodeAddr for cross-probe tie-breaking let local_node_addr = self.local_pubkey.and_then(|pk| { XOnlyPublicKey::from_slice(&pk) @@ -189,8 +203,9 @@ impl BleTransport { // Start L2CAP listener for inbound connections if self.config.accept_connections() { - match self.io.listen(psm).await { - Ok(acceptor) => { + match self.io.listen(configured_psm).await { + Ok((acceptor, bound_psm)) => { + listener_psm = bound_psm; let pool = Arc::clone(&self.pool); let packet_tx = self.packet_tx.clone(); let transport_id = self.transport_id; @@ -208,7 +223,12 @@ impl BleTransport { Arc::clone(&self.neighbor_buffer), local_node_addr, ))); - debug!(adapter = %adapter, psm = psm, "BLE accept loop started"); + debug!( + adapter = %adapter, + psm = listener_psm, + requested_psm = configured_psm, + "BLE accept loop started" + ); } Err(e) => { warn!(adapter = %adapter, error = %e, "failed to start BLE listener"); @@ -220,11 +240,15 @@ impl BleTransport { // Start continuous advertising if self.config.advertise() { - if let Err(e) = self.io.start_advertising().await { + if let Err(e) = self.io.start_advertising(listener_psm).await { warn!(adapter = %adapter, error = %e, "failed to start BLE advertising"); } else { self.stats.record_advertisement(); - debug!(adapter = %adapter, "BLE advertising started (continuous)"); + debug!( + adapter = %adapter, + psm = listener_psm, + "BLE advertising started (continuous)" + ); } } @@ -255,7 +279,7 @@ impl BleTransport { } self.state = TransportState::Up; - info!(adapter = %adapter, psm = psm, "BLE transport started"); + info!(adapter = %adapter, psm = listener_psm, "BLE transport started"); Ok(()) } @@ -999,7 +1023,7 @@ async fn scan_probe_loop( buffer: Arc, stats: Arc, local_pubkey: Option<[u8; 32]>, - psm: u16, + configured_psm: u16, connect_timeout_ms: u64, cooldown_secs: u64, local_node_addr: Option, @@ -1018,6 +1042,13 @@ async fn scan_probe_loop( // one per rotation, so entries are dropped once their node is no longer in // the pool — a peer that genuinely goes away is probed again normally. let mut known_node_of: HashMap = HashMap::new(); + // L2CAP listener PSMs read out of peers' advertisements. A peer whose + // platform assigns its listener PSM cannot be dialled at a configured + // constant, so it publishes the number it actually bound and we dial + // that. A peer that advertises nothing is dialled at `configured_psm`, + // which is every peer that predates this and every backend that does not + // advertise service data. + let mut learned_psm: HashMap = HashMap::new(); let cooldown = std::time::Duration::from_secs(cooldown_secs); let retry_interval = tokio::time::interval(std::time::Duration::from_secs(cooldown_secs)); tokio::pin!(retry_interval); @@ -1028,7 +1059,13 @@ async fn scan_probe_loop( let addr = tokio::select! { result = scanner.next() => { match result { - Some(a) => a, + Some(advert) => { + if let Some(psm) = advert.psm { + trace!(addr = %advert.addr, psm, "BLE scan: learned peer PSM"); + learned_psm.insert(advert.addr.clone(), psm); + } + advert.addr + } None => { debug!("BLE scanner ended"); break; @@ -1102,21 +1139,27 @@ async fn scan_probe_loop( } }; - // L2CAP connect + // L2CAP connect, at whatever PSM this peer advertised. + let dial_psm = learned_psm.get(&addr).copied().unwrap_or(configured_psm); let stream = match tokio::time::timeout( std::time::Duration::from_millis(connect_timeout_ms), - io.connect(&addr, psm), + io.connect(&addr, dial_psm), ) .await { Ok(Ok(s)) => s, Ok(Err(e)) => { - debug!(addr = %addr, error = %e, "BLE probe connect failed"); + debug!(addr = %addr, psm = dial_psm, error = %e, "BLE probe connect failed"); + // A learned PSM that does not answer is stale — forget it, so + // the next advert re-learns it and the fallback applies in the + // meantime. Costs one retry. + learned_psm.remove(&addr); continue; } Err(_) => { - debug!(addr = %addr, "BLE probe connect timeout"); + debug!(addr = %addr, psm = dial_psm, "BLE probe connect timeout"); stats.record_connect_timeout(); + learned_psm.remove(&addr); continue; } }; @@ -1735,6 +1778,149 @@ mod tests { transport.stop_async().await.unwrap(); } + // ------------------------------------------------------------------ + // Per-peer listener PSM + // ------------------------------------------------------------------ + + /// Every `(address, psm)` the transport tried to dial. + type DialLog = Arc>>; + + /// A scanning transport whose dials all fail, recording the PSM each was + /// attempted at. + fn psm_probe_transport( + dials: DialLog, + ) -> ( + BleTransport, + tokio::sync::mpsc::Receiver, + ) { + let io = MockBleIo::new("hci0", test_addr(1)); + io.set_connect_handler(move |addr, psm| { + dials.lock().unwrap().push((addr.clone(), psm)); + Err(TransportError::ConnectionRefused) + }); + let config = BleConfig { + adapter: Some("hci0".to_string()), + scan: Some(true), + advertise: Some(false), + accept_connections: Some(false), + probe_cooldown_secs: Some(1), + ..Default::default() + }; + let (tx, rx) = tokio::sync::mpsc::channel(64); + let mut transport = BleTransport::new(TransportId::new(1), None, config, io, tx); + transport.set_local_pubkey(test_pubkey(1)); + (transport, rx) + } + + /// A peer that advertises its listener PSM is dialled there, not at the + /// configured one — the whole point of learning it. + #[tokio::test(start_paused = true)] + async fn test_advertised_psm_is_dialled() { + let dials: DialLog = Arc::new(std::sync::Mutex::new(Vec::new())); + let (mut transport, _rx) = psm_probe_transport(Arc::clone(&dials)); + transport.start_async().await.unwrap(); + + transport + .io + .inject_scan_advert(io::ScanAdvert::with_psm(test_addr(2), 0x00C1)) + .await; + settle().await; + + assert_eq!(dials.lock().unwrap().as_slice(), &[(test_addr(2), 0x00C1)]); + transport.stop_async().await.unwrap(); + } + + /// A legacy UUID-only advertiser carries no PSM, so the configured one is + /// used. This is the path every existing peer takes and it must not + /// regress. + #[tokio::test(start_paused = true)] + async fn test_advert_without_a_psm_falls_back_to_the_configured_one() { + let dials: DialLog = Arc::new(std::sync::Mutex::new(Vec::new())); + let (mut transport, _rx) = psm_probe_transport(Arc::clone(&dials)); + transport.start_async().await.unwrap(); + + transport.io.inject_scan_result(test_addr(2)).await; + settle().await; + + assert_eq!( + dials.lock().unwrap().as_slice(), + &[(test_addr(2), DEFAULT_PSM)] + ); + transport.stop_async().await.unwrap(); + } + + /// A learned PSM that does not answer is forgotten, so a stale value + /// costs one retry rather than making the peer permanently unreachable. + #[tokio::test(start_paused = true)] + async fn test_a_failed_dial_forgets_the_learned_psm() { + let dials: DialLog = Arc::new(std::sync::Mutex::new(Vec::new())); + let (mut transport, _rx) = psm_probe_transport(Arc::clone(&dials)); + transport.start_async().await.unwrap(); + + transport + .io + .inject_scan_advert(io::ScanAdvert::with_psm(test_addr(2), 0x00C1)) + .await; + settle().await; + assert_eq!(dials.lock().unwrap().len(), 1); + + // The retry after the cooldown must not repeat the PSM that failed. + tokio::time::advance(std::time::Duration::from_secs(3)).await; + settle().await; + + let log = dials.lock().unwrap().clone(); + assert!(log.len() >= 2, "the address is retried after the cooldown"); + assert_eq!(log[0], (test_addr(2), 0x00C1)); + assert!( + log[1..].iter().all(|(_, psm)| *psm == DEFAULT_PSM), + "retries fall back to the configured PSM: {log:?}" + ); + transport.stop_async().await.unwrap(); + } + + /// The advertisement carries the PSM the listener actually bound, not the + /// one that was requested. This is the whole OS-assigned-PSM case, with + /// no platform in the assertion. + #[tokio::test] + async fn test_the_advertised_psm_is_the_one_actually_bound() { + let io = MockBleIo::new("hci0", test_addr(1)); + io.set_bound_psm(0x00C1); + let config = BleConfig { + adapter: Some("hci0".to_string()), + scan: Some(false), + advertise: Some(true), + accept_connections: Some(true), + ..Default::default() + }; + let (tx, _rx) = tokio::sync::mpsc::channel(64); + let mut transport = BleTransport::new(TransportId::new(1), None, config, io, tx); + transport.start_async().await.unwrap(); + + assert_ne!(DEFAULT_PSM, 0x00C1, "test setup: the bound PSM differs"); + assert_eq!(transport.io.advertised_psm(), Some(0x00C1)); + transport.stop_async().await.unwrap(); + } + + /// A backend that binds what it was asked for advertises that — the BlueZ + /// path, unchanged. + #[tokio::test] + async fn test_a_backend_that_honours_the_request_advertises_it() { + let io = MockBleIo::new("hci0", test_addr(1)); + let config = BleConfig { + adapter: Some("hci0".to_string()), + scan: Some(false), + advertise: Some(true), + accept_connections: Some(true), + ..Default::default() + }; + let (tx, _rx) = tokio::sync::mpsc::channel(64); + let mut transport = BleTransport::new(TransportId::new(1), None, config, io, tx); + transport.start_async().await.unwrap(); + + assert_eq!(transport.io.advertised_psm(), Some(DEFAULT_PSM)); + transport.stop_async().await.unwrap(); + } + /// A peer that opens with something other than the exchange prefix is /// rejected before the framer ever sees the bytes. #[tokio::test] diff --git a/src/transport/ble/neighbor.rs b/src/transport/ble/neighbor.rs index e0b588c3..cc6b1c01 100644 --- a/src/transport/ble/neighbor.rs +++ b/src/transport/ble/neighbor.rs @@ -1,8 +1,9 @@ //! BLE neighbor detection via advertising and scanning. //! -//! BLE advertisements carry a 128-bit FIPS service UUID for identification. -//! Post-forklift, advertisements are UUID-only (no identity material); -//! identity is exchanged during the Noise handshake. +//! BLE advertisements carry a 128-bit FIPS service UUID for identification, +//! and optionally the advertiser's L2CAP listener PSM (see `super::psm`). +//! Post-forklift they carry no identity material; identity is exchanged +//! during the Noise handshake. use crate::transport::{DiscoveredPeer, TransportId}; use secp256k1::XOnlyPublicKey; diff --git a/src/transport/ble/psm.rs b/src/transport/ble/psm.rs new file mode 100644 index 00000000..442f3d50 --- /dev/null +++ b/src/transport/ble/psm.rs @@ -0,0 +1,176 @@ +//! Advertising the L2CAP listener PSM. +//! +//! A dialer has to know which PSM a peer's L2CAP listener is bound to. On +//! BlueZ an application can *choose* that number, so both ends can agree on a +//! configured constant. BlueZ is the exception: Android's +//! `listenUsingInsecureL2capChannel` and macOS's +//! `CBPeripheralManager.publishL2CAPChannel` both return an **OS-assigned** +//! PSM the application cannot request. A dialer cannot guess it, and before a +//! connection exists there is no channel to be told it on other than the +//! advertisement itself. +//! +//! This module is the wire specification for putting it there. It is +//! deliberately state-free: learning and caching belong to the scan/probe +//! loop, which already owns per-address state. +//! +//! # Wire layout +//! +//! The PSM rides a **Service Data — 16-bit UUID** AD structure (AD type +//! `0x16`) keyed on [`PSM_SERVICE_DATA_UUID16`], carrying the PSM as two +//! bytes little-endian. +//! +//! ## Why a 16-bit key, and not the FIPS service UUID +//! +//! A legacy advertising PDU carries 31 bytes of AD payload. Keying the +//! service data on the full 128-bit FIPS service UUID does not fit: +//! +//! | AD structure | bytes | +//! |-----------------------------------------------|-------| +//! | Flags | 3 | +//! | Complete list of 128-bit service UUIDs | 18 | +//! | Service Data — **128-bit** UUID + 2-byte PSM | 20 | +//! | **total** | **41** — over by 10 | +//! +//! Keying it on the 16-bit UUID [`PSM_SERVICE_DATA_UUID16`] does: +//! +//! | AD structure | bytes | +//! |-----------------------------------------------|-------| +//! | Flags | 3 | +//! | Complete list of 128-bit service UUIDs | 18 | +//! | Service Data — **16-bit** UUID + 2-byte PSM | 6 | +//! | **total** | **27** — fits | +//! +//! `0x9C90` is the leading 16 bits of the FIPS service UUID, expanded through +//! the Bluetooth base UUID (`00009C90-0000-1000-8000-00805F9B34FB`). The +//! budget is asserted at compile time below, so a change that reverts to a +//! 128-bit key fails the build rather than the radio. It also means an +//! advertiser using this layout has no room left for a local name. +//! +//! ## Why the primary advertisement, not the scan response +//! +//! A scan response only arrives after a successful active-scan +//! request/response round-trip, and that round-trip drops asymmetrically +//! across chipsets. Peers that never answer a scan request would become +//! undiscoverable rather than merely slower. The primary advertisement is +//! received passively on every advertising interval, so the PSM must ride it. +//! +//! ## Compatibility +//! +//! A reader ignores trailing bytes, so the value can be extended without +//! breaking older peers, and an advert with no service data at all decodes to +//! `None` — which is what every legacy UUID-only advertiser produces, and +//! what makes them keep working against the configured PSM. + +/// Service-data key for the advertised L2CAP PSM. +/// +/// The leading 16 bits of the FIPS service UUID, i.e. the Bluetooth +/// base-range UUID `00009C90-0000-1000-8000-00805F9B34FB`. Backends that +/// speak in whole UUIDs must expand it through the base UUID; backends that +/// speak in AD structures emit it as AD type `0x16`. +pub const PSM_SERVICE_DATA_UUID16: u16 = 0x9C90; + +/// AD payload budget of a legacy advertising PDU, in bytes. +const LEGACY_ADV_PAYLOAD_BYTES: usize = 31; + +/// Flags AD structure: length + type + one byte of flags. +const FLAGS_AD_BYTES: usize = 3; + +/// Complete list of 128-bit service UUIDs: length + type + one UUID. +const UUID128_LIST_AD_BYTES: usize = 2 + 16; + +/// Service data keyed on a 16-bit UUID: length + type + key + PSM. +const PSM_SERVICE_DATA_AD_BYTES: usize = 2 + 2 + PSM_ENCODED_LEN; + +/// Encoded width of the PSM value itself. +const PSM_ENCODED_LEN: usize = 2; + +/// The layout above must fit a legacy advertising PDU. If this fails, the +/// advert would be silently truncated or rejected by the controller. +const _: () = assert!( + FLAGS_AD_BYTES + UUID128_LIST_AD_BYTES + PSM_SERVICE_DATA_AD_BYTES <= LEGACY_ADV_PAYLOAD_BYTES, + "PSM advert layout exceeds the 31-byte legacy advertising PDU" +); + +/// Encode a PSM as advertised service data: two bytes, little-endian. +pub fn encode_psm(psm: u16) -> [u8; PSM_ENCODED_LEN] { + psm.to_le_bytes() +} + +/// Decode a PSM from advertised service data. +/// +/// Returns `None` for absent or truncated data — a legacy UUID-only +/// advertiser, which the caller answers by dialling the configured PSM. +/// Trailing bytes are ignored so the value can be extended later without +/// breaking readers built against this version. +pub fn decode_psm(data: &[u8]) -> Option { + if data.len() < PSM_ENCODED_LEN { + return None; + } + Some(u16::from_le_bytes([data[0], data[1]])) +} + +// ============================================================================ +// Tests +// ============================================================================ + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_encode_is_little_endian() { + assert_eq!(encode_psm(0x0085), [0x85, 0x00]); + assert_eq!(encode_psm(0x1234), [0x34, 0x12]); + } + + #[test] + fn test_round_trip() { + for psm in [0u16, 1, 0x0085, 0x00FF, 0x1234, u16::MAX] { + assert_eq!(decode_psm(&encode_psm(psm)), Some(psm), "psm {psm:#06x}"); + } + } + + #[test] + fn test_absent_service_data_decodes_to_none() { + // A legacy UUID-only advertiser carries no service data at all. + assert_eq!(decode_psm(&[]), None); + } + + #[test] + fn test_truncated_service_data_decodes_to_none() { + assert_eq!(decode_psm(&[0x85]), None); + } + + #[test] + fn test_trailing_bytes_are_ignored() { + // Forward compatibility: a future advertiser may append fields. + assert_eq!(decode_psm(&[0x85, 0x00, 0xFF, 0xFF]), Some(0x0085)); + } + + /// The byte budget is a build-time assertion, not a comment. This test + /// records the arithmetic it encodes so the numbers stay legible. + #[test] + fn test_advert_fits_the_legacy_pdu() { + assert_eq!(FLAGS_AD_BYTES, 3); + assert_eq!(UUID128_LIST_AD_BYTES, 18); + assert_eq!(PSM_SERVICE_DATA_AD_BYTES, 6); + assert_eq!( + FLAGS_AD_BYTES + UUID128_LIST_AD_BYTES + PSM_SERVICE_DATA_AD_BYTES, + 27 + ); + assert_eq!(LEGACY_ADV_PAYLOAD_BYTES, 31); + // A 128-bit service-data key would need 20 bytes, not 6 — the layout + // this module exists to reject. + assert_eq!(FLAGS_AD_BYTES + UUID128_LIST_AD_BYTES + 20, 41); + } + + #[test] + fn test_key_is_the_leading_16_bits_of_the_fips_uuid() { + // FIPS service UUID: 9c90b790-2cc5-42c0-9f87-c9cc40648f4c + const FIPS_SERVICE_UUID_U128: u128 = 0x9c90_b790_2cc5_42c0_9f87_c9cc_4064_8f4c; + assert_eq!( + PSM_SERVICE_DATA_UUID16, + (FIPS_SERVICE_UUID_U128 >> 112) as u16 + ); + } +} diff --git a/src/transport/ble/stream_read.rs b/src/transport/ble/stream_read.rs index 59b12d66..8df91c95 100644 --- a/src/transport/ble/stream_read.rs +++ b/src/transport/ble/stream_read.rs @@ -5,7 +5,7 @@ //! stream-oriented backend may return a fragment of a packet, or several //! packets coalesced, from a single read. This adapter turns the //! datagram-shaped [`BleStream`] into the [`AsyncRead`] that -//! [`crate::transport::framing::read_fmp_packet`] expects, buffering bytes +//! `crate::transport::framing::read_fmp_packet` expects, buffering bytes //! left over from one read into the next so packet boundaries are recovered //! from the FMP length prefix rather than trusted to the OS. //! From 901947899f64c9e88f18435c1abcfbab5ae4dc74 Mon Sep 17 00:00:00 2001 From: Arjen <18398758+Origami74@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:32:30 +0100 Subject: [PATCH 4/9] fix(transport/ble): bound the probe retry and explain the connect outcomes Two defects in the scan/probe loop that share the same code and the same field capture, so they are fixed together. A discovered address that fails to connect was re-dialled every cooldown for the life of the process. One unreachable peer therefore consumed a dial slot forever, and because BLE hardware caps concurrent connections at roughly four to ten, a handful of them starve discovery of everything behind them. PendingProbes now backs a failing address off by powers of two up to MAX_PROBE_BACKOFF_SHIFT, which at the 30 s default caps a failing address at one attempt every sixteen minutes, and the book itself is capped at MAX_PENDING_PROBES entries so that rotating private addresses cannot grow it without bound. The stats snapshot could not explain any of it. A probe that failed and one that was never attempted were indistinguishable, so there was no way to tell a peer out of range from a peer being dialled at the wrong PSM. Each connect outcome now has its own counter and its own structured log line carrying the role, the outcome, the PSM dialled and how long the peer took to go from advertisement to conclusion. The two touch the same arms because the outcome that needed counting most is the one that also needed backing off: a dial failure now both records its reason and forgets the peer's learned PSM, so a stale advertised PSM costs one retry and is re-learned from the next advertisement rather than being retried forever at a number that cannot work. Behaviour on the deployed BlueZ path changes in one way worth naming: an address that fails is dialled less often. Nothing about which peers are reachable changes, and a peer that connects is unaffected. --- src/transport/ble/mod.rs | 731 +++++++++++++++++++++++++++++++++++-- src/transport/ble/stats.rs | 62 ++++ 2 files changed, 756 insertions(+), 37 deletions(-) diff --git a/src/transport/ble/mod.rs b/src/transport/ble/mod.rs index 61d6c916..077953b6 100644 --- a/src/transport/ble/mod.rs +++ b/src/transport/ble/mod.rs @@ -391,12 +391,19 @@ impl BleTransport { { Ok(Ok(stream)) => stream, Ok(Err(e)) => { - debug!(addr = %addr, error = %e, "BLE connect-on-send failed"); + self.stats.record_connect_error(); + debug!( + addr = %addr, role = "central", outcome = "connect-error", error = %e, + "BLE connect-on-send failed" + ); return Err(TransportError::ConnectionRefused); } Err(_) => { self.stats.record_connect_timeout(); - debug!(addr = %addr, "BLE connect-on-send timeout"); + debug!( + addr = %addr, role = "central", outcome = "connect-timeout", + "BLE connect-on-send timeout" + ); return Err(TransportError::Timeout); } }; @@ -421,7 +428,11 @@ impl BleTransport { .add_peer_with_pubkey(&announced, peer_pubkey); } Err(e) => { - warn!(addr = %addr, error = %e, "BLE outbound pubkey exchange failed"); + self.stats.record_pubkey_exchange_failure(); + warn!( + addr = %addr, role = "central", outcome = "pubkey-exchange-failed", + error = %e, "BLE outbound pubkey exchange failed" + ); return Err(e); } } @@ -553,8 +564,12 @@ impl BleTransport { neighbor_buffer.add_peer_with_pubkey(&announced, peer_pubkey); } Err(e) => { + stats.record_pubkey_exchange_failure(); warn!( - addr = %addr_clone, error = %e, + addr = %addr_clone, + role = "central", + outcome = "pubkey-exchange-failed", + error = %e, "BLE outbound pubkey exchange failed" ); return; @@ -601,11 +616,18 @@ impl BleTransport { stats.record_connection_established(); } Ok(Err(e)) => { - debug!(addr = %addr_clone, error = %e, "BLE connect failed"); + stats.record_connect_error(); + debug!( + addr = %addr_clone, role = "central", outcome = "connect-error", + error = %e, "BLE connect failed" + ); } Err(_) => { stats.record_connect_timeout(); - debug!(addr = %addr_clone, "BLE connect timeout"); + debug!( + addr = %addr_clone, role = "central", outcome = "connect-timeout", + "BLE connect timeout" + ); } } }); @@ -886,6 +908,8 @@ async fn accept_loop( { debug!( addr = %ta, + role = "peripheral", + outcome = "duplicate-node-decline", existing = %existing, "BLE inbound: peer already connected on another address, dropping duplicate" ); @@ -899,15 +923,23 @@ async fn accept_loop( if let Some(ref our_addr) = local_node_addr && our_addr < &peer_node { + stats.record_tiebreaker_drop(); debug!( addr = %ta, + role = "peripheral", + outcome = "tiebreaker-drop", "BLE inbound tie-breaker: dropping (our addr < peer, outbound wins)" ); continue; } } Err(e) => { - debug!(addr = %ta, error = %e, "BLE inbound pubkey exchange failed"); + stats.record_pubkey_exchange_failure(); + debug!( + addr = %ta, role = "peripheral", + outcome = "pubkey-exchange-failed", error = %e, + "BLE inbound pubkey exchange failed" + ); continue; } } @@ -945,8 +977,11 @@ async fn accept_loop( info!(addr = %ta, send_mtu, recv_mtu, "BLE inbound connection accepted"); } Err(e) => { - warn!(addr = %ta, error = %e, "BLE pool full, inbound connection rejected"); stats.record_connection_rejected(); + warn!( + addr = %ta, role = "peripheral", outcome = "pool-rejected", + error = %e, "BLE pool full, inbound connection rejected" + ); continue; } } @@ -1003,6 +1038,145 @@ async fn receive_loop( pool.remove(&addr); } +/// Consecutive-failure backoff ceiling for a pending address, as a power of +/// two multiple of the base cooldown. At the 30 s default this caps a failing +/// address at one dial attempt every 16 minutes. +const MAX_PROBE_BACKOFF_SHIFT: u32 = 5; + +/// Ceiling on how many discovered-but-unconnected addresses are kept for +/// retry. Resolvable private addresses rotate, so without a bound the book +/// grows for the life of the process; with one, the total retry dial rate is +/// bounded too (at most one dial per retry tick, spread over the book). +const MAX_PENDING_PROBES: usize = 32; + +/// One discovered address awaiting a successful probe. +#[derive(Debug, Clone)] +struct PendingProbe { + addr: BleAddr, + /// Consecutive failed probes. Reset only by removal from the book, which + /// every conclusive outcome (connected, duplicate declined, already + /// pooled) performs. + failures: u32, + /// Earliest instant at which this address may be dialled again. + next_attempt: tokio::time::Instant, +} + +/// The retry book for addresses the scanner has offered but which are not yet +/// connected. +/// +/// Exists because a scanner is not a reliable repeat source: BlueZ emits +/// `DeviceAdded` once per address per discovery session, so an address the +/// probe loop forgets is never offered again. Everything here therefore +/// throttles rather than discards — an entry leaves the book on a *conclusive* +/// outcome, or when [`MAX_PENDING_PROBES`] other addresses compete for its +/// slot, never because it failed. +/// +/// Two properties matter: +/// +/// - **Consecutive failures back an address off exponentially.** A dead +/// address is retried on a doubling interval up to +/// [`MAX_PROBE_BACKOFF_SHIFT`], instead of being re-dialled every cooldown +/// forever. Addresses rotate and links are lossy, so a handful of failures +/// is normal and must not retire a peer that is still there. +/// - **The retry tick rotates.** Probing only the head of the book let one +/// slow or dead address starve every other pending address behind it, which +/// on a busy radio is most of them. +#[derive(Debug)] +struct PendingProbes { + entries: Vec, + cooldown: std::time::Duration, +} + +impl PendingProbes { + fn new(cooldown: std::time::Duration) -> Self { + Self { + entries: Vec::new(), + cooldown, + } + } + + fn position(&self, addr: &BleAddr) -> Option { + self.entries.iter().position(|e| &e.addr == addr) + } + + /// Record a sighting. A previously unseen address becomes immediately + /// eligible; a known one keeps whatever backoff it has earned, so a + /// scanner that re-reports the same address many times a second cannot + /// wash out the backoff. + /// + /// When the book is full the *most-failed* entry is evicted to make room, + /// which is the entry least likely to still have a peer behind it. + fn observe(&mut self, addr: &BleAddr, now: tokio::time::Instant) { + if self.position(addr).is_some() { + return; + } + if self.entries.len() >= MAX_PENDING_PROBES + && let Some(worst) = self + .entries + .iter() + .enumerate() + .max_by_key(|(_, e)| (e.failures, e.next_attempt)) + .map(|(i, _)| i) + { + self.entries.remove(worst); + } + self.entries.push(PendingProbe { + addr: addr.clone(), + failures: 0, + next_attempt: now, + }); + } + + /// Whether `addr` may be dialled now. An address that is not in the book + /// has no history to hold it back. + fn is_due(&self, addr: &BleAddr, now: tokio::time::Instant) -> bool { + match self.position(addr) { + Some(i) => self.entries[i].next_attempt <= now, + None => true, + } + } + + /// Note that a probe is starting: hold the address for one base cooldown + /// so the attempt in flight is not duplicated. + fn mark_attempt(&mut self, addr: &BleAddr, now: tokio::time::Instant) { + if let Some(i) = self.position(addr) { + self.entries[i].next_attempt = now + self.cooldown; + } + } + + /// Note that a probe failed. Returns the new consecutive-failure count. + fn record_failure(&mut self, addr: &BleAddr, now: tokio::time::Instant) -> u32 { + let Some(i) = self.position(addr) else { + return 0; + }; + let e = &mut self.entries[i]; + e.failures = e.failures.saturating_add(1); + let shift = (e.failures - 1).min(MAX_PROBE_BACKOFF_SHIFT); + e.next_attempt = now + self.cooldown * 2u32.pow(shift); + e.failures + } + + /// Drop an address that reached a conclusive outcome. + fn resolve(&mut self, addr: &BleAddr) { + self.entries.retain(|e| &e.addr != addr); + } + + /// Drop every address for which `connected` reports a live pool entry. + fn drop_connected(&mut self, connected: impl Fn(&BleAddr) -> bool) { + self.entries.retain(|e| !connected(&e.addr)); + } + + /// The next address due for a retry, rotated to the back of the book so + /// the following tick starts after it rather than on it. + fn next_due(&mut self, now: tokio::time::Instant) -> Option { + let i = self.entries.iter().position(|e| e.next_attempt <= now)?; + let entry = self.entries.remove(i); + let addr = entry.addr.clone(); + self.entries.push(entry); + Some(addr) + } +} + /// Combined scan + probe loop. /// /// Scanner events arrive continuously (both sides advertise continuously). @@ -1030,11 +1204,12 @@ async fn scan_probe_loop( packet_tx: PacketTx, transport_id: TransportId, ) { - // Track last probe time per address for cooldown - let mut last_probed: HashMap = HashMap::new(); - // Addresses discovered but not yet connected — retried after cooldown - // even if the scanner doesn't fire again (BlueZ deduplicates). - let mut pending_addrs: Vec = Vec::new(); + // Addresses discovered but not yet connected — retried after cooldown even + // if the scanner doesn't fire again (BlueZ deduplicates), on a per-address + // backoff that widens with consecutive failures. Also the cooldown record: + // an address leaves the book the moment it reaches a conclusive outcome, + // after which the pool and `known_node_of` guards below cover it. + let mut pending = PendingProbes::new(std::time::Duration::from_secs(cooldown_secs)); // Link addresses already resolved to a node identity by a completed pubkey // exchange. Lets the loop skip an address it has *already* learned belongs // to a peer it is connected to, instead of paying a full connect and @@ -1049,7 +1224,6 @@ async fn scan_probe_loop( // which is every peer that predates this and every backend that does not // advertise service data. let mut learned_psm: HashMap = HashMap::new(); - let cooldown = std::time::Duration::from_secs(cooldown_secs); let retry_interval = tokio::time::interval(std::time::Duration::from_secs(cooldown_secs)); tokio::pin!(retry_interval); retry_interval.tick().await; // consume initial tick @@ -1075,12 +1249,13 @@ async fn scan_probe_loop( _ = retry_interval.tick() => { // Re-probe pending addresses that aren't connected let pool_guard = pool.lock().await; - pending_addrs.retain(|a| !pool_guard.contains(&a.to_transport_addr())); + pending.drop_connected(|a| pool_guard.contains(&a.to_transport_addr())); drop(pool_guard); - if let Some(a) = pending_addrs.first().cloned() { - a - } else { - continue; + // Rotating rather than always taking the head is what stops one + // slow or dead address from starving every other pending one. + match pending.next_due(tokio::time::Instant::now()) { + Some(a) => a, + None => continue, } } }; @@ -1092,21 +1267,17 @@ async fn scan_probe_loop( { let pool_guard = pool.lock().await; if pool_guard.contains(&addr.to_transport_addr()) { - pending_addrs.retain(|a| a != &addr); + pending.resolve(&addr); continue; } } // Track for retry in case probe fails and scanner doesn't re-fire - if !pending_addrs.contains(&addr) { - pending_addrs.push(addr.clone()); - } + let now = tokio::time::Instant::now(); + pending.observe(&addr, now); - // Skip if in cooldown - if last_probed - .get(&addr) - .is_some_and(|last| last.elapsed() < cooldown) - { + // Skip if in cooldown, or backed off after consecutive failures + if !pending.is_due(&addr, now) { continue; } @@ -1120,7 +1291,7 @@ async fn scan_probe_loop( pool_guard.find_by_node(node).is_some() }; if still_connected { - pending_addrs.retain(|a| a != &addr); + pending.resolve(&addr); continue; } // That peer is gone — forget the mapping and probe normally. @@ -1128,7 +1299,7 @@ async fn scan_probe_loop( } // Record probe time (before attempt, so cooldown applies on failure too) - last_probed.insert(addr.clone(), tokio::time::Instant::now()); + pending.mark_attempt(&addr, now); // Need pubkey for probe let our_pubkey = match local_pubkey { @@ -1141,6 +1312,9 @@ async fn scan_probe_loop( // L2CAP connect, at whatever PSM this peer advertised. let dial_psm = learned_psm.get(&addr).copied().unwrap_or(configured_psm); + // Stamped here so every outcome below can report how long the peer + // took to go from advertisement to conclusion. + let probe_started = tokio::time::Instant::now(); let stream = match tokio::time::timeout( std::time::Duration::from_millis(connect_timeout_ms), io.connect(&addr, dial_psm), @@ -1149,7 +1323,13 @@ async fn scan_probe_loop( { Ok(Ok(s)) => s, Ok(Err(e)) => { - debug!(addr = %addr, psm = dial_psm, error = %e, "BLE probe connect failed"); + stats.record_connect_error(); + let failures = pending.record_failure(&addr, tokio::time::Instant::now()); + debug!( + addr = %addr, role = "central", outcome = "connect-error", + psm = dial_psm, discovery_ms = probe_started.elapsed().as_millis() as u64, + failures, error = %e, "BLE probe connect failed" + ); // A learned PSM that does not answer is stale — forget it, so // the next advert re-learns it and the fallback applies in the // meantime. Costs one retry. @@ -1157,8 +1337,13 @@ async fn scan_probe_loop( continue; } Err(_) => { - debug!(addr = %addr, psm = dial_psm, "BLE probe connect timeout"); stats.record_connect_timeout(); + let failures = pending.record_failure(&addr, tokio::time::Instant::now()); + debug!( + addr = %addr, role = "central", outcome = "connect-timeout", + psm = dial_psm, discovery_ms = probe_started.elapsed().as_millis() as u64, + failures, "BLE probe connect timeout" + ); learned_psm.remove(&addr); continue; } @@ -1181,10 +1366,21 @@ async fn scan_probe_loop( if let Some(ref our_addr) = local_node_addr && our_addr >= &peer_node { + stats.record_tiebreaker_yield(); debug!( addr = %addr, + role = "central", + outcome = "tiebreaker-yield", + discovery_ms = probe_started.elapsed().as_millis() as u64, "BLE probe tie-breaker: yielding to peer's outbound" ); + // Same reasoning as the duplicate-decline path below: the + // exchange has resolved this address to a node, so once + // that node holds a link the next cooldown can skip the + // address outright instead of paying another connect and + // exchange to yield again. The tie-breaker decision itself + // is unchanged — only the cost of re-reaching it. + known_node_of.insert(addr.clone(), peer_node); let announced = announced_addr(&pool, &peer_node, &addr).await; buffer.add_peer_with_pubkey(&announced, peer_pubkey); continue; @@ -1203,7 +1399,10 @@ async fn scan_probe_loop( { debug!( addr = %ta, + role = "central", + outcome = "duplicate-node-decline", existing = %existing, + discovery_ms = probe_started.elapsed().as_millis() as u64, "BLE probe: peer already connected on another address, dropping duplicate" ); stats.record_duplicate_node_decline(); @@ -1216,7 +1415,7 @@ async fn scan_probe_loop( // connection behind it. let announced = announced_addr(&pool, &peer_node, &addr).await; buffer.add_peer_with_pubkey(&announced, peer_pubkey); - pending_addrs.retain(|a| a != &addr); + pending.resolve(&addr); continue; } @@ -1249,22 +1448,35 @@ async fn scan_probe_loop( debug!(addr = %ta, evicted = %evicted, "BLE probe promoted (evicted peer)"); } Ok(None) => { - debug!(addr = %ta, "BLE probe promoted to pool"); + debug!( + addr = %ta, role = "central", outcome = "connected", + discovery_ms = probe_started.elapsed().as_millis() as u64, + "BLE probe promoted to pool" + ); } Err(e) => { - warn!(addr = %ta, error = %e, "BLE pool full, probe connection dropped"); stats.record_connection_rejected(); + warn!( + addr = %ta, role = "central", outcome = "pool-rejected", + error = %e, "BLE pool full, probe connection dropped" + ); } } drop(pool_guard); stats.record_connection_established(); - pending_addrs.retain(|a| a != &addr); + pending.resolve(&addr); // Report to node layer for auto-connect / handshake buffer.add_peer_with_pubkey(&addr, peer_pubkey); } Err(e) => { - debug!(addr = %addr, error = %e, "BLE probe pubkey exchange failed"); + stats.record_pubkey_exchange_failure(); + let failures = pending.record_failure(&addr, tokio::time::Instant::now()); + debug!( + addr = %addr, role = "central", outcome = "pubkey-exchange-failed", + discovery_ms = probe_started.elapsed().as_millis() as u64, + failures, error = %e, "BLE probe pubkey exchange failed" + ); } } } @@ -1281,6 +1493,211 @@ mod tests { use io::{MockBleIo, MockBleStream}; use secp256k1::{Secp256k1, SecretKey}; + // ------------------------------------------------------------------ + // PendingProbes — the retry/backoff policy for discovered addresses + // ------------------------------------------------------------------ + + const TEST_COOLDOWN: std::time::Duration = std::time::Duration::from_secs(30); + + fn probes() -> PendingProbes { + PendingProbes::new(TEST_COOLDOWN) + } + + fn a(n: u8) -> BleAddr { + BleAddr::parse(&format!("ble0/AA:BB:CC:DD:EE:{:02X}", n)).unwrap() + } + + /// A fresh sighting is dialled straight away — discovery must not wait a + /// cooldown to try a peer it has never met. + #[test] + fn a_newly_seen_address_is_due_immediately() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + p.observe(&a(1), t0); + assert!(p.is_due(&a(1), t0)); + } + + /// The regression this policy exists for: an address that keeps failing + /// must be dialled exponentially less often, not once per cooldown for as + /// long as the process lives. + #[test] + fn consecutive_failures_back_an_address_off_exponentially() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + p.observe(&a(1), t0); + + for expected_shift in 0..MAX_PROBE_BACKOFF_SHIFT { + let n = p.record_failure(&a(1), t0); + assert_eq!(n, expected_shift + 1); + let wait = TEST_COOLDOWN * 2u32.pow(expected_shift); + assert!( + !p.is_due(&a(1), t0 + wait - std::time::Duration::from_millis(1)), + "due too early after {n} failures" + ); + assert!(p.is_due(&a(1), t0 + wait), "not due after {n} failures"); + } + + // And the interval stops growing at the ceiling rather than running + // away to hours. + let capped = TEST_COOLDOWN * 2u32.pow(MAX_PROBE_BACKOFF_SHIFT); + for _ in 0..8 { + p.record_failure(&a(1), t0); + assert!(p.is_due(&a(1), t0 + capped)); + } + } + + /// Under the old policy an address failing every 30 s for 37 minutes was + /// dialled 49 times. Pin the improvement rather than just the formula. + #[test] + fn a_dead_address_is_dialled_a_handful_of_times_an_hour() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + p.observe(&a(1), t0); + + let mut dials = 0; + let mut now = t0; + let deadline = t0 + std::time::Duration::from_secs(37 * 60); + // Tick at the retry interval, exactly as the loop does. + while now <= deadline { + if p.is_due(&a(1), now) { + p.mark_attempt(&a(1), now); + p.record_failure(&a(1), now); + dials += 1; + } + now += TEST_COOLDOWN; + } + assert!( + (1..=10).contains(&dials), + "expected a handful of dials in 37 minutes, got {dials}" + ); + } + + /// A scanner that re-reports the same address many times a second (which + /// Android does, at roughly 52/min) must not wash the backoff out. + #[test] + fn repeated_sightings_do_not_reset_the_backoff() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + p.observe(&a(1), t0); + for _ in 0..4 { + p.record_failure(&a(1), t0); + } + let still_blocked = t0 + TEST_COOLDOWN; + for _ in 0..100 { + p.observe(&a(1), still_blocked); + } + assert!(!p.is_due(&a(1), still_blocked)); + assert_eq!(p.entries.len(), 1); + } + + /// Failing never removes an address. This is what keeps a BlueZ node + /// recoverable: BlueZ emits `DeviceAdded` once per address per discovery + /// session, so an address dropped from the book would never be offered + /// again and the peer behind it would be unreachable for the life of the + /// process. + #[test] + fn failures_never_evict_the_address_itself() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + p.observe(&a(1), t0); + for _ in 0..500 { + p.record_failure(&a(1), t0); + } + assert_eq!(p.entries.len(), 1); + // Still reachable: once the (capped) backoff elapses it is dialled + // again, so a peer that comes back is picked up without a new sighting. + let capped = TEST_COOLDOWN * 2u32.pow(MAX_PROBE_BACKOFF_SHIFT); + assert_eq!(p.next_due(t0 + capped), Some(a(1))); + } + + /// Conclusive outcomes clear the address *and* its failure history, so a + /// peer that reconnects later starts from a clean slate. + #[test] + fn resolving_clears_the_failure_history() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + p.observe(&a(1), t0); + for _ in 0..5 { + p.record_failure(&a(1), t0); + } + p.resolve(&a(1)); + assert!(p.entries.is_empty()); + p.observe(&a(1), t0); + assert!(p.is_due(&a(1), t0)); + } + + /// The head-of-line half of the bug: probing only the first entry let one + /// address monopolise the retry tick. Rotation gives every due address a + /// turn. + #[test] + fn the_retry_tick_rotates_across_due_addresses() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + for n in 0..3 { + p.observe(&a(n), t0); + } + let order: Vec<_> = (0..6).filter_map(|_| p.next_due(t0)).collect(); + assert_eq!(order, vec![a(0), a(1), a(2), a(0), a(1), a(2)]); + } + + /// A backed-off address is skipped by the tick rather than blocking the + /// addresses behind it. + #[test] + fn a_backed_off_address_does_not_block_the_others() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + p.observe(&a(0), t0); + p.observe(&a(1), t0); + p.record_failure(&a(0), t0); + assert_eq!(p.next_due(t0), Some(a(1))); + assert_eq!(p.next_due(t0), Some(a(1))); + } + + /// Nothing is due when everything is backed off — the tick idles rather + /// than dialling something it just said it would not. + #[test] + fn next_due_yields_nothing_when_all_are_backed_off() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + p.observe(&a(0), t0); + p.record_failure(&a(0), t0); + assert_eq!(p.next_due(t0), None); + } + + /// Addresses rotate, so the book is capacity-bounded. Eviction is by + /// failure count, so the entry least likely to have a peer behind it goes + /// first and a healthy address is never displaced by a dead one. + #[test] + fn a_full_book_evicts_the_most_failed_address() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + for n in 0..MAX_PENDING_PROBES as u8 { + p.observe(&a(n), t0); + } + // One entry is much worse than the rest. + for _ in 0..3 { + p.record_failure(&a(7), t0); + } + p.observe(&a(200), t0); + assert_eq!(p.entries.len(), MAX_PENDING_PROBES); + assert!(p.position(&a(7)).is_none(), "the worst entry should go"); + assert!(p.position(&a(200)).is_some(), "the new entry should land"); + assert!(p.position(&a(0)).is_some(), "healthy entries should stay"); + } + + /// Pool membership clears entries in bulk on the retry tick. + #[test] + fn connected_addresses_leave_the_book() { + let mut p = probes(); + let t0 = tokio::time::Instant::now(); + for n in 0..3 { + p.observe(&a(n), t0); + } + p.drop_connected(|addr| addr == &a(1)); + assert_eq!(p.entries.len(), 2); + assert!(p.position(&a(1)).is_none()); + } + /// Deterministic x-only pubkey for exchange tests. fn test_pubkey(seed: u8) -> [u8; 32] { let secp = Secp256k1::new(); @@ -1642,12 +2059,48 @@ mod tests { } /// Let spawned loops make progress. + /// + /// Cooperative only: this hands the scheduler control, it does not move + /// the clock. Anything gated on a `tokio::time` timer needs + /// [`wait_for`] instead. async fn settle() { for _ in 0..64 { tokio::task::yield_now().await; } } + /// Wait until `cond` holds, or fail the test. + /// + /// A fixed number of `yield_now()` calls is not a wait, it is a race + /// against the clock, and it loses whenever a loop under test is parked + /// on a timer rather than on a channel. `scan_probe_loop` is: before it + /// reaches its `select!` it consumes the retry interval's first tick, + /// and tokio rounds a timer deadline up to the next whole millisecond of + /// its wheel — so unless the runtime clock happens to sit exactly on a + /// millisecond boundary, that tick cannot fire until real time crosses + /// the next one. No number of yields makes real time pass, so whether a + /// yield budget covers the gap depends on how long a yield takes on the + /// host: comfortably on a slow one, not at all on a fast one. + /// + /// Polling the condition with a sleep between attempts removes the + /// dependency entirely — the sleep is what lets the timer fire, and the + /// condition is what ends the wait. The already-satisfied case still + /// costs only a `settle`, so nothing that passes today gets slower. + async fn wait_for(what: &str, mut cond: impl FnMut() -> bool) { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(5); + loop { + settle().await; + if cond() { + return; + } + assert!( + tokio::time::Instant::now() < deadline, + "timed out waiting for {what}" + ); + tokio::time::sleep(std::time::Duration::from_millis(1)).await; + } + } + /// A second inbound connection from a rotated address for a peer already /// in the pool is declined, the incumbent link is kept, and the peer is /// still announced — under the address its live link is on. @@ -1938,4 +2391,208 @@ mod tests { .unwrap_err(); assert!(matches!(err, TransportError::RecvFailed(_))); } + + // ------------------------------------------------------------------ + // Connect outcome counters + // ------------------------------------------------------------------ + + /// A dial that errors is counted as an error, not as a timeout. The two + /// are different faults and blur into one useless number if merged. + #[tokio::test(start_paused = true)] + async fn test_a_refused_dial_counts_as_an_error_not_a_timeout() { + let dials: DialLog = Arc::new(std::sync::Mutex::new(Vec::new())); + let (mut transport, _rx) = psm_probe_transport(Arc::clone(&dials)); + transport.start_async().await.unwrap(); + + transport.io.inject_scan_result(test_addr(2)).await; + settle().await; + + let snap = transport.stats.snapshot(); + assert_eq!(snap.connect_errors, 1); + assert_eq!(snap.connect_timeouts, 0); + assert_eq!(snap.connections_established, 0); + transport.stop_async().await.unwrap(); + } + + /// A peer that connects and then sends a bad exchange is counted as a + /// pubkey-exchange failure — the link came up and produced nothing + /// usable, which is a different fault from never connecting. + #[tokio::test] + async fn test_a_bad_exchange_counts_as_a_pubkey_exchange_failure() { + let io = MockBleIo::new("hci0", test_addr(1)); + let (tx, _rx) = tokio::sync::mpsc::channel(64); + let mut transport = + BleTransport::new(TransportId::new(1), None, identity_test_config(), io, tx); + transport.set_local_pubkey(test_pubkey(1)); + transport.start_async().await.unwrap(); + + let (ours, peer) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + transport.io.inject_inbound(ours).await; + let mut wire = vec![0xFFu8]; + wire.extend_from_slice(&test_pubkey(2)); + peer.send(&wire).await.unwrap(); + settle().await; + + let snap = transport.stats.snapshot(); + assert_eq!(snap.pubkey_exchange_failures, 1); + assert_eq!(snap.connections_accepted, 0); + assert_eq!(transport.pool.lock().await.len(), 0); + transport.stop_async().await.unwrap(); + } + + /// The tie-breaker pair. Its convention is deterministic in source, but + /// nothing recorded whether two nodes actually agreed at runtime, and a + /// disagreement leaves every existing counter at zero. Across a pair, one + /// yield and one drop is agreement. + #[tokio::test] + async fn test_tiebreaker_records_one_yield_and_one_drop_across_a_pair() { + let (smaller, larger) = pubkeys_ordered_by_node_addr(); + + // The node with the LARGER address accepts an inbound from the + // smaller: its inbound wins, so nothing is stood down here. Invert it + // — the SMALLER node accepting from the larger stands its inbound + // down, because its own outbound is meant to win. + let io = MockBleIo::new("hci0", test_addr(1)); + let (tx, _rx) = tokio::sync::mpsc::channel(64); + let mut inbound_side = + BleTransport::new(TransportId::new(1), None, identity_test_config(), io, tx); + inbound_side.set_local_pubkey(smaller); + inbound_side.start_async().await.unwrap(); + + let (ours, peer) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + inbound_side.io.inject_inbound(ours).await; + peer_side_exchange(&peer, &larger).await; + { + let stats = Arc::clone(&inbound_side.stats); + wait_for("the inbound tie-breaker to conclude", || { + stats.snapshot().tiebreaker_drops == 1 + }) + .await; + } + + let snap = inbound_side.stats.snapshot(); + assert_eq!(snap.tiebreaker_drops, 1, "our inbound stood down"); + assert_eq!(snap.tiebreaker_yields, 0); + assert_eq!(inbound_side.pool.lock().await.len(), 0); + inbound_side.stop_async().await.unwrap(); + + // The other side of the same pair: the node with the LARGER address + // probing outbound stands its dial down, because the smaller node's + // outbound is meant to win. + let dials: DialLog = Arc::new(std::sync::Mutex::new(Vec::new())); + let io2 = MockBleIo::new("hci0", test_addr(2)); + let (peer_tx, mut peer_rx) = tokio::sync::mpsc::unbounded_channel(); + io2.set_connect_handler(move |addr, psm| { + dials.lock().unwrap().push((addr.clone(), psm)); + let (ours, theirs) = MockBleStream::pair(test_addr(2), addr.clone(), 2048); + peer_tx + .send(theirs) + .map_err(|_| TransportError::ConnectionRefused)?; + Ok(ours) + }); + tokio::spawn(async move { + let mut alive = Vec::new(); + while let Some(theirs) = peer_rx.recv().await { + peer_side_exchange(&theirs, &smaller).await; + alive.push(theirs); + } + }); + + let config = BleConfig { + scan: Some(true), + accept_connections: Some(false), + ..identity_test_config() + }; + let (tx2, _rx2) = tokio::sync::mpsc::channel(64); + let mut outbound_side = BleTransport::new(TransportId::new(2), None, config, io2, tx2); + outbound_side.set_local_pubkey(larger); + outbound_side.start_async().await.unwrap(); + + outbound_side.io.inject_scan_result(test_addr(1)).await; + { + let stats = Arc::clone(&outbound_side.stats); + wait_for("the outbound tie-breaker to conclude", || { + stats.snapshot().tiebreaker_yields == 1 + }) + .await; + } + + let snap = outbound_side.stats.snapshot(); + assert_eq!(snap.tiebreaker_yields, 1, "our outbound stood down"); + assert_eq!(snap.tiebreaker_drops, 0); + assert_eq!(outbound_side.pool.lock().await.len(), 0); + outbound_side.stop_async().await.unwrap(); + } + + /// An oversized packet is a caller bug, not a property of the peer's + /// link. Folding it into `send_errors` would make that number useless as + /// evidence. + #[tokio::test] + async fn test_mtu_rejection_does_not_count_as_a_send_error() { + let io = MockBleIo::new("hci0", test_addr(1)); + let (tx, _rx) = tokio::sync::mpsc::channel(64); + let transport = + BleTransport::new(TransportId::new(1), None, identity_test_config(), io, tx); + + let ta = test_addr(2).to_transport_addr(); + let (parked, _peer) = MockBleStream::pair(test_addr(1), test_addr(2), 2048); + transport + .pool + .lock() + .await + .insert( + ta.clone(), + BleConnection { + stream: Arc::new(parked), + recv_task: None, + send_mtu: 64, + recv_mtu: 64, + established_at: tokio::time::Instant::now(), + is_static: false, + addr: test_addr(2), + node_addr: None, + }, + ) + .unwrap(); + + let err = transport.send_async(&ta, &[0u8; 128]).await.unwrap_err(); + assert!(matches!(err, TransportError::MtuExceeded { .. })); + + let snap = transport.stats.snapshot(); + assert_eq!(snap.mtu_exceeded, 1); + assert_eq!(snap.send_errors, 0); + } + + /// The snapshot is the control-socket contract. Pin every key so a field + /// cannot be dropped or renamed without a test saying so. + #[test] + fn test_snapshot_carries_every_counter() { + let value = serde_json::to_value(BleStats::new().snapshot()).unwrap(); + let object = value.as_object().unwrap(); + let expected = [ + "packets_sent", + "bytes_sent", + "packets_recv", + "bytes_recv", + "send_errors", + "recv_errors", + "mtu_exceeded", + "connections_established", + "connections_accepted", + "connections_rejected", + "connect_timeouts", + "connect_errors", + "pubkey_exchange_failures", + "tiebreaker_yields", + "tiebreaker_drops", + "pool_evictions", + "advertisements_sent", + "scan_results", + "duplicate_node_declines", + ]; + for key in expected { + assert!(object.contains_key(key), "snapshot lost `{key}`"); + } + assert_eq!(object.len(), expected.len(), "snapshot gained a key"); + } } diff --git a/src/transport/ble/stats.rs b/src/transport/ble/stats.rs index 9e43a82d..d84cc3ef 100644 --- a/src/transport/ble/stats.rs +++ b/src/transport/ble/stats.rs @@ -1,4 +1,12 @@ //! BLE transport statistics. +//! +//! Counters reach an operator through `show_transports`, which serves them +//! off the control socket. Each connect outcome also emits a `debug!` at the +//! moment it happens, carrying a uniform field set — `addr`, `role` +//! (`central` for a dial, `peripheral` for an accept), `outcome` (a stable +//! kebab-case string matching the counter name), and `discovery_ms` where a +//! probe stamp exists. The counters give the aggregate; the trace stream +//! gives the same taxonomy per event and per peer. use portable_atomic::{AtomicU64, Ordering}; @@ -20,6 +28,14 @@ pub struct BleStats { pub connections_accepted: AtomicU64, pub connections_rejected: AtomicU64, pub connect_timeouts: AtomicU64, + /// Outbound connects that failed with an error rather than timing out. + pub connect_errors: AtomicU64, + /// Connections dropped because the pre-handshake pubkey exchange failed. + pub pubkey_exchange_failures: AtomicU64, + /// Outbound connections stood down by the cross-probe tie-breaker. + pub tiebreaker_yields: AtomicU64, + /// Inbound connections stood down by the cross-probe tie-breaker. + pub tiebreaker_drops: AtomicU64, pub pool_evictions: AtomicU64, pub advertisements_sent: AtomicU64, pub scan_results: AtomicU64, @@ -43,6 +59,10 @@ impl BleStats { connections_accepted: AtomicU64::new(0), connections_rejected: AtomicU64::new(0), connect_timeouts: AtomicU64::new(0), + connect_errors: AtomicU64::new(0), + pubkey_exchange_failures: AtomicU64::new(0), + tiebreaker_yields: AtomicU64::new(0), + tiebreaker_drops: AtomicU64::new(0), pool_evictions: AtomicU64::new(0), advertisements_sent: AtomicU64::new(0), scan_results: AtomicU64::new(0), @@ -97,6 +117,40 @@ impl BleStats { self.connect_timeouts.fetch_add(1, Ordering::Relaxed); } + /// Record an outbound connect that failed with an error. + /// + /// Kept separate from [`Self::record_connect_timeout`]: a refusal and a + /// silence are different faults and blur into one useless number if + /// merged. + pub fn record_connect_error(&self) { + self.connect_errors.fetch_add(1, Ordering::Relaxed); + } + + /// Record a failed pre-handshake pubkey exchange. + /// + /// The link came up and then produced nothing usable — a different fault + /// from never connecting at all. + pub fn record_pubkey_exchange_failure(&self) { + self.pubkey_exchange_failures + .fetch_add(1, Ordering::Relaxed); + } + + /// Record an outbound connection stood down by the cross-probe + /// tie-breaker. + /// + /// Read together with [`Self::record_tiebreaker_drop`] across a pair of + /// nodes: one yield and one drop is the two sides agreeing; two yields or + /// two drops is the disagreement that leaves no other evidence. + pub fn record_tiebreaker_yield(&self) { + self.tiebreaker_yields.fetch_add(1, Ordering::Relaxed); + } + + /// Record an inbound connection stood down by the cross-probe + /// tie-breaker. See [`Self::record_tiebreaker_yield`]. + pub fn record_tiebreaker_drop(&self) { + self.tiebreaker_drops.fetch_add(1, Ordering::Relaxed); + } + /// Record a pool eviction (non-static peer displaced). pub fn record_pool_eviction(&self) { self.pool_evictions.fetch_add(1, Ordering::Relaxed); @@ -136,6 +190,10 @@ impl BleStats { connections_accepted: self.connections_accepted.load(Ordering::Relaxed), connections_rejected: self.connections_rejected.load(Ordering::Relaxed), connect_timeouts: self.connect_timeouts.load(Ordering::Relaxed), + connect_errors: self.connect_errors.load(Ordering::Relaxed), + pubkey_exchange_failures: self.pubkey_exchange_failures.load(Ordering::Relaxed), + tiebreaker_yields: self.tiebreaker_yields.load(Ordering::Relaxed), + tiebreaker_drops: self.tiebreaker_drops.load(Ordering::Relaxed), pool_evictions: self.pool_evictions.load(Ordering::Relaxed), advertisements_sent: self.advertisements_sent.load(Ordering::Relaxed), scan_results: self.scan_results.load(Ordering::Relaxed), @@ -164,6 +222,10 @@ pub struct BleStatsSnapshot { pub connections_accepted: u64, pub connections_rejected: u64, pub connect_timeouts: u64, + pub connect_errors: u64, + pub pubkey_exchange_failures: u64, + pub tiebreaker_yields: u64, + pub tiebreaker_drops: u64, pub pool_evictions: u64, pub advertisements_sent: u64, pub scan_results: u64, From 6867a290a47636d6b7765a054cc64c2e7af299a9 Mon Sep 17 00:00:00 2001 From: Arjen <18398758+Origami74@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:34:14 +0100 Subject: [PATCH 5/9] feat(transport/ble): add an embedder-supplied radio backend for Android Every backend so far owns a Bluetooth stack in process: BluerIo talks to BlueZ over D-Bus and MockBleIo is an in-memory double. Android allows neither. Scanning, advertising, L2CAP listen and connect, and the socket I/O itself are Java APIs held under a permission and foreground-service model that only the application can satisfy, so there is no Rust-reachable radio to open. This backend therefore drives the embedder's radio rather than one of its own. The embedder implements AndroidRadio and installs it into a BleRadioSlot; the backend resolves that slot per operation, so a radio that arrives after the node is built is adopted in place instead of needing the node rebuilt around it, and one that is cleared parks the backend rather than failing it. The slot is per-node rather than a process global, because a global collapses as soon as two nodes share a process, and installing twice returns the existing slot rather than replacing it, because a slot may already hold a live radio that a second call must not orphan. Android assigns the L2CAP PSM rather than letting the application request one, which is what the PSM seam exists for: listen reports back whatever the platform bound, and that is the value advertised. A listener that fails to open is not latched, so a later resolve can retry it. The module is compiled under cfg(test) on every host as well as on the platform that selects it, so its channel machinery, slot semantics and connect routing are exercised by an ordinary test run on an ordinary runner. The platform build is linted but executed nowhere, which is precisely why the logic must not sit behind a platform-only gate. It lands as io_android.rs, beside io_linux.rs, so each backend is one file named for its platform and everything platform-neutral stays in io.rs and the modules beside it. Adding macOS should mean adding io_macos.rs and nothing else. Nothing here is reachable yet: no build selects this backend and nothing installs a radio. Both arrive in the following commit. --- src/transport/ble/io_android.rs | 1710 +++++++++++++++++++++++++++++++ src/transport/ble/mod.rs | 10 + 2 files changed, 1720 insertions(+) create mode 100644 src/transport/ble/io_android.rs diff --git a/src/transport/ble/io_android.rs b/src/transport/ble/io_android.rs new file mode 100644 index 00000000..78661575 --- /dev/null +++ b/src/transport/ble/io_android.rs @@ -0,0 +1,1710 @@ +//! A [`BleIo`] backend whose radio is supplied by the embedder. +//! +//! The two backends that came before this one own a Bluetooth stack in +//! process: `io_linux`'s `BluerIo` talks to BlueZ over D-Bus, and +//! `MockBleIo` is an in-memory double. Some platforms allow neither. On Android every BLE +//! capability this transport needs — scanning, advertising, L2CAP listen and +//! connect, and the socket I/O itself — is a Java API held under a permission +//! and foreground-service model that only the application can satisfy. There +//! is no Rust-reachable radio to open. +//! +//! So this backend does not drive a radio; it drives *the embedder's* radio. +//! +//! # Shape +//! +//! - [`AndroidRadio`] is the command surface the embedder implements: open a +//! listener, dial a peer, start and stop advertising and scanning, close a +//! channel. It is object-safe, control-plane only, and deliberately tiny. +//! - [`AndroidBleBridge`] is the channel machinery around one radio. The +//! embedder builds it over its `AndroidRadio` and drives its `deliver_*` +//! and [`next_send`](AndroidBleBridge::next_send) methods from whatever +//! foreign-function layer it has; nothing in this module knows about JNI. +//! - [`BleRadioSlot`] is where a bridge is installed. The node owns one and +//! hands out shared handles; [`AndroidIo`] resolves it per operation. +//! +//! # Bytes never cross the command surface +//! +//! Inbound bytes are **pushed** into a tokio channel by the embedder +//! ([`deliver_recv`](AndroidBleBridge::deliver_recv)) and the awaiting +//! transport task wakes. Outbound bytes are **pulled** out of a bounded queue +//! by the embedder's writer thread +//! ([`next_send`](AndroidBleBridge::next_send)), which blocks with a timeout. +//! So [`BleStream::send`] is a channel push and never makes a foreign call on +//! the byte hot path, and no foreign upcall can block a runtime worker. +//! +//! The outbound queue is bounded and shallow on purpose: see +//! [`SEND_QUEUE_CAP`]. +//! +//! # A radio that arrives late, and is replaced +//! +//! The radio's lifetime is not the node's. It belongs to a service the user +//! can start and stop — turning Bluetooth on after the mesh is already +//! running, or off and on again — and each start typically mints a fresh +//! radio object. A backend that captured its radio at construction, or a +//! transport built only once a radio existed, would make "the user enabled +//! Bluetooth" mean "tear the node down and rebuild it", dropping every peer, +//! session and route for as long as re-handshaking takes. +//! +//! Hence the slot. The transport is built and started whether or not a radio +//! is present; [`AndroidIo::listen`] and [`AndroidIo::start_scanning`] +//! succeed with an empty slot and hand back an acceptor and a scanner that +//! follow it, activating the radio when one is installed and re-activating it +//! when one replaces another. Dials attempted with an empty slot fail, and +//! the transport's own probe loop retries them later. Live streams keep the +//! bridge they were opened on rather than migrating: a channel belongs to the +//! socket that created it, and those die with the radio that owned them. +//! +//! The slot is owned by a node, not by a process global. A global is simpler +//! and wrong — it collapses as soon as two nodes share a process, which +//! includes any test that drives two backends at once. +//! +//! # Testing +//! +//! Everything here is ordinary Rust; the foreign-function layer lives in the +//! embedder. The module is therefore compiled under `cfg(test)` on any host, +//! so its channel machinery, slot semantics and connect routing are unit +//! tested and linted on an ordinary CI runner against a mock radio, with no +//! device and no cross-compilation. + +use std::collections::HashMap; +use std::sync::atomic::{AtomicBool, AtomicI64, AtomicU16, Ordering}; +use std::sync::{Arc, Mutex}; +use std::time::Duration; + +use arc_swap::ArcSwapOption; +use tokio::sync::Mutex as AsyncMutex; +use tokio::sync::{Notify, mpsc, oneshot}; +use tracing::{debug, trace}; + +use crate::transport::TransportError; + +use super::addr::BleAddr; +use super::io::{BleAcceptor, BleIo, BleScanner, BleStream, ScanAdvert}; + +/// Adapter label reported by this backend. +/// +/// Platforms that hide the radio behind an application API also hide any +/// BlueZ-style adapter name, so there is nothing to report but a stable +/// placeholder. Nothing keys off it: peers are identified by node address, +/// never by adapter or MAC. +const ANDROID_ADAPTER: &str = "ble0"; + +/// Bound on the inbound byte queue and on the accept and scan fan-ins. +/// +/// Generous, because these carry control events that should not be dropped +/// under a burst, and because inbound L2CAP data that is dropped here is +/// recovered by the retransmission above. +const CHANNEL_CAP: usize = 256; + +/// Bound on the **outbound** byte queue, from this backend to the embedder's +/// writer. +/// +/// Kept shallow deliberately. A BLE link's bandwidth-delay product is on the +/// order of a single packet, so a deep queue does not absorb a burst, it +/// bufferbloats: round-trip time balloons into seconds and the congestion +/// control above never finds the real capacity. A shallow *tail-drop* queue is +/// no better — it sheds packets and collapses throughput instead. So the queue +/// is both shallow and blocking: [`BleStream::send`] waits for a slot rather +/// than dropping, which propagates flow control up through the framing and +/// multiplexing layers to whatever is generating the traffic. +/// +/// The value is the best one observed in a throughput sweep on real hardware — +/// shallower starved the radio's connection events, deeper regressed +/// throughput — but the sweep was noisy and non-monotonic, because run-to-run +/// BLE variance (RF conditions, and whether the OS grants the faster PHY and a +/// high-priority connection interval that session) rivals the effect of the +/// knob itself. Treat it as a working value pending re-measurement with PHY +/// and connection-interval instrumentation, not as a proven optimum. +const SEND_QUEUE_CAP: usize = 32; + +/// Fallback channel MTU for a platform that reports an unknown (zero) one. +/// +/// Matches the BLE transport's configured default so an unknown MTU behaves +/// like an unconfigured one rather than like a zero-capacity link. +const DEFAULT_BLE_MTU: u16 = 2048; + +/// How long a full outbound queue is waited on before re-checking whether the +/// channel died underneath us. +const SEND_BACKPRESSURE_POLL: Duration = Duration::from_millis(2); + +// ============================================================================ +// AndroidRadio — the embedder-implemented command surface +// ============================================================================ + +/// The radio commands this backend issues to the embedder. +/// +/// Object-safe, so a bridge can hold `Arc`. Control plane +/// only: bytes never cross this trait in either direction — outbound bytes are +/// pulled by the embedder via [`AndroidBleBridge::next_send`] and inbound bytes +/// are pushed by it via [`AndroidBleBridge::deliver_recv`]. +/// +/// Every method returns immediately. Outcomes that take time arrive back +/// through the bridge: an accepted inbound channel through +/// [`AndroidBleBridge::deliver_inbound`], a dial's result through +/// [`AndroidBleBridge::deliver_connect_result`], an advertisement through +/// [`AndroidBleBridge::deliver_scan`]. +pub trait AndroidRadio: Send + Sync { + /// Open an L2CAP listener and report the PSM it was bound to. + /// + /// Returns `0` if no listener could be opened. The PSM is chosen by the + /// platform, not by this transport — which is the whole reason + /// [`BleIo::listen`] reports a PSM back rather than being assumed to have + /// bound the one it was asked for. + fn listen(&self) -> u16; + + /// Begin dialing `addr` on `psm`. + /// + /// The outcome is delivered later through + /// [`AndroidBleBridge::deliver_connect_result`], keyed by `connect_id`. + /// The transport already bounds the wait, so a dial that is never answered + /// is not a leak of anything but one map entry until the transport gives + /// up on it. + fn connect(&self, connect_id: i64, addr: &BleAddr, psm: u16); + + /// Advertise the FIPS service, carrying `psm` as the listener PSM peers + /// should dial. See [`super::psm`] for the wire layout. + fn start_advertising(&self, psm: u16); + + /// Stop advertising. + fn stop_advertising(&self); + + /// Start scanning for FIPS advertisements, delivering each one through + /// [`AndroidBleBridge::deliver_scan`]. + fn start_scanning(&self); + + /// Stop scanning. + fn stop_scanning(&self); + + /// Close the L2CAP channel `ch_id`, called when this transport drops the + /// stream that owned it. + fn close_channel(&self, ch_id: i64); +} + +// ============================================================================ +// BleRadioSlot — where the embedder installs a radio +// ============================================================================ + +/// The node-owned slot an embedder installs its radio bridge into. +/// +/// Reads are lock-free, because every backend operation resolves the slot +/// before doing anything else. Installing, replacing and clearing are safe at +/// any time and from any thread, including before the node has started and +/// long after it has. +/// +/// Replacing a bridge does not migrate live streams onto the new radio. Their +/// channels belong to sockets the old radio owned and die with it; the +/// transport notices those deaths the way it notices any other link loss. +#[derive(Default)] +pub struct BleRadioSlot { + current: ArcSwapOption, + /// Woken on every install and clear, so a backend parked on the old + /// bridge's channels re-resolves immediately instead of polling. + changed: Notify, +} + +impl BleRadioSlot { + /// An empty slot. + pub fn new() -> Self { + Self::default() + } + + /// Install a bridge, replacing whatever was there. + pub fn install(&self, bridge: Arc) { + self.current.store(Some(bridge)); + self.changed.notify_waiters(); + } + + /// Remove the installed bridge, if any. + /// + /// Operations then fail, or park, until one is installed again. Nothing is + /// torn down here beyond this reference: the embedder owns the radio's + /// lifetime and closes its sockets on its own schedule. + pub fn clear(&self) { + self.current.store(None); + self.changed.notify_waiters(); + } + + /// The currently installed bridge, if any. + pub fn current(&self) -> Option> { + self.current.load_full() + } + + /// Whether a bridge is installed. + pub fn is_installed(&self) -> bool { + self.current.load().is_some() + } +} + +// ============================================================================ +// AndroidBleBridge — channel machinery over one radio +// ============================================================================ + +/// The bridge-side half of one L2CAP channel. +struct ChannelState { + /// Bytes the embedder pushed in; the stream's `recv` awaits them. + recv_tx: mpsc::Sender>, + /// Bytes waiting to go out; the embedder's writer pulls them. + /// + /// Behind an `Arc` so [`AndroidBleBridge::next_send`] can clone it out and + /// release the channel map before blocking, rather than stalling every + /// other channel's create and close for the length of one timeout. + send_rx: Arc>>>, + closed: Arc, +} + +/// The stream-side half of one L2CAP channel, handed over once. +struct StreamEndpoints { + ch_id: i64, + remote: BleAddr, + send_mtu: u16, + recv_mtu: u16, + recv_rx: mpsc::Receiver>, + send_tx: std::sync::mpsc::SyncSender>, + closed: Arc, +} + +/// Channel machinery around one [`AndroidRadio`]. +/// +/// The embedder constructs one of these per radio it starts, installs it into +/// a [`BleRadioSlot`], keeps its own handle, and drives the `deliver_*` and +/// [`next_send`](Self::next_send) methods from its foreign-function layer. +pub struct AndroidBleBridge { + radio: Arc, + /// Source of channel and dial identifiers. Shared between the two so an + /// identifier is unambiguous in a log line. + next_id: AtomicI64, + /// The listener PSM the platform assigned, or `0` before a listener has + /// been opened. + local_psm: AtomicU16, + /// Set once each activation has been performed on this radio, so + /// re-resolving the slot cannot issue it twice. + listening: AtomicBool, + /// The PSM currently being advertised on this radio, or `None` when it is + /// not advertising. A lock rather than an atomic, so choosing the PSM and + /// telling the radio about it are one step: two callers racing here would + /// otherwise be free to land in the opposite order and leave the stale one + /// on the air. See [`Self::advertise`]. + advertising: Mutex>, + scanning: AtomicBool, + channels: Mutex>, + /// In-flight dials, by `connect_id`. + connects: Mutex>>, + accept_tx: mpsc::Sender, + accept_rx: Mutex>>, + scan_tx: mpsc::Sender, + scan_rx: Mutex>>, +} + +impl AndroidBleBridge { + /// Build a bridge over a radio. + pub fn new(radio: Arc) -> Arc { + let (accept_tx, accept_rx) = mpsc::channel(CHANNEL_CAP); + let (scan_tx, scan_rx) = mpsc::channel(CHANNEL_CAP); + Arc::new(Self { + radio, + next_id: AtomicI64::new(1), + local_psm: AtomicU16::new(0), + listening: AtomicBool::new(false), + advertising: Mutex::new(None), + scanning: AtomicBool::new(false), + channels: Mutex::new(HashMap::new()), + connects: Mutex::new(HashMap::new()), + accept_tx, + accept_rx: Mutex::new(Some(accept_rx)), + scan_tx, + scan_rx: Mutex::new(Some(scan_rx)), + }) + } + + /// The PSM this radio's listener was bound to, or `0` if it has none. + pub fn local_psm(&self) -> u16 { + self.local_psm.load(Ordering::Relaxed) + } + + fn lock_channels(&self) -> std::sync::MutexGuard<'_, HashMap> { + self.channels.lock().unwrap_or_else(|e| e.into_inner()) + } + + /// Open this radio's listener if it has not been opened yet, and report + /// the PSM it bound. Idempotent on *success*, so re-resolving the slot + /// cannot open a second listener on the same radio. + /// + /// A failed attempt is deliberately not latched. Latching it would leave + /// `local_psm` at zero for the life of the radio, so everything downstream + /// would fall back to the configured PSM — and nothing would ever be + /// listening on it. One radio that lost a race with the Bluetooth stack + /// coming up would then be undialable until the service was restarted. + /// Nothing here loops, so the retry costs one call per slot resolve. + fn open_listener(&self) -> u16 { + if !self.listening.swap(true, Ordering::AcqRel) { + let psm = self.radio.listen(); + if psm == 0 { + self.listening.store(false, Ordering::Release); + debug!("no BLE listener could be opened on the installed radio"); + return 0; + } + self.local_psm.store(psm, Ordering::Relaxed); + debug!(psm, "BLE listener opened on the installed radio"); + // The advertisement can already be on the air carrying the + // configured fallback, because `start_advertising` is allowed to + // reach a radio whose listener has not opened yet. Now that a real + // PSM exists, it has to replace what is being announced. + self.readvertise(psm); + } + self.local_psm.load(Ordering::Relaxed) + } + + /// Advertise this radio's bound listener PSM, or `fallback` until the + /// platform has assigned one. + /// + /// Choosing between the two happens *under the advertising lock*, in the + /// same step as issuing it. Reading the bound PSM first and advertising + /// second — as two steps — loses the race the transport's own start + /// sequence runs: `listen` resolves an empty slot and reports the + /// configured PSM, a radio is installed in the gap, and `start_advertising` + /// then reads a zero bound PSM off it. Meanwhile the acceptor adopts that + /// same radio, opens its listener and announces the real PSM — after + /// which the first caller lands its stale fallback on top, and sticks. + /// + /// Sticking is what makes this fatal rather than untidy. A peer dials what + /// it hears, so an advertisement carrying a PSM nothing is listening on + /// makes this node permanently undialable: the platform rejects every + /// inbound L2CAP connect request as an unknown PSM, below the application, + /// so the node never learns why nobody reaches it. + fn advertise(&self, fallback: u16) { + let mut advertising = self.advertising.lock().unwrap_or_else(|e| e.into_inner()); + let bound = self.local_psm.load(Ordering::Relaxed); + self.issue_advert(&mut advertising, if bound != 0 { bound } else { fallback }); + } + + /// Move a *live* advertisement onto `psm`. + /// + /// Does nothing if this radio was never asked to advertise: opening a + /// listener is not itself such a request. + fn readvertise(&self, psm: u16) { + let mut advertising = self.advertising.lock().unwrap_or_else(|e| e.into_inner()); + if advertising.is_some() { + self.issue_advert(&mut advertising, psm); + } + } + + /// Idempotent on the PSM, not on the call: a *different* PSM re-issues the + /// advertisement rather than being swallowed, which is the whole point — + /// see [`Self::advertise`]. Repeating the same one does not restart the + /// advertiser, so the activations a slot-follower performs on every + /// resolve stay free. + fn issue_advert(&self, advertising: &mut Option, psm: u16) { + if *advertising == Some(psm) { + return; + } + *advertising = Some(psm); + // Told under the lock, so the radio ends up carrying whatever the last + // caller to *decide* chose. Releasing first would let a slower caller + // land its already-superseded PSM afterwards. + // `AndroidRadio::start_advertising` returns immediately by contract, so + // nothing blocks here. + self.radio.start_advertising(psm); + } + + /// Stop advertising, so a later request starts it again. + fn end_advertising(&self) { + let mut advertising = self.advertising.lock().unwrap_or_else(|e| e.into_inner()); + if advertising.take().is_some() { + self.radio.stop_advertising(); + } + } + + /// Start scanning if this radio is not scanning already. + fn begin_scanning(&self) { + if !self.scanning.swap(true, Ordering::AcqRel) { + self.radio.start_scanning(); + } + } + + /// Allocate a channel, registering the bridge-side half and returning the + /// stream-side half. + fn make_channel(&self, remote: BleAddr, send_mtu: u16, recv_mtu: u16) -> StreamEndpoints { + let ch_id = self.next_id.fetch_add(1, Ordering::Relaxed); + let (recv_tx, recv_rx) = mpsc::channel(CHANNEL_CAP); + let (send_tx, send_rx) = std::sync::mpsc::sync_channel(SEND_QUEUE_CAP); + let closed = Arc::new(AtomicBool::new(false)); + self.lock_channels().insert( + ch_id, + ChannelState { + recv_tx, + send_rx: Arc::new(Mutex::new(send_rx)), + closed: Arc::clone(&closed), + }, + ); + StreamEndpoints { + ch_id, + remote, + send_mtu: if send_mtu == 0 { + DEFAULT_BLE_MTU + } else { + send_mtu + }, + recv_mtu: if recv_mtu == 0 { + DEFAULT_BLE_MTU + } else { + recv_mtu + }, + recv_rx, + send_tx, + closed, + } + } + + // --- The surface the embedder drives --------------------------------- + + /// Report an inbound channel the radio accepted, and get back the channel + /// identifier to use for it. `0` means the transport is not accepting and + /// the socket should be closed. + pub fn deliver_inbound(&self, remote: BleAddr, send_mtu: u16, recv_mtu: u16) -> i64 { + let ep = self.make_channel(remote, send_mtu, recv_mtu); + let ch_id = ep.ch_id; + if self.accept_tx.try_send(ep).is_err() { + // Nobody is accepting, or the fan-in is saturated. Reclaim the + // half-registered channel rather than leaking it. + self.lock_channels().remove(&ch_id); + return 0; + } + ch_id + } + + /// Report the outcome of a dial started by [`AndroidRadio::connect`]. + /// + /// Returns the channel identifier on success, `0` on failure or if nothing + /// is waiting on this `connect_id` any more — which is the normal outcome + /// for a dial the transport already timed out. + pub fn deliver_connect_result( + &self, + connect_id: i64, + ok: bool, + remote: BleAddr, + send_mtu: u16, + recv_mtu: u16, + ) -> i64 { + let waiter = self + .connects + .lock() + .unwrap_or_else(|e| e.into_inner()) + .remove(&connect_id); + let Some(tx) = waiter else { return 0 }; + if !ok { + // Dropping the sender is what wakes the dial as a failure. + drop(tx); + return 0; + } + let ep = self.make_channel(remote, send_mtu, recv_mtu); + let ch_id = ep.ch_id; + if tx.send(ep).is_err() { + self.lock_channels().remove(&ch_id); + return 0; + } + ch_id + } + + /// Report one observed advertisement. + /// + /// `psm` is what the advertisement carried; pass `0` when it carried none, + /// which is what a legacy advertiser produces. `rssi` is passed through + /// unchanged when the platform reports one. + pub fn deliver_scan(&self, addr: BleAddr, psm: u16, rssi: Option) { + let advert = ScanAdvert { + addr, + psm: (psm != 0).then_some(psm), + rssi, + }; + if self.scan_tx.try_send(advert).is_err() { + trace!("BLE scan fan-in full or unattached; advert dropped"); + } + } + + /// Deliver bytes read from channel `ch_id`. + /// + /// Returns `false` when the channel is unknown or gone, which is the + /// signal to stop reading it. + pub fn deliver_recv(&self, ch_id: i64, data: &[u8]) -> bool { + let tx = self.lock_channels().get(&ch_id).map(|c| c.recv_tx.clone()); + match tx { + Some(tx) => tx.try_send(data.to_vec()).is_ok(), + None => false, + } + } + + /// Pull the next outbound packet for channel `ch_id`, blocking up to + /// `timeout`. + /// + /// `None` means either that nothing was queued within the timeout or that + /// the channel is gone. The caller distinguishes them with + /// [`channel_open`](Self::channel_open): still open means loop again, closed + /// means stop the writer. Without that distinction a writer cannot tell an + /// idle link from a dead one and spins on a closed channel forever. + pub fn next_send(&self, ch_id: i64, timeout: Duration) -> Option> { + // Clone the receiver and the closed flag out, then release the channel + // map before blocking: holding it across the wait would stall every + // other channel's create and close for up to `timeout`. + let (send_rx, closed) = { + let guard = self.lock_channels(); + let state = guard.get(&ch_id)?; + (Arc::clone(&state.send_rx), Arc::clone(&state.closed)) + }; + let rx = send_rx.lock().unwrap_or_else(|e| e.into_inner()); + match rx.recv_timeout(timeout) { + Ok(bytes) => Some(bytes), + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => None, + // The sending half is gone, so the stream was dropped. Mark the + // channel closed so the writer's next `channel_open` says so. + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => { + closed.store(true, Ordering::Relaxed); + None + } + } + } + + /// Whether channel `ch_id` is still open — registered, and not marked + /// closed by a dropped stream. + pub fn channel_open(&self, ch_id: i64) -> bool { + self.lock_channels() + .get(&ch_id) + .map(|state| !state.closed.load(Ordering::Relaxed)) + .unwrap_or(false) + } + + /// Report that channel `ch_id` is closed. + /// + /// Dropping the bridge-side sender is what wakes the stream's `recv` with + /// a zero-length read, which is this transport's peer-closed signal. + pub fn channel_closed(&self, ch_id: i64) { + if let Some(state) = self.lock_channels().remove(&ch_id) { + state.closed.store(true, Ordering::Relaxed); + } + } +} + +// ============================================================================ +// BleIo implementation +// ============================================================================ + +/// Reclaims an in-flight dial's slot in [`AndroidBleBridge::connects`] when +/// the dial goes away. +/// +/// A dial cannot rely on its own code running to clean up after itself. The +/// transport bounds the wait with [`tokio::time::timeout`] and, when that +/// fires, simply *drops* the future — neither arm of the `rx.await` in +/// [`AndroidIo::connect`] gets to run. Without this the sender would sit in +/// the map waiting for an answer the embedder may never send, and a node whose +/// dials keep timing out would grow that map for the life of the process. +/// +/// Answering a dial removes the entry first, so this is then a no-op. +struct InFlightDial { + bridge: Arc, + connect_id: i64, +} + +impl Drop for InFlightDial { + fn drop(&mut self) { + self.bridge + .connects + .lock() + .unwrap_or_else(|e| e.into_inner()) + .remove(&self.connect_id); + } +} + +/// Map an operation attempted with no radio installed onto a transport error. +/// +/// Deliberately an ordinary I/O error rather than `NotSupported`: the radio is +/// absent right now, not absent in principle, and the caller should retry. +fn no_radio(op: &str) -> TransportError { + TransportError::Io(std::io::Error::other(format!( + "no BLE radio installed ({op})" + ))) +} + +/// [`BleIo`] over whatever radio is currently installed in a [`BleRadioSlot`]. +pub struct AndroidIo { + slot: Arc, + /// Shared with the acceptor and the scanner, so a radio installed later + /// gets told everything the transport asked for at startup. + intent: Arc, +} + +impl AndroidIo { + /// Drive whatever radio is installed in `slot`, now or later. + pub fn new(slot: Arc) -> Self { + Self { + slot, + intent: Arc::new(RadioIntent::default()), + } + } + + /// The slot this backend resolves. + pub fn slot(&self) -> &Arc { + &self.slot + } +} + +/// One live L2CAP channel. +pub struct AndroidStream { + ch_id: i64, + remote: BleAddr, + send_mtu: u16, + recv_mtu: u16, + recv_rx: AsyncMutex>>, + send_tx: std::sync::mpsc::SyncSender>, + closed: Arc, + radio: Arc, +} + +impl AndroidStream { + fn from_endpoints(ep: StreamEndpoints, radio: Arc) -> Self { + Self { + ch_id: ep.ch_id, + remote: ep.remote, + send_mtu: ep.send_mtu, + recv_mtu: ep.recv_mtu, + recv_rx: AsyncMutex::new(ep.recv_rx), + send_tx: ep.send_tx, + closed: ep.closed, + radio, + } + } +} + +impl Drop for AndroidStream { + fn drop(&mut self) { + // Mark before closing: a writer that wakes between the two must see a + // closed channel rather than an open one with no reader. + self.closed.store(true, Ordering::Relaxed); + self.radio.close_channel(self.ch_id); + } +} + +impl BleStream for AndroidStream { + async fn send(&self, data: &[u8]) -> Result<(), TransportError> { + // A channel push, never a foreign call: the embedder's writer pulls + // this out via `next_send`. The queue is shallow (`SEND_QUEUE_CAP`) and + // this waits for a slot rather than dropping, so backpressure reaches + // the layers above instead of the link bufferbloating. + let mut payload = data.to_vec(); + loop { + if self.closed.load(Ordering::Relaxed) { + return Err(TransportError::SendFailed("BLE channel closed".into())); + } + match self.send_tx.try_send(payload) { + Ok(()) => return Ok(()), + Err(std::sync::mpsc::TrySendError::Full(unsent)) => { + payload = unsent; + tokio::time::sleep(SEND_BACKPRESSURE_POLL).await; + } + Err(std::sync::mpsc::TrySendError::Disconnected(_)) => { + return Err(TransportError::SendFailed("BLE channel gone".into())); + } + } + } + } + + async fn recv(&self, buf: &mut [u8]) -> Result { + match self.recv_rx.lock().await.recv().await { + Some(packet) => { + let n = packet.len().min(buf.len()); + buf[..n].copy_from_slice(&packet[..n]); + Ok(n) + } + // The bridge dropped its sender: peer closed. A zero-length read + // is this transport's close signal. + None => Ok(0), + } + } + + fn send_mtu(&self) -> u16 { + self.send_mtu + } + + fn recv_mtu(&self) -> u16 { + self.recv_mtu + } + + fn remote_addr(&self) -> &BleAddr { + &self.remote + } +} + +/// What the transport asked this backend to do, held apart from any one +/// radio so it can be re-issued against the next one. +/// +/// The transport issues `listen`, `start_advertising` and `start_scanning` +/// exactly once, at startup. A radio installed after that — or one replacing +/// another — has to be told the same things, or a node whose radio restarted +/// would sit there advertising nothing and scanning for nobody. Recording the +/// request rather than only performing it is what makes that possible. +#[derive(Default)] +struct RadioIntent { + listen: AtomicBool, + advertise: AtomicBool, + /// PSM to advertise when the radio has no listener PSM of its own. + advertise_fallback_psm: AtomicU16, + scan: AtomicBool, +} + +impl RadioIntent { + /// Issue everything asked for so far against `bridge`. + /// + /// Each activation is idempotent per radio, so it does not matter which + /// slot-follower gets here first after a swap, or how often. + fn apply(&self, bridge: &AndroidBleBridge) { + if self.listen.load(Ordering::Relaxed) { + bridge.open_listener(); + } + if self.advertise.load(Ordering::Relaxed) { + // The bridge picks between its bound PSM and this fallback itself, + // under the lock that also issues the advertisement. Deciding out + // here would reintroduce the stale-PSM race — see + // [`AndroidBleBridge::advertise`]. + bridge.advertise(self.advertise_fallback_psm.load(Ordering::Relaxed)); + } + if self.scan.load(Ordering::Relaxed) { + bridge.begin_scanning(); + } + } +} + +/// Resolve the slot, re-applying `intent` whenever the installed bridge is not +/// the one `seen` was resolved from. +/// +/// Returns the current bridge, or `None` if the slot is empty. The caller +/// parks on [`BleRadioSlot::changed`] in that case. +fn resolve( + slot: &BleRadioSlot, + seen: &mut Option>, + intent: &RadioIntent, +) -> Option> { + let current = slot.current()?; + let same = seen + .as_ref() + .is_some_and(|prev| Arc::ptr_eq(prev, ¤t)); + if !same { + intent.apply(¤t); + *seen = Some(Arc::clone(¤t)); + } + Some(current) +} + +/// Yields inbound channels the installed radio accepted. +/// +/// Follows the slot: with no radio installed it parks rather than failing, and +/// a radio installed later is picked up and activated without the transport +/// being restarted. +pub struct AndroidAcceptor { + slot: Arc, + intent: Arc, + /// The bridge `rx` was taken from, so a swap is detectable. + seen: Option>, + rx: Option>, + radio: Option>, +} + +impl BleAcceptor for AndroidAcceptor { + type Stream = AndroidStream; + + async fn accept(&mut self) -> Result { + loop { + // Register interest before reading the slot, so an install that + // races this resolve wakes the park below rather than being lost. + let changed = self.slot.changed.notified(); + let previous = self.seen.clone(); + match resolve(&self.slot, &mut self.seen, &self.intent) { + Some(bridge) => { + if !previous.is_some_and(|prev| Arc::ptr_eq(&prev, &bridge)) { + self.rx = bridge + .accept_rx + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take(); + self.radio = Some(Arc::clone(&bridge.radio)); + } + let Some(rx) = self.rx.as_mut() else { + // Another acceptor already took this bridge's fan-in. + // Park until the slot changes rather than spinning. + changed.await; + continue; + }; + let radio = self.radio.clone().expect("radio set alongside rx"); + tokio::select! { + inbound = rx.recv() => match inbound { + Some(ep) => return Ok(AndroidStream::from_endpoints(ep, radio)), + // The bridge's fan-in is gone; wait for a new one. + None => self.rx = None, + }, + _ = changed => {} + } + } + None => changed.await, + } + } + } +} + +/// Yields advertisements the installed radio observed. +/// +/// Slot-following in the same way as [`AndroidAcceptor`]. It never reports +/// end-of-scan, because an absent radio is a gap rather than a stop. +pub struct AndroidScanner { + slot: Arc, + intent: Arc, + seen: Option>, + rx: Option>, +} + +impl BleScanner for AndroidScanner { + async fn next(&mut self) -> Option { + loop { + let changed = self.slot.changed.notified(); + let previous = self.seen.clone(); + match resolve(&self.slot, &mut self.seen, &self.intent) { + Some(bridge) => { + if !previous.is_some_and(|prev| Arc::ptr_eq(&prev, &bridge)) { + self.rx = bridge + .scan_rx + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take(); + } + let Some(rx) = self.rx.as_mut() else { + changed.await; + continue; + }; + tokio::select! { + advert = rx.recv() => match advert { + Some(advert) => return Some(advert), + None => self.rx = None, + }, + _ = changed => {} + } + } + None => changed.await, + } + } + } +} + +impl BleIo for AndroidIo { + type Stream = AndroidStream; + type Acceptor = AndroidAcceptor; + type Scanner = AndroidScanner; + + async fn listen(&self, psm: u16) -> Result<(AndroidAcceptor, u16), TransportError> { + // The requested PSM is a fallback only. This platform assigns the + // listener's PSM, so what gets reported back — and therefore what gets + // advertised — is whatever the radio bound. + self.intent.listen.store(true, Ordering::Relaxed); + let mut seen = None; + let bound = match resolve(&self.slot, &mut seen, &self.intent) { + Some(bridge) => { + let bound = bridge.local_psm(); + if bound == 0 { psm } else { bound } + } + // No radio yet. Succeed anyway: the acceptor opens a listener on + // whichever radio turns up, and until then there is simply nothing + // to accept. Failing here would instead put the whole transport + // into a failed state it never retries out of. + None => psm, + }; + let (rx, radio) = match seen.as_ref() { + Some(bridge) => ( + bridge + .accept_rx + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take(), + Some(Arc::clone(&bridge.radio)), + ), + None => (None, None), + }; + Ok(( + AndroidAcceptor { + slot: Arc::clone(&self.slot), + intent: Arc::clone(&self.intent), + seen, + rx, + radio, + }, + bound, + )) + } + + async fn connect(&self, addr: &BleAddr, psm: u16) -> Result { + let bridge = self.slot.current().ok_or_else(|| no_radio("connect"))?; + let connect_id = bridge.next_id.fetch_add(1, Ordering::Relaxed); + let (tx, rx) = oneshot::channel(); + bridge + .connects + .lock() + .unwrap_or_else(|e| e.into_inner()) + .insert(connect_id, tx); + // Reclaims the entry however this dial ends — including the ending + // that runs no code here at all. See [`InFlightDial`]. + let _in_flight = InFlightDial { + bridge: Arc::clone(&bridge), + connect_id, + }; + bridge.radio.connect(connect_id, addr, psm); + // The transport bounds this wait itself, so there is no timeout here. + match rx.await { + Ok(ep) => Ok(AndroidStream::from_endpoints(ep, Arc::clone(&bridge.radio))), + Err(_) => Err(TransportError::Io(std::io::Error::other(format!( + "BLE connect to {addr} failed" + )))), + } + } + + async fn start_advertising(&self, psm: u16) -> Result<(), TransportError> { + self.intent + .advertise_fallback_psm + .store(psm, Ordering::Relaxed); + self.intent.advertise.store(true, Ordering::Relaxed); + // With no radio installed there is nothing to advertise on yet, and + // that is not an error: the intent is recorded, and whichever radio + // turns up next is told to advertise as it is adopted. + if let Some(bridge) = self.slot.current() { + // `psm` is only a fallback here. The radio may have bound its own + // by now — including in the window between this call and the + // `listen` that produced `psm` — and the bridge is what resolves + // that, atomically with putting it on the air. + bridge.advertise(psm); + } + Ok(()) + } + + async fn stop_advertising(&self) -> Result<(), TransportError> { + self.intent.advertise.store(false, Ordering::Relaxed); + if let Some(bridge) = self.slot.current() { + bridge.end_advertising(); + } + Ok(()) + } + + async fn start_scanning(&self) -> Result { + self.intent.scan.store(true, Ordering::Relaxed); + let mut seen = None; + let rx = match resolve(&self.slot, &mut seen, &self.intent) { + Some(bridge) => bridge + .scan_rx + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take(), + // As with `listen`: succeed with nothing to yield yet, rather than + // leaving the transport with no scanner for the rest of its life. + None => None, + }; + Ok(AndroidScanner { + slot: Arc::clone(&self.slot), + intent: Arc::clone(&self.intent), + seen, + rx, + }) + } + + fn local_addr(&self) -> Result { + // The platform does not expose the adapter's address, and nothing in + // this transport needs it: peers are keyed by node address, and the + // remote address of a channel comes from the channel itself. + Ok(BleAddr { + adapter: ANDROID_ADAPTER.to_string(), + device: [0; 6], + }) + } + + fn adapter_name(&self) -> &str { + ANDROID_ADAPTER + } +} + +// ============================================================================ +// Tests +// ============================================================================ + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::atomic::AtomicU32; + + /// Records what the transport asked the radio to do, and lets a test play + /// the part of the embedder's foreign-function layer. + #[derive(Default)] + struct MockRadio { + listen_psm: AtomicU16, + listen_calls: AtomicU32, + advertised_psm: AtomicU16, + advertise_calls: AtomicU32, + scan_calls: AtomicU32, + closed_channels: Mutex>, + dials: Mutex>, + } + + impl MockRadio { + fn with_psm(psm: u16) -> Arc { + let radio = Self::default(); + radio.listen_psm.store(psm, Ordering::Relaxed); + Arc::new(radio) + } + + fn dials(&self) -> Vec<(i64, BleAddr, u16)> { + self.dials.lock().unwrap().clone() + } + } + + impl AndroidRadio for MockRadio { + fn listen(&self) -> u16 { + self.listen_calls.fetch_add(1, Ordering::Relaxed); + self.listen_psm.load(Ordering::Relaxed) + } + fn connect(&self, connect_id: i64, addr: &BleAddr, psm: u16) { + self.dials + .lock() + .unwrap() + .push((connect_id, addr.clone(), psm)); + } + fn start_advertising(&self, psm: u16) { + self.advertise_calls.fetch_add(1, Ordering::Relaxed); + self.advertised_psm.store(psm, Ordering::Relaxed); + } + fn stop_advertising(&self) {} + fn start_scanning(&self) { + self.scan_calls.fetch_add(1, Ordering::Relaxed); + } + fn stop_scanning(&self) {} + fn close_channel(&self, ch_id: i64) { + self.closed_channels.lock().unwrap().push(ch_id); + } + } + + fn addr(n: u8) -> BleAddr { + BleAddr { + adapter: ANDROID_ADAPTER.to_string(), + device: [0xAA, 0xBB, 0xCC, 0xDD, 0xEE, n], + } + } + + fn slot_with(radio: Arc) -> (Arc, Arc) { + let bridge = AndroidBleBridge::new(radio); + let slot = Arc::new(BleRadioSlot::new()); + slot.install(Arc::clone(&bridge)); + (slot, bridge) + } + + /// The PSM handed back by `listen` is the one the radio actually bound, + /// not the one that was requested — the whole reason the seam reports a + /// PSM at all. + #[tokio::test] + async fn listen_reports_the_psm_the_radio_bound() { + let radio = MockRadio::with_psm(0x0099); + let (slot, _bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + + let (_acceptor, bound) = io.listen(0x0085).await.unwrap(); + + assert_eq!(bound, 0x0099, "the OS-assigned PSM wins over the request"); + assert_eq!(radio.listen_calls.load(Ordering::Relaxed), 1); + } + + /// And that bound PSM is what gets advertised, so peers dial where the + /// listener really is. + #[tokio::test] + async fn advertising_carries_the_bound_psm() { + let radio = MockRadio::with_psm(0x0099); + let (slot, _bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + + let (_acceptor, bound) = io.listen(0x0085).await.unwrap(); + io.start_advertising(bound).await.unwrap(); + + assert_eq!(radio.advertised_psm.load(Ordering::Relaxed), 0x0099); + } + + /// A radio that cannot open a listener reports zero, and the configured + /// PSM is then the honest answer. + #[tokio::test] + async fn a_radio_with_no_listener_falls_back_to_the_requested_psm() { + let radio = MockRadio::with_psm(0); + let (slot, _bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + + let (_acceptor, bound) = io.listen(0x0085).await.unwrap(); + + assert_eq!(bound, 0x0085); + } + + /// Bytes round-trip both ways: pushed in by the embedder and read by the + /// stream, written by the stream and pulled out by the embedder. + #[tokio::test] + async fn bytes_round_trip_through_the_bridge() { + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + let (mut acceptor, _) = io.listen(0x0085).await.unwrap(); + + let ch_id = bridge.deliver_inbound(addr(1), 512, 512); + assert!(ch_id > 0); + assert!(bridge.deliver_recv(ch_id, b"inbound")); + + let stream = acceptor.accept().await.unwrap(); + assert_eq!(stream.remote_addr(), &addr(1)); + assert_eq!(stream.send_mtu(), 512); + + let mut buf = [0u8; 64]; + let n = stream.recv(&mut buf).await.unwrap(); + assert_eq!(&buf[..n], b"inbound"); + + stream.send(b"outbound").await.unwrap(); + let pulled = bridge.next_send(ch_id, Duration::from_millis(100)).unwrap(); + assert_eq!(pulled, b"outbound"); + } + + /// An unknown channel MTU is treated as an unconfigured one, not as a + /// zero-capacity link. + #[tokio::test] + async fn an_unknown_mtu_falls_back_to_the_transport_default() { + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + let (mut acceptor, _) = io.listen(0x0085).await.unwrap(); + + bridge.deliver_inbound(addr(2), 0, 0); + let stream = acceptor.accept().await.unwrap(); + + assert_eq!(stream.send_mtu(), DEFAULT_BLE_MTU); + assert_eq!(stream.recv_mtu(), DEFAULT_BLE_MTU); + } + + /// A writer must be able to tell "nothing queued" from "channel gone". + /// Both make `next_send` return `None`; `channel_open` is what separates + /// them, and without it a writer spins on a dead channel forever. + #[tokio::test] + async fn next_send_distinguishes_an_idle_channel_from_a_closed_one() { + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + let (mut acceptor, _) = io.listen(0x0085).await.unwrap(); + + let ch_id = bridge.deliver_inbound(addr(3), 512, 512); + let stream = acceptor.accept().await.unwrap(); + + assert!(bridge.next_send(ch_id, Duration::from_millis(5)).is_none()); + assert!(bridge.channel_open(ch_id), "idle, but still open"); + + drop(stream); + assert!(bridge.next_send(ch_id, Duration::from_millis(5)).is_none()); + assert!( + !bridge.channel_open(ch_id), + "a dropped stream must read as closed, not as idle", + ); + assert_eq!(radio.closed_channels.lock().unwrap().as_slice(), &[ch_id]); + } + + /// The bridge reporting a channel closed surfaces as a zero-length read, + /// which is this transport's peer-closed signal. + #[tokio::test] + async fn a_closed_channel_reads_as_end_of_stream() { + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + let (mut acceptor, _) = io.listen(0x0085).await.unwrap(); + + let ch_id = bridge.deliver_inbound(addr(4), 512, 512); + let stream = acceptor.accept().await.unwrap(); + + bridge.channel_closed(ch_id); + let mut buf = [0u8; 8]; + assert_eq!(stream.recv(&mut buf).await.unwrap(), 0); + } + + /// The outbound queue is bounded: a sender that outruns the writer waits + /// for a slot instead of the queue growing without bound. + #[tokio::test(start_paused = true)] + async fn a_full_outbound_queue_backpressures_rather_than_growing() { + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + let (mut acceptor, _) = io.listen(0x0085).await.unwrap(); + + let ch_id = bridge.deliver_inbound(addr(5), 512, 512); + let stream = acceptor.accept().await.unwrap(); + + for _ in 0..SEND_QUEUE_CAP { + stream.send(b"x").await.unwrap(); + } + + let mut over_cap = Box::pin(stream.send(b"one too many")); + assert!( + futures::poll!(over_cap.as_mut()).is_pending(), + "the {SEND_QUEUE_CAP}-deep queue is full, so the sender must wait", + ); + + // Draining one packet frees exactly one slot. + assert!(bridge.next_send(ch_id, Duration::from_millis(1)).is_some()); + over_cap.await.unwrap(); + } + + /// Two dials in flight at once resolve to their own streams even when the + /// embedder answers them out of order. + #[tokio::test] + async fn concurrent_dials_resolve_by_connect_id() { + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = Arc::new(AndroidIo::new(slot)); + + let io_a = Arc::clone(&io); + let dial_a = tokio::spawn(async move { io_a.connect(&addr(6), 0x00C1).await }); + let io_b = Arc::clone(&io); + let dial_b = tokio::spawn(async move { io_b.connect(&addr(7), 0x00C2).await }); + + // Wait for both dials to register before answering either. + let dials = loop { + let dials = radio.dials(); + if dials.len() == 2 { + break dials; + } + tokio::task::yield_now().await; + }; + let for_addr = |want: &BleAddr| { + dials + .iter() + .find(|(_, a, _)| a == want) + .cloned() + .expect("dial registered") + }; + let (id_a, _, psm_a) = for_addr(&addr(6)); + let (id_b, _, psm_b) = for_addr(&addr(7)); + assert_eq!((psm_a, psm_b), (0x00C1, 0x00C2), "each dial keeps its PSM"); + + // Answer B first, then A. + bridge.deliver_connect_result(id_b, true, addr(7), 512, 512); + bridge.deliver_connect_result(id_a, true, addr(6), 512, 512); + + assert_eq!(dial_a.await.unwrap().unwrap().remote_addr(), &addr(6)); + assert_eq!(dial_b.await.unwrap().unwrap().remote_addr(), &addr(7)); + } + + /// A dial the embedder reports as failed surfaces as an error rather than + /// hanging. + #[tokio::test] + async fn a_failed_dial_surfaces_as_an_error() { + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = Arc::new(AndroidIo::new(slot)); + + let dialer = Arc::clone(&io); + let dial = tokio::spawn(async move { dialer.connect(&addr(8), 0x0085).await }); + let connect_id = loop { + if let Some(id) = radio.dials().first().map(|(id, _, _)| *id) { + break id; + } + tokio::task::yield_now().await; + }; + + bridge.deliver_connect_result(connect_id, false, addr(8), 0, 0); + assert!(dial.await.unwrap().is_err()); + } + + /// An advertised PSM reaches the shared driver through the scan advert, + /// and an advert that carried none decodes to `None` rather than to zero. + #[tokio::test] + async fn scan_adverts_carry_the_advertised_psm() { + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + + let mut scanner = io.start_scanning().await.unwrap(); + assert_eq!(radio.scan_calls.load(Ordering::Relaxed), 1); + + bridge.deliver_scan(addr(9), 0x00C1, Some(-42)); + assert_eq!( + scanner.next().await, + Some(ScanAdvert { + addr: addr(9), + psm: Some(0x00C1), + rssi: Some(-42), + }), + ); + + bridge.deliver_scan(addr(10), 0, None); + assert_eq!(scanner.next().await, Some(ScanAdvert::new(addr(10)))); + } + + /// With an empty slot the transport still starts: `listen` and + /// `start_scanning` succeed with nothing to yield yet, and only a dial — + /// which the transport retries anyway — fails. + #[tokio::test] + async fn an_empty_slot_starts_cleanly_and_only_dials_fail() { + let slot = Arc::new(BleRadioSlot::new()); + let io = AndroidIo::new(Arc::clone(&slot)); + + assert!(!slot.is_installed()); + let (_acceptor, bound) = io.listen(0x0085).await.unwrap(); + assert_eq!(bound, 0x0085, "nothing bound one, so the request stands"); + assert!(io.start_scanning().await.is_ok()); + assert!(io.start_advertising(0x0085).await.is_ok()); + assert!(io.connect(&addr(11), 0x0085).await.is_err()); + } + + /// The requirement the slot exists for: a radio that arrives after the + /// transport is already running is adopted in place. The acceptor opens a + /// listener on it and starts delivering, with no node rebuild. + #[tokio::test] + async fn a_radio_installed_later_is_adopted_without_a_restart() { + let slot = Arc::new(BleRadioSlot::new()); + let io = AndroidIo::new(Arc::clone(&slot)); + let (mut acceptor, _) = io.listen(0x0085).await.unwrap(); + io.start_advertising(0x0085).await.unwrap(); + + assert!(io.connect(&addr(12), 0x0085).await.is_err()); + + let accepting = tokio::spawn(async move { acceptor.accept().await }); + + let radio = MockRadio::with_psm(0x00A1); + let bridge = AndroidBleBridge::new(Arc::clone(&radio) as Arc); + slot.install(Arc::clone(&bridge)); + + // The acceptor picks the new radio up and opens its listener. + let ch_id = loop { + let ch_id = bridge.deliver_inbound(addr(12), 512, 512); + if ch_id > 0 { + break ch_id; + } + tokio::task::yield_now().await; + }; + let stream = accepting.await.unwrap().unwrap(); + assert_eq!(stream.remote_addr(), &addr(12)); + assert!(bridge.channel_open(ch_id)); + assert_eq!( + radio.listen_calls.load(Ordering::Relaxed), + 1, + "the listener is opened once on the radio that turned up", + ); + assert_eq!( + radio.advertised_psm.load(Ordering::Relaxed), + 0x00A1, + "and it advertises its own listener PSM, not the startup fallback", + ); + } + + /// A radio installed *between* `listen` and `start_advertising` — the one + /// window the transport's start sequence leaves open — is told to + /// advertise the configured fallback, because its listener has not been + /// opened yet and it has no PSM of its own to offer. The bound PSM arrives + /// a moment later, and has to reach the air: an advertisement left + /// carrying the fallback points every peer at a PSM nothing is listening + /// on, and the platform rejects their connect requests below the + /// application, so the node is silently undialable for as long as it runs. + #[tokio::test] + async fn a_bound_psm_replaces_a_fallback_that_is_already_on_the_air() { + let slot = Arc::new(BleRadioSlot::new()); + let io = AndroidIo::new(Arc::clone(&slot)); + // Startup with an empty slot: nothing to bind, so the fallback stands. + let (mut acceptor, bound) = io.listen(0x0085).await.unwrap(); + assert_eq!(bound, 0x0085, "no radio, so the request is all there is"); + + // The radio turns up here — after `listen` resolved an empty slot and + // before the transport gets to `start_advertising`. + let radio = MockRadio::with_psm(0x00A1); + let bridge = AndroidBleBridge::new(Arc::clone(&radio) as Arc); + slot.install(Arc::clone(&bridge)); + + io.start_advertising(bound).await.unwrap(); + assert_eq!( + radio.advertised_psm.load(Ordering::Relaxed), + 0x0085, + "with no listener open yet, the fallback is the only PSM there is", + ); + + // Driving the acceptor adopts the radio, which opens its listener. + let accepting = tokio::spawn(async move { acceptor.accept().await }); + loop { + if bridge.deliver_inbound(addr(16), 512, 512) > 0 { + break; + } + tokio::task::yield_now().await; + } + accepting.await.unwrap().unwrap(); + + assert_eq!(bridge.local_psm(), 0x00A1); + assert_eq!( + radio.advertised_psm.load(Ordering::Relaxed), + 0x00A1, + "the bound PSM must replace the fallback on the air, not be swallowed \ + as 'already advertising'", + ); + } + + /// The counterpart: re-issuing is keyed on the PSM changing, so the + /// repeated activations a slot-follower performs do not restart the + /// advertiser on every resolve. + #[tokio::test] + async fn re_advertising_the_same_psm_does_not_restart_the_advertiser() { + let radio = MockRadio::with_psm(0x00A1); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(Arc::clone(&slot)); + let (_acceptor, bound) = io.listen(0x0085).await.unwrap(); + io.start_advertising(bound).await.unwrap(); + io.start_advertising(bound).await.unwrap(); + bridge.advertise(0x0085); + + assert_eq!(radio.advertised_psm.load(Ordering::Relaxed), 0x00A1); + assert_eq!( + radio.advertise_calls.load(Ordering::Relaxed), + 1, + "one PSM, one advertisement", + ); + } + + /// The stale-PSM race the *lock* is for, rather than the swallowed-call + /// one above. A caller reaches this radio before its listener is open, so + /// the only PSM it can offer is the configured fallback; the listener + /// opens underneath it and announces the real one. Whichever order those + /// two land in, the radio must be left carrying the bound PSM — which is + /// only guaranteed because the choice between bound and fallback is made + /// under the same lock that issues the advertisement, not before it. + #[tokio::test] + async fn a_fallback_decided_before_the_listener_opened_cannot_land_last() { + let radio = MockRadio::with_psm(0x00A1); + let bridge = AndroidBleBridge::new(Arc::clone(&radio) as Arc); + + // The listener opens first, and nothing is on the air yet, so opening + // it does not start an advertisement of its own. + assert_eq!(bridge.open_listener(), 0x00A1); + assert_eq!( + radio.advertise_calls.load(Ordering::Relaxed), + 0, + "opening a listener is not a request to advertise", + ); + + // Now the caller that was holding the fallback gets there. + bridge.advertise(0x0085); + assert_eq!( + radio.advertised_psm.load(Ordering::Relaxed), + 0x00A1, + "the fallback is a fallback: a bound PSM outranks it, whenever the \ + caller happened to be handed it", + ); + } + + /// A radio whose listener could not be opened is retried on the next slot + /// resolve rather than being written off. Latching the failure would pin + /// `local_psm` at zero for the life of the radio, so this node would go on + /// advertising a configured PSM nothing is listening on — the same silent + /// undialability, reached from the other direction. + #[tokio::test] + async fn a_listener_that_failed_to_open_is_retried_on_the_next_resolve() { + let radio = Arc::new(MockRadio::default()); // listen() reports 0 + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(Arc::clone(&slot)); + + let (_acceptor, bound) = io.listen(0x0085).await.unwrap(); + assert_eq!(bound, 0x0085, "nothing bound, so the request stands"); + assert_eq!(radio.listen_calls.load(Ordering::Relaxed), 1); + + // The Bluetooth stack comes up; the next resolve gets a real listener. + radio.listen_psm.store(0x00A1, Ordering::Relaxed); + let _scanner = io.start_scanning().await.unwrap(); + + assert_eq!(radio.listen_calls.load(Ordering::Relaxed), 2, "retried"); + assert_eq!(bridge.local_psm(), 0x00A1); + } + + /// Opening a listener moves a *live* advertisement onto the bound PSM. + /// This is the ordering the transport's start sequence actually produces + /// when a radio is installed in the gap between `listen` and + /// `start_advertising`, and it is the one the old `AtomicBool` swallowed. + #[tokio::test] + async fn opening_a_listener_moves_a_live_advert_onto_the_bound_psm() { + let radio = MockRadio::with_psm(0x00A1); + let bridge = AndroidBleBridge::new(Arc::clone(&radio) as Arc); + + bridge.advertise(0x0085); + assert_eq!(radio.advertised_psm.load(Ordering::Relaxed), 0x0085); + + bridge.open_listener(); + assert_eq!( + radio.advertised_psm.load(Ordering::Relaxed), + 0x00A1, + "the live advert follows the listener onto its real PSM", + ); + assert_eq!(radio.advertise_calls.load(Ordering::Relaxed), 2); + } + + /// A dial the transport gives up on must not leave its entry behind. The + /// transport bounds every dial with its own timeout and drops the future + /// when it fires, so nothing in `connect` runs — the reclamation has to + /// hang off the drop, or a node that keeps timing out grows the in-flight + /// map for the life of the process. This is the BLE-side half of the + /// Android leak: on the embedder's side the same dial is a socket holding + /// an LE connect slot. + #[tokio::test] + async fn a_dial_the_transport_abandons_reclaims_its_in_flight_slot() { + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + + let peer = addr(19); + for _ in 0..50 { + // A dial that is never answered, dropped the way + // `tokio::time::timeout` drops one it has given up on. + let mut dial = Box::pin(io.connect(&peer, 0x0085)); + assert!(futures::poll!(dial.as_mut()).is_pending()); + drop(dial); + } + + assert_eq!(radio.dials().len(), 50, "every dial reached the radio"); + assert!( + bridge + .connects + .lock() + .unwrap_or_else(|e| e.into_inner()) + .is_empty(), + "abandoned dials must not accumulate in the in-flight map", + ); + } + + /// And an answer that arrives for a dial nobody is waiting on any more is + /// reported as such rather than silently allocating a channel — otherwise + /// the embedder would start a reader and writer over a stream the + /// transport had already written off. + #[tokio::test] + async fn answering_an_abandoned_dial_allocates_nothing() { + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + + let peer = addr(20); + let mut dial = Box::pin(io.connect(&peer, 0x0085)); + assert!(futures::poll!(dial.as_mut()).is_pending()); + let connect_id = radio.dials()[0].0; + drop(dial); + + assert_eq!( + bridge.deliver_connect_result(connect_id, true, addr(20), 512, 512), + 0, + "nothing is waiting, so the embedder is told to close its socket", + ); + assert!(bridge.lock_channels().is_empty()); + } + + /// Replacing a radio re-activates the transport's intent on the new one + /// and routes new inbound channels there, while a stream opened on the old + /// radio keeps the radio it was opened on. + #[tokio::test] + async fn replacing_a_radio_re_activates_it_and_leaves_live_streams_alone() { + let first = MockRadio::with_psm(0x00A1); + let (slot, first_bridge) = slot_with(Arc::clone(&first)); + let io = AndroidIo::new(Arc::clone(&slot)); + let (mut acceptor, _) = io.listen(0x0085).await.unwrap(); + + let old_ch = first_bridge.deliver_inbound(addr(14), 512, 512); + let old_stream = acceptor.accept().await.unwrap(); + + let second = MockRadio::with_psm(0x00B2); + let second_bridge = AndroidBleBridge::new(Arc::clone(&second) as Arc); + slot.install(Arc::clone(&second_bridge)); + + let new_ch = loop { + let ch_id = second_bridge.deliver_inbound(addr(15), 512, 512); + if ch_id > 0 { + break ch_id; + } + tokio::task::yield_now().await; + }; + let new_stream = acceptor.accept().await.unwrap(); + assert_eq!(new_stream.remote_addr(), &addr(15)); + assert_eq!( + second.listen_calls.load(Ordering::Relaxed), + 1, + "the replacement radio gets its own listener", + ); + assert_eq!(second_bridge.local_psm(), 0x00B2); + + // The stream opened on the first radio still belongs to it. + old_stream.send(b"still mine").await.unwrap(); + assert_eq!( + first_bridge + .next_send(old_ch, Duration::from_millis(10)) + .as_deref(), + Some(&b"still mine"[..]), + ); + assert!(second_bridge.channel_open(new_ch)); + drop(old_stream); + assert_eq!(first.closed_channels.lock().unwrap().as_slice(), &[old_ch]); + } + + /// Two nodes in one process each drive their own radio. This is the test a + /// process-global bridge cannot pass, and the reason the slot is owned by + /// a node. + #[tokio::test] + async fn two_slots_do_not_interfere() { + let radio_a = MockRadio::with_psm(0x00A1); + let radio_b = MockRadio::with_psm(0x00B2); + let (slot_a, bridge_a) = slot_with(Arc::clone(&radio_a)); + let (slot_b, bridge_b) = slot_with(Arc::clone(&radio_b)); + let io_a = AndroidIo::new(slot_a); + let io_b = AndroidIo::new(slot_b); + + let (mut acceptor_a, bound_a) = io_a.listen(0x0085).await.unwrap(); + let (mut acceptor_b, bound_b) = io_b.listen(0x0085).await.unwrap(); + assert_eq!((bound_a, bound_b), (0x00A1, 0x00B2)); + + bridge_a.deliver_inbound(addr(16), 512, 512); + let stream_a = acceptor_a.accept().await.unwrap(); + assert_eq!(stream_a.remote_addr(), &addr(16)); + + bridge_b.deliver_inbound(addr(17), 512, 512); + let stream_b = acceptor_b.accept().await.unwrap(); + assert_eq!(stream_b.remote_addr(), &addr(17)); + + // Neither radio saw the other's traffic. + assert_eq!(radio_a.listen_calls.load(Ordering::Relaxed), 1); + assert_eq!(radio_b.listen_calls.load(Ordering::Relaxed), 1); + drop(stream_a); + drop(stream_b); + assert_eq!(radio_a.closed_channels.lock().unwrap().len(), 1); + assert_eq!(radio_b.closed_channels.lock().unwrap().len(), 1); + } + + /// This backend is byte-oriented, so it is exactly the case the shared + /// reframing adapter exists for: a peer's packet arriving in three pieces, + /// and two packets arriving in one, still read back whole. Composed here + /// against the real backend rather than against a mock stream, because + /// this pairing is what a device actually does. + #[tokio::test] + async fn fragmented_and_coalesced_deliveries_reassemble() { + use tokio::io::AsyncReadExt; + + let radio = MockRadio::with_psm(0x0099); + let (slot, bridge) = slot_with(Arc::clone(&radio)); + let io = AndroidIo::new(slot); + let (mut acceptor, _) = io.listen(0x0085).await.unwrap(); + + let ch_id = bridge.deliver_inbound(addr(18), 512, 512); + let stream = acceptor.accept().await.unwrap(); + let mut reader = super::super::stream_read::BleStreamRead::new(Arc::new(stream), 512); + + // One logical message split across three deliveries. + for piece in [&b"abc"[..], b"defg", b"hij"] { + assert!(bridge.deliver_recv(ch_id, piece)); + } + let mut whole = [0u8; 10]; + reader.read_exact(&mut whole).await.unwrap(); + assert_eq!(&whole, b"abcdefghij"); + + // Two logical messages coalesced into one delivery: the tail is not + // lost. + assert!(bridge.deliver_recv(ch_id, b"0123456789")); + let mut head = [0u8; 4]; + reader.read_exact(&mut head).await.unwrap(); + assert_eq!(&head, b"0123"); + let mut tail = [0u8; 6]; + reader.read_exact(&mut tail).await.unwrap(); + assert_eq!(&tail, b"456789"); + } +} diff --git a/src/transport/ble/mod.rs b/src/transport/ble/mod.rs index 077953b6..fb20b58b 100644 --- a/src/transport/ble/mod.rs +++ b/src/transport/ble/mod.rs @@ -29,6 +29,16 @@ pub mod addr; pub mod io; +/// A backend whose radio is supplied by the embedder rather than opened in +/// process. +/// +/// Compiled under `cfg(test)` on every host as well as on the platform that +/// will select it, so its channel machinery, slot semantics and connect +/// routing are exercised by an ordinary test run on an ordinary runner. The +/// platform build of it is linted but executed nowhere, which is exactly why +/// the logic must not be behind a platform-only gate. +#[cfg(any(target_os = "android", test))] +pub mod io_android; #[cfg(bluer_available)] pub mod io_linux; pub mod neighbor; From e0a639fd64f9b3566c4fa41fba0e4bf309565863 Mon Sep 17 00:00:00 2001 From: Arjen <18398758+Origami74@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:35:28 +0100 Subject: [PATCH 6/9] build: gate BLE on backend availability, and let an embedder install the radio Two changes that are not themselves BLE code: the build gate that decides where the transport exists, and the node-level seam an application uses to hand it a radio. They are kept out of the BLE commits so that those stay purely about the transport. The BLE transport was compiled only on target_os = "linux", which conflates the transport with one of its backends. Nothing above the BleIo seam has a platform dependency, and the part that does is already selected separately. So the module gate becomes ble_available, defined as bluer_available or Android: the set of platforms with a concrete backend, deliberately not the set that could plausibly have Bluetooth. That distinction is the whole point. The mock arm was previously written as "anything that is not BlueZ", so widening the module gate alone would have handed a platform a transport that compiles, starts, reports itself Up and never peers, with no error anywhere to find it. The backend cascade is now explicitly three-way -- BlueZ, an embedder-supplied radio, and the in-memory double under cfg(test) only -- with a compile_error! for the remaining case. A compile_error! cannot fire in a test build, which is the build everybody runs, so a unit test asserts the same condition from the other side by reading cfg! values for the target rather than for the profile. Nothing about which backend runs changes on any platform that builds today. glibc-linux still gets BlueZ. macOS and Windows still have no BLE. Android is the only new platform and it gets a real backend rather than the mock. musl now has no BLE deliberately, where before the module compiled there and resolved to the mock; a musl node with BLE configured logs a warning and starts without the transport rather than running one that could never peer. The node side gains an optional BLE radio slot. It is built whether or not a radio has been installed, because the backend resolves the slot per operation, so an embedder that starts Bluetooth after the node is running is adopted in place. Arming twice returns the same slot rather than replacing it, since a slot may already hold a live radio a second call must not orphan, and the slot is per-node rather than a process global because a global collapses as soon as two nodes share a process. --- build.rs | 17 +++++- src/node/lifecycle/mod.rs | 4 +- src/node/mod.rs | 94 +++++++++++++++++++++++++++++++- src/node/tests/mod.rs | 2 +- src/node/tests/unit.rs | 109 ++++++++++++++++++++++++++++++++++++++ src/transport/ble/mod.rs | 67 ++++++++++++++++++++--- src/transport/mod.rs | 46 ++++++++-------- 7 files changed, 303 insertions(+), 36 deletions(-) diff --git a/build.rs b/build.rs index 06ab0d5f..0b61861c 100644 --- a/build.rs +++ b/build.rs @@ -43,7 +43,22 @@ fn main() { println!("cargo:rustc-check-cfg=cfg(bluer_available)"); let target_os = std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default(); let target_env = std::env::var("CARGO_CFG_TARGET_ENV").unwrap_or_default(); - if target_os == "linux" && target_env != "musl" { + let bluer_available = target_os == "linux" && target_env != "musl"; + if bluer_available { println!("cargo:rustc-cfg=bluer_available"); } + + // Whether the BLE transport is compiled at all. + // + // This is the set of platforms that have a concrete `BleIo` backend, not + // the set that could plausibly have one. A platform listed here with no + // backend behind it does not get "BLE, degraded" — it gets an in-memory + // transport that starts, reports itself up and never peers, with no error + // anywhere. Add a platform here only in the same change that adds its + // backend; `transport::ble` carries a compile-time tripwire that refuses + // a build where the two disagree. + println!("cargo:rustc-check-cfg=cfg(ble_available)"); + if bluer_available || target_os == "android" { + println!("cargo:rustc-cfg=ble_available"); + } } diff --git a/src/node/lifecycle/mod.rs b/src/node/lifecycle/mod.rs index 4e5f7c20..1e40b272 100644 --- a/src/node/lifecycle/mod.rs +++ b/src/node/lifecycle/mod.rs @@ -2474,7 +2474,7 @@ impl Node { } } } else if addr.transport == "ble" { - #[cfg(bluer_available)] + #[cfg(ble_available)] { match self.resolve_ble_addr(&addr.addr) { Ok(result) => result, @@ -2489,7 +2489,7 @@ impl Node { } } } - #[cfg(not(bluer_available))] + #[cfg(not(ble_available))] { debug!(transport = %addr.transport, "BLE transport not available on this build"); continue; diff --git a/src/node/mod.rs b/src/node/mod.rs index ab0c30f9..ec93cb04 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -541,6 +541,17 @@ pub struct Node { /// TUN interface name (for cleanup). tun_name: Option, + /// Slot the embedder installs its BLE radio into, armed by + /// [`Self::enable_app_owned_ble_radio`]. `None` unless armed. + /// + /// Gated on the BLE transport existing *and* on its backend being the + /// embedder-supplied one — the same condition + /// `transport::ble::io_android` itself is compiled under, so the seam is + /// absent on platforms whose radio is opened in process, and present in a + /// test build so its contract is covered on an ordinary runner. + #[cfg(all(ble_available, any(target_os = "android", test)))] + ble_radio: Option>, + // === Index-Based Session Dispatch === /// Allocator for session indices. index_allocator: IndexAllocator, @@ -833,6 +844,8 @@ impl Node { )), tun_state, tun_name: None, + #[cfg(all(ble_available, any(target_os = "android", test)))] + ble_radio: None, index_allocator: IndexAllocator::new(), peers_by_index: HashMap::new(), pending_outbound: HashMap::new(), @@ -996,6 +1009,8 @@ impl Node { )), tun_state, tun_name: None, + #[cfg(all(ble_available, any(target_os = "android", test)))] + ble_radio: None, index_allocator: IndexAllocator::new(), peers_by_index: HashMap::new(), pending_outbound: HashMap::new(), @@ -1189,6 +1204,33 @@ impl Node { } } + // Create BLE transport instances over an embedder-supplied radio. + // Built whether or not a radio is installed yet: the backend resolves + // the slot per operation, so one that arrives later is adopted in + // place rather than needing the node rebuilt around it. + #[cfg(all(target_os = "android", not(bluer_available), not(test)))] + if let Some(slot) = self.ble_radio.clone() { + let ble_instances: Vec<_> = self + .config() + .transports + .ble + .iter() + .map(|(name, config)| (name.map(|s| s.to_string()), config.clone())) + .collect(); + for (name, ble_config) in ble_instances { + let transport_id = self.allocate_transport_id(); + let mut ble = crate::transport::ble::BleTransport::new( + transport_id, + name, + ble_config, + crate::transport::ble::io_android::AndroidIo::new(Arc::clone(&slot)), + packet_tx.clone(), + ); + ble.set_local_pubkey(self.identity().pubkey().serialize()); + transports.push(TransportHandle::Ble(ble)); + } + } + transports } @@ -1255,7 +1297,7 @@ impl Node { /// Resolve a BLE address string (`"adapter/AA:BB:CC:DD:EE:FF"`) to a /// (TransportId, TransportAddr) pair by finding the BLE transport /// instance matching the adapter name. - #[cfg(bluer_available)] + #[cfg(ble_available)] fn resolve_ble_addr(&self, addr_str: &str) -> Result<(TransportId, TransportAddr), NodeError> { let ta = TransportAddr::from_string(addr_str); let adapter = crate::transport::ble::addr::adapter_from_addr(&ta).ok_or_else(|| { @@ -3339,6 +3381,56 @@ impl Node { udp_fd_rx } + /// Set up an **app-owned BLE radio**: the embedder supplies the radio the + /// BLE transport drives, because on this platform there is no + /// Rust-reachable one to open. Call this after [`Node::new`] and + /// **before** [`Self::start`] — the transport is built during `start`, and + /// only a node armed by then has a slot to build it over. + /// + /// Returns the slot. Installing, replacing and clearing a radio through it + /// is safe at any time, from any thread, including long after the node is + /// running: + /// + /// ```no_run + /// # async fn f(node: &mut fips::Node, radio: std::sync::Arc) + /// # -> Result<(), Box> { + /// let slot = node.enable_app_owned_ble_radio(); // after new(), before start() + /// node.start().await?; + /// // ...whenever the embedder's radio service comes up, and again each + /// // time it restarts: + /// slot.install(fips::transport::ble::io_android::AndroidBleBridge::new(radio)); + /// # Ok(()) + /// # } + /// ``` + /// + /// The lateness is the point rather than a convenience. The radio belongs + /// to a service whose lifetime is not the node's: the user can turn it on + /// after the mesh is already running, and off and on again, and each start + /// typically produces a fresh radio. A node that had to be built around an + /// existing radio would make that mean "tear the node down and rebuild + /// it", dropping every peer, session and route for as long as + /// re-handshaking takes. So the transport is built and started whether or + /// not a radio is installed, and resolves the slot per operation: it + /// listens and scans against whichever radio is there, dials fail while + /// there is none, and everything recovers on its own when one appears. + /// Streams already open keep the radio they were opened on rather than + /// migrating. + /// + /// Deliberately narrow, and shaped like the [`Self::enable_app_owned_tun`] + /// seam it sits beside: one call, no callbacks, and no lifecycle contract + /// beyond the slot outliving the node. Arming twice returns the same slot, + /// so a second call cannot orphan a radio installed through the first. The + /// seam does not exist on platforms whose BLE backend is opened in + /// process, since there is nothing there for an embedder to supply. + #[cfg(all(ble_available, any(target_os = "android", test)))] + pub fn enable_app_owned_ble_radio( + &mut self, + ) -> Arc { + Arc::clone(self.ble_radio.get_or_insert_with(|| { + Arc::new(crate::transport::ble::io_android::BleRadioSlot::new()) + })) + } + /// Address the built-in `.fips` DNS responder is listening on, or `None` /// when it is not running (`dns.enabled = false`, the bind failed, or the /// node is stopped). diff --git a/src/node/tests/mod.rs b/src/node/tests/mod.rs index e33d946e..dd56ed0d 100644 --- a/src/node/tests/mod.rs +++ b/src/node/tests/mod.rs @@ -6,7 +6,7 @@ use crate::utils::index::SessionIndex; use std::time::Duration; mod acl; -#[cfg(target_os = "linux")] +#[cfg(ble_available)] mod ble; mod bloom; mod bloom_poison; diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index a6157729..12b0ff7b 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -3544,6 +3544,115 @@ fn app_owned_udp_fd_seam_second_arm_replaces_the_first() { ); } +/// The app-owned BLE radio seam. The slot is live from the moment it is armed +/// — before `start()`, which is when the transport that reads it gets built — +/// and installing a radio through it is a slot operation, not a node one. +#[cfg(all(ble_available, any(target_os = "android", test)))] +#[test] +fn app_owned_ble_radio_seam_hands_out_a_live_slot_before_start() { + use crate::transport::ble::io_android::{AndroidBleBridge, BleRadioSlot}; + + let mut node = make_node(); + let slot: std::sync::Arc = node.enable_app_owned_ble_radio(); + + assert!( + !slot.is_installed(), + "arming supplies the slot, not a radio to put in it", + ); + + slot.install(AndroidBleBridge::new(std::sync::Arc::new( + test_radio::TestRadio, + ))); + assert!(slot.is_installed(), "the embedder installs whenever it can"); + + slot.clear(); + assert!(!slot.is_installed(), "and can take it away again"); +} + +/// Arming twice returns the same slot, so a second call cannot orphan a radio +/// installed through the first. This is where the seam deliberately differs +/// from `enable_app_owned_udp_fd`, whose last arming wins: a channel can be +/// replaced because nothing was delivered on it yet, while a slot may already +/// be holding the embedder's live radio. +#[cfg(all(ble_available, any(target_os = "android", test)))] +#[test] +fn app_owned_ble_radio_seam_second_arm_returns_the_same_slot() { + use crate::transport::ble::io_android::AndroidBleBridge; + + let mut node = make_node(); + let first = node.enable_app_owned_ble_radio(); + first.install(AndroidBleBridge::new(std::sync::Arc::new( + test_radio::TestRadio, + ))); + + let second = node.enable_app_owned_ble_radio(); + + assert!( + std::sync::Arc::ptr_eq(&first, &second), + "re-arming must not hand back a different slot", + ); + assert!( + second.is_installed(), + "the radio installed through the first handle is still there", + ); +} + +/// The slot is per-node state, which is the whole reason it is not a process +/// global: two nodes in one process each drive their own radio. +#[cfg(all(ble_available, any(target_os = "android", test)))] +#[test] +fn app_owned_ble_radio_slots_are_per_node() { + use crate::transport::ble::io_android::AndroidBleBridge; + + let mut node_a = make_node(); + let mut node_b = make_node(); + let slot_a = node_a.enable_app_owned_ble_radio(); + let slot_b = node_b.enable_app_owned_ble_radio(); + + slot_a.install(AndroidBleBridge::new(std::sync::Arc::new( + test_radio::TestRadio, + ))); + + assert!(slot_a.is_installed()); + assert!( + !slot_b.is_installed(), + "node B's radio is node B's — no shared or global slot", + ); +} + +/// A node that never armed the seam has no slot to hand the transport, which +/// is how a build with the embedder-supplied backend distinguishes "no radio +/// yet" from "this embedder does not supply one at all". +#[cfg(all(ble_available, any(target_os = "android", test)))] +#[test] +fn app_owned_ble_radio_seam_is_absent_until_armed() { + let node = make_node(); + assert!(node.ble_radio.is_none()); +} + +#[cfg(all(ble_available, any(target_os = "android", test)))] +mod test_radio { + use crate::transport::ble::addr::BleAddr; + use crate::transport::ble::io_android::AndroidRadio; + + /// A radio that does nothing. These tests are about the seam handing one + /// over, not about what it then does — that is covered where the backend + /// lives. + pub(super) struct TestRadio; + + impl AndroidRadio for TestRadio { + fn listen(&self) -> u16 { + 0 + } + fn connect(&self, _connect_id: i64, _addr: &BleAddr, _psm: u16) {} + fn start_advertising(&self, _psm: u16) {} + fn stop_advertising(&self) {} + fn start_scanning(&self) {} + fn stop_scanning(&self) {} + fn close_channel(&self, _ch_id: i64) {} + } +} + /// The embedder-facing DNS contract, end to end. /// /// An embedder that owns the TUN fd (Android `VpnService`) has no system DNS diff --git a/src/transport/ble/mod.rs b/src/transport/ble/mod.rs index fb20b58b..69842010 100644 --- a/src/transport/ble/mod.rs +++ b/src/transport/ble/mod.rs @@ -16,10 +16,13 @@ //! //! ## Architecture //! -//! Transport logic (pool, neighbor, lifecycle) is separated from the -//! BlueZ/bluer stack via the `BleIo` trait. `BluerIo` provides the real -//! implementation (behind `cfg(bluer_available)`); `MockBleIo` provides -//! an in-memory test double for CI without hardware. +//! Transport logic (pool, neighbor, lifecycle) is separated from any one +//! Bluetooth stack via the `BleIo` trait. `BluerIo` drives BlueZ (behind +//! `cfg(bluer_available)`), [`io_android::AndroidIo`] drives a radio the +//! embedder supplies, and `MockBleIo` is an in-memory double for tests +//! without hardware. Which one `DefaultBleTransport` resolves to is decided +//! by the cascade below, and the whole module is compiled only on platforms +//! that have one of them — see `ble_available` in `build.rs`. //! //! ## Connection Pool //! @@ -80,16 +83,41 @@ use tracing::{debug, info, trace, warn}; /// dialled at when it advertises nothing. pub const DEFAULT_PSM: u16 = 0x0085; -/// Concrete BLE transport type for use in TransportHandle. +/// Concrete BLE transport type for use in `TransportHandle`. /// -/// Production builds on glibc-linux use `BluerIo` (real BlueZ stack). -/// Test builds, musl-linux, and non-Linux platforms use `MockBleIo`. +/// Three arms, in priority order: an in-process BlueZ stack where one exists, +/// otherwise a radio the embedder supplies, otherwise — and *only* in a test +/// build — the in-memory double. +/// +/// The mock arm is deliberately not written as "anything that is not BlueZ". +/// That phrasing is what makes widening the module gate dangerous: a platform +/// added to `ble_available` without a backend would silently land on an +/// in-memory transport that compiles, starts, reports [`TransportState::Up`] +/// and never peers, with nothing anywhere to say so. The tripwire below makes +/// that state unrepresentable instead. #[cfg(all(bluer_available, not(test)))] pub type DefaultBleTransport = BleTransport; -#[cfg(any(not(bluer_available), test))] +#[cfg(all(target_os = "android", not(bluer_available), not(test)))] +pub type DefaultBleTransport = BleTransport; + +#[cfg(test)] pub type DefaultBleTransport = BleTransport; +// The tripwire. This module is only compiled when `ble_available`, so +// reaching here means a platform declared it has BLE while having no concrete +// backend to provide it. It cannot fire today; it exists for whoever next +// widens `ble_available`, and it fails the build rather than shipping a +// transport that quietly never connects. +#[cfg(all(not(test), not(bluer_available), not(target_os = "android")))] +compile_error!( + "this target is `ble_available` but has no concrete `BleIo` backend. \ + Add its backend and an arm to the `DefaultBleTransport` cascade in \ + src/transport/ble/mod.rs, or drop the target from `ble_available` in \ + build.rs. Falling back to the in-memory mock in a non-test build would \ + produce a BLE transport that starts, reports itself up, and never peers." +); + // ============================================================================ // BLE Transport // ============================================================================ @@ -1708,6 +1736,29 @@ mod tests { assert!(p.position(&a(1)).is_none()); } + /// The mock backend is a *test* backend. Any target that compiles this + /// module must have a real one behind it, or a release build of it would + /// ship a BLE transport that starts, reports itself up, and never peers. + /// + /// The `compile_error!` above is what enforces that in a non-test build — + /// and by construction it cannot fire in a test build, which is exactly + /// the build everybody runs. This closes that gap: the `cfg!` values below + /// are evaluated for the *target*, not for the test profile, so this + /// asserts the same condition the tripwire does, from the one place a + /// developer will actually see it. + #[test] + fn a_target_that_compiles_this_module_has_a_real_backend() { + let has_concrete_backend = cfg!(bluer_available) || cfg!(target_os = "android"); + assert!( + has_concrete_backend, + "target {} is `ble_available` but has no concrete `BleIo` backend, \ + so a non-test build of it would select the in-memory mock. Add \ + its backend and an arm to the `DefaultBleTransport` cascade, or \ + drop it from `ble_available` in build.rs.", + std::env::consts::OS, + ); + } + /// Deterministic x-only pubkey for exchange tests. fn test_pubkey(seed: u8) -> [u8; 32] { let secp = Secp256k1::new(); diff --git a/src/transport/mod.rs b/src/transport/mod.rs index 4620f9f6..bb39a6d3 100644 --- a/src/transport/mod.rs +++ b/src/transport/mod.rs @@ -15,10 +15,10 @@ pub mod udp; #[cfg(any(target_os = "linux", target_os = "macos"))] pub mod ethernet; -#[cfg(target_os = "linux")] +#[cfg(ble_available)] pub mod ble; -#[cfg(target_os = "linux")] +#[cfg(ble_available)] use ble::DefaultBleTransport; #[cfg(any(target_os = "linux", target_os = "macos"))] use ethernet::EthernetTransport; @@ -673,7 +673,7 @@ pub enum TransportHandle { /// Nym mixnet transport (via SOCKS5). Nym(NymTransport), /// BLE L2CAP transport. - #[cfg(target_os = "linux")] + #[cfg(ble_available)] Ble(DefaultBleTransport), /// In-process loopback transport (test harness only). #[cfg(test)] @@ -690,7 +690,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.start_async().await, TransportHandle::Tor(t) => t.start_async().await, TransportHandle::Nym(t) => t.start_async().await, - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.start_async().await, #[cfg(test)] TransportHandle::Loopback(t) => t.start_async().await, @@ -706,7 +706,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.stop_async().await, TransportHandle::Tor(t) => t.stop_async().await, TransportHandle::Nym(t) => t.stop_async().await, - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.stop_async().await, #[cfg(test)] TransportHandle::Loopback(t) => t.stop_async().await, @@ -722,7 +722,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.send_async(addr, data).await, TransportHandle::Tor(t) => t.send_async(addr, data).await, TransportHandle::Nym(t) => t.send_async(addr, data).await, - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.send_async(addr, data).await, #[cfg(test)] TransportHandle::Loopback(t) => t.send_async(addr, data).await, @@ -738,7 +738,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.transport_id(), TransportHandle::Tor(t) => t.transport_id(), TransportHandle::Nym(t) => t.transport_id(), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.transport_id(), #[cfg(test)] TransportHandle::Loopback(t) => t.transport_id(), @@ -754,7 +754,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.name(), TransportHandle::Tor(t) => t.name(), TransportHandle::Nym(t) => t.name(), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.name(), #[cfg(test)] TransportHandle::Loopback(_) => None, @@ -770,7 +770,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.transport_type(), TransportHandle::Tor(t) => t.transport_type(), TransportHandle::Nym(t) => t.transport_type(), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.transport_type(), #[cfg(test)] TransportHandle::Loopback(t) => t.transport_type(), @@ -786,7 +786,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.state(), TransportHandle::Tor(t) => t.state(), TransportHandle::Nym(t) => t.state(), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.state(), #[cfg(test)] TransportHandle::Loopback(t) => t.state(), @@ -802,7 +802,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.mtu(), TransportHandle::Tor(t) => t.mtu(), TransportHandle::Nym(t) => t.mtu(), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.mtu(), #[cfg(test)] TransportHandle::Loopback(t) => t.mtu(), @@ -821,7 +821,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.link_mtu(addr), TransportHandle::Tor(t) => t.link_mtu(addr), TransportHandle::Nym(t) => t.link_mtu(addr), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.link_mtu(addr), #[cfg(test)] TransportHandle::Loopback(t) => t.link_mtu(addr), @@ -837,7 +837,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.local_addr(), TransportHandle::Tor(_) => None, TransportHandle::Nym(_) => None, - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(_) => None, #[cfg(test)] TransportHandle::Loopback(_) => None, @@ -857,7 +857,7 @@ impl TransportHandle { TransportHandle::Tcp(_) => None, TransportHandle::Tor(_) => None, TransportHandle::Nym(_) => None, - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(_) => None, #[cfg(test)] TransportHandle::Loopback(_) => None, @@ -873,7 +873,7 @@ impl TransportHandle { TransportHandle::Tcp(_) => None, TransportHandle::Tor(_) => None, TransportHandle::Nym(_) => None, - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(_) => None, #[cfg(test)] TransportHandle::Loopback(_) => None, @@ -913,7 +913,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.discover(), TransportHandle::Tor(t) => t.discover(), TransportHandle::Nym(t) => t.discover(), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.discover(), #[cfg(test)] TransportHandle::Loopback(t) => t.discover(), @@ -929,7 +929,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.auto_connect(), TransportHandle::Tor(t) => t.auto_connect(), TransportHandle::Nym(t) => t.auto_connect(), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.auto_connect(), #[cfg(test)] TransportHandle::Loopback(t) => t.auto_connect(), @@ -945,7 +945,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.accept_connections(), TransportHandle::Tor(t) => t.accept_connections(), TransportHandle::Nym(t) => t.accept_connections(), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.accept_connections(), #[cfg(test)] TransportHandle::Loopback(t) => t.accept_connections(), @@ -967,7 +967,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.connect_async(addr).await, TransportHandle::Tor(t) => t.connect_async(addr).await, TransportHandle::Nym(t) => t.connect_async(addr).await, - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.connect_async(addr).await, #[cfg(test)] TransportHandle::Loopback(_) => Ok(()), // connectionless @@ -987,7 +987,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.connection_state_sync(addr), TransportHandle::Tor(t) => t.connection_state_sync(addr), TransportHandle::Nym(t) => t.connection_state_sync(addr), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.connection_state_sync(addr), #[cfg(test)] TransportHandle::Loopback(_) => ConnectionState::Connected, @@ -1006,7 +1006,7 @@ impl TransportHandle { TransportHandle::Tcp(t) => t.close_connection_async(addr).await, TransportHandle::Tor(t) => t.close_connection_async(addr).await, TransportHandle::Nym(t) => t.close_connection_async(addr).await, - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => t.close_connection_async(addr).await, #[cfg(test)] TransportHandle::Loopback(_) => {} // connectionless no-op @@ -1031,7 +1031,7 @@ impl TransportHandle { TransportHandle::Tcp(_) => TransportCongestion::default(), TransportHandle::Tor(_) => TransportCongestion::default(), TransportHandle::Nym(_) => TransportCongestion::default(), - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(_) => TransportCongestion::default(), #[cfg(test)] TransportHandle::Loopback(_) => TransportCongestion::default(), @@ -1059,7 +1059,7 @@ impl TransportHandle { TransportHandle::Nym(t) => { serde_json::to_value(t.stats().snapshot()).unwrap_or_default() } - #[cfg(target_os = "linux")] + #[cfg(ble_available)] TransportHandle::Ble(t) => { serde_json::to_value(t.stats().snapshot()).unwrap_or_default() } From 4633c9e89a08afdd9ec17848e0c306ee28032f05 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sun, 23 Aug 2026 18:01:38 +0100 Subject: [PATCH 7/9] Run each inbound BLE handshake off the accept loop, bounded The accept loop ran the pre-handshake pubkey exchange inline, so a peer that connected an L2CAP channel and then said nothing held the loop for the full 5-second exchange deadline and no other inbound connection was accepted in that window. The max-connections argument the loop was given was never used, so the effective inbound concurrency was one. Move the per-connection work into admit_inbound and spawn it, with eight handshakes allowed in flight. At the bound, abort the oldest pending handshake rather than waiting for a slot: waiting would leave the same denial standing at eight connections instead of one, and a healthy exchange is a single round trip, so anything still pending under a flood is overwhelmingly the flooder's. Count the aborts in the transport stats. Keep the tasks in a JoinSet owned by the accept loop rather than behind a semaphore. Stopping the transport aborts that task and nothing else, so the set is what stops the handshakes; bare spawns would survive stop and could insert into a pool it had just drained. The budget is a module constant and deliberately not the pool capacity: an operator who sets max_connections low would otherwise get a serial accept loop back, which is the defect itself. Give the send half of the pubkey exchange the same deadline the receive half already had. A peer that stops draining its channel could park that write indefinitely, which also reached the outbound connect and scan-probe paths. --- CHANGELOG.md | 23 ++ src/transport/ble/mod.rs | 480 ++++++++++++++++++++++++++++--------- src/transport/ble/stats.rs | 9 + 3 files changed, 402 insertions(+), 110 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4be426b3..dee4f962 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1570,6 +1570,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 doing so, which is a denial of new sessions rather than the unbounded memory growth it replaces. +- One inbound BLE connector can no longer stall every other inbound + connection. The accept loop ran the pre-handshake pubkey exchange inline, so + a peer that connected an L2CAP channel and then said nothing held the loop + for the full 5-second exchange deadline and no other inbound connection was + accepted in that window; the maximum-connections argument the loop was given + was never used, so the effective concurrency was one. Each inbound connection + now runs its handshake in its own task, up to eight in flight, and at that + bound the oldest pending handshake is aborted to make room rather than the + loop waiting for a slot: a healthy exchange is one round trip, so anything + still pending under a flood is overwhelmingly the flooder's, and a genuinely + slow peer that is aborted reconnects, which is better than never being + accepted at all. Aborted handshakes are counted in the transport's stats as + `handshakes_aborted`. The tasks live in a set owned by the accept loop, so + stopping the transport stops them too and none can insert into a pool that + stop has just drained. Separately, the send half of the pubkey exchange had + no deadline at all while the receive half had one, so a peer that stopped + draining its channel could park the write forever; it now shares the same + 5-second deadline, which also covers the outbound connect and scan-probe + paths. **What this does not close**: eight simultaneous silent connectors + still occupy the whole in-flight budget, and no BlueZ hardware was exercised, + so the controller's own concurrent-link limit and accept backlog depth stay + unmeasured. + - An accepted inbound TCP connection no longer holds a slot indefinitely without sending anything. The cap was tested at accept and the pool insert and counter bump followed with no read in between, while the frame reader's diff --git a/src/transport/ble/mod.rs b/src/transport/ble/mod.rs index 69842010..a0146394 100644 --- a/src/transport/ble/mod.rs +++ b/src/transport/ble/mod.rs @@ -846,11 +846,17 @@ async fn pubkey_exchange( reader: &mut BleStreamRead, local_pubkey: &[u8; 32], ) -> Result { - // Send our pubkey + let timeout = std::time::Duration::from_secs(PUBKEY_EXCHANGE_TIMEOUT_SECS); + + // Send our pubkey. The deadline matters as much as the one below it: a + // peer that never drains the L2CAP channel stalls the write instead. let mut msg = [0u8; PUBKEY_EXCHANGE_SIZE]; msg[0] = PUBKEY_EXCHANGE_PREFIX; msg[1..].copy_from_slice(local_pubkey); - stream.send(&msg).await?; + match tokio::time::timeout(timeout, stream.send(&msg)).await { + Ok(result) => result?, + Err(_) => return Err(TransportError::Timeout), + } // Receive peer's pubkey (with timeout to prevent indefinite blocking) let mut buf = [0u8; PUBKEY_EXCHANGE_SIZE]; @@ -880,8 +886,30 @@ async fn pubkey_exchange( // in start_async, stopped in stop_async). BLE advertising overhead // is negligible (~0.15% duty cycle on advertising channels). -/// Accept loop: accepts inbound L2CAP connections, exchanges pubkeys, -/// and adds to pool. +/// Inbound handshakes allowed to be in flight at once. +/// +/// Deliberately independent of the pool capacity: that is the budget for +/// established links, and tying the two together would mean an operator who +/// sets `max_connections = 1` also gets a serial accept loop, which is the +/// defect this bound exists to close. A healthy exchange is one round trip +/// and completes in milliseconds, so this is never reached honestly. Raising +/// it lets a flood hold more concurrent handshakes; lowering it makes a +/// legitimate slow peer likelier to be aborted under flood. +const INBOUND_HANDSHAKE_INFLIGHT: usize = 8; + +/// Accept loop: accepts inbound L2CAP connections and hands each to its own +/// task for the pubkey exchange and pool insert. +/// +/// One iteration is bounded by `accept()` alone. Nothing a connecting peer +/// chooses to do can delay the next accept: the handshake runs off the loop, +/// and when the in-flight budget is full the oldest pending handshake is +/// aborted to make room rather than the loop waiting for one to finish. +/// +/// The in-flight tasks live in a `JoinSet` and not behind a `Semaphore` for a +/// reason that is easy to lose: `stop_async` aborts this task and nothing +/// else, so dropping the `JoinSet` with it is what stops the handshakes. Bare +/// `tokio::spawn` would leave them running past stop, able to insert into a +/// pool that stop has just drained. #[allow(clippy::too_many_arguments)] async fn accept_loop( mut acceptor: A, @@ -897,13 +925,25 @@ async fn accept_loop( A: io::BleAcceptor, A::Stream: 'static, { + let mut inflight: tokio::task::JoinSet<()> = tokio::task::JoinSet::new(); + // Spawn order, so the oldest handshake is the one evicted at the budget. + let mut pending: std::collections::VecDeque = + std::collections::VecDeque::new(); + loop { + // Reap anything finished. Neither call waits. `retain` rather than + // popping the front run, so a handshake that completed out of order + // still frees its slot instead of being aborted as the oldest later. + while inflight.try_join_next().is_some() {} + pending.retain(|handle| !handle.is_finished()); + match acceptor.accept().await { Ok(stream) => { let addr = stream.remote_addr().clone(); let ta = addr.to_transport_addr(); - // Skip if already connected (outbound won the race) + // Skip if already connected (outbound won the race). This + // awaits only our own mutex, never the peer. { let pool_guard = pool.lock().await; if pool_guard.contains(&ta) { @@ -912,118 +952,29 @@ async fn accept_loop( } } - let send_mtu = stream.send_mtu(); - let recv_mtu = stream.recv_mtu(); - let stream = Arc::new(stream); - // One reader across both phases — see `pubkey_exchange`. - let mut reader = BleStreamRead::new(Arc::clone(&stream), recv_mtu); - - // Pre-handshake pubkey exchange (temporary, pre-XX) - let mut peer_node_addr: Option = None; - if let Some(ref our_pubkey) = local_pubkey { - match pubkey_exchange(stream.as_ref(), &mut reader, our_pubkey).await { - Ok(peer_pubkey) => { - debug!(addr = %ta, "BLE inbound pubkey exchange complete"); - let peer_node = NodeAddr::from_pubkey(&peer_pubkey); - peer_node_addr = Some(peer_node); - let announced = announced_addr(&pool, &peer_node, &addr).await; - neighbor_buffer.add_peer_with_pubkey(&announced, peer_pubkey); - - // Already linked to this peer on another address? - // A peer using resolvable private addresses rotates - // continually, and every rotation dials in looking - // like a new device. Admitting those would put one - // peer in several pool slots and evict real ones. - // The incumbent link is kept: it is known-good, and - // a genuinely dead one is already reaped by the - // send-error and receive-loop paths. - let dup = { - let pool_guard = pool.lock().await; - pool_guard.find_by_node(&peer_node) - }; - if let Some(existing) = dup - && existing != ta - { - debug!( - addr = %ta, - role = "peripheral", - outcome = "duplicate-node-decline", - existing = %existing, - "BLE inbound: peer already connected on another address, dropping duplicate" - ); - stats.record_duplicate_node_decline(); - continue; - } - - // Cross-probe tie-breaker: smaller NodeAddr's - // outbound wins. If we're smaller, our outbound - // should win — drop this inbound. - if let Some(ref our_addr) = local_node_addr - && our_addr < &peer_node - { - stats.record_tiebreaker_drop(); - debug!( - addr = %ta, - role = "peripheral", - outcome = "tiebreaker-drop", - "BLE inbound tie-breaker: dropping (our addr < peer, outbound wins)" - ); - continue; - } - } - Err(e) => { - stats.record_pubkey_exchange_failure(); - debug!( - addr = %ta, role = "peripheral", - outcome = "pubkey-exchange-failed", error = %e, - "BLE inbound pubkey exchange failed" - ); - continue; - } - } + if pending.len() >= INBOUND_HANDSHAKE_INFLIGHT + && let Some(oldest) = pending.pop_front() + { + oldest.abort(); + stats.record_handshake_aborted(); + debug!( + addr = %ta, + budget = INBOUND_HANDSHAKE_INFLIGHT, + "BLE inbound handshake budget full, aborting the oldest" + ); } - // Spawn receive loop - let recv_task = tokio::spawn(receive_loop( - reader, - ta.clone(), + let handle = inflight.spawn(admit_inbound( + stream, Arc::clone(&pool), packet_tx.clone(), transport_id, Arc::clone(&stats), - recv_mtu, + local_pubkey, + Arc::clone(&neighbor_buffer), + local_node_addr, )); - - let conn = BleConnection { - stream, - recv_task: Some(recv_task), - send_mtu, - recv_mtu, - established_at: tokio::time::Instant::now(), - is_static: false, - addr, - node_addr: peer_node_addr, - }; - - let mut pool_guard = pool.lock().await; - match pool_guard.insert(ta.clone(), conn) { - Ok(Some(evicted)) => { - stats.record_pool_eviction(); - info!(addr = %ta, evicted = %evicted, "BLE inbound accepted (evicted peer)"); - } - Ok(None) => { - info!(addr = %ta, send_mtu, recv_mtu, "BLE inbound connection accepted"); - } - Err(e) => { - stats.record_connection_rejected(); - warn!( - addr = %ta, role = "peripheral", outcome = "pool-rejected", - error = %e, "BLE pool full, inbound connection rejected" - ); - continue; - } - } - stats.record_connection_accepted(); + pending.push_back(handle); } Err(e) => { warn!(error = %e, "BLE accept error"); @@ -1033,6 +984,142 @@ async fn accept_loop( } } +/// Run one inbound connection's pubkey exchange and admit it to the pool. +/// +/// Runs off the accept loop so a peer that never answers delays nobody else. +/// Everything the accept loop used to do inline lives here, including the +/// duplicate-node decline and the cross-probe tie-break that `fix/platform-ble` +/// added: moving the work off the loop must not drop the checks that guard it. +/// The loop's `continue` becomes `return` — this task admits one connection. +#[allow(clippy::too_many_arguments)] +async fn admit_inbound( + stream: S, + pool: Arc>>>, + packet_tx: PacketTx, + transport_id: TransportId, + stats: Arc, + local_pubkey: Option<[u8; 32]>, + neighbor_buffer: Arc, + local_node_addr: Option, +) where + S: BleStream + 'static, +{ + let addr = stream.remote_addr().clone(); + let ta = addr.to_transport_addr(); + let send_mtu = stream.send_mtu(); + let recv_mtu = stream.recv_mtu(); + let stream = Arc::new(stream); + // One reader across both phases — see `pubkey_exchange`. + let mut reader = BleStreamRead::new(Arc::clone(&stream), recv_mtu); + + // Pre-handshake pubkey exchange (temporary, pre-XX) + let mut peer_node_addr: Option = None; + if let Some(ref our_pubkey) = local_pubkey { + match pubkey_exchange(stream.as_ref(), &mut reader, our_pubkey).await { + Ok(peer_pubkey) => { + debug!(addr = %ta, "BLE inbound pubkey exchange complete"); + let peer_node = NodeAddr::from_pubkey(&peer_pubkey); + peer_node_addr = Some(peer_node); + let announced = announced_addr(&pool, &peer_node, &addr).await; + neighbor_buffer.add_peer_with_pubkey(&announced, peer_pubkey); + + // Already linked to this peer on another address? + // A peer using resolvable private addresses rotates + // continually, and every rotation dials in looking + // like a new device. Admitting those would put one + // peer in several pool slots and evict real ones. + // The incumbent link is kept: it is known-good, and + // a genuinely dead one is already reaped by the + // send-error and receive-loop paths. + let dup = { + let pool_guard = pool.lock().await; + pool_guard.find_by_node(&peer_node) + }; + if let Some(existing) = dup + && existing != ta + { + debug!( + addr = %ta, + role = "peripheral", + outcome = "duplicate-node-decline", + existing = %existing, + "BLE inbound: peer already connected on another address, dropping duplicate" + ); + stats.record_duplicate_node_decline(); + return; + } + + // Cross-probe tie-breaker: smaller NodeAddr's + // outbound wins. If we're smaller, our outbound + // should win — drop this inbound. + if let Some(ref our_addr) = local_node_addr + && our_addr < &peer_node + { + stats.record_tiebreaker_drop(); + debug!( + addr = %ta, + role = "peripheral", + outcome = "tiebreaker-drop", + "BLE inbound tie-breaker: dropping (our addr < peer, outbound wins)" + ); + return; + } + } + Err(e) => { + stats.record_pubkey_exchange_failure(); + debug!( + addr = %ta, role = "peripheral", + outcome = "pubkey-exchange-failed", error = %e, + "BLE inbound pubkey exchange failed" + ); + return; + } + } + } + + // Spawn receive loop + let recv_task = tokio::spawn(receive_loop( + reader, + ta.clone(), + Arc::clone(&pool), + packet_tx.clone(), + transport_id, + Arc::clone(&stats), + recv_mtu, + )); + + let conn = BleConnection { + stream, + recv_task: Some(recv_task), + send_mtu, + recv_mtu, + established_at: tokio::time::Instant::now(), + is_static: false, + addr, + node_addr: peer_node_addr, + }; + + let mut pool_guard = pool.lock().await; + match pool_guard.insert(ta.clone(), conn) { + Ok(Some(evicted)) => { + stats.record_pool_eviction(); + info!(addr = %ta, evicted = %evicted, "BLE inbound accepted (evicted peer)"); + } + Ok(None) => { + info!(addr = %ta, send_mtu, recv_mtu, "BLE inbound connection accepted"); + } + Err(e) => { + stats.record_connection_rejected(); + warn!( + addr = %ta, role = "peripheral", outcome = "pool-rejected", + error = %e, "BLE pool full, inbound connection rejected" + ); + return; + } + } + stats.record_connection_accepted(); +} + /// Receive loop: reads packets from a BLE stream and delivers to node. /// /// Takes the connection's `BleStreamRead` — already positioned past the @@ -2650,10 +2737,183 @@ mod tests { "advertisements_sent", "scan_results", "duplicate_node_declines", + "handshakes_aborted", ]; for key in expected { assert!(object.contains_key(key), "snapshot lost `{key}`"); } assert_eq!(object.len(), expected.len(), "snapshot gained a key"); } + + /// A secret/public key pair from a fixed seed. + /// + /// The exchange parses the peer's 32 bytes with `XOnlyPublicKey::from_slice`, + /// so arbitrary bytes will not do. + fn test_keypair(seed: u8) -> ([u8; 32], XOnlyPublicKey) { + let secp = secp256k1::Secp256k1::new(); + let sk = secp256k1::SecretKey::from_slice(&[seed; 32]).unwrap(); + let (xonly, _) = sk.public_key(&secp).x_only_public_key(); + (xonly.serialize(), xonly) + } + + /// Inject an inbound connection and return the peer end of the link. + /// + /// The peer end must be kept alive: dropping it closes the channel, which + /// the mock reports as a zero-length read rather than as silence. + async fn connect_inbound( + transport: &BleTransport, + peer: &BleAddr, + ) -> io::MockBleStream { + let (inbound, peer_end) = io::MockBleStream::pair(test_addr(1), peer.clone(), 512); + transport.io.inject_inbound(inbound).await; + peer_end + } + + /// Complete the peer half of the pubkey exchange. + async fn send_pubkey(stream: &io::MockBleStream, pubkey: &XOnlyPublicKey) { + let mut msg = [0u8; PUBKEY_EXCHANGE_SIZE]; + msg[0] = PUBKEY_EXCHANGE_PREFIX; + msg[1..].copy_from_slice(&pubkey.serialize()); + stream.send(&msg).await.unwrap(); + } + + /// Poll the discovery buffer until every wanted address has appeared. + /// + /// Asserts on the discovery buffer rather than the pool because the buffer + /// is populated before the cross-probe tie-breaker, which drops an inbound + /// whose NodeAddr sorts above ours and would make the result depend on the + /// keys the test happened to pick. Sleeps rather than yields, so a paused + /// clock advances instead of the runtime staying busy forever. + async fn wait_for_discovered(buffer: &NeighborBuffer, wanted: &[BleAddr]) -> bool { + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); + for _ in 0..20_000 { + for peer in buffer.take() { + if let Some(addr) = peer.addr.as_str() { + seen.insert(addr.to_string()); + } + } + if wanted.iter().all(|a| seen.contains(&a.to_string_repr())) { + return true; + } + tokio::time::sleep(std::time::Duration::from_millis(1)).await; + } + false + } + + #[tokio::test(start_paused = true)] + async fn a_silent_inbound_peer_does_not_delay_the_next_accept() { + let io = MockBleIo::new("hci0", test_addr(1)); + let (mut transport, _rx) = make_transport(io); + let (our_pubkey, _) = test_keypair(1); + transport.set_local_pubkey(our_pubkey); + transport.start_async().await.unwrap(); + + // A connects and never says anything. + let _silent = connect_inbound(&transport, &test_addr(2)).await; + + // B connects behind it and completes the exchange at once. + let good = connect_inbound(&transport, &test_addr(3)).await; + let (_, peer_pubkey) = test_keypair(7); + send_pubkey(&good, &peer_pubkey).await; + + let start = tokio::time::Instant::now(); + assert!( + wait_for_discovered(&transport.neighbor_buffer, &[test_addr(3)]).await, + "the well-behaved peer was never admitted" + ); + let elapsed = start.elapsed(); + assert!( + elapsed < std::time::Duration::from_secs(1), + "the well-behaved peer waited {:?} on the silent one's handshake deadline", + elapsed + ); + } + + #[tokio::test(start_paused = true)] + async fn a_flood_of_silent_connectors_does_not_delay_a_well_behaved_one() { + // The test that breaks what the guard guards: it fails against a bound + // that waits for a slot rather than reclaiming one. + let io = MockBleIo::new("hci0", test_addr(1)); + let (mut transport, _rx) = make_transport(io); + let (our_pubkey, _) = test_keypair(1); + transport.set_local_pubkey(our_pubkey); + transport.start_async().await.unwrap(); + + let mut silent = Vec::new(); + for n in 0..INBOUND_HANDSHAKE_INFLIGHT + 1 { + silent.push(connect_inbound(&transport, &test_addr(20 + n as u8)).await); + } + + let good = connect_inbound(&transport, &test_addr(3)).await; + let (_, peer_pubkey) = test_keypair(7); + send_pubkey(&good, &peer_pubkey).await; + + let start = tokio::time::Instant::now(); + assert!( + wait_for_discovered(&transport.neighbor_buffer, &[test_addr(3)]).await, + "the well-behaved peer was never admitted behind the flood" + ); + let elapsed = start.elapsed(); + assert!( + elapsed < std::time::Duration::from_secs(1), + "the well-behaved peer waited {:?} behind {} silent connectors", + elapsed, + silent.len() + ); + assert!(transport.stats.snapshot().handshakes_aborted > 0); + } + + #[tokio::test(start_paused = true)] + async fn inbound_peers_within_the_handshake_budget_are_all_admitted() { + // The guard must not red a legitimately clean run. + let io = MockBleIo::new("hci0", test_addr(1)); + let (mut transport, _rx) = make_transport(io); + let (our_pubkey, _) = test_keypair(1); + transport.set_local_pubkey(our_pubkey); + transport.start_async().await.unwrap(); + + let mut peers = Vec::new(); + let mut wanted = Vec::new(); + for n in 0..INBOUND_HANDSHAKE_INFLIGHT { + let addr = test_addr(40 + n as u8); + let stream = connect_inbound(&transport, &addr).await; + let (_, peer_pubkey) = test_keypair(10 + n as u8); + send_pubkey(&stream, &peer_pubkey).await; + peers.push(stream); + wanted.push(addr); + } + + assert!( + wait_for_discovered(&transport.neighbor_buffer, &wanted).await, + "a well-behaved peer inside the budget was not admitted" + ); + assert_eq!(transport.stats.snapshot().handshakes_aborted, 0); + } + + #[tokio::test(start_paused = true)] + async fn the_pubkey_exchange_send_half_has_a_deadline() { + // The mock's link is a 64-slot channel, so a peer that never reads + // parks our write once it is full — which is what an L2CAP peer that + // stops draining does. + let (ours, _peer) = io::MockBleStream::pair(test_addr(1), test_addr(2), 512); + for _ in 0..64 { + ours.send(&[0u8; 1]).await.unwrap(); + } + let (our_pubkey, _) = test_keypair(1); + // `fix/platform-ble` split the read half out; the exchange takes it as + // its own argument now. The deadline under test is on the send half, + // which never reaches a read, so the reader is only here to type-check. + let recv_mtu = ours.recv_mtu(); + let ours = Arc::new(ours); + let mut reader = BleStreamRead::new(Arc::clone(&ours), recv_mtu); + + let result = tokio::time::timeout( + std::time::Duration::from_secs(60), + pubkey_exchange(ours.as_ref(), &mut reader, &our_pubkey), + ) + .await + .expect("the pubkey exchange send half parked with no deadline of its own"); + + assert!(matches!(result, Err(TransportError::Timeout))); + } } diff --git a/src/transport/ble/stats.rs b/src/transport/ble/stats.rs index d84cc3ef..bf3cf4ee 100644 --- a/src/transport/ble/stats.rs +++ b/src/transport/ble/stats.rs @@ -27,6 +27,7 @@ pub struct BleStats { pub connections_established: AtomicU64, pub connections_accepted: AtomicU64, pub connections_rejected: AtomicU64, + pub handshakes_aborted: AtomicU64, pub connect_timeouts: AtomicU64, /// Outbound connects that failed with an error rather than timing out. pub connect_errors: AtomicU64, @@ -58,6 +59,7 @@ impl BleStats { connections_established: AtomicU64::new(0), connections_accepted: AtomicU64::new(0), connections_rejected: AtomicU64::new(0), + handshakes_aborted: AtomicU64::new(0), connect_timeouts: AtomicU64::new(0), connect_errors: AtomicU64::new(0), pubkey_exchange_failures: AtomicU64::new(0), @@ -176,6 +178,11 @@ impl BleStats { self.duplicate_node_declines.fetch_add(1, Ordering::Relaxed); } + /// Record an inbound handshake aborted to free an in-flight slot. + pub fn record_handshake_aborted(&self) { + self.handshakes_aborted.fetch_add(1, Ordering::Relaxed); + } + /// Take a snapshot of all counters. pub fn snapshot(&self) -> BleStatsSnapshot { BleStatsSnapshot { @@ -189,6 +196,7 @@ impl BleStats { connections_established: self.connections_established.load(Ordering::Relaxed), connections_accepted: self.connections_accepted.load(Ordering::Relaxed), connections_rejected: self.connections_rejected.load(Ordering::Relaxed), + handshakes_aborted: self.handshakes_aborted.load(Ordering::Relaxed), connect_timeouts: self.connect_timeouts.load(Ordering::Relaxed), connect_errors: self.connect_errors.load(Ordering::Relaxed), pubkey_exchange_failures: self.pubkey_exchange_failures.load(Ordering::Relaxed), @@ -221,6 +229,7 @@ pub struct BleStatsSnapshot { pub connections_established: u64, pub connections_accepted: u64, pub connections_rejected: u64, + pub handshakes_aborted: u64, pub connect_timeouts: u64, pub connect_errors: u64, pub pubkey_exchange_failures: u64, From 6dd4bc43aab9c6f80c92be40bc381a51a946f03d Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 26 Aug 2026 08:08:16 +0100 Subject: [PATCH 8/9] fix(transport/ble): stop counting a pool-refused probe as an established link The scan/probe loop recorded the refusal and then carried on as though the connection had been made: it incremented connections_established, resolved the address out of the retry book, and handed the peer to the node layer through the neighbour buffer. The connection object itself was already dropped by the failed insert, so the node layer was told about a link that does not exist, and the address was removed from the only structure that would have re-offered it once a slot freed. The inbound path has always returned at this point rather than falling through; this gives the outbound probe the same shape. The address stays in the retry book deliberately, because a slot may free before the peer is advertised again. Reaching the refusal at all takes `max_connections: 0`. ConnectionPool::insert fails only when the pool is full and every slot is static, and every BLE connection is built with is_static: false, so a non-empty pool always has an evictable slot. That makes this unreachable in a default deployment today and reachable the moment anything marks a connection static, which the pool is already written for. The test pins it with max_connections: 0 and was checked by reverting the fix: it fails on the established-count assertion and passes with it. The defect is on maint and master alike, at the same site. It is fixed here rather than on maint because maint is frozen. --- src/transport/ble/mod.rs | 83 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/transport/ble/mod.rs b/src/transport/ble/mod.rs index a0146394..764a6192 100644 --- a/src/transport/ble/mod.rs +++ b/src/transport/ble/mod.rs @@ -1585,6 +1585,12 @@ async fn scan_probe_loop( addr = %ta, role = "central", outcome = "pool-rejected", error = %e, "BLE pool full, probe connection dropped" ); + // The connection is dropped with `conn`, so there is + // nothing to report and nothing to resolve. Leaving the + // address in the retry book is the point: a slot may + // free before the peer is advertised again. The inbound + // path already returns here rather than falling through. + continue; } } drop(pool_guard); @@ -2379,6 +2385,83 @@ mod tests { transport.stop_async().await.unwrap(); } + /// A probe the pool refuses is not a connection, and must not be recorded + /// as one. The inbound path already returns on rejection + /// (`admit_inbound`); this pins the outbound probe path to the same shape. + /// + /// Reaching the refusal needs `max_connections: 0`. `ConnectionPool::insert` + /// only fails when the pool is full *and* every slot is static, and every + /// BLE connection is built with `is_static: false`, so a non-empty pool + /// always has an evictable slot. That makes this arm unreachable in a + /// default deployment today and reachable the moment anything marks a + /// connection static, which the pool is already written for. + #[tokio::test(start_paused = true)] + async fn a_pool_rejected_probe_is_neither_established_nor_reported() { + use std::sync::Mutex as StdMutex; + + let (ours_pk, theirs_pk) = pubkeys_ordered_by_node_addr(); + let io = MockBleIo::new("hci0", test_addr(1)); + + let connects: Arc>> = Arc::new(StdMutex::new(Vec::new())); + let (peer_tx, mut peer_rx) = tokio::sync::mpsc::unbounded_channel(); + { + let connects = Arc::clone(&connects); + io.set_connect_handler(move |addr, _psm| { + let (mine, theirs) = MockBleStream::pair(test_addr(1), addr.clone(), 2048); + connects.lock().unwrap().push(addr.clone()); + peer_tx + .send(theirs) + .map_err(|_| TransportError::ConnectionRefused)?; + Ok(mine) + }); + } + tokio::spawn(async move { + let mut alive = Vec::new(); + while let Some(theirs) = peer_rx.recv().await { + peer_side_exchange(&theirs, &theirs_pk).await; + alive.push(theirs); + } + }); + + let config = BleConfig { + scan: Some(true), + accept_connections: Some(false), + max_connections: Some(0), + ..identity_test_config() + }; + let (tx, _rx) = tokio::sync::mpsc::channel(64); + let mut transport = BleTransport::new(TransportId::new(1), None, config, io, tx); + transport.set_local_pubkey(ours_pk); + transport.start_async().await.unwrap(); + + transport.io.inject_scan_result(test_addr(2)).await; + settle().await; + + // The dial and the exchange both happened; only the pool refused. + assert_eq!(connects.lock().unwrap().len(), 1, "the peer was dialled"); + let snap = transport.stats.snapshot(); + assert_eq!(snap.connections_rejected, 1, "the refusal is recorded"); + assert_eq!( + snap.connections_established, 0, + "a refused probe is not an established connection" + ); + assert_eq!(transport.pool.lock().await.len(), 0); + assert!( + transport.neighbor_buffer.take().is_empty(), + "the node layer must not be handed a peer with no connection behind it" + ); + + // It stayed in the retry book, so a freed slot can still admit it. + tokio::time::advance(std::time::Duration::from_secs(5)).await; + settle().await; + assert!( + connects.lock().unwrap().len() >= 2, + "a refused address is retried, not resolved away" + ); + + transport.stop_async().await.unwrap(); + } + // ------------------------------------------------------------------ // Per-peer listener PSM // ------------------------------------------------------------------ From 8d3b0e0ab2ea2a6419d887f8f1358f768e76c40a Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 26 Aug 2026 08:12:54 +0100 Subject: [PATCH 9/9] fix(transport/ble): give the seam a stop_scanning, so a stop reaches the radio `BleIo` had `stop_advertising` and no counterpart for scanning, so `stop_async` stopped the advertisement and left the scan running. On BlueZ that is harmless, because discovery ends when the scanner's event stream is dropped. On a backend whose radio the embedder owns it is not: the transport aborted its own scan task, nothing told the radio, and a scan started by a transport that has since stopped ran for the life of the process. On a phone that is battery and a continued broadcast of the user's presence, both after the feature was switched off. `AndroidRadio::stop_scanning` already existed and was called from nowhere. Add `stop_scanning` to `BleIo` beside `stop_advertising` and call it from `stop_async`. `BluerIo` implements it as a no-op that says why. `AndroidIo` clears the scan intent and tells the radio through a new `AndroidBleBridge::end_scanning`, mirroring `end_advertising`. Clearing the intent matters as much as stopping the radio: without it a radio installed after the transport stopped would be told to scan by `RadioIntent::apply`, with nothing left to consume the adverts. Putting it in the seam rather than in a `Drop` on the Android scanner is what makes it available to the next backend. macOS has the same shape as Android here -- CoreBluetooth owns the radio and has its own `stopScan` -- so this is one more thing `io_macos.rs` inherits rather than rediscovers. Both tests were checked by breaking what they guard: with the `stop_async` call removed, and separately with `end_scanning`'s radio call removed, each fails on the assertion that names the behaviour. --- src/transport/ble/io.rs | 24 ++++++++++++ src/transport/ble/io_android.rs | 67 ++++++++++++++++++++++++++++++++- src/transport/ble/io_linux.rs | 8 ++++ src/transport/ble/mod.rs | 33 ++++++++++++++++ 4 files changed, 131 insertions(+), 1 deletion(-) diff --git a/src/transport/ble/io.rs b/src/transport/ble/io.rs index c7d60408..c215946f 100644 --- a/src/transport/ble/io.rs +++ b/src/transport/ble/io.rs @@ -163,6 +163,16 @@ pub trait BleIo: Send + Sync + 'static { &self, ) -> impl std::future::Future> + Send; + /// Stop scanning. + /// + /// The counterpart to [`Self::stop_advertising`], and needed for the same + /// reason: dropping the transport's scan task stops *us* reading adverts, + /// but on a backend whose radio is owned elsewhere it does not stop the + /// radio. A backend whose scan ends when its `Scanner` is dropped + /// implements this as a no-op and says so. + fn stop_scanning(&self) + -> impl std::future::Future> + Send; + /// Get the adapter's BLE address. fn local_addr(&self) -> Result; @@ -288,6 +298,8 @@ pub struct MockBleIo { bound_psm: std::sync::Mutex>, /// PSM most recently passed to `start_advertising`. advertised_psm: std::sync::Mutex>, + /// Number of times `stop_scanning` has been called. + stop_scans: std::sync::atomic::AtomicUsize, } impl MockBleIo { @@ -305,9 +317,15 @@ impl MockBleIo { connect_handler: std::sync::Mutex::new(None), bound_psm: std::sync::Mutex::new(None), advertised_psm: std::sync::Mutex::new(None), + stop_scans: std::sync::atomic::AtomicUsize::new(0), } } + /// How many times the transport has asked this backend to stop scanning. + pub fn stop_scan_calls(&self) -> usize { + self.stop_scans.load(std::sync::atomic::Ordering::Relaxed) + } + /// Inject an inbound connection (simulates a remote device connecting). pub async fn inject_inbound(&self, stream: MockBleStream) { let _ = self.accept_tx.send(stream).await; @@ -393,6 +411,12 @@ impl BleIo for MockBleIo { Ok(()) } + async fn stop_scanning(&self) -> Result<(), TransportError> { + self.stop_scans + .fetch_add(1, std::sync::atomic::Ordering::Relaxed); + Ok(()) + } + async fn start_scanning(&self) -> Result { let rx = self .scan_rx diff --git a/src/transport/ble/io_android.rs b/src/transport/ble/io_android.rs index 78661575..566bbbba 100644 --- a/src/transport/ble/io_android.rs +++ b/src/transport/ble/io_android.rs @@ -421,6 +421,18 @@ impl AndroidBleBridge { } } + /// Stop scanning, so a later request starts it again. + /// + /// The embedder's radio outlives this transport, so a scan nobody stops + /// runs for the life of the process: on a phone that is battery and a + /// broadcast of the user's presence, both after the feature was switched + /// off. + fn end_scanning(&self) { + if self.scanning.swap(false, Ordering::AcqRel) { + self.radio.stop_scanning(); + } + } + /// Allocate a channel, registering the bridge-side half and returning the /// stream-side half. fn make_channel(&self, remote: BleAddr, send_mtu: u16, recv_mtu: u16) -> StreamEndpoints { @@ -986,6 +998,17 @@ impl BleIo for AndroidIo { Ok(()) } + /// Clearing the intent matters as much as stopping the radio: without it a + /// radio installed after the transport stopped would be told to scan by + /// `RadioIntent::apply`, with nothing left to consume the adverts. + async fn stop_scanning(&self) -> Result<(), TransportError> { + self.intent.scan.store(false, Ordering::Relaxed); + if let Some(bridge) = self.slot.current() { + bridge.end_scanning(); + } + Ok(()) + } + async fn start_scanning(&self) -> Result { self.intent.scan.store(true, Ordering::Relaxed); let mut seen = None; @@ -1040,6 +1063,7 @@ mod tests { advertised_psm: AtomicU16, advertise_calls: AtomicU32, scan_calls: AtomicU32, + stop_scan_calls: AtomicU32, closed_channels: Mutex>, dials: Mutex>, } @@ -1075,7 +1099,9 @@ mod tests { fn start_scanning(&self) { self.scan_calls.fetch_add(1, Ordering::Relaxed); } - fn stop_scanning(&self) {} + fn stop_scanning(&self) { + self.stop_scan_calls.fetch_add(1, Ordering::Relaxed); + } fn close_channel(&self, ch_id: i64) { self.closed_channels.lock().unwrap().push(ch_id); } @@ -1707,4 +1733,43 @@ mod tests { reader.read_exact(&mut tail).await.unwrap(); assert_eq!(&tail, b"456789"); } + + /// The embedder's radio outlives the transport, so stopping the transport + /// has to reach the radio. Nothing did: `stop_scanning` was declared on + /// `AndroidRadio` and called from nowhere, so a scan started by a + /// transport that has since stopped ran for the life of the process. + #[tokio::test] + async fn stopping_the_transport_stops_the_radio_scanning() { + let radio = MockRadio::with_psm(0x0081); + let slot = Arc::new(BleRadioSlot::new()); + slot.install(AndroidBleBridge::new( + Arc::clone(&radio) as Arc + )); + let io = AndroidIo::new(Arc::clone(&slot)); + + let _scanner = io.start_scanning().await.unwrap(); + assert_eq!(radio.scan_calls.load(Ordering::Relaxed), 1); + assert_eq!(radio.stop_scan_calls.load(Ordering::Relaxed), 0); + + io.stop_scanning().await.unwrap(); + assert_eq!( + radio.stop_scan_calls.load(Ordering::Relaxed), + 1, + "the radio must be told, not just the scanner dropped" + ); + + // The intent is cleared too, so a radio installed after the stop is + // not told to scan with nothing left to read the adverts. + let later = MockRadio::with_psm(0x0081); + slot.install(AndroidBleBridge::new( + Arc::clone(&later) as Arc + )); + let mut seen = None; + resolve(&slot, &mut seen, &io.intent); + assert_eq!( + later.scan_calls.load(Ordering::Relaxed), + 0, + "a radio installed after the stop must not be told to scan" + ); + } } diff --git a/src/transport/ble/io_linux.rs b/src/transport/ble/io_linux.rs index 60d7605e..9c9caeb5 100644 --- a/src/transport/ble/io_linux.rs +++ b/src/transport/ble/io_linux.rs @@ -368,6 +368,14 @@ impl BleIo for BluerIo { Ok(()) } + /// A no-op: BlueZ discovery ends when [`BluerScanner`]'s event stream is + /// dropped, which happens when the transport drops the scanner. There is + /// no separate adapter-level stop to issue. + async fn stop_scanning(&self) -> Result<(), TransportError> { + debug!("BLE scanning stops with the scanner"); + Ok(()) + } + async fn start_scanning(&self) -> Result { // Clear cached devices so BlueZ fires DeviceAdded for every // advertisement. Without this, already-known devices only diff --git a/src/transport/ble/mod.rs b/src/transport/ble/mod.rs index 764a6192..ee81c34d 100644 --- a/src/transport/ble/mod.rs +++ b/src/transport/ble/mod.rs @@ -326,6 +326,11 @@ impl BleTransport { // Stop advertising let _ = self.io.stop_advertising().await; + // Stop scanning. Aborting the scan task below stops us reading + // adverts; on a backend whose radio the embedder owns, only this + // stops the radio. + let _ = self.io.stop_scanning().await; + // Abort accept loop if let Some(task) = self.accept_task.take() { task.abort(); @@ -1938,6 +1943,34 @@ mod tests { assert_eq!(transport.state(), TransportState::Down); } + /// `stop_async` has always stopped advertising; it must stop scanning + /// too. Aborting the scan task only stops the transport reading adverts — + /// on a backend whose radio the embedder owns, the radio keeps scanning + /// until it is told, which on a phone costs battery and keeps + /// broadcasting after the feature was switched off. + #[tokio::test] + async fn stop_async_tells_the_backend_to_stop_scanning() { + let io = MockBleIo::new("hci0", test_addr(1)); + let config = BleConfig { + adapter: Some("hci0".to_string()), + scan: Some(true), + advertise: Some(false), + accept_connections: Some(false), + ..Default::default() + }; + let (tx, _rx) = tokio::sync::mpsc::channel(64); + let mut transport = BleTransport::new(TransportId::new(1), None, config, io, tx); + transport.start_async().await.unwrap(); + assert_eq!(transport.io.stop_scan_calls(), 0); + + transport.stop_async().await.unwrap(); + assert_eq!( + transport.io.stop_scan_calls(), + 1, + "stopping the transport must reach the backend's scan" + ); + } + #[tokio::test(start_paused = true)] async fn test_scan_discovers_peers() { let io = MockBleIo::new("hci0", test_addr(1));