Skip to content

Commit 0f1c10f

Browse files
committed
feat: do not mark Bob as verified if auth token is old
1 parent f5d3f7f commit 0f1c10f

File tree

3 files changed

+76
-23
lines changed

3 files changed

+76
-23
lines changed

src/securejoin.rs

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ use crate::qr::check_qr;
2323
use crate::securejoin::bob::JoinerProgress;
2424
use crate::sync::Sync::*;
2525
use crate::token;
26+
use crate::tools::create_id;
27+
use crate::tools::time;
2628

2729
mod bob;
2830
mod qrinvite;
@@ -75,10 +77,21 @@ pub async fn get_securejoin_qr(context: &Context, group: Option<ChatId>) -> Resu
7577
let sync_token = token::lookup(context, Namespace::InviteNumber, grpid)
7678
.await?
7779
.is_none();
78-
// invitenumber will be used to allow starting the handshake,
79-
// auth will be used to verify the fingerprint
80+
// Invite number is used to request the inviter key.
8081
let invitenumber = token::lookup_or_new(context, Namespace::InviteNumber, grpid).await?;
81-
let auth = token::lookup_or_new(context, Namespace::Auth, grpid).await?;
82+
83+
// Auth token is used to verify the key-contact
84+
// if the token is not old
85+
// and add the contact to the group
86+
// if there is an associated group ID.
87+
//
88+
// We always generate a new auth token
89+
// because auth tokens "expire"
90+
// and can only be used to join groups
91+
// without verification afterwards.
92+
let auth = create_id();
93+
token::save(context, Namespace::Auth, grpid, &auth).await?;
94+
8295
let self_addr = context.get_primary_self_addr().await?;
8396
let self_name = context
8497
.get_config(Config::Displayname)
@@ -364,7 +377,19 @@ pub(crate) async fn handle_securejoin_handshake(
364377
);
365378
return Ok(HandshakeMessage::Ignore);
366379
};
367-
let Some(grpid) = token::auth_foreign_key(context, auth).await? else {
380+
let Some((grpid, timestamp)) = context
381+
.sql
382+
.query_row_optional(
383+
"SELECT foreign_key, timestamp FROM tokens WHERE namespc=? AND token=?",
384+
(Namespace::Auth, auth),
385+
|row| {
386+
let foreign_key: String = row.get(0)?;
387+
let timestamp: i64 = row.get(1)?;
388+
Ok((foreign_key, timestamp))
389+
},
390+
)
391+
.await?
392+
else {
368393
warn!(
369394
context,
370395
"Ignoring {step} message because of invalid auth code."
@@ -382,14 +407,23 @@ pub(crate) async fn handle_securejoin_handshake(
382407
}
383408
};
384409

385-
if !verify_sender_by_fingerprint(context, &fingerprint, contact_id).await? {
410+
let sender_contact = Contact::get_by_id(context, contact_id).await?;
411+
let sender_is_verified = sender_contact
412+
.fingerprint()
413+
.is_some_and(|fp| fp == fingerprint);
414+
if !sender_is_verified {
386415
warn!(
387416
context,
388417
"Ignoring {step} message because of fingerprint mismatch."
389418
);
390419
return Ok(HandshakeMessage::Ignore);
391420
}
392421
info!(context, "Fingerprint verified via Auth code.",);
422+
423+
// Mark the contact as verified if auth code is 600 second old.
424+
if time() < timestamp + 600 {
425+
mark_contact_id_as_verified(context, contact_id, Some(ContactId::SELF)).await?;
426+
}
393427
contact_id.regossip_keys(context).await?;
394428
ContactId::scaleup_origin(context, &[contact_id], Origin::SecurejoinInvited).await?;
395429
// for setup-contact, make Alice's one-to-one chat with Bob visible

src/securejoin/securejoin_tests.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::time::Duration;
2+
13
use deltachat_contact_tools::EmailAddress;
24

35
use super::*;
@@ -11,6 +13,7 @@ use crate::stock_str::{self, messages_e2e_encrypted};
1113
use crate::test_utils::{
1214
TestContext, TestContextManager, TimeShiftFalsePositiveNote, get_chat_msg,
1315
};
16+
use crate::tools::SystemTime;
1417

1518
#[derive(PartialEq)]
1619
enum SetupContactCase {
@@ -800,3 +803,37 @@ async fn test_wrong_auth_token() -> Result<()> {
800803

801804
Ok(())
802805
}
806+
807+
/// Tests that scanning a QR code week later
808+
/// allows Bob to establish a contact with Alice,
809+
/// but does not mark Bob as verified for Alice.
810+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
811+
async fn test_expired_auth_token() -> Result<()> {
812+
let mut tcm = TestContextManager::new();
813+
let alice = &tcm.alice().await;
814+
let bob = &tcm.bob().await;
815+
816+
// Alice creates a QR code.
817+
let alice_qr = get_securejoin_qr(alice, None).await?;
818+
819+
// One week passes, QR code expires.
820+
SystemTime::shift(Duration::from_secs(7 * 24 * 3600));
821+
822+
// Bob scans the QR code.
823+
join_securejoin(bob, &alice_qr).await?;
824+
825+
// vc-request
826+
alice.recv_msg_trash(&bob.pop_sent_msg().await).await;
827+
828+
// vc-auth-requried
829+
bob.recv_msg_trash(&alice.pop_sent_msg().await).await;
830+
831+
// vc-request-with-auth
832+
alice.recv_msg_trash(&bob.pop_sent_msg().await).await;
833+
834+
// Bob should not be verified for Alice.
835+
let contact_bob = alice.add_or_lookup_contact_no_key(bob).await;
836+
assert_eq!(contact_bob.is_verified(alice).await.unwrap(), false);
837+
838+
Ok(())
839+
}

src/token.rs

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -86,24 +86,6 @@ pub async fn exists(context: &Context, namespace: Namespace, token: &str) -> Res
8686
Ok(exists)
8787
}
8888

89-
/// Looks up foreign key by auth token.
90-
///
91-
/// Returns None if auth token is not valid.
92-
/// Returns an empty string if the token corresponds to "setup contact" rather than group join.
93-
pub async fn auth_foreign_key(context: &Context, token: &str) -> Result<Option<String>> {
94-
context
95-
.sql
96-
.query_row_optional(
97-
"SELECT foreign_key FROM tokens WHERE namespc=? AND token=?",
98-
(Namespace::Auth, token),
99-
|row| {
100-
let foreign_key: String = row.get(0)?;
101-
Ok(foreign_key)
102-
},
103-
)
104-
.await
105-
}
106-
10789
pub async fn delete(context: &Context, namespace: Namespace, token: &str) -> Result<()> {
10890
context
10991
.sql

0 commit comments

Comments
 (0)