tls: disable RC4, add --cipher-list command line switch #14413

jasnell · 2015-04-02T23:29:16Z

Disable RC4 in the default cipher list

Add the --cipher-list command line switch and NODE_CIPHER_LIST
environment variable to completely override the default cipher list.

Add the --enable-legacy-cipher-list and NODE_LEGACY_CIPHER_LIST
environment variable to selectively enable the default cipher list from
previous node.js releases.

(Targets v0.10.38-release)

Disable RC4 in the default cipher list Add the `--cipher-list` command line switch and `NODE_CIPHER_LIST` environment variable to completely override the default cipher list. Add the `--enable-legacy-cipher-list` and `NODE_LEGACY_CIPHER_LIST` environment variable to selectively enable the default cipher list from previous node.js releases.

mhdawson · 2015-04-06T15:55:38Z

I had reviewed the 0.10.X version of the patch with James and we have run it through our internal builds. FV passed without any regressions, we are starting SVT now.

lgtm

jasnell · 2015-04-06T16:24:28Z

Ok. So far the only nit appears to be making the PrintHelp output less
verbose. I can make that change then will work with Julien on getting this
landed.

On Mon, Apr 6, 2015 at 8:55 AM, Michael Dawson notifications@github.com
wrote:

I had reviewed the 0.10.X version of the patch with James and we have run
it through our internal builds. FV passed without any regressions, we are
starting SVT now.

lgtm

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

Per feedback on the commit, make the PrintHelp for --enable-legacy-cipher-list less verbose.

misterdjules · 2015-04-08T00:21:28Z

src/node.cc

@@ -2652,6 +2657,21 @@ static void ParseArgs(int argc, char **argv) {
    } else if (strcmp(arg, "--throw-deprecation") == 0) {
      argv[i] = const_cast<char*>("");
      throw_deprecation = true;
+    } else if (strncmp(arg, "--cipher-list=", 14) == 0) {


I'm wondering if we need to support a --cipher-list command line option in v0.10.x. Adding support for --enable-legacy-cipher-list already breaks the contract of not adding additional API in patch releases, and it seems sufficient to handle the use case of users not wanting to use the new default ciphers list.

Adding --cipher-list here is intended to make it so we don't have to make this kind of change in v0.10.x and v0.12.x again. If another one of the ciphers comes into question, we can tell users to simply pass in the new cipher list rather than going in and changing it.

misterdjules · 2015-04-08T00:23:16Z

@jasnell Added one additional comment specific to v0.10.x and this PR, and my comments in #14383 also apply here.

Thank you for doing all this work @jasnell 👍

Disable RC4 in the default cipher list Add the `--cipher-list` command line switch and `NODE_CIPHER_LIST` environment variable to completely override the default cipher list. Add the `--enable-legacy-cipher-list` and `NODE_LEGACY_CIPHER_LIST` environment variable to selectively enable the default cipher list from previous node.js releases. Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #14413

Per feedback on the commit, make the PrintHelp for --enable-legacy-cipher-list less verbose. Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #14413

Disable RC4 in the default cipher list Add the `--cipher-list` command line switch and `NODE_CIPHER_LIST` environment variable to completely override the default cipher list. Add the `--enable-legacy-cipher-list` and `NODE_LEGACY_CIPHER_LIST` environment variable to selectively enable the default cipher list from previous node.js releases. Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #14413

Per feedback on the commit, make the PrintHelp for --enable-legacy-cipher-list less verbose. Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #14413

jasnell · 2015-04-08T20:34:30Z

Landed in #67d9a56 and #02a549e

This change fixes the problem where the cipher list that was setup for a given test was never passed to the client, triggering some false positives with the recent changes made to the ciphers list used by default (see nodejs#14413).

Disable RC4 in the default cipher list Add the `--cipher-list` command line switch and `NODE_CIPHER_LIST` environment variable to completely override the default cipher list. Add the `--enable-legacy-cipher-list` and `NODE_LEGACY_CIPHER_LIST` environment variable to selectively enable the default cipher list from previous node.js releases. Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: nodejs#14413