feat: add enforce_outer_from_key_alignment config for bots using addresses as identity#8433
feat: add enforce_outer_from_key_alignment config for bots using addresses as identity#8433hpk42 wants to merge 2 commits into
Conversation
From address is not used for key contacts other than as the address to send replies to.
…esses as identity If enabled (default is disabled) the processing of encrypted messages changes: - the outer From address must be listed as a relay address in the signing key, otherwise the message is trashed, - the sender contact keeps the outer From address instead of adopting the one from the encrypted part. Otherwise an attacker sending from their own relay-enforced address, with their own unmodified key, puts the victim's address into the encrypted From header and the contact address becomes the victim's. Arguably a fix: Autocrypt headers from the encrypted part are now accepted without comparing their address to the outer From, so that a newly added relay address is imported from the message before the check.
198d98e to
285e8b0
Compare
iequidoo
left a comment
iequidoo
left a comment
There was a problem hiding this comment.
Commit message: feat: add enforce_outer_from_key_alignment config **needed** for bots using addresses as identity. Otherwise it's not clear whether this config only works in presense of Config::Bot (actually not)
| /// Only the outer From is enforced by the sender's relay, | ||
| /// so only it can be used as an identity. | ||
| #[strum(props(default = "0"))] | ||
| EnforceOuterFromKeyAlignment, |
There was a problem hiding this comment.
For me it looks like UseOuterFromAsContactAddr would be a better naming:
- It's what actually happens if the outer From differs from the inner one, but is still aligned with sender's key.
- Actually, alignment with the key is just an additional check which protects from the server changing the outer From to any address. The bot (or bot's server) still needs to check somehow (using DKIM i guess) that the outer From is actually owned by one who scanned the invite link.
- Alignment of the outer From with the key isn't checked if the outer From is the same as the inner one, and without reading the doc comment above it's not clear.
| Ok(()) | ||
| } | ||
|
|
||
| /// Tests that a sender key without relay addresses is accepted |
There was a problem hiding this comment.
Is there any profile without relay addresses in the key? As far as i understand both alice and bob get "pseudo transport" added as soon as their address is configured (here:
Lines 760 to 774 in bdd9d96
There was a problem hiding this comment.
UPD: i see, the test unpublishes all relays via SQL.
| for val in aheader_values.iter().rev() { | ||
| autocrypt_header = match Aheader::from_str(val) { | ||
| Ok(header) if addr_cmp(&header.addr, &from.addr) => Some(header), | ||
| Ok(header) if aheader_protected || addr_cmp(&header.addr, &from.addr) => { |
There was a problem hiding this comment.
I am not sure all this logic with aheader_protected and even address comparison is needed. We can simply take the last header with aheader_values.iter().rev().next() or something like that and not look at the address. There is nothing wrong that can happen if the address is wrong, worst case someone places forged Autocrypt header on a message that does not have Autocrypt header in the protected part, then we will take the header, add it to the "keyring" and try to verify the signature with this key. Don't see how this can do any harm, there is no way to overwrite the key for some contact, everything you can do by forging Autocrypt header you can do by asking someone to import a vCard.
There was a problem hiding this comment.
There are two functions of the Autocrypt header:
- Distributing the keys. This is the same as using Autocrypt-Gossip and vCards, it does not matter who sent you the Autocrypt header and if they are actually the owner of the key.
- Providing the key for verifying the signature if the signature does not have the issuer fingerprint (we send and receive the issuer fingerprint, see feat: lookup public keys by signature issuer fingerprint #7114). This is just because we don't want to try verifying the signature that does not have the issuer fingerprint against all the public keys we know. But there is no harm in trying to verify the signature against the wrong key.
In both cases the email address does not matter and it does not matter if the Autocrypt header is sent by the key owner.
3 participants
this is currently on hold as it might be possible to change just loginbot which could create pseudo/shadow addresses to avoid impersonation attacks
If the alignment option is enabled (default is disabled) the processing of encrypted messages changes:
the outer From address must be listed as a relay address in the signing key, otherwise the message is trashed,
the sender contact keeps the outer From address instead of adopting the one from the encrypted part.
Otherwise an attacker sending from their own relay-enforced address, with their own unmodified key, puts the victim's address into the encrypted From header and the contact address becomes the victim's.
Arguably a fix: Autocrypt headers from the encrypted part are now accepted without comparing their address to the outer From, so that a newly added relay address is imported from the message before the check.