feat: Don't affect MimeMessage with "From" and secured headers from encrypted unsigned messages

If a message is encrypted, but unsigned:
- Don't set `MimeMessage::from_is_signed`.
- Remove "secure-join-fingerprint" and "chat-verified" headers from `MimeMessage`.
- Minor: Preserve "Subject" from the unencrypted top level if there's no "Subject" in the encrypted
  part, this message is displayed w/o a padlock anyway.

Apparently it didn't lead to any vulnerabilities because there are checks for
`MimeMessage::signatures.is_empty()` in all necessary places, but still the code looked dangerous,
especially because `from_is_singed` var name didn't correspond to its actual value (it was rather
`from_is_encrypted_maybe_signed`).

src/mimeparser.rs

@@ -259,9 +259,9 @@ impl MimeMessage {
             }
         }
 
-        // remove headers that are allowed _only_ in the encrypted part
+        // Remove headers that are allowed _only_ in the encrypted+signed part. It's ok to leave
-        headers.remove("secure-join-fingerprint");
+        // them in signed-only emails, but has no value currently.
-        headers.remove("chat-verified");
+        Self::remove_secured_headers(&mut headers);
 
         let from = from.context("No from in message")?;
         let private_keyring = load_self_secret_keyring(context).await?;
@@ -305,10 +305,11 @@ impl MimeMessage {
             content
         });
         if let (Ok(mail), true) = (mail, encrypted) {
-            // Handle any gossip headers if the mail was encrypted.  See section
-            // "3.6 Key Gossip" of <https://autocrypt.org/autocrypt-spec-1.1.0.pdf>
-            // but only if the mail was correctly signed:
             if !signatures.is_empty() {
+                // Handle any gossip headers if the mail was encrypted. See section
+                // "3.6 Key Gossip" of <https://autocrypt.org/autocrypt-spec-1.1.0.pdf>
+                // but only if the mail was correctly signed. Probably it's ok to not require
+                // encryption here, but let's follow the standard.
                 let gossip_headers = mail.headers.get_all_values("Autocrypt-Gossip");
                 gossiped_addr = update_gossip_peerstates(
                     context,
@@ -318,6 +319,9 @@ impl MimeMessage {
                     gossip_headers,
                 )
                 .await?;
+                // Remove unsigned subject from messages displayed with padlock.
+                // See <https://github.com/deltachat/deltachat-core-rust/issues/1790>.
+                headers.remove("subject");
             }
 
             // let known protected headers from the decrypted
@@ -325,24 +329,20 @@ impl MimeMessage {
 
             // Signature was checked for original From, so we
             // do not allow overriding it.
-            let mut signed_from = None;
+            let mut inner_from = None;
-
-            // We do not want to allow unencrypted subject in encrypted emails because the
-            // user might falsely think that the subject is safe.
-            // See <https://github.com/deltachat/deltachat-core-rust/issues/1790>.
-            headers.remove("subject");
 
             MimeMessage::merge_headers(
                 context,
                 &mut headers,
                 &mut recipients,
-                &mut signed_from,
+                &mut inner_from,
                 &mut list_post,
                 &mut chat_disposition_notification_to,
                 &mail.headers,
             );
-            if let Some(signed_from) = signed_from {
+
-                if addr_cmp(&signed_from.addr, &from.addr) {
+            if let (Some(inner_from), true) = (inner_from, !signatures.is_empty()) {
+                if addr_cmp(&inner_from.addr, &from.addr) {
                     from_is_signed = true;
                 } else {
                     // There is a From: header in the encrypted &
@@ -360,6 +360,8 @@ impl MimeMessage {
             }
         }
         if signatures.is_empty() {
+            Self::remove_secured_headers(&mut headers);
+
             // If it is not a read receipt, degrade encryption.
             if let (Some(peerstate), Ok(mail)) = (&mut decryption_info.peerstate, mail) {
                 if message_time > peerstate.last_seen_autocrypt
@@ -1377,6 +1379,11 @@ impl MimeMessage {
             .and_then(|msgid| parse_message_id(msgid).ok())
     }
 
+    fn remove_secured_headers(headers: &mut HashMap<String, String>) {
+        headers.remove("secure-join-fingerprint");
+        headers.remove("chat-verified");
+    }
+
     fn merge_headers(
         context: &Context,
         headers: &mut HashMap<String, String>,

src/peerstate.rs

@@ -705,9 +705,7 @@ pub(crate) async fn maybe_do_aeap_transition(
                 // addresses with an MUA.
                 && mime_parser.has_chat_version()
                 // Check if the message is signed correctly.
-                // If it's not signed correctly, the whole autocrypt header will be mostly
+                // Although checking `from_is_signed` below is sufficient, let's play it safe.
-                // ignored anyway and the message shown as not encrypted, so we don't
-                // have to handle this case.
                 && !mime_parser.signatures.is_empty()
                 // Check if the From: address was also in the signed part of the email.
                 // Without this check, an attacker could replay a message from Alice 

src/receive_imf.rs

@@ -63,6 +63,11 @@ pub struct ReceivedMsg {
 
     /// Whether IMAP messages should be immediately deleted.
     pub needs_delete_job: bool,
+
+    /// Whether the From address was repeated in the signed part
+    /// (and we know that the signer intended to send from this address).
+    #[cfg(test)]
+    pub(crate) from_is_signed: bool,
 }
 
 /// Emulates reception of a message from the network.
@@ -161,6 +166,8 @@ pub(crate) async fn receive_imf_inner(
                 sort_timestamp: 0,
                 msg_ids,
                 needs_delete_job: false,
+                #[cfg(test)]
+                from_is_signed: false,
             }));
         }
         Ok(mime_parser) => mime_parser,
@@ -1393,6 +1400,8 @@ RETURNING id
         sort_timestamp,
         msg_ids: created_db_entries,
         needs_delete_job,
+        #[cfg(test)]
+        from_is_signed: mime_parser.from_is_signed,
     })
 }
 

src/receive_imf/tests.rs

@@ -3110,7 +3110,8 @@ async fn test_thunderbird_autocrypt() -> Result<()> {
     let t = TestContext::new_bob().await;
 
     let raw = include_bytes!("../../test-data/message/thunderbird_with_autocrypt.eml");
-    receive_imf(&t, raw, false).await?;
+    let received_msg = receive_imf(&t, raw, false).await?.unwrap();
+    assert!(received_msg.from_is_signed);
 
     let peerstate = Peerstate::from_addr(&t, "alice@example.org")
         .await?
@@ -3191,7 +3192,8 @@ async fn test_thunderbird_unsigned() -> Result<()> {
 
     // Alice receives an unsigned message from Bob.
     let raw = include_bytes!("../../test-data/message/thunderbird_encrypted_unsigned.eml");
-    receive_imf(&alice, raw, false).await?;
+    let received_msg = receive_imf(&alice, raw, false).await?.unwrap();
+    assert!(!received_msg.from_is_signed);
 
     let msg = alice.get_last_msg().await;
     assert!(!msg.get_showpadlock());
@@ -3200,6 +3202,27 @@ async fn test_thunderbird_unsigned() -> Result<()> {
     Ok(())
 }
 
+/// Bob receives an encrypted unsigned message with only an unencrypted Subject.
+///
+/// Test that the message is displayed without any errors,
+/// but also without a padlock, but with the Subject.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_thunderbird_unsigned_with_unencrypted_subject() -> Result<()> {
+    let bob = TestContext::new_bob().await;
+
+    let raw = include_bytes!(
+        "../../test-data/message/thunderbird_encrypted_unsigned_with_unencrypted_subject.eml"
+    );
+    receive_imf(&bob, raw, false).await?;
+
+    let msg = bob.get_last_msg().await;
+    assert!(!msg.get_showpadlock());
+    assert!(msg.error().is_none());
+    assert_eq!(msg.get_subject(), "Hello!");
+
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_mua_user_adds_member() -> Result<()> {
     let t = TestContext::new_alice().await;

test-data/message/thunderbird_encrypted_unsigned_with_unencrypted_subject.eml

@@ -0,0 +1,91 @@
+From - Sun, 19 Nov 2023 01:08:24 GMT
+X-Mozilla-Status: 0800
+X-Mozilla-Status2: 00000000
+Message-ID: <38a2a29b-8261-403b-abb5-56b0a87d2ff4@example.org>
+Date: Sat, 18 Nov 2023 22:08:23 -0300
+MIME-Version: 1.0
+User-Agent: Mozilla Thunderbird
+Content-Language: en-US
+To: bob@example.net
+From: Alice <alice@example.org>
+Autocrypt: addr=alice@example.org; keydata=
+ xsDNBGOaGzQBDADCFtBNMHRDJQRkd2tNm7CJm1Yo3Y5r3qP6v0FSwP1BIHbiIf0E/jFiKZWj
+ 1uL68J2mGUuUu+Qi4ovf1l9/QQYzg/DCaLZxlbc0LKu2LXcpUL5DPu37mdw+DKs0YvNIlc+A
+ RjyFUwd3rsZN3k58inf1mYzKuKU6NpbdXULbOEYwnVEwzQsrtS2JgJ+tLSYUvNJeMJXm/cDL
+ XKJSApAyvVVdxxteG8uWcDqWV/HcXuopXLILf3yJF0De11/7G62dHNHuhmtgRLsTN4Q372Q9
+ KNdYEFLHaN91jEzyD/+aHNskATxtcGhppI8OQsU3NzNgHyd8Smzx5oTyZ/6NdhYoh0pKB8yf
+ VAyA69t5fctQRb4+bTwL+sS9KDobQOvcyOMUSccDfUhsWMghwsMCwU4Sz9hIY6dCAfroDAiL
+ vYUfdNJstAqvLf04mZtMmkI7Q2BYLETEgu4KQzQHRQekmOE/3EaSiojNa4ZTVURMdJ9U+I3E
+ q8e6TbOY7Xa4V8krAt/F2wMAEQEAAc0ZQWxpY2UgPGFsaWNlQGV4YW1wbGUub3JnPsLBDQQT
+ AQgANxYhBBSrP2X8J0u9tfp2jCXwByRZ5HriBQJjmhs1BQm7+B4AAhsDBAsJCAcFFQgJCgsF
+ FgIDAQAACgkQJfAHJFnkeuKQ2wwAgDgiCI6bz9PjqE1GoDcy/xQdy+nnYq5pOuHGUndZ7jYK
+ cOqM8LDEaG7GgrFsbs9vGhTA1fyqncM41pB7SmwQ7zBVaMdtHoulEG4RPGVboDaY9tuMOL3/
+ GVxFbovVHyU5Lr1euryNh/0JvMITY0UHaEY1k1M7izYUMyFu8I1ODZ9Iws2trUyU3Omw/sTJ
+ x15zzCsK8Aq+r3JmB+Q33SFSgWr/YWH0dQVIQ0I5iLN2q14oucmLBaKc9EXdRLiu8S8lLSQl
+ nfISJ17GBLmH1YxmPPZ3CRHC6iEKCLR6G9wzhsTPNdK7dRCYR5wTI27RVPLBcSnCKAeTopAJ
+ YskyNndtv0iaNRT7YLOfhrsBAofSjuLegR04CNiqBHtYQ3LO3WKhJ7riRcQ/Ksv0wYkmj1gJ
+ 8myMwA+ybfYrpNqO4devnCvE3Eo5gzeYbvYU2Z17n9y6HAOG9/Tm/daiGEP2ni6iwV0kqLzw
+ eC48R1D75T66PxX/jQooujrTph8+K3ckV/q+zsDNBGOaGzUBDADV+DGgKxvCpfVFuPGrSdRU
+ 06dxowdKOKavO6WGMvN3g/+CFrIsjUFy4S0Soo5ARnLh23i49ZSjacXFpgtZUNV3iGOSOcSE
+ LldLtZk5BV9w/ATqqgu4/LVdNA9rm+o197bIeSQCRTnY/QV6FdKYxVd4NBVH9abZ7t8Tm4qC
+ urZj56MjPCg3fqT+Q6sjxH+nKBrs8s8iCJkYhGBgU3q5W+wrtZ56kI9mxJec62KHpyLZ0rTE
+ xEAeVbChUJOo11vUtJfTrDhI6lhqyr72o/A6bY1OV7WzkxtiBRl35eewQ+RDLJ4yxaNj/XTS
+ UxOz60xNggEfDVtfgfjBZrBbiHXqf8iKVV1ZPGm0ycvXZGYFw2zXLI2PwevhQCm+t4Ywty1h
+ 8l019MYmGadpQgbuA4ZippuzOSzSGMQ+S4uYEzeeymR9ksxVSXn90HEzqC7LdHCcd2IO6rfu
+ g2fuRf258Adfuoh3s8YUlWyXjEHLXKo9SRgGMfGs7qgCOL/ReAwFPtKACvEAEQEAAcLA/AQY
+ AQgAJhYhBBSrP2X8J0u9tfp2jCXwByRZ5HriBQJjmhs2BQm7+B4AAhsMAAoJECXwByRZ5Hri
+ EOkMALtq4DVYX8RfoPdU0Dt6y+yDj1NALv5GefvHbgfuaVT8PaOP0gxCjWrnUDvvJEwP1W3j
+ UXYqDwKP42hiGWsnXk2hbgXbplArgP3H987x7c8bu1wIAmkJ9eLjEM++rbOD4vWbYXRwaDiH
+ LetFJ5tGHDAIfL48NYpz2o3XZ3/O7WdTZphsAcvgPxTC+zU7WkbUl2SQlj0/qwsoD+qe9RYT
+ XhVXR7q7sjcGB4TpeqzRT7YKVLoVNq+bQw2lUX4W561gAYbZvVo/XByfDCoxmkxwuMlSmajj
+ Wy7b9TuT38t1HArv4m/LyVuBHiikX0/MUNBeSSIiKDvTL6NdHTjnZM6ptZvdvW3+ou6ET0pK
+ MGDpk/1NVuMnIHJESRg/SSFV6sElgq38k9wAT2oUqLcYvYI07nHmnuciaGygkCcGt+l2PvAa
+ j4mkQQvMU0cNRDBybk5aKi820oGIJjT7e+5RnD2mYZQdOAbQhDVCHvrfS1I60bsHT1MHqyAa
+ /qMLjKwBpKEd/w==
+X-Mozilla-Draft-Info: internal/draft; vcard=0; receipt=0; DSN=0; uuencode=0;
+ attachmentreminder=0; deliveryformat=0
+X-Identity-Key: id3
+Fcc: imap://alice%40example.org@in.example.org/Sent
+Subject: Hello!
+Content-Type: multipart/encrypted;
+ protocol="application/pgp-encrypted";
+ boundary="------------HVIlnYsnz3YPWPVlor40isrE"
+
+This is an OpenPGP/MIME encrypted message (RFC 4880 and 3156)
+--------------HVIlnYsnz3YPWPVlor40isrE
+Content-Type: application/pgp-encrypted
+Content-Description: PGP/MIME version identification
+
+Version: 1
+
+--------------HVIlnYsnz3YPWPVlor40isrE
+Content-Type: application/octet-stream; name="encrypted.asc"
+Content-Description: OpenPGP encrypted message
+Content-Disposition: inline; filename="encrypted.asc"
+
+-----BEGIN PGP MESSAGE-----
+
+wcDMA/K57StIWPW6AQv/cTRuyZRuX+Qkmxcbo34Df32PL2O3LSuwChdd4pd+WRNU5r08E0eLeFGv
+k/C/2iFrNhgmvsJEnDtTZVc7+qykMeXwwC1f2qT7OAvTbAF9Me1dBCQy8QAPSzOowNu8434qOPwz
+fZkoB4pdkKlAhGMUBdtbWYg3EDdpBUCpAuT8l3qhlfdSTRh+da2UVh9kix3SXSmLarMZGlJikh3T
+pRLAlPOtRa7k5R4rc/OCD9HUL9EHGUk4rV86+mr4Wpp1aIhdTNksfrFXvgRFx0dcU33uS74X/zme
+c1AOuJK8ed3F2Zc5IYTyE+ps1jBqVH1+CM697d5FnIMx96VyB4txDBBBjyq+oh+SY+xUxvtR/EzN
+mR0qfiSCwOKwGdzPWDkBoCePYnykiR4shVdVV3qjYZwBmY5IWNahyBtIB470V4wMiJXdZFMQu5XD
+NO4DJ6/x2UCcUiUokAwvKTSHflAHocZJ2ICfzhlJjJSiqEttwyG0xU0ZhTdZIbwzRWzYzDgMakFO
+wcBMA+PY3JvEjuMiAQf+MXJB44iQ/Ti9WDH3MOX0N+X0APf5mzeqU9MpSZ+7mAnTRVmz/iMD989J
+ngxu1mv3vQBjdNokIAfBYk29qyRBULXaof+6x5VJSWoopV6t1vNd+DB2HgLkbdCJuzikapCE+QAm
+gmgoknQap+3l4D1RkMys7w1awsMYK0wR1Iucjb8M2I4f5ObPSMS5211ZBJuYOf1OQt0jX1jCNTOi
+Q5tbufJ6EjAvP6XOYTY3in8+p7yocBgQhXaK8NB8jdg+h6IE/NNX1W5v1tMx17WRtIQGwh4cOlB6
+Fsu87eMx8x9Ew2YdpN6yvIHddy9M9k3NCROT+5rIq7+1GTy0WoI/KxNwtdLBEwE2h7VFmJ5qEWhi
+lmunWBIlA71IdNqpi+9lbg/QPwCgvRow05Gv0FyKbSvDA/fN18+CLuz8RicNFRbiPgwxMLRE3lZB
+jj7BEDa53fjnjBJG4lA9mUaB/ALScQwkGXqKPpeDN4Iexy8eBsZoBczJcPna7GNgSYiXxbWo2P8+
+T71oSht+igsyi8gYDNwmhcsQxSWtF7f53irOKWgEpgz2hMjWi764GGBzAJ231suyV2eZXVJm2Im7
+RUDu3bJNu/Q/CvxQQdcUXHEoHrTzDZ3KQPW0/oYln2WZwkRQcVVGSLVINgVTdTkI8GEuXH/4yjYF
+tUq68esMQ2WOWZ7IiTNPU9T2F5kEwR2sf55XgLckj0OLJn5CkSAWBJhHdL/9er06u8ksTs6V9UEo
+i+15XUAiNzsuIcydoGF9sSuO9nm62xYO8uY8yQl/z2Q7CXKefkXBSeaNcYyQKrCnLfwnuvR2WFE5
+OJ0HM5i/qcWmqWSvSYzdDqUbI95q9Fgn78F/bcJ9ZTdPDkWLi//SUzcWeUWgRC57U4CFTApj74EJ
+lT/gVlH/y88RNcvGGAPHHS11adLmln7CbLEF9tmRM3ou78nMm7VhXT8C/QZiTvJgG3yZbeRFlC39
+=hdiO
+-----END PGP MESSAGE-----
+
+--------------HVIlnYsnz3YPWPVlor40isrE--