crypto: honor default ciphers in client mode

jsha · indutny · commit f4c8020d1068 · 2014-01-28T03:28:09.000+04:00
Right now no default ciphers are use in, e.g. https.get, meaning that weak export ciphers like TLS_RSA_EXPORT_WITH_DES40_CBC_SHA are accepted. To reproduce: node -e "require('https').get({hostname: 'www.howsmyssl.com', \ path: '/a/check'}, function(res) {res.on('data', \ function(d) {process.stdout.write(d)})})"
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -718,7 +718,8 @@ exports.connect = function(/* [port, host], options, cb */) {
   var cb = args[1];
 
   var defaults = {
-    rejectUnauthorized: '0' !== process.env.NODE_TLS_REJECT_UNAUTHORIZED
+    rejectUnauthorized: '0' !== process.env.NODE_TLS_REJECT_UNAUTHORIZED,
+    ciphers: tls.DEFAULT_CIPHERS
   };
   options = util._extend(defaults, options || {});
 
diff --git a/test/simple/test-tls-client-default-ciphers.js b/test/simple/test-tls-client-default-ciphers.js
@@ -0,0 +1,34 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var crypto = require('crypto');
+var assert = require('assert');
+var tls = require('tls');
+
+function test1() {
+  var ciphers = '';
+  crypto.createCredentials = function(options) {
+    ciphers = options.ciphers
+  }
+  tls.connect(443);
+  assert.equal(ciphers, tls.DEFAULT_CIPHERS);
+}
+test1();
diff --git a/test/simple/test-tls-honorcipherorder.js b/test/simple/test-tls-honorcipherorder.js
@@ -30,15 +30,15 @@ var SSL_Method = 'TLSv1_method';
 var localhost = '127.0.0.1';
 
 process.on('exit', function() {
-  assert.equal(nconns, 4);
+  assert.equal(nconns, 5);
 });
 
 function test(honorCipherOrder, clientCipher, expectedCipher, cb) {
   var soptions = {
     secureProtocol: SSL_Method,
     key: fs.readFileSync(common.fixturesDir + '/keys/agent2-key.pem'),
     cert: fs.readFileSync(common.fixturesDir + '/keys/agent2-cert.pem'),
-    ciphers: 'AES256-SHA:RC4-SHA:DES-CBC-SHA',
+    ciphers: 'DES-CBC-SHA:AES256-SHA:RC4-SHA',
     honorCipherOrder: !!honorCipherOrder
   };
 
@@ -67,23 +67,30 @@ test1();
 
 function test1() {
   // Client has the preference of cipher suites by default
-  test(false, 'DES-CBC-SHA:RC4-SHA:AES256-SHA','DES-CBC-SHA', test2);
+  test(false, 'AES256-SHA:DES-CBC-SHA:RC4-SHA','AES256-SHA', test2);
 }
 
 function test2() {
-  // Server has the preference of cipher suites where AES256-SHA is in
+  // Server has the preference of cipher suites where DES-CBC-SHA is in
   // the first.
-  test(true, 'DES-CBC-SHA:RC4-SHA:AES256-SHA', 'AES256-SHA', test3);
+  test(true, 'AES256-SHA:DES-CBC-SHA:RC4-SHA', 'DES-CBC-SHA', test3);
 }
 
 function test3() {
   // Server has the preference of cipher suites. RC4-SHA is given
   // higher priority over DES-CBC-SHA among client cipher suites.
-  test(true, 'DES-CBC-SHA:RC4-SHA', 'RC4-SHA', test4);
+  test(true, 'RC4-SHA:AES256-SHA', 'AES256-SHA', test4);
 }
 
 function test4() {
   // As client has only one cipher, server has no choice in regardless
   // of honorCipherOrder.
-  test(true, 'DES-CBC-SHA', 'DES-CBC-SHA');
+  test(true, 'RC4-SHA', 'RC4-SHA', test5);
+}
+
+function test5() {
+  // Client did not explicitly set ciphers. Ensure that client defaults to
+  // sane ciphers. Even though server gives top priority to DES-CBC-SHA
+  // it should not be negotiated because it's not in default client ciphers.
+  test(true, null, 'AES256-SHA');
 }
