Skip to content

properly add a member triggered by unrelated secure-join protocol#4896

Merged
hpk42 merged 3 commits intomainfrom
hpk/blobstate-fix-4894
Oct 31, 2023
Merged

properly add a member triggered by unrelated secure-join protocol#4896
hpk42 merged 3 commits intomainfrom
hpk/blobstate-fix-4894

Conversation

@hpk42
Copy link
Copy Markdown
Contributor

@hpk42 hpk42 commented Oct 31, 2023

Fixes #4894
so that a "left-over" bobstate does not block a receiver of a MemberAdded message from adding the member if it was added through an unrelated secure-join process (i.e. the receiver was not the member being added through the secure-join protocol)

@hpk42 hpk42 changed the title [wip] write a failing test for pending bobstate issue properly add a member triggered by unrelated secure-join protocol Oct 31, 2023
@hpk42 hpk42 force-pushed the hpk/blobstate-fix-4894 branch 2 times, most recently from 8784fb7 to 49de4fd Compare October 31, 2023 18:33
@hpk42 hpk42 force-pushed the hpk/blobstate-fix-4894 branch from 49de4fd to 55b8a6c Compare October 31, 2023 19:27
break

lp.sec("ac1: let ac2 join again but shutoff ac1 in the middle of securejoin")
ac2.qr_join_chat(ch1.get_join_qr())
Copy link
Copy Markdown
Collaborator

@adbenitez adbenitez Oct 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't easier if ac1 scans the QR of ac2 after ac2 is already shutdown/offline? that was the way we tested it IRL, and would avoid the need for the next loop below 🤔

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a workaround for a check that will be removed in a follow-up PR:
https://github.com/deltachat/deltachat-core-rust/blob/6aae0276da4c20736254055ee3941cac71292f6e/src/securejoin/bobstate.rs#L338-L367
It checks that Bob knows the group he is joining is verified. IRL we probably left the verified group and rejoined it, here we terminate once we learn it is verified instead of fully joining and then leaving and then starting a join again.

Copy link
Copy Markdown
Collaborator

@Hocuri Hocuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code change LGTM; I only skimmed over the test since it's getting late here

@hpk42 hpk42 merged commit 196a346 into main Oct 31, 2023
@hpk42 hpk42 deleted the hpk/blobstate-fix-4894 branch October 31, 2023 23:58
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.

Bob state prevents processing vg-member-added

4 participants