From a327962008a0e0732c46c995f944b232362bc93e Mon Sep 17 00:00:00 2001 From: Mix Date: Sat, 17 Oct 2026 22:17:27 +0300 Subject: [PATCH] web: /healthz is answered only to local probes (audit I6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security audit 2026-10-17, finding I6 (low), stage 3. /healthz is registered outside auth and every proxy fragment passed it through: the live server answered "ok" to the internet, and "unhealthy: postfix" would have been a free tell. Now handleHealth answers only a request with no X-Forwarded-For (nor Forwarded) from loopback or a private address: the image's HEALTHCHECK (127.0.0.1), the host through the loopback-published port (the Docker gateway), a monitor on the Docker network. Every supported proxy appends X-Forwarded-For, so through it the endpoint is 404. The plan named "trusted-proxy peers"; that would have let the internet in again through the trusted proxy, hence this shape. The e2e's readiness probe is the host's, and stays answered. Test: TestHealthzAnswersOnlyLocalProbes. Docs: guide § Container health, architecture.md; CHANGELOG; plan. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++++ docs/architecture.md | 2 +- docs/guide.md | 11 ++++++---- docs/security-audit-2026-10-17.md | 7 +++++-- internal/web/healthz_test.go | 34 +++++++++++++++++++++++++++++++ internal/web/web.go | 26 ++++++++++++++++++++++- 6 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 internal/web/healthz_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d6d451..ab1b9aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,10 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version username could keep a live reset link arriving in that mailbox five times an hour. After five in a day the owner is told once that further requests are ignored for 24 hours. +- **`/healthz` is not answered through the reverse proxy** (audit I6): + only a request with no `X-Forwarded-For` from loopback or a private + address gets the liveness answer - the container's own check, the host, + a monitor next to it; the internet gets `404`. - **The TLS private key no longer has to be readable by Postfix** (audit I1): the guide gave it to group 10998 for the panel's backup, and `postfix` is in that group, so every Postfix daemon - an smtpd talking to diff --git a/docs/architecture.md b/docs/architecture.md index b77d6f8..5aabde2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -63,7 +63,7 @@ The panel runs `postfix-reload` through supervisord's XML-RPC interface and waits for its exit status (`internal/supervisor`, `RunOnce`); the program rests EXITED after a run. -**Liveness:** `GET /healthz` (unauthenticated) returns 200 when panel - whose +**Liveness:** `GET /healthz` (unauthenticated; answered only to a request with no `X-Forwarded-For` from loopback or a private address, else 404) returns 200 when panel - whose roles include the DKIM signer - and postfix are RUNNING (`internal/health`, `Liveness`); Docker `HEALTHCHECK` uses the same probe (its 90-second start period covers postfix's STARTING). diff --git a/docs/guide.md b/docs/guide.md index 5b2b287..e4d65e8 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -819,15 +819,18 @@ again whole and loses the keys of domains added under 2.8.x. ### Container health The image declares a Docker `HEALTHCHECK` that probes `GET /healthz` on port -8080 (unauthenticated). It returns `200 ok` when the panel - whose roles +8080 (unauthenticated, and answered only to a request that did not come +through a reverse proxy - no `X-Forwarded-For` - from loopback or a private +address; anyone else gets `404`). It returns `200 ok` when the panel - whose roles include the DKIM signer - and Postfix are both `RUNNING` under supervisord; otherwise `503 unhealthy`. This catches a dead mail path that would still leave the HTTP server up, but it does **not** verify TLS certificates, DNS records, or end-to-end delivery — use the authenticated [Overview and Health](#overview-and-health) pages for -that. External monitoring -can use the same endpoint through the reverse proxy if you expose it, or poll -`docker inspect` health state on the host. +that. External monitoring runs on the host - `curl -fsS +http://127.0.0.1:8080/healthz` - or polls `docker inspect` health state; +through the reverse proxy the endpoint is not found, so its answer tells a +stranger nothing. ### Server-level DNS (PTR/rDNS) diff --git a/docs/security-audit-2026-10-17.md b/docs/security-audit-2026-10-17.md index b1078c8..4ceb374 100644 --- a/docs/security-audit-2026-10-17.md +++ b/docs/security-audit-2026-10-17.md @@ -790,9 +790,12 @@ the argument. - [x] **A3.** After five unsolicited reset links in a day, one mail saying further requests are ignored for 24 h, then silence. — **Opus**, 1.5 h. *Done 2026-10-17:* TestResetLinksPauseForADayAfterFive. -- [ ] **I6.** `/healthz` answered only to loopback and trusted-proxy peers; +- [x] **I6.** `/healthz` answered only to loopback and trusted-proxy peers; the guide's "Container health" says the proxy may expose it on purpose - with the fragment line. — **Sonnet**, 0.5 h. + with the fragment line. — **Sonnet**, 0.5 h. *Done 2026-10-17,* in another + shape: answered to a request with no `X-Forwarded-For` from loopback or a + private address - a trusted proxy's peer would have let the internet in + again through that proxy; TestHealthzAnswersOnlyLocalProbes. ### Stage 4 — resources and imports (~5 h) diff --git a/internal/web/healthz_test.go b/internal/web/healthz_test.go new file mode 100644 index 0000000..0d87bd7 --- /dev/null +++ b/internal/web/healthz_test.go @@ -0,0 +1,34 @@ +package web + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// /healthz answers a probe that did not come through a proxy, from loopback +// or a private address, and is not found for anyone else (security audit +// 2026-10-17, I6). Liveness itself is not under test here: a probe that is +// answered gets 200 or 503, never 404. +func TestHealthzAnswersOnlyLocalProbes(t *testing.T) { + for _, c := range []struct { + name, remote, xff string + answered bool + }{ + {"the container's own probe", "127.0.0.1:40000", "", true}, + {"the host through the published port", "172.18.0.1:40000", "", true}, + {"through the reverse proxy", "172.18.0.1:40000", "198.51.100.7", false}, + {"a public address", "198.51.100.7:40000", "", false}, + } { + r := httptest.NewRequest(http.MethodGet, "/healthz", nil) + r.RemoteAddr = c.remote + if c.xff != "" { + r.Header.Set("X-Forwarded-For", c.xff) + } + rec := httptest.NewRecorder() + handleHealth(rec, r) + if got := rec.Code != http.StatusNotFound; got != c.answered { + t.Errorf("%s: %d, answered %v, want %v", c.name, rec.Code, got, c.answered) + } + } +} diff --git a/internal/web/web.go b/internal/web/web.go index 0b7fd1d..026d396 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -356,7 +356,17 @@ func redirectHome(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/overview", http.StatusSeeOther) } -func handleHealth(w http.ResponseWriter, _ *http.Request) { +// handleHealth answers the liveness probe - but only to a client that did not +// come through a reverse proxy: one with no X-Forwarded-For, from loopback or +// a private address (the container's own HEALTHCHECK, the host, a monitor on +// the Docker network). Every supported proxy appends X-Forwarded-For, and the +// panel's port is published on loopback only, so the internet gets a 404: +// "unhealthy: postfix" was a free tell (security audit 2026-10-17, I6). +func handleHealth(w http.ResponseWriter, r *http.Request) { + if !localProbe(r) { + http.NotFound(w, r) + return + } w.Header().Set("Content-Type", "text/plain; charset=utf-8") if err := health.Liveness(); err != nil { http.Error(w, "unhealthy\n", http.StatusServiceUnavailable) @@ -366,6 +376,20 @@ func handleHealth(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte("ok\n")) } +// localProbe is whether a request comes straight from loopback or a private +// address, not through a proxy. +func localProbe(r *http.Request) bool { + if r.Header.Get("X-Forwarded-For") != "" || r.Header.Get("Forwarded") != "" { + return false + } + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + host = r.RemoteAddr + } + ip := net.ParseIP(host) + return ip != nil && (ip.IsLoopback() || ip.IsPrivate()) +} + func handleLicense(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet && r.Method != http.MethodHead { http.Error(w, "method not allowed", http.StatusMethodNotAllowed)