From 0b686d363cd12e7fe8414ce2373b5a9741ba0b58 Mon Sep 17 00:00:00 2001 From: dirkmc Date: Mon, 22 Jul 2019 06:16:02 -0400 Subject: [PATCH] chore: add error codes (#155) * chore: add error codes * chore: create errors with new Error() * fix: better error testin * refactor: simplify random bytes error checks --- package.json | 1 + src/aes/cipher-mode.js | 17 +++++++ src/aes/index-browser.js | 6 +-- src/aes/index.js | 12 +---- src/keys/ecdh-browser.js | 6 ++- src/keys/ecdh.js | 6 +-- src/keys/ed25519-class.js | 7 +-- src/keys/index.js | 51 ++++++++++----------- src/keys/key-stretcher.js | 6 ++- src/keys/rsa-class.js | 5 +- src/keys/rsa.js | 3 +- src/keys/validate-curve-type.js | 10 ++++ src/pbkdf2.js | 4 +- src/random-bytes.js | 9 ++-- test/aes/aes.spec.js | 7 +++ test/crypto.spec.js | 13 +++++- test/helpers/test-garbage-error-handling.js | 10 +--- test/keys/ed25519.spec.js | 2 +- test/keys/ephemeral-keys.spec.js | 10 ++++ test/keys/key-stretcher.spec.js | 9 ++++ test/keys/rsa.spec.js | 7 ++- test/random-bytes.spec.js | 27 +++++++++++ test/util/index.js | 16 +++++++ 23 files changed, 172 insertions(+), 72 deletions(-) create mode 100644 src/aes/cipher-mode.js create mode 100644 src/keys/validate-curve-type.js create mode 100644 test/random-bytes.spec.js create mode 100644 test/util/index.js diff --git a/package.json b/package.json index 124ccae3..11260014 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "bn.js": "^5.0.0", "browserify-aes": "^1.2.0", "bs58": "^4.0.1", + "err-code": "^1.1.2", "iso-random-stream": "^1.1.0", "keypair": "^1.0.1", "libp2p-crypto-secp256k1": "~0.4.0", diff --git a/src/aes/cipher-mode.js b/src/aes/cipher-mode.js new file mode 100644 index 00000000..e475d193 --- /dev/null +++ b/src/aes/cipher-mode.js @@ -0,0 +1,17 @@ +'use strict' + +const errcode = require('err-code') + +const CIPHER_MODES = { + 16: 'aes-128-ctr', + 32: 'aes-256-ctr' +} + +module.exports = function (key) { + const mode = CIPHER_MODES[key.length] + if (!mode) { + const modes = Object.entries(CIPHER_MODES).map(([k, v]) => `${k} (${v})`).join(' / ') + throw errcode(new Error(`Invalid key length ${key.length} bytes. Must be ${modes}`), 'ERR_INVALID_KEY_LENGTH') + } + return mode +} diff --git a/src/aes/index-browser.js b/src/aes/index-browser.js index 7f4960be..a806917e 100644 --- a/src/aes/index-browser.js +++ b/src/aes/index-browser.js @@ -1,11 +1,11 @@ 'use strict' const asm = require('asmcrypto.js') +const validateCipherMode = require('./cipher-mode') exports.create = async function (key, iv) { // eslint-disable-line require-await - if (key.length !== 16 && key.length !== 32) { - throw new Error('Invalid key length') - } + // Throws an error if mode is invalid + validateCipherMode(key) const enc = new asm.AES_CTR.Encrypt({ key: key, diff --git a/src/aes/index.js b/src/aes/index.js index 079ddc6a..655b5478 100644 --- a/src/aes/index.js +++ b/src/aes/index.js @@ -1,18 +1,10 @@ 'use strict' const ciphers = require('./ciphers') - -const CIPHER_MODES = { - 16: 'aes-128-ctr', - 32: 'aes-256-ctr' -} +const cipherMode = require('./cipher-mode') exports.create = async function (key, iv) { // eslint-disable-line require-await - const mode = CIPHER_MODES[key.length] - if (!mode) { - throw new Error('Invalid key length') - } - + const mode = cipherMode(key) const cipher = ciphers.createCipheriv(mode, key, iv) const decipher = ciphers.createDecipheriv(mode, key, iv) diff --git a/src/keys/ecdh-browser.js b/src/keys/ecdh-browser.js index ee7c2edd..4c4134de 100644 --- a/src/keys/ecdh-browser.js +++ b/src/keys/ecdh-browser.js @@ -1,8 +1,10 @@ 'use strict' +const errcode = require('err-code') const webcrypto = require('../webcrypto.js') const BN = require('asn1.js').bignum const { toBase64, toBn } = require('../util') +const validateCurveType = require('./validate-curve-type') const bits = { 'P-256': 256, @@ -11,6 +13,8 @@ const bits = { } exports.generateEphmeralKeyPair = async function (curve) { + validateCurveType(Object.keys(bits), curve) + const pair = await webcrypto.subtle.generateKey( { name: 'ECDH', @@ -96,7 +100,7 @@ function unmarshalPublicKey (curve, key) { const byteLen = curveLengths[curve] if (!key.slice(0, 1).equals(Buffer.from([4]))) { - throw new Error('Invalid key format') + throw errcode(new Error('Cannot unmarshal public key - invalid key format'), 'ERR_INVALID_KEY_FORMAT') } const x = new BN(key.slice(1, byteLen + 1)) const y = new BN(key.slice(1 + byteLen)) diff --git a/src/keys/ecdh.js b/src/keys/ecdh.js index 4efec3e8..69651b6d 100644 --- a/src/keys/ecdh.js +++ b/src/keys/ecdh.js @@ -1,6 +1,7 @@ 'use strict' const crypto = require('crypto') +const validateCurveType = require('./validate-curve-type') const curves = { 'P-256': 'prime256v1', @@ -9,9 +10,8 @@ const curves = { } exports.generateEphmeralKeyPair = async function (curve) { // eslint-disable-line require-await - if (!curves[curve]) { - throw new Error(`Unkown curve: ${curve}`) - } + validateCurveType(Object.keys(curves), curve) + const ecdh = crypto.createECDH(curves[curve]) ecdh.generateKeys() diff --git a/src/keys/ed25519-class.js b/src/keys/ed25519-class.js index 3724e0a2..b9f71069 100644 --- a/src/keys/ed25519-class.js +++ b/src/keys/ed25519-class.js @@ -3,6 +3,7 @@ const multihashing = require('multihashing-async') const protobuf = require('protons') const bs58 = require('bs58') +const errcode = require('err-code') const crypto = require('./ed25519') const pbm = protobuf(require('./keys.proto')) @@ -49,10 +50,6 @@ class Ed25519PrivateKey { } get public () { - if (!this._publicKey) { - throw new Error('public key not provided') - } - return new Ed25519PublicKey(this._publicKey) } @@ -117,7 +114,7 @@ function ensureKey (key, length) { key = new Uint8Array(key) } if (!(key instanceof Uint8Array) || key.length !== length) { - throw new Error('Key must be a Uint8Array or Buffer of length ' + length) + throw errcode(new Error('Key must be a Uint8Array or Buffer of length ' + length), 'ERR_INVALID_KEY_TYPE') } return key } diff --git a/src/keys/index.js b/src/keys/index.js index a14d6dbc..205d1086 100644 --- a/src/keys/index.js +++ b/src/keys/index.js @@ -6,6 +6,7 @@ require('node-forge/lib/asn1') require('node-forge/lib/rsa') require('node-forge/lib/pbe') const forge = require('node-forge/lib/forge') +const errcode = require('err-code') exports = module.exports @@ -18,9 +19,18 @@ const supportedKeys = { exports.supportedKeys = supportedKeys exports.keysPBM = keysPBM -function isValidKeyType (keyType) { - const key = supportedKeys[keyType.toLowerCase()] - return key !== undefined +const ErrMissingSecp256K1 = { + message: 'secp256k1 support requires libp2p-crypto-secp256k1 package', + code: 'ERR_MISSING_PACKAGE' +} + +function typeToKey (type) { + let key = supportedKeys[type.toLowerCase()] + if (!key) { + const supported = Object.keys(supportedKeys).join(' / ') + throw errcode(new Error(`invalid or unsupported key type ${type}. Must be ${supported}`), 'ERR_UNSUPPORTED_KEY_TYPE') + } + return key } exports.keyStretcher = require('./key-stretcher') @@ -28,24 +38,15 @@ exports.generateEphemeralKeyPair = require('./ephemeral-keys') // Generates a keypair of the given type and bitsize exports.generateKeyPair = async (type, bits) => { // eslint-disable-line require-await - let key = supportedKeys[type.toLowerCase()] - - if (!key) { - throw new Error('invalid or unsupported key type') - } - - return key.generateKeyPair(bits) + return typeToKey(type).generateKeyPair(bits) } // Generates a keypair of the given type and bitsize // seed is a 32 byte uint8array exports.generateKeyPairFromSeed = async (type, seed, bits) => { // eslint-disable-line require-await - let key = supportedKeys[type.toLowerCase()] - if (!key) { - throw new Error('invalid or unsupported key type') - } + const key = typeToKey(type) if (type.toLowerCase() !== 'ed25519') { - throw new Error('Seed key derivation is unimplemented for RSA or secp256k1') + throw errcode(new Error('Seed key derivation is unimplemented for RSA or secp256k1'), 'ERR_UNSUPPORTED_KEY_DERIVATION_TYPE') } return key.generateKeyPairFromSeed(seed, bits) } @@ -65,20 +66,17 @@ exports.unmarshalPublicKey = (buf) => { if (supportedKeys.secp256k1) { return supportedKeys.secp256k1.unmarshalSecp256k1PublicKey(data) } else { - throw new Error('secp256k1 support requires libp2p-crypto-secp256k1 package') + throw errcode(new Error(ErrMissingSecp256K1.message), ErrMissingSecp256K1.code) } default: - throw new Error('invalid or unsupported key type') + typeToKey(decoded.Type) // throws because type is not supported } } // Converts a public key object into a protobuf serialized public key exports.marshalPublicKey = (key, type) => { type = (type || 'rsa').toLowerCase() - if (!isValidKeyType(type)) { - throw new Error('invalid or unsupported key type') - } - + typeToKey(type) // check type return key.bytes } @@ -97,27 +95,24 @@ exports.unmarshalPrivateKey = async (buf) => { // eslint-disable-line require-aw if (supportedKeys.secp256k1) { return supportedKeys.secp256k1.unmarshalSecp256k1PrivateKey(data) } else { - throw new Error('secp256k1 support requires libp2p-crypto-secp256k1 package') + throw errcode(new Error(ErrMissingSecp256K1.message), ErrMissingSecp256K1.code) } default: - throw new Error('invalid or unsupported key type') + typeToKey(decoded.Type) // throws because type is not supported } } // Converts a private key object into a protobuf serialized private key exports.marshalPrivateKey = (key, type) => { type = (type || 'rsa').toLowerCase() - if (!isValidKeyType(type)) { - throw new Error('invalid or unsupported key type') - } - + typeToKey(type) // check type return key.bytes } exports.import = async (pem, password) => { // eslint-disable-line require-await const key = forge.pki.decryptRsaPrivateKey(pem, password) if (key === null) { - throw new Error('Cannot read the key, most likely the password is wrong or not a RSA key') + throw errcode(new Error('Cannot read the key, most likely the password is wrong or not a RSA key'), 'ERR_CANNOT_DECRYPT_PEM') } let der = forge.asn1.toDer(forge.pki.privateKeyToAsn1(key)) der = Buffer.from(der.getBytes(), 'binary') diff --git a/src/keys/key-stretcher.js b/src/keys/key-stretcher.js index b8e1c940..cca81781 100644 --- a/src/keys/key-stretcher.js +++ b/src/keys/key-stretcher.js @@ -1,5 +1,6 @@ 'use strict' +const errcode = require('err-code') const hmac = require('../hmac') const cipherMap = { @@ -23,11 +24,12 @@ module.exports = async (cipherType, hash, secret) => { const cipher = cipherMap[cipherType] if (!cipher) { - throw new Error('unkown cipherType passed') + const allowed = Object.keys(cipherMap).join(' / ') + throw errcode(new Error(`unknown cipher type '${cipherType}'. Must be ${allowed}`), 'ERR_INVALID_CIPHER_TYPE') } if (!hash) { - throw new Error('unkown hashType passed') + throw errcode(new Error(`missing hash type`), 'ERR_MISSING_HASH_TYPE') } const cipherKeySize = cipher.keySize diff --git a/src/keys/rsa-class.js b/src/keys/rsa-class.js index 6291c0ae..84ad38bd 100644 --- a/src/keys/rsa-class.js +++ b/src/keys/rsa-class.js @@ -3,6 +3,7 @@ const multihashing = require('multihashing-async') const protobuf = require('protons') const bs58 = require('bs58') +const errcode = require('err-code') const crypto = require('./rsa') const pbm = protobuf(require('./keys.proto')) @@ -61,7 +62,7 @@ class RsaPrivateKey { get public () { if (!this._publicKey) { - throw new Error('public key not provided') + throw errcode(new Error('public key not provided'), 'ERR_PUBKEY_NOT_PROVIDED') } return new RsaPublicKey(this._publicKey) @@ -123,7 +124,7 @@ class RsaPrivateKey { } pem = forge.pki.encryptRsaPrivateKey(privateKey, password, options) } else { - throw new Error(`Unknown export format '${format}'`) + throw errcode(new Error(`Unknown export format '${format}'. Must be pkcs-8`), 'ERR_INVALID_EXPORT_FORMAT') } return pem diff --git a/src/keys/rsa.js b/src/keys/rsa.js index 23919828..019760a7 100644 --- a/src/keys/rsa.js +++ b/src/keys/rsa.js @@ -1,6 +1,7 @@ 'use strict' const crypto = require('crypto') +const errcode = require('err-code') const randomBytes = require('../random-bytes') let keypair @@ -40,7 +41,7 @@ exports.generateKey = async function (bits) { // eslint-disable-line require-awa // Takes a jwk key exports.unmarshalPrivateKey = async function (key) { // eslint-disable-line require-await if (!key) { - throw new Error('Key is invalid') + throw errcode(new Error('Missing key parameter'), 'ERR_MISSING_KEY') } return { privateKey: key, diff --git a/src/keys/validate-curve-type.js b/src/keys/validate-curve-type.js new file mode 100644 index 00000000..81ccb2e3 --- /dev/null +++ b/src/keys/validate-curve-type.js @@ -0,0 +1,10 @@ +'use strict' + +const errcode = require('err-code') + +module.exports = function (curveTypes, type) { + if (!curveTypes.includes(type)) { + const names = curveTypes.join(' / ') + throw errcode(new Error(`Unknown curve: ${type}. Must be ${names}`), 'ERR_INVALID_CURVE') + } +} diff --git a/src/pbkdf2.js b/src/pbkdf2.js index 63ebfbfd..80ad0678 100644 --- a/src/pbkdf2.js +++ b/src/pbkdf2.js @@ -2,6 +2,7 @@ const forgePbkdf2 = require('node-forge/lib/pbkdf2') const forgeUtil = require('node-forge/lib/util') +const errcode = require('err-code') /** * Maps an IPFS hash name to its node-forge equivalent. @@ -29,7 +30,8 @@ const hashName = { function pbkdf2 (password, salt, iterations, keySize, hash) { const hasher = hashName[hash] if (!hasher) { - throw new Error(`Hash '${hash}' is unknown or not supported`) + const types = Object.keys(hashName).join(' / ') + throw errcode(new Error(`Hash '${hash}' is unknown or not supported. Must be ${types}`), 'ERR_UNSUPPORTED_HASH_TYPE') } const dek = forgePbkdf2( password, diff --git a/src/random-bytes.js b/src/random-bytes.js index 78c1288b..ef1edd51 100644 --- a/src/random-bytes.js +++ b/src/random-bytes.js @@ -1,9 +1,10 @@ 'use strict' const randomBytes = require('iso-random-stream/src/random') +const errcode = require('err-code') -module.exports = function (number) { - if (!number || typeof number !== 'number') { - throw new Error('first argument must be a Number bigger than 0') +module.exports = function (length) { + if (isNaN(length) || length <= 0) { + throw errcode(new Error('random bytes length must be a Number bigger than 0'), 'ERR_INVALID_LENGTH') } - return randomBytes(number) + return randomBytes(length) } diff --git a/test/aes/aes.spec.js b/test/aes/aes.spec.js index edd13742..ff116582 100644 --- a/test/aes/aes.spec.js +++ b/test/aes/aes.spec.js @@ -6,6 +6,7 @@ const chai = require('chai') const dirtyChai = require('dirty-chai') const expect = chai.expect chai.use(dirtyChai) +const { expectErrCode } = require('../util') const crypto = require('../../src') const fixtures = require('./../fixtures/aes') @@ -84,6 +85,12 @@ describe('AES-CTR', () => { } }) }) + + it('checks key length', () => { + const key = Buffer.alloc(5) + const iv = Buffer.alloc(16) + return expectErrCode(crypto.aes.create(key, iv), 'ERR_INVALID_KEY_LENGTH') + }) }) async function encryptAndDecrypt (cipher) { diff --git a/test/crypto.spec.js b/test/crypto.spec.js index 146983eb..0476c8d2 100644 --- a/test/crypto.spec.js +++ b/test/crypto.spec.js @@ -8,6 +8,7 @@ const expect = chai.expect chai.use(dirtyChai) const crypto = require('../src') const fixtures = require('./fixtures/go-key-rsa') +const { expectErrCode } = require('./util') describe('libp2p-crypto', function () { this.timeout(20 * 1000) @@ -38,6 +39,15 @@ describe('libp2p-crypto', function () { expect(key2.public.equals(key.public)).to.be.eql(true) }) + it('generateKeyPair', () => { + return expectErrCode(crypto.keys.generateKeyPair('invalid-key-type', 512), 'ERR_UNSUPPORTED_KEY_TYPE') + }) + + it('generateKeyPairFromSeed', () => { + var seed = crypto.randomBytes(32) + return expectErrCode(crypto.keys.generateKeyPairFromSeed('invalid-key-type', seed, 512), 'ERR_UNSUPPORTED_KEY_TYPE') + }) + // marshalled keys seem to be slightly different // unsure as to if this is just a difference in encoding // or a bug @@ -93,7 +103,8 @@ describe('libp2p-crypto', function () { }) it('throws on invalid hash name', () => { - expect(() => crypto.pbkdf2('password', 'at least 16 character salt', 500, 512 / 8, 'shaX-xxx')).to.throw() + const fn = () => crypto.pbkdf2('password', 'at least 16 character salt', 500, 512 / 8, 'shaX-xxx') + expect(fn).to.throw().with.property('code', 'ERR_UNSUPPORTED_HASH_TYPE') }) }) diff --git a/test/helpers/test-garbage-error-handling.js b/test/helpers/test-garbage-error-handling.js index 50a5cf47..324789c3 100644 --- a/test/helpers/test-garbage-error-handling.js +++ b/test/helpers/test-garbage-error-handling.js @@ -29,12 +29,4 @@ function doTests (fncName, fnc, num, skipBuffersAndStrings) { }) } -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 +module.exports = { doTests } diff --git a/test/keys/ed25519.spec.js b/test/keys/ed25519.spec.js index 621c24f8..914bfc46 100644 --- a/test/keys/ed25519.spec.js +++ b/test/keys/ed25519.spec.js @@ -116,7 +116,7 @@ describe('ed25519', function () { expect(valid).to.be.eql(false) }) - describe('returns error via cb instead of crashing', () => { + describe('throws error 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)) diff --git a/test/keys/ephemeral-keys.spec.js b/test/keys/ephemeral-keys.spec.js index d353f0d0..5ad66452 100644 --- a/test/keys/ephemeral-keys.spec.js +++ b/test/keys/ephemeral-keys.spec.js @@ -60,4 +60,14 @@ describe('generateEphemeralKeyPair', () => { expect(secrets[0]).to.have.length(32) }) }) + + it(`handles bad curve name`, async () => { + try { + await crypto.keys.generateEphemeralKeyPair('bad name') + } catch (err) { + expect(err.code).equals('ERR_INVALID_CURVE') + return + } + expect.fail('Did not throw error') + }) }) diff --git a/test/keys/key-stretcher.spec.js b/test/keys/key-stretcher.spec.js index 514029cb..b0e528eb 100644 --- a/test/keys/key-stretcher.spec.js +++ b/test/keys/key-stretcher.spec.js @@ -6,6 +6,7 @@ const chai = require('chai') const dirtyChai = require('dirty-chai') const expect = chai.expect chai.use(dirtyChai) +const { expectErrCode } = require('../util') const crypto = require('../../src') const fixtures = require('../fixtures/go-stretch-key') @@ -30,6 +31,14 @@ describe('keyStretcher', () => { }) }) }) + + it('handles invalid cipher type', () => { + return expectErrCode(crypto.keys.keyStretcher('invalid-cipher', 'SHA256', 'secret'), 'ERR_INVALID_CIPHER_TYPE') + }) + + it('handles missing hash type', () => { + return expectErrCode(crypto.keys.keyStretcher('AES-128', '', 'secret'), 'ERR_MISSING_HASH_TYPE') + }) }) describe('go interop', () => { diff --git a/test/keys/rsa.spec.js b/test/keys/rsa.spec.js index 77a9cae0..175b1443 100644 --- a/test/keys/rsa.spec.js +++ b/test/keys/rsa.spec.js @@ -7,6 +7,7 @@ const dirtyChai = require('dirty-chai') const expect = chai.expect chai.use(dirtyChai) chai.use(require('chai-string')) +const { expectErrCode } = require('../util') const crypto = require('../../src') const rsa = crypto.keys.supportedKeys.rsa @@ -112,9 +113,13 @@ describe('RSA', function () { } throw new Error('Expected error to be thrown') }) + + it('handles invalid export type', () => { + return expectErrCode(key.export('secret', 'invalid-type'), 'ERR_INVALID_EXPORT_FORMAT') + }) }) - describe('returns error via cb instead of crashing', () => { + describe('throws error 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)) diff --git a/test/random-bytes.spec.js b/test/random-bytes.spec.js new file mode 100644 index 00000000..0eb95171 --- /dev/null +++ b/test/random-bytes.spec.js @@ -0,0 +1,27 @@ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) + +const randomBytes = require('../src/random-bytes') + +describe('randomBytes', () => { + it('produces random bytes', () => { + expect(randomBytes(16)).to.have.length(16) + }) + + it('throws if length is 0', () => { + expect(() => randomBytes(0)).to.throw(Error).with.property('code', 'ERR_INVALID_LENGTH') + }) + + it('throws if length is < 0', () => { + expect(() => randomBytes(-1)).to.throw(Error).with.property('code', 'ERR_INVALID_LENGTH') + }) + + it('throws if length is not a number', () => { + expect(() => randomBytes('hi')).to.throw(Error).with.property('code', 'ERR_INVALID_LENGTH') + }) +}) diff --git a/test/util/index.js b/test/util/index.js new file mode 100644 index 00000000..905fad99 --- /dev/null +++ b/test/util/index.js @@ -0,0 +1,16 @@ +'use strict' + +const chai = require('chai') +const expect = chai.expect + +const expectErrCode = async (p, code) => { + try { + await p + } catch (err) { + expect(err).to.have.property('code', code) + return + } + expect.fail(`Expected error with code ${code} but no error thrown`) +} + +module.exports = { expectErrCode }