From f4c8020d1068ffba57458b327c62b61b1f29ec63 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 23 Jan 2014 17:28:08 -0800 Subject: [PATCH] crypto: honor default ciphers in client mode 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)})})" --- lib/_tls_wrap.js | 3 +- .../simple/test-tls-client-default-ciphers.js | 34 +++++++++++++++++++ test/simple/test-tls-honorcipherorder.js | 21 ++++++++---- 3 files changed, 50 insertions(+), 8 deletions(-) create mode 100644 test/simple/test-tls-client-default-ciphers.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index df68d2bedc..9324130574 100644 --- 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 new file mode 100644 index 0000000000..bc5e33b367 --- /dev/null +++ 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 index 539a12abf4..69ea946531 100644 --- a/test/simple/test-tls-honorcipherorder.js +++ b/test/simple/test-tls-honorcipherorder.js @@ -30,7 +30,7 @@ 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) { @@ -38,7 +38,7 @@ function test(honorCipherOrder, clientCipher, expectedCipher, cb) { 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'); }