fix: receive_imf: Look up key contact by intended recipient fingerprint (#7661) #7786
Conversation
iequidoo
changed the title
iequidoo
force-pushed
the
iequidoo/intended-recipient-fps
branch
from
ff3406a to
52a8a4d
Compare
src/pgp.rs
Outdated
| chrono::Utc::now().trunc_subsecs(0), | ||
| ))?); | ||
| for key in &public_keys_for_encryption { | ||
| if private_key_for_signing.dc_fingerprint().hex() |
There was a problem hiding this comment.
I'm really not fan of having custom logic that is only intended for tests outside the tests. Would rather have test_recv_outgoing_msg_before_securejoin testing only the happy path of everyone using the new client. And if regressions happens, store encrypted messages as blobs in the test data and replay them instead of maintaining the code to regenerate messages as if they are generated by old core.
There was a problem hiding this comment.
Not blocking, but if we in the future change the code around here e.g. because this subpacket generation is moved to something like rpgpie or separate crate, will have to change the tests. And it allocates fingerprint string every time message is sent for each recipient just to evaluate to false.
There was a problem hiding this comment.
And it allocates fingerprint string every time message is sent for each recipient just to evaluate to
false.
Thanks, fixed this.
Overall the idea is not even to test compatibility with older versions, but to have the fallback code assigning messages to non-replyable ad hoc groups, if key-contacts can't be looked up for whatever reason, tested, to not offer unencrypted replies forbidden by the standard. Maybe we could just trash messages if so, and add some device message, this depends on how interoperability is important currently.
|
#7057 is not closed because there is no test? |
…eferences messages in ad-hoc group This is an important thing forgotten to be checked in 3325270. Also there's another test which currently doesn't work as we want: outgoing encrypted messages continue to arrive to ad-hoc group even if we already have contact's key. This should be fixed by sending and receiving Intended Recipient Fingerprint subpackets. The good thing is that apparently there are no scenarios requiring the contact to update their software, the user should just update own devices.
It's strange that this wasn't covered by any test.
…contacts_fallback_to_chat() It only looks up contacts by address in the given chat, so `_fallback_to_chat` suffix is more informative. It's obvious that such a lookup is done by address, there are no other reasonable options.
iequidoo
force-pushed
the
iequidoo/intended-recipient-fps
branch
from
52a8a4d to
48467fb
Compare
I just forgot about that issue. It tells that a message not intended to us should appear as an unencrypted message assigned to the outer From. But The standard says
So we can consider such a message as "not valid" and trash it, IMU. EDIT: "it" refers to the signature, so it's signature who is invalid, not the message, so probably indeed we should assign such a message to the outer From and show it as unencrypted. Otherwise everything important is tested it seems. EDIT: However, we can already merge this and handle such broken signatures in the right way later. This doesn't look very important. Trashing such messages is also acceptable, basically we can say that we just don't support them. EDIT: Another option is to treat such messages as forwarded, but still add them as unencrypted. Not going to implement anything of this for now however. |
…nt (#7661) For now, do this only for `OneOneChat` and `MailingListOrBroadcast`, this is enough to correctly support messages from modern Delta Chat versions sending Intended Recipient Fingerprint subpackets and single-recipient messages from modern versions of other MUAs.
iequidoo
force-pushed
the
iequidoo/intended-recipient-fps
branch
from
48467fb to
1664be5
Compare
For now, do this only for
OneOneChatandMailingListOrBroadcast, this is enough to correctlysupport messages from modern Delta Chat versions sending Intended Recipient Fingerprint subpackets
and single-recipient messages from modern versions of other MUAs.
Close #7661
test: Message in blocked chat arrives as InSeen-- unrelated, but just a one-line test improvement.