Catch error when unmarshaling instead of crashing

src/keys/rsa.js

@@ -18,22 +18,34 @@ exports.generateKey = function (bits, callback) {
     return done(err)
   }
 
-  done(null, {
-    privateKey: pemToJwk(key.private),
-    publicKey: pemToJwk(key.public)
-  })
+  let res
+  try {
+    res = {
+      privateKey: pemToJwk(key.private),
+      publicKey: pemToJwk(key.public)
+    }
+  } catch (err) {
+    return done(err)
+  }
+
+  done(null, res)
 }
 
 // Takes a jwk key
 exports.unmarshalPrivateKey = function (key, callback) {
-  callback(null, {
-    privateKey: key,
-    publicKey: {
-      kty: key.kty,
-      n: key.n,
-      e: key.e
+  try {
+    key = {
+      privateKey: key,
+      publicKey: {
+        kty: key.kty,
+        n: key.n,
+        e: key.e
+      }
     }
-  })
+  } catch (err) {
+    callback(new Error('Key is invalid!'))
+  }
+  callback(null, key)
 }
 
 exports.getRandomValues = function (arr) {
@@ -43,14 +55,29 @@ exports.getRandomValues = function (arr) {
 exports.hashAndSign = function (key, msg, callback) {
   const sign = crypto.createSign('RSA-SHA256')
 
-  sign.update(msg)
-  setImmediate(() => callback(null, sign.sign(jwkToPem(key))))
+  let pem
+
+  try {
+    sign.update(msg)
+    pem = jwkToPem(key)
+  } catch (err) {
+    return callback(new Error('Key or message is invalid!'))
+  }
+
+  setImmediate(() => callback(null, sign.sign(pem)))
 }
 
 exports.hashAndVerify = function (key, sig, msg, callback) {
   const verify = crypto.createVerify('RSA-SHA256')
 
-  verify.update(msg)
+  let pem
+
+  try {
+    verify.update(msg)
+    pem = jwkToPem(key)
+  } catch (err) {
+    return callback(new Error('Key or message is invalid!'))
+  }
 
-  setImmediate(() => callback(null, verify.verify(jwkToPem(key), sig)))
+  setImmediate(() => callback(null, verify.verify(pem, sig)))
 }

test/keys/ed25519.spec.js

@@ -10,6 +10,8 @@ const crypto = require('../../src')
 const ed25519 = crypto.keys.supportedKeys.ed25519
 const fixtures = require('../fixtures/go-key-ed25519')
 
+const testCb = require('../testCb')
+
 describe('ed25519', () => {
   let key
   before((done) => {
@@ -176,6 +178,12 @@ describe('ed25519', () => {
     })
   })
 
+  describe('returns error via cb instead of crashing', () => {
+    const key = crypto.keys.unmarshalPublicKey(fixtures.verify.publicKey)
+    testCb.doTests('key.verify', key.verify.bind(key), 2)
+    testCb.doTests('crypto.keys.unmarshalPrivateKey', crypto.keys.unmarshalPrivateKey.bind(crypto.keys))
+  })
+
   describe('go interop', () => {
     let privateKey
 

test/keys/rsa.spec.js

@@ -10,6 +10,8 @@ const crypto = require('../../src')
 const rsa = crypto.keys.supportedKeys.rsa
 const fixtures = require('../fixtures/go-key-rsa')
 
+const testCb = require('../testCb')
+
 describe('RSA', () => {
   let key
 
@@ -131,6 +133,12 @@ describe('RSA', () => {
     })
   })
 
+  describe('returns error via cb instead of crashing', () => {
+    const key = crypto.keys.unmarshalPublicKey(fixtures.verify.publicKey)
+    testCb.doTests('key.verify', key.verify.bind(key), 2)
+    testCb.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)

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()
   })
 

test/testCb.js

@@ -0,0 +1,42 @@
+/* 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) {
+  if (!num) {
+    num = 1
+  }
+
+  garbage.forEach(garbage => {
+    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(...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