From 7797c187275c5e340b2d4ef54aa8d4ef331a5012 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 13 May 2020 11:22:51 -0400 Subject: [PATCH 01/10] Add MSC2540 for stricter validation of event JSON. --- proposals/2540-stricter-event-validation.md | 55 +++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 proposals/2540-stricter-event-validation.md diff --git a/proposals/2540-stricter-event-validation.md b/proposals/2540-stricter-event-validation.md new file mode 100644 index 00000000000..9763b4f41c5 --- /dev/null +++ b/proposals/2540-stricter-event-validation.md @@ -0,0 +1,55 @@ +# MSC2540: Stricter event validation: JSON compliance + +## Background + +There has been [prior discussions](https://github.com/matrix-org/matrix-doc/issues/1646) +about validating events more strictly. This MSC proposes fixing a small piece of +this: JSON compliance. + +The [Canonical JSON](https://matrix.org/docs/spec/appendices#canonical-json) +specification requires that numbers that are serialized in JSON are integers in +the range of [-2 ^ 53 + 1, 2 ^ 53 - 1], which matches the requirements of +[section 6 of RFC 7159](https://tools.ietf.org/html/rfc7159). Note that it is +not explicit, but all floats are invalid. + +It is worth mentioning that there are common extensions to JSON which produce +invalid JSON according to the Matrix specification, some programming langauges +even support these by default. One common additional feature is handling +"special" float values: `Infinity`, `-Infinity`, and `NaN`. + + +## Proposal + +In a future room version, Matrix server implementations should strictly enforce +the Canonical JSON specification for events. + +The rationale for doing this in a future room version is to avoid a split brain +room -- where some federated servers believe an event is valid and others reject +it as invalid. Rooms will be able to opt into this behavior as part of a room +version upgrade. + + +## Potential issues + +N/A + + +## Alternatives + +It could be argued that this MSC is unnecessary since it does not add any new +requirements for handling of JSON data. Unfortunately starting to enforce these +requirements in current rooms could cause federation to break as homeservers +will disagree on whether events are valid. + + +## Security considerations + +N/A + + +## Unstable prefix + +A room versions of `org.matrix.strict_canonicaljson` until a future room version +is available. This room version will use +[room version 5](https://matrix.org/docs/spec/rooms/v5) as base and include the +above modifications. From 11587a53849b4e898fd13b8051a8417619590879 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 13 May 2020 11:25:31 -0400 Subject: [PATCH 02/10] Small clarification. --- proposals/2540-stricter-event-validation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2540-stricter-event-validation.md b/proposals/2540-stricter-event-validation.md index 9763b4f41c5..81acd970b4d 100644 --- a/proposals/2540-stricter-event-validation.md +++ b/proposals/2540-stricter-event-validation.md @@ -21,7 +21,7 @@ even support these by default. One common additional feature is handling ## Proposal In a future room version, Matrix server implementations should strictly enforce -the Canonical JSON specification for events. +the JSON compliance of the Canonical JSON specification for events. The rationale for doing this in a future room version is to avoid a split brain room -- where some federated servers believe an event is valid and others reject From bbbd9c4c33a5641da7bf247d9f7967278ef0802c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 13 May 2020 12:50:09 -0400 Subject: [PATCH 03/10] Update wording to avoid demand-style language. Co-authored-by: Travis Ralston --- proposals/2540-stricter-event-validation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2540-stricter-event-validation.md b/proposals/2540-stricter-event-validation.md index 81acd970b4d..d5d31b46ac6 100644 --- a/proposals/2540-stricter-event-validation.md +++ b/proposals/2540-stricter-event-validation.md @@ -20,7 +20,7 @@ even support these by default. One common additional feature is handling ## Proposal -In a future room version, Matrix server implementations should strictly enforce +In a future room version, Matrix server implementations are to strictly enforce the JSON compliance of the Canonical JSON specification for events. The rationale for doing this in a future room version is to avoid a split brain From 03588cbf0dfe47b8eb6bcc0dbb53a1c39115044a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 13 May 2020 14:02:50 -0400 Subject: [PATCH 04/10] Be clearer about errors and what to do with current room versions. --- proposals/2540-stricter-event-validation.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/proposals/2540-stricter-event-validation.md b/proposals/2540-stricter-event-validation.md index d5d31b46ac6..12996333d26 100644 --- a/proposals/2540-stricter-event-validation.md +++ b/proposals/2540-stricter-event-validation.md @@ -20,18 +20,26 @@ even support these by default. One common additional feature is handling ## Proposal -In a future room version, Matrix server implementations are to strictly enforce -the JSON compliance of the Canonical JSON specification for events. +In a future room version, homeserver implementations are to strictly enforce +the JSON compliance of the Canonical JSON specification for events. Events that +do not abide by these rules should be rejected with the error code `M_NOT_JSON`. The rationale for doing this in a future room version is to avoid a split brain room -- where some federated servers believe an event is valid and others reject it as invalid. Rooms will be able to opt into this behavior as part of a room version upgrade. +Homeserver implementations are not to strictly enforce this JSON compliance in +[stable room versions](https://matrix.org/docs/spec/#complete-list-of-room-versions). +The rationale is essentially the same as why a future room version is necessary, +this ensures that all federated servers treat the same events as valid. + ## Potential issues -N/A +Homeserver implementations might include JSON parsers which are stricter than +others. It may not be worthwhile or reasonable to loosen those restrictions for +stable room versions. ## Alternatives From 6757c60d35cba656972c8e979f83ba3514b17350 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 14 May 2020 07:22:48 -0400 Subject: [PATCH 05/10] Specify the current stable room versions. Co-authored-by: Travis Ralston --- proposals/2540-stricter-event-validation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2540-stricter-event-validation.md b/proposals/2540-stricter-event-validation.md index 12996333d26..64a64ddea60 100644 --- a/proposals/2540-stricter-event-validation.md +++ b/proposals/2540-stricter-event-validation.md @@ -30,7 +30,7 @@ it as invalid. Rooms will be able to opt into this behavior as part of a room version upgrade. Homeserver implementations are not to strictly enforce this JSON compliance in -[stable room versions](https://matrix.org/docs/spec/#complete-list-of-room-versions). +[room versions 1, 2, 3, 4, and 5](https://matrix.org/docs/spec/#complete-list-of-room-versions). The rationale is essentially the same as why a future room version is necessary, this ensures that all federated servers treat the same events as valid. From 0b6301ac80b95473ddaad44b644bbf7d5867ae92 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 15 May 2020 10:51:41 -0400 Subject: [PATCH 06/10] Fix grammar and typos from review. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- proposals/2540-stricter-event-validation.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proposals/2540-stricter-event-validation.md b/proposals/2540-stricter-event-validation.md index 64a64ddea60..94ec481e4bc 100644 --- a/proposals/2540-stricter-event-validation.md +++ b/proposals/2540-stricter-event-validation.md @@ -13,7 +13,7 @@ the range of [-2 ^ 53 + 1, 2 ^ 53 - 1], which matches the requirements of not explicit, but all floats are invalid. It is worth mentioning that there are common extensions to JSON which produce -invalid JSON according to the Matrix specification, some programming langauges +invalid JSON according to the Matrix specification; some programming langauges even support these by default. One common additional feature is handling "special" float values: `Infinity`, `-Infinity`, and `NaN`. @@ -31,7 +31,7 @@ version upgrade. Homeserver implementations are not to strictly enforce this JSON compliance in [room versions 1, 2, 3, 4, and 5](https://matrix.org/docs/spec/#complete-list-of-room-versions). -The rationale is essentially the same as why a future room version is necessary, +The rationale is essentially the same as why a future room version is necessary: this ensures that all federated servers treat the same events as valid. @@ -57,7 +57,7 @@ N/A ## Unstable prefix -A room versions of `org.matrix.strict_canonicaljson` until a future room version +A room version of `org.matrix.strict_canonicaljson` until a future room version is available. This room version will use [room version 5](https://matrix.org/docs/spec/rooms/v5) as base and include the above modifications. From 007c8b57992137c19e2ded990e141bf0b9157eb1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 15 May 2020 10:52:29 -0400 Subject: [PATCH 07/10] Clarify the range of valid values. --- proposals/2540-stricter-event-validation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2540-stricter-event-validation.md b/proposals/2540-stricter-event-validation.md index 94ec481e4bc..3027be56b3c 100644 --- a/proposals/2540-stricter-event-validation.md +++ b/proposals/2540-stricter-event-validation.md @@ -8,7 +8,7 @@ this: JSON compliance. The [Canonical JSON](https://matrix.org/docs/spec/appendices#canonical-json) specification requires that numbers that are serialized in JSON are integers in -the range of [-2 ^ 53 + 1, 2 ^ 53 - 1], which matches the requirements of +the inclusive range of `[-2 ^ 53 + 1, 2 ^ 53 - 1]`, which matches the requirements of [section 6 of RFC 7159](https://tools.ietf.org/html/rfc7159). Note that it is not explicit, but all floats are invalid. From e5fa76c8edc526bad233000019e9da8da17ce232 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 15 May 2020 10:53:26 -0400 Subject: [PATCH 08/10] Clarify range a bit more. --- proposals/2540-stricter-event-validation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2540-stricter-event-validation.md b/proposals/2540-stricter-event-validation.md index 3027be56b3c..3d58411d02e 100644 --- a/proposals/2540-stricter-event-validation.md +++ b/proposals/2540-stricter-event-validation.md @@ -8,7 +8,7 @@ this: JSON compliance. The [Canonical JSON](https://matrix.org/docs/spec/appendices#canonical-json) specification requires that numbers that are serialized in JSON are integers in -the inclusive range of `[-2 ^ 53 + 1, 2 ^ 53 - 1]`, which matches the requirements of +the inclusive range of `[-(2^53) + 1, (2^53) - 1]`, which matches the requirements of [section 6 of RFC 7159](https://tools.ietf.org/html/rfc7159). Note that it is not explicit, but all floats are invalid. From f5ebe33a9ce858ad3bde26a35bd15437e4eb3ee5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 19 May 2020 14:35:11 -0400 Subject: [PATCH 09/10] Reword how bad values are handled. --- proposals/2540-stricter-event-validation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2540-stricter-event-validation.md b/proposals/2540-stricter-event-validation.md index 3d58411d02e..ac28efc4ec5 100644 --- a/proposals/2540-stricter-event-validation.md +++ b/proposals/2540-stricter-event-validation.md @@ -22,7 +22,7 @@ even support these by default. One common additional feature is handling In a future room version, homeserver implementations are to strictly enforce the JSON compliance of the Canonical JSON specification for events. Events that -do not abide by these rules should be rejected with the error code `M_NOT_JSON`. +do not abide by these rules should be treated like any other bad JSON value. The rationale for doing this in a future room version is to avoid a split brain room -- where some federated servers believe an event is valid and others reject From 07716711f14378a08f0d45064c49ecfde8c9e66b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 20 May 2020 08:58:32 -0400 Subject: [PATCH 10/10] Give more guidance on how invalid events should be handled. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- proposals/2540-stricter-event-validation.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/proposals/2540-stricter-event-validation.md b/proposals/2540-stricter-event-validation.md index ac28efc4ec5..31c4c39b90b 100644 --- a/proposals/2540-stricter-event-validation.md +++ b/proposals/2540-stricter-event-validation.md @@ -21,8 +21,10 @@ even support these by default. One common additional feature is handling ## Proposal In a future room version, homeserver implementations are to strictly enforce -the JSON compliance of the Canonical JSON specification for events. Events that -do not abide by these rules should be treated like any other bad JSON value. +the JSON compliance of the Canonical JSON specification for events. +Non-compliant events should be treated like any other malformed event, +for example by rejecting the request with an HTTP 400 error with `M_BAD_JSON`, +or by discarding the event. The rationale for doing this in a future room version is to avoid a split brain room -- where some federated servers believe an event is valid and others reject