From ce6ec6406987960aaeeb71019e145a0c807f916e Mon Sep 17 00:00:00 2001 From: link2xt Date: Sun, 7 Nov 2021 15:55:47 +0000 Subject: [PATCH] Do not assign group IDs to ad hoc groups --- CHANGELOG.md | 1 + src/dc_receive_imf.rs | 88 ++++++++++++------------------------------- src/mimefactory.rs | 9 +++-- src/test_utils.rs | 32 ++++++++++++++++ 4 files changed, 64 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00eedd8d9..2ccf375a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ - mark messages as seen on IMAP in batches #3223 - remove Received: based draft detection heuristic #3230 - Use pkgconfig for building Python package #2590 +- Do not assign group IDs to ad-hoc groups #2798 ## 1.77.0 diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 333b501de..ee95c529f 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -9,7 +9,6 @@ use mailparse::{parse_mail, SingleInfo}; use num_traits::FromPrimitive; use once_cell::sync::Lazy; use regex::Regex; -use sha2::{Digest, Sha256}; use crate::chat::{self, Chat, ChatId, ChatIdBlocked, ProtectionStatus}; use crate::config::Config; @@ -1925,9 +1924,6 @@ async fn create_adhoc_group( return Ok(None); } - // Create a new ad-hoc group. - let grpid = create_adhoc_grp_id(context, member_ids).await?; - // use subject as initial chat name let grpname = mime_parser .get_subject() @@ -1936,7 +1932,7 @@ async fn create_adhoc_group( let new_chat_id: ChatId = ChatId::create_multiuser_record( context, Chattype::Group, - &grpid, + "", // Ad hoc groups have no ID. &grpname, create_blocked, ProtectionStatus::Unprotected, @@ -1952,56 +1948,6 @@ async fn create_adhoc_group( Ok(Some(new_chat_id)) } -/// Creates ad-hoc group ID. -/// -/// Algorithm: -/// - sort normalized, lowercased, e-mail addresses alphabetically -/// - put all e-mail addresses into a single string, separate the address by a single comma -/// - sha-256 this string (without possibly terminating null-characters) -/// - encode the first 64 bits of the sha-256 output as lowercase hex (results in 16 characters from the set [0-9a-f]) -/// -/// This ensures that different Delta Chat clients generate the same group ID unless some of them -/// are hidden in BCC. This group ID is sent by DC in the messages sent to this chat, -/// so having the same ID prevents group split. -async fn create_adhoc_grp_id(context: &Context, member_ids: &[ContactId]) -> Result { - let member_cs = context.get_primary_self_addr().await?.to_lowercase(); - let query = format!( - "SELECT addr FROM contacts WHERE id IN({}) AND id!=?", - sql::repeat_vars(member_ids.len()) - ); - let mut params = Vec::new(); - params.extend_from_slice(member_ids); - params.push(ContactId::SELF); - - let members = context - .sql - .query_map( - query, - rusqlite::params_from_iter(params), - |row| row.get::<_, String>(0), - |rows| { - let mut addrs = rows.collect::, _>>()?; - addrs.sort(); - let mut acc = member_cs.clone(); - for addr in &addrs { - acc += ","; - acc += &addr.to_lowercase(); - } - Ok(acc) - }, - ) - .await?; - - Ok(hex_hash(&members)) -} - -#[allow(clippy::indexing_slicing)] -fn hex_hash(s: &str) -> String { - let bytes = s.as_bytes(); - let result = Sha256::digest(bytes); - hex::encode(&result[..8]) -} - async fn check_verified_properties( context: &Context, mimeparser: &MimeMessage, @@ -2272,14 +2218,6 @@ mod tests { use crate::message::Message; use crate::test_utils::{get_chat_msg, TestContext, TestContextManager}; - #[test] - fn test_hex_hash() { - let data = "hello world"; - - let res = hex_hash(data); - assert_eq!(res, "b94d27b9934d3e08"); - } - #[async_std::test] async fn test_grpid_simple() { let context = TestContext::new().await; @@ -4589,6 +4527,30 @@ Second thread."#; let bob_second_reply = bob.get_last_msg().await; assert_eq!(bob_second_reply.chat_id, bob_second_msg.chat_id); + // Alice adds Fiona to both ad hoc groups. + let fiona = TestContext::new_fiona().await; + let (alice_fiona_contact_id, _) = Contact::add_or_lookup( + &alice, + "Fiona", + "fiona@example.net", + Origin::IncomingUnknownTo, + ) + .await?; + + chat::add_contact_to_chat(&alice, alice_first_msg.chat_id, alice_fiona_contact_id).await?; + let alice_first_invite = alice.pop_sent_msg().await; + fiona.recv_msg(&alice_first_invite).await; + let fiona_first_invite = fiona.get_last_msg().await; + + chat::add_contact_to_chat(&alice, alice_second_msg.chat_id, alice_fiona_contact_id).await?; + let alice_second_invite = alice.pop_sent_msg().await; + fiona.recv_msg(&alice_second_invite).await; + let fiona_second_invite = fiona.get_last_msg().await; + + // Fiona was added to two separate chats and should see two separate chats, even though they + // don't have different group IDs to distinguish them. + assert!(fiona_first_invite.chat_id != fiona_second_invite.chat_id); + Ok(()) } diff --git a/src/mimefactory.rs b/src/mimefactory.rs index e4c1a05ee..ab77900fe 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -850,9 +850,12 @@ impl<'a> MimeFactory<'a> { } if chat.typ == Chattype::Group { - headers - .protected - .push(Header::new("Chat-Group-ID".into(), chat.grpid.clone())); + // Send group ID unless it is an ad hoc group that has no ID. + if !chat.grpid.is_empty() { + headers + .protected + .push(Header::new("Chat-Group-ID".into(), chat.grpid.clone())); + } let encoded = encode_words(&chat.name); headers diff --git a/src/test_utils.rs b/src/test_utils.rs index d55e91768..f052ca07e 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -88,6 +88,13 @@ impl TestContextBuilder { self.with_key_pair(bob_keypair()) } + /// Configures as fiona@example.net with fixed secret key. + /// + /// This is a shortcut for `.with_key_pair(bob_keypair()). + pub fn configure_fiona(self) -> Self { + self.with_key_pair(fiona_keypair()) + } + /// Configures the new [`TestContext`] with the provided [`KeyPair`]. /// /// This will extract the email address from the key and configure the context with the @@ -183,6 +190,13 @@ impl TestContext { Self::builder().configure_bob().build().await } + /// Creates a new configured [`TestContext`]. + /// + /// This is a shortcut which configures fiona@example.net with a fixed key. + pub async fn new_fiona() -> Self { + Self::builder().configure_fiona().build().await + } + /// Internal constructor. /// /// `name` is used to identify this context in e.g. log output. This is useful mostly @@ -691,6 +705,24 @@ pub fn bob_keypair() -> KeyPair { } } +/// Load a pre-generated keypair for fiona@example.net from disk. +/// +/// Like [alice_keypair] but a different key and identity. +pub fn fiona_keypair() -> key::KeyPair { + let addr = EmailAddress::new("fiona@example.net").unwrap(); + let public = key::SignedPublicKey::from_asc(include_str!("../test-data/key/fiona-public.asc")) + .unwrap() + .0; + let secret = key::SignedSecretKey::from_asc(include_str!("../test-data/key/fiona-secret.asc")) + .unwrap() + .0; + key::KeyPair { + addr, + public, + secret, + } +} + /// Utility to help wait for and retrieve events. /// /// This buffers the events in order they are emitted. This allows consuming events in