tls: throw an error on getLegacyCipher

doc/api/tls.markdown

@@ -164,6 +164,34 @@ Reverting back to the defaults used by older releases can weaken the security
 of your applications. The legacy cipher suites should only be used if absolutely
 necessary.
 
+NOTE: Due to an error in Node.js v0.10.38, the default cipher list only applied
+to servers using TLS. The default cipher list would _not_ be used by clients.
+This behavior has been changed in v0.10.39 and the default cipher list is now
+used by both the server and client when using TLS. However, when using
+`--enable-legacy-cipher-list=v0.10.38`, Node.js is reverted back to the
+v0.10.38 behavior of only using the default cipher list on the server.
+
+### Cipher List Precedence
+
+Note that the `--enable-legacy-cipher-list`, `NODE_LEGACY_CIPHER_LIST`,
+`--cipher-list` and `NODE_CIPHER_LIST` options are mutually exclusive.
+
+If the `NODE_CIPHER_LIST` and `NODE_LEGACY_CIPHER_LIST` environment variables
+are both specified, the `NODE_LEGACY_CIPHER_LIST` setting will take precedence.
+
+The `--cipher-list` and `--enable-legacy-cipher-list` command line options
+will override the environment variables. If both happen to be specified, the
+right-most (second one specified) will take precedence. For instance, in the
+example:
+
+    node --cipher-list=ABC --enable-legacy-cipher-list=v0.10.38
+
+The v0.10.38 default cipher list will be used.
+
+    node --enable-legacy-cipher-list=v0.10.38 --cipher-list=ABC
+
+The custom cipher list will be used.
+
 ## tls.getCiphers()
 
 Returns an array with the names of the supported SSL ciphers.
@@ -174,6 +202,18 @@ Example:
     console.log(ciphers); // ['AES128-SHA', 'AES256-SHA', ...]
 
 
+## tls.getLegacyCiphers(version)
+
+Returns a default cipher list used in a previous version of Node.js. The
+version parameter must be a string whose value identifies previous Node.js
+release version. The only value currently supported is `v0.10.38`.
+
+A TypeError will be thrown if: (a) the `version` is any type other than a
+string, (b) the `version` parameter is not specified, or (c) additional
+parameters are passed in. An Error will be thrown if the `version` parameter is
+passed in as a string but the value does not correlate to any known Node.js
+release for which a default cipher list is available.
+
 ## tls.createServer(options, [secureConnectionListener])
 
 Creates a new [tls.Server][].  The `connectionListener` argument is

lib/tls.js

@@ -1330,6 +1330,20 @@ function normalizeConnectArgs(listArgs) {
   return (cb) ? [options, cb] : [options];
 }
 
+// return true if the --enable-legacy-cipher-list command line
+// switch, or the NODE_LEGACY_CIPHER_LIST environment variable
+// are set to v0.10.38 and the DEFAULT_CIPHERS equal the v0.10.38
+// list.
+function usingV1038Ciphers() {
+  var argv = process.execArgv;
+  if ((argv.indexOf('--enable-legacy-cipher-list=v0.10.38') > -1 ||
+       process.env['NODE_LEGACY_CIPHER_LIST'] === 'v0.10.38') &&
+      DEFAULT_CIPHERS === _crypto.getLegacyCiphers('v0.10.38')) {
+        return true;
+  }
+  return false;
+}
+
 exports.connect = function(/* [port, host], options, cb */) {
   var args = normalizeConnectArgs(arguments);
   var options = args[0];
@@ -1338,7 +1352,14 @@ exports.connect = function(/* [port, host], options, cb */) {
   var defaults = {
     rejectUnauthorized: '0' !== process.env.NODE_TLS_REJECT_UNAUTHORIZED
   };
-  if (DEFAULT_CIPHERS != _crypto.getLegacyCiphers('v0.10.38')) {
+  if (!usingV1038Ciphers()) {
+    // only set the default ciphers if we are _not_ using the
+    // v0.10.38 legacy cipher list. Node v0.10.38 had a bug
+    // that failed to set the default ciphers on the default
+    // options. This has been fixed in v0.10.39 and above.
+    // However, when the user explicitly tells node to revert
+    // back to using the v0.10.38 cipher list, node should
+    // revert back to the original v0.10.38 behavior.
     defaults.ciphers = DEFAULT_CIPHERS;
   }
   options = util._extend(defaults, options || {});

src/node.cc

@@ -2588,7 +2588,8 @@ static void PrintHelp() {
 // Parse node command line arguments.
 static void ParseArgs(int argc, char **argv) {
   int i;
-  bool using_legacy_cipher_list = false;
+  int cipher_list_options = 0;
+  bool using_cipher_list_option = false;
 
   // TODO use parse opts
   for (i = 1; i < argc; i++) {
@@ -2658,14 +2659,11 @@ static void ParseArgs(int argc, char **argv) {
       argv[i] = const_cast<char*>("");
       throw_deprecation = true;
     } else if (strncmp(arg, "--cipher-list=", 14) == 0) {
-      if (!using_legacy_cipher_list) {
-        DEFAULT_CIPHER_LIST = arg + 14;
-      }
+      DEFAULT_CIPHER_LIST = arg + 14;
       argv[i] = const_cast<char*>("");
     } else if (strncmp(arg, "--enable-legacy-cipher-list=", 28) == 0) {
       const char * legacy_list = legacy_cipher_list(arg+28);
       if (legacy_list != NULL) {
-        using_legacy_cipher_list = true;
         DEFAULT_CIPHER_LIST = legacy_list;
       } else {
         fprintf(stderr, "Error: An unknown legacy cipher list was specified\n");
@@ -2948,6 +2946,26 @@ char** Init(int argc, char *argv[]) {
   // Make inherited handles noninheritable.
   uv_disable_stdio_inheritance();
 
+  // set the cipher list from the environment variable first,
+  // the command line switch will override if specified.
+  const char * cipher_list = getenv("NODE_CIPHER_LIST");
+  if (cipher_list != NULL) {
+    DEFAULT_CIPHER_LIST = cipher_list;
+  }
+
+  // Setting NODE_LEGACY_CIPHER_LIST will override the NODE_CIPHER_LIST
+  const char * leg_cipher_id = getenv("NODE_LEGACY_CIPHER_LIST");
+  if (leg_cipher_id != NULL) {
+    const char * leg_cipher_list =
+      legacy_cipher_list(leg_cipher_id);
+    if (leg_cipher_list != NULL) {
+      DEFAULT_CIPHER_LIST = leg_cipher_list;
+    } else {
+      fprintf(stderr, "Error: An unknown legacy cipher list was specified\n");
+      exit(9);
+    }
+  }
+
   // Parse a few arguments which are specific to Node.
   node::ParseArgs(argc, argv);
   // Parse the rest of the args (up to the 'option_end_index' (where '--' was
@@ -2966,25 +2984,6 @@ char** Init(int argc, char *argv[]) {
     v8argv[option_end_index + 1] = const_cast<char*>("v8debug");
   }
 
-  const char * cipher_list = getenv("NODE_CIPHER_LIST");
-  if (cipher_list != NULL) {
-    DEFAULT_CIPHER_LIST = cipher_list;
-  }
-  // Allow the NODE_LEGACY_CIPHER_LIST envar to override the other
-  // cipher list options. NODE_LEGACY_CIPHER_LIST=v0.10.38 will use
-  // the cipher list from v0.10.38
-  const char * leg_cipher_id = getenv("NODE_LEGACY_CIPHER_LIST");
-  if (leg_cipher_id != NULL) {
-    const char * leg_cipher_list =
-      legacy_cipher_list(leg_cipher_id);
-    if (leg_cipher_list != NULL) {
-      DEFAULT_CIPHER_LIST = leg_cipher_list;
-    } else {
-      fprintf(stderr, "Error: An unknown legacy cipher list was specified\n");
-      exit(9);
-    }
-  }
-
   // For the normal stack which moves from high to low addresses when frames
   // are pushed, we can compute the limit as stack_size bytes below the
   // the address of a stack variable (e.g. &stack_var) as an approximation

src/node_crypto.cc

@@ -4199,12 +4199,20 @@ const char* ToCString(const node::Utf8Value& value) {
 
 Handle<Value> DefaultCiphers(const Arguments& args) {
   HandleScope scope;
+  unsigned int len = args.Length();
+  if (len != 1 || !args[0]->IsString()) {
+    return ThrowException(
+      Exception::TypeError(
+        String::New("A single string parameter is required")));
+  }
   node::Utf8Value key(args[0]);
   const char * list = legacy_cipher_list(ToCString(key));
-  if (list == NULL) {
-    list = DEFAULT_CIPHER_LIST_HEAD;
+  if (list != NULL) {
+    return scope.Close(v8::String::New(list));
+  } else {
+    return ThrowException(Exception::Error(String::New(
+      "Unknown legacy cipher list")));
   }
-  return scope.Close(v8::String::New(list));
 }
 
 Handle<Value> GetCiphers(const Arguments& args) {