tls_wrap: migrate synchronous errors

doc/api/errors.md

@@ -1530,6 +1530,12 @@ a hostname in the first parameter.
 An excessive amount of TLS renegotiations is detected, which is a potential
 vector for denial-of-service attacks.
 
+<a id="ERR_TLS_SNI_FROM_SERVER"></a>
+### ERR_TLS_SNI_FROM_SERVER
+
+An attempt was made to issue Server Name Indication from a TLS server-side
+socket, which is only valid from a client.
+
 <a id="ERR_TLS_RENEGOTIATION_DISABLED"></a>
 ### ERR_TLS_RENEGOTIATION_DISABLED
 

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,14 @@ TLSSocket.prototype._start = function() {
 };
 
 TLSSocket.prototype.setServername = function(name) {
+  if (typeof name !== 'string') {
+    throw new errors.Error('ERR_INVALID_ARG_TYPE', 'name', 'string');
+  }
+
+  if (this._tlsOptions.isServer) {
+    throw new errors.Error('ERR_TLS_SNI_FROM_SERVER');
+  }
+
   this._handle.setServername(name);
 };
 

lib/internal/errors.js

@@ -480,6 +480,8 @@ E('ERR_TLS_RENEGOTIATION_FAILED', 'Failed to renegotiate');
 E('ERR_TLS_REQUIRED_SERVER_NAME',
   '"servername" is required parameter for Server.addContext');
 E('ERR_TLS_SESSION_ATTACK', 'TLS session renegotiation attack detected');
+E('ERR_TLS_SNI_FROM_SERVER',
+  'Cannot issue SNI from a TLS server-side socket');
 E('ERR_TRANSFORM_ALREADY_TRANSFORMING',
   'Calling transform done when still transforming');
 E('ERR_TRANSFORM_WITH_LENGTH_0',

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>();
@@ -231,13 +225,11 @@ void TLSWrap::Receive(const FunctionCallbackInfo<Value>& args) {
 
 
 void TLSWrap::Start(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-
   TLSWrap* wrap;
   ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
 
-  if (wrap->started_)
-    return env->ThrowError("Already started.");
+  CHECK(!wrap->started_);
+
   wrap->started_ = true;
 
   // Send ClientHello handshake
@@ -753,16 +745,13 @@ int TLSWrap::DoShutdown(ShutdownWrap* req_wrap) {
 
 
 void TLSWrap::SetVerifyMode(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(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");
-
-  if (wrap->ssl_ == nullptr)
-    return env->ThrowTypeError("SetVerifyMode after destroySSL");
+  CHECK_EQ(args.Length(), 2);
+  CHECK(args[0]->IsBoolean());
+  CHECK(args[1]->IsBoolean());
+  CHECK_NE(wrap->ssl_, nullptr);
 
   int verify_mode;
   if (wrap->is_server()) {
@@ -790,10 +779,7 @@ void TLSWrap::EnableSessionCallbacks(
     const FunctionCallbackInfo<Value>& args) {
   TLSWrap* wrap;
   ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
-  if (wrap->ssl_ == nullptr) {
-    return wrap->env()->ThrowTypeError(
-        "EnableSessionCallbacks after destroySSL");
-  }
+  CHECK_NE(wrap->ssl_, nullptr);
   wrap->enable_session_callbacks();
   crypto::NodeBIO::FromBIO(wrap->enc_in_)->set_initial(kMaxHelloLength);
   wrap->hello_parser_.Start(SSLWrap<TLSWrap>::OnClientHello,
@@ -855,14 +841,10 @@ 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");
-
-  if (wrap->started_)
-    return env->ThrowError("Already started.");
-
-  if (!wrap->is_client())
-    return;
+  CHECK_EQ(args.Length(), 1);
+  CHECK(args[0]->IsString());
+  CHECK(!wrap->started_);
+  CHECK(wrap->is_client());
 
   CHECK_NE(wrap->ssl_, nullptr);
 

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,46 @@
+'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, TLSSocket } = require('tls');
+const makeDuplexPair = require('../common/duplexpair');
+const { clientSide, serverSide } = makeDuplexPair();
+
+const key = fixtures.readKey('agent1-key.pem');
+const cert = fixtures.readKey('agent1-cert.pem');
+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'
+  });
+});
+
+const server = new TLSSocket(serverSide, {
+  isServer: true,
+  key,
+  cert,
+  ca
+});
+
+common.expectsError(() => {
+  server.setServername('localhost');
+}, {
+  code: 'ERR_TLS_SNI_FROM_SERVER',
+  message: 'Cannot issue SNI from a TLS server-side socket'
+});