diff --git a/CHANGELOG.md b/CHANGELOG.md index e3b291385..f972a5717 100644 --- a/CHANGELOG.md +++ b/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 #3669 - Add more logging to `fetch_many_msgs` and refactor it #3811 - Small speedup #3780 diff --git a/src/authres.rs b/src/authres.rs index f9f9d11fb..d2b67f46a 100644 --- a/src/authres.rs +++ b/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: \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: \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: \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(()) + } } diff --git a/src/contact.rs b/src/contact.rs index 676380df8..ddaec2306 100644 --- a/src/contact.rs +++ b/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. diff --git a/src/decrypt.rs b/src/decrypt.rs index 8c57cdd36..b3413bb65 100644 --- a/src/decrypt.rs +++ b/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 { + 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. diff --git a/src/download.rs b/src/download.rs index 99b6577b2..f9fc9711b 100644 --- a/src/download.rs +++ b/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) - .await? - .rfc724_mid; + let sent2_rfc724_mid = sent2.load_from_db().await.rfc724_mid; // not downloading the status update results in an placeholder receive_imf_inner( diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 4a1f9a6e1..d9e6bcedb 100644 --- a/src/mimefactory.rs +++ b/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 { 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) diff --git a/src/test_utils.rs b/src/test_utils.rs index 472087563..43318edfc 100644 --- a/src/test_utils.rs +++ b/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 { + pub async fn pop_sent_msg_opt(&self, timeout: Duration) -> Option> { 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 { + pub async fn recv_msg_opt( + &self, + msg: &SentMessage<'_>, + ) -> Option { 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. diff --git a/src/webxdc.rs b/src/webxdc.rs index 9ee1ef9a1..a4f185900 100644 --- a/src/webxdc.rs +++ b/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(())