diff --git a/docs/TODO.md b/docs/TODO.md index 5132af9..a1ef5d9 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -14,10 +14,16 @@ avoid modulo bias. Three hardening items, in order: 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. +- **M1 — done.** `nip44Encrypt` / `nip04Encrypt` no longer take a nonce or IV; + the capability lives in the non-exported `crypto/testing.ts`, and + `crypto/index.ts` re-exports explicitly instead of `export *`. Only one test + ever used an override and `ivOverride` had no callers, so nothing was lost. + Verified from a real consumer against the built package: all three deep-import + routes fail with `ERR_PACKAGE_PATH_NOT_EXPORTED`. `knownAnswer.test.ts` green + throughout — the derivation chain is untouched. - **L1** — `Math.random` in the backup-quiz word picker (`element.ts:1471`). Not a secret, but it forces a standing exception in any `Math.random` ban rule. + Last remaining item from the entropy audit in this repo. ## Owner action items diff --git a/docs/entropy-audit-2026-07-31.md b/docs/entropy-audit-2026-07-31.md index c07c290..097491e 100644 --- a/docs/entropy-audit-2026-07-31.md +++ b/docs/entropy-audit-2026-07-31.md @@ -33,7 +33,37 @@ The work below is hardening, not repair. ## Open items -### M1 — Nonce-injection hooks are reachable from the shipped public API +### 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: diff --git a/packages/core/src/crypto/crypto.test.ts b/packages/core/src/crypto/crypto.test.ts index 517d09f..e7cc1af 100644 --- a/packages/core/src/crypto/crypto.test.ts +++ b/packages/core/src/crypto/crypto.test.ts @@ -11,6 +11,8 @@ import { bytesToHex, hexToBytes, bytesToBase64url, base64urlToBytes, utf8ToBytes 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"; +import { __unsafeNip44EncryptWithNonce, __unsafeNip04EncryptWithIv } from "./testing.js"; +import * as cryptoIndex from "./index.js"; describe("ScalarExpand (§11.4 test vector)", () => { it("derives a stable, valid scalar for fixed input on counter 0", () => { @@ -208,7 +210,9 @@ describe("NIP-44 v2 (window.nostr.nip44 provider surface)", () => { it("validates conversation-key and nonce lengths before cryptographic use", () => { const key = getConversationKey(generatePrivateKey(), getPublicKeyHex(generatePrivateKey())); expect(() => nip44Encrypt(new Uint8Array(31), "hello")).toThrow(/conversation key/u); - expect(() => nip44Encrypt(key, "hello", new Uint8Array(31))).toThrow(/nonce/u); + // The nonce check is only reachable through the test-only entry point -- + // nip44Encrypt always supplies its own 32 bytes from the CSPRNG. + expect(() => __unsafeNip44EncryptWithNonce(key, "hello", new Uint8Array(31))).toThrow(/nonce/u); }); it("rejects unsupported encodings and undersized payloads before parsing fields", () => { @@ -296,6 +300,59 @@ function bytesToBase64urlForTest(bytes: Uint8Array): string { return btoa(String.fromCharCode(...bytes)); } +describe("Nonce/IV control stays off the package surface (§11.1)", () => { + /** + * nip44Encrypt and nip04Encrypt once took optional nonceOverride/ivOverride + * parameters. No production caller passed one, but they were exported, so + * "encrypt with a nonce I chose" was a discoverable feature of the package. + * These tests pin the closed surface so it cannot quietly reopen. + */ + it("does not re-export the nonce/IV-taking helpers from crypto/index", () => { + const surface = Object.keys(cryptoIndex); + expect(surface).toContain("nip44Encrypt"); + expect(surface).toContain("nip04Encrypt"); + expect(surface).not.toContain("encryptWithNonce"); + expect(surface).not.toContain("encryptWithIv"); + expect(surface).not.toContain("__unsafeNip44EncryptWithNonce"); + expect(surface).not.toContain("__unsafeNip04EncryptWithIv"); + }); + + it("public encrypt functions take no nonce/IV argument", () => { + // Arity is the machine-checkable form of "there is no third parameter". + expect(nip44Encrypt).toHaveLength(2); + expect(nip04Encrypt).toHaveLength(3); + }); + + it("public encrypt functions produce a fresh nonce every call", () => { + const key = getConversationKey(generatePrivateKey(), getPublicKeyHex(generatePrivateKey())); + const payloads = new Set(Array.from({ length: 64 }, () => nip44Encrypt(key, "identical"))); + expect(payloads.size).toBe(64); + + const sk = generatePrivateKey(); + const peer = getPublicKeyHex(generatePrivateKey()); + const ivs = new Set( + Array.from({ length: 64 }, () => nip04Encrypt(sk, peer, "identical").split("?iv=")[1]), + ); + expect(ivs.size).toBe(64); + }); + + it("the test-only entry points still round-trip and pin their length checks", () => { + const key = getConversationKey(generatePrivateKey(), getPublicKeyHex(generatePrivateKey())); + const nonce = new Uint8Array(32).fill(7); + const encrypted = __unsafeNip44EncryptWithNonce(key, "pinned", nonce); + expect(nip44Decrypt(key, encrypted)).toBe("pinned"); + // Deterministic given a fixed nonce -- which is exactly why it is not public. + expect(__unsafeNip44EncryptWithNonce(key, "pinned", nonce)).toBe(encrypted); + + const sk = generatePrivateKey(); + const peer = getPublicKeyHex(generatePrivateKey()); + const iv = new Uint8Array(16).fill(9); + const nip04 = __unsafeNip04EncryptWithIv(sk, peer, "pinned", iv); + expect(nip04Decrypt(sk, peer, nip04)).toBe("pinned"); + expect(() => __unsafeNip04EncryptWithIv(sk, peer, "x", new Uint8Array(15))).toThrow(/iv/u); + }); +}); + describe("RNG fails closed (§11.1, §11.6)", () => { /** * Every secret this codebase mints -- recovery entropy, everyday keys, vault diff --git a/packages/core/src/crypto/index.ts b/packages/core/src/crypto/index.ts index dab3326..d5ccfff 100644 --- a/packages/core/src/crypto/index.ts +++ b/packages/core/src/crypto/index.ts @@ -9,5 +9,16 @@ export * from "./argon2id.js"; export * from "./bip39.js"; export * from "./padding.js"; export * from "./jcs.js"; -export * from "./nip44.js"; -export * from "./nip04.js"; +// Explicit rather than `export *` so the nonce/IV-taking helpers stay off the +// package surface (§11.1). `encryptWithNonce` and `encryptWithIv` are exported +// from their modules only so ./testing.js can reach them; re-exporting them here +// would put "encrypt with a nonce you chose" one autocomplete away from +// application code, which for ChaCha20 and AES-CBC is the whole risk. +export { + MAX_NIP44_BROWSER_PLAINTEXT_LEN, + MAX_NIP44_ENCODED_PAYLOAD_LEN, + getConversationKey, + nip44Encrypt, + nip44Decrypt, +} from "./nip44.js"; +export { getSharedSecret, nip04Encrypt, nip04Decrypt } from "./nip04.js"; diff --git a/packages/core/src/crypto/nip04.ts b/packages/core/src/crypto/nip04.ts index 515a03b..e6cb8ee 100644 --- a/packages/core/src/crypto/nip04.ts +++ b/packages/core/src/crypto/nip04.ts @@ -17,14 +17,33 @@ export function getSharedSecret(privateKey: Uint8Array, peerPublicKeyHex: string return shared.slice(1, 33); // drop the compressed-point prefix byte, keep the X coordinate } +/** + * Encrypts with a fresh 16-byte IV from the CSPRNG. + * + * As with nip44Encrypt, there is deliberately no caller-supplied-IV parameter + * here (§11.1). NIP-04 is AES-CBC: a repeated IV under the same shared secret + * leaks whether two messages share a leading block, and CBC gives an attacker + * far more leverage than that once IVs are predictable. Tests use + * `__unsafeNip04EncryptWithIv` from ./testing.js, which the package does not + * export. + */ export function nip04Encrypt( privateKey: Uint8Array, peerPublicKeyHex: string, plaintext: string, - ivOverride?: Uint8Array ): string { + return encryptWithIv(privateKey, peerPublicKeyHex, plaintext, randomBytes(16)); +} + +/** @internal Shared by nip04Encrypt and the test-only entry point. */ +export function encryptWithIv( + privateKey: Uint8Array, + peerPublicKeyHex: string, + plaintext: string, + iv: Uint8Array, +): string { + if (iv.length !== 16) throw new Error("NIP-04 iv must be exactly 16 bytes."); const key = getSharedSecret(privateKey, peerPublicKeyHex); - const iv = ivOverride ?? randomBytes(16); const ciphertext = cbc(key, iv).encrypt(utf8ToBytes(plaintext)); return `${base64.encode(ciphertext)}?iv=${base64.encode(iv)}`; } diff --git a/packages/core/src/crypto/nip44.ts b/packages/core/src/crypto/nip44.ts index 9e80a01..c5e6a82 100644 --- a/packages/core/src/crypto/nip44.ts +++ b/packages/core/src/crypto/nip44.ts @@ -121,8 +121,26 @@ function calcMac(hmacKey: Uint8Array, nonce: Uint8Array, ciphertext: Uint8Array) return hmac(sha256, hmacKey, concatBytes(nonce, ciphertext)); } -export function nip44Encrypt(conversationKey: Uint8Array, plaintext: string, nonceOverride?: Uint8Array): string { - const nonce = nonceOverride ?? randomBytes(32); +/** + * Encrypts with a fresh 32-byte nonce from the CSPRNG. + * + * There is deliberately no caller-supplied-nonce parameter on this function + * (§11.1). NIP-44 is ChaCha20: reusing a nonce under one conversation key + * recovers plaintext outright, so "pass your own nonce" is not a knob that + * belongs on the public surface. Tests that must pin a nonce use + * `__unsafeNip44EncryptWithNonce` from ./testing.js, which is not exported from + * the package. + */ +export function nip44Encrypt(conversationKey: Uint8Array, plaintext: string): string { + return encryptWithNonce(conversationKey, plaintext, randomBytes(32)); +} + +/** @internal Shared by nip44Encrypt and the test-only entry point. */ +export function encryptWithNonce( + conversationKey: Uint8Array, + plaintext: string, + nonce: Uint8Array, +): string { if (nonce.length !== 32) throw new Error("NIP-44 nonce must be exactly 32 bytes."); const { chachaKey, chachaNonce, hmacKey } = deriveMessageKeys(conversationKey, nonce); const padded = pad(utf8ToBytes(plaintext)); diff --git a/packages/core/src/crypto/testing.ts b/packages/core/src/crypto/testing.ts new file mode 100644 index 0000000..649b570 --- /dev/null +++ b/packages/core/src/crypto/testing.ts @@ -0,0 +1,48 @@ +/** + * Test-only entry points that accept caller-supplied nonces and IVs (§11.1). + * + * NOT EXPORTED FROM THE PACKAGE. This module is deliberately absent from + * ./index.js and from the `exports` map in package.json, so it is unreachable + * as `@bitlogin/core/crypto/testing.js` — Node's exports field blocks the deep + * import, and the explicit re-export list in ./index.ts blocks the shallow one. + * Reach it only by relative path from inside this package's own tests. + * + * WHY THIS FILE EXISTS RATHER THAN AN OPTIONAL PARAMETER: `nip44Encrypt` and + * `nip04Encrypt` used to take `nonceOverride` / `ivOverride`. No production + * caller ever passed one, but the parameters were on the public surface, which + * meant "encrypt with a nonce I picked" was a discoverable feature of the + * package. NIP-44 is ChaCha20 — a nonce repeated under one conversation key + * recovers plaintext outright, not gradually. NIP-04 is AES-CBC, where a + * repeated IV leaks block-equality between messages. Neither failure degrades + * gracefully, so the capability lives here, behind a name no one types by + * accident, instead of in an argument list. + */ +import { encryptWithNonce } from "./nip44.js"; +import { encryptWithIv } from "./nip04.js"; + +/** + * @internal Encrypts with a caller-supplied 32-byte nonce. + * UNSAFE: reusing a nonce under one conversation key breaks confidentiality. + * For test vectors and validation tests only. + */ +export function __unsafeNip44EncryptWithNonce( + conversationKey: Uint8Array, + plaintext: string, + nonce: Uint8Array, +): string { + return encryptWithNonce(conversationKey, plaintext, nonce); +} + +/** + * @internal Encrypts with a caller-supplied 16-byte IV. + * UNSAFE: a predictable or repeated IV under one shared secret leaks structure. + * For test vectors and validation tests only. + */ +export function __unsafeNip04EncryptWithIv( + privateKey: Uint8Array, + peerPublicKeyHex: string, + plaintext: string, + iv: Uint8Array, +): string { + return encryptWithIv(privateKey, peerPublicKeyHex, plaintext, iv); +}