From 56b2a0986c8f0d3cecde4ae661a4da9e08b6c43d Mon Sep 17 00:00:00 2001 From: Shigeki Ohtsu Date: Sun, 6 May 2018 13:52:34 +0900 Subject: [PATCH] tls: add min/max_version and their defaults --- doc/api/tls.md | 14 ++++++++++ lib/_tls_common.js | 12 ++++++--- lib/_tls_wrap.js | 14 ++++++++++ lib/https.js | 8 ++++++ lib/tls.js | 7 +++++ src/node_crypto.cc | 33 +++++++++++++++++++++-- test/parallel/test-https-agent-getname.js | 4 +-- 7 files changed, 84 insertions(+), 8 deletions(-) diff --git a/doc/api/tls.md b/doc/api/tls.md index 31ad74a6ce8d1f..4d94c31df56c50 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -1050,6 +1050,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} @@ -1110,6 +1114,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 7153334a141777..bb28357ad05e43 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -28,20 +28,22 @@ 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) { + +function SecureContext(secureProtocol, secureOptions, min_version, + max_version) { if (!(this instanceof SecureContext)) { return new SecureContext(secureProtocol, secureOptions); } this.context = new NativeSecureContext(); - this.context.init(secureProtocol); + this.context.init(min_version, max_version, secureProtocol); if (secureOptions) this.context.setOptions(secureOptions); } @@ -66,7 +68,9 @@ exports.createSecureContext = function createSecureContext(options) { if (options.honorCipherOrder) secureOptions |= SSL_OP_CIPHER_SERVER_PREFERENCE; - const c = new SecureContext(options.secureProtocol, secureOptions); + const c = new SecureContext(options.secureProtocol, secureOptions, + 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 f0d86f3d870f09..42812459d30f79 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -904,6 +904,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 @@ -960,6 +970,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, @@ -1012,6 +1024,8 @@ Server.prototype.setOptions = util.deprecate(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 12db2f452c17fa..11a0fae9c882d3 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 d6b86a410374b7..0ff8cdb8a245fd 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 d5e0c4f244d5b3..ad337d8b08d357 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/test/parallel/test-https-agent-getname.js b/test/parallel/test-https-agent-getname.js index c29e09731df0b2..d27763c215c540 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' );