Skip to content

Commit e0607b3

Browse files
authored
feat: If any group member is in RESET peerstate, downgrade to Ad-Hoc group (#6905)
I agree with holger that it's most important to allow communication in mixed/ambiguous groups, even if you can't modify the member list anymore. So, if any member of a group is in RESET peerstate, this downgrades the whole group to an Ad-Hoc group, rather than removing this member. Except for chatmail accounts, where we ignore the Reset state already before PGP-contacts, so, it's ignored in the migration, too. Broadcast lists keep the current behavior. Fix #6908
1 parent 7f9c6b9 commit e0607b3

File tree

1 file changed

+62
-29
lines changed

1 file changed

+62
-29
lines changed

src/sql/migrations.rs

Lines changed: 62 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,6 +1559,7 @@ fn migrate_pgp_contacts(
15591559
.context("Step 20")?
15601560
.collect::<Result<BTreeSet<u32>, rusqlite::Error>>()
15611561
.context("Step 21")?;
1562+
15621563
{
15631564
let mut stmt = transaction
15641565
.prepare(
@@ -1578,6 +1579,37 @@ fn migrate_pgp_contacts(
15781579
.context("Step 23")?;
15791580
let mut load_chat_contacts_stmt = transaction
15801581
.prepare("SELECT contact_id FROM chats_contacts WHERE chat_id=? AND contact_id>9")?;
1582+
let is_chatmail: Option<String> = transaction
1583+
.query_row(
1584+
"SELECT value FROM config WHERE keyname='is_chatmail'",
1585+
(),
1586+
|row| row.get(0),
1587+
)
1588+
.optional()
1589+
.context("Step 23.1")?;
1590+
let is_chatmail = is_chatmail
1591+
.and_then(|s| s.parse::<i32>().ok())
1592+
.unwrap_or_default()
1593+
!= 0;
1594+
let map_to_pgp_contact = |old_member: &u32| {
1595+
(
1596+
*old_member,
1597+
autocrypt_pgp_contacts
1598+
.get(old_member)
1599+
.or_else(|| {
1600+
// For chatmail servers,
1601+
// we send encrypted even if the peerstate is reset,
1602+
// because an unencrypted message likely won't arrive.
1603+
// This is the same behavior as before PGP-contacts migration.
1604+
if is_chatmail {
1605+
autocrypt_pgp_contacts_with_reset_peerstate.get(old_member)
1606+
} else {
1607+
None
1608+
}
1609+
})
1610+
.copied(),
1611+
)
1612+
};
15811613

15821614
let mut update_member_stmt = transaction
15831615
.prepare("UPDATE chats_contacts SET contact_id=? WHERE contact_id=? AND chat_id=?")?;
@@ -1598,29 +1630,6 @@ fn migrate_pgp_contacts(
15981630
orphaned_contacts.remove(m);
15991631
}
16001632
};
1601-
let retain_autocrypt_pgp_contacts = || {
1602-
old_members
1603-
.iter()
1604-
.map(|original| {
1605-
(
1606-
*original,
1607-
autocrypt_pgp_contacts
1608-
.get(original)
1609-
// TODO it's unclear whether we want to do this:
1610-
// We could also make the group unencrypted
1611-
// if any peerstate is reset.
1612-
// Also, right now, if we have no key at all,
1613-
// the member will be silently removed from the group;
1614-
// maybe we should at least post an info message?
1615-
.or_else(|| {
1616-
autocrypt_pgp_contacts_with_reset_peerstate.get(original)
1617-
})
1618-
.copied(),
1619-
)
1620-
})
1621-
.collect::<Vec<(u32, Option<u32>)>>()
1622-
};
1623-
16241633
let old_and_new_members: Vec<(u32, Option<u32>)> = match typ {
16251634
// 1:1 chats retain:
16261635
// - email-contact if peerstate is in the "reset" state,
@@ -1634,7 +1643,8 @@ fn migrate_pgp_contacts(
16341643
info!(context, "1:1 chat {chat_id} doesn't contain contact, probably it's self or device chat");
16351644
continue;
16361645
};
1637-
let Some(&new_contact) = autocrypt_pgp_contacts.get(old_member) else {
1646+
1647+
let (_, Some(new_contact)) = map_to_pgp_contact(old_member) else {
16381648
keep_email_contacts("No peerstate, or peerstate in 'reset' state");
16391649
continue;
16401650
};
@@ -1668,7 +1678,10 @@ fn migrate_pgp_contacts(
16681678
})
16691679
.collect()
16701680
} else {
1671-
retain_autocrypt_pgp_contacts()
1681+
old_members
1682+
.iter()
1683+
.map(map_to_pgp_contact)
1684+
.collect::<Vec<(u32, Option<u32>)>>()
16721685
}
16731686
}
16741687

@@ -1679,16 +1692,36 @@ fn migrate_pgp_contacts(
16791692
}
16801693

16811694
// Broadcast list
1682-
160 => retain_autocrypt_pgp_contacts(),
1683-
1695+
160 => old_members
1696+
.iter()
1697+
.map(|original| {
1698+
(
1699+
*original,
1700+
autocrypt_pgp_contacts
1701+
.get(original)
1702+
// There will be no unencrypted broadcast lists anymore,
1703+
// so, if a peerstate is reset,
1704+
// the best we can do is encrypting to this key regardless.
1705+
.or_else(|| {
1706+
autocrypt_pgp_contacts_with_reset_peerstate.get(original)
1707+
})
1708+
.copied(),
1709+
)
1710+
})
1711+
.collect::<Vec<(u32, Option<u32>)>>(),
16841712
_ => {
16851713
warn!(context, "Invalid chat type {typ}");
16861714
continue;
16871715
}
16881716
};
16891717

1690-
if old_and_new_members.iter().all(|(_old, new)| new.is_none()) {
1691-
keep_email_contacts("All contacts in chat are e-mail contacts");
1718+
// If a group contains a contact without a key or with 'reset' peerstate,
1719+
// downgrade to unencrypted Ad-Hoc group.
1720+
if typ == 120 && old_and_new_members.iter().any(|(_old, new)| new.is_none()) {
1721+
transaction
1722+
.execute("UPDATE chats SET grpid='' WHERE id=?", (chat_id,))
1723+
.context("Step 26.1")?;
1724+
keep_email_contacts("Group contains contact without peerstate");
16921725
continue;
16931726
}
16941727

0 commit comments

Comments
 (0)