Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto,tls: remove SSLv2 support #5529

Merged
merged 1 commit into from
Mar 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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/);
});