Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for mTLS #235

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/polite-pillows-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'https-proxy-agent': minor
'socks-proxy-agent': minor
'pac-proxy-agent': minor
'agent-base': minor
---

Add support for mTLS to proxy agent
34 changes: 32 additions & 2 deletions packages/agent-base/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,37 @@ export type AgentConnectOpts = HttpConnectOpts | HttpsConnectOpts;
const INTERNAL = Symbol('AgentBaseInternalState');

interface InternalState {
tlsUpgradeOpts?: TlsUpgradeOpts;
defaultPort?: number;
protocol?: string;
currentSocket?: Duplex;
}

export interface TlsUpgradeOpts {
cert?: string;
key?: string;
ca?: string;
}
TooTallNate marked this conversation as resolved.
Show resolved Hide resolved

export interface BaseAgentOptions extends http.AgentOptions {
tlsUpgradeOpts?: TlsUpgradeOpts;
}

export abstract class Agent extends http.Agent {
private [INTERNAL]: InternalState;

// Set by `http.Agent` - missing from `@types/node`
options!: Partial<net.TcpNetConnectOpts & tls.ConnectionOptions>;
keepAlive!: boolean;

constructor(opts?: http.AgentOptions) {
constructor(opts?: BaseAgentOptions) {
const tlsUpgradeOpts = opts?.tlsUpgradeOpts;
delete opts?.tlsUpgradeOpts;

super(opts);
this[INTERNAL] = {};
this[INTERNAL] = {
tlsUpgradeOpts,
};
}

abstract connect(
Expand Down Expand Up @@ -99,6 +115,20 @@ export abstract class Agent extends http.Agent {
}, cb);
}

upgradeSocketToTls(
servername: string | undefined,
opts: tls.ConnectionOptions,
socket?: net.Socket,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems weird to me that socket is optional. Also, let's make it the first parameter:

Suggested change
servername: string | undefined,
opts: tls.ConnectionOptions,
socket?: net.Socket,
socket: net.Socket,
servername: string | undefined,
opts: tls.ConnectionOptions,

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason it was optional was that pac-proxy-agent calls tls.connect without providing a socket. I've made your suggested changes and reverted the changes to pac-proxy-agent for now. Let me know how you would like to proceed in that regard.

Copy link
Owner

@TooTallNate TooTallNate Sep 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. How about if we do something like this in pac-proxy-agent?

	socket = net.connect(opts);
	
	if (secureEndpoint) {
		const servername = opts.servername || opts.host;
		socket = this.upgradeSocketToTls(
			socket,
			servername,
			opts
		);
	}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mg-dd Did this piece of feedback get addressed? I don't see any change in pac-proxy-agent so it looks like this still needs to be done?

): net.Socket {
TooTallNate marked this conversation as resolved.
Show resolved Hide resolved
const tlsUpgradeOpts = this[INTERNAL].tlsUpgradeOpts ?? {};
return tls.connect({
...opts,
socket,
servername: !servername || net.isIP(servername) ? undefined : servername,
...(tlsUpgradeOpts)
});
}

createConnection(): Duplex {
const socket = this[INTERNAL].currentSocket;
this[INTERNAL].currentSocket = undefined;
Expand Down
12 changes: 6 additions & 6 deletions packages/https-proxy-agent/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as http from 'http';
import assert from 'assert';
import createDebug from 'debug';
import type { OutgoingHttpHeaders } from 'http';
import { Agent, AgentConnectOpts } from 'agent-base';
import { Agent, AgentConnectOpts, BaseAgentOptions } from 'agent-base';
import { parseProxyResponse } from './parse-proxy-response';

const debug = createDebug('https-proxy-agent');
Expand All @@ -24,7 +24,7 @@ type ConnectOpts<T> = {
}[keyof ConnectOptsMap];

export type HttpsProxyAgentOptions<T> = ConnectOpts<T> &
http.AgentOptions & {
BaseAgentOptions & {
headers?: OutgoingHttpHeaders | (() => OutgoingHttpHeaders);
};

Expand Down Expand Up @@ -141,11 +141,11 @@ export class HttpsProxyAgent<Uri extends string> extends Agent {
// this socket connection to a TLS connection.
debug('Upgrading socket connection to TLS');
const servername = opts.servername || opts.host;
return tls.connect({
...omit(opts, 'host', 'path', 'port'),
return this.upgradeSocketToTls(
servername,
omit(opts, 'host', 'path', 'port'),
socket,
servername: net.isIP(servername) ? undefined : servername,
});
);
}

return socket;
Expand Down
21 changes: 21 additions & 0 deletions packages/https-proxy-agent/test/mtls-ca-cert-snakeoil.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
-----BEGIN CERTIFICATE-----
MIIDazCCAlOgAwIBAgIUKbRy6yNL6K0Tu8LAuMyrkIFAQFUwDQYJKoZIhvcNAQEL
BQAwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yMzA4MjIwMjMyMjJaFw0yOTAz
MjkwMjMyMjJaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw
HwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggEiMA0GCSqGSIb3DQEB
AQUAA4IBDwAwggEKAoIBAQChBv4GZh9xJ50jFRsYqkkl3Ie0DPqAATec8pcYRuge
k+vbE7kT9FvW4TGmjXT73BB+KRBikIsJcmMwt7zfXBpcW1FSptAA6uk3AslPZM4i
hCZ8OGYiDwTyIvGXM/AEL3deBfYBlHyxEYeqO5K4qWRKfwgQYjpcFEePxRmWqgaB
KpAPxjQXPURiBL+4HYTOseZ572tyPQwfv5t2NGzWk7mTiv3GAiQc3pjTRuz3r64r
x+rg3HCJapeiuJ41zUHVxnQaIvZzmtkE0GdiW3h7moi+bGUgD70xAtymOOR/KgBF
qMOpGMr30RG5kmY5pfhCDKTAiggdozjwVGD4ZMpQ54nvAgMBAAGjUzBRMB0GA1Ud
DgQWBBS5bTdNwT2MRWe2nyI2NMk/cGUx6zAfBgNVHSMEGDAWgBS5bTdNwT2MRWe2
nyI2NMk/cGUx6zAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQB6
rWSa9vPK1V7B4WAXE411wO2Yk2rTLjNwVEvCkF2goRgu32Ew4vi/MHVktLJ7+d8e
ipU109uJbaGl17CXIpuS3fFrYViU7adLDQqvW9q5CFkTYdFoO1aSei+FkpSUOa1H
/DbwIqBZu4rQVpZ2KPtkzbpu0Vr5nxZVuCumP7y7ueTiuIgRmTjWKBDdZ52xhiS5
f5xkKHGt6gmYNkhJYfuy3zraNEWYFWugnpjadJ7QsGO7agVMv8x+P71S2NwjBr6H
0M/ecRczS93IjiWOywIxTrVengbR5C5s2/1rdwIVj0aGA5cIYLN3ezbbSb/yzKFl
xlTNRDEoApDINojsni25
-----END CERTIFICATE-----
28 changes: 28 additions & 0 deletions packages/https-proxy-agent/test/mtls-ca-key-snakeoil.key
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQChBv4GZh9xJ50j
FRsYqkkl3Ie0DPqAATec8pcYRugek+vbE7kT9FvW4TGmjXT73BB+KRBikIsJcmMw
t7zfXBpcW1FSptAA6uk3AslPZM4ihCZ8OGYiDwTyIvGXM/AEL3deBfYBlHyxEYeq
O5K4qWRKfwgQYjpcFEePxRmWqgaBKpAPxjQXPURiBL+4HYTOseZ572tyPQwfv5t2
NGzWk7mTiv3GAiQc3pjTRuz3r64rx+rg3HCJapeiuJ41zUHVxnQaIvZzmtkE0Gdi
W3h7moi+bGUgD70xAtymOOR/KgBFqMOpGMr30RG5kmY5pfhCDKTAiggdozjwVGD4
ZMpQ54nvAgMBAAECggEAGW9xULFw5f7L427CCPNc+o4LIXWrW+zVTAVFuJ/6qlnT
N5e19GD04MxRe218vQvVzxfNbsRGMgfPgzKCswVpZI1IGzYeRQbWL8pQ4imaJfmZ
2qVN/LNCpLzATJH3p7GXuVJXuYgq6g1K0Kj4VBLttJa8P1pEvaa2Zw9LK3FP9bbz
OIr5R1gzxM6gitfnv95toMXs7iYZq692Qu0KwTHLu4fZbO9X6msCZps+SaXs2Irm
hNOh49CXp4GaeehtA8BpIOtk+hqld160eDmUfjWA4dgNgWDeJHrLv0sFVgW9+csg
3wDAq9wqJN6MoecBeYGeToHDR6GfrGazIkJCp0pCAQKBgQDTdq4L0g6QV8jBXKtq
ICZPjQhQHO9ERY4iU3Y9Hfv5W8DsUTEcyPBL/xlm8elLb+Da38rbShi8gvCa+6mG
ied6S0jIkk5/1iJj8AcoiqzuO5t0tJgg5vWG59U4AL+p+UJKJXhy5bn0SQTx0pm8
VXeUlPhyULQyWoUV0+7yx1P6kQKBgQDC8Ppv1GETE0aLYMa2x7cs56vQDgIDjf+S
AQdxcj+Xh6BmaFV1eqJo4bBeuuOBPRiLde/I5XDUl9Y1MVzu1AvDkWlETCOa0GgV
FJptbIbAUFhGcXFKpgXPwch0rZ9CIXFKLbms1AxIitj9sI2JEU2mIHz3TJ1WKgd9
kSMGYuB8fwKBgQDBn6Gt4SDEuhxwMNIj0lxB6vj5ogTTlnyWoaVaQOI/aOw1dgJq
QPMGIwa3ZDohgMd6of+02xvbQGne/yAyuILjT5vNS2nlU8UZjOaBELlXSe0F53aE
afXXGN/673SPxlQUYErxqbejHlkQs61g7UEZI255/buBf7DsU6ovUqRqgQKBgQCP
t5EqeOzIX2GWx9Y4Uqzc0j98t5cMf9d3EIMoRA5V7It4dFrsrWim4hxg+m9XjG9Y
Aa6x8VVppjcqKCZXfMTffYiZ7XgiXpsdT386RNRjW9h4tpHLcvK95COgwY+z9WvM
QEGvBPW5VYP8I/l0n+kbHMCEoVstdvbmv+WOg770iQKBgQC5Hnp/t+SXpWi2T+XU
4STcb7m0BqIb1052rv7trKpOaUBHVuyv3OfVlLWfVz7YpLX+a+ze8wfAOyKxZ0i1
H6P/FQT5WFky7KYI2WJ2YoN8vtO1pf8YGbgpIwrdzgKbofQFT/FX1VTyAbYAXviu
ly3RR0sA/t1GsSPaQkO3QvNIIg==
-----END PRIVATE KEY-----
15 changes: 15 additions & 0 deletions packages/https-proxy-agent/test/mtls-cert-snakeoil.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
-----BEGIN CERTIFICATE-----
MIICRTCCAS0CFBciMVWjgILMHIuw1oMonCGdHhGiMA0GCSqGSIb3DQEBCwUAMEUx
CzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRl
cm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMjMwODIyMDIzNTA5WhcNMjkwNDA4MDIz
NTA5WjBEMQswCQYDVQQGEwJVUzELMAkGA1UECAwCQ0ExFDASBgNVBAoMC015T3Jn
LCBJbmMuMRIwEAYDVQQDDAlsb2NhbGhvc3QwWTATBgcqhkjOPQIBBggqhkjOPQMB
BwNCAAQDwHmidm+bBPQuyfGji9t+UoXv0w/jHDQa9BrxNfozrvlSTT0VOypbFJ0k
Y5vVksLqvPBrkNRuVgGMJHqrSxUXMA0GCSqGSIb3DQEBCwUAA4IBAQCAF1cyeirb
QJXx6kWL4QFN12xBWH0I8apCrTey/LNMOAn951Wb9/BLv78Z89uz+ifor+oXn7o+
Czu+q55KF4Gy/8qPzVEdkeR9iuUJJSTKDFaJqdoy01wVJcY0WRqjm/3zvXhpPWUi
AXLRB/iUtk7pbMFbSnoe1sAztBhOM9kJ2i5aTrhn4+aXXuoAHL2IUDQFKR68tQaT
tpRaA3PiIgpW/u+1j3d5/TxJkLIyFuP0JNq5yFfWqlg73B8vWqrduNx37kR8uhI4
tO1ICQdMaTivMyVCO9OvYHFeSWsZKJu9UtVc/HgaTMtPLlgcv3JZtlMWC+tL/FtD
3hnPKpl2s2q9
-----END CERTIFICATE-----
5 changes: 5 additions & 0 deletions packages/https-proxy-agent/test/mtls-key-snakeoil.key
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIE9ZOyM1v50YOCaC6hnZ7I88qk5A9wOfP7YAENVligN1oAoGCCqGSM49
AwEHoUQDQgAEA8B5onZvmwT0Lsnxo4vbflKF79MP4xw0GvQa8TX6M675Uk09FTsq
WxSdJGOb1ZLC6rzwa5DUblYBjCR6q0sVFw==
-----END EC PRIVATE KEY-----
39 changes: 39 additions & 0 deletions packages/https-proxy-agent/test/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as fs from 'fs';
import * as net from 'net';
import * as http from 'http';
import * as https from 'https';
import * as path from 'path';
import assert from 'assert';
import { once } from 'events';
import { listen } from 'async-listen';
Expand All @@ -24,6 +25,9 @@ describe('HttpsProxyAgent', () => {
let sslServer: https.Server;
let sslServerUrl: URL;

let mtlsSslServer: https.Server;
let mtlsSslServerUrl: URL;

let proxy: ProxyServer;
let proxyUrl: URL;

Expand All @@ -48,6 +52,16 @@ describe('HttpsProxyAgent', () => {
sslServerUrl = await listen(sslServer);
});

beforeAll(async () => {
// setup target HTTPS server that requires mTLS
mtlsSslServer = https.createServer({
...sslOptions,
requestCert: true,
ca: fs.readFileSync(path.join(__dirname, 'mtls-ca-cert-snakeoil.pem')),
});
mtlsSslServerUrl = await listen(mtlsSslServer);
});

beforeAll(async () => {
// setup SSL HTTP proxy server
sslProxy = createProxy(https.createServer(sslOptions));
Expand All @@ -65,6 +79,7 @@ describe('HttpsProxyAgent', () => {
proxy.close();
sslServer.close();
sslProxy.close();
mtlsSslServer.close();
});

describe('constructor', () => {
Expand Down Expand Up @@ -399,5 +414,29 @@ describe('HttpsProxyAgent', () => {
agent.destroy();
}
});

it('should work with mTLS', async () => {
mtlsSslServer.once('request', (req, res) => {
if (!(req as any).client.authorized) {
res.writeHead(401).end();
return;
}
res.writeHead(200).end();
});

const agent = new HttpsProxyAgent(proxyUrl, {
tlsUpgradeOpts: {
cert: fs.readFileSync(path.join(__dirname, 'mtls-cert-snakeoil.crt')).toString('utf-8'),
key: fs.readFileSync(path.join(__dirname, 'mtls-key-snakeoil.key')).toString('utf-8'),
},
});

try {
const res = await req(mtlsSslServerUrl, { agent, rejectUnauthorized: false });
expect(res.statusCode).toEqual(200);
} finally {
agent.destroy();
}
});
});
});
12 changes: 3 additions & 9 deletions packages/pac-proxy-agent/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { once } from 'events';
import createDebug from 'debug';
import { Readable } from 'stream';
import { format } from 'url';
import { Agent, AgentConnectOpts, toBuffer } from 'agent-base';
import { Agent, AgentConnectOpts, BaseAgentOptions, toBuffer } from 'agent-base';
import { HttpProxyAgent, HttpProxyAgentOptions } from 'http-proxy-agent';
import { HttpsProxyAgent, HttpsProxyAgentOptions } from 'https-proxy-agent';
import { SocksProxyAgent, SocksProxyAgentOptions } from 'socks-proxy-agent';
Expand Down Expand Up @@ -34,7 +34,7 @@ type Protocol<T> = T extends `pac+${infer P}:${infer _}`
? P
: never;

export type PacProxyAgentOptions<T> = http.AgentOptions &
export type PacProxyAgentOptions<T> = BaseAgentOptions &
PacResolverOptions &
GetUriOptions<`${Protocol<T>}:`> &
HttpProxyAgentOptions<''> &
Expand Down Expand Up @@ -238,13 +238,7 @@ export class PacProxyAgent<Uri extends string> extends Agent {
// Direct connection to the destination endpoint
if (secureEndpoint) {
const servername = opts.servername || opts.host;
socket = tls.connect({
...opts,
servername:
!servername || net.isIP(servername)
? undefined
: servername,
});
socket = this.upgradeSocketToTls(servername, opts);
} else {
socket = net.connect(opts);
}
Expand Down
14 changes: 7 additions & 7 deletions packages/socks-proxy-agent/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { SocksClient, SocksProxy, SocksClientOptions } from 'socks';
TooTallNate marked this conversation as resolved.
Show resolved Hide resolved
import { Agent, AgentConnectOpts } from 'agent-base';
import { Agent, AgentConnectOpts, BaseAgentOptions } from 'agent-base';
import createDebug from 'debug';
import * as dns from 'dns';
import * as net from 'net';
import * as tls from 'tls';

Check warning on line 6 in packages/socks-proxy-agent/src/index.ts

View workflow job for this annotation

GitHub Actions / Lint

'tls' is defined but never used
import * as http from 'http';

const debug = createDebug('socks-proxy-agent');
Expand Down Expand Up @@ -75,7 +75,7 @@
// These come from the parsed URL
'ipaddress' | 'host' | 'port' | 'type' | 'userId' | 'password'
> &
http.AgentOptions;
BaseAgentOptions;

export class SocksProxyAgent extends Agent {
static protocols = [
Expand Down Expand Up @@ -142,7 +142,7 @@
timeout: timeout ?? undefined,
};

const cleanup = (tlsSocket?: tls.TLSSocket) => {
const cleanup = (tlsSocket?: net.Socket) => {
TooTallNate marked this conversation as resolved.
Show resolved Hide resolved
req.destroy();
socket.destroy();
if (tlsSocket) tlsSocket.destroy();
Expand All @@ -162,11 +162,11 @@
// this socket connection to a TLS connection.
debug('Upgrading socket connection to TLS');
const servername = opts.servername || opts.host;
const tlsSocket = tls.connect({
...omit(opts, 'host', 'path', 'port'),
const tlsSocket = this.upgradeSocketToTls(
servername,
omit(opts, 'host', 'path', 'port'),
socket,
servername: net.isIP(servername) ? undefined : servername,
});
);

tlsSocket.once('error', (error) => {
debug('Socket TLS error', error.message);
Expand Down