From f2402eb24d2c3e50e76af039331c672a093a577c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 23:23:26 -0400 Subject: [PATCH] Build 22: security hardening follow-ups From the audit's deferred P2 list: - Global gift-wrap decrypt ceiling (~120/min across all senders) so fresh-keypair spam can't force unbounded NIP-44 decrypts. The per-sender limit only applied after the decrypt revealed the sender; this caps total decrypt work up front. - NostrStore reads tolerate a poisoned lock (unwrap_or_else into_inner) so a single panic can't cascade into taking down all nostr storage for the session. - Avatar upload/delete reuse the service's keys directly instead of round-tripping the secret through a plaintext nsec String. 34 lib tests green. --- src/gui/views/goblin/mod.rs | 30 +++++++++--------------------- src/nostr/client.rs | 30 ++++++++++++++++++++++++++++++ src/nostr/store.rs | 20 ++++++++++---------- 3 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/gui/views/goblin/mod.rs b/src/gui/views/goblin/mod.rs index 85efffc6..0b8a7884 100644 --- a/src/gui/views/goblin/mod.rs +++ b/src/gui/views/goblin/mod.rs @@ -2580,13 +2580,9 @@ fn start_claim_flow(claim: &mut ClaimState, name: &str, wallet: &Wallet) { return; }; let server = service.config.read().nip05_server(); - let keys = match service - .nsec() - .and_then(|nsec| nostr_sdk::Keys::parse(&nsec).ok()) - { - Some(k) => k, - None => return, - }; + // Reuse the service's keys directly — never round-trip the secret through a + // plaintext nsec String to rebuild keys the service already holds. + let keys = service.keys(); claim.checking = true; claim.message = None; claim.available = None; @@ -2626,13 +2622,9 @@ fn start_release(claim: &mut ClaimState, name: &str, wallet: &Wallet) { return; }; let server = service.config.read().nip05_server(); - let keys = match service - .nsec() - .and_then(|nsec| nostr_sdk::Keys::parse(&nsec).ok()) - { - Some(k) => k, - None => return, - }; + // Reuse the service's keys directly — never round-trip the secret through a + // plaintext nsec String to rebuild keys the service already holds. + let keys = service.keys(); claim.checking = true; claim.message = None; let slot = claim.result.clone(); @@ -2664,13 +2656,9 @@ fn start_avatar_upload( return; }; let server = service.config.read().nip05_server(); - let keys = match service - .nsec() - .and_then(|nsec| nostr_sdk::Keys::parse(&nsec).ok()) - { - Some(k) => k, - None => return, - }; + // Reuse the service's keys directly — never round-trip the secret through a + // plaintext nsec String to rebuild keys the service already holds. + let keys = service.keys(); std::thread::spawn(move || { let res = (|| { let png = crate::nostr::avatar::process_avatar_file(&path)?; diff --git a/src/nostr/client.rs b/src/nostr/client.rs index 52900b6e..6f9a6d5b 100644 --- a/src/nostr/client.rs +++ b/src/nostr/client.rs @@ -134,6 +134,12 @@ impl NostrService { self.keys.secret_key().to_bech32().ok() } + /// The service's signing keys, for in-process signing (e.g. NIP-98 auth) + /// without ever serializing the secret to a plaintext `String`. + pub fn keys(&self) -> Keys { + self.keys.clone() + } + /// Fetch a pubkey's published kind-0 profile over the connected relay /// pool (one shot, short timeout). `Some` means the key is a live nostr /// identity; `None` means no profile is published (new/anonymous key) or @@ -254,6 +260,24 @@ impl NostrService { true } + /// Global ceiling on gift-wrap decrypt attempts across ALL senders. The + /// per-sender limit only kicks in after the (expensive) NIP-44 decrypt + /// reveals the sender, so an attacker minting unlimited fresh keypairs + /// would otherwise force unbounded decrypts. Bounds total decrypt work to + /// ~2/sec — far above any legitimate inbound rate. + fn allow_global_unwrap(&self) -> bool { + const GLOBAL_PER_MIN: usize = 120; + let now = unix_time(); + let mut rate = self.rate.lock(); + let hits = rate.entry("\0global".to_string()).or_default(); + hits.retain(|t| now - *t < 60); + if hits.len() >= GLOBAL_PER_MIN { + return false; + } + hits.push(now); + true + } + /// Dispatch a payment DM (slatepack + optional note) to a recipient, /// publishing to their DM relays plus our own relay set. pub async fn send_payment_dm( @@ -541,6 +565,12 @@ async fn handle_wrap(svc: &Arc, wallet: &Wallet, client: &Client, if svc.store.is_processed(&wrap_id) { return; } + // 2.5 Global decrypt ceiling: bound total NIP-44 unwrap work regardless of + // sender, so fresh-keypair spam can't burn unbounded CPU/battery. Not marked + // processed — a genuine backlog re-attempts once the window reopens. + if !svc.allow_global_unwrap() { + return; + } // 3. Unwrap (NIP-59: seal signature is verified, rumor must not be signed). let unwrapped = match client.unwrap_gift_wrap(&event).await { Ok(u) => u, diff --git a/src/nostr/store.rs b/src/nostr/store.rs index 5e5100ac..158c93dd 100644 --- a/src/nostr/store.rs +++ b/src/nostr/store.rs @@ -90,7 +90,7 @@ impl NostrStore { store: &SingleStore, key: &str, ) -> Option { - let env = self.env.read().unwrap(); + let env = self.env.read().unwrap_or_else(|e| e.into_inner()); let reader = env.read().unwrap(); if let Ok(Some(Value::Json(raw))) = store.get(&reader, key) { return serde_json::from_str(raw).ok(); @@ -100,7 +100,7 @@ impl NostrStore { fn put_json(&self, store: &SingleStore, key: &str, value: &T) { if let Ok(raw) = serde_json::to_string(value) { - let env = self.env.read().unwrap(); + let env = self.env.read().unwrap_or_else(|e| e.into_inner()); let mut writer = env.write().unwrap(); let _ = store.put(&mut writer, key, &Value::Json(&raw)); let _ = writer.commit(); @@ -108,14 +108,14 @@ impl NostrStore { } fn delete(&self, store: &SingleStore, key: &str) { - let env = self.env.read().unwrap(); + let env = self.env.read().unwrap_or_else(|e| e.into_inner()); let mut writer = env.write().unwrap(); let _ = store.delete(&mut writer, key); let _ = writer.commit(); } fn all_json(&self, store: &SingleStore) -> Vec { - let env = self.env.read().unwrap(); + let env = self.env.read().unwrap_or_else(|e| e.into_inner()); let reader = env.read().unwrap(); let mut out = vec![]; if let Ok(iter) = store.iter_start(&reader) { @@ -131,7 +131,7 @@ impl NostrStore { } fn clear(&self, store: &SingleStore) { - let env = self.env.read().unwrap(); + let env = self.env.read().unwrap_or_else(|e| e.into_inner()); let mut writer = env.write().unwrap(); let _ = store.clear(&mut writer); let _ = writer.commit(); @@ -205,13 +205,13 @@ impl NostrStore { // ── processed markers ─────────────────────────────────────────────────── pub fn is_processed(&self, key: &str) -> bool { - let env = self.env.read().unwrap(); + let env = self.env.read().unwrap_or_else(|e| e.into_inner()); let reader = env.read().unwrap(); matches!(self.processed.get(&reader, key), Ok(Some(_))) } pub fn mark_processed(&self, key: &str) { - let env = self.env.read().unwrap(); + let env = self.env.read().unwrap_or_else(|e| e.into_inner()); let mut writer = env.write().unwrap(); let _ = self .processed @@ -223,7 +223,7 @@ impl NostrStore { pub fn prune_processed(&self) { let cutoff = unix_time() - PROCESSED_TTL_SECS; let stale: Vec = { - let env = self.env.read().unwrap(); + let env = self.env.read().unwrap_or_else(|e| e.into_inner()); let reader = env.read().unwrap(); let mut stale = vec![]; if let Ok(iter) = self.processed.iter_start(&reader) { @@ -247,7 +247,7 @@ impl NostrStore { // ── settings ──────────────────────────────────────────────────────────── pub fn last_connected_at(&self) -> Option { - let env = self.env.read().unwrap(); + let env = self.env.read().unwrap_or_else(|e| e.into_inner()); let reader = env.read().unwrap(); if let Ok(Some(Value::I64(v))) = self.settings.get(&reader, "last_connected_at") { return Some(v); @@ -256,7 +256,7 @@ impl NostrStore { } pub fn set_last_connected_at(&self, ts: i64) { - let env = self.env.read().unwrap(); + let env = self.env.read().unwrap_or_else(|e| e.into_inner()); let mut writer = env.write().unwrap(); let _ = self .settings