From 37d9b704dc91f76833bdf8ec0f34f702b0dbbc06 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Wed, 5 Nov 2025 18:57:38 -0300 Subject: [PATCH 1/6] feat: imap: Don't prefetch Chat-Version; try to find out message encryption state instead Instead, prefetch Secure-Join, Content-Type and Subject headers, try to find out if the message is encrypted, i.e.: - if its Content-Type is "multipart/encrypted" - or Subject is "..." or "[...]" as some MUAs use "multipart/mixed"; we can't only look at Subject as it's not mandatory; and depending on this decide on the target folder and whether the message should be downloaded. There's no much sense in downloading unencrypted "Chat-Version"-containing messages if `ShowEmails` is `Off` or `AcceptedContacts`, unencrypted Delta Chat messages should be considered as usual emails, there's even the "New E-Mail" feature in UIs nowadays which sends such messages. Don't prefetch Auto-Submitted as well, this becomes unnecessary. Changed behavior: before, "Chat-Version"-containing messages were moved from INBOX to DeltaChat, now such encrypted messages may remain in INBOX -- if there's no parent message or it's not `MessengerMessage`. Don't unconditionally move encrypted messages yet because the account may be shared with other software which doesn't and shouldn't look into the DeltaChat folder. --- src/imap.rs | 77 ++++++++++++++++++------------------------ src/imap/imap_tests.rs | 45 +++++++++++++----------- src/imap/session.rs | 9 +++-- src/receive_imf.rs | 4 +-- 4 files changed, 65 insertions(+), 70 deletions(-) diff --git a/src/imap.rs b/src/imap.rs index 1aeca6d20a..d6acccc51c 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -1964,21 +1964,24 @@ impl Session { } } +fn is_encrypted(headers: &[mailparse::MailHeader<'_>]) -> bool { + let content_type = headers.get_header_value(HeaderDef::ContentType); + let content_type = content_type.as_ref(); + let res = content_type.is_some_and(|v| v.contains("multipart/encrypted")); + // Some MUAs use "multipart/mixed", look also at Subject in this case. We can't only look at + // Subject as it's not mandatory () + // and may be user-formed. + res || content_type.is_some_and(|v| v.contains("multipart/mixed")) + && headers + .get_header_value(HeaderDef::Subject) + .is_some_and(|v| v == "..." || v == "[...]") +} + async fn should_move_out_of_spam( context: &Context, headers: &[mailparse::MailHeader<'_>], ) -> Result { - if headers.get_header_value(HeaderDef::ChatVersion).is_some() { - // If this is a chat message (i.e. has a ChatVersion header), then this might be - // a securejoin message. We can't find out at this point as we didn't prefetch - // the SecureJoin header. So, we always move chat messages out of Spam. - // Two possibilities to change this would be: - // 1. Remove the `&& !context.is_spam_folder(folder).await?` check from - // `fetch_new_messages()`, and then let `receive_imf()` check - // if it's a spam message and should be hidden. - // 2. Or add a flag to the ChatVersion header that this is a securejoin - // request, and return `true` here only if the message has this flag. - // `receive_imf()` can then check if the securejoin request is valid. + if headers.get_header_value(HeaderDef::SecureJoin).is_some() || is_encrypted(headers) { return Ok(true); } @@ -2037,7 +2040,8 @@ async fn spam_target_folder_cfg( return Ok(None); } - if needs_move_to_mvbox(context, headers).await? + if is_encrypted(headers) && context.get_config_bool(Config::MvboxMove).await? + || needs_move_to_mvbox(context, headers).await? // If OnlyFetchMvbox is set, we don't want to move the message to // the inbox where we wouldn't fetch it again: || context.get_config_bool(Config::OnlyFetchMvbox).await? @@ -2090,20 +2094,6 @@ async fn needs_move_to_mvbox( context: &Context, headers: &[mailparse::MailHeader<'_>], ) -> Result { - let has_chat_version = headers.get_header_value(HeaderDef::ChatVersion).is_some(); - if !context.get_config_bool(Config::IsChatmail).await? - && has_chat_version - && headers - .get_header_value(HeaderDef::AutoSubmitted) - .filter(|val| val.eq_ignore_ascii_case("auto-generated")) - .is_some() - { - if let Some(from) = mimeparser::get_from(headers) { - if context.is_self_addr(&from.addr).await? { - return Ok(true); - } - } - } if !context.get_config_bool(Config::MvboxMove).await? { return Ok(false); } @@ -2117,7 +2107,7 @@ async fn needs_move_to_mvbox( return Ok(false); } - if has_chat_version { + if headers.get_header_value(HeaderDef::SecureJoin).is_some() { Ok(true) } else if let Some(parent) = get_prefetch_parent_message(context, headers).await? { match parent.is_dc_message { @@ -2309,27 +2299,24 @@ pub(crate) async fn prefetch_should_download( return Ok(false); } - let is_chat_message = headers.get_header_value(HeaderDef::ChatVersion).is_some(); let accepted_contact = origin.is_known(); - let is_reply_to_chat_message = get_prefetch_parent_message(context, headers) - .await? - .map(|parent| match parent.is_dc_message { - MessengerMessage::No => false, - MessengerMessage::Yes | MessengerMessage::Reply => true, - }) - .unwrap_or_default(); - - let show_emails = - ShowEmails::from_i32(context.get_config_int(Config::ShowEmails).await?).unwrap_or_default(); - let show = is_autocrypt_setup_message - || match show_emails { - ShowEmails::Off => is_chat_message || is_reply_to_chat_message, - ShowEmails::AcceptedContacts => { - is_chat_message || is_reply_to_chat_message || accepted_contact - } + || headers.get_header_value(HeaderDef::SecureJoin).is_some() + || is_encrypted(headers) + || match ShowEmails::from_i32(context.get_config_int(Config::ShowEmails).await?) + .unwrap_or_default() + { + ShowEmails::Off => false, + ShowEmails::AcceptedContacts => accepted_contact, ShowEmails::All => true, - }; + } + || get_prefetch_parent_message(context, headers) + .await? + .map(|parent| match parent.is_dc_message { + MessengerMessage::No => false, + MessengerMessage::Yes | MessengerMessage::Reply => true, + }) + .unwrap_or_default(); let should_download = (show && !blocked_contact) || maybe_ndn; Ok(should_download) diff --git a/src/imap/imap_tests.rs b/src/imap/imap_tests.rs index 304b9b5e20..4133f1d7b7 100644 --- a/src/imap/imap_tests.rs +++ b/src/imap/imap_tests.rs @@ -94,14 +94,14 @@ fn test_build_sequence_sets() { async fn check_target_folder_combination( folder: &str, mvbox_move: bool, - chat_msg: bool, + is_encrypted: bool, expected_destination: &str, accepted_chat: bool, outgoing: bool, setupmessage: bool, ) -> Result<()> { println!( - "Testing: For folder {folder}, mvbox_move {mvbox_move}, chat_msg {chat_msg}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}" + "Testing: For folder {folder}, mvbox_move {mvbox_move}, is_encrypted {is_encrypted}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}" ); let t = TestContext::new_alice().await; @@ -124,7 +124,6 @@ async fn check_target_folder_combination( temp = format!( "Received: (Postfix, from userid 1000); Mon, 4 Dec 2006 14:51:39 +0100 (CET)\n\ {}\ - Subject: foo\n\ Message-ID: \n\ {}\ Date: Sun, 22 Mar 2020 22:37:57 +0000\n\ @@ -135,7 +134,12 @@ async fn check_target_folder_combination( } else { "From: bob@example.net\nTo: alice@example.org\n" }, - if chat_msg { "Chat-Version: 1.0\n" } else { "" }, + if is_encrypted { + "Subject: [...]\n\ + Content-Type: multipart/mixed; boundary=\"someboundary\"\n" + } else { + "Subject: foo\n" + }, ); temp.as_bytes() }; @@ -157,30 +161,31 @@ async fn check_target_folder_combination( assert_eq!( expected, actual.as_deref(), - "For folder {folder}, mvbox_move {mvbox_move}, chat_msg {chat_msg}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}: expected {expected:?}, got {actual:?}" + "For folder {folder}, mvbox_move {mvbox_move}, is_encrypted {is_encrypted}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}: expected {expected:?}, got {actual:?}" ); Ok(()) } -// chat_msg means that the message was sent by Delta Chat -// The tuples are (folder, mvbox_move, chat_msg, expected_destination) +// The tuples are (folder, mvbox_move, is_encrypted, expected_destination) const COMBINATIONS_ACCEPTED_CHAT: &[(&str, bool, bool, &str)] = &[ ("INBOX", false, false, "INBOX"), ("INBOX", false, true, "INBOX"), ("INBOX", true, false, "INBOX"), - ("INBOX", true, true, "DeltaChat"), - ("Spam", false, false, "INBOX"), // Move classical emails in accepted chats from Spam to Inbox, not 100% sure on this, we could also just never move non-chat-msgs + ("INBOX", true, true, "INBOX"), + ("Spam", false, false, "INBOX"), ("Spam", false, true, "INBOX"), - ("Spam", true, false, "INBOX"), // Move classical emails in accepted chats from Spam to Inbox, not 100% sure on this, we could also just never move non-chat-msgs + // Move unencrypted emails in accepted chats from Spam to INBOX, not 100% sure on this, we could + // also not move unencrypted emails or, if mvbox_move=1, move them to DeltaChat. + ("Spam", true, false, "INBOX"), ("Spam", true, true, "DeltaChat"), ]; -// These are the same as above, but non-chat messages in Spam stay in Spam +// These are the same as above, but unencrypted messages in Spam stay in Spam. const COMBINATIONS_REQUEST: &[(&str, bool, bool, &str)] = &[ ("INBOX", false, false, "INBOX"), ("INBOX", false, true, "INBOX"), ("INBOX", true, false, "INBOX"), - ("INBOX", true, true, "DeltaChat"), + ("INBOX", true, true, "INBOX"), ("Spam", false, false, "Spam"), ("Spam", false, true, "INBOX"), ("Spam", true, false, "Spam"), @@ -189,11 +194,11 @@ const COMBINATIONS_REQUEST: &[(&str, bool, bool, &str)] = &[ #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_target_folder_incoming_accepted() -> Result<()> { - for (folder, mvbox_move, chat_msg, expected_destination) in COMBINATIONS_ACCEPTED_CHAT { + for (folder, mvbox_move, is_encrypted, expected_destination) in COMBINATIONS_ACCEPTED_CHAT { check_target_folder_combination( folder, *mvbox_move, - *chat_msg, + *is_encrypted, expected_destination, true, false, @@ -206,11 +211,11 @@ async fn test_target_folder_incoming_accepted() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_target_folder_incoming_request() -> Result<()> { - for (folder, mvbox_move, chat_msg, expected_destination) in COMBINATIONS_REQUEST { + for (folder, mvbox_move, is_encrypted, expected_destination) in COMBINATIONS_REQUEST { check_target_folder_combination( folder, *mvbox_move, - *chat_msg, + *is_encrypted, expected_destination, false, false, @@ -224,11 +229,11 @@ async fn test_target_folder_incoming_request() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_target_folder_outgoing() -> Result<()> { // Test outgoing emails - for (folder, mvbox_move, chat_msg, expected_destination) in COMBINATIONS_ACCEPTED_CHAT { + for (folder, mvbox_move, is_encrypted, expected_destination) in COMBINATIONS_ACCEPTED_CHAT { check_target_folder_combination( folder, *mvbox_move, - *chat_msg, + *is_encrypted, expected_destination, true, true, @@ -242,11 +247,11 @@ async fn test_target_folder_outgoing() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_target_folder_setupmsg() -> Result<()> { // Test setupmessages - for (folder, mvbox_move, chat_msg, _expected_destination) in COMBINATIONS_ACCEPTED_CHAT { + for (folder, mvbox_move, is_encrypted, _expected_destination) in COMBINATIONS_ACCEPTED_CHAT { check_target_folder_combination( folder, *mvbox_move, - *chat_msg, + *is_encrypted, if folder == &"Spam" { "INBOX" } else { folder }, // Never move setup messages, except if they are in "Spam" false, true, diff --git a/src/imap/session.rs b/src/imap/session.rs index 8cf0a17de0..a01118b077 100644 --- a/src/imap/session.rs +++ b/src/imap/session.rs @@ -14,17 +14,20 @@ use crate::tools; /// Prefetch: /// - Message-ID to check if we already have the message. /// - In-Reply-To and References to check if message is a reply to chat message. -/// - Chat-Version to check if a message is a chat message /// - Autocrypt-Setup-Message to check if a message is an autocrypt setup message, /// not necessarily sent by Delta Chat. +/// +/// NB: We don't look at Chat-Version as we don't want any "better" handling for unencrypted +/// messages. const PREFETCH_FLAGS: &str = "(UID INTERNALDATE RFC822.SIZE BODY.PEEK[HEADER.FIELDS (\ MESSAGE-ID \ DATE \ X-MICROSOFT-ORIGINAL-MESSAGE-ID \ FROM \ IN-REPLY-TO REFERENCES \ - CHAT-VERSION \ - AUTO-SUBMITTED \ + CONTENT-TYPE \ + SECURE-JOIN \ + SUBJECT \ AUTOCRYPT-SETUP-MESSAGE\ )])"; diff --git a/src/receive_imf.rs b/src/receive_imf.rs index f8d2b909b0..2d734d2b3f 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -997,7 +997,7 @@ pub(crate) async fn receive_imf_inner( if let Some(is_bot) = mime_parser.is_bot { // If the message is auto-generated and was generated by Delta Chat, // mark the contact as a bot. - if mime_parser.get_header(HeaderDef::ChatVersion).is_some() { + if mime_parser.has_chat_version() { from_id.mark_bot(context, is_bot).await?; } } @@ -2949,7 +2949,7 @@ async fn apply_group_changes( } // Allow non-Delta Chat MUAs to add members. - if mime_parser.get_header(HeaderDef::ChatVersion).is_none() { + if !mime_parser.has_chat_version() { // Don't delete any members locally, but instead add absent ones to provide group // membership consistency for all members: new_members.extend(to_ids_flat.iter()); From 9e0e0dcaf7493f00ecba979a368f707b77aadddf Mon Sep 17 00:00:00 2001 From: iequidoo Date: Sun, 9 Nov 2025 04:25:34 -0300 Subject: [PATCH 2/6] feat: Don't download group messages unconditionally There was a comment that group messages should always be downloaded to avoid inconsistent group state, but this is solved by the group consistency algo nowadays in the sense that inconsistent group state won't spread to other members if we send to the group. Moreover, encrypted messages are now always downloaded, and unencrypted chat replies too, and as for ad-hoc groups, `Config::ShowEmails` controls everything. --- src/imap.rs | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/src/imap.rs b/src/imap.rs index d6acccc51c..a99c228339 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -27,7 +27,7 @@ use crate::calls::{create_fallback_ice_servers, create_ice_servers_from_metadata use crate::chat::{self, ChatId, ChatIdBlocked, add_device_msg}; use crate::chatlist_events; use crate::config::Config; -use crate::constants::{self, Blocked, Chattype, ShowEmails}; +use crate::constants::{self, Blocked, ShowEmails}; use crate::contact::{Contact, ContactId, Modifier, Origin}; use crate::context::Context; use crate::events::EventType; @@ -2229,21 +2229,6 @@ pub(crate) fn create_message_id() -> String { format!("{}{}", GENERATED_PREFIX, create_id()) } -/// Returns chat by prefetched headers. -async fn prefetch_get_chat( - context: &Context, - headers: &[mailparse::MailHeader<'_>], -) -> Result> { - let parent = get_prefetch_parent_message(context, headers).await?; - if let Some(parent) = &parent { - return Ok(Some( - chat::Chat::load_from_db(context, parent.get_chat_id()).await?, - )); - } - - Ok(None) -} - /// Determines whether the message should be downloaded based on prefetched headers. pub(crate) async fn prefetch_should_download( context: &Context, @@ -2262,14 +2247,6 @@ pub(crate) async fn prefetch_should_download( // We do not know the Message-ID or the Message-ID is missing (in this case, we create one in // the further process). - if let Some(chat) = prefetch_get_chat(context, headers).await? { - if chat.typ == Chattype::Group && !chat.id.is_special() { - // This might be a group command, like removing a group member. - // We really need to fetch this to avoid inconsistent group state. - return Ok(true); - } - } - let maybe_ndn = if let Some(from) = headers.get_header_value(HeaderDef::From_) { let from = from.to_ascii_lowercase(); from.contains("mailer-daemon") || from.contains("mail-daemon") From e95e40c94f006cf7a7a7681cead9cd0d44ba4304 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Sun, 9 Nov 2025 04:47:16 -0300 Subject: [PATCH 3/6] feat: Move all encrypted messages to mvbox if MvboxMove is on Before, only replies to chat messages were moved to the mvbox because we're removing Chat-Version from outer headers, but there's no much sense in moving only replies and not moving original messages and MDNs. Instead, move all encrypted messages. Users should be informed about this in UIs, so if a user has another PGP-capable MUA, probably they should disable MvboxMove. Moreover, untying this logic from References and In-Reply-To allows to remove them from outer headers too, the "Header Protection for Cryptographically Protected Email" RFC even suggests such a behavior: https://datatracker.ietf.org/doc/html/rfc9788#name-offering-more-ambitious-hea. --- src/imap.rs | 12 +----------- src/imap/imap_tests.rs | 4 ++-- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/imap.rs b/src/imap.rs index a99c228339..3f083af589 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -2106,17 +2106,7 @@ async fn needs_move_to_mvbox( // there may be a non-delta device that wants to handle it return Ok(false); } - - if headers.get_header_value(HeaderDef::SecureJoin).is_some() { - Ok(true) - } else if let Some(parent) = get_prefetch_parent_message(context, headers).await? { - match parent.is_dc_message { - MessengerMessage::No => Ok(false), - MessengerMessage::Yes | MessengerMessage::Reply => Ok(true), - } - } else { - Ok(false) - } + Ok(headers.get_header_value(HeaderDef::SecureJoin).is_some() || is_encrypted(headers)) } /// Try to get the folder meaning by the name of the folder only used if the server does not support XLIST. diff --git a/src/imap/imap_tests.rs b/src/imap/imap_tests.rs index 4133f1d7b7..03a4702dc6 100644 --- a/src/imap/imap_tests.rs +++ b/src/imap/imap_tests.rs @@ -171,7 +171,7 @@ const COMBINATIONS_ACCEPTED_CHAT: &[(&str, bool, bool, &str)] = &[ ("INBOX", false, false, "INBOX"), ("INBOX", false, true, "INBOX"), ("INBOX", true, false, "INBOX"), - ("INBOX", true, true, "INBOX"), + ("INBOX", true, true, "DeltaChat"), ("Spam", false, false, "INBOX"), ("Spam", false, true, "INBOX"), // Move unencrypted emails in accepted chats from Spam to INBOX, not 100% sure on this, we could @@ -185,7 +185,7 @@ const COMBINATIONS_REQUEST: &[(&str, bool, bool, &str)] = &[ ("INBOX", false, false, "INBOX"), ("INBOX", false, true, "INBOX"), ("INBOX", true, false, "INBOX"), - ("INBOX", true, true, "INBOX"), + ("INBOX", true, true, "DeltaChat"), ("Spam", false, false, "Spam"), ("Spam", false, true, "INBOX"), ("Spam", true, false, "Spam"), From a9949f87c240cc3b7f33c636b0fd69290bd99871 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Wed, 5 Nov 2025 19:22:00 -0300 Subject: [PATCH 4/6] feat: Do not copy Chat-Version header into outer part Chat-Version is used sometimes by Sieve filters to move messages to DeltaChat folder: https://github.com/mailcow/mailcow-dockerized/blob/37beed6ad93f259b97cad41877982bce95295629/data/conf/dovecot/global_sieve_before This probably prevents notifications to MUAs that don't watch DeltaChat but watch INBOX. There are however disadvantages to exposing Chat-Version: 1. Spam filters may not like this header and it is difficult or impossible to tell if `Chat-Version` plays role in rejecting the message or delivering it into Spam folder. If there is no such header visible to the spam filter, this possibility can be ruled out. 2. Replies to chat messages may have no `Chat-Version` but have to be moved anyway. 3. The user may have no control over the Sieve filter, but it comes preconfigured in mailcow, so it is not possible to disable it on the client. Thanks to link2xt for providing this motivation. NOTE: Old Delta Chat will assign partially downloaded replies to an ad-hoc group with the sender instead of the 1:1 chat, but we're removing partial downloads anyway. --- src/mimefactory.rs | 9 ++------- src/receive_imf.rs | 12 ++++++++++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index e9b76aa72d..6aed5e6dc5 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -947,8 +947,7 @@ impl MimeFactory { // // These are standard headers such as Date, In-Reply-To, References, which cannot be placed // anywhere else according to the standard. Placing headers here also allows them to be fetched - // individually over IMAP without downloading the message body. This is why Chat-Version is - // placed here. + // individually over IMAP without downloading the message body. let mut unprotected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); // Headers that MUST NOT (only) go into IMF header section: @@ -1063,11 +1062,7 @@ impl MimeFactory { mail_builder::headers::raw::Raw::new("[...]").into(), )); } - "in-reply-to" - | "references" - | "auto-submitted" - | "chat-version" - | "autocrypt-setup-message" => { + "in-reply-to" | "references" | "auto-submitted" | "autocrypt-setup-message" => { unprotected_headers.push(header.clone()); } _ => { diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 2d734d2b3f..81f4e35cd9 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -2414,7 +2414,14 @@ async fn lookup_chat_by_reply( // If this was a private message just to self, it was probably a private reply. // It should not go into the group then, but into the private chat. - if is_probably_private_reply(context, mime_parser, parent_chat_id).await? { + if is_probably_private_reply( + context, + mime_parser, + is_partial_download.is_some(), + parent_chat_id, + ) + .await? + { return Ok(None); } @@ -2561,6 +2568,7 @@ async fn lookup_or_create_adhoc_group( async fn is_probably_private_reply( context: &Context, mime_parser: &MimeMessage, + is_partial_download: bool, parent_chat_id: ChatId, ) -> Result { // Message cannot be a private reply if it has an explicit Chat-Group-ID header. @@ -2579,7 +2587,7 @@ async fn is_probably_private_reply( return Ok(false); } - if !mime_parser.has_chat_version() { + if !is_partial_download && !mime_parser.has_chat_version() { let chat_contacts = chat::get_chat_contacts(context, parent_chat_id).await?; if chat_contacts.len() == 2 && chat_contacts.contains(&ContactId::SELF) { return Ok(false); From 8c09ca369104f1f93179c38f964407462a793440 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Mon, 10 Nov 2025 05:36:10 -0300 Subject: [PATCH 5/6] feat: Do not copy Auto-Submitted header into outer part Before, copying Auto-Submitted to the outer headers was needed for moving such messages, e.g. multi-device sync messages, to the DeltaChat folder. Now all encrypted messages are moved. --- src/mimefactory.rs | 2 +- src/securejoin/securejoin_tests.rs | 36 +++++++++++++++++++++++------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 6aed5e6dc5..d746177ec0 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1062,7 +1062,7 @@ impl MimeFactory { mail_builder::headers::raw::Raw::new("[...]").into(), )); } - "in-reply-to" | "references" | "auto-submitted" | "autocrypt-setup-message" => { + "in-reply-to" | "references" | "autocrypt-setup-message" => { unprotected_headers.push(header.clone()); } _ => { diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 0462f2f9f6..ebc22e3ed2 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -60,13 +60,13 @@ async fn test_setup_contact_ex(case: SetupContactCase) { bob.set_config(Config::Displayname, Some("Bob Examplenet")) .await .unwrap(); - let alice_auto_submitted_hdr; + let alice_auto_submitted_val; match case { SetupContactCase::AliceIsBot => { alice.set_config_bool(Config::Bot, true).await.unwrap(); - alice_auto_submitted_hdr = "Auto-Submitted: auto-generated"; + alice_auto_submitted_val = "auto-generated"; } - _ => alice_auto_submitted_hdr = "Auto-Submitted: auto-replied", + _ => alice_auto_submitted_val = "auto-replied", }; assert_eq!( @@ -121,7 +121,7 @@ async fn test_setup_contact_ex(case: SetupContactCase) { ); let sent = alice.pop_sent_msg().await; - assert!(sent.payload.contains(alice_auto_submitted_hdr)); + assert!(!sent.payload.contains("Auto-Submitted:")); assert!(!sent.payload.contains("Alice Exampleorg")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); @@ -129,6 +129,10 @@ async fn test_setup_contact_ex(case: SetupContactCase) { msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-auth-required" ); + assert_eq!( + msg.get_header(HeaderDef::AutoSubmitted).unwrap(), + alice_auto_submitted_val + ); let bob_chat = bob.get_chat(&alice).await; assert_eq!(bob_chat.can_send(&bob).await.unwrap(), true); @@ -157,7 +161,7 @@ async fn test_setup_contact_ex(case: SetupContactCase) { // Check Bob sent the right message. let sent = bob.pop_sent_msg().await; - assert!(sent.payload.contains("Auto-Submitted: auto-replied")); + assert!(!sent.payload.contains("Auto-Submitted:")); assert!(!sent.payload.contains("Bob Examplenet")); let mut msg = alice.parse_msg(&sent).await; assert!(msg.was_encrypted()); @@ -171,6 +175,10 @@ async fn test_setup_contact_ex(case: SetupContactCase) { msg.get_header(HeaderDef::SecureJoinFingerprint).unwrap(), bob_fp ); + assert_eq!( + msg.get_header(HeaderDef::AutoSubmitted).unwrap(), + "auto-replied" + ); if case == SetupContactCase::WrongAliceGossip { let wrong_pubkey = GossipedKey { @@ -248,7 +256,7 @@ async fn test_setup_contact_ex(case: SetupContactCase) { // Check Alice sent the right message to Bob. let sent = alice.pop_sent_msg().await; - assert!(sent.payload.contains(alice_auto_submitted_hdr)); + assert!(!sent.payload.contains("Auto-Submitted:")); assert!(!sent.payload.contains("Alice Exampleorg")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); @@ -256,6 +264,10 @@ async fn test_setup_contact_ex(case: SetupContactCase) { msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-contact-confirm" ); + assert_eq!( + msg.get_header(HeaderDef::AutoSubmitted).unwrap(), + alice_auto_submitted_val + ); // Bob has verified Alice already. // @@ -465,13 +477,17 @@ async fn test_secure_join() -> Result<()> { alice.recv_msg_trash(&sent).await; let sent = alice.pop_sent_msg().await; - assert!(sent.payload.contains("Auto-Submitted: auto-replied")); + assert!(!sent.payload.contains("Auto-Submitted:")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); assert_eq!( msg.get_header(HeaderDef::SecureJoin).unwrap(), "vg-auth-required" ); + assert_eq!( + msg.get_header(HeaderDef::AutoSubmitted).unwrap(), + "auto-replied" + ); tcm.section("Step 4: Bob receives vg-auth-required, sends vg-request-with-auth"); bob.recv_msg_trash(&sent).await; @@ -503,7 +519,7 @@ async fn test_secure_join() -> Result<()> { } // Check Bob sent the right handshake message. - assert!(sent.payload.contains("Auto-Submitted: auto-replied")); + assert!(!sent.payload.contains("Auto-Submitted:")); let msg = alice.parse_msg(&sent).await; assert!(msg.was_encrypted()); assert_eq!( @@ -516,6 +532,10 @@ async fn test_secure_join() -> Result<()> { msg.get_header(HeaderDef::SecureJoinFingerprint).unwrap(), bob_fp ); + assert_eq!( + msg.get_header(HeaderDef::AutoSubmitted).unwrap(), + "auto-replied" + ); // Alice should not yet have Bob verified let contact_bob = alice.add_or_lookup_contact_no_key(&bob).await; From 88fae8b813cf1ee8967cfc406105f6b169ade7cc Mon Sep 17 00:00:00 2001 From: iequidoo Date: Mon, 10 Nov 2025 14:02:28 -0300 Subject: [PATCH 6/6] feat: Don't copy References and In-Reply-To to outer headers This implements the suggestion from https://www.rfc-editor.org/rfc/rfc9788.html#name-offering-more-ambitious-hea of "Header Protection for Cryptographically Protected Email". --- deltachat-rpc-client/tests/test_something.py | 14 ++------------ src/mimefactory.rs | 2 +- src/receive_imf/receive_imf_tests.rs | 5 +++-- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index 8e882896e2..1191ab1ac9 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -660,14 +660,12 @@ def test_download_limit_chat_assignment(acfactory, tmp_path, n_accounts): contact = alice.create_contact(account) alice_group.add_contact(contact) - if n_accounts == 2: - bob_chat_alice = bob.create_chat(alice) + bob_chat_alice = bob.create_chat(alice) bob.set_config("download_limit", str(download_limit)) alice_group.send_text("hi") snapshot = bob.wait_for_incoming_msg().get_snapshot() assert snapshot.text == "hi" - bob_group = snapshot.chat path = tmp_path / "large" path.write_bytes(os.urandom(download_limit + 1)) @@ -677,15 +675,7 @@ def test_download_limit_chat_assignment(acfactory, tmp_path, n_accounts): alice_group.send_file(str(path)) snapshot = bob.wait_for_incoming_msg().get_snapshot() assert snapshot.download_state == DownloadState.AVAILABLE - if n_accounts > 2: - assert snapshot.chat == bob_group - else: - # Group contains only Alice and Bob, - # so partially downloaded messages are - # hard to distinguish from private replies to group messages. - # - # Message may be a private reply, so we assign it to 1:1 chat with Alice. - assert snapshot.chat == bob_chat_alice + assert snapshot.chat == bob_chat_alice def test_markseen_contact_request(acfactory): diff --git a/src/mimefactory.rs b/src/mimefactory.rs index d746177ec0..8212a27c45 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1062,7 +1062,7 @@ impl MimeFactory { mail_builder::headers::raw::Raw::new("[...]").into(), )); } - "in-reply-to" | "references" | "autocrypt-setup-message" => { + "autocrypt-setup-message" => { unprotected_headers.push(header.clone()); } _ => { diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 1c3aefd581..3d26d4f991 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -4849,14 +4849,15 @@ async fn test_prefer_references_to_downloaded_msgs() -> Result<()> { let received = bob.recv_msg(&sent).await; assert_eq!(received.download_state, DownloadState::Available); assert_ne!(received.chat_id, bob_chat_id); - assert_eq!(received.chat_id, bob.get_chat(alice).await.id); + let bob_alice_chat_id = bob.get_chat(alice).await.id; + assert_eq!(received.chat_id, bob_alice_chat_id); let mut msg = Message::new(Viewtype::File); msg.set_file_from_bytes(alice, "file", file_bytes, None)?; let sent = alice.send_msg(alice_chat_id, &mut msg).await; let received = bob.recv_msg(&sent).await; assert_eq!(received.download_state, DownloadState::Available); - assert_eq!(received.chat_id, bob_chat_id); + assert_eq!(received.chat_id, bob_alice_chat_id); Ok(()) }