Add test_handle_authres_fails()

src/authres.rs

@@ -317,12 +317,21 @@ fn parse_authservid_candidates_config(config: &Option<String>) -> BTreeSet<&str>
 #[cfg(test)]
 mod tests {
     #![allow(clippy::indexing_slicing)]
+    use std::time::Duration;
+
     use tokio::fs;
     use tokio::io::AsyncReadExt;
 
     use super::*;
+
+    use crate::e2ee;
     use crate::mimeparser;
+    use crate::peerstate::Peerstate;
+    use crate::securejoin::get_securejoin_qr;
+    use crate::securejoin::join_securejoin;
+    use crate::test_utils;
     use crate::test_utils::TestContext;
+    use crate::test_utils::TestContextManager;
     use crate::tools;
 
     #[test]
@@ -583,4 +592,103 @@ Authentication-Results: box.hispanilandia.net; spf=pass smtp.mailfrom=adbenitez@
         let mail = mailparse::parse_mail(bytes).unwrap();
         handle_authres(&t, &mail, "invalidfrom.com").await.unwrap();
     }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_handle_authres_fails() -> Result<()> {
+        let mut tcm = TestContextManager::new();
+        let alice = tcm.alice().await;
+        let bob = tcm.bob().await;
+
+        // Bob sends Alice a message, so she gets his key
+        tcm.send_recv_accept(&bob, &alice, "Hi").await;
+
+        // We don't need bob anymore, let's make sure it's not accidentally used
+        drop(bob);
+
+        // Assume Alice receives an email from bob@example.net in the past with
+        // correct DKIM -> `dkim_works()` was called
+        dkim_works(&alice, "example.net").await?;
+        // And Alice knows her server's authserv-id
+        alice
+            .set_config(Config::AuthservIdCandidates, Some("example.org"))
+            .await?;
+
+        tcm.section("An attacker, bob2, sends a from-forged email to Alice!");
+
+        let bob2 = tcm.unconfigured().await;
+        bob2.configure_addr("bob@example.net").await;
+        e2ee::ensure_secret_key_exists(&bob2).await?;
+
+        let chat = bob2.create_chat(&alice).await;
+        let mut sent = bob2
+            .send_text(chat.id, "Please send me lots of money")
+            .await;
+
+        sent.payload
+            .insert_str(0, "Authentication-Results: example.org; dkim=fail");
+
+        let received = alice.recv_msg(&sent).await;
+
+        // Assert that the error tells the user about the problem
+        assert!(received.error.unwrap().contains("DKIM failed"));
+
+        let bob_state = Peerstate::from_addr(&alice, "bob@example.net")
+            .await?
+            .unwrap();
+
+        // Also check that the keypair was not changed
+        assert_eq!(
+            bob_state.public_key.unwrap(),
+            test_utils::bob_keypair().public
+        );
+
+        // Since Alice didn't change the key, Bob can't read her message
+        let received = tcm
+            .try_send_recv(&alice, &bob2, "My credit card number is 1234")
+            .await;
+        assert!(!received.text.as_ref().unwrap().contains("1234"));
+        assert!(received.error.is_some());
+
+        tcm.section("Turns out bob2 wasn't an attacker at all, Bob just has a new phone and DKIM just stopped working.");
+        tcm.section("To fix the key problems, Bob scans Alice's QR code.");
+
+        let qr = get_securejoin_qr(&alice.ctx, None).await.unwrap();
+        join_securejoin(&bob2.ctx, &qr).await.unwrap();
+
+        loop {
+            if let Some(mut sent) = bob2.pop_sent_msg_opt(Duration::ZERO).await {
+                sent.payload
+                    .insert_str(0, "Authentication-Results: example.org; dkim=fail");
+                alice.recv_msg(&sent).await;
+            } else if let Some(sent) = alice.pop_sent_msg_opt(Duration::ZERO).await {
+                bob2.recv_msg(&sent).await;
+            } else {
+                break;
+            }
+        }
+
+        // Unfortunately, securejoin currently doesn't work with authres-checking,
+        // so these checks would fail:
+
+        // let contact_bob = alice.add_or_lookup_contact(&bob2).await;
+        // assert_eq!(
+        //     contact_bob.is_verified(&alice.ctx).await.unwrap(),
+        //     VerifiedStatus::BidirectVerified
+        // );
+
+        // let contact_alice = bob2.add_or_lookup_contact(&alice).await;
+        // assert_eq!(
+        //     contact_alice.is_verified(&bob2.ctx).await.unwrap(),
+        //     VerifiedStatus::BidirectVerified
+        // );
+
+        // // Bob can read Alice's messages again
+        // let received = tcm
+        //     .try_send_recv(&alice, &bob2, "Can you read this again?")
+        //     .await;
+        // assert_eq!(received.text.as_ref().unwrap(), "Can you read this again?");
+        // assert!(received.error.is_none());
+
+        Ok(())
+    }
 }

src/chat.rs

@@ -3702,11 +3702,11 @@ mod tests {
 
         // create group and sync it to the second device
         let a1_chat_id = create_group_chat(&a1, ProtectionStatus::Unprotected, "foo").await?;
-        a1.send_text(a1_chat_id, "ho!").await;
+        let sent = a1.send_text(a1_chat_id, "ho!").await;
         let a1_msg = a1.get_last_msg().await;
         let a1_chat = Chat::load_from_db(&a1, a1_chat_id).await?;
 
-        let a2_msg = a2.recv_msg(&a1.pop_sent_msg().await).await;
+        let a2_msg = a2.recv_msg(&sent).await;
         let a2_chat_id = a2_msg.chat_id;
         let a2_chat = Chat::load_from_db(&a2, a2_chat_id).await?;
 

src/mimeparser.rs

@@ -370,16 +370,16 @@ impl MimeMessage {
         parser.heuristically_parse_ndn(context).await;
         parser.parse_headers(context).await?;
 
+        if !decryption_info.dkim_results.allow_keychange {
+            for part in parser.parts.iter_mut() {
+                part.error = Some("Seems like DKIM failed, this either is an attack or (more likely) a bug in Authentication-Results-checking. Please tell us about this at https://support.delta.chat.\n\nScan the sender's QR code to make this work again.".to_string());
+            }
+        }
         if warn_empty_signature && parser.signatures.is_empty() {
             for part in parser.parts.iter_mut() {
                 part.error = Some("No valid signature".to_string());
             }
         }
-        if !decryption_info.dkim_results.allow_keychange {
-            for part in parser.parts.iter_mut() {
-                part.error = Some("No keychange allowed, this either is an attack or (more likely) a bug in authres-checking".to_string());
-            }
-        }
 
         if parser.is_mime_modified {
             parser.decoded_data = mail_raw;

src/test_utils.rs

@@ -74,6 +74,13 @@ impl TestContextManager {
             .await
     }
 
+    pub async fn unconfigured(&mut self) -> TestContext {
+        TestContext::builder()
+            .with_log_sink(self.log_tx.clone())
+            .build()
+            .await
+    }
+
     /// Writes info events to the log that mark a section, e.g.:
     ///
     /// ========== `msg` goes here ==========
@@ -89,19 +96,24 @@ impl TestContextManager {
     /// - Let the other TestContext receive it and accept the chat
     /// - Assert that the message arrived
     pub async fn send_recv_accept(&self, from: &TestContext, to: &TestContext, msg: &str) {
+        let received_msg = self.try_send_recv(from, to, msg).await;
+        assert_eq!(received_msg.text.as_ref().unwrap(), msg);
+        received_msg.chat_id.accept(to).await.unwrap();
+    }
+
+    /// - Let one TestContext send a message
+    /// - Let the other TestContext receive it
+    pub async fn try_send_recv(&self, from: &TestContext, to: &TestContext, msg: &str) -> Message {
         self.section(&format!(
             "{} sends a message '{}' to {}",
             from.name(),
             msg,
             to.name()
         ));
-
         let chat = from.create_chat(to).await;
         let sent = from.send_text(chat.id, msg).await;
-
         let received_msg = to.recv_msg(&sent).await;
-        received_msg.chat_id.accept(to).await.unwrap();
-        assert_eq!(received_msg.text.unwrap(), msg);
+        received_msg
     }
 
     pub async fn change_addr(&self, test_context: &TestContext, new_addr: &str) {
@@ -369,6 +381,12 @@ impl TestContext {
     ///
     /// Panics if there is no message or on any error.
     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> {
         let start = Instant::now();
         let (rowid, msg_id, payload, recipients) = loop {
             let row = self
@@ -393,25 +411,25 @@ impl TestContext {
             if let Some(row) = row {
                 break row;
             }
-            if start.elapsed() < Duration::from_secs(3) {
+            if start.elapsed() < timeout {
                 tokio::time::sleep(Duration::from_millis(100)).await;
             } else {
-                panic!("no sent message found in jobs table");
+                return None;
             }
         };
         self.ctx
             .sql
-            .execute("DELETE FROM jobs WHERE id=?;", paramsv![rowid])
+            .execute("DELETE FROM smtp WHERE id=?;", paramsv![rowid])
             .await
             .expect("failed to remove job");
         update_msg_state(&self.ctx, msg_id, MessageState::OutDelivered)
             .await
             .expect("failed to update message state");
-        SentMessage {
+        Some(SentMessage {
             payload,
             sender_msg_id: msg_id,
             recipients,
-        }
+        })
     }
 
     /// Parses a message.
@@ -725,7 +743,7 @@ impl Drop for LogSink {
 /// passed through a SMTP-IMAP pipeline.
 #[derive(Debug, Clone)]
 pub struct SentMessage {
-    payload: String,
+    pub payload: String,
     recipients: String,
     pub sender_msg_id: MsgId,
 }