feat: do not mark Bob as verified if auth token is old

Author: link2xt

src/securejoin.rs

@@ -22,6 +22,7 @@ use crate::qr::check_qr;
 use crate::securejoin::bob::JoinerProgress;
 use crate::sync::Sync::*;
 use crate::token;
+use crate::tools::{create_id, time};
 
 mod bob;
 mod qrinvite;
@@ -86,10 +87,21 @@ pub async fn get_securejoin_qr(context: &Context, group: Option<ChatId>) -> Resu
     let sync_token = token::lookup(context, Namespace::InviteNumber, grpid)
         .await?
         .is_none();
-    // invitenumber will be used to allow starting the handshake,
-    // auth will be used to verify the fingerprint
+    // Invite number is used to request the inviter key.
     let invitenumber = token::lookup_or_new(context, Namespace::InviteNumber, grpid).await?;
-    let auth = token::lookup_or_new(context, Namespace::Auth, grpid).await?;
+
+    // Auth token is used to verify the key-contact
+    // if the token is not old
+    // and add the contact to the group
+    // if there is an associated group ID.
+    //
+    // We always generate a new auth token
+    // because auth tokens "expire"
+    // and can only be used to join groups
+    // without verification afterwards.
+    let auth = create_id();
+    token::save(context, Namespace::Auth, grpid, &auth, time()).await?;
+
     let self_addr = context.get_primary_self_addr().await?;
     let self_name = context
         .get_config(Config::Displayname)
@@ -377,7 +389,19 @@ pub(crate) async fn handle_securejoin_handshake(
                 );
                 return Ok(HandshakeMessage::Ignore);
             };
-            let Some(grpid) = token::auth_foreign_key(context, auth).await? else {
+            let Some((grpid, timestamp)) = context
+                .sql
+                .query_row_optional(
+                    "SELECT foreign_key, timestamp FROM tokens WHERE namespc=? AND token=?",
+                    (Namespace::Auth, auth),
+                    |row| {
+                        let foreign_key: String = row.get(0)?;
+                        let timestamp: i64 = row.get(1)?;
+                        Ok((foreign_key, timestamp))
+                    },
+                )
+                .await?
+            else {
                 warn!(
                     context,
                     "Ignoring {step} message because of invalid auth code."
@@ -395,14 +419,23 @@ pub(crate) async fn handle_securejoin_handshake(
                 }
             };
 
-            if !verify_sender_by_fingerprint(context, &fingerprint, contact_id).await? {
+            let sender_contact = Contact::get_by_id(context, contact_id).await?;
+            let sender_is_verified = sender_contact
+                .fingerprint()
+                .is_some_and(|fp| fp == fingerprint);
+            if !sender_is_verified {
                 warn!(
                     context,
                     "Ignoring {step} message because of fingerprint mismatch."
                 );
                 return Ok(HandshakeMessage::Ignore);
             }
             info!(context, "Fingerprint verified via Auth code.",);
+
+            // Mark the contact as verified if auth code is 600 seconds old.
+            if time() < timestamp + 600 {
+                mark_contact_id_as_verified(context, contact_id, Some(ContactId::SELF)).await?;
+            }
             contact_id.regossip_keys(context).await?;
             ContactId::scaleup_origin(context, &[contact_id], Origin::SecurejoinInvited).await?;
             // for setup-contact, make Alice's one-to-one chat with Bob visible

src/securejoin/securejoin_tests.rs

@@ -1,16 +1,19 @@
+use std::time::Duration;
+
 use deltachat_contact_tools::EmailAddress;
 
 use super::*;
 use crate::chat::{CantSendReason, remove_contact_from_chat};
 use crate::chatlist::Chatlist;
 use crate::constants::Chattype;
 use crate::key::self_fingerprint;
-use crate::mimeparser::GossipedKey;
+use crate::mimeparser::{GossipedKey, SystemMessage};
 use crate::receive_imf::receive_imf;
 use crate::stock_str::{self, messages_e2e_encrypted};
 use crate::test_utils::{
     TestContext, TestContextManager, TimeShiftFalsePositiveNote, get_chat_msg,
 };
+use crate::tools::SystemTime;
 
 #[derive(PartialEq)]
 enum SetupContactCase {
@@ -839,3 +842,120 @@ async fn test_wrong_auth_token() -> Result<()> {
 
     Ok(())
 }
+
+/// Tests that scanning a QR code week later
+/// allows Bob to establish a contact with Alice,
+/// but does not mark Bob as verified for Alice.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_expired_contact_auth_token() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+
+    // Alice creates a QR code.
+    let qr = get_securejoin_qr(alice, None).await?;
+
+    // One week passes, QR code expires.
+    SystemTime::shift(Duration::from_secs(7 * 24 * 3600));
+
+    // Bob scans the QR code.
+    join_securejoin(bob, &qr).await?;
+
+    // vc-request
+    alice.recv_msg_trash(&bob.pop_sent_msg().await).await;
+
+    // vc-auth-requried
+    bob.recv_msg_trash(&alice.pop_sent_msg().await).await;
+
+    // vc-request-with-auth
+    alice.recv_msg_trash(&bob.pop_sent_msg().await).await;
+
+    // Bob should not be verified for Alice.
+    let contact_bob = alice.add_or_lookup_contact_no_key(bob).await;
+    assert_eq!(contact_bob.is_verified(alice).await.unwrap(), false);
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_expired_group_auth_token() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+
+    let alice_chat_id = chat::create_group_chat(alice, "Group").await?;
+
+    // Alice creates a group QR code.
+    let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap();
+
+    // One week passes, QR code expires.
+    SystemTime::shift(Duration::from_secs(7 * 24 * 3600));
+
+    // Bob scans the QR code.
+    join_securejoin(bob, &qr).await?;
+
+    // vg-request
+    alice.recv_msg_trash(&bob.pop_sent_msg().await).await;
+
+    // vg-auth-requried
+    bob.recv_msg_trash(&alice.pop_sent_msg().await).await;
+
+    // vg-request-with-auth
+    alice.recv_msg_trash(&bob.pop_sent_msg().await).await;
+
+    // vg-member-added
+    let bob_member_added_msg = bob.recv_msg(&alice.pop_sent_msg().await).await;
+    assert!(bob_member_added_msg.is_info());
+    assert_eq!(
+        bob_member_added_msg.get_info_type(),
+        SystemMessage::MemberAddedToGroup
+    );
+
+    // Bob should not be verified for Alice.
+    let contact_bob = alice.add_or_lookup_contact_no_key(bob).await;
+    assert_eq!(contact_bob.is_verified(alice).await.unwrap(), false);
+
+    Ok(())
+}
+
+/// Tests that old token is considered expired
+/// even if sync message just arrived.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_expired_synced_auth_token() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let alice2 = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+
+    alice.set_config_bool(Config::SyncMsgs, true).await?;
+    alice2.set_config_bool(Config::SyncMsgs, true).await?;
+
+    // Alice creates a QR code on the second device.
+    let qr = get_securejoin_qr(alice2, None).await?;
+
+    alice2.send_sync_msg().await.unwrap();
+    let sync_msg = alice2.pop_sent_sync_msg().await;
+
+    // One week passes, QR code expires.
+    SystemTime::shift(Duration::from_secs(7 * 24 * 3600));
+
+    alice.recv_msg_trash(&sync_msg).await;
+
+    // Bob scans the QR code.
+    join_securejoin(bob, &qr).await?;
+
+    // vc-request
+    alice.recv_msg_trash(&bob.pop_sent_msg().await).await;
+
+    // vc-auth-requried
+    bob.recv_msg_trash(&alice.pop_sent_msg().await).await;
+
+    // vc-request-with-auth
+    alice.recv_msg_trash(&bob.pop_sent_msg().await).await;
+
+    // Bob should not be verified for Alice.
+    let contact_bob = alice.add_or_lookup_contact_no_key(bob).await;
+    assert_eq!(contact_bob.is_verified(alice).await.unwrap(), false);
+
+    Ok(())
+}

src/token.rs

@@ -88,24 +88,6 @@ pub async fn exists(context: &Context, namespace: Namespace, token: &str) -> Res
     Ok(exists)
 }
 
-/// Looks up foreign key by auth token.
-///
-/// Returns None if auth token is not valid.
-/// Returns an empty string if the token corresponds to "setup contact" rather than group join.
-pub async fn auth_foreign_key(context: &Context, token: &str) -> Result<Option<String>> {
-    context
-        .sql
-        .query_row_optional(
-            "SELECT foreign_key FROM tokens WHERE namespc=? AND token=?",
-            (Namespace::Auth, token),
-            |row| {
-                let foreign_key: String = row.get(0)?;
-                Ok(foreign_key)
-            },
-        )
-        .await
-}
-
 /// Resets all tokens corresponding to the `foreign_key`.
 ///
 /// `foreign_key` is a group ID to reset all group tokens