Skip to content

Commit 5110b19

Browse files
tniessentargos
authored andcommitted
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: openssl/openssl#19082 PR-URL: #53002 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Tim Perry <[email protected]>
1 parent 0f0bc98 commit 5110b19

File tree

3 files changed

+33
-9
lines changed

3 files changed

+33
-9
lines changed

lib/internal/tls/secure-context.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ function configSecureContext(context, options = kEmptyObject, name = 'options')
311311
}
312312

313313
if (sessionTimeout !== undefined && sessionTimeout !== null) {
314-
validateInt32(sessionTimeout, `${name}.sessionTimeout`);
314+
validateInt32(sessionTimeout, `${name}.sessionTimeout`, 0);
315315
context.setSessionTimeout(sessionTimeout);
316316
}
317317
}

src/crypto/crypto_context.cc

+1
Original file line numberDiff line numberDiff line change
@@ -998,6 +998,7 @@ void SecureContext::SetSessionTimeout(const FunctionCallbackInfo<Value>& args) {
998998
CHECK(args[0]->IsInt32());
999999

10001000
int32_t sessionTimeout = args[0].As<Int32>()->Value();
1001+
CHECK_GE(sessionTimeout, 0);
10011002
SSL_CTX_set_timeout(sc->ctx_.get(), sessionTimeout);
10021003
}
10031004

test/sequential/test-tls-session-timeout.js

+31-8
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,43 @@
2222
'use strict';
2323
const common = require('../common');
2424

25-
if (!common.opensslCli)
26-
common.skip('node compiled without OpenSSL CLI.');
27-
2825
if (!common.hasCrypto)
2926
common.skip('missing crypto');
3027

3128
const tmpdir = require('../common/tmpdir');
3229
tmpdir.refresh();
3330

31+
const assert = require('assert');
32+
const tls = require('tls');
33+
const fixtures = require('../common/fixtures');
34+
35+
const key = fixtures.readKey('rsa_private.pem');
36+
const cert = fixtures.readKey('rsa_cert.crt');
37+
38+
{
39+
// Node.js should not allow setting negative timeouts since new versions of
40+
// OpenSSL do not handle those as users might expect
41+
42+
for (const sessionTimeout of [-1, -100, -(2 ** 31)]) {
43+
assert.throws(() => {
44+
tls.createServer({
45+
key: key,
46+
cert: cert,
47+
ca: [cert],
48+
sessionTimeout,
49+
maxVersion: 'TLSv1.2',
50+
});
51+
}, {
52+
code: 'ERR_OUT_OF_RANGE',
53+
message: 'The value of "options.sessionTimeout" is out of range. It ' +
54+
`must be >= 0 && <= ${2 ** 31 - 1}. Received ${sessionTimeout}`,
55+
});
56+
}
57+
}
58+
59+
if (!common.opensslCli)
60+
common.skip('node compiled without OpenSSL CLI.');
61+
3462
doTest();
3563

3664
// This test consists of three TLS requests --
@@ -42,16 +70,11 @@ doTest();
4270
// that we used has expired by now.
4371

4472
function doTest() {
45-
const assert = require('assert');
46-
const tls = require('tls');
4773
const fs = require('fs');
48-
const fixtures = require('../common/fixtures');
4974
const spawn = require('child_process').spawn;
5075

5176
const SESSION_TIMEOUT = 1;
5277

53-
const key = fixtures.readKey('rsa_private.pem');
54-
const cert = fixtures.readKey('rsa_cert.crt');
5578
const options = {
5679
key: key,
5780
cert: cert,

0 commit comments

Comments
 (0)