From f8cb0dcf672c48e9e0ed8caa8ec8bc49d5f09cab Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 2 Mar 2016 19:37:14 +0100 Subject: [PATCH] crypto,tls: remove SSLv2 support 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: https://github.com/nodejs/LTS/issues/80 PR-URL: https://github.com/nodejs/node/pull/5529 Reviewed-By: Rod Vagg --- configure | 8 --- deps/openssl/openssl.gyp | 8 ++- lib/crypto.js | 3 -- src/node.cc | 6 +-- src/node_crypto.cc | 14 ------ src/node_crypto.h | 1 - test/external/ssl-options/test.js | 74 +--------------------------- test/simple/test-tls-enable-ssl2.js | 21 ++++++++ test/simple/test-tls-ssl2-methods.js | 19 +++++++ 9 files changed, 51 insertions(+), 103 deletions(-) create mode 100644 test/simple/test-tls-enable-ssl2.js create mode 100644 test/simple/test-tls-ssl2-methods.js diff --git a/configure b/configure index 2dc664406f..5255030547 100755 --- a/configure +++ b/configure @@ -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", @@ -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'] diff --git a/deps/openssl/openssl.gyp b/deps/openssl/openssl.gyp index 9772c13fec..58feb47445 100644 --- a/deps/openssl/openssl.gyp +++ b/deps/openssl/openssl.gyp @@ -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': [ diff --git a/lib/crypto.js b/lib/crypto.js index 597d196f2f..ffa872efc3 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -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) { diff --git a/src/node.cc b/src/node.cc index ecb85cad95..c4b12a2f74 100644 --- a/src/node.cc +++ b/src/node.cc @@ -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" @@ -2675,8 +2674,9 @@ static void ParseArgs(int argc, char **argv) { argv[i] = const_cast(""); #if HAVE_OPENSSL } else if (strcmp(arg, "--enable-ssl2") == 0) { - SSL2_ENABLE = true; - argv[i] = const_cast(""); + 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(""); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 60b6b453b7..f5ad6cae68 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -69,7 +69,6 @@ const char* root_certs[] = { NULL }; -bool SSL2_ENABLE = false; bool SSL3_ENABLE = false; namespace crypto { @@ -242,23 +241,11 @@ Handle 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(); @@ -4256,7 +4243,6 @@ void InitCrypto(Handle target) { ext_key_usage_symbol = NODE_PSYMBOL("ext_key_usage"); NODE_DEFINE_CONSTANT(target, SSL3_ENABLE); - NODE_DEFINE_CONSTANT(target, SSL2_ENABLE); } } // namespace crypto diff --git a/src/node_crypto.h b/src/node_crypto.h index 54b9b88e43..11175feb1b 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -45,7 +45,6 @@ namespace node { -extern bool SSL2_ENABLE; extern bool SSL3_ENABLE; namespace crypto { diff --git a/test/external/ssl-options/test.js b/test/external/ssl-options/test.js index f7e06c93df..de7a4df61f 100644 --- a/test/external/ssl-options/test.js +++ b/test/external/ssl-options/test.js @@ -11,13 +11,10 @@ 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' @@ -25,7 +22,6 @@ var SERVER_SSL_PROTOCOLS = [ 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' @@ -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) { @@ -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; } @@ -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))) { @@ -230,12 +178,6 @@ function createTestsSetups() { }; serversSetup.push(serverSetup); - - if (isSsl2Protocol(serverSecureProtocol)) { - var setupWithSsl2Ciphers = xtend(serverSetup); - setupWithSsl2Ciphers.ciphers = SSL2_COMPATIBLE_CIPHERS; - serversSetup.push(setupWithSsl2Ciphers); - } } }); }); @@ -252,12 +194,6 @@ function createTestsSetups() { }; clientsSetup.push(clientSetup); - - if (isSsl2Protocol(clientSecureProtocol)) { - var setupWithSsl2Ciphers = xtend(clientSetup); - setupWithSsl2Ciphers.ciphers = SSL2_COMPATIBLE_CIPHERS; - clientsSetup.push(setupWithSsl2Ciphers); - } } }); }); @@ -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; } @@ -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'; } diff --git a/test/simple/test-tls-enable-ssl2.js b/test/simple/test-tls-enable-ssl2.js new file mode 100644 index 0000000000..65c91186e4 --- /dev/null +++ b/test/simple/test-tls-enable-ssl2.js @@ -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)); +}); diff --git a/test/simple/test-tls-ssl2-methods.js b/test/simple/test-tls-ssl2-methods.js new file mode 100644 index 0000000000..c4810a5393 --- /dev/null +++ b/test/simple/test-tls-ssl2-methods.js @@ -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/); +});