fix #615 -- like with c-core Chat-Version is left in unprotected headers because

it's eg used in server-filters for detecting DC messages

.gitignore

@@ -16,6 +16,7 @@ python/.tox
 *.egg-info
 __pycache__
 python/src/deltachat/capi*.so
+python/.venv/
 
 python/liveconfig*
 

deltachat-ffi/src/lib.rs

@@ -275,11 +275,11 @@ pub unsafe extern "C" fn dc_is_open(context: *mut dc_context_t) -> libc::c_int {
         eprintln!("ignoring careless call to dc_is_open()");
         return 0;
     }
-    let ffi_context = &mut *context;
+    let ffi_context = &*context;
     let inner_guard = ffi_context.inner.read().unwrap();
     match *inner_guard {
-        Some(_) => 0,
-        None => 1,
+        Some(_) => 1,
+        None => 0,
     }
 }
 

python/src/deltachat/account.py

@@ -22,7 +22,7 @@ class Account(object):
     by the underlying deltachat c-library.  All public Account methods are
     meant to be memory-safe and return memory-safe objects.
     """
-    def __init__(self, db_path, logid=None, eventlogging=True):
+    def __init__(self, db_path, logid=None, eventlogging=True, debug=True):
         """ initialize account object.
 
         :param db_path: a path to the account database. The database
@@ -30,13 +30,14 @@ class Account(object):
         :param logid: an optional logging prefix that should be used with
                       the default internal logging.
         :param eventlogging: if False no eventlogging and no context callback will be configured
+        :param debug: turn on debug logging for events.
         """
         self._dc_context = ffi.gc(
             lib.dc_context_new(lib.py_dc_callback, ffi.NULL, ffi.NULL),
             _destroy_dc_context,
         )
         if eventlogging:
-            self._evlogger = EventLogger(self._dc_context, logid)
+            self._evlogger = EventLogger(self._dc_context, logid, debug)
             deltachat.set_context_callback(self._dc_context, self._process_event)
             self._threads = IOThreads(self._dc_context, self._evlogger._log_event)
         else:

python/tests/test_lowlevel.py

@@ -130,3 +130,21 @@ def test_get_info_open(tmpdir):
     info = cutil.from_dc_charpointer(lib.dc_get_info(ctx))
     assert 'deltachat_core_version' in info
     assert 'database_dir' in info
+
+
+def test_is_open_closed():
+    ctx = ffi.gc(
+        lib.dc_context_new(lib.py_dc_callback, ffi.NULL, ffi.NULL),
+        lib.dc_context_unref,
+    )
+    assert lib.dc_is_open(ctx) == 0
+
+
+def test_is_open_actually_open(tmpdir):
+    ctx = ffi.gc(
+        lib.dc_context_new(lib.py_dc_callback, ffi.NULL, ffi.NULL),
+        lib.dc_context_unref,
+    )
+    db_fname = tmpdir.join("test.db")
+    lib.dc_open(ctx, db_fname.strpath.encode("ascii"), ffi.NULL)
+    assert lib.dc_is_open(ctx) == 1

spec.md

@@ -117,7 +117,8 @@ The sender plus the recipients are the group members.
 To allow different groups with the same members,
 groups are identified by a group-id.
 The group-id MUST be created only from the characters
-`0`-`9`, `A`-`Z`, `a`-`z` `_` and `-`.
+`0`-`9`, `A`-`Z`, `a`-`z` `_` and `-`
+and MUST have a length of at least 11 characters.
 
 Groups MUST have a group-name.
 The group-name is any non-zero-length UTF-8 string.
@@ -144,9 +145,9 @@ The message-id MUST have the format `Gr.<group-id>.<unique data>`.
     From: member1@domain
     To: member2@domain, member3@domain
     Chat-Version: 1.0
-    Chat-Group-ID: 1234xyZ
+    Chat-Group-ID: 12345uvwxyZ
     Chat-Group-Name: My Group
-    Message-ID: Gr.1234xyZ.0001@domain
+    Message-ID: Gr.12345uvwxyZ.0001@domain
     Subject: Chat: My Group: Hello group ...
 
     Hello group - this group contains three members
@@ -196,10 +197,10 @@ and the message SHOULD appear as a message or action from the sender.
     From: member1@domain
     To: member2@domain, member3@domain, member4@domain
     Chat-Version: 1.0
-    Chat-Group-ID: 1234xyZ
+    Chat-Group-ID: 12345uvwxyZ
     Chat-Group-Name: My Group
     Chat-Group-Member-Added: member4@domain
-    Message-ID: Gr.1234xyZ.0002@domain
+    Message-ID: Gr.12345uvwxyZ.0002@domain
     Subject: Chat: My Group: Hello, ...
 
     Hello, I've added member4@domain to our group.  Now we have 4 members.
@@ -209,10 +210,10 @@ To remove a member:
     From: member1@domain
     To: member2@domain, member3@domain
     Chat-Version: 1.0
-    Chat-Group-ID: 1234xyZ
+    Chat-Group-ID: 12345uvwxyZ
     Chat-Group-Name: My Group
     Chat-Group-Member-Removed: member4@domain
-    Message-ID: Gr.1234xyZ.0003@domain
+    Message-ID: Gr.12345uvwxyZ.0003@domain
     Subject: Chat: My Group: Hello, ...
 
     Hello, I've removed member4@domain from our group.  Now we have 3 members.
@@ -233,10 +234,10 @@ and the message SHOULD appear as a message or action from the sender.
     From: member1@domain
     To: member2@domain, member3@domain
     Chat-Version: 1.0
-    Chat-Group-ID: 1234xyZ
+    Chat-Group-ID: 12345uvwxyZ
     Chat-Group-Name: Our Group
     Chat-Group-Name-Changed: My Group
-    Message-ID: Gr.1234xyZ.0004@domain
+    Message-ID: Gr.12345uvwxyZ.0004@domain
     Subject: Chat: Our Group: Hello, ...
 
     Hello, I've changed the group name from "My Group" to "Our Group".
@@ -262,10 +263,10 @@ and the message SHOULD appear as a message or action from the sender.
     From: member1@domain
     To: member2@domain, member3@domain
     Chat-Version: 1.0
-    Chat-Group-ID: 1234xyZ
+    Chat-Group-ID: 12345uvwxyZ
     Chat-Group-Name: Our Group
     Chat-Group-Image: image.jpg
-    Message-ID: Gr.1234xyZ.0005@domain
+    Message-ID: Gr.12345uvwxyZ.0005@domain
     Subject: Chat: Our Group: Hello, ...
     Content-Type: multipart/mixed; boundary="==break=="
 

src/dc_imex.rs

@@ -244,14 +244,15 @@ pub fn dc_continue_key_transfer(context: &Context, msg_id: u32, setup_code: &str
     if let Some(filename) = msg.get_file(context) {
         if let Ok(buf) = dc_read_file(context, filename) {
             let norm_sc = CString::yolo(dc_normalize_setup_code(setup_code));
-            let armored_key: String;
             unsafe {
-                let sc = dc_decrypt_setup_file(context, norm_sc.as_ptr(), buf.as_ptr().cast());
-                ensure!(!sc.is_null(), "bad setup code");
-                armored_key = to_string(sc);
-                free(sc as *mut libc::c_void);
+                if let Ok(armored_key) =
+                    dc_decrypt_setup_file(context, norm_sc.as_ptr(), buf.as_ptr().cast())
+                {
+                    set_self_key(context, &armored_key, true, true)?;
+                } else {
+                    bail!("Bad setup code.")
+                }
             }
-            set_self_key(context, &armored_key, true, true)?;
             Ok(())
         } else {
             bail!("Cannot read Autocrypt Setup Message file.");
@@ -333,7 +334,7 @@ pub unsafe fn dc_decrypt_setup_file(
     context: &Context,
     passphrase: *const libc::c_char,
     filecontent: *const libc::c_char,
-) -> *mut libc::c_char {
+) -> Result<String> {
     let fc_buf: *mut libc::c_char;
     let mut fc_headerline = String::default();
     let mut fc_base64: *const libc::c_char = ptr::null();
@@ -341,7 +342,8 @@ pub unsafe fn dc_decrypt_setup_file(
     let mut binary_bytes: libc::size_t = 0;
     let mut indx: libc::size_t = 0;
 
-    let mut payload: *mut libc::c_char = ptr::null_mut();
+    let mut payload: Result<String> = Err(format_err!("Failed to decrypt"));
+
     fc_buf = dc_strdup(filecontent);
     if dc_split_armored_data(
         fc_buf,
@@ -369,12 +371,10 @@ pub unsafe fn dc_decrypt_setup_file(
                 as_str(passphrase),
                 std::slice::from_raw_parts(binary as *const u8, binary_bytes),
             ) {
-                Ok(plain) => {
-                    let payload_c = CString::new(plain).unwrap();
-                    payload = strdup(payload_c.as_ptr());
-                }
+                Ok(plain) => payload = Ok(String::from_utf8(plain).unwrap()),
                 Err(err) => {
                     error!(context, "Failed to decrypt message: {:?}", err);
+                    payload = Err(err);
                 }
             }
         }

src/dc_mimeparser.rs

@@ -1422,7 +1422,6 @@ mod tests {
     }
 
     #[test]
-    #[ignore]
     fn test_dc_mimeparser_crash() {
         let context = dummy_context();
         let raw = include_bytes!("../test-data/message/issue_523.txt");

src/dc_tools.rs

@@ -9,7 +9,7 @@ use std::time::SystemTime;
 use std::{fmt, fs, ptr};
 
 use chrono::{Local, TimeZone};
-use libc::{memcpy, strcpy, strlen, uintptr_t};
+use libc::{memcpy, strlen};
 use mmime::clist::*;
 use mmime::mailimf::types::*;
 use rand::{thread_rng, Rng};
@@ -149,66 +149,6 @@ pub(crate) fn dc_truncate(buf: &str, approx_chars: usize, do_unwrap: bool) -> Co
     }
 }
 
-#[allow(non_snake_case)]
-pub(crate) unsafe fn dc_truncate_n_unwrap_str(
-    buf: *mut libc::c_char,
-    approx_characters: libc::c_int,
-    do_unwrap: libc::c_int,
-) {
-    /* Function unwraps the given string and removes unnecessary whitespace.
-    Function stops processing after approx_characters are processed.
-    (as we're using UTF-8, for simplicity, we cut the string only at whitespaces). */
-    /* a single line is truncated `...` instead of `[...]` (the former is typically also used by the UI to fit strings in a rectangle) */
-    let ellipse_utf8: *const libc::c_char = if 0 != do_unwrap {
-        b" ...\x00" as *const u8 as *const libc::c_char
-    } else {
-        b" [...]\x00" as *const u8 as *const libc::c_char
-    };
-    let mut lastIsCharacter: libc::c_int = 0;
-    /* force unsigned - otherwise the `> ' '` comparison will fail */
-    let mut p1: *mut libc::c_uchar = buf as *mut libc::c_uchar;
-    while 0 != *p1 {
-        if *p1 as libc::c_int > ' ' as i32 {
-            lastIsCharacter = 1
-        } else if 0 != lastIsCharacter {
-            let used_bytes = (p1 as uintptr_t).wrapping_sub(buf as uintptr_t) as libc::size_t;
-            if dc_utf8_strnlen(buf, used_bytes) >= approx_characters as usize {
-                let buf_bytes = strlen(buf);
-                if buf_bytes.wrapping_sub(used_bytes) >= strlen(ellipse_utf8) {
-                    strcpy(p1 as *mut libc::c_char, ellipse_utf8);
-                }
-                break;
-            } else {
-                lastIsCharacter = 0;
-                if 0 != do_unwrap {
-                    *p1 = ' ' as i32 as libc::c_uchar
-                }
-            }
-        } else if 0 != do_unwrap {
-            *p1 = '\r' as i32 as libc::c_uchar
-        }
-        p1 = p1.offset(1isize)
-    }
-    if 0 != do_unwrap {
-        dc_remove_cr_chars(buf);
-    };
-}
-
-unsafe fn dc_utf8_strnlen(s: *const libc::c_char, n: libc::size_t) -> libc::size_t {
-    if s.is_null() {
-        return 0;
-    }
-
-    let mut j: libc::size_t = 0;
-    for i in 0..n {
-        if *s.add(i) as libc::c_int & 0xc0 != 0x80 {
-            j = j.wrapping_add(1)
-        }
-    }
-
-    j
-}
-
 pub(crate) unsafe fn dc_str_from_clist(
     list: *const clist,
     delimiter: *const libc::c_char,

src/e2ee.rs

@@ -182,7 +182,8 @@ impl EncryptHelper {
                         let opt_field = (*field).fld_data.fld_optional_field;
                         if !opt_field.is_null() && !(*opt_field).fld_name.is_null() {
                             let fld_name = to_string_lossy((*opt_field).fld_name);
-                            if fld_name.starts_with("Secure-Join") || fld_name.starts_with("Chat-")
+                            if fld_name.starts_with("Secure-Join")
+                                || (fld_name.starts_with("Chat-") && fld_name != "Chat-Version")
                             {
                                 move_to_encrypted = true;
                             }
@@ -586,7 +587,7 @@ unsafe fn decrypt_recursive(
         {
             for cur_data in (*(*mime).mm_data.mm_multipart.mm_mp_list).into_iter() {
                 let mut decrypted_mime: *mut Mailmime = ptr::null_mut();
-                if decrypt_part(
+                let decrypted = match decrypt_part(
                     context,
                     cur_data as *mut Mailmime,
                     private_keyring,
@@ -594,6 +595,10 @@ unsafe fn decrypt_recursive(
                     ret_valid_signatures,
                     &mut decrypted_mime,
                 ) {
+                    Ok(res) => res,
+                    Err(err) => bail!("decrypt_part: {}", err),
+                };
+                if decrypted {
                     if (*ret_gossip_headers).is_null() && ret_valid_signatures.len() > 0 {
                         let mut dummy: libc::size_t = 0;
                         let mut test: *mut mailimf_fields = ptr::null_mut();
@@ -659,113 +664,120 @@ unsafe fn decrypt_part(
     public_keyring_for_validate: &Keyring,
     ret_valid_signatures: &mut HashSet<String>,
     ret_decrypted_mime: *mut *mut Mailmime,
-) -> bool {
-    let mut ok_to_continue = true;
+) -> Result<bool> {
     let mime_data: *mut mailmime_data;
     let mut mime_transfer_encoding: libc::c_int = MAILMIME_MECHANISM_BINARY as libc::c_int;
-    /* mmap_string_unref()'d if set */
-    let mut transfer_decoding_buffer: *mut libc::c_char = ptr::null_mut();
-    /* must not be free()'d */
-    let mut decoded_data: *const libc::c_char = ptr::null_mut();
-    let mut decoded_data_bytes: libc::size_t = 0;
     let mut sth_decrypted = false;
 
+    let cleanup = |transfer_decoding_buffer: *mut libc::c_char| {
+        if !transfer_decoding_buffer.is_null() {
+            mmap_string_unref(transfer_decoding_buffer);
+        }
+    };
+
     *ret_decrypted_mime = ptr::null_mut();
     mime_data = (*mime).mm_data.mm_single;
     /* MAILMIME_DATA_FILE indicates, the data is in a file; AFAIK this is not used on parsing */
-    if !((*mime_data).dt_type != MAILMIME_DATA_TEXT as libc::c_int
+    if (*mime_data).dt_type != MAILMIME_DATA_TEXT as libc::c_int
         || (*mime_data).dt_data.dt_text.dt_data.is_null()
-        || (*mime_data).dt_data.dt_text.dt_length <= 0)
+        || (*mime_data).dt_data.dt_text.dt_length <= 0
     {
-        if !(*mime).mm_mime_fields.is_null() {
-            for cur_data in (*(*(*mime).mm_mime_fields).fld_list).into_iter() {
-                let field: *mut mailmime_field = cur_data as *mut _;
-                if (*field).fld_type == MAILMIME_FIELD_TRANSFER_ENCODING as libc::c_int
-                    && !(*field).fld_data.fld_encoding.is_null()
-                {
-                    mime_transfer_encoding = (*(*field).fld_data.fld_encoding).enc_type
-                }
+        return Ok(false);
+    }
+    if !(*mime).mm_mime_fields.is_null() {
+        for cur_data in (*(*(*mime).mm_mime_fields).fld_list).into_iter() {
+            let field: *mut mailmime_field = cur_data as *mut _;
+            if (*field).fld_type == MAILMIME_FIELD_TRANSFER_ENCODING as libc::c_int
+                && !(*field).fld_data.fld_encoding.is_null()
+            {
+                mime_transfer_encoding = (*(*field).fld_data.fld_encoding).enc_type
             }
         }
-        /* regard `Content-Transfer-Encoding:` */
-        if mime_transfer_encoding == MAILMIME_MECHANISM_7BIT as libc::c_int
-            || mime_transfer_encoding == MAILMIME_MECHANISM_8BIT as libc::c_int
-            || mime_transfer_encoding == MAILMIME_MECHANISM_BINARY as libc::c_int
+    }
+    /* regarding `Content-Transfer-Encoding:` */
+    /* mmap_string_unref()'d if set */
+    let mut transfer_decoding_buffer: *mut libc::c_char = ptr::null_mut();
+    let decoded_data: *const libc::c_char;
+    let mut decoded_data_bytes: libc::size_t = 0;
+    if mime_transfer_encoding == MAILMIME_MECHANISM_7BIT as libc::c_int
+        || mime_transfer_encoding == MAILMIME_MECHANISM_8BIT as libc::c_int
+        || mime_transfer_encoding == MAILMIME_MECHANISM_BINARY as libc::c_int
+    {
+        decoded_data = (*mime_data).dt_data.dt_text.dt_data;
+        decoded_data_bytes = (*mime_data).dt_data.dt_text.dt_length;
+        if decoded_data.is_null() || decoded_data_bytes <= 0 {
+            /* no error - but no data */
+            return Ok(false);
+        }
+    } else {
+        let r: libc::c_int;
+        let mut current_index: libc::size_t = 0;
+        r = mailmime_part_parse(
+            (*mime_data).dt_data.dt_text.dt_data,
+            (*mime_data).dt_data.dt_text.dt_length,
+            &mut current_index,
+            mime_transfer_encoding,
+            &mut transfer_decoding_buffer,
+            &mut decoded_data_bytes,
+        );
+        if r != MAILIMF_NO_ERROR as libc::c_int
+            || transfer_decoding_buffer.is_null()
+            || decoded_data_bytes <= 0
         {
-            decoded_data = (*mime_data).dt_data.dt_text.dt_data;
-            decoded_data_bytes = (*mime_data).dt_data.dt_text.dt_length;
-            if decoded_data.is_null() || decoded_data_bytes <= 0 {
-                /* no error - but no data */
-                ok_to_continue = false;
+            cleanup(transfer_decoding_buffer);
+            bail!("mailmime_part_parse returned error or invalid data");
+        }
+        decoded_data = transfer_decoding_buffer;
+    }
+
+    /* encrypted, decoded data in decoded_data now ... */
+    if has_decrypted_pgp_armor(decoded_data, decoded_data_bytes as libc::c_int) {
+        let add_signatures = if ret_valid_signatures.is_empty() {
+            Some(ret_valid_signatures)
+        } else {
+            None
+        };
+
+        /*if we already have fingerprints, do not add more; this ensures, only the fingerprints from the outer-most part are collected */
+        let plain = match dc_pgp_pk_decrypt(
+            std::slice::from_raw_parts(decoded_data as *const u8, decoded_data_bytes),
+            &private_keyring,
+            &public_keyring_for_validate,
+            add_signatures,
+        ) {
+            Ok(plain) => plain,
+            Err(err) => {
+                cleanup(transfer_decoding_buffer);
+                bail!("could not decrypt: {}", err)
+            }
+        };
+        let plain_bytes = plain.len();
+        let plain_buf = plain.as_ptr() as *const libc::c_char;
+
+        let mut index: libc::size_t = 0;
+        let mut decrypted_mime: *mut Mailmime = ptr::null_mut();
+        if mailmime_parse(
+            plain_buf as *const _,
+            plain_bytes,
+            &mut index,
+            &mut decrypted_mime,
+        ) != MAIL_NO_ERROR as libc::c_int
+            || decrypted_mime.is_null()
+        {
+            if !decrypted_mime.is_null() {
+                mailmime_free(decrypted_mime);
             }
         } else {
-            let r: libc::c_int;
-            let mut current_index: libc::size_t = 0;
-            r = mailmime_part_parse(
-                (*mime_data).dt_data.dt_text.dt_data,
-                (*mime_data).dt_data.dt_text.dt_length,
-                &mut current_index,
-                mime_transfer_encoding,
-                &mut transfer_decoding_buffer,
-                &mut decoded_data_bytes,
-            );
-            if r != MAILIMF_NO_ERROR as libc::c_int
-                || transfer_decoding_buffer.is_null()
-                || decoded_data_bytes <= 0
-            {
-                ok_to_continue = false;
-            } else {
-                decoded_data = transfer_decoding_buffer;
-            }
-        }
-        if ok_to_continue {
-            /* encrypted, decoded data in decoded_data now ... */
-            if has_decrypted_pgp_armor(decoded_data, decoded_data_bytes as libc::c_int) {
-                let add_signatures = if ret_valid_signatures.is_empty() {
-                    Some(ret_valid_signatures)
-                } else {
-                    None
-                };
-
-                /*if we already have fingerprints, do not add more; this ensures, only the fingerprints from the outer-most part are collected */
-                if let Ok(plain) = dc_pgp_pk_decrypt(
-                    std::slice::from_raw_parts(decoded_data as *const u8, decoded_data_bytes),
-                    &private_keyring,
-                    &public_keyring_for_validate,
-                    add_signatures,
-                ) {
-                    let plain_bytes = plain.len();
-                    let plain_buf = plain.as_ptr() as *const libc::c_char;
-
-                    let mut index: libc::size_t = 0;
-                    let mut decrypted_mime: *mut Mailmime = ptr::null_mut();
-                    if mailmime_parse(
-                        plain_buf as *const _,
-                        plain_bytes,
-                        &mut index,
-                        &mut decrypted_mime,
-                    ) != MAIL_NO_ERROR as libc::c_int
-                        || decrypted_mime.is_null()
-                    {
-                        if !decrypted_mime.is_null() {
-                            mailmime_free(decrypted_mime);
-                        }
-                    } else {
-                        *ret_decrypted_mime = decrypted_mime;
-                        sth_decrypted = true;
-                    }
-                    std::mem::forget(plain);
-                }
-            }
+            *ret_decrypted_mime = decrypted_mime;
+            sth_decrypted = true;
         }
+        std::mem::forget(plain);
     }
     //mailmime_substitute(mime, new_mime);
     //s. mailprivacy_gnupg.c::pgp_decrypt()
-    if !transfer_decoding_buffer.is_null() {
-        mmap_string_unref(transfer_decoding_buffer);
-    }
+    cleanup(transfer_decoding_buffer);
 
-    sth_decrypted
+    Ok(sth_decrypted)
 }
 
 unsafe fn has_decrypted_pgp_armor(str__: *const libc::c_char, mut str_bytes: libc::c_int) -> bool {

src/message.rs

@@ -2,7 +2,6 @@ use std::path::{Path, PathBuf};
 use std::ptr;
 
 use deltachat_derive::{FromSql, ToSql};
-use libc::free;
 
 use crate::chat::{self, Chat};
 use crate::constants::*;
@@ -108,18 +107,6 @@ impl Message {
             msg.hidden = row.get(18)?;
             msg.location_id = row.get(19)?;
             msg.chat_blocked = row.get::<_, Option<Blocked>>(20)?.unwrap_or_default();
-            if msg.chat_blocked == Blocked::Deaddrop {
-                if let Some(ref text) = msg.text {
-                    unsafe {
-                        let ptr = text.strdup();
-
-                        dc_truncate_n_unwrap_str(ptr, 256, 0);
-
-                        msg.text = Some(to_string(ptr));
-                        free(ptr.cast());
-                    }
-                }
-            };
             Ok(msg)
         })
     }

tests/stress.rs

@@ -209,7 +209,8 @@ unsafe fn stress_functions(context: &Context) {
 
     assert!(preferencrypt_0.is_null());
     free(buf_1 as *mut libc::c_void);
-    buf_1 = dc_decrypt_setup_file(context, S_EM_SETUPCODE, S_EM_SETUPFILE);
+    let decrypted = dc_decrypt_setup_file(context, S_EM_SETUPCODE, S_EM_SETUPFILE).unwrap();
+    buf_1 = decrypted.strdup();
     assert!(!buf_1.is_null());
     assert!(dc_split_armored_data(
         buf_1,