feat: Remove ProtectionBroken, make such chats Unprotected (#7041)

Chats can't break anymore.
This commit is contained in:
iequidoo
2025-07-28 14:12:15 -03:00
committed by iequidoo
parent b9183fe5eb
commit 779f58ab16
8 changed files with 40 additions and 90 deletions

View File

@@ -71,7 +71,9 @@ pub struct FullChat {
fresh_message_counter: usize, fresh_message_counter: usize,
// is_group - please check over chat.type in frontend instead // is_group - please check over chat.type in frontend instead
is_contact_request: bool, is_contact_request: bool,
is_protection_broken: bool, // deprecated 2025-07 /// Deprecated 2025-07. Chats protection cannot break any longer.
is_protection_broken: bool,
is_device_chat: bool, is_device_chat: bool,
self_in_group: bool, self_in_group: bool,
is_muted: bool, is_muted: bool,
@@ -216,7 +218,9 @@ pub struct BasicChat {
is_self_talk: bool, is_self_talk: bool,
color: String, color: String,
is_contact_request: bool, is_contact_request: bool,
/// Deprecated 2025-07. Chats protection cannot break any longer.
is_protection_broken: bool, is_protection_broken: bool,
is_device_chat: bool, is_device_chat: bool,
is_muted: bool, is_muted: bool,
} }

View File

@@ -94,14 +94,12 @@ pub enum ProtectionStatus {
/// ///
/// All members of the chat must be verified. /// All members of the chat must be verified.
Protected = 1, Protected = 1,
// `2` was never used as a value.
/// The chat was protected, but now a new message came in // Chats don't break in Core v2 anymore. Chats with broken protection existing before the
/// which was not encrypted / signed correctly. // key-contacts migration are treated as `Unprotected`.
/// The user has to confirm that this is OK. //
/// // ProtectionBroken = 3,
/// We only do this in 1:1 chats; in group chats, the chat just
/// stays protected.
ProtectionBroken = 3, // `2` was never used as a value.
} }
/// The reason why messages cannot be sent to the chat. /// The reason why messages cannot be sent to the chat.
@@ -118,10 +116,6 @@ pub(crate) enum CantSendReason {
/// The chat is a contact request, it needs to be accepted before sending a message. /// The chat is a contact request, it needs to be accepted before sending a message.
ContactRequest, ContactRequest,
/// Deprecated. The chat was protected, but now a new message came in
/// which was not encrypted / signed correctly.
ProtectionBroken,
/// Mailing list without known List-Post header. /// Mailing list without known List-Post header.
ReadOnlyMailingList, ReadOnlyMailingList,
@@ -144,10 +138,6 @@ impl fmt::Display for CantSendReason {
f, f,
"contact request chat should be accepted before sending messages" "contact request chat should be accepted before sending messages"
), ),
Self::ProtectionBroken => write!(
f,
"accept that the encryption isn't verified anymore before sending messages"
),
Self::ReadOnlyMailingList => { Self::ReadOnlyMailingList => {
write!(f, "mailing list does not have a know post address") write!(f, "mailing list does not have a know post address")
} }
@@ -479,16 +469,6 @@ impl ChatId {
let chat = Chat::load_from_db(context, self).await?; let chat = Chat::load_from_db(context, self).await?;
match chat.typ { match chat.typ {
Chattype::Single
if chat.blocked == Blocked::Not
&& chat.protected == ProtectionStatus::ProtectionBroken =>
{
// The protection was broken, then the user clicked 'Accept'/'OK',
// so, now we want to set the status to Unprotected again:
chat.id
.inner_set_protection(context, ProtectionStatus::Unprotected)
.await?;
}
Chattype::Single | Chattype::Group | Chattype::OutBroadcast | Chattype::InBroadcast => { Chattype::Single | Chattype::Group | Chattype::OutBroadcast | Chattype::InBroadcast => {
// User has "created a chat" with all these contacts. // User has "created a chat" with all these contacts.
// //
@@ -545,7 +525,7 @@ impl ChatId {
| Chattype::InBroadcast => {} | Chattype::InBroadcast => {}
Chattype::Mailinglist => bail!("Cannot protect mailing lists"), Chattype::Mailinglist => bail!("Cannot protect mailing lists"),
}, },
ProtectionStatus::Unprotected | ProtectionStatus::ProtectionBroken => {} ProtectionStatus::Unprotected => {}
}; };
context context
@@ -588,7 +568,6 @@ impl ChatId {
let cmd = match protect { let cmd = match protect {
ProtectionStatus::Protected => SystemMessage::ChatProtectionEnabled, ProtectionStatus::Protected => SystemMessage::ChatProtectionEnabled,
ProtectionStatus::Unprotected => SystemMessage::ChatProtectionDisabled, ProtectionStatus::Unprotected => SystemMessage::ChatProtectionDisabled,
ProtectionStatus::ProtectionBroken => SystemMessage::ChatProtectionDisabled,
}; };
add_info_msg_with_cmd( add_info_msg_with_cmd(
context, context,
@@ -1700,12 +1679,6 @@ impl Chat {
return Ok(Some(reason)); return Ok(Some(reason));
} }
} }
if self.is_protection_broken() {
let reason = ProtectionBroken;
if !skip_fn(&reason) {
return Ok(Some(reason));
}
}
if self.is_mailing_list() && self.get_mailinglist_addr().is_none_or_empty() { if self.is_mailing_list() && self.get_mailinglist_addr().is_none_or_empty() {
let reason = ReadOnlyMailingList; let reason = ReadOnlyMailingList;
if !skip_fn(&reason) { if !skip_fn(&reason) {
@@ -1935,25 +1908,9 @@ impl Chat {
Ok(is_encrypted) Ok(is_encrypted)
} }
/// Deprecated 2025-07. Returns true if the chat was protected, and then an incoming message broke this protection. /// Deprecated 2025-07. Returns false.
///
/// This function is only useful if the UI enabled the `verified_one_on_one_chats` feature flag,
/// otherwise it will return false for all chats.
///
/// 1:1 chats are automatically set as protected when a contact is verified.
/// When a message comes in that is not encrypted / signed correctly,
/// the chat is automatically set as unprotected again.
/// `is_protection_broken()` will return true until `chat_id.accept()` is called.
///
/// The UI should let the user confirm that this is OK with a message like
/// `Bob sent a message from another device. Tap to learn more`
/// and then call `chat_id.accept()`.
pub fn is_protection_broken(&self) -> bool { pub fn is_protection_broken(&self) -> bool {
match self.protected { false
ProtectionStatus::Protected => false,
ProtectionStatus::Unprotected => false,
ProtectionStatus::ProtectionBroken => true,
}
} }
/// Returns true if location streaming is enabled in the chat. /// Returns true if location streaming is enabled in the chat.
@@ -2947,7 +2904,7 @@ async fn prepare_send_msg(
let mut chat = Chat::load_from_db(context, chat_id).await?; let mut chat = Chat::load_from_db(context, chat_id).await?;
let skip_fn = |reason: &CantSendReason| match reason { let skip_fn = |reason: &CantSendReason| match reason {
CantSendReason::ProtectionBroken | CantSendReason::ContactRequest => { CantSendReason::ContactRequest => {
// Allow securejoin messages, they are supposed to repair the verification. // Allow securejoin messages, they are supposed to repair the verification.
// If the chat is a contact request, let the user accept it later. // If the chat is a contact request, let the user accept it later.
msg.param.get_cmd() == SystemMessage::SecurejoinMessage msg.param.get_cmd() == SystemMessage::SecurejoinMessage

View File

@@ -245,9 +245,6 @@ impl Chatlist {
.collect::<std::result::Result<Vec<_>, _>>() .collect::<std::result::Result<Vec<_>, _>>()
.map_err(Into::into) .map_err(Into::into)
}; };
// Return ProtectionBroken chats also, as that may happen to a verified chat at any
// time. It may be confusing if a chat that is normally in the list disappears
// suddenly. The UI need to deal with that case anyway.
context.sql.query_map( context.sql.query_map(
"SELECT c.id, c.type, c.param, m.id "SELECT c.id, c.type, c.param, m.id
FROM chats c FROM chats c

View File

@@ -1087,7 +1087,6 @@ impl Context {
#[derive(Default)] #[derive(Default)]
struct ChatNumbers { struct ChatNumbers {
protected: u32, protected: u32,
protection_broken: u32,
opportunistic_dc: u32, opportunistic_dc: u32,
opportunistic_mua: u32, opportunistic_mua: u32,
unencrypted_dc: u32, unencrypted_dc: u32,
@@ -1123,7 +1122,6 @@ impl Context {
// how many of the chats active in the last months are: // how many of the chats active in the last months are:
// - protected // - protected
// - protection-broken
// - opportunistic-encrypted and the contact uses Delta Chat // - opportunistic-encrypted and the contact uses Delta Chat
// - opportunistic-encrypted and the contact uses a classical MUA // - opportunistic-encrypted and the contact uses a classical MUA
// - unencrypted and the contact uses Delta Chat // - unencrypted and the contact uses Delta Chat
@@ -1166,8 +1164,6 @@ impl Context {
if protected == ProtectionStatus::Protected { if protected == ProtectionStatus::Protected {
chats.protected += 1; chats.protected += 1;
} else if protected == ProtectionStatus::ProtectionBroken {
chats.protection_broken += 1;
} else if encrypted { } else if encrypted {
if is_dc_message { if is_dc_message {
chats.opportunistic_dc += 1; chats.opportunistic_dc += 1;
@@ -1185,7 +1181,6 @@ impl Context {
) )
.await?; .await?;
res += &format!("chats_protected {}\n", chats.protected); res += &format!("chats_protected {}\n", chats.protected);
res += &format!("chats_protection_broken {}\n", chats.protection_broken);
res += &format!("chats_opportunistic_dc {}\n", chats.opportunistic_dc); res += &format!("chats_opportunistic_dc {}\n", chats.opportunistic_dc);
res += &format!("chats_opportunistic_mua {}\n", chats.opportunistic_mua); res += &format!("chats_opportunistic_mua {}\n", chats.opportunistic_mua);
res += &format!("chats_unencrypted_dc {}\n", chats.unencrypted_dc); res += &format!("chats_unencrypted_dc {}\n", chats.unencrypted_dc);

View File

@@ -1461,19 +1461,16 @@ async fn do_chat_assignment(
chat.typ == Chattype::Single, chat.typ == Chattype::Single,
"Chat {chat_id} is not Single", "Chat {chat_id} is not Single",
); );
let mut new_protection = match verified_encryption { let new_protection = match verified_encryption {
VerifiedEncryption::Verified => ProtectionStatus::Protected, VerifiedEncryption::Verified => ProtectionStatus::Protected,
VerifiedEncryption::NotVerified(_) => ProtectionStatus::Unprotected, VerifiedEncryption::NotVerified(_) => ProtectionStatus::Unprotected,
}; };
if chat.protected != ProtectionStatus::Unprotected ensure_and_debug_assert!(
&& new_protection == ProtectionStatus::Unprotected chat.protected == ProtectionStatus::Unprotected
// `chat.protected` must be maintained regardless of the `Config::VerifiedOneOnOneChats`. || new_protection == ProtectionStatus::Protected,
// That's why the config is checked here, and not above. "Chat {chat_id} can't downgrade to Unprotected",
&& context.get_config_bool(Config::VerifiedOneOnOneChats).await? );
{
new_protection = ProtectionStatus::ProtectionBroken;
}
if chat.protected != new_protection { if chat.protected != new_protection {
// The message itself will be sorted under the device message since the device // The message itself will be sorted under the device message since the device
// message is `MessageState::InNoticed`, which means that all following // message is `MessageState::InNoticed`, which means that all following

View File

@@ -1251,6 +1251,16 @@ CREATE INDEX gossip_timestamp_index ON gossip_timestamp (chat_id, fingerprint);
.await?; .await?;
} }
inc_and_check(&mut migration_version, 133)?;
if dbversion < migration_version {
// Make `ProtectionBroken` chats `Unprotected`. Chats can't break anymore.
sql.execute_migration(
"UPDATE chats SET protected=0 WHERE protected!=1",
migration_version,
)
.await?;
}
let new_version = sql let new_version = sql
.get_raw_config_int(VERSION_CFG) .get_raw_config_int(VERSION_CFG)
.await? .await?

View File

@@ -1292,7 +1292,7 @@ impl Context {
contact_id: Option<ContactId>, contact_id: Option<ContactId>,
) -> String { ) -> String {
match protect { match protect {
ProtectionStatus::Unprotected | ProtectionStatus::ProtectionBroken => { ProtectionStatus::Unprotected => {
if let Some(contact_id) = contact_id { if let Some(contact_id) = contact_id {
chat_protection_disabled(self, contact_id).await chat_protection_disabled(self, contact_id).await
} else { } else {

View File

@@ -31,7 +31,7 @@ async fn test_verified_oneonone_chat_not_broken_by_device_change() {
check_verified_oneonone_chat_protection_not_broken(false).await; check_verified_oneonone_chat_protection_not_broken(false).await;
} }
async fn check_verified_oneonone_chat_protection_not_broken(broken_by_classical_email: bool) { async fn check_verified_oneonone_chat_protection_not_broken(by_classical_email: bool) {
let mut tcm = TestContextManager::new(); let mut tcm = TestContextManager::new();
let alice = tcm.alice().await; let alice = tcm.alice().await;
let bob = tcm.bob().await; let bob = tcm.bob().await;
@@ -42,7 +42,7 @@ async fn check_verified_oneonone_chat_protection_not_broken(broken_by_classical_
assert_verified(&alice, &bob, ProtectionStatus::Protected).await; assert_verified(&alice, &bob, ProtectionStatus::Protected).await;
assert_verified(&bob, &alice, ProtectionStatus::Protected).await; assert_verified(&bob, &alice, ProtectionStatus::Protected).await;
if broken_by_classical_email { if by_classical_email {
tcm.section("Bob uses a classical MUA to send a message to Alice"); tcm.section("Bob uses a classical MUA to send a message to Alice");
receive_imf( receive_imf(
&alice, &alice,
@@ -58,7 +58,6 @@ async fn check_verified_oneonone_chat_protection_not_broken(broken_by_classical_
.await .await
.unwrap() .unwrap()
.unwrap(); .unwrap();
// Bob's contact is still verified, but the chat isn't marked as protected anymore
let contact = alice.add_or_lookup_contact(&bob).await; let contact = alice.add_or_lookup_contact(&bob).await;
assert_eq!(contact.is_verified(&alice).await.unwrap(), true); assert_eq!(contact.is_verified(&alice).await.unwrap(), true);
assert_verified(&alice, &bob, ProtectionStatus::Protected).await; assert_verified(&alice, &bob, ProtectionStatus::Protected).await;
@@ -199,7 +198,6 @@ async fn test_missing_key_reexecute_securejoin() -> Result<()> {
let chat_id = tcm.execute_securejoin(bob, alice).await; let chat_id = tcm.execute_securejoin(bob, alice).await;
let chat = Chat::load_from_db(bob, chat_id).await?; let chat = Chat::load_from_db(bob, chat_id).await?;
assert!(chat.is_protected()); assert!(chat.is_protected());
assert!(!chat.is_protection_broken());
Ok(()) Ok(())
} }
@@ -213,7 +211,6 @@ async fn test_create_unverified_oneonone_chat() -> Result<()> {
// A chat with an unknown contact should be created unprotected // A chat with an unknown contact should be created unprotected
let chat = alice.create_chat(&bob).await; let chat = alice.create_chat(&bob).await;
assert!(!chat.is_protected()); assert!(!chat.is_protected());
assert!(!chat.is_protection_broken());
receive_imf( receive_imf(
&alice, &alice,
@@ -230,14 +227,12 @@ async fn test_create_unverified_oneonone_chat() -> Result<()> {
// Now Bob is a known contact, new chats should still be created unprotected // Now Bob is a known contact, new chats should still be created unprotected
let chat = alice.create_chat(&bob).await; let chat = alice.create_chat(&bob).await;
assert!(!chat.is_protected()); assert!(!chat.is_protected());
assert!(!chat.is_protection_broken());
tcm.send_recv(&bob, &alice, "hi").await; tcm.send_recv(&bob, &alice, "hi").await;
chat.id.delete(&alice).await.unwrap(); chat.id.delete(&alice).await.unwrap();
// Now we have a public key, new chats should still be created unprotected // Now we have a public key, new chats should still be created unprotected
let chat = alice.create_chat(&bob).await; let chat = alice.create_chat(&bob).await;
assert!(!chat.is_protected()); assert!(!chat.is_protected());
assert!(!chat.is_protection_broken());
Ok(()) Ok(())
} }
@@ -525,7 +520,6 @@ async fn test_message_from_old_dc_setup() -> Result<()> {
assert!(contact.is_verified(alice).await.unwrap()); assert!(contact.is_verified(alice).await.unwrap());
let chat = alice.get_chat(bob).await; let chat = alice.get_chat(bob).await;
assert!(chat.is_protected()); assert!(chat.is_protected());
assert_eq!(chat.is_protection_broken(), false);
Ok(()) Ok(())
} }
@@ -812,19 +806,15 @@ async fn test_verified_chat_editor_reordering() -> Result<()> {
// ============== Helper Functions ============== // ============== Helper Functions ==============
async fn assert_verified(this: &TestContext, other: &TestContext, protected: ProtectionStatus) { async fn assert_verified(this: &TestContext, other: &TestContext, protected: ProtectionStatus) {
if protected != ProtectionStatus::ProtectionBroken { let contact = this.add_or_lookup_contact(other).await;
let contact = this.add_or_lookup_contact(other).await; assert_eq!(contact.is_verified(this).await.unwrap(), true);
assert_eq!(contact.is_verified(this).await.unwrap(), true);
}
let chat = this.get_chat(other).await; let chat = this.get_chat(other).await;
let (expect_protected, expect_broken) = match protected { assert_eq!(
ProtectionStatus::Unprotected => (false, false), chat.is_protected(),
ProtectionStatus::Protected => (true, false), protected == ProtectionStatus::Protected
ProtectionStatus::ProtectionBroken => (false, true), );
}; assert_eq!(chat.is_protection_broken(), false);
assert_eq!(chat.is_protected(), expect_protected);
assert_eq!(chat.is_protection_broken(), expect_broken);
} }
async fn enable_verified_oneonone_chats(test_contexts: &[&TestContext]) { async fn enable_verified_oneonone_chats(test_contexts: &[&TestContext]) {