Skip to content

Commit

Permalink
build,src: remove sslv2 support
Browse files Browse the repository at this point in the history
SSLv2 has been deprecated and known broken for nearly twenty years now.

I made SSLv2 support opt-in well over a year ago in commit 39aa894 and
now this commit removes it entirely.

PR-URL: #290
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
  • Loading branch information
bnoordhuis committed Jan 11, 2015
1 parent 3ecad1d commit 26dd9e1
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 90 deletions.
10 changes: 0 additions & 10 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,6 @@ parser.add_option('--with-perfctr',
dest='with_perfctr',
help='build with performance counters (default is true on Windows)')

parser.add_option('--with-sslv2',
action='store_true',
dest='with_sslv2',
help='enable SSL v2')

parser.add_option('--without-dtrace',
action='store_true',
dest='without_dtrace',
Expand Down Expand Up @@ -588,11 +583,6 @@ def configure_openssl(o):
if options.without_ssl:
return

# OpenSSL uses `#ifndef OPENSSL_NO_SSL2` checks so only define the
# macro when we want to _disable_ SSL2.
if not options.with_sslv2:
o['defines'] += ['OPENSSL_NO_SSL2=1']

if options.shared_openssl:
(libs, cflags) = pkg_config('openssl') or ('-lssl -lcrypto', '')

Expand Down
4 changes: 4 additions & 0 deletions deps/openssl/openssl.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,10 @@
'PURIFY',
'_REENTRANT',

# SSLv2 is known broken and has been superseded by SSLv3 for almost
# twenty years now.
'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
Expand Down
4 changes: 0 additions & 4 deletions doc/api/tls.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,6 @@ automatically set as a listener for the [secureConnection][] event. The
use this option in conjunction with the `ciphers` option to mitigate
BEAST attacks.

Note: If SSLv2 is used, the server will send its list of preferences to the
client, and the client chooses the cipher. Support for SSLv2 is disabled
unless node.js was configured with `./configure --with-sslv2`.

- `requestCert`: If `true` the server will request a certificate from
clients that connect and attempt to verify that certificate. Default:
`false`.
Expand Down
17 changes: 5 additions & 12 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -310,23 +310,11 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
const node::Utf8Value sslmethod(env->isolate(), 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 @@ -376,6 +364,11 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {

sc->ctx_ = SSL_CTX_new(method);

// Disable SSLv2 in the case when method == SSLv23_method() and the
// cipher list contains SSLv2 ciphers (not the default, should be rare.)
// The bundled OpenSSL doesn't have SSLv2 support but the system OpenSSL may.
SSL_CTX_set_options(sc->ctx_, SSL_OP_NO_SSLv2);

// SSL session cache configuration
SSL_CTX_set_session_cache_mode(sc->ctx_,
SSL_SESS_CACHE_SERVER |
Expand Down
66 changes: 8 additions & 58 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 @@ -99,24 +80,20 @@ void ClientHelloParser::ParseHeader(const uint8_t* data, size_t avail) {
if (body_offset_ + frame_len_ > avail)
return;

// 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)) {
// Check hello protocol version. Protocol tuples that we know about:
//
// (3,0) SSL v3.0
// (3,1) TLS v1.0
// (3,2) TLS v1.1
// (3,3) TLS v1.2
//
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 +122,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 @@ -269,28 +243,4 @@ bool ClientHelloParser::ParseTLSClientHello(const uint8_t* data, size_t avail) {
return true;
}


#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 @@ -77,8 +77,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 @@ -88,7 +86,6 @@ class ClientHelloParser {
enum ParseState {
kWaiting,
kTLSHeader,
kSSL2Header,
kPaused,
kEnded
};
Expand Down Expand Up @@ -117,9 +114,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

1 comment on commit 26dd9e1

@mscdex
Copy link
Contributor

@mscdex mscdex commented on 26dd9e1 Jan 11, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+10

Please sign in to comment.