security: phase D pre-release review — pass; harden saslpasswd2 argv
Fable review of the full diff from the v1.0 audit (Phase 11, bd64e80) to
HEAD plus a complete pass over the docs/security.md checklist (former spec
7.6). No exploitable findings. One defence-in-depth fix: the application
login is passed to saslpasswd2 behind a -- end-of-options marker so a
login starting with - can never be parsed as a flag. Accepted risks
unchanged; plan § D closed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -5,6 +5,16 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
|
|||||||
|
|
||||||
## [Unreleased]
|
## [Unreleased]
|
||||||
|
|
||||||
|
### Security
|
||||||
|
|
||||||
|
- Pre-release security review (plan § D, model Fable, 2026-08-06): full pass
|
||||||
|
over the diff from the v1.0 audit (Phase 11, `bd64e80`) to HEAD plus the
|
||||||
|
complete spec 7.6 checklist. No exploitable findings; one defence-in-depth
|
||||||
|
fix below. Accepted risks in `docs/security.md` unchanged.
|
||||||
|
- `saslpasswd2` argv: the application login is now passed after a `--`
|
||||||
|
end-of-options marker (`internal/app/sasl.go`), so a login starting with
|
||||||
|
`-` (legal under the whitelist) can never be parsed as a flag by getopt.
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|
||||||
- docs: `docs/code-review.md` — phase 1.5 plan for optional password encryption
|
- docs: `docs/code-review.md` — phase 1.5 plan for optional password encryption
|
||||||
|
|||||||
+17
-25
@@ -1,6 +1,7 @@
|
|||||||
# План реализации: SelfPost
|
# План реализации: SelfPost
|
||||||
|
|
||||||
**Статус:** для линии v1.0/v1.x до тега релиза остаётся **один пункт** (ниже).
|
**Статус:** линия v1.0/v1.x до тега релиза **закрыта** — предрелизная ревизия
|
||||||
|
безопасности (§ D) выполнена 2026-08-06, релизный гейт (e2e + ревизия) открыт.
|
||||||
B.1–B.3 и C.4 закрыты — as-built в [architecture.md](architecture.md),
|
B.1–B.3 и C.4 закрыты — as-built в [architecture.md](architecture.md),
|
||||||
e2e/CI в [development.md](development.md), принятые риски в
|
e2e/CI в [development.md](development.md), принятые риски в
|
||||||
[security.md](security.md). Комплексное рецензирование кодовой базы и план
|
[security.md](security.md). Комплексное рецензирование кодовой базы и план
|
||||||
@@ -11,29 +12,20 @@ e2e/CI в [development.md](development.md), принятые риски в
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## D. Предрелизная ревизия безопасности
|
## D. Предрелизная ревизия безопасности — ВЫПОЛНЕНО (2026-08-06)
|
||||||
|
|
||||||
**Проверка на уязвимости моделью Fable** — после B.1–B.3 и C.4, **до тега
|
**Проверка моделью Fable** по дифу от аудита v1.0 (Фаза 11, `bd64e80` — тега
|
||||||
релиза**. Не выполнено.
|
`v1.0.0` в репозитории нет, это его фактический эквивалент) до HEAD, плюс
|
||||||
|
полный повторный проход по чек-листу безопасности (бывшее ТЗ 7.6, теперь
|
||||||
|
[security.md](security.md)). Приоритеты из плана покрыты: аутентификация и
|
||||||
|
сессии, валидация ввода, запись в конфиги/map-файлы, `os/exec`, права в
|
||||||
|
`/data`, секреты.
|
||||||
|
|
||||||
**Почему один проход по итоговому состоянию, а не по каждому пункту.** B.1–C.4
|
**Результат.** Эксплуатируемых уязвимостей (high/medium) не найдено. Одна
|
||||||
меняли одну и ту же поверхность (сессии в SQLite, logrotate + `postfix reload`,
|
находка defence-in-depth закрыта правкой до тега: логин приложения передаётся
|
||||||
гейт `SELFPOST_HOSTNAME`, e2e-override с ослабленными настройками). Срезы по
|
в `saslpasswd2` после `--`, чтобы значение, начинающееся с `-` (допустимо
|
||||||
отдельным коммитам не заменяют просмотр дифа от `v1.0.0` до HEAD.
|
whitelist'ом), не могло быть разобрано getopt как флаг
|
||||||
|
([internal/app/sasl.go](../internal/app/sasl.go)). Принятые риски в
|
||||||
**Объём.** Диф от тега `v1.0.0` до состояния перед следующим тегом — вместе с
|
[security.md](security.md) не пополнились — существующие записи (origin-check
|
||||||
Фазами 12–14, которых в аудите v1.0 не было — плюс повторный проход по чек-листу
|
fallback, отсутствие CSRF-токенов, send-log gap) покрывают всё найденное.
|
||||||
ТЗ 7.6 целиком, а не только по изменённым строкам. Приоритет: аутентификация и
|
Сводка ревизии — в записи `Security` CHANGELOG `[Unreleased]`.
|
||||||
сессии, валидация ввода, запись в конфиги и map-файлы (injection), `os/exec` без
|
|
||||||
shell, права на файлы в `/data`, обращение с секретами (пароли приложений,
|
|
||||||
`sasldb2`, архив бэкапа).
|
|
||||||
|
|
||||||
**Модель — Fable** (не Opus): код писал Opus, проверка собственной работы
|
|
||||||
систематически слабее независимой. Правило progress.md «безопасность/инфра →
|
|
||||||
Opus» — про написание; здесь ревизия. Форма: `/security-review` по изменениям
|
|
||||||
(скилл смотрит диф) + ручной проход по 7.6.
|
|
||||||
|
|
||||||
**Гейт.** Вместе с e2e (C.4, готов): до тега. Находка класса «эксплуатируется
|
|
||||||
снаружи» откладывает тег. Каждая находка закрывается явно: правка до тега либо
|
|
||||||
запись в [security.md](security.md) как принятый риск с обоснованием. «Посмотрели
|
|
||||||
и ладно» — не закрытие.
|
|
||||||
|
|||||||
+2
-1
@@ -49,7 +49,8 @@
|
|||||||
- **C.4 реализован** (не выкачен на прод — это CI/тестовая инфраструктура, а не образ): герметичный контейнерный e2e отдельным Go-модулем `test/e2e/` (свой `go.mod`, не подхватывается `go test ./...` основного модуля) поверх поставляемого `deploy/docker-compose.yml` плюс `test/e2e/compose.override.yml` (самоподписанный сертификат, `PANEL_COOKIE_SECURE=false`, `SELFPOST_HOSTNAME=mail.e2e.test`, высокие порты `20465/20587/20080`, изолированный compose-проект `selfpost-e2e`, свой `--project-directory` — прод на том же хосте не задет). Герметичная почта: CoreDNS (`test/e2e/dns/Corefile` — авторитетна только для `e2e.test`, `file`-плагин с саб-директивой `reload` перечитывает `db.zone` по mtime, без сигналов) плюс `smtp-sink` из пакета postfix (`test/e2e/sink/`) как sink-MX. Сценарий (`test/e2e/*_test.go`): старт контейнера → все supervisord-программы `RUNNING` (`postfix-reload` — `STOPPED`) → токен из `/data/setup-token` → setup → login → добавление домена → DKIM-запись **скраплена со страницы панели** и опубликована в фейковую зону → добавление приложения → SMTP AUTH на 465 → письмо на sink → DKIM-подпись проверена (`go-msgauth/dkim` с кастомным `LookupTXT` через CoreDNS) против ключа **из DNS**, не из панели напрямую → send-log `queued → sent`. Негативы: без AUTH, relay на чужой домен без AUTH, sender/login mismatch (`reject_sender_login_mismatch` репортится Postfix'ом на RCPT, не MAIL — `smtpd_delay_reject=yes` по умолчанию), L1-лимит (anvil, override `RATE_LIMIT_MESSAGES_PER_IP=50` — специально высокий, чтобы остальные под-тесты не расходовали общий бюджет по IP раньше времени; сам тест шлёт до 60 раз, ждёт отказа), L2-лимит через панель (домен/приложение → `rejected`-строка в send-log), fail-open journal-milter'а (`supervisorctl stop panel`, письмо всё равно принято, контейнер жив), пустой/синтаксически неверный `SELFPOST_HOSTNAME` (отдельный один-разовый контейнер, не общий стенд), сессия переживает `docker restart` (плюс явное ожидание готовности smtps-порта после рестарта — панель и Postfix поднимаются независимо). `make e2e` — локальный/dev-server прогон. Найдено и исправлено по ходу стендовой проверки: `reload` — саб-директива `file`-плагина CoreDNS, а не отдельный топ-левел плагин (топ-левел `reload` следит за самим Corefile, не за зоной); `docker compose build.context` резолвится относительно `--project-directory`, а не относительно файла, где объявлен; `smtp-sink` отказывается стартовать от root без `-u`; `html/template` эскейпит `+` в `+` даже в тексте — скрапер значений со страницы обязан `html.UnescapeString`; проверки состояния сразу после `up`/`restart` должны поллиться, а не разово опрашиваться (supervisord/postfix поднимаются не мгновенно). **Проверено на dev-сервере (`selfpost.mixfed.ru`)**: `make e2e` — зелёный (`go vet`/`gofmt -l` тоже чистые в обоих модулях). `release.yml` переработан: job `prepare` (версия из тега) → матрица `[ubuntu-latest, ubuntu-24.04-arm]` — каждая нативно собирает образ (`--load`), прогоняет e2e, пушит тег `X.Y.Z-amd64`/`X.Y.Z-arm64` → job `merge` — `docker buildx imagetools create` в единый тег `X.Y.Z`; `setup-qemu-action` убран. Не проверено вживую (нельзя без реального тега): сам workflow на GitHub Actions — синтаксис вычитан, логика идентична локальному `make e2e` пути.
|
- **C.4 реализован** (не выкачен на прод — это CI/тестовая инфраструктура, а не образ): герметичный контейнерный e2e отдельным Go-модулем `test/e2e/` (свой `go.mod`, не подхватывается `go test ./...` основного модуля) поверх поставляемого `deploy/docker-compose.yml` плюс `test/e2e/compose.override.yml` (самоподписанный сертификат, `PANEL_COOKIE_SECURE=false`, `SELFPOST_HOSTNAME=mail.e2e.test`, высокие порты `20465/20587/20080`, изолированный compose-проект `selfpost-e2e`, свой `--project-directory` — прод на том же хосте не задет). Герметичная почта: CoreDNS (`test/e2e/dns/Corefile` — авторитетна только для `e2e.test`, `file`-плагин с саб-директивой `reload` перечитывает `db.zone` по mtime, без сигналов) плюс `smtp-sink` из пакета postfix (`test/e2e/sink/`) как sink-MX. Сценарий (`test/e2e/*_test.go`): старт контейнера → все supervisord-программы `RUNNING` (`postfix-reload` — `STOPPED`) → токен из `/data/setup-token` → setup → login → добавление домена → DKIM-запись **скраплена со страницы панели** и опубликована в фейковую зону → добавление приложения → SMTP AUTH на 465 → письмо на sink → DKIM-подпись проверена (`go-msgauth/dkim` с кастомным `LookupTXT` через CoreDNS) против ключа **из DNS**, не из панели напрямую → send-log `queued → sent`. Негативы: без AUTH, relay на чужой домен без AUTH, sender/login mismatch (`reject_sender_login_mismatch` репортится Postfix'ом на RCPT, не MAIL — `smtpd_delay_reject=yes` по умолчанию), L1-лимит (anvil, override `RATE_LIMIT_MESSAGES_PER_IP=50` — специально высокий, чтобы остальные под-тесты не расходовали общий бюджет по IP раньше времени; сам тест шлёт до 60 раз, ждёт отказа), L2-лимит через панель (домен/приложение → `rejected`-строка в send-log), fail-open journal-milter'а (`supervisorctl stop panel`, письмо всё равно принято, контейнер жив), пустой/синтаксически неверный `SELFPOST_HOSTNAME` (отдельный один-разовый контейнер, не общий стенд), сессия переживает `docker restart` (плюс явное ожидание готовности smtps-порта после рестарта — панель и Postfix поднимаются независимо). `make e2e` — локальный/dev-server прогон. Найдено и исправлено по ходу стендовой проверки: `reload` — саб-директива `file`-плагина CoreDNS, а не отдельный топ-левел плагин (топ-левел `reload` следит за самим Corefile, не за зоной); `docker compose build.context` резолвится относительно `--project-directory`, а не относительно файла, где объявлен; `smtp-sink` отказывается стартовать от root без `-u`; `html/template` эскейпит `+` в `+` даже в тексте — скрапер значений со страницы обязан `html.UnescapeString`; проверки состояния сразу после `up`/`restart` должны поллиться, а не разово опрашиваться (supervisord/postfix поднимаются не мгновенно). **Проверено на dev-сервере (`selfpost.mixfed.ru`)**: `make e2e` — зелёный (`go vet`/`gofmt -l` тоже чистые в обоих модулях). `release.yml` переработан: job `prepare` (версия из тега) → матрица `[ubuntu-latest, ubuntu-24.04-arm]` — каждая нативно собирает образ (`--load`), прогоняет e2e, пушит тег `X.Y.Z-amd64`/`X.Y.Z-arm64` → job `merge` — `docker buildx imagetools create` в единый тег `X.Y.Z`; `setup-qemu-action` убран. Не проверено вживую (нельзя без реального тега): сам workflow на GitHub Actions — синтаксис вычитан, логика идентична локальному `make e2e` пути.
|
||||||
- **Документация:** план D1–D9 закрыт ([documentation-plan.md](documentation-plan.md) — только метод и правила поддержки). Хвост v1.x (Codeberg в Quick start, тег образа, `docs/logo`) — [roadmap.md](roadmap.md) § «v1.x — хвост документации и деплоя».
|
- **Документация:** план D1–D9 закрыт ([documentation-plan.md](documentation-plan.md) — только метод и правила поддержки). Хвост v1.x (Codeberg в Quick start, тег образа, `docs/logo`) — [roadmap.md](roadmap.md) § «v1.x — хвост документации и деплоя».
|
||||||
- **Рецензирование кодовой базы** (2026-08-05): [code-review.md](code-review.md) — 10 разделов (архитектура, качество, docs, GUI, legacy, риски), приоритизированный план реализации и маршрутизация моделей. Критичных багов не найдено; блокер релиза — § D ниже.
|
- **Рецензирование кодовой базы** (2026-08-05): [code-review.md](code-review.md) — 10 разделов (архитектура, качество, docs, GUI, legacy, риски), приоритизированный план реализации и маршрутизация моделей. Критичных багов не найдено; блокер релиза — § D ниже.
|
||||||
- **Дальше:** [implementation-plan.md](implementation-plan.md) § D — предрелизная проверка на уязвимости моделью Fable по всему дифу от `v1.0.0` плюс повторный проход по ТЗ 7.6; вместе с e2e (готов) это гейт перед тегом релиза. Остальные пункты из [code-review.md](code-review.md) — polish (фазы 1–3).
|
- **§ D выполнен (2026-08-06):** предрелизная ревизия безопасности моделью Fable — диф от аудита v1.0 (Фаза 11, `bd64e80`) до HEAD + полный проход по чек-листу [security.md](security.md) (бывшее ТЗ 7.6). Эксплуатируемых находок нет; одна правка defence-in-depth (`--` перед логином в argv `saslpasswd2`, `internal/app/sasl.go` + тест). Принятые риски не пополнились. Детали — [implementation-plan.md](implementation-plan.md) § D и CHANGELOG `[Unreleased]/Security`. Локально `go vet`/`go test ./internal/app/...` чистые; падения `internal/domain` (`TestWriteLoadPrivateKeyRoundtrip`, `TestRenderTables`) и `internal/logtail` (`TestFollowTailsAndRotates`) — Windows-специфика (права файлов/`\` в путях/rename открытого файла), на Linux CI зелено.
|
||||||
|
- **Дальше:** релизный гейт открыт (e2e C.4 + ревизия § D) — по явной команде пользователя: резка версии в CHANGELOG, тег, пуш образа (workflow release.yml). Остальные пункты из [code-review.md](code-review.md) — polish (фазы 1–3).
|
||||||
- **Принятые риски** — [security.md](security.md). **Опционально v1.x / 2.x** — [roadmap.md](roadmap.md) (хвост документации, send-log gaps, Фаза O1+, роль администратора домена).
|
- **Принятые риски** — [security.md](security.md). **Опционально v1.x / 2.x** — [roadmap.md](roadmap.md) (хвост документации, send-log gaps, Фаза O1+, роль администратора домена).
|
||||||
- **Прод:** `selfpost.mixfed.ru`, реальный Let's Encrypt сертификат, живой e2e (DKIM/SPF pass). Контейнер там всё ещё на образе v1.0 — Фаза 14 в него не выкатывалась. При апгрейде: админа один раз разлогинит (сменилось имя cookie), а от reverse-proxy требуется передача исходного `Host` (Apache-фрагмент из `deploy/` это делает).
|
- **Прод:** `selfpost.mixfed.ru`, реальный Let's Encrypt сертификат, живой e2e (DKIM/SPF pass). Контейнер там всё ещё на образе v1.0 — Фаза 14 в него не выкатывалась. При апгрейде: админа один раз разлогинит (сменилось имя cookie), а от reverse-proxy требуется передача исходного `Host` (Apache-фрагмент из `deploy/` это делает).
|
||||||
|
|
||||||
|
|||||||
+6
-2
@@ -1,8 +1,12 @@
|
|||||||
# Безопасность
|
# Безопасность
|
||||||
|
|
||||||
**Что здесь.** (1) **Обязательные требования** — чеклист, который v1.0 обязан
|
**Что здесь.** (1) **Обязательные требования** — чеклист, который v1.0 обязан
|
||||||
выполнять; полный аудит на v1.0 пройден. (2) **Принятые риски** — сознательные
|
выполнять; полный аудит на v1.0 пройден. Предрелизная ревизия (план § D,
|
||||||
отступления сверх обязательного, чтобы решение не потерялось.
|
модель Fable, 2026-08-06) прошла по всему дифу от аудита v1.0 (Фаза 11) до
|
||||||
|
HEAD и по чек-листу целиком: эксплуатируемых находок нет; одна правка
|
||||||
|
defence-in-depth — `--` перед логином в argv `saslpasswd2`
|
||||||
|
([internal/app/sasl.go](../internal/app/sasl.go)). (2) **Принятые риски** —
|
||||||
|
сознательные отступления сверх обязательного, чтобы решение не потерялось.
|
||||||
|
|
||||||
Hardening сверх обязательного (security-заголовки, проверка origin, cookie
|
Hardening сверх обязательного (security-заголовки, проверка origin, cookie
|
||||||
`__Host-` с обнаружением дублей — Фаза 14) закрыт; история — в
|
`__Host-` с обнаружением дублей — Фаза 14) закрыт; история — в
|
||||||
|
|||||||
@@ -50,7 +50,10 @@ func (s *SASLDB) Set(login, password string) error {
|
|||||||
// -c: create the account / set the password.
|
// -c: create the account / set the password.
|
||||||
// -f: operate on our sasldb2 rather than the system default path.
|
// -f: operate on our sasldb2 rather than the system default path.
|
||||||
// -u: the realm the account lives under.
|
// -u: the realm the account lives under.
|
||||||
args := []string{"-p", "-c", "-f", s.path, "-u", s.realm, login}
|
// --: end of options, so a login can never be parsed as a flag (the
|
||||||
|
// whitelist already forbids nothing that getopt would eat, but a login
|
||||||
|
// starting with '-' is legal there — this keeps it an operand).
|
||||||
|
args := []string{"-p", "-c", "-f", s.path, "-u", s.realm, "--", login}
|
||||||
if err := s.run(args, []byte(password)); err != nil {
|
if err := s.run(args, []byte(password)); err != nil {
|
||||||
return fmt.Errorf("saslpasswd2 set %q: %w", login, err)
|
return fmt.Errorf("saslpasswd2 set %q: %w", login, err)
|
||||||
}
|
}
|
||||||
@@ -63,8 +66,8 @@ func (s *SASLDB) Delete(login string) error {
|
|||||||
if err := validateLogin(login); err != nil {
|
if err := validateLogin(login); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
// -d: delete the account.
|
// -d: delete the account. "--" as in Set: the login is always an operand.
|
||||||
args := []string{"-d", "-f", s.path, "-u", s.realm, login}
|
args := []string{"-d", "-f", s.path, "-u", s.realm, "--", login}
|
||||||
if err := s.run(args, nil); err != nil {
|
if err := s.run(args, nil); err != nil {
|
||||||
return fmt.Errorf("saslpasswd2 delete %q: %w", login, err)
|
return fmt.Errorf("saslpasswd2 delete %q: %w", login, err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -39,8 +39,9 @@ func TestSASLSetPassesPasswordOnStdinNotArgv(t *testing.T) {
|
|||||||
if strings.Contains(joined, secret) {
|
if strings.Contains(joined, secret) {
|
||||||
t.Errorf("password leaked into argv: %q", joined)
|
t.Errorf("password leaked into argv: %q", joined)
|
||||||
}
|
}
|
||||||
// Expected fixed flags and the login as its own trailing argument.
|
// Expected fixed flags and the login as its own trailing argument, behind
|
||||||
want := []string{"-p", "-c", "-f", "/data/sasl/sasldb2", "-u", "mail.example.com", "alerts"}
|
// "--" so it can never be parsed as an option.
|
||||||
|
want := []string{"-p", "-c", "-f", "/data/sasl/sasldb2", "-u", "mail.example.com", "--", "alerts"}
|
||||||
if len(fr.args) != len(want) {
|
if len(fr.args) != len(want) {
|
||||||
t.Fatalf("args = %v, want %v", fr.args, want)
|
t.Fatalf("args = %v, want %v", fr.args, want)
|
||||||
}
|
}
|
||||||
@@ -56,7 +57,7 @@ func TestSASLDeleteArgs(t *testing.T) {
|
|||||||
if err := s.Delete("alerts"); err != nil {
|
if err := s.Delete("alerts"); err != nil {
|
||||||
t.Fatalf("Delete: %v", err)
|
t.Fatalf("Delete: %v", err)
|
||||||
}
|
}
|
||||||
want := []string{"-d", "-f", "/data/sasl/sasldb2", "-u", "mail.example.com", "alerts"}
|
want := []string{"-d", "-f", "/data/sasl/sasldb2", "-u", "mail.example.com", "--", "alerts"}
|
||||||
if strings.Join(fr.args, " ") != strings.Join(want, " ") {
|
if strings.Join(fr.args, " ") != strings.Join(want, " ") {
|
||||||
t.Errorf("delete args = %v, want %v", fr.args, want)
|
t.Errorf("delete args = %v, want %v", fr.args, want)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user