deltachat / deltachat-core-rust

Delta Chat Rust Core library, used by Android/iOS/desktop apps, bindings and bots 📧
https://delta.chat/en/contribute
Other
658 stars 83 forks source link

Ignore download limit in protected (verified, green-checkmarked) 1:1 chats #5003

Open link2xt opened 9 months ago

link2xt commented 9 months ago

Here is a message with a large video and verified signature is received:

We don't know yet that it has a signature of a verified key. When we download it, the message appears in place but fixes the chat type, it becomes verified:

This is unexpected because a message with actually a good signature appears in the section marked as unverified, above the large green checkmark.


Later, we get a green checkmark back: 1

Then we receive a large message and don't know yet that it is signed with unverified key: 1

"Setup changed" is due to Autocrypt key change which comes from already downloaded Autocrypt header.

Now we download the message: 1

Green checkmark breaks, but the video with wrong signature appears in the "guaranteed e2e" section!

link2xt commented 9 months ago

No idea how to fix it easily, one option would be ignoring the download limit in verified chats and just download everything immediately for now.

missytake commented 9 months ago

No idea how to fix it easily, one option would be ignoring the download limit in verified chats and just download everything immediately for now.

That would probably be fine I think? iirc, most people we know of who need to watch out for their mobile data don't use encryption anyway? https://github.com/deltachat/bg-deltachat/blob/master/ux-feedback/personas.md#valentina-the-2g-communicator

missytake commented 9 months ago

Hm, on the other hand that is pretty invasive. What about this:

case 1 - the large message would fix the verified chat

If we are in a e2ee-broken chat, and we receive a too large message with an autocrypt header which contains a key we have verified, we show:

And as soon as the user clicks download, we delete the "1.79 MB message: download", show "e2ee-established", and re-insert the downloaded message, so that it looks like this:

case 2 - the large message has a wrong autocrypt header, so we suspect it to break the verified chat

If we are in a e2ee-established chat, and we receive a too large message with an autocrypt header which contains a key we don't know yet, we show:

And as soon as the user clicks download, we delete the "1.79 MB message: download", show "e2ee-broken", and re-insert the downloaded message, so that it looks like this:

case 3 - the large message has a correct autocrypt header, but breaks the verified chat

If we are in a e2ee-established chat, and we receive a too large message with an autocrypt header which contains a key we have verified, but has a wrong signature, we obviously just show this before downloading:

And as soon as the user clicks download, and we realize that the signature is wrong, we delete the "1.79 MB message: download", show "e2ee-broken", and re-insert the downloaded message, so that it looks like this:

I'm not deep enough into the code structure to know if we can easily do this... it basically boils down to ignore the large message until it is downloaded, and then reprocess all messages since then? But if there have been more messages after that, it might get more complicated. Do we re-process them?

r10s commented 9 months ago

This is unexpected because a message with actually a good signature appears in the section marked as unverified, above the large green checkmark.

i do not think, "too good encryption" is an issue at all. most ppl would probably not even recognise that the message is fine although the chat is declared as broken, also the user just did a download action, so this is somehow expectable. the other way round is worse:

No idea how to fix it easily, one option would be ignoring the download limit in verified chats and just download everything immediately for now.

if that would fix the issue and is easily doable, this seems a pragmatic way to go for; i do not think, it is too invasive, also it does not affect groups.

after 1.42 is out, we can reconsider and go for a more advanced option, if possible. eg. show downloaded messages that break guarantee not in place but at the end of the chat, after the "other device" message (probably not that bad as 1:1 chats are usually not that chatty and you can follow context more easily than in groups)

missytake commented 9 months ago

Ah, I missed that it doesn't affect groups. Then it is likely fine to just download large messages in verified 1:1 chats regardless of the setting; in 1:1 chats it's much more likely that the large message is actually interesting for you than in groups.

link2xt commented 9 months ago

Ok, so ignoring download limit in 1:1 chat if 1:1 chat is verified seems like a solution.

link2xt commented 9 months ago

It is actually not possible to know if the message belongs to a 1:1 chat before we download it. So we have no simple core solution.

r10s commented 9 months ago

hm, but usually, the messages are sorted to the correct chat before downloading, or? iirc, chat-changes-after-download mostly happens when the sender does not use dc. groups are detected reliable enough.

so, maybe it is good enough if we ignore the download limit if we assume by best guess a message belongs to a 1:1 chat (if this is not the case, then it's that) maybe we need to download all messages that do not have a grp-id.

or can we show downloaded messages that break guarantee not in place but at the end of the chat?

wondering also, how bad the issue would be in practise if we ignore the issue for now. the security impact would be mostly confusion iiuc

link2xt commented 9 months ago

hm, but usually, the messages are sorted to the correct chat before downloading, or?

Ok, true.

It's a bit tricky because we decide not to download the full message based on the size, then it gets into receive_imf and creates a "click to download" stub after being. I definitely do not want to pass the message to receive_imf just to find out which chat the message gets sorted to.

We can just look at the From field, find the corresponding 1:1 chat and check if the To field has no other recipients. This will not to find out if we are in a 1:1 group, but they should be rare. We want to remove group ID from the Message-ID header and protect the To: field eventually, so I would rather not rely too much at least on the Message-ID leaking the group ID for compatibility. Maybe relying on In-Reply-To and References is ok.

wondering also, how bad the issue would be in practise if we ignore the issue for now. the security impact would be mostly confusion iiuc

Yes, I also think it is ok not to solve it for 1.42.

Hocuri commented 9 months ago

wondering also, how bad the issue would be in practise if we ignore the issue for now. the security impact would be mostly confusion iiuc

Yes, I also think it is ok not to solve it for 1.42.

+1. And, you click on "Download" -> a warning pops up. Usually it will be clear that the warning might be connected to the message you just downloaded.

or can we show downloaded messages that break guarantee not in place but at the end of the chat?

That should work, too.

Septias commented 9 months ago

Or can we show downloaded messages that break the guarantee not in place but at the end of the chat?

This doesn't solve the problem of big messages creating the verified status though. For this, we would have to find the next verified section and move it downward to the top of it. Anyway, this resorting is also what I would expect from a user's point of view. Maybe @missytake has a point that we could just "forget" that we have ever seen this message and receive it as if it were a normal message, ignoring the size limit. This would then produce an out of order receival of messages which could then again affect group consistency so maybe we should only do it for said 1:1 chats.

link2xt commented 9 months ago

An option to solve this properly would be to implement a dedicated UI for verification problem #4999 and then display downloaded message in place, not break the verification state of the chat but hide the message which failed verification.

zcattacz commented 6 months ago

How about putting the technically suspicious items together into a separate list, insert a link for user to locate and inspect in that list, if they are interested. If an attacker is sending a lot of misleading material, I'd rather them not to be displayed to me inline like other messages. If it is a technical issue, it maybe useful to have some place to collect all these failed items with timestamps etc.

link2xt commented 6 months ago

The messages are not suspicious, large messages are sent by non-attackers all the time and in most cases when you download them they have a valid signature.

zcattacz commented 6 months ago

exactly, then why do I, as a user, need to know about those distractions ?

iequidoo commented 6 months ago

There's no sense to hide large messages with not yet checked signatures in protected chats, if it's an attack then there's a server allowing From forgery, but then such a network has many other ways to cause out-of-service for protected chat users.