diff --git a/deltachat-rpc-client/tests/test_securejoin.py b/deltachat-rpc-client/tests/test_securejoin.py index 8b189bfcc..4963f7fed 100644 --- a/deltachat-rpc-client/tests/test_securejoin.py +++ b/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 diff --git a/src/e2ee.rs b/src/e2ee.rs index 9b95c2879..c328a3da0 100644 --- a/src/e2ee.rs +++ b/src/e2ee.rs @@ -100,16 +100,39 @@ impl EncryptHelper { ) -> Result { let mut keyring: Vec = 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)] } diff --git a/src/peerstate.rs b/src/peerstate.rs index 4efd1f96d..75ee3d2ff 100644 --- a/src/peerstate.rs +++ b/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, + /// The address that introduced this verified key. + pub verifier: Option, + + /// 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, + + /// Fingerprint of the secondary verified public key. + pub secondary_verified_key_fingerprint: Option, + + /// The address that introduced secondary verified key. + pub secondary_verifier: Option, + /// 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, } 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> { 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> { 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> { 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::()) .transpose() .unwrap_or_default(), - fingerprint_changed: false, verifier: { let verifier: Option = 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| SignedPublicKey::from_slice(&blob).ok()), + secondary_verified_key_fingerprint: row + .get::<_, Option>("secondary_verified_key_fingerprint")? + .map(|s| s.parse::()) + .transpose() + .unwrap_or_default(), + secondary_verifier: { + let secondary_verifier: Option = 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) -> 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); diff --git a/src/qr.rs b/src/qr.rs index 7d89d3e93..ac53815ec 100644 --- a/src/qr.rs +++ b/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(), diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 86d54914a..bb3371c1c 100644 --- a/src/receive_imf.rs +++ b/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> { + 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 // and 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?; } } } diff --git a/src/securejoin.rs b/src/securejoin.rs index e700f930f..da957a28a 100644 --- a/src/securejoin.rs +++ b/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?; diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index 6e69311c5..2b3ce6000 100644 --- a/src/sql/migrations.rs +++ b/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?