From 5125def46dc96cecf48a21ed80d6d89da123d2f4 Mon Sep 17 00:00:00 2001 From: lcxhh Date: Tue, 13 Oct 2026 23:12:29 +0800 Subject: [PATCH] fix(proxy): bound SOCKS5 TLS handshake lifetime - Add an absolute TLS handshake deadline and common lifecycle cleanup - Preserve certificate verification and caller cancellation reasons - Cover timeout, abort, peer close and protocol errors with native fixtures - Document the deadline and register focused regression coverage --- .../docs/reference/configuration/server.md | 4 + scripts/test-layout/layout.json | 1 + src/lib/socks5-fetch.ts | 39 +++++- structure/transports/inventory.md | 6 + tests/fixtures/test-layout-expected.json | 3 +- tests/lib/socks5-tls-lifecycle.test.ts | 123 ++++++++++++++++++ 6 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 tests/lib/socks5-tls-lifecycle.test.ts diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index d71c2fc920e..ac9be7cc157 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -126,6 +126,10 @@ SOCKS5 (Clash mixed-port listeners included) belongs on `ALL_PROXY`, not `HTTP_P `ocx start --socks5` writes that value; `ocx start --socks5-off` clears it. +HTTPS requests through SOCKS5 have a separate 30-second TLS handshake deadline. +A stalled handshake fails without disabling certificate verification; caller +cancellation can stop it sooner. + If an older development build changed resume-history metadata before backup support existed, run `ocx recover-history --legacy-openai --yes` to force native-provider recovery. It force-relabels every user-message `opencodex` row, including legitimate dedicated-provider diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 1a6052e1ad8..429bf2a3c94 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1729,6 +1729,7 @@ "skill-ocx.test.ts": "ci-workflows", "slug-codec.test.ts": "codex-integration", "socks5-fetch.test.ts": "lib", + "socks5-tls-lifecycle.test.ts": "lib", "socks5-upload-lifecycle.test.ts": "lib", "spend-ceiling-enforcement.test.ts": "lib", "spend-instrumentation-log.test.ts": "server", diff --git a/src/lib/socks5-fetch.ts b/src/lib/socks5-fetch.ts index 52bdc3d3b50..ed2145c9e99 100644 --- a/src/lib/socks5-fetch.ts +++ b/src/lib/socks5-fetch.ts @@ -341,25 +341,50 @@ function requestHeaders(request: Request, target: URL): { text: string; chunked: async function secureSocket(socket: Socket, target: URL, signal: AbortSignal): Promise { return new Promise((resolve, reject) => { + if (signal.aborted) { + socket.destroy(); + reject(signal.reason instanceof Error ? signal.reason : new Error("The operation was aborted")); + return; + } const tlsSocket = tls.connect({ socket, servername: target.hostname, rejectUnauthorized: true, }); - const onAbort = () => { + let settled = false; + // Absolute deadline: a peer sending occasional TLS bytes must not extend the handshake. + const timer = setTimeout(() => { + fail(new Socks5FetchError("SOCKS5 TLS handshake timed out")); + }, SOCKS5_CONNECT_TIMEOUT_MS); + const cleanup = () => { + clearTimeout(timer); + signal.removeEventListener("abort", onAbort); + tlsSocket.removeListener("secureConnect", onSecureConnect); + tlsSocket.removeListener("error", onError); + tlsSocket.removeListener("close", onClose); + }; + const fail = (error: Error) => { + if (settled) return; + settled = true; + cleanup(); + // No error argument: pending event observers must not receive a second error. tlsSocket.destroy(); - reject(signal.reason instanceof Error ? signal.reason : new Error("The operation was aborted")); + reject(error); }; + const onAbort = () => fail(signal.reason instanceof Error ? signal.reason : new Error("The operation was aborted")); + const onError = (error: Error) => fail(error); + const onClose = () => fail(new Socks5FetchError("SOCKS5 upstream closed during TLS handshake")); const onSecureConnect = () => { - signal.removeEventListener("abort", onAbort); + if (settled) return; + settled = true; + cleanup(); resolve(tlsSocket); }; tlsSocket.once("secureConnect", onSecureConnect); - tlsSocket.once("error", error => { - signal.removeEventListener("abort", onAbort); - reject(error); - }); + tlsSocket.once("error", onError); + tlsSocket.once("close", onClose); signal.addEventListener("abort", onAbort, { once: true }); + if (signal.aborted) onAbort(); }); } diff --git a/structure/transports/inventory.md b/structure/transports/inventory.md index 4956c0e7165..38dd6813dca 100644 --- a/structure/transports/inventory.md +++ b/structure/transports/inventory.md @@ -307,6 +307,12 @@ admission or account-snapshot pairing. The forwarding contract is covered in ## SOCKS5 dispatch boundary +HTTPS tunnels in `src/lib/socks5-fetch.ts` give the TLS handshake its own absolute +30-second deadline after SOCKS CONNECT. Successful TLS clears that deadline before +the response timer takes over; timeout, caller cancellation, error and premature +close release the owned socket and listeners. Certificate verification stays enabled. +`tests/lib/socks5-tls-lifecycle.test.ts` exercises stalled and interrupted handshakes. + `src/config/proxy-env.ts` activates configured SOCKS5 through `src/lib/proxy-env.ts`; the compatibility config facade does not own a second activation path. The library captures `startupOutboundProxyConfigured` at module evaluation, before config diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 5b4c8519693..61fe7c8eea7 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1066,7 +1066,8 @@ "sidecar-settings-web-search-gate.test.ts": "vision", "sidecar-settings-web-search-off.test.ts": "vision", "sidecar-settings-web-search-stream.test.ts": "vision", "sidecar-tracker.test.ts": "vision", "skill-ocx.test.ts": "ci-workflows", "slug-codec.test.ts": "codex-integration", - "socks5-fetch.test.ts": "lib", "socks5-upload-lifecycle.test.ts": "lib", + "socks5-fetch.test.ts": "lib", "socks5-tls-lifecycle.test.ts": "lib", + "socks5-upload-lifecycle.test.ts": "lib", "spend-ceiling-enforcement.test.ts": "lib", "spend-instrumentation-log.test.ts": "server", "spend-ledger-file-journal.test.ts": "lib", "spend-ledger-lifecycle.test.ts": "server", "spend-ledger-owner-startup.test.ts": "server", "spend-ledger-owner.test.ts": "lib", diff --git a/tests/lib/socks5-tls-lifecycle.test.ts b/tests/lib/socks5-tls-lifecycle.test.ts new file mode 100644 index 00000000000..08e6233ff73 --- /dev/null +++ b/tests/lib/socks5-tls-lifecycle.test.ts @@ -0,0 +1,123 @@ +import { describe, test } from "bun:test"; +import assert from "node:assert/strict"; +import { createServer, type AddressInfo, type Socket } from "node:net"; +import { socks5Fetch } from "../../src/lib/socks5-fetch"; + +const CONNECT_TIMEOUT_MS = 30_000; +type Failure = "deadline" | "abort" | "null-abort" | "close" | "invalid-tls"; + +/** SOCKS accepts CONNECT, then observes real TLS bytes without contacting an upstream. */ +async function checkHandshakeFailure(failure: Failure): Promise { + const sockets = new Set(); + let markTlsSeen: () => void = () => {}; + let markPeerClosed: () => void = () => {}; + const tlsSeen = new Promise(resolve => { markTlsSeen = resolve; }); + const peerClosed = new Promise(resolve => { markPeerClosed = resolve; }); + let peer: Socket | undefined; + const server = createServer(socket => { + peer = socket; + sockets.add(socket); + socket.once("error", () => { /* fixture teardown may reset the connection */ }); + socket.once("close", () => { sockets.delete(socket); markPeerClosed(); }); + let stage: "greeting" | "connect" | "tls" = "greeting"; + let buffer = Buffer.alloc(0); + socket.on("data", chunk => { + buffer = Buffer.concat([buffer, typeof chunk === "string" ? Buffer.from(chunk) : chunk]); + if (stage === "greeting") { + if (buffer.length < 2 || buffer.length < 2 + buffer[1]!) return; + buffer = buffer.subarray(2 + buffer[1]!); + socket.write(Buffer.from([5, 0])); + stage = "connect"; + } + if (stage === "connect") { + if (buffer.length < 5 || buffer.length < 7 + buffer[4]!) return; + buffer = buffer.subarray(7 + buffer[4]!); + socket.write(Buffer.from([5, 0, 0, 1, 127, 0, 0, 1, 0, 80])); + stage = "tls"; + } + if (stage === "tls" && buffer.length > 0) { + markTlsSeen(); + buffer = Buffer.alloc(0); + } + }); + }); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + + const originalSetTimeout = globalThis.setTimeout; + const originalClearTimeout = globalThis.clearTimeout; + const deadlines = new Map, () => void>(); + let deadlinesArmed = 0; + // Capture, but do not shorten, the transport deadline. Fire it only after the + // fixture actually receives ClientHello, so machine load cannot race TCP setup. + globalThis.setTimeout = ((callback: (...args: unknown[]) => void, ms?: number, ...args: unknown[]) => { + const handle = originalSetTimeout(callback, ms, ...args); + if (ms === CONNECT_TIMEOUT_MS) { + deadlinesArmed++; + deadlines.set(handle, () => callback(...args)); + } + return handle; + }) as typeof setTimeout; + globalThis.clearTimeout = ((handle: ReturnType | undefined) => { + if (handle !== undefined) deadlines.delete(handle); + return originalClearTimeout(handle); + }) as typeof clearTimeout; + + const controller = new AbortController(); + const reason = failure === "null-abort" ? null : new Error("fixture caller cancellation"); + const watchdog = originalSetTimeout(() => controller.abort(new Error("fixture watchdog")), 2_000); + let outcome: Promise | undefined; + try { + const port = (server.address() as AddressInfo).port; + outcome = socks5Fetch("https://destination.invalid/", { signal: controller.signal }, `socks5://127.0.0.1:${port}`) + .then(() => new Error("unexpected TLS success"), (error: unknown) => error); + // Receipt is bounded by the same request outcome: a failure before ClientHello + // must fail this assertion instead of leaving the test waiting indefinitely. + const ready = await Promise.race([tlsSeen.then(() => true), outcome.then(() => false)]); + assert.equal(ready, true, "the peer must receive ClientHello before exercising TLS failure"); + // Bun also implements a socket inactivity timer through global setTimeout. + // Its already-cleared internal timer must not make the transport test Node-only. + assert.ok(deadlinesArmed >= 2, "TCP and TLS deadlines must both be observed"); + assert.equal(deadlines.size, 1, "only the TLS deadline remains active"); + + if (failure === "deadline") deadlines.values().next().value!(); + else if (failure === "abort" || failure === "null-abort") controller.abort(reason); + else if (failure === "close") peer!.destroy(); + else peer!.write("fixture invalid TLS"); + + const result = await outcome; + if (failure === "abort" || failure === "null-abort") assert.equal(result, reason); + else { + assert.ok(result instanceof Error); + if (failure === "deadline") assert.match(result.message, /SOCKS5 TLS handshake timed out/); + else if (failure === "close") assert.match(result.message, /TLS handshake|disconnected before secure TLS/); + else assert.match(result.message, /ssl|wrong version|packet|tls/i); + } + assert.equal(deadlines.size, 0, "terminal failure clears the owned deadline"); + let closeWatchdog: ReturnType | undefined; + try { + await Promise.race([peerClosed, new Promise((_, reject) => { + closeWatchdog = originalSetTimeout(() => reject(new Error("TLS peer remained open")), 2_000); + })]); + } finally { originalClearTimeout(closeWatchdog); } + } finally { + controller.abort(); + await outcome; + originalClearTimeout(watchdog); + for (const handle of deadlines.keys()) originalClearTimeout(handle); + globalThis.setTimeout = originalSetTimeout; + globalThis.clearTimeout = originalClearTimeout; + for (const socket of sockets) socket.destroy(); + await new Promise(resolve => server.close(() => resolve())); + } +} + +describe("SOCKS5 TLS handshake lifecycle", () => { + test("a stalled TLS handshake reaches an absolute deadline and releases its socket", () => checkHandshakeFailure("deadline")); + test("caller cancellation preserves its reason and clears the TLS deadline", () => checkHandshakeFailure("abort")); + test("a null cancellation reason is preserved", () => checkHandshakeFailure("null-abort")); + test("premature peer close releases the TLS deadline", () => checkHandshakeFailure("close")); + test("TLS protocol failure releases the TLS deadline", () => checkHandshakeFailure("invalid-tls")); +});