When changing self addr, transfer private key to new addr (#3351)

fix #3267

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

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?;
+        alice.set_primary_self_addr("Alice@alice.com").await?;
-        assert!(alice.is_self_addr("    aliCe@example.org").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(())
+    }
 }

src/key.rs

@@ -204,29 +204,8 @@ async fn generate_keypair(context: &Context) -> Result<KeyPair> {
     let _guard = context.generating_key_mutex.lock().await;
 
     // Check if the key appeared while we were waiting on the lock.
-    match context
+    match load_keypair(context, &addr).await? {
-        .sql
+        Some(key_pair) => Ok(key_pair),
-        .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<u8> = row.get(0)?;
-                let sec_bytes: Vec<u8> = 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)?,
-        }),
         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<KeyPair> {
     }
 }
 
+pub(crate) async fn load_keypair(
+    context: &Context,
+    addr: &EmailAddress,
+) -> Result<Option<KeyPair>> {
+    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<u8> = row.get(0)?;
+                let sec_bytes: Vec<u8> = 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
+    let mut conn = context.sql.get_conn().await?;
-    // is needed for that.
+    let transaction = conn.transaction()?;
+
     let public_key = DcKey::to_bytes(&keypair.public);
     let secret_key = DcKey::to_bytes(&keypair.secret);
-    context
+    transaction
-        .sql
         .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
+        transaction
-            .sql
             .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
+    transaction
-        .sql
         .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(())
 }