Disable EXPORT and LOW SSLv3+ ciphers by default for Argon, v0.12 and v0.10

[shigeki]

OpenSSL-1.0.1s and 1.0.2g disables EXPORT and LOW ciphers in openssl/openssl@abd5d8f .

We missed to add OPENSSL_NO_WEAK_SSL_CIPHERS at the last upgrading so I would like to add it to config. They are weak ciphers so that they are not included in the default cipher list.

I would like to add this without adding new command line flag even it leads some incpompatibilies because using them is too unsafe like SSLv2.

Ref: nodejs/node#5630

[mhdawson]

Ok so if you build by default openssl does not define OPENSSL_NO_WEAK_SSL_CIPHERS or is it that because we have a pre-configured build files we did not define OPENSSL_NO_WEAK_SSL_CIPHERS even though a default build of openssl does ?

[shigeki]

These weak ciphers are disabled by default in Configure as https://github.com/openssl/openssl/blob/OpenSSL_1_0_2-stable/Configure#L794 so that OPENSSL_NO_WEAK_SSL_CIPHERS is defined in opensslconf.h for all platforms.
They can only be enabled by adding a configure option explicitly with Configure enable-weak-ssl-ciphers.

At the last openssl upgrading of openssl in Node, we missed to add the new define to deps/openssl/config. nodejs/node#5630 is the fix against master. This issue is a discussion for LTS.

[jasnell]

@shigeki .. which LTS versions does this apply to?

[shigeki]

@jasnell For v4.x (Argon), v0.12 and v0.10. This change is included in both openssl-1.0.2 and 1.0.1.

[jasnell]

I'm OK with this but technically I think we'd have to do a minor bump in v4.

[mhdawson]

@shigeki - So that its 100% clear my understanding is:

1. If you compile openssl, by default the weeks algorithms are disabled.
2. Currently due to the way we build, they are NOT disabled in the LTS releases

If that's the case then I'd agree we want to disable them as I think we already believed we had.

[shigeki]

@mhdawson Yes for both 1) and 2). I open this issue because it affects users who are using weak ciphers(LOW and EXPORT) with explicitly defining their cipher suites. Default cipher in Node does not include them.

[rvagg]

@jasnell I reckon we could do this as semver-patch as a "bugfix" for what should have been done in 4.3. It should get in asap.

[rvagg]

@shigeki the justification for enabling LOW and EXPORT is weaker than the justification for enabling SSLv2 right? There's no good reason to be using them at all at this stage is there? I'd be happy with a PR against 0.10-staging and 0.12-staging for inclusion in the next releases there.

[shigeki]

Here is the list of ciphers to be disabled by OPENSSL_NO_WEAK_SSL_CIPHERS. 27 ciphers are listed in total, which can be used in SSLv3 and higher.

Each of them can be classified to

• EXPORT (18 ciphers): they are used in old 90's and not safe. They are subject to be attacked by FREAK.
• LOW (6 ciphers): they use 56bit DES and it was already cracked. Currently 3DES can only be recommended to be used for legacy use. DES cipher was already deprecated in NIST standard.
• Not Implemented (3 ciphers):

I think that they have no longer reasons to be supported for current use. I will submit a PR for v0.12 soon.

no	cipher name to be disabled	strength
1	RSA_RC4_40_MD5	EXPORT
2	RSA_RC2_40_MD5	EXPORT
3	RSA_DES_40_CBC_SHA	EXPORT
4	RSA_DES_64_CBC_SHA	LOW
5	DH_DSS_DES_40_CBC_SHA	EXPORT
6	DH_DSS_DES_64_CBC_SHA	Not Implemented
7	DH_RSA_DES_40_CBC_SHA	Not Implemented
8	DH_RSA_DES_64_CBC_SHA	Not Implemented
9	EDH_DSS_DES_40_CBC_SHA	EXPORT
10	EDH_DSS_DES_64_CBC_SHA	LOW
11	EDH_RSA_DES_40_CBC_SHA	EXPORT
12	EDH_RSA_DES_64_CBC_SHA	LOW
13	ADH_RC4_40_MD5	EXPORT
14	ADH_DES_40_CBC_SHA	EXPORT
15	ADH_DES_64_CBC_SHA	LOW
16	KRB5_DES_64_CBC_SHA	LOW
17	KRB5_DES_64_CBC_MD5	LOW
18	KRB5_DES_40_CBC_SHA	EXPORT
19	KRB5_RC2_40_CBC_SHA	EXPORT
20	KRB5_RC4_40_SHA	EXPORT
21	KRB5_DES_40_CBC_MD5	EXPORT
22	KRB5_RC2_40_CBC_MD5	EXPORT
23	KRB5_RC4_40_MD5	EXPORT
24	RSA_EXPORT1024_WITH_DES_CBC_SHA	EXPORT
25	DHE_DSS_EXPORT1024_WITH_DES_CBC_SHA	EXPORT
26	RSA_EXPORT1024_WITH_RC4_56_SHA	EXPORT
27	DHE_DSS_EXPORT1024_WITH_RC4_56_SHA	EXPORT

[rvagg]

Yeah, I agree, unless @nodejs/crypto or @nodejs/lts have objections I'd like us to move forward with removing these from 0.10 and 0.12 and v4. As I said above, I'm happy for them to be considered a bugfix on top of the 1.0.1s ad 1.0.2g upgrades where they should have been disabled, negating the need for semver-minor in v4. Even with my normally conservative approach to LTS changes, I find it very difficult to imagine that any of the above ciphers listed by @shigeki have any reason for being enabled in a Node stack, particularly a v4 one where presumably the user is at least putting in some effort to be "modern".

[bnoordhuis]

I'm fine with disabling them in a patch release. Realistically, no one in their right mind is using those ciphers.

[indutny]

+1 from me.