iequidoo
commented
1 month ago the message will be displayed as unprotected anyway
What do you mean by this if presence of a padlock shouldn't be affected by the message content anyway?
Open link2xt opened 1 month ago
iequidoo
commented
1 month ago the message will be displayed as unprotected anyway
What do you mean by this if presence of a padlock shouldn't be affected by the message content anyway?
link2xt
commented
1 month ago the message will be displayed as unprotected anyway
What do you mean by this if presence of a padlock shouldn't be affected by the message content anyway?
If the message consists of a single text/plain part and the only contents of this part is an inline PGP message encrypted and signed by a valid key someone may expect that it should have a padlock. In Delta Chat it will not have a padlock and will be treated the same as unencrypted message.
iequidoo
commented
1 month ago I'd also suggest that if a text/plain part only containing an inline PGP message is inside another signed part in turn (e.g. inside encrypted+signed message, though i have no idea why this can happen), then don't decrypt this inline PGP message at all if we aren't going to check its signatures, otherwise the decrypted inline message with no valid signatures can be shown with a padlock.
link2xt
commented
1 month ago otherwise the decrypted inline message with no valid signatures can be shown with a padlock.
If the whole message is encrypted+signed with PGP/MIME, then it should have a padlock. The message has valid signatures, it is signed as a whole. If it is double-encrypted for some reason, it does not make it less secure.
If there is an inline PGP inside a text part it can as well be decrypted, I see no problem with displaying this message with a padlock. No need to complicate the code with conditions to skip decrypting text/plain parts if the message was encrypted+signed outside.
Anyway, such messages are likely never sent.
link2xt
commented
1 month ago Thinking more about security of this, it seems to enable replay attacks. Someone can take an encrypted message and send it as inline PGP to someone who can decrypt it. They will decrypt the message and treat as insecure message, potentially replying to it in plaintext.
This feature should be disabled for bots and should not replace inline PGP with plaintext if decrypted plaintext looks like encrypted MIME message, e.g. has a line starting with from: or mime-version: before the first blank line.
iequidoo
commented
1 month ago If the whole message is encrypted+signed with PGP/MIME, then it should have a padlock. The message has valid signatures, it is signed as a whole.
It's signed as a whole, but then the user should see exactly what is [properly] signed (i.e. not a decrypted part of it). Although such double-encrypted messages are not expected, it's only a matter of checking Mimeparser::was_encrypted() afaiu.
Thinking more about security of this, it seems to enable replay attacks. Someone can take an encrypted message and send it as inline PGP to someone who can decrypt it. They will decrypt the message and treat as insecure message, potentially replying to it in plaintext.
So, signatures of an inline PGP still must be checked. Otherwise, even if the user won't reply to the message citing it, they would think that the sender knows the message content which is also a successful attack. If there's no valid signature corresponding to the From address, such an inline PGP must be shown without decryption.
This feature [...] should not replace inline PGP with plaintext if decrypted plaintext looks like encrypted MIME message
This doesn't help if the inline PGP is taken from another message with this inline PGP, so maybe don't complicate the code with such checks? It seems that checking signatures is sufficient as PGP messages are signed-then-encrypted as far as i remember.
link2xt
commented
1 month ago So, signatures of an inline PGP still must be checked.
In cases described on the forum such as using Mailvelope we are not going to have the key because Mailvelope does not support Autocrypt: https://github.com/mailvelope/mailvelope/issues/535
So given that we cannot check the signature, maybe indeed better only decrypt inline PGP if the message is not a PGP/MIME message.
To indicate that the message was decrypted it is also fine to keep ----BEGIN PGP MESSAGE----- and ----END PGP MESSAGE-----. For advanced users who copy-paste the text to gpg anyway this is not surprising and will discourage replaying messages as they will not look normal.
iequidoo
commented
1 month ago Then i suggest instead of ----BEGIN PGP MESSAGE----- prepend the message with a warning like "The sender may not be the author of this decrypted message and may not know its content". Then it's fine to decrypt even "double-encrypted" messages shown with padlock.
link2xt
commented
1 month ago a warning like "The sender may not be the author of this decrypted message and may not know its content"
Well, that is a bit too much for a non-practical attack that is probably never executed. An attacker can also achieve the same by simply relaying a PGP/MIME message that does not have protected From:, it will be displayed without any warning at all, just without a padlock. So protecting against PGP/Inline relaying does not gain much. Besides, this scary warning will need to be translated.
To summarize, inline PGP should be decrypted if the message is within a text/plain MIME part (could be the root part), spans the whole MIME part (starts with ----BEGIN PGP MESSAGE----- and ends with -----END PGP MESSAGE-----), root message is not signed/encrypted (i.e. it is not a PGP/MIME message). The signatures should not be checked and the message should have no padlock.
iequidoo
commented
1 month ago An attacker can also achieve the same by simply relaying a PGP/MIME message that does not have protected
From:, it will be displayed without any warning at all, just without a padlock.
It can even have protected From:, but if it doesn't have valid signatures (in our view, because we e.g. don't yet know the author's pubkey which may be attached in the same message btw, or the Autocrypt header was stripped by an attacker), such a message will be just shown w/o a padlock. Also if we quote this message later, the quote won't be protected. While all this isn't about protected chats, it's still a successful attack because the user won't be aware that the message is actually confidential, moreover quoting such a message reveals its content to the whole network. This breaks the capability of contacting the user "anonymously" (i.e. w/o the previous two-side key exchange) and looks like an interoperability issue. Ok, the chat isn't protected, so e2ee isn't guaranteed, but at least i'd expect some warning in this case that makes me to take e.g. Thunderbird to gain full control over what's happening. Currently i can't just publish my Delta Chat PGP key and say "contact me confidentially using this".
link2xt
commented
1 month ago It can even have protected
From:
As far as I understand there is no place for protected From: with inline PGP, decrypted text is just plaintext without any headers.
Also if we quote this message later, the quote won't be protected.
This is already the case for encrypted but not signed messages, e.g. if someone uses a client without Autocrypt support to send a PGP/MIME message to Delta Chat this can also happen. I suggest we ignore this in this issue to avoid expanding the scope. For PGP/MIME this will be solved once we protect Autocrypt header so it will be impossible to decrypt but fail to verify the signature. For PGP/Inline I am not sure we need to care that much, if the sender wants to have the most security including the message not being quoted they should have switched from PGP/inline to PGP/mime long ago.
iequidoo
commented
1 month ago As far as I understand there is no place for protected
From:with inline PGP, decrypted text is just plaintext without any headers.
No, i'm talking about current behaviour:
if let (Some(inner_from), true) = (inner_from, !signatures.is_empty()) {
if addr_cmp(&inner_from.addr, &from.addr) {
from_is_signed = true;
from = inner_from;
} else {
// There is a From: header in the encrypted &
// signed part, but it doesn't match the outer one.
// This _might_ be because the sender's mail server
// replaced the sending address, e.g. in a mailing list.
// Or it's because someone is doing some replay attack.
// Resending encrypted messages via mailing lists
// without reencrypting is not useful anyway,
// so we return an error below.
warn!(
context,
"From header in signed part doesn't match the outer one",
);
// Return an error from the parser.
// This will result in creating a tombstone
// and no further message processing
// as if the MIME structure is broken.
bail!("From header is forged");So, if we don't find valid signatures, we accept the message. I think we should reject it as well, otherwise the outer From can be forged and if someone tries to contact the user confidentially attaching a public key, the user would see the wrong address and may reply to that address. Moreover, the reply would be unencrypted. (EDIT: fixed in 04fd2cdcab6b0a9486cca2e64f2196bc2d209b80.) That's why i also think we should warn users about encrypted messages whose signatures can't be checked, but this is a separate topic.
This is already the case for encrypted but not signed messages, e.g. if someone uses a client without Autocrypt support to send a PGP/MIME message to Delta Chat this can also happen.
Yes, or for messages signed with an attached key, or if the Autocrypt header was stripped off.
Delta Chat currently sends PGP/MIME messages and does not support PGP/Inline.
This has been requested before as an issue and on the forum:
For minimal support we need to detect text/plain parts that start with
----BEGIN PGP MESSAGE-----and end with----END PGP MESSAGE-----and replace them with the decrypted contents if possible. This should not affect "encrypted" (padlock) status of the message, no signatures need to be verified (the message will be displayed as unprotected anyway) and no error should be reported if decryption fails, just keep the text part as is. Also no need to support messages which have only some part encrypted. PGP/Inline can also encrypt attachments, decrypting them is also out of scope for this issue.