diff --git a/src/mimeparser.rs b/src/mimeparser.rs index dc8978235..dbb5cbdec 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -259,9 +259,9 @@ impl MimeMessage { } } - // remove headers that are allowed _only_ in the encrypted part - headers.remove("secure-join-fingerprint"); - headers.remove("chat-verified"); + // Remove headers that are allowed _only_ in the encrypted+signed part. It's ok to leave + // them in signed-only emails, but has no value currently. + 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 - // 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 + // 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 . + 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; - - // We do not want to allow unencrypted subject in encrypted emails because the - // user might falsely think that the subject is safe. - // See . - headers.remove("subject"); + let mut inner_from = None; 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) { + headers.remove("secure-join-fingerprint"); + headers.remove("chat-verified"); + } + fn merge_headers( context: &Context, headers: &mut HashMap, diff --git a/src/peerstate.rs b/src/peerstate.rs index 64e6b5689..8bd0c37e8 100644 --- a/src/peerstate.rs +++ b/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 - // ignored anyway and the message shown as not encrypted, so we don't - // have to handle this case. + // Although checking `from_is_signed` below is sufficient, let's play it safe. && !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 diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 2120008f8..eb466eeb3 100644 --- a/src/receive_imf.rs +++ b/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, }) } diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index 802f3fde8..2c03c1ffb 100644 --- a/src/receive_imf/tests.rs +++ b/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; diff --git a/test-data/message/thunderbird_encrypted_unsigned_with_unencrypted_subject.eml b/test-data/message/thunderbird_encrypted_unsigned_with_unencrypted_subject.eml new file mode 100644 index 000000000..2375e7ade --- /dev/null +++ b/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 +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--