Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-04-28 19:06:35 +03:00
Code Issues Packages Projects Releases Wiki Activity

feat: Don't reveal profile data to a not yet verified contact (#5166)

Browse Source 
Follow-up to b771311593. Since that commit names are not revealed in
verified chats, but during verification (i.e. SecureJoin) they are still sent unencrypted. Moreover,
all profile data mustn't be sent even encrypted before the contact verification, i.e. before
"v{c,g}-request-with-auth". That was done for the selfavatar in
304e902fce, now it's done for From/To names and the self-status as
well. Moreover, "v{c,g}-request" and "v{c,g}-auth-required" messages are deleted right after
processing, so other devices won't see the received profile data anyway.
This commit is contained in:
iequidoo
2024-06-12 11:09:08 -03:00
committed by iequidoo
parent 0b20f69959
commit 7ad3c70b68
3 changed files with 76 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
src/mimefactory.rs
View File

@@ -142,6 +142,7 @@ struct MessageHeaders {
impl<'a> MimeFactory<'a> {
pub async fn from_msg(context: &Context, msg: &'a Message) -> Result<MimeFactory<'a>> {
let chat = Chat::load_from_db(context, msg.chat_id).await?;
let attach_profile_data = Self::should_attach_profile_data(msg);
let from_addr = context.get_primary_self_addr().await?;
let config_displayname = context
@@ -152,7 +153,11 @@ impl<'a> MimeFactory<'a> {
if let Some(override_name) = msg.param.get(Param::OverrideSenderDisplayname) {
(override_name.to_string(), Some(config_displayname))
} else {
(config_displayname, None)
let name = match attach_profile_data {
true => config_displayname,
false => "".to_string(),
};
(name, None)
};
let mut recipients = Vec::with_capacity(5);
@@ -186,7 +191,11 @@ impl<'a> MimeFactory<'a> {
for row in rows {
let (authname, addr, id) = row?;
if !recipients_contain_addr(&recipients, &addr) {
recipients.push((authname, addr));
let name = match attach_profile_data {
true => authname,
false => "".to_string(),
};
recipients.push((name, addr));
}
recipient_ids.insert(id);
}
@@ -220,16 +229,20 @@ impl<'a> MimeFactory<'a> {
},
)
.await?;
let selfstatus = match attach_profile_data {
true => context
.get_config(Config::Selfstatus)
.await?
.unwrap_or_default(),
false => "".to_string(),
};
let attach_selfavatar = Self::should_attach_selfavatar(context, msg).await;
let factory = MimeFactory {
from_addr,
from_displayname,
sender_displayname,
selfstatus: context
.get_config(Config::Selfstatus)
.await?
.unwrap_or_default(),
selfstatus,
recipients,
timestamp: msg.timestamp_sort,
loaded: Loaded::Message { chat },
@@ -396,31 +409,35 @@ impl<'a> MimeFactory<'a> {
}
}
async fn should_attach_selfavatar(context: &Context, msg: &Message) -> bool {
let cmd = msg.param.get_cmd();
(cmd != SystemMessage::SecurejoinMessage || {
fn should_attach_profile_data(msg: &Message) -> bool {
msg.param.get_cmd() != SystemMessage::SecurejoinMessage || {
let step = msg.param.get(Param::Arg).unwrap_or_default();
// Don't attach selfavatar at the earlier SecureJoin steps:
// Don't attach profile data at the earlier SecureJoin steps:
// - The corresponding messages, i.e. "v{c,g}-request" and "v{c,g}-auth-required" are
// deleted right after processing, so other devices won't see the avatar.
// deleted right after processing, so other devices won't see the avatar etc.
// - It's also good for privacy because the contact isn't yet verified and these
// messages are auto-sent unlike usual unencrypted messages.
step == "vg-request-with-auth"
|| step == "vc-request-with-auth"
|| step == "vg-member-added"
|| step == "vc-contact-confirm"
}) && match chat::shall_attach_selfavatar(context, msg.chat_id).await {
Ok(should) => should,
Err(err) => {
warn!(
context,
"should_attach_selfavatar: cannot get selfavatar state: {err:#}."
);
false
}
}
}
async fn should_attach_selfavatar(context: &Context, msg: &Message) -> bool {
Self::should_attach_profile_data(msg)
&& match chat::shall_attach_selfavatar(context, msg.chat_id).await {
Ok(should) => should,
Err(err) => {
warn!(
context,
"should_attach_selfavatar: cannot get selfavatar state: {err:#}."
);
false
}
}
}
fn grpimage(&self) -> Option<String> {
match &self.loaded {
Loaded::Message { chat } => {
@@ -480,7 +497,11 @@ impl<'a> MimeFactory<'a> {
return Ok(format!("Re: {}", remove_subject_prefix(last_subject)));
}
let self_name = &match context.get_config(Config::Displayname).await? {
let self_name = match Self::should_attach_profile_data(self.msg) {
true => context.get_config(Config::Displayname).await?,
false => None,
};
let self_name = &match self_name {
Some(name) => name,
None => context.get_config(Config::Addr).await?.unwrap_or_default(),
};
@@ -708,7 +729,8 @@ impl<'a> MimeFactory<'a> {
.unwrap(),
);
}
if is_encrypted && verified {
if is_encrypted && verified || self.msg.param.get_cmd() == SystemMessage::SecurejoinMessage
{
headers.unprotected.insert(
0,
Header::new_with_value(

40
src/securejoin.rs
View File

@@ -802,6 +802,9 @@ mod tests {
let alice = tcm.alice().await;
let alice_addr = &alice.get_config(Config::Addr).await.unwrap().unwrap();
let bob = tcm.bob().await;
bob.set_config(Config::Displayname, Some("Bob Examplenet"))
.await
.unwrap();
alice
.set_config(Config::VerifiedOneOnOneChats, Some("1"))
.await
@@ -824,6 +827,11 @@ mod tests {
// Step 1: Generate QR-code, ChatId(0) indicates setup-contact
let qr = get_securejoin_qr(&alice.ctx, None).await.unwrap();
// We want Bob to learn Alice's name from their messages, not from the QR code.
alice
.set_config(Config::Displayname, Some("Alice Exampleorg"))
.await
.unwrap();
// Step 2: Bob scans QR-code, sends vc-request
join_securejoin(&bob.ctx, &qr).await.unwrap();
@@ -895,6 +903,7 @@ mod tests {
// Check Bob sent the right message.
let sent = bob.pop_sent_msg().await;
assert!(!sent.payload.contains("Bob Examplenet"));
let mut msg = alice.parse_msg(&sent).await;
let vc_request_with_auth_ts_sent = msg
.get_header(HeaderDef::Date)
@@ -946,6 +955,7 @@ mod tests {
.await
.unwrap();
assert_eq!(contact_bob.is_verified(&alice.ctx).await.unwrap(), false);
assert_eq!(contact_bob.get_authname(), "");
if case == SetupContactCase::CheckProtectionTimestamp {
SystemTime::shift(Duration::from_secs(3600));
@@ -954,6 +964,10 @@ mod tests {
// Step 5+6: Alice receives vc-request-with-auth, sends vc-contact-confirm
alice.recv_msg_trash(&sent).await;
assert_eq!(contact_bob.is_verified(&alice.ctx).await.unwrap(), true);
let contact_bob = Contact::get_by_id(&alice.ctx, contact_bob_id)
.await
.unwrap();
assert_eq!(contact_bob.get_authname(), "Bob Examplenet");
// exactly one one-to-one chat should be visible for both now
// (check this before calling alice.create_chat() explicitly below)
@@ -981,8 +995,19 @@ mod tests {
}
}
// Make sure Alice hasn't yet sent their name to Bob.
let contact_alice_id = Contact::lookup_id_by_addr(&bob.ctx, alice_addr, Origin::Unknown)
.await
.expect("Error looking up contact")
.expect("Contact not found");
let contact_alice = Contact::get_by_id(&bob.ctx, contact_alice_id)
.await
.unwrap();
assert_eq!(contact_alice.get_authname(), "");
// Check Alice sent the right message to Bob.
let sent = alice.pop_sent_msg().await;
assert!(!sent.payload.contains("Alice Exampleorg"));
let msg = bob.parse_msg(&sent).await;
assert!(msg.was_encrypted());
assert_eq!(
@@ -991,18 +1016,15 @@ mod tests {
);
// Bob should not yet have Alice verified
let contact_alice_id = Contact::lookup_id_by_addr(&bob.ctx, alice_addr, Origin::Unknown)
.await
.expect("Error looking up contact")
.expect("Contact not found");
let contact_alice = Contact::get_by_id(&bob.ctx, contact_alice_id)
.await
.unwrap();
assert_eq!(contact_bob.is_verified(&bob.ctx).await.unwrap(), false);
assert_eq!(contact_alice.is_verified(&bob.ctx).await.unwrap(), false);
// Step 7: Bob receives vc-contact-confirm
bob.recv_msg_trash(&sent).await;
assert_eq!(contact_alice.is_verified(&bob.ctx).await.unwrap(), true);
let contact_alice = Contact::get_by_id(&bob.ctx, contact_alice_id)
.await
.unwrap();
assert_eq!(contact_alice.get_authname(), "Alice Exampleorg");
if case != SetupContactCase::SecurejoinWaitTimeout {
// Later we check that the timeout message isn't added to the already protected chat.
@@ -1119,7 +1141,7 @@ mod tests {
// Alice should not yet have Bob verified
let (contact_bob_id, _modified) = Contact::add_or_lookup(
&alice.ctx,
"Bob",
"",
&ContactAddress::new("bob@example.net")?,
Origin::ManuallyCreated,
)

8
src/test_utils.rs
View File

@@ -566,18 +566,12 @@ impl TestContext {
/// Returns the [`Contact`] for the other [`TestContext`], creating it if necessary.
pub async fn add_or_lookup_contact(&self, other: &TestContext) -> Contact {
let name = other
.ctx
.get_config(Config::Displayname)
.await
.unwrap_or_default()
.unwrap_or_default();
let primary_self_addr = other.ctx.get_primary_self_addr().await.unwrap();
let addr = ContactAddress::new(&primary_self_addr).unwrap();
// MailinglistAddress is the lowest allowed origin, we'd prefer to not modify the
// origin when creating this contact.
let (contact_id, modified) =
Contact::add_or_lookup(self, &name, &addr, Origin::MailinglistAddress)
Contact::add_or_lookup(self, "", &addr, Origin::MailinglistAddress)
.await
.expect("add_or_lookup");
match modified {