Skip to content

Commit

Permalink
[security] Drop sensitive headers when following insecure redirects
Browse files Browse the repository at this point in the history
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
  • Loading branch information
lpinca committed May 26, 2022
1 parent 2758ed3 commit dc1781b
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 9 deletions.
25 changes: 16 additions & 9 deletions lib/websocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,7 @@ function initAsClient(websocket, address, protocols, options) {

if (opts.followRedirects) {
if (websocket._redirects === 0) {
websocket._originalSecure = isSecure;
websocket._originalHost = parsedUrl.host;

const headers = options && options.headers;
Expand All @@ -699,15 +700,21 @@ function initAsClient(websocket, address, protocols, options) {
options.headers[key.toLowerCase()] = value;
}
}
} else if (parsedUrl.host !== websocket._originalHost) {
//
// Match curl 7.77.0 behavior and drop the following headers. These
// headers are also dropped when following a redirect to a subdomain.
//
delete opts.headers.authorization;
delete opts.headers.cookie;
delete opts.headers.host;
opts.auth = undefined;
} else {
const isSameHost = parsedUrl.host === websocket._originalHost;

if (!isSameHost || (websocket._originalSecure && !isSecure)) {
//
// Match curl 7.77.0 behavior and drop the following headers. These
// headers are also dropped when following a redirect to a subdomain.
//
delete opts.headers.authorization;
delete opts.headers.cookie;

if (!isSameHost) delete opts.headers.host;

opts.auth = undefined;
}
}

//
Expand Down
128 changes: 128 additions & 0 deletions test/websocket.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const assert = require('assert');
const crypto = require('crypto');
const https = require('https');
const http = require('http');
const net = require('net');
const tls = require('tls');
const fs = require('fs');
const { URL } = require('url');
Expand Down Expand Up @@ -1037,6 +1038,133 @@ describe('WebSocket', () => {
});
});

describe('When moving away from a secure context', () => {
function proxy(httpServer, httpsServer) {
const server = net.createServer({ allowHalfOpen: true });

server.on('connection', (socket) => {
socket.on('readable', function read() {
socket.removeListener('readable', read);

const buf = socket.read(1);
const target = buf[0] === 22 ? httpsServer : httpServer;

socket.unshift(buf);
target.emit('connection', socket);
});
});

return server;
}

it('drops the `auth` option', (done) => {
const httpServer = http.createServer();
const httpsServer = https.createServer({
cert: fs.readFileSync('test/fixtures/certificate.pem'),
key: fs.readFileSync('test/fixtures/key.pem')
});
const server = proxy(httpServer, httpsServer);

server.listen(() => {
const port = server.address().port;

httpsServer.on('upgrade', (req, socket) => {
socket.on('error', NOOP);
socket.end(
'HTTP/1.1 302 Found\r\n' +
`Location: ws://localhost:${port}/\r\n\r\n`
);
});

const wss = new WebSocket.Server({ server: httpServer });

wss.on('connection', (ws, req) => {
assert.strictEqual(req.headers.authorization, undefined);
ws.close();
});

const ws = new WebSocket(`wss://localhost:${server.address().port}`, {
auth: 'foo:bar',
followRedirects: true,
rejectUnauthorized: false
});

assert.strictEqual(
ws._req.getHeader('Authorization'),
'Basic Zm9vOmJhcg=='
);

ws.on('close', (code) => {
assert.strictEqual(code, 1005);
assert.strictEqual(ws.url, `ws://localhost:${port}/`);
assert.strictEqual(ws._redirects, 1);

server.close(done);
});
});
});

it('drops the Authorization, and Cookie headers', (done) => {
const headers = {
authorization: 'Basic Zm9vOmJhcg==',
cookie: 'foo=bar',
host: 'foo'
};

const httpServer = http.createServer();
const httpsServer = https.createServer({
cert: fs.readFileSync('test/fixtures/certificate.pem'),
key: fs.readFileSync('test/fixtures/key.pem')
});
const server = proxy(httpServer, httpsServer);

server.listen(() => {
const port = server.address().port;

httpsServer.on('upgrade', (req, socket) => {
socket.on('error', NOOP);
socket.end(
'HTTP/1.1 302 Found\r\n' +
`Location: ws://localhost:${port}/\r\n\r\n`
);
});

const wss = new WebSocket.Server({ server: httpServer });

wss.on('connection', (ws, req) => {
assert.strictEqual(req.headers.authorization, undefined);
assert.strictEqual(req.headers.cookie, undefined);
assert.strictEqual(req.headers.host, 'foo');

ws.close();
});

const ws = new WebSocket(`wss://localhost:${server.address().port}`, {
headers,
followRedirects: true,
rejectUnauthorized: false
});

const firstRequest = ws._req;

assert.strictEqual(
firstRequest.getHeader('Authorization'),
headers.authorization
);
assert.strictEqual(firstRequest.getHeader('Cookie'), headers.cookie);
assert.strictEqual(firstRequest.getHeader('Host'), headers.host);

ws.on('close', (code) => {
assert.strictEqual(code, 1005);
assert.strictEqual(ws.url, `ws://localhost:${port}/`);
assert.strictEqual(ws._redirects, 1);

server.close(done);
});
});
});
});

describe('When the redirect host is different', () => {
it('drops the `auth` option', (done) => {
const wss = new WebSocket.Server({ port: 0 }, () => {
Expand Down

0 comments on commit dc1781b

Please sign in to comment.