Repository navigation
[Converge] SSLv2/3 related commits #2514
Description
Continuing from nodejs/node-convergence-archive#20, marking against 4.0.0 milestone. Summary of that discussion is that SSLv2 and SSLv3 will not be reintroduced to v4, but there are some outstanding items raised by @misterdjules:
If I remember correctly, nodejs/node-v0.x-archive@69080f5 is a bug fix that is independent of SSLv2/SSLv3 support, and I would think we'd want to keep it.
nodejs/node-v0.x-archive@8d045a3 (and subsequent changes to the tests that it adds) has proven to be very valuable to catch issues when upgrading OpenSSL. It's actually what helped us find out about the problen that nodejs/node-v0.x-archive@69080f5 fixes, and others. So I would suggest to keep that too.
nodejs/node-v0.x-archive@d230fa9 fixes a minor but still real issue with documentation. I would also keep that.
@jasnell can you look back through that thread to see if there are any other commits missing from this list that should be included?
Activity
Fishrock123 commentedon Aug 24, 2015
@indutny / @shigeki could you look at the first two commits there?
indutny commentedon Aug 24, 2015
I'm fine with these changes.
shigeki commentedon Aug 25, 2015
nodejs/node-v0.x-archive@69080f5 is the fix only for node-v0.10 and is no longer needed for 0.12 and 4.0. nodejs/node-v0.x-archive@8d045a3 has large test cases, give me sometime for check them.
rvagg commentedon Aug 25, 2015
@shigeki I'm assigning this to you, can you make sure those land in master by the end of this week please and then close this issue
shigeki commentedon Aug 25, 2015
@rvagg Okay, no problem.
Checking all test cases in nodejs/node-v0.x-archive@8d045a3, I think this is not necessary to be merged into 4.0 because 4.0 has no
--enable-ssl2/ssl3command line options and configurations of SSL options/methods for SSLv2/v3 are explicitly disabled so that the remaining test cases are only for TLSv1_methods.The doc fix of nodejs/node-v0.x-archive@d230fa9 is for the description that was introduced only in joyent/node as nodejs/node-v0.x-archive@1349b68 to allow SSLv2/v3. This description is not included in 4.0 so it's no longer is needed neither.
Fishrock123 commentedon Aug 25, 2015
Are we sure those aren't useful? @misterdjules said they were in finding regressions, unless we already test all of that stuff?
shigeki commentedon Aug 26, 2015
We already have
test/parallel/test-tls-no-sslv23.jsandtest/parallel/test-tls-no-sslv3.jsthat can check regressions. They are not included in joyent/node.