Allow adding random delay to push (#15516)

This is to discourage timing based profiling on the push gateways.
This commit is contained in:
Erik Johnston 2023-05-02 17:45:44 +01:00 committed by GitHub
parent 6aca4e7cb8
commit 4de271a7fc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 68 additions and 2 deletions

View File

@ -0,0 +1 @@
Add a config option to delay push notifications by a random amount, to discourage time-based profiling.

View File

@ -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 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 rooms with unread messages in them. Set to false to instead send the number
of unread messages. 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: Example configuration:
```yaml ```yaml
@ -3449,6 +3452,7 @@ push:
enabled: true enabled: true
include_content: false include_content: false
group_unread_count_by_room: false group_unread_count_by_room: false
jitter_delay: "10s"
``` ```
--- ---
## Rooms ## Rooms

View File

@ -42,11 +42,17 @@ class PushConfig(Config):
# Now check for the one in the 'email' section and honour it, # Now check for the one in the 'email' section and honour it,
# with a warning. # with a warning.
push_config = config.get("email") or {} email_push_config = config.get("email") or {}
redact_content = push_config.get("redact_content") redact_content = email_push_config.get("redact_content")
if redact_content is not None: if redact_content is not None:
print( print(
"The 'email.redact_content' option is deprecated: " "The 'email.redact_content' option is deprecated: "
"please set push.include_content instead" "please set push.include_content instead"
) )
self.push_include_content = not redact_content 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)

View File

@ -13,6 +13,7 @@
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
import logging import logging
import random
import urllib.parse import urllib.parse
from typing import TYPE_CHECKING, Dict, List, Optional, Union from typing import TYPE_CHECKING, Dict, List, Optional, Union
@ -114,6 +115,8 @@ class HttpPusher(Pusher):
) )
self._pusherpool = hs.get_pusherpool() self._pusherpool = hs.get_pusherpool()
self.push_jitter_delay_ms = hs.config.push.push_jitter_delay_ms
self.data = pusher_config.data self.data = pusher_config.data
if self.data is None: if self.data is None:
raise PusherConfigException("'data' key can not be null for HTTP pusher") raise PusherConfigException("'data' key can not be null for HTTP pusher")
@ -327,6 +330,21 @@ class HttpPusher(Pusher):
event = await self.store.get_event(push_action.event_id, allow_none=True) event = await self.store.get_event(push_action.event_id, allow_none=True)
if event is None: if event is None:
return True # It's been redacted 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) rejected = await self.dispatch_push_event(event, tweaks, badge)
if rejected is False: if rejected is False:
return False return False

View File

@ -962,3 +962,40 @@ class HTTPPusherTests(HomeserverTestCase):
channel.json_body["pushers"][0]["org.matrix.msc3881.device_id"], channel.json_body["pushers"][0]["org.matrix.msc3881.device_id"],
lookup_result.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)