Commit 5110b19
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 <luigipinca@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>1 parent 0f0bc98 commit 5110b19
3 files changed
Lines changed: 33 additions & 9 deletions
File tree
- lib/internal/tls
- src/crypto
- test/sequential
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -311,7 +311,7 @@ function configSecureContext(context, options = kEmptyObject, name = 'options') | |||
| 311 | 311 | } | |
| 312 | 312 | ||
| 313 | 313 | if (sessionTimeout !== undefined && sessionTimeout !== null) { | |
| 314 | - validateInt32(sessionTimeout, `${name}.sessionTimeout`); | ||
| 314 | + validateInt32(sessionTimeout, `${name}.sessionTimeout`, 0); | ||
| 315 | 315 | context.setSessionTimeout(sessionTimeout); | |
| 316 | 316 | } | |
| 317 | 317 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -998,6 +998,7 @@ void SecureContext::SetSessionTimeout(const FunctionCallbackInfo<Value>& args) { | |||
| 998 | 998 | CHECK(args[0]->IsInt32()); | |
| 999 | 999 | ||
| 1000 | 1000 | int32_t sessionTimeout = args[0].As<Int32>()->Value(); | |
| 1001 | + CHECK_GE(sessionTimeout, 0); | ||
| 1001 | 1002 | SSL_CTX_set_timeout(sc->ctx_.get(), sessionTimeout); | |
| 1002 | 1003 | } | |
| 1003 | 1004 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -22,15 +22,43 @@ | |||
| 22 | 22 | 'use strict'; | |
| 23 | 23 | const common = require('../common'); | |
| 24 | 24 | ||
| 25 | - if (!common.opensslCli) | ||
| 26 | - common.skip('node compiled without OpenSSL CLI.'); | ||
| 27 | - | ||
| 28 | 25 | if (!common.hasCrypto) | |
| 29 | 26 | common.skip('missing crypto'); | |
| 30 | 27 | ||
| 31 | 28 | const tmpdir = require('../common/tmpdir'); | |
| 32 | 29 | tmpdir.refresh(); | |
| 33 | 30 | ||
| 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 | + | ||
| 34 | 62 | doTest(); | |
| 35 | 63 | ||
| 36 | 64 | // This test consists of three TLS requests -- | |
@@ -42,16 +70,11 @@ doTest(); | |||
| 42 | 70 | // that we used has expired by now. | |
| 43 | 71 | ||
| 44 | 72 | function doTest() { | |
| 45 | - const assert = require('assert'); | ||
| 46 | - const tls = require('tls'); | ||
| 47 | 73 | const fs = require('fs'); | |
| 48 | - const fixtures = require('../common/fixtures'); | ||
| 49 | 74 | const spawn = require('child_process').spawn; | |
| 50 | 75 | ||
| 51 | 76 | const SESSION_TIMEOUT = 1; | |
| 52 | 77 | ||
| 53 | - const key = fixtures.readKey('rsa_private.pem'); | ||
| 54 | - const cert = fixtures.readKey('rsa_cert.crt'); | ||
| 55 | 78 | const options = { | |
| 56 | 79 | key: key, | |
| 57 | 80 | cert: cert, | |
0 commit comments