mirror of
https://github.com/nodejs/node.git
synced 2026-11-09 15:11:11 -05:00
tls: route callback exceptions through error handlers
Wrap pskCallback and ALPNCallback invocations in try-catch blocks to route exceptions through owner.destroy() instead of letting them become uncaught exceptions. This prevents remote attackers from crashing TLS servers or causing resource exhaustion. Fixes: https://hackerone.com/reports/3473882 PR-URL: https://github.com/nodejs-private/node-private/pull/782 PR-URL: https://github.com/nodejs-private/node-private/pull/796 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> CVE-ID: CVE-2026-21637
This commit is contained in:
1 parent
37509c3ff0
commit
25d6799df6
3 files changed
+447
-86
No files matched your search
+92
-75
@@ -235,39 +235,44 @@ function callALPNCallback(protocolsBuffer) {
|
||||
const handle = this;
|
||||
const socket = handle[owner_symbol];
|
||||
|
||||
const servername = handle.getServername();
|
||||
try {
|
||||
const servername = handle.getServername();
|
||||
|
||||
// Collect all the protocols from the given buffer:
|
||||
const protocols = [];
|
||||
let offset = 0;
|
||||
while (offset < protocolsBuffer.length) {
|
||||
const protocolLen = protocolsBuffer[offset];
|
||||
offset += 1;
|
||||
// Collect all the protocols from the given buffer:
|
||||
const protocols = [];
|
||||
let offset = 0;
|
||||
while (offset < protocolsBuffer.length) {
|
||||
const protocolLen = protocolsBuffer[offset];
|
||||
offset += 1;
|
||||
|
||||
const protocol = protocolsBuffer.slice(offset, offset + protocolLen);
|
||||
offset += protocolLen;
|
||||
const protocol = protocolsBuffer.slice(offset, offset + protocolLen);
|
||||
offset += protocolLen;
|
||||
|
||||
protocols.push(protocol.toString('ascii'));
|
||||
protocols.push(protocol.toString('ascii'));
|
||||
}
|
||||
|
||||
const selectedProtocol = socket[kALPNCallback]({
|
||||
servername,
|
||||
protocols,
|
||||
});
|
||||
|
||||
// Undefined -> all proposed protocols rejected
|
||||
if (selectedProtocol === undefined) return undefined;
|
||||
|
||||
const protocolIndex = protocols.indexOf(selectedProtocol);
|
||||
if (protocolIndex === -1) {
|
||||
throw new ERR_TLS_ALPN_CALLBACK_INVALID_RESULT(selectedProtocol, protocols);
|
||||
}
|
||||
let protocolOffset = 0;
|
||||
for (let i = 0; i < protocolIndex; i++) {
|
||||
protocolOffset += 1 + protocols[i].length;
|
||||
}
|
||||
|
||||
return protocolOffset;
|
||||
} catch (err) {
|
||||
socket.destroy(err);
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const selectedProtocol = socket[kALPNCallback]({
|
||||
servername,
|
||||
protocols,
|
||||
});
|
||||
|
||||
// Undefined -> all proposed protocols rejected
|
||||
if (selectedProtocol === undefined) return undefined;
|
||||
|
||||
const protocolIndex = protocols.indexOf(selectedProtocol);
|
||||
if (protocolIndex === -1) {
|
||||
throw new ERR_TLS_ALPN_CALLBACK_INVALID_RESULT(selectedProtocol, protocols);
|
||||
}
|
||||
let protocolOffset = 0;
|
||||
for (let i = 0; i < protocolIndex; i++) {
|
||||
protocolOffset += 1 + protocols[i].length;
|
||||
}
|
||||
|
||||
return protocolOffset;
|
||||
}
|
||||
|
||||
function requestOCSP(socket, info) {
|
||||
@@ -374,63 +379,75 @@ function onnewsession(sessionId, session) {
|
||||
|
||||
function onPskServerCallback(identity, maxPskLen) {
|
||||
const owner = this[owner_symbol];
|
||||
const ret = owner[kPskCallback](owner, identity);
|
||||
if (ret == null)
|
||||
return undefined;
|
||||
|
||||
let psk;
|
||||
if (isArrayBufferView(ret)) {
|
||||
psk = ret;
|
||||
} else {
|
||||
if (typeof ret !== 'object') {
|
||||
throw new ERR_INVALID_ARG_TYPE(
|
||||
'ret',
|
||||
['Object', 'Buffer', 'TypedArray', 'DataView'],
|
||||
ret,
|
||||
try {
|
||||
const ret = owner[kPskCallback](owner, identity);
|
||||
if (ret == null)
|
||||
return undefined;
|
||||
|
||||
let psk;
|
||||
if (isArrayBufferView(ret)) {
|
||||
psk = ret;
|
||||
} else {
|
||||
if (typeof ret !== 'object') {
|
||||
throw new ERR_INVALID_ARG_TYPE(
|
||||
'ret',
|
||||
['Object', 'Buffer', 'TypedArray', 'DataView'],
|
||||
ret,
|
||||
);
|
||||
}
|
||||
psk = ret.psk;
|
||||
validateBuffer(psk, 'psk');
|
||||
}
|
||||
|
||||
if (psk.length > maxPskLen) {
|
||||
throw new ERR_INVALID_ARG_VALUE(
|
||||
'psk',
|
||||
psk,
|
||||
`Pre-shared key exceeds ${maxPskLen} bytes`,
|
||||
);
|
||||
}
|
||||
psk = ret.psk;
|
||||
validateBuffer(psk, 'psk');
|
||||
}
|
||||
|
||||
if (psk.length > maxPskLen) {
|
||||
throw new ERR_INVALID_ARG_VALUE(
|
||||
'psk',
|
||||
psk,
|
||||
`Pre-shared key exceeds ${maxPskLen} bytes`,
|
||||
);
|
||||
return psk;
|
||||
} catch (err) {
|
||||
owner.destroy(err);
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return psk;
|
||||
}
|
||||
|
||||
function onPskClientCallback(hint, maxPskLen, maxIdentityLen) {
|
||||
const owner = this[owner_symbol];
|
||||
const ret = owner[kPskCallback](hint);
|
||||
if (ret == null)
|
||||
|
||||
try {
|
||||
const ret = owner[kPskCallback](hint);
|
||||
if (ret == null)
|
||||
return undefined;
|
||||
|
||||
validateObject(ret, 'ret');
|
||||
|
||||
validateBuffer(ret.psk, 'psk');
|
||||
if (ret.psk.length > maxPskLen) {
|
||||
throw new ERR_INVALID_ARG_VALUE(
|
||||
'psk',
|
||||
ret.psk,
|
||||
`Pre-shared key exceeds ${maxPskLen} bytes`,
|
||||
);
|
||||
}
|
||||
|
||||
validateString(ret.identity, 'identity');
|
||||
if (Buffer.byteLength(ret.identity) > maxIdentityLen) {
|
||||
throw new ERR_INVALID_ARG_VALUE(
|
||||
'identity',
|
||||
ret.identity,
|
||||
`PSK identity exceeds ${maxIdentityLen} bytes`,
|
||||
);
|
||||
}
|
||||
|
||||
return { psk: ret.psk, identity: ret.identity };
|
||||
} catch (err) {
|
||||
owner.destroy(err);
|
||||
return undefined;
|
||||
|
||||
validateObject(ret, 'ret');
|
||||
|
||||
validateBuffer(ret.psk, 'psk');
|
||||
if (ret.psk.length > maxPskLen) {
|
||||
throw new ERR_INVALID_ARG_VALUE(
|
||||
'psk',
|
||||
ret.psk,
|
||||
`Pre-shared key exceeds ${maxPskLen} bytes`,
|
||||
);
|
||||
}
|
||||
|
||||
validateString(ret.identity, 'identity');
|
||||
if (Buffer.byteLength(ret.identity) > maxIdentityLen) {
|
||||
throw new ERR_INVALID_ARG_VALUE(
|
||||
'identity',
|
||||
ret.identity,
|
||||
`PSK identity exceeds ${maxIdentityLen} bytes`,
|
||||
);
|
||||
}
|
||||
|
||||
return { psk: ret.psk, identity: ret.identity };
|
||||
}
|
||||
|
||||
function onkeylog(line) {
|
||||
|
||||
@@ -253,25 +253,35 @@ function TestALPNCallback() {
|
||||
function TestBadALPNCallback() {
|
||||
// Server always returns a fixed invalid value:
|
||||
const serverOptions = {
|
||||
key: loadPEM('agent2-key'),
|
||||
cert: loadPEM('agent2-cert'),
|
||||
ALPNCallback: common.mustCall(() => 'http/5')
|
||||
};
|
||||
|
||||
const clientsOptions = [{
|
||||
ALPNProtocols: ['http/1', 'h2'],
|
||||
}];
|
||||
const server = tls.createServer(serverOptions);
|
||||
|
||||
process.once('uncaughtException', common.mustCall((error) => {
|
||||
// Error should be emitted via tlsClientError, not as uncaughtException
|
||||
server.on('tlsClientError', common.mustCall((error, socket) => {
|
||||
assert.strictEqual(error.code, 'ERR_TLS_ALPN_CALLBACK_INVALID_RESULT');
|
||||
socket.destroy();
|
||||
}));
|
||||
|
||||
runTest(clientsOptions, serverOptions, function(results) {
|
||||
// Callback returns 'http/5' => doesn't match client ALPN => error & reset
|
||||
assert.strictEqual(results[0].server, undefined);
|
||||
const allowedErrors = ['ECONNRESET', 'ERR_SSL_TLSV1_ALERT_NO_APPLICATION_PROTOCOL'];
|
||||
assert.ok(allowedErrors.includes(results[0].client.error.code), `'${results[0].client.error.code}' was not one of ${allowedErrors}.`);
|
||||
server.listen(0, serverIP, common.mustCall(() => {
|
||||
const client = tls.connect({
|
||||
port: server.address().port,
|
||||
host: serverIP,
|
||||
rejectUnauthorized: false,
|
||||
ALPNProtocols: ['http/1', 'h2'],
|
||||
}, common.mustNotCall());
|
||||
|
||||
TestALPNOptionsCallback();
|
||||
});
|
||||
client.on('error', common.mustCall((err) => {
|
||||
// Client gets reset when server handles error via tlsClientError
|
||||
const allowedErrors = ['ECONNRESET', 'ERR_SSL_TLSV1_ALERT_NO_APPLICATION_PROTOCOL'];
|
||||
assert.ok(allowedErrors.includes(err.code), `'${err.code}' was not one of ${allowedErrors}.`);
|
||||
server.close();
|
||||
TestALPNOptionsCallback();
|
||||
}));
|
||||
}));
|
||||
}
|
||||
|
||||
function TestALPNOptionsCallback() {
|
||||
|
||||
@@ -0,0 +1,334 @@
|
||||
'use strict';
|
||||
|
||||
// This test verifies that exceptions in pskCallback and ALPNCallback are
|
||||
// properly routed through tlsClientError instead of becoming uncaught
|
||||
// exceptions. This is a regression test for a vulnerability where callback
|
||||
// validation errors would bypass all standard TLS error handlers.
|
||||
//
|
||||
// The vulnerability allows remote attackers to crash TLS servers or cause
|
||||
// resource exhaustion (file descriptor leaks) when pskCallback or ALPNCallback
|
||||
// throw exceptions during validation.
|
||||
|
||||
const common = require('../common');
|
||||
|
||||
if (!common.hasCrypto)
|
||||
common.skip('missing crypto');
|
||||
|
||||
const assert = require('assert');
|
||||
const { describe, it } = require('node:test');
|
||||
const tls = require('tls');
|
||||
const fixtures = require('../common/fixtures');
|
||||
|
||||
const CIPHERS = 'PSK+HIGH';
|
||||
const TEST_TIMEOUT = 5000;
|
||||
|
||||
// Helper to create a promise that rejects on uncaughtException or timeout
|
||||
function createTestPromise() {
|
||||
const { promise, resolve, reject } = Promise.withResolvers();
|
||||
let settled = false;
|
||||
|
||||
const cleanup = () => {
|
||||
if (!settled) {
|
||||
settled = true;
|
||||
process.removeListener('uncaughtException', onUncaught);
|
||||
clearTimeout(timeout);
|
||||
}
|
||||
};
|
||||
|
||||
const onUncaught = (err) => {
|
||||
cleanup();
|
||||
reject(new Error(
|
||||
`Uncaught exception instead of tlsClientError: ${err.code || err.message}`
|
||||
));
|
||||
};
|
||||
|
||||
const timeout = setTimeout(() => {
|
||||
cleanup();
|
||||
reject(new Error('Test timed out - tlsClientError was not emitted'));
|
||||
}, TEST_TIMEOUT);
|
||||
|
||||
process.on('uncaughtException', onUncaught);
|
||||
|
||||
return {
|
||||
resolve: (value) => {
|
||||
cleanup();
|
||||
resolve(value);
|
||||
},
|
||||
reject: (err) => {
|
||||
cleanup();
|
||||
reject(err);
|
||||
},
|
||||
promise,
|
||||
};
|
||||
}
|
||||
|
||||
describe('TLS callback exception handling', () => {
|
||||
|
||||
// Test 1: PSK server callback returning invalid type should emit tlsClientError
|
||||
it('pskCallback returning invalid type emits tlsClientError', async (t) => {
|
||||
const server = tls.createServer({
|
||||
ciphers: CIPHERS,
|
||||
pskCallback: () => {
|
||||
// Return invalid type (string instead of object/Buffer)
|
||||
return 'invalid-should-be-object-or-buffer';
|
||||
},
|
||||
pskIdentityHint: 'test-hint',
|
||||
});
|
||||
|
||||
t.after(() => server.close());
|
||||
|
||||
const { promise, resolve, reject } = createTestPromise();
|
||||
|
||||
server.on('tlsClientError', common.mustCall((err, socket) => {
|
||||
try {
|
||||
assert.ok(err instanceof Error);
|
||||
assert.strictEqual(err.code, 'ERR_INVALID_ARG_TYPE');
|
||||
socket.destroy();
|
||||
resolve();
|
||||
} catch (e) {
|
||||
reject(e);
|
||||
}
|
||||
}));
|
||||
|
||||
server.on('secureConnection', common.mustNotCall(() => {
|
||||
reject(new Error('secureConnection should not fire'));
|
||||
}));
|
||||
|
||||
await new Promise((res) => server.listen(0, res));
|
||||
|
||||
const client = tls.connect({
|
||||
port: server.address().port,
|
||||
host: '127.0.0.1',
|
||||
ciphers: CIPHERS,
|
||||
checkServerIdentity: () => {},
|
||||
pskCallback: () => ({
|
||||
psk: Buffer.alloc(32),
|
||||
identity: 'test-identity',
|
||||
}),
|
||||
});
|
||||
|
||||
client.on('error', () => {});
|
||||
|
||||
await promise;
|
||||
});
|
||||
|
||||
// Test 2: PSK server callback throwing should emit tlsClientError
|
||||
it('pskCallback throwing emits tlsClientError', async (t) => {
|
||||
const server = tls.createServer({
|
||||
ciphers: CIPHERS,
|
||||
pskCallback: () => {
|
||||
throw new Error('Intentional callback error');
|
||||
},
|
||||
pskIdentityHint: 'test-hint',
|
||||
});
|
||||
|
||||
t.after(() => server.close());
|
||||
|
||||
const { promise, resolve, reject } = createTestPromise();
|
||||
|
||||
server.on('tlsClientError', common.mustCall((err, socket) => {
|
||||
try {
|
||||
assert.ok(err instanceof Error);
|
||||
assert.strictEqual(err.message, 'Intentional callback error');
|
||||
socket.destroy();
|
||||
resolve();
|
||||
} catch (e) {
|
||||
reject(e);
|
||||
}
|
||||
}));
|
||||
|
||||
server.on('secureConnection', common.mustNotCall(() => {
|
||||
reject(new Error('secureConnection should not fire'));
|
||||
}));
|
||||
|
||||
await new Promise((res) => server.listen(0, res));
|
||||
|
||||
const client = tls.connect({
|
||||
port: server.address().port,
|
||||
host: '127.0.0.1',
|
||||
ciphers: CIPHERS,
|
||||
checkServerIdentity: () => {},
|
||||
pskCallback: () => ({
|
||||
psk: Buffer.alloc(32),
|
||||
identity: 'test-identity',
|
||||
}),
|
||||
});
|
||||
|
||||
client.on('error', () => {});
|
||||
|
||||
await promise;
|
||||
});
|
||||
|
||||
// Test 3: ALPN callback returning non-matching protocol should emit tlsClientError
|
||||
it('ALPNCallback returning invalid result emits tlsClientError', async (t) => {
|
||||
const server = tls.createServer({
|
||||
key: fixtures.readKey('agent2-key.pem'),
|
||||
cert: fixtures.readKey('agent2-cert.pem'),
|
||||
ALPNCallback: () => {
|
||||
// Return a protocol not in the client's list
|
||||
return 'invalid-protocol-not-in-list';
|
||||
},
|
||||
});
|
||||
|
||||
t.after(() => server.close());
|
||||
|
||||
const { promise, resolve, reject } = createTestPromise();
|
||||
|
||||
server.on('tlsClientError', common.mustCall((err, socket) => {
|
||||
try {
|
||||
assert.ok(err instanceof Error);
|
||||
assert.strictEqual(err.code, 'ERR_TLS_ALPN_CALLBACK_INVALID_RESULT');
|
||||
socket.destroy();
|
||||
resolve();
|
||||
} catch (e) {
|
||||
reject(e);
|
||||
}
|
||||
}));
|
||||
|
||||
server.on('secureConnection', common.mustNotCall(() => {
|
||||
reject(new Error('secureConnection should not fire'));
|
||||
}));
|
||||
|
||||
await new Promise((res) => server.listen(0, res));
|
||||
|
||||
const client = tls.connect({
|
||||
port: server.address().port,
|
||||
host: '127.0.0.1',
|
||||
rejectUnauthorized: false,
|
||||
ALPNProtocols: ['http/1.1', 'h2'],
|
||||
});
|
||||
|
||||
client.on('error', () => {});
|
||||
|
||||
await promise;
|
||||
});
|
||||
|
||||
// Test 4: ALPN callback throwing should emit tlsClientError
|
||||
it('ALPNCallback throwing emits tlsClientError', async (t) => {
|
||||
const server = tls.createServer({
|
||||
key: fixtures.readKey('agent2-key.pem'),
|
||||
cert: fixtures.readKey('agent2-cert.pem'),
|
||||
ALPNCallback: () => {
|
||||
throw new Error('Intentional ALPN callback error');
|
||||
},
|
||||
});
|
||||
|
||||
t.after(() => server.close());
|
||||
|
||||
const { promise, resolve, reject } = createTestPromise();
|
||||
|
||||
server.on('tlsClientError', common.mustCall((err, socket) => {
|
||||
try {
|
||||
assert.ok(err instanceof Error);
|
||||
assert.strictEqual(err.message, 'Intentional ALPN callback error');
|
||||
socket.destroy();
|
||||
resolve();
|
||||
} catch (e) {
|
||||
reject(e);
|
||||
}
|
||||
}));
|
||||
|
||||
server.on('secureConnection', common.mustNotCall(() => {
|
||||
reject(new Error('secureConnection should not fire'));
|
||||
}));
|
||||
await new Promise((res) => server.listen(0, res));
|
||||
|
||||
const client = tls.connect({
|
||||
port: server.address().port,
|
||||
host: '127.0.0.1',
|
||||
rejectUnauthorized: false,
|
||||
ALPNProtocols: ['http/1.1'],
|
||||
});
|
||||
|
||||
client.on('error', () => {});
|
||||
|
||||
await promise;
|
||||
});
|
||||
|
||||
// Test 5: PSK client callback returning invalid type should emit error event
|
||||
it('client pskCallback returning invalid type emits error', async (t) => {
|
||||
const PSK = Buffer.alloc(32);
|
||||
|
||||
const server = tls.createServer({
|
||||
ciphers: CIPHERS,
|
||||
pskCallback: () => PSK,
|
||||
pskIdentityHint: 'test-hint',
|
||||
});
|
||||
|
||||
t.after(() => server.close());
|
||||
|
||||
const { promise, resolve, reject } = createTestPromise();
|
||||
|
||||
server.on('secureConnection', common.mustNotCall(() => {
|
||||
reject(new Error('secureConnection should not fire'));
|
||||
}));
|
||||
|
||||
await new Promise((res) => server.listen(0, res));
|
||||
|
||||
const client = tls.connect({
|
||||
port: server.address().port,
|
||||
host: '127.0.0.1',
|
||||
ciphers: CIPHERS,
|
||||
checkServerIdentity: () => {},
|
||||
pskCallback: () => {
|
||||
// Return invalid type - should cause validation error
|
||||
return 'invalid-should-be-object';
|
||||
},
|
||||
});
|
||||
|
||||
client.on('error', common.mustCall((err) => {
|
||||
try {
|
||||
assert.ok(err instanceof Error);
|
||||
assert.strictEqual(err.code, 'ERR_INVALID_ARG_TYPE');
|
||||
resolve();
|
||||
} catch (e) {
|
||||
reject(e);
|
||||
}
|
||||
}));
|
||||
|
||||
await promise;
|
||||
});
|
||||
|
||||
// Test 6: PSK client callback throwing should emit error event
|
||||
it('client pskCallback throwing emits error', async (t) => {
|
||||
const PSK = Buffer.alloc(32);
|
||||
|
||||
const server = tls.createServer({
|
||||
ciphers: CIPHERS,
|
||||
pskCallback: () => PSK,
|
||||
pskIdentityHint: 'test-hint',
|
||||
});
|
||||
|
||||
t.after(() => server.close());
|
||||
|
||||
const { promise, resolve, reject } = createTestPromise();
|
||||
|
||||
server.on('secureConnection', common.mustNotCall(() => {
|
||||
reject(new Error('secureConnection should not fire'));
|
||||
}));
|
||||
|
||||
await new Promise((res) => server.listen(0, res));
|
||||
|
||||
const client = tls.connect({
|
||||
port: server.address().port,
|
||||
host: '127.0.0.1',
|
||||
ciphers: CIPHERS,
|
||||
checkServerIdentity: () => {},
|
||||
pskCallback: () => {
|
||||
throw new Error('Intentional client PSK callback error');
|
||||
},
|
||||
});
|
||||
|
||||
client.on('error', common.mustCall((err) => {
|
||||
try {
|
||||
assert.ok(err instanceof Error);
|
||||
assert.strictEqual(err.message, 'Intentional client PSK callback error');
|
||||
resolve();
|
||||
} catch (e) {
|
||||
reject(e);
|
||||
}
|
||||
}));
|
||||
|
||||
await promise;
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user