diff --git a/docs/docs/api/Socks5ProxyAgent.md b/docs/docs/api/Socks5ProxyAgent.md index 21fec19b141..783424da511 100644 --- a/docs/docs/api/Socks5ProxyAgent.md +++ b/docs/docs/api/Socks5ProxyAgent.md @@ -60,6 +60,11 @@ added: v7.23.0 a password embedded in `proxyUrl`. **Default:** the URL password, if any. * `connect` {Function} Custom connector used to open the socket to the proxy. **Default:** a connector built from `proxyTls`. + * `connectTimeout` {number} Maximum time in milliseconds for each proxy + connection, SOCKS5 negotiation, and target TLS negotiation stage. A value of + `0` disables these stage timeouts. `proxyTls.timeout` and + `requestTls.timeout` override it for their respective TLS stages. + **Default:** `5000`. * `proxyTls` {BuildOptions} TLS options for the connection to the proxy itself (SOCKS5 over TLS). When set, the proxy connection is established over TLS and `servername` defaults to the proxy host name. @@ -69,7 +74,8 @@ added: v7.23.0 host name. Throws an `InvalidArgumentError` if `proxyUrl` is missing or does not use the -`socks5:` or `socks:` protocol. +`socks5:` or `socks:` protocol, or if `connectTimeout`, `proxyTls.timeout`, or +`requestTls.timeout` is not a finite, non-negative number. ```mjs import { Socks5ProxyAgent } from 'undici' diff --git a/lib/dispatcher/proxy-agent.js b/lib/dispatcher/proxy-agent.js index fbe50d4d61e..fd4577c1119 100644 --- a/lib/dispatcher/proxy-agent.js +++ b/lib/dispatcher/proxy-agent.js @@ -161,6 +161,7 @@ class ProxyAgent extends DispatcherBase { factory: agentFactory, username: opts.username || username, password: opts.password || password, + connectTimeout, proxyTls: opts.proxyTls, requestTls: opts.requestTls }) diff --git a/lib/dispatcher/socks5-proxy-agent.js b/lib/dispatcher/socks5-proxy-agent.js index 5a952d8cc6f..e380005ffb6 100644 --- a/lib/dispatcher/socks5-proxy-agent.js +++ b/lib/dispatcher/socks5-proxy-agent.js @@ -4,22 +4,33 @@ const { URL } = require('node:url') let tls // include tls conditionally since it is not always available const DispatcherBase = require('./dispatcher-base') -const { InvalidArgumentError } = require('../core/errors') +const { ConnectTimeoutError, InvalidArgumentError } = require('../core/errors') const { Socks5Client, STATES } = require('../core/socks5-client') const { kBusy, kConnected, kDispatch, kClose, kDestroy } = require('../core/symbols') const Pool = require('./pool') const buildConnector = require('../core/connect') +const { setupConnectTimeout } = require('../core/util') const { debuglog } = require('node:util') const debug = debuglog('undici:socks5-proxy') +const DEFAULT_SOCKS5_CONNECT_TIMEOUT = 5000 + const kProxyUrl = Symbol('proxy url') const kProxyHeaders = Symbol('proxy headers') const kProxyAuth = Symbol('proxy auth') const kProxyProtocol = Symbol('proxy protocol') const kPools = Symbol('pools') const kConnector = Symbol('connector') +const kConnectTimeout = Symbol('connect timeout') const kRequestTls = Symbol('request tls settings') +const kRequestTlsTimeout = Symbol('request tls timeout') + +function createConnectTimeoutError (hostname, port, timeout) { + return new ConnectTimeoutError( + `Connect Timeout Error (attempted address: ${hostname}:${port}, timeout: ${timeout}ms)` + ) +} // Static flag to ensure warning is only emitted once per process let experimentalWarningEmitted = false @@ -54,7 +65,20 @@ class Socks5ProxyAgent extends DispatcherBase { this[kProxyUrl] = url this[kProxyHeaders] = options.headers || {} this[kProxyProtocol] = options.proxyTls ? 'https:' : 'http:' - this[kRequestTls] = options.requestTls + + const connectTimeout = options.connectTimeout ?? DEFAULT_SOCKS5_CONNECT_TIMEOUT + if (!Number.isFinite(connectTimeout) || connectTimeout < 0) { + throw new InvalidArgumentError('invalid connectTimeout') + } + this[kConnectTimeout] = connectTimeout + + const { timeout, ...requestTls } = options.requestTls || {} + const requestTlsTimeout = timeout ?? connectTimeout + if (!Number.isFinite(requestTlsTimeout) || requestTlsTimeout < 0) { + throw new InvalidArgumentError('invalid requestTls.timeout') + } + this[kRequestTls] = requestTls + this[kRequestTlsTimeout] = requestTlsTimeout // Extract auth from URL or options this[kProxyAuth] = { @@ -63,8 +87,13 @@ class Socks5ProxyAgent extends DispatcherBase { } // Create connector for proxy connection + const proxyTlsTimeout = options.proxyTls?.timeout ?? connectTimeout + if (!Number.isFinite(proxyTlsTimeout) || proxyTlsTimeout < 0) { + throw new InvalidArgumentError('invalid proxyTls.timeout') + } this[kConnector] = options.connect || buildConnector({ ...options.proxyTls, + timeout: proxyTlsTimeout, servername: options.proxyTls?.servername || url.hostname }) @@ -113,21 +142,29 @@ class Socks5ProxyAgent extends DispatcherBase { // Wait for authentication (if required) const authenticationReady = Promise.withResolvers() - - const authenticationTimeout = setTimeout(() => { - socks5Client.destroy() - authenticationReady.reject(new Error('SOCKS5 authentication timeout')) - }, 5000) - - const onAuthenticated = () => { + const authenticationTimeout = this[kConnectTimeout] === 0 + ? null + : setTimeout(() => { + cleanupAuthenticationListeners() + socks5Client.destroy() + authenticationReady.reject( + createConnectTimeoutError(proxyHost, proxyPort, this[kConnectTimeout]) + ) + }, this[kConnectTimeout]) + + const cleanupAuthenticationListeners = () => { clearTimeout(authenticationTimeout) + socks5Client.removeListener('authenticated', onAuthenticated) socks5Client.removeListener('error', onAuthenticationError) + } + + const onAuthenticated = () => { + cleanupAuthenticationListeners() authenticationReady.resolve() } const onAuthenticationError = (err) => { - clearTimeout(authenticationTimeout) - socks5Client.removeListener('authenticated', onAuthenticated) + cleanupAuthenticationListeners() authenticationReady.reject(err) } @@ -147,22 +184,30 @@ class Socks5ProxyAgent extends DispatcherBase { // Wait for connection const connectionReady = Promise.withResolvers() - - const connectionTimeout = setTimeout(() => { - socks5Client.destroy() - connectionReady.reject(new Error('SOCKS5 connection timeout')) - }, 5000) + const connectionTimeout = this[kConnectTimeout] === 0 + ? null + : setTimeout(() => { + cleanupConnectionListeners() + socks5Client.destroy() + connectionReady.reject( + createConnectTimeoutError(targetHost, targetPort, this[kConnectTimeout]) + ) + }, this[kConnectTimeout]) + + const cleanupConnectionListeners = () => { + clearTimeout(connectionTimeout) + socks5Client.removeListener('connected', onConnected) + socks5Client.removeListener('error', onConnectionError) + } const onConnected = (info) => { debug('SOCKS5 tunnel established to', targetHost, targetPort, 'via', info) - clearTimeout(connectionTimeout) - socks5Client.removeListener('error', onConnectionError) + cleanupConnectionListeners() connectionReady.resolve() } const onConnectionError = (err) => { - clearTimeout(connectionTimeout) - socks5Client.removeListener('connected', onConnected) + cleanupConnectionListeners() connectionReady.reject(err) } @@ -215,8 +260,31 @@ class Socks5ProxyAgent extends DispatcherBase { }) const tlsReady = Promise.withResolvers() - finalSocket.once('secureConnect', tlsReady.resolve) - finalSocket.once('error', tlsReady.reject) + + const cleanupTlsListeners = () => { + queueMicrotask(clearTlsTimeout) + finalSocket.removeListener('secureConnect', onSecureConnect) + finalSocket.removeListener('error', onTlsError) + } + + const onSecureConnect = () => { + cleanupTlsListeners() + tlsReady.resolve() + } + + const onTlsError = (err) => { + cleanupTlsListeners() + tlsReady.reject(err) + } + + const clearTlsTimeout = setupConnectTimeout(new WeakRef(finalSocket), { + timeout: this[kRequestTlsTimeout], + hostname: targetHost, + port: targetPort + }) + + finalSocket.once('secureConnect', onSecureConnect) + finalSocket.once('error', onTlsError) await tlsReady.promise } diff --git a/test/socks5-proxy-agent.js b/test/socks5-proxy-agent.js index ee05908da6c..1abb2f37981 100644 --- a/test/socks5-proxy-agent.js +++ b/test/socks5-proxy-agent.js @@ -6,7 +6,7 @@ const { once } = require('node:events') const { tspl } = require('@matteo.collina/tspl') const { test, after } = require('node:test') const { request, ProxyAgent } = require('..') -const { InvalidArgumentError } = require('../lib/core/errors') +const { ConnectTimeoutError, InvalidArgumentError } = require('../lib/core/errors') const Socks5ProxyAgent = require('../lib/dispatcher/socks5-proxy-agent') const { createServer } = require('node:http') const { TestSocks5Server } = require('./fixtures/socks5-test-server') @@ -16,6 +16,52 @@ function getPools (agent) { return agent[poolsSymbol] } +async function createStalledSocks5Proxy ({ stallOnConnect = false } = {}) { + const sockets = new Set() + const server = net.createServer((socket) => { + sockets.add(socket) + socket.once('close', () => sockets.delete(socket)) + socket.on('error', () => {}) + + let state = 'handshake' + let buffer = Buffer.alloc(0) + socket.on('data', (data) => { + buffer = Buffer.concat([buffer, data]) + + if (state === 'handshake') { + if (buffer.length < 2 || buffer.length < 2 + buffer[1]) { + return + } + buffer = buffer.subarray(2 + buffer[1]) + socket.write(Buffer.from([0x05, 0x00])) + state = 'connect' + } else if (state === 'connect') { + if (buffer.length < 5) { + return + } + + const addressLength = buffer[3] === 0x01 + ? 4 + : buffer[3] === 0x04 + ? 16 + : 1 + buffer[4] + const messageLength = 4 + addressLength + 2 + if (buffer.length < messageLength) { + return + } + + buffer = buffer.subarray(messageLength) + if (!stallOnConnect) { + socket.write(Buffer.from([0x05, 0x00, 0x00, 0x01, 0, 0, 0, 0, 0, 0])) + } + state = 'connected' + } + }) + }) + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)) + return { server, sockets } +} + const tlsCerts = (() => { const forge = require('node-forge') const createCert = (cn, issuer, keyLength = 2048) => { @@ -80,6 +126,35 @@ test('Socks5ProxyAgent - constructor validation', async (t) => { await p.completed }) +test('Socks5ProxyAgent - timeout validation', async (t) => { + const p = tspl(t, { plan: 8 }) + + for (const connectTimeout of [-1, Infinity, '100']) { + p.throws(() => { + // eslint-disable-next-line no-new + new Socks5ProxyAgent('socks5://localhost:1080', { connectTimeout }) + }, InvalidArgumentError) + } + + for (const timeout of [-1, Infinity]) { + p.throws(() => { + // eslint-disable-next-line no-new + new Socks5ProxyAgent('socks5://localhost:1080', { requestTls: { timeout } }) + }, InvalidArgumentError) + p.throws(() => { + // eslint-disable-next-line no-new + new Socks5ProxyAgent('socks5://localhost:1080', { proxyTls: { timeout } }) + }, InvalidArgumentError) + } + + p.doesNotThrow(() => { + // eslint-disable-next-line no-new + new Socks5ProxyAgent('socks5://localhost:1080', { connectTimeout: 0 }) + }) + + await p.completed +}) + test('Socks5ProxyAgent - uses custom connector for proxy connection', async (t) => { const p = tspl(t, { plan: 3 }) @@ -231,6 +306,100 @@ test.skip('Socks5ProxyAgent - HTTPS connection', async (t) => { t.skip('HTTPS test requires TLS option refinement') }) +test('Socks5ProxyAgent - times out a stalled TLS upgrade', { timeout: 3000 }, async (t) => { + const p = tspl(t, { plan: 4 }) + const { server: stalledProxy, sockets: proxySockets } = await createStalledSocks5Proxy() + + let connectorSocket + const agent = new Socks5ProxyAgent(`socks5://127.0.0.1:${stalledProxy.address().port}`, { + connectTimeout: 100, + connect (opts, callback) { + const socket = net.connect({ host: opts.hostname, port: opts.port }) + connectorSocket = socket + socket.once('connect', () => callback(null, socket)) + socket.once('error', callback) + return socket + } + }) + + t.after(async () => { + await agent.destroy() + for (const socket of proxySockets) { + socket.destroy() + } + await new Promise(resolve => stalledProxy.close(resolve)) + }) + + try { + await request('https://example.invalid/', { dispatcher: agent }) + p.fail('should have thrown an error') + } catch (err) { + p.ok(err instanceof ConnectTimeoutError) + p.strictEqual(err.code, 'UND_ERR_CONNECT_TIMEOUT') + } + + p.ok(connectorSocket.destroyed, 'timed out TLS upgrade should destroy the tunnel socket') + p.strictEqual(getPools(agent).size, 0, 'failed TLS upgrade should remove the origin pool') + + await p.completed +}) + +test('Socks5ProxyAgent - times out a stalled CONNECT response', { timeout: 3000 }, async (t) => { + const p = tspl(t, { plan: 3 }) + const { server: stalledProxy, sockets: proxySockets } = await createStalledSocks5Proxy({ + stallOnConnect: true + }) + const agent = new Socks5ProxyAgent(`socks5://127.0.0.1:${stalledProxy.address().port}`, { + connectTimeout: 100 + }) + + t.after(async () => { + await agent.destroy() + for (const socket of proxySockets) { + socket.destroy() + } + await new Promise(resolve => stalledProxy.close(resolve)) + }) + + try { + await request('http://example.invalid/', { dispatcher: agent }) + p.fail('should have thrown an error') + } catch (err) { + p.ok(err instanceof ConnectTimeoutError) + p.strictEqual(err.code, 'UND_ERR_CONNECT_TIMEOUT') + } + + p.strictEqual(getPools(agent).size, 0, 'failed CONNECT should remove the origin pool') + await p.completed +}) + +test('ProxyAgent forwards connectTimeout to SOCKS5 TLS upgrades', { timeout: 3000 }, async (t) => { + const p = tspl(t, { plan: 2 }) + const { server: stalledProxy, sockets: proxySockets } = await createStalledSocks5Proxy() + const agent = new ProxyAgent({ + uri: `socks5://127.0.0.1:${stalledProxy.address().port}`, + connectTimeout: 100 + }) + + t.after(async () => { + await agent.destroy() + for (const socket of proxySockets) { + socket.destroy() + } + await new Promise(resolve => stalledProxy.close(resolve)) + }) + + try { + await request('https://example.invalid/', { dispatcher: agent }) + p.fail('should have thrown an error') + } catch (err) { + p.ok(err instanceof ConnectTimeoutError) + p.strictEqual(err.code, 'UND_ERR_CONNECT_TIMEOUT') + } + + await p.completed +}) + test('Socks5ProxyAgent - with authentication', async (t) => { const p = tspl(t, { plan: 2 }) @@ -429,7 +598,7 @@ test('Socks5ProxyAgent - connection failure', async (t) => { }) test('Socks5ProxyAgent - destroys socket when negotiation times out', async (t) => { - const p = tspl(t, { plan: 2 }) + const p = tspl(t, { plan: 3 }) // SOCKS5 proxy that accepts the TCP connection but never replies to the greeting const stalledProxy = net.createServer(() => {}) @@ -439,6 +608,7 @@ test('Socks5ProxyAgent - destroys socket when negotiation times out', async (t) let connectorSocket const agent = new Socks5ProxyAgent(`socks5://127.0.0.1:${stalledProxy.address().port}`, { + connectTimeout: 100, connect (opts, callback) { const socket = net.connect({ host: opts.hostname, port: opts.port }) connectorSocket = socket @@ -454,7 +624,8 @@ test('Socks5ProxyAgent - destroys socket when negotiation times out', async (t) }) p.fail('should have thrown an error') } catch (err) { - p.ok(err, 'should throw error when SOCKS5 negotiation stalls') + p.ok(err instanceof ConnectTimeoutError) + p.strictEqual(err.code, 'UND_ERR_CONNECT_TIMEOUT') } await agent.close() diff --git a/types/socks5-proxy-agent.d.ts b/types/socks5-proxy-agent.d.ts index 05e4331670e..4eaeb11048f 100644 --- a/types/socks5-proxy-agent.d.ts +++ b/types/socks5-proxy-agent.d.ts @@ -21,5 +21,7 @@ declare namespace Socks5ProxyAgent { connect?: buildConnector.connector; /** TLS options for the proxy connection (for SOCKS5 over TLS) */ proxyTls?: buildConnector.BuildOptions; + /** TLS options for the end-to-end connection to the target */ + requestTls?: buildConnector.BuildOptions; } }