Defer timeout-restore queue drains until Noise convergence completes

A responder timeout restores the quarantined old session generation and
requests one mandatory convergence retry, but both were dispatched
unordered onto the transport queue. When the restore handler won the
race it re-entered handleNoisePeerAuthenticated and drained the pending
private-message and typed-payload queues under the restored keys — keys
a legitimately restarted counterpart may have already discarded after
completing the replacement handshake whose message 3 was lost. Every
drained DM, read receipt, and invite then failed authentication
remotely and disappeared silently.

Restores now carry a reason. Terminal restores (claimed-identity
mismatch, or failures owning no convergence retry) drain immediately as
before: the counterpart never finished replacement keys, so the
restored generation is still valid on both ends. Pending-convergence
restores rebuild the generation-bound protocol state for receive
purposes but leave both outbound queues parked; the convergence retry
(or any later reconnect the revalidation policy initiates) re-enters
the authenticated transition with a fresh generation and drains them
under keys both sides hold. If the retry is cancelled the queues keep
waiting for the next establishment — the same contract every other
queued-before-handshake message already relies on — rather than
gambling them on keys only this side can prove.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

bitchat/Noise/NoiseSessionManager.swift

@@ -31,6 +31,20 @@ enum NoiseHandshakeRecoveryPreparation {
     case transferred
 }
 
+/// Why a quarantined transport became the active session again.
+enum NoiseSessionRestoreReason: Equatable, Sendable {
+    /// The replacement attempt failed terminally (claimed-identity mismatch,
+    /// or a failure that owns no convergence retry). The counterpart never
+    /// finished replacement keys, so the restored generation is immediately
+    /// valid for outbound traffic.
+    case terminal
+    /// The responder window expired — or a recoverable failure occurred —
+    /// and this manager owns one mandatory convergence retry. The counterpart
+    /// may already hold replacement keys that discarded the restored ones, so
+    /// outbound queue drains must wait for the retry to conclude.
+    case pendingConvergence
+}
+
 final class NoiseSessionManager {
     private var sessions: [PeerID: NoiseSession] = [:]
     /// Opaque identity for each exact entry in `sessions`. The generation is
@@ -75,7 +89,7 @@ final class NoiseSessionManager {
     
     // Callbacks
     var onSessionEstablished: ((PeerID, Curve25519.KeyAgreement.PublicKey, UUID) -> Void)?
-    var onSessionRestored: ((PeerID, UUID) -> Void)?
+    var onSessionRestored: ((PeerID, UUID, NoiseSessionRestoreReason) -> Void)?
     var onSessionFailed: ((PeerID, Error) -> Void)?
     var onHandshakeRecoveryRequired: ((NoiseHandshakeRecoveryRequest) -> Void)?
     
@@ -743,6 +757,9 @@ final class NoiseSessionManager {
                 recentOrdinaryInitiatorCompletions.removeValue(forKey: peerID)
                 session.reset()
 
+                let isIdentityMismatch =
+                    (error as? NoiseSessionError) == .peerIdentityMismatch
+
                 let restoredGeneration: UUID?
                 if let quarantined = quarantinedTransports.removeValue(forKey: peerID) {
                     sessions[peerID] = quarantined.session
@@ -753,16 +770,26 @@ final class NoiseSessionManager {
                     restoredGeneration = nil
                 }
 
+                // An identity mismatch is terminal: the counterpart failed to
+                // prove the claimed static key, so it never finished keys that
+                // could have replaced the restored ones. Every other restore
+                // that owns (or joins) a convergence retry must keep transport
+                // queues parked until that retry concludes — the counterpart
+                // may already have discarded the restored sending keys.
+                let restoreReason: NoiseSessionRestoreReason =
+                    !isIdentityMismatch
+                        && (shouldRequestRecovery
+                            || pendingHandshakeRecoveryIDs[peerID] != nil)
+                    ? .pendingConvergence
+                    : .terminal
+
                 // Schedule callback outside the synchronized block to prevent deadlock
                 DispatchQueue.global().async { [weak self] in
                     if let restoredGeneration {
-                        self?.onSessionRestored?(peerID, restoredGeneration)
+                        self?.onSessionRestored?(peerID, restoredGeneration, restoreReason)
                     }
                     self?.onSessionFailed?(peerID, error)
                 }
-
-                let isIdentityMismatch =
-                    (error as? NoiseSessionError) == .peerIdentityMismatch
                 if pendingHandshakeRecoveryIDs[peerID] != nil {
                     shouldSuppressImmediateHandlerRestart = true
                 }
@@ -912,8 +939,16 @@ final class NoiseSessionManager {
                 category: .session
             )
             if let restored {
+                // The mandatory convergence retry below owns the outbound
+                // resume: the timed-out counterpart may hold replacement keys
+                // that already discarded the restored generation's, so queue
+                // drains under it would be silently undecryptable.
                 DispatchQueue.global().async { [weak self] in
-                    self?.onSessionRestored?(peerID, restored.generation)
+                    self?.onSessionRestored?(
+                        peerID,
+                        restored.generation,
+                        .pendingConvergence
+                    )
                 }
             }
 

bitchat/Services/BLE/BLEService.swift

@@ -315,6 +315,10 @@ final class BLEService: NSObject {
     /// May block announce handling after verified-link rebind work is queued.
     /// Tests use this boundary to prove rebind and reconnect are serialized.
     var _test_afterVerifiedDirectRebindEnqueued: (() -> Void)?
+    /// May block the convergence-recovery callback on its global-queue thread
+    /// before it enqueues onto `messageQueue`. Tests use this boundary to
+    /// force the quarantine-restore handler to win the dispatch race.
+    var _test_beforeHandshakeRecoveryEnqueued: ((PeerID) -> Void)?
     #endif
     private var selfBroadcastTracker = BLESelfBroadcastTracker()
     private let meshTopology = MeshTopologyTracker()
@@ -387,6 +391,9 @@ final class BLEService: NSObject {
     // MARK: - Identity
     
     private var noiseService: NoiseEncryptionService
+    /// Injected so tests can compress the quarantine/rollback window;
+    /// production always passes the security-constant default.
+    private let noiseResponderHandshakeTimeout: TimeInterval
     private let identityManager: SecureIdentityStateManagerProtocol
     private let keychain: KeychainManagerProtocol
     private let idBridge: NostrIdentityBridge
@@ -503,14 +510,20 @@ final class BLEService: NSObject {
         identityManager: SecureIdentityStateManagerProtocol,
         initializeBluetoothManagers: Bool = true,
         incomingFileStore: BLEIncomingFileStore = BLEIncomingFileStore(),
-        startSuspendedForPanicRecovery: Bool = false
+        startSuspendedForPanicRecovery: Bool = false,
+        noiseResponderHandshakeTimeout: TimeInterval =
+            NoiseSecurityConstants.ordinaryResponderHandshakeTimeout
     ) {
         self.keychain = keychain
         self.idBridge = idBridge
         self.incomingFileStore = incomingFileStore
         self.shouldInitializeBluetoothManagers = initializeBluetoothManagers
         self._isPanicSuspended = startSuspendedForPanicRecovery
-        noiseService = NoiseEncryptionService(keychain: keychain)
+        self.noiseResponderHandshakeTimeout = noiseResponderHandshakeTimeout
+        noiseService = NoiseEncryptionService(
+            keychain: keychain,
+            ordinaryResponderHandshakeTimeout: noiseResponderHandshakeTimeout
+        )
         self.identityManager = identityManager
         super.init()
         
@@ -783,7 +796,10 @@ final class BLEService: NSObject {
             noiseService.clearEphemeralStateForPanic()
             noiseService.clearPersistentIdentity()
 
-            let newNoise = NoiseEncryptionService(keychain: keychain)
+            let newNoise = NoiseEncryptionService(
+                keychain: keychain,
+                ordinaryResponderHandshakeTimeout: noiseResponderHandshakeTimeout
+            )
             noiseService = newNoise
             configureNoiseServiceCallbacks(for: newNoise)
             refreshPeerIdentity()
@@ -3452,6 +3468,20 @@ extension BLEService {
         try noiseService.processHandshakeMessage(from: peerID, message: message)
     }
 
+    func _test_enqueuePendingPrivateMessage(
+        content: String,
+        messageID: String,
+        for peerID: PeerID
+    ) {
+        collectionsQueue.sync(flags: .barrier) {
+            pendingNoiseSessionQueues.appendPrivateMessage(
+                content: content,
+                messageID: messageID,
+                for: peerID
+            )
+        }
+    }
+
     func _test_enqueuePendingNoisePayload(
         _ payload: Data,
         transferId: String,
@@ -4697,6 +4727,9 @@ extension BLEService {
         service.onHandshakeRecoveryRequired = {
             [weak self, weak service] request in
             guard let self, let service else { return }
+            #if DEBUG
+            self._test_beforeHandshakeRecoveryEnqueued?(request.peerID)
+            #endif
             self.messageQueue.async(flags: .barrier) {
                 [weak self, weak service] in
                 guard let self,
@@ -4740,7 +4773,7 @@ extension BLEService {
                 }
             }
         }
-        service.onSessionRestoredWithGeneration = { [weak self, weak service] peerID, generation in
+        service.onSessionRestoredWithGeneration = { [weak self, weak service] peerID, generation, reason in
             guard let self, let service else { return }
             self.messageQueue.async { [weak self, weak service] in
                 guard let self,
@@ -4754,12 +4787,19 @@ extension BLEService {
                     category: .session
                 )
                 // Re-enter the same generation-bound transition used after a
-                // successful handshake. This restores authenticated protocol
+                // successful handshake to restore authenticated protocol
-                // state and drains both PM and typed-payload queues.
+                // state. Only a terminal restore may also drain the PM and
+                // typed-payload queues: after a responder timeout the
+                // counterpart may have completed the replacement handshake
+                // and discarded the restored keys, so encrypting the queues
+                // under them would lose every message silently. The mandatory
+                // convergence retry that accompanies the restore drains them
+                // under the new session instead (any establishment does).
                 self.handleNoisePeerAuthenticated(
                     peerID: peerID,
                     fingerprint: fingerprint,
-                    sessionGeneration: generation
+                    sessionGeneration: generation,
+                    deferOutboundUntilConvergence: reason == .pendingConvergence
                 )
             }
         }
@@ -4768,7 +4808,8 @@ extension BLEService {
     private func handleNoisePeerAuthenticated(
         peerID: PeerID,
         fingerprint: String,
-        sessionGeneration generation: UUID
+        sessionGeneration generation: UUID,
+        deferOutboundUntilConvergence: Bool = false
     ) {
         let normalizedPeerID = peerID.toShort()
         guard let transition = noiseService.withCurrentSessionGeneration(
@@ -4820,6 +4861,21 @@ extension BLEService {
             nonce: watchdog.nonce
         )
 
+        if deferOutboundUntilConvergence {
+            // Timeout-restore: the session is back for receive purposes and
+            // the generation-bound protocol state above is rebuilt, but the
+            // counterpart may already hold replacement keys that discarded
+            // this generation's. Encrypting the pending queues here would
+            // lose them silently, so leave them parked: the restore's
+            // mandatory convergence retry — or any later handshake the
+            // reconnect policy initiates — re-enters this transition with a
+            // fresh generation and drains them under keys both sides hold.
+            #if DEBUG
+            _test_onPrivateMediaSessionReconciled?(normalizedPeerID)
+            #endif
+            return
+        }
+
         // `onPeerAuthenticated` can fire while the initiator is returning XX
         // message 3. This callback is queued behind the handshake handler, so
         // message 3 is broadcast first. Both peers also send one idempotent

bitchat/Services/NoiseEncryptionService.swift

@@ -193,8 +193,11 @@ final class NoiseEncryptionService {
         ((_ request: NoiseHandshakeRecoveryRequest) -> Void)?
     /// An unauthenticated reconnect attempt failed or timed out and the
     /// receive-only rollback session became the active transport again.
-    /// Transport queues must be drained for this exact restored generation.
+    /// Transport queues may only be drained for this exact restored
-    var onSessionRestoredWithGeneration: ((_ peerID: PeerID, _ generation: UUID) -> Void)?
+    /// generation when the reason is terminal; a restore that owns a pending
+    /// convergence retry must keep them parked until the retry concludes.
+    var onSessionRestoredWithGeneration:
+        ((_ peerID: PeerID, _ generation: UUID, _ reason: NoiseSessionRestoreReason) -> Void)?
     
     // Add a handler for peer authentication
     func addOnPeerAuthenticatedHandler(_ handler: @escaping (PeerID, String) -> Void) {
@@ -345,8 +348,8 @@ final class NoiseEncryptionService {
                 sessionGeneration: generation
             )
         }
-        sessionManager.onSessionRestored = { [weak self] peerID, generation in
+        sessionManager.onSessionRestored = { [weak self] peerID, generation, reason in
-            self?.onSessionRestoredWithGeneration?(peerID, generation)
+            self?.onSessionRestoredWithGeneration?(peerID, generation, reason)
         }
         sessionManager.onHandshakeRecoveryRequired = { [weak self] request in
             self?.onHandshakeRecoveryRequired?(request)

bitchatTests/BLEServiceCoreTests.swift

@@ -762,6 +762,202 @@ struct BLEServiceCoreTests {
         )
     }
 
+    /// The message-loss interleaving behind a legitimate peer restart: the
+    /// remote completes a replacement handshake (discarding the old keys)
+    /// but its completion never arrives, so the local responder timeout
+    /// restores the quarantined OLD generation and requests one convergence
+    /// retry — both dispatched unordered. When the restore handler wins the
+    /// race, it must NOT drain the pending private-message/typed-payload
+    /// queues under the restored keys the remote no longer holds; the drain
+    /// has to wait for the convergence handshake and use its new session.
+    @Test
+    func timeoutRestoredSessionDefersQueueDrainUntilConvergence() async throws {
+        let ble = makeService(noiseResponderHandshakeTimeout: 0.3)
+        let alice = NoiseEncryptionService(keychain: MockKeychain())
+        let mallory = NoiseEncryptionService(keychain: MockKeychain())
+        let alicePeerID = PeerID(publicKey: alice.getStaticPublicKeyData())
+
+        // Establish BLE as responder so the inbound reconnect below is not
+        // coalesced by the initiator-completion grace path.
+        let message1 = try alice.initiateHandshake(with: ble.myPeerID)
+        let message2 = try #require(
+            try ble._test_noiseProcessHandshakeMessage(
+                from: alicePeerID,
+                message: message1
+            )
+        )
+        let message3 = try #require(
+            try alice.processHandshakeMessage(
+                from: ble.myPeerID,
+                message: message2
+            )
+        )
+        _ = try ble._test_noiseProcessHandshakeMessage(
+            from: alicePeerID,
+            message: message3
+        )
+        await ble._test_drainNoiseMessagePipeline()
+        #expect(ble.canDeliverSecurely(to: alicePeerID))
+
+        // The convergence retry only prepares for reachable peers.
+        ble._test_seedConnectedPeer(alicePeerID, nickname: "Alice")
+
+        let reconciled = SessionReconcileCounter()
+        ble._test_onPrivateMediaSessionReconciled = reconciled.record
+        // Park the convergence-recovery callback on its global-queue thread
+        // before it can enqueue onto messageQueue: the restore handler
+        // deterministically wins the dispatch race this test exercises.
+        let recoveryGate = HandshakeRecoveryEnqueueGate()
+        defer { recoveryGate.release() }
+        ble._test_beforeHandshakeRecoveryEnqueued = { _ in recoveryGate.pause() }
+        let outbound = OutboundPacketTap()
+        ble._test_onOutboundPacket = outbound.record
+
+        // Park the traffic the race would lose directly in the pending
+        // queues — the same place live sends land during quarantine — so no
+        // assertion below depends on outrunning the responder deadline under
+        // parallel test load. Nothing drains these queues while the current
+        // session stays untouched.
+        ble._test_enqueuePendingPrivateMessage(
+            content: "deferred private message",
+            messageID: "deferred-pm-\(UUID().uuidString)",
+            for: alicePeerID
+        )
+        ble._test_enqueuePendingNoisePayload(
+            NoisePayload(
+                type: .groupInvite,
+                data: Data("queued-during-quarantine".utf8)
+            ).encode(),
+            transferId: "deferred-invite-\(UUID().uuidString)",
+            for: alicePeerID
+        )
+
+        // An unauthenticated message 1 quarantines the established transport.
+        // Its message 3 never arrives, modeling the restarted peer whose
+        // completion was lost after it already discarded the old keys.
+        let reconnectMessage1 = try mallory.initiateHandshake(with: ble.myPeerID)
+        let reconnectPacket = BitchatPacket(
+            type: MessageType.noiseHandshake.rawValue,
+            senderID: Data(hexString: alicePeerID.id) ?? Data(),
+            recipientID: Data(hexString: ble.myPeerID.id),
+            timestamp: UInt64(Date().timeIntervalSince1970 * 1_000),
+            payload: reconnectMessage1,
+            signature: nil,
+            ttl: 7
+        )
+        ble._test_handlePacket(reconnectPacket, fromPeerID: alicePeerID)
+        // The responder's message 2 is a monotonic quarantine signal; the
+        // secure-delivery dip itself only lasts until the responder deadline,
+        // which parallel test load can outrun. (The recovery gate keeps the
+        // convergence retry's message 1 out of the tap until released.)
+        let responderReady = await TestHelpers.waitUntil(
+            { outbound.count(ofType: .noiseHandshake) >= 1 },
+            timeout: TestConstants.longTimeout
+        )
+        try #require(responderReady)
+        #expect(outbound.count(ofType: .noiseEncrypted) == 0)
+
+        // The responder timeout restores the quarantined generation; the
+        // gate guarantees its handler runs before the convergence retry.
+        let restoreRan = await TestHelpers.waitUntil(
+            { reconciled.count(for: alicePeerID) == 1 },
+            timeout: TestConstants.longTimeout
+        )
+        try #require(restoreRan)
+        #expect(ble.canDeliverSecurely(to: alicePeerID))
+        await ble._test_drainNoiseMessagePipeline()
+        // The parked queues must not have been encrypted under the restored
+        // old generation the remote may no longer be able to read.
+        #expect(outbound.count(ofType: .noiseEncrypted) == 0)
+
+        // Release the mandatory convergence retry: it retires the restored
+        // session and starts a fresh XX exchange with the live peer.
+        recoveryGate.release()
+        let retryStarted = await TestHelpers.waitUntil(
+            {
+                outbound.snapshot().contains {
+                    $0.type == MessageType.noiseHandshake.rawValue
+                        && PeerID(hexData: $0.senderID) == ble.myPeerID
+                        && $0.payload.count
+                            == NoiseSecurityConstants.xxInitialMessageSize
+                }
+            },
+            timeout: TestConstants.longTimeout
+        )
+        try #require(retryStarted)
+        #expect(outbound.count(ofType: .noiseEncrypted) == 0)
+        let retryMessage1 = try #require(
+            outbound.snapshot().last {
+                $0.type == MessageType.noiseHandshake.rawValue
+                    && PeerID(hexData: $0.senderID) == ble.myPeerID
+                    && $0.payload.count
+                        == NoiseSecurityConstants.xxInitialMessageSize
+            }?.payload
+        )
+
+        // Alice answers the retry as the restarted peer she models: her old
+        // session is gone, so the retry is a fresh responder exchange (and
+        // never the initiator-completion grace deferral, whose lower-peerID
+        // arm would otherwise coalesce the retry for random key orderings).
+        alice.clearSession(for: ble.myPeerID)
+        let retryMessage2 = try #require(
+            try alice.processHandshakeMessage(
+                from: ble.myPeerID,
+                message: retryMessage1
+            )
+        )
+        let retryPacket = BitchatPacket(
+            type: MessageType.noiseHandshake.rawValue,
+            senderID: Data(hexString: alicePeerID.id) ?? Data(),
+            recipientID: Data(hexString: ble.myPeerID.id),
+            timestamp: UInt64(Date().timeIntervalSince1970 * 1_000) + 1,
+            payload: retryMessage2,
+            signature: nil,
+            ttl: 7
+        )
+        ble._test_handlePacket(retryPacket, fromPeerID: alicePeerID)
+        let drained = await TestHelpers.waitUntil(
+            { outbound.count(ofType: .noiseEncrypted) >= 3 },
+            timeout: TestConstants.longTimeout
+        )
+        try #require(drained)
+        await ble._test_drainNoiseMessagePipeline()
+
+        // Alice completes with message 3, then must be able to decrypt every
+        // drained payload — proving nothing left under the old generation.
+        let retryMessage3 = try #require(
+            outbound.snapshot().last {
+                $0.type == MessageType.noiseHandshake.rawValue
+                    && PeerID(hexData: $0.senderID) == ble.myPeerID
+                    && $0.payload.count
+                        != NoiseSecurityConstants.xxInitialMessageSize
+            }?.payload
+        )
+        _ = try alice.processHandshakeMessage(
+            from: ble.myPeerID,
+            message: retryMessage3
+        )
+        let plaintexts = try outbound.snapshot()
+            .filter { $0.type == MessageType.noiseEncrypted.rawValue }
+            .map { try alice.decrypt($0.payload, from: ble.myPeerID) }
+        #expect(plaintexts.count == 3)
+        #expect(
+            plaintexts.filter {
+                $0.first == NoisePayloadType.authenticatedPeerState.rawValue
+            }.count == 1
+        )
+        #expect(
+            plaintexts.filter {
+                $0.first == NoisePayloadType.privateMessage.rawValue
+            }.count == 1
+        )
+        #expect(
+            plaintexts.filter {
+                $0.first == NoisePayloadType.groupInvite.rawValue
+            }.count == 1
+        )
+    }
+
     /// A legitimate rotation announce necessarily arrives on a link still
     /// bound to the OLD ID, so its registry upsert stores the new peer
     /// disconnected. The successful rebind must promote it: a healed
@@ -1095,6 +1291,45 @@ private final class OutboundPacketTap {
     }
 }
 
+/// Blocks the convergence-recovery callback on its global-queue thread so a
+/// test can prove the quarantine-restore handler wins the messageQueue race.
+private final class HandshakeRecoveryEnqueueGate: @unchecked Sendable {
+    private let condition = NSCondition()
+    private var released = false
+
+    func pause() {
+        condition.lock()
+        while !released {
+            condition.wait()
+        }
+        condition.unlock()
+    }
+
+    func release() {
+        condition.lock()
+        released = true
+        condition.broadcast()
+        condition.unlock()
+    }
+}
+
+/// Thread-safe counter for `_test_onPrivateMediaSessionReconciled` firings.
+private final class SessionReconcileCounter: @unchecked Sendable {
+    private let lock = NSLock()
+    private var reconciles: [PeerID] = []
+
+    func record(_ peerID: PeerID) {
+        lock.lock()
+        reconciles.append(peerID)
+        lock.unlock()
+    }
+
+    func count(for peerID: PeerID) -> Int {
+        lock.lock(); defer { lock.unlock() }
+        return reconciles.filter { $0 == peerID }.count
+    }
+}
+
 private final class VerifiedDirectRebindGate: @unchecked Sendable {
     private let condition = NSCondition()
     private var paused = false
@@ -1186,7 +1421,10 @@ private final class PanicIngressObserver: @unchecked Sendable {
     }
 }
 
-private func makeService() -> BLEService {
+private func makeService(
+    noiseResponderHandshakeTimeout: TimeInterval =
+        NoiseSecurityConstants.ordinaryResponderHandshakeTimeout
+) -> BLEService {
     let keychain = MockKeychain()
     let identityManager = MockIdentityManager(keychain)
     let idBridge = NostrIdentityBridge(keychain: MockKeychainHelper())
@@ -1194,7 +1432,8 @@ private func makeService() -> BLEService {
         keychain: keychain,
         idBridge: idBridge,
         identityManager: identityManager,
-        initializeBluetoothManagers: false
+        initializeBluetoothManagers: false,
+        noiseResponderHandshakeTimeout: noiseResponderHandshakeTimeout
     )
 }