mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-26 09:25:20 +00:00
Close forged-directness link-rebind DoS on DM routing (#1421)
* Gate connected-link trust on a secure session; deny forged direct announces the connected shortcut
Residual gap after the rotation-heal containment (#1401): "verified
direct" announces prove the signature but not directness — TTL is
unsigned — so a malicious connected peer can replay a victim's fresh
announce with its TTL restored. When the victim has no live link, the
replayer's link rebinds to the victim's ID and reads as "connected",
and MessageRouter's connected fast-path then trusts it outright: every
DM stalls on a Noise handshake the replayer can never complete and is
silently lost while showing "sent".
Router-level trust gate (no wire change):
- Transport gains canDeliverSecurely(to:) — BLE answers with an
established Noise session; Nostr keeps its prompt-delivery predicate;
the protocol default forwards to canDeliverPromptly for transports
without a forgeable link layer.
- MessageRouter.sendPrivate only trusts a connected link outright when
it can deliver securely; otherwise it still sends (kicking the
handshake on a genuine link) but retains a copy and hands a sealed
copy to couriers, like the reachable path. flushOutbox gets the same
gate so a flush over an insecure link resends instead of dropping the
retained copy.
Presence hardening (defense in depth): a "direct" announce arriving on
a link already bound to a different peer no longer shortcuts the
claimed peer into "connected" — only real link state does. Genuine
first-contact and direct announces are unaffected; only the ambiguous
heal path loses the forgeable shortcut.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Harden the insecure-link outbox bound; document the second-replay presence gap and the courier metadata tradeoff
Review follow-ups on the canDeliverSecurely gate:
- flushOutbox no longer counts connected-but-insecure flushes toward the
maxSendAttempts drop: the message was actually transmitted over a live
link, so a peer whose Noise handshake stalls across reconnect flapping
must not burn through the cap and lose the store-and-forward copy the
gate exists to preserve. Retention stays bounded by the 24h outbox TTL
and the per-peer FIFO cap; acks still clear it. Attempt-counting stays
for reachable-only (heuristic) sends. Regression test: >8 connected-
insecure flushes keep the retained copy, drop callback never fires.
- Document the known second-replay presence gap: linkBoundToOtherPeer
reads the binding before rebindLinkAfterVerifiedDirectAnnounce steals
the link, so once a first replay has rebound a link to an absent
victim's ID, a second replay marks the victim connected. Presence
display only — DMs stay on the retain+courier path via the router
gate. Not closed at the announce layer because the post-rebind state
is indistinguishable from a legitimate rotation/reconnect heal (a
supported, field-verified flow). Covered by a two-announce test.
- Note the accepted courier-spray metadata tradeoff at the connected-
insecure send: nearby verified peers receive a sealed copy (they learn
a DM exists, never its content), cleared on ack.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Promote a healed rotation to connected; normalize the secure-delivery probe; retain Codable properties in Periphery
Codex review follow-ups on 5017c232:
- P1: a legitimate rotation announce necessarily arrives on a link still
bound to the OLD peer ID, so the linkBoundToOtherPeer denial stored the
new identity as disconnected — and the rebind only fixed link state,
never the registry. A healed rotation then read as disconnected until
the peer happened to announce again. rebindLinkAfterVerifiedDirect-
Announce now promotes the rebound identity to connected after the
containment checks pass (registry markConnected + topology/snapshot/
peer-list refresh; the .peerConnected UI event already fired from the
announce path). This consciously moves the forged-presence residue
from second-replay to first-successful-rebind — bounded by the rebind
containment (never steals a live identity, one rebind per link per
cooldown) and still display-only: DMs stay gated on canDeliverSecurely.
Comments and the pinned tests updated; new regression test covers
rotate-on-open-link -> rebind -> isPeerConnected(new) == true.
- P2: BLEService.canDeliverSecurely probed the Noise session with the
peer ID as given, but sessions are keyed by the short wire ID — a send
keyed by the full 64-hex Noise key (favorites resolution) misread an
established session as insecure and needlessly retained + couriered
every DM until ack. Normalize with toShort() like isPeerConnected.
Test drives a real XX handshake and asserts both ID forms pass.
- Periphery: the PrekeyBundleStore.StoredBundle.noiseKey "assign-only"
flake fired again and slipped past its baselined USR (read exists in
loadFromDisk; the indexer intermittently misses it). Replace the
baseline entry with retain_codable_properties: true — deterministic,
and a truly-dead Codable field is a persisted-format change anyway.
Local periphery scan --strict: no unused code detected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
jack
co-authored by
jack
Claude Opus 4.8
jack
Claude Opus 4.8
parent
c8479c15e3
commit
b7c6f42b3a
@@ -20,6 +20,12 @@ struct BLEAnnounceHandlerEnvironment {
|
||||
let verifySignature: (_ packet: BitchatPacket, _ signingPublicKey: Data) -> Bool
|
||||
/// Direct link state for the peer (BLE-queue read).
|
||||
let linkState: (PeerID) -> (hasPeripheral: Bool, hasCentral: Bool)
|
||||
/// Whether the link this packet arrived on is already bound to a
|
||||
/// different peer ID (ingress-registry + BLE-queue read). Directness
|
||||
/// rides on the unsigned TTL, so a replayed announce can look "direct"
|
||||
/// on the replayer's link; that link must not shortcut an absent peer
|
||||
/// into "connected".
|
||||
let linkBoundToOtherPeer: (_ packet: BitchatPacket, _ peerID: PeerID) -> Bool
|
||||
/// Runs the registry mutation phase under the collections barrier.
|
||||
let withRegistryBarrier: (() -> Void) -> Void
|
||||
/// Upserts the verified announce into the peer registry.
|
||||
@@ -135,6 +141,23 @@ final class BLEAnnounceHandler {
|
||||
var isReconnectedPeer = false
|
||||
let directLinkState = env.linkState(peerID)
|
||||
let isDirectAnnounce = packet.ttl == env.messageTTL
|
||||
// A "direct" announce arriving on a link that another peer already
|
||||
// owns is either a rotation heal or a replay with its TTL restored;
|
||||
// both are ambiguous, so only the rebind (which containment-checks
|
||||
// the claimed identity) may promote it — never this shortcut.
|
||||
//
|
||||
// Known limitation: denying the shortcut cannot prevent forged
|
||||
// presence outright. A rebind that passes the containment checks
|
||||
// promotes the claimed peer to connected — it must, or a legitimate
|
||||
// rotation on an open link would read as disconnected — so a replay
|
||||
// that wins the rebind (absent victim, cooldown clear) still forges
|
||||
// presence. That residue is presence display only: DMs stay gated on
|
||||
// canDeliverSecurely (no Noise session means retain + courier, see
|
||||
// MessageRouter.sendPrivate). What this check buys: the ambiguous
|
||||
// announce alone never flips presence — forging requires winning the
|
||||
// containment-checked rebind (never steals an identity that owns a
|
||||
// live link; at most one rebind per link per cooldown window).
|
||||
let linkBoundToOtherPeer = isDirectAnnounce && env.linkBoundToOtherPeer(packet, peerID)
|
||||
|
||||
env.withRegistryBarrier {
|
||||
let hasPeripheralConnection = directLinkState.hasPeripheral
|
||||
@@ -152,7 +175,7 @@ final class BLEAnnounceHandler {
|
||||
let update = env.upsertVerifiedAnnounce(
|
||||
peerID,
|
||||
announcement,
|
||||
isDirectAnnounce || hasPeripheralConnection || hasCentralSubscription,
|
||||
hasPeripheralConnection || hasCentralSubscription || (isDirectAnnounce && !linkBoundToOtherPeer),
|
||||
now
|
||||
)
|
||||
isNewPeer = update.isNewPeer
|
||||
|
||||
@@ -158,6 +158,16 @@ struct BLEPeerRegistry {
|
||||
peers[peerID] = info
|
||||
}
|
||||
|
||||
/// Flips an already-known peer to connected. Returns false when the peer
|
||||
/// is unknown or already connected (nothing changed).
|
||||
@discardableResult
|
||||
mutating func markConnected(_ peerID: PeerID) -> Bool {
|
||||
guard var info = peers[peerID], !info.isConnected else { return false }
|
||||
info.isConnected = true
|
||||
peers[peerID] = info
|
||||
return true
|
||||
}
|
||||
|
||||
mutating func updateLastSeen(_ peerID: PeerID, at date: Date) {
|
||||
guard var peer = peers[peerID] else { return }
|
||||
peer.lastSeen = date
|
||||
|
||||
@@ -674,6 +674,19 @@ final class BLEService: NSObject {
|
||||
}
|
||||
}
|
||||
|
||||
func canDeliverSecurely(to peerID: PeerID) -> Bool {
|
||||
// A live link binding alone is forgeable: the rotation heal rebinds a
|
||||
// link on a signature-verified "direct" announce, but directness rides
|
||||
// on the unsigned TTL, so a replayed announce can bind an absent
|
||||
// peer's ID to the replayer's link. An established Noise session
|
||||
// proves the other end of the link holds the peer's private key.
|
||||
//
|
||||
// Sessions are keyed by the short wire ID, so normalize like
|
||||
// isPeerConnected does — a send keyed by the full 64-hex Noise key
|
||||
// must not misread an established session as insecure.
|
||||
noiseService.hasEstablishedSession(with: peerID.toShort())
|
||||
}
|
||||
|
||||
func peerNickname(peerID: PeerID) -> String? {
|
||||
collectionsQueue.sync {
|
||||
peerRegistry.nickname(for: peerID, connectedOnly: true)
|
||||
@@ -2083,6 +2096,16 @@ extension BLEService {
|
||||
}
|
||||
}
|
||||
|
||||
/// Handshake plumbing for tests that need a real established Noise
|
||||
/// session (e.g. canDeliverSecurely) without Bluetooth in the loop.
|
||||
func _test_noiseInitiateHandshake(with peerID: PeerID) throws -> Data {
|
||||
try noiseService.initiateHandshake(with: peerID)
|
||||
}
|
||||
|
||||
func _test_noiseProcessHandshakeMessage(from peerID: PeerID, message: Data) throws -> Data? {
|
||||
try noiseService.processHandshakeMessage(from: peerID, message: message)
|
||||
}
|
||||
|
||||
static func _test_shouldRediscoverBitChatService(
|
||||
invalidatedServiceUUIDs: [CBUUID],
|
||||
cachedServiceUUIDs: [CBUUID]?
|
||||
@@ -4388,6 +4411,16 @@ extension BLEService {
|
||||
}
|
||||
SecureLogger.debug("🔄 Rebinding link after peer-ID rotation: \(previousPeerID.id.prefix(8))… → \(peerID.id.prefix(8))…", category: .session)
|
||||
self.refreshLocalTopology()
|
||||
// The announce that triggered this rebind was upserted as
|
||||
// disconnected: the registry ran while the link still belonged
|
||||
// to the previous ID (the ambiguous state BLEAnnounceHandler
|
||||
// denies the connected shortcut). The rebind has now
|
||||
// containment-checked the claim and the identity owns a live
|
||||
// link, so promote it — otherwise a healed rotation leaves a
|
||||
// live link that reads as disconnected until the next announce.
|
||||
self.messageQueue.async { [weak self] in
|
||||
self?.promoteReboundPeerToConnected(peerID)
|
||||
}
|
||||
// Retire the rotated-away ID only once its last link is gone; a
|
||||
// remaining stale link heals the same way or ages out.
|
||||
guard self.linkStateStore.links(to: previousPeerID).isEmpty else { return }
|
||||
@@ -4397,6 +4430,26 @@ extension BLEService {
|
||||
}
|
||||
}
|
||||
|
||||
/// After a successful verified rebind the new identity owns a live link,
|
||||
/// but its announce was stored disconnected (the link was still bound to
|
||||
/// the rotated-away ID when the registry upsert ran). Flip it to
|
||||
/// connected and republish so routing and the peer list see the healed
|
||||
/// link. The `.peerConnected` UI event already fired from the announce
|
||||
/// path (new/reconnected + direct), so only list state needs refreshing.
|
||||
private func promoteReboundPeerToConnected(_ peerID: PeerID) {
|
||||
let promoted = collectionsQueue.sync(flags: .barrier) {
|
||||
peerRegistry.markConnected(peerID)
|
||||
}
|
||||
guard promoted else { return }
|
||||
refreshLocalTopology()
|
||||
publishFullPeerData()
|
||||
notifyUI { [weak self] in
|
||||
guard let self else { return }
|
||||
let currentPeerIDs = self.collectionsQueue.sync { self.peerRegistry.peerIDs }
|
||||
self.deliverTransportEvent(.peerListUpdated(currentPeerIDs))
|
||||
}
|
||||
}
|
||||
|
||||
/// Rotation is an implicit leave of the old identity: drop it immediately
|
||||
/// instead of letting a ghost duplicate linger for the reachability
|
||||
/// retention window.
|
||||
@@ -4434,6 +4487,25 @@ extension BLEService {
|
||||
linkState: { [weak self] peerID in
|
||||
self?.linkState(for: peerID) ?? (hasPeripheral: false, hasCentral: false)
|
||||
},
|
||||
linkBoundToOtherPeer: { [weak self] packet, peerID in
|
||||
// Reads the CURRENT binding — i.e. the state before
|
||||
// rebindLinkAfterVerifiedDirectAnnounce (which runs after the
|
||||
// handler) may steal the link and promote the new owner to
|
||||
// connected. See the caller in BLEAnnounceHandler for why the
|
||||
// residual forged-presence window this leaves is accepted.
|
||||
guard let self else { return false }
|
||||
guard let link = (self.collectionsQueue.sync { self.ingressLinks.link(for: packet) }) else { return false }
|
||||
let boundPeerID: PeerID? = self.readLinkState { store in
|
||||
switch link {
|
||||
case .peripheral(let peripheralUUID):
|
||||
return store.peerID(forPeripheralID: peripheralUUID)
|
||||
case .central(let centralUUID):
|
||||
return store.peerID(forCentralUUID: centralUUID)
|
||||
}
|
||||
}
|
||||
guard let boundPeerID else { return false }
|
||||
return boundPeerID != peerID
|
||||
},
|
||||
withRegistryBarrier: { [weak self] body in
|
||||
self?.collectionsQueue.sync(flags: .barrier) { body() }
|
||||
},
|
||||
|
||||
@@ -165,14 +165,38 @@ final class MessageRouter {
|
||||
// MARK: - Message Sending
|
||||
|
||||
func sendPrivate(_ content: String, to peerID: PeerID, recipientNickname: String, messageID: String) {
|
||||
if let transport = connectedTransport(for: peerID) {
|
||||
// A live link is a strong delivery signal; trust it outright.
|
||||
if let transport = connectedTransport(for: peerID), transport.canDeliverSecurely(to: peerID) {
|
||||
// A live link that can complete an encrypted delivery is a
|
||||
// strong delivery signal; trust it outright.
|
||||
SecureLogger.debug("Routing PM via \(type(of: transport)) (connected) to \(peerID.id.prefix(8))… id=\(messageID.prefix(8))…", category: .session)
|
||||
transport.sendPrivateMessage(content, to: peerID, recipientNickname: recipientNickname, messageID: messageID)
|
||||
return
|
||||
}
|
||||
|
||||
let message = QueuedMessage(content: content, nickname: recipientNickname, messageID: messageID, timestamp: now(), sendAttempts: 1)
|
||||
if let transport = connectedTransport(for: peerID) {
|
||||
// "Connected" without an established secure session is forgeable:
|
||||
// link bindings heal on signature-verified "direct" announces, but
|
||||
// directness rides on the unsigned TTL, so a replayed announce can
|
||||
// bind an absent peer's ID to the replayer's link — where the send
|
||||
// stalls on a handshake the replayer can never complete. Send now
|
||||
// (a genuine link finishes the handshake and delivers), but retain
|
||||
// a copy and hand a sealed copy to couriers so nothing is silently
|
||||
// lost; receivers dedup resends by message ID.
|
||||
//
|
||||
// Deliberate metadata tradeoff: every pre-handshake first DM to a
|
||||
// connected peer hands nearby verified peers a sealed copy, so
|
||||
// they learn a DM to this recipient exists (never its content —
|
||||
// the envelope is opaque). Accepted for delivery robustness; the
|
||||
// deposit is cleared on ack. Don't "optimize" the courier call
|
||||
// away.
|
||||
SecureLogger.debug("Routing PM via \(type(of: transport)) (connected, no secure session) to \(peerID.id.prefix(8))… id=\(messageID.prefix(8))…", category: .session)
|
||||
transport.sendPrivateMessage(content, to: peerID, recipientNickname: recipientNickname, messageID: messageID)
|
||||
enqueue(message, for: peerID)
|
||||
attemptCourierDeposit(messageID: messageID, for: peerID)
|
||||
return
|
||||
}
|
||||
|
||||
if let transport = reachableTransport(for: peerID) {
|
||||
// Reachability without a connection is a freshness heuristic (e.g.
|
||||
// the mesh retention window), so the send can silently go nowhere.
|
||||
@@ -397,14 +421,31 @@ final class MessageRouter {
|
||||
continue
|
||||
}
|
||||
|
||||
if let transport = connectedTransport(for: peerID) {
|
||||
// Live link: send and stop retaining.
|
||||
if let transport = connectedTransport(for: peerID), transport.canDeliverSecurely(to: peerID) {
|
||||
// Live link with a secure session: send and stop retaining.
|
||||
SecureLogger.debug("Outbox -> \(type(of: transport)) (connected) for \(peerID.id.prefix(8))… id=\(message.messageID.prefix(8))…", category: .session)
|
||||
transport.sendPrivateMessage(message.content, to: peerID, recipientNickname: message.nickname, messageID: message.messageID)
|
||||
metrics?.record(.outboxResent)
|
||||
} else if let transport = connectedTransport(for: peerID) {
|
||||
// "Connected" without a secure session — possibly a stolen
|
||||
// binding from a replayed announce: send (a genuine link
|
||||
// finishes the handshake and delivers) but keep retaining
|
||||
// until an ack clears it. These flushes do NOT count toward
|
||||
// the attempt-cap drop: the message was transmitted over a
|
||||
// live link, so a peer whose handshake stalls across
|
||||
// reconnect flapping must not burn through the cap and lose
|
||||
// the store-and-forward copy this retention exists to
|
||||
// preserve. Retention stays bounded by the 24h outbox TTL
|
||||
// and the per-peer FIFO cap.
|
||||
SecureLogger.debug("Outbox -> \(type(of: transport)) (connected, no secure session) for \(peerID.id.prefix(8))… id=\(message.messageID.prefix(8))…", category: .session)
|
||||
transport.sendPrivateMessage(message.content, to: peerID, recipientNickname: message.nickname, messageID: message.messageID)
|
||||
metrics?.record(.outboxResent)
|
||||
remaining.append(message)
|
||||
} else if let transport = reachableTransport(for: peerID) {
|
||||
// Weak signal: send but keep retaining until an ack clears it,
|
||||
// bounded by attempt count for peers that never ack.
|
||||
// Reachability without a connection is a freshness heuristic,
|
||||
// so the send can silently go nowhere: send but keep retaining
|
||||
// until an ack clears it, bounded by attempt count for peers
|
||||
// that never ack.
|
||||
guard message.sendAttempts < Self.maxSendAttempts else {
|
||||
SecureLogger.warning("📤 Dropping unacked PM for \(peerID.id.prefix(8))… id=\(message.messageID.prefix(8))… after \(message.sendAttempts) attempts", category: .session)
|
||||
dropMessage(message.messageID, for: peerID)
|
||||
|
||||
@@ -230,7 +230,14 @@ final class NostrTransport: Transport, @unchecked Sendable {
|
||||
// instead of waiting for internet that may never come.
|
||||
isPeerReachable(peerID) && queue.sync { relaysConnected }
|
||||
}
|
||||
|
||||
|
||||
func canDeliverSecurely(to peerID: PeerID) -> Bool {
|
||||
// Nostr has no link bindings to forge; a known recipient key plus a
|
||||
// connected relay is the strongest delivery signal it has. The router
|
||||
// already retains + couriers for Nostr sends, so keep that behavior.
|
||||
canDeliverPromptly(to: peerID)
|
||||
}
|
||||
|
||||
func peerNickname(peerID: PeerID) -> String? { nil }
|
||||
func getPeerNicknames() -> [PeerID: String] { [:] }
|
||||
|
||||
|
||||
@@ -23,10 +23,11 @@ import Foundation
|
||||
final class PrekeyBundleStore {
|
||||
struct StoredBundle: Codable {
|
||||
// noiseKey is read in loadFromDisk (dictionary keying), but the
|
||||
// Periphery indexer intermittently misses that read and flakes CI
|
||||
// with "assign-only" — its USR is baselined (an in-source ignore
|
||||
// can't work: strict mode flags it as superfluous on the runs where
|
||||
// the indexer gets it right).
|
||||
// Periphery indexer intermittently misses that read and flaked CI
|
||||
// with "assign-only" — even past its baselined USR. Covered
|
||||
// deterministically by retain_codable_properties in .periphery.yml
|
||||
// (an in-source ignore can't work: strict mode flags it as
|
||||
// superfluous on the runs where the indexer gets it right).
|
||||
let noiseKey: Data
|
||||
var generatedAt: UInt64
|
||||
var prekeyIDs: [UInt32]
|
||||
|
||||
@@ -116,6 +116,14 @@ protocol Transport: AnyObject {
|
||||
/// npub as reachable even with no relay connection, where a send only
|
||||
/// joins a queue waiting for internet that may never come.
|
||||
func canDeliverPromptly(to peerID: PeerID) -> Bool
|
||||
/// Whether a send to this peer can complete an end-to-end encrypted
|
||||
/// delivery right now (e.g. an established Noise session). Distinct from
|
||||
/// connectivity: a "connected" link binding alone is forgeable — link
|
||||
/// bindings heal on signature-verified "direct" announces, but directness
|
||||
/// rides on the unsigned TTL, so a replayed announce can wear an absent
|
||||
/// peer's ID on the replayer's link. Routers must not trust a connected
|
||||
/// link outright without this.
|
||||
func canDeliverSecurely(to peerID: PeerID) -> Bool
|
||||
func peerNickname(peerID: PeerID) -> String?
|
||||
func getPeerNicknames() -> [PeerID: String]
|
||||
|
||||
@@ -247,6 +255,10 @@ extension Transport {
|
||||
// straight to the radio; queue-backed transports override this.
|
||||
func canDeliverPromptly(to peerID: PeerID) -> Bool { isPeerReachable(peerID) }
|
||||
|
||||
// Transports without a forgeable link-binding layer (everything but the
|
||||
// BLE mesh) have no stronger delivery signal than prompt delivery.
|
||||
func canDeliverSecurely(to peerID: PeerID) -> Bool { canDeliverPromptly(to: peerID) }
|
||||
|
||||
// Noise identity hooks default to inert for transports that do not carry
|
||||
// Noise sessions (e.g. NostrTransport).
|
||||
func noiseSessionPublicKeyData(for peerID: PeerID) -> Data? { nil }
|
||||
|
||||
Reference in New Issue
Block a user