Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
shigeki opened this issue Mar 9, 2016 · 16 comments
Closed

Comments

@shigeki
Copy link

shigeki commented Mar 9, 2016

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
Copy link
Member

mhdawson commented Mar 9, 2016

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
Copy link
Author

shigeki commented Mar 10, 2016

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
Copy link
Member

jasnell commented Mar 11, 2016

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

@shigeki
Copy link
Author

shigeki commented Mar 11, 2016

@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
Copy link
Member

jasnell commented Mar 11, 2016

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

@mhdawson
Copy link
Member

@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
Copy link
Author

shigeki commented Mar 12, 2016

@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
Copy link
Member

rvagg commented Mar 14, 2016

@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
Copy link
Member

rvagg commented Mar 14, 2016

@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
Copy link
Author

shigeki commented Mar 14, 2016

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
Copy link
Member

rvagg commented Mar 14, 2016

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
Copy link
Member

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

@indutny
Copy link
Member

indutny commented Mar 14, 2016

+1 from me.

@mhdawson
Copy link
Member

+1 from me

@jasnell
Copy link
Member

jasnell commented Mar 14, 2016

+1. SGTM
On Mar 14, 2016 6:03 AM, "Fedor Indutny" [email protected] wrote:

+1 from me.


Reply to this email directly or view it on GitHub
#85 (comment).

shigeki pushed a commit to shigeki/node that referenced this issue Mar 15, 2016
openssl-1.0.1s disables EXPORT and LOW ciphers by default.
They are obsoleted ciphers and not safe for the current use.
Node LTS also deprecates them.

Fixes: nodejs/Release#85
shigeki pushed a commit to shigeki/node that referenced this issue Mar 15, 2016
openssl-1.0.1s disables EXPORT and LOW ciphers by default.
They are obsoleted ciphers and not safe for the current use.
Node LTS also deprecates them.

Fixes: nodejs/Release#85
PR-URL: nodejs#5712
Reviewed-By: Ben Noordhuis <[email protected]>
shigeki pushed a commit to shigeki/node that referenced this issue Mar 15, 2016
DES-CBC-SHA is LOW cipher and disabled by default and it is used in
tests of hornorcipherorder. They are changed as to

- use RC4-SHA instead of DES-CBC-SHA.
- add ECDHE-RSA-AES256-SHA to entries to keep the number of ciphers.
- remove tests for non-default cipher because only SEED and IDEA are
available in !RC4:!HIGH:ALL.

Fixes: nodejs/Release#85
PR-URL: nodejs#5712
Reviewed-By: Ben Noordhuis <[email protected]>
shigeki pushed a commit to nodejs/node that referenced this issue Mar 15, 2016
openssl-1.0.1s disables EXPORT and LOW ciphers by default.
They are obsoleted ciphers and not safe for the current use.
Node LTS also deprecates them.

Fixes: nodejs/Release#85
PR-URL: #5712
Reviewed-By: Ben Noordhuis <[email protected]>
shigeki pushed a commit to nodejs/node that referenced this issue Mar 15, 2016
DES-CBC-SHA is LOW cipher and disabled by default and it is used in
tests of hornorcipherorder. They are changed as to

- use RC4-SHA instead of DES-CBC-SHA.
- add ECDHE-RSA-AES256-SHA to entries to keep the number of ciphers.
- remove tests for non-default cipher because only SEED and IDEA are
available in !RC4:!HIGH:ALL.

Fixes: nodejs/Release#85
PR-URL: #5712
Reviewed-By: Ben Noordhuis <[email protected]>
shigeki pushed a commit to nodejs/node that referenced this issue Mar 15, 2016
openssl-1.0.1s disables EXPORT and LOW ciphers by default.
They are obsoleted ciphers and not safe for the current use.
Node LTS also deprecates them.

Fixes: nodejs/Release#85
PR-URL: #5712
Reviewed-By: Ben Noordhuis <[email protected]>
shigeki pushed a commit to nodejs/node that referenced this issue Mar 15, 2016
DES-CBC-SHA is LOW cipher and disabled by default and it is used in
tests of hornorcipherorder. They are changed as to

- use RC4-SHA instead of DES-CBC-SHA.
- add AES128-SHA to entries to keep the number of ciphers.
- remove tests for non-default cipher because only SEED and IDEA are
available in !RC4:!HIGH:ALL.

Fixes: nodejs/Release#85
PR-URL: #5712
Reviewed-By: Ben Noordhuis <[email protected]>
@shigeki
Copy link
Author

shigeki commented Mar 15, 2016

Thanks for all your agreements. The fixes were landed in v0.12-staging and v0.10-staging.

@shigeki shigeki closed this as completed Mar 15, 2016
shigeki pushed a commit to nodejs/node that referenced this issue Mar 15, 2016
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

Fixes: nodejs/Release#85
PR-URL: #5630
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
evanlucas pushed a commit to nodejs/node that referenced this issue Mar 15, 2016
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

Fixes: nodejs/Release#85
PR-URL: #5630
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
rvagg pushed a commit to nodejs/node that referenced this issue Mar 16, 2016
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

Fixes: nodejs/Release#85
PR-URL: #5630
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
MylesBorins pushed a commit to nodejs/node that referenced this issue Mar 21, 2016
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

Fixes: nodejs/Release#85
PR-URL: #5630
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
MylesBorins pushed a commit to nodejs/node that referenced this issue Mar 21, 2016
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in
opensslconf.h

Fixes: nodejs/Release#85
PR-URL: #5630
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
rvagg added a commit to nodejs/node that referenced this issue Mar 31, 2016
Notable changes:

* npm: Upgrade to v2.15.1. (Forrest L Norvell)
* openssl: OpenSSL v1.0.1s disables the EXPORT and LOW ciphers as they
  are obsolete and not considered safe. This release of Node.js turns
  on `OPENSSL_NO_WEAK_SSL_CIPHERS` to fully disable the 27 ciphers
  included in these lists which can be used in SSLv3 and higher. Full
  details can be found in our LTS discussion on the matter
  (nodejs/Release#85).
  (Shigeki Ohtsu) #5712
rvagg added a commit to nodejs/node that referenced this issue Mar 31, 2016
Notable changes:

* npm: Upgrade to v2.15.1. IMPORTANT: This is a major upgrade to npm
  v2 LTS from the previously deprecated npm v1. (Forrest L Norvell)
* openssl: OpenSSL v1.0.1s disables the EXPORT and LOW ciphers as they
  are obsolete and not considered safe. This release of Node.js turns
  on `OPENSSL_NO_WEAK_SSL_CIPHERS` to fully disable the 27 ciphers
  included in these lists which can be used in SSLv3 and higher. Full
  details can be found in our LTS discussion on the matter
  (nodejs/Release#85).
  (Shigeki Ohtsu) #5712
rvagg added a commit to nodejs/node that referenced this issue Mar 31, 2016
Notable changes:

* npm: Upgrade to v2.15.1. Fixes a security flaw in the use of
  authentication tokens in HTTP requests that would allow an attacker
  to set up a server that could collect tokens from users of the
  command-line interface. Authentication tokens have previously been
  sent with every request made by the CLI for logged-in users,
  regardless of the destination of the request. This update fixes this
  by only including those tokens for requests made against the
  registry or registries used for the current install.
  (Forrest L Norvell) #5967
* openssl: OpenSSL v1.0.1s disables the EXPORT and LOW ciphers as they
  are obsolete and not considered safe. This release of Node.js turns
  on `OPENSSL_NO_WEAK_SSL_CIPHERS` to fully disable the 27 ciphers
  included in these lists which can be used in SSLv3 and higher. Full
  details can be found in our LTS discussion on the matter
  (nodejs/Release#85).
  (Shigeki Ohtsu) #5712

PR-URL: #5967
rvagg added a commit to nodejs/node that referenced this issue Mar 31, 2016
Notable changes:

* npm: Upgrade to v2.15.1. IMPORTANT: This is a major upgrade to npm
  v2 LTS from the previously deprecated npm v1. (Forrest L Norvell)
* npm: Upgrade to v2.15.1. Fixes a security flaw in the use of
  authentication tokens in HTTP requests that would allow an attacker
  to set up a server that could collect tokens from users of the
  command-line interface. Authentication tokens have previously been
  sent with every request made by the CLI for logged-in users,
  regardless of the destination of the request. This update fixes this
  by only including those tokens for requests made against the
  registry or registries used for the current install. IMPORTANT:
  This is a major upgrade to npm v2 LTS from the previously deprecated
  npm v1. (Forrest L Norvell) #5967
* openssl: OpenSSL v1.0.1s disables the EXPORT and LOW ciphers as they
  are obsolete and not considered safe. This release of Node.js turns
  on `OPENSSL_NO_WEAK_SSL_CIPHERS` to fully disable the 27 ciphers
  included in these lists which can be used in SSLv3 and higher. Full
  details can be found in our LTS discussion on the matter
  (nodejs/Release#85).
  (Shigeki Ohtsu) #5712

PR-URL: #5968
rvagg added a commit to nodejs/node that referenced this issue Apr 1, 2016
Notable changes:

* npm: Upgrade to v2.15.1. Fixes a security flaw in the use of
  authentication tokens in HTTP requests that would allow an attacker
  to set up a server that could collect tokens from users of the
  command-line interface. Authentication tokens have previously been
  sent with every request made by the CLI for logged-in users,
  regardless of the destination of the request. This update fixes this
  by only including those tokens for requests made against the
  registry or registries used for the current install.
  (Forrest L Norvell) #5967
* openssl: OpenSSL v1.0.1s disables the EXPORT and LOW ciphers as they
  are obsolete and not considered safe. This release of Node.js turns
  on `OPENSSL_NO_WEAK_SSL_CIPHERS` to fully disable the 27 ciphers
  included in these lists which can be used in SSLv3 and higher. Full
  details can be found in our LTS discussion on the matter
  (nodejs/Release#85).
  (Shigeki Ohtsu) #5712

PR-URL: #5967
rvagg added a commit to nodejs/node that referenced this issue Apr 1, 2016
Notable changes:

* npm: Upgrade to v2.15.1. IMPORTANT: This is a major upgrade to npm
  v2 LTS from the previously deprecated npm v1. (Forrest L Norvell)
* npm: Upgrade to v2.15.1. Fixes a security flaw in the use of
  authentication tokens in HTTP requests that would allow an attacker
  to set up a server that could collect tokens from users of the
  command-line interface. Authentication tokens have previously been
  sent with every request made by the CLI for logged-in users,
  regardless of the destination of the request. This update fixes this
  by only including those tokens for requests made against the
  registry or registries used for the current install. IMPORTANT:
  This is a major upgrade to npm v2 LTS from the previously deprecated
  npm v1. (Forrest L Norvell) #5967
* openssl: OpenSSL v1.0.1s disables the EXPORT and LOW ciphers as they
  are obsolete and not considered safe. This release of Node.js turns
  on `OPENSSL_NO_WEAK_SSL_CIPHERS` to fully disable the 27 ciphers
  included in these lists which can be used in SSLv3 and higher. Full
  details can be found in our LTS discussion on the matter
  (nodejs/Release#85).
  (Shigeki Ohtsu) #5712

PR-URL: #5968
jBarz pushed a commit to ibmruntimes/node that referenced this issue Nov 4, 2016
openssl-1.0.1s disables EXPORT and LOW ciphers by default.
They are obsoleted ciphers and not safe for the current use.
Node LTS also deprecates them.

Fixes: nodejs/Release#85
PR-URL: nodejs/node#5712
Reviewed-By: Ben Noordhuis <[email protected]>
jBarz pushed a commit to ibmruntimes/node that referenced this issue Nov 4, 2016
DES-CBC-SHA is LOW cipher and disabled by default and it is used in
tests of hornorcipherorder. They are changed as to

- use RC4-SHA instead of DES-CBC-SHA.
- add ECDHE-RSA-AES256-SHA to entries to keep the number of ciphers.
- remove tests for non-default cipher because only SEED and IDEA are
available in !RC4:!HIGH:ALL.

Fixes: nodejs/Release#85
PR-URL: nodejs/node#5712
Reviewed-By: Ben Noordhuis <[email protected]>
jBarz pushed a commit to ibmruntimes/node that referenced this issue Nov 4, 2016
Notable changes:

* npm: Upgrade to v2.15.1. Fixes a security flaw in the use of
  authentication tokens in HTTP requests that would allow an attacker
  to set up a server that could collect tokens from users of the
  command-line interface. Authentication tokens have previously been
  sent with every request made by the CLI for logged-in users,
  regardless of the destination of the request. This update fixes this
  by only including those tokens for requests made against the
  registry or registries used for the current install.
  (Forrest L Norvell) nodejs/node#5967
* openssl: OpenSSL v1.0.1s disables the EXPORT and LOW ciphers as they
  are obsolete and not considered safe. This release of Node.js turns
  on `OPENSSL_NO_WEAK_SSL_CIPHERS` to fully disable the 27 ciphers
  included in these lists which can be used in SSLv3 and higher. Full
  details can be found in our LTS discussion on the matter
  (nodejs/Release#85).
  (Shigeki Ohtsu) nodejs/node#5712

PR-URL: nodejs/node#5967
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants