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

deps: Disable EXPORT and LOW ciphers in openssl for v0.12 #5712

Closed
wants to merge 2 commits into from

Conversation

shigeki
Copy link
Contributor

@shigeki shigeki commented Mar 15, 2016

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Affected core subsystem(s)

tls

Description of change

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. This PR is against v0.12-staging.

According to the deprecation, two tls tests need to be 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

R: @indutny , @rvagg , @mhdawson or @jasnell

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 shigeki added tls Issues and PRs related to the tls subsystem. openssl Issues and PRs related to the OpenSSL dependency. v0.12 labels Mar 15, 2016
@shigeki
Copy link
Contributor Author

shigeki commented Mar 15, 2016

@@ -44,6 +44,9 @@
# ifndef OPENSSL_NO_STORE
# define OPENSSL_NO_STORE
# endif
#ifndef OPENSSL_NO_WEAK_SSL_CIPHERS
# define OPENSSL_NO_WEAK_SSL_CIPHERS
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One space indent.

@bnoordhuis
Copy link
Member

LGTM with style nit.

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

shigeki commented Mar 15, 2016

@bnoordhuis Thanks for reviewing, Ben. I didn't think you are still available on line today. The style was fixed.
I'm waiting for CI on arm for v0.12 back now.

shigeki pushed a commit to shigeki/node that referenced this pull request 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 <info@bnoordhuis.nl>
shigeki pushed a commit to shigeki/node that referenced this pull request 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 <info@bnoordhuis.nl>
@shigeki
Copy link
Contributor Author

shigeki commented Mar 15, 2016

CI ran again to see if linux test failures for ipv6 on https://ci.nodejs.org/job/node-test-commit/2545/ and it was resolved. They are green except some windows flaky tests.

For v0.10, there were conflicts in fixing test/simple/test-tls-honorcipherorder.js.
I'm going to run CI for v0.10 after resolving conflicts.

shigeki pushed a commit that referenced this pull request 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 <info@bnoordhuis.nl>
shigeki pushed a commit that referenced this pull request 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 <info@bnoordhuis.nl>
@shigeki
Copy link
Contributor Author

shigeki commented Mar 15, 2016

Landed in a115779 and 9c06db7 for v0.12-staging.

shigeki pushed a commit that referenced this pull request 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 <info@bnoordhuis.nl>
shigeki pushed a commit that referenced this pull request 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 <info@bnoordhuis.nl>
@shigeki
Copy link
Contributor Author

shigeki commented Mar 15, 2016

CI for v0.10 is done in https://ci.nodejs.org/job/node-test-commit/2548/ . All is green except a few failures in windows-fanned. The fixes on test/simple/test-tls-honorcipherorder.js was changed as to

  • resolve conflicts for some test scenarios in v0.12 does not exist on v0.10
  • ECDHE is not supported on v0.10 yet. ECDHE-RSA-AES256-SHA was replaced into SHA128-AES in the test. I'm not sure why node-v0.10 has ECDHE-RSA-AES128-SHA256 in the default cipher entry.

Landed in 0847954 and 6bb86e7 for v0.10-staging.

@shigeki shigeki closed this Mar 15, 2016
@indutny
Copy link
Member

indutny commented Mar 15, 2016

Belated LGTM

@shigeki
Copy link
Contributor Author

shigeki commented Mar 15, 2016

@indutny Thanks and it's regrettable that I could not write your name in the commit log.

@jasnell
Copy link
Member

jasnell commented Mar 15, 2016

Since these landed in the staging branches, removed the lts-watch labels and added the land-on labels. /cc @thealphanerd

@jasnell jasnell mentioned this pull request Mar 15, 2016
3 tasks
rvagg added a commit that referenced this pull request 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 that referenced this pull request 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 that referenced this pull request 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 that referenced this pull request 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 that referenced this pull request 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 that referenced this pull request 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 pull request 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 <info@bnoordhuis.nl>
jBarz pushed a commit to ibmruntimes/node that referenced this pull request 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 <info@bnoordhuis.nl>
jBarz pushed a commit to ibmruntimes/node that referenced this pull request 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
openssl Issues and PRs related to the OpenSSL dependency. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants