feat: Protect From name for verified chats and To names for encrypted chats (#5166)

If a display name should be protected (i.e. opportunistically encrypted), only put the corresponding
address to the unprotected headers. We protect the From display name only for verified chats,
otherwise this would be incompatible with Thunderbird and K-9 who don't use display names from the
encrypted part. Still, we always protect To display names as compatibility seems less critical here.

When receiving a messge, overwrite the From display name but not the whole From field as that would
allow From forgery. For the To field we don't really care. Anyway as soon as we receive a message
from the user, the display name will be corrected.

Co-authored-by: iequidoo <dgreshilov@gmail.com>

src/mimefactory.rs

@@ -5,6 +5,7 @@ use std::collections::HashSet;
 use anyhow::{bail, ensure, Context as _, Result};
 use base64::Engine as _;
 use chrono::TimeZone;
+use email::Mailbox;
 use format_flowed::{format_flowed, format_flowed_quote};
 use lettre_email::{Address, Header, MimeMultipartType, PartBuilder};
 use tokio::fs;
@@ -552,8 +553,7 @@ impl<'a> MimeFactory<'a> {
         // Start with Internet Message Format headers in the order of the standard example
         // <https://datatracker.ietf.org/doc/html/rfc5322#appendix-A.1.1>.
         let from_header = Header::new_with_value("From".into(), vec![from]).unwrap();
-        headers.unprotected.push(from_header.clone());
-        headers.protected.push(from_header);
+        headers.protected.push(from_header.clone());
 
         if let Some(sender_displayname) = &self.sender_displayname {
             let sender =
@@ -563,8 +563,8 @@ impl<'a> MimeFactory<'a> {
                 .push(Header::new_with_value("Sender".into(), vec![sender]).unwrap());
         }
         headers
-            .unprotected
-            .push(Header::new_with_value("To".into(), to).unwrap());
+            .protected
+            .push(Header::new_with_value("To".into(), to.clone()).unwrap());
 
         let subject_str = self.subject_str(context).await?;
         let encoded_subject = if subject_str
@@ -681,6 +681,46 @@ impl<'a> MimeFactory<'a> {
             encrypt_helper.should_encrypt(context, e2ee_guaranteed, &peerstates)?;
         let is_encrypted = should_encrypt && !force_plaintext;
 
+        if is_encrypted {
+            headers.unprotected.insert(
+                0,
+                Header::new_with_value(
+                    "To".into(),
+                    to.into_iter()
+                        .map(|header| match header {
+                            Address::Mailbox(mb) => Address::Mailbox(Mailbox {
+                                address: mb.address,
+                                name: None,
+                            }),
+                            Address::Group(name, participants) => Address::new_group(
+                                name,
+                                participants
+                                    .into_iter()
+                                    .map(|mb| Mailbox {
+                                        address: mb.address,
+                                        name: None,
+                                    })
+                                    .collect(),
+                            ),
+                        })
+                        .collect::<Vec<_>>(),
+                )
+                .unwrap(),
+            );
+        }
+        if is_encrypted && verified {
+            headers.unprotected.insert(
+                0,
+                Header::new_with_value(
+                    "From".into(),
+                    vec![Address::new_mailbox(self.from_addr.clone())],
+                )
+                .unwrap(),
+            );
+        } else {
+            headers.unprotected.insert(0, from_header);
+        }
+
         let (main_part, parts) = match self.loaded {
             Loaded::Message { .. } => {
                 self.render_message(context, &mut headers, &grpimage, is_encrypted)
@@ -827,7 +867,6 @@ impl<'a> MimeFactory<'a> {
 
             message
         } else {
-            // Store hidden headers in the inner unencrypted message.
             let message = headers
                 .hidden
                 .into_iter()
@@ -835,8 +874,6 @@ impl<'a> MimeFactory<'a> {
             let message = PartBuilder::new()
                 .message_type(MimeMultipartType::Mixed)
                 .child(message.build());
-
-            // Store protected headers in the outer message.
             let message = headers
                 .protected
                 .iter()

src/mimeparser.rs

@@ -302,7 +302,7 @@ impl MimeMessage {
         // them in signed-only emails, but has no value currently.
         Self::remove_secured_headers(&mut headers);
 
-        let from = from.context("No from in message")?;
+        let mut from = from.context("No from in message")?;
         let private_keyring = load_self_secret_keyring(context).await?;
 
         let mut decryption_info =
@@ -398,6 +398,7 @@ impl MimeMessage {
             if let (Some(inner_from), true) = (inner_from, !signatures.is_empty()) {
                 if addr_cmp(&inner_from.addr, &from.addr) {
                     from_is_signed = true;
+                    from = inner_from;
                 } else {
                     // There is a From: header in the encrypted &
                     // signed part, but it doesn't match the outer one.

src/tests/verified_chats.rs

@@ -860,6 +860,35 @@ async fn test_verified_member_added_reordering() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_no_unencrypted_name_if_verified() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    for verified in [false, true] {
+        let alice = tcm.alice().await;
+        let bob = tcm.bob().await;
+        bob.set_config(Config::Displayname, Some("Bob Smith"))
+            .await?;
+        if verified {
+            enable_verified_oneonone_chats(&[&bob]).await;
+            mark_as_verified(&bob, &alice).await;
+        } else {
+            tcm.send_recv_accept(&alice, &bob, "hi").await;
+        }
+
+        let chat_id = bob.create_chat(&alice).await.id;
+        let msg = &bob.send_text(chat_id, "hi").await;
+
+        assert_eq!(msg.payload.contains("Bob Smith"), !verified);
+        assert!(msg.payload.contains("BEGIN PGP MESSAGE"));
+
+        let msg = alice.recv_msg(msg).await;
+        let contact = Contact::get_by_id(&alice, msg.from_id).await?;
+
+        assert_eq!(Contact::get_display_name(&contact), "Bob Smith");
+    }
+    Ok(())
+}
+
 // ============== Helper Functions ==============
 
 async fn assert_verified(this: &TestContext, other: &TestContext, protected: ProtectionStatus) {