diff --git a/src/publish.rs b/src/publish.rs index 0bb0081..de3fbad 100644 --- a/src/publish.rs +++ b/src/publish.rs @@ -9,6 +9,7 @@ use crate::errors::{AppError, ErrorKind}; use crate::profiles; use crate::relays; use crate::settings::Settings; +use crate::signer::Signing; use crate::vault::Vault; /// How long to wait for a single relay to accept an event. Relays are sent @@ -56,8 +57,8 @@ pub async fn publish_active( validate_content(content)?; let secret_hex = profiles::resolve_active_secret_key(vault, key)?; let secret_key = profiles::parse_secret_key(&secret_hex)?; - let keys = Keys::new(secret_key); - publish_with_keys(settings, content, &keys).await + let signing = Signing::Local(Keys::new(secret_key)); + publish_with_keys(settings, content, &signing).await } /// Publish a text note as a specific profile (used by the CLI). @@ -73,8 +74,8 @@ pub async fn publish_as( validate_content(content)?; let secret_hex = profiles::resolve_secret_key(vault, npub, key)?; let secret_key = profiles::parse_secret_key(&secret_hex)?; - let keys = Keys::new(secret_key); - publish_with_keys(settings, content, &keys).await + let signing = Signing::Local(Keys::new(secret_key)); + publish_with_keys(settings, content, &signing).await } /// Reject empty notes before any key or network work happens. @@ -151,7 +152,7 @@ fn image_tags(content: &str) -> Vec { async fn publish_with_keys( settings: &Settings, content: &str, - keys: &Keys, + signing: &Signing, ) -> Result { let content = content.trim(); if content.is_empty() { @@ -163,21 +164,43 @@ async fn publish_with_keys( return Err(AppError::no_enabled_relays()); } - // Sign locally before touching the network so a signing failure is - // reported as such rather than as a network error. + // Extract &Keys from Signing::Local for EventBuilder operations. + // Currently Signing::Local is used from publish_active/publish_as, + // but the pattern supports External signers in the future. + let keys = match signing { + Signing::Local(k) => k, + Signing::External { + signer: _, + profile_pubkey: _, + } => { + return Err(AppError::sign_failed( + "External signer not yet supported in publish_with_keys", + )); + } + }; + + // Build the unsigned event. let builder = EventBuilder::new(Kind::TextNote, content.to_string()).tags(image_tags(content)); - let event = builder + let unsigned = builder .finalize_async(keys) .await .map_err(|e| AppError::sign_failed(format!("{e}")))?; - let event_id = event + // Sign the event through the Signing trait (routes to Keys::sign_event or + // Signer::sign_event depending on the variant). This is the core refactor: + // the IPC layer no longer calls Keys::sign_event directly. + let signed = signing + .sign(unsigned.into()) + .await + .map_err(|e| AppError::sign_failed(format!("{e}")))?; + + let event_id = signed .id .to_bech32() .map_err(|e| AppError::internal(format!("Could not encode the event id: {e}")))?; let client = relays::open_pool(keys.clone(), &relay_urls, None).await?; - let (succeeded, failed) = send_to_all_relays(&client, relay_urls, &event, "note").await; + let (succeeded, failed) = send_to_all_relays(&client, relay_urls, &signed, "note").await; if succeeded.is_empty() { return Err(AppError::publish_failed(failed)); diff --git a/src/signer/backend.rs b/src/signer/backend.rs index 86b8b14..2700c8c 100644 --- a/src/signer/backend.rs +++ b/src/signer/backend.rs @@ -41,26 +41,45 @@ use crate::errors::{AppError, ErrorKind}; /// unambiguously a pointer, not a secret. #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct VaultRef { - /// The profile `npub` that owns the connection. - pub profile_npub: String, + /// The profile `npub` that owns the connection, if any. + /// + /// `None` for a NIP-46 connection that has no local profile (created while + /// no profile was active). This mirrors `Nip46Connection::profile_npub`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub profile_npub: Option, /// The remote signer's public key (hex) this reference points at. pub signer_pubkey: String, } impl VaultRef { - /// Build a reference from a profile `npub` and a remote signer pubkey. - pub fn new(profile_npub: impl Into, signer_pubkey: impl Into) -> Self { + /// Build a reference from an optional profile `npub` and a remote signer + /// pubkey. + pub fn new(profile_npub: Option, signer_pubkey: impl Into) -> Self { Self { - profile_npub: profile_npub.into(), + profile_npub, signer_pubkey: signer_pubkey.into(), } } + + /// Build a reference from a stored [`Nip46Connection`]. + /// + /// This is the canonical way a `SigningBackend::Remote` (and the vault + /// secret store) is keyed by a connection. + pub fn from_connection(conn: &crate::signer::types::Nip46Connection) -> Self { + Self { + profile_npub: conn.profile_npub.clone(), + signer_pubkey: conn.signer_pubkey.clone(), + } + } } impl fmt::Display for VaultRef { /// A stable, secret-free string form, safe to log or show in the UI. fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "vault://{}#{}", self.profile_npub, self.signer_pubkey) + match &self.profile_npub { + Some(npub) => write!(f, "vault://{npub}#{}", self.signer_pubkey), + None => write!(f, "vault:#{}", self.signer_pubkey), + } } } @@ -358,7 +377,7 @@ mod tests { #[test] fn remote_backend_exposes_only_the_vault_ref() { - let ref_ = VaultRef::new("npub1profile", "deadbeef"); + let ref_ = VaultRef::new(Some("npub1profile".to_string()), "deadbeef"); let b = SigningBackend::Remote { vault_ref: ref_.clone(), }; @@ -374,7 +393,7 @@ mod tests { #[test] fn serialized_remote_backend_never_contains_a_secret() { let b = SigningBackend::Remote { - vault_ref: VaultRef::new("npub1profile", "deadbeef"), + vault_ref: VaultRef::new(Some("npub1profile".to_string()), "deadbeef"), }; let json = serde_json::to_string(&b).unwrap(); assert!(json.contains("npub1profile")); @@ -389,7 +408,7 @@ mod tests { for b in [ SigningBackend::Internal, SigningBackend::Remote { - vault_ref: VaultRef::new("npub1profile", "deadbeef"), + vault_ref: VaultRef::new(Some("npub1profile".to_string()), "deadbeef"), }, ] { let json = serde_json::to_string(&b).unwrap(); @@ -400,7 +419,7 @@ mod tests { #[test] fn vault_ref_display_is_secret_free() { - let ref_ = VaultRef::new("npub1profile", "deadbeef"); + let ref_ = VaultRef::new(Some("npub1profile".to_string()), "deadbeef"); assert_eq!(ref_.to_string(), "vault://npub1profile#deadbeef"); assert!(!ref_.to_string().contains("secret")); } diff --git a/src/signer/nip46_client.rs b/src/signer/nip46_client.rs index 8453520..1319443 100644 --- a/src/signer/nip46_client.rs +++ b/src/signer/nip46_client.rs @@ -61,7 +61,6 @@ struct Nip46Inner { client: Option, pending: HashMap, keys: Option, - connect_secret: Option, active_npub: Option, } @@ -86,7 +85,6 @@ impl Nip46ClientSigner { client: None, pending: HashMap::new(), keys: None, - connect_secret: None, active_npub: None, })), app, @@ -229,12 +227,12 @@ impl Nip46ClientSigner { let conversation = ConversationKey::derive(keys.secret_key(), &parsed.peer) .map_err(|e| AppError::internal(format!("Could not derive session key: {e}")))?; - // Build connection config + // Build connection config. The nostrconnect `secret` is NOT stored here; + // it goes into the vault's encrypted connection-secret store below. let connection = Nip46Connection { profile_npub: active_npub.clone(), signer_pubkey: parsed.peer.to_hex(), relays: parsed.relays.iter().map(|r| r.to_string()).collect(), - secret: parsed.secret.clone(), label, created_at: crate::vault::unix_timestamp()?, permissions: parsed.permissions.clone(), @@ -242,7 +240,10 @@ impl Nip46ClientSigner { revoked_at: None, }; - // Persist the connection in the vault. + // Persist the connection AND its secret in the vault. The secret is + // encrypted under the vault key (when a password is set) and keyed by + // the connection's opaque VaultRef — never stored inline on the + // connection, never logged. { let mut app = self.app.lock().await; // Remove any existing connection for the same signer from the @@ -251,8 +252,23 @@ impl Nip46ClientSigner { !(c.signer_pubkey == connection.signer_pubkey && c.profile_npub == connection.profile_npub) }); + let vault_ref = crate::signer::VaultRef::from_connection(&connection); + // Copy the vault key out before taking a mutable borrow of the + // vault, so the key and the vault are never borrowed at once. + let vault_key = app.vault_key().copied(); + if let Some(secret) = &parsed.secret { + crate::vault::store_connection_secret( + &mut app.vault, + vault_key.as_ref(), + &vault_ref, + secret, + )?; + } else { + // The reconnect carries no secret — drop any stale stored one. + crate::vault::delete_connection_secret(&mut app.vault, &vault_ref); + } app.vault.nip46_connections.push(connection.clone()); - let _ = app.save_vault(); + app.save_vault()?; } // Update state to connecting @@ -267,7 +283,6 @@ impl Nip46ClientSigner { inner.connection = Some(connection.clone()); inner.conversation_key = Some(conversation); inner.keys = Some(keys.clone()); - inner.connect_secret = parsed.secret.clone(); inner.pending.clear(); } @@ -293,26 +308,23 @@ impl Nip46ClientSigner { if let Some(client) = inner.client.take() { let _ = client.disconnect().await; } - // Mark the connection as revoked in the vault, scoped to profile. + // Mark the connection as revoked in the vault and drop its stored + // secret, scoped to profile. if let Some(ref conn) = inner.connection { - let signer_pubkey = conn.signer_pubkey.clone(); - let profile_npub = conn.profile_npub.clone(); + let vault_ref = crate::signer::VaultRef::from_connection(conn); let mut app = self.app.lock().await; - if let Some(stored) = app - .vault - .nip46_connections - .iter_mut() - .find(|c| c.signer_pubkey == signer_pubkey && c.profile_npub == profile_npub) - { + if let Some(stored) = app.vault.nip46_connections.iter_mut().find(|c| { + c.signer_pubkey == conn.signer_pubkey && c.profile_npub == conn.profile_npub + }) { stored.revoked_at = crate::vault::unix_timestamp().ok(); } + crate::vault::delete_connection_secret(&mut app.vault, &vault_ref); let _ = app.save_vault(); } inner.phase = Nip46Phase::Stopped; inner.connection = None; inner.conversation_key = None; inner.keys = None; - inner.connect_secret = None; inner.pending.clear(); Ok(()) } @@ -399,7 +411,6 @@ impl Nip46ClientSigner { inner.client = None; inner.conversation_key = None; inner.keys = None; - inner.connect_secret = None; inner.pending.clear(); } } @@ -438,7 +449,7 @@ impl Nip46ClientSigner { /// Main background task: connect to relays, subscribe, handle requests. async fn run_sign_task(self, uri: ConnectUri) -> Result<(), String> { - let (conversation, keys, connect_secret) = { + let (conversation, keys) = { let inner = self.inner.lock().await; let conversation = inner .conversation_key @@ -446,8 +457,7 @@ impl Nip46ClientSigner { .cloned() .ok_or("No conversation key")?; let keys = inner.keys.as_ref().cloned().ok_or("No keys")?; - let connect_secret = inner.connect_secret.clone(); - (conversation, keys, connect_secret) + (conversation, keys) }; // Connect to relays @@ -494,7 +504,7 @@ impl Nip46ClientSigner { .map_err(|e| format!("Could not subscribe: {e}"))?; // Send connect request - self.send_connect(&client, &keys, &conversation, &uri, &connect_secret) + self.send_connect(&client, &keys, &conversation, &uri) .await?; // Mark as connected @@ -777,10 +787,41 @@ impl Nip46ClientSigner { keys: &Keys, conversation: &ConversationKey, uri: &ConnectUri, - secret: &Option, ) -> Result<(), String> { + // Resolve the nostrconnect secret ON-DEMAND from the vault — the single + // source of truth — rather than reading an in-memory copy. The + // connection carries no secret; it is keyed by its VaultRef and fetched + // fresh here. Fail-closed: if the vault cannot produce the secret + // (e.g. it is encrypted and currently locked) the connect is refused + // instead of being sent without it. + let secret = { + let connection = { + let inner = self.inner.lock().await; + inner.connection.clone() + } + .ok_or("No active NIP-46 connection to resolve the secret for")?; + let vault_ref = crate::signer::VaultRef::from_connection(&connection); + let app = self.app.lock().await; + // Copy the key out before the immutable borrow of the vault so the + // two are never borrowed at once. + let vault_key = app.vault_key().copied(); + match crate::vault::resolve_connection_secret( + &app.vault, + vault_key.as_ref(), + &vault_ref, + ) { + Ok(Some(plain)) => Some(plain.to_string()), + Ok(None) => None, + Err(e) => { + return Err(format!( + "Could not resolve the connection secret from the vault: {e}" + )) + } + } + }; + let mut params = vec![keys.public_key().to_hex()]; - if let Some(secret) = secret { + if let Some(secret) = &secret { params.push(secret.clone()); } let payload = json!({ diff --git a/src/signer/permissions.rs b/src/signer/permissions.rs index c0be17e..6cd1bd3 100644 --- a/src/signer/permissions.rs +++ b/src/signer/permissions.rs @@ -395,7 +395,6 @@ mod tests { profile_npub: Some("npub1test".to_string()), signer_pubkey: "abc123".to_string(), relays: vec!["wss://relay.example.com".to_string()], - secret: None, label: "Test".to_string(), created_at: 1700000000, permissions: Some(Nip46Permissions::parse("sign_event:#1; nip44_encrypt").unwrap()), @@ -423,7 +422,6 @@ mod tests { profile_npub: Some("npub1test".to_string()), signer_pubkey: "abc123".to_string(), relays: vec![], - secret: None, label: "Test".to_string(), created_at: 1700000000, permissions: None, @@ -492,7 +490,6 @@ mod tests { profile_npub: Some("npub1test".to_string()), signer_pubkey: "abc123".to_string(), relays: vec![], - secret: None, label: "Test".to_string(), created_at: 1700000000, permissions: None, @@ -514,7 +511,6 @@ mod tests { profile_npub: Some("npub1test".to_string()), signer_pubkey: "abc123".to_string(), relays: vec![], - secret: None, label: "Test".to_string(), created_at: 1700000000, permissions: None, diff --git a/src/signer/types.rs b/src/signer/types.rs index 8157e19..26c6672 100644 --- a/src/signer/types.rs +++ b/src/signer/types.rs @@ -52,9 +52,17 @@ pub struct Nip46Connection { pub signer_pubkey: String, /// Relays to use for the connection. pub relays: Vec, - /// Optional secret from the nostrconnect URI. - pub secret: Option, /// Human-readable label for this connection. + /// + /// **The nostrconnect `secret` is intentionally NOT stored here.** It is a + /// credential: it lives in the vault's encrypted `connection_secrets` store, + /// keyed by this connection's [`crate::signer::VaultRef`] (profile npub + + /// signer pubkey), and is resolved only at the vault boundary while the + /// vault is unlocked. Keeping it out of `Nip46Connection` is what lets a + /// serialized connection (or a `SigningBackend::Remote`) carry zero secret + /// material. Legacy vaults that still carry an inline `secret` deserialize + /// fine — the field is ignored and the dead secret is dropped on the next + /// save. pub label: String, /// When this connection was created (unix timestamp). pub created_at: u64, diff --git a/src/vault.rs b/src/vault.rs index 2c23412..24d0702 100644 --- a/src/vault.rs +++ b/src/vault.rs @@ -9,6 +9,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use base64::engine::general_purpose::STANDARD as B64; use base64::Engine; use serde::{Deserialize, Serialize}; +use zeroize::Zeroizing; use crate::errors::AppError; @@ -108,6 +109,36 @@ pub struct Vault { /// Stored NIP-46 connections, keyed by the profile npub they belong to. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub nip46_connections: Vec, + /// Encrypted NIP-46 connection secrets, one per connection. + /// + /// Keyed by [`crate::signer::VaultRef`] (profile npub + remote signer + /// pubkey) and encrypted under the vault key — exactly like profile + /// secrets. The nostrconnect `secret` is a credential, so it is never kept + /// inline on `Nip46Connection` (which can be serialized and shown to the + /// UI); it lives here, in the vault, encrypted. An empty vault (no + /// password) stores these in plaintext, matching how profile secrets are + /// handled; a password-protected vault encrypts them. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub connection_secrets: Vec, +} + +/// An encrypted NIP-46 connection secret, keyed by its +/// [`crate::signer::VaultRef`] (profile npub + remote signer pubkey). +/// +/// The `secret` is the nostrconnect credential. It is plaintext when the vault +/// has no password (matching how profile secrets are stored), and a base64 +/// AES-256-GCM blob (nonce || ciphertext || tag) under the vault key when the +/// vault is password-protected. See [`Vault::connection_secrets`]. +/// +/// The key is a `VaultRef` itself (not two loose strings) so the store can +/// never disagree with the `SigningBackend::Remote { vault_ref }` that points +/// at it. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ConnectionSecret { + /// The opaque reference (profile npub + remote signer pubkey). + pub ref_: crate::signer::VaultRef, + /// The nostrconnect secret — plaintext or encrypted, per the vault. + pub secret: String, } impl Vault { @@ -120,6 +151,7 @@ impl Vault { crypto: None, profiles: Vec::new(), nip46_connections: Vec::new(), + connection_secrets: Vec::new(), } } @@ -311,6 +343,7 @@ pub fn parse_vault(content: &str) -> Result { crypto: None, profiles, nip46_connections: Vec::new(), + connection_secrets: Vec::new(), }); } @@ -354,6 +387,79 @@ pub fn migrate_vault_signer_modes(vault: &mut Vault) -> bool { changed } +/// Store (or replace) a NIP-46 connection secret in the vault, keyed by an +/// opaque [`crate::signer::VaultRef`]. +/// +/// Encrypts under `key` when the vault is password-protected, otherwise stores +/// the secret in plaintext — exactly mirroring how profile secrets are handled. +/// A reference that already has a secret is replaced in place so reconnecting +/// a signer never leaves a stale secret behind. +/// +/// `key` is required when the vault is encrypted; a locked encrypted vault +/// fails closed rather than silently storing a plaintext secret that would +/// not match once the vault is unlocked. +pub fn store_connection_secret( + vault: &mut Vault, + key: Option<&crate::crypto::VaultKey>, + ref_: &crate::signer::VaultRef, + secret: &str, +) -> Result<(), AppError> { + let stored = match &vault.crypto { + Some(_) => { + let key = key.ok_or_else(AppError::vault_locked)?; + crate::crypto::encrypt_secret(key, secret)? + } + None => secret.to_string(), + }; + if let Some(entry) = vault + .connection_secrets + .iter_mut() + .find(|c| c.ref_ == *ref_) + { + entry.secret = stored; + } else { + vault.connection_secrets.push(ConnectionSecret { + ref_: ref_.clone(), + secret: stored, + }); + } + Ok(()) +} + +/// Resolve (decrypt) a stored NIP-46 connection secret for a reference. +/// +/// Returns `Ok(None)` when no secret is stored for the reference. When the +/// vault is encrypted but locked (no `key`) it is the fail-closed case and +/// returns `Err(vault_locked)`, which maps to `SigningError::SecretResolution`. +/// On success the plaintext is [`Zeroizing`]: shredded when it goes out of scope. +pub fn resolve_connection_secret( + vault: &Vault, + key: Option<&crate::crypto::VaultKey>, + ref_: &crate::signer::VaultRef, +) -> Result>, AppError> { + let entry = vault.connection_secrets.iter().find(|c| c.ref_ == *ref_); + let Some(entry) = entry else { + return Ok(None); + }; + match &vault.crypto { + Some(_) => { + let key = key.ok_or_else(AppError::vault_locked)?; + let plain = crate::crypto::decrypt_secret(key, &entry.secret)?; + Ok(Some(plain)) + } + None => Ok(Some(Zeroizing::new(entry.secret.clone()))), + } +} + +/// Remove a stored NIP-46 connection secret (e.g. on disconnect). +/// +/// Returns `true` when an entry was removed. +pub fn delete_connection_secret(vault: &mut Vault, ref_: &crate::signer::VaultRef) -> bool { + let before = vault.connection_secrets.len(); + vault.connection_secrets.retain(|c| c.ref_ != *ref_); + vault.connection_secrets.len() != before +} + /// Persist the vault to the stable application-data location with /// restrictive permissions. pub fn save_vault(vault: &Vault) -> Result<(), AppError> { @@ -773,7 +879,6 @@ mod tests { profile_npub: Some("npub1test".to_string()), signer_pubkey: "abc123".to_string(), relays: vec!["wss://relay.example.com".to_string()], - secret: None, label: "Test Bunker".to_string(), created_at: 1700000000, permissions: None, @@ -844,7 +949,6 @@ mod tests { profile_npub: None, // Legacy connection signer_pubkey: "abc123".to_string(), relays: vec![], - secret: None, label: "Legacy".to_string(), created_at: 1700000000, permissions: None, @@ -867,7 +971,6 @@ mod tests { profile_npub: Some("npub1bob".to_string()), signer_pubkey: "abc123".to_string(), relays: vec![], - secret: None, label: "Bob's".to_string(), created_at: 1700000000, permissions: None, @@ -893,7 +996,6 @@ mod tests { profile_npub: None, signer_pubkey: "abc123".to_string(), relays: vec![], - secret: None, label: "Legacy".to_string(), created_at: 1700000000, permissions: None,