Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Allow adding random delay to push (#15516)
Browse files Browse the repository at this point in the history
This is to discourage timing based profiling on the push gateways.
  • Loading branch information
erikjohnston committed May 2, 2023
1 parent 6aca4e7 commit 4de271a
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 2 deletions.
1 change: 1 addition & 0 deletions changelog.d/15516.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add a config option to delay push notifications by a random amount, to discourage time-based profiling.
4 changes: 4 additions & 0 deletions docs/usage/configuration/config_documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -3442,13 +3442,17 @@ 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. _Added in Synapse 1.84.0._

Example configuration:
```yaml
push:
enabled: true
include_content: false
group_unread_count_by_room: false
jitter_delay: "10s"
```
---
## Rooms
Expand Down
10 changes: 8 additions & 2 deletions synapse/config/push.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
18 changes: 18 additions & 0 deletions synapse/push/httppusher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -327,6 +330,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
Expand Down
37 changes: 37 additions & 0 deletions tests/push/test_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

0 comments on commit 4de271a

Please sign in to comment.