diff --git a/crates/relicario-core/src/recovery_qr.rs b/crates/relicario-core/src/recovery_qr.rs index 1b5bdbc..e0cadde 100644 --- a/crates/relicario-core/src/recovery_qr.rs +++ b/crates/relicario-core/src/recovery_qr.rs @@ -1,13 +1,107 @@ +//! Recovery-QR encoding for the reference image_secret. +//! +//! ## What this module produces +//! +//! Given a user-chosen recovery passphrase and the 32-byte image_secret +//! (extracted from the reference JPEG via [`crate::imgsecret::extract`]), this +//! module produces a 109-byte sealed payload that — at recovery time, with the +//! same passphrase — yields the original image_secret back. The payload is +//! intended to be rendered as a QR v40 EcLevel::M SVG via [`recovery_qr_to_svg`] +//! and printed on paper, so a user who loses access to the reference JPEG can +//! still unlock their vault if they remember the recovery passphrase. +//! +//! ## Why the format is structured this way +//! +//! The payload is an XChaCha20-Poly1305 envelope around the image_secret. The +//! AEAD key (the "wrap key") is derived by Argon2id from a domain-separated +//! input: +//! +//! ```text +//! kdf_input = b"relicario-recovery-v1\0" +//! || u64_be(len(nfc(passphrase))) +//! || nfc(passphrase) +//! wrap_key = Argon2id(kdf_input, kdf_salt, RECOVERY_PRODUCTION_PARAMS) -> 32 bytes +//! ``` +//! +//! The `b"relicario-recovery-v1\0"` prefix is **domain separation**: it +//! guarantees that even if the user reuses their vault passphrase as their +//! recovery passphrase, the wrap key derived here can never collide with a +//! vault master key derived in [`crate::crypto::derive_master_key`] (which has +//! a different input shape entirely — passphrase + image_secret, no prefix). +//! Without this prefix, a determined attacker who somehow recovered a wrap key +//! could try it as a master key and vice versa. +//! +//! Both `kdf_salt` and `wrap_nonce` are freshly randomized per call to +//! [`generate_recovery_qr`], so two QRs printed from the same passphrase and +//! image_secret are different bytes — the printed QR does not leak whether +//! the user has printed others before. +//! +//! ## Parameter-pinning rationale +//! +//! The Argon2id parameters used here are NOT [`crate::crypto::KdfParams::default`]. +//! They are pinned in `RECOVERY_PRODUCTION_PARAMS` at the value +//! `KdfParams { argon2_m: 65536, argon2_t: 3, argon2_p: 4 }` — the same values +//! the default happens to have *today*, but deliberately re-stated rather than +//! referenced. This is because `KdfParams::default()` may evolve as we re-tune +//! Argon2 cost for newer hardware, and a recovery QR printed on paper has no +//! way to negotiate parameters at decode time. Changing the pinned values here +//! would silently invalidate every recovery QR a user has ever printed under +//! the previous parameter set. The const lives at module scope so the +//! "pinned, do not change once shipped" property is visible at every use site. + use chacha20poly1305::{XChaCha20Poly1305, Key, KeyInit, aead::Aead}; use rand::RngCore; use unicode_normalization::UnicodeNormalization; use zeroize::Zeroizing; use crate::{crypto::KdfParams, error::{RelicarioError, Result}}; +// Recovery QR payload — 109 bytes total: +// +// byte field length +// ------ -------------- ------ +// 0..4 MAGIC = "RREC" 4 +// 4..5 VERSION = 0x01 1 +// 5..37 kdf_salt 32 (random per QR) +// 37..61 wrap_nonce 24 (random per QR) +// 61..109 ciphertext 48 (32 image_secret + 16 AEAD tag) +// ------------------------------ +// total 109 const MAGIC: &[u8; 4] = b"RREC"; const VERSION: u8 = 0x01; const PAYLOAD_LEN: usize = 4 + 1 + 32 + 24 + 48; // 109 +// Static assertion that the documented layout above and the PAYLOAD_LEN +// constant cannot drift apart. If a future edit changes one without the other, +// this fails to compile. +const _: () = assert!(PAYLOAD_LEN == 4 + 1 + 32 + 24 + 48); + +// Named slice ranges derived from the layout offsets above. Used by +// `unwrap_recovery_qr_with_params` so the byte-position arithmetic at the +// parse site is self-documenting. +const KDF_SALT_RANGE: std::ops::Range = 5..37; +const WRAP_NONCE_RANGE: std::ops::Range = 37..61; +const CIPHERTEXT_RANGE: std::ops::Range = 61..109; + +/// Pinned recovery-QR Argon2id parameters. Re-states `KdfParams::default()`'s +/// values rather than referencing them, because a recovery QR printed under +/// one parameter set cannot be decoded under another. **Once shipped, these +/// values MUST NOT change** — doing so silently invalidates every previously +/// printed QR. See the module header for full rationale. +const RECOVERY_PRODUCTION_PARAMS: KdfParams = KdfParams { + argon2_m: 65536, + argon2_t: 3, + argon2_p: 4, +}; + +/// A sealed 109-byte recovery payload. The bytes are an opaque package — they +/// only become useful when fed back through [`unwrap_recovery_qr`] together +/// with the recovery passphrase that was used to produce them. +/// +/// [`as_bytes`](Self::as_bytes) is the only accessor. The bytes are designed to +/// travel as a single unit; the supported transport is rendering via +/// [`recovery_qr_to_svg`] and printing the QR on paper, but a hex string +/// (sneakernet-friendly) works equally well as long as the full 109 bytes +/// are preserved. pub struct RecoveryQrPayload { bytes: [u8; PAYLOAD_LEN], } @@ -24,15 +118,12 @@ fn recovery_kdf_input(passphrase: &str) -> Vec { let prefix = b"relicario-recovery-v1\0"; let mut input = Vec::with_capacity(prefix.len() + 8 + nfc_bytes.len()); input.extend_from_slice(prefix); + // length-prefix on nfc_bytes mirrors crypto::derive_master_key (audit H1) input.extend_from_slice(&(nfc_bytes.len() as u64).to_be_bytes()); input.extend_from_slice(nfc_bytes); input } -fn production_params() -> KdfParams { - KdfParams { argon2_m: 65536, argon2_t: 3, argon2_p: 4 } -} - fn derive_wrap_key( passphrase: &str, kdf_salt: &[u8; 32], @@ -42,11 +133,38 @@ fn derive_wrap_key( crate::crypto::derive_master_key_raw(&input, kdf_salt, params) } +/// Produce a sealed [`RecoveryQrPayload`] from the recovery passphrase and the +/// 32-byte image_secret. +/// +/// # Inputs +/// +/// - `passphrase`: the user's recovery passphrase (UTF-8). Independent of the +/// vault passphrase, but the user may reuse them — the +/// `b"relicario-recovery-v1\0"` domain-separation prefix in the KDF input +/// guarantees the wrap key still cannot collide with a vault master key. +/// - `image_secret`: the 32-byte secret extracted from the reference JPEG +/// via [`crate::imgsecret::extract`]. +/// +/// # Output +/// +/// A [`RecoveryQrPayload`] whose 109 bytes encode `MAGIC || VERSION || kdf_salt +/// || wrap_nonce || ciphertext`. Both `kdf_salt` and `wrap_nonce` are freshly +/// drawn from `OsRng` on every call, so two payloads generated from the same +/// `(passphrase, image_secret)` pair are distinct bit-for-bit. The printed QR +/// therefore does not reveal that the user has printed others before. +/// +/// To render the payload as a printable SVG, see [`recovery_qr_to_svg`]. +/// +/// # Errors +/// +/// Returns [`RelicarioError::RecoveryQr`] if the AEAD wrap fails (extremely +/// unlikely in practice — this can only happen if the cipher implementation +/// itself errors, not on user input). pub fn generate_recovery_qr( passphrase: &str, image_secret: &[u8; 32], ) -> Result { - generate_recovery_qr_with_params(passphrase, image_secret, &production_params()) + generate_recovery_qr_with_params(passphrase, image_secret, &RECOVERY_PRODUCTION_PARAMS) } #[doc(hidden)] @@ -78,11 +196,39 @@ pub fn generate_recovery_qr_with_params( Ok(RecoveryQrPayload { bytes }) } +/// Decode a recovery payload back into the original 32-byte image_secret. +/// +/// # Inputs +/// +/// - `payload_bytes`: the 109 bytes produced by [`generate_recovery_qr`] (after +/// the QR has been scanned, or the hex transcribed and decoded). +/// - `passphrase`: the recovery passphrase that was used at generate time. +/// +/// # Output +/// +/// The recovered image_secret as `Zeroizing<[u8; 32]>` — the wrapper ensures +/// the secret is wiped from memory when the binding goes out of scope, so a +/// caller that immediately feeds it into [`crate::crypto::derive_master_key`] +/// and then drops it never leaves a copy in process memory longer than +/// strictly necessary. +/// +/// # Errors +/// +/// - [`RelicarioError::RecoveryQr`] for **format** problems: wrong length, +/// bad magic, unsupported version byte. These come from inspecting the +/// bytes themselves, before any cryptographic work, so they leak nothing +/// about whether the passphrase is right. +/// - [`RelicarioError::Decrypt`] for **AEAD** failure — wrong passphrase +/// (wrong wrap key) **or** a payload tampered after the fact. The two +/// cases are deliberately not distinguished, mirroring the same +/// non-distinguishing rejection as [`crate::crypto::decrypt`] (audit M4): +/// a Poly1305 tag failure cannot, in principle, leak which bytes were +/// wrong, and the API surface preserves that property. pub fn unwrap_recovery_qr( payload_bytes: &[u8], passphrase: &str, ) -> Result> { - unwrap_recovery_qr_with_params(payload_bytes, passphrase, &production_params()) + unwrap_recovery_qr_with_params(payload_bytes, passphrase, &RECOVERY_PRODUCTION_PARAMS) } #[doc(hidden)] @@ -104,9 +250,9 @@ pub fn unwrap_recovery_qr_with_params( format!("unsupported version 0x{:02x}", payload_bytes[4]) )); } - let kdf_salt: &[u8; 32] = payload_bytes[5..37].try_into().expect("slice length validated above"); - let wrap_nonce = &payload_bytes[37..61]; - let ciphertext = &payload_bytes[61..109]; + let kdf_salt: &[u8; 32] = payload_bytes[KDF_SALT_RANGE].try_into().expect("slice length validated above"); + let wrap_nonce = &payload_bytes[WRAP_NONCE_RANGE]; + let ciphertext = &payload_bytes[CIPHERTEXT_RANGE]; let wrap_key = derive_wrap_key(passphrase, kdf_salt, params)?; let cipher = XChaCha20Poly1305::new(Key::from_slice(wrap_key.as_ref())); @@ -119,6 +265,15 @@ pub fn unwrap_recovery_qr_with_params( Ok(out) } +/// Render a [`RecoveryQrPayload`] as a printable QR-code SVG string. +/// +/// The QR is encoded at **version 40** (the largest standard symbol, 177×177 +/// modules) at **error-correction level M** (~15% recoverable), with a +/// minimum rendered dimension of **140×140** SVG units. The 109-byte payload +/// fits comfortably inside v40 at level M — there is significant +/// error-correction headroom left over, which is the point: the QR is +/// expected to live on paper (where smudges, folds, and fading are normal) +/// and must still scan years later. pub fn recovery_qr_to_svg(payload: &RecoveryQrPayload) -> String { use qrcode::{QrCode, EcLevel}; let code = QrCode::with_error_correction_level(payload.bytes.as_ref(), EcLevel::M) diff --git a/crates/relicario-wasm/src/lib.rs b/crates/relicario-wasm/src/lib.rs index 5ddb7be..66825e7 100644 --- a/crates/relicario-wasm/src/lib.rs +++ b/crates/relicario-wasm/src/lib.rs @@ -12,7 +12,13 @@ use zeroize::Zeroizing; use relicario_core::{derive_master_key, imgsecret, KdfParams}; -/// Handle type returned from `unlock`. Backed by a `u32`; opaque to JS. +/// Handle returned from `unlock`. Backed by a `u32`; opaque to JS. +/// +/// Dropping the handle (or invoking `.free()` from JS) removes the entry from +/// the session registry, zeroizing the wrapped master key and image_secret. +/// `lock(handle)` remains available as the explicit early-cleanup path; the +/// `Drop` impl is the safety net that catches code paths which forget to call +/// `lock` before letting the handle go out of scope. #[wasm_bindgen] pub struct SessionHandle(u32); @@ -22,6 +28,23 @@ impl SessionHandle { pub fn value(&self) -> u32 { self.0 } } +impl Drop for SessionHandle { + fn drop(&mut self) { let _ = session::remove(self.0); } +} + +#[doc(hidden)] +pub fn __test_make_handle() -> SessionHandle { + SessionHandle(session::insert( + Zeroizing::new([0x77u8; 32]), + Zeroizing::new([0u8; 32]), + )) +} + +#[doc(hidden)] +pub fn __test_session_exists(handle: u32) -> bool { + session::with(handle, |_| ()).is_some() +} + #[wasm_bindgen] pub fn unlock( passphrase: &str, @@ -533,6 +556,19 @@ mod session_tests { assert!(!session::remove(h)); // second remove false } + #[test] + fn dropping_session_handle_clears_registry_entry() { + session::clear(); + let handle = SessionHandle(session::insert( + Zeroizing::new([0x33u8; 32]), + Zeroizing::new([0u8; 32]), + )); + let id = handle.value(); + assert!(session::with(id, |_| ()).is_some()); + drop(handle); + assert!(session::with(id, |_| ()).is_none()); + } + #[test] fn with_yields_key_only_while_session_lives() { session::clear(); diff --git a/crates/relicario-wasm/src/session.rs b/crates/relicario-wasm/src/session.rs index ae084ba..ace135f 100644 --- a/crates/relicario-wasm/src/session.rs +++ b/crates/relicario-wasm/src/session.rs @@ -46,6 +46,9 @@ where SESSIONS.with(|s| s.borrow().get(&handle).map(|d| f(&d.image_secret))) } +/// Remove a session entry. Called by both `lock(handle)` (the explicit +/// path) and `impl Drop for SessionHandle` (the safety net). Returns +/// `true` if an entry was removed, `false` if the handle was already gone. pub fn remove(handle: u32) -> bool { SESSIONS.with(|s| s.borrow_mut().remove(&handle).is_some()) } diff --git a/crates/relicario-wasm/tests/session_drop.rs b/crates/relicario-wasm/tests/session_drop.rs new file mode 100644 index 0000000..cae25f0 --- /dev/null +++ b/crates/relicario-wasm/tests/session_drop.rs @@ -0,0 +1,16 @@ +//! Belt-and-suspenders companion to the native `dropping_session_handle_clears_registry_entry` +//! test in `lib.rs`. This file exists for `wasm-pack test --node` symmetry; the +//! native test in the same crate is what gates CI. + +use wasm_bindgen_test::wasm_bindgen_test; + +use relicario_wasm::{__test_make_handle, __test_session_exists}; + +#[wasm_bindgen_test] +fn dropping_session_handle_clears_registry_entry() { + let handle = __test_make_handle(); + let id = handle.value(); + assert!(__test_session_exists(id)); + drop(handle); + assert!(!__test_session_exists(id)); +} diff --git a/extension/src/service-worker/__tests__/session.test.ts b/extension/src/service-worker/__tests__/session.test.ts new file mode 100644 index 0000000..e83f5d4 --- /dev/null +++ b/extension/src/service-worker/__tests__/session.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest'; +import type { SessionHandle } from '../../../wasm/relicario_wasm'; +import { clearCurrent, getCurrent, setCurrent } from '../session'; + +describe('session', () => { + beforeEach(() => { + // Reset module-scope `current` between tests. Overwrite first with a + // benign no-throw fake so a prior test's throwing handle can't escape. + setCurrent({ free: vi.fn(), value: 0 } as unknown as SessionHandle); + clearCurrent(); + }); + + it('clearCurrent() is a no-op when no handle is set', () => { + expect(() => clearCurrent()).not.toThrow(); + expect(getCurrent()).toBeNull(); + }); + + it('clearCurrent() calls free() exactly once and clears current', () => { + const free = vi.fn(); + setCurrent({ free, value: 1 } as unknown as SessionHandle); + + clearCurrent(); + expect(free).toHaveBeenCalledTimes(1); + expect(getCurrent()).toBeNull(); + + clearCurrent(); + expect(free).toHaveBeenCalledTimes(1); + expect(getCurrent()).toBeNull(); + }); + + it('clearCurrent() propagates exceptions from free()', () => { + const free = vi.fn(() => { throw new Error('boom'); }); + setCurrent({ free, value: 2 } as unknown as SessionHandle); + + expect(() => clearCurrent()).toThrow(/boom/); + }); +}); diff --git a/extension/src/service-worker/session.ts b/extension/src/service-worker/session.ts index 89d0992..4859646 100644 --- a/extension/src/service-worker/session.ts +++ b/extension/src/service-worker/session.ts @@ -7,6 +7,12 @@ /// Future multi-vault (β+) would replace `current` with /// `Map` and thread `vaultId` through every /// handler. Deliberate α simplicity — not an oversight. +/// +/// As of Phase 1 of the security-polish series, `impl Drop for SessionHandle` +/// on the Rust side makes `.free()` the meaningful cleanup call: it removes +/// the entry from the SESSIONS registry and zeroizes `master_key` and +/// `image_secret`. Calling `wasm.lock(handle.value)` before `.free()` would +/// be redundant belt-and-suspenders; this module intentionally does not. import type { SessionHandle } from '../../wasm/relicario_wasm'; @@ -23,6 +29,6 @@ export function requireCurrent(): SessionHandle { export function clearCurrent(): void { if (!current) return; - try { current.free(); } catch { /* already freed */ } + current.free(); current = null; } diff --git a/tools/relay/start.sh b/tools/relay/start.sh index 2682ab2..80e1198 100755 --- a/tools/relay/start.sh +++ b/tools/relay/start.sh @@ -31,6 +31,7 @@ COORD_DIR="$REPO_ROOT/docs/superpowers/coordination" PM_PROMPT="$(ls -t "$COORD_DIR"/*-pm-prompt.md 2>/dev/null | head -1 || echo "(none found — run multi-agent-kickoff skill first)")" DEV_A_PROMPT="$(ls -t "$COORD_DIR"/*-dev-a-prompt.md 2>/dev/null | head -1 || echo "(none found)")" DEV_B_PROMPT="$(ls -t "$COORD_DIR"/*-dev-b-prompt.md 2>/dev/null | head -1 || echo "(none found)")" +DEV_C_PROMPT="$(ls -t "$COORD_DIR"/*-dev-c-prompt.md 2>/dev/null | head -1 || echo "(none found)")" print_manual_instructions() { echo "" @@ -38,12 +39,13 @@ print_manual_instructions() { echo "║ RELAY SERVER — MULTI-AGENT LIFT LAUNCHER ║" echo "╚══════════════════════════════════════════════════════════════╝" echo "" - echo "Open 3 new terminals. In each, start Claude Code and paste" + echo "Open 4 new terminals. In each, start Claude Code and paste" echo "the content BELOW the '---' line from the corresponding file." echo "" echo " Terminal 1 (PM): cat '$PM_PROMPT'" echo " Terminal 2 (Dev A): cat '$DEV_A_PROMPT'" echo " Terminal 3 (Dev B): cat '$DEV_B_PROMPT'" + echo " Terminal 4 (Dev C): cat '$DEV_C_PROMPT'" echo "" echo "This terminal becomes the relay log. Keep it open." echo "" @@ -57,13 +59,15 @@ launch_tmux() { tmux new-window -t "$SESSION:" -n "pm" "cd '$REPO_ROOT' && claude" tmux new-window -t "$SESSION:" -n "dev-a" "cd '$REPO_ROOT' && claude" tmux new-window -t "$SESSION:" -n "dev-b" "cd '$REPO_ROOT' && claude" + tmux new-window -t "$SESSION:" -n "dev-c" "cd '$REPO_ROOT' && claude" echo "" - echo "[relay] Opened tmux session '$SESSION' with 4 windows: relay, pm, dev-a, dev-b." + echo "[relay] Opened tmux session '$SESSION' with 5 windows: relay, pm, dev-a, dev-b, dev-c." echo "[relay] Paste the kickoff prompt into each Claude window." echo " Prompts:" echo " PM: $PM_PROMPT" echo " Dev A: $DEV_A_PROMPT" echo " Dev B: $DEV_B_PROMPT" + echo " Dev C: $DEV_C_PROMPT" echo "" tmux attach-session -t "$SESSION" } @@ -77,12 +81,15 @@ launch_kitty() { bash -l -i -c "cd '$REPO_ROOT' && claude" kitty @ launch --type=tab --tab-title "Dev-B" --hold -- \ bash -l -i -c "cd '$REPO_ROOT' && claude" + kitty @ launch --type=tab --tab-title "Dev-C" --hold -- \ + bash -l -i -c "cd '$REPO_ROOT' && claude" echo "" - echo "[relay] Opened kitty tab 'relay' + 3 windows (PM, Dev-A, Dev-B)." + echo "[relay] Opened kitty tab 'relay' + 4 windows (PM, Dev-A, Dev-B, Dev-C)." echo " Paste the kickoff prompts into each Claude window." echo " PM: $PM_PROMPT" echo " Dev A: $DEV_A_PROMPT" echo " Dev B: $DEV_B_PROMPT" + echo " Dev C: $DEV_C_PROMPT" } case "$MODE" in