dandri/synapse
1
0
Fork 0
mirror of https://github.com/element-hq/synapse.git synced 2026-07-29 01:10:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Limit the size of push rules

Browse Source 
Fixes: https://github.com/element-hq/synapse/security/advisories/GHSA-fp53-rw9v-hcf9
Fixes: https://github.com/matrix-org/internal-config/issues/1071

Because this is an out-of-spec limit and could break someone's workflow on release day, I've opted to make it configurable.

-----

Reviewed-on: https://github.com/element-hq/synapse-private/pull/149
This commit is contained in:
Olivier 'reivilibre
2026-07-28 13:58:06 +01:00
committed by Olivier 'reivilibre
parent 4f8037ab59
commit 44216bf2b6
8 changed files with 437 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/usage/configuration/config_documentation.md
+20 -1
View File
@@ -3808,7 +3808,7 @@ This setting has the following sub-options:
Defaults to `null`.
* `update_profile_information` (boolean): Use this setting to keep a user's profile fields in sync with information from the identity provider. Fields are checked on every SSO login, and are updated if necessary. Note that enabling this option will override user profile information, regardless of whether users have opted-out of syncing that information when first signing in. Fields that will be synced:
* `update_profile_information` (boolean): Use this setting to keep a user's profile fields in sync with information from the identity provider. Fields are checked on every SSO login, and are updated if necessary. Note that enabling this option will override user profile information, regardless of whether users have opted-out of syncing that information when first signing in. Fields that will be synced:
* displayname
* picture - only if Synapse media repository is running in the main
process (i.e. not workerized) and media is stored locally Defaults to `false`.
@@ -3938,6 +3938,25 @@ push:
jitter_delay: 10s
```
---
### `push_rules`
*(object)* Options for push rules
This setting has the following sub-options:
* `limits` (object): Limits on the size of push rules that users can have
This setting has the following sub-options:
* `rule_count` (integer): This is the total number of push rules that each user can have. Power users may expect to have one push rule per room. Defaults to `10000`.
* `rule_id_length` (integer): This is the maximum length of a push rule ID, in bytes. Push rule IDs need to be allowed to be at least as long as a room ID (which are [limited to 255 bytes per specification](https://spec.matrix.org/v1.19/appendices/#room-ids))
It's recommended to leave this option as it is. We expect to remove this option if/when the specification standardises on a limit. Defaults to `300`.
* `rule_size` (integer): This is the maximum size of a push rule's body, in bytes.
The exact mechanism for calculating this size is currently an implementation detail, subject to change. This limit should be treated as a coarse sanity limit rather than something to fine-tune.
It's recommended to leave this option as it is. We expect to remove this option if/when the specification standardises on a limit and a mechanism for calculating it. Defaults to `1024`.
---
## Rooms
Config options relating to rooms.
schema/synapse-config.schema.yaml
+48 -10
View File
@@ -2408,7 +2408,7 @@ properties:
Enable the local on-disk media storage provider. When disabled, media is
stored only in configured `media_storage_providers` and temporary files are
used for processing.
**Warning:** If this option is set to `false` and no `media_storage_providers`
are configured, all media requests will return 404 errors as there will be
no storage backend available.
@@ -2962,7 +2962,7 @@ properties:
examples:
- false
matrix_rtc:
type: object
type: object
description: >-
Options related to MatrixRTC.
properties:
@@ -2982,14 +2982,13 @@ properties:
description: >-
The base URL of the LiveKit service. Should only be used with LiveKit-based transports.
example: https://matrix-rtc.example.com/livekit/jwt
description:
A list of transport types and arguments to use for MatrixRTC connections.
description: A list of transport types and arguments to use for MatrixRTC connections.
default: []
default: {}
examples:
- transports:
- type: livekit
livekit_service_url: https://matrix-rtc.example.com/livekit/jwt
- type: livekit
livekit_service_url: https://matrix-rtc.example.com/livekit/jwt
enable_registration:
type: boolean
description: >-
@@ -4664,10 +4663,10 @@ properties:
type: boolean
description: >-
Use this setting to keep a user's profile fields in sync with
information from the identity provider. Fields are checked on every
SSO login, and are updated if necessary. Note that enabling this
option will override user profile information, regardless of whether
users have opted-out of syncing that information when first signing
information from the identity provider. Fields are checked on every
SSO login, and are updated if necessary. Note that enabling this
option will override user profile information, regardless of whether
users have opted-out of syncing that information when first signing
in.
Fields that will be synced:
* displayname
@@ -4879,6 +4878,45 @@ properties:
include_content: false
group_unread_count_by_room: false
jitter_delay: 10s
push_rules:
type: object
description: Options for push rules
properties:
limits:
type: object
description: Limits on the size of push rules that users can have
properties:
rule_count:
type: integer
default: 10000
description: >-
This is the total number of push rules that each user can have.
Power users may expect to have one push rule per room.
rule_id_length:
type: integer
default: 300
description: >-
This is the maximum length of a push rule ID, in bytes.
Push rule IDs need to be allowed to be at least as long
as a room ID (which are [limited to 255 bytes per specification](https://spec.matrix.org/v1.19/appendices/#room-ids))
It's recommended to leave this option as it is.
We expect to remove this option if/when the specification standardises on
a limit.
rule_size:
type: integer
default: 1024
description: >-
This is the maximum size of a push rule's body, in bytes.
The exact mechanism for calculating this size is currently an implementation
detail, subject to change.
This limit should be treated as a coarse sanity limit rather than something
to fine-tune.
It's recommended to leave this option as it is.
We expect to remove this option if/when the specification standardises on
a limit and a mechanism for calculating it.
encryption_enabled_by_default_for_room_type:
type: string
description: >-
synapse/config/_base.pyi
+2
View File
@@ -38,6 +38,7 @@ from synapse.config import ( # noqa: F401
oidc,
password_auth_providers,
push,
push_rules,
ratelimiting,
redis,
registration,
@@ -103,6 +104,7 @@ class RootConfig:
worker: workers.WorkerConfig
authproviders: password_auth_providers.PasswordAuthProviderConfig
push: push.PushConfig
push_rules: push_rules.PushRulesConfig
spamchecker: spam_checker.SpamCheckerConfig
room: room.RoomConfig
userdirectory: user_directory.UserDirectoryConfig
synapse/config/_util.py
+12 -3
View File
@@ -18,12 +18,13 @@
# [This file includes modifications made by New Vector Limited]
#
#
from typing import Any, TypeVar
from typing import Annotated, Any, TypeVar
import jsonschema
from pydantic import BaseModel, TypeAdapter, ValidationError
from pydantic import BaseModel, BeforeValidator, StrictInt, TypeAdapter, ValidationError
from pydantic_core.core_schema import int_schema
from synapse.config._base import ConfigError
from synapse.config._base import Config, ConfigError
from synapse.types import JsonDict, StrSequence
@@ -97,3 +98,11 @@ def parse_and_validate_mapping(
except ValidationError as e:
raise ConfigError(str(e)) from e
return instances
ConfigByteSize = Annotated[
StrictInt, BeforeValidator(Config.parse_size), int_schema(ge=0)
]
"""
A size in bytes. Pydantic-compatible wrapper for `Config.parse_size`
"""
synapse/config/homeserver.py
+3
View File
@@ -19,6 +19,8 @@
#
#
from synapse.config.push_rules import PushRulesConfig
from ._base import ConfigError, RootConfig
from .account_validity import AccountValidityConfig
from .api import ApiConfig
@@ -102,6 +104,7 @@ class HomeServerConfig(RootConfig):
EmailConfig,
PasswordAuthProviderConfig,
PushConfig,
PushRulesConfig,
SpamCheckerConfig,
RoomConfig,
UserDirectoryConfig,
synapse/config/push_rules.py
+63
View File
@@ -0,0 +1,63 @@
#
# This file is licensed under the Affero General Public License (AGPL) version 3.
#
# Copyright (C) 2026 Element Creations Ltd
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as
# published by the Free Software Foundation, either version 3 of the
# License, or (at your option) any later version.
#
# See the GNU Affero General Public License for more details:
# <https://www.gnu.org/licenses/agpl-3.0.html>.
from typing import Annotated, Any
from pydantic import (
Field,
StrictInt,
ValidationError,
)
from synapse.config._util import ConfigByteSize
from synapse.types import JsonDict
from synapse.util.pydantic_models import ParseModel
from ._base import Config, ConfigError
class PushRulesLimitsConfig(ParseModel):
# Chosen arbitrarily, but with the rough rationale that a user
# might have on the order of 10k rooms and want to set a push rule override for each one.
rule_count: Annotated[StrictInt, Field(ge=0)] = 10_000
# Chosen arbitrarily, but with the rationale that room IDs are allowed to be up to 255 bytes
# and they are often used in rule IDs.
rule_id_length: Annotated[StrictInt, Field(ge=1)] = 300
# Chosen arbitrarily, but with the rationale that real-world push rules don't get
# nearly this big in practice.
# Even 512 bytes would probably have been fine, but we should leave space for the use cases
# of push rules to grow in the future.
rule_size: Annotated[ConfigByteSize, Field(ge=1)] = 1024
class PushRulesConfigModel(ParseModel):
limits: PushRulesLimitsConfig = Field(default_factory=PushRulesLimitsConfig)
class PushRulesConfig(Config):
section = "push_rules"
def read_config(self, config: JsonDict, **kwargs: Any) -> None:
raw_config = config.get("push_rules", {})
try:
parsed = PushRulesConfigModel(**raw_config)
except ValidationError as e:
raise ConfigError(
f"Could not validate configuration: {e}",
path=("push_rules",),
) from e
self.limits = parsed.limits
synapse/storage/databases/main/push_rule.py
+96 -5
View File
@@ -19,6 +19,7 @@
#
#
import logging
from http import HTTPStatus
from typing import (
TYPE_CHECKING,
Any,
@@ -31,7 +32,7 @@ from typing import (
from twisted.internet import defer
from synapse.api.errors import StoreError
from synapse.api.errors import Codes, StoreError, SynapseError
from synapse.config.homeserver import ExperimentalConfig
from synapse.logging.context import make_deferred_yieldable, run_in_background
from synapse.replication.tcp.streams import PushRulesStream
@@ -112,6 +113,22 @@ def _load_rules(
return filtered_rules
def _push_rule_size_for_limits(*, conditions_json: str, actions_json: str) -> int:
"""
Returns the size of a push rule, as used for applying the size limit.
We aren't tied to any particular definition, but currently this is
simply the size in bytes of the conditions and actions JSON added together,
so not rocket science, but this function provides a 'label' for it.
"""
# This is not a very predictable way of calculating the size from the
# point of view of the client, but since it's an out-of-spec limit
# entirely at our discretion, we don't really have to worry about
# the exact calculation.
# FIXME: Spec a predictable push rule size limit
return len(conditions_json.encode("utf-8")) + len(actions_json.encode("utf-8"))
class PushRulesWorkerStore(
ApplicationServiceWorkerStore,
PusherWorkerStore,
@@ -170,6 +187,8 @@ class PushRulesWorkerStore(
self._push_rule_id_gen = IdGenerator(db_conn, "push_rules", "id")
self._push_rules_enable_id_gen = IdGenerator(db_conn, "push_rules_enable", "id")
self._config = hs.config.push_rules
def get_max_push_rules_stream_id(self) -> int:
"""Get the position of the push rules stream.
@@ -409,6 +428,29 @@ class PushRulesWorkerStore(
conditions_json = json_encoder.encode(conditions)
actions_json = json_encoder.encode(actions)
rule_id_len = len(rule_id.encode("utf-8"))
if rule_id_len > self._config.limits.rule_id_length:
raise SynapseError(
HTTPStatus.REQUEST_ENTITY_TOO_LARGE,
f"Push rule ID length exceeds server limit ({rule_id_len} bytes > {self._config.limits.rule_id_length} bytes).",
# FIXME: Provide a better error code.
# None of the existing options seem entirely correct, though.
Codes.UNKNOWN,
)
rule_body_size = _push_rule_size_for_limits(
conditions_json=conditions_json, actions_json=actions_json
)
if rule_body_size > self._config.limits.rule_size:
raise SynapseError(
HTTPStatus.REQUEST_ENTITY_TOO_LARGE,
f"Push rule size exceeds server limit ({rule_body_size} bytes > {self._config.limits.rule_size} bytes).",
# FIXME: Provide a better error code.
# None of the existing options seem entirely correct, though.
Codes.UNKNOWN,
)
async with self._push_rules_stream_id_gen.get_next() as stream_id:
event_stream_ordering = self._stream_id_gen.get_current_token()
@@ -578,13 +620,19 @@ class PushRulesWorkerStore(
actions_json: str,
update_stream: bool = True,
) -> None:
"""Specialised version of simple_upsert_txn that picks a push_rule_id
using the _push_rule_id_gen if it needs to insert the rule.
Preconditions:
- this worker is a push writer
- the "push_rules" table is locked
- the push rule has already been validated,
including for rule ID length and rule body size.
"""
if not self._is_push_writer:
raise Exception("Not a push writer")
"""Specialised version of simple_upsert_txn that picks a push_rule_id
using the _push_rule_id_gen if it needs to insert the rule. It assumes
that the "push_rules" table is locked"""
sql = (
"UPDATE push_rules"
" SET priority_class = ?, priority = ?, conditions = ?, actions = ?"
@@ -597,6 +645,27 @@ class PushRulesWorkerStore(
)
if txn.rowcount == 0:
# About to add a new rule, so check our limits first.
txn.execute(
"""
SELECT COUNT(*) FROM push_rules
WHERE user_name = ?
""",
(user_id,),
)
(num_push_rules,) = cast(tuple[int], txn.fetchone())
if num_push_rules >= self._config.limits.rule_count:
raise SynapseError(
HTTPStatus.BAD_REQUEST,
f"Creating a push rule would exceed the limit on the number of push rules associated with your account ({num_push_rules + 1} rules > {self._config.limits.rule_count} rules)",
# FIXME: Provide a better error code, especially for this case.
# None of the existing options seem entirely correct, though.
# `M_USER_LIMIT_EXCEEDED` comes closest but needs an `info_uri`.
# Should follow up and add a built-in error page template, similarly to
# https://github.com/element-hq/synapse/pull/18876 ?
Codes.UNKNOWN,
)
# We didn't update a row with the given rule_id so insert one
push_rule_id = self._push_rule_id_gen.get_next()
@@ -839,6 +908,28 @@ class PushRulesWorkerStore(
)
else:
try:
# Before updating the push rule, we need to check that we won't exceed
# the size limit on push rules.
# For that, we need to fetch the `conditions` JSON.
conditions_json = self.db_pool.simple_select_one_onecol_txn(
txn,
"push_rules",
{"user_name": user_id, "rule_id": rule_id},
"conditions",
)
rule_body_size = _push_rule_size_for_limits(
conditions_json=conditions_json, actions_json=actions_json
)
if rule_body_size > self._config.limits.rule_size:
raise SynapseError(
HTTPStatus.REQUEST_ENTITY_TOO_LARGE,
f"Push rule size exceeds server limit ({rule_body_size} bytes > {self._config.limits.rule_size} bytes).",
# FIXME: Provide a better error code.
# None of the existing options seem entirely correct, though.
Codes.UNKNOWN,
)
self.db_pool.simple_update_one_txn(
txn,
"push_rules",
tests/rest/client/test_push_rule_attrs.py
+193
View File
@@ -3,6 +3,7 @@
#
# Copyright 2020 The Matrix.org Foundation C.I.C.
# Copyright (C) 2023 New Vector, Ltd
# Copyright (C) 2026 Element Creations Ltd
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as
@@ -20,9 +21,13 @@
#
from http import HTTPStatus
import canonicaljson
from parameterized.parameterized import parameterized
import synapse
from synapse.api.errors import Codes
from synapse.rest.client import login, push_rule, room
from synapse.types import JsonDict
from tests.unittest import HomeserverTestCase
@@ -508,3 +513,191 @@ class PushRuleAttributesTestCase(HomeserverTestCase):
Codes.INVALID_PARAM,
channel.json_body["errcode"],
)
class PushRuleLimitTestCase(HomeserverTestCase):
"""
Tests for server-configured limits on push rule size.
See: https://github.com/element-hq/synapse/security/advisories/GHSA-fp53-rw9v-hcf9
"""
servlets = [
synapse.rest.admin.register_servlets_for_client_rest_resource,
room.register_servlets,
login.register_servlets,
push_rule.register_servlets,
]
hijack_auth = False
def default_config(self) -> JsonDict:
config = super().default_config()
# Set some small limits for push rule sizes so that
# we can easily test them.
config["push_rules"] = {
"limits": {
# Size limit of each rule in bytes (canonical JSON)
"rule_size": 64,
# Size limit of the rule ID (in bytes)
"rule_id_length": 24,
# Limit on how many rules you can have
"rule_count": 2,
}
}
return config
@parameterized.expand(
(
(
{
"actions": ["notify"],
"conditions": [{"kind": "event_match", "key": "a", "pattern": "a"}],
},
58,
True,
),
(
{
"actions": ["notify"],
"conditions": [
{"kind": "event_match", "key": "a", "pattern": "abcdefg"}
],
},
64,
True,
),
(
{
"actions": ["notify"],
"conditions": [
{"kind": "event_match", "key": "a", "pattern": "abcdefgH"}
],
},
65,
False,
),
)
)
def test_limit_on_push_rule_size(
self, body: JsonDict, expected_body_num_bytes: int, expected_allowed: bool
) -> None:
"""
Tests that the `push_rules.limits.rule_size` applies to the size of the push rule.
"""
self.register_user("alice", "pass")
token = self.login("alice", "pass")
# Sanity check the test data: the canonical JSON size of the push rule body
# should be exactly as we expect, otherwise our test is void.
body_bytes = canonicaljson.encode_canonical_json(body)
# We exclude the size of the wrapper, as our implementation currently only
# counts the size of the actions and conditions fragments themselves.
self.assertEqual(
len(body_bytes) - len('{"actions":,"conditions":}'.encode("utf-8")),
expected_body_num_bytes,
)
channel = self.make_request(
"PUT",
"/pushrules/global/underride/rule1",
body,
access_token=token,
)
if expected_allowed:
self.assertEqual(
channel.code,
HTTPStatus.OK,
f"Push rule ({body_bytes!r}) within size limit should be accepted: {channel.json_body}",
)
else:
self.assertEqual(
channel.code,
HTTPStatus.REQUEST_ENTITY_TOO_LARGE,
f"Push rule ({body_bytes!r}) exceeding size limit should be rejected",
)
self.assertEqual(channel.json_body["errcode"], Codes.UNKNOWN)
@parameterized.expand(
(
(
18,
True,
),
(
24,
True,
),
(
25,
False,
),
)
)
def test_limit_on_rule_id_length(
self, rule_id_length: int, expected_allowed: bool
) -> None:
"""
Tests that the `push_rules.limits.rule_id_length` applies to the byte length
of the push rule ID.
"""
self.register_user("alice", "pass")
token = self.login("alice", "pass")
PREFIX = "global/underride/"
rule_suffix_length = rule_id_length - len(PREFIX)
assert rule_suffix_length >= 1, "can't construct a rule ID that short"
channel = self.make_request(
"PUT",
f"/pushrules/{PREFIX}{rule_suffix_length * 'a'}",
{"conditions": [], "actions": ["notify"]},
access_token=token,
)
if expected_allowed:
self.assertEqual(
channel.code,
HTTPStatus.OK,
f"Push rule ID ({rule_id_length} B) within size limit should be accepted: {channel.json_body}",
)
else:
self.assertEqual(
channel.code,
HTTPStatus.REQUEST_ENTITY_TOO_LARGE,
"Push rule ID ({rule_id_length} B) exceeding size limit should be rejected",
)
self.assertEqual(channel.json_body["errcode"], Codes.UNKNOWN)
def test_limit_on_push_rule_count(self) -> None:
"""
Tests that we are allowed to create exactly the number of push rules
specified by `push_rules.limits.rule_count`, but not a single one more.
"""
self.register_user("bob", "pass")
token = self.login("bob", "pass")
# First 2 rules are allowable
for i in range(2):
channel = self.make_request(
"PUT",
f"/pushrules/global/underride/rule{i}",
{"actions": ["notify"], "conditions": []},
access_token=token,
)
self.assertEqual(
channel.code,
HTTPStatus.OK,
f"Push rule within count limit should be allowed: {channel.json_body}",
)
# 3rd rule gets denied as it goes over the limit
channel = self.make_request(
"PUT",
"/pushrules/global/underride/rule3",
{"actions": ["notify"], "conditions": []},
access_token=token,
)
self.assertEqual(
channel.code,
HTTPStatus.BAD_REQUEST,
"Push rule exceeding count limit should be rejected",
)
self.assertEqual(channel.json_body["errcode"], Codes.UNKNOWN)