diff --git a/changelog.d/fixes/11842-socks-connect-timeout-unify-family.md b/changelog.d/fixes/11842-socks-connect-timeout-unify-family.md new file mode 100644 index 0000000000..60ee74e267 --- /dev/null +++ b/changelog.d/fixes/11842-socks-connect-timeout-unify-family.md @@ -0,0 +1 @@ +- **fix(socks):** `Agent.connectTimeout` now bounds both SOCKS handshake and TLS connect, and `family === null` no longer falls back to `fetch-socks` ([#11842](https://github.com/diegosouzapw/OmniRoute/pull/11842)) diff --git a/docs/i18n/pl/docs/security/EGRESS_POLICY.md b/docs/i18n/pl/docs/security/EGRESS_POLICY.md index d7ecbc2291..9c077633bd 100644 --- a/docs/i18n/pl/docs/security/EGRESS_POLICY.md +++ b/docs/i18n/pl/docs/security/EGRESS_POLICY.md @@ -190,10 +190,7 @@ export function buildSocksFamilySocketOptions(family: 4 | 6 | null): Record[0], - options - ) as Dispatcher) - : createSocksDispatcherWithFamily( - socksOptions as unknown as Parameters[0], - family, - options - ); + return createSocksDispatcherWithFamily( + socksOptions as unknown as Parameters[0], + family, + options + ); } // ProxyAgent omits `connect`; the client->proxy socket is built from `proxyTls`. diff --git a/open-sse/utils/socksConnectorWithFamily.ts b/open-sse/utils/socksConnectorWithFamily.ts index 37f982ba5e..adda58590f 100644 --- a/open-sse/utils/socksConnectorWithFamily.ts +++ b/open-sse/utils/socksConnectorWithFamily.ts @@ -39,12 +39,24 @@ function resolvePort(protocol: string, port: string): number { * socksConnector but threads `socket_options` (which fetch-socks does not expose) * into SocksClient so Happy Eyeballs cannot pick IPv4 for an IPv6-only egress policy. */ -function socksConnectorWithFamily( +export function socksConnectorWithFamily( proxy: SocksProxy, family: 4 | 6 | null, - tlsOpts: buildConnector.BuildOptions = {} + tlsOpts: buildConnector.BuildOptions = {}, + connectTimeout?: number, + _buildConnectorForTest?: typeof buildConnector ): buildConnector.connector { - const undiciConnect = buildConnector(tlsOpts); + const isDisabled = connectTimeout === 0; + // SOCKS lib: 0 throws (isValidTimeoutValue: value>0) and undefined → DEFAULT_TIMEOUT 30s; + // undici: 0 disables (core/util.js: if (!opts.timeout) return noop), undefined → 10s. Divergence intentional. + const handshakeTimeout = isDisabled ? undefined : (connectTimeout ?? resolveSocksHandshakeTimeoutMs()); + const tlsTimeout = connectTimeout; + // Sequential budget: both phases bounded by the same connectTimeout → wall-time up to 60s for https + // (vs 30s direct). Shared-deadline alternative rejected as unjustified complexity. + const build = _buildConnectorForTest ?? buildConnector; + const undiciConnect = build( + tlsTimeout !== undefined ? ({ ...tlsOpts, timeout: tlsTimeout } as any) : tlsOpts + ); const socketOptions = buildSocksFamilySocketOptions(family); return async (options, callback) => { const { protocol, hostname, port, httpSocket } = options as unknown as { @@ -57,7 +69,7 @@ function socksConnectorWithFamily( const r = await SocksClient.createConnection({ command: "connect", proxy, - timeout: resolveSocksHandshakeTimeoutMs(), + timeout: handshakeTimeout as any, destination: { host: hostname, port: resolvePort(protocol, port) }, existing_socket: httpSocket as never, socket_options: socketOptions as never, @@ -79,11 +91,12 @@ export function createSocksDispatcherWithFamily( family: 4 | 6 | null, agentOptions: Agent.Options = {} ): Dispatcher { - const { connect, ...rest } = agentOptions as Agent.Options & { + const { connect, connectTimeout, ...rest } = agentOptions as Agent.Options & { + connectTimeout?: number; connect?: buildConnector.BuildOptions; }; return new Agent({ ...rest, - connect: socksConnectorWithFamily(proxy, family, connect), + connect: socksConnectorWithFamily(proxy, family, connect as any, connectTimeout as any), }); } diff --git a/tests/unit/socks-connect-timeout-e2e.test.ts b/tests/unit/socks-connect-timeout-e2e.test.ts new file mode 100644 index 0000000000..9c62d0a906 --- /dev/null +++ b/tests/unit/socks-connect-timeout-e2e.test.ts @@ -0,0 +1,65 @@ +import { describe, it, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import net from "node:net"; +import { fetch } from "undici"; +import { createSocksDispatcherWithFamily } from "../../open-sse/utils/socksConnectorWithFamily.ts"; +import { clearDispatcherCache } from "../../open-sse/utils/proxyDispatcher.ts"; + +async function startFakeSocks(opts: { stallAfterGrant: boolean }): Promise<{ port: number; close: () => Promise }> { + return new Promise((resolve) => { + const serverSockets = new Set(); + const server = net.createServer((socket) => { + let step = 0; + serverSockets.add(socket); + socket.on("close", () => serverSockets.delete(socket)); + socket.on("data", (_data: Buffer) => { + if (step === 0) { + socket.write(Buffer.from([0x05, 0x00])); + step = 1; + return; + } + if (step === 1) { + if (!opts.stallAfterGrant) return; + socket.write(Buffer.from([0x05, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00])); + step = 2; + } + }); + }); + server.listen(0, "127.0.0.1", () => { + const addr = server.address() as net.AddressInfo; + resolve({ + port: addr.port, + close: () => + new Promise((r) => { + for (const s of serverSockets) s.destroy(); + server.close(() => r()); + }), + }); + }); + }); +} + +describe("stub SOCKS e2e", () => { + afterEach(() => clearDispatcherCache()); + + it("pre-grant stall (SOCKS timer) \u2192 error < 1000ms", async () => { + const { port, close } = await startFakeSocks({ stallAfterGrant: false }); + const dispatcher = createSocksDispatcherWithFamily({ host: "127.0.0.1", port, type: 5 } as any, 4 as any, { connectTimeout: 300, connect: {} } as any); + const t0 = Date.now(); + await assert.rejects(() => fetch("https://example.invalid/", { dispatcher } as any)); + assert.ok(Date.now() - t0 < 1000, `pre-grant stall must error < 1000ms, took ${Date.now() - t0}ms`); + await close(); + }); + + it("post-grant stall (TLS timer, https:// only) \u2192 error < 1500ms", async () => { + const { port, close } = await startFakeSocks({ stallAfterGrant: true }); + const dispatcher = createSocksDispatcherWithFamily({ host: "127.0.0.1", port, type: 5 } as any, 4 as any, { connectTimeout: 300, connect: {} } as any); + const t0 = Date.now(); + let caught: any = null; + await assert.rejects(async () => { try { await fetch("https://example.invalid/", { dispatcher } as any); } catch (e) { caught = e; throw e; } }); + const err: any = caught; + // The ~1000ms wall time is connectTimeout 300ms + undici immediate/queue overhead, not the 10000ms default. + assert.ok(Date.now() - t0 < 1500, `post-grant stall must error < 1500ms, took ${Date.now() - t0}ms (err: ${String((err as any)?.message ?? err).slice(0, 120)})`); + await close(); + }); +}); diff --git a/tests/unit/socks-connect-timeout.test.ts b/tests/unit/socks-connect-timeout.test.ts new file mode 100644 index 0000000000..dc54901a1d --- /dev/null +++ b/tests/unit/socks-connect-timeout.test.ts @@ -0,0 +1,91 @@ +import { describe, it, afterEach, beforeEach } from "node:test"; +import assert from "node:assert/strict"; +import { SocksClient } from "socks"; + +// Lightweight oracle — node:test, no vi.mock. +// We patch SocksClient.createConnection (writable) and inject a fake +// buildConnector via the 5th param to capture the TLS timeout without +// mutating the read-only undici module. + +describe("socks connectTimeout forwarder", () => { + let capturedTimeout: any = undefined; + let capturedTlsTimeout: any = undefined; + let capturedTlsUndefined = false; + let origCreateConnection: any; + + beforeEach(() => { + origCreateConnection = SocksClient.createConnection; + capturedTimeout = undefined; + capturedTlsTimeout = undefined; + capturedTlsUndefined = false; + (SocksClient as any).createConnection = async (opts: any) => { + capturedTimeout = opts?.timeout; + return { socket: { setNoDelay: () => ({ setNoDelay: () => {} }) } } as any; + }; + }); + + afterEach(() => { + (SocksClient as any).createConnection = origCreateConnection; + capturedTimeout = undefined; + capturedTlsTimeout = undefined; + capturedTlsUndefined = false; + }); + + function fakeBuildConnector(opts: any = {}) { + if (opts && typeof opts.timeout !== "undefined") capturedTlsTimeout = opts.timeout; + else capturedTlsUndefined = true; + return (_options: any, cb: any) => cb(null, { setNoDelay: () => ({}) } as any); + } + + async function driveConnector(args: { + family: 4 | 6 | null; + tlsOpts?: any; + connectTimeout?: number; + protocol?: string; + hostname?: string; + port?: string; + }) { + const mod: any = await import(`../../open-sse/utils/socksConnectorWithFamily.ts?t=${Date.now()}-${Math.random()}`); + const proxy = { host: "1.2.3.4", port: 1080, type: 5 } as any; + const tlsOpts = args.tlsOpts ?? {}; + const connectTimeout = args.connectTimeout; + const connector: any = mod.socksConnectorWithFamily(proxy, args.family, tlsOpts, connectTimeout, fakeBuildConnector as any); + await new Promise((resolve, reject) => + connector( + { protocol: args.protocol ?? "https:", hostname: args.hostname ?? "example.com", port: args.port ?? "443" } as any, + (err: any) => (err ? reject(err) : resolve()) + ) + ); + return { capturedTimeout, capturedTlsTimeout, capturedTlsUndefined, mod, connector }; + } + + it("U1: Agent.connectTimeout → SocksClient.timeout + TLS timeout", async () => { + const { capturedTimeout: t, capturedTlsTimeout: tls } = await driveConnector({ family: 4, tlsOpts: {}, connectTimeout: 5000, protocol: "https:", port: "443" }); + assert.equal(t, 5000); + assert.equal(tls, 5000); + }); + + it("U2: fallback sans connectTimeout → SOCKS_HANDSHAKE (TLS no timeout)", async () => { + const prev = process.env.SOCKS_HANDSHAKE_TIMEOUT_MS; + process.env.SOCKS_HANDSHAKE_TIMEOUT_MS = "7777"; + try { + const { capturedTimeout: t, capturedTlsUndefined: tlsUndef } = await driveConnector({ family: 6, tlsOpts: {}, connectTimeout: undefined, protocol: "https:", port: "443" }); + assert.equal(t, 7777); + assert.equal(tlsUndef, true); + } finally { + if (prev === undefined) delete process.env.SOCKS_HANDSHAKE_TIMEOUT_MS; + else process.env.SOCKS_HANDSHAKE_TIMEOUT_MS = prev; + } + }); + + it("U3: http (no TLS) still bounds SocksClient", async () => { + const { capturedTimeout: t } = await driveConnector({ family: null, tlsOpts: {}, connectTimeout: 5000, protocol: "http:", port: "80" }); + assert.equal(t, 5000); + }); + + it("U4: connectTimeout=0 → SocksClient undefined (SOCKS defaults to 30s) + TLS timeout 0 (disabled)", async () => { + const { capturedTimeout: t, capturedTlsTimeout: tls } = await driveConnector({ family: 4, tlsOpts: {}, connectTimeout: 0, protocol: "https:", port: "443" }); + assert.equal(t, undefined); + assert.equal(tls, 0); + }); +});