From 4c00bc109fbdf3c57a732e6003031ff2c131286c Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 21 Jan 2026 13:32:10 +0100 Subject: [PATCH] Fix some bugs when sending vc-pubkey, decrypt vc-pubkey on Bob's side --- src/mimeparser.rs | 53 +++++++++++++++++++++++++++++++++++-------- src/securejoin.rs | 8 +++---- src/securejoin/bob.rs | 1 + 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 8982416dd..6e3460dbe 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -355,7 +355,7 @@ impl MimeMessage { // Remove headers that are allowed _only_ in the encrypted+signed part. It's ok to leave // them in signed-only emails, but has no value currently. - Self::remove_secured_headers(&mut headers, &mut headers_removed); + Self::remove_secured_headers(&mut headers, &mut headers_removed, false); let mut from = from.context("No from in message")?; let private_keyring = load_self_secret_keyring(context).await?; @@ -382,6 +382,11 @@ impl MimeMessage { let mail_raw; // Memory location for a possible decrypted message. let decrypted_msg; // Decrypted signed OpenPGP message. + + // TODO performance: + // - maybe we should start sorting by timestamp + // - we shouldn't do 3 SQL requests even if the message isn't symm-encrypted + // - we're loading the whole bobstate, just to get the auth token let mut secrets: Vec = context .sql .query_map_vec("SELECT secret FROM broadcast_secrets", (), |row| { @@ -389,8 +394,24 @@ impl MimeMessage { Ok(secret) }) .await?; - secrets.extend(token::lookup_all(context, token::Namespace::Auth).await?); + context + .sql + .query_map( + "SELECT id, invite FROM bobstate", + (), + |row| { + let invite: crate::securejoin::QrInvite = row.get(1)?; + Ok(invite) + }, + |rows| { + for row in rows { + secrets.push(row?.authcode().to_string()); + } + Ok(()) + }, + ) + .await?; let (mail, is_encrypted) = match tokio::task::block_in_place(|| try_decrypt(&mail, &private_keyring, &secrets)) { @@ -608,7 +629,7 @@ impl MimeMessage { } } if signatures.is_empty() { - Self::remove_secured_headers(&mut headers, &mut headers_removed); + Self::remove_secured_headers(&mut headers, &mut headers_removed, is_encrypted); } if !is_encrypted { signatures.clear(); @@ -1707,20 +1728,34 @@ impl MimeMessage { .and_then(|msgid| parse_message_id(msgid).ok()) } + /// Remove headers that are only allowed to be present in an encrypted-and-signed message: fn remove_secured_headers( headers: &mut HashMap, removed: &mut HashSet, + // Whether the message was encrypted (but not signed): + encrypted: bool, ) { remove_header(headers, "secure-join-fingerprint", removed); - remove_header(headers, "secure-join-auth", removed); remove_header(headers, "chat-verified", removed); remove_header(headers, "autocrypt-gossip", removed); - // Secure-Join is secured unless it is an initial "vc-request"/"vg-request". - if let Some(secure_join) = remove_header(headers, "secure-join", removed) - && (secure_join == "vc-request" || secure_join == "vg-request") - { - headers.insert("secure-join".to_string(), secure_join); + if headers.get("secure-join") == Some(&"vc-request-pubkey".to_string()) && encrypted { + // vc-request-pubkey message is encrypted, but unsigned, + // and contains a Secure-Join-Auth header. + // + // It is unsigned in order not to leak Bob's identity to a server operator + // that scraped the AUTH token somewhere from the web, + // and because Alice anyways couldn't verify his signature at this step, + // because she doesn't know his public key yet. + } else { + remove_header(headers, "secure-join-auth", removed); + + // Secure-Join is secured unless it is an initial "vc-request"/"vg-request". + if let Some(secure_join) = remove_header(headers, "secure-join", removed) + && (secure_join == "vc-request" || secure_join == "vg-request") + { + headers.insert("secure-join".to_string(), secure_join); + } } } diff --git a/src/securejoin.rs b/src/securejoin.rs index 219242a71..816c4b368 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -448,7 +448,7 @@ pub(crate) async fn handle_securejoin_handshake( // https://www.rfc-editor.org/rfc/rfc9580.html#name-surreptitious-forwarding if !matches!( step, - SecureJoinStep::Request { .. } | SecureJoinStep::RequestPubkey + SecureJoinStep::Request { .. } | SecureJoinStep::RequestPubkey | SecureJoinStep::Pubkey ) { let mut self_found = false; let self_fingerprint = load_self_public_key(context).await?.dc_fingerprint(); @@ -523,8 +523,8 @@ pub(crate) async fn handle_securejoin_handshake( ==== Bob requests our public key (Securejoin v3) ===== ========================================================*/ - if !mime_message.was_encrypted() { - warn!(context, "Ignoring unencrypted RequestPubkey"); + if mime_message.signature.is_some() { + warn!(context, "RequestPubkey is not supposed to be signed"); return Ok(HandshakeMessage::Ignore); } let Some(auth) = mime_message.get_header(HeaderDef::SecureJoinAuth) else { @@ -562,7 +562,7 @@ pub(crate) async fn handle_securejoin_handshake( ==== Bob - the joiner's side ===== ==== Alice sent us her pubkey (Securejoin v3) ===== ========================================================*/ - todo!() + todo!("Hocuri Pubkey not implemented") } SecureJoinStep::RequestWithAuth => { /*========================================================== diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 6b18b3d17..fadf7cde4 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -302,6 +302,7 @@ pub(crate) async fn send_handshake_message( step: BobHandshakeMsg, ) -> Result<()> { if invite.is_v3() && matches!(step, BobHandshakeMsg::Request) { + // TODO this code might be moved "up" into the caller function // Send a minimal symmetrically-encrypted vc-request message let rfc724_mid = create_outgoing_rfc724_mid();