diff --git a/CHANGELOG.md b/CHANGELOG.md index 733efe6..3fa8e1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,75 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +Review pass over `1.4.0`…`1.9.1`: port 25 no longer shares the submission +milter chain, DMARC reports are validated before they are stored, and the auto +rate limit is correct for a non-default level-1 window. Schema migration +`0010` (index only). Upgrading is a tag bump. + +### Added + +- SQLite migration `0010_send_log_app_login_index.sql` — + `idx_send_log_app_login_created_at`. The journal-milter counts an + application's messages on every `MAIL FROM`, and the domain page computes + per-application statistics on every render; both were scanning the + `created_at` range because `app_login` was unindexed. +- DMARC aggregate reports are accepted as **zip** as well as gzip or plain XML + (several reporters ship a single-entry zip; those reports previously counted + as parse failures). +- e2e: `checkPort25ConfigCombinations` regenerates the Postfix configuration + inside the container for all four combinations of `INBOUND_RELAY_ENABLE` / + `DMARC_REPORTS_ENABLE` / `INBOUND_ANTISPAM_MILTER` and requires + `postfix check` to pass. The `1.9.1` crash-loop shipped in `1.4.0` and + survived three releases because the suite only ever exercised the flags-off + path. + +### Fixed + +- **Port 25 no longer runs the submission milter chain.** `smtpd_milters` is + now always overridden on `smtp/inet` — to the optional antispam milter, or + to nothing. Previously, with `INBOUND_ANTISPAM_MILTER` unset, inbound mail + inherited main.cf's chain: OpenDKIM (strict, `tempfail`) could defer relayed + mail during a signing outage, and the journal-milter filed inbound mail into + the outbound send log under the **sender's** domain — so Deliveries showed + other people's mail, and a forged `From:` on an inbound message consumed that + sending domain's level-2 rate-limit budget. [architecture.md](docs/architecture.md) + already described the intended behaviour; the code now matches it. +- **DMARC reports are validated before they are stored.** A report is accepted + only when the domain in `policy_published` is a sending domain configured + here and, for a per-domain hosted address, matches that address's `+tag`. + The rua address form is predictable, so anyone able to reach it could + previously file reports under any domain, skew the panel's alignment figures + and the tighten-`p=` hint, and evict genuine reports through the 500-report + retention cap. +- Auto level-2 rate limit: the ceiling is now scaled to the level-1 window + before the multiplier is applied. `ceil(avg msg/h × multiplier)` was stored + as the ceiling for a window of `RATE_LIMIT_WINDOW_SECONDS`, so any window + other than the default `3600` shifted the effective allowance by + `window/3600` (a 1800s window allowed twice the intended rate). A domain + that has sent anything now also gets a floor of 1. +- Single-domain export no longer drops rate limits silently when the store + errors — the error is returned instead of being swallowed by `err == nil && ok`. + ### Changed +- [schema-migrations.md](docs/schema-migrations.md) notes that migration `0009` + changed what the IP list *means* — permissive (those IPs got the application + ceiling) before 1.9.0, restrictive (every other IP refused) after. Its + conversion branch only fires for a database that carried application-scope + `allowed_ips` from before 1.9.0. +- Panel help, [guide.md](docs/guide.md) and [architecture.md](docs/architecture.md): + the client IP allow-list is a **sending** control, not an authentication + boundary — the SASL login still succeeds, the check runs in the + journal-milter at `MAIL FROM`, and it is fail-open like the rate limits. + architecture.md's rate-limit description still documented the pre-1.9.0 + trusted-IP semantics. +- Removed unused duplicates of the inbound validators (`Host`, `Port`, + `TLSMode`, `RecipientMode`, `MailboxInDomain`, `NormalizeHost`) from + `internal/web/validate` — the live path uses `internal/inbound/validate.go`; + the copies were reachable only from their own tests. +- `gofmt` across `cmd/` and `internal/` (10 files had drifted); dead branch + removed from `postfix.inboundNexthop`. +- The planned inbound-antispam migration is renumbered `0011_inbound_spam_log.sql`. - Operator and as-built docs aligned with the code after a full pass — [architecture.md](docs/architecture.md) (port 25 opens for inbound relay and/or DMARC ingest; antispam requires inbound relay; DMARC pipe transport diff --git a/build/postfix-config.sh b/build/postfix-config.sh index 0c3075f..c83d767 100644 --- a/build/postfix-config.sh +++ b/build/postfix-config.sh @@ -301,6 +301,13 @@ if [ "${INBOUND_RELAY_ENABLE}" = "true" ] || [ "${DMARC_REPORTS_ENABLE}" = "true "smtp_tls_policy_maps=" fi + # Milters on port 25. main.cf's chain (OpenDKIM strict + journal-milter) is + # for the SUBMISSION path only: OpenDKIM must not gate someone else's + # inbound mail (a signing outage would defer it), and the journal-milter + # would file inbound mail into the outbound send log under the *sender's* + # domain — polluting Deliveries and letting a forged From: consume that + # domain's level-2 rate-limit budget. So smtpd_milters is always overridden + # here: either the optional antispam milter, or nothing at all. INBOUND_MILTERS="" if [ "${INBOUND_RELAY_ENABLE}" = "true" ] && [ -n "${INBOUND_ANTISPAM_MILTER}" ]; then case "${INBOUND_ANTISPAM_MILTER}" in @@ -346,9 +353,7 @@ if [ "${INBOUND_RELAY_ENABLE}" = "true" ] || [ "${DMARC_REPORTS_ENABLE}" = "true "smtp/inet/smtpd_client_message_rate_limit=${PORT25_RATE}" \ "smtp/inet/message_size_limit=${PORT25_SIZE}" master_cf_set_smtp_inet_o smtpd_recipient_restrictions "${RECIPIENT_RESTRICTIONS}" - if [ -n "${INBOUND_MILTERS}" ]; then - master_cf_set_smtp_inet_o smtpd_milters "${INBOUND_MILTERS}" - fi + master_cf_set_smtp_inet_o smtpd_milters "${INBOUND_MILTERS}" else postconf -MX "smtp/inet" 2>/dev/null || true postconf -MX "dmarc-ingest/unix-pipe" 2>/dev/null || true diff --git a/cmd/panel/httpserver.go b/cmd/panel/httpserver.go index f7ac478..35f1d9f 100644 --- a/cmd/panel/httpserver.go +++ b/cmd/panel/httpserver.go @@ -92,25 +92,25 @@ func newPanel(cfg config, st *store.Store) (*web.Server, error) { // snapshot for the process lifetime (architecture.md). retryPolicy := postfix.LoadRetryPolicy() return web.New(st, ms.Domains, ms.Apps, ms.Inbound, ms.DMARC, web.Config{ - Hostname: cfg.hostname, - CookieSecure: cfg.cookieSecure, - SubmissionEnabled: cfg.submissionEnabled, - MailLogPath: cfg.mailLog, - DataDir: cfg.dataDir, - DBPath: cfg.dbPath, - DeployRoot: cfg.deployRoot, - Version: buildinfo.Version, - TrustedProxyCIDRs: cfg.trustedProxies, - TLSCertFile: cfg.tlsCertFile, - OpenDKIMSocket: cfg.opendkimSocket, - JournalSocket: cfg.journalSocket, - SessionIdleDays: cfg.sessionIdleDays, - DNSResolvers: cfg.dnsResolvers, - RateLimitMessagesPerIP: cfg.rateLimitMessagesPerIP, - RateLimitWindowSeconds: cfg.rateLimitWindowSeconds, - RetryPolicy: retryPolicy, - InboundEnabled: cfg.inboundEnabled, - DMARCEnabled: cfg.dmarcEnabled, + Hostname: cfg.hostname, + CookieSecure: cfg.cookieSecure, + SubmissionEnabled: cfg.submissionEnabled, + MailLogPath: cfg.mailLog, + DataDir: cfg.dataDir, + DBPath: cfg.dbPath, + DeployRoot: cfg.deployRoot, + Version: buildinfo.Version, + TrustedProxyCIDRs: cfg.trustedProxies, + TLSCertFile: cfg.tlsCertFile, + OpenDKIMSocket: cfg.opendkimSocket, + JournalSocket: cfg.journalSocket, + SessionIdleDays: cfg.sessionIdleDays, + DNSResolvers: cfg.dnsResolvers, + RateLimitMessagesPerIP: cfg.rateLimitMessagesPerIP, + RateLimitWindowSeconds: cfg.rateLimitWindowSeconds, + RetryPolicy: retryPolicy, + InboundEnabled: cfg.inboundEnabled, + DMARCEnabled: cfg.dmarcEnabled, SendLogRetentionEnvDefault: cfg.retentionDays, }, cfg.setupTokenPath) } diff --git a/docs/architecture.md b/docs/architecture.md index 85377eb..3a99cca 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -73,7 +73,11 @@ smtp/inet` removes Debian's stock smtpd). Outbound delivery still uses the - **587/submission** — only when `SUBMISSION_ENABLE=true`; STARTTLS with `smtpd_tls_security_level=encrypt`. - **25/smtp inet** — when `INBOUND_RELAY_ENABLE=true` and/or - `DMARC_REPORTS_ENABLE=true`. No SASL, no OpenDKIM, no journal-milter. + `DMARC_REPORTS_ENABLE=true`. No SASL, no OpenDKIM, no journal-milter — + `smtpd_milters` is always overridden on this service (to the antispam milter + or to nothing), because main.cf's submission chain would otherwise let a + signing outage defer relayed mail and would file inbound mail into the + outbound send log under the sender's domain. **Inbound relay** (`INBOUND_RELAY_ENABLE=true`): accepts only `relay_domains` + `relay_recipient_maps` (`reject_unauth_destination`, `reject_unlisted_recipient`). Maps under `/data/postfix/` (`relay_domains`, @@ -117,9 +121,12 @@ One process, four roles: 2. **journal-milter** — unix socket `JOURNAL_MILTER_SOCKET`; records From/To/ Subject/SASL user at DATA; enforces level-2 rate limits; **fail-open** (`default_action=accept`) so milter failure does not stop mail. Domain - ceilings apply to every client IP; an application ceiling with trusted IPs - raises the limit for those IPs only and skips the domain check (guide § Rate - limiting). The level-2 count is the stored send-log rows plus the messages + ceilings apply to every client IP; an application ceiling **overrides** the + domain one for that login, higher or lower (guide § Rate limiting). Since + 1.9.0 the client IP allow-list is a separate, restrictive control on the + application (`applications.auth_ip_restrict`), checked here at `MAIL FROM` + and fail-open like the rest of this milter — not an authentication + boundary. The level-2 count is the stored send-log rows plus the messages this process has admitted but not yet written (`internal/milter/inflight.go`), so concurrent sessions cannot each spend the same last slot; a reservation is released at end-of-message, on ABORT, or after a 10-minute TTL. diff --git a/docs/guide.md b/docs/guide.md index 7360d48..d4d2b20 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -729,6 +729,13 @@ 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. +**Inbound mail is not in Deliveries.** 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 +does not file it — [Deliveries](#deliveries) stays a record of what *you* sent. +This also keeps a forged `From:` on inbound mail from counting against a +sending domain's [level-2 rate limit](#rate-limiting--level-2-domain-and-application). + Inbound configuration lives in SQLite and `/data/postfix/` map files, so it is included in a [full backup](#full-backup-and-restore). Single-domain export/import is sending domains only. @@ -768,6 +775,15 @@ alone, `check_recipient_access` permits only configured report addresses; everything else is rejected. With inbound relay enabled too, both allow-lists apply. +**Reports are checked before they are stored.** The address form is +predictable, so the report XML is treated as untrusted input: a report is +accepted only when the domain it claims in `policy_published` is a sending +domain configured here, and — when it arrived at a per-domain hosted address — +only when that domain matches the address's `+tag`. Anything else is refused +and counted in *parse failures* on the DMARC page, so a stranger cannot file +reports under your domain or push genuine ones out through the retention cap. +Reports are accepted as gzip, zip, or plain XML. + Parsed report data lives in SQLite and is included in a [full backup](#full-backup-and-restore). @@ -844,9 +860,16 @@ sending as that domain. When unset, only level 1 applies for non-privileged senders. **Client IP allow-list (application)** — optional restriction on an -application: when enabled, list one or more client IPs that may authenticate -and submit mail as that application. When disabled, any client IP is allowed. -This is independent of rate limits. +application: when enabled, list one or more client IPs that may **submit** mail +as that application; a session from any other address is refused with a 4xx at +`MAIL FROM`. When disabled, any client IP is allowed. This is independent of +rate limits, and it takes exact addresses only — CIDR ranges are not parsed. + +It is **not an authentication boundary.** The SASL login itself still succeeds; +the check runs in the journal-milter, on the same fail-open path as level 2 +(below), so if the milter is unavailable the restriction is skipped rather than +enforced. Use it to narrow where an application may send from, not to contain a +leaked password — for that, regenerate the password. **Level 2 — application** — optional override of the domain limit for one application (≤ level 1). The ceiling may be **higher or lower** than the domain @@ -868,7 +891,10 @@ hours and on demand via **Recalculate now**. 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 is the backstop that keeps working even when level 2 cannot run. +The same applies if the milter itself is down — Postfix is configured to +accept rather than defer when it cannot reach it. 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. ### Deliveries diff --git a/docs/plans/inbound-antispam-panel.md b/docs/plans/inbound-antispam-panel.md index bb2c5da..2100744 100644 --- a/docs/plans/inbound-antispam-panel.md +++ b/docs/plans/inbound-antispam-panel.md @@ -125,7 +125,7 @@ New nav item **Inbound spam** (or subsection under **Inbound**) — journal tabl with filters (decision, domain, date range), list management on the same page or a tab. Mockup: add `docs/assets/panel-ui/inbound_spam.html` in the panel step. -## Schema (migration `0010_inbound_spam_log.sql`) +## Schema (migration `0011_inbound_spam_log.sql`) **`inbound_spam_log`** @@ -238,7 +238,7 @@ Target version cut: **`1.10.0`** (MINOR). One commit per step; [development.md](../development.md) § Plan checklists. - [x] Agree journal fields, retention, and RBAC (this plan § Decisions) — **Composer** -- [ ] Migration `0010_inbound_spam_log.sql` — **Composer** +- [ ] Migration `0011_inbound_spam_log.sql` — **Composer** - [ ] Inbound journal-milter + Postfix wiring — **Composer** - [ ] mail.log tailer: inbound reject rows — **Composer** - [ ] List CRUD + validation + atomic rspamd map sync — **Composer** diff --git a/docs/schema-migrations.md b/docs/schema-migrations.md index f876281..75e6e59 100644 --- a/docs/schema-migrations.md +++ b/docs/schema-migrations.md @@ -18,13 +18,14 @@ Documentation. | Field | Value | |---|---| -| Chain head | `user_version = 9` | -| Files | `0001_init.sql` … `0009_application_auth_ips.sql` (9 files) | +| Chain head | `user_version = 10` | +| Files | `0001_init.sql` … `0010_send_log_app_login_index.sql` (10 files) | | Database file | `/data/selfpost.db` (bind mount) | | Compatibility | 1.x MINOR releases must boot a `1.0.0` data directory | -Last updated with release **1.9.0** (`0009_application_auth_ips`); **1.9.1** -had no schema change. +Last updated in **[Unreleased]** (`0010_send_log_app_login_index`); **1.9.1** +had no schema change. The release column below records the cut a migration +shipped in — fill `0010`'s in at the next version cut. --- @@ -42,6 +43,7 @@ had no schema change. 0002_sessions.sql → user_version 2 … 0009_application_auth_ips.sql → user_version 9 +0010_send_log_app_login_index.sql → user_version 10 ``` **Implication:** never delete, rename, or reorder migration files while 1.x must @@ -63,6 +65,7 @@ replace the embedded set (see [Planned 2.x squash](#planned-2x-squash)). | 7 | `0007_rate_limit_auto.sql` | 1.6.0 | DDL | `rate_limits.mode`, `auto_multiplier`, `auto_updated_at` | | 8 | `0008_dmarc_reports.sql` | 1.7.0 | DDL | `dmarc_reports`, `dmarc_report_records` | | 9 | `0009_application_auth_ips.sql` | 1.9.0 | DDL + data | `applications.auth_ip_restrict`, `auth_allowed_ips`; move legacy app `rate_limits.allowed_ips` into auth columns; clear those IPs on rate limits | +| 10 | `0010_send_log_app_login_index.sql` | Unreleased | DDL | `idx_send_log_app_login_created_at` — app-scoped rate-limit counts and per-application send statistics were scanning the `created_at` range | **Kind:** *DDL* — schema only; *data* — `INSERT`/`UPDATE` that must stay correct for operators upgrading from older 1.x images. @@ -117,11 +120,17 @@ code. A 2.x baseline should define `users` directly and omit `admin`. ### Application trusted IPs (0009) -Before 1.9.0, client IP restriction for an application lived in -`rate_limits.allowed_ips` (`scope = application`). Migration 0009 copies non-empty -values into `applications.auth_allowed_ips`, sets `auth_ip_restrict = 1`, and -clears `rate_limits.allowed_ips` for application scope. Level-2 limits no longer -carry IP bindings; auth and rate limiting are separate concerns. +Before 1.9.0, client IPs for an application lived in `rate_limits.allowed_ips` +(`scope = application`). Migration 0009 copies non-empty values into +`applications.auth_allowed_ips`, sets `auth_ip_restrict = 1`, and clears +`rate_limits.allowed_ips` for application scope. Level-2 limits no longer carry +IP bindings; auth and rate limiting are separate concerns. + +Note that the two columns do not mean the same thing: the old list was +*permissive* (those IPs got the application ceiling, every other IP fell back +to the domain limit), the new one is *restrictive* (every IP not on the list is +refused for that application). The conversion branch only fires for a database +that carried application-scope `allowed_ips` from before 1.9.0. ### Profile DMARC email (0004 → 0005) diff --git a/internal/dmarc/addresses.go b/internal/dmarc/addresses.go index 24a0d08..b3902a2 100644 --- a/internal/dmarc/addresses.go +++ b/internal/dmarc/addresses.go @@ -29,3 +29,21 @@ func IsHostedOnHostname(addr, hostname string) bool { at := strings.LastIndex(addr, "@") return addr[at+1:] == hostname } + +// TaggedDomain returns the domain a hosted report address is tagged for +// (dmarc-reports+@ → ), or "" when addr carries no +// such tag. Used to check a report's claimed domain against the address it +// actually arrived at. +func TaggedDomain(addr string) string { + addr = strings.ToLower(strings.TrimSpace(addr)) + at := strings.LastIndex(addr, "@") + if at <= 0 { + return "" + } + local := addr[:at] + plus := strings.Index(local, "+") + if plus < 0 || local[:plus] != DefaultLocalPart { + return "" + } + return local[plus+1:] +} diff --git a/internal/dmarc/ingest.go b/internal/dmarc/ingest.go index 4296d0a..ece9f5f 100644 --- a/internal/dmarc/ingest.go +++ b/internal/dmarc/ingest.go @@ -18,6 +18,15 @@ import ( // IngestMessage parses a raw RFC 5322 message from r and stores any DMARC // aggregate attachment it finds. +// +// The report XML is unauthenticated input: anyone who can reach an allow-listed +// rua= address can post one, and the address form is predictable. So the domain +// the report claims is checked twice before anything is stored — it must be a +// domain this relay actually sends for, and, when the report arrived at a +// per-domain hosted address (dmarc-reports+@), it must be the +// domain that address is tagged for. Without this a stranger could file reports +// under someone else's domain, skew the panel's alignment figures, and push +// genuine reports out through the retention cap. func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt time.Time) error { raw, err := io.ReadAll(io.LimitReader(r, 12<<20)) if err != nil { @@ -31,6 +40,10 @@ func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt ti if err != nil { return err } + recipient = strings.ToLower(strings.TrimSpace(recipient)) + if err := checkReportDomain(st, parsed.Domain, recipient); err != nil { + return err + } rep := store.DMARCReport{ Domain: parsed.Domain, Reporter: parsed.Reporter, @@ -46,7 +59,7 @@ func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt ti PolicyASPF: parsed.PolicyASPF, PassCount: parsed.PassCount, FailCount: parsed.FailCount, - Recipient: strings.ToLower(strings.TrimSpace(recipient)), + Recipient: recipient, } for _, rec := range parsed.Records { rep.Records = append(rep.Records, store.DMARCReportRecord{ @@ -64,6 +77,26 @@ func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt ti return st.PruneDMARCReports() } +// checkReportDomain refuses a report whose policy domain is not one of this +// relay's sending domains, or that contradicts the +tag of the address it was +// delivered to. +func checkReportDomain(st *store.Store, domain, recipient string) error { + if domain == "" { + return fmt.Errorf("dmarc report: no policy domain") + } + if tagged := TaggedDomain(recipient); tagged != "" && tagged != domain { + return fmt.Errorf("dmarc report: domain %q does not match the address it was sent to (%s)", domain, recipient) + } + ok, err := st.DomainExists(domain) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("dmarc report: %q is not a domain configured on this relay", domain) + } + return nil +} + func extractAggregatePayload(raw []byte) ([]byte, error) { msg, err := mail.ReadMessage(bytes.NewReader(raw)) if err != nil { diff --git a/internal/dmarc/parse.go b/internal/dmarc/parse.go index 1cfdc97..080a6e2 100644 --- a/internal/dmarc/parse.go +++ b/internal/dmarc/parse.go @@ -1,6 +1,7 @@ package dmarc import ( + "archive/zip" "bytes" "compress/gzip" "encoding/xml" @@ -73,9 +74,9 @@ type feedbackXML struct { } `xml:"record"` } -// ParseAggregate decodes gzip-compressed or raw DMARC aggregate XML. +// ParseAggregate decodes gzip- or zip-compressed, or raw, DMARC aggregate XML. func ParseAggregate(raw []byte) (ParsedReport, error) { - data, err := maybeGunzip(raw) + data, err := decompress(raw) if err != nil { return ParsedReport{}, err } @@ -128,20 +129,59 @@ func dmarcRecordPasses(r ParsedRecord) bool { return strings.EqualFold(r.SPFResult, "pass") || strings.EqualFold(r.DKIMResult, "pass") } -func maybeGunzip(raw []byte) ([]byte, error) { - if len(raw) >= 2 && raw[0] == 0x1f && raw[1] == 0x8b { - zr, err := gzip.NewReader(bytes.NewReader(raw)) - if err != nil { - return nil, fmt.Errorf("dmarc gzip: %w", err) +// maxAggregateXML bounds the decompressed payload, so a small archive cannot +// expand into an unbounded allocation. +const maxAggregateXML = 8 << 20 + +// decompress unwraps the container the reporter used. Both are seen in the +// wild: most senders gzip the XML, some (notably several Microsoft and Yahoo +// endpoints) ship it as a single-entry zip. +func decompress(raw []byte) ([]byte, error) { + switch { + case len(raw) >= 2 && raw[0] == 0x1f && raw[1] == 0x8b: + return gunzip(raw) + case len(raw) >= 4 && raw[0] == 'P' && raw[1] == 'K' && raw[2] == 0x03 && raw[3] == 0x04: + return unzip(raw) + } + return raw, nil +} + +func gunzip(raw []byte) ([]byte, error) { + zr, err := gzip.NewReader(bytes.NewReader(raw)) + if err != nil { + return nil, fmt.Errorf("dmarc gzip: %w", err) + } + defer zr.Close() + data, err := io.ReadAll(io.LimitReader(zr, maxAggregateXML)) + if err != nil { + return nil, fmt.Errorf("dmarc gzip read: %w", err) + } + return data, nil +} + +// unzip returns the first .xml entry of a zip archive. Directories and any +// other members are skipped rather than trusted. +func unzip(raw []byte) ([]byte, error) { + zr, err := zip.NewReader(bytes.NewReader(raw), int64(len(raw))) + if err != nil { + return nil, fmt.Errorf("dmarc zip: %w", err) + } + for _, f := range zr.File { + if f.FileInfo().IsDir() || !strings.HasSuffix(strings.ToLower(f.Name), ".xml") { + continue } - defer zr.Close() - data, err := io.ReadAll(io.LimitReader(zr, 8<<20)) + rc, err := f.Open() if err != nil { - return nil, fmt.Errorf("dmarc gzip read: %w", err) + return nil, fmt.Errorf("dmarc zip open %s: %w", f.Name, err) + } + data, err := io.ReadAll(io.LimitReader(rc, maxAggregateXML)) + rc.Close() + if err != nil { + return nil, fmt.Errorf("dmarc zip read %s: %w", f.Name, err) } return data, nil } - return raw, nil + return nil, fmt.Errorf("dmarc zip: no xml entry") } // TightenPolicyHint summarises whether raising p= looks reasonable. diff --git a/internal/dmarc/parse_test.go b/internal/dmarc/parse_test.go index 9579d8b..0d46993 100644 --- a/internal/dmarc/parse_test.go +++ b/internal/dmarc/parse_test.go @@ -1,6 +1,7 @@ package dmarc import ( + "archive/zip" "bytes" "compress/gzip" "testing" @@ -76,6 +77,9 @@ func TestIngestMessageStoresReport(t *testing.T) { t.Fatal(err) } defer st.Close() + if _, err := st.AddDomain("example.com", "selfpost"); err != nil { + t.Fatal(err) + } var gz bytes.Buffer zw := gzip.NewWriter(&gz) @@ -114,3 +118,99 @@ func TestHostedReportAddress(t *testing.T) { t.Fatalf("got %q want %q", got, want) } } + +func TestIngestMessageRejectsUnknownDomain(t *testing.T) { + st, err := store.Open(t.TempDir() + "/test.db") + if err != nil { + t.Fatal(err) + } + defer st.Close() + + if err := IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), "dmarc-reports@mail.example.com", time.Now().UTC()); err == nil { + t.Fatal("report for an unconfigured domain was accepted") + } + list, err := st.ListDMARCReports(nil, 10) + if err != nil { + t.Fatal(err) + } + if len(list) != 0 { + t.Fatalf("stored %d report(s) for an unconfigured domain", len(list)) + } +} + +// A report claiming example.com must not be accepted at the address tagged for +// another domain, even when both domains are configured here. +func TestIngestMessageRejectsTagMismatch(t *testing.T) { + st, err := store.Open(t.TempDir() + "/test.db") + if err != nil { + t.Fatal(err) + } + defer st.Close() + if _, err := st.AddDomain("example.com", "selfpost"); err != nil { + t.Fatal(err) + } + if _, err := st.AddDomain("other.example", "selfpost"); err != nil { + t.Fatal(err) + } + + err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), + "dmarc-reports+other.example@mail.example.com", time.Now().UTC()) + if err == nil { + t.Fatal("report was accepted at an address tagged for another domain") + } +} + +func TestTaggedDomain(t *testing.T) { + cases := map[string]string{ + "dmarc-reports+example.com@mail.example.com": "example.com", + "dmarc-reports@mail.example.com": "", + "postmaster+example.com@mail.example.com": "", + "not-an-address": "", + } + for addr, want := range cases { + if got := TaggedDomain(addr); got != want { + t.Errorf("TaggedDomain(%q) = %q, want %q", addr, got, want) + } + } +} + +func TestParseAggregateFromZip(t *testing.T) { + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + w, err := zw.Create("report.xml") + if err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte(sampleXML)); err != nil { + t.Fatal(err) + } + if err := zw.Close(); err != nil { + t.Fatal(err) + } + + got, err := ParseAggregate(buf.Bytes()) + if err != nil { + t.Fatalf("ParseAggregate(zip): %v", err) + } + if got.Domain != "example.com" || got.PassCount != 10 { + t.Fatalf("zip report = %+v", got) + } +} + +// gzipMessage wraps xml in the single-part gzip message the ingest pipe sees. +func gzipMessage(t *testing.T, xml string) string { + t.Helper() + var gz bytes.Buffer + zw := gzip.NewWriter(&gz) + if _, err := zw.Write([]byte(xml)); err != nil { + t.Fatal(err) + } + if err := zw.Close(); err != nil { + t.Fatal(err) + } + return "From: noreply@google.com\r\n" + + "To: dmarc-reports@mail.example.com\r\n" + + "Subject: Report\r\n" + + "Content-Type: application/gzip; name=\"report.xml.gz\"\r\n" + + "\r\n" + gz.String() +} diff --git a/internal/domain/transfer.go b/internal/domain/transfer.go index 36dde8f..a3b9d5f 100644 --- a/internal/domain/transfer.go +++ b/internal/domain/transfer.go @@ -19,14 +19,14 @@ const FormatDomainExport = "selfpost-domain-export" // transfer without regeneration). The file is therefore as sensitive as a full // backup and must be handled as a secret. type DomainExport struct { - Format string `json:"format"` - Version string `json:"version"` - Domain string `json:"domain"` - DKIMSelector string `json:"dkim_selector"` - DKIMPrivateKey string `json:"dkim_private_key"` // PKCS#1 PEM - DMARCRua *string `json:"dmarc_rua,omitempty"` // nil = inherit profile; set = override ("" = none) + Format string `json:"format"` + Version string `json:"version"` + Domain string `json:"domain"` + DKIMSelector string `json:"dkim_selector"` + DKIMPrivateKey string `json:"dkim_private_key"` // PKCS#1 PEM + DMARCRua *string `json:"dmarc_rua,omitempty"` // nil = inherit profile; set = override ("" = none) RateLimit *RateLimitExport `json:"rate_limit,omitempty"` - Applications []AppExport `json:"applications"` + Applications []AppExport `json:"applications"` } // RateLimitExport is the transferable level-2 limit for a domain or application. @@ -40,13 +40,13 @@ type RateLimitExport struct { // AppExport is one application within a DomainExport. type AppExport struct { - Login string `json:"login"` - AddressMode string `json:"address_mode"` - Addresses []string `json:"addresses,omitempty"` // list mode only - Password string `json:"password"` - AuthIPRestrict bool `json:"auth_ip_restrict,omitempty"` - AuthAllowedIPs []string `json:"auth_allowed_ips,omitempty"` - RateLimit *RateLimitExport `json:"rate_limit,omitempty"` + Login string `json:"login"` + AddressMode string `json:"address_mode"` + Addresses []string `json:"addresses,omitempty"` // list mode only + Password string `json:"password"` + AuthIPRestrict bool `json:"auth_ip_restrict,omitempty"` + AuthAllowedIPs []string `json:"auth_allowed_ips,omitempty"` + RateLimit *RateLimitExport `json:"rate_limit,omitempty"` } // Export builds the transferable representation of a domain: its DKIM key, its @@ -78,7 +78,11 @@ func (s *Service) Export(id int64) (DomainExport, error) { s := d.DMARCRua.String exp.DMARCRua = &s } - if rl, ok, err := s.store.GetRateLimit(store.RateLimitScopeDomain, id); err == nil && ok { + rl, ok, err := s.store.GetRateLimit(store.RateLimitScopeDomain, id) + if err != nil { + return DomainExport{}, fmt.Errorf("export rate limit for %s: %w", d.Name, err) + } + if ok { exp.RateLimit = exportRateLimit(rl) } for _, a := range apps { @@ -94,8 +98,12 @@ func (s *Service) Export(id int64) (DomainExport, error) { AuthIPRestrict: a.AuthIPRestrict, AuthAllowedIPs: a.AuthAllowedIPs, } - if rl, ok, err := s.store.GetRateLimit(store.RateLimitScopeApp, a.ID); err == nil && ok { - appExp.RateLimit = exportRateLimit(rl) + appRL, ok, err := s.store.GetRateLimit(store.RateLimitScopeApp, a.ID) + if err != nil { + return DomainExport{}, fmt.Errorf("export rate limit for %s: %w", a.Login, err) + } + if ok { + appExp.RateLimit = exportRateLimit(appRL) } exp.Applications = append(exp.Applications, appExp) } diff --git a/internal/postfix/inbound.go b/internal/postfix/inbound.go index 680f2ac..5b833d3 100644 --- a/internal/postfix/inbound.go +++ b/internal/postfix/inbound.go @@ -2,7 +2,6 @@ package postfix import ( "fmt" - "net" "path/filepath" "sort" "strconv" @@ -83,12 +82,10 @@ func renderInboundMaps(routes []InboundRoute) (relay, transport, recipients, tls return []byte(relayB.String()), []byte(transportB.String()), []byte(recipB.String()), []byte(tlsB.String()), nil } -// inboundNexthop is the Postfix next-hop [host]:port form that disables MX -// lookup for the explicit upstream. +// inboundNexthop is the Postfix next-hop [host]:port form. The brackets are +// what disable the MX lookup, so the operator's explicit upstream is used as +// given; they also delimit a bare IPv6 address. func inboundNexthop(host string, port int) string { - if ip := net.ParseIP(host); ip != nil && ip.To4() == nil { - return "[" + host + "]:" + strconv.Itoa(port) - } return "[" + host + "]:" + strconv.Itoa(port) } diff --git a/internal/store/applications.go b/internal/store/applications.go index 6a94b5c..c7f7e56 100644 --- a/internal/store/applications.go +++ b/internal/store/applications.go @@ -29,14 +29,14 @@ const ( // hashed — so it can be shown exactly once at creation/regeneration // (security.md). Addresses is populated only in 'list' mode. type Application struct { - ID int64 - DomainID int64 - Login string - AddressMode string - CreatedAt time.Time - Addresses []string - AuthIPRestrict bool // when true, only AuthAllowedIPs may submit as this login - AuthAllowedIPs []string // client IPs permitted when AuthIPRestrict is set + ID int64 + DomainID int64 + Login string + AddressMode string + CreatedAt time.Time + Addresses []string + AuthIPRestrict bool // when true, only AuthAllowedIPs may submit as this login + AuthAllowedIPs []string // client IPs permitted when AuthIPRestrict is set } // AllowsAuthFromIP reports whether a client at ip may submit mail authenticated diff --git a/internal/store/domains.go b/internal/store/domains.go index 0c28e2a..6983f95 100644 --- a/internal/store/domains.go +++ b/internal/store/domains.go @@ -4,6 +4,7 @@ import ( "database/sql" "errors" "fmt" + "strings" "time" "modernc.org/sqlite" @@ -182,3 +183,17 @@ func (s *Store) UpdateDomainDMARCRua(id int64, rua sql.NullString) error { } return nil } + +// DomainExists reports whether a sending domain with this name is configured. +// Used by the DMARC ingest path to refuse reports for domains this relay does +// not send for (security.md: report XML is attacker-supplied input). +func (s *Store) DomainExists(name string) (bool, error) { + var n int + err := s.db.QueryRow( + "SELECT COUNT(*) FROM domains WHERE name = ? COLLATE NOCASE", strings.TrimSpace(name), + ).Scan(&n) + if err != nil { + return false, fmt.Errorf("domain exists %q: %w", name, err) + } + return n > 0, nil +} diff --git a/internal/store/migrations/0010_send_log_app_login_index.sql b/internal/store/migrations/0010_send_log_app_login_index.sql new file mode 100644 index 0000000..35626ac --- /dev/null +++ b/internal/store/migrations/0010_send_log_app_login_index.sql @@ -0,0 +1,9 @@ +-- Send-log lookups by application login run on two hot paths: the journal +-- milter counts messages for a level-2 rate limit on every MAIL FROM, and the +-- domain page computes 30-day statistics per application on every render. +-- 0001 indexed queue_id, domain and created_at, but not app_login, so both +-- fell back to scanning the created_at range. The composite matches the shape +-- of those queries (equality on app_login, range on created_at). + +CREATE INDEX IF NOT EXISTS idx_send_log_app_login_created_at + ON send_log (app_login, created_at); diff --git a/internal/store/ratelimit_auto.go b/internal/store/ratelimit_auto.go index 92e8f67..7820e3a 100644 --- a/internal/store/ratelimit_auto.go +++ b/internal/store/ratelimit_auto.go @@ -116,7 +116,7 @@ func (s *Store) recalcAutoRateLimit(rl RateLimit, retentionDays, l1Max, l1Window return fmt.Errorf("unknown scope %q", rl.Scope) } - maxMsgs := computeAutoMaxMessages(stats, mult, l1Max) + maxMsgs := computeAutoMaxMessages(stats, mult, l1Max, l1Window) rl.MaxMessages = maxMsgs rl.WindowSeconds = l1Window @@ -128,14 +128,26 @@ func (s *Store) recalcAutoRateLimit(rl RateLimit, retentionDays, l1Max, l1Window return s.SetRateLimit(rl) } -func computeAutoMaxMessages(stats SendStats, multiplier float64, l1Max int) int { +// computeAutoMaxMessages turns an average send rate into a ceiling for one +// level-1 window. AvgPerHour is per hour, the ceiling counts messages per +// windowSeconds, so the rate is scaled to the window before the multiplier is +// applied — otherwise a window other than the default 3600s would silently +// shift the effective allowance by windowSeconds/3600. The result is never +// below 1 for a domain that has sent anything, and never above level 1. +func computeAutoMaxMessages(stats SendStats, multiplier float64, l1Max, windowSeconds int) int { if stats.Total == 0 { return 0 } - max := int(math.Ceil(stats.AvgPerHour * multiplier)) + if windowSeconds <= 0 { + windowSeconds = 3600 + } + perWindow := stats.AvgPerHour * float64(windowSeconds) / 3600 + max := int(math.Ceil(perWindow * multiplier)) + if max < 1 { + max = 1 + } if max > l1Max { max = l1Max } return max } - diff --git a/internal/store/ratelimits_test.go b/internal/store/ratelimits_test.go index f562c7c..cabecba 100644 --- a/internal/store/ratelimits_test.go +++ b/internal/store/ratelimits_test.go @@ -154,8 +154,8 @@ func TestRateLimitActive(t *testing.T) { inactive := []RateLimit{ {}, {Scope: RateLimitScopeDomain, AllowedIPs: []string{"203.0.113.1"}}, // no ceiling - {Scope: RateLimitScopeDomain, MaxMessages: 5}, // no window - {Scope: RateLimitScopeApp, MaxMessages: 5}, // no window + {Scope: RateLimitScopeDomain, MaxMessages: 5}, // no window + {Scope: RateLimitScopeApp, MaxMessages: 5}, // no window } for i, rl := range inactive { if rl.Active() { @@ -261,3 +261,40 @@ func TestAutoRateLimitZeroTrafficInactive(t *testing.T) { t.Fatalf("zero traffic auto should be inactive: %+v", rl) } } + +// The auto ceiling counts messages per level-1 window, so the per-hour average +// must be scaled to that window: half an hour of traffic, not a full hour's. +func TestComputeAutoMaxMessagesScalesToWindow(t *testing.T) { + stats := SendStats{Total: 100, AvgPerHour: 10} + cases := []struct { + name string + windowSeconds int + want int + }{ + {"one hour", 3600, 20}, + {"half an hour", 1800, 10}, + {"two hours", 7200, 40}, + {"unset falls back to an hour", 0, 20}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := computeAutoMaxMessages(stats, 2.0, 1000, tc.windowSeconds) + if got != tc.want { + t.Fatalf("max = %d, want %d", got, tc.want) + } + }) + } +} + +func TestComputeAutoMaxMessagesBounds(t *testing.T) { + if got := computeAutoMaxMessages(SendStats{}, 2.0, 1000, 3600); got != 0 { + t.Fatalf("no traffic: max = %d, want 0 (inactive)", got) + } + // A trickle still has to leave room for one message. + if got := computeAutoMaxMessages(SendStats{Total: 1, AvgPerHour: 0.001}, 1.0, 1000, 3600); got != 1 { + t.Fatalf("trickle: max = %d, want 1", got) + } + if got := computeAutoMaxMessages(SendStats{Total: 100, AvgPerHour: 500}, 2.0, 100, 3600); got != 100 { + t.Fatalf("capped: max = %d, want the level-1 ceiling 100", got) + } +} diff --git a/internal/store/stats.go b/internal/store/stats.go index 9e97f09..2fecdd3 100644 --- a/internal/store/stats.go +++ b/internal/store/stats.go @@ -15,7 +15,7 @@ type SendStats struct { Total int64 PeakPerHour int64 AvgPerHour float64 - WindowDays int // actual days queried (min(30, retention)) + WindowDays int // actual days queried (min(30, retention)) WindowHours float64 // hours used for the average-rate denominator } diff --git a/internal/web/handlers/handlers_dmarc_reports.go b/internal/web/handlers/handlers_dmarc_reports.go index 41f564a..6b35cd7 100644 --- a/internal/web/handlers/handlers_dmarc_reports.go +++ b/internal/web/handlers/handlers_dmarc_reports.go @@ -14,9 +14,9 @@ import ( type dmarcListRow struct { store.DMARCReportSummary - DomainID int64 - ReceivedLabel string - PeriodLabel string + DomainID int64 + ReceivedLabel string + PeriodLabel string } func (h *Handlers) requireDMARC(w http.ResponseWriter, r *http.Request) (auth.Principal, bool) { diff --git a/internal/web/handlers/handlers_monitor.go b/internal/web/handlers/handlers_monitor.go index 4292d79..b3ce9f2 100644 --- a/internal/web/handlers/handlers_monitor.go +++ b/internal/web/handlers/handlers_monitor.go @@ -108,12 +108,12 @@ func (h *Handlers) HandleDelivery(w http.ResponseWriter, r *http.Request) { row.Subject = mailhdr.DecodeSubject(row.Subject) logRows, logNote := h.deliveryLog(row) h.view.Render(w, http.StatusOK, "delivery", map[string]any{ - "Title": "SelfPost — delivery", - "User": auth.CurrentUser(r), - "Active": "deliveries", - "IsGlobal": p.IsGlobal(), - "SendLogRetentionDays": h.sendLogRetentionDays(), - "Row": row, + "Title": "SelfPost — delivery", + "User": auth.CurrentUser(r), + "Active": "deliveries", + "IsGlobal": p.IsGlobal(), + "SendLogRetentionDays": h.sendLogRetentionDays(), + "Row": row, // The status in the panel's own badge vocabulary, so the headline reads // the same way as every other health signal in the panel. "Level": deliveryLevel(row.Status), diff --git a/internal/web/handlers/handlers_ratelimit_test.go b/internal/web/handlers/handlers_ratelimit_test.go index d094e75..4caaa51 100644 --- a/internal/web/handlers/handlers_ratelimit_test.go +++ b/internal/web/handlers/handlers_ratelimit_test.go @@ -82,7 +82,7 @@ func TestParseAppAuthIPsForm(t *testing.T) { restrict, ips, err := parseAppAuthIPsForm(form(url.Values{ "auth_ip_restrict": {"1"}, - "auth_allowed_ips": {"203.0.113.10"}, + "auth_allowed_ips": {"203.0.113.10"}, })) if err != nil || !restrict || len(ips) != 1 { t.Fatalf("enabled with IP = restrict=%v ips=%v err=%v", restrict, ips, err) @@ -107,15 +107,15 @@ func TestParseDomainRateLimitFormAuto(t *testing.T) { return r } in, err := parseDomainRateLimitForm(form(url.Values{ - "mode": {"auto"}, - "auto_multiplier": {"2.5"}, + "mode": {"auto"}, + "auto_multiplier": {"2.5"}, }), 100) if err != nil || in.mode != "auto" || in.autoMultiplier != 2.5 { t.Fatalf("auto domain = %+v err=%v", in, err) } _, err = parseDomainRateLimitForm(form(url.Values{ - "mode": {"auto"}, - "auto_multiplier": {"10"}, + "mode": {"auto"}, + "auto_multiplier": {"10"}, }), 100) if err == nil { t.Fatal("multiplier out of range should fail") diff --git a/internal/web/validate/validate.go b/internal/web/validate/validate.go index da8b72d..6ba5cde 100644 --- a/internal/web/validate/validate.go +++ b/internal/web/validate/validate.go @@ -3,8 +3,6 @@ package validate import ( "fmt" - "net" - "strconv" "strings" "unicode" ) @@ -148,104 +146,3 @@ func Email(addr string) error { } return nil } - -const maxHostLen = 253 - -// NormalizeHost trims, lower-cases, and strips wrapping IPv6 brackets so the -// stored value is a bare hostname or IP, safe to wrap again when writing maps. -func NormalizeHost(host string) string { - host = strings.ToLower(strings.TrimSpace(host)) - if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") { - host = host[1 : len(host)-1] - } - return host -} - -// Host enforces a whitelist for an upstream hostname or IP (security.md): a -// dotted domain, a single DNS label (LAN names), or an IPv4/IPv6 address. -func Host(host string) error { - if host == "" { - return fmt.Errorf("host is required") - } - if len(host) > maxHostLen { - return fmt.Errorf("host must be at most %d characters", maxHostLen) - } - if ip := net.ParseIP(host); ip != nil { - return nil - } - labels := strings.Split(host, ".") - for _, label := range labels { - if err := domainLabel(label); err != nil { - return fmt.Errorf("host is invalid: %w", err) - } - } - return nil -} - -// Port checks a TCP port number parsed from form input. -func Port(raw string) (int, error) { - raw = strings.TrimSpace(raw) - if raw == "" { - return 0, fmt.Errorf("port is required") - } - n, err := strconv.Atoi(raw) - if err != nil || n < 1 || n > 65535 { - return 0, fmt.Errorf("port must be between 1 and 65535") - } - return n, nil -} - -// TLSMode checks a Postfix smtp_tls_policy_maps level. -func TLSMode(mode string) error { - switch mode { - case "may", "encrypt", "none": - return nil - default: - return fmt.Errorf("invalid TLS mode") - } -} - -// RecipientMode checks an inbound-domain recipient policy. -func RecipientMode(mode string) error { - switch mode { - case "list", "any": - return nil - default: - return fmt.Errorf("invalid recipient mode") - } -} - -// MailboxInDomain checks that addr is a conservative mailbox on domain -// (security.md). domain must already be normalised. -func MailboxInDomain(addr, domain string) error { - at := strings.LastIndexByte(addr, '@') - if at <= 0 || at >= len(addr)-1 { - return fmt.Errorf("%q is not a valid email address", addr) - } - local, host := addr[:at], addr[at+1:] - if host != domain { - return fmt.Errorf("%q does not belong to domain %s", addr, domain) - } - if err := mailboxLocalPart(local); err != nil { - return fmt.Errorf("%q: %w", addr, err) - } - return nil -} - -func mailboxLocalPart(local string) error { - if local == "" { - return fmt.Errorf("missing the part before '@'") - } - if local[0] == '.' || local[len(local)-1] == '.' { - return fmt.Errorf("local part must not start or end with '.'") - } - for i := 0; i < len(local); i++ { - c := local[i] - lower := c >= 'a' && c <= 'z' - digit := c >= '0' && c <= '9' - if !lower && !digit && c != '.' && c != '-' && c != '_' && c != '+' { - return fmt.Errorf("local part may contain only lower-case letters, digits, '.', '-', '_' and '+'") - } - } - return nil -} diff --git a/internal/web/validate/validate_test.go b/internal/web/validate/validate_test.go index cbd3cd8..6a20bf4 100644 --- a/internal/web/validate/validate_test.go +++ b/internal/web/validate/validate_test.go @@ -65,69 +65,3 @@ func TestValidateDomainLongLabelRejected(t *testing.T) { t.Error("expected error for over-long label") } } - -func TestHostValid(t *testing.T) { - valid := []string{ - "10.0.0.8", - "192.0.2.20", - "2001:db8::1", - "mail.internal.example", - "mx1", - "mail-1.lan", - } - for _, h := range valid { - if err := Host(h); err != nil { - t.Errorf("Host(%q) unexpected error: %v", h, err) - } - } -} - -func TestHostInvalid(t *testing.T) { - invalid := []string{ - "", - "exa mple", - "host/name", - "host;rm", - "-bad", - "bad-", - "host\nname", - } - for _, h := range invalid { - if err := Host(h); err == nil { - t.Errorf("Host(%q) = nil, want error", h) - } - } -} - -func TestNormalizeHostStripsIPv6Brackets(t *testing.T) { - if got := NormalizeHost(" [2001:DB8::1] "); got != "2001:db8::1" { - t.Errorf("NormalizeHost IPv6 = %q", got) - } - if got := NormalizeHost("Mail.Example.COM"); got != "mail.example.com" { - t.Errorf("NormalizeHost hostname = %q", got) - } -} - -func TestPort(t *testing.T) { - n, err := Port("25") - if err != nil || n != 25 { - t.Fatalf("Port(25) = %d, %v", n, err) - } - for _, raw := range []string{"", "0", "65536", "abc", "-1"} { - if _, err := Port(raw); err == nil { - t.Errorf("Port(%q) = nil, want error", raw) - } - } -} - -func TestMailboxInDomain(t *testing.T) { - if err := MailboxInDomain("staff@lists.example.com", "lists.example.com"); err != nil { - t.Fatal(err) - } - if err := MailboxInDomain("staff@other.com", "lists.example.com"); err == nil { - t.Fatal("expected domain mismatch error") - } - if err := MailboxInDomain("bad addr@lists.example.com", "lists.example.com"); err == nil { - t.Fatal("expected local-part error") - } -} diff --git a/internal/web/view/templates/domain_detail.html b/internal/web/view/templates/domain_detail.html index d434e34..fae1611 100644 --- a/internal/web/view/templates/domain_detail.html +++ b/internal/web/view/templates/domain_detail.html @@ -330,8 +330,11 @@

Client IP allow-list {{if .AuthIPRestrict}}active{{else}}off{{end}}

-

When enabled, only these addresses may authenticate - and submit mail as this application. When off, any client IP is allowed.

+

When enabled, only these addresses may submit mail as + this application; a login from anywhere else is refused with a 4xx at + MAIL FROM. When off, any client IP is allowed. Like the + level-2 rate limit this runs in the journal-milter and is fail-open — + it is a sending control, not an authentication boundary.