This repository was archived by the owner on Oct 7, 2020. It is now read-only.
Repository navigation
This repository was archived by the owner on Oct 7, 2020. It is now read-only.
[Converge] SSLv2/3 disable/enable related commits #20
Description
These commits deal with the SSLv2/v3 enabling/disabling by default. These need to be reconciled with the related changes that have already been made in io.js. This issue may be updated to include additional related commits if necessary.
b9283cf9d17a51f9654b438216ecb743ed69a7ce tls: honorCipherOrder should not degrade defaults- nodejs/node-v0.x-archive@b9283cf1349b680badeb5d08ad82811af8350dc3e0f0397 crypto: allow forcing SSLv2/v3 via secureProtocol- nodejs/node-v0.x-archive@1349b686c8593d4568b59770aeb5b2cfa1fdc23ac422523 crypto: move disaling SSLv2/3 into JavaScript- nodejs/node-v0.x-archive@6c8593d
Activity
Fishrock123 commentedon May 21, 2015
Moved from nodejs/node#16
I think the sslv2/sslv3 stuff may need to be re-written to adapt to io.js changes, or maybe we could revert io.js changes?
Either that or we discuss dropping it again, which we should probably discuss anyways, since re-writing a patch when possible reversion in discussion afterwards before release makes little sense.
Fishrock123 commentedon May 21, 2015
0ec78c961b32abf732620682678439c51f74dc86 configure: disable ssl2/ssl3 by default
d6712917f5842fb7124f85902b7e6f68f12f4865 doc: document why SSL2/SSL3 is disabled
c1f4aacc75c0a64a81a2be8d6a3a23aa21a31b15 build: revert change to disable ssl2 and ssl3
Should also be moved here.
jasnell commentedon May 21, 2015
69080f5474369fc7fc4be7ab74ad2e1619eb2fbc tls: enforce secureOptions on incoming clients- nodejs/node-v0.x-archive@69080f5jasnell commentedon May 21, 2015
8d045a30e95602b443eb259a5021d33feb4df079 tests: add TLS tests matrix- nodejs/node-v0.x-archive@8d045a3These will likely need to be refactored a bit as well.
jasnell commentedon May 21, 2015
d230fa9eb7364edac09f65a27e11718671d8e773 doc: fix typo secureOptions in tls- nodejs/node-v0.x-archive@d230fa9bnoordhuis commentedon May 22, 2015
I've commented on it elsewhere but I'm fairly sure there is little support for enabling SSLv2/3 again in io.js, even if it's behind a flag.
/cc @nodejs/crypto
jasnell commentedon May 22, 2015
Related: nodejs/node#19
jasnell commentedon May 22, 2015
Another related:
408bffe212c350a56218e3562e5315da65235a2f test: fix ssl/tls options matrix test- nodejs/node-v0.x-archive@408bffervagg commentedon May 23, 2015
-1 from me, probably another TSC discussion topic
jasnell commentedon May 27, 2015
Ok, given that --enable-ssl2 and --enable-ssl3 were explicitly removed from io.js, this would need to be handled as a revert. @misterdjules @trevnorris @orangemocha @mhdawson @srl295 : if we think these should land in the converged repo, then we'd need to present an argument for reverting the io.js change.
We (IBM) are currently investigating our stuff to see what impact this would have on deployed code but, in general, I'm pretty certain that I'd be -1 on reverting which means these particular commits from joyent/node would not land in the converged stream.
orangemocha commentedon May 27, 2015
No objections from me on leaving those out. I think the rationale for introducing the flags was to mitigate the breaking change in a stable/LTS release. I think it's fine to drop the flags in the converged repo.
jasnell commentedon May 27, 2015
TSC Discussion: there's currently no interest in re-enabling ssl2 and ssl3 support. IBM will continue to investigate impact and will open a new issue if the changes need to be reverted. These commits should not land.
misterdjules commentedon Jun 2, 2015
That would also be a good candidate to mention in the documentation of potentially breaking changes between v0.12 and the next LTS version from the converged repository. @jasnell Do we already have a place to track these changes or should I go ahead and create it?
jasnell commentedon Jun 2, 2015
Can you go ahead and create it? I haven't done so yet
On Jun 2, 2015 11:44 AM, "Julien Gilli" notifications@github.com wrote:
misterdjules commentedon Jun 10, 2015
@jasnell I was looking at this again when documenting breaking changes between node v0.12 and the converged repo.
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.
Reopening to make sure it doesn't fall through the cracks.
misterdjules commentedon Jun 25, 2015
FYI, a RFC has recently been proposed about not using SSLv3 in any application.
mhdawson commentedon Jul 7, 2015
I think we might want to discuss that by general policy, the flags we add for fallback (like these) get dropped in the next major release. Having that documented up front would set the expectations that they should only be used as a stopgap until systems can be properly secured.
rvagg commentedon Jul 8, 2015
Dropping features is very very difficult, we'd have to go in with eyes wide open and consider how few things have ever been removed from Node; even this convergence work is an exercise in adding things back in that we tried to remove in io.js.
Fishrock123 commentedon Jul 8, 2015
@rvagg we could add placeholders for the flags so stuff doesn't break, but are you suggesting we should actually keep the flagged sslv2/3 functional from node? We basically already all voted no from both sides and said it was actually a step backwards. I mean if absolutely necessary I guess we could but...
rvagg commentedon Jul 8, 2015
@Fishrock123 no I'd like to get rid of them completely, but the argument in this thread is to provide a stepping-stone to removal for people who may be relying on it, I'm just suggesting the risk in it being difficult to remove such "temporary" features, this probably goes more for the cipher list issue than this one but same logic fits I think.
jasnell commentedon Jul 9, 2015
At this point, I'm +1 on just dropping them in the converged release. It should be understood by pretty much everyone that the converged release is going to have breaking changes relative to v0.12. Since these bits were already dropped by io.js, and since developers really should not be using these mechanisms in the first place or should already be in the process of migrating away, it shouldn't come as a surprise.
rvagg commentedon Aug 24, 2015
moved active to nodejs/node#2514