Deprecate SSLv2 support on v0.12 and v0.10

[shigeki]

OpenSSL-1.0.1s disables SSLv2 by default for against DROWN attack(CVE-2016-0800). https://www.openssl.org/news/secadv/20160301.txt

There are no reasons to continue to support SSLv2 on Node LTS.

[rvagg]

I'd like more input on this, so /cc @nodejs/crypto @nodejs/ctc (@nodejs/lts).

Reference to read up on if you're not following the details: https://drownattack.com/

Of particular relevance is this:

Modern servers and clients use the TLS encryption protocol. However, due to misconfigurations, many servers also still support SSLv2, a 1990s-era predecessor to TLS. This support did not matter in practice, since no up-to-date clients actually use SSLv2. Therefore, even though SSLv2 is known to be badly insecure, until now, merely supporting SSLv2 was not considered a security problem, because clients never used it.

DROWN shows that merely supporting SSLv2 is a threat to modern servers and clients. It allows an attacker to decrypt modern TLS connections between up-to-date clients and servers by sending probes to a server that supports SSLv2 and uses the same private key.

We are already not enabling it by default for 0.10 and 0.12 (right?) and you have to use --enable-ssl2 to get it. My preference is always to non baby our users, I'm absolutely fine with a "secure by default" approach my experience tells me that there are way too many crazy ways that people are using Node in the wild that it's difficult to know whether what we think is an appropriate course of action isn't actually going to cause significant problems for a portion of users. So caveat emptor and a conservative process is my preferred way of making changes to LTS in particular.

However, the difficulty here is that having sslv2 enabled and not even necessarily using it can open you up to attack.

At this stage my vote would be to print a verbose warning at start-up if you --enable-ssl2 that points to https://drownattack.com/ or other resource, rather than removing it entirely. But that's a soft preference and I'm not going to fight too hard if others have strong conviction here, so speak up!

[mhdawson]

I think I'd prefer if we removed the option by default, but allowed that to be reverted with --security-revert={cvenum}. So if you added --security-revert={CVE-2016-0800} you'd get the warning and be able to use --enable-ssl2. (I think --security-revert was going to warn for each one reverted)

It leverages the approach we put in place the last time and makes the bar a little higher in that you have to actively choose to add --security-revert while at the same time providing the customer a work around if they absolutely still need to keep using it.

[bnoordhuis]

I'll be the contrarian today. :-)

SSLv2 was superseded - because it was known to be flawed - by SSLv3 twenty years ago, in 1996. It was officially deprecated (in RFC 6176) half a decade ago, in 2011. From the abstract:

This document requires that when Transport Layer Security (TLS)
clients and servers establish connections, they never negotiate the
use of Secure Sockets Layer (SSL) version 2.0.

I agree with Rod that babying your users is bad but I don't think that's what we're doing if we remove SSLv2 support for good.

Honestly, I can't think of a remotely valid use case that would require SSLv2. Even if your application needs to talk to a 2001-era set-top box, it's going to communicate over SSLv3, not SSLv2.

[evanlucas]

Do we have any data on whether people are even using the --enable-ssl2 flag still?

[shigeki]

Note that the current HEAD of v0.12/v0.10-staging cannot work SSLv2 even if we use --enable-ssl2.

According to openssl/openssl@56f1acf, we need to add the following patch.

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 454c9be..754d159 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -391,6 +391,9 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
   SSL_CTX_sess_set_get_cb(sc->ctx_, SSLWrap<Connection>::GetSessionCallback);
   SSL_CTX_sess_set_new_cb(sc->ctx_, SSLWrap<Connection>::NewSessionCallback);

+  if (SSL2_ENABLE)
+    SSL_CTX_clear_options(sc->ctx_, SSL_OP_NO_SSLv2);
+
   sc->ca_store_ = NULL;
 }

Furthermore, three SSLv2 ciphers of EXP-RC2-CBC-MD5, DES-CBC-MD5 and DES-CBC3-MD5 are completely disabled by #if 0 so that we cannot provide exact the same features of SSLv2 as before unless we change them.

I don't think it is worth while continuing to support SSLv2 even in LTS with paying such costs.

[mhdawson]

While I prefer that we avoid breaking changes, we have had to accept them from openssl and pass them along in the past. In this case since we'd have to actively change the Node.js code to enable insecure algs and it would only be a partial fix without floating patches on openssl, I'd have a hard time making a strong case for doing it.

[jasnell]

I'm leaning the same direction as @shigeki and @bnoordhuis. At this point there is exceedingly little reason to continue supporting SSLv2 in any way.

[evanlucas]

+1 to removing it unless someone makes a very compelling argument against it

[Fishrock123]

🔪

Does that mean the flag will be removed, or that it simply won't do anything?

Alternatively we could make the existing flag just warn and move the functionality under --security-revert, but I doubt we feel strongly enough to do that at this point.

Edit: I just read that the security patch to OpenSSL itself disables it outright. 🔪 🔪

[brody4hire]

Alternatively we could make the existing flag just warn

+1 to this part

…

and move the functionality under `--security-revert`

-1 to this part I would leave it to the user to do this part "manually" if s/he needs the old SSL v2 functionality for any reason.

[ChALkeR]

+1 to removing, but not sure what should the existing flag do — warn or throw?

[MylesBorins]

+1 to having the flag give a warning. While that is technically a semver major I think it is way better than having individuals keep using the flag with no idea

[mhdawson]

I agree that we should not leave the option but have it do nothing. If we just remove it completely the end user would get "bad option:". A warning would allow them to run, but if they are actually using it that could result in a harder to diagnose problem later on. I think I'd lean towards removing completely

[Fishrock123]

@mhdawson has a point. Maybe detect that flag and fail early with a special error?