From eea40bcd49d169507a03d5689da3c76407e1a1f5 Mon Sep 17 00:00:00 2001 From: Mix Date: Sun, 11 Oct 2026 23:07:10 +0300 Subject: [PATCH] inbound-antispam-panel: filter lists, written as rspamd maps Step 5/12 of docs/plans/inbound-antispam-panel.md. Store: AddSpamListEntry, ListSpamListEntries, DeleteSpamListEntry. internal/inbound/lists.go validates and normalises an entry - sender address, sender domain, client address or network (canonical text, an IPv4-mapped address as IPv4, /0 refused) - and writes the lists as six map files under /data/antispam (ANTISPAM_DIR), each atomically, 0644 in a 0755 directory: the filter is another user in another container. All six are written on every change and on every start with the relay on, empty ones included. An entry whose maps could not be written is taken back out of the table. The writer refuses a stored value with whitespace, a newline or # in it. The plan's allow.map / deny.map became one file per list and entry type: to rspamd an IP map and a sender map are different map types. deploy/antispam/rspamd/multimap.conf holds the rules; the fragment mounts it and ./data/antispam into the sidecar. Deny is a prefilter reject, allow a symbol weighing -100. Tried in a throwaway rspamd 3.11 container on the test server: - with allow as a prefilter accept, an address on both lists was accepted and an IP on both was refused - the rule that ran first decided; with the weight, deny won in all five mixed cases; - IPv4, IPv6, address (any case), domain (not its subdomains) match; - a rewritten file is reread in about ten seconds, no restart; - files that appear after rspamd started are loaded the same way. No page edits the lists yet (step 6). Checked on the test server, uncommitted tree: go vet ./... ok, go test ./... ok (22 packages), make e2e ok - with the relay on /data/antispam is 755 and holds the six maps, 644, empty. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 8 + build/entrypoint.sh | 9 + cmd/panel/httpserver.go | 9 +- cmd/panel/main.go | 18 +- deploy/antispam/docker-compose.antispam.yml | 4 + deploy/antispam/rspamd/multimap.conf | 59 +++++ docs/architecture.md | 12 + docs/guide.md | 13 ++ docs/plans/inbound-antispam-panel.md | 8 +- docs/roadmap.md | 2 +- internal/inbound/lists.go | 224 ++++++++++++++++++ internal/inbound/lists_test.go | 237 ++++++++++++++++++++ internal/store/inboundspam.go | 83 +++++++ test/e2e/port25_features_test.go | 17 ++ 14 files changed, 696 insertions(+), 7 deletions(-) create mode 100644 deploy/antispam/rspamd/multimap.conf create mode 100644 internal/inbound/lists.go create mode 100644 internal/inbound/lists_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 4516a9e..9d99a4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,14 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version next to the fragment before starting it** - Docker creates a directory in place of a missing file and the sidecar then fails to read its configuration. +- **Allow and deny lists for the spam filter**, kept by SelfPost and applied + by rspamd: sender addresses, sender domains, client addresses and networks. + They are written as six files under `data/antispam/`, which the fragment + mounts into the sidecar together with `antispam/rspamd/multimap.conf`; + rspamd rereads a changed file by itself. Deny wins over allow. The files + are there from the first start with the inbound relay on; the page that + edits the lists comes next. +- New setting `ANTISPAM_DIR` (default `/data/antispam`). - New setting `INBOUND_JOURNAL_MILTER_SOCKET` (default `/run/selfpost/inbound-journal.sock`). diff --git a/build/entrypoint.sh b/build/entrypoint.sh index e61a99e..824b725 100644 --- a/build/entrypoint.sh +++ b/build/entrypoint.sh @@ -112,6 +112,15 @@ for f in relay_domains transport relay_recipients tls_policy; do done chown panel:selfpost /data/postfix chmod 2750 /data/postfix + +# Filter lists for the external spam filter (written by the panel when +# INBOUND_RELAY_ENABLE=true, read by the antispam sidecar, which mounts this +# directory). The sidecar runs under a user of its own in another container, +# so the directory and its files are readable by anyone: they hold sender +# addresses and networks, no secrets. +mkdir -p /data/antispam +chown panel:selfpost /data/antispam +chmod 0755 /data/antispam chown panel:selfpost /data/postfix/sender_login_maps chmod 0640 /data/postfix/sender_login_maps diff --git a/cmd/panel/httpserver.go b/cmd/panel/httpserver.go index 5c84a76..0078fe5 100644 --- a/cmd/panel/httpserver.go +++ b/cmd/panel/httpserver.go @@ -24,6 +24,7 @@ type mailStack struct { Domains *domain.Service Apps *app.Service Inbound *inbound.Service + Lists *inbound.Lists DMARC *dmarc.Service pf *postfix.Postfix odk *domain.OpenDKIM @@ -40,7 +41,8 @@ func newMailStack(cfg config, st *store.Store) *mailStack { dmarcSvc := dmarc.NewService(st, pf, cfg.hostname, cfg.dmarcEnabled) return &mailStack{ Domains: domains, Apps: apps, Inbound: inb, DMARC: dmarcSvc, - pf: pf, odk: odk, inboundEnabled: cfg.inboundEnabled, dmarcEnabled: cfg.dmarcEnabled, + Lists: inbound.NewLists(st, cfg.antispamDir), + pf: pf, odk: odk, inboundEnabled: cfg.inboundEnabled, dmarcEnabled: cfg.dmarcEnabled, } } @@ -57,6 +59,11 @@ func (m *mailStack) Resync() error { if err := m.Inbound.Resync(); err != nil { return fmt.Errorf("inbound maps resync: %w", err) } + // The filter's configuration names all six list files; they exist from + // the first start, empty until an entry is added. + if err := m.Lists.Sync(); err != nil { + return fmt.Errorf("filter lists resync: %w", err) + } } if m.dmarcEnabled { if err := m.DMARC.Resync(); err != nil { diff --git a/cmd/panel/main.go b/cmd/panel/main.go index 1adc4d3..a14e0e1 100644 --- a/cmd/panel/main.go +++ b/cmd/panel/main.go @@ -23,6 +23,7 @@ import ( "github.com/mixeme/selfpost/internal/backup" "github.com/mixeme/selfpost/internal/buildinfo" "github.com/mixeme/selfpost/internal/dnscheck" + "github.com/mixeme/selfpost/internal/inbound" "github.com/mixeme/selfpost/internal/logtail" "github.com/mixeme/selfpost/internal/store" ) @@ -89,6 +90,9 @@ type config struct { saslRealm string postfixDir string deployRoot string + // Where the filter lists are written as map files for the external spam + // filter; the antispam compose fragment mounts it into the sidecar. + antispamDir string inboundEnabled bool // The optional milter on the inbound listener (INBOUND_ANTISPAM_MILTER) and @@ -162,7 +166,9 @@ func loadConfig() config { saslDBPath: envDefault("SASL_DB_PATH", filepath.Join(dataDir, "sasl", "sasldb2")), saslRealm: saslRealm(), postfixDir: envDefault("POSTFIX_DIR", filepath.Join(dataDir, "postfix")), - deployRoot: envDefault("SELFPOST_DEPLOY_ROOT", "/selfpost-deploy"), + + antispamDir: envDefault("ANTISPAM_DIR", filepath.Join(dataDir, "antispam")), + deployRoot: envDefault("SELFPOST_DEPLOY_ROOT", "/selfpost-deploy"), // Optional inbound relay (backup-MX / forwarder). Off unless exactly "true". inboundEnabled: os.Getenv("INBOUND_RELAY_ENABLE") == "true", inboundAntispamMilter: os.Getenv("INBOUND_ANTISPAM_MILTER"), @@ -297,6 +303,16 @@ func run() error { } } + // The filter lists as files, on every start: the spam filter's + // configuration names all six, and a data directory from before the lists + // existed has none. Not fatal - the lists are the filter's business, and + // the panel has mail to relay either way. + if cfg.inboundEnabled && !restored { + if err := inbound.NewLists(st, cfg.antispamDir).Sync(); err != nil { + log.Printf("filter lists bootstrap: %v", err) + } + } + if err := st.EnsureSendLogRetentionDays(cfg.retentionDays); err != nil { return fmt.Errorf("bootstrap send-log retention: %w", err) } diff --git a/deploy/antispam/docker-compose.antispam.yml b/deploy/antispam/docker-compose.antispam.yml index 63194f4..588dfbb 100644 --- a/deploy/antispam/docker-compose.antispam.yml +++ b/deploy/antispam/docker-compose.antispam.yml @@ -30,3 +30,7 @@ services: volumes: # The verdict headers SelfPost's inbound journal reads. - ./antispam/rspamd/milter_headers.conf:/etc/rspamd/local.d/milter_headers.conf:ro + # The allow and deny lists kept in the panel: the rules, and the files + # SelfPost writes them to. + - ./antispam/rspamd/multimap.conf:/etc/rspamd/local.d/multimap.conf:ro + - ./data/antispam:/etc/rspamd/selfpost:ro diff --git a/deploy/antispam/rspamd/multimap.conf b/deploy/antispam/rspamd/multimap.conf new file mode 100644 index 0000000..96d832f --- /dev/null +++ b/deploy/antispam/rspamd/multimap.conf @@ -0,0 +1,59 @@ +# Mounted into the sidecar as /etc/rspamd/local.d/multimap.conf. +# +# The allow and deny lists kept in the panel (Inbound > Filter lists). SelfPost +# writes them as six files under data/antispam/, which the fragment mounts at +# /etc/rspamd/selfpost; rspamd rereads a file when it changes, no restart. +# +# Deny wins over allow, whichever entries match. A denied sender is refused +# before anything else is looked at (a prefilter). An allowed one is not +# accepted outright - rspamd would then let whichever of the two rules ran +# first decide - but given a weight no spam score outweighs, which a refusal +# by the deny list still overrides. +# +# A domain entry is that domain, not its subdomains. + +SELFPOST_DENY_IP { + type = "ip"; + map = "/etc/rspamd/selfpost/deny_ip.map"; + prefilter = true; + action = "reject"; + message = "Refused by this server's deny list"; + description = "Client address on the deny list"; +} +SELFPOST_DENY_FROM { + type = "from"; + map = "/etc/rspamd/selfpost/deny_from.map"; + prefilter = true; + action = "reject"; + message = "Refused by this server's deny list"; + description = "Sender address on the deny list"; +} +SELFPOST_DENY_DOMAIN { + type = "from"; + filter = "email:domain"; + map = "/etc/rspamd/selfpost/deny_domain.map"; + prefilter = true; + action = "reject"; + message = "Refused by this server's deny list"; + description = "Sender domain on the deny list"; +} + +SELFPOST_ALLOW_IP { + type = "ip"; + map = "/etc/rspamd/selfpost/allow_ip.map"; + score = -100.0; + description = "Client address on the allow list"; +} +SELFPOST_ALLOW_FROM { + type = "from"; + map = "/etc/rspamd/selfpost/allow_from.map"; + score = -100.0; + description = "Sender address on the allow list"; +} +SELFPOST_ALLOW_DOMAIN { + type = "from"; + filter = "email:domain"; + map = "/etc/rspamd/selfpost/allow_domain.map"; + score = -100.0; + description = "Sender domain on the allow list"; +} diff --git a/docs/architecture.md b/docs/architecture.md index 9632939..8770c1e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -102,6 +102,18 @@ smtp/inet` removes Debian's stock smtpd). Outbound delivery still uses the journal is pruned every six hours: rows older than the `inbound_spam_log_retention_days` setting, then down to 10 000. + **Filter lists** (`inbound_spam_list`): allow and deny entries for the + external filter - sender address, sender domain, client address or network - + validated and normalised by `internal/inbound` (`lists.go`) and written as + six map files under `/data/antispam/` (`ANTISPAM_DIR`), each atomically, on + every change and on every start with the relay on. SelfPost never consults + 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 is a prefilter `reject`, an allow entry a symbol + weighing -100. Allow is deliberately not a prefilter `accept`: with two + prefilters rspamd lets the one that ran first decide, and deny then won or + lost by rule order (seen on rspamd 3.11). A refusal by the deny list + overrides any score, so deny wins whichever entries match. **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`, diff --git a/docs/guide.md b/docs/guide.md index ae963f0..59c286e 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -851,6 +851,19 @@ with the message, and the upstream receives them with the mail. Copy the `antispam/rspamd/` directory next to the fragment; without the file accepted mail is still recorded, with no verdict. +The same directory holds +[multimap.conf](../deploy/antispam/rspamd/multimap.conf), the rules for the +**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 +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 +message: **deny wins**, whichever entries they are. The files are readable by +anyone on the host; they hold addresses, not secrets. + The panel only reports this setting; it does not change it. Every inbound domain page has a **Spam filter** box — on or off, what happens to mail while the filter is down, and that the filter belongs to the server, not to the diff --git a/docs/plans/inbound-antispam-panel.md b/docs/plans/inbound-antispam-panel.md index 1cf3cc1..ee21ce7 100644 --- a/docs/plans/inbound-antispam-panel.md +++ b/docs/plans/inbound-antispam-panel.md @@ -102,13 +102,13 @@ Mirror send-log retention: | Entry types | sender address, sender domain (`@domain` suffix), client IP / CIDR | | Precedence | **deny overrides allow** | | Scope | instance-wide (not per inbound domain in v1) | -| Where applied | **rspamd map files** under `/data/antispam/` (allow.map, deny.map); documented compose fragment mounts them into the sidecar | +| Where applied | **rspamd map files** under `/data/antispam/`, one per list and entry type (`allow_ip.map`, `allow_from.map`, `allow_domain.map`, `deny_…`) - an IP map and a sender map are different map types to rspamd; documented compose fragment mounts them into the sidecar. Deny is a prefilter reject, allow a weight of -100 (as built: two prefilters do not give "deny overrides allow") | | SelfPost pre-check | **out of v1** — lists live in rspamd only | | Edit access | global administrator only | | When inactive | lists editable in the panel but UI warns that sync applies only when `INBOUND_ANTISPAM_MILTER` is set; `postfix reload` / rspamd reload documented in guide | -List change → atomic map rewrite (same pattern as Postfix maps) → rspamd reload -via documented operator step or sidecar `SIGHUP` in the compose fragment. +List change → atomic map rewrite (same pattern as Postfix maps). No reload: +rspamd rereads a changed map file by itself (about ten seconds on 3.11). ### Activation @@ -247,7 +247,7 @@ Target version cut: **`2.1.0`** (MINOR), after `2.0.0`. One commit per step; - [x] Migration `0002_inbound_spam_log.sql` on the 2.0 baseline — **Composer** - [x] Inbound journal-milter + Postfix wiring — **Composer** - [x] mail.log tailer: inbound reject rows — **Composer** -- [ ] List CRUD + validation + atomic rspamd map sync — **Composer** +- [x] List CRUD + validation + atomic rspamd map sync — **Composer** - [ ] Panel: Inbound › Log and Inbound › Filter lists from the kit partials, mockups `in-log` / `in-filter-lists` (add client IP and engine columns mockup-first), evidence pairs — **Composer** - [ ] Backup includes `/data/antispam/` — **Composer** - [ ] Unit + handler tests — **Composer** diff --git a/docs/roadmap.md b/docs/roadmap.md index 4ebbb4b..2710b30 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -30,7 +30,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). |---|---|---|---|---| | preflight | Installation check page (`/preflight`) | candidate | — | [plans/preflight.md](plans/preflight.md) | | contributing | `CONTRIBUTING.md` | candidate | — | — | -| inbound-antispam-panel | Inbound antispam journal + allow/deny lists | in progress | 4/12 | [plans/inbound-antispam-panel.md](plans/inbound-antispam-panel.md) | +| inbound-antispam-panel | Inbound antispam journal + allow/deny lists | in progress | 5/12 | [plans/inbound-antispam-panel.md](plans/inbound-antispam-panel.md) | | inbound-quarantine | Inbound spam quarantine (hold / review / release) | candidate | — | [plans/inbound-quarantine.md](plans/inbound-quarantine.md) | | csrf-tokens | Per-session CSRF tokens on state-changing forms | candidate | — | — | | template-data-typing | Typed structs for template data instead of `map[string]any` | candidate | — | — | diff --git a/internal/inbound/lists.go b/internal/inbound/lists.go new file mode 100644 index 0000000..3f6b281 --- /dev/null +++ b/internal/inbound/lists.go @@ -0,0 +1,224 @@ +package inbound + +import ( + "fmt" + "net/netip" + "os" + "path/filepath" + "sort" + "strings" + "unicode/utf8" + + "github.com/mixeme/selfpost/internal/atomicfile" + "github.com/mixeme/selfpost/internal/configsafe" + "github.com/mixeme/selfpost/internal/store" +) + +// ListStore is the slice of the store the filter lists need. *store.Store +// satisfies it. +type ListStore interface { + AddSpamListEntry(list, kind, value, note string) (store.SpamListEntry, error) + ListSpamListEntries() ([]store.SpamListEntry, error) + DeleteSpamListEntry(id int64) error +} + +// Lists owns the allow and deny lists of the external spam filter: the rows in +// SQLite and the map files the filter reads. SelfPost does not consult the +// lists itself - it writes them out, six files in one directory, one entry a +// line, and the filter (rspamd, deploy/antispam/rspamd/multimap.conf) rereads +// a file when it changes. Which list wins when both match is the filter's +// configuration: deny. +type Lists struct { + store ListStore + dir string +} + +// NewLists builds the filter lists over the store and the map directory +// (/data/antispam). +func NewLists(st ListStore, dir string) *Lists { + return &Lists{store: st, dir: dir} +} + +// noteMaxRunes caps the operator's comment on an entry. +const noteMaxRunes = 200 + +// mapFiles names the file of every (list, kind). An IP map and a sender map +// are different things to the filter, and a domain entry is matched against +// another part of the sender than an address, so each has its own file. +var mapFiles = map[[2]string]string{ + {store.SpamListAllow, store.SpamEntryIP}: "allow_ip.map", + {store.SpamListAllow, store.SpamEntryAddress}: "allow_from.map", + {store.SpamListAllow, store.SpamEntryDomain}: "allow_domain.map", + {store.SpamListDeny, store.SpamEntryIP}: "deny_ip.map", + {store.SpamListDeny, store.SpamEntryAddress}: "deny_from.map", + {store.SpamListDeny, store.SpamEntryDomain}: "deny_domain.map", +} + +// Entries returns every entry of both lists. +func (l *Lists) Entries() ([]store.SpamListEntry, error) { + return l.store.ListSpamListEntries() +} + +// Add validates an entry, stores it normalised and rewrites the maps. A +// refusal is a *ValidationError; an entry already on that list is +// store.ErrSpamListEntryExists. +func (l *Lists) Add(list, kind, rawValue, rawNote string) (store.SpamListEntry, error) { + if list != store.SpamListAllow && list != store.SpamListDeny { + return store.SpamListEntry{}, invalid("choose the allow or the deny list") + } + value, err := normalizeListValue(kind, rawValue) + if err != nil { + return store.SpamListEntry{}, err + } + note, err := normalizeNote(rawNote) + if err != nil { + return store.SpamListEntry{}, err + } + e, err := l.store.AddSpamListEntry(list, kind, value, note) + if err != nil { + return store.SpamListEntry{}, err + } + if err := l.Sync(); err != nil { + // An entry the filter never got is worse than no entry: the page + // would show a rule that does nothing. + _ = l.store.DeleteSpamListEntry(e.ID) + return store.SpamListEntry{}, err + } + return e, nil +} + +// Delete removes an entry and rewrites the maps. +func (l *Lists) Delete(id int64) error { + if err := l.store.DeleteSpamListEntry(id); err != nil { + return err + } + return l.Sync() +} + +// Sync writes all six map files from SQLite, each atomically, so the filter +// reads a whole old file or a whole new one. Every file is written even when +// empty: the filter's configuration names all six. +func (l *Lists) Sync() error { + entries, err := l.store.ListSpamListEntries() + if err != nil { + return err + } + lines := make(map[string][]string, len(mapFiles)) + for _, name := range mapFiles { + lines[name] = nil + } + for _, e := range entries { + name, ok := mapFiles[[2]string{e.List, e.Kind}] + if !ok { + return fmt.Errorf("filter list entry %d: unknown list %q or kind %q", e.ID, e.List, e.Kind) + } + // Values were validated on the way in; this is the writer's own guard + // against a row that got into the table some other way. In a map file a + // newline ends the entry, whitespace starts its value and # a comment. + if err := configsafe.Token("filter list entry", e.Value, " \t\r\n#"); err != nil { + return err + } + lines[name] = append(lines[name], e.Value) + } + // The filter runs under another user in another container: the directory + // and the files are readable by anyone. They hold addresses, not secrets. + if err := os.MkdirAll(l.dir, 0o755); err != nil { + return fmt.Errorf("create %s: %w", l.dir, err) + } + for name, vals := range lines { + sort.Strings(vals) + var b strings.Builder + b.WriteString("# Written by SelfPost (Inbound > Filter lists). Do not edit: the next change in the panel replaces this file.\n") + for _, v := range vals { + b.WriteString(v) + b.WriteByte('\n') + } + if err := atomicfile.Write(filepath.Join(l.dir, name), []byte(b.String()), 0o644); err != nil { + return err + } + } + return nil +} + +// normalizeListValue validates one entry and returns the form it is stored and +// written in: lower case for an address or a domain, the canonical text of an +// address or a network for an IP. +func normalizeListValue(kind, raw string) (string, error) { + v := strings.ToLower(strings.TrimSpace(raw)) + switch kind { + case store.SpamEntryAddress: + if v == "" { + return "", invalid("sender address is required") + } + if len(v) > 254 { + return "", invalid("sender address must be at most 254 characters") + } + at := strings.LastIndexByte(v, '@') + if at <= 0 || at >= len(v)-1 { + return "", invalid("%q is not a valid email address", v) + } + local, domain := v[:at], v[at+1:] + if local[0] == '.' || local[len(local)-1] == '.' || len(local) > 64 { + return "", invalid("%q: invalid local part", v) + } + for i := 0; i < len(local); i++ { + c := local[i] + lower := c >= 'a' && c <= 'z' + digit := c >= '0' && c <= '9' + // '=' for the rewritten senders of forwarders (SRS0=…). + if !lower && !digit && !strings.ContainsRune("._+-=", rune(c)) { + return "", invalid("%q: local part contains invalid characters", v) + } + } + if err := checkDomain(domain); err != nil { + return "", err + } + return v, nil + case store.SpamEntryDomain: + // "@example.com" is how a domain is often written in such a list. + v = strings.TrimPrefix(v, "@") + if err := checkDomain(v); err != nil { + return "", err + } + return v, nil + case store.SpamEntryIP: + if v == "" { + return "", invalid("address or network is required") + } + if strings.Contains(v, "/") { + p, err := netip.ParsePrefix(v) + if err != nil { + return "", invalid("%q is not a network in CIDR form (e.g. 192.0.2.0/24)", v) + } + if p.Addr().Is4In6() { + return "", invalid("%q: write an IPv4 network as IPv4 (e.g. 192.0.2.0/24)", v) + } + if p.Bits() == 0 { + return "", invalid("%q is every address there is", v) + } + return p.Masked().String(), nil + } + a, err := netip.ParseAddr(v) + if err != nil || a.Zone() != "" { + return "", invalid("%q is not an IP address or a network in CIDR form", v) + } + return a.Unmap().String(), nil + default: + return "", invalid("choose what the entry is: a sender address, a sender domain or a client address") + } +} + +// normalizeNote trims the operator's comment and refuses what a single line of +// text cannot be. +func normalizeNote(raw string) (string, error) { + note := strings.TrimSpace(raw) + if utf8.RuneCountInString(note) > noteMaxRunes { + return "", invalid("note must be at most %d characters", noteMaxRunes) + } + for _, r := range note { + if r < 0x20 || r == 0x7f || r == utf8.RuneError { + return "", invalid("note must be one line of plain text") + } + } + return note, nil +} diff --git a/internal/inbound/lists_test.go b/internal/inbound/lists_test.go new file mode 100644 index 0000000..b6579de --- /dev/null +++ b/internal/inbound/lists_test.go @@ -0,0 +1,237 @@ +package inbound + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mixeme/selfpost/internal/store" +) + +func testLists(t *testing.T) (*Lists, *store.Store, string) { + t.Helper() + st, err := store.Open(filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { st.Close() }) + dir := filepath.Join(t.TempDir(), "antispam") + return NewLists(st, dir), st, dir +} + +// mapLines reads a map file without its comment line. +func mapLines(t *testing.T, dir, name string) []string { + t.Helper() + raw, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + t.Fatalf("read %s: %v", name, err) + } + var out []string + for _, line := range strings.Split(strings.TrimRight(string(raw), "\n"), "\n") { + if !strings.HasPrefix(line, "#") { + out = append(out, line) + } + } + return out +} + +func TestNormalizeListValue(t *testing.T) { + for _, c := range []struct{ kind, raw, want string }{ + {store.SpamEntryAddress, " Billing@Vendor.Example ", "billing@vendor.example"}, + {store.SpamEntryAddress, "srs0=ab12=cd=example.org=user@forwarder.example", "srs0=ab12=cd=example.org=user@forwarder.example"}, + {store.SpamEntryDomain, "@Evil.Example", "evil.example"}, + {store.SpamEntryDomain, "sub.evil.example", "sub.evil.example"}, + {store.SpamEntryIP, "192.0.2.7", "192.0.2.7"}, + {store.SpamEntryIP, "192.0.2.77/24", "192.0.2.0/24"}, + {store.SpamEntryIP, "::ffff:192.0.2.7", "192.0.2.7"}, + {store.SpamEntryIP, "2001:DB8:0:0::1", "2001:db8::1"}, + {store.SpamEntryIP, "2001:db8:1::5/48", "2001:db8:1::/48"}, + } { + got, err := normalizeListValue(c.kind, c.raw) + if err != nil || got != c.want { + t.Errorf("normalizeListValue(%s, %q) = %q, %v; want %q", c.kind, c.raw, got, err, c.want) + } + } + + for _, c := range []struct{ kind, raw string }{ + {store.SpamEntryAddress, ""}, + {store.SpamEntryAddress, "no-at-sign"}, + {store.SpamEntryAddress, "@example.com"}, + {store.SpamEntryAddress, "a@localhost"}, + {store.SpamEntryAddress, "a b@example.com"}, + {store.SpamEntryAddress, "a#b@example.com"}, + {store.SpamEntryAddress, "a\nb@example.com"}, + {store.SpamEntryAddress, ".a@example.com"}, + {store.SpamEntryDomain, ""}, + {store.SpamEntryDomain, "example"}, + {store.SpamEntryDomain, "exa mple.com"}, + {store.SpamEntryDomain, "example.com # all of it"}, + {store.SpamEntryDomain, "user@example.com"}, + {store.SpamEntryIP, ""}, + {store.SpamEntryIP, "example.com"}, + {store.SpamEntryIP, "192.0.2.300"}, + {store.SpamEntryIP, "192.0.2.0/33"}, + {store.SpamEntryIP, "0.0.0.0/0"}, + {store.SpamEntryIP, "::/0"}, + {store.SpamEntryIP, "fe80::1%eth0"}, + {store.SpamEntryIP, "::ffff:192.0.2.0/120"}, + {store.SpamEntryIP, "192.0.2.7 192.0.2.8"}, + {"subject", "pills"}, + {"", "x"}, + } { + got, err := normalizeListValue(c.kind, c.raw) + var ve *ValidationError + if err == nil || !errors.As(err, &ve) { + t.Errorf("normalizeListValue(%q, %q) = %q, %v; want a ValidationError", c.kind, c.raw, got, err) + } + } +} + +func TestListsAddWritesTheMaps(t *testing.T) { + l, _, dir := testLists(t) + for _, e := range []struct{ list, kind, value string }{ + {store.SpamListAllow, store.SpamEntryAddress, "Billing@Vendor.Example"}, + {store.SpamListAllow, store.SpamEntryAddress, "accounts@vendor.example"}, + {store.SpamListDeny, store.SpamEntryDomain, "@evil.example"}, + {store.SpamListDeny, store.SpamEntryIP, "198.51.100.9/24"}, + // The same value on both lists is two entries: deny wins in the filter. + {store.SpamListDeny, store.SpamEntryAddress, "billing@vendor.example"}, + } { + if _, err := l.Add(e.list, e.kind, e.value, " why "); err != nil { + t.Fatalf("Add(%s, %s, %q): %v", e.list, e.kind, e.value, err) + } + } + + want := map[string]string{ + "allow_from.map": "accounts@vendor.example billing@vendor.example", + "allow_domain.map": "", + "allow_ip.map": "", + "deny_from.map": "billing@vendor.example", + "deny_domain.map": "evil.example", + "deny_ip.map": "198.51.100.0/24", + } + for name, lines := range want { + if got := strings.Join(mapLines(t, dir, name), " "); got != lines { + t.Errorf("%s = %q, want %q", name, got, lines) + } + // The filter is another user in another container. + if info, err := os.Stat(filepath.Join(dir, name)); err != nil { + t.Errorf("stat %s: %v", name, err) + } else if info.Mode().Perm()&0o004 == 0 && os.PathSeparator == '/' { + t.Errorf("%s is mode %o, not readable by the filter", name, info.Mode().Perm()) + } + } + if files, _ := os.ReadDir(dir); len(files) != len(want) { + t.Errorf("the directory holds %d files, want %d (no temp file left behind)", len(files), len(want)) + } + + entries, err := l.Entries() + if err != nil || len(entries) != 5 { + t.Fatalf("Entries = %d, %v", len(entries), err) + } + if entries[0].Note != "why" { + t.Errorf("note = %q, want it trimmed", entries[0].Note) + } +} + +func TestListsRefusals(t *testing.T) { + l, _, dir := testLists(t) + if _, err := l.Add(store.SpamListAllow, store.SpamEntryIP, "192.0.2.7", ""); err != nil { + t.Fatalf("Add: %v", err) + } + // Written differently, the same entry. + if _, err := l.Add(store.SpamListAllow, store.SpamEntryIP, "::ffff:192.0.2.7", ""); !errors.Is(err, store.ErrSpamListEntryExists) { + t.Errorf("the same address again: %v, want ErrSpamListEntryExists", err) + } + var ve *ValidationError + for name, add := range map[string]func() error{ + "an unknown list": func() error { _, err := l.Add("block", store.SpamEntryIP, "192.0.2.8", ""); return err }, + "a bad value": func() error { _, err := l.Add(store.SpamListDeny, store.SpamEntryIP, "nope", ""); return err }, + "a long note": func() error { + _, err := l.Add(store.SpamListDeny, store.SpamEntryIP, "192.0.2.8", strings.Repeat("x", noteMaxRunes+1)) + return err + }, + "a note of two lines": func() error { + _, err := l.Add(store.SpamListDeny, store.SpamEntryIP, "192.0.2.8", "one\ntwo") + return err + }, + } { + if err := add(); !errors.As(err, &ve) { + t.Errorf("%s: %v, want a ValidationError", name, err) + } + } + if got := strings.Join(mapLines(t, dir, "deny_ip.map"), " "); got != "" { + t.Errorf("deny_ip.map = %q after refusals only", got) + } + if entries, _ := l.Entries(); len(entries) != 1 { + t.Errorf("entries = %d, want the first one alone", len(entries)) + } +} + +func TestListsDelete(t *testing.T) { + l, _, dir := testLists(t) + e, err := l.Add(store.SpamListDeny, store.SpamEntryDomain, "evil.example", "") + if err != nil { + t.Fatalf("Add: %v", err) + } + if err := l.Delete(e.ID); err != nil { + t.Fatalf("Delete: %v", err) + } + if got := mapLines(t, dir, "deny_domain.map"); len(got) != 0 { + t.Errorf("deny_domain.map = %v after the delete", got) + } + if err := l.Delete(e.ID); !errors.Is(err, store.ErrSpamListEntryNotFound) { + t.Errorf("a second delete: %v", err) + } +} + +// An entry the filter never got must not stay on the page. +func TestListsAddRollsBackWhenTheMapsCannotBeWritten(t *testing.T) { + st, err := store.Open(filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatal(err) + } + defer st.Close() + // A file where the directory should be. + blocked := filepath.Join(t.TempDir(), "antispam") + if err := os.WriteFile(blocked, nil, 0o600); err != nil { + t.Fatal(err) + } + l := NewLists(st, blocked) + if _, err := l.Add(store.SpamListDeny, store.SpamEntryDomain, "evil.example", ""); err == nil { + t.Fatal("Add succeeded with nowhere to write the maps") + } + if entries, _ := l.Entries(); len(entries) != 0 { + t.Errorf("the entry stayed: %+v", entries) + } +} + +// The writer's own guard: a row that got into the table some other way is not +// written into a map. +func TestListsSyncRefusesAnUnsafeRow(t *testing.T) { + l, st, dir := testLists(t) + if _, err := st.AddSpamListEntry(store.SpamListDeny, store.SpamEntryDomain, "evil.example\ngood.example", ""); err != nil { + t.Fatalf("AddSpamListEntry: %v", err) + } + if err := l.Sync(); err == nil { + t.Fatal("Sync wrote a value with a newline in it") + } + if _, err := os.Stat(filepath.Join(dir, "deny_domain.map")); err == nil { + t.Error("a map file was written") + } +} + +// With nothing on the lists all six files are there, empty. +func TestListsSyncWritesEmptyMaps(t *testing.T) { + l, _, dir := testLists(t) + if err := l.Sync(); err != nil { + t.Fatalf("Sync: %v", err) + } + for _, name := range mapFiles { + if got := mapLines(t, dir, name); len(got) != 0 { + t.Errorf("%s = %v", name, got) + } + } +} diff --git a/internal/store/inboundspam.go b/internal/store/inboundspam.go index b4f5575..4357e3b 100644 --- a/internal/store/inboundspam.go +++ b/internal/store/inboundspam.go @@ -95,3 +95,86 @@ func (s *Store) IsInboundDomain(name string) (bool, error) { } return true, nil } + +// Lists and entry kinds of the filter lists. Kept in sync with the CHECK +// constraints of inbound_spam_list. +const ( + SpamListAllow = "allow" + SpamListDeny = "deny" + + SpamEntryAddress = "address" // sender address + SpamEntryDomain = "domain" // sender domain + SpamEntryIP = "ip" // client address or network +) + +// ErrSpamListEntryExists is returned when the entry is already on that list. +var ErrSpamListEntryExists = errors.New("filter list entry already exists") + +// ErrSpamListEntryNotFound is returned when a filter list entry id does not exist. +var ErrSpamListEntryNotFound = errors.New("filter list entry not found") + +// SpamListEntry is one allow or deny entry for the external filter. Value is +// stored normalised by the caller (internal/inbound), which is what makes the +// uniqueness of an entry on its list mean anything. +type SpamListEntry struct { + ID int64 + List string + Kind string + Value string + Note string + CreatedAt time.Time +} + +// AddSpamListEntry inserts an entry. The caller must have validated it. +func (s *Store) AddSpamListEntry(list, kind, value, note string) (SpamListEntry, error) { + now := time.Now().UTC() + res, err := s.db.Exec( + "INSERT INTO inbound_spam_list (list_type, entry_type, value, note, created_at) VALUES (?, ?, ?, ?, ?)", + list, kind, value, note, now.Format(time.RFC3339), + ) + if err != nil { + if isUniqueViolation(err) { + return SpamListEntry{}, ErrSpamListEntryExists + } + return SpamListEntry{}, fmt.Errorf("insert inbound_spam_list: %w", err) + } + id, err := res.LastInsertId() + if err != nil { + return SpamListEntry{}, fmt.Errorf("filter list entry id: %w", err) + } + return SpamListEntry{ID: id, List: list, Kind: kind, Value: value, Note: note, CreatedAt: now.Truncate(time.Second)}, nil +} + +// ListSpamListEntries returns every entry of both lists: by list, then kind, +// then value. +func (s *Store) ListSpamListEntries() ([]SpamListEntry, error) { + rows, err := s.db.Query( + "SELECT id, list_type, entry_type, value, note, created_at FROM inbound_spam_list ORDER BY list_type, entry_type, value") + if err != nil { + return nil, fmt.Errorf("list inbound_spam_list: %w", err) + } + defer rows.Close() + var out []SpamListEntry + for rows.Next() { + var e SpamListEntry + var created string + if err := rows.Scan(&e.ID, &e.List, &e.Kind, &e.Value, &e.Note, &created); err != nil { + return nil, fmt.Errorf("scan inbound_spam_list: %w", err) + } + e.CreatedAt, _ = time.Parse(time.RFC3339, created) + out = append(out, e) + } + return out, rows.Err() +} + +// DeleteSpamListEntry removes an entry by id. +func (s *Store) DeleteSpamListEntry(id int64) error { + res, err := s.db.Exec("DELETE FROM inbound_spam_list WHERE id = ?", id) + if err != nil { + return fmt.Errorf("delete inbound_spam_list: %w", err) + } + if n, _ := res.RowsAffected(); n == 0 { + return ErrSpamListEntryNotFound + } + return nil +} diff --git a/test/e2e/port25_features_test.go b/test/e2e/port25_features_test.go index f39ee0d..501623b 100644 --- a/test/e2e/port25_features_test.go +++ b/test/e2e/port25_features_test.go @@ -268,6 +268,23 @@ func testInboundRelay(t *testing.T, sc *scenario) { // The inbound journal milter saw it go through. No page shows the journal // yet, so the row is read where it is kept. No filter is attached on the // stand: the row is an accept by nobody. + // The filter lists exist as files from the first start with the relay on, + // readable by a filter that is another user in another container. Nothing + // is on them: no page edits them yet. + maps, err := h.execIn("selfpost", "sh", "-c", `cd /data/antispam && stat -c "%a %n" . *.map && { grep -vc "^#" *.map; true; }`) + if err != nil { + t.Fatalf("list /data/antispam: %v\n%s", err, maps) + } + for _, want := range []string{"755 .", + "644 allow_domain.map", "644 allow_from.map", "644 allow_ip.map", + "644 deny_domain.map", "644 deny_from.map", "644 deny_ip.map", + "allow_ip.map:0", "deny_from.map:0", + } { + if !strings.Contains(maps, want) { + t.Fatalf("/data/antispam: no %q in\n%s", want, maps) + } + } + // journalHas waits for a row and returns the journal as it then stands. journalHas := func(want string) string { t.Helper()