From 8378ff949a5ec707e7e2f2eea69192f3a7e7c892 Mon Sep 17 00:00:00 2001 From: jack Date: Fri, 12 Jun 2026 14:18:44 +0200 Subject: [PATCH] Address Codex review: verify public messages against registry signing key The public-message signature check fell back to signedSenderDisplayName, which only searches the asynchronously-persisted identity cache. Because the peer registry is updated synchronously on a verified announce, a message arriving immediately after that announce could have a valid signature and a verified registry entry yet still be dropped (cache not caught up). Verify the packet signature against the signing key already present in the synchronously-updated peer registry first; fall back to the persisted-identity lookup only for peers not yet in the registry. The security property is unchanged: a spoofed senderID claiming a registry peer still fails registry verification and the persisted fallback, and is dropped. Adds tests for the race (delivered via registry key before cache persists) and the spoof case (invalid signature falls back and drops). Co-Authored-By: Claude Fable 5 --- .../BLE/BLEPublicMessageHandler.swift | 16 ++++- bitchat/Services/BLE/BLEService.swift | 3 + .../BLEPublicMessageHandlerTests.swift | 68 ++++++++++++++++++- 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/bitchat/Services/BLE/BLEPublicMessageHandler.swift b/bitchat/Services/BLE/BLEPublicMessageHandler.swift index 73e8f4a9..49699c81 100644 --- a/bitchat/Services/BLE/BLEPublicMessageHandler.swift +++ b/bitchat/Services/BLE/BLEPublicMessageHandler.swift @@ -16,6 +16,8 @@ struct BLEPublicMessageHandlerEnvironment { let now: () -> Date /// Snapshot of known peers keyed by ID (registry read). let peersSnapshot: () -> [PeerID: BLEPeerInfo] + /// Verifies a packet's signature against a known signing public key. + let verifyPacketSignature: (_ packet: BitchatPacket, _ signingPublicKey: Data) -> Bool /// Resolves a display name from a verified packet signature for peers missing from the registry. let signedSenderDisplayName: (_ packet: BitchatPacket, _ peerID: PeerID) -> String? /// Tracks the broadcast message packet for gossip sync. @@ -74,9 +76,19 @@ final class BLEPublicMessageHandler { // 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). + // + // Verify against the signing key already in the (synchronously-updated) + // peer registry first: identity-cache persistence is asynchronous, so a + // message arriving right after a verified announce would otherwise be + // dropped because `signedSenderDisplayName` only searches the persisted + // cache. Fall back to that persisted-identity lookup for peers not (yet) + // in the registry. let isSelf = peerID == env.localPeerID() - let signedDisplayName = isSelf ? nil : env.signedSenderDisplayName(packet, peerID) - guard isSelf || signedDisplayName != nil else { + let registrySigningKey = peersSnapshot[peerID]?.signingPublicKey + let verifiedViaRegistry = !isSelf + && (registrySigningKey.map { env.verifyPacketSignature(packet, $0) } ?? false) + let signedDisplayName = (isSelf || verifiedViaRegistry) ? nil : env.signedSenderDisplayName(packet, peerID) + guard isSelf || verifiedViaRegistry || signedDisplayName != nil else { SecureLogger.warning("🚫 Dropping public message with missing/invalid signature for claimed sender \(peerID.id.prefix(8))…", category: .security) return } diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index 121bb38f..b1fe7897 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/bitchat/Services/BLE/BLEService.swift @@ -3124,6 +3124,9 @@ extension BLEService { guard let self = self else { return [:] } return self.collectionsQueue.sync { self.peerRegistry.snapshotByID } }, + verifyPacketSignature: { [weak self] packet, signingPublicKey in + self?.noiseService.verifyPacketSignature(packet, publicKey: signingPublicKey) ?? false + }, signedSenderDisplayName: { [weak self] packet, peerID in self?.signedSenderDisplayName(for: packet, from: peerID) }, diff --git a/bitchatTests/Services/BLEPublicMessageHandlerTests.swift b/bitchatTests/Services/BLEPublicMessageHandlerTests.swift index 24e174ab..056d3ca9 100644 --- a/bitchatTests/Services/BLEPublicMessageHandlerTests.swift +++ b/bitchatTests/Services/BLEPublicMessageHandlerTests.swift @@ -8,10 +8,12 @@ struct BLEPublicMessageHandlerTests { var localNickname = "Me" var peers: [PeerID: BLEPeerInfo] = [:] var signedName: String? + var verifyPacketSignatureResult = false var linkState: (hasPeripheral: Bool, hasCentral: Bool) = (false, false) var selfBroadcastMessageID: String? var peersSnapshotReads = 0 + var verifyPacketSignatureQueries: [PeerID] = [] var signedNameQueries: [PeerID] = [] var trackedPackets: [BitchatPacket] = [] var selfBroadcastTakes: [BitchatPacket] = [] @@ -35,6 +37,10 @@ struct BLEPublicMessageHandlerTests { recorder.peersSnapshotReads += 1 return recorder.peers }, + verifyPacketSignature: { packet, _ in + recorder.verifyPacketSignatureQueries.append(PeerID(hexData: packet.senderID)) + return recorder.verifyPacketSignatureResult + }, signedSenderDisplayName: { _, peerID in recorder.signedNameQueries.append(peerID) return recorder.signedName @@ -137,6 +143,63 @@ struct BLEPublicMessageHandlerTests { #expect(recorder.deliveries.isEmpty) } + @Test + func registryVerifiedPeerDeliveredBeforeIdentityCachePersists() { + // A freshly verified announce updates the peer registry synchronously, + // but identity-cache persistence is async. A message arriving in that + // window has a valid signature and a registry signing key, yet the + // persisted-identity lookup (signedName) would still return nil. It must + // be verified against the registry key and delivered, not dropped. + let now = Date(timeIntervalSince1970: 1_000) + let recorder = Recorder() + recorder.peers = [remotePeerID: makePeerInfo( + remotePeerID, + nickname: "Alice", + isVerified: true, + signingPublicKey: Data(repeating: 0xAB, count: 32) + )] + recorder.verifyPacketSignatureResult = true + recorder.signedName = nil + let handler = makeHandler(recorder: recorder, now: now) + let packet = makeMessagePacket(sender: remotePeerID, content: "first msg", timestamp: timestamp(now)) + + handler.handle(packet, from: remotePeerID) + + #expect(recorder.verifyPacketSignatureQueries == [remotePeerID]) + // Verified via the registry key, so no fallback to the persisted lookup. + #expect(recorder.signedNameQueries.isEmpty) + #expect(recorder.trackedPackets.count == 1) + #expect(recorder.deliveries.count == 1) + #expect(recorder.deliveries.first?.nickname == "Alice") + #expect(recorder.deliveries.first?.content == "first msg") + } + + @Test + func registryPeerWithInvalidSignatureFallsBackAndDrops() { + // Spoofed senderID: the peer is in the registry with a signing key, but + // the packet signature does not verify against it. The handler must fall + // back to the persisted lookup and, finding nothing, drop the message. + let now = Date(timeIntervalSince1970: 1_000) + let recorder = Recorder() + recorder.peers = [remotePeerID: makePeerInfo( + remotePeerID, + nickname: "Alice", + isVerified: true, + signingPublicKey: Data(repeating: 0xAB, count: 32) + )] + recorder.verifyPacketSignatureResult = false + recorder.signedName = nil + let handler = makeHandler(recorder: recorder, now: now) + let packet = makeMessagePacket(sender: remotePeerID, content: "spoofed", timestamp: timestamp(now)) + + handler.handle(packet, from: remotePeerID) + + #expect(recorder.verifyPacketSignatureQueries == [remotePeerID]) + #expect(recorder.signedNameQueries == [remotePeerID]) + #expect(recorder.trackedPackets.isEmpty) + #expect(recorder.deliveries.isEmpty) + } + @Test func signedSenderFallbackDeliversWithSignedName() { let now = Date(timeIntervalSince1970: 1_000) @@ -219,14 +282,15 @@ struct BLEPublicMessageHandlerTests { _ peerID: PeerID, nickname: String, isVerified: Bool, - isConnected: Bool = true + isConnected: Bool = true, + signingPublicKey: Data? = nil ) -> BLEPeerInfo { BLEPeerInfo( peerID: peerID, nickname: nickname, isConnected: isConnected, noisePublicKey: nil, - signingPublicKey: nil, + signingPublicKey: signingPublicKey, isVerifiedNickname: isVerified, lastSeen: Date(timeIntervalSince1970: 999) )