diff --git a/docs/TODO.md b/docs/TODO.md index fda77a2..5132af9 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -9,9 +9,11 @@ randomness-integration audit. **No predictable-fallback defect:** `random.ts` is the single chokepoint, throws rather than degrading, and rejection-samples to avoid modulo bias. Three hardening items, in order: -- **M2** — nothing tests that the RNG fails closed. Do this first; it is - additive, cannot touch the derivation chain, and it is what stops M1's fix - from regressing later. +- **M2 — done.** Nine tests in `packages/core/src/crypto/crypto.test.ts` pin + fail-closed behaviour and `randomUniformInt`'s uniformity. Mutation-verified: + a zero-filling stub in place of the throw turns five of them red, and + swapping rejection sampling for bare modulo gives chi-square 5614 against a + threshold of 147. - **M1** — `nonceOverride` / `ivOverride` are reachable from the public `@bitlogin/core/crypto` surface. Move behind a non-exported test module. - **L1** — `Math.random` in the backup-quiz word picker (`element.ts:1471`). diff --git a/docs/entropy-audit-2026-07-31.md b/docs/entropy-audit-2026-07-31.md index a3750ce..c07c290 100644 --- a/docs/entropy-audit-2026-07-31.md +++ b/docs/entropy-audit-2026-07-31.md @@ -81,7 +81,23 @@ rule in `TODO.md`, that means the change is account-breaking. --- -### M2 — Nothing tests that the RNG fails closed +### M2 — Nothing tests that the RNG fails closed — **DONE 2026-07-31** + +Landed in `packages/core/src/crypto/crypto.test.ts` as two new blocks, nine +tests, bringing that file to 59 and the suite to 181. + +Both guards were mutation-verified rather than assumed: + +| Mutation | Result | +| --- | --- | +| `webcrypto()` returns a zero-filling stub instead of throwing | 5 fail-closed tests red | +| `randomUniformInt` uses bare `value % maxExclusive` | chi-square **5614** against a threshold of 147 | + +The second number is the point of the exercise: a modulo-biased generator +still produces different-looking values every call and passes every +self-consistency test in the suite. Only a distribution test sees it. + +The original problem statement follows. This is the gap the whole exercise turns on. The crypto suite is genuinely good — known-answer coverage for HKDF, ScalarExpand, the Argon2id profile, BIP-39 diff --git a/packages/core/src/crypto/crypto.test.ts b/packages/core/src/crypto/crypto.test.ts index a6691dc..517d09f 100644 --- a/packages/core/src/crypto/crypto.test.ts +++ b/packages/core/src/crypto/crypto.test.ts @@ -10,6 +10,7 @@ import { aesGcmOpen, aesGcmSeal } from "./aesGcm.js"; import { bytesToHex, hexToBytes, bytesToBase64url, base64urlToBytes, utf8ToBytes } from "./encoding.js"; import { getConversationKey, MAX_NIP44_BROWSER_PLAINTEXT_LEN, MAX_NIP44_ENCODED_PAYLOAD_LEN, nip44Encrypt, nip44Decrypt } from "./nip44.js"; import { nip04Encrypt, nip04Decrypt } from "./nip04.js"; +import { randomBytes, randomEntropy128, randomUniformInt } from "./random.js"; describe("ScalarExpand (§11.4 test vector)", () => { it("derives a stable, valid scalar for fixed input on counter 0", () => { @@ -295,6 +296,103 @@ function bytesToBase64urlForTest(bytes: Uint8Array): string { return btoa(String.fromCharCode(...bytes)); } +describe("RNG fails closed (§11.1, §11.6)", () => { + /** + * Every secret this codebase mints -- recovery entropy, everyday keys, vault + * roots, AEAD nonces, account ids, generated passphrases -- funnels through + * random.ts. It is correct today: it throws when no secure source exists + * rather than falling back. Nothing else in this suite would notice if that + * stopped being true. + * + * That is the gap these tests close. A deterministic stream satisfies every + * self-consistency check here -- same input, same output -- which is exactly + * how a predictable RNG survives a health check that only asks whether the + * output looks random. + */ + const withGlobalCrypto = (value: unknown, run: () => void): void => { + const saved = Object.getOwnPropertyDescriptor(globalThis, "crypto"); + try { + Object.defineProperty(globalThis, "crypto", { value, configurable: true }); + run(); + } finally { + if (saved) Object.defineProperty(globalThis, "crypto", saved); + else delete (globalThis as { crypto?: unknown }).crypto; + } + }; + + it("throws when no crypto object exists at all", () => { + withGlobalCrypto(undefined, () => { + expect(() => randomBytes(32)).toThrow(/no cryptographically secure/iu); + }); + }); + + it("throws when crypto exists but getRandomValues does not", () => { + withGlobalCrypto({}, () => { + expect(() => randomBytes(32)).toThrow(/no cryptographically secure/iu); + }); + }); + + it("throws when getRandomValues is not callable", () => { + withGlobalCrypto({ getRandomValues: "nope" }, () => { + expect(() => randomBytes(32)).toThrow(/no cryptographically secure/iu); + }); + }); + + it("aborts key and entropy generation rather than substituting a value", () => { + withGlobalCrypto(undefined, () => { + // Each must fail, not silently degrade: a returned value here is the + // COLDCARD defect class -- predictable material that looks fine. + expect(() => randomEntropy128()).toThrow(); + expect(() => generatePrivateKey()).toThrow(); + expect(() => randomUniformInt(7776)).toThrow(); + }); + }); + + it("recovers normally once a secure source is available again", () => { + withGlobalCrypto(undefined, () => { + expect(() => randomBytes(1)).toThrow(); + }); + expect(randomBytes(32)).toHaveLength(32); + }); +}); + +describe("randomUniformInt is unbiased (§9.2)", () => { + /** + * Guards the rejection loop specifically. Replacing it with a bare + * `value % maxExclusive` would leave every other test green while giving 68 + * of 94 charset positions a 50% higher probability than the other 26. + */ + it("stays uniform across a modulus that does not divide 256", () => { + const modulus = 94; + const samples = 200_000; + const counts = new Array(modulus).fill(0); + for (let i = 0; i < samples; i++) counts[randomUniformInt(modulus)]! += 1; + + const expected = samples / modulus; + const chi2 = counts.reduce((acc, c) => acc + (c - expected) ** 2 / expected, 0); + // df = 93; the p<0.001 critical value is ~147. A modulo-biased generator + // lands in the thousands here. + expect(chi2).toBeLessThan(147); + expect(Math.min(...counts)).toBeGreaterThan(0); + }); + + it("covers the full range of the EFF wordlist modulus", () => { + const seen = new Set(); + for (let i = 0; i < 100_000; i++) seen.add(randomUniformInt(7776)); + expect(seen.size).toBeGreaterThan(7000); + }); + + it("rejects invalid bounds instead of returning something", () => { + for (const bad of [0, -1, 1.5, 2 ** 32 + 1, Number.NaN]) { + expect(() => randomUniformInt(bad)).toThrow(/maxExclusive/u); + } + }); + + it("always returns 0 for a modulus of 1", () => { + for (let i = 0; i < 100; i++) expect(randomUniformInt(1)).toBe(0); + }); +}); + describe("Encoding helpers", () => { it("hex round-trips", () => { const bytes = new Uint8Array([0, 1, 2, 253, 254, 255]);