Perfect your code
With built-in code review tools, GitHub makes it easy to raise the quality bar before you ship. Join the 36 million developers who've merged over 200 million pull requests.
Sign up for free See pricing for teams and enterprisestls: add min/max protocol version options #24405
Conversation
sam-github
added
the
semver-minor
label
Nov 16, 2018
This comment has been minimized.
This comment has been minimized.
nodejs-github-bot
added
the
lib / src
label
Nov 16, 2018
sam-github
requested review from
bnoordhuis,
shigeki and
rvagg
Nov 16, 2018
refack
approved these changes
Nov 16, 2018
|
LGTM % semverity |
This comment has been minimized.
This comment has been minimized.
|
Missing test for |
refack
added
the
crypto
label
Nov 16, 2018
vsemozhetbyt
reviewed
Nov 16, 2018
sam-github
force-pushed the
sam-github:tls-min-max-version
branch
from
77ae2fe
to
9b267ff
Nov 17, 2018
mscdex
reviewed
Nov 17, 2018
doc/api/tls.md Outdated
sam-github
added
semver-major
and removed
semver-minor
labels
Nov 17, 2018
bnoordhuis
reviewed
Nov 19, 2018
|
Just to make sure we're all on the same page: Sam, can you add a few more tests that verify that passing in the new options actually affects a TLS connection? Bonus points if it also tests the interaction with the |
rvagg
requested changes
Nov 19, 2018
sam-github
force-pushed the
sam-github:tls-min-max-version
branch
2 times, most recently
from
017939b
to
2cae9ab
Nov 19, 2018
sam-github
added
semver-minor
and removed
semver-major
labels
Nov 19, 2018
sam-github
force-pushed the
sam-github:tls-min-max-version
branch
from
2cae9ab
to
61e08ae
Nov 19, 2018
This comment has been minimized.
This comment has been minimized.
|
@shigeki I removed your name from the commit so you don't get |
vsemozhetbyt
reviewed
Nov 19, 2018
doc/api/tls.md Outdated
sam-github
force-pushed the
sam-github:tls-min-max-version
branch
from
61e08ae
to
e07e955
Nov 19, 2018
This comment has been minimized.
This comment has been minimized.
|
@rvagg @bnoordhuis PTAL |
sam-github commentedNov 16, 2018
•
edited
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.
This is a rework of
tls: add min/max_version and their defaultsfrom https://github.com/shigeki/node/commits/WIP_upgrade_openssl111_tls12_only onto master. The original conflicted with more recent commits to master, but while doing the docs for #24386 I realized it also broke #23814. I'm PRing this directly now because it doesn't have a dependency on OpenSSL 1.1.1. Getting it into master should make @shigeki 's work easier, and his openssl 1.1.1 branch shorter. Also, landing it will stop it from getting more conflicts.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes