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 <fedor@indutny.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

5 files changed, 99 insertions, 14 deletions

diff --git a/deps/openssl/openssl.gyp b/deps/openssl/openssl.gyp
index 93f7f740ee..6b644ab253 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 26ac54bf36..c088fe25db 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -288,6 +288,10 @@ 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) {
@@ -295,23 +299,11 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& 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<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_,
diff --git a/src/node_crypto_clienthello.cc b/src/node_crypto_clienthello.cc
index 34507858c9..8fbc3161f8 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 0000000000..64f5206720
--- /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 0000000000..8bdc4c7423
--- /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));
+}));