crypto: support multiple ECDH curves and auto

rogaps · jasnell · commit bd07574718bc · 2017-09-20T07:52:59.000-07:00
Using SSL_CTX_set1_curves_list() (OpenSSL 1.0.2+), this allows to set colon separated ECDH curve names in SecureContext's ecdhCurve option. The option can also be set to "auto" to select the curve automatically from list built in OpenSSL by enabling SSL_CTX_set_ecdh_auto() (OpenSSL 1.0.2+). PR-URL: #15206 Ref: #15054 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
diff --git a/doc/api/tls.md b/doc/api/tls.md
@@ -101,8 +101,8 @@ openssl dhparam -outform PEM -out dhparam.pem 2048
 
 If using Perfect Forward Secrecy using `ECDHE`, Diffie-Hellman parameters are
 not required and a default ECDHE curve will be used. The `ecdhCurve` property
-can be used when creating a TLS Server to specify the name of an alternative
-curve to use, see [`tls.createServer()`] for more info.
+can be used when creating a TLS Server to specify the list of names of supported
+curves to use, see [`tls.createServer()`] for more info.
 
 ### ALPN, NPN and SNI
 
@@ -984,11 +984,13 @@ changes:
     preferences instead of the client's. When `true`, causes
     `SSL_OP_CIPHER_SERVER_PREFERENCE` to be set in `secureOptions`, see
     [OpenSSL Options][] for more information.
-  * `ecdhCurve` {string} A string describing a named curve to use for ECDH key
-    agreement or `false` to disable ECDH. Defaults to
-    [`tls.DEFAULT_ECDH_CURVE`].  Use [`crypto.getCurves()`][] to obtain a list
-    of available curve names. On recent releases, `openssl ecparam -list_curves`
-    will also display the name and description of each available elliptic curve.
+  * `ecdhCurve` {string} A string describing a named curve or a colon separated
+    list of curve NIDs or names, for example `P-521:P-384:P-256`, to use for
+    ECDH key agreement, or `false` to disable ECDH. Set to `auto` to select the
+    curve automatically. Defaults to [`tls.DEFAULT_ECDH_CURVE`]. Use
+    [`crypto.getCurves()`][] to obtain a list of available curve names. On
+    recent releases, `openssl ecparam -list_curves` will also display the name
+    and description of each available elliptic curve.
   * `dhparam` {string|Buffer} Diffie Hellman parameters, required for
     [Perfect Forward Secrecy][]. Use `openssl dhparam` to create the parameters.
     The key length must be greater than or equal to 1024 bits, otherwise an
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -923,20 +923,14 @@ void SecureContext::SetECDHCurve(const FunctionCallbackInfo<Value>& args) {
 
   node::Utf8Value curve(env->isolate(), args[0]);
 
-  int nid = OBJ_sn2nid(*curve);
-
-  if (nid == NID_undef)
-    return env->ThrowTypeError("First argument should be a valid curve name");
-
-  EC_KEY* ecdh = EC_KEY_new_by_curve_name(nid);
-
-  if (ecdh == nullptr)
-    return env->ThrowTypeError("First argument should be a valid curve name");
-
   SSL_CTX_set_options(sc->ctx_, SSL_OP_SINGLE_ECDH_USE);
-  SSL_CTX_set_tmp_ecdh(sc->ctx_, ecdh);
+  SSL_CTX_set_ecdh_auto(sc->ctx_, 1);
+
+  if (strcmp(*curve, "auto") == 0)
+    return;
 
-  EC_KEY_free(ecdh);
+  if (!SSL_CTX_set1_curves_list(sc->ctx_, *curve))
+    return env->ThrowError("Failed to set ECDH curve");
 }
 
 
diff --git a/test/parallel/test-tls-ecdh-auto.js b/test/parallel/test-tls-ecdh-auto.js
@@ -0,0 +1,64 @@
+'use strict';
+const common = require('../common');
+
+// This test ensures that the value "auto" on ecdhCurve option is
+// supported to enable automatic curve selection in TLS server.
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+if (!common.opensslCli)
+  common.skip('missing openssl-cli');
+
+const assert = require('assert');
+const tls = require('tls');
+const spawn = require('child_process').spawn;
+const fixtures = require('../common/fixtures');
+
+function loadPEM(n) {
+  return fixtures.readKey(`${n}.pem`);
+}
+
+const options = {
+  key: loadPEM('agent2-key'),
+  cert: loadPEM('agent2-cert'),
+  ciphers: '-ALL:ECDHE-RSA-AES128-SHA256',
+  ecdhCurve: 'auto'
+};
+
+const reply = 'I AM THE WALRUS'; // something recognizable
+
+const server = tls.createServer(options, function(conn) {
+  conn.end(reply);
+});
+
+let gotReply = false;
+
+server.listen(0, function() {
+  const args = ['s_client',
+                '-cipher', `${options.ciphers}`,
+                '-connect', `127.0.0.1:${this.address().port}`];
+
+  // for the performance and stability issue in s_client on Windows
+  if (common.isWindows)
+    args.push('-no_rand_screen');
+
+  const client = spawn(common.opensslCli, args);
+
+  client.stdout.on('data', function(data) {
+    const message = data.toString();
+    if (message.includes(reply))
+      gotReply = true;
+  });
+
+  client.on('exit', function(code) {
+    assert.strictEqual(0, code);
+    server.close();
+  });
+
+  client.on('error', assert.ifError);
+});
+
+process.on('exit', function() {
+  assert.ok(gotReply);
+});
diff --git a/test/parallel/test-tls-ecdh-multiple.js b/test/parallel/test-tls-ecdh-multiple.js
@@ -0,0 +1,80 @@
+'use strict';
+const common = require('../common');
+
+// This test ensures that ecdhCurve option of TLS server supports colon
+// separated ECDH curve names as value.
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+if (!common.opensslCli)
+  common.skip('missing openssl-cli');
+
+const assert = require('assert');
+const tls = require('tls');
+const spawn = require('child_process').spawn;
+const fixtures = require('../common/fixtures');
+
+function loadPEM(n) {
+  return fixtures.readKey(`${n}.pem`);
+}
+
+const options = {
+  key: loadPEM('agent2-key'),
+  cert: loadPEM('agent2-cert'),
+  ciphers: '-ALL:ECDHE-RSA-AES128-SHA256',
+  ecdhCurve: 'secp256k1:prime256v1:secp521r1'
+};
+
+const reply = 'I AM THE WALRUS'; // something recognizable
+
+const server = tls.createServer(options, function(conn) {
+  conn.end(reply);
+});
+
+let gotReply = false;
+
+server.listen(0, function() {
+  const args = ['s_client',
+                '-cipher', `${options.ciphers}`,
+                '-connect', `127.0.0.1:${this.address().port}`];
+
+  // for the performance and stability issue in s_client on Windows
+  if (common.isWindows)
+    args.push('-no_rand_screen');
+
+  const client = spawn(common.opensslCli, args);
+
+  client.stdout.on('data', function(data) {
+    const message = data.toString();
+    if (message.includes(reply))
+      gotReply = true;
+  });
+
+  client.on('exit', function(code) {
+    assert.strictEqual(0, code);
+    server.close();
+  });
+
+  client.on('error', assert.ifError);
+});
+
+process.on('exit', function() {
+  assert.ok(gotReply);
+
+  // Some of unsupported curves
+  const unsupportedCurves = [
+    'wap-wsg-idm-ecid-wtls1',
+    'c2pnb163v1',
+    'prime192v3'
+  ];
+
+  // Brainpool is not supported in FIPS mode
+  if (common.hasFipsCrypto)
+    unsupportedCurves.push('brainpoolP256r1');
+
+  unsupportedCurves.forEach((ecdhCurve) => {
+    assert.throws(() => tls.createServer({ ecdhCurve: ecdhCurve }),
+                  /Error: Failed to set ECDH curve/);
+  });
+});
