diff --git a/src/decrypt.rs b/src/decrypt.rs index 8e242a57ea..8c3b9de150 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -178,7 +178,8 @@ mod tests { let bob = TestContext::new_bob().await; receive_imf(&bob, attachment_mime, false).await?; let msg = bob.get_last_msg().await; - assert_eq!(msg.text, "Hello from Thunderbird!"); + // Subject should be prepended because the attachment doesn't have "Chat-Version". + assert_eq!(msg.text, "Hello, Bob! – Hello from Thunderbird!"); Ok(()) } diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 4241dcd65b..20923acef0 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -246,6 +246,7 @@ impl MimeMessage { MimeMessage::merge_headers( context, &mut headers, + &mut headers_removed, &mut recipients, &mut past_members, &mut from, @@ -273,6 +274,7 @@ impl MimeMessage { MimeMessage::merge_headers( context, &mut headers, + &mut headers_removed, &mut recipients, &mut past_members, &mut from, @@ -446,26 +448,11 @@ impl MimeMessage { }); if let (Ok(mail), true) = (mail, is_encrypted) { if !signatures.is_empty() { - // Remove unsigned opportunistically protected headers from messages considered - // Autocrypt-encrypted / displayed with padlock. - // For "Subject" see . - for h in [ - HeaderDef::Subject, - HeaderDef::ChatGroupId, - HeaderDef::ChatGroupName, - HeaderDef::ChatGroupNameChanged, - HeaderDef::ChatGroupNameTimestamp, - HeaderDef::ChatGroupAvatar, - HeaderDef::ChatGroupMemberRemoved, - HeaderDef::ChatGroupMemberAdded, - HeaderDef::ChatGroupMemberTimestamps, - HeaderDef::ChatGroupPastMembers, - HeaderDef::ChatDelete, - HeaderDef::ChatEdit, - HeaderDef::ChatUserAvatar, - ] { - remove_header(&mut headers, h.get_headername(), &mut headers_removed); - } + // Unsigned "Subject" mustn't be prepended to messages shown as encrypted + // (). + // Other headers are removed by `MimeMessage::merge_headers()` except for "List-ID". + remove_header(&mut headers, "subject", &mut headers_removed); + remove_header(&mut headers, "list-id", &mut headers_removed); } // let known protected headers from the decrypted @@ -478,6 +465,7 @@ impl MimeMessage { MimeMessage::merge_headers( context, &mut headers, + &mut headers_removed, &mut recipients, &mut past_members, &mut inner_from, @@ -1558,6 +1546,7 @@ impl MimeMessage { fn merge_headers( context: &Context, headers: &mut HashMap, + headers_removed: &mut HashSet, recipients: &mut Vec, past_members: &mut Vec, from: &mut Option, @@ -1565,6 +1554,12 @@ impl MimeMessage { chat_disposition_notification_to: &mut Option, fields: &[mailparse::MailHeader<'_>], ) { + headers.retain(|k, _| { + !is_protected(k) || { + headers_removed.insert(k.to_string()); + false + } + }); for field in fields { // lowercasing all headers is technically not correct, but makes things work better let key = field.get_key().to_lowercase(); @@ -2005,6 +2000,32 @@ pub(crate) fn parse_message_id(ids: &str) -> Result { } } +/// Returns whether the outer header value must be ignored if the message contains a signed (and +/// optionally encrypted) part. +/// +/// NB: There are known cases when Subject and List-ID only appear in the outer headers of +/// signed-only messages. Such messages are shown as unencrypted anyway. +fn is_protected(key: &str) -> bool { + key.starts_with("chat-") + || matches!( + key, + "return-path" + | "auto-submitted" + | "autocrypt-setup-message" + | "date" + | "from" + | "sender" + | "reply-to" + | "to" + | "cc" + | "bcc" + | "message-id" + | "in-reply-to" + | "references" + | "secure-join" + ) +} + /// Returns if the header is hidden and must be ignored in the IMF section. pub(crate) fn is_hidden(key: &str) -> bool { matches!( diff --git a/src/mimeparser/mimeparser_tests.rs b/src/mimeparser/mimeparser_tests.rs index 4ca2d0c5c0..73ea93b211 100644 --- a/src/mimeparser/mimeparser_tests.rs +++ b/src/mimeparser/mimeparser_tests.rs @@ -1402,6 +1402,26 @@ async fn test_x_microsoft_original_message_id_precedence() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_extra_imf_chat_header() -> Result<()> { + let mut tcm = TestContextManager::new(); + let t = &tcm.alice().await; + let chat_id = t.get_self_chat().await.id; + + chat::send_text_msg(t, chat_id, "hi!".to_string()).await?; + let sent_msg = t.pop_sent_msg().await; + // Check removal of some nonexistent "Chat-*" header to protect the code from future breakages. + let payload = sent_msg + .payload + .replace("Message-ID:", "Chat-Forty-Two: 42\r\nMessage-ID:"); + let msg = MimeMessage::from_bytes(t, payload.as_bytes(), None) + .await + .unwrap(); + assert!(msg.headers.contains_key("chat-version")); + assert!(!msg.headers.contains_key("chat-forty-two")); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_long_in_reply_to() -> Result<()> { let t = TestContext::new_alice().await; diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 170b2d8a11..8be0e2beb2 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -3682,6 +3682,24 @@ async fn test_unsigned_chat_group_hdr() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_ignore_protected_headers_in_outer_msg() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let bob_chat_id = tcm.send_recv_accept(alice, bob, "hi").await.chat_id; + send_text_msg(bob, bob_chat_id, "hi all!".to_string()).await?; + let mut sent_msg = bob.pop_sent_msg().await; + sent_msg.payload = sent_msg.payload.replace( + "Chat-Version:", + "Auto-Submitted: auto-generated\r\nChat-Version:", + ); + alice.recv_msg(&sent_msg).await; + let ab_contact = alice.add_or_lookup_contact(bob).await; + assert!(!ab_contact.is_bot()); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_sync_member_list_on_rejoin() -> Result<()> { let mut tcm = TestContextManager::new();