From 379fc72094ff0efef827d83904f1b9fc07f1bb86 Mon Sep 17 00:00:00 2001 From: Friedel Ziegelmayer Date: Fri, 17 May 2019 10:19:43 +0200 Subject: [PATCH] perf: reduce verification load (#75) - assume valid keys in the db - verify keys on import from headers + disk - use references in keyring when possible --- src/dc_aheader.rs | 8 +++++++- src/dc_e2ee.rs | 14 +++++--------- src/dc_imex.rs | 1 + src/dc_key.rs | 9 +-------- src/dc_keyring.rs | 22 ++++++++++++++++------ src/dc_pgp.rs | 20 ++++++++++++++++---- tests/stress.rs | 16 ++++++++-------- 7 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/dc_aheader.rs b/src/dc_aheader.rs index aa1d7769d..97c69f996 100644 --- a/src/dc_aheader.rs +++ b/src/dc_aheader.rs @@ -160,7 +160,13 @@ impl str::FromStr for Aheader { .remove("keydata") .and_then(|raw| Key::from_base64(&raw, KeyType::Public)) { - Some(key) => key, + Some(key) => { + if key.verify() { + key + } else { + return Err(()); + } + } None => { return Err(()); } diff --git a/src/dc_e2ee.rs b/src/dc_e2ee.rs index 4e06364f2..dd9e412da 100644 --- a/src/dc_e2ee.rs +++ b/src/dc_e2ee.rs @@ -121,11 +121,9 @@ pub unsafe fn dc_e2ee_encrypt( recipient_addr, ) && (peerstate.prefer_encrypt == 1i32 || 0 != e2ee_guaranteed) { - if let Some(key_to_use) = - dc_apeerstate_peek_key(&peerstate, min_verified) + if let Some(key) = dc_apeerstate_peek_key(&peerstate, min_verified) { - // TODO: avoid clone - keyring.add(key_to_use.clone()); + keyring.add_owned(key.clone()); peerstates.push(peerstate); } } else { @@ -143,8 +141,7 @@ pub unsafe fn dc_e2ee_encrypt( } } let sign_key = if 0 != do_encrypt { - // TODO: avoid clone - keyring.add(public_key.clone()); + keyring.add_ref(&public_key); let key = Key::from_self_private(context, addr, &context.sql.clone().read().unwrap()); @@ -670,12 +667,11 @@ pub unsafe fn dc_e2ee_decrypt( if 0 != peerstate.degrade_event { dc_handle_degrade_event(context, &peerstate); } - // TODO: avoid clone if let Some(ref key) = peerstate.gossip_key { - public_keyring_for_validate.add(key.clone()); + public_keyring_for_validate.add_ref(key); } if let Some(ref key) = peerstate.public_key { - public_keyring_for_validate.add(key.clone()); + public_keyring_for_validate.add_ref(key); } (*helper).signatures = malloc(::std::mem::size_of::()) as *mut dc_hash_t; dc_hash_init((*helper).signatures, 3i32, 1i32); diff --git a/src/dc_imex.rs b/src/dc_imex.rs index 1857357d1..0abb39df2 100644 --- a/src/dc_imex.rs +++ b/src/dc_imex.rs @@ -522,6 +522,7 @@ unsafe fn set_self_key( CStr::from_ptr(buf_base64).to_str().unwrap(), KeyType::Private, ) + .and_then(|k| if k.verify() { Some(k) } else { None }) .and_then(|k| k.split_key().map(|pub_key| (k, pub_key))) { stmt = dc_sqlite3_prepare( diff --git a/src/dc_key.rs b/src/dc_key.rs index 57d8195b0..af3fba42c 100644 --- a/src/dc_key.rs +++ b/src/dc_key.rs @@ -97,14 +97,7 @@ impl Key { }; match res { - Ok(key) => { - if key.verify() { - Some(key) - } else { - eprintln!("Invalid key: {:?}", key); - None - } - } + Ok(key) => Some(key), Err(err) => { eprintln!("Invalid key bytes: {:?}\n{}", err, hex::encode(bytes)); None diff --git a/src/dc_keyring.rs b/src/dc_keyring.rs index 1901e53d1..ff28187f0 100644 --- a/src/dc_keyring.rs +++ b/src/dc_keyring.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use crate::constants::*; use crate::dc_context::dc_context_t; use crate::dc_key::*; @@ -5,16 +7,24 @@ use crate::dc_sqlite3::*; use crate::types::*; #[derive(Default, Clone, Debug)] -pub struct Keyring { - keys: Vec, +pub struct Keyring<'a> { + keys: Vec>, } -impl Keyring { - pub fn add(&mut self, key: Key) { +impl<'a> Keyring<'a> { + pub fn add_owned(&mut self, key: Key) { + self.add(Cow::Owned(key)) + } + + pub fn add_ref(&mut self, key: &'a Key) { + self.add(Cow::Borrowed(key)) + } + + fn add(&mut self, key: Cow<'a, Key>) { self.keys.push(key); } - pub fn keys(&self) -> &[Key] { + pub fn keys(&self) -> &[Cow<'a, Key>] { &self.keys } @@ -39,7 +49,7 @@ impl Keyring { unsafe { sqlite3_bind_text(stmt, 1, self_addr, -1, None) }; while unsafe { sqlite3_step(stmt) == 100 } { if let Some(key) = Key::from_stmt(stmt, 0, KeyType::Private) { - self.add(key); + self.add_owned(key); } } unsafe { sqlite3_finalize(stmt) }; diff --git a/src/dc_pgp.rs b/src/dc_pgp.rs index a4109bffa..0109eded2 100644 --- a/src/dc_pgp.rs +++ b/src/dc_pgp.rs @@ -182,6 +182,9 @@ pub fn dc_pgp_create_keypair(addr: *const libc::c_char) -> Option<(Key, Key)> { .sign(&private_key, || "".into()) .expect("failed to sign public key"); + private_key.verify().expect("invalid private key generated"); + public_key.verify().expect("invalid public key generated"); + Some((Key::Public(public_key), Key::Secret(private_key))) } @@ -197,8 +200,11 @@ pub fn dc_pgp_pk_encrypt( let lit_msg = Message::new_literal_bytes("", bytes); let pkeys: Vec<&SignedPublicKey> = public_keys_for_encryption .keys() - .iter() - .filter_map(|key| key.try_into().ok()) + .into_iter() + .filter_map(|key| { + let k: &Key = &key; + k.try_into().ok() + }) .collect(); let mut rng = thread_rng(); @@ -237,7 +243,10 @@ pub fn dc_pgp_pk_decrypt( let skeys: Vec<&SignedSecretKey> = private_keys_for_decryption .keys() .iter() - .filter_map(|key| key.try_into().ok()) + .filter_map(|key| { + let k: &Key = &key; + k.try_into().ok() + }) .collect(); msg.decrypt(|| "".into(), || "".into(), &skeys[..]) @@ -252,7 +261,10 @@ pub fn dc_pgp_pk_decrypt( let pkeys: Vec<&SignedPublicKey> = public_keys_for_validation .keys() .iter() - .filter_map(|key| key.try_into().ok()) + .filter_map(|key| { + let k: &Key = &key; + k.try_into().ok() + }) .collect(); for pkey in &pkeys { diff --git a/tests/stress.rs b/tests/stress.rs index 6bc1275d1..c9261559c 100644 --- a/tests/stress.rs +++ b/tests/stress.rs @@ -2239,8 +2239,8 @@ fn test_encryption_decryption() { let original_text: *const libc::c_char = b"This is a test\x00" as *const u8 as *const libc::c_char; let mut keyring = Keyring::default(); - keyring.add(public_key.clone()); - keyring.add(public_key2.clone()); + keyring.add_owned(public_key.clone()); + keyring.add_ref(&public_key2); let ctext = dc_pgp_pk_encrypt( original_text as *const libc::c_void, @@ -2270,13 +2270,13 @@ fn test_encryption_decryption() { let ctext_unsigned = CString::new(ctext).unwrap(); let mut keyring = Keyring::default(); - keyring.add(private_key); + keyring.add_owned(private_key); let mut public_keyring = Keyring::default(); - public_keyring.add(public_key.clone()); + public_keyring.add_ref(&public_key); let mut public_keyring2 = Keyring::default(); - public_keyring2.add(public_key2.clone()); + public_keyring2.add_owned(public_key2.clone()); let mut valid_signatures = dc_hash_t { keyClass: 0, @@ -2338,7 +2338,7 @@ fn test_encryption_decryption() { dc_hash_clear(&mut valid_signatures); - public_keyring2.add(public_key.clone()); + public_keyring2.add_ref(&public_key); let plain = dc_pgp_pk_decrypt( ctext_signed.as_ptr() as *const _, @@ -2371,9 +2371,9 @@ fn test_encryption_decryption() { dc_hash_clear(&mut valid_signatures); let mut keyring = Keyring::default(); - keyring.add(private_key2); + keyring.add_ref(&private_key2); let mut public_keyring = Keyring::default(); - public_keyring.add(public_key); + public_keyring.add_ref(&public_key); let plain = dc_pgp_pk_decrypt( ctext_signed.as_ptr() as *const _,