feat: add secondary verified key

When a key is gossiped for the contact in a verified chat,
it is stored in the secondary verified key slot.

The messages are then encrypted to the secondary verified key
if they are also encrypted to the contact introducing this secondary key.

Chat-Group-Member-Added no longer updates the verified key.
Verified group recovery only relies on the secondary verified key.

When a message is received from a contact
signed with a secondary verified key,
secondary verified key replaces the primary verified key.
When verified key is changed for the contact
in response to receiving a message
signed with a secondary verified key,
"Setup changed" message is added
to the same chat where the message is received.

deltachat-rpc-client/tests/test_securejoin.py

@@ -1,6 +1,5 @@
 import logging
 
-import pytest
 from deltachat_rpc_client import Chat, SpecialContactId
 
 
@@ -61,7 +60,6 @@ def test_qr_securejoin(acfactory):
     assert bob_contact_alice_snapshot.is_verified
 
 
-@pytest.mark.xfail()
 def test_verified_group_recovery(acfactory, rpc) -> None:
     ac1, ac2, ac3 = acfactory.get_online_accounts(3)
 
@@ -152,6 +150,10 @@ def test_verified_group_recovery(acfactory, rpc) -> None:
     ac1_contact_ac3 = ac1.get_contact_by_addr(ac3.get_config("addr"))
     assert ac1_contact_ac2.get_snapshot().verifier_id == ac1_contact_ac3.id
 
+    ac1_chat_messages = snapshot.chat.get_messages()
+    ac2_addr = ac2.get_config("addr")
+    assert ac1_chat_messages[-1].get_snapshot().text == f"Changed setup for {ac2_addr}"
+
 
 def test_verified_group_member_added_recovery(acfactory) -> None:
     ac1, ac2, ac3 = acfactory.get_online_accounts(3)
@@ -226,12 +228,6 @@ def test_verified_group_member_added_recovery(acfactory) -> None:
     snapshot = ac1.get_message_by_id(ac1.wait_for_incoming_msg_event().msg_id).get_snapshot()
     assert "added" in snapshot.text
 
-    logging.info("ac2 address is %s", ac2.get_config("addr"))
-    ac1_contact_ac2 = ac1.get_contact_by_addr(ac2.get_config("addr"))
-    ac1_contact_ac2_snapshot = ac1_contact_ac2.get_snapshot()
-    # assert ac1_contact_ac2_snapshot.is_verified
-    assert ac1_contact_ac2_snapshot.verifier_id == ac1.get_contact_by_addr(ac3.get_config("addr")).id
-
     chat = Chat(ac2, chat_id)
     chat.send_text("Works again!")
 
@@ -243,6 +239,11 @@ def test_verified_group_member_added_recovery(acfactory) -> None:
     snapshot = ac1.get_message_by_id(ac1.wait_for_incoming_msg_event().msg_id).get_snapshot()
     assert snapshot.text == "Works again!"
 
+    ac1_contact_ac2 = ac1.get_contact_by_addr(ac2.get_config("addr"))
+    ac1_contact_ac2_snapshot = ac1_contact_ac2.get_snapshot()
+    assert ac1_contact_ac2_snapshot.is_verified
+    assert ac1_contact_ac2_snapshot.verifier_id == ac1.get_contact_by_addr(ac3.get_config("addr")).id
+
     # ac2 is now verified by ac3 for ac1
     ac1_contact_ac3 = ac1.get_contact_by_addr(ac3.get_config("addr"))
     assert ac1_contact_ac2.get_snapshot().verifier_id == ac1_contact_ac3.id

src/e2ee.rs

@@ -100,16 +100,39 @@ impl EncryptHelper {
     ) -> Result<String> {
         let mut keyring: Vec<SignedPublicKey> = Vec::new();
 
+        let mut verifier_addresses: Vec<&str> = Vec::new();
+
         for (peerstate, addr) in peerstates
-            .into_iter()
-            .filter_map(|(state, addr)| state.map(|s| (s, addr)))
+            .iter()
+            .filter_map(|(state, addr)| state.clone().map(|s| (s, addr)))
         {
             let key = peerstate
                 .take_key(min_verified)
                 .with_context(|| format!("proper enc-key for {addr} missing, cannot encrypt"))?;
             keyring.push(key);
+            verifier_addresses.push(addr);
         }
+
+        // Encrypt to self.
         keyring.push(self.public_key.clone());
+
+        // Encrypt to secondary verified keys
+        // if we also encrypt to the introducer ("verifier") of the key.
+        if min_verified == PeerstateVerifiedStatus::BidirectVerified {
+            for (peerstate, _addr) in peerstates {
+                if let Some(peerstate) = peerstate {
+                    if let (Some(key), Some(verifier)) = (
+                        peerstate.secondary_verified_key.as_ref(),
+                        peerstate.secondary_verifier.as_deref(),
+                    ) {
+                        if verifier_addresses.contains(&verifier) {
+                            keyring.push(key.clone());
+                        }
+                    }
+                }
+            }
+        }
+
         let sign_key = load_self_secret_key(context).await?;
 
         let raw_message = mail_to_encrypt.build().as_string().into_bytes();
@@ -296,8 +319,11 @@ Sent with my Delta Chat Messenger: https://delta.chat";
             gossip_key_fingerprint: Some(pub_key.fingerprint()),
             verified_key: Some(pub_key.clone()),
             verified_key_fingerprint: Some(pub_key.fingerprint()),
-            fingerprint_changed: false,
             verifier: None,
+            secondary_verified_key: None,
+            secondary_verified_key_fingerprint: None,
+            secondary_verifier: None,
+            fingerprint_changed: false,
         };
         vec![(Some(peerstate), addr)]
     }

src/peerstate.rs

@@ -1,7 +1,5 @@
 //! # [Autocrypt Peer State](https://autocrypt.org/level1.html#peer-state-management) module.
 
-use std::collections::HashSet;
-
 use anyhow::{Context as _, Error, Result};
 use num_traits::FromPrimitive;
 
@@ -40,7 +38,7 @@ pub enum PeerstateVerifiedStatus {
 }
 
 /// Peerstate represents the state of an Autocrypt peer.
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Debug, PartialEq, Eq, Clone)]
 pub struct Peerstate {
     /// E-mail address of the contact.
     pub addr: String,
@@ -82,13 +80,24 @@ pub struct Peerstate {
     /// Fingerprint of the verified public key.
     pub verified_key_fingerprint: Option<Fingerprint>,
 
+    /// The address that introduced this verified key.
+    pub verifier: Option<String>,
+
+    /// Secondary public verified key of the contact.
+    /// It could be a contact gossiped by another verified contact in a shared group
+    /// or a key that was previously used as a verified key.
+    pub secondary_verified_key: Option<SignedPublicKey>,
+
+    /// Fingerprint of the secondary verified public key.
+    pub secondary_verified_key_fingerprint: Option<Fingerprint>,
+
+    /// The address that introduced secondary verified key.
+    pub secondary_verifier: Option<String>,
+
     /// True if it was detected
     /// that the fingerprint of the key used in chats with
     /// opportunistic encryption was changed after Peerstate creation.
     pub fingerprint_changed: bool,
-
-    /// The address that verified this contact
-    pub verifier: Option<String>,
 }
 
 impl Peerstate {
@@ -106,8 +115,11 @@ impl Peerstate {
             gossip_timestamp: 0,
             verified_key: None,
             verified_key_fingerprint: None,
-            fingerprint_changed: false,
             verifier: None,
+            secondary_verified_key: None,
+            secondary_verified_key_fingerprint: None,
+            secondary_verifier: None,
+            fingerprint_changed: false,
         }
     }
 
@@ -132,8 +144,11 @@ impl Peerstate {
             gossip_timestamp: message_time,
             verified_key: None,
             verified_key_fingerprint: None,
-            fingerprint_changed: false,
             verifier: None,
+            secondary_verified_key: None,
+            secondary_verified_key_fingerprint: None,
+            secondary_verifier: None,
+            fingerprint_changed: false,
         }
     }
 
@@ -141,7 +156,10 @@ impl Peerstate {
     pub async fn from_addr(context: &Context, addr: &str) -> Result<Option<Peerstate>> {
         let query = "SELECT addr, last_seen, last_seen_autocrypt, prefer_encrypted, public_key, \
                      gossip_timestamp, gossip_key, public_key_fingerprint, gossip_key_fingerprint, \
-                     verified_key, verified_key_fingerprint, verifier \
+                     verified_key, verified_key_fingerprint, \
+                     verifier, \
+                     secondary_verified_key, secondary_verified_key_fingerprint, \
+                     secondary_verifier \
                      FROM acpeerstates \
                      WHERE addr=? COLLATE NOCASE LIMIT 1;";
         Self::from_stmt(context, query, (addr,)).await
@@ -154,7 +172,10 @@ impl Peerstate {
     ) -> Result<Option<Peerstate>> {
         let query = "SELECT addr, last_seen, last_seen_autocrypt, prefer_encrypted, public_key, \
                      gossip_timestamp, gossip_key, public_key_fingerprint, gossip_key_fingerprint, \
-                     verified_key, verified_key_fingerprint, verifier \
+                     verified_key, verified_key_fingerprint, \
+                     verifier, \
+                     secondary_verified_key, secondary_verified_key_fingerprint, \
+                     secondary_verifier \
                      FROM acpeerstates  \
                      WHERE public_key_fingerprint=? \
                      OR gossip_key_fingerprint=? \
@@ -174,7 +195,10 @@ impl Peerstate {
     ) -> Result<Option<Peerstate>> {
         let query = "SELECT addr, last_seen, last_seen_autocrypt, prefer_encrypted, public_key, \
                      gossip_timestamp, gossip_key, public_key_fingerprint, gossip_key_fingerprint, \
-                     verified_key, verified_key_fingerprint, verifier \
+                     verified_key, verified_key_fingerprint, \
+                     verifier, \
+                     secondary_verified_key, secondary_verified_key_fingerprint, \
+                     secondary_verifier \
                      FROM acpeerstates  \
                      WHERE verified_key_fingerprint=? \
                      OR addr=? COLLATE NOCASE \
@@ -191,11 +215,6 @@ impl Peerstate {
         let peerstate = context
             .sql
             .query_row_optional(query, params, |row| {
-                // all the above queries start with this: SELECT
-                //   addr, last_seen, last_seen_autocrypt, prefer_encrypted,
-                //   public_key, gossip_timestamp, gossip_key, public_key_fingerprint,
-                //   gossip_key_fingerprint, verified_key, verified_key_fingerprint
-
                 let res = Peerstate {
                     addr: row.get("addr")?,
                     last_seen: row.get("last_seen")?,
@@ -230,11 +249,24 @@ impl Peerstate {
                         .map(|s| s.parse::<Fingerprint>())
                         .transpose()
                         .unwrap_or_default(),
-                    fingerprint_changed: false,
                     verifier: {
                         let verifier: Option<String> = row.get("verifier")?;
-                        verifier.filter(|verifier| !verifier.is_empty())
+                        verifier.filter(|s| !s.is_empty())
                     },
+                    secondary_verified_key: row
+                        .get("secondary_verified_key")
+                        .ok()
+                        .and_then(|blob: Vec<u8>| SignedPublicKey::from_slice(&blob).ok()),
+                    secondary_verified_key_fingerprint: row
+                        .get::<_, Option<String>>("secondary_verified_key_fingerprint")?
+                        .map(|s| s.parse::<Fingerprint>())
+                        .transpose()
+                        .unwrap_or_default(),
+                    secondary_verifier: {
+                        let secondary_verifier: Option<String> = row.get("secondary_verifier")?;
+                        secondary_verifier.filter(|s| !s.is_empty())
+                    },
+                    fingerprint_changed: false,
                 };
 
                 Ok(res)
@@ -421,8 +453,8 @@ impl Peerstate {
     /// Make sure to call `self.save_to_db` to save these changes
     /// Params:
     /// verifier:
-    ///   The address which verifies the given contact
-    ///   If we are verifying the contact, use that contacts address
+    ///   The address which introduces the given contact.
+    ///   If we are verifying the contact, use that contacts address.
     pub fn set_verified(
         &mut self,
         which_key: PeerstateKeyType,
@@ -461,6 +493,19 @@ impl Peerstate {
         }
     }
 
+    /// Sets current gossiped key as the secondary verified key.
+    ///
+    /// If gossiped key is the same as the current verified key,
+    /// do nothing to avoid overwriting secondary verified key
+    /// which may be different.
+    pub fn set_secondary_verified_key_from_gossip(&mut self, verifier: String) {
+        if self.gossip_key_fingerprint != self.verified_key_fingerprint {
+            self.secondary_verified_key = self.gossip_key.clone();
+            self.secondary_verified_key_fingerprint = self.gossip_key_fingerprint.clone();
+            self.secondary_verifier = Some(verifier);
+        }
+    }
+
     /// Saves the peerstate to the database.
     pub async fn save_to_db(&self, sql: &Sql) -> Result<()> {
         sql.execute(
@@ -475,9 +520,12 @@ impl Peerstate {
                 gossip_key_fingerprint,
                 verified_key,
                 verified_key_fingerprint,
-                addr,
-                verifier)
-                VALUES (?,?,?,?,?,?,?,?,?,?,?,?)
+                verifier,
+                secondary_verified_key,
+                secondary_verified_key_fingerprint,
+                secondary_verifier,
+                addr)
+                VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
                 ON CONFLICT (addr)
                 DO UPDATE SET
                   last_seen = excluded.last_seen,
@@ -490,7 +538,10 @@ impl Peerstate {
                   gossip_key_fingerprint = excluded.gossip_key_fingerprint,
                   verified_key = excluded.verified_key,
                   verified_key_fingerprint = excluded.verified_key_fingerprint,
-                  verifier = excluded.verifier",
+                  verifier = excluded.verifier,
+                  secondary_verified_key = excluded.secondary_verified_key,
+                  secondary_verified_key_fingerprint = excluded.secondary_verified_key_fingerprint,
+                  secondary_verifier = excluded.secondary_verifier",
             (
                 self.last_seen,
                 self.last_seen_autocrypt,
@@ -502,23 +553,19 @@ impl Peerstate {
                 self.gossip_key_fingerprint.as_ref().map(|fp| fp.hex()),
                 self.verified_key.as_ref().map(|k| k.to_bytes()),
                 self.verified_key_fingerprint.as_ref().map(|fp| fp.hex()),
-                &self.addr,
                 self.verifier.as_deref().unwrap_or(""),
+                self.secondary_verified_key.as_ref().map(|k| k.to_bytes()),
+                self.secondary_verified_key_fingerprint
+                    .as_ref()
+                    .map(|fp| fp.hex()),
+                self.secondary_verifier.as_deref().unwrap_or(""),
+                &self.addr,
             ),
         )
         .await?;
         Ok(())
     }
 
-    /// Returns true if verified key is contained in the given set of fingerprints.
-    pub fn has_verified_key(&self, fingerprints: &HashSet<Fingerprint>) -> bool {
-        if let Some(vkc) = &self.verified_key_fingerprint {
-            fingerprints.contains(vkc) && self.verified_key.is_some()
-        } else {
-            false
-        }
-    }
-
     /// Returns the address that verified the contact
     pub fn get_verifier(&self) -> Option<&str> {
         self.verifier.as_deref()
@@ -769,8 +816,11 @@ mod tests {
             gossip_key_fingerprint: Some(pub_key.fingerprint()),
             verified_key: Some(pub_key.clone()),
             verified_key_fingerprint: Some(pub_key.fingerprint()),
-            fingerprint_changed: false,
             verifier: None,
+            secondary_verified_key: None,
+            secondary_verified_key_fingerprint: None,
+            secondary_verifier: None,
+            fingerprint_changed: false,
         };
 
         assert!(
@@ -809,8 +859,11 @@ mod tests {
             gossip_key_fingerprint: None,
             verified_key: None,
             verified_key_fingerprint: None,
-            fingerprint_changed: false,
             verifier: None,
+            secondary_verified_key: None,
+            secondary_verified_key_fingerprint: None,
+            secondary_verifier: None,
+            fingerprint_changed: false,
         };
 
         assert!(
@@ -842,8 +895,11 @@ mod tests {
             gossip_key_fingerprint: None,
             verified_key: None,
             verified_key_fingerprint: None,
-            fingerprint_changed: false,
             verifier: None,
+            secondary_verified_key: None,
+            secondary_verified_key_fingerprint: None,
+            secondary_verifier: None,
+            fingerprint_changed: false,
         };
 
         assert!(
@@ -905,8 +961,11 @@ mod tests {
             gossip_key_fingerprint: None,
             verified_key: None,
             verified_key_fingerprint: None,
-            fingerprint_changed: false,
             verifier: None,
+            secondary_verified_key: None,
+            secondary_verified_key_fingerprint: None,
+            secondary_verifier: None,
+            fingerprint_changed: false,
         };
 
         peerstate.apply_header(&header, 100);

src/qr.rs

@@ -1052,8 +1052,11 @@ mod tests {
             gossip_key_fingerprint: None,
             verified_key: None,
             verified_key_fingerprint: None,
-            fingerprint_changed: false,
             verifier: None,
+            secondary_verified_key: None,
+            secondary_verified_key_fingerprint: None,
+            secondary_verifier: None,
+            fingerprint_changed: false,
         };
         assert!(
             peerstate.save_to_db(&ctx.ctx.sql).await.is_ok(),

src/receive_imf.rs

@@ -227,6 +227,9 @@ pub(crate) async fn receive_imf_inner(
         .and_then(|value| mailparse::dateparse(value).ok())
         .map_or(rcvd_timestamp, |value| min(value, rcvd_timestamp + 60));
 
+    let updated_verified_key_addr =
+        update_verified_keys(context, &mut mime_parser, from_id).await?;
+
     // Add parts
     let received_msg = add_parts(
         context,
@@ -278,6 +281,11 @@ pub(crate) async fn receive_imf_inner(
         MsgId::new_unset()
     };
 
+    if let Some(addr) = updated_verified_key_addr {
+        let msg = stock_str::contact_setup_changed(context, &addr).await;
+        chat::add_info_msg(context, chat_id, &msg, received_msg.sort_timestamp).await?;
+    }
+
     save_locations(context, &mime_parser, chat_id, from_id, insert_msg_id).await?;
 
     if let Some(ref sync_items) = mime_parser.sync_items {
@@ -2271,10 +2279,74 @@ enum VerifiedEncryption {
     NotVerified(String), // The string contains the reason why it's not verified
 }
 
+/// Moves secondary verified key to primary verified key
+/// if the message is signed with a secondary verified key.
+/// Removes secondary verified key if the message is signed with primary key.
+///
+/// Returns address of the peerstate if the primary verified key was updated,
+/// the caller then needs to add "Setup changed" notification somewhere.
+async fn update_verified_keys(
+    context: &Context,
+    mimeparser: &mut MimeMessage,
+    from_id: ContactId,
+) -> Result<Option<String>> {
+    if from_id == ContactId::SELF {
+        return Ok(None);
+    }
+
+    if !mimeparser.was_encrypted() {
+        return Ok(None);
+    }
+
+    let Some(peerstate) = &mut mimeparser.decryption_info.peerstate else {
+        // No peerstate means no verified keys.
+        return Ok(None);
+    };
+
+    let signed_with_primary_verified_key = peerstate
+        .verified_key_fingerprint
+        .as_ref()
+        .filter(|fp| mimeparser.signatures.contains(fp))
+        .is_some();
+    let signed_with_secondary_verified_key = peerstate
+        .secondary_verified_key_fingerprint
+        .as_ref()
+        .filter(|fp| mimeparser.signatures.contains(fp))
+        .is_some();
+
+    if signed_with_primary_verified_key {
+        // Remove secondary key if it exists.
+        if peerstate.secondary_verified_key.is_some()
+            || peerstate.secondary_verified_key_fingerprint.is_some()
+            || peerstate.secondary_verifier.is_some()
+        {
+            peerstate.secondary_verified_key = None;
+            peerstate.secondary_verified_key_fingerprint = None;
+            peerstate.secondary_verifier = None;
+            peerstate.save_to_db(&context.sql).await?;
+        }
+
+        // No need to notify about secondary key removal.
+        Ok(None)
+    } else if signed_with_secondary_verified_key {
+        peerstate.verified_key = peerstate.secondary_verified_key.take();
+        peerstate.verified_key_fingerprint = peerstate.secondary_verified_key_fingerprint.take();
+        peerstate.verifier = peerstate.secondary_verifier.take();
+        peerstate.save_to_db(&context.sql).await?;
+
+        // Primary verified key changed.
+        Ok(Some(peerstate.addr.clone()))
+    } else {
+        Ok(None)
+    }
+}
+
 /// Checks whether the message is allowed to appear in a protected chat.
 ///
 /// This means that it is encrypted, signed with a verified key,
 /// and if it's a group, all the recipients are verified.
+///
+/// Also propagates gossiped keys to verified if needed.
 async fn has_verified_encryption(
     context: &Context,
     mimeparser: &MimeMessage,
@@ -2311,7 +2383,13 @@ async fn has_verified_encryption(
             ));
         };
 
-        if !peerstate.has_verified_key(&mimeparser.signatures) {
+        let signed_with_verified_key = peerstate
+            .verified_key_fingerprint
+            .as_ref()
+            .filter(|fp| mimeparser.signatures.contains(fp))
+            .is_some();
+
+        if !signed_with_verified_key {
             return Ok(NotVerified(
                 "The message was sent with non-verified encryption".to_string(),
             ));
@@ -2357,23 +2435,26 @@ async fn has_verified_encryption(
         if mimeparser.gossiped_addr.contains(&to_addr.to_lowercase()) {
             if let Some(mut peerstate) = Peerstate::from_addr(context, &to_addr).await? {
                 // If we're here, we know the gossip key is verified.
-                // Use the gossip-key as verified-key if there is no verified-key
-                // or a member is reintroduced to a verified group.
+                //
+                // Use the gossip-key as verified-key if there is no verified-key.
+                //
+                // Store gossip key as secondary verified key if there is a verified key and
+                // gossiped key is different.
                 //
                 // See <https://github.com/nextleap-project/countermitm/issues/46>
                 // and <https://github.com/deltachat/deltachat-core-rust/issues/4541> for discussion.
                 let verifier_addr = contact.get_addr().to_owned();
-                if !is_verified
-                    || mimeparser
-                        .get_header(HeaderDef::ChatGroupMemberAdded)
-                        .filter(|s| s.as_str() == to_addr)
-                        .is_some()
-                {
+                if !is_verified {
                     info!(context, "{verifier_addr} has verified {to_addr}.");
                     if let Some(fp) = peerstate.gossip_key_fingerprint.clone() {
                         peerstate.set_verified(PeerstateKeyType::GossipKey, fp, verifier_addr)?;
                         peerstate.save_to_db(&context.sql).await?;
                     }
+                } else {
+                    // The contact already has a verified key.
+                    // Store gossiped key as the secondary verified key.
+                    peerstate.set_secondary_verified_key_from_gossip(verifier_addr);
+                    peerstate.save_to_db(&context.sql).await?;
                 }
             }
         }

src/securejoin.rs

@@ -1026,8 +1026,11 @@ mod tests {
             gossip_key_fingerprint: Some(alice_pubkey.fingerprint()),
             verified_key: None,
             verified_key_fingerprint: None,
-            fingerprint_changed: false,
             verifier: None,
+            secondary_verified_key: None,
+            secondary_verified_key_fingerprint: None,
+            secondary_verifier: None,
+            fingerprint_changed: false,
         };
         peerstate.save_to_db(&bob.ctx.sql).await?;
 

src/sql/migrations.rs

@@ -750,6 +750,19 @@ CREATE INDEX smtp_messageid ON imap(rfc724_mid);
         .await?;
     }
 
+    if dbversion < 104 {
+        sql.execute_migration(
+            "ALTER TABLE acpeerstates
+             ADD COLUMN secondary_verified_key;
+             ALTER TABLE acpeerstates
+             ADD COLUMN secondary_verified_key_fingerprint TEXT DEFAULT '';
+             ALTER TABLE acpeerstates
+             ADD COLUMN secondary_verifier TEXT DEFAULT ''",
+            104,
+        )
+        .await?;
+    }
+
     let new_version = sql
         .get_raw_config_int(VERSION_CFG)
         .await?