Skip to content

Commit

Permalink
crypto,tls: remove SSLv2 support
Browse files Browse the repository at this point in the history
Remove support for SSLv2 because of DROWN (CVE-2016-0800).

Use of the `--enable-ssl2` flag is now an error; node will print an
error message and exit.

Fixes: nodejs/Release#80
PR-URL: #5529
Reviewed-By: Rod Vagg <[email protected]>
  • Loading branch information
bnoordhuis committed Mar 2, 2016
1 parent e483f3f commit f8cb0dc
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 103 deletions.
8 changes: 0 additions & 8 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,6 @@ parser.add_option("--systemtap-includes",
dest="systemtap_includes",
help=optparse.SUPPRESS_HELP)

parser.add_option("--without-ssl2",
action="store_true",
dest="ssl2",
help="Disable SSL v2")

parser.add_option("--without-ssl3",
action="store_true",
dest="ssl3",
Expand Down Expand Up @@ -632,9 +627,6 @@ def configure_openssl(o):
if options.without_ssl:
return

if options.ssl2:
o['defines'] += ['OPENSSL_NO_SSL2=1']

if options.ssl3:
o['defines'] += ['OPENSSL_NO_SSL3=1']

Expand Down
8 changes: 7 additions & 1 deletion deps/openssl/openssl.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -1093,13 +1093,19 @@
'L_ENDIAN',
'PURIFY',
'_REENTRANT',

'OPENSSL_NO_SSL2',
# Heartbeat is a TLS extension, that couldn't be turned off or
# asked to be not advertised. Unfortunately this is unacceptable for
# Microsoft's IIS, which seems to be ignoring whole ClientHello after
# seeing this extension.
'OPENSSL_NO_HEARTBEATS',
],
'direct_dependent_settings': {
'defines': [
'OPENSSL_NO_SSL2',
'OPENSSL_NO_HEARTBEATS',
],
},
'conditions': [
['OS=="win"', {
'defines': [
Expand Down
3 changes: 0 additions & 3 deletions lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ function getSecureOptions(secureProtocol, secureOptions) {

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) {
Expand Down
6 changes: 3 additions & 3 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2637,7 +2637,6 @@ static void PrintHelp() {
" --v8-options print v8 command line options\n"
" --max-stack-size=val set max v8 stack size (bytes)\n"
#if HAVE_OPENSSL
" --enable-ssl2 enable ssl2\n"
" --enable-ssl3 enable ssl3\n"
#endif
"\n"
Expand Down Expand Up @@ -2675,8 +2674,9 @@ static void ParseArgs(int argc, char **argv) {
argv[i] = const_cast<char*>("");
#if HAVE_OPENSSL
} else if (strcmp(arg, "--enable-ssl2") == 0) {
SSL2_ENABLE = true;
argv[i] = const_cast<char*>("");
fprintf(stderr,
"Error: --enable-ssl2 is no longer supported (CVE-2016-0800).\n");
exit(12);
} else if (strcmp(arg, "--enable-ssl3") == 0) {
SSL3_ENABLE = true;
argv[i] = const_cast<char*>("");
Expand Down
14 changes: 0 additions & 14 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ const char* root_certs[] = {
NULL
};

bool SSL2_ENABLE = false;
bool SSL3_ENABLE = false;

namespace crypto {
Expand Down Expand Up @@ -242,23 +241,11 @@ Handle<Value> SecureContext::Init(const Arguments& args) {
node::Utf8Value sslmethod(args[0]);

if (strcmp(*sslmethod, "SSLv2_method") == 0) {
#ifndef OPENSSL_NO_SSL2
method = SSLv2_method();
#else
return ThrowException(Exception::Error(String::New("SSLv2 methods disabled")));
#endif
} else if (strcmp(*sslmethod, "SSLv2_server_method") == 0) {
#ifndef OPENSSL_NO_SSL2
method = SSLv2_server_method();
#else
return ThrowException(Exception::Error(String::New("SSLv2 methods disabled")));
#endif
} else if (strcmp(*sslmethod, "SSLv2_client_method") == 0) {
#ifndef OPENSSL_NO_SSL2
method = SSLv2_client_method();
#else
return ThrowException(Exception::Error(String::New("SSLv2 methods disabled")));
#endif
} else if (strcmp(*sslmethod, "SSLv3_method") == 0) {
#ifndef OPENSSL_NO_SSL3
method = SSLv3_method();
Expand Down Expand Up @@ -4256,7 +4243,6 @@ void InitCrypto(Handle<Object> target) {
ext_key_usage_symbol = NODE_PSYMBOL("ext_key_usage");

NODE_DEFINE_CONSTANT(target, SSL3_ENABLE);
NODE_DEFINE_CONSTANT(target, SSL2_ENABLE);
}

} // namespace crypto
Expand Down
1 change: 0 additions & 1 deletion src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@

namespace node {

extern bool SSL2_ENABLE;
extern bool SSL3_ENABLE;

namespace crypto {
Expand Down
74 changes: 1 addition & 73 deletions test/external/ssl-options/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,17 @@ var debug = require('debug')('test-node-ssl');

var common = require('../../common');

var SSL2_COMPATIBLE_CIPHERS = 'RC4-MD5';

var CMD_LINE_OPTIONS = [ null, "--enable-ssl2", "--enable-ssl3" ];
var CMD_LINE_OPTIONS = [ null, "--enable-ssl3" ];

var SERVER_SSL_PROTOCOLS = [
null,
'SSLv2_method', 'SSLv2_server_method',
'SSLv3_method', 'SSLv3_server_method',
'TLSv1_method', 'TLSv1_server_method',
'SSLv23_method','SSLv23_server_method'
];

var CLIENT_SSL_PROTOCOLS = [
null,
'SSLv2_method', 'SSLv2_client_method',
'SSLv3_method', 'SSLv3_client_method',
'TLSv1_method', 'TLSv1_client_method',
'SSLv23_method','SSLv23_client_method'
Expand All @@ -34,9 +30,7 @@ var CLIENT_SSL_PROTOCOLS = [
var SECURE_OPTIONS = [
null,
0,
constants.SSL_OP_NO_SSLv2,
constants.SSL_OP_NO_SSLv3,
constants.SSL_OP_NO_SSLv2 | constants.SSL_OP_NO_SSLv3
];

function xtend(source) {
Expand Down Expand Up @@ -105,30 +99,13 @@ function isSsl3Protocol(secureProtocol) {
secureProtocol === 'SSLv3_server_method';
}

function isSsl2Protocol(secureProtocol) {
assert(secureProtocol === null || typeof secureProtocol === 'string');

return secureProtocol === 'SSLv2_method' ||
secureProtocol === 'SSLv2_client_method' ||
secureProtocol === 'SSLv2_server_method';
}

function secureProtocolCompatibleWithSecureOptions(secureProtocol, secureOptions, cmdLineOption) {
if (secureOptions == null) {
if (isSsl2Protocol(secureProtocol) &&
(!cmdLineOption || cmdLineOption.indexOf('--enable-ssl2') === -1)) {
return false;
}

if (isSsl3Protocol(secureProtocol) &&
(!cmdLineOption || cmdLineOption.indexOf('--enable-ssl3') === -1)) {
return false;
}
} else {
if (secureOptions & constants.SSL_OP_NO_SSLv2 && isSsl2Protocol(secureProtocol)) {
return false;
}

if (secureOptions & constants.SSL_OP_NO_SSLv3 && isSsl3Protocol(secureProtocol)) {
return false;
}
Expand Down Expand Up @@ -169,39 +146,10 @@ function testSetupsCompatible(serverSetup, clientSetup) {
return false;
}

if (isSsl2Protocol(serverSetup.secureProtocol) ||
isSsl2Protocol(clientSetup.secureProtocol)) {

/*
* It seems that in order to be able to use SSLv2, at least the server
* *needs* to advertise at least one cipher compatible with it.
*/
if (serverSetup.ciphers !== SSL2_COMPATIBLE_CIPHERS) {
return false;
}

/*
* If only either one of the client or server specify SSLv2 as their
* protocol, then *both* of them *need* to advertise at least one cipher
* that is compatible with SSLv2.
*/
if ((!isSsl2Protocol(serverSetup.secureProtocol) || !isSsl2Protocol(clientSetup.secureProtocol)) &&
(clientSetup.ciphers !== SSL2_COMPATIBLE_CIPHERS || serverSetup.ciphers !== SSL2_COMPATIBLE_CIPHERS)) {
return false;
}
}

return true;
}

function sslSetupMakesSense(cmdLineOption, secureProtocol, secureOption) {
if (isSsl2Protocol(secureProtocol)) {
if (secureOption & constants.SSL_OP_NO_SSLv2 ||
(secureOption == null && (!cmdLineOption || cmdLineOption.indexOf('--enable-ssl2') === -1))) {
return false;
}
}

if (isSsl3Protocol(secureProtocol)) {
if (secureOption & constants.SSL_OP_NO_SSLv3 ||
(secureOption == null && (!cmdLineOption || cmdLineOption.indexOf('--enable-ssl3') === -1))) {
Expand Down Expand Up @@ -230,12 +178,6 @@ function createTestsSetups() {
};

serversSetup.push(serverSetup);

if (isSsl2Protocol(serverSecureProtocol)) {
var setupWithSsl2Ciphers = xtend(serverSetup);
setupWithSsl2Ciphers.ciphers = SSL2_COMPATIBLE_CIPHERS;
serversSetup.push(setupWithSsl2Ciphers);
}
}
});
});
Expand All @@ -252,12 +194,6 @@ function createTestsSetups() {
};

clientsSetup.push(clientSetup);

if (isSsl2Protocol(clientSecureProtocol)) {
var setupWithSsl2Ciphers = xtend(clientSetup);
setupWithSsl2Ciphers.ciphers = SSL2_COMPATIBLE_CIPHERS;
clientsSetup.push(setupWithSsl2Ciphers);
}
}
});
});
Expand Down Expand Up @@ -367,10 +303,6 @@ function stringToSecureOptions(secureOptionsString) {

var optionStrings = secureOptionsString.split('|');
optionStrings.forEach(function (option) {
if (option === 'SSL_OP_NO_SSLv2') {
secureOptions |= constants.SSL_OP_NO_SSLv2;
}

if (option === 'SSL_OP_NO_SSLv3') {
secureOptions |= constants.SSL_OP_NO_SSLv3;
}
Expand Down Expand Up @@ -430,10 +362,6 @@ function checkTestExitCode(testSetup, serverExitCode, clientExitCode) {
function secureOptionsToString(secureOptions) {
var secureOptsString = '';

if (secureOptions & constants.SSL_OP_NO_SSLv2) {
secureOptsString += 'SSL_OP_NO_SSLv2';
}

if (secureOptions & constants.SSL_OP_NO_SSLv3) {
secureOptsString += '|SSL_OP_NO_SSLv3';
}
Expand Down
21 changes: 21 additions & 0 deletions test/simple/test-tls-enable-ssl2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

var common = require('../common');
var assert = require('assert');
var spawn = require('child_process').spawn;

var stdout = '';
var stderr = '';
var proc = spawn(process.execPath, ['--enable-ssl2', '-e', '0']);
proc.stdout.setEncoding('utf8');
proc.stderr.setEncoding('utf8');
proc.stdout.on('data', function(data) { stdout += data; });
proc.stderr.on('data', function(data) { stderr += data; });
proc.on('exit', common.mustCall(function(exitCode, signalCode) {
assert.strictEqual(exitCode, 12);
assert.strictEqual(signalCode, null);
}));
process.on('exit', function() {
assert.strictEqual(stdout, '');
assert(/Error: --enable-ssl2 is no longer supported/.test(stderr));
});
19 changes: 19 additions & 0 deletions test/simple/test-tls-ssl2-methods.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

var common = require('../common');
var assert = require('assert');

try {
var crypto = require('crypto');
} catch (e) {
console.log('1..0 # Skipped: missing crypto');
return;
}

var methods = ['SSLv2_method', 'SSLv2_client_method', 'SSLv2_server_method'];

methods.forEach(function(method) {
assert.throws(function() {
crypto.createCredentials({ secureProtocol: method });
}, /SSLv2 methods disabled/);
});

0 comments on commit f8cb0dc

Please sign in to comment.