From 6ed798b7a2abc60727b7935baac4d3fa7ab9ca9e Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 25 Aug 2026 05:22:37 -0700 Subject: [PATCH 1/3] test(iroh): failing tests for unbounded dials and zero-route refresh fallbacks Behavior the client transport must have but does not yet (red on this commit, fixed in the next): - CmxIrohClientSession: an admission barrier that never answers must fail at the dial bound and be superseded by the next attempt (cmux#9724 16.2s dial, cmux#8531 silent redial hang). - CmxIrohRegistryContextProvider: when the staleness-forced discovery refresh fails, dial with the last verified snapshot instead of refusing to dial. - CmxIrohRelayPolicyService.restore: a recently-expired last-good policy must keep its routes dialable instead of publishing a zero-route managed profile (cmux#10375). - CmxIrohRelayCredentialCoordinator.refreshIfNeeded: a failed mint with a last-good installed credential must not throw; the bounded retry loop continues in the background (cmux#10375). --- .../CmxIrohClientSessionDialBoundTests.swift | 157 ++++++++++++++++++ ...egistryContextProviderStalenessTests.swift | 34 ++++ ...xIrohRelayCredentialCoordinatorTests.swift | 35 ++++ .../CmxIrohRelayPolicyServiceTests.swift | 33 ++++ 4 files changed, 259 insertions(+) create mode 100644 Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohClientSessionDialBoundTests.swift diff --git a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohClientSessionDialBoundTests.swift b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohClientSessionDialBoundTests.swift new file mode 100644 index 000000000000..6718b1cd2c42 --- /dev/null +++ b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohClientSessionDialBoundTests.swift @@ -0,0 +1,157 @@ +import CMUXMobileCore +import Foundation +import Testing +@testable import CmuxIrohTransport + +/// Bounded-dial behavior (cmux#9724, cmux#8531): a dial attempt whose +/// admission barrier never answers must fail at the configured dial bound and +/// leave the session redialable, instead of holding the reconnect owner for +/// an unbounded time. A half-ready Mac accepts the QUIC connection but never +/// serves the admission frames; that exact shape produced the unbounded +/// 16.2-second dial in the cmux#9724 trace. +@Suite +struct CmxIrohClientSessionDialBoundTests { + let localIdentity: CmxIrohPeerIdentity + let remoteIdentity: CmxIrohPeerIdentity + let credential: CmxIrohAdmissionCredential + + init() throws { + localIdentity = try CmxIrohPeerIdentity( + endpointID: String(repeating: "ab", count: 32) + ) + remoteIdentity = try CmxIrohPeerIdentity( + endpointID: String(repeating: "cd", count: 32) + ) + credential = try .pairGrant("e30.e30.AA") + } + + @Test("an admission barrier that never answers fails at the dial bound") + func admissionBarrierThatNeverAnswersFailsAtTheDialBound() async throws { + let control = CmxIrohBidirectionalStream( + receiveStream: TestHangingIrohReceiveStream(), + sendStream: TestIrohSendStream() + ) + let connection = TestIrohConnection( + remoteIdentity: remoteIdentity, + bidirectionalStreams: [control] + ) + let endpoint = TestDialingIrohEndpoint( + localIdentity: localIdentity, + dialResults: [.connection(connection)] + ) + let session = try CmxIrohClientSession( + endpoint: endpoint, + targetIdentity: remoteIdentity, + dialPlan: try testIrohDialPlan(publicPaths: [try publicRelayHint()]), + credential: credential, + dialPhaseTimeout: .milliseconds(40) + ) + + let failure = await boundedConnectFailure(session, within: .seconds(2)) + #expect(failure as? CmxIrohClientSessionError == .dialTimedOut) + #expect(await connection.observedCloseCallCount() >= 1) + await session.close() + } + + @Test("a timed-out admission is superseded by the next connect attempt") + func timedOutAdmissionIsSupersededByTheNextConnectAttempt() async throws { + let hangingControl = CmxIrohBidirectionalStream( + receiveStream: TestHangingIrohReceiveStream(), + sendStream: TestIrohSendStream() + ) + let hangingConnection = TestIrohConnection( + remoteIdentity: remoteIdentity, + bidirectionalStreams: [hangingControl] + ) + let goodConnection = TestIrohConnection( + remoteIdentity: remoteIdentity, + bidirectionalStreams: [answeringControlStream()] + ) + let endpoint = TestDialingIrohEndpoint( + localIdentity: localIdentity, + dialResults: [ + .connection(hangingConnection), + .connection(goodConnection), + ] + ) + let session = try CmxIrohClientSession( + endpoint: endpoint, + targetIdentity: remoteIdentity, + dialPlan: try testIrohDialPlan(publicPaths: [try publicRelayHint()]), + credential: credential, + dialPhaseTimeout: .milliseconds(40) + ) + + let failure = await boundedConnectFailure(session, within: .seconds(2)) + #expect(failure as? CmxIrohClientSessionError == .dialTimedOut) + + // The timed-out attempt must have been retired cleanly: the very next + // attempt on the same session dials again and admits. + try await session.connect() + + #expect(await endpoint.observedDialedAddresses().count == 2) + #expect(await hangingConnection.observedCloseCallCount() >= 1) + await session.close() + } + + // MARK: - Support + + /// Runs `connect()` under a test watchdog so the red state (an unbounded + /// admission hang) fails this test quickly instead of hanging the suite. + private func boundedConnectFailure( + _ session: CmxIrohClientSession, + within limit: Duration + ) async -> (any Error)? { + let connectTask = Task { try await session.connect() } + let watchdog = Task { + try? await ContinuousClock().sleep(for: limit) + connectTask.cancel() + } + defer { watchdog.cancel() } + do { + _ = try await connectTask.value + return nil + } catch { + return error + } + } + + private func answeringControlStream() -> CmxIrohBidirectionalStream { + let codec = CmxIrohAdmissionAckCodec() + let accepted = codec.encodeFrame(.acceptedPendingNatTraversal) + let serverReady = codec.encodeFrame(.serverReady) + return CmxIrohBidirectionalStream( + receiveStream: TestIrohReceiveStream(buffer: accepted + serverReady), + sendStream: TestIrohSendStream() + ) + } + + private func publicRelayHint() throws -> CmxIrohPathHint { + try CmxIrohPathHint( + kind: .relayURL, + value: "https://use1-1.relay.lawrence.cmux.iroh.link/", + source: .native, + privacyScope: .publicInternet + ) + } +} + +/// A control stream that never yields admission bytes. The hang is +/// cancellable, mirroring the production stream contract the phase bound +/// relies on (`TestIrohDialResult.hang` models the same shape for dials). +actor TestHangingIrohReceiveStream: CmxIrohReceiveStream { + private var stoppedCodes: [UInt64] = [] + + func receive(maximumByteCount _: Int) async throws -> Data? { + try await Task.sleep(for: .seconds(3_600)) + return nil + } + + func stop(errorCode: UInt64) { + stoppedCodes.append(errorCode) + } + + func observedStoppedCodes() -> [UInt64] { + stoppedCodes + } +} diff --git a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRegistryContextProviderStalenessTests.swift b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRegistryContextProviderStalenessTests.swift index 9fa448ad5975..68e61dff9800 100644 --- a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRegistryContextProviderStalenessTests.swift +++ b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRegistryContextProviderStalenessTests.swift @@ -383,6 +383,40 @@ struct CmxIrohRegistryContextProviderStalenessTests { #expect(context.dialPlan.publicPaths == [relay]) } + @Test + func refreshFailureAfterStalenessFallsBackToLastVerifiedSnapshot() async throws { + let fixture = try RegistryFixture() + let relay = try managedRelayHint(fixture) + let broker = ConfigurableRegistryBroker( + discovery: try fixture.discovery(targetHints: [relay]), + pairGrantResponses: [try fixture.pairGrantResponse( + issuedAt: fixture.nowSeconds, + expiresAt: fixture.nowSeconds + 7 * 24 * 60 * 60 + )] + ) + let provider = try await makeProvider( + fixture: fixture, + broker: broker, + verifiedDiscovery: try fixture.discovery(targetHints: [relay]) + ) + // A timed-out dial marked the peer stale, so the next attempt must + // try one fresh fetch first. + await provider.noteDialFailure( + for: try fixture.request(hints: []), + dialPlan: try nonEmptyPlan(fixture, hints: [relay]), + failure: .timedOut + ) + await broker.setDiscoverError(CmxIrohTrustBrokerClientError.connectivity) + + // The forced refresh failed. Dialing with the last verified snapshot + // beats not dialing at all (cmux#9724): the staleness mark survives, + // so a later attempt still refetches once the broker recovers. + let context = try await provider.context(for: fixture.request(hints: [])) + + #expect(await broker.discoveryRequestCount() == 1) + #expect(context.dialPlan.publicPaths == [relay]) + } + @Test func cooldownDuringEmptyPlanRefetchKeepsResolvedContextInsteadOfSpinning() async throws { let fixture = try RegistryFixture() diff --git a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayCredentialCoordinatorTests.swift b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayCredentialCoordinatorTests.swift index c258d8206c01..e6ed9e03f3af 100644 --- a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayCredentialCoordinatorTests.swift +++ b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayCredentialCoordinatorTests.swift @@ -343,6 +343,41 @@ struct CmxIrohRelayCredentialCoordinatorTests { await coordinator.deactivate() } + @Test + func refreshFailureKeepsLastGoodCredentialWithoutThrowing() async throws { + let fixture = try RelayCoordinatorFixture() + let endpoint = TestIrohEndpoint(identity: fixture.identity) + let supervisor = try await fixture.activeSupervisor(endpoint: endpoint) + let broker = TestRelayTokenBroker(steps: [.failure]) + let clock = TestRelayClock(now: fixture.now) + let coordinator = CmxIrohRelayCredentialCoordinator( + supervisor: supervisor, + broker: broker, + managedRelayURLs: Set(fixture.relayURLs), + clock: clock, + jitter: { _, refreshAfter in refreshAfter }, + retryJitter: { 0 } + ) + try await coordinator.activate( + bindingID: fixture.bindingID, + endpointIdentity: fixture.identity, + bootstrap: try fixture.response() + ) + // The installed credential is due for refresh but far from expiry. + clock.setNowWithoutResuming(fixture.refreshAfter.addingTimeInterval(1)) + + // A transient mint failure must not fail the caller while the + // last-good credential is installed (cmux#10375). The bounded retry + // loop keeps refreshing in the background; only the relay itself can + // reject the installed credential. + try await coordinator.refreshIfNeeded() + + #expect(await coordinator.credentialExpiresAt() == fixture.expiresAt) + #expect(await endpoint.observedRelayUpdates().count == 1) + #expect(await broker.observedEndpointIDs() == [fixture.identity]) + await coordinator.deactivate() + } + @Test func rateLimitRetryNeverPrecedesValidatedServerFloor() async throws { let fixture = try RelayCoordinatorFixture() diff --git a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayPolicyServiceTests.swift b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayPolicyServiceTests.swift index 272faa3df11a..dadeed557f8c 100644 --- a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayPolicyServiceTests.swift +++ b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayPolicyServiceTests.swift @@ -335,6 +335,39 @@ struct CmxIrohRelayPolicyServiceTests { #expect(await stores.service.diagnosticsSnapshot().failure == .policyExpired) } + @Test + func restoreKeepsRecentlyExpiredLastGoodPolicyRoutesForDialing() async throws { + let fixture = RelayPolicyServiceTestFixture() + let stores = makeStores() + _ = try await stores.service.install( + response: CmxIrohRelayPolicyResponse( + policy: fixture.token(sequence: 1), + preference: .automatic, + preferenceRevision: 1 + ), + accountID: "account-a", + trustRoot: fixture.firstTrustRoot, + relayCredential: fixture.relayCredential(), + now: fixture.now + ) + + // The one-hour policy expired five minutes ago and the broker refresh + // failed (cmux#10375). Restoring must keep the last-good catalog + // dialable instead of publishing a zero-route managed profile; the + // relay itself remains the authority on credential validity. + let restored = await stores.service.restore( + accountID: "account-a", + trustRoot: try fixture.firstTrustRoot, + relayCredential: nil, + now: fixture.now.addingTimeInterval(3_600 + 300) + ) + + #expect(restored.source == .managed) + #expect(restored.usedCachedPolicy) + #expect(restored.endpointRelayProfile.allowedRelayURLs == Set(fixture.relayURLs)) + #expect(await stores.service.diagnosticsSnapshot().failure == .policyExpired) + } + @Test func implicitRevisionZeroStillRejectsEquivocation() async throws { let fixture = RelayPolicyServiceTestFixture() From 01c9410e49e5d01ef3d73f2dc709a4e5b033f5db Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:20:26 -0700 Subject: [PATCH 2/3] fix(iroh): bound the admission barrier, dial cached hints on refresh failure, fail open on credential refresh Three client-transport behavior changes for iOS dialing (cmux#9724, cmux#8531, cmux#10375): 1. Bounded dials. The admission barrier after QUIC connect (control stream open, admission frames, NAT-traversal authorize, server ready) was unbounded; a half-ready Mac that accepts the connection and never answers admission produced the 16.2s hang in the #9724 trace. The barrier now runs under the same bounded race as the connect phases and fails typed as dialTimedOut, so the redial machinery supersedes it. The per-phase deadline is injected end to end: CmxIrohClientRuntimeConfiguration.dialPhaseTimeout (default 5s) -> CmxConnectivityEngine -> every CmxIrohClientSession. 2. Hint refresh fallback. A failed or timed-out dial already marks the peer's discovery stale and forces a broker refetch on the next attempt. When that refetch itself fails, the provider now dials the last verified snapshot's hints instead of refusing to dial; the staleness mark survives so a later attempt still refetches. Broker cooldowns still propagate unchanged when no last-good snapshot exists. 3. Fail-open credential refresh. CmxIrohRelayPolicyService.restore grants a bounded expired-policy reuse grace (default 6h, injectable): the cache re-verifies the record at its final valid instant, so signature, rollback, and claim checks run unweakened and only the expiry gate is graced; the graced state reports .policyExpired without zeroing routes. Beyond the grace or on any verification rejection, restore still fails closed. CmxIrohRelayCredentialCoordinator.refreshIfNeeded no longer throws on a failed mint while a last-good credential is installed; the bounded backoff retry loop keeps refreshing in the background and the relay stays the authority on token validity. --- .../CmxConnectivityEngine.swift | 14 +- .../CmxIrohClientRuntime.swift | 3 +- .../CmxIrohClientRuntimeConfiguration.swift | 12 +- .../CmxIrohClientSession.swift | 123 +++++++++++------- .../CmxIrohRegistryContextProvider.swift | 22 ++++ .../CmxIrohRelayCredentialCoordinator.swift | 8 ++ .../CmxIrohRelayPolicyCache.swift | 35 ++++- .../CmxIrohRelayPolicyService.swift | 29 ++++- .../CmxIrohRelayPolicyServiceTests.swift | 70 +++++++++- 9 files changed, 255 insertions(+), 61 deletions(-) diff --git a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxConnectivityEngine.swift b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxConnectivityEngine.swift index 784a545bb1f1..5f7b27c70973 100644 --- a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxConnectivityEngine.swift +++ b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxConnectivityEngine.swift @@ -26,6 +26,9 @@ public actor CmxConnectivityEngine { private let installRouteSnapshot: RouteSnapshotInstaller? private let diagnosticLog: DiagnosticLog? private let clock: any CmxIrohRelayClock + /// Deadline for each dial phase (public paths, private fallback, and the + /// admission barrier) of every peer session this engine creates. + private let dialPhaseTimeout: Duration private var desiredActive = false private var lifecycleRevision: UInt64 = 0 private var endpointGeneration: UInt64? @@ -59,7 +62,8 @@ public actor CmxConnectivityEngine { authority: (any CmxConnectivityAuthorityServing)? = nil, installRouteSnapshot: RouteSnapshotInstaller? = nil, diagnosticLog: DiagnosticLog? = nil, - clock: any CmxIrohRelayClock = CmxIrohSystemRelayClock() + clock: any CmxIrohRelayClock = CmxIrohSystemRelayClock(), + dialPhaseTimeout: Duration = .seconds(5) ) { precondition((authority == nil) == (installRouteSnapshot == nil)) supervisor = CmxIrohEndpointSupervisor( @@ -72,6 +76,7 @@ public actor CmxConnectivityEngine { self.installRouteSnapshot = installRouteSnapshot self.diagnosticLog = diagnosticLog self.clock = clock + self.dialPhaseTimeout = dialPhaseTimeout } /// Creates a stopped endpoint-only engine for a host acceptor. @@ -90,6 +95,7 @@ public actor CmxConnectivityEngine { installRouteSnapshot = nil diagnosticLog = nil clock = CmxIrohSystemRelayClock() + dialPhaseTimeout = .seconds(5) } init( @@ -99,7 +105,8 @@ public actor CmxConnectivityEngine { authority: (any CmxConnectivityAuthorityServing)? = nil, installRouteSnapshot: RouteSnapshotInstaller? = nil, diagnosticLog: DiagnosticLog? = nil, - clock: any CmxIrohRelayClock = CmxIrohSystemRelayClock() + clock: any CmxIrohRelayClock = CmxIrohSystemRelayClock(), + dialPhaseTimeout: Duration = .seconds(5) ) { precondition((authority == nil) == (installRouteSnapshot == nil)) self.supervisor = supervisor @@ -109,6 +116,7 @@ public actor CmxConnectivityEngine { self.installRouteSnapshot = installRouteSnapshot self.diagnosticLog = diagnosticLog self.clock = clock + self.dialPhaseTimeout = dialPhaseTimeout } /// Returns the current immutable UI-safe state. @@ -533,6 +541,7 @@ public actor CmxConnectivityEngine { let protocolConfiguration = protocolConfiguration let diagnosticLog = diagnosticLog let clock = clock + let dialPhaseTimeout = dialPhaseTimeout let peer = CmxConnectivityPeerSession( peerID: peerID, buildSession: { request in @@ -551,6 +560,7 @@ public actor CmxConnectivityEngine { basedOn: context ) }, + dialPhaseTimeout: dialPhaseTimeout, protocolConfiguration: protocolConfiguration, diagnostics: diagnosticLog ) diff --git a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntime.swift b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntime.swift index 9c3818e05d2b..444d10faecb8 100644 --- a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntime.swift +++ b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntime.swift @@ -172,7 +172,8 @@ public actor CmxIrohClientRuntime { supervisor: supervisor, contextProvider: contextRouter, protocolConfiguration: protocolConfiguration, - diagnosticLog: diagnosticLog + diagnosticLog: diagnosticLog, + dialPhaseTimeout: configuration.dialPhaseTimeout ) self.supervisor = supervisor self.connectivityEngine = connectivityEngine diff --git a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntimeConfiguration.swift b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntimeConfiguration.swift index 893a1262e811..fe4813303713 100644 --- a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntimeConfiguration.swift +++ b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntimeConfiguration.swift @@ -44,6 +44,12 @@ public struct CmxIrohClientRuntimeConfiguration: Equatable, Sendable { /// flight. A signed registration refresh follows after activation. public let cachedBinding: CmxIrohBrokerBindingMetadata? + /// Deadline for each dial phase (public paths, private fallback, and the + /// admission barrier) of every peer dial this runtime performs. A phase + /// that never answers fails typed at this bound and hands control back to + /// recovery instead of holding the reconnect owner (cmux#9724). + public let dialPhaseTimeout: Duration + /// Creates an immutable iOS client lifecycle configuration. /// /// Broker-facing validation occurs when ``CmxIrohClientRuntime/start()`` @@ -60,6 +66,8 @@ public struct CmxIrohClientRuntimeConfiguration: Equatable, Sendable { /// - endpointRelayProfile: An optional local selection or custom override. /// - cachedRelayCredential: A validated cached relay capability. /// - cachedBinding: A previously verified exact local binding tuple. + /// - dialPhaseTimeout: The per-phase dial deadline, injectable so tests + /// can shrink it. public init( accountID: String, deviceID: String, @@ -72,7 +80,8 @@ public struct CmxIrohClientRuntimeConfiguration: Equatable, Sendable { managedRelayURLs: Set, endpointRelayProfile: CmxIrohEndpointRelayProfile? = nil, cachedRelayCredential: CmxIrohRelayTokenResponse? = nil, - cachedBinding: CmxIrohBrokerBindingMetadata? = nil + cachedBinding: CmxIrohBrokerBindingMetadata? = nil, + dialPhaseTimeout: Duration = .seconds(5) ) { self.accountID = accountID self.deviceID = cmxCanonicalDeviceID(deviceID) @@ -86,5 +95,6 @@ public struct CmxIrohClientRuntimeConfiguration: Equatable, Sendable { self.endpointRelayProfile = endpointRelayProfile self.cachedRelayCredential = cachedRelayCredential self.cachedBinding = cachedBinding + self.dialPhaseTimeout = dialPhaseTimeout } } diff --git a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientSession.swift b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientSession.swift index e812caa454e6..9c73968bbe57 100644 --- a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientSession.swift +++ b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientSession.swift @@ -431,56 +431,71 @@ public actor CmxIrohClientSession { do { try Task.checkCancellation() - guard await establishedConnection.remoteIdentity() == targetIdentity else { - throw CmxIrohClientSessionError.remoteIdentityMismatch + // A half-ready peer can accept the QUIC connection and then never + // serve the admission frames. Bound the whole barrier like a dial + // phase so a silent peer hands control back to recovery instead + // of holding the redial owner open-endedly (cmux#9724). + return try await boundedByDialPhase { [weak self] in + guard let self else { throw CancellationError() } + return try await self.performAdmissionBarrier( + on: establishedConnection + ) + } + } catch { + await establishedConnection.close(errorCode: 1, reason: "admission_failed") + throw error + } + } + + private func performAdmissionBarrier( + on establishedConnection: any CmxIrohConnection + ) async throws -> CmxIrohConnectedControl { + guard await establishedConnection.remoteIdentity() == targetIdentity else { + throw CmxIrohClientSessionError.remoteIdentityMismatch + } + try await establishedConnection.setIncomingStreamLimits( + maximumBidirectionalStreamCount: 0, + maximumUnidirectionalStreamCount: 0 + ) + let stream = try await establishedConnection.openBidirectionalStream() + let header = try CmxIrohStreamHeader( + lane: .control, + credential: credential + ) + try await stream.sendStream.send(headerCodec.encode(header)) + let admission = try await readAdmissionFrame(from: stream.receiveStream) + switch admission.frame { + case .acceptedPendingNatTraversal, .acceptedRelayOnly: + if admission.frame == .acceptedPendingNatTraversal { + try Task.checkCancellation() + try await establishedConnection.authorizeNatTraversal() } - try await establishedConnection.setIncomingStreamLimits( - maximumBidirectionalStreamCount: 0, - maximumUnidirectionalStreamCount: 0 + try Task.checkCancellation() + try await stream.sendStream.send( + admissionCodec.encodeFrame(.clientReady) ) - let stream = try await establishedConnection.openBidirectionalStream() - let header = try CmxIrohStreamHeader( - lane: .control, - credential: credential + let confirmation = try await readAdmissionFrame( + from: stream.receiveStream, + initialBuffer: admission.trailingBytes ) - try await stream.sendStream.send(headerCodec.encode(header)) - let admission = try await readAdmissionFrame(from: stream.receiveStream) - switch admission.frame { - case .acceptedPendingNatTraversal, .acceptedRelayOnly: - if admission.frame == .acceptedPendingNatTraversal { - try Task.checkCancellation() - try await establishedConnection.authorizeNatTraversal() - } - try Task.checkCancellation() - try await stream.sendStream.send( - admissionCodec.encodeFrame(.clientReady) - ) - let confirmation = try await readAdmissionFrame( - from: stream.receiveStream, - initialBuffer: admission.trailingBytes - ) - switch confirmation.frame { - case .serverReady: - break - case let .denied(code): - throw CmxIrohClientSessionError.admissionDenied(code: code) - case .acceptedPendingNatTraversal, .acceptedRelayOnly, .clientReady: - throw CmxIrohClientSessionError.invalidAdmissionFrame - } - try Task.checkCancellation() - return CmxIrohConnectedControl( - connection: establishedConnection, - stream: stream, - initialReceiveBuffer: confirmation.trailingBytes - ) + switch confirmation.frame { + case .serverReady: + break case let .denied(code): throw CmxIrohClientSessionError.admissionDenied(code: code) - case .clientReady, .serverReady: + case .acceptedPendingNatTraversal, .acceptedRelayOnly, .clientReady: throw CmxIrohClientSessionError.invalidAdmissionFrame } - } catch { - await establishedConnection.close(errorCode: 1, reason: "admission_failed") - throw error + try Task.checkCancellation() + return CmxIrohConnectedControl( + connection: establishedConnection, + stream: stream, + initialReceiveBuffer: confirmation.trailingBytes + ) + case let .denied(code): + throw CmxIrohClientSessionError.admissionDenied(code: code) + case .clientReady, .serverReady: + throw CmxIrohClientSessionError.invalidAdmissionFrame } } @@ -498,22 +513,32 @@ public actor CmxIrohClientSession { ) async throws -> any CmxIrohConnection { let endpoint = endpoint let alpn = protocolConfiguration.alpn + return try await boundedByDialPhase { + try await endpoint.connect(to: address, alpn: alpn) + } + } + + /// Races one dial-phase operation against the injected phase bound. The + /// operation must cancel cooperatively; on timeout the loser is cancelled + /// and the phase fails typed so the redial machinery supersedes it + /// instead of wedging behind it (cmux#8531). + private func boundedByDialPhase( + _ operation: @escaping @Sendable () async throws -> Value + ) async throws -> Value { let bound = dialPhaseTimeout - return try await withThrowingTaskGroup( - of: (any CmxIrohConnection)?.self - ) { group in + return try await withThrowingTaskGroup(of: Value?.self) { group in group.addTask { - try await endpoint.connect(to: address, alpn: alpn) + try await operation() } group.addTask { try await ContinuousClock().sleep(for: bound) return nil } defer { group.cancelAll() } - guard let first = try await group.next(), let connection = first else { + guard let first = try await group.next(), let value = first else { throw CmxIrohClientSessionError.dialTimedOut } - return connection + return value } } diff --git a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRegistryContextProvider.swift b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRegistryContextProvider.swift index a5fb93af343a..2c0fd5d50e3d 100644 --- a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRegistryContextProvider.swift +++ b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRegistryContextProvider.swift @@ -193,6 +193,28 @@ public actor CmxIrohRegistryContextProvider: CmxIrohClientContextProvider { ) usedFreshDiscovery = true } catch { + try Task.checkCancellation() + // The refresh failed. Dialing with the last verified snapshot + // beats not dialing at all (cmux#9724): the staleness mark + // survives, so the next attempt still refetches once the + // broker recovers. Verification is not weakened; this + // snapshot passed the same checks when it was fetched. + if let lastGood = authoritativeDiscovery { + do { + return try await resolveContext( + for: request, + targetIdentity: targetIdentity, + routeHints: routeHints, + discovery: lastGood, + at: clock + ) + } catch is CancellationError { + throw CancellationError() + } catch { + // The last-good snapshot no longer authorizes this + // peer; fall through to the offline cache. + } + } guard Self.isConnectivity(error), let cached = try await cachedPolicy( for: request, diff --git a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayCredentialCoordinator.swift b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayCredentialCoordinator.swift index a2fad66b2758..5e39c1d90275 100644 --- a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayCredentialCoordinator.swift +++ b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayCredentialCoordinator.swift @@ -374,6 +374,14 @@ public actor CmxIrohRelayCredentialCoordinator { ), initialFailureCount: 1 ) + // Fail open while a last-good credential is installed on the + // endpoint: a failed mint must not fail the caller's foreground + // path into zero-route dial churn (cmux#10375). The bounded retry + // loop above keeps refreshing in the background, and the relay is + // the authority that rejects a credential that truly went bad. + if installedCredential != nil, !(error is CancellationError) { + return + } throw error } } diff --git a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayPolicyCache.swift b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayPolicyCache.swift index 6712e257bc9d..df245a1ea796 100644 --- a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayPolicyCache.swift +++ b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayPolicyCache.swift @@ -97,16 +97,47 @@ public actor CmxIrohRelayPolicyCache { /// - Parameters: /// - trustRoot: App-pinned public verification keys. /// - now: Verification time. + /// - expiredPolicyReuseGrace: Bounded fail-open window after the signed + /// expiry in which the last-good policy still loads. The policy is + /// re-verified at its final valid instant, so signature, rollback, + /// and claim checks run unweakened; only the expiry gate is graced + /// (cmux#10375). Zero preserves strict expiry. /// - Returns: The verified policy, or `nil` when no policy is cached. /// - Throws: ``CmxIrohRelayPolicyError`` or a secure-storage error. public func load( trustRoot: CmxIrohRelayPolicyTrustRoot, - now: Date + now: Date, + expiredPolicyReuseGrace: TimeInterval = 0 ) async throws -> CmxIrohManagedRelayPolicy? { await acquire() defer { release() } guard let record = try await storedRecord() else { return nil } - let policy = try verifier.verify(record.signedPolicy, trustRoot: trustRoot, now: now) + let policy: CmxIrohManagedRelayPolicy + do { + policy = try verifier.verify( + record.signedPolicy, + trustRoot: trustRoot, + now: now + ) + } catch CmxIrohRelayPolicyError.expired { + // The recorded expiry is cross-checked against the signed claims + // below; a record that overstates it re-fails as expired here or + // as rollback below. + guard expiredPolicyReuseGrace > 0, + let recordedExpiry = record.expiresAt, + now.timeIntervalSince1970 + <= TimeInterval(recordedExpiry) + expiredPolicyReuseGrace else { + throw CmxIrohRelayPolicyError.expired + } + let lastValidInstant = Date( + timeIntervalSince1970: TimeInterval(recordedExpiry) - 1 + ) + policy = try verifier.verify( + record.signedPolicy, + trustRoot: trustRoot, + now: min(now, lastValidInstant) + ) + } guard policy.sequence == record.highestSequence, Self.metadataMatches(policy, record: record) else { throw CmxIrohRelayPolicyError.rollback diff --git a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayPolicyService.swift b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayPolicyService.swift index 8405a2292b6c..408f2305caaa 100644 --- a/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayPolicyService.swift +++ b/Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohRelayPolicyService.swift @@ -6,10 +6,18 @@ public actor CmxIrohRelayPolicyService { private typealias Resolution = CmxIrohRelayPolicyResolutionResult private typealias Resolver = CmxIrohRelayPolicyResolution + /// Bounded fail-open window in which ``restore`` keeps a recently-expired + /// last-good managed policy dialable after a failed refresh (cmux#10375). + /// A failed policy refresh must degrade to the last verified catalog, not + /// to a zero-route profile; the relay itself remains the authority on + /// credential validity and rejects a truly stale token. + public static let defaultExpiredPolicyReuseGrace: TimeInterval = 6 * 60 * 60 + private let policyCache: CmxIrohRelayPolicyCache private let preferenceStore: CmxIrohRelayPreferenceStore private let credentialStore: CmxIrohCustomRelayCredentialStore private let broker: (any CmxIrohRelayPolicyServing)? + private let expiredPolicyReuseGrace: TimeInterval private var currentEffective: CmxIrohEffectiveRelayPolicy? private var currentDiagnostics = CmxIrohRelayDiagnosticsSnapshot.inactive private var continuations: [UUID: AsyncStream.Continuation] = [:] @@ -20,12 +28,15 @@ public actor CmxIrohRelayPolicyService { policyCache: CmxIrohRelayPolicyCache = CmxIrohRelayPolicyCache(), preferenceStore: CmxIrohRelayPreferenceStore = CmxIrohRelayPreferenceStore(), credentialStore: CmxIrohCustomRelayCredentialStore = CmxIrohCustomRelayCredentialStore(), - broker: (any CmxIrohRelayPolicyServing)? = nil + broker: (any CmxIrohRelayPolicyServing)? = nil, + expiredPolicyReuseGrace: TimeInterval = CmxIrohRelayPolicyService + .defaultExpiredPolicyReuseGrace ) { self.policyCache = policyCache self.preferenceStore = preferenceStore self.credentialStore = credentialStore self.broker = broker + self.expiredPolicyReuseGrace = max(0, expiredPolicyReuseGrace) } /// Fetches and installs the broker's current relay bootstrap response. @@ -196,7 +207,15 @@ public actor CmxIrohRelayPolicyService { } do { - guard let policy = try await policyCache.load(trustRoot: trustRoot, now: now) else { + // Restore is the failed-refresh fallback path, so it alone grants + // the bounded expired-policy grace: a recently-expired last-good + // catalog must stay dialable instead of zeroing every relay route + // (cmux#10375). The graced state is visible as `.policyExpired`. + guard let policy = try await policyCache.load( + trustRoot: trustRoot, + now: now, + expiredPolicyReuseGrace: expiredPolicyReuseGrace + ) else { return publishUnavailable( configuration: persisted.requested, revision: persisted.revision, @@ -205,6 +224,8 @@ public actor CmxIrohRelayPolicyService { failure: .policyUnavailable ) } + let policyIsExpired = TimeInterval(policy.expiresAt) + <= now.timeIntervalSince1970 let resolution = await Resolver.resolve( configuration: persisted.requested, revision: persisted.revision, @@ -218,7 +239,9 @@ public actor CmxIrohRelayPolicyService { return commit( Resolution( effective: resolution.effective, - failure: cleanupFailure ?? resolution.failure + failure: policyIsExpired + ? .policyExpired + : cleanupFailure ?? resolution.failure ), operation: operation ) diff --git a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayPolicyServiceTests.swift b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayPolicyServiceTests.swift index dadeed557f8c..cbe688287df4 100644 --- a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayPolicyServiceTests.swift +++ b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohRelayPolicyServiceTests.swift @@ -288,6 +288,66 @@ struct CmxIrohRelayPolicyServiceTests { #expect(await stores.service.diagnosticsSnapshot().failure == .preferenceRollback) } + @Test + func restoreFailsClosedBeyondTheExpiredPolicyReuseGrace() async throws { + let fixture = RelayPolicyServiceTestFixture() + let stores = makeStores() + _ = try await stores.service.install( + response: CmxIrohRelayPolicyResponse( + policy: fixture.token(sequence: 1), + preference: .automatic, + preferenceRevision: 1 + ), + accountID: "account-a", + trustRoot: fixture.firstTrustRoot, + relayCredential: fixture.relayCredential(), + now: fixture.now + ) + + let expired = await stores.service.restore( + accountID: "account-a", + trustRoot: try fixture.firstTrustRoot, + relayCredential: nil, + now: fixture.now.addingTimeInterval( + 3_600 + CmxIrohRelayPolicyService.defaultExpiredPolicyReuseGrace + 1 + ) + ) + + #expect(expired.source == .managedUnavailable) + #expect(expired.endpointRelayProfile.allowedRelayURLs.isEmpty) + #expect(await stores.service.diagnosticsSnapshot().failure == .policyExpired) + } + + @Test + func expiredPolicyGraceNeverBypassesSignatureVerification() async throws { + let fixture = RelayPolicyServiceTestFixture() + let stores = makeStores() + _ = try await stores.service.install( + response: CmxIrohRelayPolicyResponse( + policy: fixture.token(sequence: 1), + preference: .automatic, + preferenceRevision: 1 + ), + accountID: "account-a", + trustRoot: fixture.firstTrustRoot, + relayCredential: fixture.relayCredential(), + now: fixture.now + ) + + // A trust root that rejects the cached policy's signing key must + // fail closed even inside the expiry grace window: the grace covers + // only time, never a rejected credential. + let rejected = await stores.service.restore( + accountID: "account-a", + trustRoot: try fixture.secondTrustRoot, + relayCredential: nil, + now: fixture.now.addingTimeInterval(3_600 + 300) + ) + + #expect(rejected.source == .managedUnavailable) + #expect(rejected.endpointRelayProfile.allowedRelayURLs.isEmpty) + } + @Test func cacheRestoresUntilSignedExpiryAndSupportsStagedKeyRotation() async throws { let fixture = RelayPolicyServiceTestFixture() @@ -324,14 +384,18 @@ struct CmxIrohRelayPolicyServiceTests { #expect(restored.usedCachedPolicy) #expect(restored.managedSnapshot?.policy.sequence == 2) - let expired = await stores.service.restore( + // Immediately past the signed expiry the last-good policy stays + // dialable inside the bounded reuse grace (cmux#10375); the graced + // state is reported as `.policyExpired` without zeroing routes. + let graced = await stores.service.restore( accountID: "account-a", trustRoot: try fixture.secondTrustRoot, relayCredential: fixture.relayCredential(), now: fixture.now.addingTimeInterval(3_600) ) - #expect(expired.source == .managedUnavailable) - #expect(expired.endpointRelayProfile.allowedRelayURLs.isEmpty) + #expect(graced.source == .managed) + #expect(graced.usedCachedPolicy) + #expect(!graced.endpointRelayProfile.allowedRelayURLs.isEmpty) #expect(await stores.service.diagnosticsSnapshot().failure == .policyExpired) } From d5ad10198bfd75dfa916b495c2c1c2bb49b1fbd8 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Mon, 31 Aug 2026 19:28:38 -0700 Subject: [PATCH 3/3] test(iroh): prove the dial-phase deadline disarms on success and supersession spares established sessions Field dogfood on tag ibdl reported established-then-cancelled sessions (~250-300ms after establishment, ~5s after dial start) and suspected a dial-phase deadline that stays armed after its phase completes. These two behavior tests pin the required semantics and PASS against the current bounded-dial implementation, refuting that hypothesis at the client-session layer: - establishment just inside the deadline survives past the deadline (the timeout child is cancelled when the phase completes) - a superseded timed-out attempt cannot kill the next established session They stay as regression coverage while the field investigation moves to other layers. --- .../CmxIrohClientSessionDialBoundTests.swift | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohClientSessionDialBoundTests.swift b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohClientSessionDialBoundTests.swift index 6718b1cd2c42..82e7ba0aa6fb 100644 --- a/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohClientSessionDialBoundTests.swift +++ b/Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohClientSessionDialBoundTests.swift @@ -155,3 +155,139 @@ actor TestHangingIrohReceiveStream: CmxIrohReceiveStream { stoppedCodes } } + +/// Field regression coverage (ibdl device dogfood): sessions established just +/// inside the dial bound were observed closing ~250-300 ms after +/// establishment with a local cancelled attribution. The deadline must be +/// disarmed the moment its phase completes, and no remnant of a superseded +/// earlier attempt may cancel a later established session. +extension CmxIrohClientSessionDialBoundTests { + @Test("establishment just inside the deadline survives past the deadline") + func establishmentJustInsideTheDeadlineSurvivesPastTheDeadline() async throws { + let codec = CmxIrohAdmissionAckCodec() + let control = CmxIrohBidirectionalStream( + receiveStream: SlowIrohReceiveStream( + buffer: codec.encodeFrame(.acceptedPendingNatTraversal) + + codec.encodeFrame(.serverReady), + delay: .milliseconds(180) + ), + sendStream: TestIrohSendStream() + ) + let connection = TestIrohConnection( + remoteIdentity: remoteIdentity, + bidirectionalStreams: [control] + ) + let endpoint = TestDialingIrohEndpoint( + localIdentity: localIdentity, + dialResults: [.connection(connection)] + ) + let session = try CmxIrohClientSession( + endpoint: endpoint, + targetIdentity: remoteIdentity, + dialPlan: try testIrohDialPlan(publicPaths: [try publicRelayHint()]), + credential: credential, + dialPhaseTimeout: .milliseconds(250) + ) + + // Admission completes at ~180ms, inside the 250ms bound. + try await session.connect() + #expect(await connection.observedCloseCallCount() == 0) + + // Cross the original deadline (and a margin) while established. A + // deadline that was not disarmed on success fires in this window and + // closes the admitted connection. + try await ContinuousClock().sleep(for: .milliseconds(400)) + + #expect(await connection.isClosed() == false) + #expect(await connection.observedCloseCallCount() == 0) + await session.close() + } + + @Test("a superseded timed-out attempt cannot kill the next established session") + func supersededTimedOutAttemptCannotKillTheNextEstablishedSession() async throws { + let codec = CmxIrohAdmissionAckCodec() + let hangingControl = CmxIrohBidirectionalStream( + receiveStream: TestHangingIrohReceiveStream(), + sendStream: TestIrohSendStream() + ) + let hangingConnection = TestIrohConnection( + remoteIdentity: remoteIdentity, + bidirectionalStreams: [hangingControl] + ) + let goodControl = CmxIrohBidirectionalStream( + receiveStream: SlowIrohReceiveStream( + buffer: codec.encodeFrame(.acceptedPendingNatTraversal) + + codec.encodeFrame(.serverReady), + delay: .milliseconds(60) + ), + sendStream: TestIrohSendStream() + ) + let goodConnection = TestIrohConnection( + remoteIdentity: remoteIdentity, + bidirectionalStreams: [goodControl] + ) + let endpoint = TestDialingIrohEndpoint( + localIdentity: localIdentity, + dialResults: [ + .connection(hangingConnection), + .connection(goodConnection), + ] + ) + let session = try CmxIrohClientSession( + endpoint: endpoint, + targetIdentity: remoteIdentity, + dialPlan: try testIrohDialPlan(publicPaths: [try publicRelayHint()]), + credential: credential, + dialPhaseTimeout: .milliseconds(100) + ) + + let failure = await boundedConnectFailure(session, within: .seconds(2)) + #expect(failure as? CmxIrohClientSessionError == .dialTimedOut) + + // The superseding retry establishes inside its own bound. + try await session.connect() + #expect(await goodConnection.observedCloseCallCount() == 0) + + // No remnant of the superseded attempt (its deadline, its cancelled + // children, or its close path) may touch the established session. + try await ContinuousClock().sleep(for: .milliseconds(300)) + + #expect(await goodConnection.isClosed() == false) + #expect(await goodConnection.observedCloseCallCount() == 0) + #expect(await hangingConnection.observedCloseCallCount() >= 1) + await session.close() + } +} + +/// Delivers its admission bytes only after a fixed delay, modeling a peer +/// that answers admission just inside the dial bound. The delay is +/// cancellable like every production stream await. +actor SlowIrohReceiveStream: CmxIrohReceiveStream { + private var buffer: Data + private var delay: Duration? + private var stoppedCodes: [UInt64] = [] + + init(buffer: Data, delay: Duration) { + self.buffer = buffer + self.delay = delay + } + + func receive(maximumByteCount: Int) async throws -> Data? { + guard maximumByteCount > 0 else { + throw CmxIrohClientSessionError.invalidMaximumByteCount(maximumByteCount) + } + if let pending = delay { + delay = nil + try await Task.sleep(for: pending) + } + guard !buffer.isEmpty else { return nil } + let count = min(maximumByteCount, buffer.count) + let value = Data(buffer.prefix(count)) + buffer.removeFirst(count) + return value + } + + func stop(errorCode: UInt64) { + stoppedCodes.append(errorCode) + } +}