From 736a6c143ec735740721f7cc3fa1d05f901a3574 Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 2 Sep 2026 00:24:41 +0300 Subject: [PATCH] fix(relay): pin the rate-limit source to the proxy-supplied address (#43) User-Visible: no Issue: #43 --- scripts/support-relay/README.md | 23 +++++++++++++++---- .../support-relay/deploy/Caddyfile.fragment | 10 ++++++-- scripts/support-relay/hp_relay/app.py | 8 ++++++- scripts/support-relay/tests/test_relay.py | 18 +++++++++++++++ 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/scripts/support-relay/README.md b/scripts/support-relay/README.md index 2d23d917..2db9bcc2 100644 --- a/scripts/support-relay/README.md +++ b/scripts/support-relay/README.md @@ -36,6 +36,13 @@ сегодняшней даты, ключ живёт сутки. Штатный логгер `BaseHTTPRequestHandler` заменён — он печатал адрес клиента. +Из `X-Forwarded-For` берётся **последний** элемент, а не первый: первый прислал +клиент, и подделать его может кто угодно, а последний проставлен ближайшим +звеном — нашим же Caddy. Caddy при этом настроен перезаписывать заголовок +целиком (`header_up X-Forwarded-For {remote_host}`). Две меры вместо одной +потому, что цена ошибки здесь — обход частотного лимита сменой одной строки в +запросе, и полагаться на умолчания чужого конфига для этого нельзя. + Сообщение и контакт нормализуются и очищаются от управляющих символов, включая маркеры двунаправленного письма: доставка выводит их буквальным текстом без разметки, поэтому подделать вид сообщения нельзя. @@ -93,6 +100,11 @@ sudo systemctl enable --now hp-support-relay-purge@prod.timer hp-support-relay-p недоступным доменом уронит сервис при рестарте, тогда как reload оставит работать прежний. +## Ответственный + +За рубильник, оповещения и ротацию секрета отвечает Sergey Matyunin +(владелец проекта). + ## Runbook **Проверить состояние** @@ -139,10 +151,11 @@ sudo -u hprelay cat /var/lib/hp-support-relay/prod/reports/2026-09/hpr-…/repor cd scripts/support-relay && python3 -m unittest discover -s tests -q ``` -Тридцать проверок: схема, размеры, хеш, идемпотентность, частота, ретеншн, -буквальность текста, отсутствие адреса в журналах, поведение рубильника. -Каждая проверялась отрицательным прогоном — десять мутаций рабочего кода +Тридцать одна проверка: схема, размеры, хеш, идемпотентность, частота, +ретеншн, буквальность текста, отсутствие адреса в журналах, невозможность +выбрать себе корзину лимита подделкой заголовка, поведение рубильника. +Каждая проверялась отрицательным прогоном — одиннадцать мутаций рабочего кода (снять сверку хеша, разрешить лишнюю часть, не чистить управляющие символы, снять лимит, писать адрес в журнал, игнорировать идемпотентность, отключить -рубильник, отключить ретеншн, не проверять секции пакета) роняют ровно те -проверки, ради которых написаны. +рубильник, отключить ретеншн, не проверять секции пакета, брать первый элемент +`X-Forwarded-For`) роняют ровно те проверки, ради которых написаны. diff --git a/scripts/support-relay/deploy/Caddyfile.fragment b/scripts/support-relay/deploy/Caddyfile.fragment index 732ad365..f0b8d640 100644 --- a/scripts/support-relay/deploy/Caddyfile.fragment +++ b/scripts/support-relay/deploy/Caddyfile.fragment @@ -8,12 +8,18 @@ support.houseplan.tech { request_body { max_size 8.7MB } - reverse_proxy 127.0.0.1:8130 + reverse_proxy 127.0.0.1:8130 { + # Заголовок ПЕРЕЗАПИСЫВАЕТСЯ, а не дополняется: иначе клиент задаёт + # первый элемент сам и выбирает себе корзину частотного лимита. + header_up X-Forwarded-For {remote_host} + } } support-staging.houseplan.tech { request_body { max_size 8.7MB } - reverse_proxy 127.0.0.1:8131 + reverse_proxy 127.0.0.1:8131 { + header_up X-Forwarded-For {remote_host} + } } diff --git a/scripts/support-relay/hp_relay/app.py b/scripts/support-relay/hp_relay/app.py index 590eed8a..86a17e8c 100644 --- a/scripts/support-relay/hp_relay/app.py +++ b/scripts/support-relay/hp_relay/app.py @@ -145,7 +145,13 @@ def make_handler(service: Service): if service.cfg.trusted_proxy: forwarded = self.headers.get("X-Forwarded-For", "") if forwarded: - return forwarded.split(",")[0].strip() + # ПОСЛЕДНИЙ элемент, а не первый. Первый — тот, что прислал + # клиент, и подделать его может кто угодно; последний + # проставлен ближайшим звеном, то есть нашим же Caddy. + # Caddy настроен перезаписывать заголовок целиком, но код не + # обязан полагаться на чужой конфиг: цена ошибки здесь — + # обход частотного лимита сменой одной строки в запросе. + return forwarded.split(",")[-1].strip() return self.client_address[0] def do_GET(self) -> None: # noqa: N802 - имя задано базовым классом diff --git a/scripts/support-relay/tests/test_relay.py b/scripts/support-relay/tests/test_relay.py index 7b015ba0..f4d2daa2 100644 --- a/scripts/support-relay/tests/test_relay.py +++ b/scripts/support-relay/tests/test_relay.py @@ -377,6 +377,24 @@ class HttpSurfaceTestCase(unittest.TestCase): self.assertNotIn("127.0.0.1", written) # адрес соединения self.assertNotIn("198.51.100.5", written) # адрес из заголовка + def test_client_cannot_pick_its_own_rate_bucket(self): + """Подделанный X-Forwarded-For не создаёт новый ключ источника. + + Каждый запрос приходит с чужим адресом в заголовке; ключей частоты после + этого должно остаться столько же, сколько при одном источнике, иначе + лимит обходится сменой одной строки в запросе. + """ + blob = package_bytes() + for index, forged in enumerate(("203.0.113.1", "198.51.100.2", "192.0.2.3")): + content_type, body = build_body(request_json(blob, key=f"forged-key-{index:04d}"), blob) + status, _ = self.call("POST", "/v1/reports", body, content_type, + headers={"X-Forwarded-For": f"{forged}, 127.0.0.1"}) + self.assertEqual(status, 200) + spool = self.service.cfg.spool + buckets = [path.name for path in (spool / "rate").glob("*.json") + if path.name != "_global.json"] + self.assertEqual(len(buckets), 1, buckets) + def test_forwarded_for_is_used_as_the_source(self): blob = package_bytes() content_type, body = build_body(request_json(blob), blob)