diff --git a/apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts b/apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts index c515fa871..c540970cd 100644 --- a/apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts +++ b/apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts @@ -20,6 +20,7 @@ import type { SyncHelloPayload, SyncMobileProjectSummary, SyncPairingRequestPayload, + SyncPairingResultPayload, SyncPeerMetadata, SyncProjectForgetRequestPayload, SyncProjectForgetResultPayload, @@ -37,6 +38,10 @@ import { SYNC_HOST_BIND_LOOPBACK_ONLY } from "./sharedSyncListener"; import type { SyncCredentialStore } from "../credentials/credentialStore"; import { createSyncPairingStore, type SyncPairingRecord } from "./syncPairingStore"; import { createSyncDpopNonceCache, evaluatePairedHelloDpop } from "./syncDpop"; +import { + createPairFailureTracker, + type PairFailureSubject, +} from "./syncPairFailureTracker"; import { createRelayAuthorizationLifecycle, SYNC_RELAY_AUTHORIZATION_CLOSE_CODE, @@ -110,17 +115,8 @@ type BrainPeerState = { const WS_OPEN = 1; const BOOTSTRAP_TOKEN_KEY = "sync.bootstrapToken.v1"; -const PAIR_FAILURE_THRESHOLD = 5; -const PAIR_COOLDOWN_MS = 10 * 60_000; -const PAIR_FAILURE_WINDOW_MS = 10 * 60_000; const BRAIN_SYNC_AUTH_TIMEOUT_MS = 15_000; -type PairFailureEntry = { - count: number; - cooldownUntilMs: number; - updatedAtMs: number; -}; - function ensureSecretFile(filePath: string, bytes: number): string { fs.mkdirSync(path.dirname(filePath), { recursive: true }); if (!fs.existsSync(filePath)) { @@ -366,73 +362,6 @@ function projectActionsEnabled(provider: SyncProjectCatalogProvider): boolean { ); } -function createPairFailureTracker() { - const pairFailures = new Map(); - const globalPairFailures: PairFailureEntry = { - count: 0, - cooldownUntilMs: 0, - updatedAtMs: 0, - }; - - const reset = (entry: PairFailureEntry): void => { - entry.count = 0; - entry.cooldownUntilMs = 0; - entry.updatedAtMs = 0; - }; - const expired = (entry: PairFailureEntry, now: number): boolean => { - if (entry.updatedAtMs <= 0) return false; - return (entry.cooldownUntilMs > 0 && entry.cooldownUntilMs <= now) - || entry.updatedAtMs + PAIR_FAILURE_WINDOW_MS <= now; - }; - const prune = (now = Date.now()): void => { - for (const [ip, entry] of pairFailures) { - if (expired(entry, now)) { - pairFailures.delete(ip); - } - } - if (expired(globalPairFailures, now)) { - reset(globalPairFailures); - } - }; - const increment = (entry: PairFailureEntry, now: number): void => { - entry.count += 1; - entry.updatedAtMs = now; - if (entry.count >= PAIR_FAILURE_THRESHOLD) { - entry.cooldownUntilMs = now + PAIR_COOLDOWN_MS; - entry.count = 0; - } - }; - - return { - cooldownMsRemaining(ip: string | null): number { - const now = Date.now(); - prune(now); - const globalRemaining = Math.max(0, globalPairFailures.cooldownUntilMs - now); - const ipEntry = ip ? pairFailures.get(ip) ?? null : null; - const ipRemaining = ipEntry ? Math.max(0, ipEntry.cooldownUntilMs - now) : 0; - return Math.max(globalRemaining, ipRemaining); - }, - registerFailure(ip: string | null): void { - const now = Date.now(); - prune(now); - increment(globalPairFailures, now); - if (ip) { - const entry = pairFailures.get(ip) ?? { - count: 0, - cooldownUntilMs: 0, - updatedAtMs: now, - }; - increment(entry, now); - pairFailures.set(ip, entry); - } - }, - clearAfterSuccess(ip: string | null): void { - reset(globalPairFailures); - if (ip) pairFailures.delete(ip); - }, - }; -} - async function projectCatalog(provider: SyncProjectCatalogProvider, logger: Logger): Promise { try { return await provider.listProjects(); @@ -1204,7 +1133,11 @@ export function createBrainProjectActionsSyncHandler( return; } if (!isPeerCurrent(lifecycleGeneration)) return; - const cooldownMs = pairFailures.cooldownMsRemaining(remoteAddress ?? null); + const pairFailureSubject: PairFailureSubject = { + ip: remoteAddress ?? null, + deviceId: payload.peer.deviceId, + }; + const cooldownMs = pairFailures.cooldownMsRemaining(pairFailureSubject); if (cooldownMs > 0) { const minutes = Math.ceil(cooldownMs / 60_000); send(ws, "pairing_result", { @@ -1226,8 +1159,20 @@ export function createBrainProjectActionsSyncHandler( dpopPublicKey: payload.dpopPublicKey ?? null, runtimeHostGrant: payload.runtimeHostGrant ?? null, }); - pairFailures.clearAfterSuccess(remoteAddress ?? null); - send(ws, "pairing_result", { ok: true, deviceId: paired.deviceId, secret: paired.secret }, envelope.requestId); + pairFailures.clearAfterSuccess(pairFailureSubject); + send(ws, "pairing_result", { + ok: true, + deviceId: paired.deviceId, + secret: paired.secret, + ...(paired.pendingRotationExpiresAtMs != null + ? { + rotation: { + pendingCommit: true, + expiresInMs: Math.max(0, paired.pendingRotationExpiresAtMs - Date.now()), + }, + } + : {}), + } satisfies SyncPairingResultPayload, envelope.requestId); } catch (error) { const code = (error as { code?: string } | null)?.code === "pin_not_set" ? "pin_not_set" @@ -1242,7 +1187,7 @@ export function createBrainProjectActionsSyncHandler( }, }, envelope.requestId); if (code === "invalid_pin" || code === "pairing_failed") { - pairFailures.registerFailure(remoteAddress ?? null); + pairFailures.registerFailure(pairFailureSubject); } try { ws.close(4003, "Pairing failed"); diff --git a/apps/ade-cli/src/services/sync/syncDpop.ts b/apps/ade-cli/src/services/sync/syncDpop.ts index 18ff3ccc1..aeae663b1 100644 --- a/apps/ade-cli/src/services/sync/syncDpop.ts +++ b/apps/ade-cli/src/services/sync/syncDpop.ts @@ -164,6 +164,26 @@ export function evaluatePairedHelloDpop(input: { return input.requireDpop ? "dpop_required" : null; } +/** + * The next step a user can actually take for each DPoP rejection reason. These + * are sent verbatim to the client, which used to render every one of them as + * "Sync authentication failed." — a clock skew and a stolen-key rejection are + * not the same problem and must not read the same. + */ +export function syncDpopFailureMessage(reason: string): string { + switch (reason) { + case "proof_required": + case "dpop_required": + return "This device did not present its security key. Update ADE on the device, then pair it again."; + case "stale_timestamp": + return "This device's clock is too far from this machine's. Fix the date and time on both, then try again."; + case "replayed_nonce": + return "This device's security proof had already been used. Try connecting again."; + default: + return "This device could not prove it holds the security key this machine has on record. Pair it again."; + } +} + /** Bounded nonce replay cache keyed by `deviceId:nonce`. */ export function createSyncDpopNonceCache(args?: { ttlMs?: number; maxEntries?: number }) { const ttlMs = args?.ttlMs ?? SYNC_DPOP_NONCE_TTL_MS; diff --git a/apps/ade-cli/src/services/sync/syncHostService.test.ts b/apps/ade-cli/src/services/sync/syncHostService.test.ts index 964ee59dd..33990f2bd 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.test.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.test.ts @@ -67,6 +67,7 @@ import { createSharedSyncListener, SYNC_RELAY_BRIDGE_PROOF_HEADER } from "./shar import type { SyncLoopbackProbeResult } from "./syncLoopbackProbe"; import { createSyncPairingStore, type SyncPairingRecord } from "./syncPairingStore"; import { createSyncPinStore } from "./syncPinStore"; +import { PAIR_FAILURE_THRESHOLD } from "./syncPairFailureTracker"; import { buildSyncDpopChallenge, sha256Hex } from "./syncDpop"; import { buildRelayReauthorizationChallenge, @@ -1463,7 +1464,7 @@ describe("brain project actions fallback handler", () => { }; }; - for (let attempt = 0; attempt < 5; attempt += 1) { + for (let attempt = 0; attempt < PAIR_FAILURE_THRESHOLD; attempt += 1) { const failed = await sendPairingRequest(`bad-pin-${attempt}`, "000000", `ios-bad-${attempt}`); expect(failed.payload.ok).toBe(false); expect(failed.payload.error?.code).toBe("invalid_pin"); @@ -2350,7 +2351,7 @@ describe("sync host account authentication", () => { payload: { v: 1, nonce: "not-32-bytes", clientEphemeralPublicKey: "bad" }, })); }; - for (let index = 0; index < 5; index += 1) { + for (let index = 0; index < PAIR_FAILURE_THRESHOLD; index += 1) { const badClient = await openAccountClient(port); clients.push(badClient); sendMalformedChallenge(badClient, `malformed-${index}`); @@ -2694,6 +2695,7 @@ describe("sync host account authentication", () => { ); expect(bootstrapRejected.payload).toMatchObject({ code: "relay_account_required", + message: "Sign in with the same ADE account on both machines.", }); } finally { for (const client of clients) client.ws.close(); @@ -3107,10 +3109,14 @@ describe("sync host account authentication", () => { accountToken, }), }); - await waitForValue( + const directAccountRejected = await waitForValue( () => directAccountClient.envelopes.find((envelope) => envelope.type === "hello_error"), "direct stored-key account rejection", ); + expect(directAccountRejected.payload).toMatchObject({ + code: "auth_failed", + message: expect.stringMatching(/Connect through ADE Relay, or pair this device with a PIN/), + }); expect(pairingStore.getPairingRecord(peer.deviceId)?.dpopPublicKey).toBe(legitimateKey.publicKeyX963); const relayAttackerClient = await openAccountClient(port, listener.getRelayBridgeProof()); @@ -3126,10 +3132,14 @@ describe("sync host account authentication", () => { accountToken, }), }); - await waitForValue( + const relayAttackerRejected = await waitForValue( () => relayAttackerClient.envelopes.find((envelope) => envelope.type === "hello_error"), "relay stored-key account hijack rejection", ); + expect(relayAttackerRejected.payload).toMatchObject({ + code: "auth_failed", + message: expect.stringMatching(/could not prove it holds the security key.*Pair it again/i), + }); expect(pairingStore.getPairingRecord(peer.deviceId)?.dpopPublicKey).toBe(legitimateKey.publicKeyX963); const relayAccountClient = await openAccountClient(port, listener.getRelayBridgeProof()); @@ -3206,6 +3216,7 @@ describe("sync host account authentication", () => { ); expect(missingRelayProof.payload).toMatchObject({ code: "relay_account_required", + message: "Sign in with the same ADE account on both machines.", }); const relayWrongAccount = await openAccountClient(port, listener.getRelayBridgeProof()); @@ -3228,6 +3239,7 @@ describe("sync host account authentication", () => { ); expect(wrongRelayProof.payload).toMatchObject({ code: "relay_account_required", + message: "Sign in with the same ADE account on both machines.", }); const relayPairedClient = await openAccountClient(port, listener.getRelayBridgeProof()); @@ -3527,10 +3539,14 @@ describe("sync host account authentication", () => { const missing = await openAccountClient(port, listener.getRelayBridgeProof()); clients.push(missing); sendAccountHello({ ws: missing.ws, peer: missingPeer, accountToken, dpop: null }); - await waitForValue( + const missingDpopRejected = await waitForValue( () => missing.envelopes.find((envelope) => envelope.type === "hello_error"), "missing DPoP hello_error", ); + expect(missingDpopRejected.payload).toMatchObject({ + code: "auth_failed", + message: expect.stringMatching(/did not present its security key.*pair it again/i), + }); const invalidPeer = { ...missingPeer, @@ -3553,10 +3569,14 @@ describe("sync host account authentication", () => { signedDeviceId: "different-account-device", }), }); - await waitForValue( + const invalidDpopRejected = await waitForValue( () => invalid.envelopes.find((envelope) => envelope.type === "hello_error"), "invalid DPoP hello_error", ); + expect(invalidDpopRejected.payload).toMatchObject({ + code: "auth_failed", + message: expect.stringMatching(/could not prove it holds the security key.*Pair it again/i), + }); expect(pairingStore.getPairingRecord(missingPeer.deviceId)).toBeNull(); expect(pairingStore.getPairingRecord(invalidPeer.deviceId)).toBeNull(); @@ -3614,10 +3634,14 @@ describe("sync host account authentication", () => { accountToken, }), }); - await waitForValue( + const signedOutRejected = await waitForValue( () => accountClient.envelopes.find((envelope) => envelope.type === "hello_error"), "signed-out account hello_error", ); + expect(signedOutRejected.payload).toMatchObject({ + code: "auth_failed", + message: expect.stringMatching(/not signed in.*Sign in on the Mac/i), + }); const pinClient = await openAccountClient(port); clients.push(pinClient); @@ -3643,6 +3667,149 @@ describe("sync host account authentication", () => { cleanup(); } }); + + // A re-pair used to overwrite the device's working secret the instant the + // host answered, two round trips before the device could persist the reply. + // Dropping the socket in that gap — the ordinary outcome on a flaky network — + // left the host holding credentials the phone never saw, and the only way + // back was walking to the Mac for another PIN. Retrying was what broke you. + it("survives a re-pair that drops before the device ever uses the new secret", async () => { + const { projectRoot, cleanup } = createTempProjectRoot(); + const secretsDir = path.join(projectRoot, ".ade", "secrets"); + const pinStore = createSyncPinStore({ filePath: path.join(secretsDir, "sync-pin.json") }); + pinStore.setPin("428193"); + const pairingSecretsPath = path.join(secretsDir, "sync-paired-devices.json"); + const pairingStore = createSyncPairingStore({ filePath: pairingSecretsPath, pinStore }); + const baseArgs = createHostArgs(projectRoot, []); + const host = createSyncHostService({ + ...baseArgs, + ...accountDependencies(), + pinStore, + pairingStore, + pairingSecretsPath, + discoveryEnabled: false, + deviceRegistryService: { + ...baseArgs.deviceRegistryService, + upsertPeerMetadata: vi.fn(), + }, + } as unknown as Parameters[0]); + const peer = { + deviceId: "flaky-repair-phone", + deviceName: "Flaky iPhone", + platform: "iOS", + deviceType: "phone", + siteId: "flaky-repair-site", + dbVersion: 0, + } satisfies SyncPeerMetadata; + const dpopKey = makeDpopKeyPair(); + const clients: Array>> = []; + + const requestPairing = async (requestId: string, pairingCommitVersion?: 1) => { + const port = await host.waitUntilListening(); + const client = await openAccountClient(port); + clients.push(client); + client.ws.send(encodeSyncEnvelope({ + type: "pairing_request", + requestId, + payload: { + code: "428193", + peer, + dpopPublicKey: dpopKey.publicKeyX963, + ...(pairingCommitVersion ? { pairingCommitVersion } : {}), + }, + })); + const result = await waitForValue( + () => client.envelopes.find((envelope) => + envelope.type === "pairing_result" && envelope.requestId === requestId), + `pairing_result ${requestId}`, + ); + return { + client, + payload: result.payload as { secret: string; rotation?: unknown }, + }; + }; + + const helloWith = async (secret: string) => { + const port = await host.waitUntilListening(); + const client = await openAccountClient(port); + clients.push(client); + sendPairedHello({ + ws: client.ws, + peer, + secret, + dpop: signPairedDpop({ + privateKey: dpopKey.privateKey, + publicKeyX963: dpopKey.publicKeyX963, + deviceId: peer.deviceId, + secret, + }), + }); + const envelope = await waitForValue( + () => client.envelopes.find((candidate) => + candidate.type === "hello_ok" || candidate.type === "hello_error"), + `hello outcome for ${secret.slice(0, 6)}`, + ); + return envelope.type; + }; + + try { + const first = await requestPairing("pair-initial"); + // Nothing to protect on a first pair, so nothing is staged. + expect(first.payload.rotation).toBeUndefined(); + expect(await helloWith(first.payload.secret)).toBe("hello_ok"); + + const rotated = await requestPairing("pair-retry", 1); + expect(rotated.payload.secret).not.toBe(first.payload.secret); + // The drop: the device never gets to send its hello. + rotated.client.ws.close(); + + // Both ends still agree on the secret the device is actually holding. + expect(await helloWith(first.payload.secret)).toBe("hello_ok"); + expect(rotated.payload.rotation).toMatchObject({ pendingCommit: true }); + + const committed = await requestPairing("pair-commit", 1); + // A commit-capable client uses the same socket for its staged hello. Even + // after hello_ok, the old secret remains live until the client explicitly + // proves it received that response. + sendPairedHello({ + ws: committed.client.ws, + peer, + secret: committed.payload.secret, + dpop: signPairedDpop({ + privateKey: dpopKey.privateKey, + publicKeyX963: dpopKey.publicKeyX963, + deviceId: peer.deviceId, + secret: committed.payload.secret, + }), + }); + await waitForValue( + () => committed.client.envelopes.find((candidate) => candidate.type === "hello_ok"), + "staged hello_ok", + ); + expect(pairingStore.verifySecret(peer.deviceId, first.payload.secret)).toBe("committed"); + expect(pairingStore.verifySecret(peer.deviceId, committed.payload.secret)).toBe("pending"); + + committed.client.ws.send(encodeSyncEnvelope({ + type: "pairing_commit", + requestId: "pairing-commit", + payload: { deviceId: peer.deviceId }, + })); + await expect(waitForValue( + () => committed.client.envelopes.find((candidate) => + candidate.type === "pairing_commit_result" + && candidate.requestId === "pairing-commit"), + "pairing_commit_result", + )).resolves.toMatchObject({ payload: { ok: true } }); + + // The explicit acknowledgement retires the previous credential. + expect(await helloWith(first.payload.secret)).toBe("hello_error"); + expect(await helloWith(committed.payload.secret)).toBe("hello_ok"); + } finally { + for (const client of clients) client.ws.close(); + await host.dispose(); + cleanup(); + } + }); }); describe("paired runtime host authorization", () => { diff --git a/apps/ade-cli/src/services/sync/syncHostService.ts b/apps/ade-cli/src/services/sync/syncHostService.ts index eda978726..0f323ff61 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.ts @@ -144,7 +144,15 @@ import type { AdeDb } from "../../../../desktop/src/main/services/state/kvDb"; import { hasNullByte, normalizeRelative, nowIso, resolvePathWithinRoot, safeJsonParse, toOptionalString, uniqueStrings, writeTextAtomic } from "../../../../desktop/src/main/services/shared/utils"; import type { DeviceRegistryService } from "./deviceRegistryService"; import { createSyncPairingStore, type SyncPairingRecord } from "./syncPairingStore"; -import { createSyncDpopNonceCache, evaluatePairedHelloDpop } from "./syncDpop"; +import { + createPairFailureTracker, + type PairFailureSubject, +} from "./syncPairFailureTracker"; +import { + createSyncDpopNonceCache, + evaluatePairedHelloDpop, + syncDpopFailureMessage, +} from "./syncDpop"; import { createRelayAuthorizationLifecycle, SYNC_RELAY_AUTHORIZATION_CLOSE_CODE, @@ -567,6 +575,12 @@ type PeerState = { authKind: SyncHostAuthKind; pairedDeviceId: string | null; pairingRecord: SyncPairingRecord | null; + /** Commit negotiation belongs to the socket that requested this re-pair. */ + pairingCommitOfferedForDeviceId: string | null; + /** Set only after this socket authenticates with that staged replacement. */ + pendingPairingCommitDeviceId: string | null; + /** Binds a later commit to the exact rotation this socket authenticated. */ + pendingPairingCommitSecret: string | null; connectedAt: string; lastSeenAt: string; lastAppliedAt: string | null; @@ -1548,6 +1562,7 @@ function parsePairingRequestPayload(payload: unknown): SyncPairingRequestPayload return null; } const runtimeHostGrant = toOptionalString(value?.runtimeHostGrant); + const pairingCommitVersion = value?.pairingCommitVersion === 1 ? 1 : null; return { code, peer: { @@ -1561,6 +1576,7 @@ function parsePairingRequestPayload(payload: unknown): SyncPairingRequestPayload ...(dpopPublicKey ? { dpopPublicKey } : {}), ...(relayAccountToken ? { relayAccountToken } : {}), ...(runtimeHostGrant ? { runtimeHostGrant } : {}), + ...(pairingCommitVersion ? { pairingCommitVersion } : {}), }; } @@ -2475,115 +2491,28 @@ export function createSyncHostService(args: SyncHostServiceArgs) { loadPersistedCommandLedger(); const lanePresenceByLaneId = new Map>(); let localActiveLaneIds = new Set(); - const PAIR_FAILURE_THRESHOLD = 5; - const PAIR_COOLDOWN_MS = 10 * 60_000; - const PAIR_FAILURE_WINDOW_MS = 10 * 60_000; - type PairFailureEntry = { count: number; cooldownUntilMs: number; updatedAtMs: number }; - const pairFailures = new Map(); - const globalPairFailures: PairFailureEntry = { count: 0, cooldownUntilMs: 0, updatedAtMs: 0 }; - const adoptChallengeIssuances = new Map(); - const globalAdoptChallengeIssuances: PairFailureEntry = { - count: 0, - cooldownUntilMs: 0, - updatedAtMs: 0, - }; - const resetPairFailureEntry = (entry: PairFailureEntry): void => { - entry.count = 0; - entry.cooldownUntilMs = 0; - entry.updatedAtMs = 0; + const pairFailureTracker = createPairFailureTracker(); + const registerPairFailure = (subject: PairFailureSubject): void => { + pairFailureTracker.registerFailure(subject); }; - const isPairFailureEntryExpired = (entry: PairFailureEntry, now: number): boolean => { - if (entry.updatedAtMs <= 0) return false; - return (entry.cooldownUntilMs > 0 && entry.cooldownUntilMs <= now) - || entry.updatedAtMs + PAIR_FAILURE_WINDOW_MS <= now; - }; - const pruneExpiredPairFailures = (now = Date.now()): boolean => { - let changed = false; - for (const [ip, entry] of pairFailures) { - if (isPairFailureEntryExpired(entry, now)) { - pairFailures.delete(ip); - changed = true; - } - } - if (isPairFailureEntryExpired(globalPairFailures, now)) { - resetPairFailureEntry(globalPairFailures); - changed = true; - } - return changed; - }; - const incrementPairFailureEntry = (entry: PairFailureEntry, now: number): void => { - entry.count += 1; - entry.updatedAtMs = now; - if (entry.count >= PAIR_FAILURE_THRESHOLD) { - entry.cooldownUntilMs = now + PAIR_COOLDOWN_MS; - entry.count = 0; - } - }; - const registerPairFailure = (ip: string | null): void => { - const now = Date.now(); - pruneExpiredPairFailures(now); - incrementPairFailureEntry(globalPairFailures, now); - if (ip) { - const entry = pairFailures.get(ip) ?? { count: 0, cooldownUntilMs: 0, updatedAtMs: now }; - incrementPairFailureEntry(entry, now); - pairFailures.set(ip, entry); - } - }; - const pairingCooldownMsRemaining = (ip: string | null): number => { - const now = Date.now(); - pruneExpiredPairFailures(now); - const globalRemaining = Math.max(0, globalPairFailures.cooldownUntilMs - now); - const ipEntry = ip ? pairFailures.get(ip) ?? null : null; - const ipRemaining = ipEntry ? Math.max(0, ipEntry.cooldownUntilMs - now) : 0; - return Math.max(globalRemaining, ipRemaining); - }; - const clearPairFailuresAfterSuccessfulPair = (ip: string | null): void => { - resetPairFailureEntry(globalPairFailures); - if (ip) pairFailures.delete(ip); - }; - const pruneExpiredAdoptChallengeIssuances = (now = Date.now()): void => { - for (const [ip, entry] of adoptChallengeIssuances) { - if (isPairFailureEntryExpired(entry, now)) { - adoptChallengeIssuances.delete(ip); - } - } - if (isPairFailureEntryExpired(globalAdoptChallengeIssuances, now)) { - resetPairFailureEntry(globalAdoptChallengeIssuances); - } + const pairingCooldownMsRemaining = (subject: PairFailureSubject): number => + pairFailureTracker.cooldownMsRemaining(subject); + const clearPairFailuresAfterSuccessfulPair = (subject: PairFailureSubject): void => { + pairFailureTracker.clearAfterSuccess(subject); }; - // Only called for malformed/anomalous challenges (genuine abuse signals); - // well-formed challenges deliberately feed no limiter. Counts both per-IP and - // globally so a distributed malformed flood is bounded as well as a single - // origin's. + // Only fed by malformed/anomalous challenges (genuine abuse signals); + // well-formed challenges deliberately feed no limiter. A separate tracker so + // a malformed-challenge flood cannot spend the PIN budget, or vice versa. + const adoptChallengeTracker = createPairFailureTracker(); const registerAdoptChallengeIssuance = (ip: string | null): void => { - const now = Date.now(); - pruneExpiredAdoptChallengeIssuances(now); - incrementPairFailureEntry(globalAdoptChallengeIssuances, now); - if (ip) { - const entry = adoptChallengeIssuances.get(ip) - ?? { count: 0, cooldownUntilMs: 0, updatedAtMs: now }; - incrementPairFailureEntry(entry, now); - adoptChallengeIssuances.set(ip, entry); - } - }; - const adoptChallengeCooldownMsRemaining = (ip: string | null): number => { - const now = Date.now(); - pruneExpiredAdoptChallengeIssuances(now); - const globalRemaining = Math.max( - 0, - globalAdoptChallengeIssuances.cooldownUntilMs - now, - ); - const ipEntry = ip ? adoptChallengeIssuances.get(ip) ?? null : null; - const ipRemaining = ipEntry - ? Math.max(0, ipEntry.cooldownUntilMs - now) - : 0; - return Math.max(globalRemaining, ipRemaining); + adoptChallengeTracker.registerFailure({ ip }); }; + const adoptChallengeCooldownMsRemaining = (ip: string | null): number => + adoptChallengeTracker.cooldownMsRemaining({ ip }); const clearAdoptChallengeIssuancesAfterSuccessfulAuth = ( ip: string | null, ): void => { - resetPairFailureEntry(globalAdoptChallengeIssuances); - if (ip) adoptChallengeIssuances.delete(ip); + adoptChallengeTracker.clearAfterSuccess({ ip }); }; const normalizeLaneId = (laneId: string | null | undefined): string | null => { @@ -2943,7 +2872,8 @@ export function createSyncHostService(args: SyncHostServiceArgs) { runPollPump(); }, pollIntervalMs); const heartbeatTimer = setInterval(() => { - pruneExpiredPairFailures(); + pairFailureTracker.pruneExpired(); + adoptChallengeTracker.pruneExpired(); const refreshedLocalPresence = refreshLocalLanePresence(); if (refreshedLocalPresence || pruneExpiredLanePresence()) { args.onStateChanged?.(); @@ -3251,6 +3181,9 @@ export function createSyncHostService(args: SyncHostServiceArgs) { authKind: null, pairedDeviceId: null, pairingRecord: null, + pairingCommitOfferedForDeviceId: null, + pendingPairingCommitDeviceId: null, + pendingPairingCommitSecret: null, connectedAt: nowIso(), lastSeenAt: nowIso(), framesReceived: 0, @@ -6590,8 +6523,10 @@ export function createSyncHostService(args: SyncHostServiceArgs) { return; } if (envelope.type === "account_challenge") { + // No device identity is available before a challenge, so this can only + // consult the address and global buckets. const cooldownMs = Math.max( - pairingCooldownMsRemaining(peer.remoteAddress), + pairingCooldownMsRemaining({ ip: peer.remoteAddress }), adoptChallengeCooldownMsRemaining(peer.remoteAddress), ); if (cooldownMs > 0) { @@ -6757,7 +6692,11 @@ export function createSyncHostService(args: SyncHostServiceArgs) { return; } } - const cooldownMs = pairingCooldownMsRemaining(peer.remoteAddress); + const pairFailureSubject: PairFailureSubject = { + ip: peer.remoteAddress, + deviceId: pairing.peer.deviceId, + }; + const cooldownMs = pairingCooldownMsRemaining(pairFailureSubject); if (cooldownMs > 0) { const minutes = Math.ceil(cooldownMs / 60_000); send(peer.ws, "pairing_result", { @@ -6799,7 +6738,7 @@ export function createSyncHostService(args: SyncHostServiceArgs) { allowDirectPinRuntimeHost: peer.transportOrigin !== "relay-bridge", }); closeExistingPeersForDevice(pairing.peer.deviceId, peer); - clearPairFailuresAfterSuccessfulPair(peer.remoteAddress); + clearPairFailuresAfterSuccessfulPair(pairFailureSubject); args.deviceRegistryService?.upsertPeerMetadata(pairing.peer, { lastSeenAt: nowIso(), lastHost: peer.remoteAddress, @@ -6809,7 +6748,22 @@ export function createSyncHostService(args: SyncHostServiceArgs) { ok: true, deviceId: result.deviceId, secret: result.secret, + // Advisory only. Present when this re-pair was staged behind the + // device's existing secret, which stays valid until a hello proves + // the replacement arrived. Clients that ignore it stay correct. + ...(result.pendingRotationExpiresAtMs != null + ? { + rotation: { + pendingCommit: true, + expiresInMs: Math.max(0, result.pendingRotationExpiresAtMs - Date.now()), + }, + } + : {}), }, envelope.requestId); + peer.pairingCommitOfferedForDeviceId = + result.pendingRotationExpiresAtMs != null && pairing.pairingCommitVersion === 1 + ? pairing.peer.deviceId + : null; } catch (error) { const message = error instanceof Error ? error.message : String(error); const thrownCode = (error as { code?: string } | null)?.code ?? null; @@ -6828,7 +6782,7 @@ export function createSyncHostService(args: SyncHostServiceArgs) { // PIN requires a new TCP+WS handshake per attempt, and track per-IP // failures so sustained guessers hit a cooldown. if (resultCode === "invalid_pin" || resultCode === "pairing_failed") { - registerPairFailure(peer.remoteAddress); + registerPairFailure(pairFailureSubject); } try { peer.ws.close(4003, "Pairing failed"); } catch { /* ignore */ } } @@ -6936,20 +6890,39 @@ export function createSyncHostService(args: SyncHostServiceArgs) { let relayAccountExpiresAtMs: number | null = null; let connectionAttemptReserved = false; let connectionAttemptRejected = false; + let pendingPairingCommitDeviceId: string | null = null; let authFailureCode: SyncHelloErrorPayload["code"] = "auth_failed"; + // Every rejection below used to arrive as this one string. The client + // renders whatever it is handed, so a bare "authentication failed" left + // the user with no idea whether to re-pair, sign in, update, or give up. + // Each `authFail(...)` names one cause and one next step; the generic + // wording survives only as the unreachable default. let authFailureMessage = "Sync authentication failed."; + const authFail = ( + message: string, + code?: SyncHelloErrorPayload["code"], + ): true => { + authFailureMessage = message; + if (code) authFailureCode = code; + return true; + }; + const REPAIR_REQUIRED = "This device is not paired with this machine, or its saved" + + " pairing is no longer valid. Pair it again."; + const ACCOUNT_SESSION_CHANGED = "The ADE account session on this machine changed" + + " while connecting. Try again."; // Return semantics: `true` means authentication FAILED -> the caller below - // sends a `hello_error` (auth_failed) and closes the socket (4003). - // `false` means the device is authenticated. + // sends a `hello_error` and closes the socket (4003). `false` means the + // device is authenticated. const authFailed = await (async () => { if (hello.auth?.kind === "bootstrap") { if (peer.transportOrigin === "relay-bridge") { - authFailureCode = "relay_account_required"; - authFailureMessage = "Sign in with the same ADE account on both machines."; args.logger.warn("sync_host.bootstrap_relay_rejected", { deviceId: hello.peer.deviceId, }); - return true; + return authFail( + "Sign in with the same ADE account on both machines.", + "relay_account_required", + ); } // The bootstrap token is a shared, plaintext, never-rotating secret. // Once the sync host is bound to the LAN (the new 0.0.0.0 default), @@ -6963,13 +6936,22 @@ export function createSyncHostService(args: SyncHostServiceArgs) { // which verifies the 6-digit PIN via pinStore. This preserves // legitimate already-paired reconnects (which use the token) while // forcing every new device through the PIN gate. - if (!safeStringEquals(bootstrapToken, hello.auth.token)) return true; + if (!safeStringEquals(bootstrapToken, hello.auth.token)) { + return authFail( + "This device's saved setup token does not match this machine. Pair it again.", + ); + } const bootstrapPairingRecord = pairingStore.getPairingRecord(hello.peer.deviceId); // A device whose pairing record carries a DPoP key must prove key // possession on every connection. Letting it in via the shared // bootstrap token would be a downgrade path: a stolen token plus a // spoofed deviceId would bypass the enclave binding entirely. - if (bootstrapPairingRecord?.dpopPublicKey) return true; + if (bootstrapPairingRecord?.dpopPublicKey) { + return authFail( + "This device must reconnect with its saved pairing secret and device key," + + " not the shared setup token. Pair it again.", + ); + } if (SYNC_HOST_BIND_LOOPBACK_ONLY) { // Loopback-only hosts (ADE_SYNC_BIND_HOST=127.0.0.1) are already a // trust boundary — only local processes can connect — so retain the @@ -6983,17 +6965,27 @@ export function createSyncHostService(args: SyncHostServiceArgs) { args.logger.warn("sync_host.dpop_required_bootstrap_rejected", { deviceId: hello.peer.deviceId, }); - return true; + return authFail( + "This machine requires every device to pair with its own security key." + + " Pair this device again to create one.", + ); } // LAN-bound default: bootstrap is reconnect-only. A device must already // be paired; unknown devices must pair via the PIN flow. Existing // paired phones from older releases may not have a host PIN configured // yet, and should still be able to reconnect with their stored token. - return bootstrapPairingRecord == null; + if (bootstrapPairingRecord == null) { + return authFail("This device is not paired with this machine. Pair it with a PIN first."); + } + return false; } if (hello.auth?.kind === "paired") { const pairedAuth = hello.auth; - if (pairedAuth.deviceId !== hello.peer.deviceId) return true; + if (pairedAuth.deviceId !== hello.peer.deviceId) { + return authFail( + "The pairing identity in this connection did not match the device that sent it.", + ); + } if (peer.transportOrigin === "relay-bridge") { const authorization = await captureAccountAuthorization(); if (!isPeerLifecycleCurrent(peer, lifecycleGeneration)) return true; @@ -7047,21 +7039,35 @@ export function createSyncHostService(args: SyncHostServiceArgs) { // nothing and the client showed a bare "authentication". Name it, // and tell the client the one thing that actually resolves it. const knownRecord = pairingStore.getPairingRecord(pairedAuth.deviceId); - if (!pairingStore.authenticate(pairedAuth.deviceId, pairedAuth.secret)) { + const presentedSecretState = pairingStore.verifySecret( + pairedAuth.deviceId, + pairedAuth.secret, + ); + const deferPendingCommit = + presentedSecretState === "pending" + && peer.pairingCommitOfferedForDeviceId === pairedAuth.deviceId; + if (!pairingStore.authenticate( + pairedAuth.deviceId, + pairedAuth.secret, + { deferPendingCommit }, + )) { // Deliberately identical for both cases. The host knows which it // is and logs it below, but telling an UNAUTHENTICATED caller // whether a device id exists here turns this into an existence // oracle, and the user's next step is the same either way. - authFailureMessage = "This device is not paired with this machine, or its saved" - + " pairing is no longer valid. Pair it again."; args.logger.warn("sync_host.paired_device_rejected", { deviceId: pairedAuth.deviceId, reason: knownRecord ? "secret_mismatch" : "unknown_device", }); - return true; + return authFail(REPAIR_REQUIRED); + } + authenticatedPairingRecord = pairingStore.getPairingRecordForSecret( + pairedAuth.deviceId, + pairedAuth.secret, + ); + if (!authenticatedPairingRecord) { + return authFail("This machine could not read its pairing record for this device. Pair it again."); } - authenticatedPairingRecord = pairingStore.getPairingRecord(pairedAuth.deviceId); - if (!authenticatedPairingRecord) return true; const pairingAccountOwner = toOptionalString(authenticatedPairingRecord.accountOwnerUserId); if (pairingAccountOwner) { const currentOwner = await refreshAccountLease(); @@ -7069,13 +7075,14 @@ export function createSyncHostService(args: SyncHostServiceArgs) { if (currentOwner !== pairingAccountOwner) { // A LAN client rejected here sees only "authentication"; the // account mismatch is the actionable part. - authFailureMessage = "This machine is signed in to a different ADE account than the one that paired this device."; args.logger.warn("sync_host.paired_account_owner_mismatch", { deviceId: pairedAuth.deviceId, hasCurrentOwner: Boolean(currentOwner), ownerMatches: false, }); - return true; + return authFail( + "This machine is signed in to a different ADE account than the one that paired this device.", + ); } } const dpopFailure = evaluatePairedHelloDpop({ @@ -7095,19 +7102,29 @@ export function createSyncHostService(args: SyncHostServiceArgs) { deviceId: pairedAuth.deviceId, reason: dpopFailure, }); - return true; + return authFail(syncDpopFailureMessage(dpopFailure)); } // evaluatePairedHelloDpop may TOFU-pin the first DPoP key for a // legacy keyless record. Reload it before installing Relay refresh; // otherwise this first socket advertises reauthorization while its // peer-local record still has no usable key. - authenticatedPairingRecord = pairingStore.getPairingRecord(pairedAuth.deviceId); - if (!authenticatedPairingRecord) return true; + authenticatedPairingRecord = pairingStore.getPairingRecordForSecret( + pairedAuth.deviceId, + pairedAuth.secret, + ); + if (!authenticatedPairingRecord) { + return authFail("This machine could not read its pairing record for this device. Pair it again."); + } + pendingPairingCommitDeviceId = deferPendingCommit ? pairedAuth.deviceId : null; return false; } if (hello.auth?.kind === "account") { const accountAuth = hello.auth; - if (accountAuth.deviceId !== hello.peer.deviceId) return true; + if (accountAuth.deviceId !== hello.peer.deviceId) { + return authFail( + "The account identity in this connection did not match the device that sent it.", + ); + } // Account bearer credentials must never traverse or authenticate a // plaintext direct sync route. Existing devices reconnect directly // with their stored paired secret + DPoP key instead. @@ -7116,7 +7133,10 @@ export function createSyncHostService(args: SyncHostServiceArgs) { deviceId: accountAuth.deviceId, transportOrigin: peer.transportOrigin, }); - return true; + return authFail( + "Signing in cannot authenticate a direct connection to this machine." + + " Connect through ADE Relay, or pair this device with a PIN.", + ); } try { const authorization = await captureAccountAuthorization(); @@ -7125,10 +7145,16 @@ export function createSyncHostService(args: SyncHostServiceArgs) { args.logger.warn("sync_host.account_owner_missing", { deviceId: accountAuth.deviceId, }); - return true; + return authFail( + "This machine is not signed in to an ADE account. Sign in on the Mac, then try again.", + ); } const config = args.getAccountAttestationConfig?.(); - if (!config) return true; + if (!config) { + return authFail( + "This machine cannot verify ADE accounts. Update ADE on the Mac, then try again.", + ); + } const attestation = await verifyAccountAttestation({ token: accountAuth.accountToken, expectedUserId: authorization.userId, @@ -7147,7 +7173,7 @@ export function createSyncHostService(args: SyncHostServiceArgs) { args.logger.warn("sync_host.account_auth_session_changed", { deviceId: accountAuth.deviceId, }); - return true; + return authFail(ACCOUNT_SESSION_CHANGED); } return await withHelloCommitLock(accountAuth.deviceId, async () => { if (!isPeerLifecycleCurrent(peer, lifecycleGeneration)) return true; @@ -7160,14 +7186,19 @@ export function createSyncHostService(args: SyncHostServiceArgs) { !lockedAuthorization || lockedAuthorization.userId !== authorization.userId || lockedAuthorization.generation !== authorization.generation - ) return true; + ) { + return authFail(ACCOUNT_SESSION_CHANGED); + } const existingPairingRecord = pairingStore.getPairingRecord(accountAuth.deviceId); if (existingPairingRecord && !existingPairingRecord.dpopPublicKey) { args.logger.warn("sync_host.account_existing_keyless_rejected", { deviceId: accountAuth.deviceId, }); - return true; + return authFail( + "This device's saved pairing predates device-key security." + + " Remove it on the Mac and pair it again.", + ); } const dpopFailure = evaluatePairedHelloDpop({ storedPublicKey: existingPairingRecord?.dpopPublicKey ?? null, @@ -7182,10 +7213,14 @@ export function createSyncHostService(args: SyncHostServiceArgs) { deviceId: accountAuth.deviceId, reason: dpopFailure, }); - return true; + return authFail(syncDpopFailureMessage(dpopFailure)); } const existingAccountOwner = toOptionalString(existingPairingRecord?.accountOwnerUserId); - if (existingAccountOwner && existingAccountOwner !== authorization.userId) return true; + if (existingAccountOwner && existingAccountOwner !== authorization.userId) { + return authFail( + "This device is already paired to this machine under a different ADE account.", + ); + } if (!arbitrateConnectionAttempt(hello.peer.deviceId, peer, hello.peer)) { connectionAttemptRejected = true; return false; @@ -7203,10 +7238,18 @@ export function createSyncHostService(args: SyncHostServiceArgs) { : accountAuth.dpop?.publicKey ?? null, runtimeHostGrant: accountAuth.runtimeHostGrant ?? null, }); - if (!pairingStore.authenticate(paired.deviceId, paired.secret)) return true; + // Read-only: this confirms the record we just wrote is readable, + // and must not be mistaken for the device proving it received the + // secret (which is what promotes a staged rotation). + if (!pairingStore.verifySecret(paired.deviceId, paired.secret)) { + return authFail("This machine could not save the new pairing for this device. Try again."); + } accountPairing = paired; authenticatedPairingRecord = pairingStore.getPairingRecord(paired.deviceId); - return authenticatedPairingRecord == null; + if (!authenticatedPairingRecord) { + return authFail("This machine could not save the new pairing for this device. Try again."); + } + return false; }); } catch (error) { args.logger.warn("sync_host.account_auth_rejected", { @@ -7215,10 +7258,15 @@ export function createSyncHostService(args: SyncHostServiceArgs) { ? (error as { code: string }).code : "verification_failed", }); - return true; + return authFail( + "This machine could not verify your ADE account session." + + " Sign out and back in on this device, then try again.", + ); } } - return true; + return authFail( + "This connection did not present any way to authenticate. Update ADE on this device.", + ); })(); if (!isPeerLifecycleCurrent(peer, lifecycleGeneration)) return; if (authFailed) { @@ -7271,6 +7319,10 @@ export function createSyncHostService(args: SyncHostServiceArgs) { const recordBackedAuth = auth.kind === "paired" || auth.kind === "account"; peer.pairedDeviceId = recordBackedAuth ? auth.deviceId : null; peer.pairingRecord = recordBackedAuth ? authenticatedPairingRecord : null; + peer.pendingPairingCommitDeviceId = pendingPairingCommitDeviceId; + peer.pendingPairingCommitSecret = pendingPairingCommitDeviceId + ? (auth.kind === "paired" ? auth.secret : null) + : null; installRelayAuthorization( peer, peer.transportOrigin === "relay-bridge" @@ -7361,6 +7413,43 @@ export function createSyncHostService(args: SyncHostServiceArgs) { return; } + if (envelope.type === "pairing_commit") { + const payload = safeObjectValue(envelope.payload); + const deviceId = toOptionalString(payload?.deviceId); + if (!deviceId || peer.pendingPairingCommitDeviceId !== deviceId) { + send(peer.ws, "pairing_commit_result", { + ok: false, + error: { + code: "no_pending_rotation", + message: "This connection has no staged pairing to commit.", + }, + }, envelope.requestId); + return; + } + const authenticatedSecret = peer.pendingPairingCommitSecret; + const committed = authenticatedSecret + ? pairingStore.commitPendingRotation(deviceId, authenticatedSecret) + : null; + if (!committed) { + peer.pendingPairingCommitDeviceId = null; + peer.pendingPairingCommitSecret = null; + send(peer.ws, "pairing_commit_result", { + ok: false, + error: { + code: "pairing_commit_failed", + message: "The staged pairing expired. Pair this device again.", + }, + }, envelope.requestId); + return; + } + peer.pairingRecord = committed; + peer.pendingPairingCommitDeviceId = null; + peer.pendingPairingCommitSecret = null; + peer.pairingCommitOfferedForDeviceId = null; + send(peer.ws, "pairing_commit_result", { ok: true }, envelope.requestId); + return; + } + if (isPairedRuntimeEnvelopeType(envelope.type)) { await pairedChannelService.handleEnvelope( peer.ws, diff --git a/apps/ade-cli/src/services/sync/syncPairFailureTracker.test.ts b/apps/ade-cli/src/services/sync/syncPairFailureTracker.test.ts new file mode 100644 index 000000000..90b1bcdd4 --- /dev/null +++ b/apps/ade-cli/src/services/sync/syncPairFailureTracker.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from "vitest"; +import { + createPairFailureTracker, + PAIR_COOLDOWN_MS, + PAIR_FAILURE_THRESHOLD, + PAIR_FAILURE_WINDOW_MS, + PAIR_GLOBAL_FAILURE_THRESHOLD, +} from "./syncPairFailureTracker"; + +function createClock(startMs = 1_000_000) { + let nowMs = startMs; + return { + now: () => nowMs, + advance(ms: number) { + nowMs += ms; + }, + }; +} + +describe("pair failure tracker", () => { + it("cools down the device that failed without touching anyone else", () => { + const clock = createClock(); + const tracker = createPairFailureTracker({ now: clock.now }); + + for (let attempt = 0; attempt < PAIR_FAILURE_THRESHOLD; attempt += 1) { + tracker.registerFailure({ ip: "192.168.1.20", deviceId: "phone-a" }); + } + + expect(tracker.cooldownMsRemaining({ ip: "192.168.1.20", deviceId: "phone-a" })) + .toBe(PAIR_COOLDOWN_MS); + // The regression this exists for: five fumbled PINs on ONE phone used to + // trip the global bucket and block pairing (and account adoption, which + // consults the same cooldown) for every other device in the house. + expect(tracker.cooldownMsRemaining({ ip: "10.0.0.9", deviceId: "phone-b" })).toBe(0); + expect(tracker.cooldownMsRemaining({})).toBe(0); + }); + + it("follows a device that moves to a new address", () => { + const clock = createClock(); + const tracker = createPairFailureTracker({ now: clock.now }); + + for (let attempt = 0; attempt < PAIR_FAILURE_THRESHOLD; attempt += 1) { + tracker.registerFailure({ ip: "192.168.1.20", deviceId: "phone-a" }); + } + + expect(tracker.cooldownMsRemaining({ ip: "172.16.0.4", deviceId: "phone-a" })) + .toBe(PAIR_COOLDOWN_MS); + }); + + it("still stops one address cycling through fresh device ids", () => { + const clock = createClock(); + const tracker = createPairFailureTracker({ now: clock.now }); + + for (let attempt = 0; attempt < PAIR_FAILURE_THRESHOLD; attempt += 1) { + tracker.registerFailure({ ip: "192.168.1.20", deviceId: `spoofed-${attempt}` }); + } + + expect(tracker.cooldownMsRemaining({ ip: "192.168.1.20", deviceId: "spoofed-99" })) + .toBe(PAIR_COOLDOWN_MS); + }); + + it("keeps a global breaker for guessing spread across many origins", () => { + const clock = createClock(); + const tracker = createPairFailureTracker({ now: clock.now }); + + for (let attempt = 0; attempt < PAIR_GLOBAL_FAILURE_THRESHOLD - 1; attempt += 1) { + tracker.registerFailure({ ip: `10.0.0.${attempt}`, deviceId: `device-${attempt}` }); + } + expect(tracker.cooldownMsRemaining({ ip: "10.0.1.1", deviceId: "innocent" })).toBe(0); + + tracker.registerFailure({ ip: "10.0.1.250", deviceId: "device-last" }); + + expect(tracker.cooldownMsRemaining({ ip: "10.0.1.1", deviceId: "innocent" })) + .toBe(PAIR_COOLDOWN_MS); + }); + + it("expires a cooldown and forgets the bucket once its window lapses", () => { + const clock = createClock(); + const tracker = createPairFailureTracker({ now: clock.now }); + const subject = { ip: "192.168.1.20", deviceId: "phone-a" }; + + for (let attempt = 0; attempt < PAIR_FAILURE_THRESHOLD; attempt += 1) { + tracker.registerFailure(subject); + } + clock.advance(PAIR_COOLDOWN_MS + 1); + + expect(tracker.cooldownMsRemaining(subject)).toBe(0); + + clock.advance(PAIR_FAILURE_WINDOW_MS + 1); + tracker.pruneExpired(); + for (let attempt = 0; attempt < PAIR_FAILURE_THRESHOLD - 1; attempt += 1) { + tracker.registerFailure(subject); + } + expect(tracker.cooldownMsRemaining(subject)).toBe(0); + }); + + it("clears the buckets a correct PIN was charged against", () => { + const clock = createClock(); + const tracker = createPairFailureTracker({ now: clock.now }); + const subject = { ip: "192.168.1.20", deviceId: "phone-a" }; + + for (let attempt = 0; attempt < PAIR_FAILURE_THRESHOLD - 1; attempt += 1) { + tracker.registerFailure(subject); + } + tracker.clearAfterSuccess(subject); + tracker.registerFailure(subject); + + expect(tracker.cooldownMsRemaining(subject)).toBe(0); + }); +}); diff --git a/apps/ade-cli/src/services/sync/syncPairFailureTracker.ts b/apps/ade-cli/src/services/sync/syncPairFailureTracker.ts new file mode 100644 index 000000000..b850d1020 --- /dev/null +++ b/apps/ade-cli/src/services/sync/syncPairFailureTracker.ts @@ -0,0 +1,140 @@ +/** + * Rate limiting for failed PIN pairing attempts. + * + * Failures are charged to three independent buckets: the requesting device, the + * requesting address, and one global circuit breaker. The per-device and + * per-address buckets are the ones that describe a real user fumbling a 6-digit + * code, so they keep the tight threshold. The global bucket used to share that + * same threshold, which meant a handful of bad PINs typed on ONE phone locked pairing + * (and account adoption, which consults the same cooldown) for every device in + * the house. It is a last-resort defence against guessing + * distributed across many addresses, so it sits far above the point where an + * honest household is still trying. + * + * A device id is self-asserted, so the per-device bucket alone would be trivial + * to evade by minting a new one per attempt. That is what the per-address + * bucket is for; the device bucket exists to make the cooldown land on the + * device that actually failed. + */ + +// Tuned for onboarding friendliness over strictness: a fumbling human gets ten +// tries and a two-minute wait, which still prices a 6-digit LAN brute force out +// of reach (worst case ~7 codes/min sustained against a 1e6 space). +export const PAIR_FAILURE_THRESHOLD = 10; +export const PAIR_GLOBAL_FAILURE_THRESHOLD = 50; +export const PAIR_COOLDOWN_MS = 2 * 60_000; +export const PAIR_FAILURE_WINDOW_MS = 10 * 60_000; + +export type PairFailureEntry = { + count: number; + cooldownUntilMs: number; + updatedAtMs: number; +}; + +/** Who a pairing attempt came from. Both parts are optional and untrusted. */ +export type PairFailureSubject = { + ip?: string | null; + deviceId?: string | null; +}; + +export type PairFailureTracker = ReturnType; + +type PairFailureTrackerOptions = { + now?: () => number; +}; + +const normalize = (value: string | null | undefined): string | null => { + const trimmed = value?.trim(); + return trimmed ? trimmed : null; +}; + +export function createPairFailureTracker(options: PairFailureTrackerOptions = {}) { + const now = options.now ?? (() => Date.now()); + // Namespaced so an address can never collide with a device id. + const buckets = new Map(); + const globalFailures: PairFailureEntry = { + count: 0, + cooldownUntilMs: 0, + updatedAtMs: 0, + }; + + const keysFor = (subject: PairFailureSubject): string[] => { + const keys: string[] = []; + const ip = normalize(subject.ip); + const deviceId = normalize(subject.deviceId); + if (ip) keys.push(`ip:${ip}`); + if (deviceId) keys.push(`device:${deviceId}`); + return keys; + }; + + const reset = (entry: PairFailureEntry): void => { + entry.count = 0; + entry.cooldownUntilMs = 0; + entry.updatedAtMs = 0; + }; + + const expired = (entry: PairFailureEntry, nowMs: number): boolean => { + if (entry.updatedAtMs <= 0) return false; + return (entry.cooldownUntilMs > 0 && entry.cooldownUntilMs <= nowMs) + || entry.updatedAtMs + PAIR_FAILURE_WINDOW_MS <= nowMs; + }; + + const prune = (nowMs: number): void => { + for (const [key, entry] of buckets) { + if (expired(entry, nowMs)) buckets.delete(key); + } + if (expired(globalFailures, nowMs)) reset(globalFailures); + }; + + const increment = (entry: PairFailureEntry, nowMs: number, threshold: number): void => { + entry.count += 1; + entry.updatedAtMs = nowMs; + if (entry.count >= threshold) { + entry.cooldownUntilMs = nowMs + PAIR_COOLDOWN_MS; + entry.count = 0; + } + }; + + return { + /** + * Drops entries whose window has lapsed. Every other method prunes as it + * goes; this exists so a host that is idle for hours does not hold buckets + * for devices that stopped failing long ago. + */ + pruneExpired(): void { + prune(now()); + }, + + cooldownMsRemaining(subject: PairFailureSubject): number { + const nowMs = now(); + prune(nowMs); + let remaining = Math.max(0, globalFailures.cooldownUntilMs - nowMs); + for (const key of keysFor(subject)) { + const entry = buckets.get(key); + if (!entry) continue; + remaining = Math.max(remaining, entry.cooldownUntilMs - nowMs); + } + return Math.max(0, remaining); + }, + + registerFailure(subject: PairFailureSubject): void { + const nowMs = now(); + prune(nowMs); + increment(globalFailures, nowMs, PAIR_GLOBAL_FAILURE_THRESHOLD); + for (const key of keysFor(subject)) { + const entry = buckets.get(key) ?? { count: 0, cooldownUntilMs: 0, updatedAtMs: nowMs }; + increment(entry, nowMs, PAIR_FAILURE_THRESHOLD); + buckets.set(key, entry); + } + }, + + /** + * A correct PIN proves this attempt was legitimate, so it clears the + * buckets it was charged against and relieves the global breaker. + */ + clearAfterSuccess(subject: PairFailureSubject): void { + reset(globalFailures); + for (const key of keysFor(subject)) buckets.delete(key); + }, + }; +} diff --git a/apps/ade-cli/src/services/sync/syncPairingStore.test.ts b/apps/ade-cli/src/services/sync/syncPairingStore.test.ts index 54a17e829..5d83ea541 100644 --- a/apps/ade-cli/src/services/sync/syncPairingStore.test.ts +++ b/apps/ade-cli/src/services/sync/syncPairingStore.test.ts @@ -1,9 +1,9 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import type { SyncPeerMetadata } from "../../../../desktop/src/shared/types"; -import { createSyncPairingStore } from "./syncPairingStore"; +import { createSyncPairingStore, PAIRING_ROTATION_WINDOW_MS } from "./syncPairingStore"; import { createSyncPinStore } from "./syncPinStore"; const VALID_DPOP_PUBLIC_KEY = Buffer.concat([ @@ -129,3 +129,269 @@ describe("sync SSH pairing trust", () => { expect(store.getPairingRecord(peer.deviceId)?.runtimeHostGranted).toBe(expected); }); }); + +describe("PIN re-pair staged rotation", () => { + const roots: string[] = []; + + afterEach(() => { + vi.useRealTimers(); + for (const root of roots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + const PIN = "428193"; + + function createStore() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "ade-rotation-")); + roots.push(root); + const pinStore = createSyncPinStore({ filePath: path.join(root, "pin.json") }); + pinStore.setPin(PIN); + return createSyncPairingStore({ filePath: path.join(root, "paired.json"), pinStore }); + } + + const peer = { + deviceId: "iphone-rotation", + deviceName: "Arul's iPhone", + platform: "iOS", + deviceType: "phone", + siteId: "iphone-rotation-site", + dbVersion: 0, + } satisfies SyncPeerMetadata; + + it("commits a first-time pair immediately, with nothing staged", () => { + const store = createStore(); + const first = store.pairPeer(peer, PIN); + + expect(first.pendingRotationExpiresAtMs).toBeNull(); + expect(store.hasPendingRotation(peer.deviceId)).toBe(false); + expect(store.authenticate(peer.deviceId, first.secret)).toBe(true); + }); + + // THE regression test for this bug class: the connection dies between + // `pairing_result` and `hello`, exactly where the old code had already + // destroyed the working secret. Both secrets must work on the next attempt. + it("keeps a previously working secret alive when a re-pair is never proven", () => { + const store = createStore(); + const original = store.pairPeer(peer, PIN); + const rotated = store.pairPeer(peer, PIN); + + expect(rotated.secret).not.toBe(original.secret); + expect(rotated.pendingRotationExpiresAtMs).toBeGreaterThan(Date.now()); + // The device that dropped mid-rotation still holds the original. + expect(store.verifySecret(peer.deviceId, original.secret)).toBe("committed"); + // The device that DID save the replacement is equally able to connect. + expect(store.verifySecret(peer.deviceId, rotated.secret)).toBe("pending"); + expect(store.authenticate(peer.deviceId, original.secret)).toBe(true); + }); + + it("promotes the staged secret the first time a hello authenticates with it", () => { + const store = createStore(); + const original = store.pairPeer(peer, PIN); + const rotated = store.pairPeer(peer, PIN); + + expect(store.authenticate(peer.deviceId, rotated.secret)).toBe(true); + + expect(store.hasPendingRotation(peer.deviceId)).toBe(false); + expect(store.authenticate(peer.deviceId, original.secret)).toBe(false); + expect(store.authenticate(peer.deviceId, rotated.secret)).toBe(true); + }); + + it("keeps both secrets live until a commit-capable client acknowledges hello_ok", () => { + const store = createStore(); + const original = store.pairPeer(peer, PIN); + const rotated = store.pairPeer( + { ...peer, deviceName: "Committed iPhone" }, + PIN, + { dpopPublicKey: VALID_DPOP_PUBLIC_KEY }, + ); + + expect(store.authenticate( + peer.deviceId, + rotated.secret, + { deferPendingCommit: true }, + )).toBe(true); + expect(store.getPairingRecordForSecret(peer.deviceId, rotated.secret)).toMatchObject({ + peerName: "Committed iPhone", + dpopPublicKey: VALID_DPOP_PUBLIC_KEY, + }); + expect(store.verifySecret(peer.deviceId, original.secret)).toBe("committed"); + expect(store.verifySecret(peer.deviceId, rotated.secret)).toBe("pending"); + + expect(store.commitPendingRotation(peer.deviceId, rotated.secret)).toMatchObject({ + peerName: "Committed iPhone", + dpopPublicKey: VALID_DPOP_PUBLIC_KEY, + }); + expect(store.verifySecret(peer.deviceId, original.secret)).toBeNull(); + expect(store.verifySecret(peer.deviceId, rotated.secret)).toBe("committed"); + }); + + it("refuses an explicit commit after its staged rotation expires", () => { + vi.useFakeTimers(); + const store = createStore(); + const original = store.pairPeer(peer, PIN); + const rotated = store.pairPeer(peer, PIN); + + expect(store.authenticate( + peer.deviceId, + rotated.secret, + { deferPendingCommit: true }, + )).toBe(true); + vi.advanceTimersByTime(PAIRING_ROTATION_WINDOW_MS + 1_000); + + expect(store.commitPendingRotation(peer.deviceId, rotated.secret)).toBeNull(); + expect(store.verifySecret(peer.deviceId, original.secret)).toBe("committed"); + expect(store.verifySecret(peer.deviceId, rotated.secret)).toBeNull(); + }); + + it("does not let a stale socket commit a newer staged rotation", () => { + const store = createStore(); + const original = store.pairPeer(peer, PIN); + const stale = store.pairPeer(peer, PIN); + expect(store.authenticate( + peer.deviceId, + stale.secret, + { deferPendingCommit: true }, + )).toBe(true); + + const newer = store.pairPeer(peer, PIN); + + expect(store.commitPendingRotation(peer.deviceId, stale.secret)).toBeNull(); + expect(store.verifySecret(peer.deviceId, original.secret)).toBe("committed"); + expect(store.verifySecret(peer.deviceId, stale.secret)).toBeNull(); + expect(store.verifySecret(peer.deviceId, newer.secret)).toBe("pending"); + }); + + it("reverts to the committed secret once the rotation window lapses", () => { + vi.useFakeTimers(); + const store = createStore(); + const original = store.pairPeer(peer, PIN); + const rotated = store.pairPeer(peer, PIN); + + vi.advanceTimersByTime(PAIRING_ROTATION_WINDOW_MS + 1_000); + + expect(store.authenticate(peer.deviceId, rotated.secret)).toBe(false); + expect(store.authenticate(peer.deviceId, original.secret)).toBe(true); + expect(store.hasPendingRotation(peer.deviceId)).toBe(false); + }); + + it("keeps at most one outstanding rotation and never chains off an unproven one", () => { + const store = createStore(); + const original = store.pairPeer(peer, PIN); + const firstRetry = store.pairPeer(peer, PIN); + const secondRetry = store.pairPeer(peer, PIN); + + expect(store.verifySecret(peer.deviceId, original.secret)).toBe("committed"); + expect(store.verifySecret(peer.deviceId, firstRetry.secret)).toBeNull(); + expect(store.verifySecret(peer.deviceId, secondRetry.secret)).toBe("pending"); + }); + + it("does not let a read-only verification promote the staged secret", () => { + const store = createStore(); + const original = store.pairPeer(peer, PIN); + const rotated = store.pairPeer(peer, PIN); + + expect(store.verifySecret(peer.deviceId, rotated.secret)).toBe("pending"); + + expect(store.hasPendingRotation(peer.deviceId)).toBe(true); + expect(store.authenticate(peer.deviceId, original.secret)).toBe(true); + }); + + it("hides the staged record from every authorization read", () => { + const store = createStore(); + store.pairPeer(peer, PIN); + const rotated = store.pairPeer( + { ...peer, deviceName: "Renamed iPhone" }, + PIN, + { dpopPublicKey: VALID_DPOP_PUBLIC_KEY }, + ); + + // The committed record is what every gate must read until the device + // proves it has the replacement: the staged secret and the DPoP binding it + // carries stay invisible. Descriptive fields are not credentials, so a + // rename lands immediately rather than waiting on the acknowledgement. + const beforeCommit = store.getPairingRecord(peer.deviceId); + expect(beforeCommit?.peerName).toBe("Renamed iPhone"); + expect(beforeCommit?.dpopPublicKey ?? null).toBeNull(); + expect(beforeCommit).not.toHaveProperty("pendingRotation"); + expect(store.authenticate(peer.deviceId, rotated.secret)).toBe(true); + + store.authenticate(peer.deviceId, rotated.secret); + + const afterCommit = store.getPairingRecord(peer.deviceId); + expect(afterCommit?.peerName).toBe("Renamed iPhone"); + expect(afterCommit?.dpopPublicKey).toBe(VALID_DPOP_PUBLIC_KEY); + }); + + // Only PIN pairing crosses the wire twice more before the device can save + // what it was given. The in-process trust paths hand their secret straight + // back to the caller, so they commit immediately — and supersede a staged + // rotation nobody claimed rather than chaining off it. + it("commits immediately on the in-process trust paths and clears a stale staged rotation", () => { + const store = createStore(); + const original = store.pairPeer(peer, PIN); + const stranded = store.pairPeer(peer, PIN); + expect(store.hasPendingRotation(peer.deviceId)).toBe(true); + + const local = store.pairPeerViaLocalTrust(peer); + + expect(local.pendingRotationExpiresAtMs).toBeNull(); + expect(store.hasPendingRotation(peer.deviceId)).toBe(false); + expect(store.verifySecret(peer.deviceId, local.secret)).toBe("committed"); + expect(store.verifySecret(peer.deviceId, stranded.secret)).toBeNull(); + expect(store.verifySecret(peer.deviceId, original.secret)).toBeNull(); + }); +}); + +describe("staged re-pair privilege direction", () => { + const roots: string[] = []; + + afterEach(() => { + for (const root of roots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + const PIN = "428193"; + + function createStore() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "ade-rotation-privilege-")); + roots.push(root); + const pinStore = createSyncPinStore({ filePath: path.join(root, "pin.json") }); + pinStore.setPin(PIN); + return createSyncPairingStore({ filePath: path.join(root, "paired.json"), pinStore }); + } + + const desktopPeer = { + deviceId: "desktop-rotation", + deviceName: "MacBook Pro", + platform: "macOS", + deviceType: "desktop", + siteId: "desktop-rotation-site", + dbVersion: 0, + } satisfies SyncPeerMetadata; + + // A staged rotation must never park a privilege the re-pair took away: the + // committed record is what every gate reads, so a late withdrawal is a leak. + it("withdraws a runtime-host grant immediately even though the secret stages", () => { + const store = createStore(); + store.pairPeer(desktopPeer, PIN, { allowDirectPinRuntimeHost: true }); + expect(store.getPairingRecord(desktopPeer.deviceId)?.runtimeHostGranted).toBe(true); + + // Re-pairing over Relay cannot authorize a runtime host. + const rotated = store.pairPeer(desktopPeer, PIN, { allowDirectPinRuntimeHost: false }); + + expect(store.hasPendingRotation(desktopPeer.deviceId)).toBe(true); + expect(store.getPairingRecord(desktopPeer.deviceId)?.runtimeHostGranted).toBe(false); + expect(store.authenticate(desktopPeer.deviceId, rotated.secret)).toBe(true); + expect(store.getPairingRecord(desktopPeer.deviceId)?.runtimeHostGranted).toBe(false); + }); + + it("keeps an existing grant while a re-pair that would elevate is unproven", () => { + const store = createStore(); + store.pairPeer(desktopPeer, PIN, { allowDirectPinRuntimeHost: true }); + store.pairPeer(desktopPeer, PIN, { allowDirectPinRuntimeHost: true }); + + expect(store.getPairingRecord(desktopPeer.deviceId)?.runtimeHostGranted).toBe(true); + }); +}); diff --git a/apps/ade-cli/src/services/sync/syncPairingStore.ts b/apps/ade-cli/src/services/sync/syncPairingStore.ts index 3b4905009..fc6b84dac 100644 --- a/apps/ade-cli/src/services/sync/syncPairingStore.ts +++ b/apps/ade-cli/src/services/sync/syncPairingStore.ts @@ -28,8 +28,28 @@ export type SyncPairingRecord = { * null are deliberately local/manual for backward compatibility. */ accountOwnerUserId?: string | null; + /** + * A PIN re-pair that the device has not proven it received. See + * `writeNewPairingRecord`: the fields above stay live and authoritative until + * a legacy hello promotes the staged secret or a commit-capable client + * acknowledges hello_ok. + */ + pendingRotation?: PendingPairingRotation | null; +}; + +/** A pairing record as committed on disk, with no rotation staged behind it. */ +export type CommittedPairingRecord = Omit; + +export type PendingPairingRotation = { + /** Wall-clock ms after which the staged record is abandoned. */ + expiresAtMs: number; + /** The complete record that replaces the committed one when promoted. */ + record: CommittedPairingRecord; }; +/** How long a staged re-pair waits for the device to prove it received it. */ +export const PAIRING_ROTATION_WINDOW_MS = 10 * 60_000; + type PairingSecretsFile = Record; type RuntimeHostGrantFile = Record; @@ -48,6 +68,20 @@ type NewPairingRecordOptions = { allowDirectPinRuntimeHost?: boolean; }; +type AuthenticatePairingOptions = { + /** + * Accept a staged secret for this hello without retiring the committed one. + * The authenticated socket must call `commitPendingRotation` after hello_ok. + */ + deferPendingCommit?: boolean; +}; + +type NewPairingResult = { + deviceId: string; + secret: string; + pendingRotationExpiresAtMs: number | null; +}; + type PairingTrust = | { kind: "pin" } | { kind: "local" } @@ -143,11 +177,35 @@ export function createSyncPairingStore(args: SyncPairingStoreArgs) { return granted; }; + /** + * Drops a staged rotation that nobody claimed. Returns the record to use, or + * null when `records[deviceId]` is absent. Callers persist when it mutates. + */ + const pruneExpiredRotation = ( + record: SyncPairingRecord | undefined, + nowMs: number, + ): { record: SyncPairingRecord | null; changed: boolean } => { + if (!record) return { record: null, changed: false }; + const pending = record.pendingRotation; + if (!pending) return { record, changed: false }; + if (pending.expiresAtMs > nowMs) return { record, changed: false }; + // The device never proved it received the replacement, so the committed + // secret it is still holding stays authoritative. + const { pendingRotation: _dropped, ...committed } = record; + return { record: { ...committed, pendingRotation: null }, changed: true }; + }; + + const committedView = (record: SyncPairingRecord): SyncPairingRecord => { + const { pendingRotation: _pending, ...committed } = record; + return committed; + }; + const writeNewPairingRecord = ( peer: SyncPeerMetadata, options?: NewPairingRecordOptions, trust: PairingTrust = { kind: "pin" }, - ): { deviceId: string; secret: string } => { + stageRotation = false, + ): NewPairingResult => { // Consume every presented grant to preserve its one-time semantics. A // direct LAN/tailnet PIN may authorize a desktop runtime host explicitly; // Relay PIN pairing never gets that exception. Verified same-owner account @@ -158,6 +216,9 @@ export function createSyncPairingStore(args: SyncPairingStoreArgs) { // mobile allowlist even when they authenticate with the same account. const secret = randomBytes(24).toString("hex"); const records = readRecords(); + // Always decide against the COMMITTED record. A second re-pair arriving + // while a rotation is still staged replaces that staged record; it never + // chains off it, so at most one rotation is ever outstanding. const existing = records[peer.deviceId] ?? null; const existingAccountOwnerUserId = normalizeAccountOwnerUserId(existing?.accountOwnerUserId); let accountOwnerUserId: string | null = null; @@ -194,7 +255,7 @@ export function createSyncPairingStore(args: SyncPairingStoreArgs) { const dpopPublicKey = trust.kind === "account" && existing?.dpopPublicKey ? existing.dpopPublicKey : validatedOfferedDpopKey ?? existing?.dpopPublicKey ?? null; - records[peer.deviceId] = { + const replacement: CommittedPairingRecord = { secretHash: hashSecret(secret), createdAt: existing?.createdAt ?? nowIso(), lastUsedAt: null, @@ -209,10 +270,48 @@ export function createSyncPairingStore(args: SyncPairingStoreArgs) { // record. A legacy record with no field remains local until rewritten. accountOwnerUserId, }; + // Re-pairing used to overwrite the record the instant the host answered, + // two round trips before the device could persist the reply. A drop in that + // gap left the host holding credentials the device never saw, and the only + // way out was walking back to the Mac for another PIN. So a re-pair now + // STAGES: the committed secret the device is still holding stays live, and + // the replacement waits for a legacy hello or a commit-capable client's + // post-hello acknowledgement. Nothing to promote means nothing was lost — + // the staged record simply expires. A first-time pair has no committed + // secret to protect and is written straight through. + if (stageRotation && existing) { + const expiresAtMs = Date.now() + PAIRING_ROTATION_WINDOW_MS; + records[peer.deviceId] = { + ...committedView(existing), + // Descriptive fields are not credentials, so a phone that was renamed + // shows its new name immediately instead of after a handshake the user + // cannot see. + peerName: replacement.peerName, + peerPlatform: replacement.peerPlatform, + peerDeviceType: replacement.peerDeviceType, + // Elevations wait for proof; reductions must not. Staging a PIN + // re-pair's declassification would leave the record account-owned, so + // the next account switch would revoke the very pairing the user just + // re-established at the Mac. Same reasoning for a lost runtime-host + // grant: withdrawing authority late is a privilege leak, withdrawing it + // early costs at most one re-grant. + accountOwnerUserId: replacement.accountOwnerUserId === null + ? null + : existing.accountOwnerUserId ?? null, + runtimeHostGranted: replacement.runtimeHostGranted === false + ? false + : existing.runtimeHostGranted, + pendingRotation: { expiresAtMs, record: replacement }, + }; + writeRecords(records); + return { deviceId: peer.deviceId, secret, pendingRotationExpiresAtMs: expiresAtMs }; + } + records[peer.deviceId] = replacement; writeRecords(records); return { deviceId: peer.deviceId, secret, + pendingRotationExpiresAtMs: null, }; }; @@ -231,21 +330,34 @@ export function createSyncPairingStore(args: SyncPairingStoreArgs) { return token; }, - pairPeer(peer: SyncPeerMetadata, pin: string, options?: NewPairingRecordOptions): { deviceId: string; secret: string } { + /** + * PIN pairing over the wire. This is the only entry point that stages its + * rotation: it is the one whose credential crosses two more round trips + * (pairing_result, then hello) before the device can persist it, and the + * only one whose recovery costs the user a walk back to the Mac. Account + * adoption returns its secret inside `hello_ok` with no acknowledged + * follow-up, so staging it would strand devices that never send another + * hello; local OS/SSH trust hands the secret back in-process. + */ + pairPeer( + peer: SyncPeerMetadata, + pin: string, + options?: NewPairingRecordOptions, + ): NewPairingResult { if (!args.pinStore.hasPin()) { throw pairingError("pin_not_set", "No pairing PIN is set on this computer."); } if (!args.pinStore.verifyPin(pin)) { throw pairingError("invalid_pin", "Incorrect pairing PIN."); } - return writeNewPairingRecord(peer, options, { kind: "pin" }); + return writeNewPairingRecord(peer, options, { kind: "pin" }, true); }, pairPeerViaAccount( peer: SyncPeerMetadata, attestation: VerifiedAccountAttestation, options?: NewPairingRecordOptions, - ): { deviceId: string; secret: string } { + ): NewPairingResult { if (!isVerifiedAccountAttestation(attestation)) { throw pairingError("account_not_verified", "Account attestation was not verified."); } @@ -265,7 +377,7 @@ export function createSyncPairingStore(args: SyncPairingStoreArgs) { pairPeerViaLocalTrust( peer: SyncPeerMetadata, options?: NewPairingRecordOptions, - ): { deviceId: string; secret: string } { + ): NewPairingResult { return writeNewPairingRecord(peer, options, { kind: "local" }); }, @@ -287,22 +399,130 @@ export function createSyncPairingStore(args: SyncPairingStoreArgs) { return true; }, - authenticate(deviceId: string, secret: string): boolean { + /** + * Authenticates a device. Legacy clients promote a staged secret when their + * hello proves possession. Commit-capable clients defer that promotion until + * their post-hello `pairing_commit`, keeping both secrets valid across a + * dropped hello_ok. + */ + authenticate( + deviceId: string, + secret: string, + options: AuthenticatePairingOptions = {}, + ): boolean { const normalized = deviceId.trim(); if (!normalized) return false; const records = readRecords(); - const entry = records[normalized]; + const pruned = pruneExpiredRotation(records[normalized], Date.now()); + const entry = pruned.record; if (!entry) return false; - if (!safeHashEquals(entry.secretHash, hashSecret(secret))) return false; - entry.lastUsedAt = nowIso(); + const presented = hashSecret(secret); + if (safeHashEquals(entry.secretHash, presented)) { + records[normalized] = { ...entry, lastUsedAt: nowIso() }; + writeRecords(records); + return true; + } + const pending = entry.pendingRotation; + if (pending && safeHashEquals(pending.record.secretHash, presented)) { + if (options.deferPendingCommit) { + records[normalized] = { + ...entry, + pendingRotation: { + ...pending, + record: { ...pending.record, lastUsedAt: nowIso() }, + }, + }; + } else { + records[normalized] = { ...pending.record, lastUsedAt: nowIso() }; + } + writeRecords(records); + return true; + } + if (pruned.changed) { + records[normalized] = entry; + writeRecords(records); + } + return false; + }, + + /** + * Returns the record whose secret authenticated this socket. Unlike + * `getPairingRecord`, this may expose the staged replacement to the one + * connection that proved it, while every unrelated authorization read + * continues to see only the committed record. + */ + getPairingRecordForSecret(deviceId: string, secret: string): SyncPairingRecord | null { + const normalized = deviceId.trim(); + if (!normalized) return null; + const entry = pruneExpiredRotation(readRecords()[normalized], Date.now()).record; + if (!entry) return null; + const presented = hashSecret(secret); + if (safeHashEquals(entry.secretHash, presented)) return committedView(entry); + const pending = entry.pendingRotation; + return pending && safeHashEquals(pending.record.secretHash, presented) + ? { ...pending.record } + : null; + }, + + /** + * Promotes the replacement only after a socket authenticated with it and + * received hello_ok. The host owns that session check; the store makes the + * mutation atomic and refuses absent/expired rotations. + */ + commitPendingRotation(deviceId: string, authenticatedSecret: string): SyncPairingRecord | null { + const normalized = deviceId.trim(); + if (!normalized) return null; + const records = readRecords(); + const pruned = pruneExpiredRotation(records[normalized], Date.now()); + const pending = pruned.record?.pendingRotation; + const presented = hashSecret(authenticatedSecret); + if (!pending || !safeHashEquals(pending.record.secretHash, presented)) { + if (pruned.changed && pruned.record) { + records[normalized] = pruned.record; + writeRecords(records); + } + return null; + } + records[normalized] = { ...pending.record, lastUsedAt: nowIso() }; writeRecords(records); - return true; + return committedView(records[normalized]!); + }, + + /** + * Read-only counterpart of `authenticate` for callers that only need to + * confirm a secret they just minted is on record. It must not promote: + * writing a record is not the device receiving it. + */ + verifySecret(deviceId: string, secret: string): "committed" | "pending" | null { + const normalized = deviceId.trim(); + if (!normalized) return null; + const entry = pruneExpiredRotation(readRecords()[normalized], Date.now()).record; + if (!entry) return null; + const presented = hashSecret(secret); + if (safeHashEquals(entry.secretHash, presented)) return "committed"; + const pending = entry.pendingRotation; + if (pending && safeHashEquals(pending.record.secretHash, presented)) return "pending"; + return null; }, + /** Whether an unproven re-pair is still waiting on this device. */ + hasPendingRotation(deviceId: string): boolean { + const normalized = deviceId.trim(); + if (!normalized) return false; + const entry = pruneExpiredRotation(readRecords()[normalized], Date.now()).record; + return Boolean(entry?.pendingRotation); + }, + + /** + * The committed record. A staged rotation is deliberately not visible here: + * every authorization decision (DPoP key, account owner, runtime grant) has + * to read what is live right now, not what a device might promote later. + */ getPairingRecord(deviceId: string): SyncPairingRecord | null { const normalized = deviceId.trim(); if (!normalized) return null; - return readRecords()[normalized] ?? null; + const entry = pruneExpiredRotation(readRecords()[normalized], Date.now()).record; + return entry ? committedView(entry) : null; }, hasPairingRecord(deviceId: string): boolean { diff --git a/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts b/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts index 660d2e6f5..887b2d121 100644 --- a/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts +++ b/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts @@ -591,15 +591,14 @@ export class DesktopPairedMachineStore { // can never match again — one orphaned, still-valid secret per re-pair, // forever. // - // Known tradeoff: because the host upserts on this id, `pairPeer` rotates - // the secret and DPoP binding for an EXISTING record before it answers, and - // this client only persists the replacement after `hello_ok`. A drop in - // between leaves the host holding credentials the desktop never saved, so a - // previously working pairing needs one more manual re-pair. Minting a fresh - // id instead would avoid that window at the cost of the unbounded orphaned- - // record leak this reuse exists to stop — strictly worse. Closing it - // properly needs an atomic commit/ack in the pairing protocol, which is a - // host-side change and not something to land in a merge loop. + // Reusing the id used to mean a re-pair destroyed the working secret before + // this client could persist the replacement (which it only does after + // `hello_ok`), so a drop in between needed another manual re-pair. The host + // now stages a re-pair instead: the existing secret stays live and the + // replacement is promoted by the first hello that authenticates with it, so + // a drop in that window leaves BOTH ends still holding the old, still-valid + // secret. See `writeNewPairingRecord` in + // apps/ade-cli/src/services/sync/syncPairingStore.ts. // // The hello that reports the host identity arrives after the pairing request // that carries this id, so the prior record has to be recovered up front: diff --git a/apps/desktop/src/shared/types/sync.ts b/apps/desktop/src/shared/types/sync.ts index 823a67315..163fd458a 100644 --- a/apps/desktop/src/shared/types/sync.ts +++ b/apps/desktop/src/shared/types/sync.ts @@ -993,6 +993,12 @@ export type SyncPairingQrPayload = { export type SyncPairingRequestPayload = { code: string; peer: SyncPeerMetadata; + /** + * Version 1 clients persist a replacement secret before hello and explicitly + * commit it after hello_ok. Older hosts ignore this field; older clients omit + * it and retain the host's hello-as-commit compatibility path. + */ + pairingCommitVersion?: 1; /** * Short-lived account bearer presented only when this request crosses ADE * Relay. The host verifies that it belongs to its current signed-in user; @@ -1016,6 +1022,16 @@ export type SyncPairingResultPayload = { ok: boolean; deviceId?: string; secret?: string; + /** + * Advisory. Present only when this was a RE-pair that the host staged behind + * the device's existing secret. Commit-capable clients keep that older secret + * valid until `pairing_commit` follows hello_ok; legacy clients use + * hello-as-commit. Hosts predating staged rotation never send this field. + */ + rotation?: { + pendingCommit: true; + expiresInMs: number; + }; error?: { code: | "invalid_pin" @@ -1026,6 +1042,18 @@ export type SyncPairingResultPayload = { }; }; +export type SyncPairingCommitPayload = { + deviceId: string; +}; + +export type SyncPairingCommitResultPayload = { + ok: boolean; + error?: { + code: "no_pending_rotation" | "pairing_commit_failed"; + message: string; + }; +}; + export type SyncChangesetBatchPayload = { batchId: string; reason: "catchup" | "broadcast" | "relay"; @@ -1831,6 +1859,8 @@ export type SyncProjectListMyGitHubReposRequestEnvelope = SyncEnvelopeWithPayloa export type SyncProjectListMyGitHubReposResultEnvelope = SyncEnvelopeWithPayload<"project_list_my_github_repos_result", SyncProjectListMyGitHubReposResultPayload>; export type SyncPairingRequestEnvelope = SyncEnvelopeWithPayload<"pairing_request", SyncPairingRequestPayload>; export type SyncPairingResultEnvelope = SyncEnvelopeWithPayload<"pairing_result", SyncPairingResultPayload>; +export type SyncPairingCommitEnvelope = SyncEnvelopeWithPayload<"pairing_commit", SyncPairingCommitPayload>; +export type SyncPairingCommitResultEnvelope = SyncEnvelopeWithPayload<"pairing_commit_result", SyncPairingCommitResultPayload>; export type SyncChangesetBatchEnvelope = SyncEnvelopeWithPayload<"changeset_batch", SyncChangesetBatchPayload>; export type SyncInvalidationBatchEnvelope = SyncEnvelopeWithPayload<"invalidation_batch", SyncInvalidationBatchPayload>; export type SyncChangesetAckEnvelope = SyncEnvelopeWithPayload<"changeset_ack", SyncChangesetAckPayload>; @@ -1906,6 +1936,8 @@ export type SyncEnvelope = | SyncProjectListMyGitHubReposResultEnvelope | SyncPairingRequestEnvelope | SyncPairingResultEnvelope + | SyncPairingCommitEnvelope + | SyncPairingCommitResultEnvelope | SyncChangesetBatchEnvelope | SyncInvalidationBatchEnvelope | SyncChangesetAckEnvelope diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 3d250bc43..3427034f6 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -443,6 +443,56 @@ struct AccountAdoptionIdentityVerificationError: LocalizedError, Equatable { } } +/// A route this build cannot negotiate -- today, a host that named an adoption +/// cipher this client does not implement. That is a version gap, not evidence +/// the Mac is an impostor, so it fails only its own route: another route (or +/// another host build) may negotiate fine, and the user needs "update", not a +/// security warning. The unsupported cipher itself is still never used. +struct AccountAdoptionRouteCompatibilityError: LocalizedError, Equatable { + let machineName: String + + var errorDescription: String? { + "\(machineName) offered a security cipher this version of ADE doesn't support. Update ADE on both devices." + } +} + +/// One-time repair of a persisted auto-reconnect pause. The flag survives app +/// updates and blocks reconnecting forever, and builds that set it as FAILURE +/// fallout are why the app could open on a dead "Disconnected" screen. A pause +/// written by a user action records its source alongside it, so a paused flag +/// with no source is provably fallout and is cleared. +func syncAutoReconnectPausedAfterMigration(paused: Bool, pauseSource: String?) -> Bool { + guard paused else { return false } + return pauseSource?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == false +} + +/// Whether an adoption failure ends the whole attempt or only its own route. +/// Identity verification, a changed authorization, and a superseded attempt all +/// describe the ATTEMPT; everything else describes one route. +func syncAccountAdoptionFailureIsFatal(_ error: Error) -> Bool { + error is AccountAdoptionIdentityVerificationError + || error is AccountPairingAuthorizationChangedError + || error is AccountPairingConnectionSupersededError +} + +/// Old hosts omit `rotation`; new hosts advertise it only when they staged an +/// existing credential and can consume the post-hello commit. +func syncPairingCommitRequired(_ payload: [String: Any]) -> Bool { + guard let rotation = payload["rotation"] as? [String: Any] else { return false } + return (rotation["pendingCommit"] as? Bool) == true +} + +/// Keep the ordering contract testable without exposing Keychain internals. +/// Persisting first means a dropped hello_ok cannot leave the host committed to +/// a secret this phone never saved. +func syncPersistPairingBeforeHello( + persist: () -> Void, + hello: () async throws -> Void +) async rethrows { + persist() + try await hello() +} + private struct AccountAdoptionRoutesExhaustedError: LocalizedError { let machineName: String let routeLabels: [String] @@ -3103,6 +3153,10 @@ final class SyncService: ObservableObject { private let profilesKey = "ade.sync.hostProfiles" private let legacyDeviceIdKey = "ade.sync.deviceId" private let autoReconnectPausedKey = "ade.sync.autoReconnectPausedByUser" + /// Records that a paused flag came from a user action rather than from a + /// failed attempt. See the migration in `init`. + private let autoReconnectPauseSourceKey = "ade.sync.autoReconnectPauseSource" + private let autoReconnectPauseSourceUser = "user" private let lastProjectRouteIdKey = "ade.sync.lastProjectRoute.projectId" private let lastProjectRouteSavedAtKey = "ade.sync.lastProjectRoute.savedAt" private let lastProjectRouteWorkSessionIdKey = "ade.sync.lastProjectRoute.workSessionId" @@ -4503,13 +4557,24 @@ final class SyncService: ObservableObject { self.socketSessionDelegate = socketSessionDelegate self.socketSession = socketSession self.database = database - // One-time migration: builds before the chunked-envelope transport set - // this flag on "message too long" transport errors and it survives app - // updates, silently blocking auto-reconnect forever. Current builds only - // set it for a real user pause, so clear the stale value once. - let pausedFlagMigrationKey = "ade.sync.autoReconnectPausedMigratedV2" + // This flag survives app updates and blocks auto-reconnect forever, so a + // build that ever set it as FAILURE fallout leaves users opening the app on + // a dead "Disconnected" screen with a manual Reconnect button. A V2 + // migration cleared it once, but the failure paths kept re-latching it, so + // clearing alone was never going to hold. It is now written only by an + // explicit user pause, which stamps `autoReconnectPauseSourceKey` + // alongside it -- so a paused flag with no source is provably fallout from + // an older build and is cleared. The V3 one-shot means a genuine pause + // taken on this build is never swept later. + let pausedFlagMigrationKey = "ade.sync.autoReconnectPausedMigratedV3" if !UserDefaults.standard.bool(forKey: pausedFlagMigrationKey) { - UserDefaults.standard.removeObject(forKey: autoReconnectPausedKey) + let keepsPause = syncAutoReconnectPausedAfterMigration( + paused: UserDefaults.standard.bool(forKey: autoReconnectPausedKey), + pauseSource: UserDefaults.standard.string(forKey: autoReconnectPauseSourceKey) + ) + if !keepsPause { + UserDefaults.standard.removeObject(forKey: autoReconnectPausedKey) + } UserDefaults.standard.set(true, forKey: pausedFlagMigrationKey) } self.autoReconnectPausedByUser = UserDefaults.standard.bool(forKey: autoReconnectPausedKey) @@ -5316,6 +5381,112 @@ final class SyncService: ObservableObject { return true } + /// One racing adoption candidate's private socket. Adoption used to run on + /// the service's single shared socket, which is exactly why it could only try + /// one route at a time. + private struct AdoptionCandidateTransport { + let task: URLSessionWebSocketTask + let mailbox: SyncConnectionRaceTextMailbox + } + + private struct AdoptedConnectionCandidate: @unchecked Sendable { + var scheduled: SyncConnectionRaceScheduledCandidate + var route: AccountAdoptionRoute + var task: URLSessionWebSocketTask + var helloPayload: [String: Any] + var negotiatedReadyV2: Bool + } + + private enum AdoptedConnectionRaceEvent: @unchecked Sendable { + case adopted(AdoptedConnectionCandidate) + case failed(candidateId: Int, error: Error) + case budgetExpired + } + + /// Carries the race bookkeeping the caller needs out through a `throw`. + private struct AccountAdoptionRaceFailure: Error { + let underlying: Error + let relayRouteFailed: Bool + } + + /// Request/response over ONE candidate socket. Mirrors what `awaitResponse` + /// does for the shared socket, minus the shared state: frames arrive through + /// this candidate's mailbox, so a losing candidate can never resolve a + /// winner's request. + private func adoptionCandidateRequest( + _ transport: AdoptionCandidateTransport, + type: String, + payload: [String: Any], + timeoutNanoseconds: UInt64, + timeoutMessage: String, + relayAccountOwnerId: String? + ) async throws -> Any { + let requestId = makeRequestId() + let text = try encodedCandidateEnvelope(type: type, requestId: requestId, payload: payload) + try await transport.task.send(.string(text)) + let deadlineUptime = ProcessInfo.processInfo.systemUptime + + TimeInterval(timeoutNanoseconds) / 1_000_000_000 + while true { + let event = try await nextRelayNegotiationEvent( + mailbox: transport.mailbox, + deadlineUptime: deadlineUptime + ) + guard case .frame(let frame) = event else { + throw NSError( + domain: "ADE", + code: 35, + userInfo: [NSLocalizedDescriptionKey: timeoutMessage] + ) + } + if let object = try? JSONSerialization.jsonObject(with: Data(frame.utf8)), + syncRelayTransportControl(from: object) != nil { + continue + } + guard let preprocessed = try await Task.detached(priority: .userInitiated, operation: { + try syncPreprocessIncoming(frame) + }).value else { continue } + guard preprocessed.requestId == requestId else { continue } + switch preprocessed.type { + case "account_challenge_ok", "hello_ok": + return preprocessed.payload + case "account_challenge_error": + let message = syncNonEmpty((preprocessed.payload as? [String: Any])?["message"] as? String) + ?? "That route could not verify the Mac's identity." + throw NSError( + domain: "ADE.AdoptChannel", + code: 6, + userInfo: [NSLocalizedDescriptionKey: message] + ) + case "hello_error": + throw adoptionCandidateHelloError( + preprocessed.payload, + relayAccountOwnerId: relayAccountOwnerId + ) + default: + continue + } + } + } + + private func adoptionCandidateHelloError( + _ payload: Any, + relayAccountOwnerId: String? + ) -> Error { + let errorPayload = payload as? [String: Any] + let code = (errorPayload?["code"] as? String) ?? "auth_failed" + let message = (errorPayload?["message"] as? String) ?? "Authentication failed." + if code == "relay_account_required" { + return syncRelayAuthorizationRequirementForHostRejection( + relayAccountOwnerId: relayAccountOwnerId, + currentAccountOwnerId: AccountService.shared.identity?.userId + ) + } + return NSError(domain: "ADE", code: 5, userInfo: [ + NSLocalizedDescriptionKey: message, + "ADEErrorCode": code, + ]) + } + private struct AccountAdoptionChallengeSession { let key: SymmetricKey let hostDeviceId: String @@ -5406,6 +5577,7 @@ final class SyncService: ObservableObject { } private func performAccountAdoptionChallenge( + transport: AdoptionCandidateTransport, expectedHostIdentity: String, signingPublicKey: Curve25519.Signing.PublicKey, machineName: String, @@ -5420,20 +5592,19 @@ final class SyncService: ObservableObject { let nonce = Data(nonceBytes) let nonceBase64 = nonce.base64EncodedString() let clientEphemeralPublicKey = clientPrivateKey.publicKey.rawRepresentation.base64EncodedString() - let requestId = makeRequestId() - let raw = try await awaitResponse( - requestId: requestId, - disconnectOnTimeout: false, - timeoutMessage: "That Mac did not answer the secure identity challenge.", - timeoutNanoseconds: AdoptChannelCrypto.challengeTimeoutNanoseconds - ) { - self.sendEnvelope(type: "account_challenge", requestId: requestId, payload: [ + let raw = try await adoptionCandidateRequest( + transport, + type: "account_challenge", + payload: [ "v": 1, "nonce": nonceBase64, "clientEphemeralPublicKey": clientEphemeralPublicKey, "supportedAeads": AdoptChannelCrypto.supportedAeads.map(\.rawValue), - ]) - } + ], + timeoutNanoseconds: AdoptChannelCrypto.challengeTimeoutNanoseconds, + timeoutMessage: "That Mac did not answer the secure identity challenge.", + relayAccountOwnerId: nil + ) guard isCurrentCandidate() else { throw AccountPairingConnectionSupersededError() } @@ -5456,7 +5627,7 @@ final class SyncService: ObservableObject { do { aead = try AdoptChannelCrypto.resolveHostAead(responseAead) } catch { - throw AccountAdoptionIdentityVerificationError(machineName: machineName) + throw AccountAdoptionRouteCompatibilityError(machineName: machineName) } let timestampValue = timestampNumber.doubleValue // `Double(Int64.max)` rounds up to 2^63, so `<=` would admit a value that @@ -5502,6 +5673,7 @@ final class SyncService: ObservableObject { } private func performAccountAdoptionHello( + transport: AdoptionCandidateTransport, route: AccountAdoptionRoute, expectedHostIdentity: String, signingPublicKey: Curve25519.Signing.PublicKey?, @@ -5509,6 +5681,7 @@ final class SyncService: ObservableObject { owner: String, authorization: AccountPairingAuthorization, generation: UInt64, + connectionAttempt: SyncConnectionAttemptMetadata, isCurrentCandidate: @escaping () -> Bool ) async throws -> Any { guard isCurrentConnectAttempt(generation) else { @@ -5521,6 +5694,7 @@ final class SyncService: ObservableObject { } publishAccountConnectStage("Verifying it's really \(machineName)…") challenge = try await performAccountAdoptionChallenge( + transport: transport, expectedHostIdentity: expectedHostIdentity, signingPublicKey: signingPublicKey, machineName: machineName, @@ -5583,16 +5757,17 @@ final class SyncService: ObservableObject { auth = unsealedAuth } - let requestId = makeRequestId() - let raw = try await awaitResponse( - requestId: requestId, - timeoutMessage: "That Mac did not finish account connection. Try again." - ) { - self.sendEnvelope(type: "hello", requestId: requestId, payload: [ - "peer": self.currentPeerMetadata(), + let raw = try await adoptionCandidateRequest( + transport, + type: "hello", + payload: [ + "peer": self.currentPeerMetadata(connectionAttempt: connectionAttempt), "auth": auth, - ]) - } + ], + timeoutNanoseconds: SyncConnectionRaceTiming.overallBudgetNanoseconds, + timeoutMessage: "That Mac did not finish account connection. Try again.", + relayAccountOwnerId: owner + ) guard isCurrentCandidate() else { throw AccountPairingConnectionSupersededError() } @@ -5621,6 +5796,317 @@ final class SyncService: ObservableObject { } } + /// Dials one adoption candidate on its own socket and runs the full + /// challenge + hello on it. Relay candidates keep the `?ready=2` negotiation + /// and one legacy redial, and hold the same single-flight dial key as the + /// paired path so two tunnels never open on one Durable Object. + private func adoptionCandidateAttempt( + _ candidate: SyncConnectionRaceScheduledCandidate, + route: AccountAdoptionRoute, + expectedHostIdentity: String, + signingPublicKey: Curve25519.Signing.PublicKey?, + machineName: String, + owner: String, + authorization: AccountPairingAuthorization, + connectAttemptGeneration: UInt64, + connectionAttempt: SyncConnectionAttemptMetadata + ) async throws -> AdoptedConnectionCandidate { + guard isCurrentConnectAttempt(connectAttemptGeneration) else { throw CancellationError() } + let endpoint = candidate.endpoint + let isRelay = syncIsFullWebSocketRoute(endpoint.address) + let parsed = syncParseRouteEndpoint(endpoint.address) + let socketHost = parsed?.host ?? endpoint.address.trimmingCharacters(in: .whitespacesAndNewlines) + let socketPort = parsed?.port ?? endpoint.port + let urlHost = parsed?.scheme == nil ? socketHost : endpoint.address + guard let rawURLString = syncWebSocketURLString(host: urlHost, port: socketPort) else { + throw NSError(domain: "ADE", code: 1, userInfo: [NSLocalizedDescriptionKey: "Invalid machine address."]) + } + let legacyURLString = isRelay + ? syncRelayCorrelatedURL( + syncRelayLegacyURL(rawURLString), + correlationID: connectionAttempt.id + ) + : rawURLString + guard let legacyURL = URL(string: legacyURLString) else { + throw NSError(domain: "ADE", code: 1, userInfo: [NSLocalizedDescriptionKey: "Invalid machine address."]) + } + + if isRelay { + guard let readyV2URL = URL(string: syncRelayReadyV2URL(legacyURLString)) else { + throw NSError(domain: "ADE", code: 1, userInfo: [NSLocalizedDescriptionKey: "Invalid machine address."]) + } + do { + return try await adoptionCandidateSocket( + candidate, + route: route, + url: readyV2URL, + isRelay: true, + awaitsRelayReadyV2: true, + expectedHostIdentity: expectedHostIdentity, + signingPublicKey: signingPublicKey, + machineName: machineName, + owner: owner, + authorization: authorization, + connectAttemptGeneration: connectAttemptGeneration, + connectionAttempt: connectionAttempt + ) + } catch SyncRelayReadyNegotiationError.retryLegacySocket { + guard isCurrentConnectAttempt(connectAttemptGeneration), !Task.isCancelled else { + throw CancellationError() + } + // Adoption has no saved profile yet, so there is no per-endpoint memory + // of a previous ready-v2 handshake: a silent window here is always the + // first meeting with this endpoint and earns its one legacy redial. + syncConnectLog.info( + "ADE_SYNC_TRACE adoption relay candidate did not negotiate ready-v2; retrying on a legacy socket" + ) + } + } + + return try await adoptionCandidateSocket( + candidate, + route: route, + url: legacyURL, + isRelay: isRelay, + awaitsRelayReadyV2: false, + expectedHostIdentity: expectedHostIdentity, + signingPublicKey: signingPublicKey, + machineName: machineName, + owner: owner, + authorization: authorization, + connectAttemptGeneration: connectAttemptGeneration, + connectionAttempt: connectionAttempt + ) + } + + private func adoptionCandidateSocket( + _ candidate: SyncConnectionRaceScheduledCandidate, + route: AccountAdoptionRoute, + url: URL, + isRelay: Bool, + awaitsRelayReadyV2: Bool, + expectedHostIdentity: String, + signingPublicKey: Curve25519.Signing.PublicKey?, + machineName: String, + owner: String, + authorization: AccountPairingAuthorization, + connectAttemptGeneration: UInt64, + connectionAttempt: SyncConnectionAttemptMetadata + ) async throws -> AdoptedConnectionCandidate { + let dialKey = isRelay ? syncRelayMachineKey(from: url.absoluteString) : nil + var dialToken: UUID? + if let dialKey { + guard let token = relayDialRegistry.acquire(dialKey) else { + throw SyncRelayDialInFlight() + } + dialToken = token + } + defer { + if let dialKey, let dialToken { relayDialRegistry.release(dialKey, token: dialToken) } + } + let candidateTask = socketSession.webSocketTask(with: url) + candidateTask.maximumMessageSize = 32 * 1_024 * 1_024 + return try await withTaskCancellationHandler { + do { + try await awaitSocketOpen(candidateTask) + guard isCurrentConnectAttempt(connectAttemptGeneration), !Task.isCancelled else { + throw CancellationError() + } + let mailbox = SyncConnectionRaceTextMailbox() + let reader = Task { + do { + while !Task.isCancelled { + let message = try await candidateTask.receive() + let text: String + switch message { + case .string(let value): text = value + case .data(let data): text = String(decoding: data, as: UTF8.self) + @unknown default: text = "" + } + await mailbox.deliver(text) + if self.isCandidateHelloTerminalFrame(text) { return } + } + } catch { + await mailbox.finish(message: error.localizedDescription) + } + } + defer { reader.cancel() } + + if awaitsRelayReadyV2 { + try await awaitRelayCandidateReady(mailbox: mailbox) + } + guard isCurrentConnectAttempt(connectAttemptGeneration), !Task.isCancelled else { + throw CancellationError() + } + guard AccountService.shared.isPairingCommitAuthorized(authorization) else { + throw AccountPairingAuthorizationChangedError() + } + + let transport = AdoptionCandidateTransport(task: candidateTask, mailbox: mailbox) + let isCurrentCandidate = { [weak self] in + guard let self else { return false } + return self.isCurrentConnectAttempt(connectAttemptGeneration) && !Task.isCancelled + } + let raw = try await performAccountAdoptionHello( + transport: transport, + route: route, + expectedHostIdentity: expectedHostIdentity, + signingPublicKey: signingPublicKey, + machineName: machineName, + owner: owner, + authorization: authorization, + generation: connectAttemptGeneration, + connectionAttempt: connectionAttempt, + isCurrentCandidate: isCurrentCandidate + ) + guard let helloPayload = raw as? [String: Any] else { + throw AccountAdoptionIdentityVerificationError(machineName: machineName) + } + return AdoptedConnectionCandidate( + scheduled: candidate, + route: route, + task: candidateTask, + helloPayload: helloPayload, + negotiatedReadyV2: awaitsRelayReadyV2 + ) + } catch { + candidateTask.cancel(with: .goingAway, reason: nil) + throw error + } + } onCancel: { + candidateTask.cancel(with: .goingAway, reason: nil) + } + } + + /// Races every adoption route at once, the way the paired reconnect path + /// already does. The serial version paid a socket open plus a 3s identity + /// challenge per route before moving on, so on cellular — where no direct + /// route can ever succeed — the user watched 10-20s of dead time before relay + /// was even dialed. + private func raceAccountAdoptionCandidates( + _ candidates: [SyncConnectionRaceScheduledCandidate], + routesByEndpoint: [SyncConnectionEndpointAttempt: AccountAdoptionRoute], + expectedHostIdentity: String, + signingPublicKey: Curve25519.Signing.PublicKey?, + machineName: String, + owner: String, + authorization: AccountPairingAuthorization, + connectAttemptGeneration: UInt64 + ) async throws -> AdoptedConnectionCandidate { + guard !candidates.isEmpty else { throw noConnectableAddressError() } + let connectionAttempt = makeConnectionAttemptMetadata() + var relayRouteFailed = false + let triedRouteLabels = candidates + .compactMap { routesByEndpoint[$0.endpoint]?.attemptLabel } + .reduce(into: [String]()) { labels, label in + if !labels.contains(label) { labels.append(label) } + } + func fail(_ error: Error) -> AccountAdoptionRaceFailure { + AccountAdoptionRaceFailure(underlying: error, relayRouteFailed: relayRouteFailed) + } + + return try await withThrowingTaskGroup(of: AdoptedConnectionRaceEvent.self) { group in + var scheduler = SyncConnectionRaceWaveScheduler(candidates: candidates) + func start(_ candidate: SyncConnectionRaceScheduledCandidate, in group: inout ThrowingTaskGroup) { + guard let route = routesByEndpoint[candidate.endpoint] else { return } + group.addTask { @MainActor [weak self] in + guard let self else { return .failed(candidateId: candidate.id, error: CancellationError()) } + do { + if candidate.delayNanoseconds > 0 { + try await Task.sleep(nanoseconds: candidate.delayNanoseconds) + } + return .adopted(try await self.adoptionCandidateAttempt( + candidate, + route: route, + expectedHostIdentity: expectedHostIdentity, + signingPublicKey: signingPublicKey, + machineName: machineName, + owner: owner, + authorization: authorization, + connectAttemptGeneration: connectAttemptGeneration, + connectionAttempt: connectionAttempt + )) + } catch { + return .failed(candidateId: candidate.id, error: error) + } + } + } + for candidate in scheduler.startInitialCandidates() { start(candidate, in: &group) } + group.addTask { + do { + try await Task.sleep(nanoseconds: SyncConnectionRaceTiming.overallBudgetNanoseconds) + return .budgetExpired + } catch { + return .failed(candidateId: -1, error: error) + } + } + + var ownership = SyncConnectionRaceOwnership(candidateIds: Set(candidates.map(\.id))) + var lastFailure: Error? + func drainAndCancel(_ group: inout ThrowingTaskGroup) async throws { + group.cancelAll() + while let lateEvent = try await group.next() { + if case .adopted(let lateCandidate) = lateEvent { + lateCandidate.task.cancel(with: .goingAway, reason: nil) + } + } + } + + while let event = try await group.next() { + switch event { + case .adopted(let candidate): + switch ownership.authenticated(candidateId: candidate.scheduled.id) { + case .acceptWinner: + try await drainAndCancel(&group) + return candidate + default: + candidate.task.cancel(with: .goingAway, reason: nil) + } + case .failed(let candidateId, let error): + lastFailure = error + if routesByEndpoint[candidates.first(where: { $0.id == candidateId })?.endpoint + ?? SyncConnectionEndpointAttempt(address: "", port: 0)]?.kind == .relay { + relayRouteFailed = true + } + syncConnectLog.info( + "ADE_SYNC_TRACE adoption race failure error=\(syncLogErrorSummary(error), privacy: .public)" + ) + // An impostor host, a revoked authorization, or a superseded attempt + // all describe the ATTEMPT. Everything else — including a route this + // build cannot negotiate — only describes its own route. + if syncAccountAdoptionFailureIsFatal(error) { + try await drainAndCancel(&group) + throw fail(error) + } + if ownership.failed(candidateId: candidateId) == .exhausted { + group.cancelAll() + throw fail(AccountAdoptionRoutesExhaustedError( + machineName: machineName, + routeLabels: triedRouteLabels, + finalFailure: lastFailure + )) + } + if let nextCandidate = scheduler.candidateFinished(candidateId) { + start(nextCandidate, in: &group) + } + case .budgetExpired: + _ = ownership.expireBudget() + try await drainAndCancel(&group) + throw fail(AccountAdoptionRoutesExhaustedError( + machineName: machineName, + routeLabels: triedRouteLabels, + finalFailure: lastFailure + )) + } + } + throw fail(AccountAdoptionRoutesExhaustedError( + machineName: machineName, + routeLabels: triedRouteLabels, + finalFailure: lastFailure + )) + } + } + /// Connects a directory machine using the signed-in Clerk session. The /// bearer token is used over the directory-verified WSS relay to mint the /// same device-bound paired secret used by QR/PIN/SSH. Later direct reconnects @@ -5746,138 +6232,122 @@ final class SyncService: ObservableObject { connectionState = .connecting markConnectAttemptStarted(generation) - var lastFailure: Error? - var triedRouteLabels: [String] = [] - for route in routes { - var candidateSocket: URLSessionWebSocketTask? - let routeAttemptStartedAt = ProcessInfo.processInfo.systemUptime - publishAccountConnectStage(route.stageLabel) - if !triedRouteLabels.contains(route.attemptLabel) { - triedRouteLabels.append(route.attemptLabel) - } - do { - guard AccountService.shared.isPairingCommitAuthorized(authorization) else { - throw AccountPairingAuthorizationChangedError() + publishAccountConnectStage("Connecting to \(machine.displayName)…") + let routesByEndpoint = Dictionary( + routes.map { ($0.endpoint, $0) }, + uniquingKeysWith: { first, _ in first } + ) + // The same planner the paired reconnect path uses: direct candidates + // lead, the relay joins the SAME race behind them by a short delay rather + // than waiting for them to exhaust a 10s budget, and the whole race is + // bounded once. + let racePlan = syncConnectionRaceCandidatePlan( + rankedAttempts: routes.map(\.endpoint) + ) + let raceStartedAt = ProcessInfo.processInfo.systemUptime + let winner: AdoptedConnectionCandidate + do { + winner = try await raceAccountAdoptionCandidates( + racePlan, + routesByEndpoint: routesByEndpoint, + expectedHostIdentity: expectedHostIdentity, + signingPublicKey: signingPublicKey, + machineName: machine.displayName, + owner: owner, + authorization: authorization, + connectAttemptGeneration: generation + ) + } catch let failure as AccountAdoptionRaceFailure { + relayRouteFailed = failure.relayRouteFailed + throw failure.underlying + } + guard isCurrentConnectAttempt(generation) else { + winner.task.cancel(with: .goingAway, reason: nil) + throw AccountPairingConnectionSupersededError() + } + let route = winner.route + // Only the winner touches app state. Its socket becomes the service + // socket before the hello payload is applied, exactly as the paired race + // installs its own winner. + teardownSocket(closeCode: .goingAway) + socket = winner.task + if syncIsFullWebSocketRoute(route.endpoint.address) { + relayTransportNegotiations[winner.task.taskIdentifier] = SyncRelayReadyNegotiation() + } + receiveLoop(for: winner.task) + _ = try await performAuthorizedAccountPairingCommit( + authorization: authorization, + receiveHello: { winner.helloPayload }, + prepare: { raw in + guard let payload = raw as? [String: Any], + let brain = payload["brain"] as? [String: Any], + self.syncNonEmpty(brain["deviceId"] as? String) == expectedHostIdentity else { + throw AccountAdoptionIdentityVerificationError(machineName: machine.displayName) } - try await openSocket( - host: route.endpoint.address, - port: route.endpoint.port, - connectAttemptGeneration: generation - ) - guard let openedSocket = socket else { + let pairing = payload["accountPairing"] as? [String: Any] + guard self.syncNonEmpty(pairing?["deviceId"] as? String) == self.deviceId, + let pairedSecret = self.syncNonEmpty(pairing?["secret"] as? String) else { throw NSError( domain: "ADE", - code: 31, - userInfo: [NSLocalizedDescriptionKey: "That Mac connection closed before account verification began."] + code: 33, + userInfo: [NSLocalizedDescriptionKey: "The Mac did not return saved connection details. Remove this iPhone from the Mac and try again."] ) } - candidateSocket = openedSocket - let isCurrentCandidate = { - self.isCurrentConnectAttempt(generation) && self.socket === openedSocket - } - let raw = try await performAccountAdoptionHello( - route: route, - expectedHostIdentity: expectedHostIdentity, - signingPublicKey: signingPublicKey, - machineName: machine.displayName, - owner: owner, - authorization: authorization, - generation: generation, - isCurrentCandidate: isCurrentCandidate - ) - _ = try await performAuthorizedAccountPairingCommit( - authorization: authorization, - receiveHello: { raw }, - prepare: { raw in - guard let payload = raw as? [String: Any], - let brain = payload["brain"] as? [String: Any], - self.syncNonEmpty(brain["deviceId"] as? String) == expectedHostIdentity else { - throw AccountAdoptionIdentityVerificationError(machineName: machine.displayName) - } - let pairing = payload["accountPairing"] as? [String: Any] - guard self.syncNonEmpty(pairing?["deviceId"] as? String) == self.deviceId, - let pairedSecret = self.syncNonEmpty(pairing?["secret"] as? String) else { - throw NSError( - domain: "ADE", - code: 33, - userInfo: [NSLocalizedDescriptionKey: "The Mac did not return saved connection details. Remove this iPhone from the Mac and try again."] - ) - } - let advertisedRelay = self.syncNonEmpty(payload["cloudRelayWssUrl"] as? String) - let allRelays = self.deduplicatedAddresses(relayRoutes + (advertisedRelay.map { [$0] } ?? [])) - let profile = HostConnectionProfile( - hostIdentity: expectedHostIdentity, - hostName: self.syncNonEmpty(brain["deviceName"] as? String) ?? machine.displayName, - siteId: self.syncNonEmpty(brain["siteId"] as? String), - port: preferredDirectPort ?? route.endpoint.port, - authKind: "paired", - pairedDeviceId: self.deviceId, - lastRemoteDbVersion: 0, - lastHostDeviceId: expectedHostIdentity, - lastSuccessfulAddress: route.endpoint.address, - savedAddressCandidates: directHosts, - discoveredLanAddresses: lanHosts, - tailscaleAddress: tailnetHosts.first, - savedRelayCandidates: allRelays, - accountOwnerId: owner, - relayAccountOwnerId: owner - ) - return (payload: payload, pairedSecret: pairedSecret, profile: profile) - }, - isAuthorized: { candidate in - AccountService.shared.isPairingCommitAuthorized(candidate) - }, - isCurrentCandidate: isCurrentCandidate, - commit: { prepared in - self.keychain.saveToken(prepared.pairedSecret) - if let key = self.profileStorageKey(prepared.profile) { - self.keychain.saveToken(prepared.pairedSecret, hostKey: key) - } - self.saveProfile(prepared.profile) - try self.applyHelloPayload( - prepared.payload, - connectedHost: route.endpoint.address, - port: prepared.profile.port, - authKind: "paired", - pairedDeviceId: self.deviceId, - expectedHostIdentity: expectedHostIdentity, - connectAttemptGeneration: generation - ) - } - ) - lastConnectedRouteKind = route.connectionRouteKind - schedulePostHelloWork(for: generation) - Task { await PushNotificationService.shared.enableIfPaired() } - LiveActivityService.shared.start() - accountPairingPinFallbackHost = nil - publishAccountConnectSuccess( - route: route, - attemptStartedAt: routeAttemptStartedAt + let advertisedRelay = self.syncNonEmpty(payload["cloudRelayWssUrl"] as? String) + let allRelays = self.deduplicatedAddresses(relayRoutes + (advertisedRelay.map { [$0] } ?? [])) + let profile = HostConnectionProfile( + hostIdentity: expectedHostIdentity, + hostName: self.syncNonEmpty(brain["deviceName"] as? String) ?? machine.displayName, + siteId: self.syncNonEmpty(brain["siteId"] as? String), + port: preferredDirectPort ?? route.endpoint.port, + authKind: "paired", + pairedDeviceId: self.deviceId, + lastRemoteDbVersion: 0, + lastHostDeviceId: expectedHostIdentity, + lastSuccessfulAddress: route.endpoint.address, + savedAddressCandidates: directHosts, + discoveredLanAddresses: lanHosts, + tailscaleAddress: tailnetHosts.first, + savedRelayCandidates: allRelays, + accountOwnerId: owner, + relayAccountOwnerId: owner ) - ProductAnalytics.shared.captureMachineAdoptionOutcome(.adopted) - return true - } catch { - lastFailure = error - if route.kind == .relay { - relayRouteFailed = true - } - if let candidateSocket, socket === candidateSocket { - teardownSocket() - } - if error is AccountAdoptionIdentityVerificationError - || error is AccountPairingAuthorizationChangedError - || error is AccountPairingConnectionSupersededError - || !isCurrentConnectAttempt(generation) { - throw error + return (payload: payload, pairedSecret: pairedSecret, profile: profile) + }, + isAuthorized: { candidate in + AccountService.shared.isPairingCommitAuthorized(candidate) + }, + isCurrentCandidate: { self.isCurrentConnectAttempt(generation) && self.socket === winner.task }, + commit: { prepared in + self.keychain.saveToken(prepared.pairedSecret) + if let key = self.profileStorageKey(prepared.profile) { + self.keychain.saveToken(prepared.pairedSecret, hostKey: key) } + self.saveProfile(prepared.profile) + try self.applyHelloPayload( + prepared.payload, + connectedHost: route.endpoint.address, + port: prepared.profile.port, + authKind: "paired", + pairedDeviceId: self.deviceId, + expectedHostIdentity: expectedHostIdentity, + connectAttemptGeneration: generation, + negotiatedReadyV2: winner.negotiatedReadyV2 + ) } - } - throw AccountAdoptionRoutesExhaustedError( - machineName: machine.displayName, - routeLabels: triedRouteLabels, - finalFailure: lastFailure ) + lastConnectedRouteKind = route.connectionRouteKind + schedulePostHelloWork(for: generation) + Task { await PushNotificationService.shared.enableIfPaired() } + LiveActivityService.shared.start() + accountPairingPinFallbackHost = nil + publishAccountConnectSuccess( + route: route, + attemptStartedAt: raceStartedAt + ) + ProductAnalytics.shared.captureMachineAdoptionOutcome(.adopted) + return true } catch { if error is AccountPairingConnectionSupersededError || pairingGeneration.map({ !isCurrentConnectAttempt($0) }) == true { @@ -5885,8 +6355,9 @@ final class SyncService: ObservableObject { } let message = SyncUserFacingError.message(for: error) resetAndCancelReconnectLoop() + // `allowAutoReconnect` stops THIS attempt from retrying. Persisting a + // pause here would outlive the relaunch and the network that caused it. allowAutoReconnect = false - setAutoReconnectPausedByUser(true) teardownSocket(reason: message) clearConnectTimingMetrics() accountConnectSuccessClearTask?.cancel() @@ -6733,6 +7204,9 @@ final class SyncService: ObservableObject { var pairingPayload: [String: Any] = [ "code": code.trimmingCharacters(in: .whitespacesAndNewlines).uppercased(), "peer": self.currentPeerMetadata(), + // New hosts keep both secrets live until this client acknowledges + // hello_ok. Old hosts ignore the field and never advertise rotation. + "pairingCommitVersion": 1, ] // Register this device's DPoP public key at pairing time so the host // has a key on record from the first paired hello onward. @@ -6782,15 +7256,54 @@ final class SyncService: ObservableObject { savedRelayCandidates: nil ) currentAddress = preferredAddress - try await hello( - host: preferredAddress, - port: preferredPort, - token: secret, - authKind: "paired", - pairedDeviceId: pairedDeviceId, - expectedHostIdentity: hostIdentity, - connectAttemptGeneration: connectAttemptGeneration + let pairingCommitRequired = syncPairingCommitRequired(payload) + if let rotation = payload["rotation"] as? [String: Any] { + syncConnectLog.info( + "ADE_SYNC_TRACE re-pair staged by host, previous secret stays valid for \((rotation["expiresInMs"] as? NSNumber)?.intValue ?? 0)ms" + ) + } + // Persist BEFORE the hello. The host may commit this secret while the + // hello_ok that reports it is still in flight, and a drop right there + // used to leave the phone holding a secret the Mac had already retired -- + // recoverable only by typing another PIN at the Mac. Saving first cannot + // strand the phone the other way: a host that never commits keeps + // accepting the previous secret, and this device reconnects within + // seconds, far inside that window. + try await syncPersistPairingBeforeHello( + persist: { + self.keychain.saveToken(secret) + self.keychain.saveToken(secret, hostKey: self.profileStorageKey(profile)) + }, + hello: { + try await self.hello( + host: preferredAddress, + port: preferredPort, + token: secret, + authKind: "paired", + pairedDeviceId: pairedDeviceId, + expectedHostIdentity: hostIdentity, + connectAttemptGeneration: connectAttemptGeneration + ) + } ) + if pairingCommitRequired { + let commitRequestId = makeRequestId() + let commitRaw = try await awaitResponse(requestId: commitRequestId) { + self.sendEnvelope( + type: "pairing_commit", + requestId: commitRequestId, + payload: ["deviceId": pairedDeviceId] + ) + } + guard let commitPayload = commitRaw as? [String: Any], + (commitPayload["ok"] as? Bool) == true else { + throw NSError( + domain: "ADE", + code: 36, + userInfo: [NSLocalizedDescriptionKey: "The Mac did not finish saving this pairing. Try again."] + ) + } + } let finalizedProfile = syncProfileAfterDirectPairing( connectedProfile: activeHostProfile ?? profile, previousProfile: previousProfile, @@ -6809,8 +7322,8 @@ final class SyncService: ObservableObject { guard isCurrentConnectAttempt(connectAttemptGeneration) else { return } let friendlyMessage = SyncUserFacingError.message(for: error) resetAndCancelReconnectLoop() + // Session-scoped only; a failed pairing must not persist a pause. allowAutoReconnect = false - setAutoReconnectPausedByUser(true) teardownSocket(reason: friendlyMessage) clearConnectTimingMetrics() lastError = friendlyMessage @@ -6845,6 +7358,9 @@ final class SyncService: ObservableObject { } } + /// - Parameter suspendAutoReconnect: pass `true` only when the USER asked to + /// stop connecting. It persists across relaunches, so it must never be used + /// to mean "this attempt failed". func disconnect(clearCredentials: Bool = false, suspendAutoReconnect: Bool = true) { beginConnectAttempt() clearConnectTimingMetrics() @@ -12185,9 +12701,21 @@ final class SyncService: ObservableObject { return normalized.isEmpty ? nil : normalized } + /// Only ever called for an explicit user action. A failed connection attempt + /// must use `allowAutoReconnect`, which is session-scoped: latching a + /// persisted flag because the network was bad is how the app ends up refusing + /// to reconnect across relaunches. private func setAutoReconnectPausedByUser(_ paused: Bool) { autoReconnectPausedByUser = paused UserDefaults.standard.set(paused, forKey: autoReconnectPausedKey) + if paused { + UserDefaults.standard.set( + autoReconnectPauseSourceUser, + forKey: autoReconnectPauseSourceKey + ) + } else { + UserDefaults.standard.removeObject(forKey: autoReconnectPauseSourceKey) + } } @discardableResult @@ -15343,7 +15871,7 @@ final class SyncService: ObservableObject { code: 5, userInfo: userInfo ))) - case "pairing_result": + case "pairing_result", "pairing_commit_result": resolve(requestId: requestId, result: .success(payload)) case "changeset_batch": var batchPayload = payload diff --git a/apps/ios/ADETests/ADETests.swift b/apps/ios/ADETests/ADETests.swift index 47121fff0..be6e4ac2a 100644 --- a/apps/ios/ADETests/ADETests.swift +++ b/apps/ios/ADETests/ADETests.swift @@ -3597,6 +3597,79 @@ final class ADETests: XCTestCase { ) } + // Account adoption used to walk [lan, tailnet, relay] strictly in order, + // paying a socket open plus a 3s identity challenge per route. On cellular, + // where no direct route can ever succeed, that is 10-20s of dead time before + // relay is even dialed. + func testAccountAdoptionRelayJoinsTheSameRaceAsDirectRoutes() { + let plan = syncConnectionRaceCandidatePlan(rankedAttempts: [ + SyncConnectionEndpointAttempt(address: "192.168.1.40", port: 8787), + SyncConnectionEndpointAttempt(address: "100.94.1.5", port: 8787), + SyncConnectionEndpointAttempt(address: "wss://relay.ade.dev/connect/machine-key", port: 443), + ]) + + XCTAssertEqual(plan.count, 3) + let relay = plan.first { syncConnectionRouteKind($0.endpoint.address) == .relay } + XCTAssertNotNil(relay) + // Scheduled inside the same race, not after the direct routes exhaust it. + XCTAssertLessThanOrEqual( + relay?.delayNanoseconds ?? .max, + SyncConnectionRaceTiming.relayJoinDelayNanoseconds + + SyncConnectionRaceTiming.candidateStaggerNanoseconds + ) + XCTAssertLessThan( + relay?.delayNanoseconds ?? .max, + SyncConnectionRaceTiming.overallBudgetNanoseconds + ) + XCTAssertTrue(plan.allSatisfy { + $0.delayNanoseconds < SyncConnectionRaceTiming.overallBudgetNanoseconds + }) + } + + // A host naming a cipher this build does not implement is a version gap, not + // evidence the Mac is an impostor: it must cost that route, not the attempt. + func testUnsupportedAdoptionCipherFailsOneRouteRatherThanTheWholeAttempt() { + let compatibility = AccountAdoptionRouteCompatibilityError(machineName: "Arul's Mac") + XCTAssertFalse(syncAccountAdoptionFailureIsFatal(compatibility)) + XCTAssertTrue( + compatibility.localizedDescription.contains("Update ADE"), + "the message must say what to do, not imply a security failure" + ) + + XCTAssertTrue(syncAccountAdoptionFailureIsFatal( + AccountAdoptionIdentityVerificationError(machineName: "Arul's Mac") + )) + XCTAssertTrue(syncAccountAdoptionFailureIsFatal(AccountPairingAuthorizationChangedError())) + XCTAssertTrue(syncAccountAdoptionFailureIsFatal(AccountPairingConnectionSupersededError())) + } + + func testPairingSecretPersistsBeforeHelloAndCommitIsVersionNegotiated() async throws { + var order: [String] = [] + try await syncPersistPairingBeforeHello( + persist: { order.append("persist") }, + hello: { order.append("hello") } + ) + + XCTAssertEqual(order, ["persist", "hello"]) + XCTAssertFalse(syncPairingCommitRequired([:])) + XCTAssertFalse(syncPairingCommitRequired([ + "rotation": ["pendingCommit": false] + ])) + XCTAssertTrue(syncPairingCommitRequired([ + "rotation": ["pendingCommit": true, "expiresInMs": 600_000] + ])) + } + + func testAutoReconnectPauseMigrationClearsFailureFalloutButKeepsAUserPause() { + // Written by a build that latched on any failed attempt: no source, so it + // is swept. + XCTAssertFalse(syncAutoReconnectPausedAfterMigration(paused: true, pauseSource: nil)) + XCTAssertFalse(syncAutoReconnectPausedAfterMigration(paused: true, pauseSource: " ")) + // An explicit user disconnect records its source and must survive. + XCTAssertTrue(syncAutoReconnectPausedAfterMigration(paused: true, pauseSource: "user")) + XCTAssertFalse(syncAutoReconnectPausedAfterMigration(paused: false, pauseSource: "user")) + } + @MainActor func testSyncDisconnectCancelsScheduledReconnectWork() { let pausedKey = "ade.sync.autoReconnectPausedByUser"