From 5d02196e6a930558a874ff7d059e6f5bef325829 Mon Sep 17 00:00:00 2001 From: osmaczko <33099791+osmaczko@users.noreply.github.com> Date: Sat, 27 Jun 2026 13:58:05 +0200 Subject: [PATCH] fix: deliver DirectV1 Welcome to account-associated invitees (routing_id) DirectV1 over a shared key-package registry (HttpRegistry) could not deliver the MLS Welcome to an account-associated invitee. Key-package resolution requires the shared address to be the account key (the account directory resolves it to the device key HttpRegistry stores the package under), but the Welcome was routed to, and the invitee's InboxV2 subscribed and gated on, the credential id (hex of the DelegateCredential TLV). The two strings never matched, so the Welcome fell through the dispatch gate to PayloadOutcome::Empty and the invitee never joined. EphemeralRegistry hid this by keying key-packages on the credential id, collapsing both halves onto one string. Decouple the 1:1 routing identity from the credential identity: - Add a defaulted IdentityProvider::routing_id() (defaults to id()). - DelegateSigner returns its account address from routing_id() once associated. - Core::assemble feeds InboxV2 routing_id() instead of id(); the MLS credential, member id, sender id, and decode_sender keep reading id(), so MLS membership and sender attribution are unchanged. Add a regression test (direct_v1_associated_invitee_receives_welcome) over a DeviceKeyedRegistry that keys key-packages by the device verifying key, as the deployed HttpRegistry does; it fails without routing_id and passes with it. --- core/conversations/src/core.rs | 6 +- core/shared-traits/src/lib.rs | 7 ++ crates/client/src/delegate.rs | 9 +- crates/client/tests/saro_and_raya.rs | 125 ++++++++++++++++++++++++++- 4 files changed, 143 insertions(+), 4 deletions(-) diff --git a/core/conversations/src/core.rs b/core/conversations/src/core.rs index 7f2d5e4..bb85bb4 100644 --- a/core/conversations/src/core.rs +++ b/core/conversations/src/core.rs @@ -112,7 +112,11 @@ where store: CS, ) -> Result { let inbox = Inbox::new(&identity); - let ident_id = ident.id().clone(); + // InboxV2 (1:1) routes on routing_id, not the credential id: an + // account-associated delegate receives under its account address, which + // is what a peer resolves and addresses the Welcome to. The MLS identity + // below still wraps the raw `ident`, so the credential is unchanged. + let ident_id = ident.routing_id().clone(); let mls_identity = MlsIdentityProvider::new(ident); let mls_provider = MlsEphemeralPqProvider::new().map_err(ChatError::generic)?; let causal = CausalHistoryStore::new(); diff --git a/core/shared-traits/src/lib.rs b/core/shared-traits/src/lib.rs index 06316cb..13e9b29 100644 --- a/core/shared-traits/src/lib.rs +++ b/core/shared-traits/src/lib.rs @@ -36,4 +36,11 @@ pub trait IdentityProvider { fn display_name(&self) -> String; fn sign(&self, payload: &[u8]) -> Ed25519Signature; fn public_key(&self) -> &Ed25519VerifyingKey; + + /// Identifier this identity receives 1:1 (InboxV2) messages under; defaults + /// to [`id`](Self::id). An account-associated delegate overrides it so + /// routing keys on the account address, not the credential. + fn routing_id(&self) -> IdentIdRef<'_> { + self.id() + } } diff --git a/crates/client/src/delegate.rs b/crates/client/src/delegate.rs index c2cc712..98dbb42 100644 --- a/crates/client/src/delegate.rs +++ b/crates/client/src/delegate.rs @@ -13,6 +13,7 @@ pub struct DelegateSigner { signing_key: Ed25519SigningKey, verifying_key: Ed25519VerifyingKey, identifier: IdentId, + routing_id: IdentId, account_addr: Option, } @@ -21,10 +22,11 @@ impl DelegateSigner { pub fn random() -> Self { let signing_key = Ed25519SigningKey::generate(); let verifying_key = signing_key.verifying_key(); - let identifier = DelegateCredential::unassociated(&verifying_key).into(); + let identifier: IdentId = DelegateCredential::unassociated(&verifying_key).into(); Self { signing_key, verifying_key, + routing_id: identifier.clone(), identifier, account_addr: None, } @@ -34,6 +36,7 @@ impl DelegateSigner { pub fn associate(&mut self, account_addr: AccountAddr) { self.identifier = DelegateCredential::associated(&self.verifying_key, account_addr.as_str()).into(); + self.routing_id = IdentId::new(account_addr.clone()); self.account_addr = Some(account_addr); } @@ -47,6 +50,10 @@ impl IdentityProvider for DelegateSigner { &self.identifier } + fn routing_id(&self) -> libchat::IdentIdRef<'_> { + &self.routing_id + } + fn display_name(&self) -> String { trunc(self.identifier.as_str()) } diff --git a/crates/client/tests/saro_and_raya.rs b/crates/client/tests/saro_and_raya.rs index 091083c..0224d35 100644 --- a/crates/client/tests/saro_and_raya.rs +++ b/crates/client/tests/saro_and_raya.rs @@ -1,9 +1,14 @@ +use std::collections::HashMap; +use std::sync::{Arc, Mutex}; use std::time::Duration; use components::EphemeralRegistry; use crossbeam_channel::{Receiver, Sender}; use crypto::Ed25519VerifyingKey; -use libchat::{AccountDirectory, IdentityProvider, SignedDeviceBundle, encode_bundle_payload}; +use libchat::{ + AccountDirectory, DeviceSet, IdentityProvider, RegistrationService, SignedDeviceBundle, + encode_bundle_payload, verify_bundle, +}; use logos_account::TestLogosAccount; use logos_chat::{ AddressedEnvelope, ChatClient, ChatClientBuilder, DelegateSigner, DeliveryService, Event, @@ -13,7 +18,7 @@ use logos_chat::{ /// Publish a signed device bundle endorsing `device` as a device of `account`, /// so a receiver can verify the sender's account → device mapping. fn publish_device_bundle( - reg: &mut EphemeralRegistry, + reg: &mut impl AccountDirectory, account: &TestLogosAccount, device: &Ed25519VerifyingKey, ) { @@ -146,6 +151,122 @@ fn direct_v1_standalone_integration() { }); } +/// Test registry that keys keypackages by `hex(public_key())`, exactly as the +/// deployed `HttpRegistry` does (the server keys by the device verifying key for +/// proof-of-possession). [`EphemeralRegistry`] instead keys by `id()`, which +/// collapses the account/credential split and so cannot exercise an associated +/// invitee: the account directory resolves a shared account address to a device +/// key, and the keypackage must be retrievable under that device key. +#[derive(Clone, Default)] +struct DeviceKeyedRegistry { + key_packages: Arc>>>, + installations: Arc>>, +} + +impl std::fmt::Debug for DeviceKeyedRegistry { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("DeviceKeyedRegistry") + } +} + +impl RegistrationService for DeviceKeyedRegistry { + type Error = String; + + fn register( + &mut self, + identity: &dyn IdentityProvider, + key_bundle: Vec, + ) -> Result<(), String> { + self.key_packages + .lock() + .unwrap() + .insert(hex::encode(identity.public_key().as_ref()), key_bundle); + Ok(()) + } + + fn retrieve(&self, device_id: &str) -> Result>, String> { + Ok(self.key_packages.lock().unwrap().get(device_id).cloned()) + } +} + +impl AccountDirectory for DeviceKeyedRegistry { + type Error = String; + + fn publish(&mut self, bundle: &SignedDeviceBundle) -> Result<(), String> { + self.installations + .lock() + .unwrap() + .insert(hex::encode(bundle.account_pub.as_ref()), bundle.clone()); + Ok(()) + } + + fn fetch(&self, account: &Ed25519VerifyingKey) -> Result, String> { + let Some(bundle) = self + .installations + .lock() + .unwrap() + .get(&hex::encode(account.as_ref())) + .cloned() + else { + return Ok(None); + }; + verify_bundle(account, &bundle) + .map(Some) + .map_err(|e| e.to_string()) + } +} + +/// Regression test for the account-flow Welcome-routing fix (`routing_id`). The +/// invitee (raya) is an account-associated delegate, so the address the inviter +/// shares is raya's *account* key. Before the fix raya's InboxV2 subscribed and +/// gated on her *credential* id, so the Welcome — routed to the account address — +/// never reached her; with `routing_id` she routes on her account address, so it +/// lands and she joins. Needs a `hex(public_key())`-keyed registry because the +/// account directory resolves raya's account to her device key. +#[test] +fn direct_v1_associated_invitee_receives_welcome() { + let bus = MessageBus::default(); + let mut reg = DeviceKeyedRegistry::default(); + + let raya_account = TestLogosAccount::new("Raya"); + let raya_account_id = hex::encode(raya_account.public_key().as_ref()); + let mut raya_delegate = DelegateSigner::random(); + raya_delegate.associate(raya_account_id.clone()); + publish_device_bundle(&mut reg, &raya_account, raya_delegate.public_key()); + + let (mut saro, _saro_events) = ChatClientBuilder::new() + .transport(InProcessDelivery::new(bus.clone())) + .registration(reg.clone()) + .build() + .expect("client create"); + let (raya, raya_events) = ChatClientBuilder::new() + .ident(raya_delegate) + .transport(InProcessDelivery::new(bus.clone())) + .registration(reg.clone()) + .build() + .expect("client create"); + + // Raya is addressed by her account id (routing_id), not her credential TLV. + assert_eq!(raya.addr(), raya_account_id.as_str()); + + // Saro opens the conversation with raya's account address, shared out of band. + let convo_id = saro.create_direct_conversation(&raya_account_id).unwrap(); + + expect_event(&raya_events, "ConversationStarted", |e| match e { + Event::ConversationStarted { .. } => Ok(()), + other => Err(other), + }); + + saro.send_message(&convo_id, b"hey raya").unwrap(); + expect_event(&raya_events, "MessageReceived", |e| match e { + Event::MessageReceived { content, .. } => { + assert_eq!(content.as_slice(), b"hey raya"); + Ok(()) + } + other => Err(other), + }); +} + #[test] fn saro_raya_message_exchange() { let bus = MessageBus::default();