Merge detached signature parsing

src/dc_receive_imf.rs

@@ -4845,4 +4845,31 @@ Message with references."#;
 
         Ok(())
     }
+
+    /// Test a message with RFC 1847 encapsulation as created by Thunderbird.
+    #[async_std::test]
+    async fn test_rfc1847_encapsulation() -> Result<()> {
+        let alice = TestContext::new_alice().await;
+        let bob = TestContext::new_bob().await;
+        alice.configure_addr("alice@example.org").await;
+
+        // Alice sends an Autocrypt message to Bob so Bob gets Alice's key.
+        let chat_alice = alice.create_chat(&bob).await;
+        let first_msg = alice
+            .send_text(chat_alice.id, "Sending Alice key to Bob.")
+            .await;
+        bob.recv_msg(&first_msg).await;
+        message::delete_msgs(&bob, &[bob.get_last_msg().await.id]).await?;
+
+        bob.set_config(Config::ShowEmails, Some("2")).await?;
+
+        // Alice sends a message to Bob using Thunderbird.
+        let raw = include_bytes!("../test-data/message/rfc1847_encapsulation.eml");
+        dc_receive_imf(&bob, raw, "INBOX", 2, false).await?;
+
+        let msg = bob.get_last_msg().await;
+        assert!(msg.get_showpadlock());
+
+        Ok(())
+    }
 }

src/e2ee.rs

@@ -2,7 +2,7 @@
 
 use std::collections::HashSet;
 
-use anyhow::{bail, ensure, format_err, Result};
+use anyhow::{bail, format_err, Result};
 use mailparse::ParsedMail;
 use num_traits::FromPrimitive;
 
@@ -179,7 +179,6 @@ pub async fn try_decrypt(
     // Possibly perform decryption
     let private_keyring: Keyring<SignedSecretKey> = Keyring::new_self(context).await?;
     let mut public_keyring_for_validate: Keyring<SignedPublicKey> = Keyring::new();
-    let mut signatures = HashSet::default();
 
     if let Some(ref mut peerstate) = peerstate {
         peerstate
@@ -192,14 +191,17 @@ pub async fn try_decrypt(
         }
     }
 
-    let out_mail = decrypt_if_autocrypt_message(
+    let (out_mail, signatures) = match decrypt_if_autocrypt_message(
         context,
         mail,
         private_keyring,
         public_keyring_for_validate,
-        &mut signatures,
     )
-    .await?;
+    .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.
@@ -275,8 +277,7 @@ async fn decrypt_if_autocrypt_message(
     mail: &ParsedMail<'_>,
     private_keyring: Keyring<SignedSecretKey>,
     public_keyring_for_validate: Keyring<SignedPublicKey>,
-    ret_valid_signatures: &mut HashSet<Fingerprint>,
+) -> Result<Option<(Vec<u8>, HashSet<Fingerprint>)>> {
-) -> Result<Option<Vec<u8>>> {
     let encrypted_data_part = match get_autocrypt_mime(mail).or_else(|| get_mixed_up_mime(mail)) {
         None => {
             // not an autocrypt mime message, abort and ignore
@@ -290,36 +291,60 @@ async fn decrypt_if_autocrypt_message(
         encrypted_data_part,
         private_keyring,
         public_keyring_for_validate,
-        ret_valid_signatures,
     )
     .await
 }
 
+/// Validates signatures of Multipart/Signed message part, as defined in RFC 1847.
+///
+/// Returns `None` if the part is not a Multipart/Signed part, otherwise retruns the set of key
+/// fingerprints for which there is a valid signature.
+async fn validate_detached_signature(
+    mail: &ParsedMail<'_>,
+    public_keyring_for_validate: &Keyring<SignedPublicKey>,
+) -> Result<Option<(Vec<u8>, HashSet<Fingerprint>)>> {
+    if mail.ctype.mimetype != "multipart/signed" {
+        return Ok(None);
+    }
+
+    if let [first_part, second_part] = &mail.subparts[..] {
+        // First part is the content, second part is the signature.
+        let content = first_part.raw_bytes;
+        let signature = second_part.get_body_raw()?;
+        let ret_valid_signatures =
+            pgp::pk_validate(content, &signature, public_keyring_for_validate).await?;
+
+        Ok(Some((content.to_vec(), ret_valid_signatures)))
+    } else {
+        Ok(None)
+    }
+}
+
 /// Returns Ok(None) if nothing encrypted was found.
 async fn decrypt_part(
     mail: &ParsedMail<'_>,
     private_keyring: Keyring<SignedSecretKey>,
     public_keyring_for_validate: Keyring<SignedPublicKey>,
-    ret_valid_signatures: &mut HashSet<Fingerprint>,
+) -> Result<Option<(Vec<u8>, HashSet<Fingerprint>)>> {
-) -> Result<Option<Vec<u8>>> {
     let data = mail.get_body_raw()?;
 
     if has_decrypted_pgp_armor(&data) {
-        // we should only have one decryption happening
+        let (plain, ret_valid_signatures) =
-        ensure!(ret_valid_signatures.is_empty(), "corrupt signatures");
+            pgp::pk_decrypt(data, private_keyring, &public_keyring_for_validate).await?;
 
-        let plain = pgp::pk_decrypt(
+        // Check for detached signatures.
-            data,
+        // If decrypted part is a multipart/signed, then there is a detached signature.
-            private_keyring,
+        let decrypted_part = mailparse::parse_mail(&plain)?;
-            public_keyring_for_validate,
+        if let Some((content, valid_detached_signatures)) =
-            Some(ret_valid_signatures),
+            validate_detached_signature(&decrypted_part, &public_keyring_for_validate).await?
-        )
+        {
-        .await?;
+            return Ok(Some((content, valid_detached_signatures)));
+        } else {
+            // If the message was wrongly or not signed, still return the plain text.
+            // The caller has to check the signatures then.
 
-        // If the message was wrongly or not signed, still return the plain text.
+            return Ok(Some((plain, ret_valid_signatures)));
-        // The caller has to check the signatures then.
+        }
-
-        return Ok(Some(plain));
     }
 
     Ok(None)

src/pgp.rs

@@ -8,7 +8,7 @@ use anyhow::{bail, ensure, format_err, Result};
 use pgp::armor::BlockType;
 use pgp::composed::{
     Deserializable, KeyType as PgpKeyType, Message, SecretKeyParamsBuilder, SignedPublicKey,
-    SignedPublicSubKey, SignedSecretKey, SubkeyParamsBuilder,
+    SignedPublicSubKey, SignedSecretKey, StandaloneSignature, SubkeyParamsBuilder,
 };
 use pgp::crypto::{HashAlgorithm, SymmetricKeyAlgorithm};
 use pgp::types::{
@@ -277,16 +277,17 @@ pub async fn pk_encrypt(
 /// Receiver private keys are provided in
 /// `private_keys_for_decryption`.
 ///
-/// If `ret_signature_fingerprints` is not `None`, stores fingerprints
+/// Returns decrypted message and fingerprints
 /// of all keys from the `public_keys_for_validation` keyring that
 /// have valid signatures there.
 #[allow(clippy::implicit_hasher)]
 pub async fn pk_decrypt(
     ctext: Vec<u8>,
     private_keys_for_decryption: Keyring<SignedSecretKey>,
-    public_keys_for_validation: Keyring<SignedPublicKey>,
+    public_keys_for_validation: &Keyring<SignedPublicKey>,
-    ret_signature_fingerprints: Option<&mut HashSet<Fingerprint>>,
+) -> Result<(Vec<u8>, HashSet<Fingerprint>)> {
-) -> Result<Vec<u8>> {
+    let mut ret_signature_fingerprints: HashSet<Fingerprint> = Default::default();
+
     let msgs = async_std::task::spawn_blocking(move || {
         let cursor = Cursor::new(ctext);
         let (msg, _) = Message::from_armor_single(cursor)?;
@@ -308,33 +309,54 @@ pub async fn pk_decrypt(
             None => bail!("The decrypted message is empty"),
         };
 
-        if let Some(ret_signature_fingerprints) = ret_signature_fingerprints {
+        if !public_keys_for_validation.is_empty() {
-            if !public_keys_for_validation.is_empty() {
+            let pkeys = public_keys_for_validation.keys();
-                let fingerprints = async_std::task::spawn_blocking(move || {
-                    let pkeys = public_keys_for_validation.keys();
 
-                    let mut fingerprints: Vec<Fingerprint> = Vec::new();
+            let mut fingerprints: Vec<Fingerprint> = Vec::new();
-                    if let signed_msg @ pgp::composed::Message::Signed { .. } = msg {
+            if let signed_msg @ pgp::composed::Message::Signed { .. } = msg {
-                        for pkey in pkeys {
+                for pkey in pkeys {
-                            if signed_msg.verify(&pkey.primary_key).is_ok() {
+                    if signed_msg.verify(&pkey.primary_key).is_ok() {
-                                let fp = DcKey::fingerprint(pkey);
+                        let fp = DcKey::fingerprint(pkey);
-                                fingerprints.push(fp);
+                        fingerprints.push(fp);
-                            }
-                        }
                     }
-                    fingerprints
+                }
-                })
-                .await;
-
-                ret_signature_fingerprints.extend(fingerprints);
             }
+
+            ret_signature_fingerprints.extend(fingerprints);
         }
-        Ok(content)
+        Ok((content, ret_signature_fingerprints))
     } else {
         bail!("No valid messages found");
     }
 }
 
+/// Validates detached signature.
+pub async fn pk_validate(
+    content: &[u8],
+    signature: &[u8],
+    public_keys_for_validation: &Keyring<SignedPublicKey>,
+) -> Result<HashSet<Fingerprint>> {
+    let mut ret: HashSet<Fingerprint> = Default::default();
+
+    let standalone_signature = StandaloneSignature::from_armor_single(Cursor::new(signature))?.0;
+    let pkeys = public_keys_for_validation.keys();
+
+    // Remove trailing CRLF before the delimiter.
+    // According to RFC 3156 it is considered to be part of the MIME delimiter for the purpose of
+    // OpenPGP signature calculation.
+    let content = content
+        .get(..content.len().saturating_sub(2))
+        .ok_or_else(|| format_err!("index is out of range"))?;
+
+    for pkey in pkeys {
+        if standalone_signature.verify(pkey, content).is_ok() {
+            let fp = DcKey::fingerprint(pkey);
+            ret.insert(fp);
+        }
+    }
+    Ok(ret)
+}
+
 /// Symmetric encryption.
 pub async fn symm_encrypt(passphrase: &str, plain: &[u8]) -> Result<String> {
     let lit_msg = Message::new_literal_bytes("", plain);
@@ -492,12 +514,10 @@ mod tests {
         decrypt_keyring.add(KEYS.alice_secret.clone());
         let mut sig_check_keyring: Keyring<SignedPublicKey> = Keyring::new();
         sig_check_keyring.add(KEYS.alice_public.clone());
-        let mut valid_signatures: HashSet<Fingerprint> = Default::default();
+        let (plain, valid_signatures) = pk_decrypt(
-        let plain = pk_decrypt(
             CTEXT_SIGNED.as_bytes().to_vec(),
             decrypt_keyring,
-            sig_check_keyring,
+            &sig_check_keyring,
-            Some(&mut valid_signatures),
         )
         .await
         .map_err(|err| println!("{:?}", err))
@@ -510,12 +530,10 @@ mod tests {
         decrypt_keyring.add(KEYS.bob_secret.clone());
         let mut sig_check_keyring = Keyring::new();
         sig_check_keyring.add(KEYS.alice_public.clone());
-        let mut valid_signatures: HashSet<Fingerprint> = Default::default();
+        let (plain, valid_signatures) = pk_decrypt(
-        let plain = pk_decrypt(
             CTEXT_SIGNED.as_bytes().to_vec(),
             decrypt_keyring,
-            sig_check_keyring,
+            &sig_check_keyring,
-            Some(&mut valid_signatures),
         )
         .await
         .map_err(|err| println!("{:?}", err))
@@ -529,15 +547,10 @@ mod tests {
         let mut keyring = Keyring::new();
         keyring.add(KEYS.alice_secret.clone());
         let empty_keyring = Keyring::new();
-        let mut valid_signatures: HashSet<Fingerprint> = Default::default();
+        let (plain, valid_signatures) =
-        let plain = pk_decrypt(
+            pk_decrypt(CTEXT_SIGNED.as_bytes().to_vec(), keyring, &empty_keyring)
-            CTEXT_SIGNED.as_bytes().to_vec(),
+                .await
-            keyring,
+                .unwrap();
-            empty_keyring,
-            Some(&mut valid_signatures),
-        )
-        .await
-        .unwrap();
         assert_eq!(plain, CLEARTEXT);
         assert_eq!(valid_signatures.len(), 0);
     }
@@ -549,12 +562,10 @@ mod tests {
         decrypt_keyring.add(KEYS.bob_secret.clone());
         let mut sig_check_keyring = Keyring::new();
         sig_check_keyring.add(KEYS.bob_public.clone());
-        let mut valid_signatures: HashSet<Fingerprint> = Default::default();
+        let (plain, valid_signatures) = pk_decrypt(
-        let plain = pk_decrypt(
             CTEXT_SIGNED.as_bytes().to_vec(),
             decrypt_keyring,
-            sig_check_keyring,
+            &sig_check_keyring,
-            Some(&mut valid_signatures),
         )
         .await
         .unwrap();
@@ -567,34 +578,14 @@ mod tests {
         let mut decrypt_keyring = Keyring::new();
         decrypt_keyring.add(KEYS.bob_secret.clone());
         let sig_check_keyring = Keyring::new();
-        let mut valid_signatures: HashSet<Fingerprint> = Default::default();
+        let (plain, valid_signatures) = pk_decrypt(
-        let plain = pk_decrypt(
             CTEXT_UNSIGNED.as_bytes().to_vec(),
             decrypt_keyring,
-            sig_check_keyring,
+            &sig_check_keyring,
-            Some(&mut valid_signatures),
         )
         .await
         .unwrap();
         assert_eq!(plain, CLEARTEXT);
         assert_eq!(valid_signatures.len(), 0);
     }
-
-    #[async_std::test]
-    async fn test_decrypt_signed_no_sigret() {
-        // Check decrypting signed cyphertext without providing the HashSet for signatures.
-        let mut decrypt_keyring = Keyring::new();
-        decrypt_keyring.add(KEYS.bob_secret.clone());
-        let mut sig_check_keyring = Keyring::new();
-        sig_check_keyring.add(KEYS.alice_public.clone());
-        let plain = pk_decrypt(
-            CTEXT_SIGNED.as_bytes().to_vec(),
-            decrypt_keyring,
-            sig_check_keyring,
-            None,
-        )
-        .await
-        .unwrap();
-        assert_eq!(plain, CLEARTEXT);
-    }
 }

test-data/message/rfc1847_encapsulation.eml

@@ -0,0 +1,60 @@
+Message-ID: <4718cf7f-67f4-291b-ccb9-a167842729ed@example.org>
+Date: Sun, 5 Dec 2021 00:00:00 +0000
+MIME-Version: 1.0
+User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
+ Thunderbird/91.3.2
+Content-Language: en-US
+To: Bob <bob@example.net>
+From: Alice <alice@example.org>
+Subject: ...
+Content-Type: multipart/encrypted;
+ protocol="application/pgp-encrypted";
+ boundary="------------68Kl9HSVGFVUMdZIowLUKskt"
+
+This is an OpenPGP/MIME encrypted message (RFC 4880 and 3156)
+--------------68Kl9HSVGFVUMdZIowLUKskt
+Content-Type: application/pgp-encrypted
+Content-Description: PGP/MIME version identification
+
+Version: 1
+
+--------------68Kl9HSVGFVUMdZIowLUKskt
+Content-Type: application/octet-stream; name="encrypted.asc"
+Content-Description: OpenPGP encrypted message
+Content-Disposition: inline; filename="encrypted.asc"
+
+-----BEGIN PGP MESSAGE-----
+
+wV4D5tq63hTeebASAQdAt2c3rVUh+l0Ps7/Je83NaA7M6HsobtfMueqLUBaeancw0rRAo7PbLDLL
+cVX3SiPw6qqZyD99JZEgxZJFWM2GVILGqdvJFl11OKqXUDbzRgq6wcBMA+PY3JvEjuMiAQf6An2O
+xxjJsLgY3Ys6Ndqm8Tqp0XxK3gQuj5Vqpgd7Qv+57psL5jLHc46RxUR/txlY3Kay3yITG82iDvi4
+fbpkes7/t8eWOrtGdyPVokhfekuCLBoF24F4tEYBsumcurkNDqY1l+dxMzGB9goQWiVOUK3n+IV8
+fWPTazXTxO5o0VbCFU6RklpW07JEQUrmTzc+cwlIMhttU+h9rkfu8lm+9+KpI8GOHGV3RSCfZ1ns
+PiZL2xgJsTXAb7dF4vaAWozS7BFfxGZ1DknrySGMUBV3nmDjy/na5YiOqe/PWaZE19LcYEUdR6K5
+AFyifXDAwi0EoMe9w+aFWqnvuOkPWnhTVNLEPAFlODnAMgqeFMfHCiIrRI/UcA/NUNuY/MCFUC17
+aAw4Gl4v/pGRnVU3H+4KhW7AqNuqXQC0SpqZDuLEfr5DqUtd7at9TJh+n3kACs7sMzj3pLmZwBcg
+HddQoI35SuiLQwa79Ws/BwwSPKjRNYcKjwrjuG+k0gk+x5vd9PfUIX1ypatyJC5ZeIpFUiqPZYlg
+RCzYaWkGvvSFKIOrEWHMcUaP1p51L3n4Bc8UjVcvoeXjD2w5/SzbQ9/gp8Pno+lk1F1StDOQcRGw
+wzlKzw9KyznRCXtBtnGqgjr1gW2c1nt3BDBqq4KKTaf64eorkWOe29Qwk7jWkh+4HOe9uYd4raU3
+sLSY/LRSbYpJnNVsympMqPYopr7pO5W7sgqU1VFtfdCVZfzgvXi1USgnqQ++2BA253nrN203ZERL
+sHwWPIjeo5kULPqV7tUfU0goc7uerEFeFjJOg+Z1ZNU9/fhfJYoJTbo+2Kd6v93PPPgGzxeAU+zL
+in4yDAAJB9yJzkbVL83G7yfJ+3J5h+19aTc6XSlkXzNyLmQvTKFqDdq2SHooAlG7UJoE6vRK+mDz
+vbND9KbAAtQ4aQp10OYNyb+ZSXiwsKrgxMP3FE3j6Ui7Q9Fp3GgJC5SR0gTcGwqRWODgQau8E26r
+ukYKlB6XJ9tPAf2BwXeqwiQ3QU1704BzbO5G3tby9TpWqnAdtEfT2LdhllrwQmPWo+lNNWf1oLWu
+ylhJ1yEWETzeClDFxeyAoehJLZImlISQQsEoEPxCqHZ60o9x6ANto6xv3CIbu0WziA2A6R7tweBi
+mCAsyZdVCL2gg2nw+UWUyv6baTDpkxtKJOvYZeyzR0TH6KExRgeKjBrWPuHxJ7b+e70/DLvfNg+x
+Q6pulf+LWDKgZ9bGCZWbutp2uFyvdW+RdJXXXmhSZ3nrhusw/PVdGeQz+3N6LK3yiVOcvLeyNqGW
+/yYST6Rmqen0/JQPDDdKh4JjmLnJ/SmPTDOCD29uB03tCDDU2mzOUUncJWURE3jmJlKGGoOq4Ar9
+W03ud3E1ks/ZXk+aqz3jQ354cqSampZcxqX90esibuV/guUI3u0N3ah+FW1IfRhP2xJ36SIzc1lu
+Bs/jehRDJ9/BSFH+lHRftcYoGjNNFzl7Hx4me8EDdfhzX0HXNUZhVYJlFktdr1cjhPNzlxlnCL8b
+MgERav2VKFBvW0LR4Mm+trtbFU1ajybVihk7R56yJ/itnTHd3BxR7s8sRsG/6a8d2QiKjfNHBU05
+KEATHBFwTz3WWBbtBMN8fmIg8g2MrOfjcaHoTAgRJVr0rf+ww+KyaI8ZsraB+KTzXk+iVegNaUe/
+CiLI+Yl9ePNkFFbi4MyrY0ujXM6zRp7nbUlDewzGpI4LTyyAQ9IUqkCnAi0k7AkM1BIp8z1wxWlW
+JRAnxGSzxgibYLZ9f/fd9vBAiYA1ZVsuZTN2iUtt2/VJr2K7zPHwgO4j2OLtR4DKazCd7IlrArRH
+BfawosWYQ7cQJyo/+wxjXccvHVrZRn8vBvmFWdKz9mi1wC1HYyLeMJwYpaPsK79TRedA34pQSuAa
+QkAO79MxOVnknYS8pEGxrwD9l9vxrlZEllnFtG+QJeXsZgMIjwCaByJs7I3skUAHcuimN1X8htU2
+ofVNpLp9SUsrtXbFp89Dxiuflj10VvcLGU2AjSsUtjEpPl0nobeJmA3RzFxJZ61RG+E=
+=dcQr
+-----END PGP MESSAGE-----
+
+--------------68Kl9HSVGFVUMdZIowLUKskt--