test: deflake sequential/test-tls-session-timeout

This patch:
- Splits the validation tests into a separate file and keep the
test focus on functional test of the sessionTimeout option.
- Increase the testing timeout to 5 seconds in case it takes too
long for the first connection to complete and the session is
already expired when the second connection is started.
- Use a specific `sessionIdContext` to ensure stable session ID.
- Fix the s_client arguments by specifying CA file and server name.
- Do not use the serialized session ticket for the first connection.
That was genearted years ago and may not work in different OpenSSL
versions. Let the first fresh connection generate the ticket.
- Use random port instead of the common port.
- Add a timeout before the second connection to ensure session ticket
is properly written.
- Log information to faciliate debugging.
PR-URL: #59423Fixes: #26839
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>

Author: joyeecheung

test/parallel/test-tls-session-timeout-errors.js

@@ -0,0 +1,36 @@
+'use strict';
+// This tests validation of sessionTimeout option in TLS server.
+constcommon=require('../common');
+
+if(!common.hasCrypto){
+common.skip('missing crypto');
+}
+
+consttmpdir=require('../common/tmpdir');
+tmpdir.refresh();
+
+constassert=require('assert');
+consttls=require('tls');
+constfixtures=require('../common/fixtures');
+
+constkey=fixtures.readKey('rsa_private.pem');
+constcert=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(constsessionTimeoutof[-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}`,
+});
+}

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

@@ -35,34 +35,13 @@ const assert = require('assert');
 consttls=require('tls');
 constfixtures=require('../common/fixtures');
 
-constkey=fixtures.readKey('rsa_private.pem');
-constcert=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(constsessionTimeoutof[-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(!opensslCli){
 common.skip('node compiled without OpenSSL CLI.');
 }
 
+constkey=fixtures.readKey('rsa_private.pem');
+constcert=fixtures.readKey('rsa_cert.crt');
+
 doTest();
 
 // This test consists of three TLS requests --
@@ -77,40 +56,34 @@ function doTest() {
 constfs=require('fs');
 constspawn=require('child_process').spawn;
 
-constSESSION_TIMEOUT=1;
+constSESSION_TIMEOUT=5;
 
 constoptions={
 key: key,
 cert: cert,
 ca: [cert],
 sessionTimeout: SESSION_TIMEOUT,
 maxVersion: 'TLSv1.2',
+sessionIdContext: 'test-session-timeout',
 };
 
-// We need to store a sample session ticket in the fixtures directory because
-// `s_client` behaves incorrectly if we do not pass in both the `-sess_in`
-// and the `-sess_out` flags, and the `-sess_in` argument must point to a
-// file containing a proper serialization of a session ticket.
-// To avoid a source control diff, we copy the ticket to a temporary file.
-
-constsessionFileName=(function(){
-constticketFileName='tls-session-ticket.txt';
-consttmpPath=tmpdir.resolve(ticketFileName);
-fs.writeFileSync(tmpPath,fixtures.readSync(ticketFileName));
-returntmpPath;
-}());
-
-// Expects a callback -- cb(connectionType : enum ['New'|'Reused'])
-
-functionClient(cb){
+constsessionFileName=tmpdir.resolve('tls-session-ticket.txt');
+// Expects a callback -- cb()
+functionClient(port,sessIn,sessOut,expectedType,cb){
 constflags=[
 's_client',
-'-connect',`localhost:${common.PORT}`,
-'-sess_in',sessionFileName,
-'-sess_out',sessionFileName,
+'-connect',`localhost:${port}`,
+'-CAfile',fixtures.path('keys','rsa_cert.crt'),
+'-servername','localhost',
 ];
+if(sessIn){
+flags.push('-sess_in',sessIn);
+}
+if(sessOut){
+flags.push('-sess_out',sessOut);
+}
 constclient=spawn(opensslCli,flags,{
-stdio: ['ignore','pipe','ignore']
+stdio: ['ignore','pipe','inherit']
 });
 
 letclientOutput='';
@@ -119,6 +92,20 @@ function doTest() {
 });
 client.on('exit',(code)=>{
 letconnectionType;
+// Log the output for debugging purposes. Don't remove them or otherwise
+// the CI output is useless when this test flakes.
+console.log(' ----- [COMMAND] ---');
+console.log(`${opensslCli}, ${flags.join(' ')}`);
+console.log(' ----- [STDOUT] ---');
+console.log(clientOutput);
+console.log(' ----- [SESSION FILE] ---');
+try{
+conststat=fs.statSync(sessionFileName);
+console.log(`Session file size: ${stat.size} bytes`);
+}catch(err){
+console.log('Error reading session file:',err);
+}
+
 constgrepConnectionType=(line)=>{
 constmatches=line.match(/(New|Reused),/);
 if(matches){
@@ -131,6 +118,7 @@ function doTest() {
 thrownewError('unexpected output from openssl client');
 }
 assert.strictEqual(code,0);
+assert.strictEqual(connectionType,expectedType);
 cb(connectionType);
 });
 }
@@ -143,18 +131,18 @@ function doTest() {
 cleartext.end();
 });
 
-server.listen(common.PORT,()=>{
-Client((connectionType)=>{
-assert.strictEqual(connectionType,'New');
-Client((connectionType)=>{
-assert.strictEqual(connectionType,'Reused');
-setTimeout(()=>{
-Client((connectionType)=>{
-assert.strictEqual(connectionType,'New');
-server.close();
-});
-},(SESSION_TIMEOUT+1)*1000);
-});
+server.listen(0,()=>{
+constport=server.address().port;
+Client(port,undefined,sessionFileName,'New',()=>{
+setTimeout(()=>{
+Client(port,sessionFileName,sessionFileName,'Reused',()=>{
+setTimeout(()=>{
+Client(port,sessionFileName,sessionFileName,'New',()=>{
+server.close();
+});
+},(SESSION_TIMEOUT+1)*1000);
+});
+},100);// Wait a bit to ensure the session ticket is saved.
 });
 });
 }