From: Sam Roberts Date: Sun, 6 May 2018 04:52:34 +0000 (+0900) Subject: tls: add min/max protocol version options X-Git-Tag: archive/raspbian/10.15.2_dfsg-2+rpi1^2~13 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=0267fa22f1202c156035726ca26ca12b0fe0ccc4;p=nodejs.git tls: add min/max protocol version options The existing secureProtocol option only allows setting the allowed protocol to a specific version, or setting it to "all supported versions". It also used obscure strings based on OpenSSL C API functions. Directly setting the min or max is easier to use and explain. Backport-PR-URL: https://github.com/nodejs/node/pull/24676 PR-URL: https://github.com/nodejs/node/pull/24405 Reviewed-By: Refael Ackermann Reviewed-By: Rod Vagg Gbp-Pq: Topic ssl Gbp-Pq: Name acb73518b7274bacdfc133fd121e91dfd6ba460b.patch --- diff --git a/doc/api/errors.md b/doc/api/errors.md index 72df6b27c..ea0ff3cb5 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1652,6 +1652,17 @@ recommended to use 2048 bits or larger for stronger security. A TLS/SSL handshake timed out. In this case, the server must also abort the connection. + +### ERR_TLS_INVALID_PROTOCOL_VERSION + +Valid TLS protocol versions are `'TLSv1'`, `'TLSv1.1'`, or `'TLSv1.2'`. + + +### ERR_TLS_PROTOCOL_VERSION_CONFLICT + +Attempting to set a TLS protocol `minVersion` or `maxVersion` conflicts with an +attempt to set the `secureProtocol` explicitly. Use one mechanism or the other. + ### ERR_TLS_RENEGOTIATE diff --git a/doc/api/tls.md b/doc/api/tls.md index e66e7ffe7..6aeb58175 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -1069,6 +1069,14 @@ 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. + * `maxVersion` {string} Optionally set the maximum TLS version to allow. One + of `TLSv1.2'`, `'TLSv1.1'`, or `'TLSv1'`. Cannot be specified along with the + `secureProtocol` option, use one or the other. **Default:** `'TLSv1.2'`. + * `minVersion` {string} Optionally set the minimum TLS version to allow. One + of `TLSv1.2'`, `'TLSv1.1'`, or `'TLSv1'`. Cannot be specified along with the + `secureProtocol` option, use one or the other. It is not recommended to use + less than TLSv1.2, but it may be required for interoperability. + **Default:** `'TLSv1'`. * `passphrase` {string} Shared passphrase used for a single private key and/or a PFX. * `pfx` {string|string[]|Buffer|Buffer[]|Object[]} PFX or PKCS12 encoded @@ -1084,9 +1092,12 @@ changes: which is not usually necessary. This should be used carefully if at all! Value is a numeric bitmask of the `SSL_OP_*` options from [OpenSSL Options][]. - * `secureProtocol` {string} SSL method to use. The possible values are listed - as [SSL_METHODS][], use the function names as strings. For example, - `'TLSv1_2_method'` to force TLS version 1.2. **Default:** `'TLS_method'`. + * `secureProtocol` {string} The TLS protocol version to use. The possible + values are listed as [SSL_METHODS][], use the function names as strings. For + example, use `'TLSv1_1_method'` to force TLS version 1.1, or `'TLS_method'` + to allow any TLS protocol version. It is not recommended to use TLS versions + less than 1.2, but it may be required for interoperability. **Default:** + none, see `minVersion`. * `sessionIdContext` {string} Opaque identifier used by servers to ensure session state is not shared between applications. Unused by clients. diff --git a/lib/_tls_common.js b/lib/_tls_common.js index 1073f5d52..658bd1eaa 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -26,19 +26,36 @@ const { isArrayBufferView } = require('internal/util/types'); const tls = require('tls'); const { ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED, - ERR_INVALID_ARG_TYPE + ERR_INVALID_ARG_TYPE, + ERR_TLS_INVALID_PROTOCOL_VERSION, + ERR_TLS_PROTOCOL_VERSION_CONFLICT, } = require('internal/errors').codes; -const { SSL_OP_CIPHER_SERVER_PREFERENCE } = process.binding('constants').crypto; +const { + SSL_OP_CIPHER_SERVER_PREFERENCE, + TLS1_VERSION, + TLS1_1_VERSION, + TLS1_2_VERSION, +} = internalBinding('constants').crypto; // Lazily loaded var crypto = null; +function toV(which, v, def) { + if (v == null) v = def; + if (v === 'TLSv1') return TLS1_VERSION; + if (v === 'TLSv1.1') return TLS1_1_VERSION; + if (v === 'TLSv1.2') return TLS1_2_VERSION; + throw new ERR_TLS_INVALID_PROTOCOL_VERSION(v, which); +} + const { SecureContext: NativeSecureContext } = internalBinding('crypto'); -function SecureContext(secureProtocol, secureOptions, context) { +function SecureContext(secureProtocol, secureOptions, context, + minVersion, maxVersion) { if (!(this instanceof SecureContext)) { - return new SecureContext(secureProtocol, secureOptions, context); + return new SecureContext(secureProtocol, secureOptions, context, + minVersion, maxVersion); } if (context) { @@ -47,10 +64,15 @@ function SecureContext(secureProtocol, secureOptions, context) { this.context = new NativeSecureContext(); if (secureProtocol) { - this.context.init(secureProtocol); - } else { - this.context.init(); + if (minVersion != null) + throw new ERR_TLS_PROTOCOL_VERSION_CONFLICT(minVersion, secureProtocol); + if (maxVersion != null) + throw new ERR_TLS_PROTOCOL_VERSION_CONFLICT(maxVersion, secureProtocol); } + + this.context.init(secureProtocol, + toV('minimum', minVersion, tls.DEFAULT_MIN_VERSION), + toV('maximum', maxVersion, tls.DEFAULT_MAX_VERSION)); } if (secureOptions) this.context.setOptions(secureOptions); @@ -76,7 +98,8 @@ 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.minVersion, options.maxVersion); var i; var val; diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 9bfdd4062..88b7bad40 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -877,6 +877,8 @@ function Server(options, listener) { ciphers: this.ciphers, ecdhCurve: this.ecdhCurve, dhparam: this.dhparam, + minVersion: this.minVersion, + maxVersion: this.maxVersion, secureProtocol: this.secureProtocol, secureOptions: this.secureOptions, honorCipherOrder: this.honorCipherOrder, @@ -948,6 +950,8 @@ Server.prototype.setOptions = function(options) { if (options.clientCertEngine) this.clientCertEngine = options.clientCertEngine; if (options.ca) this.ca = options.ca; + if (options.minVersion) this.minVersion = options.minVersion; + if (options.maxVersion) this.maxVersion = options.maxVersion; 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 c1053da1a..6f31315a5 100644 --- a/lib/https.js +++ b/lib/https.js @@ -188,6 +188,14 @@ Agent.prototype.getName = function getName(options) { if (options.servername && options.servername !== options.host) name += options.servername; + name += ':'; + if (options.minVersion) + name += options.minVersion; + + name += ':'; + if (options.maxVersion) + name += options.maxVersion; + name += ':'; if (options.secureProtocol) name += options.secureProtocol; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index b418fa295..79f7085f2 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -870,6 +870,10 @@ E('ERR_TLS_CERT_ALTNAME_INVALID', 'Hostname/IP does not match certificate\'s altnames: %s', Error); E('ERR_TLS_DH_PARAM_SIZE', 'DH parameter size %s is less than 2048', Error); E('ERR_TLS_HANDSHAKE_TIMEOUT', 'TLS handshake timeout', Error); +E('ERR_TLS_INVALID_PROTOCOL_VERSION', + '%j is not a valid %s TLS protocol version', TypeError); +E('ERR_TLS_PROTOCOL_VERSION_CONFLICT', + 'TLS protocol version %j conflicts with secureProtocol %j', TypeError); E('ERR_TLS_RENEGOTIATE', 'Attempt to renegotiate TLS session failed', Error); E('ERR_TLS_RENEGOTIATION_DISABLED', 'TLS session renegotiation disabled for this socket', Error); diff --git a/lib/tls.js b/lib/tls.js index b8de6efc8..1e8686c8e 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -52,6 +52,10 @@ exports.DEFAULT_CIPHERS = exports.DEFAULT_ECDH_CURVE = 'auto'; +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_constants.cc b/src/node_constants.cc index 753b8f8fe..9cd50fe4e 100644 --- a/src/node_constants.cc +++ b/src/node_constants.cc @@ -1237,6 +1237,10 @@ void DefineCryptoConstants(Local target) { NODE_DEFINE_STRING_CONSTANT(target, "defaultCipherList", per_process_opts->tls_cipher_list.c_str()); + + NODE_DEFINE_CONSTANT(target, TLS1_VERSION); + NODE_DEFINE_CONSTANT(target, TLS1_1_VERSION); + NODE_DEFINE_CONSTANT(target, TLS1_2_VERSION); #endif NODE_DEFINE_CONSTANT(target, INT_MAX); } diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 1027f5f76..a43789a9b 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -395,11 +395,15 @@ void SecureContext::Init(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&sc, args.Holder()); Environment* env = sc->env(); - int min_version = 0; - int max_version = 0; + CHECK_EQ(args.Length(), 3); + CHECK(args[1]->IsInt32()); + CHECK(args[2]->IsInt32()); + + int min_version = args[1].As()->Value(); + int max_version = args[2].As()->Value(); const SSL_METHOD* method = TLS_method(); - if (args.Length() == 1 && args[0]->IsString()) { + if (args[0]->IsString()) { const node::Utf8Value sslmethod(env->isolate(), args[0]); // Note that SSLv2 and SSLv3 are disallowed but SSLv23_method and friends @@ -424,6 +428,9 @@ void SecureContext::Init(const FunctionCallbackInfo& args) { method = TLS_server_method(); } else if (strcmp(*sslmethod, "SSLv23_client_method") == 0) { method = TLS_client_method(); + } else if (strcmp(*sslmethod, "TLS_method") == 0) { + min_version = 0; + max_version = 0; } else if (strcmp(*sslmethod, "TLSv1_method") == 0) { min_version = TLS1_VERSION; max_version = TLS1_VERSION; diff --git a/test/fixtures/tls-connect.js b/test/fixtures/tls-connect.js index 43c3e6f0f..303061adc 100644 --- a/test/fixtures/tls-connect.js +++ b/test/fixtures/tls-connect.js @@ -46,28 +46,45 @@ exports.connect = function connect(options, callback) { const client = {}; const pair = { server, client }; - tls.createServer(options.server, function(conn) { - server.conn = conn; - conn.pipe(conn); - maybeCallback() - }).listen(0, function() { - server.server = this; + try { + tls.createServer(options.server, function(conn) { + server.conn = conn; + conn.pipe(conn); + maybeCallback() + }).listen(0, function() { + server.server = this; - const optClient = util._extend({ - port: this.address().port, - }, options.client); + const optClient = util._extend({ + port: this.address().port, + }, options.client); - tls.connect(optClient) - .on('secureConnect', function() { - client.conn = this; - maybeCallback(); - }) - .on('error', function(err) { + try { + tls.connect(optClient) + .on('secureConnect', function() { + client.conn = this; + maybeCallback(); + }) + .on('error', function(err) { + client.err = err; + client.conn = this; + maybeCallback(); + }); + } catch (err) { client.err = err; - client.conn = this; - maybeCallback(); - }); - }); + // The server won't get a connection, we are done. + callback(err, pair, cleanup); + callback = null; + } + }).on('tlsClientError', function(err, sock) { + server.conn = sock; + server.err = err; + maybeCallback(); + }); + } catch (err) { + // Invalid options can throw, report the error. + pair.server.err = err; + callback(err, pair, () => {}); + } function maybeCallback() { if (!callback) @@ -76,13 +93,13 @@ exports.connect = function connect(options, callback) { const err = pair.client.err || pair.server.err; callback(err, pair, cleanup); callback = null; - - function cleanup() { - if (server.server) - server.server.close(); - if (client.conn) - client.conn.end(); - } } } + + function cleanup() { + if (server.server) + server.server.close(); + if (client.conn) + client.conn.end(); + } } diff --git a/test/parallel/test-https-agent-getname.js b/test/parallel/test-https-agent-getname.js index b68850f21..5c95da549 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 @@ -40,5 +40,5 @@ const options = { assert.strictEqual( agent.getName(options), '0.0.0.0:443:192.168.1.1:ca:cert:dynamic:ciphers:key:pfx:false:localhost:' + - 'secureProtocol:c,r,l:false:ecdhCurve:dhparam:0:sessionIdContext' + '::secureProtocol:c,r,l:false:ecdhCurve:dhparam:0:sessionIdContext' );