Merge branch 'main' into fix/nostr-transport-thread-safety

bitchat/Noise/NoiseProtocol.swift

@@ -610,14 +610,20 @@ final class NoiseHandshakeState {
                         throw NoiseError.missingKeys
                     }
                     let shared = try localEphemeral.sharedSecretFromKeyAgreement(with: remoteStatic)
-                    symmetricState.mixKey(shared.withUnsafeBytes { Data($0) })
+                    var sharedData = shared.withUnsafeBytes { Data($0) }
+                    symmetricState.mixKey(sharedData)
+                    // Clear sensitive shared secret
+                    keychain.secureClear(&sharedData)
                 } else {
                     guard let localStatic = localStaticPrivate,
                           let remoteEphemeral = remoteEphemeralPublic else {
                         throw NoiseError.missingKeys
                     }
                     let shared = try localStatic.sharedSecretFromKeyAgreement(with: remoteEphemeral)
-                    symmetricState.mixKey(shared.withUnsafeBytes { Data($0) })
+                    var sharedData = shared.withUnsafeBytes { Data($0) }
+                    symmetricState.mixKey(sharedData)
+                    // Clear sensitive shared secret
+                    keychain.secureClear(&sharedData)
                 }
                 
             case .se:
@@ -628,14 +634,20 @@ final class NoiseHandshakeState {
                         throw NoiseError.missingKeys
                     }
                     let shared = try localStatic.sharedSecretFromKeyAgreement(with: remoteEphemeral)
-                    symmetricState.mixKey(shared.withUnsafeBytes { Data($0) })
+                    var sharedData = shared.withUnsafeBytes { Data($0) }
+                    symmetricState.mixKey(sharedData)
+                    // Clear sensitive shared secret
+                    keychain.secureClear(&sharedData)
                 } else {
                     guard let localEphemeral = localEphemeralPrivate,
                           let remoteStatic = remoteStaticPublic else {
                         throw NoiseError.missingKeys
                     }
                     let shared = try localEphemeral.sharedSecretFromKeyAgreement(with: remoteStatic)
-                    symmetricState.mixKey(shared.withUnsafeBytes { Data($0) })
+                    var sharedData = shared.withUnsafeBytes { Data($0) }
+                    symmetricState.mixKey(sharedData)
+                    // Clear sensitive shared secret
+                    keychain.secureClear(&sharedData)
                 }
                 
             case .ss:
@@ -724,8 +736,11 @@ final class NoiseHandshakeState {
                 throw NoiseError.missingKeys
             }
             let shared = try localEphemeral.sharedSecretFromKeyAgreement(with: remoteEphemeral)
-            symmetricState.mixKey(shared.withUnsafeBytes { Data($0) })
-            
+            var sharedData = shared.withUnsafeBytes { Data($0) }
+            symmetricState.mixKey(sharedData)
+            // Clear sensitive shared secret
+            keychain.secureClear(&sharedData)
+
         case .es:
             if role == .initiator {
                 guard let localEphemeral = localEphemeralPrivate,
@@ -778,8 +793,11 @@ final class NoiseHandshakeState {
                 throw NoiseError.missingKeys
             }
             let shared = try localStatic.sharedSecretFromKeyAgreement(with: remoteStatic)
-            symmetricState.mixKey(shared.withUnsafeBytes { Data($0) })
-            
+            var sharedData = shared.withUnsafeBytes { Data($0) }
+            symmetricState.mixKey(sharedData)
+            // Clear sensitive shared secret
+            keychain.secureClear(&sharedData)
+
         case .e, .s:
             break
         }

bitchat/Services/MessageDeduplicationService.swift

@@ -12,6 +12,8 @@ import Foundation
 
 /// Generic LRU (Least Recently Used) cache for deduplication.
 /// Uses an efficient O(1) lookup with periodic compaction.
+/// Thread-safe via @MainActor - all callers are already on main actor.
+@MainActor
 final class LRUDeduplicationCache<Value> {
     private var map: [String: Value] = [:]
     private var order: [String] = []
@@ -157,6 +159,8 @@ enum ContentNormalizer {
 
 /// Service that manages message deduplication using LRU caches.
 /// Provides separate caches for content-based dedup and Nostr event ID dedup.
+/// Thread-safe via @MainActor - all callers are already on main actor.
+@MainActor
 final class MessageDeduplicationService {
 
     /// Cache for content-based near-duplicate detection

bitchatTests/MessageDeduplicationServiceTests.swift

@@ -12,6 +12,8 @@ import Foundation
 
 // MARK: - LRU Deduplication Cache Tests
 
+@Suite("LRU Deduplication Cache")
+@MainActor
 struct LRUDeduplicationCacheTests {
 
     // MARK: - Basic Operations
@@ -265,6 +267,8 @@ struct ContentNormalizerTests {
 
 // MARK: - Message Deduplication Service Tests
 
+@Suite("Message Deduplication Service")
+@MainActor
 struct MessageDeduplicationServiceTests {
 
     // MARK: - Content Deduplication
@@ -467,4 +471,134 @@ struct MessageDeduplicationServiceTests {
         #expect(service.contentTimestamp(for: "hello world") == now)
         #expect(service.contentTimestamp(for: "Hello World") == now)
     }
+
+    // MARK: - Thread Safety Tests (via @MainActor enforcement)
+
+    @Test("Concurrent content recording is safe via MainActor")
+    func concurrentContentRecording() async {
+        let service = MessageDeduplicationService(contentCapacity: 1000, nostrEventCapacity: 1000)
+        let iterations = 100
+
+        // All operations run on MainActor due to @MainActor annotation
+        // This test verifies the pattern works correctly
+        await withTaskGroup(of: Void.self) { group in
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    service.recordContent("Message \(i)", timestamp: Date())
+                }
+            }
+        }
+
+        // Verify some entries were recorded
+        #expect(service.contentTimestamp(for: "Message 0") != nil)
+        #expect(service.contentTimestamp(for: "Message 99") != nil)
+    }
+
+    @Test("Concurrent Nostr event recording is safe via MainActor")
+    func concurrentNostrEventRecording() async {
+        let service = MessageDeduplicationService(contentCapacity: 1000, nostrEventCapacity: 1000)
+        let iterations = 100
+
+        await withTaskGroup(of: Void.self) { group in
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    service.recordNostrEvent("event_\(i)")
+                }
+            }
+        }
+
+        // Verify events were recorded
+        #expect(service.hasProcessedNostrEvent("event_0"))
+        #expect(service.hasProcessedNostrEvent("event_99"))
+    }
+
+    @Test("Mixed concurrent operations are safe via MainActor")
+    func concurrentMixedOperations() async {
+        let service = MessageDeduplicationService(contentCapacity: 1000, nostrEventCapacity: 1000)
+        let iterations = 50
+
+        await withTaskGroup(of: Void.self) { group in
+            // Content recording tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    service.recordContent("Content \(i)", timestamp: Date())
+                }
+            }
+
+            // Event recording tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    service.recordNostrEvent("event_\(i)")
+                }
+            }
+
+            // ACK recording tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    service.recordNostrAck("ack_\(i)")
+                }
+            }
+
+            // Read tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    _ = service.contentTimestamp(for: "Content \(i)")
+                    _ = service.hasProcessedNostrEvent("event_\(i)")
+                    _ = service.hasProcessedNostrAck("ack_\(i)")
+                }
+            }
+        }
+
+        // If we reach here without crashes, the test passes
+    }
+}
+
+// MARK: - LRU Cache Thread Safety Tests
+
+@Suite("LRU Cache Thread Safety")
+@MainActor
+struct LRUCacheThreadSafetyTests {
+
+    @Test("Concurrent cache access is safe via MainActor")
+    func concurrentCacheAccess() async {
+        let cache = LRUDeduplicationCache<Int>(capacity: 500)
+        let iterations = 100
+
+        await withTaskGroup(of: Void.self) { group in
+            // Write tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    cache.record("key_\(i)", value: i)
+                }
+            }
+
+            // Read tasks
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    _ = cache.contains("key_\(i)")
+                    _ = cache.value(for: "key_\(i)")
+                }
+            }
+        }
+
+        // Verify cache is in consistent state
+        #expect(cache.count <= 500) // Respects capacity
+    }
+
+    @Test("Cache eviction under concurrent load is safe")
+    func cacheEvictionUnderLoad() async {
+        let cache = LRUDeduplicationCache<Int>(capacity: 10)
+        let iterations = 100
+
+        await withTaskGroup(of: Void.self) { group in
+            for i in 0..<iterations {
+                group.addTask { @MainActor in
+                    cache.record("key_\(i)", value: i)
+                }
+            }
+        }
+
+        // Cache should maintain its capacity constraint
+        #expect(cache.count == 10)
+    }
 }

bitchatTests/Mocks/MockKeychain.swift

@@ -65,3 +65,90 @@ final class MockKeychain: KeychainManagerProtocol {
 
 /// Typealias for backwards compatibility with tests using MockKeychainHelper
 typealias MockKeychainHelper = MockKeychain
+
+/// Mock keychain that tracks secureClear calls for testing DH secret clearing
+final class TrackingMockKeychain: KeychainManagerProtocol {
+    private var storage: [String: Data] = [:]
+    private var serviceStorage: [String: [String: Data]] = [:]
+
+    /// Thread-safe counter for secureClear calls
+    private let lock = NSLock()
+    private var _secureClearDataCallCount = 0
+    private var _secureClearStringCallCount = 0
+
+    var secureClearDataCallCount: Int {
+        lock.lock()
+        defer { lock.unlock() }
+        return _secureClearDataCallCount
+    }
+
+    var secureClearStringCallCount: Int {
+        lock.lock()
+        defer { lock.unlock() }
+        return _secureClearStringCallCount
+    }
+
+    var totalSecureClearCallCount: Int {
+        return secureClearDataCallCount + secureClearStringCallCount
+    }
+
+    func resetCounts() {
+        lock.lock()
+        defer { lock.unlock() }
+        _secureClearDataCallCount = 0
+        _secureClearStringCallCount = 0
+    }
+
+    func saveIdentityKey(_ keyData: Data, forKey key: String) -> Bool {
+        storage[key] = keyData
+        return true
+    }
+
+    func getIdentityKey(forKey key: String) -> Data? {
+        storage[key]
+    }
+
+    func deleteIdentityKey(forKey key: String) -> Bool {
+        storage.removeValue(forKey: key)
+        return true
+    }
+
+    func deleteAllKeychainData() -> Bool {
+        storage.removeAll()
+        serviceStorage.removeAll()
+        return true
+    }
+
+    func secureClear(_ data: inout Data) {
+        lock.lock()
+        _secureClearDataCallCount += 1
+        lock.unlock()
+        data = Data()
+    }
+
+    func secureClear(_ string: inout String) {
+        lock.lock()
+        _secureClearStringCallCount += 1
+        lock.unlock()
+        string = ""
+    }
+
+    func verifyIdentityKeyExists() -> Bool {
+        storage["identity_noiseStaticKey"] != nil
+    }
+
+    func save(key: String, data: Data, service: String, accessible: CFString?) {
+        if serviceStorage[service] == nil {
+            serviceStorage[service] = [:]
+        }
+        serviceStorage[service]?[key] = data
+    }
+
+    func load(key: String, service: String) -> Data? {
+        serviceStorage[service]?[key]
+    }
+
+    func delete(key: String, service: String) {
+        serviceStorage[service]?.removeValue(forKey: key)
+    }
+}

bitchatTests/Noise/NoiseProtocolTests.swift

@@ -789,4 +789,159 @@ struct NoiseProtocolTests {
                 "Message \(index + 1): Decrypted payload should match original")
         }
     }
+
+    // MARK: - DH Shared Secret Clearing Tests
+
+    @Test func secureClearCalledDuringHandshake() throws {
+        // Use TrackingMockKeychain to verify secureClear is called
+        let trackingKeychain = TrackingMockKeychain()
+
+        let aliceKey = Curve25519.KeyAgreement.PrivateKey()
+        let bobKey = Curve25519.KeyAgreement.PrivateKey()
+
+        let alice = NoiseSession(
+            peerID: PeerID(str: "alice-test"),
+            role: .initiator,
+            keychain: trackingKeychain,
+            localStaticKey: aliceKey
+        )
+
+        let bob = NoiseSession(
+            peerID: PeerID(str: "bob-test"),
+            role: .responder,
+            keychain: trackingKeychain,
+            localStaticKey: bobKey
+        )
+
+        // Perform handshake
+        let msg1 = try alice.startHandshake()
+        let msg2 = try bob.processHandshakeMessage(msg1)!
+        let msg3 = try alice.processHandshakeMessage(msg2)!
+        _ = try bob.processHandshakeMessage(msg3)
+
+        // In Noise XX pattern handshake:
+        // - Message 1 (initiator): e token only (no DH)
+        // - Message 2 (responder): e, ee, s, es tokens (2 DH operations: ee, es)
+        // - Message 3 (initiator): s, se tokens (1 DH operation: se)
+        // Total in writeMessage: 3 DH operations (ee, es, se)
+        //
+        // In readMessage (performDHOperation):
+        // - After msg1: no DH
+        // - After msg2: ee, es (2 DH operations)
+        // - After msg3: se (1 DH operation)
+        // Total in performDHOperation: 3 DH operations
+        //
+        // Grand total: 6 DH operations requiring secureClear
+        //
+        // Note: .ss pattern is only used in certain handshake patterns, not XX
+        let expectedMinimumCalls = 6
+        #expect(
+            trackingKeychain.secureClearDataCallCount >= expectedMinimumCalls,
+            "Expected at least \(expectedMinimumCalls) secureClear calls for DH secrets, got \(trackingKeychain.secureClearDataCallCount)"
+        )
+    }
+
+    @Test func encryptionWorksAfterSecureClear() throws {
+        // Verify that encryption/decryption still works correctly after adding secureClear
+        let trackingKeychain = TrackingMockKeychain()
+
+        let aliceKey = Curve25519.KeyAgreement.PrivateKey()
+        let bobKey = Curve25519.KeyAgreement.PrivateKey()
+
+        let alice = NoiseSession(
+            peerID: PeerID(str: "alice-test-enc"),
+            role: .initiator,
+            keychain: trackingKeychain,
+            localStaticKey: aliceKey
+        )
+
+        let bob = NoiseSession(
+            peerID: PeerID(str: "bob-test-enc"),
+            role: .responder,
+            keychain: trackingKeychain,
+            localStaticKey: bobKey
+        )
+
+        // Perform handshake
+        let msg1 = try alice.startHandshake()
+        let msg2 = try bob.processHandshakeMessage(msg1)!
+        let msg3 = try alice.processHandshakeMessage(msg2)!
+        _ = try bob.processHandshakeMessage(msg3)
+
+        // Verify both sessions are established
+        #expect(alice.isEstablished())
+        #expect(bob.isEstablished())
+
+        // Verify secureClear was called (basic sanity check)
+        #expect(trackingKeychain.secureClearDataCallCount > 0)
+
+        // Test encryption from Alice to Bob
+        let plaintext1 = "Hello from Alice after secureClear!".data(using: .utf8)!
+        let ciphertext1 = try alice.encrypt(plaintext1)
+        let decrypted1 = try bob.decrypt(ciphertext1)
+        #expect(decrypted1 == plaintext1)
+
+        // Test encryption from Bob to Alice
+        let plaintext2 = "Hello from Bob after secureClear!".data(using: .utf8)!
+        let ciphertext2 = try bob.encrypt(plaintext2)
+        let decrypted2 = try alice.decrypt(ciphertext2)
+        #expect(decrypted2 == plaintext2)
+
+        // Test multiple messages to verify cipher state is correct
+        for i in 1...10 {
+            let msg = "Message \(i) from Alice".data(using: .utf8)!
+            let cipher = try alice.encrypt(msg)
+            let dec = try bob.decrypt(cipher)
+            #expect(dec == msg)
+        }
+    }
+
+    @Test func secureClearCalledInBothWriteAndReadPaths() throws {
+        // Verify secureClear is called in both writeMessage and readMessage paths
+        // We do this by checking the count increases at each step
+
+        let aliceKeychain = TrackingMockKeychain()
+        let bobKeychain = TrackingMockKeychain()
+
+        let aliceKey = Curve25519.KeyAgreement.PrivateKey()
+        let bobKey = Curve25519.KeyAgreement.PrivateKey()
+
+        let alice = NoiseSession(
+            peerID: PeerID(str: "alice-paths"),
+            role: .initiator,
+            keychain: aliceKeychain,
+            localStaticKey: aliceKey
+        )
+
+        let bob = NoiseSession(
+            peerID: PeerID(str: "bob-paths"),
+            role: .responder,
+            keychain: bobKeychain,
+            localStaticKey: bobKey
+        )
+
+        // Message 1: Alice writes (e token only, no DH)
+        let msg1 = try alice.startHandshake()
+        let aliceCountAfterMsg1 = aliceKeychain.secureClearDataCallCount
+        // No DH in message 1 for initiator
+        #expect(aliceCountAfterMsg1 == 0, "No DH secrets in message 1 write")
+
+        // Bob reads message 1 (no DH) and writes message 2 (ee, es DH operations)
+        let msg2 = try bob.processHandshakeMessage(msg1)!
+        let bobCountAfterMsg2 = bobKeychain.secureClearDataCallCount
+        // Bob should have cleared secrets for: ee (read), es (read), ee (write), es (write)
+        #expect(bobCountAfterMsg2 >= 2, "Bob should clear DH secrets when processing/writing message 2")
+
+        // Alice reads message 2 (ee, es) and writes message 3 (se)
+        let msg3 = try alice.processHandshakeMessage(msg2)!
+        let aliceCountAfterMsg3 = aliceKeychain.secureClearDataCallCount
+        // Alice should have cleared: ee (read), es (read), se (write)
+        #expect(aliceCountAfterMsg3 >= 3, "Alice should clear DH secrets when processing/writing message 3")
+
+        // Bob reads message 3 (se)
+        _ = try bob.processHandshakeMessage(msg3)
+        let bobFinalCount = bobKeychain.secureClearDataCallCount
+        // Bob should have additionally cleared: se (read)
+        #expect(bobFinalCount > bobCountAfterMsg2, "Bob should clear DH secrets when processing message 3")
+    }
 }