From a080f5d3e72e3bf4f5865f51c9c578c819dcd8b0 Mon Sep 17 00:00:00 2001 From: mkg20001 Date: Mon, 27 Nov 2017 12:25:27 +0100 Subject: [PATCH 1/8] test: add failing tests --- test/peer-id.spec.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/peer-id.spec.js b/test/peer-id.spec.js index 2695945..3340d46 100644 --- a/test/peer-id.spec.js +++ b/test/peer-id.spec.js @@ -245,6 +245,22 @@ describe('PeerId', () => { }) }) + describe('returns error via cb instead of crashing', () => { + const garbage = Buffer.from('00010203040506070809', 'hex') + + const fncs = ['createFromPubKey', 'createFromPrivKey', 'createFromJSON'] + + fncs.forEach(fnc => { + it(fnc + '(garbage)', cb => { + PeerId[fnc](garbage, (err, res) => { + expect(err).to.exist() + expect(res).to.not.exist() + cb() + }) + }) + }) + }) + describe('throws on inconsistent data', () => { let k1 let k2 From aab09364d8a638170e29100a405141c4ae563af1 Mon Sep 17 00:00:00 2001 From: mkg20001 Date: Mon, 27 Nov 2017 14:25:08 +0100 Subject: [PATCH 2/8] test: Add more garbage test cases --- test/peer-id.spec.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test/peer-id.spec.js b/test/peer-id.spec.js index 3340d46..5db37cc 100644 --- a/test/peer-id.spec.js +++ b/test/peer-id.spec.js @@ -12,6 +12,8 @@ const parallel = require('async/parallel') const PeerId = require('../src') +const util = require('util') + const testId = require('./fixtures/sample-id') const testIdHex = testId.id const testIdBytes = mh.fromHexString(testId.id) @@ -246,16 +248,18 @@ describe('PeerId', () => { }) describe('returns error via cb instead of crashing', () => { - const garbage = Buffer.from('00010203040506070809', 'hex') + const garbage = [Buffer.from('00010203040506070809', 'hex'), {}, null, false, undefined, true, 1, 0, Buffer.from('')] const fncs = ['createFromPubKey', 'createFromPrivKey', 'createFromJSON'] - fncs.forEach(fnc => { - it(fnc + '(garbage)', cb => { - PeerId[fnc](garbage, (err, res) => { - expect(err).to.exist() - expect(res).to.not.exist() - cb() + garbage.forEach(garbage => { + fncs.forEach(fnc => { + it(fnc + '(' + util.inspect(garbage) + ')', cb => { + PeerId[fnc](garbage, (err, res) => { + expect(err).to.exist() + expect(res).to.not.exist() + cb() + }) }) }) }) From 444337e070f001cac7344131e696913b1c7c9821 Mon Sep 17 00:00:00 2001 From: mkg20001 Date: Mon, 27 Nov 2017 14:25:34 +0100 Subject: [PATCH 3/8] fix: Catch error when unmarshaling instead of crashing --- src/index.js | 132 ++++++++++++++++++++++++++++----------------------- 1 file changed, 73 insertions(+), 59 deletions(-) diff --git a/src/index.js b/src/index.js index e8b6a00..fd322e1 100644 --- a/src/index.js +++ b/src/index.js @@ -165,45 +165,55 @@ exports.createFromPubKey = function (key, callback) { throw new Error('callback is required') } - let buf = key - if (typeof buf === 'string') { - buf = Buffer.from(key, 'base64') - } + try { + let buf = key + if (typeof buf === 'string') { + buf = Buffer.from(key, 'base64') + } - const pubKey = crypto.keys.unmarshalPublicKey(buf) + if (!Buffer.isBuffer(buf)) throw new Error('Supplied key is neither a base64 string nor a buffer') - pubKey.hash((err, digest) => { - if (err) { - return callback(err) - } + const pubKey = crypto.keys.unmarshalPublicKey(buf) - callback(null, new PeerId(digest, null, pubKey)) - }) + pubKey.hash((err, digest) => { + if (err) { + return callback(err) + } + + callback(null, new PeerId(digest, null, pubKey)) + }) + } catch (e) { + callback(e) + } } // Private key input will be a string exports.createFromPrivKey = function (key, callback) { - let buf = key - if (typeof buf === 'string') { - buf = Buffer.from(key, 'base64') - } - if (typeof callback !== 'function') { throw new Error('callback is required') } - waterfall([ - (cb) => crypto.keys.unmarshalPrivateKey(buf, cb), - (privKey, cb) => privKey.public.hash((err, digest) => { - cb(err, digest, privKey) - }) - ], (err, digest, privKey) => { - if (err) { - return callback(err) + try { + let buf = key + if (typeof buf === 'string') { + buf = Buffer.from(key, 'base64') } - callback(null, new PeerId(digest, privKey, privKey.public)) - }) + waterfall([ + (cb) => crypto.keys.unmarshalPrivateKey(buf, cb), + (privKey, cb) => privKey.public.hash((err, digest) => { + cb(err, digest, privKey) + }) + ], (err, digest, privKey) => { + if (err) { + return callback(err) + } + + callback(null, new PeerId(digest, privKey, privKey.public)) + }) + } catch (e) { + callback(e) + } } exports.createFromJSON = function (obj, callback) { @@ -211,43 +221,47 @@ exports.createFromJSON = function (obj, callback) { throw new Error('callback is required') } - const id = mh.fromB58String(obj.id) - const rawPrivKey = obj.privKey && Buffer.from(obj.privKey, 'base64') - const rawPubKey = obj.pubKey && Buffer.from(obj.pubKey, 'base64') - const pub = rawPubKey && crypto.keys.unmarshalPublicKey(rawPubKey) - - if (rawPrivKey) { - waterfall([ - (cb) => crypto.keys.unmarshalPrivateKey(rawPrivKey, cb), - (priv, cb) => priv.public.hash((err, digest) => { - cb(err, digest, priv) - }), - (privDigest, priv, cb) => { - if (pub) { - pub.hash((err, pubDigest) => { - cb(err, privDigest, priv, pubDigest) - }) - } else { - cb(null, privDigest, priv) + try { + const id = mh.fromB58String(obj.id) + const rawPrivKey = obj.privKey && Buffer.from(obj.privKey, 'base64') + const rawPubKey = obj.pubKey && Buffer.from(obj.pubKey, 'base64') + const pub = rawPubKey && crypto.keys.unmarshalPublicKey(rawPubKey) + + if (rawPrivKey) { + waterfall([ + (cb) => crypto.keys.unmarshalPrivateKey(rawPrivKey, cb), + (priv, cb) => priv.public.hash((err, digest) => { + cb(err, digest, priv) + }), + (privDigest, priv, cb) => { + if (pub) { + pub.hash((err, pubDigest) => { + cb(err, privDigest, priv, pubDigest) + }) + } else { + cb(null, privDigest, priv) + } + } + ], (err, privDigest, priv, pubDigest) => { + if (err) { + return callback(err) } - } - ], (err, privDigest, priv, pubDigest) => { - if (err) { - return callback(err) - } - if (pub && !privDigest.equals(pubDigest)) { - return callback(new Error('Public and private key do not match')) - } + if (pub && !privDigest.equals(pubDigest)) { + return callback(new Error('Public and private key do not match')) + } - if (id && !privDigest.equals(id)) { - return callback(new Error('Id and private key do not match')) - } + if (id && !privDigest.equals(id)) { + return callback(new Error('Id and private key do not match')) + } - callback(null, new PeerId(id, priv, pub)) - }) - } else { - callback(null, new PeerId(id, null, pub)) + callback(null, new PeerId(id, priv, pub)) + }) + } else { + callback(null, new PeerId(id, null, pub)) + } + } catch (e) { + callback(e) } } From 1ffd912a665b535cea22000aaab5944953d4ec17 Mon Sep 17 00:00:00 2001 From: mkg20001 Date: Mon, 27 Nov 2017 14:27:52 +0100 Subject: [PATCH 4/8] misc: Add buffer error to createFromPrivKey --- src/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/index.js b/src/index.js index fd322e1..da01972 100644 --- a/src/index.js +++ b/src/index.js @@ -199,6 +199,8 @@ exports.createFromPrivKey = function (key, callback) { buf = Buffer.from(key, 'base64') } + if (!Buffer.isBuffer(buf)) throw new Error('Supplied key is neither a base64 string nor a buffer') + waterfall([ (cb) => crypto.keys.unmarshalPrivateKey(buf, cb), (privKey, cb) => privKey.public.hash((err, digest) => { From 96c9537f87b073e4e85690fe3d50f101d0f3f202 Mon Sep 17 00:00:00 2001 From: mkg20001 Date: Mon, 27 Nov 2017 14:49:42 +0100 Subject: [PATCH 5/8] misc: apply requested changes --- src/index.js | 130 +++++++++++++++++++++++++++------------------------ 1 file changed, 69 insertions(+), 61 deletions(-) diff --git a/src/index.js b/src/index.js index da01972..1e2538e 100644 --- a/src/index.js +++ b/src/index.js @@ -165,6 +165,8 @@ exports.createFromPubKey = function (key, callback) { throw new Error('callback is required') } + let pubKey + try { let buf = key if (typeof buf === 'string') { @@ -173,18 +175,18 @@ exports.createFromPubKey = function (key, callback) { if (!Buffer.isBuffer(buf)) throw new Error('Supplied key is neither a base64 string nor a buffer') - const pubKey = crypto.keys.unmarshalPublicKey(buf) + pubKey = crypto.keys.unmarshalPublicKey(buf) + } catch (err) { + return callback(err) + } - pubKey.hash((err, digest) => { - if (err) { - return callback(err) - } + pubKey.hash((err, digest) => { + if (err) { + return callback(err) + } - callback(null, new PeerId(digest, null, pubKey)) - }) - } catch (e) { - callback(e) - } + callback(null, new PeerId(digest, null, pubKey)) + }) } // Private key input will be a string @@ -193,29 +195,30 @@ exports.createFromPrivKey = function (key, callback) { throw new Error('callback is required') } + let buf = key + try { - let buf = key if (typeof buf === 'string') { buf = Buffer.from(key, 'base64') } if (!Buffer.isBuffer(buf)) throw new Error('Supplied key is neither a base64 string nor a buffer') + } catch (err) { + callback(err) + } - waterfall([ - (cb) => crypto.keys.unmarshalPrivateKey(buf, cb), - (privKey, cb) => privKey.public.hash((err, digest) => { - cb(err, digest, privKey) - }) - ], (err, digest, privKey) => { - if (err) { - return callback(err) - } - - callback(null, new PeerId(digest, privKey, privKey.public)) + waterfall([ + (cb) => crypto.keys.unmarshalPrivateKey(buf, cb), + (privKey, cb) => privKey.public.hash((err, digest) => { + cb(err, digest, privKey) }) - } catch (e) { - callback(e) - } + ], (err, digest, privKey) => { + if (err) { + return callback(err) + } + + callback(null, new PeerId(digest, privKey, privKey.public)) + }) } exports.createFromJSON = function (obj, callback) { @@ -223,47 +226,52 @@ exports.createFromJSON = function (obj, callback) { throw new Error('callback is required') } + let id + let rawPrivKey + let rawPubKey + let pub + try { - const id = mh.fromB58String(obj.id) - const rawPrivKey = obj.privKey && Buffer.from(obj.privKey, 'base64') - const rawPubKey = obj.pubKey && Buffer.from(obj.pubKey, 'base64') - const pub = rawPubKey && crypto.keys.unmarshalPublicKey(rawPubKey) - - if (rawPrivKey) { - waterfall([ - (cb) => crypto.keys.unmarshalPrivateKey(rawPrivKey, cb), - (priv, cb) => priv.public.hash((err, digest) => { - cb(err, digest, priv) - }), - (privDigest, priv, cb) => { - if (pub) { - pub.hash((err, pubDigest) => { - cb(err, privDigest, priv, pubDigest) - }) - } else { - cb(null, privDigest, priv) - } - } - ], (err, privDigest, priv, pubDigest) => { - if (err) { - return callback(err) - } + id = mh.fromB58String(obj.id) + rawPrivKey = obj.privKey && Buffer.from(obj.privKey, 'base64') + rawPubKey = obj.pubKey && Buffer.from(obj.pubKey, 'base64') + pub = rawPubKey && crypto.keys.unmarshalPublicKey(rawPubKey) + } catch (err) { + callback(err) + } - if (pub && !privDigest.equals(pubDigest)) { - return callback(new Error('Public and private key do not match')) + if (rawPrivKey) { + waterfall([ + (cb) => crypto.keys.unmarshalPrivateKey(rawPrivKey, cb), + (priv, cb) => priv.public.hash((err, digest) => { + cb(err, digest, priv) + }), + (privDigest, priv, cb) => { + if (pub) { + pub.hash((err, pubDigest) => { + cb(err, privDigest, priv, pubDigest) + }) + } else { + cb(null, privDigest, priv) } + } + ], (err, privDigest, priv, pubDigest) => { + if (err) { + return callback(err) + } - if (id && !privDigest.equals(id)) { - return callback(new Error('Id and private key do not match')) - } + if (pub && !privDigest.equals(pubDigest)) { + return callback(new Error('Public and private key do not match')) + } - callback(null, new PeerId(id, priv, pub)) - }) - } else { - callback(null, new PeerId(id, null, pub)) - } - } catch (e) { - callback(e) + if (id && !privDigest.equals(id)) { + return callback(new Error('Id and private key do not match')) + } + + callback(null, new PeerId(id, priv, pub)) + }) + } else { + callback(null, new PeerId(id, null, pub)) } } From 88a4900f0b82b57678fa4bbffe9c0c7cd0d91591 Mon Sep 17 00:00:00 2001 From: mkg20001 Date: Mon, 27 Nov 2017 14:54:00 +0100 Subject: [PATCH 6/8] misc: Apply requested changes --- src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index 1e2538e..87145c6 100644 --- a/src/index.js +++ b/src/index.js @@ -204,7 +204,7 @@ exports.createFromPrivKey = function (key, callback) { if (!Buffer.isBuffer(buf)) throw new Error('Supplied key is neither a base64 string nor a buffer') } catch (err) { - callback(err) + return callback(err) } waterfall([ @@ -237,7 +237,7 @@ exports.createFromJSON = function (obj, callback) { rawPubKey = obj.pubKey && Buffer.from(obj.pubKey, 'base64') pub = rawPubKey && crypto.keys.unmarshalPublicKey(rawPubKey) } catch (err) { - callback(err) + return callback(err) } if (rawPrivKey) { From ccac12cceb6088289e4339a91b8b000fa54790eb Mon Sep 17 00:00:00 2001 From: mkg20001 Date: Mon, 27 Nov 2017 15:02:08 +0100 Subject: [PATCH 7/8] test: Add more garbage --- test/peer-id.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/peer-id.spec.js b/test/peer-id.spec.js index 5db37cc..57fdcd7 100644 --- a/test/peer-id.spec.js +++ b/test/peer-id.spec.js @@ -248,7 +248,7 @@ describe('PeerId', () => { }) describe('returns error via cb instead of crashing', () => { - const garbage = [Buffer.from('00010203040506070809', 'hex'), {}, null, false, undefined, true, 1, 0, Buffer.from('')] + const garbage = [Buffer.from('00010203040506070809', 'hex'), {}, null, false, undefined, true, 1, 0, Buffer.from(''), 'aGVsbG93b3JsZA==', 'helloworld', ''] const fncs = ['createFromPubKey', 'createFromPrivKey', 'createFromJSON'] From 84dfe0184bae71662c00a49f46c0d211d6ecbffe Mon Sep 17 00:00:00 2001 From: David Dias Date: Fri, 1 Dec 2017 08:47:56 +0000 Subject: [PATCH 8/8] chore: update deps --- .gitignore | 3 ++- package.json | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 2ea0e2b..9a7ba0e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +docs **/node_modules/ **/*.log test/repo-tests* @@ -41,4 +42,4 @@ test/test-data/go-ipfs-repo/LOG.old # while testing npm5 package-lock.json -yarn.lock \ No newline at end of file +yarn.lock diff --git a/package.json b/package.json index 84a1946..eb6a58c 100644 --- a/package.json +++ b/package.json @@ -33,16 +33,16 @@ }, "homepage": "https://github.com/libp2p/js-peer-id", "devDependencies": { - "aegir": "^12.0.8", + "aegir": "^12.2.0", "chai": "^4.1.2", "dirty-chai": "^2.0.1", "pre-commit": "^1.2.2" }, "dependencies": { - "async": "^2.5.0", - "libp2p-crypto": "~0.10.3", + "async": "^2.6.0", + "libp2p-crypto": "~0.10.4", "lodash": "^4.17.4", - "multihashes": "~0.4.9" + "multihashes": "~0.4.12" }, "repository": { "type": "git",