mirror of
https://github.com/chatmail/core.git
synced 2026-04-21 15:36:30 +03:00
feat: Securejoin v3, encrypt all securejoin messages (#7754)
Close https://github.com/chatmail/core/issues/7396. Before reviewing,
you should read the issue description of
https://github.com/chatmail/core/issues/7396.
I recommend to review with hidden whitespace changes.
TODO:
- [x] Implement the new protocol
- [x] Make Rust tests pass
- [x] Make Python tests pass
- [x] Test it manually on a phone
- [x] Print the sent messages, and check that they look how they should:
[test_secure_join_group_with_mime_printed.txt](https://github.com/user-attachments/files/24800556/test_secure_join_group.txt)
- [x] Fix bug: If Alice has a second device, then Bob's chat won't be
shown yet on that second device. Also, Bob's contact isn't shown in her
contact list. As soon as either party writes something into the chat,
the that shows up and everything is fine. All of this is still a way
better UX than in WhatsApp, where Bob always has to write first 😂
Still, I should fix that.
- This is actually caused by a larger bug: AUTH tokens aren't synced if
there is no corresponding INVITE token.
- Fixed by 6b658a0e0
- [x] Either make a new `auth_tokens` table with a proper UNIQUE bound,
or put a UNIQUE bound on the `tokens` table
- [x] Benchmarking
- [x] TODOs in the code, maybe change naming of the new functions
- [x] Write test for interop with older DC (esp. that the original
securejoin runs if you remove the &v=3 param)
- [x] From a cryptography perspective, is it fine that vc-request is
encrypted with AUTH, rather than a separate secret (like INVITE)?
- [x] Make sure that QR codes without INVITE work, so that we can remove
it eventually
- [x] Self-review, and comment on some of my code changes to explain
what they do
- [x] ~~Maybe use a new table rather than reusing AUTH token.~~ See
https://github.com/chatmail/core/pull/7754#discussion_r2728544725
- [ ] Update documentation; I'll do that in a separate PR. All necessary
information is in the https://github.com/chatmail/core/issues/7396 issue
description
- [ ] Update tests and other code to use the new names (e.g.
`request-pubkey` rather than `request` and `pubkey` rather than
`auth-required`); I'll do that in a follow-up PR
**Backwards compatibility:**
Everything works seamlessly in my tests. If both devices are updated,
then the new protocol is used; otherwise, the old protocol is used. If
there is a not-yet-updated second device, it will correctly observe the
protocol, and mark the chat partner as verified.
Note that I removed the `Auto-Submitted: auto-replied` header from
securejoin messages. We don't need it ourselves, it's a cleartext header
that leaks too much information, and I can't see any reason to have it.
---------
Co-authored-by: iequidoo <117991069+iequidoo@users.noreply.github.com>
This commit is contained in:
@@ -20,14 +20,14 @@ use crate::headerdef::HeaderDef;
|
||||
use crate::key::{DcKey, Fingerprint, load_self_public_key};
|
||||
use crate::log::LogExt as _;
|
||||
use crate::log::warn;
|
||||
use crate::message::{Message, Viewtype};
|
||||
use crate::message::{self, Message, MsgId, Viewtype};
|
||||
use crate::mimeparser::{MimeMessage, SystemMessage};
|
||||
use crate::param::Param;
|
||||
use crate::qr::check_qr;
|
||||
use crate::securejoin::bob::JoinerProgress;
|
||||
use crate::sync::Sync::*;
|
||||
use crate::tools::{create_id, time};
|
||||
use crate::{SecurejoinSource, stats};
|
||||
use crate::tools::{create_id, create_outgoing_rfc724_mid, time};
|
||||
use crate::{SecurejoinSource, mimefactory, stats};
|
||||
use crate::{SecurejoinUiPath, token};
|
||||
|
||||
mod bob;
|
||||
@@ -127,9 +127,6 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option<ChatId>) -> Resul
|
||||
None => None,
|
||||
};
|
||||
let grpid = chat.as_ref().map(|c| c.grpid.as_str());
|
||||
let sync_token = token::lookup(context, Namespace::InviteNumber, grpid)
|
||||
.await?
|
||||
.is_none();
|
||||
// Invite number is used to request the inviter key.
|
||||
let invitenumber = token::lookup_or_new(context, Namespace::InviteNumber, grpid).await?;
|
||||
|
||||
@@ -156,12 +153,10 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option<ChatId>) -> Resul
|
||||
.unwrap_or_default();
|
||||
|
||||
let qr = if let Some(chat) = chat {
|
||||
if sync_token {
|
||||
context
|
||||
.sync_qr_code_tokens(Some(chat.grpid.as_str()))
|
||||
.await?;
|
||||
context.scheduler.interrupt_smtp().await;
|
||||
}
|
||||
context
|
||||
.sync_qr_code_tokens(Some(chat.grpid.as_str()))
|
||||
.await?;
|
||||
context.scheduler.interrupt_smtp().await;
|
||||
|
||||
let chat_name = chat.get_name();
|
||||
let chat_name_shortened = shorten_name(chat_name, 25);
|
||||
@@ -178,11 +173,11 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option<ChatId>) -> Resul
|
||||
if chat.typ == Chattype::OutBroadcast {
|
||||
// For historic reansons, broadcasts currently use j instead of i for the invitenumber.
|
||||
format!(
|
||||
"https://i.delta.chat/#{fingerprint}&x={grpid}&j={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&b={chat_name_urlencoded}",
|
||||
"https://i.delta.chat/#{fingerprint}&v=3&x={grpid}&j={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&b={chat_name_urlencoded}",
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
"https://i.delta.chat/#{fingerprint}&x={grpid}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&g={chat_name_urlencoded}",
|
||||
"https://i.delta.chat/#{fingerprint}&v=3&x={grpid}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}&g={chat_name_urlencoded}",
|
||||
)
|
||||
}
|
||||
} else {
|
||||
@@ -190,12 +185,12 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option<ChatId>) -> Resul
|
||||
let self_name_urlencoded = utf8_percent_encode(&self_name_shortened, DISALLOWED_CHARACTERS)
|
||||
.to_string()
|
||||
.replace("%20", "+");
|
||||
if sync_token {
|
||||
context.sync_qr_code_tokens(None).await?;
|
||||
context.scheduler.interrupt_smtp().await;
|
||||
}
|
||||
|
||||
context.sync_qr_code_tokens(None).await?;
|
||||
context.scheduler.interrupt_smtp().await;
|
||||
|
||||
format!(
|
||||
"https://i.delta.chat/#{fingerprint}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}",
|
||||
"https://i.delta.chat/#{fingerprint}&v=3&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}",
|
||||
)
|
||||
};
|
||||
|
||||
@@ -346,12 +341,18 @@ pub(crate) enum HandshakeMessage {
|
||||
/// Step of Secure-Join protocol.
|
||||
#[derive(Debug, Display, PartialEq, Eq)]
|
||||
pub(crate) enum SecureJoinStep {
|
||||
/// vc-request or vg-request
|
||||
/// vc-request or vg-request; only used in legacy securejoin
|
||||
Request { invitenumber: String },
|
||||
|
||||
/// vc-auth-required or vg-auth-required
|
||||
/// vc-auth-required or vg-auth-required; only used in legacy securejoin
|
||||
AuthRequired,
|
||||
|
||||
/// vc-request-pubkey; only used in securejoin v3
|
||||
RequestPubkey,
|
||||
|
||||
/// vc-pubkey; only used in securejoin v3
|
||||
Pubkey,
|
||||
|
||||
/// vc-request-with-auth or vg-request-with-auth
|
||||
RequestWithAuth,
|
||||
|
||||
@@ -381,6 +382,8 @@ pub(crate) fn get_secure_join_step(mime_message: &MimeMessage) -> Option<SecureJ
|
||||
})
|
||||
} else if let Some(step) = mime_message.get_header(HeaderDef::SecureJoin) {
|
||||
match step {
|
||||
"vc-request-pubkey" => Some(SecureJoinStep::RequestPubkey),
|
||||
"vc-pubkey" => Some(SecureJoinStep::Pubkey),
|
||||
"vg-auth-required" | "vc-auth-required" => Some(SecureJoinStep::AuthRequired),
|
||||
"vg-request-with-auth" | "vc-request-with-auth" => {
|
||||
Some(SecureJoinStep::RequestWithAuth)
|
||||
@@ -439,7 +442,10 @@ pub(crate) async fn handle_securejoin_handshake(
|
||||
// will improve security (completely unrelated to the securejoin protocol)
|
||||
// and is something we want to do in the future:
|
||||
// https://www.rfc-editor.org/rfc/rfc9580.html#name-surreptitious-forwarding
|
||||
if !matches!(step, SecureJoinStep::Request { .. }) {
|
||||
if !matches!(
|
||||
step,
|
||||
SecureJoinStep::Request { .. } | SecureJoinStep::RequestPubkey | SecureJoinStep::Pubkey
|
||||
) {
|
||||
let mut self_found = false;
|
||||
let self_fingerprint = load_self_public_key(context).await?.dc_fingerprint();
|
||||
for (addr, key) in &mime_message.gossiped_keys {
|
||||
@@ -505,7 +511,54 @@ pub(crate) async fn handle_securejoin_handshake(
|
||||
==== Bob - the joiner's side =====
|
||||
==== Step 4 in "Setup verified contact" protocol =====
|
||||
========================================================*/
|
||||
bob::handle_auth_required(context, mime_message).await
|
||||
bob::handle_auth_required_or_pubkey(context, mime_message).await
|
||||
}
|
||||
SecureJoinStep::RequestPubkey => {
|
||||
/*========================================================
|
||||
==== Alice - the inviter's side =====
|
||||
==== Bob requests our public key (Securejoin v3) =====
|
||||
========================================================*/
|
||||
|
||||
debug_assert!(
|
||||
mime_message.signature.is_none(),
|
||||
"RequestPubkey is not supposed to be signed"
|
||||
);
|
||||
let Some(auth) = mime_message.get_header(HeaderDef::SecureJoinAuth) else {
|
||||
warn!(
|
||||
context,
|
||||
"Ignoring {step} message because of missing auth code."
|
||||
);
|
||||
return Ok(HandshakeMessage::Ignore);
|
||||
};
|
||||
if !token::exists(context, token::Namespace::Auth, auth).await? {
|
||||
warn!(context, "Secure-join denied (bad auth).");
|
||||
return Ok(HandshakeMessage::Ignore);
|
||||
}
|
||||
|
||||
let rfc724_mid = create_outgoing_rfc724_mid();
|
||||
let addr = ContactAddress::new(&mime_message.from.addr)?;
|
||||
let attach_self_pubkey = true;
|
||||
let rendered_message = mimefactory::render_symm_encrypted_securejoin_message(
|
||||
context,
|
||||
"vc-pubkey",
|
||||
&rfc724_mid,
|
||||
attach_self_pubkey,
|
||||
auth,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let msg_id = message::insert_tombstone(context, &rfc724_mid).await?;
|
||||
insert_into_smtp(context, &rfc724_mid, &addr, rendered_message, msg_id).await?;
|
||||
context.scheduler.interrupt_smtp().await;
|
||||
|
||||
Ok(HandshakeMessage::Done)
|
||||
}
|
||||
SecureJoinStep::Pubkey => {
|
||||
/*========================================================
|
||||
==== Bob - the joiner's side =====
|
||||
==== Alice sent us her pubkey (Securejoin v3) =====
|
||||
========================================================*/
|
||||
bob::handle_auth_required_or_pubkey(context, mime_message).await
|
||||
}
|
||||
SecureJoinStep::RequestWithAuth => {
|
||||
/*==========================================================
|
||||
@@ -666,6 +719,24 @@ pub(crate) async fn handle_securejoin_handshake(
|
||||
}
|
||||
}
|
||||
|
||||
async fn insert_into_smtp(
|
||||
context: &Context,
|
||||
rfc724_mid: &str,
|
||||
recipient: &str,
|
||||
rendered_message: String,
|
||||
msg_id: MsgId,
|
||||
) -> Result<(), Error> {
|
||||
context
|
||||
.sql
|
||||
.execute(
|
||||
"INSERT INTO smtp (rfc724_mid, recipients, mime, msg_id)
|
||||
VALUES (?1, ?2, ?3, ?4)",
|
||||
(&rfc724_mid, &recipient, &rendered_message, msg_id),
|
||||
)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Observe self-sent Securejoin message.
|
||||
///
|
||||
/// In a multi-device-setup, there may be other devices that "see" the handshake messages.
|
||||
@@ -697,6 +768,8 @@ pub(crate) async fn observe_securejoin_on_other_device(
|
||||
match step {
|
||||
SecureJoinStep::Request { .. }
|
||||
| SecureJoinStep::AuthRequired
|
||||
| SecureJoinStep::RequestPubkey
|
||||
| SecureJoinStep::Pubkey
|
||||
| SecureJoinStep::Deprecated
|
||||
| SecureJoinStep::Unknown { .. } => {
|
||||
return Ok(HandshakeMessage::Ignore);
|
||||
|
||||
Reference in New Issue
Block a user