Skip to content

Commit

Permalink
feat: Add device message about outgoing undecryptable messages (#5164)
Browse files Browse the repository at this point in the history
Currently when a user sets up another device by logging in, a new key is created. If a message is
sent from either device outside, it cannot be decrypted by the other device.

The message is replaced with square bracket error like this:
```
<string name="systemmsg_cannot_decrypt">This message cannot be decrypted.\n\n• It might already help to simply reply to this message and ask the sender to send the message again.\n\n• If you just re-installed Delta Chat then it is best if you re-setup Delta Chat now and choose "Add as second device" or import a backup.</string>
```
(taken from Android repo `res/values/strings.xml`)

If the message is outgoing, it does not help to "simply reply to this message". Instead, we should
add a translatable device message of a special type so UI can link to the FAQ entry about second
device. But let's limit such notifications to 1 per day. And as for the undecryptable message
itself, let it go to Trash if it can't be assigned to a chat by its references.
  • Loading branch information
iequidoo committed Feb 8, 2024
1 parent ec9d104 commit 91f52c6
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 18 deletions.
1 change: 1 addition & 0 deletions node/test/test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ describe('Basic offline Tests', function () {
'journal_mode',
'key_gen_type',
'last_housekeeping',
'last_cant_decrypt_outgoing_msgs',
'level',
'mdns_enabled',
'media_quality',
Expand Down
3 changes: 3 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,9 @@ pub enum Config {
/// Timestamp of the last time housekeeping was run
LastHousekeeping,

/// Timestamp of the last `CantDecryptOutgoingMsgs` notification.
LastCantDecryptOutgoingMsgs,

/// To how many seconds to debounce scan_all_folders. Used mainly in tests, to disable debouncing completely.
#[strum(props(default = "60"))]
ScanAllFoldersDebounceSecs,
Expand Down
6 changes: 6 additions & 0 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,12 @@ impl Context {
.await?
.to_string(),
);
res.insert(
"last_cant_decrypt_outgoing_msgs",
self.get_config_int(Config::LastCantDecryptOutgoingMsgs)
.await?
.to_string(),
);
res.insert(
"scan_all_folders_debounce_secs",
self.get_config_int(Config::ScanAllFoldersDebounceSecs)
Expand Down
4 changes: 4 additions & 0 deletions src/mimeparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ pub(crate) struct MimeMessage {
/// Whether the From address was repeated in the signed part
/// (and we know that the signer intended to send from this address)
pub from_is_signed: bool,
/// Whether the message is incoming or outgoing (self-sent).
pub incoming: bool,
/// The List-Post address is only set for mailing lists. Users can send
/// messages to this address to post them to the list.
pub list_post: Option<String>,
Expand Down Expand Up @@ -396,13 +398,15 @@ impl MimeMessage {
}
}

let incoming = !context.is_self_addr(&from.addr).await?;
let mut parser = MimeMessage {
parts: Vec::new(),
headers,
recipients,
list_post,
from,
from_is_signed,
incoming,
chat_disposition_notification_to,
decryption_info,
decrypting_failed: mail.is_err(),
Expand Down
59 changes: 43 additions & 16 deletions src/receive_imf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::simplify;
use crate::sql;
use crate::stock_str;
use crate::sync::Sync::*;
use crate::tools::{buf_compress, extract_grpid_from_rfc724_mid, strip_rtlo_characters};
use crate::tools::{self, buf_compress, extract_grpid_from_rfc724_mid, strip_rtlo_characters};
use crate::{contact, imap};

/// This is the struct that is returned after receiving one email (aka MIME message).
Expand Down Expand Up @@ -220,7 +220,6 @@ pub(crate) async fn receive_imf_inner(
context,
"Receiving message {rfc724_mid_orig:?}, seen={seen}...",
);
let incoming = !context.is_self_addr(&mime_parser.from.addr).await?;

// check, if the mail is already in our database.
// make sure, this check is done eg. before securejoin-processing.
Expand Down Expand Up @@ -278,7 +277,7 @@ pub(crate) async fn receive_imf_inner(
// Need to update chat id in the db.
} else if let Some(msg_id) = replace_msg_id {
info!(context, "Message is already downloaded.");
if incoming {
if mime_parser.incoming {
return Ok(None);
}
// For the case if we missed a successful SMTP response. Be optimistic that the message is
Expand Down Expand Up @@ -331,7 +330,7 @@ pub(crate) async fn receive_imf_inner(
let to_ids = add_or_lookup_contacts_by_address_list(
context,
&mime_parser.recipients,
if !incoming {
if !mime_parser.incoming {
Origin::OutgoingTo
} else if incoming_origin.is_known() {
Origin::IncomingTo
Expand All @@ -346,7 +345,7 @@ pub(crate) async fn receive_imf_inner(
let received_msg;
if mime_parser.get_header(HeaderDef::SecureJoin).is_some() {
let res;
if incoming {
if mime_parser.incoming {
res = handle_securejoin_handshake(context, &mime_parser, from_id)
.await
.context("error in Secure-Join message handling")?;
Expand Down Expand Up @@ -413,7 +412,6 @@ pub(crate) async fn receive_imf_inner(
context,
&mut mime_parser,
imf_raw,
incoming,
&to_ids,
rfc724_mid_orig,
from_id,
Expand Down Expand Up @@ -571,7 +569,7 @@ pub(crate) async fn receive_imf_inner(
} else if !chat_id.is_trash() {
let fresh = received_msg.state == MessageState::InFresh;
for msg_id in &received_msg.msg_ids {
chat_id.emit_msg_event(context, *msg_id, incoming && fresh);
chat_id.emit_msg_event(context, *msg_id, mime_parser.incoming && fresh);
}
}
context.new_msgs_notify.notify_one();
Expand Down Expand Up @@ -647,7 +645,6 @@ async fn add_parts(
context: &Context,
mime_parser: &mut MimeMessage,
imf_raw: &[u8],
incoming: bool,
to_ids: &[ContactId],
rfc724_mid: &str,
from_id: ContactId,
Expand Down Expand Up @@ -715,8 +712,9 @@ async fn add_parts(
// (of course, the user can add other chats manually later)
let to_id: ContactId;
let state: MessageState;
let mut hidden = false;
let mut needs_delete_job = false;
if incoming {
if mime_parser.incoming {
to_id = ContactId::SELF;

let test_normal_chat = if from_id == ContactId::UNDEFINED {
Expand Down Expand Up @@ -1013,6 +1011,34 @@ async fn add_parts(
}
}

if mime_parser.decrypting_failed && !fetching_existing_messages {
if chat_id.is_none() {
chat_id = Some(DC_CHAT_ID_TRASH);
} else {
hidden = true;
}
let last_time = context
.get_config_i64(Config::LastCantDecryptOutgoingMsgs)
.await?;
let now = tools::time();
let update_config = if last_time.saturating_add(24 * 60 * 60) <= now {
let mut msg = Message::new(Viewtype::Text);
msg.text = stock_str::cant_decrypt_outgoing_msgs(context).await;
chat::add_device_msg(context, None, Some(&mut msg))
.await
.log_err(context)
.ok();
true
} else {
last_time > now
};
if update_config {
context
.set_config(Config::LastCantDecryptOutgoingMsgs, Some(&now.to_string()))
.await?;
}
}

if !to_ids.is_empty() {
if chat_id.is_none() {
if let Some((new_chat_id, new_chat_id_blocked)) = create_or_lookup_group(
Expand Down Expand Up @@ -1155,7 +1181,7 @@ async fn add_parts(
context,
mime_parser.timestamp_sent,
sort_to_bottom,
incoming,
mime_parser.incoming,
)
.await?;

Expand Down Expand Up @@ -1249,7 +1275,7 @@ async fn add_parts(
// -> Showing info messages everytime would be a lot of noise
// 3. The info messages that are shown to the user ("Your chat partner
// likely reinstalled DC" or similar) would be wrong.
if chat.is_protected() && (incoming || chat.typ != Chattype::Single) {
if chat.is_protected() && (mime_parser.incoming || chat.typ != Chattype::Single) {
if let VerifiedEncryption::NotVerified(err) = verified_encryption {
warn!(context, "Verification problem: {err:#}.");
let s = format!("{err}. See 'Info' for more details");
Expand Down Expand Up @@ -1415,7 +1441,7 @@ INSERT INTO msgs
rfc724_mid, chat_id,
from_id, to_id, timestamp, timestamp_sent,
timestamp_rcvd, type, state, msgrmsg,
txt, subject, txt_raw, param,
txt, subject, txt_raw, param, hidden,
bytes, mime_headers, mime_compressed, mime_in_reply_to,
mime_references, mime_modified, error, ephemeral_timer,
ephemeral_timestamp, download_state, hop_info
Expand All @@ -1424,7 +1450,7 @@ INSERT INTO msgs
?,
?, ?, ?, ?,
?, ?, ?, ?,
?, ?, ?, ?,
?, ?, ?, ?, ?,
?, ?, ?, ?, 1,
?, ?, ?, ?,
?, ?, ?, ?
Expand All @@ -1434,7 +1460,7 @@ SET rfc724_mid=excluded.rfc724_mid, chat_id=excluded.chat_id,
from_id=excluded.from_id, to_id=excluded.to_id, timestamp_sent=excluded.timestamp_sent,
type=excluded.type, msgrmsg=excluded.msgrmsg,
txt=excluded.txt, subject=excluded.subject, txt_raw=excluded.txt_raw, param=excluded.param,
bytes=excluded.bytes, mime_headers=excluded.mime_headers,
hidden=excluded.hidden,bytes=excluded.bytes, mime_headers=excluded.mime_headers,
mime_compressed=excluded.mime_compressed, mime_in_reply_to=excluded.mime_in_reply_to,
mime_references=excluded.mime_references, mime_modified=excluded.mime_modified, error=excluded.error, ephemeral_timer=excluded.ephemeral_timer,
ephemeral_timestamp=excluded.ephemeral_timestamp, download_state=excluded.download_state, hop_info=excluded.hop_info
Expand All @@ -1461,6 +1487,7 @@ RETURNING id
} else {
param.to_string()
},
hidden,
part.bytes as isize,
if (save_mime_headers || mime_modified) && !trash {
mime_headers.clone()
Expand Down Expand Up @@ -1526,7 +1553,7 @@ RETURNING id
);

// new outgoing message from another device marks the chat as noticed.
if !incoming && !chat_id.is_special() {
if !mime_parser.incoming && !chat_id.is_special() {
chat::marknoticed_chat_if_older_than(context, chat_id, sort_timestamp).await?;
}

Expand All @@ -1549,7 +1576,7 @@ RETURNING id
}
}

if !incoming && is_mdn && is_dc_message == MessengerMessage::Yes {
if !mime_parser.incoming && is_mdn && is_dc_message == MessengerMessage::Yes {
// Normally outgoing MDNs sent by us never appear in mailboxes, but Gmail saves all
// outgoing messages, including MDNs, to the Sent folder. If we detect such saved MDN,
// delete it.
Expand Down
49 changes: 49 additions & 0 deletions src/receive_imf/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,24 @@ async fn test_grpid_simple() {
let mimeparser = MimeMessage::from_bytes(&context.ctx, &raw[..], None)
.await
.unwrap();
assert_eq!(mimeparser.incoming, true);
assert_eq!(extract_grpid(&mimeparser, HeaderDef::InReplyTo), None);
let grpid = Some("HcxyMARjyJy");
assert_eq!(extract_grpid(&mimeparser, HeaderDef::References), grpid);
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_outgoing() -> Result<()> {
let context = TestContext::new_alice().await;
let raw = b"Received: (Postfix, from userid 1000); Mon, 4 Dec 2006 14:51:39 +0100 (CET)\n\
From: alice@example.org\n\
\n\
hello";
let mimeparser = MimeMessage::from_bytes(&context.ctx, &raw[..], None).await?;
assert_eq!(mimeparser.incoming, false);
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_bad_from() {
let context = TestContext::new_alice().await;
Expand Down Expand Up @@ -3219,6 +3232,42 @@ async fn test_blocked_contact_creates_group() -> Result<()> {
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_outgoing_undecryptable() -> Result<()> {
let alice = &TestContext::new().await;
alice.configure_addr("alice@example.org").await;

let raw = include_bytes!("../../test-data/message/thunderbird_with_autocrypt.eml");
receive_imf(alice, raw, false).await?;

let bob_contact_id = Contact::lookup_id_by_addr(alice, "bob@example.net", Origin::OutgoingTo)
.await?
.unwrap();
assert!(ChatId::lookup_by_contact(alice, bob_contact_id)
.await?
.is_none());

let dev_chat_id = ChatId::lookup_by_contact(alice, ContactId::DEVICE)
.await?
.unwrap();
let dev_msg = alice.get_last_msg_in(dev_chat_id).await;
assert!(dev_msg.error().is_none());
assert!(dev_msg
.text
.contains(&stock_str::cant_decrypt_outgoing_msgs(alice).await));

let raw = include_bytes!("../../test-data/message/thunderbird_encrypted_signed.eml");
receive_imf(alice, raw, false).await?;

assert!(ChatId::lookup_by_contact(alice, bob_contact_id)
.await?
.is_none());
// The device message mustn't be added too frequently.
assert_eq!(alice.get_last_msg_in(dev_chat_id).await.id, dev_msg.id);

Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_thunderbird_autocrypt() -> Result<()> {
let t = TestContext::new_bob().await;
Expand Down
10 changes: 10 additions & 0 deletions src/stock_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,11 @@ pub enum StockMessage {
fallback = "⚠️ Your email provider %1$s requires end-to-end encryption which is not setup yet."
))]
InvalidUnencryptedMail = 174,

#[strum(props(
fallback = "⚠️ It seems you are using Delta Chat on multiple devices that cannot decrypt each other's outgoing messages. To fix this, on the older device use \"Settings / Add Second Device\" and follow the instructions."
))]
CantDecryptOutgoingMsgs = 175,
}

impl StockMessage {
Expand Down Expand Up @@ -750,6 +755,11 @@ pub(crate) async fn cant_decrypt_msg_body(context: &Context) -> String {
translated(context, StockMessage::CantDecryptMsgBody).await
}

/// Stock string:`Got outgoing message(s) encrypted for another setup...`.
pub(crate) async fn cant_decrypt_outgoing_msgs(context: &Context) -> String {
translated(context, StockMessage::CantDecryptOutgoingMsgs).await
}

/// Stock string: `Fingerprints`.
pub(crate) async fn finger_prints(context: &Context) -> String {
translated(context, StockMessage::FingerPrints).await
Expand Down
2 changes: 0 additions & 2 deletions test-data/message/thunderbird_encrypted_signed.eml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
Content-Language: en-US
To: bob@example.net
From: Alice <alice@example.org>
X-Mozilla-Draft-Info: internal/draft; vcard=0; receipt=0; DSN=0; uuencode=0;
attachmentreminder=0; deliveryformat=0
X-Identity-Key: id3
Fcc: imap://alice%40example.org@in.example.org/Sent
Subject: ...
Expand Down

0 comments on commit 91f52c6

Please sign in to comment.