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

[v10.x backport] tls: add min/max protocol version options #24979

Closed

Conversation

sam-github
Copy link
Contributor

Backport of a backport... prep work for openssl 1.1.1 to land on v10.x

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: #24676
PR-URL: #24405
Reviewed-By: Refael Ackermann refack@gmail.com
Reviewed-By: Rod Vagg rod@vagg.org

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

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v10.x labels Dec 12, 2018
@sam-github
Copy link
Contributor Author

ci: https://ci.nodejs.org/job/node-test-pull-request/19455/

@nodejs/lts @nodejs/crypto

@rvagg rvagg added crypto Issues and PRs related to the crypto subsystem. semver-minor PRs that contain new features and should be released in the next minor version. labels Dec 18, 2018
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌 sweet, I don't recall the rules but I think it needs to be live in 11.x for a release or two before going in. Also tagged as semver-minor which may conflict with plans for 10.x, but this really needs to happen because we're running out of time on 1.1.1.

@MylesBorins
Copy link
Contributor

Our next semver minor isn't planned until march... should we do it earlier?

@sam-github
Copy link
Contributor Author

no hurry, it's a pre-req for openssl-1.1.1, but can land in the same semver-minor as the openssl update

Fill in correct pr-url: value in the YAML changelog that was missing
from f512f5e. The stanza was also sorted in the wrong order, most
recent is supposed to be in the beginning of the changes, not the end.

PR-URL: nodejs#24759
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
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: nodejs#24676
PR-URL: nodejs#24405
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@sam-github
Copy link
Contributor Author

Closed in favour of #26270

@sam-github sam-github closed this Feb 23, 2019
@sam-github sam-github deleted the backport-tls-min-max-to-10.x branch March 20, 2019 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants