feat(signer): end-to-end external NIP-46 signing in publish and upload auth
Step 3 sub-step 2 (IPC reroute) — the publish path now actually signs remotely instead of returning 'not yet supported': - src/signer/nip46_client.rs: outbound NIP-46 request half — send sign_event over the encrypted channel, demux responses to waiting callers, 30s timeout, waiters woken on disconnect/fail. Signer::sign_event verifies the returned event matches the requested unsigned event, is signed by the connected identity, and carries a valid signature; no local fallback. Permission-denied audit uses try_lock so a denied in-flight sign cannot deadlock the dispatcher. - src/app.rs: App::signing_for / signing_active — one place that maps a profile's SignerMode to a Signing source. Embedded -> Local(vault key); Nip46Client -> External(live signer) only when connected, otherwise ExternalSignerNotConnected; Nip46Bunker fails closed. - src/publish.rs: publish_with_keys builds the unsigned event from the Signing's own pubkey and signs via Signing::sign; publish_signed entry point for IPC (CLI keeps publish_active local path). - src/ipc.rs: PublishNote and UploadAuth route through signing_active. Nip46Connect/Nip46Disconnect drop the App guard before awaiting connect()/disconnect() (they re-lock internally — latent deadlock). - src/relays.rs: keyless relay pool (open_pool_inner(Option<Keys>)) so external signing publishes without local keys. - src/uploads.rs: nip98_authorization takes a Signing source, so upload auth signs remotely for external profiles too. - src/profiles.rs: store_remote_profile — connecting a NIP-46 signer creates/refreshes a secretless Nip46Client profile row; refuses to silently convert an existing local profile. Tests: signing selection (local, fail-closed external, no profile), store_remote_profile create + no-clobber. 200 tests pass; clippy clean; fmt clean; release build green.
This commit is contained in:
Avi
parent
8780ef3fbb
commit
1af79d81cd
7 changed files with 486 additions and 142 deletions
117
src/app.rs
117
src/app.rs
|
|
@ -9,9 +9,12 @@ use crate::crypto::{self, VaultKey};
|
|||
use crate::errors::AppError;
|
||||
use crate::profiles::{self, ProfileSummary};
|
||||
use crate::settings::Settings;
|
||||
use crate::signer::Signer as SignerTrait;
|
||||
use crate::signer::Signing;
|
||||
use crate::vault::{
|
||||
self, KdfParams, SignerMode, StoredProfile, StoredPublishReport, Vault, VaultCrypto,
|
||||
};
|
||||
use nostr_sdk::prelude::{Keys, PublicKey};
|
||||
|
||||
/// Minimum password length accepted when encrypting the vault.
|
||||
pub const MIN_PASSWORD_LEN: usize = 8;
|
||||
|
|
@ -116,6 +119,56 @@ impl App {
|
|||
self.vault.is_encrypted() && self.unlock_key.is_none()
|
||||
}
|
||||
|
||||
/// The [`Signing`] source for user content of a specific profile.
|
||||
///
|
||||
/// The single place the "where does signing happen" decision is made, so
|
||||
/// no caller branches on signer mode itself:
|
||||
///
|
||||
/// - Profile mode `Embedded` → [`Signing::Local`] with the vault-resolved
|
||||
/// key (locked vault surfaces as the usual `VaultLocked` error).
|
||||
/// - Profile mode `Nip46Client` → [`Signing::External`] wrapping the live
|
||||
/// NIP-46 client signer, **only** when one is present and connected.
|
||||
/// Never falls back to the local key: an external profile that cannot
|
||||
/// reach its signer fails with `ExternalSignerNotConnected`.
|
||||
/// - `Nip46Bunker` (legacy, not wired) → fails closed like a missing
|
||||
/// connection.
|
||||
pub async fn signing_for(&self, npub: &str) -> Result<Signing, AppError> {
|
||||
let profile = profiles::find_stored_profile(&self.vault, npub)?;
|
||||
match profile.signer_mode {
|
||||
SignerMode::Embedded => {
|
||||
let secret_hex = profiles::resolve_secret_key(&self.vault, npub, self.vault_key())?;
|
||||
let secret_key = profiles::parse_secret_key(&secret_hex)?;
|
||||
Ok(Signing::Local(Keys::new(secret_key)))
|
||||
}
|
||||
SignerMode::Nip46Client => {
|
||||
let Some(signer) = self.nip46_signer.clone() else {
|
||||
return Err(AppError::external_signer_not_connected());
|
||||
};
|
||||
if !signer.is_available().await {
|
||||
return Err(AppError::external_signer_not_connected());
|
||||
}
|
||||
let profile_pubkey = PublicKey::parse(npub).map_err(|e| {
|
||||
AppError::internal(format!("Stored profile npub is not valid: {e}"))
|
||||
})?;
|
||||
Ok(Signing::External {
|
||||
signer,
|
||||
profile_pubkey,
|
||||
})
|
||||
}
|
||||
SignerMode::Nip46Bunker => Err(AppError::external_signer_not_connected()),
|
||||
}
|
||||
}
|
||||
|
||||
/// [`Signing`] for the active profile (see [`App::signing_for`]).
|
||||
pub async fn signing_active(&self) -> Result<Signing, AppError> {
|
||||
let npub = self
|
||||
.vault
|
||||
.active_profile
|
||||
.clone()
|
||||
.ok_or_else(AppError::no_active_profile)?;
|
||||
self.signing_for(&npub).await
|
||||
}
|
||||
|
||||
/// Verify a password and keep the derived key in memory for the session.
|
||||
pub fn unlock(&mut self, password: &str) -> Result<(), AppError> {
|
||||
let crypto = self
|
||||
|
|
@ -451,6 +504,70 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signing_for_embedded_profile_yields_local_signing() {
|
||||
let app = sample_app();
|
||||
let npub = app.vault.profiles[0].public_key.clone();
|
||||
let runtime = tokio::runtime::Runtime::new().unwrap();
|
||||
let signing = runtime
|
||||
.block_on(app.signing_for(&npub))
|
||||
.expect("embedded profile must select local signing");
|
||||
assert!(matches!(signing, crate::signer::Signing::Local(_)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signing_for_external_profile_without_connection_fails_closed() {
|
||||
let mut app = sample_app();
|
||||
// Mark the active profile as externally signed; no signer is
|
||||
// connected (and none can be without a live NIP-46 session).
|
||||
app.vault.profiles[0].signer_mode = SignerMode::Nip46Client;
|
||||
let npub = app.vault.profiles[0].public_key.clone();
|
||||
let runtime = tokio::runtime::Runtime::new().unwrap();
|
||||
match runtime.block_on(app.signing_for(&npub)) {
|
||||
Err(err) => assert_eq!(err.kind(), ErrorKind::ExternalSignerNotConnected),
|
||||
Ok(_) => panic!("external profile with no signer must fail closed"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signing_active_requires_a_profile() {
|
||||
let mut app = sample_app();
|
||||
app.vault.active_profile = None;
|
||||
let runtime = tokio::runtime::Runtime::new().unwrap();
|
||||
match runtime.block_on(app.signing_active()) {
|
||||
Err(err) => assert_eq!(err.kind(), ErrorKind::NoActiveProfile),
|
||||
Ok(_) => panic!("no active profile must error"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn store_remote_profile_creates_secretless_external_profile() {
|
||||
use nostr::nips::nip19::ToBech32;
|
||||
let mut vault = plaintext_vault();
|
||||
let remote = Keys::generate();
|
||||
let npub = remote.public_key().to_bech32().unwrap();
|
||||
let summary = profiles::store_remote_profile(&mut vault, &npub, "Remote".to_string())
|
||||
.expect("remote profile must be created");
|
||||
assert_eq!(summary.npub, npub);
|
||||
assert!(summary.is_active);
|
||||
let stored = profiles::find_stored_profile(&vault, &npub).unwrap();
|
||||
assert_eq!(stored.signer_mode, SignerMode::Nip46Client);
|
||||
assert!(stored.secret_key.is_empty(), "no local secret for remote");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn store_remote_profile_refuses_to_clobber_local_profile() {
|
||||
let mut vault = plaintext_vault();
|
||||
let existing = vault.profiles[0].public_key.clone();
|
||||
let err = profiles::store_remote_profile(&mut vault, &existing, "Hijack".to_string())
|
||||
.expect_err("a local profile must not be converted silently");
|
||||
assert!(err.message().contains("local profile"));
|
||||
// Untouched: still embedded, secret intact, active unchanged.
|
||||
let stored = profiles::find_stored_profile(&vault, &existing).unwrap();
|
||||
assert_eq!(stored.signer_mode, SignerMode::Embedded);
|
||||
assert!(!stored.secret_key.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn set_password_encrypts_every_secret() {
|
||||
let mut app = sample_app();
|
||||
|
|
|
|||
42
src/ipc.rs
42
src/ipc.rs
|
|
@ -396,15 +396,21 @@ async fn run(app: &Arc<Mutex<App>>, request: Request) -> Result<serde_json::Valu
|
|||
|
||||
// NIP-46 client signer
|
||||
Request::Nip46Connect { uri, label } => {
|
||||
// Take the signer handle under the lock, then drop the guard
|
||||
// before awaiting: connect() re-locks the App internally (to
|
||||
// persist the connection and resolve its secret), so holding the
|
||||
// guard across the await would deadlock.
|
||||
let signer = {
|
||||
let guard = app.lock().await;
|
||||
if let Some(signer) = &guard.nip46_signer {
|
||||
guard.nip46_signer.clone()
|
||||
};
|
||||
let Some(signer) = signer else {
|
||||
return Err(AppError::config(
|
||||
"NIP-46 signer not initialized. Set signer mode to nip46 first.",
|
||||
));
|
||||
};
|
||||
let status = signer.connect(&uri, label).await?;
|
||||
Ok(json!(status))
|
||||
} else {
|
||||
Err(AppError::config(
|
||||
"NIP-46 signer not initialized. Set signer mode to nip46 first.",
|
||||
))
|
||||
}
|
||||
}
|
||||
Request::Nip46Disconnect => {
|
||||
let guard = app.lock().await;
|
||||
|
|
@ -646,9 +652,17 @@ async fn run_with_app(app: &mut App, request: Request) -> Result<serde_json::Val
|
|||
}
|
||||
|
||||
Request::PublishNote { content } => {
|
||||
let report =
|
||||
publish::publish_active(&app.vault, &app.settings, &content, app.vault_key())
|
||||
.await?;
|
||||
// Signer selection lives in App::signing_for: an embedded profile
|
||||
// signs with the vault key; an external (NIP-46) profile's note
|
||||
// round-trips to the connected signer, and an unconnected one
|
||||
// fails closed — never with a silent fallback to the local key.
|
||||
//
|
||||
// As before, the shared App guard is held across the publish.
|
||||
// The signer's background task needs no App lock to deliver the
|
||||
// sign response (only audit paths take it, briefly), so the
|
||||
// round-trip completes with the guard held.
|
||||
let signing = app.signing_active().await?;
|
||||
let report = publish::publish_signed(&app.settings, &content, &signing).await?;
|
||||
let stored = crate::vault::StoredPublishReport {
|
||||
event_id: report.event_id.clone(),
|
||||
succeeded: report.succeeded.clone(),
|
||||
|
|
@ -765,13 +779,9 @@ async fn run_with_app(app: &mut App, request: Request) -> Result<serde_json::Val
|
|||
}
|
||||
|
||||
Request::UploadAuth { url, http_method } => {
|
||||
let authorization = crate::uploads::nip98_authorization(
|
||||
&app.vault,
|
||||
&url,
|
||||
&http_method,
|
||||
app.vault_key(),
|
||||
)
|
||||
.await?;
|
||||
let signing = app.signing_active().await?;
|
||||
let authorization =
|
||||
crate::uploads::nip98_authorization(&url, &http_method, &signing).await?;
|
||||
Ok(json!({ "authorization": authorization }))
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ use crate::errors::AppError;
|
|||
use crate::publish::RelayFailure;
|
||||
use crate::relays;
|
||||
use crate::settings::Settings;
|
||||
use crate::vault::{unix_timestamp, StoredProfile, Vault};
|
||||
use crate::vault::{unix_timestamp, SignerMode, StoredProfile, Vault};
|
||||
|
||||
/// A safe view of a profile that contains no secret key material.
|
||||
#[derive(Debug, Clone, Serialize, PartialEq, Eq)]
|
||||
|
|
@ -463,6 +463,58 @@ pub fn find_stored_profile<'a>(
|
|||
.ok_or_else(|| AppError::profile_not_found(npub))
|
||||
}
|
||||
|
||||
/// Create or refresh the vault profile for a remote (NIP-46) identity.
|
||||
///
|
||||
/// When a NIP-46 client connection is established the identity lives on the
|
||||
/// remote signer, but the user still needs a profile row so publishing has a
|
||||
/// selection. The row is marked `Nip46Client` and carries **no secret key**
|
||||
/// (there is none locally): every signing operation for it must go through
|
||||
/// the connected signer, and key export refuses it. Re-connecting updates the
|
||||
/// label and re-activates the profile rather than duplicating it.
|
||||
pub fn store_remote_profile(
|
||||
vault: &mut Vault,
|
||||
npub: &str,
|
||||
label: String,
|
||||
) -> Result<ProfileSummary, AppError> {
|
||||
// Validate the identity before writing anything.
|
||||
PublicKey::parse(npub)
|
||||
.map_err(|e| AppError::internal(format!("Remote signer identity is not valid: {e}")))?;
|
||||
// An existing local profile must never be silently converted to remote:
|
||||
// refuse *before* mutating if it carries a local secret.
|
||||
if let Some(existing) = vault.profiles.iter().find(|p| p.public_key == npub) {
|
||||
if existing.signer_mode != SignerMode::Nip46Client && !existing.secret_key.trim().is_empty()
|
||||
{
|
||||
return Err(AppError::config(
|
||||
"That identity already exists as a local profile. Delete it first if you want to use an external signer for it.",
|
||||
));
|
||||
}
|
||||
}
|
||||
if let Some(existing) = vault.profiles.iter_mut().find(|p| p.public_key == npub) {
|
||||
existing.signer_mode = SignerMode::Nip46Client;
|
||||
existing.label = label;
|
||||
} else {
|
||||
vault.profiles.push(StoredProfile {
|
||||
label: label.clone(),
|
||||
public_key: npub.to_string(),
|
||||
secret_key: String::new(), // no local key — identity lives on the signer
|
||||
created_at: unix_timestamp()?,
|
||||
picture: None,
|
||||
nip05: None,
|
||||
signer_mode: SignerMode::Nip46Client,
|
||||
});
|
||||
}
|
||||
vault.active_profile = Some(npub.to_string());
|
||||
let stored = find_profile(vault, npub)?;
|
||||
Ok(ProfileSummary {
|
||||
label: stored.label.clone(),
|
||||
npub: stored.public_key.clone(),
|
||||
created_at: stored.created_at,
|
||||
is_active: true,
|
||||
picture: stored.picture.clone(),
|
||||
nip05: stored.nip05.clone(),
|
||||
})
|
||||
}
|
||||
|
||||
fn find_profile<'a>(vault: &'a Vault, npub: &str) -> Result<&'a StoredProfile, AppError> {
|
||||
vault
|
||||
.profiles
|
||||
|
|
|
|||
|
|
@ -48,6 +48,9 @@ impl PublishReport {
|
|||
/// Publish a text note with the active profile.
|
||||
///
|
||||
/// `key` must be the unlocked vault key when the vault is password-protected.
|
||||
/// Always signs locally from the vault — used by the CLI, which has no signer
|
||||
/// instances. GUI callers use [`publish_signed`] with an [`App::signing_for`]
|
||||
/// signing source so external-signer profiles route to their remote signer.
|
||||
pub async fn publish_active(
|
||||
vault: &Vault,
|
||||
settings: &Settings,
|
||||
|
|
@ -61,6 +64,17 @@ pub async fn publish_active(
|
|||
publish_with_keys(settings, content, &signing).await
|
||||
}
|
||||
|
||||
/// Publish a text note through an explicit [`Signing`] source (embedded or
|
||||
/// external). This is what the GUI publish path uses.
|
||||
pub async fn publish_signed(
|
||||
settings: &Settings,
|
||||
content: &str,
|
||||
signing: &Signing,
|
||||
) -> Result<PublishReport, AppError> {
|
||||
validate_content(content)?;
|
||||
publish_with_keys(settings, content, signing).await
|
||||
}
|
||||
|
||||
/// Publish a text note as a specific profile (used by the CLI).
|
||||
///
|
||||
/// `key` must be the unlocked vault key when the vault is password-protected.
|
||||
|
|
@ -164,33 +178,19 @@ async fn publish_with_keys(
|
|||
return Err(AppError::no_enabled_relays());
|
||||
}
|
||||
|
||||
// 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",
|
||||
));
|
||||
}
|
||||
};
|
||||
// The pubkey comes from the Signing itself. For an external signer this
|
||||
// performs identity validation first: a signer that does not control the
|
||||
// active profile's key fails here, before any event is built.
|
||||
let pubkey = signing.pubkey().await.map_err(AppError::from)?;
|
||||
|
||||
// Build the unsigned event.
|
||||
// Build the unsigned event under the signing identity, then sign it
|
||||
// through `Signing` (local key or the NIP-46 round-trip). This is the
|
||||
// core reroute: the IPC layer never calls Keys::sign_event directly, and
|
||||
// an external profile never needs a local secret.
|
||||
let builder = EventBuilder::new(Kind::TextNote, content.to_string()).tags(image_tags(content));
|
||||
let unsigned = builder
|
||||
.finalize_async(keys)
|
||||
.await
|
||||
.map_err(|e| AppError::sign_failed(format!("{e}")))?;
|
||||
|
||||
// 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 unsigned = builder.finalize_unsigned(pubkey);
|
||||
let signed = signing
|
||||
.sign(unsigned.into())
|
||||
.sign(unsigned)
|
||||
.await
|
||||
.map_err(|e| AppError::sign_failed(format!("{e}")))?;
|
||||
|
||||
|
|
@ -199,7 +199,13 @@ async fn publish_with_keys(
|
|||
.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?;
|
||||
// Local signing can answer NIP-42 AUTH challenges; with an external
|
||||
// signer the app holds no key, so the pool opens without an
|
||||
// authenticator and auth-gated relays report their rejection per-relay.
|
||||
let client = match signing {
|
||||
Signing::Local(keys) => relays::open_pool(keys.clone(), &relay_urls, None).await?,
|
||||
Signing::External { .. } => relays::open_pool_anon(&relay_urls, None).await?,
|
||||
};
|
||||
let (succeeded, failed) = send_to_all_relays(&client, relay_urls, &signed, "note").await;
|
||||
|
||||
if succeeded.is_empty() {
|
||||
|
|
|
|||
|
|
@ -78,12 +78,36 @@ pub(crate) async fn open_pool(
|
|||
keys: Keys,
|
||||
relay_urls: &[String],
|
||||
wait: Option<Duration>,
|
||||
) -> Result<Client, AppError> {
|
||||
open_pool_inner(Some(keys), relay_urls, wait).await
|
||||
}
|
||||
|
||||
/// Open a relay pool with no signing identity.
|
||||
///
|
||||
/// Used when user content is signed by an external (NIP-46) signer: the app
|
||||
/// holds no key to answer NIP-42 AUTH challenges with, so the pool is built
|
||||
/// without an authenticator. Relays that demand auth will reject reads/
|
||||
/// writes at the protocol level, which `send_to_all_relays` already reports
|
||||
/// per-relay.
|
||||
pub(crate) async fn open_pool_anon(
|
||||
relay_urls: &[String],
|
||||
wait: Option<Duration>,
|
||||
) -> Result<Client, AppError> {
|
||||
open_pool_inner(None, relay_urls, wait).await
|
||||
}
|
||||
|
||||
async fn open_pool_inner(
|
||||
keys: Option<Keys>,
|
||||
relay_urls: &[String],
|
||||
wait: Option<Duration>,
|
||||
) -> Result<Client, AppError> {
|
||||
// The authenticator answers NIP-42 AUTH challenges automatically on every
|
||||
// path that opens a client (nostr-sdk >= 0.45 has no implicit signer).
|
||||
let client = Client::builder()
|
||||
.authenticator(SignerAuthenticator::new(keys))
|
||||
.build();
|
||||
let builder = match keys {
|
||||
Some(keys) => Client::builder().authenticator(SignerAuthenticator::new(keys)),
|
||||
None => Client::builder(),
|
||||
};
|
||||
let client = builder.build();
|
||||
for url in relay_urls {
|
||||
client
|
||||
.add_relay(url.as_str())
|
||||
|
|
|
|||
|
|
@ -31,6 +31,9 @@ const CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
|
|||
const APPROVAL_TIMEOUT: Duration = Duration::from_secs(300);
|
||||
/// Maximum number of requests kept waiting for approval at once.
|
||||
const MAX_PENDING_APPROVALS: usize = 20;
|
||||
/// How long an outbound NIP-46 request (e.g. our own `sign_event`) may wait
|
||||
/// for the remote signer's response before it is abandoned.
|
||||
const REQUEST_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
|
||||
/// Internal state for a pending approval.
|
||||
struct PendingApprovalInner {
|
||||
|
|
@ -39,6 +42,12 @@ struct PendingApprovalInner {
|
|||
sender: oneshot::Sender<ApprovalResult>,
|
||||
}
|
||||
|
||||
/// A response awaited from the *remote signer* for a request we sent
|
||||
/// (the client half of the NIP-46 flow, e.g. our `sign_event` request).
|
||||
struct PendingRemoteRequest {
|
||||
sender: oneshot::Sender<Result<String, String>>,
|
||||
}
|
||||
|
||||
/// Parsed nostrconnect:// URI.
|
||||
struct ConnectUri {
|
||||
peer: PublicKey,
|
||||
|
|
@ -60,6 +69,9 @@ struct Nip46Inner {
|
|||
conversation_key: Option<ConversationKey>,
|
||||
client: Option<Client>,
|
||||
pending: HashMap<String, PendingApprovalInner>,
|
||||
/// Outbound requests we sent to the remote signer (e.g. `sign_event`)
|
||||
/// waiting for its encrypted response, keyed by request id.
|
||||
remote_pending: HashMap<String, PendingRemoteRequest>,
|
||||
keys: Option<Keys>,
|
||||
active_npub: Option<String>,
|
||||
}
|
||||
|
|
@ -84,6 +96,7 @@ impl Nip46ClientSigner {
|
|||
conversation_key: None,
|
||||
client: None,
|
||||
pending: HashMap::new(),
|
||||
remote_pending: HashMap::new(),
|
||||
keys: None,
|
||||
active_npub: None,
|
||||
})),
|
||||
|
|
@ -97,10 +110,16 @@ impl Nip46ClientSigner {
|
|||
}
|
||||
|
||||
/// Emit an audit event for a permission-denied NIP-46 operation.
|
||||
///
|
||||
/// Uses `try_lock`: the IPC dispatcher may hold the App lock while a
|
||||
/// sign request is in flight (PublishNote), and audit is best-effort —
|
||||
/// blocking here would deadlock the very request being denied.
|
||||
async fn audit_permission_denied(&self, method: &str) {
|
||||
let npub = self.inner.lock().await.active_npub.clone();
|
||||
if let Some(npub) = npub {
|
||||
let mut app = self.app.lock().await;
|
||||
let Ok(mut app) = self.app.try_lock() else {
|
||||
return;
|
||||
};
|
||||
if let Some(ref mut log) = app.audit_log {
|
||||
let _ = log.record(
|
||||
&npub,
|
||||
|
|
@ -243,8 +262,8 @@ impl Nip46ClientSigner {
|
|||
// 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.
|
||||
{
|
||||
// connection, never logged. Evaluates to the remote identity's npub.
|
||||
let remote_npub = {
|
||||
let mut app = self.app.lock().await;
|
||||
// Remove any existing connection for the same signer from the
|
||||
// same profile (reconnect replaces the old connection).
|
||||
|
|
@ -268,8 +287,19 @@ impl Nip46ClientSigner {
|
|||
crate::vault::delete_connection_secret(&mut app.vault, &vault_ref);
|
||||
}
|
||||
app.vault.nip46_connections.push(connection.clone());
|
||||
// The remote identity gets its own profile row so publishing has
|
||||
// a selection: marked Nip46Client with no local secret, so every
|
||||
// signing path for it routes through this connection and key
|
||||
// export refuses it. If the peer key collides with an existing
|
||||
// local (embedded) profile the connect fails closed.
|
||||
let remote_npub = PublicKey::from_hex(&connection.signer_pubkey)
|
||||
.map_err(|e| AppError::internal(format!("Invalid signer public key: {e}")))?
|
||||
.to_bech32()
|
||||
.map_err(|e| AppError::internal(format!("Could not encode npub: {e}")))?;
|
||||
profiles::store_remote_profile(&mut app.vault, &remote_npub, connection.label.clone())?;
|
||||
app.save_vault()?;
|
||||
}
|
||||
remote_npub
|
||||
};
|
||||
|
||||
// Update state to connecting
|
||||
{
|
||||
|
|
@ -283,6 +313,9 @@ impl Nip46ClientSigner {
|
|||
inner.connection = Some(connection.clone());
|
||||
inner.conversation_key = Some(conversation);
|
||||
inner.keys = Some(keys.clone());
|
||||
// The signing identity is the remote signer's key, and the vault
|
||||
// now holds a matching Nip46Client profile row (active).
|
||||
inner.active_npub = Some(remote_npub);
|
||||
inner.pending.clear();
|
||||
}
|
||||
|
||||
|
|
@ -326,6 +359,13 @@ impl Nip46ClientSigner {
|
|||
inner.conversation_key = None;
|
||||
inner.keys = None;
|
||||
inner.pending.clear();
|
||||
// Wake any callers awaiting a remote response; their waiters turn
|
||||
// into `NotConnected` rather than hanging until the request timeout.
|
||||
for (_, waiter) in inner.remote_pending.drain() {
|
||||
let _ = waiter.sender.send(Err(
|
||||
"Disconnected from the signer before it responded.".to_string()
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
|
@ -412,6 +452,11 @@ impl Nip46ClientSigner {
|
|||
inner.conversation_key = None;
|
||||
inner.keys = None;
|
||||
inner.pending.clear();
|
||||
for (_, waiter) in inner.remote_pending.drain() {
|
||||
let _ = waiter.sender.send(Err(
|
||||
"Lost the connection to the signer before it responded.".to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -447,6 +492,83 @@ impl Nip46ClientSigner {
|
|||
}
|
||||
}
|
||||
|
||||
/// Send a NIP-46 request to the connected remote signer and await its
|
||||
/// encrypted response (the client half of the protocol — e.g. asking the
|
||||
/// signer to `sign_event` an event for us).
|
||||
///
|
||||
/// Fails closed: no connection, no live session, or a signer error all
|
||||
/// return [`SigningError`] rather than falling back to any local key. The
|
||||
/// waiter is always removed from the pending map, even on timeout, so a
|
||||
/// late response to an abandoned request finds nothing to wake.
|
||||
async fn send_remote_request(
|
||||
&self,
|
||||
method: &str,
|
||||
params: Vec<String>,
|
||||
) -> Result<String, SigningError> {
|
||||
let id = uuid::Uuid::new_v4().to_string();
|
||||
let (sender, receiver) = oneshot::channel();
|
||||
|
||||
// Snapshot the live session, register the waiter, and send the
|
||||
// encrypted request in one lock pass. Holding the lock across the send
|
||||
// is safe (the send is a relay hand-off, never a re-entry into this
|
||||
// signer) and guarantees registration cannot interleave with the send.
|
||||
let send_result = {
|
||||
let mut inner = self.inner.lock().await;
|
||||
let client = inner.client.clone().ok_or(SigningError::NotConnected)?;
|
||||
let keys = inner.keys.clone().ok_or(SigningError::NotConnected)?;
|
||||
let conversation = inner
|
||||
.conversation_key
|
||||
.as_ref()
|
||||
.cloned()
|
||||
.ok_or(SigningError::NotConnected)?;
|
||||
let connection = inner.connection.clone().ok_or(SigningError::NotConnected)?;
|
||||
if !connection_valid_now(&connection) {
|
||||
return Err(if connection.revoked_at.is_some() {
|
||||
SigningError::ConnectionRevoked
|
||||
} else {
|
||||
SigningError::ConnectionExpired
|
||||
});
|
||||
}
|
||||
if !matches!(inner.phase, Nip46Phase::Connected) {
|
||||
return Err(SigningError::NotConnected);
|
||||
}
|
||||
if inner.remote_pending.len() >= MAX_PENDING_APPROVALS {
|
||||
return Err(SigningError::Internal {
|
||||
detail: "Too many requests already awaiting the signer.".to_string(),
|
||||
});
|
||||
}
|
||||
let peer = PublicKey::from_hex(&connection.signer_pubkey).map_err(|e| {
|
||||
SigningError::Internal {
|
||||
detail: format!("Invalid signer public key: {e}"),
|
||||
}
|
||||
})?;
|
||||
inner
|
||||
.remote_pending
|
||||
.insert(id.clone(), PendingRemoteRequest { sender });
|
||||
|
||||
let payload = json!({ "id": id, "method": method, "params": params }).to_string();
|
||||
self.publish_payload(&client, &keys, &conversation, &peer, &payload)
|
||||
.await
|
||||
};
|
||||
if let Err(detail) = send_result {
|
||||
self.inner.lock().await.remote_pending.remove(&id);
|
||||
return Err(SigningError::Network { detail });
|
||||
}
|
||||
|
||||
match tokio::time::timeout(REQUEST_TIMEOUT, receiver).await {
|
||||
Ok(Ok(result)) => result.map_err(|detail| SigningError::Internal { detail }),
|
||||
Ok(Err(_)) => {
|
||||
// The waiter was dropped (disconnect/fail) while awaiting.
|
||||
Err(SigningError::NotConnected)
|
||||
}
|
||||
Err(_) => {
|
||||
// Abandon the waiter so a late response finds nothing.
|
||||
self.inner.lock().await.remote_pending.remove(&id);
|
||||
Err(SigningError::Timeout)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Main background task: connect to relays, subscribe, handle requests.
|
||||
async fn run_sign_task(self, uri: ConnectUri) -> Result<(), String> {
|
||||
let (conversation, keys) = {
|
||||
|
|
@ -534,6 +656,35 @@ impl Nip46ClientSigner {
|
|||
Err(_) => continue,
|
||||
};
|
||||
|
||||
// A payload carrying `result`/`error` is a response to a request
|
||||
// WE sent (e.g. `sign_event`), not an incoming signer request.
|
||||
// Deliver it to the waiting caller; unknown ids are ignored — a
|
||||
// stray response must never earn an error reply back to the
|
||||
// signer, and must never fall through to the request path.
|
||||
let shaped: serde_json::Value =
|
||||
serde_json::from_str(&plaintext).unwrap_or(serde_json::Value::Null);
|
||||
if shaped.get("result").is_some() || shaped.get("error").is_some() {
|
||||
let id = shaped.get("id").and_then(|v| v.as_str()).unwrap_or("");
|
||||
let outcome = if shaped.get("error").is_some() && !shaped["error"].is_null() {
|
||||
Err(shaped["error"]
|
||||
.as_str()
|
||||
.unwrap_or("The signer reported an error.")
|
||||
.to_string())
|
||||
} else {
|
||||
Ok(shaped["result"].as_str().unwrap_or("").to_string())
|
||||
};
|
||||
let waiter = self.inner.lock().await.remote_pending.remove(id);
|
||||
match waiter {
|
||||
Some(pending) => {
|
||||
let _ = pending.sender.send(outcome);
|
||||
}
|
||||
None => {
|
||||
eprintln!("Ignoring NIP-46 response with no waiting request: {id}");
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
let request: RawRequest = match serde_json::from_str(&plaintext) {
|
||||
Ok(r) => r,
|
||||
Err(_) => continue,
|
||||
|
|
@ -879,16 +1030,41 @@ impl Signer for Nip46ClientSigner {
|
|||
})
|
||||
}
|
||||
|
||||
async fn sign_event(&self, _event: UnsignedEvent) -> Result<Event, SigningError> {
|
||||
// For NIP-46 client, signing happens via the NIP-46 channel with user
|
||||
// approval. The actual flow uses request_approval +
|
||||
// respond_to_approval; this method exists to satisfy the object-safe
|
||||
// trait and fails closed if a caller tries to bypass it.
|
||||
Err(SigningError::Internal {
|
||||
detail:
|
||||
"NIP-46 signing uses the async approval flow; direct sign_event is not supported"
|
||||
.to_string(),
|
||||
})
|
||||
async fn sign_event(&self, event: UnsignedEvent) -> Result<Event, SigningError> {
|
||||
// The client half of NIP-46: ask the remote signer to sign, await the
|
||||
// encrypted response, and verify the returned event is exactly what we
|
||||
// asked for (identity + id + signature) before handing it back. A
|
||||
// misbehaving or MITM'd signer cannot swap content or keys.
|
||||
if !self.can_sign_event(event.kind.as_u16()) {
|
||||
self.audit_permission_denied("sign_event").await;
|
||||
return Err(SigningError::PermissionDenied {
|
||||
method: "sign_event".to_string(),
|
||||
});
|
||||
}
|
||||
let unsigned_json = event.try_as_json().map_err(|e| SigningError::Internal {
|
||||
detail: format!("Could not encode the event for signing: {e}"),
|
||||
})?;
|
||||
let response = self
|
||||
.send_remote_request("sign_event", vec![unsigned_json])
|
||||
.await?;
|
||||
let signed = Event::from_json(response.as_bytes()).map_err(|e| SigningError::Internal {
|
||||
detail: format!("The signer returned an unreadable event: {e}"),
|
||||
})?;
|
||||
// The signer must sign WITH the identity it is connected as...
|
||||
let signer_pubkey = Signer::get_public_key(self).await?;
|
||||
if signed.pubkey != signer_pubkey {
|
||||
return Err(SigningError::IdentityMismatch);
|
||||
}
|
||||
// ...the exact event we sent (same id covers pubkey, kind, tags,
|
||||
// content, timestamp)...
|
||||
if signed.id != event.compute_id() {
|
||||
return Err(SigningError::InvalidSignature);
|
||||
}
|
||||
// ...and with a cryptographically valid signature.
|
||||
signed
|
||||
.verify()
|
||||
.map_err(|_| SigningError::InvalidSignature)?;
|
||||
Ok(signed)
|
||||
}
|
||||
|
||||
fn get_signer_type(&self) -> SignerType {
|
||||
|
|
@ -1009,6 +1185,19 @@ fn response_ok(id: &str, result: String) -> String {
|
|||
json!({ "id": id, "result": result, "error": null }).to_string()
|
||||
}
|
||||
|
||||
/// Whether a stored connection is usable *right now* (not revoked, not
|
||||
/// expired), computed without taking the signer's async lock. The trait's
|
||||
/// `is_connection_valid` cannot be used from contexts that already hold it.
|
||||
fn connection_valid_now(conn: &Nip46Connection) -> bool {
|
||||
if conn.revoked_at.is_some() {
|
||||
return false;
|
||||
}
|
||||
match conn.expires_at {
|
||||
Some(expires_at) => crate::vault::unix_timestamp().unwrap_or(0) < expires_at,
|
||||
None => true,
|
||||
}
|
||||
}
|
||||
|
||||
fn response_err(id: &str, error: String) -> String {
|
||||
json!({ "id": id, "result": null, "error": error }).to_string()
|
||||
}
|
||||
|
|
|
|||
108
src/uploads.rs
108
src/uploads.rs
|
|
@ -1,21 +1,22 @@
|
|||
use base64::engine::general_purpose::STANDARD as B64;
|
||||
use base64::Engine;
|
||||
use nostr::nips::nip98::{HttpData, HttpMethod};
|
||||
use nostr_sdk::prelude::*;
|
||||
|
||||
use crate::crypto::VaultKey;
|
||||
use crate::errors::{AppError, ErrorKind};
|
||||
use crate::profiles;
|
||||
use crate::signer::Signing;
|
||||
|
||||
/// Sign a NIP-98 HTTP auth event for `url` with the active profile's key and
|
||||
/// return the `Authorization` header value (`Nostr <base64>`).
|
||||
/// Sign a NIP-98 HTTP auth event for `url` with the given [`Signing`] source
|
||||
/// and return the `Authorization` header value (`Nostr <base64>`).
|
||||
///
|
||||
/// This is what image hosts like nostr.build require before accepting an
|
||||
/// upload. Like publishing, it needs an unlocked vault when the vault is
|
||||
/// password-protected.
|
||||
/// upload. It follows the same signer selection as publishing: an embedded
|
||||
/// profile signs with the vault key, an external profile round-trips the
|
||||
/// auth event through its connected NIP-46 signer.
|
||||
pub async fn nip98_authorization(
|
||||
vault: &crate::vault::Vault,
|
||||
url: &str,
|
||||
method: &str,
|
||||
key: Option<&VaultKey>,
|
||||
signing: &Signing,
|
||||
) -> Result<String, AppError> {
|
||||
let http_method = match method.to_ascii_uppercase().as_str() {
|
||||
"GET" => HttpMethod::GET,
|
||||
|
|
@ -33,112 +34,57 @@ pub async fn nip98_authorization(
|
|||
let parsed_url = Url::parse(url)
|
||||
.map_err(|e| AppError::config(format!("The upload URL is not valid: {e}")))?;
|
||||
|
||||
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);
|
||||
|
||||
let header = HttpData::new(parsed_url, http_method)
|
||||
.to_authorization(&keys)
|
||||
// Build the same event HttpData::to_authorization would build (kind
|
||||
// 27235 with the u/method tags), but sign it through `Signing` so an
|
||||
// external profile never needs a local secret.
|
||||
let http_data = HttpData::new(parsed_url, http_method);
|
||||
let pubkey = signing.pubkey().await.map_err(AppError::from)?;
|
||||
let unsigned = IntoEventBuilder::into_event_builder(http_data).finalize_unsigned(pubkey);
|
||||
let event = signing
|
||||
.sign(unsigned)
|
||||
.await
|
||||
.map_err(|e| AppError::sign_failed(format!("Could not sign the upload request: {e}")))?;
|
||||
|
||||
Ok(header)
|
||||
let encoded = B64.encode(event.as_json());
|
||||
Ok(format!("Nostr {encoded}"))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::settings::Settings;
|
||||
use crate::vault::Vault;
|
||||
|
||||
/// Settings with no relays so tests never touch the network.
|
||||
fn offline_settings() -> Settings {
|
||||
Settings {
|
||||
relays: Vec::new(),
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
fn vault_with_profile() -> Vault {
|
||||
let mut vault = Vault::empty();
|
||||
crate::profiles::create_profile(&mut vault, "A".to_string(), None, &offline_settings())
|
||||
.unwrap();
|
||||
vault
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_profile_errors() {
|
||||
let vault = Vault::empty();
|
||||
let runtime = tokio::runtime::Runtime::new().unwrap();
|
||||
let err = runtime
|
||||
.block_on(nip98_authorization(
|
||||
&vault,
|
||||
"https://nostr.build/api/v2/upload/files",
|
||||
"POST",
|
||||
None,
|
||||
))
|
||||
.expect_err("no active profile must error");
|
||||
assert_eq!(err.kind(), ErrorKind::NoActiveProfile);
|
||||
fn local_signing() -> Signing {
|
||||
Signing::Local(Keys::generate())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_method_errors() {
|
||||
let vault = vault_with_profile();
|
||||
let signing = local_signing();
|
||||
let runtime = tokio::runtime::Runtime::new().unwrap();
|
||||
let err = runtime
|
||||
.block_on(nip98_authorization(
|
||||
&vault,
|
||||
"https://example.com/upload",
|
||||
"DELETE",
|
||||
None,
|
||||
&signing,
|
||||
))
|
||||
.expect_err("unsupported method must error");
|
||||
assert_eq!(err.kind(), ErrorKind::Config);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn locked_encrypted_vault_errors() {
|
||||
let mut vault = vault_with_profile();
|
||||
vault.crypto = Some(crate::vault::VaultCrypto {
|
||||
kdf: crate::vault::KdfParams {
|
||||
algorithm: "argon2id".to_string(),
|
||||
salt: "c2FsdA==".to_string(),
|
||||
m_cost: 1,
|
||||
t_cost: 1,
|
||||
p_cost: 1,
|
||||
},
|
||||
verifier: "dmVyaWZpZXI=".to_string(),
|
||||
});
|
||||
vault.profiles[0].secret_key = "encrypted-blob".to_string();
|
||||
let runtime = tokio::runtime::Runtime::new().unwrap();
|
||||
let err = runtime
|
||||
.block_on(nip98_authorization(
|
||||
&vault,
|
||||
"https://nostr.build/api/v2/upload/files",
|
||||
"POST",
|
||||
None,
|
||||
))
|
||||
.expect_err("locked vault must error");
|
||||
assert_eq!(err.kind(), ErrorKind::VaultLocked);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signs_a_nip98_auth_header_for_the_active_profile() {
|
||||
let vault = vault_with_profile();
|
||||
fn signs_a_nip98_auth_header() {
|
||||
let signing = local_signing();
|
||||
let runtime = tokio::runtime::Runtime::new().unwrap();
|
||||
let header = runtime
|
||||
.block_on(nip98_authorization(
|
||||
&vault,
|
||||
"https://nostr.build/api/v2/upload/files",
|
||||
"POST",
|
||||
None,
|
||||
&signing,
|
||||
))
|
||||
.expect("valid profile must sign");
|
||||
.expect("local signing must produce a header");
|
||||
|
||||
assert!(header.starts_with("Nostr "), "expected a Nostr auth header");
|
||||
let encoded = header.trim_start_matches("Nostr ").trim();
|
||||
use base64::engine::general_purpose::STANDARD as B64;
|
||||
use base64::Engine as _;
|
||||
let raw = B64
|
||||
.decode(encoded)
|
||||
.expect("the header payload must be base64");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue