From 7def6e70ba612b1b8c5cf666fcdd26d7cca4438e Mon Sep 17 00:00:00 2001 From: link2xt Date: Sat, 28 May 2022 21:05:56 +0000 Subject: [PATCH] mimeparser: explicitly handle decryption errors mimeparser now handles try_decrypt() errors instead of simply logging them. If try_decrypt() returns an error, a single message bubble with an error is added to the chat. The case when encrypted part is found in a non-standard MIME structure is not treated as an encryption failure anymore. Instead, encrypted MIME part is presented as a file to the user, so they can download the part and decrypt it manually. Because try_decrypt() errors are handled by mimeparser now, try_decrypt() was fixed to avoid trying to load private_keyring if the message is not encrypted. In tests the context receiving message usually does not have self address configured, so loading private keyring via Keyring::new_self() fails together with the try_decrypt(). --- CHANGELOG.md | 1 + src/e2ee.rs | 21 +++++++----------- src/mimeparser.rs | 55 +++++++++++++++++++---------------------------- 3 files changed, 31 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd55a6214..d882c5486 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ via Autocrypt Setup Message #3352 - Keep pgp key when you change your own email address #3351 - Do not ignore Sent and Spam folders on Gmail #3369 +- handle decryption errors explicitly and don't get confused by encrypted mail attachments #3374 ## 1.83.0 diff --git a/src/e2ee.rs b/src/e2ee.rs index 26fc5cf15..8d19b44b0 100644 --- a/src/e2ee.rs +++ b/src/e2ee.rs @@ -171,7 +171,6 @@ pub async fn try_decrypt( } // Possibly perform decryption - let private_keyring: Keyring = Keyring::new_self(context).await?; let mut public_keyring_for_validate: Keyring = Keyring::new(); if let Some(ref mut peerstate) = peerstate { @@ -185,17 +184,11 @@ pub async fn try_decrypt( } } - let (out_mail, signatures) = match decrypt_if_autocrypt_message( - context, - mail, - private_keyring, - public_keyring_for_validate, - ) - .await? - { - Some((out_mail, signatures)) => (Some(out_mail), signatures), - None => (None, Default::default()), - }; + let (out_mail, signatures) = + match decrypt_if_autocrypt_message(context, mail, public_keyring_for_validate).await? { + Some((out_mail, signatures)) => (Some(out_mail), signatures), + None => (None, Default::default()), + }; if let Some(mut peerstate) = peerstate { // If message is not encrypted and it is not a read receipt, degrade encryption. @@ -293,7 +286,6 @@ fn get_attachment_mime<'a, 'b>(mail: &'a ParsedMail<'b>) -> Option<&'a ParsedMai async fn decrypt_if_autocrypt_message( context: &Context, mail: &ParsedMail<'_>, - private_keyring: Keyring, public_keyring_for_validate: Keyring, ) -> Result, HashSet)>> { let encrypted_data_part = match get_autocrypt_mime(mail) @@ -307,6 +299,9 @@ async fn decrypt_if_autocrypt_message( Some(res) => res, }; info!(context, "Detected Autocrypt-mime message"); + let private_keyring: Keyring = Keyring::new_self(context) + .await + .context("failed to get own keyring")?; decrypt_part( encrypted_data_part, diff --git a/src/mimeparser.rs b/src/mimeparser.rs index f757fb47f..55b25d4a2 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -265,22 +265,15 @@ impl MimeMessage { &decrypted_mail.headers, ); - (decrypted_mail, signatures, true) + (Ok(decrypted_mail), signatures, true) } else { // Message was not encrypted - (mail, signatures, false) + (Ok(mail), signatures, false) } } Err(err) => { - // continue with the current, still encrypted, mime tree. - // unencrypted parts will be replaced by an error message - // that is added as "the message" to the chat then. - // - // if we just return here, the header is missing - // and the caller cannot display the message - // and try to assign the message to a chat warn!(context, "decryption failed: {}", err); - (mail, Default::default(), true) + (Err(err), Default::default(), true) } }; @@ -291,7 +284,7 @@ impl MimeMessage { list_post, from, chat_disposition_notification_to, - decrypting_failed: false, + decrypting_failed: mail.is_err(), // only non-empty if it was a valid autocrypt message signatures, @@ -318,9 +311,24 @@ impl MimeMessage { .create_stub_from_partial_download(context, org_bytes) .await?; } - None => { - parser.parse_mime_recursive(context, &mail, false).await?; - } + None => match mail { + Ok(mail) => { + parser.parse_mime_recursive(context, &mail, false).await?; + } + Err(err) => { + let msg_body = stock_str::cant_decrypt_msg_body(context).await; + let txt = format!("[{}]", msg_body); + + let part = Part { + typ: Viewtype::Text, + msg_raw: Some(txt.clone()), + msg: txt, + error: Some(format!("Decrypting failed: {}", err)), + ..Default::default() + }; + parser.parts.push(part); + } + }, }; parser.maybe_remove_bad_parts(); @@ -779,25 +787,6 @@ impl MimeMessage { self.is_mime_modified = true; } } - (mime::MULTIPART, "encrypted") => { - // we currently do not try to decrypt non-autocrypt messages - // at all. If we see an encrypted part, we set - // decrypting_failed. - let msg_body = stock_str::cant_decrypt_msg_body(context).await; - let txt = format!("[{}]", msg_body); - - let part = Part { - typ: Viewtype::Text, - msg_raw: Some(txt.clone()), - msg: txt, - error: Some("Decryption failed".to_string()), - ..Default::default() - }; - self.parts.push(part); - - any_part_added = true; - self.decrypting_failed = true; - } (mime::MULTIPART, "signed") => { /* RFC 1847: "The multipart/signed content type contains exactly two body parts. The first body