The time has come to say goodbye to "verification" (phew!) - #8691
Conversation
|
Closed related #7340 |
| def test_verified_group_member_added_recovery(acf) -> None: | ||
| """Tests verified group recovery by reverifying then removing and adding a member back.""" | ||
| def test_group_member_added_recovery(acf) -> None: | ||
| """Tests group recovery after a member resets its key.""" |
There was a problem hiding this comment.
Note: "resets its key" refers to resetup_account which is setting up a new account but using the same transports (email addresses and mailboxes) as the old account.
| id: ContactId, | ||
|
|
||
| verified: VerifiedStatus, | ||
| encrypted: bool, |
There was a problem hiding this comment.
cc @Hocuri, also for all stats.rs changes
The changes look good, but statistics bot will get both old reports with verified and new reports with encrypted fields after this change. Maybe needs translation to VerifiedStatus::Opportunistic and VerifiedStatus::Unencrypted when processing or the other way round.
There was a problem hiding this comment.
yes, i had it only mentioned in the first commit as breaking change but not in the PR description:
BREAKING CHANGE: the statistics JSON sent to the self-reporting-bot on Android changes: Contacts have
encryptedinstead ofverifiedand losetransitive_chainproperties and message stats haveencryptedinstead ofverifiedandunverified_encrypted, and securejoin invites losealready_verified. The collecting bot stores incoming reports verbatim but analysis will have to make sense of older and newer reports.
There was a problem hiding this comment.
Sounds fine to handle this while processing the reports
| })? | ||
| } 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 | ||
| // handshake must be processed before chats are created |
There was a problem hiding this comment.
Comment on comment, but since you modify it anyway: i'd remove it completely because it is outdated. observe_securejoin_on_other_device now does close to nothing, just decides whether to process the message further (for vg-member-added) or only create a tombstone, and emits an event, so it is not important to call it before creating a chat.
Since V2 a contact is its key, there is no address-to-key binding left to verify. The JSONRPC and FFI APIs are unchanged and report nothing as verified. BREAKING CHANGE: the statistics JSON sent to the self-reporting-bot on Android changes: Contacts have `encrypted` instead of `verified` and lose `transitive_chain` properties and message stats have `encrypted` instead of `verified` and `unverified_encrypted`, and securejoin invites lose `already_verified`. The collecting bot stores incoming reports verbatim but analysis will have to make sense of older and newer reports.
Core no longer tracks verification, so the API has nothing left to report and UIs should drop their checkmark and "Introduced by" code. BREAKING CHANGE: dc_contact_is_verified() and dc_contact_get_verifier_id() are removed. BREAKING CHANGE: the JSON-RPC Contact object loses the `isVerified` and `verifierId` fields. A bot reading `snapshot.is_verified` now gets an `AttributeError` at runtime. BREAKING CHANGE: the Python bindings lose `Contact.is_verified()` and `Contact.get_verifier()`. BREAKING CHANGE: DC_STR_CONTACT_VERIFIED (35) is removed, so UIs should stop registering a translation for it. A stock id core does not know is logged and otherwise ignored, so an un-updated client keeps working.
fc533e8 to
2f1c0fa
Compare
Hocuri
left a comment
Hocuri
left a comment
There was a problem hiding this comment.
I only looked at it very lightly - LGTM overall, I just think that the PR description and title sounds way too alarming. E.g. we're still keeping securejoin, along with the code that ensures that the fingerprint maches and that the auth code is correct. We're only removing the UI signalling around it, because we have reason to believe that it's not helpful and just adds unnecessary complexity, and we want to replace it with better UI signalling soon.
4 participants
After several discussions in the last year, the time has come: This PR removes all support and user-signals around "verification" while the securejoin protocol remains fully operational for "setup-contact", "join-group" and "join-broadcast" interactions. Every way of establishing contact already ensures that the relay operator can not exchange the public key for a contact. The current UI signalling of contact verification levels / trust levels is of no real value, and was in its current version never based on user research, and the whole "verification" aspect came from a time when servers could potentially machine-in-the-middle Delta Chat users.
Brief history note: it took some ~20 hours to do this removal PR, while the decade-long developments of securejoin and its core implementation first in C then in Rust took many thousands of combined hours from many contributors, also a lot from me personally. However, "the best code is no code" along with "the best API is no API" is my lifelong motto as a programmer :) I am just happy we can remove all the complexity around key-verification, providing another competitive edge against Signal and WhatsApp because they need to maintain phone-number->key bindings and introduced complex machinery ("key transparency") with unclear UX benefits while since our July 2025 V2 releases we don't maintain any binding and a contact is represented by its cryptographic key alone, rendering the question of "verified bindings" moot.
Related PRs:
make a note that the doc is severely outdated聽securejoin#12 which changes the web page and PDF for https://securejoin.delta.chat (which should really be securejoin.chatmail.at but i digress) for background.
verification is to be dropped with the next release聽deltachat/deltachat-pages#1417 which changes the FAQ to make verification a thing of the past
Note to reviewers: I'd appreciate to not overly discuss wording, or small tweaks which can be done in follow up PRs if needed. The main review concern should be that the production code changes are proper. We'd like to merge this PR soon so UI developers can adapt, and we can go for a new release end of the week if at all possible
BREAKING CHANGES
I decided to remove the following APIs so that UIs also have to remove all invocations and usages. Alternative would be to return some fake
0/falsebut UIs anyway need to follow up before release (see notes below) so it's better to do a clean sweep. In the "STALE" section there are many things i did not break to minimize the churn, otherwise.CFFI
dc_contact_is_verified()anddc_contact_get_verifier_id()are removed.DC_STR_CONTACT_VERIFIEDwas removed, don't call translation or use it at all.JSON-RPC Contact object loses the
isVerifiedandverifierIdfields. A bot readingsnapshot.is_verifiednow gets anAttributeErrorat runtime.Python bindings lose
Contact.is_verified()andContact.get_verifier().Notes for UI developers
Remove the "Introduced by" row and all interaction around it.
Drop code that renders the green checkmark everywhere
Make sure to always pass
0/falsetodc_create_group_chatandcreateGroupChatQR codesare completely unchangedKeep rendering
ChatProtectionEnabledandChatProtectionDisabledsystem messages. Old peers might still send them.DC_STR_CHAT_PROTECTION_ENABLEDis not a verification string, it says since v2 "Messages are end-to-end encrypted."STALE but remaining for now
While i mostly worked by changing/removing anything that mentions "verified" and code, tests and docs around it, i deliberately did not do some changes and renames to reduce the diff and breaking changes:
The
DC_QR_ASK*VERIFY*constants andcheckQr's return strings remain for now, because there might be consumers that compare to strings, and renaming them would produce considerably more churn on UI/bot sides.the
protectargument todc_create_group_chatandcreateGroupChatremains but is ignoredis_ask_verifycontactandis_ask_verifygroupremain for now despite the misnomersrc/tests/verified_chats.rswas not renamed because it would make the diff unreadableSystemMessage::ChatProtectionDisabledand itsSystemMessageTypebecause peers still send itChatProtectionEnabledand its stock string is a misnomer since V2 already but remains for nowcontacts.verifierandchats.protectedsql columns remain but are unusedstats_securejoin_invites.already_verifiedis written as constant 0,stats_msgs.verifiedis not written at all anymoreacpeerstatestable has 7 verification columns and an index which is already dead since migration 132