From f4b19a0680eb2658f2e133123aedc216fcda5f7e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 May 2023 09:27:38 +0100 Subject: [PATCH 1/3] Allow adding random delay to push This is to discourage timing based profiling on the push gateways. --- .../configuration/config_documentation.md | 4 ++ synapse/config/push.py | 10 ++++- synapse/push/httppusher.py | 18 +++++++++ tests/push/test_http.py | 37 +++++++++++++++++++ 4 files changed, 67 insertions(+), 2 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 1b6f2569490e..acf9e0f3353f 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3442,6 +3442,9 @@ This option has a number of sub-options. They are as follows: user has unread messages in. Defaults to true, meaning push clients will see the number of rooms with unread messages in them. Set to false to instead send the number of unread messages. +* `jitter_delay`: Delays push notifications by a random amount up to the given + duration. Useful for mitigating timing attacks. Optional, defaults to no + delay. Example configuration: ```yaml @@ -3449,6 +3452,7 @@ push: enabled: true include_content: false group_unread_count_by_room: false + jitter_delay: "10s" ``` --- ## Rooms diff --git a/synapse/config/push.py b/synapse/config/push.py index 3b5378e6ea52..8177ff52e24a 100644 --- a/synapse/config/push.py +++ b/synapse/config/push.py @@ -42,11 +42,17 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # Now check for the one in the 'email' section and honour it, # with a warning. - push_config = config.get("email") or {} - redact_content = push_config.get("redact_content") + email_push_config = config.get("email") or {} + redact_content = email_push_config.get("redact_content") if redact_content is not None: print( "The 'email.redact_content' option is deprecated: " "please set push.include_content instead" ) self.push_include_content = not redact_content + + # Whether to apply a random delay to outbound push. + self.push_jitter_delay_ms = None + push_jitter_delay = push_config.get("jitter_delay", None) + if push_jitter_delay: + self.push_jitter_delay_ms = self.parse_duration(push_jitter_delay) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 4f8fa445d951..e91ee05e9960 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +import random import urllib.parse from typing import TYPE_CHECKING, Dict, List, Optional, Union @@ -114,6 +115,8 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig): ) self._pusherpool = hs.get_pusherpool() + self.push_jitter_delay_ms = hs.config.push.push_jitter_delay_ms + self.data = pusher_config.data if self.data is None: raise PusherConfigException("'data' key can not be null for HTTP pusher") @@ -326,6 +329,21 @@ async def _process_one(self, push_action: HttpPushAction) -> bool: event = await self.store.get_event(push_action.event_id, allow_none=True) if event is None: return True # It's been redacted + + # Check if we should delay sending out the notification by a random + # amount. + # + # Note: we base the delay off of when the event was sent, rather than + # now, to handle the case where we need to send out many notifications + # at once. If we just slept the random amount each loop then the last + # push notification in the set could be delayed by many times the max + # delay. + if self.push_jitter_delay_ms: + delay_ms = random.randint(1, self.push_jitter_delay_ms) + diff_ms = event.origin_server_ts + delay_ms - self.clock.time_msec() + if diff_ms > 0: + await self.clock.sleep(diff_ms / 1000) + rejected = await self.dispatch_push_event(event, tweaks, badge) if rejected is False: return False diff --git a/tests/push/test_http.py b/tests/push/test_http.py index 99cec0836b1d..54f558742dfe 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -962,3 +962,40 @@ def test_device_id(self) -> None: channel.json_body["pushers"][0]["org.matrix.msc3881.device_id"], lookup_result.device_id, ) + + @override_config({"push": {"jitter_delay": "10s"}}) + def test_jitter(self) -> None: + """Tests that enabling jitter actually delays sending push.""" + user_id, access_token = self._make_user_with_pusher("user") + other_user_id, other_access_token = self._make_user_with_pusher("otheruser") + + room = self.helper.create_room_as(user_id, tok=access_token) + self.helper.join(room=room, user=other_user_id, tok=other_access_token) + + # Send a message and check that it did not generate a push, as it should + # be delayed. + self.helper.send(room, body="Hi!", tok=other_access_token) + self.assertEqual(len(self.push_attempts), 0) + + # Now advance time past the max jitter, and assert the message was sent. + self.reactor.advance(15) + self.assertEqual(len(self.push_attempts), 1) + + self.push_attempts[0][0].callback({}) + + # Now we send a bunch of messages and assert that they were all sent + # within the 10s max delay. + for _ in range(10): + self.helper.send(room, body="Hi!", tok=other_access_token) + + index = 1 + for _ in range(11): + while len(self.push_attempts) > index: + self.push_attempts[index][0].callback({}) + self.pump() + index += 1 + + self.reactor.advance(1) + self.pump() + + self.assertEqual(len(self.push_attempts), 11) From 96089c06a72e2a2c0f6e045bf9d905a01948e77b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 May 2023 11:31:43 +0100 Subject: [PATCH 2/3] Newsfile --- changelog.d/15516.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15516.feature diff --git a/changelog.d/15516.feature b/changelog.d/15516.feature new file mode 100644 index 000000000000..02a101bb88c7 --- /dev/null +++ b/changelog.d/15516.feature @@ -0,0 +1 @@ +Add a config option to delay push notifications by a random amount, to discourage time-based profiling. From 4a231264715e019d2bd6722131551bf79c51ba99 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 May 2023 17:04:20 +0100 Subject: [PATCH 3/3] Add note on which version the option was added in --- docs/usage/configuration/config_documentation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index acf9e0f3353f..b6516191e813 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3444,7 +3444,7 @@ This option has a number of sub-options. They are as follows: of unread messages. * `jitter_delay`: Delays push notifications by a random amount up to the given duration. Useful for mitigating timing attacks. Optional, defaults to no - delay. + delay. _Added in Synapse 1.84.0._ Example configuration: ```yaml