diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index fee12a9..13c5449 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -13,7 +13,7 @@ use storage::ChatStore; use crate::delegate::DelegateCredential; use crate::errors::ClientError; -use crate::event::Event; +use crate::event::{Event, MessageSender}; type ClientCore = Core<(I, T, R, ThreadedWakeupService, S)>; type AccountAddressRef<'a> = &'a str; @@ -257,47 +257,73 @@ fn account_key_from_hex(addr: &str) -> Option { Ed25519VerifyingKey::from_bytes(&bytes).ok() } -/// Whether to surface a received message, given its sender credential checked -/// against the account → device directory (our account store). +/// Why a message's sender could not be accepted, so the message is dropped. +#[derive(Debug, PartialEq, Eq)] +enum SenderError { + /// No credential at all, so no sender can be attributed. Every delivered + /// message must carry an explicit sender. + Missing, + /// Credential bytes were not valid hex. + NotHex, + /// Credential bytes did not decode to a delegate credential. + Malformed, + /// The claimed account address is not an Ed25519 verifying key. + AccountNotAKey, + /// The account → device mapping is wrong or could not be confirmed: the + /// device is not in the account's published set, the account published none, + /// or the directory lookup failed. + Unverified, +} + +/// Decode and verify a message's sender from its credential, checked against the +/// account → device directory (our account store). /// -/// The credential binds a delegate device key to an optional account address. -/// When it claims an account, that account's published device set must include -/// this device — otherwise the account→device mapping is wrong or unconfirmable -/// and the message is dropped (`false`). A credential that claims no account (or -/// no credential at all) asserts no mapping, so it is delivered (`true`). -fn should_deliver(directory: &impl AccountDirectory, encoded: &[u8]) -> bool { - // No credential (e.g. the PrivateV1 placeholder) asserts no account mapping. +/// `Ok(sender)` — deliver with the sender; its `account` is set only when the +/// directory confirmed the device, so it is always verified. `Err` — drop the +/// message (including when no credential is present, since every delivered +/// message must carry an explicit sender). +fn decode_sender( + directory: &impl AccountDirectory, + encoded: &[u8], +) -> Result { + // No credential at all: there is no sender to attribute, so drop it. if encoded.is_empty() { - return true; + return Err(SenderError::Missing); } let Ok(data) = hex::decode(encoded) else { tracing::warn!("sender credential is not valid hex; dropping message"); - return false; + return Err(SenderError::NotHex); }; let cred = match DelegateCredential::try_from(data) { Ok(cred) => cred, Err(_) => { tracing::warn!("malformed sender credential; dropping message"); - return false; + return Err(SenderError::Malformed); } }; let device = hex::encode(cred.delegate_id().as_ref()); // An unassociated delegate asserts no account → device mapping. let Some(account_addr) = cred.account_addr() else { - return true; + return Ok(MessageSender { + account: None, + local_identity: IdentId::new(device), + }); }; let Some(account_key) = account_key_from_hex(account_addr) else { tracing::warn!( account_addr, "sender account address is not a verifying key; dropping message" ); - return false; + return Err(SenderError::AccountNotAKey); }; match directory.fetch(&account_key) { - Ok(Some(set)) if set.devices.iter().any(|d| d == &device) => true, + Ok(Some(set)) if set.devices.iter().any(|d| d == &device) => Ok(MessageSender { + account: Some(IdentId::new(account_addr.to_string())), + local_identity: IdentId::new(device), + }), _ => { tracing::warn!(account_addr, %device, "account → device mapping is wrong or unconfirmable; dropping message"); - false + Err(SenderError::Unverified) } } } @@ -305,10 +331,13 @@ fn should_deliver(directory: &impl AccountDirectory, encoded: &[u8]) -> bool { fn convo_events(outcome: ConvoOutcome, directory: &impl AccountDirectory) -> Vec { let ConvoOutcome { convo_id, content } = outcome; content - .filter(|c| should_deliver(directory, &c.encoded_credential)) - .map(|c| Event::MessageReceived { - convo_id: Arc::from(convo_id), - content: c.bytes, + .and_then(|c| { + let sender = decode_sender(directory, &c.encoded_credential).ok()?; + Some(Event::MessageReceived { + convo_id: Arc::from(convo_id), + content: c.bytes, + sender, + }) }) .into_iter() .collect() @@ -326,11 +355,12 @@ fn inbox_events(outcome: InboxOutcome, directory: &impl AccountDirectory) -> Vec class: new_conversation.class, }); if let Some(c) = initial.and_then(|co| co.content) - && should_deliver(directory, &c.encoded_credential) + && let Ok(sender) = decode_sender(directory, &c.encoded_credential) { events.push(Event::MessageReceived { convo_id: Arc::clone(&id), content: c.bytes, + sender, }); } events @@ -341,9 +371,9 @@ mod sender_check_tests { use std::collections::HashMap; use crypto::{Ed25519SigningKey, Ed25519VerifyingKey}; - use libchat::{DeviceSet, SignedDeviceBundle}; + use libchat::{DeviceSet, IdentId, SignedDeviceBundle}; - use super::should_deliver; + use super::{MessageSender, SenderError, decode_sender}; use crate::delegate::DelegateCredential; /// In-test account → device directory. Holds device id sets keyed by the hex @@ -400,15 +430,25 @@ mod sender_check_tests { hex::encode(cred.serialize()).into_bytes() } + fn local_id(k: &Ed25519VerifyingKey) -> IdentId { + IdentId::new(hex::encode(k.as_ref())) + } + /// The account published a device set that includes the sending device — the - /// claim checks out, so the message is delivered. + /// claim checks out, so the message is delivered with a verified account. #[test] - fn verified_sender_is_delivered() { + fn verified_sender_surfaces_account_and_device() { let account = key(); let device = key(); let dir = FakeDir::with_devices(&account, &[&device]); let cred = DelegateCredential::associated(&device, &hex::encode(account.as_ref())); - assert!(should_deliver(&dir, &encoded(cred))); + assert_eq!( + decode_sender(&dir, &encoded(cred)), + Ok(MessageSender { + account: Some(local_id(&account)), + local_identity: local_id(&device), + }) + ); } /// The account published a device set that does NOT include the sending @@ -420,15 +460,25 @@ mod sender_check_tests { let spoofer = key(); let dir = FakeDir::with_devices(&account, &[&endorsed]); let cred = DelegateCredential::associated(&spoofer, &hex::encode(account.as_ref())); - assert!(!should_deliver(&dir, &encoded(cred))); + assert_eq!( + decode_sender(&dir, &encoded(cred)), + Err(SenderError::Unverified) + ); } - /// A delegate that claims no account makes no mapping to contradict. + /// A delegate that claims no account surfaces its device but no account. #[test] - fn unassociated_sender_is_delivered() { + fn unassociated_sender_surfaces_device_only() { let dir = FakeDir::default(); - let cred = DelegateCredential::unassociated(&key()); - assert!(should_deliver(&dir, &encoded(cred))); + let device = key(); + let cred = DelegateCredential::unassociated(&device); + assert_eq!( + decode_sender(&dir, &encoded(cred)), + Ok(MessageSender { + account: None, + local_identity: local_id(&device), + }) + ); } /// The claimed account has never published a device set — the mapping is @@ -439,7 +489,10 @@ mod sender_check_tests { let device = key(); let dir = FakeDir::default(); // nothing published let cred = DelegateCredential::associated(&device, &hex::encode(account.as_ref())); - assert!(!should_deliver(&dir, &encoded(cred))); + assert_eq!( + decode_sender(&dir, &encoded(cred)), + Err(SenderError::Unverified) + ); } /// A directory outage leaves the mapping unconfirmed, so the message is @@ -453,15 +506,18 @@ mod sender_check_tests { ..Default::default() }; let cred = DelegateCredential::associated(&device, &hex::encode(account.as_ref())); - assert!(!should_deliver(&dir, &encoded(cred))); + assert_eq!( + decode_sender(&dir, &encoded(cred)), + Err(SenderError::Unverified) + ); } - /// No credential at all (e.g. the PrivateV1 placeholder) asserts no account - /// mapping and is delivered. + /// No credential at all (e.g. the PrivateV1 placeholder) leaves no sender to + /// attribute, so the message is dropped. #[test] - fn empty_credential_is_delivered() { + fn empty_credential_is_dropped() { let dir = FakeDir::default(); - assert!(should_deliver(&dir, b"")); + assert_eq!(decode_sender(&dir, b""), Err(SenderError::Missing)); } /// Bytes that aren't a well-formed credential leave the sender's mapping @@ -469,8 +525,11 @@ mod sender_check_tests { #[test] fn malformed_credential_is_dropped() { let dir = FakeDir::default(); - assert!(!should_deliver(&dir, b"not hex")); - assert!(!should_deliver(&dir, hex::encode([0u8; 4]).as_bytes())); + assert_eq!(decode_sender(&dir, b"not hex"), Err(SenderError::NotHex)); + assert_eq!( + decode_sender(&dir, hex::encode([0u8; 4]).as_bytes()), + Err(SenderError::Malformed) + ); } /// An account address that isn't a verifying key can't be looked up, so the @@ -479,6 +538,9 @@ mod sender_check_tests { fn non_key_account_address_is_dropped() { let dir = FakeDir::default(); let cred = DelegateCredential::associated(&key(), "user@example.com"); - assert!(!should_deliver(&dir, &encoded(cred))); + assert_eq!( + decode_sender(&dir, &encoded(cred)), + Err(SenderError::AccountNotAKey) + ); } } diff --git a/crates/client/src/event.rs b/crates/client/src/event.rs index e704050..eaf5dbe 100644 --- a/crates/client/src/event.rs +++ b/crates/client/src/event.rs @@ -8,7 +8,20 @@ use std::sync::Arc; -use libchat::ConversationClass; +use libchat::{ConversationClass, IdentId}; + +/// The sender of a received message, recovered from its credential. +/// +/// `account` is present only when the sender associated an account *and* the +/// account → device directory confirmed this device belongs to it — spoofed or +/// unconfirmable claims never reach the application, so a `Some` account is +/// always verified. `local_identity` is the sending device (delegate key), +/// hex-encoded. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MessageSender { + pub account: Option, + pub local_identity: IdentId, +} /// A discrete chat event. #[non_exhaustive] @@ -23,6 +36,7 @@ pub enum Event { MessageReceived { convo_id: Arc, content: Vec, + sender: MessageSender, }, InboundError { message: String, diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index a349036..becf852 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -10,7 +10,7 @@ pub use client::{ChatClient, Transport}; pub use delegate::DelegateSigner; pub use delivery_in_process::{InProcessDelivery, MessageBus}; pub use errors::ClientError; -pub use event::Event; +pub use event::{Event, MessageSender}; // Re-export types callers need to interact with ChatClient. pub use libchat::{ diff --git a/crates/client/tests/saro_and_raya.rs b/crates/client/tests/saro_and_raya.rs index d3a3637..091083c 100644 --- a/crates/client/tests/saro_and_raya.rs +++ b/crates/client/tests/saro_and_raya.rs @@ -95,8 +95,10 @@ fn direct_v1_standalone_integration() { // address, and publish a device bundle so the receiver can verify the // account → device mapping carried in the sender's credential. let saro_account = TestLogosAccount::new("Saro"); + let saro_account_id = hex::encode(saro_account.public_key().as_ref()); let mut saro_delegate = DelegateSigner::random(); - saro_delegate.associate(hex::encode(saro_account.public_key().as_ref())); + saro_delegate.associate(saro_account_id.clone()); + let saro_device_id = hex::encode(saro_delegate.public_key().as_ref()); publish_device_bundle(&mut reg_service, &saro_account, saro_delegate.public_key()); let raya_account = TestLogosAccount::new("Raya"); @@ -104,8 +106,14 @@ fn direct_v1_standalone_integration() { raya_delegate.associate(hex::encode(raya_account.public_key().as_ref())); publish_device_bundle(&mut reg_service, &raya_account, raya_delegate.public_key()); - let (mut saro, _saro_events) = - create_test_client(bus.clone(), reg_service.clone()).expect("client create"); + // Build saro's client with its associated delegate so its outbound messages + // carry a credential the receiver can verify against the published bundle. + let (mut saro, _saro_events) = ChatClientBuilder::new() + .ident(saro_delegate) + .transport(InProcessDelivery::new(bus.clone())) + .registration(reg_service.clone()) + .build() + .expect("client create"); let (raya, raya_events) = create_test_client(bus.clone(), reg_service.clone()).expect("client create"); @@ -121,8 +129,17 @@ fn direct_v1_standalone_integration() { saro.send_message(&convo_id, b"Hey from saro") .expect("payload mismatch"); expect_event(&raya_events, "MessageReceived", |e| match e { - Event::MessageReceived { content, .. } => { + Event::MessageReceived { + content, sender, .. + } => { assert_eq!(content.as_slice(), b"Hey from saro"); + // saro associated an account and published a matching bundle, so the + // sender surfaces with a verified account and its device. + assert_eq!( + sender.account.as_ref().map(|a| a.as_str()), + Some(saro_account_id.as_str()) + ); + assert_eq!(sender.local_identity.as_str(), saro_device_id.as_str()); Ok(()) } other => Err(other), @@ -153,9 +170,17 @@ fn saro_raya_message_exchange() { saro.send_message(&saro_convo_id, b"hello raya").unwrap(); expect_event(&raya_events, "MessageReceived", |e| match e { - Event::MessageReceived { convo_id, content } => { + Event::MessageReceived { + convo_id, + content, + sender, + } => { assert_eq!(convo_id, raya_convo_id); assert_eq!(content.as_slice(), b"hello raya"); + // saro's delegate is unassociated, so the sender surfaces its device + // but claims no account. + assert!(sender.account.is_none()); + assert!(!sender.local_identity.as_str().is_empty()); Ok(()) } other => Err(other),