tls: migrate argument type-checking errors

* Throw ERR_INVALID_ARG_TYPE from public APIs
* Assert argument types in bindings instead of throwing errors

PR-URL: https://github.com/nodejs/node/pull/18125
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/_tls_wrap.js

@@ -36,6 +36,9 @@ const { Timer } = process.binding('timer_wrap');
 const tls_wrap = process.binding('tls_wrap');
 const { TCP, constants: TCPConstants } = process.binding('tcp_wrap');
 const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
+const {
+  SecureContext: NativeSecureContext
+} = process.binding('crypto');
 const errors = require('internal/errors');
 const kConnectOptions = Symbol('connect-options');
 const kDisableRenegotiation = Symbol('disable-renegotiation');
@@ -407,7 +410,12 @@ TLSSocket.prototype._wrapHandle = function(wrap) {
   const context = options.secureContext ||
                   options.credentials ||
                   tls.createSecureContext(options);
-  const res = tls_wrap.wrap(handle._externalStream,
+  const externalStream = handle._externalStream;
+  assert(typeof externalStream === 'object',
+         'handle must be a LibuvStreamWrap');
+  assert(context.context instanceof NativeSecureContext,
+         'context.context must be a NativeSecureContext');
+  const res = tls_wrap.wrap(externalStream,
                             context.context,
                             !!options.isServer);
   res._parent = handle;
@@ -548,8 +556,8 @@ TLSSocket.prototype.renegotiate = function(options, callback) {
   if (this.destroyed)
     return;
 
-  let requestCert = this._requestCert;
-  let rejectUnauthorized = this._rejectUnauthorized;
+  let requestCert = !!this._requestCert;
+  let rejectUnauthorized = !!this._rejectUnauthorized;
 
   if (options.requestCert !== undefined)
     requestCert = !!options.requestCert;
@@ -649,6 +657,9 @@ TLSSocket.prototype._start = function() {
 };
 
 TLSSocket.prototype.setServername = function(name) {
+  if (typeof name !== 'string') {
+    throw new errors.Error('ERR_INVALID_ARG_TYPE', 'name', 'string');
+  }
   this._handle.setServername(name);
 };
 

src/tls_wrap.cc

@@ -181,16 +181,10 @@ void TLSWrap::InitSSL() {
 void TLSWrap::Wrap(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  if (args.Length() < 1 || !args[0]->IsObject()) {
-    return env->ThrowTypeError(
-        "First argument should be a LibuvStreamWrap instance");
-  }
-  if (args.Length() < 2 || !args[1]->IsObject()) {
-    return env->ThrowTypeError(
-        "Second argument should be a SecureContext instance");
-  }
-  if (args.Length() < 3 || !args[2]->IsBoolean())
-    return env->ThrowTypeError("Third argument should be boolean");
+  CHECK_EQ(args.Length(), 3);
+  CHECK(args[0]->IsObject());
+  CHECK(args[1]->IsObject());
+  CHECK(args[2]->IsBoolean());
 
   Local<External> stream_obj = args[0].As<External>();
   Local<Object> sc = args[1].As<Object>();
@@ -758,8 +752,9 @@ void TLSWrap::SetVerifyMode(const FunctionCallbackInfo<Value>& args) {
   TLSWrap* wrap;
   ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
 
-  if (args.Length() < 2 || !args[0]->IsBoolean() || !args[1]->IsBoolean())
-    return env->ThrowTypeError("Bad arguments, expected two booleans");
+  CHECK_EQ(args.Length(), 2);
+  CHECK(args[0]->IsBoolean());
+  CHECK(args[1]->IsBoolean());
 
   if (wrap->ssl_ == nullptr)
     return env->ThrowTypeError("SetVerifyMode after destroySSL");
@@ -855,8 +850,8 @@ void TLSWrap::SetServername(const FunctionCallbackInfo<Value>& args) {
   TLSWrap* wrap;
   ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
 
-  if (args.Length() < 1 || !args[0]->IsString())
-    return env->ThrowTypeError("First argument should be a string");
+  CHECK_EQ(args.Length(), 1);
+  CHECK(args[0]->IsString());
 
   if (wrap->started_)
     return env->ThrowError("Already started.");

test/parallel/test-tls-basic-validations.js

@@ -39,8 +39,13 @@ assert.throws(() => tls.createServer({ ticketKeys: 'abcd' }),
 assert.throws(() => tls.createServer({ ticketKeys: Buffer.alloc(0) }),
               /TypeError: Ticket keys length must be 48 bytes/);
 
-assert.throws(() => tls.createSecurePair({}),
-              /TypeError: Second argument should be a SecureContext instance/);
+common.expectsError(
+  () => tls.createSecurePair({}),
+  {
+    code: 'ERR_ASSERTION',
+    message: 'context.context must be a NativeSecureContext'
+  }
+);
 
 {
   const buffer = Buffer.from('abcd');

test/parallel/test-tls-error-servername.js

@@ -0,0 +1,30 @@
+'use strict';
+
+// This tests the errors thrown from TLSSocket.prototype.setServername
+
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const { connect } = require('tls');
+const makeDuplexPair = require('../common/duplexpair');
+const { clientSide } = makeDuplexPair();
+
+const ca = fixtures.readKey('ca1-cert.pem');
+
+const client = connect({
+  socket: clientSide,
+  ca,
+  host: 'agent1'  // Hostname from certificate
+});
+
+[undefined, null, 1, true, {}].forEach((value) => {
+  common.expectsError(() => {
+    client.setServername(value);
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+    message: 'The "name" argument must be of type string'
+  });
+});