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 <noreply@anthropic.com>
This commit is contained in:
jack
2026-06-12 14:18:44 +02:00
co-authored by Claude Fable 5
parent ca63893197
commit 8378ff949a
3 changed files with 83 additions and 4 deletions
@@ -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
}
+3
View File
@@ -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)
},
@@ -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)
)