Max Ronzhin
c60047c594
Два бага в одном месте, оба вскрылись на нагрузочном тесте 31.07.2026. 1. Ключ лимита строился по `request.client.host`. Backend стоит за nginx, поэтому это адрес КОНТЕЙНЕРА NGINX, одинаковый для всех пользователей. Проверено на проде: в Redis лежал единственный ключ `rate_limit:resolve:172.18.0.13`. То есть лимит «10 запросов в минуту» действовал на весь инстанс разом, а не на клиента. 2. Считались все запросы подряд, включая успешные. Одиннадцатый человек, открывший ссылку на конференцию в течение минуты, получал 429 — и видел «Не удалось найти конференцию» для существующей и активной конференции. Люди попадали внутрь с пятой-десятой попытки, попадая в новое окно. Что изменилось: - адрес клиента берётся из `X-Real-IP` (nginx его уже передаёт). Именно `X-Real-IP`, а не первый элемент `X-Forwarded-For`: последний заполняется через `$proxy_add_x_forwarded_for`, то есть дописывается к присланному клиентом, и лимит обходился бы одним заголовком; - жёсткий счётчик (10/мин, как было) теперь считает только ПРОМАХИ: конференция не найдена или пароль неверен. Именно так выглядит перебор номера, от которого лимит и защищает по ADR-001, п.4; - на общий поток с адреса оставлен мягкий потолок 300/мин — против тупого флуда. Офис за общим NAT это один адрес, поэтому потолок заведомо выше правдоподобного числа участников одной конференции. Тесты: успешные резолвы и гостевые входы не упираются в лимит (50 и 30 подряд); перебор номера, несуществующий идентификатор и подбор пароля по-прежнему упираются; лимит одного клиента не задевает другого.
81 lines
5.0 KiB
Python
81 lines
5.0 KiB
Python
"""Rate limit на основе Redis `INCR`+`EXPIRE` для публичных (без auth) эндпоинтов.
|
||
|
||
Используется резолвом конференций и гостевым входом (`api/conferences.py`) —
|
||
эндпоинтами без аутентификации, уязвимыми к перебору номера/ссылки конференции
|
||
(см. ADR-001, п.4 — оценка энтропии и рекомендуемый лимит 10 запросов/мин на IP).
|
||
|
||
## Два счётчика вместо одного (0.0.18)
|
||
|
||
Прежняя схема считала ВСЕ запросы подряд с лимитом 10/мин. На нагрузочном
|
||
тесте 31.07.2026 это остановило вход целой конференции: люди открывали ссылку
|
||
одновременно, одиннадцатый получал 429, а фронтенд показывал «Не удалось найти
|
||
конференцию» — при том что конференция существовала и была активна.
|
||
|
||
Смысл лимита по ADR-001 — защита от ПЕРЕБОРА номера конференции. Перебор — это
|
||
поток промахов; легитимный участник открывает существующую ссылку и получает
|
||
успех. Поэтому:
|
||
|
||
- `RATE_LIMIT_MISS_MAX_REQUESTS` — жёсткий счётчик промахов (конференция не
|
||
найдена, неверный пароль). Именно он защищает от перебора, и он остался
|
||
прежним — 10/мин;
|
||
- `RATE_LIMIT_SOFT_MAX_REQUESTS` — мягкий потолок на общее число обращений с
|
||
одного адреса. Нужен только против тупого флуда; рассчитан так, чтобы сотня
|
||
человек из офиса за общим NAT спокойно зашла в одну конференцию.
|
||
"""
|
||
|
||
from fastapi import HTTPException, Request, status
|
||
|
||
from core.redis import redis_client
|
||
|
||
RATE_LIMIT_MAX_REQUESTS = 10
|
||
RATE_LIMIT_WINDOW_SECONDS = 60
|
||
|
||
# Промахи: перебор номера/ссылки или подбор пароля конференции.
|
||
RATE_LIMIT_MISS_MAX_REQUESTS = 10
|
||
# Общий поток с одного IP. Офис за общим NAT — это ОДИН адрес, поэтому потолок
|
||
# заведомо выше правдоподобного числа участников одной конференции.
|
||
RATE_LIMIT_SOFT_MAX_REQUESTS = 300
|
||
|
||
|
||
def client_ip(request: Request) -> str:
|
||
"""IP клиента для rate limit — с учётом того, что backend стоит за nginx.
|
||
|
||
`request.client.host` — это TCP-peer, то есть контейнер nginx, один и тот же
|
||
для всех пользователей. С ним лимит превращался в общий на весь инстанс:
|
||
на проде в Redis лежал единственный ключ `rate_limit:resolve:172.18.0.13`,
|
||
и десяти запросов в минуту хватало, чтобы заблокировать вход всем сразу.
|
||
|
||
Берём `X-Real-IP`, а НЕ первый элемент `X-Forwarded-For`: nginx заполняет
|
||
его через `$proxy_add_x_forwarded_for`, то есть ДОПИСЫВАЕТ к присланному
|
||
клиентом. Первый элемент там подделывается одним заголовком, и лимит
|
||
обходился бы тривиально. `X-Real-IP` nginx всегда перезаписывает своим
|
||
`$remote_addr` (см. deploy/nginx/nginx.conf.template).
|
||
"""
|
||
real_ip = request.headers.get("x-real-ip")
|
||
if real_ip:
|
||
return real_ip.strip()
|
||
return request.client.host if request.client else "unknown"
|
||
|
||
|
||
async def enforce_rate_limit(
|
||
key: str,
|
||
*,
|
||
max_requests: int = RATE_LIMIT_MAX_REQUESTS,
|
||
window_seconds: int = RATE_LIMIT_WINDOW_SECONDS,
|
||
) -> None:
|
||
"""Увеличить счётчик запросов по ключу; бросить 429, если лимит превышен.
|
||
|
||
`INCR` атомарно создаёт ключ со значением 1, если его ещё не было; TTL
|
||
выставляется только при первом инкременте в окне (когда счётчик стал
|
||
равен 1) — иначе окно продлевалось бы при каждом запросе и лимит
|
||
никогда бы не истекал.
|
||
"""
|
||
redis_key = f"rate_limit:{key}"
|
||
current = await redis_client.incr(redis_key)
|
||
if current == 1:
|
||
await redis_client.expire(redis_key, window_seconds)
|
||
if current > max_requests:
|
||
raise HTTPException(
|
||
status_code=status.HTTP_429_TOO_MANY_REQUESTS, detail="rate_limit_exceeded"
|
||
)
|