diff --git a/doc/api/errors.md b/doc/api/errors.md index 3f2737f4df6e74..32281dfe2d8a1f 100644 --- a/doc/api/errors.md +++ b/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. + +### 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. + ### ERR_TLS_RENEGOTIATION_DISABLED diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index bb25eb5f155ea8..16d8b24c2979d4 100644 --- a/lib/_tls_wrap.js +++ b/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); }; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a057e628d90fcd..8bdf30a9e7f0fa 100644 --- a/lib/internal/errors.js +++ b/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', diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 78cc20810532c7..18b3cf01f406f0 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -181,16 +181,10 @@ void TLSWrap::InitSSL() { void TLSWrap::Wrap(const FunctionCallbackInfo& 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 stream_obj = args[0].As(); Local sc = args[1].As(); @@ -231,13 +225,11 @@ void TLSWrap::Receive(const FunctionCallbackInfo& args) { void TLSWrap::Start(const FunctionCallbackInfo& 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& 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& 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::OnClientHello, @@ -855,14 +841,10 @@ void TLSWrap::SetServername(const FunctionCallbackInfo& 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); diff --git a/test/parallel/test-tls-basic-validations.js b/test/parallel/test-tls-basic-validations.js index fc2743ce042064..501cfd91d0ed87 100644 --- a/test/parallel/test-tls-basic-validations.js +++ b/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'); diff --git a/test/parallel/test-tls-error-servername.js b/test/parallel/test-tls-error-servername.js new file mode 100644 index 00000000000000..8f6d1c5c6d8a46 --- /dev/null +++ b/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' +});