From d797de7a8d1f83d1b311b08e70157e4db586b65e Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 24 Jul 2023 16:34:24 +0000 Subject: [PATCH] refactor: use slices and vectors instead of Keyring wrapper This change removes all traces of dc_keyring_t, which was a C implementation of dynamically sized array. --- deltachat-ffi/Doxyfile | 2 +- src/decrypt.rs | 19 +++++---- src/e2ee.rs | 7 ++-- src/keyring.rs | 91 ------------------------------------------ src/lib.rs | 1 - src/mimeparser.rs | 6 +-- src/pgp.rs | 76 +++++++++++------------------------ 7 files changed, 40 insertions(+), 162 deletions(-) delete mode 100644 src/keyring.rs diff --git a/deltachat-ffi/Doxyfile b/deltachat-ffi/Doxyfile index 03365989d..5c60f15e3 100644 --- a/deltachat-ffi/Doxyfile +++ b/deltachat-ffi/Doxyfile @@ -846,7 +846,7 @@ EXCLUDE_PATTERNS = # exclude all test directories use the pattern */test/* ###################################################### -EXCLUDE_SYMBOLS = dc_aheader_t dc_apeerstate_t dc_e2ee_helper_t dc_imap_t dc_job*_t dc_key_t dc_keyring_t dc_loginparam_t dc_mime*_t +EXCLUDE_SYMBOLS = dc_aheader_t dc_apeerstate_t dc_e2ee_helper_t dc_imap_t dc_job*_t dc_key_t dc_loginparam_t dc_mime*_t EXCLUDE_SYMBOLS += dc_saxparser_t dc_simplify_t dc_smtp_t dc_sqlite3_t dc_strbuilder_t dc_param_t dc_hash_t dc_hashelem_t EXCLUDE_SYMBOLS += _dc_* jsmn* ###################################################### diff --git a/src/decrypt.rs b/src/decrypt.rs index e84e87ce9..009be6161 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -13,7 +13,6 @@ use crate::contact::addr_cmp; use crate::context::Context; use crate::headerdef::{HeaderDef, HeaderDefMap}; use crate::key::{DcKey, Fingerprint, SignedPublicKey, SignedSecretKey}; -use crate::keyring::Keyring; use crate::peerstate::Peerstate; use crate::pgp; @@ -26,8 +25,8 @@ use crate::pgp; pub fn try_decrypt( context: &Context, mail: &ParsedMail<'_>, - private_keyring: &Keyring, - public_keyring_for_validate: &Keyring, + private_keyring: &[SignedSecretKey], + public_keyring_for_validate: &[SignedPublicKey], ) -> Result, HashSet)>> { let encrypted_data_part = match get_autocrypt_mime(mail) .or_else(|| get_mixed_up_mime(mail)) @@ -211,8 +210,8 @@ fn get_autocrypt_mime<'a, 'b>(mail: &'a ParsedMail<'b>) -> Option<&'a ParsedMail /// Returns Ok(None) if nothing encrypted was found. fn decrypt_part( mail: &ParsedMail<'_>, - private_keyring: &Keyring, - public_keyring_for_validate: &Keyring, + private_keyring: &[SignedSecretKey], + public_keyring_for_validate: &[SignedPublicKey], ) -> Result, HashSet)>> { let data = mail.get_body_raw()?; @@ -247,7 +246,7 @@ fn has_decrypted_pgp_armor(input: &[u8]) -> bool { /// Returns None if the message is not Multipart/Signed or doesn't contain necessary parts. pub(crate) fn validate_detached_signature<'a, 'b>( mail: &'a ParsedMail<'b>, - public_keyring_for_validate: &Keyring, + public_keyring_for_validate: &[SignedPublicKey], ) -> Option<(&'a ParsedMail<'b>, HashSet)> { if mail.ctype.mimetype != "multipart/signed" { return None; @@ -267,13 +266,13 @@ pub(crate) fn validate_detached_signature<'a, 'b>( } } -pub(crate) fn keyring_from_peerstate(peerstate: Option<&Peerstate>) -> Keyring { - let mut public_keyring_for_validate: Keyring = Keyring::new(); +pub(crate) fn keyring_from_peerstate(peerstate: Option<&Peerstate>) -> Vec { + let mut public_keyring_for_validate = Vec::new(); if let Some(peerstate) = peerstate { if let Some(key) = &peerstate.public_key { - public_keyring_for_validate.add(key.clone()); + public_keyring_for_validate.push(key.clone()); } else if let Some(key) = &peerstate.gossip_key { - public_keyring_for_validate.add(key.clone()); + public_keyring_for_validate.push(key.clone()); } } public_keyring_for_validate diff --git a/src/e2ee.rs b/src/e2ee.rs index b870f89fe..160609348 100644 --- a/src/e2ee.rs +++ b/src/e2ee.rs @@ -7,7 +7,6 @@ use crate::aheader::{Aheader, EncryptPreference}; use crate::config::Config; use crate::context::Context; use crate::key::{DcKey, SignedPublicKey, SignedSecretKey}; -use crate::keyring::Keyring; use crate::peerstate::{Peerstate, PeerstateVerifiedStatus}; use crate::pgp; @@ -104,7 +103,7 @@ impl EncryptHelper { mail_to_encrypt: lettre_email::PartBuilder, peerstates: Vec<(Option, &str)>, ) -> Result { - let mut keyring: Keyring = Keyring::new(); + let mut keyring: Vec = Vec::new(); for (peerstate, addr) in peerstates .into_iter() @@ -113,9 +112,9 @@ impl EncryptHelper { let key = peerstate .take_key(min_verified) .with_context(|| format!("proper enc-key for {addr} missing, cannot encrypt"))?; - keyring.add(key); + keyring.push(key); } - keyring.add(self.public_key.clone()); + keyring.push(self.public_key.clone()); let sign_key = SignedSecretKey::load_self(context).await?; let raw_message = mail_to_encrypt.build().as_string().into_bytes(); diff --git a/src/keyring.rs b/src/keyring.rs deleted file mode 100644 index fa5e9b5f4..000000000 --- a/src/keyring.rs +++ /dev/null @@ -1,91 +0,0 @@ -//! Keyring to perform rpgp operations with. - -use anyhow::Result; - -use crate::context::Context; -use crate::key::DcKey; - -/// An in-memory keyring. -/// -/// Instances are usually constructed just for the rpgp operation and -/// short-lived. -#[derive(Clone, Debug, Default)] -pub struct Keyring -where - T: DcKey, -{ - keys: Vec, -} - -impl Keyring -where - T: DcKey, -{ - /// New empty keyring. - pub fn new() -> Keyring { - Keyring { keys: Vec::new() } - } - - /// Create a new keyring with the the user's secret key loaded. - pub async fn new_self(context: &Context) -> Result> { - let mut keyring: Keyring = Keyring::new(); - keyring.load_self(context).await?; - Ok(keyring) - } - - /// Load the user's key into the keyring. - pub async fn load_self(&mut self, context: &Context) -> Result<()> { - self.add(T::load_self(context).await?); - Ok(()) - } - - /// Add a key to the keyring. - pub fn add(&mut self, key: T) { - self.keys.push(key); - } - - pub fn len(&self) -> usize { - self.keys.len() - } - - pub fn is_empty(&self) -> bool { - self.keys.is_empty() - } - - /// A vector with reference to all the keys in the keyring. - pub fn keys(&self) -> &[T] { - &self.keys - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::key::{SignedPublicKey, SignedSecretKey}; - use crate::test_utils::{alice_keypair, TestContext}; - - #[test] - fn test_keyring_add_keys() { - let alice = alice_keypair(); - let mut pub_ring: Keyring = Keyring::new(); - pub_ring.add(alice.public.clone()); - assert_eq!(pub_ring.keys(), [alice.public]); - - let mut sec_ring: Keyring = Keyring::new(); - sec_ring.add(alice.secret.clone()); - assert_eq!(sec_ring.keys(), [alice.secret]); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_keyring_load_self() { - // new_self() implies load_self() - let t = TestContext::new_alice().await; - let alice = alice_keypair(); - - let pub_ring: Keyring = Keyring::new_self(&t).await.unwrap(); - assert_eq!(pub_ring.keys(), [alice.public]); - - let sec_ring: Keyring = Keyring::new_self(&t).await.unwrap(); - assert_eq!(sec_ring.keys(), [alice.secret]); - } -} diff --git a/src/lib.rs b/src/lib.rs index b1230e2d6..316507333 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -70,7 +70,6 @@ mod scheduler; #[macro_use] mod job; pub mod key; -mod keyring; pub mod location; mod login_param; pub mod message; diff --git a/src/mimeparser.rs b/src/mimeparser.rs index a0cccd1b2..bd0879398 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -28,7 +28,6 @@ use crate::dehtml::dehtml; use crate::events::EventType; use crate::headerdef::{HeaderDef, HeaderDefMap}; use crate::key::{DcKey, Fingerprint, SignedPublicKey, SignedSecretKey}; -use crate::keyring::Keyring; use crate::message::{self, set_msg_failed, update_msg_state, MessageState, MsgId, Viewtype}; use crate::param::{Param, Params}; use crate::peerstate::Peerstate; @@ -276,9 +275,10 @@ impl MimeMessage { headers.remove("chat-verified"); let from = from.context("No from in message")?; - let private_keyring: Keyring = Keyring::new_self(context) + let private_keyring = vec![SignedSecretKey::load_self(context) .await - .context("failed to get own keyring")?; + .context("Failed to get own key")?]; + let mut decryption_info = prepare_decryption(context, &mail, &from.addr, message_time).await?; diff --git a/src/pgp.rs b/src/pgp.rs index 43ea19f9a..2eaf7e826 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -20,7 +20,6 @@ use tokio::runtime::Handle; use crate::constants::KeyGenType; use crate::key::{DcKey, Fingerprint}; -use crate::keyring::Keyring; use crate::tools::EmailAddress; #[allow(missing_docs)] @@ -229,7 +228,7 @@ fn select_pk_for_encryption(key: &SignedPublicKey) -> Option, + public_keys_for_encryption: Vec, private_key_for_signing: Option, ) -> Result { let lit_msg = Message::new_literal_bytes("", plain); @@ -237,7 +236,6 @@ pub async fn pk_encrypt( Handle::current() .spawn_blocking(move || { let pkeys: Vec = public_keys_for_encryption - .keys() .iter() .filter_map(select_pk_for_encryption) .collect(); @@ -288,15 +286,15 @@ pub fn pk_calc_signature( #[allow(clippy::implicit_hasher)] pub fn pk_decrypt( ctext: Vec, - private_keys_for_decryption: &Keyring, - public_keys_for_validation: &Keyring, + private_keys_for_decryption: &[SignedSecretKey], + public_keys_for_validation: &[SignedPublicKey], ) -> Result<(Vec, HashSet)> { let mut ret_signature_fingerprints: HashSet = Default::default(); let cursor = Cursor::new(ctext); let (msg, _) = Message::from_armor_single(cursor)?; - let skeys: Vec<&SignedSecretKey> = private_keys_for_decryption.keys().iter().collect(); + let skeys: Vec<&SignedSecretKey> = private_keys_for_decryption.iter().collect(); let (decryptor, _) = msg.decrypt(|| "".into(), &skeys[..])?; let msgs = decryptor.collect::>>()?; @@ -311,20 +309,13 @@ pub fn pk_decrypt( None => bail!("The decrypted message is empty"), }; - if !public_keys_for_validation.is_empty() { - let pkeys = public_keys_for_validation.keys(); - - let mut fingerprints: Vec = Vec::new(); - if let signed_msg @ pgp::composed::Message::Signed { .. } = msg { - for pkey in pkeys { - if signed_msg.verify(&pkey.primary_key).is_ok() { - let fp = DcKey::fingerprint(pkey); - fingerprints.push(fp); - } + if let signed_msg @ pgp::composed::Message::Signed { .. } = msg { + for pkey in public_keys_for_validation { + if signed_msg.verify(&pkey.primary_key).is_ok() { + let fp = DcKey::fingerprint(pkey); + ret_signature_fingerprints.insert(fp); } } - - ret_signature_fingerprints.extend(fingerprints); } Ok((content, ret_signature_fingerprints)) } else { @@ -336,12 +327,11 @@ pub fn pk_decrypt( pub fn pk_validate( content: &[u8], signature: &[u8], - public_keys_for_validation: &Keyring, + public_keys_for_validation: &[SignedPublicKey], ) -> Result> { let mut ret: HashSet = Default::default(); let standalone_signature = StandaloneSignature::from_armor_single(Cursor::new(signature))?.0; - let pkeys = public_keys_for_validation.keys(); // Remove trailing CRLF before the delimiter. // According to RFC 3156 it is considered to be part of the MIME delimiter for the purpose of @@ -350,7 +340,7 @@ pub fn pk_validate( .get(..content.len().saturating_sub(2)) .context("index is out of range")?; - for pkey in pkeys { + for pkey in public_keys_for_validation { if standalone_signature.verify(pkey, content).is_ok() { let fp = DcKey::fingerprint(pkey); ret.insert(fp); @@ -485,9 +475,7 @@ mod tests { async fn ctext_signed() -> &'static String { CTEXT_SIGNED .get_or_init(|| async { - let mut keyring = Keyring::new(); - keyring.add(KEYS.alice_public.clone()); - keyring.add(KEYS.bob_public.clone()); + let keyring = vec![KEYS.alice_public.clone(), KEYS.bob_public.clone()]; pk_encrypt(CLEARTEXT, keyring, Some(KEYS.alice_secret.clone())) .await @@ -500,9 +488,7 @@ mod tests { async fn ctext_unsigned() -> &'static String { CTEXT_UNSIGNED .get_or_init(|| async { - let mut keyring = Keyring::new(); - keyring.add(KEYS.alice_public.clone()); - keyring.add(KEYS.bob_public.clone()); + let keyring = vec![KEYS.alice_public.clone(), KEYS.bob_public.clone()]; pk_encrypt(CLEARTEXT, keyring, None).await.unwrap() }) .await @@ -527,10 +513,8 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_decrypt_singed() { // Check decrypting as Alice - let mut decrypt_keyring: Keyring = Keyring::new(); - decrypt_keyring.add(KEYS.alice_secret.clone()); - let mut sig_check_keyring: Keyring = Keyring::new(); - sig_check_keyring.add(KEYS.alice_public.clone()); + let decrypt_keyring = vec![KEYS.alice_secret.clone()]; + let sig_check_keyring = vec![KEYS.alice_public.clone()]; let (plain, valid_signatures) = pk_decrypt( ctext_signed().await.as_bytes().to_vec(), &decrypt_keyring, @@ -541,10 +525,8 @@ mod tests { assert_eq!(valid_signatures.len(), 1); // Check decrypting as Bob - let mut decrypt_keyring = Keyring::new(); - decrypt_keyring.add(KEYS.bob_secret.clone()); - let mut sig_check_keyring = Keyring::new(); - sig_check_keyring.add(KEYS.alice_public.clone()); + let decrypt_keyring = vec![KEYS.bob_secret.clone()]; + let sig_check_keyring = vec![KEYS.alice_public.clone()]; let (plain, valid_signatures) = pk_decrypt( ctext_signed().await.as_bytes().to_vec(), &decrypt_keyring, @@ -557,15 +539,9 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_decrypt_no_sig_check() { - let mut keyring = Keyring::new(); - keyring.add(KEYS.alice_secret.clone()); - let empty_keyring = Keyring::new(); - let (plain, valid_signatures) = pk_decrypt( - ctext_signed().await.as_bytes().to_vec(), - &keyring, - &empty_keyring, - ) - .unwrap(); + let keyring = vec![KEYS.alice_secret.clone()]; + let (plain, valid_signatures) = + pk_decrypt(ctext_signed().await.as_bytes().to_vec(), &keyring, &[]).unwrap(); assert_eq!(plain, CLEARTEXT); assert_eq!(valid_signatures.len(), 0); } @@ -573,10 +549,8 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_decrypt_signed_no_key() { // The validation does not have the public key of the signer. - let mut decrypt_keyring = Keyring::new(); - decrypt_keyring.add(KEYS.bob_secret.clone()); - let mut sig_check_keyring = Keyring::new(); - sig_check_keyring.add(KEYS.bob_public.clone()); + let decrypt_keyring = vec![KEYS.bob_secret.clone()]; + let sig_check_keyring = vec![KEYS.bob_public.clone()]; let (plain, valid_signatures) = pk_decrypt( ctext_signed().await.as_bytes().to_vec(), &decrypt_keyring, @@ -589,13 +563,11 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_decrypt_unsigned() { - let mut decrypt_keyring = Keyring::new(); - decrypt_keyring.add(KEYS.bob_secret.clone()); - let sig_check_keyring = Keyring::new(); + let decrypt_keyring = vec![KEYS.bob_secret.clone()]; let (plain, valid_signatures) = pk_decrypt( ctext_unsigned().await.as_bytes().to_vec(), &decrypt_keyring, - &sig_check_keyring, + &[], ) .unwrap(); assert_eq!(plain, CLEARTEXT);