From 132120a88eb044a8ee6adafa33d03b159242131a Mon Sep 17 00:00:00 2001 From: jack <212554440+jackjackbits@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:01:38 +0200 Subject: [PATCH 1/3] Close the three holes the #1486 review found (#1488) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix the three follow-ups from the #1486 review Three defects shipped with the censorship-resilience merge, all confirmed against main: 1. Source-manifest verification silently accepted added files. shasum -c checks only the files the manifest lists, and the Xcode project compiles every source file present in the tree — so a hostile mirror could pass verification by adding a file rather than modifying one. The manifest header and VERIFYING-A-BUILD.md now require the completeness check (git status --porcelain, or a path diff for tarballs) alongside the hash check. 2. A relay removed while Tor was bootstrapping reconnected anyway. dropRelays never subtracted from pendingTorConnectionURLs, and a custom relay passes the allow-list filter, so draining the pending queue resurrected a relay someone had explicitly deleted. 3. Turning Tor off mid-bootstrap read as 'network may be blocking tor'. shutdownCompletely left the detached 75s poll loop running, which then stamped bootstrapDidStall over the clean shutdown state; and the stall handler guarded on torEnforced, which is compile-time true in release, instead of the runtime preference. The poll loop is now generation-fenced (shutdown, dormancy, and restart each invalidate it) and the handler consults persistedTorPreference(). Both app-side fixes carry regression tests proven to fail pre-fix. Co-Authored-By: Claude Fable 5 * Address Codex review: ignored files and manifest placement git status --porcelain omits ignored paths, and .gitignore covers build/ — a planted bitchat/build/Evil.swift would compile via the synchronized group while the documented check stayed silent. The checkout check now uses --ignored. The downloaded manifest also has to live outside the tree, or it trips the completeness checks itself; the doc now says so and references it at /tmp throughout. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: jack Co-authored-by: Claude Fable 5 --- .github/workflows/source-manifest.yml | 8 +++- bitchat/Nostr/NostrRelayManager.swift | 3 ++ .../Extensions/ChatViewModel+Tor.swift | 4 ++ .../ChatViewModelExtensionsTests.swift | 34 ++++++++++++++ .../Services/NostrRelayManagerTests.swift | 40 +++++++++++++++++ docs/VERIFYING-A-BUILD.md | 25 +++++++++-- localPackages/Arti/Sources/TorManager.swift | 44 +++++++++++-------- 7 files changed, 135 insertions(+), 23 deletions(-) diff --git a/.github/workflows/source-manifest.yml b/.github/workflows/source-manifest.yml index 6f8a0c60..e05ae34f 100644 --- a/.github/workflows/source-manifest.yml +++ b/.github/workflows/source-manifest.yml @@ -63,7 +63,13 @@ jobs: echo "#" echo "# Verify a checkout of this ref with:" echo "# shasum -a 256 -c files.sha256" - echo "# The git tree hash above is the single value that covers all of it:" + echo "# Hash checking alone ignores files this manifest does not list, and" + echo "# the Xcode project compiles any source file present in the tree. So" + echo "# also confirm nothing extra is present:" + echo "# git status --porcelain --ignored # git checkout: must print nothing" + echo "# or, for a tarball, diff this manifest's path list against find(1)." + echo "# Full instructions: docs/VERIFYING-A-BUILD.md" + echo "# The git tree hash above is the single value covering all tracked content:" echo "# git rev-parse HEAD^{tree}" echo "#" } > SOURCE-MANIFEST.txt diff --git a/bitchat/Nostr/NostrRelayManager.swift b/bitchat/Nostr/NostrRelayManager.swift index 5873247e..fca24465 100644 --- a/bitchat/Nostr/NostrRelayManager.swift +++ b/bitchat/Nostr/NostrRelayManager.swift @@ -848,6 +848,9 @@ final class NostrRelayManager: ObservableObject { } } messageQueueLock.unlock() + // A relay queued while Tor bootstraps would otherwise reconnect when + // the queue drains, overriding the explicit removal. + pendingTorConnectionURLs.subtract(urls) relays.removeAll { urls.contains($0.url) } updateConnectionStatus() } diff --git a/bitchat/ViewModels/Extensions/ChatViewModel+Tor.swift b/bitchat/ViewModels/Extensions/ChatViewModel+Tor.swift index 52eaca7c..3bf7a5a1 100644 --- a/bitchat/ViewModels/Extensions/ChatViewModel+Tor.swift +++ b/bitchat/ViewModels/Extensions/ChatViewModel+Tor.swift @@ -64,6 +64,10 @@ extension ChatViewModel { @objc func handleTorBootstrapDidStall() { Task { @MainActor in guard TorManager.shared.torEnforced else { return } + // torEnforced is a compile-time constant in release builds; the + // runtime preference is what says whether anyone is waiting on + // Tor. Turning Tor off mid-bootstrap must not read as blocking. + guard NetworkActivationService.persistedTorPreference() else { return } guard !self.torStallAnnounced else { return } self.torStallAnnounced = true self.addGeohashOnlySystemMessage( diff --git a/bitchatTests/ChatViewModelExtensionsTests.swift b/bitchatTests/ChatViewModelExtensionsTests.swift index e4a0ab85..a1d12550 100644 --- a/bitchatTests/ChatViewModelExtensionsTests.swift +++ b/bitchatTests/ChatViewModelExtensionsTests.swift @@ -1374,3 +1374,37 @@ private func makeImageData() throws -> Data { return data #endif } + +// MARK: - Tor Extension Tests + +struct ChatViewModelTorExtensionTests { + + /// Turning Tor off mid-bootstrap must not read as "the network is + /// blocking tor": `torEnforced` is a compile-time constant, so the stall + /// handler has to consult the runtime preference before announcing. + @Test @MainActor + func bootstrapStall_withTorPreferenceOff_announcesNothing() async { + let key = NetworkActivationService.torPreferenceKey + let previous = UserDefaults.standard.object(forKey: key) + defer { + if let previous { + UserDefaults.standard.set(previous, forKey: key) + } else { + UserDefaults.standard.removeObject(forKey: key) + } + } + let (viewModel, _) = makeTestableViewModel() + + UserDefaults.standard.set(false, forKey: key) + viewModel.handleTorBootstrapDidStall() + try? await Task.sleep(nanoseconds: 50_000_000) + #expect(viewModel.torStallAnnounced == false) + + // The same stall with the preference on (the persisted default) is + // exactly what must still be announced. + UserDefaults.standard.set(true, forKey: key) + viewModel.handleTorBootstrapDidStall() + try? await Task.sleep(nanoseconds: 50_000_000) + #expect(viewModel.torStallAnnounced == true) + } +} diff --git a/bitchatTests/Services/NostrRelayManagerTests.swift b/bitchatTests/Services/NostrRelayManagerTests.swift index c50d1ba4..8e3849f0 100644 --- a/bitchatTests/Services/NostrRelayManagerTests.swift +++ b/bitchatTests/Services/NostrRelayManagerTests.swift @@ -44,6 +44,46 @@ final class NostrRelayManagerTests: XCTestCase { XCTAssertTrue(context.sessionFactory.allConnections.allSatisfy { $0.cancelCallCount >= 1 }) } + /// A relay removed while its connection is queued behind Tor bootstrap + /// must stay removed: draining the pending set used to resurrect it, + /// because `dropRelays` never touched `pendingTorConnectionURLs` and a + /// custom relay is in neither the default set nor the allow-list filter. + func test_relayRemovedWhileWaitingForTor_staysRemovedWhenTorBecomesReady() async { + let customURL = "wss://custom-removed.example" + let center = NotificationCenter() + let customRelays = MutableRelayList(urls: [customURL]) + let context = makeContext( + permission: .authorized, + userTorEnabled: true, + torEnforced: true, + torIsReady: false, + notificationCenter: center, + customRelays: customRelays + ) + + // Defaults plus the custom relay all queue while Tor bootstraps. + context.manager.connect() + XCTAssertTrue(context.sessionFactory.requestedURLs.isEmpty) + XCTAssertEqual(context.torWaiter.awaitCallCount, 1) + + // The relay is removed by hand before Tor is ready. + customRelays.urls = [] + center.post(name: NostrRelaySettings.didChangeNotification, object: nil) + // The settings sink hops through the main queue; let it land. + try? await Task.sleep(nanoseconds: 20_000_000) + + context.torWaiter.resolve(true) + + let defaultsConnected = await waitUntil { + context.sessionFactory.requestedURLs.count == self.expectedDefaultRelayCount + } + XCTAssertTrue(defaultsConnected) + XCTAssertFalse( + context.sessionFactory.requestedURLs.contains(customURL), + "a relay removed while Tor was bootstrapping must not reconnect when the pending queue drains" + ) + } + func test_connect_waitsForTorReadinessBeforeCreatingSessions() async { let context = makeContext(permission: .authorized, userTorEnabled: true, torEnforced: true, torIsReady: false) diff --git a/docs/VERIFYING-A-BUILD.md b/docs/VERIFYING-A-BUILD.md index 818c831b..23b5181b 100644 --- a/docs/VERIFYING-A-BUILD.md +++ b/docs/VERIFYING-A-BUILD.md @@ -18,26 +18,43 @@ In order of how much verification is possible: Every tagged release has a `SOURCE-MANIFEST.txt` produced by `.github/workflows/source-manifest.yml`. It records the tag, the commit, the git tree hash, and a SHA-256 for every tracked file. -Check a copy of the source against it: +Keep the downloaded manifest *outside* the source tree (say, `/tmp`) — a stray copy inside the checkout would itself trip the completeness checks below. Then check a copy of the source against it: ```sh -# From the root of the source you obtained -grep -v '^#' SOURCE-MANIFEST.txt > /tmp/files.sha256 +# From the root of the source you obtained, with the manifest at /tmp +grep -v '^#' /tmp/SOURCE-MANIFEST.txt > /tmp/files.sha256 shasum -a 256 -c /tmp/files.sha256 ``` Any `FAILED` line means that file differs from the released source. Investigate before building. +That check alone is not enough. `shasum -c` verifies the files the manifest lists and says nothing about files it does not list — and the Xcode project compiles every source file present in the tree automatically, so a hostile mirror can pass the hash check by leaving every listed file intact and *adding* one. Confirm nothing extra is present: + +```sh +# The manifest's path list must match the tree exactly — no missing files, no extras +grep -v '^#' /tmp/SOURCE-MANIFEST.txt | sed 's/^[0-9a-f]* //' | LC_ALL=C sort > /tmp/manifest-paths +find . -type f ! -path './.git/*' | sed 's|^\./||' | LC_ALL=C sort > /tmp/actual-paths +diff /tmp/manifest-paths /tmp/actual-paths # must print nothing +``` + +In a git checkout the same assurance is one command — it also catches extra files, because they show as untracked. `--ignored` matters: `.gitignore` covers paths like `build/`, plain `git status` would not report a planted file there, and Xcode compiles it all the same: + +```sh +git status --porcelain --ignored # must print nothing before you build +``` + The single value that covers the whole tree is the git tree hash in the manifest header: ```sh git rev-parse HEAD^{tree} # must equal the "tree:" line in the manifest ``` +Note the tree hash covers tracked content only; it does not see untracked files sitting in the working directory, which is why the emptiness checks above come first. + The manifest itself carries a provenance attestation tying it to the workflow run that produced it, so a manifest handed to you along with a mirror is checkable too: ```sh -gh attestation verify SOURCE-MANIFEST.txt --repo permissionlesstech/bitchat +gh attestation verify /tmp/SOURCE-MANIFEST.txt --repo permissionlesstech/bitchat ``` That last step is what makes this resistant to a hostile mirror. Without it, whoever gives you the source can give you a matching manifest. diff --git a/localPackages/Arti/Sources/TorManager.swift b/localPackages/Arti/Sources/TorManager.swift index 1bb42b63..1e9f2eb8 100644 --- a/localPackages/Arti/Sources/TorManager.swift +++ b/localPackages/Arti/Sources/TorManager.swift @@ -84,6 +84,10 @@ public final class TorManager: ObservableObject { private var shutdownsInFlight = 0 private var startPendingAfterShutdown = false private var bootstrapMonitorStarted = false + // Fences the detached poll loop: shutdown, dormancy, and restart each bump + // this, so a loop from a previous attempt cannot run out its deadline and + // report a stall over state that a newer lifecycle event already owns. + private var bootstrapGeneration = 0 private var pathMonitor: NWPathMonitor? private var isAppForeground: Bool = true private var lastRestartAt: Date? = nil @@ -268,24 +272,25 @@ public final class TorManager: ObservableObject { private func startBootstrapMonitor() { guard !bootstrapMonitorStarted else { return } bootstrapMonitorStarted = true + bootstrapGeneration += 1 + let generation = bootstrapGeneration Task.detached(priority: .utility) { [weak self] in - await self?.bootstrapPollLoop() + await self?.bootstrapPollLoop(generation: generation) } } - private func bootstrapPollLoop() async { + private func bootstrapPollLoop(generation: Int) async { let deadline = Date().addingTimeInterval(75) var didComplete = false while Date() < deadline { + guard generation == bootstrapGeneration else { return } let progress = Int(arti_bootstrap_progress()) let summary = getBootstrapSummary() - await MainActor.run { - self.bootstrapProgress = progress - self.bootstrapSummary = summary - if progress >= 100 { self.isStarting = false } - self.recomputeReady() - } + self.bootstrapProgress = progress + self.bootstrapSummary = summary + if progress >= 100 { self.isStarting = false } + self.recomputeReady() if progress >= 100 { didComplete = true @@ -296,17 +301,17 @@ public final class TorManager: ObservableObject { // Running out the deadline is a reportable outcome, not silence. The // loop previously just ended, leaving `isStarting` true forever, so a - // blocked network was indistinguishable from a slow one. + // blocked network was indistinguishable from a slow one. A deliberate + // shutdown mid-bootstrap is not a stall, hence the generation check. if !didComplete { - await MainActor.run { - self.isStarting = false - self.bootstrapDidStall = true - SecureLogger.warning( - "TorManager: bootstrap did not complete within its deadline (progress=\(self.bootstrapProgress)); network may be blocking Tor", - category: .session - ) - NotificationCenter.default.post(name: .TorBootstrapDidStall, object: nil) - } + guard generation == bootstrapGeneration else { return } + self.isStarting = false + self.bootstrapDidStall = true + SecureLogger.warning( + "TorManager: bootstrap did not complete within its deadline (progress=\(self.bootstrapProgress)); network may be blocking Tor", + category: .session + ) + NotificationCenter.default.post(name: .TorBootstrapDidStall, object: nil) } } @@ -352,6 +357,7 @@ public final class TorManager: ObservableObject { // Clear isStarting so foreground recovery can proceed if bootstrap was interrupted. SecureLogger.debug("TorManager: goDormantOnBackground() called", category: .session) Task { @MainActor in + self.bootstrapGeneration += 1 self.isReady = false self.socksReady = false self.isStarting = false @@ -361,6 +367,7 @@ public final class TorManager: ObservableObject { public func shutdownCompletely() { SecureLogger.debug("TorManager: shutdownCompletely() called", category: .session) startPendingAfterShutdown = false + bootstrapGeneration += 1 shutdownsInFlight += 1 Task.detached { [weak self] in guard let self = self else { return } @@ -398,6 +405,7 @@ public final class TorManager: ObservableObject { SecureLogger.debug("TorManager: restartArti() starting", category: .session) await MainActor.run { NotificationCenter.default.post(name: .TorWillRestart, object: nil) + self.bootstrapGeneration += 1 self.isReady = false self.socksReady = false self.bootstrapProgress = 0 From c671e3df6631365443fc30694102b00f60d1113e Mon Sep 17 00:00:00 2001 From: jack <212554440+jackjackbits@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:19:12 +0200 Subject: [PATCH 2/3] Keep the composer focused after sending with the return key (#1490) * Keep the composer focused after sending with the return key On iOS, return sends and then drops the keyboard, so every message in a back-and-forth costs an extra tap to reopen it. sendMessage() is the onSubmit handler; reasserting the FocusState there keeps the keyboard up between messages. macOS already refocuses on appear and is unaffected. Fixes #457 Co-Authored-By: Claude Fable 5 * Address Codex review: refocus only on return-key submission The reassert moves from sendMessage() into the TextField's onSubmit, so the send button no longer reopens a deliberately dismissed keyboard on iOS and never moves focus on macOS. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: jack Co-authored-by: Claude Fable 5 --- bitchat/Views/ContentComposerView.swift | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bitchat/Views/ContentComposerView.swift b/bitchat/Views/ContentComposerView.swift index 9913ce03..b5020980 100644 --- a/bitchat/Views/ContentComposerView.swift +++ b/bitchat/Views/ContentComposerView.swift @@ -73,7 +73,14 @@ struct ContentComposerView: View { .textInputAutocapitalization(.sentences) #endif .submitLabel(.send) - .onSubmit(onSendMessage) + .onSubmit { + onSendMessage() + // Only the return-key path: it steals focus on iOS, so + // every message would cost a tap to reopen the keyboard. + // The send button must not reopen a deliberately + // dismissed keyboard, so it stays out of this. + isTextFieldFocused.wrappedValue = true + } .padding(.vertical, theme.usesGlassChrome ? 8 : 4) .padding(.horizontal, 6) .themedInputBackground() From 14e7b428d9847fc3c11cf57f61207d9146c3b2d4 Mon Sep 17 00:00:00 2001 From: jack <212554440+jackjackbits@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:33:59 +0200 Subject: [PATCH 3/3] Add a real security policy (#1489) * Add a real security policy Two drive-by template PRs (#1118, #1482) tried to fill this gap with unedited boilerplate. This is the actual policy: private vulnerability reporting (now enabled on the repo) as the channel, honest expectations for a volunteer project, and a scope section that separates the properties the app promises from the documented design behaviors that keep getting reported as vulnerabilities. Closes #1081 Co-Authored-By: Claude Fable 5 * Address Codex review: name the Nostr envelope format precisely 'Gift-wrapped' reads as NIP-59, and the scope section is exactly where a researcher calibrates expectations. Say what it is: bitchat's own private-envelope scheme, explicitly not NIP-17/44/59. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: jack Co-authored-by: Claude Fable 5 --- SECURITY.md | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 SECURITY.md diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..c53c92c5 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,45 @@ +# Security policy + +bitchat is a security-focused messenger, and reports about its security are taken seriously. This page says how to report, what counts as a vulnerability here, and what to expect. + +## Reporting a vulnerability + +**Use GitHub's private vulnerability reporting:** [Report a vulnerability](https://github.com/permissionlesstech/bitchat/security/advisories/new) (Security tab → "Report a vulnerability"). + +Please do not open a public issue for anything that could put people at risk before a fix ships. bitchat is used by people in hostile network environments; a public proof-of-concept can be acted on faster than a patch can reach them. + +A useful report says what an attacker can do, against which build (App Store version or commit hash), and how to reproduce it. A failing test or a packet capture is worth more than speculation about impact. + +## What to expect + +This is a volunteer-maintained project. The aim is to acknowledge reports within a week and to move on confirmed vulnerabilities immediately — historically, confirmed protocol and key-handling issues have been fixed within days. You'll be kept in the loop in the advisory thread, and credited in the fix unless you'd rather not be. There is no bug bounty. + +## Supported versions + +Fixes ship to the latest App Store release and `main`. Older releases are not patched; the fix is to update. + +## Scope + +In scope — the properties the app promises: + +- Confidentiality and integrity of private messages and media (Noise sessions over BLE; over Nostr, bitchat's own ephemeral private-envelope format — a proprietary scheme, *not* NIP-17/NIP-44/NIP-59, see `WHITEPAPER.md`) +- Identity: key handling, verification, impersonation, session binding +- The panic wipe actually destroying what it claims to destroy +- Metadata exposure beyond what the documentation already discloses (see `PRIVACY_POLICY.md` and `docs/privacy-assessment.md`) +- Downgrade paths: anything that silently moves traffic from an encrypted path to a plaintext one +- Tor routing: anything that makes traffic bypass Tor while the Tor preference is on +- Supply-chain integrity of the source and its vendored binaries (see `docs/VERIFYING-A-BUILD.md`) + +Out of scope — documented design properties, not vulnerabilities: + +- Public visibility of mesh announces and geohash channels: broadcast content, nicknames, and public keys are public by design +- Bluetooth proximity being observable: anyone in radio range can tell a BLE device is present +- Mesh flooding/relay behavior inherent to a broadcast mesh (rate limits exist; the topology is what it is) +- Behavior of third-party Nostr relays +- Denial of service requiring physical proximity, and battery-drain attacks in general + +If you're unsure whether something is in scope, report it privately anyway — a false alarm costs a few minutes; a real issue reported publicly can cost much more. + +## Verifying what you're running + +If your concern is that the app or source you have has been tampered with, that has its own document: `docs/VERIFYING-A-BUILD.md`.