From f6b5c5d15003bca8feaa9a32383bce4ea0c4d826 Mon Sep 17 00:00:00 2001
From: link2xt <link2xt@testrun.org>
Date: Fri, 20 Sep 2024 15:04:02 +0000
Subject: [PATCH] feat: generate 144-bit group IDs

Instead of generating 72 random bits
and reducing them to 66 bits of Base64 characters,
generate 144 bits (18 bytes)
which is exactly 24 Base64 characters.

This should still be accepted by existing
Delta Chat clients which expect group ID
to be between 11 and 32 characters.

Message-ID creation is also simplified
to not have `Mr.` prefix
and dot in between two IDs.
Now it is a single ID followed by `@localhost`.

Some outdated documentation comments
are removed, e.g. group messages
don't start with `Gr.` already.
---
 src/chat.rs  | 11 +++++------
 src/tools.rs | 34 +++++++++++++++-------------------
 2 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/src/chat.rs b/src/chat.rs
index 4ee894bf74..a557df6e5b 100644
--- a/src/chat.rs
+++ b/src/chat.rs
@@ -6292,11 +6292,10 @@ mod tests {
         // Alice has an SMTP-server replacing the `Message-ID:`-header (as done eg. by outlook.com).
         let sent_msg = alice.pop_sent_msg().await;
         let msg = sent_msg.payload();
-        assert_eq!(msg.match_indices("Message-ID: <Mr.").count(), 2);
-        assert_eq!(msg.match_indices("References: <Mr.").count(), 1);
-        let msg = msg.replace("Message-ID: <Mr.", "Message-ID: <XXX");
-        assert_eq!(msg.match_indices("Message-ID: <Mr.").count(), 0);
-        assert_eq!(msg.match_indices("References: <Mr.").count(), 1);
+        assert_eq!(msg.match_indices("Message-ID: <").count(), 2);
+        assert_eq!(msg.match_indices("References: <").count(), 1);
+        let msg = msg.replace("Message-ID: <", "Message-ID: <X.X");
+        assert_eq!(msg.match_indices("References: <").count(), 1);
 
         // Bob receives this message, he may detect group by `References:`- or `Chat-Group:`-header
         receive_imf(&bob, msg.as_bytes(), false).await.unwrap();
@@ -6313,7 +6312,7 @@ mod tests {
         send_text_msg(&bob, bob_chat.id, "ho!".to_string()).await?;
         let sent_msg = bob.pop_sent_msg().await;
         let msg = sent_msg.payload();
-        let msg = msg.replace("Message-ID: <Mr.", "Message-ID: <XXX");
+        let msg = msg.replace("Message-ID: <", "Message-ID: <X.X");
         let msg = msg.replace("Chat-", "XXXX-");
         assert_eq!(msg.match_indices("Chat-").count(), 0);
 
diff --git a/src/tools.rs b/src/tools.rs
index cdfce4effe..2b0f6618c1 100644
--- a/src/tools.rs
+++ b/src/tools.rs
@@ -272,30 +272,26 @@ async fn maybe_warn_on_outdated(context: &Context, now: i64, approx_compile_time
 
 /* Message-ID tools */
 
-/// Generate an ID. The generated ID should be as short and as unique as possible:
-/// - short, because it may also used as part of Message-ID headers or in QR codes
-/// - unique as two IDs generated on two devices should not be the same. However, collisions are not world-wide but only by the few contacts.
+/// Generate an unique ID.
 ///
-/// IDs generated by this function are 66 bit wide and are returned as 11 base64 characters.
+/// The generated ID should be short but unique:
+/// - short, because it used in Message-ID and Chat-Group-ID headers and in QR codes
+/// - unique as two IDs generated on two devices should not be the same
 ///
-/// Additional information when used as a message-id or group-id:
-/// - for OUTGOING messages this ID is written to the header as `Chat-Group-ID:` and is added to the message ID as `Gr.<grpid>.<random>@<random>`
-/// - for INCOMING messages, the ID is taken from the Chat-Group-ID-header or from the Message-ID in the In-Reply-To: or References:-Header
-/// - the group-id should be a string with the characters [a-zA-Z0-9\-_]
+/// IDs generated by this function have 144 bits of entropy
+/// and are returned as 24 Base64 characters, each containing 6 bits of entropy.
+/// 144 is chosen because it is sufficiently secure
+/// (larger than AES-128 keys used for message encryption)
+/// and divides both by 8 (byte size) and 6 (number of bits in a single Base64 character).
 pub(crate) fn create_id() -> String {
     // ThreadRng implements CryptoRng trait and is supposed to be cryptographically secure.
     let mut rng = thread_rng();
 
-    // Generate 72 random bits.
-    let mut arr = [0u8; 9];
+    // Generate 144 random bits.
+    let mut arr = [0u8; 18];
     rng.fill(&mut arr[..]);
 
-    // Take 11 base64 characters containing 66 random bits.
-    base64::engine::general_purpose::URL_SAFE
-        .encode(arr)
-        .chars()
-        .take(11)
-        .collect()
+    base64::engine::general_purpose::URL_SAFE.encode(arr)
 }
 
 /// Returns true if given string is a valid ID.
@@ -311,7 +307,7 @@ pub(crate) fn validate_id(s: &str) -> bool {
 /// - the message ID should be globally unique
 /// - do not add a counter or any private data as this leaks information unnecessarily
 pub(crate) fn create_outgoing_rfc724_mid() -> String {
-    format!("Mr.{}.{}@localhost", create_id(), create_id())
+    format!("{}@localhost", create_id())
 }
 
 // the returned suffix is lower-case
@@ -925,7 +921,7 @@ DKIM Results: Passed=true";
     #[test]
     fn test_create_id() {
         let buf = create_id();
-        assert_eq!(buf.len(), 11);
+        assert_eq!(buf.len(), 24);
     }
 
     #[test]
@@ -961,7 +957,7 @@ DKIM Results: Passed=true";
     #[test]
     fn test_create_outgoing_rfc724_mid() {
         let mid = create_outgoing_rfc724_mid();
-        assert!(mid.starts_with("Mr."));
+        assert_eq!(mid.len(), 34);
         assert!(mid.ends_with("@localhost"));
     }