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

Describe which rate limiter was hit in logs #16135

Merged
merged 9 commits into from
Aug 29, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/16135.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Describe which rate limiter was hit in logs.
14 changes: 12 additions & 2 deletions synapse/api/errors.py
Original file line number Diff line number Diff line change
@@ -211,6 +211,11 @@ def __init__(
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, **self._additional_fields)

@property
def debug_context(self) -> Optional[str]:
"""Override this to add debugging context that shouldn't be sent to clients."""
return None


class InvalidAPICallError(SynapseError):
"""You called an existing API endpoint, but fed that endpoint
@@ -508,8 +513,8 @@ class LimitExceededError(SynapseError):

def __init__(
self,
limiter_name: str,
code: int = 429,
msg: str = "Too Many Requests",
retry_after_ms: Optional[int] = None,
errcode: str = Codes.LIMIT_EXCEEDED,
):
@@ -518,12 +523,17 @@ def __init__(
if self.include_retry_after_header and retry_after_ms is not None
else None
)
super().__init__(code, msg, errcode, headers=headers)
super().__init__(code, "Too Many Requests", errcode, headers=headers)
self.retry_after_ms = retry_after_ms
self.limiter_name = limiter_name

def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, retry_after_ms=self.retry_after_ms)

@property
def debug_context(self) -> Optional[str]:
return self.limiter_name
Comment on lines +533 to +535
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug_context could probably just be a class-property too and __init__ could set self.debug_context = limiter_name. I don't feel strongly either way though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong feelings here either. Suggest we leave it as-is in the interests of landing this sooner rather than later.



class RoomKeysVersionError(SynapseError):
"""A client has tried to upload to a non-current version of the room_keys store"""
20 changes: 13 additions & 7 deletions synapse/api/ratelimiting.py
Original file line number Diff line number Diff line change
@@ -61,12 +61,16 @@ class Ratelimiter:
"""

def __init__(
self, store: DataStore, clock: Clock, rate_hz: float, burst_count: int
self,
store: DataStore,
clock: Clock,
cfg: RatelimitSettings,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sumnerevans pointed out to me that the docstring needs updating for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #16255.

):
self.clock = clock
self.rate_hz = rate_hz
self.burst_count = burst_count
self.rate_hz = cfg.per_second
self.burst_count = cfg.burst_count
self.store = store
self._limiter_name = cfg.key

# An ordered dictionary representing the token buckets tracked by this rate
# limiter. Each entry maps a key of arbitrary type to a tuple representing:
@@ -305,7 +309,8 @@ async def ratelimit(

if not allowed:
raise LimitExceededError(
retry_after_ms=int(1000 * (time_allowed - time_now_s))
limiter_name=self._limiter_name,
retry_after_ms=int(1000 * (time_allowed - time_now_s)),
)


@@ -322,7 +327,9 @@ def __init__(

# The rate_hz and burst_count are overridden on a per-user basis
self.request_ratelimiter = Ratelimiter(
store=self.store, clock=self.clock, rate_hz=0, burst_count=0
store=self.store,
clock=self.clock,
cfg=RatelimitSettings(key=rc_message.key, per_second=0, burst_count=0),
)
self._rc_message = rc_message

@@ -332,8 +339,7 @@ def __init__(
self.admin_redaction_ratelimiter: Optional[Ratelimiter] = Ratelimiter(
store=self.store,
clock=self.clock,
rate_hz=rc_admin_redaction.per_second,
burst_count=rc_admin_redaction.burst_count,
cfg=rc_admin_redaction,
)
else:
self.admin_redaction_ratelimiter = None
132 changes: 88 additions & 44 deletions synapse/config/ratelimiting.py
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Any, Dict, Optional
from typing import Any, Dict, Optional, cast

import attr

@@ -21,16 +21,47 @@
from ._base import Config


@attr.s(slots=True, frozen=True, auto_attribs=True)
class RatelimitSettings:
def __init__(
self,
config: Dict[str, float],
key: str
per_second: float
burst_count: int

@classmethod
def parse(
cls,
config: Dict[str, Any],
key: str,
defaults: Optional[Dict[str, float]] = None,
):
) -> "RatelimitSettings":
"""Parse config[key] as a new-style rate limiter config.
The key may refer to a nested dictionary using a full stop (.) to separate
each nested key. For example, use the key "a.b.c" to parse the following:
a:
b:
c:
per_second: 10
burst_count: 200
If this lookup fails, we'll fallback to the defaults.
"""
defaults = defaults or {"per_second": 0.17, "burst_count": 3.0}

self.per_second = config.get("per_second", defaults["per_second"])
self.burst_count = int(config.get("burst_count", defaults["burst_count"]))
rl_config = config
for part in key.split("."):
rl_config = rl_config.get(part, {})

# By this point we should have hit the rate limiter parameters.
# We don't actually check this though!
rl_config = cast(Dict[str, float], rl_config)

return cls(
key=key,
per_second=rl_config.get("per_second", defaults["per_second"]),
burst_count=int(rl_config.get("burst_count", defaults["burst_count"])),
)


@attr.s(auto_attribs=True)
@@ -49,15 +80,14 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
# Load the new-style messages config if it exists. Otherwise fall back
# to the old method.
if "rc_message" in config:
self.rc_message = RatelimitSettings(
config["rc_message"], defaults={"per_second": 0.2, "burst_count": 10.0}
self.rc_message = RatelimitSettings.parse(
config, "rc_message", defaults={"per_second": 0.2, "burst_count": 10.0}
)
else:
self.rc_message = RatelimitSettings(
{
"per_second": config.get("rc_messages_per_second", 0.2),
"burst_count": config.get("rc_message_burst_count", 10.0),
}
key="rc_messages",
per_second=config.get("rc_messages_per_second", 0.2),
burst_count=config.get("rc_message_burst_count", 10.0),
)

# Load the new-style federation config, if it exists. Otherwise, fall
@@ -79,51 +109,59 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
}
)

self.rc_registration = RatelimitSettings(config.get("rc_registration", {}))
self.rc_registration = RatelimitSettings.parse(config, "rc_registration", {})

self.rc_registration_token_validity = RatelimitSettings(
config.get("rc_registration_token_validity", {}),
self.rc_registration_token_validity = RatelimitSettings.parse(
config,
"rc_registration_token_validity",
defaults={"per_second": 0.1, "burst_count": 5},
)

# It is reasonable to login with a bunch of devices at once (i.e. when
# setting up an account), but it is *not* valid to continually be
# logging into new devices.
rc_login_config = config.get("rc_login", {})
self.rc_login_address = RatelimitSettings(
rc_login_config.get("address", {}),
self.rc_login_address = RatelimitSettings.parse(
config,
"rc_login.address",
defaults={"per_second": 0.003, "burst_count": 5},
)
self.rc_login_account = RatelimitSettings(
rc_login_config.get("account", {}),
self.rc_login_account = RatelimitSettings.parse(
config,
"rc_login.account",
defaults={"per_second": 0.003, "burst_count": 5},
)
self.rc_login_failed_attempts = RatelimitSettings(
rc_login_config.get("failed_attempts", {})
self.rc_login_failed_attempts = RatelimitSettings.parse(
config,
"rc_login.failed_attempts",
{},
)

self.federation_rr_transactions_per_room_per_second = config.get(
"federation_rr_transactions_per_room_per_second", 50
)

rc_admin_redaction = config.get("rc_admin_redaction")
self.rc_admin_redaction = None
if rc_admin_redaction:
self.rc_admin_redaction = RatelimitSettings(rc_admin_redaction)
if "rc_admin_redaction" in config:
self.rc_admin_redaction = RatelimitSettings.parse(
config, "rc_admin_redaction", {}
)

self.rc_joins_local = RatelimitSettings(
config.get("rc_joins", {}).get("local", {}),
self.rc_joins_local = RatelimitSettings.parse(
config,
"rc_joins.local",
defaults={"per_second": 0.1, "burst_count": 10},
)
self.rc_joins_remote = RatelimitSettings(
config.get("rc_joins", {}).get("remote", {}),
self.rc_joins_remote = RatelimitSettings.parse(
config,
"rc_joins.remote",
defaults={"per_second": 0.01, "burst_count": 10},
)

# Track the rate of joins to a given room. If there are too many, temporarily
# prevent local joins and remote joins via this server.
self.rc_joins_per_room = RatelimitSettings(
config.get("rc_joins_per_room", {}),
self.rc_joins_per_room = RatelimitSettings.parse(
config,
"rc_joins_per_room",
defaults={"per_second": 1, "burst_count": 10},
)

@@ -132,31 +170,37 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
# * For requests received over federation this is keyed by the origin.
#
# Note that this isn't exposed in the configuration as it is obscure.
self.rc_key_requests = RatelimitSettings(
config.get("rc_key_requests", {}),
self.rc_key_requests = RatelimitSettings.parse(
config,
"rc_key_requests",
defaults={"per_second": 20, "burst_count": 100},
)

self.rc_3pid_validation = RatelimitSettings(
config.get("rc_3pid_validation") or {},
self.rc_3pid_validation = RatelimitSettings.parse(
config,
"rc_3pid_validation",
defaults={"per_second": 0.003, "burst_count": 5},
)

self.rc_invites_per_room = RatelimitSettings(
config.get("rc_invites", {}).get("per_room", {}),
self.rc_invites_per_room = RatelimitSettings.parse(
config,
"rc_invites.per_room",
defaults={"per_second": 0.3, "burst_count": 10},
)
self.rc_invites_per_user = RatelimitSettings(
config.get("rc_invites", {}).get("per_user", {}),
self.rc_invites_per_user = RatelimitSettings.parse(
config,
"rc_invites.per_user",
defaults={"per_second": 0.003, "burst_count": 5},
)

self.rc_invites_per_issuer = RatelimitSettings(
config.get("rc_invites", {}).get("per_issuer", {}),
self.rc_invites_per_issuer = RatelimitSettings.parse(
config,
"rc_invites.per_issuer",
defaults={"per_second": 0.3, "burst_count": 10},
)

self.rc_third_party_invite = RatelimitSettings(
config.get("rc_third_party_invite", {}),
self.rc_third_party_invite = RatelimitSettings.parse(
config,
"rc_third_party_invite",
defaults={"per_second": 0.0025, "burst_count": 5},
)
8 changes: 3 additions & 5 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
@@ -218,19 +218,17 @@ def __init__(self, hs: "HomeServer"):
self._failed_uia_attempts_ratelimiter = Ratelimiter(
store=self.store,
clock=self.clock,
rate_hz=self.hs.config.ratelimiting.rc_login_failed_attempts.per_second,
burst_count=self.hs.config.ratelimiting.rc_login_failed_attempts.burst_count,
cfg=self.hs.config.ratelimiting.rc_login_failed_attempts,
)

# The number of seconds to keep a UI auth session active.
self._ui_auth_session_timeout = hs.config.auth.ui_auth_session_timeout

# Ratelimitier for failed /login attempts
# Ratelimiter for failed /login attempts
self._failed_login_attempts_ratelimiter = Ratelimiter(
store=self.store,
clock=hs.get_clock(),
rate_hz=self.hs.config.ratelimiting.rc_login_failed_attempts.per_second,
burst_count=self.hs.config.ratelimiting.rc_login_failed_attempts.burst_count,
cfg=self.hs.config.ratelimiting.rc_login_failed_attempts,
)

self._clock = self.hs.get_clock()
3 changes: 1 addition & 2 deletions synapse/handlers/devicemessage.py
Original file line number Diff line number Diff line change
@@ -90,8 +90,7 @@ def __init__(self, hs: "HomeServer"):
self._ratelimiter = Ratelimiter(
store=self.store,
clock=hs.get_clock(),
rate_hz=hs.config.ratelimiting.rc_key_requests.per_second,
burst_count=hs.config.ratelimiting.rc_key_requests.burst_count,
cfg=hs.config.ratelimiting.rc_key_requests,
)

async def on_direct_to_device_edu(self, origin: str, content: JsonDict) -> None:
6 changes: 2 additions & 4 deletions synapse/handlers/identity.py
Original file line number Diff line number Diff line change
@@ -66,14 +66,12 @@ def __init__(self, hs: "HomeServer"):
self._3pid_validation_ratelimiter_ip = Ratelimiter(
store=self.store,
clock=hs.get_clock(),
rate_hz=hs.config.ratelimiting.rc_3pid_validation.per_second,
burst_count=hs.config.ratelimiting.rc_3pid_validation.burst_count,
cfg=hs.config.ratelimiting.rc_3pid_validation,
)
self._3pid_validation_ratelimiter_address = Ratelimiter(
store=self.store,
clock=hs.get_clock(),
rate_hz=hs.config.ratelimiting.rc_3pid_validation.per_second,
burst_count=hs.config.ratelimiting.rc_3pid_validation.burst_count,
cfg=hs.config.ratelimiting.rc_3pid_validation,
)

async def ratelimit_request_token_requests(
Loading