introduce safety and a particular EncryptHelper

python/tests/test_account.py

@@ -353,18 +353,6 @@ class TestOnlineAccount:
         ev = ac1._evlogger.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED")
         assert ev[1] == msg_out.id
 
-    def test_two_accounts_send_receive(self, acfactory):
-        ac1, ac2 = acfactory.get_two_online_accounts()
-        chat = self.get_chat(ac1, ac2)
-
-        msg_out = chat.send_text("message1")
-
-        # wait for other account to receive
-        ev = ac2._evlogger.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED")
-        assert ev[2] == msg_out.id
-        msg_in = ac2.get_message_by_id(msg_out.id)
-        assert msg_in.text == "message1"
-
     def test_mvbox_sentbox_threads(self, acfactory):
         ac1 = acfactory.get_online_configuring_account(mvbox=True, sentbox=True)
         ac2 = acfactory.get_online_configuring_account()
@@ -400,7 +388,7 @@ class TestOnlineAccount:
         ac2.delete_messages(messages)
         assert not chat3.get_messages()
 
-    def test_send_and_receive_message(self, acfactory, lp):
+    def test_send_and_receive_message_markseen(self, acfactory, lp):
         ac1, ac2 = acfactory.get_two_online_accounts()
 
         lp.sec("ac1: create chat with ac2")

src/dc_mimefactory.rs

@@ -17,7 +17,7 @@ use crate::chat::{self, Chat};
 use crate::constants::*;
 use crate::contact::*;
 use crate::context::{get_version_str, Context};
-use crate::dc_mimeparser::SystemMessage;
+use crate::dc_mimeparser::{mailmime_find_mailimf_fields, SystemMessage};
 use crate::dc_strencode::*;
 use crate::dc_tools::*;
 use crate::e2ee::*;
@@ -340,6 +340,9 @@ pub unsafe fn dc_mimefactory_render(
         }
     };
     let message = mailmime_new_message_data(0 as *mut mailmime);
+    if message.is_null() {
+        bail!("could not create mime message data")
+    }
     mailmime_set_imf_fields(message, imf_fields);
 
     // 1=add Autocrypt-header (needed eg. for handshaking), 2=no Autocrypte-header (used for MDN)
@@ -664,6 +667,7 @@ pub unsafe fn dc_mimefactory_render(
             set_body_text(mach_mime_part, &message_text2)?;
             mailmime_add_part(multipart, mach_mime_part);
             force_plaintext = DC_FP_NO_AUTOCRYPT_HEADER;
+            info!(context, "sending MDM {:?}", message_text2);
             /* currently, we do not send MDNs encrypted:
             - in a multi-device-setup that is not set up properly, MDNs would disturb the communication as they
               are send automatically which may lead to spreading outdated Autocrypt headers.
@@ -681,7 +685,7 @@ pub unsafe fn dc_mimefactory_render(
         }
     };
 
-    /* Encrypt the message
+    /* Create the mime message
      *************************************************************************/
 
     mailimf_fields_add(
@@ -713,17 +717,30 @@ pub unsafe fn dc_mimefactory_render(
         ),
     );
 
-    let mut e2ee_helper = E2eeHelper::default();
+    /*just a pointer into mailmime structure, must not be freed*/
+    let imffields_unprotected = mailmime_find_mailimf_fields(message);
+    if imffields_unprotected.is_null() {
+        bail!("could not find mime fields");
+    }
+
+    let mut encrypt_helper = EncryptHelper::new(&context)?;
     if force_plaintext != DC_FP_NO_AUTOCRYPT_HEADER {
-        if e2ee_helper.encrypt(
+        // unless determined otherwise we add Autocrypt header
+        let aheader = encrypt_helper.get_aheader().to_string();
+        new_custom_field(imffields_unprotected, "Autocrypt", &aheader);
+    }
+    if force_plaintext == 0 {
+        let was_encrypted = encrypt_helper.try_encrypt(
             factory.context,
             &factory.recipients_addr,
-            force_plaintext == DC_FP_ADD_AUTOCRYPT_HEADER,
             e2ee_guaranteed,
             min_verified,
             do_gossip,
             message,
-        )? {
+            imffields_unprotected,
+        )?;
+        if was_encrypted {
+            info!(context, "message was encrypted");
             factory.out_encrypted = true;
             if do_gossip {
                 factory.out_gossiped = true;
@@ -733,7 +750,7 @@ pub unsafe fn dc_mimefactory_render(
     factory.out = mmap_string_new(b"\x00" as *const u8 as *const libc::c_char);
     let mut col: libc::c_int = 0;
     mailmime_write_mem(factory.out, &mut col, message);
-    e2ee_helper.thanks();
+    encrypt_helper.thanks();
     cleanup(message);
     Ok(())
 }

src/dc_receive_imf.rs

@@ -652,6 +652,17 @@ unsafe fn add_parts(
                             .set_int(Param::Cmd, mime_parser.is_system_message as i32);
                     }
 
+                    /*
+                    info!(
+                        context,
+                        "received mime message {:?}",
+                        String::from_utf8_lossy(std::slice::from_raw_parts(
+                            imf_raw_not_terminated as *const u8,
+                            imf_raw_bytes,
+                        ))
+                    );
+                    */
+
                     stmt.execute(params![
                         rfc724_mid,
                         server_folder.as_ref(),

src/e2ee.rs

@@ -34,15 +34,15 @@ use crate::securejoin::handle_degrade_event;
 use crate::wrapmime;
 use crate::wrapmime::*;
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
-pub struct E2eeHelper {
+pub struct EncryptHelper {
     cdata_to_free: Option<Box<dyn Any>>,
-    pub encrypted: bool,
+    pub prefer_encrypt: EncryptPreference,
-    pub signatures: HashSet<String>,
+    pub addr: String,
-    pub gossipped_addr: HashSet<String>,
+    pub public_key: Key,
 }
 
-impl E2eeHelper {
+impl EncryptHelper {
     /// 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) {
@@ -51,99 +51,110 @@ impl E2eeHelper {
         }
     }
 
-    pub unsafe fn encrypt(
+    pub fn new(context: &Context) -> Result<EncryptHelper> {
-        &mut self,
-        context: &Context,
-        recipients_addr: &Vec<String>,
-        force_unencrypted: bool,
-        e2ee_guaranteed: bool,
-        min_verified: libc::c_int,
-        do_gossip: bool,
-        mut in_out_message: *mut mailmime,
-    ) -> Result<bool> {
-        /* libEtPan's pgp_encrypt_mime() takes the parent as the new root. We just expect the root as being given to this function. */
-        if in_out_message.is_null() || !(*in_out_message).mm_parent.is_null() {
-            bail!("invalid inputs");
-        }
-
-        let addr = match context.get_config(Config::ConfiguredAddr) {
-            Some(addr) => addr,
-            None => {
-                bail!("addr not configured");
-            }
-        };
-
-        let public_key = match load_or_generate_self_public_key(context, &addr) {
-            Err(err) => {
-                bail!("Failed to load own public key: {}", err);
-            }
-            Ok(public_key) => public_key,
-        };
-
         let e2ee = context.sql.get_config_int(&context, "e2ee_enabled");
-
         let prefer_encrypt = if 0 != e2ee.unwrap_or_default() {
             EncryptPreference::Mutual
         } else {
             EncryptPreference::NoPreference
         };
+        let addr = match context.get_config(Config::ConfiguredAddr) {
+            None => {
+                bail!("addr not configured!");
+            }
+            Some(addr) => addr,
+        };
+
+        let public_key = match load_or_generate_self_public_key(context, &addr) {
+            Ok(res) => res,
+            Err(err) => {
+                bail!("failed to load own public key: {}", err);
+            }
+        };
+        Ok(EncryptHelper {
+            cdata_to_free: None,
+            prefer_encrypt: prefer_encrypt,
+            addr: addr,
+            public_key: public_key,
+        })
+    }
+
+    pub fn get_aheader(&self) -> Aheader {
+        let pk = self.public_key.clone();
+        let addr = self.addr.to_string();
+        Aheader::new(addr, pk, self.prefer_encrypt)
+    }
+
+    pub fn try_encrypt(
+        &mut self,
+        context: &Context,
+        recipients_addr: &Vec<String>,
+        e2ee_guaranteed: bool,
+        min_verified: libc::c_int,
+        do_gossip: bool,
+        mut in_out_message: *mut mailmime,
+        imffields_unprotected: *mut mailimf_fields,
+    ) -> Result<bool> {
+        /* libEtPan's pgp_encrypt_mime() takes the parent as the new root.
+        We just expect the root as being given to this function. */
+        if in_out_message.is_null() || unsafe { !(*in_out_message).mm_parent.is_null() } {
+            bail!("corrupted inputs");
+        }
+        if !(self.prefer_encrypt == EncryptPreference::Mutual || e2ee_guaranteed) {
+            return Ok(false);
+        }
 
-        let mut encryption_successfull = false;
-        let mut do_encrypt = false;
         let mut keyring = Keyring::default();
-        let mut peerstates: Vec<Peerstate> = Vec::new();
+        let mut gossip_headers: Vec<String> = Vec::with_capacity(recipients_addr.len());
 
-        /*only for random-seed*/
+        // determine if we can and should encrypt
-        if prefer_encrypt == EncryptPreference::Mutual || e2ee_guaranteed {
+        for recipient_addr in recipients_addr.iter() {
-            do_encrypt = true;
+            if *recipient_addr == self.addr {
-            for recipient_addr in recipients_addr.iter() {
+                continue;
-                if *recipient_addr != addr {
+            }
-                    let peerstate = Peerstate::from_addr(context, &context.sql, &recipient_addr);
+            let peerstate = match Peerstate::from_addr(context, &context.sql, &recipient_addr) {
-                    if peerstate.is_some()
+                Some(peerstate) => peerstate,
-                        && (peerstate.as_ref().unwrap().prefer_encrypt == EncryptPreference::Mutual
+                None => {
-                            || e2ee_guaranteed)
+                    let msg = format!("peerstate for {} missing, cannot encrypt", recipient_addr);
-                    {
+                    if e2ee_guaranteed {
-                        let peerstate = peerstate.unwrap();
+                        bail!("{}", msg);
-                        info!(context, "dc_e2ee_encrypt {} has peerstate", recipient_addr);
-                        if let Some(key) = peerstate.peek_key(min_verified as usize) {
-                            keyring.add_owned(key.clone());
-                            peerstates.push(peerstate);
-                        }
                     } else {
-                        info!(
+                        info!(context, "{}", msg);
-                            context,
+                        return Ok(false);
-                            "dc_e2ee_encrypt {} HAS NO peerstate {}",
-                            recipient_addr,
-                            peerstate.is_some()
-                        );
-                        do_encrypt = false;
-                        /* if we cannot encrypt to a single recipient, we cannot encrypt the message at all */
-                        break;
                     }
                 }
+            };
+            if peerstate.prefer_encrypt != EncryptPreference::Mutual && !e2ee_guaranteed {
+                info!(context, "peerstate for {} is no-encrypt", recipient_addr);
+                return Ok(false);
+            }
+
+            if let Some(key) = peerstate.peek_key(min_verified as usize) {
+                keyring.add_owned(key.clone());
+                if do_gossip {
+                    if let Some(header) = peerstate.render_gossip_header(min_verified as usize) {
+                        gossip_headers.push(header.to_string());
+                    }
+                }
+            } else {
+                bail!(
+                    "proper enc-key for {} missing, cannot encrypt",
+                    recipient_addr
+                );
             }
         }
-        let sign_key = if do_encrypt {
-            keyring.add_ref(&public_key);
-            let key = Key::from_self_private(context, addr.clone(), &context.sql);
 
+        let sign_key = {
+            keyring.add_ref(&self.public_key);
+            let key = Key::from_self_private(context, self.addr.clone(), &context.sql);
             if key.is_none() {
-                do_encrypt = false;
+                bail!("no own private key found")
             }
             key
-        } else {
-            None
         };
-        if force_unencrypted {
+
-            do_encrypt = false;
+        /* encrypt message */
-        }
+        unsafe {
-        /*just a pointer into mailmime structure, must not be freed*/
-        let imffields_unprotected = mailmime_find_mailimf_fields(in_out_message);
-        if imffields_unprotected.is_null() {
-            bail!("could not find mime fields");
-        }
-        /* encrypt message, if possible */
-        if do_encrypt {
             mailprivacy_prepare_mime(in_out_message);
             let mut part_to_encrypt: *mut mailmime =
                 (*in_out_message).mm_data.mm_message.mm_msg_mime;
@@ -163,22 +174,13 @@ impl E2eeHelper {
                 imffields_encrypted,
                 part_to_encrypt,
             );
-            if do_gossip {
+
-                for peerstate in peerstates {
+            for header in gossip_headers {
-                    peerstate
+                wrapmime::new_custom_field(imffields_encrypted, "Autocrypt-Gossip", &header)
-                        .render_gossip_header(min_verified as usize)
-                        .map(|header| {
-                            wrapmime::new_custom_field(
-                                imffields_encrypted,
-                                "Autocrypt-Gossip",
-                                &header,
-                            )
-                        });
-                }
             }
-            /* memoryhole headers */
+
-            // XXX we can't use clist's into_iter()
+            /* memoryhole headers: move some headers into encrypted part */
-            // because the loop body also removes items
+            // XXX note we can't use clist's into_iter() because the loop body also removes items
             let mut cur: *mut clistiter = (*(*imffields_unprotected).fld_list).first;
             while !cur.is_null() {
                 let field: *mut mailimf_field = (*cur).data as *mut mailimf_field;
@@ -253,10 +255,6 @@ impl E2eeHelper {
             mmap_string_free(plain);
 
             if let Ok(ctext_v) = ctext {
-                let ctext_bytes = ctext_v.len();
-                let ctext = ctext_v.strdup();
-                self.cdata_to_free = Some(Box::new(ctext));
-
                 /* create MIME-structure that will contain the encrypted text */
                 let mut encrypted_part: *mut mailmime = new_data_part(
                     ptr::null_mut(),
@@ -275,6 +273,11 @@ impl E2eeHelper {
                     MAILMIME_MECHANISM_7BIT as i32,
                 );
                 mailmime_smart_add_part(encrypted_part, version_mime);
+
+                let ctext_bytes = ctext_v.len();
+                let ctext = ctext_v.strdup();
+                self.cdata_to_free = Some(Box::new(ctext));
+
                 let ctext_part: *mut mailmime = new_data_part(
                     ctext as *mut libc::c_void,
                     ctext_bytes,
@@ -285,12 +288,31 @@ impl E2eeHelper {
                 (*in_out_message).mm_data.mm_message.mm_msg_mime = encrypted_part;
                 (*encrypted_part).mm_parent = in_out_message;
                 mailmime_free(message_to_encrypt);
-                encryption_successfull = true;
+                Ok(true)
+            } else {
+                bail!("encryption failed")
             }
         }
-        let aheader = Aheader::new(addr, public_key, prefer_encrypt).to_string();
+    }
-        new_custom_field(imffields_unprotected, "Autocrypt", &aheader);
+}
-        Ok(encryption_successfull)
+
+#[derive(Debug, Default)]
+pub struct E2eeHelper {
+    cdata_to_free: Option<Box<dyn Any>>,
+
+    // for decrypting only
+    pub encrypted: bool,
+    pub signatures: HashSet<String>,
+    pub gossipped_addr: HashSet<String>,
+}
+
+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 decrypt(&mut self, context: &Context, in_out_message: *mut mailmime) {

src/job.rs

@@ -1016,6 +1016,7 @@ fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) ->
             path_filename.display(),
         );
     } else {
+        info!(context, "add_smtp_job file written: {:?}", path_filename);
         let recipients = mimefactory.recipients_addr.join("\x1e");
         param.set(Param::File, path_filename.to_string_lossy());
         param.set(Param::Recipients, &recipients);

src/wrapmime.rs

@@ -78,8 +78,8 @@ pub fn append_ct_param(
     value: &str,
 ) -> Result<(), Error> {
     unsafe {
-        let name_c = CString::new(name).unwrap().as_ptr();
+        let name_c = name.strdup();
-        let value_c = CString::new(value).unwrap().as_ptr();
+        let value_c = value.strdup();
 
         clist_append!(
             (*content).ct_parameters,
@@ -88,6 +88,8 @@ pub fn append_ct_param(
                 value_c as *const u8 as *const libc::c_char as *mut libc::c_char
             )
         );
+        libc::free(name_c as *mut libc::c_void);
+        libc::free(value_c as *mut libc::c_void);
     }
     Ok(())
 }