rate_limit: Add rate limiting of ZulipRemoteServer.

docs/production/mobile-push-notifications.md

@@ -173,6 +173,20 @@ itself. If your organization does not want to submit these statistics,
 you can disable this feature at any time by setting
 `SUBMIT_USAGE_STATISTICS=False` in `/etc/zulip/settings.py`.
 
+## Rate limits
+
+The Mobile Push Notifications Service API has a very high default rate
+limit of 1000 requests per minute. A Zulip server makes requests to
+this API every time it sends a push notification, which is fairly
+frequent, but we believe it to be unlikely that a self-hosted
+installation will hit this limit.
+
+This limit is primarily intended to protect the service against DoS
+attacks (intentional or otherwise). If you hit this limit or you
+anticipate that your server will require sending more push
+notifications than the limit permits, please [contact
+support](https://zulip.com/help/contact-support).
+
 ## Sending push notifications directly from your server
 
 This section documents an alternative way to send push notifications

zerver/decorator.py

@@ -48,7 +48,11 @@ from zerver.lib.utils import has_api_key_format, statsd
 from zerver.models import Realm, UserProfile, get_client, get_user_profile_by_api_key
 
 if settings.ZILENCER_ENABLED:
-    from zilencer.models import RemoteZulipServer, get_remote_server_by_uuid
+    from zilencer.models import (
+        RateLimitedRemoteZulipServer,
+        RemoteZulipServer,
+        get_remote_server_by_uuid,
+    )
 
 webhook_logger = logging.getLogger("zulip.zerver.webhooks")
 webhook_unsupported_events_logger = logging.getLogger("zulip.zerver.webhooks.unsupported")
@@ -851,6 +855,12 @@ def rate_limit_ip(request: HttpRequest, ip_addr: str, domain: str) -> None:
     RateLimitedIPAddr(ip_addr, domain=domain).rate_limit_request(request)
 
 
+def rate_limit_remote_server(
+    request: HttpRequest, remote_server: "RemoteZulipServer", domain: str
+) -> None:
+    RateLimitedRemoteZulipServer(remote_server, domain=domain).rate_limit_request(request)
+
+
 def rate_limit() -> Callable[[ViewFuncT], ViewFuncT]:
     """Rate-limits a view. Returns a decorator"""
 
@@ -869,9 +879,7 @@ def rate_limit() -> Callable[[ViewFuncT], ViewFuncT]:
 
             user = request.user
 
-            if isinstance(user, AnonymousUser) or (
-                settings.ZILENCER_ENABLED and isinstance(user, RemoteZulipServer)
-            ):
+            if isinstance(user, AnonymousUser):
                 # REMOTE_ADDR is set by SetRemoteAddrFromRealIpHeader in conjunction
                 # with the nginx configuration to guarantee this to be *the* correct
                 # IP address to use - without worrying we'll grab the IP of a proxy.
@@ -879,6 +887,8 @@ def rate_limit() -> Callable[[ViewFuncT], ViewFuncT]:
                 assert ip_addr
                 rate_limit_ip(request, ip_addr, domain="api_by_ip")
                 return func(request, *args, **kwargs)
+            elif settings.ZILENCER_ENABLED and isinstance(user, RemoteZulipServer):
+                rate_limit_remote_server(request, user, domain="api_by_remote_server")
             else:
                 assert isinstance(user, UserProfile)
                 rate_limit_user(request, user, domain="api_by_user")

zerver/tests/test_decorators.py

@@ -674,7 +674,7 @@ class RateLimitTestCase(ZulipTestCase):
         self.assertTrue(rate_limit_mock.called)
 
     @skipUnless(settings.ZILENCER_ENABLED, "requires zilencer")
-    def test_rate_limiting_happens_by_ip_if_remote_server(self) -> None:
+    def test_rate_limiting_happens_if_remote_server(self) -> None:
         server_uuid = "1234-abcd"
         server = RemoteZulipServer(
             uuid=server_uuid,
@@ -698,7 +698,7 @@ class RateLimitTestCase(ZulipTestCase):
 
         f = rate_limit()(f)
         with self.settings(RATE_LIMITING=True):
-            with mock.patch("zerver.decorator.rate_limit_ip") as rate_limit_mock:
+            with mock.patch("zerver.decorator.rate_limit_remote_server") as rate_limit_mock:
                 with self.errors_disallowed():
                     self.assertEqual(f(req), "some value")
 

zerver/tests/test_external.py

@@ -1,11 +1,12 @@
 import time
 from typing import Callable
-from unittest import mock
+from unittest import mock, skipUnless
 
 import DNS
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.http import HttpResponse
+from django.utils.timezone import now as timezone_now
 
 from zerver.forms import email_is_not_mit_mailing_list
 from zerver.lib.rate_limiter import (
@@ -17,7 +18,10 @@ from zerver.lib.rate_limiter import (
 )
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.zephyr import compute_mit_user_fullname
-from zerver.models import UserProfile
+from zerver.models import PushDeviceToken, UserProfile
+
+if settings.ZILENCER_ENABLED:
+    from zilencer.models import RateLimitedRemoteZulipServer, RemoteZulipServer
 
 
 class MITNameTest(ZulipTestCase):
@@ -168,6 +172,31 @@ class RateLimitTests(ZulipTestCase):
             # even in case of failure, to avoid polluting the rules state.
             remove_ratelimit_rule(1, 5, domain="api_by_ip")
 
+    @skipUnless(settings.ZILENCER_ENABLED, "requires zilencer")
+    def test_hit_ratelimits_as_remote_server(self) -> None:
+        add_ratelimit_rule(1, 5, domain="api_by_remote_server")
+        server_uuid = "1234-abcd"
+        server = RemoteZulipServer(
+            uuid=server_uuid,
+            api_key="magic_secret_api_key",
+            hostname="demo.example.com",
+            last_updated=timezone_now(),
+        )
+        server.save()
+
+        endpoint = "/api/v1/remotes/push/register"
+        payload = {"user_id": 10, "token": "111222", "token_kind": PushDeviceToken.GCM}
+        try:
+            # Remote servers can only make requests to the root subdomain.
+            original_default_subdomain = self.DEFAULT_SUBDOMAIN
+            self.DEFAULT_SUBDOMAIN = ""
+
+            RateLimitedRemoteZulipServer(server).clear_history()
+            self.do_test_hit_ratelimits(lambda: self.uuid_post(server_uuid, endpoint, payload))
+        finally:
+            self.DEFAULT_SUBDOMAIN = original_default_subdomain
+            remove_ratelimit_rule(1, 5, domain="api_by_remote_server")
+
     def test_hit_ratelimiterlockingexception(self) -> None:
         user = self.example_user("cordelia")
         RateLimitedUser(user).clear_history()

zilencer/models.py

@@ -1,8 +1,12 @@
 import datetime
+from typing import List, Tuple
 
+from django.conf import settings
 from django.db import models
 
 from analytics.models import BaseCount
+from zerver.lib.rate_limiter import RateLimitedObject
+from zerver.lib.rate_limiter import rules as rate_limiter_rules
 from zerver.models import AbstractPushDeviceToken, AbstractRealmAuditLog
 
 
@@ -87,3 +91,24 @@ class RemoteRealmCount(BaseCount):
 
     def __str__(self) -> str:
         return f"{self.server} {self.realm_id} {self.property} {self.subgroup} {self.value}"
+
+
+class RateLimitedRemoteZulipServer(RateLimitedObject):
+    def __init__(
+        self, remote_server: RemoteZulipServer, domain: str = "api_by_remote_server"
+    ) -> None:
+        # Remote servers can only make API requests regarding push notifications
+        # which requires ZILENCED_ENABLED and of course can't happen on API endpoints
+        # inside Tornado.
+        assert not settings.RUNNING_INSIDE_TORNADO
+        assert settings.ZILENCER_ENABLED
+
+        self.uuid = remote_server.uuid
+        self.domain = domain
+        super().__init__()
+
+    def key(self) -> str:
+        return f"{type(self).__name__}:<{self.uuid}>:{self.domain}"
+
+    def rules(self) -> List[Tuple[int, int]]:
+        return rate_limiter_rules[self.domain]

zproject/computed_settings.py

@@ -378,6 +378,9 @@ RATE_LIMITING_RULES = {
     "api_by_ip": [
         (60, 100),
     ],
+    "api_by_remote_server": [
+        (60, 1000),
+    ],
     "authenticate_by_username": [
         (1800, 5),  # 5 login attempts within 30 minutes
     ],

zproject/test_extra_settings.py

@@ -262,6 +262,7 @@ SOCIAL_AUTH_SAML_ENABLED_IDPS: Dict[str, SAMLIdPConfigDict] = {
 RATE_LIMITING_RULES: Dict[str, List[Tuple[int, int]]] = {
     "api_by_user": [],
     "api_by_ip": [],
+    "api_by_remote_server": [],
     "authenticate_by_username": [],
     "password_reset_form_by_email": [],
 }