mirror of
https://github.com/logos-messaging/libchat.git
synced 2026-07-03 06:30:06 +00:00
fix: subscribe InboxV2 on routing_id, not the credential id
InboxV2 gated inbound on the credential id (hex of the DelegateCredential TLV), but a sender addresses the Welcome to the invitee's routing address, which the account directory resolves to the device key HttpRegistry stores the key-package under. The two differ, so the Welcome fell through to PayloadOutcome::Empty and the invitee never joined. EphemeralRegistry hid this by keying key-packages on the credential id. This is routing-vs-credential, not account-vs-device: the TLV-wrapped credential differs from the address even when account key == device key (testnet today), so a client associating over HttpRegistry hits it; only the in-process EphemeralRegistry path is unaffected. Add IdentityProvider::routing_id() (defaults to id()); DelegateSigner derives it from its credential (the account address once associated, else id()); Core::assemble subscribes InboxV2 on routing_id(). id() still backs the MLS credential, member id, sender id, and decode_sender, so membership and attribution are unchanged. Regression test direct_v1_welcome_routes_on_routing_id over a device-key-keyed registry (as HttpRegistry) fails without the fix.
This commit is contained in:
parent
ebae3317d6
commit
2321e4cb6d
@ -112,7 +112,11 @@ where
|
||||
store: CS,
|
||||
) -> Result<Self, ChatError> {
|
||||
let inbox = Inbox::new(&identity);
|
||||
let ident_id = ident.id().clone();
|
||||
// InboxV2 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();
|
||||
let mls_identity = MlsIdentityProvider::new(ident);
|
||||
let mls_provider = MlsEphemeralPqProvider::new().map_err(ChatError::generic)?;
|
||||
let causal = CausalHistoryStore::new();
|
||||
|
||||
@ -36,4 +36,12 @@ pub trait IdentityProvider {
|
||||
fn display_name(&self) -> String;
|
||||
fn sign(&self, payload: &[u8]) -> Ed25519Signature;
|
||||
fn public_key(&self) -> &Ed25519VerifyingKey;
|
||||
|
||||
/// Identifier this identity's InboxV2 receives MLS Welcomes (GroupV1 and
|
||||
/// GroupV2 invites) 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) -> IdentId {
|
||||
self.id().clone()
|
||||
}
|
||||
}
|
||||
|
||||
@ -13,7 +13,6 @@ pub struct DelegateSigner {
|
||||
signing_key: Ed25519SigningKey,
|
||||
verifying_key: Ed25519VerifyingKey,
|
||||
identifier: IdentId,
|
||||
account_addr: Option<AccountAddr>,
|
||||
}
|
||||
|
||||
impl DelegateSigner {
|
||||
@ -21,12 +20,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,
|
||||
identifier,
|
||||
account_addr: None,
|
||||
}
|
||||
}
|
||||
|
||||
@ -34,11 +32,12 @@ impl DelegateSigner {
|
||||
pub fn associate(&mut self, account_addr: AccountAddr) {
|
||||
self.identifier =
|
||||
DelegateCredential::associated(&self.verifying_key, account_addr.as_str()).into();
|
||||
self.account_addr = Some(account_addr);
|
||||
}
|
||||
|
||||
pub fn account_addr(&self) -> Option<&str> {
|
||||
self.account_addr.as_deref()
|
||||
pub fn account_addr(&self) -> Option<String> {
|
||||
DelegateCredential::try_from(self.identifier.clone())
|
||||
.ok()
|
||||
.and_then(|c| c.account_addr().map(String::from))
|
||||
}
|
||||
}
|
||||
|
||||
@ -47,6 +46,12 @@ impl IdentityProvider for DelegateSigner {
|
||||
&self.identifier
|
||||
}
|
||||
|
||||
fn routing_id(&self) -> IdentId {
|
||||
self.account_addr()
|
||||
.map(IdentId::new)
|
||||
.unwrap_or_else(|| self.identifier.clone())
|
||||
}
|
||||
|
||||
fn display_name(&self) -> String {
|
||||
trunc(self.identifier.as_str())
|
||||
}
|
||||
@ -298,4 +303,24 @@ mod tests {
|
||||
Err(ClientError::BadlyFormedCredential)
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unassociated_routing_id_equals_id() {
|
||||
let signer = DelegateSigner::random();
|
||||
assert!(signer.account_addr().is_none());
|
||||
let id = signer.id().clone();
|
||||
assert_eq!(signer.routing_id(), id);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn associated_routing_id_derives_account_addr() {
|
||||
let addr = "alice@libchat.example";
|
||||
let mut signer = DelegateSigner::random();
|
||||
signer.associate(addr.to_string());
|
||||
assert_eq!(signer.account_addr().as_deref(), Some(addr));
|
||||
assert_eq!(signer.routing_id(), IdentId::new(addr));
|
||||
// id() stays the full credential, distinct from the routing address.
|
||||
let id = signer.id().clone();
|
||||
assert_ne!(signer.routing_id(), id);
|
||||
}
|
||||
}
|
||||
|
||||
@ -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<Error = String>,
|
||||
account: &TestLogosAccount,
|
||||
device: &Ed25519VerifyingKey,
|
||||
) {
|
||||
@ -146,6 +151,123 @@ 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<Mutex<HashMap<String, Vec<u8>>>>,
|
||||
installations: Arc<Mutex<HashMap<String, SignedDeviceBundle>>>,
|
||||
}
|
||||
|
||||
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<u8>,
|
||||
) -> 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<Option<Vec<u8>>, 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<Option<DeviceSet>, 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 `routing_id` fix: InboxV2 must subscribe on the
|
||||
/// routing address, not the credential id. The inviter shares raya's routing
|
||||
/// address (her account key); the account directory resolves it to her device
|
||||
/// key, under which the registry stores her key-package. Before the fix raya's
|
||||
/// InboxV2 gated on her *credential* id, so the Welcome, addressed to the routing
|
||||
/// address, never reached her; with the fix she subscribes on the routing address
|
||||
/// and joins. Needs a `hex(public_key())`-keyed registry (as the deployed
|
||||
/// HttpRegistry is) so the routing address and the credential id are distinct.
|
||||
#[test]
|
||||
fn direct_v1_welcome_routes_on_routing_id() {
|
||||
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();
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user