From 5a8793b78fc2466a9f5cd9ae0e10f389d0a96fd0 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Tue, 4 Aug 2020 17:53:04 +0100 Subject: [PATCH 1/3] fix: allow certain keychain operations without a password Listing, removing, renaming etc keys do not require a password so the user should not be required to provide one. This means we don't have to prompt the user to create a password when they aren't going to do any operations that require a password. --- src/keychain/index.js | 26 ++++++++++++------ test/keychain/keychain.spec.js | 50 +++++++++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/keychain/index.js b/src/keychain/index.js index c5009a54a6..bf2b55537d 100644 --- a/src/keychain/index.js +++ b/src/keychain/index.js @@ -110,7 +110,7 @@ class Keychain { this.opts = mergeOptions(defaultOptions, options) // Enforce NIST SP 800-132 - if (!this.opts.passPhrase || this.opts.passPhrase.length < 20) { + if (this.opts.passPhrase && this.opts.passPhrase.length < 20) { throw new Error('passPhrase must be least 20 characters') } if (this.opts.dek.keyLength < NIST.minKeyLength) { @@ -123,14 +123,22 @@ class Keychain { throw new Error(`dek.iterationCount must be least ${NIST.minIterationCount}`) } - // Create the derived encrypting key - const dek = crypto.pbkdf2( - this.opts.passPhrase, - this.opts.dek.salt, - this.opts.dek.iterationCount, - this.opts.dek.keyLength, - this.opts.dek.hash) - Object.defineProperty(this, '_', { value: () => dek }) + // Lazily create the derived encrypting key + let dek + Object.defineProperty(this, '_', { + value: () => { + if (!dek) { + dek = crypto.pbkdf2( + this.opts.passPhrase, + this.opts.dek.salt, + this.opts.dek.iterationCount, + this.opts.dek.keyLength, + this.opts.dek.hash) + } + + return dek + } + }) } /** diff --git a/test/keychain/keychain.spec.js b/test/keychain/keychain.spec.js index 73ff15bd78..c9ce249da1 100644 --- a/test/keychain/keychain.spec.js +++ b/test/keychain/keychain.spec.js @@ -2,10 +2,8 @@ /* eslint-env mocha */ 'use strict' -const chai = require('chai') -const { expect } = chai +const { chai, expect } = require('aegir/utils/chai') const fail = expect.fail -chai.use(require('dirty-chai')) chai.use(require('chai-string')) const peerUtils = require('../utils/creators/peer') @@ -40,8 +38,8 @@ describe('keychain', () => { emptyKeystore = new Keychain(datastore1, { passPhrase: passPhrase }) }) - it('needs a pass phrase to encrypt a key', () => { - expect(() => new Keychain(datastore2)).to.throw() + it('can start without a password', () => { + expect(() => new Keychain(datastore2)).to.not.throw() }) it('needs a NIST SP 800-132 non-weak pass phrase', () => { @@ -56,10 +54,48 @@ describe('keychain', () => { expect(Keychain.options).to.exist() }) - it('needs a supported hashing alorithm', () => { + it('supports supported hashing alorithms', () => { const ok = new Keychain(datastore2, { passPhrase: passPhrase, dek: { hash: 'sha2-256' } }) expect(ok).to.exist() - expect(() => new Keychain(datastore2, { passPhrase: passPhrase, dek: { hash: 'my-hash' } })).to.throw() + }) + + it('does not support unsupported hashing alorithms', () => { + const keychain = new Keychain(datastore2, { passPhrase: passPhrase, dek: { hash: 'my-hash' } }) + + expect(keychain.createKey('derp')).to.be.rejected() + }) + + it('can list keys without a password', async () => { + const keychain = new Keychain(datastore2) + + expect(await keychain.listKeys()).to.have.lengthOf(0) + }) + + it('can find a key without a password', async () => { + const keychain = new Keychain(datastore2) + const keychainWithPassword = new Keychain(datastore2, { passPhrase: `hello-${Date.now()}-${Date.now()}` }) + const id = `key-${Math.random()}` + + await keychainWithPassword.createKey(id, 'rsa', 2048) + + await expect(keychain.findKeyById(id)).to.eventually.be.ok() + }) + + it('can remove a key without a password', async () => { + const keychainWithoutPassword = new Keychain(datastore2) + const keychainWithPassword = new Keychain(datastore2, { passPhrase: `hello-${Date.now()}-${Date.now()}` }) + const name = `key-${Math.random()}` + + expect(await keychainWithPassword.createKey(name, 'rsa', 2048)).to.have.property('name', name) + expect(await keychainWithoutPassword.findKeyByName(name)).to.have.property('name', name) + await keychainWithoutPassword.removeKey(name) + await expect(keychainWithoutPassword.findKeyByName(name)).to.be.rejectedWith(/does not exist/) + }) + + it('requires a key to create a password', async () => { + const keychain = new Keychain(datastore2) + + await expect(keychain.createKey('derp')).to.be.rejected() }) it('can generate options', () => { From 80f015eaaafa007edeab8d0a67c99e18f363ffa6 Mon Sep 17 00:00:00 2001 From: Jacob Heun Date: Wed, 5 Aug 2020 17:48:18 +0200 Subject: [PATCH 2/3] fix: make keychain pass optional --- src/keychain/index.js | 24 ++++++++---------------- test/keychain/keychain.spec.js | 4 +--- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/keychain/index.js b/src/keychain/index.js index bf2b55537d..ef6aac7dee 100644 --- a/src/keychain/index.js +++ b/src/keychain/index.js @@ -123,22 +123,14 @@ class Keychain { throw new Error(`dek.iterationCount must be least ${NIST.minIterationCount}`) } - // Lazily create the derived encrypting key - let dek - Object.defineProperty(this, '_', { - value: () => { - if (!dek) { - dek = crypto.pbkdf2( - this.opts.passPhrase, - this.opts.dek.salt, - this.opts.dek.iterationCount, - this.opts.dek.keyLength, - this.opts.dek.hash) - } - - return dek - } - }) + const dek = this.opts.passPhrase ? crypto.pbkdf2( + this.opts.passPhrase, + this.opts.dek.salt, + this.opts.dek.iterationCount, + this.opts.dek.keyLength, + this.opts.dek.hash) : '' + + Object.defineProperty(this, '_', { value: () => dek }) } /** diff --git a/test/keychain/keychain.spec.js b/test/keychain/keychain.spec.js index c9ce249da1..84a84ef228 100644 --- a/test/keychain/keychain.spec.js +++ b/test/keychain/keychain.spec.js @@ -60,9 +60,7 @@ describe('keychain', () => { }) it('does not support unsupported hashing alorithms', () => { - const keychain = new Keychain(datastore2, { passPhrase: passPhrase, dek: { hash: 'my-hash' } }) - - expect(keychain.createKey('derp')).to.be.rejected() + expect(() => new Keychain(datastore2, { passPhrase: passPhrase, dek: { hash: 'my-hash' } })).to.throw() }) it('can list keys without a password', async () => { From 386c3d31ad49c86c5d47432d489e7567aad36428 Mon Sep 17 00:00:00 2001 From: Jacob Heun Date: Wed, 5 Aug 2020 18:20:45 +0200 Subject: [PATCH 3/3] fix: support libp2p creation without keychain pass --- src/index.js | 2 +- test/keychain/keychain.spec.js | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index 4d5e578caa..c9e249764e 100644 --- a/src/index.js +++ b/src/index.js @@ -82,7 +82,7 @@ class Libp2p extends EventEmitter { } // Create keychain - if (this._options.keychain && this._options.keychain.pass && this._options.keychain.datastore) { + if (this._options.keychain && this._options.keychain.datastore) { log('creating keychain') const keychainOpts = Keychain.generateOptions() diff --git a/test/keychain/keychain.spec.js b/test/keychain/keychain.spec.js index 84a84ef228..0451cd37fc 100644 --- a/test/keychain/keychain.spec.js +++ b/test/keychain/keychain.spec.js @@ -509,7 +509,7 @@ describe('libp2p.keychain', () => { throw new Error('should throw an error using the keychain if no passphrase provided') }) - it('can be used if a passphrase is provided', async () => { + it('can be used when a passphrase is provided', async () => { const [libp2p] = await peerUtils.createPeer({ started: false, config: { @@ -526,6 +526,22 @@ describe('libp2p.keychain', () => { expect(kInfo).to.exist() }) + it('does not require a keychain passphrase', async () => { + const [libp2p] = await peerUtils.createPeer({ + started: false, + config: { + keychain: { + datastore: new MemoryDatastore() + } + } + }) + + await libp2p.loadKeychain() + + const kInfo = await libp2p.keychain.createKey('keyName', 'ed25519') + expect(kInfo).to.exist() + }) + it('can reload keys', async () => { const datastore = new MemoryDatastore() const [libp2p] = await peerUtils.createPeer({