diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index bd41e32e7..60dfe7934 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -1519,7 +1519,7 @@ pub unsafe extern "C" fn dc_imex( context: *mut dc_context_t, what: libc::c_int, param1: *mut libc::c_char, - param2: *mut libc::c_char, + _param2: *mut libc::c_char, ) { if context.is_null() { eprintln!("ignoring careless call to dc_imex()"); @@ -1527,7 +1527,7 @@ pub unsafe extern "C" fn dc_imex( } let ffi_context = &*context; ffi_context - .with_inner(|ctx| dc_imex::dc_imex(ctx, what, as_opt_str(param1), as_opt_str(param2))) + .with_inner(|ctx| dc_imex::dc_imex(ctx, what, as_opt_str(param1))) .ok(); } diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index 54b7b0242..896ecc174 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -1,5 +1,4 @@ use std::path::Path; -use std::ptr; use std::str::FromStr; use deltachat::chat::{self, Chat}; @@ -328,6 +327,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E }; let arg2 = args.next().unwrap_or_default(); + let blobdir = context.get_blobdir(); match arg0 { "help" | "?" => match arg1 { // TODO: reuse commands definition in main.rs. @@ -451,27 +451,24 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E } } "has-backup" => { - let ret = dc_imex_has_backup(context, context.get_blobdir()); - if ret.is_null() { - println!("No backup found."); - } + dc_imex_has_backup(context, blobdir)?; } "export-backup" => { - dc_imex(context, 11, Some(context.get_blobdir()), ptr::null()); + dc_imex(context, 11, Some(blobdir)); } "import-backup" => { ensure!(!arg1.is_empty(), "Argument missing."); - dc_imex(context, 12, Some(arg1), ptr::null()); + dc_imex(context, 12, Some(arg1)); } "export-keys" => { - dc_imex(context, 1, Some(context.get_blobdir()), ptr::null()); + dc_imex(context, 1, Some(blobdir)); } "import-keys" => { - dc_imex(context, 2, Some(context.get_blobdir()), ptr::null()); + dc_imex(context, 2, Some(blobdir)); } "export-setup" => { let setup_code = dc_create_setup_code(context); - let file_name = context.get_blobdir().join("autocrypt-setup-message.html"); + let file_name = blobdir.join("autocrypt-setup-message.html"); let file_content = dc_render_setup_file(context, &setup_code)?; std::fs::write(&file_name, file_content)?; println!( diff --git a/python/src/deltachat/account.py b/python/src/deltachat/account.py index 5445c4906..c2be949c1 100644 --- a/python/src/deltachat/account.py +++ b/python/src/deltachat/account.py @@ -2,7 +2,6 @@ from __future__ import print_function import threading -import os import re import time from array import array @@ -298,10 +297,10 @@ class Account(object): (chats, contacts, keys, media, ...). The file is created in the the `path` directory. """ - l = self._export(path, 11) - if len(l) != 1: + export_files = self._export(path, 11) + if len(export_files) != 1: raise RuntimeError("found more than one new file") - return l[0] + return export_files[0] def _imex_events_clear(self): try: @@ -311,7 +310,6 @@ class Account(object): pass def _export(self, path, imex_cmd): - snap_files = os.listdir(path) self._imex_events_clear() lib.dc_imex(self._dc_context, imex_cmd, as_dc_charpointer(path), ffi.NULL) if not self._threads.is_started(): @@ -464,6 +462,7 @@ class Account(object): def on_dc_event_imex_file_written(self, data1, data2): self._imex_events.put(data1) + class IOThreads: def __init__(self, dc_context, log_event=lambda *args: None): self._dc_context = dc_context diff --git a/python/tests/test_account.py b/python/tests/test_account.py index bf9dfb61e..d47bb074d 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -293,10 +293,10 @@ class TestOfflineChat: assert contact == ac1.get_self_contact() assert not backupdir.listdir() - path = ac1.export_to_dir(backupdir.strpath) + path = ac1.export_all(backupdir.strpath) assert os.path.exists(path) ac2 = acfactory.get_unconfigured_account() - ac2.import_from_file(path) + ac2.import_all(path) contacts = ac2.get_contacts(query="some1") assert len(contacts) == 1 contact2 = contacts[0] @@ -343,9 +343,9 @@ class TestOnlineAccount: def test_export_import_self_keys(self, acfactory, tmpdir): ac1, ac2 = acfactory.get_two_online_accounts() dir = tmpdir.mkdir("exportdir") - l = ac1.export_self_keys(dir.strpath) - assert len(l) == 2 - for x in l: + export_files = ac1.export_self_keys(dir.strpath) + assert len(export_files) == 2 + for x in export_files: assert x.startswith(dir.strpath) ac1._evlogger.consume_events() ac1.import_self_keys(dir.strpath) diff --git a/src/dc_imex.rs b/src/dc_imex.rs index a5283ce5a..97a59f48c 100644 --- a/src/dc_imex.rs +++ b/src/dc_imex.rs @@ -2,7 +2,7 @@ use std::ffi::CString; use std::path::Path; use std::ptr; -use libc::{free, strcmp, strlen, strstr}; +use libc::{free, strlen}; use mmime::mailmime_content::*; use mmime::mmapstring::*; use mmime::other::*; @@ -30,20 +30,12 @@ use crate::stock::StockMessage; // param1 is a directory where the keys are searched in and read from // param1 is a directory where the backup is written to // param1 is the file with the backup to import -pub fn dc_imex( - context: &Context, - what: libc::c_int, - param1: Option>, - param2: Option>, -) { +pub fn dc_imex(context: &Context, what: libc::c_int, param1: Option>) { let mut param = Params::new(); param.set_int(Param::Cmd, what as i32); if let Some(param1) = param1 { param.set(Param::Arg, param1.as_ref().to_string_lossy()); } - if let Some(param2) = param2 { - param.set(Param::Arg, param2.as_ref().to_string_lossy()); - } job_kill_action(context, Action::ImexImap); job_add(context, Action::ImexImap, 0, param, 0); @@ -268,7 +260,20 @@ pub unsafe fn dc_continue_key_transfer(context: &Context, msg_id: u32, setup_cod free(armored_key as *mut libc::c_void); true } else { - false + armored_key = dc_decrypt_setup_file(context, norm_sc, buf.as_ptr().cast()); + if armored_key.is_null() { + error!(context, "Cannot decrypt Autocrypt Setup Message.",); + } else { + let armored_key_r = as_str(armored_key); + match set_self_key(context, armored_key_r, true, true) { + Ok(()) => { + success = true; + } + Err(err) => { + warn!(context, "set-self-key failed: {}", err); + } + } + } } } else { error!(context, "Cannot read Autocrypt Setup Message file.",); @@ -282,55 +287,61 @@ pub unsafe fn dc_continue_key_transfer(context: &Context, msg_id: u32, setup_cod fn set_self_key( context: &Context, - armored_c: *const libc::c_char, - set_default: libc::c_int, -) -> bool { - assert!(!armored_c.is_null(), "invalid buffer"); - let armored = as_str(armored_c); - + armored: &str, + set_default: bool, + prefer_encrypt_required: bool, +) -> Result<()> { + // try hard to only modify key-state let keys = Key::from_armored_string(armored, KeyType::Private) .and_then(|(k, h)| if k.verify() { Some((k, h)) } else { None }) .and_then(|(k, h)| k.split_key().map(|pub_key| (k, pub_key, h))); if keys.is_none() { - error!(context, "File does not contain a valid private key.",); - return false; + bail!("File does not contain a valid private key."); } let (private_key, public_key, header) = keys.unwrap(); let preferencrypt = header.get("Autocrypt-Prefer-Encrypt"); + match preferencrypt.map(|s| s.as_str()) { + Some(headerval) => { + let e2ee_enabled = match headerval { + "nopreference" => 0, + "mutual" => 1, + _ => { + bail!("invalid Autocrypt-Prefer-Encrypt header: {:?}", header); + } + }; + context + .sql + .set_config_int(context, "e2ee_enabled", e2ee_enabled)?; + } + None => { + if prefer_encrypt_required { + bail!("missing Autocrypt-Prefer-Encrypt header"); + } + } + }; - if sql::execute( + let self_addr = context.get_config(Config::ConfiguredAddr); + if self_addr.is_none() { + bail!("Missing self addr"); + } + + // XXX maybe better make dc_key_save_self_keypair delete things + sql::execute( context, &context.sql, "DELETE FROM keypairs WHERE public_key=? OR private_key=?;", params![public_key.to_bytes(), private_key.to_bytes()], - ) - .is_err() - { - return false; - } + )?; - if 0 != set_default { - if sql::execute( + if set_default { + sql::execute( context, &context.sql, "UPDATE keypairs SET is_default=0;", params![], - ) - .is_err() - { - return false; - } - } else { - error!(context, "File does not contain a private key.",); - } - - let self_addr = context.get_config(Config::ConfiguredAddr); - - if self_addr.is_none() { - error!(context, "Missing self addr"); - return false; + )?; } if !dc_key_save_self_keypair( @@ -338,25 +349,12 @@ fn set_self_key( &public_key, &private_key, self_addr.unwrap(), - set_default, + set_default as libc::c_int, &context.sql, ) { - error!(context, "Cannot save keypair."); - return false; - } - - match preferencrypt.map(|s| s.as_str()) { - Some("") => false, - Some("nopreference") => context - .sql - .set_config_int(context, "e2ee_enabled", 0) - .is_ok(), - Some("mutual") => context - .sql - .set_config_int(context, "e2ee_enabled", 1) - .is_ok(), - _ => true, + bail!("Cannot save keypair, internal key-state possibly corrupted now!"); } + Ok(()) } pub unsafe fn dc_decrypt_setup_file( @@ -365,7 +363,7 @@ pub unsafe fn dc_decrypt_setup_file( filecontent: *const libc::c_char, ) -> *mut libc::c_char { let fc_buf: *mut libc::c_char; - let mut fc_headerline: *const libc::c_char = ptr::null(); + let mut fc_headerline = String::default(); let mut fc_base64: *const libc::c_char = ptr::null(); let mut binary: *mut libc::c_char = ptr::null_mut(); let mut binary_bytes: libc::size_t = 0; @@ -379,11 +377,7 @@ pub unsafe fn dc_decrypt_setup_file( ptr::null_mut(), ptr::null_mut(), &mut fc_base64, - ) && !fc_headerline.is_null() - && strcmp( - fc_headerline, - b"-----BEGIN PGP MESSAGE-----\x00" as *const u8 as *const libc::c_char, - ) == 0 + ) && fc_headerline == "-----BEGIN PGP MESSAGE-----" && !fc_base64.is_null() { /* convert base64 to binary */ @@ -464,7 +458,7 @@ pub fn dc_job_do_DC_JOB_IMEX_IMAP(context: &Context, job: &Job) -> Result<()> { } let success = match what { DC_IMEX_EXPORT_SELF_KEYS => export_self_keys(context, ¶m1_s), - DC_IMEX_IMPORT_SELF_KEYS => unsafe { import_self_keys(context, ¶m1_s) }, + DC_IMEX_IMPORT_SELF_KEYS => import_self_keys(context, ¶m1_s), DC_IMEX_EXPORT_BACKUP => unsafe { export_backup(context, param1.as_ptr()) }, DC_IMEX_IMPORT_BACKUP => unsafe { import_backup(context, param1.as_ptr()) }, _ => { @@ -773,105 +767,88 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool { /******************************************************************************* * Classic key import ******************************************************************************/ -unsafe fn import_self_keys(context: &Context, dir_name: &str) -> bool { +fn import_self_keys(context: &Context, dir_name: &str) -> bool { /* hint: even if we switch to import Autocrypt Setup Files, we should leave the possibility to import plain ASC keys, at least keys without a password, if we do not want to implement a password entry function. Importing ASC keys is useful to use keys in Delta Chat used by any other non-Autocrypt-PGP implementation. Maybe we should make the "default" key handlong also a little bit smarter (currently, the last imported key is the standard key unless it contains the string "legacy" in its name) */ - let mut set_default: libc::c_int; - let mut buf: *mut libc::c_char = ptr::null_mut(); - // a pointer inside buf, MUST NOT be free()'d - let mut private_key: *const libc::c_char; - let mut buf2: *mut libc::c_char = ptr::null_mut(); - // a pointer inside buf2, MUST NOT be free()'d - let mut buf2_headerline: *const libc::c_char = ptr::null_mut(); + if dir_name.is_empty() { + return false; + } + let mut set_default: bool; + let mut key: String; let mut imported_cnt = 0; - if !dir_name.is_empty() { - let dir = std::path::Path::new(dir_name); - if let Ok(dir_handle) = std::fs::read_dir(dir) { - for entry in dir_handle { - if let Err(err) = entry { + + let dir = std::path::Path::new(dir_name); + if let Ok(dir_handle) = std::fs::read_dir(dir) { + for entry in dir_handle { + let entry_fn = match entry { + Err(err) => { info!(context, "file-dir error: {}", err); break; } - let entry_fn = entry.unwrap().file_name(); - let name_f = entry_fn.to_string_lossy(); - - info!(context, "Checking name_f: {}", name_f); - match dc_get_filesuffix_lc(&name_f) { - Some(suffix) => { - if suffix != "asc" { - continue; - } - } - None => { + Ok(res) => res.file_name(), + }; + let name_f = entry_fn.to_string_lossy(); + let path_plus_name = dir.join(&entry_fn); + match dc_get_filesuffix_lc(&name_f) { + Some(suffix) => { + if suffix != "asc" { continue; } + set_default = if name_f.contains("legacy") { + info!(context, "found legacy key '{}'", path_plus_name.display()); + false + } else { + true + } } - let path_plus_name = dir.join(&entry_fn); - info!(context, "Checking: {}", path_plus_name.display()); - - free(buf.cast()); - buf = ptr::null_mut(); - - if let Ok(buf_r) = dc_read_file(context, &path_plus_name) { - buf = buf_r.as_ptr() as *mut _; - std::mem::forget(buf_r); - } else { + None => { continue; - }; + } + } + if let Ok(content) = dc_read_file(context, &path_plus_name) { + key = String::from_utf8_lossy(&content).to_string(); + } else { + continue; + }; - private_key = buf; - - free(buf2 as *mut libc::c_void); - buf2 = dc_strdup(buf); - if dc_split_armored_data( + /* only import if we have a private key */ + let mut buf2_headerline = String::default(); + let split_res: bool; + unsafe { + let buf2 = "".strdup(); + split_res = dc_split_armored_data( buf2, &mut buf2_headerline, ptr::null_mut(), ptr::null_mut(), ptr::null_mut(), - ) && strcmp( - buf2_headerline, - b"-----BEGIN PGP PUBLIC KEY BLOCK-----\x00" as *const u8 as *const libc::c_char, - ) == 0 - { - private_key = strstr( - buf, - b"-----BEGIN PGP PRIVATE KEY BLOCK\x00" as *const u8 as *const libc::c_char, - ); - if private_key.is_null() { - /* this is no error but quite normal as we always export the public keys together with the private ones */ - continue; - } - } - set_default = 1; - if name_f.contains("legacy") { - info!( - context, - "Treating \"{}\" as a legacy private key.", - path_plus_name.display(), - ); - set_default = 0i32 - } - if !set_self_key(context, private_key, set_default) { - continue; - } - imported_cnt += 1 + ); + free(buf2 as *mut libc::c_void); } - if imported_cnt == 0i32 { - error!(context, "No private keys found in \"{}\".", dir_name,); + if split_res + && buf2_headerline.contains("-----BEGIN PGP PUBLIC KEY BLOCK-----") + && !key.contains("-----BEGIN PGP PRIVATE KEY BLOCK") + { + info!(context, "ignoring public key file '{}", name_f); + // it's fine: DC exports public with private + continue; } - } else { - error!(context, "Import: Cannot open directory \"{}\".", dir_name,); + if let Err(err) = set_self_key(context, &key, set_default, false) { + error!(context, "set_self_key: {}", err); + continue; + } + imported_cnt += 1 } + if imported_cnt == 0i32 { + error!(context, "No private keys found in \"{}\".", dir_name,); + } + } else { + error!(context, "Import: Cannot open directory \"{}\".", dir_name,); } - - free(buf as *mut libc::c_void); - free(buf2 as *mut libc::c_void); - imported_cnt != 0 } diff --git a/src/message.rs b/src/message.rs index e1b5af3fa..b6155b6a8 100644 --- a/src/message.rs +++ b/src/message.rs @@ -2,7 +2,7 @@ use std::path::{Path, PathBuf}; use std::ptr; use deltachat_derive::{FromSql, ToSql}; -use libc::{free, strcmp}; +use libc::free; use crate::chat::{self, Chat}; use crate::constants::*; @@ -353,7 +353,7 @@ impl Message { if let Ok(mut buf) = dc_read_file(context, filename) { unsafe { // just a pointer inside buf, MUST NOT be free()'d - let mut buf_headerline = ptr::null(); + let mut buf_headerline = String::default(); // just a pointer inside buf, MUST NOT be free()'d let mut buf_setupcodebegin = ptr::null(); @@ -363,10 +363,7 @@ impl Message { &mut buf_setupcodebegin, ptr::null_mut(), ptr::null_mut(), - ) && strcmp( - buf_headerline, - b"-----BEGIN PGP MESSAGE-----\x00" as *const u8 as *const libc::c_char, - ) == 0 + ) && buf_headerline == "-----BEGIN PGP MESSAGE-----" && !buf_setupcodebegin.is_null() { return Some(to_string(buf_setupcodebegin)); diff --git a/src/pgp.rs b/src/pgp.rs index 96b144700..980703b01 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -19,7 +19,7 @@ use crate::keyring::*; pub unsafe fn dc_split_armored_data( buf: *mut libc::c_char, - ret_headerline: *mut *const libc::c_char, + ret_headerline: *mut String, ret_setupcodebegin: *mut *const libc::c_char, ret_preferencrypt: *mut *const libc::c_char, ret_base64: *mut *const libc::c_char, @@ -31,9 +31,6 @@ pub unsafe fn dc_split_armored_data( let mut p2: *mut libc::c_char; let mut headerline: *mut libc::c_char = ptr::null_mut(); let mut base64: *mut libc::c_char = ptr::null_mut(); - if !ret_headerline.is_null() { - *ret_headerline = ptr::null() - } if !ret_setupcodebegin.is_null() { *ret_setupcodebegin = ptr::null_mut(); } @@ -43,7 +40,7 @@ pub unsafe fn dc_split_armored_data( if !ret_base64.is_null() { *ret_base64 = ptr::null(); } - if !(buf.is_null() || ret_headerline.is_null()) { + if !buf.is_null() { dc_remove_cr_chars(buf); while 0 != *p1 { if *p1 as libc::c_int == '\n' as i32 { @@ -62,9 +59,7 @@ pub unsafe fn dc_split_armored_data( ) == 0i32 { headerline = line; - if !ret_headerline.is_null() { - *ret_headerline = headerline - } + *ret_headerline = as_str(headerline).to_string(); } } else if strspn(line, b"\t\r\n \x00" as *const u8 as *const libc::c_char) == strlen(line) diff --git a/tests/stress.rs b/tests/stress.rs index 5866580f5..ac08999c5 100644 --- a/tests/stress.rs +++ b/tests/stress.rs @@ -59,7 +59,7 @@ unsafe fn stress_functions(context: &Context) { assert!(res.contains(" configured_server_flags ")); let mut buf_0: *mut libc::c_char; - let mut headerline: *const libc::c_char = ptr::null(); + let mut headerline = String::default(); let mut setupcodebegin: *const libc::c_char = ptr::null(); let mut preferencrypt: *const libc::c_char = ptr::null(); let mut base64: *const libc::c_char = ptr::null(); @@ -75,14 +75,8 @@ unsafe fn stress_functions(context: &Context) { &mut base64, ); assert!(ok); - assert!(!headerline.is_null()); - assert_eq!( - strcmp( - headerline, - b"-----BEGIN PGP MESSAGE-----\x00" as *const u8 as *const libc::c_char, - ), - 0 - ); + assert!(!headerline.is_empty()); + assert_eq!(headerline, "-----BEGIN PGP MESSAGE-----"); assert!(!base64.is_null()); assert_eq!(as_str(base64 as *const libc::c_char), "data",); @@ -101,14 +95,7 @@ unsafe fn stress_functions(context: &Context) { ); assert!(ok); - assert!(!headerline.is_null()); - assert_eq!( - strcmp( - headerline, - b"-----BEGIN PGP MESSAGE-----\x00" as *const u8 as *const libc::c_char, - ), - 0 - ); + assert_eq!(headerline, "-----BEGIN PGP MESSAGE-----"); assert!(!base64.is_null()); assert_eq!(as_str(base64 as *const libc::c_char), "dat1",); @@ -128,14 +115,7 @@ unsafe fn stress_functions(context: &Context) { ); assert!(ok); - assert!(!headerline.is_null()); - assert_eq!( - strcmp( - headerline, - b"-----BEGIN PGP MESSAGE-----\x00" as *const u8 as *const libc::c_char, - ), - 0 - ); + assert_eq!(headerline, "-----BEGIN PGP MESSAGE-----"); assert!(setupcodebegin.is_null()); assert!(!base64.is_null()); @@ -165,14 +145,7 @@ unsafe fn stress_functions(context: &Context) { &mut base64, ); assert!(ok); - assert!(!headerline.is_null()); - assert_eq!( - strcmp( - headerline, - b"-----BEGIN PGP MESSAGE-----\x00" as *const u8 as *const libc::c_char, - ), - 0 - ); + assert_eq!(headerline, "-----BEGIN PGP MESSAGE-----"); assert!(!setupcodebegin.is_null()); assert_eq!( @@ -199,15 +172,7 @@ unsafe fn stress_functions(context: &Context) { &mut base64, ); assert!(ok); - assert!(!headerline.is_null()); - assert_eq!( - strcmp( - headerline, - b"-----BEGIN PGP PRIVATE KEY BLOCK-----\x00" as *const u8 as *const libc::c_char, - ), - 0 - ); - + assert_eq!(headerline, "-----BEGIN PGP PRIVATE KEY BLOCK-----"); assert!(!preferencrypt.is_null()); assert_eq!( strcmp( @@ -223,7 +188,7 @@ unsafe fn stress_functions(context: &Context) { free(buf_0 as *mut libc::c_void); let mut buf_1: *mut libc::c_char; - let mut headerline_0: *const libc::c_char = ptr::null(); + let mut headerline_0 = String::default(); let mut setupcodebegin_0: *const libc::c_char = ptr::null(); let mut preferencrypt_0: *const libc::c_char = ptr::null(); buf_1 = strdup(S_EM_SETUPFILE); @@ -234,14 +199,7 @@ unsafe fn stress_functions(context: &Context) { &mut preferencrypt_0, ptr::null_mut(), )); - assert!(!headerline_0.is_null()); - assert_eq!( - 0, - strcmp( - headerline_0, - b"-----BEGIN PGP MESSAGE-----\x00" as *const u8 as *const libc::c_char, - ) - ); + assert_eq!(headerline_0, "-----BEGIN PGP MESSAGE-----"); assert!(!setupcodebegin_0.is_null()); assert!(strlen(setupcodebegin_0) < strlen(S_EM_SETUPCODE)); assert_eq!( @@ -260,14 +218,7 @@ unsafe fn stress_functions(context: &Context) { &mut preferencrypt_0, ptr::null_mut(), )); - assert!(!headerline_0.is_null()); - assert_eq!( - strcmp( - headerline_0, - b"-----BEGIN PGP PRIVATE KEY BLOCK-----\x00" as *const u8 as *const libc::c_char, - ), - 0 - ); + assert_eq!(headerline_0, "-----BEGIN PGP PRIVATE KEY BLOCK-----"); assert!(setupcodebegin_0.is_null()); assert!(!preferencrypt_0.is_null()); assert_eq!(