mirror of
https://github.com/chatmail/core.git
synced 2026-04-16 21:16:42 +03:00
fix: Don't add SELF to unencrypted chat created from encrypted message (#7661)
I.e. create a non-replyable ad-hoc group in such cases. Unencrypted replies to encrypted messages are a security issue and "Composing a Reply Message" from RFC 9787 and "Replying and Forwarding Guidance" from RFC 9788 forbid such replies.
This commit is contained in:
@@ -196,7 +196,7 @@ async fn insert_tombstone(context: &Context, rfc724_mid: &str) -> Result<MsgId>
|
||||
async fn get_to_and_past_contact_ids(
|
||||
context: &Context,
|
||||
mime_parser: &MimeMessage,
|
||||
chat_assignment: &ChatAssignment,
|
||||
chat_assignment: &mut ChatAssignment,
|
||||
parent_message: &Option<Message>,
|
||||
incoming_origin: Origin,
|
||||
) -> Result<(Vec<Option<ContactId>>, Vec<Option<ContactId>>)> {
|
||||
@@ -385,32 +385,6 @@ async fn get_to_and_past_contact_ids(
|
||||
// mapped it to a key contact.
|
||||
// This is an encrypted 1:1 chat.
|
||||
to_ids = pgp_to_ids
|
||||
} else if let Some(chat_id) = chat_id {
|
||||
to_ids = match mime_parser.was_encrypted() {
|
||||
true => {
|
||||
lookup_key_contacts_by_address_list(
|
||||
context,
|
||||
&mime_parser.recipients,
|
||||
to_member_fingerprints,
|
||||
Some(chat_id),
|
||||
)
|
||||
.await?
|
||||
}
|
||||
false => {
|
||||
add_or_lookup_contacts_by_address_list(
|
||||
context,
|
||||
&mime_parser.recipients,
|
||||
if !mime_parser.incoming {
|
||||
Origin::OutgoingTo
|
||||
} else if incoming_origin.is_known() {
|
||||
Origin::IncomingTo
|
||||
} else {
|
||||
Origin::IncomingUnknownTo
|
||||
},
|
||||
)
|
||||
.await?
|
||||
}
|
||||
}
|
||||
} else {
|
||||
let ids = match mime_parser.was_encrypted() {
|
||||
true => {
|
||||
@@ -418,7 +392,7 @@ async fn get_to_and_past_contact_ids(
|
||||
context,
|
||||
&mime_parser.recipients,
|
||||
to_member_fingerprints,
|
||||
None,
|
||||
chat_id,
|
||||
)
|
||||
.await?
|
||||
}
|
||||
@@ -426,13 +400,20 @@ async fn get_to_and_past_contact_ids(
|
||||
};
|
||||
if mime_parser.was_encrypted() && !ids.contains(&None)
|
||||
// Prefer creating PGP chats if there are any key-contacts. At least this prevents
|
||||
// from replying unencrypted.
|
||||
// from replying unencrypted. Otherwise downgrade to a non-replyable ad-hoc group.
|
||||
|| ids
|
||||
.iter()
|
||||
.any(|&c| c.is_some() && c != Some(ContactId::SELF))
|
||||
{
|
||||
to_ids = ids;
|
||||
} else {
|
||||
if mime_parser.was_encrypted() {
|
||||
warn!(
|
||||
context,
|
||||
"No key-contact looked up. Downgrading to AdHocGroup."
|
||||
);
|
||||
*chat_assignment = ChatAssignment::AdHocGroup;
|
||||
}
|
||||
to_ids = add_or_lookup_contacts_by_address_list(
|
||||
context,
|
||||
&mime_parser.recipients,
|
||||
@@ -634,12 +615,12 @@ pub(crate) async fn receive_imf_inner(
|
||||
.await?
|
||||
.filter(|p| Some(p.id) != replace_msg_id);
|
||||
|
||||
let chat_assignment =
|
||||
let mut chat_assignment =
|
||||
decide_chat_assignment(context, &mime_parser, &parent_message, rfc724_mid, from_id).await?;
|
||||
let (to_ids, past_ids) = get_to_and_past_contact_ids(
|
||||
context,
|
||||
&mime_parser,
|
||||
&chat_assignment,
|
||||
&mut chat_assignment,
|
||||
&parent_message,
|
||||
incoming_origin,
|
||||
)
|
||||
@@ -2698,6 +2679,9 @@ async fn lookup_or_create_adhoc_group(
|
||||
let to_ids: Vec<ContactId> = to_ids.iter().filter_map(|x| *x).collect();
|
||||
let mut contact_ids = BTreeSet::<ContactId>::from_iter(to_ids.iter().copied());
|
||||
contact_ids.insert(from_id);
|
||||
if mime_parser.was_encrypted() {
|
||||
contact_ids.remove(&ContactId::SELF);
|
||||
}
|
||||
let trans_fn = |t: &mut rusqlite::Transaction| {
|
||||
t.pragma_update(None, "query_only", "0")?;
|
||||
t.execute(
|
||||
@@ -3099,7 +3083,9 @@ async fn apply_group_changes(
|
||||
if !from_id.is_special() {
|
||||
new_members.insert(from_id);
|
||||
}
|
||||
|
||||
if mime_parser.was_encrypted() && chat.grpid.is_empty() {
|
||||
new_members.remove(&ContactId::SELF);
|
||||
}
|
||||
context
|
||||
.sql
|
||||
.transaction(|transaction| {
|
||||
@@ -3173,6 +3159,10 @@ async fn apply_group_changes(
|
||||
new_members.remove(&removed_id);
|
||||
}
|
||||
|
||||
if mime_parser.was_encrypted() && chat.grpid.is_empty() {
|
||||
new_members.remove(&ContactId::SELF);
|
||||
}
|
||||
|
||||
if new_members != chat_contacts {
|
||||
chat::update_chat_contacts_table(
|
||||
context,
|
||||
@@ -3814,11 +3804,15 @@ async fn create_adhoc_group(
|
||||
to_ids: &[ContactId],
|
||||
grpname: &str,
|
||||
) -> Result<Option<(ChatId, Blocked)>> {
|
||||
let mut member_ids: Vec<ContactId> = to_ids.to_vec();
|
||||
if !member_ids.contains(&(from_id)) {
|
||||
let mut member_ids: Vec<ContactId> = to_ids
|
||||
.iter()
|
||||
.copied()
|
||||
.filter(|&id| id != ContactId::SELF)
|
||||
.collect();
|
||||
if from_id != ContactId::SELF && !member_ids.contains(&from_id) {
|
||||
member_ids.push(from_id);
|
||||
}
|
||||
if !member_ids.contains(&(ContactId::SELF)) {
|
||||
if !mime_parser.was_encrypted() {
|
||||
member_ids.push(ContactId::SELF);
|
||||
}
|
||||
|
||||
|
||||
@@ -4,8 +4,9 @@ use tokio::fs;
|
||||
|
||||
use super::*;
|
||||
use crate::chat::{
|
||||
ChatItem, ChatVisibility, add_contact_to_chat, add_to_chat_contacts_table, create_group,
|
||||
get_chat_contacts, get_chat_msgs, is_contact_in_chat, remove_contact_from_chat, send_text_msg,
|
||||
CantSendReason, ChatItem, ChatVisibility, add_contact_to_chat, add_to_chat_contacts_table,
|
||||
create_group, get_chat_contacts, get_chat_msgs, is_contact_in_chat, remove_contact_from_chat,
|
||||
send_text_msg,
|
||||
};
|
||||
use crate::chatlist::Chatlist;
|
||||
use crate::constants::DC_GCL_FOR_FORWARDING;
|
||||
@@ -5106,6 +5107,42 @@ async fn test_dont_verify_by_verified_by_unknown() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_recv_outgoing_msg_before_securejoin() -> Result<()> {
|
||||
let mut tcm = TestContextManager::new();
|
||||
let a0 = &tcm.alice().await;
|
||||
let a1 = &tcm.alice().await;
|
||||
let bob = &tcm.bob().await;
|
||||
|
||||
tcm.execute_securejoin(bob, a0).await;
|
||||
let chat_id_a0_bob = a0.create_chat_id(bob).await;
|
||||
let sent_msg = a0.send_text(chat_id_a0_bob, "Hi").await;
|
||||
let msg_a1 = a1.recv_msg(&sent_msg).await;
|
||||
assert!(msg_a1.get_showpadlock());
|
||||
let chat_a1 = Chat::load_from_db(a1, msg_a1.chat_id).await?;
|
||||
assert_eq!(chat_a1.typ, Chattype::Group);
|
||||
assert!(!chat_a1.is_encrypted(a1).await?);
|
||||
assert_eq!(
|
||||
chat::get_chat_contacts(a1, chat_a1.id).await?,
|
||||
[a1.add_or_lookup_address_contact_id(bob).await]
|
||||
);
|
||||
assert_eq!(
|
||||
chat_a1.why_cant_send(a1).await?,
|
||||
Some(CantSendReason::NotAMember)
|
||||
);
|
||||
|
||||
let sent_msg = a0.send_text(chat_id_a0_bob, "Hi again").await;
|
||||
let msg_a1 = a1.recv_msg(&sent_msg).await;
|
||||
assert!(msg_a1.get_showpadlock());
|
||||
assert_eq!(msg_a1.chat_id, chat_a1.id);
|
||||
let chat_a1 = Chat::load_from_db(a1, msg_a1.chat_id).await?;
|
||||
assert_eq!(
|
||||
chat_a1.why_cant_send(a1).await?,
|
||||
Some(CantSendReason::NotAMember)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_sanitize_filename_in_received() -> Result<()> {
|
||||
let alice = &TestContext::new_alice().await;
|
||||
@@ -5365,16 +5402,14 @@ async fn test_encrypted_adhoc_group_message() -> Result<()> {
|
||||
assert_eq!(chat.is_encrypted(bob).await?, false);
|
||||
|
||||
let contact_ids = get_chat_contacts(bob, chat.id).await?;
|
||||
assert_eq!(contact_ids.len(), 3);
|
||||
assert!(chat.is_self_in_chat(bob).await?);
|
||||
assert_eq!(contact_ids.len(), 2);
|
||||
assert!(!chat.is_self_in_chat(bob).await?);
|
||||
|
||||
// Since the group is unencrypted, all contacts have
|
||||
// to be address-contacts.
|
||||
for contact_id in contact_ids {
|
||||
let contact = Contact::get_by_id(bob, contact_id).await?;
|
||||
if contact_id != ContactId::SELF {
|
||||
assert_eq!(contact.is_key_contact(), false);
|
||||
}
|
||||
assert_eq!(contact.is_key_contact(), false);
|
||||
}
|
||||
|
||||
// `from_id` of the message corresponds to key-contact of Alice
|
||||
|
||||
Reference in New Issue
Block a user