bitlogin/docs/entropy-audit-2026-07-31.md

BitLogin — randomness and RNG-integration audit, 2026-07-31

Audit of every security-sensitive random value in packages/, against a Coldcard-class checklist: where each bit of unpredictability originates, whether that source is present in the production build, and what happens when it fails.

Verdict

No predictable-fallback defect exists. packages/core/src/crypto/random.ts is the single chokepoint for every secret this codebase mints, and it is correct:

• It throws when crypto.getRandomValues is unavailable rather than falling back to anything (random.ts:5-9). There is no second code path.
• randomUniformInt (random.ts:36-49) uses rejection sampling, so wordlist and charset selection carry no modulo bias.
• generatePrivateKey (crypto/secp256k1.ts:16-21) rejection-samples until the scalar is in range rather than clamping or reducing.
• Nothing seeds a CSPRNG manually; nothing uses timestamps, PIDs, device IDs or counters as entropy.
• Crypto dependencies are exact-pinned, not caret-ranged (@noble/curves 1.9.7, @noble/hashes 1.8.0, @scure/bip39 1.6.0, @scure/base 1.2.6), and CI emits a CycloneDX SBOM plus per-artifact SHA256SUMS.

Entropy accounting is sound: 128 bits into a 12-word BIP-39 phrase (randomEntropy128), 256-bit connection-vault root and sudo key, 128-bit account id, ~77 bits for the default six-word EFF passphrase with a hard floor at 64 bits (account/passphrase.ts:28-30). No source is truncated before use, and no transformation is treated as creating entropy.

The work below is hardening, not repair.

---

Open items

M1 — Nonce-injection hooks are reachable from the shipped public API — DONE 2026-07-31

nip44Encrypt and nip04Encrypt no longer take a nonce or IV at all. The capability moved to packages/core/src/crypto/testing.ts as __unsafeNip44EncryptWithNonce / __unsafeNip04EncryptWithIv, and crypto/index.ts now re-exports explicitly instead of export *, so the underlying encryptWithNonce / encryptWithIv stay off the package surface.

It turned out only one test ever used an override — a nonce-length validation check — and ivOverride had no callers at all. The official NIP-44 vectors do not need it. So nothing was lost by closing this.

Verified from a real consumer package linking the built @bitlogin/core, not from source:

Attempt	Result
import "@bitlogin/core/crypto/testing.js"	ERR_PACKAGE_PATH_NOT_EXPORTED
import "@bitlogin/core/dist/crypto/testing.js"	ERR_PACKAGE_PATH_NOT_EXPORTED
import "@bitlogin/core/crypto/nip44.js"	ERR_PACKAGE_PATH_NOT_EXPORTED
"encryptWithNonce" in cryptoSurface	false
nip44Encrypt.length / nip04Encrypt.length	2 / 3 — no nonce or IV parameter

Four regression tests pin the closed surface, including a fresh-nonce check across 64 calls. knownAnswer.test.ts stayed green throughout, which is the signal that matters: the derivation chain is untouched and no account is affected.

The original problem statement follows.

M1 (original) — Nonce-injection hooks are reachable from the shipped public API

nip44Encrypt and nip04Encrypt accept caller-supplied nonces:

// packages/core/src/crypto/nip44.ts:124
export function nip44Encrypt(conversationKey: Uint8Array, plaintext: string, nonceOverride?: Uint8Array): string {
  const nonce = nonceOverride ?? randomBytes(32);

// packages/core/src/crypto/nip04.ts:24
  ivOverride?: Uint8Array
): string {
  const iv = ivOverride ?? randomBytes(16);

These exist for a legitimate reason — the official NIP-44 v2 test vectors pin the nonce, and crypto.test.ts cannot verify against them otherwise. No production caller passes either parameter; the only other references are the generated .d.ts declarations.

The problem is the surface, not the current callers. Both functions are exported from @bitlogin/core/crypto, so the override is part of what this package advertises. Nonce reuse in NIP-44 (ChaCha20) and NIP-04 (AES-CBC) is not a gradual weakening — it is immediate plaintext recovery, and for CBC a repeated IV leaks the first block relationship directly. A parameter that silently accepts a fixed nonce should not be one autocomplete away from application code.

Fix. Keep the vectors working, remove the footgun:

1. Rename the parameterised forms to __unsafeNip44EncryptWithNonce / __unsafeNip04EncryptWithIv and move them into a crypto/testing.ts module that is not listed in the exports map of packages/core/package.json.
2. Leave nip44Encrypt / nip04Encrypt with two- and three-argument signatures that always call randomBytes.
3. Point crypto.test.ts and knownAnswer.test.ts at the new module via a relative import.

Acceptance. crypto.test.ts still passes the official NIP-44 vectors; import { nip44Encrypt } from "@bitlogin/core/crypto" exposes no nonce parameter in its type signature; npm run security:ci is green.

Care required. This touches the derivation chain's neighbourhood but not the chain itself — no capsule bytes change. knownAnswer.test.ts must stay green without its expected values being edited. If it goes red, stop: per the standing rule in TODO.md, that means the change is account-breaking.

---

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 phrase and seed, Schnorr, AES-GCM tamper-detection, JCS ordering, padding buckets. None of it asserts what happens when randomness is unavailable.

random.ts:5-9 is correct today. Nothing in CI would report it if a future refactor made it return zeros, or added an ?? fallback, or moved the guard behind a condition that no longer fires. A deterministic stream passes every self-consistency test in the suite.

Fix. Add to packages/core/src/crypto/crypto.test.ts:

describe("RNG fails closed (§11.1)", () => {
  it("throws when no secure random source exists", () => {
    const saved = Object.getOwnPropertyDescriptor(globalThis, "crypto");
    try {
      Object.defineProperty(globalThis, "crypto", { value: undefined, configurable: true });
      expect(() => randomBytes(32)).toThrow(/no cryptographically secure/i);
      expect(() => randomEntropy128()).toThrow();
      expect(() => generatePrivateKey()).toThrow();
    } finally {
      if (saved) Object.defineProperty(globalThis, "crypto", saved);
    }
  });

  it("throws when crypto exists but getRandomValues does not", () => {
    const saved = Object.getOwnPropertyDescriptor(globalThis, "crypto");
    try {
      Object.defineProperty(globalThis, "crypto", { value: {}, configurable: true });
      expect(() => randomBytes(32)).toThrow(/no cryptographically secure/i);
    } finally {
      if (saved) Object.defineProperty(globalThis, "crypto", saved);
    }
  });
});

Add a companion uniformity test proving the rejection loop actually rejects — without it, randomUniformInt could degrade to value % maxExclusive and stay green:

it("randomUniformInt is unbiased across a modulus that does not divide 256", () => {
  const M = 94, N = 200_000;
  const counts = new Array(M).fill(0);
  for (let i = 0; i < N; i++) counts[randomUniformInt(M)]++;
  const expected = N / M;
  const chi2 = counts.reduce((a, c) => a + (c - expected) ** 2 / expected, 0);
  expect(chi2).toBeLessThan(160); // df=93, p<0.001 critical value ~147
});

Acceptance. Both tests pass on main. Deliberately breaking random.ts (returning a zero-filled array, or swapping the rejection loop for a bare modulo) must turn them red — verify this locally before committing, then revert.

---

L1 — Math.random in the backup-verification quiz — DONE 2026-07-31

pickRandomIndices now uses randomUniformInt, and relay.ts's subscription id is bytesToHex(randomBytes(8)) rather than a hash over Math.random(). Neither was a secret; both were changed so the ban below has no standing exception.

That ban is now machine-enforced. .semgrep.yml adds three rules over packages/*/src/**, run in CI as a separate step after --config auto:

• bitlogin-no-math-random — no Math.random anywhere, secret or not
• bitlogin-no-crypto-fallback — no ?? fallback, || fallback, or catch-and-substitute around getRandomValues
• bitlogin-no-time-or-id-seeded-secrets — no hashing Date.now() / performance.now() into a key

Verified the rules actually fire rather than merely passing: reintroducing the old Math.floor(Math.random() * pool.length) produces 1 blocking finding and exit 1; the tree is clean at 0 findings with it restored.

The original note follows.

L1 (original) — Math.random in the backup-verification quiz

packages/widget/src/element.ts:1471:

function pickRandomIndices(max: number, count: number): number[] {
  ...
  const idx = Math.floor(Math.random() * pool.length);

Called once, at element.ts:857, to choose which three recovery-phrase words to ask the user to re-type after registration. This is not a secret and leaks no entropy — the phrase itself comes from randomEntropy128, and an attacker who cannot see the phrase gains nothing from predicting which words get quizzed.

It is worth changing anyway, for a process reason. As long as one Math.random call survives in security-adjacent source, a static-analysis rule banning Math.random (checklist §13) needs a standing exception — and standing exceptions are where the next real one hides.

Fix. Import randomUniformInt from @bitlogin/core/crypto and replace the Math.floor(Math.random() * pool.length) expression. Then add the ban rule to the semgrep config so the file cannot regress.

Note. src/vendor/bitlogin/bitlogin.js:1696 in the bitunlock repo is this same function, vendored. Fixing it here and re-running scripts/build-bitlogin-widget.mjs there clears both.

---

L2 — Math.random in the relay subscription id — DONE 2026-07-31 (folded into L1)

packages/core/src/nostr/relay.ts:223 mixes Math.random() into the material hashed for a subscription id. A subscription id is a public wire identifier scoped to one socket; it is not a secret and collision is the only failure mode. No change needed. Recorded so the next audit does not re-flag it.

---

Not findings, recorded so they are not re-litigated

• params.now test hook in account/create.ts:62 injects a timestamp, not entropy. Timestamps are already public in Nostr event envelopes.
• @noble v2 upgrade. Out of scope here, and blocked for an unrelated reason already captured in TODO.md — noble v2 dropped the extensionless subpath exports this code imports.

Sequencing

M2 first — it is additive, cannot break the derivation chain, and it is what protects M1's fix from silently regressing later. Then M1. Then L1, batched with whatever else next touches element.ts.