From ce0f18fd4eb89059107e379cc1aed18b416a70de Mon Sep 17 00:00:00 2001 From: Roga Pria Sembada Date: Tue, 5 Sep 2017 01:49:28 +0700 Subject: [PATCH] crypto: support multiple ECDH curves and auto MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/15206 Ref: https://github.com/nodejs/node/issues/15054 Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis Reviewed-By: Ruben Bridgewater Reviewed-By: Tobias Nießen --- doc/api/tls.md | 16 ++--- src/node_crypto.cc | 18 ++---- test/parallel/test-tls-ecdh-auto.js | 64 ++++++++++++++++++++ test/parallel/test-tls-ecdh-multiple.js | 80 +++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 19 deletions(-) create mode 100644 test/parallel/test-tls-ecdh-auto.js create mode 100644 test/parallel/test-tls-ecdh-multiple.js diff --git a/doc/api/tls.md b/doc/api/tls.md index ebcf85438f..9979895a75 100644 --- 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 index d5038b7ab5..0066d9592b 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -923,20 +923,14 @@ void SecureContext::SetECDHCurve(const FunctionCallbackInfo& 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 new file mode 100644 index 0000000000..eaa7e1da6d --- /dev/null +++ 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 new file mode 100644 index 0000000000..3ea17a99e9 --- /dev/null +++ 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/); + }); +});