From 317d713db7ff030438b52ae087f0c2c131d0b215 Mon Sep 17 00:00:00 2001 From: underthesun49 Date: Thu, 9 Jul 2026 09:04:20 +1000 Subject: [PATCH] feat(crypto): upgrade keystore encryption to AES-256-CTR (v2) encryptToKeyStore now derives a 64-byte key (dklen 32 -> 64) and splits it into an independent 32-byte AES-256 key and 32-byte MAC key, writing cipher "aes-256-ctr" and version 2. Previously only the first 16 of 32 derived bytes were used, with aes-128-ctr. decryptFromKeystore derives the AES-key/MAC-key split from the keystore's own cipher and dklen, so existing v1 (aes-128-ctr) keystores still decrypt unchanged. Files newly written are v2 and cannot be read by older xchain-crypto releases (forward-incompatible write; documented in changeset). Tests: v2 known-answer (ciphertext + mac) test, v2 decrypt, and preserved v1 backward-compat decrypt fixtures. All 14 tests pass; build, typecheck, and lint clean. Closes #1720 Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/crypto-aes256-keystore.md | 13 +++++ .../xchain-crypto/__tests__/crypto.test.ts | 52 +++++++++++++++---- packages/xchain-crypto/src/crypto.ts | 48 ++++++++++++++--- 3 files changed, 96 insertions(+), 17 deletions(-) create mode 100644 .changeset/crypto-aes256-keystore.md diff --git a/.changeset/crypto-aes256-keystore.md b/.changeset/crypto-aes256-keystore.md new file mode 100644 index 000000000..bbf924e82 --- /dev/null +++ b/.changeset/crypto-aes256-keystore.md @@ -0,0 +1,13 @@ +--- +'@xchainjs/xchain-crypto': minor +--- + +Upgrade keystore encryption to AES-256-CTR (keystore format v2). + +`encryptToKeyStore` now derives a 64-byte key via PBKDF2 (`dklen` 32 → 64) and splits it into an independent 32-byte AES-256 key and 32-byte MAC key, writing `cipher: "aes-256-ctr"` and `version: 2`. Previously it used `aes-128-ctr` with only the first 16 of 32 derived bytes as the AES key (the remaining 16 were the MAC key). + +**Backward compatible (reading):** `decryptFromKeystore` derives the AES-key/MAC-key split from the keystore's own `cipher` and `dklen` fields, so existing v1 (`aes-128-ctr`, `dklen` 32) keystores continue to decrypt unchanged. No user action or migration is required to keep opening existing wallets. + +**Forward-incompatible (writing) — note for integrators:** keystore files newly created by this version are `aes-256-ctr` / v2 and **cannot be opened by older `@xchainjs/xchain-crypto` releases** (older code hard-codes a 16-byte key slice and will throw on the 32-byte AES-256 key). Existing files are untouched and remain openable everywhere. If your app shares keystore files across components pinned to different `xchain-crypto` versions, upgrade them together before creating new keystores. + +Security note: AES-128 had no known practical break, so this is a modernization to the expected standard for seed-phrase storage rather than a fix for an exploitable weakness. The AES-256-CTR path is also supported by `crypto-browserify`, so browser/Electron bundles are unaffected. diff --git a/packages/xchain-crypto/__tests__/crypto.test.ts b/packages/xchain-crypto/__tests__/crypto.test.ts index 8a3c8c83b..c473552bb 100755 --- a/packages/xchain-crypto/__tests__/crypto.test.ts +++ b/packages/xchain-crypto/__tests__/crypto.test.ts @@ -18,6 +18,30 @@ describe('Keystore regression test for encrypt/decrypt with internal migration', const phrase = 'patient use either flash couple jump castle true broccoli cancel brand mechanic' const password = '1234' + // v2 keystore (aes-256-ctr, dklen 64) — the format written by the current encryptToKeyStore. + // Known-answer test: produced from the fixed salt/iv/password/phrase above. + const expectedKeystoreV2 = { + crypto: { + cipher: 'aes-256-ctr', + ciphertext: + '31607fd3f65a69ded1701f2f45f1bc7eba8d8ac3eb3a9c50e95ac9d5915631e34c59ba05047bfe4f1ae5639081ea705eef5e65fea253b39faa5e2c0e8b606c60606cb1e5c4081518ca4dcb472fe7d9', + cipherparams: { iv: 'dffdb8bbe92e9a00e173eaa20f1a3784' }, + kdf: 'pbkdf2', + kdfparams: { + prf: 'hmac-sha256', + dklen: 64, + salt: 'ead4ad6c09f5a5586235a642fa39c95741b35283304e3fd464d942e300fe0514', + c: 600000, + }, + mac: '7c437ee1126fe587317a32dccef99970d16384c21db28ed6d8a7dedf8a77eaf0', + }, + id: '9ad9ea91-22ad-46a7-9613-4f9d190e32ab', + version: 2, + meta: 'xchain-keystore', + } + + // v1 keystore (aes-128-ctr, dklen 32) — the format written before the AES-256 upgrade. + // Must remain decryptable so existing wallets keep working (backward-compatible read). const expectedKeystore = { crypto: { cipher: 'aes-128-ctr', @@ -60,21 +84,28 @@ describe('Keystore regression test for encrypt/decrypt with internal migration', meta: 'xchain-keystore', } - it('encryptToKeyStore() should produce expected ciphertext and mac', async () => { + it('encryptToKeyStore() should produce the expected v2 (aes-256-ctr) ciphertext and mac', async () => { jest .spyOn(crypto, 'randomBytes') - .mockImplementationOnce(() => Buffer.from(expectedKeystore.crypto.kdfparams.salt, 'hex')) // salt - .mockImplementationOnce(() => Buffer.from(expectedKeystore.crypto.cipherparams.iv, 'hex')) // iv + .mockImplementationOnce(() => Buffer.from(expectedKeystoreV2.crypto.kdfparams.salt, 'hex')) // salt + .mockImplementationOnce(() => Buffer.from(expectedKeystoreV2.crypto.cipherparams.iv, 'hex')) // iv const keystore = await encryptToKeyStore(phrase, password) - expect(keystore.crypto.ciphertext).toBe(expectedKeystore.crypto.ciphertext) - expect(keystore.crypto.mac).toBe(expectedKeystore.crypto.mac) - expect(keystore.crypto.kdfparams).toEqual(expectedKeystore.crypto.kdfparams) - expect(keystore.crypto.cipherparams).toEqual(expectedKeystore.crypto.cipherparams) + expect(keystore.crypto.cipher).toBe('aes-256-ctr') + expect(keystore.version).toBe(2) + expect(keystore.crypto.ciphertext).toBe(expectedKeystoreV2.crypto.ciphertext) + expect(keystore.crypto.mac).toBe(expectedKeystoreV2.crypto.mac) + expect(keystore.crypto.kdfparams).toEqual(expectedKeystoreV2.crypto.kdfparams) + expect(keystore.crypto.cipherparams).toEqual(expectedKeystoreV2.crypto.cipherparams) + }) + + it('decryptFromKeystore() should decrypt a v2 (aes-256-ctr) keystore', async () => { + const result = await decryptFromKeystore(expectedKeystoreV2, password) + expect(result).toBe(phrase) }) - it('decryptFromKeystore() should return original phrase', async () => { + it('decryptFromKeystore() should still decrypt a legacy v1 (aes-128-ctr) keystore', async () => { const result = await decryptFromKeystore(expectedKeystore, password) expect(result).toBe(phrase) }) @@ -136,11 +167,12 @@ describe('Export Keystore', () => { const phrase = 'flush viable fury sword mention dignity ethics secret nasty gallery teach fever' const password = 'thorchain' const keystore = await encryptToKeyStore(phrase, password) - expect(keystore.crypto.cipher).toEqual('aes-128-ctr') + expect(keystore.crypto.cipher).toEqual('aes-256-ctr') expect(keystore.crypto.kdf).toEqual('pbkdf2') expect(keystore.crypto.kdfparams.prf).toEqual('hmac-sha256') + expect(keystore.crypto.kdfparams.dklen).toEqual(64) expect(keystore.crypto.kdfparams.c).toEqual(600000) - expect(keystore.version).toEqual(1) + expect(keystore.version).toEqual(2) expect(keystore.meta).toEqual('xchain-keystore') }) }) diff --git a/packages/xchain-crypto/src/crypto.ts b/packages/xchain-crypto/src/crypto.ts index 6107fb7e1..6ec55ffb2 100755 --- a/packages/xchain-crypto/src/crypto.ts +++ b/packages/xchain-crypto/src/crypto.ts @@ -8,13 +8,37 @@ import { v4 as uuidv4 } from 'uuid' import { pbkdf2Async } from './utils' // Constants -const cipher = 'aes-128-ctr' // Encryption cipher +const cipher = 'aes-256-ctr' // Encryption cipher (keystore v2). Legacy v1 keystores use aes-128-ctr. const kdf = 'pbkdf2' // Key derivation function const prf = 'hmac-sha256' // Pseudorandom function -const dklen = 32 // Derived key length +const dklen = 64 // Derived key length: 32-byte AES-256 key + 32-byte independent MAC key const c = 600000 // Iteration count (OWASP-recommended minimum for PBKDF2-HMAC-SHA256) const hashFunction = 'sha256' // Hash function const meta = 'xchain-keystore' // Metadata +const keystoreVersion = 2 // Keystore format version written by encryptToKeyStore + +/** + * Returns the AES key length in bytes for a supported keystore cipher. + * + * The keystore is self-describing: its `cipher` field determines how many of the + * PBKDF2-derived bytes are the AES key; the remaining `dklen - keyLength` bytes are + * the independent MAC key. This lets legacy v1 keystores (aes-128-ctr, 16-byte key, + * dklen 32) stay decryptable alongside v2 (aes-256-ctr, 32-byte key, dklen 64). + * + * @param {string} cipherName The cipher name read from the keystore. + * @returns {number} AES key length in bytes. + * @throws {Error} Thrown if the cipher is not supported. + */ +const aesKeyLengthForCipher = (cipherName: string): number => { + switch (cipherName) { + case 'aes-256-ctr': + return 32 + case 'aes-128-ctr': + return 16 + default: + throw new Error(`Unsupported keystore cipher: ${cipherName}`) + } +} /** * The Keystore interface. @@ -136,9 +160,14 @@ export const encryptToKeyStore = async (phrase: string, password: string): Promi } const derivedKey = await pbkdf2Async(Buffer.from(password), salt, kdfParams.c, kdfParams.dklen, hashFunction) - const cipherIV = crypto.createCipheriv(cipher, derivedKey.slice(0, 16), iv) + // Split the derived key into an AES key and an independent MAC key — the two key + // materials must not overlap, so the MAC key is the bytes after the AES key. + const aesKeyLength = aesKeyLengthForCipher(cipher) + const encryptionKey = derivedKey.slice(0, aesKeyLength) + const macKey = derivedKey.slice(aesKeyLength, kdfParams.dklen) + const cipherIV = crypto.createCipheriv(cipher, encryptionKey, iv) const cipherText = Buffer.concat([cipherIV.update(Buffer.from(phrase, 'utf8')), cipherIV.final()]) - const mac_bytes: Uint8Array = blake2b(Buffer.concat([derivedKey.slice(16, 32), Buffer.from(cipherText)]), { + const mac_bytes: Uint8Array = blake2b(Buffer.concat([macKey, Buffer.from(cipherText)]), { dkLen: 32, }) const mac: string = Buffer.from(mac_bytes).toString('hex') @@ -155,7 +184,7 @@ export const encryptToKeyStore = async (phrase: string, password: string): Promi const keystore = { crypto: cryptoStruct, id: ID, - version: 1, + version: keystoreVersion, meta: meta, } @@ -180,7 +209,12 @@ export const decryptFromKeystore = async (keystore: Keystore, password: string): ) const ciphertext = Buffer.from(keystore.crypto.ciphertext, 'hex') - const mac_bytes: Uint8Array = blake2b(Buffer.concat([derivedKey.slice(16, 32), ciphertext]), { dkLen: 32 }) + // Derive the AES key / MAC key split from the keystore's own cipher and dklen, so + // legacy v1 (aes-128-ctr, 16-byte key) and v2 (aes-256-ctr, 32-byte key) both decrypt. + const aesKeyLength = aesKeyLengthForCipher(keystore.crypto.cipher) + const encryptionKey = derivedKey.slice(0, aesKeyLength) + const macKey = derivedKey.slice(aesKeyLength, kdfparams.dklen) + const mac_bytes: Uint8Array = blake2b(Buffer.concat([macKey, ciphertext]), { dkLen: 32 }) const computedMac = Buffer.from(mac_bytes) const expectedMac = Buffer.from(keystore.crypto.mac, 'hex') @@ -189,7 +223,7 @@ export const decryptFromKeystore = async (keystore: Keystore, password: string): throw new Error('Invalid password') const decipher = crypto.createDecipheriv( keystore.crypto.cipher, - derivedKey.slice(0, 16), + encryptionKey, Buffer.from(keystore.crypto.cipherparams.iv, 'hex'), )