Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tls: add min/max protocol version options #24405

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1655,6 +1655,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.

<a id="ERR_TLS_INVALID_PROTOCOL_VERSION"></a>
### ERR_TLS_INVALID_PROTOCOL_VERSION

Valid TLS protocol versions are `'TLSv1'`, `'TLSv1.1'`, or `'TLSv1.2'`.

<a id="ERR_TLS_PROTOCOL_VERSION_CONFLICT"></a>
### 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.

<a id="ERR_TLS_RENEGOTIATE"></a>
### ERR_TLS_RENEGOTIATE

Expand Down
19 changes: 15 additions & 4 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: REPLACEME
pr-url: REPLACEME
description: The `minVersion` and `maxVersion` can be used to restrict
the allowed TLS protocol versions.
-->

* `options` {Object}
Expand Down Expand Up @@ -1103,6 +1107,16 @@ changes:
passphrase: <string>]}`. 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.2'`, unless changed using CLI options. Using
`--tls-v1.0` changes the default to `'TLSv1'`. Using `--tls-v1.1` changes
the default to `'TLSv1.1'`.
* `passphrase` {string} Shared passphrase used for a single private key and/or
a PFX.
* `pfx` {string|string[]|Buffer|Buffer[]|Object[]} PFX or PKCS12 encoded
Expand All @@ -1123,10 +1137,7 @@ changes:
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:**
`'TLSv1_2_method'`, unless changed using CLI options. Using the `--tlsv1.0`
CLI option is like `'TLS_method'` except protocols earlier than TLSv1.0 are
not allowed, and using the `--tlsv1.1` CLI option is like `'TLS_method'`
except that protocols earlier than TLSv1.1 are not allowed.
none, see `minVersion`.
* `sessionIdContext` {string} Opaque identifier used by servers to ensure
session state is not shared between applications. Unused by clients.

Expand Down
39 changes: 32 additions & 7 deletions lib/_tls_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,46 @@ 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 } = internalBinding('constants').crypto;
const {
SSL_OP_CIPHER_SERVER_PREFERENCE,
TLS1_VERSION,
TLS1_1_VERSION,
TLS1_2_VERSION,
} = internalBinding('constants').crypto;

// Lazily loaded from internal/crypto/util.
let toBuf = 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) {
function SecureContext(secureProtocol, secureOptions, minVersion, maxVersion) {
if (!(this instanceof SecureContext)) {
return new SecureContext(secureProtocol, secureOptions);
return new SecureContext(secureProtocol, secureOptions, minVersion,
maxVersion);
}

if (secureProtocol) {
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 = new NativeSecureContext();
this.context.init(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);
}
Expand All @@ -66,7 +90,8 @@ 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.minVersion, options.maxVersion);
var i;
var val;

Expand Down
14 changes: 14 additions & 0 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,16 @@ Server.prototype.setSecureContext = function(options) {
else
this.ca = undefined;

if (options.minVersion)
this.minVersion = options.minVersion;
else
this.minVersion = undefined;

if (options.maxVersion)
this.maxVersion = options.maxVersion;
else
this.maxVersion = undefined;

if (options.secureProtocol)
this.secureProtocol = options.secureProtocol;
else
Expand Down Expand Up @@ -974,6 +984,8 @@ Server.prototype.setSecureContext = function(options) {
ciphers: this.ciphers,
ecdhCurve: this.ecdhCurve,
dhparam: this.dhparam,
minVersion: this.minVersion,
maxVersion: this.maxVersion,
secureProtocol: this.secureProtocol,
secureOptions: this.secureOptions,
honorCipherOrder: this.honorCipherOrder,
Expand Down Expand Up @@ -1026,6 +1038,8 @@ Server.prototype.setOptions = util.deprecate(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;
Expand Down
8 changes: 8 additions & 0 deletions lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,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;
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,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);
Expand Down
10 changes: 10 additions & 0 deletions lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ internalUtil.assertCrypto();
const { isArrayBufferView } = require('internal/util/types');

const net = require('net');
const { getOptionValue } = require('internal/options');
const url = require('url');
const binding = internalBinding('crypto');
const { Buffer } = require('buffer');
Expand All @@ -53,6 +54,15 @@ exports.DEFAULT_CIPHERS =

exports.DEFAULT_ECDH_CURVE = 'auto';

exports.DEFAULT_MAX_VERSION = 'TLSv1.2';

if (getOptionValue('--tls-v1.0'))
exports.DEFAULT_MIN_VERSION = 'TLSv1';
else if (getOptionValue('--tls-v1.1'))
exports.DEFAULT_MIN_VERSION = 'TLSv1.1';
else
exports.DEFAULT_MIN_VERSION = 'TLSv1.2';

exports.getCiphers = internalUtil.cachedResult(
() => internalUtil.filterDuplicateStrings(binding.getSSLCiphers(), true)
);
Expand Down
4 changes: 4 additions & 0 deletions src/node_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1237,6 +1237,10 @@ void DefineCryptoConstants(Local<Object> 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);
}
Expand Down
14 changes: 8 additions & 6 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -396,14 +396,15 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&sc, args.Holder());
Environment* env = sc->env();

int min_version = TLS1_2_VERSION;
int max_version = 0;
const SSL_METHOD* method = TLS_method();
CHECK_EQ(args.Length(), 3);
CHECK(args[1]->IsInt32());
CHECK(args[2]->IsInt32());

if (env->options()->tls_v1_1) min_version = TLS1_1_VERSION;
if (env->options()->tls_v1_0) min_version = TLS1_VERSION;
int min_version = args[1].As<Int32>()->Value();
int max_version = args[2].As<Int32>()->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
Expand Down Expand Up @@ -500,6 +501,7 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {

SSL_CTX_set_min_proto_version(sc->ctx_.get(), min_version);
SSL_CTX_set_max_proto_version(sc->ctx_.get(), max_version);

// OpenSSL 1.1.0 changed the ticket key size, but the OpenSSL 1.0.x size was
// exposed in the public API. To retain compatibility, install a callback
// which restores the old algorithm.
Expand Down
69 changes: 43 additions & 26 deletions test/fixtures/tls-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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();
}
}
4 changes: 2 additions & 2 deletions test/parallel/test-https-agent-getname.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const agent = new https.Agent();
// empty options
assert.strictEqual(
agent.getName({}),
'localhost:::::::::::::::::'
'localhost:::::::::::::::::::'
refack marked this conversation as resolved.
Show resolved Hide resolved
);

// pass all options arguments
Expand Down Expand Up @@ -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'
);
15 changes: 15 additions & 0 deletions test/parallel/test-tls-cli-min-version-1.0.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Flags: --tls-v1.0 --tls-v1.1
sam-github marked this conversation as resolved.
Show resolved Hide resolved
'use strict';
const common = require('../common');
if (!common.hasCrypto) common.skip('missing crypto');

// Check that `node --tls-v1.0` is supported, and overrides --tls-v1.1.

const assert = require('assert');
const tls = require('tls');

assert.strictEqual(tls.DEFAULT_MAX_VERSION, 'TLSv1.2');
assert.strictEqual(tls.DEFAULT_MIN_VERSION, 'TLSv1');

// Check the min-max version protocol versions against these CLI settings.
require('./test-tls-min-max-version.js');
15 changes: 15 additions & 0 deletions test/parallel/test-tls-cli-min-version-1.1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Flags: --tls-v1.1
'use strict';
const common = require('../common');
if (!common.hasCrypto) common.skip('missing crypto');

// Check that node `--tls-v1.1` is supported.

const assert = require('assert');
const tls = require('tls');

assert.strictEqual(tls.DEFAULT_MAX_VERSION, 'TLSv1.2');
assert.strictEqual(tls.DEFAULT_MIN_VERSION, 'TLSv1.1');

// Check the min-max version protocol versions against these CLI settings.
require('./test-tls-min-max-version.js');
Loading