FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: honor requestTls when proxy is SOCKS5 · nodejs/undici@04201f8 · GitHub

/ undici Public

Commit 04201f8

Browse files
tonghuaroot
authored and
mcollina
committed
fix: honor requestTls when proxy is SOCKS5
ProxyAgent silently dropped opts.requestTls when the configured proxy URI was socks5:// (or socks://). Socks5ProxyAgent also lacked any equivalent of requestTls, so even constructed directly it had no way to configure target HTTPS TLS settings (ca, cert, key, rejectUnauthorized, servername). The inner connect callback called tls.connect with ...connectOpts.tls || {} but undici's internal connectOpts never populates a .tls field, so the spread was a dead expression. This change: - Forwards opts.requestTls from ProxyAgent to Socks5ProxyAgent. - Adds a requestTls constructor option on Socks5ProxyAgent stored in the kRequestTls symbol. - Applies it on the inner tls.connect for the target HTTPS upgrade, with socket overriding (must point at the SOCKS5 tunnel) and servername defaulting to targetHost but allowing user override to match the HTTP-proxy path semantics. - Documents the new option in Socks5ProxyAgent.md. - Adds two regression tests covering both the direct Socks5ProxyAgent path and the ProxyAgent forwarding path. Refs GHSA-vmh5-mc38-953g (cherry picked from commit 42d4955) Signed-off-by: Matteo Collina <hello@matteocollina.com>
1 parent fcd642f commit 04201f8

4 files changed

Lines changed: 132 additions & 5 deletions

File tree

‎docs/docs/api/Socks5ProxyAgent.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Extends: [`PoolOptions`](/docs/docs/api/Pool.md#parameter-pooloptions)
2222
* **password** `string` (optional) - SOCKS5 proxy password for authentication. Can also be provided in the proxy URL.
2323
* **connect** `Function` (optional) - Custom connector function for the proxy connection.
2424
* **proxyTls** `BuildOptions` (optional) - TLS options for the proxy connection (when using SOCKS5 over TLS).
25+
* **requestTls** `BuildOptions` (optional) - TLS options applied to the HTTPS connection to the target server through the SOCKS5 tunnel. Use this to configure `ca`, `cert`, `key`, `rejectUnauthorized`, `servername`, etc. for the target HTTPS endpoint.
2526

2627
Examples:
2728

‎lib/dispatcher/proxy-agent.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,8 @@ class ProxyAgent extends DispatcherBase {
142142
factory: agentFactory,
143143
username: opts.username || username,
144144
password: opts.password || password,
145-
proxyTls: opts.proxyTls
145+
proxyTls: opts.proxyTls,
146+
requestTls: opts.requestTls
146147
})
147148
}
148149

‎lib/dispatcher/socks5-proxy-agent.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const kProxyAuth = Symbol('proxy auth')
1919
const kProxyProtocol = Symbol('proxy protocol')
2020
const kPools = Symbol('pools')
2121
const kConnector = Symbol('connector')
22+
const kRequestTls = Symbol('request tls settings')
2223

2324
// Static flag to ensure warning is only emitted once per process
2425
let experimentalWarningEmitted = false
@@ -53,6 +54,7 @@ class Socks5ProxyAgent extends DispatcherBase {
5354
this[kProxyUrl] = url
5455
this[kProxyHeaders] = options.headers || {}
5556
this[kProxyProtocol] = options.proxyTls ? 'https:' : 'http:'
57+
this[kRequestTls] = options.requestTls
5658

5759
// Extract auth from URL or options
5860
this[kProxyAuth] = {
@@ -199,9 +201,9 @@ class Socks5ProxyAgent extends DispatcherBase {
199201
}
200202
debug('upgrading to TLS')
201203
finalSocket = tls.connect({
204+
...this[kRequestTls],
202205
socket,
203-
servername: targetHost,
204-
...connectOpts.tls || {}
206+
servername: this[kRequestTls]?.servername || targetHost
205207
})
206208

207209
await new Promise((resolve, reject) => {

‎test/socks5-proxy-agent.js‎

Lines changed: 125 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,53 @@
11
'use strict'
22

33
const net = require('node:net')
4+
const https = require('node:https')
45
const { tspl } = require('@matteo.collina/tspl')
5-
const { test } = require('node:test')
6-
const { request } = require('..')
6+
const { test, after } = require('node:test')
7+
const { request, ProxyAgent } = require('..')
78
const { InvalidArgumentError } = require('../lib/core/errors')
89
const Socks5ProxyAgent = require('../lib/dispatcher/socks5-proxy-agent')
910
const { createServer } = require('node:http')
1011
const { TestSocks5Server } = require('./fixtures/socks5-test-server')
1112

13+
const tlsCerts = (() => {
14+
const forge = require('node-forge')
15+
const createCert = (cn, issuer, keyLength = 2048) => {
16+
const keys = forge.pki.rsa.generateKeyPair(keyLength)
17+
const cert = forge.pki.createCertificate()
18+
cert.publicKey = keys.publicKey
19+
cert.serialNumber = '' + Date.now()
20+
cert.validity.notBefore = new Date()
21+
cert.validity.notAfter = new Date()
22+
cert.validity.notAfter.setFullYear(cert.validity.notBefore.getFullYear() + 10)
23+
const attrs = [{ name: 'commonName', value: cn }]
24+
cert.setSubject(attrs)
25+
const isCa = issuer === undefined
26+
cert.setExtensions([
27+
{ name: 'basicConstraints', cA: isCa },
28+
{ name: 'subjectAltName', altNames: [{ type: 2, value: cn }] }
29+
])
30+
const alg = forge.md.sha256.create()
31+
if (issuer !== undefined) {
32+
cert.setIssuer(issuer.certificate.subject.attributes)
33+
cert.sign(issuer.privateKey, alg)
34+
} else {
35+
cert.setIssuer(attrs)
36+
cert.sign(keys.privateKey, alg)
37+
}
38+
return { privateKey: keys.privateKey, publicKey: keys.publicKey, certificate: cert }
39+
}
40+
const root = createCert('socks5-test-ca')
41+
const server = createCert('localhost', root)
42+
return {
43+
root: { crt: forge.pki.certificateToPem(root.certificate) },
44+
server: {
45+
key: forge.pki.privateKeyToPem(server.privateKey),
46+
crt: forge.pki.certificateToPem(server.certificate)
47+
}
48+
}
49+
})()
50+
1251
test('Socks5ProxyAgent - constructor validation', async (t) => {
1352
const p = tspl(t, { plan: 4 })
1453

@@ -432,3 +471,87 @@ test('Socks5ProxyAgent - URL parsing edge cases', async (t) => {
432471

433472
await p.completed
434473
})
474+
475+
test('Socks5ProxyAgent - requestTls is honored for target HTTPS connection (GHSA-vmh5-mc38-953g)', async (t) => {
476+
const p = tspl(t, { plan: 2 })
477+
478+
// HTTPS server with a cert signed by tlsCerts.root, NOT in Node's default trust store
479+
const server = https.createServer({
480+
key: tlsCerts.server.key,
481+
cert: tlsCerts.server.crt
482+
}, (req, res) => {
483+
res.writeHead(200, { 'content-type': 'application/json' })
484+
res.end(JSON.stringify({ ok: true }))
485+
})
486+
await new Promise(resolve => server.listen(0, '127.0.0.1', resolve))
487+
const serverPort = server.address().port
488+
489+
// SOCKS5 server that forwards CONNECT to the local HTTPS server
490+
const socksServer = new TestSocks5Server()
491+
const socksAddress = await socksServer.listen()
492+
493+
// Use Socks5ProxyAgent directly with requestTls
494+
const proxyAgent = new Socks5ProxyAgent(`socks5://127.0.0.1:${socksAddress.port}`, {
495+
requestTls: {
496+
ca: [tlsCerts.root.crt]
497+
}
498+
})
499+
500+
after(async () => {
501+
await proxyAgent.close()
502+
server.close()
503+
await socksServer.close()
504+
})
505+
506+
// The HTTPS request via SOCKS5 should succeed because requestTls.ca contains the root CA
507+
// that signed the server cert. Without honoring requestTls, Node would default to Mozilla
508+
// CA bundle and reject the cert.
509+
const response = await request(`https://localhost:${serverPort}/`, {
510+
dispatcher: proxyAgent
511+
})
512+
p.strictEqual(response.statusCode, 200, 'request should succeed when requestTls.ca is honored')
513+
const body = await response.body.json()
514+
p.deepStrictEqual(body, { ok: true })
515+
516+
await p.completed
517+
})
518+
519+
test('ProxyAgent forwards requestTls to Socks5ProxyAgent (GHSA-vmh5-mc38-953g)', async (t) => {
520+
const p = tspl(t, { plan: 2 })
521+
522+
const server = https.createServer({
523+
key: tlsCerts.server.key,
524+
cert: tlsCerts.server.crt
525+
}, (req, res) => {
526+
res.writeHead(200, { 'content-type': 'application/json' })
527+
res.end(JSON.stringify({ ok: true }))
528+
})
529+
await new Promise(resolve => server.listen(0, '127.0.0.1', resolve))
530+
const serverPort = server.address().port
531+
532+
const socksServer = new TestSocks5Server()
533+
const socksAddress = await socksServer.listen()
534+
535+
// Use top-level ProxyAgent with socks5 URI + requestTls; the option must be forwarded.
536+
const proxyAgent = new ProxyAgent({
537+
uri: `socks5://127.0.0.1:${socksAddress.port}`,
538+
requestTls: {
539+
ca: [tlsCerts.root.crt]
540+
}
541+
})
542+
543+
after(async () => {
544+
await proxyAgent.close()
545+
server.close()
546+
await socksServer.close()
547+
})
548+
549+
const response = await request(`https://localhost:${serverPort}/`, {
550+
dispatcher: proxyAgent
551+
})
552+
p.strictEqual(response.statusCode, 200, 'request should succeed when ProxyAgent forwards requestTls')
553+
const body = await response.body.json()
554+
p.deepStrictEqual(body, { ok: true })
555+
556+
await p.completed
557+
})

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL