diff --git a/js/packages/cli/src/appDir.ts b/js/packages/cli/src/appDir.ts index 7afe753e..7e7007b8 100644 --- a/js/packages/cli/src/appDir.ts +++ b/js/packages/cli/src/appDir.ts @@ -12,7 +12,7 @@ import { mkdir, readFile, rm } from "node:fs/promises"; import { existsSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join, resolve } from "node:path"; -import { atomicWrite } from "./vaultFile.js"; +import { atomicWrite, withVaultLock } from "./vaultFile.js"; import process from "node:process"; import { base64url } from "@scure/base"; import { @@ -105,7 +105,22 @@ export class AppDir { async writeFingerprints(data: FingerprintsFile): Promise { await mkdir(this.root, { recursive: true }); // indent=4 matches the Python writer - await atomicWrite(this.fingerprintsPath, utf8(JSON.stringify(data, null, 4))); + await withVaultLock(this.fingerprintsPath, () => + atomicWrite(this.fingerprintsPath, utf8(JSON.stringify(data, null, 4))), + ); + } + + /** Read-modify-write the registry under one lock. */ + async mutateFingerprints( + fn: (data: FingerprintsFile) => T | Promise, + ): Promise { + await mkdir(this.root, { recursive: true }); + return withVaultLock(this.fingerprintsPath, async () => { + const data = await this.readFingerprints(); + const result = await fn(data); + await atomicWrite(this.fingerprintsPath, utf8(JSON.stringify(data, null, 4))); + return result; + }); } /** Create a profile from a mnemonic; returns its fingerprint. */ @@ -121,6 +136,20 @@ export class AppDir { throw new Error(`profile ${fingerprint} already exists`); } const dir = this.profileDir(fingerprint); + // The registry is not proof of absence: readFingerprints swallows a + // corrupt or unreadable fingerprints.json and reports no profiles. The + // files on disk are what matter — restoring a profile directory from a + // backup and re-registering it is the canonical "recover my vault" + // gesture, and it must not overwrite the seed and index. + for (const existing of [PARENT_SEED_FILENAME, INDEX_FILENAME]) { + if (existsSync(join(dir, existing))) { + throw new Error( + `refusing to create profile ${fingerprint}: ${join(dir, existing)} already ` + + `exists. If this profile is missing from fingerprints.json, repair the ` + + `registry rather than re-adding the profile.`, + ); + } + } await mkdir(dir, { recursive: true, mode: 0o700 }); // parent_seed.enc: kdf/ct wrapper around a V3 blob under the password key. diff --git a/js/packages/cli/src/configFile.ts b/js/packages/cli/src/configFile.ts index 3bc1791f..f592d50a 100644 --- a/js/packages/cli/src/configFile.ts +++ b/js/packages/cli/src/configFile.ts @@ -14,7 +14,7 @@ import { utf8, } from "@seedpass/core"; import { CONFIG_FILENAME } from "./appDir.js"; -import { atomicWrite } from "./vaultFile.js"; +import { atomicWrite, withVaultLock } from "./vaultFile.js"; export const DEFAULT_RELAYS = [ "wss://relay.snort.social", @@ -68,7 +68,33 @@ export async function saveConfig( mnemonic: string, config: Record, ): Promise { - const key = deriveIndexKeyBytes(mnemonic); - const payload = await encryptV3(key, utf8(JSON.stringify(config))); - await atomicWrite(join(profileDir, CONFIG_FILENAME), payload); + const path = join(profileDir, CONFIG_FILENAME); + await withVaultLock(path, async () => { + const key = deriveIndexKeyBytes(mnemonic); + const payload = await encryptV3(key, utf8(JSON.stringify(config))); + await atomicWrite(path, payload); + }); +} + +/** + * Read, modify and write the config under one lock. + * + * `config set`, `nostr add-relay` and `nostr remove-relay` are all + * read-modify-write cycles. Locking only the write loses concurrent updates + * — the config holds the relay list and password/pin hashes, so a lost + * update there is not cosmetic. + */ +export async function mutateConfig( + profileDir: string, + mnemonic: string, + fn: (config: Record) => T | Promise, +): Promise { + const path = join(profileDir, CONFIG_FILENAME); + return withVaultLock(path, async () => { + const config = await loadConfig(profileDir, mnemonic); + const result = await fn(config); + const key = deriveIndexKeyBytes(mnemonic); + await atomicWrite(path, await encryptV3(key, utf8(JSON.stringify(config)))); + return result; + }); } diff --git a/js/packages/cli/src/program.ts b/js/packages/cli/src/program.ts index e6348dad..f8ea25b4 100644 --- a/js/packages/cli/src/program.ts +++ b/js/packages/cli/src/program.ts @@ -63,6 +63,7 @@ import { deriveSshKeyPair, derivePgpKey, sshPublicKeyOpenSsh, + utf8, } from "@seedpass/core"; import { readFile, writeFile, mkdir } from "node:fs/promises"; import { existsSync, statSync } from "node:fs"; @@ -130,7 +131,7 @@ function resolveHome(p: string): string { return p.startsWith("~") ? join(homedir(), p.slice(1)) : p; } import { AppDir, resolveAppDir, INDEX_FILENAME } from "./appDir.js"; -import { loadConfig, saveConfig } from "./configFile.js"; +import { loadConfig, saveConfig, mutateConfig } from "./configFile.js"; import { AgentClient, AgentDaemon, agentSocketPath, DEFAULT_TTL_SECONDS } from "./agent.js"; import { AuditLog } from "./audit.js"; @@ -647,7 +648,10 @@ export function buildProgram(io: ProgramIo = defaultIo): Command { if (existsSync(dest) && !o.overwrite) { throw new Error(`File already exists: ${dest} (use --overwrite)`); } - await writeFile(dest, hit.entry.content, { mode: 0o600 }); + // atomicWrite creates a fresh 0600 inode and renames it into place, so + // a pre-existing world-readable file (or a symlink pointing somewhere + // else) cannot end up holding this plaintext. + await atomicWrite(dest, utf8(hit.entry.content)); io.out(JSON.stringify({ exported: dest, ref: hit.ref, bytes: hit.entry.content.length })); }); @@ -963,15 +967,15 @@ export function buildProgram(io: ProgramIo = defaultIo): Command { const app = new AppDir(resolveAppDir(opts.appDir)); const fp = await currentFingerprint(app, opts); const mnemonic = await resolveMnemonic(app, opts); - const cfg = await loadConfig(app.profileDir(fp), mnemonic); let parsed: unknown = value; try { parsed = JSON.parse(value); } catch { // keep as string } - cfg[key] = parsed; - await saveConfig(app.profileDir(fp), mnemonic, cfg); + await mutateConfig(app.profileDir(fp), mnemonic, (cfg) => { + cfg[key] = parsed; + }); io.out(JSON.stringify({ [key]: parsed })); }); @@ -1008,10 +1012,13 @@ export function buildProgram(io: ProgramIo = defaultIo): Command { .description("add a relay URL") .action(async (url: string) => { if (!/^wss?:\/\//.test(url)) throw new Error("relay URL must start with ws:// or wss://"); - const { app, fp, mnemonic, cfg } = await nostrContext(program.opts()); - const relays = cfg["relays"] as string[]; - if (!relays.includes(url)) relays.push(url); - await saveConfig(app.profileDir(fp), mnemonic, cfg); + const { app, fp, mnemonic } = await nostrContext(program.opts()); + const relays = await mutateConfig(app.profileDir(fp), mnemonic, (cfg) => { + const list = Array.isArray(cfg["relays"]) ? (cfg["relays"] as string[]) : []; + if (!list.includes(url)) list.push(url); + cfg["relays"] = list; + return list; + }); io.out(JSON.stringify({ relays })); }); @@ -1019,15 +1026,16 @@ export function buildProgram(io: ProgramIo = defaultIo): Command { .command("remove-relay ") .description("remove a relay by 1-based index") .action(async (indexStr: string) => { - const { app, fp, mnemonic, cfg } = await nostrContext(program.opts()); - const relays = cfg["relays"] as string[]; + const { app, fp, mnemonic } = await nostrContext(program.opts()); const i = parseIntOption(indexStr, "index", { min: 1 }) - 1; - if (!Number.isInteger(i) || i < 0 || i >= relays.length) { - throw new Error(`index out of range 1..${relays.length}`); - } - if (relays.length === 1) throw new Error("at least one relay must remain"); - relays.splice(i, 1); - await saveConfig(app.profileDir(fp), mnemonic, cfg); + const relays = await mutateConfig(app.profileDir(fp), mnemonic, (cfg) => { + const list = Array.isArray(cfg["relays"]) ? (cfg["relays"] as string[]) : []; + if (i >= list.length) throw new Error(`index out of range 1..${list.length}`); + if (list.length === 1) throw new Error("at least one relay must remain"); + list.splice(i, 1); + cfg["relays"] = list; + return list; + }); io.out(JSON.stringify({ relays })); }); @@ -1106,7 +1114,12 @@ export function buildProgram(io: ProgramIo = defaultIo): Command { await decryptPayload(indexKey, parseEncryptedFile(delta).ciphertext), ), ); - remote = mergeIndexPayloads(remote, incoming, sha256Hex(delta).slice(0, 16)); + // preserve-current here too: the default would refuse any + // Python profile that carries atlas state as soon as one delta + // exists. + remote = mergeIndexPayloads(remote, incoming, sha256Hex(delta).slice(0, 16), { + index0: "preserve-current", + }); deltaCount++; } } @@ -1343,18 +1356,22 @@ export function buildProgram(io: ProgramIo = defaultIo): Command { .command("export ") .description("write a portable backup (encrypted by default)") .option("--plaintext", "HIGH RISK: export without encryption") - .action(async (destFile: string, cmdOpts: { plaintext?: boolean }) => { + .option("--overwrite", "replace an existing file") + .action(async (destFile: string, o: { plaintext?: boolean; overwrite?: boolean }) => { const vault = await openFromOptions(program.opts()); const wrapper = await exportBackup(vault.index as Record, { mnemonic: vault.mnemonic, fingerprint: generateFingerprint(vault.mnemonic), - encrypt: !cmdOpts.plaintext, + encrypt: !o.plaintext, }); - await writeFile(destFile, JSON.stringify(wrapper, null, 2) + "\n", { mode: 0o600 }); + if (existsSync(destFile) && !o.overwrite) { + throw new Error(`File already exists: ${destFile} (use --overwrite)`); + } + await atomicWrite(destFile, utf8(JSON.stringify(wrapper, null, 2) + "\n")); io.out( JSON.stringify({ exported: destFile, - encrypted: !cmdOpts.plaintext, + encrypted: !o.plaintext, checksum: wrapper.checksum, }), ); diff --git a/js/packages/cli/src/vaultFile.ts b/js/packages/cli/src/vaultFile.ts index b2175be1..eaa92d5b 100644 --- a/js/packages/cli/src/vaultFile.ts +++ b/js/packages/cli/src/vaultFile.ts @@ -3,7 +3,7 @@ * or without the JSON kdf/ct wrapper) unlocked by the parent seed mnemonic. */ -import { readFile, open, rename, rm, stat, chmod } from "node:fs/promises"; +import { readFile, open, rename, rm, stat } from "node:fs/promises"; import { basename, dirname, join } from "node:path"; import { randomBytes } from "node:crypto"; import process from "node:process"; @@ -81,16 +81,39 @@ export async function atomicWrite(path: string, data: Uint8Array): Promise const dir = dirname(path); const tmp = join(dir, `.${basename(path)}.${process.pid}.${randomBytes(6).toString("hex")}.tmp`); const handle = await open(tmp, "wx", 0o600); + let renamed = false; try { - await handle.write(data); + // A short write is not an error in POSIX, and ignoring it is how a + // truncated file gets fsynced and renamed over the only good copy — + // silently, with a success exit code. Loop until everything lands and + // fail loudly if it cannot. + let written = 0; + while (written < data.length) { + const { bytesWritten } = await handle.write(data, written, data.length - written); + if (bytesWritten <= 0) { + throw new Error( + `short write to ${tmp}: wrote ${written} of ${data.length} bytes`, + ); + } + written += bytesWritten; + } + if (written !== data.length) { + throw new Error(`short write to ${tmp}: wrote ${written} of ${data.length} bytes`); + } + // fchmod on the handle, not the path: a path-based chmod after the file + // is visible can be redirected through a symlink. + await handle.chmod(0o600); await handle.sync(); - } finally { await handle.close(); + await rename(tmp, path); + renamed = true; + } finally { + if (!renamed) { + // Never leave a partial ciphertext copy behind. + await handle.close().catch(() => {}); + await rm(tmp, { force: true }).catch(() => {}); + } } - await rename(tmp, path); - // Repair permissions if the target already existed with looser modes: - // writeFile's mode argument applies only on creation. - await chmod(path, 0o600); const dirHandle = await open(dir, "r"); try { await dirHandle.sync(); @@ -110,38 +133,84 @@ export async function atomicWrite(path: string, data: Uint8Array): Promise */ const LOCK_STALE_MS = 30_000; +/** Is a pid still running (and therefore still holding its lock)? */ +function pidAlive(pid: number): boolean { + if (!Number.isInteger(pid) || pid <= 0) return false; + try { + process.kill(pid, 0); + return true; + } catch (e) { + // EPERM means it exists but belongs to someone else — still alive. + return (e as NodeJS.ErrnoException).code === "EPERM"; + } +} + export async function withVaultLock(path: string, fn: () => Promise): Promise { const lockPath = `${path}.lock`; + // A token unique to this acquisition. Releasing or breaking a lock without + // checking it is how two processes end up both believing they hold it: + // rm-then-create is not atomic, so a slow holder's lock can be deleted by + // a racer that already decided it was stale. + const token = `${process.pid}:${randomBytes(12).toString("hex")}`; const deadline = Date.now() + LOCK_STALE_MS; + for (;;) { try { const handle = await open(lockPath, "wx", 0o600); - await handle.write(String(process.pid)); - await handle.close(); + try { + await handle.write(token); + await handle.sync(); + } finally { + await handle.close(); + } break; } catch (e) { if ((e as NodeJS.ErrnoException).code !== "EEXIST") throw e; + + let holder = ""; let age = 0; try { + holder = await readFile(lockPath, "utf8"); age = Date.now() - (await stat(lockPath)).mtimeMs; } catch { - continue; // holder released it between our attempts + continue; // released between our attempts } - if (age > LOCK_STALE_MS) { - await rm(lockPath, { force: true }); + const holderPid = Number.parseInt(holder.split(":")[0] ?? "", 10); + + // Only break a lock whose owner is demonstrably gone. Age alone is a + // guess — a slow disk or a suspended laptop is not a crash. + if (!pidAlive(holderPid) && age > LOCK_STALE_MS) { + // Steal atomically: move the stale file aside and only proceed if + // this process is the one that managed to move it. + const claim = `${lockPath}.claim.${randomBytes(8).toString("hex")}`; + try { + await rename(lockPath, claim); + await rm(claim, { force: true }); + } catch { + // Another process won the steal; fall through and retry. + } continue; } if (Date.now() > deadline) { throw new Error( - `another SeedPass process is holding ${lockPath}; retry, or remove it if no process is running`, + `another SeedPass process (pid ${holderPid || "unknown"}) is holding ` + + `${lockPath}; retry, or remove it if no such process is running`, ); } await new Promise((r) => setTimeout(r, 25)); } } + try { return await fn(); } finally { - await rm(lockPath, { force: true }); + // Release only if we still own it: a lock broken out from under us + // belongs to someone else now, and deleting it would evict them. + try { + const current = await readFile(lockPath, "utf8"); + if (current === token) await rm(lockPath, { force: true }); + } catch { + // Already gone. + } } } diff --git a/js/packages/core/.audit1.mts b/js/packages/core/.audit1.mts deleted file mode 100644 index 2fb52245..00000000 --- a/js/packages/core/.audit1.mts +++ /dev/null @@ -1,9 +0,0 @@ -import { hexToBytes, bytesToHex } from "./src/util/bytes.js"; -const cases = ["1z", "-1", "+1", " 1", "0x", "1 ", "gg", "1\n", "aA", "1e", "Infinity".slice(0,2), "e5", "1_", ".5", "0b"]; -for (const c of cases) { - try { console.log(JSON.stringify(c), "->", bytesToHex(hexToBytes(c))); } - catch (e) { console.log(JSON.stringify(c), "-> THROW", (e as Error).message); } -} -// bech32 roundtrip malleability -import { hexToBech32 } from "./src/derive/nostr.js"; -console.log("npub from 'zz'*32:", (()=>{try{return hexToBech32("zz".repeat(32) as string, "npub")}catch(e){return "throw "+(e as Error).message}})()); diff --git a/js/packages/core/src/derive/bip85.ts b/js/packages/core/src/derive/bip85.ts index ff9880bb..1bb72847 100644 --- a/js/packages/core/src/derive/bip85.ts +++ b/js/packages/core/src/derive/bip85.ts @@ -19,9 +19,24 @@ import { import { wordlist as english } from "@scure/bip39/wordlists/english.js"; import { utf8 } from "../util/bytes.js"; +/** + * Canonical form of a mnemonic: NFKD, lowercase, single-space separated. + * + * This must be applied before BOTH validation and derivation. `@scure`'s + * normalize only counts words and then PBKDF2s the raw string, so a phrase + * with a trailing newline validates yet derives a completely different seed + * — while Python's Bip39SeedGenerator canonicalizes and derives the correct + * one. Validating one string and deriving from another produced a vault + * that neither implementation, nor the user's written-down phrase, could + * ever reopen. + */ +export function canonicalizeMnemonic(mnemonic: string): string { + return mnemonic.normalize("NFKD").trim().toLowerCase().split(/\s+/u).join(" "); +} + /** True when `mnemonic` is a valid English BIP-39 phrase (checksum included). */ export function isValidMnemonic(mnemonic: string): boolean { - return scureValidateMnemonic(mnemonic.trim(), english); + return scureValidateMnemonic(canonicalizeMnemonic(mnemonic), english); } /** @@ -60,7 +75,7 @@ export class Bip85 { } static fromMnemonic(mnemonic: string, passphrase = ""): Bip85 { - return new Bip85(mnemonicToSeedSync(mnemonic, passphrase)); + return new Bip85(mnemonicToSeedSync(canonicalizeMnemonic(mnemonic), passphrase)); } deriveEntropy(opts: DeriveEntropyOptions): Uint8Array { diff --git a/js/packages/core/src/derive/fingerprint.ts b/js/packages/core/src/derive/fingerprint.ts index 5ff676e2..5479f246 100644 --- a/js/packages/core/src/derive/fingerprint.ts +++ b/js/packages/core/src/derive/fingerprint.ts @@ -5,9 +5,12 @@ import { sha256 } from "@noble/hashes/sha2.js"; import { bytesToHex, utf8 } from "../util/bytes.js"; +import { canonicalizeMnemonic } from "./bip85.js"; export function generateFingerprint(seedPhrase: string, length = 16): string { - const normalized = seedPhrase.trim().toLowerCase(); + // Same canonical form as derivation, so the fingerprint identifies the + // vault the seed actually opens. + const normalized = canonicalizeMnemonic(seedPhrase); const digest = sha256(utf8(normalized)); return bytesToHex(digest).slice(0, length).toUpperCase(); } diff --git a/js/packages/core/src/derive/password.ts b/js/packages/core/src/derive/password.ts index 2cdbb902..31041b1a 100644 --- a/js/packages/core/src/derive/password.ts +++ b/js/packages/core/src/derive/password.ts @@ -371,7 +371,13 @@ function generateV2( */ export function passwordPolicyFromRecord(raw: unknown): PasswordPolicy { if (typeof raw !== "object" || raw === null || Array.isArray(raw)) return {}; - const o = raw as Record; + const source = raw as Record; + // A JSON null means "not set". String(null) is "null", which would turn + // the special-character alphabet into the letters n, u, l. + const o: Record = {}; + for (const [k, v] of Object.entries(source)) { + if (v !== null) o[k] = v; + } const num = (v: unknown): number | undefined => v === undefined || v === null ? undefined : Math.trunc(Number(v)); return { diff --git a/js/packages/core/src/derive/totp.ts b/js/packages/core/src/derive/totp.ts index 28079acc..88f9fe49 100644 --- a/js/packages/core/src/derive/totp.ts +++ b/js/packages/core/src/derive/totp.ts @@ -13,13 +13,15 @@ import { base32 } from "@scure/base"; import { mnemonicToSeedSync } from "@scure/bip39"; import { HDKey } from "@scure/bip32"; import { utf8 } from "../util/bytes.js"; +import { canonicalizeMnemonic } from "./bip85.js"; const TOTP_PURPOSE = 39; const TOTP_INT = 0x544f5450; // 1414485072 = int.from_bytes(b"TOTP", "big") /** Derive the base32 TOTP secret for an index from a mnemonic or raw seed. */ export function deriveTotpSecret(seed: string | Uint8Array, index: number): string { - const seedBytes = typeof seed === "string" ? mnemonicToSeedSync(seed) : seed; + const seedBytes = + typeof seed === "string" ? mnemonicToSeedSync(canonicalizeMnemonic(seed)) : seed; const root = HDKey.fromMasterSeed(seedBytes); const path = `m/83696968'/${TOTP_PURPOSE}'/${TOTP_INT}'/${index}'`; const child = root.derive(path); @@ -29,9 +31,23 @@ export function deriveTotpSecret(seed: string | Uint8Array, index: number): stri return base32.encode(hashed.slice(0, 20)); } +/** + * Accept the base32 forms pyotp does. + * + * pyotp calls b32decode(casefold=True) and pads, so lowercase and + * unpadded secrets work there. @scure's decoder rejects both, which would + * make imported entries — including SeedPass's own, since random_totp_secret + * strips padding — unusable here but fine in Python. + */ +function decodeTotpSecret(secretB32: string): Uint8Array { + const cleaned = secretB32.trim().replace(/\s+/g, "").toUpperCase().replace(/=+$/, ""); + const padded = cleaned.padEnd(Math.ceil(cleaned.length / 8) * 8, "="); + return base32.decode(padded); +} + /** RFC 4226 HOTP with SHA-1 (the TOTP default the Python side uses via pyotp). */ export function hotp(secretB32: string, counter: number, digits = 6): string { - const key = base32.decode(secretB32); + const key = decodeTotpSecret(secretB32); const msg = new Uint8Array(8); const view = new DataView(msg.buffer); view.setBigUint64(0, BigInt(counter), false); diff --git a/js/packages/core/src/index.ts b/js/packages/core/src/index.ts index 75df372e..64cc637c 100644 --- a/js/packages/core/src/index.ts +++ b/js/packages/core/src/index.ts @@ -9,6 +9,7 @@ export { Bip85, isValidMnemonic, assertValidMnemonic, + canonicalizeMnemonic, type DeriveEntropyOptions, } from "./derive/bip85.js"; export { @@ -50,6 +51,7 @@ export { deriveKeyFromPassword, deriveKeyFromPasswordArgon2, kdfConfigSchema, + KDF_LIMITS, type KdfConfig, } from "./vault/passwordKdf.js"; export { fernetDecrypt, FernetError } from "./vault/fernet.js"; diff --git a/js/packages/core/src/sync/canonical.ts b/js/packages/core/src/sync/canonical.ts index b80a5072..624fcc1e 100644 --- a/js/packages/core/src/sync/canonical.ts +++ b/js/packages/core/src/sync/canonical.ts @@ -41,13 +41,68 @@ function encodeString(s: string): string { return out + '"'; } +/** + * Numbers, formatted the way CPython's json module would. + * + * JS and Python disagree in several places, and every disagreement changes + * an entry hash — which decides merge winners, so a mismatch means two + * clients can diverge permanently. Handled here: + * + * value JS toString CPython json + * 1e16 10000000000000000 1e+16 + * 1e-7 1e-7 1e-07 + * 1e23 1e+23 1e+23 (agrees) + * -0 0 -0.0 + * + * Integral values are emitted without a decimal point, matching Python ints; + * SeedPass data carries integers (timestamps, lengths, counts) and never + * float-typed whole numbers, so the int/float ambiguity CPython would + * otherwise expose does not arise. A non-integral or non-safe value would + * be ambiguous, so it is rejected rather than guessed at. + */ function encodeNumber(n: number): string { if (!Number.isFinite(n)) throw new Error("non-finite number in canonical JSON"); - if (Number.isInteger(n) && Object.is(n, -0) === false) return n.toString(); - // Python repr(float) and JS toString agree on shortest-roundtrip decimals - // for doubles; integral floats (1.0) cannot be distinguished from ints in - // JSON-parsed data, so both sides serialize them identically. - return n.toString(); + if (Object.is(n, -0)) return "-0.0"; + if (Number.isInteger(n)) { + if (!Number.isSafeInteger(n)) { + throw new Error( + `integer ${n} exceeds the safe range; its canonical form would differ ` + + `from the Python reference and break hash agreement`, + ); + } + return n.toString(); + } + return pythonFloatRepr(n); +} + +/** CPython repr() for a float: shortest round-trip, two-digit exponent. */ +function pythonFloatRepr(n: number): string { + const s = n.toString(); + const m = /^(-?)(\d(?:\.\d+)?)e([+-])(\d+)$/.exec(s); + if (m) { + // JS writes 1e-7; CPython writes 1e-07 (exponent padded to two digits). + const [, sign, mantissa, expSign, expDigits] = m; + return `${sign}${mantissa}e${expSign}${expDigits!.padStart(2, "0")}`; + } + if (Math.abs(n) >= 1e16) { + // CPython switches to exponent form at 1e16; JS does so only at 1e21. + const exp = n.toExponential(); + const em = /^(-?\d(?:\.\d+)?)e([+-])(\d+)$/.exec(exp)!; + return `${em[1]}e${em[2]}${em[3]!.padStart(2, "0")}`; + } + return s; +} + +/** Code-point ordering, matching Python's sorted() on str. */ +function compareCodePoints(a: string, b: string): number { + const ai = Array.from(a); + const bi = Array.from(b); + const len = Math.min(ai.length, bi.length); + for (let i = 0; i < len; i++) { + const d = ai[i]!.codePointAt(0)! - bi[i]!.codePointAt(0)!; + if (d !== 0) return d; + } + return ai.length - bi.length; } export function canonicalJson(value: unknown): string { @@ -67,7 +122,10 @@ export function canonicalJson(value: unknown): string { if (proto !== Object.prototype && proto !== null) { throw new Error("non-plain object in canonical JSON"); } - const keys = Object.keys(value as Record).sort(); + // Python sorts by code point; JS's default sort compares UTF-16 code + // units, which disagree once a supplementary-plane key (an emoji in a + // tag, say) sits alongside one at or above U+E000. + const keys = Object.keys(value as Record).sort(compareCodePoints); const parts = keys.map( (k) => encodeString(k) + ":" + canonicalJson((value as Record)[k]), ); diff --git a/js/packages/core/src/sync/events.ts b/js/packages/core/src/sync/events.ts index e1a6970b..b6fb0d1b 100644 --- a/js/packages/core/src/sync/events.ts +++ b/js/packages/core/src/sync/events.ts @@ -91,7 +91,62 @@ export function signEvent(privateKeyHex: string, unsigned: UnsignedEvent): Nostr } /** Verify an event's id derivation and BIP-340 signature. */ +/** + * Structural validation of an event received from a relay. + * + * The wire is untrusted: `parseRelayMessage` casts whatever JSON arrived, so + * types must be checked before anything reads them. `created_at` in + * particular is interpolated into the id preimage and used for ordering — + * `1e999` parses to Infinity and would sort ahead of every real event + * forever, and a string collapses comparisons to NaN. + */ +export function isWellFormedEvent(event: unknown): event is NostrEvent { + if (typeof event !== "object" || event === null) return false; + const e = event as Record; + if (typeof e["id"] !== "string" || !/^[0-9a-f]{64}$/.test(e["id"])) return false; + if (typeof e["pubkey"] !== "string" || !/^[0-9a-f]{64}$/.test(e["pubkey"])) return false; + if (typeof e["sig"] !== "string" || !/^[0-9a-f]{128}$/.test(e["sig"])) return false; + if (typeof e["content"] !== "string") return false; + if ( + typeof e["created_at"] !== "number" || + !Number.isSafeInteger(e["created_at"]) || + e["created_at"] < 0 + ) { + return false; + } + if (typeof e["kind"] !== "number" || !Number.isSafeInteger(e["kind"])) return false; + const tags = e["tags"]; + if (!Array.isArray(tags)) return false; + return tags.every((t) => Array.isArray(t) && t.every((v) => typeof v === "string")); +} + +/** + * Does an event actually satisfy the filter it was requested with? + * + * A relay is asked for a filter, it is not obliged to honour it. Without + * this check, `fetch` accepts an event signed by any key at all — the + * signature only proves internal consistency, not authorship by the pubkey + * we asked for. + */ +export function eventMatchesFilter(event: NostrEvent, filter: Filter): boolean { + if (filter.ids && !filter.ids.includes(event.id)) return false; + if (filter.authors && !filter.authors.includes(event.pubkey)) return false; + if (filter.kinds && !filter.kinds.includes(event.kind)) return false; + if (filter.since !== undefined && event.created_at < filter.since) return false; + if (filter.until !== undefined && event.created_at > filter.until) return false; + for (const tagFilter of ["#d", "#e"] as const) { + const wanted = filter[tagFilter]; + if (wanted) { + const name = tagFilter[1]!; + const values = event.tags.filter((t) => t[0] === name).map((t) => t[1]); + if (!wanted.some((w) => values.includes(w))) return false; + } + } + return true; +} + export function verifyEvent(event: NostrEvent): boolean { + if (!isWellFormedEvent(event)) return false; const expectedId = computeEventId( event.pubkey, event.created_at, diff --git a/js/packages/core/src/sync/merge.ts b/js/packages/core/src/sync/merge.ts index 579c881f..542a8fc0 100644 --- a/js/packages/core/src/sync/merge.ts +++ b/js/packages/core/src/sync/merge.ts @@ -54,8 +54,21 @@ function entryKind(entry: Dict): string { return String(raw).trim().toLowerCase(); } +/** + * Python truthiness, which differs from JS on containers: bool([]) and + * bool({}) are False in Python but Boolean([]) is true in JS. A + * `"_deleted": []` field would tombstone an entry in JS and keep it in + * Python — the two clients would then disagree about whether it exists. + */ +function pyTruthy(value: unknown): boolean { + if (value === null || value === undefined) return false; + if (Array.isArray(value)) return value.length > 0; + if (typeof value === "object") return Object.keys(value as Dict).length > 0; + return Boolean(value); +} + function isDeletedEntry(entry: Dict): boolean { - return Boolean(entry["_deleted"] ?? false) || Boolean(entry["deleted"] ?? false); + return pyTruthy(entry["_deleted"]) || pyTruthy(entry["deleted"]); } function entryHash(entry: Dict): string { @@ -85,7 +98,10 @@ function preferTombstone(current: Dict, incoming: Dict, idx: string): boolean { } function normalizeTombstones(value: unknown): Record { - const out: Record = {}; + // Object.create(null): "__proto__" must behave as an ordinary key. On a + // normal object it silently replaces the prototype instead of being + // stored, which drops the record here and diverges from Python. + const out: Record = Object.create(null); if (!isDict(value)) return out; for (const [k, v] of Object.entries(value)) { if (!isDict(v)) continue; @@ -105,7 +121,7 @@ function mergeTombstones( current: Record, incoming: Record, ): Record { - const merged: Record = { ...current }; + const merged: Record = Object.assign(Object.create(null), current); for (const [idx, rec] of Object.entries(incoming)) { const cur = merged[idx]; if (cur === undefined || preferTombstone(cur, rec, idx)) { @@ -274,7 +290,13 @@ export function mergeIndexPayloads( options, ); - const curEntries: Dict = isDict(out["entries"]) ? (out["entries"] as Dict) : {}; + // Rebuild entries on a null prototype for the same reason: an incoming + // entry keyed "__proto__" would otherwise be dropped and pollute the + // object's prototype chain. + const curEntries: Dict = Object.assign( + Object.create(null) as Dict, + isDict(out["entries"]) ? (out["entries"] as Dict) : {}, + ); const incEntries: Dict = isDict(incoming["entries"]) ? (incoming["entries"] as Dict) : {}; const curMeta: Dict = isDict(out["_sync_meta"]) ? (out["_sync_meta"] as Dict) : {}; const incMeta: Dict = isDict(incoming["_sync_meta"]) ? (incoming["_sync_meta"] as Dict) : {}; @@ -332,7 +354,9 @@ export function mergeIndexPayloads( } // Apply tombstones after the entry merge - for (const [idx, rec] of Object.entries({ ...tombstones })) { + for (const [idx, rec] of Object.entries( + Object.assign(Object.create(null) as Record, tombstones), + )) { const entry = curEntries[idx]; if (!isDict(entry)) continue; const eTs = entryTs(entry); diff --git a/js/packages/core/src/sync/relay.ts b/js/packages/core/src/sync/relay.ts index 649ab689..f14ab95a 100644 --- a/js/packages/core/src/sync/relay.ts +++ b/js/packages/core/src/sync/relay.ts @@ -9,7 +9,9 @@ */ import { + eventMatchesFilter, eventMessage, + isWellFormedEvent, parseRelayMessage, reqMessage, verifyEvent, @@ -138,7 +140,18 @@ export class RelayPool { const parsed = parseRelayMessage(String(raw.data)); if (parsed.type === "EVENT" && parsed.subscriptionId === subId) { const ev = parsed.event; - if (!byId.has(ev.id) && verifyEvent(ev)) byId.set(ev.id, ev); + // A relay is asked for a filter; it is not obliged to obey + // one. Accept an event only if it is well formed, actually + // satisfies a filter we sent (author, kind, id, time, tags) + // and carries a valid signature over its own id. + if ( + !byId.has(ev.id) && + isWellFormedEvent(ev) && + filters.some((f) => eventMatchesFilter(ev, f)) && + verifyEvent(ev) + ) { + byId.set(ev.id, ev); + } } else if ( (parsed.type === "EOSE" || parsed.type === "CLOSED") && parsed.subscriptionId === subId diff --git a/js/packages/core/src/sync/snapshot.ts b/js/packages/core/src/sync/snapshot.ts index 80c88b96..70a7b1f7 100644 --- a/js/packages/core/src/sync/snapshot.ts +++ b/js/packages/core/src/sync/snapshot.ts @@ -18,15 +18,29 @@ import { hkdf } from "@noble/hashes/hkdf.js"; import { mnemonicToSeedSync } from "@scure/bip39"; import { z } from "zod"; import { bytesToHex, concatBytes, utf8 } from "../util/bytes.js"; +import { canonicalizeMnemonic } from "../derive/bip85.js"; export const KIND_MANIFEST = 30070; export const KIND_SNAPSHOT_CHUNK = 30071; export const KIND_DELTA = 30072; +/** + * Resource budgets for relay-supplied snapshots. + * + * A manifest is signed by the user, but a relay chooses WHICH signed + * manifest to serve, and nothing stops a manifest from describing an + * enormous snapshot. These caps bound the work a relay can make a client do + * before the payload is authenticated by decryption. + */ +export const MAX_CHUNKS = 4096; +export const MAX_CHUNK_BYTES = 1_048_576; // 1 MiB +export const MAX_COMPRESSED_BYTES = 64 * 1_048_576; +export const MAX_DECOMPRESSED_BYTES = 256 * 1_048_576; + export const chunkMetaSchema = z .object({ - id: z.string(), - size: z.number().int().nonnegative(), + id: z.string().max(256), + size: z.number().int().nonnegative().max(MAX_CHUNK_BYTES), hash: z.string().regex(/^[0-9a-f]{64}$/), event_id: z.string().nullable().optional().default(null), }) @@ -36,7 +50,7 @@ export const manifestSchema = z .object({ ver: z.number().int(), algo: z.string(), - chunks: z.array(chunkMetaSchema), + chunks: z.array(chunkMetaSchema).max(MAX_CHUNKS), delta_since: z.number().int().nullable().optional().default(null), nonce: z.string().nullable().optional().default(null), index0: z.record(z.string(), z.unknown()).nullable().optional().default(null), @@ -53,17 +67,32 @@ export function parseManifest(json: string): Manifest { async function pipeThrough( data: Uint8Array, stream: { readable: ReadableStream; writable: WritableStream }, + maxOutputBytes = Number.POSITIVE_INFINITY, ): Promise { const writer = stream.writable.getWriter(); const writeDone = writer.write(data as BufferSource).then(() => writer.close()); const reader = stream.readable.getReader(); const parts: Uint8Array[] = []; - for (;;) { - const { done, value } = await reader.read(); - if (done) break; - parts.push(value as Uint8Array); + let total = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + const chunk = value as Uint8Array; + total += chunk.length; + // Stop a decompression bomb while it is still streaming rather than + // after it has already been materialized. + if (total > maxOutputBytes) { + await reader.cancel(); + throw new Error( + `decompressed output exceeds ${maxOutputBytes} bytes; refusing to continue`, + ); + } + parts.push(chunk); + } + } finally { + await writeDone.catch(() => {}); } - await writeDone; return concatBytes(...parts); } @@ -71,8 +100,11 @@ export async function gzipCompress(data: Uint8Array): Promise { return pipeThrough(data, new CompressionStream("gzip")); } -export async function gzipDecompress(data: Uint8Array): Promise { - return pipeThrough(data, new DecompressionStream("gzip")); +export async function gzipDecompress( + data: Uint8Array, + maxOutputBytes = MAX_DECOMPRESSED_BYTES, +): Promise { + return pipeThrough(data, new DecompressionStream("gzip"), maxOutputBytes); } /** Compress and split the encrypted vault into chunks (prepare_snapshot). */ @@ -120,9 +152,26 @@ export async function reassembleSnapshot( `chunk count mismatch: manifest lists ${manifest.chunks.length}, got ${chunks.length}`, ); } + let compressedTotal = 0; for (let i = 0; i < chunks.length; i++) { const meta = manifest.chunks[i]!; - const actual = bytesToHex(sha256(chunks[i]!)); + const chunk = chunks[i]!; + // The manifest's declared size is signed; a chunk that disagrees with it + // is wrong regardless of what its hash says. + if (chunk.length !== meta.size) { + throw new ChunkVerificationError( + `chunk ${meta.id} size mismatch (manifest says ${meta.size}, got ${chunk.length})`, + meta.id, + ); + } + compressedTotal += chunk.length; + if (compressedTotal > MAX_COMPRESSED_BYTES) { + throw new ChunkVerificationError( + `snapshot exceeds ${MAX_COMPRESSED_BYTES} compressed bytes`, + meta.id, + ); + } + const actual = bytesToHex(sha256(chunk)); if (actual !== meta.hash) { throw new ChunkVerificationError( `chunk ${meta.id} hash mismatch (expected ${meta.hash}, got ${actual})`, @@ -135,7 +184,7 @@ export async function reassembleSnapshot( /** key_index: HKDF chain master -> "seedpass:v1:index" (manager.py KEY_INDEX). */ export function deriveKeyIndex(mnemonic: string): Uint8Array { - const seed = mnemonicToSeedSync(mnemonic); + const seed = mnemonicToSeedSync(canonicalizeMnemonic(mnemonic)); const master = hkdf(sha256, seed, undefined, utf8("seedpass:v1:master"), 32); return hkdf(sha256, master, undefined, utf8("seedpass:v1:index"), 32); } diff --git a/js/packages/core/src/sync/syncFlows.ts b/js/packages/core/src/sync/syncFlows.ts index 2ed37362..c9b21942 100644 --- a/js/packages/core/src/sync/syncFlows.ts +++ b/js/packages/core/src/sync/syncFlows.ts @@ -92,6 +92,12 @@ export interface FetchedSnapshot { manifestEvent: NostrEvent; /** Decompressed, still-encrypted vault payload. */ encrypted: Uint8Array; + /** + * Manifests newer than the one returned that could not be assembled. + * Non-empty means the client was walked backwards — usually because a + * relay withheld a chunk — and the caller should say so out loud. + */ + skippedNewerManifests: string[]; } /** Fetch the newest manifest and all its chunks, verifying every hash. */ @@ -102,20 +108,24 @@ export async function fetchLatestSnapshot( const pubkey = signerPublicKeyHex(privateKeyHex); const manifests = await pool.fetch({ authors: [pubkey], kinds: [KIND_MANIFEST] }); if (manifests.length === 0) return null; - // Newest first. Two snapshots published within the same second tie on - // created_at, so fall back to the order the relay returned them in - // (later = more recently accepted); without this the restore can pick a - // stale manifest after a same-second republish. - const order = new Map(manifests.map((ev, i) => [ev.id, i])); + // Newest first, with a tie-break the relay cannot choose. Arrival order + // is relay-controlled — sorting by it lets a relay decide which of two + // same-second snapshots wins. Event id is a hash of the event's own + // contents, so it gives a stable total order no party can steer. manifests.sort( - (a, b) => b.created_at - a.created_at || (order.get(b.id)! - order.get(a.id)!), + (a, b) => b.created_at - a.created_at || (a.id < b.id ? 1 : a.id > b.id ? -1 : 0), ); + // Falling back to an older manifest is a downgrade: a relay that withholds + // one chunk of the newest snapshot can walk a client backwards through its + // history. Record what was skipped so callers can surface it. + const skipped: string[] = []; for (const manifestEvent of manifests) { let manifest: Manifest; try { manifest = parseManifest(manifestEvent.content); } catch { + skipped.push(manifestEvent.id); continue; } const chunks: Uint8Array[] = []; @@ -125,18 +135,28 @@ export async function fetchLatestSnapshot( ? { ids: [meta.event_id], authors: [pubkey], kinds: [KIND_SNAPSHOT_CHUNK] } : { authors: [pubkey], kinds: [KIND_SNAPSHOT_CHUNK], "#d": [meta.id], limit: 1 }; const events = await pool.fetch(filter); - const match = events.find( - (ev) => bytesToHex(sha256(base64.decode(ev.content))) === meta.hash, - ); + // Decode defensively: base64.decode throws on malformed input, and a + // relay can include any event it likes in the response. An unguarded + // decode here let one extra frame abort the entire restore. + const match = events.find((ev) => { + try { + return bytesToHex(sha256(base64.decode(ev.content))) === meta.hash; + } catch { + return false; + } + }); if (!match) { complete = false; break; } chunks.push(base64.decode(match.content)); } - if (!complete) continue; + if (!complete) { + skipped.push(manifestEvent.id); + continue; + } const encrypted = await reassembleSnapshot(manifest, chunks); - return { manifest, manifestEvent, encrypted }; + return { manifest, manifestEvent, encrypted, skippedNewerManifests: skipped }; } return null; } diff --git a/js/packages/core/src/util/bytes.ts b/js/packages/core/src/util/bytes.ts index 30a024c6..df51b0d0 100644 --- a/js/packages/core/src/util/bytes.ts +++ b/js/packages/core/src/util/bytes.ts @@ -11,6 +11,23 @@ export function hmacSha256Hex(key: Uint8Array, data: Uint8Array): string { return bytesToHex(hmac(sha256, key, data)); } +/** + * Strip the characters Python's str.strip() treats as whitespace. + * + * JS trim() and Python strip() are not the same set: trim() removes U+FEFF + * but leaves U+0085 and U+001C..U+001F, and Python does the reverse. That + * difference changes derived keys and fingerprints between implementations. + */ +const PYTHON_WHITESPACE = "\t\n\v\f\r \u001c\u001d\u001e\u001f\u0085\u00a0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000"; + +export function pythonStrip(value: string): string { + let start = 0; + let end = value.length; + while (start < end && PYTHON_WHITESPACE.includes(value[start]!)) start++; + while (end > start && PYTHON_WHITESPACE.includes(value[end - 1]!)) end--; + return value.slice(start, end); +} + export function utf8(s: string): Uint8Array { return new TextEncoder().encode(s); } @@ -22,12 +39,14 @@ export function bytesToHex(b: Uint8Array): string { } export function hexToBytes(hex: string): Uint8Array { + // parseInt is prefix-lenient and sign-aware: "1z" parses as 1, "-1" as -1 + // (stored as 0xff). Relay-supplied ids and signatures reach this function, + // so validate the whole string first, as Python's bytes.fromhex does. + if (!/^[0-9a-fA-F]*$/.test(hex)) throw new Error("invalid hex"); if (hex.length % 2 !== 0) throw new Error("odd-length hex"); const out = new Uint8Array(hex.length / 2); for (let i = 0; i < out.length; i++) { - const byte = Number.parseInt(hex.slice(i * 2, i * 2 + 2), 16); - if (Number.isNaN(byte)) throw new Error("invalid hex"); - out[i] = byte; + out[i] = Number.parseInt(hex.slice(i * 2, i * 2 + 2), 16); } return out; } diff --git a/js/packages/core/src/vault/entryOps.ts b/js/packages/core/src/vault/entryOps.ts index f87ccd43..cc57e99b 100644 --- a/js/packages/core/src/vault/entryOps.ts +++ b/js/packages/core/src/vault/entryOps.ts @@ -34,9 +34,34 @@ export function isoFromUnix(ts: number): string { return iso.replace(/\.\d{3}Z$/, "+00:00"); } -/** max(int keys) + 1, or 0 for an empty index (get_next_index). */ +/** + * max(int keys) + 1, or 0 for an empty index (get_next_index). + * + * Every key must be a plain non-negative integer within the safe range. A + * non-numeric key makes Math.max return NaN and a huge key makes +1 a no-op, + * and `insert` would then assign the same id repeatedly — each new entry + * silently overwriting the last. Such an index can arrive through import or + * a merge, so refuse it loudly rather than destroying entries. + */ export function nextIndex(index: VaultIndex): number { - const ids = Object.keys(index.entries).map((k) => parseInt(k, 10)); + const keys = Object.keys(index.entries); + const ids: number[] = []; + for (const key of keys) { + if (!/^(0|[1-9][0-9]*)$/.test(key)) { + throw new Error( + `vault index contains a non-numeric entry id ${JSON.stringify(key)}; ` + + `refusing to allocate a new id that could overwrite an existing entry`, + ); + } + const id = Number(key); + if (!Number.isSafeInteger(id)) { + throw new Error( + `vault index contains entry id ${key}, which is outside the safe integer ` + + `range; refusing to allocate a new id`, + ); + } + ids.push(id); + } return ids.length > 0 ? Math.max(...ids) + 1 : 0; } @@ -54,8 +79,13 @@ function stamp(clock: Clock): { now_unix: number; now_iso: string } { } function insert(index: VaultIndex, id: number, entry: Dict): string { - (index.entries as unknown as Dict)[String(id)] = entry; - return String(id); + const key = String(id); + // Never silently replace an entry: allocation bugs show up here first. + if (Object.prototype.hasOwnProperty.call(index.entries, key)) { + throw new Error(`refusing to overwrite existing entry ${key}`); + } + (index.entries as unknown as Dict)[key] = entry; + return key; } export interface CommonOptions { diff --git a/js/packages/core/src/vault/indexKey.ts b/js/packages/core/src/vault/indexKey.ts index b3a6b914..5f36497a 100644 --- a/js/packages/core/src/vault/indexKey.ts +++ b/js/packages/core/src/vault/indexKey.ts @@ -11,6 +11,7 @@ import { sha256 } from "@noble/hashes/sha2.js"; import { base64url } from "@scure/base"; import { mnemonicToSeedSync } from "@scure/bip39"; import { utf8 } from "../util/bytes.js"; +import { canonicalizeMnemonic } from "../derive/bip85.js"; function kd(root: Uint8Array, info: Uint8Array, length = 32): Uint8Array { return hkdf(sha256, root, undefined, info, length); @@ -18,7 +19,9 @@ function kd(root: Uint8Array, info: Uint8Array, length = 32): Uint8Array { /** Raw 32-byte index encryption key. */ export function deriveIndexKeyBytes(mnemonic: string): Uint8Array { - const seed = mnemonicToSeedSync(mnemonic); + // Canonicalize first: deriving from a raw string that merely passed a + // word-count check yields a key nothing else can reproduce. + const seed = mnemonicToSeedSync(canonicalizeMnemonic(mnemonic)); const master = kd(seed, utf8("seedpass:v1:master")); return kd(master, utf8("seedpass:v1:storage")); } diff --git a/js/packages/core/src/vault/passwordKdf.ts b/js/packages/core/src/vault/passwordKdf.ts index 773e15b8..f467577f 100644 --- a/js/packages/core/src/vault/passwordKdf.ts +++ b/js/packages/core/src/vault/passwordKdf.ts @@ -11,7 +11,7 @@ import { pbkdf2 } from "@noble/hashes/pbkdf2.js"; import { argon2id } from "@noble/hashes/argon2.js"; import { sha256 } from "@noble/hashes/sha2.js"; import { base64, base64url } from "@scure/base"; -import { utf8 } from "../util/bytes.js"; +import { pythonStrip, utf8 } from "../util/bytes.js"; import { z } from "zod"; export const kdfConfigSchema = z @@ -28,7 +28,38 @@ export type KdfConfig = z.infer; function normalizePassword(password: string): Uint8Array { if (!password) throw new Error("Password cannot be empty."); - return utf8(password.normalize("NFKD").trim()); + // Python strips a different whitespace set than JS trim(); using trim() + // here made JS accept passwords Python would reject and derive different + // keys for others. + return utf8(pythonStrip(password.normalize("NFKD"))); +} + +/** + * Bounds for KDF parameters read from a vault file. + * + * The kdf block sits outside the AEAD in the parent-seed wrapper, so anyone + * who can write the profile directory chooses these numbers. Unbounded, they + * are a hang or a multi-gigabyte allocation — and this core also runs in a + * browser tab. + */ +export const KDF_LIMITS = { + maxTimeCost: 10, + maxMemoryCost: 1_048_576, // KiB => 1 GiB + maxParallelism: 16, + maxIterations: 10_000_000, +} as const; + +function boundedParam(value: unknown, fallback: number, max: number, name: string): number { + const n = value === undefined || value === null ? fallback : Number(value); + if (!Number.isFinite(n) || !Number.isInteger(n) || n < 1) { + throw new Error(`KDF parameter ${name} must be a positive integer (got ${String(value)})`); + } + if (n > max) { + throw new Error( + `KDF parameter ${name} is ${n}, above the maximum ${max}; refusing to run it`, + ); + } + return n; } /** @@ -43,7 +74,7 @@ export function deriveKeyFromPassword( const salt = typeof fingerprint === "string" ? sha256(utf8(fingerprint)).slice(0, 16) : fingerprint; const key = pbkdf2(sha256, normalizePassword(password), salt, { - c: iterations, + c: boundedParam(iterations, 100_000, KDF_LIMITS.maxIterations, "iterations"), dkLen: 32, }); return base64url.encode(key); @@ -58,9 +89,9 @@ export function deriveKeyFromPasswordArgon2(password: string, kdf: KdfConfig): s }; const salt = base64.decode(kdf.salt_b64); const key = argon2id(normalizePassword(password), salt, { - t: params.time_cost ?? 2, - m: params.memory_cost ?? 64 * 1024, - p: params.parallelism ?? 8, + t: boundedParam(params.time_cost, 2, KDF_LIMITS.maxTimeCost, "time_cost"), + m: boundedParam(params.memory_cost, 64 * 1024, KDF_LIMITS.maxMemoryCost, "memory_cost"), + p: boundedParam(params.parallelism, 8, KDF_LIMITS.maxParallelism, "parallelism"), dkLen: 32, }); return base64url.encode(key); diff --git a/js/packages/core/test/canonical.test.ts b/js/packages/core/test/canonical.test.ts new file mode 100644 index 00000000..d23bc931 --- /dev/null +++ b/js/packages/core/test/canonical.test.ts @@ -0,0 +1,43 @@ +/** + * canonicalJson must agree with CPython's json.dumps byte-for-byte. + * + * Entry and tombstone hashes are computed over this output and decide merge + * winners, so any disagreement lets two clients keep different versions of + * the same entry and never converge. A security review found real + * divergences here (exponent padding, code-unit vs code-point key order) + * that the older fixtures missed by only ever using ASCII keys and integers. + */ + +import { describe, expect, it } from "vitest"; +import { canonicalJsonCases, canonicalJsonUnsupported } from "@seedpass/test-vectors"; +import { canonicalJson, canonicalHash } from "@seedpass/core"; + +describe("canonical JSON parity with Python", () => { + it.each(canonicalJsonCases.map((c, i) => ({ ...c, i })))( + "case $i matches json.dumps", + ({ value, canonical }) => { + expect(canonicalJson(value)).toBe(canonical); + }, + ); + + it("orders keys by code point, not UTF-16 code unit", () => { + // U+FF01 sorts before U+1F600 by code point; the default JS sort, which + // compares code units, puts the surrogate pair first. + const out = canonicalJson({ "\u{1F600}": 1, "!": 2 }); + expect(out.indexOf("uff01")).toBeLessThan(out.indexOf("ud83d")); + }); + + it("refuses values whose canonical form would be ambiguous", () => { + // Above 2**53 JSON cannot distinguish a Python int from a float, so the + // two implementations cannot be made to agree; refuse rather than guess. + expect(() => canonicalJson({ n: Number.MAX_SAFE_INTEGER + 2 })).toThrow(/safe range/); + expect(() => canonicalJson({ n: Infinity })).toThrow(/non-finite/); + for (const value of canonicalJsonUnsupported) { + expect(() => canonicalJson(value)).toThrow(); + } + }); + + it("hashes are stable for the same logical value", () => { + expect(canonicalHash({ a: 1, b: 2 })).toBe(canonicalHash({ b: 2, a: 1 })); + }); +}); diff --git a/js/packages/test-vectors/fixtures/canonical_json.json b/js/packages/test-vectors/fixtures/canonical_json.json new file mode 100644 index 00000000..5bf27370 --- /dev/null +++ b/js/packages/test-vectors/fixtures/canonical_json.json @@ -0,0 +1,198 @@ +{ + "cases": [ + { + "canonical": "{}", + "value": {} + }, + { + "canonical": "{\"a\":1}", + "value": { + "a": 1 + } + }, + { + "canonical": "{\"a\":1,\"b\":2}", + "value": { + "a": 1, + "b": 2 + } + }, + { + "canonical": "{\"label\":\"site\",\"modified_ts\":1700000000}", + "value": { + "label": "site", + "modified_ts": 1700000000 + } + }, + { + "canonical": "{\"notes\":\"\",\"tags\":[\"b\",\"a\"]}", + "value": { + "notes": "", + "tags": [ + "b", + "a" + ] + } + }, + { + "canonical": "{\"\\uff01\":2,\"\\ud83d\\ude00\":1}", + "value": { + "\uff01": 2, + "\ud83d\ude00": 1 + } + }, + { + "canonical": "{\"z\":1,\"\\u00e9\":2,\"\\u4e2d\":3,\"\\ud83d\\ude00\":4}", + "value": { + "z": 1, + "\u00e9": 2, + "\u4e2d": 3, + "\ud83d\ude00": 4 + } + }, + { + "canonical": "{\"quote\\\"\":\"back\\\\slash\"}", + "value": { + "quote\"": "back\\slash" + } + }, + { + "canonical": "{\"ctrl\":\"\\u0000\\u0001\\u001f\"}", + "value": { + "ctrl": "\u0000\u0001\u001f" + } + }, + { + "canonical": "{\"nl\":\"line\\nbreak\\ttab\"}", + "value": { + "nl": "line\nbreak\ttab" + } + }, + { + "canonical": "{\"unicode\":\"caf\\u00e9 \\u4e2d\\u6587 \\ud83d\\udd12\"}", + "value": { + "unicode": "caf\u00e9 \u4e2d\u6587 \ud83d\udd12" + } + }, + { + "canonical": "{\"n\":0}", + "value": { + "n": 0 + } + }, + { + "canonical": "{\"n\":0}", + "value": { + "n": 0 + } + }, + { + "canonical": "{\"n\":1}", + "value": { + "n": 1 + } + }, + { + "canonical": "{\"n\":-17}", + "value": { + "n": -17 + } + }, + { + "canonical": "{\"n\":9007199254740991}", + "value": { + "n": 9007199254740991 + } + }, + { + "canonical": "{\"n\":0.5}", + "value": { + "n": 0.5 + } + }, + { + "canonical": "{\"n\":-0.25}", + "value": { + "n": -0.25 + } + }, + { + "canonical": "{\"n\":1e-07}", + "value": { + "n": 1e-07 + } + }, + { + "canonical": "{\"n\":1.5e-09}", + "value": { + "n": 1.5e-09 + } + }, + { + "canonical": "{\"links\":[{\"note\":\"\",\"relation\":\"totp\",\"target_id\":1}]}", + "value": { + "links": [ + { + "note": "", + "relation": "totp", + "target_id": 1 + } + ] + } + }, + { + "canonical": "[1,\"two\",null,true,false]", + "value": [ + 1, + "two", + null, + true, + false + ] + }, + { + "canonical": "{\"deep\":{\"a\":[{\"b\":{\"c\":[1,2,3]}}]}}", + "value": { + "deep": { + "a": [ + { + "b": { + "c": [ + 1, + 2, + 3 + ] + } + } + ] + } + } + }, + { + "canonical": "{\"archived\":false,\"custom_fields\":[],\"kind\":\"password\",\"label\":\"example.com\",\"length\":16,\"links\":[],\"modified_ts\":1700000000,\"notes\":\"\",\"tags\":[\"web\"],\"type\":\"password\"}", + "value": { + "archived": false, + "custom_fields": [], + "kind": "password", + "label": "example.com", + "length": 16, + "links": [], + "modified_ts": 1700000000, + "notes": "", + "tags": [ + "web" + ], + "type": "password" + } + } + ], + "description": "canonicalJson must match json.dumps(sort_keys=True, separators=(',',':')) exactly: entry hashes derived from it decide merge winners across implementations.", + "unsupported": [ + { + "n": 100000000000000000000 + }, + { + "n": 12345678901234567890 + } + ] +} diff --git a/js/packages/test-vectors/src/index.ts b/js/packages/test-vectors/src/index.ts index b2c16ecc..d9cdfac6 100644 --- a/js/packages/test-vectors/src/index.ts +++ b/js/packages/test-vectors/src/index.ts @@ -30,6 +30,7 @@ import entrySecretsJson from "../fixtures/entry_secrets.json"; import nostrEventsJson from "../fixtures/nostr_events.json"; import entryModsJson from "../fixtures/entry_mods.json"; import migrationsJson from "../fixtures/migrations.json"; +import canonicalJsonJson from "../fixtures/canonical_json.json"; import sshKeysJson from "../fixtures/ssh_keys.json"; import pgpKeysJson from "../fixtures/pgp_keys.json"; @@ -235,6 +236,16 @@ export const nostrEvents = nostrEventsJson as { events: { name: string; event: SignedNostrEvent }[]; }; +export interface CanonicalJsonCase { + value: unknown; + canonical: string; +} + +export const canonicalJsonCases = (canonicalJsonJson as { cases: CanonicalJsonCase[] }) + .cases; +export const canonicalJsonUnsupported = (canonicalJsonJson as { unsupported: unknown[] }) + .unsupported; + export interface SshKeyCase { mnemonic_id: string; index: number; diff --git a/scripts/canonical_json_cases.py b/scripts/canonical_json_cases.py new file mode 100644 index 00000000..21e50ea7 --- /dev/null +++ b/scripts/canonical_json_cases.py @@ -0,0 +1,102 @@ +#!/usr/bin/env python3 +"""Emit canonical-JSON differential cases from the Python reference. + +Entry hashes decide merge winners, so if the two implementations serialize +the same value differently they can pick different winners and never +converge. A security review found several such disagreements (number +formatting, key ordering) that the existing fixtures missed because they +only ever used ASCII keys and integer values. + +Writes js/packages/test-vectors/fixtures/canonical_json.json. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +OUT = REPO / "js" / "packages" / "test-vectors" / "fixtures" / "canonical_json.json" + +# Values a vault can realistically contain, plus the edges that broke. +CASES: list = [ + {}, + {"a": 1}, + {"b": 2, "a": 1}, + {"modified_ts": 1700000000, "label": "site"}, + {"tags": ["b", "a"], "notes": ""}, + # Key ordering: supplementary plane vs BMP above U+E000 + {"\U0001f600": 1, "!": 2}, + {"z": 1, "é": 2, "中": 3, "\U0001f600": 4}, + # Escaping + {"quote\"": "back\\slash"}, + {"ctrl": "\x00\x01\x1f"}, + {"nl": "line\nbreak\ttab"}, + {"unicode": "café 中文 \U0001f512"}, + # Numbers + {"n": 0}, + {"n": -0}, + {"n": 1}, + {"n": -17}, + {"n": 9007199254740991}, + {"n": 0.5}, + {"n": -0.25}, + {"n": 1e-7}, + {"n": 1.5e-9}, + # Values above 2**53 are deliberately absent: JSON gives no way to tell a + # Python int from a float there, so the two implementations cannot agree + # on a canonical form. canonicalJson refuses them rather than guessing; + # see the "unsupported" list below. + # Nesting and arrays + {"links": [{"target_id": 1, "relation": "totp", "note": ""}]}, + [1, "two", None, True, False], + {"deep": {"a": [{"b": {"c": [1, 2, 3]}}]}}, + # Entry-shaped, the actual hashed object + { + "type": "password", + "kind": "password", + "label": "example.com", + "length": 16, + "archived": False, + "notes": "", + "tags": ["web"], + "modified_ts": 1700000000, + "links": [], + "custom_fields": [], + }, +] + + +# Values TypeScript must refuse rather than serialize ambiguously. +UNSUPPORTED = [ + {"n": 10**20}, + {"n": 12345678901234567890}, +] + + +def main() -> None: + cases = [] + for value in CASES: + canonical = json.dumps(value, sort_keys=True, separators=(",", ":")) + cases.append({"value": value, "canonical": canonical}) + OUT.write_text( + json.dumps( + { + "description": ( + "canonicalJson must match json.dumps(sort_keys=True, " + "separators=(',',':')) exactly: entry hashes derived from " + "it decide merge winners across implementations." + ), + "cases": cases, + "unsupported": UNSUPPORTED, + }, + indent=2, + sort_keys=True, + ) + + "\n" + ) + print(f"wrote {OUT.relative_to(REPO)} ({len(cases)} cases)") + + +if __name__ == "__main__": + main()