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

[v11.x backport] tls: add min/max protocol version options #24676

Closed
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
21 changes: 18 additions & 3 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,10 @@ argument.
<!-- YAML
added: v0.11.13
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/24405
description: The `minVersion` and `maxVersion` can be used to restrict
the allowed TLS protocol versions.
- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/19794
description: The `ecdhCurve` cannot be set to `false` anymore due to a
Expand Down Expand Up @@ -1130,6 +1134,14 @@ 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'`.
* `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 @@ -1145,9 +1157,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.

Expand Down
40 changes: 31 additions & 9 deletions lib/_tls_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,34 @@ 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, 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) {
Expand All @@ -46,10 +62,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);
Expand All @@ -75,7 +96,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;

Expand Down
14 changes: 14 additions & 0 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,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 @@ -969,6 +979,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 @@ -1021,6 +1033,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;
Expand Down
8 changes: 8 additions & 0 deletions lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,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
4 changes: 4 additions & 0 deletions lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,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)
);
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
13 changes: 10 additions & 3 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -405,11 +405,15 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& 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<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 All @@ -434,6 +438,9 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& 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;
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:::::::::::::::::::'
);

// 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'
);
Loading