From 5165d71048a0cc20c319fcd62ac4c50465ff0414 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 13 Jan 2015 00:45:31 +0100 Subject: [PATCH] build,src: remove sslv3 support SSLv3 is susceptible to downgrade attacks. Provide secure defaults, disable v3 protocol support entirely. PR-URL: https://github.com/iojs/io.js/pull/315 Reviewed-By: Fedor Indutny Reviewed-By: Trevor Norris --- deps/openssl/openssl.gyp | 3 ++ src/node_crypto.cc | 18 ++++------ src/node_crypto_clienthello.cc | 6 ++-- test/parallel/test-tls-no-sslv23.js | 52 +++++++++++++++++++++++++++++ test/parallel/test-tls-no-sslv3.js | 34 +++++++++++++++++++ 5 files changed, 99 insertions(+), 14 deletions(-) create mode 100644 test/parallel/test-tls-no-sslv23.js create mode 100644 test/parallel/test-tls-no-sslv3.js diff --git a/deps/openssl/openssl.gyp b/deps/openssl/openssl.gyp index 93f7f740ee1757..6b644ab253906b 100644 --- a/deps/openssl/openssl.gyp +++ b/deps/openssl/openssl.gyp @@ -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 diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 26ac54bf368c27..c088fe25db0d98 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -288,6 +288,10 @@ void SecureContext::Init(const FunctionCallbackInfo& 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) { @@ -295,23 +299,11 @@ void SecureContext::Init(const FunctionCallbackInfo& args) { } 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) { @@ -346,7 +338,9 @@ void SecureContext::Init(const FunctionCallbackInfo& 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_, diff --git a/src/node_crypto_clienthello.cc b/src/node_crypto_clienthello.cc index 34507858c9f04b..8fbc3161f89969 100644 --- a/src/node_crypto_clienthello.cc +++ b/src/node_crypto_clienthello.cc @@ -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) { diff --git a/test/parallel/test-tls-no-sslv23.js b/test/parallel/test-tls-no-sslv23.js new file mode 100644 index 00000000000000..64f5206720510d --- /dev/null +++ b/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' }); +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' }); diff --git a/test/parallel/test-tls-no-sslv3.js b/test/parallel/test-tls-no-sslv3.js new file mode 100644 index 00000000000000..8bdc4c74232c9e --- /dev/null +++ b/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)); +}));