several fixes and streamlinings, probably verified-group encryption is fixed, or at least we should see better errors

mmime/src/display.rs

@@ -1,4 +1,3 @@
-
 use crate::clist::*;
 
 use crate::mailimf::types::*;

mmime/src/lib.rs

@@ -19,29 +19,24 @@
 pub mod charconv;
 pub mod chash;
 pub mod clist;
+pub mod display;
 pub mod mailimf;
 pub mod mailmime;
 pub mod mmapstring;
 pub mod other;
-pub mod display;
 
 pub use self::charconv::*;
 pub use self::chash::*;
 pub use self::clist::*;
+pub use self::display::*;
 pub use self::mailimf::*;
 pub use self::mailmime::*;
 pub use self::mmapstring::*;
 pub use self::other::*;
-pub use self::display::*;
-
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::mailimf::types::*;
-    use crate::mailmime::types::*;
-
-    use std::ffi::CStr;
 
     #[test]
     fn mailmime_parse_test() {

mmime/src/mailmime/types.rs

@@ -154,7 +154,6 @@ pub struct unnamed_6 {
     pub dt_data: *const libc::c_char,
     pub dt_length: size_t,
 }
-
 pub type unnamed_7 = libc::c_uint;
 pub const MAILMIME_MESSAGE: unnamed_7 = 3;
 pub const MAILMIME_MULTIPLE: unnamed_7 = 2;

src/dc_mimeparser.rs

@@ -890,7 +890,6 @@ impl<'a> Drop for MimeParser<'a> {
         if !self.mimeroot.is_null() {
             unsafe { mailmime_free(self.mimeroot) };
         }
-        unsafe { self.e2ee_helper.thanks() };
     }
 }
 
@@ -1057,7 +1056,8 @@ unsafe fn mailmime_get_mime_type(mime: *mut Mailmime) -> (libc::c_int, Viewtype,
                     Some("alternative") => DC_MIMETYPE_MP_ALTERNATIVE,
                     Some("related") => DC_MIMETYPE_MP_RELATED,
                     Some("encrypted") => {
-                        // apparently try_decrypt failed to decrypt
+                        // maybe try_decrypt failed to decrypt
+                        // or it wasn't in proper Autocrypt format
                         DC_MIMETYPE_MP_NOT_DECRYPTABLE
                     }
                     Some("signed") => DC_MIMETYPE_MP_SIGNED,

src/dc_receive_imf.rs

@@ -1648,7 +1648,7 @@ fn check_verified_properties(
             to_ids_str,
         ),
         params![],
-        |row| Ok((row.get::<_, String>(0)?, row.get::<_, i32>(1)?)),
+        |row| Ok((row.get::<_, String>(0)?, row.get::<_, i32>(1).unwrap_or(0))),
         |rows| {
             rows.collect::<std::result::Result<Vec<_>, _>>()
                 .map_err(Into::into)
@@ -1672,7 +1672,7 @@ fn check_verified_properties(
                 || peerstate.verified_key_fingerprint != peerstate.public_key_fingerprint
                     && peerstate.verified_key_fingerprint != peerstate.gossip_key_fingerprint
             {
-                info!(context, "{} has verfied {}.", contact.get_addr(), to_addr,);
+                info!(context, "{} has verified {}.", contact.get_addr(), to_addr,);
                 let fp = peerstate.gossip_key_fingerprint.clone();
                 if let Some(fp) = fp {
                     peerstate.set_verified(0, &fp, 2);

src/e2ee.rs

@@ -1,13 +1,11 @@
 //! End-to-end encryption support.
 
-use std::any::Any;
 use std::collections::HashSet;
 use std::ffi::CStr;
 use std::ptr;
 use std::str::FromStr;
 
-use libc::{free, strcmp, strlen, strncmp};
+use libc::{strcmp, strlen, strncmp};
-use mmime::display::display_mime;
 use mmime::clist::*;
 use mmime::mailimf::types::*;
 use mmime::mailimf::types_helper::*;
@@ -289,8 +287,6 @@ impl EncryptHelper {
 
 #[derive(Debug, Default)]
 pub struct E2eeHelper {
-    cdata_to_free: Option<Box<dyn Any>>,
-
     // for decrypting only
     pub encrypted: bool,
     pub signatures: HashSet<String>,
@@ -298,14 +294,6 @@ pub struct E2eeHelper {
 }
 
 impl E2eeHelper {
-    /// Frees data referenced by "mailmime" but not freed by mailmime_free(). After calling this function,
-    /// in_out_message cannot be used any longer!
-    pub unsafe fn thanks(&mut self) {
-        if let Some(data) = self.cdata_to_free.take() {
-            free(Box::into_raw(data) as *mut _)
-        }
-    }
-
     pub unsafe fn try_decrypt(
         &mut self,
         context: &Context,
@@ -318,6 +306,8 @@ impl E2eeHelper {
         let mut private_keyring = Keyring::default();
         let mut public_keyring_for_validate = Keyring::default();
         let mut gossip_headers: *mut mailimf_fields = ptr::null_mut();
+
+        // XXX do wrapmime:: helper for the next block
         if !(in_out_message.is_null() || imffields.is_null()) {
             let mut field = mailimf_find_field(imffields, MAILIMF_FIELD_FROM as libc::c_int);
 
@@ -410,7 +400,6 @@ impl E2eeHelper {
                 }
             }
         }
-        //mailmime_print(in_out_message);
         if !gossip_headers.is_null() {
             mailimf_fields_free(gossip_headers);
         }
@@ -505,6 +494,7 @@ unsafe fn update_gossip_peerstates(
     imffields: *mut mailimf_fields,
     gossip_headers: *const mailimf_fields,
 ) -> HashSet<String> {
+    // XXX split the parsing from the modification part
     let mut recipients: Option<HashSet<String>> = None;
     let mut gossipped_addr: HashSet<String> = Default::default();
 
@@ -569,57 +559,29 @@ fn decrypt_if_autocrypt_message(
     ret_gossip_headers: *mut *mut mailimf_fields,
 ) -> Result<(bool)> {
     /* The returned bool is true if we detected an Autocrypt-encrypted
-    message and successfully decrypted it.  Decryption modifies the
+    message and successfully decrypted it. Decryption then modifies the
-    passed in mime structure in place.  The returned bool is false
+    passed in mime structure in place. The returned bool is false
     if it was not an Autocrypt message.
-    Errors are returned for failures related to decryption.
+
+    Errors are returned for failures related to decryption of AC-messages.
     */
     ensure!(!mime_undetermined.is_null(), "Invalid mime reference");
-    let mime: *mut Mailmime; 
+
-    unsafe {
+    let (mime, encrypted_data_part) = match wrapmime::get_autocrypt_mime(mime_undetermined) {
-        println!("****** INCOMING MSG BEGIN");
+        Err(_) => {
-        display_mime(mime_undetermined);
+            // not a proper autocrypt message, abort and ignore
-        println!("****** INCOMING MSG END");
-        
-        if (*mime_undetermined).mm_type != MAILMIME_MESSAGE as libc::c_int {
             return Ok(false);
         }
-        mime = (*mime_undetermined).mm_data.mm_message.mm_msg_mime;
+        Ok(res) => res,
-            
+    };
-        if (*mime).mm_type != MAILMIME_MULTIPLE as libc::c_int
-            || "encrypted" != wrapmime::get_ct_subtype(mime).unwrap_or_default()
-        {
-            return Ok(false);
-        }
-    }
-    info!(context, "found OpenPGP-encrypted message");
-    // we may have a proper Multipart/Encrypted Autocrypt Level 1 message
-    // XXX: more precise check we have exactly this specified OpenPGP-mime structure
-    // https://tools.ietf.org/html/rfc3156.html#section-4
 
-    let mut parts: Vec<*mut libc::c_void> = Vec::new();
+    let decrypted_mime = decrypt_part(
-    unsafe {
-        parts.extend((*(*mime).mm_data.mm_multipart.mm_mp_list).into_iter());
-    }
-    ensure!(parts.len() == 2, "Invalid Autocrypt Level 1 Mime Parts");
-
-    info!(context, "decrypt_if_autocrypt_message found AC-encrypted message");
-
-    // ensure protocol-parameter "application/pgp-encrypted")
-    // ensure wrapmime::get_content_type(parts[1])) == "application/octetstream"
-
-    let encrypted_mime_payload = parts[1] as *mut mmime::mailmime::types::Mailmime;
-
-    let decrypted_mime = match decrypt_part(
         context,
-        encrypted_mime_payload,
+        encrypted_data_part,
         private_keyring,
         public_keyring_for_validate,
         ret_valid_signatures,
-    ) {
+    )?;
-        Ok(res) => res,
-        Err(err) => bail!("decrypt_part failed: {}", err),
-    };
     /* decrypted_mime is a dangling pointer which we now put into
     mailmime's Ownership */
     unsafe {
@@ -627,7 +589,9 @@ fn decrypt_if_autocrypt_message(
         mailmime_free(mime);
     }
 
-    /* finally, let's return any gossip headers */
+    /* finally, let's also return gossip headers
+    XXX better return parsed headers so that upstream
+    does not need to dive into mmime-stuff again. */
     unsafe {
         if (*ret_gossip_headers).is_null() && ret_valid_signatures.len() > 0 {
             let mut dummy: libc::size_t = 0;
@@ -660,7 +624,7 @@ fn decrypt_part(
     unsafe {
         mime_data = (*mime).mm_data.mm_single;
     }
-    if !wrapmime::has_decryptable_data_(mime_data) {
+    if !wrapmime::has_decryptable_data(mime_data) {
         return Ok(ptr::null_mut());
     }
 
@@ -671,17 +635,15 @@ fn decrypt_part(
     let (decoded_data, decoded_data_bytes) =
         wrapmime::decode_dt_data(mime_data, mime_transfer_encoding)?;
     /* encrypted, non-NULL decoded data in decoded_data now ...
-    Note that we need to take care of freeing decoded_data ourself.
+    Note that we need to take care of freeing decoded_data ourself,
-    Once decryption is finished we unref() can do this, so our caller does not
+    after encryption has been attempted.
-    need to care for it.
-
     */
     let mut ret_decrypted_mime = ptr::null_mut();
 
     unsafe {
         if has_decrypted_pgp_armor(decoded_data, decoded_data_bytes as libc::c_int) {
             /* we should only have one decryption happening */
-            assert!(ret_valid_signatures.is_empty(), "corrupted");
+            ensure!(ret_valid_signatures.is_empty(), "corrupt signatures");
 
             let plain = match dc_pgp_pk_decrypt(
                 std::slice::from_raw_parts(decoded_data as *const u8, decoded_data_bytes),
@@ -689,7 +651,10 @@ fn decrypt_part(
                 &public_keyring_for_validate,
                 Some(ret_valid_signatures),
             ) {
-                Ok(plain) => plain,
+                Ok(plain) => {
+                    ensure!(!ret_valid_signatures.is_empty(), "no valid signatures");
+                    plain
+                }
                 Err(err) => {
                     mmap_string_unref(decoded_data);
                     bail!("could not decrypt: {}", err)
@@ -800,6 +765,7 @@ pub fn ensure_secret_key_exists(context: &Context) -> Result<String> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use libc::free;
 
     use crate::test_utils::*;
 

src/wrapmime.rs

@@ -37,14 +37,48 @@ pub fn get_ct_subtype(mime: *mut Mailmime) -> Option<String> {
 
         if !ct.is_null() && !(*ct).ct_subtype.is_null() {
             println!("ct_subtype: {}", to_string((*ct).ct_subtype));
-            Some(to_string((*ct).ct_subtype)) 
+            Some(to_string((*ct).ct_subtype))
         } else {
             None
         }
     }
 }
 
-pub fn has_decryptable_data_(mime_data: *mut mailmime_data) -> bool {
+pub fn get_autocrypt_mime(
+    mime_undetermined: *mut Mailmime,
+) -> Result<(*mut Mailmime, *mut Mailmime), Error> {
+    /* return Result with two mime pointers:
+       First mime pointer is to the multipart-mime message
+       (which is replaced with a decrypted version later)
+       Second one is to the encrypted payload.
+       For non-autocrypt message an Error is returned.
+    */
+    unsafe {
+        ensure!(
+            (*mime_undetermined).mm_type == MAILMIME_MESSAGE as libc::c_int,
+            "Not a root mime message"
+        );
+        let mime = (*mime_undetermined).mm_data.mm_message.mm_msg_mime;
+
+        ensure!(
+            (*mime).mm_type == MAILMIME_MULTIPLE as libc::c_int
+                && "encrypted" == get_ct_subtype(mime).unwrap_or_default(),
+            "Not a multipart/encrypted message"
+        );
+        let parts: Vec<_> = (*(*mime).mm_data.mm_multipart.mm_mp_list)
+            .into_iter()
+            .map(|c| c as *mut Mailmime)
+            .collect();
+        ensure!(parts.len() == 2, "Invalid Autocrypt Level 1 Mime Parts");
+        // XXX ensure protocol-parameter "application/pgp-encrypted")
+        // XXX ensure wrapmime::get_content_type(parts[1])) == "application/octetstream"
+        // a proper OpenPGP multipart/encrypted Autocrypt Level 1 message
+        // https://tools.ietf.org/html/rfc3156.html#section-4
+        Ok((mime, parts[1]))
+    }
+}
+
+pub fn has_decryptable_data(mime_data: *mut mailmime_data) -> bool {
     /* MAILMIME_DATA_FILE indicates, the data is in a file; AFAIK this is not used on parsing */
     unsafe {
         (*mime_data).dt_type == MAILMIME_DATA_TEXT as libc::c_int