diff --git a/doc/api/tls.md b/doc/api/tls.md index 392052986e..b0b8180351 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -1043,6 +1043,10 @@ changes: pr-url: https://github.com/nodejs/node/pull/4099 description: The `ca` option can now be a single string containing multiple CA certificates. + - version: XXX + pr-url: XXX + description: The `min_version` and `max_version` can be used to restrict + the allowed TLS protocol versions. --> * `options` {Object} @@ -1103,6 +1107,16 @@ changes: passphrase: ]}`. The object form can only occur in an array. `object.passphrase` is optional. Encrypted keys will be decrypted with `object.passphrase` if provided, or `options.passphrase` if it is not. + * `max_version`: Maximum TLS version to allow. One of `'TLSv1.3'`, `TLSv1.2'`, + `'TLSv1.1'`, or `'TLSv1'`. Optional, defaults to `'TLSv1.2'`. Note that + TLS1.3 is not currently supported, do not attempt to allow it. If + `secureProtocol` is used to select a specific protocol version, + `max_version` will be ignored. + * `min_version`: Minimum TLS version to allow. One of `'TLSv1.3'`, `TLSv1.2'`, + `'TLSv1.1'`, or `'TLSv1'`. Optional, defaults to `'TLSv1'`. Note that + TLS1.3 is not currently supported, do not attempt to allow it. If + `secureProtocol` is used to select a specific protocol version, + `min_version` will be ignored. * `passphrase` {string} Shared passphrase used for a single private key and/or a PFX. * `pfx` {string|string[]|Buffer|Buffer[]|Object[]} PFX or PKCS12 encoded diff --git a/lib/_tls_common.js b/lib/_tls_common.js index 843c582b1f..1d5117169c 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -28,16 +28,18 @@ const { ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED, ERR_INVALID_ARG_TYPE } = require('internal/errors').codes; - const { SSL_OP_CIPHER_SERVER_PREFERENCE } = internalBinding('constants').crypto; // Lazily loaded from internal/crypto/util. let toBuf = null; + const { SecureContext: NativeSecureContext } = internalBinding('crypto'); -function SecureContext(secureProtocol, secureOptions, context) { +function SecureContext(secureProtocol, secureOptions, context, min_version, + max_version) { if (!(this instanceof SecureContext)) { - return new SecureContext(secureProtocol, secureOptions, context); + return new SecureContext(secureProtocol, secureOptions, context, + min_version, max_version); } if (context) { @@ -46,9 +48,9 @@ function SecureContext(secureProtocol, secureOptions, context) { this.context = new NativeSecureContext(); if (secureProtocol) { - this.context.init(secureProtocol); + this.context.init(min_version, max_version, secureProtocol); } else { - this.context.init(); + this.context.init(min_version, max_version); } } @@ -75,7 +77,9 @@ exports.createSecureContext = function createSecureContext(options, context) { if (options.honorCipherOrder) secureOptions |= SSL_OP_CIPHER_SERVER_PREFERENCE; - const c = new SecureContext(options.secureProtocol, secureOptions, context); + const c = new SecureContext(options.secureProtocol, secureOptions, context, + options.min_version || tls.DEFAULT_MIN_VERSION, + options.max_version || tls.DEFAULT_MAX_VERSION); var i; var val; diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 36440ad4ea..b7f2a49062 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -911,6 +911,16 @@ Server.prototype.setSecureContext = function(options) { else this.ca = undefined; + if (options.min_version) + this.min_version = options.min_version; + else + this.min_version = undefined; + + if (options.max_version) + this.max_version = options.max_version; + else + this.max_version = undefined; + if (options.secureProtocol) this.secureProtocol = options.secureProtocol; else @@ -967,6 +977,8 @@ Server.prototype.setSecureContext = function(options) { ciphers: this.ciphers, ecdhCurve: this.ecdhCurve, dhparam: this.dhparam, + min_version: this.min_version, + max_version: this.max_version, secureProtocol: this.secureProtocol, secureOptions: this.secureOptions, honorCipherOrder: this.honorCipherOrder, @@ -1019,6 +1031,8 @@ Server.prototype.setOptions = function(options) { if (options.clientCertEngine) this.clientCertEngine = options.clientCertEngine; if (options.ca) this.ca = options.ca; + if (options.min_version) this.min_version = options.min_version; + if (options.max_version) this.max_version = options.max_version; if (options.secureProtocol) this.secureProtocol = options.secureProtocol; if (options.crl) this.crl = options.crl; if (options.ciphers) this.ciphers = options.ciphers; diff --git a/lib/https.js b/lib/https.js index 12db2f452c..11a0fae9c8 100644 --- a/lib/https.js +++ b/lib/https.js @@ -186,6 +186,14 @@ Agent.prototype.getName = function getName(options) { if (options.servername && options.servername !== options.host) name += options.servername; + name += ':'; + if (options.min_version) + name += options.min_version; + + name += ':'; + if (options.max_version) + name += options.max_version; + name += ':'; if (options.secureProtocol) name += options.secureProtocol; diff --git a/lib/tls.js b/lib/tls.js index d6b86a4103..0ff8cdb8a2 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -53,6 +53,13 @@ exports.DEFAULT_CIPHERS = exports.DEFAULT_ECDH_CURVE = 'auto'; +// Disable TLS1.3 by default. The only reason for enabling it for now is to work +// on fixing cipher suite incompatibilities with TLS1.2 that prevent node from +// working with TLS1.3 in OpenSSL 1.1.1. +exports.DEFAULT_MAX_VERSION = 'TLSv1.2'; + +exports.DEFAULT_MIN_VERSION = 'TLSv1'; + exports.getCiphers = internalUtil.cachedResult( () => internalUtil.filterDuplicateStrings(binding.getSSLCiphers(), true) ); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index eab8aae180..98b6be2565 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -391,6 +391,24 @@ void SecureContext::New(const FunctionCallbackInfo& args) { } +int string_to_tls_protocol(const char* version_str) { + int version; + + if (strcmp(version_str, "TLSv1.3") == 0) { + version = TLS1_3_VERSION; + } else if (strcmp(version_str, "TLSv1.2") == 0) { + version = TLS1_2_VERSION; + } else if (strcmp(version_str, "TLSv1.1") == 0) { + version = TLS1_1_VERSION; + } else if (strcmp(version_str, "TLSv1") == 0) { + version = TLS1_VERSION; + } else { + version = 0; + } + return version; +} + + void SecureContext::Init(const FunctionCallbackInfo& args) { SecureContext* sc; ASSIGN_OR_RETURN_UNWRAP(&sc, args.Holder()); @@ -398,10 +416,21 @@ void SecureContext::Init(const FunctionCallbackInfo& args) { int min_version = 0; int max_version = 0; + + if (args[0]->IsString()) { + const node::Utf8Value min(env->isolate(), args[0]); + min_version = string_to_tls_protocol(*min); + } + + if (args[1]->IsString()) { + const node::Utf8Value max(env->isolate(), args[1]); + max_version = string_to_tls_protocol(*max); + } + const SSL_METHOD* method = TLS_method(); - if (args.Length() == 1 && args[0]->IsString()) { - const node::Utf8Value sslmethod(env->isolate(), args[0]); + if (args.Length() == 3 && args[2]->IsString()) { + const node::Utf8Value sslmethod(env->isolate(), args[2]); // Note that SSLv2 and SSLv3 are disallowed but SSLv23_method and friends // are still accepted. They are OpenSSL's way of saying that all known diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 52d4e73813..d452a8d673 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -222,7 +222,10 @@ void TLSWrap::SSLInfoCallback(const SSL* ssl_, int where, int ret) { } } - if (where & SSL_CB_HANDSHAKE_DONE) { + // SSL_CB_HANDSHAKE_START and SSL_CB_HANDSHAKE_DONE are called + // sending HelloRequest in OpenSSL-1.1.1. + // We need to check whether this is in a renegotiation state or not. + if (where & SSL_CB_HANDSHAKE_DONE && !SSL_renegotiate_pending(ssl)) { Local callback; c->established_ = true; diff --git a/test/parallel/test-https-agent-getname.js b/test/parallel/test-https-agent-getname.js index c29e09731d..d27763c215 100644 --- a/test/parallel/test-https-agent-getname.js +++ b/test/parallel/test-https-agent-getname.js @@ -12,7 +12,7 @@ const agent = new https.Agent(); // empty options assert.strictEqual( agent.getName({}), - 'localhost:::::::::::::::::' + 'localhost:::::::::::::::::::' ); // pass all options arguments @@ -39,5 +39,5 @@ const options = { assert.strictEqual( agent.getName(options), '0.0.0.0:443:192.168.1.1:ca:cert::ciphers:key:pfx:false:localhost:' + - 'secureProtocol:c,r,l:false:ecdhCurve:dhparam:0:sessionIdContext' + '::secureProtocol:c,r,l:false:ecdhCurve:dhparam:0:sessionIdContext' ); diff --git a/test/parallel/test-tls-handshake-error.js b/test/parallel/test-tls-handshake-error.js index 2aef1adcc3..13f366f41f 100644 --- a/test/parallel/test-tls-handshake-error.js +++ b/test/parallel/test-tls-handshake-error.js @@ -16,12 +16,12 @@ const server = tls.createServer({ rejectUnauthorized: true }, function(c) { }).listen(0, common.mustCall(function() { - assert.throws(() => { - tls.connect({ - port: this.address().port, - ciphers: 'RC4' - }, common.mustNotCall()); - }, /no cipher match/i); - - server.close(); + const client = tls.connect({ + port: this.address().port, + ciphers: 'RC4' + }, common.mustNotCall()); + client.on('error', common.mustCall((err) => { + assert(/No ciphers/.test(err.message)); + server.close(); + })); })); diff --git a/test/parallel/test-tls-set-ciphers-error.js b/test/parallel/test-tls-set-ciphers-error.js deleted file mode 100644 index 5ef08dda04..0000000000 --- a/test/parallel/test-tls-set-ciphers-error.js +++ /dev/null @@ -1,22 +0,0 @@ -'use strict'; -const common = require('../common'); - -if (!common.hasCrypto) - common.skip('missing crypto'); - -const assert = require('assert'); -const tls = require('tls'); -const fixtures = require('../common/fixtures'); - -{ - const options = { - key: fixtures.readKey('agent2-key.pem'), - cert: fixtures.readKey('agent2-cert.pem'), - ciphers: 'aes256-sha' - }; - assert.throws(() => tls.createServer(options, common.mustNotCall()), - /no cipher match/i); - options.ciphers = 'FOOBARBAZ'; - assert.throws(() => tls.createServer(options, common.mustNotCall()), - /no cipher match/i); -} diff --git a/test/sequential/test-tls-connect.js b/test/sequential/test-tls-connect.js index 291747aea7..680ab7bc86 100644 --- a/test/sequential/test-tls-connect.js +++ b/test/sequential/test-tls-connect.js @@ -50,12 +50,13 @@ const tls = require('tls'); const cert = fixtures.readSync('test_cert.pem'); const key = fixtures.readSync('test_key.pem'); - assert.throws(() => { - tls.connect({ - cert: cert, - key: key, - port: common.PORT, - ciphers: 'rick-128-roll' - }, common.mustNotCall()); - }, /no cipher match/i); + const client = tls.connect({ + cert: cert, + key: key, + port: common.PORT, + ciphers: 'rick-128-roll' + }, common.mustNotCall()); + client.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ECONNREFUSED'); + })); }