fix: Don't reverify contacts by SELF on receipt of a message from another device

src/contact.rs

@@ -1948,16 +1948,21 @@ pub(crate) async fn update_last_seen(
 }
 
 /// Marks contact `contact_id` as verified by `verifier_id`.
+///
+/// `verifier_id == None` means that the verifier is unknown.
 pub(crate) async fn mark_contact_id_as_verified(
     context: &Context,
     contact_id: ContactId,
-    verifier_id: ContactId,
+    verifier_id: Option<ContactId>,
 ) -> Result<()> {
+    ensure_and_debug_assert_ne!(contact_id, ContactId::SELF,);
     ensure_and_debug_assert_ne!(
-        contact_id,
+        Some(contact_id),
         verifier_id,
         "Contact cannot be verified by self",
     );
+    let by_self = verifier_id == Some(ContactId::SELF);
+    let mut verifier_id = verifier_id.unwrap_or(contact_id);
     context
         .sql
         .transaction(|transaction| {
@@ -1970,21 +1975,33 @@ pub(crate) async fn mark_contact_id_as_verified(
                 bail!("Non-key-contact {contact_id} cannot be verified");
             }
             if verifier_id != ContactId::SELF {
-                let verifier_fingerprint: String = transaction.query_row(
-                    "SELECT fingerprint FROM contacts WHERE id=?",
-                    (verifier_id,),
-                    |row| row.get(0),
-                )?;
+                let (verifier_fingerprint, verifier_verifier_id): (String, ContactId) = transaction
+                    .query_row(
+                        "SELECT fingerprint, verifier FROM contacts WHERE id=?",
+                        (verifier_id,),
+                        |row| Ok((row.get(0)?, row.get(1)?)),
+                    )?;
                 if verifier_fingerprint.is_empty() {
                     bail!(
                         "Contact {contact_id} cannot be verified by non-key-contact {verifier_id}"
                     );
                 }
+                ensure!(
+                    verifier_id == contact_id || verifier_verifier_id != ContactId::UNDEFINED,
+                    "Contact {contact_id} cannot be verified by unverified contact {verifier_id}",
+                );
+                if verifier_verifier_id == verifier_id {
+                    // Avoid introducing incorrect reverse chains: if the verifier itself has an
+                    // unknown verifier, it may be `contact_id` actually (directly or indirectly) on
+                    // the other device (which is needed for getting "verified by unknown contact"
+                    // in the first place).
+                    verifier_id = contact_id;
+                }
             }
             transaction.execute(
                 "UPDATE contacts SET verifier=?1
-                 WHERE id=?2 AND (verifier=0 OR ?1=?3)",
-                (verifier_id, contact_id, ContactId::SELF),
+                 WHERE id=?2 AND (verifier=0 OR verifier=id OR ?3)",
+                (verifier_id, contact_id, by_self),
             )?;
             Ok(())
         })

src/context.rs

@@ -1215,7 +1215,7 @@ impl Context {
             .await?
             .first()
             .context("Self reporting bot vCard does not contain a contact")?;
-        mark_contact_id_as_verified(self, contact_id, ContactId::SELF).await?;
+        mark_contact_id_as_verified(self, contact_id, Some(ContactId::SELF)).await?;
 
         let chat_id = ChatId::create_for_contact(self, contact_id).await?;
         chat_id

src/receive_imf.rs

@@ -3691,12 +3691,13 @@ async fn mark_recipients_as_verified(
     if mimeparser.get_header(HeaderDef::ChatVerified).is_none() {
         return Ok(());
     }
+    let verifier_id = Some(from_id).filter(|&id| id != ContactId::SELF);
     for to_id in to_ids.iter().filter_map(|&x| x) {
         if to_id == ContactId::SELF || to_id == from_id {
             continue;
         }
 
-        mark_contact_id_as_verified(context, to_id, from_id).await?;
+        mark_contact_id_as_verified(context, to_id, verifier_id).await?;
         ChatId::set_protection_for_contact(context, to_id, mimeparser.timestamp_sent).await?;
     }
 

src/receive_imf/receive_imf_tests.rs

@@ -5154,6 +5154,32 @@ async fn test_unverified_member_msg() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_dont_reverify_by_self_on_outgoing_msg() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let a0 = &tcm.alice().await;
+    let a1 = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+    let fiona = &tcm.fiona().await;
+
+    let bob_chat_id = chat::create_group_chat(bob, ProtectionStatus::Protected, "Group").await?;
+    let qr = get_securejoin_qr(bob, Some(bob_chat_id)).await?;
+    tcm.exec_securejoin_qr(fiona, bob, &qr).await;
+    tcm.exec_securejoin_qr(a0, bob, &qr).await;
+    tcm.exec_securejoin_qr(a1, bob, &qr).await;
+
+    let a0_chat_id = a0.get_last_msg().await.chat_id;
+    let a0_sent_msg = a0.send_text(a0_chat_id, "Hi").await;
+    a1.recv_msg(&a0_sent_msg).await;
+    let a1_bob_id = a1.add_or_lookup_contact_id(bob).await;
+    let a1_fiona = a1.add_or_lookup_contact(fiona).await;
+    assert_eq!(
+        a1_fiona.get_verifier_id(a1).await?.unwrap().unwrap(),
+        a1_bob_id
+    );
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_sanitize_filename_in_received() -> Result<()> {
     let alice = &TestContext::new_alice().await;

src/securejoin.rs

@@ -210,7 +210,7 @@ async fn verify_sender_by_fingerprint(
     let contact = Contact::get_by_id(context, contact_id).await?;
     let is_verified = contact.fingerprint().is_some_and(|fp| &fp == fingerprint);
     if is_verified {
-        mark_contact_id_as_verified(context, contact_id, ContactId::SELF).await?;
+        mark_contact_id_as_verified(context, contact_id, Some(ContactId::SELF)).await?;
     }
     Ok(is_verified)
 }
@@ -548,7 +548,7 @@ pub(crate) async fn observe_securejoin_on_other_device(
         return Ok(HandshakeMessage::Ignore);
     }
 
-    mark_contact_id_as_verified(context, contact_id, ContactId::SELF).await?;
+    mark_contact_id_as_verified(context, contact_id, Some(ContactId::SELF)).await?;
 
     ChatId::set_protection_for_contact(context, contact_id, mime_message.timestamp_sent).await?;
 

src/test_utils.rs

@@ -1420,7 +1420,7 @@ fn print_logevent(logevent: &LogEvent) {
 /// Saves the other account's public key as verified
 pub(crate) async fn mark_as_verified(this: &TestContext, other: &TestContext) {
     let contact_id = this.add_or_lookup_contact_id(other).await;
-    mark_contact_id_as_verified(this, contact_id, ContactId::SELF)
+    mark_contact_id_as_verified(this, contact_id, Some(ContactId::SELF))
         .await
         .unwrap();
 }