From b7af53c7d960205710fb12d5bbf82376f3093c1b Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 23 May 2022 19:25:53 +0200 Subject: [PATCH] When changing self addr, transfer private key to new addr (#3351) fix #3267 --- CHANGELOG.md | 1 + src/config.rs | 89 ++++++++++++++++++++++++++++++++++++++++++++++++--- src/key.rs | 77 ++++++++++++++++++++++++-------------------- 3 files changed, 128 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45fb6b01a..aa464be09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - trim chat encryption info #3350 - fix failure to decrypt first message to self after key synchronization via Autocrypt Setup Message #3352 +- Keep pgp key when you change your own email address #3351 ## 1.83.0 diff --git a/src/config.rs b/src/config.rs index b1a2cf2eb..0b1c094c8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,7 +8,7 @@ use crate::blob::BlobObject; use crate::constants::DC_VERSION_STR; use crate::contact::addr_cmp; use crate::context::Context; -use crate::dc_tools::{dc_get_abs_path, improve_single_line_input}; +use crate::dc_tools::{dc_get_abs_path, improve_single_line_input, EmailAddress}; use crate::events::EventType; use crate::mimefactory::RECOMMENDED_FILE_SIZE; use crate::provider::{get_provider_by_id, Provider}; @@ -355,6 +355,8 @@ impl Context { /// /// This should only be used by test code and during configure. pub(crate) async fn set_primary_self_addr(&self, primary_new: &str) -> Result<()> { + let old_addr = self.get_config(Config::ConfiguredAddr).await?; + // add old primary address (if exists) to secondary addresses let mut secondary_addrs = self.get_all_self_addrs().await?; // never store a primary address also as a secondary @@ -368,6 +370,17 @@ impl Context { self.set_config(Config::ConfiguredAddr, Some(primary_new)) .await?; + if let Some(old_addr) = old_addr { + let old_addr = EmailAddress::new(&old_addr)?; + let old_keypair = crate::key::load_keypair(self, &old_addr).await?; + + if let Some(mut old_keypair) = old_keypair { + old_keypair.addr = EmailAddress::new(primary_new)?; + crate::key::store_self_keypair(self, &old_keypair, crate::key::KeyPairUse::Default) + .await?; + } + } + Ok(()) } @@ -419,7 +432,9 @@ mod tests { use std::string::ToString; use crate::constants; + use crate::dc_receive_imf::dc_receive_imf; use crate::test_utils::TestContext; + use crate::test_utils::TestContextManager; use num_traits::FromPrimitive; #[test] @@ -499,14 +514,14 @@ mod tests { assert_eq!(alice.get_all_self_addrs().await?, vec!["Alice@Example.Org"]); // Test adding a new (primary) self address - // The address is trimmed during by `LoginParam::from_database()`, + // The address is trimmed during configure by `LoginParam::from_database()`, // so `set_primary_self_addr()` doesn't have to trim it. - alice.set_primary_self_addr(" Alice@alice.com ").await?; - assert!(alice.is_self_addr(" aliCe@example.org").await?); + alice.set_primary_self_addr("Alice@alice.com").await?; + assert!(alice.is_self_addr("aliCe@example.org").await?); assert!(alice.is_self_addr("alice@alice.com").await?); assert_eq!( alice.get_all_self_addrs().await?, - vec![" Alice@alice.com ", "Alice@Example.Org"] + vec!["Alice@alice.com", "Alice@Example.Org"] ); // Check that the entry is not duplicated @@ -537,4 +552,68 @@ mod tests { Ok(()) } + + #[async_std::test] + async fn test_change_primary_self_addr() -> Result<()> { + let mut tcm = TestContextManager::new().await; + let alice = tcm.alice().await; + let bob = tcm.bob().await; + + // Alice sends a message to Bob + let alice_bob_chat = alice.create_chat(&bob).await; + let sent = alice.send_text(alice_bob_chat.id, "Hi").await; + let bob_msg = bob.recv_msg(&sent).await; + bob_msg.chat_id.accept(&bob).await?; + assert_eq!(bob_msg.text.unwrap(), "Hi"); + + // Alice changes her self address and reconfigures + // (ensure_secret_key_exists() is called during configure) + alice + .set_primary_self_addr("alice@someotherdomain.xyz") + .await?; + crate::e2ee::ensure_secret_key_exists(&alice).await?; + + assert_eq!( + alice.get_primary_self_addr().await?, + "alice@someotherdomain.xyz" + ); + + // Bob sends a message to Alice, encrypting to her previous key + let sent = bob.send_text(bob_msg.chat_id, "hi back").await; + + // Alice set up message forwarding so that she still receives + // the message with her new address + let alice_msg = alice.recv_msg(&sent).await; + assert_eq!(alice_msg.text, Some("hi back".to_string())); + assert_eq!(alice_msg.get_showpadlock(), true); + assert_eq!(alice_msg.chat_id, alice_bob_chat.id); + + // Even if Bob sends a message to Alice without In-Reply-To, + // it's still assigned to the 1:1 chat with Bob and not to + // a group (without secondary addresses, an ad-hoc group + // would be created) + dc_receive_imf( + &alice, + b"From: bob@example.net +To: alice@example.org +Chat-Version: 1.0 +Message-ID: <456@example.com> + +Message w/out In-Reply-To +", + false, + ) + .await?; + + let alice_msg = alice.get_last_msg().await; + + assert_eq!( + alice_msg.text, + Some("Message w/out In-Reply-To".to_string()) + ); + assert_eq!(alice_msg.get_showpadlock(), false); + assert_eq!(alice_msg.chat_id, alice_bob_chat.id); + + Ok(()) + } } diff --git a/src/key.rs b/src/key.rs index a0296118d..923314dc0 100644 --- a/src/key.rs +++ b/src/key.rs @@ -204,29 +204,8 @@ async fn generate_keypair(context: &Context) -> Result { let _guard = context.generating_key_mutex.lock().await; // Check if the key appeared while we were waiting on the lock. - match context - .sql - .query_row_optional( - r#" - SELECT public_key, private_key - FROM keypairs - WHERE addr=?1 - AND is_default=1; - "#, - paramsv![addr], - |row| { - let pub_bytes: Vec = row.get(0)?; - let sec_bytes: Vec = row.get(1)?; - Ok((pub_bytes, sec_bytes)) - }, - ) - .await? - { - Some((pub_bytes, sec_bytes)) => Ok(KeyPair { - addr, - public: SignedPublicKey::from_slice(&pub_bytes)?, - secret: SignedSecretKey::from_slice(&sec_bytes)?, - }), + match load_keypair(context, &addr).await? { + Some(key_pair) => Ok(key_pair), None => { let start = std::time::SystemTime::now(); let keytype = KeyGenType::from_i32(context.get_config_int(Config::KeyGenType).await?) @@ -246,6 +225,39 @@ async fn generate_keypair(context: &Context) -> Result { } } +pub(crate) async fn load_keypair( + context: &Context, + addr: &EmailAddress, +) -> Result> { + let res = context + .sql + .query_row_optional( + r#" + SELECT public_key, private_key + FROM keypairs + WHERE addr=?1 + AND is_default=1; + "#, + paramsv![addr], + |row| { + let pub_bytes: Vec = row.get(0)?; + let sec_bytes: Vec = row.get(1)?; + Ok((pub_bytes, sec_bytes)) + }, + ) + .await?; + + Ok(if let Some((pub_bytes, sec_bytes)) = res { + Some(KeyPair { + addr: addr.clone(), + public: SignedPublicKey::from_slice(&pub_bytes)?, + secret: SignedSecretKey::from_slice(&sec_bytes)?, + }) + } else { + None + }) +} + /// Use of a [KeyPair] for encryption or decryption. /// /// This is used by [store_self_keypair] to know what kind of key is @@ -275,23 +287,20 @@ pub async fn store_self_keypair( keypair: &KeyPair, default: KeyPairUse, ) -> Result<()> { - // Everything should really be one transaction, more refactoring - // is needed for that. + let mut conn = context.sql.get_conn().await?; + let transaction = conn.transaction()?; + let public_key = DcKey::to_bytes(&keypair.public); let secret_key = DcKey::to_bytes(&keypair.secret); - context - .sql + transaction .execute( "DELETE FROM keypairs WHERE public_key=? OR private_key=?;", paramsv![public_key, secret_key], ) - .await .context("failed to remove old use of key")?; if default == KeyPairUse::Default { - context - .sql + transaction .execute("UPDATE keypairs SET is_default=0;", paramsv![]) - .await .context("failed to clear default")?; } let is_default = match default { @@ -302,16 +311,16 @@ pub async fn store_self_keypair( let addr = keypair.addr.to_string(); let t = time(); - context - .sql + transaction .execute( "INSERT INTO keypairs (addr, is_default, public_key, private_key, created) VALUES (?,?,?,?,?);", paramsv![addr, is_default, public_key, secret_key, t], ) - .await .context("failed to insert keypair")?; + transaction.commit()?; + Ok(()) }