From 518db9a20fd35aa9989638b668ddc7a17e9bcec1 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Tue, 7 May 2024 18:47:26 -0300 Subject: [PATCH] feat: Make one-to-one chats read-only the first seconds of a SecureJoin (#5512) This protects Bob (the joiner) of sending unexpected-unencrypted messages during an otherwise nicely running SecureJoin. If things get stuck, however, we do not want to block communication -- the chat is just opportunistic as usual, but that needs to be communicated: 1. If Bob's chat with Alice is `Unprotected` and a SecureJoin is started, then add info-message "Establishing guaranteed end-to-end encryption, please wait..." and let `Chat::can_send()` return `false`. 2. Once the info-message "Messages are guaranteed to be e2ee from now on" is added, let `Chat::can_send()` return `true`. 3. If after SECUREJOIN_WAIT_TIMEOUT seconds `2.` did not happen, add another info-message "Could not yet establish guaranteed end-to-end encryption but you may already send a message" and also let `Chat::can_send()` return `true`. Both `2.` and `3.` require the event `ChatModified` being sent out so that UI pick up the change wrt `Chat::can_send()` (this is the same way how groups become updated wrt `can_send()` changes). SECUREJOIN_WAIT_TIMEOUT should be 10-20 seconds so that we are reasonably sure that the app remains active and receiving also on mobile devices. If the app is killed during this time then we may need to do step 3 for any pending Bob-join chats (right now, Bob can only join one chat at a time). --- deltachat-jsonrpc/src/api/types/message.rs | 10 ++ src/chat.rs | 117 ++++++++++++++++++++- src/constants.rs | 5 + src/mimeparser.rs | 8 ++ src/securejoin.rs | 14 +-- src/securejoin/bob.rs | 28 ++++- src/securejoin/bobstate.rs | 9 ++ src/sql.rs | 32 +++--- src/stock_str.rs | 18 ++++ src/tests/verified_chats.rs | 10 +- 10 files changed, 221 insertions(+), 30 deletions(-) diff --git a/deltachat-jsonrpc/src/api/types/message.rs b/deltachat-jsonrpc/src/api/types/message.rs index 0c4aad4d5..05c8801ee 100644 --- a/deltachat-jsonrpc/src/api/types/message.rs +++ b/deltachat-jsonrpc/src/api/types/message.rs @@ -346,6 +346,14 @@ pub enum SystemMessageType { LocationOnly, InvalidUnencryptedMail, + /// 1:1 chats info message telling that SecureJoin has started and the user should wait for it + /// to complete. + SecurejoinWait, + + /// 1:1 chats info message telling that SecureJoin is still running, but the user may already + /// send messages. + SecurejoinWaitTimeout, + /// Chat ephemeral message timer is changed. EphemeralTimerChanged, @@ -386,6 +394,8 @@ impl From for SystemMessageType { SystemMessage::WebxdcStatusUpdate => SystemMessageType::WebxdcStatusUpdate, SystemMessage::WebxdcInfoMessage => SystemMessageType::WebxdcInfoMessage, SystemMessage::InvalidUnencryptedMail => SystemMessageType::InvalidUnencryptedMail, + SystemMessage::SecurejoinWait => SystemMessageType::SecurejoinWait, + SystemMessage::SecurejoinWaitTimeout => SystemMessageType::SecurejoinWaitTimeout, } } } diff --git a/src/chat.rs b/src/chat.rs index 13bf5328c..9c8dbe289 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -12,6 +12,7 @@ use deltachat_contact_tools::{strip_rtlo_characters, ContactAddress}; use deltachat_derive::{FromSql, ToSql}; use serde::{Deserialize, Serialize}; use strum_macros::EnumIter; +use tokio::task; use crate::aheader::EncryptPreference; use crate::blob::BlobObject; @@ -38,6 +39,7 @@ use crate::mimeparser::SystemMessage; use crate::param::{Param, Params}; use crate::peerstate::Peerstate; use crate::receive_imf::ReceivedMsg; +use crate::securejoin::BobState; use crate::smtp::send_msg_to_smtp; use crate::sql; use crate::stock_str; @@ -126,6 +128,10 @@ pub(crate) enum CantSendReason { /// Not a member of the chat. NotAMember, + + /// Temporary state for 1:1 chats while SecureJoin is in progress, after a timeout sending + /// messages (incl. unencrypted if we don't yet know the contact's pubkey) is allowed. + SecurejoinWait, } impl fmt::Display for CantSendReason { @@ -145,6 +151,7 @@ impl fmt::Display for CantSendReason { write!(f, "mailing list does not have a know post address") } Self::NotAMember => write!(f, "not a member of the chat"), + Self::SecurejoinWait => write!(f, "awaiting SecureJoin for 1:1 chat"), } } } @@ -610,7 +617,10 @@ impl ChatId { let sort_to_bottom = true; let ts = self .calc_sort_timestamp(context, timestamp_sent, sort_to_bottom, false) - .await?; + .await? + // Always sort protection messages below `SystemMessage::SecurejoinWait{,Timeout}` ones + // in case of race conditions. + .saturating_add(1); self.set_protection_for_timestamp_sort(context, protect, ts, contact_id) .await } @@ -1407,6 +1417,18 @@ impl ChatId { Ok(sort_timestamp) } + + /// Spawns a task checking after a timeout whether the SecureJoin has finished for the 1:1 chat + /// and otherwise notifying the user accordingly. + pub(crate) fn spawn_securejoin_wait(self, context: &Context, timeout: u64) { + let context = context.clone(); + task::spawn(async move { + tokio::time::sleep(Duration::from_secs(timeout)).await; + let chat = Chat::load_from_db(&context, self).await?; + chat.check_securejoin_wait(&context, 0).await?; + Result::<()>::Ok(()) + }); + } } impl std::fmt::Display for ChatId { @@ -1586,6 +1608,12 @@ impl Chat { Some(ReadOnlyMailingList) } else if !self.is_self_in_chat(context).await? { Some(NotAMember) + } else if self + .check_securejoin_wait(context, constants::SECUREJOIN_WAIT_TIMEOUT) + .await? + > 0 + { + Some(SecurejoinWait) } else { None }; @@ -1599,6 +1627,69 @@ impl Chat { Ok(self.why_cant_send(context).await?.is_none()) } + /// Returns the remaining timeout for the 1:1 chat in-progress SecureJoin. + /// + /// If the timeout has expired, notifies the user that sending messages is possible. See also + /// [`CantSendReason::SecurejoinWait`]. + pub(crate) async fn check_securejoin_wait( + &self, + context: &Context, + timeout: u64, + ) -> Result { + if self.typ != Chattype::Single || self.protected != ProtectionStatus::Unprotected { + return Ok(0); + } + let (mut param0, mut param1) = (Params::new(), Params::new()); + param0.set_cmd(SystemMessage::SecurejoinWait); + param1.set_cmd(SystemMessage::SecurejoinWaitTimeout); + let (param0, param1) = (param0.to_string(), param1.to_string()); + let Some((param, ts_sort, ts_start)) = context + .sql + .query_row_optional( + "SELECT param, timestamp, timestamp_sent FROM msgs WHERE id=\ + (SELECT MAX(id) FROM msgs WHERE chat_id=? AND param IN (?, ?))", + (self.id, ¶m0, ¶m1), + |row| { + let param: String = row.get(0)?; + let ts_sort: i64 = row.get(1)?; + let ts_start: i64 = row.get(2)?; + Ok((param, ts_sort, ts_start)) + }, + ) + .await? + else { + return Ok(0); + }; + if param == param1 { + return Ok(0); + } + let now = time(); + // Don't await SecureJoin if the clock was set back. + if ts_start <= now { + let timeout = ts_start + .saturating_add(timeout.try_into()?) + .saturating_sub(now); + if timeout > 0 { + return Ok(timeout as u64); + } + } + add_info_msg_with_cmd( + context, + self.id, + &stock_str::securejoin_wait_timeout(context).await, + SystemMessage::SecurejoinWaitTimeout, + // Use the sort timestamp of the "please wait" message, this way the added message is + // never sorted below the protection message if the SecureJoin finishes in parallel. + ts_sort, + Some(now), + None, + None, + ) + .await?; + context.emit_event(EventType::ChatModified(self.id)); + Ok(0) + } + /// Checks if the user is part of a chat /// and has basically the permissions to edit the chat therefore. /// The function does not check if the chat type allows editing of concrete elements. @@ -2334,6 +2425,26 @@ pub(crate) async fn update_special_chat_names(context: &Context) -> Result<()> { Ok(()) } +/// Checks if there is a 1:1 chat in-progress SecureJoin for Bob and, if necessary, schedules a task +/// unblocking the chat and notifying the user accordingly. +pub(crate) async fn resume_securejoin_wait(context: &Context) -> Result<()> { + let Some(bobstate) = BobState::from_db(&context.sql).await? else { + return Ok(()); + }; + if !bobstate.in_progress() { + return Ok(()); + } + let chat_id = bobstate.alice_chat(); + let chat = Chat::load_from_db(context, chat_id).await?; + let timeout = chat + .check_securejoin_wait(context, constants::SECUREJOIN_WAIT_TIMEOUT) + .await?; + if timeout > 0 { + chat_id.spawn_securejoin_wait(context, timeout); + } + Ok(()) +} + /// Handle a [`ChatId`] and its [`Blocked`] status at once. /// /// This struct is an optimisation to read a [`ChatId`] and its [`Blocked`] status at once @@ -2587,7 +2698,9 @@ async fn prepare_msg_common( if let Some(reason) = chat.why_cant_send(context).await? { if matches!( reason, - CantSendReason::ProtectionBroken | CantSendReason::ContactRequest + CantSendReason::ProtectionBroken + | CantSendReason::ContactRequest + | CantSendReason::SecurejoinWait ) && msg.param.get_cmd() == SystemMessage::SecurejoinMessage { // Send out the message, the securejoin message is supposed to repair the verification. diff --git a/src/constants.rs b/src/constants.rs index ce30520b7..deb7d9cdf 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -223,6 +223,11 @@ pub(crate) const DC_BACKGROUND_FETCH_QUOTA_CHECK_RATELIMIT: u64 = 12 * 60 * 60; /// in the group membership consistency algo to reject outdated membership changes. pub(crate) const TIMESTAMP_SENT_TOLERANCE: i64 = 60; +/// How long a 1:1 chat can't be used for sending while the SecureJoin is in progress. This should +/// be 10-20 seconds so that we are reasonably sure that the app remains active and receiving also +/// on mobile devices. See also [`crate::chat::CantSendReason::SecurejoinWait`]. +pub(crate) const SECUREJOIN_WAIT_TIMEOUT: u64 = 15; + #[cfg(test)] mod tests { use num_traits::FromPrimitive; diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 36df10780..8032ef45b 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -179,6 +179,14 @@ pub enum SystemMessage { /// which is sent by chatmail servers. InvalidUnencryptedMail = 13, + /// 1:1 chats info message telling that SecureJoin has started and the user should wait for it + /// to complete. + SecurejoinWait = 14, + + /// 1:1 chats info message telling that SecureJoin is still running, but the user may already + /// send messages. + SecurejoinWaitTimeout = 15, + /// Self-sent-message that contains only json used for multi-device-sync; /// if possible, we attach that to other messages as for locations. MultiDeviceSync = 20, diff --git a/src/securejoin.rs b/src/securejoin.rs index 3976cad85..edc6727ea 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -29,7 +29,7 @@ mod bob; mod bobstate; mod qrinvite; -use bobstate::BobState; +pub(crate) use bobstate::BobState; use qrinvite::QrInvite; use crate::token::Namespace; @@ -764,7 +764,7 @@ mod tests { use crate::constants::Chattype; use crate::imex::{imex, ImexMode}; use crate::receive_imf::receive_imf; - use crate::stock_str::chat_protection_enabled; + use crate::stock_str::{self, chat_protection_enabled}; use crate::test_utils::get_chat_msg; use crate::test_utils::{TestContext, TestContextManager}; use crate::tools::SystemTime; @@ -961,7 +961,7 @@ mod tests { let expected_text = chat_protection_enabled(&alice).await; assert_eq!(msg.get_text(), expected_text); if case == SetupContactCase::CheckProtectionTimestamp { - assert_eq!(msg.timestamp_sort, vc_request_with_auth_ts_sent); + assert_eq!(msg.timestamp_sort, vc_request_with_auth_ts_sent + 1); } } @@ -990,10 +990,12 @@ mod tests { // Check Bob got the verified message in his 1:1 chat. let chat = bob.create_chat(&alice).await; - let msg = get_chat_msg(&bob, chat.get_id(), 0, 1).await; + let msg = get_chat_msg(&bob, chat.get_id(), 0, 2).await; assert!(msg.is_info()); - let expected_text = chat_protection_enabled(&bob).await; - assert_eq!(msg.get_text(), expected_text); + assert_eq!(msg.get_text(), stock_str::securejoin_wait(&bob).await); + let msg = get_chat_msg(&bob, chat.get_id(), 1, 2).await; + assert!(msg.is_info()); + assert_eq!(msg.get_text(), chat_protection_enabled(&bob).await); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 106b49ae1..bd7a15e3e 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -9,11 +9,11 @@ use super::bobstate::{BobHandshakeStage, BobState}; use super::qrinvite::QrInvite; use super::HandshakeMessage; use crate::chat::{is_contact_in_chat, ChatId, ProtectionStatus}; -use crate::constants::{Blocked, Chattype}; +use crate::constants::{self, Blocked, Chattype}; use crate::contact::Contact; use crate::context::Context; use crate::events::EventType; -use crate::mimeparser::MimeMessage; +use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::sync::Sync::*; use crate::tools::{create_smeared_timestamp, time}; use crate::{chat, stock_str}; @@ -69,7 +69,29 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul QrInvite::Contact { .. } => { // For setup-contact the BobState already ensured the 1:1 chat exists because it // uses it to send the handshake messages. - Ok(state.alice_chat()) + let chat_id = state.alice_chat(); + // Calculate the sort timestamp before checking the chat protection status so that if we + // race with its change, we don't add our message below the protection message. + let sort_to_bottom = true; + let ts_sort = chat_id + .calc_sort_timestamp(context, 0, sort_to_bottom, false) + .await?; + if chat_id.is_protected(context).await? == ProtectionStatus::Unprotected { + let ts_start = time(); + chat::add_info_msg_with_cmd( + context, + chat_id, + &stock_str::securejoin_wait(context).await, + SystemMessage::SecurejoinWait, + ts_sort, + Some(ts_start), + None, + None, + ) + .await?; + chat_id.spawn_securejoin_wait(context, constants::SECUREJOIN_WAIT_TIMEOUT); + } + Ok(chat_id) } } } diff --git a/src/securejoin/bobstate.rs b/src/securejoin/bobstate.rs index c9bf390a0..708987fb5 100644 --- a/src/securejoin/bobstate.rs +++ b/src/securejoin/bobstate.rs @@ -341,6 +341,15 @@ impl BobState { async fn send_handshake_message(&self, context: &Context, step: BobHandshakeMsg) -> Result<()> { send_handshake_message(context, &self.invite, self.chat_id, step).await } + + /// Returns whether we are waiting for a SecureJoin message from Alice, i.e. the protocol hasn't + /// yet completed. + pub(crate) fn in_progress(&self) -> bool { + !matches!( + self.next, + SecureJoinStep::Terminated | SecureJoinStep::Completed + ) + } } /// Sends the requested handshake message to Alice. diff --git a/src/sql.rs b/src/sql.rs index 8300a2c33..7c2779f53 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -8,7 +8,7 @@ use rusqlite::{config::DbConfig, types::ValueRef, Connection, OpenFlags, Row}; use tokio::sync::{Mutex, MutexGuard, RwLock}; use crate::blob::BlobObject; -use crate::chat::{add_device_msg, update_device_icon, update_saved_messages_icon}; +use crate::chat::{self, add_device_msg, update_device_icon, update_saved_messages_icon}; use crate::config::Config; use crate::constants::DC_CHAT_ID_TRASH; use crate::context::Context; @@ -289,21 +289,23 @@ impl Sql { let passphrase_nonempty = !passphrase.is_empty(); if let Err(err) = self.try_open(context, &self.dbfile, passphrase).await { self.close().await; - Err(err) - } else { - info!(context, "Opened database {:?}.", self.dbfile); - *self.is_encrypted.write().await = Some(passphrase_nonempty); - - // setup debug logging if there is an entry containing its id - if let Some(xdc_id) = self - .get_raw_config_u32(Config::DebugLogging.as_ref()) - .await? - { - set_debug_logging_xdc(context, Some(MsgId::new(xdc_id))).await?; - } - - Ok(()) + return Err(err); } + info!(context, "Opened database {:?}.", self.dbfile); + *self.is_encrypted.write().await = Some(passphrase_nonempty); + + // setup debug logging if there is an entry containing its id + if let Some(xdc_id) = self + .get_raw_config_u32(Config::DebugLogging.as_ref()) + .await? + { + set_debug_logging_xdc(context, Some(MsgId::new(xdc_id))).await?; + } + chat::resume_securejoin_wait(context) + .await + .log_err(context) + .ok(); + Ok(()) } /// Changes the passphrase of encrypted database. diff --git a/src/stock_str.rs b/src/stock_str.rs index 579982571..984a658f8 100644 --- a/src/stock_str.rs +++ b/src/stock_str.rs @@ -435,6 +435,14 @@ pub enum StockMessage { #[strum(props(fallback = "%1$s reacted %2$s to \"%3$s\""))] MsgReactedBy = 177, + + #[strum(props(fallback = "Establishing guaranteed end-to-end encryption, please wait…"))] + SecurejoinWait = 190, + + #[strum(props( + fallback = "Could not yet establish guaranteed end-to-end encryption, but you may already send a message." + ))] + SecurejoinWaitTimeout = 191, } impl StockMessage { @@ -842,6 +850,16 @@ pub(crate) async fn secure_join_replies(context: &Context, contact_id: ContactId .replace1(&contact_id.get_stock_name(context).await) } +/// Stock string: `Establishing guaranteed end-to-end encryption, please wait…`. +pub(crate) async fn securejoin_wait(context: &Context) -> String { + translated(context, StockMessage::SecurejoinWait).await +} + +/// Stock string: `Could not yet establish guaranteed end-to-end encryption, but you may already send a message.`. +pub(crate) async fn securejoin_wait_timeout(context: &Context) -> String { + translated(context, StockMessage::SecurejoinWaitTimeout).await +} + /// Stock string: `Scan to chat with %1$s`. pub(crate) async fn setup_contact_qr_description( context: &Context, diff --git a/src/tests/verified_chats.rs b/src/tests/verified_chats.rs index 864c02b4c..06ee18441 100644 --- a/src/tests/verified_chats.rs +++ b/src/tests/verified_chats.rs @@ -376,7 +376,7 @@ async fn test_old_message_2() -> Result<()> { SystemMessage::ChatProtectionEnabled ); - // This creates protection-changed info message #2. + // This creates protection-changed info message #2 with `timestamp_sort` greater by 1. let first_email = receive_imf( &alice, b"From: Bob \n\ @@ -390,8 +390,7 @@ async fn test_old_message_2() -> Result<()> { .await? .unwrap(); - // Both messages will get the same timestamp as the protection-changed - // message, so this one will be sorted under the previous one + // Both messages will get the same timestamp, so this one will be sorted under the previous one // even though it has an older timestamp. let second_email = receive_imf( &alice, @@ -407,7 +406,10 @@ async fn test_old_message_2() -> Result<()> { .unwrap(); assert_eq!(first_email.sort_timestamp, second_email.sort_timestamp); - assert_eq!(first_email.sort_timestamp, protection_msg.timestamp_sort); + assert_eq!( + first_email.sort_timestamp, + protection_msg.timestamp_sort + 1 + ); alice.golden_test_chat(chat.id, "test_old_message_2").await;