MSC4277: Harmonizing the reporting endpoints #4277
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| # MSC4277: Harmonizing the reporting endpoints | ||
|
|
||
| Matrix currently has three reporting endpoints: | ||
|
|
||
| - [`/_matrix/client/v3/rooms/{roomId}/report/{eventId}`] ([added] in Matrix | ||
Johennes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 0.4.0, no MSC known) | ||
| - [`/_matrix/client/v3/rooms/{roomId}/report`] ([added][1] in Matrix 1.13 as per | ||
| [MSC4151]) | ||
| - `/_matrix/client/v3/users/{userId}/report` (to be [added][2] to Matrix as per | ||
| [MSC4260]) | ||
|
|
||
| The spec contains a number of subtle differences for these endpoints: | ||
|
|
||
| 1. The `reason` parameter on the event reporting endpoint was made optional by | ||
| [MSC2414]. The other two reporting endpoints didn't pick up on this, | ||
| however, and still require `reason` to be present while allowing it to be | ||
| blank. | ||
| 2. The user reporting endpoint [allows] servers to respond with 200 even if the | ||
| user doesn't exist to deny enumerating users. This option is not allowed in | ||
| the event and room reporting endpoints. | ||
| 3. The user and event reporting endpoints allow servers to add a random delay | ||
| when generating responses. This is a more sophisticated measure against | ||
| enumeration attacks. While the spec doesn't explicit forbid this technique | ||
| on the room reporting endpoint it doesn't explicitly mention or recommend | ||
| it either. | ||
|
|
||
| These differences seem unnecessary and were likely introduced by accident only. | ||
| The present proposal, therefore, seeks to align the three endpoints. | ||
|
|
||
| ## Proposal | ||
|
|
||
| On all three endpoints: | ||
|
|
||
| 1. The `reason` parameter is made optional. | ||
Johennes marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| 2. Servers MAY respond with 200 and no content regardless of whether the | ||
| reported subject exists or not to combat enumeration attacks. | ||
|
|
||
| 3. Servers MAY add a random delay or use constant time functions when | ||
| processing responses to combat enumeration attacks. | ||
|
|
||
| All of these changes appear applicable regardless of the reported subject and it | ||
| is not clear why the spec should differentiate the endpoints here. | ||
|
|
||
| ## Potential issues | ||
|
|
||
| The `reason` field on the room and user reporting endpoints is currently | ||
|
||
| required. Making it optional, therefore, is a breaking change. Clients should | ||
| either act on the servers supported version or blanketly include an empty string | ||
| if the user doesn't provide a reason. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| None. | ||
|
|
||
| ## Security considerations | ||
|
|
||
| Enumeration attacks are likely more severe for users than for rooms or events. | ||
| There are still not irrelevant, however. | ||
|
|
||
| ## Unstable prefix | ||
|
|
||
| None. | ||
|
|
||
| ## Dependencies | ||
|
|
||
| None. | ||
|
|
||
| [`/_matrix/client/v3/rooms/{roomId}/report/{eventId}`]: https://spec.matrix.org/v1.13/client-server-api/#post_matrixclientv3roomsroomidreporteventid | ||
| [added]: https://github.com/matrix-org/matrix-spec-proposals/pull/1264 | ||
| [`/_matrix/client/v3/rooms/{roomId}/report`]: https://spec.matrix.org/v1.13/client-server-api/#post_matrixclientv3roomsroomidreport | ||
| [1]: https://github.com/matrix-org/matrix-spec/pull/1938 | ||
| [MSC4151]: https://github.com/matrix-org/matrix-spec-proposals/pull/4151 | ||
| [2]: https://github.com/matrix-org/matrix-spec/pull/2093 | ||
| [MSC4260]: https://github.com/matrix-org/matrix-spec-proposals/pull/4260 | ||
| [MSC2414]: https://github.com/matrix-org/matrix-spec-proposals/pull/2414 | ||
| [allows]: https://github.com/matrix-org/matrix-spec-proposals/pull/4260/files#diff-cbb17920e2617e7a20ab0838879675f7aa70e828f0263a3cfa5f4c53913ce5f7R34-R35 | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Implementation requirements:
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.
200 regardless of event / room existence: element-hq/synapse#18263
I'm unsure if the random delay / constant time part requires an implementation. The spec already mentions this for the event reporting endpoint but Synapse doesn't appear to implement it and it's not part of element-hq/synapse#18120 either.
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.
I've added the removal of
scoreinto element-hq/synapse#18263. Synapse didn't do anything else than assert the type anyway.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.
Implemented in continuwuity #919.