diff --git a/bitchat/Identity/SecureIdentityStateManager.swift b/bitchat/Identity/SecureIdentityStateManager.swift index a5d7b3cc..47adc201 100644 --- a/bitchat/Identity/SecureIdentityStateManager.swift +++ b/bitchat/Identity/SecureIdentityStateManager.swift @@ -151,38 +151,69 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { // Thread safety private let queue = DispatchQueue(label: "bitchat.identity.state", attributes: .concurrent) - // Debouncing for keychain saves - private var saveTimer: Timer? + // Debouncing for keychain saves. + // A DispatchSourceTimer on `queue` is used rather than Timer.scheduledTimer: + // saves are scheduled from inside `queue.async(flags: .barrier)` blocks that + // run on GCD worker threads, which have no active run loop, so a + // Timer.scheduledTimer there would never fire. + private var saveTimer: DispatchSourceTimer? private let saveDebounceInterval: TimeInterval = 2.0 // Save at most once every 2 seconds private var pendingSave = false - + // Encryption key private let encryptionKey: SymmetricKey + /// True when `encryptionKey` is a throwaway generated this session because the + /// persisted key could not be read (device locked / access denied). In that + /// state we must NOT persist (it would overwrite the real cache with data the + /// next launch can't decrypt) and must NOT delete the existing cache. + private let encryptionKeyIsEphemeral: Bool init(_ keychain: KeychainManagerProtocol) { self.keychain = keychain - // Generate or retrieve encryption key from keychain + // Retrieve (or, only on genuine first run, generate) the cache + // encryption key. We MUST distinguish "key doesn't exist yet" from a + // transient failure (device locked / access denied): the legacy + // getIdentityKey(forKey:) collapses both to nil, and generating+saving a + // new key deletes the existing one first — permanently orphaning the + // encrypted cache on a launch that merely couldn't read the key. let loadedKey: SymmetricKey - - // Try to load from keychain - if let keyData = keychain.getIdentityKey(forKey: encryptionKeyName) { + let keyIsEphemeral: Bool + + switch keychain.getIdentityKeyWithResult(forKey: encryptionKeyName) { + case .success(let keyData): loadedKey = SymmetricKey(data: keyData) + keyIsEphemeral = false SecureLogger.logKeyOperation(.load, keyType: "identity cache encryption key", success: true) - } - // Generate new key if needed - else { - loadedKey = SymmetricKey(size: .bits256) - let keyData = loadedKey.withUnsafeBytes { Data($0) } - // Save to keychain + + case .itemNotFound: + // Genuine first run: generate and persist a new key. + let newKey = SymmetricKey(size: .bits256) + let keyData = newKey.withUnsafeBytes { Data($0) } let saved = keychain.saveIdentityKey(keyData, forKey: encryptionKeyName) + loadedKey = newKey + // If even the save failed, treat the key as ephemeral so we don't + // later try to persist a cache the next launch can't read. + keyIsEphemeral = !saved SecureLogger.logKeyOperation(.generate, keyType: "identity cache encryption key", success: saved) + + case .deviceLocked, .authenticationFailed, .accessDenied, .otherError: + // Transient/critical read failure. Do NOT overwrite the persisted + // key. Use a session-only ephemeral key; the real key and cache are + // left intact for a healthy launch. + SecureLogger.warning("Identity cache key unavailable; using ephemeral key for this session (not persisting)", category: .security) + loadedKey = SymmetricKey(size: .bits256) + keyIsEphemeral = true } - + self.encryptionKey = loadedKey - - // Load identity cache on init - loadIdentityCache() + self.encryptionKeyIsEphemeral = keyIsEphemeral + + // Only read the persisted cache when we hold the real key; with an + // ephemeral key the decrypt would fail and discard the real cache. + if !keyIsEphemeral { + loadIdentityCache() + } } deinit { @@ -211,23 +242,38 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { } } + /// Schedules a debounced save. Always invoked on `queue` under a barrier, so + /// the timer/pendingSave bookkeeping is serialized with all other state. private func saveIdentityCache() { // Mark that we need to save pendingSave = true - - // Cancel any existing timer - saveTimer?.invalidate() - - // Schedule a new save after the debounce interval - saveTimer = Timer.scheduledTimer(withTimeInterval: saveDebounceInterval, repeats: false) { [weak self] _ in - self?.performSave() + + // Cancel any pending timer and schedule a fresh one on `queue`. + saveTimer?.cancel() + let timer = DispatchSource.makeTimerSource(queue: queue) + timer.schedule(deadline: .now() + saveDebounceInterval) + timer.setEventHandler { [weak self] in + guard let self else { return } + // Hop to a barrier so performSave reads/writes state exclusively. + self.queue.async(flags: .barrier) { self.performSave() } } + saveTimer = timer + timer.resume() } - + + /// Writes the cache to the keychain. Must run on `queue` with exclusive + /// (barrier) access. private func performSave() { guard pendingSave else { return } pendingSave = false - + + // Never persist under an ephemeral key — it would overwrite the real + // cache with data the next launch cannot decrypt. + guard !encryptionKeyIsEphemeral else { + SecureLogger.debug("Skipping identity cache save (ephemeral key this session)", category: .security) + return + } + do { let data = try JSONEncoder().encode(cache) let sealedBox = try AES.GCM.seal(data, using: encryptionKey) @@ -239,11 +285,15 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { SecureLogger.error(error, context: "Failed to save identity cache", category: .security) } } - - // Force immediate save (for app termination) + + // Force immediate save (for app termination). Runs synchronously on `queue` + // so the write completes before the caller proceeds (e.g. app exit). func forceSave() { - saveTimer?.invalidate() - performSave() + queue.sync(flags: .barrier) { + self.saveTimer?.cancel() + self.saveTimer = nil + self.performSave() + } } // MARK: - Social Identity Management diff --git a/bitchat/Noise/NoiseProtocol.swift b/bitchat/Noise/NoiseProtocol.swift index 665a5001..6c960742 100644 --- a/bitchat/Noise/NoiseProtocol.swift +++ b/bitchat/Noise/NoiseProtocol.swift @@ -322,6 +322,13 @@ final class NoiseCipherState { throw NoiseError.replayDetected } + // The 4-byte nonce prefix has been stripped, so the remaining bytes + // must still hold at least the 16-byte Poly1305 tag. The up-front + // `ciphertext.count >= 16` guard is not sufficient here (it counts + // the nonce), and `prefix(count - 16)` would trap on a short payload. + guard actualCiphertext.count >= 16 else { + throw NoiseError.invalidCiphertext + } // Split ciphertext and tag encryptedData = actualCiphertext.prefix(actualCiphertext.count - 16) tag = actualCiphertext.suffix(16) diff --git a/bitchat/Nostr/NostrIdentityBridge.swift b/bitchat/Nostr/NostrIdentityBridge.swift index c9fa8014..a2e091ba 100644 --- a/bitchat/Nostr/NostrIdentityBridge.swift +++ b/bitchat/Nostr/NostrIdentityBridge.swift @@ -82,6 +82,13 @@ final class NostrIdentityBridge { } deviceSeedCache = nil + // Also drop the in-memory derived per-geohash identities. These hold the + // actual secp256k1 private keys; if left cached, post-panic geohash + // messages would still be signed with pre-panic keys (linkable across the + // wipe) until the app is force-quit. + cacheLock.lock() + derivedIdentityCache.removeAll() + cacheLock.unlock() } // MARK: - Per-Geohash Identities (Location Channels) diff --git a/bitchat/Nostr/NostrProtocol.swift b/bitchat/Nostr/NostrProtocol.swift index 4b15bb58..e2d432a8 100644 --- a/bitchat/Nostr/NostrProtocol.swift +++ b/bitchat/Nostr/NostrProtocol.swift @@ -39,22 +39,23 @@ struct NostrProtocol { content: content ) - // 2. Create ephemeral key for this message - let ephemeralKey = try P256K.Schnorr.PrivateKey() - // Created ephemeral key for seal - - // 3. Seal the rumor (encrypt to recipient) + // 2. Seal the rumor (encrypt to recipient) and sign it with the SENDER'S + // real identity key. NIP-17 requires the seal be signed by the sender + // so the recipient can authenticate who sent the message; signing with + // a throwaway key leaves DMs forgeable/impersonatable. + let senderKey = try senderIdentity.schnorrSigningKey() let sealedEvent = try createSeal( rumor: rumor, recipientPubkey: recipientPubkey, - senderKey: ephemeralKey + senderKey: senderKey ) - - // 4. Gift wrap the sealed event (encrypt to recipient again) + + // 3. Gift wrap the sealed event with a throwaway ephemeral key (the wrap + // layer hides the sender's identity from relays; createGiftWrap mints + // its own ephemeral key internally). let giftWrap = try createGiftWrap( seal: sealedEvent, - recipientPubkey: recipientPubkey, - senderKey: ephemeralKey + recipientPubkey: recipientPubkey ) // Created gift wrap @@ -84,7 +85,15 @@ struct NostrProtocol { throw error } - // 2. Open the seal + // 2. Authenticate the seal. The seal MUST be signed by the sender's real + // identity key (NIP-17); without this check a DM is forgeable by anyone + // who knows the recipient's npub. Verify the seal's own signature. + guard seal.isValidSignature() else { + SecureLogger.error("❌ Rejecting DM: seal signature is missing or invalid", category: .session) + throw NostrError.invalidEvent + } + + // 3. Open the seal let rumor: NostrEvent do { rumor = try openSeal( @@ -96,8 +105,17 @@ struct NostrProtocol { SecureLogger.error("❌ Failed to open seal: \(error)", category: .session) throw error } - - return (content: rumor.content, senderPubkey: rumor.pubkey, timestamp: rumor.created_at) + + // 4. The sender claimed inside the rumor must match the key that actually + // signed the seal, otherwise the sender field is unauthenticated and + // spoofable. + guard seal.pubkey == rumor.pubkey else { + SecureLogger.error("❌ Rejecting DM: rumor pubkey does not match seal signer", category: .session) + throw NostrError.invalidEvent + } + + // Return the seal signer's pubkey as the authenticated sender. + return (content: rumor.content, senderPubkey: seal.pubkey, timestamp: rumor.created_at) } /// Create a geohash-scoped ephemeral public message (kind 20000) @@ -195,10 +213,9 @@ struct NostrProtocol { private static func createGiftWrap( seal: NostrEvent, - recipientPubkey: String, - senderKey: P256K.Schnorr.PrivateKey // This is the ephemeral key used for the seal + recipientPubkey: String ) throws -> NostrEvent { - + let sealJSON = try seal.jsonString() // Create new ephemeral key for gift wrap diff --git a/bitchat/Services/BLE/BLEAnnounceHandler.swift b/bitchat/Services/BLE/BLEAnnounceHandler.swift index e69a0d9a..fbd594a2 100644 --- a/bitchat/Services/BLE/BLEAnnounceHandler.swift +++ b/bitchat/Services/BLE/BLEAnnounceHandler.swift @@ -168,8 +168,13 @@ final class BLEAnnounceHandler { env.updateTopology(peerID, neighbors) } - // Persist cryptographic identity and signing key for robust offline verification - env.persistIdentity(announcement) + // Persist cryptographic identity and signing key for robust offline + // verification — only for verified announces. Persisting unverified + // announces would let an attacker who replays a victim's noisePublicKey + // overwrite the victim's stored signing key/nickname (identity poisoning). + if verifiedAnnounce { + env.persistIdentity(announcement) + } let announceBackID = "announce-back-\(peerID)" let shouldSendBack = !env.dedupContains(announceBackID) diff --git a/bitchat/Services/BLE/BLEPublicMessageHandler.swift b/bitchat/Services/BLE/BLEPublicMessageHandler.swift index 434eeaee..73e8f4a9 100644 --- a/bitchat/Services/BLE/BLEPublicMessageHandler.swift +++ b/bitchat/Services/BLE/BLEPublicMessageHandler.swift @@ -68,14 +68,29 @@ final class BLEPublicMessageHandler { // Snapshot peers to avoid concurrent mutation while iterating during nickname collision checks. let peersSnapshot = env.peersSnapshot() + // Public messages are always signed by their sender. `senderID` is + // attacker-controlled, so registry membership alone is NOT proof of + // identity — a peer in the registry as "verified" could be impersonated + // by anyone spoofing their senderID. Require a valid packet signature + // from the claimed sender (our own echoes are exempt; they are matched + // by self-broadcast tracking below). + let isSelf = peerID == env.localPeerID() + let signedDisplayName = isSelf ? nil : env.signedSenderDisplayName(packet, peerID) + guard isSelf || signedDisplayName != nil else { + SecureLogger.warning("🚫 Dropping public message with missing/invalid signature for claimed sender \(peerID.id.prefix(8))…", category: .security) + return + } + + // Authenticity is established; prefer the registry's collision-resolved + // display name, then the signature-derived name. guard let senderNickname = BLEPeerSenderDisplayName.resolveKnownPeer( peerID: peerID, localPeerID: env.localPeerID(), localNickname: env.localNickname(), peers: peersSnapshot, allowConnectedUnverified: false - ) ?? env.signedSenderDisplayName(packet, peerID) else { - SecureLogger.warning("🚫 Dropping public message from unverified or unknown peer \(peerID.id.prefix(8))…", category: .security) + ) ?? signedDisplayName else { + SecureLogger.warning("🚫 Dropping public message from unknown peer \(peerID.id.prefix(8))…", category: .security) return } diff --git a/bitchat/Services/BLE/BLEReceivePipeline.swift b/bitchat/Services/BLE/BLEReceivePipeline.swift index d8b46119..05bf81f4 100644 --- a/bitchat/Services/BLE/BLEReceivePipeline.swift +++ b/bitchat/Services/BLE/BLEReceivePipeline.swift @@ -12,7 +12,13 @@ struct BLEReceivedPacketContext: Equatable { struct BLEReceivePipeline { static func context(for packet: BitchatPacket, localPeerID: PeerID) -> BLEReceivedPacketContext { let senderID = PeerID(hexData: packet.senderID) - let messageID = "\(senderID)-\(packet.timestamp)-\(packet.type)" + // Include a payload digest so that distinct packets sharing the same + // sender/timestamp(ms)/type are not collapsed as duplicates. The + // post-handshake flush sends queued messages, delivery and read receipts + // back-to-back within a single millisecond; without the digest every + // packet after the first would be silently dropped. + let digestPrefix = packet.payload.sha256Hash().prefix(4).hexEncodedString() + let messageID = "\(senderID)-\(packet.timestamp)-\(packet.type)-\(digestPrefix)" let messageType = MessageType(rawValue: packet.type) let allowSelfSyncReplay = packet.ttl == 0 && senderID == localPeerID let shouldDeduplicate = messageType != .fragment && !allowSelfSyncReplay diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index 9959f179..121bb38f 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/bitchat/Services/BLE/BLEService.swift @@ -234,15 +234,7 @@ final class BLEService: NSObject { } // Single maintenance timer for all periodic tasks (dispatch-based for determinism) - let timer = DispatchSource.makeTimerSource(queue: bleQueue) - timer.schedule(deadline: .now() + TransportConfig.bleMaintenanceInterval, - repeating: TransportConfig.bleMaintenanceInterval, - leeway: .seconds(TransportConfig.bleMaintenanceLeewaySeconds)) - timer.setEventHandler { [weak self] in - self?.performMaintenance() - } - timer.resume() - maintenanceTimer = timer + startMaintenanceTimer() // Publish initial empty state requestPeerDataPublish() @@ -435,7 +427,29 @@ final class BLEService: NSObject { // MARK: Lifecycle + /// Creates and starts the periodic maintenance timer if it is not already + /// running. Idempotent so it can be called from both `init` and + /// `startServices()` — the latter matters after a panic reset, where + /// `stopServices()` cancels and nils the timer. + private func startMaintenanceTimer() { + guard maintenanceTimer == nil else { return } + let timer = DispatchSource.makeTimerSource(queue: bleQueue) + timer.schedule(deadline: .now() + TransportConfig.bleMaintenanceInterval, + repeating: TransportConfig.bleMaintenanceInterval, + leeway: .seconds(TransportConfig.bleMaintenanceLeewaySeconds)) + timer.setEventHandler { [weak self] in + self?.performMaintenance() + } + timer.resume() + maintenanceTimer = timer + } + func startServices() { + // Restart the maintenance timer if a prior stopServices() cancelled it + // (e.g. the panic flow), otherwise periodic announces, peer reconciliation + // and cache cleanup would never resume until app restart. + startMaintenanceTimer() + // Start BLE services if not already running if centralManager?.state == .poweredOn { centralManager?.scanForPeripherals( diff --git a/bitchat/Services/LocationStateManager.swift b/bitchat/Services/LocationStateManager.swift index 8ac62df4..aca9fb87 100644 --- a/bitchat/Services/LocationStateManager.swift +++ b/bitchat/Services/LocationStateManager.swift @@ -594,6 +594,22 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl } } + /// Removes all persisted location state and resets the in-memory view. + /// Used by the panic wipe — selected channel, teleport set and bookmarks + /// (which reveal where the user has been) must not survive on device. + func panicWipe() { + storage.removeObject(forKey: selectedChannelKey) + storage.removeObject(forKey: teleportedStoreKey) + storage.removeObject(forKey: bookmarksKey) + storage.removeObject(forKey: bookmarkNamesKey) + teleportedSet.removeAll() + bookmarkMembership.removeAll() + bookmarks = [] + bookmarkNames = [:] + teleported = false + selectedChannel = .mesh + } + private static func normalizeGeohash(_ s: String) -> String { let allowed = Set("0123456789bcdefghjkmnpqrstuvwxyz") return s diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 00f10ae3..1958caf8 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1129,6 +1129,11 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele userDefaults.removeObject(forKey: "bitchat.noiseIdentityKey") userDefaults.removeObject(forKey: "bitchat.messageRetentionKey") + // Wipe persisted location state (selected channel, teleport set, + // bookmarks). For an activist-safety wipe, where the user has been is + // exactly the data an adversary inspecting the device wants. + LocationStateManager.shared.panicWipe() + // Reset nickname to anonymous nickname = "anon\(Int.random(in: 1000...9999))" saveNickname() @@ -1180,8 +1185,12 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele // Small delay to ensure cleanup completes try? await Task.sleep(nanoseconds: TransportConfig.uiAsyncShortSleepNs) // 0.1 seconds - // Reinitialize Nostr relay manager with new identity - nostrRelayManager = NostrRelayManager() + // Reinitialize Nostr relay manager with new identity. Reuse the + // shared singleton — every other component (NostrTransport, geohash + // subscriptions, AppRuntime observers) is bound to `.shared`, so + // creating a fresh instance here would split relay state and leave + // sends running against a disconnected manager. + nostrRelayManager = NostrRelayManager.shared setupNostrMessageHandling() nostrRelayManager?.connect() } diff --git a/bitchatTests/Services/BLEAnnounceHandlerTests.swift b/bitchatTests/Services/BLEAnnounceHandlerTests.swift index 2aece692..80944ed4 100644 --- a/bitchatTests/Services/BLEAnnounceHandlerTests.swift +++ b/bitchatTests/Services/BLEAnnounceHandlerTests.swift @@ -173,7 +173,10 @@ struct BLEAnnounceHandlerTests { #expect(recorder.uiEventDeliveries.count == 1) #expect(recorder.uiEventDeliveries.first?.notifyPeerConnected == false) #expect(recorder.uiEventDeliveries.first?.scheduleInitialSync == false) - #expect(recorder.persistedIdentities.count == 1) + // Identity persistence MUST NOT occur for unverified announces: + // persisting would let an attacker who replays a victim's noisePublicKey + // overwrite the victim's stored signing key/nickname (identity poisoning). + #expect(recorder.persistedIdentities.isEmpty) #expect(recorder.trackedPackets.count == 1) #expect(recorder.announceBacks == 1) } diff --git a/bitchatTests/Services/BLEPublicMessageHandlerTests.swift b/bitchatTests/Services/BLEPublicMessageHandlerTests.swift index d669f4d4..24e174ab 100644 --- a/bitchatTests/Services/BLEPublicMessageHandlerTests.swift +++ b/bitchatTests/Services/BLEPublicMessageHandlerTests.swift @@ -59,13 +59,17 @@ struct BLEPublicMessageHandlerTests { let now = Date(timeIntervalSince1970: 1_000) let recorder = Recorder() recorder.peers = [remotePeerID: makePeerInfo(remotePeerID, nickname: "Alice", isVerified: true)] + // A valid packet signature is required even for a registry-verified peer: + // senderID is spoofable, so registry membership alone is not authentication. + recorder.signedName = "SignedAlice" let handler = makeHandler(recorder: recorder, now: now) let packet = makeMessagePacket(sender: remotePeerID, content: "hello mesh", timestamp: timestamp(now)) handler.handle(packet, from: remotePeerID) #expect(recorder.peersSnapshotReads == 1) - #expect(recorder.signedNameQueries.isEmpty) + // Signature is verified, then the registry's collision-resolved name is preferred. + #expect(recorder.signedNameQueries == [remotePeerID]) #expect(recorder.trackedPackets.count == 1) #expect(recorder.selfBroadcastTakes.isEmpty) #expect(recorder.deliveries.count == 1) @@ -154,6 +158,7 @@ struct BLEPublicMessageHandlerTests { let now = Date(timeIntervalSince1970: 1_000) let recorder = Recorder() recorder.peers = [remotePeerID: makePeerInfo(remotePeerID, nickname: "Alice", isVerified: true)] + recorder.signedName = "SignedAlice" let handler = makeHandler(recorder: recorder, now: now) let packet = makeMessagePacket(sender: remotePeerID, payload: Data([0xFF, 0xFE, 0xFD]), timestamp: timestamp(now)) @@ -187,6 +192,7 @@ struct BLEPublicMessageHandlerTests { let now = Date(timeIntervalSince1970: 1_000) let recorder = Recorder() recorder.peers = [remotePeerID: makePeerInfo(remotePeerID, nickname: "Alice", isVerified: true)] + recorder.signedName = "SignedAlice" let handler = makeHandler(recorder: recorder, now: now) let packet = makeMessagePacket( sender: remotePeerID, diff --git a/bitchatTests/Services/BLEReceivePipelineTests.swift b/bitchatTests/Services/BLEReceivePipelineTests.swift index 91a63efd..d434f7db 100644 --- a/bitchatTests/Services/BLEReceivePipelineTests.swift +++ b/bitchatTests/Services/BLEReceivePipelineTests.swift @@ -14,7 +14,10 @@ struct BLEReceivePipelineTests { let context = BLEReceivePipeline.context(for: packet, localPeerID: local) #expect(context.senderID == sender) - #expect(context.messageID == "\(sender)-1234-\(MessageType.message.rawValue)") + // The message ID includes a payload digest so distinct packets sharing a + // sender/timestamp(ms)/type are not collapsed as duplicates. + let digest = packet.payload.sha256Hash().prefix(4).hexEncodedString() + #expect(context.messageID == "\(sender)-1234-\(MessageType.message.rawValue)-\(digest)") #expect(context.messageType == .message) #expect(context.shouldDeduplicate) #expect(context.logsHandlingDetails)