fix(tradein/auth): доля слотов сверки пароля на адрес — потолок перестаёт бить по своим (#2714) #2717
No reviewers
Labels
No labels
Fable 5 ревью
GG-форсайт
admin
analytics
auth
automation
bug
business
chore
ci
compliance
data
data-moat
docs
duplicate
dx
enhancement
feedback/max
generative
needs-discussion
needs-human
observability
pause-bots
performance
priority/p0
priority/p1
priority/p2
priority/p3
scope/backend
scope/db
scope/devops
scope/frontend
scope/qa
scrapers
security
site-finder
stage/1
stage/2
status/blocked
status/done
status/needs-analysis
status/needs-fix
status/qa
status/ready
status/review
status/wip
tech-debt
tradein
ux
week ревью 1
wontfix
ИРД
вторичка
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: lekss361/gendesign#2717
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "fix/2714-verify-slot-fairness"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Что чинится
Потолок темпа из #2665 держал слоты сверки пароля общим котлом: флуд занимал все четыре, и легитимный вход с верным паролем получал 429.
Замер ДО и ПОСЛЕ (проба с чужого адреса, открытая петля, джиттер 0.2–0.8с, разные живые имена — чтобы мерить потолок сверок, а не
_LOGIN_LIMITER):Потолок сверок больше не отказывает легитимному входу ни разу. Перебор при этом режется как и раньше (флуд получает 429 на всё сверх своей доли).
Как
Ключ (у единственного вызывающего — IP клиента) не берёт больше
max_inflight // 2слотов. Счётчик по ключу живёт в самойverify_password_boundedи отдаётся тем же_release_verify_slot, что и общий, — инвариант «одна точка выноса = одна точка учёта», на котором держится #2712, не делится надвое. Запись словаря удаляется на нуле, поэтому его размер ограничен числом слотов, а не числом виденных адресов.Новой настройки нет сознательно: это доля, а не величина. Подкручивать её нечем — 100% возвращает ровно то поведение, ради отказа от которого правка написана.
Модель угрозы: постановка #2714 верна
RateLimitMiddlewareпропускает 300 запросов за 60с — то есть ровно 5 в секунду бессрочно (в 76-секундном прогоне флуд получил один отказ мидлвари из 383). Сервис-рейт сверок = 1/0.276 = 3.6/с. Пять больше трёх с половиной → очередь переполнена постоянно, а не всплеском. Атакующему всплеск и не нужен: оптимально держаться ровно под бюджетом мидлвари, а плотный флуд самоубивается (20 req/s — первый отказ мидлвари на 15.0с, 100 req/s — на 3.0с).Дефект = ~60% отказов легитимному входу, зато бессрочно, с одного адреса, без единого валидного пароля.
Мой первый замер давал обратное — две ошибки методики, обе в сторону «мягче»
Вывод для протокола: пробу нельзя гнать вплотную, иначе меришь её собственный ритм, а не доступность.
Граница применимости — честно
Ключом может быть только IP, а IP:
_client_ipберёт правый элемент XFF; появится второй — ключ станет клиентским вводом);Правка поднимает стоимость атаки, но не закрывает её. Принципиально закрывают только доказательство работы на входе или второй фактор — отдельный разговор и отдельная цена.
Цена: соседям по NAT стало хуже
Проба с ТОГО ЖЕ адреса, что и флуд, 36 попыток:
Детерминированно: три одновременных входа с одного офисного адреса в окне ~282 мс — третий получает 429 при наполовину пустом пуле (до правки требовалось пять). Порог, за которым сосед перестаёт входить, падает с ~14 до ~7 req/s. Размен положительный (с чужих адресов 37% → 100%) и записан в docstring
verify_password_bounded— не только здесь.Тесты
test_flood_from_one_ip_leaves_login_open_for_another_ip— API-уровень, мерит заявленное: во время флуда с одного адреса вход с другого проходит. На коде до правки красный.test_one_key_cannot_take_more_than_its_share— доля слотов на ключ + явно закреплённая цена по NAT (третий с того же адреса получает 429 при_verify_inflight == 2из 4).test_bounded_frees_slot_when_pool_refuses_work—submitбросил, слот отдан синхронно (единственная строка без покрытия; мутант «старый код» краснит тест и сторож).test_cancelling_queued_work_returns_the_key_slot— отмена ЕЩЁ НЕ НАЧАТОЙ работы (другая ветка future, чем отмена начатой).test_bounded_frees_slot_when_verify_raises— исключение внутри сверки.test_per_key_cap_never_rounds_down_to_zero— полmax(1, …): при очереди в 1 доля не округляется в ноль._no_leaked_password_verify_slots(autouse вtests/conftest.py) — «слотов занято 0» после каждого теста репозитория: счётчик глобальный, аpytest-asyncioдаёт цикл на тест, так что известная ловушкаexcept RuntimeErrorкопилась бы молча и роняла не тот тест.max_inflight == 4,_per_key_slot_cap() == 2), а не арифметику от настройки.Test plan
pytest tests/test_password.py tests/test_auth_api.py— 65 passedtradein-mvp/backend— 3645 passed, 9 skippedRefs #2714, #2665, #2712