element-hq / element-ios

A glossy Matrix collaboration client for iOS
https://element.io
GNU Affero General Public License v3.0
1.73k stars 493 forks source link

Encrypt m.new_content separately for message edits to aid server-side aggregation #2536

Open manuroe opened 5 years ago

manuroe commented 5 years ago

From riot-web created by bwindels: vector-im/riot-web#10203

Server-side aggregation of m.replace events should replace the original event's content with the m.new_content in the content of the replacing event. As it stands, riot on all platforms encrypts m.new_content as part of encrypting the whole content, so the server can't access it.

As a consequence, clients that don't support edits (and ignore m.new_content) would show the fallback content of the replacement when receiving the original message after server-side aggregation has happened. The original event would also look like a relation, although that can be safely ignored by unsupporting clients.

We need to encrypt m.new_content separately, so the server can pick it out to replace the original content with it, while preserving the rest of the original message.

The new format of an edit in an encrypted room (after encryption) would look like this:

"content": {
    "algorithm": "m.megolm.v1.aes-sha2",
    "ciphertext": "AwgDErABDBWSkLKJ5IQRePEP/kjpmVETrDzJp5jpWRJ0IIB/XAI0i6J8Cgpe/UEzhB9IpwJCMaWh7QKqN1h4kJn+nA55S/VgJEE01/g5Trihg5YGsaC5KGhEuH6gGG4tbtgX9hhwdN4PyTvmdSWxMtS8egKKBwS7Wux/461BX4UsSz25XqECIu6P8u7X/uBh5c6tTae6wTLVCvUTDtdYUnRk956GAbDnYgc27m8A16W8ex2BcgIE2MkzGJphZUm6PMa7vC9QOdTNACpylxtCrSEJhUWH8y4KseVXavVLi28lt4QhoRw8dLcId6LDJfexf+I8wMFr4HQ6DSd6iAg",
    "device_id": "JHYBCAOWGF",
    "m.relates_to": {
        "event_id": "$5xJzoD0Cbz7vxIFcyPOCIJ2ORy4_MpHQUW4nIUdlmvg",
        "rel_type": "m.replace"
    },
    "sender_key": "Ern8B3Dj56iXRvFeubfe5PDHp8bYWaHZbLiUcK1oAAc",
    "session_id": "6/cfdpp8mMCEgbDJsmxAY66sBHlDeTfarVLeLu5VRzw",
    "m.new_content": {
        "algorithm": "m.megolm.v1.aes-sha2",
        "ciphertext": "AwgDErABDBWSkLKJ5IQRePEP/kjpmVETrDzJp5jpWRJ0IIB/XAI0i6J8Cgpe/UEzhB9IpwJCMaWh7QKqN1h4kJn+nA55S/VgJEE01/g5Trihg5YGsaC5KGhEuH6gGG4tbtgX9hhwdN4PyTvmdSWxMtS8egKKBwS7Wux/461BX4UsSz25XqECIu6P8u7X/uBh5c6tTae6wTLVCvUTDtdYUnRk956GAbDnYgc27m8A16W8ex2BcgIE2MkzGJphZUm6PMa7vC9QOdTNACpylxtCrSEJhUWH8y4KseVXavVLi28lt4QhoRw8dLcId6LDJfexf+I8wMFr4HQ6DSd6iAg",
        "device_id": "JHYBCAOWGF",
        "sender_key": "Ern8B3Dj56iXRvFeubfe5PDHp8bYWaHZbLiUcK1oAAc",
        "session_id": "6/cfdpp8mMCEgbDJsmxAY66sBHlDeTfarVLeLu5VRzw"
    }

Also see discussion on matrix.

manuroe commented 5 years ago

Unsure about priority, the only downside atm is that non-supporting clients will show the fallback content, even after server-side aggregation ... Which doesn't seem like a disaster to me, but would be good not to change the format after taking the feature out of labs.

manuroe commented 5 years ago

Unsure if this needs any changes in synapse, e.g. whether or not it's blocked by backend.