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: re-define max supported version as 1.2 #25024

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@sam-github
Copy link
Member

sam-github commented Dec 13, 2018

Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: #24658

Was part of the openssl 1.1.1 upgrade work, but given that distros are assuming that openssl 1.1.1 is API and ABI compatible with 1.1.0, and can be dropped in, and that node does in fact build when that happens, I think we should apply this change to master, and likely backport through to 10.x

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Show resolved Hide resolved src/node_crypto.cc
@danbev

danbev approved these changes Dec 14, 2018

@danbev

This comment has been minimized.

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Dec 14, 2018

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Dec 14, 2018

@Trott Trott added the author ready label Dec 14, 2018

@sam-github

This comment has been minimized.

Copy link
Member

sam-github commented Dec 14, 2018

@nodejs/lts Note that this cherry-picks clean to 11.x and 10.x, and should eventually land there. It is semver-patch.

tls: re-define max supported version as 1.2
Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: #24658

@Trott Trott force-pushed the sam-github:set-max-tls-to-v1.2 branch from baa6d5a to 058a96d Dec 17, 2018

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Dec 17, 2018

@sam-github

This comment has been minimized.

Copy link
Member

sam-github commented Dec 17, 2018

Landed in 19b59bf

@sam-github sam-github closed this Dec 17, 2018

@sam-github sam-github deleted the sam-github:set-max-tls-to-v1.2 branch Dec 17, 2018

sam-github added a commit that referenced this pull request Dec 17, 2018

tls: re-define max supported version as 1.2
Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: #24658

PR-URL: #25024
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins added a commit that referenced this pull request Dec 25, 2018

tls: re-define max supported version as 1.2
Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: #24658

PR-URL: #25024
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

@MylesBorins MylesBorins referenced this pull request Dec 25, 2018

Merged

v11.6.0 proposal #25175

refack added a commit to refack/node that referenced this pull request Jan 14, 2019

tls: re-define max supported version as 1.2
Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: nodejs#24658

PR-URL: nodejs#25024
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment