feat: track gossiping per (chat, fingerprint) pair

This change simplifies
updating the gossip timestamps
when we receive a message
because we only need to know
the keys received in Autocrypt-Gossip
header and which chat the message is
assigned to.
We no longer need to iterate
over the member list.

This is a preparation
for PGP contacts
and member lists that contain
key fingerprints rather than
email addresses.

This change also removes encryption preference
from Autocrypt-Gossip header.
It SHOULD NOT be gossiped
according to the Autocrypt specification
and we ignore encryption preference anyway
since 1.157.0.

test_gossip_optimization is removed
because it relied on a per-chat gossip_timestamp.
This commit is contained in:
link2xt
2025-04-08 15:13:53 +00:00
parent 0b82b42128
commit 9f5e608c61
8 changed files with 109 additions and 173 deletions

View File

@@ -16,8 +16,6 @@ class TestGroupStressTests:
lp.sec("ac1: send message to new group chat")
msg1 = chat.send_text("hello")
assert msg1.is_encrypted()
gossiped_timestamp = chat.get_summary()["gossiped_timestamp"]
assert gossiped_timestamp > 0
assert chat.num_contacts() == 3 + 1
@@ -46,19 +44,13 @@ class TestGroupStressTests:
assert to_remove.addr in sysmsg.text
assert sysmsg.chat.num_contacts() == 3
# Receiving message about removed contact does not reset gossip
assert chat.get_summary()["gossiped_timestamp"] == gossiped_timestamp
lp.sec("ac1: sending another message to the chat")
chat.send_text("hello2")
msg = ac2._evtracker.wait_next_incoming_message()
assert msg.text == "hello2"
assert chat.get_summary()["gossiped_timestamp"] == gossiped_timestamp
lp.sec("ac1: adding fifth member to the chat")
chat.add_contact(ac5)
# Adding contact to chat resets gossiped_timestamp
assert chat.get_summary()["gossiped_timestamp"] >= gossiped_timestamp
lp.sec("ac2: receiving system message about contact addition")
sysmsg = ac2._evtracker.wait_next_incoming_message()

View File

@@ -798,12 +798,6 @@ def test_send_and_receive_will_encrypt_decrypt(acfactory, lp):
msg3.mark_seen()
assert not list(ac1.get_fresh_messages())
# Test that we do not gossip peer keys in 1-to-1 chat,
# as it makes no sense to gossip to peers their own keys.
# Gossip is only sent in encrypted messages,
# and we sent encrypted msg_back right above.
assert chat2b.get_summary()["gossiped_timestamp"] == 0
lp.sec("create group chat with two members, one of which has no encrypt state")
chat = ac1.create_group_chat("encryption test")
chat.add_contact(ac2)
@@ -813,41 +807,6 @@ def test_send_and_receive_will_encrypt_decrypt(acfactory, lp):
ac1._evtracker.get_matching("DC_EVENT_SMTP_MESSAGE_SENT")
def test_gossip_optimization(acfactory, lp):
"""Test that gossip timestamp is updated when someone else sends gossip,
so we don't have to send gossip ourselves.
"""
ac1, ac2, ac3 = acfactory.get_online_accounts(3)
acfactory.introduce_each_other([ac1, ac2])
acfactory.introduce_each_other([ac2, ac3])
lp.sec("ac1 creates a group chat with ac2")
group_chat = ac1.create_group_chat("hello")
group_chat.add_contact(ac2)
msg = group_chat.send_text("hi")
# No Autocrypt gossip was sent yet.
gossiped_timestamp = msg.chat.get_summary()["gossiped_timestamp"]
assert gossiped_timestamp == 0
msg = ac2._evtracker.wait_next_incoming_message()
assert msg.is_encrypted()
assert msg.text == "hi"
lp.sec("ac2 adds ac3 to the group")
msg.chat.add_contact(ac3)
lp.sec("ac1 receives message from ac2 and updates gossip timestamp")
msg = ac1._evtracker.wait_next_incoming_message()
assert msg.is_encrypted()
# ac1 has updated the gossip timestamp even though no gossip was sent by ac1.
# ac1 does not need to send gossip because ac2 already did it.
gossiped_timestamp = msg.chat.get_summary()["gossiped_timestamp"]
assert gossiped_timestamp == int(msg.time_sent.timestamp())
def test_send_first_message_as_long_unicode_with_cr(acfactory, lp):
ac1, ac2 = acfactory.get_online_accounts(2)

View File

@@ -1376,41 +1376,10 @@ impl ChatId {
}
pub(crate) async fn reset_gossiped_timestamp(self, context: &Context) -> Result<()> {
self.set_gossiped_timestamp(context, 0).await
}
/// Get timestamp of the last gossip sent in the chat.
/// Zero return value means that gossip was never sent.
pub async fn get_gossiped_timestamp(self, context: &Context) -> Result<i64> {
let timestamp: Option<i64> = context
.sql
.query_get_value("SELECT gossiped_timestamp FROM chats WHERE id=?;", (self,))
.await?;
Ok(timestamp.unwrap_or_default())
}
pub(crate) async fn set_gossiped_timestamp(
self,
context: &Context,
timestamp: i64,
) -> Result<()> {
ensure!(
!self.is_special(),
"can not set gossiped timestamp for special chats"
);
info!(
context,
"Set gossiped_timestamp for chat {} to {}.", self, timestamp,
);
context
.sql
.execute(
"UPDATE chats SET gossiped_timestamp=? WHERE id=?;",
(timestamp, self),
)
.execute("DELETE FROM gossip_timestamp WHERE chat_id=?", (self,))
.await?;
Ok(())
}
@@ -1917,7 +1886,6 @@ impl Chat {
name: self.name.clone(),
archived: self.visibility == ChatVisibility::Archived,
param: self.param.to_string(),
gossiped_timestamp: self.id.get_gossiped_timestamp(context).await?,
is_sending_locations: self.is_sending_locations,
color: self.get_color(context).await?,
profile_image: self
@@ -2460,9 +2428,6 @@ pub struct ChatInfo {
/// This is the string-serialised version of `Params` currently.
pub param: String,
/// Last time this client sent autocrypt gossip headers to this chat.
pub gossiped_timestamp: i64,
/// Whether this chat is currently sending location-stream messages.
pub is_sending_locations: bool,
@@ -3101,10 +3066,6 @@ pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -
let now = smeared_time(context);
if rendered_msg.is_gossiped {
msg.chat_id.set_gossiped_timestamp(context, now).await?;
}
if rendered_msg.last_added_location_id.is_some() {
if let Err(err) = location::set_kml_sent_timestamp(context, msg.chat_id, now).await {
error!(context, "Failed to set kml sent_timestamp: {err:#}.");

View File

@@ -13,6 +13,7 @@ use mail_builder::headers::HeaderType;
use mail_builder::mime::MimePart;
use tokio::fs;
use crate::aheader::{Aheader, EncryptPreference};
use crate::blob::BlobObject;
use crate::chat::{self, Chat};
use crate::config::Config;
@@ -22,6 +23,7 @@ use crate::contact::{Contact, ContactId, Origin};
use crate::context::Context;
use crate::e2ee::EncryptHelper;
use crate::ephemeral::Timer as EphemeralTimer;
use crate::key::DcKey;
use crate::location;
use crate::message::{self, Message, MsgId, Viewtype};
use crate::mimeparser::{is_hidden, SystemMessage};
@@ -131,7 +133,6 @@ pub struct RenderedEmail {
pub message: String,
// pub envelope: Envelope,
pub is_encrypted: bool,
pub is_gossiped: bool,
pub last_added_location_id: Option<u32>,
/// A comma-separated string of sync-IDs that are used by the rendered email and must be deleted
@@ -433,41 +434,6 @@ impl MimeFactory {
}
}
async fn should_do_gossip(&self, context: &Context, multiple_recipients: bool) -> Result<bool> {
match &self.loaded {
Loaded::Message { chat, msg } => {
if chat.typ == Chattype::Broadcast {
// Never send Autocrypt-Gossip in broadcast lists
// as it discloses recipient email addresses.
return Ok(false);
}
let cmd = msg.param.get_cmd();
if cmd == SystemMessage::MemberAddedToGroup
|| cmd == SystemMessage::SecurejoinMessage
{
Ok(true)
} else if multiple_recipients {
// beside key- and member-changes, force a periodic re-gossip.
let gossiped_timestamp = chat.id.get_gossiped_timestamp(context).await?;
let gossip_period = context.get_config_i64(Config::GossipPeriod).await?;
// `gossip_period == 0` is a special case for testing,
// enabling gossip in every message.
// Otherwise "smeared timestamps" may result in the condition
// to fail even if the clock is monotonic.
if gossip_period == 0 || time() >= gossiped_timestamp + gossip_period {
Ok(true)
} else {
Ok(false)
}
} else {
Ok(false)
}
}
Loaded::Mdn { .. } => Ok(false),
}
}
fn should_attach_profile_data(msg: &Message) -> bool {
msg.param.get_cmd() != SystemMessage::SecurejoinMessage || {
let step = msg.param.get(Param::Arg).unwrap_or_default();
@@ -787,8 +753,6 @@ impl MimeFactory {
}
}
let mut is_gossiped = false;
let peerstates = self.peerstates_for_recipients(context).await?;
let is_encrypted = !self.should_force_plaintext()
&& (e2ee_guaranteed || encrypt_helper.should_encrypt(context, &peerstates).await?);
@@ -956,16 +920,78 @@ impl MimeFactory {
// Add gossip headers in chats with multiple recipients
let multiple_recipients =
peerstates.len() > 1 || context.get_config_bool(Config::BccSelf).await?;
if self.should_do_gossip(context, multiple_recipients).await? {
for peerstate in peerstates.iter().filter_map(|(state, _)| state.as_ref()) {
if let Some(header) = peerstate.render_gossip_header(verified) {
message = message.header(
"Autocrypt-Gossip",
mail_builder::headers::raw::Raw::new(header),
);
is_gossiped = true;
let gossip_period = context.get_config_i64(Config::GossipPeriod).await?;
let now = time();
match &self.loaded {
Loaded::Message { chat, msg } => {
if chat.typ != Chattype::Broadcast {
for peerstate in peerstates.iter().filter_map(|(state, _)| state.as_ref()) {
let Some(key) = peerstate.peek_key(verified) else {
continue;
};
let fingerprint = key.dc_fingerprint().hex();
let cmd = msg.param.get_cmd();
let should_do_gossip = cmd == SystemMessage::MemberAddedToGroup
|| cmd == SystemMessage::SecurejoinMessage
|| multiple_recipients && {
let gossiped_timestamp: Option<i64> = context
.sql
.query_get_value(
"SELECT timestamp
FROM gossip_timestamp
WHERE chat_id=? AND fingerprint=?",
(chat.id, &fingerprint),
)
.await?;
// `gossip_period == 0` is a special case for testing,
// enabling gossip in every message.
//
// If current time is in the past compared to
// `gossiped_timestamp`, we also gossip because
// either the `gossiped_timestamp` or clock is wrong.
gossip_period == 0
|| gossiped_timestamp
.is_none_or(|ts| now >= ts + gossip_period || now < ts)
};
if !should_do_gossip {
continue;
}
let header = Aheader::new(
peerstate.addr.clone(),
key.clone(),
// Autocrypt 1.1.0 specification says that
// `prefer-encrypt` attribute SHOULD NOT be included.
EncryptPreference::NoPreference,
)
.to_string();
message = message.header(
"Autocrypt-Gossip",
mail_builder::headers::raw::Raw::new(header),
);
context
.sql
.execute(
"INSERT INTO gossip_timestamp (chat_id, fingerprint, timestamp)
VALUES (?, ?, ?)
ON CONFLICT (chat_id, fingerprint)
DO UPDATE SET timestamp=excluded.timestamp",
(chat.id, &fingerprint, now),
)
.await?;
}
}
}
Loaded::Mdn { .. } => {
// Never gossip in MDNs.
}
}
// Set the appropriate Content-Type for the inner message.
@@ -1109,7 +1135,6 @@ impl MimeFactory {
message,
// envelope: Envelope::new,
is_encrypted,
is_gossiped,
last_added_location_id,
sync_ids_to_delete: self.sync_ids_to_delete,
rfc724_mid,

View File

@@ -405,28 +405,6 @@ impl Peerstate {
};
}
/// Returns the contents of the `Autocrypt-Gossip` header for outgoing messages.
pub fn render_gossip_header(&self, verified: bool) -> Option<String> {
if let Some(key) = self.peek_key(verified) {
let header = Aheader::new(
self.addr.clone(),
key.clone(), // TODO: avoid cloning
// Autocrypt 1.1.0 specification says that
// `prefer-encrypt` attribute SHOULD NOT be included,
// but we include it anyway to propagate encryption
// preference to new members in group chats.
if self.last_seen_autocrypt > 0 {
self.prefer_encrypt
} else {
EncryptPreference::NoPreference
},
);
Some(header.to_string())
} else {
None
}
}
/// Converts the peerstate into the contact public key.
///
/// Similar to [`Self::peek_key`], but consumes the peerstate and returns owned key.

View File

@@ -24,6 +24,7 @@ use crate::ephemeral::{stock_ephemeral_timer_changed, Timer as EphemeralTimer};
use crate::events::EventType;
use crate::headerdef::{HeaderDef, HeaderDefMap};
use crate::imap::{markseen_on_imap_table, GENERATED_PREFIX};
use crate::key::DcKey;
use crate::log::LogExt;
use crate::message::{
self, rfc724_mid_exists, Message, MessageState, MessengerMessage, MsgId, Viewtype,
@@ -452,22 +453,25 @@ pub(crate) async fn receive_imf_inner(
}
// Update gossiped timestamp for the chat if someone else or our other device sent
// Autocrypt-Gossip for all recipients in the chat to avoid sending Autocrypt-Gossip ourselves
// Autocrypt-Gossip header to avoid sending Autocrypt-Gossip ourselves
// and waste traffic.
let chat_id = received_msg.chat_id;
if !chat_id.is_special()
&& mime_parser.recipients.iter().all(|recipient| {
recipient.addr == mime_parser.from.addr
|| mime_parser.gossiped_keys.contains_key(&recipient.addr)
})
{
info!(
context,
"Received message contains Autocrypt-Gossip for all members of {chat_id}, updating timestamp."
);
if chat_id.get_gossiped_timestamp(context).await? < mime_parser.timestamp_sent {
chat_id
.set_gossiped_timestamp(context, mime_parser.timestamp_sent)
if !chat_id.is_special() {
for gossiped_key in mime_parser.gossiped_keys.values() {
context
.sql
.transaction(move |transaction| {
let fingerprint = gossiped_key.dc_fingerprint().hex();
transaction.execute(
"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),
)?;
Ok(())
})
.await?;
}
}

View File

@@ -1169,6 +1169,23 @@ CREATE INDEX msgs_status_updates_index2 ON msgs_status_updates (uid);
.await?;
}
inc_and_check(&mut migration_version, 130)?;
if dbversion < migration_version {
sql.execute_migration(
"
CREATE TABLE gossip_timestamp (
chat_id INTEGER NOT NULL,
fingerprint TEXT NOT NULL, -- Upper-case fingerprint of the key.
timestamp INTEGER NOT NULL,
UNIQUE (chat_id, fingerprint)
) STRICT;
CREATE INDEX gossip_timestamp_index ON gossip_timestamp (chat_id, fingerprint);
",
migration_version,
)
.await?;
}
let new_version = sql
.get_raw_config_int(VERSION_CFG)
.await?

View File

@@ -32,7 +32,7 @@ CREATE TABLE chats (
grpid TEXT DEFAULT '',
param TEXT DEFAULT '',
archived INTEGER DEFAULT 0,
gossiped_timestamp INTEGER DEFAULT 0,
gossiped_timestamp INTEGER DEFAULT 0, -- deprecated 2025-04-08, replaced with gossiped_timestamp table
locations_send_begin INTEGER DEFAULT 0,
locations_send_until INTEGER DEFAULT 0,
locations_last_sent INTEGER DEFAULT 0,