diff --git a/CHANGELOG.md b/CHANGELOG.md index 770da59..b4397b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,18 @@ Hysteria-интеграции с официальной документацие `сохранённое изменение → живая сессия`, `планировщик → полностью принадлежащая ему работа`. +Двенадцатый проход — разбор состояния после одиннадцатого, снова со сверкой с +официальной документацией Hysteria 2. Тема: **вторая попытка**. Одиннадцатый +проход сделал правильным порядок «сначала запись, потом разрыв» и правильно +запретил откат при неудаче разрыва — но не дал системе способа прийти к +согласованному состоянию потом. Два состояния оставались навсегда: сессия +пира, которого импорт переподписал или удаление убрало, и превышение лимита +устройств после неудавшегося снижения. Вместе с ними закрыт второй TOCTOU в +лимите устройств — переупорядочивание снимков `/online`, которое учёт +разрешений сам по себе не ловил, а детектор гонок не мог показать в принципе. +Разбор задокументирован в +[docs/acceptance/2026-09-02-v1.0.0-rc3-preflight-findings.md](docs/acceptance/2026-09-02-v1.0.0-rc3-preflight-findings.md). + ### Исправлено — правило доступа - **Исчерпанная квота не отключала пира никогда.** Правило доступа @@ -167,6 +179,66 @@ Hysteria-интеграции с официальной документацие резерваций, протухшие снимаются по внутреннему TTL. Ни Redis, ни таблиц в базе, ни распределённых блокировок: HY2XS — один процесс на одном сервере. +### Исправлено — сходимость отзыва доступа (двенадцатый проход) + +- **Устаревший снимок `/online` возвращал уже занятое место.** Учёт выданных + разрешений закрыл сравнение двух одинаковых снимков, но сетевой запрос + по-прежнему выполнялся вне блокировки, поэтому снимки приходили в резервацию + в произвольном порядке. Более старый обгонял более новый и откатывал + `lastOnline` назад: `A` получил разрешение при `online = 0`; `C` обработал + `online = 1` первым и признал разрешение `A` проявившимся; пришедший следом + `B` со своим устаревшим `0` увидел место снова свободным. При + `maxDevices = 1` подключений становилось два. Детектор гонок здесь молчит + принципиально — вся работа с памятью защищена мьютексом, гонка логическая. + + Последовательность «прочитать `/online` → занять место» выполняется под + замком **по `authId`**, а не одним на процесс: внутри неё идёт сетевой + запрос, и общий замок выстроил бы подключения всех пиров в очередь за одним + HTTP-обменом. Карта замков не растёт — запись живёт ровно столько, сколько + есть желающие её взять. + +- **Живая сессия без строки в базе не завершалась никогда.** Цикл учёта читал + `dao.ListPeer("auth_id in ?")` и обходил найденные строки, поэтому `authId`, + которому в базе ничего не соответствует, молча выпадал. А именно он и + остаётся единственным следом сессии после неудавшегося второго шага: импорт + заменил `auth_id`, удаление убрало строку. Повторить операцию в этом + состоянии невозможно — повтор того же импорта читает из базы уже новое + значение и рвёт его, — а восстановить состояние переподключением нельзя: + авторизация нового значения не знает. Сессия жила неограниченно долго. + +- **Снижение `maxDevices` после неудавшегося разрыва не имело второй попытки.** + Условие сравнивало `*peerDto.MaxDevices < *before.MaxDevices`, а форма при + правке отправляет все поля, поэтому повторное сохранение давало `1 < 1` и + разрыва не делало. Лимит устройств в политику доступа не входит и входить не + должен — это свойство сессий, а не пира, — поэтому механизма схождения у него + не было вовсе, в отличие от `disabled`, квоты, срока и блокировки. + +- **Цикл учёта стал сверкой живых сессий.** Обход идёт по каждому `authId` из + `/online`: нет строки в базе → разрыв; `peerAccessDenied` → разрыв; + непригодный `maxDevices` → разрыв; устройств больше разрешённого → разрыв. + Отказ базы при этом не рвёт ничего: «пира нет» и «прочитать не удалось» — + разные ответы, и трактовка второго как первого отключила бы всех + подключённых пиров сразу при недоступной SQLite. Таблицы отложенных + операций, очереди retry и хранимого «списка того, что не удалось разорвать» + не появилось: список живых сессий уже есть, и это `/online`. Число устройств + сверено с официальным контрактом Traffic Stats API — `/online` возвращает + количество экземпляров клиента Hysteria, а не число proxy-потоков. + +- **Go 1.26.7 → 1.26.8.** Patch-релиз от 2026-09-01 (fixes в cgo, компиляторе, + runtime, `debug/elf` и `os`). Stdlib целиком попадает в production-бинарь, + поэтому «на один патч позади» — свойство выпускаемого артефакта, а не среды + сборки. Обновлены `GO_VERSION` с контрольной суммой и `toolchain` в + `apps/go.mod`: расхождение между ними роняет сборку на + `verify_go_toolchain_contract`. Major не менялся — линия 1.26 поддерживается. + +- **Учёт разрешений больше не растёт бесконечно.** Запись снималась только на + ветке отказа: после успешной выдачи она оставалась с непустым списком, а + когда разрешение протухало, снять её было уже некому — следующего обращения + к этому `authId` могло не быть никогда. В карте копились удалённые пиры и + старые идентификаторы, переписанные импортом. Уборка идёт по фактической + картине подключений в том же цикле учёта — единственном месте продукта, где + она известна целиком. + ### Исправлено — гейты сборки - **Гейт fail-open срабатывал на корректном коде.** Проверка «авторизация не diff --git a/README.md b/README.md index f09c323..8a23720 100644 --- a/README.md +++ b/README.md @@ -928,9 +928,9 @@ HY2XS development environment contract: versions.env (HY2XS 1.0.0, release line 1) Go: - required: 1.26.7 + required: 1.26.8 found: 1.25.6 - FAIL — локальный Go собирает не ту stdlib, что уедет в релиз; поставьте 1.26.7 + FAIL — локальный Go собирает не ту stdlib, что уедет в релиз; поставьте 1.26.8 Node: required: 24.20.0 diff --git a/apps/go.mod b/apps/go.mod index eca7e9a..589f117 100644 --- a/apps/go.mod +++ b/apps/go.mod @@ -10,14 +10,17 @@ go 1.25.0 // // Директива `go` выше — это языковой baseline модуля, и она НЕ выбирает // компилятор: с ней одной локальный `go build` на 1.25 проходил успешно, хотя -// релизный бинарь собирается на 1.26.7 и наследует её stdlib. То есть +// релизный бинарь собирается версией из versions.env и наследует её stdlib. +// Номер здесь не повторяется намеренно — он живёт строкой ниже и в +// GO_VERSION, а третья копия в прозе устаревала бы на каждом patch-релизе. То +// есть // разработчик и сборка проверяли разный код, а расхождение не было видно ни в // одном из выводов. // // Значение обязано совпадать с GO_VERSION из versions.env; это проверяет // verify_go_toolchain_contract, а `tools/dev/doctor` показывает то же // расхождение локально, до сборки. -toolchain go1.26.7 +toolchain go1.26.8 require ( github.com/didip/tollbooth v4.0.2+incompatible diff --git a/apps/service/cron.go b/apps/service/cron.go index 8c1f437..2989b5d 100644 --- a/apps/service/cron.go +++ b/apps/service/cron.go @@ -2,6 +2,7 @@ package service import ( "fmt" + "sort" "sync" "time" @@ -242,7 +243,8 @@ func (e *trafficLossError) Error() string { ) } -// enforcePeerAccess завершает сессии пиров, которым доступ уже закрыт. +// enforcePeerAccess приводит ЖИВЫЕ СЕССИИ в соответствие с сохранённым +// состоянием. // // Политика берётся из peerAccessDenied — той же функции, по которой пира // пускает или не пускает авторизация. Собственного SQL-условия здесь больше @@ -250,6 +252,25 @@ func (e *trafficLossError) Error() string { // расходилось на границах quota, expiry и ban, и исчерпавший квоту пир не // пускался заново, но и не отключался никогда. // +// Обход идёт по КАЖДОМУ authID, который Hysteria считает живым, а не по +// найденным в базе пирам. Прежняя реализация читала +// +// peers, err := dao.ListPeer("auth_id in ?", chunk) +// for _, peer := range peers { ... } +// +// и потому не видела сессий, которым в базе больше ничего не соответствует. +// Это не теоретический случай: `auth_id` перезаписывает импорт, а строку +// целиком убирает удаление. Обе операции рвут старую сессию сами, но их второй +// шаг может не удаться — и тогда единственным местом, где о ней ещё известно, +// остаётся сам `/online`. Пропуская незнакомый идентификатор молча, cron +// оставлял такую сессию жить неограниченно долго. Подробности — в +// peer_session-разделе peer_access.go. +// +// Отказ базы НЕ приводит к разрыву. «Пира нет» и «прочитать не удалось» — +// разные ответы, и второй не даёт права рвать ничьи сессии: недоступная SQLite +// иначе означала бы отключение всех подключённых пиров сразу. Ошибка чтения +// прекращает цикл до единого обращения к `/kick`. +// // Обход последовательный. Прежняя реализация раскладывала online-пиров на // чанки по 10 и запускала по горутине на чанк с sync.WaitGroup внутри уже // отсоединённой горутины. Параллельность здесь не нужна: обращений к базе @@ -259,12 +280,20 @@ func enforcePeerAccess(apiPort int64, trafficStatsSecret string) error { if err != nil { return err } + + // Учёт выданных разрешений чистится по фактической картине подключений, и + // это единственное место продукта, где она известна целиком. Делается это + // до любых решений: уборка ни на что не влияет и ничего не рвёт. + sweepDeviceAdmissions(online, time.Now()) + if len(online) == 0 { return nil } authIDs := make([]string, 0, len(online)) for authID := range online { + // Пустой ключ ничему не соответствует: рвать по нему нечего, и в + // dedup disconnectAuthIDs он всё равно не попал бы. if authID == "" { continue } @@ -273,25 +302,45 @@ func enforcePeerAccess(apiPort int64, trafficStatsSecret string) error { if len(authIDs) == 0 { return nil } + // Порядок ключей карты в Go случаен; сортировка делает и обращение к + // `/kick`, и журнал воспроизводимыми. + sort.Strings(authIDs) - now := time.Now().UnixMilli() - kick := make([]string, 0, len(authIDs)) + known := make(map[string]entity.Peer, len(authIDs)) for _, chunk := range util.SplitArr(authIDs, 100) { peers, err := dao.ListPeer("auth_id in ?", chunk) if err != nil { return err } for _, peer := range peers { - // Строка без authId Hysteria не знает: рвать нечего. Раньше здесь - // стояло `*item.AuthId` без проверки — паника на повреждённой - // строке внутри отсоединённой горутины. + // Строка без authId Hysteria не знает. Раньше здесь стояло + // `*item.AuthId` без проверки — паника на повреждённой строке + // внутри отсоединённой горутины. authID := authIDOf(peer) if authID == "" { continue } - if peerAccessDenied(peer, now) { - kick = append(kick, authID) - } + known[authID] = peer + } + } + + now := time.Now().UnixMilli() + kick := make([]string, 0, len(authIDs)) + for _, authID := range authIDs { + peer, found := known[authID] + if !found { + // Сессия, которой в базе больше ничего не соответствует: пир удалён + // либо его идентификатор заменён импортом, а разрыв в тот момент не + // удался. Восстановить такое состояние переподключением нельзя — + // авторизация нового значения не знает, — поэтому единственный + // правильный исход тот же, что и у первой попытки. + logrus.WithField("authId", authID). + Warn("cron: живая сессия без пира в базе; сессия завершается") + kick = append(kick, authID) + continue + } + if peerSessionNeedsReconcile(peer, online[authID], now) { + kick = append(kick, authID) } } diff --git a/apps/service/cron_test.go b/apps/service/cron_test.go index c3f5909..4dc618e 100644 --- a/apps/service/cron_test.go +++ b/apps/service/cron_test.go @@ -13,6 +13,7 @@ import ( "hy2xs-admin/dao" "hy2xs-admin/model/bo" "hy2xs-admin/model/constant" + "hy2xs-admin/model/dto" ) // Цикл учёта проверяется против НАСТОЯЩЕГО Traffic Stats API. @@ -289,6 +290,197 @@ func TestCronIgnoresOfflinePeers(t *testing.T) { } } +// --- Сверка живых сессий ----------------------------------------------------- + +// Живая сессия, которой в базе больше ничего не соответствует, завершается. +// +// Прежний обход шёл по НАЙДЕННЫМ пирам, поэтому authID, которого нет в базе, +// молча выпадал: `dao.ListPeer("auth_id in ?")` просто не возвращала строку. +// Такое состояние возникает после неудавшегося второго шага удаления или +// импорта, заменившего `auth_id`, и восстановить его переподключением нельзя — +// авторизация нового значения не знает. Сессия жила неограниченно долго. +func TestCronKicksSessionWithoutPeerRow(t *testing.T) { + newTestDB(t) + stub := startAccountStats(t, &accountStatsStub{ + online: map[string]int64{"ghost-auth-id": 1}, + }) + seedPeer(t, "alpha1", "alpha-auth-id") + + CronHandleAccount() + + if got := stub.kicked(); len(got) != 1 || got[0] != "ghost-auth-id" { + t.Fatalf("сессия без пира в базе не завершена: %v", got) + } +} + +// Превышение лимита устройств — свойство живых сессий, а не хранимого +// состояния пира, поэтому peerAccessDenied его не видит и видеть не должен. +// Без этой проверки неудавшийся разрыв при снижении `maxDevices` оставался бы +// навсегда: повторное сохранение формы сравнивает `1 < 1` и разрыва не делает. +func TestCronKicksWhenOnlineExceedsMaxDevices(t *testing.T) { + newTestDB(t) + stub := startAccountStats(t, &accountStatsStub{ + online: map[string]int64{"alpha-auth-id": 3}, + }) + id := seedPeer(t, "alpha1", "alpha-auth-id") + peerUsage(t, id, map[string]interface{}{"max_devices": int64(1)}) + + CronHandleAccount() + + if got := stub.kicked(); len(got) != 1 || got[0] != "alpha-auth-id" { + t.Fatalf("превышение лимита устройств не отключено: %v", got) + } +} + +// Граница: устройств ровно столько, сколько разрешено, — рвать нечего. +func TestCronDoesNotKickAtExactDeviceLimit(t *testing.T) { + newTestDB(t) + stub := startAccountStats(t, &accountStatsStub{ + online: map[string]int64{"alpha-auth-id": 3}, + }) + // seedPeer создаёт пира с maxDevices = 3. + seedPeer(t, "alpha1", "alpha-auth-id") + + CronHandleAccount() + + if got := stub.kicked(); len(got) != 0 { + t.Fatalf("пир на границе лимита отключён: %v", got) + } +} + +// Повреждённая граница — не «безлимит». На пути авторизации такая строка ведёт +// к отказу, и живая сессия обязана следовать тому же правилу. +func TestCronKicksPeerWithUnusableMaxDevices(t *testing.T) { + newTestDB(t) + stub := startAccountStats(t, &accountStatsStub{ + online: map[string]int64{"alpha-auth-id": 1}, + }) + id := seedPeer(t, "alpha1", "alpha-auth-id") + peerUsage(t, id, map[string]interface{}{"max_devices": int64(0)}) + + CronHandleAccount() + + if got := stub.kicked(); len(got) != 1 { + t.Fatalf("пир с непригодным лимитом устройств не отключён: %v", got) + } +} + +// Отказ базы НЕ является основанием рвать сессии. +// +// «Пира нет» и «прочитать не удалось» — разные ответы, и решение «сессии +// неизвестны, значит лишние» на втором из них отключило бы всех подключённых +// пиров сразу при недоступной SQLite. Проверка существует именно потому, что +// правило «неизвестный authID -> kick» делает это различие решающим. +func TestCronSendsNoKickWhenPeerLookupFails(t *testing.T) { + newTestDB(t) + stub := startAccountStats(t, &accountStatsStub{ + online: map[string]int64{"alpha-auth-id": 1}, + }) + seedPeer(t, "alpha1", "alpha-auth-id") + + apiPort, err := GetHysteria2ApiPort() + if err != nil { + t.Fatalf("порт Traffic Stats API: %v", err) + } + // Порт и секрет читаются из конфига Hysteria, поэтому база после этого уже + // не нужна ни для чего, кроме самой выборки пиров. + if err := dao.CloseSqliteDB(); err != nil { + t.Fatalf("не удалось закрыть базу: %v", err) + } + + if err := enforcePeerAccess(apiPort, testTrafficStatsSecret); err == nil { + t.Fatal("отказ базы не сообщён вызывающему") + } + if got := stub.kicked(); len(got) != 0 { + t.Fatalf("отказ базы привёл к разрыву сессий: %v", got) + } +} + +// Учёт выданных разрешений чистится по фактической картине подключений. +func TestCronSweepsAdmissionsOfOfflinePeers(t *testing.T) { + newTestDB(t) + startAccountStats(t, &accountStatsStub{online: map[string]int64{}}) + seedPeer(t, "alpha1", "alpha-auth-id") + + // Разрешение выдано давно и уже протухло, подключения так и не случилось. + if !reserveDeviceSlot("alpha-auth-id", 0, 3, time.Now().Add(-2*pendingAdmissionTTL)) { + t.Fatal("подготовка учёта: разрешение отклонено") + } + if admissionEntries() != 1 { + t.Fatal("подготовка учёта: запись не создана") + } + + CronHandleAccount() + + if got := admissionEntries(); got != 0 { + t.Fatalf("учёт не убран: записей %d", got) + } +} + +// --- Сходимость после неудавшегося разрыва ----------------------------------- + +// Импорт заменил `auth_id`, а разрыв старой сессии не удался. Повторить его +// операцией импорта невозможно: в базе уже новое значение, и повтор того же +// файла разорвал бы именно его. Сходимость обеспечивает cron. +func TestCronReconcilesSessionAfterFailedImportKick(t *testing.T) { + newTestDB(t) + stub := startAccountStats(t, &accountStatsStub{ + kickStatus: http.StatusInternalServerError, + online: map[string]int64{"old-auth-id": 1}, + }) + seedPeer(t, "keeper", "old-auth-id") + + requireDisconnectError(t, UpsertPeerExport([]bo.PeerExport{importItem("keeper", "new-auth-id")})) + + after := snapshotPeers(t)["keeper"] + if after.AuthId == nil || *after.AuthId != "new-auth-id" { + t.Fatalf("импорт не применён: authId=%v", after.AuthId) + } + + stub.mu.Lock() + stub.kickStatus = 0 + stub.kickedKeys = nil + stub.mu.Unlock() + + CronHandleAccount() + + if got := stub.kicked(); len(got) != 1 || got[0] != "old-auth-id" { + t.Fatalf("старая сессия не завершена следующим циклом учёта: %v", got) + } +} + +// Лимит устройств снижен, разрыв не удался, оператор повторяет сохранение +// формы — и получает успех без разрыва, потому что новое значение уже в базе. +// Единственный механизм схождения здесь — cron. +func TestCronReconcilesSessionAfterFailedMaxDevicesReduction(t *testing.T) { + newTestDB(t) + stub := startAccountStats(t, &accountStatsStub{ + kickStatus: http.StatusInternalServerError, + online: map[string]int64{"alpha-auth-id": 3}, + }) + // seedPeer создаёт пира с maxDevices = 3. + id := seedPeer(t, "alpha1", "alpha-auth-id") + + requireDisconnectError(t, UpdatePeer(id, dto.PeerUpdateDto{MaxDevices: int64Ptr(1)})) + + // Повтор формы: значение то же самое, разрыва не будет — и это правильно, + // иначе каждое сохранение любой правки рвало бы сессии. + if err := UpdatePeer(id, dto.PeerUpdateDto{MaxDevices: int64Ptr(1)}); err != nil { + t.Fatalf("повторное сохранение формы отказало: %v", err) + } + + stub.mu.Lock() + stub.kickStatus = 0 + stub.kickedKeys = nil + stub.mu.Unlock() + + CronHandleAccount() + + if got := stub.kicked(); len(got) != 1 || got[0] != "alpha-auth-id" { + t.Fatalf("превышение лимита не устранено следующим циклом учёта: %v", got) + } +} + // --- Порядок и устройство цикла ---------------------------------------------- // Enforcement принимает решение по счётчикам, поэтому счётчики обязаны быть diff --git a/apps/service/hysteria2_api.go b/apps/service/hysteria2_api.go index 5ad5ba5..7348cc9 100644 --- a/apps/service/hysteria2_api.go +++ b/apps/service/hysteria2_api.go @@ -114,6 +114,22 @@ func Hysteria2Auth(conPass string) (int64, string, error) { // недоступность здесь — не штатное состояние, а аномалия, и пускать // подключения без единственной проверки, которая ещё не выполнена, значит // молча снять лимит со всех пиров сразу. + + // Чтение `/online` и резервация места — ОДНА последовательность, и она + // выполняется под замком этого пира. + // + // Без замка снимки приходили в резервацию в произвольном порядке, и + // устаревший откатывал учёт назад: разрешение, уже признанное проявившимся, + // возвращалось в «свободное место». Подробный разбор — в начале + // peer_admission.go. + // + // Замок берётся именно здесь, а не раньше: до этой точки известен только + // секрет, а сериализовать нужно подключения ОДНОГО пира, то есть замок + // невозможно взять, пока не прочитан его authId. Всё, что выше, — работа с + // базой и политикой доступа, и разным пирам она не мешает. + unlockAdmission := lockPeerAdmission(*peer.AuthId) + defer unlockAdmission() + onlineUsers, err := hysteria2Online() if err != nil { logrus.WithError(err). diff --git a/apps/service/peer_access.go b/apps/service/peer_access.go index 818ba52..5ddb26b 100644 --- a/apps/service/peer_access.go +++ b/apps/service/peer_access.go @@ -100,3 +100,73 @@ func peerAccessDenied(peer entity.Peer, now int64) bool { return false } + +// Живая сессия сверяется с сохранённым состоянием ПОВТОРЯЕМО. +// +// Что было. Приведение сессий к состоянию базы выполнялось ровно один раз — в +// той же операции, которая это состояние записала. Порядок «сначала запись, +// потом `/kick`» правильный, и откат при неудаче разрыва делать нельзя: часть +// операции, закрывающая доступ, уже достигнута. Но второй попытки после +// неудачи не существовало вовсе, и два состояния оставались навсегда. +// +// Первое — замена `auth_id` импортом: +// +// импорт old-auth -> new-auth, COMMIT прошёл +// /kick old-auth -> 500 +// оператор повторяет тот же импорт +// applyPeerImportEntry читает из базы уже new-auth и рвёт ЕГО +// +// Старый идентификатор после первой же неудачи не хранился нигде, а cron его +// пропускал: `dao.ListPeer("auth_id in ?")` просто не возвращала строку, и +// authID, которого нет в базе, молча выпадал из обхода. Живая QUIC-сессия +// удалённого или переподписанного пира продолжалась сколько угодно долго. +// +// Второе — снижение `maxDevices`: +// +// 5 -> 1, запись прошла, /kick -> 500 +// оператор повторяет сохранение формы +// updateRequiresReconcile сравнивает 1 < 1 -> false, разрыва нет +// +// Для `disabled` повторяемость сделана специально (условие смотрит на +// ЗАПРОШЕННОЕ состояние, а не на переход), для квоты и срока её обеспечивает +// cron через peerAccessDenied. Лимит устройств в политику доступа не входит и +// входить не должен — это свойство не пира, а его сессий, — поэтому здесь у +// него не было ни одного механизма схождения. +// +// Оба состояния закрывает один и тот же приём: cron сверяет не «кого из +// известных пиров пора отключить», а КАЖДЫЙ authID, который Hysteria считает +// живым. Отдельная таблица retry, очередь отложенных операций и хранимый +// «список того, что не удалось разорвать» для этого не нужны: `/online` и есть +// список живых сессий, и сверять его достаточно. + +// peerSessionNeedsReconcile отвечает, устарела ли живая сессия пира. +// +// Вторым экземпляром политики доступа не является: disabled, quota, expiry и +// ban остаются целиком за peerAccessDenied, и эта функция их не повторяет, а +// вызывает. Своего здесь ровно одно — инвариант живых сессий, которого в +// хранимом состоянии пира нет: число подключённых устройств. +// +// доступ закрыт -> сессия устарела +// maxDevices непригоден -> сессия устарела +// устройств больше, чем разрешено -> сессии устарели +// +// Непригодный `maxDevices` ведёт к разрыву по той же причине, по которой он +// ведёт к отказу в авторизации: повреждённая граница — это не «безлимит». +// +// Число устройств берётся из `/online`, который по официальному контракту +// Traffic Stats API возвращает количество экземпляров клиента Hysteria +// («устройства»), а не число proxy-потоков. То есть сравнение с `maxDevices` +// здесь опирается на upstream-контракт, а не на предположение. +// +// Выбирать «лишнее устройство» не нужно и невозможно: `/kick` оперирует +// идентификатором клиента. После разрыва клиенты переподключаются, и +// admission пропустит ровно столько, сколько разрешено теперь. +func peerSessionNeedsReconcile(peer entity.Peer, onlineDevices int64, now int64) bool { + if peerAccessDenied(peer, now) { + return true + } + if peer.MaxDevices == nil || *peer.MaxDevices < 1 { + return true + } + return onlineDevices > *peer.MaxDevices +} diff --git a/apps/service/peer_admission.go b/apps/service/peer_admission.go index e56ff6b..9b710b9 100644 --- a/apps/service/peer_admission.go +++ b/apps/service/peer_admission.go @@ -42,6 +42,37 @@ import ( // «соединение установлено / не установлено» математически точной системы // резервирования не построить. Он закрывает конкретный и реальный TOCTOU — // параллельные HTTP-auth одного процесса, — и делает это fail-closed. +// +// Второй TOCTOU: ПЕРЕУПОРЯДОЧИВАНИЕ снимков `/online`. +// +// Учёта выданных разрешений самого по себе оказалось недостаточно, и это +// отдельный дефект, а не оттенок первого. Сетевой запрос выполнялся вне +// мьютекса, поэтому снимки приходили в критическую секцию в произвольном +// порядке — более старый мог обогнать более новый: +// +// 1. A получает разрешение при `/online = 0`; pending = [A], lastOnline = 0 +// 2. B читает `/online = 0` и задерживается на обратном пути +// 3. A действительно подключается, Hysteria показывает `/online = 1` +// 4. C читает `/online = 1` и входит в резервацию ПЕРВЫМ: +// разрешение A признано проявившимся, lastOnline = 1, C получает отказ +// 5. B входит со своим устаревшим `online = 0` +// 6. `online > lastOnline` ложно, после чего lastOnline откатывается в 0 +// 7. `0 + pending(0) < 1` -> B получает ALLOW +// +// При `maxDevices = 1` подключений становится два. Это НЕ data race: вся +// работа с памятью защищена мьютексом, поэтому детектор гонок здесь молчит +// принципиально, и поймать дефект может только семантическая проверка. +// +// Лечится это не глобальным мьютексом вокруг сети — он сериализовал бы +// подключения всех пиров через один HTTP-обмен, — а замком на ОДИН authID: +// см. lockPeerAdmission. Конкурируют между собой только авторизации одного и +// того же пира, а их упорядоченность и есть требуемое свойство: снимок, +// прочитанный под замком, не может оказаться старше уже обработанного. +// +// Оба механизма нужны одновременно и закрывают разные половины: +// +// замок по authID — снимки не переупорядочиваются; +// учёт разрешений — снимок не успевает измениться к следующему запросу. // pendingAdmissionTTL — срок жизни выданного разрешения, которое ещё не // проявилось в `/online`. @@ -74,6 +105,67 @@ var deviceAdmissions = struct { byAuthID map[string]*admissionState }{byAuthID: map[string]*admissionState{}} +// admissionGate — замок одного authID вместе со счётчиком тех, кому он сейчас +// нужен. +// +// Счётчик существует ради удаления записи. Без него карта замков росла бы по +// одной записи на каждый когда-либо авторизовавшийся authId и не уменьшалась +// бы никогда — то есть та же утечка, от которой в учёте разрешений защищает +// forgetIfIdle, только этажом выше. +type admissionGate struct { + mu sync.Mutex + // waiting — сколько вызывающих держат замок или ждут его. Пока значение + // больше нуля, запись обязана оставаться в карте: удалив её, второй + // вызывающий создал бы НОВЫЙ замок и разошёлся бы с первым. + waiting int +} + +var admissionGates = struct { + sync.Mutex + byAuthID map[string]*admissionGate +}{byAuthID: map[string]*admissionGate{}} + +// lockPeerAdmission сериализует последовательность «прочитать `/online` -> +// занять место» для ОДНОГО пира и возвращает функцию освобождения. +// +// Замок именно по authID, а не один на процесс, и это существенно. Внутри него +// выполняется сетевой запрос к Traffic Stats API, поэтому общий замок означал +// бы, что все подключения всех пиров выстраиваются в очередь за одним +// HTTP-обменом. Здесь же конкурируют только авторизации одного пира — то есть +// ровно те, для которых порядок и решается. +// +// Время удержания ограничено сверху таймаутом самого обращения к Traffic Stats +// API (proxy.Hysteria2Api ставит контекст на 3 секунды), поэтому «застрявшая» +// Hysteria не превращает замок в бессрочный. +// +// Карта замков и карта учёта разрешений намеренно раздельны: первая описывает, +// кто сейчас проходит авторизацию, вторая — что уже выдано. Совмещение их в +// одной структуре означало бы удерживать мьютекс учёта на время сетевого +// запроса. +func lockPeerAdmission(authID string) func() { + admissionGates.Lock() + gate := admissionGates.byAuthID[authID] + if gate == nil { + gate = &admissionGate{} + admissionGates.byAuthID[authID] = gate + } + gate.waiting++ + admissionGates.Unlock() + + gate.mu.Lock() + + return func() { + gate.mu.Unlock() + + admissionGates.Lock() + defer admissionGates.Unlock() + gate.waiting-- + if gate.waiting == 0 { + delete(admissionGates.byAuthID, authID) + } + } +} + // reserveDeviceSlot решает, есть ли для нового подключения свободное место, и // занимает его. // @@ -83,6 +175,12 @@ var deviceAdmissions = struct { // передаёт сюда уже полученное число. Под блокировкой остаются только // несколько операций с map: держать её на время HTTP-обмена значило бы // сериализовать все подключения всех пиров через один сетевой запрос. +// +// Упорядоченность снимков обеспечивает не этот мьютекс, а замок по authID: +// вызывающий обязан удерживать lockPeerAdmission от чтения `/online` и до +// возврата отсюда. Без него сюда попадал бы снимок старше уже обработанного, и +// строка `state.lastOnline = online` откатывала бы учёт назад — см. описание +// второго TOCTOU в начале файла. func reserveDeviceSlot(authID string, online int64, maxDevices int64, now time.Time) bool { deviceAdmissions.Lock() defer deviceAdmissions.Unlock() @@ -159,10 +257,62 @@ func (s *admissionState) forgetIfIdle(authID string) { } } +// sweepDeviceAdmissions убирает записи о пирах, за которыми ничего не числится. +// +// Зачем это нужно отдельно от forgetIfIdle. Тот срабатывает ТОЛЬКО на ветке +// отказа: после успешной выдачи разрешения запись остаётся с непустым pending, +// а когда разрешение протухает, снять запись уже некому — следующего обращения +// к этому authId может не быть никогда. Так в карте оставались пиры, удалённые +// из панели, и старые authId, переписанные импортом: за время жизни процесса +// она только росла. +// +// Решение принимается по ФАКТИЧЕСКОЙ картине подключений, а не по хранимому +// lastOnline: последний обновляется только на пути авторизации, поэтому у +// отключившегося пира он остаётся прежним сколь угодно долго. +// +// Удаление записи, у которой нет ни одного действующего разрешения и нет +// подключений, не меняет ни одного будущего решения. Следующая резервация +// начнёт с чистой записи и придёт к тому же ответу: dropMaterialized на пустом +// списке — no-op, а `state.lastOnline` в любом случае перезаписывается +// пришедшим значением до сравнения с лимитом. +// +// Замок authID здесь не берётся намеренно: удерживать его на всём обходе карты +// значило бы останавливать авторизацию каждые 30 секунд. Гонка с параллельной +// авторизацией безопасна — она либо уже добавила разрешение (тогда pending не +// пуст и запись остаётся), либо ещё не дошла до учёта (тогда она создаст +// запись заново, и это ровно та же чистая запись). +func sweepDeviceAdmissions(online map[string]int64, now time.Time) { + deviceAdmissions.Lock() + defer deviceAdmissions.Unlock() + + for authID, state := range deviceAdmissions.byAuthID { + state.dropExpired(now) + if len(state.pending) > 0 { + continue + } + if online[authID] > 0 { + continue + } + delete(deviceAdmissions.byAuthID, authID) + } +} + // resetDeviceAdmissions очищает учёт. Существует ради тестов: состояние здесь // принадлежит процессу, и без сброса тесты видели бы резервации друг друга. func resetDeviceAdmissions() { deviceAdmissions.Lock() - defer deviceAdmissions.Unlock() deviceAdmissions.byAuthID = map[string]*admissionState{} + deviceAdmissions.Unlock() + + // Замки сбрасывать НЕЛЬЗЯ: удерживаемый кем-то замок, потерянный из карты, + // перестал бы исключать второго вызывающего. Вместо сброса тесты проверяют, + // что после завершения работы карта пуста сама. +} + +// heldPeerAdmissionGates — число живых замков. Существует ради тестов: утечка +// здесь выглядит именно как незакрытая запись в карте. +func heldPeerAdmissionGates() int { + admissionGates.Lock() + defer admissionGates.Unlock() + return len(admissionGates.byAuthID) } diff --git a/apps/service/peer_admission_test.go b/apps/service/peer_admission_test.go index c22a321..f6a72ce 100644 --- a/apps/service/peer_admission_test.go +++ b/apps/service/peer_admission_test.go @@ -2,6 +2,7 @@ package service import ( "encoding/json" + "fmt" "net/http" "net/http/httptest" "sync" @@ -22,37 +23,268 @@ import ( // Последовательный тест этого не поймает никогда — нужен барьер, на котором // оба запроса гарантированно видят ОДНО И ТО ЖЕ состояние Hysteria. -// --- Барьерный тест против настоящего HTTP ----------------------------------- +// --- Параллельные подключения одного пира ------------------------------------ -// startBarrierTrafficStats поднимает Traffic Stats API, который задерживает -// первые `hold` обращений к `/online` до тех пор, пока не придут все. -// -// Так воспроизводится ровно то состояние гонки, которое случается на живом -// сервере: оба запроса авторизации прочитали статистику до того, как хоть один -// из них успел превратиться в подключение. -func startBarrierTrafficStats(t *testing.T, online map[string]int64, hold int) { +// authResults прогоняет count одновременных авторизаций и возвращает их исходы. +func authResults(t *testing.T, secret string, count int) []error { t.Helper() - var mu sync.Mutex - arrived := 0 - release := make(chan struct{}) + var wg sync.WaitGroup + results := make([]error, count) + for i := range results { + wg.Add(1) + go func(idx int) { + defer wg.Done() + _, _, err := Hysteria2Auth(secret) + results[idx] = err + }(i) + } + wg.Wait() + return results +} + +func allowedCount(results []error) int { + allowed := 0 + for _, err := range results { + if err == nil { + allowed++ + } + } + return allowed +} + +// Главная регрессия AUTH-03: при `online = max-1` разрешение обязан получить +// ровно ОДИН из двух одновременных запросов. +// +// Барьера, заставлявшего оба запроса увидеть один снимок, здесь больше нет — и +// это следствие исправления, а не упрощение теста. Авторизации одного authId +// сериализованы замком (см. lockPeerAdmission), поэтому одновременно внутри +// `/online` они оказаться не могут, и барьер на двоих просто не собрался бы. +// Доказываемое свойство от этого не изменилось: `/online` отвечает обоим +// одинаково — именно так и ведёт себя Hysteria, пока клиент ещё устанавливает +// соединение, — и без учёта выданных разрешений оба сравнивали бы `2 < 3`. +func TestHysteria2AuthHoldsDeviceLimitUnderConcurrency(t *testing.T) { + newTestDB(t) + // seedPeer создаёт пира с maxDevices = 3, поэтому online = 2 — это + // последнее свободное место. + startTrafficStats(t, &trafficStatsStub{online: map[string]int64{"alpha-auth-id": 2}}) + seedPeer(t, "alpha1", "alpha-auth-id") + + if allowed := allowedCount(authResults(t, "alpha1-secret", 2)); allowed != 1 { + t.Fatalf("на последнее свободное место допущено %d подключений из 2", allowed) + } +} + +// Свободных мест два — проходят оба: механизм не должен превращаться в отказ +// всем, кроме первого. +func TestHysteria2AuthAdmitsBothWhenTwoSlotsFree(t *testing.T) { + newTestDB(t) + startTrafficStats(t, &trafficStatsStub{online: map[string]int64{"alpha-auth-id": 1}}) + seedPeer(t, "alpha1", "alpha-auth-id") + + for idx, err := range authResults(t, "alpha1-secret", 2) { + if err != nil { + t.Fatalf("подключение %d отклонено при двух свободных местах: %v", idx, err) + } + } +} + +// sequencedTrafficStats — Traffic Stats API, у которого ОДНО заранее названное +// обращение к `/online` удерживается до команды теста. +// +// Именно так воспроизводится переупорядочивание снимков: удерживаемый ответ +// содержит картину на момент ПРИХОДА запроса, а к моменту его доставки картина +// уже другая. Ответ формируется до блокировки намеренно — иначе тест проверял +// бы не устаревший снимок, а свежий. +type sequencedTrafficStats struct { + mu sync.Mutex + online map[string]int64 + calls int + holdCall int + + held chan struct{} + released chan struct{} +} + +func startSequencedTrafficStats(t *testing.T, online map[string]int64, holdCall int) *sequencedTrafficStats { + t.Helper() + + stats := &sequencedTrafficStats{ + online: online, + holdCall: holdCall, + held: make(chan struct{}), + released: make(chan struct{}), + } server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/online": - mu.Lock() - arrived++ - last := arrived == hold - mu.Unlock() + stats.mu.Lock() + stats.calls++ + call := stats.calls + snapshot := make(map[string]int64, len(stats.online)) + for key, value := range stats.online { + snapshot[key] = value + } + stats.mu.Unlock() + + if call == stats.holdCall { + close(stats.held) + select { + case <-stats.released: + case <-time.After(5 * time.Second): + // Тест не дал команды — отпускаем, чтобы падение было по + // существу, а не по таймауту всего прогона. + } + } + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(snapshot) + case "/kick": + w.WriteHeader(http.StatusOK) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + + pointHysteriaConfigAt(t, server.URL) + if err := dao.UpsertConfigValue(constant.Hysteria2TrafficStatsSecret, testTrafficStatsSecret); err != nil { + t.Fatalf("не удалось записать секрет Traffic Stats API: %v", err) + } + return stats +} + +func (s *sequencedTrafficStats) setOnline(online map[string]int64) { + s.mu.Lock() + defer s.mu.Unlock() + s.online = online +} + +func (s *sequencedTrafficStats) awaitHeld(t *testing.T) { + t.Helper() + + select { + case <-s.held: + case <-time.After(5 * time.Second): + t.Fatal("удерживаемое обращение к /online так и не пришло") + } +} + +func (s *sequencedTrafficStats) release() { + close(s.released) +} + +// Устаревший снимок `/online` не возвращает уже занятое место. +// +// Регрессия второго TOCTOU. Учёт выданных разрешений сам по себе его не +// закрывал: сетевой запрос выполнялся вне мьютекса, поэтому снимки приходили в +// резервацию в произвольном порядке, и более старый откатывал `lastOnline` +// назад: +// +// A получил разрешение при online = 0 +// B прочитал online = 0 и задержался +// A подключился, Hysteria показывает online = 1 +// C прочитал online = 1 и первым вошёл в резервацию: +// разрешение A признано проявившимся, lastOnline = 1, C отклонён +// B входит со своим устаревшим 0 -> lastOnline снова 0 -> B ДОПУЩЕН +// +// При maxDevices = 1 подключений становилось два. Детектор гонок здесь +// бесполезен принципиально: вся работа с памятью защищена мьютексом, и гонка +// тут логическая, а не по памяти. +func TestHysteria2AuthRejectsStaleOnlineSnapshot(t *testing.T) { + newTestDB(t) + // Удерживается ВТОРОЕ обращение к `/online` — то самое, которое в разборе + // принадлежит запросу B. + stats := startSequencedTrafficStats(t, map[string]int64{"alpha-auth-id": 0}, 2) + id := seedPeer(t, "alpha1", "alpha-auth-id") + if err := dao.UpdatePeer([]int64{id}, map[string]interface{}{"max_devices": 1}); err != nil { + t.Fatalf("не удалось выставить лимит устройств: %v", err) + } + + // A занимает единственное место. + if _, _, err := Hysteria2Auth("alpha1-secret"); err != nil { + t.Fatalf("первое подключение отклонено: %v", err) + } + + // B читает `/online = 0` и застревает на обратном пути. + var bErr error + bDone := make(chan struct{}) + go func() { + defer close(bDone) + _, _, bErr = Hysteria2Auth("alpha1-secret") + }() + stats.awaitHeld(t) + + // A действительно подключился: Hysteria показывает одно устройство. + stats.setOnline(map[string]int64{"alpha-auth-id": 1}) + + // C авторизуется уже по НОВОМУ снимку. На исправленном коде он ждёт замок и + // до `/online` не доходит, поэтому ожидание ограничено: тест не должен + // зависеть от того, успел C или нет. + var cErr error + cDone := make(chan struct{}) + go func() { + defer close(cDone) + _, _, cErr = Hysteria2Auth("alpha1-secret") + }() + select { + case <-cDone: + case <-time.After(300 * time.Millisecond): + } + + stats.release() + <-bDone + <-cDone + + if bErr == nil { + t.Fatal("устаревший снимок /online вернул уже занятое место: допущено второе устройство при лимите 1") + } + if cErr == nil { + t.Fatal("допущено второе устройство при лимите 1") + } +} + +// --- Параллельные подключения разных пиров ----------------------------------- + +// barrierTrafficStats — Traffic Stats API, который задерживает первые `hold` +// обращений к `/online`, пока не придут все. +// +// Так проверяется, что замок авторизации НЕ является общим на процесс: два +// запроса разных пиров обязаны оказаться внутри `/online` одновременно. С +// глобальным замком барьер не собрался бы никогда. +type barrierTrafficStats struct { + mu sync.Mutex + arrived int + collected bool + release chan struct{} +} + +func startBarrierTrafficStats(t *testing.T, online map[string]int64, hold int) *barrierTrafficStats { + t.Helper() + + barrier := &barrierTrafficStats{release: make(chan struct{})} + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/online": + barrier.mu.Lock() + barrier.arrived++ + last := barrier.arrived == hold + if last { + barrier.collected = true + } + barrier.mu.Unlock() if last { - close(release) + close(barrier.release) } else { select { - case <-release: + case <-barrier.release: case <-time.After(5 * time.Second): // Барьер не собрался — отпускаем, чтобы тест упал по - // существу, а не по таймауту всего прогона. + // существу (см. requireCollected), а не по таймауту всего + // прогона. } } @@ -70,71 +302,28 @@ func startBarrierTrafficStats(t *testing.T, online map[string]int64, hold int) { if err := dao.UpsertConfigValue(constant.Hysteria2TrafficStatsSecret, testTrafficStatsSecret); err != nil { t.Fatalf("не удалось записать секрет Traffic Stats API: %v", err) } + return barrier } -// Главная регрессия AUTH-03: при `online = max-1` разрешение обязан получить -// ровно ОДИН из двух одновременных запросов. -func TestHysteria2AuthHoldsDeviceLimitUnderConcurrency(t *testing.T) { - newTestDB(t) - // seedPeer создаёт пира с maxDevices = 3, поэтому online = 2 — это - // последнее свободное место. - startBarrierTrafficStats(t, map[string]int64{"alpha-auth-id": 2}, 2) - seedPeer(t, "alpha1", "alpha-auth-id") +// requireCollected требует, чтобы барьер действительно собрался. +// +// Без этой проверки тест проходил бы и при глобальном замке: барьер молча +// разошёлся бы по таймауту, а исходы авторизации остались бы прежними. +func (b *barrierTrafficStats) requireCollected(t *testing.T) { + t.Helper() - var wg sync.WaitGroup - results := make([]error, 2) - for i := range results { - wg.Add(1) - go func(idx int) { - defer wg.Done() - _, _, err := Hysteria2Auth("alpha1-secret") - results[idx] = err - }(i) - } - wg.Wait() - - allowed := 0 - for _, err := range results { - if err == nil { - allowed++ - } - } - if allowed != 1 { - t.Fatalf("на последнее свободное место допущено %d подключений из 2", allowed) - } -} - -// Свободных мест два — проходят оба: механизм не должен превращаться в -// сериализацию подключений. -func TestHysteria2AuthAdmitsBothWhenTwoSlotsFree(t *testing.T) { - newTestDB(t) - startBarrierTrafficStats(t, map[string]int64{"alpha-auth-id": 1}, 2) - seedPeer(t, "alpha1", "alpha-auth-id") - - var wg sync.WaitGroup - results := make([]error, 2) - for i := range results { - wg.Add(1) - go func(idx int) { - defer wg.Done() - _, _, err := Hysteria2Auth("alpha1-secret") - results[idx] = err - }(i) - } - wg.Wait() - - for idx, err := range results { - if err != nil { - t.Fatalf("подключение %d отклонено при двух свободных местах: %v", idx, err) - } + b.mu.Lock() + defer b.mu.Unlock() + if !b.collected { + t.Fatalf("запросы разных пиров не оказались в /online одновременно: пришло %d — авторизация сериализована глобально", b.arrived) } } // Резервации принадлежат КОНКРЕТНОМУ пиру: занятое место одного не должно -// закрывать доступ другому. +// закрывать доступ другому, а замок одного не должен задерживать другого. func TestDeviceAdmissionsAreIsolatedPerPeer(t *testing.T) { newTestDB(t) - startBarrierTrafficStats(t, map[string]int64{"alpha-auth-id": 2, "bravo-auth-id": 0}, 2) + barrier := startBarrierTrafficStats(t, map[string]int64{"alpha-auth-id": 2, "bravo-auth-id": 0}, 2) seedPeer(t, "alpha1", "alpha-auth-id") seedPeer(t, "bravo2", "bravo-auth-id") @@ -151,6 +340,8 @@ func TestDeviceAdmissionsAreIsolatedPerPeer(t *testing.T) { }() wg.Wait() + barrier.requireCollected(t) + if alphaErr != nil { t.Fatalf("первое подключение пира на последнее место отклонено: %v", alphaErr) } @@ -312,6 +503,143 @@ func TestReserveDeviceSlotForgetsIdlePeer(t *testing.T) { } } +// --- Замок последовательности «/online -> резервация» ------------------------ + +// Авторизации ОДНОГО пира исключают друг друга: только так снимок, прочитанный +// вторым, не может оказаться старше уже обработанного. +func TestPeerAdmissionGateSerializesSameAuthID(t *testing.T) { + first := lockPeerAdmission("gate-auth") + + entered := make(chan struct{}) + done := make(chan struct{}) + go func() { + defer close(done) + second := lockPeerAdmission("gate-auth") + close(entered) + second() + }() + + select { + case <-entered: + t.Fatal("второй вызывающий вошёл в критическую секцию при удерживаемом замке") + case <-time.After(100 * time.Millisecond): + } + + first() + + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("замок не освободился после возврата функции освобождения") + } +} + +// Разные пиры не мешают друг другу: замок именно по authId, а не один на +// процесс. Внутри него выполняется сетевой запрос, поэтому общий замок +// выстроил бы подключения всех пиров в одну очередь. +func TestPeerAdmissionGateDoesNotSerializeDifferentAuthIDs(t *testing.T) { + held := lockPeerAdmission("gate-alpha") + defer held() + + done := make(chan struct{}) + go func() { + defer close(done) + other := lockPeerAdmission("gate-bravo") + other() + }() + + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("замок одного пира задержал авторизацию другого") + } +} + +// Карта замков не растёт: запись живёт ровно столько, сколько есть желающие её +// взять. Без счётчика ссылок здесь появлялась бы строка на каждый когда-либо +// авторизовавшийся authId — та же утечка, от которой в учёте разрешений +// защищает forgetIfIdle. +func TestPeerAdmissionGateLeavesNoEntriesBehind(t *testing.T) { + if got := heldPeerAdmissionGates(); got != 0 { + t.Fatalf("перед проверкой уже удерживается %d замков", got) + } + + for i := 0; i < 50; i++ { + unlock := lockPeerAdmission("gate-transient") + unlock() + } + + var wg sync.WaitGroup + for i := 0; i < 20; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + unlock := lockPeerAdmission(fmt.Sprintf("gate-%d", idx%3)) + unlock() + }(i) + } + wg.Wait() + + if got := heldPeerAdmissionGates(); got != 0 { + t.Fatalf("после освобождения осталось %d замков", got) + } +} + +// --- Уборка учёта ------------------------------------------------------------ + +// Запись о пире, за которым не числится ни подключений, ни действующих +// разрешений, убирается. +// +// forgetIfIdle этого не делал: он срабатывает только на ветке отказа, а после +// успешной выдачи разрешения запись оставалась с непустым pending, и снять её +// было некому. В карте накапливались удалённые пиры и старые authId, +// переписанные импортом. +func TestSweepDeviceAdmissionsForgetsIdlePeers(t *testing.T) { + resetDeviceAdmissions() + t.Cleanup(resetDeviceAdmissions) + + now := time.Now() + if !reserveDeviceSlot(admissionAuthID, 0, 3, now) { + t.Fatal("разрешение отклонено при свободном месте") + } + if admissionEntries() != 1 { + t.Fatal("учёт не запомнил выданное разрешение") + } + + // Разрешение ещё действует — запись обязана остаться, даже если пира нет в + // `/online`: подключение может проявиться в любой момент. + sweepDeviceAdmissions(map[string]int64{}, now) + if admissionEntries() != 1 { + t.Fatal("уборка сняла действующее разрешение") + } + + // Разрешение протухло, подключений нет: помнить нечего. + sweepDeviceAdmissions(map[string]int64{}, now.Add(pendingAdmissionTTL+time.Second)) + if got := admissionEntries(); got != 0 { + t.Fatalf("запись о неактивном пире осталась: записей %d", got) + } +} + +// Пир, который сейчас на связи, из учёта не убирается: его состояние ещё +// участвует в решениях. +func TestSweepDeviceAdmissionsKeepsOnlinePeers(t *testing.T) { + resetDeviceAdmissions() + t.Cleanup(resetDeviceAdmissions) + + now := time.Now() + if !reserveDeviceSlot(admissionAuthID, 1, 3, now) { + t.Fatal("разрешение отклонено при свободном месте") + } + + sweepDeviceAdmissions( + map[string]int64{admissionAuthID: 1}, + now.Add(pendingAdmissionTTL+time.Second), + ) + if got := admissionEntries(); got != 1 { + t.Fatalf("уборка сняла запись подключённого пира: записей %d", got) + } +} + // Учёт выдерживает параллельный доступ и не выдаёт больше мест, чем есть. // Проверка ловит дефект и без детектора гонок: он виден по числу разрешений. func TestReserveDeviceSlotIsConcurrencySafe(t *testing.T) { diff --git a/docs/acceptance/2026-09-02-v1.0.0-rc3-preflight-findings.md b/docs/acceptance/2026-09-02-v1.0.0-rc3-preflight-findings.md new file mode 100644 index 0000000..5845ffe --- /dev/null +++ b/docs/acceptance/2026-09-02-v1.0.0-rc3-preflight-findings.md @@ -0,0 +1,274 @@ +# Разбор кода перед сборкой `1.0.0-rc3` + +Источник — не прогон на хосте, а разбор дерева на коммите `6d1686b8` и повторная +сверка Hysteria-интеграции с официальной документацией Hysteria 2. Разбор +проводился по состоянию **после** предыдущего прохода +([2026-09-01-v1.0.0-rc2-preflight-findings.md](2026-09-01-v1.0.0-rc2-preflight-findings.md)), +то есть проверялись выводы уже принятых исправлений, а не исходное состояние. + +Тема прохода — **вторая попытка**. Предыдущий проход сделал правильным порядок +«сначала долговременная запись, потом разрыв сессии» и правильно запретил откат +при неудаче разрыва. Но способа прийти к согласованному состоянию **потом** он +не дал: у двух операций повтор не работал вовсе, и их частичный результат +оставался навсегда. + +Проверить на живом сервере ещё предстоит: список ручных проверок — в конце +документа. + +## Сводка + +| ID | Дефект | Приоритет | Статус | +| --- | --- | --- | --- | +| ADMIT-04 | Устаревший снимок `/online` возвращал уже занятое место | P0 | закрыт | +| RECON-01 | Живая сессия без строки в базе не завершалась никогда | P0 | закрыт | +| RECON-02 | Снижение `maxDevices` после неудачного `/kick` не имело второй попытки | P1 | закрыт | +| ADMIT-05 | Учёт выданных разрешений рос неограниченно | P2 | закрыт | +| GATE-02 | Гейты приёмки не покрывали ни одного из новых инвариантов | P1 | закрыт | +| DOC-02 | Два места документации описывали снятую архитектуру | P3 | закрыт | +| VER-01 | `GO_VERSION` отставала на patch-релиз | P3 | закрыт | + +ADMIT-04 и RECON-01 найдены внешним разбором; RECON-02, ADMIT-05, GATE-02 и +вторая половина DOC-02 — при проверке его выводов по коду. + +--- + +## ADMIT-04 — устаревший снимок `/online` возвращал уже занятое место + +**Наблюдалось только рассуждением:** тестами это состояние не воспроизводилось, +а `go test -race` был и остаётся зелёным. + +Предыдущий проход закрыл сравнение двух ОДИНАКОВЫХ снимков: появился +process-local учёт выданных, но ещё не проявившихся разрешений +(`apps/service/peer_admission.go`). Сетевой запрос при этом по-прежнему +выполнялся **вне** блокировки — сознательно, чтобы не сериализовать подключения +всех пиров через один HTTP-обмен, — и это оставило вторую половину гонки +открытой: снимки приходили в резервацию в произвольном порядке. + +```text +1. A получает разрешение при /online = 0; pending = [A], lastOnline = 0 +2. B читает /online = 0 и задерживается на обратном пути +3. A подключается — Hysteria показывает /online = 1 +4. C читает /online = 1 и первым входит в резервацию: + online > lastOnline -> разрешение A признано проявившимся + lastOnline = 1, C получает отказ (верно) +5. B входит со своим устаревшим online = 0 +6. `online > lastOnline` ложно; следом безусловное lastOnline = online +7. lastOnline = 0, pending пуст -> 0 < 1 -> B ДОПУЩЕН +``` + +При `maxDevices = 1` подключений становится два — ровно тем способом, от +которого лимит и должен защищать. + +**Это не data race.** Все обращения к памяти корректно защищены мьютексом, +поэтому детектор гонок здесь молчит принципиально, и «`-race` зелёный» не +является свидетельством. Доказать свойство может только семантический тест. + +**Исправление.** Последовательность «прочитать `/online` → занять место» +выполняется под замком **по `authId`** (`lockPeerAdmission`). Не глобальный +мьютекс вокруг сети: внутри замка идёт HTTP-обмен, и общий замок выстроил бы +подключения всех пиров в одну очередь. Конкурируют только авторизации одного +пира, а их упорядоченность и есть требуемое свойство — снимок, прочитанный под +замком, не может оказаться старше уже обработанного. Время удержания ограничено +сверху таймаутом обращения к Traffic Stats API (3 с). + +Учёт разрешений при этом остаётся нужен: сериализация не устраняет задержку +между ответом `allow` и появлением клиента в `/online`. Механизмы закрывают +разные половины и работают вместе. + +Карта замков не растёт: запись живёт ровно столько, сколько есть желающие её +взять (счётчик ссылок). + +**Закреплено:** `TestHysteria2AuthRejectsStaleOnlineSnapshot` — воспроизводит +последовательность выше через удержание конкретного ответа `/online`; проверено, +что тест падает на коде без замка. Плюс `TestPeerAdmissionGate*` — исключение +одного `authId`, отсутствие сериализации разных, отсутствие утечки записей. + +--- + +## RECON-01 — живая сессия без строки в базе не завершалась никогда + +**Корневая причина — форма обхода в cron:** + +```go +peers, err := dao.ListPeer("auth_id in ?", chunk) +for _, peer := range peers { ... } +``` + +Обход шёл по НАЙДЕННЫМ строкам, поэтому `authId`, которому в базе ничего не +соответствует, молча выпадал. А именно он и остаётся единственным следом сессии +после неудавшегося второго шага: + +```text +DB: old-auth +импорт old-auth -> new-auth, COMMIT прошёл +/kick old-auth -> 500 +``` + +Повторить операцию в этом состоянии **невозможно**: `applyPeerImportEntry` +читает `replaced := authIDOf(existing)`, то есть повтор того же файла найдёт в +базе уже `new-auth` и разорвёт ЕГО. Старое значение после первой неудачи не +хранится нигде. Восстановить состояние переподключением тоже нельзя — +авторизация нового значения не знает, — а старая QUIC-сессия живёт своей жизнью +сколь угодно долго. То же самое даёт удаление пира, у которого не удался `/kick` +и следом всё-таки прошло удаление строки. + +Это ровно тот класс stale session, от которого весь предыдущий проход и должен +был защитить. + +**Исправление.** `enforcePeerAccess` обходит каждый `authId` из `/online`, а не +строки выборки. Идентификатор без строки в базе — orphan-сессия, и правильный +исход у неё тот же, что и у первой попытки: `/kick`. + +**Отдельно проверено, что отказ базы не рвёт сессии.** «Пира нет» и «прочитать +не удалось» — разные ответы; трактовка второго как первого отключила бы всех +подключённых пиров сразу при недоступной SQLite. Ошибка выборки прекращает цикл +до единого обращения к `/kick`. + +**Закреплено:** `TestCronKicksSessionWithoutPeerRow`, +`TestCronReconcilesSessionAfterFailedImportKick` (end-to-end: импорт `old→new`, +`/kick` 500, затем обычный цикл учёта рвёт `old`), +`TestCronSendsNoKickWhenPeerLookupFails`. + +--- + +## RECON-02 — снижение `maxDevices` после неудачного `/kick` не имело второй попытки + +```text +maxDevices: 5 -> 1 +DB update: OK +/kick: FAIL +``` + +Оператор повторяет сохранение формы. Панель при правке отправляет все поля, +включая `maxDevices`, но условие разрыва сравнивает + +```go +*peerDto.MaxDevices < *before.MaxDevices +``` + +то есть `1 < 1` → `false`. Второй `PATCH` возвращает успех, `/kick` больше не +вызывается, и пять подключённых клиентов продолжают работать при лимите 1. + +Для `disabled` повторяемость сделана специально (условие смотрит на +ЗАПРОШЕННОЕ состояние, а не на переход), для квоты и срока её обеспечивает cron +через `peerAccessDenied`. Лимит устройств в политику доступа не входит и входить +не должен — это свойство сессий, а не хранимого состояния пира, — поэтому +механизма схождения у него не было вовсе. + +**Исправление — не в условии `updateRequiresReconcile`.** Делать разрыв при +каждом сохранении формы нельзя: тогда любая правка пометки рвала бы сессии. +Схождение обеспечивает та же сверка живых сессий: `онлайн-устройств > +maxDevices` → `/kick`. Побочно это лечит и любое другое случайное превышение +лимита. + +**Число устройств взято из upstream-контракта, а не из предположения.** +Официальная документация Traffic Stats API: `/online` возвращает «*the number of +Hysteria client instances ("devices"), NOT the number of active proxy +connections*». Сравнение с `maxDevices` корректно. + +Предикат `peerSessionNeedsReconcile` **не является вторым экземпляром политики +доступа**: `disabled`, квота, срок и блокировка остаются целиком за +`peerAccessDenied`, и предикат его вызывает, а не повторяет. + +**Закреплено:** `TestCronKicksWhenOnlineExceedsMaxDevices`, +`TestCronDoesNotKickAtExactDeviceLimit` (граница), +`TestCronKicksPeerWithUnusableMaxDevices`, +`TestCronReconcilesSessionAfterFailedMaxDevicesReduction` (end-to-end). + +--- + +## ADMIT-05 — учёт выданных разрешений рос неограниченно + +`forgetIfIdle` вызывался **только на ветке отказа**. После успешной выдачи +запись оставалась с непустым списком разрешений, а когда разрешение протухало, +снять её было уже некому: следующего обращения к этому `authId` могло не быть +никогда. В карте копились удалённые пиры и старые идентификаторы, переписанные +импортом, — за время жизни процесса она только росла. + +**Исправление.** Уборка идёт по ФАКТИЧЕСКОЙ картине подключений в цикле учёта — +единственном месте продукта, где она известна целиком. Решение принимается не по +хранимому `lastOnline`: тот обновляется только на пути авторизации и у +отключившегося пира остаётся прежним сколь угодно долго. + +Удаление записи без действующих разрешений и без подключений не меняет ни одного +будущего решения: следующая резервация начнёт с чистой записи и придёт к тому же +ответу. + +**Закреплено:** `TestSweepDeviceAdmissionsForgetsIdlePeers`, +`TestSweepDeviceAdmissionsKeepsOnlinePeers`, +`TestCronSweepsAdmissionsOfOfflinePeers`. + +--- + +## GATE-02 — гейты приёмки не покрывали новых инвариантов + +`run_access_revocation_acceptance` проверял наличие `reserveDeviceSlot` и +`reconcileLiveSessions`, но ни порядок «замок → чтение `/online`», ни форму +обхода в cron. То есть исправления ADMIT-04 и RECON-01 можно было бы снять +следующим проходом, не уронив сборку. + +Добавлены три гейта: + +1. авторизация берёт замок **до** чтения `/online` и ключует его `*peer.AuthId` + (литеральный ключ означал бы один замок на процесс); +2. `enforcePeerAccess` обходит `authIDs` из `/online`, ветка `!found` + **завершает** сессию, а не пропускает её, отказ выборки прекращает цикл до + `/kick`, и уборка учёта вызывается; +3. `peerSessionNeedsReconcile` вызывает `peerAccessDenied` и не упоминает ни + одного поля политики доступа самостоятельно. + +Каждый гейт проверен в обе стороны: он проходит на исправленном коде и падает на +восстановленном состоянии «до». + +--- + +## DOC-02 — документация описывала снятую архитектуру + +Два места, а не одно: + +1. `docs/testing/11-3-target-and-runtime.md:40` — «traffic accounting/kick + ориентируются на systemd status». Прямо противоположно реализации после + предыдущего прохода и остальной документации; +2. `docs/admin/04-admin-panel.md` — «GET /online (вне блокировки: сеть не должна + сериализовать все подключения)». Верно описывало прежнее устройство и стало + ложным вместе с исправлением ADMIT-04. + +Второе найдено при закрытии первого: документация здесь входит в +acceptance-контракт, поэтому расхождение — не косметика. + +--- + +## VER-01 — `GO_VERSION` отставала на patch-релиз + +`GO_VERSION=1.26.7`; 1.26.8 вышел 2026-09-01 (fixes в cgo, компиляторе, runtime, +`debug/elf` и `os`). Stdlib целиком попадает в production-бинарь, поэтому «на +один патч позади» — свойство выпускаемого артефакта, а не среды сборки. + +Не архитектурный blocker и не security emergency; закрыто вместе с остальным, +раз проход всё равно затрагивает контракт версий. Major не менялся: линия 1.26 +поддерживается, переход на 1.27 ради номера не нужен. + +Обновлены `GO_VERSION`, `GO_LINUX_AMD64_SHA256` и `toolchain` в `apps/go.mod` — +расхождение между ними роняет сборку на `verify_go_toolchain_contract`. +Контрольная сумма взята из `https://go.dev/dl/?mode=json&include=all` и сверена +повторным независимым запросом. + +--- + +## Что осталось проверить на живом хосте + +Автоматика доказывает логику; следующие свойства наблюдаемы только на реальном +сервере с Hysteria и настоящими клиентами: + +1. подключить `maxDevices + 1` устройств одновременно и убедиться, что принято + ровно `maxDevices`; +2. снизить `maxDevices` при нескольких подключённых устройствах, оборвав + Traffic Stats API на время операции, и убедиться, что следующий цикл учёта + (не позднее 30 с) разрывает сессии; +3. импортировать пира с новым `auth_id`, оборвав `/kick`, и убедиться, что + старая сессия завершается следующим циклом; +4. удалить пира с оборванным `/kick`, затем восстановить API и убедиться, что + повтор удаления проходит целиком; +5. убедиться, что при недоступной SQLite (симуляция) ни одна сессия не рвётся; +6. `go test -race ./service/...` на релизном билдере: локально проверка + невыполнима, C-компилятора на машине разработчика нет. diff --git a/docs/acceptance/README.md b/docs/acceptance/README.md index 12ac614..56cd860 100644 --- a/docs/acceptance/README.md +++ b/docs/acceptance/README.md @@ -41,3 +41,4 @@ | Дата | Основание | Находки | | --- | --- | --- | | 2026-09-01 | коммит `c0a43ae9`, сверка с Hysteria 2 и Element Plus | [UX-06…UX-10, LOG-01…LOG-05, AUTH-01/02, CORE-01/02, TYPE-01](2026-09-01-v1.0.0-rc2-preflight-findings.md) | +| 2026-09-02 | коммит `6d1686b8`, повторная сверка с Hysteria 2 | [ADMIT-04/05, RECON-01/02, GATE-02, DOC-02, VER-01](2026-09-02-v1.0.0-rc3-preflight-findings.md) | diff --git a/docs/admin/04-admin-panel.md b/docs/admin/04-admin-panel.md index 7f5f3ef..dd499ab 100644 --- a/docs/admin/04-admin-panel.md +++ b/docs/admin/04-admin-panel.md @@ -694,6 +694,25 @@ access-changing», то есть второго места, где полити отвечает кодом `peer_disconnect_failed`, панель показывает его предупреждением и обновляет список. +**И не оставляет систему в этом состоянии навсегда.** Сообщить оператору о +частичном результате недостаточно: повторить второй шаг он может не всегда. + +```text +операция повтор той же операции после неудачного /kick +─────────────────────────────────────────────────────────────── +disabled = 1 работает: условие смотрит на ЗАПРОШЕННОЕ состояние +удаление работает: строка осталась с disabled = 1 +блокировка работает: cron видит banned_until через peerAccessDenied +квота / срок работает: cron видит их через peerAccessDenied +maxDevices ↓ НЕ работает: 1 < 1 -> false, разрыва больше не будет +импорт old→new НЕ работает: в базе уже new, повтор разорвёт ЕГО +``` + +Две нижние строки не имели механизма схождения вовсе, и обе закрывает cron — +см. «Сверка живых сессий» ниже. Отдельной таблицы retry, очереди отложенных +операций и хранимого «списка того, что не удалось разорвать» для этого не +нужно: `/online` и есть список живых сессий. + Формулировка сообщения **не называет конкретную операцию**: через этот код отчитываются все восемь строк таблицы выше, а для удалённого пира фраза «новые подключения пира запрещены» была бы просто бессмысленной. @@ -728,7 +747,7 @@ TryLock (пропустить тик, если предыдущий ещё ид ↓ GET /traffic?clear=1 → записать дельты в счётчики пиров ↓ -GET /online → применить peerAccessDenied → POST /kick +GET /online → сверить живые сессии → POST /kick ``` Порядок обязателен: enforcement принимает решение по счётчикам, значит счётчики @@ -750,6 +769,47 @@ Hysteria обнуляются сразу после отправки ответ не вводится. Квота здесь — операционный предел доступа, а не учёт с финансово значимым каждым байтом. +#### Сверка живых сессий + +Cron обходит **каждый `authId`, который Hysteria считает живым**, а не тех +пиров, которых удалось найти в базе. Разница между этими двумя формулировками и +есть то, что делает частичный результат обратимым. + +```text +для каждого authId из GET /online: + + выборка пиров не удалась → не рвать НИЧЕГО (цикл прекращается) + строки в базе нет → /kick (пир удалён либо переподписан) + peerAccessDenied → /kick (disabled / квота / срок / блокировка) + maxDevices непригоден → /kick (повреждённая граница — не «безлимит») + устройств > maxDevices → /kick (лимит снижен, сессии остались) +``` + +Прежний обход выглядел как `ListPeer("auth_id in ?") → range peers`, поэтому +идентификатор, которому в базе ничего не соответствует, **молча выпадал**. А +именно он и остаётся единственным следом сессии после неудачного второго шага +удаления или импорта: `auth_id` в строке уже заменён либо строки нет вовсе, и +восстановить состояние переподключением невозможно — авторизация нового +значения не знает, а старая сессия живёт своей жизнью. + +**Отказ базы не является основанием рвать сессии.** «Пира нет» и «прочитать не +удалось» — разные ответы, и трактовать второй как первый значит отключить всех +подключённых пиров сразу при недоступной SQLite. Ошибка выборки прекращает +цикл до единого обращения к `/kick`. + +**Число устройств берётся из upstream-контракта, а не из предположения.** +`GET /online` по официальной документации Traffic Stats API возвращает +количество экземпляров клиента Hysteria («устройства»), а не число proxy-потоков. + +Предикат живых сессий (`peerSessionNeedsReconcile`) **не является вторым +экземпляром политики доступа**: `disabled`, квота, срок и блокировка остаются +целиком за `peerAccessDenied`, и предикат его вызывает, а не повторяет. Своего +у него ровно одно — инвариант, которого в хранимом состоянии пира нет: сколько +устройств сейчас на связи. + +Побочное следствие того же обхода — уборка учёта выданных разрешений: цикл +учёта единственный в продукте знает фактическую картину подключений целиком. + ### Ограничение устройств проверяется fail-closed `maxDevices` проверяется по `/online` Traffic Stats API, который возвращает @@ -793,11 +853,13 @@ A: 2 < 3 -> allow B: 2 < 3 -> allow ```text 1. обычная проверка политики доступа -2. GET /online (вне блокировки: сеть не должна сериализовать все подключения) -3. снять протухшие разрешения -4. рост online означает, что столько же разрешений превратились в подключения -5. решение по сумме: online + выданные разрешения -6. свободно -> занять место и allow; иначе deny +2. взять замок ЭТОГО пира +3. GET /online +4. снять протухшие разрешения +5. рост online означает, что столько же разрешений превратились в подключения +6. решение по сумме: online + выданные разрешения +7. свободно -> занять место и allow; иначе deny +8. отпустить замок ``` Учёт **process-local**: HY2XS — один процесс на одном сервере с Hysteria, и ни @@ -807,10 +869,46 @@ Redis, ни таблицы в базе, ни распределённых бло клиента в статистике, а не политика доступа. Если клиент авторизовался и не подключился, резервация исчезает сама. +#### Снимки `/online` не переупорядочиваются + +Учёта разрешений самого по себе оказалось недостаточно, и это отдельный дефект, +а не оттенок предыдущего. Пока сетевой запрос выполнялся **вне** блокировки, +снимки приходили в резервацию в произвольном порядке, и более старый откатывал +учёт назад: + +```text +A получил разрешение при online = 0; pending = [A], lastOnline = 0 +B прочитал online = 0 и задержался на обратном пути +A подключился — Hysteria показывает online = 1 +C прочитал online = 1 и вошёл ПЕРВЫМ: + разрешение A признано проявившимся, lastOnline = 1, C отклонён +B входит со своим устаревшим 0 -> lastOnline снова 0 -> B ДОПУЩЕН +``` + +При `maxDevices = 1` подключений становилось два. Детектор гонок здесь +бесполезен **принципиально**: вся работа с памятью защищена мьютексом, и гонка +логическая, а не по памяти. Доказать такое свойство может только семантический +тест. + +Поэтому последовательность «прочитать `/online` → занять место» выполняется под +замком, и замок этот — **по `authId`, а не один на процесс**. Внутри него идёт +сетевой запрос: общий замок выстроил бы подключения всех пиров в очередь за +одним HTTP-обменом. Конкурируют только авторизации одного и того же пира, а их +упорядоченность и есть требуемое свойство. Время удержания ограничено сверху +таймаутом обращения к Traffic Stats API. + +Оба механизма нужны одновременно и закрывают разные половины: + +```text +замок по authId — снимки не переупорядочиваются +учёт разрешений — снимок не успевает измениться к следующему запросу +``` + Чего механизм не обещает: без обратного вызова от Hysteria «соединение установлено / не установлено» математически точной системы резервирования не -построить. Он закрывает конкретный и реальный случай — параллельные HTTP-auth -одного процесса — и делает это fail-closed. +построить. Он закрывает конкретные и реальные случаи — параллельные HTTP-auth +одного процесса — и делает это fail-closed. Случайное превышение лимита по +любой другой причине устраняет сверка живых сессий в цикле учёта. ### Что нельзя делать @@ -829,6 +927,15 @@ Redis, ни таблицы в базе, ни распределённых бло - обращаться к `/kick` мимо `disconnectAuthIDs`; - удалять пира, не запомнив его `authId` и не завершив сессию до удаления; - разрывать сессии импорта до `COMMIT` либо по новым `authId`; +- читать `/online` вне замка пира на пути авторизации: устаревший снимок + возвращает уже занятое место, и детектор гонок этого не показывает; +- заводить один замок авторизации на процесс: внутри него идёт сетевой запрос; +- пропускать в цикле учёта `authId`, которому в базе ничего не соответствует, — + это единственный след сессии после неудавшегося разрыва при удалении и + импорте; +- трактовать отказ базы как «пира нет» и рвать по нему сессии; +- заводить таблицу отложенных операций или очередь retry ради схождения: + список живых сессий уже есть, и это `/online`; - запускать работу джобы учёта в отсоединённых горутинах: планировщик обязан её видеть, иначе `StopCron()` вернётся раньше, чем она закончит; - считать квоту биллинговым учётом: чтение `/traffic?clear=1` деструктивно; @@ -1012,4 +1119,10 @@ Compatibility-ветка пережила слой совместимости, 21. удаление пира завершает его сессию до того, как исчезнет `authId` 22. импорт разрывает старые сессии после `COMMIT` и по старым `authId` 23. джоба учёта выполняется синхронно, и `StopCron()` её дожидается -24. лимит устройств не превышается параллельными запросами авторизации +24. лимит устройств не превышается параллельными запросами авторизации — в том + числе когда снимки `/online` приходят в обратном порядке +25. цикл учёта сверяет КАЖДУЮ живую сессию из `/online`, а не только тех пиров, + которых удалось найти в базе; отказ базы при этом не рвёт ничего +26. неудавшийся разрыв не оставляет систему в несогласованном состоянии + навсегда: сессия удалённого либо переподписанного пира и превышение + `maxDevices` устраняются очередным циклом учёта diff --git a/docs/build/02-build-layer-and-package.md b/docs/build/02-build-layer-and-package.md index f481606..20fc1ee 100644 --- a/docs/build/02-build-layer-and-package.md +++ b/docs/build/02-build-layer-and-package.md @@ -78,7 +78,7 @@ HY2XS_TARGET_OS=debian HY2XS_TARGET_OS_VERSION=13 HY2XS_TARGET_ARCH=amd64 -GO_VERSION=1.26.7 +GO_VERSION=1.26.8 GO_LINUX_AMD64_SHA256= BUN_VERSION=1.3.13 BUN_LINUX_X64_SHA256= @@ -117,8 +117,14 @@ Go здесь не просто сборщик: им компилируется Цифры, ради которых это записано. На `GO_VERSION=1.21.13` — линия, давно вне поддержки — `govulncheck ./...` находил **21 вызываемую уязвимость**, из них 17 в -одной только stdlib. После перехода на 1.26.7 и обновления графа зависимостей — -**ноль**. +одной только stdlib. После перехода на линию 1.26 и обновления графа +зависимостей — **ноль**. + +По той же причине здесь держится актуальный **patch**-релиз, а не просто +поддерживаемая линия: «на один патч позади» — свойство выпускаемого артефакта, +а не среды сборки. Смена patch-версии затрагивает два места сразу — +`GO_VERSION` с контрольной суммой здесь и `toolchain` в `apps/go.mod`, — и +расхождение между ними роняет сборку на `verify_go_toolchain_contract`. Node живёт только на build-хосте и в артефакт не попадает, но 20.x достигла EOL, то есть перестала получать security-обновления, а собирает она код, который diff --git a/docs/testing/11-3-target-and-runtime.md b/docs/testing/11-3-target-and-runtime.md index 8d1c25b..8fcf9d1 100644 --- a/docs/testing/11-3-target-and-runtime.md +++ b/docs/testing/11-3-target-and-runtime.md @@ -37,13 +37,15 @@ 17. `nft -c -f /etc/nftables.conf` проходит после apply 18. пароль admin и `con_pass` не перезаписываются при рестарте `hy2xs-admin` 19. остановка/рестарт UI не останавливает `hysteria-server` -20. traffic accounting/kick ориентируются на systemd status; ключа `HYSTERIA2_ENABLE` в базе больше нет +20. traffic accounting/kick обращаются к Traffic Stats API напрямую и **не** используют systemd status как гейт принятия решений; ключа `HYSTERIA2_ENABLE` в базе больше нет 21. `/etc/hysteria/config.yaml` имеет `0640 hysteria:hy2xs-admin` 22. `hy2xs-admin` может читать `/etc/hysteria/config.yaml`, но не может писать 23. смена расписания сброса трафика применяется **без** перезапуска `hy2xs-admin`, и число джоб планировщика не растёт 24. невалидное cron-выражение отклоняется API, а значение в базе не меняется 25. `systemctl restart hy2xs-admin` завершает сервис штатно: планировщик остановлен до закрытия SQLite, в журнале нет `database is closed` 26. `govulncheck ./...` на графе релиза не находит вызываемых уязвимостей +27. живая сессия, которой в базе больше ничего не соответствует (пир удалён либо его `auth_id` заменён импортом, а разрыв в тот момент не удался), завершается очередным циклом учёта — не позднее 30 секунд +28. превышение `maxDevices` живыми сессиями устраняется тем же циклом: после неудавшегося разрыва при снижении лимита повтор формы даёт успех без `/kick`, и единственный механизм схождения здесь — cron ## C1. Семантический smoke конфига diff --git a/tools/build/lib/acceptance.sh b/tools/build/lib/acceptance.sh index 0a956bd..7fd62c4 100644 --- a/tools/build/lib/acceptance.sh +++ b/tools/build/lib/acceptance.sh @@ -1688,6 +1688,131 @@ run_access_revocation_acceptance() { code_has apps/service/hysteria2_api.go -F -- 'reserveDeviceSlot(' \ || fail "acceptance: auth does not reserve a device slot" + log_step "Acceptance: the online snapshot cannot be reordered against admission" + # Учёта выданных разрешений одного НЕДОСТАТОЧНО. Сетевой запрос выполнялся + # вне мьютекса, поэтому снимки приходили в резервацию в произвольном порядке, + # и более старый откатывал `lastOnline` назад: + # + # A получил разрешение при online = 0 + # C обработал online = 1 первым: разрешение A признано проявившимся + # B пришёл со своим устаревшим 0 -> место снова «свободно» -> ALLOW + # + # Детектор гонок здесь молчит принципиально: вся работа с памятью защищена + # мьютексом, гонка логическая. Единственная защита — замок по authID вокруг + # ВСЕЙ последовательности «прочитать /online -> занять место». + code_has apps/service/peer_admission.go -F -- 'func lockPeerAdmission' \ + || fail "acceptance: the per-peer admission gate is missing" + "$BUN_BIN" -e ' + const source = require("node:fs").readFileSync("apps/service/hysteria2_api.go", "utf8"); + const start = source.indexOf("func Hysteria2Auth"); + if (start < 0) throw new Error("Hysteria2Auth is missing"); + const rest = source.slice(start + 1); + const end = rest.indexOf("\nfunc "); + const body = (end < 0 ? rest : rest.slice(0, end)) + .split("\n") + .filter((line) => !/^\s*\/\//.test(line)) + .join("\n"); + + const gate = body.indexOf("lockPeerAdmission("); + const online = body.indexOf("hysteria2Online()"); + const reserve = body.indexOf("reserveDeviceSlot("); + if (gate < 0) throw new Error("auth does not take the admission gate"); + if (online < 0 || reserve < 0) throw new Error("auth no longer reads /online or reserves a slot"); + // Замок обязан быть взят ДО чтения статистики: взятый после него он + // защищал бы только резервацию, а переупорядочиваются именно снимки. + if (gate > online) throw new Error("the admission gate is taken after the /online read"); + if (!/defer\s+\w+\(\)/.test(body)) throw new Error("the admission gate is never released"); + // Замок именно по authId. Литеральный ключ означал бы один замок на + // процесс, то есть очередь из подключений ВСЕХ пиров за одним HTTP-обменом. + if (!/lockPeerAdmission\(\*peer\.AuthId\)/.test(body)) { + throw new Error("the admission gate is not keyed by the peer auth id"); + } + ' || fail "acceptance: the /online read and the reservation must be one serialized sequence" + + log_step "Acceptance: cron reconciles every live session, not only known peers" + # Прежний обход шёл по НАЙДЕННЫМ пирам, поэтому authID, которого нет в базе, + # молча выпадал: `dao.ListPeer("auth_id in ?")` просто не возвращала строку. + # Так после неудавшегося второго шага удаления или импорта, заменившего + # `auth_id`, живая сессия оставалась навсегда — восстановить её + # переподключением уже нельзя, авторизация нового значения не знает. + "$BUN_BIN" -e ' + const source = require("node:fs").readFileSync("apps/service/cron.go", "utf8"); + const start = source.indexOf("func enforcePeerAccess"); + if (start < 0) throw new Error("enforcePeerAccess is missing"); + const rest = source.slice(start + 1); + const end = rest.indexOf("\nfunc "); + const body = (end < 0 ? rest : rest.slice(0, end)) + .split("\n") + .filter((line) => !/^\s*\/\//.test(line)) + .join("\n"); + + // Обход идёт по идентификаторам из `/online`, а не по строкам выборки. + if (!/for\s+_,\s*authID\s*:=\s*range\s+authIDs\s*\{/.test(body)) { + throw new Error("enforcePeerAccess no longer walks the online auth ids"); + } + // Мало отличить сессию без строки в базе — её надо ЗАВЕРШИТЬ. Прежний код + // ровно на этом месте делал молчаливый пропуск, поэтому ветка проверяется + // по содержимому, а не по факту существования. + const orphan = body.indexOf("if !found {"); + if (orphan < 0) { + throw new Error("enforcePeerAccess does not distinguish a session without a peer row"); + } + let depth = 0; + let orphanEnd = orphan; + for (let i = body.indexOf("{", orphan); i < body.length; i++) { + if (body[i] === "{") depth++; + else if (body[i] === "}") { + depth--; + if (depth === 0) { orphanEnd = i; break; } + } + } + if (!/kick\s*=\s*append\(/.test(body.slice(orphan, orphanEnd + 1))) { + throw new Error("a live session without a peer row is skipped instead of terminated"); + } + if (!body.includes("peerSessionNeedsReconcile(")) { + throw new Error("enforcePeerAccess does not apply the live-session predicate"); + } + // Отказ базы не даёт права рвать сессии: недоступная SQLite иначе + // отключила бы всех подключённых пиров сразу. + const lookup = body.indexOf("dao.ListPeer("); + const kickCall = body.indexOf("disconnectAuthIDs("); + if (lookup < 0 || kickCall < 0) throw new Error("enforcePeerAccess lost its lookup or its disconnect"); + const between = body.slice(lookup, kickCall); + if (!/if\s+err\s*!=\s*nil\s*\{\s*return\s+err/.test(between)) { + throw new Error("a storage failure no longer stops enforcement before /kick"); + } + if (!body.includes("sweepDeviceAdmissions(")) { + throw new Error("the admission tracker is never swept against the real online picture"); + } + ' || fail "acceptance: cron must reconcile every live session reported by /online" + + log_step "Acceptance: the live-session predicate does not duplicate the access policy" + # peerAccessDenied остаётся единственным владельцем disabled/quota/expiry/ban. + # Своего у предиката ровно одно — число подключённых устройств, которого в + # хранимом состоянии пира нет. + code_has apps/service/peer_access.go -F -- 'func peerSessionNeedsReconcile' \ + || fail "acceptance: the live-session predicate is missing" + "$BUN_BIN" -e ' + const source = require("node:fs").readFileSync("apps/service/peer_access.go", "utf8"); + const start = source.indexOf("func peerSessionNeedsReconcile"); + if (start < 0) throw new Error("peerSessionNeedsReconcile is missing"); + const rest = source.slice(start + 1); + const end = rest.indexOf("\nfunc "); + const body = (end < 0 ? rest : rest.slice(0, end)) + .split("\n") + .filter((line) => !/^\s*\/\//.test(line)) + .join("\n"); + + if (!body.includes("peerAccessDenied(")) { + throw new Error("the live-session predicate does not reuse the access policy"); + } + for (const field of ["Disabled", "QuotaBytes", "DownloadBytes", "UploadBytes", "ExpiresAt", "BannedUntil"]) { + if (body.includes("peer." + field)) { + throw new Error("the access policy is duplicated inside the live-session predicate: " + field); + } + } + ' || fail "acceptance: the live-session predicate must not restate peerAccessDenied" + log_step "Acceptance: deleting a peer revokes access before removing the row" # `return dao.DeletePeer(...)` убирал строку вместе с auth_id — то есть # вместе с единственным, чем можно было бы завершить живую сессию. diff --git a/versions.env b/versions.env index 8658168..f735137 100644 --- a/versions.env +++ b/versions.env @@ -46,9 +46,15 @@ HY2XS_TARGET_ARCH=amd64 # более новые), а не по удобству. # # Было 1.21.13 — линия вне поддержки. На ней `govulncheck ./...` находил 21 -# ВЫЗЫВАЕМУЮ уязвимость, из них 17 в stdlib. На 1.26.7 — ноль. -GO_VERSION=1.26.7 -GO_LINUX_AMD64_SHA256=ffb5f8de10c62550dfddab66b36b57030721e0a44a3218e9e1181d7b59f121ca +# ВЫЗЫВАЕМУЮ уязвимость, из них 17 в stdlib. На 1.26.x — ноль. +# +# Patch-релиз держится актуальным по той же причине: stdlib целиком попадает в +# бинарь, поэтому «на один патч позади» — это свойство выпускаемого артефакта, +# а не среды сборки. 1.26.8 вышел 2026-09-01 (fixes в cgo, компиляторе, +# runtime, `debug/elf` и `os`); major при этом не меняется — линия 1.26 +# поддерживается, и переход на 1.27 ради номера не нужен. +GO_VERSION=1.26.8 +GO_LINUX_AMD64_SHA256=d0f743b33e8d8945e6b1f432edd15785c70507121d6e2a723b21285eddf8b57b # Bun выбирает артефакт по наличию AVX2, поэтому одной контрольной суммы # архитектурно недостаточно: обе должны быть зафиксированы заранее.