diff --git a/bitchat/Services/BLE/BLEAnnounceHandler.swift b/bitchat/Services/BLE/BLEAnnounceHandler.swift index d5b73464..39eca869 100644 --- a/bitchat/Services/BLE/BLEAnnounceHandler.swift +++ b/bitchat/Services/BLE/BLEAnnounceHandler.swift @@ -145,6 +145,16 @@ final class BLEAnnounceHandler { // owns is either a rotation heal or a replay with its TTL restored; // both are ambiguous, so only the rebind (which containment-checks // the claimed identity) may promote it — never this shortcut. + // + // Known limitation: this reads the binding BEFORE the rebind runs + // (BLEService rebinds after this handler returns), so once a first + // replay has stolen the link for an absent peer, a SECOND replay + // finds the link bound to the claimed peer — the live-link terms + // below then mark the absent peer connected. Presence display only: + // DMs stay gated on canDeliverSecurely (no Noise session means + // retain + courier, see MessageRouter.sendPrivate). Not closed here + // because the same post-rebind state is exactly what legitimate + // rotation/reconnect heals look like. let linkBoundToOtherPeer = isDirectAnnounce && env.linkBoundToOtherPeer(packet, peerID) env.withRegistryBarrier { diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index d75b1ced..8d7b36d4 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/bitchat/Services/BLE/BLEService.swift @@ -4440,6 +4440,12 @@ extension BLEService { self?.linkState(for: peerID) ?? (hasPeripheral: false, hasCentral: false) }, linkBoundToOtherPeer: { [weak self] packet, peerID in + // Reads the CURRENT binding — i.e. the state before + // rebindLinkAfterVerifiedDirectAnnounce (which runs after the + // handler) may steal the link. After a first replayed announce + // has rebound a link to an absent peer's ID, a second replay + // answers false here; see the caller in BLEAnnounceHandler + // for why that residual presence gap is accepted. guard let self else { return false } guard let link = (self.collectionsQueue.sync { self.ingressLinks.link(for: packet) }) else { return false } let boundPeerID: PeerID? = self.readLinkState { store in diff --git a/bitchat/Services/MessageRouter.swift b/bitchat/Services/MessageRouter.swift index ede20949..321875fb 100644 --- a/bitchat/Services/MessageRouter.swift +++ b/bitchat/Services/MessageRouter.swift @@ -179,6 +179,13 @@ final class MessageRouter { // (a genuine link finishes the handshake and delivers), but retain // a copy and hand a sealed copy to couriers so nothing is silently // lost; receivers dedup resends by message ID. + // + // Deliberate metadata tradeoff: every pre-handshake first DM to a + // connected peer hands nearby verified peers a sealed copy, so + // they learn a DM to this recipient exists (never its content — + // the envelope is opaque). Accepted for delivery robustness; the + // deposit is cleared on ack. Don't "optimize" the courier call + // away. SecureLogger.debug("Routing PM via \(type(of: transport)) (connected, no secure session) to \(peerID.id.prefix(8))… id=\(messageID.prefix(8))…", category: .session) transport.sendPrivateMessage(content, to: peerID, recipientNickname: recipientNickname, messageID: messageID) enqueue(message, for: peerID) @@ -408,11 +415,26 @@ final class MessageRouter { SecureLogger.debug("Outbox -> \(type(of: transport)) (connected) for \(peerID.id.prefix(8))… id=\(message.messageID.prefix(8))…", category: .session) transport.sendPrivateMessage(message.content, to: peerID, recipientNickname: message.nickname, messageID: message.messageID) metrics?.record(.outboxResent) - } else if let transport = connectedTransport(for: peerID) ?? reachableTransport(for: peerID) { - // Weak signal (reachability heuristic, or a "connected" link - // without a secure session — possibly a stolen binding from a - // replayed announce): send but keep retaining until an ack - // clears it, bounded by attempt count for peers that never ack. + } else if let transport = connectedTransport(for: peerID) { + // "Connected" without a secure session — possibly a stolen + // binding from a replayed announce: send (a genuine link + // finishes the handshake and delivers) but keep retaining + // until an ack clears it. These flushes do NOT count toward + // the attempt-cap drop: the message was transmitted over a + // live link, so a peer whose handshake stalls across + // reconnect flapping must not burn through the cap and lose + // the store-and-forward copy this retention exists to + // preserve. Retention stays bounded by the 24h outbox TTL + // and the per-peer FIFO cap. + SecureLogger.debug("Outbox -> \(type(of: transport)) (connected, no secure session) for \(peerID.id.prefix(8))… id=\(message.messageID.prefix(8))…", category: .session) + transport.sendPrivateMessage(message.content, to: peerID, recipientNickname: message.nickname, messageID: message.messageID) + metrics?.record(.outboxResent) + remaining.append(message) + } else if let transport = reachableTransport(for: peerID) { + // Reachability without a connection is a freshness heuristic, + // so the send can silently go nowhere: send but keep retaining + // until an ack clears it, bounded by attempt count for peers + // that never ack. guard message.sendAttempts < Self.maxSendAttempts else { SecureLogger.warning("📤 Dropping unacked PM for \(peerID.id.prefix(8))… id=\(message.messageID.prefix(8))… after \(message.sendAttempts) attempts", category: .session) dropMessage(message.messageID, for: peerID) diff --git a/bitchatTests/Services/BLEAnnounceHandlerTests.swift b/bitchatTests/Services/BLEAnnounceHandlerTests.swift index 54046cf6..6ad87598 100644 --- a/bitchatTests/Services/BLEAnnounceHandlerTests.swift +++ b/bitchatTests/Services/BLEAnnounceHandlerTests.swift @@ -390,6 +390,51 @@ struct BLEAnnounceHandlerTests { #expect(recorder.upsertCalls.first?.isConnected == true) } + /// Documents a known, accepted limitation: the linkBoundToOtherPeer read + /// happens before the rebind steals the link, so while the FIRST replayed + /// "direct" announce is denied the connected shortcut, the SECOND finds + /// the link already rebound to the (absent) victim and the live-link + /// terms mark it connected. This is presence display only — DMs remain + /// gated on canDeliverSecurely, so without a Noise session they take the + /// retain + courier path (see MessageRouterTests + /// .sendPrivate_connectedWithoutSecureSessionRetainsAndDepositsWithCourier). + /// Not closed at the announce layer because the post-rebind state is + /// indistinguishable from a legitimate rotation/reconnect heal. + @Test + func secondReplayedDirectAnnounceAfterRebindMarksAbsentPeerConnected() throws { + let now = Date(timeIntervalSince1970: 1_000) + let noiseKey = Data(repeating: 0xA1, count: 32) + let victim = PeerID(publicKey: noiseKey) + let packet = try makeAnnouncePacket( + noisePublicKey: noiseKey, + peerID: victim, + timestamp: timestamp(now), + signature: Data(repeating: 0xEE, count: 64) + ) + + let recorder = Recorder() + let handler = makeHandler(recorder: recorder, now: now) + + // First replay: the link still belongs to the replayer and the victim + // has no live link of its own — the connected shortcut is denied. + recorder.linkBoundToOtherPeer = true + recorder.linkState = (hasPeripheral: false, hasCentral: false) + handler.handle(packet, from: victim) + #expect(recorder.upsertCalls.count == 1) + #expect(recorder.upsertCalls.first?.isConnected == false) + + // The rebind then binds the replayer's link to the victim's ID (the + // rotation-heal path, containment-checked in BLEService). A second + // replay now finds the link owned by the claimed peer … + recorder.linkBoundToOtherPeer = false + recorder.linkState = (hasPeripheral: false, hasCentral: true) + handler.handle(packet, from: victim) + + // … and the absent victim reads as connected: the residual gap. + #expect(recorder.upsertCalls.count == 2) + #expect(recorder.upsertCalls.last?.isConnected == true) + } + @Test func announceBackIsSkippedWhenAlreadyMarked() throws { let now = Date(timeIntervalSince1970: 1_000) diff --git a/bitchatTests/Services/MessageRouterTests.swift b/bitchatTests/Services/MessageRouterTests.swift index b9a3df90..ea8ba4da 100644 --- a/bitchatTests/Services/MessageRouterTests.swift +++ b/bitchatTests/Services/MessageRouterTests.swift @@ -360,6 +360,38 @@ struct MessageRouterTests { #expect(transport.sentPrivateMessages.count == 3) } + /// Flushes over a connected-but-insecure link never count toward the + /// attempt-cap drop: the message was actually transmitted over a live + /// link, so a peer whose Noise handshake stalls across reconnect flapping + /// must not burn through the cap and lose the store-and-forward copy the + /// secure-session gate exists to preserve. Retention stays bounded by + /// the 24h outbox TTL and the per-peer FIFO cap; an ack clears it. + @Test @MainActor + func flushOutbox_connectedInsecureFlushesNeverDropTheRetainedCopy() async { + let peerID = PeerID(str: "00000000000000ac") + let transport = MockTransport() + transport.connectedPeers.insert(peerID) + transport.securePeers = [] // handshake never completes + + let router = MessageRouter(transports: [transport]) + var dropped: [String] = [] + router.onMessageDropped = { messageID, _ in dropped.append(messageID) } + + router.sendPrivate("Hello", to: peerID, recipientNickname: "Peer", messageID: "ci1") + + // Well past maxSendAttempts (8): every flush resends, none drops. + for _ in 0..<10 { + router.flushOutbox(for: peerID) + } + #expect(dropped.isEmpty) + #expect(transport.sentPrivateMessages.count == 11) + + // The copy is still retained and an ack still clears it. + router.markDelivered("ci1") + router.flushOutbox(for: peerID) + #expect(transport.sentPrivateMessages.count == 11) + } + /// With an established secure session the connected fast-path stays /// exactly as before: trusted outright, no retained copy, no courier. @Test @MainActor