Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-04-17 13:36:30 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix: Always pass the correct sort timestamp to ChatId::set_protection() (#5088)

Browse Source 
Before in some places it was correctly calculated by passing the "sent" timestamp to
`calc_sort_timestamp()`, but in other places just the system time was used. In some complex
scenarios like #5088 (restoration of a backup made before a contact verification) it led to wrong
sort timestamps of protection messages and also messages following by them.

But to reduce number of args passed to functions needing to calculate the sort timestamp, add
message timestamps to `struct MimeMessage` which is anyway passed everywhere.
This commit is contained in:
iequidoo
2023-12-07 21:32:30 -03:00
committed by iequidoo
parent 2a0a51bea0
commit fce3f80654
5 changed files with 128 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
src/chat.rs
View File

@@ -36,7 +36,7 @@ use crate::mimefactory::MimeFactory;
use crate::mimeparser::SystemMessage;
use crate::param::{Param, Params};
use crate::peerstate::Peerstate;
use crate::receive_imf::ReceivedMsg;
use crate::receive_imf::{calc_sort_timestamp, ReceivedMsg};
use crate::smtp::send_msg_to_smtp;
use crate::sql;
use crate::stock_str;
@@ -578,7 +578,7 @@ impl ChatId {
///
/// `timestamp_sort` is used as the timestamp of the added message
/// and should be the timestamp of the change happening.
pub(crate) async fn set_protection(
async fn set_protection_for_timestamp_sort(
self,
context: &Context,
protect: ProtectionStatus,
@@ -600,6 +600,22 @@ impl ChatId {
}
}
/// Sets protection and sends or adds a message.
///
/// `timestamp_sent` is the "sent" timestamp of a message caused the protection state change.
pub(crate) async fn set_protection(
self,
context: &Context,
protect: ProtectionStatus,
timestamp_sent: i64,
contact_id: Option<ContactId>,
) -> Result<()> {
let sort_to_bottom = true;
let ts = calc_sort_timestamp(context, timestamp_sent, self, sort_to_bottom, false).await?;
self.set_protection_for_timestamp_sort(context, protect, ts, contact_id)
.await
}
/// Sets the 1:1 chat with the given address to ProtectionStatus::Protected,
/// and posts a `SystemMessage::ChatProtectionEnabled` into it.
///
@@ -607,6 +623,7 @@ impl ChatId {
pub(crate) async fn set_protection_for_contact(
context: &Context,
contact_id: ContactId,
timestamp: i64,
) -> Result<()> {
let chat_id = ChatId::create_for_contact_with_blocked(context, contact_id, Blocked::Yes)
.await
@@ -615,7 +632,7 @@ impl ChatId {
.set_protection(
context,
ProtectionStatus::Protected,
smeared_time(context),
timestamp,
Some(contact_id),
)
.await?;
@@ -3258,7 +3275,7 @@ pub async fn create_group_chat(
if protect == ProtectionStatus::Protected {
chat_id
.set_protection(context, protect, timestamp, None)
.set_protection_for_timestamp_sort(context, protect, timestamp, None)
.await?;
}

29
src/mimeparser.rs
View File

@@ -1,5 +1,6 @@
//! # MIME message parsing module.
use std::cmp::min;
use std::collections::{HashMap, HashSet};
use std::future::Future;
use std::path::Path;
@@ -37,8 +38,8 @@ use crate::simplify::{simplify, SimplifiedText};
use crate::stock_str;
use crate::sync::SyncItems;
use crate::tools::{
create_smeared_timestamp, get_filemeta, parse_receive_headers, strip_rtlo_characters,
truncate_by_lines,
create_smeared_timestamp, get_filemeta, parse_receive_headers, smeared_time,
strip_rtlo_characters, truncate_by_lines,
};
use crate::{location, tools};
@@ -118,6 +119,12 @@ pub(crate) struct MimeMessage {
/// Whether the contact sending this should be marked as bot.
pub(crate) is_bot: bool,
/// When the message was received, in secs since epoch.
pub(crate) timestamp_rcvd: i64,
/// Sender timestamp in secs since epoch. Allowed to be in the future due to unsynchronized
/// clocks, but not too much.
pub(crate) timestamp_sent: i64,
}
#[derive(Debug, PartialEq)]
@@ -386,6 +393,12 @@ impl MimeMessage {
// Auto-submitted is also set by holiday-notices so we also check `chat-version`
let is_bot = headers.contains_key("auto-submitted") && headers.contains_key("chat-version");
let timestamp_rcvd = smeared_time(context);
let timestamp_sent = headers
.get(HeaderDef::Date.get_headername())
.and_then(|value| mailparse::dateparse(value).ok())
.map_or(timestamp_rcvd, |value| min(value, timestamp_rcvd + 60));
let mut parser = MimeMessage {
parts: Vec::new(),
headers,
@@ -415,6 +428,8 @@ impl MimeMessage {
decoded_data: Vec::new(),
hop_info,
is_bot,
timestamp_rcvd,
timestamp_sent,
};
match partial {
@@ -1630,13 +1645,7 @@ impl MimeMessage {
/// Handle reports
/// (MDNs = Message Disposition Notification, the message was read
/// and NDNs = Non delivery notification, the message could not be delivered)
pub async fn handle_reports(
&self,
context: &Context,
from_id: ContactId,
sent_timestamp: i64,
parts: &[Part],
) {
pub async fn handle_reports(&self, context: &Context, from_id: ContactId, parts: &[Part]) {
for report in &self.mdn_reports {
for original_message_id in report
.original_message_id
@@ -1644,7 +1653,7 @@ impl MimeMessage {
.chain(&report.additional_message_ids)
{
if let Err(err) =
handle_mdn(context, from_id, original_message_id, sent_timestamp).await
handle_mdn(context, from_id, original_message_id, self.timestamp_sent).await
{
warn!(context, "Could not handle MDN: {err:#}.");
}

120
src/receive_imf.rs
View File

@@ -172,19 +172,10 @@ pub(crate) async fn receive_imf_inner(
Ok(mime_parser) => mime_parser,
};
let rcvd_timestamp = smeared_time(context);
// Sender timestamp is allowed to be a bit in the future due to
// unsynchronized clocks, but not too much.
let sent_timestamp = mime_parser
.get_header(HeaderDef::Date)
.and_then(|value| mailparse::dateparse(value).ok())
.map_or(rcvd_timestamp, |value| min(value, rcvd_timestamp + 60));
crate::peerstate::maybe_do_aeap_transition(context, &mut mime_parser).await?;
if let Some(peerstate) = &mime_parser.decryption_info.peerstate {
peerstate
.handle_fingerprint_change(context, sent_timestamp)
.handle_fingerprint_change(context, mime_parser.timestamp_sent)
.await?;
// When peerstate is set to Mutual, it's saved immediately to not lose that fact in case
// of an error. Otherwise we don't save peerstate until get here to reduce the number of
@@ -284,7 +275,7 @@ pub(crate) async fn receive_imf_inner(
received_msg = Some(ReceivedMsg {
chat_id: DC_CHAT_ID_TRASH,
state: MessageState::InSeen,
sort_timestamp: sent_timestamp,
sort_timestamp: mime_parser.timestamp_sent,
msg_ids: vec![msg_id],
needs_delete_job: res == securejoin::HandshakeMessage::Done,
#[cfg(test)]
@@ -313,8 +304,6 @@ pub(crate) async fn receive_imf_inner(
incoming,
&to_ids,
rfc724_mid,
sent_timestamp,
rcvd_timestamp,
from_id,
seen || replace_partial_download.is_some(),
is_partial_download,
@@ -328,7 +317,7 @@ pub(crate) async fn receive_imf_inner(
};
if !from_id.is_special() {
contact::update_last_seen(context, from_id, sent_timestamp).await?;
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
@@ -345,9 +334,9 @@ pub(crate) async fn receive_imf_inner(
context,
"Received message contains Autocrypt-Gossip for all members of {chat_id}, updating timestamp."
);
if chat_id.get_gossiped_timestamp(context).await? < sent_timestamp {
if chat_id.get_gossiped_timestamp(context).await? < mime_parser.timestamp_sent {
chat_id
.set_gossiped_timestamp(context, sent_timestamp)
.set_gossiped_timestamp(context, mime_parser.timestamp_sent)
.await?;
}
}
@@ -384,7 +373,11 @@ pub(crate) async fn receive_imf_inner(
if let Some(avatar_action) = &mime_parser.user_avatar {
if from_id != ContactId::UNDEFINED
&& context
.update_contacts_timestamp(from_id, Param::AvatarTimestamp, sent_timestamp)
.update_contacts_timestamp(
from_id,
Param::AvatarTimestamp,
mime_parser.timestamp_sent,
)
.await?
{
if let Err(err) = contact::set_profile_image(
@@ -405,7 +398,11 @@ pub(crate) async fn receive_imf_inner(
if !mime_parser.is_mailinglist_message()
&& from_id != ContactId::UNDEFINED
&& context
.update_contacts_timestamp(from_id, Param::StatusTimestamp, sent_timestamp)
.update_contacts_timestamp(
from_id,
Param::StatusTimestamp,
mime_parser.timestamp_sent,
)
.await?
{
if let Err(err) = contact::set_status(
@@ -454,7 +451,7 @@ pub(crate) async fn receive_imf_inner(
context.new_msgs_notify.notify_one();
mime_parser
.handle_reports(context, from_id, sent_timestamp, &mime_parser.parts)
.handle_reports(context, from_id, &mime_parser.parts)
.await;
from_id.mark_bot(context, mime_parser.is_bot).await?;
@@ -525,8 +522,6 @@ async fn add_parts(
incoming: bool,
to_ids: &[ContactId],
rfc724_mid: &str,
sent_timestamp: i64,
rcvd_timestamp: i64,
from_id: ContactId,
seen: bool,
is_partial_download: Option<u32>,
@@ -665,7 +660,6 @@ async fn add_parts(
from_id,
to_ids,
&verified_encryption,
sent_timestamp,
)
.await?
{
@@ -711,7 +705,6 @@ async fn add_parts(
group_changes_msgs = apply_group_changes(
context,
mime_parser,
sent_timestamp,
group_chat_id,
from_id,
to_ids,
@@ -729,7 +722,6 @@ async fn add_parts(
allow_creation,
mailinglist_header,
mime_parser,
sent_timestamp,
)
.await?
{
@@ -740,7 +732,7 @@ async fn add_parts(
}
if let Some(chat_id) = chat_id {
apply_mailinglist_changes(context, mime_parser, sent_timestamp, chat_id).await?;
apply_mailinglist_changes(context, mime_parser, chat_id).await?;
}
// if contact renaming is prevented (for mailinglists and bots),
@@ -826,11 +818,13 @@ async fn add_parts(
// The message itself will be sorted under the device message since the device
// message is `MessageState::InNoticed`, which means that all following
// messages are sorted under it.
let sort_timestamp =
calc_sort_timestamp(context, sent_timestamp, chat_id, true, incoming)
.await?;
chat_id
.set_protection(context, new_protection, sort_timestamp, Some(from_id))
.set_protection(
context,
new_protection,
mime_parser.timestamp_sent,
Some(from_id),
)
.await?;
}
}
@@ -898,7 +892,6 @@ async fn add_parts(
from_id,
to_ids,
&verified_encryption,
sent_timestamp,
)
.await?
{
@@ -936,7 +929,6 @@ async fn add_parts(
group_changes_msgs = apply_group_changes(
context,
mime_parser,
sent_timestamp,
chat_id,
from_id,
to_ids,
@@ -1025,8 +1017,14 @@ async fn add_parts(
};
let in_fresh = state == MessageState::InFresh;
let sort_timestamp =
calc_sort_timestamp(context, sent_timestamp, chat_id, false, incoming).await?;
let sort_timestamp = calc_sort_timestamp(
context,
mime_parser.timestamp_sent,
chat_id,
false,
incoming,
)
.await?;
// Apply ephemeral timer changes to the chat.
//
@@ -1056,7 +1054,11 @@ async fn add_parts(
"Ignoring ephemeral timer change to {ephemeral_timer:?} for chat {chat_id} to avoid rollback.",
);
} else if chat_id
.update_timestamp(context, Param::EphemeralSettingsTimestamp, sent_timestamp)
.update_timestamp(
context,
Param::EphemeralSettingsTimestamp,
mime_parser.timestamp_sent,
)
.await?
{
if let Err(err) = chat_id
@@ -1259,7 +1261,7 @@ async fn add_parts(
match ephemeral_timer {
EphemeralTimer::Disabled => 0,
EphemeralTimer::Enabled { duration } => {
rcvd_timestamp.saturating_add(duration.into())
mime_parser.timestamp_rcvd.saturating_add(duration.into())
}
}
};
@@ -1312,8 +1314,8 @@ RETURNING id
if trash { ContactId::UNDEFINED } else { from_id },
if trash { ContactId::UNDEFINED } else { to_id },
sort_timestamp,
sent_timestamp,
rcvd_timestamp,
mime_parser.timestamp_sent,
mime_parser.timestamp_rcvd,
typ,
state,
is_dc_message,
@@ -1485,7 +1487,7 @@ async fn save_locations(
Ok(())
}
async fn calc_sort_timestamp(
pub(crate) async fn calc_sort_timestamp(
context: &Context,
message_timestamp: i64,
chat_id: ChatId,
@@ -1620,7 +1622,6 @@ async fn create_or_lookup_group(
from_id: ContactId,
to_ids: &[ContactId],
verified_encryption: &VerifiedEncryption,
timestamp: i64,
) -> Result<Option<(ChatId, Blocked)>> {
let grpid = if let Some(grpid) = try_getting_grpid(mime_parser) {
grpid
@@ -1633,7 +1634,7 @@ async fn create_or_lookup_group(
member_ids.push(ContactId::SELF);
}
let res = create_adhoc_group(context, mime_parser, create_blocked, &member_ids, timestamp)
let res = create_adhoc_group(context, mime_parser, create_blocked, &member_ids)
.await
.context("could not create ad hoc group")?
.map(|chat_id| (chat_id, create_blocked));
@@ -1715,7 +1716,7 @@ async fn create_or_lookup_group(
create_blocked,
create_protected,
None,
timestamp,
mime_parser.timestamp_sent,
)
.await
.with_context(|| format!("Failed to create group '{grpname}' for grpid={grpid}"))?;
@@ -1762,7 +1763,6 @@ async fn create_or_lookup_group(
async fn apply_group_changes(
context: &Context,
mime_parser: &mut MimeMessage,
sent_timestamp: i64,
chat_id: ChatId,
from_id: ContactId,
to_ids: &[ContactId],
@@ -1800,7 +1800,11 @@ async fn apply_group_changes(
let allow_member_list_changes = !is_partial_download
&& is_from_in_chat
&& chat_id
.update_timestamp(context, Param::MemberListTimestamp, sent_timestamp)
.update_timestamp(
context,
Param::MemberListTimestamp,
mime_parser.timestamp_sent,
)
.await?;
// Whether to rebuild the member list from scratch.
@@ -1839,7 +1843,7 @@ async fn apply_group_changes(
.set_protection(
context,
ProtectionStatus::Protected,
smeared_time(context),
mime_parser.timestamp_sent,
Some(from_id),
)
.await?;
@@ -1893,7 +1897,11 @@ async fn apply_group_changes(
.filter(|grpname| grpname.len() < 200)
{
if chat_id
.update_timestamp(context, Param::GroupNameTimestamp, sent_timestamp)
.update_timestamp(
context,
Param::GroupNameTimestamp,
mime_parser.timestamp_sent,
)
.await?
{
info!(context, "Updating grpname for chat {chat_id}.");
@@ -2000,7 +2008,7 @@ async fn apply_group_changes(
info!(context, "Group-avatar change for {chat_id}.");
if chat
.param
.update_timestamp(Param::AvatarTimestamp, sent_timestamp)?
.update_timestamp(Param::AvatarTimestamp, mime_parser.timestamp_sent)?
{
match avatar_action {
AvatarAction::Change(profile_image) => {
@@ -2049,7 +2057,6 @@ async fn create_or_lookup_mailinglist(
allow_creation: bool,
list_id_header: &str,
mime_parser: &MimeMessage,
timestamp: i64,
) -> Result<Option<(ChatId, Blocked)>> {
let listid = mailinglist_header_listid(list_id_header)?;
@@ -2081,7 +2088,7 @@ async fn create_or_lookup_mailinglist(
blocked,
ProtectionStatus::Unprotected,
param,
timestamp,
mime_parser.timestamp_sent,
)
.await
.with_context(|| {
@@ -2169,7 +2176,6 @@ fn compute_mailinglist_name(
async fn apply_mailinglist_changes(
context: &Context,
mime_parser: &MimeMessage,
sent_timestamp: i64,
chat_id: ChatId,
) -> Result<()> {
let Some(mailinglist_header) = mime_parser.get_mailinglist_header() else {
@@ -2185,7 +2191,11 @@ async fn apply_mailinglist_changes(
let new_name = compute_mailinglist_name(mailinglist_header, listid, mime_parser);
if chat.name != new_name
&& chat_id
.update_timestamp(context, Param::GroupNameTimestamp, sent_timestamp)
.update_timestamp(
context,
Param::GroupNameTimestamp,
mime_parser.timestamp_sent,
)
.await?
{
info!(context, "Updating listname for chat {chat_id}.");
@@ -2266,7 +2276,6 @@ async fn create_adhoc_group(
mime_parser: &MimeMessage,
create_blocked: Blocked,
member_ids: &[ContactId],
timestamp: i64,
) -> Result<Option<ChatId>> {
if mime_parser.is_mailinglist_message() {
return Ok(None);
@@ -2305,7 +2314,7 @@ async fn create_adhoc_group(
create_blocked,
ProtectionStatus::Unprotected,
None,
timestamp,
mime_parser.timestamp_sent,
)
.await?;
@@ -2502,7 +2511,12 @@ async fn mark_recipients_as_verified(
Origin::Hidden,
)
.await?;
ChatId::set_protection_for_contact(context, to_contact_id).await?;
ChatId::set_protection_for_contact(
context,
to_contact_id,
mimeparser.timestamp_sent,
)
.await?;
}
}
} else {

19
src/securejoin.rs
View File

@@ -441,7 +441,13 @@ pub(crate) async fn handle_securejoin_handshake(
};
match chat::get_chat_id_by_grpid(context, field_grpid).await? {
Some((group_chat_id, _, _)) => {
secure_connection_established(context, contact_id, group_chat_id).await?;
secure_connection_established(
context,
contact_id,
group_chat_id,
mime_message.timestamp_sent,
)
.await?;
chat::add_contact_to_chat_ex(
context,
Nosync,
@@ -461,6 +467,7 @@ pub(crate) async fn handle_securejoin_handshake(
context,
contact_id,
info_chat_id(context, contact_id).await?,
mime_message.timestamp_sent,
)
.await?;
send_alice_handshake_msg(
@@ -609,7 +616,12 @@ pub(crate) async fn observe_securejoin_on_other_device(
peerstate.prefer_encrypt = EncryptPreference::Mutual;
peerstate.save_to_db(&context.sql).await.unwrap_or_default();
ChatId::set_protection_for_contact(context, contact_id).await?;
ChatId::set_protection_for_contact(
context,
contact_id,
mime_message.timestamp_sent,
)
.await?;
} else if let Some(fingerprint) =
mime_message.get_header(HeaderDef::SecureJoinFingerprint)
{
@@ -675,6 +687,7 @@ async fn secure_connection_established(
context: &Context,
contact_id: ContactId,
chat_id: ChatId,
timestamp: i64,
) -> Result<()> {
if context
.get_config_bool(Config::VerifiedOneOnOneChats)
@@ -687,7 +700,7 @@ async fn secure_connection_established(
.set_protection(
context,
ProtectionStatus::Protected,
time(),
timestamp,
Some(contact_id),
)
.await?;

8
src/securejoin/bob.rs
View File

@@ -130,7 +130,9 @@ pub(super) async fn handle_contact_confirm(
// Note this goes to the 1:1 chat, as when joining a group we implicitly also
// verify both contacts (this could be a bug/security issue, see
// e.g. https://github.com/deltachat/deltachat-core-rust/issues/1177).
bobstate.notify_peer_verified(context).await?;
bobstate
.notify_peer_verified(context, message.timestamp_sent)
.await?;
bobstate.emit_progress(context, JoinerProgress::Succeeded);
Ok(retval)
}
@@ -220,7 +222,7 @@ impl BobState {
/// Notifies the user that the SecureJoin peer is verified.
///
/// This creates an info message in the chat being joined.
async fn notify_peer_verified(&self, context: &Context) -> Result<()> {
async fn notify_peer_verified(&self, context: &Context, timestamp: i64) -> Result<()> {
let contact = Contact::get_by_id(context, self.invite().contact_id()).await?;
let chat_id = self.joining_chat_id(context).await?;
@@ -232,7 +234,7 @@ impl BobState {
.set_protection(
context,
ProtectionStatus::Protected,
time(),
timestamp,
Some(contact.id),
)
.await?;