diff --git a/bitchat/Nostr/NostrRelayManager.swift b/bitchat/Nostr/NostrRelayManager.swift index 5e0fc55d..da66f6ad 100644 --- a/bitchat/Nostr/NostrRelayManager.swift +++ b/bitchat/Nostr/NostrRelayManager.swift @@ -48,7 +48,14 @@ private struct URLSessionAdapter: NostrRelaySessionProtocol { let base: URLSession func webSocketTask(with url: URL) -> NostrRelayConnectionProtocol { - URLSessionWebSocketTaskAdapter(base: base.webSocketTask(with: url)) + let task = base.webSocketTask(with: url) + // Byte bound per inbound frame; without it the per-relay buffer cap + // (nostrInboundPerRelayBufferCap) bounds FRAMES but not BYTES, and a + // hostile relay could pile up cap × 1 MiB (URLSession default) per + // connection. See TransportConfig.nostrInboundMaxFrameBytes for the + // sizing rationale. Oversized frames fail the receive with an error. + task.maximumMessageSize = TransportConfig.nostrInboundMaxFrameBytes + return URLSessionWebSocketTaskAdapter(base: task) } } @@ -106,7 +113,10 @@ private extension NostrRelayManagerDependencies { @MainActor final class NostrRelayManager: ObservableObject { static let shared = NostrRelayManager() - // Track gift-wraps (kind 1059) we initiated so we can log OK acks at info + // Track gift-wraps (kind 1059) we initiated so we can log OK acks at info. + // Entries are removed only on OK acks (or panic wipe); relays that never + // ack leave entries behind for the process lifetime. Observability-only + // state, bounded in practice by outbound DM volume. private(set) static var pendingGiftWrapIDs = Set() static func registerPendingGiftWrap(id: String) { pendingGiftWrapIDs.insert(id) @@ -1427,7 +1437,9 @@ private final class InboundFrameRouter: @unchecked Sendable { /// Start a relay's stream + consumer if one does not already exist. /// Returns true when a new pipeline was created. The bounded /// `.bufferingNewest` policy makes a single relay shed its OWN oldest - /// frames under a flood, never other relays' frames and never memory. + /// frames under a flood, never other relays' frames. Buffered memory per + /// relay is bounded (not eliminated) at the frame cap times the per-frame + /// byte cap (`maximumMessageSize`) — see TransportConfig. func startPipeline( for relayUrl: String, makeConsumer: (AsyncStream) -> Task diff --git a/bitchat/Services/TransportConfig.swift b/bitchat/Services/TransportConfig.swift index ab0047f0..46777f2d 100644 --- a/bitchat/Services/TransportConfig.swift +++ b/bitchat/Services/TransportConfig.swift @@ -58,11 +58,23 @@ enum TransportConfig { // Bounded per-relay inbound frame buffer. Each relay connection owns its // own serial verify pipeline; if a relay floods faster than its Schnorr // verification drains, the oldest buffered frames for THAT relay are - // dropped (bufferingNewest) so one relay can neither exhaust memory nor - // stall other relays. Nostr inbound is already best-effort (relays are - // redundant and events replay), so dropping a flooding relay's backlog is - // safe. + // dropped (bufferingNewest) so one relay cannot stall other relays. + // Nostr inbound is already best-effort (relays are redundant and events + // replay), so dropping a flooding relay's backlog is safe. Together with + // nostrInboundMaxFrameBytes this caps buffered inbound bytes at + // cap × maxFrameBytes (128 MiB) per hostile relay — bounded, not zero. static let nostrInboundPerRelayBufferCap: Int = 256 + // Hard per-frame byte bound, applied as URLSessionWebSocketTask + // .maximumMessageSize (oversized frames fail the receive instead of + // buffering). BitChat's legitimate Nostr traffic is small: geohash chat / + // presence events (kind 20000/20001), kind-1 notes, and NIP-17 + // gift-wrapped DMs carrying text payloads or receipts are all a few KiB, + // and most public relays reject events beyond ~64–256 KiB anyway. 512 KiB + // leaves an order-of-magnitude margin over anything we produce or expect + // while halving the URLSession default (1 MiB), so a hostile relay's + // worst-case buffered pile-up per connection is + // nostrInboundPerRelayBufferCap × 512 KiB = 128 MiB instead of 256 MiB. + static let nostrInboundMaxFrameBytes: Int = 512 * 1024 // Conversation store diagnostics (field observability) // Sample interval for the periodic store-audit "OK" heartbeat line diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index a38dc529..8dbf39f4 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1177,6 +1177,11 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele // Geohash DM handlers can capture pre-wipe Nostr identities, so a plain // disconnect is not enough here. NostrRelayManager.shared.resetForPanicWipe() + // Clearing relay handlers stops NEW events, but a detached gift-wrap + // decrypt spawned just before the wipe still holds a pre-wipe key and + // ciphertext; bump the pipeline's wipe generation so its result is + // dropped at the main-actor delivery hop instead of landing here. + nostrCoordinator.inbound.invalidateInFlightDecrypts() nostrRelayManager = nil // Clear Nostr identity associations diff --git a/bitchat/ViewModels/NostrInboundPipeline.swift b/bitchat/ViewModels/NostrInboundPipeline.swift index e62e65f9..fbcd4703 100644 --- a/bitchat/ViewModels/NostrInboundPipeline.swift +++ b/bitchat/ViewModels/NostrInboundPipeline.swift @@ -91,6 +91,22 @@ final class NostrInboundPipeline { private let presence: GeoPresenceTracker private var geoEventLogCount = 0 + /// Monotonic panic-wipe generation for this pipeline. A panic wipe clears + /// relay handlers so no NEW events flow, but a detached decrypt task + /// spawned just BEFORE the wipe — which strongly captures a pre-wipe Nostr + /// private key and ciphertext — survives it. Spawn sites capture this + /// value; the task compares it at its main-actor hops and drops its result + /// (no delivery; the captured identity and plaintext die with the task) + /// if `invalidateInFlightDecrypts()` bumped it in between. + @MainActor private(set) var wipeGeneration: UInt64 = 0 + + /// Called from `ChatViewModel.panicClearAllData()` so plaintext decrypted + /// with pre-wipe keys can never land in post-wipe state. + @MainActor + func invalidateInFlightDecrypts() { + wipeGeneration &+= 1 + } + init(context: any NostrInboundPipelineContext, presence: GeoPresenceTracker) { self.context = context self.presence = presence @@ -269,8 +285,13 @@ final class NostrInboundPipeline { // once, off the main actor) by NostrRelayManager. guard !context.hasProcessedNostrEvent(giftWrap.id) else { return } + // Capture the wipe generation at spawn, alongside the per-geohash + // identity (private key) the detached task strongly captures. A panic + // wipe between spawn and delivery bumps the generation, and the task + // drops its result instead of delivering plaintext post-wipe. + let wipeGeneration = self.wipeGeneration Task.detached(priority: .userInitiated) { [weak self] in - await self?.processGeohashGiftWrap(giftWrap, id: id, verbose: false) + await self?.processGeohashGiftWrap(giftWrap, id: id, verbose: false, wipeGeneration: wipeGeneration) } } @@ -282,8 +303,10 @@ final class NostrInboundPipeline { return } + // Spawn-time wipe-generation capture; see subscribeGiftWrap. + let wipeGeneration = self.wipeGeneration Task.detached(priority: .userInitiated) { [weak self] in - await self?.processGeohashGiftWrap(giftWrap, id: id, verbose: true) + await self?.processGeohashGiftWrap(giftWrap, id: id, verbose: true, wipeGeneration: wipeGeneration) } } @@ -291,11 +314,24 @@ final class NostrInboundPipeline { /// layers) runs off the main actor; results hop back for state updates. /// `verbose` keeps `handleGiftWrap`'s decrypt logging without adding it /// to the sampling path. - private func processGeohashGiftWrap(_ giftWrap: NostrEvent, id: NostrIdentity, verbose: Bool) async { + /// + /// `wipeGeneration` is this pipeline's generation captured at spawn (the + /// moment the pre-wipe `id` was captured); a mismatch at either main-actor + /// hop means a panic wipe happened in between, so the task bails without + /// decrypting (first hop) or without delivering the plaintext (second + /// hop) — the captured identity and any decrypted material are simply + /// dropped with the task. + private func processGeohashGiftWrap( + _ giftWrap: NostrEvent, + id: NostrIdentity, + verbose: Bool, + wipeGeneration: UInt64 + ) async { guard let context else { return } // Authoritative check-and-record, atomic on the main actor so two // concurrent detached tasks can't both process the same event. let alreadyProcessed: Bool = await MainActor.run { + guard self.wipeGeneration == wipeGeneration else { return true } if context.hasProcessedNostrEvent(giftWrap.id) { return true } context.recordProcessedNostrEvent(giftWrap.id) return false @@ -320,6 +356,10 @@ final class NostrInboundPipeline { } await MainActor.run { + // A panic wipe during the off-main decrypt must not let the + // pre-wipe plaintext reach post-wipe state; drop it here, atomic + // with the wipe on the main actor. + guard self.wipeGeneration == wipeGeneration else { return } guard let packet = Self.decodeEmbeddedBitChatPacket(from: content), packet.type == MessageType.noiseEncrypted.rawValue, let payload = NoisePayload.decode(packet.payload) @@ -376,8 +416,13 @@ final class NostrInboundPipeline { return false } if alreadyProcessed { return } - let currentIdentity: NostrIdentity? = await MainActor.run { - context.currentNostrIdentity() + // Fetch the identity and the wipe generation in ONE main-actor hop: + // the generation then vouches for exactly this identity. A wipe after + // this point bumps the generation and the delivery hop below drops + // the decrypted result (same guard as processGeohashGiftWrap; this + // account-mailbox path had the identical hazard). + let (currentIdentity, wipeGeneration): (NostrIdentity?, UInt64) = await MainActor.run { + (context.currentNostrIdentity(), self.wipeGeneration) } guard let currentIdentity else { return } @@ -409,6 +454,9 @@ final class NostrInboundPipeline { let payload = NoisePayload.decode(packet.payload) { let messageTimestamp = Date(timeIntervalSince1970: TimeInterval(rumorTimestamp)) await MainActor.run { + // Drop pre-wipe plaintext if a panic wipe landed + // during the off-main decrypt (see above). + guard self.wipeGeneration == wipeGeneration else { return } context.registerNostrKeyMapping(senderPubkey, for: targetPeerID) switch payload.type { diff --git a/bitchatTests/ChatNostrCoordinatorContextTests.swift b/bitchatTests/ChatNostrCoordinatorContextTests.swift index deb3ccb9..b21e6d60 100644 --- a/bitchatTests/ChatNostrCoordinatorContextTests.swift +++ b/bitchatTests/ChatNostrCoordinatorContextTests.swift @@ -405,6 +405,45 @@ struct ChatNostrCoordinatorContextTests { #expect(context.handledPrivateMessages.count == 1) } + @Test @MainActor + func handleGiftWrap_panicWipeAfterSpawnDropsDecryptedResult() async throws { + let context = MockChatNostrContext() + let coordinator = ChatNostrCoordinator(context: context) + + let recipient = try NostrIdentity.generate() + let sender = try NostrIdentity.generate() + let embedded = try #require(NostrEmbeddedBitChat.encodePMForNostrNoRecipient( + content: "pre-wipe secret", + messageID: "gm-wipe-1", + senderPeerID: PeerID(str: "aabbccddeeff0011") + )) + let giftWrap = try NostrProtocol.createPrivateMessage( + content: embedded, + recipientPubkey: recipient.publicKeyHex, + senderIdentity: sender + ) + + // Spawn the detached decrypt (it strongly captures the pre-wipe + // identity), then panic-wipe in the SAME main-actor turn — guaranteed + // to land before the task's first main-actor hop. + coordinator.inbound.handleGiftWrap(giftWrap, id: recipient) + coordinator.inbound.invalidateInFlightDecrypts() + + // Give the detached task ample time to have delivered if the wipe + // guard were broken. + try? await Task.sleep(nanoseconds: 200_000_000) + await drainMainQueue() + + #expect(context.handledPrivateMessages.isEmpty) + #expect(context.recordedNostrEventIDs.isEmpty) + + // The pipeline itself stays usable: a gift wrap spawned AFTER the + // wipe (new generation) still decrypts and delivers. + coordinator.inbound.handleGiftWrap(giftWrap, id: recipient) + let delivered = await TestHelpers.waitUntil({ context.handledPrivateMessages.count == 1 }) + #expect(delivered) + } + // NOTE: Inbound Schnorr signature verification (and the forged-copy // dedup-poisoning invariant) is enforced once, off the main actor, at the // relay boundary — see NostrRelayManagerTests diff --git a/bitchatTests/Services/NostrRelayManagerTests.swift b/bitchatTests/Services/NostrRelayManagerTests.swift index 92ffe652..e2901653 100644 --- a/bitchatTests/Services/NostrRelayManagerTests.swift +++ b/bitchatTests/Services/NostrRelayManagerTests.swift @@ -639,11 +639,15 @@ final class NostrRelayManagerTests: XCTestCase { try context.sessionFactory.latestConnection(for: firstRelayURL)?.emitEventMessage(subscriptionID: "events", event: event) try context.sessionFactory.latestConnection(for: secondRelayURL)?.emitEventMessage(subscriptionID: "events", event: event) - let countedOnBothRelays = await waitUntil { + // Wait on the DELIVERY-side state: handler dispatch and the duplicate + // drop both land on the second main hop, after off-main verification. + let settled = await waitUntil { context.manager.relays.first(where: { $0.url == firstRelayURL })?.messagesReceived == 1 && - context.manager.relays.first(where: { $0.url == secondRelayURL })?.messagesReceived == 1 + context.manager.relays.first(where: { $0.url == secondRelayURL })?.messagesReceived == 1 && + receivedIDs == [event.id] && + context.manager.debugDuplicateInboundEventDropCount == 1 } - XCTAssertTrue(countedOnBothRelays) + XCTAssertTrue(settled) XCTAssertEqual(receivedIDs, [event.id]) XCTAssertEqual(context.manager.debugDuplicateInboundEventDropCount, 1) XCTAssertEqual(context.manager.debugDuplicateInboundEventDropCount(forSubscriptionID: "events"), 1) @@ -676,12 +680,17 @@ final class NostrRelayManagerTests: XCTestCase { ) } - let countedOnEveryRelay = await waitUntil { + // Wait on the DELIVERY-side state: the winner's handler dispatch and + // the losers' duplicate drops both land on the second main hop, after + // off-main verification — messagesReceived (first hop) settles sooner. + let settled = await waitUntil { relayURLs.allSatisfy { relayURL in context.manager.relays.first(where: { $0.url == relayURL })?.messagesReceived == 1 - } + } && + receivedIDs == [event.id] && + context.manager.debugDuplicateInboundEventDropCount == relayURLs.count - 1 } - XCTAssertTrue(countedOnEveryRelay) + XCTAssertTrue(settled) XCTAssertEqual(receivedIDs, [event.id]) XCTAssertEqual(context.manager.debugDuplicateInboundEventDropCount, relayURLs.count - 1) XCTAssertEqual( @@ -714,11 +723,15 @@ final class NostrRelayManagerTests: XCTestCase { try context.sessionFactory.latestConnection(for: firstRelayURL)?.emitEventMessage(subscriptionID: "events", event: invalidEvent) try context.sessionFactory.latestConnection(for: secondRelayURL)?.emitEventMessage(subscriptionID: "events", event: event) - let countedOnBothRelays = await waitUntil { + // Wait on the DELIVERY-side state (second main hop, after off-main + // verification), not just messagesReceived (first main hop) — the + // handler only fires after verify + a second hop. + let genuineDelivered = await waitUntil { context.manager.relays.first(where: { $0.url == firstRelayURL })?.messagesReceived == 1 && - context.manager.relays.first(where: { $0.url == secondRelayURL })?.messagesReceived == 1 + context.manager.relays.first(where: { $0.url == secondRelayURL })?.messagesReceived == 1 && + receivedIDs == [event.id] } - XCTAssertTrue(countedOnBothRelays) + XCTAssertTrue(genuineDelivered) XCTAssertEqual(receivedIDs, [event.id]) XCTAssertEqual(context.manager.debugDuplicateInboundEventDropCount, 0) XCTAssertEqual(context.manager.debugDuplicateInboundEventDropCount(forSubscriptionID: "events"), 0) @@ -758,11 +771,15 @@ final class NostrRelayManagerTests: XCTestCase { try context.sessionFactory.latestConnection(for: firstRelayURL)?.emitEventMessage(subscriptionID: "gift-wraps", event: tampered) try context.sessionFactory.latestConnection(for: secondRelayURL)?.emitEventMessage(subscriptionID: "gift-wraps", event: giftWrap) - let countedOnBothRelays = await waitUntil { + // Wait on the DELIVERY-side state (second main hop, after off-main + // verification), not just messagesReceived (first main hop) — the + // handler only fires after verify + a second hop. + let genuineDelivered = await waitUntil { context.manager.relays.first(where: { $0.url == firstRelayURL })?.messagesReceived == 1 && - context.manager.relays.first(where: { $0.url == secondRelayURL })?.messagesReceived == 1 + context.manager.relays.first(where: { $0.url == secondRelayURL })?.messagesReceived == 1 && + receivedIDs == [giftWrap.id] } - XCTAssertTrue(countedOnBothRelays) + XCTAssertTrue(genuineDelivered) XCTAssertEqual(receivedIDs, [giftWrap.id]) XCTAssertEqual(context.manager.debugDuplicateInboundEventDropCount, 0) }