test: do not assume tls handshake order

test/parallel/test-tls-alert-handling.js

@@ -48,6 +48,9 @@ server.listen(0, common.mustCall(function() {
   sendClient();
 }));
 
+server.on('tlsClientError', common.mustNotCall());
+
+server.on('error', common.mustNotCall());
 
 function sendClient() {
   const client = tls.connect(server.address().port, {
@@ -78,8 +81,10 @@ function sendBADTLSRecord() {
     socket: socket,
     rejectUnauthorized: false
   }, common.mustCall(function() {
-    socket.write(BAD_RECORD);
-    socket.end();
+    client.write('x');
+    client.on('data', (data) => {
+      socket.end(BAD_RECORD);
+    });
   }));
   client.on('error', common.mustCall((err) => {
     assert.strictEqual(err.code, 'ERR_SSL_TLSV1_ALERT_PROTOCOL_VERSION');

test/parallel/test-tls-alpn-server-client.js

@@ -23,9 +23,10 @@ function runTest(clientsOptions, serverOptions, cb) {
   serverOptions.key = loadPEM('agent2-key');
   serverOptions.cert = loadPEM('agent2-cert');
   const results = [];
-  let index = 0;
+  let clientIndex = 0;
+  let serverIndex = 0;
   const server = tls.createServer(serverOptions, function(c) {
-    results[index].server = { ALPN: c.alpnProtocol };
+    results[serverIndex++].server = { ALPN: c.alpnProtocol };
   });
 
   server.listen(0, serverIP, function() {
@@ -38,16 +39,18 @@ function runTest(clientsOptions, serverOptions, cb) {
     opt.host = serverIP;
     opt.rejectUnauthorized = false;
 
-    results[index] = {};
+    results[clientIndex] = {};
     const client = tls.connect(opt, function() {
-      results[index].client = { ALPN: client.alpnProtocol };
-      client.destroy();
+      results[clientIndex].client = { ALPN: client.alpnProtocol };
+      client.end();
       if (options.length) {
-        index++;
+        clientIndex++;
         connectClient(options);
       } else {
         server.close();
-        cb(results);
+        server.on('close', () => {
+          cb(results);
+        });
       }
     });
   }

test/parallel/test-tls-client-getephemeralkeyinfo.js

@@ -40,13 +40,14 @@ function test(size, type, name, cipher) {
     const client = tls.connect({
       port: server.address().port,
       rejectUnauthorized: false
-    }, function() {
+    }, common.mustCall(function() {
       const ekeyinfo = client.getEphemeralKeyInfo();
       assert.strictEqual(ekeyinfo.type, type);
       assert.strictEqual(ekeyinfo.size, size);
       assert.strictEqual(ekeyinfo.name, name);
       server.close();
-    });
+    }));
+    client.on('secureConnect', common.mustCall());
   }));
 }
 

test/parallel/test-tls-client-reject.js

@@ -33,49 +33,57 @@ const options = {
   cert: fixtures.readSync('test_cert.pem')
 };
 
-const server = tls.createServer(options, common.mustCall(function(socket) {
-  socket.on('data', function(data) {
-    console.error(data.toString());
-    assert.strictEqual(data.toString(), 'ok');
-  });
-}, 3)).listen(0, function() {
+const server = tls.createServer(options, function(socket) {
+  socket.pipe(socket);
+  socket.on('end', () => socket.end());
+}).listen(0, common.mustCall(function() {
   unauthorized();
-});
+}));
 
 function unauthorized() {
+  console.log('connect unauthorized');
   const socket = tls.connect({
     port: server.address().port,
     servername: 'localhost',
     rejectUnauthorized: false
   }, common.mustCall(function() {
+    console.log('... unauthorized');
     assert(!socket.authorized);
-    socket.end();
-    rejectUnauthorized();
+    socket.on('data', common.mustCall((data) => {
+      assert.strictEqual(data.toString(), 'ok');
+    }));
+    socket.on('end', () => rejectUnauthorized());
   }));
   socket.on('error', common.mustNotCall());
-  socket.write('ok');
+  socket.end('ok');
 }
 
 function rejectUnauthorized() {
+  console.log('reject unauthorized');
   const socket = tls.connect(server.address().port, {
     servername: 'localhost'
   }, common.mustNotCall());
+  socket.on('data', common.mustNotCall());
   socket.on('error', common.mustCall(function(err) {
-    console.error(err);
+    console.log('... rejected:', err);
     authorized();
   }));
-  socket.write('ng');
+  socket.end('ng');
 }
 
 function authorized() {
+  console.log('connect authorized');
   const socket = tls.connect(server.address().port, {
     ca: [fixtures.readSync('test_cert.pem')],
     servername: 'localhost'
   }, common.mustCall(function() {
+    console.log('... authorized');
     assert(socket.authorized);
-    socket.end();
-    server.close();
+    socket.on('data', common.mustCall((data) => {
+      assert.strictEqual(data.toString(), 'ok');
+    }));
+    socket.on('end', () => server.close());
   }));
   socket.on('error', common.mustNotCall());
-  socket.write('ok');
+  socket.end('ok');
 }

test/parallel/test-tls-connect-address-family.js

@@ -15,7 +15,7 @@ function runTest() {
   tls.createServer({
     cert: fixtures.readKey('agent1-cert.pem'),
     key: fixtures.readKey('agent1-key.pem'),
-  }, common.mustCall(function() {
+  }).on('connection', common.mustCall(function() {
     this.close();
   })).listen(0, '::1', common.mustCall(function() {
     const options = {
@@ -32,7 +32,9 @@ function runTest() {
   }));
 }
 
-dns.lookup('localhost', { family: 6, all: true }, (err, addresses) => {
+dns.lookup('localhost', {
+  family: 6, all: true
+}, common.mustCall((err, addresses) => {
   if (err) {
     if (err.code === 'ENOTFOUND' || err.code === 'EAI_AGAIN')
       common.skip('localhost does not resolve to ::1');
@@ -44,4 +46,4 @@ dns.lookup('localhost', { family: 6, all: true }, (err, addresses) => {
     runTest();
   else
     common.skip('localhost does not resolve to ::1');
-});
+}));

test/parallel/test-tls-friendly-error-message.js

@@ -31,14 +31,15 @@ const tls = require('tls');
 const key = fixtures.readKey('agent1-key.pem');
 const cert = fixtures.readKey('agent1-cert.pem');
 
-tls.createServer({ key, cert }, common.mustCall(function(conn) {
-  conn.end();
+tls.createServer({ key, cert }).on('connection', common.mustCall(function() {
+  // Server only receives one TCP connection, stop listening when that
+  // connection is destroyed by the client, which it should do after the cert is
+  // rejected as unauthorized.
   this.close();
 })).listen(0, common.mustCall(function() {
   const options = { port: this.address().port, rejectUnauthorized: true };
   tls.connect(options).on('error', common.mustCall(function(err) {
     assert.strictEqual(err.code, 'UNABLE_TO_VERIFY_LEAF_SIGNATURE');
     assert.strictEqual(err.message, 'unable to verify the first certificate');
-    this.destroy();
   }));
 }));

test/parallel/test-tls-set-encoding.js

@@ -40,6 +40,7 @@ const messageUtf8 = 'x√ab c';
 const messageAscii = 'xb\b\u001aab c';
 
 const server = tls.Server(options, common.mustCall(function(socket) {
+  console.log('server: on secureConnection', socket.getProtocol());
   socket.end(messageUtf8);
 }));
 
@@ -55,12 +56,18 @@ server.listen(0, function() {
   client.setEncoding('ascii');
 
   client.on('data', function(d) {
+    console.log('client: on data', d);
     assert.ok(typeof d === 'string');
     buffer += d;
   });
 
+  client.on('secureConnect', common.mustCall(() => {
+    console.log('client: on secureConnect');
+  }));
+
+  client.on('close', common.mustCall(function() {
+    console.log('client: on close');
 
-  client.on('close', function() {
     // readyState is deprecated but we want to make
     // sure this isn't triggering an assert in lib/net.js
     // See https://github.com/nodejs/node-v0.x-archive/issues/1069.
@@ -75,5 +82,5 @@ server.listen(0, function() {
     assert.strictEqual(messageAscii, buffer);
 
     server.close();
-  });
+  }));
 });

test/parallel/test-tls-sni-option.js

@@ -115,6 +115,7 @@ let clientError;
 
 const server = tls.createServer(serverOptions, function(c) {
   serverResults.push({ sni: c.servername, authorized: c.authorized });
+  c.end();
 });
 
 server.on('tlsClientError', function(err) {
@@ -135,7 +136,6 @@ function startTest() {
       clientResults.push(
         client.authorizationError &&
          (client.authorizationError === 'ERR_TLS_CERT_ALTNAME_INVALID'));
-      client.destroy();
 
       next();
     });

test/parallel/test-tls-sni-server-client.js

@@ -86,6 +86,7 @@ const clientResults = [];
 
 const server = tls.createServer(serverOptions, function(c) {
   serverResults.push(c.servername);
+  c.end();
 });
 
 server.addContext('a.example.com', SNIContexts['a.example.com']);
@@ -107,7 +108,6 @@ function startTest() {
       clientResults.push(
         client.authorizationError &&
         (client.authorizationError === 'ERR_TLS_CERT_ALTNAME_INVALID'));
-      client.destroy();
 
       // Continue
       start();

test/parallel/test-tls-socket-close.js

@@ -8,6 +8,19 @@ const tls = require('tls');
 const net = require('net');
 const fixtures = require('../common/fixtures');
 
+// Regression test for https://github.com/nodejs/node/issues/8074
+//
+// This test has a dependency on the order in which the TCP connection is made,
+// and TLS server handshake completes. It assumes those server side events occur
+// before the client side write callback, which is not guaranteed by the TLS
+// API. It usally passes with TLS1.3, but TLS1.3 didn't exist at the time the
+// bug existed.
+//
+// Pin the test to TLS1.2, since the test shouldn't be changed in a way that
+// doesn't trigger a segfault in Node.js 7.7.3:
+//   https://github.com/nodejs/node/issues/13184#issuecomment-303700377
+tls.DEFAULT_MAX_VERSION = 'TLSv1.2';
+
 const key = fixtures.readKey('agent2-key.pem');
 const cert = fixtures.readKey('agent2-cert.pem');
 

test/parallel/test-tls-socket-constructor-alpn-options-parsing.js

@@ -20,8 +20,6 @@ const fixtures = require('../common/fixtures');
 const key = fixtures.readKey('agent1-key.pem');
 const cert = fixtures.readKey('agent1-cert.pem');
 
-const protocols = [];
-
 const server = net.createServer(common.mustCall((s) => {
   const tlsSocket = new tls.TLSSocket(s, {
     isServer: true,
@@ -32,10 +30,9 @@ const server = net.createServer(common.mustCall((s) => {
   });
 
   tlsSocket.on('secure', common.mustCall(() => {
-    protocols.push({
-      alpnProtocol: tlsSocket.alpnProtocol,
-    });
+    assert.strictEqual(tlsSocket.alpnProtocol, 'http/1.1');
     tlsSocket.end();
+    server.close();
   }));
 }));
 
@@ -46,13 +43,7 @@ server.listen(0, common.mustCall(() => {
     ALPNProtocols: ['h2', 'http/1.1']
   };
 
-  tls.connect(alpnOpts, function() {
+  tls.connect(alpnOpts, common.mustCall(function() {
     this.end();
-
-    server.close();
-
-    assert.deepStrictEqual(protocols, [
-      { alpnProtocol: 'http/1.1' },
-    ]);
-  });
+  }));
 }));

test/parallel/test-tls-socket-default-options.js

@@ -54,8 +54,9 @@ function test(client, callback) {
       }));
     }));
 
-    // Client doesn't support the 'secureConnect' event, and doesn't error if
-    // authentication failed. Caller must explicitly check for failure.
+    // `new TLSSocket` doesn't support the 'secureConnect' event on client side,
+    // and doesn't error if authentication failed. Caller must explicitly check
+    // for failure.
     (new tls.TLSSocket(null, client)).connect(pair.server.server.address().port)
       .on('connect', common.mustCall(function() {
         this.end('hello');