diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 3ee7f35920..7abae86212 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -25,7 +25,7 @@ use crate::e2ee::EncryptHelper; use crate::ensure_and_debug_assert; use crate::ephemeral::Timer as EphemeralTimer; use crate::headerdef::HeaderDef; -use crate::key::{DcKey, SignedPublicKey, self_fingerprint}; +use crate::key::{DcKey, SignedPublicKey, load_self_public_key, self_fingerprint}; use crate::location; use crate::log::warn; use crate::message::{Message, MsgId, Viewtype}; @@ -578,10 +578,12 @@ impl MimeFactory { let timestamp = time(); let addr = contact.get_addr().to_string(); + let mut recipients = vec![addr.clone()]; let encryption_pubkeys = if from_id == ContactId::SELF { Some(Vec::new()) } else if contact.is_key_contact() { if let Some(key) = contact.public_key(context).await? { + recipients = addresses_from_public_key(&key).unwrap_or_else(|| vec![addr.clone()]); Some(vec![(addr.clone(), key)]) } else { Some(Vec::new()) @@ -595,7 +597,7 @@ impl MimeFactory { from_displayname: "".to_string(), sender_displayname: None, selfstatus: "".to_string(), - recipients: vec![addr], + recipients, encryption_pubkeys, to: vec![("".to_string(), contact.get_addr().to_string())], past_members: vec![], @@ -619,11 +621,68 @@ impl MimeFactory { Ok(res) } - fn should_skip_autocrypt(&self) -> bool { - match &self.loaded { - Loaded::Message { .. } => false, - Loaded::Mdn { .. } => true, + /// Returns whether own Autocrypt key should be attached to this MDN + /// and if so, records the attachment. + /// + /// The key is attached to encrypted MDNs + /// once per `gossip_period` for each recipient + /// and immediately when own key gains a newer self-signature, + /// so that contacts we only read messages from + /// still learn our current key and relay list + /// and will likely re-gossip it to group chats. + async fn update_mdn_pubkey_attachment(&self, context: &Context) -> Result { + debug_assert!( + self.encryption_pubkeys + .as_deref() + .is_none_or(|keys| keys.len() <= 1), + "MDNs have at most one recipient key; own key is only added at encryption time" + ); + let Some([(_, key)]) = self.encryption_pubkeys.as_deref() else { + return Ok(false); + }; + let fingerprint = key.dc_fingerprint().hex(); + let self_key_created = load_self_public_key(context) + .await? + .details + .direct_signatures + .iter() + .filter_map(|sig| sig.created()) + .max() + .map_or(0, |created| i64::from(created.as_secs())); + let gossip_period = context.get_config_i64(Config::GossipPeriod).await?; + let now = time(); + let attached_timestamp: Option = context + .sql + .query_get_value( + "SELECT attached_timestamp FROM mdn_autocrypt_timestamp WHERE fingerprint=?", + (&fingerprint,), + ) + .await?; + + // Attach when our key gained a newer self-signature + // (e.g. relay addresses changed) or every `gossip_period`. + // If clocks are skewed, attach always. + let should_attach = attached_timestamp.is_none_or(|attached_timestamp| { + self_key_created > attached_timestamp + || now >= attached_timestamp.saturating_add(gossip_period) + || now < attached_timestamp + }); + if should_attach { + // We don't track or care if the MDN fails to be send or received + // because attaching a potentially fresh key is only best-effort + // and we want to keep the attach-key mechanism simple and localized. + context + .sql + .execute( + "INSERT INTO mdn_autocrypt_timestamp (fingerprint, attached_timestamp) + VALUES (?, ?) + ON CONFLICT (fingerprint) + DO UPDATE SET attached_timestamp=excluded.attached_timestamp", + (&fingerprint, now), + ) + .await?; } + Ok(should_attach) } fn should_attach_profile_data(msg: &Message) -> bool { @@ -945,11 +1004,13 @@ impl MimeFactory { } let grpimage = self.grpimage(); - let skip_autocrypt = self.should_skip_autocrypt(); + let should_attach_pubkey = match &self.loaded { + Loaded::Message { .. } => true, + Loaded::Mdn { .. } => self.update_mdn_pubkey_attachment(context).await?, + }; let encrypt_helper = EncryptHelper::new(context).await?; - if !skip_autocrypt { - // unless determined otherwise we add the Autocrypt header + if should_attach_pubkey { let aheader = encrypt_helper.get_aheader().to_string(); headers.push(( "Autocrypt", diff --git a/src/mimefactory/mimefactory_tests.rs b/src/mimefactory/mimefactory_tests.rs index 32ee79849b..937fdacbd6 100644 --- a/src/mimefactory/mimefactory_tests.rs +++ b/src/mimefactory/mimefactory_tests.rs @@ -14,8 +14,9 @@ use crate::chat::{ }; use crate::chatlist::Chatlist; use crate::constants; -use crate::contact::{Origin, import_vcard}; +use crate::contact::{Origin, import_public_key, import_vcard}; use crate::headerdef::HeaderDef; +use crate::key::{load_self_secret_key, secret_key_to_public_key}; use crate::message; use crate::mimeparser::MimeMessage; use crate::receive_imf::receive_imf; @@ -327,6 +328,87 @@ async fn test_mdn_create_encrypted() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_mdn_sent_to_all_relays() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + let rcvd = tcm.send_recv_accept(bob, alice, "Heyho").await; + + // Bob's key gets a second relay address and Alice merges the newer key. + let bob_secret_key = load_self_secret_key(bob).await?; + let bob_public_key = secret_key_to_public_key( + bob, + bob_secret_key, + u32::try_from(time())? + 100, + "bob@example.net", + "bob@example.net,bob@relay2.example", + )?; + import_public_key(alice, &bob_public_key).await?; + + let mimefactory = MimeFactory::from_mdn(alice, rcvd.from_id, rcvd.rfc724_mid, vec![]).await?; + let mut recipients = mimefactory.recipients(); + recipients.sort(); + assert_eq!(recipients, vec!["bob@example.net", "bob@relay2.example"]); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_mdn_autocrypt_throttle() -> Result<()> { + async fn mdn_has_aheader( + bob: &TestContext, + alice: &TestContext, + rcvd: &Message, + ) -> Result { + let mf = MimeFactory::from_mdn(bob, rcvd.from_id, rcvd.rfc724_mid.clone(), vec![]).await?; + let rendered_msg = mf.render(bob).await?; + let mime = MimeMessage::from_bytes(alice, rendered_msg.message.as_bytes()).await?; + Ok(mime.autocrypt_fingerprint.is_some()) + } + + let mut tcm = TestContextManager::new(); + let alice = tcm.alice().await; + let bob = tcm.bob().await; + bob.set_config_bool(Config::MdnsEnabled, true).await?; + + let rcvd = tcm.send_recv_accept(&alice, &bob, "Heyho").await; + message::markseen_msgs(&bob, vec![rcvd.id]).await?; + + assert!(mdn_has_aheader(&bob, &alice, &rcvd).await?); + assert!(!mdn_has_aheader(&bob, &alice, &rcvd).await?); + + // Own key change forces the header: + // a relay list change bumps the transports timestamp + // which becomes the key signature timestamp, + // so drop the cached self key to re-derive it. + SystemTime::shift(Duration::from_secs(100)); + bob.sql + .execute("UPDATE transports SET add_timestamp=?", (time(),)) + .await?; + *bob.self_public_key.lock().await = None; + assert!(mdn_has_aheader(&bob, &alice, &rcvd).await?); + assert!(!mdn_has_aheader(&bob, &alice, &rcvd).await?); + + // A stored timestamp from the future is ignored + // and replaced by one from the current clock. + bob.sql + .execute( + "UPDATE mdn_autocrypt_timestamp SET attached_timestamp=?", + (time() + 1000,), + ) + .await?; + assert!(mdn_has_aheader(&bob, &alice, &rcvd).await?); + assert!(!mdn_has_aheader(&bob, &alice, &rcvd).await?); + + let gossip_period = bob.get_config_i64(Config::GossipPeriod).await?; + SystemTime::shift(Duration::from_secs(gossip_period.try_into()?)); + assert!(mdn_has_aheader(&bob, &alice, &rcvd).await?); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_subject_in_group() -> Result<()> { async fn send_msg_get_subject( diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 4014354238..654b3df20e 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -1027,6 +1027,24 @@ impl MimeMessage { self.signature.is_some() } + /// Returns the fingerprints of all keys distributed by this message: + /// - keys from Autocrypt-Gossip headers + /// - the key from the sender's Autocrypt header ("self-gossip") + /// + /// Nothing is returned unless the message was correctly encrypted. + pub(crate) fn distributed_key_fingerprints(&self) -> Vec { + let sender_fingerprint = if self.was_encrypted() { + self.autocrypt_fingerprint.clone() + } else { + None + }; + self.gossiped_keys + .values() + .map(|gossiped_key| gossiped_key.public_key.dc_fingerprint().hex()) + .chain(sender_fingerprint) + .collect() + } + /// Returns whether the email contains a `chat-version` header. /// This indicates that the email is a DC-email. pub(crate) fn has_chat_version(&self) -> bool { diff --git a/src/receive_imf.rs b/src/receive_imf.rs index c04592881c..72ced0958a 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -755,24 +755,26 @@ pub(crate) async fn receive_imf_inner( contact::update_last_seen(context, from_id, mime_parser.timestamp_sent).await?; } - // Update gossiped timestamp for the chat if someone else or our other device sent - // Autocrypt-Gossip header to avoid sending Autocrypt-Gossip ourselves - // and waste traffic. + // Update gossiped timestamp for the chat if someone else or our other device + // distributed keys to this chat, via Autocrypt-Gossip headers + // or the sender's own Autocrypt header which is a kind of self-gossip. let chat_id = received_msg.chat_id; if !chat_id.is_special() { - for gossiped_key in mime_parser.gossiped_keys.values() { + let fingerprints = mime_parser.distributed_key_fingerprints(); + if !fingerprints.is_empty() { + let timestamp_sent = mime_parser.timestamp_sent; context .sql .transaction(move |transaction| { - let fingerprint = gossiped_key.public_key.dc_fingerprint().hex(); - transaction.execute( + let mut stmt = transaction.prepare( "INSERT INTO gossip_timestamp (chat_id, fingerprint, timestamp) VALUES (?, ?, ?) ON CONFLICT (chat_id, fingerprint) DO UPDATE SET timestamp=MAX(timestamp, excluded.timestamp)", - (chat_id, &fingerprint, mime_parser.timestamp_sent), )?; - + for fingerprint in &fingerprints { + stmt.execute((chat_id, fingerprint, timestamp_sent))?; + } Ok(()) }) .await?; diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index e6d1c0f374..36597f08ea 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -5503,6 +5503,35 @@ async fn test_group_introduction_no_gossip() -> Result<()> { Ok(()) } +/// Tests that the sender's own Autocrypt header counts like received gossip: +/// members do not re-gossip a key that its owner just distributed themselves. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_autocrypt_header_suppresses_gossip() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let fiona = &tcm.fiona().await; + + let alice_chat_id = alice + .create_group_with_members("Group", &[bob, fiona]) + .await; + let sent = alice.send_text(alice_chat_id, "Hello group").await; + + // Alice's first message gossips the other members' keys. + let msg = bob.recv_msg(&sent).await; + assert!(!bob.parse_msg(&sent).await.gossiped_keys.is_empty()); + + // Bob got Alice's key from her Autocrypt header + // and the other members' keys from her gossip, + // so Bob has nothing left to gossip. + let bob_chat_id = msg.chat_id; + bob_chat_id.accept(bob).await?; + let sent = bob.send_text(bob_chat_id, "Hello back").await; + assert!(fiona.parse_msg(&sent).await.gossiped_keys.is_empty()); + + Ok(()) +} + /// Tests reception of an encrypted group message /// without Chat-Group-ID. /// diff --git a/src/smtp.rs b/src/smtp.rs index 9c29fb7e55..0655c761a1 100644 --- a/src/smtp.rs +++ b/src/smtp.rs @@ -596,13 +596,14 @@ async fn send_mdn_rfc724_mid( ) .await?; let encrypted = mimefactory.will_be_encrypted(); + let mut recipients = if contact_id == ContactId::SELF { + Vec::new() + } else { + mimefactory.recipients() + }; let rendered_msg = Box::pin(mimefactory.render(context)).await?; let body = rendered_msg.message; - let mut recipients = Vec::new(); - if contact_id != ContactId::SELF { - recipients.push(contact.get_addr().to_string()); - } if context.get_config_bool(Config::BccSelf).await? { add_self_recipients(context, &mut recipients, encrypted).await?; } diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index e0304bf70e..f435cc2dba 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -2520,6 +2520,20 @@ UPDATE msgs SET state=24 WHERE state=18; -- Change OutPreparing to OutFailed. .await?; } + inc_and_check(&mut migration_version, 160)?; + if dbversion < migration_version { + // Tracks when own key was last attached to an MDN + // so it is not attached to every MDN. + sql.execute_migration( + "CREATE TABLE mdn_autocrypt_timestamp ( + fingerprint TEXT PRIMARY KEY NOT NULL, -- Upper-case fingerprint of the recipient key. + attached_timestamp INTEGER NOT NULL + ) STRICT", + migration_version, + ) + .await?; + } + let new_version = sql .get_raw_config_int(VERSION_CFG) .await?