[Converge] SSLv2/3 disable/enable related commits #20
Comments
|
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. |
|
0ec78c961b32abf732620682678439c51f74dc86 configure: disable ssl2/ssl3 by default Should also be moved here. |
|
|
|
These will likely need to be refactored a bit as well. |
|
|
|
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 |
|
Related: nodejs/node#19 |
|
Another related: |
|
-1 from me, probably another TSC discussion topic |
|
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. |
|
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. |
|
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. |
|
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? |
|
Can you go ahead and create it? I haven't done so yet
|
|
@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. |
|
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. |
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. |
|
@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... |
|
@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. |
|
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. |
|
moved active to nodejs/node#2514 |
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@6c8593dThe text was updated successfully, but these errors were encountered: