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, it's unlikely that this will break subsequent messages chat assignment,
anyway this undecryptable message wouldn't be assigned to the correct chat as well.
  • Loading branch information
iequidoo committed Jan 14, 2024
1 parent b5c0372 commit f881b20
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 16 deletions.
1 change: 1 addition & 0 deletions node/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,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 @@ -746,6 +746,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 @@ -390,6 +392,7 @@ impl MimeMessage {
}
}

let incoming = !context.is_self_addr(&from.addr).await?;
// 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");

Expand All @@ -406,6 +409,7 @@ impl MimeMessage {
list_post,
from,
from_is_signed,
incoming,
chat_disposition_notification_to,
decryption_info,
decrypting_failed: mail.is_err(),
Expand Down
50 changes: 36 additions & 14 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 @@ -191,11 +191,10 @@ 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?;

// For the case if we missed a successful SMTP response. Be optimistic that the message is
// delivered also.
let delivered = !incoming && {
let delivered = !mime_parser.incoming && {
let self_addr = context.get_primary_self_addr().await?;
context
.sql
Expand Down Expand Up @@ -290,7 +289,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 @@ -305,7 +304,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 @@ -372,7 +371,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 @@ -530,7 +528,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 @@ -604,7 +602,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 @@ -673,7 +670,7 @@ async fn add_parts(
let to_id: ContactId;
let state: MessageState;
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 @@ -1064,10 +1061,35 @@ async fn add_parts(
}
}

if fetching_existing_messages && mime_parser.decrypting_failed {
if !mime_parser.decrypting_failed { // Nothing special to do.
} else if fetching_existing_messages {
chat_id = Some(DC_CHAT_ID_TRASH);
// We are only gathering old messages on first start. We do not want to add loads of non-decryptable messages to the chats.
info!(context, "Existing non-decipherable message (TRASH).");
} else if !mime_parser.incoming {
if chat_id.is_none() {
chat_id = Some(DC_CHAT_ID_TRASH);
}
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 mime_parser.webxdc_status_update.is_some() && mime_parser.parts.len() == 1 {
Expand Down Expand Up @@ -1112,7 +1134,7 @@ async fn add_parts(
context,
mime_parser.timestamp_sent,
sort_to_bottom,
incoming,
mime_parser.incoming,
)
.await?;

Expand Down Expand Up @@ -1206,7 +1228,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 @@ -1483,7 +1505,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 @@ -1506,7 +1528,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
48 changes: 48 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: [email protected]\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 @@ -3139,6 +3152,41 @@ 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("[email protected]").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, "[email protected]", Origin::OutgoingTo)
.await?
.unwrap();
let bob_chat_id = ChatId::lookup_by_contact(alice, bob_contact_id)
.await?
.unwrap();
assert!(chat::get_chat_msgs(alice, bob_chat_id).await?.is_empty());

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!(chat::get_chat_msgs(alice, bob_chat_id).await?.is_empty());
// 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 @@ -419,6 +419,11 @@ pub enum StockMessage {

#[strum(props(fallback = "Member %1$s added."))]
MsgAddMember = 173,

#[strum(props(
fallback = "Got outgoing message(s) encrypted for another setup. It seems you are using Delta Chat on multiple devices, but they cannot decrypt each other messages. You can use \"Add as Second Device\" feature to fix this."
))]
CantDecryptOutgoingMsgs = 174,
}

impl StockMessage {
Expand Down Expand Up @@ -745,6 +750,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: [email protected]
From: Alice <[email protected]>
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%[email protected]/Sent
Subject: ...
Expand Down

0 comments on commit f881b20

Please sign in to comment.