-
Couldn't load subscription status.
- Fork 414
MSC2730: Verifiable forwarded events #2730
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: old_master
Are you sure you want to change the base?
Changes from 4 commits
57d91c8
3987938
01cb1ab
fa61324
548a5a3
823dbf7
018c92d
fc9ea6a
a0b3ef6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,251 @@ | ||
| # Verifiable forwarded events | ||
| This is an alternative to [MSC2723](https://github.com/matrix-org/matrix-doc/pull/2723) | ||
| that handles the issue of faking forwards. | ||
|
|
||
| ## Proposal | ||
| The proposed solution is copying the entire federation event data, which allows | ||
| recipients to validate the signatures even if they are not in the origin room. | ||
|
|
||
| As clients generally don't have access to signatures nor any way to validate | ||
| them, both sending and validating require server support. Sending is | ||
| implemented as a new endpoint, while validating happens automatically and the | ||
| server adds the validation result to the top-level `unsigned` object. | ||
|
|
||
| ### `PUT /_matrix/client/r0/rooms/{roomId}/event/{eventId}/forward/{targetRoomId}/{txnId}` | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: This seems like the wrong place for this. We aren't modifying There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The The current path is that way because you're doing something with the event. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But you aren't actually doing something to the event. You are creating a new event that is based on it and references it. |
||
| This endpoint requests the server to find `eventId` from `roomId` and forward | ||
| it to `targetRoomId`. The `txnId` behaves the same way as in the `/send` | ||
| endpoint. The request body is an empty JSON object. | ||
|
|
||
| Only unredacted message events can be forwarded. If the given event ID is a | ||
| state event, a redaction or a redacted message event, the request will be | ||
| rejected with a standard error response using the code `M_NOT_FORWARDABLE`. | ||
|
|
||
| If the generated event is too large, the request is rejected with a standard | ||
| error response using the code `M_TOO_LARGE`. Before rejecting the request, | ||
| servers MAY check if the event would be small enough without the profile data | ||
| in `unsigned`, and send the event without that data if it is. | ||
|
|
||
| Similar to the `/send` endpoint, this endpoint returns an object containing the | ||
| `event_id` of the forwarded event. | ||
|
|
||
| #### Generating forwarded event | ||
| To forward an event, the server creates a new event with the same event type | ||
| and normal top-level fields. To determine the content, the server has to | ||
| inspect the content of the source event: | ||
|
|
||
| * If the source event was already forwarded from some other room, the `content` | ||
| should simply be copied with no modifications. This means that an event | ||
| forwarded many times will only remember the original source, not any hops it | ||
| made on the way. | ||
| * If the source event was not a forward, but contains (invalid) data in the | ||
| `m.forwarded` key, the request will be rejected with `M_NOT_FORWARDABLE` like | ||
| other unforwardable events. This limitation also can be used to intentionally | ||
| mark messages as unforwardable (e.g. `"m.forwarded": {"allow": false}`). | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Putting "invalid" data in this key seems like something that we shouldn't recommend. If we are going to recommend this I would prefer to update the forwarding field to explicitly allow this option. |
||
| * If the source event does not contain `m.forwarded` at all, the server must | ||
| generate a new one. After generating the object, it is placed in `content` of | ||
| the new event along with everything from the `content` of the source event. | ||
|
|
||
| ##### Generating `m.forwarded` object | ||
| `m.forwarded` is an object that contains all the top-level keys of the source | ||
| event, except for `type`, `content` and `unsigned`. The following keys are | ||
| therefore at least required: | ||
|
|
||
| * `auth_events` | ||
| * `prev_events` | ||
| * `room_id` | ||
| * `sender` | ||
| * `depth` | ||
| * `origin` | ||
| * `origin_server_ts` | ||
| * `hashes` | ||
| * `signatures` | ||
|
|
||
| The following keys may also be present: | ||
|
|
||
| * `prev_state`, may be present as an empty array even in non-state events | ||
| * `event_id`, only in v1 and v2 rooms | ||
|
|
||
| Additionally, when generating `m.forwarded` objects, the server SHOULD include | ||
| its own `unsigned` object in the `m.forwarded` data that contains the | ||
| `displayname` and `avatar_url` of the sender. | ||
|
|
||
| #### Example | ||
|
|
||
| <details> | ||
| <summary>Source event (federation format)</summary> | ||
|
|
||
| ```json | ||
| { | ||
| "auth_events": [ | ||
| "$wChClfXonLE8RZikJ446AXvRpbh_JjDK8sNpMpZbqPs", | ||
| "$RaXN_RayMvoEmMnUHlZScIdSpShT8zggd4p6qcQk9L8", | ||
| "$kFop6R7AohiYSTh_ijUctTujdVTg3rwBPdaMLeZMNrg" | ||
| ], | ||
| "prev_events": [ | ||
| "$pIFO6_sI1Ul_3jPixtbnJn_h0Pe0yB__TJD_VCW9Q-Q" | ||
| ], | ||
| "type": "m.room.message", | ||
| "room_id": "!FIIWlyqwNLyMAtmRBF:maunium.net", | ||
| "sender": "@tulir:maunium.net", | ||
| "content": { | ||
| "msgtype": "m.text", | ||
| "body": "test" | ||
| }, | ||
| "depth": 115, | ||
| "prev_state": [], | ||
| "origin": "maunium.net", | ||
| "origin_server_ts": 1597257769634, | ||
| "hashes": { | ||
| "sha256": "xBR7NmH2WQBx0auQWEDEYNbcPf9ATlDSwkv9EBxueMI" | ||
| }, | ||
| "signatures": { | ||
| "maunium.net": { | ||
| "ed25519:a_xxeS": "cc9XnH9ByO7yadC6CdMhh3c/TN1tQ9FiZdKYyRDi4Og1dZMylmBM9uSI7c4GUEqswLBLxW5DTFU3n7vMHAGhAw" | ||
| } | ||
| }, | ||
| "unsigned": { | ||
| "age_ts": 1597257769634 | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
| Request: | ||
|
|
||
| ``` | ||
| PUT /_matrix/client/r0/rooms/!FIIWlyqwNLyMAtmRBF:maunium.net/event/$BfxMy-oNFOeE0eFt6r-l3h7MtwNVIX0GrructyJq1wA/forward/!eVRGrjZQgJZGNllOkw:grin.hu/myTxnId1 | ||
| {} | ||
| ``` | ||
|
|
||
| Response: | ||
|
|
||
| ```json | ||
| { | ||
| "event_id": "$r8h8W9A5KS8D65_Df8fwLkTe7aqOm48KmyaJ6tRNAmE" | ||
| } | ||
| ``` | ||
|
|
||
| <details> | ||
| <summary>Forwarded event (client format)</summary> | ||
|
|
||
| ```json | ||
| { | ||
| "type": "m.room.message", | ||
| "room_id": "!eVRGrjZQgJZGNllOkw:grin.hu", | ||
| "event_id": "$r8h8W9A5KS8D65_Df8fwLkTe7aqOm48KmyaJ6tRNAmE", | ||
| "sender": "@tulir:maunium.net", | ||
| "origin_server_ts": 1597263764138, | ||
| "content": { | ||
| "msgtype": "m.text", | ||
| "body": "test", | ||
| "m.forwarded": { | ||
| "auth_events": [ | ||
| "$wChClfXonLE8RZikJ446AXvRpbh_JjDK8sNpMpZbqPs", | ||
| "$RaXN_RayMvoEmMnUHlZScIdSpShT8zggd4p6qcQk9L8", | ||
| "$kFop6R7AohiYSTh_ijUctTujdVTg3rwBPdaMLeZMNrg" | ||
| ], | ||
| "prev_events": [ | ||
| "$pIFO6_sI1Ul_3jPixtbnJn_h0Pe0yB__TJD_VCW9Q-Q" | ||
| ], | ||
| "room_id": "!FIIWlyqwNLyMAtmRBF:maunium.net", | ||
| "sender": "@tulir:maunium.net", | ||
| "depth": 115, | ||
| "prev_state": [], | ||
| "origin": "maunium.net", | ||
| "origin_server_ts": 1597257769634, | ||
| "hashes": { | ||
| "sha256": "xBR7NmH2WQBx0auQWEDEYNbcPf9ATlDSwkv9EBxueMI" | ||
| }, | ||
| "signatures": { | ||
| "maunium.net": { | ||
| "ed25519:a_xxeS": "cc9XnH9ByO7yadC6CdMhh3c/TN1tQ9FiZdKYyRDi4Og1dZMylmBM9uSI7c4GUEqswLBLxW5DTFU3n7vMHAGhAw" | ||
| } | ||
| }, | ||
| "unsigned": { | ||
| "displayname": "tulir", | ||
| "avatar_url": "mxc://maunium.net/jdlSfvudiMSmcRrleeiYjjFO" | ||
| } | ||
|
Comment on lines
+169
to
+172
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This example is missing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To expand on this what if the room version is not supported by clients? I guess in most cases it will just work but it seems unfortunate if a client can't see a forward in a room with a supported version because the event is from a newer versioned room. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clients don't need to support the room version, but servers do. I guess missing or unsupported There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That isn't a great experience. If I am in a room version X why am I being sent content in a different room version? Maybe we should support some sort of fallback content? Of course now you need to worry about verifying that the fallback content matches the source content. (which is basically impossible if the room version the source is from us unsupported) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only client-facing change in room versions so far is the event ID (which is mentioned in the proposal). Even in the future there most likely won't be any huge client-facing changes, since room versions are for changing the federation protocol. Major client-facing changes like extensible events won't bump the room version. Servers currently support all room versions, deprecating older room versions hasn't been decided yet. Even if there are unsupported versions, it only means the forward metadata won't be verified. |
||
| } | ||
| }, | ||
| "unsigned": { | ||
| "m.forwarded": { | ||
| "valid": true, | ||
| "event_id": "$BfxMy-oNFOeE0eFt6r-l3h7MtwNVIX0GrructyJq1wA" | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
| ### Receiving events with `m.forwarded` | ||
| When a server receives a message event that has the `m.forwarded` key in its | ||
| `content`, the server MUST use the data to validate the signatures, then add a | ||
| `m.forwarded` key to the top-level `unsigned` of the event with the validation | ||
| information. | ||
|
|
||
| #### Validating signatures | ||
| To validate a signature, the server should start with the `m.forwarded` object | ||
| and modify it as follows: | ||
|
|
||
| * If the object is missing any of the required keys, mark it as invalid without | ||
| trying to validate it. | ||
| * Remove the `unsigned` key (if present). | ||
| * Copy `type` from the top level into `m.forwarded`. | ||
| * Make a copy of the top-level `content`, remove `m.forwarded` and put it in | ||
| the `m.forwarded`. | ||
| * Using the result object, validate the signature, calculate the reference hash | ||
| and check the content hash of the event as specified in sections 26.2 through | ||
| 26.4 of the server-server specification: https://matrix.org/docs/spec/server_server/r0.1.4#validating-hashes-and-signatures-on-received-events | ||
|
|
||
| #### Unsigned `m.forwarded` object | ||
| For any message event with `m.forwarded` in the content, the server MUST add or | ||
| override the `m.forwarded` key in the `unsigned` object of the event. The key | ||
| MUST be an object that contains the keys `valid` and `event_id`. | ||
|
|
||
| If the `m.forwarded` object was valid and the signatures were validated, the | ||
| `valid` value should be `true`. In any other case (invalid signature, bogus | ||
| data, etc), the value should be `false`. | ||
|
|
||
| In v1 and v2 rooms, the `event_id` is copied from the `m.forwarded` object in | ||
| `content`. In v3 and up, the `event_id` is based on the reference hash that was | ||
| calculated in the previous section. Copying the event ID in v1/v2 rooms is for | ||
| convenience of clients: they only need to look in one place regardless of the | ||
| room version. | ||
|
|
||
| ## Client behavior | ||
| Clients SHOULD NOT trust forward metadata in the event content without an | ||
| explicit `"valid": true` in the unsigned `m.forwarded` object. Additionally, | ||
| clients SHOULD make sure the server supports this proposal before trusting | ||
| forwards even if the `valid` flag is present. | ||
|
|
||
| Not trusting forward metadata does not necessarily mean it must be completely | ||
| ignored. For example, clients could render the event as a forward, but include | ||
| a notice saying it's unverified. | ||
|
|
||
| ## Potential issues | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A concern I have is that this requires forwarding all of the information in the original event. I can imagine cases where I may want to reference the author, but not expose the room. Other cases could not want to reveal the time of the message. Basically there is a legitimate use case for including any subset of the information from the original event. Furthermore it is likely unclear to the forwarder exactly what information they are sharing. This proposal seems to suggest that it is not allowed to forward modified (including redacted) events. Of course modified events can not be verified but I think it is still nice to have all of this structured data about the event even if it can't be verified to be coming from the original user's homeserver. For example I may just want to forward (message, author, time) and am perfectly fine saying "according to Kevin". So basically how should we handle non-verifiable forwarded events? The same as this protocol but just without There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion copying content from a message to other selectively is a totally different use-case than forwarding a full message. This MSC should be tightly restricted to forwarding full events only, and more importantly, I would explicitly deny forwarding of encrypted events to un-encrypted rooms! When forwarding an encrypted event, you don't only copy the original message, but the actual point-in-time event sent by an original author and including all the meta information in it to another context. While this probably has a bunch of valid use-cases, I consider it a very bad practice to make it easy for 2nd parties of original communication to leak all this to the HSes when the original message content is deliberately shielded from server side spying. When a user copy-pastes a content of a originally encrypted message, the relation to the original encrypted counterpart is not explicitly leaked to the homeservers, and the relation to the original encrypted context is not explicitly crytpographically proven. This is a major difference that should be considered when forwarding the original events to other contexts. Thus, my recommendation is that forwarding of encrypted messages to unencrypted rooms should be blocked by both the server and the client implementations explicitly. Forwarding of encrypted events to other encrypted rooms does not break the separation of trust contexts between the HS and the clients as long as the decryption keys are encrypted with the keys of the target room. Regarding the thoughts on selectively "forwarding" some of the event information the current signing scheme does not support being picky on what to include or exclude if you want to be able to verify the signature. Building something that would allow forwarding selectively, while still providing cryptographic proofs would require a hugely more complex cryptosystem design than Matrix currently has. One possibility to that direction would be using something like Zero Knowledge proofs, but that is totally out of scope in terms of added complexity for this single use case. |
||
| * This is not as simple as MSC2723 and requires server support. | ||
| * Events with bogus data in `m.forwarded` can't be forwarded. | ||
| * Events that are close to the 64 KiB size limit can't be forwarded. MSC2723 | ||
| has the same problem, but this proposal has even more extra data. The amount | ||
| of extra data in both proposals is rather low (<1kb), so this should not be | ||
| a problem in practice. | ||
| * Encrypted events forwarded to other rooms won't be decryptable. Clients | ||
tulir marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| should probably either prevent forwarding completely or only allow forwarding | ||
| to the same room for encrypted events. | ||
|
|
||
| ## Alternatives | ||
| ### Endpoint behavior | ||
| Instead of an endpoint for sending a forward, the new endpoint could be used to | ||
| generate the forward content and leave sending it up to the client with the | ||
| normal /send endpoint. However, this is an extra roundtrip for the client and | ||
| it is not clear if there are any significant benefits in doing so. | ||
|
|
||
| ## Unstable prefix | ||
| While this MSC is not in a released version of the spec, implementations should | ||
| use `net.maunium.msc2730` as a prefix and as a `unstable_features` flag in the | ||
| `/versions` endpoint. | ||
|
|
||
| * `PUT /_matrix/client/unstable/net.maunium.msc2730/rooms/{roomId}/event/{eventId}/forward/{targetRoomId}/{txnId}` as the endpoint | ||
| * `net.maunium.msc2730.forwarded` instead of `m.forwarded` in `content` and `unsigned` | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this more I'm not sure that this represents a valuable use case. I also think the risk of forwarding more information than you wanted to is too high.
Particularly for the best forwarding experience it probably makes sense to trim the message anyways. If we want to support various ways of redacting parts of the forwarded event (which I think we should) we currently don't have a way to verify that and it will be confusing to clients to warn about "according to the sender" for some forwards but having other forwards be verified.
The most compelling use case I can think of are abuse reports but I think in most cases the moderator receiving the report can see the room anyways or otherwise check the legitimacy of the report.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forwarding message means that it isn't changed, if you modify the message text (redact part of it), it is already quoting, not forwarding.