From 880676254aec0d52dcb195c623236a0a7f743d4f Mon Sep 17 00:00:00 2001 From: holger krekel Date: Sat, 29 Aug 2026 20:36:13 +0200 Subject: [PATCH 1/2] feat: introduce keyupdate messages informing contacts about relay changes When the published relay list changes, key-contacts are informed with an unsigned message carrying the re-signed key, encrypted to a chunk of contacts at a time. It is shaped like a receipt notification naming no message, so that cores which know nothing about keyupdates trash it as well. See the src/keyupdate.rs module docs for the design. --- deltachat-rpc-client/tests/test_cross_core.py | 44 ++ src/config.rs | 7 + src/context.rs | 12 +- src/context/context_tests.rs | 1 + src/keyupdate.rs | 206 ++++++++++ src/keyupdate/keyupdate_tests.rs | 389 ++++++++++++++++++ src/lib.rs | 1 + src/mimefactory.rs | 73 +++- src/receive_imf.rs | 1 + src/scheduler.rs | 27 +- src/securejoin.rs | 5 +- src/securejoin/bob.rs | 5 +- src/smtp.rs | 7 +- src/sql/migrations.rs | 13 + src/sql/migrations/migrations_tests.rs | 24 ++ src/test_utils.rs | 30 +- src/transport.rs | 11 +- 17 files changed, 830 insertions(+), 26 deletions(-) create mode 100644 src/keyupdate.rs create mode 100644 src/keyupdate/keyupdate_tests.rs diff --git a/deltachat-rpc-client/tests/test_cross_core.py b/deltachat-rpc-client/tests/test_cross_core.py index ac528a7f60..e0ae356680 100644 --- a/deltachat-rpc-client/tests/test_cross_core.py +++ b/deltachat-rpc-client/tests/test_cross_core.py @@ -1,4 +1,5 @@ import subprocess +import time import pytest @@ -86,3 +87,46 @@ def test_second_device(acf, alice_and_remote_bob) -> None: remote_eval("locals()['future']()") assert new_account.get_config("addr") == remote_eval("bob.get_config('addr')") + + +def test_keyupdate_against_core_2_48_march_2026(acf, alice_and_remote_bob): + """Test 2.48 Bob learns a new relay of Alice from a keyupdate, and is shown nothing.""" + alice, alice_contact_bob, remote_eval = alice_and_remote_bob("2.48.0") + + def bob_sees(): + return remote_eval( + "{'chats': len(bob.get_chatlist())," + " 'fresh': len(bob._rpc.get_fresh_msgs(bob.id))," + " 'contacts': len(bob.get_contacts())," + " 'alice_chat': bob._rpc.get_chat_id_by_contact_id(bob.id, bob_contact_alice.id) or 0}", + ) + + # Keyupdates go to contacts who plausibly hold our key: + # an accepted chat alone is not enough, a message must have flowed. + alice_chat = alice_contact_bob.create_chat() + alice.set_config("keyupdate_debounce", "1") + old_addr = alice.get_config("configured_addr") + alice_chat.send_text("hi") + assert remote_eval("bob.wait_for_incoming_msg().get_snapshot().text") == "hi" + before = bob_sees() + + # Certificate merging keeps the newest direct key signature, + # and signature timestamps have one-second resolution: + # without waiting, the re-signed key can tie with the copy Bob holds, keeping his. + time.sleep(2) + alice.add_transport_from_qr(acf.get_account_qr()) + alice.bring_online() + (new_addr,) = [t["addr"] for t in alice.list_transports() if t["addr"] != old_addr] + + # The 2.48 core has no encryption enforcement, but the keyupdate MDN without + # referenced message keeps it invisible; merging happens before the trashing. + for _ in range(60): + if new_addr in remote_eval("bob_contact_alice.get_encryption_info()"): + break + time.sleep(1) + else: + pytest.fail("Bob never received the keyupdate") + + # It also leaves no trace: no chat with Alice, no message anywhere, + # and no address-contact for the address it was sent from. + assert bob_sees() == before diff --git a/src/config.rs b/src/config.rs index 0dad28570b..3dce2bfdde 100644 --- a/src/config.rs +++ b/src/config.rs @@ -361,6 +361,13 @@ pub enum Config { /// Whether automatic relay management successfully added the desired number of relays AutorelayFinished, + /// Sorted, space-separated relay list for which no keyupdate is due. + KeyupdateBaseline, + + /// For tests only: keyupdate debounce window in seconds. + #[strum(props(default = "30"))] + KeyupdateDebounce, + /// Whether to avoid using IMAP IDLE even if the server supports it. /// /// This is a developer option for testing "fake idle". diff --git a/src/context.rs b/src/context.rs index fdac5a6ae2..04034afa80 100644 --- a/src/context.rs +++ b/src/context.rs @@ -4,7 +4,7 @@ use std::collections::{BTreeMap, HashMap}; use std::ffi::OsString; use std::ops::Deref; use std::path::{Path, PathBuf}; -use std::sync::atomic::AtomicBool; +use std::sync::atomic::{AtomicBool, AtomicI64}; use std::sync::{Arc, OnceLock, Weak}; use std::time::Duration; @@ -330,6 +330,9 @@ pub struct InnerContext { /// `Connectivity` values for published relays, unordered. Used to compute the aggregate connectivity, /// see [`Context::get_connectivity()`]. pub(crate) published_connectivities: parking_lot::Mutex>, + + /// Timestamp after which the SMTP loop checks for a keyupdate to send, or 0 if none is due. + pub(crate) next_keyupdate_check: AtomicI64, } /// The state of ongoing process. @@ -506,6 +509,7 @@ impl Context { self_fingerprint: OnceLock::new(), self_public_key: Mutex::new(None), published_connectivities: parking_lot::Mutex::new(Vec::new()), + next_keyupdate_check: AtomicI64::new(0), }; let ctx = Context { @@ -1062,6 +1066,12 @@ impl Context { .await? .to_string(), ); + res.insert( + "keyupdate_debounce", + self.get_config_int(Config::KeyupdateDebounce) + .await? + .to_string(), + ); let elapsed = time_elapsed(&self.creation_time); res.insert("uptime", duration_to_str(elapsed)); diff --git a/src/context/context_tests.rs b/src/context/context_tests.rs index c6c5e33c24..db5056e4e2 100644 --- a/src/context/context_tests.rs +++ b/src/context/context_tests.rs @@ -298,6 +298,7 @@ async fn test_get_info_completeness() { "stats_last_update", "stats_last_old_contact_id", "simulate_receive_imf_error", // only used in tests + "keyupdate_baseline", // Our own addresses, don't leak them to the logs. ]; let t = TestContext::new().await; let info = t.get_info().await.unwrap(); diff --git a/src/keyupdate.rs b/src/keyupdate.rs new file mode 100644 index 0000000000..13b250b3da --- /dev/null +++ b/src/keyupdate.rs @@ -0,0 +1,206 @@ +//! # Keyupdate messages. +//! +//! Contacts learn our relay list from the key our messages carry, +//! so after a relay change a "mutually silent" contact may keep writing +//! to relays we no longer read. A keyupdate tells them proactively, +//! carrying the re-signed key to [`KEYUPDATE_CHUNK_CONTACTS`] contacts at a time. +//! +//! Keyupdates are *unsigned*, because a signature +//! carries one intended recipient fingerprint per recipient, +//! which would tell everyone in the chunk who the others are. +//! Receivers still learn the key, as the Autocrypt header is merged +//! before any signature is checked, and trash the message itself. +//! +//! Nothing authenticates the sender and nothing needs to: +//! certificate merging verifies the relay list in the direct key signature +//! and keeps the newest one, which also defeats replaying an old keyupdate. +//! +//! Recipients ([`keyupdate_recipients`]) are the key-contacts who can +//! plausibly hold our key, aged out after [`KEYUPDATE_MAX_SILENCE`] and capped +//! at [`KEYUPDATE_MAX_RECIPIENTS`] so one relay change cannot cause unbounded traffic. +//! +//! A transport change only schedules a check, +//! debouncing several changes into one message, +//! which the SMTP loop sends once its queue is drained. +//! Whether anything is due is a diff against [`Config::KeyupdateBaseline`]: +//! a migration seeds it so upgrading sends nothing, and a device ingesting a sync +//! records the new list instead of sending ([`set_current_relays_as_keyupdate_baseline`]). + +use std::collections::BTreeSet; +use std::sync::atomic::Ordering; + +use anyhow::Result; +use deltachat_contact_tools::addr_normalize; +use rand::seq::SliceRandom; + +use crate::chat::ChatId; +use crate::config::Config; +use crate::constants::Chattype; +use crate::contact::ContactId; +use crate::context::Context; +use crate::key::{DcKey, SignedPublicKey}; +use crate::log::warn; +use crate::mimefactory::render_keyupdate_message; +use crate::pgp::{pubkey_can_encrypt, relay_addrs}; +use crate::smtp::insert_into_smtp; +use crate::tools::{create_outgoing_rfc724_mid, time}; + +/// Maximum number of contacts one (chunk of a) keyupdate message is encrypted to. +const KEYUPDATE_CHUNK_CONTACTS: usize = 200; + +/// How long a contact may show no sign of life before keyupdates skip them. +const KEYUPDATE_MAX_SILENCE: i64 = 3 * 365 * 24 * 3600; + +/// Upper bound on the contacts informed after a relay list change, keeping the freshest. +const KEYUPDATE_MAX_RECIPIENTS: usize = 5000; + +/// A contact to inform: the relays to reach them at, and the key to encrypt to. +struct KeyupdateRecipient { + relays: Vec, + public_key: SignedPublicKey, +} + +/// Returns at most `max_recipients` key-contacts to inform. +async fn keyupdate_recipients( + context: &Context, + max_recipients: usize, +) -> Result> { + // Single chat contacts only become keyupdate recipient candidates + // if we have a record of a sent message or `last_seen` is not 0. + // Ephemeral expiry and `delete_device_after` trash messages and drop their `from_id`. + // The ephemeral timer change message is exempt and usually carries such a chat, + // but `delete_device_after` has no equivalent: + // if we only ever sent messages, now removed, and never received one, + // the contact does not qualify as a keyupdate recipient. + let alive_since = time().saturating_sub(KEYUPDATE_MAX_SILENCE); + let rows = context + .sql + .query_map_vec( + // The outer single-argument MAX aggregates over the contact's chats, + // the inner multi-argument one picks the newest signal per chat. + "SELECT c.addr, k.public_key, + MAX(MAX(c.last_seen, + CASE WHEN ch.type=? THEN 0 ELSE ch.created_timestamp END, + cc.add_timestamp, + CASE WHEN ch.type=? THEN IFNULL( + (SELECT MAX(m.timestamp) FROM msgs m + WHERE m.chat_id=ch.id AND m.from_id=?), 0) + ELSE 0 END)) AS freshness + FROM contacts c + INNER JOIN public_keys k ON k.fingerprint=c.fingerprint + INNER JOIN chats_contacts cc ON cc.contact_id=c.id + INNER JOIN chats ch ON ch.id=cc.chat_id + WHERE c.id>? AND c.fingerprint<>'' AND c.blocked=0 + AND cc.add_timestamp >= cc.remove_timestamp + AND ch.id>? AND ch.type IN (?, ?, ?) AND ch.blocked=0 + GROUP BY c.id + HAVING freshness>? + ORDER BY freshness DESC + LIMIT ?", + ( + Chattype::Single, + Chattype::Single, + ContactId::SELF, + ContactId::LAST_SPECIAL, + ChatId::LAST_SPECIAL, + Chattype::Single, + Chattype::Group, + Chattype::InBroadcast, + alive_since, + max_recipients, + ), + |row| { + let addr: String = row.get(0)?; + let public_key_bytes: Vec = row.get(1)?; + Ok((addr, public_key_bytes)) + }, + ) + .await?; + + let mut recipients = Vec::with_capacity(rows.len()); + for (addr, public_key_bytes) in rows { + let public_key = match SignedPublicKey::from_slice(&public_key_bytes) { + Ok(public_key) => public_key, + Err(err) => { + warn!(context, "Cannot parse stored key for {addr:?}: {err:#}."); + continue; + } + }; + if !pubkey_can_encrypt(&public_key) { + warn!(context, "Stored key for {addr:?} cannot be encrypted to."); + continue; + } + let relays = relay_addrs(&public_key, &addr); + debug_assert!(relays.iter().all(|relay| !relay.is_empty())); + if !relays.is_empty() { + recipients.push(KeyupdateRecipient { relays, public_key }); + } + } + + // Freshness decides who is informed at all, but it must not decide who shares a chunk: + // an envelope would otherwise group contacts by how active they are. + recipients.shuffle(&mut rand::rng()); + Ok(recipients) +} + +/// Returns the deduplicated relay addresses to put into the SMTP envelope +/// for the keyupdate encrypted to a `chunk` of recipients. +fn envelope_recipients(chunk: &[KeyupdateRecipient]) -> String { + let mut addrs = BTreeSet::new(); + for recipient in chunk { + for relay in &recipient.relays { + addrs.insert(addr_normalize(relay)); + } + } + Vec::from_iter(addrs).join(" ") +} + +/// Returns the published relay list in the format stored in [`Config::KeyupdateBaseline`]. +async fn published_relays_joined(context: &Context) -> Result { + let mut relays = context.get_published_self_addrs().await?; + relays.sort(); + Ok(relays.join(" ")) +} + +/// Schedules a check for whether a keyupdate needs sending, after the debounce period. +pub(crate) async fn schedule_keyupdate_check(context: &Context) -> Result<()> { + let debounce = context.get_config_i64(Config::KeyupdateDebounce).await?; + context + .next_keyupdate_check + .store(time().saturating_add(debounce), Ordering::Relaxed); + Ok(()) +} + +/// Records the currently published relay list as not needing a keyupdate, see the module docs. +pub(crate) async fn set_current_relays_as_keyupdate_baseline(context: &Context) -> Result<()> { + let current = published_relays_joined(context).await?; + context + .set_config_internal(Config::KeyupdateBaseline, Some(¤t)) + .await +} + +/// Sends a keyupdate message if the published relay list differs from the recorded baseline. +pub(crate) async fn maybe_send_keyupdate_message(context: &Context) -> Result<()> { + let current = published_relays_joined(context).await?; + let last = context.get_config(Config::KeyupdateBaseline).await?; + if last.unwrap_or_default() == current { + return Ok(()); + } + + let recipients = keyupdate_recipients(context, KEYUPDATE_MAX_RECIPIENTS).await?; + for chunk in recipients.chunks(KEYUPDATE_CHUNK_CONTACTS) { + let envelope = envelope_recipients(chunk); + let rfc724_mid = create_outgoing_rfc724_mid(); + let keys = chunk.iter().map(|r| r.public_key.clone()).collect(); + let rendered_message = render_keyupdate_message(context, &rfc724_mid, keys).await?; + insert_into_smtp(context, &rfc724_mid, &envelope, rendered_message).await?; + } + + // Record only after queueing, so failed queueing is retried by a later check. + context + .set_config_internal(Config::KeyupdateBaseline, Some(¤t)) + .await +} + +#[cfg(test)] +mod keyupdate_tests; diff --git a/src/keyupdate/keyupdate_tests.rs b/src/keyupdate/keyupdate_tests.rs new file mode 100644 index 0000000000..8ca78a428a --- /dev/null +++ b/src/keyupdate/keyupdate_tests.rs @@ -0,0 +1,389 @@ +use std::collections::BTreeSet; +use std::num::NonZero; +use std::time::Duration; + +use deltachat_contact_tools::EmailAddress; +use pgp::composed::{Esk, Message}; +use pgp::packet::PublicKeyEncryptedSessionKey; + +use super::*; +use crate::chat::{ + ChatId, add_to_chat_contacts_table, create_broadcast, create_group, + remove_from_chat_contacts_table, +}; +use crate::constants::Blocked; +use crate::contact::{Contact, Origin, import_public_key, update_last_seen}; +use crate::decrypt::{decrypt, get_encrypted_pgp_message_boxed}; +use crate::ephemeral::{Timer, delete_expired_messages}; +use crate::pgp::{addresses_from_public_key, create_keypair}; +use crate::securejoin::get_securejoin_qr; +use crate::test_utils::{TestContext, TestContextManager}; +use crate::tools::SystemTime; +use crate::transport::send_sync_transports; + +/// Asserts that no session key packet names the recipient it is meant for. +fn assert_anonymous_recipients(payload: &str, expected: usize) { + let mail = mailparse::parse_mail(payload.as_bytes()).unwrap(); + let msg = get_encrypted_pgp_message_boxed(&mail).unwrap().unwrap(); + let Message::Encrypted { esk, .. } = &*msg else { + panic!("Expected encrypted message"); + }; + assert_eq!(esk.len(), expected); + for encrypted_session_key in esk { + let Esk::PublicKeyEncryptedSessionKey(pkesk) = encrypted_session_key else { + panic!("Expected asymmetric encryption"); + }; + match pkesk { + PublicKeyEncryptedSessionKey::V3 { id, .. } => assert!(id.is_wildcard()), + PublicKeyEncryptedSessionKey::V6 { fingerprint, .. } => assert!(fingerprint.is_none()), + PublicKeyEncryptedSessionKey::Other { .. } => unreachable!(), + } + } +} + +/// Creates a single chat with `other` and writes to it. +async fn send_text_message(context: &TestContext, other: &TestContext) { + let chat_id = context.create_chat_id(other).await; + context.send_text(chat_id, "hi").await; +} + +/// Adds a key-contact for `addr`, with a fresh key of its own, +/// or with a fingerprint whose certificate is not stored. +async fn add_key_contact(context: &Context, addr: &str, with_key: bool) -> Result { + let public_key = create_keypair(EmailAddress::new(addr)?)?.to_public_key(); + if with_key { + import_public_key(context, &public_key).await?; + } + let fingerprint = public_key.dc_fingerprint().hex(); + let (contact_id, _modifier) = + Contact::add_or_lookup_ext(context, "", addr, &fingerprint, Origin::ManuallyCreated) + .await?; + Ok(contact_id) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_keyupdate_recipients() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob_1to1 = &tcm.bob().await; + let charlie_in_group = &tcm.charlie().await; + let dom_subscriber = &tcm.dom().await; + let elena_blocked = &tcm.elena().await; + let fiona_unaccepted = &tcm.fiona().await; + let pqc_broadcaster = &tcm.pqc().await; + + send_text_message(alice, bob_1to1).await; + let group = alice + .create_group_with_members("Group", &[charlie_in_group, elena_blocked]) + .await; + + // The group chat stays accepted, so only the contact-level block excludes elena. + Contact::block(alice, alice.add_or_lookup_contact_id(elena_blocked).await).await?; + + // Dom joins our channel the way a subscriber does. + // This leaves no single chat on our side, + // because only a setup-contact QR creates one, + // so subscribing alone does not make someone a keyupdate recipient. + let own_channel = create_broadcast(alice, "Channel".to_string()).await?; + let qr = get_securejoin_qr(alice, Some(own_channel)).await?; + tcm.exec_securejoin_qr(dom_subscriber, alice, &qr).await; + + // A channel we follow contains exactly its owner (`InBroadcast`). + let followed_channel = ChatId::create_multiuser_record( + alice, + Chattype::InBroadcast, + "grpid", + "Followed channel", + Blocked::Not, + None, + time(), + ) + .await?; + let pqc_id = alice.add_or_lookup_contact_id(pqc_broadcaster).await; + add_to_chat_contacts_table(alice, time(), followed_channel, &[pqc_id]).await?; + + // Add to, and remove fiona from, the group, keeping her unaccepted. + tcm.send_recv(fiona_unaccepted, alice, "hi").await; + let fiona_id = alice.add_or_lookup_contact_id(fiona_unaccepted).await; + add_to_chat_contacts_table(alice, time(), group, &[fiona_id]).await?; + remove_from_chat_contacts_table(alice, group, fiona_id).await?; + + // A key-contact whose certificate we do not store cannot be encrypted to. + let with_key = false; + let keyless = add_key_contact(alice, "keyless@example.net", with_key).await?; + ChatId::create_for_contact(alice, keyless).await?; + + let no_msgs_contact = add_key_contact(alice, "no_msgs@example.net", true).await?; + ChatId::create_for_contact(alice, no_msgs_contact).await?; + + // Blocked, unaccepted, keyless or message-less contacts are left out. + let mut recipients: Vec = keyupdate_recipients(alice, KEYUPDATE_MAX_RECIPIENTS) + .await? + .into_iter() + .flat_map(|recipient| recipient.relays) + .collect(); + recipients.sort(); + assert_eq!( + recipients, + ["bob@example.net", "charlie@example.net", "pqc@example.org"] + ); + + Ok(()) +} + +/// Tests that contacts falling silent drop out of the recipient set, +/// that any sign of life brings them back, and that the cap keeps the freshest. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_keyupdate_recipients_freshness() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob_written_to = &tcm.bob().await; + let charlie_heard_from = &tcm.charlie().await; + + send_text_message(alice, bob_written_to).await; + alice + .create_group_with_members("Group", &[charlie_heard_from]) + .await; + let charlie_id = alice.add_or_lookup_contact_id(charlie_heard_from).await; + let max = KEYUPDATE_MAX_RECIPIENTS; + assert_eq!(keyupdate_recipients(alice, max).await?.len(), 2); + + // Contacts that showed no sign of life for years are left out. + SystemTime::shift(Duration::from_secs(KEYUPDATE_MAX_SILENCE as u64 + 1)); + assert!(keyupdate_recipients(alice, max).await?.is_empty()); + + send_text_message(alice, bob_written_to).await; + update_last_seen(alice, charlie_id, time().saturating_sub(1000)).await?; + assert_eq!(keyupdate_recipients(alice, max).await?.len(), 2); + + // Beyond the cap the freshest contacts are kept. + let capped = keyupdate_recipients(alice, 1).await?; + assert_eq!(capped.len(), 1); + assert_eq!(capped[0].relays, ["bob@example.net"]); + + Ok(()) +} + +/// Tests that a contact in an ephemeral single chat +/// stays a recipient once the written messages expired. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_keyupdate_recipients_ephemeral() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + let chat_id = alice.create_chat_id(bob).await; + let duration = NonZero::new(60).unwrap(); + chat_id + .set_ephemeral_timer(alice, Timer::Enabled { duration }) + .await?; + alice.send_text(chat_id, "hi").await; + + let max = KEYUPDATE_MAX_RECIPIENTS; + assert_eq!(keyupdate_recipients(alice, max).await?.len(), 1); + + let msg_cnt = chat_id.get_msg_cnt(alice).await?; + SystemTime::shift(Duration::from_secs(61)); + delete_expired_messages(alice, time()).await?; + assert_eq!(chat_id.get_msg_cnt(alice).await?, msg_cnt - 1); + assert_eq!(keyupdate_recipients(alice, max).await?.len(), 1); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_send_and_receive_keyupdate() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + send_text_message(alice, bob).await; + let alice_contact = bob.add_or_lookup_contact(alice).await; + assert_eq!(alice_contact.last_seen(), 0); + // Create Bob's chat before the transport change: creating it later + // would re-import Alice's current key and hide a failing keyupdate. + let bob_chat_id = bob.create_chat_id(alice).await; + + // Merely created chats without any message cause no keyupdate recipient. + let no_msgs_contact = add_key_contact(alice, "no_msgs@example.net", true).await?; + ChatId::create_for_contact(alice, no_msgs_contact).await?; + + alice.add_transport("alice@relay.example.net").await; + maybe_send_keyupdate_message(alice).await?; + let keyupdate = alice.pop_sent_msg().await; + assert_eq!(keyupdate.recipients, "bob@example.net"); + assert!(keyupdate.payload.contains("Subject: [...]")); + + // A keyupdate is trashed on old cores because it's unsigned MDN + // without a message reference. See also cross-core Python tests. + let mail = mailparse::parse_mail(keyupdate.payload.as_bytes())?; + let (mut decrypted, _fingerprint) = decrypt(bob, &mail).await?.unwrap(); + // The next line is important: A key update message must NOT be signed, + // as the signature might contain intended recipient fingerprints, + // leaking all of the sender's contacts to all the other contacts. + assert!(!decrypted.is_signed()); + let plain = String::from_utf8(decrypted.as_data_vec()?)?; + assert!(plain.contains("multipart/report; report-type=disposition-notification")); + assert!(!plain.contains("Original-Message-ID")); + + bob.recv_msg_trash(&keyupdate).await; + // No green "online" dot, unlike for a regular message, see `test_last_seen()`. + let alice_contact = Contact::get_by_id(bob, alice_contact.id).await?; + assert_eq!(alice_contact.last_seen(), 0); + + let bob_message = bob.send_text(bob_chat_id, "hi").await; + assert!(bob_message.recipients.contains("alice@relay.example.net")); + + // The removal direction: unpublishing the relay sends a keyupdate + // whose list no longer contains it, so Bob stops sending there. + // The time shift gives the re-signed key a later signature timestamp, + // so that certificate merging prefers the removal. + SystemTime::shift(Duration::from_secs(2)); + alice + .set_transport_unpublished("alice@relay.example.net", true) + .await?; + maybe_send_keyupdate_message(alice).await?; + bob.recv_msg_trash(&alice.pop_sent_msg().await).await; + let bob_message = bob.send_text(bob_chat_id, "hi again").await; + assert!(!bob_message.recipients.contains("alice@relay.example.net")); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_keyupdate_trigger_dedup() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + // No contacts yet, so this only records the baseline. + maybe_send_keyupdate_message(alice).await?; + assert!(alice.pop_sent_msg_opt().await.is_none()); + + send_text_message(alice, bob).await; + maybe_send_keyupdate_message(alice).await?; + assert!(alice.pop_sent_msg_opt().await.is_none()); + + let debounce = 60; + alice + .set_config(Config::KeyupdateDebounce, Some(&debounce.to_string())) + .await?; + let before = time(); + alice.add_transport("alice@relay.example.net").await; + send_sync_transports(alice).await?; + let next_check = alice.next_keyupdate_check.load(Ordering::Relaxed); + assert!(next_check >= before.saturating_add(debounce)); + assert!(next_check <= time().saturating_add(debounce)); + + // Sending is driven by the changed relay list, not by the scheduled check. + maybe_send_keyupdate_message(alice).await?; + assert!(alice.pop_sent_msg_opt().await.is_some()); + assert!(alice.pop_sent_msg_opt().await.is_none()); + maybe_send_keyupdate_message(alice).await?; + assert!(alice.pop_sent_msg_opt().await.is_none()); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_keyupdate_not_sent_by_synced_device() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let alice2 = &tcm.alice().await; + let bob = &tcm.bob().await; + for a in [alice, alice2] { + a.set_config_bool(Config::SyncMsgs, true).await?; + a.set_config_bool(Config::BccSelf, true).await?; + // Both devices need a recipient, otherwise silence proves nothing. + send_text_message(a, bob).await; + } + + alice.add_transport("alice@relay.example.net").await; + send_sync_transports(alice).await?; + alice.send_sync_msg().await?; + alice2.recv_msg_trash(&alice.pop_sent_msg().await).await; + // The sync was applied, so silence below is meaningful. + let published = alice2.get_published_self_addrs().await?; + assert!(published.contains(&"alice@relay.example.net".to_string())); + + maybe_send_keyupdate_message(alice2).await?; + assert!(alice2.pop_sent_msg_opt().await.is_none()); + maybe_send_keyupdate_message(alice).await?; + assert_eq!(alice.pop_sent_msg().await.recipients, "bob@example.net"); + + Ok(()) +} + +/// Tests that more contacts than fit into one message are sent in chunks, +/// each chunk being a message of its own with its own recipients. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_keyupdate_chunks() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + + let chat_id = create_group(alice, "Group").await?; + let with_key = true; + let mut contact_ids = Vec::new(); + for i in 0..=KEYUPDATE_CHUNK_CONTACTS { + let addr = format!("member{i}@example.net"); + contact_ids.push(add_key_contact(alice, &addr, with_key).await?); + } + add_to_chat_contacts_table(alice, time(), chat_id, &contact_ids).await?; + assert_eq!( + keyupdate_recipients(alice, KEYUPDATE_MAX_RECIPIENTS) + .await? + .len(), + KEYUPDATE_CHUNK_CONTACTS + 1 + ); + + alice.add_transport("alice@relay.example.net").await; + maybe_send_keyupdate_message(alice).await?; + + let sent = [alice.pop_sent_msg().await, alice.pop_sent_msg().await]; + assert!(alice.pop_sent_msg_opt().await.is_none()); + + let recipients: Vec> = sent + .iter() + .map(|msg| msg.recipients.split(' ').collect()) + .collect(); + let mut sizes: Vec = recipients.iter().map(BTreeSet::len).collect(); + sizes.sort(); + assert_eq!(sizes, [1, KEYUPDATE_CHUNK_CONTACTS]); + assert!(recipients[0].is_disjoint(&recipients[1])); + + // Each recipient only ever sees the size of its own chunk; + // the extra session key packet is the sender's own key. + for (msg, recipients) in sent.iter().zip(&recipients) { + assert_anonymous_recipients(&msg.payload, recipients.len() + 1); + } + + Ok(()) +} + +/// Tests that a receiver who does not know the sender yet +/// still stores the key, and still gets nothing to see. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_keyupdate_from_unknown_sender() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + send_text_message(alice, bob).await; + let contacts = Contact::get_real_cnt(bob).await?; + + alice.add_transport("alice@relay.example.net").await; + maybe_send_keyupdate_message(alice).await?; + bob.recv_msg_trash(&alice.pop_sent_msg().await).await; + + assert_eq!(Contact::get_real_cnt(bob).await?, contacts); + + // Look the contact up by fingerprint alone, so the key can only come + // from the keyupdate and not from importing Alice's current vCard. + let alice_contact_id = bob.add_or_lookup_contact_id_no_key(alice).await; + let alice_contact = Contact::get_by_id(bob, alice_contact_id).await?; + let alice_key = alice_contact.public_key(bob).await?.unwrap(); + let addrs = addresses_from_public_key(&alice_key).unwrap(); + assert!(addrs.contains(&"alice@relay.example.net".to_string())); + + Ok(()) +} diff --git a/src/lib.rs b/src/lib.rs index 1c576ba22b..d430487f76 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -72,6 +72,7 @@ pub mod ephemeral; mod imap; pub mod imex; pub mod key; +mod keyupdate; pub mod location; pub mod login_param; pub mod message; diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 0c5807fc3a..6c9eaf6df6 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -2437,7 +2437,7 @@ fn b_encode(value: &str) -> String { /// Returns the headers to place into the encrypted part /// of messages that are not part of a chat. -async fn non_chat_protected_headers( +async fn non_chat_headers( context: &Context, subject: &str, ) -> Result)>> { @@ -2483,7 +2483,7 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( let message: MimePart<'static> = MimePart::new("text/plain", "Secure-Join"); - let mut headers = non_chat_protected_headers(context, "Secure-Join").await?; + let mut headers = non_chat_headers(context, "Secure-Join").await?; headers.push(("Secure-Join", Raw::new(step.to_string()).into())); headers.push(("Secure-Join-Auth", Text::new(auth.to_string()).into())); @@ -2508,6 +2508,75 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( render_with_self_key(context, queued_mail).await } +/// Returns the body of a keyupdate message, shaped like a receipt notification. +/// +/// The shape is what every core goes by, as a keyupdate carries no marker: +/// a `multipart/report` is trashed as an MDN even where unencrypted mail is accepted, +/// while a plain text body would end up in a contact request. +/// The report deliberately names no original message, see [`crate::keyupdate`]. +fn keyupdate_body(from_addr: &str) -> MimePart<'static> { + // Human-readable first part as RFC 6522 requires, untranslated like in `render_mdn`. + let text_part = MimePart::new( + "text/plain", + "This message updates the sender's encryption key and relay list.", + ); + let mut message = MimePart::new( + "multipart/report; report-type=disposition-notification", + vec![text_part], + ); + message.add_part(MimePart::new( + "message/disposition-notification", + format!( + "Original-Recipient: rfc822;{from_addr}\r\n\ + Final-Recipient: rfc822;{from_addr}\r\n\ + Disposition: automatic-action/MDN-sent-automatically; processed\r\n" + ), + )); + message +} + +/// Renders a keyupdate message informing the owners of `recipient_keys` +/// about the current key and relay list, see [`crate::keyupdate`]. +pub(crate) async fn render_keyupdate_message( + context: &Context, + rfc724_mid: &str, + recipient_keys: Vec, +) -> Result { + info!( + context, + "Sending keyupdate message to {} recipients.", + recipient_keys.len() + ); + let message = keyupdate_body(&context.get_primary_self_addr().await?); + + let headers = non_chat_headers(context, "Keyupdate").await?; + let message = add_headers_to_encrypted_part(message, headers); + + let queued_mail = QueuedMail { + raw_message: part_to_bytes(message), + display_name: String::new(), + rfc724_mid: rfc724_mid.to_string(), + encryption: Encryption::Asymmetric { + encryption_pubkeys: recipient_keys + .into_iter() + .map(|key| (String::new(), key)) + .collect(), + }, + + // Attached key with its relay list notation is the actual payload. + should_attach_pubkey: true, + + // Unsigned, so that no intended recipient fingerprint subpacket + // reveals the chunk's recipients to each other. + should_sign: false, + + // Disable compression to avoid side channels, message body is small anyway. + should_compress: false, + }; + + render_with_self_key(context, queued_mail).await +} + /// Renders MIME part into a vector of bytes. pub(crate) fn part_to_bytes(message: MimePart<'static>) -> Vec { let mut raw_message = Vec::new(); diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 4dd51d59b0..fdd4ffc64a 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -514,6 +514,7 @@ pub(crate) async fn receive_imf_inner( // A report naming no message can never be applied to one, // and nothing else should come out of it: no contact, no chat, // and no `last_seen` update lighting up an online dot. + // This is also how keyupdates are trashed, see `crate::keyupdate`. info!(context, "Report without message reference (TRASH)."); return trash().await; } diff --git a/src/scheduler.rs b/src/scheduler.rs index 30a27eb8de..0c22dbe8ee 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -21,6 +21,7 @@ use crate::download::{download_known_post_messages_without_pre_message, download use crate::ephemeral; use crate::events::EventType; use crate::imap::{Imap, session::Session}; +use crate::keyupdate::{maybe_send_keyupdate_message, schedule_keyupdate_check}; use crate::location; use crate::log::{LogExt, warn}; use crate::reaction::broadcast_reactions::maybe_broadcast_reactions; @@ -573,6 +574,9 @@ async fn smtp_loop( return; } + // Reschedule the check to catch changes lost to a restart. + schedule_keyupdate_check(&ctx).await.log_err(&ctx).ok(); + let mut timeout = None; loop { if let Err(err) = send_smtp_messages(&ctx, &mut connection).await { @@ -626,8 +630,29 @@ async fn smtp_loop( slept.saturating_add(rand::random_range((slept / 2)..=slept)), )); } else { + // Queue is drained: send a due keyupdate without delaying real messages. + let next_check = ctx.next_keyupdate_check.load(Ordering::Relaxed); + let wait = u64::try_from(next_check.saturating_sub(time())).unwrap_or_default(); + if next_check != 0 && wait == 0 { + // Clear first so that an intervening transport change schedules a new check. + ctx.next_keyupdate_check.store(0, Ordering::Relaxed); + maybe_send_keyupdate_message(&ctx) + .await + .context("Failed to send keyupdate message") + .log_err(&ctx) + .ok(); + continue; + } + info!(ctx, "SMTP has no messages to retry, waiting for interrupt."); - idle_interrupt_receiver.recv().await.unwrap_or_default(); + let interrupt = idle_interrupt_receiver.recv(); + if next_check != 0 { + tokio::time::timeout(std::time::Duration::from_secs(wait), interrupt) + .await + .ok(); + } else { + interrupt.await.ok(); + } }; info!(ctx, "SMTP fake idle interrupted.") diff --git a/src/securejoin.rs b/src/securejoin.rs index 2c93945280..81a86ef235 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -16,7 +16,7 @@ use crate::key; use crate::key::{DcKey, Fingerprint, load_self_public_key, self_fingerprint}; use crate::log::LogExt as _; use crate::log::warn; -use crate::message::{self, Message, Viewtype}; +use crate::message::{Message, Viewtype}; use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::param::Param; use crate::qr::check_qr; @@ -567,8 +567,7 @@ pub(crate) async fn handle_securejoin_handshake( ) .await?; - let msg_id = message::insert_tombstone(context, &rfc724_mid).await?; - insert_into_smtp(context, &rfc724_mid, &addr, rendered_message, msg_id).await?; + insert_into_smtp(context, &rfc724_mid, &addr, rendered_message).await?; context.scheduler.interrupt_smtp().await; Ok(HandshakeMessage::Done) diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 2db5542041..781d8b9298 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -12,7 +12,7 @@ use crate::context::Context; use crate::events::EventType; use crate::key::{DcKey as _, self_fingerprint}; use crate::log::LogExt; -use crate::message::{self, Message, MsgId, Viewtype}; +use crate::message::{Message, MsgId, Viewtype}; use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::param::{Param, Params}; use crate::pgp::addresses_from_public_key; @@ -340,8 +340,7 @@ pub(crate) async fn send_handshake_message( ) .await?; - let msg_id = message::insert_tombstone(context, &rfc724_mid).await?; - insert_into_smtp(context, &rfc724_mid, &recipients, rendered_message, msg_id).await?; + insert_into_smtp(context, &rfc724_mid, &recipients, rendered_message).await?; context.scheduler.interrupt_smtp().await; } else { let mut msg = Message { diff --git a/src/smtp.rs b/src/smtp.rs index f404d2daa4..850ac61581 100644 --- a/src/smtp.rs +++ b/src/smtp.rs @@ -327,14 +327,15 @@ pub(crate) async fn smtp_send( status } -/// Inserts a rendered message into the `smtp` table for sending. +/// Inserts a tombstone for `rfc724_mid` +/// and queues the rendered message for SMTP sending. pub(crate) async fn insert_into_smtp( context: &Context, rfc724_mid: &str, recipients: &str, rendered_message: String, - msg_id: MsgId, -) -> Result<(), Error> { +) -> Result<()> { + let msg_id = message::insert_tombstone(context, rfc724_mid).await?; context .sql .execute( diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index 805295d714..da9025863a 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -2610,6 +2610,19 @@ UPDATE msgs SET state=24 WHERE state=18; -- Change OutPreparing to OutFailed. .await?; } + inc_and_check(&mut migration_version, 164)?; + if dbversion < migration_version { + // Seed the keyupdate baseline so that upgrading alone sends nothing, + // see `keyupdate.rs`. + sql.execute_migration( + "INSERT OR REPLACE INTO config (keyname, value) + SELECT 'keyupdate_baseline', IFNULL(group_concat(addr, ' ' ORDER BY addr), '') + FROM transports WHERE is_published=1", + migration_version, + ) + .await?; + } + let new_version = sql .get_raw_config_int(VERSION_CFG) .await? diff --git a/src/sql/migrations/migrations_tests.rs b/src/sql/migrations/migrations_tests.rs index fe09e21f4f..2aa2c4ffae 100644 --- a/src/sql/migrations/migrations_tests.rs +++ b/src/sql/migrations/migrations_tests.rs @@ -8,6 +8,7 @@ use crate::contact::ContactId; use crate::contact::Origin; use crate::test_utils::TestContext; use crate::tools; +use crate::transport::add_pseudo_transport; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_clear_config_cache() -> anyhow::Result<()> { @@ -29,6 +30,29 @@ async fn test_clear_config_cache() -> anyhow::Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_keyupdate_baseline_migration() -> Result<()> { + let configured = STOP_MIGRATIONS_AT + .scope(163, async move { TestContext::new_alice().await }) + .await; + // An address sorting before the primary pins the seed's ORDER BY, + // an unpublished transport pins its filter. + add_pseudo_transport(&configured, "aa@example.org").await?; + add_pseudo_transport(&configured, "unpublished@example.org").await?; + configured + .sql + .execute( + "UPDATE transports SET is_published=0 WHERE addr='unpublished@example.org'", + (), + ) + .await?; + configured.sql.run_migrations(&configured).await?; + let relays = configured.get_config(Config::KeyupdateBaseline).await?; + assert_eq!(relays.as_deref(), Some("aa@example.org alice@example.org")); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_key_contacts_migration_autocrypt() -> Result<()> { let t = STOP_MIGRATIONS_AT diff --git a/src/test_utils.rs b/src/test_utils.rs index a20f01934e..d3acd52825 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -36,7 +36,6 @@ use crate::contact::{ use crate::context::Context; use crate::events::{Event, EventEmitter, EventType, Events}; use crate::key::{self, DcKey, self_fingerprint}; -use crate::login_param::EnteredLoginParam; use crate::message::{Message, MessageState, MsgId}; use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::pgp::SeipdVersion; @@ -45,6 +44,7 @@ use crate::securejoin::{get_securejoin_qr, join_securejoin}; use crate::smtp::msg_has_pending_smtp_job; use crate::stock_str::StockStrings; use crate::tools::time; +use crate::transport::add_pseudo_transport; /// The number of info messages added to new e2ee chats. /// Currently this is "Messages are end-to-end encrypted.", string `ChatProtectionEnabled`. @@ -199,17 +199,7 @@ impl TestContextManager { test_context.name() )); - // Insert a transport for the new address. - test_context.sql - .execute( - "INSERT OR IGNORE INTO transports (addr, entered_param, configured_param) VALUES (?, ?, ?)", - ( - new_addr, - serde_json::to_string(&EnteredLoginParam{addr: new_addr.to_string(), ..Default::default()}).unwrap(), - format!(r#"{{"addr":"{new_addr}","imap":[],"imap_user":"","imap_password":"","smtp":[],"smtp_user":"","smtp_password":"","certificate_checks":"Automatic"}}"#) - ), - ).await.unwrap(); - + test_context.add_transport(new_addr).await; test_context.set_primary_self_addr(new_addr).await.unwrap(); // ensure_secret_key_exists() is called during configure key::ensure_secret_key_exists(test_context).await.unwrap(); @@ -577,6 +567,22 @@ impl TestContext { } } + /// Adds a published transport for `addr` without any network activity. + pub async fn add_transport(&self, addr: &str) { + add_pseudo_transport(self, addr).await.unwrap(); + // A fresh `add_timestamp` makes the re-signed self key newer than the copies + // contacts hold, so that certificate merging prefers the new relay list. + self.sql + .execute( + "UPDATE transports SET add_timestamp=? WHERE addr=?", + (time(), addr), + ) + .await + .unwrap(); + // Invalidate the cached self key so that it is regenerated with the new list. + self.self_public_key.lock().await.take(); + } + /// Retrieves a sent message from the jobs table. /// /// This retrieves and removes a message which has been scheduled to send from the jobs diff --git a/src/transport.rs b/src/transport.rs index 7c77f4da72..c20367201d 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -19,6 +19,7 @@ use crate::config::Config; use crate::context::Context; use crate::ensure_and_debug_assert; use crate::events::EventType; +use crate::keyupdate::{schedule_keyupdate_check, set_current_relays_as_keyupdate_baseline}; use crate::login_param::EnteredLoginParam; use crate::net::load_connection_timestamp; use crate::provider::Socket; @@ -614,6 +615,8 @@ pub(crate) async fn send_sync_transports(context: &Context) -> Result<()> { removed_transports, }) .await?; + // Schedule the check before interrupting, so the woken SMTP loop sees it. + schedule_keyupdate_check(context).await?; context.scheduler.interrupt_smtp().await; Ok(()) @@ -678,6 +681,12 @@ pub(crate) async fn sync_transports( .restart_io_after_fetch .store(true, Ordering::Relaxed); context.emit_event(EventType::TransportsModified); + + // Without setting the baseline a restarting SMTP loop + // would send duplicate keyupdates on every second device. + // Acceptable gap: a user changing relay setup on two devices concurrently + // will not trigger a message with the "merged" set of the concurrent changes. + set_current_relays_as_keyupdate_baseline(context).await?; } Ok(()) } @@ -728,7 +737,7 @@ fn maybe_reelect_local_primary(transaction: &mut rusqlite::Transaction) -> Resul pub(crate) async fn add_pseudo_transport(context: &Context, addr: &str) -> Result<()> { context.sql .execute( - "INSERT INTO transports (addr, entered_param, configured_param) VALUES (?, ?, ?)", + "INSERT OR IGNORE INTO transports (addr, entered_param, configured_param) VALUES (?, ?, ?)", ( addr, serde_json::to_string(&EnteredLoginParam{addr: addr.to_string(), ..Default::default()})?, From 6ef0cd16a02691a870313fb5817a1888473cc024 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Sat, 29 Aug 2026 20:36:22 +0200 Subject: [PATCH 2/2] fix: return no relay address for key-contacts without an address `relay_addrs()` fell back to the contact address even when it is empty, which happens for key-contacts created from a sync message or for the self-contact, putting an empty string into the SMTP recipient list. --- src/pgp.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/pgp.rs b/src/pgp.rs index 0e3f68c38b..6f66a5d828 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -474,7 +474,18 @@ pub(crate) fn addresses_from_public_key(public_key: &SignedPublicKey) -> Option< /// Returns the addresses to reach the owner of `public_key`, /// falling back to `addr` if the key carries no relay list. pub(crate) fn relay_addrs(public_key: &SignedPublicKey, addr: &str) -> Vec { - addresses_from_public_key(public_key).unwrap_or_else(|| vec![addr.to_string()]) + addresses_from_public_key(public_key).unwrap_or_else(|| { + if addr.is_empty() { + vec![] + } else { + vec![addr.to_string()] + } + }) +} + +/// Returns true if the key can be encrypted to, i.e. has an encryption subkey. +pub(crate) fn pubkey_can_encrypt(public_key: &SignedPublicKey) -> bool { + select_pk_for_encryption(public_key).is_some() } /// Returns true if public key advertises SEIPDv2 feature.