From 9a8f334029a6a50a40955c9c5ca9b3c5a20892a8 Mon Sep 17 00:00:00 2001 From: Avi Date: Fri, 21 Aug 2026 16:07:32 -0500 Subject: [PATCH] Refresh checkpoint: all audit findings closed --- CHECKPOINT-encryption.md | 54 +++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/CHECKPOINT-encryption.md b/CHECKPOINT-encryption.md index 1b3748a..00d188c 100644 --- a/CHECKPOINT-encryption.md +++ b/CHECKPOINT-encryption.md @@ -1,4 +1,4 @@ -# Checkpoint — Key material zeroization (2026-08-21) +# Checkpoint — All audit items closed (2026-08-21) A stopping point you can return to if this session is closed. Everything below was verified green at the moment this file was written. @@ -6,14 +6,15 @@ verified green at the moment this file was written. ## Where things are - Project: `/home/avi/Projects/0_Nostr` -- Git repo: `master` @ `130d7e2` ("Zeroize transient secret key material in memory"). - Before it: `4461307` (owner-only writes), `4bd7660` (legacy vault perms), `4d4dfde` - (CSP + navigation guards), `6e627a3` (upload pick tokens), `4bde395` (IPC allowlist). +- Git repo: `master` @ `f7db29e` ("Add SSRF guard, signer queue cap, secret echo, request + timeout"). Before it: `130d7e2` (zeroization), `4461307` (owner-only writes), `4bd7660` + (legacy vault perms), `4d4dfde` (CSP + navigation guards), `6e627a3` (upload tokens), + `4bde395` (IPC allowlist). - Working tree is **clean** apart from this checkpoint update, which is committed right after. -## What was completed: security hardening items #1–#6 +## What was completed: the full 2026-08-21 security audit remediation -Fixes from the 2026-08-21 security audit. +All ten findings from the 2026-08-21 security audit are fixed. **#1 IPC method allowlist (`4bde395`, `main.ts` only):** - `RENDERER_METHODS` set in the Electron main process: the three native methods @@ -93,8 +94,25 @@ Decrypted secrets and derived keys no longer linger in unscrubbed heap memory: `std::mem::take` without extra copies (plaintext-at-rest is that feature's purpose). - Callers in `publish.rs`, `uploads.rs`, `signer.rs` needed no changes (deref coercion). +**Smaller items (`f7db29e`, main.ts + signer.rs):** +- **SSRF guard**: `fetchLinkPreview` resolves the URL's host and refuses loopback/private/ + link-local targets — literal IPs, DNS answers, and localhost/.local names all checked + (`ipv4IsPrivate`/`ipv6IsPrivate`/`resolvesToPrivateAddress`). Crafted note links can no + longer make the app probe localhost or the LAN. +- **Signer queue cap** (`MAX_PENDING_APPROVALS = 20`): a relay flooding `sign_event` + requests cannot grow the approval queue unboundedly or bury a genuine prompt; overflow + requests get the standard "no decision" error. +- **NIP-46 secret echo**: when the nostrconnect:// link carried a secret, the client's + `connect` request must echo it back or it is refused ("did not include the expected + secret"), proving the link arrived unmodified. +- **Backend request timeout**: `backendRequest` reaps any round-trip after 120s (generous + enough for multi-relay publishes), deleting its pending entry so a hung backend cannot + leak promises. + ## Commits +- `f7db29e` "Add SSRF guard, signer queue cap, secret echo, request timeout" — main.ts + + signer.rs (+204/−3). - `130d7e2` "Zeroize transient secret key material in memory" — Cargo.toml, crypto.rs, profiles.rs, app.rs (+77/−23). - `4461307` "Create vault files owner-only from the first byte" — src/vault.rs (+48/−2). @@ -137,13 +155,21 @@ Manual protocol checks worth doing once: ## Outstanding / next steps (if you continue) -Remaining smaller hardening items from the audit: +Nothing outstanding from the audit — all ten findings are closed: -1. Link-preview SSRF guard — block loopback/private IPs in `fetchLinkPreview` (`main.ts`). -2. Signer pending-cap — bound the approval queue (e.g. max 10) against relay spam. -3. NIP-46 secret echo check — verify the client echoes the handshake `secret`. -4. Backend request timeout — `backendRequest` in `main.ts` has no timeout; hung backend - leaks pending promises. +| # | Finding | Fix commit | +|---|---------|-----------| +| 1 | Unrestricted renderer→backend IPC | `4bde395` | +| 2 | Arbitrary file upload paths | `6e627a3` | +| 3 | CSP `unsafe-inline`, no nav guards | `4d4dfde` | +| 4 | Legacy vault world-readable | `4bd7660` | +| 5 | Permission race windows on write | `4461307` | +| 6 | Key material not zeroized | `130d7e2` | +| 7 | Link-preview SSRF | `f7db29e` | +| 8 | Signer queue flooding | `f7db29e` | +| 9 | NIP-46 secret not verified | `f7db29e` | +| 10 | No backend request timeout | `f7db29e` | -All six primary findings are closed (#1 IPC allowlist, #2 upload tokens, #3 CSP + -navigation, #4 legacy vault perms, #5 write-race windows, #6 zeroization). +Possible future work (not audit items): DNS-rebinding TOCTOU in the SSRF guard, OS keyring +integration for the vault password, an automated dependency-audit CI job (`cargo audit`, +`npm audit`).