Skip to content

Commit 5165d71

Browse files
bnoordhuisbnoordhuis
committed
build,src: remove sslv3 support
SSLv3 is susceptible to downgrade attacks. Provide secure defaults, disable v3 protocol support entirely. PR-URL: nodejs#315 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
1 parent 635337f commit 5165d71

5 files changed

Lines changed: 99 additions & 14 deletions

File tree

‎deps/openssl/openssl.gyp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,6 +1098,9 @@
10981098
# twenty years now.
10991099
'OPENSSL_NO_SSL2',
11001100

1101+
# SSLv3 is susceptible to downgrade attacks (POODLE.)
1102+
'OPENSSL_NO_SSL3',
1103+
11011104
# Heartbeat is a TLS extension, that couldn't be turned off or
11021105
# asked to be not advertised. Unfortunately this is unacceptable for
11031106
# Microsoft's IIS, which seems to be ignoring whole ClientHello after

‎src/node_crypto.cc‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -288,30 +288,22 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
288288
if (args.Length() == 1 && args[0]->IsString()) {
289289
const node::Utf8Value sslmethod(env->isolate(), args[0]);
290290

291+
// Note that SSLv2 and SSLv3 are disallowed but SSLv2_method and friends
292+
// are still accepted. They are OpenSSL's way of saying that all known
293+
// protocols are supported unless explicitly disabled (which we do below
294+
// for SSLv2 and SSLv3.)
291295
if (strcmp(*sslmethod, "SSLv2_method") == 0) {
292296
return env->ThrowError("SSLv2 methods disabled");
293297
} else if (strcmp(*sslmethod, "SSLv2_server_method") == 0) {
294298
return env->ThrowError("SSLv2 methods disabled");
295299
} else if (strcmp(*sslmethod, "SSLv2_client_method") == 0) {
296300
return env->ThrowError("SSLv2 methods disabled");
297301
} else if (strcmp(*sslmethod, "SSLv3_method") == 0) {
298-
#ifndef OPENSSL_NO_SSL3
299-
method = SSLv3_method();
300-
#else
301302
return env->ThrowError("SSLv3 methods disabled");
302-
#endif
303303
} else if (strcmp(*sslmethod, "SSLv3_server_method") == 0) {
304-
#ifndef OPENSSL_NO_SSL3
305-
method = SSLv3_server_method();
306-
#else
307304
return env->ThrowError("SSLv3 methods disabled");
308-
#endif
309305
} else if (strcmp(*sslmethod, "SSLv3_client_method") == 0) {
310-
#ifndef OPENSSL_NO_SSL3
311-
method = SSLv3_client_method();
312-
#else
313306
return env->ThrowError("SSLv3 methods disabled");
314-
#endif
315307
} else if (strcmp(*sslmethod, "SSLv23_method") == 0) {
316308
method = SSLv23_method();
317309
} else if (strcmp(*sslmethod, "SSLv23_server_method") == 0) {
@@ -346,7 +338,9 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
346338
// Disable SSLv2 in the case when method == SSLv23_method() and the
347339
// cipher list contains SSLv2 ciphers (not the default, should be rare.)
348340
// The bundled OpenSSL doesn't have SSLv2 support but the system OpenSSL may.
341+
// SSLv3 is disabled because it's susceptible to downgrade attacks (POODLE.)
349342
SSL_CTX_set_options(sc->ctx_, SSL_OP_NO_SSLv2);
343+
SSL_CTX_set_options(sc->ctx_, SSL_OP_NO_SSLv3);
350344

351345
// SSL session cache configuration
352346
SSL_CTX_set_session_cache_mode(sc->ctx_,

‎src/node_crypto_clienthello.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,15 @@ void ClientHelloParser::ParseHeader(const uint8_t* data, size_t avail) {
6161

6262
// Check hello protocol version. Protocol tuples that we know about:
6363
//
64-
// (3,0) SSL v3.0
6564
// (3,1) TLS v1.0
6665
// (3,2) TLS v1.1
6766
// (3,3) TLS v1.2
6867
//
69-
if (data[body_offset_ + 4] != 0x03 || data[body_offset_ + 5] > 0x03)
68+
if (data[body_offset_ + 4] != 0x03 ||
69+
data[body_offset_ + 5] < 0x01 ||
70+
data[body_offset_ + 5] > 0x03) {
7071
goto fail;
72+
}
7173

7274
if (data[body_offset_] == kClientHello) {
7375
if (state_ == kTLSHeader) {

‎test/parallel/test-tls-no-sslv23.js‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
if (!process.versions.openssl) {
2+
console.error('Skipping because node compiled without OpenSSL.');
3+
process.exit(0);
4+
}
5+
6+
var common = require('../common');
7+
var assert = require('assert');
8+
var tls = require('tls');
9+
10+
assert.throws(function() {
11+
tls.createSecureContext({ secureProtocol: 'blargh' });
12+
}, /Unknown method/);
13+
14+
assert.throws(function() {
15+
tls.createSecureContext({ secureProtocol: 'SSLv2_method' });
16+
}, /SSLv2 methods disabled/);
17+
18+
assert.throws(function() {
19+
tls.createSecureContext({ secureProtocol: 'SSLv2_client_method' });
20+
}, /SSLv2 methods disabled/);
21+
22+
assert.throws(function() {
23+
tls.createSecureContext({ secureProtocol: 'SSLv2_server_method' });
24+
}, /SSLv2 methods disabled/);
25+
26+
assert.throws(function() {
27+
tls.createSecureContext({ secureProtocol: 'SSLv3_method' });
28+
}, /SSLv3 methods disabled/);
29+
30+
assert.throws(function() {
31+
tls.createSecureContext({ secureProtocol: 'SSLv3_client_method' });
32+
}, /SSLv3 methods disabled/);
33+
34+
assert.throws(function() {
35+
tls.createSecureContext({ secureProtocol: 'SSLv3_server_method' });
36+
}, /SSLv3 methods disabled/);
37+
38+
// Note that SSLv2 and SSLv3 are disallowed but SSLv2_method and friends are
39+
// still accepted. They are OpenSSL's way of saying that all known protocols
40+
// are supported unless explicitly disabled (which we do for SSLv2 and SSLv3.)
41+
tls.createSecureContext({ secureProtocol: 'SSLv23_method' });
42+
tls.createSecureContext({ secureProtocol: 'SSLv23_client_method' });
43+
tls.createSecureContext({ secureProtocol: 'SSLv23_server_method' });
44+
tls.createSecureContext({ secureProtocol: 'TLSv1_method' });
45+
tls.createSecureContext({ secureProtocol: 'TLSv1_client_method' });
46+
tls.createSecureContext({ secureProtocol: 'TLSv1_server_method' });
47+
tls.createSecureContext({ secureProtocol: 'TLSv1_1_method' });
48+
tls.createSecureContext({ secureProtocol: 'TLSv1_1_client_method' });
49+
tls.createSecureContext({ secureProtocol: 'TLSv1_1_server_method' });
50+
tls.createSecureContext({ secureProtocol: 'TLSv1_2_method' });
51+
tls.createSecureContext({ secureProtocol: 'TLSv1_2_client_method' });
52+
tls.createSecureContext({ secureProtocol: 'TLSv1_2_server_method' });

‎test/parallel/test-tls-no-sslv3.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
if (!process.versions.openssl) {
2+
console.error('Skipping because node compiled without OpenSSL.');
3+
process.exit(0);
4+
}
5+
6+
var common = require('../common');
7+
var assert = require('assert');
8+
var fs = require('fs');
9+
var spawn = require('child_process').spawn;
10+
var tls = require('tls');
11+
12+
var cert = fs.readFileSync(common.fixturesDir + '/test_cert.pem');
13+
var key = fs.readFileSync(common.fixturesDir + '/test_key.pem');
14+
var server = tls.createServer({ cert: cert, key: key }, assert.fail);
15+
16+
server.listen(common.PORT, '127.0.0.1', function() {
17+
var address = this.address().address + ':' + this.address().port;
18+
var args = ['s_client',
19+
'-no_ssl2',
20+
'-ssl3',
21+
'-no_tls1',
22+
'-no_tls1_1',
23+
'-no_tls1_2',
24+
'-connect', address];
25+
var client = spawn(common.opensslCli, args, { stdio: 'inherit' });
26+
client.once('exit', common.mustCall(function(exitCode) {
27+
assert.equal(exitCode, 1);
28+
server.close();
29+
}));
30+
});
31+
32+
server.once('clientError', common.mustCall(function(err, conn) {
33+
assert(/SSL3_GET_CLIENT_HELLO:wrong version number/.test(err.message));
34+
}));

0 commit comments

Comments
 (0)