nip4e: fix message encryption and sync #57

Merged
reya merged 10 commits from fix-nip4e into master 2026-10-04 06:33:33 +00:00
2 changed files with 171 additions and 51 deletions
Showing only changes of commit bdf9249eab - Show all commits
+158 -49
View File
@@ -324,6 +324,9 @@ impl DeviceRegistry {
.finalize_async(&signer)
.await?;
// Persist the key locally before announcing it
set_keys(&client, &signer, &secret).await?;
// Publish announcement
client
.send_event(&event)
@@ -331,9 +334,6 @@ impl DeviceRegistry {
.ack_policy(AckPolicy::none())
.await?;
// Save device keys to the database
set_keys(&client, &signer, &secret).await?;
Ok(keys)
})
}
@@ -384,11 +384,8 @@ impl DeviceRegistry {
let public_key = signer.get_public_key_async().await?;
let id = SubscriptionId::new("dekey-requests");
// Construct a filter for encryption key requests
let filter = Filter::new()
.kind(Kind::Custom(4454))
.author(public_key)
.since(Timestamp::now());
// Construct a filter for encryption key requests.
let filter = Filter::new().kind(Kind::Custom(4454)).author(public_key);
// Subscribe to the device key requests on user's write relays
client.subscribe(vec![filter]).with_id(id).await?;
@@ -417,26 +414,30 @@ impl DeviceRegistry {
.pubkey(app_pubkey)
.limit(1);
match client.database().query(filter).await?.into_iter().next() {
// Found an approval event
Some(event) => Ok(Some(event)),
// No approval event found, construct a request event
None => {
// Construct an event for device key request
let event = EventBuilder::new(Kind::Custom(4454), "")
.tags(vec![
Tag::custom("P", vec![app_pubkey]),
Tag::custom("client", vec![CLIENT_NAME]),
])
.finalize_async(&signer)
.await?;
// Send the event to write relays
client.send_event(&event).to_nip65().await?;
Ok(None)
if let Some(event) = client.database().query(filter).await?.into_iter().next() {
// Only reuse the stored approval if its key is still valid,
// otherwise it's stale and a fresh request is needed
if decrypt_approval(&client, &app_keys, public_key, &event)
.await
.is_ok()
{
return Ok(Some(event));
}
}
// No usable approval event found, construct a request event
let event = EventBuilder::new(Kind::Custom(4454), "")
.tags(vec![
Tag::custom("P", vec![app_pubkey]),
Tag::custom("client", vec![CLIENT_NAME]),
])
.finalize_async(&signer)
.await?;
// Send the event to write relays
client.send_event(&event).to_nip65().await?;
Ok(None)
});
self.tasks.push(cx.spawn(async move |this, cx| {
@@ -466,20 +467,41 @@ impl DeviceRegistry {
let nostr = NostrRegistry::global(cx);
let client = nostr.read(cx).client();
let signer = nostr.read(cx).signer();
let app_keys_task = get_or_init_app_keys(cx);
cx.emit(DeviceEvent::Requesting);
self.tasks.push(cx.background_spawn(async move {
let public_key = signer.get_public_key_async().await?;
self.tasks.push(cx.spawn(async move |this, cx| {
let app_keys = match app_keys_task.await {
Ok(app_keys) => app_keys,
Err(e) => {
this.update(cx, |_this, cx| {
cx.emit(DeviceEvent::error(e.to_string()));
})?;
return Ok(());
}
};
// Construct a filter for device key requests
let filter = Filter::new()
.kind(Kind::Custom(4455))
.author(public_key)
.since(Timestamp::now());
let task: Task<Result<(), Error>> = cx.background_spawn(async move {
let public_key = signer.get_public_key_async().await?;
// Subscribe to the device key requests on user's write relays
client.subscribe(filter).await?;
// Only approvals addressed to this device's app key matter
let filter = Filter::new()
.kind(Kind::Custom(4455))
.author(public_key)
.pubkey(app_keys.public_key());
// Subscribe to the device key approvals on user's write relays
client.subscribe(filter).await?;
Ok(())
});
if let Err(e) = task.await {
this.update(cx, |_this, cx| {
cx.emit(DeviceEvent::error(e.to_string()));
})?;
}
Ok(())
}));
@@ -488,22 +510,16 @@ impl DeviceRegistry {
/// Parse the approval event to get encryption key then set it
fn extract_encryption(&mut self, event: Event, cx: &mut Context<Self>) {
let app_keys_task = get_or_init_app_keys(cx);
let nostr = NostrRegistry::global(cx);
let client = nostr.read(cx).client();
let signer = nostr.read(cx).signer();
let approval_id = event.id;
let task: Task<Result<Keys, Error>> = cx.background_spawn(async move {
let app_keys = app_keys_task.await?;
let master = event
.tags
.iter()
.find(|tag| tag.kind() == "P")
.and_then(|tag| tag.content())
.and_then(|content| PublicKey::parse(content).ok())
.context("Invalid event's tags")?;
let payload = event.content.as_str();
let decrypted = app_keys.nip44_decrypt_async(&master, payload).await?;
let secret = SecretKey::from_hex(&decrypted)?;
let keys = Keys::new(secret);
let identity = signer.get_public_key_async().await?;
let keys = decrypt_approval(&client, &app_keys, identity, &event).await?;
Ok(keys)
});
@@ -513,6 +529,8 @@ impl DeviceRegistry {
Ok(keys) => {
this.update(cx, |this, cx| {
this.set_signer(keys, cx);
// Clean up our request and the approval event from relays
this.cleanup_sync_events(approval_id, cx);
})?;
}
Err(e) => {
@@ -525,6 +543,47 @@ impl DeviceRegistry {
}));
}
/// Delete our key request and the consumed approval event from relays
fn cleanup_sync_events(&mut self, approval_id: EventId, cx: &mut Context<Self>) {
let nostr = NostrRegistry::global(cx);
let client = nostr.read(cx).client();
let signer = nostr.read(cx).signer();
self.tasks.push(cx.background_spawn(async move {
let result: Result<(), Error> = async {
let public_key = signer.get_public_key_async().await?;
// Collect our key request events
let filter = Filter::new().kind(Kind::Custom(4454)).author(public_key);
let mut ids: Vec<EventId> = client
.database()
.query(filter)
.await?
.into_iter()
.map(|event| event.id)
.collect();
ids.push(approval_id);
let deletion = EventBuilder::new(Kind::EventDeletion, "")
.tags(Tags::from_list(ids.into_iter().map(Tag::event).collect()))
.finalize_async(&signer)
.await?;
client.send_event(&deletion).to_nip65().await?;
Ok(())
}
.await;
if let Err(e) = result {
log::error!("Failed to delete sync events: {e}");
}
Ok(())
}));
}
/// Approve requests for device keys from other devices
fn approve(&mut self, event: &Event, window: &mut Window, cx: &mut Context<Self>) {
let nostr = NostrRegistry::global(cx);
@@ -570,9 +629,14 @@ impl DeviceRegistry {
Ok(())
});
self.tasks.push(cx.spawn_in(window, async move |_this, cx| {
self.tasks.push(cx.spawn_in(window, async move |this, cx| {
match task.await {
Ok(_) => {
this.update(cx, |this, cx| {
// Allow future requests to surface again
this.set_pending_request(false, cx);
})?;
cx.update(|window, cx| {
window.clear_notification_by_id::<DeviceNotification>(id, cx);
})
@@ -758,6 +822,51 @@ fn get_or_init_app_keys(cx: &App) -> Task<Result<Keys, Error>> {
})
}
/// Decrypt and validate an approval event against the current announcement.
async fn decrypt_approval(
client: &Client,
app_keys: &Keys,
identity: PublicKey,
event: &Event,
) -> Result<Keys, Error> {
let master = event
.tags
.iter()
.find(|tag| tag.kind() == "P")
.and_then(|tag| tag.content())
.and_then(|content| PublicKey::parse(content).ok())
.context("Invalid event's tags")?;
let decrypted = app_keys
.nip44_decrypt_async(&master, event.content.as_str())
.await?;
let secret = SecretKey::from_hex(&decrypted)?;
let keys = Keys::new(secret);
// Reject approvals carrying a key that no longer matches the published announcement.
let filter = Filter::new()
.kind(Kind::Custom(10044))
.author(identity)
.limit(1);
if let Some(announcement) = client.database().query(filter).await?.into_iter().next() {
let announced = announcement
.tags
.iter()
.find(|tag| tag.kind() == "n")
.and_then(|tag| tag.content())
.and_then(|content| PublicKey::parse(content).ok())
.context("Announcement has no valid encryption key")?;
if keys.public_key() != announced {
return Err(anyhow!("Approval key doesn't match the announcement"));
}
}
Ok(keys)
}
/// Encrypt and store device keys in the local database.
async fn set_keys<S>(client: &Client, signer: &S, secret: &str) -> Result<(), Error>
where
+13 -2
View File
@@ -1,9 +1,20 @@
# NIP-4e Audit Report
> **Update**: The P0 findings (C1, C2, C3) are now fixed — seals are always signed by the
> **Update**: The P0 findings (C1, C2, C3) are fixed — seals are always signed by the
> identity key, `extract_rumor` falls back to the device key, the `DEVICE_GIFTWRAP`
> subscription is created, and trashed wraps are retried on `DeviceEvent::Set`.
> The remaining findings (F1–F8, P2 items, UX bugs) are still open.
>
> **Update 2**: The P1 sync findings are fixed — F1 (4455 approvals now filtered by
> the requester's `p` tag so the approver no longer processes its own response), F2
> (decrypted keys are validated against the current 10044 announcement via
> `decrypt_approval`), F3 (`since(now)` removed so offline requests/approvals
> backfill), F4 (`pending_request` resets after an approval completes), F5 (a NIP-09
> deletion now removes the 4454 requests and the consumed 4455 after a successful
> sync), F6 (stale stored 4455s no longer short-circuit `request()` — they are
> validated first and a fresh request is published when invalid), F8 (device keys
> are saved locally *before* the 10044 announcement is published, closing the
> echo/self-request race). Still open: the P2 state-hygiene items and the UX bugs
> in section 5.
**Scope**: `crates/nip4e/src/lib.rs` (key announcement/sync), the send path (`crates/chat/src/room.rs`), the receive path (`crates/chat/src/lib.rs`), UI integration (`crates/workspace`), compared against the [NIP-4e draft](https://github.com/nostr-protocol/nips/blob/per-device-keys/4e.md) and [PsstPsst's implementation](https://github.com/CodyTseng/psstpsst) (`docs/protocols/nip4e.md` + `docs/protocols/nip17-extensions.md`, plus rust-nostr's NIP-59 source to verify crypto mechanics).