security: phase D pre-release review — pass; harden saslpasswd2 argv
Fable review of the full diff from the v1.0 audit (Phase 11, 65a420d) 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]
|
||||
|
||||
### 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
|
||||
|
||||
- docs: `docs/code-review.md` — phase 1.5 plan for optional password encryption
|
||||
|
||||
+17
-25
@@ -1,6 +1,7 @@
|
||||
# План реализации: 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),
|
||||
e2e/CI в [development.md](development.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
|
||||
меняли одну и ту же поверхность (сессии в SQLite, logrotate + `postfix reload`,
|
||||
гейт `SELFPOST_HOSTNAME`, e2e-override с ослабленными настройками). Срезы по
|
||||
отдельным коммитам не заменяют просмотр дифа от `v1.0.0` до HEAD.
|
||||
|
||||
**Объём.** Диф от тега `v1.0.0` до состояния перед следующим тегом — вместе с
|
||||
Фазами 12–14, которых в аудите v1.0 не было — плюс повторный проход по чек-листу
|
||||
ТЗ 7.6 целиком, а не только по изменённым строкам. Приоритет: аутентификация и
|
||||
сессии, валидация ввода, запись в конфиги и map-файлы (injection), `os/exec` без
|
||||
shell, права на файлы в `/data`, обращение с секретами (пароли приложений,
|
||||
`sasldb2`, архив бэкапа).
|
||||
|
||||
**Модель — Fable** (не Opus): код писал Opus, проверка собственной работы
|
||||
систематически слабее независимой. Правило progress.md «безопасность/инфра →
|
||||
Opus» — про написание; здесь ревизия. Форма: `/security-review` по изменениям
|
||||
(скилл смотрит диф) + ручной проход по 7.6.
|
||||
|
||||
**Гейт.** Вместе с e2e (C.4, готов): до тега. Находка класса «эксплуатируется
|
||||
снаружи» откладывает тег. Каждая находка закрывается явно: правка до тега либо
|
||||
запись в [security.md](security.md) как принятый риск с обоснованием. «Посмотрели
|
||||
и ладно» — не закрытие.
|
||||
**Результат.** Эксплуатируемых уязвимостей (high/medium) не найдено. Одна
|
||||
находка defence-in-depth закрыта правкой до тега: логин приложения передаётся
|
||||
в `saslpasswd2` после `--`, чтобы значение, начинающееся с `-` (допустимо
|
||||
whitelist'ом), не могло быть разобрано getopt как флаг
|
||||
([internal/app/sasl.go](../internal/app/sasl.go)). Принятые риски в
|
||||
[security.md](security.md) не пополнились — существующие записи (origin-check
|
||||
fallback, отсутствие CSRF-токенов, send-log gap) покрывают всё найденное.
|
||||
Сводка ревизии — в записи `Security` CHANGELOG `[Unreleased]`.
|
||||
|
||||
+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.example.com`)**: `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 — хвост документации и деплоя».
|
||||
- **Рецензирование кодовой базы** (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+, роль администратора домена).
|
||||
- **Прод:** `selfpost.example.com`, реальный 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 обязан
|
||||
выполнять; полный аудит на 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
|
||||
`__Host-` с обнаружением дублей — Фаза 14) закрыт; история — в
|
||||
|
||||
@@ -50,7 +50,10 @@ func (s *SASLDB) Set(login, password string) error {
|
||||
// -c: create the account / set the password.
|
||||
// -f: operate on our sasldb2 rather than the system default path.
|
||||
// -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 {
|
||||
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 {
|
||||
return err
|
||||
}
|
||||
// -d: delete the account.
|
||||
args := []string{"-d", "-f", s.path, "-u", s.realm, login}
|
||||
// -d: delete the account. "--" as in Set: the login is always an operand.
|
||||
args := []string{"-d", "-f", s.path, "-u", s.realm, "--", login}
|
||||
if err := s.run(args, nil); err != nil {
|
||||
return fmt.Errorf("saslpasswd2 delete %q: %w", login, err)
|
||||
}
|
||||
|
||||
@@ -39,8 +39,9 @@ func TestSASLSetPassesPasswordOnStdinNotArgv(t *testing.T) {
|
||||
if strings.Contains(joined, secret) {
|
||||
t.Errorf("password leaked into argv: %q", joined)
|
||||
}
|
||||
// Expected fixed flags and the login as its own trailing argument.
|
||||
want := []string{"-p", "-c", "-f", "/data/sasl/sasldb2", "-u", "mail.example.com", "alerts"}
|
||||
// Expected fixed flags and the login as its own trailing argument, behind
|
||||
// "--" 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) {
|
||||
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 {
|
||||
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, " ") {
|
||||
t.Errorf("delete args = %v, want %v", fr.args, want)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user