diff --git a/src/keys/ed25519.js b/src/keys/ed25519.js index ec5c7c5..e197730 100644 --- a/src/keys/ed25519.js +++ b/src/keys/ed25519.js @@ -7,30 +7,28 @@ exports.publicKeyLength = nacl.sign.publicKeyLength exports.privateKeyLength = nacl.sign.secretKeyLength exports.generateKey = function (callback) { - const done = (err, res) => setImmediate(() => { - callback(err, res) + setImmediate(() => { + let result + try { + result = nacl.sign.keyPair() + } catch (err) { + return callback(err) + } + callback(null, result) }) - - let keys - try { - keys = nacl.sign.keyPair() - } catch (err) { - return done(err) - } - done(null, keys) } // seed should be a 32 byte uint8array exports.generateKeyFromSeed = function (seed, callback) { - const done = (err, res) => setImmediate(() => callback(err, res)) - - let keys - try { - keys = nacl.sign.keyPair.fromSeed(seed) - } catch (err) { - return done(err) - } - done(null, keys) + setImmediate(() => { + let result + try { + result = nacl.sign.keyPair.fromSeed(seed) + } catch (err) { + return callback(err) + } + callback(null, result) + }) } exports.hashAndSign = function (key, msg, callback) { @@ -41,6 +39,13 @@ exports.hashAndSign = function (key, msg, callback) { exports.hashAndVerify = function (key, sig, msg, callback) { setImmediate(() => { - callback(null, nacl.sign.detached.verify(msg, sig, key)) + let result + try { + result = nacl.sign.detached.verify(msg, sig, key) + } catch (err) { + return callback(err) + } + + callback(null, result) }) } diff --git a/src/keys/index.js b/src/keys/index.js index 8405ebf..a8ce873 100644 --- a/src/keys/index.js +++ b/src/keys/index.js @@ -81,7 +81,13 @@ exports.marshalPublicKey = (key, type) => { // Converts a protobuf serialized private key into its // representative object exports.unmarshalPrivateKey = (buf, callback) => { - const decoded = keysPBM.PrivateKey.decode(buf) + let decoded + try { + decoded = keysPBM.PrivateKey.decode(buf) + } catch (err) { + return callback(err) + } + const data = decoded.Data switch (decoded.Type) { diff --git a/src/keys/rsa.js b/src/keys/rsa.js index 3d66f86..2da2678 100644 --- a/src/keys/rsa.js +++ b/src/keys/rsa.js @@ -9,30 +9,36 @@ const jwkToPem = require('pem-jwk').jwk2pem exports.utils = require('./rsa-utils') exports.generateKey = function (bits, callback) { - const done = (err, res) => setImmediate(() => callback(err, res)) + setImmediate(() => { + let result + try { + const key = keypair({ bits: bits }) + result = { + privateKey: pemToJwk(key.private), + publicKey: pemToJwk(key.public) + } + } catch (err) { + return callback(err) + } - let key - try { - key = keypair({ bits: bits }) - } catch (err) { - return done(err) - } - - done(null, { - privateKey: pemToJwk(key.private), - publicKey: pemToJwk(key.public) + callback(null, result) }) } // Takes a jwk key exports.unmarshalPrivateKey = function (key, callback) { - callback(null, { - privateKey: key, - publicKey: { - kty: key.kty, - n: key.n, - e: key.e + setImmediate(() => { + if (!key) { + return callback(new Error('Key is invalid')) } + callback(null, { + privateKey: key, + publicKey: { + kty: key.kty, + n: key.n, + e: key.e + } + }) }) } @@ -41,16 +47,33 @@ exports.getRandomValues = function (arr) { } exports.hashAndSign = function (key, msg, callback) { - const sign = crypto.createSign('RSA-SHA256') + setImmediate(() => { + let result + try { + const sign = crypto.createSign('RSA-SHA256') + sign.update(msg) + const pem = jwkToPem(key) + result = sign.sign(pem) + } catch (err) { + return callback(new Error('Key or message is invalid!: ' + err.message)) + } - sign.update(msg) - setImmediate(() => callback(null, sign.sign(jwkToPem(key)))) + callback(null, result) + }) } exports.hashAndVerify = function (key, sig, msg, callback) { - const verify = crypto.createVerify('RSA-SHA256') + setImmediate(() => { + let result + try { + const verify = crypto.createVerify('RSA-SHA256') + verify.update(msg) + const pem = jwkToPem(key) + result = verify.verify(pem, sig) + } catch (err) { + return callback(new Error('Key or message is invalid!:' + err.message)) + } - verify.update(msg) - - setImmediate(() => callback(null, verify.verify(jwkToPem(key), sig))) + callback(null, result) + }) } diff --git a/test/crypto.spec.js b/test/crypto.spec.js index d6df5f4..c2a22aa 100644 --- a/test/crypto.spec.js +++ b/test/crypto.spec.js @@ -9,7 +9,8 @@ chai.use(dirtyChai) const crypto = require('../src') const fixtures = require('./fixtures/go-key-rsa') -describe('libp2p-crypto', () => { +describe('libp2p-crypto', function () { + this.timeout(20 * 1000) let key before((done) => { crypto.keys.generateKeyPair('RSA', 2048, (err, _key) => { diff --git a/test/helpers/test-garbage-error-handling.js b/test/helpers/test-garbage-error-handling.js new file mode 100644 index 0000000..46fedea --- /dev/null +++ b/test/helpers/test-garbage-error-handling.js @@ -0,0 +1,46 @@ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) + +const util = require('util') +const garbage = [Buffer.from('00010203040506070809', 'hex'), {}, null, false, undefined, true, 1, 0, Buffer.from(''), 'aGVsbG93b3JsZA==', 'helloworld', ''] + +function doTests (fncName, fnc, num, skipBuffersAndStrings) { + if (!num) { + num = 1 + } + + garbage.forEach((garbage) => { + if (skipBuffersAndStrings && (Buffer.isBuffer(garbage) || (typeof garbage) === 'string')) { + // skip this garbage because it's a buffer or a string and we were told do do that + return + } + let args = [] + for (let i = 0; i < num; i++) { + args.push(garbage) + } + it(fncName + '(' + args.map(garbage => util.inspect(garbage)).join(', ') + ')', cb => { + args.push((err, res) => { + expect(err).to.exist() + expect(res).to.not.exist() + cb() + }) + + fnc.apply(null, args) + }) + }) +} + +module.exports = (obj, fncs, num) => { + describe('returns error via cb instead of crashing', () => { + fncs.forEach(fnc => { + doTests(fnc, obj[fnc], num) + }) + }) +} + +module.exports.doTests = doTests diff --git a/test/keys/ed25519.spec.js b/test/keys/ed25519.spec.js index 5aa62f7..5a12ccf 100644 --- a/test/keys/ed25519.spec.js +++ b/test/keys/ed25519.spec.js @@ -10,7 +10,10 @@ const crypto = require('../../src') const ed25519 = crypto.keys.supportedKeys.ed25519 const fixtures = require('../fixtures/go-key-ed25519') -describe('ed25519', () => { +const testGarbage = require('../helpers/test-garbage-error-handling') + +describe('ed25519', function () { + this.timeout(20 * 1000) let key before((done) => { crypto.keys.generateKeyPair('Ed25519', 512, (err, _key) => { @@ -176,6 +179,12 @@ describe('ed25519', () => { }) }) + describe('returns error via cb instead of crashing', () => { + const key = crypto.keys.unmarshalPublicKey(fixtures.verify.publicKey) + testGarbage.doTests('key.verify', key.verify.bind(key), 2) + testGarbage.doTests('crypto.keys.unmarshalPrivateKey', crypto.keys.unmarshalPrivateKey.bind(crypto.keys)) + }) + describe('go interop', () => { let privateKey diff --git a/test/keys/rsa.spec.js b/test/keys/rsa.spec.js index d7341bb..781bf19 100644 --- a/test/keys/rsa.spec.js +++ b/test/keys/rsa.spec.js @@ -10,7 +10,10 @@ const crypto = require('../../src') const rsa = crypto.keys.supportedKeys.rsa const fixtures = require('../fixtures/go-key-rsa') -describe('RSA', () => { +const testGarbage = require('../helpers/test-garbage-error-handling') + +describe('RSA', function () { + this.timeout(20 * 1000) let key before((done) => { @@ -131,6 +134,12 @@ describe('RSA', () => { }) }) + describe('returns error via cb instead of crashing', () => { + const key = crypto.keys.unmarshalPublicKey(fixtures.verify.publicKey) + testGarbage.doTests('key.verify', key.verify.bind(key), 2, true) + testGarbage.doTests('crypto.keys.unmarshalPrivateKey', crypto.keys.unmarshalPrivateKey.bind(crypto.keys)) + }) + describe('go interop', () => { it('verifies with data from go', (done) => { const key = crypto.keys.unmarshalPublicKey(fixtures.verify.publicKey) diff --git a/test/keys/secp256k1.spec.js b/test/keys/secp256k1.spec.js index 86ffd38..cbb4a6d 100644 --- a/test/keys/secp256k1.spec.js +++ b/test/keys/secp256k1.spec.js @@ -32,7 +32,7 @@ const mockSecp256k1Module = { } describe('without libp2p-crypto-secp256k1 module present', () => { - crypto.keys.supportedKeys['secp256k1'] = undefined + crypto.keys.supportedKeys.secp256k1 = undefined it('fails to generate a secp256k1 key', (done) => { crypto.keys.generateKeyPair('secp256k1', 256, (err, key) => { @@ -61,7 +61,7 @@ describe('with libp2p-crypto-secp256k1 module present', () => { let key before((done) => { - crypto.keys.supportedKeys['secp256k1'] = mockSecp256k1Module + crypto.keys.supportedKeys.secp256k1 = mockSecp256k1Module crypto.keys.generateKeyPair('secp256k1', 256, (err, _key) => { if (err) return done(err) key = _key @@ -70,7 +70,7 @@ describe('with libp2p-crypto-secp256k1 module present', () => { }) after((done) => { - delete crypto.keys['secp256k1'] + delete crypto.keys.secp256k1 done() })