From 5110b19a07b5ecca9daf4b0f85218cdfe9846187 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Sat, 18 May 2024 02:26:11 +0200
Subject: [PATCH] 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>
---
 lib/internal/tls/secure-context.js          |  2 +-
 src/crypto/crypto_context.cc                |  1 +
 test/sequential/test-tls-session-timeout.js | 39 ++++++++++++++++-----
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/lib/internal/tls/secure-context.js b/lib/internal/tls/secure-context.js
index 40d995f9890d53..b0f971e4eef273 100644
--- a/lib/internal/tls/secure-context.js
+++ b/lib/internal/tls/secure-context.js
@@ -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);
   }
 }
diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc
index 17b99177d69767..231208a56f7fa7 100644
--- a/src/crypto/crypto_context.cc
+++ b/src/crypto/crypto_context.cc
@@ -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);
 }
 
diff --git a/test/sequential/test-tls-session-timeout.js b/test/sequential/test-tls-session-timeout.js
index f0ec612b449867..09107011aeda52 100644
--- a/test/sequential/test-tls-session-timeout.js
+++ b/test/sequential/test-tls-session-timeout.js
@@ -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,