diff --git a/CHANGELOG.md b/CHANGELOG.md index 790622c..ce3aab0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,6 +118,25 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version carry a damaged database whose restore would refuse every application. The change is now made to a copy that replaces the file whole (assumptions C-S3, C-Q3). +- **Server › Health connects to the milter sockets instead of finding their + files.** A socket's file outlives a killed OpenDKIM or panel and refuses + every connection; Health showed it as listening while Postfix answered + every sender with a temporary error. The panel now connects as Postfix + does and says when nothing listens (assumptions A-38). +- **Server › Health shows the certificate Postfix serves.** Postfix serves + its own copy of the mounted certificate and refreshes it once a day; the + card read the mounted file, so a renewal showed as served up to a day + before it was, and a renewal Postfix never took up - a key that does not + belong to it, a failed reload - not at all. The card now shows Postfix's + copy, names a renewal waiting in the mount, and warns when it has waited + longer than the daily check allows; the expiry notice is not sent while a + renewal waits on time (assumptions A-65). +- **Words for what happens to mail match Postfix's.** A message refused with + a temporary error while the panel or the spam filter does not answer was + called deferred, which in Postfix's words is mail waiting in its queue; + the Upstream help said Required TLS holds the mail until TLS works, but + an upstream that refuses STARTTLS with a permanent error bounces it + (assumptions D-P1, D-P3). ### Changed diff --git a/cmd/panel/envdoc_test.go b/cmd/panel/envdoc_test.go index dfc3d06..b4cfa39 100644 --- a/cmd/panel/envdoc_test.go +++ b/cmd/panel/envdoc_test.go @@ -83,6 +83,7 @@ var loadConfigKeys = []string{ "PANEL_SESSION_IDLE_DAYS", "SELFPOST_DNS_RESOLVERS", "TLS_CERT_FILE", + "TLS_RELOAD_INTERVAL_SECONDS", "OPENDKIM_SOCKET", "OPENDKIM_DIR", "DKIM_SELECTOR_DEFAULT", diff --git a/cmd/panel/httpserver.go b/cmd/panel/httpserver.go index afae6dd..b51bafc 100644 --- a/cmd/panel/httpserver.go +++ b/cmd/panel/httpserver.go @@ -114,6 +114,8 @@ func newPanel(cfg config, st *store.Store) (*web.Server, error) { Version: buildinfo.Version, TrustedProxyCIDRs: cfg.trustedProxies, TLSCertFile: cfg.tlsCertFile, + TLSServedCertFile: cfg.tlsServedCertFile, + TLSReloadInterval: cfg.tlsReloadInterval, OpenDKIMSocket: cfg.opendkimSocket, JournalSocket: cfg.journalSocket, SenderSocket: cfg.senderSocket, diff --git a/cmd/panel/main.go b/cmd/panel/main.go index 3ed1fd8..a17da64 100644 --- a/cmd/panel/main.go +++ b/cmd/panel/main.go @@ -19,10 +19,12 @@ import ( "strings" "sync" "syscall" + "time" "github.com/mixeme/selfpost/internal/backup" "github.com/mixeme/selfpost/internal/buildinfo" "github.com/mixeme/selfpost/internal/dnscheck" + "github.com/mixeme/selfpost/internal/health" "github.com/mixeme/selfpost/internal/inbound" "github.com/mixeme/selfpost/internal/logtail" "github.com/mixeme/selfpost/internal/store" @@ -89,6 +91,11 @@ type config struct { // was configured with. tlsCertFile string opendkimSocket string + // tlsServedCertFile is the certificate Postfix serves (its copy of + // tlsCertFile), read from postconf at the start; tlsReloadInterval is + // how often build/postfix-cert-reload.sh looks for a renewal. + tlsServedCertFile string + tlsReloadInterval time.Duration opendkimDir string dkimSelectorDef string @@ -158,8 +165,9 @@ func loadConfig() config { rateLimitMessagesPerIP: envInt("RATE_LIMIT_MESSAGES_PER_IP", 100), rateLimitWindowSeconds: envInt("RATE_LIMIT_WINDOW_SECONDS", 3600), - tlsCertFile: envDefault("TLS_CERT_FILE", "/etc/postfix/tls/fullchain.pem"), - opendkimSocket: envDefault("OPENDKIM_SOCKET", "/run/opendkim/opendkim.sock"), + tlsCertFile: envDefault("TLS_CERT_FILE", "/etc/postfix/tls/fullchain.pem"), + tlsReloadInterval: time.Duration(envInt("TLS_RELOAD_INTERVAL_SECONDS", 86400)) * time.Second, + opendkimSocket: envDefault("OPENDKIM_SOCKET", "/run/opendkim/opendkim.sock"), // Per-domain DKIM state (architecture.md § OpenDKIM). The directory layout // matches what entrypoint.sh prepares (setgid, shared `selfpost` group). @@ -280,6 +288,9 @@ func parseTrustedProxies(raw string) []*net.IPNet { // Image and processes). func run() error { cfg := loadConfig() + // postfix-config.sh has written main.cf before supervisord started the + // panel (build/entrypoint.sh), and nothing rewrites it after. + cfg.tlsServedCertFile = health.ServedCertPath() ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() diff --git a/cmd/panel/notifier.go b/cmd/panel/notifier.go index 80c7f6c..a257bcf 100644 --- a/cmd/panel/notifier.go +++ b/cmd/panel/notifier.go @@ -3,6 +3,7 @@ package main import ( "context" "log" + "time" "github.com/mixeme/selfpost/internal/dnscheck" "github.com/mixeme/selfpost/internal/health" @@ -52,8 +53,10 @@ func runNotifier(ctx context.Context, cfg config, st *store.Store) error { DMARCReportEmail: d.DMARCRua, }, true), nil }, - Certificate: func() health.Certificate { return health.CheckCertificate(cfg.tlsCertFile) }, - Queue: postfix.QueueMessages, + Certificate: func() health.Certificate { + return health.CheckServedCertificate(cfg.tlsServedCertFile, cfg.tlsCertFile, cfg.tlsReloadInterval, time.Now()) + }, + Queue: postfix.QueueMessages, }, notify.NewMailer(cfg.hostname), cfg.hostname) return n.Run(ctx) } diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index 0b226f3..189ec81 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -980,11 +980,15 @@ those after it move up one patch number each. *Done 2026-10-15:* no lock for the backup after all: it reads each file through one descriptor, and a rename leaves that file whole; the panel's own changes are serialised by a mutex. -- [ ] **Health says what is served** (A-38, A-65, D-P1, D-P3): a socket file +- [x] **Health says what is served** (A-38, A-65, D-P1, D-P3): a socket file is not a listener - unix sockets are dialled as inet ones are; the certificate shown is the one Postfix serves (`postconf -h smtpd_tls_cert_file`), with a renewal waiting named; the wording of "Postfix defers mail" and of Required TLS (mockup first). - **Opus** + *Done 2026-10-15:* `ea8b6bf` (mockup) and the commit after it; a probe + connection is silent in all three milters (libmilter read: sendmail + 8.17.1.9 fetched into dev/upstream); the expiry notice waits for a renewal + that is on time. - [ ] **The panel in browsers without Sec-Fetch-Site** (D-B3, D-B13): under `Referrer-Policy: no-referrer` a form POST carries `Origin: null`, which the origin check refuses - every POST, sign-in included, fails in Safari diff --git a/docs/roadmap.md b/docs/roadmap.md index 9a7f905..623d0f0 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`) | 40/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`) | 41/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/health/cert.go b/internal/health/cert.go index 2be7b13..33006b6 100644 --- a/internal/health/cert.go +++ b/internal/health/cert.go @@ -1,10 +1,13 @@ package health import ( + "bytes" "crypto/x509" "encoding/pem" "fmt" "os" + "os/exec" + "strings" "time" ) @@ -25,6 +28,8 @@ const certWarnDays = 14 // TLS_RELOAD_INTERVAL_SECONDS later, and each smtpd loads it when it starts // (Postfix 3.7.11, src/smtpd/smtpd.c, pre_jail_init(), TLS_SERVER_INIT) - so // the mounted file can be newer than what is served (assumptions A-65). +// CheckServedCertificate reads the copy, and names the mounted file as a +// renewal waiting while the two differ. type Certificate struct { Path string Subject string @@ -32,6 +37,98 @@ type Certificate struct { DaysLeft int Status Status Detail string + // Renewal is the mounted certificate while Postfix serves an older one. + Renewal *Renewal +} + +// Renewal is a renewed certificate in the mounted file that Postfix has not +// taken up yet. +type Renewal struct { + NotAfter time.Time + Mounted time.Time // the mounted file's modification time + // Late is true once it has waited longer than cert-reload's interval + // and an hour: the check that should have taken it up has passed. + Late bool +} + +// ServedCertPath is the certificate file Postfix serves, smtpd_tls_cert_file +// as postconf prints it (Postfix 3.7.11, postconf(1): -h, the value without +// its name): build/postfix-config.sh's copy when the mounted files were there +// at the start, the mounted file itself otherwise. "" when postconf fails. +// It is read once, at the panel's start, as the retry policy is: main.cf is +// written before supervisord starts the panel and not after. +func ServedCertPath() string { + out, err := exec.Command("postconf", "-h", "smtpd_tls_cert_file").Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +// CheckServedCertificate checks the certificate Postfix serves (served) and, +// when the mounted file differs from it byte for byte - which is how +// build/postfix-cert-reload.sh decides to copy it (cmp -s) - reports the +// mounted one as a renewal waiting. interval is TLS_RELOAD_INTERVAL_SECONDS: +// cert-reload looks for a renewal that often. A renewal that waits is the +// normal state for up to that long; the served certificate's nearing expiry +// is then no warning, its expiry still an error (clients see what is served). +// One waiting longer is a warning: cert-reload refused it (the key in the +// mount does not belong to it) or Postfix's reload failed, and cert-reload's +// lines in the container log say which. With served empty the mounted file is +// checked, as before. +func CheckServedCertificate(served, mounted string, interval time.Duration, now time.Time) Certificate { + if served == "" { + return CheckCertificate(mounted) + } + c := CheckCertificate(served) + if mounted == "" || mounted == served { + return c + } + have, err1 := os.ReadFile(served) + want, err2 := os.ReadFile(mounted) + fi, err3 := os.Stat(mounted) + if err1 != nil || err2 != nil || err3 != nil || bytes.Equal(have, want) { + return c + } + leaf, err := parseLeaf(want) + if err != nil { + return c + } + r := &Renewal{NotAfter: leaf.NotAfter, Mounted: fi.ModTime(), Late: now.Sub(fi.ModTime()) > interval+time.Hour} + c.Renewal = r + switch { + case r.Late: + c.Status = Worst(c.Status, StatusWarn) + c.Detail = fmt.Sprintf("A renewed certificate has been in the mounted file since %s UTC, longer than Postfix's check for a renewal allows, and Postfix still serves the one above. cert-reload's lines in the container log say why.", + r.Mounted.UTC().Format("2006-01-02 15:04")) + case c.Status == StatusError: + c.Detail += " A renewed one is waiting in the mounted file; Postfix takes it up at its next check for a renewal, at most " + intervalText(interval) + " after it was mounted." + default: + c.Status = StatusOK + c.Detail = "Postfix still serves the certificate above. A renewed one is waiting in the mounted file; Postfix takes it up at its next check for a renewal, at most " + intervalText(interval) + " after it was mounted." + } + return c +} + +// intervalText says an interval the way the help says the default: "a day". +func intervalText(d time.Duration) string { + unit, size := "second", time.Second + switch { + case d >= 24*time.Hour && d%(24*time.Hour) == 0: + unit, size = "day", 24*time.Hour + case d >= time.Hour && d%time.Hour == 0: + unit, size = "hour", time.Hour + case d >= time.Minute && d%time.Minute == 0: + unit, size = "minute", time.Minute + } + switch n := int64(d / size); { + case n == 1 && unit == "hour": + return "an hour" + case n == 1: + return "a " + unit + default: + return fmt.Sprintf("%d %ss", n, unit) + } } // CheckCertificate parses the leaf certificate at path and reports how much diff --git a/internal/health/health_test.go b/internal/health/health_test.go index 94122fc..0fea95b 100644 --- a/internal/health/health_test.go +++ b/internal/health/health_test.go @@ -10,6 +10,7 @@ import ( "net" "os" "path/filepath" + "strings" "testing" "time" ) @@ -213,3 +214,77 @@ func TestUptime(t *testing.T) { } } } + +// The certificate shown is the one Postfix serves; a different mounted file is +// a renewal waiting, a warning once it waited past cert-reload's interval +// (assumptions A-65). +func TestCheckServedCertificate(t *testing.T) { + dir := t.TempDir() + served := filepath.Join(dir, "served.pem") + mounted := filepath.Join(dir, "mounted.pem") + day := 24 * time.Hour + + // The same file: nothing waits. + writeCert(t, served, "mail.example.com", 60*day) + b, _ := os.ReadFile(served) + if err := os.WriteFile(mounted, b, 0o600); err != nil { + t.Fatal(err) + } + if got := CheckServedCertificate(served, mounted, day, time.Now()); got.Status != StatusOK || got.Renewal != nil || got.Path != served { + t.Errorf("same certificate: %+v", got) + } + + // Postfix's copy nears its end, a renewal was mounted an hour ago: no + // warning, the renewal named. + writeCert(t, served, "mail.example.com", 5*day) + writeCert(t, mounted, "mail.example.com", 90*day) + now := time.Now() + if err := os.Chtimes(mounted, now.Add(-time.Hour), now.Add(-time.Hour)); err != nil { + t.Fatal(err) + } + got := CheckServedCertificate(served, mounted, day, now) + if got.Status != StatusOK || got.Renewal == nil || got.Renewal.Late || got.DaysLeft > 5 { + t.Fatalf("renewal waiting: %+v", got) + } + if !strings.Contains(got.Detail, "at most a day after it was mounted") { + t.Errorf("renewal waiting, detail: %q", got.Detail) + } + if d := got.Renewal.NotAfter.Sub(now); d < 89*day { + t.Errorf("the renewal's expiry is the mounted certificate's: %v", d) + } + + // Waiting past the interval and an hour: a warning, cert-reload named. + if err := os.Chtimes(mounted, now.Add(-26*time.Hour), now.Add(-26*time.Hour)); err != nil { + t.Fatal(err) + } + got = CheckServedCertificate(served, mounted, day, now) + if got.Status != StatusWarn || got.Renewal == nil || !got.Renewal.Late || !strings.Contains(got.Detail, "cert-reload") { + t.Errorf("renewal late: %+v", got) + } + + // The served one expired: still an error, the renewal said to be coming. + writeCert(t, served, "mail.example.com", -day) + if err := os.Chtimes(mounted, now, now); err != nil { + t.Fatal(err) + } + got = CheckServedCertificate(served, mounted, day, now) + if got.Status != StatusError || got.Renewal == nil || !strings.Contains(got.Detail, "A renewed one is waiting") { + t.Errorf("expired, renewal waiting: %+v", got) + } + + // No served path known: the mounted file, as before. + if got := CheckServedCertificate("", mounted, day, now); got.Path != mounted || got.Renewal != nil { + t.Errorf("served path unknown: %+v", got) + } +} + +func TestIntervalText(t *testing.T) { + for d, want := range map[time.Duration]string{ + 24 * time.Hour: "a day", 48 * time.Hour: "2 days", time.Hour: "an hour", + 6 * time.Hour: "6 hours", 2 * time.Second: "2 seconds", 90 * time.Minute: "90 minutes", + } { + if got := intervalText(d); got != want { + t.Errorf("intervalText(%v) = %q, want %q", d, got, want) + } + } +} diff --git a/internal/health/milter.go b/internal/health/milter.go index 523e033..7a69ced 100644 --- a/internal/health/milter.go +++ b/internal/health/milter.go @@ -54,6 +54,19 @@ func checkMilter(name, addr string, required bool, timeout time.Duration) Socket case fi.Mode()&os.ModeSocket == 0: return fail("The path exists but is not a socket.") } + // Dialled as an inet address is: a socket file outlives the + // filter that made it (CheckSocket; assumptions A-38). + switch err := dialUnix(path); { + case err == nil: + case errors.Is(err, syscall.EACCES): + s.Status = StatusUnknown + s.Detail = "The panel may not connect to the socket, so whether the filter listens is not known." + return s + case errors.Is(err, syscall.ECONNREFUSED): + return fail("Nothing listens on the socket: the filter that made it has stopped.") + default: + return fail("The socket does not answer.") + } case strings.HasPrefix(addr, "inet:"): host, port, ok := splitInet(strings.TrimPrefix(addr, "inet:")) if !ok { diff --git a/internal/health/milter_test.go b/internal/health/milter_test.go index b17f243..160c4fa 100644 --- a/internal/health/milter_test.go +++ b/internal/health/milter_test.go @@ -5,6 +5,7 @@ import ( "net" "os" "path/filepath" + "runtime" "strconv" "strings" "testing" @@ -144,3 +145,33 @@ func TestCheckMilterInet6(t *testing.T) { t.Errorf("IPv6 listener: %+v", got) } } + +// A socket file whose listener is gone - a killed OpenDKIM or panel leaves +// one (unix(7)) - is not a listener, though the file is there (A-38). +func TestStaleSocketFileIsNotListening(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("a stale AF_UNIX file is a Linux matter here") + } + sock := filepath.Join(t.TempDir(), "stale.sock") + l, err := net.Listen("unix", sock) + if err != nil { + t.Skipf("unix sockets unavailable here: %v", err) + } + l.(*net.UnixListener).SetUnlinkOnClose(false) + l.Close() + if _, err := os.Stat(sock); err != nil { + t.Fatalf("the socket file should outlive its listener: %v", err) + } + + got := CheckSocket("OpenDKIM", sock, true) + if got.Present || got.Status != StatusError || !strings.HasPrefix(got.Detail, "Nothing listens on the OpenDKIM socket") { + t.Errorf("stale required socket: %+v", got) + } + if got := CheckSocket("send-log", sock, false); got.Status != StatusWarn { + t.Errorf("stale optional socket: status %q", got.Status) + } + got = CheckMilter("Spam filter", "unix:"+sock, true) + if got.Present || got.Status != StatusError || got.Detail != "Nothing listens on the socket: the filter that made it has stopped." { + t.Errorf("stale filter socket: %+v", got) + } +} diff --git a/internal/health/socket.go b/internal/health/socket.go index 3c0cd84..1bba430 100644 --- a/internal/health/socket.go +++ b/internal/health/socket.go @@ -1,8 +1,12 @@ package health import ( + "errors" "fmt" + "net" "os" + "syscall" + "time" ) // Socket is the state of one milter socket Postfix connects to. @@ -17,7 +21,7 @@ type Socket struct { Detail string } -// CheckSocket stats a milter socket. required distinguishes the two milters: +// CheckSocket checks a milter socket. required distinguishes the two milters: // Postfix attaches OpenDKIM with default_action=tempfail, so a missing socket // stops mail leaving the server, while the journal-milter is attached with // default_action=accept and fails open — mail still goes out, only the send @@ -25,10 +29,17 @@ type Socket struct { // 3.7.11, src/milter/milter8.c, milter8_comm_error(): tempfail answers "451 // 4.7.1 Service unavailable - try again later", accept answers nothing). // -// A socket file that is there is reported as listening. It is not proof of -// it: a unix socket's file outlives the process that bound it unless that -// process unlinks it (unix(7)), so a killed OpenDKIM or panel leaves one that -// refuses every connection - assumptions A-38. +// The socket is dialled, as Postfix will dial it; a file is not a listener. +// A unix socket's file outlives the process that bound it unless that process +// unlinks it, and a connect() to it then fails with ECONNREFUSED (unix(7)), so +// a killed OpenDKIM or panel leaves a file that refuses every connection - +// assumptions A-38. The connection is closed before a byte is sent, which +// none of the three milters logs: OpenDKIM's libmilter reads the EOF as +// SMFIC_EOF and ends the session without a line (sendmail 8.17.1.9, +// libmilter/comm.c, mi_rd_cmd(); engine.c, mi_engine()), and OpenDKIM's +// mlfi_close() has no connection to clean up (opendkim 2.11.0~beta2, +// opendkim/opendkim.c); go-milter v0.4.1 returns quietly on io.EOF +// (session.go, HandleMilterCommands()). func CheckSocket(name, path string, required bool) Socket { s := Socket{Name: name, Path: path} if path == "" { @@ -45,13 +56,38 @@ func CheckSocket(name, path string, required bool) Socket { s.Status = missingStatus(required) s.Detail = fmt.Sprintf("%s exists but is not a socket.", path) default: - s.Present = true - s.Status = StatusOK - s.Detail = "Listening" + switch err := dialUnix(path); { + case err == nil: + s.Present = true + s.Status = StatusOK + s.Detail = "Listening" + case errors.Is(err, syscall.EACCES): + s.Status = StatusUnknown + s.Detail = fmt.Sprintf("The panel may not connect to %s, so whether anything listens is not known.", path) + case errors.Is(err, syscall.ECONNREFUSED): + s.Status = missingStatus(required) + s.Detail = fmt.Sprintf("Nothing listens on the %s socket: the program that made it has stopped. %s", name, missingCost(required)) + default: + s.Status = missingStatus(required) + s.Detail = fmt.Sprintf("The %s socket does not answer. %s", name, missingCost(required)) + } } return s } +// unixDialTimeout bounds a dial of a local socket; a listener that is there +// answers at once, from the kernel's backlog. +const unixDialTimeout = 2 * time.Second + +// dialUnix connects to the unix socket at path and closes the connection. +func dialUnix(path string) error { + c, err := net.DialTimeout("unix", path, unixDialTimeout) + if err != nil { + return err + } + return c.Close() +} + func missingStatus(required bool) Status { if required { return StatusError @@ -60,8 +96,14 @@ func missingStatus(required bool) Status { } func missingDetail(name string, required bool) string { + return fmt.Sprintf("The %s socket is missing. %s", name, missingCost(required)) +} + +// missingCost is what a milter that cannot be reached costs, by its +// default_action (see CheckSocket). +func missingCost(required bool) string { if required { - return fmt.Sprintf("The %s socket is missing. Postfix rejects mail with a temporary error until it is back.", name) + return "Postfix rejects mail with a temporary error until it is back." } - return fmt.Sprintf("The %s socket is missing. Mail still goes out, but the send log is not being written.", name) + return "Mail still goes out, but the send log is not being written." } diff --git a/internal/notify/notify.go b/internal/notify/notify.go index 31eae1b..b4f0e13 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -370,6 +370,9 @@ func (n *Notifier) checkCertificate() { if c.NotAfter.IsZero() { return // no certificate configured, or one that cannot be read: Health says so } + if c.Renewal != nil && !c.Renewal.Late { + return // a renewal is mounted and Postfix takes it up at its next check + } const key = "cert" mark := c.NotAfter.UTC().Format(time.RFC3339) st, have, err := n.st.GetNotifyState(key) diff --git a/internal/notify/notify_test.go b/internal/notify/notify_test.go index 91b9f60..e8b0cc9 100644 --- a/internal/notify/notify_test.go +++ b/internal/notify/notify_test.go @@ -251,6 +251,18 @@ func TestCertificateIsSaidTwice(t *testing.T) { want(t, "no certificate", b.hour()) } +// The certificate Postfix serves nears its end while a renewal waits in the +// mount: nothing is said while Postfix's check for it is still to come, and +// the usual notice once the renewal has waited past it (A-65). +func TestCertificateWithARenewalWaiting(t *testing.T) { + b := newBench(t) + expiry := b.now.Add(10 * 24 * time.Hour) + b.cert = health.Certificate{NotAfter: expiry, Renewal: &health.Renewal{NotAfter: expiry.Add(80 * 24 * time.Hour)}} + want(t, "renewal waiting", b.hour()) + b.cert.Renewal.Late = true + want(t, "renewal late", b.hour(), "root@example.org|[SelfPost] TLS certificate of mail.example.org expires in 10 days") +} + // A message older than three days is said once, and its going once; a queue // that cannot be listed says nothing. func TestQueueIsSaidOnceAndClearedOnce(t *testing.T) { diff --git a/internal/web/handlers/handlers.go b/internal/web/handlers/handlers.go index e991b9e..d3baeda 100644 --- a/internal/web/handlers/handlers.go +++ b/internal/web/handlers/handlers.go @@ -3,6 +3,7 @@ package handlers import ( "log" + "time" "github.com/mixeme/selfpost/internal/app" "github.com/mixeme/selfpost/internal/dmarc" @@ -26,6 +27,8 @@ type Config struct { DeployRoot string Version string TLSCertFile string + TLSServedCertFile string + TLSReloadInterval time.Duration OpenDKIMSocket string JournalSocket string SenderSocket string diff --git a/internal/web/handlers/handlers_health_page_test.go b/internal/web/handlers/handlers_health_page_test.go index d9aa616..0e4cf0a 100644 --- a/internal/web/handlers/handlers_health_page_test.go +++ b/internal/web/handlers/handlers_health_page_test.go @@ -150,6 +150,31 @@ func TestHealthPageStatesOfTheOtherChecks(t *testing.T) { } } +// The certificate card names a renewal Postfix has not taken up yet: its +// expiry and when it was mounted, beside the served one's (A-65). +func TestHealthPageNamesARenewalWaiting(t *testing.T) { + h, _ := settingsServer(t) + c := healthReading() + cert := c["Cert"].(health.Certificate) + cert.Renewal = &health.Renewal{ + NotAfter: time.Date(2027, 1, 31, 8, 2, 0, 0, time.UTC), + Mounted: time.Date(2026, 10, 15, 3, 12, 0, 0, time.UTC), + } + cert.Detail = "Postfix still serves the certificate above. A renewed one is waiting in the mounted file." + c["Cert"] = cert + out := renderHealth(t, h, c, "") + for _, want := range []string{ + "
Expires
", "2026-11-08 09:14 UTC", + "
Renewal expires
", "2027-01-31 08:02 UTC", + "
Renewal mounted
", "2026-10-15 03:12 UTC", + "Postfix still serves the certificate above.", + } { + if !strings.Contains(out, want) { + t.Errorf("health page is missing %q", want) + } + } +} + // The fragment is what the page holds in its polled rows, read again. func TestHealthFragmentCarriesTheSameRows(t *testing.T) { h, _ := settingsServer(t) diff --git a/internal/web/handlers/handlers_status.go b/internal/web/handlers/handlers_status.go index ca87b50..d2f2733 100644 --- a/internal/web/handlers/handlers_status.go +++ b/internal/web/handlers/handlers_status.go @@ -207,6 +207,11 @@ func healthPage(m view.Meta, c map[string]any, flash string) *view.Health { if cert.Subject != "" { page.CertFacts = append(page.CertFacts, view.Fact{Label: "Names", Value: view.Plain(cert.Subject), Mono: true}) } + if r := cert.Renewal; r != nil { + page.CertFacts = append(page.CertFacts, + view.Fact{Label: "Renewal expires", Value: view.Plain(r.NotAfter.UTC().Format("2006-01-02 15:04 UTC")), Mono: true}, + view.Fact{Label: "Renewal mounted", Value: view.Plain(r.Mounted.UTC().Format("2006-01-02 15:04 UTC")), Mono: true}) + } page.CertDetail, page.CertProblem = cert.Detail, cert.Status != health.StatusOK page.Sockets.End = view.Verdict(string(c["SocketStatus"].(health.Status))) @@ -430,7 +435,7 @@ func (h *Handlers) healthChecks() map[string]any { queueStatus = health.StatusWarn } - cert := health.CheckCertificate(h.cfg.TLSCertFile) + cert := health.CheckServedCertificate(h.cfg.TLSServedCertFile, h.cfg.TLSCertFile, h.cfg.TLSReloadInterval, time.Now()) sockets := []health.Socket{ health.CheckSocket("OpenDKIM", h.cfg.OpenDKIMSocket, true), health.CheckSocket("sender-check", h.cfg.SenderSocket, true), diff --git a/internal/web/view/health.go b/internal/web/view/health.go index 8f1a3d7..309191b 100644 --- a/internal/web/view/health.go +++ b/internal/web/view/health.go @@ -175,7 +175,7 @@ func (h *Health) WithSenderCheck(rows []SenderCheckRow, since, status string) *H h.SenderCheck.End = Verdict(status) h.SenderRows = rows h.SenderNote = Rich("The panel holds each message's ", Code("From:"), - " against the application that sends it. While the panel does not answer, Postfix defers mail instead of accepting it unchecked. Time is counted only while Postfix itself was running; ≈ marks a period with a start at which that could not be asked, and its time is at most what is shown.") + " against the application that sends it. While the panel does not answer, Postfix answers the sending application with a temporary error instead of accepting mail unchecked, and the application tries again later. Time is counted only while Postfix itself was running; ≈ marks a period with a start at which that could not be asked, and its time is at most what is shown.") if since != "" { h.SenderNote = append(h.SenderNote, Rich(" Counted since ", Mono(since), ".")...) } diff --git a/internal/web/view/help.go b/internal/web/view/help.go index 0c5c39f..ae60a93 100644 --- a/internal/web/view/help.go +++ b/internal/web/view/help.go @@ -148,7 +148,7 @@ var overviewTopics = []HelpTopic{ " shows the readings behind them: CPU and memory are the container's own, not the host's spare capacity, and network is a short window, not a daily total."), }}, {ID: HelpCertificate, Title: "TLS certificate", Body: []Text{ - Plain("The certificate Postfix presents on 465 and 587. The reverse proxy or the image mounts it, the panel does not issue it. It warns 14 days before expiry and fails once expired, or when the file is missing or unreadable — clients that verify certificates then refuse to connect."), + Plain("The certificate Postfix presents on 465 and 587. The reverse proxy or the image mounts it, the panel does not issue it. Postfix serves its own copy of the mounted file and looks for a renewed one once a day by default; until it takes one up, the card shows it as a renewal waiting, and warns once it has waited longer than that check allows - the container log's cert-reload lines say why. It warns 14 days before expiry unless a renewal is already waiting, and fails once expired, or when the file is missing or unreadable — clients that verify certificates then refuse to connect."), }}, {ID: HelpRDNS, Title: "Hostname and reverse DNS", Body: []Text{ Rich("The hostname (", Code("SELFPOST_HOSTNAME"), ") must resolve to this server's address, and that address must resolve back to the hostname. Large receivers reject or spam-folder mail from hosts where the two disagree. The PTR record is set at your hosting provider, not in your DNS zone."), @@ -171,7 +171,7 @@ var outboundTopics = []HelpTopic{ Code("From:"), " - another of your domains, the server's own name, an address somewhere else, or none at all - is refused for good with ", Code("550 5.7.1"), ". It is a rejected row of the Outbound log, and its page says which address was refused. This is what keeps one application from getting mail signed with another domain's key.", Br(), Br(), - "The check is strict. While the panel does not answer, Postfix does not accept mail unchecked: it replies with a temporary error and the application sends again later. Server › Health counts what that costs: for how long the panel did not answer while Postfix itself was running; how many connections Postfix turned away because of it - it refuses from the connection on, before a message is offered, so a client that tries three times is three and a monitor that only connects is one; and how many messages were deferred in the middle, when the panel stopped answering after a message had begun. A time marked ≈ includes a start at which supervisord could not say since when Postfix was running: the time since the panel's last sign of life is counted, which may be more than Postfix was up. The card warns when a connection was turned away or a message deferred in the last 24 hours. If the numbers stay at nothing, the check is where it should be; if they do not, it belongs in a process of its own."), + "The check is strict. While the panel does not answer, Postfix does not accept mail unchecked: it replies with a temporary error and the application sends again later. Server › Health counts what that costs: for how long the panel did not answer while Postfix itself was running; how many connections Postfix turned away because of it - it refuses from the connection on, before a message is offered, so a client that tries three times is three and a monitor that only connects is one; and how many messages were refused with a temporary error in the middle, when the panel stopped answering after a message had begun. A time marked ≈ includes a start at which supervisord could not say since when Postfix was running: the time since the panel's last sign of life is counted, which may be more than Postfix was up. The card warns when a connection was turned away or a message refused in the last 24 hours. If the numbers stay at nothing, the check is where it should be; if they do not, it belongs in a process of its own."), }}, {ID: HelpLimits, Title: "Rate limits", Body: []Text{ Rich("Level 1 is a per-IP ceiling set in ", Code(".env"), ". Level 2 is optional, per domain and per application, entered by hand or computed from the last 30 days of sending, and can never exceed level 1. An application's own limit replaces its domain's."), @@ -196,14 +196,14 @@ var inboundTopics = []HelpTopic{ Rich("An optional backup-MX or forwarder, switched on in the server's ", Code(".env"), "; the Inbound menu exists only then. Postfix accepts mail on port 25 for the listed domains only and hands each message on to that domain's upstream; there are no mailboxes. For each domain publish the MX record its page shows, and keep a primary MX the domain already has if this is a backup. The MX tag is green when at least one MX of the domain points at this server, and a domain accepts no mail until an upstream is saved on its page. Adding and deleting domains is for the global role."), }}, {ID: HelpUpstream, Title: "Upstream", Body: []Text{ - Plain("The upstream is the server that actually holds the mailboxes. Give its host and port — used exactly as written, with no MX lookup — and choose the TLS for the hop to it: Opportunistic uses TLS when the upstream offers it and otherwise sends in the clear, Required holds the mail until a TLS session is possible, Off never uses TLS. While the upstream does not answer, Postfix keeps the mail and retries as for any other mail."), + Plain("The upstream is the server that actually holds the mailboxes. Give its host and port — used exactly as written, with no MX lookup — and choose the TLS for the hop to it: Opportunistic uses TLS when the upstream offers it and otherwise sends in the clear; Required sends only over TLS, and the mail waits in the queue while the upstream does not offer it or the TLS handshake fails - an upstream that answers STARTTLS with a permanent error bounces it; Off never uses TLS. While the upstream does not answer, Postfix keeps the mail and retries as for any other mail."), }}, {ID: HelpRecipients, Title: "Valid recipients", Body: []Text{ Plain("Choose whom this domain accepts. Listed addresses only: the relay accepts exactly the addresses you list, one per line or comma-separated, and refuses any other while the sender is still connected, so it never has to send a bounce for them. Any recipient: everything at the domain is accepted and passed on, and an address the upstream then rejects comes back as a bounce from this server to a sender who is often forged — backscatter. Prefer a list, and keep it in step with the mailboxes upstream."), }}, {ID: HelpFilter, Title: "Spam filter", Body: []Text{ Rich("The spam filter belongs to the server, not to a domain. The server's administrator attaches it in the server's ", Code(".env"), - ", and from then on every inbound domain is screened by it before mail is handed on; the box shows on or off and has nothing to switch. Off means mail goes to the upstream unfiltered. While the filter is down, mail is either accepted unfiltered (the default) or deferred, so the sending server tries again later, as the server's setting says; the box states which."), + ", and from then on every inbound domain is screened by it before mail is handed on; the box shows on or off and has nothing to switch. Off means mail goes to the upstream unfiltered. While the filter is down, mail is either accepted unfiltered (the default) or refused with a temporary error, so the sending server tries again later, as the server's setting says; the box states which."), }}, {ID: HelpInboundLog, Title: "Inbound log", Body: []Text{ Rich("One row per recipient of a message offered on port 25 for an inbound domain, with what was decided: ", diff --git a/internal/web/web.go b/internal/web/web.go index e41a02c..b768daf 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -8,6 +8,7 @@ import ( "net" "net/http" "strings" + "time" "github.com/mixeme/selfpost/internal/app" "github.com/mixeme/selfpost/internal/dmarc" @@ -57,12 +58,16 @@ type Config struct { // honoured, so the header can't be spoofed by anyone but a trusted proxy. // Empty (the default) keeps rate-limiting keyed on RemoteAddr only. TrustedProxyCIDRs []*net.IPNet - // TLSCertFile is the certificate Postfix serves on 465/587 (guide § - // Environment variables), read read-only by the status page to report how - // much validity is left. - TLSCertFile string + // TLSCertFile is the mounted certificate (guide § Environment variables); + // TLSServedCertFile is Postfix's copy of it, the one served on 465/587, + // and TLSReloadInterval how often that copy is refreshed. Health reads + // the served one and names the mounted one while they differ + // (health.CheckServedCertificate). + TLSCertFile string + TLSServedCertFile string + TLSReloadInterval time.Duration // OpenDKIMSocket and JournalSocket are the two milter sockets Postfix - // connects to. The status page stats them: the first is required for mail + // connects to. The status page dials them: the first is required for mail // to leave at all (OpenDKIM runs with default_action=tempfail), the second // only for the send log (the journal-milter fails open). OpenDKIMSocket string @@ -138,6 +143,8 @@ func New(st *store.Store, domains *domain.Service, apps *app.Service, inboundSvc DeployRoot: cfg.DeployRoot, Version: cfg.Version, TLSCertFile: cfg.TLSCertFile, + TLSServedCertFile: cfg.TLSServedCertFile, + TLSReloadInterval: cfg.TLSReloadInterval, OpenDKIMSocket: cfg.OpenDKIMSocket, JournalSocket: cfg.JournalSocket, SenderSocket: cfg.SenderSocket, diff --git a/test/e2e/health_served_test.go b/test/e2e/health_served_test.go new file mode 100644 index 0000000..ab9cbb5 --- /dev/null +++ b/test/e2e/health_served_test.go @@ -0,0 +1,139 @@ +package e2e + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "net" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// testHealthShowsWhatIsServed: Server › Health dials the milter sockets as +// Postfix does, rather than finding their files (assumptions A-38), and shows +// the certificate Postfix serves, naming a different mounted one as a renewal +// waiting (A-65). +func testHealthShowsWhatIsServed(t *testing.T, sc *scenario) { + health := func() string { + t.Helper() + resp, body, err := sc.panel.get("/server/health") + if err != nil || resp.StatusCode != 200 { + t.Fatalf("GET /server/health: %v, %v", resp, err) + } + return body + } + box := func(body, id string) string { + t.Helper() + i := strings.Index(body, `id="`+id+`"`) + if i < 0 { + t.Fatalf("no box %s on Health:\n%.2000s", id, body) + } + rest := body[i:] + if j := strings.Index(rest[1:], `
`); n != 3 { + t.Fatalf("Milter sockets has %d rows, want 3:\n%s", n, sockets) + } + if strings.Contains(sockets, `sp-muted sp-small">`) { + t.Fatalf("a milter socket is not answering the panel's dial:\n%s", sockets) + } + + // A certificate mounted with a key it does not belong to: cert-reload + // refuses to copy it (build/postfix-cert-reload.sh, same_pair), so + // Postfix keeps serving the old one and the mounted one waits. + certPath := filepath.Join(h.stageDir, "certs", "fullchain.pem") + keyPath := filepath.Join(h.stageDir, "certs", "privkey.pem") + served, err := servedNotAfter() + if err != nil { + t.Fatal(err) + } + renewalNotAfter := time.Now().Add(72 * time.Hour).UTC().Truncate(time.Minute) + if err := writeCertWithoutItsKey(certPath, renewalNotAfter); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + // A pair that belongs together, picked up as any renewal is. + _ = writeSelfSignedCert(certPath, keyPath) + _ = waitFor("the mended certificate served", 30*time.Second, 500*time.Millisecond, func() (bool, error) { + want, err := fileSerial(certPath) + if err != nil { + return false, err + } + got, err := servedSerial() + return err == nil && got.Cmp(want) == 0, err + }) + }) + if err := waitFor("cert-reload to refuse the mismatched pair", 20*time.Second, 500*time.Millisecond, func() (bool, error) { + return strings.Contains(h.logs("selfpost"), "do not belong together"), nil + }); err != nil { + t.Fatalf("%v\n%s", err, h.logs("selfpost")) + } + cert := box(health(), "certificate") + for _, want := range []string{ + "
Expires
" + served.UTC().Format("2006-01-02 15:04 UTC"), + "
Renewal expires
" + renewalNotAfter.Format("2006-01-02 15:04 UTC"), + "
Renewal mounted
", + "Postfix still serves the certificate above.", + } { + if !strings.Contains(cert, want) { + t.Fatalf("the TLS certificate card lacks %q:\n%s", want, cert) + } + } +} + +// servedNotAfter is the expiry of the certificate port 465 presents. +func servedNotAfter() (time.Time, error) { + conn, err := tls.DialWithDialer(&net.Dialer{Timeout: 5 * time.Second}, "tcp", smtpsAddr, &tls.Config{ + InsecureSkipVerify: true, + ServerName: selfpostHostname, + }) + if err != nil { + return time.Time{}, err + } + defer conn.Close() + certs := conn.ConnectionState().PeerCertificates + if len(certs) == 0 { + return time.Time{}, fmt.Errorf("no certificate presented") + } + return certs[0].NotAfter, nil +} + +// writeCertWithoutItsKey writes a certificate for a key that is thrown away, +// leaving the mounted key as it was. +func writeCertWithoutItsKey(certPath string, notAfter time.Time) error { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return err + } + serial, err := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128)) + if err != nil { + return err + } + tmpl := &x509.Certificate{ + SerialNumber: serial, + Subject: pkix.Name{CommonName: selfpostHostname}, + DNSNames: []string{selfpostHostname}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: notAfter, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + return err + } + return os.WriteFile(certPath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), 0o644) +} diff --git a/test/e2e/main_test.go b/test/e2e/main_test.go index 6c162a3..c930b3a 100644 --- a/test/e2e/main_test.go +++ b/test/e2e/main_test.go @@ -171,6 +171,10 @@ func TestE2E(t *testing.T) { sc.panel = p }) + run("health_shows_what_is_served", func(t *testing.T) { + testHealthShowsWhatIsServed(t, sc) + }) + run("inbound_ui_absent_when_disabled", func(t *testing.T) { resp, body, err := sc.panel.get("/overview") if err != nil {