mirror of
https://github.com/PR0M3TH3AN/bitlogin.git
synced 2026-10-27 08:31:21 +00:00
Pin fail-closed RNG behaviour and randomUniformInt's uniformity
crypto/random.ts is the single chokepoint for every secret this codebase mints -- recovery entropy, everyday keys, vault roots, AEAD nonces, account ids, generated passphrases. It is correct today: it throws when no secure source exists rather than falling back, and rejection-samples to avoid modulo bias. Nothing in CI would have reported it if either stopped being true. That is the gap this closes. The existing suite is strong on known-answer vectors -- HKDF, ScalarExpand, Argon2id, BIP-39, Schnorr, GCM tamper detection, NIP-44 official vectors -- but every one of them is satisfied by a deterministic stream. Same input, same output. That is exactly how a predictable RNG survives a health check that only asks whether the output looks random. Adds two blocks, nine tests: - Fail-closed: no crypto object, crypto without getRandomValues, and getRandomValues present but not callable must each throw, and randomEntropy128 / generatePrivateKey / randomUniformInt must abort rather than substitute a value. Plus a check that normal operation resumes once a source is available again, so the guard cannot be satisfied by breaking everything permanently. - Uniformity: chi-square over 200k draws at modulus 94, full-range coverage at the EFF wordlist modulus of 7776, rejection of invalid bounds, and the degenerate modulus of 1. Both mutation-verified rather than assumed. Replacing the throw with a zero-filling stub turns five fail-closed tests red. Replacing rejection sampling with a bare `value % maxExclusive` gives chi-square 5614 against a threshold of 147 -- while leaving every other test in the suite green, which is the whole reason the distribution test has to exist. Tests only; no source or behaviour change. Suite 172 -> 181. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01HtNMsKzzFTPURemX7tT19u
This commit is contained in:
1 parent
ba49673ef9
commit
b85e1572ea
3 files changed
+120
-4
No files matched your search
+5
-3
@@ -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`).
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<number>(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<number>();
|
||||
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]);
|
||||
|
||||
Reference in new issue
Block a user