mirror of
https://github.com/logos-messaging/libchat.git
synced 2026-06-30 13:09:28 +00:00
fix: deliver anonymous PrivateV1 messages to applications
A PrivateV1 conversation is an out-of-band X3DH intro that binds no sender credential, so its messages carry an empty credential. The client's `decode_sender` returns `Missing` for an empty credential, and the event emitters dropped any message whose sender could not be decoded. A PrivateV1 conversation therefore surfaced `ConversationStarted` but never the message content, for both the initial message and every reply. Deliver these messages with no sender instead of dropping them: - `Event::MessageReceived.sender` becomes `Option<MessageSender>`; `None` marks an anonymous PrivateV1 message. - `ConvoOutcome` carries the conversation `class` (mirroring `InboxOutcome`), so the client can tell a legitimately credential-less PrivateV1 message from a group message that is missing one. - A new `message_sender` helper gates the anonymous case on `class == Private`; a missing credential on any group class is still dropped, preserving the invariant that group messages must be attributable. Covered by a new `private_v1_integration` test exercising both the inbox (initial message) and convo (reply) receive paths.
This commit is contained in:
parent
0d38dd80b7
commit
a94ba2a7e0
@ -254,7 +254,10 @@ impl<S: ExternalServices> Convo<S> for GroupV1Convo {
|
||||
let msg_hash = blake2b_hex::<hash_size::MessageId>(&[bytes.as_ref()]);
|
||||
if self.outbound_msgs.contains(&msg_hash) {
|
||||
debug!("Dropping message, sent from self");
|
||||
return Ok(ConvoOutcome::empty(self.convo_id.to_string()));
|
||||
return Ok(ConvoOutcome::empty(
|
||||
self.convo_id.to_string(),
|
||||
crate::ConversationClass::Group,
|
||||
));
|
||||
}
|
||||
|
||||
let mls_message: MlsMessageIn =
|
||||
@ -266,7 +269,10 @@ impl<S: ExternalServices> Convo<S> for GroupV1Convo {
|
||||
|
||||
if protocol_message.epoch() < self.mls_group.epoch() {
|
||||
// TODO: (P1) Add logging for messages arriving from past epoch.
|
||||
return Ok(ConvoOutcome::empty(self.id().to_string()));
|
||||
return Ok(ConvoOutcome::empty(
|
||||
self.id().to_string(),
|
||||
crate::ConversationClass::Group,
|
||||
));
|
||||
}
|
||||
|
||||
let processed = self
|
||||
@ -299,6 +305,7 @@ impl<S: ExternalServices> Convo<S> for GroupV1Convo {
|
||||
Ok(ConvoOutcome {
|
||||
convo_id: self.id().to_string(),
|
||||
content,
|
||||
class: crate::ConversationClass::Group,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@ -239,7 +239,12 @@ where
|
||||
let frame = GroupV2Frame::decode(bytes.as_ref()).map_err(ChatError::generic)?;
|
||||
let inner = match frame.payload {
|
||||
Some(GroupV2Payload::DeMlsWrapper(b)) => b.to_vec(),
|
||||
_ => return Ok(ConvoOutcome::empty(self.convo_id.clone())),
|
||||
_ => {
|
||||
return Ok(ConvoOutcome::empty(
|
||||
self.convo_id.clone(),
|
||||
crate::ConversationClass::Group,
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
self.conversation.process_inbound(
|
||||
@ -256,7 +261,10 @@ where
|
||||
Some(o) => Ok(o),
|
||||
None => {
|
||||
warn!("returning None as ConvoOutcome");
|
||||
Ok(ConvoOutcome::empty(self.convo_id.to_string()))
|
||||
Ok(ConvoOutcome::empty(
|
||||
self.convo_id.to_string(),
|
||||
crate::ConversationClass::Group,
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -387,6 +395,7 @@ impl GroupV2Convo {
|
||||
bytes: cm.message.clone(),
|
||||
encoded_credential: cm.sender.clone(),
|
||||
}),
|
||||
class: crate::ConversationClass::Group,
|
||||
}),
|
||||
_ => None,
|
||||
})
|
||||
|
||||
@ -276,6 +276,7 @@ impl<S: ExternalServices> Convo<S> for PrivateV1Convo {
|
||||
Ok(ConvoOutcome {
|
||||
convo_id: self.id().to_string(),
|
||||
content,
|
||||
class: crate::ConversationClass::Private,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@ -22,13 +22,18 @@ pub struct Content {
|
||||
pub struct ConvoOutcome {
|
||||
pub convo_id: ConversationId,
|
||||
pub content: Option<Content>,
|
||||
/// Class of the conversation this outcome belongs to. Surfaced so a
|
||||
/// consumer can tell an anonymous PrivateV1 message (no sender credential
|
||||
/// by design) from a group message that is missing one.
|
||||
pub class: ConversationClass,
|
||||
}
|
||||
|
||||
impl ConvoOutcome {
|
||||
pub fn empty(convo_id: ConversationId) -> Self {
|
||||
pub fn empty(convo_id: ConversationId, class: ConversationClass) -> Self {
|
||||
Self {
|
||||
convo_id,
|
||||
content: None,
|
||||
class,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -5,8 +5,9 @@ use components::{ThreadedWakeupService, WakeupEvent};
|
||||
use crossbeam_channel::{Receiver, Sender, select};
|
||||
use crypto::Ed25519VerifyingKey;
|
||||
use libchat::{
|
||||
AccountDirectory, ConversationId, ConvoOutcome, Core, DeliveryService, IdentId, IdentIdRef,
|
||||
IdentityProvider, InboxOutcome, Introduction, PayloadOutcome, RegistrationService,
|
||||
AccountDirectory, ConversationClass, ConversationId, ConvoOutcome, Core, DeliveryService,
|
||||
IdentId, IdentIdRef, IdentityProvider, InboxOutcome, Introduction, PayloadOutcome,
|
||||
RegistrationService,
|
||||
};
|
||||
use parking_lot::Mutex;
|
||||
use storage::ChatStore;
|
||||
@ -260,8 +261,9 @@ fn account_key_from_hex(addr: &str) -> Option<Ed25519VerifyingKey> {
|
||||
/// 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.
|
||||
/// No credential at all, so no sender can be attributed. The caller decides
|
||||
/// what this means per conversation class: dropped for a group, delivered
|
||||
/// without a sender for an anonymous PrivateV1 intro.
|
||||
Missing,
|
||||
/// Credential bytes were not valid hex.
|
||||
NotHex,
|
||||
@ -278,10 +280,11 @@ enum SenderError {
|
||||
/// Decode and verify a message's sender from its credential, checked against the
|
||||
/// account → device directory (our account store).
|
||||
///
|
||||
/// `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).
|
||||
/// `Ok(sender)` — the sender was attributed; its `account` is set only when the
|
||||
/// directory confirmed the device, so it is always verified. `Err` — no sender
|
||||
/// could be attributed (see [`SenderError`]). Whether an unattributed message is
|
||||
/// dropped or delivered without a sender is decided by [`message_sender`]
|
||||
/// according to the conversation class.
|
||||
fn decode_sender(
|
||||
directory: &impl AccountDirectory,
|
||||
encoded: &[u8],
|
||||
@ -328,11 +331,36 @@ fn decode_sender(
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve the sender to attach to a received message, honouring the
|
||||
/// conversation class. Returns `None` to drop the message, `Some(None)` to
|
||||
/// deliver it with no sender (an anonymous PrivateV1 message, which binds no
|
||||
/// credential by design), and `Some(Some(sender))` for a credential-bearing
|
||||
/// message whose sender verified.
|
||||
fn message_sender(
|
||||
directory: &impl AccountDirectory,
|
||||
encoded_credential: &[u8],
|
||||
class: ConversationClass,
|
||||
) -> Option<Option<MessageSender>> {
|
||||
match decode_sender(directory, encoded_credential) {
|
||||
Ok(sender) => Some(Some(sender)),
|
||||
// PrivateV1 is an out-of-band X3DH intro and attaches no credential, so
|
||||
// surface its messages with no sender. For any other class an absent
|
||||
// credential is a protocol violation, dropped along with every other
|
||||
// unverifiable-sender case.
|
||||
Err(SenderError::Missing) if class == ConversationClass::Private => Some(None),
|
||||
Err(_) => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn convo_events(outcome: ConvoOutcome, directory: &impl AccountDirectory) -> Vec<Event> {
|
||||
let ConvoOutcome { convo_id, content } = outcome;
|
||||
let ConvoOutcome {
|
||||
convo_id,
|
||||
content,
|
||||
class,
|
||||
} = outcome;
|
||||
content
|
||||
.and_then(|c| {
|
||||
let sender = decode_sender(directory, &c.encoded_credential).ok()?;
|
||||
let sender = message_sender(directory, &c.encoded_credential, class)?;
|
||||
Some(Event::MessageReceived {
|
||||
convo_id: Arc::from(convo_id),
|
||||
content: c.bytes,
|
||||
@ -355,7 +383,8 @@ fn inbox_events(outcome: InboxOutcome, directory: &impl AccountDirectory) -> Vec
|
||||
class: new_conversation.class,
|
||||
});
|
||||
if let Some(c) = initial.and_then(|co| co.content)
|
||||
&& let Ok(sender) = decode_sender(directory, &c.encoded_credential)
|
||||
&& let Some(sender) =
|
||||
message_sender(directory, &c.encoded_credential, new_conversation.class)
|
||||
{
|
||||
events.push(Event::MessageReceived {
|
||||
convo_id: Arc::clone(&id),
|
||||
|
||||
@ -32,11 +32,13 @@ pub enum Event {
|
||||
convo_id: Arc<str>,
|
||||
class: ConversationClass,
|
||||
},
|
||||
/// User content arrived on an existing conversation.
|
||||
/// User content arrived on an existing conversation. `sender` is `None` for
|
||||
/// an anonymous PrivateV1 message: that conversation is an out-of-band X3DH
|
||||
/// intro and binds no sender credential, so no identity can be resolved.
|
||||
MessageReceived {
|
||||
convo_id: Arc<str>,
|
||||
content: Vec<u8>,
|
||||
sender: MessageSender,
|
||||
sender: Option<MessageSender>,
|
||||
},
|
||||
InboundError {
|
||||
message: String,
|
||||
|
||||
@ -130,7 +130,9 @@ fn direct_v1_standalone_integration() {
|
||||
.expect("payload mismatch");
|
||||
expect_event(&raya_events, "MessageReceived", |e| match e {
|
||||
Event::MessageReceived {
|
||||
content, sender, ..
|
||||
content,
|
||||
sender: Some(sender),
|
||||
..
|
||||
} => {
|
||||
assert_eq!(content.as_slice(), b"Hey from saro");
|
||||
// saro associated an account and published a matching bundle, so the
|
||||
@ -173,7 +175,7 @@ fn saro_raya_message_exchange() {
|
||||
Event::MessageReceived {
|
||||
convo_id,
|
||||
content,
|
||||
sender,
|
||||
sender: Some(sender),
|
||||
} => {
|
||||
assert_eq!(convo_id, raya_convo_id);
|
||||
assert_eq!(content.as_slice(), b"hello raya");
|
||||
@ -229,6 +231,56 @@ fn saro_raya_message_exchange() {
|
||||
assert_eq!(raya.list_conversations().unwrap().len(), 1);
|
||||
}
|
||||
|
||||
/// PrivateV1 (intro-bundle) is an out-of-band X3DH intro that binds no sender
|
||||
/// credential, so its messages must still surface — with no sender — rather
|
||||
/// than be dropped. Covers both receive paths: the recipient's initial message
|
||||
/// (inbox) and the reply on the established conversation (convo).
|
||||
#[test]
|
||||
fn private_v1_integration() {
|
||||
let bus = MessageBus::default();
|
||||
let reg_service = EphemeralRegistry::new();
|
||||
|
||||
let (mut saro, saro_events) =
|
||||
create_test_client(bus.clone(), reg_service.clone()).expect("client create");
|
||||
let (mut raya, raya_events) =
|
||||
create_test_client(bus.clone(), reg_service.clone()).expect("client create");
|
||||
|
||||
let raya_bundle = raya.create_intro_bundle().expect("intro bundle");
|
||||
saro.create_conversation(&raya_bundle, b"hello raya")
|
||||
.expect("convo create");
|
||||
|
||||
let raya_convo_id = expect_event(&raya_events, "ConversationStarted", |e| match e {
|
||||
Event::ConversationStarted { convo_id, .. } => Ok(convo_id),
|
||||
other => Err(other),
|
||||
});
|
||||
expect_event(&raya_events, "MessageReceived", |e| match e {
|
||||
Event::MessageReceived {
|
||||
content, sender, ..
|
||||
} => {
|
||||
assert_eq!(content.as_slice(), b"hello raya");
|
||||
assert!(
|
||||
sender.is_none(),
|
||||
"PrivateV1 message must surface with no sender"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
other => Err(other),
|
||||
});
|
||||
|
||||
raya.send_message(&raya_convo_id, b"hi saro")
|
||||
.expect("reply");
|
||||
expect_event(&saro_events, "MessageReceived", |e| match e {
|
||||
Event::MessageReceived {
|
||||
content, sender, ..
|
||||
} => {
|
||||
assert_eq!(content.as_slice(), b"hi saro");
|
||||
assert!(sender.is_none());
|
||||
Ok(())
|
||||
}
|
||||
other => Err(other),
|
||||
});
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct FailingDelivery {
|
||||
inbound_tx: Sender<Vec<u8>>,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user