Fail closed on unverifiable handshake peer IDs + check SecRandomCopyBytes results (#1645)

Two hardening fixes from the repo evaluation:

- NoiseSessionManager.authenticatedRemoteKey returned true for peer IDs
  that are neither 16-hex wire IDs nor full Noise-key IDs — an
  accept-any-key fallback kept for test harnesses. It now fails closed;
  the Noise/integration/E2E tests that relied on it address peers by
  key-derived wire IDs instead (the pattern NoiseCoverageTests already
  used), and a new regression test pins the rejection.

- Four SecRandomCopyBytes call sites discarded the return status. The
  two verification nonces now fail their operation on error, the Nostr
  device identity seed uses CryptoKit key generation (cannot fail, and
  can no longer silently persist an all-zero seed), and BIP-340 aux
  randomness throws on failure like the adjacent nonce path.

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

bitchat/Noise/NoiseSessionManager.swift

@@ -1126,8 +1126,8 @@ final class NoiseSessionManager {
 
     /// Mesh handshakes normally use a 16-hex wire ID. Full Noise-key IDs are
     /// also accepted by internal callers when they exactly match the static
-    /// key. Non-wire identifiers remain available to protocol test harnesses;
-    /// BLE packet ingress always supplies a short hexadecimal ID.
+    /// key. Anything else fails closed: an identifier that can't be checked
+    /// against the remote static key must never complete a handshake.
     private func authenticatedRemoteKey(
         _ remoteKey: Curve25519.KeyAgreement.PublicKey,
         matches claimedPeerID: PeerID
@@ -1139,7 +1139,7 @@ final class NoiseSessionManager {
         if let claimedNoiseKey = claimedPeerID.noiseKey {
             return claimedNoiseKey == rawKey
         }
-        return true
+        return false
     }
     
     // MARK: - Encryption/Decryption

bitchat/Nostr/NostrIdentityBridge.swift

@@ -76,10 +76,9 @@ final class NostrIdentityBridge {
             deviceSeedCache = existing
             return existing
         }
-        var seed = Data(count: 32)
-        _ = seed.withUnsafeMutableBytes { ptr in
-            SecRandomCopyBytes(kSecRandomDefault, 32, ptr.baseAddress!)
-        }
+        // CryptoKit key generation cannot fail, unlike SecRandomCopyBytes —
+        // a discarded failure here would persist an all-zero identity seed.
+        let seed = SymmetricKey(size: .bits256).withUnsafeBytes { Data($0) }
         // Ensure availability after first unlock to prevent unintended rotation when locked
         keychain.save(key: deviceSeedKey, data: seed, service: keychainService, accessible: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly)
         deviceSeedCache = seed

bitchat/Nostr/NostrProtocol.swift

@@ -836,9 +836,12 @@ struct NostrEvent: Codable {
         // Sign with Schnorr (BIP-340)
         var messageBytes = [UInt8](eventIdHash)
         var auxRand = [UInt8](repeating: 0, count: 32)
-        _ = auxRand.withUnsafeMutableBytes { ptr in
+        let auxStatus = auxRand.withUnsafeMutableBytes { ptr in
             SecRandomCopyBytes(kSecRandomDefault, 32, ptr.baseAddress!)
         }
+        guard auxStatus == errSecSuccess else {
+            throw NostrError.cryptographicFailure
+        }
         let schnorrSignature = try key.signature(message: &messageBytes, auxiliaryRand: &auxRand)
         
         let signatureHex = schnorrSignature.dataRepresentation.hexEncodedString()

bitchat/Services/VerificationService.swift

@@ -84,7 +84,8 @@ final class VerificationService {
         let signKey = transport.noiseSigningPublicKeyData().hexEncodedString()
         let ts = Int64(Date().timeIntervalSince1970)
         var nonce = Data(count: 16)
-        _ = nonce.withUnsafeMutableBytes { SecRandomCopyBytes(kSecRandomDefault, 16, $0.baseAddress!) }
+        let status = nonce.withUnsafeMutableBytes { SecRandomCopyBytes(kSecRandomDefault, 16, $0.baseAddress!) }
+        guard status == errSecSuccess else { return nil }
         let nonceB64 = nonce.base64EncodedString().replacingOccurrences(of: "+", with: "-").replacingOccurrences(of: "/", with: "_").replacingOccurrences(of: "=", with: "")
         let payload = VerificationQR(v: 1, noiseKeyHex: noiseKey, signKeyHex: signKey, npub: npub, nickname: nickname, ts: ts, nonceB64: nonceB64, sigHex: "")
         let msg = payload.canonicalBytes()

bitchat/ViewModels/ChatVerificationCoordinator.swift

@@ -294,7 +294,8 @@ final class ChatVerificationCoordinator {
         }
 
         var nonce = Data(count: 16)
-        _ = nonce.withUnsafeMutableBytes { SecRandomCopyBytes(kSecRandomDefault, 16, $0.baseAddress!) }
+        let status = nonce.withUnsafeMutableBytes { SecRandomCopyBytes(kSecRandomDefault, 16, $0.baseAddress!) }
+        guard status == errSecSuccess else { return false }
         var pending = PendingVerification(
             noiseKeyHex: qr.noiseKeyHex,
             signKeyHex: qr.signKeyHex,

bitchatTests/EndToEnd/PrivateChatE2ETests.swift

@@ -115,13 +115,18 @@ struct PrivateChatE2ETests {
         
         let aliceManager = NoiseSessionManager(localStaticKey: aliceKey, keychain: mockKeychain)
         let bobManager = NoiseSessionManager(localStaticKey: bobKey, keychain: mockKeychain)
-        
+
+        // Manager sessions are keyed by key-derived wire IDs: handshake
+        // completion fails closed on IDs the remote key can't vouch for.
+        let aliceNoiseID = PeerID(publicKey: aliceKey.publicKey.rawRepresentation)
+        let bobNoiseID = PeerID(publicKey: bobKey.publicKey.rawRepresentation)
+
         // Establish encrypted session
         do {
-            let handshake1 = try aliceManager.initiateHandshake(with: bob.peerID)
-            let handshake2 = try bobManager.handleIncomingHandshake(from: alice.peerID, message: handshake1)!
-            let handshake3 = try aliceManager.handleIncomingHandshake(from: bob.peerID, message: handshake2)!
-            _ = try bobManager.handleIncomingHandshake(from: alice.peerID, message: handshake3)
+            let handshake1 = try aliceManager.initiateHandshake(with: bobNoiseID)
+            let handshake2 = try bobManager.handleIncomingHandshake(from: aliceNoiseID, message: handshake1)!
+            let handshake3 = try aliceManager.handleIncomingHandshake(from: bobNoiseID, message: handshake2)!
+            _ = try bobManager.handleIncomingHandshake(from: aliceNoiseID, message: handshake3)
         } catch {
             Issue.record("Failed to establish Noise session: \(error)")
         }
@@ -134,7 +139,7 @@ struct PrivateChatE2ETests {
                    let message = BitchatMessage(packet.payload),
                    message.isPrivate {
                     do {
-                        let encrypted = try aliceManager.encrypt(packet.payload, for: bob.peerID)
+                        let encrypted = try aliceManager.encrypt(packet.payload, for: bobNoiseID)
                         let encryptedPacket = BitchatPacket(
                             type: 0x02, // Encrypted message type
                             senderID: packet.senderID,
@@ -155,7 +160,7 @@ struct PrivateChatE2ETests {
                 // Decrypt incoming encrypted messages
                 if packet.type == 0x02 {
                     do {
-                        let decrypted = try bobManager.decrypt(packet.payload, from: alice.peerID)
+                        let decrypted = try bobManager.decrypt(packet.payload, from: aliceNoiseID)
                         if let message = BitchatMessage(decrypted) {
                             #expect(message.content == TestConstants.testMessage1)
                             #expect(message.isPrivate)

bitchatTests/Integration/IntegrationTests.swift

@@ -18,10 +18,10 @@ struct IntegrationTests {
     private var helper = TestNetworkHelper()
     
     init() {
-        helper.createNode("Alice", peerID: PeerID(str: UUID().uuidString))
-        helper.createNode("Bob", peerID: PeerID(str: UUID().uuidString))
-        helper.createNode("Charlie", peerID: PeerID(str: UUID().uuidString))
-        helper.createNode("David", peerID: PeerID(str: UUID().uuidString))
+        helper.createNode("Alice")
+        helper.createNode("Bob")
+        helper.createNode("Charlie")
+        helper.createNode("David")
     }
     
     // MARK: - Multi-Peer Scenarios
@@ -266,9 +266,11 @@ struct IntegrationTests {
             helper.nodes["Alice"]!.sendPrivateMessage("Before restart", to: helper.nodes["Bob"]!.peerID, recipientNickname: "Bob")
         }
         
-        // Simulate Bob restart by recreating his Noise manager
+        // Simulate Bob restart by recreating his Noise manager. A new static
+        // key means a new key-derived wire ID, just like production.
         let bobKey = Curve25519.KeyAgreement.PrivateKey()
         helper.noiseManagers["Bob"] = NoiseSessionManager(localStaticKey: bobKey, keychain: helper.mockKeychain)
+        helper.nodes["Bob"]!.myPeerID = PeerID(publicKey: bobKey.publicKey.rawRepresentation)
         
         // Re-establish Noise handshake explicitly via managers
         do {
@@ -320,7 +322,7 @@ struct IntegrationTests {
     @Test func largeScaleNetwork() async throws {
         // Create larger network
         for i in 5...10 {
-            helper.createNode("Node\(i)", peerID: PeerID(str: "PEER\(i)"))
+            helper.createNode("Node\(i)")
         }
         
         // Connect in ring topology with cross-connections

bitchatTests/Integration/LargeTopologyTests.swift

@@ -37,7 +37,7 @@ struct LargeTopologyTests {
 
     private func makeNodes(_ names: [String]) {
         for name in names {
-            helper.createNode(name, peerID: PeerID(str: UUID().uuidString))
+            helper.createNode(name)
         }
     }
 

bitchatTests/Integration/TestNetworkHelper.swift

@@ -22,15 +22,17 @@ final class TestNetworkHelper {
     // MARK: - Node/Manager management
     
     @discardableResult
-    func createNode(_ name: String, peerID: PeerID) -> MockBLEService {
+    func createNode(_ name: String) -> MockBLEService {
         let node = MockBLEService(bus: bus)
-        node.myPeerID = peerID
+        // Wire IDs must derive from the node's Noise static key: handshake
+        // completion fails closed on IDs the remote key can't vouch for.
+        let key = Curve25519.KeyAgreement.PrivateKey()
+        node.myPeerID = PeerID(publicKey: key.publicKey.rawRepresentation)
         node.mockNickname = name
         nodes[name] = node
-        
+
         // This synchronous helper directly drives all three XX messages and
         // has no transport callback loop for delayed collision recovery.
-        let key = Curve25519.KeyAgreement.PrivateKey()
         noiseManagers[name] = NoiseSessionManager(
             localStaticKey: key,
             keychain: mockKeychain,

bitchatTests/Noise/NoiseCoverageTests.swift

@@ -578,6 +578,26 @@ struct NoiseCoverageTests {
         #expect(failingManager.getSession(for: charliePeerID) == nil)
     }
 
+    @Test("Handshake completion fails closed on non-wire peer IDs")
+    func handshakeCompletionRejectsNonWirePeerIDs() throws {
+        let aliceManager = NoiseSessionManager(localStaticKey: aliceStaticKey, keychain: keychain)
+        let bobManager = NoiseSessionManager(localStaticKey: bobStaticKey, keychain: keychain)
+
+        // Alice addresses Bob by an identifier no static key can vouch for:
+        // neither a 16-hex wire ID nor a full Noise-key ID. Completion must
+        // reject it rather than accept any remote static key.
+        let nonWireID = PeerID(str: "not-a-wire-identifier")
+        let msg1 = try aliceManager.initiateHandshake(with: nonWireID)
+        let msg2 = try #require(
+            try bobManager.handleIncomingHandshake(from: bobPeerID, message: msg1)
+        )
+
+        #expect(throws: (any Error).self) {
+            try aliceManager.handleIncomingHandshake(from: nonWireID, message: msg2)
+        }
+        #expect(aliceManager.getSession(for: nonWireID)?.isEstablished() != true)
+    }
+
     @Test("Session manager cleans up initiator sessions after start-handshake failures")
     func sessionManagerCleansUpInitiatorSessionsAfterStartHandshakeFailures() {
         let manager = NoiseSessionManager(

bitchatTests/Noise/NoiseProtocolTests.swift

@@ -68,22 +68,29 @@ struct NoiseProtocolTests {
     private let bobKey = Curve25519.KeyAgreement.PrivateKey()
     private let mockKeychain = MockKeychain()
     
-    private let alicePeerID = PeerID(str: UUID().uuidString)
-    private let bobPeerID = PeerID(str: UUID().uuidString)
-    
+    // Manager sessions are keyed by the remote peer. Keep the historical
+    // names, but derive each wire ID from the static key that the
+    // corresponding manager authenticates during the handshake.
+    private var alicePeerID: PeerID {
+        PeerID(publicKey: bobKey.publicKey.rawRepresentation)
+    }
+    private var bobPeerID: PeerID {
+        PeerID(publicKey: aliceKey.publicKey.rawRepresentation)
+    }
+
     private let aliceSession: NoiseSession
     private let bobSession: NoiseSession
-    
+
     init() {
         aliceSession = NoiseSession(
-            peerID: alicePeerID,
+            peerID: PeerID(publicKey: bobKey.publicKey.rawRepresentation),
             role: .initiator,
             keychain: mockKeychain,
             localStaticKey: aliceKey
         )
-        
+
         bobSession = NoiseSession(
-            peerID: bobPeerID,
+            peerID: PeerID(publicKey: aliceKey.publicKey.rawRepresentation),
             role: .responder,
             keychain: mockKeychain,
             localStaticKey: bobKey