feat: Move unverified messages which don't introduce a new Autocrypt key from protected 1:1 chats to ad-hoc groups

This prevents protected 1:1 chats from breaking by messages sent by classical MUAs.

TODO: Some `tests::verified_chats` tests are failing, but as far as i see from the log, the tests
must be fixed, the change itself looks ok. Not fixing the tests now, not sure we will merge this as
it's just POC.

src/receive_imf.rs

@@ -23,6 +23,7 @@ use crate::ephemeral::{stock_ephemeral_timer_changed, Timer as EphemeralTimer};
 use crate::events::EventType;
 use crate::headerdef::{HeaderDef, HeaderDefMap};
 use crate::imap::{markseen_on_imap_table, GENERATED_PREFIX};
+use crate::key::DcKey;
 use crate::location;
 use crate::log::LogExt;
 use crate::message::{
@@ -601,11 +602,7 @@ async fn add_parts(
                 context,
                 mime_parser,
                 is_partial_download.is_some(),
-                if test_normal_chat.is_none() {
-                    allow_creation
-                } else {
-                    true
-                },
+                test_normal_chat.is_some() || allow_creation,
                 create_blocked,
                 from_id,
                 to_ids,
@@ -617,6 +614,7 @@ async fn add_parts(
             }
         }
 
+        loop {
         // if the chat is somehow blocked but we want to create a non-blocked chat,
         // unblock the chat
         if chat_id_blocked != Blocked::Not && create_blocked != Blocked::Yes {
@@ -705,7 +703,7 @@ async fn add_parts(
                 }
             };
 
-            if let Some(chat) = test_normal_chat {
+            if let Some(chat) = test_normal_chat.as_ref() {
                 chat_id = Some(chat.id);
                 chat_id_blocked = chat.blocked;
             } else if allow_creation {
@@ -719,7 +717,8 @@ async fn add_parts(
                 }
             }
 
-            if let Some(chat_id) = chat_id {
+            let chat_id_ref = &mut chat_id;
+            if let Some(chat_id) = *chat_id_ref {
                 if chat_id_blocked != Blocked::Not {
                     if chat_id_blocked != create_blocked {
                         chat_id.set_blocked(context, create_blocked).await?;
@@ -766,7 +765,44 @@ async fn add_parts(
                         // That's why the config is checked here, and not above.
                         && context.get_config_bool(Config::VerifiedOneOnOneChats).await?
                     {
-                        new_protection = ProtectionStatus::ProtectionBroken;
+                        let decryption_info = &mime_parser.decryption_info;
+                        new_protection =
+                            match decryption_info.autocrypt_header.as_ref().filter(|ah| {
+                                Some(&ah.public_key.fingerprint())
+                                    != decryption_info
+                                        .peerstate
+                                        .as_ref()
+                                        .and_then(|p| p.verified_key_fingerprint.as_ref())
+                            }) {
+                                None => {
+                                    if let Some(new_chat_id) = create_adhoc_group(
+                                        context,
+                                        mime_parser,
+                                        create_blocked,
+                                        &[ContactId::SELF, from_id],
+                                    )
+                                    .await
+                                    .context("could not create ad hoc group")?
+                                    {
+                                        info!(
+                                            context,
+                                            "Moving message to a new ad-hoc group keeping 1:1 chat \
+                                            protection.",
+                                        );
+                                        *chat_id_ref = Some(new_chat_id);
+                                        chat_id_blocked = create_blocked;
+                                        continue;
+                                    } else {
+                                        warn!(
+                                            context,
+                                            "Rejected to create an ad-hoc group, but keeping 1:1 chat \
+                                            protection.",
+                                        );
+                                    }
+                                    chat.protected
+                                }
+                                Some(_) => ProtectionStatus::ProtectionBroken,
+                            };
                     }
                     if chat.protected != new_protection {
                         // The message itself will be sorted under the device message since the device
@@ -795,6 +831,8 @@ async fn add_parts(
         } else {
             MessageState::InFresh
         };
+        break;
+        }
     } else {
         // Outgoing
 
@@ -1481,7 +1519,16 @@ 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, to_ids, from_id, mime_parser, parent_chat.id).await? {
+    if is_probably_private_reply(
+        context,
+        to_ids,
+        from_id,
+        mime_parser,
+        parent_chat.id,
+        &parent_chat.grpid,
+    )
+    .await?
+    {
         return Ok(None);
     }
 
@@ -1511,13 +1558,14 @@ async fn is_probably_private_reply(
     from_id: ContactId,
     mime_parser: &MimeMessage,
     parent_chat_id: ChatId,
+    parent_chat_grpid: &str,
 ) -> Result<bool> {
     // Usually we don't want to show private replies in the parent chat, but in the
     // 1:1 chat with the sender.
     //
-    // There is one exception: Classical MUA replies to two-member groups
-    // should be assigned to the group chat. We restrict this exception to classical emails, as chat-group-messages
-    // contain a Chat-Group-Id header and can be sorted into the correct chat this way.
+    // An exception is replies to 2-member groups from classical MUAs or to 2-member ad-hoc
+    // groups. Such messages can't contain a Chat-Group-Id header and need to be sorted purely by
+    // References/In-Reply-To.
 
     let private_message =
         (to_ids == [ContactId::SELF]) || (from_id == ContactId::SELF && to_ids.len() == 1);
@@ -1525,7 +1573,7 @@ async fn is_probably_private_reply(
         return Ok(false);
     }
 
-    if !mime_parser.has_chat_version() {
+    if !mime_parser.has_chat_version() || parent_chat_grpid.is_empty() {
         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);
@@ -1560,6 +1608,10 @@ async fn create_or_lookup_group(
             member_ids.push(ContactId::SELF);
         }
 
+        if member_ids.len() < 3 {
+            info!(context, "Not creating ad-hoc group: too few contacts.");
+            return Ok(None);
+        }
         let res = create_adhoc_group(context, mime_parser, create_blocked, &member_ids)
             .await
             .context("could not create ad hoc group")?
@@ -1584,7 +1636,8 @@ async fn create_or_lookup_group(
     // they belong to the group because of the Chat-Group-Id or Message-Id header
     if let Some(chat_id) = chat_id {
         if !mime_parser.has_chat_version()
-            && is_probably_private_reply(context, to_ids, from_id, mime_parser, chat_id).await?
+            && is_probably_private_reply(context, to_ids, from_id, mime_parser, chat_id, &grpid)
+                .await?
         {
             return Ok(None);
         }
@@ -2213,11 +2266,6 @@ async fn create_adhoc_group(
         return Ok(None);
     }
 
-    if member_ids.len() < 3 {
-        info!(context, "Not creating ad-hoc group: too few contacts.");
-        return Ok(None);
-    }
-
     // use subject as initial chat name
     let grpname = mime_parser
         .get_subject()

src/tests/verified_chats.rs

@@ -4,7 +4,7 @@ use pretty_assertions::assert_eq;
 use crate::chat::{Chat, ProtectionStatus};
 use crate::chatlist::Chatlist;
 use crate::config::Config;
-use crate::constants::DC_GCL_FOR_FORWARDING;
+use crate::constants::{Chattype, DC_GCL_FOR_FORWARDING};
 use crate::contact::VerifiedStatus;
 use crate::contact::{Contact, Origin};
 use crate::message::{Message, Viewtype};
@@ -12,7 +12,9 @@ use crate::mimefactory::MimeFactory;
 use crate::mimeparser::SystemMessage;
 use crate::receive_imf::receive_imf;
 use crate::stock_str;
-use crate::test_utils::{get_chat_msg, mark_as_verified, TestContext, TestContextManager};
+use crate::test_utils::{
+    get_chat_msg, mark_as_verified, TestContext, TestContextManager,
+};
 use crate::{e2ee, message};
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -742,6 +744,50 @@ async fn test_create_oneonone_chat_with_former_verified_contact() -> Result<()>
     Ok(())
 }
 
+/// Some messages are sent unencrypted, but they mustn't break a verified chat protection.
+/// They must go to a new 2-member ad-hoc group instead.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_unencrypted() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = tcm.alice().await;
+    let bob = tcm.bob().await;
+    enable_verified_oneonone_chats(&[&alice]).await;
+    mark_as_verified(&alice, &bob).await;
+    alice.create_chat(&bob).await;
+
+    let msg = tcm.send_recv(&bob, &alice, "hi").await;
+    assert!(!msg.get_showpadlock());
+    let alice_chat = Chat::load_from_db(&alice, msg.chat_id).await?;
+    assert_eq!(alice_chat.typ, Chattype::Group);
+
+    // The next unencrypted message must get to the same ad-hoc group thanks to "In-Reply-To".
+    let msg = tcm.send_recv(&bob, &alice, "hi again").await;
+    assert!(!msg.get_showpadlock());
+    assert_eq!(msg.chat_id, alice_chat.id);
+    let alice_chat = Chat::load_from_db(&alice, msg.chat_id).await?;
+    assert_eq!(alice_chat.typ, Chattype::Group);
+
+    // This message is missed by Alice.
+    let chat_id = bob.get_chat(&alice).await.id;
+    bob.send_text(chat_id, "hi to the void").await;
+
+    // But the next message must get to the same ad-hoc group thanks to "References".
+    let msg = tcm.send_recv(&bob, &alice, "hi in a new group").await;
+    assert!(!msg.get_showpadlock());
+    assert_eq!(msg.chat_id, alice_chat.id);
+    let alice_chat = Chat::load_from_db(&alice, msg.chat_id).await?;
+    assert_eq!(alice_chat.typ, Chattype::Group);
+
+    let alice_chat = alice.get_chat(&bob).await;
+    assert!(alice_chat.is_protected());
+    assert!(!alice_chat.is_protection_broken());
+    alice
+        .golden_test_chat(alice_chat.id, "verified_chats_test_unencrypted")
+        .await;
+
+    Ok(())
+}
+
 // ============== Helper Functions ==============
 
 async fn assert_verified(this: &TestContext, other: &TestContext, protected: ProtectionStatus) {

test-data/golden/verified_chats_test_unencrypted

@@ -0,0 +1,4 @@
+Single#Chat#10: bob@example.net [bob@example.net] 🛡️
+--------------------------------------------------------------------------------
+Msg#10: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️]
+--------------------------------------------------------------------------------