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()