diff --git a/crates/nip4e/src/lib.rs b/crates/nip4e/src/lib.rs index be189731..b08ffe52 100644 --- a/crates/nip4e/src/lib.rs +++ b/crates/nip4e/src/lib.rs @@ -39,7 +39,7 @@ pub enum DeviceEvent { /// User have not setup encryption key NotSet, /// The device is requesting an encryption key - Requesting, + Requesting(SharedString), /// An error occurred Error(SharedString), } @@ -319,10 +319,7 @@ impl DeviceRegistry { cx.background_spawn(async move { // Construct an announcement event let event = EventBuilder::new(Kind::Custom(10044), "") - .tags(vec![ - Tag::custom("n", vec![n]), - Tag::custom("client", vec![CLIENT_NAME]), - ]) + .tag(Tag::custom("n", vec![n])) .finalize_async(&signer) .await?; @@ -433,10 +430,7 @@ impl DeviceRegistry { // 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]), - ]) + .tags(vec![Tag::custom("P", vec![app_pubkey])]) .finalize_async(&signer) .await?; @@ -475,8 +469,6 @@ impl DeviceRegistry { let signer = nostr.read(cx).signer(); let app_keys_task = get_or_init_app_keys(cx); - cx.emit(DeviceEvent::Requesting); - self.tasks.push(cx.spawn(async move |this, cx| { let app_keys = match app_keys_task.await { Ok(app_keys) => app_keys, @@ -488,6 +480,12 @@ impl DeviceRegistry { } }; + this.update(cx, |_this, cx| { + cx.emit(DeviceEvent::Requesting(authorization_code( + &app_keys.public_key(), + ))); + })?; + let task: Task> = cx.background_spawn(async move { let public_key = signer.get_public_key_async().await?; @@ -694,7 +692,9 @@ impl DeviceRegistry { /// Build a notification for the encryption request. fn notification(&self, event: Event, cx: &Context) -> Result { const MSG: &str = "You've requested an encryption key from another device. \ - Approve to allow Coop to share with it."; + Approve to allow Coop to share with it. Before approving, \ + compare the code with the one shown on the requesting \ + device."; let requester = event .tags @@ -704,16 +704,10 @@ impl DeviceRegistry { .and_then(|content| PublicKey::parse(content).ok()) .context("Request has no valid requester key")?; - let client_name = event - .tags - .iter() - .find(|tag| tag.kind() == "client") - .and_then(|tag| tag.content()) - .map(|c| c.to_string()); - - let request = Announcement::new(requester, client_name); + let code = authorization_code(&requester); let persons = PersonRegistry::global(cx); let profile = persons.read(cx).get(&requester, cx); + let code_label = code.clone(); let entity = cx.entity().downgrade(); let loading = Rc::new(Cell::new(false)); @@ -776,7 +770,7 @@ impl DeviceRegistry { .font_semibold() .text_xs() .text_color(cx.theme().text_muted) - .child(SharedString::from("Client:")), + .child(SharedString::from("Code:")), ) .child( div() @@ -785,7 +779,7 @@ impl DeviceRegistry { .px_2() .rounded(cx.theme().radius) .bg(cx.theme().elevated_surface_background) - .child(request.client_name()), + .child(code_label.clone()), ), ), ) @@ -817,6 +811,13 @@ impl DeviceRegistry { struct DeviceNotification; +/// The first eight hexadecimal characters of the client public key. +fn authorization_code(public_key: &PublicKey) -> SharedString { + let hex = public_key.to_hex(); + let code = hex[..8].to_uppercase(); + SharedString::from(format!("{} {}", &code[..4], &code[4..])) +} + /// Get or create new app keys (async, returns a task) fn get_or_init_app_keys(cx: &App) -> Task> { let read = cx.read_credentials(CLIENT_NAME); diff --git a/crates/state/src/nip4e.rs b/crates/state/src/nip4e.rs index dbb18804..179e863f 100644 --- a/crates/state/src/nip4e.rs +++ b/crates/state/src/nip4e.rs @@ -1,4 +1,3 @@ -use gpui::SharedString; use nostr_sdk::prelude::*; /// Announcement @@ -6,12 +5,12 @@ use nostr_sdk::prelude::*; pub struct Announcement { /// The public key of the device that created this announcement. public_key: PublicKey, - - /// The name of the device that created this announcement. - client_name: Option, } impl Announcement { + /// Parse an announcement from an event, returning `None` when the `n` + /// tag is missing or malformed so callers never mistake the author's + /// identity key for an encryption key. pub fn from_event(event: &Event) -> Option { let public_key = event .tags @@ -20,33 +19,15 @@ impl Announcement { .and_then(|tag| tag.content()) .and_then(|c| PublicKey::parse(c).ok())?; - let client_name = event - .tags - .iter() - .find(|tag| tag.kind() == "client") - .and_then(|tag| tag.content()) - .map(|c| c.to_string()); - - Some(Self::new(public_key, client_name)) + Some(Self { public_key }) } - pub fn new(public_key: PublicKey, client_name: Option) -> Self { - Self { - public_key, - client_name, - } + pub fn new(public_key: PublicKey) -> Self { + Self { public_key } } /// Returns the public key of the device that created this announcement. pub fn public_key(&self) -> PublicKey { self.public_key } - - /// Returns the client name of the device that created this announcement. - pub fn client_name(&self) -> SharedString { - self.client_name - .as_ref() - .map(SharedString::from) - .unwrap_or(SharedString::from("Unknown")) - } } diff --git a/crates/workspace/src/lib.rs b/crates/workspace/src/lib.rs index f4ecac10..8135dd4e 100644 --- a/crates/workspace/src/lib.rs +++ b/crates/workspace/src/lib.rs @@ -106,14 +106,15 @@ impl Workspace { // Observe all events emitted by the device registry cx.subscribe_in(&device, window, |_this, _device, event, window, cx| { match event { - DeviceEvent::Requesting => { - const MSG: &str = - "Please open other client and approve the request for encryption key."; + DeviceEvent::Requesting(code) => { + const MSG: &str = "Please open another client and approve the request \ + for the encryption key. Compare the code with the \ + one shown there before approving."; let note = Notification::new() .id::() .autohide(false) - .title("Wait for approval") + .title(format!("Wait for approval · {code}")) .message(MSG) .with_kind(NotificationKind::Info); diff --git a/docs/nip4e-audit.md b/docs/nip4e-audit.md index 99573565..0eb70499 100644 --- a/docs/nip4e-audit.md +++ b/docs/nip4e-audit.md @@ -23,9 +23,13 @@ > pubkey, the 4454 notification parses the requester key from the `P` tag, the > NotSet "Retry" button re-checks for an announcement instead of silently > generating (rotating) a new encryption key, and the restore dialog's -> inverted empty-key check is fixed. Still open: the remaining security notes -> in section 4 (plaintext backup file, one-click approval without a -> verification code). +> inverted empty-key check is fixed. Additionally, the `client` tag and +> `client_name` were dropped from announcements/requests, and the PsstPsst +> authorization code was adapted: both sides display the first 8 hex chars of +> the requester client pubkey (`XXXX XXXX`) — the approver in the request +> notification, the requester in the "Wait for approval" notification — so the +> transfer can be verified out of band before approving. Still open: the +> plaintext backup file in section 4. **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).