diff --git a/CHANGELOG.md b/CHANGELOG.md index 421f4bc..a913e02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,26 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version `schema-squash` (replace the 1.x SQLite migration chain with a 2.x baseline; not a reason to cut a major on its own). +- docs: security and operator docs updated for the panel that has shipped + global administrators and domain-admins since 1.2.0. The CSRF ADR in + [security.md](docs/security.md) no longer argues from "single-user"; it now + states that cross-user CSRF between panel roles is not the threat the origin + check defends against, and gives a new revisit trigger. Dropped the + unimplemented "or argon2" alternative for the password hash. + [guide.md](docs/guide.md) documents the Users page and the two roles, + the Settings page's default DMARC report address, level-2 rate limiting's + fail-open behaviour, that restoring an older backup can resurrect sessions, + and that a domain-admin can export working SASL passwords for domains + assigned to them. [architecture.md](docs/architecture.md)'s route table now + lists `/license` and the `/account` → `/settings` redirect. Fixed the + `internal/backup` package comment, which claimed the panel regenerates + Postfix/OpenDKIM state from SQLite on every restore boot (it only runs the + version guard; drifted daemon state is healed by the Status page's Reload + button). Corrected stale `admin.dmarc_report_email` references in + [roadmap.md](docs/roadmap.md) and + [docs/plans/dmarc-reports.md](docs/plans/dmarc-reports.md) to the setting's + actual home after migration `0005`. No behaviour change. + ## [1.2.5] - 2026-08-13 Rate-limit form polish after 1.2.4. Upgrading is a tag bump; no migration. diff --git a/docs/architecture.md b/docs/architecture.md index 315fb2a..cffd813 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -161,8 +161,10 @@ unless noted. The table below is a summary — HTMX fragment endpoints | Route | Purpose | |---|---| | `/healthz` | Liveness (no auth) | +| `/license` | Embedded `LICENSE` text (no auth) | | `/setup/*` | One-time admin bootstrap | | `/login`, `/logout` | Session auth | +| `/account` | 308 redirect to `/settings` (pre-1.2.3 route, kept as a compat shim) | | `/status` | Process, cert, socket, PTR checks; machine CPU/memory/network | | `/domains`, `/domains/*` | Domain and application CRUD, DKIM, L2 limits | | `/domains/import` | Domain import (`POST`; form on the Backup page) | diff --git a/docs/development.md b/docs/development.md index 40bc595..1513b89 100644 --- a/docs/development.md +++ b/docs/development.md @@ -265,7 +265,11 @@ operator would actually use. **Coverage (summary):** bootstrap → SMTP AUTH → delivery → DKIM verify → send-log `queued → sent`; negatives (no AUTH, relay, sender/login mismatch, L1/L2 limits, milter fail-open, bad `SELFPOST_HOSTNAME`, session survives -`docker restart`). Polling with timeouts only — no fixed `sleep`. +`docker restart`); startup checks that supervisord actually brought up +OpenDKIM, the panel, and Postfix (`checkSupervisorProcesses`), plus logrotate +config-mode and forced-rotation checks (`checkLogrotateConfigMode`, +`checkLogrotateRotation` — [test/e2e/logrotate_check.go](../test/e2e/logrotate_check.go)). +Polling with timeouts only — no fixed `sleep`. Requires **Docker + Compose v2** on the machine running the suite. diff --git a/docs/guide.md b/docs/guide.md index 499efd8..23adee7 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -230,8 +230,22 @@ service healthy and will mail be accepted?" - **Backup** (`/backup`) — download a full-server backup; the same page hosts the domain-import form (`POST /domains/import`). See [Backup, restore, and moving a single domain](#backup-restore-and-moving-a-single-domain). -- **Settings** (`/settings`) — change the administrator username and/or password. - Application SASL logins are separate and are not changed here. +- **Settings** (`/settings`) — change the administrator username and/or + password, and the panel-wide default DMARC report address (`rua=`) offered + when a domain doesn't set its own — see [DNS setup](#dns-setup). Application + SASL logins are separate and are not changed here. +- **Users** (`/users`, global administrator only) — create, edit, and delete + panel users. There are two roles: + - **Global administrator** — full access to every page and every domain, + including Users, Backup, Status, Mail queue, and System log. + - **Domain-admin** — scoped to one or more domains assigned by a global + administrator. Sees only those domains' pages, applications, and + Deliveries rows; `/users`, `/backup`, `/status`, `/mail-queue`, and + `/system-log` are not reachable (404). A domain-admin can *export* the + domains assigned to them — see the note on working credentials below. + + The panel refuses to remove or demote the **last** global administrator, so + it can never end up with none. **Sessions.** A login survives a container restart: sessions live in SQLite, not in memory. Expiry is a sliding idle window (`PANEL_SESSION_IDLE_DAYS`, default @@ -296,6 +310,12 @@ An application override without trusted IPs is inactive. When a level-2 ceiling is exceeded, Postfix returns a 4xx and the refusal is recorded in Deliveries as `rejected`. +**Level 2 is best-effort, not a guarantee.** It runs inside the journal-milter +and is deliberately fail-open: if the rate-limit lookup hits a store error, or +the connecting client's IP is not available to the milter, level 2 is skipped +and the message is accepted rather than held up. Level 1 (the Postfix anvil +limit above) is the backstop that keeps working even when level 2 cannot run. + ## Backup, restore, and moving a single domain Two related but distinct operations @@ -317,6 +337,12 @@ Two related but distinct operations `:latest`: without a known version, there'd be no way to tell which image restoring a given backup actually requires. + Restoring an archive taken **before** you invalidated a session (password + change, logout everywhere) can bring that session back: session rows travel + with the backup, and a browser that still holds the matching cookie is + logged in again once the idle timeout allows it. If a restore might do this, + changing every user's password afterwards clears it out. + **Alternative: archive `./data` while stopped.** If the service can be taken offline, `docker compose down` then `tar czf selfpost-data.tar.gz ./data` on the host is safe — nothing is writing to SQLite. Unlike the panel/CLI backup @@ -332,7 +358,12 @@ Two related but distinct operations file, *Backup* → *Import a domain* to read it back in): moves one domain — its DKIM key and its applications' **working** SASL passwords — to a different SelfPost instance without regenerating anything, so DNS (the DKIM TXT record) doesn't need to change. Unlike a full - restore, this works across different hostnames/instances. + restore, this works across different hostnames/instances. *Import* is + global-administrator only; *export* is available to any user who can access + the domain, **including a domain-admin** for a domain assigned to them — so + a domain-admin can walk away with that domain's working SASL passwords in + the clear. Weigh that when deciding which domains to assign to a + domain-admin account. Both files are **secrets** — they contain the admin password hash (full backup) or working application credentials (domain export) in the clear or in diff --git a/docs/plans/code-review.md b/docs/plans/code-review.md index 3bd7aa6..31a454e 100644 --- a/docs/plans/code-review.md +++ b/docs/plans/code-review.md @@ -522,21 +522,21 @@ the list, the fragment, or query parameters. `go test ./...` green. **Model: Sonnet** (English docs). No code behaviour change except copy. -- [ ] Rewrite the CSRF ADR in [security.md](../security.md) for a panel that +- [x] Rewrite the CSRF ADR in [security.md](../security.md) for a panel that already has global + domain-admin. Keep the origin-check mechanism unless a new decision says otherwise. New revisit trigger (e.g. untrusted domain-admins, or a requirement that does not depend on browser headers). -- [ ] Drop “or argon2” unless argon2 is implemented. -- [ ] [guide.md](../guide.md): Users / roles; Settings DMARC field; L2 +- [x] Drop “or argon2” unless argon2 is implemented. +- [x] [guide.md](../guide.md): Users / roles; Settings DMARC field; L2 fail-open; restore can resurrect sessions; domain-admin can export working SASL passwords for assigned domains. -- [ ] Architecture route table: `/license`, `/account` → `/settings`. -- [ ] Fix `internal/backup` package comment (restore = extract tarball + +- [x] Architecture route table: `/license`, `/account` → `/settings`. +- [x] Fix `internal/backup` package comment (restore = extract tarball + `CheckRestore`; maps come from the archive; Reload heals drift). -- [ ] `admin.dmarc_report_email` → `settings` in roadmap + dmarc-reports plan. -- [ ] development.md e2e summary: logrotate + process checks. -- [ ] `setupManager` / `store` package comments: `users`, not `admin` row. +- [x] `admin.dmarc_report_email` → `settings` in roadmap + dmarc-reports plan. +- [x] development.md e2e summary: logrotate + process checks. +- [x] `setupManager` / `store` package comments: `users`, not `admin` row. **Done when:** an operator who reads only the guide knows the panel has two roles, and security.md no longer calls the panel single-user. diff --git a/docs/plans/dmarc-reports.md b/docs/plans/dmarc-reports.md index eda56a2..6bf70e4 100644 --- a/docs/plans/dmarc-reports.md +++ b/docs/plans/dmarc-reports.md @@ -18,7 +18,8 @@ operator. - gzip + XML aggregate parsing → SQLite summaries per sending domain. - Panel page and/or per-domain section: recent reports, third-party senders, delivery health of report ingestion. -- Reuse `admin.dmarc_report_email` and `domains.dmarc_rua` for DNS templates; +- Reuse the `dmarc_report_email` setting (moved off the old `admin` table into + `settings` by migration `0005`) and `domains.dmarc_rua` for DNS templates; when enabled, suggest a SelfPost-hosted report address. **Out:** diff --git a/docs/roadmap.md b/docs/roadmap.md index 5c70aee..a3710b3 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -118,9 +118,10 @@ reports (`ruf=`) out of scope for v1. **Dependencies / risks:** inbound SMTP in the image (may share infrastructure with [inbound-relay](plans/inbound-relay.md) but must not require backup-MX); -storage and retention of parsed summaries; the `admin.dmarc_report_email` and -`domains.dmarc_rua` settings added in the DMARC template work must stay the -source of truth for `rua=` in DNS guidance. +storage and retention of parsed summaries; the `dmarc_report_email` setting +(migration `0005` moved it off the old `admin` table into `settings`) and +`domains.dmarc_rua` added in the DMARC template work must stay the source of +truth for `rua=` in DNS guidance. **Order:** after the DMARC `rua=` settings ship; may follow or overlap with inbound-relay depending on how port 25 acceptance is structured. diff --git a/docs/security.md b/docs/security.md index 6849944..c204095 100644 --- a/docs/security.md +++ b/docs/security.md @@ -34,8 +34,8 @@ The panel is exposed to the internet — the items below are **not optional**. - Failed attempts do **not** invalidate the token early (protects setup from being DoS-ed). - Once the administrator exists the token is void forever, `/setup/*` → 404. -- The administrator password is bcrypt (or argon2) in SQLite only; no plaintext - and no MD5. +- The administrator password is bcrypt in SQLite only; no plaintext and no + MD5. - `PANEL_USERNAME` / `PANEL_PASSWORD_HASH` in env are **not used**. ### Application SASL passwords @@ -112,19 +112,21 @@ deferred item from the roadmap. - **A `POST` with neither `Sec-Fetch-Site` nor `Origin` is allowed through.** A client that sends neither — a genuinely old browser, or a webview with a frozen engine — stays vulnerable to CSRF from any site. Accepted - deliberately: the panel is single-user, the administrator picks the browser, - and a strict mode would not "protect" such a client, it would simply break the - panel in it. Tightening is one line in `originAllowed` + deliberately: every panel user (global or domain-admin) is an operator who + picks their own browser, not an untrusted party the panel needs to defend + against, and a strict mode would not "protect" such a client, it would + simply break the panel in it. Tightening is one line in `originAllowed` ([internal/web/security.go](../internal/web/security.go)): return `false` instead of `true` in the "neither header present" branch. - **Session-bound CSRF tokens are not implemented.** The origin check closes the neighbouring-subdomain case but depends on browser behaviour; a token does not. The price is a hidden field in roughly two dozen forms. The trigger to - revisit is a requirement for protection that holds regardless of the browser. - A token would not save the panel from XSS inside it either: code executing in - the panel's origin sends the request itself — against that, `html/template` - auto-escaping and CSP do the work, which is why templates must contain no - inline scripts and no inline styles. + revisit is a requirement for protection that holds regardless of the browser, + or a domain-admin population the global administrator does not fully trust + (see the ADR below). A token would not save the panel from XSS inside it + either: code executing in the panel's origin sends the request itself — + against that, `html/template` auto-escaping and CSP do the work, which is why + templates must contain no inline scripts and no inline styles. - **Encrypting backups and exports is an option, not the default.** With the checkbox cleared the file downloads in the clear, as in 1.0. Otherwise an operator with nowhere to keep a password would lose the ability to take a @@ -158,24 +160,35 @@ deferred item from the roadmap. **Context.** The panel is forms (`POST`) with a cookie session — the classic CSRF surface. What is needed is a way to tell a request from the panel's own -page apart from one initiated by a third-party site in the logged-in -administrator's browser. +page apart from one initiated by a third-party site in a logged-in user's +browser. The panel is multi-user since 1.2.0 (a global administrator plus +zero or more domain-admin users, each scoped to their assigned domains), but +that is an authorization boundary (who can see or change what), not a change +to the CSRF threat: the attacker in scope here is still an external site +riding a legitimate user's cookie, not one panel user attacking another +through the browser. **Decision.** `originAllowed` in [internal/web/security.go](../internal/web/security.go) checks `Sec-Fetch-Site` (when the browser sends it) or `Origin` (fallback) against the panel's host; a request carrying neither header is **allowed through** rather than rejected. -There are no session-bound tokens embedded in forms. +There are no session-bound tokens embedded in forms. The check applies the same +way regardless of the requesting user's role. -**Why not tokens.** The panel is single-user (one administrator per instance) — -the threat model does not include cross-user CSRF inside the panel itself, only -an external site making the administrator's browser send a request. The origin -check covers that without touching a single template: a token would need a -hidden field in roughly two dozen forms and synchronisation with every new form, -and it would still not protect against XSS inside the panel — code executing in -the panel's origin reads the token and sends the request itself. XSS is handled -by `html/template` auto-escaping and CSP, so that is a separate line of defence, -not a CSRF token. +**Why not tokens.** Cross-user CSRF is not the threat model here: a +domain-admin's browser sending a request still needs that domain-admin's own +cookie, so a token would not add a boundary between roles that the +authorization checks (`Principal.CanAccessDomain`, +[internal/web/auth/principal.go](../internal/web/auth/principal.go); route +gating in [internal/web/auth/middleware.go](../internal/web/auth/middleware.go)) +don't already enforce. The remaining case is an external site making a +logged-in user's browser send a request, which the origin check covers without +touching a single template. +A token would need a hidden field in roughly two dozen forms and +synchronisation with every new form, and it would still not protect against +XSS inside the panel — code executing in the panel's origin reads the token +and sends the request itself. XSS is handled by `html/template` auto-escaping +and CSP, so that is a separate line of defence, not a CSRF token. **Trade-off.** A client that sends neither `Sec-Fetch-Site` nor `Origin` (a genuinely old browser, or a webview with a frozen engine) stays vulnerable — see @@ -183,7 +196,11 @@ genuinely old browser, or a webview with a frozen engine) stays vulnerable — s such a client, at the price of a narrow residual surface. **Revisit if:** a requirement appears for protection that does not depend on -browser behaviour, or the panel becomes multi-user. +browser behaviour, or domain-admin accounts stop being trusted operators (for +example, if a future release lets a global administrator invite domain-admins +whose browsers/devices are not vetted) — at that point cross-role request +forgery inside the panel would need its own analysis, separate from the +external-site case this ADR covers. ## How this list grows diff --git a/internal/backup/backup.go b/internal/backup/backup.go index 2c2c6aa..80f1513 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -6,13 +6,18 @@ // it. TLS certificates (the reverse proxy's responsibility) and the Postfix // queue are deliberately excluded (architecture.md § Persistence). // -// Restore is intentionally not a separate code path: a backup is extracted -// into the /data bind mount before first start, and the panel regenerates -// Postfix and OpenDKIM from the restored SQLite state exactly as on any normal -// start. The only restore-specific step is CheckRestore, which refuses to boot -// if the manifest's version does not match the running binary, so -// schema/format skew between versions cannot silently corrupt state -// (architecture.md § Persistence). +// Restore is not a separate code path in the panel: a backup is extracted into +// the /data bind mount before first start, and the archive already carries +// everything the mail path needs — DKIM keys, sasldb2, and Postfix's sender +// map — so nothing needs to be regenerated from SQLite for the daemons to +// start correctly. The only restore-specific step the panel runs is +// CheckRestore, which refuses to boot if the manifest's version does not match +// the running binary, so schema/format skew between versions cannot silently +// corrupt state (architecture.md § Persistence). If a daemon's on-disk state +// ever drifts from what SQLite records — for example after a manual edit +// under /data — the Status page's "Reload configuration" button re-derives +// OpenDKIM's tables and the Postfix sender map from the database; that is a +// deliberate, operator-triggered heal, not something restore does on its own. package backup import ( diff --git a/internal/store/store.go b/internal/store/store.go index 4434eba..c211e97 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -1,7 +1,8 @@ // Package store owns the SelfPost SQLite database: the single file under /data -// that persists the administrator account, sending domains and applications, -// the send log and rate-limit settings (architecture.md § Persistence). It -// exposes typed queries so the rest of the panel never builds SQL by hand. +// that persists panel users (global administrators and domain-admins), sending +// domains and applications, the send log and rate-limit settings +// (architecture.md § Persistence). It exposes typed queries so the rest of the +// panel never builds SQL by hand. package store import ( diff --git a/internal/web/auth/setup.go b/internal/web/auth/setup.go index b560810..569773b 100644 --- a/internal/web/auth/setup.go +++ b/internal/web/auth/setup.go @@ -16,8 +16,9 @@ const setupTokenTTL = 10 * time.Minute // setupManager owns the one-time administrator setup token. The token itself is // ephemeral (regenerated on restart or expiry) and lives only in memory; the -// persistent "setup complete" fact is the presence of the admin row in the -// store, so once that exists the token is gone for good (security.md). +// persistent "setup complete" fact is the presence of any row in the store's +// users table (`store.UserExists`), so once the first global administrator is +// created the token is gone for good (security.md). type setupManager struct { store *store.Store hostname string