From d95bd3bd0afe4c7531894eb2c9630ab2585ba52d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 17 Aug 2020 15:35:41 +0100 Subject: [PATCH 1/7] Remove : from allowed client_secret chars --- synapse/util/stringutils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/util/stringutils.py b/synapse/util/stringutils.py index 2e2b40a4264b..61d96a6c28c3 100644 --- a/synapse/util/stringutils.py +++ b/synapse/util/stringutils.py @@ -24,9 +24,7 @@ _string_with_symbols = string.digits + string.ascii_letters + ".,;:^&*-_+=#~@" # https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-register-email-requesttoken -# Note: The : character is allowed here for older clients, but will be removed in a -# future release. Context: https://github.com/matrix-org/synapse/issues/6766 -client_secret_regex = re.compile(r"^[0-9a-zA-Z\.\=\_\-\:]+$") +client_secret_regex = re.compile(r"^[0-9a-zA-Z\.\=\_\-]+$") # random_string and random_string_with_symbols are used for a range of things, # some cryptographically important, some less so. We use SystemRandom to make sure From f8e138882f88e2a67648693c09ee42c013cc32ac Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 17 Aug 2020 16:21:12 +0100 Subject: [PATCH 2/7] Changelog --- changelog.d/8101.removal | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8101.removal diff --git a/changelog.d/8101.removal b/changelog.d/8101.removal new file mode 100644 index 000000000000..067e212ed8cd --- /dev/null +++ b/changelog.d/8101.removal @@ -0,0 +1 @@ +Disallow `:` characters from being included in `client_secret` parameters across all related endpoints. This is a breaking change. \ No newline at end of file From da179df51a5b52ce1a02d88ca5d02fe9c26733e2 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 17 Aug 2020 16:35:21 +0100 Subject: [PATCH 3/7] Add warning to CHANGES.md --- CHANGES.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index d4cc179489d5..8e8ffd0e9526 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,18 @@ +For the next release +==================== + +Removal warning +--------------- + +Some older clients added `:` to the `client_secret` parameter of various endpoints, however the +Client-Server API specification +[disallows this](https://matrix.org/docs/spec/client_server/unstable#post-matrix-client-r0-register-email-requesttoken). +Adding `:` in a `client_secret` string has been allowed for some time to allow that client to update, +and we are now removing it, as most users have updated. Further context can be found at https://github.com/matrix-org/synapse/issues/6766. + +We are not currently aware of any other clients that send incorrect `client_secret` values. + + Synapse 1.19.0 (2020-08-17) =========================== From ab9b4b80c461a15f2bd068a3965f3e73c978c19d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 17 Aug 2020 16:55:41 +0100 Subject: [PATCH 4/7] Remove : as an allowed character in stringutils test --- tests/util/test_stringutils.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/util/test_stringutils.py b/tests/util/test_stringutils.py index 4f4da29a98ba..8491f7cc8346 100644 --- a/tests/util/test_stringutils.py +++ b/tests/util/test_stringutils.py @@ -28,9 +28,6 @@ def test_client_secret_regex(self): "_--something==_", "...--==-18913", "8Dj2odd-e9asd.cd==_--ddas-secret-", - # We temporarily allow : characters: https://github.com/matrix-org/synapse/issues/6766 - # To be removed in a future release - "SECRET:1234567890", ] bad = [ From d3134ccf609f54b610be4ce23785667d31cf3647 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Mon, 17 Aug 2020 16:55:55 +0100 Subject: [PATCH 5/7] Update CHANGES.md Co-authored-by: Patrick Cloke --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 8e8ffd0e9526..a615c3d73a43 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,7 +8,7 @@ Some older clients added `:` to the `client_secret` parameter of various endpoin Client-Server API specification [disallows this](https://matrix.org/docs/spec/client_server/unstable#post-matrix-client-r0-register-email-requesttoken). Adding `:` in a `client_secret` string has been allowed for some time to allow that client to update, -and we are now removing it, as most users have updated. Further context can be found at https://github.com/matrix-org/synapse/issues/6766. +and we are now removing it, as most users have updated. Further context can be found at [\#6766](https://github.com/matrix-org/synapse/issues/6766). We are not currently aware of any other clients that send incorrect `client_secret` values. From 27f76ae31fdb2ba7424115734eb47399b01f6038 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 17 Aug 2020 17:22:20 +0100 Subject: [PATCH 6/7] better engerish in CHANGES.md --- CHANGES.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a615c3d73a43..c1b8673c04e2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,13 +4,12 @@ For the next release Removal warning --------------- -Some older clients added `:` to the `client_secret` parameter of various endpoints, however the -Client-Server API specification -[disallows this](https://matrix.org/docs/spec/client_server/unstable#post-matrix-client-r0-register-email-requesttoken). -Adding `:` in a `client_secret` string has been allowed for some time to allow that client to update, -and we are now removing it, as most users have updated. Further context can be found at [\#6766](https://github.com/matrix-org/synapse/issues/6766). - -We are not currently aware of any other clients that send incorrect `client_secret` values. +Some older clients used a +[disallowed character](https://matrix.org/docs/spec/client_server/r0.6.1#post-matrix-client-r0-register-email-requesttoken) +(`:`) in the `client_secret` parameter of various endpoints. The incorrect +behaviour was allowed for backwards compatibility, but is now being removed +from Synapse as most users have updated their client. Further context can be +found at [\#6766](https://github.com/matrix-org/synapse/issues/6766). Synapse 1.19.0 (2020-08-17) From 59347e826ebbc797c731f75153fa718aa5bf2137 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 18 Aug 2020 11:28:28 +0100 Subject: [PATCH 7/7] removal -> bugfix --- changelog.d/8101.bugfix | 1 + changelog.d/8101.removal | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 changelog.d/8101.bugfix delete mode 100644 changelog.d/8101.removal diff --git a/changelog.d/8101.bugfix b/changelog.d/8101.bugfix new file mode 100644 index 000000000000..703bba4234e8 --- /dev/null +++ b/changelog.d/8101.bugfix @@ -0,0 +1 @@ +Synapse now correctly enforces the valid characters in the `client_secret` parameter used in various endpoints. diff --git a/changelog.d/8101.removal b/changelog.d/8101.removal deleted file mode 100644 index 067e212ed8cd..000000000000 --- a/changelog.d/8101.removal +++ /dev/null @@ -1 +0,0 @@ -Disallow `:` characters from being included in `client_secret` parameters across all related endpoints. This is a breaking change. \ No newline at end of file