From 9694ba3a75ed03cf1162b615f5f8afd801925272 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 2 Oct 2025 16:44:48 +0200 Subject: [PATCH 01/27] [WIP] QR codes and symmetric encryption for broadcast channels --- Cargo.toml | 5 + benches/benchmark_decrypting.rs | 199 +++++++ deltachat-ffi/src/lot.rs | 6 + deltachat-jsonrpc/src/api.rs | 2 +- deltachat-jsonrpc/src/api/types/qr.rs | 39 ++ .../src/deltachat_rpc_client/account.py | 2 +- .../src/deltachat_rpc_client/message.py | 11 + deltachat-rpc-client/tests/test_securejoin.py | 127 ++++ deltachat-rpc-client/tests/test_something.py | 127 +++- src/chat.rs | 275 ++++++--- src/chat/chat_tests.rs | 549 ++++++++++++++---- src/decrypt.rs | 6 +- src/e2ee.rs | 21 + src/headerdef.rs | 6 + src/imex/key_transfer.rs | 2 +- src/internals_for_benchmarks.rs | 42 ++ src/lib.rs | 6 + src/mimefactory.rs | 149 ++++- src/mimeparser.rs | 23 +- src/param.rs | 28 +- src/pgp.rs | 232 +++++++- src/qr.rs | 74 ++- src/receive_imf.rs | 193 ++++-- src/receive_imf/receive_imf_tests.rs | 2 +- src/securejoin.rs | 129 ++-- src/securejoin/bob.rs | 94 ++- src/securejoin/qrinvite.rs | 41 +- src/securejoin/securejoin_tests.rs | 72 ++- src/sql/migrations.rs | 12 + src/sync.rs | 3 +- src/test_utils.rs | 61 +- src/tools.rs | 24 + .../test_broadcast_joining_golden_alice | 6 + ...test_broadcast_joining_golden_alice_direct | 4 + .../golden/test_broadcast_joining_golden_bob | 6 + test-data/golden/test_sync_broadcast_alice1 | 7 + test-data/golden/test_sync_broadcast_alice2 | 7 + test-data/golden/test_sync_broadcast_bob | 8 + .../message/text_from_alice_encrypted.eml | 87 +++ .../message/text_symmetrically_encrypted.eml | 56 ++ 40 files changed, 2326 insertions(+), 417 deletions(-) create mode 100644 benches/benchmark_decrypting.rs create mode 100644 src/internals_for_benchmarks.rs create mode 100644 test-data/golden/test_broadcast_joining_golden_alice create mode 100644 test-data/golden/test_broadcast_joining_golden_alice_direct create mode 100644 test-data/golden/test_broadcast_joining_golden_bob create mode 100644 test-data/golden/test_sync_broadcast_alice1 create mode 100644 test-data/golden/test_sync_broadcast_alice2 create mode 100644 test-data/golden/test_sync_broadcast_bob create mode 100644 test-data/message/text_from_alice_encrypted.eml create mode 100644 test-data/message/text_symmetrically_encrypted.eml diff --git a/Cargo.toml b/Cargo.toml index dc116e8f4c..4c236ad144 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -157,6 +157,11 @@ name = "receive_emails" required-features = ["internals"] harness = false +[[bench]] +name = "benchmark_decrypting" +required-features = ["internals"] +harness = false + [[bench]] name = "get_chat_msgs" harness = false diff --git a/benches/benchmark_decrypting.rs b/benches/benchmark_decrypting.rs new file mode 100644 index 0000000000..159b628f35 --- /dev/null +++ b/benches/benchmark_decrypting.rs @@ -0,0 +1,199 @@ +//! Benchmarks for message decryption, +//! comparing decryption of symmetrically-encrypted messages +//! to decryption of asymmetrically-encrypted messages. +//! +//! Call with +//! +//! ```text +//! cargo bench --bench benchmark_decrypting --features="internals" +//! ``` +//! +//! or, if you want to only run e.g. the 'Decrypt a symmetrically encrypted message' benchmark: +//! +//! ```text +//! cargo bench --bench benchmark_decrypting --features="internals" -- 'Decrypt a symmetrically encrypted message' +//! ``` +//! +//! You can also pass a substring. +//! So, you can run all 'Decrypt and parse' benchmarks with: +//! +//! ```text +//! cargo bench --bench benchmark_decrypting --features="internals" -- 'Decrypt and parse' +//! ``` +//! +//! Symmetric decryption has to try out all known secrets, +//! You can benchmark this by adapting the `NUM_SECRETS` variable. + +use std::hint::black_box; + +use criterion::{Criterion, criterion_group, criterion_main}; +use deltachat::internals_for_benchmarks::create_broadcast_shared_secret; +use deltachat::internals_for_benchmarks::create_dummy_keypair; +use deltachat::internals_for_benchmarks::save_broadcast_shared_secret; +use deltachat::{ + Events, + chat::ChatId, + config::Config, + context::Context, + internals_for_benchmarks::key_from_asc, + internals_for_benchmarks::parse_and_get_text, + internals_for_benchmarks::store_self_keypair, + pgp::{KeyPair, decrypt, pk_encrypt, symm_encrypt_message}, + stock_str::StockStrings, +}; +use rand::{Rng, thread_rng}; +use tempfile::tempdir; + +const NUM_SECRETS: usize = 500; + +async fn create_context() -> Context { + let dir = tempdir().unwrap(); + let dbfile = dir.path().join("db.sqlite"); + let context = Context::new(dbfile.as_path(), 100, Events::new(), StockStrings::new()) + .await + .unwrap(); + + context + .set_config(Config::ConfiguredAddr, Some("bob@example.net")) + .await + .unwrap(); + let secret = key_from_asc(include_str!("../test-data/key/bob-secret.asc")).unwrap(); + let public = secret.signed_public_key(); + let key_pair = KeyPair { public, secret }; + store_self_keypair(&context, &key_pair) + .await + .expect("Failed to save key"); + + context +} + +fn criterion_benchmark(c: &mut Criterion) { + let mut group = c.benchmark_group("Decrypt"); + + // =========================================================================================== + // Benchmarks for decryption only, without any other parsing + // =========================================================================================== + + group.sample_size(10); + + group.bench_function("Decrypt a symmetrically encrypted message", |b| { + let plain = generate_plaintext(); + let secrets = generate_secrets(); + let encrypted = tokio::runtime::Runtime::new().unwrap().block_on(async { + let secret = secrets[NUM_SECRETS / 2].clone(); + symm_encrypt_message( + plain.clone(), + black_box(&secret), + create_dummy_keypair("alice@example.org").unwrap().secret, + true, + ) + .await + .unwrap() + }); + + b.iter(|| { + let mut msg = + decrypt(encrypted.clone().into_bytes(), &[], black_box(&secrets)).unwrap(); + let decrypted = msg.as_data_vec().unwrap(); + + assert_eq!(black_box(decrypted), plain); + }); + }); + + group.bench_function("Decrypt a public-key encrypted message", |b| { + let plain = generate_plaintext(); + let key_pair = create_dummy_keypair("alice@example.org").unwrap(); + let secrets = generate_secrets(); + let encrypted = tokio::runtime::Runtime::new().unwrap().block_on(async { + pk_encrypt( + plain.clone(), + vec![black_box(key_pair.public.clone())], + Some(key_pair.secret.clone()), + true, + ) + .await + .unwrap() + }); + + b.iter(|| { + let mut msg = decrypt( + encrypted.clone().into_bytes(), + std::slice::from_ref(&key_pair.secret), + black_box(&secrets), + ) + .unwrap(); + let decrypted = msg.as_data_vec().unwrap(); + + assert_eq!(black_box(decrypted), plain); + }); + }); + + // =========================================================================================== + // Benchmarks for the whole parsing pipeline, incl. decryption (but excl. receive_imf()) + // =========================================================================================== + + let rt = tokio::runtime::Runtime::new().unwrap(); + let mut secrets = generate_secrets(); + + // "secret" is the shared secret that was used to encrypt text_symmetrically_encrypted.eml. + // Put it into the middle of our secrets: + secrets[NUM_SECRETS / 2] = "secret".to_string(); + + let context = rt.block_on(async { + let context = create_context().await; + for (i, secret) in secrets.iter().enumerate() { + save_broadcast_shared_secret(&context, ChatId::new(10 + i as u32), secret) + .await + .unwrap(); + } + context + }); + + group.bench_function("Decrypt and parse a symmetrically encrypted message", |b| { + b.to_async(&rt).iter(|| { + let ctx = context.clone(); + async move { + let text = parse_and_get_text( + &ctx, + include_bytes!("../test-data/message/text_symmetrically_encrypted.eml"), + ) + .await + .unwrap(); + assert_eq!(text, "Symmetrically encrypted message"); + } + }); + }); + + group.bench_function("Decrypt and parse a public-key encrypted message", |b| { + b.to_async(&rt).iter(|| { + let ctx = context.clone(); + async move { + let text = parse_and_get_text( + &ctx, + include_bytes!("../test-data/message/text_from_alice_encrypted.eml"), + ) + .await + .unwrap(); + assert_eq!(text, "hi"); + } + }); + }); + + group.finish(); +} + +fn generate_secrets() -> Vec { + let secrets: Vec = (0..NUM_SECRETS) + .map(|_| create_broadcast_shared_secret()) + .collect(); + secrets +} + +fn generate_plaintext() -> Vec { + let mut plain: Vec = vec![0; 500]; + thread_rng().fill(&mut plain[..]); + plain +} + +criterion_group!(benches, criterion_benchmark); +criterion_main!(benches); diff --git a/deltachat-ffi/src/lot.rs b/deltachat-ffi/src/lot.rs index e77483ef7d..392a2b4bae 100644 --- a/deltachat-ffi/src/lot.rs +++ b/deltachat-ffi/src/lot.rs @@ -45,6 +45,7 @@ impl Lot { Self::Qr(qr) => match qr { Qr::AskVerifyContact { .. } => None, Qr::AskVerifyGroup { grpname, .. } => Some(Cow::Borrowed(grpname)), + Qr::AskJoinBroadcast { broadcast_name, .. } => Some(Cow::Borrowed(broadcast_name)), Qr::FprOk { .. } => None, Qr::FprMismatch { .. } => None, Qr::FprWithoutAddr { fingerprint, .. } => Some(Cow::Borrowed(fingerprint)), @@ -99,6 +100,7 @@ impl Lot { Self::Qr(qr) => match qr { Qr::AskVerifyContact { .. } => LotState::QrAskVerifyContact, Qr::AskVerifyGroup { .. } => LotState::QrAskVerifyGroup, + Qr::AskJoinBroadcast { .. } => LotState::QrAskJoinBroadcast, Qr::FprOk { .. } => LotState::QrFprOk, Qr::FprMismatch { .. } => LotState::QrFprMismatch, Qr::FprWithoutAddr { .. } => LotState::QrFprWithoutAddr, @@ -126,6 +128,7 @@ impl Lot { Self::Qr(qr) => match qr { Qr::AskVerifyContact { contact_id, .. } => contact_id.to_u32(), Qr::AskVerifyGroup { .. } => Default::default(), + Qr::AskJoinBroadcast { .. } => Default::default(), Qr::FprOk { contact_id } => contact_id.to_u32(), Qr::FprMismatch { contact_id } => contact_id.unwrap_or_default().to_u32(), Qr::FprWithoutAddr { .. } => Default::default(), @@ -169,6 +172,9 @@ pub enum LotState { /// text1=groupname QrAskVerifyGroup = 202, + /// text1=broadcast_name + QrAskJoinBroadcast = 204, + /// id=contact QrFprOk = 210, diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index aa68b7a1a9..8b456f1c9b 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -1010,7 +1010,7 @@ impl CommandApi { .await } - /// 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, diff --git a/deltachat-jsonrpc/src/api/types/qr.rs b/deltachat-jsonrpc/src/api/types/qr.rs index 61d8141f76..5ed6ddb969 100644 --- a/deltachat-jsonrpc/src/api/types/qr.rs +++ b/deltachat-jsonrpc/src/api/types/qr.rs @@ -34,6 +34,26 @@ pub enum QrObject { /// Authentication code. authcode: String, }, + /// Ask the user whether to join the broadcast channel. + AskJoinBroadcast { + /// Chat name. + broadcast_name: String, + /// A string of random characters, + /// uniquely identifying this broadcast channel in the database. + /// Called `grpid` for historic reasons: + /// The id of multi-user chats is always called `grpid` in the database + /// because groups were once the only multi-user chats. + grpid: String, + /// ID of the contact who owns the channel and created the QR code. + contact_id: u32, + /// Fingerprint of the contact's key as scanned from the QR code. + fingerprint: String, + + /// Invite number. + invitenumber: String, + /// Authentication code. + authcode: String, + }, /// Contact fingerprint is verified. /// /// Ask the user if they want to start chatting. @@ -207,6 +227,25 @@ impl From for QrObject { authcode, } } + Qr::AskJoinBroadcast { + broadcast_name, + grpid, + contact_id, + fingerprint, + authcode, + invitenumber, + } => { + let contact_id = contact_id.to_u32(); + let fingerprint = fingerprint.to_string(); + QrObject::AskJoinBroadcast { + broadcast_name, + grpid, + contact_id, + fingerprint, + authcode, + invitenumber, + } + } Qr::FprOk { contact_id } => { let contact_id = contact_id.to_u32(); QrObject::FprOk { contact_id } diff --git a/deltachat-rpc-client/src/deltachat_rpc_client/account.py b/deltachat-rpc-client/src/deltachat_rpc_client/account.py index e1f0667201..cfc4acff98 100644 --- a/deltachat-rpc-client/src/deltachat_rpc_client/account.py +++ b/deltachat-rpc-client/src/deltachat_rpc_client/account.py @@ -325,7 +325,7 @@ def create_group(self, name: str, protect: bool = False) -> Chat: return Chat(self, self._rpc.create_group_chat(self.id, name, protect)) def create_broadcast(self, name: str) -> Chat: - """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, diff --git a/deltachat-rpc-client/src/deltachat_rpc_client/message.py b/deltachat-rpc-client/src/deltachat_rpc_client/message.py index c7cd378ba0..a52a48b55a 100644 --- a/deltachat-rpc-client/src/deltachat_rpc_client/message.py +++ b/deltachat-rpc-client/src/deltachat_rpc_client/message.py @@ -93,6 +93,17 @@ def wait_until_delivered(self) -> None: if event.kind == EventType.MSG_DELIVERED and event.msg_id == self.id: break + def resend(self) -> None: + """Resend messages and make information available for newly added chat members. + Resending sends out the original message, however, recipients and webxdc-status may differ. + Clients that already have the original message can still ignore the resent message as + they have tracked the state by dedicated updates. + + Some messages cannot be resent, eg. info-messages, drafts, already pending messages, + or messages that are not sent by SELF. + """ + self._rpc.resend_messages(self.account.id, [self.id]) + @futuremethod def send_webxdc_realtime_advertisement(self): """Send an advertisement to join the realtime channel.""" diff --git a/deltachat-rpc-client/tests/test_securejoin.py b/deltachat-rpc-client/tests/test_securejoin.py index 31f4835207..c93acce57a 100644 --- a/deltachat-rpc-client/tests/test_securejoin.py +++ b/deltachat-rpc-client/tests/test_securejoin.py @@ -3,6 +3,7 @@ import pytest from deltachat_rpc_client import Chat, EventType, SpecialContactId +from deltachat_rpc_client.const import ChatType from deltachat_rpc_client.rpc import JsonRpcError @@ -112,6 +113,132 @@ def test_qr_securejoin(acfactory, protect): fiona.wait_for_securejoin_joiner_success() +@pytest.mark.parametrize("all_devices_online", [True, False]) +def test_qr_securejoin_broadcast(acfactory, all_devices_online): + alice, bob, fiona = acfactory.get_online_accounts(3) + + alice2 = alice.clone() + bob2 = bob.clone() + + if all_devices_online: + alice2.start_io() + bob2.start_io() + + logging.info("===================== Alice creates a broadcast =====================") + alice_chat = alice.create_broadcast("Broadcast channel for everyone!") + snapshot = alice_chat.get_basic_snapshot() + assert not snapshot.is_unpromoted # Broadcast channels are never unpromoted + + logging.info("===================== Bob joins the broadcast =====================") + + qr_code = alice_chat.get_qr_code() + bob.secure_join(qr_code) + alice.wait_for_securejoin_inviter_success() + bob.wait_for_securejoin_joiner_success() + alice_chat.send_text("Hello everyone!") + + def wait_for_group_messages(ac): + snapshot = ac.wait_for_incoming_msg().get_snapshot() + assert snapshot.text == f"Member Me added by {alice.get_config('addr')}." + + snapshot = ac.wait_for_incoming_msg().get_snapshot() + assert snapshot.text == "Hello everyone!" + + def check_account(ac, contact, inviter_side, please_wait_info_msg=False): + # Check that the chat partner is verified. + contact_snapshot = contact.get_snapshot() + assert contact_snapshot.is_verified + + chat = ac.get_chatlist()[0] + chat_msgs = chat.get_messages() + + if please_wait_info_msg: + first_msg = chat_msgs.pop(0).get_snapshot() + assert first_msg.text == "Establishing guaranteed end-to-end encryption, please wait…" + assert first_msg.is_info + + encrypted_msg = chat_msgs[0].get_snapshot() + assert encrypted_msg.text == "Messages are end-to-end encrypted." + assert encrypted_msg.is_info + + member_added_msg = chat_msgs[1].get_snapshot() + if inviter_side: + assert member_added_msg.text == f"Member {contact_snapshot.display_name} added." + else: + assert member_added_msg.text == f"Member Me added by {contact_snapshot.display_name}." + assert member_added_msg.is_info + + hello_msg = chat_msgs[2].get_snapshot() + assert hello_msg.text == "Hello everyone!" + assert not hello_msg.is_info + assert hello_msg.show_padlock + assert hello_msg.error is None + + assert len(chat_msgs) == 3 + + chat_snapshot = chat.get_full_snapshot() + assert chat_snapshot.is_encrypted + assert chat_snapshot.name == "Broadcast channel for everyone!" + if inviter_side: + assert chat_snapshot.chat_type == ChatType.OUT_BROADCAST + else: + assert chat_snapshot.chat_type == ChatType.IN_BROADCAST + assert chat_snapshot.can_send == inviter_side + + chat_contacts = chat_snapshot.contact_ids + assert contact.id in chat_contacts + if inviter_side: + assert len(chat_contacts) == 1 + else: + assert len(chat_contacts) == 2 + assert SpecialContactId.SELF in chat_contacts + assert chat_snapshot.self_in_group + + wait_for_group_messages(bob) + + check_account(alice, alice.create_contact(bob), inviter_side=True) + check_account(bob, bob.create_contact(alice), inviter_side=False, please_wait_info_msg=True) + + logging.info("===================== Test Alice's second device =====================") + + # Start second Alice device, if it wasn't started already. + alice2.start_io() + alice2.wait_for_securejoin_inviter_success() + + while True: + msg_id = alice2.wait_for_msgs_changed_event().msg_id + if msg_id: + snapshot = alice2.get_message_by_id(msg_id).get_snapshot() + if snapshot.text == "Hello everyone!": + break + + check_account(alice2, alice2.create_contact(bob), inviter_side=True) + + logging.info("===================== Test Bob's second device =====================") + + # Start second Bob device, if it wasn't started already. + bob2.start_io() + bob2.wait_for_securejoin_joiner_success() + wait_for_group_messages(bob2) + check_account(bob2, bob2.create_contact(alice), inviter_side=False) + + # The QR code token is synced, so alice2 must be able to handle join requests. + logging.info("===================== Fiona joins the group via alice2 =====================") + alice.stop_io() + fiona.secure_join(qr_code) + alice2.wait_for_securejoin_inviter_success() + fiona.wait_for_securejoin_joiner_success() + + snapshot = fiona.wait_for_incoming_msg().get_snapshot() + assert snapshot.text == f"Member Me added by {alice.get_config('addr')}." + + alice2.get_chatlist()[0].get_messages()[2].resend() + snapshot = fiona.wait_for_incoming_msg().get_snapshot() + assert snapshot.text == "Hello everyone!" + + check_account(fiona, fiona.create_contact(alice), inviter_side=False, please_wait_info_msg=True) + + def test_qr_securejoin_contact_request(acfactory) -> None: """Alice invites Bob to a group when Bob's chat with Alice is in a contact request mode.""" alice, bob = acfactory.get_online_accounts(2) diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index f7c144a4a4..1170576d9e 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -11,7 +11,7 @@ import pytest from deltachat_rpc_client import Contact, EventType, Message, events -from deltachat_rpc_client.const import ChatType, DownloadState, MessageState +from deltachat_rpc_client.const import DownloadState, MessageState from deltachat_rpc_client.pytestplugin import E2EE_INFO_MSGS from deltachat_rpc_client.rpc import JsonRpcError @@ -924,34 +924,103 @@ def test_delete_deltachat_folder(acfactory, direct_imap): assert "DeltaChat" in ac1_direct_imap.list_folders() -def test_broadcast(acfactory): +@pytest.mark.parametrize("all_devices_online", [True, False]) +def test_leave_broadcast(acfactory, all_devices_online): alice, bob = acfactory.get_online_accounts(2) - alice_chat = alice.create_broadcast("My great channel") - snapshot = alice_chat.get_basic_snapshot() - assert snapshot.name == "My great channel" - assert snapshot.is_unpromoted - assert snapshot.is_encrypted - assert snapshot.chat_type == ChatType.OUT_BROADCAST + bob2 = bob.clone() - alice_contact_bob = alice.create_contact(bob, "Bob") - alice_chat.add_contact(alice_contact_bob) - - alice_msg = alice_chat.send_message(text="hello").get_snapshot() - assert alice_msg.text == "hello" - assert alice_msg.show_padlock - - bob_msg = bob.wait_for_incoming_msg().get_snapshot() - assert bob_msg.text == "hello" - assert bob_msg.show_padlock - assert bob_msg.error is None - - bob_chat = bob.get_chat_by_id(bob_msg.chat_id) - bob_chat_snapshot = bob_chat.get_basic_snapshot() - assert bob_chat_snapshot.name == "My great channel" - assert not bob_chat_snapshot.is_unpromoted - assert bob_chat_snapshot.is_encrypted - assert bob_chat_snapshot.chat_type == ChatType.IN_BROADCAST - assert bob_chat_snapshot.is_contact_request - - assert not bob_chat.can_send() + if all_devices_online: + bob2.start_io() + + logging.info("===================== Alice creates a broadcast =====================") + alice_chat = alice.create_broadcast("Broadcast channel for everyone!") + + logging.info("===================== Bob joins the broadcast =====================") + qr_code = alice_chat.get_qr_code() + bob.secure_join(qr_code) + alice.wait_for_securejoin_inviter_success() + bob.wait_for_securejoin_joiner_success() + + alice_bob_contact = alice.create_contact(bob) + alice_contacts = alice_chat.get_contacts() + assert len(alice_contacts) == 1 # 1 recipient + assert alice_contacts[0].id == alice_bob_contact.id + + member_added_msg = bob.wait_for_incoming_msg() + assert member_added_msg.get_snapshot().text == f"Member Me added by {alice.get_config('addr')}." + + def get_broadcast(ac): + chat = ac.get_chatlist()[0] + assert chat.get_basic_snapshot().name == "Broadcast channel for everyone!" + return chat + + def check_account(ac, contact, inviter_side, please_wait_info_msg=False): + chat = get_broadcast(ac) + contact_snapshot = contact.get_snapshot() + chat_msgs = chat.get_messages() + + if please_wait_info_msg: + first_msg = chat_msgs.pop(0).get_snapshot() + assert first_msg.text == "Establishing guaranteed end-to-end encryption, please wait…" + assert first_msg.is_info + + encrypted_msg = chat_msgs.pop(0).get_snapshot() + assert encrypted_msg.text == "Messages are end-to-end encrypted." + assert encrypted_msg.is_info + + member_added_msg = chat_msgs.pop(0).get_snapshot() + if inviter_side: + assert member_added_msg.text == f"Member {contact_snapshot.display_name} added." + else: + assert member_added_msg.text == f"Member Me added by {contact_snapshot.display_name}." + assert member_added_msg.is_info + + if not inviter_side: + leave_msg = chat_msgs.pop(0).get_snapshot() + assert leave_msg.text == "You left." + + assert len(chat_msgs) == 0 + + chat_snapshot = chat.get_full_snapshot() + + # On Alice's side, SELF is not in the list of contact ids + # because OutBroadcast chats never contain SELF in the list. + # On Bob's side, SELF is not in the list because he left. + if inviter_side: + assert len(chat_snapshot.contact_ids) == 0 + else: + assert chat_snapshot.contact_ids == [contact.id] + + logging.info("===================== Bob leaves the broadcast =====================") + bob_chat = get_broadcast(bob) + assert bob_chat.get_full_snapshot().self_in_group + assert len(bob_chat.get_contacts()) == 2 # Alice and Bob + + bob_chat.leave() + assert not bob_chat.get_full_snapshot().self_in_group + # After Bob left, only Alice will be left in Bob's memberlist + assert len(bob_chat.get_contacts()) == 1 + + check_account(bob, bob.create_contact(alice), inviter_side=False, please_wait_info_msg=True) + + logging.info("===================== Test Alice's device =====================") + while len(alice_chat.get_contacts()) != 0: # After Bob left, there will be 0 recipients + alice.wait_for_event(EventType.CHAT_MODIFIED) + + check_account(alice, alice.create_contact(bob), inviter_side=True) + + logging.info("===================== Test Bob's second device =====================") + # Start second Bob device, if it wasn't started already. + bob2.start_io() + + member_added_msg = bob2.wait_for_incoming_msg() + assert member_added_msg.get_snapshot().text == f"Member Me added by {alice.get_config('addr')}." + + bob2_chat = get_broadcast(bob2) + + # After Bob left, only Alice will be left in Bob's memberlist + while len(bob2_chat.get_contacts()) != 1: + bob2.wait_for_event(EventType.CHAT_MODIFIED) + + check_account(bob2, bob2.create_contact(alice), inviter_side=False) diff --git a/src/chat.rs b/src/chat.rs index 7537e1c417..96f49f33a3 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -43,9 +43,9 @@ 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_shared_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}; @@ -600,11 +600,23 @@ impl ChatId { || chat.is_device_talk() || chat.is_self_talk() || (!chat.can_send(context).await? && !chat.is_contact_request()) + // For chattype InBrodacast, the info message is added when the member-added message is received + // by directly calling add_encrypted_msg() + || chat.typ == Chattype::InBroadcast || chat.blocked == Blocked::Yes { return Ok(()); } + self.add_encrypted_msg(context, timestamp_sort).await?; + Ok(()) + } + + 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, @@ -1725,8 +1737,9 @@ impl Chat { pub(crate) async fn is_self_in_chat(&self, context: &Context) -> Result { 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 + } } } @@ -2833,8 +2846,9 @@ pub async fn is_contact_in_chat( ) -> Result { // 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 @@ -2924,8 +2938,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 @@ -3763,7 +3781,7 @@ pub 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, @@ -3780,60 +3798,92 @@ pub async fn create_group_ex( /// Returns the created chat's id. pub async fn create_broadcast(context: &Context, chat_name: String) -> Result { let grpid = create_id(); - create_broadcast_ex(context, Sync, grpid, chat_name).await + let secret = create_broadcast_shared_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 broadcasts_shared_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 { - 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 { + 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.maybe_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, + shared_secret: secret, + }; self::sync(context, id, action).await.log_err(context).ok(); } Ok(chat_id) } +pub(crate) async fn load_broadcast_shared_secret( + context: &Context, + chat_id: ChatId, +) -> Result> { + context + .sql + .query_get_value( + "SELECT secret FROM broadcasts_shared_secrets WHERE chat_id=?", + (chat_id,), + ) + .await +} + +pub(crate) async fn save_broadcast_shared_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(()) +} + /// Set chat contacts in the `chats_contacts` table. pub(crate) async fn update_chat_contacts_table( context: &Context, @@ -3921,6 +3971,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( @@ -3948,8 +4022,8 @@ 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, - "{} is not a group/broadcast where one can add members", + chat.typ == Chattype::Group || (from_handshake && chat.typ == Chattype::OutBroadcast), + "{} is not a group where one can add members", chat_id ); ensure!( @@ -3957,7 +4031,6 @@ pub(crate) async fn add_contact_to_chat_ex( "invalid contact_id {} for adding to group", contact_id ); - 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." @@ -4008,21 +4081,33 @@ pub(crate) async fn add_contact_to_chat_ex( ); return Ok(false); } - 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()); msg.param .set_int(Param::ContactAddedRemoved, contact.id.to_u32() as i32); + if chat.typ == Chattype::OutBroadcast { + let secret = load_broadcast_shared_secret(context, chat_id) + .await? + .context("Failed to find broadcast shared secret")?; + msg.param.set(Param::Arg3, secret); + } send_msg(context, chat_id, &mut msg).await?; sync = Nosync; @@ -4177,7 +4262,17 @@ 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" + ); + } + + 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." @@ -4190,24 +4285,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_id, contact_id, addr).await; + let res = send_member_removal_msg( + context, + chat_id, + contact_id, + addr, + fingerprint.as_deref(), + ) + .await; if contact_id == ContactId::SELF { res?; @@ -4227,11 +4323,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_id, contact_id, &self_addr).await?; } else { bail!("Cannot remove members from non-group chats."); } @@ -4244,6 +4335,7 @@ async fn send_member_removal_msg( chat_id: ChatId, contact_id: ContactId, addr: &str, + fingerprint: Option<&str>, ) -> Result { let mut msg = Message::new(Viewtype::Text); @@ -4255,6 +4347,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::Arg2, fingerprint); msg.param .set(Param::ContactAddedRemoved, contact_id.to_u32()); @@ -5039,7 +5132,13 @@ pub(crate) enum SyncAction { SetVisibility(ChatVisibility), SetMuted(MuteDuration), /// Create broadcast channel with the given name. - CreateBroadcast(String), + CreateOutBroadcast { + chat_name: String, + shared_secret: String, + }, + /// Mark the contact with the given fingerprint as verified by self. + // TODO check if I can remove this + MarkVerified, Rename(String), /// Set chat contacts by their addresses. SetContacts(Vec), @@ -5095,6 +5194,16 @@ impl Context { SyncAction::Unblock => { return contact::set_blocked(self, Nosync, contact_id, false).await; } + SyncAction::MarkVerified => { + ContactId::scaleup_origin(self, &[contact_id], Origin::SecurejoinJoined) + .await?; + return contact::mark_contact_id_as_verified( + self, + contact_id, + Some(ContactId::SELF), + ) + .await; + } _ => (), } ChatIdBlocked::get_for_contact(self, contact_id, Blocked::Request) @@ -5102,8 +5211,8 @@ impl Context { .id } SyncId::Grpid(grpid) => { - if let SyncAction::CreateBroadcast(name) = action { - create_broadcast_ex(self, Nosync, grpid.clone(), name.clone()).await?; + let handled = self.handle_sync_create_chat(action, grpid).await?; + if handled { return Ok(()); } get_chat_id_by_grpid(self, grpid) @@ -5126,7 +5235,9 @@ 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::CreateOutBroadcast { .. } | SyncAction::MarkVerified => { + // Create action should have been handled by handle_sync_create_chat() already. + // MarkVerified action should have been handled by mark_contact_id_as_verified() already. Err(anyhow!("sync_alter_chat({id:?}, {action:?}): Bad request.")) } SyncAction::Rename(to) => rename_ex(self, Nosync, chat_id, to).await, @@ -5138,6 +5249,26 @@ impl Context { } } + async fn handle_sync_create_chat(&self, action: &SyncAction, grpid: &str) -> Result { + match action { + SyncAction::CreateOutBroadcast { + chat_name, + shared_secret, + } => { + create_out_broadcast_ex( + self, + Nosync, + grpid.to_string(), + chat_name.clone(), + shared_secret.to_string(), + ) + .await?; + Ok(true) + } + _ => Ok(false), + } + } + /// Emits the appropriate `MsgsChanged` event. Should be called if the number of unnoticed /// archived chats could decrease. In general we don't want to make an extra db query to know if /// a noticed chat is archived. Emitting events should be cheap, a false-positive `MsgsChanged` diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 611826a1b3..4fa07c9a94 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use super::*; use crate::chatlist::get_archived_cnt; use crate::constants::{DC_GCL_ARCHIVED_ONLY, DC_GCL_NO_SPECIALS}; @@ -7,6 +9,7 @@ use crate::imex::{ImexMode, has_backup, imex}; use crate::message::{MessengerMessage, delete_msgs}; use crate::mimeparser::{self, MimeMessage}; use crate::receive_imf::receive_imf; +use crate::securejoin::{get_securejoin_qr, join_securejoin}; use crate::test_utils::{ AVATAR_64x64_BYTES, AVATAR_64x64_DEDUPLICATED, E2EE_INFO_MSGS, TestContext, TestContextManager, TimeShiftFalsePositiveNote, sync, @@ -2276,7 +2279,8 @@ async fn test_only_minimal_data_are_forwarded() -> Result<()> { let group_id = create_group_chat(&bob, ProtectionStatus::Unprotected, "group2").await?; add_contact_to_chat(&bob, group_id, charlie_id).await?; let broadcast_id = create_broadcast(&bob, "Channel".to_string()).await?; - add_contact_to_chat(&bob, broadcast_id, charlie_id).await?; + let qr = get_securejoin_qr(&bob, Some(broadcast_id)).await?; + tcm.exec_securejoin_qr(&charlie, &bob, &qr).await; for chat_id in &[single_id, group_id, broadcast_id] { forward_msgs(&bob, &[orig_msg.id], *chat_id).await?; let sent_msg = bob.pop_sent_msg().await; @@ -2639,44 +2643,67 @@ async fn test_can_send_group() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_broadcast() -> Result<()> { +async fn test_broadcast_change_name() -> Result<()> { // create two context, send two messages so both know the other - let alice = TestContext::new_alice().await; - let bob = TestContext::new_bob().await; - let fiona = TestContext::new_fiona().await; + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let fiona = &tcm.fiona().await; - let chat_alice = alice.create_chat(&bob).await; - send_text_msg(&alice, chat_alice.id, "hi!".to_string()).await?; + tcm.section("Alice sends a message to Bob"); + let chat_alice = alice.create_chat(bob).await; + send_text_msg(alice, chat_alice.id, "hi!".to_string()).await?; bob.recv_msg(&alice.pop_sent_msg().await).await; - let chat_bob = bob.create_chat(&alice).await; - send_text_msg(&bob, chat_bob.id, "ho!".to_string()).await?; + tcm.section("Bob sends a message to Alice"); + let chat_bob = bob.create_chat(alice).await; + send_text_msg(bob, chat_bob.id, "ho!".to_string()).await?; let msg = alice.recv_msg(&bob.pop_sent_msg().await).await; assert!(msg.get_showpadlock()); - // test broadcast channel - let broadcast_id = create_broadcast(&alice, "Channel".to_string()).await?; - add_contact_to_chat( - &alice, - broadcast_id, - get_chat_contacts(&alice, chat_bob.id).await?.pop().unwrap(), - ) - .await?; - let fiona_contact_id = alice.add_or_lookup_contact_id(&fiona).await; - add_contact_to_chat(&alice, broadcast_id, fiona_contact_id).await?; - set_chat_name(&alice, broadcast_id, "Broadcast channel").await?; + let broadcast_id = create_broadcast(alice, "Channel".to_string()).await?; + let qr = get_securejoin_qr(alice, Some(broadcast_id)).await.unwrap(); + + tcm.section("Alice invites Bob to her channel"); + tcm.exec_securejoin_qr(bob, alice, &qr).await; + tcm.section("Alice invites Fiona to her channel"); + tcm.exec_securejoin_qr(fiona, alice, &qr).await; + { - let chat = Chat::load_from_db(&alice, broadcast_id).await?; + tcm.section("Alice changes the chat name"); + set_chat_name(alice, broadcast_id, "My great broadcast").await?; + let sent = alice.pop_sent_msg().await; + + tcm.section("Bob receives the name-change system message"); + let msg = bob.recv_msg(&sent).await; + assert_eq!(msg.subject, "Re: My great broadcast"); + let bob_chat = Chat::load_from_db(bob, msg.chat_id).await?; + assert_eq!(bob_chat.name, "My great broadcast"); + + tcm.section("Fiona receives the name-change system message"); + let msg = fiona.recv_msg(&sent).await; + assert_eq!(msg.subject, "Re: My great broadcast"); + let fiona_chat = Chat::load_from_db(fiona, msg.chat_id).await?; + assert_eq!(fiona_chat.name, "My great broadcast"); + } + + { + tcm.section("Alice changes the chat name again, but the system message is lost somehow"); + set_chat_name(alice, broadcast_id, "Broadcast channel").await?; + + let chat = Chat::load_from_db(alice, broadcast_id).await?; assert_eq!(chat.typ, Chattype::OutBroadcast); assert_eq!(chat.name, "Broadcast channel"); assert!(!chat.is_self_talk()); - send_text_msg(&alice, broadcast_id, "ola!".to_string()).await?; + tcm.section("Alice sends a text message 'ola!'"); + send_text_msg(alice, broadcast_id, "ola!".to_string()).await?; let msg = alice.get_last_msg().await; assert_eq!(msg.chat_id, chat.id); } { + tcm.section("Bob receives the 'ola!' message"); let sent_msg = alice.pop_sent_msg().await; let msg = bob.parse_msg(&sent_msg).await; assert!(msg.was_encrypted()); @@ -2689,25 +2716,23 @@ async fn test_broadcast() -> Result<()> { let msg = bob.recv_msg(&sent_msg).await; assert_eq!(msg.get_text(), "ola!"); - assert_eq!(msg.subject, "Broadcast channel"); + assert_eq!(msg.subject, "Re: Broadcast channel"); assert!(msg.get_showpadlock()); assert!(msg.get_override_sender_name().is_none()); - let chat = Chat::load_from_db(&bob, msg.chat_id).await?; + let chat = Chat::load_from_db(bob, msg.chat_id).await?; assert_eq!(chat.typ, Chattype::InBroadcast); assert_ne!(chat.id, chat_bob.id); assert_eq!(chat.name, "Broadcast channel"); assert!(!chat.is_self_talk()); - } - - { - // Alice changes the name: - set_chat_name(&alice, broadcast_id, "My great broadcast").await?; - let sent = alice.send_text(broadcast_id, "I changed the title!").await; - let msg = bob.recv_msg(&sent).await; - assert_eq!(msg.subject, "Re: My great broadcast"); - let bob_chat = Chat::load_from_db(&bob, msg.chat_id).await?; - assert_eq!(bob_chat.name, "My great broadcast"); + tcm.section("Fiona receives the 'ola!' message"); + let msg = fiona.recv_msg(&sent_msg).await; + assert_eq!(msg.get_text(), "ola!"); + assert!(msg.get_showpadlock()); + assert!(msg.get_override_sender_name().is_none()); + let chat = Chat::load_from_db(fiona, msg.chat_id).await?; + assert_eq!(chat.typ, Chattype::InBroadcast); + assert_eq!(chat.name, "Broadcast channel"); } Ok(()) @@ -2723,45 +2748,43 @@ async fn test_broadcast() -> Result<()> { /// `test_sync_broadcast()` tests that synchronization works via sync messages. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_broadcast_multidev() -> Result<()> { - let alices = [ - TestContext::new_alice().await, - TestContext::new_alice().await, - ]; - let bob = TestContext::new_bob().await; - let a1b_contact_id = alices[1].add_or_lookup_contact(&bob).await.id; - - let a0_broadcast_id = create_broadcast(&alices[0], "Channel".to_string()).await?; - let a0_broadcast_chat = Chat::load_from_db(&alices[0], a0_broadcast_id).await?; - set_chat_name(&alices[0], a0_broadcast_id, "Broadcast channel 42").await?; - let sent_msg = alices[0].send_text(a0_broadcast_id, "hi").await; - let msg = alices[1].recv_msg(&sent_msg).await; - let a1_broadcast_id = get_chat_id_by_grpid(&alices[1], &a0_broadcast_chat.grpid) + let mut tcm = TestContextManager::new(); + let alice0 = &tcm.alice().await; + let alice1 = &tcm.alice().await; + for a in &[alice0, alice1] { + a.set_config_bool(Config::SyncMsgs, true).await?; + } + let bob = &tcm.bob().await; + + let a0_broadcast_id = create_broadcast(alice0, "Channel".to_string()).await?; + sync(alice0, alice1).await; + let a0_broadcast_chat = Chat::load_from_db(alice0, a0_broadcast_id).await?; + set_chat_name(alice0, a0_broadcast_id, "Broadcast channel 42").await?; + let sent_msg = alice0.send_text(a0_broadcast_id, "hi").await; + let msg = alice1.recv_msg(&sent_msg).await; + let a1_broadcast_id = get_chat_id_by_grpid(alice1, &a0_broadcast_chat.grpid) .await? .unwrap() .0; assert_eq!(msg.chat_id, a1_broadcast_id); - let a1_broadcast_chat = Chat::load_from_db(&alices[1], a1_broadcast_id).await?; + let a1_broadcast_chat = Chat::load_from_db(alice1, a1_broadcast_id).await?; assert_eq!(a1_broadcast_chat.get_type(), Chattype::OutBroadcast); assert_eq!(a1_broadcast_chat.get_name(), "Broadcast channel 42"); - assert!( - get_chat_contacts(&alices[1], a1_broadcast_id) - .await? - .is_empty() - ); + assert!(get_chat_contacts(alice1, a1_broadcast_id).await?.is_empty()); + + let qr = get_securejoin_qr(alice1, Some(a1_broadcast_id)) + .await + .unwrap(); + tcm.exec_securejoin_qr(bob, alice1, &qr).await; - add_contact_to_chat(&alices[1], a1_broadcast_id, a1b_contact_id).await?; - set_chat_name(&alices[1], a1_broadcast_id, "Broadcast channel 43").await?; - let sent_msg = alices[1].send_text(a1_broadcast_id, "hi").await; - let msg = alices[0].recv_msg(&sent_msg).await; + set_chat_name(alice1, a1_broadcast_id, "Broadcast channel 43").await?; + let sent_msg = alice1.send_text(a1_broadcast_id, "hi").await; + let msg = alice0.recv_msg(&sent_msg).await; assert_eq!(msg.chat_id, a0_broadcast_id); - let a0_broadcast_chat = Chat::load_from_db(&alices[0], a0_broadcast_id).await?; + let a0_broadcast_chat = Chat::load_from_db(alice0, a0_broadcast_id).await?; assert_eq!(a0_broadcast_chat.get_type(), Chattype::OutBroadcast); assert_eq!(a0_broadcast_chat.get_name(), "Broadcast channel 42"); - assert!( - get_chat_contacts(&alices[0], a0_broadcast_id) - .await? - .is_empty() - ); + assert!(get_chat_contacts(alice0, a0_broadcast_id).await?.is_empty()); Ok(()) } @@ -2779,7 +2802,6 @@ async fn test_broadcasts_name_and_avatar() -> Result<()> { let alice = &tcm.alice().await; alice.set_config(Config::Displayname, Some("Alice")).await?; let bob = &tcm.bob().await; - let alice_bob_contact_id = alice.add_or_lookup_contact_id(bob).await; tcm.section("Create a broadcast channel"); let alice_chat_id = create_broadcast(alice, "My Channel".to_string()).await?; @@ -2787,14 +2809,15 @@ async fn test_broadcasts_name_and_avatar() -> Result<()> { assert_eq!(alice_chat.typ, Chattype::OutBroadcast); let alice_chat = Chat::load_from_db(alice, alice_chat_id).await?; - assert_eq!(alice_chat.is_promoted(), false); + assert_eq!(alice_chat.is_promoted(), true); // Broadcast channels are never unpromoted let sent = alice.send_text(alice_chat_id, "Hi nobody").await; let alice_chat = Chat::load_from_db(alice, alice_chat_id).await?; assert_eq!(alice_chat.is_promoted(), true); assert_eq!(sent.recipients, "alice@example.org"); tcm.section("Add a contact to the chat and send a message"); - add_contact_to_chat(alice, alice_chat_id, alice_bob_contact_id).await?; + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap(); + tcm.exec_securejoin_qr(bob, alice, &qr).await; let sent = alice.send_text(alice_chat_id, "Hi somebody").await; assert_eq!(sent.recipients, "bob@example.net alice@example.org"); @@ -2852,6 +2875,67 @@ async fn test_broadcasts_name_and_avatar() -> Result<()> { Ok(()) } +/// Tests that directly after broadcast-securejoin, +/// the brodacast is shown correctly on both devices. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_broadcast_joining_golden() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + alice.set_config(Config::Displayname, Some("Alice")).await?; + + tcm.section("Create a broadcast channel with an avatar"); + let alice_chat_id = create_broadcast(alice, "My Channel".to_string()).await?; + let file = alice.get_blobdir().join("avatar.png"); + tokio::fs::write(&file, AVATAR_64x64_BYTES).await?; + set_chat_profile_image(alice, alice_chat_id, file.to_str().unwrap()).await?; + // Because broadcasts are always 'promoted', + // set_chat_profile_image() sends out a message, + // which we need to pop: + alice.pop_sent_msg().await; + + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap(); + let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; + + alice + .golden_test_chat(alice_chat_id, "test_broadcast_joining_golden_alice") + .await; + bob.golden_test_chat(bob_chat_id, "test_broadcast_joining_golden_bob") + .await; + + let alice_bob_contact = alice.add_or_lookup_contact_no_key(bob).await; + let direct_chat = ChatIdBlocked::lookup_by_contact(alice, alice_bob_contact.id) + .await? + .unwrap(); + // The 1:1 chat with Bob should not be visible to the user: + assert_eq!(direct_chat.blocked, Blocked::Yes); + alice + .golden_test_chat(direct_chat.id, "test_broadcast_joining_golden_alice_direct") + .await; + + assert_eq!( + alice_bob_contact + .get_verifier_id(alice) + .await? + .unwrap() + .unwrap(), + ContactId::SELF + ); + + let bob_alice_contact = bob.add_or_lookup_contact_no_key(alice).await; + assert_eq!( + bob_alice_contact + .get_verifier_id(bob) + .await? + .unwrap() + .unwrap(), + ContactId::SELF + ); + + Ok(()) +} + /// - Create a broadcast channel /// - Block it /// - Check that the broadcast channel appears in the list of blocked contacts @@ -2863,11 +2947,13 @@ async fn test_block_broadcast() -> Result<()> { let mut tcm = TestContextManager::new(); let alice = &tcm.alice().await; let bob = &tcm.bob().await; - let alice_bob_contact_id = alice.add_or_lookup_contact_id(bob).await; tcm.section("Create a broadcast channel with Bob, and send a message"); let alice_chat_id = create_broadcast(alice, "My Channel".to_string()).await?; - add_contact_to_chat(alice, alice_chat_id, alice_bob_contact_id).await?; + + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap(); + tcm.exec_securejoin_qr(bob, alice, &qr).await; + let sent = alice.send_text(alice_chat_id, "Hi somebody").await; let rcvd = bob.recv_msg(&sent).await; @@ -2875,7 +2961,7 @@ async fn test_block_broadcast() -> Result<()> { assert_eq!(chats.len(), 1); assert_eq!(chats.get_chat_id(0)?, rcvd.chat_id); - assert_eq!(rcvd.chat_blocked, Blocked::Request); + assert_eq!(rcvd.chat_blocked, Blocked::Not); let blocked = Contact::get_all_blocked(bob).await.unwrap(); assert_eq!(blocked.len(), 0); @@ -2929,11 +3015,13 @@ async fn test_broadcast_channel_protected_listid() -> Result<()> { let mut tcm = TestContextManager::new(); let alice = &tcm.alice().await; let bob = &tcm.bob().await; - let alice_bob_contact_id = alice.add_or_lookup_contact_id(bob).await; tcm.section("Create a broadcast channel with Bob, and send a message"); let alice_chat_id = create_broadcast(alice, "My Channel".to_string()).await?; - add_contact_to_chat(alice, alice_chat_id, alice_bob_contact_id).await?; + + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap(); + tcm.exec_securejoin_qr(bob, alice, &qr).await; + let mut sent = alice.send_text(alice_chat_id, "Hi somebody").await; assert!(!sent.payload.contains("List-ID")); @@ -2978,8 +3066,8 @@ async fn test_leave_broadcast() -> Result<()> { tcm.section("Alice creates broadcast channel with Bob."); let alice_chat_id = create_broadcast(alice, "foo".to_string()).await?; - let bob_contact = alice.add_or_lookup_contact(bob).await.id; - add_contact_to_chat(alice, alice_chat_id, bob_contact).await?; + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap(); + tcm.exec_securejoin_qr(bob, alice, &qr).await; tcm.section("Alice sends first message to broadcast."); let sent_msg = alice.send_text(alice_chat_id, "Hello!").await; @@ -3002,7 +3090,12 @@ async fn test_leave_broadcast() -> Result<()> { let leave_msg = bob.pop_sent_msg().await; alice.recv_msg_trash(&leave_msg).await; - assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 0); + assert!(get_chat_contacts(alice, alice_chat_id).await?.is_empty()); + assert!( + get_past_chat_contacts(alice, alice_chat_id) + .await? + .is_empty() + ); alice.emit_event(EventType::Test); alice @@ -3033,11 +3126,35 @@ async fn test_leave_broadcast_multidevice() -> Result<()> { let alice = &tcm.alice().await; let bob0 = &tcm.bob().await; let bob1 = &tcm.bob().await; + for b in [bob0, bob1] { + b.set_config_bool(Config::SyncMsgs, true).await?; + } tcm.section("Alice creates broadcast channel with Bob."); let alice_chat_id = create_broadcast(alice, "foo".to_string()).await?; - let bob_contact = alice.add_or_lookup_contact(bob0).await.id; - add_contact_to_chat(alice, alice_chat_id, bob_contact).await?; + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap(); + join_securejoin(bob0, &qr).await.unwrap(); + let request = bob0.pop_sent_msg().await; + + // Bob must send the message only to Alice, not to Self, + // because otherwise, his second device would show a device message + // "⚠️ It seems you are using Delta Chat on multiple devices that cannot decrypt each other's outgoing messages. + // To fix this, on the older device use \"Settings / Add Second Device\" and follow the instructions." + assert_eq!(request.recipients, "alice@example.org"); + + alice.recv_msg_trash(&request).await; + let answer = alice.pop_sent_msg().await; + bob0.recv_msg(&answer).await; + + // Sync Bob's verification of Alice: + sync(bob0, bob1).await; + bob1.recv_msg(&answer).await; + + // The 1:1 chat should not be visible to the user on any of the devices. + // The contact should be marked as verified. + check_direct_chat_is_hidden_and_contact_is_verified(alice, bob0).await; + check_direct_chat_is_hidden_and_contact_is_verified(bob0, alice).await; + check_direct_chat_is_hidden_and_contact_is_verified(bob1, alice).await; tcm.section("Alice sends first message to broadcast."); let sent_msg = alice.send_text(alice_chat_id, "Hello!").await; @@ -3069,6 +3186,180 @@ async fn test_leave_broadcast_multidevice() -> Result<()> { Ok(()) } +async fn check_direct_chat_is_hidden_and_contact_is_verified( + t: &TestContext, + contact: &TestContext, +) { + let contact = t.add_or_lookup_contact_no_key(contact).await; + if let Some(direct_chat) = ChatIdBlocked::lookup_by_contact(t, contact.id) + .await + .unwrap() + { + assert_eq!(direct_chat.blocked, Blocked::Yes); + } + assert!(contact.is_verified(t).await.unwrap()); +} + +/// Test that only the owner of the broadcast channel +/// can send messages into the chat. +/// +/// To do so, we change Alice's public key on Bob's side, +/// so that she is supposed to appear as a new contact when we receive another message, +/// and check that she can't write into the channel. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_only_broadcast_owner_can_send_1() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + tcm.section("Alice creates broadcast channel and creates a QR code."); + let alice_chat_id = create_broadcast(alice, "foo".to_string()).await?; + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap(); + + tcm.section("Bob now scans the QR code sends the request message"); + let bob_broadcast_id = join_securejoin(bob, &qr).await.unwrap(); + let request = bob.pop_sent_msg().await; + alice.recv_msg_trash(&request).await; + + tcm.section("Alice answers"); + let answer = alice.pop_sent_msg().await; + + tcm.section("Change Alice's fingerprint for Bob, so that she is a different contact from Bob's point of view"); + let bob_alice_id = bob.add_or_lookup_contact_no_key(alice).await.id; + bob.sql + .execute( + "UPDATE contacts + SET fingerprint='1234567890123456789012345678901234567890' + WHERE id=?", + (bob_alice_id,), + ) + .await?; + + tcm.section("Bob receives an answer, but it ignored because of a fingerprint mismatch"); + bob.recv_msg(&answer).await; + assert!( + load_broadcast_shared_secret(bob, bob_broadcast_id) + .await? + .is_none() + ); + + Ok(()) +} + +/// Same as the previous test, but Alice's fingerprint is changed later, +/// so that we can check that until the fingerprint change, everything works fine. +/// +/// Also, this changes Alice's fingerprint in Alice's database, rather than Bob's database, +/// in order to test for the same thing in different ways. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_only_broadcast_owner_can_send_2() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &mut tcm.bob().await; + + tcm.section("Alice creates broadcast channel and creates a QR code."); + let alice_broadcast_id = create_broadcast(alice, "foo".to_string()).await?; + let qr = get_securejoin_qr(alice, Some(alice_broadcast_id)) + .await + .unwrap(); + + tcm.section("Bob now scans the QR code"); + let bob_broadcast_id = join_securejoin(bob, &qr).await.unwrap(); + let request = bob.pop_sent_msg().await; + alice.recv_msg_trash(&request).await; + let answer = alice.pop_sent_msg().await; + + tcm.section("Bob receives an answer, and processes it"); + let rcvd = bob.recv_msg(&answer).await; + assert!( + load_broadcast_shared_secret(bob, bob_broadcast_id) + .await? + .is_some() + ); + assert_eq!(rcvd.param.get_cmd(), SystemMessage::MemberAddedToGroup); + + tcm.section("Alice sends a message, which still arrives fine"); + let sent = alice.send_text(alice_broadcast_id, "Hi").await; + let rcvd = bob.recv_msg(&sent).await; + assert_eq!(rcvd.text, "Hi"); + + tcm.section("Now, Alice's fingerprint changes"); + + alice.sql.execute("DELETE FROM keypairs", ()).await?; + alice + .sql + .execute("DELETE FROM config WHERE keyname='key_id'", ()) + .await?; + // Invalidate cached self fingerprint: + Arc::get_mut(&mut bob.ctx.inner) + .unwrap() + .self_fingerprint + .take(); + + tcm.section("Alice sends a message, which doesn't arrive fine"); + let sent = alice.send_text(alice_broadcast_id, "Hi").await; + let rcvd = bob.recv_msg(&sent).await; + assert_eq!( + rcvd.text, + "[Error: This message was not sent by the channel owner]" + ); + assert_eq!( + rcvd.error.unwrap(), + r#"Error: This message was not sent by the channel owner: +"Hi""# + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_encrypt_decrypt_broadcast() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let bob_without_secret = &tcm.bob().await; + + let secret = "secret"; + let grpid = "grpid"; + + let alice_bob_contact_id = alice.add_or_lookup_contact_id(bob).await; + + tcm.section("Create a broadcast channel with Bob, and send a message"); + let alice_chat_id = create_out_broadcast_ex( + alice, + Sync, + "My Channel".to_string(), + grpid.to_string(), + secret.to_string(), + ) + .await?; + add_to_chat_contacts_table(alice, time(), alice_chat_id, &[alice_bob_contact_id]).await?; + + let bob_chat_id = ChatId::create_multiuser_record( + bob, + Chattype::InBroadcast, + grpid, + "My Channel", + Blocked::Not, + ProtectionStatus::Unprotected, + None, + time(), + ) + .await?; + save_broadcast_shared_secret(bob, bob_chat_id, secret).await?; + + let sent = alice + .send_text(alice_chat_id, "Symmetrically encrypted message") + .await; + let rcvd = bob.recv_msg(&sent).await; + assert_eq!(rcvd.text, "Symmetrically encrypted message"); + + tcm.section("If Bob doesn't know the secret, he can't decrypt the message"); + bob_without_secret.recv_msg_trash(&sent).await; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_create_for_contact_with_blocked() -> Result<()> { let t = TestContext::new().await; @@ -3783,55 +4074,86 @@ async fn test_sync_muted() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_sync_broadcast() -> Result<()> { let mut tcm = TestContextManager::new(); - let alice0 = &tcm.alice().await; let alice1 = &tcm.alice().await; - for a in [alice0, alice1] { + let alice2 = &tcm.alice().await; + for a in [alice1, alice2] { a.set_config_bool(Config::SyncMsgs, true).await?; } let bob = &tcm.bob().await; - let a0b_contact_id = alice0.add_or_lookup_contact(bob).await.id; + let a1b_contact_id = alice1.add_or_lookup_contact(bob).await.id; - let a0_broadcast_id = create_broadcast(alice0, "Channel".to_string()).await?; - sync(alice0, alice1).await; - let a0_broadcast_chat = Chat::load_from_db(alice0, a0_broadcast_id).await?; - let a1_broadcast_id = get_chat_id_by_grpid(alice1, &a0_broadcast_chat.grpid) + tcm.section("Alice creates a channel on her first device"); + let a1_broadcast_id = create_broadcast(alice1, "Channel".to_string()).await?; + + tcm.section("The channel syncs to her second device"); + sync(alice1, alice2).await; + let a1_broadcast_chat = Chat::load_from_db(alice1, a1_broadcast_id).await?; + let a2_broadcast_id = get_chat_id_by_grpid(alice2, &a1_broadcast_chat.grpid) .await? .unwrap() .0; - let a1_broadcast_chat = Chat::load_from_db(alice1, a1_broadcast_id).await?; - assert_eq!(a1_broadcast_chat.get_type(), Chattype::OutBroadcast); - assert_eq!(a1_broadcast_chat.get_name(), a0_broadcast_chat.get_name()); - assert!(get_chat_contacts(alice1, a1_broadcast_id).await?.is_empty()); - add_contact_to_chat(alice0, a0_broadcast_id, a0b_contact_id).await?; - sync(alice0, alice1).await; + let a2_broadcast_chat = Chat::load_from_db(alice2, a2_broadcast_id).await?; + assert_eq!(a2_broadcast_chat.get_type(), Chattype::OutBroadcast); + assert_eq!(a2_broadcast_chat.get_name(), a1_broadcast_chat.get_name()); + assert!(get_chat_contacts(alice2, a2_broadcast_id).await?.is_empty()); - // This also imports Bob's key from the vCard. - // Otherwise it is possible that second device - // does not have Bob's key as only the fingerprint - // is transferred in the sync message. - let a1b_contact_id = alice1.add_or_lookup_contact(bob).await.id; + tcm.section("Bob scans Alice's QR code, both of Alice's devices answer"); + let qr = get_securejoin_qr(alice1, Some(a1_broadcast_id)) + .await + .unwrap(); + sync(alice1, alice2).await; // Sync QR code + let bob_broadcast_id = tcm + .exec_securejoin_qr_multi_device(bob, &[alice1, alice2], &qr) + .await; + + let a2b_contact_id = alice2.add_or_lookup_contact_no_key(bob).await.id; assert_eq!( - get_chat_contacts(alice1, a1_broadcast_id).await?, - vec![a1b_contact_id] + get_chat_contacts(alice2, a2_broadcast_id).await?, + vec![a2b_contact_id] ); - let sent_msg = alice1.send_text(a1_broadcast_id, "hi").await; + + tcm.section("Alice's second device sends a message to the channel"); + let sent_msg = alice2.send_text(a2_broadcast_id, "hi").await; let msg = bob.recv_msg(&sent_msg).await; let chat = Chat::load_from_db(bob, msg.chat_id).await?; assert_eq!(chat.get_type(), Chattype::InBroadcast); - let msg = alice0.recv_msg(&sent_msg).await; - assert_eq!(msg.chat_id, a0_broadcast_id); - remove_contact_from_chat(alice0, a0_broadcast_id, a0b_contact_id).await?; - sync(alice0, alice1).await; - assert!(get_chat_contacts(alice1, a1_broadcast_id).await?.is_empty()); + let msg = alice1.recv_msg(&sent_msg).await; + assert_eq!(msg.chat_id, a1_broadcast_id); + + tcm.section("Alice's first device removes Bob"); + remove_contact_from_chat(alice1, a1_broadcast_id, a1b_contact_id).await?; + let sent = alice1.pop_sent_msg().await; + + tcm.section("Alice's second device receives the removal-message"); + alice2.recv_msg(&sent).await; + assert!(get_chat_contacts(alice2, a2_broadcast_id).await?.is_empty()); assert!( - get_past_chat_contacts(alice1, a1_broadcast_id) + get_past_chat_contacts(alice2, a2_broadcast_id) .await? .is_empty() ); - a0_broadcast_id.delete(alice0).await?; - sync(alice0, alice1).await; - alice1.assert_no_chat(a1_broadcast_id).await; + tcm.section("Bob receives the removal-message"); + bob.recv_msg(&sent).await; + let bob_chat = Chat::load_from_db(bob, bob_broadcast_id).await?; + assert!(!bob_chat.is_self_in_chat(bob).await?); + + bob.golden_test_chat(bob_broadcast_id, "test_sync_broadcast_bob") + .await; + + // Alice1 and Alice2 are supposed to show the chat in the same way: + alice1 + .golden_test_chat(a1_broadcast_id, "test_sync_broadcast_alice1") + .await; + alice2 + .golden_test_chat(a2_broadcast_id, "test_sync_broadcast_alice2") + .await; + + tcm.section("Alice's first device deletes the chat"); + a1_broadcast_id.delete(alice1).await?; + sync(alice1, alice2).await; + alice2.assert_no_chat(a2_broadcast_id).await; + Ok(()) } @@ -3845,12 +4167,25 @@ async fn test_sync_name() -> Result<()> { let a0_broadcast_id = create_broadcast(alice0, "Channel".to_string()).await?; sync(alice0, alice1).await; let a0_broadcast_chat = Chat::load_from_db(alice0, a0_broadcast_id).await?; + set_chat_name(alice0, a0_broadcast_id, "Broadcast channel 42").await?; - sync(alice0, alice1).await; + //sync(alice0, alice1).await; // crash + + let sent = alice0.pop_sent_msg().await; + let rcvd = alice1.recv_msg(&sent).await; + assert_eq!(rcvd.from_id, ContactId::SELF); + assert_eq!(rcvd.to_id, ContactId::SELF); + assert_eq!( + rcvd.text, + "You changed group name from \"Channel\" to \"Broadcast channel 42\"." + ); + assert_eq!(rcvd.param.get_cmd(), SystemMessage::GroupNameChanged); let a1_broadcast_id = get_chat_id_by_grpid(alice1, &a0_broadcast_chat.grpid) .await? .unwrap() .0; + assert_eq!(rcvd.chat_id, a1_broadcast_id); + let a1_broadcast_chat = Chat::load_from_db(alice1, a1_broadcast_id).await?; assert_eq!(a1_broadcast_chat.get_type(), Chattype::OutBroadcast); assert_eq!(a1_broadcast_chat.get_name(), "Broadcast channel 42"); diff --git a/src/decrypt.rs b/src/decrypt.rs index 8c3b9de150..4012a8f41e 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -10,17 +10,19 @@ use crate::pgp; /// Tries to decrypt a message, but only if it is structured as an Autocrypt message. /// -/// If successful and the message is encrypted, returns decrypted body. +/// If successful and the message was encrypted, +/// returns the decrypted and decompressed message. pub fn try_decrypt<'a>( mail: &'a ParsedMail<'a>, private_keyring: &'a [SignedSecretKey], + shared_secrets: &[String], ) -> Result>> { let Some(encrypted_data_part) = get_encrypted_mime(mail) else { return Ok(None); }; let data = encrypted_data_part.get_body_raw()?; - let msg = pgp::pk_decrypt(data, private_keyring)?; + let msg = pgp::decrypt(data, private_keyring, shared_secrets)?; Ok(Some(msg)) } diff --git a/src/e2ee.rs b/src/e2ee.rs index 1eabbb4e1c..60b27b95c7 100644 --- a/src/e2ee.rs +++ b/src/e2ee.rs @@ -58,6 +58,27 @@ impl EncryptHelper { Ok(ctext) } + /// Symmetrically encrypt the message. This is used for broadcast channels. + /// `shared secret` is the secret that will be used for symmetric encryption. + pub async fn encrypt_symmetrically( + self, + context: &Context, + shared_secret: &str, + mail_to_encrypt: MimePart<'static>, + compress: bool, + ) -> Result { + let sign_key = load_self_secret_key(context).await?; + + let mut raw_message = Vec::new(); + let cursor = Cursor::new(&mut raw_message); + mail_to_encrypt.clone().write_part(cursor).ok(); + + let ctext = + pgp::symm_encrypt_message(raw_message, shared_secret, sign_key, compress).await?; + + Ok(ctext) + } + /// Signs the passed-in `mail` using the private key from `context`. /// Returns the payload and the signature. pub async fn sign(self, context: &Context, mail: &MimePart<'static>) -> Result { diff --git a/src/headerdef.rs b/src/headerdef.rs index 1669c91c12..055b57b073 100644 --- a/src/headerdef.rs +++ b/src/headerdef.rs @@ -63,6 +63,7 @@ pub enum HeaderDef { ChatUserAvatar, ChatVoiceMessage, ChatGroupMemberRemoved, + ChatGroupMemberRemovedFpr, ChatGroupMemberAdded, ChatContent, @@ -94,6 +95,11 @@ pub enum HeaderDef { /// This message obsoletes the text of the message defined here by rfc724_mid. ChatEdit, + /// The secret shared amongst all recipients of this broadcast channel, + /// used to encrypt and decrypt messages. + /// This secret is sent to a new member in the member-addition message. + ChatBroadcastSecret, + /// [Autocrypt](https://autocrypt.org/) header. Autocrypt, AutocryptGossip, diff --git a/src/imex/key_transfer.rs b/src/imex/key_transfer.rs index 6d1f9f418e..c995094fa0 100644 --- a/src/imex/key_transfer.rs +++ b/src/imex/key_transfer.rs @@ -95,7 +95,7 @@ pub async fn render_setup_file(context: &Context, passphrase: &str) -> Result Result { + key::SignedSecretKey::from_asc(data) +} + +pub async fn store_self_keypair(context: &Context, keypair: &KeyPair) -> Result<()> { + key::store_self_keypair(context, keypair).await +} + +pub async fn parse_and_get_text(context: &Context, imf_raw: &[u8]) -> Result { + let mime_parser = MimeMessage::from_bytes(context, imf_raw, None).await?; + Ok(mime_parser.parts.into_iter().next().unwrap().msg) +} + +pub async fn save_broadcast_shared_secret( + context: &Context, + chat_id: ChatId, + secret: &str, +) -> Result<()> { + crate::chat::save_broadcast_shared_secret(context, chat_id, secret).await +} + +pub fn create_dummy_keypair(addr: &str) -> Result { + pgp::create_keypair(EmailAddress::new(addr)?) +} + +pub fn create_broadcast_shared_secret() -> String { + crate::tools::create_broadcast_shared_secret() +} diff --git a/src/lib.rs b/src/lib.rs index 6a33b23c92..19e3990e99 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -75,7 +75,10 @@ mod mimefactory; pub mod mimeparser; pub mod oauth2; mod param; +#[cfg(not(feature = "internals"))] mod pgp; +#[cfg(feature = "internals")] +pub mod pgp; pub mod provider; pub mod qr; pub mod qr_code_generator; @@ -109,6 +112,9 @@ pub mod accounts; pub mod peer_channels; pub mod reaction; +#[cfg(feature = "internals")] +pub mod internals_for_benchmarks; + /// If set IMAP/incoming and SMTP/outgoing MIME messages will be printed. pub const DCC_MIME_DEBUG: &str = "DCC_MIME_DEBUG"; diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 4cee76a496..84c5e092e2 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -3,7 +3,7 @@ use std::collections::{BTreeSet, HashSet}; use std::io::Cursor; -use anyhow::{Context as _, Result, bail, ensure}; +use anyhow::{Context as _, Result, bail}; use base64::Engine as _; use data_encoding::BASE32_NOPAD; use deltachat_contact_tools::sanitize_bidi_characters; @@ -15,7 +15,7 @@ use tokio::fs; use crate::aheader::{Aheader, EncryptPreference}; use crate::blob::BlobObject; -use crate::chat::{self, Chat}; +use crate::chat::{self, Chat, load_broadcast_shared_secret}; use crate::config::Config; use crate::constants::ASM_SUBJECT; use crate::constants::{Chattype, DC_FROM_HANDSHAKE}; @@ -182,7 +182,7 @@ impl MimeFactory { let now = time(); let chat = Chat::load_from_db(context, msg.chat_id).await?; let attach_profile_data = Self::should_attach_profile_data(&msg); - let undisclosed_recipients = chat.typ == Chattype::OutBroadcast; + let undisclosed_recipients = should_hide_recipients(&msg, &chat); let from_addr = context.get_primary_self_addr().await?; let config_displayname = context @@ -329,7 +329,7 @@ impl MimeFactory { if let Some(public_key) = public_key_opt { keys.push((addr.clone(), public_key)) - } else if id != ContactId::SELF { + } else if id != ContactId::SELF && !should_encrypt_symmetrically(&msg, &chat) { missing_key_addresses.insert(addr.clone()); if is_encrypted { warn!(context, "Missing key for {addr}"); @@ -350,7 +350,7 @@ impl MimeFactory { if let Some(public_key) = public_key_opt { keys.push((addr.clone(), public_key)) - } else if id != ContactId::SELF { + } else if id != ContactId::SELF && !should_encrypt_symmetrically(&msg, &chat) { missing_key_addresses.insert(addr.clone()); if is_encrypted { warn!(context, "Missing key for {addr}"); @@ -415,8 +415,24 @@ impl MimeFactory { req_mdn = true; } + // If undisclosed_recipients, and this is a member-added/removed message, + // only send to the added/removed member + if undisclosed_recipients + && matches!( + msg.param.get_cmd(), + SystemMessage::MemberRemovedFromGroup | SystemMessage::MemberAddedToGroup + ) + { + if let Some(member) = msg.param.get(Param::Arg) { + recipients.retain(|addr| addr == member); + } + } + encryption_keys = if !is_encrypted { None + } else if should_encrypt_symmetrically(&msg, &chat) { + // Encrypt, but only symmetrically, not with the public keys. + Some(Vec::new()) } else { if keys.is_empty() && !recipients.is_empty() { bail!( @@ -563,7 +579,13 @@ impl MimeFactory { // messages are auto-sent unlike usual unencrypted messages. step == "vg-request-with-auth" || step == "vc-request-with-auth" + || step == "vb-request-with-auth" + // Note that for "vg-member-added" and "vb-member-added", + // get_cmd() returns `MemberAddedToGroup` rather than `SecurejoinMessage`, + // so, it wouldn't actually be necessary to have them in the list here. + // Still, they are here for completeness. || step == "vg-member-added" + || step == "vb-member-added" || step == "vc-contact-confirm" } } @@ -806,7 +828,7 @@ impl MimeFactory { } else if let Loaded::Message { msg, .. } = &self.loaded { if msg.param.get_cmd() == SystemMessage::SecurejoinMessage { let step = msg.param.get(Param::Arg).unwrap_or_default(); - if step != "vg-request" && step != "vc-request" { + if step != "vg-request" && step != "vc-request" && step != "vb-request" { headers.push(( "Auto-Submitted", mail_builder::headers::raw::Raw::new("auto-replied".to_string()).into(), @@ -815,7 +837,7 @@ impl MimeFactory { } } - if let Loaded::Message { chat, .. } = &self.loaded { + if let Loaded::Message { msg, chat } = &self.loaded { if chat.typ == Chattype::OutBroadcast || chat.typ == Chattype::InBroadcast { headers.push(( "List-ID", @@ -825,6 +847,15 @@ impl MimeFactory { )) .into(), )); + + if msg.param.get_cmd() == SystemMessage::MemberAddedToGroup { + if let Some(secret) = msg.param.get(Param::Arg3) { + headers.push(( + "Chat-Broadcast-Secret", + mail_builder::headers::text::Text::new(secret.to_string()).into(), + )); + } + } } } @@ -1005,6 +1036,15 @@ impl MimeFactory { } else { unprotected_headers.push(header.clone()); } + } else if header_name == "chat-broadcast-secret" { + if is_encrypted { + protected_headers.push(header.clone()); + } else { + warn!( + context, + "Message is unnecrypted, not including broadcast secret" + ); + } } else if is_encrypted { protected_headers.push(header.clone()); @@ -1060,7 +1100,7 @@ impl MimeFactory { match &self.loaded { Loaded::Message { chat, msg } => { - if chat.typ != Chattype::OutBroadcast { + if !should_hide_recipients(msg, chat) { for (addr, key) in &encryption_keys { let fingerprint = key.dc_fingerprint().hex(); let cmd = msg.param.get_cmd(); @@ -1144,18 +1184,45 @@ impl MimeFactory { Loaded::Mdn { .. } => true, }; - // Encrypt to self unconditionally, - // even for a single-device setup. - let mut encryption_keyring = vec![encrypt_helper.public_key.clone()]; - encryption_keyring.extend(encryption_keys.iter().map(|(_addr, key)| (*key).clone())); + let shared_secret: Option = match &self.loaded { + Loaded::Message { chat, msg } + if should_encrypt_with_broadcast_secret(msg, chat) => + { + // If there is no shared secret yet + // (because this is an old broadcast channel, + // created before we had symmetric encryption), + // we just encrypt asymmetrically. + // Symmetric encryption exists since 2025-10; + // some time after that, we can think about requiring everyone + // to switch to symmetrically-encrypted broadcast lists. + load_broadcast_shared_secret(context, chat.id).await? + } + _ => None, + }; + + let encrypted = if let Some(shared_secret) = shared_secret { + info!(context, "Encrypting symmetrically."); + encrypt_helper + .encrypt_symmetrically(context, &shared_secret, message, compress) + .await? + } else { + // Asymmetric encryption + + // Encrypt to self unconditionally, + // even for a single-device setup. + let mut encryption_keyring = vec![encrypt_helper.public_key.clone()]; + encryption_keyring + .extend(encryption_keys.iter().map(|(_addr, key)| (*key).clone())); + + encrypt_helper + .encrypt(context, encryption_keyring, message, compress) + .await? + }; // XXX: additional newline is needed // to pass filtermail at - // - let encrypted = encrypt_helper - .encrypt(context, encryption_keyring, message, compress) - .await? - + "\n"; + // : + let encrypted = encrypted + "\n"; // Set the appropriate Content-Type for the outer message MimePart::new( @@ -1364,8 +1431,8 @@ impl MimeFactory { match command { SystemMessage::MemberRemovedFromGroup => { - ensure!(chat.typ != Chattype::OutBroadcast); let email_to_remove = msg.param.get(Param::Arg).unwrap_or_default(); + let fingerprint_to_remove = msg.param.get(Param::Arg2).unwrap_or_default(); if email_to_remove == context @@ -1386,9 +1453,16 @@ impl MimeFactory { .into(), )); } + + if !fingerprint_to_remove.is_empty() { + headers.push(( + "Chat-Group-Member-Removed-Fpr", + mail_builder::headers::raw::Raw::new(fingerprint_to_remove.to_string()) + .into(), + )); + } } SystemMessage::MemberAddedToGroup => { - ensure!(chat.typ != Chattype::OutBroadcast); // TODO: lookup the contact by ID rather than email address. // We are adding key-contacts, the cannot be looked up by address. let email_to_add = msg.param.get(Param::Arg).unwrap_or_default(); @@ -1402,14 +1476,15 @@ impl MimeFactory { )); } if 0 != msg.param.get_int(Param::Arg2).unwrap_or_default() & DC_FROM_HANDSHAKE { - info!( - context, - "Sending secure-join message {:?}.", "vg-member-added", - ); + let step = match chat.typ { + Chattype::Group => "vg-member-added", + Chattype::OutBroadcast => "vb-member-added", + _ => bail!("Wrong chattype {}", chat.typ), + }; + info!(context, "Sending secure-join message {:?}.", step,); headers.push(( "Secure-Join", - mail_builder::headers::raw::Raw::new("vg-member-added".to_string()) - .into(), + mail_builder::headers::raw::Raw::new(step.to_string()).into(), )); } } @@ -1485,7 +1560,10 @@ impl MimeFactory { let param2 = msg.param.get(Param::Arg2).unwrap_or_default(); if !param2.is_empty() { headers.push(( - if step == "vg-request-with-auth" || step == "vc-request-with-auth" { + if step == "vg-request-with-auth" + || step == "vc-request-with-auth" + || step == "vb-request-with-auth" + { "Secure-Join-Auth" } else { "Secure-Join-Invitenumber" @@ -1822,6 +1900,25 @@ fn hidden_recipients() -> Address<'static> { Address::new_group(Some("hidden-recipients".to_string()), Vec::new()) } +fn should_encrypt_with_broadcast_secret(msg: &Message, chat: &Chat) -> bool { + chat.typ == Chattype::OutBroadcast + // The only `SystemMessage::SecurejoinMessage` that is ever sent into a broadcast, + // which is `vb-request-with-auth`, + // should be encrypted with the AUTH token rather than the broadcast secret. + && msg.param.get_cmd() != SystemMessage::SecurejoinMessage + // The member-added message in a broadcast must be asymmetrically encrypted, + // because the newly-added member doesn't know the broadcast shared secret yet: + && msg.param.get_cmd() != SystemMessage::MemberAddedToGroup +} + +fn should_hide_recipients(msg: &Message, chat: &Chat) -> bool { + should_encrypt_with_broadcast_secret(msg, chat) +} + +fn should_encrypt_symmetrically(msg: &Message, chat: &Chat) -> bool { + should_encrypt_with_broadcast_secret(msg, chat) +} + async fn build_body_file(context: &Context, msg: &Message) -> Result> { let file_name = msg.get_filename().context("msg has no file")?; let blob = msg diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 573e0c5994..93e0b4e38e 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -355,9 +355,21 @@ impl MimeMessage { let mail_raw; // Memory location for a possible decrypted message. let decrypted_msg; // Decrypted signed OpenPGP message. + let secrets: Vec = context + .sql + .query_map( + "SELECT secret FROM broadcasts_shared_secrets", + (), + |row| row.get(0), + |rows| { + rows.collect::, _>>() + .map_err(Into::into) + }, + ) + .await?; let (mail, is_encrypted) = - match tokio::task::block_in_place(|| try_decrypt(&mail, &private_keyring)) { + match tokio::task::block_in_place(|| try_decrypt(&mail, &private_keyring, &secrets)) { Ok(Some(mut msg)) => { mail_raw = msg.as_data_vec().unwrap_or_default(); @@ -1590,6 +1602,15 @@ impl MimeMessage { .is_some_and(|part| part.typ == Viewtype::Call) } + pub fn replace_msg_by_error(&mut self, error_msg: &str) { + self.is_system_message = SystemMessage::Unknown; + if let Some(part) = self.parts.first_mut() { + part.typ = Viewtype::Text; + part.msg = format!("[{error_msg}]"); + self.parts.truncate(1); + } + } + pub(crate) fn get_rfc724_mid(&self) -> Option { self.get_header(HeaderDef::MessageId) .and_then(|msgid| parse_message_id(msgid).ok()) diff --git a/src/param.rs b/src/param.rs index 6f8c1af24e..f36691a5b4 100644 --- a/src/param.rs +++ b/src/param.rs @@ -99,19 +99,39 @@ pub enum Param { /// For Messages /// - /// For "MemberRemovedFromGroup" this is the email address + /// For "MemberRemovedFromGroup", this is the email address /// removed from the group. /// - /// For "MemberAddedToGroup" this is the email address added to the group. + /// For "MemberAddedToGroup", this is the email address added to the group. + /// + /// For securejoin messages, this is the step, + /// which is put into the `Secure-Join` header. Arg = b'E', /// For Messages + /// + /// For `BobHandshakeMsg::Request`, this is the `Secure-Join-Invitenumber` header. + /// + /// For `BobHandshakeMsg::RequestWithAuth`, this is the `Secure-Join-Auth` header. + /// + /// For [`SystemMessage::MultiDeviceSync`], this contains the ids that are synced. + /// + /// For [`SystemMessage::MemberAddedToGroup`], + /// this is '1' if it was added because of a securejoin-handshake, and '0' otherwise. Arg2 = b'F', - /// `Secure-Join-Fingerprint` header for `{vc,vg}-request-with-auth` messages. + /// For Messages + /// + /// For `BobHandshakeMsg::RequestWithAuth`, + /// this contains the `Secure-Join-Fingerprint` header. + /// + /// For [`SystemMessage::MemberAddedToGroup`] that add to a broadcast channel, + /// this contains the broadcast channel's shared secret. Arg3 = b'G', - /// Deprecated `Secure-Join-Group` header for messages. + /// For Messages + /// + /// Deprecated `Secure-Join-Group` header for `BobHandshakeMsg::RequestWithAuth` messages. Arg4 = b'H', /// For Messages diff --git a/src/pgp.rs b/src/pgp.rs index 047e214a26..e4dcfc231b 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -3,7 +3,7 @@ use std::collections::{BTreeMap, HashSet}; use std::io::{BufRead, Cursor}; -use anyhow::{Context as _, Result}; +use anyhow::{Context as _, Result, bail}; use chrono::SubsecRound; use deltachat_contact_tools::EmailAddress; use pgp::armor::BlockType; @@ -12,12 +12,13 @@ use pgp::composed::{ Message, MessageBuilder, SecretKeyParamsBuilder, SignedPublicKey, SignedPublicSubKey, SignedSecretKey, SubkeyParamsBuilder, TheRing, }; +use pgp::crypto::aead::{AeadAlgorithm, ChunkSize}; use pgp::crypto::ecc_curve::ECCCurve; use pgp::crypto::hash::HashAlgorithm; use pgp::crypto::sym::SymmetricKeyAlgorithm; use pgp::packet::{SignatureConfig, SignatureType, Subpacket, SubpacketData}; use pgp::types::{CompressionAlgorithm, KeyDetails, Password, PublicKeyTrait, StringToKey}; -use rand::thread_rng; +use rand::{Rng as _, thread_rng}; use tokio::runtime::Handle; use crate::key::{DcKey, Fingerprint}; @@ -231,13 +232,17 @@ pub fn pk_calc_signature( Ok(sig.to_armored_string(ArmorOptions::default())?) } -/// Decrypts the message with keys from the private key keyring. +/// Decrypts the message: +/// - with keys from the private key keyring (passed in `private_keys_for_decryption`) +/// if the message was asymmetrically encrypted, +/// - with a shared secret/password (passed in `shared_secrets`), +/// if the message was symmetrically encrypted. /// -/// Receiver private keys are provided in -/// `private_keys_for_decryption`. -pub fn pk_decrypt( +/// Returns the decrypted and decompressed message. +pub fn decrypt( ctext: Vec, private_keys_for_decryption: &[SignedSecretKey], + mut shared_secrets: &[String], ) -> Result> { let cursor = Cursor::new(ctext); let (msg, _headers) = Message::from_armor(cursor)?; @@ -246,18 +251,43 @@ pub fn pk_decrypt( let empty_pw = Password::empty(); let decrypt_options = DecryptionOptions::new().enable_legacy(); + let try_symmetric_decryption = should_try_symmetric_decryption(&msg); + if try_symmetric_decryption.is_err() { + shared_secrets = &[]; + } + + // We always try out all passwords here, which is not great for performance. + // But benchmarking (see `benchmark_decrypting.rs`) + // showed that the performance penalty is acceptable. + // We could include a short (~2 character) identifier of the secret in cleartext + // (or just include the first 2 characters of the secret in cleartext) + // in order to narrow down the number of shared secrets that have to be tried out. + let message_password: Vec = shared_secrets + .iter() + .map(|p| Password::from(p.as_str())) + .collect(); + let message_password: Vec<&Password> = message_password.iter().collect(); + let ring = TheRing { secret_keys: skeys, key_passwords: vec![&empty_pw], - message_password: vec![], + message_password, session_keys: vec![], decrypt_options, }; - let (msg, ring_result) = msg.decrypt_the_ring(ring, true)?; - anyhow::ensure!( - !ring_result.secret_keys.is_empty(), - "decryption failed, no matching secret keys" - ); + + let res = msg.decrypt_the_ring(ring, true); + + let (msg, _ring_result) = match res { + Ok(it) => it, + Err(err) => { + if let Err(reason) = try_symmetric_decryption { + bail!("{err:#} (Note: symmetric decryption was not tried: {reason})") + } else { + bail!("{err:#}"); + } + } + }; // remove one layer of compression let msg = msg.decompress()?; @@ -265,6 +295,34 @@ pub fn pk_decrypt( Ok(msg) } +/// Returns Ok(()) if we want to try symmetrically decrypting the message, +/// and Err with a reason if symmetric decryption should not be tried. +/// +/// 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. +/// We would then try to decrypt all of the encrypted session keys +/// with all of the known shared secrets. +/// In order to prevent this, we do not try to symmetrically decrypt messages +/// that use a string2key algorithm other than 'Salted'. +fn should_try_symmetric_decryption(msg: &Message<'_>) -> std::result::Result<(), &'static str> { + let Message::Encrypted { esk, .. } = msg else { + return Err("not encrypted"); + }; + + if esk.len() > 1 { + return Err("too many esks"); + } + + let [pgp::composed::Esk::SymKeyEncryptedSessionKey(esk)] = &esk[..] else { + return Err("not symmetrically encrypted"); + }; + + match esk.s2k() { + Some(StringToKey::Salted { .. }) => Ok(()), + _ => Err("unsupported string2key algorithm"), + } +} + /// Returns fingerprints /// of all keys from the `public_keys_for_validation` keyring that /// have valid signatures there. @@ -305,8 +363,8 @@ pub fn pk_validate( Ok(ret) } -/// Symmetric encryption. -pub async fn symm_encrypt(passphrase: &str, plain: Vec) -> Result { +/// Symmetric encryption for the autocrypt setup file. +pub async fn symm_encrypt_setup_file(passphrase: &str, plain: Vec) -> Result { let passphrase = Password::from(passphrase.to_string()); tokio::task::spawn_blocking(move || { @@ -323,6 +381,46 @@ pub async fn symm_encrypt(passphrase: &str, plain: Vec) -> Result { .await? } +/// Symmetrically encrypt the message. +/// This is used for broadcast channels and for version 2 of the Securejoin protocol. +/// `shared secret` is the secret that will be used for symmetric encryption. +pub async fn symm_encrypt_message( + plain: Vec, + shared_secret: &str, + private_key_for_signing: SignedSecretKey, + compress: bool, +) -> Result { + let shared_secret = Password::from(shared_secret.to_string()); + + tokio::task::spawn_blocking(move || { + let msg = MessageBuilder::from_bytes("", plain); + let mut rng = thread_rng(); + let mut salt = [0u8; 8]; + rng.fill(&mut salt[..]); + let s2k = StringToKey::Salted { + hash_alg: HashAlgorithm::default(), + salt, + }; + let mut msg = msg.seipd_v2( + &mut rng, + SymmetricKeyAlgorithm::AES128, + AeadAlgorithm::Ocb, + ChunkSize::C8KiB, + ); + msg.encrypt_with_password(&mut rng, s2k, &shared_secret)?; + + msg.sign(&*private_key_for_signing, Password::empty(), HASH_ALGORITHM); + if compress { + msg.compression(CompressionAlgorithm::ZLIB); + } + + let encoded_msg = msg.to_armored_string(&mut rng, Default::default())?; + + Ok(encoded_msg) + }) + .await? +} + /// Symmetric decryption. pub async fn symm_decrypt( passphrase: &str, @@ -346,7 +444,10 @@ mod tests { use tokio::sync::OnceCell; use super::*; - use crate::test_utils::{alice_keypair, bob_keypair}; + use crate::{ + key::{load_self_public_key, load_self_secret_key}, + test_utils::{TestContextManager, alice_keypair, bob_keypair}, + }; fn pk_decrypt_and_validate<'a>( ctext: &'a [u8], @@ -357,7 +458,7 @@ mod tests { HashSet, Vec, )> { - let mut msg = pk_decrypt(ctext.to_vec(), private_keys_for_decryption)?; + let mut msg = decrypt(ctext.to_vec(), private_keys_for_decryption, &[])?; let content = msg.as_data_vec()?; let ret_signature_fingerprints = valid_signature_fingerprints(&msg, public_keys_for_validation); @@ -543,4 +644,103 @@ mod tests { assert_eq!(content, CLEARTEXT); assert_eq!(valid_signatures.len(), 0); } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_encrypt_decrypt_broadcast() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + let plain = Vec::from(b"this is the secret message"); + let shared_secret = "shared secret"; + let ctext = symm_encrypt_message( + plain.clone(), + shared_secret, + load_self_secret_key(alice).await?, + true, + ) + .await?; + + let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?; + let mut decrypted = decrypt( + ctext.into(), + &bob_private_keyring, + &[shared_secret.to_string()], + )?; + + assert_eq!(decrypted.as_data_vec()?, plain); + + Ok(()) + } + + /// Test that we don't try to decrypt a message + /// that is symmetrically encrypted + /// with an expensive string2key algorithm + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_dont_decrypt_expensive_message() -> Result<()> { + let mut tcm = TestContextManager::new(); + let bob = &tcm.bob().await; + + let plain = Vec::from(b"this is the secret message"); + let shared_secret = "shared secret"; + + // Create a symmetrically encrypted message + // with an IteratedAndSalted string2key algorithm: + + let shared_secret_pw = Password::from(shared_secret.to_string()); + let msg = MessageBuilder::from_bytes("", plain); + let mut rng = thread_rng(); + let s2k = StringToKey::new_default(&mut rng); // Default is IteratedAndSalted + + let mut msg = msg.seipd_v2( + &mut rng, + SymmetricKeyAlgorithm::AES128, + AeadAlgorithm::Ocb, + ChunkSize::C8KiB, + ); + msg.encrypt_with_password(&mut rng, s2k, &shared_secret_pw)?; + + let ctext = msg.to_armored_string(&mut rng, Default::default())?; + + // Trying to decrypt it should fail with a helpful error message: + + let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?; + let error = decrypt( + ctext.into(), + &bob_private_keyring, + &[shared_secret.to_string()], + ) + .unwrap_err(); + + assert_eq!( + error.to_string(), + "missing key (Note: symmetric decryption was not tried: unsupported string2key algorithm)" + ); + + Ok(()) + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_decryption_error_msg() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + let plain = Vec::from(b"this is the secret message"); + let pk_for_encryption = load_self_public_key(alice).await?; + + // Encrypt a message, but only to self, not to Bob: + let ctext = pk_encrypt(plain, vec![pk_for_encryption], None, true).await?; + + // Trying to decrypt it should fail with an OK error message: + let bob_private_keyring = crate::key::load_self_secret_keyring(bob).await?; + let error = decrypt(ctext.into(), &bob_private_keyring, &[]).unwrap_err(); + + assert_eq!( + error.to_string(), + "missing key (Note: symmetric decryption was not tried: not symmetrically encrypted)" + ); + + Ok(()) + } } diff --git a/src/qr.rs b/src/qr.rs index 553a2a9243..7c5d5f9b38 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -84,6 +84,30 @@ pub enum Qr { authcode: String, }, + /// Ask whether to join the broadcast channel. + AskJoinBroadcast { + /// The user-visible name of this broadcast channel + broadcast_name: String, + + /// A string of random characters, + /// uniquely identifying this broadcast channel in the database. + /// Called `grpid` for historic reasons: + /// The id of multi-user chats is always called `grpid` in the database + /// because groups were once the only multi-user chats. + grpid: String, + + /// ID of the contact who owns the channel and created the QR code. + contact_id: ContactId, + + /// Fingerprint of the contact's key as scanned from the QR code. + fingerprint: Fingerprint, + + /// Invite number. + invitenumber: String, + /// Authentication code. + authcode: String, + }, + /// Contact fingerprint is verified. /// /// Ask the user if they want to start chatting. @@ -381,6 +405,7 @@ pub fn format_backup(qr: &Qr) -> Result { /// scheme: `OPENPGP4FPR:FINGERPRINT#a=ADDR&n=NAME&i=INVITENUMBER&s=AUTH` /// or: `OPENPGP4FPR:FINGERPRINT#a=ADDR&g=GROUPNAME&x=GROUPID&i=INVITENUMBER&s=AUTH` +/// or: `OPENPGP4FPR:FINGERPRINT#a=ADDR&b=BROADCAST_NAME&x=BROADCAST_ID&i=INVITENUMBER&s=AUTH` /// or: `OPENPGP4FPR:FINGERPRINT#a=ADDR` async fn decode_openpgp(context: &Context, qr: &str) -> Result { let payload = qr @@ -417,15 +442,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { None }; - let name = if let Some(encoded_name) = param.get("n") { - let encoded_name = encoded_name.replace('+', "%20"); // sometimes spaces are encoded as `+` - match percent_decode_str(&encoded_name).decode_utf8() { - Ok(name) => name.to_string(), - Err(err) => bail!("Invalid name: {}", err), - } - } else { - "".to_string() - }; + let name = decode_name(¶m, "n")?.unwrap_or_default(); let invitenumber = param .get("i") @@ -440,19 +457,8 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { .filter(|&s| validate_id(s)) .map(|s| s.to_string()); - let grpname = if grpid.is_some() { - if let Some(encoded_name) = param.get("g") { - let encoded_name = encoded_name.replace('+', "%20"); // sometimes spaces are encoded as `+` - match percent_decode_str(&encoded_name).decode_utf8() { - Ok(name) => Some(name.to_string()), - Err(err) => bail!("Invalid group name: {}", err), - } - } else { - None - } - } else { - None - }; + let grpname = decode_name(¶m, "g")?; + let broadcast_name = decode_name(¶m, "b")?; if let (Some(addr), Some(invitenumber), Some(authcode)) = (&addr, invitenumber, authcode) { let addr = ContactAddress::new(addr)?; @@ -466,7 +472,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { .await .with_context(|| format!("failed to add or lookup contact for address {addr:?}"))?; - if let (Some(grpid), Some(grpname)) = (grpid, grpname) { + if let (Some(grpid), Some(grpname)) = (grpid.clone(), grpname) { if context .is_self_addr(&addr) .await @@ -501,6 +507,16 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { authcode, }) } + } else if let (Some(grpid), Some(broadcast_name)) = (grpid, broadcast_name) { + // This is a broadcast channel invite link. + Ok(Qr::AskJoinBroadcast { + broadcast_name, + grpid, + contact_id, + fingerprint, + invitenumber, + authcode, + }) } else if context.is_self_addr(&addr).await? { if token::exists(context, token::Namespace::InviteNumber, &invitenumber).await? { Ok(Qr::WithdrawVerifyContact { @@ -546,6 +562,18 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { } } +fn decode_name(param: &BTreeMap<&str, &str>, key: &str) -> Result> { + if let Some(encoded_name) = param.get(key) { + let encoded_name = encoded_name.replace('+', "%20"); // sometimes spaces are encoded as `+` + match percent_decode_str(&encoded_name).decode_utf8() { + Ok(name) => Ok(Some(name.to_string())), + Err(err) => bail!("Invalid QR param {key}: {err}"), + } + } else { + Ok(None) + } +} + /// scheme: `https://i.delta.chat[/]#FINGERPRINT&a=ADDR[&OPTIONAL_PARAMS]` async fn decode_ideltachat(context: &Context, prefix: &str, qr: &str) -> Result { let qr = qr.replacen(prefix, OPENPGP4FPR_SCHEME, 1); diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 3e9e3d762d..44ebbc9534 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -15,7 +15,7 @@ use num_traits::FromPrimitive; use regex::Regex; use crate::chat::{ - self, Chat, ChatId, ChatIdBlocked, ProtectionStatus, remove_from_chat_contacts_table, + self, Chat, ChatId, ChatIdBlocked, ProtectionStatus, save_broadcast_shared_secret, }; use crate::config::Config; use crate::constants::{self, Blocked, Chattype, DC_CHAT_ID_TRASH, EDITED_PREFIX, ShowEmails}; @@ -27,8 +27,8 @@ use crate::ephemeral::{Timer as EphemeralTimer, stock_ephemeral_timer_changed}; use crate::events::EventType; use crate::headerdef::{HeaderDef, HeaderDefMap}; use crate::imap::{GENERATED_PREFIX, markseen_on_imap_table}; -use crate::key::self_fingerprint_opt; use crate::key::{DcKey, Fingerprint}; +use crate::key::{self_fingerprint, self_fingerprint_opt}; use crate::log::LogExt; use crate::log::{info, warn}; use crate::logged_debug_assert; @@ -44,7 +44,10 @@ use crate::securejoin::{self, handle_securejoin_handshake, observe_securejoin_on use crate::simplify; use crate::stock_str; use crate::sync::Sync::*; -use crate::tools::{self, buf_compress, remove_subject_prefix}; +use crate::tools::{ + self, buf_compress, create_broadcast_shared_secret, remove_subject_prefix, + validate_broadcast_shared_secret, +}; use crate::{chatlist_events, ensure_and_debug_assert, ensure_and_debug_assert_eq, location}; use crate::{contact, imap}; @@ -1387,6 +1390,7 @@ async fn do_chat_assignment( create_or_lookup_mailinglist_or_broadcast( context, allow_creation, + create_blocked, mailinglist_header, from_id, mime_parser, @@ -1571,7 +1575,9 @@ async fn do_chat_assignment( } else { let name = compute_mailinglist_name(mailinglist_header, &listid, mime_parser); - chat::create_broadcast_ex(context, Nosync, listid, name).await? + let secret = create_broadcast_shared_secret(); + chat::create_out_broadcast_ex(context, Nosync, listid, name, secret) + .await? }, ); } @@ -1699,6 +1705,15 @@ async fn add_parts( for part in &mut mime_parser.parts { part.param.set(Param::OverrideSenderDisplayname, name); } + + if chat.typ == Chattype::InBroadcast { + let s = stock_str::error(context, "This message was not sent by the channel owner") + .await; + if let Some(part) = mime_parser.parts.first_mut() { + part.error = Some(format!("{s}:\n\"{}\"", part.msg)); + } + mime_parser.replace_msg_by_error(&s); + } } } @@ -2897,20 +2912,18 @@ async fn apply_group_changes( } if let Some(removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) { - // TODO: if address "alice@example.org" is a member of the group twice, - // with old and new key, - // and someone (maybe Alice's new contact) just removed Alice's old contact, - // we may lookup the wrong contact because we only look up by the address. - // The result is that info message may contain the new Alice's display name - // rather than old display name. - // This could be fixed by looking up the contact with the highest - // `remove_timestamp` after applying Chat-Group-Member-Timestamps. if !is_from_in_chat { better_msg = Some(String::new()); - } else if let Some(id) = - lookup_key_contact_by_address(context, removed_addr, Some(chat.id)).await? + } else if let Some(removed_fpr) = + mime_parser.get_header(HeaderDef::ChatGroupMemberRemovedFpr) { - removed_id = Some(id); + removed_id = lookup_key_contact_by_fingerprint(context, removed_fpr).await?; + } else { + // Removal message sent by a legacy Delta Chat client. + removed_id = + lookup_key_contact_by_address(context, removed_addr, Some(chat.id)).await?; + } + if let Some(id) = removed_id { better_msg = if id == from_id { silent = true; Some(stock_str::msg_group_left_local(context, from_id).await) @@ -2928,6 +2941,8 @@ async fn apply_group_changes( // we may lookup the wrong contact. // This could be fixed by looking up the contact with // highest `add_timestamp` to disambiguate. + // Alternatively, this can be fixed by a header ChatGroupMemberAddedFpr, + // just like we have ChatGroupMemberRemovedFpr. // The result of the error is that info message // may contain display name of the wrong contact. let fingerprint = key.public_key.dc_fingerprint().hex(); @@ -3069,9 +3084,7 @@ async fn apply_group_changes( if let Some(added_id) = added_id { if !added_ids.remove(&added_id) && !self_added { - // No-op "Member added" message. - // - // Trash it. + info!(context, "No-op 'Member added' message (TRASH)"); better_msg = Some(String::new()); } } @@ -3267,6 +3280,7 @@ fn mailinglist_header_listid(list_id_header: &str) -> Result { async fn create_or_lookup_mailinglist_or_broadcast( context: &Context, allow_creation: bool, + create_blocked: Blocked, list_id_header: &str, from_id: ContactId, mime_parser: &MimeMessage, @@ -3299,18 +3313,12 @@ async fn create_or_lookup_mailinglist_or_broadcast( p.to_string() }); - let is_bot = context.get_config_bool(Config::Bot).await?; - let blocked = if is_bot { - Blocked::Not - } else { - Blocked::Request - }; let chat_id = ChatId::create_multiuser_record( context, chattype, &listid, name, - blocked, + create_blocked, ProtectionStatus::Unprotected, param, mime_parser.timestamp_sent, @@ -3323,13 +3331,6 @@ async fn create_or_lookup_mailinglist_or_broadcast( ) })?; - chat::add_to_chat_contacts_table( - context, - mime_parser.timestamp_sent, - chat_id, - &[ContactId::SELF], - ) - .await?; if chattype == Chattype::InBroadcast { chat::add_to_chat_contacts_table( context, @@ -3339,7 +3340,12 @@ async fn create_or_lookup_mailinglist_or_broadcast( ) .await?; } - Ok(Some((chat_id, blocked))) + + context.emit_event(EventType::ChatModified(chat_id)); + chatlist_events::emit_chatlist_changed(context); + chatlist_events::emit_chatlist_item_changed(context, chat_id); + + Ok(Some((chat_id, create_blocked))) } else { info!(context, "Creating list forbidden by caller."); Ok(None) @@ -3492,19 +3498,55 @@ async fn apply_out_broadcast_changes( ) -> Result { ensure!(chat.typ == Chattype::OutBroadcast); - if let Some(_removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) { - // The sender of the message left the broadcast channel - remove_from_chat_contacts_table(context, chat.id, from_id).await?; + let mut send_event_chat_modified = false; + let mut better_msg = None; - return Ok(GroupChangesInfo { - better_msg: Some("".to_string()), - added_removed_id: None, - silent: true, - extra_msgs: vec![], - }); + if from_id == ContactId::SELF { + apply_chat_name_and_avatar_changes( + context, + mime_parser, + from_id, + chat, + &mut send_event_chat_modified, + &mut better_msg, + ) + .await?; + } + + if let Some(removed_fpr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemovedFpr) { + send_event_chat_modified = true; + let removed_id = lookup_key_contact_by_fingerprint(context, removed_fpr).await?; + if removed_id == Some(from_id) { + // The sender of the message left the broadcast channel + // Silently remove them without notifying the user + chat::remove_from_chat_contacts_table_without_trace(context, chat.id, from_id).await?; + info!(context, "Broadcast leave message (TRASH)"); + better_msg = Some("".to_string()); + } else if from_id == ContactId::SELF { + if let Some(removed_id) = removed_id { + chat::remove_from_chat_contacts_table_without_trace(context, chat.id, removed_id) + .await?; + + better_msg.get_or_insert( + stock_str::msg_del_member_local(context, removed_id, ContactId::SELF).await, + ); + } + } } + // No need to check for ChatGroupMemberAdded: + // The only way to add a member is by having them scan a QR code. + // All devices will receive Bob's vb-request-with-auth message and add him to the channel. - Ok(GroupChangesInfo::default()) + if send_event_chat_modified { + context.emit_event(EventType::ChatModified(chat.id)); + chatlist_events::emit_chatlist_item_changed(context, chat.id); + } + Ok(GroupChangesInfo { + better_msg, + added_removed_id: None, + silent: false, + extra_msgs: vec![], + }) } async fn apply_in_broadcast_changes( @@ -3515,6 +3557,16 @@ async fn apply_in_broadcast_changes( ) -> Result { ensure!(chat.typ == Chattype::InBroadcast); + if let Some(part) = mime_parser.parts.first() { + if let Some(error) = &part.error { + warn!( + context, + "Not applying broadcast changes from message with error: {error}" + ); + return Ok(GroupChangesInfo::default()); + } + } + let mut send_event_chat_modified = false; let mut better_msg = None; @@ -3528,12 +3580,61 @@ async fn apply_in_broadcast_changes( ) .await?; - if let Some(_removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) { - // The only member added/removed message that is ever sent is "I left.", - // so, this is the only case we need to handle here + if let Some(added_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberAdded) { + if context.is_self_addr(added_addr).await? { + let msg; + + if chat.is_self_in_chat(context).await? { + // Self is already in the chat. + // Probably Alice has two devices and her second device added us again; + // just hide the message. + info!(context, "No-op broadcast 'Member added' message (TRASH)"); + msg = "".to_string(); + } else { + chat.id + .add_encrypted_msg(context, mime_parser.timestamp_sent) + .await?; + msg = stock_str::msg_add_member_local(context, ContactId::SELF, from_id).await; + } + + better_msg.get_or_insert(msg); + send_event_chat_modified = true; + } + } + + if let Some(removed_fpr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemovedFpr) { + // We are not supposed to receive a notification when someone else than self is removed: + ensure!(removed_fpr == self_fingerprint(context).await?); + if from_id == ContactId::SELF { better_msg .get_or_insert(stock_str::msg_group_left_local(context, ContactId::SELF).await); + } else { + better_msg.get_or_insert( + stock_str::msg_del_member_local(context, ContactId::SELF, from_id).await, + ); + } + + chat::remove_from_chat_contacts_table_without_trace(context, chat.id, ContactId::SELF) + .await?; + send_event_chat_modified = true; + } else if !chat.is_self_in_chat(context).await? { + // Apparently, self is in the chat now, because we're receiving messages + chat::add_to_chat_contacts_table( + context, + mime_parser.timestamp_sent, + chat.id, + &[ContactId::SELF], + ) + .await?; + send_event_chat_modified = true; + } + + if let Some(secret) = mime_parser.get_header(HeaderDef::ChatBroadcastSecret) { + if validate_broadcast_shared_secret(secret) { + save_broadcast_shared_secret(context, chat.id, secret).await?; + } else { + warn!(context, "Not saving invalid broadcast secret"); } } diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 618a807e73..1919dd0cea 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -881,7 +881,7 @@ async fn test_github_mailing_list() -> Result<()> { Some("reply+elernshsetushoyseshetihseusaferuhsedtisneu@reply.github.com") ); assert_eq!(chat.name, "deltachat/deltachat-core-rust"); - assert_eq!(chat::get_chat_contacts(&t.ctx, chat_id).await?.len(), 1); + assert_eq!(chat::get_chat_contacts(&t.ctx, chat_id).await?.len(), 0); receive_imf(&t.ctx, GH_MAILINGLIST2.as_bytes(), false).await?; diff --git a/src/securejoin.rs b/src/securejoin.rs index 04642ac945..ce43495227 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -35,14 +35,8 @@ fn inviter_progress( context: &Context, contact_id: ContactId, chat_id: ChatId, - is_group: bool, + chat_type: Chattype, ) -> Result<()> { - let chat_type = if is_group { - Chattype::Group - } else { - Chattype::Single - }; - // No other values are used. let progress = 1000; context.emit_event(EventType::SecurejoinInviterProgress { @@ -57,9 +51,9 @@ fn inviter_progress( /// Generates a Secure Join QR code. /// -/// With `group` set to `None` this generates a setup-contact QR code, with `group` set to a -/// [`ChatId`] generates a join-group QR code for the given chat. -pub async fn get_securejoin_qr(context: &Context, group: Option) -> Result { +/// With `chat` set to `None` this generates a setup-contact QR code, with `chat` set to a +/// [`ChatId`] generates a join-group/join-broadcast-channel QR code for the given chat. +pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Result { /*======================================================= ==== Alice - the inviter side ==== ==== Step 1 in "Setup verified contact" protocol ==== @@ -67,12 +61,13 @@ pub async fn get_securejoin_qr(context: &Context, group: Option) -> Resu ensure_secret_key_exists(context).await.ok(); - let chat = match group { + let chat = match chat { Some(id) => { let chat = Chat::load_from_db(context, id).await?; ensure!( - chat.typ == Chattype::Group, - "Can't generate SecureJoin QR code for 1:1 chat {id}" + chat.typ == Chattype::Group || chat.typ == Chattype::OutBroadcast, + "Can't generate SecureJoin QR code for chat {id} of type {}", + chat.typ ); if chat.grpid.is_empty() { let err = format!("Can't generate QR code, chat {id} is a email thread"); @@ -105,24 +100,36 @@ pub async fn get_securejoin_qr(context: &Context, group: Option) -> Resu utf8_percent_encode(&self_name, NON_ALPHANUMERIC_WITHOUT_DOT).to_string(); let qr = if let Some(chat) = chat { - // parameters used: a=g=x=i=s= - let group_name = chat.get_name(); - let group_name_urlencoded = utf8_percent_encode(group_name, NON_ALPHANUMERIC).to_string(); if sync_token { context .sync_qr_code_tokens(Some(chat.grpid.as_str())) .await?; context.scheduler.interrupt_inbox().await; } - format!( - "https://i.delta.chat/#{}&a={}&g={}&x={}&i={}&s={}", - fingerprint.hex(), - self_addr_urlencoded, - &group_name_urlencoded, - &chat.grpid, - &invitenumber, - &auth, - ) + + let chat_name = chat.get_name(); + let chat_name_urlencoded = utf8_percent_encode(chat_name, NON_ALPHANUMERIC).to_string(); + if chat.typ == Chattype::OutBroadcast { + format!( + "https://i.delta.chat/#{}&a={}&b={}&x={}&i={}&s={}", + fingerprint.hex(), + self_addr_urlencoded, + &chat_name_urlencoded, + &chat.grpid, + &invitenumber, + &auth, + ) + } else { + format!( + "https://i.delta.chat/#{}&a={}&g={}&x={}&i={}&s={}", + fingerprint.hex(), + self_addr_urlencoded, + &chat_name_urlencoded, + &chat.grpid, + &invitenumber, + &auth, + ) + } } else { // parameters used: a=n=i=s= if sync_token { @@ -277,6 +284,21 @@ pub(crate) async fn handle_securejoin_handshake( info!(context, "Received secure-join message {step:?}."); + // Opportunistically protect against a theoretical 'surreptitious forwarding' attack: + // If Eve obtains a QR code from Alice and starts a securejoin with her, + // and also lets Bob scan a manipulated QR code, + // she could reencrypt the v*-request-with-auth message to Bob while maintaining the signature, + // and Bob would regard the message as valid. + // + // This attack is not actually relevant in any threat model, + // because if Eve can see Alice's QR code and have Bob scan a manipulated QR code, + // she can just do a classical MitM attack. + // + // Protecting all messages sent by Delta Chat against 'surreptitious forwarding' + // by checking the 'intended recipient fingerprint' + // 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, "vg-request" | "vc-request") { let mut self_found = false; let self_fingerprint = load_self_public_key(context).await?.dc_fingerprint(); @@ -347,7 +369,8 @@ pub(crate) async fn handle_securejoin_handshake( ========================================================*/ bob::handle_auth_required(context, mime_message).await } - "vg-request-with-auth" | "vc-request-with-auth" => { + // TODO possibly rename vb-request* to vg-request* or vc-request* + "vg-request-with-auth" | "vc-request-with-auth" | "vb-request-with-auth" => { /*========================================================== ==== Alice - the inviter side ==== ==== Steps 5+6 in "Setup verified contact" protocol ==== @@ -385,7 +408,7 @@ pub(crate) async fn handle_securejoin_handshake( ); return Ok(HandshakeMessage::Ignore); }; - let group_chat_id = match grpid.as_str() { + let joining_chat_id = match grpid.as_str() { "" => None, id => { let Some((chat_id, ..)) = get_chat_id_by_grpid(context, id).await? else { @@ -412,22 +435,31 @@ pub(crate) async fn handle_securejoin_handshake( ChatId::create_for_contact(context, contact_id).await?; } context.emit_event(EventType::ContactsChanged(Some(contact_id))); - if let Some(group_chat_id) = group_chat_id { + if let Some(joining_chat_id) = joining_chat_id { // Join group. secure_connection_established( context, contact_id, - group_chat_id, + joining_chat_id, mime_message.timestamp_sent, ) .await?; - chat::add_contact_to_chat_ex(context, Nosync, group_chat_id, contact_id, true) + + chat::add_contact_to_chat_ex(context, Nosync, joining_chat_id, contact_id, true) .await?; - let is_group = true; - inviter_progress(context, contact_id, group_chat_id, is_group)?; - // IMAP-delete the message to avoid handling it by another device and adding the - // member twice. Another device will know the member's key from Autocrypt-Gossip. - Ok(HandshakeMessage::Done) + let chat = Chat::load_from_db(context, joining_chat_id).await?; + + inviter_progress(context, contact_id, joining_chat_id, chat.typ)?; + if chat.typ == Chattype::InBroadcast { + // For broadcasts, we don't want to delete the message, + // because the other device should also internally add the member + // and see the key (because it won't see the member via autocrypt-gossip). + Ok(HandshakeMessage::Ignore) + } else { + // IMAP-delete the message to avoid handling it by another device and adding the + // member twice. Another device will know the member's key from Autocrypt-Gossip. + Ok(HandshakeMessage::Done) + } } else { let chat_id = info_chat_id(context, contact_id).await?; // Setup verified contact. @@ -442,8 +474,7 @@ pub(crate) async fn handle_securejoin_handshake( .await .context("failed sending vc-contact-confirm message")?; - let is_group = false; - inviter_progress(context, contact_id, chat_id, is_group)?; + inviter_progress(context, contact_id, chat_id, Chattype::Single)?; Ok(HandshakeMessage::Ignore) // "Done" would delete the message and break multi-device (the key from Autocrypt-header is needed) } } @@ -458,7 +489,7 @@ pub(crate) async fn handle_securejoin_handshake( }); Ok(HandshakeMessage::Ignore) } - "vg-member-added" => { + "vg-member-added" | "vb-member-added" => { let Some(member_added) = mime_message.get_header(HeaderDef::ChatGroupMemberAdded) else { warn!( @@ -525,8 +556,15 @@ pub(crate) async fn observe_securejoin_on_other_device( if !matches!( step, - "vg-request-with-auth" | "vc-request-with-auth" | "vg-member-added" | "vc-contact-confirm" + "vg-request-with-auth" + | "vb-request-with-auth" + | "vc-request-with-auth" + | "vg-member-added" + | "vc-contact-confirm" ) { + // `vb-member-added` can be ignored + // because all devices receive the `vb-request-with-auth` message + // and mark Bob as verified because of this. return Ok(HandshakeMessage::Ignore); }; @@ -563,9 +601,16 @@ pub(crate) async fn observe_securejoin_on_other_device( ChatId::set_protection_for_contact(context, contact_id, mime_message.timestamp_sent).await?; if step == "vg-member-added" || step == "vc-contact-confirm" { - let is_group = mime_message + let chat_type = if mime_message .get_header(HeaderDef::ChatGroupMemberAdded) - .is_some(); + .is_none() + { + Chattype::Single + } else if mime_message.get_header(HeaderDef::ListId).is_some() { + Chattype::InBroadcast + } else { + Chattype::Group + }; // We don't know the chat ID // as we may not know about the group yet. @@ -575,7 +620,7 @@ pub(crate) async fn observe_securejoin_on_other_device( // and tests which don't care about the chat ID, // so we pass invalid chat ID here. let chat_id = ChatId::new(0); - inviter_progress(context, contact_id, chat_id, is_group)?; + inviter_progress(context, contact_id, chat_id, chat_type)?; } if step == "vg-request-with-auth" || step == "vc-request-with-auth" { diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 5392f94692..ebdc5261b7 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -17,7 +17,7 @@ use crate::param::Param; use crate::securejoin::{ContactId, encrypted_and_signed, verify_sender_by_fingerprint}; use crate::stock_str; use crate::sync::Sync::*; -use crate::tools::{create_smeared_timestamp, time}; +use crate::tools::{smeared_time, time}; /// Starts the securejoin protocol with the QR `invite`. /// @@ -47,10 +47,17 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul let hidden = match invite { QrInvite::Contact { .. } => Blocked::Not, QrInvite::Group { .. } => Blocked::Yes, + QrInvite::Broadcast { .. } => Blocked::Yes, }; - let chat_id = ChatId::create_for_contact_with_blocked(context, invite.contact_id(), hidden) - .await - .with_context(|| format!("can't create chat for contact {}", invite.contact_id()))?; + + // The 1:1 chat with the inviter + let private_chat_id = + ChatId::create_for_contact_with_blocked(context, invite.contact_id(), hidden) + .await + .with_context(|| format!("can't create chat for contact {}", invite.contact_id()))?; + + // The chat id of the 1:1 chat, group or broadcast that is being joined + let joining_chat_id = joining_chat_id(context, &invite, private_chat_id).await?; ContactId::scaleup_origin(context, &[invite.contact_id()], Origin::SecurejoinJoined).await?; context.emit_event(EventType::ContactsChanged(None)); @@ -71,11 +78,16 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul { // The scanned fingerprint matches Alice's key, we can proceed to step 4b. info!(context, "Taking securejoin protocol shortcut"); - send_handshake_message(context, &invite, chat_id, BobHandshakeMsg::RequestWithAuth) - .await?; + send_handshake_message( + context, + &invite, + private_chat_id, + BobHandshakeMsg::RequestWithAuth, + ) + .await?; // Mark 1:1 chat as verified already. - chat_id + private_chat_id .set_protection( context, ProtectionStatus::Protected, @@ -89,9 +101,10 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul progress: JoinerProgress::RequestWithAuthSent.to_usize(), }); } else { - send_handshake_message(context, &invite, chat_id, BobHandshakeMsg::Request).await?; + send_handshake_message(context, &invite, private_chat_id, BobHandshakeMsg::Request) + .await?; - insert_new_db_entry(context, invite.clone(), chat_id).await?; + insert_new_db_entry(context, invite.clone(), private_chat_id).await?; } } @@ -99,19 +112,35 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul QrInvite::Group { .. } => { // For a secure-join we need to create the group and add the contact. The group will // only become usable once the protocol is finished. - let group_chat_id = joining_chat_id(context, &invite, chat_id).await?; - if !is_contact_in_chat(context, group_chat_id, invite.contact_id()).await? { + if !is_contact_in_chat(context, joining_chat_id, invite.contact_id()).await? { chat::add_to_chat_contacts_table( context, time(), - group_chat_id, + joining_chat_id, &[invite.contact_id()], ) .await?; } let msg = stock_str::secure_join_started(context, invite.contact_id()).await; - chat::add_info_msg(context, group_chat_id, &msg, time()).await?; - Ok(group_chat_id) + chat::add_info_msg(context, joining_chat_id, &msg, time()).await?; + Ok(joining_chat_id) + } + QrInvite::Broadcast { .. } => { + if !is_contact_in_chat(context, joining_chat_id, invite.contact_id()).await? { + chat::add_to_chat_contacts_table( + context, + time(), + joining_chat_id, + &[invite.contact_id()], + ) + .await?; + } + + if !is_contact_in_chat(context, joining_chat_id, ContactId::SELF).await? { + let msg = stock_str::securejoin_wait(context).await; + chat::add_info_msg(context, joining_chat_id, &msg, time()).await?; + } + Ok(joining_chat_id) } QrInvite::Contact { .. } => { // For setup-contact the BobState already ensured the 1:1 chat exists because it @@ -120,14 +149,14 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul // race with its change, we don't add our message below the protection message. let sort_to_bottom = true; let (received, incoming) = (false, false); - let ts_sort = chat_id + let ts_sort = private_chat_id .calc_sort_timestamp(context, 0, sort_to_bottom, received, incoming) .await?; - if chat_id.is_protected(context).await? == ProtectionStatus::Unprotected { + if private_chat_id.is_protected(context).await? == ProtectionStatus::Unprotected { let ts_start = time(); chat::add_info_msg_with_cmd( context, - chat_id, + private_chat_id, &stock_str::securejoin_wait(context).await, SystemMessage::SecurejoinWait, ts_sort, @@ -138,7 +167,7 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul ) .await?; } - Ok(chat_id) + Ok(private_chat_id) } } } @@ -204,7 +233,7 @@ pub(super) async fn handle_auth_required( .await?; match invite { - QrInvite::Contact { .. } => {} + QrInvite::Contact { .. } | QrInvite::Broadcast { .. } => {} QrInvite::Group { .. } => { // The message reads "Alice replied, waiting to be added to the group…", // so only show it on secure-join and not on setup-contact. @@ -297,9 +326,9 @@ pub(crate) async fn send_handshake_message( /// Identifies the SecureJoin handshake messages Bob can send. pub(crate) enum BobHandshakeMsg { - /// vc-request or vg-request + /// vc-request, vg-request, or vb-request Request, - /// vc-request-with-auth or vg-request-with-auth + /// vc-request-with-auth, vg-request-with-auth, or vb-request-with-auth RequestWithAuth, } @@ -323,10 +352,12 @@ impl BobHandshakeMsg { Self::Request => match invite { QrInvite::Contact { .. } => "vc-request", QrInvite::Group { .. } => "vg-request", + QrInvite::Broadcast { .. } => "vb-request", }, Self::RequestWithAuth => match invite { QrInvite::Contact { .. } => "vc-request-with-auth", QrInvite::Group { .. } => "vg-request-with-auth", + QrInvite::Broadcast { .. } => "vb-request-with-auth", }, } } @@ -346,8 +377,19 @@ async fn joining_chat_id( ) -> Result { match invite { QrInvite::Contact { .. } => Ok(alice_chat_id), - QrInvite::Group { grpid, name, .. } => { - let group_chat_id = match chat::get_chat_id_by_grpid(context, grpid).await? { + QrInvite::Group { grpid, name, .. } + | QrInvite::Broadcast { + broadcast_name: name, + grpid, + .. + } => { + let chattype = if matches!(invite, QrInvite::Group { .. }) { + Chattype::Group + } else { + Chattype::InBroadcast + }; + + let chat_id = match chat::get_chat_id_by_grpid(context, grpid).await? { Some((chat_id, _protected, _blocked)) => { chat_id.unblock_ex(context, Nosync).await?; chat_id @@ -355,18 +397,18 @@ async fn joining_chat_id( None => { ChatId::create_multiuser_record( context, - Chattype::Group, + chattype, grpid, name, Blocked::Not, ProtectionStatus::Unprotected, // protection is added later as needed None, - create_smeared_timestamp(context), + smeared_time(context), ) .await? } }; - Ok(group_chat_id) + Ok(chat_id) } } } diff --git a/src/securejoin/qrinvite.rs b/src/securejoin/qrinvite.rs index 023d6875b6..323882d65c 100644 --- a/src/securejoin/qrinvite.rs +++ b/src/securejoin/qrinvite.rs @@ -29,6 +29,14 @@ pub enum QrInvite { invitenumber: String, authcode: String, }, + Broadcast { + contact_id: ContactId, + fingerprint: Fingerprint, + broadcast_name: String, + grpid: String, + invitenumber: String, + authcode: String, + }, } impl QrInvite { @@ -38,28 +46,36 @@ impl QrInvite { /// translated to a contact ID. pub fn contact_id(&self) -> ContactId { match self { - Self::Contact { contact_id, .. } | Self::Group { contact_id, .. } => *contact_id, + Self::Contact { contact_id, .. } + | Self::Group { contact_id, .. } + | Self::Broadcast { contact_id, .. } => *contact_id, } } /// The fingerprint of the inviter. pub fn fingerprint(&self) -> &Fingerprint { match self { - Self::Contact { fingerprint, .. } | Self::Group { fingerprint, .. } => fingerprint, + Self::Contact { fingerprint, .. } + | Self::Group { fingerprint, .. } + | Self::Broadcast { fingerprint, .. } => fingerprint, } } /// The `INVITENUMBER` of the setup-contact/secure-join protocol. pub fn invitenumber(&self) -> &str { match self { - Self::Contact { invitenumber, .. } | Self::Group { invitenumber, .. } => invitenumber, + Self::Contact { invitenumber, .. } + | Self::Group { invitenumber, .. } + | Self::Broadcast { invitenumber, .. } => invitenumber, } } /// The `AUTH` code of the setup-contact/secure-join protocol. pub fn authcode(&self) -> &str { match self { - Self::Contact { authcode, .. } | Self::Group { authcode, .. } => authcode, + Self::Contact { authcode, .. } + | Self::Group { authcode, .. } + | Self::Broadcast { authcode, .. } => authcode, } } } @@ -95,7 +111,22 @@ impl TryFrom for QrInvite { invitenumber, authcode, }), - _ => bail!("Unsupported QR type"), + Qr::AskJoinBroadcast { + broadcast_name, + grpid, + contact_id, + fingerprint, + authcode, + invitenumber, + } => Ok(QrInvite::Broadcast { + broadcast_name, + grpid, + contact_id, + fingerprint, + authcode, + invitenumber, + }), + _ => bail!("Unsupported QR type: {qr:?}"), } } } diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 2e2b925a93..a7a0dec455 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -9,7 +9,8 @@ use crate::mimeparser::GossipedKey; use crate::receive_imf::receive_imf; use crate::stock_str::{self, messages_e2e_encrypted}; use crate::test_utils::{ - TestContext, TestContextManager, TimeShiftFalsePositiveNote, get_chat_msg, + AVATAR_64x64_BYTES, AVATAR_64x64_DEDUPLICATED, TestContext, TestContextManager, + TimeShiftFalsePositiveNote, get_chat_msg, }; use crate::tools::SystemTime; use std::time::Duration; @@ -872,3 +873,72 @@ async fn test_wrong_auth_token() -> Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_send_avatar_in_securejoin() -> Result<()> { + async fn exec_securejoin_group( + tcm: &TestContextManager, + scanner: &TestContext, + scanned: &TestContext, + ) { + let chat_id = chat::create_group_chat(scanned, ProtectionStatus::Protected, "group") + .await + .unwrap(); + let qr = get_securejoin_qr(scanned, Some(chat_id)).await.unwrap(); + tcm.exec_securejoin_qr(scanner, scanned, &qr).await; + } + async fn exec_securejoin_broadcast( + tcm: &TestContextManager, + scanner: &TestContext, + scanned: &TestContext, + ) { + let chat_id = chat::create_broadcast(scanned, "group".to_string()) + .await + .unwrap(); + let qr = get_securejoin_qr(scanned, Some(chat_id)).await.unwrap(); + tcm.exec_securejoin_qr(scanner, scanned, &qr).await; + } + + for round in 0..6 { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + let file = alice.dir.path().join("avatar.png"); + tokio::fs::write(&file, AVATAR_64x64_BYTES).await?; + alice + .set_config(Config::Selfavatar, Some(file.to_str().unwrap())) + .await?; + + match round { + 0 => { + tcm.execute_securejoin(alice, bob).await; + } + 1 => { + tcm.execute_securejoin(bob, alice).await; + } + 2 => { + exec_securejoin_group(&tcm, alice, bob).await; + } + 3 => { + exec_securejoin_group(&tcm, bob, alice).await; + } + 4 => { + exec_securejoin_broadcast(&tcm, alice, bob).await; + } + 5 => { + exec_securejoin_broadcast(&tcm, bob, alice).await; + } + _ => panic!(), + } + + let alice_on_bob = bob.add_or_lookup_contact_no_key(alice).await; + let avatar = alice_on_bob.get_profile_image(bob).await?.unwrap(); + assert_eq!( + avatar.file_name().unwrap().to_str().unwrap(), + AVATAR_64x64_DEDUPLICATED + ); + } + + Ok(()) +} diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index 4a7c40e911..2de6979d00 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -1261,6 +1261,18 @@ CREATE INDEX gossip_timestamp_index ON gossip_timestamp (chat_id, fingerprint); .await?; } + inc_and_check(&mut migration_version, 134)?; + if dbversion < migration_version { + sql.execute_migration( + "CREATE TABLE broadcasts_shared_secrets( + chat_id INTEGER PRIMARY KEY NOT NULL, + secret TEXT NOT NULL + ) STRICT", + migration_version, + ) + .await?; + } + let new_version = sql .get_raw_config_int(VERSION_CFG) .await? diff --git a/src/sync.rs b/src/sync.rs index 1f17ff48c7..1f7acf31d0 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -1,6 +1,6 @@ //! # Synchronize items between devices. -use anyhow::Result; +use anyhow::{Context as _, Result}; use mail_builder::mime::MimePart; use serde::{Deserialize, Serialize}; @@ -270,6 +270,7 @@ impl Context { Ok(()) } } + .with_context(|| format!("Sync data {:?}", item.data)) .log_err(self) .ok(); } diff --git a/src/test_utils.rs b/src/test_utils.rs index 0c6c590dbe..35077d80ca 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -222,24 +222,55 @@ impl TestContextManager { self.exec_securejoin_qr(scanner, scanned, &qr).await } - /// Executes SecureJoin initiated by `scanner` scanning `qr` generated by `scanned`. + /// Executes SecureJoin initiated by `joiner` scanning `qr` generated by `inviter`. /// /// The [`ChatId`] of the created chat is returned, for a SetupContact QR this is the 1:1 - /// chat with `scanned`, for a SecureJoin QR this is the group chat. + /// chat with `inviter`, for a SecureJoin QR this is the group chat. pub async fn exec_securejoin_qr( &self, - scanner: &TestContext, - scanned: &TestContext, + joiner: &TestContext, + inviter: &TestContext, qr: &str, ) -> ChatId { - let chat_id = join_securejoin(&scanner.ctx, qr).await.unwrap(); + self.exec_securejoin_qr_multi_device(joiner, &[inviter], qr) + .await + } + + /// Executes SecureJoin initiated by `joiner` + /// scanning `qr` generated by one of the `inviters` devices. + /// All of the `inviters` devices will get the messages and send replies. + /// + /// The [`ChatId`] of the created chat is returned, for a SetupContact QR this is the 1:1 + /// chat with `inviter`, for a SecureJoin QR this is the group chat. + pub async fn exec_securejoin_qr_multi_device( + &self, + joiner: &TestContext, + inviters: &[&TestContext], + qr: &str, + ) -> ChatId { + assert!(joiner.pop_sent_msg_opt(Duration::ZERO).await.is_none()); + for inviter in inviters { + assert!(inviter.pop_sent_msg_opt(Duration::ZERO).await.is_none()); + } + + let chat_id = join_securejoin(&joiner.ctx, qr).await.unwrap(); loop { - if let Some(sent) = scanner.pop_sent_msg_opt(Duration::ZERO).await { - scanned.recv_msg_opt(&sent).await; - } else if let Some(sent) = scanned.pop_sent_msg_opt(Duration::ZERO).await { - scanner.recv_msg_opt(&sent).await; - } else { + let mut something_sent = false; + if let Some(sent) = joiner.pop_sent_msg_opt(Duration::ZERO).await { + for inviter in inviters { + inviter.recv_msg_opt(&sent).await; + } + something_sent = true; + } + for inviter in inviters { + if let Some(sent) = inviter.pop_sent_msg_opt(Duration::ZERO).await { + joiner.recv_msg_opt(&sent).await; + something_sent = true; + } + } + + if !something_sent { break; } } @@ -960,9 +991,15 @@ impl TestContext { .await .unwrap_or_else(|e| panic!("Error writing {filename:?}: {e}")); } else { + let green = Color::Green.normal(); + let red = Color::Red.normal(); assert_eq!( - actual, expected, - "To update the expected value, run `UPDATE_GOLDEN_TESTS=1 cargo test`" + actual, + expected, + "{} != {} on {}'s device.\nTo update the expected value, run with `UPDATE_GOLDEN_TESTS=1` environment variable", + red.paint("actual chat content (shown in red)"), + green.paint("expected chat content (shown in green)"), + self.name(), ); } } diff --git a/src/tools.rs b/src/tools.rs index 59cca8d158..a9b335d4c2 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -300,6 +300,25 @@ pub(crate) fn create_id() -> String { base64::engine::general_purpose::URL_SAFE.encode(arr) } +/// Generate a shared secret for a broadcast channel, consisting of 43 characters. +/// +/// The string generated by this function has 258 bits of entropy +/// and is returned as 43 Base64 characters, each containing 6 bits of entropy. +/// 256 is chosen because we may switch to AES-256 keys in the future, +/// and so that the shared secret definitely won't be the weak spot. +pub(crate) fn create_broadcast_shared_secret() -> String { + // ThreadRng implements CryptoRng trait and is supposed to be cryptographically secure. + let mut rng = thread_rng(); + + // Generate 264 random bits. + let mut arr = [0u8; 33]; + rng.fill(&mut arr[..]); + + let mut res = base64::engine::general_purpose::URL_SAFE.encode(arr); + res.truncate(43); + res +} + /// Returns true if given string is a valid ID. /// /// All IDs generated with `create_id()` should be considered valid. @@ -308,6 +327,11 @@ pub(crate) fn validate_id(s: &str) -> bool { s.chars().all(|c| alphabet.contains(c)) && s.len() > 10 && s.len() <= 32 } +pub(crate) fn validate_broadcast_shared_secret(s: &str) -> bool { + let alphabet = base64::alphabet::URL_SAFE.as_str(); + s.chars().all(|c| alphabet.contains(c)) && s.len() >= 43 && s.len() <= 100 +} + /// Function generates a Message-ID that can be used for a new outgoing message. /// - this function is called for all outgoing messages. /// - the message ID should be globally unique diff --git a/test-data/golden/test_broadcast_joining_golden_alice b/test-data/golden/test_broadcast_joining_golden_alice new file mode 100644 index 0000000000..d436657278 --- /dev/null +++ b/test-data/golden/test_broadcast_joining_golden_alice @@ -0,0 +1,6 @@ +OutBroadcast#Chat#10: My Channel [1 member(s)] Icon: e9b6c7a78aa2e4f415644f55a553e73.png +-------------------------------------------------------------------------------- +Msg#10: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#11πŸ”’: Me (Contact#Contact#Self): You changed the group image. [INFO] √ +Msg#13πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +-------------------------------------------------------------------------------- diff --git a/test-data/golden/test_broadcast_joining_golden_alice_direct b/test-data/golden/test_broadcast_joining_golden_alice_direct new file mode 100644 index 0000000000..68fb5acf17 --- /dev/null +++ b/test-data/golden/test_broadcast_joining_golden_alice_direct @@ -0,0 +1,4 @@ +Single#Chat#11: bob@example.net [KEY bob@example.net] πŸ›‘οΈ +-------------------------------------------------------------------------------- +Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO πŸ›‘οΈ] +-------------------------------------------------------------------------------- diff --git a/test-data/golden/test_broadcast_joining_golden_bob b/test-data/golden/test_broadcast_joining_golden_bob new file mode 100644 index 0000000000..c184cc9256 --- /dev/null +++ b/test-data/golden/test_broadcast_joining_golden_bob @@ -0,0 +1,6 @@ +InBroadcast#Chat#11: My Channel [2 member(s)] Icon: e9b6c7a78aa2e4f415644f55a553e73.png +-------------------------------------------------------------------------------- +Msg#11: info (Contact#Contact#Info): Establishing guaranteed end-to-end encryption, please wait… [NOTICED][INFO] +Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#13πŸ”’: (Contact#Contact#10): Member Me added by Alice. [FRESH][INFO] +-------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_alice1 b/test-data/golden/test_sync_broadcast_alice1 new file mode 100644 index 0000000000..3230fc1cb9 --- /dev/null +++ b/test-data/golden/test_sync_broadcast_alice1 @@ -0,0 +1,7 @@ +OutBroadcast#Chat#10: Channel [0 member(s)] +-------------------------------------------------------------------------------- +Msg#10: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#14πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#16πŸ”’: Me (Contact#Contact#Self): hi √ +Msg#17πŸ”’: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ +-------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_alice2 b/test-data/golden/test_sync_broadcast_alice2 new file mode 100644 index 0000000000..623562b2a0 --- /dev/null +++ b/test-data/golden/test_sync_broadcast_alice2 @@ -0,0 +1,7 @@ +OutBroadcast#Chat#10: Channel [0 member(s)] +-------------------------------------------------------------------------------- +Msg#11: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#14πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#16πŸ”’: Me (Contact#Contact#Self): hi √ +Msg#17πŸ”’: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ +-------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_bob b/test-data/golden/test_sync_broadcast_bob new file mode 100644 index 0000000000..117647e813 --- /dev/null +++ b/test-data/golden/test_sync_broadcast_bob @@ -0,0 +1,8 @@ +InBroadcast#Chat#11: Channel [1 member(s)] +-------------------------------------------------------------------------------- +Msg#11: info (Contact#Contact#Info): Establishing guaranteed end-to-end encryption, please wait… [NOTICED][INFO] +Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#13πŸ”’: (Contact#Contact#10): Member Me added by alice@example.org. [FRESH][INFO] +Msg#15πŸ”’: (Contact#Contact#10): hi [FRESH] +Msg#16πŸ”’: (Contact#Contact#10): Member Me removed by alice@example.org. [FRESH][INFO] +-------------------------------------------------------------------------------- diff --git a/test-data/message/text_from_alice_encrypted.eml b/test-data/message/text_from_alice_encrypted.eml new file mode 100644 index 0000000000..6e7952e911 --- /dev/null +++ b/test-data/message/text_from_alice_encrypted.eml @@ -0,0 +1,87 @@ +Content-Type: multipart/encrypted; protocol="application/pgp-encrypted"; + boundary="1858da4ad2d3c8a9_43c3a6383df12469_9f050814a95fd805" +MIME-Version: 1.0 +From: +To: +Subject: [...] +Date: Tue, 5 Aug 2025 11:07:50 +0000 +References: <0e547a9e-0785-421b-a867-ee204695fecc@localhost> +Chat-Version: 1.0 +Autocrypt: addr=alice@example.org; prefer-encrypt=mutual; keydata=mDMEXlh13RYJKwYBBAHaRw8BAQdAzfVIAleCXMJrq8VeLlEVof6ITCviMktKjmcBKAu4m5 + C0GUFsaWNlIDxhbGljZUBleGFtcGxlLm9yZz7CkgQQFggAOgUCaJHmBRYhBC5vossjtTLXKGNLWGSw + j2Gp7ZRDAhsDAh4BBQsJCAcCBhUKCQgLAgQWAgMBAScCGQEACgkQZLCPYantlENwpwEAq3zTDP9K1u + pV6yNLz6F+ylJ9U0WFIglz/CRWEu8Ma6YBAOZxBxIEJ3QFcoYaZwNUQ7lKffFiyb0cgA7hQM2cokMN + uDgEXlh13RIKKwYBBAGXVQEFAQEHQAbtyNbLZIUBTwqeW2W5tVbrusWLJ+nTUmtF7perLbYdAwEIB8 + J4BBgWCAAgBQJokeYFAhsMFiEELm+iyyO1MtcoY0tYZLCPYantlEMACgkQZLCPYantlENQgQD8CTIi + nPoPpFmnGuLXMOBH8PEDxTL+RQJgUms3dpkj2MUA/iB3L8TEtOC4A2eu5XAHttLrF3GYo7dlTq4LfO + oJtmIC + + +--1858da4ad2d3c8a9_43c3a6383df12469_9f050814a95fd805 +Content-Type: application/pgp-encrypted; charset="utf-8" +Content-Description: PGP/MIME version identification +Content-Transfer-Encoding: 7bit + +Version: 1 + +--1858da4ad2d3c8a9_43c3a6383df12469_9f050814a95fd805 +Content-Type: application/octet-stream; name="encrypted.asc"; + charset="utf-8" +Content-Description: OpenPGP encrypted message +Content-Disposition: inline; filename="encrypted.asc"; +Content-Transfer-Encoding: 7bit + +-----BEGIN PGP MESSAGE----- + +wU4D5tq63hTeebASAQdAzFyWEue9h9wPPAcI7hz99FfwjcEvff4ctFRyEmPOgBMg +vHjt4qNpXUoFavfv2Qz2+/1/EcbNANpWQ+NsU5lal9fBwEwD49jcm8SO4yIBB/9X +qCUWtr2j4A+wCb/yVMY2vlpAnA56LAz86fksVqjjYF2rGYBpjHbNAG1OyQMKNDGQ +iIjo4PIb9OHQJx71H1M8W8Tr4U0Z9BiZqOf+VLc9EvOKNl/mADS73MV9iZiHGwDy +8evrO6IdoiGOxvyO62X+cjxpSOB607vdFeJksPOkHwmLNc5SZ/S7zMHr5Qz1qoLI +ikZdxPspJrV157VrguTVuBnoM/QtVoSBy9F/DbmXMEPbwyybG4owFiGHGvC4chQi +LwJStREmEumj8W27ZtWWYp67U1bOQtldCv9iZJDczn0sa0bpOmmdAKft8ru/6NNM +CQT/U3+zTJlTSH5hLvLv0sZ6AeV7U983n4JkFsz2t0wqmpIHrjP/Q4dJ62L8EfLm +n+3y/w1MagdbjeiCBAevclH5F/E/kL5b2wc7TXrLFbKPe9juK8xddysX3do35PGH +aXWmPDj6rM53L1lLS61Jqxof+mW6AyhIcNAOoWOgDx4dOQu0vrKFLCDVjBht9NG6 +5DxNi7yKWZfMxVd5hBdOznGMsbaw4WqT516Hj5/Xb8ZtXjneatdX6aQGtJimgEC3 +WMCqmY1n/iqa/K9auFbbfxPoMkFNZChKtje0azqmPnlvDgAzG0n80446D/xbC4UZ +zcpw7Sug6Mi2heI0/Y8uvyTtVRaO2ZxTA2dt8RTFQbunhvIze8MDrscz3TTIZds+ +TelyYEETPJbxbjT0z34oGDY3nXfNAZalnmceHCsAYOw61BdlJ/2reQyxDjuZRPn7 +kT4P3DAbYLwJ7BhMr+lTWfJVPG7wD9BMfBOAg1yF1WsUPztskQoWluvDYcNACkbA +CdsuIo3Pe0lNgUillmAZN0IZNof7SvKoxXdJKP31re8cDB9fiE4utjjtWtSkLbIe +cBY/Pu/67+ohABu5DaRQFZ918rLQo82CAiRh7Y+iHvJtixs+7BhieKPtXs/hdgyn +WpPwmu1nVXJWVdUplYZE/VWK45y4JqSMU+I+yD9uBFi5HfKM2UbE6VvxwO6yOygQ +Ry0jOjennXnPEWbIQh4i3qjqNqciGIwcwJaUDf7OdnU7OMqmGNews6wsWbLXllC8 +hVXbrIO5wgQX1CiYHOi1l1mQLjAQWQLE9KYxgs0CH7b7BsSXBcty2FF3jOJoB2LF +NKtVfI6X/m8x6v0bKQ4qw579momfrmWgPyJCkoaqTeEJLEF9PiA3IgkObthw7Pwn +lLf3ku1QZfbKWHrUDSaPgMC9/Hxwer2SMBgQpX+MSJxTsEQJTXrCraB12aj4+dYm +cC8UE0re0MrGXgOYVixI5Gsegr04vlogY0AAokZfvyxO17EA31T2ML7QJfAJv7Dg +X8/3SsABJwP2J7O/G24sj+lmVfApgHVbe4JpQ4VbH6f5Ev38p4PisFvMKDREVdJw +Mxpaa9EFHDqMCX4gGpfDt+r/xy1WvtO4Qif5meqpyD/1dj7ZGJ/TfcGp8pK413T+ +wQflh2uQeXQZu11HKtx+Hp2vADTed7Ngu08fHdfdqT09ZH0VQSaTlrAbF1ZOzk2t +Dbg1XTudlKlGdJptRpKQX7oF7Q/t0antqBybTcGyFXEWsC08L3EgSf5XoI4ZrYXk +cMuXvP/4g78na0BMOeruVSDpzciFhEc4BHJDHr3vf+g/Ch4Aytwk7ACn58APv5O4 +7Eo6+oLPhOn3B7LVnyUcAIdW5qSfLGGtxjtfdFFrSeoK6alS25JmZJFFDjpKUotS +3SFSTVxovyNKbtluGt9p2i9sXQC8Hm4tU8+RwuD09Ld27i17WCILslOouq2k2NIu +9fBiOdO301pzFLZY9cqQ+g1SX9JTobPEkQrvm1lfn5CAuElmkQuoqa10GZF0CC+D +HKbCrDHCU7G4vv5fco4bYHJBc04Q8QhxO1jMq+rxow4nbTUvuJxuyB7bEhlraskm +Z5XWdHCYd+Lzek0hg8bdJts5wntG79MfFBrnWet6a3QQdi0zwA/KL40d58lSorWU +/mfdzWCkzH5TU4s7VHiIedIiN/fSanEXP8BayNcrnUscR2Tgl6ZkxhLJ/7/O+8i5 +vtMRlUVwzVJ/0JZbP/PrE+dcMBO/0bptQadAzJX2AukxYhS5jdPMSzfjFHSWxufv +Trek577NL0J0U/bH59BK+zOwmV89oCsHyfWvpZzwM7D5gQUJBdcSBsD/riVK56Du +/FzmKHOyRXxC7joVkduLxqOrMIyETPiZ38I3xvbMnQrJo3Mxvz20c5gEmIZ1RuuI +wUenj2lxjYabFgNVCFGx5wLmwMaaLJqvrH4Z8aB7m5W5xJtAWt1ZHs2sS37YEyY/ +pKDRCF0Dunwsnzrt1i+YjvzzM0cbSkmcByGgkkKIzNjUpxpxylYL6cwZNAxne4+i +yHZAH3Cb6OoC1jAs0i2jLaQOLfKJTf1G/eV27HLTTEX7CGU0f00k4GRDcgtvQyB7 ++klDI0Uf/SrrOAEc5AY6KhvqjQRsLpOC4dDkrXTfxxm6XqDxXTk4lgH0tuSPTFRw +K1NLoMDwT2yUGchYH5HG+FptZP8gtQFBWeTzSOrINlPe+upZYMDmEtsgmxPman3v +XmVFQs4m3hG4wx3f7SPtx0/+z+AkgTUzCuudvV+oLxbbq+7ZvTcYZoe36Bm/CIgM +t97rNeC3oXS+aIHEk6LU9ER+/7eI5R7jNY/c4K111DQu7o+cM3dxF08r+iUu8lR0 +O8C0FM6a45PcOsaIanFiTgv238UCkb9vwjXrJI572tjOCKHSXrhIEweKziq1bU4q +0tDEbUG5dRZk87HI8Vh1JNei8V8Nyq6A7XfHV3WBxgNWvjUCgIx/SCzitbg= +=indJ +-----END PGP MESSAGE----- + + +--1858da4ad2d3c8a9_43c3a6383df12469_9f050814a95fd805-- + diff --git a/test-data/message/text_symmetrically_encrypted.eml b/test-data/message/text_symmetrically_encrypted.eml new file mode 100644 index 0000000000..44406e7d43 --- /dev/null +++ b/test-data/message/text_symmetrically_encrypted.eml @@ -0,0 +1,56 @@ +Content-Type: multipart/encrypted; protocol="application/pgp-encrypted"; + boundary="1858db5a667e9817_308656fa7edcc46c_c8ec5a6c260b51bc" +MIME-Version: 1.0 +From: +To: "hidden-recipients": ; +Subject: [...] +Date: Tue, 5 Aug 2025 11:27:17 +0000 +Message-ID: <5f9a3e21-fbbd-43aa-9638-1927da98b772@localhost> +References: <5f9a3e21-fbbd-43aa-9638-1927da98b772@localhost> +Chat-Version: 1.0 +Autocrypt: addr=alice@example.org; prefer-encrypt=mutual; keydata=mDMEXlh13RYJKwYBBAHaRw8BAQdAzfVIAleCXMJrq8VeLlEVof6ITCviMktKjmcBKAu4m5 + C0GUFsaWNlIDxhbGljZUBleGFtcGxlLm9yZz7CkgQQFggAOgUCaJHqlBYhBC5vossjtTLXKGNLWGSw + j2Gp7ZRDAhsDAh4BBQsJCAcCBhUKCQgLAgQWAgMBAScCGQEACgkQZLCPYantlEM55QD9H8bPo4J8Yz + TlMuMQms7o7rW89FYX+WH//0IDbfgWysAA/2lDEwfcP0ufyJPvUMGUi62JcFS9LBwS0riKGpC6hiMM + uDgEXlh13RIKKwYBBAGXVQEFAQEHQAbtyNbLZIUBTwqeW2W5tVbrusWLJ+nTUmtF7perLbYdAwEIB8 + J4BBgWCAAgBQJokeqUAhsMFiEELm+iyyO1MtcoY0tYZLCPYantlEMACgkQZLCPYantlEPdsAEA8cjS + XsAtWnQtW6m7Yn53j5Wk+jl5b3plydWhh8kk8uAA/2gx7wuDYDW9V32NdacJFV2H7UtItsTjN3qp8f + l00TQB + + +--1858db5a667e9817_308656fa7edcc46c_c8ec5a6c260b51bc +Content-Type: application/pgp-encrypted; charset="utf-8" +Content-Description: PGP/MIME version identification +Content-Transfer-Encoding: 7bit + +Version: 1 + +--1858db5a667e9817_308656fa7edcc46c_c8ec5a6c260b51bc +Content-Type: application/octet-stream; name="encrypted.asc"; + charset="utf-8" +Content-Description: OpenPGP encrypted message +Content-Disposition: inline; filename="encrypted.asc"; +Content-Transfer-Encoding: 7bit + +-----BEGIN PGP MESSAGE----- + +wz4GHAcCCgEI44vuKOnsZubFQrI4MW7LbfmxKq5N2VIQ8c2CIRIAnvAa3AMV3Deq +P69ilwwDCf2NRy8Xg42Dc9LBkAIHAgdRy6G2xao09tPMEBBhY9dF01x21w+MyWd4 +Hm8Qz/No8BPkvxJO8WqFmbO/U0EHMEXGpADzNjU82I1bamslr0xjohgkL7goDkKl +ZbHMV1XTrG4No57fpXZSlWKRK+cJaY9S5pdwAboHuzdxhbWf+lAT2mqntkXLAtdT +tYv0piXH5+czWFsFpJRH4egYknhO+V9kpE4QX4wnwSwDinsBqAeMawcU93V4Eso+ +JYacb9Rd6Sv3ApjB12vAQTlc5KAxSFdCRGQBFIWNAMf6X04dSrURgh/gy2AnnO4q +ViU2+o5yITN+6KXxQrfmtL+xcPY1vKiATH/n5HYo/MgkwkwCSqvC5eajuMmKqncX +4877OzvCq7ohAnZVuaQFHLJlavKNzS76Hx4AGKX8MojCzhpUfmLwcjBtmteohAJd +COxhIS6hQDrgipscFmPW7fHIlHPvz0B4G/oorMzg9sN/vu+IerCoP8DCIbVIN3eK +Nt8XZtY2bNnzzQyh6XP5E5dhHWMGFlJFA1rdnAZ6O36Vdmm5++E4oFhluOTXNKRd +XapcxtXwwHfm+294pi9P8TWpADXwH6Mt2gwhHh9BE68SstjdM29hSA89q4Kn4y8p +EEsplNl2A4ZeD2Xz868PwoLnRa1f2b5nzdeZhUtj4K2JFGbAJ6alJ5sjRZaZIxnE +rQVvpwRVgaBp9scIsKVT14czCVAYW3n4RMYB3zwTkSIoW0prWZAGlzMAjzlaspnU +zxXzeY7woy+vjRPCFJCxWRrZ20cDQzs5pnrjapxS8j72ByQ= +=SwRI +-----END PGP MESSAGE----- + + +--1858db5a667e9817_308656fa7edcc46c_c8ec5a6c260b51bc-- + From 62d7441631a0d81ee31a0e18a173c732cb239ea3 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 2 Oct 2025 18:15:18 +0200 Subject: [PATCH 02/27] test: Fix test_broadcast_multidev --- src/chat/chat_tests.rs | 58 ++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index d2f9e5d7f2..070fff0710 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -2741,51 +2741,49 @@ async fn test_broadcast_change_name() -> Result<()> { /// - Alice has multiple devices /// - Alice creates a broadcast and sends a message into it /// - Alice's second device sees the broadcast -/// - Alice adds Bob to the broadcast -/// - Synchronization is only implemented via sync messages for now, -/// which are not enabled in tests by default, -/// so, Alice's second device doesn't see the change yet. -/// `test_sync_broadcast()` tests that synchronization works via sync messages. +/// - Alice's second device changes the name, +/// Alice's first device sees the name change #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_broadcast_multidev() -> Result<()> { let mut tcm = TestContextManager::new(); - let alice0 = &tcm.alice().await; let alice1 = &tcm.alice().await; - for a in &[alice0, alice1] { + let alice2 = &tcm.alice().await; + for a in &[alice1, alice2] { a.set_config_bool(Config::SyncMsgs, true).await?; } - let bob = &tcm.bob().await; - let a0_broadcast_id = create_broadcast(alice0, "Channel".to_string()).await?; - sync(alice0, alice1).await; - let a0_broadcast_chat = Chat::load_from_db(alice0, a0_broadcast_id).await?; - set_chat_name(alice0, a0_broadcast_id, "Broadcast channel 42").await?; - let sent_msg = alice0.send_text(a0_broadcast_id, "hi").await; - let msg = alice1.recv_msg(&sent_msg).await; - let a1_broadcast_id = get_chat_id_by_grpid(alice1, &a0_broadcast_chat.grpid) + let a1_broadcast_id = create_broadcast(alice1, "Channel".to_string()).await?; + sync(alice1, alice2).await; + let a1_broadcast_chat = Chat::load_from_db(alice1, a1_broadcast_id).await?; + set_chat_name(alice1, a1_broadcast_id, "Broadcast channel 42").await?; + let sent_msg = alice1.send_text(a1_broadcast_id, "hi").await; + let msg = alice2.recv_msg(&sent_msg).await; + let a2_broadcast_id = get_chat_id_by_grpid(alice2, &a1_broadcast_chat.grpid) .await? .unwrap() .0; + assert_eq!(msg.chat_id, a2_broadcast_id); + let a2_broadcast_chat = Chat::load_from_db(alice2, a2_broadcast_id).await?; + assert_eq!(a2_broadcast_chat.get_type(), Chattype::OutBroadcast); + assert_eq!(a2_broadcast_chat.get_name(), "Broadcast channel 42"); + assert!(get_chat_contacts(alice2, a2_broadcast_id).await?.is_empty()); + + SystemTime::shift(Duration::from_secs(10)); + + tcm.section("Alice2 changes the broadcast channel name"); + set_chat_name(alice2, a2_broadcast_id, "Broadcast channel 43").await?; + + tcm.section("Alice2 sends a message"); + let sent_msg = alice2.send_text(a2_broadcast_id, "hi").await; + + tcm.section("Alice1 receives it"); + let msg = alice1.recv_msg(&sent_msg).await; assert_eq!(msg.chat_id, a1_broadcast_id); let a1_broadcast_chat = Chat::load_from_db(alice1, a1_broadcast_id).await?; assert_eq!(a1_broadcast_chat.get_type(), Chattype::OutBroadcast); - assert_eq!(a1_broadcast_chat.get_name(), "Broadcast channel 42"); + assert_eq!(a1_broadcast_chat.get_name(), "Broadcast channel 43"); assert!(get_chat_contacts(alice1, a1_broadcast_id).await?.is_empty()); - let qr = get_securejoin_qr(alice1, Some(a1_broadcast_id)) - .await - .unwrap(); - tcm.exec_securejoin_qr(bob, alice1, &qr).await; - - set_chat_name(alice1, a1_broadcast_id, "Broadcast channel 43").await?; - let sent_msg = alice1.send_text(a1_broadcast_id, "hi").await; - let msg = alice0.recv_msg(&sent_msg).await; - assert_eq!(msg.chat_id, a0_broadcast_id); - let a0_broadcast_chat = Chat::load_from_db(alice0, a0_broadcast_id).await?; - assert_eq!(a0_broadcast_chat.get_type(), Chattype::OutBroadcast); - assert_eq!(a0_broadcast_chat.get_name(), "Broadcast channel 42"); - assert!(get_chat_contacts(alice0, a0_broadcast_id).await?.is_empty()); - Ok(()) } From a434e6eb6cf380d8c184aff74136eb7a7cc7934f Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 6 Oct 2025 20:46:51 +0200 Subject: [PATCH 03/27] Fix some things, so that all tests pass sometimes. Some tests are still flaky --- deltachat-rpc-client/tests/test_securejoin.py | 6 +- deltachat-rpc-client/tests/test_something.py | 2 +- src/chat.rs | 2 +- src/chat/chat_tests.rs | 85 +++++++++++++------ src/mimefactory.rs | 24 ++---- src/receive_imf.rs | 15 ++-- src/securejoin.rs | 18 ++-- src/securejoin/bob.rs | 8 +- .../test_broadcast_joining_golden_alice | 2 +- ...test_broadcast_joining_golden_alice_direct | 2 +- .../golden/test_broadcast_joining_golden_bob | 4 +- test-data/golden/test_sync_broadcast_alice1 | 6 +- test-data/golden/test_sync_broadcast_alice2 | 6 +- test-data/golden/test_sync_broadcast_bob | 8 +- 14 files changed, 103 insertions(+), 85 deletions(-) diff --git a/deltachat-rpc-client/tests/test_securejoin.py b/deltachat-rpc-client/tests/test_securejoin.py index c93acce57a..4fee7cc61a 100644 --- a/deltachat-rpc-client/tests/test_securejoin.py +++ b/deltachat-rpc-client/tests/test_securejoin.py @@ -137,7 +137,7 @@ def test_qr_securejoin_broadcast(acfactory, all_devices_online): bob.wait_for_securejoin_joiner_success() alice_chat.send_text("Hello everyone!") - def wait_for_group_messages(ac): + def wait_for_broadcast_messages(ac): snapshot = ac.wait_for_incoming_msg().get_snapshot() assert snapshot.text == f"Member Me added by {alice.get_config('addr')}." @@ -194,7 +194,7 @@ def check_account(ac, contact, inviter_side, please_wait_info_msg=False): assert SpecialContactId.SELF in chat_contacts assert chat_snapshot.self_in_group - wait_for_group_messages(bob) + wait_for_broadcast_messages(bob) check_account(alice, alice.create_contact(bob), inviter_side=True) check_account(bob, bob.create_contact(alice), inviter_side=False, please_wait_info_msg=True) @@ -219,7 +219,7 @@ def check_account(ac, contact, inviter_side, please_wait_info_msg=False): # Start second Bob device, if it wasn't started already. bob2.start_io() bob2.wait_for_securejoin_joiner_success() - wait_for_group_messages(bob2) + wait_for_broadcast_messages(bob2) check_account(bob2, bob2.create_contact(alice), inviter_side=False) # The QR code token is synced, so alice2 must be able to handle join requests. diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index 1170576d9e..15a91f3057 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -978,7 +978,7 @@ def check_account(ac, contact, inviter_side, please_wait_info_msg=False): if not inviter_side: leave_msg = chat_msgs.pop(0).get_snapshot() - assert leave_msg.text == "You left." + assert leave_msg.text == "You left the channel." assert len(chat_msgs) == 0 diff --git a/src/chat.rs b/src/chat.rs index 17a69f37d6..ef1a05faa9 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -345,7 +345,7 @@ impl ChatId { info!( context, - "Created group/mailinglist '{}' grpid={} as {}, blocked={}, protected={create_protected}.", + "Created group/broadcast '{}' grpid={} as {}, blocked={}, protected={create_protected}.", &grpname, grpid, chat_id, diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 070fff0710..3a8fbf2b19 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -3132,36 +3132,63 @@ async fn test_leave_broadcast_multidevice() -> Result<()> { let alice_chat_id = create_broadcast(alice, "foo".to_string()).await?; let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap(); join_securejoin(bob0, &qr).await.unwrap(); - let request = bob0.pop_sent_msg().await; - // Bob must send the message only to Alice, not to Self, - // because otherwise, his second device would show a device message - // "⚠️ It seems you are using Delta Chat on multiple devices that cannot decrypt each other's outgoing messages. - // To fix this, on the older device use \"Settings / Add Second Device\" and follow the instructions." - assert_eq!(request.recipients, "alice@example.org"); + let request = bob0.pop_sent_msg().await; + assert_eq!(request.recipients, "alice@example.org bob@example.net"); alice.recv_msg_trash(&request).await; - let answer = alice.pop_sent_msg().await; - bob0.recv_msg(&answer).await; + let auth_required = alice.pop_sent_msg().await; + assert_eq!( + auth_required.recipients, + "bob@example.net alice@example.org" + ); + + bob0.recv_msg_trash(&auth_required).await; + let request_with_auth = bob0.pop_sent_msg().await; + assert_eq!( + request_with_auth.recipients, + "alice@example.org bob@example.net" + ); - // Sync Bob's verification of Alice: - sync(bob0, bob1).await; - bob1.recv_msg(&answer).await; + alice.recv_msg_trash(&request_with_auth).await; + let member_added = alice.pop_sent_msg().await; + assert_eq!( + request_with_auth.recipients, + "alice@example.org bob@example.net" + ); + + tcm.section("Bob receives the member-added message answer, and processes it"); + let rcvd = bob0.recv_msg(&member_added).await; + assert_eq!(rcvd.param.get_cmd(), SystemMessage::MemberAddedToGroup); + + tcm.section("Bob's second device also receives these messages"); + bob1.recv_msg_trash(&auth_required).await; + bob1.recv_msg_trash(&request_with_auth).await; + bob1.recv_msg(&member_added).await; // The 1:1 chat should not be visible to the user on any of the devices. // The contact should be marked as verified. check_direct_chat_is_hidden_and_contact_is_verified(alice, bob0).await; check_direct_chat_is_hidden_and_contact_is_verified(bob0, alice).await; - check_direct_chat_is_hidden_and_contact_is_verified(bob1, alice).await; + + // TODO: There is a known bug in `observe_securejoin_on_other_device()`: + // When Bob joins a group or broadcast with his first device, + // then a chat with Alice will pop up on his second device. + // When it's fixed, the following line can be uncommented, + // and the 2 following lines can be removed. + //check_direct_chat_is_hidden_and_contact_is_verified(bob1, alice).await; + let bob1_alice_contact = bob1.add_or_lookup_contact_no_key(alice).await; + assert!(bob1_alice_contact.is_verified(bob1).await.unwrap()); tcm.section("Alice sends first message to broadcast."); let sent_msg = alice.send_text(alice_chat_id, "Hello!").await; let bob0_hello = bob0.recv_msg(&sent_msg).await; + assert_eq!(bob0_hello.chat_blocked, Blocked::Not); let bob1_hello = bob1.recv_msg(&sent_msg).await; + assert_eq!(bob1_hello.chat_blocked, Blocked::Not); tcm.section("Bob leaves the broadcast channel with his first device."); let bob_chat_id = bob0_hello.chat_id; - bob_chat_id.accept(bob0).await?; remove_contact_from_chat(bob0, bob_chat_id, ContactId::SELF).await?; let leave_msg = bob0.pop_sent_msg().await; @@ -3211,13 +3238,15 @@ async fn test_only_broadcast_owner_can_send_1() -> Result<()> { let alice_chat_id = create_broadcast(alice, "foo".to_string()).await?; let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap(); - tcm.section("Bob now scans the QR code sends the request message"); + tcm.section("Bob now scans the QR code"); let bob_broadcast_id = join_securejoin(bob, &qr).await.unwrap(); let request = bob.pop_sent_msg().await; alice.recv_msg_trash(&request).await; - - tcm.section("Alice answers"); - let answer = alice.pop_sent_msg().await; + let auth_required = alice.pop_sent_msg().await; + bob.recv_msg_trash(&auth_required).await; + let request_with_auth = bob.pop_sent_msg().await; + alice.recv_msg_trash(&request_with_auth).await; + let member_added = alice.pop_sent_msg().await; tcm.section("Change Alice's fingerprint for Bob, so that she is a different contact from Bob's point of view"); let bob_alice_id = bob.add_or_lookup_contact_no_key(alice).await.id; @@ -3230,8 +3259,17 @@ async fn test_only_broadcast_owner_can_send_1() -> Result<()> { ) .await?; - tcm.section("Bob receives an answer, but it ignored because of a fingerprint mismatch"); - bob.recv_msg(&answer).await; + tcm.section("Bob receives an answer, but ignores it because of a fingerprint mismatch"); + let rcvd = bob.recv_msg(&member_added).await; + assert_eq!( + rcvd.text, + "[Error: This message was not sent by the channel owner]" + ); + assert_eq!( + rcvd.error.unwrap(), + "Error: This message was not sent by the channel owner:\n\"I added member bob@example.net.\"" + ); + assert!( load_broadcast_shared_secret(bob, bob_broadcast_id) .await? @@ -3254,24 +3292,19 @@ async fn test_only_broadcast_owner_can_send_2() -> Result<()> { tcm.section("Alice creates broadcast channel and creates a QR code."); let alice_broadcast_id = create_broadcast(alice, "foo".to_string()).await?; + let qr = get_securejoin_qr(alice, Some(alice_broadcast_id)) .await .unwrap(); tcm.section("Bob now scans the QR code"); - let bob_broadcast_id = join_securejoin(bob, &qr).await.unwrap(); - let request = bob.pop_sent_msg().await; - alice.recv_msg_trash(&request).await; - let answer = alice.pop_sent_msg().await; + let bob_broadcast_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; - tcm.section("Bob receives an answer, and processes it"); - let rcvd = bob.recv_msg(&answer).await; assert!( load_broadcast_shared_secret(bob, bob_broadcast_id) .await? .is_some() ); - assert_eq!(rcvd.param.get_cmd(), SystemMessage::MemberAddedToGroup); tcm.section("Alice sends a message, which still arrives fine"); let sent = alice.send_text(alice_broadcast_id, "Hi").await; diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 84c5e092e2..541a21f40d 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -579,13 +579,11 @@ impl MimeFactory { // messages are auto-sent unlike usual unencrypted messages. step == "vg-request-with-auth" || step == "vc-request-with-auth" - || step == "vb-request-with-auth" - // Note that for "vg-member-added" and "vb-member-added", + // Note that for "vg-member-added" // get_cmd() returns `MemberAddedToGroup` rather than `SecurejoinMessage`, // so, it wouldn't actually be necessary to have them in the list here. // Still, they are here for completeness. || step == "vg-member-added" - || step == "vb-member-added" || step == "vc-contact-confirm" } } @@ -828,7 +826,7 @@ impl MimeFactory { } else if let Loaded::Message { msg, .. } = &self.loaded { if msg.param.get_cmd() == SystemMessage::SecurejoinMessage { let step = msg.param.get(Param::Arg).unwrap_or_default(); - if step != "vg-request" && step != "vc-request" && step != "vb-request" { + if step != "vg-request" && step != "vc-request" { headers.push(( "Auto-Submitted", mail_builder::headers::raw::Raw::new("auto-replied".to_string()).into(), @@ -1476,12 +1474,8 @@ impl MimeFactory { )); } if 0 != msg.param.get_int(Param::Arg2).unwrap_or_default() & DC_FROM_HANDSHAKE { - let step = match chat.typ { - Chattype::Group => "vg-member-added", - Chattype::OutBroadcast => "vb-member-added", - _ => bail!("Wrong chattype {}", chat.typ), - }; - info!(context, "Sending secure-join message {:?}.", step,); + let step = "vg-member-added"; + info!(context, "Sending secure-join message {:?}.", step); headers.push(( "Secure-Join", mail_builder::headers::raw::Raw::new(step.to_string()).into(), @@ -1560,10 +1554,7 @@ impl MimeFactory { let param2 = msg.param.get(Param::Arg2).unwrap_or_default(); if !param2.is_empty() { headers.push(( - if step == "vg-request-with-auth" - || step == "vc-request-with-auth" - || step == "vb-request-with-auth" - { + if step == "vg-request-with-auth" || step == "vc-request-with-auth" { "Secure-Join-Auth" } else { "Secure-Join-Invitenumber" @@ -1902,9 +1893,8 @@ fn hidden_recipients() -> Address<'static> { fn should_encrypt_with_broadcast_secret(msg: &Message, chat: &Chat) -> bool { chat.typ == Chattype::OutBroadcast - // The only `SystemMessage::SecurejoinMessage` that is ever sent into a broadcast, - // which is `vb-request-with-auth`, - // should be encrypted with the AUTH token rather than the broadcast secret. + // Securejoin messages that are sent into a broadcast + // are encrypted asymmetrically: && msg.param.get_cmd() != SystemMessage::SecurejoinMessage // The member-added message in a broadcast must be asymmetrically encrypted, // because the newly-added member doesn't know the broadcast shared secret yet: diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 34d6dd587b..3c68a7d9aa 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -710,12 +710,13 @@ pub(crate) async fn receive_imf_inner( handle_securejoin_handshake(context, &mut mime_parser, from_id) .await .context("error in Secure-Join message handling")? - } else { - let to_id = to_ids.first().copied().flatten().unwrap_or(ContactId::SELF); + } else if let Some(to_id) = to_ids.first().copied().flatten() { // handshake may mark contacts as verified and must be processed before chats are created observe_securejoin_on_other_device(context, &mime_parser, to_id) .await .context("error in Secure-Join watching")? + } else { + securejoin::HandshakeMessage::Propagate }; match res { @@ -3513,7 +3514,12 @@ async fn apply_out_broadcast_changes( .await?; } - if let Some(removed_fpr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemovedFpr) { + if let Some(_added_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberAdded) { + // This message can be safely ignored, + // because the only way to add a member is by having them scan a QR code. + // All devices will receive Bob's vg-request-with-auth message and add him to the channel. + better_msg.get_or_insert("".to_string()); + } else if let Some(removed_fpr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemovedFpr) { send_event_chat_modified = true; let removed_id = lookup_key_contact_by_fingerprint(context, removed_fpr).await?; if removed_id == Some(from_id) { @@ -3533,9 +3539,6 @@ async fn apply_out_broadcast_changes( } } } - // No need to check for ChatGroupMemberAdded: - // The only way to add a member is by having them scan a QR code. - // All devices will receive Bob's vb-request-with-auth message and add him to the channel. if send_event_chat_modified { context.emit_event(EventType::ChatModified(chat.id)); diff --git a/src/securejoin.rs b/src/securejoin.rs index ce43495227..c65e8756b7 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -369,8 +369,7 @@ pub(crate) async fn handle_securejoin_handshake( ========================================================*/ bob::handle_auth_required(context, mime_message).await } - // TODO possibly rename vb-request* to vg-request* or vc-request* - "vg-request-with-auth" | "vc-request-with-auth" | "vb-request-with-auth" => { + "vg-request-with-auth" | "vc-request-with-auth" => { /*========================================================== ==== Alice - the inviter side ==== ==== Steps 5+6 in "Setup verified contact" protocol ==== @@ -450,7 +449,7 @@ pub(crate) async fn handle_securejoin_handshake( let chat = Chat::load_from_db(context, joining_chat_id).await?; inviter_progress(context, contact_id, joining_chat_id, chat.typ)?; - if chat.typ == Chattype::InBroadcast { + if chat.typ == Chattype::OutBroadcast { // For broadcasts, we don't want to delete the message, // because the other device should also internally add the member // and see the key (because it won't see the member via autocrypt-gossip). @@ -489,7 +488,7 @@ pub(crate) async fn handle_securejoin_handshake( }); Ok(HandshakeMessage::Ignore) } - "vg-member-added" | "vb-member-added" => { + "vg-member-added" => { let Some(member_added) = mime_message.get_header(HeaderDef::ChatGroupMemberAdded) else { warn!( @@ -556,15 +555,8 @@ pub(crate) async fn observe_securejoin_on_other_device( if !matches!( step, - "vg-request-with-auth" - | "vb-request-with-auth" - | "vc-request-with-auth" - | "vg-member-added" - | "vc-contact-confirm" + "vg-request-with-auth" | "vc-request-with-auth" | "vg-member-added" | "vc-contact-confirm" ) { - // `vb-member-added` can be ignored - // because all devices receive the `vb-request-with-auth` message - // and mark Bob as verified because of this. return Ok(HandshakeMessage::Ignore); }; @@ -607,7 +599,7 @@ pub(crate) async fn observe_securejoin_on_other_device( { Chattype::Single } else if mime_message.get_header(HeaderDef::ListId).is_some() { - Chattype::InBroadcast + Chattype::OutBroadcast } else { Chattype::Group }; diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index ebdc5261b7..05659cd295 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -326,9 +326,9 @@ pub(crate) async fn send_handshake_message( /// Identifies the SecureJoin handshake messages Bob can send. pub(crate) enum BobHandshakeMsg { - /// vc-request, vg-request, or vb-request + /// vc-request or vg-request Request, - /// vc-request-with-auth, vg-request-with-auth, or vb-request-with-auth + /// vc-request-with-auth or vg-request-with-auth RequestWithAuth, } @@ -352,12 +352,12 @@ impl BobHandshakeMsg { Self::Request => match invite { QrInvite::Contact { .. } => "vc-request", QrInvite::Group { .. } => "vg-request", - QrInvite::Broadcast { .. } => "vb-request", + QrInvite::Broadcast { .. } => "vg-request", }, Self::RequestWithAuth => match invite { QrInvite::Contact { .. } => "vc-request-with-auth", QrInvite::Group { .. } => "vg-request-with-auth", - QrInvite::Broadcast { .. } => "vb-request-with-auth", + QrInvite::Broadcast { .. } => "vg-request-with-auth", }, } } diff --git a/test-data/golden/test_broadcast_joining_golden_alice b/test-data/golden/test_broadcast_joining_golden_alice index d436657278..2077f09c9e 100644 --- a/test-data/golden/test_broadcast_joining_golden_alice +++ b/test-data/golden/test_broadcast_joining_golden_alice @@ -2,5 +2,5 @@ OutBroadcast#Chat#10: My Channel [1 member(s)] Icon: e9b6c7a78aa2e4f415644f55a55 -------------------------------------------------------------------------------- Msg#10: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] Msg#11πŸ”’: Me (Contact#Contact#Self): You changed the group image. [INFO] √ -Msg#13πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#15πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_broadcast_joining_golden_alice_direct b/test-data/golden/test_broadcast_joining_golden_alice_direct index 68fb5acf17..ede3724ea3 100644 --- a/test-data/golden/test_broadcast_joining_golden_alice_direct +++ b/test-data/golden/test_broadcast_joining_golden_alice_direct @@ -1,4 +1,4 @@ Single#Chat#11: bob@example.net [KEY bob@example.net] πŸ›‘οΈ -------------------------------------------------------------------------------- -Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO πŸ›‘οΈ] +Msg#14: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO πŸ›‘οΈ] -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_broadcast_joining_golden_bob b/test-data/golden/test_broadcast_joining_golden_bob index c184cc9256..83c72bec7c 100644 --- a/test-data/golden/test_broadcast_joining_golden_bob +++ b/test-data/golden/test_broadcast_joining_golden_bob @@ -1,6 +1,6 @@ InBroadcast#Chat#11: My Channel [2 member(s)] Icon: e9b6c7a78aa2e4f415644f55a553e73.png -------------------------------------------------------------------------------- Msg#11: info (Contact#Contact#Info): Establishing guaranteed end-to-end encryption, please wait… [NOTICED][INFO] -Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#13πŸ”’: (Contact#Contact#10): Member Me added by Alice. [FRESH][INFO] +Msg#15: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#16πŸ”’: (Contact#Contact#10): Member Me added by Alice. [FRESH][INFO] -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_alice1 b/test-data/golden/test_sync_broadcast_alice1 index 3230fc1cb9..0bc41a2dde 100644 --- a/test-data/golden/test_sync_broadcast_alice1 +++ b/test-data/golden/test_sync_broadcast_alice1 @@ -1,7 +1,7 @@ OutBroadcast#Chat#10: Channel [0 member(s)] -------------------------------------------------------------------------------- Msg#10: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#14πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -Msg#16πŸ”’: Me (Contact#Contact#Self): hi √ -Msg#17πŸ”’: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ +Msg#16πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#18πŸ”’: Me (Contact#Contact#Self): hi √ +Msg#19πŸ”’: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_alice2 b/test-data/golden/test_sync_broadcast_alice2 index 623562b2a0..28a8cc10af 100644 --- a/test-data/golden/test_sync_broadcast_alice2 +++ b/test-data/golden/test_sync_broadcast_alice2 @@ -1,7 +1,7 @@ OutBroadcast#Chat#10: Channel [0 member(s)] -------------------------------------------------------------------------------- Msg#11: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#14πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -Msg#16πŸ”’: Me (Contact#Contact#Self): hi √ -Msg#17πŸ”’: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ +Msg#16πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#18πŸ”’: Me (Contact#Contact#Self): hi √ +Msg#19πŸ”’: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_bob b/test-data/golden/test_sync_broadcast_bob index 117647e813..052b2e9a60 100644 --- a/test-data/golden/test_sync_broadcast_bob +++ b/test-data/golden/test_sync_broadcast_bob @@ -1,8 +1,8 @@ InBroadcast#Chat#11: Channel [1 member(s)] -------------------------------------------------------------------------------- Msg#11: info (Contact#Contact#Info): Establishing guaranteed end-to-end encryption, please wait… [NOTICED][INFO] -Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#13πŸ”’: (Contact#Contact#10): Member Me added by alice@example.org. [FRESH][INFO] -Msg#15πŸ”’: (Contact#Contact#10): hi [FRESH] -Msg#16πŸ”’: (Contact#Contact#10): Member Me removed by alice@example.org. [FRESH][INFO] +Msg#16: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#17πŸ”’: (Contact#Contact#10): Member Me added by alice@example.org. [FRESH][INFO] +Msg#19πŸ”’: (Contact#Contact#10): hi [FRESH] +Msg#20πŸ”’: (Contact#Contact#10): Member Me removed by alice@example.org. [FRESH][INFO] -------------------------------------------------------------------------------- From 17fdac2a35ccaee287ca307550e7b680e1c9c150 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 6 Oct 2025 20:51:57 +0200 Subject: [PATCH 04/27] clippy --- src/pgp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pgp.rs b/src/pgp.rs index 43a9e223a0..405fa9e41a 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -26,7 +26,7 @@ use crate::key::{DcKey, Fingerprint}; #[cfg(test)] pub(crate) const HEADER_AUTOCRYPT: &str = "autocrypt-prefer-encrypt"; -pub const HEADER_SETUPCODE: &str = "passphrase-begin"; +pub(crate) const HEADER_SETUPCODE: &str = "passphrase-begin"; /// Preferred symmetric encryption algorithm. const SYMMETRIC_KEY_ALGORITHM: SymmetricKeyAlgorithm = SymmetricKeyAlgorithm::AES128; From 8b3b597d601cc4dc0c7902126ee7616cbe1c8e1d Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 6 Oct 2025 21:59:47 +0200 Subject: [PATCH 05/27] test: Fix the flaky tests. All tests pass now! --- deltachat-rpc-client/tests/test_securejoin.py | 13 +++++++++++-- deltachat-rpc-client/tests/test_something.py | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/deltachat-rpc-client/tests/test_securejoin.py b/deltachat-rpc-client/tests/test_securejoin.py index 4fee7cc61a..f618393b22 100644 --- a/deltachat-rpc-client/tests/test_securejoin.py +++ b/deltachat-rpc-client/tests/test_securejoin.py @@ -137,19 +137,28 @@ def test_qr_securejoin_broadcast(acfactory, all_devices_online): bob.wait_for_securejoin_joiner_success() alice_chat.send_text("Hello everyone!") + def get_broadcast(ac): + chat = ac.get_chatlist(query="Broadcast channel for everyone!")[0] + assert chat.get_basic_snapshot().name == "Broadcast channel for everyone!" + return chat + def wait_for_broadcast_messages(ac): + chat = get_broadcast(ac) + snapshot = ac.wait_for_incoming_msg().get_snapshot() assert snapshot.text == f"Member Me added by {alice.get_config('addr')}." + assert snapshot.chat_id == chat.id snapshot = ac.wait_for_incoming_msg().get_snapshot() assert snapshot.text == "Hello everyone!" + assert snapshot.chat_id == chat.id def check_account(ac, contact, inviter_side, please_wait_info_msg=False): # Check that the chat partner is verified. contact_snapshot = contact.get_snapshot() assert contact_snapshot.is_verified - chat = ac.get_chatlist()[0] + chat = get_broadcast(ac) chat_msgs = chat.get_messages() if please_wait_info_msg: @@ -232,7 +241,7 @@ def check_account(ac, contact, inviter_side, please_wait_info_msg=False): snapshot = fiona.wait_for_incoming_msg().get_snapshot() assert snapshot.text == f"Member Me added by {alice.get_config('addr')}." - alice2.get_chatlist()[0].get_messages()[2].resend() + get_broadcast(alice2).get_messages()[2].resend() snapshot = fiona.wait_for_incoming_msg().get_snapshot() assert snapshot.text == "Hello everyone!" diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index 15a91f3057..70a6309d69 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -951,7 +951,7 @@ def test_leave_broadcast(acfactory, all_devices_online): assert member_added_msg.get_snapshot().text == f"Member Me added by {alice.get_config('addr')}." def get_broadcast(ac): - chat = ac.get_chatlist()[0] + chat = ac.get_chatlist(query="Broadcast channel for everyone!")[0] assert chat.get_basic_snapshot().name == "Broadcast channel for everyone!" return chat From 915bad92c32823f1224ef41e81b644110ab225c6 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 7 Oct 2025 11:52:11 +0200 Subject: [PATCH 06/27] refactor: Remove unused MarkVerified sync action --- src/chat.rs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 0d416c70cd..b6808acfce 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -5111,9 +5111,6 @@ pub(crate) enum SyncAction { chat_name: String, shared_secret: String, }, - /// Mark the contact with the given fingerprint as verified by self. - // TODO check if I can remove this - MarkVerified, Rename(String), /// Set chat contacts by their addresses. SetContacts(Vec), @@ -5169,16 +5166,6 @@ impl Context { SyncAction::Unblock => { return contact::set_blocked(self, Nosync, contact_id, false).await; } - SyncAction::MarkVerified => { - ContactId::scaleup_origin(self, &[contact_id], Origin::SecurejoinJoined) - .await?; - return contact::mark_contact_id_as_verified( - self, - contact_id, - Some(ContactId::SELF), - ) - .await; - } _ => (), } ChatIdBlocked::get_for_contact(self, contact_id, Blocked::Request) @@ -5210,9 +5197,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::CreateOutBroadcast { .. } | SyncAction::MarkVerified => { + SyncAction::CreateOutBroadcast { .. } => { // Create action should have been handled by handle_sync_create_chat() already. - // MarkVerified action should have been handled by mark_contact_id_as_verified() already. Err(anyhow!("sync_alter_chat({id:?}, {action:?}): Bad request.")) } SyncAction::Rename(to) => rename_ex(self, Nosync, chat_id, to).await, From d755f61e0a492223138651a4fdc09e6518e51b03 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 7 Oct 2025 12:25:01 +0200 Subject: [PATCH 07/27] improve comment --- src/chat/chat_tests.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 578139db8b..6d8640e295 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -3174,9 +3174,8 @@ async fn test_leave_broadcast_multidevice() -> Result<()> { // TODO: There is a known bug in `observe_securejoin_on_other_device()`: // When Bob joins a group or broadcast with his first device, // then a chat with Alice will pop up on his second device. - // When it's fixed, the following line can be uncommented, - // and the 2 following lines can be removed. - //check_direct_chat_is_hidden_and_contact_is_verified(bob1, alice).await; + // When it's fixed, the 2 following lines can be replaced with + // `check_direct_chat_is_hidden_and_contact_is_verified(bob1, alice).await;` let bob1_alice_contact = bob1.add_or_lookup_contact_no_key(alice).await; assert!(bob1_alice_contact.is_verified(bob1).await.unwrap()); From e00106781bee164faabf8e76fb7048fd46014de1 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 7 Oct 2025 18:42:20 +0200 Subject: [PATCH 08/27] Some things I noticed while self-reviewing --- src/chat/chat_tests.rs | 64 +++++++++++++++++-- src/tools.rs | 2 +- ...est_broadcast_joining_golden_private_chat} | 0 3 files changed, 61 insertions(+), 5 deletions(-) rename test-data/golden/{test_broadcast_joining_golden_alice_direct => test_broadcast_joining_golden_private_chat} (100%) diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 6d8640e295..4c6e9edfe4 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -2903,13 +2903,16 @@ async fn test_broadcast_joining_golden() -> Result<()> { .await; let alice_bob_contact = alice.add_or_lookup_contact_no_key(bob).await; - let direct_chat = ChatIdBlocked::lookup_by_contact(alice, alice_bob_contact.id) + let private_chat = ChatIdBlocked::lookup_by_contact(alice, alice_bob_contact.id) .await? .unwrap(); // The 1:1 chat with Bob should not be visible to the user: - assert_eq!(direct_chat.blocked, Blocked::Yes); + assert_eq!(private_chat.blocked, Blocked::Yes); alice - .golden_test_chat(direct_chat.id, "test_broadcast_joining_golden_alice_direct") + .golden_test_chat( + private_chat.id, + "test_broadcast_joining_golden_private_chat", + ) .await; assert_eq!( @@ -3339,6 +3342,59 @@ async fn test_only_broadcast_owner_can_send_2() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_sync_broadcast_avatar_and_name() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice1 = &tcm.alice().await; + let alice2 = &tcm.alice().await; + + alice1.set_config_bool(Config::SyncMsgs, true).await?; + alice2.set_config_bool(Config::SyncMsgs, true).await?; + + tcm.section("Alice1 creates broadcast channel."); + let a1_broadcast_id = create_broadcast(alice1, "foo".to_string()).await?; + + tcm.section("The channel syncs to her second device"); + sync(alice1, alice2).await; + + let a1_broadcast_chat = Chat::load_from_db(alice1, a1_broadcast_id).await?; + let a2_broadcast_id = get_chat_id_by_grpid(alice2, &a1_broadcast_chat.grpid) + .await? + .unwrap() + .0; + let a2_broadcast_chat = Chat::load_from_db(alice2, a2_broadcast_id).await?; + assert_eq!(a2_broadcast_chat.get_name(), "foo".to_string()); + + set_chat_name(&alice1, a1_broadcast_id, "New name").await?; + let sent = alice1.pop_sent_msg().await; + let rcvd = alice2.recv_msg(&sent).await; + assert_eq!(rcvd.chat_id, a2_broadcast_id); + assert_eq!(rcvd.param.get_cmd(), SystemMessage::GroupNameChanged); + assert_eq!( + rcvd.text, + r#"You changed group name from "foo" to "New name"."# + ); + + let a2_broadcast_chat = Chat::load_from_db(alice2, a2_broadcast_id).await?; + assert_eq!(a2_broadcast_chat.get_name(), "New name".to_string()); + + let file = alice2.get_blobdir().join("avatar.png"); + tokio::fs::write(&file, AVATAR_64x64_BYTES).await?; + set_chat_profile_image(alice2, a2_broadcast_id, file.to_str().unwrap()).await?; + + let sent = alice2.pop_sent_msg().await; + let rcvd = alice1.recv_msg(&sent).await; + assert_eq!(rcvd.chat_id, a1_broadcast_id); + assert_eq!(rcvd.param.get_cmd(), SystemMessage::GroupImageChanged); + assert_eq!(rcvd.text, "You changed the group image."); + + let a1_broadcast_chat = Chat::load_from_db(alice1, a1_broadcast_id).await?; + let avatar = a1_broadcast_chat.get_profile_image(alice1).await?.unwrap(); + assert_eq!(avatar.file_name().unwrap(), AVATAR_64x64_DEDUPLICATED); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_encrypt_decrypt_broadcast() -> Result<()> { let mut tcm = TestContextManager::new(); @@ -4099,7 +4155,7 @@ async fn test_sync_muted() -> Result<()> { /// Tests that synchronizing broadcast channels via sync-messages works #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_sync_broadcast() -> Result<()> { +async fn test_sync_broadcast_and_send_message() -> Result<()> { let mut tcm = TestContextManager::new(); let alice1 = &tcm.alice().await; let alice2 = &tcm.alice().await; diff --git a/src/tools.rs b/src/tools.rs index a9b335d4c2..7954796a61 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -304,7 +304,7 @@ pub(crate) fn create_id() -> String { /// /// The string generated by this function has 258 bits of entropy /// and is returned as 43 Base64 characters, each containing 6 bits of entropy. -/// 256 is chosen because we may switch to AES-256 keys in the future, +/// 258 is chosen because we may switch to AES-256 keys in the future, /// and so that the shared secret definitely won't be the weak spot. pub(crate) fn create_broadcast_shared_secret() -> String { // ThreadRng implements CryptoRng trait and is supposed to be cryptographically secure. diff --git a/test-data/golden/test_broadcast_joining_golden_alice_direct b/test-data/golden/test_broadcast_joining_golden_private_chat similarity index 100% rename from test-data/golden/test_broadcast_joining_golden_alice_direct rename to test-data/golden/test_broadcast_joining_golden_private_chat From c60258644ea171737a1b5a16978afda01ba74cbb Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 7 Oct 2025 19:13:50 +0200 Subject: [PATCH 09/27] Small comment improvements --- src/receive_imf.rs | 1 + src/securejoin/bob.rs | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 3c68a7d9aa..cd45810e3f 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1576,6 +1576,7 @@ async fn do_chat_assignment( } else { let name = compute_mailinglist_name(mailinglist_header, &listid, mime_parser); + // TODO what? We can't just choose a secret! let secret = create_broadcast_shared_secret(); chat::create_out_broadcast_ex(context, Nosync, listid, name, secret) .await? diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 05659cd295..ba942dd5cf 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -110,8 +110,8 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul match invite { QrInvite::Group { .. } => { - // For a secure-join we need to create the group and add the contact. The group will - // only become usable once the protocol is finished. + // We created the group already, now we need to add Alice to the group. + // The group will only become usable once the protocol is finished. if !is_contact_in_chat(context, joining_chat_id, invite.contact_id()).await? { chat::add_to_chat_contacts_table( context, @@ -126,6 +126,7 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul Ok(joining_chat_id) } QrInvite::Broadcast { .. } => { + // We created the broadcast channel already, now we need to add Alice to the group. if !is_contact_in_chat(context, joining_chat_id, invite.contact_id()).await? { chat::add_to_chat_contacts_table( context, @@ -136,6 +137,9 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul .await?; } + // If we were not in the broadcast channel before, show a 'please wait' info message. + // Since we don't have any specific stock string for this, + // use the generic `Establishing guaranteed end-to-end encryption, please wait…` if !is_contact_in_chat(context, joining_chat_id, ContactId::SELF).await? { let msg = stock_str::securejoin_wait(context).await; chat::add_info_msg(context, joining_chat_id, &msg, time()).await?; @@ -236,7 +240,7 @@ pub(super) async fn handle_auth_required( QrInvite::Contact { .. } | QrInvite::Broadcast { .. } => {} QrInvite::Group { .. } => { // The message reads "Alice replied, waiting to be added to the group…", - // so only show it on secure-join and not on setup-contact. + // so only show it when joining a group and not for a 1:1 chat or broadcast channel. let contact_id = invite.contact_id(); let msg = stock_str::secure_join_replies(context, contact_id).await; let chat_id = joining_chat_id(context, &invite, chat_id).await?; From 7d92fcd9c6b30fd3fa7fa21ee9809524a5418da5 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 7 Oct 2025 19:20:18 +0200 Subject: [PATCH 10/27] fix: Don't invent a secret when receiving an outgoing broadcast message --- src/receive_imf.rs | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index cd45810e3f..0b2efeb48c 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1569,19 +1569,33 @@ async fn do_chat_assignment( // (it can't be a mailing list, since it's outgoing) if let Some(mailinglist_header) = mime_parser.get_mailinglist_header() { let listid = mailinglist_header_listid(mailinglist_header)?; - chat_id = Some( - if let Some((id, ..)) = chat::get_chat_id_by_grpid(context, &listid).await? + if let Some((id, ..)) = chat::get_chat_id_by_grpid(context, &listid).await? { + chat_id = Some(id); + } else { + // Looks like we missed the sync message that was creating this broadcast channel + let name = + compute_mailinglist_name(mailinglist_header, &listid, mime_parser); + if let Some(secret) = mime_parser + .get_header(HeaderDef::ChatBroadcastSecret) + .filter(|s| validate_broadcast_shared_secret(s)) { - id + chat_id = Some( + chat::create_out_broadcast_ex( + context, + Nosync, + listid, + name, + secret.to_string(), + ) + .await?, + ); } else { - let name = - compute_mailinglist_name(mailinglist_header, &listid, mime_parser); - // TODO what? We can't just choose a secret! - let secret = create_broadcast_shared_secret(); - chat::create_out_broadcast_ex(context, Nosync, listid, name, secret) - .await? - }, - ); + warn!( + context, + "Not creating outgoing broadcast, because secret is unknown" + ); + } + } } } ChatAssignment::AdHocGroup => { From faa149aea5425b3ba23e93b2576be4635992b968 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 8 Oct 2025 14:28:10 +0200 Subject: [PATCH 11/27] small comment improvement --- src/receive_imf.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 0b2efeb48c..7e71d1a4d9 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -3322,7 +3322,7 @@ async fn create_or_lookup_mailinglist_or_broadcast( }; if allow_creation { - // list does not exist but should be created + // Broadcast channel / mailinglist does not exist but should be created let param = mime_parser.list_post.as_ref().map(|list_post| { let mut p = Params::new(); p.set(Param::ListPost, list_post); From 5a91887ec2f94935ab5c24e68372dd9d606b6511 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 8 Oct 2025 15:05:30 +0200 Subject: [PATCH 12/27] more comment improvements --- deltachat-ffi/deltachat.h | 9 ++++++--- deltachat-jsonrpc/src/api/types/qr.rs | 2 +- src/imex/key_transfer.rs | 2 +- src/param.rs | 2 +- src/pgp.rs | 6 +++--- src/qr.rs | 1 - src/receive_imf.rs | 2 +- 7 files changed, 13 insertions(+), 11 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 053f5fc120..88bfa8f396 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -2565,6 +2565,7 @@ void dc_stop_ongoing_process (dc_context_t* context); #define DC_QR_ASK_VERIFYCONTACT 200 // id=contact #define DC_QR_ASK_VERIFYGROUP 202 // text1=groupname +#define DC_QR_ASK_VERIFYBROADCAST 204 #define DC_QR_FPR_OK 210 // id=contact #define DC_QR_FPR_MISMATCH 220 // id=contact #define DC_QR_FPR_WITHOUT_ADDR 230 // test1=formatted fingerprint @@ -2597,7 +2598,8 @@ void dc_stop_ongoing_process (dc_context_t* context); * ask whether to verify the contact; * if so, start the protocol with dc_join_securejoin(). * - * - DC_QR_ASK_VERIFYGROUP with dc_lot_t::text1=Group name: + * - DC_QR_ASK_VERIFYGROUP or DC_QR_ASK_VERIFYBROADCAST + * with dc_lot_t::text1=Group name: * ask whether to join the group; * if so, start the protocol with dc_join_securejoin(). * @@ -2681,7 +2683,8 @@ dc_lot_t* dc_check_qr (dc_context_t* context, const char* * Get QR code text that will offer an Setup-Contact or Verified-Group invitation. * * The scanning device will pass the scanned content to dc_check_qr() then; - * if dc_check_qr() returns DC_QR_ASK_VERIFYCONTACT or DC_QR_ASK_VERIFYGROUP + * if dc_check_qr() returns + * DC_QR_ASK_VERIFYCONTACT, DC_QR_ASK_VERIFYGROUP or DC_QR_ASK_VERIFYBROADCAST * an out-of-band-verification can be joined using dc_join_securejoin() * * The returned text will also work as a normal https:-link, @@ -2722,7 +2725,7 @@ char* dc_get_securejoin_qr_svg (dc_context_t* context, uint32_ * Continue a Setup-Contact or Verified-Group-Invite protocol * started on another device with dc_get_securejoin_qr(). * This function is typically called when dc_check_qr() returns - * lot.state=DC_QR_ASK_VERIFYCONTACT or lot.state=DC_QR_ASK_VERIFYGROUP. + * lot.state=DC_QR_ASK_VERIFYCONTACT, lot.state=DC_QR_ASK_VERIFYGROUP or lot.state=DC_QR_ASK_VERIFYBROADCAST * * The function returns immediately and the handshake runs in background, * sending and receiving several messages. diff --git a/deltachat-jsonrpc/src/api/types/qr.rs b/deltachat-jsonrpc/src/api/types/qr.rs index 51d8a5ec52..005bbeebe3 100644 --- a/deltachat-jsonrpc/src/api/types/qr.rs +++ b/deltachat-jsonrpc/src/api/types/qr.rs @@ -36,7 +36,7 @@ pub enum QrObject { }, /// Ask the user whether to join the broadcast channel. AskJoinBroadcast { - /// Chat name. + /// The user-visible name of this broadcast channel broadcast_name: String, /// A string of random characters, /// uniquely identifying this broadcast channel in the database. diff --git a/src/imex/key_transfer.rs b/src/imex/key_transfer.rs index c995094fa0..4d1d3d1044 100644 --- a/src/imex/key_transfer.rs +++ b/src/imex/key_transfer.rs @@ -95,7 +95,7 @@ pub async fn render_setup_file(context: &Context, passphrase: &str) -> Result) -> Result { +/// Symmetric encryption for the autocrypt setup message (ASM). +pub async fn symm_encrypt_autocrypt_setup(passphrase: &str, plain: Vec) -> Result { let passphrase = Password::from(passphrase.to_string()); tokio::task::spawn_blocking(move || { diff --git a/src/qr.rs b/src/qr.rs index 39b9634ce1..670b318586 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -495,7 +495,6 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { }) } } else if let (Some(grpid), Some(broadcast_name)) = (grpid, broadcast_name) { - // This is a broadcast channel invite link. Ok(Qr::AskJoinBroadcast { broadcast_name, grpid, diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 7e71d1a4d9..dbd12998b5 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -3532,7 +3532,7 @@ async fn apply_out_broadcast_changes( if let Some(_added_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberAdded) { // This message can be safely ignored, // because the only way to add a member is by having them scan a QR code. - // All devices will receive Bob's vg-request-with-auth message and add him to the channel. + // All devices will receive the vg-request-with-auth message and add him to the channel. better_msg.get_or_insert("".to_string()); } else if let Some(removed_fpr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemovedFpr) { send_event_chat_modified = true; From 41b4953f11e978ec8e32f579a3f60deab8ee3064 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 8 Oct 2025 15:20:47 +0200 Subject: [PATCH 13/27] more tweaks --- src/chat.rs | 4 +++- src/mimefactory.rs | 7 +++---- src/param.rs | 4 ++++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index b6808acfce..2b8d592027 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -50,6 +50,8 @@ use crate::tools::{ use crate::webxdc::StatusUpdateSerial; use crate::{chatlist_events, imap}; +pub(crate) const PARAM_BROADCAST_SHARED_SECRET: Param = Param::Arg3; + /// An chat item, such as a message or a marker. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum ChatItem { @@ -4071,7 +4073,7 @@ pub(crate) async fn add_contact_to_chat_ex( let secret = load_broadcast_shared_secret(context, chat_id) .await? .context("Failed to find broadcast shared secret")?; - msg.param.set(Param::Arg3, secret); + msg.param.set(PARAM_BROADCAST_SHARED_SECRET, secret); } send_msg(context, chat_id, &mut msg).await?; diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 22acf941da..7c01db6729 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -15,7 +15,7 @@ use tokio::fs; use crate::aheader::{Aheader, EncryptPreference}; use crate::blob::BlobObject; -use crate::chat::{self, Chat, load_broadcast_shared_secret}; +use crate::chat::{self, Chat, PARAM_BROADCAST_SHARED_SECRET, load_broadcast_shared_secret}; use crate::config::Config; use crate::constants::ASM_SUBJECT; use crate::constants::{Chattype, DC_FROM_HANDSHAKE}; @@ -847,7 +847,7 @@ impl MimeFactory { )); if msg.param.get_cmd() == SystemMessage::MemberAddedToGroup { - if let Some(secret) = msg.param.get(Param::Arg3) { + if let Some(secret) = msg.param.get(PARAM_BROADCAST_SHARED_SECRET) { headers.push(( "Chat-Broadcast-Secret", mail_builder::headers::text::Text::new(secret.to_string()).into(), @@ -1888,8 +1888,7 @@ fn hidden_recipients() -> Address<'static> { fn should_encrypt_with_broadcast_secret(msg: &Message, chat: &Chat) -> bool { chat.typ == Chattype::OutBroadcast - // Securejoin messages that are sent into a broadcast - // are encrypted asymmetrically: + // We encrypt securejoin messages asymmetrically && msg.param.get_cmd() != SystemMessage::SecurejoinMessage // The member-added message in a broadcast must be asymmetrically encrypted, // because the newly-added member doesn't know the broadcast shared secret yet: diff --git a/src/param.rs b/src/param.rs index 08157faeae..25242dddc3 100644 --- a/src/param.rs +++ b/src/param.rs @@ -118,6 +118,8 @@ pub enum Param { /// /// For [`SystemMessage::MemberAddedToGroup`], /// this is '1' if it was added because of a securejoin-handshake, and '0' otherwise. + /// + /// For call messages, this is the accept timestamp. Arg2 = b'F', /// For Messages @@ -132,6 +134,8 @@ pub enum Param { /// For Messages /// /// Deprecated `Secure-Join-Group` header for `BobHandshakeMsg::RequestWithAuth` messages. + /// + /// For call messages, this is the end timsetamp. Arg4 = b'H', /// For Messages From 27d1d3559bee1f53a55e026e55256a197fe53082 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 8 Oct 2025 15:43:36 +0200 Subject: [PATCH 14/27] small tweaks --- src/receive_imf.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index dd4f84f50f..8e152b54cb 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -44,10 +44,7 @@ use crate::securejoin::{self, handle_securejoin_handshake, observe_securejoin_on use crate::simplify; use crate::stock_str; use crate::sync::Sync::*; -use crate::tools::{ - self, buf_compress, create_broadcast_shared_secret, remove_subject_prefix, - validate_broadcast_shared_secret, -}; +use crate::tools::{self, buf_compress, remove_subject_prefix, validate_broadcast_shared_secret}; use crate::{chatlist_events, ensure_and_debug_assert, ensure_and_debug_assert_eq, location}; use crate::{contact, imap}; @@ -3536,7 +3533,8 @@ async fn apply_out_broadcast_changes( if let Some(_added_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberAdded) { // This message can be safely ignored, // because the only way to add a member is by having them scan a QR code. - // All devices will receive the vg-request-with-auth message and add him to the channel. + // All devices will receive the vg-request-with-auth message and add the member to the channel. + info!(context, "Second device adding a member (TRASH)"); better_msg.get_or_insert("".to_string()); } else if let Some(removed_fpr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemovedFpr) { send_event_chat_modified = true; From 89f1e528e56fffd8d05bb3807a43b29bad631575 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 8 Oct 2025 19:25:10 +0200 Subject: [PATCH 15/27] clippy --- src/chat/chat_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 4c6e9edfe4..f317c6f413 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -3365,7 +3365,7 @@ async fn test_sync_broadcast_avatar_and_name() -> Result<()> { let a2_broadcast_chat = Chat::load_from_db(alice2, a2_broadcast_id).await?; assert_eq!(a2_broadcast_chat.get_name(), "foo".to_string()); - set_chat_name(&alice1, a1_broadcast_id, "New name").await?; + set_chat_name(alice1, a1_broadcast_id, "New name").await?; let sent = alice1.pop_sent_msg().await; let rcvd = alice2.recv_msg(&sent).await; assert_eq!(rcvd.chat_id, a2_broadcast_id); From 7fcbf39300fd6153058830ecddfe3c3eee4ae15f Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 15 Oct 2025 23:28:45 +0200 Subject: [PATCH 16/27] Simon's review --- src/sync.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sync.rs b/src/sync.rs index 1f7acf31d0..d73c9973fe 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -270,7 +270,6 @@ impl Context { Ok(()) } } - .with_context(|| format!("Sync data {:?}", item.data)) .log_err(self) .ok(); } From a3a932a430fac30e24016909ddcfe7b6c55352a4 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 15 Oct 2025 23:49:57 +0200 Subject: [PATCH 17/27] clippy --- src/sync.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sync.rs b/src/sync.rs index d73c9973fe..1f17ff48c7 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -1,6 +1,6 @@ //! # Synchronize items between devices. -use anyhow::{Context as _, Result}; +use anyhow::Result; use mail_builder::mime::MimePart; use serde::{Deserialize, Serialize}; From 7bd1ceddae1b856f1e3723feec96d8c20eb3da05 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 20 Oct 2025 11:51:08 +0200 Subject: [PATCH 18/27] Update src/securejoin/bob.rs Co-authored-by: iequidoo <117991069+iequidoo@users.noreply.github.com> --- src/securejoin/bob.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index ba942dd5cf..248d290b87 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -126,7 +126,7 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul Ok(joining_chat_id) } QrInvite::Broadcast { .. } => { - // We created the broadcast channel already, now we need to add Alice to the group. + // We created the broadcast channel already, now we need to add Alice to it. if !is_contact_in_chat(context, joining_chat_id, invite.contact_id()).await? { chat::add_to_chat_contacts_table( context, From e6bb2545f7114c1ea7946bdd533d864c05769c44 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 20 Oct 2025 19:15:19 +0200 Subject: [PATCH 19/27] Update golden tests --- test-data/golden/test_broadcast_joining_golden_alice | 8 ++++---- test-data/golden/test_broadcast_joining_golden_bob | 8 ++++---- .../test_broadcast_joining_golden_private_chat | 4 ++-- test-data/golden/test_sync_broadcast_alice1 | 10 +++++----- test-data/golden/test_sync_broadcast_alice2 | 10 +++++----- test-data/golden/test_sync_broadcast_bob | 12 ++++++------ test-data/golden/two_group_securejoins | 2 +- test-data/golden/verified_chats_editor_reordering | 2 +- 8 files changed, 28 insertions(+), 28 deletions(-) diff --git a/test-data/golden/test_broadcast_joining_golden_alice b/test-data/golden/test_broadcast_joining_golden_alice index 2077f09c9e..260726c625 100644 --- a/test-data/golden/test_broadcast_joining_golden_alice +++ b/test-data/golden/test_broadcast_joining_golden_alice @@ -1,6 +1,6 @@ -OutBroadcast#Chat#10: My Channel [1 member(s)] Icon: e9b6c7a78aa2e4f415644f55a553e73.png +OutBroadcast#Chat#1001: My Channel [1 member(s)] Icon: e9b6c7a78aa2e4f415644f55a553e73.png -------------------------------------------------------------------------------- -Msg#10: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#11πŸ”’: Me (Contact#Contact#Self): You changed the group image. [INFO] √ -Msg#15πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#1001: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#1002πŸ”’: Me (Contact#Contact#Self): You changed the group image. [INFO] √ +Msg#1006πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_broadcast_joining_golden_bob b/test-data/golden/test_broadcast_joining_golden_bob index 83c72bec7c..a8d11023c2 100644 --- a/test-data/golden/test_broadcast_joining_golden_bob +++ b/test-data/golden/test_broadcast_joining_golden_bob @@ -1,6 +1,6 @@ -InBroadcast#Chat#11: My Channel [2 member(s)] Icon: e9b6c7a78aa2e4f415644f55a553e73.png +InBroadcast#Chat#2002: My Channel [2 member(s)] Icon: e9b6c7a78aa2e4f415644f55a553e73.png -------------------------------------------------------------------------------- -Msg#11: info (Contact#Contact#Info): Establishing guaranteed end-to-end encryption, please wait… [NOTICED][INFO] -Msg#15: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#16πŸ”’: (Contact#Contact#10): Member Me added by Alice. [FRESH][INFO] +Msg#2004: info (Contact#Contact#Info): Establishing guaranteed end-to-end encryption, please wait… [NOTICED][INFO] +Msg#2002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#2008πŸ”’: (Contact#Contact#2001): Member Me added by Alice. [FRESH][INFO] -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_broadcast_joining_golden_private_chat b/test-data/golden/test_broadcast_joining_golden_private_chat index ede3724ea3..d4a1739d41 100644 --- a/test-data/golden/test_broadcast_joining_golden_private_chat +++ b/test-data/golden/test_broadcast_joining_golden_private_chat @@ -1,4 +1,4 @@ -Single#Chat#11: bob@example.net [KEY bob@example.net] πŸ›‘οΈ +Single#Chat#1002: bob@example.net [KEY bob@example.net] -------------------------------------------------------------------------------- -Msg#14: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO πŸ›‘οΈ] +Msg#1003: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_alice1 b/test-data/golden/test_sync_broadcast_alice1 index 0bc41a2dde..d0d42cc228 100644 --- a/test-data/golden/test_sync_broadcast_alice1 +++ b/test-data/golden/test_sync_broadcast_alice1 @@ -1,7 +1,7 @@ -OutBroadcast#Chat#10: Channel [0 member(s)] +OutBroadcast#Chat#1001: Channel [0 member(s)] -------------------------------------------------------------------------------- -Msg#10: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#16πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -Msg#18πŸ”’: Me (Contact#Contact#Self): hi √ -Msg#19πŸ”’: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ +Msg#1001: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#1007πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#1009πŸ”’: Me (Contact#Contact#Self): hi √ +Msg#1010πŸ”’: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_alice2 b/test-data/golden/test_sync_broadcast_alice2 index 28a8cc10af..ef143e0d8d 100644 --- a/test-data/golden/test_sync_broadcast_alice2 +++ b/test-data/golden/test_sync_broadcast_alice2 @@ -1,7 +1,7 @@ -OutBroadcast#Chat#10: Channel [0 member(s)] +OutBroadcast#Chat#1001: Channel [0 member(s)] -------------------------------------------------------------------------------- -Msg#11: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#16πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ -Msg#18πŸ”’: Me (Contact#Contact#Self): hi √ -Msg#19πŸ”’: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ +Msg#1002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#1007πŸ”’: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √ +Msg#1009πŸ”’: Me (Contact#Contact#Self): hi √ +Msg#1010πŸ”’: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √ -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_bob b/test-data/golden/test_sync_broadcast_bob index 052b2e9a60..8d74c91274 100644 --- a/test-data/golden/test_sync_broadcast_bob +++ b/test-data/golden/test_sync_broadcast_bob @@ -1,8 +1,8 @@ -InBroadcast#Chat#11: Channel [1 member(s)] +InBroadcast#Chat#2002: Channel [1 member(s)] -------------------------------------------------------------------------------- -Msg#11: info (Contact#Contact#Info): Establishing guaranteed end-to-end encryption, please wait… [NOTICED][INFO] -Msg#16: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#17πŸ”’: (Contact#Contact#10): Member Me added by alice@example.org. [FRESH][INFO] -Msg#19πŸ”’: (Contact#Contact#10): hi [FRESH] -Msg#20πŸ”’: (Contact#Contact#10): Member Me removed by alice@example.org. [FRESH][INFO] +Msg#2004: info (Contact#Contact#Info): Establishing guaranteed end-to-end encryption, please wait… [NOTICED][INFO] +Msg#2002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#2009πŸ”’: (Contact#Contact#2001): Member Me added by alice@example.org. [FRESH][INFO] +Msg#2011πŸ”’: (Contact#Contact#2001): hi [FRESH] +Msg#2012πŸ”’: (Contact#Contact#2001): Member Me removed by alice@example.org. [FRESH][INFO] -------------------------------------------------------------------------------- diff --git a/test-data/golden/two_group_securejoins b/test-data/golden/two_group_securejoins index 5e20c36b6c..1e1ae3ec50 100644 --- a/test-data/golden/two_group_securejoins +++ b/test-data/golden/two_group_securejoins @@ -3,7 +3,7 @@ Group#Chat#6002: Group [3 member(s)] Msg#6004: info (Contact#Contact#Info): alice@example.org invited you to join this group. Waiting for the device of alice@example.org to reply… [NOTICED][INFO] +Msg#6002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] Msg#6006: info (Contact#Contact#Info): alice@example.org replied, waiting for being added to the group… [NOTICED][INFO] -Msg#6003: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] Msg#6008πŸ”’: (Contact#Contact#6001): Member Me added by alice@example.org. [FRESH][INFO] -------------------------------------------------------------------------------- diff --git a/test-data/golden/verified_chats_editor_reordering b/test-data/golden/verified_chats_editor_reordering index 3bf488c00c..a7e5b75d76 100644 --- a/test-data/golden/verified_chats_editor_reordering +++ b/test-data/golden/verified_chats_editor_reordering @@ -4,7 +4,7 @@ Msg#3004: info (Contact#Contact#Info): alice@example.org invited you to join thi Waiting for the device of alice@example.org to reply… [NOTICED][INFO] Msg#3006: info (Contact#Contact#Info): alice@example.org replied, waiting for being added to the group… [NOTICED][INFO] -Msg#3003: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] +Msg#3002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] Msg#3008πŸ”’: (Contact#Contact#3002): [FRESH] Msg#3009: info (Contact#Contact#Info): Member bob@example.net added. [NOTICED][INFO] Msg#3010πŸ”’: (Contact#Contact#3001): Member Me added by alice@example.org. [FRESH][INFO] From 24cbcbe9f7dfb3ad839301a1ab1dff0a7150ac51 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Oct 2025 13:46:58 +0200 Subject: [PATCH 20/27] fix: Don't add duplicate member-added message --- src/receive_imf.rs | 3 --- test-data/golden/test_broadcast_joining_golden_bob | 2 +- test-data/golden/test_sync_broadcast_bob | 6 +++--- test-data/golden/two_group_securejoins | 2 +- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 035e05ec14..97df912fc8 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -3530,9 +3530,6 @@ async fn apply_in_broadcast_changes( info!(context, "No-op broadcast 'Member added' message (TRASH)"); msg = "".to_string(); } else { - chat.id - .add_encrypted_msg(context, mime_parser.timestamp_sent) - .await?; msg = stock_str::msg_add_member_local(context, ContactId::SELF, from_id).await; } diff --git a/test-data/golden/test_broadcast_joining_golden_bob b/test-data/golden/test_broadcast_joining_golden_bob index a8d11023c2..77b33d473b 100644 --- a/test-data/golden/test_broadcast_joining_golden_bob +++ b/test-data/golden/test_broadcast_joining_golden_bob @@ -2,5 +2,5 @@ InBroadcast#Chat#2002: My Channel [2 member(s)] Icon: e9b6c7a78aa2e4f415644f55a5 -------------------------------------------------------------------------------- Msg#2004: info (Contact#Contact#Info): Establishing guaranteed end-to-end encryption, please wait… [NOTICED][INFO] Msg#2002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#2008πŸ”’: (Contact#Contact#2001): Member Me added by Alice. [FRESH][INFO] +Msg#2007πŸ”’: (Contact#Contact#2001): Member Me added by Alice. [FRESH][INFO] -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_sync_broadcast_bob b/test-data/golden/test_sync_broadcast_bob index 8d74c91274..bb0a3ed548 100644 --- a/test-data/golden/test_sync_broadcast_bob +++ b/test-data/golden/test_sync_broadcast_bob @@ -2,7 +2,7 @@ InBroadcast#Chat#2002: Channel [1 member(s)] -------------------------------------------------------------------------------- Msg#2004: info (Contact#Contact#Info): Establishing guaranteed end-to-end encryption, please wait… [NOTICED][INFO] Msg#2002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] -Msg#2009πŸ”’: (Contact#Contact#2001): Member Me added by alice@example.org. [FRESH][INFO] -Msg#2011πŸ”’: (Contact#Contact#2001): hi [FRESH] -Msg#2012πŸ”’: (Contact#Contact#2001): Member Me removed by alice@example.org. [FRESH][INFO] +Msg#2008πŸ”’: (Contact#Contact#2001): Member Me added by alice@example.org. [FRESH][INFO] +Msg#2010πŸ”’: (Contact#Contact#2001): hi [FRESH] +Msg#2011πŸ”’: (Contact#Contact#2001): Member Me removed by alice@example.org. [FRESH][INFO] -------------------------------------------------------------------------------- diff --git a/test-data/golden/two_group_securejoins b/test-data/golden/two_group_securejoins index 1e1ae3ec50..1e857fa60d 100644 --- a/test-data/golden/two_group_securejoins +++ b/test-data/golden/two_group_securejoins @@ -3,7 +3,7 @@ Group#Chat#6002: Group [3 member(s)] Msg#6004: info (Contact#Contact#Info): alice@example.org invited you to join this group. Waiting for the device of alice@example.org to reply… [NOTICED][INFO] -Msg#6002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] Msg#6006: info (Contact#Contact#Info): alice@example.org replied, waiting for being added to the group… [NOTICED][INFO] +Msg#6002: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] Msg#6008πŸ”’: (Contact#Contact#6001): Member Me added by alice@example.org. [FRESH][INFO] -------------------------------------------------------------------------------- From 691887fb9f539815417e12fddb72b675be30e0b6 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Oct 2025 13:53:31 +0200 Subject: [PATCH 21/27] iequidoo's review --- src/mimeparser.rs | 2 +- src/pgp.rs | 8 +++----- src/receive_imf.rs | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 7e6cb79e6a..583c44ba56 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -358,7 +358,7 @@ impl MimeMessage { let secrets: Vec = context .sql .query_map( - "SELECT secret FROM broadcasts_shared_secrets", + "SELECT secret FROM broadcasts_shared_secrets ORDER BY chat_id DESC", (), |row| row.get(0), |rows| { diff --git a/src/pgp.rs b/src/pgp.rs index 92a4ee3185..4c0a1744d7 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -256,12 +256,10 @@ pub fn decrypt( shared_secrets = &[]; } - // We always try out all passwords here, which is not great for performance. - // But benchmarking (see `benchmark_decrypting.rs`) + // We always try out all passwords here, + // but benchmarking (see `benchmark_decrypting.rs`) // showed that the performance impact is negligible. - // We could include a short (~2 character) identifier of the secret in cleartext - // (or just include the first 2 characters of the secret in cleartext) - // in order to narrow down the number of shared secrets that have to be tried out. + // We can improve this in the future if necessary. let message_password: Vec = shared_secrets .iter() .map(|p| Password::from(p.as_str())) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 97df912fc8..9a585b0e5d 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1544,7 +1544,7 @@ async fn do_chat_assignment( } else { warn!( context, - "Not creating outgoing broadcast, because secret is unknown" + "Not creating outgoing broadcast with id {listid}, because secret is unknown" ); } } From e74f31221f0b41c5f14d8fe7db6935e639039afa Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Oct 2025 13:55:46 +0200 Subject: [PATCH 22/27] clippy --- src/receive_imf.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 9a585b0e5d..81735e0f4d 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -3521,17 +3521,15 @@ async fn apply_in_broadcast_changes( if let Some(added_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberAdded) { if context.is_self_addr(added_addr).await? { - let msg; - - if chat.is_self_in_chat(context).await? { + let msg = if chat.is_self_in_chat(context).await? { // Self is already in the chat. // Probably Alice has two devices and her second device added us again; // just hide the message. info!(context, "No-op broadcast 'Member added' message (TRASH)"); - msg = "".to_string(); + "".to_string() } else { - msg = stock_str::msg_add_member_local(context, ContactId::SELF, from_id).await; - } + stock_str::msg_add_member_local(context, ContactId::SELF, from_id).await + }; better_msg.get_or_insert(msg); send_event_chat_modified = true; From f336adde1322d8b0ceb284d50824f2744933d5da Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 23 Oct 2025 16:17:44 +0200 Subject: [PATCH 23/27] iequidoo's review --- deltachat-ffi/deltachat.h | 4 ++-- deltachat-ffi/src/lot.rs | 2 +- deltachat-jsonrpc/src/api/types/qr.rs | 6 +++--- src/chat.rs | 22 ++++++++-------------- src/mimeparser.rs | 2 +- src/qr.rs | 6 +++--- src/securejoin/bob.rs | 7 +------ src/securejoin/qrinvite.rs | 8 ++++---- src/sql/migrations.rs | 2 +- 9 files changed, 24 insertions(+), 35 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index e80a3c469d..931a924d61 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -2563,7 +2563,7 @@ void dc_stop_ongoing_process (dc_context_t* context); #define DC_QR_ASK_VERIFYCONTACT 200 // id=contact #define DC_QR_ASK_VERIFYGROUP 202 // text1=groupname -#define DC_QR_ASK_VERIFYBROADCAST 204 +#define DC_QR_ASK_VERIFYBROADCAST 204 // text1=broadcast name #define DC_QR_FPR_OK 210 // id=contact #define DC_QR_FPR_MISMATCH 220 // id=contact #define DC_QR_FPR_WITHOUT_ADDR 230 // test1=formatted fingerprint @@ -2598,7 +2598,7 @@ void dc_stop_ongoing_process (dc_context_t* context); * * - DC_QR_ASK_VERIFYGROUP or DC_QR_ASK_VERIFYBROADCAST * with dc_lot_t::text1=Group name: - * ask whether to join the group; + * ask whether to join the chat; * if so, start the protocol with dc_join_securejoin(). * * - DC_QR_FPR_OK with dc_lot_t::id=Contact ID: diff --git a/deltachat-ffi/src/lot.rs b/deltachat-ffi/src/lot.rs index f437b8a319..fdea2c6d4c 100644 --- a/deltachat-ffi/src/lot.rs +++ b/deltachat-ffi/src/lot.rs @@ -45,7 +45,7 @@ impl Lot { Self::Qr(qr) => match qr { Qr::AskVerifyContact { .. } => None, Qr::AskVerifyGroup { grpname, .. } => Some(Cow::Borrowed(grpname)), - Qr::AskJoinBroadcast { broadcast_name, .. } => Some(Cow::Borrowed(broadcast_name)), + Qr::AskJoinBroadcast { name, .. } => Some(Cow::Borrowed(name)), Qr::FprOk { .. } => None, Qr::FprMismatch { .. } => None, Qr::FprWithoutAddr { fingerprint, .. } => Some(Cow::Borrowed(fingerprint)), diff --git a/deltachat-jsonrpc/src/api/types/qr.rs b/deltachat-jsonrpc/src/api/types/qr.rs index 459d8ed64b..4fbe071f64 100644 --- a/deltachat-jsonrpc/src/api/types/qr.rs +++ b/deltachat-jsonrpc/src/api/types/qr.rs @@ -38,7 +38,7 @@ pub enum QrObject { /// Ask the user whether to join the broadcast channel. AskJoinBroadcast { /// The user-visible name of this broadcast channel - broadcast_name: String, + name: String, /// A string of random characters, /// uniquely identifying this broadcast channel in the database. /// Called `grpid` for historic reasons: @@ -229,7 +229,7 @@ impl From for QrObject { } } Qr::AskJoinBroadcast { - broadcast_name, + name, grpid, contact_id, fingerprint, @@ -239,7 +239,7 @@ impl From for QrObject { let contact_id = contact_id.to_u32(); let fingerprint = fingerprint.to_string(); QrObject::AskJoinBroadcast { - broadcast_name, + name, grpid, contact_id, fingerprint, diff --git a/src/chat.rs b/src/chat.rs index 31b4282408..ce9cc5cf23 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3501,7 +3501,7 @@ pub async fn create_broadcast(context: &Context, chat_name: String) -> Result Result { match action { - SyncAction::CreateOutBroadcast { - chat_name, - shared_secret, - } => { + SyncAction::CreateOutBroadcast { chat_name, secret } => { create_out_broadcast_ex( self, Nosync, grpid.to_string(), chat_name.clone(), - shared_secret.to_string(), + secret.to_string(), ) .await?; Ok(true) diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 504e985692..24c5d54a7e 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -358,7 +358,7 @@ impl MimeMessage { let secrets: Vec = context .sql .query_map( - "SELECT secret FROM broadcasts_shared_secrets ORDER BY chat_id DESC", + "SELECT secret FROM broadcast_secrets ORDER BY chat_id DESC", (), |row| row.get(0), |rows| { diff --git a/src/qr.rs b/src/qr.rs index 943a138bd9..9200dfc65a 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -85,7 +85,7 @@ pub enum Qr { /// Ask whether to join the broadcast channel. AskJoinBroadcast { /// The user-visible name of this broadcast channel - broadcast_name: String, + name: String, /// A string of random characters, /// uniquely identifying this broadcast channel in the database. @@ -494,9 +494,9 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { authcode, }) } - } else if let (Some(grpid), Some(broadcast_name)) = (grpid, broadcast_name) { + } else if let (Some(grpid), Some(name)) = (grpid, broadcast_name) { Ok(Qr::AskJoinBroadcast { - broadcast_name, + name, grpid, contact_id, fingerprint, diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 5544282853..a69df1874a 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -360,12 +360,7 @@ async fn joining_chat_id( ) -> Result { match invite { QrInvite::Contact { .. } => Ok(alice_chat_id), - QrInvite::Group { grpid, name, .. } - | QrInvite::Broadcast { - broadcast_name: name, - grpid, - .. - } => { + QrInvite::Group { grpid, name, .. } | QrInvite::Broadcast { name, grpid, .. } => { let chattype = if matches!(invite, QrInvite::Group { .. }) { Chattype::Group } else { diff --git a/src/securejoin/qrinvite.rs b/src/securejoin/qrinvite.rs index 323882d65c..4bb3b71e11 100644 --- a/src/securejoin/qrinvite.rs +++ b/src/securejoin/qrinvite.rs @@ -32,7 +32,7 @@ pub enum QrInvite { Broadcast { contact_id: ContactId, fingerprint: Fingerprint, - broadcast_name: String, + name: String, grpid: String, invitenumber: String, authcode: String, @@ -112,21 +112,21 @@ impl TryFrom for QrInvite { authcode, }), Qr::AskJoinBroadcast { - broadcast_name, + name, grpid, contact_id, fingerprint, authcode, invitenumber, } => Ok(QrInvite::Broadcast { - broadcast_name, + name, grpid, contact_id, fingerprint, authcode, invitenumber, }), - _ => bail!("Unsupported QR type: {qr:?}"), + _ => bail!("Unsupported QR type"), } } } diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index b9e8736fd8..d7aa5f1dac 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -1313,7 +1313,7 @@ CREATE INDEX gossip_timestamp_index ON gossip_timestamp (chat_id, fingerprint); inc_and_check(&mut migration_version, 137)?; if dbversion < migration_version { sql.execute_migration( - "CREATE TABLE broadcasts_shared_secrets( + "CREATE TABLE broadcast_secrets( chat_id INTEGER PRIMARY KEY NOT NULL, secret TEXT NOT NULL ) STRICT", From 24d5335f8c2f06cf6fe026b065135e80eaa32ff8 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 23 Oct 2025 16:20:45 +0200 Subject: [PATCH 24/27] Fix compilation error --- src/stats.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/stats.rs b/src/stats.rs index d360f96819..3fcdc14929 100644 --- a/src/stats.rs +++ b/src/stats.rs @@ -156,7 +156,8 @@ struct JoinedInvite { already_verified: bool, /// The type of the invite: /// "contact" for 1:1 invites that setup a verified contact, - /// "group" for invites that invite to a group + /// "group" for invites that invite to a group, + /// "broadcast" for invites that invite to a broadcast channel, /// and also perform the contact verification 'along the way'. typ: String, } @@ -851,6 +852,7 @@ pub(crate) async fn count_securejoin_invite(context: &Context, invite: &QrInvite let typ = match invite { QrInvite::Contact { .. } => "contact", QrInvite::Group { .. } => "group", + QrInvite::Broadcast { .. } => "broadcast", }; context From 8a4044e152cbf2666395e52710d9a7f44c962ab9 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 23 Oct 2025 17:19:47 +0200 Subject: [PATCH 25/27] Another compilation error --- src/securejoin/bob.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index 41089f97e4..ea7efb7300 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -73,9 +73,8 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul .await?; // Chat ID of the group we are joining, unused otherwise. - let group_chat_id = joining_chat_id(context, &invite, chat_id).await?; if matches!(invite, QrInvite::Group { .. }) - && is_contact_in_chat(context, group_chat_id, ContactId::SELF).await? + && is_contact_in_chat(context, joining_chat_id, ContactId::SELF).await? { // If QR code is a group invite // and we are already in the chat, @@ -85,7 +84,7 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul contact_id: invite.contact_id(), progress: JoinerProgress::Succeeded.to_usize(), }); - return Ok(group_chat_id); + return Ok(joining_chat_id); } else if has_key && verify_sender_by_fingerprint(context, invite.fingerprint(), invite.contact_id()) .await? From 1d671d26271c5af5c30a727433043409849e95a3 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 23 Oct 2025 17:42:26 +0200 Subject: [PATCH 26/27] Fix test --- src/stats/stats_tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/stats/stats_tests.rs b/src/stats/stats_tests.rs index ab1ab9c291..5fcbeb6778 100644 --- a/src/stats/stats_tests.rs +++ b/src/stats/stats_tests.rs @@ -407,6 +407,7 @@ async fn test_stats_securejoin_invites() -> Result<()> { let bob = &tcm.bob().await; let charlie = &tcm.charlie().await; alice.set_config_bool(Config::StatsSending, true).await?; + let _first_sent_stats = alice.pop_sent_msg().await; let mut expected = vec![]; From efc323f602b774e93e29866c9da2d8b99cfb469e Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 23 Oct 2025 20:03:28 +0200 Subject: [PATCH 27/27] fix: Show an error message when trying to send into an old channel Previously, sending a message appeared to work, but didn't actually arrive. --- src/mimefactory.rs | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 44358dd363..ce31f09dfb 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1216,14 +1216,27 @@ impl MimeFactory { Loaded::Message { chat, msg } if should_encrypt_with_broadcast_secret(msg, chat) => { - // If there is no shared secret yet - // (because this is an old broadcast channel, - // created before we had symmetric encryption), - // we just encrypt asymmetrically. - // Symmetric encryption exists since 2025-10; - // some time after that, we can think about requiring everyone - // to switch to symmetrically-encrypted broadcast lists. - load_broadcast_shared_secret(context, chat.id).await? + let secret = load_broadcast_shared_secret(context, chat.id).await?; + if secret.is_none() { + // If there is no shared secret yet + // because this is an old broadcast channel, + // created before we had symmetric encryption, + // we show an error message. + let text = r#"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/."#; + chat::add_info_msg(context, chat.id, text, time()).await?; + bail!(text); + } + secret } _ => None, };