From a6888c451506a9a2bbb734adf0e2b9466cf5799f Mon Sep 17 00:00:00 2001 From: link2xt Date: Wed, 21 Oct 2026 14:54:26 +0000 Subject: [PATCH 1/6] docs: document arguments to secret_key_to_public_key() It is not obvious that relay list is comma-separated. --- src/key.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/key.rs b/src/key.rs index 0c7d23b506..4d5e8c5f3a 100644 --- a/src/key.rs +++ b/src/key.rs @@ -124,6 +124,9 @@ pub trait DcKey: Serialize + Deserializable + Clone { } /// Converts secret key to public key. +/// +/// `addr` is the address used in the User ID which is generated for compatibility. +/// `relay_addrs` is the comma-separated list of relay addresses. pub(crate) fn secret_key_to_public_key( mut signed_secret_key: SignedSecretKey, timestamp: u32, From eb3272d3d9e8a7edd925996571bebd37f99c7128 Mon Sep 17 00:00:00 2001 From: link2xt Date: Wed, 21 Oct 2026 14:54:26 +0000 Subject: [PATCH 2/6] refactor: pass current timestamp as an argument to signature_subpackets() pk_encrypt() already uses current timestamp to select the key for encryption, no need to use a potentially different timestamp to sign the message. --- src/pgp.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/pgp.rs b/src/pgp.rs index 3bc4317e5b..907bd35a03 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -162,12 +162,13 @@ pub enum SeipdVersion { /// encrypted to `public_keys_for_encryption`. #[expect(clippy::arithmetic_side_effects)] fn signature_subpackets( + now: pgp::types::Timestamp, private_key_for_signing: &SignedSecretKey, public_keys_for_encryption: &[SignedPublicKey], ) -> Result { let mut hashed = Vec::with_capacity(1 + public_keys_for_encryption.len() + 1); hashed.push(Subpacket::critical(SubpacketData::SignatureCreationTime( - pgp::types::Timestamp::now(), + now, ))?); for key in public_keys_for_encryption { let data = SubpacketData::IntendedRecipientFingerprint(key.fingerprint()); @@ -219,7 +220,8 @@ pub fn pk_encrypt( } if let Some(secret_key) = private_key_for_signing { - let subpkts = signature_subpackets(secret_key, &public_keys_for_encryption)?; + let subpkts = + signature_subpackets(now, secret_key, &public_keys_for_encryption)?; let hash_algorithm = secret_key.hash_alg(); msg.sign_with_subpackets( &**secret_key, @@ -247,7 +249,8 @@ pub fn pk_encrypt( } if let Some(secret_key) = private_key_for_signing { - let subpkts = signature_subpackets(secret_key, &public_keys_for_encryption)?; + let subpkts = + signature_subpackets(now, secret_key, &public_keys_for_encryption)?; let hash_algorithm = secret_key.hash_alg(); msg.sign_with_subpackets( &**secret_key, From b55831d76210a154cc55ea85e47f1ac888f18c49 Mon Sep 17 00:00:00 2001 From: link2xt Date: Fri, 23 Oct 2026 09:33:03 +0000 Subject: [PATCH 3/6] refactor: factor out Contact.relay_addresses() --- src/contact.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/contact.rs b/src/contact.rs index fb6716baa5..37860a40ad 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -1472,9 +1472,7 @@ WHERE addr=? ); } - if let Some(public_key) = contact.public_key(context).await? - && let Some(relay_addrs) = addresses_from_public_key(&public_key) - { + if let Some(relay_addrs) = contact.relay_addresses(context).await? { ret += "\n\nRelays:"; for relay in &relay_addrs { ret += "\n"; @@ -1604,6 +1602,17 @@ WHERE addr=? } } + /// Returns a list of relay addresses for the contact if it is distributed in the public key. + pub(crate) async fn relay_addresses(&self, context: &Context) -> Result>> { + if let Some(public_key) = self.public_key(context).await? + && let Some(relay_addrs) = addresses_from_public_key(&public_key) + { + Ok(Some(relay_addrs)) + } else { + Ok(None) + } + } + /// Get name authorized by the contact. pub fn get_authname(&self) -> &str { &self.authname From 2d080ea9261fb64bb935917307d583c899379b70 Mon Sep 17 00:00:00 2001 From: link2xt Date: Wed, 21 Oct 2026 14:54:26 +0000 Subject: [PATCH 4/6] test: add test_merge_openpgp_certificates_tiebreak() This tests that the order of arguments to merge_openpgp_certificates() is important if signature timestamp is the same. --- src/pgp/pgp_tests.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/pgp/pgp_tests.rs b/src/pgp/pgp_tests.rs index 65d85ab22f..05f6bd1458 100644 --- a/src/pgp/pgp_tests.rs +++ b/src/pgp/pgp_tests.rs @@ -2,6 +2,7 @@ use std::sync::LazyLock; use tokio::sync::OnceCell; use super::*; +use crate::key::secret_key_to_public_key; use crate::transport::add_pseudo_transport; use crate::{ config::Config, @@ -388,6 +389,31 @@ fn test_merge_openpgp_certificates() { assert!(merge_openpgp_certificates(bob, alice).is_err()); } +/// Tests that Direct Key Signature from the new certificate is preferred +/// when the timestamp is the same. +#[test] +fn test_merge_openpgp_certificates_tiebreak() -> Result<()> { + let secret_key = alice_keypair(); + let timestamp = pgp::types::Timestamp::now(); + + let addr = "alice@example.net"; + let relays1 = "alice@example.net,alice@example.org"; + let relays2 = "alice@example.net,alice@example.com"; + + let public_key1 = + secret_key_to_public_key(secret_key.clone(), timestamp.as_secs(), addr, relays1)?; + let public_key2 = secret_key_to_public_key(secret_key, timestamp.as_secs(), addr, relays2)?; + + let merge12 = merge_openpgp_certificates(public_key1.clone(), public_key2.clone())?; + let merge21 = merge_openpgp_certificates(public_key2.clone(), public_key1.clone())?; + + assert_eq!(merge12, public_key2); + assert_eq!(merge21, public_key1); + assert_ne!(merge12, merge21); + + Ok(()) +} + /// Test PQC support. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_pqc() -> Result<()> { From 3c11f617fc35b2dcf9b5e26a936fd2b04864f871 Mon Sep 17 00:00:00 2001 From: link2xt Date: Fri, 23 Oct 2026 09:33:03 +0000 Subject: [PATCH 5/6] test: add test_import_public_key_tiebreak() --- src/contact/contact_tests.rs | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/contact/contact_tests.rs b/src/contact/contact_tests.rs index 5a7e748d48..06d1dea4f3 100644 --- a/src/contact/contact_tests.rs +++ b/src/contact/contact_tests.rs @@ -3,6 +3,7 @@ use deltachat_contact_tools::{addr_cmp, may_be_valid_addr}; use super::*; use crate::chat::{Chat, get_chat_contacts, send_text_msg}; use crate::chatlist::Chatlist; +use crate::key::secret_key_to_public_key; use crate::receive_imf::receive_imf; use crate::securejoin::get_securejoin_qr; use crate::test_utils::{self, TestContext, TestContextManager, TimeShiftFalsePositiveNote, sync}; @@ -1560,3 +1561,44 @@ async fn test_same_avatar_no_changed_event() -> Result<()> { Ok(()) } + +/// Tests that relay list from imported key is preferred if the signature +/// has the same timestamp as already imported key. +/// If they key is signed twice with different relay list at the same second, +/// we want to use the relay list from the message that arrives last +/// so the newest relay list is used unless the messages are reordered. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_import_public_key_tiebreak() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + let contact = bob.add_or_lookup_contact(alice).await; + + let secret_key = test_utils::alice_keypair(); + let timestamp = pgp::types::Timestamp::now(); + + let addr = "alice@example.net"; + let relays1 = "alice@example.net,alice@example.org"; + let relays2 = "alice@example.net,alice@example.com"; + let relays1_vec: Vec = relays1.split(',').map(String::from).collect(); + let relays2_vec: Vec = relays2.split(',').map(String::from).collect(); + + let public_key1 = + secret_key_to_public_key(secret_key.clone(), timestamp.as_secs(), addr, relays1)?; + let public_key2 = secret_key_to_public_key(secret_key, timestamp.as_secs(), addr, relays2)?; + + import_public_key(bob, &public_key1).await?; + assert_eq!(contact.relay_addresses(bob).await?.unwrap(), relays1_vec); + + import_public_key(bob, &public_key2).await?; + assert_eq!(contact.relay_addresses(bob).await?.unwrap(), relays2_vec); + + import_public_key(bob, &public_key1).await?; + assert_eq!(contact.relay_addresses(bob).await?.unwrap(), relays1_vec); + + import_public_key(bob, &public_key2).await?; + assert_eq!(contact.relay_addresses(bob).await?.unwrap(), relays2_vec); + + Ok(()) +} From d755d695a356d36a8050e68ad2f5def790fc46ea Mon Sep 17 00:00:00 2001 From: link2xt Date: Tue, 20 Oct 2026 12:55:30 +0000 Subject: [PATCH 6/6] fix: fix the order of arguments to merge_openpgp_certificates() call This fixes flaky tests/test_securejoin.py::test_qr_scan_updates_new_relay_address which hanged during the second securejoin on fast network (e.g. when testing against local container) as Bob received securejoin pubkey message from Alice but did not merge the new relay list. --- src/contact.rs | 6 +++++- src/pgp.rs | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/contact.rs b/src/contact.rs index 37860a40ad..9981d7d86a 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -372,7 +372,11 @@ pub(crate) async fn import_public_key( .await? { let old_public_key = SignedPublicKey::from_slice(&public_key_bytes)?; - merged_public_key = merge_openpgp_certificates(public_key.clone(), old_public_key) + + // Order of merge_openpgp_certificates() arguments is important. + // Second argument is the new key, it is preferred in case of ties, + // i.e. same signature timestamps. + merged_public_key = merge_openpgp_certificates(old_public_key, public_key.clone()) .context("Failed to merge public keys")?; &merged_public_key } else { diff --git a/src/pgp.rs b/src/pgp.rs index 907bd35a03..68ffa3e124 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -382,6 +382,8 @@ pub fn symm_encrypt_message( /// or even have some packets maliciously dropped /// (for example, all encryption subkeys dropped) /// or restored from some older version of the certificate. +/// +/// In case of ties, i.e. same timestamps, data from the new certificate is preferred. pub fn merge_openpgp_certificates( old_certificate: SignedPublicKey, new_certificate: SignedPublicKey,