From 93ffe76cce1f20e4d7abab8ff72506f02731d5a1 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. PR-URL: https://github.com/nodejs/node/pull/5536 Reviewed-By: Rod Vagg --- configure | 8 ---- deps/openssl/openssl.gyp | 8 +++- lib/_tls_common.js | 3 -- src/node.cc | 5 ++- src/node_crypto.cc | 14 ------ src/node_crypto.h | 1 - src/node_crypto_clienthello.cc | 56 +----------------------- src/node_crypto_clienthello.h | 6 --- test/external/ssl-options/test.js | 65 +--------------------------- test/simple/test-tls-enable-ssl2.js | 21 +++++++++ test/simple/test-tls-ssl2-methods.js | 19 ++++++++ 11 files changed, 52 insertions(+), 154 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 7ad946eaf80770..06f32a5e15073c 100755 --- a/configure +++ b/configure @@ -313,11 +313,6 @@ parser.add_option('--without-ssl', dest='without_ssl', help='build without SSL') -parser.add_option('--without-ssl2', - action='store_true', - dest='ssl2', - help='Disable SSL v2') - parser.add_option('--without-ssl3', action='store_true', dest='ssl3', @@ -687,9 +682,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 9772c13fec4ab5..58feb474453bb8 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/_tls_common.js b/lib/_tls_common.js index 72648be9843338..0a48af19317ba5 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -37,9 +37,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 2335988bcd6b62..487855cde9b329 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3012,7 +3012,6 @@ static void PrintHelp() { " present.\n" #endif #endif - " --enable-ssl2 enable ssl2\n" " --enable-ssl3 enable ssl3\n" "\n" "Environment variables:\n" @@ -3083,7 +3082,9 @@ static void ParseArgs(int* argc, exit(0); } else if (strcmp(arg, "--enable-ssl2") == 0) { #if HAVE_OPENSSL - SSL2_ENABLE = true; + fprintf(stderr, + "Error: --enable-ssl2 is no longer supported (CVE-2016-0800).\n"); + exit(12); #endif } else if (strcmp(arg, "--enable-ssl3") == 0) { #if HAVE_OPENSSL diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 454c9be190a351..06b3f0d28a9726 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -75,7 +75,6 @@ static const int X509_NAME_FLAGS = ASN1_STRFLGS_ESC_CTRL namespace node { -bool SSL2_ENABLE = false; bool SSL3_ENABLE = false; bool ALLOW_INSECURE_SERVER_DHPARAM = false; @@ -317,23 +316,11 @@ void SecureContext::Init(const FunctionCallbackInfo& args) { const node::Utf8Value sslmethod(args[0]); if (strcmp(*sslmethod, "SSLv2_method") == 0) { -#ifndef OPENSSL_NO_SSL2 - method = SSLv2_method(); -#else return env->ThrowError("SSLv2 methods disabled"); -#endif } else if (strcmp(*sslmethod, "SSLv2_server_method") == 0) { -#ifndef OPENSSL_NO_SSL2 - method = SSLv2_server_method(); -#else return env->ThrowError("SSLv2 methods disabled"); -#endif } else if (strcmp(*sslmethod, "SSLv2_client_method") == 0) { -#ifndef OPENSSL_NO_SSL2 - method = SSLv2_client_method(); -#else return env->ThrowError("SSLv2 methods disabled"); -#endif } else if (strcmp(*sslmethod, "SSLv3_method") == 0) { #ifndef OPENSSL_NO_SSL3 method = SSLv3_method(); @@ -5171,7 +5158,6 @@ void InitCrypto(Handle target, EVP_PKEY_decrypt>); 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 4687d40e2f8226..ed840b310bf35c 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -61,7 +61,6 @@ namespace node { -extern bool SSL2_ENABLE; extern bool SSL3_ENABLE; extern bool ALLOW_INSECURE_SERVER_DHPARAM; diff --git a/src/node_crypto_clienthello.cc b/src/node_crypto_clienthello.cc index ad0235343cc4eb..12fa4d5738c5ea 100644 --- a/src/node_crypto_clienthello.cc +++ b/src/node_crypto_clienthello.cc @@ -32,7 +32,6 @@ void ClientHelloParser::Parse(const uint8_t* data, size_t avail) { break; // Fall through case kTLSHeader: - case kSSL2Header: ParseHeader(data, avail); break; case kPaused: @@ -59,20 +58,8 @@ bool ClientHelloParser::ParseRecordHeader(const uint8_t* data, size_t avail) { state_ = kTLSHeader; body_offset_ = 5; } else { -#ifdef OPENSSL_NO_SSL2 - frame_len_ = ((data[0] << 8) & kSSL2HeaderMask) + data[1]; - state_ = kSSL2Header; - if (data[0] & kSSL2TwoByteHeaderBit) { - // header without padding - body_offset_ = 2; - } else { - // header with padding - body_offset_ = 3; - } -#else End(); return false; -#endif // OPENSSL_NO_SSL2 } // Sanity check (too big frame, or too small) @@ -85,12 +72,6 @@ bool ClientHelloParser::ParseRecordHeader(const uint8_t* data, size_t avail) { return true; } -#ifdef OPENSSL_NO_SSL2 -# define NODE_SSL2_VER_CHECK(buf) false -#else -# define NODE_SSL2_VER_CHECK(buf) ((buf)[0] == 0x00 && (buf)[1] == 0x02) -#endif // OPENSSL_NO_SSL2 - void ClientHelloParser::ParseHeader(const uint8_t* data, size_t avail) { ClientHello hello; @@ -101,22 +82,13 @@ void ClientHelloParser::ParseHeader(const uint8_t* data, size_t avail) { // Skip unsupported frames and gather some data from frame // Check hello protocol version - if (!(data[body_offset_ + 4] == 0x03 && data[body_offset_ + 5] <= 0x03) && - !NODE_SSL2_VER_CHECK(data + body_offset_ + 4)) { + if (!(data[body_offset_ + 4] == 0x03 && data[body_offset_ + 5] <= 0x03)) goto fail; - } if (data[body_offset_] == kClientHello) { if (state_ == kTLSHeader) { if (!ParseTLSClientHello(data, avail)) goto fail; - } else if (state_ == kSSL2Header) { -#ifdef OPENSSL_NO_SSL2 - if (!ParseSSL2ClientHello(data, avail)) - goto fail; -#else - abort(); // Unreachable -#endif // OPENSSL_NO_SSL2 } else { // We couldn't get here, but whatever goto fail; @@ -145,9 +117,6 @@ void ClientHelloParser::ParseHeader(const uint8_t* data, size_t avail) { } -#undef NODE_SSL2_VER_CHECK - - void ClientHelloParser::ParseExtension(ClientHelloParser::ExtensionType type, const uint8_t* data, size_t len) { @@ -270,27 +239,4 @@ bool ClientHelloParser::ParseTLSClientHello(const uint8_t* data, size_t avail) { } -#ifdef OPENSSL_NO_SSL2 -bool ClientHelloParser::ParseSSL2ClientHello(const uint8_t* data, - size_t avail) { - const uint8_t* body; - - // Skip header, version - size_t session_offset = body_offset_ + 3; - - if (session_offset + 4 < avail) { - body = data + session_offset; - - uint16_t ciphers_size = (body[0] << 8) + body[1]; - - if (body + 4 + ciphers_size < data + avail) { - session_size_ = (body[2] << 8) + body[3]; - session_id_ = body + 4 + ciphers_size; - } - } - - return true; -} -#endif // OPENSSL_NO_SSL2 - } // namespace node diff --git a/src/node_crypto_clienthello.h b/src/node_crypto_clienthello.h index 3ebcead0c36c7b..54446317683587 100644 --- a/src/node_crypto_clienthello.h +++ b/src/node_crypto_clienthello.h @@ -80,8 +80,6 @@ class ClientHelloParser { inline bool IsEnded() const; private: - static const uint8_t kSSL2TwoByteHeaderBit = 0x80; - static const uint8_t kSSL2HeaderMask = 0x3f; static const size_t kMaxTLSFrameLen = 16 * 1024 + 5; static const size_t kMaxSSLExFrameLen = 32 * 1024; static const uint8_t kServernameHostname = 0; @@ -91,7 +89,6 @@ class ClientHelloParser { enum ParseState { kWaiting, kTLSHeader, - kSSL2Header, kPaused, kEnded }; @@ -120,9 +117,6 @@ class ClientHelloParser { const uint8_t* data, size_t len); bool ParseTLSClientHello(const uint8_t* data, size_t avail); -#ifdef OPENSSL_NO_SSL2 - bool ParseSSL2ClientHello(const uint8_t* data, size_t avail); -#endif // OPENSSL_NO_SSL2 ParseState state_; OnHelloCb onhello_cb_; diff --git a/test/external/ssl-options/test.js b/test/external/ssl-options/test.js index 320d323077e706..b8bb2659a7de34 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,30 +146,10 @@ function testSetupsCompatible(serverSetup, clientSetup) { return false; } - var ssl2Used = isSsl2Protocol(serverSetup.secureProtocol) || - isSsl2Protocol(clientSetup.secureProtocol); - if (ssl2Used && - ((serverSetup.ciphers !== SSL2_COMPATIBLE_CIPHERS) || - (clientSetup.ciphers !== SSL2_COMPATIBLE_CIPHERS))) { - /* - * Default ciphers are not compatible with SSLv2. Both client *and* - * server need to specify a SSLv2 compatible cipher to be able to use - * SSLv2. - */ - 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))) { @@ -221,12 +178,6 @@ function createTestsSetups() { }; serversSetup.push(serverSetup); - - if (isSsl2Protocol(serverSecureProtocol)) { - var setupWithSsl2Ciphers = xtend(serverSetup); - setupWithSsl2Ciphers.ciphers = SSL2_COMPATIBLE_CIPHERS; - serversSetup.push(setupWithSsl2Ciphers); - } } }); }); @@ -243,12 +194,6 @@ function createTestsSetups() { }; clientsSetup.push(clientSetup); - - if (isSsl2Protocol(clientSecureProtocol)) { - var setupWithSsl2Ciphers = xtend(clientSetup); - setupWithSsl2Ciphers.ciphers = SSL2_COMPATIBLE_CIPHERS; - clientsSetup.push(setupWithSsl2Ciphers); - } } }); }); @@ -359,10 +304,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; } @@ -422,10 +363,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 00000000000000..65c91186e4ee0d --- /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 00000000000000..c4810a539317a2 --- /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/); +});