diff --git a/src/contact.rs b/src/contact.rs index fb6716baa5..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 { @@ -1472,9 +1476,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 +1606,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 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(()) +} 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, diff --git a/src/pgp.rs b/src/pgp.rs index 3bc4317e5b..68ffa3e124 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, @@ -379,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, 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<()> {