Float patch for OpenSSL low-severity TLS handshake using a DH(E) DoS (?)

[rvagg]

As per https://mta.openssl.org/pipermail/openssl-announce/2018-June/000127.html:

OpenSSL Security Advisory [12 June 2018]
========================================

Client DoS due to large DH parameter (CVE-2018-0732)
====================================================

Severity: Low

During key agreement in a TLS handshake using a DH(E) based ciphersuite a
malicious server can send a very large prime value to the client. This will
cause the client to spend an unreasonably long period of time generating a key
for this prime resulting in a hang until the client has finished. This could be
exploited in a Denial Of Service attack.

Due to the low severity of this issue we are not issuing a new release of
OpenSSL 1.1.0 or 1.0.2 at this time. The fix will be included in OpenSSL 1.1.0i
and OpenSSL 1.0.2p when they become available. The fix is also available in
commit ea7abeeab (for 1.1.0) and commit 3984ef0b7 (for 1.0.2) in the OpenSSL git
repository.

This PR is ea7abeeab and would apply to master and v10.x. 3984ef0b7 is basically identical and would apply to v6.x and v8.x.

So the question is whether we want to bother backporting this. I don't have a strong opinion either way but if we do include it I don't think it needs to go out in any special security releases, just along with other releases (if they happen before the next OpenSSLs are released). I don't know why this is labelled "low" and why it's not embargoed (suggesting something more like "very low")—maybe simply because it's the server disrupting clients and that's more difficult to cause widespread problems than the other way around?

/cc @nodejs/security, @nodejs/tsc, @nodejs/release, @nodejs/crypto - sorry for the broad pings but opinions are needed!

---

Pending OpenSSL 1.1.0i release.

Original message:

Reject excessively large primes in DH key generation.

CVE-2018-0732

Signed-off-by: Guido Vranken <[email protected]>

(cherry picked from commit 91f7361f47b082ae61ffe1a7b17bb2adf213c7fe)

Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from https://github.com/openssl/openssl/pull/6457)

[nodejs-github-bot]

@rvagg sadly an error occured when I tried to trigger a build :(

[rvagg]

FYI @addaleax raised a good question in the TSC meeting re whether this would be a breaking change in any way. I don't have a good answer to that but I believe it wouldn't be. It introduces a new error path (returning an OpenSSL DH_R_MODULUS_TOO_LARGE) but I believe that's only in a case that is likely unusable anyway (i.e. the hang / DoS path). If someone else from @nodejs/crypto could weigh in that would be helpful.

[bnoordhuis]

This limits the key to 10,000 bits. That's so unreasonably large that it's pretty much impossible to break any real-world deployment.

[tniessen]

I agree with @bnoordhuis.

[shigeki]

Current statics as of June 03, 2018 shows that most of DH Key Exchange Strength are less than or equal to 3k.
https://www.ssllabs.com/ssl-pulse/#chart-key-exchange

I think this does not break any compatibilities in real world.

[shigeki]

It seems we might have an another floating patch as openssl/openssl@0c27d79.

Details are https://www.nccgroup.trust/us/our-research/technical-advisory-return-of-the-hidden-number-problem/.

[addaleax]

CI: https://ci.nodejs.org/job/node-test-pull-request/15454/

[bnoordhuis]

It seems we might have an another floating patch as openssl/openssl@0c27d79.

I'd be surprised if you could observe that in Node.js (too many moving parts; maybe with really big keys) but it shouldn't hurt to float it. Better safe than sorry.

[rvagg]

OK, I'm going to go ahead and land this. Can I just push backport commits to staging branches of other active lines these days or do they each need their own PRs?

As long as we're dong this, I agree with also doing the next one that @shigeki pointed out, CVE-2018-0495. The details are pretty amazing in terms of the hoops you'd have to jump through to exploit it so I get why that's not embargoed either (plus it impacts lots of libraries so there's too much coordination!).