cryto: don't set default ciphers if client uses 10.38 legacy cipher

This change makes crypto.createCredentials not set default ciphers if
options.ciphers is undefined and --enable-legacy-cipher-list=v0.10.38 is
passed on the command line.

It doesn't do anything for tls.Server, since tls.Server instances always
set the default ciphers list if none is passed explicitly.

For tls.connect, it means that if no ciphers is explicitly passed and
--enable-legacy-cipher-list=v0.10.38 is passed on the command line, no
default cipher list will be set. This is used to preserve the buggy
behavior of node <= v0.10.38 and not break existing applications.

With this change, tls.createSecurePair also doesn't set any default
cipher if --enable-legacy-cipher-list=v0.10.38 is passed on the command
line, as well as tls.Server.addContext if no credentials argument is
passed.

The change also updates test/external/ssl-options/test.js by adding
appropriate tests (see testRC4LegacyCiphers and testSSLv2Setups).

lib/crypto.js

@@ -41,6 +41,7 @@ var constants = process.binding('constants');
 
 var stream = require('stream');
 var util = require('util');
+var tls = require('tls');
 
 // This is here because many functions accepted binary strings without
 // any explicit encoding in older versions of node, and we don't want
@@ -114,7 +115,6 @@ function Credentials(secureProtocol, flags, context) {
 
 exports.Credentials = Credentials;
 
-
 exports.createCredentials = function(options, context) {
   if (!options) options = {};
 
@@ -136,7 +136,14 @@ exports.createCredentials = function(options, context) {
 
   if (options.ciphers) {
     c.context.setCiphers(options.ciphers);
-  } else {
+  } else if (!(tls.usingV1038Ciphers() && options.ciphers === undefined)) {
+    // Set the ciphers to the default ciphers list unless
+    // --enable-legacy-cipher-list=v0.10.38 was passed on the command line and
+    // no ciphers value was passed explicitly. In that case, we want to
+    // preserve the previous buggy behavior that existed in v0.10.x until
+    // v0.10.39, otherwise, a lot of client code might be broken. Server
+    // side TLS/HTTPS code always sets a default cipher list explicitly so it
+    // never reaches this, even for versions < v0.10.39.
     c.context.setCiphers(binding.DEFAULT_CIPHER_LIST);
   }
 

lib/tls.js

@@ -1330,7 +1330,7 @@ function normalizeConnectArgs(listArgs) {
   return (cb) ? [options, cb] : [options];
 }
 
-// return true if the --enable-legacy-cipher-list command line
+// Returns 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.
@@ -1343,6 +1343,7 @@ function usingV1038Ciphers() {
   }
   return false;
 }
+exports.usingV1038Ciphers = usingV1038Ciphers;
 
 exports.connect = function(/* [port, host], options, cb */) {
   var args = normalizeConnectArgs(arguments);

test/external/ssl-options/test.js

@@ -162,6 +162,8 @@ function secureProtocolCompatibleWithSecureOptions(secureProtocol, secureOptions
   return true;
 }
 
+// Returns true if the server and client setups passed as parameters
+// would lead to a successful connection, false otherwise.
 function testSSLv2Setups(serverSetup, clientSetup) {
   // SSLv2 has to be explicitly specified on both sides to work
   if (isSsl2Protocol(serverSetup.secureProtocol) &&
@@ -174,17 +176,36 @@ function testSSLv2Setups(serverSetup, clientSetup) {
 
   var ssl2UsedOnBothSides = isSsl2Protocol(serverSetup.secureProtocol) &&
                             isSsl2Protocol(clientSetup.secureProtocol);
-  if (ssl2UsedOnBothSides &&
-      ((serverSetup.ciphers !== RC4_MD5_CIPHER) ||
-      (clientSetup.ciphers !== RC4_MD5_CIPHER))) {
-    /*
-     * Default ciphers are not compatible with SSLv2. Both client *and*
-     * server need to specify a SSLv2 compatible cipher to be able to use
-     * SSLv2.
-     */
+  if (ssl2UsedOnBothSides) {
+    // Even when SSLv2 is specified on both sides, a SSLv2 compatible cipher
+    // has to be used on both sides too.
+
+    // This is the case if for instance RC4-MD5 is passed explicitly
+    // as a cipher option
+    if (serverSetup.ciphers === RC4_MD5_CIPHER &&
+        clientSetup.ciphers === RC4_MD5_CIPHER)
+      return true;
+
+    // It is also the case if the server passes explicitly RC4-MD%
+    // but the client doesn't pass any cipher and passes
+    // --enable-legacy-cipher-list=v0.10.38 on the command line. This basically
+    // keeps the buggy be behavior of clients not using the default ciphers
+    // list when not explicitly passing any cipher, and as a result
+    // allowing RC4 and MD5 to be used.
+    if (serverSetup.ciphers === RC4_MD5_CIPHER &&
+        clientSetup.ciphers === undefined &&
+        clientSetup.cmdLine === '--enable-legacy-cipher-list=v0.10.38')
+      return true;
+
+    // In all other cases, when using SSLv2 on both sides,
+    // the connection should fail.
     return false;
   }
 
+  // When the client nor the server is using SSlv2, by default the connection
+  // should succeed.
+  // Other test functions looking for other incompatibilites between SSL/TLS
+  // options will take care of testing them.
   return true;
 }
 
@@ -193,6 +214,9 @@ function usesDefaultCiphers(setup) {
   return setup.ciphers == null;
 }
 
+
+// Returns true if the server and client setups passed as parameters
+// would lead to a successful connection, false otherwise.
 function testRC4LegacyCiphers(serverSetup, clientSetup) {
 
   // To be able to use a RC4 cipher suite, either both ends specify it (like
@@ -203,22 +227,31 @@ function testRC4LegacyCiphers(serverSetup, clientSetup) {
   // default and not RC4, so we know that we're only testing disabling/enabling
   // RC4.
 
-  if (serverSetup.ciphers === RC4_SHA_CIPHER) {
-    if (clientSetup.ciphers !== RC4_SHA_CIPHER &&
-        (!usesDefaultCiphers(clientSetup) ||
-         clientSetup.cmdLine !== '--enable-legacy-cipher-list=v0.10.38')) {
-        return false;
-      }
-  }
+  if (serverSetup.ciphers === RC4_SHA_CIPHER ||
+      clientSetup.ciphers === RC4_SHA_CIPHER) {
 
-  if (clientSetup.ciphers === RC4_SHA_CIPHER) {
-    if (serverSetup.ciphers !== RC4_SHA_CIPHER &&
-        (!usesDefaultCiphers(serverSetup) ||
-         serverSetup.cmdLine !== '--enable-legacy-cipher-list=v0.10.38')) {
-      return false;
-    }
+    if (serverSetup.ciphers === RC4_SHA_CIPHER &&
+        clientSetup.ciphers === RC4_SHA_CIPHER)
+      return true;
+
+    if (serverSetup.ciphers === RC4_SHA_CIPHER &&
+        usesDefaultCiphers(clientSetup) &&
+        clientSetup.cmdLine === '--enable-legacy-cipher-list=v0.10.38')
+      return true;
+
+    if (clientSetup.ciphers === RC4_SHA_CIPHER &&
+        usesDefaultCiphers(serverSetup) &&
+        serverSetup.cmdLine === '--enable-legacy-cipher-list=v0.10.38')
+      return true;
+
+    // Otherwise, if only one end passes a RC4 cipher suite explicitly,
+    // connection should fail.
+    return false;
   }
 
+  // When not using explicitly a RC4 cipher suite, connection should succeed.
+  // Other test functions looking for other incompatibilites between SSL/TLS
+  // options will take care of testing them.
   return true;
 }