Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor securejoin #5059

Merged
merged 7 commits into from
Nov 30, 2023
Merged

Refactor securejoin #5059

merged 7 commits into from
Nov 30, 2023

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Nov 29, 2023

This PR cleans up Securejoin protocol to better match currently refreshed https://securejoin.readthedocs.io/ documentation.

Steps vc-contact-confirm-received and vg-member-added-received are removed. They are not used because Bob's observing device marks Alice as verified when vc-request-with-auth is observed and by the time *-received message is observed Alice is already verified. vc-request-with-auth is not deleted (HandshakeMessage::Ignore instead of HandshakeMessage::Done) from the server on receival, so it is observable.

Unused PeerstateVerifiedStatus enum is removed. I plan to reintroduce the concept of bidirectional verification in next PRs by storing the fingerprint of the key we think Alice has as verified rather than just marking Alice as bidirectionally verified. Public API is just going to tell if green checkmark is to be displayed, so nothing is going to change for UIs.

Chat-Verified headers are now sent in verified 1:1 chat. I plan to use it later to upgrade one-way verification to two-way verification.

Securejoin processing is factored out of add_parts and centralized, both primary device and observing device paths are next to each other now. add_parts is not even called for most messages, we simply create a single tombstone in msgs table manually.

@link2xt link2xt force-pushed the link2xt/refactor-securejoin branch from 87d844d to c87cea4 Compare November 29, 2023 03:04
@link2xt link2xt force-pushed the link2xt/refactor-securejoin branch from c87cea4 to 407a2ec Compare November 29, 2023 03:34
@link2xt link2xt changed the title WIP: refactor securejoin Refactor securejoin Nov 29, 2023
@link2xt link2xt marked this pull request as ready for review November 29, 2023 03:47
@iequidoo
Copy link
Collaborator

Another one thing to remove is Secure-Join-Fingerprint header from vc-contact-confirm/vg-member-added, see 4da9e39

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 29, 2023

Another one thing to remove is Secure-Join-Fingerprint header from vc-contact-confirm/vg-member-added, see 4da9e39

This is from PR #4345

1.107.1 which does not require Secure-Join-Fingerprint was tagged on 2023-02-01. So makes sense to get rid of it, but probably not in the bugfix releases now that we are still stabilizing them.

The other commit from #4345 is very close to the one in this PR, the difference is that #4345 HandshakeMessage::Ignore is returned for "vg-member-added-received" | "vc-contact-confirm-received", while in this PR it is HandshakeMessage::Done which results in the deletion of the messages from IMAP as well.

@link2xt link2xt merged commit 1c9662a into main Nov 30, 2023
@link2xt link2xt deleted the link2xt/refactor-securejoin branch November 30, 2023 12:04
@Hocuri
Copy link
Collaborator

Hocuri commented Nov 30, 2023

Chat-Verified headers are now sent in verified 1:1 chat. I plan to use it later to upgrade one-way verification to two-way verification.

BTW, I don't think this will be possible to do securely: Delta Chat can't just accept a Chat-Verified header as a signal to mark your chat partner's key as verified, because this would allow for the following attack:

  • Eve does a MitM on the opportunistically encrypted communication between Alice and Bob
  • Eve inserts a Chat-Verified header into a message Alice sends to Bob
    • Bob's Delta Chat marks Alice's opportunistic key (that is MitM-ed) as verified
  • Bob sends a message to Alice with the Chat-Verified header
    • Alice's Delta Chat marks Bob's opportunistic key (that is MitM-ed) as verified
  • Alice and Bob have each other as verified, even though there is an ongoing MitM attack!

Note that Alice (the person), does know that the end-to-end encryption is verified if Bob tells her in a second channel that he has her verified, because if Eve wants to do a MitM attack, then she has to replace the keys in both ways.

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 30, 2023

BTW, I don't think this will be possible to do securely: Delta Chat can't just accept a Chat-Verified header as a signal to mark your chat partner's key as verified, because this would allow for the following attack:

As long as Chat-Verified: 1 comes from a one-verified contact, I don't see any security problem. It is not even about security, it is just for backwards verification to know that our current key is marked as verified by the other side and we can start adding them to verified chats. Worst case is that we do it incorrectly and the other side starts receiving square bracket errors about us not being verified, just like it already happens today if the last messages in the protocol are lost.

Bob's Delta Chat marks Alice's opportunistic key (that is MitM-ed) as verified

Worst case that happens here is that Bob marks Alice as thinking that Bob's key is verified. We are not going to mark Alice's key as verified based on Chat-Verified header. But even that is not going to happen as Chat-Verified header is not signed with a verified key of Alice at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants