From f0927fb0f02b9d6a1142fd01bf67f075ebfeee96 Mon Sep 17 00:00:00 2001 From: Claus Lohmar Date: Thu, 15 Oct 2026 05:40:36 +0000 Subject: [PATCH] feat(server): gate /setup by the direct tunnel and trust the proxy scheme MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements D-025 (revised 2026-10-15) and D-043. Setup gate (D-025): - Remove the bind refusal (CheckSetupBind/ErrNonLoopbackSetup/server.IsLoopback); Serve binds any address, so the app can listen on 0.0.0.0:8080 behind Zoraxy. - The gate is now per request and strict (httpx.SetupRequestAllowed): a loopback TCP peer AND no X-Forwarded-For — the direct SSH tunnel. Anything else is 404 before dispatch; an unparseable peer fails closed (AC-7/AC-10). The raw XFF is read before ClientIP SETS it (A6). Proxy scheme (D-043): - httpx.RequestIsHTTPS resolves the scheme per request: direct TLS, or a trusted proxy's X-Forwarded-Proto. The door carries it on the request context (httpx.WithSecure) so the session cookie, its __Host- prefix, the pre-session and ceremony cookies, and HSTS all agree (AC-13). - Over plain http (the tunnel) cookies stay bare and non-Secure, which fixes the bootstrap from an https:// config.example.yaml; Config.IsSecure() is removed as now dead. Tests: rewrite the setup-gate tests for AC-7/AC-10, add AC-13, and unit-test SetupRequestAllowed / RequestIsHTTPS. --- PROJECT_PLAN.md | 14 +- cmd/myday/main.go | 16 +- cmd/myday/main_test.go | 2 +- internal/core/auth/flow/flow.go | 25 +-- internal/core/config/config.go | 13 +- internal/core/config/config_test.go | 6 - internal/core/httpx/cookie.go | 4 + internal/core/httpx/httpx_test.go | 84 ++++++++++ internal/core/httpx/loopback.go | 31 +++- internal/core/httpx/presession.go | 12 +- internal/core/httpx/presession_test.go | 2 +- internal/core/httpx/scheme.go | 47 ++++++ internal/core/recovery/flow_test.go | 2 +- internal/core/server/server.go | 84 ++++------ internal/core/server/server_test.go | 155 +++++++++++++----- internal/core/server/setup_wizard_test.go | 39 +++-- internal/core/server/spec007_recovery_test.go | 6 +- internal/core/setup/setup.go | 8 +- 18 files changed, 385 insertions(+), 165 deletions(-) create mode 100644 internal/core/httpx/scheme.go diff --git a/PROJECT_PLAN.md b/PROJECT_PLAN.md index a9b0ff0..246c0a2 100644 --- a/PROJECT_PLAN.md +++ b/PROJECT_PLAN.md @@ -287,7 +287,7 @@ the highest role, the **root of the chain**. When the `users` table is empty, th ### 5.1 First run — the `/setup` wizard ``` -fresh deploy → config.setup: true → the app serves ONLY /setup (loopback-only, D-025) +fresh deploy → config.setup: true → the app serves ONLY /setup (the tunnel-only gate, D-025) Step 1 the domain / base URL Step 2 the top admin — name, email, Passkey + TOTP enrollment (user_id = 1, role: admin) Step 3 mail / SMTP · locale · timezone @@ -296,7 +296,8 @@ fresh deploy → config.setup: true → the app serves ONLY /setup (loopback-onl ``` - `setup` is a **reserved `PARAM1`** (always — so no user can ever be named `setup`). -- In setup mode, `/setup` is the **only served route**, and it is **loopback-only** (D-025). +- In setup mode, `/setup` is the **only served route**, reachable **only over the SSH tunnel** (a + loopback TCP peer with **no** `X-Forwarded-For` — D-025). - **It supersedes `{bootstrap_guid}`** — the wizard *is* the bootstrap; the operator runs it once over an **SSH tunnel** to `127.0.0.1:8080`, then exposes the instance. - *README: do NOT expose MyDay publicly before the first admin exists.* @@ -457,8 +458,8 @@ the top admin (first user) | **D-022** | 2026-10-13 | **Bootstrap:** `users` empty → **setup mode**; the top admin is created at a **gated** path; setup ends at the first user. | **Gate it** (loopback and/or setup token) or "first user" = "first visitor". | | **D-023** | 2026-10-13 | **Admin recovery = `reset_admin` config toggle** — rotate the top admin's identity to `pending_enrollment`, log the new GUID; audited; auto-clears. | The config is the offline-held root of trust. | | **D-024** | 2026-10-13 | **Roles (KISS): `admin > director > manager > team`.** You may create only roles **below** your own; the new user's manager defaults to the creator. | Answers "who may create a user?" | -| **D-025** | 2026-10-13 | **The bootstrap is loopback-only, enforced.** While in **setup mode** the app **refuses to serve a non-loopback address**; the operator reaches it over an **SSH tunnel** to `127.0.0.1:8080`. Optional `{bootstrap_guid}` = defence-in-depth. | Closes first-visitor-wins **in code**, not only in the README. | -| **D-026** | 2026-10-13 | **A `/setup` web wizard is the bootstrap** (a reserved `PARAM1`). It configures the **domain**, creates the **top admin** (user 1) with Passkey + TOTP, and mail/locale — **setup-mode only · loopback-only** — then the app runs productive. `config.setup` forces the mode. | Supersedes `{bootstrap_guid}`; configuration through a UI instead of hand-editing config. | +| **D-025** *(rev. 2026-10-15)* | 2026-10-13 | **The bootstrap is loopback-only — enforced PER REQUEST, not at bind.** In **setup mode** the app serves only a request whose **TCP peer is loopback *and* which carries no `X-Forwarded-For`** — the **direct SSH tunnel**; anything else → 404. The app **may bind `0.0.0.0`** (D-043). | **Revised**: the old bind refusal is incompatible with the Zoraxy deployment (D-043). The per-request gate still closes first-visitor-wins **in code**. | +| **D-026** | 2026-10-13 | **A `/setup` web wizard is the bootstrap** (a reserved `PARAM1`). It configures the **domain**, creates the **top admin** (user 1) with Passkey + TOTP, and mail/locale — **setup-mode only · the tunnel-only gate (D-025)** — then the app runs productive. `config.setup` forces the mode. | Supersedes `{bootstrap_guid}`; configuration through a UI instead of hand-editing config. | | **D-027** | 2026-10-13 | **Modular architecture.** The **core is `MyDay.auth`** (identity · session · roles · the door · the **module registry**); features are **modules** mounted at **`/{GUID}/{module}/{function}/{param}`**. A module depends only on core interfaces and **owns its own tables**. | The natural generalisation of the `PARAM2` switch — the same URL shape now carries a plugin model. Makes `expense`/IDM additive, not invasive. | | **D-028** | 2026-10-13 | **Drop-in modules, discovered at BUILD time.** A module is a directory (`internal/modules//`) that **self-registers** via `init()`; a **generated aggregator** blank-imports every module dir, so the directory is the source of truth. **No runtime plugins.** | The **Caddy** pattern. Drop a dir → `go generate` → rebuild. Deterministic order; log the loaded set at startup. | | **D-029** | 2026-10-13 | **User management is CORE, not a module.** Identities, roles, and the chain live in `MyDay.auth`. | "Management" decomposes into **core (users)** + `requests` (workflow) + `hr` (records). | @@ -475,6 +476,7 @@ the top admin (first user) | **D-040** | 2026-10-13 | **Agent-workflow variables live in `.opencode/git.env`** (gitignored), with a committed `.opencode/git.env.example`. Git-native settings (identity · remote · branch · pull) live in repo-local `.git/config`. | Two-tier config (SPEC-001 A-04): git owns what git owns; automation knobs stay out of git. | | **D-041** | 2026-10-14 | **The setup wizard enrolls TOTP only.** Any **TOTP login with no passkey** is **nudged** to enroll one, with *not now* / ***don't ask again*** (`users.passkey_opt_out`). The nudge is **suppressed while the origin is loopback** (or in setup mode). | Resolves **O-13**: a passkey enrolled over loopback binds RP ID `127.0.0.1` and is unusable at the public domain. Passkeys are encouraged, not forced — TOTP stays a valid factor (D-005). | | **D-042** | 2026-10-14 | **TOTP-only enrollment is specific to the bootstrap admin (`/setup`).** **Invited users enroll both factors** (Passkey + TOTP) at the real origin; the passkey **nudge** remains the safety net for any identity without a passkey. | Resolves **O-14** (owner, Option A): keep the **strongest default** where the origin permits it. | +| **D-043** | 2026-10-15 | **Deployment: behind Zoraxy.** The app listens on **`0.0.0.0:8080` over HTTP**; **TLS terminates at Zoraxy**; the public origin is **`https://myday.sechpoint.app`** (`base_url`). `trusted_proxies` = Zoraxy. The app **honours `X-Forwarded-Proto`** from the proxy to set `Secure` / `__Host-` cookies and HSTS. | The app is **not** the TLS endpoint, so it must honour the proxy's scheme header. **Firewall `:8080` to Zoraxy only.** | --- @@ -521,7 +523,7 @@ the top admin (first user) | A10 | GUID **tombstoned, never recycled** | HIGH | designed (D-008) | | A11 | Every auth event audited (bootstrap, enroll, login, recovery request/approve/deny, revoke, `reset_admin`) | HIGH | open (L4c: `Sessions.GC` · `UpdateWebAuthnSignCount` unaudited) | | A12 | Changing the **email on record** is a privileged, audited operation | HIGH | open | -| A13 | **Setup window hardened** — `/setup` served **only** in setup mode; **loopback-only, enforced per request (H2)**; the `setup` module **pinned at the door (M1c/L3s)**; rate-limited; `/setup` → 404 once productive | HIGH | **enforced** | +| A13 | **Setup window hardened** — `/setup` served **only** in setup mode; **tunnel-only, enforced per request** (a loopback TCP peer **with no `X-Forwarded-For`**; D-025 revised); the `setup` module **pinned at the door (M1c/L3s)**; rate-limited; `/setup` → 404 once productive | HIGH | **enforced** | | A14 | **Top admin protected** — cannot be deleted; `reset_admin` rotations audited; startup warning while setup mode is reachable | HIGH | open | | A15 | **Pre-session CSRF** — the anonymous forms (`/setup` · `/access` · enroll/login) need a signed **double-submit** cookie (SPEC-005/006/007) | HIGH | **enforced** — signing keys generated at startup in setup mode (M1s) | @@ -640,3 +642,5 @@ shell/layout lives in `core/web`; a module's views live with the module. | 2026-10-14 | **REVIEW-001 (M0) written** — two independent passes (correctness · adversarial security). Verdict: the **identity/session/CSRF core is sound** (no auth bypass, no session forgery, no CSRF gap, no SQLi, no traversal) but the **perimeter is weak**: **H2** (loopback enforced only at *bind*, not per request) and **H1** (XFF resolved *leftmost*-untrusted, forgeable through an append-style proxy) are **HIGH**; plus M1c/M1s/M2s/M3s/M2c and L1c/L3s/L1s/L5s/L4s/L7c/L8c. `docs/reviews/review-001-m0.md`; register rows re-opened where the review contradicted "enforced". | | 2026-10-14 | **REVIEW-001 FIX PASS (security).** **H2** — a **per-request** loopback gate in `dispatch` while in setup mode (a non-loopback *resolved* client IP → 404; the bind check alone is defeated by a reverse proxy — the exact first-visitor-wins D-025 claims to close); SPEC-003 §7 + AC-10, A13 → **enforced**. **H1** — `ClientIP` walks XFF **right→left** and returns the first untrusted hop, falling back to the peer (A6/AC-12); the unit test that encoded the vulnerable expectation is rewritten. **M1c/L3s** — the `setup` **module is pinned at the door**: `serveSetupAction` rejects `Module != "setup"`, and any other resolution naming it 404s in every mode instead of falling through to `serveGUID`. **M1s** — the CSRF/cookie signing keys are **generated at startup** in setup mode (mirroring the TOTP key, D-1/R-6), so the wizard's ceremony is never signed with a shipped public placeholder; `finish` persists the same values. **M2s** — the `/access` notification runs **off the request path** (a `Dispatch` seam, `go fn()` by default, context detached) so a hit is not timed against a miss (A3). **M3s** — the rate limiter's state map is **pruned** (time-gated sweep at a size threshold) and **capped** (memory-DoS). **L1s** — the session cookie is **`__Host-myday_session`** over https (conditional: a browser rejects `__Host-` without `Secure`, which would break the http loopback bootstrap). **L5s** — **TOTP replay** refused: the highest accepted counter per identity is tracked (reset on re-enrollment). **L4s** — `Permissions-Policy` · `Cross-Origin-Opener-Policy` · `Cross-Origin-Resource-Policy`. **L7c** — the nudge's origin signal is the configured **`base_url` host** (falling back to the request `Host`), so a proxy rewriting `Host` cannot suppress it. SPEC-003 amended (§4/§6/§7/§9/§10 + §11 the fix table). | | 2026-10-14 | **REVIEW-001 FIX PASS (correctness & cleanup).** **L1c/L2s** — dead `secrets.session_key` **removed** (`config.go` · `setup.go` · `config.example.yaml`; SPEC-003 D-2 amended). **L8c** — README corrected (M0/M0.1 status; `/setup` **persists `config.yaml`** and a **restart adopts the generated secrets**; the per-request loopback rule stated). **M2c** — an automated **restart-survival test** (`cmd/myday/main_test.go`) generates the TOTP key at startup, enrolls a secret, persists via the wizard, reopens the DB with the persisted key and asserts the secret still unseals — the **D-1** invariant, previously covered only by a manual smoke. Gate green (`build` · `vet` · `test -race`), **static CGO-free** binary, `routing` purity intact; the H2 guard was reverted to confirm its test fails without it. **Still open (deferred, low): L3c · L4c · L5c · L6c.** | +| 2026-10-15 | **Deployment topology (D-043) + D-025 REVISED.** The app sits **behind Zoraxy** (`myday.sechpoint.app`); **TLS terminates at the proxy**, so the app **listens on `0.0.0.0:8080` over HTTP** — a **bind refusal is impossible**. **D-025 becomes a per-request gate**: in setup mode serve only a request whose **TCP peer is loopback *and* which carries no `X-Forwarded-For`** (the **direct SSH tunnel**); anything else → 404. **D-043:** honour the proxy's **`X-Forwarded-Proto`** (`Secure` + `__Host-` cookies, HSTS); `trusted_proxies` = Zoraxy; **firewall `:8080` to Zoraxy only**. SPEC-003 §4/§6/§7/§9 amended (AC-7 · AC-10 · **AC-13**). | +| 2026-10-15 | **D-025 (revised) + D-043 ✅ IMPLEMENTED.** **Bind refusal removed** — `server.CheckSetupBind` / `ErrNonLoopbackSetup` / `server.IsLoopback` deleted; `Serve` binds any address (the deployment binds `0.0.0.0:8080`). **Setup gate is per request and strict** — `httpx.SetupRequestAllowed` requires a **loopback TCP peer AND no `X-Forwarded-For`**, evaluated on the RAW request (captured before `ClientIP` SETS the header); anything else → **404 before dispatch**; an unparseable peer fails closed (AC-7/AC-10). **Scheme is per request (D-043)** — `httpx.RequestIsHTTPS` (direct TLS, or a **trusted** proxy's `X-Forwarded-Proto`), carried on the request context (`httpx.WithSecure`), so the session cookie, the **`__Host-`** prefix, the pre-session/ceremony cookies and **HSTS** all agree; the plain-http tunnel keeps the **bare, non-Secure** cookie — which also fixes the latent `config.example.yaml` bootstrap (an `https://` `base_url` no longer force-marks the cookie Secure over the tunnel). `config.IsSecure` **removed** (dead). `config.example.yaml` documents the Zoraxy topology + the **firewall-`:8080`-to-Zoraxy-only** rule; the README gains a **Deployment** section and the tunnel-only bootstrap. Syncs **SPEC-003 §4/§6/§7/§9** and **SPEC-005 §3/§5/§6**. Gate green (`build` · `vet` · `test -race` — 18 pkgs) · **static CGO-free** binary · `routing` purity intact. Live smoke (real binary): `/setup` via the tunnel → **200**, `+ X-Forwarded-For` → **404**, `/home` → **404**, `X-Forwarded-Proto: https` → **HSTS + Secure** cookie, plain http → **neither**. | diff --git a/cmd/myday/main.go b/cmd/myday/main.go index 73ca679..011947e 100644 --- a/cmd/myday/main.go +++ b/cmd/myday/main.go @@ -1,8 +1,9 @@ // Command myday is the MyDay entrypoint. // // It assembles config → core services → the registry → the door and serves -// (SPEC-003 §3). In setup mode the door refuses a non-loopback bind, so a fresh -// instance bootstraps over an SSH tunnel to 127.0.0.1:8080 (D-025). +// (SPEC-003 §3). Any address may be bound (D-025 revised / D-043): the +// bootstrap is protected per request — in setup mode the door serves only the +// direct SSH tunnel — not by refusing a non-loopback bind. // // Spec: docs/specs/spec-003-core-door.md, docs/specs/spec-006-enroll-login.md. package main @@ -172,7 +173,8 @@ func run() error { return err } // A15: the anonymous enroll/login forms carry a signed double-submit cookie. - preSession, err := httpx.NewPreSessionCSRF([]byte(cfg.Secrets.CookieSecret), cfg.IsSecure()) + // Its `Secure` flag is resolved per request by the door (D-043). + preSession, err := httpx.NewPreSessionCSRF([]byte(cfg.Secrets.CookieSecret)) if err != nil { return err } @@ -204,7 +206,6 @@ func run() error { Audit: aud, SuppressNudge: setupMode, BaseURL: cfg.Server.BaseURL, - Secure: cfg.IsSecure(), Logger: slog.Default(), }) if err != nil { @@ -227,7 +228,6 @@ func run() error { TOTPOnlyEnroll: true, SuccessPath: func(string) string { return "/setup" }, BaseURL: cfg.Server.BaseURL, - Secure: cfg.IsSecure(), Logger: slog.Default(), }) if err != nil { @@ -342,8 +342,8 @@ func run() error { } if setupMode() { - slog.Warn("myday: SETUP MODE — serving /setup over loopback only; " + - "bootstrap via an SSH tunnel to 127.0.0.1:8080 before exposing this instance") + slog.Warn("myday: SETUP MODE — serving /setup over the direct SSH tunnel only; " + + "bootstrap via `ssh -L 8080:127.0.0.1:8080` before exposing this instance") } if err := srv.Serve(cfg.Server.Listen); err != nil { @@ -463,7 +463,7 @@ func configTarget(cfg *config.Config) string { // webauthnConfig derives the Relying Party configuration from server.base_url. // An empty/unparseable base URL falls back to loopback, which keeps setup mode -// (loopback-only, D-025) working before the domain is configured. +// (the tunnel-only gate, D-025) working before the domain is configured. func webauthnConfig(baseURL string) webauthn.Config { defaultCfg := webauthn.Config{ RPID: "localhost", diff --git a/cmd/myday/main_test.go b/cmd/myday/main_test.go index 57bc7ce..5823631 100644 --- a/cmd/myday/main_test.go +++ b/cmd/myday/main_test.go @@ -107,7 +107,7 @@ func TestTOTPKeySurvivesSetupAndRestart(t *testing.T) { } // The wizard persists the config (`finish`). - pre, err := httpx.NewPreSessionCSRF([]byte(cfg.Secrets.CookieSecret), false) + pre, err := httpx.NewPreSessionCSRF([]byte(cfg.Secrets.CookieSecret)) if err != nil { t.Fatalf("NewPreSessionCSRF: %v", err) } diff --git a/internal/core/auth/flow/flow.go b/internal/core/auth/flow/flow.go index ec1acf8..72b4830 100644 --- a/internal/core/auth/flow/flow.go +++ b/internal/core/auth/flow/flow.go @@ -77,8 +77,6 @@ type Deps struct { // signal: a proxy that rewrites `Host` must not flip the decision (L7c). // Empty falls back to the request `Host`. BaseURL string - // Secure marks cookies Secure (https). - Secure bool // Logger receives structured logs. Defaults to slog.Default(). Logger *slog.Logger // Now is the clock (tests). @@ -189,7 +187,7 @@ func (a *Actions) EnrollBegin(w http.ResponseWriter, r *http.Request, guid strin writeError(w, http.StatusBadRequest, "bad request") return nil } - if err := a.putCeremony(w, begin); err != nil { + if err := a.putCeremony(w, r, begin); err != nil { return err } return writeJSON(w, http.StatusOK, begin.Options) @@ -349,7 +347,7 @@ func (a *Actions) LoginBegin(w http.ResponseWriter, r *http.Request, guid string writeError(w, http.StatusBadRequest, "bad request") return nil } - if err := a.putCeremony(w, begin); err != nil { + if err := a.putCeremony(w, r, begin); err != nil { return err } return writeJSON(w, http.StatusOK, begin.Options) @@ -391,12 +389,14 @@ func (a *Actions) LoginFinish(w http.ResponseWriter, r *http.Request, guid strin // Logout revokes the current session and clears the cookie (SPEC-006 §5). func (a *Actions) Logout(w http.ResponseWriter, r *http.Request, guid string) error { ctx := r.Context() - if c, err := r.Cookie(httpx.SessionCookieNameFor(a.d.Secure)); err == nil && c.Value != "" { + // The cookie name/Secure flag follow the request scheme (D-043). + secure := httpx.SecureFromRequest(r) + if c, err := r.Cookie(httpx.SessionCookieNameFor(secure)); err == nil && c.Value != "" { if err := a.d.Sessions.Revoke(ctx, c.Value); err != nil { a.d.Logger.Warn("flow: logout revoke failed", "error", err) } } - httpx.ClearSessionCookie(w, httpx.SessionCookieNameFor(a.d.Secure), a.d.Secure) + httpx.ClearSessionCookie(w, httpx.SessionCookieNameFor(secure), secure) http.Redirect(w, r, "/home", http.StatusSeeOther) return nil } @@ -487,7 +487,8 @@ func (a *Actions) establishSessionAt(w http.ResponseWriter, r *http.Request, idn if err != nil { return err } - httpx.SetSessionCookie(w, httpx.SessionCookieNameFor(a.d.Secure), sess.ID, a.d.Secure) + secure := httpx.SecureFromRequest(r) // per-request scheme (D-043) + httpx.SetSessionCookie(w, httpx.SessionCookieNameFor(secure), sess.ID, secure) if target == "" { target = a.successTarget(idn.GUID) } @@ -579,7 +580,7 @@ func (a *Actions) PasskeyPrompt(w http.ResponseWriter, r *http.Request, guid str writeError(w, http.StatusBadRequest, "bad request") return nil } - if err := a.putCeremony(w, begin); err != nil { + if err := a.putCeremony(w, r, begin); err != nil { return err } return renderHTML(w, http.StatusOK, passkeyPage(guid, httpx.TokenFromRequest(r), string(begin.Options))) @@ -662,7 +663,7 @@ func (a *Actions) issueSession(r *http.Request, idn auth.Identity) (auth.Session if m, ok := audit.MetaFrom(ctx); ok { ip = m.ClientIP } - if c, err := r.Cookie(httpx.SessionCookieNameFor(a.d.Secure)); err == nil && c.Value != "" { + if c, err := r.Cookie(httpx.SessionCookieNameFor(httpx.SecureFromRequest(r))); err == nil && c.Value != "" { if old, err := a.d.Sessions.Validate(ctx, c.Value); err == nil && old.IdentityID == idn.ID { return a.d.Sessions.Rotate(ctx, c.Value) } @@ -671,7 +672,7 @@ func (a *Actions) issueSession(r *http.Request, idn auth.Identity) (auth.Session } // putCeremony signs the WebAuthn state and stores it in a short-lived cookie. -func (a *Actions) putCeremony(w http.ResponseWriter, b *webauthn.Begin) error { +func (a *Actions) putCeremony(w http.ResponseWriter, r *http.Request, b *webauthn.Begin) error { raw, err := json.Marshal(b) if err != nil { return fmt.Errorf("flow: marshal ceremony: %w", err) @@ -681,7 +682,7 @@ func (a *Actions) putCeremony(w http.ResponseWriter, b *webauthn.Begin) error { Value: a.d.CSRF.Signer().Sign(string(raw)), Path: "/", HttpOnly: true, - Secure: a.d.Secure, + Secure: httpx.SecureFromRequest(r), // per-request scheme (D-043) SameSite: http.SameSiteLaxMode, MaxAge: 300, }) @@ -708,7 +709,7 @@ func (a *Actions) takeCeremony(w http.ResponseWriter, r *http.Request) (*webauth Value: "", Path: "/", HttpOnly: true, - Secure: a.d.Secure, + Secure: httpx.SecureFromRequest(r), // per-request scheme (D-043) SameSite: http.SameSiteLaxMode, MaxAge: -1, }) diff --git a/internal/core/config/config.go b/internal/core/config/config.go index fad80c5..2a33a96 100644 --- a/internal/core/config/config.go +++ b/internal/core/config/config.go @@ -4,12 +4,12 @@ // Settings live in the config file, not the database (D-035): infra · secrets · // domain · mail · locale. The real `config.yaml` is gitignored (D-010); the // committed `config.example.yaml` is used as a development fallback so a fresh -// checkout can still start (in setup mode, loopback-only). +// checkout can still start (in setup mode, reachable only over the tunnel). // // Fail-closed rule (SPEC-003 §1): in PRODUCTIVE mode (`setup: false`) the // process refuses to start if a required secret is missing or weak. Setup mode // tolerates the placeholder values in the example, because the operator has not -// provisioned the instance yet — and setup mode is loopback-only (D-025). +// provisioned the instance yet — and setup mode is tunnel-only (D-025). // // Spec: docs/specs/spec-003-core-door.md §1, §7. package config @@ -46,7 +46,7 @@ const ( // Config is the typed shape of `config.yaml` / `config.example.yaml`. type Config struct { // Setup forces setup mode (D-026). Together with "no user exists" it makes - // the instance serve only /setup, loopback-only (D-025, SPEC-005 §3). + // the instance serve only /setup, tunnel-only (D-025, SPEC-005 §3). Setup bool `yaml:"setup"` // AdminRecovery is the config approval flag for admin recovery (D-033). @@ -187,11 +187,6 @@ func Marshal(c *Config) ([]byte, error) { return append([]byte(header), body...), nil } -// IsSecure reports whether the app is served over https (so cookies are Secure). -func (c *Config) IsSecure() bool { - return strings.HasPrefix(strings.ToLower(strings.TrimSpace(c.Server.BaseURL)), "https://") -} - // GenerateSecret returns n cryptographically-random bytes, base64url-encoded // without padding (R-6): the value carries no YAML-hostile characters and its // encoded length is 4*ceil(n/3) — 24 bytes yield the 32 characters the TOTP @@ -226,7 +221,7 @@ func (c *Config) Validate() error { } // Setup mode may run with placeholder secrets: the operator has not yet - // provisioned the instance, and it is loopback-only (D-025). Productive + // provisioned the instance, and it is tunnel-only (D-025). Productive // mode is strict. if c.Setup { return nil diff --git a/internal/core/config/config_test.go b/internal/core/config/config_test.go index 105b534..e297c35 100644 --- a/internal/core/config/config_test.go +++ b/internal/core/config/config_test.go @@ -84,9 +84,6 @@ func TestLoadParsesTypedStruct(t *testing.T) { if !cfg.Modules["requests"] || cfg.Modules["expense"] { t.Errorf("Modules = %v", cfg.Modules) } - if cfg.IsSecure() { - t.Error("IsSecure = true for an http base_url") - } } // TestLoadFallsBackToExample covers the dev fallback: a fresh checkout has only @@ -211,9 +208,6 @@ func TestProductiveModeStrongConfigLoads(t *testing.T) { if cfg.Setup { t.Error("Setup = true, want false") } - if !cfg.IsSecure() { - t.Error("IsSecure = false for an https base_url") - } } // TestUnknownKeyFailsClosed: a typo in config.yaml must not be silently ignored. diff --git a/internal/core/httpx/cookie.go b/internal/core/httpx/cookie.go index 04bb26e..372f4ff 100644 --- a/internal/core/httpx/cookie.go +++ b/internal/core/httpx/cookie.go @@ -22,6 +22,10 @@ const HostPrefix = "__Host-" // `__Host-myday_session` over https (Secure is set — required by the prefix) // and the bare name otherwise, because a browser REJECTS a `__Host-` cookie // sent without Secure over plain http (the loopback bootstrap / human test). +// +// The `secure` decision is made PER REQUEST by the door (D-043): TLS, or a +// trusted proxy's `X-Forwarded-Proto: https`. Over the plain-http bootstrap it +// is false, so the bare name is used. func SessionCookieNameFor(secure bool) string { if secure { return HostPrefix + SessionCookieName diff --git a/internal/core/httpx/httpx_test.go b/internal/core/httpx/httpx_test.go index c31d1c8..f98aadc 100644 --- a/internal/core/httpx/httpx_test.go +++ b/internal/core/httpx/httpx_test.go @@ -2,6 +2,7 @@ package httpx_test import ( "context" + "crypto/tls" "net" "net/http" "net/http/httptest" @@ -64,6 +65,89 @@ func TestParseTrustedProxies(t *testing.T) { } } +// TestRequestIsHTTPS pins D-043: https is decided by the transport — a direct +// TLS connection, or a TRUSTED proxy's X-Forwarded-Proto. An untrusted peer's +// header is ignored. +func TestRequestIsHTTPS(t *testing.T) { + tp, _ := httpx.ParseTrustedProxies([]string{"10.0.0.5"}) + newReq := func(peer string, useTLS bool, xfp string) *http.Request { + r := httptest.NewRequest("GET", "/", nil) + r.RemoteAddr = peer + if useTLS { + r.TLS = &tls.ConnectionState{} + } + if xfp != "" { + r.Header.Set("X-Forwarded-Proto", xfp) + } + return r + } + cases := []struct { + name string + req *http.Request + want bool + }{ + {"direct TLS", newReq("203.0.113.9:4444", true, ""), true}, + {"trusted proxy, XFP https", newReq("10.0.0.5:4444", false, "https"), true}, + {"trusted proxy, XFP http", newReq("10.0.0.5:4444", false, "http"), false}, + {"trusted proxy, no XFP", newReq("10.0.0.5:4444", false, ""), false}, + {"trusted proxy, XFP list", newReq("10.0.0.5:4444", false, "https, http"), true}, + {"untrusted peer spoofs XFP https", newReq("203.0.113.9:4444", false, "https"), false}, + {"loopback peer, no XFP", newReq("127.0.0.1:4444", false, ""), false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := httpx.RequestIsHTTPS(tc.req, tp); got != tc.want { + t.Errorf("RequestIsHTTPS = %v, want %v", got, tc.want) + } + }) + } +} + +// TestSecureFromRequestDefaultsFalse pins the transport-neutral default: a +// request that never passed through the door is plain http, so cookie writers +// fall back to the non-Secure (bare) behaviour. +func TestSecureFromRequestDefaultsFalse(t *testing.T) { + r := httptest.NewRequest("GET", "/", nil) + if httpx.SecureFromRequest(r) { + t.Error("SecureFromRequest = true with no door-set value, want false") + } + ctx := httpx.WithSecure(r.Context(), true) + if !httpx.SecureFromRequest(r.WithContext(ctx)) { + t.Error("SecureFromRequest did not read the door-set value") + } +} + +// TestSetupRequestAllowed pins D-025 (revised): only a loopback TCP peer with +// NO X-Forwarded-For is the direct tunnel; everything else — including a +// loopback peer carrying XFF, or an unparseable peer — fails closed. +func TestSetupRequestAllowed(t *testing.T) { + cases := []struct { + name string + peer string + xff string + want bool + }{ + {"loopback peer, no XFF", "127.0.0.1:52345", "", true}, + {"loopback peer with XFF", "127.0.0.1:52345", "203.0.113.9", false}, + {"remote peer", "203.0.113.9:4444", "", false}, + {"remote peer with XFF", "203.0.113.9:4444", "198.51.100.7", false}, + {"empty peer", "", "", false}, + {"unparseable peer", "not-an-address", "", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r := httptest.NewRequest("GET", "/setup", nil) + r.RemoteAddr = tc.peer + if tc.xff != "" { + r.Header.Set("X-Forwarded-For", tc.xff) + } + if got := httpx.SetupRequestAllowed(r); got != tc.want { + t.Errorf("SetupRequestAllowed = %v, want %v", got, tc.want) + } + }) + } +} + func TestClientIPUntrustedPeerIgnoresXFF(t *testing.T) { tp, _ := httpx.ParseTrustedProxies([]string{"127.0.0.1"}) r := httptest.NewRequest("GET", "/", nil) diff --git a/internal/core/httpx/loopback.go b/internal/core/httpx/loopback.go index c794ae4..185a06a 100644 --- a/internal/core/httpx/loopback.go +++ b/internal/core/httpx/loopback.go @@ -2,6 +2,7 @@ package httpx import ( "net" + "net/http" "strings" ) @@ -18,18 +19,34 @@ func IsLoopbackHost(hostport string) bool { return !known || loopback } -// IsLoopbackIP reports whether a RESOLVED client IP is loopback. It gates the -// per-request setup-mode check (SPEC-003 §7, H2) and therefore FAILS CLOSED in -// the other direction: an empty or unparseable value is NOT loopback. -// -// A bind-time check alone is not enough (D-025): a reverse proxy on 127.0.0.1 -// relaying remote clients makes `/setup` reachable past the bind check, so the -// door must re-check every request. +// IsLoopbackIP reports whether a RESOLVED client IP (or a raw TCP peer address) +// is loopback. It gates the setup-mode check (SPEC-003 §7, D-025) and therefore +// FAILS CLOSED in the other direction: an empty or unparseable value is NOT +// loopback. func IsLoopbackIP(ip string) bool { loopback, known := classifyHost(ip) return known && loopback } +// SetupRequestAllowed reports whether a request may be served while the +// instance is in setup mode (SPEC-003 §7, D-025 revised 2026-10-15): the TCP +// PEER (the connection's RemoteAddr) must be loopback AND the request must +// carry NO `X-Forwarded-For` — i.e. the direct SSH tunnel. Anything else, +// including a request relayed by a trusted proxy, is refused (the door answers +// 404 before dispatch). +// +// Both tests are required: a proxy on the same host presents a loopback peer, +// so the peer alone is not enough — the ABSENCE of `X-Forwarded-For` is what +// proves the request did not traverse a proxy. An empty/unparseable peer fails +// closed. NB: this must be evaluated on the RAW request, before ClientIP() SETS +// the header (A6). +func SetupRequestAllowed(r *http.Request) bool { + if strings.TrimSpace(r.Header.Get("X-Forwarded-For")) != "" { + return false + } + return IsLoopbackIP(r.RemoteAddr) +} + // classifyHost reports whether hostport ("host", "host:port", or a bare IP) is // loopback, and whether it could be classified at all. The two callers above // resolve "unknown" in opposite directions on purpose: each is fail-closed for diff --git a/internal/core/httpx/presession.go b/internal/core/httpx/presession.go index d02cbc9..dde3fbf 100644 --- a/internal/core/httpx/presession.go +++ b/internal/core/httpx/presession.go @@ -72,18 +72,22 @@ func (s *Signer) mac(payload string) string { // before a session exists, so this fills the gap: the server issues a signed // random token in an HttpOnly cookie AND embeds the same token in the form; on // submit both must match and the signature must verify. +// +// Its cookie's `Secure` flag is resolved PER REQUEST from the door (D-043): the +// cookie is Secure behind a TLS-terminating trusted proxy and on a direct TLS +// connection, and non-Secure over the plain-http loopback bootstrap, where a +// Secure cookie would break the wizard. type PreSessionCSRF struct { signer *Signer - secure bool } // NewPreSessionCSRF builds the helper from `secrets.cookie_secret`. -func NewPreSessionCSRF(secret []byte, secure bool) (*PreSessionCSRF, error) { +func NewPreSessionCSRF(secret []byte) (*PreSessionCSRF, error) { s, err := NewSigner(secret) if err != nil { return nil, err } - return &PreSessionCSRF{signer: s, secure: secure}, nil + return &PreSessionCSRF{signer: s}, nil } // Signer exposes the underlying signer so the flows can sign their own @@ -116,7 +120,7 @@ func (p *PreSessionCSRF) Ensure(w http.ResponseWriter, r *http.Request) (string, Value: tok, Path: "/", HttpOnly: true, - Secure: p.secure, + Secure: SecureFromRequest(r), SameSite: http.SameSiteLaxMode, }) return tok, nil diff --git a/internal/core/httpx/presession_test.go b/internal/core/httpx/presession_test.go index df6beee..4dde0d4 100644 --- a/internal/core/httpx/presession_test.go +++ b/internal/core/httpx/presession_test.go @@ -37,7 +37,7 @@ func TestSignerRoundTripAndTamper(t *testing.T) { } func TestPreSessionCSRFEnsureAndValid(t *testing.T) { - p, err := httpx.NewPreSessionCSRF([]byte("a-cookie-secret-0123456789abcdef"), false) + p, err := httpx.NewPreSessionCSRF([]byte("a-cookie-secret-0123456789abcdef")) if err != nil { t.Fatalf("NewPreSessionCSRF: %v", err) } diff --git a/internal/core/httpx/scheme.go b/internal/core/httpx/scheme.go new file mode 100644 index 0000000..d9362b7 --- /dev/null +++ b/internal/core/httpx/scheme.go @@ -0,0 +1,47 @@ +package httpx + +import ( + "context" + "net/http" + "strings" +) + +// secureKey carries the per-request "served over https" decision on the request +// context. The door resolves it ONCE (RequestIsHTTPS) and every cookie writer +// reads it back, so the session cookie's `Secure` flag, the `__Host-` prefix, +// and HSTS all agree on one value (D-043). +type secureKey struct{} + +// WithSecure records whether the request is served over https (D-043). The door +// sets it before dispatching; a request that never passed through the door has +// no value and is treated as plain http. +func WithSecure(ctx context.Context, secure bool) context.Context { + return context.WithValue(ctx, secureKey{}, secure) +} + +// SecureFromRequest reports whether the request is served over https, as +// resolved by the door (D-043). The zero value is false — plain http — which is +// what keeps the loopback bootstrap (the SSH tunnel) working. +func SecureFromRequest(r *http.Request) bool { + v, _ := r.Context().Value(secureKey{}).(bool) + return v +} + +// RequestIsHTTPS reports whether r reached the app over https (D-043): either +// the connection itself is TLS, or a TRUSTED proxy terminated TLS and said so +// via `X-Forwarded-Proto`. A header from an untrusted peer is ignored, so a +// client cannot spoof `Secure` cookies onto a plain-http connection. The +// leftmost token is the original client-facing scheme. +func RequestIsHTTPS(r *http.Request, trusted TrustedProxies) bool { + if r.TLS != nil { + return true + } + if !trusted.Contains(remoteIP(r.RemoteAddr)) { + return false + } + proto := r.Header.Get("X-Forwarded-Proto") + if i := strings.IndexByte(proto, ','); i >= 0 { + proto = proto[:i] + } + return strings.EqualFold(strings.TrimSpace(proto), "https") +} diff --git a/internal/core/recovery/flow_test.go b/internal/core/recovery/flow_test.go index 31b1090..19947d3 100644 --- a/internal/core/recovery/flow_test.go +++ b/internal/core/recovery/flow_test.go @@ -67,7 +67,7 @@ func TestAccessNotificationIsAsynchronous(t *testing.T) { t.Fatalf("NewAEADSealer: %v", err) } core := auth.New(db.DB, aud, auth.WithSealer(sealer)) - pre, err := httpx.NewPreSessionCSRF([]byte("flow-cookie-secret-0123456789abcdef"), false) + pre, err := httpx.NewPreSessionCSRF([]byte("flow-cookie-secret-0123456789abcdef")) if err != nil { t.Fatalf("NewPreSessionCSRF: %v", err) } diff --git a/internal/core/server/server.go b/internal/core/server/server.go index ac707b7..15665eb 100644 --- a/internal/core/server/server.go +++ b/internal/core/server/server.go @@ -22,9 +22,7 @@ package server import ( "errors" - "fmt" "log/slog" - "net" "net/http" "strconv" "strings" @@ -39,10 +37,6 @@ import ( "git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/users" ) -// ErrNonLoopbackSetup is returned when a non-loopback listen address is -// requested while in setup mode (D-025) — the process must refuse to start. -var ErrNonLoopbackSetup = errors.New("server: refusing to bind a non-loopback address in setup mode") - // Deps assembles the door's dependencies. type Deps struct { // Config is the loaded configuration. @@ -95,7 +89,6 @@ type Server struct { csrf *httpx.CSRF precsrf *httpx.PreSessionCSRF trusted httpx.TrustedProxies - secure bool setup func() bool roles RoleGate log *slog.Logger @@ -165,7 +158,6 @@ func New(d Deps) (*Server, error) { csrf: d.CSRF, precsrf: d.PreSession, trusted: trusted, - secure: d.Config.IsSecure(), setup: setup, roles: roles, log: logger, @@ -177,12 +169,11 @@ func New(d Deps) (*Server, error) { // Handler wraps the door with the correlation-id middleware (SPEC-003 §4). func (s *Server) Handler() http.Handler { return middleware.CorrelationID(s) } -// Serve binds listen and serves the door until it fails. In setup mode a -// non-loopback listen is refused outright (D-025, AC-7). +// Serve binds listen and serves the door until it fails. Any address may be +// bound — including `0.0.0.0:8080` behind a reverse proxy (D-025 revised, +// D-043); the bootstrap is protected per request by the setup gate, not by a +// bind check. func (s *Server) Serve(listen string) error { - if err := CheckSetupBind(s.setup(), listen); err != nil { - return err - } s.log.Info("myday: listening", "addr", listen, "setup_mode", s.setup()) srv := &http.Server{ Addr: listen, @@ -196,6 +187,10 @@ func (s *Server) Serve(listen string) error { func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { start := s.now() + // D-025 (revised): the setup gate must see the RAW X-Forwarded-For, before + // step 2 below SETS it (A6). Capture the tunnel verdict now. + setupTunnel := httpx.SetupRequestAllowed(r) + ctx := r.Context() cid := middleware.CorrelationFrom(ctx) if cid == "" { @@ -207,8 +202,13 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { // 2. client IP (SET XFF, never append — A6). ip := httpx.ClientIP(r, s.trusted) + // The scheme is resolved per request (D-043): TLS, or a trusted proxy's + // X-Forwarded-Proto. Every cookie writer reads it back off the context. + secure := httpx.RequestIsHTTPS(r, s.trusted) + ctx = httpx.WithSecure(ctx, secure) + // 3. security headers. - httpx.SetSecurityHeaders(w, s.secure) + httpx.SetSecurityHeaders(w, secure) // The door attaches audit.Meta before any service call (R-1). The actor is // filled in once the identity gate resolves it. @@ -248,18 +248,19 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { res = s.disp.Dispatch(r.URL.Path, r.FormValue("do"), r.FormValue("action")) // 6. the Kind switch. - s.dispatch(sw, r, res, cid, ip) + s.dispatch(sw, r, res, cid, ip, setupTunnel) } // dispatch is the Kind switch (SPEC-003 §4 step 5). -func (s *Server) dispatch(w http.ResponseWriter, r *http.Request, res routing.Resolution, cid, ip string) { - // SPEC-003 §7 / D-025 / A13 (H2): setup mode is loopback-only PER REQUEST. - // The bind-time check (`Serve`/`CheckSetupBind`) is necessary but not - // sufficient: a reverse proxy on 127.0.0.1 relaying remote clients makes - // /setup reachable past it, which is exactly the first-visitor-wins the - // design claims to close. An unparseable/empty client IP fails closed. - if s.setup() && !httpx.IsLoopbackIP(ip) { - s.log.Warn("setup request from a non-loopback client rejected", +func (s *Server) dispatch(w http.ResponseWriter, r *http.Request, res routing.Resolution, cid, ip string, setupTunnel bool) { + // SPEC-003 §7 / D-025 (revised 2026-10-15) / A13: setup mode is + // tunnel-only PER REQUEST. Serve only a request whose TCP peer is loopback + // AND which carries no X-Forwarded-For — the direct SSH tunnel. A proxy on + // the same host presents a loopback peer, so the ABSENCE of XFF is the + // proof it did not traverse one. Anything else → 404 before dispatch; an + // empty/unparseable peer fails closed. + if s.setup() && !setupTunnel { + s.log.Warn("setup request refused: not the direct tunnel", "correlation_id", cid, "client_ip", ip) s.writeError(w, http.StatusNotFound, "not found") return @@ -466,8 +467,9 @@ func (s *Server) serveGUID(w http.ResponseWriter, r *http.Request, res routing.R ctx := r.Context() // (a) load the session cookie; an absent or empty id is rejected (A2). - // Over https the cookie name carries the `__Host-` prefix (L1s). - cookieName := httpx.SessionCookieNameFor(s.secure) + // Over https the cookie name carries the `__Host-` prefix (L1s/D-043). + secure := httpx.SecureFromRequest(r) + cookieName := httpx.SessionCookieNameFor(secure) cookie, err := r.Cookie(cookieName) if err != nil || cookie.Value == "" { s.loginRequired(w, r, res, cid, ip) @@ -477,7 +479,7 @@ func (s *Server) serveGUID(w http.ResponseWriter, r *http.Request, res routing.R // Validate: exists · not expired · not revoked · non-empty (A2/AC-5). sess, err := s.sessions.Validate(ctx, cookie.Value) if err != nil { - httpx.ClearSessionCookie(w, cookieName, s.secure) + httpx.ClearSessionCookie(w, cookieName, secure) s.loginRequired(w, r, res, cid, ip) return } @@ -565,36 +567,6 @@ func (s *Server) writeError(w http.ResponseWriter, status int, msg string) { _, _ = w.Write([]byte(msg + "\n")) } -// CheckSetupBind enforces D-025: while in setup mode the process MUST NOT bind -// a non-loopback address. It is fail-closed — an unparseable or wildcard listen -// is refused. -func CheckSetupBind(setup bool, listen string) error { - if !setup { - return nil - } - if !IsLoopback(listen) { - return fmt.Errorf("%w: listen %q", ErrNonLoopbackSetup, listen) - } - return nil -} - -// IsLoopback reports whether a listen address ("host:port") is loopback-only. -func IsLoopback(listen string) bool { - host := strings.TrimSpace(listen) - if h, _, err := net.SplitHostPort(listen); err == nil { - host = h - } - host = strings.Trim(strings.TrimSpace(host), "[]") - if host == "" { - return false // ":8080" / "0.0.0.0" bind every interface — never loopback - } - if strings.EqualFold(host, "localhost") { - return true - } - ip := net.ParseIP(host) - return ip != nil && ip.IsLoopback() -} - // statusWriter captures the response status for the access log. type statusWriter struct { http.ResponseWriter diff --git a/internal/core/server/server_test.go b/internal/core/server/server_test.go index 0324469..23c70ac 100644 --- a/internal/core/server/server_test.go +++ b/internal/core/server/server_test.go @@ -3,7 +3,6 @@ package server_test import ( "bytes" "context" - "errors" "log/slog" "net/http" "net/http/httptest" @@ -180,7 +179,7 @@ func newHarness(t *testing.T, opts ...func(*server.Deps)) *harness { if err != nil { t.Fatalf("NewCSRF: %v", err) } - pre, err := httpx.NewPreSessionCSRF([]byte(cookieSecret), false) + pre, err := httpx.NewPreSessionCSRF([]byte(cookieSecret)) if err != nil { t.Fatalf("NewPreSessionCSRF: %v", err) } @@ -268,8 +267,9 @@ func (h *harness) cookie() *http.Cookie { } // loopbackPeer is the RemoteAddr the test door speaks from. Setup mode is -// loopback-only PER REQUEST (SPEC-003 §7, H2), so tests must explicitly opt in -// to a non-loopback peer to exercise the rejection. +// tunnel-only PER REQUEST (SPEC-003 §7, D-025): a loopback peer with no XFF. +// Tests must explicitly opt in to a different peer (or add XFF) to exercise +// the rejection. const loopbackPeer = "127.0.0.1:52345" // request issues a request directly against the door (no network). @@ -457,36 +457,45 @@ func TestAC6_ActionRequiresCSRF(t *testing.T) { } } -// ── AC-7 ───────────────────────────────────────────────────────────────────── +// ── AC-7 · AC-10 ───────────────────────────────────────────────────────────── + +// TestAC7_SetupGateIsTunnelOnly pins D-025 (revised): in setup mode the app +// serves only the direct SSH tunnel — a loopback TCP peer with NO +// X-Forwarded-For. A request relayed by a same-host proxy presents a loopback +// peer but carries XFF, so it is refused (AC-7). Any non-loopback peer is +// refused too (AC-10). The bind is no longer checked: any address may be bound +// (D-043). +func TestAC7_SetupGateIsTunnelOnly(t *testing.T) { + h := newHarness(t, func(d *server.Deps) { d.SetupMode = func() bool { return true } }) -func TestAC7_LoopbackEnforcement(t *testing.T) { cases := []struct { - setup bool - listen string - want bool // want an error? + name string + peer string + xff string + wantOK bool + comment string }{ - {true, "0.0.0.0:8080", true}, - {true, ":8080", true}, - {true, "192.168.1.5:8080", true}, - {true, "127.0.0.1:8080", false}, - {true, "localhost:8080", false}, - {true, "[::1]:8080", false}, - {false, "0.0.0.0:8080", false}, // productive: a public bind is allowed + {"the direct tunnel", loopbackPeer, "", true, "loopback peer, no XFF"}, + {"relayed by a same-host proxy", "127.0.0.1:9999", "203.0.113.9", false, "loopback peer WITH XFF (AC-7)"}, + {"a remote peer", "203.0.113.9:4444", "", false, "non-loopback peer (AC-10)"}, + {"a remote peer behind a proxy", "203.0.113.9:4444", "198.51.100.7", false, "non-loopback peer + XFF (AC-10)"}, + {"an unparseable peer", "", "", false, "fail closed"}, } for _, tc := range cases { - err := server.CheckSetupBind(tc.setup, tc.listen) - if tc.want && !errors.Is(err, server.ErrNonLoopbackSetup) { - t.Errorf("CheckSetupBind(%v, %q) = %v, want ErrNonLoopbackSetup", tc.setup, tc.listen, err) - } - if !tc.want && err != nil { - t.Errorf("CheckSetupBind(%v, %q) = %v, want nil", tc.setup, tc.listen, err) - } - } - - // The full Serve path refuses to bind too (no listener is opened). - h := newHarness(t, func(d *server.Deps) { d.SetupMode = func() bool { return true } }) - if err := h.srv.Serve("0.0.0.0:0"); !errors.Is(err, server.ErrNonLoopbackSetup) { - t.Errorf("Serve with a public listen in setup mode = %v, want ErrNonLoopbackSetup (AC-7)", err) + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest("GET", "/setup", nil) + req.RemoteAddr = tc.peer + if tc.xff != "" { + req.Header.Set("X-Forwarded-For", tc.xff) + } + rec := httptest.NewRecorder() + h.srv.ServeHTTP(rec, req) + if want := http.StatusNotFound; tc.wantOK && rec.Code != http.StatusOK { + t.Fatalf("GET /setup (%s) = %d, want 200", tc.comment, rec.Code) + } else if !tc.wantOK && rec.Code != want { + t.Errorf("GET /setup (%s) = %d, want 404", tc.comment, rec.Code) + } + }) } } @@ -541,6 +550,74 @@ func TestAC9_RateLimiterOnAuthRoutes(t *testing.T) { } } +// ── AC-13 ──────────────────────────────────────────────────────────────────── + +// TestAC13_ProxySchemeDrivesCookieAndHSTS covers D-043: a TRUSTED proxy's +// `X-Forwarded-Proto: https` ⇒ the session cookie is `Secure` + `__Host-`, and +// HSTS is sent. Plain http keeps the bare, non-Secure cookie (the bootstrap), +// and an untrusted peer's header is ignored. +func TestAC13_ProxySchemeDrivesCookieAndHSTS(t *testing.T) { + login := func(h *harness, peer, xfp string) *httptest.ResponseRecorder { + t.Helper() + code := h.issueTOTP(t, h.session.IdentityID) + cookie, token := h.preCSRF() + form := url.Values{"csrf": {token}, "code": {code}} + req := httptest.NewRequest("POST", "/"+h.guid+"?do=auth&action=totp.verify", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.RemoteAddr = peer + if xfp != "" { + req.Header.Set("X-Forwarded-Proto", xfp) + } + req.AddCookie(cookie) + rec := httptest.NewRecorder() + h.srv.ServeHTTP(rec, req) + return rec + } + + // Behind the proxy: the peer is trusted, XFP says https. + h := newHarness(t, func(d *server.Deps) { + d.Config.Server.TrustedProxies = []string{"127.0.0.1"} + }) + rec := login(h, "127.0.0.1:9999", "https") + if rec.Code != http.StatusSeeOther { + t.Fatalf("TOTP login behind the proxy = %d, want 303 (AC-13)", rec.Code) + } + if rec.Header().Get("Strict-Transport-Security") == "" { + t.Error("no HSTS behind an https proxy (AC-13)") + } + if !hasCookieNamed(rec, httpx.HostPrefix+httpx.SessionCookieName, true) { + t.Error("the session cookie is not Secure + __Host- behind the proxy (AC-13)") + } + + // Plain http (the tunnel): no XFP → bare, non-Secure cookie, no HSTS. + h2 := newHarness(t) + plain := login(h2, "127.0.0.1:9999", "") + if plain.Header().Get("Strict-Transport-Security") != "" { + t.Error("HSTS set on a plain-http request (AC-13)") + } + if !hasCookieNamed(plain, httpx.SessionCookieName, false) { + t.Error("expected the bare, non-Secure session cookie over plain http (AC-13)") + } + + // An UNTRUSTED peer's XFP is ignored — no `__Host-` cookie. + h3 := newHarness(t) + spoof := login(h3, "203.0.113.9:4444", "https") + if hasCookieNamed(spoof, httpx.HostPrefix+httpx.SessionCookieName, true) { + t.Error("an untrusted peer's X-Forwarded-Proto was honoured (AC-13)") + } +} + +// hasCookieNamed reports whether the response set a non-empty cookie with the +// given name and Secure flag. +func hasCookieNamed(rec *httptest.ResponseRecorder, name string, secure bool) bool { + for _, c := range rec.Result().Cookies() { + if c.Name == name && c.Value != "" && c.Secure == secure { + return true + } + } + return false +} + // ── supporting behaviour ───────────────────────────────────────────────────── // TestRequestContextHandover proves §5: the module function receives the @@ -600,9 +677,11 @@ func TestSetupModeRouting(t *testing.T) { } } -// TestSpec003SetupIsLoopbackOnlyPerRequest is H2: the bind-time check is not -// enough — a reverse proxy on 127.0.0.1 relaying a remote client must not make -// /setup reachable (first-visitor-wins). +// TestSpec003SetupIsLoopbackOnlyPerRequest covers AC-10/D-025 (revised): the +// setup gate is per request and strict — a loopback TCP peer AND no +// X-Forwarded-For (the direct SSH tunnel). A proxy on 127.0.0.1 relaying a +// remote client presents a loopback peer but carries XFF, so it is NOT +// laundered into the bootstrap. func TestSpec003SetupIsLoopbackOnlyPerRequest(t *testing.T) { h := newHarness(t, func(d *server.Deps) { d.SetupMode = func() bool { return true } }) @@ -617,17 +696,17 @@ func TestSpec003SetupIsLoopbackOnlyPerRequest(t *testing.T) { rec := httptest.NewRecorder() h.srv.ServeHTTP(rec, req) if rec.Code != http.StatusNotFound { - t.Errorf("%s %s from a non-loopback client = %d, want 404 (H2)", tc.method, tc.target, rec.Code) + t.Errorf("%s %s from a non-loopback client = %d, want 404 (AC-10)", tc.method, tc.target, rec.Code) } } - // The tunnel (a loopback peer) still works. + // The tunnel (a loopback peer, no XFF) still works. if rec := request(h.srv, "GET", "/setup"); rec.Code != http.StatusOK { - t.Errorf("GET /setup from loopback = %d, want 200 (H2 must not break the tunnel)", rec.Code) + t.Errorf("GET /setup from loopback = %d, want 200 (the tunnel must work)", rec.Code) } - // A trusted proxy forwarding X-Forwarded-For does NOT launder a remote - // client into loopback: the resolved client IP is what is checked. + // A TRUSTED proxy forwarding X-Forwarded-For is refused even though its + // peer is loopback: the absence of XFF is what proves the direct tunnel. h2 := newHarness(t, func(d *server.Deps) { d.SetupMode = func() bool { return true } d.Config.Server.TrustedProxies = []string{"127.0.0.1"} @@ -638,7 +717,7 @@ func TestSpec003SetupIsLoopbackOnlyPerRequest(t *testing.T) { rec := httptest.NewRecorder() h2.srv.ServeHTTP(rec, req) if rec.Code != http.StatusNotFound { - t.Errorf("GET /setup relayed by a trusted proxy = %d, want 404 (H2: the RESOLVED client is remote)", rec.Code) + t.Errorf("GET /setup relayed by a trusted proxy = %d, want 404 (AC-7: tunnel-only)", rec.Code) } } diff --git a/internal/core/server/setup_wizard_test.go b/internal/core/server/setup_wizard_test.go index a62ec60..24f5f05 100644 --- a/internal/core/server/setup_wizard_test.go +++ b/internal/core/server/setup_wizard_test.go @@ -85,7 +85,7 @@ func newSetupHarness(t *testing.T, opts ...func(*setupHarness)) *setupHarness { if err != nil { t.Fatalf("NewCSRF: %v", err) } - pre, err := httpx.NewPreSessionCSRF([]byte(cookieSecret), false) + pre, err := httpx.NewPreSessionCSRF([]byte(cookieSecret)) if err != nil { t.Fatalf("NewPreSessionCSRF: %v", err) } @@ -198,8 +198,8 @@ func (h *setupHarness) post(action string, form url.Values, cookies ...*http.Coo req := httptest.NewRequest("POST", "/setup?do=setup&action="+url.QueryEscape(action), strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - // The wizard is loopback-only per request (SPEC-003 §7, H2): the operator - // reaches it over an SSH tunnel, so the tests speak from loopback. + // The wizard is tunnel-only per request (SPEC-003 §7, D-025): the operator + // reaches it over an SSH tunnel, so the tests speak from loopback with no XFF. req.RemoteAddr = loopbackPeer req.AddCookie(cookie) for _, c := range cookies { @@ -329,19 +329,34 @@ func TestR6_WizardGeneratesStrongSecrets(t *testing.T) { // ── AC-2 ───────────────────────────────────────────────────────────────────── -func TestSpec005AC2_NonLoopbackRefusedInSetupMode(t *testing.T) { +// TestSpec005AC2_ServedOnlyOverTheTunnel covers D-025 (revised): there is no +// bind refusal (any address may be bound, D-043) — the bootstrap is protected +// per request: in setup mode only the direct SSH tunnel (a loopback TCP peer +// with no X-Forwarded-For) is served. +func TestSpec005AC2_ServedOnlyOverTheTunnel(t *testing.T) { h := newSetupHarness(t) - // The process refuses to START on a non-loopback listen (D-025). - if err := h.srv.Serve("0.0.0.0:8080"); !errors.Is(err, server.ErrNonLoopbackSetup) { - t.Errorf("Serve(0.0.0.0:8080) in setup mode = %v, want ErrNonLoopbackSetup (AC-2)", err) + // The tunnel works. + if rec := request(h.srv, "GET", "/setup"); rec.Code != http.StatusOK { + t.Errorf("GET /setup over the tunnel = %d, want 200 (AC-2)", rec.Code) } - if err := h.srv.Serve("192.168.1.5:8080"); !errors.Is(err, server.ErrNonLoopbackSetup) { - t.Errorf("Serve(192.168.1.5:8080) in setup mode = %v, want ErrNonLoopbackSetup (AC-2)", err) + + // A remote peer, and a same-host proxy relaying a client, are both refused. + remote := httptest.NewRequest("GET", "/setup", nil) + remote.RemoteAddr = "203.0.113.9:4444" + rec := httptest.NewRecorder() + h.srv.ServeHTTP(rec, remote) + if rec.Code != http.StatusNotFound { + t.Errorf("GET /setup from a remote peer = %d, want 404 (AC-2)", rec.Code) } - // Loopback is allowed (no error, and no listener is opened for :0). - if err := server.CheckSetupBind(true, "127.0.0.1:8080"); err != nil { - t.Errorf("CheckSetupBind(loopback) = %v, want nil", err) + + proxied := httptest.NewRequest("GET", "/setup", nil) + proxied.RemoteAddr = "127.0.0.1:9999" + proxied.Header.Set("X-Forwarded-For", "203.0.113.9") + rec = httptest.NewRecorder() + h.srv.ServeHTTP(rec, proxied) + if rec.Code != http.StatusNotFound { + t.Errorf("GET /setup relayed by a same-host proxy = %d, want 404 (AC-2)", rec.Code) } } diff --git a/internal/core/server/spec007_recovery_test.go b/internal/core/server/spec007_recovery_test.go index 3fc53fd..6081c82 100644 --- a/internal/core/server/spec007_recovery_test.go +++ b/internal/core/server/spec007_recovery_test.go @@ -3,6 +3,7 @@ package server_test import ( "bytes" "context" + "crypto/tls" "net/http" "net/http/httptest" "net/url" @@ -85,7 +86,7 @@ func newRecoveryHarness(t *testing.T) *recoveryHarness { if err != nil { t.Fatalf("NewCSRF: %v", err) } - pre, err := httpx.NewPreSessionCSRF([]byte(cookieSecret), false) + pre, err := httpx.NewPreSessionCSRF([]byte(cookieSecret)) if err != nil { t.Fatalf("NewPreSessionCSRF: %v", err) } @@ -203,6 +204,9 @@ func (h *recoveryHarness) decide(action string, actor member, requestID int64) * } req := httptest.NewRequest("POST", "/"+actor.guid+"?do=recovery&action="+action, strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + // D-043: the request scheme (here, a direct TLS connection) decides the + // `__Host-` cookie name the door looks for. + req.TLS = &tls.ConnectionState{} req.AddCookie(h.sessionCookie(actor.sessionID)) rec := httptest.NewRecorder() h.srv.ServeHTTP(rec, req) diff --git a/internal/core/setup/setup.go b/internal/core/setup/setup.go index 14bb970..dc45b2e 100644 --- a/internal/core/setup/setup.go +++ b/internal/core/setup/setup.go @@ -1,5 +1,5 @@ -// Package setup implements SPEC-005: the loopback-only `/setup` wizard that -// brings a fresh instance to life. +// Package setup implements SPEC-005: the `/setup` wizard, reachable only over +// the direct SSH tunnel, that brings a fresh instance to life. // // The wizard walks three steps — (1) the domain / base URL, (2) the top admin // with TOTP enrollment, (3) mail · locale · timezone — and finishes by @@ -14,8 +14,8 @@ // no duplication. // // The wizard holds no routing of its own — the door (SPEC-003/005) gates it to -// setup mode and loopback, and verifies the pre-session CSRF token (A15) before -// invoking any action here. +// setup mode and the tunnel, and verifies the pre-session CSRF token (A15) +// before invoking any action here. // // Spec: docs/specs/spec-005-setup-wizard.md. package setup