diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index d9dadbb..4bf6661 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -875,7 +875,7 @@ those after it move up one patch number each. longer address is refused - cut there, a quoted local part could end in another tenant's domain. The check asks for header values with their leading space, as OpenDKIM does; the SigningTable reader goes. -- [ ] **The port checked against OpenDKIM itself.** On the stand, several +- [x] **The port checked against OpenDKIM itself.** On the stand, several hundred generated `From:` lines - quotes, escapes, comments nested and open, groups, brackets, encoded words, a second domain of the server in every place - are sent; for each, the domain the port names must be the @@ -883,6 +883,16 @@ those after it move up one patch number each. shared files of lines (`from-lines-passed.txt`, `from-lines-other.txt`) are kept: the second one's test becomes "never signed for {other}" rather than "refused". - **Opus** + *Done 2026-10-15:* not through Postfix, where the check refuses most of + the lines first, but straight to OpenDKIM: `TestPortAgainstOpenDKIM` + (internal/milter, test/upstream/opendkim-probe.sh) + starts OpenDKIM from the image with keys for three names and plays it 385 + lines - the two shared files and lines composed from display names, + comments, groups, routes, literals and quoted local parts - through + go-milter's client; for each, the d= OpenDKIM signs with is the domain the + port predicts (its address, cut at 256 bytes, against the SigningTable), + 385 of 385, 287 signed. The two lines padded to be cut at 256 bytes are + signed by OpenDKIM with the other domain's key, as read in the source. - [x] **The heuristics review: every assumption about another component, answered from its source** (owner, 2026-10-14: "нам нужно полное ревью кода на предмет всей этой эвристики. Её не должно там, где можно @@ -1057,9 +1067,10 @@ those after it move up one patch number each. PNG (D-B16); a published port nothing listens on is refused over IPv4, accepted by the proxy otherwise (D-guide:61); deny beats allow in every mix on rspamd 3.11.1-3 (D-arch:121); rspamd runs as uid 11333 (D-sec:424). -- [ ] Docs and wording: help topic sender-check and guide § Who an +- [x] Docs and wording: help topic sender-check and guide § Who an application may send as (mockup first), security.md § Mail path, CHANGELOG. - **Sonnet** + *Done 2026-10-15* with the port (`2cf118c` the help mockup, `3584adc`). ### Stage 3 — persistence and restore (M1–M5, L1–L4, L7) → `2.5.3` diff --git a/docs/roadmap.md b/docs/roadmap.md index d2963b2..c59ffeb 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -42,7 +42,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | from-header-binding | Hold a message's `From:` header against the application that sends it | shipped `2.4.0` | 9/9 | [plans/from-header-binding.md](plans/from-header-binding.md) | | panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | | schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) | -| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`, stages 1+2 `2.5.0`, 2b `2.5.1`; next stage 2c → `2.5.2`) | 47/81 | [audit-2026-10-12.md](audit-2026-10-12.md) | +| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`, stages 1+2 `2.5.0`, 2b `2.5.1`; next stage 2c → `2.5.2`) | 49/81 | [audit-2026-10-12.md](audit-2026-10-12.md) | **Recommended order** (not binding; owner, 2026-10-09: feature order follows development convenience): **panel-redesign** shipped as `2.0.0` (2026-10-11) and diff --git a/internal/milter/opendkim_probe_test.go b/internal/milter/opendkim_probe_test.go new file mode 100644 index 0000000..33871ae --- /dev/null +++ b/internal/milter/opendkim_probe_test.go @@ -0,0 +1,183 @@ +package milter + +import ( + "bufio" + "fmt" + "os" + "strings" + "testing" + "time" + + "github.com/emersion/go-milter" +) + +// TestPortAgainstOpenDKIM checks the port where it matters, in OpenDKIM +// itself: for every From: line, the domain OpenDKIM signs the message for +// must be the one the port predicts - the port's address, matched against +// the SigningTable as dkimf_apply_signtable() matches it, cut to 256 bytes +// (MAXADDRESS) - or both must say no signature. It runs where +// OPENDKIM_PROBE_SOCKET names a running OpenDKIM's socket and +// OPENDKIM_PROBE_TABLE its refile SigningTable of "*@" lines: +// test/upstream/opendkim-probe.sh starts one from the image and runs this +// (audit 2026-10-12, stage 2c). +func TestPortAgainstOpenDKIM(t *testing.T) { + sock, table := os.Getenv("OPENDKIM_PROBE_SOCKET"), os.Getenv("OPENDKIM_PROBE_TABLE") + if sock == "" || table == "" { + t.Skip("OPENDKIM_PROBE_SOCKET and OPENDKIM_PROBE_TABLE not set") + } + signed := map[string]bool{} + f, err := os.Open(table) + if err != nil { + t.Fatal(err) + } + sc := bufio.NewScanner(f) + for sc.Scan() { + if fields := strings.Fields(sc.Text()); len(fields) > 0 && strings.HasPrefix(fields[0], "*@") { + signed[strings.ToLower(fields[0][2:])] = true + } + } + f.Close() + + const own, other, server = "sender.e2e.test", "other.e2e.test", "mail.e2e.test" + lines := probeLines(t, own, other, server) + + agree, signedCount := 0, 0 + for i, line := range lines { + want := predictSigner(line, signed) + got, err := opendkimSigns(sock, line) + if err != nil { + t.Fatalf("From: %q: %v", line, err) + } + if i >= len(lines)-2 && got != other { + // The padded lines: cut at 256 bytes they end in "@", + // and OpenDKIM signs them with that domain's key - the reason + // the sender check refuses an address over 256 bytes. + t.Errorf("a padded line of %d bytes: OpenDKIM signs for %q, not %q", len(line), got, other) + } + if got != "" { + signedCount++ + } + if got != want { + t.Errorf("From: %q: OpenDKIM signs for %q, the port predicts %q", line, got, want) + continue + } + agree++ + } + t.Logf("%d From: lines, OpenDKIM and the port agree on %d; %d signed", len(lines), agree, signedCount) +} + +// predictSigner is the domain OpenDKIM signs a message with this From: line +// for, by the port: none when it reads no address; otherwise the domain of +// the SigningTable whose "^.*@$" (case-insensitive) matches +// "user@domain" as snprintf() leaves it in MAXADDRESS + 1 bytes, the domain +// first cut to DKIM_MAXHOSTNAMELEN by strlcpy() and lower-cased. +func predictSigner(line string, signed map[string]bool) string { + r := dkimMailParse(line) + if r.err != mailparseOK || !r.hasUser || !r.hasDomain || r.user == "" || r.domain == "" { + return "" + } + domain := r.domain + if len(domain) > 256 { + domain = domain[:256] + } + addr := r.user + "@" + asciiLower(domain) + if len(addr) > 256 { + addr = addr[:256] + } + d := asciiLower(addr[strings.LastIndexByte(addr, '@')+1:]) + if signed[d] { + return d + } + return "" +} + +// opendkimSigns plays one message to OpenDKIM as Postfix would for a client +// it trusts (127.0.0.1, its InternalHosts by default) and returns the d= of +// the signature it adds, lower-cased, or "". +func opendkimSigns(sock, from string) (string, error) { + cl := milter.NewClientWithOptions("unix", sock, milter.ClientOptions{ + ReadTimeout: 10 * time.Second, + WriteTimeout: 10 * time.Second, + ActionMask: milter.OptAddHeader | milter.OptChangeHeader, + }) + defer cl.Close() + s, err := cl.Session() + if err != nil { + return "", err + } + defer s.Close() + // A step OpenDKIM answers with anything but "continue" ends the message + // for it - an accept where it has nothing to do (libmilter, engine.c) - + // and it answers nothing after; no signature then. + step := func(act *milter.Action, err error) (bool, error) { + if err != nil { + return false, err + } + return act.Code == milter.ActContinue, nil + } + if ok, err := step(s.Conn("localhost", milter.FamilyInet, 40000, "127.0.0.1")); !ok { + return "", err + } + if ok, err := step(s.Mail("", nil)); !ok { + return "", err + } + if ok, err := step(s.Rcpt("", nil)); !ok { + return "", err + } + for _, h := range [][2]string{{"From", from}, {"To", "user@elsewhere.example"}, {"Subject", "probe"}, {"Date", "Thu, 15 Oct 2026 03:00:00 +0000"}} { + if ok, err := step(s.HeaderField(h[0], h[1])); !ok { + return "", err + } + } + if ok, err := step(s.HeaderEnd()); !ok { + return "", err + } + // No body chunk: OpenDKIM signs an empty body as well. + mods, _, err := s.End() + if err != nil { + return "", fmt.Errorf("end: %w", err) + } + for _, m := range mods { + if !strings.EqualFold(m.HeaderName, "DKIM-Signature") { + continue + } + for _, tag := range strings.Split(m.HeaderValue, ";") { + k, v, ok := strings.Cut(strings.TrimSpace(tag), "=") + if ok && strings.TrimSpace(k) == "d" { + return strings.ToLower(strings.TrimSpace(v)), nil + } + } + } + return "", nil +} + +// probeLines are the shared files' lines and several hundred composed from +// pieces: display names plain, quoted, escaped and encoded, comments nested +// and left open, groups, lists, brackets, source routes, domain literals and +// quoted local parts - with the other sender's name, and the server's, in +// each place - and the line padded to be cut at 256 bytes. +func probeLines(t *testing.T, own, other, server string) []string { + var lines []string + for _, file := range []string{"from-lines-passed.txt", "from-lines-other.txt"} { + lines = append(lines, fromLines(t, file, own, other)...) + } + names := []string{"", "Shop ", `"Shop, Inc." `, `"x@` + other + `" `, "x@" + other + " ", `"a\"b " `, + "=?utf-8?q?x=40" + other + "?= ", "(x@" + other + ") ", "((x@" + other + ")) ", "(open ", `"open `, "[x@" + other + "] ", + "group: ", other + ": "} + addrs := []string{ + "a@" + own, "", "", "a@" + other, "", "a@" + strings.ToUpper(own), + `<"x@` + other + `"@` + own + `>`, `"x@` + own + `"@` + other, "<@" + other + ":a@" + own + ">", "<@" + own + ":a@" + other + ">", + "a@[192.0.2.1]", ", b@" + other, "a@" + own + ", ", "a(c)@" + own, "a@" + own + "(x@" + other + ")", + "a@ " + own, "a @" + own, "", "a", `"a b"@` + own, `a\@` + other + "@" + own, + "a@" + own + " x@" + other, + } + for _, n := range names { + for _, a := range addrs { + lines = append(lines, n+a) + } + } + // Cut at 256 bytes, the quoted local part ends in "@". + pad := strings.Repeat("a", 256-len("@"+other)-1) + lines = append(lines, `"`+pad+`@`+other+`x"@`+own, `<"`+pad+`@`+other+`x"@`+own+`>`) + return lines +} diff --git a/internal/milter/testdata/mailparse_harness.c b/internal/milter/testdata/mailparse_harness.c index 1d3d466..09448c3 100644 --- a/internal/milter/testdata/mailparse_harness.c +++ b/internal/milter/testdata/mailparse_harness.c @@ -5,8 +5,9 @@ * line. Output per line: the error, then the local part and the domain, each * "-" when NULL and "=" followed by its bytes in hex otherwise. * - * cc -o mailparse_harness -I /libopendkim mailparse_harness.c - * MAILPARSE_C=./mailparse_harness go test -run TestMailParseMatchesOpenDKIM ./internal/milter + * test/upstream/mailparse-diff.sh + * + * builds it in debian:bookworm and runs TestMailParseMatchesOpenDKIM with it. */ #include #include diff --git a/test/upstream/mailparse-diff.sh b/test/upstream/mailparse-diff.sh new file mode 100644 index 0000000..b72411e --- /dev/null +++ b/test/upstream/mailparse-diff.sh @@ -0,0 +1,19 @@ +#!/bin/sh +# The From: parser port (internal/milter/mailparse.go) against OpenDKIM's own +# dkim_mail_parse(): builds internal/milter/testdata/mailparse_harness.c +# against the libopendkim source the image runs, in a debian:bookworm +# container, and runs TestMailParseMatchesOpenDKIM with it. +# +# test/upstream/mailparse-diff.sh +# +# The source dir is what `apt-get source opendkim` unpacks in debian:bookworm +# (docs/development.md); it must contain libopendkim/. +set -eu +src=$(cd "$1" && pwd) +repo=$(cd "$(dirname "$0")/../.." && pwd) +docker run --rm -v "$src":/src:ro -v "$repo/internal/milter/testdata":/h -w /h debian:bookworm sh -c ' +apt-get update -qq >/dev/null && apt-get install -y -qq gcc libc6-dev >/dev/null 2>&1 +gcc -O1 -static -o /h/mailparse_harness -I /src/libopendkim /h/mailparse_harness.c' +trap 'rm -f "$repo/internal/milter/testdata/mailparse_harness"' EXIT +cd "$repo" && MAILPARSE_C="$repo/internal/milter/testdata/mailparse_harness" \ + go test -count=1 -v -run 'TestMailParseMatchesOpenDKIM' ./internal/milter diff --git a/test/upstream/opendkim-probe.sh b/test/upstream/opendkim-probe.sh new file mode 100644 index 0000000..3950495 --- /dev/null +++ b/test/upstream/opendkim-probe.sh @@ -0,0 +1,28 @@ +#!/bin/sh +# The From: parser port against OpenDKIM's signing: starts OpenDKIM from the +# SelfPost image (default selfpost:e2e, the one make e2e builds) in a +# throwaway container with keys for three names, and runs +# TestPortAgainstOpenDKIM there against its socket. +# +# test/upstream/opendkim-probe.sh [image] +set -eu +image=${1:-selfpost:e2e} +repo=$(cd "$(dirname "$0")/../.." && pwd) +bin=$(mktemp) +trap 'rm -f "$bin"' EXIT +cd "$repo" && CGO_ENABLED=0 GOOS=linux go test -c -o "$bin" ./internal/milter +docker run --rm -v "$repo/internal/milter":/pkg:ro -v "$bin":/milter.test:ro --entrypoint sh "$image" -c ' +set -e +mkdir -p /tmp/odk && cd /tmp/odk +for d in sender.e2e.test other.e2e.test mail.e2e.test; do + mkdir -p keys/$d + opendkim-genkey -D keys/$d -d $d -s sel + echo "$d $d:sel:/tmp/odk/keys/$d/sel.private" >> KeyTable + echo "*@$d $d" >> SigningTable +done +printf "Mode s\nKeyTable /tmp/odk/KeyTable\nSigningTable refile:/tmp/odk/SigningTable\nSocket local:/tmp/odk/sock\nSyslog no\nRequireSafeKeys no\nBackground no\n" > o.conf +opendkim -V | head -1 +opendkim -f -x /tmp/odk/o.conf & +for i in 1 2 3 4 5 6 7 8 9 10; do [ -S /tmp/odk/sock ] && break; sleep 0.5; done +cd /pkg +OPENDKIM_PROBE_SOCKET=/tmp/odk/sock OPENDKIM_PROBE_TABLE=/tmp/odk/SigningTable /milter.test -test.run TestPortAgainstOpenDKIM -test.v -test.timeout 600s'