mirror of
https://github.com/PR0M3TH3AN/bitlogin.git
synced 2026-10-27 08:31:21 +00:00
Take nonce and IV control off the public crypto surface
nip44Encrypt and nip04Encrypt took optional nonceOverride / ivOverride parameters. No production caller ever passed one -- but they were exported from @bitlogin/core/crypto, so "encrypt with a nonce I chose" was a discoverable feature of the package rather than an internal detail. That is worth closing because neither primitive degrades gracefully. NIP-44 is ChaCha20: a nonce reused under one conversation key recovers plaintext outright. NIP-04 is AES-CBC, where a repeated IV leaks block-equality between messages and a predictable one gives an attacker considerably more. A parameter that silently accepts a fixed nonce should not sit one autocomplete away from application code. The public functions now take no nonce or IV and always draw from the CSPRNG. The capability moved to crypto/testing.ts as __unsafeNip44EncryptWithNonce / __unsafeNip04EncryptWithIv, which is absent from both crypto/index.ts and the package.json exports map. crypto/index.ts also switches from `export *` to an explicit re-export list for these two modules, so the shared encryptWithNonce / encryptWithIv helpers stay off the surface even though their modules must export them for the test entry point to reach them. Closing this cost nothing: exactly one test used an override -- a nonce-length validation check -- and ivOverride had no callers at all. The official NIP-44 vectors do not need it. Verified from a throwaway consumer package linking the BUILT @bitlogin/core rather than from source. Deep imports of crypto/testing.js, dist/crypto/testing.js and crypto/nip44.js all fail with ERR_PACKAGE_PATH_NOT_EXPORTED; the crypto surface contains neither encryptWithNonce nor encryptWithIv; and nip44Encrypt/nip04Encrypt report arity 2/3, i.e. no nonce or IV parameter. Adds four regression tests pinning the closed surface, including that 64 calls with identical plaintext produce 64 distinct nonces and IVs. knownAnswer.test.ts stayed green throughout, which is the signal that matters here: no capsule bytes change and no existing account is affected. Typecheck, build and the full suite pass (185 tests). 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
b85e1572ea
commit
e131f0313b
7 files changed
+199
-10
No files matched your search
+8
-2
@@ -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
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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";
|
||||
@@ -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)}`;
|
||||
}
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
Reference in new issue
Block a user