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.

PR-URL: #5536
Reviewed-By: Rod Vagg <rod@vagg.org>
  • Loading branch information
bnoordhuis committed Mar 3, 2016
1 parent ce58c2c commit 93ffe76
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 154 deletions.
8 changes: 0 additions & 8 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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']

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/_tls_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
14 changes: 0 additions & 14 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -317,23 +316,11 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& 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();
Expand Down Expand Up @@ -5171,7 +5158,6 @@ void InitCrypto(Handle<Object> target,
EVP_PKEY_decrypt>);

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 @@ -61,7 +61,6 @@

namespace node {

extern bool SSL2_ENABLE;
extern bool SSL3_ENABLE;
extern bool ALLOW_INSECURE_SERVER_DHPARAM;

Expand Down
56 changes: 1 addition & 55 deletions src/node_crypto_clienthello.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
6 changes: 0 additions & 6 deletions src/node_crypto_clienthello.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -91,7 +89,6 @@ class ClientHelloParser {
enum ParseState {
kWaiting,
kTLSHeader,
kSSL2Header,
kPaused,
kEnded
};
Expand Down Expand Up @@ -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_;
Expand Down
65 changes: 1 addition & 64 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,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))) {
Expand Down Expand Up @@ -221,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 @@ -243,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 @@ -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;
}
Expand Down Expand Up @@ -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';
}
Expand Down
Loading

0 comments on commit 93ffe76

Please sign in to comment.