Skip to content

fix: Rerun the full securejoin protocol if the address was outdated #8358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Hocuri wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix: Rerun the full securejoin protocol if the address was outdated #8358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a9d19bc
fix: use new QR-contained addresses when alice and bob have dropped t…
hpk42 Jun 20, 2026
fdcfb20
test: In test_qr_scan_updates_new_relay_address(), test that sending …
Hocuri Jun 22, 2026
3f2bb38
Revert "fix: use new QR-contained addresses when alice and bob have d…
Hocuri Jun 22, 2026
c9974d0
fix: Rerun the full securejoin protocol if the address was outdated
Hocuri Jun 22, 2026
d8c70da
clippy
Hocuri Jun 22, 2026
8fd602b
Make sure that we can have multiple addresses in the future
Hocuri Jun 23, 2026
f4103c6
Small simplification
Hocuri Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion deltachat-contact-tools/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions deltachat-jsonrpc/src/api/types/qr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ impl From<Qr> for QrObject {
invitenumber,
authcode,
is_v3,
..
} => {
let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.human_readable();
Expand All @@ -255,6 +256,7 @@ impl From<Qr> for QrObject {
invitenumber,
authcode,
is_v3,
..
} => {
let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.human_readable();
Expand All @@ -276,6 +278,7 @@ impl From<Qr> for QrObject {
authcode,
invitenumber,
is_v3,
..
} => {
let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.human_readable();
Expand Down
22 changes: 22 additions & 0 deletions deltachat-rpc-client/tests/test_securejoin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
12 changes: 12 additions & 0 deletions src/qr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,

/// Invite number.
invitenumber: String,

Expand All @@ -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<String>,

/// Invite number.
invitenumber: String,

Expand Down Expand Up @@ -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<String>,

/// Invite number.
invitenumber: String,
/// Authentication code.
Expand Down Expand Up @@ -563,6 +572,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result<Qr> {
grpid,
contact_id,
fingerprint,
addrs: vec![addr.to_string()],
invitenumber,
authcode,
is_v3,
Expand Down Expand Up @@ -599,6 +609,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result<Qr> {
grpid,
contact_id,
fingerprint,
addrs: vec![addr.to_string()],
invitenumber,
authcode,
is_v3,
Expand All @@ -624,6 +635,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result<Qr> {
Ok(Qr::AskVerifyContact {
contact_id,
fingerprint,
addrs: vec![addr.to_string()],
invitenumber,
authcode,
is_v3,
Expand Down
4 changes: 2 additions & 2 deletions src/securejoin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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(())
Expand Down
30 changes: 20 additions & 10 deletions src/securejoin/bob.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -58,14 +60,23 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul
QrInvite::Broadcast { .. } => {}
}

let has_key = context
let public_key_bytes: Option<Vec<_>> = 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?;

// The key is up to date iff it contains all the addresses from the QR code:
let has_up_to_date_key = if let Some(public_key_bytes) = public_key_bytes {
let public_key = SignedPublicKey::from_slice(&public_key_bytes)?;
let addrs_in_key = addresses_from_public_key(&public_key).unwrap_or_default();

@link2xt link2xt Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unwrap_or_default() hides a corner case when we have a key for a key-contact, but the key has no addresses (because it does not have the notation subpacket). In this case if the address of the key-contact (the one in the contacts table which is usually the last seen From address and the address we are going to send messages to) is the same the address in the invite, there is no need to request a new key.

Can be fixed like this:

let inviter_addrs = if let Some(addrs_in_key) = addresses_from_public_key(&public_key) {
  addrs_in_key
} else if  let Some(contact_addr) = context.query_get_value("SELECT addr FROM contacts WHERE fingerprint=?", (invite.fingerprint().hex(),)).await? {
  vec![contact_addr]
} else {
  Vec::new()
}

invite.addrs().iter().all(|a| addrs_in_key.contains(a))
} else {
false
};
Comment on lines +72 to +78

@Hocuri Hocuri Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit unsure whether I prefer this PR here or #8355. For example, this logic here is somewhat hard to read. OTOH, the fix here is more "self-contained" in the securejoin logic, rather than needing logic changes elsewhere.

@hpk42 hpk42 Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it's readable enough.
maybe rename the var to key_contains_all_invite_addrs and remove the comment? When i read "has_up_to_date_key" in other places i immediately wonder what that means concretely. It might also be that someone is scanning an old invite link, and the relay addresses in the Alice key that Bob has are actually the correct ones. #8355 would use both the key and the invite addresses but i think it's fine to only use invite addresses. This handshake would fail anyway if Alice has meanwhile changed all her relays.


// Now start the protocol and initialise the state.
{
// `joining_chat_id` is `Some` if group chat
Expand Down Expand Up @@ -97,7 +108,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 has_up_to_date_key
&& verify_sender_by_fingerprint(context, invite.fingerprint(), invite.contact_id())
.await?
{
Expand Down Expand Up @@ -154,7 +165,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 !has_up_to_date_key {
chat::add_info_msg_with_cmd(
context,
private_chat_id,
Expand Down Expand Up @@ -310,8 +321,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}");
Expand All @@ -327,7 +337,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 {
Expand Down
20 changes: 20 additions & 0 deletions src/securejoin/qrinvite.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ pub enum QrInvite {
Contact {
contact_id: ContactId,
fingerprint: Fingerprint,
#[serde(default)]
addrs: Vec<String>,
invitenumber: String,
authcode: String,
#[serde(default)]
Expand All @@ -26,6 +28,8 @@ pub enum QrInvite {
Group {
contact_id: ContactId,
fingerprint: Fingerprint,
#[serde(default)]
addrs: Vec<String>,
name: String,
grpid: String,
invitenumber: String,
Expand All @@ -36,6 +40,8 @@ pub enum QrInvite {
Broadcast {
contact_id: ContactId,
fingerprint: Fingerprint,
#[serde(default)]
addrs: Vec<String>,
name: String,
grpid: String,
invitenumber: String,
Expand Down Expand Up @@ -92,6 +98,14 @@ impl QrInvite {
QrInvite::Broadcast { is_v3, .. } => is_v3,
}
}

pub(crate) fn addrs(&self) -> &Vec<String> {
match self {
QrInvite::Contact { addrs, .. } => addrs,
QrInvite::Group { addrs, .. } => addrs,
QrInvite::Broadcast { addrs, .. } => addrs,
}
}
}

impl TryFrom<Qr> for QrInvite {
Expand All @@ -102,12 +116,14 @@ impl TryFrom<Qr> for QrInvite {
Qr::AskVerifyContact {
contact_id,
fingerprint,
addrs,
invitenumber,
authcode,
is_v3,
} => Ok(QrInvite::Contact {
contact_id,
fingerprint,
addrs,
invitenumber,
authcode,
is_v3,
Expand All @@ -117,12 +133,14 @@ impl TryFrom<Qr> for QrInvite {
grpid,
contact_id,
fingerprint,
addrs,
invitenumber,
authcode,
is_v3,
} => Ok(QrInvite::Group {
contact_id,
fingerprint,
addrs,
name: grpname,
grpid,
invitenumber,
Expand All @@ -134,6 +152,7 @@ impl TryFrom<Qr> for QrInvite {
grpid,
contact_id,
fingerprint,
addrs,
authcode,
invitenumber,
is_v3,
Expand All @@ -142,6 +161,7 @@ impl TryFrom<Qr> for QrInvite {
grpid,
contact_id,
fingerprint,
addrs,
authcode,
invitenumber,
is_v3,
Expand Down
Loading