tls: make deprecated tls.createSecurePair() use public API

Make the deprecated `tls.createSecurePair()` method use other public
APIs only (`TLSSocket` in particular).

Since `tls.createSecurePair()` has been runtime-deprecated only
since Node 8, it probably isn’t quite time to remove it yet,
but this patch removes almost all of the code complexity that
is retained by it.

The API, as it is documented, is retained. However, it is very likely
that some users have come to rely on parts of undocumented API
of the `SecurePair` class, especially since some of the existing
tests checked for those. Therefore, this should definitely be
considered a breaking change.

PR-URL: https://github.com/nodejs/node/pull/17882
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

7 files changed, 11 insertions, 194 deletions

diff --git a/test/async-hooks/test-connection.ssl.js b/test/async-hooks/test-connection.ssl.js
deleted file mode 100644
index faee0fdf08..0000000000
--- a/test/async-hooks/test-connection.ssl.js
+++ /dev/null
@@ -1,86 +0,0 @@
-'use strict';
-
-const common = require('../common');
-if (!common.hasCrypto)
-  common.skip('missing crypto');
-
-const initHooks = require('./init-hooks');
-const tick = require('./tick');
-const assert = require('assert');
-const { checkInvocations } = require('./hook-checks');
-
-const tls = require('tls');
-const Connection = process.binding('crypto').Connection;
-const hooks = initHooks();
-hooks.enable();
-
-function createServerConnection(
-  onhandshakestart,
-  certificate = null,
-  isServer = true,
-  servername = 'some server',
-  rejectUnauthorized
-) {
-  if (certificate == null) certificate = tls.createSecureContext();
-  const ssl = new Connection(
-    certificate.context, isServer, servername, rejectUnauthorized
-  );
-  if (isServer) {
-    ssl.onhandshakestart = onhandshakestart;
-    ssl.lastHandshakeTime = 0;
-  }
-  return ssl;
-}
-
-// creating first server connection
-const sc1 = createServerConnection(common.mustCall(onfirstHandShake));
-
-let as = hooks.activitiesOfTypes('SSLCONNECTION');
-assert.strictEqual(as.length, 1);
-const f1 = as[0];
-assert.strictEqual(f1.type, 'SSLCONNECTION');
-assert.strictEqual(typeof f1.uid, 'number');
-assert.strictEqual(typeof f1.triggerAsyncId, 'number');
-checkInvocations(f1, { init: 1 }, 'first connection, when first created');
-
-// creating second server connection
-const sc2 = createServerConnection(common.mustCall(onsecondHandShake));
-
-as = hooks.activitiesOfTypes('SSLCONNECTION');
-assert.strictEqual(as.length, 2);
-const f2 = as[1];
-assert.strictEqual(f2.type, 'SSLCONNECTION');
-assert.strictEqual(typeof f2.uid, 'number');
-assert.strictEqual(typeof f2.triggerAsyncId, 'number');
-checkInvocations(f1, { init: 1 }, 'first connection, when second created');
-checkInvocations(f2, { init: 1 }, 'second connection, when second created');
-
-// starting the connections which results in handshake starts
-sc1.start();
-sc2.start();
-
-function onfirstHandShake() {
-  checkInvocations(f1, { init: 1, before: 1 },
-                   'first connection, when first handshake');
-  checkInvocations(f2, { init: 1 }, 'second connection, when first handshake');
-}
-
-function onsecondHandShake() {
-  checkInvocations(f1, { init: 1, before: 1, after: 1 },
-                   'first connection, when second handshake');
-  checkInvocations(f2, { init: 1, before: 1 },
-                   'second connection, when second handshake');
-  tick(1E4);
-}
-
-process.on('exit', onexit);
-
-function onexit() {
-  hooks.disable();
-  hooks.sanityCheck('SSLCONNECTION');
-
-  checkInvocations(f1, { init: 1, before: 1, after: 1 },
-                   'first connection, when process exits');
-  checkInvocations(f2, { init: 1, before: 1, after: 1 },
-                   'second connection, when process exits');
-}
diff --git a/test/async-hooks/test-graph.connection.js b/test/async-hooks/test-graph.connection.js
deleted file mode 100644
index fcc764b5cc..0000000000
--- a/test/async-hooks/test-graph.connection.js
+++ /dev/null
@@ -1,55 +0,0 @@
-'use strict';
-
-const common = require('../common');
-if (!common.hasCrypto)
-  common.skip('missing crypto');
-
-const initHooks = require('./init-hooks');
-const verifyGraph = require('./verify-graph');
-
-const tls = require('tls');
-const Connection = process.binding('crypto').Connection;
-const hooks = initHooks();
-hooks.enable();
-
-function createServerConnection(
-  onhandshakestart,
-  certificate = null,
-  isServer = true,
-  servername = 'some server',
-  rejectUnauthorized
-) {
-  if (certificate == null) certificate = tls.createSecureContext();
-  const ssl = new Connection(
-    certificate.context, isServer, servername, rejectUnauthorized
-  );
-  if (isServer) {
-    ssl.onhandshakestart = onhandshakestart;
-    ssl.lastHandshakeTime = 0;
-  }
-  return ssl;
-}
-
-// creating first server connection and start it
-const sc1 = createServerConnection(common.mustCall(onfirstHandShake));
-sc1.start();
-
-function onfirstHandShake() {
-  // Create second connection inside handshake of first to show
-  // that the triggerAsyncId of the second will be set to id of the first
-  const sc2 = createServerConnection(common.mustCall(onsecondHandShake));
-  sc2.start();
-}
-function onsecondHandShake() { }
-
-process.on('exit', onexit);
-
-function onexit() {
-  hooks.disable();
-  verifyGraph(
-    hooks,
-    [ { type: 'CONNECTION', id: 'connection:1', triggerAsyncId: null },
-      { type: 'CONNECTION', id: 'connection:2',
-        triggerAsyncId: 'connection:1' } ]
-  );
-}
diff --git a/test/parallel/test-accessor-properties.js b/test/parallel/test-accessor-properties.js
index 13535ceda9..b4ebf30f9b 100644
--- a/test/parallel/test-accessor-properties.js
+++ b/test/parallel/test-accessor-properties.js
@@ -59,20 +59,10 @@ const UDP = process.binding('udp_wrap').UDP;
       crypto.SecureContext.prototype._external;
     }, TypeError);
 
-    assert.throws(() => {
-      crypto.Connection.prototype._external;
-    }, TypeError);
-
     assert.strictEqual(
       typeof Object.getOwnPropertyDescriptor(
         crypto.SecureContext.prototype, '_external'),
       'object'
     );
-
-    assert.strictEqual(
-      typeof Object.getOwnPropertyDescriptor(
-        crypto.Connection.prototype, '_external'),
-      'object'
-    );
   }
 }
diff --git a/test/parallel/test-tls-basic-validations.js b/test/parallel/test-tls-basic-validations.js
index e747f5a516..fc2743ce04 100644
--- a/test/parallel/test-tls-basic-validations.js
+++ b/test/parallel/test-tls-basic-validations.js
@@ -40,7 +40,7 @@ assert.throws(() => tls.createServer({ ticketKeys: Buffer.alloc(0) }),
               /TypeError: Ticket keys length must be 48 bytes/);
 
 assert.throws(() => tls.createSecurePair({}),
-              /Error: First argument must be a tls module SecureContext/);
+              /TypeError: Second argument should be a SecureContext instance/);
 
 {
   const buffer = Buffer.from('abcd');
diff --git a/test/parallel/test-tls-legacy-onselect.js b/test/parallel/test-tls-legacy-onselect.js
deleted file mode 100644
index efcc5c2c92..0000000000
--- a/test/parallel/test-tls-legacy-onselect.js
+++ /dev/null
@@ -1,27 +0,0 @@
-'use strict';
-const common = require('../common');
-
-if (!common.hasCrypto)
-  common.skip('missing crypto');
-
-const tls = require('tls');
-const net = require('net');
-
-const server = net.Server(common.mustCall(function(raw) {
-  const pair = tls.createSecurePair(null, true, false, false);
-  pair.on('error', function() {});
-  pair.ssl.setSNICallback(common.mustCall(function() {
-    raw.destroy();
-    server.close();
-  }));
-  require('_tls_legacy').pipe(pair, raw);
-})).listen(0, function() {
-  tls.connect({
-    port: this.address().port,
-    rejectUnauthorized: false,
-    servername: 'server'
-  }, function() {
-  }).on('error', function() {
-    // Just ignore
-  });
-});
diff --git a/test/parallel/test-tls-securepair-leak.js b/test/parallel/test-tls-securepair-leak.js
index cbc7c7dadd..4cd927d64a 100644
--- a/test/parallel/test-tls-securepair-leak.js
+++ b/test/parallel/test-tls-securepair-leak.js
@@ -7,7 +7,7 @@ if (!common.hasCrypto)
 
 const assert = require('assert');
 const { createSecureContext } = require('tls');
-const { createSecurePair } = require('_tls_legacy');
+const { createSecurePair } = require('tls');
 
 const before = process.memoryUsage().external;
 {
@@ -16,11 +16,13 @@ const before = process.memoryUsage().external;
   for (let i = 0; i < 1e4; i += 1)
     createSecurePair(context, false, false, false, options).destroy();
 }
-global.gc();
-const after = process.memoryUsage().external;
+setImmediate(() => {
+  global.gc();
+  const after = process.memoryUsage().external;
 
-// It's not an exact science but a SecurePair grows .external by about 45 kB.
-// Unless AdjustAmountOfExternalAllocatedMemory() is called on destruction,
-// 10,000 instances make it grow by well over 400 MB.  Allow for some slop
-// because objects like buffers also affect the external limit.
-assert(after - before < 25 << 20);
+  // It's not an exact science but a SecurePair grows .external by about 45 kB.
+  // Unless AdjustAmountOfExternalAllocatedMemory() is called on destruction,
+  // 10,000 instances make it grow by well over 400 MB.  Allow for some slop
+  // because objects like buffers also affect the external limit.
+  assert(after - before < 25 << 20);
+});
diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js
index a96c5032ad..58d6b77469 100644
--- a/test/sequential/test-async-wrap-getasyncid.js
+++ b/test/sequential/test-async-wrap-getasyncid.js
@@ -88,13 +88,6 @@ function testInitialized(req, ctor_name) {
 
 
 if (common.hasCrypto) { // eslint-disable-line crypto-check
-  const tls = require('tls');
-  // SecurePair
-  testInitialized(tls.createSecurePair().ssl, 'Connection');
-}
-
-
-if (common.hasCrypto) { // eslint-disable-line crypto-check
   const crypto = require('crypto');
 
   // The handle for PBKDF2 and RandomBytes isn't returned by the function call,