mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-24 23:25:19 +00:00
Fix CI media-wipe race, per-link ping rate limiting, and /ping output routing
Three fixes for PR #1377 review: 1. CI flake (sendImage_privateChatProcessesAndTransfersImage): the panicClearAllData / clearCurrentPublicTimeline detached utility-priority tasks delete the real ~/Library/Application Support/files tree, which the test process shares. The wipe fires at a nondeterministic time and raced the sendImage test's JPEG in files/images/outgoing (write then re-read), so prepareImagePacket threw and the test timed out. Both wipes are now skipped under tests (existing TestEnvironment.isRunningTests pattern); this also stops test runs from deleting the developer's real media. 2. Codex P1: ping packets are unsigned, so keying the pong rate limiter on packet.senderID let one connected peer rotate forged sender IDs to bypass the 5-per-10s budget. The limiter now keys on the ingress link (the directly connected peer that delivered the packet); the pong still goes to the claimed sender. Regression test proves rotating senders over one link exhaust one budget (fails 10 vs 5 pongs on the old code). 3. Codex P2: /ping output arrived up to 10s later and was routed from selectedPrivateChatPeer at callback time, misrouting the result after a chat switch. The origin conversation is now captured when the command is issued (CommandOutputDestination) and deferred output is routed there: a DM result lands in the origin chat's history even if deselected, and a mesh-timeline result pins to #mesh instead of the active channel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -69,9 +69,11 @@ final class BLEService: NSObject {
|
||||
private let meshTopology = MeshTopologyTracker()
|
||||
|
||||
// Mesh diagnostics: outstanding /ping probes keyed by nonce, plus the
|
||||
// inbound per-peer ping budget so a directed unencrypted probe cannot be
|
||||
// turned into an amplification primitive. Both are owned by
|
||||
// collectionsQueue barriers like the other mutable collections.
|
||||
// inbound ping budget — keyed by the ingress link (the directly connected
|
||||
// peer that delivered the packet), since the unsigned claimed sender is
|
||||
// spoofable — so a directed unencrypted probe cannot be turned into an
|
||||
// amplification primitive. Both are owned by collectionsQueue barriers
|
||||
// like the other mutable collections.
|
||||
private struct PendingMeshPing {
|
||||
let peerID: PeerID
|
||||
let sentAt: Date
|
||||
@@ -80,7 +82,7 @@ final class BLEService: NSObject {
|
||||
}
|
||||
private var pendingMeshPings: [Data: PendingMeshPing] = [:]
|
||||
private var meshPingResponseLimiter = SyncResponseRateLimiter(
|
||||
maxResponses: TransportConfig.meshPingInboundMaxPerPeer,
|
||||
maxResponses: TransportConfig.meshPingInboundMaxPerLink,
|
||||
window: TransportConfig.meshPingInboundWindowSeconds
|
||||
)
|
||||
|
||||
@@ -2474,18 +2476,25 @@ extension BLEService {
|
||||
|
||||
/// Answers a ping addressed to us with a pong echoing its nonce; pings
|
||||
/// addressed elsewhere are left to the generic directed-relay path.
|
||||
private func handleMeshPing(_ packet: BitchatPacket, from peerID: PeerID) {
|
||||
///
|
||||
/// `linkPeerID` is the directly connected peer that delivered the packet
|
||||
/// (the ingress link), NOT the packet's claimed sender: pings are
|
||||
/// unsigned, so `packet.senderID` is attacker-controlled, and keying the
|
||||
/// response budget on it would let one connected peer rotate forged
|
||||
/// sender IDs to emit unbounded pongs. The budget is per physical link;
|
||||
/// the pong still goes to the claimed sender (that's the protocol).
|
||||
private func handleMeshPing(_ packet: BitchatPacket, fromLink linkPeerID: PeerID) {
|
||||
guard packet.recipientID == myPeerIDData else { return }
|
||||
guard let ping = MeshPingPayload.decode(packet.payload) else {
|
||||
SecureLogger.debug("⚠️ Malformed ping from \(peerID.id.prefix(8))…", category: .session)
|
||||
SecureLogger.debug("⚠️ Malformed ping via \(linkPeerID.id.prefix(8))…", category: .session)
|
||||
return
|
||||
}
|
||||
let allowed = collectionsQueue.sync(flags: .barrier) {
|
||||
meshPingResponseLimiter.shouldRespond(to: peerID, now: Date())
|
||||
meshPingResponseLimiter.shouldRespond(to: linkPeerID, now: Date())
|
||||
}
|
||||
guard allowed else {
|
||||
if logRateLimiter.shouldLog(key: "ping-limit:\(peerID.id)") {
|
||||
SecureLogger.warning("🚫 Rate-limiting pings from \(peerID.id.prefix(8))…", category: .security)
|
||||
if logRateLimiter.shouldLog(key: "ping-limit:\(linkPeerID.id)") {
|
||||
SecureLogger.warning("🚫 Rate-limiting pings via link \(linkPeerID.id.prefix(8))…", category: .security)
|
||||
}
|
||||
return
|
||||
}
|
||||
@@ -3357,7 +3366,10 @@ extension BLEService {
|
||||
handleCourierEnvelope(packet, from: peerID)
|
||||
|
||||
case .ping:
|
||||
handleMeshPing(packet, from: senderID)
|
||||
// Rate limiting must key on the ingress link (`peerID`), not the
|
||||
// packet-claimed sender: pings are unsigned, so `senderID` is
|
||||
// attacker-controlled and rotating it would reset the budget.
|
||||
handleMeshPing(packet, fromLink: peerID)
|
||||
|
||||
case .pong:
|
||||
handleMeshPong(packet, from: senderID)
|
||||
|
||||
@@ -22,6 +22,17 @@ struct CommandGeoParticipant {
|
||||
let displayName: String
|
||||
}
|
||||
|
||||
/// The conversation a command was typed into, captured when the command is
|
||||
/// issued so deferred output (e.g. an async /ping result, which can arrive
|
||||
/// many seconds later) lands there even if the user switches chats first.
|
||||
enum CommandOutputDestination: Equatable {
|
||||
/// The #mesh public timeline. Commands that defer output (/ping) are
|
||||
/// mesh-only, so a non-DM origin is always the mesh timeline.
|
||||
case meshTimeline
|
||||
/// The private chat that was open when the command was typed.
|
||||
case privateChat(PeerID)
|
||||
}
|
||||
|
||||
/// Protocol defining what CommandProcessor needs from its context.
|
||||
/// This breaks the circular dependency between CommandProcessor and ChatViewModel.
|
||||
@MainActor
|
||||
@@ -49,9 +60,13 @@ protocol CommandContextProvider: AnyObject {
|
||||
// MARK: - System Messages
|
||||
func addLocalPrivateSystemMessage(_ content: String, to peerID: PeerID)
|
||||
func addPublicSystemMessage(_ content: String)
|
||||
/// The conversation the user is typing into right now. Commands that
|
||||
/// finish asynchronously capture this BEFORE starting async work, so a
|
||||
/// chat switch cannot misroute their deferred output.
|
||||
func currentCommandDestination() -> CommandOutputDestination
|
||||
/// Routes deferred command output (e.g. an async /ping result) into the
|
||||
/// conversation where the user typed the command.
|
||||
func addCommandOutput(_ content: String)
|
||||
/// conversation captured when the command was issued.
|
||||
func addCommandOutput(_ content: String, to destination: CommandOutputDestination)
|
||||
|
||||
// MARK: - Favorites
|
||||
/// Toggles the favorite via the unified peer flow, which persists by the
|
||||
@@ -373,16 +388,20 @@ final class CommandProcessor {
|
||||
|
||||
let nickname = target.nickname
|
||||
let currentProvider = contextProvider
|
||||
// Capture the origin conversation now: the pong can arrive up to
|
||||
// meshPingTimeoutSeconds later, and reading the selected chat at
|
||||
// callback time would misroute the result after a chat switch.
|
||||
let destination = contextProvider?.currentCommandDestination() ?? .meshTimeline
|
||||
meshService?.sendMeshPing(to: target.peerID) { [weak currentProvider] result in
|
||||
let provider = currentProvider
|
||||
guard let result else {
|
||||
provider?.addCommandOutput("no reply from \(nickname)")
|
||||
provider?.addCommandOutput("no reply from \(nickname)", to: destination)
|
||||
return
|
||||
}
|
||||
let hopText: String = result.hops.map { hops in
|
||||
hops == 1 ? " · direct (1 hop)" : " · \(hops) hops"
|
||||
} ?? ""
|
||||
provider?.addCommandOutput("pong from \(nickname): \(result.rttMs) ms\(hopText)")
|
||||
provider?.addCommandOutput("pong from \(nickname): \(result.rttMs) ms\(hopText)", to: destination)
|
||||
}
|
||||
return .success(message: "pinging \(nickname)…")
|
||||
}
|
||||
|
||||
@@ -18,7 +18,7 @@ enum TransportConfig {
|
||||
|
||||
// Mesh diagnostics (/ping)
|
||||
static let meshPingTimeoutSeconds: TimeInterval = 10 // Give up on a probe after this window
|
||||
static let meshPingInboundMaxPerPeer: Int = 5 // Inbound ping budget per peer...
|
||||
static let meshPingInboundMaxPerLink: Int = 5 // Inbound ping budget per ingress link (claimed sender is spoofable)...
|
||||
static let meshPingInboundWindowSeconds: TimeInterval = 10 // ...per sliding window (anti-amplification)
|
||||
|
||||
// UI / Storage Caps
|
||||
|
||||
@@ -291,6 +291,11 @@ final class ChatPublicConversationCoordinator: PublicMessagePipelineDelegate {
|
||||
context.clearPublicConversation(ConversationID(channelID: context.activeChannel))
|
||||
|
||||
Task.detached(priority: .utility) {
|
||||
// Skipped under tests: the test process shares the user's real
|
||||
// ~/Library/Application Support/files tree, and this detached
|
||||
// wipe fires at a nondeterministic time — racing tests that
|
||||
// write media there (see the same guard in panicClearAllData).
|
||||
guard !TestEnvironment.isRunningTests else { return }
|
||||
do {
|
||||
let base = try FileManager.default.url(
|
||||
for: .applicationSupportDirectory,
|
||||
|
||||
@@ -1273,6 +1273,13 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele
|
||||
|
||||
// Delete ALL media files (incoming and outgoing) in background
|
||||
Task.detached(priority: .utility) {
|
||||
// Skipped under tests: the test process shares the user's real
|
||||
// ~/Library/Application Support/files tree, and this detached
|
||||
// utility-priority wipe fires at a nondeterministic time —
|
||||
// deleting media that concurrently running tests (e.g. the
|
||||
// sendImage flow) just wrote there, and the developer's real
|
||||
// app data with it.
|
||||
guard !TestEnvironment.isRunningTests else { return }
|
||||
do {
|
||||
let base = try FileManager.default.url(for: .applicationSupportDirectory, in: .userDomainMask, appropriateFor: nil, create: true)
|
||||
let filesDir = base.appendingPathComponent("files", isDirectory: true)
|
||||
@@ -1514,11 +1521,9 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele
|
||||
/// Command output belongs in the conversation where the user typed the
|
||||
/// command; the public timeline is invisible while a DM is open. The DM
|
||||
/// selection is read *after* processing so commands that switch chats
|
||||
/// (`/msg`) print into the conversation they just opened. Internal (not
|
||||
/// private) because CommandProcessor routes deferred output (async /ping
|
||||
/// results) through it via CommandContextProvider.
|
||||
/// (`/msg`) print into the conversation they just opened.
|
||||
@MainActor
|
||||
func addCommandOutput(_ content: String) {
|
||||
private func addCommandOutput(_ content: String) {
|
||||
if let peerID = selectedPrivateChatPeer {
|
||||
addLocalPrivateSystemMessage(content, to: peerID)
|
||||
} else {
|
||||
@@ -1526,6 +1531,33 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele
|
||||
}
|
||||
}
|
||||
|
||||
/// Origin conversation for deferred command output, captured when the
|
||||
/// command is issued (before any async work starts).
|
||||
@MainActor
|
||||
func currentCommandDestination() -> CommandOutputDestination {
|
||||
if let peerID = selectedPrivateChatPeer {
|
||||
return .privateChat(peerID)
|
||||
}
|
||||
// Deferring commands (/ping) are rejected in geohash channels, so a
|
||||
// non-DM origin is always the #mesh timeline.
|
||||
return .meshTimeline
|
||||
}
|
||||
|
||||
/// Routes deferred command output (async /ping results) into the
|
||||
/// conversation captured at issue time, immune to chat switches in the
|
||||
/// meantime. A DM result lands in the origin chat's history even if that
|
||||
/// chat is no longer selected (or was cleared — it then reappears as the
|
||||
/// first message when the chat is reopened).
|
||||
@MainActor
|
||||
func addCommandOutput(_ content: String, to destination: CommandOutputDestination) {
|
||||
switch destination {
|
||||
case .privateChat(let peerID):
|
||||
addLocalPrivateSystemMessage(content, to: peerID)
|
||||
case .meshTimeline:
|
||||
publicConversationCoordinator.addMeshOnlySystemMessage(content)
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Message Reception
|
||||
|
||||
@MainActor
|
||||
|
||||
@@ -216,6 +216,69 @@ struct BLEServiceCoreTests {
|
||||
cachedServiceUUIDs: [BLEService.serviceUUID, otherService]
|
||||
))
|
||||
}
|
||||
|
||||
/// Pings are unsigned, so their claimed sender is attacker-controlled.
|
||||
/// The pong budget must be keyed on the ingress link (the directly
|
||||
/// connected peer that delivered the packet): rotating forged sender IDs
|
||||
/// over one link exhausts one budget instead of resetting it, so a single
|
||||
/// malicious link cannot turn /ping into an amplification primitive.
|
||||
@Test
|
||||
func meshPingResponseBudget_isPerIngressLinkNotClaimedSender() async throws {
|
||||
let ble = makeService()
|
||||
let outbound = OutboundPacketTap()
|
||||
ble._test_onOutboundPacket = outbound.record
|
||||
|
||||
let link = PeerID(str: "1122334455667788")
|
||||
let budget = TransportConfig.meshPingInboundMaxPerLink
|
||||
let myRecipientData = try #require(Data(hexString: ble.myPeerID.id))
|
||||
|
||||
for i in 0..<(budget * 2) {
|
||||
// A fresh forged sender for every ping, all arriving on one link.
|
||||
let forgedSender = PeerID(str: String(format: "%016x", 0xA0_0000 + i))
|
||||
var nonce = Data(repeating: 0, count: MeshPingPayload.nonceLength)
|
||||
nonce[0] = UInt8(i)
|
||||
let payload = try #require(MeshPingPayload(nonce: nonce, originTTL: 7))
|
||||
let packet = BitchatPacket(
|
||||
type: MessageType.ping.rawValue,
|
||||
senderID: Data(hexString: forgedSender.id) ?? Data(),
|
||||
recipientID: myRecipientData,
|
||||
timestamp: UInt64(Date().timeIntervalSince1970 * 1000),
|
||||
payload: payload.encode(),
|
||||
signature: nil,
|
||||
ttl: 7
|
||||
)
|
||||
ble._test_handlePacket(packet, fromPeerID: link, preseedPeer: false)
|
||||
}
|
||||
|
||||
let reachedBudget = await TestHelpers.waitUntil(
|
||||
{ outbound.count(ofType: .pong) >= budget },
|
||||
timeout: TestConstants.defaultTimeout
|
||||
)
|
||||
#expect(reachedBudget)
|
||||
// Give any over-budget pong a chance to surface, then confirm the
|
||||
// rotated sender IDs never bought a sixth response.
|
||||
let exceededBudget = await TestHelpers.waitUntil(
|
||||
{ outbound.count(ofType: .pong) > budget },
|
||||
timeout: TestConstants.shortTimeout
|
||||
)
|
||||
#expect(!exceededBudget)
|
||||
#expect(outbound.count(ofType: .pong) == budget)
|
||||
}
|
||||
}
|
||||
|
||||
/// Thread-safe capture of packets leaving the service under test.
|
||||
private final class OutboundPacketTap {
|
||||
private let lock = NSLock()
|
||||
private var packets: [BitchatPacket] = []
|
||||
|
||||
func record(_ packet: BitchatPacket) {
|
||||
lock.lock(); packets.append(packet); lock.unlock()
|
||||
}
|
||||
|
||||
func count(ofType type: MessageType) -> Int {
|
||||
lock.lock(); defer { lock.unlock() }
|
||||
return packets.filter { $0.type == type.rawValue }.count
|
||||
}
|
||||
}
|
||||
|
||||
private func makeService() -> BLEService {
|
||||
|
||||
@@ -436,6 +436,7 @@ private final class MockCommandContextProvider: CommandContextProvider {
|
||||
private(set) var localPrivateSystemMessages: [(content: String, peerID: PeerID)] = []
|
||||
private(set) var publicSystemMessages: [String] = []
|
||||
private(set) var commandOutputs: [String] = []
|
||||
private(set) var commandOutputDestinations: [CommandOutputDestination] = []
|
||||
private(set) var toggledFavorites: [PeerID] = []
|
||||
private(set) var favoriteNotifications: [(peerID: PeerID, isFavorite: Bool)] = []
|
||||
|
||||
@@ -486,8 +487,16 @@ private final class MockCommandContextProvider: CommandContextProvider {
|
||||
publicSystemMessages.append(content)
|
||||
}
|
||||
|
||||
func addCommandOutput(_ content: String) {
|
||||
func currentCommandDestination() -> CommandOutputDestination {
|
||||
if let peerID = selectedPrivateChatPeer {
|
||||
return .privateChat(peerID)
|
||||
}
|
||||
return .meshTimeline
|
||||
}
|
||||
|
||||
func addCommandOutput(_ content: String, to destination: CommandOutputDestination) {
|
||||
commandOutputs.append(content)
|
||||
commandOutputDestinations.append(destination)
|
||||
}
|
||||
|
||||
func toggleFavorite(peerID: PeerID) {
|
||||
|
||||
@@ -129,6 +129,44 @@ struct MeshDiagnosticsTests {
|
||||
#expect(context.commandOutputs == ["no reply from alice"])
|
||||
}
|
||||
|
||||
@MainActor
|
||||
@Test func pingOutputRoutesToConversationWhereCommandWasIssued() async {
|
||||
let context = DiagnosticsMockContext()
|
||||
let alice = PeerID(str: "abcd1234abcd1234")
|
||||
let bob = PeerID(str: "b0b0b0b0b0b0b0b0")
|
||||
context.nicknameToPeerID["alice"] = alice
|
||||
let transport = MockTransport()
|
||||
transport.meshPingResult = MeshPingResult(rttMs: 42, hops: 2)
|
||||
let processor = makeProcessor(context: context, transport: transport)
|
||||
|
||||
// Issue the ping from bob's DM, then switch chats before the async
|
||||
// result lands. The output must follow the origin conversation, not
|
||||
// whatever is selected at callback time.
|
||||
context.selectedPrivateChatPeer = bob
|
||||
_ = processor.process("/ping @alice")
|
||||
context.selectedPrivateChatPeer = nil
|
||||
|
||||
await waitForCommandOutput(context)
|
||||
#expect(context.commandOutputDestinations == [.privateChat(bob)])
|
||||
}
|
||||
|
||||
@MainActor
|
||||
@Test func pingIssuedFromPublicTimelineRoutesToMeshTimeline() async {
|
||||
let context = DiagnosticsMockContext()
|
||||
let alice = PeerID(str: "abcd1234abcd1234")
|
||||
context.nicknameToPeerID["alice"] = alice
|
||||
let transport = MockTransport()
|
||||
transport.meshPingResult = MeshPingResult(rttMs: 7, hops: 1)
|
||||
let processor = makeProcessor(context: context, transport: transport)
|
||||
|
||||
_ = processor.process("/ping @alice")
|
||||
// Opening a DM afterwards must not swallow the public-timeline result.
|
||||
context.selectedPrivateChatPeer = alice
|
||||
|
||||
await waitForCommandOutput(context)
|
||||
#expect(context.commandOutputDestinations == [.meshTimeline])
|
||||
}
|
||||
|
||||
// MARK: - /trace
|
||||
|
||||
@MainActor
|
||||
@@ -225,6 +263,7 @@ private final class DiagnosticsMockContext: CommandContextProvider {
|
||||
|
||||
var nicknameToPeerID: [String: PeerID] = [:]
|
||||
private(set) var commandOutputs: [String] = []
|
||||
private(set) var commandOutputDestinations: [CommandOutputDestination] = []
|
||||
|
||||
func getPeerIDForNickname(_ nickname: String) -> PeerID? {
|
||||
nicknameToPeerID[nickname]
|
||||
@@ -241,7 +280,15 @@ private final class DiagnosticsMockContext: CommandContextProvider {
|
||||
func addPublicSystemMessage(_ content: String) {}
|
||||
func toggleFavorite(peerID: PeerID) {}
|
||||
|
||||
func addCommandOutput(_ content: String) {
|
||||
func currentCommandDestination() -> CommandOutputDestination {
|
||||
if let peerID = selectedPrivateChatPeer {
|
||||
return .privateChat(peerID)
|
||||
}
|
||||
return .meshTimeline
|
||||
}
|
||||
|
||||
func addCommandOutput(_ content: String, to destination: CommandOutputDestination) {
|
||||
commandOutputs.append(content)
|
||||
commandOutputDestinations.append(destination)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user