Newer
Older
smart-home-server / server / docs / review-phase-1-security-auth-device.md

Ревью серверной части — Этап 1: Security, Auth, Device Integration

Дата: 2026-06-02 Охват: App.php, Tools/DeviceAPI/Base.php, Models/Devices.php, Controllers/EventsController.php, Tools/RateLimiter.php, Sessions.php, Middleware/Entity.php, Entity-классы.


🔴 Критические

1. RateLimiter не работает в PHP-FPM/Apache (RateLimiter.php)

RateLimiter хранит $requests в static property процесса. При стандартном deployment (PHP-FPM, Apache mod_php) каждый HTTP-запуск — новый процесс, поэтому счётчик сбрасывается. Rate limit фактически отсутствует. Рекомендация: перенести на Redis / memcached / DB, или хотя бы на file-based storage.

2. /events/new не требует авторизации (Routes.php, EventsController.php)

check_api_auth() пропускает любой URI, не начинающийся с /api/v1/. /events/new — вне этого префикса, поэтому endpoint публичен. Любой может отправить события от имени любого устройства, зная его device_hard_id. Рекомендация: добавить device_token в заголовок Authorization и проверять его в EventsController::new_event перед by_hard_id, или перенести endpoint под /api/v1/ с auth guard.

3. Race condition в connect_new_device() (Models/Devices.php)

Нет atomic проверки статуса устройства. Два параллельных запроса могут одновременно увидеть "setup", создать две записи в devices+device_auth, и оба пройдут. Устройство перейдёт в normal mode, но в БД останутся дубли. Рекомендация: UNIQUE constraint на device_hard_id + status='active', или distributed lock через SELECT ... FOR UPDATE внутри транзакции.

4. Entity::update() unconditionally требует update_at (Middleware/Entity.php)

Entity::update() всегда inject-ит update_at в UPDATE SQL. DeviceAuth (таблица device_auth) не содержит update_at в $fields, и скорее всего нет и в production schema. Вызов DeviceAuth::kill()update() вызовет SQL error. Рекомендация: добавить update_at TEXT в production schema device_auth, или сделать update_at опциональным в Entity (например, проверять in_array($this->field_name_of_update_at, static::$fields)).


🟡 Высокие

5. Post-commit remote state inconsistency в connect_new_device()

Транзакция commit-ится, а затем вызывается:

  • $device->set_device_token($device_token)remote_set_token на устройство
  • $device->device_api()->set_device_name($name)POST /set_device_name

Если устройство offline или не отвечает после commit, в БД устройство зарегистрировано, но физически оно не знает токен/имя. Управление невозможно. Рекомендация: либо делать remote setup до commit (и rollback при fail), либо иметь флаг setup_pending + background retry job.

6. DeviceAuth::kill() не инвалидирует кэш device_auth_instance

Device::auth() кэширует device_auth_instance. После $device_auth->kill() статус меняется на "killed", но кэш в $device->device_auth_instance остаётся со старым статусом "active". Последующие вызовы $device->auth()->is_active() вернут true. Рекомендация: сбрасывать $device->device_auth_instance = null в DeviceAuth::kill() или в Device::set_device_token().

7. EventsController::new_eventdevice_id приходит извне без подписи

Устройство доказывает свою идентичность только через device_hard_id, который передаётся в POST body. Нет HMAC, timestamp, nonce. Replay attack возможен. Рекомендация: добавить Authorization: Bearer <device_token> (см. пункт 2) и/или подпись полезной нагрузки.


🟢 Средние

8. generate_token(16) — низкая энтропия

16 hex-символов = 64 бита. Для устройственных токенов в локальной сети это терпимо, но на грани. Рекомендация: поднять до 32 (128 бит) для device_token.

9. Base::request() — нет валидации $ip_address

http:// + произвольная строка. Если $ip_address содержит path (192.168.1.1/path), SSRF возможен. Контролируется server-side, но всё же. Рекомендация: валидировать filter_var($ip_address, FILTER_VALIDATE_IP) в конструкторе.

10. Entity::update() — тихий success при SQL error?

Entity::update() вызывает $this->thin_builder()->update(...) и не проверяет return value. ThinBuilder::update возвращает Statement или null? Возможно, exception вылетит (PDO ERRMODE_EXCEPTION), но если нет — тихий fail. Рекомендация: явно проверять результат thin_builder()->update() и возвращать false при ошибке.

11. Sessions::get_current_session() — лишний UPDATE на каждый запрос

Обновление last_using_at при каждом вызове создаёт write load. Для домашнего сервера не критично, но при масштабе заметно. Рекомендация: обновлять раз в N минут (например, если delta > 60s).

12. EventsControllerset_time_limit(10) может обрезать handlers

Если channel_alias_device_event_call и последующие вызовы занимают >10s — fatal. Это фоновая обработка после flush. Рекомендация: set_time_limit(0) или background queue (cron / worker).

13. check_api_auth() — Bearer token parsing

substr($auth_header, 7) без проверки длины. Header "Bearer" (без пробела) даст пустой token, который пройдёт в !$token → 401. Не критично, но стоит использовать str_starts_with + explode(' ', $auth_header, 2)[1] ?? null.


✅ Что сделано хорошо

  • Auth guard разделён на check_api_auth() (чистая логика) и api_auth_guard() (side-effects). Это позволяет тестировать без exit.
  • Cookie security flagshttponly, secure, samesite=Strict.
  • Retry/backoff в DeviceAPI\Base — exponential backoff, clear separation через executeCurl/getCurlInfo.
  • Транзакции в connect_new_device — rollback при ошибке.
  • PDO prepared statements в ThinBuilder — SQL injection минимизирован.

📋 Action Items (итог)

Приоритет Задача Файл
🔴 Крит Починить rate limiter для stateless PHP RateLimiter.php
🔴 Крит Защитить /events/new токеном устройства Routes.php, EventsController.php
🔴 Крит Убрать race condition в connect_new_device Models/Devices.php
🔴 Крит Добавить update_at в device_auth schema или сделать optional Middleware/Entity.php / DB schema
🟡 Выс Post-commit remote calls → pre-commit или pending flag Models/Devices.php
🟡 Выс Инвалидировать device_auth_instance при kill() Entities/DeviceAuth.php
🟡 Выс Replay-защита на /events/new EventsController.php
🟢 Сред Увеличить энтропию device_token Utils.php / Models/Devices.php
🟢 Сред Валидировать IP в DeviceAPI\Base Tools/DeviceAPI/Base.php
🟢 Сред Проверять return value в Entity::update() Middleware/Entity.php
🟢 Сред Throttle last_using_at updates Sessions.php
🟢 Сред set_time_limit(0) в EventsController Controllers/EventsController.php