From fbb3381dfeb9f321640181af940b8a441d48bfaa Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 31 Oct 2026 21:22:55 -0700 Subject: [PATCH] fix(mobile): give a direct handshake as long as the desktop allows it (#36864) The phone gave a direct socket 5 s from open to authenticated, while the desktop keeps an unauthenticated socket for 10 s. On a slow link (a handshake is two round trips, so ~2.5 s RTT is enough) the phone dropped the socket the desktop would have accepted, then redialed into the same deadline forever: the 'Can't connect' loop a test documented. Both ends now share PAIRED_E2EE_HANDSHAKE_DEADLINE_MS (10 s). A link that needs 6 s to authenticate now connects on the first dial; one that never authenticates still escalates the same way, 5 s later. Relay dials are unaffected: they keep their own stage budgets and race the direct dial after 2.5 s. --- .../cellular-connecting-label-stall.test.ts | 21 +++++++++++++++---- .../transport/rpc-client-socket-session.ts | 4 ++-- ...ient-synthesized-close-diagnostics.test.ts | 15 ++++++------- src/main/runtime/rpc/e2ee-channel.ts | 4 ++-- src/shared/paired-e2ee-handshake-deadline.ts | 6 ++++++ 5 files changed, 35 insertions(+), 15 deletions(-) create mode 100644 src/shared/paired-e2ee-handshake-deadline.ts diff --git a/mobile/src/transport/cellular-connecting-label-stall.test.ts b/mobile/src/transport/cellular-connecting-label-stall.test.ts index 9db78b0ab67..81c89b5d052 100644 --- a/mobile/src/transport/cellular-connecting-label-stall.test.ts +++ b/mobile/src/transport/cellular-connecting-label-stall.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { connect } from './rpc-client' import { classifyConnection, verdictDisplayLabel } from './connection-health' +import { PAIRED_E2EE_HANDSHAKE_DEADLINE_MS } from '../../../src/shared/paired-e2ee-handshake-deadline' // Keep escalation through socket opens/redials until E2EE succeeds; otherwise cellular failures loop on Connecting (#10119). @@ -159,8 +160,8 @@ describe('issue #10119 — what a phone shows while it cannot reach the desktop' globalThis.WebSocket = originalWebSocket }) - it('escalates and stays escalated when the handshake never fits the 5s budget', () => { - carrier = { kind: 'slow-handshake', readyAfterMs: 6_000 } + it('escalates and stays escalated when the handshake never fits its budget', () => { + carrier = { kind: 'slow-handshake', readyAfterMs: PAIRED_E2EE_HANDSHAKE_DEADLINE_MS + 1_000 } const samples = observe('ws://192.168.0.56:6769', 900_000) expect(samples.some((s) => s.state === 'handshaking')).toBe(true) @@ -214,10 +215,22 @@ describe('issue #10119 — what a phone shows while it cannot reach the desktop' expect(after.every((s) => s.label === "Can't reach desktop")).toBe(true) }) - it('resets the failure counter only once a handshake actually completes', async () => { + // The desktop holds an unauthenticated socket open this long, so the phone waits as long. + it('connects on the first dial when a slow link takes six seconds to authenticate', async () => { carrier = { kind: 'slow-handshake', readyAfterMs: 6_000 } const client = connect('ws://192.168.0.56:6769', 'device-token', 'server-public-key') + await vi.advanceTimersByTimeAsync(PAIRED_E2EE_HANDSHAKE_DEADLINE_MS) + expect(client.getState()).toBe('connected') + expect(sockets).toHaveLength(1) + + client.close() + }) + + it('resets the failure counter only once a handshake actually completes', async () => { + carrier = { kind: 'slow-handshake', readyAfterMs: PAIRED_E2EE_HANDSHAKE_DEADLINE_MS + 1_000 } + const client = connect('ws://192.168.0.56:6769', 'device-token', 'server-public-key') + // Flush promise jobs between timers because auth is awaited. await vi.advanceTimersByTimeAsync(40_000) expect(client.getState()).not.toBe('connected') @@ -235,7 +248,7 @@ describe('issue #10119 — what a phone shows while it cannot reach the desktop' carrier = { kind: 'wedged-open-then-silent' } const client = connect('ws://192.168.0.56:6769', 'device-token', 'server-public-key') - vi.advanceTimersByTime(5_400) + vi.advanceTimersByTime(PAIRED_E2EE_HANDSHAKE_DEADLINE_MS + 400) // A missing close callback must not strand handshaking without a retry or foreground recovery. expect(sockets).toHaveLength(1) diff --git a/mobile/src/transport/rpc-client-socket-session.ts b/mobile/src/transport/rpc-client-socket-session.ts index 9a2d70d49ae..d651625e8a0 100644 --- a/mobile/src/transport/rpc-client-socket-session.ts +++ b/mobile/src/transport/rpc-client-socket-session.ts @@ -1,4 +1,5 @@ import type { PairedE2EEFraming } from '../../../src/shared/paired-e2ee-client-handshake' +import { PAIRED_E2EE_HANDSHAKE_DEADLINE_MS } from '../../../src/shared/paired-e2ee-handshake-deadline' import type { PairedE2EEFramingPolicy } from '../../../src/shared/paired-e2ee-framing-policy' import { createDirectSocketFraming, type DirectSocketFraming } from './direct-socket-framing' import { isRpcResponse } from './rpc-response-shape' @@ -9,7 +10,6 @@ import { startSocketDeadlineClock } from './socket-deadline-clock' import type { ConnectionLogEmitter, ConnectionState, RpcRequest, RpcResponse } from './types' const CONNECT_TIMEOUT_MS = 12_000 -const HANDSHAKE_TIMEOUT_MS = 5_000 const WEBSOCKET_CONNECTING_STATE = 0 const UNAUTHORIZED_CLOSE_CODE = 4001 @@ -238,7 +238,7 @@ export class RpcClientSocketSession { } private armHandshakeTimeout(): void { - this.armSocketDeadline('handshake', HANDSHAKE_TIMEOUT_MS) + this.armSocketDeadline('handshake', PAIRED_E2EE_HANDSHAKE_DEADLINE_MS) } private armSocketDeadline(kind: SocketDeadlineKind, timeoutMs: number): void { diff --git a/mobile/src/transport/rpc-client-synthesized-close-diagnostics.test.ts b/mobile/src/transport/rpc-client-synthesized-close-diagnostics.test.ts index ec1a8bfab7c..af53a36429f 100644 --- a/mobile/src/transport/rpc-client-synthesized-close-diagnostics.test.ts +++ b/mobile/src/transport/rpc-client-synthesized-close-diagnostics.test.ts @@ -1,6 +1,7 @@ // Pin diagnostic clocks and auth-failed latches when RN omits close and later delivers the stale event. import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from 'vitest' import { connect } from './rpc-client' +import { PAIRED_E2EE_HANDSHAKE_DEADLINE_MS } from '../../../src/shared/paired-e2ee-handshake-deadline' import { diagnoseConnection } from '../diagnostics/connection-diagnostics-analysis' import type { ConnectionLogEntry } from './types' import { recordSocketDeadlineAppState } from './socket-deadline-clock' @@ -113,7 +114,7 @@ describe('synthesized close diagnostics', () => { if (kind === 'handshake') { lastSocket().open() } - const timeout = kind === 'connect' ? 12_000 : 5_000 + const timeout = kind === 'connect' ? 12_000 : PAIRED_E2EE_HANDSHAKE_DEADLINE_MS try { elapsed = 164 * 60_000 vi.setSystemTime(Date.now() + elapsed - timeout) @@ -216,7 +217,7 @@ describe('synthesized close diagnostics', () => { const client = connect('ws://desktop.invalid', 'token', 'server-key') lastSocket().open() - await vi.advanceTimersByTimeAsync(5_000) + await vi.advanceTimersByTimeAsync(PAIRED_E2EE_HANDSHAKE_DEADLINE_MS) expect(client.getState()).toBe('reconnecting') await vi.advanceTimersByTimeAsync(500) @@ -235,7 +236,7 @@ describe('synthesized close diagnostics', () => { await vi.advanceTimersByTimeAsync(100) wedged.receive(JSON.stringify({ type: 'e2ee_ready' })) - await vi.advanceTimersByTimeAsync(4_900) + await vi.advanceTimersByTimeAsync(PAIRED_E2EE_HANDSHAKE_DEADLINE_MS - 100) await vi.advanceTimersByTimeAsync(500) expect(sockets).toHaveLength(2) const replacement = lastSocket() @@ -249,9 +250,9 @@ describe('synthesized close diagnostics', () => { const staleCloseLog = logFields( logSpy.mock.calls.findLast((call) => call[0] === '[net] ws.onclose')?.[1] ) - expect(staleCloseLog?.constructToCloseMs).toBe(5_700) + expect(staleCloseLog?.constructToCloseMs).toBe(PAIRED_E2EE_HANDSHAKE_DEADLINE_MS + 700) expect(staleCloseLog?.aliveMs).toBeNull() - expect(staleCloseLog?.inboundIdleMs).toBe(5_600) + expect(staleCloseLog?.inboundIdleMs).toBe(PAIRED_E2EE_HANDSHAKE_DEADLINE_MS + 600) await vi.advanceTimersByTimeAsync(300) lastSocket().deliverClose(1006) @@ -269,7 +270,7 @@ describe('synthesized close diagnostics', () => { const client = connect('ws://desktop.invalid', 'token', 'server-key') const wedged = lastSocket() wedged.open() - await vi.advanceTimersByTimeAsync(5_000) + await vi.advanceTimersByTimeAsync(PAIRED_E2EE_HANDSHAKE_DEADLINE_MS) expect(client.getState()).toBe('reconnecting') await vi.advanceTimersByTimeAsync(500) const replacement = lastSocket() @@ -308,7 +309,7 @@ describe('synthesized close diagnostics', () => { const client = connect('ws://desktop.invalid', 'token', 'server-key') const wedged = lastSocket() wedged.open() - await vi.advanceTimersByTimeAsync(5_500) + await vi.advanceTimersByTimeAsync(PAIRED_E2EE_HANDSHAKE_DEADLINE_MS + 500) const replacement = lastSocket() replacement.open() diff --git a/src/main/runtime/rpc/e2ee-channel.ts b/src/main/runtime/rpc/e2ee-channel.ts index f5a90222ca1..c727132f1f7 100644 --- a/src/main/runtime/rpc/e2ee-channel.ts +++ b/src/main/runtime/rpc/e2ee-channel.ts @@ -1,5 +1,6 @@ // Keep E2EE framing outside transport-independent plaintext RPC handlers. import type { WebSocket } from 'ws' +import { PAIRED_E2EE_HANDSHAKE_DEADLINE_MS } from '../../../shared/paired-e2ee-handshake-deadline' import { deriveSharedKey, encrypt, decrypt, decryptBytes } from '../../../shared/e2ee-crypto' import { DesktopMobileE2EEV2Session, @@ -30,7 +31,6 @@ import type { } from './rpc-binary-sender' import type { DevicePrincipal } from '../device-principal' -const HANDSHAKE_TIMEOUT_MS = 10_000 const MAX_CONSECUTIVE_DECRYPT_FAILURES = 5 export type E2EEChannelOptions = { @@ -104,7 +104,7 @@ export class E2EEChannel { this.handshakeTimer = setTimeout(() => { this.onError(4002, 'E2EE handshake timeout') - }, HANDSHAKE_TIMEOUT_MS) + }, PAIRED_E2EE_HANDSHAKE_DEADLINE_MS) } onMessage( diff --git a/src/shared/paired-e2ee-handshake-deadline.ts b/src/shared/paired-e2ee-handshake-deadline.ts new file mode 100644 index 00000000000..867056da26c --- /dev/null +++ b/src/shared/paired-e2ee-handshake-deadline.ts @@ -0,0 +1,6 @@ +/** + * How long a paired socket may take from open to authenticated, on both ends. The desktop closes a + * socket that has not authenticated by then, so a phone that gives up sooner drops slow links the + * desktop would still have accepted. + */ +export const PAIRED_E2EE_HANDSHAKE_DEADLINE_MS = 10_000