Skip to content

Commit dc1781b

Browse files
committed
[security] Drop sensitive headers when following insecure redirects
Drop the `Authorization` and `Cookie` headers if the original request for the opening handshake is sent over HTTPS and the client is redirected to the same host over plain HTTP (wss: to ws:). If an HTTPS server redirects to same host over plain HTTP, the problem is on the server, but handling this condition is not hard and reduces the risk of leaking credentials due to MITM issues. Refs: 6946f5fe
1 parent 2758ed3 commit dc1781b

File tree

2 files changed

+144
-9
lines changed

2 files changed

+144
-9
lines changed

lib/websocket.js

+16-9
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,7 @@ function initAsClient(websocket, address, protocols, options) {
684684

685685
if (opts.followRedirects) {
686686
if (websocket._redirects === 0) {
687+
websocket._originalSecure = isSecure;
687688
websocket._originalHost = parsedUrl.host;
688689

689690
const headers = options && options.headers;
@@ -699,15 +700,21 @@ function initAsClient(websocket, address, protocols, options) {
699700
options.headers[key.toLowerCase()] = value;
700701
}
701702
}
702-
} else if (parsedUrl.host !== websocket._originalHost) {
703-
//
704-
// Match curl 7.77.0 behavior and drop the following headers. These
705-
// headers are also dropped when following a redirect to a subdomain.
706-
//
707-
delete opts.headers.authorization;
708-
delete opts.headers.cookie;
709-
delete opts.headers.host;
710-
opts.auth = undefined;
703+
} else {
704+
const isSameHost = parsedUrl.host === websocket._originalHost;
705+
706+
if (!isSameHost || (websocket._originalSecure && !isSecure)) {
707+
//
708+
// Match curl 7.77.0 behavior and drop the following headers. These
709+
// headers are also dropped when following a redirect to a subdomain.
710+
//
711+
delete opts.headers.authorization;
712+
delete opts.headers.cookie;
713+
714+
if (!isSameHost) delete opts.headers.host;
715+
716+
opts.auth = undefined;
717+
}
711718
}
712719

713720
//

test/websocket.test.js

+128
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const assert = require('assert');
66
const crypto = require('crypto');
77
const https = require('https');
88
const http = require('http');
9+
const net = require('net');
910
const tls = require('tls');
1011
const fs = require('fs');
1112
const { URL } = require('url');
@@ -1037,6 +1038,133 @@ describe('WebSocket', () => {
10371038
});
10381039
});
10391040

1041+
describe('When moving away from a secure context', () => {
1042+
function proxy(httpServer, httpsServer) {
1043+
const server = net.createServer({ allowHalfOpen: true });
1044+
1045+
server.on('connection', (socket) => {
1046+
socket.on('readable', function read() {
1047+
socket.removeListener('readable', read);
1048+
1049+
const buf = socket.read(1);
1050+
const target = buf[0] === 22 ? httpsServer : httpServer;
1051+
1052+
socket.unshift(buf);
1053+
target.emit('connection', socket);
1054+
});
1055+
});
1056+
1057+
return server;
1058+
}
1059+
1060+
it('drops the `auth` option', (done) => {
1061+
const httpServer = http.createServer();
1062+
const httpsServer = https.createServer({
1063+
cert: fs.readFileSync('test/fixtures/certificate.pem'),
1064+
key: fs.readFileSync('test/fixtures/key.pem')
1065+
});
1066+
const server = proxy(httpServer, httpsServer);
1067+
1068+
server.listen(() => {
1069+
const port = server.address().port;
1070+
1071+
httpsServer.on('upgrade', (req, socket) => {
1072+
socket.on('error', NOOP);
1073+
socket.end(
1074+
'HTTP/1.1 302 Found\r\n' +
1075+
`Location: ws://localhost:${port}/\r\n\r\n`
1076+
);
1077+
});
1078+
1079+
const wss = new WebSocket.Server({ server: httpServer });
1080+
1081+
wss.on('connection', (ws, req) => {
1082+
assert.strictEqual(req.headers.authorization, undefined);
1083+
ws.close();
1084+
});
1085+
1086+
const ws = new WebSocket(`wss://localhost:${server.address().port}`, {
1087+
auth: 'foo:bar',
1088+
followRedirects: true,
1089+
rejectUnauthorized: false
1090+
});
1091+
1092+
assert.strictEqual(
1093+
ws._req.getHeader('Authorization'),
1094+
'Basic Zm9vOmJhcg=='
1095+
);
1096+
1097+
ws.on('close', (code) => {
1098+
assert.strictEqual(code, 1005);
1099+
assert.strictEqual(ws.url, `ws://localhost:${port}/`);
1100+
assert.strictEqual(ws._redirects, 1);
1101+
1102+
server.close(done);
1103+
});
1104+
});
1105+
});
1106+
1107+
it('drops the Authorization, and Cookie headers', (done) => {
1108+
const headers = {
1109+
authorization: 'Basic Zm9vOmJhcg==',
1110+
cookie: 'foo=bar',
1111+
host: 'foo'
1112+
};
1113+
1114+
const httpServer = http.createServer();
1115+
const httpsServer = https.createServer({
1116+
cert: fs.readFileSync('test/fixtures/certificate.pem'),
1117+
key: fs.readFileSync('test/fixtures/key.pem')
1118+
});
1119+
const server = proxy(httpServer, httpsServer);
1120+
1121+
server.listen(() => {
1122+
const port = server.address().port;
1123+
1124+
httpsServer.on('upgrade', (req, socket) => {
1125+
socket.on('error', NOOP);
1126+
socket.end(
1127+
'HTTP/1.1 302 Found\r\n' +
1128+
`Location: ws://localhost:${port}/\r\n\r\n`
1129+
);
1130+
});
1131+
1132+
const wss = new WebSocket.Server({ server: httpServer });
1133+
1134+
wss.on('connection', (ws, req) => {
1135+
assert.strictEqual(req.headers.authorization, undefined);
1136+
assert.strictEqual(req.headers.cookie, undefined);
1137+
assert.strictEqual(req.headers.host, 'foo');
1138+
1139+
ws.close();
1140+
});
1141+
1142+
const ws = new WebSocket(`wss://localhost:${server.address().port}`, {
1143+
headers,
1144+
followRedirects: true,
1145+
rejectUnauthorized: false
1146+
});
1147+
1148+
const firstRequest = ws._req;
1149+
1150+
assert.strictEqual(
1151+
firstRequest.getHeader('Authorization'),
1152+
headers.authorization
1153+
);
1154+
assert.strictEqual(firstRequest.getHeader('Cookie'), headers.cookie);
1155+
assert.strictEqual(firstRequest.getHeader('Host'), headers.host);
1156+
1157+
ws.on('close', (code) => {
1158+
assert.strictEqual(code, 1005);
1159+
assert.strictEqual(ws.url, `ws://localhost:${port}/`);
1160+
assert.strictEqual(ws._redirects, 1);
1161+
1162+
server.close(done);
1163+
});
1164+
});
1165+
});
1166+
});
1167+
10401168
describe('When the redirect host is different', () => {
10411169
it('drops the `auth` option', (done) => {
10421170
const wss = new WebSocket.Server({ port: 0 }, () => {

0 commit comments

Comments
 (0)