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

migrated address lose verified status and key #5535

Closed
adbenitez opened this issue May 3, 2024 · 6 comments · Fixed by #5632
Closed

migrated address lose verified status and key #5535

adbenitez opened this issue May 3, 2024 · 6 comments · Fixed by #5632
Assignees
Labels
bug Something is not working

Comments

@adbenitez
Copy link
Member

  1. I have a contact/peer that is verified
  2. we share a non-protected/non-verified/normal group
  3. peer goes to setting and changes they address, doing what is called "account migration"
  4. peer writes to me in private
  5. in the old chat with the old address I see a "foo changed address to bar"
  6. the normal group still has the old address since it is non-verified address is not migrated automatically
  7. I try to remove old address and add new one to group
  8. "you removed foo" message is unencrypted and not delivered because I am using chatmail instance
  9. old address/contact of my peer now doesn't have the verified checkmark anymore and apparently I lost their public key, sending a message in private to old address also fails because it is unencrypted
@adbenitez adbenitez added the bug Something is not working label May 3, 2024
@iequidoo
Copy link
Collaborator

iequidoo commented May 4, 2024

Looks like this is quite expected, after the peer wrote to you in private an AEAP occured (i assume that the message was encrypted because we only do AEAP for encrypted messages now), so the peerstate was moved (not copied) to the new address and the old address has no peerstate anymore. This is apparently a result of b6db015. In general moving (not copying) a peerstate looks correct, but the described wrong behaviour for non-verified groups must be fixed somehow.

Btw, asking just in case, does the new contact have the verified checkmark?

@hpk42
Copy link
Contributor

hpk42 commented May 4, 2024

2. we share a non-protected/non-verified/normal group

more useful to know is if the group is encrypted because your contact moved to a chatmail account which only allows encrypted outgoing messages -- so if the group is not encrypted then there will be unavoidable message sending problems -- i guess this is not the issue in your situation, though?

3. peer goes to setting and changes they address, doing what is called "account migration"

4. peer writes to me in private

5. in the old chat with the old address I see a "foo changed address to bar"

6. the normal group still has the old address since it is non-verified address is not migrated automatically

7. I try to remove old address and add new one to group

It's a kind of known loose end of AEAP: your contact should have just written to the group, not to you in private and then everyone's device in the group would have followed the move. Before you scream -- i am aware that this is not optimal. I am just saying how "to hold it right" :)

@adbenitez
Copy link
Member Author

adbenitez commented May 4, 2024

but the described wrong behaviour for non-verified groups must be fixed somehow.

yes it is a pretty bad behavior for no verified groups, account is not migrated automatically there so the problem is unavoidable that people need to delete manually and will be unable due to missing encryption

@adbenitez
Copy link
Member Author

adbenitez commented May 4, 2024

more useful to know is if the group is encrypted because your contact moved to a chatmail account which only allows encrypted outgoing messages

the group used to be encrypted, before I lost the key of that peer due to the migration bug/behavior, I am also using chatmail as most members of the group, so the group can ONLY be encrypted, the unencrypted status came from the migration because the old address is still in the group (even for me even if the contact wrote in private, as said there was no automatic migration in the group it is a normal group, non-verified group) so I have to remove manually but I can't

@adbenitez
Copy link
Member Author

adbenitez commented May 4, 2024

It's a kind of known loose end of AEAP: your contact should have just written to the group, not to you in private and then everyone's device in the group would have followed the move. Before you scream -- i am aware that this is not optimal. I am just saying how "to hold it right" :)

see my previous message,and also in the initial post, it is a normal group so the migration can't happen by she writing in the group, that would report only "unknown sender for this chat" it is a normal group

@iequidoo
Copy link
Collaborator

An easy fix that doesn't break the current AEAP logic is to not delete the old peerstate completely (currently it is being deleted in preference of new one), but leave it with the verified_key moved to public_key (in this case the old address just loses its verified status, but i think it's minor) or to secondary_verified_key (this is a bit more complex fix, but this way the verified status can be preserved. Though i think it isn't needed).

But also we can fix this at the level of groups -- just encrypt the "member removed" message to the remaining members if we don't have a key for the being removed one.

@iequidoo iequidoo self-assigned this May 27, 2024
iequidoo added a commit that referenced this issue May 27, 2024
…whole peerstate (#5535)

When doing an AEAP transition, we mustn't just delete the old peerstate as this would break
encryption to it. This is critical for non-verified groups -- if we can't encrypt to the old
address, we can't securely remove it from the group (to add the new one instead).
iequidoo added a commit that referenced this issue May 30, 2024
…whole peerstate (#5535)

When doing an AEAP transition, we mustn't just delete the old peerstate as this would break
encryption to it. This is critical for non-verified groups -- if we can't encrypt to the old
address, we can't securely remove it from the group (to add the new one instead).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working
Projects
None yet
3 participants