tls: fix negative sessionTimeout handling

For historical reasons, the second argument of SSL_CTX_set_timeout is a
signed integer, and Node.js has so far passed arbitrary (signed) int32_t
values. However, new versions of OpenSSL have changed the handling of
negative values inside SSL_CTX_set_timeout, and we should shield users
of Node.js from both the old and the new behavior. Hence, reject any
negative values by throwing an error from within createSecureContext.

Refs: https://github.com/openssl/openssl/pull/19082
PR-URL: https://github.com/nodejs/node/pull/53002
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
This commit is contained in:
Tobias Nießen 2024-05-18 02:26:11 +02:00 committed by GitHub
parent 00550b043b
commit 559212e64c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 33 additions and 9 deletions

View File

@ -311,7 +311,7 @@ function configSecureContext(context, options = kEmptyObject, name = 'options')
}
if (sessionTimeout !== undefined && sessionTimeout !== null) {
validateInt32(sessionTimeout, `${name}.sessionTimeout`);
validateInt32(sessionTimeout, `${name}.sessionTimeout`, 0);
context.setSessionTimeout(sessionTimeout);
}
}

View File

@ -998,6 +998,7 @@ void SecureContext::SetSessionTimeout(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsInt32());
int32_t sessionTimeout = args[0].As<Int32>()->Value();
CHECK_GE(sessionTimeout, 0);
SSL_CTX_set_timeout(sc->ctx_.get(), sessionTimeout);
}

View File

@ -22,15 +22,43 @@
'use strict';
const common = require('../common');
if (!common.opensslCli)
common.skip('node compiled without OpenSSL CLI.');
if (!common.hasCrypto)
common.skip('missing crypto');
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
const assert = require('assert');
const tls = require('tls');
const fixtures = require('../common/fixtures');
const key = fixtures.readKey('rsa_private.pem');
const cert = fixtures.readKey('rsa_cert.crt');
{
// Node.js should not allow setting negative timeouts since new versions of
// OpenSSL do not handle those as users might expect
for (const sessionTimeout of [-1, -100, -(2 ** 31)]) {
assert.throws(() => {
tls.createServer({
key: key,
cert: cert,
ca: [cert],
sessionTimeout,
maxVersion: 'TLSv1.2',
});
}, {
code: 'ERR_OUT_OF_RANGE',
message: 'The value of "options.sessionTimeout" is out of range. It ' +
`must be >= 0 && <= ${2 ** 31 - 1}. Received ${sessionTimeout}`,
});
}
}
if (!common.opensslCli)
common.skip('node compiled without OpenSSL CLI.');
doTest();
// This test consists of three TLS requests --
@ -42,16 +70,11 @@ doTest();
// that we used has expired by now.
function doTest() {
const assert = require('assert');
const tls = require('tls');
const fs = require('fs');
const fixtures = require('../common/fixtures');
const spawn = require('child_process').spawn;
const SESSION_TIMEOUT = 1;
const key = fixtures.readKey('rsa_private.pem');
const cert = fixtures.readKey('rsa_cert.crt');
const options = {
key: key,
cert: cert,