fix(Server): set tls.DEFAULT_ECDH_CURVE to 'auto'
#1531
Conversation
nekolab
requested review from
alexander-akait and
michael-ciniawsky
as code owners
|
|
|
@nekolab on |
Codecov Report
@@ Coverage Diff @@
## master #1531 +/- ##
==========================================
+ Coverage 74.02% 74.02% +<.01%
==========================================
Files 10 10
Lines 666 670 +4
==========================================
+ Hits 493 496 +3
- Misses 173 174 +1
Continue to review full report at Codecov.
|
|
@evilebottnawi |
|
@nekolab thanks! |
michael-ciniawsky
changed the title
tls.DEFAULT_ECDH_CURVE to 'auto'
lib/Server.js
Outdated
| // breaking connection when certificate is not signed with prime256v1 | ||
| // change it to auto allows OpenSSL to select the curve automatically | ||
| // See https://github.com/nodejs/node/issues/16196 for more infomation | ||
| const verMatch = process.version.match(/^v(\d+).(\d+)/); |
There was a problem hiding this comment.
Is the version check required here or could this be defaulted to 'auto' in general? E.g was this being 'reverted' for node >= 10.0.0 for some reason? If so, why?
| const verMatch = process.version.match(/^v(\d+).(\d+)/); | |
| const version = process.version.match(/^v(\d+).(\d+)/); |
There was a problem hiding this comment.
Just saw #1531 (comment) Hmm....
There was a problem hiding this comment.
@michael-ciniawsky node@8 also affected
There was a problem hiding this comment.
const version = parseFloat(process.version.slice(1))
if (10 > version && version >= 8.6) {
...
}There was a problem hiding this comment.
@michael-ciniawsky node@10 is not affected
There was a problem hiding this comment.
Thanks for guiding, I've update the code and please take a look again.
There is another place using regex to match the version, I've changed it too
nekolab
force-pushed
the
master
branch
2 times, most recently
from
9926a44
to
81ec101
Compare
The default value of tls.DEFAULT_ECDH_CURVE is 'prime256v1', it breaks the connection when certificate is not compatible with the default curve since node 8.6.0. To fix this issue, we need set it to 'auto', makes OpenSSL select the curve automatically.
michael-ciniawsky
added
status: approved
and removed
pr: review
severity: 2 (regression)
severity: 3 (broken)
labels
The default value of tls.DEFAULT_ECDH_CURVE is 'prime256v1',
it breaks the connection when certificate is not compatible
with the default curve since node^8.6.0.
To fix this issue, we need set it to 'auto', makes OpenSSL
select the curve automatically.