tls: migrate C++ errors to internal/errors.js

* Throw ERR_TLS_SNI_FROM_SERVER when setting server names on a
  server-side socket instead of returning silently
* Assert on wrap_->started and wrap_->ssl instead of throwing
  errors since these errors indicate that the user either uses
  private APIs, or monkey-patches internals, or hits a bug.

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>
This commit is contained in:
Joyee Cheung 2018-01-11 03:33:49 +08:00
parent 9ffebeab48
commit 1c29da8236
No known key found for this signature in database
GPG Key ID: F586868AAD831D0C
5 changed files with 37 additions and 21 deletions

View File

@ -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

View File

@ -660,6 +660,11 @@ 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);
};

View File

@ -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',

View File

@ -225,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
@ -747,17 +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());
CHECK_EQ(args.Length(), 2);
CHECK(args[0]->IsBoolean());
CHECK(args[1]->IsBoolean());
if (wrap->ssl_ == nullptr)
return env->ThrowTypeError("SetVerifyMode after destroySSL");
CHECK_NE(wrap->ssl_, nullptr);
int verify_mode;
if (wrap->is_server()) {
@ -785,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,
@ -852,12 +843,8 @@ void TLSWrap::SetServername(const FunctionCallbackInfo<Value>& args) {
CHECK_EQ(args.Length(), 1);
CHECK(args[0]->IsString());
if (wrap->started_)
return env->ThrowError("Already started.");
if (!wrap->is_client())
return;
CHECK(!wrap->started_);
CHECK(wrap->is_client());
CHECK_NE(wrap->ssl_, nullptr);

View File

@ -8,10 +8,12 @@ const fixtures = require('../common/fixtures');
if (!common.hasCrypto)
common.skip('missing crypto');
const { connect } = require('tls');
const { connect, TLSSocket } = require('tls');
const makeDuplexPair = require('../common/duplexpair');
const { clientSide } = makeDuplexPair();
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({
@ -28,3 +30,17 @@ const client = connect({
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'
});