From 816bcabbea395cf45fec0735b189160898327cd9 Mon Sep 17 00:00:00 2001 From: James Chen Date: Wed, 28 Aug 2019 13:54:51 +0900 Subject: [PATCH 01/10] refactor: crypto.scryptSync accepts plain password string --- .../neuron-wallet/src/models/keys/keystore.ts | 32 +++++++------------ 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/packages/neuron-wallet/src/models/keys/keystore.ts b/packages/neuron-wallet/src/models/keys/keystore.ts index 97fe3f4ed4..b67364cab2 100644 --- a/packages/neuron-wallet/src/models/keys/keystore.ts +++ b/packages/neuron-wallet/src/models/keys/keystore.ts @@ -61,7 +61,7 @@ export default class Keystore { salt: salt.toString('hex'), ...params, } - const derivedKey: Buffer = crypto.scryptSync(Buffer.from(password), salt, kdfparams.dklen, { + const derivedKey: Buffer = crypto.scryptSync(password, salt, kdfparams.dklen, { N: kdfparams.n, r: kdfparams.r, p: kdfparams.p, @@ -99,16 +99,11 @@ export default class Keystore { // Decrypt and return serialized extended private key. decrypt(password: string): string { const { kdfparams } = this.crypto - const derivedKey: Buffer = crypto.scryptSync( - Buffer.from(password), - Buffer.from(kdfparams.salt, 'hex'), - kdfparams.dklen, - { - N: kdfparams.n, - r: kdfparams.r, - p: kdfparams.p, - } - ) + const derivedKey: Buffer = crypto.scryptSync(password, Buffer.from(kdfparams.salt, 'hex'), kdfparams.dklen, { + N: kdfparams.n, + r: kdfparams.r, + p: kdfparams.p, + }) const ciphertext = Buffer.from(this.crypto.ciphertext, 'hex') const mac = new SHA3(256) .update(Buffer.concat([derivedKey.slice(16, 32), ciphertext])) @@ -131,16 +126,11 @@ export default class Keystore { checkPassword = (password: string) => { const { kdfparams } = this.crypto - const derivedKey: Buffer = crypto.scryptSync( - Buffer.from(password), - Buffer.from(kdfparams.salt, 'hex'), - kdfparams.dklen, - { - N: kdfparams.n, - r: kdfparams.r, - p: kdfparams.p, - } - ) + const derivedKey: Buffer = crypto.scryptSync(password, Buffer.from(kdfparams.salt, 'hex'), kdfparams.dklen, { + N: kdfparams.n, + r: kdfparams.r, + p: kdfparams.p, + }) const ciphertext = Buffer.from(this.crypto.ciphertext, 'hex') const mac = new SHA3(256) .update(Buffer.concat([derivedKey.slice(16, 32), ciphertext])) From 1c73d37f4d846c5b115afb642b1cff293cf71d11 Mon Sep 17 00:00:00 2001 From: James Chen Date: Wed, 28 Aug 2019 14:02:31 +0900 Subject: [PATCH 02/10] test: Add a ckb cli light keystore failing test case --- .../tests/models/keys/keystore.test.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/neuron-wallet/tests/models/keys/keystore.test.ts b/packages/neuron-wallet/tests/models/keys/keystore.test.ts index a0b9fc2367..7ee9fdc171 100644 --- a/packages/neuron-wallet/tests/models/keys/keystore.test.ts +++ b/packages/neuron-wallet/tests/models/keys/keystore.test.ts @@ -19,7 +19,7 @@ describe('load and check password', () => { expect(keystore.checkPassword(password)).toBe(true) }) - it('descrypt', () => { + it('decrypts', () => { expect(keystore.decrypt(password)).toEqual( new ExtendedPrivateKey(fixture.privateKey, fixture.chainCode).serialize() ) @@ -31,3 +31,14 @@ describe('load and check password', () => { expect(extendedPrivateKey.chainCode).toEqual(fixture.chainCode) }) }) + +describe('load ckb cli light keystore', () => { + const password = '123' + const keystoreString = + '{"address":"c99d0619cc212febaf347eb265ae9517c9099ee0","crypto":{"cipher":"aes-128-ctr","cipherparams":{"iv":"aeaff7e3cecb7bb10fcc9b46a107f15e"},"ciphertext":"9d6aec2980833a6ee52bcdab4dd6f0945a838f66ef4e300e2b0bf32c56249bd8ce32007d47441ac4b8ea307c2a8a131b5f6c53690c1e51ff71b44ce73ab68d68","kdf":"scrypt","kdfparams":{"dklen":32,"n":4096,"p":6,"r":8,"salt":"9c6ab8596703e6934faaa1a48f41dbc96ee590dc7b1f3dc3c05139ef588dde6d"},"mac":"f2a3975897d4794b8b4e74ca5f1be09cd5069d90165ec3acf53bda11ac37338e"},"id":"af3de9c9-530e-4304-9db0-d4e5596cf2c6","version":3}' + const keystore = Keystore.fromJson(keystoreString) + + it('checks correct password', () => { + expect(keystore.checkPassword(password)).toBe(true) + }) +}) From 2c68e34bf9bf22065a344ad5673613623295814e Mon Sep 17 00:00:00 2001 From: James Chen Date: Wed, 28 Aug 2019 17:07:22 +0900 Subject: [PATCH 03/10] refactor: Extract keystore derived key and mac logic --- .../neuron-wallet/src/models/keys/keystore.ts | 50 +++++++------------ 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/packages/neuron-wallet/src/models/keys/keystore.ts b/packages/neuron-wallet/src/models/keys/keystore.ts index b67364cab2..3ad696f08b 100644 --- a/packages/neuron-wallet/src/models/keys/keystore.ts +++ b/packages/neuron-wallet/src/models/keys/keystore.ts @@ -1,5 +1,5 @@ import crypto from 'crypto' -import SHA3 from 'sha3' +import { Keccak } from 'sha3' import { v4 as uuid } from 'uuid' import { UnsupportedCipher, IncorrectPassword, InvalidKeystore } from 'exceptions' @@ -51,17 +51,14 @@ export default class Keystore { static create = (extendedPrivateKey: ExtendedPrivateKey, password: string) => { const salt = crypto.randomBytes(32) const iv = crypto.randomBytes(16) - const params = { - n: 8192, - r: 8, - p: 1, - } const kdfparams: KdfParams = { dklen: 32, salt: salt.toString('hex'), - ...params, + n: 8192, + r: 8, + p: 1, } - const derivedKey: Buffer = crypto.scryptSync(password, salt, kdfparams.dklen, { + const derivedKey = crypto.scryptSync(password, salt, kdfparams.dklen, { N: kdfparams.n, r: kdfparams.r, p: kdfparams.p, @@ -75,11 +72,7 @@ export default class Keystore { cipher.update(Buffer.from(extendedPrivateKey.serialize(), 'hex')), cipher.final(), ]) - const hash = new SHA3(256) - const mac = hash - .update(Buffer.concat([derivedKey.slice(16, 32), ciphertext])) - .digest() - .toString('hex') + const mac = new Keccak(256).update(Buffer.concat([derivedKey.slice(16, 32), ciphertext])).digest('hex') return new Keystore( { @@ -98,18 +91,9 @@ export default class Keystore { // Decrypt and return serialized extended private key. decrypt(password: string): string { - const { kdfparams } = this.crypto - const derivedKey: Buffer = crypto.scryptSync(password, Buffer.from(kdfparams.salt, 'hex'), kdfparams.dklen, { - N: kdfparams.n, - r: kdfparams.r, - p: kdfparams.p, - }) + const derivedKey = this.derivedKey(password) const ciphertext = Buffer.from(this.crypto.ciphertext, 'hex') - const mac = new SHA3(256) - .update(Buffer.concat([derivedKey.slice(16, 32), ciphertext])) - .digest() - .toString('hex') - if (mac !== this.crypto.mac) { + if (this.mac(derivedKey, ciphertext) !== this.crypto.mac) { throw new IncorrectPassword() } const decipher = crypto.createDecipheriv( @@ -125,17 +109,21 @@ export default class Keystore { } checkPassword = (password: string) => { + const derivedKey = this.derivedKey(password) + const ciphertext = Buffer.from(this.crypto.ciphertext, 'hex') + return this.mac(derivedKey, ciphertext) === this.crypto.mac + } + + derivedKey = (password: string) => { const { kdfparams } = this.crypto - const derivedKey: Buffer = crypto.scryptSync(password, Buffer.from(kdfparams.salt, 'hex'), kdfparams.dklen, { + return crypto.scryptSync(password, Buffer.from(kdfparams.salt, 'hex'), kdfparams.dklen, { N: kdfparams.n, r: kdfparams.r, p: kdfparams.p, }) - const ciphertext = Buffer.from(this.crypto.ciphertext, 'hex') - const mac = new SHA3(256) - .update(Buffer.concat([derivedKey.slice(16, 32), ciphertext])) - .digest() - .toString('hex') - return mac === this.crypto.mac + } + + mac = (derivedKey: Buffer, ciphertext: Buffer) => { + return new Keccak(256).update(Buffer.concat([derivedKey.slice(16, 32), ciphertext])).digest('hex') } } From dd6c721b7be8656bbb82bf07062d5b2d03da1da4 Mon Sep 17 00:00:00 2001 From: James Chen Date: Wed, 28 Aug 2019 17:36:09 +0900 Subject: [PATCH 04/10] refactor: Allow keystore creation to customize salt and iv --- packages/neuron-wallet/src/models/keys/keystore.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/neuron-wallet/src/models/keys/keystore.ts b/packages/neuron-wallet/src/models/keys/keystore.ts index 3ad696f08b..da00ecd88b 100644 --- a/packages/neuron-wallet/src/models/keys/keystore.ts +++ b/packages/neuron-wallet/src/models/keys/keystore.ts @@ -48,9 +48,13 @@ export default class Keystore { } } - static create = (extendedPrivateKey: ExtendedPrivateKey, password: string) => { - const salt = crypto.randomBytes(32) - const iv = crypto.randomBytes(16) + static create = ( + extendedPrivateKey: ExtendedPrivateKey, + password: string, + options: { salt?: Buffer; iv?: Buffer } = {} + ) => { + const salt = options.salt || crypto.randomBytes(32) + const iv = options.iv || crypto.randomBytes(16) const kdfparams: KdfParams = { dklen: 32, salt: salt.toString('hex'), From 7ea91f01cbc026f3d87f6c27a6c879490126856a Mon Sep 17 00:00:00 2001 From: James Chen Date: Wed, 28 Aug 2019 17:44:06 +0900 Subject: [PATCH 05/10] refactor: Reuse Keystore.mac --- packages/neuron-wallet/src/models/keys/keystore.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/neuron-wallet/src/models/keys/keystore.ts b/packages/neuron-wallet/src/models/keys/keystore.ts index da00ecd88b..8c459cebf7 100644 --- a/packages/neuron-wallet/src/models/keys/keystore.ts +++ b/packages/neuron-wallet/src/models/keys/keystore.ts @@ -76,7 +76,6 @@ export default class Keystore { cipher.update(Buffer.from(extendedPrivateKey.serialize(), 'hex')), cipher.final(), ]) - const mac = new Keccak(256).update(Buffer.concat([derivedKey.slice(16, 32), ciphertext])).digest('hex') return new Keystore( { @@ -87,7 +86,7 @@ export default class Keystore { cipher: CIPHER, kdf: 'scrypt', kdfparams, - mac, + mac: Keystore.mac(derivedKey, ciphertext), }, uuid() ) @@ -97,7 +96,7 @@ export default class Keystore { decrypt(password: string): string { const derivedKey = this.derivedKey(password) const ciphertext = Buffer.from(this.crypto.ciphertext, 'hex') - if (this.mac(derivedKey, ciphertext) !== this.crypto.mac) { + if (Keystore.mac(derivedKey, ciphertext) !== this.crypto.mac) { throw new IncorrectPassword() } const decipher = crypto.createDecipheriv( @@ -115,7 +114,7 @@ export default class Keystore { checkPassword = (password: string) => { const derivedKey = this.derivedKey(password) const ciphertext = Buffer.from(this.crypto.ciphertext, 'hex') - return this.mac(derivedKey, ciphertext) === this.crypto.mac + return Keystore.mac(derivedKey, ciphertext) === this.crypto.mac } derivedKey = (password: string) => { @@ -127,7 +126,7 @@ export default class Keystore { }) } - mac = (derivedKey: Buffer, ciphertext: Buffer) => { + static mac = (derivedKey: Buffer, ciphertext: Buffer) => { return new Keccak(256).update(Buffer.concat([derivedKey.slice(16, 32), ciphertext])).digest('hex') } } From ff95102c50c744df9bdd88bc600965a07cf397c3 Mon Sep 17 00:00:00 2001 From: James Chen Date: Wed, 28 Aug 2019 21:00:55 +0900 Subject: [PATCH 06/10] test: Update ckb cli light keystore json fixture --- packages/neuron-wallet/tests/models/keys/keystore.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/neuron-wallet/tests/models/keys/keystore.test.ts b/packages/neuron-wallet/tests/models/keys/keystore.test.ts index 7ee9fdc171..37c864af5b 100644 --- a/packages/neuron-wallet/tests/models/keys/keystore.test.ts +++ b/packages/neuron-wallet/tests/models/keys/keystore.test.ts @@ -35,7 +35,7 @@ describe('load and check password', () => { describe('load ckb cli light keystore', () => { const password = '123' const keystoreString = - '{"address":"c99d0619cc212febaf347eb265ae9517c9099ee0","crypto":{"cipher":"aes-128-ctr","cipherparams":{"iv":"aeaff7e3cecb7bb10fcc9b46a107f15e"},"ciphertext":"9d6aec2980833a6ee52bcdab4dd6f0945a838f66ef4e300e2b0bf32c56249bd8ce32007d47441ac4b8ea307c2a8a131b5f6c53690c1e51ff71b44ce73ab68d68","kdf":"scrypt","kdfparams":{"dklen":32,"n":4096,"p":6,"r":8,"salt":"9c6ab8596703e6934faaa1a48f41dbc96ee590dc7b1f3dc3c05139ef588dde6d"},"mac":"f2a3975897d4794b8b4e74ca5f1be09cd5069d90165ec3acf53bda11ac37338e"},"id":"af3de9c9-530e-4304-9db0-d4e5596cf2c6","version":3}' + '{"crypto":{"cipher": "aes-128-ctr", "ciphertext": "253397209cae86474e368720f9baa30f448767047d2cc5a7672ef121861974ed", "cipherparams": {"iv": "8bd8523e0048db3a4ae2534aec6d303a"}, "kdf": "scrypt", "kdfparams": {"dklen": 32, "n": 4096, "p": 6, "r": 8, "salt": "be3d86c99f4895f99d1a0048afb61a34153fa83d5edd033fc914de2c502f57e7"}, "mac": "4453cf5d4f6ec43d0664c3895c4ab9b1c9bcd2d02c7abb190c84375a42739099" },"id": "id", "version": 3}' const keystore = Keystore.fromJson(keystoreString) it('checks correct password', () => { From 34e3370029c66c19b503a433396acbe8d34127be Mon Sep 17 00:00:00 2001 From: James Chen Date: Wed, 28 Aug 2019 21:17:21 +0900 Subject: [PATCH 07/10] test: Add failing test case for cli standard keystore import to reproduce the 060B50AC:digital envelope routines:EVP_PBE_scrypt:memory limit exceeded error --- .../neuron-wallet/tests/models/keys/keystore.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/neuron-wallet/tests/models/keys/keystore.test.ts b/packages/neuron-wallet/tests/models/keys/keystore.test.ts index 37c864af5b..ca1747620e 100644 --- a/packages/neuron-wallet/tests/models/keys/keystore.test.ts +++ b/packages/neuron-wallet/tests/models/keys/keystore.test.ts @@ -42,3 +42,14 @@ describe('load ckb cli light keystore', () => { expect(keystore.checkPassword(password)).toBe(true) }) }) + +describe('load ckb cli standard keystore', () => { + const password = '123' + const keystoreString = + '{"address":"02bf67769d8e12bd956550c71e7a4e344755afd9","crypto":{"cipher":"aes-128-ctr","cipherparams":{"iv":"4e530f7bc3b59909ce97d4e7baced7ad"},"ciphertext":"e26a1e4affd919c4c00a46920a44113a526d29aa4472d0d0f84e169841853b3f350a5d76f0de6c0a0a5bdd7b03495bb904b49c11d1e241090b77792480ba255d","kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"37495d0caf3a816db294dc3f97ad8b4dd266820cccbabd7593356ff19be99e8d"},"mac":"5054f1fe0bf13cf6a1e3e4d7538eee451e263e61a935163a5b80a962849e6af3"},"id":"7f60eca5-3128-45e0-95be-08ee34c9ab37","version":3}' + const keystore = Keystore.fromJson(keystoreString) + + it('checks correct password', () => { + expect(keystore.checkPassword(password)).toBe(true) + }) +}) From eac433915d13cb5e726c7642b5d2559dbbd31cd1 Mon Sep 17 00:00:00 2001 From: James Chen Date: Wed, 28 Aug 2019 21:40:15 +0900 Subject: [PATCH 08/10] fix: Error when handling crypto.scryptSync with N > 16384 --- .../neuron-wallet/src/models/keys/keystore.ts | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/neuron-wallet/src/models/keys/keystore.ts b/packages/neuron-wallet/src/models/keys/keystore.ts index 8c459cebf7..5a07e46402 100644 --- a/packages/neuron-wallet/src/models/keys/keystore.ts +++ b/packages/neuron-wallet/src/models/keys/keystore.ts @@ -62,11 +62,7 @@ export default class Keystore { r: 8, p: 1, } - const derivedKey = crypto.scryptSync(password, salt, kdfparams.dklen, { - N: kdfparams.n, - r: kdfparams.r, - p: kdfparams.p, - }) + const derivedKey = crypto.scryptSync(password, salt, kdfparams.dklen, Keystore.scryptOptions(kdfparams)) const cipher = crypto.createCipheriv(CIPHER, derivedKey.slice(0, 16), iv) if (!cipher) { @@ -119,14 +115,24 @@ export default class Keystore { derivedKey = (password: string) => { const { kdfparams } = this.crypto - return crypto.scryptSync(password, Buffer.from(kdfparams.salt, 'hex'), kdfparams.dklen, { - N: kdfparams.n, - r: kdfparams.r, - p: kdfparams.p, - }) + return crypto.scryptSync( + password, + Buffer.from(kdfparams.salt, 'hex'), + kdfparams.dklen, + Keystore.scryptOptions(kdfparams) + ) } static mac = (derivedKey: Buffer, ciphertext: Buffer) => { return new Keccak(256).update(Buffer.concat([derivedKey.slice(16, 32), ciphertext])).digest('hex') } + + static scryptOptions = (kdfparams: KdfParams) => { + return { + N: kdfparams.n, + r: kdfparams.r, + p: kdfparams.p, + maxmem: 128 * (kdfparams.n + kdfparams.p + 2) * kdfparams.r, + } + } } From f865177cff5c260926d105e7877611b95c9c84a6 Mon Sep 17 00:00:00 2001 From: James Chen Date: Wed, 28 Aug 2019 22:06:16 +0900 Subject: [PATCH 09/10] test: Add test case for loading private key from standard keystore --- packages/neuron-wallet/tests/models/keys/keystore.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/neuron-wallet/tests/models/keys/keystore.test.ts b/packages/neuron-wallet/tests/models/keys/keystore.test.ts index ca1747620e..5611af6a1e 100644 --- a/packages/neuron-wallet/tests/models/keys/keystore.test.ts +++ b/packages/neuron-wallet/tests/models/keys/keystore.test.ts @@ -46,10 +46,16 @@ describe('load ckb cli light keystore', () => { describe('load ckb cli standard keystore', () => { const password = '123' const keystoreString = - '{"address":"02bf67769d8e12bd956550c71e7a4e344755afd9","crypto":{"cipher":"aes-128-ctr","cipherparams":{"iv":"4e530f7bc3b59909ce97d4e7baced7ad"},"ciphertext":"e26a1e4affd919c4c00a46920a44113a526d29aa4472d0d0f84e169841853b3f350a5d76f0de6c0a0a5bdd7b03495bb904b49c11d1e241090b77792480ba255d","kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"37495d0caf3a816db294dc3f97ad8b4dd266820cccbabd7593356ff19be99e8d"},"mac":"5054f1fe0bf13cf6a1e3e4d7538eee451e263e61a935163a5b80a962849e6af3"},"id":"7f60eca5-3128-45e0-95be-08ee34c9ab37","version":3}' + '{"address":"ea22142fa5be326e834681144ca30326f99a6d5a","crypto":{"cipher":"aes-128-ctr","cipherparams":{"iv":"29304e5bcbb1885ef5cdcb40b5312b58"},"ciphertext":"93054530a8fbe5b11995acda856585d7362ac7d2b1e4f268c633d997be2d6532c4962501d0835bf52a4693ae7a091ac9bac9297793f4116ef7c123edb00dbc85","kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"724327e67ca321ccf15035bb78a0a05c816bebbe218a0840abdc26da8453c1f4"},"mac":"1d0e5660ffbfc1f9ff4da97aefcfc2153c0ec1b411e35ffee26ee92815cc06f9"},"id":"43c1116e-efd5-4c9e-a86a-3ec0ab163122","version":3}' const keystore = Keystore.fromJson(keystoreString) it('checks correct password', () => { expect(keystore.checkPassword(password)).toBe(true) }) + + it('loads private key', () => { + const extendedPrivateKey = keystore.extendedPrivateKey(password) + expect(extendedPrivateKey.privateKey).toEqual('8af124598932440269a81771ad662642e83a38b323b2f70223b8ae0b6c5e0779') + expect(extendedPrivateKey.chainCode).toEqual('615302e2c93151a55c29121dd02ad554e47908a6df6d7374f357092cec11675b') + }) }) From 06f5ac69ed1103c76243b3f448c54e9546f5d797 Mon Sep 17 00:00:00 2001 From: James Chen Date: Wed, 28 Aug 2019 22:20:00 +0900 Subject: [PATCH 10/10] feat: Increase KDF params N value to 2**18 to produce more secure keystore --- packages/neuron-wallet/src/models/keys/keystore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/neuron-wallet/src/models/keys/keystore.ts b/packages/neuron-wallet/src/models/keys/keystore.ts index 5a07e46402..018ffe0288 100644 --- a/packages/neuron-wallet/src/models/keys/keystore.ts +++ b/packages/neuron-wallet/src/models/keys/keystore.ts @@ -58,7 +58,7 @@ export default class Keystore { const kdfparams: KdfParams = { dklen: 32, salt: salt.toString('hex'), - n: 8192, + n: 2 ** 18, r: 8, p: 1, }