From a8fd96c75221acdc1cedda79921d3258369cc2f8 Mon Sep 17 00:00:00 2001 From: Mix Date: Fri, 16 Oct 2026 23:51:42 +0300 Subject: [PATCH] health-setup-checks: the certificate's name, Postfix's greeting, the proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decided by the owner on 2026-10-13 (in place of preflight), taken up on 2026-10-16; mockups b2dcefc, cc33890, 0feaa55 (his: the greeting on a row of its own, the name check a tag beside Names). - The certificate: health.Certificate keeps its names (DNS names, or the CN without any) and ForHost holds it to SELFPOST_HOSTNAME as a TLS client does - VerifyHostname over the DNS names, the CN only without any, as OpenSSL's X509_check_host and so a sending Postfix still take it (src/tls/tls_client.c:439, 513). A certificate for another name fails; the tag beside Names says matches / not for the hostname. - The greeting: postconf -x -h smtpd_banner, read once at the panel's start like the served certificate's path (main.cf is not rewritten after), is what smtpd writes after "220 " (src/smtpd/smtpd.c:5979). A first word other than the hostname warns, with what to set. A fact may now take the whole row (Fact.Wide, sp-wide) and carry a tag beside its label (Fact.LabelTag); both on the kit page. - The proxy: auth.Module.Seen says the address the panel counts sign-ins against, the peer, and whether X-Forwarded-For was read or left unread for an untrusted peer; the Configuration card shows "Your address" and, in the second case, warns that every visitor shares one sign-in limit and names the /32 (or /128) to put in TRUSTED_PROXY_CIDR. Tests: TestCertificateForHost (CN-only, DNS names, a wildcard, the CN ignored beside DNS names), TestSeenAddress, TestHealthSaysWhereItSeesTheViewer, TestHealthGreetingAndCertificateName; e2e reads the stand's right state on Health (matches, "220 mail.e2e.test ESMTP", a direct client). go vet + go test (26 packages) and make e2e on the test server for this tree. Docs: help (word for word with the mockup), guide § Overview and Health, CHANGELOG [Unreleased] § Added, roadmap row done. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 11 ++++ cmd/panel/httpserver.go | 1 + cmd/panel/main.go | 4 ++ docs/guide.md | 15 ++++- docs/roadmap.md | 14 ++++- internal/health/cert.go | 57 +++++++++++++++++++ internal/health/health_test.go | 45 +++++++++++++++ internal/web/auth/auth.go | 26 +++++++++ internal/web/auth/seen_test.go | 33 +++++++++++ internal/web/handlers/handlers.go | 1 + internal/web/handlers/handlers_status.go | 32 +++++++++-- internal/web/handlers/handlers_status_test.go | 42 ++++++++++++++ internal/web/view/components.go | 6 ++ internal/web/view/health.go | 33 +++++++++++ internal/web/view/help.go | 11 +++- internal/web/view/kit.go | 2 + internal/web/view/server_test.go | 15 +++++ internal/web/view/static/panel.css | 5 ++ internal/web/view/templates/components.html | 2 +- internal/web/view/templates/health.html | 4 +- internal/web/web.go | 4 ++ test/e2e/health_served_test.go | 15 ++++- 22 files changed, 364 insertions(+), 14 deletions(-) create mode 100644 internal/web/auth/seen_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ae41b32..800800d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Added + +- **Server › Health checks three things it did not** (health-setup-checks): + whether the TLS certificate is for the server's own name - a tag beside + its names, and a failure when it is not, since a client that checks the + name refuses it however valid it is; Postfix's greeting, the `220` line, + which must name the hostname; and the address the panel sees your browser + at, warning when a reverse proxy it does not trust stands in front - every + visitor then shares one sign-in limit - and naming what to put in + `TRUSTED_PROXY_CIDR`. + ## [2.10.4] - 2026-10-16 The origin check is strict. No migration; schema 12. diff --git a/cmd/panel/httpserver.go b/cmd/panel/httpserver.go index a3b1e68..82e04e5 100644 --- a/cmd/panel/httpserver.go +++ b/cmd/panel/httpserver.go @@ -112,6 +112,7 @@ func newPanel(cfg config, st *store.Store) (*web.Server, error) { TrustedProxyCIDRs: cfg.trustedProxies, TLSCertFile: cfg.tlsCertFile, TLSServedCertFile: cfg.tlsServedCertFile, + SMTPBanner: cfg.smtpBanner, TLSReloadInterval: cfg.tlsReloadInterval, SignSocket: cfg.signSocket, SignLocalSocket: cfg.signLocalSocket, diff --git a/cmd/panel/main.go b/cmd/panel/main.go index 9778830..32f4c27 100644 --- a/cmd/panel/main.go +++ b/cmd/panel/main.go @@ -97,6 +97,9 @@ type config struct { // how often build/postfix-cert-reload.sh looks for a renewal. tlsServedCertFile string tlsReloadInterval time.Duration + // smtpBanner is Postfix's smtpd_banner, expanded, read from postconf at + // the start: Health shows the greeting it makes. + smtpBanner string dkimDir string dkimSelectorDef string @@ -333,6 +336,7 @@ func run() error { // postfix-config.sh has written main.cf before supervisord started the // panel (build/entrypoint.sh), and nothing rewrites it after. cfg.tlsServedCertFile = health.ServedCertPath() + cfg.smtpBanner = health.SMTPBanner() ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() diff --git a/docs/guide.md b/docs/guide.md index d9c1dbe..f0ec906 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -468,8 +468,10 @@ readings, in the order of what stops mail first: one refresh after the page opens. A fully busy processor or a machine out of memory is a warning, because both delay or kill the mail path; throughput is only reported. -- **Processes**, **TLS certificate** (validity and expiry) and **Milter - sockets**: the DKIM signer's two - `dkim-signer · applications` for what +- **Processes**, **TLS certificate** (validity and expiry, and whether its + names cover `SELFPOST_HOSTNAME` - the tag beside *Names*; a certificate for + another name fails, as a client that checks the name refuses it) and + **Milter sockets**: the DKIM signer's two - `dkim-signer · applications` for what applications send on 465/587, `dkim-signer · notifications` for the panel's own mail - the sender check, the send log (the journal-milter) and, with one configured, the spam filter. Postfix attaches the signer @@ -493,7 +495,14 @@ readings, in the order of what stops mail first: - **Hostname and reverse DNS** — compares `SELFPOST_HOSTNAME` against the PTR record the internet publishes for this server's IP (forward-confirmed reverse DNS), see [Server-level DNS](#server-level-dns-ptrrdns); use - *Re-check DNS* after changing DNS. + *Re-check DNS* after changing DNS. *Greeting* is the `220` line Postfix + opens every connection with; it must name the hostname, and another name + there is a warning. +- **Your address** (in the Configuration card) — the address the panel sees + your browser at, and whether it took it from `X-Forwarded-For`. Behind a + reverse proxy that the panel does not trust it is the proxy's own: every + visitor then shares one sign-in limit, and the card says what to set in + `TRUSTED_PROXY_CIDR`. - **Configuration** — the **Reload configuration** button writes again, from the database, every file the panel keeps for the mail path - the DKIM selector table (`/data/dkim/selectors`), the Postfix sender map, the diff --git a/docs/roadmap.md b/docs/roadmap.md index ba986c6..7168e0d 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -29,7 +29,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | ID | Topic | Status | Progress | Plan | |---|---|---|---|---| | inbound-upstream-result | Inbound › Log shows what the upstream answered: sent, deferred, bounced, with its last reply | shipped `2.10.0` | 7/7 | [plans/inbound-upstream-result.md](plans/inbound-upstream-result.md) | -| health-setup-checks | Server › Health also checks the certificate's name, Postfix's greeting and the trusted proxy (was preflight) | agreed (2026-10-13) | — | — | +| health-setup-checks | Server › Health also checks the certificate's name, Postfix's greeting and the trusted proxy (was preflight) | done 2026-10-16, unreleased | — | — | | test-email | Send a test message from the panel (was preflight) | agreed (2026-10-13) | — | — | | inbound-antispam-panel | Inbound antispam journal + allow/deny lists | shipped `2.1.0` | 12/12 | [plans/inbound-antispam-panel.md](plans/inbound-antispam-panel.md) | | origin-strict | Refuse a state-changing request that carries neither `Sec-Fetch-Site` nor `Origin` (was csrf-tokens) | shipped `2.10.4` | — | — | @@ -176,6 +176,18 @@ mockup `preflight` stays drawn and unbuilt. wrong certificate name, a wrong greeting and an untrusted proxy, and right on the test server. +**Done (2026-10-16):** mockups `b2dcefc`, `cc33890`, `0feaa55` (the owner: +the greeting on a row of its own, the name check a tag beside *Names*). +The certificate: `health.Certificate.ForHost` - DNS names as a TLS client +matches them, the CN only without any, as OpenSSL and so a sending Postfix +still does - fails a certificate for another name. The greeting: +`postconf -x -h smtpd_banner`, read once at the start (main.cf is not +rewritten after), is what smtpd writes after `220` +(src/smtpd/smtpd.c:5979); a first word other than the hostname warns. The +proxy: `auth.Module.Seen` - an `X-Forwarded-For` left unread because the +peer is not trusted warns, with the `/32` (or `/128`) to set. Wrong cases in +unit tests, right ones in e2e (`testFormPostWithTheBrowsersOrigin`). + **Version:** `2.x` MINOR; `agreed`. --- diff --git a/internal/health/cert.go b/internal/health/cert.go index c613772..e086cde 100644 --- a/internal/health/cert.go +++ b/internal/health/cert.go @@ -37,8 +37,60 @@ type Certificate struct { DaysLeft int Status Status Detail string + // Names are the names the certificate is for: its DNS names, or its + // common name when it has none. MatchesHost says whether one of them is + // the server's (ForHost); nil until that is asked. + Names []string + MatchesHost *bool // Renewal is the mounted certificate while Postfix serves an older one. Renewal *Renewal + + leaf *x509.Certificate +} + +// ForHost holds the certificate to the server's own name, SELFPOST_HOSTNAME: +// a client that verifies the name - Go's crypto/tls does by default, so does +// any MTA under DANE or MTA-STS - refuses a certificate for another however +// valid it is, so one that is not for host is a failure. The name is matched +// against the DNS names as a TLS client matches it (crypto/x509, +// Certificate.VerifyHostname: a wildcard for one label), and against the +// common name only when there are none - which Go refuses but a sending +// Postfix still takes: its TLS client names the host with SSL_add1_host() +// and only X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS (Postfix 3.10.13, +// src/tls/tls_client.c:439, 513), and OpenSSL's X509_check_host() checks the +// subject CN when there is no DNS subjectAltName unless told +// X509_CHECK_FLAG_NEVER_CHECK_SUBJECT (OpenSSL 3.5, X509_check_host(3)). +// Nothing is said with no certificate read or no host. +func (c Certificate) ForHost(host string) Certificate { + if c.leaf == nil || host == "" { + return c + } + var ok bool + if len(c.leaf.DNSNames) > 0 { + ok = c.leaf.VerifyHostname(host) == nil + } else { + ok = strings.EqualFold(strings.TrimSuffix(c.leaf.Subject.CommonName, "."), strings.TrimSuffix(host, ".")) + } + c.MatchesHost = &ok + if !ok { + c.Status = StatusError + c.Detail = fmt.Sprintf("None of the certificate's names is %s (SELFPOST_HOSTNAME): a client that verifies the name refuses the connection, however valid the certificate is. Mount a certificate for %s.", host, host) + } + return c +} + +// SMTPBanner is Postfix's smtpd_banner with its parameters expanded, as +// postconf -x prints it: what smtpd puts after "220 " when a client connects +// (Postfix 3.10.13, src/smtpd/smtpd.c:5979, smtpd_chat_reply(state, "220 +// %s", var_smtpd_banner); postconf(1) -x expands $name). build/postfix- +// config.sh sets it to "$myhostname ESMTP". It is read once, at the start, +// as ServedCertPath is; "" when postconf fails. +func SMTPBanner() string { + out, err := exec.Command("postconf", "-x", "-h", "smtpd_banner").Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) } // Renewal is a renewed certificate in the mounted file that Postfix has not @@ -155,6 +207,11 @@ func CheckCertificate(path string) Certificate { } c.Subject = leaf.Subject.CommonName + c.Names = leaf.DNSNames + if len(c.Names) == 0 && c.Subject != "" { + c.Names = []string{c.Subject} + } + c.leaf = leaf c.NotAfter = leaf.NotAfter c.DaysLeft = int(time.Until(leaf.NotAfter).Hours() / 24) switch { diff --git a/internal/health/health_test.go b/internal/health/health_test.go index 96b441d..f8438d5 100644 --- a/internal/health/health_test.go +++ b/internal/health/health_test.go @@ -286,3 +286,48 @@ func TestIntervalText(t *testing.T) { } } } + +// A certificate for another name is no certificate for this server: a client +// that checks the name refuses it however valid it is. Its DNS names are +// matched as a TLS client matches them, a wildcard for one label; with none, +// its common name, as OpenSSL - and so a sending Postfix - still does. +func TestCertificateForHost(t *testing.T) { + dir := t.TempDir() + cnOnly := filepath.Join(dir, "cn.pem") + writeCert(t, cnOnly, "mail.example.org", 90*24*time.Hour) + if c := CheckCertificate(cnOnly).ForHost("MAIL.example.org"); c.Status != StatusOK || c.MatchesHost == nil || !*c.MatchesHost { + t.Errorf("a CN-only certificate for the hostname: %+v", c) + } + if c := CheckCertificate(cnOnly).ForHost("other.example.org"); c.Status != StatusError || *c.MatchesHost || + !strings.Contains(c.Detail, "None of the certificate's names is other.example.org") { + t.Errorf("a certificate for another name: %+v", c) + } + + san := filepath.Join(dir, "san.pem") + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + tmpl := &x509.Certificate{SerialNumber: big.NewInt(2), Subject: pkix.Name{CommonName: "mail.example.org"}, + DNSNames: []string{"*.example.net", "relay.example.org"}, NotBefore: time.Now().Add(-time.Hour), NotAfter: time.Now().Add(90 * 24 * time.Hour)} + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(san, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), 0o600); err != nil { + t.Fatal(err) + } + c := CheckCertificate(san) + if strings.Join(c.Names, " ") != "*.example.net relay.example.org" { + t.Errorf("names = %q, want the DNS names", c.Names) + } + for host, want := range map[string]bool{"mail.example.net": true, "relay.example.org": true, "a.b.example.net": false, + "mail.example.org": false /* the CN does not count beside DNS names */} { + if got := c.ForHost(host); *got.MatchesHost != want { + t.Errorf("ForHost(%s) matches = %v, want %v", host, *got.MatchesHost, want) + } + } + if c := (Certificate{Status: StatusError}).ForHost("mail.example.org"); c.MatchesHost != nil { + t.Error("a certificate that was not read was matched") + } +} diff --git a/internal/web/auth/auth.go b/internal/web/auth/auth.go index 2df9f48..765ddfe 100644 --- a/internal/web/auth/auth.go +++ b/internal/web/auth/auth.go @@ -125,6 +125,32 @@ func clientIP(r *http.Request, trustedProxies []*net.IPNet) string { return host } +// SeenAddress is how the panel sees a request's address, for Health: the +// address it counts sign-ins against (Client), the transport peer (Peer), +// and whether Client came from X-Forwarded-For (FromHeader) or the header +// was there and left unread because the peer is not a trusted proxy +// (HeaderIgnored) - the case where every visitor shares the proxy's address +// and one sign-in limit. +type SeenAddress struct { + Client string + Peer string + FromHeader bool + HeaderIgnored bool +} + +// Seen reads a request's address as clientIP does. +func (m *Module) Seen(r *http.Request) SeenAddress { + peer, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + peer = r.RemoteAddr + } + s := SeenAddress{Client: clientIP(r, m.trustedProxies), Peer: peer} + hasHeader := r.Header.Get("X-Forwarded-For") != "" + s.FromHeader = hasHeader && s.Client != peer + s.HeaderIgnored = hasHeader && !s.FromHeader + return s +} + func ipInAny(ip net.IP, nets []*net.IPNet) bool { for _, n := range nets { if n.Contains(ip) { diff --git a/internal/web/auth/seen_test.go b/internal/web/auth/seen_test.go new file mode 100644 index 0000000..4454a25 --- /dev/null +++ b/internal/web/auth/seen_test.go @@ -0,0 +1,33 @@ +package auth + +import ( + "net" + "net/http/httptest" + "testing" +) + +// Health says where the panel sees a request from: through a trusted proxy, +// the client's own address from X-Forwarded-For; through a proxy it does not +// trust, the proxy's, with the header left unread - every visitor then +// shares one sign-in limit; and with no header, the connection's own. +func TestSeenAddress(t *testing.T) { + _, proxyNet, _ := net.ParseCIDR("172.18.0.0/16") + m := &Module{trustedProxies: []*net.IPNet{proxyNet}} + untrusting := &Module{} + + r := httptest.NewRequest("GET", "/server/health", nil) + r.RemoteAddr = "172.18.0.1:40112" + r.Header.Set("X-Forwarded-For", "198.51.100.7") + if s := m.Seen(r); s != (SeenAddress{Client: "198.51.100.7", Peer: "172.18.0.1", FromHeader: true}) { + t.Errorf("through a trusted proxy: %+v", s) + } + if s := untrusting.Seen(r); s != (SeenAddress{Client: "172.18.0.1", Peer: "172.18.0.1", HeaderIgnored: true}) { + t.Errorf("through a proxy not trusted: %+v", s) + } + + r.Header.Del("X-Forwarded-For") + r.RemoteAddr = "203.0.113.9:5000" + if s := m.Seen(r); s != (SeenAddress{Client: "203.0.113.9", Peer: "203.0.113.9"}) { + t.Errorf("direct: %+v", s) + } +} diff --git a/internal/web/handlers/handlers.go b/internal/web/handlers/handlers.go index 15adaaf..e8a5652 100644 --- a/internal/web/handlers/handlers.go +++ b/internal/web/handlers/handlers.go @@ -29,6 +29,7 @@ type Config struct { TLSCertFile string TLSServedCertFile string TLSReloadInterval time.Duration + SMTPBanner string SignSocket string SignLocalSocket string JournalSocket string diff --git a/internal/web/handlers/handlers_status.go b/internal/web/handlers/handlers_status.go index 34362a5..1dcd635 100644 --- a/internal/web/handlers/handlers_status.go +++ b/internal/web/handlers/handlers_status.go @@ -143,6 +143,8 @@ func (h *Handlers) HandleHealthRecheck(w http.ResponseWriter, r *http.Request) { // of Overview and the Notification mail card, which only Health shows. func (h *Handlers) healthView(r *http.Request, flash string) *view.Health { page := healthPage(h.shellMeta(r), h.healthChecks(), flash) + seen := h.auth.Seen(r) + page.WithClientAddress(seen.Client, seen.Peer, seen.FromHeader, seen.HeaderIgnored) mail := h.serverMail(false) page.WithNotificationMail(notify.From(h.cfg.Hostname), mail.rows, mail.status) stats, err := h.store.SenderCheckStats(time.Now()) @@ -205,8 +207,15 @@ func healthPage(m view.Meta, c healthReadings, flash string) *view.Health { if !cert.NotAfter.IsZero() { page.CertFacts = append(page.CertFacts, view.Fact{Label: "Expires", Value: view.Plain(cert.NotAfter.UTC().Format("2006-01-02 15:04 UTC")), Mono: true}) } - if cert.Subject != "" { - page.CertFacts = append(page.CertFacts, view.Fact{Label: "Names", Value: view.Plain(cert.Subject), Mono: true}) + if len(cert.Names) > 0 { + names := view.Fact{Label: "Names", Value: view.Plain(strings.Join(cert.Names, ", ")), Mono: true} + if m := cert.MatchesHost; m != nil { + names.LabelTag = &view.Tag{Status: "ok", Label: "matches"} + if !*m { + names.LabelTag = &view.Tag{Status: "fail", Label: "not for the hostname"} + } + } + page.CertFacts = append(page.CertFacts, names) } if r := cert.Renewal; r != nil { page.CertFacts = append(page.CertFacts, @@ -225,7 +234,7 @@ func healthPage(m view.Meta, c healthReadings, flash string) *view.Health { } ptr := c.PTR - page.Hostname.End = view.Verdict(string(ptr.Status)) + hostStatus := ptr.Status host := c.Hostname if host == "" { host = "(SELFPOST_HOSTNAME is not set)" @@ -241,6 +250,18 @@ func healthPage(m view.Meta, c healthReadings, flash string) *view.Health { } page.HostFacts = append(page.HostFacts, view.Fact{Label: "Lookup", Value: lookup, Mono: true}) } + // The greeting must name the hostname: a receiver compares it with the + // name it looked up, and a container id or localhost there reads as a + // misconfigured host. A banner that could not be read says nothing. + if c.Banner != "" { + fact := view.Fact{Label: "Greeting", Value: view.Plain("220 " + c.Banner), Mono: true, Wide: true} + if f := strings.Fields(c.Banner); c.Hostname != "" && (len(f) == 0 || !strings.EqualFold(f[0], c.Hostname)) { + hostStatus = health.Worst(hostStatus, health.StatusWarn) + fact.Note = view.Plain("Postfix greets with another name than " + c.Hostname + ": set myhostname to it (build/postfix-config.sh does, from SELFPOST_HOSTNAME).") + } + page.HostFacts = append(page.HostFacts, fact) + } + page.Hostname.End = view.Verdict(string(hostStatus)) return page } @@ -439,6 +460,8 @@ type healthReadings struct { SocketStatus health.Status Hostname string PTR dnscheck.Result + // Banner is Postfix's smtpd_banner, expanded: the greeting after "220". + Banner string } // healthChecks runs every server check once. @@ -459,7 +482,7 @@ func (h *Handlers) healthChecks() healthReadings { queueStatus = health.StatusWarn } - cert := health.CheckServedCertificate(h.cfg.TLSServedCertFile, h.cfg.TLSCertFile, h.cfg.TLSReloadInterval, time.Now()) + cert := health.CheckServedCertificate(h.cfg.TLSServedCertFile, h.cfg.TLSCertFile, h.cfg.TLSReloadInterval, time.Now()).ForHost(h.cfg.Hostname) sockets := []health.Socket{ signerSocket(h.cfg.SignSocket, "applications"), signerSocket(h.cfg.SignLocalSocket, "notifications"), @@ -490,6 +513,7 @@ func (h *Handlers) healthChecks() healthReadings { SocketStatus: socketStatus, Hostname: h.cfg.Hostname, PTR: srv.PTR, + Banner: h.cfg.SMTPBanner, } } diff --git a/internal/web/handlers/handlers_status_test.go b/internal/web/handlers/handlers_status_test.go index eec7571..3bb3f95 100644 --- a/internal/web/handlers/handlers_status_test.go +++ b/internal/web/handlers/handlers_status_test.go @@ -6,6 +6,7 @@ import ( "github.com/mixeme/selfpost/internal/dnscheck" "github.com/mixeme/selfpost/internal/health" + "github.com/mixeme/selfpost/internal/web/view" ) // checksFixture is one reading of every server check, shaped as healthChecks @@ -127,3 +128,44 @@ func TestProcessRowOfAnIdleOneShotSaysOnDemand(t *testing.T) { t.Errorf("stopped daemon = %+v", daemon) } } + +// Health's Hostname card shows Postfix's greeting on a row of its own, and +// warns when it names another host; the certificate's names carry whether +// they are the hostname's. +func TestHealthGreetingAndCertificateName(t *testing.T) { + c := checksFixture() + c.Banner = "mail.example.org ESMTP" + matches := true + c.Cert.Names, c.Cert.MatchesHost = []string{"mail.example.org"}, &matches + page := healthPage(view.Meta{}, c, "") + var greeting *view.Fact + for i := range page.HostFacts { + if page.HostFacts[i].Label == "Greeting" { + greeting = &page.HostFacts[i] + } + } + if greeting == nil || greeting.Value[0].Text != "220 mail.example.org ESMTP" || !greeting.Wide || greeting.Note != nil { + t.Fatalf("greeting = %+v", greeting) + } + names := func(p *view.Health) *view.Tag { + for _, f := range p.CertFacts { + if f.Label == "Names" { + return f.LabelTag + } + } + return nil + } + if tag := names(page); tag == nil || tag.Label != "matches" { + t.Errorf("names = %+v", page.CertFacts) + } + + c.Banner = "3f2a9c1d0e7b ESMTP" + matches = false + page = healthPage(view.Meta{}, c, "") + if end := page.Hostname.End; len(end) == 0 || end[0].Tag == nil || end[0].Tag.Status != "warn" { + t.Errorf("a greeting with a container id: card %+v, want warn", page.Hostname.End) + } + if tag := names(page); tag == nil || tag.Status != "fail" || tag.Label != "not for the hostname" { + t.Errorf("a certificate for another name: %+v", tag) + } +} diff --git a/internal/web/view/components.go b/internal/web/view/components.go index 7f3aa65..9279c79 100644 --- a/internal/web/view/components.go +++ b/internal/web/view/components.go @@ -452,6 +452,12 @@ type Fact struct { Small bool Big bool Note Text + // Wide takes the whole row of the grid (sp-wide): a line a server wrote, + // read as one - Postfix's greeting. + Wide bool + // LabelTag is a verdict on the value, beside the label: the + // certificate's names against the hostname. + LabelTag *Tag } // SideMenu is the input of side_menu, the left column of a detail page: groups diff --git a/internal/web/view/health.go b/internal/web/view/health.go index b9dac24..48fb46f 100644 --- a/internal/web/view/health.go +++ b/internal/web/view/health.go @@ -2,6 +2,7 @@ package view import ( "strconv" + "strings" "time" ) @@ -53,6 +54,38 @@ type Health struct { Configuration Box Reload Action + // ConfigFacts is the address the panel sees the viewer at + // (WithClientAddress); ConfigProblem says what to do when it is the + // reverse proxy's own. + ConfigFacts []Fact + ConfigProblem Text +} + +// WithClientAddress says where the panel sees the viewer's request from: +// client is the address it counts sign-ins against, peer the connection's, +// fromHeader that client came from X-Forwarded-For of a trusted proxy, and +// ignored that the header was there but peer is no trusted proxy - then +// every visitor has the proxy's address and shares one sign-in limit, which +// nothing else shows (auth.Module.Seen; guide § TRUSTED_PROXY_CIDR). +func (h *Health) WithClientAddress(client, peer string, fromHeader, ignored bool) *Health { + f := Fact{Label: "Your address", Value: Plain(client), Mono: true} + switch { + case fromHeader: + f.Note = Plain("From X-Forwarded-For, which the trusted proxy " + peer + " set") + case ignored: + f.Note = Plain("The proxy's own: its X-Forwarded-For is not read") + mask := "/32" + if strings.Contains(peer, ":") { + mask = "/128" + } + h.ConfigProblem = Rich("Every visitor reaches the panel as ", Mono(peer), + ", the reverse proxy, and they all share one sign-in limit. Set ", Code("TRUSTED_PROXY_CIDR"), + " to ", Code(peer+mask), " - or the proxy's network - and restart the container.") + default: + f.Note = Plain("The connection's own; no proxy in between said otherwise") + } + h.ConfigFacts = []Fact{f} + return h } // SenderCheckRow is one period of the Sender check table. diff --git a/internal/web/view/help.go b/internal/web/view/help.go index f00f2c6..8eececf 100644 --- a/internal/web/view/help.go +++ b/internal/web/view/help.go @@ -145,13 +145,18 @@ var overviewTopics = []HelpTopic{ Rich("Six checks, and the worst of them is the postmark. ", TagOf("ok", ""), " needs nothing; ", TagOf("warn", ""), " still delivers mail but will stop doing so if ignored; ", TagOf("fail", ""), " means mail is being lost or refused now. ", Link("/server/health", "Health"), - " 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."), + " 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. ", + Em("Your address"), ", in the Configuration card, is the address the panel sees your browser at: behind a reverse proxy it must be your own, taken from ", Code("X-Forwarded-For"), + "; when it is the proxy's, every visitor shares one sign-in limit, and Health warns and names the address to put in ", Code("TRUSTED_PROXY_CIDR"), "."), }}, {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. 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."), + Rich("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. It also fails when none of its names is the hostname (", + Code("SELFPOST_HOSTNAME"), ") - the tag beside ", Em("Names"), " says which: a client that verifies the name refuses it however valid it is."), }}, {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."), + 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. ", + Em("Greeting"), " is the line Postfix opens every connection with, ", Code("220"), " and its banner; it must name the hostname too - a receiver compares it with the name it looked up - and a container id or ", + Code("localhost"), " there is a warning."), }}, } diff --git a/internal/web/view/kit.go b/internal/web/view/kit.go index bcc303a..6c88b65 100644 --- a/internal/web/view/kit.go +++ b/internal/web/view/kit.go @@ -145,6 +145,8 @@ func KitPage() *Kit { {Label: "Label", Value: Plain("Value")}, {Label: "Mono value", Value: Plain("4XcB7k2Jm9z1"), Mono: true}, {Label: "sp-big", Value: Plain("5 days"), Big: true, Note: Plain("a supporting line")}, + {Label: "Label with a tag", Value: Plain("a verdict on the value"), LabelTag: &Tag{Status: "ok", Label: "matches"}}, + {Label: "sp-wide", Value: Plain("a value that takes the whole row, a line of a server's own"), Mono: true, Wide: true}, }, Timeline: Box{No: "08", Title: "timeline"}, diff --git a/internal/web/view/server_test.go b/internal/web/view/server_test.go index 8c3ead0..a07ff5c 100644 --- a/internal/web/view/server_test.go +++ b/internal/web/view/server_test.go @@ -331,3 +331,18 @@ func TestUserDeleteSaysNothingOfReportAddresses(t *testing.T) { pageLacks(t, "Delete user", out, `report address`, `will have no`, `The domains`) } } + +// The Configuration card says where the panel sees the viewer from, and what +// to set when it is the reverse proxy's own address. +func TestHealthSaysWhereItSeesTheViewer(t *testing.T) { + out := renderSignedIn(t, "health", NewHealth(admin(), "").WithClientAddress("198.51.100.7", "172.18.0.1", true, false)) + pageHas(t, "Health", out, `
Your address
198.51.100.7
From X-Forwarded-For, which the trusted proxy 172.18.0.1 set
`) + pageLacks(t, "Health", out, `TRUSTED_PROXY_CIDR`) + + out = renderSignedIn(t, "health", NewHealth(admin(), "").WithClientAddress("172.18.0.1", "172.18.0.1", false, true)) + pageHas(t, "Health", out, `The proxy's own: its X-Forwarded-For is not read`, + `

Every visitor reaches the panel as 172.18.0.1, the reverse proxy, and they all share one sign-in limit. Set TRUSTED_PROXY_CIDR to 172.18.0.1/32`) + + out = renderSignedIn(t, "health", NewHealth(admin(), "").WithClientAddress("fd00::1", "fd00::1", false, true)) + pageHas(t, "Health", out, `fd00::1/128`) +} diff --git a/internal/web/view/static/panel.css b/internal/web/view/static/panel.css index b4ade8d..8d22fac 100644 --- a/internal/web/view/static/panel.css +++ b/internal/web/view/static/panel.css @@ -278,6 +278,11 @@ code { background: var(--sp-well); color: var(--sp-ink); border-radius: 3px; pad .sp-facts { display: grid; grid-template-columns: repeat(auto-fit, minmax(11rem, 1fr)); gap: 1rem 1.5rem; margin: 0; } .sp-facts dt { font-family: var(--sp-mono); font-size: var(--sp-fs-xs); letter-spacing: .08em; text-transform: uppercase; color: var(--sp-muted); } .sp-facts dd { margin: 0; overflow-wrap: anywhere; } +/* A label may carry a tag, the verdict on its value (the certificate's + names against the hostname); it keeps the tag's own type. */ +.sp-facts dt .tag { margin-left: .4rem; text-transform: none; letter-spacing: 0; vertical-align: 1px; } +/* A fact that takes the whole row: a line a server wrote, read as one. */ +.sp-facts .sp-wide { grid-column: 1 / -1; } .sp-facts dd.sp-big { font-size: 1.5rem; font-weight: 600; line-height: 1.2; } .sp-side { position: sticky; top: 1.5rem; } .menu-label { font-family: var(--sp-mono); letter-spacing: .1em; } diff --git a/internal/web/view/templates/components.html b/internal/web/view/templates/components.html index 88a43df..ee7abe3 100644 --- a/internal/web/view/templates/components.html +++ b/internal/web/view/templates/components.html @@ -169,7 +169,7 @@ {{/* facts is a label / value grid (input: []Fact); Big is for a headline number, with its supporting line under it. */}} {{define "facts"}}

{{range .}} -
{{.Label}}
{{template "inline" .Value}}{{if .Note}}
{{template "inline" .Note}}
{{end}}
{{end}} +
{{.Label}}{{with .LabelTag}} {{template "tag" .}}{{end}}
{{template "inline" .Value}}{{if .Note}}
{{template "inline" .Note}}
{{end}}{{end}}
{{end}} {{/* side_menu is the Bulma menu of a detail page (input: SideMenu): icons, diff --git a/internal/web/view/templates/health.html b/internal/web/view/templates/health.html index c15d966..79692b7 100644 --- a/internal/web/view/templates/health.html +++ b/internal/web/view/templates/health.html @@ -15,7 +15,9 @@ {{template "health_mail" .}} {{template "box_open" .Configuration}} {{template "body_open"}} -

Regenerates the Postfix configuration and the spam filter's lists from the database, removes the keys of deleted domains, and reloads Postfix. Use it after editing the files by hand, after restoring a backup, or when the running configuration looks out of step with the domain and application lists. It does not touch the queue or the TLS certificate.

+ {{if .ConfigFacts}}
{{template "facts" .ConfigFacts}}
+ {{end}}{{if .ConfigProblem}}

{{template "inline" .ConfigProblem}}

+ {{end}}

Regenerates the Postfix configuration and the spam filter's lists from the database, removes the keys of deleted domains, and reloads Postfix. Use it after editing the files by hand, after restoring a backup, or when the running configuration looks out of step with the domain and application lists. It does not touch the queue or the TLS certificate.

{{template "action" .Reload}} {{template "body_close"}} {{template "box_close"}} diff --git a/internal/web/web.go b/internal/web/web.go index 323cffb..eab655c 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -66,6 +66,9 @@ type Config struct { TLSCertFile string TLSServedCertFile string TLSReloadInterval time.Duration + // SMTPBanner is Postfix's smtpd_banner as postconf expands it, "" when + // it could not be read (health.SMTPBanner). + SMTPBanner string // SignSocket and SignLocalSocket are the DKIM signer's two milter // sockets, applications' mail and the panel's own; JournalSocket is the // journal-milter's. The status page dials them: the signer's are required @@ -146,6 +149,7 @@ func New(st *store.Store, domains *domain.Service, apps *app.Service, inboundSvc Version: cfg.Version, TLSCertFile: cfg.TLSCertFile, TLSServedCertFile: cfg.TLSServedCertFile, + SMTPBanner: cfg.SMTPBanner, TLSReloadInterval: cfg.TLSReloadInterval, SignSocket: cfg.SignSocket, SignLocalSocket: cfg.SignLocalSocket, diff --git a/test/e2e/health_served_test.go b/test/e2e/health_served_test.go index f21f87c..dc5690d 100644 --- a/test/e2e/health_served_test.go +++ b/test/e2e/health_served_test.go @@ -148,13 +148,26 @@ func writeCertWithoutItsKey(certPath string, notAfter time.Time) error { // panel sent until 2.5.2 and which the origin check refuses (assumptions // D-B3). The client here is not a browser: it sends what one would. func testFormPostWithTheBrowsersOrigin(t *testing.T, sc *scenario) { - resp, _, err := sc.panel.get("/server/health") + resp, page, err := sc.panel.get("/server/health") if err != nil { t.Fatal(err) } if got := resp.Header.Get("Referrer-Policy"); got != "same-origin" { t.Fatalf("Referrer-Policy %q, want same-origin", got) } + // The setup checks of health-setup-checks, on a stand that passes them: + // the stand's certificate is for its hostname, Postfix greets with it + // (postconf -x smtpd_banner, read at the panel's start), and the client + // reaches the panel directly, with no proxy in between. + for _, want := range []string{ + `
Names matches
`, + `
Greeting
220 ` + selfpostHostname + ` ESMTP
`, + `
Your address
`, `The connection's own; no proxy in between said otherwise`, + } { + if !strings.Contains(page, want) { + t.Fatalf("Server › Health lacks %q:\n%.3000s", want, page) + } + } post := func(origin string) int { t.Helper() req, err := http.NewRequest(http.MethodPost, panelBaseURL+"/server/health/recheck", strings.NewReader(""))