fix(crypto): include CTR IV in keystore MAC (v2)

[Thorian1te]

Summary

Fixes #1721.

Keystore MAC previously covered only macKey || ciphertext, not the CTR IV. An attacker who could modify cipherparams.iv could change the decrypted plaintext without invalidating the MAC (CTR malleability). Low practical impact for BIP39 (checksum usually fails), but the MAC should authenticate everything an attacker can edit.

Change

	v1 (legacy, still readable)	v2 (new writes)
MAC input	macKey || ciphertext	macKey || iv || ciphertext
version	1	2
Cipher	unchanged (aes-128-ctr)	unchanged

• Encrypt: always writes version: 2 with IV bound into the MAC.
• Decrypt: version >= 2 → verify with IV; version < 2 → legacy MAC (existing wallets keep working).
• Tests: known-answer v2 vector; v1 + old PBKDF2 iteration fixtures still decrypt; IV tamper fails on v2; documents that v1 still accepts IV tamper (legacy gap).

Not in this PR

• AES-256 / independent 32-byte keys — tracked by xchain-crypto: keystore uses aes-128-ctr (only 16 of 32 derived bytes); consider aes-256-ctr #1720 / open feat(crypto): upgrade keystore encryption to AES-256-CTR (v2) #1723. When that lands, please keep IV-in-MAC for new versions (rebase onto this or fold the same MAC rule).

Compatibility

• Reading old keystores: yes (v1 MAC path retained).
• Older library reading new keystores: no — v2 MAC will not match old verifiers. Integrators that share keystore files across mixed xchain-crypto versions should upgrade together before creating new keystores.

Test plan

• yarn workspace @xchainjs/xchain-crypto test
• Reviewer: confirm version gate (>= 2) is the right switch vs cipher-based
• Note interaction with feat(crypto): upgrade keystore encryption to AES-256-CTR (v2) #1723 (AES-256 v2) for sequencing

Summary by CodeRabbit

• Security Enhancements
  • New keystores use an updated format that authenticates the initialization vector, helping detect IV tampering.
  • Existing version 1 keystores remain readable.
  • Incorrect passwords and modified keystore data continue to be rejected safely.
• Compatibility
  • Version 2 keystores require a release that supports the updated format.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 98950631-2090-44bb-a18b-2aecb30f2977

📥 Commits

Reviewing files that changed from the base of the PR and between fe91856 and b1dbb00.

📒 Files selected for processing (3)

• .changeset/crypto-keystore-mac-iv.md
• packages/xchain-crypto/__tests__/crypto.test.ts
• packages/xchain-crypto/src/crypto.ts

---

📝 Walkthrough

Walkthrough

The keystore format now uses version 2 for newly written files. Version 2 MACs include the IV. Decryption remains compatible with version 1 keystores. Tests cover encryption, compatibility, tampering, and browser-polyfill behavior.

Changes

Keystore MAC authentication

Layer / File(s)	Summary
Version-aware MAC calculation packages/xchain-crypto/src/crypto.ts	Adds version constants and selects MAC input based on the keystore version.
Version 2 encryption and compatibility decryption packages/xchain-crypto/src/crypto.ts	New keystores use version 2 and authenticate the IV. Decryption supports both version 1 and version 2 MAC formats.
Compatibility and tampering coverage packages/xchain-crypto/__tests__/crypto.test.ts, .changeset/crypto-keystore-mac-iv.md	Tests cover version 2 output, version 1 compatibility, IV tampering, incorrect passwords, and browser-polyfill behavior. The changeset documents the format update.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• xchainjs/xchainjs-lib#1494: Earlier keystore MAC changes extended by this PR with IV authentication and versioned compatibility.
• xchainjs/xchainjs-lib#1716: Related changes to keystore MAC verification and its tests.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: authenticating the CTR IV in the keystore MAC for version 2.
Linked Issues check	✅ Passed	The implementation authenticates the IV for version 2, preserves legacy verification for version 1, and adds coverage for compatibility and tampering.
Out of Scope Changes check	✅ Passed	The code, tests, and changeset directly support the linked issue and stated objectives without unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/crypto-keystore-mac-includes-iv

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Thorian1te]

Practical risk scale for #1721 (for reviewers)

Assuming the question is “how likely is an attacker to actually use CTR IV not being in the MAC?” — put it on a scale:

Overall practical risk: ~1–2 / 10 (low)

Dimension	Score	Why
Crypto correctness	7/10 “should fix”	Real AE gap: MAC doesn’t bind IV; textbook CTR malleability
Exploitability in the wild	1–2/10	Needs write access to the keystore file and a useful plaintext goal
Impact if fully exploited	3–4/10 for BIP39 wallets	Garbled mnemonic almost always fails BIP39 wordlist/checksum → decrypt “succeeds” as garbage or fails later
Impact if non-BIP39 / raw secrets	6–7/10	If you ever stored arbitrary secrets this way, bit flips matter more
Priority vs other crypto work	Below wrong password handling, seed theft, XSS, malicious extension	Those actually steal funds

The issue text already says this: low severity in practice for BIP39.

What an attacker would need

1. Modify the stored keystore (disk, backup, synced config, compromised device, malicious backup provider).
2. Change cipherparams.iv (and leave ciphertext + MAC as-is).
3. Victim opens wallet with the correct password.
4. MAC still passes (v1), CTR decrypts with wrong keystream → wrong bytes, not the real seed “extracted cleanly.”

They do not get the password or a clean copy of the mnemonic from this alone. They get integrity failure that looks like “valid password, bad data” (or a non-mnemonic string).

Likelihood of real-world abuse

Scenario	Chance someone “uses” this	Notes
Random internet attacker, remote	~0	No keystore write path
Malware already on the machine	Low value	If they can edit keystore, they can often steal it + wait for password, or replace whole file, or keylog — better attacks
Evil backup / cloud sync	Low	Same: full file replace or ransomware is simpler than surgical IV flips
Targeted crypto nerd demo	Yes, in a lab	Easy to show MAC passes + plaintext changes
Turning flipped bits into a chosen valid BIP39	Very hard	BIP39 checksum + wordlist; not free bit-control to a funded wallet

So: great for a security write-up / “fix the construction”; poor as a primary theft primitive.

Scale analogy

0 ──────── 1–2 ──── 3–4 ──────── 6–7 ──────── 9–10
|           |        |            |             |
|        #1721    serious      seed in       active
|        practical  bug if      plaintext    RCE +
|        BIP39      non-BIP39   / bad KDF     wallet drain
|        (here)

• Chance an attacker uses this for real theft: about 1–2/10.
• Should we still fix it? Yes (~8/10 for hygiene) — cheap, versioned, correct crypto; that’s why this PR exists.

Compared to things that are higher risk

Issue class	Relative risk
Malicious extension / clipboard / phishing	Much higher
Keystore file + password phishing	Higher
Weak password + stolen keystore	Higher
#1721 IV not in MAC	Low practical
AES-128 vs 256 (#1720)	Also mostly hygiene at this threat model

Bottom line: Treat #1721 as real but niche — fix for correctness (this PR), don’t rank it as “attackers are probably doing this.” On a 0–10 “will this get used against users” scale: about 1–2.