fix: Create new Peerstate for unencrypted message with already known Autocrypt key, but a new address

[iequidoo]

See commit messages.

This is aimed to solve the @adbenitez's problem with sharing the same key by several accounts.

[iequidoo]

One more occurence of #5201:
https://github.com/deltachat/deltachat-core-rust/actions/runs/7926082001/job/21640415854?pr=5269

[link2xt]

Could you rebase it and test with #5274 merged?

vc-request cannot do AEAP because it is not encrypted. It was only encrypted due to a bug.

[iequidoo]

Could you rebase it and test with #5274 merged?

vc-request cannot do AEAP because it is not encrypted. It was only encrypted due to a bug.

It still doesn't work. Apparently AEAP doesn't happen, but a Peerstate isn't created for the new address and that should be fixed. So, instead of looking at the SecureJoin message type, we should check if the message is encrypted.

[iequidoo]

Finally i think that relying on encryption here is not right. vc-request can easily be signed (w/o encryption) and that would be a good reason for doing AEAP. Of course we can state in the SecureJoin doc that vc-request should never be encrypted or signed (as now), but i'd better exculde it explicitly for AEAP.

[iequidoo]

#5280 again:
https://github.com/deltachat/deltachat-core-rust/actions/runs/7999217931/job/21846663121?pr=5269

[link2xt]

I still don't understand why vc-request is special. What if it is a vg-request? What if first message coming from a new address uses existing key, will it also fail to create peerstate?

@adbenitez has scanned a QR code and sent vc-request's, but if just a "hello!" text message is sent, will it fail the same way?

[iequidoo]

I still don't understand why vc-request is special.

I thought it's a good heuristic because vc-request means that it's a new contact, not the existing one changed its address. But not only vc-request means that, you're right. We should check if a message is encrypted. And do AEAP only if the message is encrypted + signed with a verified key. Otherwise a new peerstate must be created, but we must make sure that this doesn't break the possibility of a further AEAP because this may be an attack if the message isn't signed.

What if it is a vg-request?

It won't work the same way.

What if first message coming from a new address uses existing key, will it also fail to create peerstate?

Yes.

@adbenitez has scanned a QR code and sent vc-request's, but if just a "hello!" text message is sent, will it fail the same way?

Almost. The difference is that Alice will answer unencrypted because a new peerstate isn't created. But anyway this is wrong. So, the solution is either do AEAP or create a new peerstate, there is no third option.

[Hocuri]

Phew, quite a complicated-to-fix bug, thanks for digging in!

Off-topic

FWIW right now both AEAP and DKIM-checking don't really seem to be worth the complexity, maybe we can make it work nicely or maybe we can think about removing it. Nothing for now, though.

For context, AEAP was an NLnet-funded project, but it turned out to be harder to implement securely than thought. DKIM-checking was one of the attempts to implement it securely.

[link2xt]

With AEAP it is unclear what should happen if we have two contacts with the same key in a verified group and then someone writes to this group with the same key from a third address. Do we remove all contacts and replace with a single one? Currently I think Delta Chat will only AEAP-port one of them arbitrarily selected.

[iequidoo]

With AEAP it is unclear what should happen if we have two contacts with the same key in a verified group and then someone writes to this group with the same key from a third address. Do we remove all contacts and replace with a single one? Currently I think Delta Chat will only AEAP-port one of them arbitrarily selected.

Yes, but not arbitrarily selected -- the recently appeared one. Anyway this looks strange, yes. But i decided not to change this right now. Probably removing all old contacts should be done, need to think a little of it and fix in a separate PR.

[Hocuri]

Ta-Daaaaa, one bug less!

[iequidoo]

With AEAP it is unclear what should happen if we have two contacts with the same key in a verified group and then someone writes to this group with the same key from a third address. Do we remove all contacts and replace with a single one? Currently I think Delta Chat will only AEAP-port one of them arbitrarily selected.

Yes, but not arbitrarily selected -- the recently appeared one.

Finally i think that in such a case it's better not to port any contact at all. We don't know which one to port. If we port both (i.e. replace them with the new one), then it's a question why they both exist before (probably because the key is shared). Sharing a key isn't a common scenario, if we want AEAP to work in this case too, we better add an additional attribute to the Autocrypt header, smth like setup-id.

So, for now i suggest to avoid doing AEAP if the key is shared.