All checks were successful
CI / changes (pull_request) Successful in 8s
CI Trade-In / changes (pull_request) Successful in 9s
CI / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 3m18s
Два пункта хвоста #2712/#2717. 1. След инцидента. Отказ при насыщении сознательно не пишет login_failed и не тратит бюджет неудач по имени (иначе насыщением блокируют чужую учётку) — значит инцидент был виден только строкой logger.warning НА КАЖДЫЙ отказ. Лог у бэкенда общий и ограниченный (docker json-file, 20m × 3): при флуде в сотни запросов в секунду 60 МБ прокручиваются за минуты и выселяют все прочие логи ровно во время атаки. Теперь на окно (1с) — одна строка и одно событие login_verify_saturated в user_events, оба с числом отказов, накопленных с прошлой записи. Событие важнее строки: аудит переживает и ротацию логов, и редеплой. Имя в событии пустое намеренно — отказ случился до того, как мы на имя посмотрели, а запись присланного дала бы атакующему строки аудита с любым именем на выбор. 2. Гейт насыщения переехал ПЕРЕД выборкой пользователя. Раньше заведомо отклоняемый запрос всё равно брал соединение из пула и делал SELECT по имени — и это была единственная работа на пути отказа, чьё время зависит от существования учётки (bcrypt, который эту разницу ровняет, до отказанного запроса не доходит). Добавлен verify_slots_saturated(key) — тот же предикат, что решает отказ, но без взятия слота; verify_password_bounded зовёт его же, так что двум условиям разъехаться нечем и инвариант «одна точка выноса = одна точка учёта» цел. Предчек учитывает и общий потолок, и долю на ключ (#2714): при флуде с одного адреса первой упирается именно доля. Замер (20 заведомо отклоняемых запросов): до — 20 выборок из БД, 20 строк лога; после — 0 выборок, 1 строка. Все три ветки проверены мутацией кода: тесты краснеют. Refs #2715
281 lines
20 KiB
Python
281 lines
20 KiB
Python
"""Bcrypt password hashing для DB-auth (#2550 — foundation, эпик #2549).
|
||
|
||
bcrypt тихо обрезает пароли длиннее 72 байт (UTF-8) — это silent-truncation
|
||
дыра (два разных пароля с общим 72-байтовым префиксом хешируются одинаково).
|
||
`hash_password` явно ловит это и падает с ValueError вместо тихого поведения.
|
||
`verify_password` на длинном пароле возвращает False (не raise) — сравнение
|
||
паролей не должно ронять запрос авторизации.
|
||
|
||
#2665: из `async def` зови ТОЛЬКО `verify_password_bounded` — см. её docstring.
|
||
Синхронный `verify_password` остаётся для sync-кода (сидов, тестов, CLI) и как
|
||
тело, которое исполняется в пуле.
|
||
|
||
Правило про пул относится к СВЕРКЕ, не к хешированию. `hash_password` — тот же
|
||
cost 12 и те же ~282 мс на цикле — сознательно остаётся синхронным в
|
||
`app/api/v1/team.py` (заведение сотрудника, смена пароля): это редкая операция
|
||
АУТЕНТИФИЦИРОВАННОГО менеджера, её нельзя вызвать анонимно и потому нельзя
|
||
превратить в поток. Станет их много — переносить тем же приёмом.
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import asyncio
|
||
import logging
|
||
from concurrent.futures import ThreadPoolExecutor
|
||
|
||
import bcrypt
|
||
|
||
from app.core.config import settings
|
||
|
||
logger = logging.getLogger(__name__)
|
||
|
||
_BCRYPT_MAX_BYTES = 72
|
||
_BCRYPT_ROUNDS = 12
|
||
|
||
|
||
def hash_password(plain: str) -> str:
|
||
"""Хеширует пароль через bcrypt (rounds=12).
|
||
|
||
Raises:
|
||
ValueError: пустой пароль или пароль длиннее 72 байт в UTF-8
|
||
(bcrypt тихо обрезает — недопустимо, см. модульный docstring).
|
||
"""
|
||
if not plain:
|
||
raise ValueError("password must not be empty")
|
||
|
||
encoded = plain.encode("utf-8")
|
||
if len(encoded) > _BCRYPT_MAX_BYTES:
|
||
raise ValueError(
|
||
f"password too long: {len(encoded)} bytes (bcrypt max {_BCRYPT_MAX_BYTES})"
|
||
)
|
||
|
||
salt = bcrypt.gensalt(rounds=_BCRYPT_ROUNDS)
|
||
hashed = bcrypt.hashpw(encoded, salt)
|
||
return hashed.decode("utf-8")
|
||
|
||
|
||
def verify_password(plain: str, hashed: str) -> bool:
|
||
"""Сверяет пароль с bcrypt-хешем.
|
||
|
||
Пустой пароль или пароль длиннее 72 байт в UTF-8 → False (не raise —
|
||
verify — это false/true проверка на этапе логина, а не валидация ввода).
|
||
"""
|
||
if not plain or not hashed:
|
||
return False
|
||
|
||
encoded = plain.encode("utf-8")
|
||
if len(encoded) > _BCRYPT_MAX_BYTES:
|
||
return False
|
||
|
||
try:
|
||
return bcrypt.checkpw(encoded, hashed.encode("utf-8"))
|
||
except (ValueError, TypeError) as e:
|
||
# Malformed hash (напр. не-bcrypt строка в БД) — не должно ронять login.
|
||
logger.warning("verify_password: malformed hash rejected: %s", e)
|
||
return False
|
||
|
||
|
||
class PasswordVerifyOverloadedError(RuntimeError):
|
||
"""Свободных слотов на проверку пароля нет. Вызывающий обязан ответить 429."""
|
||
|
||
|
||
# Пул, в котором крутится bcrypt. `max_workers` — не тюнинг пропускной
|
||
# способности, а САМ ПОТОЛОК ТЕМПА: проверок в секунду не больше, чем
|
||
# workers / 282мс, независимо от числа соединений. Читается один раз на импорте
|
||
# — размер пула по определению статичен (см. `login_password_verify_workers`).
|
||
_VERIFY_POOL = ThreadPoolExecutor(
|
||
max_workers=settings.login_password_verify_workers,
|
||
thread_name_prefix="pw-verify",
|
||
)
|
||
|
||
# Сколько проверок сейчас в работе ИЛИ ждут очереди в пуле. Обычный int без
|
||
# лока — намеренно: и инкремент, и декремент выполняются в потоке событийного
|
||
# цикла, между чтением и записью нет ни одного `await`, так что чередования
|
||
# внутри пары нет. Счётчик, а не `asyncio.Semaphore`: мы никогда не ЖДЁМ на нём
|
||
# (сверх лимита — сразу отказ), а int не имеет привязки к конкретному циклу и
|
||
# потому одинаково честен под несколькими event loop'ами в тестах.
|
||
_verify_inflight = 0
|
||
|
||
# То же самое, но в разрезе ключа (#2714). Запись живёт РОВНО пока ключ держит
|
||
# хотя бы слот и удаляется на нуле: размер словаря ограничен числом слотов
|
||
# (`login_password_verify_max_inflight`), а не числом когда-либо виденных
|
||
# адресов — иначе перебор с ротацией IP растил бы его без границы.
|
||
_verify_inflight_by_key: dict[str, int] = {}
|
||
|
||
|
||
def _per_key_slot_cap() -> int:
|
||
"""Сколько слотов из общего лимита разрешено ОДНОМУ ключу.
|
||
|
||
Половина — минимальное деление, при котором один источник, сколько бы он ни
|
||
слал, физически не может занять всё: вторая половина остаётся тем, кто
|
||
приходит впервые. Настройкой не сделано сознательно — это доля, а не
|
||
величина, и подкручивать её нечем: 100% возвращает поведение, ради отказа
|
||
от которого правка написана.
|
||
|
||
Читается на каждом вызове, а не на импорте, — как `_throttle_delay_s`:
|
||
иначе тестовый monkeypatch лимита не влиял бы на долю.
|
||
|
||
`max(1, …)`: при `max_inflight=1` половина округлилась бы в 0, и КАЖДЫЙ вход
|
||
получал бы отказ молча (свободных слотов нет ни у кого). Молчаливый отказ
|
||
всем — ровно тот класс поломки, от которого страхует `ge=1` на самой
|
||
настройке; здесь тот же страховочный пол, но от деления.
|
||
"""
|
||
return max(1, settings.login_password_verify_max_inflight // 2)
|
||
|
||
|
||
def verify_slots_saturated(key: str) -> bool:
|
||
"""Тот же предикат, по которому отказывает `verify_password_bounded`, но БЕЗ взятия слота.
|
||
|
||
Нужен вызывающему ровно затем, чтобы отказать ДО похода в БД (#2715). Гейт
|
||
стоял ПОСЛЕ выборки пользователя, и каждый заведомо отклоняемый запрос всё
|
||
равно брал соединение из пула и делал SELECT по имени — тогда, когда система
|
||
уже перегружена. Хуже того, под насыщением эта выборка оставалась
|
||
ЕДИНСТВЕННОЙ работой на пути отказа: bcrypt, который ровняет время ответа
|
||
для существующего и несуществующего имени, ниже по течению и до него не
|
||
доходит, так что разницу «строка найдена / не найдена» ничто не маскировало.
|
||
|
||
Предчек, а не решение: авторитетная проверка остаётся внутри
|
||
`verify_password_bounded` — она зовёт ЭТУ ЖЕ функцию, так что разъехаться
|
||
двум условиям нечем, и инвариант «одна точка выноса = одна точка учёта»
|
||
цел (слот здесь не резервируется и не отдаётся).
|
||
|
||
Учитывает и общий потолок, и долю на ключ (#2714) — иначе предчек не
|
||
покрывал бы главный случай: при флуде с ОДНОГО адреса первым упирается
|
||
именно доля, и большинство отказов снова ходило бы в базу.
|
||
"""
|
||
return (
|
||
_verify_inflight >= settings.login_password_verify_max_inflight
|
||
or _verify_inflight_by_key.get(key, 0) >= _per_key_slot_cap()
|
||
)
|
||
|
||
|
||
async def verify_password_bounded(plain: str, hashed: str, *, key: str) -> bool:
|
||
"""`verify_password`, унесённая с событийного цикла И с сознательным потолком темпа (#2665).
|
||
|
||
ДВЕ ПОЛОВИНЫ ОДНОЙ ПРАВКИ, И ЖИВУТ ОНИ ЗДЕСЬ ВМЕСТЕ НЕ ИЗ ЛЮБВИ К ПОРЯДКУ.
|
||
Порознь каждая делает хуже, чем было:
|
||
- вынести bcrypt в пул, не поставив потолок → перебор УСКОРЯЕТСЯ (замер
|
||
ниже: 3.6/с → 16/с на дефолтном executor'е);
|
||
- поставить потолок, не вынося bcrypt → 282 мс простоя всего API на каждую
|
||
попытку остаются.
|
||
Поэтому единственная точка выноса в поток и единственная точка учёта слотов —
|
||
одна и та же функция: состояние «вынесено, но потолка нет» невыразимо.
|
||
|
||
Замер в прод-контейнере (2026-08-06, cost 12, все живые хеши `$2b$12$`):
|
||
verify_password = 282 мс медиана;
|
||
вызов прямо в `async def` — 3.6 проверки/с, стойло событийного цикла 836 мс
|
||
(это и был «потолок» — случайный, ценой отказа в обслуживании всего API);
|
||
`asyncio.to_thread` без потолка — 16 проверок/с, стойло 6 мс.
|
||
Отсюда дефолт `workers=1`: потолок остаётся тем же ~3.5/с, что был, а API
|
||
перестаёт стоять. Числа перепроверяемы: tests/test_password.py.
|
||
|
||
Потолок держится ПРОЦЕССОМ, а не общим хранилищем. Это проверено, а не
|
||
предположено: прод-бэкенд запущен `uvicorn app.main:app` без `--workers`
|
||
(один процесс), а `REDIS_URL` в окружении tradein-backend НЕ ЗАДАН вовсе
|
||
(`printenv | grep -c ^REDIS_URL=` → 0, находка эпика #2674 — кэш поиска всю
|
||
жизнь стучится в localhost и получает отказ). Потолок на Redis был бы
|
||
потолком, который молча не работает.
|
||
Ceiling: появятся `--workers N` (или `WEB_CONCURRENCY=N` в `.env.runtime` —
|
||
uvicorn читает число процессов и оттуда, а файл правится руками на VPS) —
|
||
темп множится на N, как и у соседних in-memory лимитеров в
|
||
app/api/v1/auth.py; тогда потолок надо переносить в общее хранилище,
|
||
предварительно убедившись, что оно реально доступно.
|
||
|
||
ДОЛЯ НА КЛЮЧ (#2714). Слоты — общий котёл, и потолок исправно бил по своим:
|
||
пока флуд держал все четыре, легитимный вход с ВЕРНЫМ паролем получал 429
|
||
столько раз, сколько пытался. Поэтому *key* (у единственного вызывающего —
|
||
IP клиента) не берёт больше `_per_key_slot_cap()`: сколько бы один источник
|
||
ни слал, половина ёмкости остаётся тем, кто приходит впервые. Учёт по ключу
|
||
живёт ЗДЕСЬ ЖЕ и отдаётся тем же `_release_verify_slot` — инвариант «одна
|
||
точка выноса = одна точка учёта» не делится надвое.
|
||
|
||
Чего это НЕ делает, и это не оговорка ради приличия. Ключом может быть
|
||
только IP, а IP:
|
||
- подделывается, если между нами и клиентом окажется ещё один прокси
|
||
(сейчас доверенный хоп ровно один — Caddy, `ratelimit._client_ip` берёт
|
||
правый элемент XFF; появится второй — ключ станет клиентским вводом);
|
||
- разделяется: за NAT/корпоративным шлюзом вся организация приходит с
|
||
одного адреса и делит одну долю с чужим перебором. СОСЕДЯМ ПО АДРЕСУ
|
||
СТАЛО ХУЖЕ, и это честный размен, а не побочный эффект: при флуде в
|
||
3 запроса/с с того же адреса свои входят 69% попыток против 94% до
|
||
правки, а порог, за которым сосед перестаёт входить, падает с ~14 до
|
||
~7 запросов/с. Взамен вход С ЧУЖИХ адресов идёт 100% против 37%;
|
||
размен принят сознательно — офис за одним NAT это единицы адресов,
|
||
а «все остальные» это все;
|
||
- меняется: ботнет или ротация прокси дают злоумышленнику столько ключей,
|
||
сколько ему нужно, и доля на ключ перестаёт быть ограничением.
|
||
То есть это ПОДНИМАЕТ СТОИМОСТЬ атаки (одного адреса больше не хватает,
|
||
чтобы закрыть вход всем), но не закрывает её. Закрывают принципиально
|
||
только доказательство работы на входе или второй фактор — отдельный разговор
|
||
и отдельная цена.
|
||
|
||
Raises:
|
||
PasswordVerifyOverloadedError: очередь на проверку заполнена
|
||
(`login_password_verify_max_inflight`) ЛИБО *key* уже держит свою
|
||
долю (`_per_key_slot_cap`). Отказ мгновенный: ждать нельзя, ждущий
|
||
запрос держит соединение к БД. Оба случая неразличимы снаружи
|
||
намеренно — отказ приходит ДО сверки и потому ничего не сообщает о
|
||
том, существует ли учётка.
|
||
"""
|
||
global _verify_inflight
|
||
|
||
# АВТОРИТЕТНАЯ проверка. Вызывающий может спросить то же самое заранее
|
||
# (`verify_slots_saturated`, #2715), но решение принимается здесь и только
|
||
# здесь — предчек экономит поход в БД, а не заменяет этот отказ.
|
||
if verify_slots_saturated(key):
|
||
raise PasswordVerifyOverloadedError
|
||
|
||
loop = asyncio.get_running_loop()
|
||
_verify_inflight += 1
|
||
_verify_inflight_by_key[key] = _verify_inflight_by_key.get(key, 0) + 1
|
||
try:
|
||
work = _VERIFY_POOL.submit(verify_password, plain, hashed)
|
||
except BaseException:
|
||
# Работа в пул НЕ встала — колбэка не будет, слот отдаём здесь. Иначе
|
||
# утёкший слот навсегда отнимает у входа часть и без того малой ёмкости.
|
||
_release_verify_slot(key)
|
||
raise
|
||
|
||
# Слот освобождает ЗАВЕРШЕНИЕ РАБОТЫ, а не выход из этой корутины. Отмена
|
||
# (клиент отвалился, таймаут) прекращает корутину, но УЖЕ НАЧАТУЮ сверку не
|
||
# снимает — поток занят ею все 282 мс. Отдавай мы слот в `finally`, на это
|
||
# время слот считался бы свободным: одновременно работающих сверок стало бы
|
||
# больше, чем разрешено, и очередь пула поехала бы вслед за ними.
|
||
# (Ещё не начатую работу отмена как раз снимает — `cancel()` пробрасывается
|
||
# на future пула, — так что вреда от неё нет; проблема ровно в начатой.)
|
||
#
|
||
# Именно поэтому колбэк висит на future ПУЛА, а не на обёртке из
|
||
# `run_in_executor`: у обёртки «готово» наступает и при отмене — тест
|
||
# `test_bounded_slot_freed_by_the_work_not_by_cancellation` ловит эту разницу.
|
||
work.add_done_callback(lambda _f: _schedule_verify_slot_release(loop, key))
|
||
return await asyncio.wrap_future(work)
|
||
|
||
|
||
def _schedule_verify_slot_release(loop: asyncio.AbstractEventLoop, key: str) -> None:
|
||
"""Возвращает слот по факту завершения работы в пуле (см. вызывающую).
|
||
|
||
Колбэк future пула исполняется В ПОТОКЕ ПУЛА, а счётчики — собственность
|
||
потока событийного цикла (на том и держится арифметика без лока), поэтому
|
||
декремент переносим в цикл через `call_soon_threadsafe`.
|
||
"""
|
||
try:
|
||
loop.call_soon_threadsafe(_release_verify_slot, key)
|
||
except RuntimeError:
|
||
# Цикл уже закрыт (остановка процесса) — освобождать нечего и некому.
|
||
logger.debug("verify slot release skipped: event loop is closed")
|
||
|
||
|
||
def _release_verify_slot(key: str) -> None:
|
||
"""Единственное место, где слот отдают: и общий счётчик, и счётчик ключа.
|
||
|
||
Оба — одним движением и здесь же, а не по одному на каждом пути выхода:
|
||
разъедься они, и достаточно забыть одну строчку, чтобы ключ навсегда унёс
|
||
с собой долю ёмкости, которую никто уже не вернёт.
|
||
"""
|
||
global _verify_inflight
|
||
_verify_inflight -= 1
|
||
left = _verify_inflight_by_key.get(key, 0) - 1
|
||
if left > 0:
|
||
_verify_inflight_by_key[key] = left
|
||
else:
|
||
_verify_inflight_by_key.pop(key, None)
|