diff --git a/bitchat/Noise/NoiseSessionManager.swift b/bitchat/Noise/NoiseSessionManager.swift index dfe9377b..d70c3aa5 100644 --- a/bitchat/Noise/NoiseSessionManager.swift +++ b/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 @@ -752,17 +769,27 @@ final class NoiseSessionManager { } else { 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 + ) } } diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index 82558893..5a806a97 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/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 - // state and drains both PM and typed-payload queues. + // successful handshake to restore authenticated protocol + // 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 diff --git a/bitchat/Services/NoiseEncryptionService.swift b/bitchat/Services/NoiseEncryptionService.swift index c1a1e878..9055d77c 100644 --- a/bitchat/Services/NoiseEncryptionService.swift +++ b/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. - var onSessionRestoredWithGeneration: ((_ peerID: PeerID, _ generation: UUID) -> Void)? + /// Transport queues may only be drained for this exact restored + /// 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 - self?.onSessionRestoredWithGeneration?(peerID, generation) + sessionManager.onSessionRestored = { [weak self] peerID, generation, reason in + self?.onSessionRestoredWithGeneration?(peerID, generation, reason) } sessionManager.onHandshakeRecoveryRequired = { [weak self] request in self?.onHandshakeRecoveryRequired?(request) diff --git a/bitchatTests/BLEServiceCoreTests.swift b/bitchatTests/BLEServiceCoreTests.swift index 5848e2c6..517fdf2a 100644 --- a/bitchatTests/BLEServiceCoreTests.swift +++ b/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 ) }