Skip to content

Commit f4c8020

Browse files
jshaindutny
authored andcommitted
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)})})"
1 parent dc1ffd0 commit f4c8020

File tree

3 files changed

+50
-8
lines changed

3 files changed

+50
-8
lines changed

lib/_tls_wrap.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,8 @@ exports.connect = function(/* [port, host], options, cb */) {
718718
var cb = args[1];
719719

720720
var defaults = {
721-
rejectUnauthorized: '0' !== process.env.NODE_TLS_REJECT_UNAUTHORIZED
721+
rejectUnauthorized: '0' !== process.env.NODE_TLS_REJECT_UNAUTHORIZED,
722+
ciphers: tls.DEFAULT_CIPHERS
722723
};
723724
options = util._extend(defaults, options || {});
724725

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
var crypto = require('crypto');
23+
var assert = require('assert');
24+
var tls = require('tls');
25+
26+
function test1() {
27+
var ciphers = '';
28+
crypto.createCredentials = function(options) {
29+
ciphers = options.ciphers
30+
}
31+
tls.connect(443);
32+
assert.equal(ciphers, tls.DEFAULT_CIPHERS);
33+
}
34+
test1();

test/simple/test-tls-honorcipherorder.js

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@ var SSL_Method = 'TLSv1_method';
3030
var localhost = '127.0.0.1';
3131

3232
process.on('exit', function() {
33-
assert.equal(nconns, 4);
33+
assert.equal(nconns, 5);
3434
});
3535

3636
function test(honorCipherOrder, clientCipher, expectedCipher, cb) {
3737
var soptions = {
3838
secureProtocol: SSL_Method,
3939
key: fs.readFileSync(common.fixturesDir + '/keys/agent2-key.pem'),
4040
cert: fs.readFileSync(common.fixturesDir + '/keys/agent2-cert.pem'),
41-
ciphers: 'AES256-SHA:RC4-SHA:DES-CBC-SHA',
41+
ciphers: 'DES-CBC-SHA:AES256-SHA:RC4-SHA',
4242
honorCipherOrder: !!honorCipherOrder
4343
};
4444

@@ -67,23 +67,30 @@ test1();
6767

6868
function test1() {
6969
// Client has the preference of cipher suites by default
70-
test(false, 'DES-CBC-SHA:RC4-SHA:AES256-SHA','DES-CBC-SHA', test2);
70+
test(false, 'AES256-SHA:DES-CBC-SHA:RC4-SHA','AES256-SHA', test2);
7171
}
7272

7373
function test2() {
74-
// Server has the preference of cipher suites where AES256-SHA is in
74+
// Server has the preference of cipher suites where DES-CBC-SHA is in
7575
// the first.
76-
test(true, 'DES-CBC-SHA:RC4-SHA:AES256-SHA', 'AES256-SHA', test3);
76+
test(true, 'AES256-SHA:DES-CBC-SHA:RC4-SHA', 'DES-CBC-SHA', test3);
7777
}
7878

7979
function test3() {
8080
// Server has the preference of cipher suites. RC4-SHA is given
8181
// higher priority over DES-CBC-SHA among client cipher suites.
82-
test(true, 'DES-CBC-SHA:RC4-SHA', 'RC4-SHA', test4);
82+
test(true, 'RC4-SHA:AES256-SHA', 'AES256-SHA', test4);
8383
}
8484

8585
function test4() {
8686
// As client has only one cipher, server has no choice in regardless
8787
// of honorCipherOrder.
88-
test(true, 'DES-CBC-SHA', 'DES-CBC-SHA');
88+
test(true, 'RC4-SHA', 'RC4-SHA', test5);
89+
}
90+
91+
function test5() {
92+
// Client did not explicitly set ciphers. Ensure that client defaults to
93+
// sane ciphers. Even though server gives top priority to DES-CBC-SHA
94+
// it should not be negotiated because it's not in default client ciphers.
95+
test(true, null, 'AES256-SHA');
8996
}

0 commit comments

Comments
 (0)