Completely disable Autocrypt & Authres-checking for mailing lists (#3765)

* Because both only make problems with mailing lists, it's easiest to just disable them. If we want, we can make them work properly with mailing lists one day and re-enable them, but this needs some further thoughts.

Part of #3701

* Use load_from_db() in more tests

* clippy

* Changelog

* Downgrade warning to info, improve message

* Use lifetimes instead of cloning

CHANGELOG.md

@@ -3,6 +3,8 @@
 ## Unreleased
 
 ### Changes
+- Disable Autocrypt & Authres-checking for mailing lists,
+  because they don't work well with mailing lists #3765
 - Refactor: Remove the remaining AsRef<str> #3669
 - Add more logging to `fetch_many_msgs` and refactor it #3811
 - Small speedup #3780

src/authres.rs

@@ -45,7 +45,7 @@ pub(crate) async fn handle_authres(
     compute_dkim_results(context, authres, &from_domain, message_time).await
 }
 
-#[derive(Default, Debug)]
+#[derive(Debug)]
 pub(crate) struct DkimResults {
     /// Whether DKIM passed for this particular e-mail.
     pub dkim_passed: bool,
@@ -358,6 +358,7 @@ mod tests {
 
     use crate::aheader::EncryptPreference;
     use crate::e2ee;
+    use crate::message;
     use crate::mimeparser;
     use crate::peerstate::Peerstate;
     use crate::securejoin::get_securejoin_qr;
@@ -750,4 +751,86 @@ Authentication-Results: dkim=";
 
         Ok(())
     }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_autocrypt_in_mailinglist_ignored() -> Result<()> {
+        let mut tcm = TestContextManager::new();
+        let alice = tcm.alice().await;
+        let bob = tcm.bob().await;
+
+        let alice_bob_chat = alice.create_chat(&bob).await;
+        let bob_alice_chat = bob.create_chat(&alice).await;
+        let mut sent = alice.send_text(alice_bob_chat.id, "hellooo").await;
+        sent.payload
+            .insert_str(0, "List-Post: <mailto:deltachat-community.example.net>\n");
+        bob.recv_msg(&sent).await;
+        let peerstate = Peerstate::from_addr(&bob, "alice@example.org").await?;
+        assert!(peerstate.is_none());
+
+        // Do the same without the mailing list header, this time the peerstate should be accepted
+        let sent = alice
+            .send_text(alice_bob_chat.id, "hellooo without mailing list")
+            .await;
+        bob.recv_msg(&sent).await;
+        let peerstate = Peerstate::from_addr(&bob, "alice@example.org").await?;
+        assert!(peerstate.is_some());
+
+        // This also means that Bob can now write encrypted to Alice:
+        let mut sent = bob
+            .send_text(bob_alice_chat.id, "hellooo in the mailinglist again")
+            .await;
+        assert!(sent.load_from_db().await.get_showpadlock());
+
+        // But if Bob writes to a mailing list, Alice doesn't show a padlock
+        // since she can't verify the signature without accepting Bob's key:
+        sent.payload
+            .insert_str(0, "List-Post: <mailto:deltachat-community.example.net>\n");
+        let rcvd = alice.recv_msg(&sent).await;
+        assert!(!rcvd.get_showpadlock());
+        assert_eq!(&rcvd.text.unwrap(), "hellooo in the mailinglist again");
+
+        Ok(())
+    }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_authres_in_mailinglist_ignored() -> Result<()> {
+        let mut tcm = TestContextManager::new();
+        let alice = tcm.alice().await;
+        let bob = tcm.bob().await;
+
+        // Assume Bob received an email from something@example.net with
+        // correct DKIM -> `set_dkim_works()` was called
+        set_dkim_works_timestamp(&bob, "example.org", time()).await?;
+        // And Bob knows his server's authserv-id
+        bob.set_config(Config::AuthservIdCandidates, Some("example.net"))
+            .await?;
+
+        let alice_bob_chat = alice.create_chat(&bob).await;
+        let mut sent = alice.send_text(alice_bob_chat.id, "hellooo").await;
+        sent.payload
+            .insert_str(0, "List-Post: <mailto:deltachat-community.example.net>\n");
+        sent.payload
+            .insert_str(0, "Authentication-Results: example.net; dkim=fail\n");
+        let rcvd = bob.recv_msg(&sent).await;
+        assert!(rcvd.error.is_none());
+
+        // Do the same without the mailing list header, this time the failed
+        // authres isn't ignored
+        let mut sent = alice
+            .send_text(alice_bob_chat.id, "hellooo without mailing list")
+            .await;
+        sent.payload
+            .insert_str(0, "Authentication-Results: example.net; dkim=fail\n");
+        let rcvd = bob.recv_msg(&sent).await;
+
+        // Disallowing keychanges is disabled for now:
+        // assert!(rcvd.error.unwrap().contains("DKIM failed"));
+        // The message info should contain a warning:
+        assert!(message::get_msg_info(&bob, rcvd.id)
+            .await
+            .unwrap()
+            .contains("KEYCHANGES NOT ALLOWED"));
+
+        Ok(())
+    }
 }

src/contact.rs

@@ -1578,7 +1578,6 @@ mod tests {
 
     use crate::chat::{get_chat_contacts, send_text_msg, Chat};
     use crate::chatlist::Chatlist;
-    use crate::message::Message;
     use crate::receive_imf::receive_imf;
     use crate::test_utils::{self, TestContext, TestContextManager};
 
@@ -2328,7 +2327,7 @@ CCCB 5AA9 F6E1 141C 9431
         let sent_msg = alice1.pop_sent_msg().await;
 
         // Message is not encrypted.
-        let message = Message::load_from_db(&alice1, sent_msg.sender_msg_id).await?;
+        let message = sent_msg.load_from_db().await;
         assert!(!message.get_showpadlock());
 
         // Alice's second devices receives a copy of outgoing message.
@@ -2355,7 +2354,7 @@ CCCB 5AA9 F6E1 141C 9431
         let sent_msg = alice1.pop_sent_msg().await;
 
         // Second message is encrypted.
-        let message = Message::load_from_db(&alice1, sent_msg.sender_msg_id).await?;
+        let message = sent_msg.load_from_db().await;
         assert!(message.get_showpadlock());
 
         // Alice's second devices receives a copy of second outgoing message.
@@ -2410,7 +2409,7 @@ CCCB 5AA9 F6E1 141C 9431
         let sent_msg = alice1.pop_sent_msg().await;
 
         // The message is encrypted.
-        let message = Message::load_from_db(&alice1, sent_msg.sender_msg_id).await?;
+        let message = sent_msg.load_from_db().await;
         assert!(message.get_showpadlock());
 
         // Alice's second device receives a copy of the outgoing message.

src/decrypt.rs

@@ -6,10 +6,11 @@ use anyhow::{Context as _, Result};
 use mailparse::ParsedMail;
 
 use crate::aheader::{Aheader, EncryptPreference};
-use crate::authres;
 use crate::authres::handle_authres;
+use crate::authres::{self, DkimResults};
 use crate::contact::addr_cmp;
 use crate::context::Context;
+use crate::headerdef::{HeaderDef, HeaderDefMap};
 use crate::key::{DcKey, Fingerprint, SignedPublicKey, SignedSecretKey};
 use crate::keyring::Keyring;
 use crate::log::LogExt;
@@ -62,6 +63,27 @@ pub(crate) async fn prepare_decryption(
     message_time: i64,
     is_thunderbird: bool,
 ) -> Result<DecryptionInfo> {
+    if mail.headers.get_header(HeaderDef::ListPost).is_some() {
+        if mail.headers.get_header(HeaderDef::Autocrypt).is_some() {
+            info!(
+                context,
+                "Ignoring autocrypt header since this is a mailing list message. \
+                NOTE: For privacy reasons, the mailing list software should remove Autocrypt headers."
+            );
+        }
+        return Ok(DecryptionInfo {
+            from: from.to_string(),
+            autocrypt_header: None,
+            peerstate: None,
+            message_time,
+            dkim_results: DkimResults {
+                dkim_passed: false,
+                dkim_should_work: false,
+                allow_keychange: true,
+            },
+        });
+    }
+
     let mut autocrypt_header = Aheader::from_headers(from, &mail.headers)
         .ok_or_log_msg(context, "Failed to parse Autocrypt header")
         .flatten();
@@ -93,7 +115,7 @@ pub(crate) async fn prepare_decryption(
     })
 }
 
-#[derive(Default, Debug)]
+#[derive(Debug)]
 pub struct DecryptionInfo {
     /// The From address. This is the address from the unnencrypted, outer
     /// From header.

src/download.rs

@@ -428,9 +428,7 @@ mod tests {
             .await?;
         alice.flush_status_updates().await?;
         let sent2 = alice.pop_sent_msg().await;
-        let sent2_rfc724_mid = Message::load_from_db(&alice, sent2.sender_msg_id)
+        let sent2_rfc724_mid = sent2.load_from_db().await.rfc724_mid;
-            .await?
-            .rfc724_mid;
 
         // not downloading the status update results in an placeholder
         receive_imf_inner(

src/mimefactory.rs

@@ -1733,11 +1733,11 @@ mod tests {
         }
         async fn get_subject(
             t: &TestContext,
-            sent: crate::test_utils::SentMessage,
+            sent: crate::test_utils::SentMessage<'_>,
         ) -> Result<String> {
             let parsed_subject = t.parse_msg(&sent).await.get_subject().unwrap();
 
-            let sent_msg = Message::load_from_db(t, sent.sender_msg_id).await?;
+            let sent_msg = sent.load_from_db().await;
             assert_eq!(parsed_subject, sent_msg.subject);
 
             Ok(parsed_subject)

src/test_utils.rs

@@ -380,13 +380,13 @@ impl TestContext {
     /// table.  Messages are returned in the order they have been sent.
     ///
     /// Panics if there is no message or on any error.
-    pub async fn pop_sent_msg(&self) -> SentMessage {
+    pub async fn pop_sent_msg(&self) -> SentMessage<'_> {
         self.pop_sent_msg_opt(Duration::from_secs(3))
             .await
             .expect("no sent message found in jobs table")
     }
 
-    pub async fn pop_sent_msg_opt(&self, timeout: Duration) -> Option<SentMessage> {
+    pub async fn pop_sent_msg_opt(&self, timeout: Duration) -> Option<SentMessage<'_>> {
         let start = Instant::now();
         let (rowid, msg_id, payload, recipients) = loop {
             let row = self
@@ -428,6 +428,7 @@ impl TestContext {
         Some(SentMessage {
             payload,
             sender_msg_id: msg_id,
+            sender_context: &self.ctx,
             recipients,
         })
     }
@@ -439,7 +440,7 @@ impl TestContext {
     /// peerstates will be updated.  Later receiving the message using [recv_msg] is
     /// unlikely to be affected as the peerstate would be processed again in exactly the
     /// same way.
-    pub async fn parse_msg(&self, msg: &SentMessage) -> MimeMessage {
+    pub async fn parse_msg(&self, msg: &SentMessage<'_>) -> MimeMessage {
         MimeMessage::from_bytes(&self.ctx, msg.payload().as_bytes())
             .await
             .unwrap()
@@ -447,7 +448,7 @@ impl TestContext {
 
     /// Receive a message using the `receive_imf()` pipeline. Panics if it's not shown
     /// in the chat as exactly one message.
-    pub async fn recv_msg(&self, msg: &SentMessage) -> Message {
+    pub async fn recv_msg(&self, msg: &SentMessage<'_>) -> Message {
         let received = self
             .recv_msg_opt(msg)
             .await
@@ -476,7 +477,10 @@ impl TestContext {
 
     /// Receive a message using the `receive_imf()` pipeline. This is similar
     /// to `recv_msg()`, but doesn't assume that the message is shown in the chat.
-    pub async fn recv_msg_opt(&self, msg: &SentMessage) -> Option<crate::receive_imf::ReceivedMsg> {
+    pub async fn recv_msg_opt(
+        &self,
+        msg: &SentMessage<'_>,
+    ) -> Option<crate::receive_imf::ReceivedMsg> {
         receive_imf(self, msg.payload().as_bytes(), false)
             .await
             .unwrap()
@@ -583,7 +587,7 @@ impl TestContext {
     /// This is not hooked up to any SMTP-IMAP pipeline, so the other account must call
     /// [`TestContext::recv_msg`] with the returned [`SentMessage`] if it wants to receive
     /// the message.
-    pub async fn send_text(&self, chat_id: ChatId, txt: &str) -> SentMessage {
+    pub async fn send_text(&self, chat_id: ChatId, txt: &str) -> SentMessage<'_> {
         let mut msg = Message::new(Viewtype::Text);
         msg.set_text(Some(txt.to_string()));
         self.send_msg(chat_id, &mut msg).await
@@ -594,7 +598,7 @@ impl TestContext {
     /// This is not hooked up to any SMTP-IMAP pipeline, so the other account must call
     /// [`TestContext::recv_msg`] with the returned [`SentMessage`] if it wants to receive
     /// the message.
-    pub async fn send_msg(&self, chat_id: ChatId, msg: &mut Message) -> SentMessage {
+    pub async fn send_msg(&self, chat_id: ChatId, msg: &mut Message) -> SentMessage<'_> {
         chat::prepare_msg(self, chat_id, msg).await.unwrap();
         let msg_id = chat::send_msg(self, chat_id, msg).await.unwrap();
         let res = self.pop_sent_msg().await;
@@ -742,13 +746,14 @@ impl Drop for LogSink {
 /// This is a raw message, probably in the shape DC was planning to send it but not having
 /// passed through a SMTP-IMAP pipeline.
 #[derive(Debug, Clone)]
-pub struct SentMessage {
+pub struct SentMessage<'a> {
     pub payload: String,
     recipients: String,
     pub sender_msg_id: MsgId,
+    sender_context: &'a Context,
 }
 
-impl SentMessage {
+impl SentMessage<'_> {
     /// A recipient the message was destined for.
     ///
     /// If there are multiple recipients this is just a random one, so is not very useful.
@@ -765,6 +770,12 @@ impl SentMessage {
     pub fn payload(&self) -> &str {
         &self.payload
     }
+
+    pub async fn load_from_db(&self) -> Message {
+        Message::load_from_db(self.sender_context, self.sender_msg_id)
+            .await
+            .unwrap()
+    }
 }
 
 /// Load a pre-generated keypair for alice@example.org from disk.

src/webxdc.rs

@@ -1076,7 +1076,7 @@ mod tests {
         )
         .await?;
         let sent1 = alice.send_msg(chat.id, &mut alice_instance).await;
-        let alice_instance = Message::load_from_db(&alice, sent1.sender_msg_id).await?;
+        let alice_instance = sent1.load_from_db().await;
         alice
             .send_webxdc_status_update(
                 alice_instance.id,
@@ -1378,7 +1378,7 @@ mod tests {
         alice.flush_status_updates().await?;
         expect_status_update_event(&alice, alice_instance.id).await?;
         let sent2 = &alice.pop_sent_msg().await;
-        let alice_update = Message::load_from_db(&alice, sent2.sender_msg_id).await?;
+        let alice_update = sent2.load_from_db().await;
         assert!(alice_update.hidden);
         assert_eq!(alice_update.viewtype, Viewtype::Text);
         assert_eq!(alice_update.get_filename(), None);
@@ -2197,7 +2197,7 @@ sth_for_the = "future""#
             .await?;
         alice.flush_status_updates().await?;
         let sent2 = &alice.pop_sent_msg().await;
-        let update_msg = Message::load_from_db(&alice, sent2.sender_msg_id).await?;
+        let update_msg = sent2.load_from_db().await;
         assert!(alice_instance.get_showpadlock());
         assert!(update_msg.get_showpadlock());
 
@@ -2217,7 +2217,7 @@ sth_for_the = "future""#
             .await?;
         bob.flush_status_updates().await?;
         let sent3 = bob.pop_sent_msg().await;
-        let update_msg = Message::load_from_db(&bob, sent3.sender_msg_id).await?;
+        let update_msg = sent3.load_from_db().await;
         assert!(!update_msg.get_showpadlock());
 
         Ok(())