From c0b86f5c7fc05fed1f2771ca32a506ce7d3a5889 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 15 Jan 2026 14:19:35 +0100 Subject: [PATCH 01/36] Send v=3 in QR codes --- src/securejoin.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/securejoin.rs b/src/securejoin.rs index 8bf532d692..5286b25161 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -178,11 +178,11 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul if chat.typ == Chattype::OutBroadcast { // For historic reansons, broadcasts currently use j instead of i for the invitenumber. format!( - "https://i.delta.chat/#{fingerprint}&x={grpid}&j={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&b={chat_name_urlencoded}", + "https://i.delta.chat/#{fingerprint}&v=3&x={grpid}&j={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&b={chat_name_urlencoded}", ) } else { format!( - "https://i.delta.chat/#{fingerprint}&x={grpid}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&g={chat_name_urlencoded}", + "https://i.delta.chat/#{fingerprint}&v=3&x={grpid}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&g={chat_name_urlencoded}", ) } } else { @@ -195,7 +195,7 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul context.scheduler.interrupt_smtp().await; } format!( - "https://i.delta.chat/#{fingerprint}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}", + "https://i.delta.chat/#{fingerprint}&v=3&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}", ) }; From ad584d2857ba3c7637b0d51e113bae414f817d23 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 20 Jan 2026 10:39:54 +0100 Subject: [PATCH 02/36] [wip, not compiling] Add render_symm_encrypted_securejoin_message() --- src/mimefactory.rs | 285 +++++++++++++++++++++++++++++++++++++ src/securejoin/bob.rs | 180 +++++++++++++++++------ src/securejoin/qrinvite.rs | 13 +- 3 files changed, 436 insertions(+), 42 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 562ab23f58..f2eb6508e6 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -2205,5 +2205,290 @@ fn b_encode(value: &str) -> String { ) } +pub(crate) async fn render_symm_encrypted_securejoin_message( + context: &Context, + step: &str, + rfc724_mid: &str, + attach_self_pubkey: bool, + auth: &str, +) -> Result { + info!(context, "Sending secure-join message {step:?}."); + + let mut headers = Vec::<(&'static str, HeaderType<'static>)>::new(); + + let from_addr = context.get_primary_self_addr().await?; + let from = new_address_with_name("", from_addr); + + let mut to: Vec> = Vec::new(); + to.push(hidden_recipients()); + + headers.push(("From", from.into())); + + headers.push(( + "To", + mail_builder::headers::address::Address::new_list(to.clone()).into(), + )); + + // TODO not sure if we even need a timestamp + let timestamp = create_smeared_timestamp(context); + let date = chrono::DateTime::::from_timestamp(timestamp, 0) + .unwrap() + .to_rfc2822(); + headers.push(("Date", mail_builder::headers::raw::Raw::new(date).into())); + + headers.push(( + "Message-ID", + mail_builder::headers::message_id::MessageId::new(rfc724_mid.to_string()).into(), + )); + + // Automatic Response headers + if context.get_config_bool(Config::Bot).await? { + headers.push(( + "Auto-Submitted", + mail_builder::headers::raw::Raw::new("auto-generated".to_string()).into(), + )); + } else if step != "vc-request" { + headers.push(( + "Auto-Submitted", + mail_builder::headers::raw::Raw::new("auto-replied".to_string()).into(), + )); + } + + let encrypt_helper = EncryptHelper::new(context).await?; + + if attach_self_pubkey { + let aheader = encrypt_helper.get_aheader().to_string(); + headers.push(( + "Autocrypt", + mail_builder::headers::raw::Raw::new(aheader).into(), + )); + } + + headers.push(( + "Secure-Join", + mail_builder::headers::raw::Raw::new(step.to_string()).into(), + )); + + headers.push(( + "Secure-Join-Auth", + mail_builder::headers::text::Text::new(auth.to_string()).into(), + )); + + let message: MimePart<'static> = MimePart::new("text/plain", "Secure-Join"); + + // Split headers based on header confidentiality policy. + + // Headers that must go into IMF header section. + // + // These are standard headers such as Date, In-Reply-To, References, which cannot be placed + // anywhere else according to the standard. Placing headers here also allows them to be fetched + // individually over IMAP without downloading the message body. This is why Chat-Version is + // placed here. + let mut unprotected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); + + // Headers that MUST NOT (only) go into IMF header section: + // - Large headers which may hit the header section size limit on the server, such as + // Chat-User-Avatar with a base64-encoded image inside. + // - Headers duplicated here that servers mess up with in the IMF header section, like + // Message-ID. + // - Nonstandard headers that should be DKIM-protected because e.g. OpenDKIM only signs + // known headers. + // + // The header should be hidden from MTA + // by moving it either into protected part + // in case of encrypted mails + // or unprotected MIME preamble in case of unencrypted mails. + let mut hidden_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); + + // Opportunistically protected headers. + // + // These headers are placed into encrypted part *if* the message is encrypted. Place headers + // which are not needed before decryption (e.g. Chat-Group-Name) or are not interesting if the + // message cannot be decrypted (e.g. Chat-Disposition-Notification-To) here. + // + // If the message is not encrypted, these headers are placed into IMF header section, so make + // sure that the message will be encrypted if you place any sensitive information here. + let mut protected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); + + // MIME header . + unprotected_headers.push(( + "MIME-Version", + mail_builder::headers::raw::Raw::new("1.0").into(), + )); + + for header @ (original_header_name, _header_value) in &headers { + let header_name = original_header_name.to_lowercase(); + if header_name == "message-id" { + unprotected_headers.push(header.clone()); + hidden_headers.push(header.clone()); + } else if is_hidden(&header_name) { + hidden_headers.push(header.clone()); + } else if header_name == "from" { + protected_headers.push(header.clone()); + + unprotected_headers.push(header.clone()); + } else if header_name == "to" { + unprotected_headers.push(("To", hidden_recipients().into())); + } else if header_name == "date" { + protected_headers.push(header.clone()); + + // Randomized date goes to unprotected header. + // + // We cannot just send "Thu, 01 Jan 1970 00:00:00 +0000" + // or omit the header because GMX then fails with + // + // host mx00.emig.gmx.net[212.227.15.9] said: + // 554-Transaction failed + // 554-Reject due to policy restrictions. + // 554 For explanation visit https://postmaster.gmx.net/en/case?... + // (in reply to end of DATA command) + // + // and the explanation page says + // "The time information deviates too much from the actual time". + // + // We also limit the range to 6 days (518400 seconds) + // because with a larger range we got + // error "500 Date header far in the past/future" + // which apparently originates from Symantec Messaging Gateway + // and means the message has a Date that is more + // than 7 days in the past: + // + let timestamp_offset = rand::random_range(0..518400); + let protected_timestamp = timestamp.saturating_sub(timestamp_offset); + let unprotected_date = + chrono::DateTime::::from_timestamp(protected_timestamp, 0) + .unwrap() + .to_rfc2822(); + unprotected_headers.push(( + "Date", + mail_builder::headers::raw::Raw::new(unprotected_date).into(), + )); + } else { + protected_headers.push(header.clone()); + + match header_name.as_str() { + "subject" => { + unprotected_headers.push(( + "Subject", + mail_builder::headers::raw::Raw::new("[...]").into(), + )); + } + "in-reply-to" + | "references" + | "auto-submitted" + | "chat-version" + | "autocrypt-setup-message" => { + unprotected_headers.push(header.clone()); + } + _ => { + // Other headers are removed from unprotected part. + } + } + } + } + + let outer_message = { + // Store protected headers in the inner message. + let message = protected_headers + .into_iter() + .fold(message, |message, (header, value)| { + message.header(header, value) + }); + + // Add hidden headers to encrypted payload. + let mut message: MimePart<'static> = hidden_headers + .into_iter() + .fold(message, |message, (header, value)| { + message.header(header, value) + }); + + message = unprotected_headers + .iter() + // Structural headers shouldn't be added as "HP-Outer". They are defined in + // . + .filter(|(name, _)| { + !(name.eq_ignore_ascii_case("mime-version") + || name.eq_ignore_ascii_case("content-type") + || name.eq_ignore_ascii_case("content-transfer-encoding") + || name.eq_ignore_ascii_case("content-disposition")) + }) + .fold(message, |message, (name, value)| { + message.header(format!("HP-Outer: {name}"), value.clone()) + }); + + // Set the appropriate Content-Type for the inner message. + for (h, v) in &mut message.headers { + if h == "Content-Type" + && let mail_builder::headers::HeaderType::ContentType(ct) = v + { + let mut ct_new = ct.clone(); + ct_new = ct_new.attribute("protected-headers", "v1"); + ct_new = ct_new.attribute("hp", "cipher"); + *ct = ct_new; + break; + } + } + + // Disable compression for SecureJoin to ensure + // there are no compression side channels + // leaking information about the tokens. + let compress = false; + + if context.get_config_bool(Config::TestHooks).await? + && let Some(hook) = &*context.pre_encrypt_mime_hook.lock() + { + message = hook(context, message); + } + + let encrypted = encrypt_helper + .encrypt_symmetrically(context, auth, message, compress) + .await?; + + // XXX: additional newline is needed + // to pass filtermail at + // : + let encrypted = encrypted + "\n"; + + // Set the appropriate Content-Type for the outer message + MimePart::new( + "multipart/encrypted; protocol=\"application/pgp-encrypted\"", + vec![ + // Autocrypt part 1 + MimePart::new("application/pgp-encrypted", "Version: 1\r\n").header( + "Content-Description", + mail_builder::headers::raw::Raw::new("PGP/MIME version identification"), + ), + // Autocrypt part 2 + MimePart::new( + "application/octet-stream; name=\"encrypted.asc\"", + encrypted, + ) + .header( + "Content-Description", + mail_builder::headers::raw::Raw::new("OpenPGP encrypted message"), + ) + .header( + "Content-Disposition", + mail_builder::headers::raw::Raw::new("inline; filename=\"encrypted.asc\";"), + ), + ], + ) + }; + + // Store the unprotected headers on the outer message. + let outer_message = unprotected_headers + .into_iter() + .fold(outer_message, |message, (header, value)| { + message.header(header, value) + }); + + let mut buffer = Vec::new(); + let cursor = Cursor::new(&mut buffer); + outer_message.clone().write_part(cursor).ok(); + let message = String::from_utf8_lossy(&buffer).to_string(); + + Ok(message) +} + #[cfg(test)] mod mimefactory_tests; diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index fd92985625..e754f8ddb1 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -1,13 +1,12 @@ //! Bob's side of SecureJoin handling, the joiner-side. -use anyhow::{Context as _, Result}; +use anyhow::{Context as _, Result, bail}; use super::HandshakeMessage; use super::qrinvite::QrInvite; use crate::chat::{self, ChatId, is_contact_in_chat}; -use crate::chatlist_events; use crate::constants::{Blocked, Chattype}; -use crate::contact::Origin; +use crate::contact::{Contact, Origin}; use crate::context::Context; use crate::events::EventType; use crate::key::self_fingerprint; @@ -18,7 +17,8 @@ use crate::param::{Param, Params}; use crate::securejoin::{ContactId, encrypted_and_signed, verify_sender_by_fingerprint}; use crate::stock_str; use crate::sync::Sync::*; -use crate::tools::{smeared_time, time}; +use crate::tools::{create_outgoing_rfc724_mid, smeared_time, time}; +use crate::{chatlist_events, mimefactory}; /// Starts the securejoin protocol with the QR `invite`. /// @@ -299,47 +299,145 @@ pub(crate) async fn send_handshake_message( chat_id: ChatId, step: BobHandshakeMsg, ) -> Result<()> { - let mut msg = Message { - viewtype: Viewtype::Text, - text: step.body_text(invite), - hidden: true, - ..Default::default() - }; - msg.param.set_cmd(SystemMessage::SecurejoinMessage); + if invite.is_v3() && matches!(step, BobHandshakeMsg::Request) { + // Send a minimal symmetrically-encrypted vc-request message + + // TODO: Either add a message to the database, or make sure that smtp.rs gets along with a 0 or NULL msg_id + /* + msg.state = MessageState::OutPending; + msg.timestamp_sort = create_smeared_timestamp(context); + msg.rfc724_mid = create_outgoing_rfc724_mid(); + let is_bot = context.get_config_bool(Config::Bot).await?; + msg.param + .set_optional(Param::Bot, Some("1").filter(|_| is_bot)); + + let raw_id = context + .sql + .insert( + "INSERT INTO msgs ( + rfc724_mid, + chat_id, + from_id, + to_id, + timestamp, + type, + state, + txt, + txt_normalized, + subject, + param, + hidden, + mime_in_reply_to, + mime_references, + mime_modified, + mime_headers, + mime_compressed, + location_id, + ephemeral_timer, + ephemeral_timestamp) + VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,1,?,?,?);", + params_slice![ + msg.rfc724_mid, + msg.chat_id, + msg.from_id, + to_id, + msg.timestamp_sort, + msg.viewtype, + msg.state, + msg_text, + normalize_text(&msg_text), + &msg.subject, + msg.param.to_string(), + msg.hidden, + msg.in_reply_to.as_deref().unwrap_or_default(), + new_references, + new_mime_headers.is_some(), + new_mime_headers.unwrap_or_default(), + location_id as i32, + ephemeral_timer, + ephemeral_timestamp + ], + ) + .await?; + context.new_msgs_notify.notify_one(); + msg.id = MsgId::new(u32::try_from(raw_id)?); + */ - // Sends the step in Secure-Join header. - msg.param.set(Param::Arg, step.securejoin_header(invite)); + let row_ids = create_send_msg_jobs(context, msg) + .await + .context("Failed to create send jobs")?; - match step { - BobHandshakeMsg::Request => { - // Sends the Secure-Join-Invitenumber header in mimefactory.rs. - msg.param.set(Param::Arg2, invite.invitenumber()); - msg.force_plaintext(); - } - BobHandshakeMsg::RequestWithAuth => { - // Sends the Secure-Join-Auth header in mimefactory.rs. - msg.param.set(Param::Arg2, invite.authcode()); - msg.param.set_int(Param::GuaranteeE2ee, 1); - - // Sends our own fingerprint in the Secure-Join-Fingerprint header. - let bob_fp = self_fingerprint(context).await?; - msg.param.set(Param::Arg3, bob_fp); - - // Sends the grpid in the Secure-Join-Group header. - // - // `Secure-Join-Group` header is deprecated, - // but old Delta Chat core requires that Alice receives it. - // - // Previous Delta Chat core also sent `Secure-Join-Group` header - // in `vg-request` messages, - // but it was not used on the receiver. - if let QrInvite::Group { grpid, .. } = invite { - msg.param.set(Param::Arg4, grpid); + let rfc724_mid = create_outgoing_rfc724_mid(); + let contact = Contact::get_by_id(context, invite.contact_id()).await?; + let recipient = contact.get_addr(); + + let rendered_message = mimefactory::render_symm_encrypted_securejoin_message( + context, + recipient, + rfc724_mid, + invite.authcode(), + ) + .await?; + + context + .sql + .execute( + "INSERT INTO smtp (rfc724_mid, recipients, mime, msg_id) + VALUES (?1, ?2, ?3, ?4)", + ( + &rfc724_mid, + &recipient, + &rendered_message, + 0, // TODO + ), + ) + .await?; + + context.scheduler.interrupt_smtp().await; + } else { + let mut msg = Message { + viewtype: Viewtype::Text, + text: step.body_text(invite), + hidden: true, + ..Default::default() + }; + + msg.param.set_cmd(SystemMessage::SecurejoinMessage); + + // Sends the step in Secure-Join header. + msg.param.set(Param::Arg, step.securejoin_header(invite)); + + match step { + BobHandshakeMsg::Request => { + // Sends the Secure-Join-Invitenumber header in mimefactory.rs. + msg.param.set(Param::Arg2, invite.invitenumber()); + msg.force_plaintext(); } - } - }; + BobHandshakeMsg::RequestWithAuth => { + // Sends the Secure-Join-Auth header in mimefactory.rs. + msg.param.set(Param::Arg2, invite.authcode()); + msg.param.set_int(Param::GuaranteeE2ee, 1); + + // Sends our own fingerprint in the Secure-Join-Fingerprint header. + let bob_fp = self_fingerprint(context).await?; + msg.param.set(Param::Arg3, bob_fp); + + // Sends the grpid in the Secure-Join-Group header. + // + // `Secure-Join-Group` header is deprecated, + // but old Delta Chat core requires that Alice receives it. + // + // Previous Delta Chat core also sent `Secure-Join-Group` header + // in `vg-request` messages, + // but it was not used on the receiver. + if let QrInvite::Group { grpid, .. } = invite { + msg.param.set(Param::Arg4, grpid); + } + } + }; - chat::send_msg(context, chat_id, &mut msg).await?; + chat::send_msg(context, chat_id, &mut msg).await?; + } Ok(()) } diff --git a/src/securejoin/qrinvite.rs b/src/securejoin/qrinvite.rs index 4bb3b71e11..26c4cb4363 100644 --- a/src/securejoin/qrinvite.rs +++ b/src/securejoin/qrinvite.rs @@ -12,7 +12,7 @@ use crate::qr::Qr; /// Represents the data from a QR-code scan. /// -/// There are methods to conveniently access fields present in both variants. +/// There are methods to conveniently access fields present in all three variants. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub enum QrInvite { Contact { @@ -20,6 +20,7 @@ pub enum QrInvite { fingerprint: Fingerprint, invitenumber: String, authcode: String, + is_v3: bool, }, Group { contact_id: ContactId, @@ -28,6 +29,7 @@ pub enum QrInvite { grpid: String, invitenumber: String, authcode: String, + is_v3: bool, }, Broadcast { contact_id: ContactId, @@ -36,6 +38,7 @@ pub enum QrInvite { grpid: String, invitenumber: String, authcode: String, + is_v3: bool, }, } @@ -78,6 +81,14 @@ impl QrInvite { | Self::Broadcast { authcode, .. } => authcode, } } + + pub fn is_v3(&self) -> bool { + match self { + QrInvite::Contact { is_v3, .. } => is_v3, + QrInvite::Group { is_v3, .. } => is_v3, + QrInvite::Broadcast { is_v3, .. } => is_v3, + } + } } impl TryFrom for QrInvite { From 3b666117fcf092712b3c47ce3e991e1ac91b35fe Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 20 Jan 2026 11:07:45 +0100 Subject: [PATCH 03/36] [wip, not compiling] extract function group_headers_by_confidentiality() --- src/mimefactory.rs | 453 +++++++++++++++++++-------------------------- 1 file changed, 193 insertions(+), 260 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index f2eb6508e6..9c1ed4875a 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -947,6 +947,22 @@ impl MimeFactory { )); } + if self.pre_message_mode == PreMessageMode::Post { + headers.push(( + "Chat-Is-Post-Message", + mail_builder::headers::raw::Raw::new("1").into(), + )); + } else if let PreMessageMode::Pre { + post_msg_rfc724_mid, + } = &self.pre_message_mode + { + headers.push(( + "Chat-Post-Message-ID", + mail_builder::headers::message_id::MessageId::new(post_msg_rfc724_mid.clone()) + .into(), + )); + } + let is_encrypted = self.will_be_encrypted(); // Add ephemeral timer for non-MDN messages. @@ -993,155 +1009,17 @@ impl MimeFactory { Loaded::Mdn { .. } => self.render_mdn()?, }; - // Split headers based on header confidentiality policy. - - // Headers that must go into IMF header section. - // - // These are standard headers such as Date, In-Reply-To, References, which cannot be placed - // anywhere else according to the standard. Placing headers here also allows them to be fetched - // individually over IMAP without downloading the message body. This is why Chat-Version is - // placed here. - let mut unprotected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); - - // Headers that MUST NOT (only) go into IMF header section: - // - Large headers which may hit the header section size limit on the server, such as - // Chat-User-Avatar with a base64-encoded image inside. - // - Headers duplicated here that servers mess up with in the IMF header section, like - // Message-ID. - // - Nonstandard headers that should be DKIM-protected because e.g. OpenDKIM only signs - // known headers. - // - // The header should be hidden from MTA - // by moving it either into protected part - // in case of encrypted mails - // or unprotected MIME preamble in case of unencrypted mails. - let mut hidden_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); - - // Opportunistically protected headers. - // - // These headers are placed into encrypted part *if* the message is encrypted. Place headers - // which are not needed before decryption (e.g. Chat-Group-Name) or are not interesting if the - // message cannot be decrypted (e.g. Chat-Disposition-Notification-To) here. - // - // If the message is not encrypted, these headers are placed into IMF header section, so make - // sure that the message will be encrypted if you place any sensitive information here. - let mut protected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); - - // MIME header . - unprotected_headers.push(( - "MIME-Version", - mail_builder::headers::raw::Raw::new("1.0").into(), - )); - - if self.pre_message_mode == PreMessageMode::Post { - unprotected_headers.push(( - "Chat-Is-Post-Message", - mail_builder::headers::raw::Raw::new("1").into(), - )); - } else if let PreMessageMode::Pre { - post_msg_rfc724_mid, - } = &self.pre_message_mode - { - protected_headers.push(( - "Chat-Post-Message-ID", - mail_builder::headers::message_id::MessageId::new(post_msg_rfc724_mid.clone()) - .into(), - )); - } - - for header @ (original_header_name, _header_value) in &headers { - let header_name = original_header_name.to_lowercase(); - if header_name == "message-id" { - unprotected_headers.push(header.clone()); - hidden_headers.push(header.clone()); - } else if is_hidden(&header_name) { - hidden_headers.push(header.clone()); - } else if header_name == "from" { - // Unencrypted securejoin messages should _not_ include the display name: - if is_encrypted || !is_securejoin_message { - protected_headers.push(header.clone()); - } - - unprotected_headers.push(( - original_header_name, - Address::new_address(None::<&'static str>, self.from_addr.clone()).into(), - )); - } else if header_name == "to" { - protected_headers.push(header.clone()); - if is_encrypted { - unprotected_headers.push(("To", hidden_recipients().into())); - } else { - unprotected_headers.push(header.clone()); - } - } else if header_name == "chat-broadcast-secret" { - if is_encrypted { - protected_headers.push(header.clone()); - } else { - bail!("Message is unecrypted, cannot include broadcast secret"); - } - } else if is_encrypted && header_name == "date" { - protected_headers.push(header.clone()); - - // Randomized date goes to unprotected header. - // - // We cannot just send "Thu, 01 Jan 1970 00:00:00 +0000" - // or omit the header because GMX then fails with - // - // host mx00.emig.gmx.net[212.227.15.9] said: - // 554-Transaction failed - // 554-Reject due to policy restrictions. - // 554 For explanation visit https://postmaster.gmx.net/en/case?... - // (in reply to end of DATA command) - // - // and the explanation page says - // "The time information deviates too much from the actual time". - // - // We also limit the range to 6 days (518400 seconds) - // because with a larger range we got - // error "500 Date header far in the past/future" - // which apparently originates from Symantec Messaging Gateway - // and means the message has a Date that is more - // than 7 days in the past: - // - let timestamp_offset = rand::random_range(0..518400); - let protected_timestamp = self.timestamp.saturating_sub(timestamp_offset); - let unprotected_date = - chrono::DateTime::::from_timestamp(protected_timestamp, 0) - .unwrap() - .to_rfc2822(); - unprotected_headers.push(( - "Date", - mail_builder::headers::raw::Raw::new(unprotected_date).into(), - )); - } else if is_encrypted { - protected_headers.push(header.clone()); - - match header_name.as_str() { - "subject" => { - unprotected_headers.push(( - "Subject", - mail_builder::headers::raw::Raw::new("[...]").into(), - )); - } - "in-reply-to" - | "references" - | "auto-submitted" - | "chat-version" - | "autocrypt-setup-message" => { - unprotected_headers.push(header.clone()); - } - _ => { - // Other headers are removed from unprotected part. - } - } - } else { - // Copy the header to the protected headers - // in case of signed-only message. - // If the message is not signed, this value will not be used. - protected_headers.push(header.clone()); - unprotected_headers.push(header.clone()) - } - } + let HeadersByConfidentiality { + mut unprotected_headers, + hidden_headers, + protected_headers, + } = group_headers_by_confidentiality( + headers, + &self.from_addr, + self.timestamp, + is_encrypted, + is_securejoin_message, + ); let use_std_header_protection = context .get_config_bool(Config::StdHeaderProtectionComposing) @@ -2098,6 +1976,158 @@ impl MimeFactory { } } +struct HeadersByConfidentiality { + /// Headers that must go into IMF header section. + /// + /// These are standard headers such as Date, In-Reply-To, References, which cannot be placed + /// anywhere else according to the standard. Placing headers here also allows them to be fetched + /// individually over IMAP without downloading the message body. This is why Chat-Version is + /// placed here. + unprotected_headers: Vec<(&'static str, HeaderType<'static>)>, + + /// Headers that MUST NOT (only) go into IMF header section: + /// - Large headers which may hit the header section size limit on the server, such as + /// Chat-User-Avatar with a base64-encoded image inside. + /// - Headers duplicated here that servers mess up with in the IMF header section, like + /// Message-ID. + /// - Nonstandard headers that should be DKIM-protected because e.g. OpenDKIM only signs + /// known headers. + /// + /// The header should be hidden from MTA + /// by moving it either into protected part + /// in case of encrypted mails + /// or unprotected MIME preamble in case of unencrypted mails. + hidden_headers: Vec<(&'static str, HeaderType<'static>)>, + + /// Opportunistically protected headers. + /// + /// These headers are placed into encrypted part *if* the message is encrypted. Place headers + /// which are not needed before decryption (e.g. Chat-Group-Name) or are not interesting if the + /// message cannot be decrypted (e.g. Chat-Disposition-Notification-To) here. + /// + /// If the message is not encrypted, these headers are placed into IMF header section, so make + /// sure that the message will be encrypted if you place any sensitive information here. + protected_headers: Vec<(&'static str, HeaderType<'static>)>, +} + +/// Split headers based on header confidentiality policy. +/// See [`HeadersByConfidentiality`] for more info. +fn group_headers_by_confidentiality( + headers: Vec<(&'static str, HeaderType<'static>)>, + from_addr: &str, + timestamp: i64, + is_encrypted: bool, + is_securejoin_message: bool, +) -> HeadersByConfidentiality { + let mut unprotected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); + let mut hidden_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); + let mut protected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); + + // MIME header . + unprotected_headers.push(( + "MIME-Version", + mail_builder::headers::raw::Raw::new("1.0").into(), + )); + + for header @ (original_header_name, _header_value) in &headers { + let header_name = original_header_name.to_lowercase(); + if header_name == "message-id" { + unprotected_headers.push(header.clone()); + hidden_headers.push(header.clone()); + } else if is_hidden(&header_name) { + hidden_headers.push(header.clone()); + } else if header_name == "from" { + // Unencrypted securejoin messages should _not_ include the display name: + if is_encrypted || !is_securejoin_message { + protected_headers.push(header.clone()); + } + + unprotected_headers.push(( + original_header_name, + Address::new_address(None::<&'static str>, from_addr.to_string()).into(), + )); + } else if header_name == "to" { + protected_headers.push(header.clone()); + if is_encrypted { + unprotected_headers.push(("To", hidden_recipients().into())); + } else { + unprotected_headers.push(header.clone()); + } + } else if header_name == "chat-broadcast-secret" { + if is_encrypted { + protected_headers.push(header.clone()); + } + } else if is_encrypted && header_name == "date" { + protected_headers.push(header.clone()); + + // Randomized date goes to unprotected header. + // + // We cannot just send "Thu, 01 Jan 1970 00:00:00 +0000" + // or omit the header because GMX then fails with + // + // host mx00.emig.gmx.net[212.227.15.9] said: + // 554-Transaction failed + // 554-Reject due to policy restrictions. + // 554 For explanation visit https://postmaster.gmx.net/en/case?... + // (in reply to end of DATA command) + // + // and the explanation page says + // "The time information deviates too much from the actual time". + // + // We also limit the range to 6 days (518400 seconds) + // because with a larger range we got + // error "500 Date header far in the past/future" + // which apparently originates from Symantec Messaging Gateway + // and means the message has a Date that is more + // than 7 days in the past: + // + let timestamp_offset = rand::random_range(0..518400); + let protected_timestamp = timestamp.saturating_sub(timestamp_offset); + let unprotected_date = + chrono::DateTime::::from_timestamp(protected_timestamp, 0) + .unwrap() + .to_rfc2822(); + unprotected_headers.push(( + "Date", + mail_builder::headers::raw::Raw::new(unprotected_date).into(), + )); + } else if is_encrypted { + protected_headers.push(header.clone()); + + match header_name.as_str() { + "subject" => { + unprotected_headers.push(( + "Subject", + mail_builder::headers::raw::Raw::new("[...]").into(), + )); + } + "in-reply-to" + | "references" + | "auto-submitted" + | "chat-version" + | "autocrypt-setup-message" + | "chat-is-post-message" => { + unprotected_headers.push(header.clone()); + } + _ => { + // Other headers are removed from unprotected part. + } + } + } else { + // Copy the header to the protected headers + // in case of signed-only message. + // If the message is not signed, this value will not be used. + protected_headers.push(header.clone()); + unprotected_headers.push(header.clone()) + } + } + HeadersByConfidentiality { + unprotected_headers, + hidden_headers, + protected_headers, + } +} + fn hidden_recipients() -> Address<'static> { Address::new_group(Some("hidden-recipients".to_string()), Vec::new()) } @@ -2217,7 +2247,7 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( let mut headers = Vec::<(&'static str, HeaderType<'static>)>::new(); let from_addr = context.get_primary_self_addr().await?; - let from = new_address_with_name("", from_addr); + let from = new_address_with_name("", from_addr.to_string()); let mut to: Vec> = Vec::new(); to.push(hidden_recipients()); @@ -2276,116 +2306,19 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( let message: MimePart<'static> = MimePart::new("text/plain", "Secure-Join"); - // Split headers based on header confidentiality policy. - - // Headers that must go into IMF header section. - // - // These are standard headers such as Date, In-Reply-To, References, which cannot be placed - // anywhere else according to the standard. Placing headers here also allows them to be fetched - // individually over IMAP without downloading the message body. This is why Chat-Version is - // placed here. - let mut unprotected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); - - // Headers that MUST NOT (only) go into IMF header section: - // - Large headers which may hit the header section size limit on the server, such as - // Chat-User-Avatar with a base64-encoded image inside. - // - Headers duplicated here that servers mess up with in the IMF header section, like - // Message-ID. - // - Nonstandard headers that should be DKIM-protected because e.g. OpenDKIM only signs - // known headers. - // - // The header should be hidden from MTA - // by moving it either into protected part - // in case of encrypted mails - // or unprotected MIME preamble in case of unencrypted mails. - let mut hidden_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); - - // Opportunistically protected headers. - // - // These headers are placed into encrypted part *if* the message is encrypted. Place headers - // which are not needed before decryption (e.g. Chat-Group-Name) or are not interesting if the - // message cannot be decrypted (e.g. Chat-Disposition-Notification-To) here. - // - // If the message is not encrypted, these headers are placed into IMF header section, so make - // sure that the message will be encrypted if you place any sensitive information here. - let mut protected_headers: Vec<(&'static str, HeaderType<'static>)> = Vec::new(); - - // MIME header . - unprotected_headers.push(( - "MIME-Version", - mail_builder::headers::raw::Raw::new("1.0").into(), - )); - - for header @ (original_header_name, _header_value) in &headers { - let header_name = original_header_name.to_lowercase(); - if header_name == "message-id" { - unprotected_headers.push(header.clone()); - hidden_headers.push(header.clone()); - } else if is_hidden(&header_name) { - hidden_headers.push(header.clone()); - } else if header_name == "from" { - protected_headers.push(header.clone()); - - unprotected_headers.push(header.clone()); - } else if header_name == "to" { - unprotected_headers.push(("To", hidden_recipients().into())); - } else if header_name == "date" { - protected_headers.push(header.clone()); - - // Randomized date goes to unprotected header. - // - // We cannot just send "Thu, 01 Jan 1970 00:00:00 +0000" - // or omit the header because GMX then fails with - // - // host mx00.emig.gmx.net[212.227.15.9] said: - // 554-Transaction failed - // 554-Reject due to policy restrictions. - // 554 For explanation visit https://postmaster.gmx.net/en/case?... - // (in reply to end of DATA command) - // - // and the explanation page says - // "The time information deviates too much from the actual time". - // - // We also limit the range to 6 days (518400 seconds) - // because with a larger range we got - // error "500 Date header far in the past/future" - // which apparently originates from Symantec Messaging Gateway - // and means the message has a Date that is more - // than 7 days in the past: - // - let timestamp_offset = rand::random_range(0..518400); - let protected_timestamp = timestamp.saturating_sub(timestamp_offset); - let unprotected_date = - chrono::DateTime::::from_timestamp(protected_timestamp, 0) - .unwrap() - .to_rfc2822(); - unprotected_headers.push(( - "Date", - mail_builder::headers::raw::Raw::new(unprotected_date).into(), - )); - } else { - protected_headers.push(header.clone()); - - match header_name.as_str() { - "subject" => { - unprotected_headers.push(( - "Subject", - mail_builder::headers::raw::Raw::new("[...]").into(), - )); - } - "in-reply-to" - | "references" - | "auto-submitted" - | "chat-version" - | "autocrypt-setup-message" => { - unprotected_headers.push(header.clone()); - } - _ => { - // Other headers are removed from unprotected part. - } - } - } - } + let is_encrypted = true; + let is_securejoin_message = true; + let HeadersByConfidentiality { + unprotected_headers, + hidden_headers, + protected_headers, + } = group_headers_by_confidentiality( + headers, + &from_addr, + timestamp, + is_encrypted, + is_securejoin_message, + ); let outer_message = { // Store protected headers in the inner message. From 2025e9bbfd5bd47af075b2de332a52e09aca5df8 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 20 Jan 2026 14:34:28 +0100 Subject: [PATCH 04/36] [wip, doesn't compile] refactor: exctract add_headers_to_encrypted_part() and set_content_type_to_encrypted() functions --- src/mimefactory.rs | 246 +++++++++++++++++++-------------------------- 1 file changed, 104 insertions(+), 142 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 9c1ed4875a..46a4dd0f62 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1025,35 +1025,13 @@ impl MimeFactory { .get_config_bool(Config::StdHeaderProtectionComposing) .await?; let outer_message = if let Some(encryption_pubkeys) = self.encryption_pubkeys { - // Store protected headers in the inner message. - let message = protected_headers - .into_iter() - .fold(message, |message, (header, value)| { - message.header(header, value) - }); - - // Add hidden headers to encrypted payload. - let mut message: MimePart<'static> = hidden_headers - .into_iter() - .fold(message, |message, (header, value)| { - message.header(header, value) - }); - - if use_std_header_protection { - message = unprotected_headers - .iter() - // Structural headers shouldn't be added as "HP-Outer". They are defined in - // . - .filter(|(name, _)| { - !(name.eq_ignore_ascii_case("mime-version") - || name.eq_ignore_ascii_case("content-type") - || name.eq_ignore_ascii_case("content-transfer-encoding") - || name.eq_ignore_ascii_case("content-disposition")) - }) - .fold(message, |message, (name, value)| { - message.header(format!("HP-Outer: {name}"), value.clone()) - }); - } + let mut message = add_headers_to_encrypted_part( + message, + &unprotected_headers, + hidden_headers, + protected_headers, + use_std_header_protection, + ); // Add gossip headers in chats with multiple recipients let multiple_recipients = @@ -1144,21 +1122,6 @@ impl MimeFactory { } } - // Set the appropriate Content-Type for the inner message. - for (h, v) in &mut message.headers { - if h == "Content-Type" - && let mail_builder::headers::HeaderType::ContentType(ct) = v - { - let mut ct_new = ct.clone(); - ct_new = ct_new.attribute("protected-headers", "v1"); - if use_std_header_protection { - ct_new = ct_new.attribute("hp", "cipher"); - } - *ct = ct_new; - break; - } - } - // Disable compression for SecureJoin to ensure // there are no compression side channels // leaking information about the tokens. @@ -1241,35 +1204,7 @@ impl MimeFactory { .await? }; - // XXX: additional newline is needed - // to pass filtermail at - // : - let encrypted = encrypted + "\n"; - - // Set the appropriate Content-Type for the outer message - MimePart::new( - "multipart/encrypted; protocol=\"application/pgp-encrypted\"", - vec![ - // Autocrypt part 1 - MimePart::new("application/pgp-encrypted", "Version: 1\r\n").header( - "Content-Description", - mail_builder::headers::raw::Raw::new("PGP/MIME version identification"), - ), - // Autocrypt part 2 - MimePart::new( - "application/octet-stream; name=\"encrypted.asc\"", - encrypted, - ) - .header( - "Content-Description", - mail_builder::headers::raw::Raw::new("OpenPGP encrypted message"), - ) - .header( - "Content-Disposition", - mail_builder::headers::raw::Raw::new("inline; filename=\"encrypted.asc\";"), - ), - ], - ) + set_content_type_to_encrypted(encrypted) } else if matches!(self.loaded, Loaded::Mdn { .. }) { // Never add outer multipart/mixed wrapper to MDN // as multipart/report Content-Type is used to recognize MDNs @@ -1976,6 +1911,93 @@ impl MimeFactory { } } +/// Set the appropriate Content-Type for the outer message +fn set_content_type_to_encrypted(encrypted: String) -> MimePart<'static> { + // XXX: additional newline is needed + // to pass filtermail at + // : + let encrypted = encrypted + "\n"; + + MimePart::new( + "multipart/encrypted; protocol=\"application/pgp-encrypted\"", + vec![ + // Autocrypt part 1 + MimePart::new("application/pgp-encrypted", "Version: 1\r\n").header( + "Content-Description", + mail_builder::headers::raw::Raw::new("PGP/MIME version identification"), + ), + // Autocrypt part 2 + MimePart::new( + "application/octet-stream; name=\"encrypted.asc\"", + encrypted, + ) + .header( + "Content-Description", + mail_builder::headers::raw::Raw::new("OpenPGP encrypted message"), + ) + .header( + "Content-Disposition", + mail_builder::headers::raw::Raw::new("inline; filename=\"encrypted.asc\";"), + ), + ], + ) +} + +fn add_headers_to_encrypted_part( + message: MimePart<'static>, + unprotected_headers: &[(&'static str, HeaderType<'static>)], + hidden_headers: Vec<(&'static str, HeaderType<'static>)>, + protected_headers: Vec<(&'static str, HeaderType<'static>)>, + use_std_header_protection: bool, +) -> MimePart<'static> { + // Store protected headers in the inner message. + let message = protected_headers + .into_iter() + .fold(message, |message, (header, value)| { + message.header(header, value) + }); + + // Add hidden headers to encrypted payload. + let mut message: MimePart<'static> = hidden_headers + .into_iter() + .fold(message, |message, (header, value)| { + message.header(header, value) + }); + + if use_std_header_protection { + message = unprotected_headers + .iter() + // Structural headers shouldn't be added as "HP-Outer". They are defined in + // . + .filter(|(name, _)| { + !(name.eq_ignore_ascii_case("mime-version") + || name.eq_ignore_ascii_case("content-type") + || name.eq_ignore_ascii_case("content-transfer-encoding") + || name.eq_ignore_ascii_case("content-disposition")) + }) + .fold(message, |message, (name, value)| { + message.header(format!("HP-Outer: {name}"), value.clone()) + }); + } + + // Set the appropriate Content-Type for the inner message + for (h, v) in &mut message.headers { + if h == "Content-Type" + && let mail_builder::headers::HeaderType::ContentType(ct) = v + { + let mut ct_new = ct.clone(); + ct_new = ct_new.attribute("protected-headers", "v1"); + if use_std_header_protection { + ct_new = ct_new.attribute("hp", "cipher"); + } + *ct = ct_new; + break; + } + } + + message +} + struct HeadersByConfidentiality { /// Headers that must go into IMF header section. /// @@ -2321,46 +2343,14 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( ); let outer_message = { - // Store protected headers in the inner message. - let message = protected_headers - .into_iter() - .fold(message, |message, (header, value)| { - message.header(header, value) - }); - - // Add hidden headers to encrypted payload. - let mut message: MimePart<'static> = hidden_headers - .into_iter() - .fold(message, |message, (header, value)| { - message.header(header, value) - }); - - message = unprotected_headers - .iter() - // Structural headers shouldn't be added as "HP-Outer". They are defined in - // . - .filter(|(name, _)| { - !(name.eq_ignore_ascii_case("mime-version") - || name.eq_ignore_ascii_case("content-type") - || name.eq_ignore_ascii_case("content-transfer-encoding") - || name.eq_ignore_ascii_case("content-disposition")) - }) - .fold(message, |message, (name, value)| { - message.header(format!("HP-Outer: {name}"), value.clone()) - }); - - // Set the appropriate Content-Type for the inner message. - for (h, v) in &mut message.headers { - if h == "Content-Type" - && let mail_builder::headers::HeaderType::ContentType(ct) = v - { - let mut ct_new = ct.clone(); - ct_new = ct_new.attribute("protected-headers", "v1"); - ct_new = ct_new.attribute("hp", "cipher"); - *ct = ct_new; - break; - } - } + let use_std_header_protection = true; + let mut message = add_headers_to_encrypted_part( + message, + &unprotected_headers, + hidden_headers, + protected_headers, + use_std_header_protection, + ); // Disable compression for SecureJoin to ensure // there are no compression side channels @@ -2377,35 +2367,7 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( .encrypt_symmetrically(context, auth, message, compress) .await?; - // XXX: additional newline is needed - // to pass filtermail at - // : - let encrypted = encrypted + "\n"; - - // Set the appropriate Content-Type for the outer message - MimePart::new( - "multipart/encrypted; protocol=\"application/pgp-encrypted\"", - vec![ - // Autocrypt part 1 - MimePart::new("application/pgp-encrypted", "Version: 1\r\n").header( - "Content-Description", - mail_builder::headers::raw::Raw::new("PGP/MIME version identification"), - ), - // Autocrypt part 2 - MimePart::new( - "application/octet-stream; name=\"encrypted.asc\"", - encrypted, - ) - .header( - "Content-Description", - mail_builder::headers::raw::Raw::new("OpenPGP encrypted message"), - ) - .header( - "Content-Disposition", - mail_builder::headers::raw::Raw::new("inline; filename=\"encrypted.asc\";"), - ), - ], - ) + set_content_type_to_encrypted(encrypted) }; // Store the unprotected headers on the outer message. From 2eba55207f73ccd10294d315ac59d4d855e8da8c Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 20 Jan 2026 14:41:32 +0100 Subject: [PATCH 05/36] [wip, doesn't compile] fix: Set subject for securejoin message --- src/mimefactory.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 46a4dd0f62..3c89c97a38 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -2281,6 +2281,11 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( mail_builder::headers::address::Address::new_list(to.clone()).into(), )); + headers.push(( + "Subject", + mail_builder::headers::text::Text::new("Secure-Join".to_string()).into(), + )); + // TODO not sure if we even need a timestamp let timestamp = create_smeared_timestamp(context); let date = chrono::DateTime::::from_timestamp(timestamp, 0) @@ -2344,7 +2349,7 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( let outer_message = { let use_std_header_protection = true; - let mut message = add_headers_to_encrypted_part( + let message = add_headers_to_encrypted_part( message, &unprotected_headers, hidden_headers, @@ -2356,13 +2361,6 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( // there are no compression side channels // leaking information about the tokens. let compress = false; - - if context.get_config_bool(Config::TestHooks).await? - && let Some(hook) = &*context.pre_encrypt_mime_hook.lock() - { - message = hook(context, message); - } - let encrypted = encrypt_helper .encrypt_symmetrically(context, auth, message, compress) .await?; From 2566c5bcf4eafe017e2042a1e7d325d80a679f30 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 20 Jan 2026 14:56:53 +0100 Subject: [PATCH 06/36] [wip, doesn't compile] Extract render_outer_message() functions --- src/mimefactory.rs | 53 ++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 3c89c97a38..ff6e50b303 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1204,7 +1204,7 @@ impl MimeFactory { .await? }; - set_content_type_to_encrypted(encrypted) + wrap_encrypted_part(encrypted) } else if matches!(self.loaded, Loaded::Mdn { .. }) { // Never add outer multipart/mixed wrapper to MDN // as multipart/report Content-Type is used to recognize MDNs @@ -1271,22 +1271,12 @@ impl MimeFactory { } }; - // Store the unprotected headers on the outer message. - let outer_message = unprotected_headers - .into_iter() - .fold(outer_message, |message, (header, value)| { - message.header(header, value) - }); - let MimeFactory { last_added_location_id, .. } = self; - let mut buffer = Vec::new(); - let cursor = Cursor::new(&mut buffer); - outer_message.clone().write_part(cursor).ok(); - let message = String::from_utf8_lossy(&buffer).to_string(); + let message = render_outer_message(unprotected_headers, outer_message); Ok(RenderedEmail { message, @@ -1911,8 +1901,27 @@ impl MimeFactory { } } -/// Set the appropriate Content-Type for the outer message -fn set_content_type_to_encrypted(encrypted: String) -> MimePart<'static> { +/// Stores the unprotected headers on the outer message, and renders it. +fn render_outer_message( + unprotected_headers: Vec<(&'static str, HeaderType<'static>)>, + outer_message: MimePart<'static>, +) -> String { + let outer_message = unprotected_headers + .into_iter() + .fold(outer_message, |message, (header, value)| { + message.header(header, value) + }); + + let mut buffer = Vec::new(); + let cursor = Cursor::new(&mut buffer); + outer_message.clone().write_part(cursor).ok(); + let message = String::from_utf8_lossy(&buffer).to_string(); + message +} + +/// Takes the encrypted part, wraps it in a MimePart, +/// and sets the appropriate Content-Type for the outer message +fn wrap_encrypted_part(encrypted: String) -> MimePart<'static> { // XXX: additional newline is needed // to pass filtermail at // : @@ -2275,12 +2284,10 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( to.push(hidden_recipients()); headers.push(("From", from.into())); - headers.push(( "To", mail_builder::headers::address::Address::new_list(to.clone()).into(), )); - headers.push(( "Subject", mail_builder::headers::text::Text::new("Secure-Join".to_string()).into(), @@ -2365,20 +2372,10 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( .encrypt_symmetrically(context, auth, message, compress) .await?; - set_content_type_to_encrypted(encrypted) + wrap_encrypted_part(encrypted) }; - // Store the unprotected headers on the outer message. - let outer_message = unprotected_headers - .into_iter() - .fold(outer_message, |message, (header, value)| { - message.header(header, value) - }); - - let mut buffer = Vec::new(); - let cursor = Cursor::new(&mut buffer); - outer_message.clone().write_part(cursor).ok(); - let message = String::from_utf8_lossy(&buffer).to_string(); + let message = render_outer_message(unprotected_headers, outer_message); Ok(message) } From 1014bf17a45c3d1931212f95fda5bb31079bf4b4 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 20 Jan 2026 15:11:22 +0100 Subject: [PATCH 07/36] It compiles --- deltachat-jsonrpc/src/api/types/qr.rs | 12 ++++++++++++ src/aheader.rs | 2 +- src/mimefactory.rs | 10 ++++------ src/qr.rs | 14 ++++++++++++++ src/securejoin/bob.rs | 16 +++++++--------- src/securejoin/qrinvite.rs | 8 +++++++- 6 files changed, 45 insertions(+), 17 deletions(-) diff --git a/deltachat-jsonrpc/src/api/types/qr.rs b/deltachat-jsonrpc/src/api/types/qr.rs index de05e79c6a..5b58a06d85 100644 --- a/deltachat-jsonrpc/src/api/types/qr.rs +++ b/deltachat-jsonrpc/src/api/types/qr.rs @@ -19,6 +19,8 @@ pub enum QrObject { invitenumber: String, /// Authentication code. authcode: String, + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Ask the user whether to join the group. AskVerifyGroup { @@ -34,6 +36,8 @@ pub enum QrObject { invitenumber: String, /// Authentication code. authcode: String, + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Ask the user whether to join the broadcast channel. AskJoinBroadcast { @@ -54,6 +58,8 @@ pub enum QrObject { invitenumber: String, /// Authentication code. authcode: String, + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Contact fingerprint is verified. /// @@ -229,6 +235,7 @@ impl From for QrObject { fingerprint, invitenumber, authcode, + is_v3, } => { let contact_id = contact_id.to_u32(); let fingerprint = fingerprint.to_string(); @@ -237,6 +244,7 @@ impl From for QrObject { fingerprint, invitenumber, authcode, + is_v3, } } Qr::AskVerifyGroup { @@ -246,6 +254,7 @@ impl From for QrObject { fingerprint, invitenumber, authcode, + is_v3, } => { let contact_id = contact_id.to_u32(); let fingerprint = fingerprint.to_string(); @@ -256,6 +265,7 @@ impl From for QrObject { fingerprint, invitenumber, authcode, + is_v3, } } Qr::AskJoinBroadcast { @@ -265,6 +275,7 @@ impl From for QrObject { fingerprint, authcode, invitenumber, + is_v3, } => { let contact_id = contact_id.to_u32(); let fingerprint = fingerprint.to_string(); @@ -275,6 +286,7 @@ impl From for QrObject { fingerprint, authcode, invitenumber, + is_v3, } } Qr::FprOk { contact_id } => { diff --git a/src/aheader.rs b/src/aheader.rs index 7f84ca0f43..2ae8f3f06a 100644 --- a/src/aheader.rs +++ b/src/aheader.rs @@ -47,7 +47,7 @@ pub struct Aheader { pub public_key: SignedPublicKey, pub prefer_encrypt: EncryptPreference, - // Whether `_verified` attribute is present. + /// Whether `_verified` attribute is present. // // `_verified` attribute is an extension to `Autocrypt-Gossip` // header that is used to tell that the sender diff --git a/src/mimefactory.rs b/src/mimefactory.rs index ff6e50b303..9b818b25e6 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1915,8 +1915,7 @@ fn render_outer_message( let mut buffer = Vec::new(); let cursor = Cursor::new(&mut buffer); outer_message.clone().write_part(cursor).ok(); - let message = String::from_utf8_lossy(&buffer).to_string(); - message + String::from_utf8_lossy(&buffer).to_string() } /// Takes the encrypted part, wraps it in a MimePart, @@ -2279,15 +2278,14 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( let from_addr = context.get_primary_self_addr().await?; let from = new_address_with_name("", from_addr.to_string()); - - let mut to: Vec> = Vec::new(); - to.push(hidden_recipients()); - headers.push(("From", from.into())); + + let to: Vec> = vec![hidden_recipients()]; headers.push(( "To", mail_builder::headers::address::Address::new_list(to.clone()).into(), )); + headers.push(( "Subject", mail_builder::headers::text::Text::new("Secure-Join".to_string()).into(), diff --git a/src/qr.rs b/src/qr.rs index a289d9aafa..1ad3484756 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -61,6 +61,9 @@ pub enum Qr { /// Authentication code. authcode: String, + + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Ask the user whether to join the group. @@ -82,6 +85,9 @@ pub enum Qr { /// Authentication code. authcode: String, + + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Ask whether to join the broadcast channel. @@ -106,6 +112,9 @@ pub enum Qr { invitenumber: String, /// Authentication code. authcode: String, + + /// Whether the sender supports the new Securejoin v3 protocol + is_v3: bool, }, /// Contact fingerprint is verified. @@ -501,6 +510,8 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { let grpname = decode_name(¶m, "g")?; let broadcast_name = decode_name(¶m, "b")?; + let is_v3 = param.get("v") == Some(&"3"); + if let (Some(addr), Some(invitenumber), Some(authcode)) = (&addr, invitenumber, authcode) { let addr = ContactAddress::new(addr)?; let (contact_id, _) = Contact::add_or_lookup_ex( @@ -546,6 +557,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { fingerprint, invitenumber, authcode, + is_v3, }) } } else if let (Some(grpid), Some(name)) = (grpid, broadcast_name) { @@ -581,6 +593,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { fingerprint, invitenumber, authcode, + is_v3, }) } } else if context.is_self_addr(&addr).await? { @@ -605,6 +618,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { fingerprint, invitenumber, authcode, + is_v3, }) } } else if let Some(addr) = addr { diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index e754f8ddb1..533c8c8554 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -1,6 +1,6 @@ //! Bob's side of SecureJoin handling, the joiner-side. -use anyhow::{Context as _, Result, bail}; +use anyhow::{Context as _, Result}; use super::HandshakeMessage; use super::qrinvite::QrInvite; @@ -363,27 +363,25 @@ pub(crate) async fn send_handshake_message( msg.id = MsgId::new(u32::try_from(raw_id)?); */ - let row_ids = create_send_msg_jobs(context, msg) - .await - .context("Failed to create send jobs")?; - let rfc724_mid = create_outgoing_rfc724_mid(); let contact = Contact::get_by_id(context, invite.contact_id()).await?; let recipient = contact.get_addr(); - + let attach_self_pubkey = false; let rendered_message = mimefactory::render_symm_encrypted_securejoin_message( context, - recipient, - rfc724_mid, + step.securejoin_header(invite), + &rfc724_mid, + attach_self_pubkey, invite.authcode(), ) .await?; + // TODO code duplication context .sql .execute( "INSERT INTO smtp (rfc724_mid, recipients, mime, msg_id) - VALUES (?1, ?2, ?3, ?4)", + VALUES (?1, ?2, ?3, ?4)", ( &rfc724_mid, &recipient, diff --git a/src/securejoin/qrinvite.rs b/src/securejoin/qrinvite.rs index 26c4cb4363..ecef2b2726 100644 --- a/src/securejoin/qrinvite.rs +++ b/src/securejoin/qrinvite.rs @@ -83,7 +83,7 @@ impl QrInvite { } pub fn is_v3(&self) -> bool { - match self { + match *self { QrInvite::Contact { is_v3, .. } => is_v3, QrInvite::Group { is_v3, .. } => is_v3, QrInvite::Broadcast { is_v3, .. } => is_v3, @@ -101,11 +101,13 @@ impl TryFrom for QrInvite { fingerprint, invitenumber, authcode, + is_v3, } => Ok(QrInvite::Contact { contact_id, fingerprint, invitenumber, authcode, + is_v3, }), Qr::AskVerifyGroup { grpname, @@ -114,6 +116,7 @@ impl TryFrom for QrInvite { fingerprint, invitenumber, authcode, + is_v3, } => Ok(QrInvite::Group { contact_id, fingerprint, @@ -121,6 +124,7 @@ impl TryFrom for QrInvite { grpid, invitenumber, authcode, + is_v3, }), Qr::AskJoinBroadcast { name, @@ -129,6 +133,7 @@ impl TryFrom for QrInvite { fingerprint, authcode, invitenumber, + is_v3, } => Ok(QrInvite::Broadcast { name, grpid, @@ -136,6 +141,7 @@ impl TryFrom for QrInvite { fingerprint, authcode, invitenumber, + is_v3, }), _ => bail!("Unsupported QR type"), } From c91b349f6c4fdc87220540355f148c7a9c43e0a1 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 20 Jan 2026 17:10:34 +0100 Subject: [PATCH 08/36] feat: Decrypt and answer on Alice's side (sending the message seems not to work yet) --- src/message.rs | 16 ++++++++ src/mimefactory.rs | 2 +- src/mimeparser.rs | 6 ++- src/receive_imf.rs | 20 +--------- src/securejoin.rs | 90 ++++++++++++++++++++++++++++++++++++++++--- src/securejoin/bob.rs | 86 ++++------------------------------------- src/token.rs | 11 ++++++ 7 files changed, 125 insertions(+), 106 deletions(-) diff --git a/src/message.rs b/src/message.rs index 82ca488b4b..13e1e1e035 100644 --- a/src/message.rs +++ b/src/message.rs @@ -2064,6 +2064,22 @@ pub(crate) async fn set_msg_failed( Ok(()) } +/// Inserts a tombstone into `msgs` table +/// to prevent downloading the same message in the future. +/// +/// Returns tombstone database row ID. +pub(crate) async fn insert_tombstone(context: &Context, rfc724_mid: &str) -> Result { + let row_id = context + .sql + .insert( + "INSERT INTO msgs(rfc724_mid, chat_id) VALUES (?,?)", + (rfc724_mid, DC_CHAT_ID_TRASH), + ) + .await?; + let msg_id = MsgId::new(u32::try_from(row_id)?); + Ok(msg_id) +} + /// The number of messages assigned to unblocked chats pub async fn get_unblocked_msg_cnt(context: &Context) -> usize { match context diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 9b818b25e6..c82cd416b0 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -2367,7 +2367,7 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( // leaking information about the tokens. let compress = false; let encrypted = encrypt_helper - .encrypt_symmetrically(context, auth, message, compress) + .encrypt_symmetrically(context, auth, message, compress) // TODO this also signs the message. vc-request-pubkey shouldn't be signed. .await?; wrap_encrypted_part(encrypted) diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 33a02830ce..8982416dd1 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -18,7 +18,6 @@ use crate::authres::handle_authres; use crate::blob::BlobObject; use crate::chat::ChatId; use crate::config::Config; -use crate::constants; use crate::contact::ContactId; use crate::context::Context; use crate::decrypt::{try_decrypt, validate_detached_signature}; @@ -36,6 +35,7 @@ use crate::tools::{ get_filemeta, parse_receive_headers, smeared_time, time, truncate_msg_text, validate_id, }; use crate::{chatlist_events, location, tools}; +use crate::{constants, token}; /// Public key extracted from `Autocrypt-Gossip` /// header with associated information. @@ -382,7 +382,7 @@ impl MimeMessage { let mail_raw; // Memory location for a possible decrypted message. let decrypted_msg; // Decrypted signed OpenPGP message. - let secrets: Vec = context + let mut secrets: Vec = context .sql .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| { let secret: String = row.get(0)?; @@ -390,6 +390,8 @@ impl MimeMessage { }) .await?; + secrets.extend(token::lookup_all(context, token::Namespace::Auth).await?); + let (mail, is_encrypted) = match tokio::task::block_in_place(|| try_decrypt(&mail, &private_keyring, &secrets)) { Ok(Some(mut msg)) => { diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 8a58fd6ded..37aea42667 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -177,22 +177,6 @@ pub(crate) async fn receive_imf_from_inbox( receive_imf_inner(context, rfc724_mid, imf_raw, seen).await } -/// Inserts a tombstone into `msgs` table -/// to prevent downloading the same message in the future. -/// -/// Returns tombstone database row ID. -async fn insert_tombstone(context: &Context, rfc724_mid: &str) -> Result { - let row_id = context - .sql - .insert( - "INSERT INTO msgs(rfc724_mid, chat_id) VALUES (?,?)", - (rfc724_mid, DC_CHAT_ID_TRASH), - ) - .await?; - let msg_id = MsgId::new(u32::try_from(row_id)?); - Ok(msg_id) -} - async fn get_to_and_past_contact_ids( context: &Context, mime_parser: &MimeMessage, @@ -477,7 +461,7 @@ pub(crate) async fn receive_imf_inner( return Ok(None); } - let msg_ids = vec![insert_tombstone(context, rfc724_mid).await?]; + let msg_ids = vec![message::insert_tombstone(context, rfc724_mid).await?]; return Ok(Some(ReceivedMsg { chat_id: DC_CHAT_ID_TRASH, @@ -653,7 +637,7 @@ pub(crate) async fn receive_imf_inner( match res { securejoin::HandshakeMessage::Done | securejoin::HandshakeMessage::Ignore => { - let msg_id = insert_tombstone(context, rfc724_mid).await?; + let msg_id = message::insert_tombstone(context, rfc724_mid).await?; received_msg = Some(ReceivedMsg { chat_id: DC_CHAT_ID_TRASH, state: MessageState::InSeen, diff --git a/src/securejoin.rs b/src/securejoin.rs index 5286b25161..219242a71e 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -20,14 +20,14 @@ use crate::headerdef::HeaderDef; use crate::key::{DcKey, Fingerprint, load_self_public_key}; use crate::log::LogExt as _; use crate::log::warn; -use crate::message::{Message, Viewtype}; +use crate::message::{self, Message, MsgId, Viewtype}; use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::param::Param; use crate::qr::check_qr; use crate::securejoin::bob::JoinerProgress; use crate::sync::Sync::*; -use crate::tools::{create_id, time}; -use crate::{SecurejoinSource, stats}; +use crate::tools::{create_id, create_outgoing_rfc724_mid, time}; +use crate::{SecurejoinSource, mimefactory, stats}; use crate::{SecurejoinUiPath, token}; mod bob; @@ -346,12 +346,18 @@ pub(crate) enum HandshakeMessage { /// Step of Secure-Join protocol. #[derive(Debug, Display, PartialEq, Eq)] pub(crate) enum SecureJoinStep { - /// vc-request or vg-request + /// vc-request or vg-request; only used in legacy securejoin Request { invitenumber: String }, - /// vc-auth-required or vg-auth-required + /// vc-auth-required or vg-auth-required; only used in legacy securejoin AuthRequired, + /// vc-request-pubkey; only used in securejoin v3 + RequestPubkey, + + /// vc-pubkey; only used in securejoin v3 + Pubkey, + /// vc-request-with-auth or vg-request-with-auth RequestWithAuth, @@ -381,6 +387,8 @@ pub(crate) fn get_secure_join_step(mime_message: &MimeMessage) -> Option Some(SecureJoinStep::RequestPubkey), + "vc-pubkey" => Some(SecureJoinStep::Pubkey), "vg-auth-required" | "vc-auth-required" => Some(SecureJoinStep::AuthRequired), "vg-request-with-auth" | "vc-request-with-auth" => { Some(SecureJoinStep::RequestWithAuth) @@ -438,7 +446,10 @@ pub(crate) async fn handle_securejoin_handshake( // will improve security (completely unrelated to the securejoin protocol) // and is something we want to do in the future: // https://www.rfc-editor.org/rfc/rfc9580.html#name-surreptitious-forwarding - if !matches!(step, SecureJoinStep::Request { .. }) { + if !matches!( + step, + SecureJoinStep::Request { .. } | SecureJoinStep::RequestPubkey + ) { let mut self_found = false; let self_fingerprint = load_self_public_key(context).await?.dc_fingerprint(); for (addr, key) in &mime_message.gossiped_keys { @@ -506,6 +517,53 @@ pub(crate) async fn handle_securejoin_handshake( ========================================================*/ bob::handle_auth_required(context, mime_message).await } + SecureJoinStep::RequestPubkey => { + /*======================================================== + ==== Alice - the inviter's side ===== + ==== Bob requests our public key (Securejoin v3) ===== + ========================================================*/ + + if !mime_message.was_encrypted() { + warn!(context, "Ignoring unencrypted RequestPubkey"); + return Ok(HandshakeMessage::Ignore); + } + let Some(auth) = mime_message.get_header(HeaderDef::SecureJoinAuth) else { + warn!( + context, + "Ignoring {step} message because of missing auth code." + ); + return Ok(HandshakeMessage::Ignore); + }; + if !token::exists(context, token::Namespace::Auth, auth).await? { + warn!(context, "Secure-join denied (bad auth)."); + return Ok(HandshakeMessage::Ignore); + } + + let rfc724_mid = create_outgoing_rfc724_mid(); + let addr = ContactAddress::new(&mime_message.from.addr)?; + let attach_self_pubkey = true; + let rendered_message = mimefactory::render_symm_encrypted_securejoin_message( + context, + "vc-pubkey", + &rfc724_mid, + attach_self_pubkey, + auth, + ) + .await?; + + let msg_id = message::insert_tombstone(context, &rfc724_mid).await?; + insert_into_smtp(context, &rfc724_mid, &addr, rendered_message, msg_id).await?; + context.scheduler.interrupt_smtp().await; + + Ok(HandshakeMessage::Done) + } + SecureJoinStep::Pubkey => { + /*======================================================== + ==== Bob - the joiner's side ===== + ==== Alice sent us her pubkey (Securejoin v3) ===== + ========================================================*/ + todo!() + } SecureJoinStep::RequestWithAuth => { /*========================================================== ==== Alice - the inviter side ==== @@ -665,6 +723,24 @@ pub(crate) async fn handle_securejoin_handshake( } } +async fn insert_into_smtp( + context: &Context, + rfc724_mid: &str, + recipient: &str, + rendered_message: String, + msg_id: MsgId, +) -> Result<(), Error> { + context + .sql + .execute( + "INSERT INTO smtp (rfc724_mid, recipients, mime, msg_id) + VALUES (?1, ?2, ?3, ?4)", + (&rfc724_mid, &recipient, &rendered_message, msg_id), + ) + .await?; + Ok(()) +} + /// Observe self-sent Securejoin message. /// /// In a multi-device-setup, there may be other devices that "see" the handshake messages. @@ -696,6 +772,8 @@ pub(crate) async fn observe_securejoin_on_other_device( match step { SecureJoinStep::Request { .. } | SecureJoinStep::AuthRequired + | SecureJoinStep::RequestPubkey + | SecureJoinStep::Pubkey | SecureJoinStep::Deprecated | SecureJoinStep::Unknown { .. } => { return Ok(HandshakeMessage::Ignore); diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 533c8c8554..6b18b3d179 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -11,10 +11,12 @@ use crate::context::Context; use crate::events::EventType; use crate::key::self_fingerprint; use crate::log::LogExt; -use crate::message::{Message, MsgId, Viewtype}; +use crate::message::{self, Message, MsgId, Viewtype}; use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::param::{Param, Params}; -use crate::securejoin::{ContactId, encrypted_and_signed, verify_sender_by_fingerprint}; +use crate::securejoin::{ + ContactId, encrypted_and_signed, insert_into_smtp, verify_sender_by_fingerprint, +}; use crate::stock_str; use crate::sync::Sync::*; use crate::tools::{create_outgoing_rfc724_mid, smeared_time, time}; @@ -302,95 +304,21 @@ pub(crate) async fn send_handshake_message( if invite.is_v3() && matches!(step, BobHandshakeMsg::Request) { // Send a minimal symmetrically-encrypted vc-request message - // TODO: Either add a message to the database, or make sure that smtp.rs gets along with a 0 or NULL msg_id - /* - msg.state = MessageState::OutPending; - msg.timestamp_sort = create_smeared_timestamp(context); - msg.rfc724_mid = create_outgoing_rfc724_mid(); - let is_bot = context.get_config_bool(Config::Bot).await?; - msg.param - .set_optional(Param::Bot, Some("1").filter(|_| is_bot)); - - let raw_id = context - .sql - .insert( - "INSERT INTO msgs ( - rfc724_mid, - chat_id, - from_id, - to_id, - timestamp, - type, - state, - txt, - txt_normalized, - subject, - param, - hidden, - mime_in_reply_to, - mime_references, - mime_modified, - mime_headers, - mime_compressed, - location_id, - ephemeral_timer, - ephemeral_timestamp) - VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,1,?,?,?);", - params_slice![ - msg.rfc724_mid, - msg.chat_id, - msg.from_id, - to_id, - msg.timestamp_sort, - msg.viewtype, - msg.state, - msg_text, - normalize_text(&msg_text), - &msg.subject, - msg.param.to_string(), - msg.hidden, - msg.in_reply_to.as_deref().unwrap_or_default(), - new_references, - new_mime_headers.is_some(), - new_mime_headers.unwrap_or_default(), - location_id as i32, - ephemeral_timer, - ephemeral_timestamp - ], - ) - .await?; - context.new_msgs_notify.notify_one(); - msg.id = MsgId::new(u32::try_from(raw_id)?); - */ - let rfc724_mid = create_outgoing_rfc724_mid(); let contact = Contact::get_by_id(context, invite.contact_id()).await?; let recipient = contact.get_addr(); let attach_self_pubkey = false; let rendered_message = mimefactory::render_symm_encrypted_securejoin_message( context, - step.securejoin_header(invite), + "vc-request-pubkey", &rfc724_mid, attach_self_pubkey, invite.authcode(), ) .await?; - // TODO code duplication - context - .sql - .execute( - "INSERT INTO smtp (rfc724_mid, recipients, mime, msg_id) - VALUES (?1, ?2, ?3, ?4)", - ( - &rfc724_mid, - &recipient, - &rendered_message, - 0, // TODO - ), - ) - .await?; - + let msg_id = message::insert_tombstone(context, &rfc724_mid).await?; + insert_into_smtp(context, &rfc724_mid, recipient, rendered_message, msg_id).await?; context.scheduler.interrupt_smtp().await; } else { let mut msg = Message { diff --git a/src/token.rs b/src/token.rs index 1a189a151f..d144b71ab0 100644 --- a/src/token.rs +++ b/src/token.rs @@ -62,6 +62,17 @@ pub async fn lookup( .await } +pub async fn lookup_all(context: &Context, namespace: Namespace) -> Result> { + context + .sql + .query_map_vec( + "SELECT token FROM tokens WHERE namespc=? ORDER BY timestamp DESC LIMIT 1", + (namespace,), + |row| Ok(row.get(0)?), + ) + .await +} + pub async fn lookup_or_new( context: &Context, namespace: Namespace, From 4c00bc109fbdf3c57a732e6003031ff2c131286c Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 21 Jan 2026 13:32:10 +0100 Subject: [PATCH 09/36] Fix some bugs when sending vc-pubkey, decrypt vc-pubkey on Bob's side --- src/mimeparser.rs | 53 +++++++++++++++++++++++++++++++++++-------- src/securejoin.rs | 8 +++---- src/securejoin/bob.rs | 1 + 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 8982416dd1..6e3460dbec 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -355,7 +355,7 @@ impl MimeMessage { // Remove headers that are allowed _only_ in the encrypted+signed part. It's ok to leave // them in signed-only emails, but has no value currently. - Self::remove_secured_headers(&mut headers, &mut headers_removed); + Self::remove_secured_headers(&mut headers, &mut headers_removed, false); let mut from = from.context("No from in message")?; let private_keyring = load_self_secret_keyring(context).await?; @@ -382,6 +382,11 @@ impl MimeMessage { let mail_raw; // Memory location for a possible decrypted message. let decrypted_msg; // Decrypted signed OpenPGP message. + + // TODO performance: + // - maybe we should start sorting by timestamp + // - we shouldn't do 3 SQL requests even if the message isn't symm-encrypted + // - we're loading the whole bobstate, just to get the auth token let mut secrets: Vec = context .sql .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| { @@ -389,8 +394,24 @@ impl MimeMessage { Ok(secret) }) .await?; - secrets.extend(token::lookup_all(context, token::Namespace::Auth).await?); + context + .sql + .query_map( + "SELECT id, invite FROM bobstate", + (), + |row| { + let invite: crate::securejoin::QrInvite = row.get(1)?; + Ok(invite) + }, + |rows| { + for row in rows { + secrets.push(row?.authcode().to_string()); + } + Ok(()) + }, + ) + .await?; let (mail, is_encrypted) = match tokio::task::block_in_place(|| try_decrypt(&mail, &private_keyring, &secrets)) { @@ -608,7 +629,7 @@ impl MimeMessage { } } if signatures.is_empty() { - Self::remove_secured_headers(&mut headers, &mut headers_removed); + Self::remove_secured_headers(&mut headers, &mut headers_removed, is_encrypted); } if !is_encrypted { signatures.clear(); @@ -1707,20 +1728,34 @@ impl MimeMessage { .and_then(|msgid| parse_message_id(msgid).ok()) } + /// Remove headers that are only allowed to be present in an encrypted-and-signed message: fn remove_secured_headers( headers: &mut HashMap, removed: &mut HashSet, + // Whether the message was encrypted (but not signed): + encrypted: bool, ) { remove_header(headers, "secure-join-fingerprint", removed); - remove_header(headers, "secure-join-auth", removed); remove_header(headers, "chat-verified", removed); remove_header(headers, "autocrypt-gossip", removed); - // Secure-Join is secured unless it is an initial "vc-request"/"vg-request". - if let Some(secure_join) = remove_header(headers, "secure-join", removed) - && (secure_join == "vc-request" || secure_join == "vg-request") - { - headers.insert("secure-join".to_string(), secure_join); + if headers.get("secure-join") == Some(&"vc-request-pubkey".to_string()) && encrypted { + // vc-request-pubkey message is encrypted, but unsigned, + // and contains a Secure-Join-Auth header. + // + // It is unsigned in order not to leak Bob's identity to a server operator + // that scraped the AUTH token somewhere from the web, + // and because Alice anyways couldn't verify his signature at this step, + // because she doesn't know his public key yet. + } else { + remove_header(headers, "secure-join-auth", removed); + + // Secure-Join is secured unless it is an initial "vc-request"/"vg-request". + if let Some(secure_join) = remove_header(headers, "secure-join", removed) + && (secure_join == "vc-request" || secure_join == "vg-request") + { + headers.insert("secure-join".to_string(), secure_join); + } } } diff --git a/src/securejoin.rs b/src/securejoin.rs index 219242a71e..816c4b368a 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -448,7 +448,7 @@ pub(crate) async fn handle_securejoin_handshake( // https://www.rfc-editor.org/rfc/rfc9580.html#name-surreptitious-forwarding if !matches!( step, - SecureJoinStep::Request { .. } | SecureJoinStep::RequestPubkey + SecureJoinStep::Request { .. } | SecureJoinStep::RequestPubkey | SecureJoinStep::Pubkey ) { let mut self_found = false; let self_fingerprint = load_self_public_key(context).await?.dc_fingerprint(); @@ -523,8 +523,8 @@ pub(crate) async fn handle_securejoin_handshake( ==== Bob requests our public key (Securejoin v3) ===== ========================================================*/ - if !mime_message.was_encrypted() { - warn!(context, "Ignoring unencrypted RequestPubkey"); + if mime_message.signature.is_some() { + warn!(context, "RequestPubkey is not supposed to be signed"); return Ok(HandshakeMessage::Ignore); } let Some(auth) = mime_message.get_header(HeaderDef::SecureJoinAuth) else { @@ -562,7 +562,7 @@ pub(crate) async fn handle_securejoin_handshake( ==== Bob - the joiner's side ===== ==== Alice sent us her pubkey (Securejoin v3) ===== ========================================================*/ - todo!() + todo!("Hocuri Pubkey not implemented") } SecureJoinStep::RequestWithAuth => { /*========================================================== diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 6b18b3d179..fadf7cde4b 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -302,6 +302,7 @@ pub(crate) async fn send_handshake_message( step: BobHandshakeMsg, ) -> Result<()> { if invite.is_v3() && matches!(step, BobHandshakeMsg::Request) { + // TODO this code might be moved "up" into the caller function // Send a minimal symmetrically-encrypted vc-request message let rfc724_mid = create_outgoing_rfc724_mid(); From d979938b44adb35e77f1998e9376fbe95b5435ca Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 21 Jan 2026 15:38:50 +0100 Subject: [PATCH 10/36] Handle vc-pubkey on Bob's side --- src/mimefactory.rs | 2 +- src/securejoin.rs | 4 ++-- src/securejoin/bob.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index c82cd416b0..0034a1db5f 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -2309,7 +2309,7 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( "Auto-Submitted", mail_builder::headers::raw::Raw::new("auto-generated".to_string()).into(), )); - } else if step != "vc-request" { + } else if step != "vc-request-pubkey" { headers.push(( "Auto-Submitted", mail_builder::headers::raw::Raw::new("auto-replied".to_string()).into(), diff --git a/src/securejoin.rs b/src/securejoin.rs index 816c4b368a..70e7bb0185 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -515,7 +515,7 @@ pub(crate) async fn handle_securejoin_handshake( ==== Bob - the joiner's side ===== ==== Step 4 in "Setup verified contact" protocol ===== ========================================================*/ - bob::handle_auth_required(context, mime_message).await + bob::handle_auth_required_or_pubkey(context, mime_message).await } SecureJoinStep::RequestPubkey => { /*======================================================== @@ -562,7 +562,7 @@ pub(crate) async fn handle_securejoin_handshake( ==== Bob - the joiner's side ===== ==== Alice sent us her pubkey (Securejoin v3) ===== ========================================================*/ - todo!("Hocuri Pubkey not implemented") + bob::handle_auth_required_or_pubkey(context, mime_message).await } SecureJoinStep::RequestWithAuth => { /*========================================================== diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index fadf7cde4b..e4e9befa36 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -215,11 +215,11 @@ LIMIT 1 Ok(()) } -/// Handles `vc-auth-required` and `vg-auth-required` handshake messages. +/// Handles `vc-auth-required`, `vg-auth-required`, and `vc-pubkey` handshake messages. /// /// # Bob - the joiner's side /// ## Step 4 in the "Setup Contact protocol" -pub(super) async fn handle_auth_required( +pub(super) async fn handle_auth_required_or_pubkey( context: &Context, message: &MimeMessage, ) -> Result { From c9bd5d09c21d40f845d5981ab9ecf690a6b9465c Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 21 Jan 2026 15:39:41 +0100 Subject: [PATCH 11/36] Adapt some tests --- src/chat/chat_tests.rs | 35 ++++++++++++------------------ src/securejoin/securejoin_tests.rs | 30 ++++++++++++++----------- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 5befc53cff..1be8cb46d2 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -2731,27 +2731,24 @@ async fn test_broadcast_members_cant_see_each_other() -> Result<()> { join_securejoin(charlie, &qr).await.unwrap(); let request = charlie.pop_sent_msg().await; - assert_eq!(request.recipients, "alice@example.org charlie@example.net"); + assert_eq!(request.recipients, "alice@example.org"); alice.recv_msg_trash(&request).await; } - tcm.section("Alice sends auth-required"); + tcm.section("Alice sends vc-pubkey"); { - let auth_required = alice.pop_sent_msg().await; - assert_eq!( - auth_required.recipients, - "charlie@example.net alice@example.org" - ); - let parsed = charlie.parse_msg(&auth_required).await; + let vc_pubkey = alice.pop_sent_msg().await; + assert_eq!(vc_pubkey.recipients, "charlie@example.net"); + let parsed = charlie.parse_msg(&vc_pubkey).await; assert!(parsed.get_header(HeaderDef::AutocryptGossip).is_some()); assert!(parsed.decoded_data_contains("charlie@example.net")); assert_eq!(parsed.decoded_data_contains("bob@example.net"), false); - let parsed_by_bob = bob.parse_msg(&auth_required).await; + let parsed_by_bob = bob.parse_msg(&vc_pubkey).await; assert!(parsed_by_bob.decrypting_failed); - charlie.recv_msg_trash(&auth_required).await; + charlie.recv_msg_trash(&vc_pubkey).await; } tcm.section("Charlie sends request-with-auth"); @@ -2992,9 +2989,8 @@ async fn test_broadcast_recipients_sync1() -> Result<()> { alice1.recv_msg_trash(&request).await; alice2.recv_msg_trash(&request).await; - let auth_required = alice1.pop_sent_msg().await; - charlie.recv_msg_trash(&auth_required).await; - alice2.recv_msg_trash(&auth_required).await; + let vc_pubkey = alice1.pop_sent_msg().await; + charlie.recv_msg_trash(&vc_pubkey).await; let request_with_auth = charlie.pop_sent_msg().await; alice1.recv_msg_trash(&request_with_auth).await; @@ -3470,16 +3466,13 @@ async fn test_leave_broadcast_multidevice() -> Result<()> { join_securejoin(bob0, &qr).await.unwrap(); let request = bob0.pop_sent_msg().await; - assert_eq!(request.recipients, "alice@example.org bob@example.net"); + assert_eq!(request.recipients, "alice@example.org"); alice.recv_msg_trash(&request).await; - let auth_required = alice.pop_sent_msg().await; - assert_eq!( - auth_required.recipients, - "bob@example.net alice@example.org" - ); + let vc_pubkey = alice.pop_sent_msg().await; + assert_eq!(vc_pubkey.recipients, "bob@example.net"); - bob0.recv_msg_trash(&auth_required).await; + bob0.recv_msg_trash(&vc_pubkey).await; let request_with_auth = bob0.pop_sent_msg().await; assert_eq!( request_with_auth.recipients, @@ -3495,7 +3488,7 @@ async fn test_leave_broadcast_multidevice() -> Result<()> { assert_eq!(rcvd.param.get_cmd(), SystemMessage::MemberAddedToGroup); tcm.section("Bob's second device also receives these messages"); - bob1.recv_msg_trash(&auth_required).await; + bob1.recv_msg_trash(&vc_pubkey).await; bob1.recv_msg_trash(&request_with_auth).await; bob1.recv_msg(&member_added).await; diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 6c309089e9..18e2403bcb 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -27,7 +27,7 @@ enum SetupContactCase { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_setup_contact() { +async fn test_setup_contact_basic() { test_setup_contact_ex(SetupContactCase::Normal).await } @@ -118,12 +118,15 @@ async fn test_setup_contact_ex(case: SetupContactCase) { assert!(!sent.payload.contains("Bob Examplenet")); assert_eq!(sent.recipient(), EmailAddress::new(alice_addr).unwrap()); let msg = alice.parse_msg(&sent).await; - assert!(!msg.was_encrypted()); - assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-request"); - assert!(msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some()); + assert!(msg.signature.is_none()); + assert_eq!( + msg.get_header(HeaderDef::SecureJoin).unwrap(), + "vc-request-pubkey" + ); + assert!(msg.get_header(HeaderDef::SecureJoinAuth).is_some()); assert!(!msg.header_exists(HeaderDef::AutoSubmitted)); - tcm.section("Step 3: Alice receives vc-request, sends vc-auth-required"); + tcm.section("Step 3: Alice receives vc-request-pubkey, sends vc-pubkey"); alice.recv_msg_trash(&sent).await; assert_eq!( Chatlist::try_load(&alice, 0, None, None) @@ -138,10 +141,7 @@ async fn test_setup_contact_ex(case: SetupContactCase) { assert!(!sent.payload.contains("Alice Exampleorg")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); - assert_eq!( - msg.get_header(HeaderDef::SecureJoin).unwrap(), - "vc-auth-required" - ); + assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-pubkey"); let bob_chat = bob.get_chat(&alice).await; assert_eq!(bob_chat.can_send(&bob).await.unwrap(), true); @@ -421,11 +421,12 @@ async fn test_setup_contact_concurrent_calls() -> Result<()> { assert!(!alice_id.is_special()); assert_eq!(chat.typ, Chattype::Single); assert_ne!(claire_id, alice_id); - assert!( + assert_eq!( bob.pop_sent_msg() .await .payload() - .contains("alice@example.org") + .contains("alice@example.org"), + false // Alice's address must not be sent in cleartext ); Ok(()) @@ -456,8 +457,11 @@ async fn test_secure_join() -> Result<()> { EmailAddress::new("alice@example.org").unwrap() ); let msg = alice.parse_msg(&sent).await; - assert!(!msg.was_encrypted()); - assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vg-request"); + assert!(msg.signature.is_none()); + assert_eq!( + msg.get_header(HeaderDef::SecureJoin).unwrap(), + "vg-request-pubkey" + ); assert!(msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some()); assert!(!msg.header_exists(HeaderDef::AutoSubmitted)); From c3d4c438e6e39ec4a147004261e7b985d75afac3 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 21 Jan 2026 16:19:16 +0100 Subject: [PATCH 12/36] fix: Don't leak cryptographic identity by signing vc-request-pubkey --- benches/decrypting.rs | 2 +- src/e2ee.rs | 7 ++++++- src/mimefactory.rs | 7 +++++-- src/pgp.rs | 10 ++++++---- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/benches/decrypting.rs b/benches/decrypting.rs index 4358507821..6dd7e89faf 100644 --- a/benches/decrypting.rs +++ b/benches/decrypting.rs @@ -83,7 +83,7 @@ fn criterion_benchmark(c: &mut Criterion) { let secret = secrets[NUM_SECRETS / 2].clone(); symm_encrypt_message( plain.clone(), - create_dummy_keypair("alice@example.org").unwrap().secret, + Some(create_dummy_keypair("alice@example.org").unwrap().secret), black_box(&secret), true, ) diff --git a/src/e2ee.rs b/src/e2ee.rs index 5c3ce91dcb..e98785bd20 100644 --- a/src/e2ee.rs +++ b/src/e2ee.rs @@ -70,8 +70,13 @@ impl EncryptHelper { shared_secret: &str, mail_to_encrypt: MimePart<'static>, compress: bool, + sign: bool, ) -> Result { - let sign_key = load_self_secret_key(context).await?; + let sign_key = if sign { + Some(load_self_secret_key(context).await?) + } else { + None + }; let mut raw_message = Vec::new(); let cursor = Cursor::new(&mut raw_message); diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 0034a1db5f..22af1b2bfe 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1169,8 +1169,9 @@ impl MimeFactory { } let encrypted = if let Some(shared_secret) = shared_secret { + let sign = true; encrypt_helper - .encrypt_symmetrically(context, &shared_secret, message, compress) + .encrypt_symmetrically(context, &shared_secret, message, compress, sign) .await? } else { // Asymmetric encryption @@ -2366,8 +2367,10 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( // there are no compression side channels // leaking information about the tokens. let compress = false; + // Only sign the message if we attach the pubkey. + let sign = attach_self_pubkey; let encrypted = encrypt_helper - .encrypt_symmetrically(context, auth, message, compress) // TODO this also signs the message. vc-request-pubkey shouldn't be signed. + .encrypt_symmetrically(context, auth, message, compress, sign) .await?; wrap_encrypted_part(encrypted) diff --git a/src/pgp.rs b/src/pgp.rs index bcf888873d..28f7ba9c49 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -480,7 +480,7 @@ pub async fn symm_encrypt_autocrypt_setup(passphrase: &str, plain: Vec) -> R /// `shared secret` is the secret that will be used for symmetric encryption. pub async fn symm_encrypt_message( plain: Vec, - private_key_for_signing: SignedSecretKey, + private_key_for_signing: Option, shared_secret: &str, compress: bool, ) -> Result { @@ -503,8 +503,10 @@ pub async fn symm_encrypt_message( ); msg.encrypt_with_password(&mut rng, s2k, &shared_secret)?; - let hash_algorithm = private_key_for_signing.hash_alg(); - msg.sign(&*private_key_for_signing, Password::empty(), hash_algorithm); + if let Some(private_key_for_signing) = private_key_for_signing.as_deref() { + let hash_algorithm = private_key_for_signing.hash_alg(); + msg.sign(private_key_for_signing, Password::empty(), hash_algorithm); + } if compress { msg.compression(CompressionAlgorithm::ZLIB); } @@ -737,7 +739,7 @@ mod tests { let shared_secret = "shared secret"; let ctext = symm_encrypt_message( plain.clone(), - load_self_secret_key(alice).await?, + Some(load_self_secret_key(alice).await?), shared_secret, true, ) From 7ff5cd63f8ac35335ba52c2058f41ce2a8fea4cb Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 21 Jan 2026 16:23:58 +0100 Subject: [PATCH 13/36] fix: Don't limit number of auth tokens to 1 --- src/token.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/token.rs b/src/token.rs index d144b71ab0..debd6366dd 100644 --- a/src/token.rs +++ b/src/token.rs @@ -66,7 +66,7 @@ pub async fn lookup_all(context: &Context, namespace: Namespace) -> Result Date: Wed, 21 Jan 2026 16:30:29 +0100 Subject: [PATCH 14/36] modify test --- src/securejoin/securejoin_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 18e2403bcb..62a59ece52 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -460,7 +460,7 @@ async fn test_secure_join() -> Result<()> { assert!(msg.signature.is_none()); assert_eq!( msg.get_header(HeaderDef::SecureJoin).unwrap(), - "vg-request-pubkey" + "vc-request-pubkey" ); assert!(msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some()); assert!(!msg.header_exists(HeaderDef::AutoSubmitted)); From 2358c9d51e2a3c9d713f276f595e12bcce14249e Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 21 Jan 2026 16:42:36 +0100 Subject: [PATCH 15/36] Update golden tests --- test-data/golden/test_broadcast_joining_golden_alice | 2 +- test-data/golden/test_sync_broadcast_alice1 | 6 +++--- test-data/golden/test_sync_broadcast_alice2 | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test-data/golden/test_broadcast_joining_golden_alice b/test-data/golden/test_broadcast_joining_golden_alice index 260726c625..2aaf2fdbc8 100644 --- a/test-data/golden/test_broadcast_joining_golden_alice +++ b/test-data/golden/test_broadcast_joining_golden_alice @@ -2,5 +2,5 @@ OutBroadcast#Chat#1001: My Channel [1 member(s)] Icon: e9b6c7a78aa2e4f415644f55a -------------------------------------------------------------------------------- Msg#1001: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] Msg#1002🔒: Me (Contact#Contact#Self): You changed the group image. [INFO] √ -Msg#1006🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#1005🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_alice1 b/test-data/golden/test_sync_broadcast_alice1 index d0d42cc228..7d450fb717 100644 --- a/test-data/golden/test_sync_broadcast_alice1 +++ b/test-data/golden/test_sync_broadcast_alice1 @@ -1,7 +1,7 @@ OutBroadcast#Chat#1001: Channel [0 member(s)] -------------------------------------------------------------------------------- Msg#1001: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#1007🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -Msg#1009🔒: Me (Contact#Contact#Self): hi √ -Msg#1010🔒: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ +Msg#1006🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#1008🔒: Me (Contact#Contact#Self): hi √ +Msg#1009🔒: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_alice2 b/test-data/golden/test_sync_broadcast_alice2 index ef143e0d8d..6672cd38df 100644 --- a/test-data/golden/test_sync_broadcast_alice2 +++ b/test-data/golden/test_sync_broadcast_alice2 @@ -1,7 +1,7 @@ OutBroadcast#Chat#1001: Channel [0 member(s)] -------------------------------------------------------------------------------- Msg#1002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#1007🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -Msg#1009🔒: Me (Contact#Contact#Self): hi √ -Msg#1010🔒: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ +Msg#1006🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#1008🔒: Me (Contact#Contact#Self): hi √ +Msg#1009🔒: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ -------------------------------------------------------------------------------- From 90b0f946ec49eca8449f0ef137e62ef21416cb0e Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 21 Jan 2026 22:16:08 +0100 Subject: [PATCH 16/36] Make the remaining Rust tests pass --- src/chat/chat_tests.rs | 17 ++++++++------ src/mimefactory.rs | 1 + src/securejoin/securejoin_tests.rs | 22 ++++++++----------- ...test_broadcast_joining_golden_private_chat | 2 +- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 1be8cb46d2..8e26366267 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -2741,8 +2741,8 @@ async fn test_broadcast_members_cant_see_each_other() -> Result<()> { let vc_pubkey = alice.pop_sent_msg().await; assert_eq!(vc_pubkey.recipients, "charlie@example.net"); let parsed = charlie.parse_msg(&vc_pubkey).await; - assert!(parsed.get_header(HeaderDef::AutocryptGossip).is_some()); - assert!(parsed.decoded_data_contains("charlie@example.net")); + assert!(parsed.get_header(HeaderDef::AutocryptGossip).is_none()); + assert_eq!(parsed.decoded_data_contains("charlie@example.net"), false); assert_eq!(parsed.decoded_data_contains("bob@example.net"), false); let parsed_by_bob = bob.parse_msg(&vc_pubkey).await; @@ -3182,14 +3182,17 @@ async fn test_broadcast_joining_golden() -> Result<()> { .await; let alice_bob_contact = alice.add_or_lookup_contact_no_key(bob).await; - let private_chat = ChatIdBlocked::lookup_by_contact(alice, alice_bob_contact.id) - .await? - .unwrap(); // The 1:1 chat with Bob should not be visible to the user: - assert_eq!(private_chat.blocked, Blocked::Yes); + assert!( + ChatIdBlocked::lookup_by_contact(alice, alice_bob_contact.id) + .await? + .is_none() + ); + let private_chat_id = + ChatId::create_for_contact_with_blocked(alice, alice_bob_contact.id, Blocked::Not).await?; alice .golden_test_chat( - private_chat.id, + private_chat_id, "test_broadcast_joining_golden_private_chat", ) .await; diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 22af1b2bfe..08500899bb 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -2310,6 +2310,7 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( "Auto-Submitted", mail_builder::headers::raw::Raw::new("auto-generated".to_string()).into(), )); + // TODO it's not nice that we're comparing strings here } else if step != "vc-request-pubkey" { headers.push(( "Auto-Submitted", diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 62a59ece52..6e79e288ea 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -462,7 +462,7 @@ async fn test_secure_join() -> Result<()> { msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-request-pubkey" ); - assert!(msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some()); + assert!(msg.get_header(HeaderDef::SecureJoinAuth).is_some()); assert!(!msg.header_exists(HeaderDef::AutoSubmitted)); // Old Delta Chat core sent `Secure-Join-Group` header in `vg-request`, @@ -473,19 +473,16 @@ async fn test_secure_join() -> Result<()> { // is only sent in `vg-request-with-auth` for compatibility. assert!(!msg.header_exists(HeaderDef::SecureJoinGroup)); - tcm.section("Step 3: Alice receives vg-request, sends vg-auth-required"); + tcm.section("Step 3: Alice receives vc-request-pubkey, sends vc-pubkey"); alice.recv_msg_trash(&sent).await; let sent = alice.pop_sent_msg().await; assert!(sent.payload.contains("Auto-Submitted: auto-replied")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); - assert_eq!( - msg.get_header(HeaderDef::SecureJoin).unwrap(), - "vg-auth-required" - ); + assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-pubkey"); - tcm.section("Step 4: Bob receives vg-auth-required, sends vg-request-with-auth"); + tcm.section("Step 4: Bob receives vc-pubkey, sends vg-request-with-auth"); bob.recv_msg_trash(&sent).await; let sent = bob.pop_sent_msg().await; @@ -579,13 +576,12 @@ async fn test_secure_join() -> Result<()> { { // Now Alice's chat with Bob should still be hidden, the verified message should // appear in the group chat. - - let chat = alice.get_chat(&bob).await; - assert_eq!( - chat.blocked, - Blocked::Yes, - "Alice's 1:1 chat with Bob is not hidden" + assert!( + ChatIdBlocked::lookup_by_contact(&alice, contact_bob.id) + .await? + .is_none() ); + // There should be 2 messages in the chat: // - The ChatProtectionEnabled message // - You added member bob@example.net diff --git a/test-data/golden/test_broadcast_joining_golden_private_chat b/test-data/golden/test_broadcast_joining_golden_private_chat index d4a1739d41..a77ef169a8 100644 --- a/test-data/golden/test_broadcast_joining_golden_private_chat +++ b/test-data/golden/test_broadcast_joining_golden_private_chat @@ -1,4 +1,4 @@ Single#Chat#1002: bob@example.net [KEY bob@example.net] -------------------------------------------------------------------------------- -Msg#1003: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#1007: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -------------------------------------------------------------------------------- From aceb75bd280e32cfe5320f3ef6ebd904ce1f136c Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 22 Jan 2026 10:59:40 +0100 Subject: [PATCH 17/36] Update src/mimeparser.rs --- src/mimeparser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 6e3460dbec..a228df4b97 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -1728,7 +1728,7 @@ impl MimeMessage { .and_then(|msgid| parse_message_id(msgid).ok()) } - /// Remove headers that are only allowed to be present in an encrypted-and-signed message: + /// Remove headers that are only allowed to be present in an encrypted-and-signed message fn remove_secured_headers( headers: &mut HashMap, removed: &mut HashSet, From 8ee3c2a1317a9591f1975dcadd205546c48f9e3a Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 28 Jan 2026 15:55:48 +0100 Subject: [PATCH 18/36] Add failing test test_auth_token_is_synchronized() --- src/securejoin/securejoin_tests.rs | 40 +++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 6e79e288ea..0c32321f51 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -14,7 +14,7 @@ use crate::receive_imf::receive_imf; use crate::stock_str::{self, messages_e2e_encrypted}; use crate::test_utils::{ AVATAR_64x64_BYTES, AVATAR_64x64_DEDUPLICATED, TestContext, TestContextManager, - TimeShiftFalsePositiveNote, get_chat_msg, + TimeShiftFalsePositiveNote, get_chat_msg, sync, }; use crate::tools::SystemTime; @@ -1370,3 +1370,41 @@ gU6dGXsFMe/RpRHrIAkMAaM5xkxMDRuRJDxiUdS/X+Y8 Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_auth_token_is_synchronized() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice1 = &tcm.alice().await; + let alice2 = &tcm.alice().await; + let bob = &tcm.bob().await; + bob.set_config(Config::Displayname, Some("Bob")).await?; + + alice1.set_config_bool(Config::SyncMsgs, true).await?; + alice2.set_config_bool(Config::SyncMsgs, true).await?; + + // This creates first auth token: + let qr1 = get_securejoin_qr(alice1, None).await?; + + // This creates another auth token; both of them need to be synchronized + let qr2 = get_securejoin_qr(alice1, None).await?; + sync(alice1, alice2).await; + tcm.exec_securejoin_qr_multi_device(bob, &[alice1, alice2], &qr2) + .await; + + let contacts = Contact::get_all(alice2, 0, Some("Bob")).await?; + assert_eq!(contacts[0], alice2.add_or_lookup_contact_id(bob).await); + assert_eq!(contacts.len(), 1); + + let chatlist = Chatlist::try_load(alice2, 0, Some("Bob"), None).await?; + assert_eq!(chatlist.get_chat_id(0)?, alice2.get_chat(bob).await.id); + assert_eq!(chatlist.len(), 1); + + for qr in [qr1, qr2] { + let qr = check_qr(alice2, &qr).await?; + let qr = QrInvite::try_from(qr)?; + assert!(token::exists(alice2, Namespace::InviteNumber, qr.invitenumber()).await?); + assert!(token::exists(alice2, Namespace::Auth, qr.authcode()).await?); + } + + Ok(()) +} From 6b658a0e08d81f47f657bb3b50ab9dedac39067e Mon Sep 17 00:00:00 2001 From: Hocuri Date: Fri, 30 Jan 2026 17:43:41 +0100 Subject: [PATCH 19/36] fix: Synchronize all AUTH tokens Previously, AUTH tokens were only synchronized if an INVITE token was generated at the same time. This led to several user-visible bugs. Now, we always send a sync message after generating a new AUTH token. Since the INVITE token may have stayed the same, I added a UNIQUE bound to the `tokens` table, in order to prevent saving the same INVITE token many times. --- src/securejoin.rs | 21 ++++++++------------ src/securejoin/securejoin_tests.rs | 31 ++++++++++++++++++++++++++++-- src/sql/migrations.rs | 21 ++++++++++++++++++++ src/token.rs | 6 +++--- 4 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/securejoin.rs b/src/securejoin.rs index 70e7bb0185..59e9659f51 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -127,9 +127,6 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul None => None, }; let grpid = chat.as_ref().map(|c| c.grpid.as_str()); - let sync_token = token::lookup(context, Namespace::InviteNumber, grpid) - .await? - .is_none(); // Invite number is used to request the inviter key. let invitenumber = token::lookup_or_new(context, Namespace::InviteNumber, grpid).await?; @@ -156,12 +153,10 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul .unwrap_or_default(); let qr = if let Some(chat) = chat { - if sync_token { - context - .sync_qr_code_tokens(Some(chat.grpid.as_str())) - .await?; - context.scheduler.interrupt_smtp().await; - } + context + .sync_qr_code_tokens(Some(chat.grpid.as_str())) + .await?; + context.scheduler.interrupt_smtp().await; let chat_name = chat.get_name(); let chat_name_shortened = shorten_name(chat_name, 25); @@ -190,10 +185,10 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul let self_name_urlencoded = utf8_percent_encode(&self_name_shortened, DISALLOWED_CHARACTERS) .to_string() .replace("%20", "+"); - if sync_token { - context.sync_qr_code_tokens(None).await?; - context.scheduler.interrupt_smtp().await; - } + + context.sync_qr_code_tokens(None).await?; + context.scheduler.interrupt_smtp().await; + format!( "https://i.delta.chat/#{fingerprint}&v=3&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}", ) diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 0c32321f51..6a701abcda 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -1388,6 +1388,11 @@ async fn test_auth_token_is_synchronized() -> Result<()> { // This creates another auth token; both of them need to be synchronized let qr2 = get_securejoin_qr(alice1, None).await?; sync(alice1, alice2).await; + + // Note that Bob will throw away the AUTH token after sending `vc-request-with-auth`. + // Therefore, he will fail to decrypt the answer from alice, + // which leads to a "decryption failed: missing key" message in the logs. + // This is fine. tcm.exec_securejoin_qr_multi_device(bob, &[alice1, alice2], &qr2) .await; @@ -1400,11 +1405,33 @@ async fn test_auth_token_is_synchronized() -> Result<()> { assert_eq!(chatlist.len(), 1); for qr in [qr1, qr2] { - let qr = check_qr(alice2, &qr).await?; - let qr = QrInvite::try_from(qr)?; + let qr = check_qr(bob, &qr).await?; + let qr = QrInvite::try_from(dbg!(qr))?; assert!(token::exists(alice2, Namespace::InviteNumber, qr.invitenumber()).await?); assert!(token::exists(alice2, Namespace::Auth, qr.authcode()).await?); } + // Check that alice2 only saves the invite number twice: + let invite_count: u32 = alice2 + .sql + .query_get_value( + "SELECT COUNT(*) FROM tokens WHERE namespc=?;", + (Namespace::InviteNumber,), + ) + .await? + .unwrap(); + assert_eq!(invite_count, 1); + + // ...but knows two AUTH tokens: + let auth_count: u32 = alice2 + .sql + .query_get_value( + "SELECT COUNT(*) FROM tokens WHERE namespc=?;", + (Namespace::Auth,), + ) + .await? + .unwrap(); + assert_eq!(auth_count, 2); + Ok(()) } diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index 18d1330256..c3937eaac1 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -1525,6 +1525,27 @@ ALTER TABLE contacts ADD COLUMN name_normalized TEXT; .await?; } + // Add UNIQUE bound to token, in order to avoid saving the same token multiple times + inc_and_check(&mut migration_version, 146)?; + if dbversion < migration_version { + sql.execute_migration( + "CREATE TABLE tokens_new ( + id INTEGER PRIMARY KEY, + namespc INTEGER DEFAULT 0, + foreign_key TEXT DEFAULT '', + token TEXT DEFAULT '' UNIQUE, + timestamp INTEGER DEFAULT 0, + foreign_id INTEGER NOT NULL DEFAULT 0 + ) STRICT; + INSERT OR IGNORE INTO tokens_new + SELECT id, namespc, foreign_key, token, timestamp, foreign_id FROM tokens; + DROP TABLE tokens; + ALTER TABLE tokens_new RENAME TO tokens;", + migration_version, + ) + .await?; + } + let new_version = sql .get_raw_config_int(VERSION_CFG) .await? diff --git a/src/token.rs b/src/token.rs index debd6366dd..87e85e5aaf 100644 --- a/src/token.rs +++ b/src/token.rs @@ -33,7 +33,7 @@ pub async fn save( context .sql .execute( - "INSERT INTO tokens (namespc, foreign_key, token, timestamp) VALUES (?, ?, ?, ?)", + "INSERT OR IGNORE INTO tokens (namespc, foreign_key, token, timestamp) VALUES (?, ?, ?, ?)", (namespace, foreign_key.unwrap_or(""), token, timestamp), ) .await?; @@ -56,7 +56,7 @@ pub async fn lookup( context .sql .query_get_value( - "SELECT token FROM tokens WHERE namespc=? AND foreign_key=? ORDER BY timestamp DESC LIMIT 1", + "SELECT token FROM tokens WHERE namespc=? AND foreign_key=? ORDER BY id DESC LIMIT 1", (namespace, foreign_key.unwrap_or("")), ) .await @@ -66,7 +66,7 @@ pub async fn lookup_all(context: &Context, namespace: Namespace) -> Result Date: Tue, 10 Feb 2026 12:28:26 +0100 Subject: [PATCH 20/36] bench: Also add auth tokens into benchmark --- benches/decrypting.rs | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/benches/decrypting.rs b/benches/decrypting.rs index 7067d8f8a8..e93eb1309f 100644 --- a/benches/decrypting.rs +++ b/benches/decrypting.rs @@ -25,11 +25,13 @@ //! You can benchmark this by adapting the `NUM_SECRETS` variable. use std::hint::black_box; +use std::sync::LazyLock; use criterion::{Criterion, criterion_group, criterion_main}; use deltachat::internals_for_benches::create_broadcast_secret; use deltachat::internals_for_benches::create_dummy_keypair; use deltachat::internals_for_benches::save_broadcast_secret; +use deltachat::securejoin::get_securejoin_qr; use deltachat::{ Events, chat::ChatId, @@ -44,7 +46,18 @@ use deltachat::{ use rand::{Rng, rng}; use tempfile::tempdir; -const NUM_SECRETS: usize = 500; +static NUM_BROADCAST_SECRETS: LazyLock = LazyLock::new(|| { + std::env::var("NUM_BROADCAST_SECRETS") + .unwrap_or("500".to_string()) + .parse() + .unwrap() +}); +static NUM_AUTH_TOKENS: LazyLock = LazyLock::new(|| { + std::env::var("NUM_AUTH_TOKENS") + .unwrap_or("5000".to_string()) + .parse() + .unwrap() +}); async fn create_context() -> Context { let dir = tempdir().unwrap(); @@ -80,7 +93,7 @@ fn criterion_benchmark(c: &mut Criterion) { let plain = generate_plaintext(); let secrets = generate_secrets(); let encrypted = tokio::runtime::Runtime::new().unwrap().block_on(async { - let secret = secrets[NUM_SECRETS / 2].clone(); + let secret = secrets[*NUM_BROADCAST_SECRETS / 2].clone(); symm_encrypt_message( plain.clone(), Some(create_dummy_keypair("alice@example.org").unwrap().secret), @@ -139,7 +152,7 @@ fn criterion_benchmark(c: &mut Criterion) { // "secret" is the shared secret that was used to encrypt text_symmetrically_encrypted.eml. // Put it into the middle of our secrets: - secrets[NUM_SECRETS / 2] = "secret".to_string(); + secrets[*NUM_BROADCAST_SECRETS / 2] = "secret".to_string(); let context = rt.block_on(async { let context = create_context().await; @@ -148,6 +161,10 @@ fn criterion_benchmark(c: &mut Criterion) { .await .unwrap(); } + for _i in 0..*NUM_AUTH_TOKENS { + get_securejoin_qr(&context, None).await.unwrap(); + } + println!("NUM_AUTH_TOKENS={}", *NUM_AUTH_TOKENS); context }); @@ -185,9 +202,10 @@ fn criterion_benchmark(c: &mut Criterion) { } fn generate_secrets() -> Vec { - let secrets: Vec = (0..NUM_SECRETS) + let secrets: Vec = (0..*NUM_BROADCAST_SECRETS) .map(|_| create_broadcast_secret()) .collect(); + println!("NUM_BROADCAST_SECRETS={}", *NUM_BROADCAST_SECRETS); secrets } From 50eb9237090bb1954b7317ec65e72ece3e38d3cc Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 13:12:43 +0100 Subject: [PATCH 21/36] bench: Add black_box --- benches/decrypting.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benches/decrypting.rs b/benches/decrypting.rs index e93eb1309f..c8cdb65c55 100644 --- a/benches/decrypting.rs +++ b/benches/decrypting.rs @@ -178,7 +178,7 @@ fn criterion_benchmark(c: &mut Criterion) { ) .await .unwrap(); - assert_eq!(text, "Symmetrically encrypted message"); + assert_eq!(black_box(text), "Symmetrically encrypted message"); } }); }); @@ -193,7 +193,7 @@ fn criterion_benchmark(c: &mut Criterion) { ) .await .unwrap(); - assert_eq!(text, "hi"); + assert_eq!(black_box(text), "hi"); } }); }); From 1b258530280b32a5ce8c091465ca852d7278f879 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 13:26:59 +0100 Subject: [PATCH 22/36] bench: Remove benchmarks that only decrypt without parsing I don't need them anymore, it seems unlikely they will be needed soon, and they made it hard to implement a performance improvement I'm planning --- benches/decrypting.rs | 79 +++---------------------------------------- 1 file changed, 5 insertions(+), 74 deletions(-) diff --git a/benches/decrypting.rs b/benches/decrypting.rs index c8cdb65c55..5cfcc7f7dd 100644 --- a/benches/decrypting.rs +++ b/benches/decrypting.rs @@ -8,17 +8,16 @@ //! cargo bench --bench decrypting --features="internals" //! ``` //! -//! or, if you want to only run e.g. the 'Decrypt a symmetrically encrypted message' benchmark: +//! or, if you want to only run e.g. the 'Decrypt and parse a symmetrically encrypted message' benchmark: //! //! ```text -//! cargo bench --bench decrypting --features="internals" -- 'Decrypt a symmetrically encrypted message' +//! cargo bench --bench decrypting --features="internals" -- 'Decrypt and parse a symmetrically encrypted message' //! ``` //! -//! You can also pass a substring. -//! So, you can run all 'Decrypt and parse' benchmarks with: +//! You can also pass a substring: //! //! ```text -//! cargo bench --bench decrypting --features="internals" -- 'Decrypt and parse' +//! cargo bench --bench decrypting --features="internals" -- 'symmetrically' //! ``` //! //! Symmetric decryption has to try out all known secrets, @@ -29,7 +28,6 @@ use std::sync::LazyLock; use criterion::{Criterion, criterion_group, criterion_main}; use deltachat::internals_for_benches::create_broadcast_secret; -use deltachat::internals_for_benches::create_dummy_keypair; use deltachat::internals_for_benches::save_broadcast_secret; use deltachat::securejoin::get_securejoin_qr; use deltachat::{ @@ -40,10 +38,9 @@ use deltachat::{ internals_for_benches::key_from_asc, internals_for_benches::parse_and_get_text, internals_for_benches::store_self_keypair, - pgp::{KeyPair, SeipdVersion, decrypt, pk_encrypt, symm_encrypt_message}, + pgp::KeyPair, stock_str::StockStrings, }; -use rand::{Rng, rng}; use tempfile::tempdir; static NUM_BROADCAST_SECRETS: LazyLock = LazyLock::new(|| { @@ -83,66 +80,6 @@ async fn create_context() -> Context { fn criterion_benchmark(c: &mut Criterion) { let mut group = c.benchmark_group("Decrypt"); - // =========================================================================================== - // Benchmarks for decryption only, without any other parsing - // =========================================================================================== - - group.sample_size(10); - - group.bench_function("Decrypt a symmetrically encrypted message", |b| { - let plain = generate_plaintext(); - let secrets = generate_secrets(); - let encrypted = tokio::runtime::Runtime::new().unwrap().block_on(async { - let secret = secrets[*NUM_BROADCAST_SECRETS / 2].clone(); - symm_encrypt_message( - plain.clone(), - Some(create_dummy_keypair("alice@example.org").unwrap().secret), - black_box(&secret), - true, - ) - .await - .unwrap() - }); - - b.iter(|| { - let mut msg = - decrypt(encrypted.clone().into_bytes(), &[], black_box(&secrets)).unwrap(); - let decrypted = msg.as_data_vec().unwrap(); - - assert_eq!(black_box(decrypted), plain); - }); - }); - - group.bench_function("Decrypt a public-key encrypted message", |b| { - let plain = generate_plaintext(); - let key_pair = create_dummy_keypair("alice@example.org").unwrap(); - let secrets = generate_secrets(); - let encrypted = tokio::runtime::Runtime::new().unwrap().block_on(async { - pk_encrypt( - plain.clone(), - vec![black_box(key_pair.public.clone())], - key_pair.secret.clone(), - true, - true, - SeipdVersion::V2, - ) - .await - .unwrap() - }); - - b.iter(|| { - let mut msg = decrypt( - encrypted.clone().into_bytes(), - std::slice::from_ref(&key_pair.secret), - black_box(&secrets), - ) - .unwrap(); - let decrypted = msg.as_data_vec().unwrap(); - - assert_eq!(black_box(decrypted), plain); - }); - }); - // =========================================================================================== // Benchmarks for the whole parsing pipeline, incl. decryption (but excl. receive_imf()) // =========================================================================================== @@ -209,11 +146,5 @@ fn generate_secrets() -> Vec { secrets } -fn generate_plaintext() -> Vec { - let mut plain: Vec = vec![0; 500]; - rng().fill(&mut plain[..]); - plain -} - criterion_group!(benches, criterion_benchmark); criterion_main!(benches); From 61ae74396239e0d3a569afe437b11d256bbb5e33 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 13:30:01 +0100 Subject: [PATCH 23/36] perf: Only load shared secrets if the message actually is symm-encrypted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This improves performance, though not a lot - 8% performance improvement if there is a huge number of shared secrets. Anyways, I think it's worth it. ``` NUM_BROADCAST_SECRETS=500 NUM_AUTH_TOKENS=5000 Decrypt/Decrypt and parse a symmetrically encrypted message time: [12.354 ms 12.364 ms 12.376 ms] change: [−0.5375% −0.3815% −0.2345%] (p = 0.00 < 0.05) Change within noise threshold. Found 7 outliers among 100 measurements (7.00%) 3 (3.00%) high mild 4 (4.00%) high severe Decrypt/Decrypt and parse a public-key encrypted message time: [12.269 ms 12.273 ms 12.278 ms] change: [−7.9163% −7.8439% −7.7724%] (p = 0.00 < 0.05) Performance has improved. Found 13 outliers among 100 measurements (13.00%) 8 (8.00%) high mild 5 (5.00%) high severe ``` --- src/decrypt.rs | 24 +++--- src/internals_for_benches.rs | 2 +- src/mimeparser.rs | 143 +++++++++++++++++++---------------- src/pgp.rs | 27 ++++--- 4 files changed, 105 insertions(+), 91 deletions(-) diff --git a/src/decrypt.rs b/src/decrypt.rs index 4012a8f41e..c85a17e361 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -1,34 +1,28 @@ -//! End-to-end decryption support. +//! Helper functions for decryption. +//! The actual decryption is done in the [`crate::pgp`] module. use std::collections::HashSet; +use std::io::Cursor; +use ::pgp::composed::Message; use anyhow::Result; use mailparse::ParsedMail; -use crate::key::{Fingerprint, SignedPublicKey, SignedSecretKey}; +use crate::key::{Fingerprint, SignedPublicKey}; use crate::pgp; -/// Tries to decrypt a message, but only if it is structured as an Autocrypt message. -/// -/// If successful and the message was encrypted, -/// returns the decrypted and decompressed message. -pub fn try_decrypt<'a>( - mail: &'a ParsedMail<'a>, - private_keyring: &'a [SignedSecretKey], - shared_secrets: &[String], -) -> Result>> { +pub fn get_encrypted_pgp_message<'a>(mail: &'a ParsedMail<'a>) -> Result>> { let Some(encrypted_data_part) = get_encrypted_mime(mail) else { return Ok(None); }; - let data = encrypted_data_part.get_body_raw()?; - let msg = pgp::decrypt(data, private_keyring, shared_secrets)?; - + let cursor = Cursor::new(data); + let (msg, _headers) = Message::from_armor(cursor)?; Ok(Some(msg)) } /// Returns a reference to the encrypted payload of a message. -pub(crate) fn get_encrypted_mime<'a, 'b>(mail: &'a ParsedMail<'b>) -> Option<&'a ParsedMail<'b>> { +pub fn get_encrypted_mime<'a, 'b>(mail: &'a ParsedMail<'b>) -> Option<&'a ParsedMail<'b>> { get_autocrypt_mime(mail) .or_else(|| get_mixed_up_mime(mail)) .or_else(|| get_attachment_mime(mail)) diff --git a/src/internals_for_benches.rs b/src/internals_for_benches.rs index 61fc9d7fca..b53cc37b72 100644 --- a/src/internals_for_benches.rs +++ b/src/internals_for_benches.rs @@ -35,4 +35,4 @@ pub fn create_dummy_keypair(addr: &str) -> Result { pub fn create_broadcast_secret() -> String { crate::tools::create_broadcast_secret() -} +} \ No newline at end of file diff --git a/src/mimeparser.rs b/src/mimeparser.rs index c699afa1fb..87b0baedee 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -20,7 +20,7 @@ use crate::chat::ChatId; use crate::config::Config; use crate::contact::ContactId; use crate::context::Context; -use crate::decrypt::{try_decrypt, validate_detached_signature}; +use crate::decrypt::{get_encrypted_pgp_message, validate_detached_signature}; use crate::dehtml::dehtml; use crate::download::PostMsgMetadata; use crate::events::EventType; @@ -380,82 +380,93 @@ impl MimeMessage { PreMessageMode::None }; - let mail_raw; // Memory location for a possible decrypted message. - let decrypted_msg; // Decrypted signed OpenPGP message. - // TODO performance: // - maybe we should start sorting by timestamp - // - we shouldn't do 3 SQL requests even if the message isn't symm-encrypted // - we're loading the whole bobstate, just to get the auth token - let mut secrets: Vec = context - .sql - .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| { - let secret: String = row.get(0)?; - Ok(secret) - }) - .await?; - secrets.extend(token::lookup_all(context, token::Namespace::Auth).await?); - context - .sql - .query_map( - "SELECT id, invite FROM bobstate", - (), - |row| { - let invite: crate::securejoin::QrInvite = row.get(1)?; - Ok(invite) - }, - |rows| { - for row in rows { - secrets.push(row?.authcode().to_string()); - } - Ok(()) - }, - ) - .await?; - let (mail, is_encrypted) = - match tokio::task::block_in_place(|| try_decrypt(&mail, &private_keyring, &secrets)) { - Ok(Some(mut msg)) => { - mail_raw = msg.as_data_vec().unwrap_or_default(); + let encrypted_pgp_message = get_encrypted_pgp_message(&mail)?; - let decrypted_mail = mailparse::parse_mail(&mail_raw)?; - if std::env::var(crate::DCC_MIME_DEBUG).is_ok() { - info!( - context, - "decrypted message mime-body:\n{}", - String::from_utf8_lossy(&mail_raw), - ); - } + let mut secrets: Vec; + if let Some(e) = &encrypted_pgp_message + && crate::pgp::check_symmetric_encryption(e).is_ok() + { + secrets = context + .sql + .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| { + let secret: String = row.get(0)?; + Ok(secret) + }) + .await?; + secrets.extend(token::lookup_all(context, token::Namespace::Auth).await?); + context + .sql + .query_map( + "SELECT id, invite FROM bobstate", + (), + |row| { + let invite: crate::securejoin::QrInvite = row.get(1)?; + Ok(invite) + }, + |rows| { + for row in rows { + secrets.push(row?.authcode().to_string()); + } + Ok(()) + }, + ) + .await?; + } else { + // No need to load all the secrets if the message isn't symmetrically encrypted + secrets = vec![]; + } - decrypted_msg = Some(msg); + let mail_raw; // Memory location for a possible decrypted message. + let decrypted_msg; // Decrypted signed OpenPGP message. - timestamp_sent = Self::get_timestamp_sent( - &decrypted_mail.headers, - timestamp_sent, - timestamp_rcvd, + let (mail, is_encrypted) = match tokio::task::block_in_place(|| { + encrypted_pgp_message.map(|e| crate::pgp::decrypt(e, &private_keyring, &secrets)) + }) { + Some(Ok(mut msg)) => { + mail_raw = msg.as_data_vec().unwrap_or_default(); + + let decrypted_mail = mailparse::parse_mail(&mail_raw)?; + if std::env::var(crate::DCC_MIME_DEBUG).is_ok() { + info!( + context, + "decrypted message mime-body:\n{}", + String::from_utf8_lossy(&mail_raw), ); + } - let protected_aheader_values = decrypted_mail - .headers - .get_all_values(HeaderDef::Autocrypt.into()); - if !protected_aheader_values.is_empty() { - aheader_values = protected_aheader_values; - } + decrypted_msg = Some(msg); - (Ok(decrypted_mail), true) - } - Ok(None) => { - mail_raw = Vec::new(); - decrypted_msg = None; - (Ok(mail), false) - } - Err(err) => { - mail_raw = Vec::new(); - decrypted_msg = None; - warn!(context, "decryption failed: {:#}", err); - (Err(err), false) + timestamp_sent = Self::get_timestamp_sent( + &decrypted_mail.headers, + timestamp_sent, + timestamp_rcvd, + ); + + let protected_aheader_values = decrypted_mail + .headers + .get_all_values(HeaderDef::Autocrypt.into()); + if !protected_aheader_values.is_empty() { + aheader_values = protected_aheader_values; } - }; + + (Ok(decrypted_mail), true) + } + None => { + mail_raw = Vec::new(); + decrypted_msg = None; + (Ok(mail), false) + } + Some(Err(err)) => { + mail_raw = Vec::new(); + decrypted_msg = None; + warn!(context, "decryption failed: {:#}", err); + (Err(err), false) + } + }; let mut autocrypt_header = None; if incoming { diff --git a/src/pgp.rs b/src/pgp.rs index b6841984b9..2172e48b35 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -327,13 +327,10 @@ pub fn pk_calc_signature( /// /// Returns the decrypted and decompressed message. pub fn decrypt( - ctext: Vec, + msg: Message<'static>, private_keys_for_decryption: &[SignedSecretKey], mut shared_secrets: &[String], ) -> Result> { - let cursor = Cursor::new(ctext); - let (msg, _headers) = Message::from_armor(cursor)?; - let skeys: Vec<&SignedSecretKey> = private_keys_for_decryption.iter().collect(); let empty_pw = Password::empty(); @@ -389,7 +386,9 @@ pub fn decrypt( /// with all of the known shared secrets. /// In order to prevent this, we do not try to symmetrically decrypt messages /// that use a string2key algorithm other than 'Salted'. -fn check_symmetric_encryption(msg: &Message<'_>) -> std::result::Result<(), &'static str> { +pub(crate) fn check_symmetric_encryption( + msg: &Message<'_>, +) -> std::result::Result<(), &'static str> { let Message::Encrypted { esk, .. } = msg else { return Err("not encrypted"); }; @@ -548,6 +547,16 @@ mod tests { use pgp::composed::Esk; use pgp::packet::PublicKeyEncryptedSessionKey; + fn decrypt_bytes( + bytes: Vec, + private_keys_for_decryption: &[SignedSecretKey], + shared_secrets: &[String], + ) -> Result> { + let cursor = Cursor::new(bytes); + let (msg, _headers) = Message::from_armor(cursor).unwrap(); + decrypt(msg, private_keys_for_decryption, shared_secrets) + } + #[expect(clippy::type_complexity)] fn pk_decrypt_and_validate<'a>( ctext: &'a [u8], @@ -558,7 +567,7 @@ mod tests { HashMap>, Vec, )> { - let mut msg = decrypt(ctext.to_vec(), private_keys_for_decryption, &[])?; + let mut msg = decrypt_bytes(ctext.to_vec(), private_keys_for_decryption, &[])?; let content = msg.as_data_vec()?; let ret_signature_fingerprints = valid_signature_fingerprints(&msg, public_keys_for_validation); @@ -746,7 +755,7 @@ mod tests { .await?; let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?; - let mut decrypted = decrypt( + let mut decrypted = decrypt_bytes( ctext.into(), &bob_private_keyring, &[shared_secret.to_string()], @@ -789,7 +798,7 @@ mod tests { // Trying to decrypt it should fail with a helpful error message: let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?; - let error = decrypt( + let error = decrypt_bytes( ctext.into(), &bob_private_keyring, &[shared_secret.to_string()], @@ -826,7 +835,7 @@ mod tests { // Trying to decrypt it should fail with an OK error message: let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?; - let error = decrypt(ctext.into(), &bob_private_keyring, &[]).unwrap_err(); + let error = decrypt_bytes(ctext.into(), &bob_private_keyring, &[]).unwrap_err(); assert_eq!( error.to_string(), From 7a814116a7d9ff65b894b53ce4cd2628b1df996d Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 15:10:45 +0100 Subject: [PATCH 24/36] perf: Load BobState shared secrets first; refactor: extract fn load_shared_secrets() --- src/mimeparser.rs | 62 ++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 87b0baedee..9b05071196 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -380,41 +380,13 @@ impl MimeMessage { PreMessageMode::None }; - // TODO performance: - // - maybe we should start sorting by timestamp - // - we're loading the whole bobstate, just to get the auth token - let encrypted_pgp_message = get_encrypted_pgp_message(&mail)?; - let mut secrets: Vec; + let secrets: Vec; if let Some(e) = &encrypted_pgp_message && crate::pgp::check_symmetric_encryption(e).is_ok() { - secrets = context - .sql - .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| { - let secret: String = row.get(0)?; - Ok(secret) - }) - .await?; - secrets.extend(token::lookup_all(context, token::Namespace::Auth).await?); - context - .sql - .query_map( - "SELECT id, invite FROM bobstate", - (), - |row| { - let invite: crate::securejoin::QrInvite = row.get(1)?; - Ok(invite) - }, - |rows| { - for row in rows { - secrets.push(row?.authcode().to_string()); - } - Ok(()) - }, - ) - .await?; + secrets = load_shared_secrets(context).await?; } else { // No need to load all the secrets if the message isn't symmetrically encrypted secrets = vec![]; @@ -2122,6 +2094,36 @@ impl MimeMessage { } } +/// Loads all the shared secrets +/// that can be used to decrypt a symmetrically-encrypted message +async fn load_shared_secrets(context: &Context) -> Result> { + // First, try decrypting using the bobstate, + // because usually there will only be 1 or 2 of it, + // so, it should be fast + let mut secrets: Vec = context + .sql + .query_map_vec("SELECT id, invite FROM bobstate", (), |row| { + let invite: crate::securejoin::QrInvite = row.get(1)?; + Ok(invite.authcode().to_string()) + }) + .await?; + // Then, try decrypting using broadcast secrets + // Usually, the user won't be in more than ~10 broadcast channels + secrets.extend( + context + .sql + .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| { + let secret: String = row.get(0)?; + Ok(secret) + }) + .await?, + ); + // Finally, try decrypting using AUTH tokens + // There can be a lot of AUTH tokens, because a new one is generated every time a QR code is shown + secrets.extend(token::lookup_all(context, token::Namespace::Auth).await?); + Ok(secrets) +} + fn rm_legacy_display_elements(text: &str) -> String { let mut res = None; for l in text.lines() { From a442077f282f2c050c784bf6db22a0892ad1bbaf Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 15:34:15 +0100 Subject: [PATCH 25/36] Add explanatory performance comment --- src/token.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/token.rs b/src/token.rs index 87e85e5aaf..e8f15b3498 100644 --- a/src/token.rs +++ b/src/token.rs @@ -66,6 +66,9 @@ pub async fn lookup_all(context: &Context, namespace: Namespace) -> Result Date: Tue, 10 Feb 2026 15:37:04 +0100 Subject: [PATCH 26/36] Remove TODO --- src/securejoin/bob.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index e4e9befa36..114371c71e 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -302,9 +302,7 @@ pub(crate) async fn send_handshake_message( step: BobHandshakeMsg, ) -> Result<()> { if invite.is_v3() && matches!(step, BobHandshakeMsg::Request) { - // TODO this code might be moved "up" into the caller function // Send a minimal symmetrically-encrypted vc-request message - let rfc724_mid = create_outgoing_rfc724_mid(); let contact = Contact::get_by_id(context, invite.contact_id()).await?; let recipient = contact.get_addr(); From 642ba1d92c28e1d6e9567a8591b29db16c81f749 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 16:55:24 +0100 Subject: [PATCH 27/36] feat: Make it possible to omit the invitenumber and v=3 for Securejoin v3 When we finally get rid of legacy Securejoin, we can just leave them out, then. --- benches/decrypting.rs | 12 ++--- src/internals_for_benches.rs | 2 +- src/qr.rs | 13 +++++- src/securejoin/securejoin_tests.rs | 73 +++++++++++++++++++++++++----- 4 files changed, 76 insertions(+), 24 deletions(-) diff --git a/benches/decrypting.rs b/benches/decrypting.rs index 5cfcc7f7dd..b998754a5d 100644 --- a/benches/decrypting.rs +++ b/benches/decrypting.rs @@ -31,15 +31,9 @@ use deltachat::internals_for_benches::create_broadcast_secret; use deltachat::internals_for_benches::save_broadcast_secret; use deltachat::securejoin::get_securejoin_qr; use deltachat::{ - Events, - chat::ChatId, - config::Config, - context::Context, - internals_for_benches::key_from_asc, - internals_for_benches::parse_and_get_text, - internals_for_benches::store_self_keypair, - pgp::KeyPair, - stock_str::StockStrings, + Events, chat::ChatId, config::Config, context::Context, internals_for_benches::key_from_asc, + internals_for_benches::parse_and_get_text, internals_for_benches::store_self_keypair, + pgp::KeyPair, stock_str::StockStrings, }; use tempfile::tempdir; diff --git a/src/internals_for_benches.rs b/src/internals_for_benches.rs index b53cc37b72..61fc9d7fca 100644 --- a/src/internals_for_benches.rs +++ b/src/internals_for_benches.rs @@ -35,4 +35,4 @@ pub fn create_dummy_keypair(addr: &str) -> Result { pub fn create_broadcast_secret() -> String { crate::tools::create_broadcast_secret() -} \ No newline at end of file +} diff --git a/src/qr.rs b/src/qr.rs index 1ad3484756..906fbbfef9 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -492,7 +492,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { let name = decode_name(¶m, "n")?.unwrap_or_default(); - let invitenumber = param + let mut invitenumber = param .get("i") // For historic reansons, broadcasts currently use j instead of i for the invitenumber: .or_else(|| param.get("j")) @@ -510,7 +510,15 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { let grpname = decode_name(¶m, "g")?; let broadcast_name = decode_name(¶m, "b")?; - let is_v3 = param.get("v") == Some(&"3"); + let mut is_v3 = param.get("v") == Some(&"3"); + + if authcode.is_some() && invitenumber.is_none() { + // Securejoin v3 doesn't need an invitenumber. + // We want to remove the invitenumber and the `v=3` parameter eventually; + // therefore, we accept v3 QR codes without an invitenumber. + is_v3 = true; + invitenumber = Some("".to_string()); + } if let (Some(addr), Some(invitenumber), Some(authcode)) = (&addr, invitenumber, authcode) { let addr = ContactAddress::new(addr)?; @@ -530,6 +538,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { .await .with_context(|| format!("can't check if address {addr:?} is our address"))? { + // TODO check AUTH instead if token::exists(context, token::Namespace::InviteNumber, &invitenumber).await? { Ok(Qr::WithdrawVerifyGroup { grpname, diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 6a701abcda..229593b5b5 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -1,6 +1,7 @@ use std::time::Duration; use deltachat_contact_tools::EmailAddress; +use regex::Regex; use super::*; use crate::chat::{CantSendReason, add_contact_to_chat, remove_contact_from_chat}; @@ -433,7 +434,19 @@ async fn test_setup_contact_concurrent_calls() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_secure_join() -> Result<()> { +async fn test_secure_join_legacy() -> Result<()> { + test_secure_join(false, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_secure_join_v3() -> Result<()> { + test_secure_join(true, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_secure_join_v3_without_invite() -> Result<()> { + test_secure_join(true, true).await +} + +async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { let mut tcm = TestContextManager::new(); let alice = tcm.alice().await; let bob = tcm.bob().await; @@ -445,7 +458,21 @@ async fn test_secure_join() -> Result<()> { let alice_chatid = chat::create_group(&alice, "the chat").await?; tcm.section("Step 1: Generate QR-code, secure-join implied by chatid"); - let qr = get_securejoin_qr(&alice, Some(alice_chatid)).await.unwrap(); + let mut qr = get_securejoin_qr(&alice, Some(alice_chatid)).await.unwrap(); + if !v3 { + // Force legacy securejoin to run by removing the &v=3 parameter + let new_qr = Regex::new("&v=3").unwrap().replace(&qr, ""); + assert!(new_qr != qr); + qr = new_qr.to_string(); + } + if remove_invite { + // Remove the INVITENUBMER. It's not needed in Securejoin v3, + // but still included for backwards compatibility reasons. + // We want to be able to remove it in the future, however. + let new_qr = Regex::new("&i=.*?&").unwrap().replace(&qr, "&"); + assert!(new_qr != qr); + qr = new_qr.to_string(); + } tcm.section("Step 2: Bob scans QR-code, sends vg-request"); let bob_chatid = join_securejoin(&bob, &qr).await?; @@ -460,9 +487,17 @@ async fn test_secure_join() -> Result<()> { assert!(msg.signature.is_none()); assert_eq!( msg.get_header(HeaderDef::SecureJoin).unwrap(), - "vc-request-pubkey" + if v3 { + "vc-request-pubkey" + } else { + "vg-request" + } + ); + assert_eq!(msg.get_header(HeaderDef::SecureJoinAuth).is_some(), v3); + assert_eq!( + msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some(), + !v3 ); - assert!(msg.get_header(HeaderDef::SecureJoinAuth).is_some()); assert!(!msg.header_exists(HeaderDef::AutoSubmitted)); // Old Delta Chat core sent `Secure-Join-Group` header in `vg-request`, @@ -473,16 +508,19 @@ async fn test_secure_join() -> Result<()> { // is only sent in `vg-request-with-auth` for compatibility. assert!(!msg.header_exists(HeaderDef::SecureJoinGroup)); - tcm.section("Step 3: Alice receives vc-request-pubkey, sends vc-pubkey"); + tcm.section("Step 3: Alice receives vc-request-pubkey and sends vc-pubkey, or vg-request and sends vg-auth-required"); alice.recv_msg_trash(&sent).await; let sent = alice.pop_sent_msg().await; assert!(sent.payload.contains("Auto-Submitted: auto-replied")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); - assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-pubkey"); + assert_eq!( + msg.get_header(HeaderDef::SecureJoin).unwrap(), + if v3 { "vc-pubkey" } else { "vg-auth-required" } + ); - tcm.section("Step 4: Bob receives vc-pubkey, sends vg-request-with-auth"); + tcm.section("Step 4: Bob receives vc-pubkey or vg-auth-required, sends v*-request-with-auth"); bob.recv_msg_trash(&sent).await; let sent = bob.pop_sent_msg().await; @@ -576,11 +614,22 @@ async fn test_secure_join() -> Result<()> { { // Now Alice's chat with Bob should still be hidden, the verified message should // appear in the group chat. - assert!( - ChatIdBlocked::lookup_by_contact(&alice, contact_bob.id) - .await? - .is_none() - ); + if v3 { + // In version 3 of the Securejoin protocol, + // the chat is not even created + assert!( + ChatIdBlocked::lookup_by_contact(&alice, contact_bob.id) + .await? + .is_none() + ); + } else { + let chat = alice.get_chat(&bob).await; + assert_eq!( + chat.blocked, + Blocked::Yes, + "Alice's 1:1 chat with Bob is not hidden" + ); + } // There should be 2 messages in the chat: // - The ChatProtectionEnabled message From b5ae93c14c6bd09642d35aa9b3905ab8276f78e6 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 16:56:39 +0100 Subject: [PATCH 28/36] test: Test that securejoin v3 runs even without `v=3` when there is no invitenumber --- src/securejoin/securejoin_tests.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 229593b5b5..a864b8b384 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -459,12 +459,6 @@ async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { tcm.section("Step 1: Generate QR-code, secure-join implied by chatid"); let mut qr = get_securejoin_qr(&alice, Some(alice_chatid)).await.unwrap(); - if !v3 { - // Force legacy securejoin to run by removing the &v=3 parameter - let new_qr = Regex::new("&v=3").unwrap().replace(&qr, ""); - assert!(new_qr != qr); - qr = new_qr.to_string(); - } if remove_invite { // Remove the INVITENUBMER. It's not needed in Securejoin v3, // but still included for backwards compatibility reasons. @@ -473,6 +467,14 @@ async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { assert!(new_qr != qr); qr = new_qr.to_string(); } + if !v3 || remove_invite { + // If `!v3`, force legacy securejoin to run by removing the &v=3 parameter. + // If `remove_invite`, we can also remove the v=3 parameter, + // because any QR with AUTH but no INVITE must treated as a v3 QR code. + let new_qr = Regex::new("&v=3").unwrap().replace(&qr, ""); + assert!(new_qr != qr); + qr = new_qr.to_string(); + } tcm.section("Step 2: Bob scans QR-code, sends vg-request"); let bob_chatid = join_securejoin(&bob, &qr).await?; From 80db50129a3bc0494e02de70568d75aa5092565d Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 17:15:28 +0100 Subject: [PATCH 29/36] test: Test that Securejoin without v=3 and without INVITENUMBER also succeeds with 1:1 chats and broadcasts --- src/securejoin/securejoin_tests.rs | 102 ++++++++++++++++++++++++----- 1 file changed, 86 insertions(+), 16 deletions(-) diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index a864b8b384..bad8404508 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -459,22 +459,7 @@ async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { tcm.section("Step 1: Generate QR-code, secure-join implied by chatid"); let mut qr = get_securejoin_qr(&alice, Some(alice_chatid)).await.unwrap(); - if remove_invite { - // Remove the INVITENUBMER. It's not needed in Securejoin v3, - // but still included for backwards compatibility reasons. - // We want to be able to remove it in the future, however. - let new_qr = Regex::new("&i=.*?&").unwrap().replace(&qr, "&"); - assert!(new_qr != qr); - qr = new_qr.to_string(); - } - if !v3 || remove_invite { - // If `!v3`, force legacy securejoin to run by removing the &v=3 parameter. - // If `remove_invite`, we can also remove the v=3 parameter, - // because any QR with AUTH but no INVITE must treated as a v3 QR code. - let new_qr = Regex::new("&v=3").unwrap().replace(&qr, ""); - assert!(new_qr != qr); - qr = new_qr.to_string(); - } + manipulate_qr(v3, remove_invite, &mut qr); tcm.section("Step 2: Bob scans QR-code, sends vg-request"); let bob_chatid = join_securejoin(&bob, &qr).await?; @@ -691,6 +676,91 @@ async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_secure_join_broadcast_legacy() -> Result<()> { + test_secure_join_broadcast(false, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_secure_join_broadcast_v3() -> Result<()> { + test_secure_join_broadcast(true, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_secure_join_broadcast_v3_without_invite() -> Result<()> { + test_secure_join_broadcast(true, true).await +} + +async fn test_secure_join_broadcast(v3: bool, remove_invite: bool) -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + let alice_chat_id = chat::create_broadcast(alice, "Channel".to_string()).await?; + let mut qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?; + manipulate_qr(v3, remove_invite, &mut qr); + let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; + + let sent = alice.send_text(alice_chat_id, "Hi channel").await; + let rcvd = bob.recv_msg(&sent).await; + assert_eq!(rcvd.chat_id, bob_chat_id); + assert_eq!(rcvd.text, "Hi channel"); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_setup_contact_compatibility_legacy() -> Result<()> { + test_setup_contact_compatibility(false, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_setup_contact_compatibility_v3() -> Result<()> { + test_setup_contact_compatibility(true, false).await +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_setup_contact_compatibility_v3_without_invite() -> Result<()> { + test_setup_contact_compatibility(true, true).await +} + +async fn test_setup_contact_compatibility(v3: bool, remove_invite: bool) -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + let mut qr = get_securejoin_qr(alice, None).await?; + manipulate_qr(v3, remove_invite, &mut qr); + let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; + assert_eq!(bob_chat_id, bob.get_chat(alice).await.id); + + let sent = alice + .send_text(alice.get_chat(bob).await.id, "Hi Bob") + .await; + let rcvd = bob.recv_msg(&sent).await; + assert_eq!(rcvd.chat_id, bob_chat_id); + assert_eq!(rcvd.text, "Hi Bob"); + + Ok(()) +} + +fn manipulate_qr(v3: bool, remove_invite: bool, qr: &mut String) { + if remove_invite { + // Remove the INVITENUBMER. It's not needed in Securejoin v3, + // but still included for backwards compatibility reasons. + // We want to be able to remove it in the future, however. + let new_qr = Regex::new("&i=.*?&").unwrap().replace(qr, "&"); + // Broadcast channels use `j` for the INVITENUMBER, so we need to remove it, too: + let new_qr = Regex::new("&j=.*?&").unwrap().replace(&new_qr, "&"); + assert!(new_qr != *qr); + *qr = new_qr.to_string(); + } + if !v3 || remove_invite { + // If `!v3`, force legacy securejoin to run by removing the &v=3 parameter. + // If `remove_invite`, we can also remove the v=3 parameter, + // because any QR with AUTH but no INVITE must treated as a v3 QR code. + let new_qr = Regex::new("&v=3").unwrap().replace(qr, ""); + assert!(new_qr != *qr); + *qr = new_qr.to_string(); + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_adhoc_group_no_qr() -> Result<()> { let alice = TestContext::new_alice().await; From 81d9fa7aa50e125471f5f2c2df6da4674d8a70df Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 17:35:04 +0100 Subject: [PATCH 30/36] fix: For future-compatibility, don't rely on Invite for checking whether we know this QR code --- src/qr.rs | 7 +++---- src/qr/qr_tests.rs | 24 ++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/qr.rs b/src/qr.rs index 906fbbfef9..b5adbc39ec 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -538,8 +538,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { .await .with_context(|| format!("can't check if address {addr:?} is our address"))? { - // TODO check AUTH instead - if token::exists(context, token::Namespace::InviteNumber, &invitenumber).await? { + if token::exists(context, token::Namespace::Auth, &authcode).await? { Ok(Qr::WithdrawVerifyGroup { grpname, grpid, @@ -575,7 +574,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { .await .with_context(|| format!("Can't check if {addr:?} is our address"))? { - if token::exists(context, token::Namespace::InviteNumber, &invitenumber).await? { + if token::exists(context, token::Namespace::Auth, &authcode).await? { Ok(Qr::WithdrawJoinBroadcast { name, grpid, @@ -606,7 +605,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { }) } } else if context.is_self_addr(&addr).await? { - if token::exists(context, token::Namespace::InviteNumber, &invitenumber).await? { + if token::exists(context, token::Namespace::Auth, &authcode).await? { Ok(Qr::WithdrawVerifyContact { contact_id, fingerprint, diff --git a/src/qr/qr_tests.rs b/src/qr/qr_tests.rs index d6dc88f726..f710915a4f 100644 --- a/src/qr/qr_tests.rs +++ b/src/qr/qr_tests.rs @@ -1,3 +1,5 @@ +use regex::Regex; + use super::*; use crate::chat::{Chat, create_broadcast, create_group, get_chat_contacts}; use crate::config::Config; @@ -445,9 +447,27 @@ async fn test_decode_openpgp_without_addr() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_withdraw_verifycontact() -> Result<()> { +async fn test_withdraw_verifycontact_basic() -> Result<()> { + test_withdraw_verifycontact(false).await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_withdraw_verifycontact_without_invite() -> Result<()> { + test_withdraw_verifycontact(true).await +} + +async fn test_withdraw_verifycontact(remove_invite: bool) -> Result<()> { let alice = TestContext::new_alice().await; - let qr = get_securejoin_qr(&alice, None).await?; + let mut qr = get_securejoin_qr(&alice, None).await?; + + if remove_invite { + // Remove the INVITENUBMER. It's not needed in Securejoin v3, + // but still included for backwards compatibility reasons. + // We want to be able to remove it in the future, however. + let new_qr = Regex::new("&i=.*?&").unwrap().replace(&qr, "&"); + assert!(new_qr != *qr); + qr = new_qr.to_string(); + } // scanning own verify-contact code offers withdrawing assert!(matches!( From e773468b2fe82b2c87c76f5b7cf682701108f97e Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 17:42:03 +0100 Subject: [PATCH 31/36] fix: Don't save empty tokens --- src/qr/qr_tests.rs | 6 ++++++ src/token.rs | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/qr/qr_tests.rs b/src/qr/qr_tests.rs index f710915a4f..919e5e6d7b 100644 --- a/src/qr/qr_tests.rs +++ b/src/qr/qr_tests.rs @@ -486,6 +486,12 @@ async fn test_withdraw_verifycontact(remove_invite: bool) -> Result<()> { check_qr(&alice, &qr).await?, Qr::WithdrawVerifyContact { .. } )); + // Even if we removed the INVITE token above, + // there must not be a "" token in the database: + assert_eq!( + token::exists(&alice, token::Namespace::InviteNumber, "").await?, + false + ); // someone else always scans as ask-verify-contact let bob = TestContext::new_bob().await; diff --git a/src/token.rs b/src/token.rs index e8f15b3498..b63d09e2ba 100644 --- a/src/token.rs +++ b/src/token.rs @@ -30,6 +30,10 @@ pub async fn save( token: &str, timestamp: i64, ) -> Result<()> { + if token.is_empty() { + info!(context, "Not saving empty {namespace} token"); + return Ok(()); + } context .sql .execute( From 0e359e99be9cc5a976f4d6f3119b4344232a8463 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 19:20:19 +0100 Subject: [PATCH 32/36] Improve comment --- src/qr/qr_tests.rs | 3 +-- src/securejoin/bob.rs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qr/qr_tests.rs b/src/qr/qr_tests.rs index 919e5e6d7b..2afd66bd80 100644 --- a/src/qr/qr_tests.rs +++ b/src/qr/qr_tests.rs @@ -486,8 +486,7 @@ async fn test_withdraw_verifycontact(remove_invite: bool) -> Result<()> { check_qr(&alice, &qr).await?, Qr::WithdrawVerifyContact { .. } )); - // Even if we removed the INVITE token above, - // there must not be a "" token in the database: + // Test that removing the INVITENUMBER doesn't result in saving empty token: assert_eq!( token::exists(&alice, token::Namespace::InviteNumber, "").await?, false diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 114371c71e..e676b274af 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -302,7 +302,7 @@ pub(crate) async fn send_handshake_message( step: BobHandshakeMsg, ) -> Result<()> { if invite.is_v3() && matches!(step, BobHandshakeMsg::Request) { - // Send a minimal symmetrically-encrypted vc-request message + // Send a minimal symmetrically-encrypted vc-request-pubkey message let rfc724_mid = create_outgoing_rfc724_mid(); let contact = Contact::get_by_id(context, invite.contact_id()).await?; let recipient = contact.get_addr(); From 46c9fc31698e3f578d00f5729a4743a22f25a4de Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 19:52:46 +0100 Subject: [PATCH 33/36] feat: Remove Auto-Submitted: auto-replied header from securejoin messages --- src/mimefactory.rs | 17 ----------------- src/securejoin/securejoin_tests.rs | 19 +++++++++++-------- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index f543a23089..f3fdd7d031 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -869,16 +869,6 @@ impl MimeFactory { "Auto-Submitted", mail_builder::headers::raw::Raw::new("auto-generated".to_string()).into(), )); - } else if let Loaded::Message { msg, .. } = &self.loaded - && msg.param.get_cmd() == SystemMessage::SecurejoinMessage - { - let step = msg.param.get(Param::Arg).unwrap_or_default(); - if step != "vg-request" && step != "vc-request" { - headers.push(( - "Auto-Submitted", - mail_builder::headers::raw::Raw::new("auto-replied".to_string()).into(), - )); - } } if let Loaded::Message { msg, chat } = &self.loaded @@ -2302,7 +2292,6 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( mail_builder::headers::text::Text::new("Secure-Join".to_string()).into(), )); - // TODO not sure if we even need a timestamp let timestamp = create_smeared_timestamp(context); let date = chrono::DateTime::::from_timestamp(timestamp, 0) .unwrap() @@ -2320,12 +2309,6 @@ pub(crate) async fn render_symm_encrypted_securejoin_message( "Auto-Submitted", mail_builder::headers::raw::Raw::new("auto-generated".to_string()).into(), )); - // TODO it's not nice that we're comparing strings here - } else if step != "vc-request-pubkey" { - headers.push(( - "Auto-Submitted", - mail_builder::headers::raw::Raw::new("auto-replied".to_string()).into(), - )); } let encrypt_helper = EncryptHelper::new(context).await?; diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index bad8404508..8db9eefddc 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -63,13 +63,13 @@ async fn test_setup_contact_ex(case: SetupContactCase) { bob.set_config(Config::Displayname, Some("Bob Examplenet")) .await .unwrap(); - let alice_auto_submitted_hdr; + let alice_auto_submitted_hdr: bool; match case { SetupContactCase::AliceIsBot => { alice.set_config_bool(Config::Bot, true).await.unwrap(); - alice_auto_submitted_hdr = "Auto-Submitted: auto-generated"; + alice_auto_submitted_hdr = true; } - _ => alice_auto_submitted_hdr = "Auto-Submitted: auto-replied", + _ => alice_auto_submitted_hdr = false, }; assert_eq!( @@ -138,7 +138,10 @@ async fn test_setup_contact_ex(case: SetupContactCase) { ); let sent = alice.pop_sent_msg().await; - assert!(sent.payload.contains(alice_auto_submitted_hdr)); + assert_eq!( + sent.payload.contains("Auto-Submitted: auto-generated"), + alice_auto_submitted_hdr + ); assert!(!sent.payload.contains("Alice Exampleorg")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); @@ -171,7 +174,6 @@ async fn test_setup_contact_ex(case: SetupContactCase) { // Check Bob sent the right message. let sent = bob.pop_sent_msg().await; - assert!(sent.payload.contains("Auto-Submitted: auto-replied")); assert!(!sent.payload.contains("Bob Examplenet")); let mut msg = alice.parse_msg(&sent).await; assert!(msg.was_encrypted()); @@ -262,7 +264,10 @@ async fn test_setup_contact_ex(case: SetupContactCase) { // Check Alice sent the right message to Bob. let sent = alice.pop_sent_msg().await; - assert!(sent.payload.contains(alice_auto_submitted_hdr)); + assert_eq!( + sent.payload.contains("Auto-Submitted: auto-generated"), + alice_auto_submitted_hdr + ); assert!(!sent.payload.contains("Alice Exampleorg")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); @@ -499,7 +504,6 @@ async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { alice.recv_msg_trash(&sent).await; let sent = alice.pop_sent_msg().await; - assert!(sent.payload.contains("Auto-Submitted: auto-replied")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); assert_eq!( @@ -537,7 +541,6 @@ async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { } // Check Bob sent the right handshake message. - assert!(sent.payload.contains("Auto-Submitted: auto-replied")); let msg = alice.parse_msg(&sent).await; assert!(msg.was_encrypted()); assert_eq!( From 10933cc1a07803320a9c0ff750a5956df9749148 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 20:13:59 +0100 Subject: [PATCH 34/36] test: Improve test --- src/securejoin/securejoin_tests.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 8db9eefddc..ad0b0d28bc 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -703,6 +703,7 @@ async fn test_secure_join_broadcast(v3: bool, remove_invite: bool) -> Result<()> let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; let sent = alice.send_text(alice_chat_id, "Hi channel").await; + assert!(sent.recipients.contains("bob@example.net")); let rcvd = bob.recv_msg(&sent).await; assert_eq!(rcvd.chat_id, bob_chat_id); assert_eq!(rcvd.text, "Hi channel"); @@ -727,18 +728,22 @@ async fn test_setup_contact_compatibility(v3: bool, remove_invite: bool) -> Resu let mut tcm = TestContextManager::new(); let alice = &tcm.alice().await; let bob = &tcm.bob().await; + alice.set_config(Config::Displayname, Some("Alice")).await?; let mut qr = get_securejoin_qr(alice, None).await?; manipulate_qr(v3, remove_invite, &mut qr); let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; - assert_eq!(bob_chat_id, bob.get_chat(alice).await.id); - let sent = alice - .send_text(alice.get_chat(bob).await.id, "Hi Bob") - .await; - let rcvd = bob.recv_msg(&sent).await; - assert_eq!(rcvd.chat_id, bob_chat_id); - assert_eq!(rcvd.text, "Hi Bob"); + let bob_chat = Chat::load_from_db(bob, bob_chat_id).await?; + assert_eq!(bob_chat.name, "Alice"); + assert!(bob_chat.can_send(bob).await?); + assert_eq!(bob_chat.typ, Chattype::Single); + assert_eq!(bob_chat.id, bob.get_chat(alice).await.id); + + let alice_chat = alice.get_chat(bob).await; + assert_eq!(alice_chat.name, "bob@example.net"); + assert!(alice_chat.can_send(alice).await?); + assert_eq!(alice_chat.typ, Chattype::Single); Ok(()) } From fd61423fb30b4012314456e368e37ea8fe576e52 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 20:21:00 +0100 Subject: [PATCH 35/36] Some more comments improvements --- src/qr/qr_tests.rs | 3 ++- src/securejoin/securejoin_tests.rs | 19 ++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/qr/qr_tests.rs b/src/qr/qr_tests.rs index 2afd66bd80..43516c419c 100644 --- a/src/qr/qr_tests.rs +++ b/src/qr/qr_tests.rs @@ -463,7 +463,8 @@ async fn test_withdraw_verifycontact(remove_invite: bool) -> Result<()> { if remove_invite { // Remove the INVITENUBMER. It's not needed in Securejoin v3, // but still included for backwards compatibility reasons. - // We want to be able to remove it in the future, however. + // We want to be able to remove it in the future, + // therefore we test that things work without it. let new_qr = Regex::new("&i=.*?&").unwrap().replace(&qr, "&"); assert!(new_qr != *qr); qr = new_qr.to_string(); diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index ad0b0d28bc..165903f526 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -500,7 +500,7 @@ async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { // is only sent in `vg-request-with-auth` for compatibility. assert!(!msg.header_exists(HeaderDef::SecureJoinGroup)); - tcm.section("Step 3: Alice receives vc-request-pubkey and sends vc-pubkey, or vg-request and sends vg-auth-required"); + tcm.section("Step 3: Alice receives vc-request-pubkey and sends vc-pubkey, or receives vg-request and sends vg-auth-required"); alice.recv_msg_trash(&sent).await; let sent = alice.pop_sent_msg().await; @@ -752,17 +752,18 @@ fn manipulate_qr(v3: bool, remove_invite: bool, qr: &mut String) { if remove_invite { // Remove the INVITENUBMER. It's not needed in Securejoin v3, // but still included for backwards compatibility reasons. - // We want to be able to remove it in the future, however. + // We want to be able to remove it in the future, + // therefore we test that things work without it. let new_qr = Regex::new("&i=.*?&").unwrap().replace(qr, "&"); - // Broadcast channels use `j` for the INVITENUMBER, so we need to remove it, too: + // Broadcast channels use `j` for the INVITENUMBER let new_qr = Regex::new("&j=.*?&").unwrap().replace(&new_qr, "&"); assert!(new_qr != *qr); *qr = new_qr.to_string(); } + // If `!v3`, force legacy securejoin to run by removing the &v=3 parameter. + // If `remove_invite`, we can also remove the v=3 parameter, + // because a QR with AUTH but no INVITE is obviously v3 QR code. if !v3 || remove_invite { - // If `!v3`, force legacy securejoin to run by removing the &v=3 parameter. - // If `remove_invite`, we can also remove the v=3 parameter, - // because any QR with AUTH but no INVITE must treated as a v3 QR code. let new_qr = Regex::new("&v=3").unwrap().replace(qr, ""); assert!(new_qr != *qr); *qr = new_qr.to_string(); @@ -1519,7 +1520,7 @@ async fn test_auth_token_is_synchronized() -> Result<()> { sync(alice1, alice2).await; // Note that Bob will throw away the AUTH token after sending `vc-request-with-auth`. - // Therefore, he will fail to decrypt the answer from alice, + // Therefore, he will fail to decrypt the answer from Alice's second device, // which leads to a "decryption failed: missing key" message in the logs. // This is fine. tcm.exec_securejoin_qr_multi_device(bob, &[alice1, alice2], &qr2) @@ -1535,12 +1536,12 @@ async fn test_auth_token_is_synchronized() -> Result<()> { for qr in [qr1, qr2] { let qr = check_qr(bob, &qr).await?; - let qr = QrInvite::try_from(dbg!(qr))?; + let qr = QrInvite::try_from(qr)?; assert!(token::exists(alice2, Namespace::InviteNumber, qr.invitenumber()).await?); assert!(token::exists(alice2, Namespace::Auth, qr.authcode()).await?); } - // Check that alice2 only saves the invite number twice: + // Check that alice2 only saves the invite number once: let invite_count: u32 = alice2 .sql .query_get_value( From 0d47a520da49da139207d5236cd71be0de0a3b53 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 10 Feb 2026 21:52:02 +0100 Subject: [PATCH 36/36] More comments and renamings --- src/aheader.rs | 8 ++++---- src/mimeparser.rs | 2 +- src/securejoin/securejoin_tests.rs | 32 ++++++++++++++---------------- src/token.rs | 3 +-- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/aheader.rs b/src/aheader.rs index 1cfca0fd0e..cd2d494df8 100644 --- a/src/aheader.rs +++ b/src/aheader.rs @@ -48,10 +48,10 @@ pub struct Aheader { pub prefer_encrypt: EncryptPreference, /// Whether `_verified` attribute is present. - // - // `_verified` attribute is an extension to `Autocrypt-Gossip` - // header that is used to tell that the sender - // marked this key as verified. + /// + /// `_verified` attribute is an extension to `Autocrypt-Gossip` + /// header that is used to tell that the sender + /// marked this key as verified. pub verified: bool, } diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 9b05071196..d391e6e058 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -2095,7 +2095,7 @@ impl MimeMessage { } /// Loads all the shared secrets -/// that can be used to decrypt a symmetrically-encrypted message +/// that will be tried to decrypt a symmetrically-encrypted message async fn load_shared_secrets(context: &Context) -> Result> { // First, try decrypting using the bobstate, // because usually there will only be 1 or 2 of it, diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 165903f526..a429bff4cb 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -439,19 +439,19 @@ async fn test_setup_contact_concurrent_calls() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_secure_join_legacy() -> Result<()> { - test_secure_join(false, false).await +async fn test_secure_join_group_legacy() -> Result<()> { + test_secure_join_group_ex(false, false).await } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_secure_join_v3() -> Result<()> { - test_secure_join(true, false).await +async fn test_secure_join_group_v3() -> Result<()> { + test_secure_join_group_ex(true, false).await } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_secure_join_v3_without_invite() -> Result<()> { - test_secure_join(true, true).await +async fn test_secure_join_group_v3_without_invite() -> Result<()> { + test_secure_join_group_ex(true, true).await } -async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { +async fn test_secure_join_group_ex(v3: bool, remove_invite: bool) -> Result<()> { let mut tcm = TestContextManager::new(); let alice = tcm.alice().await; let bob = tcm.bob().await; @@ -605,8 +605,6 @@ async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { // Now Alice's chat with Bob should still be hidden, the verified message should // appear in the group chat. if v3 { - // In version 3 of the Securejoin protocol, - // the chat is not even created assert!( ChatIdBlocked::lookup_by_contact(&alice, contact_bob.id) .await? @@ -681,18 +679,18 @@ async fn test_secure_join(v3: bool, remove_invite: bool) -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_secure_join_broadcast_legacy() -> Result<()> { - test_secure_join_broadcast(false, false).await + test_secure_join_broadcast_ex(false, false).await } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_secure_join_broadcast_v3() -> Result<()> { - test_secure_join_broadcast(true, false).await + test_secure_join_broadcast_ex(true, false).await } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_secure_join_broadcast_v3_without_invite() -> Result<()> { - test_secure_join_broadcast(true, true).await + test_secure_join_broadcast_ex(true, true).await } -async fn test_secure_join_broadcast(v3: bool, remove_invite: bool) -> Result<()> { +async fn test_secure_join_broadcast_ex(v3: bool, remove_invite: bool) -> Result<()> { let mut tcm = TestContextManager::new(); let alice = &tcm.alice().await; let bob = &tcm.bob().await; @@ -713,18 +711,18 @@ async fn test_secure_join_broadcast(v3: bool, remove_invite: bool) -> Result<()> #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_setup_contact_compatibility_legacy() -> Result<()> { - test_setup_contact_compatibility(false, false).await + test_setup_contact_compatibility_ex(false, false).await } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_setup_contact_compatibility_v3() -> Result<()> { - test_setup_contact_compatibility(true, false).await + test_setup_contact_compatibility_ex(true, false).await } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_setup_contact_compatibility_v3_without_invite() -> Result<()> { - test_setup_contact_compatibility(true, true).await + test_setup_contact_compatibility_ex(true, true).await } -async fn test_setup_contact_compatibility(v3: bool, remove_invite: bool) -> Result<()> { +async fn test_setup_contact_compatibility_ex(v3: bool, remove_invite: bool) -> Result<()> { let mut tcm = TestContextManager::new(); let alice = &tcm.alice().await; let bob = &tcm.bob().await; diff --git a/src/token.rs b/src/token.rs index b63d09e2ba..2569e7a0ac 100644 --- a/src/token.rs +++ b/src/token.rs @@ -71,8 +71,7 @@ pub async fn lookup_all(context: &Context, namespace: Namespace) -> Result