From 503939c6cfe9b14fc7c7639c89dc4d7b2c468f16 Mon Sep 17 00:00:00 2001 From: Mix Date: Fri, 16 Oct 2026 23:16:27 +0300 Subject: [PATCH] origin-strict: a change with neither Sec-Fetch-Site nor Origin is refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agreed with the owner on 2026-10-13 (in place of csrf-tokens), taken up on 2026-10-16 ("Да"). originAllowed (internal/web/security.go) returns false in the "neither header" branch: such a POST is answered 403 and logged like a cross-origin one. It was let through until now as an accepted risk, which left a browser that sends neither open to a request forged by any site. A browser that follows Fetch § 3.2 never sends a POST without Origin; one too old to (Firefox before 70) can no longer save anything. What posts to the panel without a browser: the e2e suite's client, which now sends Origin on every form (panelClient.postForm); cmd/panel's tests already did. Nothing else posts to it (the CLI runs in the container). Tests: security_test's case of no headers is refused; e2e posts with neither header and gets 403 (testFormPostWithTheBrowsersOrigin). go vet + go test (26 packages) and make e2e on the test server for this tree. Docs: security.md - the accepted risk struck, the ADR's decision and trade-off say refused, the origin-check bullet; architecture § Web, guide § the proxy; CHANGELOG [Unreleased] § Security; roadmap row done. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 10 ++++++++++ docs/architecture.md | 3 ++- docs/guide.md | 3 +++ docs/roadmap.md | 8 +++++++- docs/security.md | 24 +++++++++--------------- internal/web/security.go | 16 ++++++++-------- internal/web/security_test.go | 2 +- test/e2e/health_served_test.go | 9 ++++++++- test/e2e/panel_client.go | 11 ++++++++++- 9 files changed, 58 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dba61d..f95cb37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Security + +- **A change with no word of where it came from is refused** + (origin-strict): a `POST` that carries neither `Sec-Fetch-Site` nor + `Origin` is answered 403, as a cross-origin one is. It was let through + before, as an accepted risk, which left a browser that sends neither open + to a request forged by any site. Every current browser sends `Origin` with + a form; a browser too old to (Firefox before 70) can no longer save + anything in the panel. + ## [2.10.3] - 2026-10-16 The backup asks for your password. No migration; schema 12. diff --git a/docs/architecture.md b/docs/architecture.md index 59f77dc..8c4c78e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -994,7 +994,8 @@ send it (an older one, or a current one over plain HTTP at a name other than `localhost`), `Origin` must name the panel's host - which a form of the panel carries under its `Referrer-Policy: same-origin`, the page's own origin (under `no-referrer` it would be `null` and refused); a request with neither header -is let through (an accepted risk). A refusal logs the `Sec-Fetch-Site`, +is refused too (origin-strict, `2.10.4`; it was let through before, as an +accepted risk). A refusal logs the `Sec-Fetch-Site`, `Origin` and `Host` it compared. There are no CSRF tokens — see [security.md](security.md). diff --git a/docs/guide.md b/docs/guide.md index 10c80da..3bb651a 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -295,6 +295,9 @@ browser that does not send `Sec-Fetch-Site` (Safari before 16.4, or any browser on a panel served over plain HTTP at a name): there the panel compares `Origin` with `Host` and refuses every change as cross-origin - the log says so explicitly, printing the `Sec-Fetch-Site`, `Origin` and `Host` it compared. +A request that carries neither header - a browser too old to send `Origin`, +or a script posting to the panel without one - is refused the same way +since 2.10.4. In every case the proxy terminates HTTPS for the panel; the resulting certificate must end up under `./certs` as `fullchain.pem` and `privkey.pem` — diff --git a/docs/roadmap.md b/docs/roadmap.md index 52494fb..7abfa53 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -32,7 +32,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.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) | — | — | | 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) | agreed (2026-10-13) | — | — | +| origin-strict | Refuse a state-changing request that carries neither `Sec-Fetch-Site` nor `Origin` (was csrf-tokens) | done 2026-10-16, unreleased | — | — | | backup-reauth | Ask for the account password before a backup or a domain export is downloaded | shipped `2.10.3` | — | — | | review-2026-08-followups | Remaining findings of the 2026-08-19 code review | closed 2026-10-14 (rows placed in [audit-2026-10-12](audit-2026-10-12.md)) | — | — | | panel-notifications | E-mail notifications about important events | shipped `2.3.0` | 10/10 | [plans/panel-notifications.md](plans/panel-notifications.md) | @@ -224,6 +224,12 @@ panel without a browser (the e2e stand, scripts) and make it send `Origin`. it, the e2e stand passes, the accepted risk about header-less requests is struck from [security.md](security.md) and its ADR states the strict rule. +**Done (2026-10-16):** `originAllowed` returns false in the "neither +header" branch; the unit test's case is refused, e2e posts with neither +header and gets 403, and the e2e suite's client sends `Origin` on every +form it posts. The accepted risk is struck from security.md and the ADR's +decision says refused; architecture and the guide say so. + **Dependencies / risks:** a browser that sends neither header can no longer save anything in the panel - accepted, the panel's users pick their own browsers. Tokens stay a recorded alternative, not a task: the conditions under diff --git a/docs/security.md b/docs/security.md index 18f3b61..11ae358 100644 --- a/docs/security.md +++ b/docs/security.md @@ -204,6 +204,8 @@ assigned outbound domains and inbound domains **separately**, each list with host (the panel's `Referrer-Policy` is `same-origin`, so a browser puts the panel's own origin on a form post of the panel); the session cookie is `SameSite` as well. +- A request with neither header is refused as well (origin-strict, + `2.10.4`). - There are **no CSRF tokens**. The decision, its price and the conditions for revisiting it are in the accepted risks and the ADR below. @@ -512,17 +514,6 @@ operator can act on it. An accepted risk is a decision with a condition for revisiting it, not a deferred item from the roadmap. -- **A `POST` with neither `Sec-Fetch-Site` nor `Origin` is allowed through.** - A client that sends neither — a genuinely old browser, or a webview with a - frozen engine — stays vulnerable to CSRF from any site. Accepted - deliberately: every panel user (role `global` or `domain`) is an operator who - picks their own browser, not an untrusted party the panel needs to defend - against, and a strict mode would not "protect" such a client, it would - simply break the panel in it. Tightening is one line in `originAllowed` - ([internal/web/security.go](../internal/web/security.go)): return `false` - instead of `true` in the "neither header present" branch. That tightening - is agreed (owner, 2026-10-13) as - [origin-strict](roadmap.md#origin-strict); this bullet goes when it ships. - **Session-bound CSRF tokens are not implemented.** The origin check closes the neighbouring-subdomain case but depends on browser behaviour; a token does not. The price is a hidden field in roughly two dozen forms. The trigger to @@ -844,7 +835,8 @@ through the browser. **Decision.** `originAllowed` in [internal/web/security.go](../internal/web/security.go) checks `Sec-Fetch-Site` (when the browser sends it) or `Origin` (fallback) against the panel's host; a -request carrying neither header is **allowed through** rather than rejected. +request carrying neither header is **refused** (403), since `2.10.4` +(origin-strict; until then it was let through as an accepted risk). There are no session-bound tokens embedded in forms. The check applies the same way regardless of the requesting user's role. @@ -864,9 +856,11 @@ and sends the request itself. XSS is handled by `html/template` auto-escaping and CSP, so that is a separate line of defence, not a CSRF token. **Trade-off.** A client that sends neither `Sec-Fetch-Site` nor `Origin` (a -genuinely old browser, or a webview with a frozen engine) stays vulnerable — see -"Accepted risks" above. This is a deliberate choice not to break the panel in -such a client, at the price of a narrow residual surface. +genuinely old browser - Firefox before 70 sent no Origin with a same-origin +form - or a webview with a frozen engine) can no longer save anything in the +panel: refused rather than left open to a request forged by any site. Until +`2.10.4` the choice was the other way round, as an accepted risk. A script +that posts to the panel sends `Origin` itself (the e2e suite's client does). **Revisit if:** a requirement appears for protection that does not depend on browser behaviour, or `domain` accounts stop being trusted operators (for diff --git a/internal/web/security.go b/internal/web/security.go index bb8fc9a..977fa26 100644 --- a/internal/web/security.go +++ b/internal/web/security.go @@ -146,14 +146,14 @@ func originAllowed(r *http.Request) bool { // https. origin := r.Header.Get("Origin") if origin == "" { - // Neither header. A client this old cannot be checked at all; it is - // let through as the risk consciously accepted (single-admin panel, - // the administrator picks the browser). Turning this return into - // false is the whole of the stricter policy. By Fetch § 3.2 a browser - // that follows it never sends a POST without Origin; Firefox did for a - // same-origin form before 70 (MDN browser-compat-data 6.0.0, - // http.headers.Origin). - return true + // Neither header: nothing says where the request came from, so it is + // refused (origin-strict, owner 2026-10-13; until 2.10.4 it was let + // through as an accepted risk). By Fetch § 3.2 a browser that follows + // it never sends a POST without Origin; Firefox did for a same-origin + // form before 70 (MDN browser-compat-data 6.0.0, http.headers.Origin), + // and such a browser can no longer save anything here. A script that + // posts to the panel sends Origin itself (test/e2e, panelClient). + return false } u, err := url.Parse(origin) if err != nil || u.Host == "" { diff --git a/internal/web/security_test.go b/internal/web/security_test.go index 53d9c68..f8b7a8f 100644 --- a/internal/web/security_test.go +++ b/internal/web/security_test.go @@ -42,7 +42,7 @@ func TestOriginCheck(t *testing.T) { req *http.Request wantRejected bool }{ - {"no headers at all is let through (accepted risk)", post("", ""), false}, + {"no headers at all is refused (origin-strict)", post("", ""), true}, {"same-origin", post("same-origin", "https://panel.example.com"), false}, {"same-site, i.e. a neighbouring subdomain", post("same-site", ""), true}, {"cross-site", post("cross-site", "https://evil.example.net"), true}, diff --git a/test/e2e/health_served_test.go b/test/e2e/health_served_test.go index 5420ced..f21f87c 100644 --- a/test/e2e/health_served_test.go +++ b/test/e2e/health_served_test.go @@ -162,7 +162,9 @@ func testFormPostWithTheBrowsersOrigin(t *testing.T, sc *scenario) { t.Fatal(err) } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", origin) + if origin != "" { + req.Header.Set("Origin", origin) + } resp, err := sc.panel.http.Do(req) if err != nil { t.Fatal(err) @@ -176,4 +178,9 @@ func testFormPostWithTheBrowsersOrigin(t *testing.T, sc *scenario) { if code := post("null"); code != http.StatusForbidden { t.Fatalf("POST with Origin: null: status %d, want 403", code) } + // Neither Sec-Fetch-Site nor Origin: nothing says where it came from, and + // it is refused (origin-strict, 2.10.4; allowed before as an accepted risk). + if code := post(""); code != http.StatusForbidden { + t.Fatalf("POST with neither header: status %d, want 403", code) + } } diff --git a/test/e2e/panel_client.go b/test/e2e/panel_client.go index e0747ff..b107c92 100644 --- a/test/e2e/panel_client.go +++ b/test/e2e/panel_client.go @@ -85,8 +85,17 @@ func readSetupToken(s *stack) (string, error) { return strings.TrimPrefix(u.Path, "/setup/"), nil } +// postForm posts a form as the panel's own page would: with the Origin a +// browser sends, which the panel requires of every state-changing request +// (origin-strict; internal/web/security.go, originAllowed). func (c *panelClient) postForm(path string, form url.Values) (*http.Response, string, error) { - resp, err := c.http.PostForm(panelBaseURL+path, form) + req, err := http.NewRequest(http.MethodPost, panelBaseURL+path, strings.NewReader(form.Encode())) + if err != nil { + return nil, "", err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Origin", panelBaseURL) + resp, err := c.http.Do(req) if err != nil { return nil, "", err }