diff --git a/.env.example b/.env.example index d80a32c..7f0f8b8 100644 --- a/.env.example +++ b/.env.example @@ -1,16 +1,22 @@ # База метаданных DATABASE_URL=sqlite:///./data/ros_control.db -# Ключ Fernet для шифрования паролей устройств: +# Ключ Fernet для шифрования паролей устройств (обязателен, валидный ключ Fernet): # python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())" SECRET_KEY= -# Секрет подписи cookie-сессии UI -SESSION_SECRET=change-me +# Секрет подписи cookie-сессии UI (обязателен, ≥ 32 символов): +# python -c "import secrets; print(secrets.token_urlsafe(32))" +SESSION_SECRET= +# Secure-флаг cookie сессии (требует HTTPS); включить, когда приложение работает за TLS +SESSION_COOKIE_SECURE=false -# Администратор UI и токен API +# Администратор UI (пароль обязателен, ≥ 12 символов) и токен API (обязателен, ≥ 32 символов): +# python -c "import secrets; print(secrets.token_urlsafe(32))" ADMIN_USER=admin -ADMIN_PASSWORD=change-me -API_TOKEN=change-me +ADMIN_PASSWORD= +API_TOKEN= +# Без заданных выше значений (или со значением change-me/короче требуемой длины) приложение не запускается — +# см. README «Безопасность». # Yandex Object Storage (S3) S3_ENDPOINT=https://storage.yandexcloud.net diff --git a/.gitignore b/.gitignore index ca8e5de..07850a1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ venv/ data/ .env +.env.bak-* __pycache__/ *.pyc .pytest_cache/ diff --git a/README.md b/README.md index 24fa4aa..c09a8eb 100644 --- a/README.md +++ b/README.md @@ -51,19 +51,21 @@ Admin Dashboard ⇄ Control Server ⇄ RouterOS REST (на каждом устр ## Безопасность -- Замените значения по умолчанию `API_TOKEN` и `SESSION_SECRET` на случайные (`openssl rand -hex 24`); порт 8000 не стоит открывать в недоверенную сеть. +- **Секреты обязательны:** `API_TOKEN`, `SESSION_SECRET` (≥ 32 символов) и `ADMIN_PASSWORD` (≥ 12 символов) не могут быть пустыми или равными `change-me`; `SECRET_KEY` должен быть валидным ключом Fernet. С небезопасными значениями приложение не стартует (`RuntimeError` при старте, без секретов в тексте ошибки и в логе) — сгенерировать: `python -c "import secrets; print(secrets.token_urlsafe(32))"`. Порт 8000 не стоит открывать в недоверенную сеть. +- **Вход в UI ограничен по IP клиента**: 5 неверных попыток за 10 минут блокируют IP на 10 минут (в блокировке пароль не проверяется); единственного администратора нельзя заблокировать чужими попытками, т.к. блокировка не привязана к имени пользователя. Ограничение достоверно только пока перед приложением нет reverse-proxy — иначе все запросы приходят с одного IP прокси, и понадобится доверенный `X-Forwarded-For`. +- **Cookie сессии UI**: `SESSION_COOKIE_SECURE=false` по умолчанию (UI по HTTP продолжает работать); включите `true`, когда приложение работает за TLS. - Пароли устройств шифруются ключом `SECRET_KEY` (Fernet); потеря ключа = потеря доступа к сохранённым паролям. - **Секреты в бэкапах:** `.rsc` создаётся с `show-sensitive`, а `.backup` — без шифрования, поэтому файлы содержат пароли и ключи открытым текстом. Ограничьте доступ к бакету и ссылкам скачивания. ## Ограничения -- **Один процесс на БД**: сервер держит файловую блокировку `<файл БД>.lock` рядом с БД (снимается при остановке); второй процесс на той же БД (`--workers 2+`, вторая копия контейнера на том же томе) не стартует — понятная ошибка вместо молчаливой порчи данных. В памяти процесса (не переживает перезапуск и не разделяется между процессами) — семафор фоновых задач, блокировка синхронизации бэкапов, счётчики неудачных попыток очистки журнала и кэш списка бакета. +- **Один процесс на БД**: сервер держит файловую блокировку `<файл БД>.lock` рядом с БД (снимается при остановке); второй процесс на той же БД (`--workers 2+`, вторая копия контейнера на том же томе) не стартует — понятная ошибка вместо молчаливой порчи данных. В памяти процесса (не переживает перезапуск и не разделяется между процессами) — семафор фоновых задач, блокировка синхронизации бэкапов, счётчики неудачных попыток входа в UI и очистки журнала, кэш списка бакета. - **Кэш списка бакета**: страница «Бэкапы» и `GET /api/v1/backups` не перечитывают бакет на каждый просмотр — список живёт `BACKUPS_CACHE_TTL` секунд (по умолчанию 60; `0` — кэш выключен). Изменения бакета, сделанные не через это приложение, видны не позже TTL или сразу — кнопкой «Обновить список» (`refresh=1`). Собственные изменения (бэкап, удаление) сбрасывают кэш сами. ## Запуск ```bash -cp .env.example .env # заполнить SECRET_KEY, ADMIN_PASSWORD, API_TOKEN, S3_* +cp .env.example .env # заполнить SECRET_KEY, SESSION_SECRET, ADMIN_PASSWORD, API_TOKEN, S3_* docker compose up -d --build # UI: http://localhost:8000, OpenAPI: /docs ``` @@ -76,9 +78,10 @@ docker compose up -d --build # UI: http://localhost:8000, OpenAPI: /docs | Переменная | По умолчанию | Назначение | |---|---|---| | `SECRET_KEY` | — | ключ Fernet для паролей устройств (обязателен) | -| `SESSION_SECRET` | `change-me` | подпись cookie-сессии UI | -| `ADMIN_USER` / `ADMIN_PASSWORD` | `admin` / `change-me` | вход в UI | -| `API_TOKEN` | `change-me` | Bearer-токен API | +| `SESSION_SECRET` | — | подпись cookie-сессии UI (обязателен, ≥ 32 символов) | +| `SESSION_COOKIE_SECURE` | `false` | Secure-флаг cookie сессии; включить за TLS | +| `ADMIN_USER` / `ADMIN_PASSWORD` | `admin` / — | вход в UI (`ADMIN_PASSWORD` обязателен, ≥ 12 символов) | +| `API_TOKEN` | — | Bearer-токен API (обязателен, ≥ 32 символов) | | `DATABASE_URL` | `sqlite:///./data/ros_control.db` | база метаданных (в compose — том) | | `S3_ENDPOINT`, `S3_REGION`, `S3_BUCKET`, `S3_ACCESS_KEY`, `S3_SECRET_KEY`, `S3_PREFIX` | Yandex Object Storage, `backups` | бакет для резервных копий | | `BACKUPS_CACHE_TTL` | `60` | кэш списка бакета, с; `0` — выключить (см. «Ограничения») | @@ -95,7 +98,7 @@ docker compose up -d --build # UI: http://localhost:8000, OpenAPI: /docs python3 -m venv venv && ./venv/bin/pip install -r requirements.txt set -a; . ./.env; set +a ./venv/bin/uvicorn app.main:app --reload -./venv/bin/python -m pytest # 24 теста, фоновый опрос в тестах выключен +./venv/bin/python -m pytest # 28 тестов, фоновый опрос в тестах выключен ``` ## API v1 @@ -162,3 +165,4 @@ curl -s -H "Authorization: Bearer $API_TOKEN" http://localhost:8000/api/v1/devic - `018-correctness-consistency` — одиночное удаление бэкапа в UI через общий сервис `delete_many`, единая система миграций (колонки старой схемы — в `migrations.run` до миграции ID), групповая смена канала фоновыми задачами (`set_channel`). - `019-performance-scaling` — кэш списка бакета (`BACKUPS_CACHE_TTL`, «Обновить список»); обработчики без обращений к event loop — обычные функции (пул потоков FastAPI), запись статуса и тяжёлые операции с БД в фоне — через `asyncio.to_thread`; SQLite — WAL и `busy_timeout`; файловая блокировка БД — один процесс на БД. - `020-custom-select-menus` — выпадающие списки (фильтры, «Группа») в стиле меню действий «⋯»: прогрессивное улучшение в JS, нативный `select` остаётся в разметке и работает без JavaScript. +- `021-security-hardening` — отказ старта при небезопасных секретах (`API_TOKEN`/`SESSION_SECRET`/`ADMIN_PASSWORD`/`SECRET_KEY`), блокировка входа в UI по IP клиента, `SESSION_COOKIE_SECURE`, безопасный `next` в редиректах (`/ui/move`, `/backups/delete-many`). diff --git a/app/config.py b/app/config.py index 26eab19..9965e7d 100644 --- a/app/config.py +++ b/app/config.py @@ -1,7 +1,13 @@ from functools import lru_cache +from cryptography.fernet import Fernet from pydantic_settings import BaseSettings, SettingsConfigDict +# минимальная длина секретов, принимаемая при старте (см. insecure_settings) +API_TOKEN_MIN_LEN = 32 +SESSION_SECRET_MIN_LEN = 32 +ADMIN_PASSWORD_MIN_LEN = 12 + class Settings(BaseSettings): model_config = SettingsConfigDict(env_file=".env", extra="ignore") @@ -9,6 +15,7 @@ class Settings(BaseSettings): database_url: str = "sqlite:///./data/ros_control.db" secret_key: str = "" # ключ Fernet для паролей устройств session_secret: str = "change-me" + session_cookie_secure: bool = False # Secure-флаг cookie сессии; включить за TLS admin_user: str = "admin" admin_password: str = "change-me" @@ -37,3 +44,28 @@ class Settings(BaseSettings): @lru_cache def get_settings() -> Settings: return Settings() + + +def insecure_settings(s: Settings) -> list[str]: + """Проблемы конфигурации, с которыми приложению нельзя стартовать. Без значений секретов — только названия проблем.""" + problems = [] + if not s.api_token or s.api_token == "change-me": + problems.append("API_TOKEN: пустой или change-me") + elif len(s.api_token) < API_TOKEN_MIN_LEN: + problems.append(f"API_TOKEN: короче {API_TOKEN_MIN_LEN} символов") + if not s.session_secret or s.session_secret == "change-me": + problems.append("SESSION_SECRET: пустой или change-me") + elif len(s.session_secret) < SESSION_SECRET_MIN_LEN: + problems.append(f"SESSION_SECRET: короче {SESSION_SECRET_MIN_LEN} символов") + if not s.admin_password or s.admin_password == "change-me": + problems.append("ADMIN_PASSWORD: пустой или change-me") + elif len(s.admin_password) < ADMIN_PASSWORD_MIN_LEN: + problems.append(f"ADMIN_PASSWORD: короче {ADMIN_PASSWORD_MIN_LEN} символов") + if not s.secret_key: + problems.append("SECRET_KEY: пустой") + else: + try: + Fernet(s.secret_key.encode()) + except Exception: # noqa: BLE001 + problems.append("SECRET_KEY: не ключ Fernet") + return problems diff --git a/app/main.py b/app/main.py index 25d3e45..43b2d1f 100644 --- a/app/main.py +++ b/app/main.py @@ -10,7 +10,7 @@ from starlette.middleware.sessions import SessionMiddleware from app import process_lock from app.api import v1 -from app.config import get_settings +from app.config import get_settings, insecure_settings from app.db import init_db from app.services import backups, jobs, poller, rotation from app.ui import routes as ui @@ -18,6 +18,9 @@ from app.ui import routes as ui @asynccontextmanager async def lifespan(app: FastAPI): + problems = insecure_settings(get_settings()) + if problems: + raise RuntimeError("Небезопасная конфигурация: " + "; ".join(problems) + "; см. README «Настройки»") process_lock.acquire(get_settings().database_url) # один процесс на файловую БД (--workers 1) try: init_db() @@ -39,7 +42,7 @@ def create_app() -> FastAPI: app = FastAPI(title="ros_control", lifespan=lifespan) app.add_middleware( SessionMiddleware, secret_key=get_settings().session_secret, - same_site="strict", https_only=False, + same_site="strict", https_only=get_settings().session_cookie_secure, ) app.mount("/static", StaticFiles(directory=Path(__file__).parent / "ui" / "static"), name="static") app.include_router(v1.router) diff --git a/app/security.py b/app/security.py index 17cde2e..1bd7f73 100644 --- a/app/security.py +++ b/app/security.py @@ -33,7 +33,7 @@ def check_admin(user: str, password: str) -> bool: return ok_user and ok_pass -# --- подтверждение действий паролем (очистка журнала) и защита от перебора --- +# --- защита от перебора: вход в UI (по IP) и подтверждение действий паролем (очистка журнала, по пользователю) --- FAIL_LIMIT, WINDOW_S, LOCK_S = 5, 600, 600 # 5 неверных за 10 минут → блокировка на 10 минут _guard = threading.Lock() _fails: dict[str, list[float]] = {} @@ -47,29 +47,32 @@ def verify_password(user: str, password: str) -> bool: and hmac.compare_digest(password.encode(), s.admin_password.encode())) -def lockout_remaining(user: str) -> int: - """Сколько секунд осталось до конца блокировки (0 — не заблокирован).""" +def lockout_remaining(key: str) -> int: + """Сколько секунд осталось до конца блокировки по ключу (0 — не заблокирован). + + Ключ — своё пространство на каждый вид перебора (например, `f"login:{ip}"` для входа в UI + и имя пользователя для очистки журнала), чтобы счётчики не пересекались.""" with _guard: - return max(0, int(_locked_until.get(user, 0) - time.monotonic() + 0.999)) + return max(0, int(_locked_until.get(key, 0) - time.monotonic() + 0.999)) -def register_failure(user: str) -> int: - """Учитывает неверный пароль; возвращает число оставшихся попыток (0 — пользователь заблокирован).""" +def register_failure(key: str) -> int: + """Учитывает неверную попытку по ключу; возвращает число оставшихся попыток (0 — ключ заблокирован).""" now = time.monotonic() with _guard: - recent = [t for t in _fails.get(user, []) if now - t < WINDOW_S] + [now] - _fails[user] = recent + recent = [t for t in _fails.get(key, []) if now - t < WINDOW_S] + [now] + _fails[key] = recent if len(recent) >= FAIL_LIMIT: - _locked_until[user] = now + LOCK_S - _fails[user] = [] + _locked_until[key] = now + LOCK_S + _fails[key] = [] return 0 return FAIL_LIMIT - len(recent) -def reset_failures(user: str) -> None: +def reset_failures(key: str) -> None: with _guard: - _fails.pop(user, None) - _locked_until.pop(user, None) + _fails.pop(key, None) + _locked_until.pop(key, None) async def require_api_token(cred: HTTPAuthorizationCredentials | None = Depends(_bearer)) -> None: diff --git a/app/ui/routes.py b/app/ui/routes.py index 52c44cf..36fa631 100644 --- a/app/ui/routes.py +++ b/app/ui/routes.py @@ -2,7 +2,7 @@ import json from datetime import date from pathlib import Path -from urllib.parse import urlencode +from urllib.parse import urlencode, urlsplit from fastapi import APIRouter, Depends, Form, Request, Response from fastapi.responses import HTMLResponse, RedirectResponse @@ -34,7 +34,8 @@ templates.env.filters["num"] = lambda n: f"{int(n):,}".replace(",", "\u00a0") templates.env.filters["dts"] = lambda v: v.strftime("%Y-%m-%d %H:%M:%S") if v else "—" # смысловая окраска типов событий (как в утверждённом макете) -_EV_KIND = {"bad": ("device.offline", "job.failed", "auth.failed", "backup.failed", "journal.clear_denied", "journal.clear_locked"), +_EV_KIND = {"bad": ("device.offline", "job.failed", "auth.failed", "auth.locked", "backup.failed", "journal.clear_denied", + "journal.clear_locked"), "ok": ("device.online", "job.done", "backup.done", "auth.login"), "run": ("job.started",), "warn": ("backup.deleted", "device.deleted", "group.deleted", "system.migrated", "journal.cleared", "journal.rotated")} @@ -61,8 +62,8 @@ async def require_login(request: Request) -> None: router = APIRouter(include_in_schema=False) -def _render(request: Request, name: str, **ctx): - return templates.TemplateResponse(request, name, ctx) +def _render(request: Request, name: str, status_code: int = 200, **ctx): + return templates.TemplateResponse(request, name, ctx, status_code=status_code) def _is_htmx(request: Request) -> bool: @@ -137,13 +138,31 @@ def login_form(request: Request): @router.post("/login") def login(request: Request, username: str = Form(), password: str = Form()): + """Вход в UI: попытки ограничены по IP клиента (не по имени — иначе одного администратора + можно заблокировать чужими неверными попытками). При блокировке пароль не проверяется.""" + ip = request.client.host if request.client else "unknown" + key = f"login:{ip}" + events.set_actor("anonymous") + lock = security.lockout_remaining(key) + if lock: + # событие auth.locked уже записано в момент начала блокировки (ветка left == 0 ниже); + # повторные попытки во время блокировки в журнал не пишем — иначе перебор засыпает его и вытесняет историю ротацией + minutes = max(1, -(-lock // 60)) + return _render(request, "login.html", status_code=429, + error=f"Слишком много попыток входа. Повторите через {minutes} мин.") if not security.check_admin(username, password): - events.set_actor("anonymous") - events.record("auth.failed", "session", None, "Неудачная попытка входа в UI", data={"user": username[:64]}) - return _render(request, "login.html", error="Неверный логин или пароль") + left = security.register_failure(key) + if left == 0: + events.record("auth.locked", "session", None, "Вход заблокирован: превышено число попыток входа", + data={"ip": ip, "user": username[:64]}) + else: + events.record("auth.failed", "session", None, "Неудачная попытка входа в UI", + data={"ip": ip, "user": username[:64], "attempts_left": left}) + return _render(request, "login.html", status_code=401, error="Неверный логин или пароль") + security.reset_failures(key) request.session["user"] = username events.set_actor(f"ui:{username}") - events.record("auth.login", "session", None, f"Вход в UI: {username}", data={"user": username}) + events.record("auth.login", "session", None, f"Вход в UI: {username}", data={"user": username, "ip": ip}) return RedirectResponse("/", status_code=303) @@ -213,12 +232,23 @@ async def device_action(request: Request, device_id: str, action: str, channel: return _render(request, "_devices.html", oob=True, **_devices_ctx(_flt(await request.form()))) +def _safe_next(value: str, default: str = "/", prefix: str = "/") -> str: + """Адрес для редиректа после формы, присланный пользователем (`next`/`back`): только локальный путь, + иначе открытый редирект (в т.ч. `\\evil.com`, который браузеры трактуют как `//evil.com`).""" + value = value or "" + if (not value.startswith(prefix) or value.startswith("//") + or any(ord(ch) < 0x20 or ch in ("\\", "\x7f") for ch in value)): + return default + parts = urlsplit(value) + return value if not parts.scheme and not parts.netloc else default + + @router.post("/ui/move", dependencies=[Depends(require_login)]) def move(device_ids: list[str] = Form(default=[]), group_id: str = Form(""), next: str = Form("/")): """Перенос отмеченных устройств в группу (обычный POST + редирект: счётчики вкладок обновляются).""" if device_ids: groups.move_devices(device_ids, _opt_id(group_id, "grp")) - return RedirectResponse(next if next.startswith("/") and not next.startswith("//") else "/", status_code=303) + return RedirectResponse(_safe_next(next), status_code=303) # --- окна (dialog) и формы: одна разметка для окна и запасной страницы --- @@ -402,7 +432,7 @@ async def backups_page(request: Request, device: str = "", group: str = "", kind async def backups_delete_many(key: list[str] = Form(default=[]), next: str = Form("/backups")): """Групповое удаление выбранных файлов; возврат на страницу с теми же фильтрами и итогом.""" deleted, failed = await backups.delete_many(key) - back = next if next.startswith("/backups") else "/backups" + back = _safe_next(next, default="/backups", prefix="/backups") return RedirectResponse(back + ("&" if "?" in back else "?") + urlencode({"deleted": deleted, "failed": failed}), status_code=303) diff --git a/docs/changes/021-security-hardening/plan.md b/docs/changes/021-security-hardening/plan.md new file mode 100644 index 0000000..2a9cd64 --- /dev/null +++ b/docs/changes/021-security-hardening/plan.md @@ -0,0 +1,87 @@ +# План: 021 — безопасность (по ревью 2026-09-27) + +## Context + +Ревью `docs/reviews/2026-09-27-codebase-review.md`, раздел «Безопасность», пункты 1–4: + +- **п. 1** — `API_TOKEN`, `ADMIN_PASSWORD`, `SESSION_SECRET` по умолчанию `change-me` (`app/config.py`) и при старте не проверяются. + На стенде так и было: `API_TOKEN=change-me` — API был открыт с общеизвестным токеном. +- **п. 2** — вход в UI (`POST /login`, `app/ui/routes.py::login`) без ограничения попыток: пароль администратора можно перебирать. +- **п. 3** — сессионная cookie без флага Secure (`SessionMiddleware(..., https_only=False)` в `app/main.py`). +- **п. 4** — `POST /ui/move`: проверка `next` (`startswith("/") and not startswith("//")`) пропускает `/\evil.com`, + которое браузеры трактуют как `//evil.com` — открытый редирект. + +Решения пользователя: +- п. 1 — при старте приложение **не запускается**, если `API_TOKEN`, `ADMIN_PASSWORD`, `SESSION_SECRET` пустые или `change-me`, + а также при недостаточной длине: `API_TOKEN` и `SESSION_SECRET` ≥ 32 символов, `ADMIN_PASSWORD` ≥ 12. +- `.env` стенда — оркестратор уже сгенерировал новые `API_TOKEN` (43), `SESSION_SECRET` (64), `ADMIN_PASSWORD` (20); + прежний файл — `.env.bak-20260928` (добавлен в `.gitignore` шаблоном `.env.bak-*`). Сессии UI и внешние скрипты со старым токеном перестанут работать. +- п. 2 — блокировка **по IP клиента**: 5 неверных попыток за 10 минут → IP заблокирован на 10 минут. Администратора (единственного) нельзя + заблокировать чужими попытками. Reverse-proxy сейчас нет — `request.client.host` достоверен (ограничение описать в README). +- п. 3 — настройка `SESSION_COOKIE_SECURE`, **по умолчанию `false`** (UI по http продолжает работать); README — включить за TLS. + +## Изменения + +### п. 1 — проверка секретов при старте (`app/config.py`, `app/main.py`, `.env.example`, `tests/conftest.py`) +- `config.py`: `def insecure_settings(s: Settings) -> list[str]` — список проблем **без значений секретов** (например, + «API_TOKEN: пустой или change-me», «SESSION_SECRET: короче 32 символов», «SECRET_KEY: не ключ Fernet»). + Правила: `API_TOKEN`, `SESSION_SECRET` — не пустые, не `change-me`, длина ≥ 32; `ADMIN_PASSWORD` — не пустой, не `change-me`, длина ≥ 12; + `SECRET_KEY` — не пустой и валидный ключ Fernet (`Fernet(key)` без исключения) — сейчас пустой ключ всплывает только при первом шифровании. + Константы минимальной длины — рядом с правилами. +- `main.py::lifespan`: **первым шагом** (до `process_lock.acquire`) — если список не пуст, `RuntimeError("Небезопасная конфигурация: …; см. README «Настройки»")`, + приложение не стартует. `create_app()` проверку не делает (модуль импортируется в тестах). +- `.env.example`: у секретов — пустые значения вместо `change-me` и комментарии с требованиями и командой генерации + (`python -c "import secrets; print(secrets.token_urlsafe(32))"`). +- `tests/conftest.py`: `API_TOKEN`, `ADMIN_PASSWORD` — значения, проходящие проверку (≥ 32 / ≥ 12); обновить тесты, + где эти значения захардкожены (`"pw"`, `"test-token"`) — через общие константы в conftest, а не копипастой. + +### п. 2 — ограничение попыток входа по IP (`app/security.py`, `app/ui/routes.py`, `app/ui/templates/login.html`) +- **Переиспользовать** счётчики `security.lockout_remaining / register_failure / reset_failures` (5 за 10 минут → блок 10 минут). + Они сейчас ключуются именем пользователя (очистка журнала) — параметр переименовать в `key`, для входа ключ `f"login:{ip}"`, + для очистки журнала — прежний (имя пользователя), чтобы пространства ключей не пересекались. Поведение очистки журнала не меняется. +- `login`: `ip = request.client.host if request.client else "unknown"`. + 1) если `lockout_remaining(key) > 0` — пароль **не проверяется**, форма с ошибкой «Слишком много попыток входа. Повторите через N мин.», + код 429, событие `auth.locked` (IP, введённый логин ≤ 64 символов); + 2) неверные данные — `register_failure(key)`; при 0 оставшихся — событие `auth.locked`, иначе `auth.failed` (в `data` — IP и число оставшихся попыток); + ответ, как сейчас (ошибка «Неверный логин или пароль», код 200 → **401**); + 3) успех — `reset_failures(key)`, событие `auth.login` (в `data` добавить IP). +- `_EV_KIND` в `routes.py`: `auth.locked` — в группу `bad`. +- Пароли и секреты в журнал не пишутся (как сейчас). + +### п. 3 — Secure-cookie (`app/config.py`, `app/main.py`, `.env.example`, README) +- `Settings.session_cookie_secure: bool = False`; `SessionMiddleware(..., https_only=get_settings().session_cookie_secure)`. + +### п. 4 — безопасный `next` (`app/ui/routes.py`) +- Хелпер `_safe_next(value: str, default: str = "/", prefix: str = "/") -> str`: допускается только локальный путь — + начинается с `prefix`, не начинается с `//`, не содержит `\` и управляющих символов (`ord < 32`, `\x7f`), + `urllib.parse.urlsplit(value)` без `scheme` и `netloc`; иначе `default`. +- Применить в `move` (`prefix="/"`) и в `backups_delete_many` (`prefix="/backups"`, вместо текущего `startswith("/backups")`). +- Найти (grep `RedirectResponse(` и параметры `next`/`back`) другие редиректы с пользовательским вводом и применить тот же хелпер. + +## Тесты (минимально, `tests/test_app.py`) +- п. 1: `insecure_settings` для `change-me`, коротких значений и невалидного `SECRET_KEY` возвращает проблемы и **не содержит значений секретов**; + запуск `TestClient(create_app())` с `API_TOKEN=change-me` падает с `RuntimeError`. +- п. 2: 5 неверных входов → 6-й (даже с верным паролем) → 429 и событие `auth.locked`; с другого IP (TestClient `client=("10.0.0.2", 123)`) + вход работает; после успешного входа счётчик сброшен. +- п. 4: `POST /ui/move` с `next` = `/\evil.com`, `//evil.com`, `https://evil.com`, `/%0d%0aX` → `Location: /`; `next=/?f_group=none` сохраняется. +- п. 3 — без отдельного теста (одна строка конфигурации). + +## Документация +README: «Безопасность» (требования к секретам и отказ старта, блокировка входа по IP и ограничение за reverse-proxy, `SESSION_COOKIE_SECURE`), +«Настройки» (новые требования, `SESSION_COOKIE_SECURE`), число тестов, строка 021 в истории изменений. `summary.md` — оркестратор. + +## Исполнение +Исполнитель (Sonnet): код, тесты, README, `.env.example`, пересборка стенда. `.env` стенда **не трогает** (уже обновлён оркестратором) и не выводит. +Тесты не запускает, не коммитит. + +## Проверка +- `pytest` — все зелёные. +- Стенд (override 8001, `--force-recreate`): стартует с новым `.env`; `/login` → 200; новый код в контейнере. +- Отказ старта: временный контейнер из того же образа **без тома** с `API_TOKEN=change-me` → `RuntimeError`, в логе нет значений секретов. +- API: старый токен `change-me` → 401, новый → 200. +- Вход: 5 неверных паролей с одного адреса → 6-я попытка 429 даже с верным паролем; события `auth.failed` ×4, `auth.locked`; верный пароль с + другого адреса — вход (стенд без прокси: второй адрес — запрос изнутри контейнера или с другого интерфейса хоста). + После проверки — блокировка в памяти снимается перезапуском контейнера (данные не затрагиваются). +- Редирект: `POST /ui/move` с `next=/\evil.com` → `Location: /`. +- Боевые данные: сверка по ID до/после (согласованная копия через `sqlite3 backup`). +- Ручная проверка UI — пользователь: вход с новым паролем из `.env`, сообщение о блокировке после 5 ошибок. diff --git a/docs/changes/021-security-hardening/summary.md b/docs/changes/021-security-hardening/summary.md new file mode 100644 index 0000000..f9b73ec --- /dev/null +++ b/docs/changes/021-security-hardening/summary.md @@ -0,0 +1,35 @@ +# Итоги: 021 — безопасность (по ревью 2026-09-27) + +Источник — ревью `docs/reviews/2026-09-27-codebase-review.md`, раздел «Безопасность» (пункты 1–4). + +## Сделано +- **п. 1 — проверка секретов при старте** (`app/config.py::insecure_settings`, `app/main.py::lifespan` — первым шагом): + `API_TOKEN`, `SESSION_SECRET` — не пустые, не `change-me`, ≥ 32 символов; `ADMIN_PASSWORD` — ≥ 12; `SECRET_KEY` — валидный ключ Fernet. + Иначе `RuntimeError` со списком проблем без значений секретов, приложение не стартует. `.env.example` — пустые секреты с требованиями и командой генерации. +- **п. 2 — ограничение попыток входа по IP** (`app/ui/routes.py::login`): 5 неверных за 10 минут → IP заблокирован на 10 минут, пароль не проверяется, 429. + Неверный пароль — 401 (было 200). Счётчики `security.lockout_remaining/register_failure/reset_failures` (параметр `user` → `key`), ключ `login:` + не пересекается с ключом очистки журнала. События: `auth.failed` (IP, оставшиеся попытки), `auth.locked` — **один раз** при начале блокировки, `auth.login` с IP. +- **п. 3 — Secure-cookie**: `SESSION_COOKIE_SECURE` (по умолчанию `false`), README — включить за TLS. +- **п. 4 — безопасный `next`** (`_safe_next`): только локальный путь с нужным префиксом; `//`, `\`, управляющие символы, схема или хост → адрес по умолчанию. + Применён в `/ui/move` и групповом удалении бэкапов. +- `.env` стенда: сгенерированы новые `API_TOKEN` (43), `SESSION_SECRET` (64), `ADMIN_PASSWORD` (20); прежний — `.env.bak-20260928`; + `.gitignore` — шаблон `.env.bak-*`. README: «Безопасность», «Настройки», «Ограничения», история изменений. + +## Найдено на ревью и исправлено +- Каждая попытка входа с заблокированного IP писала `auth.locked` — перебором можно было засыпать журнал и вытеснить историю ротацией. Теперь событие пишется один раз. +- Тест блокировки проверял 303 без `follow_redirects=False` и падал. +- Найдено до реализации: на стенде `API_TOKEN` был `change-me` (API открыт с общеизвестным токеном), `SESSION_SECRET` — 10 символов, `ADMIN_PASSWORD` — 8. + +## Проверено +- `pytest`: 28 из 28 (новые: слабые секреты без утечки значений, отказ старта, блокировка входа по IP, открытый редирект). +- Отказ старта: временный контейнер без тома с `API_TOKEN=change-me` → `RuntimeError`; значений секретов в логе нет. +- API: `change-me` → 401, новый токен → 200. +- Вход: 5 неверных → 401 ×5; 6-я с верным паролем и ещё 3 → 429; с 127.0.0.1 (изнутри контейнера) → 303; события: `auth.failed` ×4, `auth.locked` ×1. +- Редирект: `/\evil.com`, `//evil.com`, `https://evil.com` → `/`; `/?f_group=none` сохраняется. +- Боевые данные: группы 4/4, устройства 14/14, бэкапы 47/47, задачи 73/73 — совпадают по ID; добавлено 7 событий входа проверки. Блокировка снята перезапуском контейнера. + +## Оговорки +- **Ломающее для эксплуатации**: новые пароль администратора и токен API (в `.env`); старые сессии UI и скрипты со старым токеном не работают. +- Блокировка входа — в памяти процесса (сбрасывается перезапуском). За reverse-proxy все клиенты видны с IP прокси — нужна настройка доверенных прокси. +- API-токен не ограничен по попыткам (≥ 32 случайных символа — перебор непрактичен). +- Ручная проверка UI пользователем на момент коммита не подтверждена. diff --git a/tests/conftest.py b/tests/conftest.py index 03f7fa8..9cdd387 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,19 +5,26 @@ from app import db, security from app.config import get_settings from app.services import backups +# секреты для тестов: значения, проходящие проверку insecure_settings (см. app/config.py), но не боевые +API_TOKEN = "test-api-token-0123456789abcdef0" # 32 символа +ADMIN_PASSWORD = "test-admin-pass123" # 12+ символов +SESSION_SECRET = "test-session-secret-0123456789ab" # 32 символа + @pytest.fixture(autouse=True) def env(tmp_path, monkeypatch): monkeypatch.setenv("DATABASE_URL", f"sqlite:///{tmp_path}/test.db") monkeypatch.setenv("SECRET_KEY", Fernet.generate_key().decode()) - monkeypatch.setenv("API_TOKEN", "test-token") + monkeypatch.setenv("SESSION_SECRET", SESSION_SECRET) + monkeypatch.setenv("API_TOKEN", API_TOKEN) monkeypatch.setenv("S3_BUCKET", "test-bucket") monkeypatch.setenv("S3_ACCESS_KEY", "") # без ключей S3: сверка бакета при старте отключена monkeypatch.setenv("POLL_INTERVAL", "0") # фоновый опрос в тестах выключен monkeypatch.setenv("ADMIN_USER", "admin") # не зависеть от реального .env - monkeypatch.setenv("ADMIN_PASSWORD", "pw") + monkeypatch.setenv("ADMIN_PASSWORD", ADMIN_PASSWORD) get_settings.cache_clear() security.reset_failures("admin") # блокировки очистки журнала — в памяти процесса + security.reset_failures("login:testclient") # блокировки входа — в памяти процесса (IP TestClient по умолчанию) backups.invalidate() # кэш списка бакета — модульное состояние, не должен переживать тест db.init_db() yield diff --git a/tests/test_app.py b/tests/test_app.py index d018f2b..33b11ad 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -7,15 +7,18 @@ from datetime import date, datetime, timedelta, timezone import httpx import pytest +from cryptography.fernet import Fernet from fastapi.testclient import TestClient from app import db, ids, process_lock, s3, security +from app.config import Settings, get_settings, insecure_settings from app.main import create_app from app.models import Backup, Device, Event, now from app.db import session_scope from app.ros import operations as ros from app.ros.client import RosClient from app.services import backups, devices, events, groups, jobs, ops, settings +from tests.conftest import ADMIN_PASSWORD, API_TOKEN def ros_client(handler) -> RosClient: @@ -88,7 +91,7 @@ async def test_backup_flow(monkeypatch): def test_api_auth_and_no_password_leak(): with TestClient(create_app()) as client: assert client.get("/api/v1/devices").status_code == 401 - h = {"Authorization": "Bearer test-token"} + h = {"Authorization": f"Bearer {API_TOKEN}"} r = client.post("/api/v1/devices", headers=h, json={ "name": "r1", "host": "10.0.0.1", "username": "admin", "password": "pw"}) assert r.status_code == 201 and "password" not in r.text @@ -185,7 +188,7 @@ def test_create_device_via_ui_binds_group(monkeypatch): form = dict(host="10.0.0.1", port="80", username="u", password="p") htmx = {"HX-Request": "true"} with TestClient(create_app()) as c: - c.post("/login", data={"username": "admin", "password": "pw"}) + c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}) # 1) существующая группа r = c.post("/devices/new", data={**form, "name": "a", "group_id": str(office.id), "note": " Серверная, 2 этаж "}, follow_redirects=False) @@ -256,7 +259,7 @@ def test_bulk_delete_backups(monkeypatch): monkeypatch.setattr(s3, "list_backups", empty_list) ok = ["backups/a/1.backup", "backups/a/1.rsc"] with TestClient(create_app()) as c: - c.post("/login", data={"username": "admin", "password": "pw"}) + c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}) # UI: ключ вне префикса бэкапов -> ошибка, ничего не удалено r = c.post("/backups/delete-many", data={"key": ok + ["other/secret.txt"]}, follow_redirects=False) assert r.status_code == 400 and removed == [] @@ -266,7 +269,7 @@ def test_bulk_delete_backups(monkeypatch): assert sorted(removed) == ok # API removed.clear() - h = {"Authorization": "Bearer test-token"} + h = {"Authorization": f"Bearer {API_TOKEN}"} assert c.post("/api/v1/backups/delete", headers=h, json={"keys": ok}).json() == {"deleted": 2, "failed": 0} assert c.post("/api/v1/backups/delete", headers=h, json={"keys": ["x/../y"]}).status_code == 400 # UI: одиночное удаление — тот же сервис delete_many (метаданные и событие backup.deleted не теряются) @@ -299,8 +302,8 @@ async def test_chr_status_and_version_compare(): def test_device_name_is_immutable(): """Имя задаётся только при создании: API отклоняет смену, форма изменения имя игнорирует.""" with TestClient(create_app()) as c: - c.post("/login", data={"username": "admin", "password": "pw"}) - h = {"Authorization": "Bearer test-token"} + c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}) + h = {"Authorization": f"Bearer {API_TOKEN}"} d = c.post("/api/v1/devices", headers=h, json={"name": "r1", "host": "10.0.0.1", "username": "u", "password": "p"}).json() r = c.patch(f"/api/v1/devices/{d['id']}", headers=h, json={"name": "r2"}) assert r.status_code == 400 and "нельзя изменить" in r.text @@ -434,7 +437,7 @@ async def test_batch_channel_runs_as_jobs(monkeypatch): d2 = devices.create_device("r2", "10.0.0.2", 443, "admin", "pw") with TestClient(create_app()) as c: - h = {"Authorization": "Bearer test-token"} + h = {"Authorization": f"Bearer {API_TOKEN}"} r = c.put("/api/v1/batch/channel", headers=h, json={"device_ids": [d1.id, d2.id], "channel": "testing"}) assert r.status_code == 202 job_ids = r.json()["job_ids"] @@ -490,9 +493,9 @@ async def test_events_link_entities(monkeypatch): def test_events_api_and_id_validation(): """API отдаёт журнал с ID; ID чужого типа и числовые ID дают 404; вход в UI попадает в журнал.""" with TestClient(create_app()) as c: - h = {"Authorization": "Bearer test-token"} + h = {"Authorization": f"Bearer {API_TOKEN}"} c.post("/login", data={"username": "admin", "password": "wrong"}) - c.post("/login", data={"username": "admin", "password": "pw"}) + c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}) d = c.post("/api/v1/devices", headers=h, json={"name": "r1", "host": "10.0.0.1", "username": "u", "password": "p"}).json() assert ids.is_id(d["id"], "dev") evs = c.get("/api/v1/events", headers=h, params={"entity_id": d["id"]}).json() @@ -500,7 +503,7 @@ def test_events_api_and_id_validation(): assert c.get(f"/api/v1/events/{evs[0]['id']}", headers=h).json()["entity_id"] == d["id"] auth = c.get("/api/v1/events", headers=h, params={"type": "auth"}).json() assert sorted(e["type"] for e in auth) == ["auth.failed", "auth.login"] - assert "pw" not in json.dumps(auth) # пароли в журнал не попадают + assert ADMIN_PASSWORD not in json.dumps(auth) # пароли в журнал не попадают assert c.get("/api/v1/devices/1", headers=h).status_code == 404 assert c.get(f"/api/v1/devices/{ids.new_id('job')}", headers=h).status_code == 404 assert c.get(f"/api/v1/jobs/{d['id']}", headers=h).status_code == 404 @@ -567,14 +570,14 @@ def test_journal_clear_requires_password_through_modal(): успех оставляет одну запись об очистке; в API очистки нет.""" htmx = {"HX-Request": "true"} with TestClient(create_app()) as c: - h = {"Authorization": "Bearer test-token"} - c.post("/login", data={"username": "admin", "password": "pw"}) + h = {"Authorization": f"Bearer {API_TOKEN}"} + c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}) _fill_events(30) before = events.count() page = c.get("/ui/dialog/events-clear", headers=htmx).text assert "Очистить журнал" in page and 'id="clear-confirm" disabled' in page # кнопка неактивна без пароля - assert c.post("/events/clear", data={"password": "pw"}).status_code == 400 # не через окно — отклонено + assert c.post("/events/clear", data={"password": ADMIN_PASSWORD}).status_code == 400 # не через окно — отклонено assert events.count() == before r = c.post("/events/clear", data={"password": ""}, headers=htmx) assert "Введите пароль" in r.text and events.count() == before @@ -583,29 +586,29 @@ def test_journal_clear_requires_password_through_modal(): assert f"Осталось попыток: {left}" in r.text and "HX-Refresh" not in r.headers r = c.post("/events/clear", data={"password": "wrong"}, headers=htmx) # 5-я неверная — блокировка assert "Слишком много неверных попыток" in r.text - r = c.post("/events/clear", data={"password": "pw"}, headers=htmx) # верный пароль при блокировке не принимается + r = c.post("/events/clear", data={"password": ADMIN_PASSWORD}, headers=htmx) # верный пароль при блокировке не принимается assert "Повторите через" in r.text and "HX-Refresh" not in r.headers denied = events.list_events(type_="journal.clear_denied") assert len(denied) == 5 and "wrong" not in json.dumps([e.data for e in denied]) # пароль в журнал не пишется assert events.list_events(type_="journal.clear_locked") security.reset_failures("admin") # окончание блокировки - r = c.post("/events/clear", data={"password": "pw"}, headers=htmx) + r = c.post("/events/clear", data={"password": ADMIN_PASSWORD}, headers=htmx) assert r.headers["HX-Refresh"] == "true" left = events.list_events() assert [e.type for e in left] == ["journal.cleared"] and left[0].actor == "ui:admin" assert json.loads(left[0].data)["deleted"] > 30 assert c.delete("/api/v1/events", headers=h).status_code in (404, 405) # очистки через API нет - assert c.post("/api/v1/events/clear", headers=h, json={"password": "pw"}).status_code in (404, 405, 422) + assert c.post("/api/v1/events/clear", headers=h, json={"password": ADMIN_PASSWORD}).status_code in (404, 405, 422) def test_journal_page_filters_cursor_dialogs_and_settings(): """Страница журнала: фильтры, «Показать ещё» по курсору, окно записи, сохранение настроек.""" htmx = {"HX-Request": "true"} with TestClient(create_app()) as c: - h = {"Authorization": "Bearer test-token"} - c.post("/login", data={"username": "admin", "password": "pw"}) + h = {"Authorization": f"Bearer {API_TOKEN}"} + c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}) d = c.post("/api/v1/devices", headers=h, json={"name": "r1", "host": "10.0.0.1", "username": "u", "password": "p"}).json() _fill_events(105) page = c.get("/events").text @@ -685,3 +688,66 @@ def test_process_lock_blocks_second_process(tmp_path): process_lock.release() process_lock.acquire(db_url) # после освобождения — успешно process_lock.release() + + +def test_insecure_settings_rejects_weak_secrets_without_leaking_them(): + """Небезопасные секреты выявляются по каждому правилу; значения секретов в описание проблем не попадают.""" + secret = "s3cr3t-value-must-not-leak-anywhere" + bad = Settings(api_token="change-me", session_secret=secret[:10], admin_password=secret[:8], secret_key=secret) + problems = insecure_settings(bad) + assert len(problems) == 4 # все четыре секрета нарушают правила + text = " ".join(problems) + assert secret not in text and secret[:10] not in text and secret[:8] not in text + + ok = Settings(api_token="x" * 32, session_secret="y" * 32, admin_password="z" * 12, + secret_key=Fernet.generate_key().decode()) + assert insecure_settings(ok) == [] + + +def test_app_refuses_to_start_with_insecure_config(monkeypatch): + """Приложение не стартует с небезопасной конфигурацией (например, API_TOKEN=change-me).""" + monkeypatch.setenv("API_TOKEN", "change-me") + get_settings.cache_clear() + with pytest.raises(RuntimeError, match="Небезопасная конфигурация"): + with TestClient(create_app()): + pass + get_settings.cache_clear() + + +def test_login_lockout_by_ip(): + """5 неверных попыток входа с одного IP блокируют его на 6-ю (пароль уже не проверяется, код 429); + повторные попытки во время блокировки не засоряют журнал новыми auth.locked; блокировка не распространяется + на другой IP; успешный вход сбрасывает счётчик неудач.""" + with TestClient(create_app()) as c: + for _ in range(5): + r = c.post("/login", data={"username": "admin", "password": "wrong"}) + assert r.status_code == 401 + r = c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}) # верный пароль уже не спасает + assert r.status_code == 429 and "Слишком много попыток" in r.text + assert len(events.list_events(type_="auth.locked")) == 1 + for _ in range(3): # попытки во время блокировки — 429, но новых auth.locked не пишут (не засоряют журнал) + assert c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}).status_code == 429 + assert len(events.list_events(type_="auth.locked")) == 1 + + with TestClient(create_app(), client=("10.0.0.2", 1)) as c2: + r = c2.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}, follow_redirects=False) + assert r.status_code == 303 # другой IP свободен + + with TestClient(create_app(), client=("10.0.0.3", 1)) as c3: + for _ in range(3): + assert c3.post("/login", data={"username": "admin", "password": "wrong"}).status_code == 401 + r = c3.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}, follow_redirects=False) + assert r.status_code == 303 + for _ in range(4): # если бы счётчик не сбросился успехом, пятая по счёту неудача (2-я в этом цикле) заблокировала бы + assert c3.post("/login", data={"username": "admin", "password": "wrong"}).status_code == 401 + + +def test_move_redirect_rejects_open_redirect_next(): + """`next` в /ui/move принимает только локальный путь — иначе редирект на «/» (открытый редирект).""" + with TestClient(create_app()) as c: + c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}) + for bad in ("/\\evil.com", "//evil.com", "https://evil.com", "/\r\nX"): # последний — декодированный /%0d%0aX + r = c.post("/ui/move", data={"next": bad}, follow_redirects=False) + assert r.headers["location"] == "/" + r = c.post("/ui/move", data={"next": "/?f_group=none"}, follow_redirects=False) + assert r.headers["location"] == "/?f_group=none" # обычный путь с фильтром сохраняется