From cc7b5ead407294f96cdac540015d20c444427ae2 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 26 Nov 2019 02:46:54 -0400 Subject: [PATCH] Replace `withAppKeyOrigin` option with `getAppKey` The new `getAppKey` method will return the private app key derived from the provided origin. This serves as an alternative to optionally augmenting each keyring method with a set of options that may contain a `withAppKeyOrigin` value. Moving app key generation to a single place makes it easier to limit access to sensitive information, such as the primary private keys. It also allows the consumer to use app keys more efficiently by using the same wallet for successive operations, rather than generating a new private key each time with `keccak`. The private app key returned can itself be used to instantiate a new keyring instance, which allows app keys to perform all the same functions that a 'normal' key would allow. --- index.js | 14 ++----- package.json | 2 +- test/index.js | 110 ++++++++++++++++++++++++++++++-------------------- 3 files changed, 71 insertions(+), 55 deletions(-) diff --git a/index.js b/index.js index 555af11..88ba1d6 100644 --- a/index.js +++ b/index.js @@ -85,7 +85,7 @@ class HdKeyring extends SimpleKeyring { } - _getWalletForAccount (account, opts = {}) { + _getWalletForAccount (account) { const targetAddress = sigUtil.normalize(account) let wallet = this.wallets.find((w) => { @@ -94,22 +94,14 @@ class HdKeyring extends SimpleKeyring { (sigUtil.normalize(address) === targetAddress)) }) - if (opts.withAppKeyOrigin) { - const privKey = wallet.getPrivateKey() - const appKeyOriginBuffer = Buffer.from(opts.withAppKeyOrigin, 'utf8') - const appKeyBuffer = Buffer.concat([privKey, appKeyOriginBuffer]) - const appKeyPrivKey = ethUtil.keccak(appKeyBuffer, 256) - wallet = Wallet.fromPrivateKey(appKeyPrivKey) - } - return wallet } - getPrivateKeyFor (address, opts = {}) { + getPrivateKeyFor (address) { if (!address) { throw new Error('Must specify address.'); } - const wallet = this._getWalletForAccount(address, opts) + const wallet = this._getWalletForAccount(address) const privKey = ethUtil.toBuffer(wallet.getPrivateKey()) return privKey; } diff --git a/package.json b/package.json index 99fb153..a5d70bf 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "dependencies": { "bip39": "^2.2.0", "eth-sig-util": "^2.4.4", - "eth-simple-keyring": "^3.4.0", + "eth-simple-keyring": "^4.0.0", "ethereumjs-abi": "^0.6.5", "ethereumjs-util": "^5.1.1", "ethereumjs-wallet": "^0.6.0", diff --git a/test/index.js b/test/index.js index 32f0923..e8f3abb 100644 --- a/test/index.js +++ b/test/index.js @@ -385,63 +385,87 @@ describe('hd-keyring', function() { }) }) - describe('signing methods withAppKeyOrigin option', function () { - it('should signPersonalMessage with the expected key when passed a withAppKeyOrigin', function (done) { + describe('getAppKey', function () { + it('should return a private key custom to the provided app origin', async function () { + keyring = new HdKeyring({ + mnemonic: sampleMnemonic, + numberOfAccounts: 1, + }) const address = firstAcct - const message = '0x68656c6c6f20776f726c64' + const hexKey = await keyring.exportAccount(address) - const privateKeyBuffer = Buffer.from('8e82d2d74c50e5c8460f771d38a560ebe1151a9134c65a7e92b28ad0cfae7151', 'hex') - const expectedSig = sigUtil.personalSign(privateKeyBuffer, { data: message }) + const appKey = await keyring.getAppKey(address, 'someapp.origin.io') - keyring.deserialize({ + assert.notEqual(hexKey, appKey.toString('hex')) + assert(ethUtil.isValidPrivate(appKey)) + }) + + it('should return different keyrings when provided different app origins', async function () { + keyring = new HdKeyring({ mnemonic: sampleMnemonic, numberOfAccounts: 1, }) - .then(() => { - return keyring.signPersonalMessage(address, message, { - withAppKeyOrigin: 'someapp.origin.io', - }) - }) - .then((sig) => { - assert.equal(sig, expectedSig, 'signed with app key') - done() - }) - .catch((reason) => { - assert(!reason, reason.message) - done() - }) + const address = firstAcct + + const appKey1 = await keyring.getAppKey(address, 'someapp.origin.io') + + assert(ethUtil.isValidPrivate(appKey1)) + + const appKey2 = await keyring.getAppKey(address, 'anotherapp.origin.io') + + assert(ethUtil.isValidPrivate(appKey2)) + + assert.notEqual(appKey1.toString('hex'), appKey2.toString('hex')) }) - it('should signTypedData with the expected key when passed a withAppKeyOrigin', function (done) { + it('should return the same keyring when called multiple times with the same params', async function () { + keyring = new HdKeyring({ + mnemonic: sampleMnemonic, + numberOfAccounts: 1, + }) const address = firstAcct - const typedData = { - types: { - EIP712Domain: [] - }, - domain: {}, - primaryType: 'EIP712Domain', - message: {} - } - const privateKeyBuffer = Buffer.from('8e82d2d74c50e5c8460f771d38a560ebe1151a9134c65a7e92b28ad0cfae7151', 'hex') - const expectedSig = sigUtil.signTypedData(privateKeyBuffer, { data: typedData }) + const appKey1 = await keyring.getAppKey(address, 'someapp.origin.io') - keyring.deserialize({ + assert(ethUtil.isValidPrivate(appKey1)) + + const appKey2 = await keyring.getAppKey(address, 'someapp.origin.io') + + assert(ethUtil.isValidPrivate(appKey2)) + + assert.equal(appKey1.toString('hex'), appKey2.toString('hex')) + }) + + it('should throw error if the provided origin is not a string', async function () { + keyring = new HdKeyring({ mnemonic: sampleMnemonic, - numberOfAccounts: 1 - }).then(() => { - return keyring.signTypedData_v3(address, typedData, { - withAppKeyOrigin: 'someapp.origin.io', - }) - }) - .then((sig) => { - assert.equal(sig, expectedSig, 'signed with app key') - done() + numberOfAccounts: 1, }) - .catch((reason) => { - assert(!reason, reason.message) - done() + const address = firstAcct + + try { + await keyring.getAppKey(address, []) + } catch (error) { + assert(error instanceof Error, 'Value thrown is not an error') + return + } + assert.fail('Should have thrown error') + }) + + it('should throw error if the provided origin is an empty string', async function () { + keyring = new HdKeyring({ + mnemonic: sampleMnemonic, + numberOfAccounts: 1, }) + const address = firstAcct + + try { + await keyring.getAppKey(address, '') + } catch (error) { + assert(error instanceof Error, 'Value thrown is not an error') + return + } + assert.fail('Should have thrown error') }) }) })