From 372bf073c1a5ac5e66cca717ce9aa72c43ba9404 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Thu, 16 Aug 2018 21:25:16 +0100 Subject: [PATCH 1/4] block event creation and room creation on hitting resource limits --- synapse/handlers/message.py | 6 +++++- synapse/handlers/room.py | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 893c9bcdc4db..4d006df63c62 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -276,10 +276,14 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None, where *hashes* is a map from algorithm to hash. If None, they will be requested from the database. - + Raises: + ResourceLimitError if server is blocked to some resource being + exceeded Returns: Tuple of created event (FrozenEvent), Context """ + yield self.auth.check_auth_blocking(requester.user.to_string()) + builder = self.event_builder_factory.new(event_dict) self.validator.validate_new(builder) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 6a17c42238ec..c3f820b9753b 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -98,9 +98,13 @@ def create_room(self, requester, config, ratelimit=True, Raises: SynapseError if the room ID couldn't be stored, or something went horribly wrong. + ResourceLimitError if server is blocked to some resource being + exceeded """ user_id = requester.user.to_string() + self.auth.check_auth_blocking(user_id) + if not self.spam_checker.user_may_create_room(user_id): raise SynapseError(403, "You are not permitted to create rooms") From bcfeb44afe750dadd4199e7c02302b0157bdab11 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Thu, 16 Aug 2018 22:55:32 +0100 Subject: [PATCH 2/4] call reap on start up and fix under reaping bug --- synapse/api/auth.py | 2 +- synapse/app/homeserver.py | 1 + synapse/storage/monthly_active_users.py | 5 ++++- tests/storage/test_monthly_active_users.py | 13 +++++++++++++ 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 3b2a2ab77ade..ab1e3a4e353f 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -799,7 +799,7 @@ def check_auth_blocking(self, user_id=None): current_mau = yield self.store.get_monthly_active_count() if current_mau >= self.hs.config.max_mau_value: raise AuthError( - 403, "Monthly Active User Limits AU Limit Exceeded", + 403, "Monthly Active User Limit Exceeded", admin_uri=self.hs.config.admin_uri, errcode=Codes.RESOURCE_LIMIT_EXCEED ) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index a98bb506e5fb..800b9c0e3c64 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -525,6 +525,7 @@ def generate_user_daily_visit_stats(): clock.looping_call( hs.get_datastore().reap_monthly_active_users, 1000 * 60 * 60 ) + yield hs.get_datastore().reap_monthly_active_users() @defer.inlineCallbacks def generate_monthly_active_users(): diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py index 7e417f811e9e..06f9a75a9779 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -96,7 +96,10 @@ def _reap_users(txn): # While Postgres does not require 'LIMIT', but also does not support # negative LIMIT values. So there is no way to write it that both can # support - query_args = [self.hs.config.max_mau_value] + safe_guard = self.hs.config.max_mau_value - len(self.reserved_users) + # Must be greater than zero for postgres + safe_guard = safe_guard if safe_guard > 0 else 0 + query_args = [safe_guard] base_sql = """ DELETE FROM monthly_active_users diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index 511acbde9bc2..f2ed866ae7fc 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -75,6 +75,19 @@ def test_initialise_reserved_users(self): active_count = yield self.store.get_monthly_active_count() self.assertEquals(active_count, user_num) + # Test that regalar users are removed from the db + ru_count = 2 + yield self.store.upsert_monthly_active_user("@ru1:server") + yield self.store.upsert_monthly_active_user("@ru2:server") + active_count = yield self.store.get_monthly_active_count() + + self.assertEqual(active_count, user_num + ru_count) + self.hs.config.max_mau_value = user_num + yield self.store.reap_monthly_active_users() + + active_count = yield self.store.get_monthly_active_count() + self.assertEquals(active_count, user_num) + @defer.inlineCallbacks def test_can_insert_and_count_mau(self): count = yield self.store.get_monthly_active_count() From 7e513421969738428c40d9d6032c798edd9ebab6 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Thu, 16 Aug 2018 23:05:20 +0100 Subject: [PATCH 3/4] For resource limit blocked users, prevent writing into rooms --- changelog.d/3708.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3708.feature diff --git a/changelog.d/3708.feature b/changelog.d/3708.feature new file mode 100644 index 000000000000..2f146ba62b34 --- /dev/null +++ b/changelog.d/3708.feature @@ -0,0 +1 @@ +For resource limit blocked users, prevent writing into rooms From 521d369e7a6d6497427a03f3e4f81d20bd2e5761 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Fri, 17 Aug 2018 10:12:11 +0100 Subject: [PATCH 4/4] remove errant yield --- synapse/app/homeserver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 800b9c0e3c64..005921dcf733 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -525,7 +525,7 @@ def generate_user_daily_visit_stats(): clock.looping_call( hs.get_datastore().reap_monthly_active_users, 1000 * 60 * 60 ) - yield hs.get_datastore().reap_monthly_active_users() + hs.get_datastore().reap_monthly_active_users() @defer.inlineCallbacks def generate_monthly_active_users():