From a15ea5d7ca1cf0f48ce1f2be62b1cd446a50b06d Mon Sep 17 00:00:00 2001
From: Brian White <mscdex@mscdex.net>
Date: Wed, 27 Jun 2018 03:08:06 -0400
Subject: [PATCH] tls: throw error on bad ciphers option

PR-URL: https://github.com/nodejs/node/pull/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>
---
 src/node_crypto.cc                          |  8 +++++++-
 test/parallel/test-tls-handshake-error.js   | 19 +++++++-----------
 test/parallel/test-tls-set-ciphers-error.js | 22 +++++++++++++++++++++
 test/sequential/test-tls-connect.js         | 19 ++++++++----------
 4 files changed, 44 insertions(+), 24 deletions(-)
 create mode 100644 test/parallel/test-tls-set-ciphers-error.js

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 818311eddeab62..31abeec6f3d7d8 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -904,7 +904,13 @@ void SecureContext::SetCiphers(const FunctionCallbackInfo<Value>& args) {
   THROW_AND_RETURN_IF_NOT_STRING(env, args[0], "Ciphers");
 
   const node::Utf8Value ciphers(args.GetIsolate(), args[0]);
-  SSL_CTX_set_cipher_list(sc->ctx_.get(), *ciphers);
+  if (!SSL_CTX_set_cipher_list(sc->ctx_.get(), *ciphers)) {
+    unsigned long err = ERR_get_error();  // NOLINT(runtime/int)
+    if (!err) {
+      return env->ThrowError("Failed to set ciphers");
+    }
+    return ThrowCryptoError(env, err);
+  }
 }
 
 
diff --git a/test/parallel/test-tls-handshake-error.js b/test/parallel/test-tls-handshake-error.js
index 776004cf228f31..2aef1adcc38c1f 100644
--- a/test/parallel/test-tls-handshake-error.js
+++ b/test/parallel/test-tls-handshake-error.js
@@ -16,17 +16,12 @@ const server = tls.createServer({
   rejectUnauthorized: true
 }, function(c) {
 }).listen(0, common.mustCall(function() {
-  const c = tls.connect({
-    port: this.address().port,
-    ciphers: 'RC4'
-  }, common.mustNotCall());
+  assert.throws(() => {
+    tls.connect({
+      port: this.address().port,
+      ciphers: 'RC4'
+    }, common.mustNotCall());
+  }, /no cipher match/i);
 
-  c.on('error', common.mustCall(function(err) {
-    assert.notStrictEqual(err.code, 'ECONNRESET');
-  }));
-
-  c.on('close', common.mustCall(function(err) {
-    assert.ok(err);
-    server.close();
-  }));
+  server.close();
 }));
diff --git a/test/parallel/test-tls-set-ciphers-error.js b/test/parallel/test-tls-set-ciphers-error.js
new file mode 100644
index 00000000000000..5ef08dda041f01
--- /dev/null
+++ b/test/parallel/test-tls-set-ciphers-error.js
@@ -0,0 +1,22 @@
+'use strict';
+const common = require('../common');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const tls = require('tls');
+const fixtures = require('../common/fixtures');
+
+{
+  const options = {
+    key: fixtures.readKey('agent2-key.pem'),
+    cert: fixtures.readKey('agent2-cert.pem'),
+    ciphers: 'aes256-sha'
+  };
+  assert.throws(() => tls.createServer(options, common.mustNotCall()),
+                /no cipher match/i);
+  options.ciphers = 'FOOBARBAZ';
+  assert.throws(() => tls.createServer(options, common.mustNotCall()),
+                /no cipher match/i);
+}
diff --git a/test/sequential/test-tls-connect.js b/test/sequential/test-tls-connect.js
index 2f1fb32323bdcb..291747aea77b49 100644
--- a/test/sequential/test-tls-connect.js
+++ b/test/sequential/test-tls-connect.js
@@ -50,15 +50,12 @@ const tls = require('tls');
   const cert = fixtures.readSync('test_cert.pem');
   const key = fixtures.readSync('test_key.pem');
 
-  const conn = tls.connect({
-    cert: cert,
-    key: key,
-    port: common.PORT,
-    ciphers: 'rick-128-roll'
-  }, common.mustNotCall());
-
-  conn.on(
-    'error',
-    common.mustCall((e) => { assert.strictEqual(e.code, 'ECONNREFUSED'); })
-  );
+  assert.throws(() => {
+    tls.connect({
+      cert: cert,
+      key: key,
+      port: common.PORT,
+      ciphers: 'rick-128-roll'
+    }, common.mustNotCall());
+  }, /no cipher match/i);
 }