feat: introduce keyupdate message to inform contacts about relay changes - #8601
feat: introduce keyupdate message to inform contacts about relay changes#8601hpk42 wants to merge 1 commit into
Conversation
a4c6195 to
337c787
Compare
When the published relay list changes, a symmetrically encrypted keyupdate message carries the re-signed key, and is broadcast to all non-blocked key-contacts from accepted chats. The encryption secret is derived from the public primary key packet, so every holder of a copy of that key can decrypt, forever. For more details see src/keyupdate.rs module docs.
337c787 to
a31f144
Compare
link2xt
left a comment
link2xt
left a comment
There was a problem hiding this comment.
Haven't really reviewed, some very high level comments and moving discussion from a previous place for now
| pub(crate) fn keyupdate_secret(public_key: &SignedPublicKey) -> Result<String> { | ||
| let mut hasher = Sha256::new(); | ||
| hasher.update(b"keyupdate"); | ||
| hasher.update(Serialize::to_bytes(&public_key.primary_key)?); |
There was a problem hiding this comment.
Autocrypt 2 in a similar setting hashes the public key packet together with the packet header. There is probably no good reason to hash the packet header, in which case I think we should remove it from Autocrypt 2. If there is a reason why Autocrypt 2 hashes the packet like this, then it should be hashed in a similar way here. Otherwise we will eventually end up with similar code doing the same operation slightly different in the same codebase.
| // Receivers drop symmetrically encrypted messages | ||
| // that are not signed by the contact owning the secret. | ||
| should_sign: true, | ||
| // Compression normalizes the size: uncompressed, the length grows |
There was a problem hiding this comment.
If anything, compression leaks information about the actual data, and is discouraged. We still use it to negate base64 overhead for base64-encoded attachments (which should likely be sent as binary in encrypted messages), but if you actually want to protect the data in key updates, it should be padding rather than compression.
| }, | ||
| // The attached key with its relay list notation is the actual payload. | ||
| should_attach_pubkey: true, | ||
| // Receivers drop symmetrically encrypted messages |
There was a problem hiding this comment.
As I commented in #8595 (comment), as far as I see we process Autocrypt header before checking the signatures, so also before trashing the messages that are not signed by expected sender for symmetrically encrypted ones. There might be reasons to sign, but I don't think this is a problem. We actually want key update messages to only update the key, and otherwise it is good that they are dropped, not assigned to the key-contact and not update the "last seen".
| } | ||
|
|
||
| /// Inserts a rendered message into the `smtp` table for sending. | ||
| pub(crate) async fn insert_into_smtp( |
There was a problem hiding this comment.
For other reviewers: this is moved as is from securejoin.rs except for the added doc comment.
|
|
||
| /// Keyupdate message informing contacts about the current key and relay list. | ||
| /// Never shown in chats: receivers apply the key and then trash the message. | ||
| Keyupdate, |
There was a problem hiding this comment.
If this can be avoided, i'd remove new message type. Otherwise we are changing recipients and need to wait for all recipients to upgrade so they don't show such messages. This only does not matter because old recipients don't know how to decrypt the messages anyway.
But otherwise i'd use unsolicited securejoin key response message. Or MDN, whatever.
| let rows = context | ||
| .sql | ||
| .query_map_vec( | ||
| "SELECT c.addr, k.public_key |
There was a problem hiding this comment.
I still think this should have at least some limit. If you really think it should be unlimited, then LIMIT 9999 or so.
2 participants
Introduces keyupdate messages: when the published relay list changes, a symmetrically encrypted message carries the re-signed key to non-blocked key-contacts from accepted chats.
Wire format, audience and sending policy:
src/keyupdate.rsmodule docs.For full background and motivation, see https://github.com/chatmail/core/blob/hpk/draft-keyupdates/draft/keyupdate-draft.md as discussed in #8595.
You can use the following SQL statement to check the number of contacts versus keyupdate recipients as selected by this PR:
For some profiles I have access to, some real-world data on keyupdate recipient sizes:
Even for the oldest profile, reaching ~2000 key contacts can re-establish chat connectivity that is currently broken (the "mutual silence" case), so it's not clear that constraining the recipient set further would be worth the loss. Most likely there will be a lot of failure DSNs that we currently don't do much with, but that's arguably a general problem for all other chat messages, too.
Known problem: old contacts who uninstalled Delta Chat but still read the mailbox with another client
Key-contacts who once used Delta Chat on a classic email provider and later uninstalled might receive keyupdates as undecryptable mail in a mailbox they still read (given the keyupdate passes the spam filter). They have no way to opt out, and we get no signal either, because their provider may accept the mail, without failure DSNs. With relay changes currently being rare manual acts, this probably stays below the noise that group messages or other randomly arriving spam can already cause for such contacts. But it might become a real annoyance with frequent relay changes: automatic relay management should be careful to ship without rate-limiting keyupdates in some way. Note, however, that this whole problem usually only exists for old profiles, and then only for their contacts who uninstalled Delta Chat and continue to use the mailbox with another client, so we should be careful not to optimize for this particular case at the expense of helping "mutually silent" contacts, or peers with otherwise de-synchronized relay knowledge about each other, to re-establish chat connectivity so message sending will succeed afterwards.