diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c40f57..a3837ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,6 +158,22 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version A full backup follows links under `certs/` and carries the files; a link that leads nowhere stops it, named (assumptions C-A1, C-Q4). If you use certbot this way, follow the guide's new "Reverse proxy" steps. +- **A DMARC report with `pct=0` says 0.** An explicit `0` - a + policy applied to no message - was shown as 100, the value for a report + that leaves `pct` out (assumptions C-M7). +- **The nginx fragment's first certificate is issued, and renewed.** The + command it gave passed its arguments to the renewal loop, which ignored + them, and used the webroot nginx could not serve yet. It is now + `run --entrypoint certbot ... --standalone` with port 80 free, and the + renewal loop names nginx's webroot, which a standalone certificate would + otherwise not use (assumptions D-guide:301). +- **The guide, architecture and security documents say what the code + does.** Some forty claims the sources contradict, or only partly bear out, + are corrected: what the rate limits count, what a backup restore message + looks like, how to pass the backup password, what the queue and Health + show, a backup MX's preference, the DMARC report address, what the + password store keeps, what the delivery log holds, and others + (assumptions, Answers E). - **Words for what happens to mail match Postfix's.** A message refused with a temporary error while the panel or the spam filter does not answer was called deferred, which in Postfix's words is mail waiting in its queue; diff --git a/build/postfix-config.sh b/build/postfix-config.sh index 25fb294..088d088 100644 --- a/build/postfix-config.sh +++ b/build/postfix-config.sh @@ -89,10 +89,13 @@ HOSTNAME_VALUE="${SELFPOST_HOSTNAME:-$(hostname -f 2>/dev/null || hostname)}" # request dropped ... TLS context initialization failed"); every new smtpd # process loads the files again in pre_jail_init(). # -# Postfix insists the private key is root-owned and mode 0600. The bind mount -# is often :ro and owned by the host user (e2e TempDir / CI runner UID), so -# copy into a writable internal dir and normalise ownership before postconf -# and `postfix check`. +# Postfix's TLS_README advises a private key owned by root and readable by no +# other user; Postfix does not enforce it (smtpd loads the key as root in +# pre_jail_init(), Postfix 3.7.11 src/smtpd/smtpd.c), but the mount cannot +# give that mode - it is :ro and owned by the host's user, often readable by +# group 998 for the panel's backup. So the pair is copied into a directory of +# Postfix's own, the key root:root 0600, before postconf and `postfix check` +# (assumptions D-arch:154). TLS_CERT_SRC="${TLS_CERT_FILE:-/etc/postfix/tls/fullchain.pem}" TLS_KEY_SRC="${TLS_KEY_FILE:-/etc/postfix/tls/privkey.pem}" TLS_INTERNAL_DIR=/etc/postfix/tls-internal diff --git a/deploy/nginx/docker-compose.nginx.yml b/deploy/nginx/docker-compose.nginx.yml index 604b206..71ba325 100644 --- a/deploy/nginx/docker-compose.nginx.yml +++ b/deploy/nginx/docker-compose.nginx.yml @@ -14,10 +14,15 @@ # # docker compose -f docker-compose.yml -f nginx/docker-compose.nginx.yml up -d # -# First-run certificate issuance (webroot method, before nginx has a cert to -# serve — run once): +# First-run certificate issuance, before nginx has a certificate to serve - +# run once, with port 80 free (certbot answers the challenge itself; +# --webroot would need nginx already serving /.well-known). --entrypoint is +# needed: the service's own entrypoint is the renewal loop below, an +# `sh -c` that ignores any arguments given to `run` +# (docs.docker.com/reference/compose-file/services/#entrypoint; assumptions +# D-guide:301): # docker compose -f docker-compose.yml -f nginx/docker-compose.nginx.yml \ -# run --rm certbot certonly --webroot -w /var/www/certbot \ +# run --rm -p 80:80 --entrypoint certbot certbot certonly --standalone \ # -d mail.example.com --email you@example.com --agree-tos --no-eff-email services: @@ -61,5 +66,9 @@ services: # until a certificate is within its renewal window. # (eff-certbot.readthedocs.io, User Guide, "Automated Renewals": the # sample cron runs "certbot renew" at 0 and 12 h; renew acts only on a - # certificate near expiry.) - entrypoint: sh -c 'trap exit TERM; while :; do certbot renew; sleep 12h & wait $${!}; done' + # certificate near expiry.) The first certificate was issued --standalone + # (see the top of this file), and certbot renews with the plugin it was + # issued with "unless you specify other plugins or options" (same page, + # "Renewing certificates") - by then nginx holds port 80 and serves the + # webroot, so renewal names it; a renewal that succeeds with it saves it. + entrypoint: sh -c 'trap exit TERM; while :; do certbot renew --webroot -w /var/www/certbot; sleep 12h & wait $${!}; done' diff --git a/docs/architecture.md b/docs/architecture.md index dc11953..5d2bc9f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -45,7 +45,9 @@ that will not start, is an exit supervisord counts as too quick: once each start it is STARTING. `crashexit` event listener exits the container when a program of the mail path -is FATAL, so Docker `restart: unless-stopped` starts it again. `logrotate`'s +is FATAL, so Docker `restart: unless-stopped` starts it again - the same +container, not a new one: the entrypoint clears what the last run left in +`/run` (the milter sockets, the sender check's alive mark). `logrotate`'s FATAL (a config logrotate would ignore) stays on Server › Health instead. The panel runs `postfix-reload` through supervisord's XML-RPC interface and waits for its exit status (`internal/supervisor`, `RunOnce`); the program @@ -106,6 +108,11 @@ smtp/inet` removes Debian's stock smtpd). Outbound delivery still uses the message's queue id under the key in `/data/antispam/filter.key` (`inbound.FilterKey` makes it on first start; rspamd reads it through the mounted directory and signs in a custom routine of `milter_headers.conf`). + rspamd removes a sender's own copies of the three headers and writes and + signs its own for every message it lets through: `skip_local` and + `skip_authenticated` are off in that file, so this holds whatever the + sender's address (rspamd 3.11.1, `src/plugins/lua/milter_headers.lua`, + `skip_wanted()`). With the filter down - it is attached fail-open - a sender's own verdict headers carry no signature and the row is that of mail nobody judged. Refused mail never reaches this milter. @@ -116,8 +123,8 @@ smtp/inet` removes Debian's stock smtpd). Outbound delivery still uses the counted only for a queue id that listener opened) into a row, dated by the line, when the recipient is at an inbound domain ([internal/logtail/inbound.go](../internal/logtail/inbound.go)). A line - names one recipient, so a message refused at end-of-message is one row - whatever its recipient count. The tailer saves its place every five seconds, + names one recipient (the last), so a message refused at end-of-message is + one row whatever its recipient count. The tailer saves its place every five seconds, so after an unclean stop it reads some lines again; the lines it reads on start are written only if the same row is not there already. The journal is pruned every six hours: rows @@ -131,7 +138,9 @@ smtp/inet` removes Debian's stock smtpd). Outbound delivery still uses the them. The sidecar mounts the directory and rspamd's multimap rules ([deploy/antispam/rspamd/multimap.conf](../deploy/antispam/rspamd/multimap.conf)) read them: a deny entry's rule carries the action `reject`, which rspamd - sets as a pre-result, an allow entry is a symbol weighing -100. Allow is + sets as a pre-result (a passthrough result: the remaining filters are not + run and the action stands whatever the score), an allow entry is a symbol + weighing -100. Allow is deliberately not an `accept` action: with two pre-results rspamd keeps the one set first, and deny then won or lost by rule order (seen on rspamd 3.11, date not recorded). A refusal by the deny list overrides any score, @@ -153,8 +162,10 @@ smtp/inet` removes Debian's stock smtpd). Outbound delivery still uses the [internal/postfix/inbound.go](../internal/postfix/inbound.go). Domains with an empty upstream host are omitted from the maps. Optional `INBOUND_ANTISPAM_MILTER` only when inbound relay is enabled (not on - DMARC-only port 25); default `milter_default_action` is fail-open - (`accept`). **DMARC ingest** (`DMARC_REPORTS_ENABLE=true`): allow-listed + DMARC-only port 25); it is attached with its own `default_action`, + `INBOUND_ANTISPAM_MILTER_ACTION`, which is `accept` (fail-open) unless set + to `tempfail` (main.cf's `milter_default_action` is `tempfail`). + **DMARC ingest** (`DMARC_REPORTS_ENABLE=true`): allow-listed report addresses via `check_recipient_access` on `postfix/dmarc_recipients`; messages pipe to `dmarc-ingest` (symlink to `panel`). When both flags are on, one listener serves both paths; per-IP rate uses @@ -168,8 +179,9 @@ smtp/inet` removes Debian's stock smtpd). Outbound delivery still uses the `anvil_rate_time_unit` from `RATE_LIMIT_*` env vars; independent of milter. - **Chroot disabled** for all services (DNS/TLS inside container). - **TLS certs** — read-only mount at `TLS_CERT_FILE` / `TLS_KEY_FILE`, copied - at start into `/etc/postfix/tls-internal` (Postfix wants the key root-owned - and `0600`, which the mount cannot give), and Postfix reads the copy. + at start into `/etc/postfix/tls-internal`, owned by root and `0600` as + Postfix's TLS_README advises (Postfix itself reads the key as root and does + not check its mode), which the mount cannot give; Postfix reads the copy. `cert-reload` compares the mount with the copy daily; when they differ and the certificate and key belong together it renews the copy (each file renamed into place) and reloads Postfix. @@ -301,8 +313,9 @@ server itself. how many more there were: a row per recipient would let one refused message write a thousand. - **What it costs is counted.** The role marks itself alive every two - seconds (the socket's path with `.alive`, under `/run`, so a new container - starts without one). At start the time since the mark is the check's + seconds (the socket's path with `.alive`, under `/run`; the entrypoint + removes it at every container start, so a restart begins without one). At + start the time since the mark is the check's downtime - counted from Postfix's own start if that was later, and not at all when supervisord says Postfix is not running, since mail was then stopped for another reason. Supervisord is asked for half a minute; when it @@ -312,14 +325,16 @@ server itself. writes a warning naming the socket and then, from the same smtpd process, `milter-reject: … 451 4.7.1 Service unavailable` from the connection on (the client is told `454 4.3.0` at `AUTH` and leaves), so what is counted - is a connection turned away, once - a client brings a message when it - connects. The reject line does not name the milter and the journal milter's + is a connection turned away, once - how many messages it held back is not + known: a client may bring one or several, or come back for the same one. + The reject line does not name the milter and the journal milter's level-2 refusal reads the same, hence the warning. A panel that is running and stops answering in the middle of a message (its socket accepts, nothing replies) is met by cleanup, not smtpd: past `DATA` it is cleanup that talks to the milters, and after the content timeout it writes `warning: milter unix:: can't read SMFIC_HEADER reply packet header` and refuses the - message with `4.7.1` (seen on the e2e stand with the panel frozen). One + message with `4.7.1` (seen on the e2e stand with the panel frozen; + `panel_hung_mid_message_is_counted` checks it at every run). One cleanup process serves message after message, so each such warning is counted as a message. The two are counted apart (the owner, 2026-10-14): connections turned away in `deferred`, messages deferred mid-message in @@ -412,14 +427,13 @@ Rotation uses rename + `postfix reload` ([build/logrotate-mail.conf](../build/logrotate-mail.conf)), not `copytruncate` — the latter can drop `status=sent` lines and leave send-log rows stuck at `queued`. After rename, logrotate runs `create 0640 postfix selfpost` (postlogd -recreates the file lazily on first write as mode `0600`, which the unprivileged -panel user cannot read). `follow()` drains the old inode once more before -switching descriptors; the panel treats a missing log file as an empty tail, not -an error. That drain does not close the gap entirely: postlogd keeps writing to -the renamed file until the reload reaches it, seconds later, and a line written -there after the drain is not read. Keeping the old file open until it has -stopped growing is the fix; it waits for the measurement below to say how -much it matters. +recreates the file on its first line after the reload, as root and mode +`0600`, which the unprivileged panel user cannot read). `follow()` drains the +old inode once more before switching descriptors, and keeps it open and reads +it for a minute after: postlogd keeps writing to the renamed file until the +reload makes it exit, seconds later (Postfix 3.7.11, src/postlogd/postlogd.c; +src/master/dgram_server.c, `dgram_server_abort()`). The panel treats a missing +log file as an empty tail, not an error. **Delivery lines are read at the head of the line.** The pattern that turns a line into a row's status (`deliveryRe`) is matched after the timestamp, the @@ -786,8 +800,13 @@ with scripts off. htmx is told not to inject its indicator stylesheet (`htmx-config` meta), which the CSP would otherwise have to allow. State-changing requests (every non-`GET`/`HEAD`/`OPTIONS`) pass `originAllowed` -first: `Sec-Fetch-Site` must say `same-origin`, or `Origin` must name the -panel's host; a request with neither header is let through (an accepted risk). +first: `Sec-Fetch-Site` must say `same-origin`, or, where the browser does not +send it (an older one, or a current one over plain HTTP at a name other than +`localhost`), `Origin` must name the panel's host - which a form of the panel +carries under its `Referrer-Policy: same-origin`, the page's own origin (under +`no-referrer` it would be `null` and refused); a request with neither header +is let through (an accepted risk). A refusal logs the `Sec-Fetch-Site`, +`Origin` and `Host` it compared. There are no CSRF tokens — see [security.md](security.md). ### Errors shown to users @@ -977,9 +996,14 @@ The tables that carry the panel's model: `logtail_state`, `inbound_domains`, `inbound_transports`, `inbound_recipients`, `dmarc_reports`, `dmarc_report_records`. -The panel is the only writer; SQLite runs in WAL mode on one connection (see -[Panel binary](#panel-binary-cmdpanel)), which is why a full backup snapshots -with `VACUUM INTO` instead of copying the file. +Two processes write: the panel, on one connection, and `dmarc-ingest`, which +Postfix starts per report. SQLite runs in WAL mode (see +[Panel binary](#panel-binary-cmdpanel)) and allows one write transaction at a +time; every transaction that may write takes the lock at its `BEGIN` +(`_txlock=immediate`, internal/store), so the other process waits for it +within `busy_timeout`, and a schema migration is read and applied under that +lock, once. A full backup snapshots with `VACUUM INTO`, which gives a +consistent copy of a live database, instead of copying the file. **Rotation:** send-log retention `SEND_LOG_RETENTION_DAYS` (default 90), which the stored log lines follow; diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index d3082d7..69bd539 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -1015,12 +1015,17 @@ those after it move up one patch number each. mounted read-only at the same path; certbot's own `chmod 0755` and `chgrp`/`chmod 0640` for a server that is not root (kept on renewal) let the panel read them; no copying hook. -- [ ] **The rest of what the sources contradict** (C-M7 `0`, +- [x] **The rest of what the sources contradict** (C-M7 `0`, C-E2 the e2e zone serial, C-D1 a release gate comparing the image's package versions with the ones cited, C-K8/K9 and the comments the answers name), and **the documents**: every claim of architecture.md, security.md and guide.md the answers find false or partly true is rewritten as the answers propose (D-arch, D-sec, D-guide). - **Sonnet** + *Done 2026-10-15:* the release gate is `checkCitedPackageVersions` (e2e) + against the table in development.md; the documents by three agents, one + per document, reviewed. Two register proposals were outdated by this + stage's fixes (a bounce's `From:` line, `Origin: null`) and not used; the + nginx renewal needed `--webroot` in the loop, which the register missed. - [ ] **What only the stand can answer** (A-43, C-K5, C-M8, C-X1, D-S7, D-B16 and the document claims marked so): postlog's set-gid in the image; the client address Postfix sees through Docker's port publishing diff --git a/docs/development.md b/docs/development.md index ab8c6d6..17ec136 100644 --- a/docs/development.md +++ b/docs/development.md @@ -234,9 +234,29 @@ what it relies on and where that is written, for the version the image runs The Debian sources of the image's versions are fetched with `apt-get source` inside a `debian:bookworm` container (`postfix`, `opendkim`, `cyrus-sasl2`, -`supervisor`, `logrotate`); `dev/upstream/` (not in git) is where they are -kept for reading. Go modules are read in the module cache at the version -`go.mod` pins. +`supervisor`, `logrotate`, and `sendmail` for libmilter); `dev/upstream/` +(not in git) is where they are kept for reading. Go modules are read in the +module cache at the version `go.mod` pins. + +The image takes `debian:bookworm-slim` and installs without pinning (pinned +Debian versions leave the mirrors), so a rebuild after a point or security +release runs packages the citations were not read against. The versions +read are these; the e2e stand compares them with `dpkg-query` in the image +it built (`checkCitedPackageVersions`), and a difference fails the run - +re-read what changed, update the citations, then this table: + + +| Package | Version read | +|---|---| +| `postfix` | `3.7.11-0+deb12u1` | +| `opendkim` | `2.11.0~beta2-8+deb12u1` | +| `libopendkim11` | `2.11.0~beta2-8+deb12u1` | +| `libmilter1.0.1` | `8.17.1.9-2+deb12u2` | +| `libsasl2-2` | `2.1.28+dfsg-10` | +| `sasl2-bin` | `2.1.28+dfsg-10` | +| `supervisor` | `4.2.5-1` | +| `logrotate` | `3.21.0-1` | + A document instead of source: its URL and section. Where neither answers the question, the comment says so and what was measured instead - "measured on diff --git a/docs/guide.md b/docs/guide.md index 4b8f74a..f8fe014 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -216,21 +216,21 @@ expected to set; defaults match the code exactly. | `SELFPOST_HOSTNAME` | Mail-server identity: Postfix HELO/EHLO, SASL realm, certificate CN/SAN, and the hostname the PTR check expects. Bare FQDN only — no scheme or port. | *(required)* | `.env` | | `SUBMISSION_ENABLE` | When `true`, also listen on port 587 with STARTTLS (RFC 6409 submission) alongside the primary 465/smtps listener. | `false` | `.env` | | `INBOUND_RELAY_ENABLE` | When `true`, accept mail on port 25 for domains configured under *Inbound › Domains* in the panel and forward them to the upstream you set. Off by default — the outbound path is unchanged. See [Inbound relay](#inbound-relay). | `false` | `.env` | -| `DMARC_REPORTS_ENABLE` | When `true`, accept DMARC aggregate reports on port 25 only for report addresses configured in the panel, parse gzip/XML, and show summaries under *Outbound › DMARC reports*. Off by default. See [DMARC reports](#dmarc-reports). | `false` | `.env` | +| `DMARC_REPORTS_ENABLE` | When `true`, accept DMARC aggregate reports on port 25 only for report addresses configured in the panel, parse gzip, zip or plain XML, and show summaries under *Outbound › DMARC reports*. Off by default. See [DMARC reports](#dmarc-reports). | `false` | `.env` | | `DMARC_RATE_LIMIT_MESSAGES_PER_IP` | Per-client-IP cap on port 25 when DMARC ingest is on. When inbound relay is also enabled, **`INBOUND_RATE_LIMIT_MESSAGES_PER_IP`** applies to the shared listener instead. | `20` | `.env` | | `DMARC_MESSAGE_SIZE_LIMIT` | Maximum report message size in bytes when DMARC ingest is on. | `5242880` (5 MiB) | `.env` | | `INBOUND_ANTISPAM_MILTER` | Optional milter on the inbound relay listener only (requires `INBOUND_RELAY_ENABLE=true`; not 465/587 or DMARC-only port 25). Empty = off. Format `inet:host:port` or `unix:/path`. Example with [deploy/antispam/docker-compose.antispam.yml](../deploy/antispam/docker-compose.antispam.yml): `inet:antispam:11332`. The panel reads it to report on the *Spam filter* box of the inbound pages and to check that the filter answers (Server › Health). | *(empty)* | `.env` | -| `INBOUND_ANTISPAM_MILTER_ACTION` | What Postfix does if that milter is down: `accept` (fail-open) or `tempfail` (defer). The *Spam filter* box states which. | `accept` | `.env` | +| `INBOUND_ANTISPAM_MILTER_ACTION` | What Postfix does if that milter is down: `accept` (fail-open) or `tempfail` (refuse with a temporary error). The *Spam filter* box states which. | `accept` | `.env` | | `INBOUND_SPAM_LOG_RETENTION_DAYS` | How long the inbound journal keeps a row, 7-365 days; it also never holds more than 10 000 rows. Seeds the setting on the first start with the inbound relay on. | `90` | `.env` | | `INBOUND_JOURNAL_MILTER_SOCKET` | Unix socket of the panel's inbound journal milter, the last milter on port 25 while the inbound relay is on. It records accepted inbound mail and never refuses any; there is no reason to change it. | `/run/selfpost/inbound-journal.sock` | compose | -| `INBOUND_RATE_LIMIT_MESSAGES_PER_IP` | Coarse per-client-IP cap on inbound smtpd (`smtpd_client_message_rate_limit`). Uses the same window as `RATE_LIMIT_WINDOW_SECONDS`. | `20` | `.env` | -| `INBOUND_MESSAGE_SIZE_LIMIT` | Maximum message size in bytes on inbound smtpd (`message_size_limit`). When both inbound relay and DMARC ingest are on, the shared listener uses the **greater** of this and `DMARC_MESSAGE_SIZE_LIMIT`. | `26214400` (25 MiB) | `.env` | -| `RATE_LIMIT_MESSAGES_PER_IP` | Level-1 backstop: maximum messages one client IP may submit per window (Postfix `smtpd_client_message_rate_limit`). See [Rate limiting — level 1](#rate-limiting--level-1-ip-backstop). | `100` | `.env` | +| `INBOUND_RATE_LIMIT_MESSAGES_PER_IP` | Coarse per-client-IP cap on port 25: how many times one client IP may start a message (`MAIL FROM`) per window, accepted or not (`smtpd_client_message_rate_limit`). Uses the same window as `RATE_LIMIT_WINDOW_SECONDS`. | `20` | `.env` | +| `INBOUND_MESSAGE_SIZE_LIMIT` | Maximum message size in bytes on inbound smtpd (`message_size_limit`). When both inbound relay and DMARC ingest are on, the shared listener uses the **greater** of this and `DMARC_MESSAGE_SIZE_LIMIT`. This is port 25's whole limit; 465 and 587 keep Postfix's own, 10 240 000 bytes. | `26214400` (25 MiB) | `.env` | +| `RATE_LIMIT_MESSAGES_PER_IP` | Level-1 backstop: how many times one client IP may start a message (`MAIL FROM`) on one listener per window, accepted or not; 465 and 587 count separately (Postfix `smtpd_client_message_rate_limit`). See [Rate limiting — level 1](#rate-limiting--level-1-ip-backstop). | `100` | `.env` | | `RATE_LIMIT_WINDOW_SECONDS` | Level-1 window length in seconds (Postfix `anvil_rate_time_unit`). | `3600` | `.env` | | `SEND_LOG_RETENTION_DAYS` | Initial default for how many days of send-log history are kept before the background sweep deletes rows, and with them the `mail.log` lines kept for each message (1–2 KB a message) — the main driver of `/data` growth over time and of the size of a full backup. After the first panel start, change retention under **Server › Settings** (global role); the env value is only used to seed SQLite when the setting has never been saved. | `90` | `.env` | | `PANEL_SESSION_IDLE_DAYS` | Sliding idle timeout for the panel login session, in days. There is no absolute cap: a user who keeps coming back stays signed in indefinitely. | `7` | `.env` | | `SELFPOST_DNS_RESOLVERS` | Comma-separated recursive resolvers the panel's PTR/SPF/DKIM/DMARC checks query directly (so they report what the internet sees, not what this host's stub resolver synthesises). | `1.1.1.1:53`, `8.8.8.8:53`, `9.9.9.9:53` when unset | `.env` | -| `TRUSTED_PROXY_CIDR` | Comma-separated CIDRs (bare IPs allowed) of reverse proxies allowed to supply `X-Forwarded-For` for login, setup, and account-change rate-limiting. **Leave unset unless you know the exact address of your reverse proxy.** A wrong value lets a client spoof its rate-limit key by sending a forged `X-Forwarded-For` header — the panel trusts the last hop only when the TCP peer matches one of these CIDRs. Behind the default Apache host-network setup this is typically the Docker bridge gateway, e.g. `172.18.0.1`. | *(empty — XFF ignored)* | `.env` | +| `TRUSTED_PROXY_CIDR` | Comma-separated CIDRs (bare IPs allowed) of reverse proxies allowed to supply `X-Forwarded-For` for login, setup, and account-change rate-limiting. **Leave unset unless you know the exact address of your reverse proxy.** A wrong value lets a client spoof its rate-limit key by sending a forged `X-Forwarded-For` header — the panel trusts the last hop only when the TCP peer matches one of these CIDRs. With Apache on the host (the default), the panel sees the proxy's connections come from the Docker network's gateway, e.g. `172.18.0.1` (`docker network inspect` shows it). | *(empty — XFF ignored)* | `.env` | TLS certificate paths (`TLS_CERT_FILE`, `TLS_KEY_FILE`) are fixed in [deploy/docker-compose.yml](../deploy/docker-compose.yml) to match the `./certs` @@ -271,16 +271,18 @@ that's easy to get wrong lives in the service rather than in a config file somebody edits under pressure. There is exactly one thing the proxy must do: **pass the original `Host` header through**. All four fragments above already do (Apache `ProxyPreserveHost On`, nginx `proxy_set_header Host $host`, Caddy -and Traefik by default). A proxy that rewrites `Host` instead makes the panel -reject every form submission as cross-origin — the log says so explicitly, -printing the `Origin` and `Host` it compared. +and Traefik by default). A proxy that rewrites `Host` instead breaks the panel in a +browser that does not send `Sec-Fetch-Site` (Safari before 16.4, or any +browser on a panel served over plain HTTP at a name): there the panel compares +`Origin` with `Host` and refuses every change as cross-origin - the log says +so explicitly, printing the `Sec-Fetch-Site`, `Origin` and `Host` it compared. In every case the proxy terminates HTTPS for the panel; the resulting certificate must end up under `./certs` as `fullchain.pem` and `privkey.pem` — the files, or links that lead to them inside the container. -**Apache (recommended, on the host).** Install Apache with `ssl`, `proxy`, and -`proxy_http` enabled. Copy +**Apache (recommended, on the host).** Install Apache with `ssl`, `proxy`, +`proxy_http` and `rewrite` enabled. Copy [deploy/apache/selfpost-vhost.conf](../deploy/apache/selfpost-vhost.conf) into your vhost directory, replace `mail.example.com` with your hostname, enable the site, then issue a certificate: @@ -315,23 +317,25 @@ sudo chgrp 998 /etc/letsencrypt/archive/mail.example.com/privkey*.pem sudo chmod 0640 /etc/letsencrypt/archive/mail.example.com/privkey*.pem ``` -**nginx (containerised).** From the `deploy/` directory, merge the nginx -fragment and issue the first certificate before nginx can serve HTTPS: +**nginx (containerised).** Edit +[deploy/nginx/nginx.conf.example](../deploy/nginx/nginx.conf.example) and +replace `mail.example.com` first. Then, from the `deploy/` directory, issue +the first certificate - before nginx can serve HTTPS, so certbot answers the +challenge itself (`--standalone`), and port 80 must be free while it runs. +The `certbot` service's own entrypoint is its renewal loop, which ignores +any arguments, hence `--entrypoint certbot`: ```sh docker compose -f docker-compose.yml -f nginx/docker-compose.nginx.yml \ - run --rm certbot certonly --webroot -w /var/www/certbot \ + run --rm -p 80:80 --entrypoint certbot certbot certonly --standalone \ -d mail.example.com --email you@example.com --agree-tos --no-eff-email - -docker compose -f docker-compose.yml -f nginx/docker-compose.nginx.yml up -d ``` -Edit [deploy/nginx/nginx.conf.example](../deploy/nginx/nginx.conf.example) and -replace `mail.example.com` first. The fragment mounts certbot's directory into -both nginx and SelfPost, at `/etc/letsencrypt`; `./certs` holds two links to +The fragment mounts certbot's directory into both nginx and SelfPost, at +`/etc/letsencrypt`; `./certs` holds two links to `/etc/letsencrypt/live/mail.example.com/` — a path that exists inside the container, not on the host — and the panel reads the key through group 998, -as in the Apache case: +as in the Apache case. Then start everything: ```sh mkdir certs @@ -340,8 +344,16 @@ ln -s /etc/letsencrypt/live/mail.example.com/privkey.pem certs/privkey.pem sudo chmod 0755 nginx/certbot-etc/live nginx/certbot-etc/archive sudo chgrp 998 nginx/certbot-etc/archive/mail.example.com/privkey*.pem sudo chmod 0640 nginx/certbot-etc/archive/mail.example.com/privkey*.pem + +docker compose -f docker-compose.yml -f nginx/docker-compose.nginx.yml up -d ``` +Renewals cannot use the standalone method: nginx holds port 80 by then. The +fragment's renewal loop runs `certbot renew --webroot -w /var/www/certbot`, +the directory nginx serves the challenges from; certbot renews with the +options it is given and keeps them for the certificate once a renewal with +them succeeds (certbot's documentation, "Renewing certificates"). + **Caddy (containerised, automatic ACME).** Edit [deploy/caddy/Caddyfile](../deploy/caddy/Caddyfile) and the `` placeholders in [deploy/caddy/docker-compose.caddy.yml](../deploy/caddy/docker-compose.caddy.yml), @@ -427,9 +439,10 @@ on, the inbound one. Each card leads to its table on **Server › Health** (`/server/health`) — the queue card to the Outbound queue — and Health has the readings, in the order of what stops mail first: -- **Machine** — processor (core and thread counts), memory and swap, and - per-interface network throughput and totals, read from the kernel's - counters; CPU and throughput are measured between refreshes, so they appear +- **Machine** — processor (core and thread counts), memory and swap of the + host (a memory limit set on the container is not reflected), and + per-interface network throughput and totals of the container's own network + interfaces, read from the kernel's counters; CPU and throughput are measured between refreshes, so they appear one refresh after the page opens. A fully busy processor or a machine out of memory is a warning, because both delay or kill the mail path; throughput is only reported. @@ -438,16 +451,16 @@ readings, in the order of what stops mail first: - **Sender check** — what the strict check of the `From:` line has cost today, in the last 30 days and in the last 12 months: for how long the panel did not answer while Postfix was running, how many connections - Postfix turned away because of it, and how many messages it deferred in - the middle (see [Who an application may send + Postfix turned away because of it, and how many messages it refused with a + temporary error in the middle (see [Who an application may send as](#who-an-application-may-send-as)). A restart of the whole container is not counted: mail was stopped for another reason then. A connection turned away is refused before a message is offered, so a monitor that only - connects during an outage is counted too; a message deferred in the middle + connects during an outage is counted too; a message refused in the middle is one the panel stopped answering for after it had begun. A time marked ≈ is at most what it shows: at that start supervisord could not say since when Postfix ran. Days are UTC days. The card warns when - a connection was turned away or a message deferred in the last 24 hours. If these numbers do not stay + a connection was turned away or a message refused in the last 24 hours. If these numbers do not stay at nothing, the panel is in the way of your mail more than it should be - report it. - **Hostname and reverse DNS** — compares `SELFPOST_HOSTNAME` against the PTR @@ -463,8 +476,8 @@ readings, in the order of what stops mail first: ### Queue and System log - **Outbound › Queue** (`/outbound/queue`, global role) — live view of - messages Postfix is still trying to deliver or deferring, as `postqueue -p` - lists them. It only shows: nothing there deletes or forces a message. A box + messages Postfix is still trying to deliver, deferring or holding, as + `postqueue -p` lists them. It only shows: nothing there deletes or forces a message. A box states this instance's retry policy — first retry delay, later backoff cap, how long a message stays in the queue — from `postconf -h`, read once when the panel starts. A `postconf -e` override inside the container is visible @@ -608,8 +621,9 @@ left open will not keep a session alive forever. ### Upgrading -Bump the pinned image tag in `docker-compose.yml` to the target release, then -`docker compose up -d`. The backup version check requires the running image +Bump the pinned image tag in `docker-compose.yml` to the target release, build +that tag as under [Fixed image tag](#fixed-image-tag) while no 2.x image is +published, then `docker compose up -d`. The backup version check requires the running image to match the version that created a full backup — see [Fixed image tag](#fixed-image-tag). @@ -654,8 +668,8 @@ Level 1 is always on, configured via `.env`: - `RATE_LIMIT_MESSAGES_PER_IP` → Postfix `smtpd_client_message_rate_limit` - `RATE_LIMIT_WINDOW_SECONDS` → Postfix `anvil_rate_time_unit` -This is an anvil limit per connecting client IP. It keeps working even if the -journal-milter (level 2) is down. There is no per-IP bypass. +This is an anvil limit per client IP and listener. It keeps working even if the +journal-milter (level 2) is down. There is no per-IP bypass except the container's own loopback. ### Full backup and restore @@ -818,18 +832,19 @@ A version mismatch at step 5 refuses to start and leaves `/data` untouched — the panel exits with a message naming the tag to use, e.g.: ``` -backup: this backup was created by SelfPost 1.2.3 but this image is 1.5.0 — restore into the matching image (selfpost:1.2.3) +backup: this backup was created by SelfPost 2.5.0 but this image is 2.5.1 — restore into the matching image (selfpost:2.5.0) ``` -Fix the tag in `docker-compose.yml`, `docker compose pull && docker compose up --d` again — the manifest is still there because the failed boot never got to +Fix the tag in `docker-compose.yml` (build that tag as under [Fixed image +tag](#fixed-image-tag) if it is not published), then `docker compose up -d` +again — the manifest is still there because the failed boot never got to delete it. **Moving to a different host** is the same flow: create an empty project directory, unpack the backup there, edit `.env` (and `docker-compose.yml` if needed) for the new hostname or proxy, then `docker compose up -d`. The archive -carries `certs/` from the old host — re-issue certificates when the hostname or -IP changes. Set up the reverse-proxy vhost separately (not in the backup). +carries `certs/` from the old host — re-issue certificates when the hostname +changes; a new IP needs only DNS (the A record and the PTR). Set up the reverse-proxy vhost separately (not in the backup). The inbound log and the filter lists are in the database and so in the backup, and so are the list files under `data/antispam/`; the panel writes @@ -911,10 +926,12 @@ docker exec -i selfpost-backup -decrypt < backup.spbk > backup.tar.g The CLI also *writes* encrypted backups for scripted/cron use. The password comes from `SELFPOST_BACKUP_PASSWORD` or `-password-file ` (first line), -never from a command-line argument, which would be visible in the process list: +never from a command-line argument, which would be visible in the process list. +`-e` with the name only takes the value from your shell's environment, so it is +not written on the command line: ```sh -docker exec -e SELFPOST_BACKUP_PASSWORD="$PW" selfpost-backup > backup.spbk +SELFPOST_BACKUP_PASSWORD="$PW" docker exec -e SELFPOST_BACKUP_PASSWORD selfpost-backup > backup.spbk ``` With no password set, the CLI keeps writing the plain `.tar.gz` it always has. @@ -948,7 +965,9 @@ with nowhere to send it. points at `SELFPOST_HOSTNAME`. The domain page shows the value to publish (`10 .`) and a check that succeeds when *any* MX host matches this server — other MX targets (a primary mail server) are fine; this is how -backup-MX is meant to work. Use *Re-check DNS* after publishing. +backup-MX is meant to work. When SelfPost is the backup of another MX, give it +a larger number than the primary's: MX records of equal preference are tried +in no fixed order, so the mail would be shared, not backed up. Use *Re-check DNS* after publishing. **Not an open relay.** The inbound smtpd offers no SASL. It accepts only domains in `relay_domains` and only listed recipients (`relay_recipient_maps`); @@ -969,7 +988,7 @@ docker compose -f docker-compose.yml -f antispam/docker-compose.antispam.yml up The milter sees the real client IP, HELO and PTR — unlike the upstream, which only sees SelfPost. Default action is fail-open (`accept`) so a down sidecar does not block backup-MX; set `INBOUND_ANTISPAM_MILTER_ACTION=tempfail` to -defer instead. +refuse with a temporary error instead, so the sending server tries again. The fragment mounts one file into the sidecar, [deploy/antispam/rspamd/milter_headers.conf](../deploy/antispam/rspamd/milter_headers.conf): @@ -998,8 +1017,8 @@ The same directory holds **allow and deny lists** kept in the panel. SelfPost does not apply the lists itself: it writes them as six files under `data/antispam/` (`allow_ip.map`, `allow_from.map`, `allow_domain.map` and the three `deny_` ones), the fragment -mounts that directory into the sidecar, and rspamd rereads a file within -seconds of a change - no restart. An entry is a sender address, a sender +mounts that directory into the sidecar, and rspamd rereads a changed file within +about a minute - no restart. An entry is a sender address, a sender domain (that domain, not its subdomains) or a client address or network. A denied sender is refused outright. An allowed one is given a weight that no spam score outweighs, so it is accepted - unless a deny entry matches the same @@ -1007,7 +1026,8 @@ message: **deny wins**, whichever entries they are. **A sender address or domain on the allow list is not proof of anything**: it is the envelope sender, which the sending server states and nobody checks, so whoever knows an allowed address can send as it. Allow a client address or network where you -can. The files are readable by +can. Only the envelope sender is looked at: a bounce, whose envelope sender +is empty, matches no sender entry, whatever its `From:` line says. The files are readable by anyone on the host; they hold addresses, not secrets. In the panel the lists are **Inbound › Filter lists** (global role), and what @@ -1032,12 +1052,12 @@ so a user with the *domain* role sees the state without the server's internals. For the global role the filter is also one of the milter sockets on **Server › Health** and in the Overview card: the panel connects to the address (one second at most, nothing is sent) and, if there is no answer, says whether -inbound mail is being deferred (`tempfail`) or going through unfiltered +inbound mail is being refused with a temporary error (`tempfail`) or going through unfiltered (`accept`). **Inbound mail is not in the Outbound log.** Port 25 runs its own milter chain: the optional anti-spam milter, and nothing else. OpenDKIM does not sign relayed -mail (and a signing outage therefore cannot defer it), and the journal-milter +mail (and a signing outage therefore cannot hold it up), and the journal-milter does not file it — the [Outbound log](#outbound-log) stays a record of what *you* sent. This also keeps a forged `From:` on inbound mail from counting against a @@ -1082,8 +1102,9 @@ is no mailbox for the server as a whole. **DNS.** Publish the usual `_dmarc` TXT on each sending domain with `rua=mailto:…` pointing at the address the domain shows. Receivers deliver to the -address domain — publish **MX** for `SELFPOST_HOSTNAME` (or the report -address domain if different) so reports reach this server. If a hub domain +address's domain: to its MX, or, without one, to its A record - +`SELFPOST_HOSTNAME` has that already; a report address on another domain needs +that domain's **MX** to point here. If a hub domain authorises external destinations, publish `_report._dmarc` there too; the panel checks it on the domain page. @@ -1187,8 +1208,9 @@ What this stops working when you upgrade from an earlier version: DMARC requires. The check is strict: while the panel does not answer, Postfix answers with a -temporary error (`451`, and `454` to the login) and applications send again -later; nothing is accepted unchecked. **Server › Health** counts how often +temporary error (`451`, and `454` to the login): an application that queues +its mail sends it again later, one that sends directly gets an error to retry; +nothing is accepted unchecked. **Server › Health** counts how often that happens. *Export domain* writes a single-domain file; *Import a domain* on *Server › @@ -1216,9 +1238,9 @@ records; see [Inbound relay](#inbound-relay). it. Public mail hosts (Gmail, Outlook, …) cannot be used as external report destinations. -Skipping any of the three records is the single most common reason mail -lands in spam even though SelfPost delivered it correctly — DKIM passing -doesn't help if SPF/DMARC are absent. **Whenever you add a new domain in the +Publish all three: receivers weigh SPF, DKIM and DMARC together, and large +receivers require them for bulk senders - a domain without them is a common +way for mail SelfPost delivered correctly to land in spam. **Whenever you add a new domain in the panel, add its DNS records at the same time**, not later. Each domain's page has a *DNS records* box comparing the published DKIM @@ -1289,8 +1311,9 @@ 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. The same applies if the journal-milter itself does not answer — Postfix is -configured to accept rather than defer when it cannot reach it. (Since -`2.4.0` a panel that is down altogether defers mail anyway, through the +configured to accept rather than refuse it when it cannot reach it. (Since +`2.4.0` a panel that is down altogether makes Postfix refuse mail with a +temporary error anyway, through the [sender check](#who-an-application-may-send-as) of the same process.) Level 1 is the backstop that keeps working even when level 2 cannot run, and it is the only one of the two that does not depend on the panel process. @@ -1303,10 +1326,10 @@ rows of their domains. It refreshes its rows without resetting a filter being chosen. A row identifies its message and nothing more — time, sender, recipient, subject and status `queued` (accepted, not yet delivered), `sent` (handed off successfully), `deferred` (Postfix is -retrying), `bounced` (final failure), or refused before it was queued. -`bounced` is usually Postfix's word - a message it gave up on at the end of -its queue lifetime is `bounced` too, since Postfix returns it to the sender; -it is the panel's when Postfix no longer holds a message and `mail.log` never +retrying), `bounced` (final failure: refused for good by the receiving server, or given +up when the queue lifetime ran out), or refused before it was queued. +`bounced` is usually Postfix's word - it returns a message it gave up on to +the sender, as it does one refused for good; it is the panel's when Postfix no longer holds a message and `mail.log` never said what became of it (the log rotated away while the panel was down, or was not readable) - the panel then closes the row as a failure rather than show a delivery it cannot evidence, and diff --git a/docs/roadmap.md b/docs/roadmap.md index 970fb40..6d7df40 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -42,7 +42,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | from-header-binding | Hold a message's `From:` header against the application that sends it | shipped `2.4.0` | 9/9 | [plans/from-header-binding.md](plans/from-header-binding.md) | | panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | | schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) | -| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`, stages 1+2 `2.5.0`, 2b `2.5.1`; next stage 2c → `2.5.2`) | 44/81 | [audit-2026-10-12.md](audit-2026-10-12.md) | +| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`, stages 1+2 `2.5.0`, 2b `2.5.1`; next stage 2c → `2.5.2`) | 45/81 | [audit-2026-10-12.md](audit-2026-10-12.md) | **Recommended order** (not binding; owner, 2026-10-09: feature order follows development convenience): **panel-redesign** shipped as `2.0.0` (2026-10-11) and diff --git a/docs/security.md b/docs/security.md index 1348c8a..a56dda7 100644 --- a/docs/security.md +++ b/docs/security.md @@ -41,10 +41,12 @@ there since `1.9.0`, that **the level-2 rate limit and an application's client-address list held only for the first message of a connection** - see [Mail path](#mail-path-security-relevant) - which was reproduced on the test stand and fixed. The security one (Fable) found no Critical or High: a client's -HELO name could supply the sender, recipient and reply of a journal row and so -make a refusal look like the deny list's (the reply now ends at Postfix's own -`; from=<`); the journal's row cap was applied only every six hours, between -which refusals were written at whatever rate a stranger sent recipients (the +HELO name could supply the sender, recipient and reply of a journal row, as the +parser then read the line, and so make a refusal look like the deny list's +(Postfix itself logs a HELO name with a space, `<>()\";@` and non-printable +characters turned into `?`; the reply now ends at Postfix's own `; from=<`); +the journal's row cap was applied only every six hours, between which +refusals were written at whatever rate a stranger sent recipients (the tailer now cuts to the cap every 500 rows); and two behaviours are recorded under Accepted risks (the allow list keyed on an address anyone can claim, the list files readable by anyone). A third it named - with the filter down, a @@ -88,8 +90,10 @@ The panel is exposed to the internet — the items below are **not optional**. - The panel **generates** the password on creation or reissue and shows it **once**. -- In `sasldb2` it is stored in the form SASL requires (not plaintext held by the - panel); a lost password can only be reissued. +- In `sasldb2` it is stored in plaintext - Cyrus keeps `userPassword` as given + (Cyrus SASL 2.1.28, lib/server.c `sasl_setpass()`), and a domain export reads + it back from there. The panel never shows it again: a lost password is + reissued. `sasldb2` is as secret as the exports (§ Backup and domain export). ### Input and configuration @@ -174,7 +178,9 @@ assigned outbound domains and inbound domains **separately**, each list with - Every non-read request first passes the origin check (`originAllowed`, [internal/web/security.go](../internal/web/security.go)): `Sec-Fetch-Site` must be `same-origin`, or `Origin` must name the panel's own - host; the session cookie is `SameSite` as well. + host (the panel's `Referrer-Policy` is `same-origin`, so a browser puts the + panel's own origin on a form post of the panel); the session cookie is + `SameSite` as well. - There are **no CSRF tokens**. The decision, its price and the conditions for revisiting it are in the accepted risks and the ADR below. @@ -276,8 +282,9 @@ operator can act on it. it fits one queue id. Refusals are parsed from `mail.log` from the head of the line, with the reply ending at Postfix's own `; from=<` so that the HELO name after it - the client's text, - logged as written - cannot stand in for the sender, the recipient or the - reply; an address with a quote, a bracket or a space in it is not kept. + which Postfix logs with non-printable characters and ` <>()\";@` turned into + `?` (Postfix 3.7.11, src/smtpd/smtpd.c `helo_cmd()`) - cannot stand in for + the sender, the recipient or the reply; an address with a quote, a bracket or a space in it is not kept. Only a recipient at an inbound domain makes a row. The journal is cut to 10 000 rows every 500 rows the tailer writes, and by age every six hours. - **Filter lists** reach another container as files: values are whitelisted on @@ -464,10 +471,13 @@ deferred item from the roadmap. - ~~**A row of the inbound log belongs to a domain by name.**~~ Struck in `2.5.0` (audit 2026-10-12, L12): both logs now hold a domain by id, see § Roles. -- **Level-2 in-flight reservations can outlive a dropped SMTP session until TTL.** - The journal milter cannot observe connection-close directly through - go-milter, so if a client disconnects without an explicit `Abort`, its - in-memory reservation is released by TTL expiry, not immediately. The map is +- **Level-2 in-flight reservations can outlive a broken SMTP session until TTL.** + Postfix tells the milters ABORT when a client drops the connection + mid-transaction (Postfix 3.7.11, src/smtpd/smtpd.c `mail_reset()` -> + `milter_abort()`), and the journal milter releases the reservation then. + When that never arrives - smtpd killed, the milter connection lost - go-milter + has no close callback, and the in-memory reservation is released by TTL + expiry, not immediately. The map is bounded by key and TTL (currently 10 minutes), so the effect is temporary over-counting on that app/domain key under churn. Accepted deliberately: immediate cleanup would require protocol hooks the current milter interface @@ -477,7 +487,9 @@ deferred item from the roadmap. directory is `2750 postfix:selfpost` and the file is `0640`: `postlogd` (user `postfix`) writes, the panel reads through the shared `selfpost` group, and the file is inaccessible to others. The log holds envelope addresses and - client IPs, but neither message bodies nor headers; it is excluded from + client IPs and SASL logins, the `Message-ID` of each message and the replies + given - for a refused `From:` line the reply quotes it (printable ASCII, + cut) - but no message bodies and no other headers; it is excluded from backups (`log/` is skipped) so that a dump stays state rather than diagnostics. Since 2.2.0 the tailer copies the lines of queued outbound messages into the database for the message's page, and those are in a @@ -513,9 +525,11 @@ deferred item from the roadmap. every submission: availability of sending now depends on the panel, which it did not before `2.4.0`. Chosen by the owner over the alternative, a check that is absent whenever the panel is - that one could be switched - off by whoever can make the panel slow. Supervisord restarts the panel at - once and takes the container down after three failed starts, so the - outage is seconds or the whole instance; Server › Health counts the + off by whoever can make the panel slow. Supervisord starts a panel that + died again at once; one that dies within a second of starting is tried + four times in a few seconds, and then SelfPost's `crashexit` listener takes + the container down (supervisor 4.2.5, `startretries=3`), so the outage is + seconds or the whole instance; Server › Health counts the downtime and the mail deferred for it. Trigger to revisit: those numbers not staying at nothing - then the check moves into a process of its own ([plans/from-header-binding.md](plans/from-header-binding.md)). diff --git a/internal/dmarc/parse.go b/internal/dmarc/parse.go index e80434f..a3adfb0 100644 --- a/internal/dmarc/parse.go +++ b/internal/dmarc/parse.go @@ -60,7 +60,7 @@ type feedbackXML struct { Domain string `xml:"domain"` P string `xml:"p"` SP string `xml:"sp"` - Pct int `xml:"pct"` + Pct *int `xml:"pct"` ADKIM string `xml:"adkim"` ASPF string `xml:"aspf"` } `xml:"policy_published"` @@ -132,12 +132,15 @@ func ParseAggregate(raw []byte) (ParsedReport, error) { Domain: strings.ToLower(field(doc.Policy.Domain)), PolicyP: field(doc.Policy.P), PolicySP: field(doc.Policy.SP), - PolicyPct: doc.Policy.Pct, PolicyADKIM: field(doc.Policy.ADKIM), PolicyASPF: field(doc.Policy.ASPF), } - if out.PolicyPct == 0 { - out.PolicyPct = 100 + // RFC 7489 § 6.3: pct is "between 0 and 100, inclusive", 100 when absent. + // An explicit 0 is a policy applied to no message, and stays 0; only a + // report without the element gets the default (assumptions C-M7). + out.PolicyPct = 100 + if doc.Policy.Pct != nil { + out.PolicyPct = *doc.Policy.Pct } for _, rec := range doc.Records { row := ParsedRecord{ diff --git a/internal/dmarc/parse_test.go b/internal/dmarc/parse_test.go index ab61a57..7cf24a6 100644 --- a/internal/dmarc/parse_test.go +++ b/internal/dmarc/parse_test.go @@ -5,6 +5,7 @@ import ( "bytes" "compress/gzip" "errors" + "strings" "testing" "time" @@ -258,3 +259,26 @@ func gzipMessage(t *testing.T, xml string) string { "Content-Type: application/gzip; name=\"report.xml.gz\"\r\n" + "\r\n" + gz.String() } + +// pct is 100 when the report leaves it out (RFC 7489 § 6.3), and an explicit +// 0 - a policy applied to no message - stays 0 (assumptions C-M7). +func TestPolicyPct(t *testing.T) { + for _, c := range []struct { + pct string + want int + }{ + {"100", 100}, + {"0", 0}, + {"25", 25}, + {"", 100}, + } { + doc := strings.Replace(sampleXML, "100", c.pct, 1) + got, err := ParseAggregate([]byte(doc)) + if err != nil { + t.Fatalf("%q: %v", c.pct, err) + } + if got.PolicyPct != c.want { + t.Errorf("%q: PolicyPct %d, want %d", c.pct, got.PolicyPct, c.want) + } + } +} diff --git a/internal/milter/inflight.go b/internal/milter/inflight.go index 206ab92..33a8b73 100644 --- a/internal/milter/inflight.go +++ b/internal/milter/inflight.go @@ -6,12 +6,16 @@ import ( ) // reservationTTL bounds how long a message may stay reserved. A reservation is -// released at end-of-message or on ABORT, but a client that simply drops the -// connection after MAIL FROM produces neither callback (go-milter has no -// connection-close hook), and a reservation that never expired would count -// against the limit forever — a fail-closed drift this milter must not have. -// The TTL is generously longer than any realistic DATA transfer, so a message -// still being received is never dropped from the count. +// released at end-of-message or on ABORT. A client that drops the connection +// mid-transaction still produces ABORT: Postfix 3.7.11, src/smtpd/smtpd.c, +// smtpd_proto() on a lost connection runs mail_reset(), which calls +// milter_abort() (src/milter/milter8.c, milter8_abort(), SMFIC_ABORT) +// (assumptions B-G7). What produces neither is smtpd itself dying, or the +// milter connection breaking - and go-milter has no connection-close hook to +// catch that - so a reservation that never expired would count against the +// limit forever, a fail-closed drift this milter must not have. The TTL is +// generously longer than any realistic DATA transfer, so a message still +// being received is never dropped from the count. // // go-milter v0.4.1, server.go, type Milter: no callback for the end of a // connection; session.go, HandleMilterCommands(), returns on SMFIC_QUIT or a diff --git a/test/e2e/compose.override.yml b/test/e2e/compose.override.yml index acd65aa..6502118 100644 --- a/test/e2e/compose.override.yml +++ b/test/e2e/compose.override.yml @@ -25,9 +25,11 @@ services: restart: "no" build: # Resolved relative to --project-directory (.stage), NOT this file's own - # directory — compose build.context paths follow the project directory, - # unlike bind-mount volume paths, which is the same reason ./data and - # ./certs above need no path here at all. + # directory: with --project-directory, build.context and bind-mount + # volume paths alike resolve against it - which is why ./data, ./certs + # and the ./dns-stage and ./mail-stage below land in .stage + # (docs.docker.com/compose/how-tos/multiple-compose-files/merge/, "Paths + # are evaluated relative to the base file"; assumptions C-K8). context: ../../.. dockerfile: build/Dockerfile args: diff --git a/test/e2e/dns/Corefile b/test/e2e/dns/Corefile index 89c2eb5..ecaff1f 100644 --- a/test/e2e/dns/Corefile +++ b/test/e2e/dns/Corefile @@ -3,8 +3,10 @@ # the panel nor Postfix ever needs to resolve anything outside this zone here. # # The `file` plugin's own `reload` sub-directive (NOT the top-level `reload` -# plugin, which only watches the Corefile itself) polls db.zone's mtime/serial -# and reloads with no signal needed, so the test harness can rewrite the zone +# plugin, which only watches the Corefile itself) re-reads db.zone every +# second and takes it when its SOA serial has grown - the serial, not the +# mtime (CoreDNS v1.14.6, plugin/file/reload.go; dns_zone.go keeps it +# growing) - with no signal needed, so the test harness can rewrite the zone # file (to publish a domain's DKIM record) and just poll until the new record # is visible — no container restart, no sleep. e2e.test { diff --git a/test/e2e/dns_zone.go b/test/e2e/dns_zone.go index 66cd800..76df871 100644 --- a/test/e2e/dns_zone.go +++ b/test/e2e/dns_zone.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "strings" + "sync" "time" ) @@ -38,12 +39,16 @@ type txtRecord struct { // writeZone (re)writes the whole authoritative db.zone for e2e.test: the fixed // records CoreDNS/sink need to be reachable, plus whatever TXT records the -// test has published so far. CoreDNS's `reload 2s` (see dns/Corefile) picks up -// the new mtime with no signal needed. +// test has published so far. The file plugin's `reload 1s` (see dns/Corefile) +// reloads the zone when its SOA serial has grown - not on a new mtime: +// CoreDNS v1.14.6, plugin/file/reload.go, Reload() re-parses with the current +// serial and file.go, Parse(), refuses a zone whose serial did not change +// (coredns.io/plugins/file, "reload"; assumptions C-E2). So the serial only +// ever grows, by one at least, even for two writes within a second. func writeZone(stageDir string, extra []txtRecord) error { var b strings.Builder fmt.Fprintf(&b, "$ORIGIN e2e.test.\n$TTL 300\n") - fmt.Fprintf(&b, "@ IN SOA ns.e2e.test. admin.e2e.test. ( %d 3600 900 604800 300 )\n", time.Now().Unix()) + fmt.Fprintf(&b, "@ IN SOA ns.e2e.test. admin.e2e.test. ( %d 3600 900 604800 300 )\n", nextZoneSerial()) fmt.Fprintf(&b, "@ IN NS ns.e2e.test.\n") fmt.Fprintf(&b, "ns IN A 10.77.0.10\n") // The sink-MX: Postfix's outbound delivery for the recipient domain used by @@ -58,6 +63,20 @@ func writeZone(stageDir string, extra []txtRecord) error { return os.WriteFile(filepath.Join(stageDir, "dns-stage", "db.zone"), []byte(b.String()), 0o644) } +var zoneSerial struct { + sync.Mutex + last int64 +} + +// nextZoneSerial is the time in seconds, or one more than the last serial +// when that is not later. +func nextZoneSerial() int64 { + zoneSerial.Lock() + defer zoneSerial.Unlock() + zoneSerial.last = max(zoneSerial.last+1, time.Now().Unix()) + return zoneSerial.last +} + // chunkTXT splits a TXT value into <255-byte quoted strings (the DNS // limit, RFC 1035 § 3.3), which a DKIM verifier // concatenates back into one value (RFC 6376 § 3.6.2.2). diff --git a/test/e2e/dns_zone_test.go b/test/e2e/dns_zone_test.go new file mode 100644 index 0000000..59aa833 --- /dev/null +++ b/test/e2e/dns_zone_test.go @@ -0,0 +1,39 @@ +package e2e + +import ( + "os" + "path/filepath" + "regexp" + "strconv" + "testing" +) + +// Two zones written within one second still get two serials, the second +// larger: CoreDNS reloads a zone only when its serial grows (C-E2). +func TestZoneSerialGrows(t *testing.T) { + stage := t.TempDir() + if err := os.MkdirAll(filepath.Join(stage, "dns-stage"), 0o755); err != nil { + t.Fatal(err) + } + soa := regexp.MustCompile(`IN SOA ns\.e2e\.test\. admin\.e2e\.test\. \( (\d+) `) + serial := func() int64 { + t.Helper() + if err := writeZone(stage, nil); err != nil { + t.Fatal(err) + } + b, err := os.ReadFile(filepath.Join(stage, "dns-stage", "db.zone")) + if err != nil { + t.Fatal(err) + } + m := soa.FindSubmatch(b) + if m == nil { + t.Fatalf("no SOA serial in\n%s", b) + } + n, _ := strconv.ParseInt(string(m[1]), 10, 64) + return n + } + first, second := serial(), serial() + if second <= first { + t.Fatalf("serials %d then %d: the second write would not be served", first, second) + } +} diff --git a/test/e2e/main_test.go b/test/e2e/main_test.go index 5f3646e..fade600 100644 --- a/test/e2e/main_test.go +++ b/test/e2e/main_test.go @@ -116,6 +116,9 @@ func TestE2E(t *testing.T) { if err := checkInboundRelayOff(h); err != nil { t.Fatal(err) } + if err := checkCitedPackageVersions(h); err != nil { + t.Fatal(err) + } if err := checkPort25ConfigCombinations(h); err != nil { t.Fatal(err) } diff --git a/test/e2e/process_check.go b/test/e2e/process_check.go index 40d147b..23d3394 100644 --- a/test/e2e/process_check.go +++ b/test/e2e/process_check.go @@ -2,6 +2,9 @@ package e2e import ( "fmt" + "os" + "path/filepath" + "regexp" "strings" "time" ) @@ -83,3 +86,46 @@ func checkInboundRelayOff(s *stack) error { } return nil } + +// checkCitedPackageVersions compares the Debian packages the image runs with +// the versions their sources were read at (docs/development.md, the table +// between the cited-package-versions markers). The image installs unpinned +// from a moving bookworm, so a point release changes them on a rebuild +// without anyone reading what changed (assumptions C-D1). +func checkCitedPackageVersions(s *stack) error { + doc, err := os.ReadFile(filepath.Join(s.repoRoot, "docs", "development.md")) + if err != nil { + return err + } + text := string(doc) + start := strings.Index(text, "") + end := strings.Index(text, "") + if start < 0 || end < start { + return fmt.Errorf("docs/development.md has no cited-package-versions table") + } + row := regexp.MustCompile("(?m)^[|] `([^`]+)` [|] `([^`]+)` [|]$") + want := map[string]string{} + var names []string + for _, m := range row.FindAllStringSubmatch(text[start:end], -1) { + want[m[1]] = m[2] + names = append(names, m[1]) + } + if len(names) == 0 { + return fmt.Errorf("the cited-package-versions table lists no package") + } + out, err := s.execIn("selfpost", append([]string{"dpkg-query", "-W", "-f", "${Package} ${Version}\n"}, names...)...) + if err != nil { + return fmt.Errorf("dpkg-query: %v\n%s", err, out) + } + var diffs []string + for _, line := range strings.Split(strings.TrimSpace(out), "\n") { + f := strings.Fields(line) + if len(f) == 2 && want[f[0]] != f[1] { + diffs = append(diffs, fmt.Sprintf("%s: the image has %s, the citations were read at %s", f[0], f[1], want[f[0]])) + } + } + if len(diffs) > 0 { + return fmt.Errorf("packages changed under the citations - re-read them (docs/development.md):\n%s", strings.Join(diffs, "\n")) + } + return nil +} diff --git a/test/e2e/stack.go b/test/e2e/stack.go index 2ac2c3b..cccd102 100644 --- a/test/e2e/stack.go +++ b/test/e2e/stack.go @@ -63,10 +63,11 @@ func (s *stack) compose(args ...string) ([]byte, error) { }, args...) cmd := exec.Command("docker", full...) // deploy/docker-compose.yml interpolates ${SELFPOST_HOSTNAME:?...} at - // compose-file parse time, straight from the process environment — that - // happens before the override file's `environment:` mapping is even - // looked at (that mapping only reaches the container, not compose's own - // variable interpolation), so it has to be set here too. + // compose-file parse time, from the shell's environment or the project + // directory's .env, the shell first (docs.docker.com/compose/how-tos/ + // environment-variables/variable-interpolation/; assumptions C-K9) - the + // override file's `environment:` mapping only reaches the container, not + // compose's own interpolation - so it is set here, where it wins. cmd.Env = append(os.Environ(), "SELFPOST_HOSTNAME="+selfpostHostname) cmd.Env = append(cmd.Env, s.env...) var out bytes.Buffer diff --git a/test/e2e/stage.go b/test/e2e/stage.go index 568691e..d43e1aa 100644 --- a/test/e2e/stage.go +++ b/test/e2e/stage.go @@ -55,8 +55,11 @@ func prepareStage(s *stack) error { // writeDeployFiles puts docker-compose.yml and .env into the stage directory. // The stage directory is what the container sees as /selfpost-deploy, the // operator's project directory, and a full backup refuses to run unless both -// files are there to be archived. compose reads neither: the stand names its -// files with -f, and the .env repeats what compose is given anyway. +// files are there to be archived. compose reads the .env too - without +// --env-file it takes .env from --project-directory - but the shell's value +// set by stack.compose takes precedence, and the two agree anyway +// (docs.docker.com/compose/how-tos/environment-variables/variable-interpolation/; +// assumptions C-K9). The compose file itself is named with -f. func writeDeployFiles(s *stack) error { compose, err := os.ReadFile(s.baseComposeFile) if err != nil {