Improve and speed up autocrypt/pgp gossipping with MDNs

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<bool> {
+        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<i64> = 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",

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<bool> {
+        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(

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<String> {
+        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 {

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?;

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.
 ///

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?;
     }

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?