Replace ethereumjs-wallet with an in-package V3 keystore implementation

.changeset/keystores-drop-ethereumjs-wallet.md

@@ -0,0 +1,17 @@
+---
+'@celo/keystores': patch
+---
+
+Replace the deprecated `ethereumjs-wallet` dependency with an in-package V3
+keystore implementation built on `@noble/hashes` and `@noble/ciphers`.
+
+`ethereumjs-wallet` pulled in the unmaintained browserify crypto stack
+(`ethereum-cryptography@0.1.3`, `pbkdf2`, `sha.js`, `cipher-base`, `secp256k1@4`,
+`elliptic`), which carried several critical advisories — among them `pbkdf2`
+silently returning static keys for `Uint8Array` input, and hash-rewind flaws in
+`sha.js` and `cipher-base`. Those packages are no longer in the dependency tree.
+
+The keystore format is unchanged: encryption still uses geth's defaults (scrypt
+n=262144, r=8, p=1, AES-128-CTR, keccak256 MAC), and geth-generated keystores
+still decrypt. Decryption now accepts only the `aes-128-ctr` cipher rather than
+passing the file's `cipher` field to an arbitrary cipher constructor.

packages/sdk/keystores/package.json

@@ -27,7 +27,8 @@
   "dependencies": {
     "@celo/utils": "^8.0.3",
     "@celo/wallet-local": "^8.0.1",
-    "ethereumjs-wallet": "^1.0.1",
+    "@noble/ciphers": "1.1.3",
+    "@noble/hashes": "1.3.3",
     "rimraf": "^4.4.1"
   },
   "devDependencies": {

packages/sdk/keystores/src/keystore-base.ts

@@ -3,7 +3,7 @@ import {
   normalizeAddressWith0x,
   privateKeyToAddress,
 } from '@celo/utils/lib/address'
-import Wallet from 'ethereumjs-wallet'
+import { decryptV3, encryptV3, v3Filename } from './v3-keystore'
 
 export enum ErrorMessages {
   KEYSTORE_ENTRY_EXISTS = 'Existing encrypted keystore for address',
@@ -90,12 +90,10 @@ export abstract class KeystoreBase {
       throw new Error(ErrorMessages.KEYSTORE_ENTRY_EXISTS)
     }
 
-    const key = Buffer.from(privateKey, 'hex')
-    const wallet = Wallet.fromPrivateKey(key)
-    const keystore = await wallet.toV3String(passphrase)
-    const keystoreName = wallet.getV3Filename(Date.now())
+    const keystore = await encryptV3(privateKey, passphrase)
+    const keystoreName = v3Filename(address, Date.now())
 
-    this.persistKeystore(keystoreName, keystore)
+    this.persistKeystore(keystoreName, JSON.stringify(keystore))
   }
 
   /**
@@ -120,7 +118,7 @@ export abstract class KeystoreBase {
   async getPrivateKey(address: string, passphrase: string): Promise<string> {
     const rawKeystore = this.getRawKeystore(await this.getKeystoreName(address))
     // TODO do we want to trim leading 0x here? what is the best practice here?
-    return (await Wallet.fromV3(rawKeystore, passphrase)).getPrivateKeyString()
+    return decryptV3(rawKeystore, passphrase)
   }
 
   /**
@@ -132,10 +130,9 @@ export abstract class KeystoreBase {
   async changeKeystorePassphrase(address: string, oldPassphrase: string, newPassphrase: string) {
     const keystoreName = await this.getKeystoreName(address)
     const rawKeystore = this.getRawKeystore(keystoreName)
-    const newKeystore = await (await Wallet.fromV3(rawKeystore, oldPassphrase)).toV3String(
-      newPassphrase
-    )
-    this.persistKeystore(keystoreName, newKeystore)
+    const privateKey = await decryptV3(rawKeystore, oldPassphrase)
+    const newKeystore = await encryptV3(privateKey, newPassphrase)
+    this.persistKeystore(keystoreName, JSON.stringify(newKeystore))
   }
 
   /**

packages/sdk/keystores/src/test-constants.ts

@@ -11,3 +11,9 @@ export const PK2 = 'bb6f3fa4a83b7b06e72e580a3b09df5dd6fb4fa745ee2b0d865413ad6299
 export const KEYSTORE_NAME2 = 'PK2 keystore name'
 export const GETH_GEN_KEYSTORE2 = `{"address":"b81a82696018fd9d8b43431966b60c31bdcdc2e8","blspublickey":"b9f862e2ced58bb2eef8ffde7020189ab2bb050603630eceec9b80c1636d98f8c3b9bd517d673937a0551c3a0698a00086bda4db1f0d859912a91988775ae388886013e7eb254d195871f9ced6643e288755da0b483ebe6dda448fea2eb75481","crypto":{"cipher":"aes-128-ctr","ciphertext":"6f3cd02b2d3d81b2bbf76743396c9c3c1685ddc6cfafbba34195ab03476831d3","cipherparams":{"iv":"af1f9853e0ff20ee5d495cf7d9461e1c"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"cf0446914e5d214f2a312c08ef24e7e3dd15e948d2ca67d59b3bb97903a96147"},"mac":"58348f6d843d28b3ac8cf40542d10da198016f28f132c32389ab56a945c858e1"},"id":"b224dac6-c089-4b47-8557-e04ae60b3506","version":3}`
 export const ADDRESS2 = normalizeAddressWith0x(privateKeyToAddress(PK2))
+
+// The pbkdf2 test vector from the Web3 Secret Storage Definition. Keystores in
+// the wild almost always use scrypt, so this pins the pbkdf2 derivation path.
+export const PBKDF2_PASSPHRASE = 'testpassword'
+export const PBKDF2_PK = '7a28b5ba57c53603b0b07b56bba752f7784bf506fa95edc395f5cf6c7514fe9d'
+export const PBKDF2_KEYSTORE = `{"crypto":{"cipher":"aes-128-ctr","cipherparams":{"iv":"6087dab2f9fdbbfaddc31a909735c1e6"},"ciphertext":"5318b4d5bcd28de64ee5559e671353e16f075ecae9f99c7a79a38af5f869aa46","kdf":"pbkdf2","kdfparams":{"c":262144,"dklen":32,"prf":"hmac-sha256","salt":"ae3cd4e7013836a3df6bd7241b12db061dbe2c6785853cce422d148a624ce0bd"},"mac":"517ead924a9d0dc3124507e3393d175ce3ff7c1e96529c6c555ce9e51205e9b2"},"id":"3198bc9c-6672-5ab3-d995-4942343ae5b6","version":3}`

packages/sdk/keystores/src/v3-keystore.test.ts

@@ -0,0 +1,130 @@
+import { trimLeading0x } from '@celo/utils/lib/address'
+import {
+  ADDRESS1,
+  GETH_GEN_KEYSTORE1,
+  GETH_GEN_KEYSTORE2,
+  PASSPHRASE1,
+  PASSPHRASE2,
+  PBKDF2_KEYSTORE,
+  PBKDF2_PASSPHRASE,
+  PBKDF2_PK,
+  PK1,
+  PK2,
+} from './test-constants'
+import { decryptV3, encryptV3, V3ErrorMessages, v3Filename } from './v3-keystore'
+
+jest.setTimeout(30000)
+
+describe('decryptV3', () => {
+  it('decrypts a geth-generated keystore', async () => {
+    expect(trimLeading0x(await decryptV3(GETH_GEN_KEYSTORE1, PASSPHRASE1))).toBe(PK1)
+    expect(trimLeading0x(await decryptV3(GETH_GEN_KEYSTORE2, PASSPHRASE2))).toBe(PK2)
+  })
+
+  it('returns the private key 0x-prefixed', async () => {
+    expect(await decryptV3(GETH_GEN_KEYSTORE1, PASSPHRASE1)).toBe(`0x${PK1}`)
+  })
+
+  it('decrypts a pbkdf2 keystore', async () => {
+    expect(trimLeading0x(await decryptV3(PBKDF2_KEYSTORE, PBKDF2_PASSPHRASE))).toBe(PBKDF2_PK)
+  })
+
+  it('rejects a wrong passphrase for a pbkdf2 keystore', async () => {
+    await expect(decryptV3(PBKDF2_KEYSTORE, `${PBKDF2_PASSPHRASE}!`)).rejects.toThrow(
+      V3ErrorMessages.WRONG_PASSPHRASE
+    )
+  })
+
+  it('rejects a wrong passphrase', async () => {
+    await expect(decryptV3(GETH_GEN_KEYSTORE1, `${PASSPHRASE1}!`)).rejects.toThrow(
+      V3ErrorMessages.WRONG_PASSPHRASE
+    )
+  })
+
+  it('rejects a non-V3 keystore', async () => {
+    const v1 = JSON.stringify({ ...JSON.parse(GETH_GEN_KEYSTORE1), version: 1 })
+    await expect(decryptV3(v1, PASSPHRASE1)).rejects.toThrow(V3ErrorMessages.NOT_V3)
+  })
+
+  it('rejects an unsupported cipher', async () => {
+    const parsed = JSON.parse(GETH_GEN_KEYSTORE1)
+    parsed.crypto.cipher = 'aes-128-cbc'
+    await expect(decryptV3(JSON.stringify(parsed), PASSPHRASE1)).rejects.toThrow(
+      V3ErrorMessages.UNSUPPORTED_CIPHER
+    )
+  })
+
+  it('rejects an unsupported key derivation scheme', async () => {
+    const parsed = JSON.parse(GETH_GEN_KEYSTORE1)
+    parsed.crypto.kdf = 'bcrypt'
+    await expect(decryptV3(JSON.stringify(parsed), PASSPHRASE1)).rejects.toThrow(
+      V3ErrorMessages.UNSUPPORTED_KDF
+    )
+  })
+
+  it('rejects pbkdf2 with a pseudorandom function other than hmac-sha256', async () => {
+    const parsed = JSON.parse(GETH_GEN_KEYSTORE1)
+    parsed.crypto.kdf = 'pbkdf2'
+    parsed.crypto.kdfparams = { dklen: 32, salt: '00'.repeat(32), c: 1, prf: 'hmac-sha512' }
+    await expect(decryptV3(JSON.stringify(parsed), PASSPHRASE1)).rejects.toThrow(
+      V3ErrorMessages.UNSUPPORTED_PBKDF2_PARAMS
+    )
+  })
+
+  it('detects a tampered ciphertext via the mac', async () => {
+    const parsed = JSON.parse(GETH_GEN_KEYSTORE1)
+    parsed.crypto.ciphertext = `${parsed.crypto.ciphertext.slice(0, -2)}00`
+    await expect(decryptV3(JSON.stringify(parsed), PASSPHRASE1)).rejects.toThrow(
+      V3ErrorMessages.WRONG_PASSPHRASE
+    )
+  })
+})
+
+describe('encryptV3', () => {
+  it('round-trips a private key', async () => {
+    const keystore = await encryptV3(PK1, PASSPHRASE1)
+    expect(trimLeading0x(await decryptV3(JSON.stringify(keystore), PASSPHRASE1))).toBe(PK1)
+  })
+
+  it('round-trips a 0x-prefixed private key', async () => {
+    const keystore = await encryptV3(`0x${PK1}`, PASSPHRASE1)
+    expect(trimLeading0x(await decryptV3(JSON.stringify(keystore), PASSPHRASE1))).toBe(PK1)
+  })
+
+  it('writes geth-compatible parameters', async () => {
+    const { version, crypto } = await encryptV3(PK1, PASSPHRASE1)
+    expect(version).toBe(3)
+    expect(crypto.cipher).toBe('aes-128-ctr')
+    expect(crypto.kdf).toBe('scrypt')
+    expect(crypto.kdfparams).toMatchObject({ dklen: 32, n: 262144, r: 8, p: 1 })
+    expect(crypto.cipherparams.iv).toHaveLength(32)
+    expect(crypto.kdfparams.salt).toHaveLength(64)
+    expect(crypto.mac).toHaveLength(64)
+  })
+
+  it('stores the address unprefixed and lowercased', async () => {
+    const { address } = await encryptV3(PK1, PASSPHRASE1)
+    expect(address).toBe(trimLeading0x(ADDRESS1).toLowerCase())
+  })
+
+  it('uses a fresh salt and iv for each keystore', async () => {
+    const [a, b] = await Promise.all([encryptV3(PK1, PASSPHRASE1), encryptV3(PK1, PASSPHRASE1)])
+    expect(a.crypto.kdfparams.salt).not.toBe(b.crypto.kdfparams.salt)
+    expect(a.crypto.cipherparams.iv).not.toBe(b.crypto.cipherparams.iv)
+    expect(a.crypto.ciphertext).not.toBe(b.crypto.ciphertext)
+    expect(a.id).not.toBe(b.id)
+  })
+})
+
+describe('v3Filename', () => {
+  it('builds the geth filename convention', () => {
+    expect(v3Filename(ADDRESS1, Date.parse('2016-03-15T17:11:33.007Z'))).toBe(
+      `UTC--2016-03-15T17-11-33.007Z--${trimLeading0x(ADDRESS1).toLowerCase()}`
+    )
+  })
+
+  it('accepts an address with or without a 0x prefix', () => {
+    const timestamp = Date.parse('2016-03-15T17:11:33.007Z')
+    expect(v3Filename(ADDRESS1, timestamp)).toBe(v3Filename(trimLeading0x(ADDRESS1), timestamp))
+  })
+})

packages/sdk/keystores/src/v3-keystore.ts

@@ -0,0 +1,176 @@
+/**
+ * Encryption and decryption of Web3 Secret Storage V3 keystores.
+ * See https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition
+ *
+ * Parameter defaults and error messages match those of geth-generated keystores.
+ */
+
+import { randomUUID } from 'node:crypto'
+import {
+  ensureLeading0x,
+  hexToBuffer,
+  privateKeyToAddress,
+  trimLeading0x,
+} from '@celo/utils/lib/address'
+import { ctr } from '@noble/ciphers/aes'
+import { pbkdf2Async } from '@noble/hashes/pbkdf2'
+import { scryptAsync } from '@noble/hashes/scrypt'
+import { keccak_256 } from '@noble/hashes/sha3'
+import { sha256 } from '@noble/hashes/sha256'
+import { randomBytes } from '@noble/hashes/utils'
+
+/** The only cipher used by geth and by the wider V3 ecosystem. */
+const CIPHER = 'aes-128-ctr'
+const SCRYPT_N = 262144
+const SCRYPT_R = 8
+const SCRYPT_P = 1
+const DERIVED_KEY_LENGTH = 32
+const SALT_LENGTH = 32
+const IV_LENGTH = 16
+
+export enum V3ErrorMessages {
+  NOT_V3 = 'Not a V3 wallet',
+  UNSUPPORTED_CIPHER = 'Unsupported cipher',
+  UNSUPPORTED_KDF = 'Unsupported key derivation scheme',
+  UNSUPPORTED_PBKDF2_PARAMS = 'Unsupported parameters to PBKDF2',
+  WRONG_PASSPHRASE = 'Key derivation failed - possibly wrong passphrase',
+}
+
+interface ScryptParams {
+  dklen: number
+  salt: string
+  n: number
+  r: number
+  p: number
+}
+
+interface Pbkdf2Params {
+  dklen: number
+  salt: string
+  c: number
+  prf: string
+}
+
+export interface V3Keystore {
+  version: number
+  id: string
+  address: string
+  crypto: {
+    ciphertext: string
+    cipherparams: { iv: string }
+    cipher: string
+    kdf: string
+    kdfparams: ScryptParams | Pbkdf2Params
+    mac: string
+  }
+}
+
+/**
+ * Derives the symmetric key that protects a keystore's ciphertext.
+ * The first half encrypts the private key, the second half authenticates it.
+ */
+async function deriveKey(
+  kdf: string,
+  kdfparams: ScryptParams | Pbkdf2Params,
+  passphrase: string
+): Promise<Uint8Array> {
+  const password = Buffer.from(passphrase, 'utf8')
+  const salt = hexToBuffer(kdfparams.salt)
+
+  if (kdf === 'scrypt') {
+    const { n, r, p } = kdfparams as ScryptParams
+    return scryptAsync(password, salt, { N: n, r, p, dkLen: kdfparams.dklen })
+  }
+  if (kdf === 'pbkdf2') {
+    const { c, prf } = kdfparams as Pbkdf2Params
+    if (prf !== 'hmac-sha256') {
+      throw new Error(V3ErrorMessages.UNSUPPORTED_PBKDF2_PARAMS)
+    }
+    return pbkdf2Async(sha256, password, salt, { c, dkLen: kdfparams.dklen })
+  }
+  throw new Error(V3ErrorMessages.UNSUPPORTED_KDF)
+}
+
+/**
+ * The V3 message authentication code, over the second half of the derived key
+ * and the ciphertext.
+ */
+function macOf(derivedKey: Uint8Array, ciphertext: Uint8Array): string {
+  return Buffer.from(
+    keccak_256(Buffer.concat([Buffer.from(derivedKey.subarray(16, 32)), Buffer.from(ciphertext)]))
+  ).toString('hex')
+}
+
+/**
+ * Decrypts a V3 keystore
+ * @param keystore Serialized V3 keystore
+ * @param passphrase Secret phrase the private key was encrypted with
+ * @returns Private key as a 0x-prefixed hex string
+ */
+export async function decryptV3(keystore: string, passphrase: string): Promise<string> {
+  const json: V3Keystore = JSON.parse(keystore)
+  if (json.version !== 3) {
+    throw new Error(V3ErrorMessages.NOT_V3)
+  }
+  if (json.crypto.cipher !== CIPHER) {
+    throw new Error(`${V3ErrorMessages.UNSUPPORTED_CIPHER}: ${json.crypto.cipher}`)
+  }
+
+  const derivedKey = await deriveKey(json.crypto.kdf, json.crypto.kdfparams, passphrase)
+  const ciphertext = hexToBuffer(json.crypto.ciphertext)
+  if (macOf(derivedKey, ciphertext) !== json.crypto.mac) {
+    throw new Error(V3ErrorMessages.WRONG_PASSPHRASE)
+  }
+
+  const privateKey = ctr(
+    derivedKey.subarray(0, 16),
+    hexToBuffer(json.crypto.cipherparams.iv)
+  ).decrypt(ciphertext)
+  return ensureLeading0x(Buffer.from(privateKey).toString('hex'))
+}
+
+/**
+ * Encrypts a private key into a V3 keystore, using geth's default parameters
+ * @param privateKey Private key to encrypt, with or without a 0x prefix
+ * @param passphrase Secret phrase to encrypt the private key with
+ */
+export async function encryptV3(privateKey: string, passphrase: string): Promise<V3Keystore> {
+  const salt = randomBytes(SALT_LENGTH)
+  const iv = randomBytes(IV_LENGTH)
+  const kdfparams: ScryptParams = {
+    dklen: DERIVED_KEY_LENGTH,
+    salt: Buffer.from(salt).toString('hex'),
+    n: SCRYPT_N,
+    r: SCRYPT_R,
+    p: SCRYPT_P,
+  }
+
+  const derivedKey = await deriveKey('scrypt', kdfparams, passphrase)
+  const ciphertext = ctr(derivedKey.subarray(0, 16), iv).encrypt(hexToBuffer(privateKey))
+
+  return {
+    version: 3,
+    id: randomUUID(),
+    // The V3 spec omits the address, but geth and its ecosystem always write it.
+    address: trimLeading0x(privateKeyToAddress(privateKey)).toLowerCase(),
+    crypto: {
+      ciphertext: Buffer.from(ciphertext).toString('hex'),
+      cipherparams: { iv: Buffer.from(iv).toString('hex') },
+      cipher: CIPHER,
+      kdf: 'scrypt',
+      kdfparams,
+      mac: macOf(derivedKey, ciphertext),
+    },
+  }
+}
+
+/**
+ * Builds the conventional geth filename for a V3 keystore entry,
+ * e.g. UTC--2016-03-15T17-11-33.007Z--<address>
+ * @param address Account address of the keystore entry
+ * @param timestamp Creation time in milliseconds since the epoch
+ */
+export function v3Filename(address: string, timestamp: number): string {
+  const isoTime = new Date(timestamp).toJSON().replace(/:/g, '-')
+  return `UTC--${isoTime}--${trimLeading0x(address).toLowerCase()}`
+}