Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
tls: honorCipherOrder should not degrade defaults
Browse files Browse the repository at this point in the history
Specifying honorCipherOrder should not change the SSLv2/SSLv3 defaults
for a TLS server.

Use secureOptions logic in both lib/tls.js and lib/crypto.js
  • Loading branch information
tjfontaine committed Oct 23, 2014
1 parent fe2e8a4 commit b9283cf
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 20 deletions.
44 changes: 26 additions & 18 deletions lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,31 @@ var StringDecoder = require('string_decoder').StringDecoder;

var CONTEXT_DEFAULT_OPTIONS = undefined;

function getSecureOptions(secureProtocol, secureOptions) {
if (CONTEXT_DEFAULT_OPTIONS === undefined) {
CONTEXT_DEFAULT_OPTIONS = 0;

if (!binding.SSL3_ENABLE)
CONTEXT_DEFAULT_OPTIONS |= constants.SSL_OP_NO_SSLv3;

if (!binding.SSL2_ENABLE)
CONTEXT_DEFAULT_OPTIONS |= constants.SSL_OP_NO_SSLv2;
}

if (secureOptions === undefined) {
if (secureProtocol === undefined ||
secureProtocol === 'SSLv23_method' ||
secureProtocol === 'SSLv23_server_method' ||
secureProtocol === 'SSLv23_client_method') {
secureOptions |= CONTEXT_DEFAULT_OPTIONS;
}
}

return secureOptions;
}
exports._getSecureOptions = getSecureOptions;


function Credentials(secureProtocol, flags, context) {
if (!(this instanceof Credentials)) {
return new Credentials(secureProtocol, flags, context);
Expand All @@ -82,24 +107,7 @@ function Credentials(secureProtocol, flags, context) {
}
}

if (CONTEXT_DEFAULT_OPTIONS === undefined) {
CONTEXT_DEFAULT_OPTIONS = 0;

if (!binding.SSL3_ENABLE)
CONTEXT_DEFAULT_OPTIONS |= constants.SSL_OP_NO_SSLv3;

if (!binding.SSL2_ENABLE)
CONTEXT_DEFAULT_OPTIONS |= constants.SSL_OP_NO_SSLv2;
}

if (flags === undefined) {
if (secureProtocol === undefined ||
secureProtocol === 'SSLv23_method' ||
secureProtocol === 'SSLv23_server_method' ||
secureProtocol === 'SSLv23_client_method') {
flags |= CONTEXT_DEFAULT_OPTIONS;
}
}
flags = getSecureOptions(secureProtocol, flags);

this.context.setOptions(flags);
}
Expand Down
12 changes: 10 additions & 2 deletions lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -1239,11 +1239,16 @@ Server.prototype.setOptions = function(options) {
if (options.secureProtocol) this.secureProtocol = options.secureProtocol;
if (options.crl) this.crl = options.crl;
if (options.ciphers) this.ciphers = options.ciphers;
var secureOptions = options.secureOptions || 0;

var secureOptions = crypto._getSecureOptions(options.secureProtocol,
options.secureOptions);

if (options.honorCipherOrder) {
secureOptions |= constants.SSL_OP_CIPHER_SERVER_PREFERENCE;
}
if (secureOptions) this.secureOptions = secureOptions;

this.secureOptions = secureOptions;

if (options.NPNProtocols) convertNPNProtocols(options.NPNProtocols, this);
if (options.SNICallback) {
this.SNICallback = options.SNICallback;
Expand Down Expand Up @@ -1326,6 +1331,9 @@ exports.connect = function(/* [port, host], options, cb */) {
};
options = util._extend(defaults, options || {});

options.secureOptions = crypto._getSecureOptions(options.secureProtocol,
options.secureOptions);

var socket = options.socket ? options.socket : new net.Stream();

var sslcontext = crypto.createCredentials(options);
Expand Down
131 changes: 131 additions & 0 deletions test/simple/test-tls-honorcipherorder-secureOptions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// 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 common = require('../common');
var assert = require('assert');
var tls = require('tls');
var fs = require('fs');
var nconns = 0;
var SSL_Method = 'SSLv23_method';
var localhost = '127.0.0.1';
var opCipher = process.binding('constants').SSL_OP_CIPHER_SERVER_PREFERENCE;

/*
* This test is to make sure we are preserving secureOptions that are passed
* to the server.
*
* Also that if honorCipherOrder is passed we are preserving that in the
* options.
*
* And that if we are passing in secureOptions no new options (aside from the
* honorCipherOrder case) are added to the secureOptions
*/


process.on('exit', function() {
assert.equal(nconns, 6);
});

function test(honorCipherOrder, clientCipher, expectedCipher, secureOptions, 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',
secureOptions: secureOptions,
honorCipherOrder: !!honorCipherOrder
};

var server = tls.createServer(soptions, function(cleartextStream) {
nconns++;
});

if (!!honorCipherOrder) {
assert.strictEqual(server.secureOptions & opCipher, opCipher, 'we should preserve cipher preference');
}

if (secureOptions) {
var expectedSecureOpts = secureOptions;
if (!!honorCipherOrder) expectedSecureOpts |= opCipher;

assert.strictEqual(server.secureOptions & expectedSecureOpts,
expectedSecureOpts, 'we should preserve secureOptions');
assert.strictEqual(server.secureOptions & ~expectedSecureOpts,
0,
'we should not add extra options');
}

server.listen(common.PORT, localhost, function() {
var coptions = {
rejectUnauthorized: false,
secureProtocol: SSL_Method
};
if (clientCipher) {
coptions.ciphers = clientCipher;
}
var client = tls.connect(common.PORT, localhost, coptions, function() {
var cipher = client.getCipher();
client.end();
server.close();
assert.equal(cipher.name, expectedCipher);
if (cb) cb();
});
});
}

test1();

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

function test2() {
// Server has the preference of cipher suites where AES256-SHA is in
// the first.
test(true, 'DES-CBC-SHA:RC4-SHA:AES256-SHA', 'AES256-SHA', 0, 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', 0, 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', 0, test5);
}

function test5() {
test(false,
'DES-CBC-SHA',
'DES-CBC-SHA',
process.binding('constants').SSL_OP_SINGLE_DH_USE, test6);
}

function test6() {
test(true,
'DES-CBC-SHA',
'DES-CBC-SHA',
process.binding('constants').SSL_OP_SINGLE_DH_USE);
}

0 comments on commit b9283cf

Please sign in to comment.