Merge pull request #6015 from matrix-org/erikj/ratelimit_admin_redaction

Allow use of different ratelimits for admin redactions.
pull/6020/head
Erik Johnston 2019-09-11 15:39:38 +01:00 committed by GitHub
commit cbcbfe64a2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 102 additions and 11 deletions

1
changelog.d/6015.feature Normal file
View File

@ -0,0 +1 @@
Add config option to increase ratelimits for room admins redacting messages.

View File

@ -518,6 +518,9 @@ log_config: "CONFDIR/SERVERNAME.log.config"
# - one for login that ratelimits login requests based on the account the # - one for login that ratelimits login requests based on the account the
# client is attempting to log into, based on the amount of failed login # client is attempting to log into, based on the amount of failed login
# attempts for this account. # attempts for this account.
# - one for ratelimiting redactions by room admins. If this is not explicitly
# set then it uses the same ratelimiting as per rc_message. This is useful
# to allow room admins to deal with abuse quickly.
# #
# The defaults are as shown below. # The defaults are as shown below.
# #
@ -539,6 +542,10 @@ log_config: "CONFDIR/SERVERNAME.log.config"
# failed_attempts: # failed_attempts:
# per_second: 0.17 # per_second: 0.17
# burst_count: 3 # burst_count: 3
#
#rc_admin_redaction:
# per_second: 1
# burst_count: 50
# Ratelimiting settings for incoming federation # Ratelimiting settings for incoming federation

View File

@ -80,6 +80,12 @@ class RatelimitConfig(Config):
"federation_rr_transactions_per_room_per_second", 50 "federation_rr_transactions_per_room_per_second", 50
) )
rc_admin_redaction = config.get("rc_admin_redaction")
if rc_admin_redaction:
self.rc_admin_redaction = RateLimitConfig(rc_admin_redaction)
else:
self.rc_admin_redaction = None
def generate_config_section(self, **kwargs): def generate_config_section(self, **kwargs):
return """\ return """\
## Ratelimiting ## ## Ratelimiting ##
@ -102,6 +108,9 @@ class RatelimitConfig(Config):
# - one for login that ratelimits login requests based on the account the # - one for login that ratelimits login requests based on the account the
# client is attempting to log into, based on the amount of failed login # client is attempting to log into, based on the amount of failed login
# attempts for this account. # attempts for this account.
# - one for ratelimiting redactions by room admins. If this is not explicitly
# set then it uses the same ratelimiting as per rc_message. This is useful
# to allow room admins to deal with abuse quickly.
# #
# The defaults are as shown below. # The defaults are as shown below.
# #
@ -123,6 +132,10 @@ class RatelimitConfig(Config):
# failed_attempts: # failed_attempts:
# per_second: 0.17 # per_second: 0.17
# burst_count: 3 # burst_count: 3
#
#rc_admin_redaction:
# per_second: 1
# burst_count: 50
# Ratelimiting settings for incoming federation # Ratelimiting settings for incoming federation

View File

@ -45,6 +45,7 @@ class BaseHandler(object):
self.state_handler = hs.get_state_handler() self.state_handler = hs.get_state_handler()
self.distributor = hs.get_distributor() self.distributor = hs.get_distributor()
self.ratelimiter = hs.get_ratelimiter() self.ratelimiter = hs.get_ratelimiter()
self.admin_redaction_ratelimiter = hs.get_admin_redaction_ratelimiter()
self.clock = hs.get_clock() self.clock = hs.get_clock()
self.hs = hs self.hs = hs
@ -53,7 +54,7 @@ class BaseHandler(object):
self.event_builder_factory = hs.get_event_builder_factory() self.event_builder_factory = hs.get_event_builder_factory()
@defer.inlineCallbacks @defer.inlineCallbacks
def ratelimit(self, requester, update=True): def ratelimit(self, requester, update=True, is_admin_redaction=False):
"""Ratelimits requests. """Ratelimits requests.
Args: Args:
@ -62,6 +63,9 @@ class BaseHandler(object):
Set to False when doing multiple checks for one request (e.g. Set to False when doing multiple checks for one request (e.g.
to check up front if we would reject the request), and set to to check up front if we would reject the request), and set to
True for the last call for a given request. True for the last call for a given request.
is_admin_redaction (bool): Whether this is a room admin/moderator
redacting an event. If so then we may apply different
ratelimits depending on config.
Raises: Raises:
LimitExceededError if the request should be ratelimited LimitExceededError if the request should be ratelimited
@ -90,16 +94,33 @@ class BaseHandler(object):
messages_per_second = override.messages_per_second messages_per_second = override.messages_per_second
burst_count = override.burst_count burst_count = override.burst_count
else: else:
messages_per_second = self.hs.config.rc_message.per_second # We default to different values if this is an admin redaction and
burst_count = self.hs.config.rc_message.burst_count # the config is set
if is_admin_redaction and self.hs.config.rc_admin_redaction:
messages_per_second = self.hs.config.rc_admin_redaction.per_second
burst_count = self.hs.config.rc_admin_redaction.burst_count
else:
messages_per_second = self.hs.config.rc_message.per_second
burst_count = self.hs.config.rc_message.burst_count
allowed, time_allowed = self.ratelimiter.can_do_action( if is_admin_redaction and self.hs.config.rc_admin_redaction:
user_id, # If we have separate config for admin redactions we use a separate
time_now, # ratelimiter
rate_hz=messages_per_second, allowed, time_allowed = self.admin_redaction_ratelimiter.can_do_action(
burst_count=burst_count, user_id,
update=update, time_now,
) rate_hz=messages_per_second,
burst_count=burst_count,
update=update,
)
else:
allowed, time_allowed = self.ratelimiter.can_do_action(
user_id,
time_now,
rate_hz=messages_per_second,
burst_count=burst_count,
update=update,
)
if not allowed: if not allowed:
raise LimitExceededError( raise LimitExceededError(
retry_after_ms=int(1000 * (time_allowed - time_now)) retry_after_ms=int(1000 * (time_allowed - time_now))

View File

@ -729,7 +729,27 @@ class EventCreationHandler(object):
assert not self.config.worker_app assert not self.config.worker_app
if ratelimit: if ratelimit:
yield self.base_handler.ratelimit(requester) # We check if this is a room admin redacting an event so that we
# can apply different ratelimiting. We do this by simply checking
# it's not a self-redaction (to avoid having to look up whether the
# user is actually admin or not).
is_admin_redaction = False
if event.type == EventTypes.Redaction:
original_event = yield self.store.get_event(
event.redacts,
check_redacted=False,
get_prev_content=False,
allow_rejected=False,
allow_none=True,
)
is_admin_redaction = (
original_event and event.sender != original_event.sender
)
yield self.base_handler.ratelimit(
requester, is_admin_redaction=is_admin_redaction
)
yield self.base_handler.maybe_kick_guest_users(event, context) yield self.base_handler.maybe_kick_guest_users(event, context)

View File

@ -221,6 +221,7 @@ class HomeServer(object):
self.clock = Clock(reactor) self.clock = Clock(reactor)
self.distributor = Distributor() self.distributor = Distributor()
self.ratelimiter = Ratelimiter() self.ratelimiter = Ratelimiter()
self.admin_redaction_ratelimiter = Ratelimiter()
self.registration_ratelimiter = Ratelimiter() self.registration_ratelimiter = Ratelimiter()
self.datastore = None self.datastore = None
@ -279,6 +280,9 @@ class HomeServer(object):
def get_registration_ratelimiter(self): def get_registration_ratelimiter(self):
return self.registration_ratelimiter return self.registration_ratelimiter
def get_admin_redaction_ratelimiter(self):
return self.admin_redaction_ratelimiter
def build_federation_client(self): def build_federation_client(self):
return FederationClient(self) return FederationClient(self)

View File

@ -30,6 +30,14 @@ class RedactionsTestCase(HomeserverTestCase):
sync.register_servlets, sync.register_servlets,
] ]
def make_homeserver(self, reactor, clock):
config = self.default_config()
config["rc_message"] = {"per_second": 0.2, "burst_count": 10}
config["rc_admin_redaction"] = {"per_second": 1, "burst_count": 100}
return self.setup_test_homeserver(config=config)
def prepare(self, reactor, clock, hs): def prepare(self, reactor, clock, hs):
# register a couple of users # register a couple of users
self.mod_user_id = self.register_user("user1", "pass") self.mod_user_id = self.register_user("user1", "pass")
@ -177,3 +185,20 @@ class RedactionsTestCase(HomeserverTestCase):
self._redact_event( self._redact_event(
self.other_access_token, self.room_id, create_event_id, expect_code=403 self.other_access_token, self.room_id, create_event_id, expect_code=403
) )
def test_redact_event_as_moderator_ratelimit(self):
"""Tests that the correct ratelimiting is applied to redactions
"""
message_ids = []
# as a regular user, send messages to redact
for _ in range(20):
b = self.helper.send(room_id=self.room_id, tok=self.other_access_token)
message_ids.append(b["event_id"])
self.reactor.advance(10) # To get around ratelimits
# as the moderator, send a bunch of redactions
for msg_id in message_ids:
# These should all succeed, even though this would be denied by
# the standard message ratelimiter
self._redact_event(self.mod_access_token, self.room_id, msg_id)