Skip to content

Commit d68ba9e

Browse files
committedMay 26, 2022
[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 a690791 commit d68ba9e

File tree

2 files changed

+227
-12
lines changed

2 files changed

+227
-12
lines changed
 

‎lib/websocket.js

+16-12
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,7 @@ function initAsClient(websocket, address, protocols, options) {
771771

772772
if (opts.followRedirects) {
773773
if (websocket._redirects === 0) {
774+
websocket._originalSecure = isSecure;
774775
websocket._originalHost = parsedUrl.host;
775776

776777
const headers = options && options.headers;
@@ -786,18 +787,21 @@ function initAsClient(websocket, address, protocols, options) {
786787
options.headers[key.toLowerCase()] = value;
787788
}
788789
}
789-
} else if (
790-
websocket.listenerCount('redirect') === 0 &&
791-
parsedUrl.host !== websocket._originalHost
792-
) {
793-
//
794-
// Match curl 7.77.0 behavior and drop the following headers. These
795-
// headers are also dropped when following a redirect to a subdomain.
796-
//
797-
delete opts.headers.authorization;
798-
delete opts.headers.cookie;
799-
delete opts.headers.host;
800-
opts.auth = undefined;
790+
} else if (websocket.listenerCount('redirect') === 0) {
791+
const isSameHost = parsedUrl.host === websocket._originalHost;
792+
793+
if (!isSameHost || (websocket._originalSecure && !isSecure)) {
794+
//
795+
// Match curl 7.77.0 behavior and drop the following headers. These
796+
// headers are also dropped when following a redirect to a subdomain.
797+
//
798+
delete opts.headers.authorization;
799+
delete opts.headers.cookie;
800+
801+
if (!isSameHost) delete opts.headers.host;
802+
803+
opts.auth = undefined;
804+
}
801805
}
802806

803807
//

‎test/websocket.test.js

+211
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');
@@ -1226,6 +1227,216 @@ describe('WebSocket', () => {
12261227
});
12271228
});
12281229

1230+
describe('When moving away from a secure context', () => {
1231+
function proxy(httpServer, httpsServer) {
1232+
const server = net.createServer({ allowHalfOpen: true });
1233+
1234+
server.on('connection', (socket) => {
1235+
socket.on('readable', function read() {
1236+
socket.removeListener('readable', read);
1237+
1238+
const buf = socket.read(1);
1239+
const target = buf[0] === 22 ? httpsServer : httpServer;
1240+
1241+
socket.unshift(buf);
1242+
target.emit('connection', socket);
1243+
});
1244+
});
1245+
1246+
return server;
1247+
}
1248+
1249+
describe("If there is no 'redirect' event listener", () => {
1250+
it('drops the `auth` option', (done) => {
1251+
const httpServer = http.createServer();
1252+
const httpsServer = https.createServer({
1253+
cert: fs.readFileSync('test/fixtures/certificate.pem'),
1254+
key: fs.readFileSync('test/fixtures/key.pem')
1255+
});
1256+
const server = proxy(httpServer, httpsServer);
1257+
1258+
server.listen(() => {
1259+
const port = server.address().port;
1260+
1261+
httpsServer.on('upgrade', (req, socket) => {
1262+
socket.on('error', NOOP);
1263+
socket.end(
1264+
'HTTP/1.1 302 Found\r\n' +
1265+
`Location: ws://localhost:${port}/\r\n\r\n`
1266+
);
1267+
});
1268+
1269+
const wss = new WebSocket.Server({ server: httpServer });
1270+
1271+
wss.on('connection', (ws, req) => {
1272+
assert.strictEqual(req.headers.authorization, undefined);
1273+
ws.close();
1274+
});
1275+
1276+
const ws = new WebSocket(
1277+
`wss://localhost:${server.address().port}`,
1278+
{
1279+
auth: 'foo:bar',
1280+
followRedirects: true,
1281+
rejectUnauthorized: false
1282+
}
1283+
);
1284+
1285+
assert.strictEqual(
1286+
ws._req.getHeader('Authorization'),
1287+
'Basic Zm9vOmJhcg=='
1288+
);
1289+
1290+
ws.on('close', (code) => {
1291+
assert.strictEqual(code, 1005);
1292+
assert.strictEqual(ws.url, `ws://localhost:${port}/`);
1293+
assert.strictEqual(ws._redirects, 1);
1294+
1295+
server.close(done);
1296+
});
1297+
});
1298+
});
1299+
1300+
it('drops the Authorization, and Cookie headers', (done) => {
1301+
const headers = {
1302+
authorization: 'Basic Zm9vOmJhcg==',
1303+
cookie: 'foo=bar',
1304+
host: 'foo'
1305+
};
1306+
1307+
const httpServer = http.createServer();
1308+
const httpsServer = https.createServer({
1309+
cert: fs.readFileSync('test/fixtures/certificate.pem'),
1310+
key: fs.readFileSync('test/fixtures/key.pem')
1311+
});
1312+
const server = proxy(httpServer, httpsServer);
1313+
1314+
server.listen(() => {
1315+
const port = server.address().port;
1316+
1317+
httpsServer.on('upgrade', (req, socket) => {
1318+
socket.on('error', NOOP);
1319+
socket.end(
1320+
'HTTP/1.1 302 Found\r\n' +
1321+
`Location: ws://localhost:${port}/\r\n\r\n`
1322+
);
1323+
});
1324+
1325+
const wss = new WebSocket.Server({ server: httpServer });
1326+
1327+
wss.on('connection', (ws, req) => {
1328+
assert.strictEqual(req.headers.authorization, undefined);
1329+
assert.strictEqual(req.headers.cookie, undefined);
1330+
assert.strictEqual(req.headers.host, 'foo');
1331+
1332+
ws.close();
1333+
});
1334+
1335+
const ws = new WebSocket(
1336+
`wss://localhost:${server.address().port}`,
1337+
{ headers, followRedirects: true, rejectUnauthorized: false }
1338+
);
1339+
1340+
const firstRequest = ws._req;
1341+
1342+
assert.strictEqual(
1343+
firstRequest.getHeader('Authorization'),
1344+
headers.authorization
1345+
);
1346+
assert.strictEqual(
1347+
firstRequest.getHeader('Cookie'),
1348+
headers.cookie
1349+
);
1350+
assert.strictEqual(firstRequest.getHeader('Host'), headers.host);
1351+
1352+
ws.on('close', (code) => {
1353+
assert.strictEqual(code, 1005);
1354+
assert.strictEqual(ws.url, `ws://localhost:${port}/`);
1355+
assert.strictEqual(ws._redirects, 1);
1356+
1357+
server.close(done);
1358+
});
1359+
});
1360+
});
1361+
});
1362+
1363+
describe("If there is at least one 'redirect' event listener", () => {
1364+
it('does not drop any headers by default', (done) => {
1365+
const headers = {
1366+
authorization: 'Basic Zm9vOmJhcg==',
1367+
cookie: 'foo=bar',
1368+
host: 'foo'
1369+
};
1370+
1371+
const httpServer = http.createServer();
1372+
const httpsServer = https.createServer({
1373+
cert: fs.readFileSync('test/fixtures/certificate.pem'),
1374+
key: fs.readFileSync('test/fixtures/key.pem')
1375+
});
1376+
const server = proxy(httpServer, httpsServer);
1377+
1378+
server.listen(() => {
1379+
const port = server.address().port;
1380+
1381+
httpsServer.on('upgrade', (req, socket) => {
1382+
socket.on('error', NOOP);
1383+
socket.end(
1384+
'HTTP/1.1 302 Found\r\n' +
1385+
`Location: ws://localhost:${port}/\r\n\r\n`
1386+
);
1387+
});
1388+
1389+
const wss = new WebSocket.Server({ server: httpServer });
1390+
1391+
wss.on('connection', (ws, req) => {
1392+
assert.strictEqual(
1393+
req.headers.authorization,
1394+
headers.authorization
1395+
);
1396+
assert.strictEqual(req.headers.cookie, headers.cookie);
1397+
assert.strictEqual(req.headers.host, headers.host);
1398+
1399+
ws.close();
1400+
});
1401+
1402+
const ws = new WebSocket(
1403+
`wss://localhost:${server.address().port}`,
1404+
{ headers, followRedirects: true, rejectUnauthorized: false }
1405+
);
1406+
1407+
const firstRequest = ws._req;
1408+
1409+
assert.strictEqual(
1410+
firstRequest.getHeader('Authorization'),
1411+
headers.authorization
1412+
);
1413+
assert.strictEqual(
1414+
firstRequest.getHeader('Cookie'),
1415+
headers.cookie
1416+
);
1417+
assert.strictEqual(firstRequest.getHeader('Host'), headers.host);
1418+
1419+
ws.on('redirect', (url, req) => {
1420+
assert.strictEqual(ws._redirects, 1);
1421+
assert.strictEqual(url, `ws://localhost:${port}/`);
1422+
assert.notStrictEqual(firstRequest, req);
1423+
assert.strictEqual(
1424+
req.getHeader('Authorization'),
1425+
headers.authorization
1426+
);
1427+
assert.strictEqual(req.getHeader('Cookie'), headers.cookie);
1428+
assert.strictEqual(req.getHeader('Host'), headers.host);
1429+
1430+
ws.on('close', (code) => {
1431+
assert.strictEqual(code, 1005);
1432+
server.close(done);
1433+
});
1434+
});
1435+
});
1436+
});
1437+
});
1438+
});
1439+
12291440
describe('When the redirect host is different', () => {
12301441
describe("If there is no 'redirect' event listener", () => {
12311442
it('drops the `auth` option', (done) => {

0 commit comments

Comments
 (0)
Please sign in to comment.