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
Show file tree
Hide file tree
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.
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 12 additions & 2 deletions synapse/api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
clokep marked this conversation as resolved.
Show resolved Hide resolved
return None


class InvalidAPICallError(SynapseError):
"""You called an existing API endpoint, but fed that endpoint
Expand Down Expand Up @@ -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,
):
Expand All @@ -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"""
Expand Down
20 changes: 13 additions & 7 deletions synapse/api/ratelimiting.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved

# An ordered dictionary representing the token buckets tracked by this rate
# limiter. Each entry maps a key of arbitrary type to a tuple representing:
Expand Down Expand Up @@ -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)),
)


Expand All @@ -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

Expand All @@ -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
Expand Down
132 changes: 88 additions & 44 deletions synapse/config/ratelimiting.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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},
)

Expand All @@ -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
Expand Up @@ -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()
Expand Down
3 changes: 1 addition & 2 deletions synapse/handlers/devicemessage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 2 additions & 4 deletions synapse/handlers/identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading