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

build,src: remove sslv3 support #315

Merged
merged 1 commit into from Jan 13, 2015
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions deps/openssl/openssl.gyp
Expand Up @@ -1098,6 +1098,9 @@
# twenty years now.
'OPENSSL_NO_SSL2',

# SSLv3 is susceptible to downgrade attacks (POODLE.)
'OPENSSL_NO_SSL3',

# 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
18 changes: 6 additions & 12 deletions src/node_crypto.cc
Expand Up @@ -288,30 +288,22 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
if (args.Length() == 1 && args[0]->IsString()) {
const node::Utf8Value sslmethod(env->isolate(), args[0]);

// Note that SSLv2 and SSLv3 are disallowed but SSLv2_method and friends
// are still accepted. They are OpenSSL's way of saying that all known
// protocols are supported unless explicitly disabled (which we do below
// for SSLv2 and SSLv3.)
if (strcmp(*sslmethod, "SSLv2_method") == 0) {
return env->ThrowError("SSLv2 methods disabled");
} else if (strcmp(*sslmethod, "SSLv2_server_method") == 0) {
return env->ThrowError("SSLv2 methods disabled");
} else if (strcmp(*sslmethod, "SSLv2_client_method") == 0) {
return env->ThrowError("SSLv2 methods disabled");
} else if (strcmp(*sslmethod, "SSLv3_method") == 0) {
#ifndef OPENSSL_NO_SSL3
method = SSLv3_method();
#else
return env->ThrowError("SSLv3 methods disabled");
#endif
} else if (strcmp(*sslmethod, "SSLv3_server_method") == 0) {
#ifndef OPENSSL_NO_SSL3
method = SSLv3_server_method();
#else
return env->ThrowError("SSLv3 methods disabled");
#endif
} else if (strcmp(*sslmethod, "SSLv3_client_method") == 0) {
#ifndef OPENSSL_NO_SSL3
method = SSLv3_client_method();
#else
return env->ThrowError("SSLv3 methods disabled");
#endif
} else if (strcmp(*sslmethod, "SSLv23_method") == 0) {
method = SSLv23_method();
} else if (strcmp(*sslmethod, "SSLv23_server_method") == 0) {
Expand Down Expand Up @@ -346,7 +338,9 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
// 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.
// SSLv3 is disabled because it's susceptible to downgrade attacks (POODLE.)
SSL_CTX_set_options(sc->ctx_, SSL_OP_NO_SSLv2);
SSL_CTX_set_options(sc->ctx_, SSL_OP_NO_SSLv3);

// SSL session cache configuration
SSL_CTX_set_session_cache_mode(sc->ctx_,
Expand Down
6 changes: 4 additions & 2 deletions src/node_crypto_clienthello.cc
Expand Up @@ -61,13 +61,15 @@ void ClientHelloParser::ParseHeader(const uint8_t* data, size_t avail) {

// 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)
if (data[body_offset_ + 4] != 0x03 ||
data[body_offset_ + 5] < 0x01 ||
data[body_offset_ + 5] > 0x03) {
goto fail;
}

if (data[body_offset_] == kClientHello) {
if (state_ == kTLSHeader) {
Expand Down
52 changes: 52 additions & 0 deletions test/parallel/test-tls-no-sslv23.js
@@ -0,0 +1,52 @@
if (!process.versions.openssl) {
console.error('Skipping because node compiled without OpenSSL.');
process.exit(0);
}

var common = require('../common');
var assert = require('assert');
var tls = require('tls');

assert.throws(function() {
tls.createSecureContext({ secureProtocol: 'blargh' });
}, /Unknown method/);

assert.throws(function() {
tls.createSecureContext({ secureProtocol: 'SSLv2_method' });
}, /SSLv2 methods disabled/);

assert.throws(function() {
tls.createSecureContext({ secureProtocol: 'SSLv2_client_method' });
}, /SSLv2 methods disabled/);

assert.throws(function() {
tls.createSecureContext({ secureProtocol: 'SSLv2_server_method' });
}, /SSLv2 methods disabled/);

assert.throws(function() {
tls.createSecureContext({ secureProtocol: 'SSLv3_method' });
}, /SSLv3 methods disabled/);

assert.throws(function() {
tls.createSecureContext({ secureProtocol: 'SSLv3_client_method' });
}, /SSLv3 methods disabled/);

assert.throws(function() {
tls.createSecureContext({ secureProtocol: 'SSLv3_server_method' });
}, /SSLv3 methods disabled/);

// Note that SSLv2 and SSLv3 are disallowed but SSLv2_method and friends are
// still accepted. They are OpenSSL's way of saying that all known protocols
// are supported unless explicitly disabled (which we do for SSLv2 and SSLv3.)
tls.createSecureContext({ secureProtocol: 'SSLv23_method' });
Copy link
Member

Choose a reason for hiding this comment

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

var methods = []; methods.forEach(...)

Copy link
Member Author

Choose a reason for hiding this comment

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

I like the explicitness. The url tests use (humongous!) arrays and when something breaks, it's always a pain to figure out what.

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed with @bnoordhuis

tls.createSecureContext({ secureProtocol: 'SSLv23_client_method' });
tls.createSecureContext({ secureProtocol: 'SSLv23_server_method' });
tls.createSecureContext({ secureProtocol: 'TLSv1_method' });
tls.createSecureContext({ secureProtocol: 'TLSv1_client_method' });
tls.createSecureContext({ secureProtocol: 'TLSv1_server_method' });
tls.createSecureContext({ secureProtocol: 'TLSv1_1_method' });
tls.createSecureContext({ secureProtocol: 'TLSv1_1_client_method' });
tls.createSecureContext({ secureProtocol: 'TLSv1_1_server_method' });
tls.createSecureContext({ secureProtocol: 'TLSv1_2_method' });
tls.createSecureContext({ secureProtocol: 'TLSv1_2_client_method' });
tls.createSecureContext({ secureProtocol: 'TLSv1_2_server_method' });
34 changes: 34 additions & 0 deletions test/parallel/test-tls-no-sslv3.js
@@ -0,0 +1,34 @@
if (!process.versions.openssl) {
console.error('Skipping because node compiled without OpenSSL.');
process.exit(0);
}

var common = require('../common');
var assert = require('assert');
var fs = require('fs');
var spawn = require('child_process').spawn;
var tls = require('tls');

var cert = fs.readFileSync(common.fixturesDir + '/test_cert.pem');
var key = fs.readFileSync(common.fixturesDir + '/test_key.pem');
var server = tls.createServer({ cert: cert, key: key }, assert.fail);

server.listen(common.PORT, '127.0.0.1', function() {
var address = this.address().address + ':' + this.address().port;
var args = ['s_client',
'-no_ssl2',
'-ssl3',
'-no_tls1',
'-no_tls1_1',
'-no_tls1_2',
'-connect', address];
var client = spawn(common.opensslCli, args, { stdio: 'inherit' });
client.once('exit', common.mustCall(function(exitCode) {
assert.equal(exitCode, 1);
server.close();
}));
});

server.once('clientError', common.mustCall(function(err, conn) {
assert(/SSL3_GET_CLIENT_HELLO:wrong version number/.test(err.message));
}));