diff --git a/CHANGELOG.md b/CHANGELOG.md index 739778f..2bc039b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Added + +- **The panel signs outgoing mail with DKIM itself** (panel-dkim-signing, + stage 1 of [the plan](docs/plans/panel-dkim-signing.md)): a milter of the + panel's on two sockets, one for applications' mail - signed with the key of + the domain the login belongs to - and one for the panel's own, signed with + the server's name. `relaxed/simple`, RFC 6376 § 5.4.1's headers and `From` + once more, as OpenDKIM signs; strict. Until OpenDKIM leaves the image both + sign every message, and the e2e suite holds the two to the same `h=` and + `bh=` and has dkimpy - an independent implementation - verify each. + +### Fixed + +- **An 8-bit message could leave with a broken signature.** Postfix rewrote + it as quoted-printable for a server that does not announce 8BITMIME, after + it was signed; `disable_mime_output_conversion=yes` now delivers it as + signed. OpenDKIM's signatures were exposed to this too. + ## [2.7.0] - 2026-10-15 The image runs Debian trixie and Postfix 3.10.13, with upstream's 2026 diff --git a/build/postfix-config.sh b/build/postfix-config.sh index a63ebd5..3214dda 100644 --- a/build/postfix-config.sh +++ b/build/postfix-config.sh @@ -185,6 +185,11 @@ OPENDKIM_SOCK="${OPENDKIM_SOCKET:-/run/opendkim/opendkim.sock}" JOURNAL_SOCK="${JOURNAL_MILTER_SOCKET:-/run/selfpost/journal.sock}" # The panel's sender check (the From: line against the login, strict). SENDER_SOCK="${SENDER_CHECK_SOCKET:-/run/selfpost/sender.sock}" +# The panel's DKIM signer (strict): applications' mail, signed with the key +# of the login's domain, and the panel's own, with the server's name's +# (cmd/panel/signer.go). Matched by postfix-wrapper.sh. +SIGN_SOCK="${DKIM_SIGN_SOCKET:-/run/selfpost/sign.sock}" +SIGN_LOCAL_SOCK="${DKIM_SIGN_LOCAL_SOCKET:-/run/selfpost/sign-local.sock}" # Persistent panel-managed sender map; texthash needs no postmap, so the # unprivileged panel can rewrite it and just ask for a reload. Postfix 3.10.13, @@ -405,10 +410,16 @@ postconf -e \ # message it could not judge must not reach the signer. While the panel does # not answer on that socket, mail is deferred (docs/security.md). # +# The panel's DKIM signer follows the sender check, strict too +# (docs/plans/panel-dkim-signing.md). Until OpenDKIM leaves the image both +# sign (stage 1 of that plan): OpenDKIM after the panel, so that every +# message carries both signatures and the e2e suite compares them. +# # non_smtpd_milters is the chain for mail submitted locally with sendmail(1) - -# the panel's own notifications, sent as postmaster@ the hostname. OpenDKIM -# signs them with the key of the server's own name; the journal-milter is not -# on this chain, since they are not an application's mail. Bounces Postfix +# the panel's own notifications, sent as postmaster@ the hostname. The +# signer's own socket for it, and OpenDKIM, sign them with the key of the +# server's own name; the journal-milter is not on this chain, since they are +# not an application's mail. Bounces Postfix # generates itself do not pass it (internal_mail_filter_classes is empty). # # Postfix 3.10.13: html/MILTER_README.html:584, "Different settings for different @@ -420,8 +431,18 @@ postconf -e \ postconf -e \ "milter_protocol=6" \ "milter_default_action=tempfail" \ - "smtpd_milters={ unix:${SENDER_SOCK}, default_action=tempfail }, { unix:${OPENDKIM_SOCK}, default_action=tempfail }, { unix:${JOURNAL_SOCK}, default_action=accept }" \ - "non_smtpd_milters={ unix:${OPENDKIM_SOCK}, default_action=tempfail }" + "smtpd_milters={ unix:${SENDER_SOCK}, default_action=tempfail }, { unix:${SIGN_SOCK}, default_action=tempfail }, { unix:${OPENDKIM_SOCK}, default_action=tempfail }, { unix:${JOURNAL_SOCK}, default_action=accept }" \ + "non_smtpd_milters={ unix:${SIGN_LOCAL_SOCK}, default_action=tempfail }, { unix:${OPENDKIM_SOCK}, default_action=tempfail }" + +# A signed message is delivered as it was signed: Postfix's SMTP client would +# otherwise rewrite an 8-bit message as quoted-printable for a server that +# does not announce 8BITMIME, and every body hash over it would fail. +# Postfix 3.10.13, src/global/mail_params.h:3088, DEF_DISABLE_MIME_OCONV 0 +# (the conversion is on by default); proto/postconf.proto:7992, +# disable_mime_output_conversion: "Disable the conversion of 8BITMIME format +# to 7BIT format". Not set until 2.8.0 - OpenDKIM's signatures were exposed +# to it as well. +postconf -e "disable_mime_output_conversion=yes" # What Postfix tells a milter with MAIL FROM: its defaults plus the client's # address. The panel's milters judge every message by that address (an diff --git a/build/postfix-wrapper.sh b/build/postfix-wrapper.sh index 25094fd..3f72991 100644 --- a/build/postfix-wrapper.sh +++ b/build/postfix-wrapper.sh @@ -2,8 +2,9 @@ # Postfix start wrapper: solves the cold-start race where Postfix would try to # reach the milter sockets before they are listening. # -# It blocks until the milter sockets — OpenDKIM, the panel's sender check and -# journal-milter and, with the inbound relay on, its inbound journal milter — +# It blocks until the milter sockets — OpenDKIM, the panel's sender check, +# DKIM signer (two) and journal-milter and, with the inbound relay on, its +# inbound journal milter — # are present, then runs `postfix start-fg` and stays its parent. If they are # not ready within the timeout it exits non-zero WITHOUT starting Postfix. # supervisord counts that exit as one that came too quickly - the program's @@ -22,10 +23,12 @@ set -eu OPENDKIM_SOCK="${OPENDKIM_SOCKET:-/run/opendkim/opendkim.sock}" JOURNAL_SOCK="${JOURNAL_MILTER_SOCKET:-/run/selfpost/journal.sock}" SENDER_SOCK="${SENDER_CHECK_SOCKET:-/run/selfpost/sender.sock}" +SIGN_SOCK="${DKIM_SIGN_SOCKET:-/run/selfpost/sign.sock}" +SIGN_LOCAL_SOCK="${DKIM_SIGN_LOCAL_SOCKET:-/run/selfpost/sign-local.sock}" TIMEOUT="${MILTER_WAIT_TIMEOUT:-30}" INTERVAL=1 -SOCKS="$OPENDKIM_SOCK $JOURNAL_SOCK $SENDER_SOCK" +SOCKS="$OPENDKIM_SOCK $JOURNAL_SOCK $SENDER_SOCK $SIGN_SOCK $SIGN_LOCAL_SOCK" if [ "${INBOUND_RELAY_ENABLE:-false}" = "true" ]; then SOCKS="$SOCKS ${INBOUND_JOURNAL_MILTER_SOCKET:-/run/selfpost/inbound-journal.sock}" fi diff --git a/cmd/panel/main.go b/cmd/panel/main.go index d1febeb..ee7539b 100644 --- a/cmd/panel/main.go +++ b/cmd/panel/main.go @@ -78,9 +78,12 @@ type config struct { httpAddr string journalSocket string // The sender check's socket; the default mirrors build/postfix-config.sh. - senderSocket string - mailLog string - retentionDays int + senderSocket string + // The DKIM signer's sockets: applications' mail and the panel's own + // (cmd/panel/signer.go). + signSocket, signLocalSocket string + mailLog string + retentionDays int dataDir string dbPath string @@ -138,6 +141,10 @@ func loadConfig() config { httpAddr: envDefault("PANEL_HTTP_ADDR", ":8080"), journalSocket: envDefault("JOURNAL_MILTER_SOCKET", "/run/selfpost/journal.sock"), senderSocket: envDefault("SENDER_CHECK_SOCKET", "/run/selfpost/sender.sock"), + // The DKIM signer's two sockets; the defaults must match + // build/postfix-config.sh and build/postfix-wrapper.sh. + signSocket: envDefault("DKIM_SIGN_SOCKET", "/run/selfpost/sign.sock"), + signLocalSocket: envDefault("DKIM_SIGN_LOCAL_SOCKET", "/run/selfpost/sign-local.sock"), // Postfix's delivery log, under /data so the lines that resolve a // "queued" send-log row outlive the container. The default must match // maillog_file in build/postfix-config.sh. @@ -402,8 +409,8 @@ func run() error { } var wg sync.WaitGroup - // Room for every role to report a failure without blocking: seven at most. - errc := make(chan error, 7) + // Room for every role to report a failure without blocking: eight at most. + errc := make(chan error, 8) roles := []struct { name string @@ -412,6 +419,7 @@ func run() error { {"http", func(ctx context.Context) error { return serveHTTP(ctx, cfg, st) }}, {"journal-milter", func(ctx context.Context) error { return serveJournal(ctx, cfg, st) }}, {"sender-check", func(ctx context.Context) error { return serveSenderCheck(ctx, cfg, st) }}, + {"dkim-signer", func(ctx context.Context) error { return serveSigner(ctx, cfg) }}, {"log-tailer", func(ctx context.Context) error { return logtail.Run(ctx, cfg.mailLog, st, func() int { days, err := st.GetSendLogRetentionDays(cfg.retentionDays) diff --git a/cmd/panel/signer.go b/cmd/panel/signer.go new file mode 100644 index 0000000..8797502 --- /dev/null +++ b/cmd/panel/signer.go @@ -0,0 +1,51 @@ +package main + +import ( + "context" + "log" + "path/filepath" + "strings" + + "github.com/mixeme/selfpost/internal/milter" + "github.com/mixeme/selfpost/internal/postfix" +) + +// serveSigner runs the DKIM signer (internal/milter/sign.go) on its two +// sockets: signSocket for what applications send on 465/587, signed with the +// key of the domain the login belongs to - the sender_login_maps Postfix and +// the sender check read (Bindings.DomainOf); signLocalSocket for what the +// panel itself submits with sendmail(1), signed with the key of the server's +// name. Postfix attaches both strictly (default_action=tempfail, +// build/postfix-config.sh), so while this role is not serving, mail waits. +// +// The keys and selectors come from the KeyTable under opendkimDir, which the +// panel writes for OpenDKIM too while both sign (docs/plans/panel-dkim-signing.md, +// stage 1). +func serveSigner(ctx context.Context, cfg config) error { + keys := milter.NewSignKeys(filepath.Join(cfg.opendkimDir, "KeyTable"), filepath.Join(cfg.opendkimDir, "keys")) + bind := milter.NewBindings(postfix.New(cfg.postfixDir).SenderLoginMapsPath()) + server := strings.ToLower(cfg.hostname) + + apps, err := listenMilter(cfg.signSocket) + if err != nil { + return err + } + local, err := listenMilter(cfg.signLocalSocket) + if err != nil { + apps.Close() + return err + } + log.Printf("dkim-signer listening on %s (applications) and %s (the panel's own mail, as %s)", cfg.signSocket, cfg.signLocalSocket, server) + + errc := make(chan error, 2) + go func() { errc <- milter.ServeSign(ctx, apps, keys, bind.DomainOf) }() + go func() { + errc <- milter.ServeSign(ctx, local, keys, func(string) string { return server }) + }() + // The first to stop decides: an error stops the panel (run()), and on a + // cancelled ctx both return nil. + if err := <-errc; err != nil { + return err + } + return <-errc +} diff --git a/docs/architecture.md b/docs/architecture.md index 8b6153c..2f0b9bf 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -28,13 +28,13 @@ Managed programs ([build/supervisord.conf](../build/supervisord.conf)): |---|---|---|---| | `syslog-relay` | panel | 50 | The container's `/dev/log`: passes what OpenDKIM logs through syslog(3) to the container's log (`panel -syslog-relay`) | | `opendkim` | root → `opendkim` (`UserID` in opendkim.conf) | 100 | DKIM signing milter | -| `panel` | panel | 200 | HTTP UI + sender check + journal-milter + log-tailer + rate-limit recalc; with the inbound relay on also the inbound journal milter | -| `postfix` | root (wrapper) | 300 | MTA — started only after the milter sockets exist (OpenDKIM's, the sender check's, the journal milter's and, with the inbound relay on, the inbound journal milter's) | +| `panel` | panel | 200 | HTTP UI + sender check + DKIM signer + journal-milter + log-tailer + rate-limit recalc; with the inbound relay on also the inbound journal milter | +| `postfix` | root (wrapper) | 300 | MTA — started only after the milter sockets exist (OpenDKIM's, the sender check's, the DKIM signer's two, the journal milter's and, with the inbound relay on, the inbound journal milter's) | | `postfix-reload` | root | — | On-demand `postfix reload` (autostart off) | | `cert-reload` | root | 400 | Daily: re-copies a renewed TLS certificate for Postfix and reloads it | | `logrotate` | root | 400 | Periodic `mail.log` rotation | -Start order: syslog-relay → OpenDKIM → panel (opens the sender check's and the journal-milter's sockets) → Postfix wrapper +Start order: syslog-relay → OpenDKIM → panel (opens the sender check's, the DKIM signer's and the journal-milter's sockets) → Postfix wrapper polls unix sockets (timeout `MILTER_WAIT_TIMEOUT`, default 30s) then runs `postfix start-fg` and stays its parent: on TERM it runs `postfix stop`, so a stop of the program or of the container stops Postfix as Postfix means it to @@ -237,7 +237,7 @@ domain is added or deleted. ### Panel binary ([cmd/panel](../cmd/panel)) -One process, six roles (seven with the inbound relay on: its inbound journal +One process, seven roles (eight with the inbound relay on: its inbound journal milter, § Postfix above, port 25): 1. **HTTP server** — `:8080` (`PANEL_HTTP_ADDR`); HTTPS terminated by reverse @@ -256,7 +256,15 @@ milter, § Postfix above, port 25): (`300s` / `4000s` / `5d` / `0`) with a muted note on the box. 2. **sender check** — unix socket `SENDER_CHECK_SOCKET`, first in the chain and **strict** (`default_action=tempfail`): § Sender check below. -3. **journal-milter** — unix socket `JOURNAL_MILTER_SOCKET`; records From/To/ +3. **DKIM signer** (`2.8.0`, [plans/panel-dkim-signing.md](plans/panel-dkim-signing.md)) + — unix sockets `DKIM_SIGN_SOCKET` (applications' mail on 465/587, signed + with the key of the domain the SASL login belongs to) and + `DKIM_SIGN_LOCAL_SOCKET` (the panel's own mail, `non_smtpd_milters`, + signed with the key of the server's name); after the sender check and + **strict**. `relaxed/simple`, RFC 6376 § 5.4.1's headers and `From` once + more, go-msgauth/dkim; keys and selectors from OpenDKIM's KeyTable while + both sign (`internal/milter/sign.go`). +4. **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 **overrides** the @@ -268,22 +276,25 @@ milter, § Postfix above, port 25): 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. -4. **log-tailer** — follows `MAIL_LOG`, updates send-log delivery status by +5. **log-tailer** — follows `MAIL_LOG`, updates send-log delivery status by queue-id. Send-log `queued → sent` transitions depend on this goroutine alone (`UpdateStatus` is only called from [internal/logtail](../internal/logtail/logtail.go)). It also keeps the lines Postfix writes under a queue id, for the message's page (§ Log tailer, *Stored lines*), and counts the mail Postfix deferred because the sender check did not answer. -5. **rate-limit recalc** — every six hours (and on demand from Domain settings and +6. **rate-limit recalc** — every six hours (and on demand from Domain settings and the application form), recomputes level-2 **Auto** rate limits from send-log statistics. -6. **notifier** — writes to the panel's users when something needs them +7. **notifier** — writes to the panel's users when something needs them (§ Notifications below). -Milter chain in Postfix: the sender check (tempfail), OpenDKIM (tempfail), -then the journal (accept on failure). Mail submitted locally with -`sendmail(1)` — the notifier's — passes OpenDKIM alone (`non_smtpd_milters`): -it is signed, it is the panel's own and so needs no check of whose it is, and -it is not an application's mail to be journalled or limited. Bounces Postfix +Milter chain in Postfix: the sender check (tempfail), the panel's DKIM +signer (tempfail), OpenDKIM (tempfail), then the journal (accept on +failure); until OpenDKIM leaves the image both sign every message +(docs/plans/panel-dkim-signing.md, stage 1). Mail submitted locally with +`sendmail(1)` — the notifier's — passes the signer's own socket and OpenDKIM +(`non_smtpd_milters`): it is signed, it is the panel's own and so needs no +check of whose it is, and it is not an application's mail to be journalled +or limited. Bounces Postfix generates pass no milter. ### Sender check @@ -1129,6 +1140,9 @@ unsupported rather than as a missing doc: (`/data/setup-token`), `PANEL_HTTP_ADDR` (`:8080`), `JOURNAL_MILTER_SOCKET` (`/run/selfpost/journal.sock`), `SENDER_CHECK_SOCKET` (`/run/selfpost/sender.sock`), + `DKIM_SIGN_SOCKET` (`/run/selfpost/sign.sock`) and `DKIM_SIGN_LOCAL_SOCKET` + (`/run/selfpost/sign-local.sock` — the DKIM signer's, read by the panel, + `build/postfix-config.sh` and `build/postfix-wrapper.sh`, which must agree), `INBOUND_JOURNAL_MILTER_SOCKET` (`/run/selfpost/inbound-journal.sock` — the inbound journal milter's socket, read by the panel, `build/postfix-config.sh` and `build/postfix-wrapper.sh`, which must agree), `MAIL_LOG` diff --git a/go.mod b/go.mod index d06972e..0d5ac99 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( require ( github.com/dustin/go-humanize v1.0.1 // indirect github.com/emersion/go-message v0.18.1 // indirect + github.com/emersion/go-msgauth v0.7.0 // indirect github.com/google/uuid v1.6.0 // indirect github.com/mattn/go-isatty v0.0.24 // indirect github.com/ncruces/go-strftime v1.0.0 // indirect diff --git a/go.sum b/go.sum index 2f9a67c..288264e 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,8 @@ github.com/emersion/go-message v0.18.1 h1:tfTxIoXFSFRwWaZsgnqS1DSZuGpYGzSmCZD8SK github.com/emersion/go-message v0.18.1/go.mod h1:XpJyL70LwRvq2a8rVbHXikPgKj8+aI0kGdHlg16ibYA= github.com/emersion/go-milter v0.4.1 h1:gLs9QD0zEHF8omgEw8M+aGz6iwBNpWLAcwgSur0ra4M= github.com/emersion/go-milter v0.4.1/go.mod h1:erCQVl0mH4SX9jEvwe+wyndit0rQtmvMLH86V6NGtkI= +github.com/emersion/go-msgauth v0.7.0 h1:vj2hMn6KhFtW41kshIBTXvp6KgYSqpA/ZN9Pv4g1INc= +github.com/emersion/go-msgauth v0.7.0/go.mod h1:mmS9I6HkSovrNgq0HNXTeu8l3sRAAuQ9RMvbM4KU7Ck= github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3 h1:LMLX+LgTNWpfvCBdFebv6EsYotImrt/Ppc5cXIriCSo= github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3/go.mod h1:jl5iWTm0/hd5PjEYEOuwAJ57L/CibdZfrqZ5XA5GrCk= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= diff --git a/internal/milter/sign.go b/internal/milter/sign.go new file mode 100644 index 0000000..1c4f65d --- /dev/null +++ b/internal/milter/sign.go @@ -0,0 +1,388 @@ +package milter + +// The DKIM signer (docs/plans/panel-dkim-signing.md): the panel's own milter +// that signs what applications send on 465/587 with the key of the domain +// their login belongs to, and what the panel itself sends with the key of +// the server's name. Each of the two is a socket of its own, so the socket a +// message comes through - not the absence of a login - decides that the +// server's key may sign it (Q1 of the plan). + +import ( + "context" + "crypto" + "crypto/x509" + "encoding/pem" + "errors" + "fmt" + "log" + "net" + "net/textproto" + "os" + "path/filepath" + "strings" + "sync" + "time" + + "github.com/emersion/go-milter" + "github.com/emersion/go-msgauth/dkim" +) + +// signHeaders is the set of header names signed when present: OpenDKIM's +// dkim_should_signhdrs[], RFC 6376 § 5.4.1's list (OpenDKIM 2.11.0~beta2, +// libopendkim/dkim.c:223-246), which OpenDKIM signed with until now. Every +// occurrence is signed, in message order, and From once more (signLines). +var signHeaders = map[string]bool{ + "from": true, "reply-to": true, "subject": true, "date": true, "to": true, "cc": true, + "resent-date": true, "resent-from": true, "resent-sender": true, "resent-to": true, "resent-cc": true, + "in-reply-to": true, "references": true, + "list-id": true, "list-help": true, "list-unsubscribe": true, "list-subscribe": true, + "list-post": true, "list-owner": true, "list-archive": true, +} + +// signLimit bounds how long a message may take from the end of its headers +// to its end. go-milter v0.4.1 tells a session nothing when its connection +// ends early (session.go, HandleMilterCommands(): it returns on EOF or QUIT +// without calling Abort), and go-msgauth's signer reads the message in a +// goroutine of its own (dkim/sign.go, NewSigner(): an io.Pipe), which would +// wait for the rest for ever. At the limit the signer is closed and a message +// that does go on is deferred, never signed over part of its body. An hour is +// far above Postfix's own bounds on a transaction. +const signLimit = time.Hour + +// SignKeys reads the domains the server signs for, their selectors and key +// files from the KeyTable the panel writes (internal/domain, renderTables(): +// " :://.private"), the +// table OpenDKIM reads too until it leaves the image. A key file is read +// when it changes, and only from under keysDir. +type SignKeys struct { + table, keysDir string + + mu sync.Mutex + stamp fileStamp + rows map[string]keyRow + keys map[string]loadedKey +} + +type keyRow struct{ selector, path string } + +type loadedKey struct { + stamp fileStamp + signer crypto.Signer +} + +// fileStamp tells a file's versions apart: the panel replaces a table or a +// key by writing a new file and renaming it over the old one +// (internal/atomicfile), so a new version is a new modification time or +// size. +type fileStamp struct { + mod time.Time + size int64 +} + +func stampOf(fi os.FileInfo) fileStamp { return fileStamp{fi.ModTime(), fi.Size()} } + +// errNoKey is a domain the KeyTable has no line for. +var errNoKey = errors.New("no signing key") + +// NewSignKeys reads keyTable; keysDir is the directory every key in it must +// be under. +func NewSignKeys(keyTable, keysDir string) *SignKeys { + return &SignKeys{table: keyTable, keysDir: keysDir, keys: map[string]loadedKey{}} +} + +// Lookup gives the selector and key of domain. +func (k *SignKeys) Lookup(domain string) (string, crypto.Signer, error) { + domain = strings.ToLower(domain) + k.mu.Lock() + defer k.mu.Unlock() + if err := k.readTable(); err != nil { + return "", nil, err + } + row, ok := k.rows[domain] + if !ok { + return "", nil, fmt.Errorf("%w for %s", errNoKey, domain) + } + fi, err := os.Stat(row.path) + if err != nil { + return "", nil, fmt.Errorf("key of %s: %w", domain, err) + } + if have, ok := k.keys[row.path]; ok && have.stamp == stampOf(fi) { + return row.selector, have.signer, nil + } + signer, err := readKey(row.path) + if err != nil { + return "", nil, fmt.Errorf("key of %s: %w", domain, err) + } + k.keys[row.path] = loadedKey{stampOf(fi), signer} + return row.selector, signer, nil +} + +func (k *SignKeys) readTable() error { + fi, err := os.Stat(k.table) + if err != nil { + return fmt.Errorf("key table: %w", err) + } + if k.rows != nil && k.stamp == stampOf(fi) { + return nil + } + data, err := os.ReadFile(k.table) + if err != nil { + return fmt.Errorf("key table: %w", err) + } + rows := map[string]keyRow{} + for n, line := range strings.Split(string(data), "\n") { + if strings.TrimSpace(line) == "" { + continue + } + name, value, ok := strings.Cut(line, " ") + // Domain and selector never hold a colon (internal/domain, + // opendkimForbidden); the path takes the rest. + parts := strings.SplitN(value, ":", 3) + if !ok || len(parts) != 3 || !strings.EqualFold(parts[0], name) { + return fmt.Errorf("key table: line %d is not \" ::\"", n+1) + } + domain, selector, path := strings.ToLower(name), parts[1], parts[2] + // The panel writes exactly this path (renderTables); anything else + // is not read. + if want := filepath.Join(k.keysDir, domain, selector+".private"); filepath.Clean(path) != want { + return fmt.Errorf("key table: line %d names %s, not %s", n+1, path, want) + } + rows[domain] = keyRow{selector, path} + } + k.rows, k.stamp = rows, stampOf(fi) + return nil +} + +// readKey reads a PEM private key: PKCS#1 as internal/domain writes it +// (dkim.go, writePrivateKeyPEM()), PKCS#8 as well. +func readKey(path string) (crypto.Signer, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + block, _ := pem.Decode(data) + if block == nil { + return nil, errors.New("no PEM block") + } + if key, err := x509.ParsePKCS1PrivateKey(block.Bytes); err == nil { + return key, nil + } + key, err := x509.ParsePKCS8PrivateKey(block.Bytes) + if err != nil { + return nil, fmt.Errorf("not an RSA or PKCS#8 key: %w", err) + } + signer, ok := key.(crypto.Signer) + if !ok { + return nil, fmt.Errorf("key type %T cannot sign", key) + } + return signer, nil +} + +// signSession is one connection of a signer socket. +type signSession struct { + milter.NoOpMilter + keys *SignKeys + // domainOf gives the domain whose key signs the message: the login's on + // the applications' socket, the server's name on the panel's own. + domainOf func(login string) string + + login string + headers []field + signer *dkim.Signer + domain string + expired chan struct{} // closed by the watchdog at signLimit + done chan struct{} // closed when the message ends, to stop it +} + +type field struct{ name, value string } + +func (s *signSession) reset() { + if s.signer != nil { + close(s.done) + _ = s.signer.Close() + } + s.login, s.headers, s.signer, s.domain = "", nil, nil, "" +} + +// MailFrom starts a message. {auth_authen} is the SASL login (see +// session.MailFrom in milter.go for the macro list Postfix sends). +func (s *signSession) MailFrom(from string, m *milter.Modifier) (milter.Response, error) { + s.reset() + s.login = macro(m, "auth_authen") + return milter.RespContinue, nil +} + +// Header keeps every header as Postfix hands it over. Postfix 3.10.13, +// src/milter/milter8.c:2329-2400, milter8_header(): the value with its leading +// white space, for a milter that asks for it (OptHeaderLeadingSpace, +// ServeSign); a header of several lines as text separated by "\n"; and not +// Postfix's own Received:, which goes above the signature anyway (an +// inserted header is placed relative to the message as delivered). +func (s *signSession) Header(name, value string, m *milter.Modifier) (milter.Response, error) { + s.headers = append(s.headers, field{name, value}) + return milter.RespContinue, nil +} + +// Headers chooses the key and starts the signer with the header block. +// What cannot be signed is deferred: the socket is attached with +// default_action=tempfail, and so is every refusal here. +// +// h is not read: go-milter v0.4.1 keeps every header of the connection in it +// (see senderSession.Headers). +func (s *signSession) Headers(h textproto.MIMEHeader, m *milter.Modifier) (milter.Response, error) { + s.domain = s.domainOf(s.login) + if s.domain == "" { + log.Printf("dkim-signer: no sending domain for login %q - deferring", s.login) + return milter.RespTempFail, nil + } + selector, key, err := s.keys.Lookup(s.domain) + if err != nil { + log.Printf("dkim-signer: %v - deferring", err) + return milter.RespTempFail, nil + } + keys, block := signLines(s.headers) + signer, err := dkim.NewSigner(&dkim.SignOptions{ + Domain: s.domain, + Selector: selector, + Signer: key, + HeaderCanonicalization: dkim.CanonicalizationRelaxed, + BodyCanonicalization: dkim.CanonicalizationSimple, + HeaderKeys: keys, + }) + if err != nil { + log.Printf("dkim-signer: %s: %v - deferring", s.domain, err) + return milter.RespTempFail, nil + } + s.signer, s.expired, s.done = signer, make(chan struct{}), make(chan struct{}) + go watch(signer, s.expired, s.done) + if _, err := signer.Write(block); err != nil { + log.Printf("dkim-signer: %s: %v - deferring", s.domain, err) + s.reset() + return milter.RespTempFail, nil + } + return milter.RespContinue, nil +} + +// watch closes signer at signLimit unless done is closed first. +func watch(signer *dkim.Signer, expired, done chan struct{}) { + t := time.NewTimer(signLimit) + defer t.Stop() + select { + case <-done: + case <-t.C: + close(expired) + _ = signer.Close() + } +} + +// signLines is the header block the signer reads and the names it signs. +// Each header is written as Postfix will deliver it: the name, the colon, the +// value with its own leading white space, the lines of a folded value joined +// with CRLF; and the empty line that ends the block. The names: every header +// in signHeaders, in message order, and From once more, so that a From: +// added later breaks the signature (RFC 6376 § 5.4.2; OversignHeaders From in +// OpenDKIM's configuration until now; go-msgauth v0.7.0, dkim/sign.go, +// NewSigner(): a name with no instance left is kept in h= and hashes +// nothing). +func signLines(headers []field) ([]string, []byte) { + var keys []string + var b strings.Builder + for _, f := range headers { + b.WriteString(f.name) + b.WriteString(":") + b.WriteString(strings.ReplaceAll(strings.ReplaceAll(f.value, "\r\n", "\n"), "\n", "\r\n")) + b.WriteString("\r\n") + if signHeaders[strings.ToLower(strings.TrimSpace(f.name))] { + keys = append(keys, strings.TrimSpace(f.name)) + } + } + b.WriteString("\r\n") + return append(keys, "From"), []byte(b.String()) +} + +// BodyChunk hashes the body as it comes. Postfix 3.10.13, +// src/milter/milter8.c:2470-2500, milter8_body(): every line ends "\r\n". +func (s *signSession) BodyChunk(chunk []byte, m *milter.Modifier) (milter.Response, error) { + if s.signer == nil { + return milter.RespContinue, nil + } + if _, err := s.signer.Write(chunk); err != nil { + log.Printf("dkim-signer: %s: %v - deferring", s.domain, err) + s.reset() + return milter.RespTempFail, nil + } + return milter.RespContinue, nil +} + +// Body is the end of the message: the signature goes in at the top, where +// OpenDKIM put it (opendkim/opendkim.c:3673, dkimf_insheader(ctx, 0, ...)). +// Postfix 3.10.13, src/milter/milter8.c:1445-1465, SMFIR_INSHEADER: the +// index is passed on plus one; src/cleanup/cleanup_milter.c:1193, +// cleanup_ins_header(): "Index 1 is the top-most header" - above Postfix's +// own Received:, which a milter never sees (milter8.c:2352-2358, "insert +// requests are relative to the message as delivered"). go-milter v0.4.1, +// modifier.go, InsertHeader() turns the folded value's CRLF into LF, as +// Postfix keeps a header. +func (s *signSession) Body(m *milter.Modifier) (milter.Response, error) { + signer := s.signer + if signer == nil { + // Headers did not start one, and answered tempfail already. + return milter.RespTempFail, nil + } + close(s.done) + s.signer = nil + err := signer.Close() + select { + case <-s.expired: + err = fmt.Errorf("the message took longer than %s", signLimit) + default: + } + if err != nil { + log.Printf("dkim-signer: %s: %v - deferring", s.domain, err) + return milter.RespTempFail, nil + } + name, value, ok := strings.Cut(signer.Signature(), ": ") + if !ok || !strings.EqualFold(name, "DKIM-Signature") { + log.Printf("dkim-signer: %s: malformed signature from the library - deferring", s.domain) + return milter.RespTempFail, nil + } + if err := m.InsertHeader(0, name, strings.TrimSuffix(value, "\r\n")); err != nil { + return nil, err + } + return milter.RespContinue, nil +} + +func (s *signSession) Abort(m *milter.Modifier) error { + s.reset() + return nil +} + +// ServeSign runs a signer on ln until ctx is cancelled; domainOf chooses the +// key of each message (signSession.domainOf). +// +// The protocol: headers with their leading space (as for the sender check, +// ServeSender) and the body. The milter says it adds headers (go-milter +// v0.4.1, milter.go: OptAddHeader is SMFIF_ADDHDRS), as the protocol asks of +// one that does; Postfix 3.10.13 does not hold an insert request against it +// (src/milter/milter8.c:1445-1465 reads SMFIR_INSHEADER without looking at +// the flag). +func ServeSign(ctx context.Context, ln net.Listener, keys *SignKeys, domainOf func(login string) string) error { + srv := &milter.Server{ + NewMilter: func() milter.Milter { + return &signSession{keys: keys, domainOf: domainOf} + }, + Actions: milter.OptAddHeader, + Protocol: milter.OptHeaderLeadingSpace, + } + go func() { + <-ctx.Done() + _ = srv.Close() + }() + if err := srv.Serve(ln); err != nil { + if ctx.Err() != nil { + return nil + } + return err + } + return nil +} diff --git a/internal/milter/sign_test.go b/internal/milter/sign_test.go new file mode 100644 index 0000000..a81c920 --- /dev/null +++ b/internal/milter/sign_test.go @@ -0,0 +1,302 @@ +package milter + +import ( + "bytes" + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/base64" + "encoding/pem" + "net" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/emersion/go-milter" + "github.com/emersion/go-msgauth/dkim" +) + +// signFixture is a key directory and KeyTable laid out as internal/domain +// writes them (renderTables(), writePrivateKeyPEM(): PKCS#1 PEM) for +// example.org with selector sel, and the DNS record of the key. +type signFixture struct { + dir, table, keysDir string + record string +} + +func newSignFixture(t *testing.T) signFixture { + t.Helper() + dir := t.TempDir() + keysDir := filepath.Join(dir, "keys") + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + keyPath := filepath.Join(keysDir, "example.org", "sel.private") + if err := os.MkdirAll(filepath.Dir(keyPath), 0o750); err != nil { + t.Fatal(err) + } + pemBytes := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) + if err := os.WriteFile(keyPath, pemBytes, 0o640); err != nil { + t.Fatal(err) + } + table := filepath.Join(dir, "KeyTable") + if err := os.WriteFile(table, []byte("example.org example.org:sel:"+keyPath+"\n"), 0o640); err != nil { + t.Fatal(err) + } + pub, err := x509.MarshalPKIXPublicKey(&key.PublicKey) + if err != nil { + t.Fatal(err) + } + return signFixture{dir, table, keysDir, "v=DKIM1; k=rsa; p=" + base64.StdEncoding.EncodeToString(pub)} +} + +// startSigner serves a signer whose logins map by domains and returns a +// session of go-milter's client on it. +func startSigner(t *testing.T, f signFixture, domainOf func(string) string) *milter.ClientSession { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + go ServeSign(ctx, ln, NewSignKeys(f.table, f.keysDir), domainOf) + // The library's server speaks milter version 2; the client asks for no + // action only version 6 has (see TestRecipientsThroughGoMilter). + cl := milter.NewClientWithOptions("tcp", ln.Addr().String(), milter.ClientOptions{ + ActionMask: milter.OptAddHeader | milter.OptChangeHeader, + }) + t.Cleanup(func() { cl.Close() }) + s, err := cl.Session() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { s.Close() }) + return s +} + +func appDomains(login string) string { + if login == "app1" { + return "example.org" + } + return "" +} + +// The headers of a test message as Postfix hands them to a milter that asks +// for the leading space: the value after the colon, a folded one with "\n" +// between its lines (Postfix 3.10.13, src/milter/milter8.c:2329-2400, +// milter8_header()). Composed from that source, not seen on a stand. +var testHeaders = []field{ + {"From", " Alice "}, + {"To", " bob@example.net"}, + {"Subject", " a subject that is\n\tfolded over two lines"}, + {"Date", " Thu, 15 Oct 2026 20:00:00 +0000"}, + {"Message-ID", " <1@example.org>"}, + {"X-Mailer", "no space after the colon"}, +} + +const testBody = "Hello,\r\n\r\nthe body, with a blank line in it.\r\n" + +// sendMessage runs one message through s and returns the modifications +// asked for at its end and the final action. +func sendMessage(t *testing.T, s *milter.ClientSession, login string, headers []field, body string) ([]milter.ModifyAction, *milter.Action) { + t.Helper() + if err := s.Macros(milter.CodeMail, "{auth_authen}", login); err != nil { + t.Fatal(err) + } + if act, err := s.Mail("", nil); err != nil || act.Code != milter.ActContinue { + t.Fatalf("mail: %+v %v", act, err) + } + if act, err := s.Rcpt("", nil); err != nil || act.Code != milter.ActContinue { + t.Fatalf("rcpt: %+v %v", act, err) + } + for _, h := range headers { + if act, err := s.HeaderField(h.name, h.value); err != nil || act.Code != milter.ActContinue { + t.Fatalf("header %s: %+v %v", h.name, act, err) + } + } + act, err := s.HeaderEnd() + if err != nil { + t.Fatal(err) + } + if act.Code != milter.ActContinue { + return nil, act + } + if act, err := s.BodyChunk([]byte(body)); err != nil || act.Code != milter.ActContinue { + t.Fatalf("body: %+v %v", act, err) + } + mods, act, err := s.End() + if err != nil { + t.Fatal(err) + } + return mods, act +} + +// delivered is the message as Postfix delivers it with the signature +// inserted at the top: every header line ending CRLF. +func delivered(sig milter.ModifyAction, headers []field, body string) []byte { + var b bytes.Buffer + crlf := func(v string) string { return strings.ReplaceAll(v, "\n", "\r\n") } + b.WriteString(sig.HeaderName + ": " + crlf(sig.HeaderValue) + "\r\n") + for _, h := range headers { + b.WriteString(h.name + ":" + crlf(h.value) + "\r\n") + } + b.WriteString("\r\n" + body) + return b.Bytes() +} + +func verifyWith(t *testing.T, record string, msg []byte) *dkim.Verification { + t.Helper() + vs, err := dkim.VerifyWithOptions(bytes.NewReader(msg), &dkim.VerifyOptions{ + LookupTXT: func(string) ([]string, error) { return []string{record}, nil }, + }) + if err != nil { + t.Fatal(err) + } + if len(vs) != 1 { + t.Fatalf("%d signatures, want 1", len(vs)) + } + return vs[0] +} + +// signed sends one message and returns the signature the signer inserted. +func signed(t *testing.T, s *milter.ClientSession, login string, headers []field, body string) milter.ModifyAction { + t.Helper() + mods, act := sendMessage(t, s, login, headers, body) + if act.Code != milter.ActContinue { + t.Fatalf("end of message: %+v, want continue", act) + } + if len(mods) != 1 || mods[0].Code != milter.ActInsertHeader || mods[0].HeaderIndex != 0 || mods[0].HeaderName != "DKIM-Signature" { + t.Fatalf("modifications %+v, want one DKIM-Signature inserted at 0", mods) + } + return mods[0] +} + +func TestSignerSignsWithTheLoginsDomain(t *testing.T) { + f := newSignFixture(t) + s := startSigner(t, f, appDomains) + sig := signed(t, s, "app1", testHeaders, testBody) + + v := verifyWith(t, f.record, delivered(sig, testHeaders, testBody)) + if v.Err != nil { + t.Fatalf("signature does not verify: %v\n%s", v.Err, delivered(sig, testHeaders, testBody)) + } + if v.Domain != "example.org" { + t.Errorf("d=%s, want example.org", v.Domain) + } + for _, want := range []string{"c=relaxed/simple", "s=sel", "h=From:To:Subject:Date:From"} { + if !strings.Contains(strings.ReplaceAll(sig.HeaderValue, "\n", ""), want) { + t.Errorf("signature lacks %q: %s", want, sig.HeaderValue) + } + } + // A second From: added on the way breaks it: From is over-signed. + tampered := append([]field{{"From", " Mallory "}}, testHeaders...) + if v := verifyWith(t, f.record, delivered(sig, tampered, testBody)); v.Err == nil { + t.Error("a From: added above still verifies") + } + // A changed body breaks it. + if v := verifyWith(t, f.record, delivered(sig, testHeaders, testBody+"more\r\n")); v.Err == nil { + t.Error("a changed body still verifies") + } +} + +// Two messages on one connection are signed each on their own. +func TestSignerSignsEachMessageOfAConnection(t *testing.T) { + f := newSignFixture(t) + s := startSigner(t, f, appDomains) + for i, body := range []string{testBody, "Another body.\r\n"} { + sig := signed(t, s, "app1", testHeaders, body) + if v := verifyWith(t, f.record, delivered(sig, testHeaders, body)); v.Err != nil { + t.Fatalf("message %d: %v", i+1, v.Err) + } + } +} + +// An empty body and a body without a final line end are signed as Postfix +// delivers them. +func TestSignerSignsOddBodies(t *testing.T) { + f := newSignFixture(t) + for _, body := range []string{"", "no final line end"} { + s := startSigner(t, f, appDomains) + sig := signed(t, s, "app1", testHeaders, body) + if v := verifyWith(t, f.record, delivered(sig, testHeaders, body)); v.Err != nil { + t.Fatalf("body %q: %v", body, v.Err) + } + } +} + +// The panel's own socket signs with the server's name whatever the login. +func TestSignerOnTheLocalSocketUsesItsDomain(t *testing.T) { + f := newSignFixture(t) + s := startSigner(t, f, func(string) string { return "example.org" }) + sig := signed(t, s, "", testHeaders, testBody) + if v := verifyWith(t, f.record, delivered(sig, testHeaders, testBody)); v.Err != nil || v.Domain != "example.org" { + t.Fatalf("d=%s err=%v", v.Domain, v.Err) + } +} + +// What cannot be signed is deferred, never passed on unsigned. +func TestSignerDefersWhatItCannotSign(t *testing.T) { + cases := []struct { + name string + login string + setup func(t *testing.T, f signFixture) + }{ + {"a login with no domain", "stranger", nil}, + {"no line for the domain", "app1", func(t *testing.T, f signFixture) { + if err := os.WriteFile(f.table, []byte("other.example other.example:sel:"+filepath.Join(f.keysDir, "other.example", "sel.private")+"\n"), 0o640); err != nil { + t.Fatal(err) + } + }}, + {"a key file that is gone", "app1", func(t *testing.T, f signFixture) { + if err := os.Remove(filepath.Join(f.keysDir, "example.org", "sel.private")); err != nil { + t.Fatal(err) + } + }}, + {"a key path outside the key directory", "app1", func(t *testing.T, f signFixture) { + outside := filepath.Join(f.dir, "sel.private") + if err := os.Rename(filepath.Join(f.keysDir, "example.org", "sel.private"), outside); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(f.table, []byte("example.org example.org:sel:"+outside+"\n"), 0o640); err != nil { + t.Fatal(err) + } + }}, + {"a key table that is not one", "app1", func(t *testing.T, f signFixture) { + if err := os.WriteFile(f.table, []byte("example.org\n"), 0o640); err != nil { + t.Fatal(err) + } + }}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + f := newSignFixture(t) + if c.setup != nil { + c.setup(t, f) + } + s := startSigner(t, f, appDomains) + mods, act := sendMessage(t, s, c.login, testHeaders, testBody) + if act.Code != milter.ActTempFail || len(mods) != 0 { + t.Fatalf("got %+v with %d modifications, want tempfail and none", act, len(mods)) + } + }) + } +} + +// signLines writes the block Postfix will deliver and names what is signed. +func TestSignLines(t *testing.T) { + keys, block := signLines(testHeaders) + if got := strings.Join(keys, ":"); got != "From:To:Subject:Date:From" { + t.Errorf("keys %s", got) + } + want := "From: Alice \r\nTo: bob@example.net\r\n" + + "Subject: a subject that is\r\n\tfolded over two lines\r\n" + + "Date: Thu, 15 Oct 2026 20:00:00 +0000\r\nMessage-ID: <1@example.org>\r\n" + + "X-Mailer:no space after the colon\r\n\r\n" + if string(block) != want { + t.Errorf("block\n%q\nwant\n%q", block, want) + } +} diff --git a/test/e2e/compose.override.yml b/test/e2e/compose.override.yml index 6502118..a8e13d4 100644 --- a/test/e2e/compose.override.yml +++ b/test/e2e/compose.override.yml @@ -109,6 +109,9 @@ services: context: ../sink volumes: - ./mail-stage:/mail + # dkimverify looks the DKIM keys up in the fake zone. + dns: + - 10.77.0.10 networks: e2e: ipv4_address: 10.77.0.11 diff --git a/test/e2e/mail_helpers.go b/test/e2e/mail_helpers.go index 443a74e..b5a5d1d 100644 --- a/test/e2e/mail_helpers.go +++ b/test/e2e/mail_helpers.go @@ -28,7 +28,15 @@ const ( // sleeping a fixed duration is what keeps this deterministic even under a // slow CI runner (plan C.4). func findSinkMessage(stageDir, token string, timeout time.Duration) ([]byte, error) { + _, raw, err := findSinkFile(stageDir, token, timeout) + return raw, err +} + +// findSinkFile is findSinkMessage that also says the file's name in the +// sink's /mail (the same directory, mounted). +func findSinkFile(stageDir, token string, timeout time.Duration) (string, []byte, error) { dir := filepath.Join(stageDir, "mail-stage") + var name string var found []byte err := waitFor(fmt.Sprintf("sink-MX to receive a message tagged %q", token), timeout, 300*time.Millisecond, func() (bool, error) { entries, err := os.ReadDir(dir) @@ -44,19 +52,20 @@ func findSinkMessage(stageDir, token string, timeout time.Duration) ([]byte, err continue } if bytes.Contains(b, []byte(token)) { - found = b + name, found = e.Name(), b return true, nil } } return false, fmt.Errorf("no dumped message contains %q yet (%d on disk)", token, len(entries)) }) - return found, err + return name, found, err } // verifyDKIM checks that raw (a message dumped by the sink) carries a DKIM -// signature for domain that validates against the public key published in the -// fake zone — i.e. exactly what a real receiver would check, using DNS the -// harness itself controls instead of net.DefaultResolver (plan C.4). +// signature for domain and that every signature for it validates against the +// public key published in the fake zone — i.e. exactly what a real receiver +// would check, using DNS the harness itself controls instead of +// net.DefaultResolver (plan C.4). func verifyDKIM(raw []byte, domain string) error { verifications, err := dkim.VerifyWithOptions(bytes.NewReader(raw), &dkim.VerifyOptions{ LookupTXT: lookupTXTFunc(), @@ -64,15 +73,73 @@ func verifyDKIM(raw []byte, domain string) error { if err != nil { return fmt.Errorf("dkim verify: %w", err) } + n := 0 for _, v := range verifications { if v.Domain == domain { if v.Err != nil { return fmt.Errorf("dkim signature for %s did not validate: %w", domain, v.Err) } - return nil + n++ } } - return fmt.Errorf("no DKIM signature found for domain %s (got %d signature(s))", domain, len(verifications)) + if n == 0 { + return fmt.Errorf("no DKIM signature found for domain %s (got %d signature(s))", domain, len(verifications)) + } + return nil +} + +// checkBothSigners is the oracle of docs/plans/panel-dkim-signing.md, stage +// 1: the panel's signer and OpenDKIM both sign (OpenDKIM after the panel in +// the chain, so its signature is the top one - build/postfix-config.sh); +// the two must agree on what they sign - the same h= and bh= - and each +// must verify with dkimpy (python3-dkim in the sink image), an +// implementation that is neither go-msgauth, the panel's library, nor +// OpenDKIM. name is the file in the sink's /mail. +func checkBothSigners(s *stack, name string, raw []byte, domain string) error { + msg, err := mail.ReadMessage(bytes.NewReader(raw)) + if err != nil { + return fmt.Errorf("parse the delivered message: %w", err) + } + var sigs []map[string]string + for _, v := range msg.Header["Dkim-Signature"] { + tags := dkimTags(v) + if tags["d"] == domain { + sigs = append(sigs, tags) + } + } + if len(sigs) != 2 { + return fmt.Errorf("%d DKIM signatures for %s, want 2 (the panel's and OpenDKIM's)", len(sigs), domain) + } + for _, tag := range []string{"bh", "h", "c", "s", "a"} { + a, b := sigs[0][tag], sigs[1][tag] + if tag == "h" { + a, b = strings.ToLower(a), strings.ToLower(b) + } + if a != b { + return fmt.Errorf("the signatures differ in %s=: OpenDKIM %q, the panel %q", tag, a, b) + } + } + for i := range sigs { + out, err := s.execIn("sink", "sh", "-c", fmt.Sprintf("dkimverify --index %d < '/mail/%s'", i, name)) + if err != nil || !strings.Contains(out, "signature ok") { + return fmt.Errorf("dkimpy on signature %d (%s): %v\n%s", i, []string{"OpenDKIM's", "the panel's"}[i], err, out) + } + } + return nil +} + +// dkimTags splits a DKIM-Signature value into its tags, white space removed +// (RFC 6376 § 3.2: tag=value pairs separated by ";", FWS anywhere). +func dkimTags(v string) map[string]string { + tags := map[string]string{} + for _, part := range strings.Split(v, ";") { + k, val, ok := strings.Cut(part, "=") + if !ok { + continue + } + tags[strings.TrimSpace(k)] = strings.Join(strings.Fields(val), "") + } + return tags } // postfixReceived returns the Received: headers of raw that a Postfix smtpd diff --git a/test/e2e/main_test.go b/test/e2e/main_test.go index d16f21c..8dfe64a 100644 --- a/test/e2e/main_test.go +++ b/test/e2e/main_test.go @@ -267,13 +267,16 @@ func TestE2E(t *testing.T) { res.dialErr, res.authErr, res.mailErr, res.rcptErr, res.dataErr) } - raw, err := findSinkMessage(h.stageDir, token, 30*time.Second) + file, raw, err := findSinkFile(h.stageDir, token, 30*time.Second) if err != nil { t.Fatalf("message never reached the sink: %v", err) } if err := verifyDKIM(raw, senderDomain); err != nil { t.Fatalf("DKIM did not verify against the record the panel published: %v", err) } + if err := checkBothSigners(h, file, raw, senderDomain); err != nil { + t.Fatalf("the panel's signer: %v\n%s", err, raw) + } if err := checkReceivedHidden(raw); err != nil { t.Fatalf("port 465 should hide the application's session: %v", err) } diff --git a/test/e2e/notifications_test.go b/test/e2e/notifications_test.go index b6f9765..aed366d 100644 --- a/test/e2e/notifications_test.go +++ b/test/e2e/notifications_test.go @@ -93,7 +93,7 @@ func testNotificationsSetUp(t *testing.T, sc *scenario) { // (internal/notify): here it would be a minute of waiting for nothing to happen. func testRateLimitRefusalIsNotified(t *testing.T, sc *scenario) { subject := "[SelfPost] " + senderDomain + ": a rate limit is refusing mail" - raw, err := findSinkMessage(h.stageDir, subject, 100*time.Second) + file, raw, err := findSinkFile(h.stageDir, subject, 100*time.Second) if err != nil { t.Fatalf("no notification reached %s: %v\npanel log:\n%s", adminEmail, err, tailLines(h.logs("selfpost"), "notify", 20)) } @@ -108,6 +108,9 @@ func testRateLimitRefusalIsNotified(t *testing.T, sc *scenario) { t.Errorf("the notification lacks %q:\n%s", want, msg) } } + if err := checkBothSigners(h, file, raw, serverName); err != nil { + t.Errorf("the panel's signer on its own mail: %v", err) + } if err := verifyDKIM(raw, serverName); err != nil { t.Errorf("the notification is not signed as %s with the key Settings shows: %v", serverName, err) } diff --git a/test/e2e/sink/Dockerfile b/test/e2e/sink/Dockerfile index eef8711..7bef09d 100644 --- a/test/e2e/sink/Dockerfile +++ b/test/e2e/sink/Dockerfile @@ -2,14 +2,17 @@ # each transaction to its own file under /mail, so the test harness can read # the raw received message (headers + body, including the DKIM-Signature # Postfix/OpenDKIM added) straight off disk. smtp-sink ships in the postfix -# package itself — no new dependency. +# package itself. python3-dkim (dkimpy 1.1.8-2 in trixie, /usr/bin/dkimverify) +# is the independent DKIM verifier the suite holds the panel's signer to +# (docs/plans/panel-dkim-signing.md); it looks the keys up through the +# stand's CoreDNS (compose.override.yml, sink's dns). FROM debian:trixie-slim ENV DEBIAN_FRONTEND=noninteractive RUN echo "postfix postfix/mailname string localhost" | debconf-set-selections \ && echo "postfix postfix/main_mailer_type string Internet Site" | debconf-set-selections \ && apt-get update \ - && apt-get install -y --no-install-recommends postfix \ + && apt-get install -y --no-install-recommends postfix python3-dkim \ && rm -rf /var/lib/apt/lists/* RUN mkdir -p /mail