-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
[v11.x backport] tls: add min/max protocol version options #24676
Conversation
@sam-github sadly an error occured when I tried to trigger a build :( |
da30b3f
to
1e29a78
Compare
1e29a78
to
deea5a0
Compare
@nodejs/crypto @nodejs/lts Turns out backporting this is not a trivial git cherry-pick, cleanup, and ci, it will take a couple hours of work. I'll take a shot at it next week. It depends on non-semver-major parts of 60eca6a:
Can I squash the addition of "TLS_method" into this commit? Is it still a "backport" if I do that? |
Does it matter? I think do whatever is easiest, I can see how this is going to be complicated. Thanks for taking it on though! |
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>
deea5a0
to
eabf3e4
Compare
Resumed build: https://ci.nodejs.org/job/node-test-pull-request/19226/ ✔️ |
@rvagg PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I guess it would actually be possible to also add the cli flags just that they would work the other way around in this case: they would set the minimum to a higher level instead of lowering that. But since that's also possible with the option, I guess it's fine this way and we could also go ahead and backport that later on if we feel like it. |
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>
Landed in 1fba610 🎉 |
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>
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>
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>
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>
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.
PR-URL: #24405
Reviewed-By: Refael Ackermann refack@gmail.com
Reviewed-By: Rod Vagg rod@vagg.org
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes