Skip to content

Commit 37698be

Browse files
committed
node:https: give the request what tls.connect() throws over a proxy tunnel (#44346)
establishTunnel() calls tls.connect() from the proxy socket's 'readable' handler, after https.request() has returned. An option it rejects (minVersion, a wrong passphrase) was thrown with nothing to catch it: an uncaught exception, no 'error' or 'close' on the request, and the CONNECT socket left open. Catch it, close the proxy socket and report it through the same callback as the other tunnel failures. Port of nodejs/node#66096.
1 parent 9153f84 commit 37698be

3 files changed

Lines changed: 334 additions & 2 deletions

File tree

‎src/js/node/https.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,15 @@ function establishTunnel(agent, socket, options, tunnelConfig, afterSocket) {
203203
$debug("Propagate free event from tunneled socket to tunnel socket");
204204
socket.emit("free");
205205
}
206-
tunneledSocket = require("node:tls").connect(requestOptions, onTLSHandshakeSuccess);
206+
// https://github.com/nodejs/node/blob/ada8c5c9f82f620885820a2268e19dbf8a2228d5/lib/https.js#L295-L308
207+
try {
208+
tunneledSocket = require("node:tls").connect(requestOptions, onTLSHandshakeSuccess);
209+
} catch (err) {
210+
$debug("tls.connect() over tunnel threw", err);
211+
socket.destroy();
212+
afterSocket(err, socket);
213+
return headerEndIndex;
214+
}
207215
tunneledSocket.on("free", onTunneledSocketFree);
208216
tunneledSocket.on("error", onTLSHandshakeError);
209217
const agentKey = requestOptions._agentKey;

‎test/js/node/http/node-http-agent-tls-options.test.mts‎

Lines changed: 250 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010

1111
import { HttpsProxyAgent } from "https-proxy-agent";
12+
import assert from "node:assert";
1213
import { once } from "node:events";
1314
import { readFileSync } from "node:fs";
1415
import http from "node:http";
@@ -17,6 +18,7 @@ import type { AddressInfo } from "node:net";
1718
import net from "node:net";
1819
import { dirname, join } from "node:path";
1920
import { describe, test } from "node:test";
21+
import tls from "node:tls";
2022
import { fileURLToPath } from "node:url";
2123

2224
const __dirname = dirname(fileURLToPath(import.meta.url));
@@ -670,9 +672,256 @@ describe("http.request agent options", () => {
670672
});
671673
});
672674

675+
describe(
676+
"https.request through a proxy from proxyEnv, with TLS options that tls.connect() rejects",
677+
// tls.connect() runs when the proxy has answered the CONNECT, after https.request() returned. nodejs/node#66096 gives
678+
// what it throws there to the request. No Node release has that change (v26.10.0 throws it uncaught). It is on Node
679+
// main, so v27 runs this block, and on v26.x-staging (33e4ba358b), which is not in a v26 release.
680+
{ skip: typeof Bun === "undefined" && Number(process.versions.node.split(".")[0]) < 27 },
681+
() => {
682+
const target = { host: "example.invalid", port: 443, path: "/" };
683+
const invalidVersion = { minVersion: "TLSv9" };
684+
const rejected = [
685+
{ name: "minVersion", options: invalidVersion, code: "ERR_TLS_INVALID_PROTOCOL_VERSION" },
686+
{
687+
name: "wrong passphrase",
688+
options: { key: tlsCerts.encryptedKey, cert: tlsCerts.cert, passphrase: "wrong" },
689+
code: "ERR_OSSL_BAD_DECRYPT",
690+
},
691+
];
692+
693+
function proxiedAgent(proxyUrl: string, options: https.AgentOptions = {}) {
694+
return new https.Agent({ ...options, proxyEnv: { https_proxy: proxyUrl } } as any);
695+
}
696+
697+
// The error that tls.connect() throws for these options when nothing is between it and its caller.
698+
function thrownByTlsConnect(options: object) {
699+
try {
700+
tls.connect({ ...options } as any);
701+
} catch (err: any) {
702+
return { code: err.code, message: err.message };
703+
}
704+
}
705+
706+
// A proxy that answers each CONNECT with 200 and then holds the connection.
707+
function createHoldingProxy(scheme: string) {
708+
const sockets = new Set<net.Socket>();
709+
const connects: string[] = [];
710+
// What the client sent after the CONNECT.
711+
const afterConnect: Buffer[] = [];
712+
// Settles when the first connection is closed. The proxy does not close it, so the client did.
713+
const { promise: closed, resolve: onClosed } = Promise.withResolvers<void>();
714+
715+
function onTunnel(socket: net.Socket) {
716+
let head = "";
717+
socket.on("error", () => {});
718+
socket.on("data", function onHead(chunk: Buffer) {
719+
head += chunk.toString("latin1");
720+
if (!head.includes("\r\n\r\n")) return;
721+
socket.removeListener("data", onHead);
722+
connects.push(head.slice(0, head.indexOf("\r\n")));
723+
socket.on("data", (chunk: Buffer) => afterConnect.push(chunk));
724+
socket.write("HTTP/1.1 200 Connection established\r\n\r\n");
725+
});
726+
}
727+
728+
const proxy =
729+
scheme === "https"
730+
? tls.createServer({ key: tlsCerts.key, cert: tlsCerts.cert }, onTunnel)
731+
: net.createServer(onTunnel);
732+
// The TCP connection, for an https proxy too.
733+
proxy.on("connection", (socket: net.Socket) => {
734+
sockets.add(socket);
735+
socket.on("close", () => {
736+
sockets.delete(socket);
737+
onClosed();
738+
});
739+
});
740+
741+
function close() {
742+
for (const socket of sockets) socket.destroy();
743+
proxy.close();
744+
}
745+
return { proxy, connects, afterConnect, closed, close };
746+
}
747+
748+
// What the request emits, in order, up to and including its 'close'.
749+
function eventsOf(req: http.ClientRequest) {
750+
const events: unknown[] = [];
751+
const { promise, resolve } = Promise.withResolvers<unknown[]>();
752+
req.on("socket", () => events.push("socket"));
753+
req.on("response", res => {
754+
events.push(`response ${res.statusCode}`);
755+
res.resume();
756+
});
757+
req.on("timeout", () => events.push("timeout"));
758+
req.on("error", (err: NodeJS.ErrnoException) => events.push({ code: err.code, message: err.message }));
759+
req.on("close", () => {
760+
events.push("close");
761+
resolve(events);
762+
});
763+
return promise;
764+
}
765+
766+
for (const scheme of ["http", "https"]) {
767+
for (const { name, options, code } of scheme === "http" ? rejected : rejected.slice(0, 1)) {
768+
test(`${scheme} proxy: ${name}`, async () => {
769+
const defaults = scheme === "https" ? tls.getCACertificates("default") : undefined;
770+
const { proxy, connects, closed, close } = createHoldingProxy(scheme);
771+
const agent = proxiedAgent(`${scheme}://127.0.0.1:${await startProxy(proxy)}`);
772+
try {
773+
// The client has to trust the certificate of an https proxy.
774+
if (defaults) tls.setDefaultCACertificates([...defaults, tlsCerts.cert]);
775+
const expected = { code, message: thrownByTlsConnect(options)?.message };
776+
const req = https.get({ ...target, agent, ...options } as any);
777+
assert.deepStrictEqual(await eventsOf(req), [expected, "close"]);
778+
await closed;
779+
assert.deepStrictEqual(connects, ["CONNECT example.invalid:443 HTTP/1.1"]);
780+
} finally {
781+
agent.destroy();
782+
close();
783+
if (defaults) tls.setDefaultCACertificates(defaults);
784+
}
785+
});
786+
}
787+
}
788+
789+
test("agent.createConnection() gives its callback the error and the closed proxy socket", async () => {
790+
const { proxy, closed, close } = createHoldingProxy("http");
791+
const agent = proxiedAgent(`http://127.0.0.1:${await startProxy(proxy)}`);
792+
try {
793+
const calls: unknown[] = [];
794+
const { promise: called, resolve: onCalled } = Promise.withResolvers<void>();
795+
const proxySocket = (agent as any).createConnection(
796+
{ ...target, ...invalidVersion },
797+
(err: NodeJS.ErrnoException, socket: net.Socket) => {
798+
calls.push({ code: err.code, proxySocket: socket === proxySocket, destroyed: socket.destroyed });
799+
onCalled();
800+
},
801+
);
802+
await called;
803+
assert.deepStrictEqual(calls, [
804+
{ code: "ERR_TLS_INVALID_PROTOCOL_VERSION", proxySocket: true, destroyed: true },
805+
]);
806+
await closed;
807+
} finally {
808+
agent.destroy();
809+
close();
810+
}
811+
});
812+
813+
test("a falsy value thrown by tls.connect(): the proxy gets nothing of the request", async () => {
814+
const { proxy, connects, afterConnect, closed, close } = createHoldingProxy("http");
815+
const agent = proxiedAgent(`http://127.0.0.1:${await startProxy(proxy)}`);
816+
try {
817+
// agent.getName() reads the key too: throw at the first read after the CONNECT, which is tls.connect()'s.
818+
let thrown = false;
819+
const key = [
820+
{
821+
get pem() {
822+
if (connects.length === 0 || thrown) return tlsCerts.key;
823+
thrown = true;
824+
throw null;
825+
},
826+
},
827+
];
828+
const req = https.request({ ...target, agent, method: "POST", key } as any);
829+
const events = eventsOf(req);
830+
req.end("for the target only");
831+
// The Agent takes a falsy error for success. The socket that it gives the request is closed.
832+
const [socket] = await once(req, "socket");
833+
assert.strictEqual(socket.destroyed, true);
834+
assert.deepStrictEqual(await events, ["socket", { code: "ECONNRESET", message: "socket hang up" }, "close"]);
835+
await closed;
836+
assert.strictEqual(Buffer.concat(afterConnect).toString("latin1"), "");
837+
} finally {
838+
agent.destroy();
839+
close();
840+
}
841+
});
842+
843+
test("a request that waited behind maxTotalSockets", async () => {
844+
const { server, port, hostname } = await createHttpsServer();
845+
const proxy = createConnectProxy();
846+
let tunnels = 0;
847+
proxy.on("connection", () => tunnels++);
848+
const agent = proxiedAgent(`http://127.0.0.1:${await startProxy(proxy)}`, {
849+
ca: tlsCerts.ca,
850+
maxTotalSockets: 1,
851+
});
852+
try {
853+
const request = { hostname, port, path: "/", agent };
854+
const first = https.get(request);
855+
const firstEvents = eventsOf(first);
856+
// The Agent counts the socket of a tunnel from when the tunnel is there. Later requests then wait.
857+
await once(first, "socket");
858+
const rejectedEvents = eventsOf(https.get({ ...request, ...invalidVersion } as any));
859+
const lastEvents = eventsOf(https.get(request));
860+
assert.strictEqual(Object.values(agent.requests).flat().length, 2);
861+
862+
assert.deepStrictEqual(await rejectedEvents, [thrownByTlsConnect(invalidVersion), "close"]);
863+
assert.deepStrictEqual(await firstEvents, ["socket", "response 200", "close"]);
864+
assert.deepStrictEqual(await lastEvents, ["socket", "response 200", "close"]);
865+
assert.strictEqual(tunnels, 3);
866+
} finally {
867+
agent.destroy();
868+
server.close();
869+
proxy.close();
870+
}
871+
});
872+
},
873+
);
874+
673875
// Only run in Bun to avoid infinite loop when Node.js runs this file
674876
if (typeof Bun !== "undefined") {
675-
const { bunEnv, nodeExe } = await import("harness");
877+
const { bunEnv, bunExe, nodeExe } = await import("harness");
878+
879+
describe("https.request through a proxy from NODE_USE_ENV_PROXY=1", () => {
880+
test("the request gets what tls.connect() throws, and the process exits", async () => {
881+
// Answers each CONNECT with 200 and then holds the connection.
882+
const sockets = new Set<net.Socket>();
883+
const proxy = net.createServer(socket => {
884+
sockets.add(socket);
885+
socket.on("error", () => {});
886+
socket.once("data", () => socket.write("HTTP/1.1 200 Connection established\r\n\r\n"));
887+
});
888+
const port = await startProxy(proxy);
889+
try {
890+
// The global agent takes the proxy from the environment when node:https loads.
891+
const script = `
892+
const events = [];
893+
const req = require("node:https").get("https://example.invalid/", { minVersion: "TLSv9" });
894+
req.on("error", err => events.push(err.code));
895+
req.on("close", () => {
896+
events.push("close");
897+
console.log(JSON.stringify(events));
898+
});
899+
`;
900+
await using proc = Bun.spawn({
901+
cmd: [bunExe(), "-e", script],
902+
env: {
903+
...bunEnv,
904+
NODE_USE_ENV_PROXY: "1",
905+
HTTPS_PROXY: `http://127.0.0.1:${port}`,
906+
https_proxy: undefined,
907+
NO_PROXY: undefined,
908+
no_proxy: undefined,
909+
},
910+
stdout: "pipe",
911+
stderr: "pipe",
912+
});
913+
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
914+
// The proxy holds the connection. The process can only exit when the client closed it.
915+
assert.deepStrictEqual(
916+
{ stdout: stdout.trim(), stderr, exitCode },
917+
{ stdout: JSON.stringify(["ERR_TLS_INVALID_PROTOCOL_VERSION", "close"]), stderr: "", exitCode: 0 },
918+
);
919+
} finally {
920+
for (const socket of sockets) socket.destroy();
921+
proxy.close();
922+
}
923+
});
924+
});
676925

677926
describe("Node.js compatibility", () => {
678927
test("all tests pass in Node.js", async () => {
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// This tests that invalid TLS options do not result in an uncaught exception
2+
// after an HTTPS proxy tunnel has been established.
3+
4+
import * as common from '../common/index.mjs';
5+
import assert from 'node:assert';
6+
import { once } from 'events';
7+
import fixtures from '../common/fixtures.js';
8+
import { createProxyServer } from '../common/proxy-server.js';
9+
10+
if (!common.hasCrypto)
11+
common.skip('missing crypto');
12+
13+
// https must be dynamically imported so that builds without crypto support
14+
// can skip it.
15+
const { default: https } = await import('node:https');
16+
17+
const server = https.createServer({
18+
cert: fixtures.readKey('agent8-cert.pem'),
19+
key: fixtures.readKey('agent8-key.pem'),
20+
}, common.mustNotCall());
21+
server.on('error', common.mustNotCall());
22+
server.listen(0);
23+
await once(server, 'listening');
24+
25+
const { proxy, logs } = createProxyServer();
26+
proxy.listen(0);
27+
await once(proxy, 'listening');
28+
29+
const serverHost = `localhost:${server.address().port}`;
30+
31+
// tls.connect() throws for these options while building the secure context,
32+
// before any handshake happens.
33+
const testCases = [
34+
{ options: { minVersion: 'definitely-invalid' }, code: 'ERR_TLS_INVALID_PROTOCOL_VERSION' },
35+
{ options: { ciphers: 123 }, code: 'ERR_INVALID_ARG_TYPE' },
36+
{ options: { secureProtocol: 'definitely-invalid' }, code: 'ERR_TLS_INVALID_PROTOCOL_METHOD' },
37+
];
38+
39+
for (const { options, code } of testCases) {
40+
const agent = new https.Agent({
41+
ca: fixtures.readKey('fake-startcom-root-cert.pem'),
42+
proxyEnv: {
43+
HTTPS_PROXY: `http://localhost:${proxy.address().port}`,
44+
},
45+
});
46+
const req = https.get({
47+
host: 'localhost',
48+
port: server.address().port,
49+
path: '/test',
50+
agent,
51+
...options,
52+
}, common.mustNotCall());
53+
const [err] = await once(req, 'error');
54+
assert.strictEqual(err.code, code);
55+
agent.destroy();
56+
}
57+
58+
// Verify that the requests went through the proxy and the tunnel was established.
59+
const requests = logs.filter((log) => !('error' in log));
60+
// The client resets the tunnel as soon as tls.connect() throws, which the
61+
// proxy may observe as an ECONNRESET while still relaying it.
62+
const errors = logs.filter((log) =>
63+
'error' in log && log.error.code !== 'ECONNRESET');
64+
65+
assert.deepStrictEqual(requests, testCases.map(() => ({
66+
method: 'CONNECT',
67+
url: serverHost,
68+
headers: {
69+
'host': serverHost,
70+
},
71+
})));
72+
assert.deepStrictEqual(errors, []);
73+
74+
proxy.close();
75+
server.close();

0 commit comments

Comments
 (0)