Skip to content

Commit

Permalink
tls: revert default max to TLSv1.2
Browse files Browse the repository at this point in the history
TLSv1.3 is still supported when explicitly configured, but it is not the
default.

PR-URL: nodejs#26951
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
  • Loading branch information
sam-github authored and BethGriggs committed Apr 15, 2019
1 parent 7393e37 commit 109c097
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 13 deletions.
4 changes: 2 additions & 2 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -1351,7 +1351,7 @@ changes:
* `maxVersion` {string} Optionally set the maximum TLS version to allow. One
of `TLSv1.3`, `TLSv1.2'`, `'TLSv1.1'`, or `'TLSv1'`. Cannot be specified
along with the `secureProtocol` option, use one or the other.
**Default:** `'TLSv1.3'`, unless changed using CLI options. Using
**Default:** `'TLSv1.2'`, unless changed using CLI options. Using
`--tls-max-v1.2` sets the default to `'TLSv1.2`'. Using `--tls-max-v1.3`
sets the default to `'TLSv1.3'`. If multiple of the options are provided,
the highest maximum is used.
Expand All @@ -1360,7 +1360,7 @@ changes:
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
**Default:** `'TLSv1'`, unless changed using CLI options. Using
`--tls-min-v1.0` sets the default to `'TLSv1'`. Using `--tls-min-v1.1` sets
the default to `'TLSv1.1'`. Using `--tls-min-v1.3` sets the default to
`'TLSv1.3'`. If multiple of the options are provided, the lowest minimum is
Expand Down
4 changes: 1 addition & 3 deletions lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ exports.DEFAULT_CIPHERS =

exports.DEFAULT_ECDH_CURVE = 'auto';

exports.DEFAULT_MAX_VERSION = 'TLSv1.3';

if (getOptionValue('--tls-min-v1.0'))
exports.DEFAULT_MIN_VERSION = 'TLSv1';
else if (getOptionValue('--tls-min-v1.1'))
Expand All @@ -70,7 +68,7 @@ if (getOptionValue('--tls-max-v1.3'))
else if (getOptionValue('--tls-max-v1.2'))
exports.DEFAULT_MAX_VERSION = 'TLSv1.2';
else
exports.DEFAULT_MAX_VERSION = 'TLSv1.3'; // Will depend on node version.
exports.DEFAULT_MAX_VERSION = 'TLSv1.2'; // Will depend on node version.


exports.getCiphers = internalUtil.cachedResult(
Expand Down
4 changes: 2 additions & 2 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -341,15 +341,15 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
&EnvironmentOptions::tls_min_v1_3,
kAllowedInEnvironment);
AddOption("--tls-max-v1.2",
"set default TLS maximum to TLSv1.2 (default: TLSv1.3)",
"set default TLS maximum to TLSv1.2 (default: TLSv1.2)",
&EnvironmentOptions::tls_max_v1_2,
kAllowedInEnvironment);
// Current plan is:
// - 11.x and below: TLS1.3 is opt-in with --tls-max-v1.3
// - 12.x: TLS1.3 is opt-out with --tls-max-v1.2
// In either case, support both options they are uniformly available.
AddOption("--tls-max-v1.3",
"set default TLS maximum to TLSv1.3 (default: TLSv1.3)",
"set default TLS maximum to TLSv1.3 (default: TLSv1.2)",
&EnvironmentOptions::tls_max_v1_3,
kAllowedInEnvironment);
}
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-tls-cli-min-version-1.0.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ if (!common.hasCrypto) common.skip('missing crypto');
const assert = require('assert');
const tls = require('tls');

assert.strictEqual(tls.DEFAULT_MAX_VERSION, 'TLSv1.3');
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.
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-tls-cli-min-version-1.1.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ if (!common.hasCrypto) common.skip('missing crypto');
const assert = require('assert');
const tls = require('tls');

assert.strictEqual(tls.DEFAULT_MAX_VERSION, 'TLSv1.3');
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.
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-tls-cli-min-version-1.3.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ if (!common.hasCrypto) common.skip('missing crypto');
const assert = require('assert');
const tls = require('tls');

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

// Check the min-max version protocol versions against these CLI settings.
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-tls-client-renegotiation-13.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --tls-max-v1.3
'use strict';

const common = require('../common');
Expand Down
9 changes: 7 additions & 2 deletions test/parallel/test-tls-min-max-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@ function test(cmin, cmax, cprot, smin, smax, sprot, proto, cerr, serr) {

const U = undefined;

// Default protocol is the max version.
test(U, U, U, U, U, U, DEFAULT_MAX_VERSION);
if (DEFAULT_MAX_VERSION === 'TLSv1.2' && DEFAULT_MIN_VERSION === 'TLSv1.3') {
// No connections are possible by default.
test(U, U, U, U, U, U, U, 'ERR_SSL_NO_PROTOCOLS_AVAILABLE', U);
} else {
// Default protocol is the max version.
test(U, U, U, U, U, U, DEFAULT_MAX_VERSION);
}

// Insecure or invalid protocols cannot be enabled.
test(U, U, U, U, U, 'SSLv2_method',
Expand Down
6 changes: 5 additions & 1 deletion test/parallel/test-tls-set-ciphers.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ const fixtures = require('../common/fixtures');
// Test cipher: option for TLS.

const {
assert, connect, keys
assert, connect, keys, tls
} = require(fixtures.path('tls-connect'));

const tls13 = !!require('constants').TLS1_3_VERSION;

if (tls13)
tls.DEFAULT_MAX_VERSION = 'TLSv1.3';

function test(cciphers, sciphers, cipher, cerr, serr) {
assert(cipher || cerr || serr, 'test missing any expectations');
Expand Down

0 comments on commit 109c097

Please sign in to comment.