rate_limit: Restrict tornado backend to explicitly specified domains.

This will protect us in case of some kinds of bugs that could allow
making requests such as password authentication attempts to tornado.
Without restricting the domains to which the in-memory backend can
be applied, such bugs would lead to attackers having multiple times
larger rate limits for these sensitive requests.
This commit is contained in:
Mateusz Mandera
2020-05-25 21:50:07 +02:00
committed by Tim Abbott
parent ad99bba121
commit 13c3eaf086
3 changed files with 17 additions and 4 deletions

View File

@@ -32,8 +32,6 @@ class RateLimitedObject(ABC):
def __init__(self, backend: Optional['Type[RateLimiterBackend]']=None) -> None: def __init__(self, backend: Optional['Type[RateLimiterBackend]']=None) -> None:
if backend is not None: if backend is not None:
self.backend: Type[RateLimiterBackend] = backend self.backend: Type[RateLimiterBackend] = backend
elif settings.RUNNING_INSIDE_TORNADO:
self.backend = TornadoInMemoryRateLimiterBackend
else: else:
self.backend = RedisRateLimiterBackend self.backend = RedisRateLimiterBackend
@@ -111,7 +109,11 @@ class RateLimitedUser(RateLimitedObject):
def __init__(self, user: UserProfile, domain: str='api_by_user') -> None: def __init__(self, user: UserProfile, domain: str='api_by_user') -> None:
self.user = user self.user = user
self.domain = domain self.domain = domain
super().__init__() if settings.RUNNING_INSIDE_TORNADO and domain in settings.RATE_LIMITING_DOMAINS_FOR_TORNADO:
backend: Optional[Type[RateLimiterBackend]] = TornadoInMemoryRateLimiterBackend
else:
backend = None
super().__init__(backend=backend)
def key(self) -> str: def key(self) -> str:
return "{}:{}:{}".format(type(self).__name__, self.user.id, self.domain) return "{}:{}:{}".format(type(self).__name__, self.user.id, self.domain)

View File

@@ -184,9 +184,13 @@ class TornadoInMemoryRateLimiterBackendTest(RateLimiterBackendBase):
def test_used_in_tornado(self) -> None: def test_used_in_tornado(self) -> None:
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
with self.settings(RUNNING_INSIDE_TORNADO=True): with self.settings(RUNNING_INSIDE_TORNADO=True):
obj = RateLimitedUser(user_profile) obj = RateLimitedUser(user_profile, domain='api_by_user')
self.assertEqual(obj.backend, TornadoInMemoryRateLimiterBackend) self.assertEqual(obj.backend, TornadoInMemoryRateLimiterBackend)
with self.settings(RUNNING_INSIDE_TORNADO=True):
obj = RateLimitedUser(user_profile, domain='some_domain')
self.assertEqual(obj.backend, RedisRateLimiterBackend)
def test_block_access(self) -> None: def test_block_access(self) -> None:
obj = self.create_object('test', [(2, 5), ]) obj = self.create_object('test', [(2, 5), ])
start_time = time.time() start_time = time.time()

View File

@@ -367,6 +367,13 @@ RATE_LIMITING_RULES = {
], ],
} }
# List of domains that, when applied to a request in a Tornado process,
# will be handled with the separate in-memory rate limiting backend for Tornado,
# which has its own buckets separate from the default backend.
# In principle, it should be impossible to make requests to tornado that fall into
# other domains, but we use this list as an extra precaution.
RATE_LIMITING_DOMAINS_FOR_TORNADO = ['api_by_user']
RATE_LIMITING_MIRROR_REALM_RULES = [ RATE_LIMITING_MIRROR_REALM_RULES = [
(60, 50), # 50 emails per minute (60, 50), # 50 emails per minute
(300, 120), # 120 emails per 5 minutes (300, 120), # 120 emails per 5 minutes