From 6c417a5fa22b02e0dcf76c14b213f953d7c26c84 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 22 Jun 2019 14:06:11 -0600 Subject: [PATCH 1/4] test: move non-pummel crypto DH tests to parallel Some parts of pummel/test-crypto-dh.js will be just fine in parallel/test-crypto-dh.js. Move them there. --- test/parallel/test-crypto-dh.js | 27 +++++++++++++++++++++++++++ test/pummel/test-crypto-dh.js | 27 --------------------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/test/parallel/test-crypto-dh.js b/test/parallel/test-crypto-dh.js index 69d18f458e60bd..1fc8564ca16fcd 100644 --- a/test/parallel/test-crypto-dh.js +++ b/test/parallel/test-crypto-dh.js @@ -388,3 +388,30 @@ common.expectsError( message: 'The "curve" argument must be of type string. ' + 'Received type undefined' }); + +assert.throws( + function() { + crypto.getDiffieHellman('unknown-group'); + }, + /^Error: Unknown group$/, + 'crypto.getDiffieHellman(\'unknown-group\') ' + + 'failed to throw the expected error.' +); +assert.throws( + function() { + crypto.getDiffieHellman('modp1').setPrivateKey(''); + }, + new RegExp('^TypeError: crypto\\.getDiffieHellman\\(\\.\\.\\.\\)\\.' + + 'setPrivateKey is not a function$'), + 'crypto.getDiffieHellman(\'modp1\').setPrivateKey(\'\') ' + + 'failed to throw the expected error.' +); +assert.throws( + function() { + crypto.getDiffieHellman('modp1').setPublicKey(''); + }, + new RegExp('^TypeError: crypto\\.getDiffieHellman\\(\\.\\.\\.\\)\\.' + + 'setPublicKey is not a function$'), + 'crypto.getDiffieHellman(\'modp1\').setPublicKey(\'\') ' + + 'failed to throw the expected error.' +); diff --git a/test/pummel/test-crypto-dh.js b/test/pummel/test-crypto-dh.js index f64f982c0e1100..a88e105a8aa0b7 100644 --- a/test/pummel/test-crypto-dh.js +++ b/test/pummel/test-crypto-dh.js @@ -27,33 +27,6 @@ if (!common.hasCrypto) const assert = require('assert'); const crypto = require('crypto'); -assert.throws( - function() { - crypto.getDiffieHellman('unknown-group'); - }, - /^Error: Unknown group$/, - 'crypto.getDiffieHellman(\'unknown-group\') ' + - 'failed to throw the expected error.' -); -assert.throws( - function() { - crypto.getDiffieHellman('modp1').setPrivateKey(''); - }, - new RegExp('^TypeError: crypto\\.getDiffieHellman\\(\\.\\.\\.\\)\\.' + - 'setPrivateKey is not a function$'), - 'crypto.getDiffieHellman(\'modp1\').setPrivateKey(\'\') ' + - 'failed to throw the expected error.' -); -assert.throws( - function() { - crypto.getDiffieHellman('modp1').setPublicKey(''); - }, - new RegExp('^TypeError: crypto\\.getDiffieHellman\\(\\.\\.\\.\\)\\.' + - 'setPublicKey is not a function$'), - 'crypto.getDiffieHellman(\'modp1\').setPublicKey(\'\') ' + - 'failed to throw the expected error.' -); - const hashes = { modp1: '630e9acd2cc63f7e80d8507624ba60ac0757201a', modp2: '18f7aa964484137f57bca64b21917a385b6a0b60', From 6a4c29bdbb695eff52578061cec23772664b7c62 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 22 Jun 2019 15:38:22 -0600 Subject: [PATCH 2/4] test: split pummel crypto dh test into two separate tests Split test-crypto-dh into two tests so that it does not time out in CI. With a recent OpenSSL upgrade, getDiffieHellman() is much slower than before. --- ...st-crypto-dh.js => test-crypto-dh-hash.js} | 13 ------ test/pummel/test-crypto-dh-keys.js | 42 +++++++++++++++++++ 2 files changed, 42 insertions(+), 13 deletions(-) rename test/pummel/{test-crypto-dh.js => test-crypto-dh-hash.js} (82%) create mode 100644 test/pummel/test-crypto-dh-keys.js diff --git a/test/pummel/test-crypto-dh.js b/test/pummel/test-crypto-dh-hash.js similarity index 82% rename from test/pummel/test-crypto-dh.js rename to test/pummel/test-crypto-dh-hash.js index a88e105a8aa0b7..ffe11d1c12491b 100644 --- a/test/pummel/test-crypto-dh.js +++ b/test/pummel/test-crypto-dh-hash.js @@ -47,16 +47,3 @@ for (const name in hashes) { assert.strictEqual(hash1, hash2); assert.strictEqual(group.getGenerator('hex'), '02'); } - -for (const name in hashes) { - // modp1 is 768 bits, FIPS requires >= 1024 - if (name === 'modp1' && common.hasFipsCrypto) - continue; - const group1 = crypto.getDiffieHellman(name); - const group2 = crypto.getDiffieHellman(name); - group1.generateKeys(); - group2.generateKeys(); - const key1 = group1.computeSecret(group2.getPublicKey()); - const key2 = group2.computeSecret(group1.getPublicKey()); - assert.deepStrictEqual(key1, key2); -} diff --git a/test/pummel/test-crypto-dh-keys.js b/test/pummel/test-crypto-dh-keys.js new file mode 100644 index 00000000000000..0a9b1792b6f4c2 --- /dev/null +++ b/test/pummel/test-crypto-dh-keys.js @@ -0,0 +1,42 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('node compiled without OpenSSL.'); + +const assert = require('assert'); +const crypto = require('crypto'); + +[ 'modp1', 'modp2', 'modp5', 'modp14', 'modp15', 'modp16', 'modp17', 'modp18' ] +.forEach((name) => { + // modp1 is 768 bits, FIPS requires >= 1024 + if (name === 'modp1' && common.hasFipsCrypto) + return; + const group1 = crypto.getDiffieHellman(name); + const group2 = crypto.getDiffieHellman(name); + group1.generateKeys(); + group2.generateKeys(); + const key1 = group1.computeSecret(group2.getPublicKey()); + const key2 = group2.computeSecret(group1.getPublicKey()); + assert.deepStrictEqual(key1, key2); +}); From caceabd828d28f627ec7fd711bcb876d76cd20df Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 22 Jun 2019 15:57:14 -0600 Subject: [PATCH 3/4] test: make test-dh-regr more efficient where possible test-dh-regr is timing out in CI because crypto.createDiffieHellman() is considerably slower after an OpenSSL upgrade. This PR modifies the change from 11ad744a92374ad71730cbfb7abea71fda0abb74 which made the test FIPS-compatible. When not in FIPS mode, the test will use a shorter key which will enable it to run much faster. --- test/pummel/test-dh-regr.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/pummel/test-dh-regr.js b/test/pummel/test-dh-regr.js index ff964ec13a47c6..f2da464bbbfbd7 100644 --- a/test/pummel/test-dh-regr.js +++ b/test/pummel/test-dh-regr.js @@ -27,7 +27,11 @@ if (!common.hasCrypto) const assert = require('assert'); const crypto = require('crypto'); -const p = crypto.createDiffieHellman(1024).getPrime(); +// FIPS requires length >= 1024 but we use 256 in this test to keep it from +// taking too long and timing out in CI. +const length = common.hasFipsCrypto ? 1024 : 256; + +const p = crypto.createDiffieHellman(length).getPrime(); for (let i = 0; i < 2000; i++) { const a = crypto.createDiffieHellman(p); From f1c93696d1e6da0ab6f84b49ca98699719879a77 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 23 Jun 2019 01:35:46 -0700 Subject: [PATCH 4/4] fixup! test: split pummel crypto dh test into two separate tests --- test/pummel/test-crypto-dh-keys.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/pummel/test-crypto-dh-keys.js b/test/pummel/test-crypto-dh-keys.js index 0a9b1792b6f4c2..f8f990b5c5ee4a 100644 --- a/test/pummel/test-crypto-dh-keys.js +++ b/test/pummel/test-crypto-dh-keys.js @@ -27,7 +27,7 @@ if (!common.hasCrypto) const assert = require('assert'); const crypto = require('crypto'); -[ 'modp1', 'modp2', 'modp5', 'modp14', 'modp15', 'modp16', 'modp17', 'modp18' ] +[ 'modp1', 'modp2', 'modp5', 'modp14', 'modp15', 'modp16', 'modp17' ] .forEach((name) => { // modp1 is 768 bits, FIPS requires >= 1024 if (name === 'modp1' && common.hasFipsCrypto)