From abb84efc3750592d24e1743771b69484c79dc5a9 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 25 Jun 2026 18:59:08 +0200 Subject: [PATCH] fix: Rerun the full securejoin protocol if the address was outdated (#8358) Alternative to https://github.com/chatmail/core/pull/8355/, fixes https://github.com/chatmail/core/issues/8329 This fixes the following bug: Scanning QR for existing contact doesn't update relay list Two chat partners suddenly lost connectivity to all of their relays. They added another relay, which is accessible, and set it as main. Of course, they still couldn't communicate, because there was no way to announce a new relay list to each other (clients know only about old relays so they try to send updates there). That's why the friends used a different channel (e.g. met IRL) to send their QR codes once again. Expected behavior: They would expect that clients update their relay lists according to the contact info inside the QR codes, and they can continue talking in Delta. Actual behavior: But Delta Chat client doesn't update a list of announced relays on scanning a QR in case a contact already exists. It simply opens the existing chat without altering any contact info. --------- Co-authored-by: holger krekel --- deltachat-contact-tools/src/lib.rs | 2 +- deltachat-jsonrpc/src/api/types/qr.rs | 3 ++ deltachat-rpc-client/tests/test_securejoin.py | 22 ++++++++++++ src/qr.rs | 12 +++++++ src/securejoin.rs | 4 +-- src/securejoin/bob.rs | 35 +++++++++++++------ src/securejoin/qrinvite.rs | 20 +++++++++++ 7 files changed, 85 insertions(+), 13 deletions(-) diff --git a/deltachat-contact-tools/src/lib.rs b/deltachat-contact-tools/src/lib.rs index 8ea7c0b6d..1a958d51f 100644 --- a/deltachat-contact-tools/src/lib.rs +++ b/deltachat-contact-tools/src/lib.rs @@ -39,7 +39,7 @@ mod vcard; pub use vcard::{make_vcard, parse_vcard, VcardContact}; /// Valid contact address. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct ContactAddress(String); impl Deref for ContactAddress { diff --git a/deltachat-jsonrpc/src/api/types/qr.rs b/deltachat-jsonrpc/src/api/types/qr.rs index 6a4f4cd7b..4bf55f398 100644 --- a/deltachat-jsonrpc/src/api/types/qr.rs +++ b/deltachat-jsonrpc/src/api/types/qr.rs @@ -236,6 +236,7 @@ impl From for QrObject { invitenumber, authcode, is_v3, + .. } => { let contact_id = contact_id.to_u32(); let fingerprint = fingerprint.human_readable(); @@ -255,6 +256,7 @@ impl From for QrObject { invitenumber, authcode, is_v3, + .. } => { let contact_id = contact_id.to_u32(); let fingerprint = fingerprint.human_readable(); @@ -276,6 +278,7 @@ impl From for QrObject { authcode, invitenumber, is_v3, + .. } => { let contact_id = contact_id.to_u32(); let fingerprint = fingerprint.human_readable(); diff --git a/deltachat-rpc-client/tests/test_securejoin.py b/deltachat-rpc-client/tests/test_securejoin.py index 54799adf1..d9da04de9 100644 --- a/deltachat-rpc-client/tests/test_securejoin.py +++ b/deltachat-rpc-client/tests/test_securejoin.py @@ -704,3 +704,25 @@ def test_withdraw_securejoin_qr(acfactory): and "Ignoring RequestWithAuth message because of invalid auth code." in event.msg ): break + + +def test_qr_scan_updates_new_relay_address(acfactory): + alice, bob = acfactory.get_online_accounts(2) + + bob_alice_chat = bob.secure_join(alice.get_qr_code()) + alice.wait_for_securejoin_inviter_success() + bob.wait_for_securejoin_joiner_success() + + for ac in [alice, bob]: + old_addr = ac.get_config("configured_addr") + ac.add_transport_from_qr(acfactory.get_account_qr()) + ac.set_config("configured_addr", ac.list_transports()[1]["addr"]) + ac.delete_transport(old_addr) + + bob.secure_join(alice.get_qr_code()) + alice.wait_for_securejoin_inviter_success() + bob.wait_for_securejoin_joiner_success() + + bob_alice_chat.send_text("hi") + snapshot = alice.wait_for_incoming_msg().get_snapshot() + assert snapshot.text == "hi" diff --git a/src/qr.rs b/src/qr.rs index d4531442c..9a0724955 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -56,6 +56,9 @@ pub enum Qr { /// Fingerprint of the contact key as scanned from the QR code. fingerprint: Fingerprint, + /// The inviter's addresses. + addrs: Vec, + /// Invite number. invitenumber: String, @@ -80,6 +83,9 @@ pub enum Qr { /// Fingerprint of the contact key as scanned from the QR code. fingerprint: Fingerprint, + /// The inviter's addresses. + addrs: Vec, + /// Invite number. invitenumber: String, @@ -108,6 +114,9 @@ pub enum Qr { /// Fingerprint of the contact's key as scanned from the QR code. fingerprint: Fingerprint, + /// The inviter's addresses. + addrs: Vec, + /// Invite number. invitenumber: String, /// Authentication code. @@ -563,6 +572,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { grpid, contact_id, fingerprint, + addrs: vec![addr.to_string()], invitenumber, authcode, is_v3, @@ -599,6 +609,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { grpid, contact_id, fingerprint, + addrs: vec![addr.to_string()], invitenumber, authcode, is_v3, @@ -624,6 +635,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { Ok(Qr::AskVerifyContact { contact_id, fingerprint, + addrs: vec![addr.to_string()], invitenumber, authcode, is_v3, diff --git a/src/securejoin.rs b/src/securejoin.rs index e6b9182db..3ab31805d 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -741,7 +741,7 @@ pub(crate) async fn handle_securejoin_handshake( async fn insert_into_smtp( context: &Context, rfc724_mid: &str, - recipient: &str, + recipients: &str, rendered_message: String, msg_id: MsgId, ) -> Result<(), Error> { @@ -750,7 +750,7 @@ async fn insert_into_smtp( .execute( "INSERT INTO smtp (rfc724_mid, recipients, mime, msg_id) VALUES (?1, ?2, ?3, ?4)", - (&rfc724_mid, &recipient, &rendered_message, msg_id), + (&rfc724_mid, &recipients, &rendered_message, msg_id), ) .await?; Ok(()) diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index f6bd9e678..e4d64556f 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -1,19 +1,21 @@ //! Bob's side of SecureJoin handling, the joiner-side. use anyhow::{Context as _, Result}; +use pgp::composed::SignedPublicKey; use super::HandshakeMessage; use super::qrinvite::QrInvite; use crate::chat::{self, ChatId, is_contact_in_chat}; use crate::constants::{Blocked, Chattype}; -use crate::contact::{Contact, Origin}; +use crate::contact::Origin; use crate::context::Context; use crate::events::EventType; -use crate::key::self_fingerprint; +use crate::key::{DcKey as _, self_fingerprint}; use crate::log::LogExt; use crate::message::{self, Message, MsgId, Viewtype}; use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::param::{Param, Params}; +use crate::pgp::addresses_from_public_key; use crate::securejoin::{ ContactId, encrypted_and_signed, insert_into_smtp, verify_sender_by_fingerprint, }; @@ -58,14 +60,28 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul QrInvite::Broadcast { .. } => {} } - let has_key = context + let public_key_bytes: Option> = context .sql - .exists( - "SELECT COUNT(*) FROM public_keys WHERE fingerprint=?", + .query_get_value( + "SELECT public_key FROM public_keys WHERE fingerprint=?", (invite.fingerprint().hex(),), ) .await?; + let key_contains_all_invite_addrs = if let Some(public_key_bytes) = public_key_bytes { + let public_key = SignedPublicKey::from_slice(&public_key_bytes)?; + if let Some(addrs_in_key) = addresses_from_public_key(&public_key) { + invite.addrs().iter().all(|a| addrs_in_key.contains(a)) + } else { + // This can happen if the inviter is using an old version of Delta Chat + // that doesn't put the relay list into the key. + // In this case, we never take the securejoin protocol shortcut, which is fine. + false + } + } else { + false + }; + // Now start the protocol and initialise the state. { // `joining_chat_id` is `Some` if group chat @@ -97,7 +113,7 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul progress: JoinerProgress::Succeeded.into_u16(), }); return Ok(joining_chat_id); - } else if has_key + } else if key_contains_all_invite_addrs && verify_sender_by_fingerprint(context, invite.fingerprint(), invite.contact_id()) .await? { @@ -154,7 +170,7 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul QrInvite::Contact { .. } => { // For setup-contact the BobState already ensured the 1:1 chat exists because it is // used to send the handshake messages. - if !has_key { + if !key_contains_all_invite_addrs { chat::add_info_msg_with_cmd( context, private_chat_id, @@ -310,8 +326,7 @@ pub(crate) async fn send_handshake_message( if invite.is_v3() && matches!(step, BobHandshakeMsg::Request) { // 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(); + let recipients = invite.addrs().join(" "); let alice_fp = invite.fingerprint().hex(); let auth = invite.authcode(); let shared_secret = format!("securejoin/{alice_fp}/{auth}"); @@ -327,7 +342,7 @@ pub(crate) async fn send_handshake_message( .await?; let msg_id = message::insert_tombstone(context, &rfc724_mid).await?; - insert_into_smtp(context, &rfc724_mid, recipient, rendered_message, msg_id).await?; + insert_into_smtp(context, &rfc724_mid, &recipients, rendered_message, msg_id).await?; context.scheduler.interrupt_smtp().await; } else { let mut msg = Message { diff --git a/src/securejoin/qrinvite.rs b/src/securejoin/qrinvite.rs index 8c224b029..a912a4d38 100644 --- a/src/securejoin/qrinvite.rs +++ b/src/securejoin/qrinvite.rs @@ -18,6 +18,8 @@ pub enum QrInvite { Contact { contact_id: ContactId, fingerprint: Fingerprint, + #[serde(default)] + addrs: Vec, invitenumber: String, authcode: String, #[serde(default)] @@ -26,6 +28,8 @@ pub enum QrInvite { Group { contact_id: ContactId, fingerprint: Fingerprint, + #[serde(default)] + addrs: Vec, name: String, grpid: String, invitenumber: String, @@ -36,6 +40,8 @@ pub enum QrInvite { Broadcast { contact_id: ContactId, fingerprint: Fingerprint, + #[serde(default)] + addrs: Vec, name: String, grpid: String, invitenumber: String, @@ -92,6 +98,14 @@ impl QrInvite { QrInvite::Broadcast { is_v3, .. } => is_v3, } } + + pub(crate) fn addrs(&self) -> &Vec { + match self { + QrInvite::Contact { addrs, .. } => addrs, + QrInvite::Group { addrs, .. } => addrs, + QrInvite::Broadcast { addrs, .. } => addrs, + } + } } impl TryFrom for QrInvite { @@ -102,12 +116,14 @@ impl TryFrom for QrInvite { Qr::AskVerifyContact { contact_id, fingerprint, + addrs, invitenumber, authcode, is_v3, } => Ok(QrInvite::Contact { contact_id, fingerprint, + addrs, invitenumber, authcode, is_v3, @@ -117,12 +133,14 @@ impl TryFrom for QrInvite { grpid, contact_id, fingerprint, + addrs, invitenumber, authcode, is_v3, } => Ok(QrInvite::Group { contact_id, fingerprint, + addrs, name: grpname, grpid, invitenumber, @@ -134,6 +152,7 @@ impl TryFrom for QrInvite { grpid, contact_id, fingerprint, + addrs, authcode, invitenumber, is_v3, @@ -142,6 +161,7 @@ impl TryFrom for QrInvite { grpid, contact_id, fingerprint, + addrs, authcode, invitenumber, is_v3,