From c852379bec67f9c62645f3bc80ff3cd966807c0a Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 11:47:39 +0200 Subject: [PATCH 01/21] feat: addSeeds --- features/keychain/module/keychain.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index c2417db1..bb5f1826 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -42,6 +42,10 @@ export class Keychain { this.secp256k1 = secp256k1.create({ getPrivateHDKey: this.#getPrivateHDKey }) } + addSeeds(seeds) { + return seeds.map((seed) => this.addSeed(seed)) + } + addSeed(seed) { assert(Buffer.isBuffer(seed) && seed.length === 64, 'seed must be buffer of 64 bytes') const masters = Object.assign( From 24632c346a55ee16f862e3700bc2e9e35bb57656 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 11:48:00 +0200 Subject: [PATCH 02/21] feat: lockPrivateKeys and unlockPrivateKeys --- features/keychain/module/keychain.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index bb5f1826..7a6cb41a 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -25,6 +25,7 @@ export const MODULE_ID = 'keychain' export class Keychain { #masters = Object.create(null) #legacyPrivToPub = null + #lockedPrivateKeys = false // TODO: remove default param. Use it temporarily for backward compatibility. constructor({ legacyPrivToPub = Object.create(null) }) { @@ -42,6 +43,25 @@ export class Keychain { this.secp256k1 = secp256k1.create({ getPrivateHDKey: this.#getPrivateHDKey }) } + lockPrivateKeys() { + this.#lockedPrivateKeys = true + } + + unlockPrivateKeys(seeds) { + assert( + seeds?.length === Object.values(this.#masters).length, + 'must pass in same number of seeds' + ) + for (const seed of seeds) { + const seedId = getSeedId(seed) + assert(!!this.#masters[seedId], 'must pass in existing seed') + } + + this.removeAllSeeds() + this.#lockedPrivateKeys = false + return this.addSeeds(seeds) + } + addSeeds(seeds) { return seeds.map((seed) => this.addSeed(seed)) } From c3562925b0a4778a5d692e463ad204baacd00346 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 11:51:56 +0200 Subject: [PATCH 03/21] feat: block private key usage while locked --- features/keychain/module/keychain.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index 7a6cb41a..c6f29e33 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -84,6 +84,7 @@ export class Keychain { } #getPrivateHDKey = ({ seedId, keyId }) => { + assert(!this.#lockedPrivateKeys, 'private keys are not locked') throwIfInvalidKeyIdentifier(keyId) assert(typeof seedId === 'string', 'seedId must be a BIP32 key identifier in hex encoding') @@ -125,6 +126,7 @@ export class Keychain { } async signTx({ seedId, keyIds, signTxCallback, unsignedTx }) { + assert(!this.#lockedPrivateKeys, 'private keys are not locked') assert(typeof signTxCallback === 'function', 'signTxCallback must be a function') const hdkeys = Object.fromEntries( keyIds.map((keyId) => { From 64a11ad2ee318910adefadc09fe10a768954f0c2 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 12:14:06 +0200 Subject: [PATCH 04/21] feat: allow reading getPrivateHDKey to return public keys --- features/keychain/module/keychain.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index c6f29e33..dc76a153 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -26,6 +26,7 @@ export class Keychain { #masters = Object.create(null) #legacyPrivToPub = null #lockedPrivateKeys = false + #canUsePrivateKeysSymbol = Symbol('canUsePrivateKeys') // TODO: remove default param. Use it temporarily for backward compatibility. constructor({ legacyPrivToPub = Object.create(null) }) { @@ -83,8 +84,9 @@ export class Keychain { this.#masters = Object.create(null) } - #getPrivateHDKey = ({ seedId, keyId }) => { - assert(!this.#lockedPrivateKeys, 'private keys are not locked') + #getPrivateHDKey = ({ seedId, keyId, canUsePrivateKeysSymbol }) => { + if (canUsePrivateKeysSymbol !== this.#canUsePrivateKeysSymbol) + assert(!this.#lockedPrivateKeys, 'private keys are not locked') throwIfInvalidKeyIdentifier(keyId) assert(typeof seedId === 'string', 'seedId must be a BIP32 key identifier in hex encoding') @@ -99,7 +101,11 @@ export class Keychain { async exportKey({ seedId, keyId, exportPrivate }) { keyId = new KeyIdentifier(keyId) - const hdkey = this.#getPrivateHDKey({ seedId, keyId }) + const hdkey = this.#getPrivateHDKey({ + seedId, + keyId, + canUsePrivateKeysSymbol: this.#canUsePrivateKeysSymbol, + }) const privateKey = hdkey.privateKey let publicKey = hdkey.publicKey From 2b2f4ce8754d859288838a2b5cf5781d305e6dd5 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 12:16:40 +0200 Subject: [PATCH 05/21] feat: don't allow privateKey export while locked --- features/keychain/module/keychain.js | 1 + 1 file changed, 1 insertion(+) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index dc76a153..55f02843 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -99,6 +99,7 @@ export class Keychain { } async exportKey({ seedId, keyId, exportPrivate }) { + if (exportPrivate) assert(!this.#lockedPrivateKeys, 'private keys are not locked') keyId = new KeyIdentifier(keyId) const hdkey = this.#getPrivateHDKey({ From b9d51503d29d290fb1f787ab0da15065a547109e Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 12:18:51 +0200 Subject: [PATCH 06/21] refactor canUsePrivateKeysSymbol -> getPrivateHDKeySymbol --- features/keychain/module/keychain.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index 55f02843..5a5c0a90 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -26,7 +26,7 @@ export class Keychain { #masters = Object.create(null) #legacyPrivToPub = null #lockedPrivateKeys = false - #canUsePrivateKeysSymbol = Symbol('canUsePrivateKeys') + #getPrivateHDKeySymbol = Symbol('getPrivateHDKey') // TODO: remove default param. Use it temporarily for backward compatibility. constructor({ legacyPrivToPub = Object.create(null) }) { @@ -84,8 +84,8 @@ export class Keychain { this.#masters = Object.create(null) } - #getPrivateHDKey = ({ seedId, keyId, canUsePrivateKeysSymbol }) => { - if (canUsePrivateKeysSymbol !== this.#canUsePrivateKeysSymbol) + #getPrivateHDKey = ({ seedId, keyId, getPrivateHDKeySymbol }) => { + if (getPrivateHDKeySymbol !== this.#getPrivateHDKeySymbol) assert(!this.#lockedPrivateKeys, 'private keys are not locked') throwIfInvalidKeyIdentifier(keyId) @@ -105,7 +105,7 @@ export class Keychain { const hdkey = this.#getPrivateHDKey({ seedId, keyId, - canUsePrivateKeysSymbol: this.#canUsePrivateKeysSymbol, + getPrivateHDKeySymbol: this.#getPrivateHDKeySymbol, }) const privateKey = hdkey.privateKey let publicKey = hdkey.publicKey From aacf2aa5076d2bff37c33d7a11cee7fb7ae5d5d8 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 12:52:48 +0200 Subject: [PATCH 07/21] feat: block clone when locked --- features/keychain/module/keychain.js | 1 + 1 file changed, 1 insertion(+) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index 5a5c0a90..246909c1 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -166,6 +166,7 @@ export class Keychain { } clone() { + assert(!this.#lockedPrivateKeys, 'private keys are not locked') return new Keychain({ legacyPrivToPub: this.#legacyPrivToPub }) } From d8014357e739bbad6f58c6bc28110004053ab938 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 12:53:08 +0200 Subject: [PATCH 08/21] feat: add tests for locked keychain --- .../__tests__/lock-private-keys.test.js | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 features/keychain/module/__tests__/lock-private-keys.test.js diff --git a/features/keychain/module/__tests__/lock-private-keys.test.js b/features/keychain/module/__tests__/lock-private-keys.test.js new file mode 100644 index 00000000..4a6cbafe --- /dev/null +++ b/features/keychain/module/__tests__/lock-private-keys.test.js @@ -0,0 +1,114 @@ +import { mnemonicToSeed } from 'bip39' +import { createKeyIdentifierForExodus } from '@exodus/key-ids' +import KeyIdentifier from '@exodus/key-identifier' +import createKeychain from './create-keychain' +import { getSeedId } from '../crypto/seed-id' + +const seed = mnemonicToSeed( + 'menu memory fury language physical wonder dog valid smart edge decrease worth' +) + +const seedId = getSeedId(seed) + +describe('lockPrivateKeys', () => { + it('should allow private key usage when unlocked', async () => { + const keychain = createKeychain({ seed }) + const keyId = createKeyIdentifierForExodus({ exoType: 'FUSION' }) + const exportedKeys = await keychain.exportKey({ + seedId, + keyId, + exportPrivate: true, + }) + + // Public keys should be the same + const sodiumEncryptor = keychain.createSodiumEncryptor(keyId) + const { + sign: { publicKey }, + } = await sodiumEncryptor.getSodiumKeysFromSeed({ seedId }) + expect(Buffer.compare(publicKey, exportedKeys.publicKey)).toBe(0) + }) + + it('should allow addSeed when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + keychain.addSeed(seed) + }) + + it('should allow addSeeds when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + keychain.addSeeds([seed]) + }) + + it('should allow removeAllSeeds when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + keychain.removeAllSeeds() + }) + + it('should allow exportKey for public keys when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + const keyId = createKeyIdentifierForExodus({ exoType: 'FUSION' }) + const exportedKeys = await keychain.exportKey({ + seedId, + keyId, + }) + + expect(!!exportedKeys.publicKey).toBe(true) + }) + + it('should block exportKey for private keys when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + const keyId = createKeyIdentifierForExodus({ exoType: 'FUSION' }) + await expect( + keychain.exportKey({ + seedId, + keyId, + exportPrivate: true, + }) + ).rejects.toThrow(/private keys are not locked/) + }) + + it('should block signTx when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + await expect(keychain.signTx({})).rejects.toThrow(/private keys are not locked/) + }) + + it('should block clone when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + await expect(async () => keychain.clone()).rejects.toThrow(/private keys are not locked/) + }) + + it('should block sodium when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + await expect(keychain.sodium.getSodiumKeysFromSeed({})).rejects.toThrow( + /private keys are not locked/ + ) + await expect(keychain.createSodiumEncryptor({}).getSodiumKeysFromSeed({})).rejects.toThrow( + /private keys are not locked/ + ) + }) + + it('should block ed25519 when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + await expect(keychain.ed25519.signBuffer({})).rejects.toThrow(/private keys are not locked/) + await expect(keychain.createEd25519Signer({}).signBuffer({})).rejects.toThrow( + /private keys are not locked/ + ) + }) + + it('should block secp256k1 when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + await expect(keychain.secp256k1.signBuffer({})).rejects.toThrow(/private keys are not locked/) + await expect(keychain.createSecp256k1Signer({}).signBuffer({})).rejects.toThrow( + /private keys are not locked/ + ) + }) +}) From 8e3ed88d6a20886bcb9e089e3156d4998eb82334 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 12:57:43 +0200 Subject: [PATCH 09/21] feat: unlock tests --- .../__tests__/lock-private-keys.test.js | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/features/keychain/module/__tests__/lock-private-keys.test.js b/features/keychain/module/__tests__/lock-private-keys.test.js index 4a6cbafe..2f97bc31 100644 --- a/features/keychain/module/__tests__/lock-private-keys.test.js +++ b/features/keychain/module/__tests__/lock-private-keys.test.js @@ -8,6 +8,10 @@ const seed = mnemonicToSeed( 'menu memory fury language physical wonder dog valid smart edge decrease worth' ) +const seed1 = mnemonicToSeed( + 'menu memory fury language physical wonder dog valid smart edge decrease test' +) + const seedId = getSeedId(seed) describe('lockPrivateKeys', () => { @@ -58,6 +62,45 @@ describe('lockPrivateKeys', () => { expect(!!exportedKeys.publicKey).toBe(true) }) + it('should allow exportKeys after lock/unlock', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + keychain.unlockPrivateKeys([seed]) + + const keyId = createKeyIdentifierForExodus({ exoType: 'FUSION' }) + const exportedKeys = await keychain.exportKey({ + seedId, + keyId, + exportPrivate: true, + }) + + // Public keys should be the same + const sodiumEncryptor = keychain.createSodiumEncryptor(keyId) + const { + sign: { publicKey }, + } = await sodiumEncryptor.getSodiumKeysFromSeed({ seedId }) + expect(Buffer.compare(publicKey, exportedKeys.publicKey)).toBe(0) + }) + + it('should block unlock for wrong seeds length', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + await expect(async () => keychain.unlockPrivateKeys([])).rejects.toThrow( + /must pass in same number of seeds/ + ) + await expect(async () => keychain.unlockPrivateKeys([seed, seed])).rejects.toThrow( + /must pass in same number of seeds/ + ) + }) + + it('should block unlock for wrong seed ids', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + await expect(async () => keychain.unlockPrivateKeys([seed1])).rejects.toThrow( + /must pass in existing seed/ + ) + }) + it('should block exportKey for private keys when locked', async () => { const keychain = createKeychain({ seed }) keychain.lockPrivateKeys() From f279d8cdcd2903b5c9493637c0140cfd8765f10e Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 13:00:27 +0200 Subject: [PATCH 10/21] feat: don't allow double unlock --- features/keychain/module/__tests__/lock-private-keys.test.js | 5 +++++ features/keychain/module/keychain.js | 1 + 2 files changed, 6 insertions(+) diff --git a/features/keychain/module/__tests__/lock-private-keys.test.js b/features/keychain/module/__tests__/lock-private-keys.test.js index 2f97bc31..f3c22829 100644 --- a/features/keychain/module/__tests__/lock-private-keys.test.js +++ b/features/keychain/module/__tests__/lock-private-keys.test.js @@ -93,6 +93,11 @@ describe('lockPrivateKeys', () => { ) }) + it('should block unlock when already unlocked', async () => { + const keychain = createKeychain({ seed }) + await expect(async () => keychain.unlockPrivateKeys([seed])).rejects.toThrow(/already unlocked/) + }) + it('should block unlock for wrong seed ids', async () => { const keychain = createKeychain({ seed }) keychain.lockPrivateKeys() diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index 246909c1..6c65603e 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -49,6 +49,7 @@ export class Keychain { } unlockPrivateKeys(seeds) { + assert(this.#lockedPrivateKeys, 'already unlocked') assert( seeds?.length === Object.values(this.#masters).length, 'must pass in same number of seeds' From 64c7e5f1e265382f80e15b814df0cdceee5f4f13 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 13:03:55 +0200 Subject: [PATCH 11/21] feat: add hasLockedPrivateKeys --- features/keychain/module/keychain.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index 6c65603e..16cf4de3 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -44,6 +44,10 @@ export class Keychain { this.secp256k1 = secp256k1.create({ getPrivateHDKey: this.#getPrivateHDKey }) } + hasLockedPrivateKeys() { + return this.#lockedPrivateKeys + } + lockPrivateKeys() { this.#lockedPrivateKeys = true } From da1c4283abe6e0b7901281d6475b5e8c6a7dfb74 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 13:14:08 +0200 Subject: [PATCH 12/21] refactor: error message --- .../module/__tests__/lock-private-keys.test.js | 18 +++++++++--------- features/keychain/module/keychain.js | 8 ++++---- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/features/keychain/module/__tests__/lock-private-keys.test.js b/features/keychain/module/__tests__/lock-private-keys.test.js index f3c22829..0398cc80 100644 --- a/features/keychain/module/__tests__/lock-private-keys.test.js +++ b/features/keychain/module/__tests__/lock-private-keys.test.js @@ -116,47 +116,47 @@ describe('lockPrivateKeys', () => { keyId, exportPrivate: true, }) - ).rejects.toThrow(/private keys are not locked/) + ).rejects.toThrow(/private keys are locked/) }) it('should block signTx when locked', async () => { const keychain = createKeychain({ seed }) keychain.lockPrivateKeys() - await expect(keychain.signTx({})).rejects.toThrow(/private keys are not locked/) + await expect(keychain.signTx({})).rejects.toThrow(/private keys are locked/) }) it('should block clone when locked', async () => { const keychain = createKeychain({ seed }) keychain.lockPrivateKeys() - await expect(async () => keychain.clone()).rejects.toThrow(/private keys are not locked/) + await expect(async () => keychain.clone()).rejects.toThrow(/private keys are locked/) }) it('should block sodium when locked', async () => { const keychain = createKeychain({ seed }) keychain.lockPrivateKeys() await expect(keychain.sodium.getSodiumKeysFromSeed({})).rejects.toThrow( - /private keys are not locked/ + /private keys are locked/ ) await expect(keychain.createSodiumEncryptor({}).getSodiumKeysFromSeed({})).rejects.toThrow( - /private keys are not locked/ + /private keys are locked/ ) }) it('should block ed25519 when locked', async () => { const keychain = createKeychain({ seed }) keychain.lockPrivateKeys() - await expect(keychain.ed25519.signBuffer({})).rejects.toThrow(/private keys are not locked/) + await expect(keychain.ed25519.signBuffer({})).rejects.toThrow(/private keys are locked/) await expect(keychain.createEd25519Signer({}).signBuffer({})).rejects.toThrow( - /private keys are not locked/ + /private keys are locked/ ) }) it('should block secp256k1 when locked', async () => { const keychain = createKeychain({ seed }) keychain.lockPrivateKeys() - await expect(keychain.secp256k1.signBuffer({})).rejects.toThrow(/private keys are not locked/) + await expect(keychain.secp256k1.signBuffer({})).rejects.toThrow(/private keys are locked/) await expect(keychain.createSecp256k1Signer({}).signBuffer({})).rejects.toThrow( - /private keys are not locked/ + /private keys are locked/ ) }) }) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index 16cf4de3..cb59090d 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -91,7 +91,7 @@ export class Keychain { #getPrivateHDKey = ({ seedId, keyId, getPrivateHDKeySymbol }) => { if (getPrivateHDKeySymbol !== this.#getPrivateHDKeySymbol) - assert(!this.#lockedPrivateKeys, 'private keys are not locked') + assert(!this.#lockedPrivateKeys, 'private keys are locked') throwIfInvalidKeyIdentifier(keyId) assert(typeof seedId === 'string', 'seedId must be a BIP32 key identifier in hex encoding') @@ -104,7 +104,7 @@ export class Keychain { } async exportKey({ seedId, keyId, exportPrivate }) { - if (exportPrivate) assert(!this.#lockedPrivateKeys, 'private keys are not locked') + if (exportPrivate) assert(!this.#lockedPrivateKeys, 'private keys are locked') keyId = new KeyIdentifier(keyId) const hdkey = this.#getPrivateHDKey({ @@ -138,7 +138,7 @@ export class Keychain { } async signTx({ seedId, keyIds, signTxCallback, unsignedTx }) { - assert(!this.#lockedPrivateKeys, 'private keys are not locked') + assert(!this.#lockedPrivateKeys, 'private keys are locked') assert(typeof signTxCallback === 'function', 'signTxCallback must be a function') const hdkeys = Object.fromEntries( keyIds.map((keyId) => { @@ -171,7 +171,7 @@ export class Keychain { } clone() { - assert(!this.#lockedPrivateKeys, 'private keys are not locked') + assert(!this.#lockedPrivateKeys, 'private keys are locked') return new Keychain({ legacyPrivToPub: this.#legacyPrivToPub }) } From 71dafa20deec142295c917ac94d9d196a221cbae Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Mon, 1 Apr 2024 13:27:04 +0200 Subject: [PATCH 13/21] lint --- features/keychain/module/__tests__/lock-private-keys.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/features/keychain/module/__tests__/lock-private-keys.test.js b/features/keychain/module/__tests__/lock-private-keys.test.js index 0398cc80..6b87a451 100644 --- a/features/keychain/module/__tests__/lock-private-keys.test.js +++ b/features/keychain/module/__tests__/lock-private-keys.test.js @@ -1,6 +1,5 @@ import { mnemonicToSeed } from 'bip39' import { createKeyIdentifierForExodus } from '@exodus/key-ids' -import KeyIdentifier from '@exodus/key-identifier' import createKeychain from './create-keychain' import { getSeedId } from '../crypto/seed-id' From 45c8994f960ab61806d466e2959ae683f21e6350 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Tue, 2 Apr 2024 15:53:17 +0200 Subject: [PATCH 14/21] refactor: lockedPrivateKeys -> privateKeysAreLocked --- features/keychain/module/keychain.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index cb59090d..83097be1 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -25,7 +25,7 @@ export const MODULE_ID = 'keychain' export class Keychain { #masters = Object.create(null) #legacyPrivToPub = null - #lockedPrivateKeys = false + #privateKeysAreLocked = false #getPrivateHDKeySymbol = Symbol('getPrivateHDKey') // TODO: remove default param. Use it temporarily for backward compatibility. @@ -45,15 +45,15 @@ export class Keychain { } hasLockedPrivateKeys() { - return this.#lockedPrivateKeys + return this.#privateKeysAreLocked } lockPrivateKeys() { - this.#lockedPrivateKeys = true + this.#privateKeysAreLocked = true } unlockPrivateKeys(seeds) { - assert(this.#lockedPrivateKeys, 'already unlocked') + assert(this.#privateKeysAreLocked, 'already unlocked') assert( seeds?.length === Object.values(this.#masters).length, 'must pass in same number of seeds' @@ -64,7 +64,7 @@ export class Keychain { } this.removeAllSeeds() - this.#lockedPrivateKeys = false + this.#privateKeysAreLocked = false return this.addSeeds(seeds) } @@ -91,7 +91,7 @@ export class Keychain { #getPrivateHDKey = ({ seedId, keyId, getPrivateHDKeySymbol }) => { if (getPrivateHDKeySymbol !== this.#getPrivateHDKeySymbol) - assert(!this.#lockedPrivateKeys, 'private keys are locked') + assert(!this.#privateKeysAreLocked, 'private keys are locked') throwIfInvalidKeyIdentifier(keyId) assert(typeof seedId === 'string', 'seedId must be a BIP32 key identifier in hex encoding') @@ -104,7 +104,7 @@ export class Keychain { } async exportKey({ seedId, keyId, exportPrivate }) { - if (exportPrivate) assert(!this.#lockedPrivateKeys, 'private keys are locked') + if (exportPrivate) assert(!this.#privateKeysAreLocked, 'private keys are locked') keyId = new KeyIdentifier(keyId) const hdkey = this.#getPrivateHDKey({ @@ -138,7 +138,7 @@ export class Keychain { } async signTx({ seedId, keyIds, signTxCallback, unsignedTx }) { - assert(!this.#lockedPrivateKeys, 'private keys are locked') + assert(!this.#privateKeysAreLocked, 'private keys are locked') assert(typeof signTxCallback === 'function', 'signTxCallback must be a function') const hdkeys = Object.fromEntries( keyIds.map((keyId) => { @@ -171,7 +171,7 @@ export class Keychain { } clone() { - assert(!this.#lockedPrivateKeys, 'private keys are locked') + assert(!this.#privateKeysAreLocked, 'private keys are locked') return new Keychain({ legacyPrivToPub: this.#legacyPrivToPub }) } From b9cb85e735166d6b128cba960825c31fa5c548d7 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Tue, 2 Apr 2024 15:55:35 +0200 Subject: [PATCH 15/21] refactor: extract this.#assertPrivateKeysUnlocked() --- features/keychain/module/keychain.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index 83097be1..486e3d9a 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -44,6 +44,10 @@ export class Keychain { this.secp256k1 = secp256k1.create({ getPrivateHDKey: this.#getPrivateHDKey }) } + #assertPrivateKeysUnlocked() { + assert(!this.#privateKeysAreLocked, 'private keys are locked') + } + hasLockedPrivateKeys() { return this.#privateKeysAreLocked } @@ -90,8 +94,7 @@ export class Keychain { } #getPrivateHDKey = ({ seedId, keyId, getPrivateHDKeySymbol }) => { - if (getPrivateHDKeySymbol !== this.#getPrivateHDKeySymbol) - assert(!this.#privateKeysAreLocked, 'private keys are locked') + if (getPrivateHDKeySymbol !== this.#getPrivateHDKeySymbol) this.#assertPrivateKeysUnlocked() throwIfInvalidKeyIdentifier(keyId) assert(typeof seedId === 'string', 'seedId must be a BIP32 key identifier in hex encoding') @@ -104,7 +107,7 @@ export class Keychain { } async exportKey({ seedId, keyId, exportPrivate }) { - if (exportPrivate) assert(!this.#privateKeysAreLocked, 'private keys are locked') + if (exportPrivate) this.#assertPrivateKeysUnlocked() keyId = new KeyIdentifier(keyId) const hdkey = this.#getPrivateHDKey({ @@ -138,7 +141,7 @@ export class Keychain { } async signTx({ seedId, keyIds, signTxCallback, unsignedTx }) { - assert(!this.#privateKeysAreLocked, 'private keys are locked') + this.#assertPrivateKeysUnlocked() assert(typeof signTxCallback === 'function', 'signTxCallback must be a function') const hdkeys = Object.fromEntries( keyIds.map((keyId) => { @@ -171,7 +174,7 @@ export class Keychain { } clone() { - assert(!this.#privateKeysAreLocked, 'private keys are locked') + this.#assertPrivateKeysUnlocked() return new Keychain({ legacyPrivToPub: this.#legacyPrivToPub }) } From f019ea84779df5cf1fee05a7145e9e028b76f183 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Tue, 2 Apr 2024 15:56:50 +0200 Subject: [PATCH 16/21] feat: don't re-add seeds --- .../keychain/module/__tests__/lock-private-keys.test.js | 6 ------ features/keychain/module/keychain.js | 6 ------ 2 files changed, 12 deletions(-) diff --git a/features/keychain/module/__tests__/lock-private-keys.test.js b/features/keychain/module/__tests__/lock-private-keys.test.js index 6b87a451..9bd50b92 100644 --- a/features/keychain/module/__tests__/lock-private-keys.test.js +++ b/features/keychain/module/__tests__/lock-private-keys.test.js @@ -37,12 +37,6 @@ describe('lockPrivateKeys', () => { keychain.addSeed(seed) }) - it('should allow addSeeds when locked', async () => { - const keychain = createKeychain({ seed }) - keychain.lockPrivateKeys() - keychain.addSeeds([seed]) - }) - it('should allow removeAllSeeds when locked', async () => { const keychain = createKeychain({ seed }) keychain.lockPrivateKeys() diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index 486e3d9a..f02e43c1 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -67,13 +67,7 @@ export class Keychain { assert(!!this.#masters[seedId], 'must pass in existing seed') } - this.removeAllSeeds() this.#privateKeysAreLocked = false - return this.addSeeds(seeds) - } - - addSeeds(seeds) { - return seeds.map((seed) => this.addSeed(seed)) } addSeed(seed) { From 698a2073524e3b2d3754211295eeef893700455a Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Tue, 2 Apr 2024 16:05:14 +0200 Subject: [PATCH 17/21] feat: failing test for unlockPrivateKeys --- .../keychain/module/__tests__/lock-private-keys.test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/features/keychain/module/__tests__/lock-private-keys.test.js b/features/keychain/module/__tests__/lock-private-keys.test.js index 9bd50b92..9f28b98f 100644 --- a/features/keychain/module/__tests__/lock-private-keys.test.js +++ b/features/keychain/module/__tests__/lock-private-keys.test.js @@ -97,6 +97,13 @@ describe('lockPrivateKeys', () => { await expect(async () => keychain.unlockPrivateKeys([seed1])).rejects.toThrow( /must pass in existing seed/ ) + + const keychain1 = createKeychain({ seed }) + keychain1.addSeed(seed1) + keychain1.lockPrivateKeys() + await expect(async () => keychain1.unlockPrivateKeys([seed, seed])).rejects.toThrow( + /must pass in existing seed/ + ) }) it('should block exportKey for private keys when locked', async () => { From 4c518d489f303c4701cf2b48624798656c890b73 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Tue, 2 Apr 2024 16:07:28 +0200 Subject: [PATCH 18/21] fix: unlockPrivateKeys should check all seeds are passed in --- features/keychain/module/keychain.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index f02e43c1..65e676d9 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -62,9 +62,9 @@ export class Keychain { seeds?.length === Object.values(this.#masters).length, 'must pass in same number of seeds' ) - for (const seed of seeds) { - const seedId = getSeedId(seed) - assert(!!this.#masters[seedId], 'must pass in existing seed') + const seedIds = new Set(seeds.map((seed) => getSeedId(seed))) + for (const seedId of Object.keys(this.#masters)) { + assert(seedIds.has(seedId), 'must pass in existing seed') } this.#privateKeysAreLocked = false From 9ccde2b26705f304e0c0e245ff5c5425d2c5d733 Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Tue, 2 Apr 2024 16:09:56 +0200 Subject: [PATCH 19/21] revert: allow clone --- .../module/__tests__/lock-private-keys.test.js | 12 ++++++------ features/keychain/module/keychain.js | 1 - 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/features/keychain/module/__tests__/lock-private-keys.test.js b/features/keychain/module/__tests__/lock-private-keys.test.js index 9f28b98f..b72e9f82 100644 --- a/features/keychain/module/__tests__/lock-private-keys.test.js +++ b/features/keychain/module/__tests__/lock-private-keys.test.js @@ -55,6 +55,12 @@ describe('lockPrivateKeys', () => { expect(!!exportedKeys.publicKey).toBe(true) }) + it('should allow clone when locked', async () => { + const keychain = createKeychain({ seed }) + keychain.lockPrivateKeys() + keychain.clone() + }) + it('should allow exportKeys after lock/unlock', async () => { const keychain = createKeychain({ seed }) keychain.lockPrivateKeys() @@ -125,12 +131,6 @@ describe('lockPrivateKeys', () => { await expect(keychain.signTx({})).rejects.toThrow(/private keys are locked/) }) - it('should block clone when locked', async () => { - const keychain = createKeychain({ seed }) - keychain.lockPrivateKeys() - await expect(async () => keychain.clone()).rejects.toThrow(/private keys are locked/) - }) - it('should block sodium when locked', async () => { const keychain = createKeychain({ seed }) keychain.lockPrivateKeys() diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index 65e676d9..889786ad 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -168,7 +168,6 @@ export class Keychain { } clone() { - this.#assertPrivateKeysUnlocked() return new Keychain({ legacyPrivToPub: this.#legacyPrivToPub }) } From 185ca1e3a268054a53d515668643f48fa955031c Mon Sep 17 00:00:00 2001 From: andrejborstnik Date: Tue, 2 Apr 2024 18:26:20 +0200 Subject: [PATCH 20/21] feat: export arePrivateKeysLocked in api --- features/keychain/api/index.js | 1 + features/keychain/module/keychain.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/features/keychain/api/index.js b/features/keychain/api/index.js index 4a242026..e148f977 100644 --- a/features/keychain/api/index.js +++ b/features/keychain/api/index.js @@ -2,6 +2,7 @@ const createKeychainApi = ({ keychain }) => { return { keychain: { exportKey: (...args) => keychain.exportKey(...args), + arePrivateKeysLocked: () => keychain.arePrivateKeysLocked(), sodium: { signDetached: keychain.sodium.signDetached, getKeysFromSeed: (...args) => diff --git a/features/keychain/module/keychain.js b/features/keychain/module/keychain.js index 889786ad..bd2bade5 100644 --- a/features/keychain/module/keychain.js +++ b/features/keychain/module/keychain.js @@ -48,7 +48,7 @@ export class Keychain { assert(!this.#privateKeysAreLocked, 'private keys are locked') } - hasLockedPrivateKeys() { + arePrivateKeysLocked() { return this.#privateKeysAreLocked } From 12bb54167e948ea7e993c3547e65d9fe3c41e059 Mon Sep 17 00:00:00 2001 From: Andrej Date: Thu, 4 Apr 2024 09:24:10 +0200 Subject: [PATCH 21/21] test nit Co-authored-by: Jan W --- features/keychain/module/__tests__/lock-private-keys.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/keychain/module/__tests__/lock-private-keys.test.js b/features/keychain/module/__tests__/lock-private-keys.test.js index b72e9f82..24d80baa 100644 --- a/features/keychain/module/__tests__/lock-private-keys.test.js +++ b/features/keychain/module/__tests__/lock-private-keys.test.js @@ -52,7 +52,7 @@ describe('lockPrivateKeys', () => { keyId, }) - expect(!!exportedKeys.publicKey).toBe(true) + expect(exportedKeys.publicKey).toBeDefined() }) it('should allow clone when locked', async () => {