Skip to content

Commit a15ea5d

Browse files
committed
tls: throw error on bad ciphers option
PR-URL: #21557 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
1 parent c267639 commit a15ea5d

File tree

4 files changed

+44
-24
lines changed

4 files changed

+44
-24
lines changed

src/node_crypto.cc

+7-1
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,13 @@ void SecureContext::SetCiphers(const FunctionCallbackInfo<Value>& args) {
904904
THROW_AND_RETURN_IF_NOT_STRING(env, args[0], "Ciphers");
905905

906906
const node::Utf8Value ciphers(args.GetIsolate(), args[0]);
907-
SSL_CTX_set_cipher_list(sc->ctx_.get(), *ciphers);
907+
if (!SSL_CTX_set_cipher_list(sc->ctx_.get(), *ciphers)) {
908+
unsigned long err = ERR_get_error(); // NOLINT(runtime/int)
909+
if (!err) {
910+
return env->ThrowError("Failed to set ciphers");
911+
}
912+
return ThrowCryptoError(env, err);
913+
}
908914
}
909915

910916

test/parallel/test-tls-handshake-error.js

+7-12
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,12 @@ const server = tls.createServer({
1616
rejectUnauthorized: true
1717
}, function(c) {
1818
}).listen(0, common.mustCall(function() {
19-
const c = tls.connect({
20-
port: this.address().port,
21-
ciphers: 'RC4'
22-
}, common.mustNotCall());
19+
assert.throws(() => {
20+
tls.connect({
21+
port: this.address().port,
22+
ciphers: 'RC4'
23+
}, common.mustNotCall());
24+
}, /no cipher match/i);
2325

24-
c.on('error', common.mustCall(function(err) {
25-
assert.notStrictEqual(err.code, 'ECONNRESET');
26-
}));
27-
28-
c.on('close', common.mustCall(function(err) {
29-
assert.ok(err);
30-
server.close();
31-
}));
26+
server.close();
3227
}));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const assert = require('assert');
8+
const tls = require('tls');
9+
const fixtures = require('../common/fixtures');
10+
11+
{
12+
const options = {
13+
key: fixtures.readKey('agent2-key.pem'),
14+
cert: fixtures.readKey('agent2-cert.pem'),
15+
ciphers: 'aes256-sha'
16+
};
17+
assert.throws(() => tls.createServer(options, common.mustNotCall()),
18+
/no cipher match/i);
19+
options.ciphers = 'FOOBARBAZ';
20+
assert.throws(() => tls.createServer(options, common.mustNotCall()),
21+
/no cipher match/i);
22+
}

test/sequential/test-tls-connect.js

+8-11
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,12 @@ const tls = require('tls');
5050
const cert = fixtures.readSync('test_cert.pem');
5151
const key = fixtures.readSync('test_key.pem');
5252

53-
const conn = tls.connect({
54-
cert: cert,
55-
key: key,
56-
port: common.PORT,
57-
ciphers: 'rick-128-roll'
58-
}, common.mustNotCall());
59-
60-
conn.on(
61-
'error',
62-
common.mustCall((e) => { assert.strictEqual(e.code, 'ECONNREFUSED'); })
63-
);
53+
assert.throws(() => {
54+
tls.connect({
55+
cert: cert,
56+
key: key,
57+
port: common.PORT,
58+
ciphers: 'rick-128-roll'
59+
}, common.mustNotCall());
60+
}, /no cipher match/i);
6461
}

0 commit comments

Comments
 (0)