feat!: QR codes and symmetric encryption for broadcast channels (#7268)

Follow-up for https://github.com/chatmail/core/pull/7042, part of
https://github.com/chatmail/core/issues/6884.

This will make it possible to create invite-QR codes for broadcast
channels, and make them symmetrically end-to-end encrypted.

- [x] Go through all the changes in #7042, and check which ones I still
need, and revert all other changes
- [x] Use the classical Securejoin protocol, rather than the new 2-step
protocol
- [x] Make the Rust tests pass
- [x] Make the Python tests pass
- [x] Fix TODOs in the code
- [x] Test it, and fix any bugs I find
- [x] I found a bug when exporting all profiles at once fails sometimes,
though this bug is unrelated to channels:
https://github.com/chatmail/core/issues/7281
- [x] Do a self-review (i.e. read all changes, and check if I see some
things that should be changed)
- [x] Have this PR reviewed and merged
- [ ] Open an issue for "TODO: There is a known bug in the securejoin
protocol"
- [ ] Create an issue that outlines how we can improve the Securejoin
protocol in the future (I don't have the time to do this right now, but
want to do it sometime in winter)
- [ ] Write a guide for UIs how to adapt to the changes (see
https://github.com/deltachat/deltachat-android/pull/3886)

## Backwards compatibility

This is not very backwards compatible:
- Trying to join a symmetrically-encrypted broadcast channel with an old
device will fail
- If you joined a symmetrically-encrypted broadcast channel with one
device, and use an old core on the other device, then the other device
will show a mostly empty chat (except for two device messages)
- If you created a broadcast channel in the past, then you will get an
error message when trying to send into the channel:

> The up to now "experimental channels feature" is about to become an officially supported one. By that, privacy will be improved, it will become faster, and less traffic will be consumed.
> 
> As we do not guarantee feature-stability for such experiments, this means, that you will need to create the channel again. 
> 
> Here is what to do:
>  • Create a new channel
>  • Tap on the channel name
>  • Tap on "QR Invite Code"
>  • Have all recipients scan the QR code, or send them the link
> 
> If you have any questions, please send an email to delta@merlinux.eu or ask at https://support.delta.chat/.


## The symmetric encryption

Symmetric encryption uses a shared secret. Currently, we use AES128 for
encryption everywhere in Delta Chat, so, this is what I'm using for
broadcast channels (though it wouldn't be hard to switch to AES256).

The secret shared between all members of a broadcast channel has 258
bits of entropy (see `fn create_broadcast_shared_secret` in the code).

Since the shared secrets have more entropy than the AES session keys,
it's not necessary to have a hard-to-compute string2key algorithm, so,
I'm using the string2key algorithm `salted`. This is fast enough that
Delta Chat can just try out all known shared secrets. [^1] In order to
prevent DOS attacks, Delta Chat will not attempt to decrypt with a
string2key algorithm other than `salted` [^2].

## The "Securejoin" protocol that adds members to the channel after they
scanned a QR code

This PR uses the classical securejoin protocol, the same that is also
used for group and 1:1 invitations.

The messages sent back and forth are called `vg-request`,
`vg-auth-required`, `vg-request-with-auth`, and `vg-member-added`. I
considered using the `vc-` prefix, because from a protocol-POV, the
distinction between `vc-` and `vg-` isn't important (as @link2xt pointed
out in an in-person discussion), but
1. it would be weird if groups used `vg-` while broadcasts and 1:1 chats
used `vc-`,
2. we don't have a `vc-member-added` message yet, so, this would mean
one more different kind of message
3. we anyways want to switch to a new securejoin protocol soon, which
will be a backwards incompatible change with a transition phase. When we
do this change, we can make everything `vc-`.



[^1]: In a symmetrically encrypted message, it's not visible which
secret was used to encrypt without trying out all secrets. If this does
turn out to be too slow in the future, then we can remember which secret
was used more recently, and and try the most recent secret first. If
this is still too slow, then we can assign a short, non-unique (~2
characters) id to every shared secret, and send it in cleartext. The
receiving Delta Chat will then only try out shared secrets with this id.
Of course, this would leak a little bit of metadata in cleartext, so, I
would like to avoid it.
[^2]: A DOS attacker could send a message with a lot of encrypted
session keys, all of which use a very hard-to-compute string2key
algorithm. Delta Chat would then try to decrypt all of the encrypted
session keys with all of the known shared secrets. In order to prevent
this, as I said, Delta Chat will not attempt to decrypt with a
string2key algorithm other than `salted`

BREAKING CHANGE: A new QR type AskJoinBroadcast; cloning a broadcast
channel is no longer possible; manually adding a member to a broadcast
channel is no longer possible (only by having them scan a QR code)
This commit is contained in:
Hocuri
2025-11-03 21:02:13 +01:00
committed by GitHub
parent 997e8216bf
commit 5034449009
43 changed files with 2635 additions and 475 deletions

View File

@@ -43,13 +43,15 @@ use crate::smtp::send_msg_to_smtp;
use crate::stock_str;
use crate::sync::{self, Sync::*, SyncData};
use crate::tools::{
IsNoneOrEmpty, SystemTime, buf_compress, create_id, create_outgoing_rfc724_mid,
create_smeared_timestamp, create_smeared_timestamps, get_abs_path, gm2local_offset,
smeared_time, time, truncate_msg_text,
IsNoneOrEmpty, SystemTime, buf_compress, create_broadcast_secret, create_id,
create_outgoing_rfc724_mid, create_smeared_timestamp, create_smeared_timestamps, get_abs_path,
gm2local_offset, smeared_time, time, truncate_msg_text,
};
use crate::webxdc::StatusUpdateSerial;
use crate::{chatlist_events, imap};
pub(crate) const PARAM_BROADCAST_SECRET: Param = Param::Arg3;
/// An chat item, such as a message or a marker.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum ChatItem {
@@ -304,7 +306,7 @@ impl ChatId {
info!(
context,
"Created group/mailinglist '{}' grpid={} as {}, blocked={}.",
"Created group/broadcast '{}' grpid={} as {}, blocked={}.",
&grpname,
grpid,
chat_id,
@@ -460,7 +462,11 @@ impl ChatId {
}
/// Adds message "Messages are end-to-end encrypted".
async fn add_encrypted_msg(self, context: &Context, timestamp_sort: i64) -> Result<()> {
pub(crate) async fn add_encrypted_msg(
self,
context: &Context,
timestamp_sort: i64,
) -> Result<()> {
let text = stock_str::messages_e2e_encrypted(context).await;
add_info_msg_with_cmd(
context,
@@ -1489,8 +1495,9 @@ impl Chat {
pub async fn is_self_in_chat(&self, context: &Context) -> Result<bool> {
match self.typ {
Chattype::Single | Chattype::OutBroadcast | Chattype::Mailinglist => Ok(true),
Chattype::Group => is_contact_in_chat(context, self.id, ContactId::SELF).await,
Chattype::InBroadcast => Ok(false),
Chattype::Group | Chattype::InBroadcast => {
is_contact_in_chat(context, self.id, ContactId::SELF).await
}
}
}
@@ -2568,8 +2575,9 @@ pub async fn is_contact_in_chat(
) -> Result<bool> {
// this function works for group and for normal chats, however, it is more useful
// for group chats.
// ContactId::SELF may be used to check, if the user itself is in a group
// chat (ContactId::SELF is not added to normal chats)
// ContactId::SELF may be used to check whether oneself
// is in a group or incoming broadcast chat
// (ContactId::SELF is not added to 1:1 chats or outgoing broadcast channels)
let exists = context
.sql
@@ -2659,8 +2667,12 @@ async fn prepare_send_msg(
// Allow to send "Member removed" messages so we can leave the group/broadcast.
// Necessary checks should be made anyway before removing contact
// from the chat.
CantSendReason::NotAMember | CantSendReason::InBroadcast => {
msg.param.get_cmd() == SystemMessage::MemberRemovedFromGroup
CantSendReason::NotAMember => msg.param.get_cmd() == SystemMessage::MemberRemovedFromGroup,
CantSendReason::InBroadcast => {
matches!(
msg.param.get_cmd(),
SystemMessage::MemberRemovedFromGroup | SystemMessage::SecurejoinMessage
)
}
CantSendReason::MissingKey => msg
.param
@@ -3443,7 +3455,7 @@ pub(crate) async fn create_group_ex(
Ok(chat_id)
}
/// Create a new **broadcast channel**
/// Create a new, outgoing **broadcast channel**
/// (called "Channel" in the UI).
///
/// Broadcast channels are similar to groups on the sending device,
@@ -3460,60 +3472,99 @@ pub(crate) async fn create_group_ex(
/// Returns the created chat's id.
pub async fn create_broadcast(context: &Context, chat_name: String) -> Result<ChatId> {
let grpid = create_id();
create_broadcast_ex(context, Sync, grpid, chat_name).await
let secret = create_broadcast_secret();
create_out_broadcast_ex(context, Sync, grpid, chat_name, secret).await
}
pub(crate) async fn create_broadcast_ex(
const SQL_INSERT_BROADCAST_SECRET: &str =
"INSERT INTO broadcast_secrets (chat_id, secret) VALUES (?, ?)
ON CONFLICT(chat_id) DO UPDATE SET secret=excluded.secret";
pub(crate) async fn create_out_broadcast_ex(
context: &Context,
sync: sync::Sync,
grpid: String,
chat_name: String,
secret: String,
) -> Result<ChatId> {
let row_id = {
let chat_name = &chat_name;
let grpid = &grpid;
let trans_fn = |t: &mut rusqlite::Transaction| {
let cnt = t.execute("UPDATE chats SET name=? WHERE grpid=?", (chat_name, grpid))?;
ensure!(cnt <= 1, "{cnt} chats exist with grpid {grpid}");
if cnt == 1 {
return Ok(t.query_row(
"SELECT id FROM chats WHERE grpid=? AND type=?",
(grpid, Chattype::OutBroadcast),
|row| {
let id: isize = row.get(0)?;
Ok(id)
},
)?);
}
t.execute(
"INSERT INTO chats \
(type, name, grpid, param, created_timestamp) \
VALUES(?, ?, ?, \'U=1\', ?);",
(
Chattype::OutBroadcast,
&chat_name,
&grpid,
create_smeared_timestamp(context),
),
)?;
Ok(t.last_insert_rowid().try_into()?)
};
context.sql.transaction(trans_fn).await?
let chat_name = sanitize_single_line(&chat_name);
if chat_name.is_empty() {
bail!("Invalid broadcast channel name: {chat_name}.");
}
let timestamp = create_smeared_timestamp(context);
let trans_fn = |t: &mut rusqlite::Transaction| -> Result<ChatId> {
let cnt: u32 = t.query_row(
"SELECT COUNT(*) FROM chats WHERE grpid=?",
(&grpid,),
|row| row.get(0),
)?;
ensure!(cnt == 0, "{cnt} chats exist with grpid {grpid}");
t.execute(
"INSERT INTO chats
(type, name, grpid, created_timestamp)
VALUES(?, ?, ?, ?);",
(Chattype::OutBroadcast, &chat_name, &grpid, timestamp),
)?;
let chat_id = ChatId::new(t.last_insert_rowid().try_into()?);
t.execute(SQL_INSERT_BROADCAST_SECRET, (chat_id, &secret))?;
Ok(chat_id)
};
let chat_id = ChatId::new(u32::try_from(row_id)?);
let chat_id = context.sql.transaction(trans_fn).await?;
chat_id.add_encrypted_msg(context, timestamp).await?;
context.emit_msgs_changed_without_ids();
chatlist_events::emit_chatlist_changed(context);
chatlist_events::emit_chatlist_item_changed(context, chat_id);
if sync.into() {
let id = SyncId::Grpid(grpid);
let action = SyncAction::CreateBroadcast(chat_name);
let action = SyncAction::CreateOutBroadcast { chat_name, secret };
self::sync(context, id, action).await.log_err(context).ok();
}
Ok(chat_id)
}
pub(crate) async fn load_broadcast_secret(
context: &Context,
chat_id: ChatId,
) -> Result<Option<String>> {
context
.sql
.query_get_value(
"SELECT secret FROM broadcast_secrets WHERE chat_id=?",
(chat_id,),
)
.await
}
pub(crate) async fn save_broadcast_secret(
context: &Context,
chat_id: ChatId,
secret: &str,
) -> Result<()> {
info!(context, "Saving broadcast secret for chat {chat_id}");
context
.sql
.execute(SQL_INSERT_BROADCAST_SECRET, (chat_id, secret))
.await?;
Ok(())
}
pub(crate) async fn delete_broadcast_secret(context: &Context, chat_id: ChatId) -> Result<()> {
info!(context, "Removing broadcast secret for chat {chat_id}");
context
.sql
.execute("DELETE FROM broadcast_secrets WHERE chat_id=?", (chat_id,))
.await?;
Ok(())
}
/// Set chat contacts in the `chats_contacts` table.
pub(crate) async fn update_chat_contacts_table(
context: &Context,
@@ -3601,6 +3652,30 @@ pub(crate) async fn remove_from_chat_contacts_table(
Ok(())
}
/// Removes a contact from the chat
/// without leaving a trace.
///
/// Note that if we call this function,
/// and then receive a message from another device
/// that doesn't know that this this member was removed
/// then the group membership algorithm will wrongly re-add this member.
pub(crate) async fn remove_from_chat_contacts_table_without_trace(
context: &Context,
chat_id: ChatId,
contact_id: ContactId,
) -> Result<()> {
context
.sql
.execute(
"DELETE FROM chats_contacts
WHERE chat_id=? AND contact_id=?",
(chat_id, contact_id),
)
.await?;
Ok(())
}
/// Adds a contact to the chat.
/// If the group is promoted, also sends out a system message to all group members
pub async fn add_contact_to_chat(
@@ -3628,14 +3703,13 @@ pub(crate) async fn add_contact_to_chat_ex(
// this also makes sure, no contacts are added to special or normal chats
let mut chat = Chat::load_from_db(context, chat_id).await?;
ensure!(
chat.typ == Chattype::Group || chat.typ == Chattype::OutBroadcast,
"{chat_id} is not a group/broadcast where one can add members"
chat.typ == Chattype::Group || (from_handshake && chat.typ == Chattype::OutBroadcast),
"{chat_id} is not a group where one can add members",
);
ensure!(
Contact::real_exists_by_id(context, contact_id).await? || contact_id == ContactId::SELF,
"invalid contact_id {contact_id} for adding to group"
);
ensure!(!chat.is_mailing_list(), "Mailing lists can't be changed");
ensure!(
chat.typ != Chattype::OutBroadcast || contact_id != ContactId::SELF,
"Cannot add SELF to broadcast channel."
@@ -3679,21 +3753,35 @@ pub(crate) async fn add_contact_to_chat_ex(
}
} else {
// else continue and send status mail
if is_contact_in_chat(context, chat_id, contact_id).await? {
return Ok(false);
}
add_to_chat_contacts_table(context, time(), chat_id, &[contact_id]).await?;
}
if chat.typ == Chattype::Group && chat.is_promoted() {
if chat.is_promoted() {
msg.viewtype = Viewtype::Text;
let contact_addr = contact.get_addr().to_lowercase();
msg.text = stock_str::msg_add_member_local(context, contact.id, ContactId::SELF).await;
let added_by = if from_handshake && chat.typ == Chattype::OutBroadcast {
// The contact was added via a QR code rather than explicit user action,
// so it could be confusing to say 'You added member Alice'.
// And in a broadcast, SELF is the only one who can add members,
// so, no information is lost by just writing 'Member Alice added' instead.
ContactId::UNDEFINED
} else {
ContactId::SELF
};
msg.text = stock_str::msg_add_member_local(context, contact.id, added_by).await;
msg.param.set_cmd(SystemMessage::MemberAddedToGroup);
msg.param.set(Param::Arg, contact_addr);
msg.param.set_int(Param::Arg2, from_handshake.into());
let fingerprint = contact.fingerprint().map(|f| f.hex());
msg.param.set_optional(Param::Arg4, fingerprint);
msg.param
.set_int(Param::ContactAddedRemoved, contact.id.to_u32() as i32);
if chat.typ == Chattype::OutBroadcast {
let secret = load_broadcast_secret(context, chat_id)
.await?
.context("Failed to find broadcast shared secret")?;
msg.param.set(PARAM_BROADCAST_SECRET, secret);
}
send_msg(context, chat_id, &mut msg).await?;
sync = Nosync;
@@ -3847,7 +3935,18 @@ pub async fn remove_contact_from_chat(
);
let chat = Chat::load_from_db(context, chat_id).await?;
if chat.typ == Chattype::Group || chat.typ == Chattype::OutBroadcast {
if chat.typ == Chattype::InBroadcast {
ensure!(
contact_id == ContactId::SELF,
"Cannot remove other member from incoming broadcast channel"
);
delete_broadcast_secret(context, chat_id).await?;
}
if matches!(
chat.typ,
Chattype::Group | Chattype::OutBroadcast | Chattype::InBroadcast
) {
if !chat.is_self_in_chat(context).await? {
let err_msg = format!(
"Cannot remove contact {contact_id} from chat {chat_id}: self not in group."
@@ -3860,24 +3959,25 @@ pub async fn remove_contact_from_chat(
if chat.is_promoted() {
remove_from_chat_contacts_table(context, chat_id, contact_id).await?;
} else {
context
.sql
.execute(
"DELETE FROM chats_contacts
WHERE chat_id=? AND contact_id=?",
(chat_id, contact_id),
)
.await?;
remove_from_chat_contacts_table_without_trace(context, chat_id, contact_id).await?;
}
// We do not return an error if the contact does not exist in the database.
// This allows to delete dangling references to deleted contacts
// in case of the database becoming inconsistent due to a bug.
if let Some(contact) = Contact::get_by_id_optional(context, contact_id).await? {
if chat.typ == Chattype::Group && chat.is_promoted() {
if chat.is_promoted() {
let addr = contact.get_addr();
let fingerprint = contact.fingerprint().map(|f| f.hex());
let res = send_member_removal_msg(context, &chat, contact_id, addr).await;
let res = send_member_removal_msg(
context,
&chat,
contact_id,
addr,
fingerprint.as_deref(),
)
.await;
if contact_id == ContactId::SELF {
res?;
@@ -3896,11 +3996,6 @@ pub async fn remove_contact_from_chat(
chat.sync_contacts(context).await.log_err(context).ok();
}
}
} else if chat.typ == Chattype::InBroadcast && contact_id == ContactId::SELF {
// For incoming broadcast channels, it's not possible to remove members,
// but it's possible to leave:
let self_addr = context.get_primary_self_addr().await?;
send_member_removal_msg(context, &chat, contact_id, &self_addr).await?;
} else {
bail!("Cannot remove members from non-group chats.");
}
@@ -3913,6 +4008,7 @@ async fn send_member_removal_msg(
chat: &Chat,
contact_id: ContactId,
addr: &str,
fingerprint: Option<&str>,
) -> Result<MsgId> {
let mut msg = Message::new(Viewtype::Text);
@@ -3928,6 +4024,7 @@ async fn send_member_removal_msg(
msg.param.set_cmd(SystemMessage::MemberRemovedFromGroup);
msg.param.set(Param::Arg, addr.to_lowercase());
msg.param.set_optional(Param::Arg4, fingerprint);
msg.param
.set(Param::ContactAddedRemoved, contact_id.to_u32());
@@ -4694,7 +4791,10 @@ pub(crate) enum SyncAction {
SetVisibility(ChatVisibility),
SetMuted(MuteDuration),
/// Create broadcast channel with the given name.
CreateBroadcast(String),
CreateOutBroadcast {
chat_name: String,
secret: String,
},
/// Create encrypted group chat with the given name.
CreateGroupEncrypted(String),
Rename(String),
@@ -4759,12 +4859,23 @@ impl Context {
.id
}
SyncId::Grpid(grpid) => {
if let SyncAction::CreateBroadcast(name) = action {
create_broadcast_ex(self, Nosync, grpid.clone(), name.clone()).await?;
return Ok(());
} else if let SyncAction::CreateGroupEncrypted(name) = action {
create_group_ex(self, Nosync, grpid.clone(), name).await?;
return Ok(());
match action {
SyncAction::CreateOutBroadcast { chat_name, secret } => {
create_out_broadcast_ex(
self,
Nosync,
grpid.to_string(),
chat_name.clone(),
secret.to_string(),
)
.await?;
return Ok(());
}
SyncAction::CreateGroupEncrypted(name) => {
create_group_ex(self, Nosync, grpid.clone(), name).await?;
return Ok(());
}
_ => {}
}
get_chat_id_by_grpid(self, grpid)
.await?
@@ -4786,7 +4897,8 @@ impl Context {
SyncAction::Accept => chat_id.accept_ex(self, Nosync).await,
SyncAction::SetVisibility(v) => chat_id.set_visibility_ex(self, Nosync, *v).await,
SyncAction::SetMuted(duration) => set_muted_ex(self, Nosync, chat_id, *duration).await,
SyncAction::CreateBroadcast(_) | SyncAction::CreateGroupEncrypted(..) => {
SyncAction::CreateOutBroadcast { .. } | SyncAction::CreateGroupEncrypted(..) => {
// Create action should have been handled above already.
Err(anyhow!("sync_alter_chat({id:?}, {action:?}): Bad request."))
}
SyncAction::Rename(to) => rename_ex(self, Nosync, chat_id, to).await,