Fail closed on unverified egress and gate the already-ready connect path

Address two review findings on the Tor egress self-check: 1. Bypass on the already-bootstrapped path: shouldWaitForTorBeforeConnecting only checked torIsReady, so once Tor was up, connectToRelays/connectToRelay (initial connect, reconnect backoff timers, manual retry, subscription- triggered connects) opened relay WebSockets without ever running the egress canary. The gate now also requires a fresh cached egress verification (TorManager.isEgressVerified, backed by a nonisolated TTL snapshot on TorEgressVerifier); any path lacking one queues behind awaitEgressReady(). 2. unreachable canary no longer allows: an unreachable canary is exactly the ambiguous case where we cannot tell whether the platform honored the SOCKS proxy, so it now refuses relay connections (fail-closed) instead of allow-with-warning. A verifiedTor verdict still allows connection opens for its 300s TTL (a cached good verdict covers brief canary blips); after TTL expiry or invalidate() (Tor restart/dormant/shutdown) connections stay closed until a probe succeeds again. Probe cadence stays bounded via the verifier's minRetryInterval, and the relay manager schedules a bounded 30s-cadence gate retry after wait-attempt exhaustion so a transient canary outage recovers automatically (GeoRelayDirectory already retries with its own backoff). Tests: verifier policy (unreachable refuses, recovery via retry, cached- within-TTL allows during blip, TTL expiry + unreachable refuses, snapshot invalidation) and relay-manager gating (ready path awaits egress verification, reconnect requeues behind the gate, exhaustion schedules the bounded retry and recovers). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-25 08:45:19 +00:00 · 2026-07-01 23:57:29 +02:00
parent c7ee2a4cb4
commit ad5fb1ddc6
6 changed files with 335 additions and 35 deletions
@@ -83,36 +83,119 @@ struct TorEgressVerifierTests {
        #expect(await v.verify() == true)
    }

-    @Test("unreachable allows (session stays fail-closed) but does not cache a Tor verification")
-    func unreachableAllowsButNotCached() async {
+    @Test("unreachable refuses: an unverified egress must not proceed (fail-closed)")
+    func unreachableRefuses() async {
        let h = Harness()
        h.setResult(.unreachable("down"))
        let v = makeVerifier(h, ttl: 300, minRetry: 0)

-        #expect(await v.verify() == true)
-        // Not a verified state: once it turns into a real leak, we must refuse.
-        h.setResult(.notTor)
        #expect(await v.verify() == false)
+        #expect(v.hasFreshVerification == false)
    }

-    @Test("minRetryInterval throttles re-probing when not verified")
+    @Test("unreachable-then-reachable recovers via retry")
+    func unreachableRecoversWhenCanaryReturns() async {
+        let h = Harness()
+        h.setResult(.unreachable("down"))
+        let v = makeVerifier(h, ttl: 300, minRetry: 5)
+
+        #expect(await v.verify() == false)
+        #expect(h.probeCount == 1)
+
+        // Canary comes back; the next probe (after the retry throttle window)
+        // verifies and allows again.
+        h.setResult(.verifiedTor)
+        h.advance(5)
+        #expect(await v.verify() == true)
+        #expect(h.probeCount == 2)
+        #expect(v.hasFreshVerification == true)
+    }
+
+    @Test("cached verifiedTor within TTL allows during a canary blip without re-probing")
+    func cachedVerifiedAllowsDuringCanaryBlip() async {
+        let h = Harness()
+        h.setResult(.verifiedTor)
+        let v = makeVerifier(h, ttl: 300, minRetry: 0)
+
+        #expect(await v.verify() == true)
+        #expect(h.probeCount == 1)
+
+        // The canary goes down inside the TTL window: the cached positive
+        // verdict is authoritative, no probe runs, traffic stays allowed.
+        h.setResult(.unreachable("down"))
+        h.advance(100)
+        #expect(await v.verify() == true)
+        #expect(h.probeCount == 1)
+        #expect(v.hasFreshVerification == true)
+    }
+
+    @Test("TTL expiry + unreachable refuses until a probe succeeds again")
+    func expiredCacheWithUnreachableRefuses() async {
+        let h = Harness()
+        h.setResult(.verifiedTor)
+        let v = makeVerifier(h, ttl: 300, minRetry: 0)
+
+        #expect(await v.verify() == true)
+
+        // Past the TTL the old verdict no longer stands: an unreachable canary
+        // means unverified egress, so connection opens are refused.
+        h.setResult(.unreachable("down"))
+        h.advance(301)
+        #expect(v.hasFreshVerification == false)
+        #expect(await v.verify() == false)
+        #expect(h.probeCount == 2)
+
+        // A subsequent successful probe restores service.
+        h.setResult(.verifiedTor)
+        #expect(await v.verify() == true)
+        #expect(v.hasFreshVerification == true)
+    }
+
+    @Test("minRetryInterval bounds re-probing while unverified (no canary hammering)")
    func throttleReprobe() async {
        let h = Harness()
        h.setResult(.unreachable("down"))
        let v = makeVerifier(h, ttl: 300, minRetry: 5)

-        #expect(await v.verify() == true)
+        #expect(await v.verify() == false)
        #expect(h.probeCount == 1)
-        // Within minRetry window: reuse last decision, no new probe.
+        // Within minRetry window: reuse last (refusing) decision, no new probe.
        h.advance(1)
-        #expect(await v.verify() == true)
+        #expect(await v.verify() == false)
        #expect(h.probeCount == 1)
        // After the window: re-probe.
        h.advance(5)
-        #expect(await v.verify() == true)
+        #expect(await v.verify() == false)
        #expect(h.probeCount == 2)
    }

+    @Test("invalidate clears the synchronous cache snapshot")
+    func invalidateClearsSnapshot() async {
+        let h = Harness()
+        h.setResult(.verifiedTor)
+        let v = makeVerifier(h, ttl: 300)
+
+        #expect(await v.verify() == true)
+        #expect(v.hasFreshVerification == true)
+        await v.invalidate()
+        #expect(v.hasFreshVerification == false)
+    }
+
+    @Test("notTor drops any cached verification snapshot")
+    func notTorClearsSnapshot() async {
+        let h = Harness()
+        h.setResult(.verifiedTor)
+        let v = makeVerifier(h, ttl: 300, minRetry: 0)
+
+        #expect(await v.verify() == true)
+        #expect(v.hasFreshVerification == true)
+
+        h.setResult(.notTor)
+        h.advance(301)
+        #expect(await v.verify() == false)
+        #expect(v.hasFreshVerification == false)
+    }
+
    @Test("invalidate forces a fresh probe")
    func invalidateForcesReprobe() async {
        let h = Harness()
@@ -111,6 +111,116 @@ final class NostrRelayManagerTests: XCTestCase {
        XCTAssertTrue(connected)
    }

+    func test_connect_whenTorAlreadyReady_waitsForEgressVerificationBeforeCreatingSessions() async {
+        // Tor is bootstrapped, but the egress self-check has no fresh verdict:
+        // the ready path must still queue behind the async egress gate instead
+        // of opening sockets directly.
+        let context = makeContext(
+            permission: .authorized,
+            userTorEnabled: true,
+            torEnforced: true,
+            torIsReady: true,
+            torEgressVerified: false
+        )
+
+        context.manager.connect()
+
+        XCTAssertTrue(context.sessionFactory.requestedURLs.isEmpty)
+        XCTAssertEqual(context.torWaiter.awaitCallCount, 1)
+
+        // Egress verification succeeds (awaitEgressReady returned true, which
+        // implies the verifier now holds a fresh cached verdict).
+        context.torEgressVerified.value = true
+        context.torWaiter.resolve(true)
+
+        let connectedAfterVerification = await waitUntil {
+            context.sessionFactory.requestedURLs.count == self.expectedDefaultRelayCount &&
+            context.manager.relays.allSatisfy(\.isConnected)
+        }
+        XCTAssertTrue(connectedAfterVerification)
+    }
+
+    func test_reconnect_requeuesBehindEgressGateWhenVerificationLapses() async {
+        // Reconnect backoff timers call connectToRelay directly; when the
+        // cached egress verification has lapsed by then, the reconnect must go
+        // back through the gate rather than opening a socket.
+        let relayURL = "wss://egress-reconnect.example"
+        let context = makeContext(
+            permission: .denied,
+            userTorEnabled: true,
+            torEnforced: true,
+            torIsReady: true,
+            torEgressVerified: true
+        )
+
+        context.manager.ensureConnections(to: [relayURL])
+        let connected = await waitUntil {
+            context.manager.relays.first(where: { $0.url == relayURL })?.isConnected == true
+        }
+        XCTAssertTrue(connected)
+        XCTAssertEqual(context.sessionFactory.requestedURLs.count, 1)
+
+        // The socket drops and the cached verification expires meanwhile.
+        context.torEgressVerified.value = false
+        context.sessionFactory.latestConnection(for: relayURL)?
+            .fail(error: NSError(domain: NSURLErrorDomain, code: NSURLErrorTimedOut))
+        let reconnectScheduled = await waitUntil { context.scheduler.scheduled.count == 1 }
+        XCTAssertTrue(reconnectScheduled)
+
+        context.scheduler.runNext()
+        let queuedBehindGate = await waitUntil { context.torWaiter.awaitCallCount == 1 }
+        XCTAssertTrue(queuedBehindGate)
+        // No new socket until the egress gate passes.
+        XCTAssertEqual(context.sessionFactory.requestedURLs.count, 1)
+
+        context.torEgressVerified.value = true
+        context.torWaiter.resolve(true)
+
+        let reconnected = await waitUntil {
+            context.sessionFactory.requestedURLs.count == 2 &&
+            context.manager.relays.first(where: { $0.url == relayURL })?.isConnected == true
+        }
+        XCTAssertTrue(reconnected)
+    }
+
+    func test_connect_egressGateExhaustionSchedulesBoundedRetryAndRecovers() async {
+        // A persistent unverified egress exhausts the wait attempts; a bounded
+        // low-frequency retry must then recover automatically once
+        // verification succeeds (e.g. transient canary outage ends).
+        let relayURL = "wss://egress-gate-retry.example"
+        let context = makeContext(
+            permission: .denied,
+            userTorEnabled: true,
+            torEnforced: true,
+            torIsReady: true,
+            torEgressVerified: false
+        )
+
+        context.manager.ensureConnections(to: [relayURL])
+        for _ in 0..<TransportConfig.nostrTorReadyMaxWaitAttempts {
+            context.torWaiter.resolve(false)
+        }
+
+        // Fail-closed, with a single bounded-cadence retry scheduled.
+        XCTAssertTrue(context.sessionFactory.requestedURLs.isEmpty)
+        XCTAssertEqual(context.scheduler.scheduled.count, 1)
+        XCTAssertEqual(context.scheduler.scheduled.first?.delay, TransportConfig.nostrTorGateRetrySeconds)
+
+        // The outage ends before the retry fires.
+        let attemptsBefore = context.torWaiter.awaitCallCount
+        context.scheduler.runNext()
+        let regated = await waitUntil { context.torWaiter.awaitCallCount == attemptsBefore + 1 }
+        XCTAssertTrue(regated)
+
+        context.torEgressVerified.value = true
+        context.torWaiter.resolve(true)
+
+        let recovered = await waitUntil {
+            context.manager.relays.first(where: { $0.url == relayURL })?.isConnected == true
+        }
+        XCTAssertTrue(recovered)
+    }
+
    func test_subscribe_unblocksDeferredEOSEWhenTorWaitAttemptsExhausted() async {
        let relayURL = "wss://tor-eose-unblock.example"
        let context = makeContext(permission: .denied, userTorEnabled: true, torEnforced: true, torIsReady: false)
@@ -1449,6 +1559,7 @@ final class NostrRelayManagerTests: XCTestCase {
        userTorEnabled: Bool = false,
        torEnforced: Bool = false,
        torIsReady: Bool = true,
+        torEgressVerified: Bool = true,
        torIsForeground: Bool = true,
        jitterUnit: @escaping () -> Double = { 0.5 } // 0.5 -> jitter factor 1.0 (no jitter)
    ) -> RelayManagerTestContext {
@@ -1458,6 +1569,7 @@ final class NostrRelayManagerTests: XCTestCase {
        let scheduler = MockRelayScheduler()
        let clock = MutableClock(now: Date(timeIntervalSince1970: 1_700_000_000))
        let torWaiter = MockTorWaiter(isReady: torIsReady)
+        let torEgressVerifiedFlag = MutableBool(value: torEgressVerified)
        let torForeground = MutableBool(value: torIsForeground)
        let activationFlag = MutableBool(value: activationAllowed)
        let manager = NostrRelayManager(
@@ -1470,6 +1582,7 @@ final class NostrRelayManagerTests: XCTestCase {
                locationPermissionPublisher: permissionSubject.eraseToAnyPublisher(),
                torEnforced: { torEnforced },
                torIsReady: { torWaiter.isReady },
+                torEgressVerified: { torEgressVerifiedFlag.value },
                torIsForeground: { torForeground.value },
                awaitTorReady: torWaiter.await(completion:),
                makeSession: { sessionFactory },
@@ -1489,6 +1602,7 @@ final class NostrRelayManagerTests: XCTestCase {
            clock: clock,
            activationAllowed: activationFlag,
            torWaiter: torWaiter,
+            torEgressVerified: torEgressVerifiedFlag,
            torForeground: torForeground
        )
    }
@@ -1543,6 +1657,7 @@ private struct RelayManagerTestContext {
    let clock: MutableClock
    let activationAllowed: MutableBool
    let torWaiter: MockTorWaiter
+    let torEgressVerified: MutableBool
    let torForeground: MutableBool
 }