fix: throw errors with correct stack trace (#35)

The stack trace of thrown error objects is created when the object is
instantiated - if we defer to a function to create the error we end
up with misleading stack traces.

This PR instantiates errors where errors occur and also uses the
`err-code` module to add a `.code` property so we don't have to depend
on string error messages for the type of error that was thrown.
This commit is contained in:
Alex Potsides 2019-06-13 14:35:12 +01:00 committed by Vasco Santos
parent ef47374941
commit 7051b9c530
4 changed files with 48 additions and 30 deletions

View File

@ -43,6 +43,7 @@
"homepage": "https://github.com/libp2p/js-libp2p-keychain#readme",
"dependencies": {
"async": "^2.6.2",
"err-code": "^1.1.2",
"interface-datastore": "~0.6.0",
"libp2p-crypto": "~0.16.1",
"merge-options": "^1.0.1",

View File

@ -8,6 +8,7 @@ require('node-forge/lib/pkcs7')
require('node-forge/lib/pbe')
const forge = require('node-forge/lib/forge')
const util = require('./util')
const errcode = require('err-code')
/**
* Cryptographic Message Syntax (aka PKCS #7)
@ -26,7 +27,7 @@ class CMS {
*/
constructor (keychain) {
if (!keychain) {
throw new Error('keychain is required')
throw errcode(new Error('keychain is required'), 'ERR_KEYCHAIN_REQUIRED')
}
this.keychain = keychain
@ -47,7 +48,7 @@ class CMS {
const done = (err, result) => setImmediate(() => callback(err, result))
if (!Buffer.isBuffer(plain)) {
return done(new Error('Plain data must be a Buffer'))
return done(errcode(new Error('Plain data must be a Buffer'), 'ERR_INVALID_PARAMS'))
}
series([
@ -93,7 +94,7 @@ class CMS {
const done = (err, result) => setImmediate(() => callback(err, result))
if (!Buffer.isBuffer(cmsData)) {
return done(new Error('CMS data is required'))
return done(errcode(new Error('CMS data is required'), 'ERR_INVALID_PARAMS'))
}
const self = this
@ -103,7 +104,7 @@ class CMS {
const obj = forge.asn1.fromDer(buf)
cms = forge.pkcs7.messageFromAsn1(obj)
} catch (err) {
return done(new Error('Invalid CMS: ' + err.message))
return done(errcode(new Error('Invalid CMS: ' + err.message), 'ERR_INVALID_CMS'))
}
// Find a recipient whose key we hold. We only deal with recipient certs
@ -124,8 +125,9 @@ class CMS {
if (err) return done(err)
if (!r) {
const missingKeys = recipients.map(r => r.keyId)
err = new Error('Decryption needs one of the key(s): ' + missingKeys.join(', '))
err.missingKeys = missingKeys
err = errcode(new Error('Decryption needs one of the key(s): ' + missingKeys.join(', ')), 'ERR_MISSING_KEYS', {
missingKeys
})
return done(err)
}

View File

@ -8,6 +8,7 @@ const DS = require('interface-datastore')
const collect = require('pull-stream/sinks/collect')
const pull = require('pull-stream/pull')
const CMS = require('./cms')
const errcode = require('err-code')
const keyPrefix = '/pkcs8/'
const infoPrefix = '/info/'
@ -50,7 +51,7 @@ function _error (callback, err) {
const min = 200
const max = 1000
const delay = Math.random() * (max - min) + min
if (typeof err === 'string') err = new Error(err)
setTimeout(callback, delay, err, null)
}
@ -181,26 +182,26 @@ class Keychain {
const self = this
if (!validateKeyName(name) || name === 'self') {
return _error(callback, `Invalid key name '${name}'`)
return _error(callback, errcode(new Error(`Invalid key name '${name}'`), 'ERR_INVALID_KEY_NAME'))
}
if (typeof type !== 'string') {
return _error(callback, `Invalid key type '${type}'`)
return _error(callback, errcode(new Error(`Invalid key type '${type}'`), 'ERR_INVALID_KEY_TYPE'))
}
if (!Number.isSafeInteger(size)) {
return _error(callback, `Invalid key size '${size}'`)
return _error(callback, errcode(new Error(`Invalid key size '${size}'`), 'ERR_INVALID_KEY_SIZE'))
}
const dsname = DsName(name)
self.store.has(dsname, (err, exists) => {
if (err) return _error(callback, err)
if (exists) return _error(callback, `Key '${name}' already exists`)
if (exists) return _error(callback, errcode(new Error(`Key '${name}' already exists`), 'ERR_KEY_ALREADY_EXISTS'))
switch (type.toLowerCase()) {
case 'rsa':
if (size < 2048) {
return _error(callback, `Invalid RSA key size ${size}`)
return _error(callback, errcode(new Error(`Invalid RSA key size ${size}`), 'ERR_INVALID_KEY_SIZE'))
}
break
default:
@ -278,13 +279,13 @@ class Keychain {
*/
findKeyByName (name, callback) {
if (!validateKeyName(name)) {
return _error(callback, `Invalid key name '${name}'`)
return _error(callback, errcode(new Error(`Invalid key name '${name}'`), 'ERR_INVALID_KEY_NAME'))
}
const dsname = DsInfoName(name)
this.store.get(dsname, (err, res) => {
if (err) {
return _error(callback, `Key '${name}' does not exist. ${err.message}`)
return _error(callback, errcode(new Error(`Key '${name}' does not exist. ${err.message}`), 'ERR_KEY_NOT_FOUND'))
}
callback(null, JSON.parse(res.toString()))
@ -301,7 +302,7 @@ class Keychain {
removeKey (name, callback) {
const self = this
if (!validateKeyName(name) || name === 'self') {
return _error(callback, `Invalid key name '${name}'`)
return _error(callback, errcode(new Error(`Invalid key name '${name}'`), 'ERR_INVALID_KEY_NAME'))
}
const dsname = DsName(name)
self.findKeyByName(name, (err, keyinfo) => {
@ -327,10 +328,10 @@ class Keychain {
renameKey (oldName, newName, callback) {
const self = this
if (!validateKeyName(oldName) || oldName === 'self') {
return _error(callback, `Invalid old key name '${oldName}'`)
return _error(callback, errcode(new Error(`Invalid old key name '${oldName}'`), 'ERR_OLD_KEY_NAME_INVALID'))
}
if (!validateKeyName(newName) || newName === 'self') {
return _error(callback, `Invalid new key name '${newName}'`)
return _error(callback, errcode(new Error(`Invalid new key name '${newName}'`), 'ERR_NEW_KEY_NAME_INVALID'))
}
const oldDsname = DsName(oldName)
const newDsname = DsName(newName)
@ -338,12 +339,12 @@ class Keychain {
const newInfoName = DsInfoName(newName)
this.store.get(oldDsname, (err, res) => {
if (err) {
return _error(callback, `Key '${oldName}' does not exist. ${err.message}`)
return _error(callback, errcode(new Error(`Key '${oldName}' does not exist. ${err.message}`), 'ERR_KEY_NOT_FOUND'))
}
const pem = res.toString()
self.store.has(newDsname, (err, exists) => {
if (err) return _error(callback, err)
if (exists) return _error(callback, `Key '${newName}' already exists`)
if (exists) return _error(callback, errcode(new Error(`Key '${newName}' already exists`), 'ERR_KEY_ALREADY_EXISTS'))
self.store.get(oldInfoName, (err, res) => {
if (err) return _error(callback, err)
@ -374,16 +375,16 @@ class Keychain {
*/
exportKey (name, password, callback) {
if (!validateKeyName(name)) {
return _error(callback, `Invalid key name '${name}'`)
return _error(callback, errcode(new Error(`Invalid key name '${name}'`), 'ERR_INVALID_KEY_NAME'))
}
if (!password) {
return _error(callback, 'Password is required')
return _error(callback, errcode(new Error('Password is required'), 'ERR_PASSWORD_REQUIRED'))
}
const dsname = DsName(name)
this.store.get(dsname, (err, res) => {
if (err) {
return _error(callback, `Key '${name}' does not exist. ${err.message}`)
return _error(callback, errcode(new Error(`Key '${name}' does not exist. ${err.message}`), 'ERR_KEY_NOT_FOUND'))
}
const pem = res.toString()
crypto.keys.import(pem, this._(), (err, privateKey) => {
@ -405,7 +406,7 @@ class Keychain {
importKey (name, pem, password, callback) {
const self = this
if (!validateKeyName(name) || name === 'self') {
return _error(callback, `Invalid key name '${name}'`)
return _error(callback, errcode(new Error(`Invalid key name '${name}'`), 'ERR_INVALID_KEY_NAME'))
}
if (!pem) {
return _error(callback, 'PEM encoded key is required')
@ -413,9 +414,9 @@ class Keychain {
const dsname = DsName(name)
self.store.has(dsname, (err, exists) => {
if (err) return _error(callback, err)
if (exists) return _error(callback, `Key '${name}' already exists`)
if (exists) return _error(callback, errcode(new Error(`Key '${name}' already exists`), 'ERR_KEY_ALREADY_EXISTS'))
crypto.keys.import(pem, password, (err, privateKey) => {
if (err) return _error(callback, 'Cannot read the key, most likely the password is wrong')
if (err) return _error(callback, errcode(new Error('Cannot read the key, most likely the password is wrong'), 'ERR_CANNOT_READ_KEY'))
privateKey.id((err, kid) => {
if (err) return _error(callback, err)
privateKey.export(this._(), (err, pem) => {
@ -441,17 +442,17 @@ class Keychain {
importPeer (name, peer, callback) {
const self = this
if (!validateKeyName(name)) {
return _error(callback, `Invalid key name '${name}'`)
return _error(callback, errcode(new Error(`Invalid key name '${name}'`), 'ERR_INVALID_KEY_NAME'))
}
if (!peer || !peer.privKey) {
return _error(callback, 'Peer.privKey is required')
return _error(callback, errcode(new Error('Peer.privKey is required'), 'ERR_MISSING_PRIVATE_KEY'))
}
const privateKey = peer.privKey
const dsname = DsName(name)
self.store.has(dsname, (err, exists) => {
if (err) return _error(callback, err)
if (exists) return _error(callback, `Key '${name}' already exists`)
if (exists) return _error(callback, errcode(new Error(`Key '${name}' already exists`), 'ERR_KEY_ALREADY_EXISTS'))
privateKey.id((err, kid) => {
if (err) return _error(callback, err)
@ -484,11 +485,11 @@ class Keychain {
*/
_getPrivateKey (name, callback) {
if (!validateKeyName(name)) {
return _error(callback, `Invalid key name '${name}'`)
return _error(callback, errcode(new Error(`Invalid key name '${name}'`), 'ERR_INVALID_KEY_NAME'))
}
this.store.get(DsName(name), (err, res) => {
if (err) {
return _error(callback, `Key '${name}' does not exist. ${err.message}`)
return _error(callback, errcode(new Error(`Key '${name}' does not exist. ${err.message}`), 'ERR_KEY_NOT_FOUND'))
}
callback(null, res.toString())
})

View File

@ -59,22 +59,27 @@ module.exports = (datastore1, datastore2) => {
ks.removeKey('../../nasty', (err) => {
expect(err).to.exist()
expect(err).to.have.property('message', 'Invalid key name \'../../nasty\'')
expect(err).to.have.property('code', 'ERR_INVALID_KEY_NAME')
})
ks.removeKey('', (err) => {
expect(err).to.exist()
expect(err).to.have.property('message', 'Invalid key name \'\'')
expect(err).to.have.property('code', 'ERR_INVALID_KEY_NAME')
})
ks.removeKey(' ', (err) => {
expect(err).to.exist()
expect(err).to.have.property('message', 'Invalid key name \' \'')
expect(err).to.have.property('code', 'ERR_INVALID_KEY_NAME')
})
ks.removeKey(null, (err) => {
expect(err).to.exist()
expect(err).to.have.property('message', 'Invalid key name \'null\'')
expect(err).to.have.property('code', 'ERR_INVALID_KEY_NAME')
})
ks.removeKey(undefined, (err) => {
expect(err).to.exist()
expect(err).to.have.property('message', 'Invalid key name \'undefined\'')
expect(err).to.have.property('code', 'ERR_INVALID_KEY_NAME')
})
})
})
@ -106,6 +111,7 @@ module.exports = (datastore1, datastore2) => {
it('does not overwrite existing key', (done) => {
ks.createKey(rsaKeyName, 'rsa', 2048, (err) => {
expect(err).to.exist()
expect(err).to.have.property('code', 'ERR_KEY_ALREADY_EXISTS')
done()
})
})
@ -146,6 +152,7 @@ module.exports = (datastore1, datastore2) => {
ks.createKey('bad-nist-rsa', 'rsa', 1024, (err) => {
expect(err).to.exist()
expect(err).to.have.property('message', 'Invalid RSA key size 1024')
expect(err).to.have.property('code', 'ERR_INVALID_KEY_SIZE')
done()
})
})
@ -246,6 +253,7 @@ module.exports = (datastore1, datastore2) => {
expect(err).to.exist()
expect(err).to.have.property('missingKeys')
expect(err.missingKeys).to.eql([rsaKeyInfo.id])
expect(err).to.have.property('code', 'ERR_MISSING_KEYS')
done()
})
})
@ -344,6 +352,7 @@ module.exports = (datastore1, datastore2) => {
it('requires an existing key name', (done) => {
ks.renameKey('not-there', renamedRsaKeyName, (err) => {
expect(err).to.exist()
expect(err).to.have.property('code', 'ERR_KEY_NOT_FOUND')
done()
})
})
@ -351,6 +360,7 @@ module.exports = (datastore1, datastore2) => {
it('requires a valid new key name', (done) => {
ks.renameKey(rsaKeyName, '..\not-valid', (err) => {
expect(err).to.exist()
expect(err).to.have.property('code', 'ERR_NEW_KEY_NAME_INVALID')
done()
})
})
@ -358,6 +368,7 @@ module.exports = (datastore1, datastore2) => {
it('does not overwrite existing key', (done) => {
ks.renameKey(rsaKeyName, rsaKeyName, (err) => {
expect(err).to.exist()
expect(err).to.have.property('code', 'ERR_KEY_ALREADY_EXISTS')
done()
})
})
@ -365,6 +376,7 @@ module.exports = (datastore1, datastore2) => {
it('cannot create the "self" key', (done) => {
ks.renameKey(rsaKeyName, 'self', (err) => {
expect(err).to.exist()
expect(err).to.have.property('code', 'ERR_NEW_KEY_NAME_INVALID')
done()
})
})
@ -406,6 +418,7 @@ module.exports = (datastore1, datastore2) => {
it('cannot remove the "self" key', (done) => {
ks.removeKey('self', (err) => {
expect(err).to.exist()
expect(err).to.have.property('code', 'ERR_INVALID_KEY_NAME')
done()
})
})
@ -413,6 +426,7 @@ module.exports = (datastore1, datastore2) => {
it('cannot remove an unknown key', (done) => {
ks.removeKey('not-there', (err) => {
expect(err).to.exist()
expect(err).to.have.property('code', 'ERR_KEY_NOT_FOUND')
done()
})
})