tls: make server not use DHE in less than 1024 bits

[jasnell]

@nodejs/lts ... See: tls: make server not use DHE in less than 1024bits ... from what I understand, this would technically be a semver-major.

@shigeki @mhdawson ... which versions are affected by this? is it only v0.12.x? or v4.2.x also?

[shigeki]

nodejs/node#1831 was only applied to 5.0 so that this affects all (4.2.x, 0.12, 0.10).

[shigeki]

Correction. nodejs/node#1831 was client side:

Server side is affected in 0.12 and 0.10 [EDIT: tls server of v0.10 does not support DHE]
Client side is affected in 4.2.x, 0.12 and 010.

I think we have two options for LTS. Any idea?

• Limit DH key size with code but disabled by default and introduce a new command line option.
• Leave it.

[rvagg]

Another option is to force it through as semver-minor or semver-patch, we did say that we may use that as an option.

In this case I think I'm in favour of just leaving it as is. In the majority of situations we should end up with secure enough defaults when negotiating with the majority of the clients that are in use right now (I think that's correct anyway).

[mhdawson]

@shigeki I think the earlier discussion was that it does not apply to 0.10.X because it did not support DHE so it would only be 0.12.X and 4.X. Did I misunderstand ?

As one data point the IBM security teams believe we must make it secure by default which includes this change on the server side. To make is secure by default it would need to be on by default but with an option to turn it off.

[shigeki]

@mhdawson I mistook that a TLS server of 0.10 is affected but the tls client of 0.10 can support DHE as below. I fixed the list above. Thanks.

$ ./node ~/tls_client.js
process.version: v0.10.38
clearText.getCipher(): { name: 'DHE-RSA-AES128-SHA256', version: 'TLSv1/SSLv3' }

As one data point the IBM security teams believe we must make it secure by default which includes this change on the server side.

That's glad to me if we can make security hardening to change default behaviors even in LTS and we must notify users that a new option will be deprecated 5.x.

[mhdawson]

I think the client side was covered by updating the openssl version which we did for 0.10.X and 0.12.X, I don't think we need to do anything else for this on the client side.

So I think for LTS we only need to address the server side. The change is already in 4.X and 0.10.X is not affected so that just leaves 0.12.X. I'll put together a PR to backport to 0.12.X with a command line option to revert

[mhdawson]

From this https://www.openssl.org/blog/blog/2015/05/20/logjam-freak-upcoming-changes/ the change on the client side in openssl was to disallow DH parameters smaller than 768 so I believe its appropriate to have the same limit on the server side for the 0.12.X release as opposed to 1024 used in later releases. (1024 is fine for later releases but for 0.12.X we need to limit the potential impact to users. Given we match the same limit, and current clients don't accept anything smaller than 768 then unless all of the clients accessing an application are old, a key of 768 on the server side should already have been found/fixed)

[mhdawson]

Created a PR to cover this for 0.12.X nodejs/node#3890

[mhdawson]

@jasnell @shigeki can you both chime in on whether we can get this into an upcoming 0.12.X LTS release.

[ChALkeR]

from what I understand, this would technically be a semver-major.

It prevents a security misconfiguration and that is the only thing that it breaks. That should be enough reasons for making it not a semver-major.

[mhdawson]

I believe this is done I think it should be closed.