From 5855b46902be5d10a0b003a59eb5d12a063ce15e Mon Sep 17 00:00:00 2001 From: marcoome Date: Mon, 21 Sep 2026 09:48:26 +0200 Subject: [PATCH 01/12] security(nodes): require the authenticated agent admin proxy for remote nodes Caddy's admin API authenticates nothing, so "can route to :2019" was equivalent to root on a node. The agent admin proxy existed but was opt-in and the shipped remote-node profile published 2019 on the WireGuard address. - deploy/remote-node: publish the admin port on 127.0.0.1 only, default to the custom edge image, add the shared GeoIP volume; the bootstrap Caddyfile now binds 0.0.0.0 inside the container (a WG address cannot be bound from a bridge container at all - that config could never start). - deploy/node-agent example: admin proxy vars are no longer commented out. - panel: refuse to register a node whose api_url is a raw remote :2019, in the admin form and POST /api/v1/nodes; `server doctor` prints an "admin API auth" row per node; approving an auto-joined node warns instead of blocking. - scripts/node-join.sh: fix the unbindable admin address and print the steps to close the direct port. Rollout order (a wrong order cuts the panel off from the node): 1. Enable tunnel / Rotate in the panel, copy HPG_ADMIN_PROXY_KEY. 2. Start the agent with HPG_ADMIN_PROXY_LISTEN/_KEY/HPG_CADDY_ADMIN_URL. 3. Repoint the node's API URL at http://:2021 and verify with doctor. 4. Only then change the caddy mapping to "127.0.0.1:2019:2019". Existing nodes are untouched; HPG_ALLOW_UNAUTHENTICATED_NODE_ADMIN=1 re-opens registration for a fleet mid-migration. (cherry picked from commit 6f0d15ba6d65b5de177c538b1c17abd61fa6b798) --- cmd/node-agent/main.go | 8 +++ cmd/server/doctor.go | 11 ++++ deploy/node-agent/docker-compose.example.yml | 33 +++++----- deploy/remote-node/Caddyfile.bootstrap | 8 ++- deploy/remote-node/docker-compose.yml | 29 ++++++--- docs/MULTI_NODE.md | 64 ++++++++++++-------- docs/SECURITY.md | 29 ++++++--- internal/httpserver/handlers/admin.go | 20 +++++- internal/httpserver/handlers/api_v1.go | 5 ++ internal/security/nodeadmin.go | 48 +++++++++++++++ internal/security/nodeadmin_test.go | 39 ++++++++++++ scripts/node-join.sh | 15 ++++- 12 files changed, 248 insertions(+), 61 deletions(-) create mode 100644 internal/security/nodeadmin.go create mode 100644 internal/security/nodeadmin_test.go diff --git a/cmd/node-agent/main.go b/cmd/node-agent/main.go index 6bdbed3b..138ad557 100644 --- a/cmd/node-agent/main.go +++ b/cmd/node-agent/main.go @@ -1556,6 +1556,14 @@ func syncGeoIP(ctx context.Context, log *slog.Logger, c config) { log.Debug("geoip: panel has no DB yet") return } + // The DB is only useful if Caddy can read it, so the directory must be a + // volume shared with the caddy container. Creating it inside the agent's + // own filesystem would "succeed" forever while GeoIP never works (OPS-005). + if st, err := os.Stat(filepath.Dir(geoipDBPath)); err != nil || !st.IsDir() { + log.Warn("geoip: target directory missing - mount a volume shared with the caddy container", + "path", filepath.Dir(geoipDBPath)) + return + } localSHA, _ := fileSHA256(geoipDBPath) if localSHA == remoteSHA { return // already current diff --git a/cmd/server/doctor.go b/cmd/server/doctor.go index 22fce219..53bbb0c3 100644 --- a/cmd/server/doctor.go +++ b/cmd/server/doctor.go @@ -18,6 +18,7 @@ import ( "github.com/host-yt/caddy-proxy-manager/internal/caddyapi" "github.com/host-yt/caddy-proxy-manager/internal/config" "github.com/host-yt/caddy-proxy-manager/internal/installstate" + "github.com/host-yt/caddy-proxy-manager/internal/security" "github.com/host-yt/caddy-proxy-manager/internal/store" ) @@ -251,6 +252,16 @@ func doctorNodes(ctx context.Context, db *sql.DB, rawCfg *config.Config) []check checks = append(checks, check{label + ": admin API", statusPass, n.apiURL + " reachable"}) } + // SEC-002: Caddy's admin API authenticates nothing. A node addressed at + // a remote :2019 is owned by whatever can route to it. + if security.UnauthenticatedNodeAdminURL(n.apiURL) { + checks = append(checks, check{label + ": admin API auth", statusWarn, + n.apiURL + " is Caddy's unauthenticated admin API - front it with the node-agent " + + "admin proxy and repoint api_url at http://:2021 (docs/MULTI_NODE.md)"}) + } else if n.adminProxyKeyEnc.Valid && n.adminProxyKeyEnc.String != "" { + checks = append(checks, check{label + ": admin API auth", statusPass, "agent admin proxy, bearer key issued"}) + } + if !n.modulesProbedAt.Valid { checks = append(checks, check{label + ": module probe", statusWarn, "not yet probed - populates after the next health-probe cycle"}) diff --git a/deploy/node-agent/docker-compose.example.yml b/deploy/node-agent/docker-compose.example.yml index 9cb35990..2434c659 100644 --- a/deploy/node-agent/docker-compose.example.yml +++ b/deploy/node-agent/docker-compose.example.yml @@ -12,7 +12,7 @@ version: "3.9" services: hpg-node-agent: - image: ghcr.io/host-yt/caddy-proxy-manager-node-agent:1.3.2 + image: ghcr.io/host-yt/caddy-proxy-manager-node-agent:1.5.1 restart: unless-stopped network_mode: host # needed for wg-tun0 + nftables + UDP listen cap_add: @@ -49,26 +49,31 @@ services: # forward new lines from startup. # HPG_FORWARD_TAIL_ONLY: "1" # - # --- Optional: front this node's Caddy admin API ------------------- + # --- REQUIRED: front this node's Caddy admin API (SEC-002) ---------- # Caddy's admin API has no authentication of its own, so publishing it on # the WireGuard IP makes "can reach 10.66.0.x:2019" equivalent to root on - # this node. With these two set, the agent serves an authenticated proxy + # this node. With these set, the agent serves an authenticated proxy # instead: it checks the panel-issued key and refuses anything outside the - # small set of admin calls the control plane makes. + # small set of admin calls the control plane makes. The panel refuses to + # register or approve a node whose api_url is a raw remote :2019. # - # Migrating a node (order matters, or the panel loses it mid-way): - # 1. set both vars here and restart the agent; - # 2. in the panel, change the node's API URL to http://:2021; - # 3. re-bind Caddy's admin to 127.0.0.1: drop the ":2019:2019" - # port mapping from the caddy service and set `admin 127.0.0.1:2019` - # in its Caddyfile, then restart Caddy; - # 4. confirm with `server doctor` that the node's admin API is reachable. - # HPG_ADMIN_PROXY_LISTEN: "10.66.0.2:2021" # this node's WG IP; never 0.0.0.0 - # HPG_ADMIN_PROXY_KEY: "${HPG_ADMIN_PROXY_KEY}" # from the panel flash, one-shot - # HPG_CADDY_ADMIN_URL: "http://127.0.0.1:2019" # where Caddy's admin listens + # Migrating an EXISTING node (order matters, or the panel loses it): + # 1. set these three vars and restart the agent; + # 2. in the panel, change the node's API URL to http://:2021 + # and confirm `server doctor` still reaches it; + # 3. only then republish Caddy's admin on loopback: change the caddy + # service's port mapping from ":2019:2019" to + # "127.0.0.1:2019:2019" and restart Caddy; + # 4. re-run `server doctor` - the "admin API auth" row must be PASS. + HPG_ADMIN_PROXY_LISTEN: "10.66.0.2:2021" # this node's WG IP; never 0.0.0.0 + HPG_ADMIN_PROXY_KEY: "${HPG_ADMIN_PROXY_KEY}" # from the panel flash, one-shot + HPG_CADDY_ADMIN_URL: "http://127.0.0.1:2019" # Caddy's admin, published on host loopback volumes: # Shared with caddy: agent reads the rolling access log Caddy writes. - /opt/hostyt-node/caddy-logs:/var/log/caddy:ro + # Shared with caddy (read-only there): the agent syncs the GeoIP mmdb + # from the panel to the path Caddy's maxmind matcher reads (OPS-005). + - /opt/hostyt-node/geoip:/data/geoip # Writable state: keeps forward offsets across restarts. - /opt/hostyt-node/agent-state:/var/lib/hpg-node-agent # Healthcheck: agent prints reconcile JSON every poll; if no log diff --git a/deploy/remote-node/Caddyfile.bootstrap b/deploy/remote-node/Caddyfile.bootstrap index c8088715..d00d53b2 100644 --- a/deploy/remote-node/Caddyfile.bootstrap +++ b/deploy/remote-node/Caddyfile.bootstrap @@ -3,9 +3,11 @@ # is superseded by the JSON config pushed via the Admin API. { - # Admin API listens on the WireGuard IP only - never publicly. - # Edit the IP below to match this node's wg0 address. - admin 10.66.0.2:2019 + # 0.0.0.0 here is the CONTAINER's namespace, not the host: the compose file + # publishes this port on host loopback only and the node-agent fronts it + # with authentication (SEC-002). A WireGuard IP cannot be bound here at all + # - it lives on the host, outside this container's netns. + admin 0.0.0.0:2019 email {$ACME_EMAIL} diff --git a/deploy/remote-node/docker-compose.yml b/deploy/remote-node/docker-compose.yml index 6960e1f0..c0b0737e 100644 --- a/deploy/remote-node/docker-compose.yml +++ b/deploy/remote-node/docker-compose.yml @@ -1,5 +1,8 @@ # Remote Caddy node - runs on a VPS connected to the control plane over -# WireGuard. Admin API is bound to the WG interface only. +# WireGuard. Caddy's admin API is published on host loopback only; the +# hpg-node-agent sidecar (deploy/node-agent/docker-compose.example.yml) fronts +# it with an authenticated proxy on the WireGuard IP. Deploy BOTH - the panel +# refuses to register or approve a node whose api_url is a raw remote :2019. # # Do NOT run this on the same host as deploy/docker-compose.yml - that # stack's bundled `caddy` service already owns 80/443/443udp there and @@ -11,21 +14,28 @@ # See docs/MULTI_NODE.md. # 2. Public ports 80/443/443udp open. UDP 51820 open for WG. # 3. /opt/hostyt-node/ contains this file + Caddyfile.bootstrap. +# 4. mkdir -p /opt/hostyt-node/{caddy-logs,geoip,agent-state} - shared with +# the agent, which writes the GeoIP mmdb and reads the access log. +# 5. The hpg-node-agent stack is up with HPG_ADMIN_PROXY_LISTEN/_KEY set. # # Then: # cd /opt/hostyt-node && docker compose up -d services: caddy: - # Stock Caddy used only as fallback for nodes that don't run the - # custom cache-handler build. For cache_enabled routes the node MUST - # run ghcr.io/host-yt/caddy-proxy-manager-edge (deploy/caddy/Dockerfile). - image: ${IMAGE_CADDY:-caddy:2.11.4} + # The custom edge image is the default (OPS-005): stock caddy:2.11.4 has + # no cache-handler, coraza, caddy-l4, rate_limit or maxmind-geolocation, + # so cache_enabled / WAF / streams / GeoIP routes are rejected or silently + # absent there. Override with IMAGE_CADDY=caddy:2.11.4 for a plain node. + image: ${IMAGE_CADDY:-ghcr.io/host-yt/caddy-proxy-manager-edge:1.5.1} restart: unless-stopped - # IMPORTANT: bind :2019 to the WireGuard IP only. Public ports - # (80/443/443-udp) are bound on 0.0.0.0 as usual. + # SEC-002: :2019 is Caddy's admin API and it authenticates NOTHING, so it + # is published on host loopback only. The host-networked hpg-node-agent + # reaches it there and fronts it with an authenticated proxy on the + # WireGuard IP (:2021) - that is what the panel's api_url must point at. + # Never republish this on a WireGuard or public address. ports: - - "10.66.0.2:2019:2019" # ← change to this node's wg IP + - "127.0.0.1:2019:2019" - "80:80" - "443:443" - "443:443/udp" @@ -43,6 +53,9 @@ services: # Access logs: shared with hpg-node-agent, which tails + forwards them to # the panel. Host bind path so the host-networked agent can read it too. - /opt/hostyt-node/caddy-logs:/var/log/caddy + # GeoIP mmdb the node-agent syncs from the panel; Caddy's maxmind + # matcher reads it. Read-only here: only the agent writes (OPS-005). + - /opt/hostyt-node/geoip:/data/geoip:ro environment: ASK_ENDPOINT_URL: ${ASK_ENDPOINT_URL:-http://10.66.0.1:8080/internal/ask} ACME_EMAIL: ${ACME_EMAIL:-ops@example.com} diff --git a/docs/MULTI_NODE.md b/docs/MULTI_NODE.md index b028e9a4..958eb25d 100644 --- a/docs/MULTI_NODE.md +++ b/docs/MULTI_NODE.md @@ -14,8 +14,10 @@ WireGuard mesh: - Manager gets a private WG IP (`10.66.0.1` by default). - Each remote node gets a unique private WG IP (`10.66.0.2`, `.3`, etc.). -- Caddy Admin API on every node binds to its WG IP only, never to - `0.0.0.0`. +- Caddy's Admin API is published on the node's host loopback only, and the + node-agent fronts it with an authenticated proxy on the node's WG IP + (`:2021`) - see Section 12. Legacy nodes that publish `:2019:2019` + directly still work, but `server doctor` warns about them. - All `/load`, `/config`, `/reverse_proxy` and metrics calls from the manager travel over the encrypted WG tunnel, not over the public internet. - Public traffic (HTTP/HTTPS) arrives at the node directly; WG carries @@ -296,8 +298,9 @@ wg-quick up wg0 ### Step 4 - Write Caddy compose and Caddyfile -Creates `/opt/hostyt-node/docker-compose.yml` binding Caddy's Admin API to the -WG IP only: +Creates `/opt/hostyt-node/docker-compose.yml` publishing Caddy's Admin API on +the node's WG IP (the legacy direct path - the script prints the steps to close +it, see Section 12): ``` - "10.66.0.3:2019:2019" # WG IP only - never public @@ -310,7 +313,7 @@ Creates `/opt/hostyt-node/Caddyfile.bootstrap`: ```caddyfile { - admin 10.66.0.3:2019 + admin 0.0.0.0:2019 email ops@example.com on_demand_tls { ask http://10.66.0.1:8080/internal/ask @@ -391,12 +394,12 @@ ping -c3 10.66.0.1 # should reach the manager ### 7.4 Write Caddy compose Create `/opt/hostyt-node/docker-compose.yml` using the template from -`deploy/remote-node/docker-compose.yml`. Change the Admin API bind address to -this node's WG IP: +`deploy/remote-node/docker-compose.yml`. Keep the Admin API on host loopback +and let the node-agent front it (Section 12): ```yaml ports: - - "10.66.0.X:2019:2019" # replace X with this node's last octet + - "127.0.0.1:2019:2019" # node-agent proxies it, authenticated, on :2021 - "80:80" - "443:443" - "443:443/udp" @@ -426,7 +429,8 @@ extra_hosts: ```bash cat > /opt/hostyt-node/Caddyfile.bootstrap < on_demand_tls { ask @@ -599,22 +603,22 @@ was still holding. Revoked peers in the group are left alone. ### Node shows offline in the panel 1. Verify WG handshake is up (step above). -2. Check the Caddy Admin API is reachable from the manager over WG: +2. Check the node's admin endpoint is reachable from the manager over WG: ```bash - # On manager: - curl http://10.66.0.X:2019/config/ + # On manager - :2021 is the node-agent admin proxy (401 without the key is + # the healthy answer); :2019 only on a legacy, unmigrated node. + curl -o /dev/null -w '%{http_code}\n' http://10.66.0.X:2021/config/ ``` - `connection refused` - Caddy is not running on the node, or it is bound to - the wrong IP. + `connection refused` - the agent (or Caddy) is not running on the node. 3. Check Caddy is running on the node: ```bash # On node: cd /opt/hostyt-node && docker compose ps docker compose logs caddy ``` -4. Confirm the Admin API bind in `Caddyfile.bootstrap` matches the node's WG - IP. It must be `admin 10.66.0.X:2019`, not `admin localhost:2019` or - `admin :2019`. +4. Confirm the Admin API bind in `Caddyfile.bootstrap` is `admin 0.0.0.0:2019` + - that is the container's namespace. A WG address cannot be bound from + inside a bridge container; the `ports:` mapping decides reachability. ### Node is approved but receives no routes / domains return 503 @@ -823,12 +827,19 @@ panel: ## 12. Authenticating the node's admin API -Caddy's admin API has no authentication of its own. In the default topology it -is published on the node's WireGuard address, so anything that can route to -`:2019` can replace that node's entire configuration - every tenant on -it. See [SECURITY.md](SECURITY.md#caddy-admin-api---known-limitation). +Caddy's admin API has no authentication of its own. Published on a node's +WireGuard address, anything that can route to `:2019` can replace that +node's entire configuration - every tenant on it. See +[SECURITY.md](SECURITY.md#caddy-admin-api---known-limitation). + +**This is now required for remote nodes.** `deploy/remote-node/docker-compose.yml` +publishes Caddy's admin port on host loopback only, the node-agent example has +the proxy switched on, and the panel refuses to register a node whose API URL is +a raw remote `:2019` (`/admin/nodes` and `POST /api/v1/nodes`). A fleet that is +mid-migration can set `HPG_ALLOW_UNAUTHENTICATED_NODE_ADMIN=1` on the panel to +restore the old behaviour; `server doctor` warns for every node still on it. -A node can close that by putting its **node-agent in front of the admin API**: +The node-agent goes **in front of the admin API**: ``` panel ──(bearer key, over the WG mesh)──▶ node-agent :2021 ──(127.0.0.1)──▶ Caddy admin :2019 @@ -862,9 +873,12 @@ Order matters: the panel must be able to reach the node at every step. 3. **Point the panel at the agent.** Edit the node in `/admin/nodes` and set its API URL to `http://10.66.0.2:2021`. Pushes now carry the key. -4. **Close the direct port.** In the node's compose, drop the - `"10.66.0.2:2019:2019"` mapping from the `caddy` service, and set - `admin 127.0.0.1:2019` in its Caddyfile. Restart Caddy. +4. **Close the direct port.** In the node's compose, change the `caddy` + service's mapping from `"10.66.0.2:2019:2019"` to `"127.0.0.1:2019:2019"` + and restart Caddy. Leave `admin 0.0.0.0:2019` in the Caddyfile: that is the + *container's* namespace, and the host-networked agent reaches the published + loopback port. Binding `127.0.0.1` inside the container would hide the admin + API from the agent too. 5. **Verify.** `docker compose exec app /app/server doctor` should still report the node's admin API as reachable, and a **Resync** from the panel should diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 6e04b74e..40f52324 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -385,10 +385,12 @@ The security model is therefore **network reachability only**: - On the manager stack, `:2019` is reachable inside the compose network (`CADDY_ADMIN_URL: http://caddy:2019`) and the compose file deliberately never publishes the port to the host. -- On a remote node it is published on the node's WireGuard address - - `deploy/remote-node/docker-compose.yml` binds `":2019:2019"` (the - shipped example is `10.66.0.2:2019:2019`). It is reachable from anything on - the control-plane mesh. +- On a remote node it is published on **host loopback only** + (`deploy/remote-node/docker-compose.yml` binds `"127.0.0.1:2019:2019"`), and + the node-agent fronts it with an authenticated proxy on the node's WireGuard + address (`:2021`). Nodes deployed before 1.5.2 published `":2019:2019"` + and are reachable from anything on the control-plane mesh until migrated - + `server doctor` flags each one. Two of the critical findings closed in 1.4.4/1.4.5 were paths into that API from tenant-controlled configuration, not from the network: @@ -423,11 +425,20 @@ panel ──(bearer key, over the WG mesh)──▶ node-agent ──(127.0.0.1) - It refuses to start bound to `0.0.0.0`, and refuses a key shorter than 32 characters, rather than serving something that only looks authenticated. -Turning it on is per node and opt-in (`HPG_ADMIN_PROXY_LISTEN` + -`HPG_ADMIN_PROXY_KEY` on the agent, then point the node's API URL at the agent -and re-bind Caddy's admin to `127.0.0.1`) - see -[MULTI_NODE.md](MULTI_NODE.md#12-authenticating-the-nodes-admin-api). A node -without the key is reached directly, exactly as before. +It is required for remote nodes: the panel refuses to register a node whose +API URL is a raw remote `:2019`, in `/admin/nodes` and in `POST /api/v1/nodes`. +Set it up with `HPG_ADMIN_PROXY_LISTEN` + `HPG_ADMIN_PROXY_KEY` on the agent, +then point the node's API URL at the agent and publish Caddy's admin port on +host loopback - see +[MULTI_NODE.md](MULTI_NODE.md#12-authenticating-the-nodes-admin-api) for the +order, which matters. + +Existing fleets are not cut off: nothing changes for nodes already registered, +the auto-join script still onboards a node on the direct path (its agent, and +therefore its key, only exists after tunnel-enable), and +`HPG_ALLOW_UNAUTHENTICATED_NODE_ADMIN=1` on the panel re-opens registration for +a fleet mid-migration. `server doctor` prints an `admin API auth` row per node +so the remaining ones are visible. **Still outstanding.** Until a node is migrated, treat reachability of `:2019` as equivalent to root on that node and keep the control-plane diff --git a/internal/httpserver/handlers/admin.go b/internal/httpserver/handlers/admin.go index 460af6f9..2323a801 100644 --- a/internal/httpserver/handlers/admin.go +++ b/internal/httpserver/handlers/admin.go @@ -842,6 +842,12 @@ func (h *AdminHandlers) NodesCreate(w http.ResponseWriter, r *http.Request) { redirectWithFlash(w, r, "/admin/nodes", "", "api_url must start with http:// or https://") return } + // SEC-002: a remote node must be reached through the agent's authenticated + // admin proxy, never Caddy's own unauthenticated :2019. + if err := security.RejectUnauthenticatedNodeAdminURL(apiURL); err != nil { + redirectWithFlash(w, r, "/admin/nodes", "", err.Error()) + return + } if publicIP != "" && net.ParseIP(publicIP) == nil { redirectWithFlash(w, r, "/admin/nodes", "", "public_ip is not a valid IP") return @@ -1509,6 +1515,13 @@ func (h *AdminHandlers) NodesApprove(w http.ResponseWriter, r *http.Request) { } ctx, cancel := context.WithTimeout(r.Context(), 5_000_000_000) defer cancel() + // SEC-002: the auto-join script registers http://:2019 - Caddy's + // unauthenticated admin API - and the node-agent that fronts it is only + // installed after approval (tunnel-enable mints its key). Approval is + // therefore the first place an operator can be told, but blocking it would + // break the documented onboarding order, so this warns instead. + var joinURL string + _ = db.QueryRowContext(ctx, "SELECT api_url FROM caddy_nodes WHERE id = ?", id).Scan(&joinURL) if _, err := db.ExecContext(ctx, "UPDATE caddy_nodes SET is_enabled = 1, approved_at = NOW(), approved_by = ? WHERE id = ? AND approved_at IS NULL", approvedBy, id); err != nil { @@ -1523,7 +1536,12 @@ func (h *AdminHandlers) NodesApprove(w http.ResponseWriter, r *http.Request) { UserID: actorUserID(sess), Action: "node.approve", Entity: "node", EntityID: fmt.Sprintf("%d", id), }) - redirectWithFlash(w, r, "/admin/nodes", "Node approved", "") + msg := "Node approved" + if security.UnauthenticatedNodeAdminURL(joinURL) { + msg += ". This node is reached over Caddy's unauthenticated admin API - " + + "enable the tunnel, run the node-agent admin proxy, then repoint its API URL at :2021 (docs/MULTI_NODE.md)" + } + redirectWithFlash(w, r, "/admin/nodes", msg, "") } // NodesResync rebuilds the node's full Caddy config from DB and POSTs /load. diff --git a/internal/httpserver/handlers/api_v1.go b/internal/httpserver/handlers/api_v1.go index c0eb187a..be30eda7 100644 --- a/internal/httpserver/handlers/api_v1.go +++ b/internal/httpserver/handlers/api_v1.go @@ -780,6 +780,11 @@ func (h *APIHandlers) NodeCreate(w http.ResponseWriter, r *http.Request) { apiErr(w, http.StatusBadRequest, "api_url host not allowed (loopback/link-local/metadata)") return } + // SEC-002: remote nodes go through the agent's authenticated admin proxy. + if err := security.RejectUnauthenticatedNodeAdminURL(in.APIURL); err != nil { + apiErr(w, http.StatusBadRequest, err.Error()) + return + } ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second) defer cancel() var pubIP sql.NullString diff --git a/internal/security/nodeadmin.go b/internal/security/nodeadmin.go new file mode 100644 index 00000000..492eba6b --- /dev/null +++ b/internal/security/nodeadmin.go @@ -0,0 +1,48 @@ +package security + +import ( + "fmt" + "net" + "net/url" + "os" + "strings" +) + +// caddyAdminPort is Caddy's admin API default port. That API has no +// authentication of any kind, so being able to open a TCP connection to it is +// equivalent to root on the node. +const caddyAdminPort = "2019" + +// AllowUnauthenticatedNodeAdminEnv re-opens the legacy path for a fleet that is +// mid-migration to the node-agent admin proxy. +const AllowUnauthenticatedNodeAdminEnv = "HPG_ALLOW_UNAUTHENTICATED_NODE_ADMIN" + +// UnauthenticatedNodeAdminURL reports whether apiURL addresses a raw Caddy +// admin endpoint on a remote machine (SEC-002). +// +// Only a literal, non-loopback IP on :2019 counts. The manager's own bundled +// Caddy is addressed by compose service name (http://caddy:2019) on a bridge +// that publishes nothing, and a node-agent admin proxy listens on a different +// port - neither is flagged. +func UnauthenticatedNodeAdminURL(apiURL string) bool { + u, err := url.Parse(strings.TrimSpace(apiURL)) + if err != nil || u.Host == "" || u.Port() != caddyAdminPort { + return false + } + ip := net.ParseIP(u.Hostname()) + return ip != nil && !ip.IsLoopback() +} + +// RejectUnauthenticatedNodeAdminURL refuses to register a node against a raw +// remote Caddy admin API. Remote nodes must be reached through the node-agent +// admin proxy, which authenticates the panel with a per-node key. +func RejectUnauthenticatedNodeAdminURL(apiURL string) error { + if !UnauthenticatedNodeAdminURL(apiURL) || os.Getenv(AllowUnauthenticatedNodeAdminEnv) == "1" { + return nil + } + host, _, _ := net.SplitHostPort(strings.TrimPrefix(strings.TrimPrefix(apiURL, "http://"), "https://")) + return fmt.Errorf("api_url points straight at Caddy's unauthenticated admin API on :%s - "+ + "run the node-agent admin proxy on that node (HPG_ADMIN_PROXY_LISTEN/HPG_ADMIN_PROXY_KEY) "+ + "and register http://%s:2021 instead, or set %s=1 to keep the legacy path while migrating "+ + "(see docs/MULTI_NODE.md)", caddyAdminPort, host, AllowUnauthenticatedNodeAdminEnv) +} diff --git a/internal/security/nodeadmin_test.go b/internal/security/nodeadmin_test.go new file mode 100644 index 00000000..4fdb0d6a --- /dev/null +++ b/internal/security/nodeadmin_test.go @@ -0,0 +1,39 @@ +package security + +import "testing" + +func TestUnauthenticatedNodeAdminURL(t *testing.T) { + cases := []struct { + url string + want bool + }{ + {"http://10.66.0.2:2019", true}, // the shipped remote-node topology + {"http://10.66.0.2:2021", false}, // node-agent admin proxy + {"http://caddy:2019", false}, // compose-local, bridge only + {"http://127.0.0.1:2019", false}, // node-local, agent side + {"http://[::1]:2019", false}, // IPv6 loopback + {"http://[fd00::2]:2019", true}, // remote IPv6 + {"http://10.66.0.2", false}, // not the admin port + {"https://node.example.com", false}, // hostname, no admin port + {"", false}, // caller validates shape + {"::not a url", false}, // + } + for _, c := range cases { + if got := UnauthenticatedNodeAdminURL(c.url); got != c.want { + t.Errorf("UnauthenticatedNodeAdminURL(%q) = %v, want %v", c.url, got, c.want) + } + } +} + +func TestRejectUnauthenticatedNodeAdminURL(t *testing.T) { + if err := RejectUnauthenticatedNodeAdminURL("http://10.66.0.2:2019"); err == nil { + t.Fatal("expected a raw remote admin URL to be refused") + } + if err := RejectUnauthenticatedNodeAdminURL("http://10.66.0.2:2021"); err != nil { + t.Fatalf("admin-proxy URL must be accepted: %v", err) + } + t.Setenv(AllowUnauthenticatedNodeAdminEnv, "1") + if err := RejectUnauthenticatedNodeAdminURL("http://10.66.0.2:2019"); err != nil { + t.Fatalf("escape hatch must restore the legacy path: %v", err) + } +} diff --git a/scripts/node-join.sh b/scripts/node-join.sh index 7d96693c..41187102 100755 --- a/scripts/node-join.sh +++ b/scripts/node-join.sh @@ -160,6 +160,9 @@ services: image: caddy:2.11.4 restart: unless-stopped ports: + # SEC-002: Caddy's admin API authenticates nothing, so whoever can route + # to this address owns the node. Published on the WG address for the + # legacy direct path; see the migration steps this script prints. - "${admin_listen}:2019" - "80:80" - "443:443" @@ -177,9 +180,12 @@ volumes: EOF log "Writing $INSTALL_DIR/Caddyfile.bootstrap" +# admin binds 0.0.0.0 in the CONTAINER's namespace - the WG address lives on +# the host and cannot be bound from inside a bridge container. Reachability is +# decided by the port mapping above, not by this line. cat > "$INSTALL_DIR/Caddyfile.bootstrap" < Date: Mon, 21 Sep 2026 09:49:19 +0200 Subject: [PATCH 02/12] docs(ha): drop the shared certificate store claim, document per-node issuance README and the placement comment promised Redis-backed shared cert storage (caddy-tlsredis), but the edge image builds no TLS-storage module, the Caddyfile line was commented out, and every node has its own caddy_data. The claim was chosen for removal rather than implementation: shared TLS storage holds private keys, needs Redis reachable from every remote node over the mesh, and fails closed on a Redis outage - a much larger availability and blast-radius change than the failover latency it saves. MULTI_NODE gains a "Certificates on failover" section stating what actually happens: the peer issues its own certificate on the first handshake it serves, so plan ACME rate limits for nodes x hosts. The Caddyfile comment now warns that adding a `storage` directive breaks config load instead of inviting it. (cherry picked from commit c2bbd3743c8c632fdefdd5e63eabef81cbb91cea) --- README.md | 6 +++--- deploy/caddy/Caddyfile | 5 +++-- docs/MULTI_NODE.md | 23 +++++++++++++++++++++++ internal/domain/routes/placement.go | 7 +++++-- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index f9ed9016..c4d447d7 100644 --- a/README.md +++ b/README.md @@ -37,9 +37,9 @@ Single binary ~21 MB image, ~28 MB idle RAM. hosts across the group automatically. Customer/origin backends reach the fleet over a dedicated WireGuard tunnel that falls back to WSS (WebSocket-over-TLS) when UDP is blocked, so backends behind NAT or a - restrictive firewall stay reachable. Certificate storage can be shared - across a group (Redis-backed) so a failover or active_active peer already - holds the cert before it needs to serve traffic. Node health is scraped + restrictive firewall stay reachable. Each node keeps its own certificate + store and issues on demand, so a peer taking over a host issues its own + certificate on the first TLS handshake it serves. Node health is scraped from Caddy's Prometheus endpoint, and automatic failover moves routes off a dead node onto a healthy sibling in the same group. diff --git a/deploy/caddy/Caddyfile b/deploy/caddy/Caddyfile index cf29afa7..056f260c 100644 --- a/deploy/caddy/Caddyfile +++ b/deploy/caddy/Caddyfile @@ -22,8 +22,9 @@ # burst 5 } - # Storage: filesystem by default. Switch to Redis module for HA later. - # storage redis { host redis:6379 password "{$REDIS_PASSWORD}" } + # Storage: filesystem, per node. The image builds no shared-storage + # module (see deploy/caddy/Dockerfile), so do NOT uncomment a `storage` + # directive here - Caddy would reject the whole config at load. # Use staging while testing to avoid burning Let's Encrypt rate limit. # acme_ca https://acme-staging-v02.api.letsencrypt.org/directory diff --git a/docs/MULTI_NODE.md b/docs/MULTI_NODE.md index 958eb25d..2c10adb8 100644 --- a/docs/MULTI_NODE.md +++ b/docs/MULTI_NODE.md @@ -529,6 +529,29 @@ to another peer in the same group without requiring manual intervention. whether IP forwarding and iptables/nftables rules are correctly set on each node. +### Certificates on failover + +**Certificate storage is per node.** Each node keeps its own `caddy_data` +volume and there is no shared store: the edge image (`deploy/caddy/Dockerfile`) +builds no TLS-storage module, so do not add a `storage` directive to a node's +Caddyfile - Caddy would reject the whole config at load. + +What that means in practice: + +- A route on an `active_active`/`failover` group is pushed to every node in the + group, so a peer can answer the moment traffic arrives - but it holds no + certificate for that host until it serves a handshake itself. +- The first TLS handshake a peer serves triggers on-demand issuance, gated by + the panel's `ask` endpoint. That is one ACME order per node per host, and it + costs the client a slow first handshake (seconds), not an error - unless the + CA's rate limit has been hit. +- Budget ACME rate limits for `nodes x hosts`, not `hosts`. Let's Encrypt's + 50 certificates/registered-domain/week is the one that bites on a large + active_active group; move to a CA with a higher limit or a wildcard via + DNS-01 (Section on DNS providers in [DNS_PROVIDERS.md](DNS_PROVIDERS.md)) if you are near it. +- To pre-warm a peer before a planned failover, resolve the host to that node + and complete one HTTPS request against it. + **Automatic failover** is implemented. When `failover.auto_enabled` is set (`Admin → Settings → Failover`), the alert evaluator moves active routes from a dead node to a healthy sibling in the same `mode=failover` node group and diff --git a/internal/domain/routes/placement.go b/internal/domain/routes/placement.go index 4a45b326..f9d6b548 100644 --- a/internal/domain/routes/placement.go +++ b/internal/domain/routes/placement.go @@ -13,8 +13,11 @@ import ( // active_active → every enabled+approved node in the group with capacity. // failover → primary (highest priority enabled+approved+healthy); // warm-secondary (next-highest) tracked for future -// promotion. We deploy to both so the cert exists when -// we need it (caddy-tlsredis shares cert storage). +// promotion. We deploy the route to both so the secondary +// can answer immediately; certificate storage is per node +// (the shipped image has no shared-storage module), so the +// secondary issues its own cert on the first handshake it +// serves. See docs/MULTI_NODE.md "Certificates on failover". // // Capacity check uses current_routes < max_routes per node. func nodePlacement(ctx context.Context, db *sql.DB, groupID int64) (primary int64, all []int64, mode string, err error) { From 52ab064c6847e02e9a5103c363abc544f74a55c4 Mon Sep 17 00:00:00 2001 From: marcoome Date: Mon, 21 Sep 2026 09:50:11 +0200 Subject: [PATCH 03/12] build(deploy): bump lite/Portainer pins to 1.5.1 and gate drift in CI The lite and Portainer profiles still shipped 1.3.2, so an operator who picked either got a panel missing every 1.4/1.5 fix. Bumped, plus two guards in the existing CI policy block: every ghcr.io/host-yt pin under deploy/ must name one version, and the README status line must be that version. `make pin-version V=x.y.z` rewrites all of them at once so the bump cannot be partial. (cherry picked from commit 6c978d8104ffaf8b4b0da5f69c131c4ac6108813) --- .github/workflows/ci.yml | 10 ++++++++++ Makefile | 8 ++++++++ deploy/docker-compose.lite.yml | 6 +++--- deploy/portainer-external-db.yml | 6 +++--- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9a186bf1..72bf654b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,6 +58,16 @@ jobs: deploy/remote-node/docker-compose.yml scripts/node-join.sh docs/ README.md | sort -u || true) if [ "$(printf '%s' "$caddy_pins" | grep -c .)" -ne 1 ]; then echo "::error::caddy image pins disagree: $(echo $caddy_pins)"; fail=1; fi + # OPS-002: every own-image pin across deploy/ must name one version, + # or an alternative profile silently deploys an older panel. + hpg_pins=$(grep -rhoE 'ghcr\.io/host-yt/[a-z-]+:[0-9]+\.[0-9]+\.[0-9]+' deploy/ \ + | grep -oE '[0-9]+\.[0-9]+\.[0-9]+$' | sort -u) + if [ "$(printf '%s' "$hpg_pins" | grep -c .)" -ne 1 ]; then + echo "::error::hpg image pins in deploy/ disagree: $(echo $hpg_pins) (run: make pin-version V=x.y.z)"; fail=1; fi + # The README status line names the same version operators will deploy. + readme_ver=$(grep -oE '^\*\*Status:\*\* v[0-9]+\.[0-9]+\.[0-9]+' README.md | grep -oE '[0-9]+\.[0-9]+\.[0-9]+') + if [ "$readme_ver" != "$hpg_pins" ]; then + echo "::error::README status v$readme_ver != deploy/ image pins $hpg_pins"; fail=1; fi # The node image and the customer install script must ship the same # wstunnel, or a bump upgrades one side of the tunnel only. wst_vers=$( { grep -hoE 'WSTUNNEL_VERSION=[0-9]+\.[0-9]+\.[0-9]+' deploy/node-agent/Dockerfile; \ diff --git a/Makefile b/Makefile index 1b87dcd5..ce4d256e 100644 --- a/Makefile +++ b/Makefile @@ -87,6 +87,14 @@ build: gen build-css ## Build server binary (CSS first so it embeds). run: build-css ## Run locally (loads .env). $(GO) run ./cmd/server +.PHONY: pin-version +pin-version: ## Rewrite every deploy/ image pin + README status to V=x.y.z (single version source). + @test -n "$(V)" || { echo "usage: make pin-version V=1.5.2"; exit 1; } + @grep -rlE 'ghcr\.io/host-yt/[a-z-]+:[0-9]+\.[0-9]+\.[0-9]+' deploy/ \ + | xargs perl -pi -e 's|(ghcr\.io/host-yt/[a-z-]+):\d+\.\d+\.\d+|$$1:$(V)|g' + @perl -pi -e 's|^\*\*Status:\*\* v\d+\.\d+\.\d+|**Status:** v$(V)|' README.md + @echo "pinned $(V); CI fails if anything still disagrees" + .PHONY: dev dev: build-css ## Hot-reload dev (air + templ watcher). air diff --git a/deploy/docker-compose.lite.yml b/deploy/docker-compose.lite.yml index cf30eb1a..3d66351f 100644 --- a/deploy/docker-compose.lite.yml +++ b/deploy/docker-compose.lite.yml @@ -22,7 +22,7 @@ services: app: - image: ${IMAGE_APP:-ghcr.io/host-yt/caddy-proxy-manager:1.3.2} + image: ${IMAGE_APP:-ghcr.io/host-yt/caddy-proxy-manager:1.5.1} pull_policy: ${IMAGE_PULL_POLICY:-if_not_present} restart: unless-stopped security_opt: @@ -139,7 +139,7 @@ services: # access-log dashboard + analytics rollups) and runs the customer tunnel. # No WAF audit log in lite (WAF is off); the access log is enough. hpg-node-agent: - image: ${IMAGE_NODE_AGENT:-ghcr.io/host-yt/caddy-proxy-manager-node-agent:1.3.2} + image: ${IMAGE_NODE_AGENT:-ghcr.io/host-yt/caddy-proxy-manager-node-agent:1.5.1} pull_policy: ${IMAGE_PULL_POLICY:-if_not_present} restart: unless-stopped network_mode: host @@ -171,7 +171,7 @@ services: - node wireguard: - image: ${IMAGE_WG:-ghcr.io/host-yt/caddy-proxy-manager-wg:1.3.2} + image: ${IMAGE_WG:-ghcr.io/host-yt/caddy-proxy-manager-wg:1.5.1} pull_policy: ${IMAGE_PULL_POLICY:-if_not_present} restart: unless-stopped cap_add: diff --git a/deploy/portainer-external-db.yml b/deploy/portainer-external-db.yml index 09cd6f91..4669b1b0 100644 --- a/deploy/portainer-external-db.yml +++ b/deploy/portainer-external-db.yml @@ -23,7 +23,7 @@ services: - internal app: - image: ghcr.io/host-yt/caddy-proxy-manager:1.3.2 + image: ghcr.io/host-yt/caddy-proxy-manager:1.5.1 pull_policy: if_not_present restart: unless-stopped environment: @@ -82,7 +82,7 @@ services: retries: 5 caddy: - image: ghcr.io/host-yt/caddy-proxy-manager-edge:1.3.2 + image: ghcr.io/host-yt/caddy-proxy-manager-edge:1.5.1 pull_policy: if_not_present restart: unless-stopped cap_add: @@ -109,7 +109,7 @@ services: - internal hpg-node-agent: - image: ghcr.io/host-yt/caddy-proxy-manager-node-agent:1.3.2 + image: ghcr.io/host-yt/caddy-proxy-manager-node-agent:1.5.1 pull_policy: if_not_present restart: unless-stopped network_mode: host From 210a64eced11721dafdda168e5b1abbd056218d6 Mon Sep 17 00:00:00 2001 From: marcoome Date: Mon, 21 Sep 2026 09:52:01 +0200 Subject: [PATCH 04/12] ops(compose): healthcheck app and Caddy MariaDB, Redis and the node-agent had healthchecks; the two services that matter did not, so a wedged-but-running panel or Caddy stayed "running". The runtime image is distroless - no shell, curl or wget - so the app probes itself: `server healthcheck` GETs /readyz over loopback and exits 0/1. Caddy uses the local admin endpoint rather than :80/:443, which legitimately 503s before the first route is pushed. start_period is 300s for the app (a first boot runs every migration before /readyz turns green) and 60s for Caddy; an ACME order in flight must never read as wedged. Compose reports health but does not restart on it - that needs an external supervisor. (cherry picked from commit 1acff05ef575345810fb19816e5c04f1540e8e6f) --- cmd/server/doctor.go | 32 ++++++++++++++++++++++++++++++++ cmd/server/main.go | 5 +++++ deploy/docker-compose.lite.yml | 12 ++++++++++++ deploy/docker-compose.yml | 19 +++++++++++++++++++ deploy/portainer-external-db.yml | 12 ++++++++++++ 5 files changed, 80 insertions(+) diff --git a/cmd/server/doctor.go b/cmd/server/doctor.go index 53bbb0c3..8af5eb99 100644 --- a/cmd/server/doctor.go +++ b/cmd/server/doctor.go @@ -7,7 +7,9 @@ import ( "context" "database/sql" "fmt" + "io" "net" + "net/http" "os" "os/exec" "text/tabwriter" @@ -353,3 +355,33 @@ func summarize(checks []check) int { } return 0 } + +// runHealthcheck probes the panel's own /readyz over loopback and returns a +// process exit code. This is the container HEALTHCHECK: the runtime image is +// distroless, so no shell, curl or wget exists to do it from Compose. +func runHealthcheck() int { + bind := os.Getenv("APP_BIND") + if bind == "" { + bind = "0.0.0.0:8080" + } + _, port, err := net.SplitHostPort(bind) + if err != nil { + fmt.Fprintf(os.Stderr, "healthcheck: APP_BIND %q is not host:port\n", bind) + return 1 + } + ctx, cancel := context.WithTimeout(context.Background(), 4*time.Second) + defer cancel() + req, _ := http.NewRequestWithContext(ctx, http.MethodGet, "http://127.0.0.1:"+port+"/readyz", nil) + resp, err := (&http.Client{}).Do(req) + if err != nil { + fmt.Fprintln(os.Stderr, "healthcheck:", err) + return 1 + } + defer resp.Body.Close() + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 4096)) + if resp.StatusCode != http.StatusOK { + fmt.Fprintf(os.Stderr, "healthcheck: /readyz returned %d\n", resp.StatusCode) + return 1 + } + return 0 +} diff --git a/cmd/server/main.go b/cmd/server/main.go index 4f8c52da..b83b8fda 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -80,6 +80,11 @@ func main() { if len(os.Args) > 1 && os.Args[1] == "doctor" { os.Exit(runDoctor()) } + // "server healthcheck" is the container HEALTHCHECK: the image is + // distroless, so there is no curl/wget to probe /readyz with. + if len(os.Args) > 1 && os.Args[1] == "healthcheck" { + os.Exit(runHealthcheck()) + } doctor := flag.Bool("doctor", false, "run preflight diagnostics and exit") flag.Parse() if *doctor { diff --git a/deploy/docker-compose.lite.yml b/deploy/docker-compose.lite.yml index 3d66351f..432271d7 100644 --- a/deploy/docker-compose.lite.yml +++ b/deploy/docker-compose.lite.yml @@ -25,6 +25,12 @@ services: image: ${IMAGE_APP:-ghcr.io/host-yt/caddy-proxy-manager:1.5.1} pull_policy: ${IMAGE_PULL_POLICY:-if_not_present} restart: unless-stopped + healthcheck: + test: ["CMD", "/app/server", "healthcheck"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 300s security_opt: - no-new-privileges:true environment: @@ -115,6 +121,12 @@ services: image: caddy:2.11.4 pull_policy: ${IMAGE_PULL_POLICY:-if_not_present} restart: unless-stopped + healthcheck: + test: ["CMD", "wget", "-q", "-O", "/dev/null", "http://127.0.0.1:2019/config/"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 60s cap_add: - NET_BIND_SERVICE ports: diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 547fb26c..aab28965 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -75,6 +75,15 @@ services: GEOIP_AVAILABLE: ${GEOIP_AVAILABLE:-1} RATE_LIMIT_AVAILABLE: ${RATE_LIMIT_AVAILABLE:-1} WEIGHTED_LB_AVAILABLE: ${WEIGHTED_LB_AVAILABLE:-1} + # Reports readiness; it does not restart anything on its own (Compose has + # no autoheal). start_period is generous: a first boot runs every migration + # before /readyz turns green, and a restart during that would loop forever. + healthcheck: + test: ["CMD", "/app/server", "healthcheck"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 300s depends_on: mariadb: condition: service_healthy @@ -153,6 +162,16 @@ services: - "80:80" - "443:443" - "443:443/udp" # HTTP/3 + # Probes the local admin API - it answers as soon as the config is loaded + # and carries no secrets. Deliberately not the :80/:443 listeners: those + # legitimately 503 before any route is pushed, and an ACME order in flight + # must never look like a wedged process. + healthcheck: + test: ["CMD", "wget", "-q", "-O", "/dev/null", "http://127.0.0.1:2019/config/"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 60s depends_on: geoip-init: condition: service_completed_successfully diff --git a/deploy/portainer-external-db.yml b/deploy/portainer-external-db.yml index 4669b1b0..73f90328 100644 --- a/deploy/portainer-external-db.yml +++ b/deploy/portainer-external-db.yml @@ -26,6 +26,12 @@ services: image: ghcr.io/host-yt/caddy-proxy-manager:1.5.1 pull_policy: if_not_present restart: unless-stopped + healthcheck: + test: ["CMD", "/app/server", "healthcheck"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 300s environment: APP_ENV: ${APP_ENV:-production} APP_URL: ${APP_URL} @@ -85,6 +91,12 @@ services: image: ghcr.io/host-yt/caddy-proxy-manager-edge:1.5.1 pull_policy: if_not_present restart: unless-stopped + healthcheck: + test: ["CMD", "wget", "-q", "-O", "/dev/null", "http://127.0.0.1:2019/config/"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 60s cap_add: - NET_BIND_SERVICE extra_hosts: From 3b4fd2cbf67cb7052de15866c315e4c3132a8a01 Mon Sep 17 00:00:00 2001 From: marcoome Date: Mon, 21 Sep 2026 09:54:14 +0200 Subject: [PATCH 05/12] ops(preflight): name every missing required var and tighten secret file modes `docker compose up` aborted on an empty REDIS_PASSWORD deep inside interpolation, and the install guide never mentioned the variable at all. `server doctor` now lists every required variable that is missing or empty in one row and prints a generated value for each secret one, so the fix is a paste rather than a loop of compose errors. Secret files: install_state.json is chmod 600 after the rename, not only on create, so a file restored from a backup or synced by a file-sync client stops being world-readable. Doctor reports the mode of .env and install_state.json plus the process umask. INSTALL.md sets umask 077 and chmods .env. (cherry picked from commit 2011619313efc8829ec93cdd62c72b919486c346) --- cmd/server/doctor.go | 143 +++++++++++++++++++++++ docs/INSTALL.md | 29 ++++- internal/installstate/state.go | 6 + internal/installstate/state_perm_test.go | 31 +++++ 4 files changed, 204 insertions(+), 5 deletions(-) create mode 100644 internal/installstate/state_perm_test.go diff --git a/cmd/server/doctor.go b/cmd/server/doctor.go index 8af5eb99..a8f41211 100644 --- a/cmd/server/doctor.go +++ b/cmd/server/doctor.go @@ -5,13 +5,17 @@ package main import ( "context" + "crypto/rand" "database/sql" + "encoding/hex" "fmt" "io" "net" "net/http" "os" "os/exec" + "strconv" + "strings" "text/tabwriter" "time" @@ -54,6 +58,8 @@ func runDoctor() int { var checks []check checks = append(checks, doctorConfigCheck(cfgErr)) + checks = append(checks, doctorRequiredEnv()...) + checks = append(checks, doctorSecretFiles()...) dbChecks, db := doctorDB(ctx, rawCfg) checks = append(checks, dbChecks...) @@ -385,3 +391,140 @@ func runHealthcheck() int { } return 0 } + +// requiredEnv is every variable deploy/docker-compose.yml marks `:?` plus the +// two config.Load() refuses to start without. Compose fails on the first one +// it hits, deep inside interpolation; doctor names them all at once (OPS-004). +var requiredEnv = []struct { + name string + secret bool // generate a value to paste when it is missing +}{ + {"APP_URL", false}, + {"APP_SECRET", true}, + {"DB_NAME", false}, + {"DB_USER", false}, + {"DB_PASSWORD", true}, + {"REDIS_PASSWORD", true}, + {"MARIADB_ROOT_PASSWORD", true}, + {"INSTALL_TOKEN", true}, +} + +// envFilePath is the .env doctor reads when it runs from a compose checkout +// rather than inside the container. HPG_ENV_FILE overrides it. +func envFilePath() string { + if p := os.Getenv("HPG_ENV_FILE"); p != "" { + return p + } + return ".env" +} + +// parseEnvFile reads KEY=VALUE lines. Deliberately dumb - it exists to answer +// "is this var set and non-empty", never to interpolate. +func parseEnvFile(path string) (map[string]string, error) { + b, err := os.ReadFile(path) + if err != nil { + return nil, err + } + out := map[string]string{} + for _, line := range strings.Split(string(b), "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + k, v, ok := strings.Cut(line, "=") + if !ok { + continue + } + out[strings.TrimSpace(k)] = strings.Trim(strings.TrimSpace(v), `"'`) + } + return out, nil +} + +// doctorRequiredEnv fails when a required variable is missing or empty, naming +// every one of them and offering a generated value for the secrets. An empty +// required var is a failure, not a default: `docker compose up` would abort. +func doctorRequiredEnv() []check { + fileVals, ferr := parseEnvFile(envFilePath()) + var missing []string + var gen []string + for _, v := range requiredEnv { + val := fileVals[v.name] + if val == "" { + val = os.Getenv(v.name) + } + if val != "" { + continue + } + missing = append(missing, v.name) + if v.secret { + gen = append(gen, v.name+"="+randomSecret()) + } + } + src := envFilePath() + if ferr != nil { + src = "process environment (" + envFilePath() + " not readable)" + } + if len(missing) == 0 { + return []check{{"config: required variables", statusPass, "all set in " + src}} + } + detail := "missing or empty in " + src + ": " + strings.Join(missing, ", ") + if len(gen) > 0 { + detail += " | paste: " + strings.Join(gen, " ") + } + return []check{{"config: required variables", statusFail, detail}} +} + +// randomSecret returns 32 bytes of hex - the same shape as +// `openssl rand -hex 32`, which the compose file's error message suggests. +func randomSecret() string { + var b [32]byte + if _, err := rand.Read(b[:]); err != nil { + return "" + } + return hex.EncodeToString(b[:]) +} + +// doctorSecretFiles reports the permissions of the two files that hold panel +// secrets. Group/other-readable means any other local account can read the +// credentials (OPS-003). +func doctorSecretFiles() []check { + var checks []check + for _, path := range []string{envFilePath(), stateDir + "/install_state.json"} { + fi, err := os.Stat(path) + if err != nil { + continue // not this deployment's layout; nothing to report + } + mode := fi.Mode().Perm() + label := "secrets: " + path + if mode&0o077 != 0 { + checks = append(checks, check{label, statusWarn, + fmt.Sprintf("mode %04o is readable by other local accounts - chmod 600 %s (and umask 077 before creating it)", mode, path)}) + continue + } + checks = append(checks, check{label, statusPass, fmt.Sprintf("mode %04o", mode)}) + } + if u, ok := procUmask(); ok && u&0o077 != 0o077 { + checks = append(checks, check{"secrets: umask", statusWarn, + fmt.Sprintf("umask %04o lets newly created files be group/other-readable - set umask 077 in the shell that runs the installer", u)}) + } + return checks +} + +// procUmask reads the current umask from /proc/self/status (Linux). Returns +// ok=false elsewhere - reading it via syscall would mean setting it first. +func procUmask() (int, bool) { + b, err := os.ReadFile("/proc/self/status") + if err != nil { + return 0, false + } + for _, line := range strings.Split(string(b), "\n") { + if v, ok := strings.CutPrefix(line, "Umask:"); ok { + n, err := strconv.ParseInt(strings.TrimSpace(v), 8, 32) + if err != nil { + return 0, false + } + return int(n), true + } + } + return 0, false +} diff --git a/docs/INSTALL.md b/docs/INSTALL.md index 79c7ba46..d8132d54 100644 --- a/docs/INSTALL.md +++ b/docs/INSTALL.md @@ -33,10 +33,13 @@ Port 2019 (Caddy Admin API) must **not** be exposed - it is internal-only. ```bash git clone https://github.com/host-yt/caddy-proxy-manager.git hostyt-proxy-gateway cd hostyt-proxy-gateway +umask 077 # so .env is not created world-readable cp .env.example .env +chmod 600 .env ``` -Open `.env` and set the four required variables: +Open `.env` and set the required variables. Compose refuses to render without +them, and it reports only the first one it hits: ```bash # Publicly reachable URL of the panel (must match your DNS A record) @@ -45,15 +48,31 @@ APP_URL=https://panel.example.com # Random 64-character hex secret - generate with: APP_SECRET=$(openssl rand -hex 32) -# MariaDB passwords - use strong, unique values +# MariaDB + Redis passwords - use strong, unique values DB_PASSWORD=change_me_strong MARIADB_ROOT_PASSWORD=change_me_root_strong +REDIS_PASSWORD=change_me_redis_strong + +# One-shot token that unlocks the install wizard +INSTALL_TOKEN=$(openssl rand -hex 16) # Let's Encrypt contact address CADDY_ACME_EMAIL=ops@example.com ``` -### 2.2 Start the stack +`DB_NAME` and `DB_USER` are also required; `.env.example` already fills them in. + +### 2.2 Preflight + +Check the whole set at once, with generated values for anything missing, and +the file permissions on your secrets: + +```bash +docker run --rm -v "$PWD/.env:/app/.env:ro" \ + ghcr.io/host-yt/caddy-proxy-manager:1.5.1 doctor +``` + +### 2.3 Start the stack ```bash docker compose -f deploy/docker-compose.yml --env-file .env up -d @@ -68,13 +87,13 @@ Watch startup logs: docker compose -f deploy/docker-compose.yml logs -f --tail=100 ``` -### 2.3 Open the install wizard +### 2.4 Open the install wizard Navigate to `http://:8080/install` (or the URL set in `APP_URL`). The wizard is only reachable while `INSTALLED=0`. It flips itself to `1` on completion. -### 2.4 Full vs Lite stack +### 2.5 Full vs Lite stack Two compose files ship. Pick by whether you can run the custom Caddy build. diff --git a/internal/installstate/state.go b/internal/installstate/state.go index 27285d09..69506f47 100644 --- a/internal/installstate/state.go +++ b/internal/installstate/state.go @@ -179,6 +179,12 @@ func (m *Manager) Save(s *State) error { if err := os.Rename(tmp, m.path); err != nil { return fmt.Errorf("rename: %w", err) } + // The 0600 above only applies when WriteFile creates the temp file; a file + // restored from a backup, synced by a file-sync client, or written by an + // older build can still be world-readable. This holds the invariant. + if err := os.Chmod(m.path, 0o600); err != nil { + return fmt.Errorf("chmod: %w", err) + } m.cache = s return nil } diff --git a/internal/installstate/state_perm_test.go b/internal/installstate/state_perm_test.go new file mode 100644 index 00000000..fef72eaa --- /dev/null +++ b/internal/installstate/state_perm_test.go @@ -0,0 +1,31 @@ +package installstate + +import ( + "os" + "path/filepath" + "testing" +) + +// Save must leave the state file unreadable by other local accounts even when +// an earlier, looser file is already in place (OPS-003). +func TestSaveTightensPermissions(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "install_state.json") + if err := os.WriteFile(path, []byte("{}"), 0o644); err != nil { + t.Fatal(err) + } + m, err := New(dir, "0123456789abcdef0123456789abcdef") + if err != nil { + t.Fatal(err) + } + if err := m.Save(&State{CurrentStep: StepWelcome}); err != nil { + t.Fatal(err) + } + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if mode := fi.Mode().Perm(); mode&0o077 != 0 { + t.Fatalf("state file mode %04o is group/other readable", mode) + } +} From 95d1030b116ee78c929fa0a1c75d67b3d0a11a59 Mon Sep 17 00:00:00 2001 From: marcoome Date: Mon, 21 Sep 2026 09:54:45 +0200 Subject: [PATCH 06/12] ops(geoip): make the remote-node profile able to serve country matching The agent wrote /data/geoip/GeoLite2-Country.mmdb into its own container - the example mounted only logs and state - and the remote Caddy ran stock caddy:2.11.4, which has no maxmind_geolocation matcher. The feature could not work in the profile that documents it. Agent and Caddy now share /opt/hostyt-node/geoip (read-only on the Caddy side, only the agent writes), the remote profile defaults to the custom edge image, and syncGeoIP refuses to write when the directory is not a mount instead of succeeding into a path Caddy never reads. The per-node GeoIP capability flag stays the operator's gate: Caddy's admin API exposes no module list, so there is nothing to probe it with. (cherry picked from commit adfcb0530b33f9e98c52d6d5b8587f00bdd968c5) --- docs/GEOIP.md | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/docs/GEOIP.md b/docs/GEOIP.md index c389ccda..05013cdd 100644 --- a/docs/GEOIP.md +++ b/docs/GEOIP.md @@ -67,6 +67,26 @@ The admin UI accepts a comma-separated list. Blank value with `allow` or `deny` is treated as "no countries" which effectively blocks or allows everything - validate the list before saving. +## Remote nodes + +A remote node does not get the mmdb from the panel's volume - the node-agent +syncs it (`/api/node/geoip/meta` + `/api/node/geoip/mmdb`, sha256-checked) and +writes `/data/geoip/GeoLite2-Country.mmdb`. Three things have to line up, and +all three ship in `deploy/remote-node/` + `deploy/node-agent/`: + +1. **Shared volume.** The agent writes and Caddy reads the same directory: + `/opt/hostyt-node/geoip:/data/geoip` on the agent, the same path `:ro` on + the caddy service. Without it the agent warns + `geoip: target directory missing` and skips the sync rather than writing + into its own container where Caddy will never see it. +2. **A Caddy build with the module.** Stock `caddy:2.11.4` has no + `maxmind_geolocation` matcher and rejects the whole config; the remote-node + profile therefore defaults to + `ghcr.io/host-yt/caddy-proxy-manager-edge`. +3. **The capability flag.** Tick **GeoIP** under Module capabilities on the + node's edit page (or set the fleet-wide `GEOIP_AVAILABLE=1`) only once the + first two are true - see the never-flip-early note below. + ## Per-node capability Admin → Caddy Nodes shows a `GeoIP` badge on nodes where the module was detected. @@ -88,5 +108,6 @@ Admin → Settings → GeoIP shows: - GeoIP accuracy depends on MaxMind's data; VPN/proxy users may appear in the wrong country. - The DB must be present on the container filesystem at `/data/geoip/`; bind-mount or - volume required in production deployments. + volume required in production deployments. On a remote node that volume has to be + shared between the node-agent (writer) and Caddy (reader). - MaxMind GeoLite2 is free but requires a license key; GeoIP2 (paid) is not tested. From 6a88fee814b53526a8dfbff61673a805662c1228 Mon Sep 17 00:00:00 2001 From: marcoome Date: Mon, 21 Sep 2026 09:56:13 +0200 Subject: [PATCH 07/12] docs: correct make run, the APP_SECRET rotation claim and the reconcile SLA - `make run` advertised loading .env but only ran `go run`, and nothing in the Go code parses a .env file. The target now sources it when present, and both the package doc and ARCHITECTURE say the process environment is the only source. - SECURITY claimed APP_SECRET rotation was downtime-free. cmd/rotate-secret requires the panel stopped and nulls api_keys.key_hmac, so the runbook now spells out stop, back up, rotate, restart, re-issue every API key. - ARCHITECTURE states the actual recovery window for deferred work instead of leaving "no durable queue" as the whole answer: boot push at +10s, reconcile at 60s, drift at 5 min, so a route change is durable but can take ~5 minutes after a crash; mail is the work that can be lost outright. No queue built. (cherry picked from commit c4db9474603b807300ed3f318b51753c3c11973b) --- Makefile | 4 ++-- docs/ARCHITECTURE.md | 14 +++++++++++++- docs/SECURITY.md | 26 +++++++++++++++++++++++++- internal/config/config.go | 5 +++-- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index ce4d256e..3b362259 100644 --- a/Makefile +++ b/Makefile @@ -84,8 +84,8 @@ build: gen build-css ## Build server binary (CSS first so it embeds). CGO_ENABLED=0 $(GO) build -trimpath -ldflags="-s -w" -o $(BIN) ./cmd/server .PHONY: run -run: build-css ## Run locally (loads .env). - $(GO) run ./cmd/server +run: build-css ## Run locally (sources .env into the environment when present). + @set -a; [ -f .env ] && . ./.env; set +a; $(GO) run ./cmd/server .PHONY: pin-version pin-version: ## Rewrite every deploy/ image pin + README status to V=x.y.z (single version source). diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9752b6f1..32166f8d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -76,7 +76,8 @@ mechanism. ### `internal/config/` Loads the full runtime config from environment variables into a typed -`Config` struct. No file parsing in production; `.env` is dev-only. +`Config` struct. It never parses a file: Compose interpolates `.env`, and +`make run` sources it into the shell before starting the binary. Validates required fields (e.g. `APP_SECRET` ≥ 32 chars after install, `APP_URL`, DB credentials). Exposes module-availability gates (`CACHE_HANDLER_AVAILABLE`, `LAYER4_AVAILABLE`, `WAF_MODULE_AVAILABLE`, @@ -255,6 +256,17 @@ the periodic sweeps: health probe, drift resync, alias re-check, backups. A Redis-backed queue is a candidate for the work that today survives only as long as the process does; it is not implemented. +**What that costs, honestly.** A crash between the DB commit and the push +loses only the immediate attempt - the row is already durable, and a later +sweep re-pushes it. The recovery window is bounded by those sweeps, not by a +queue: boot push runs 10s after the leader starts, `reconcile` every 60s picks +up routes left in a stuck state, and `drift` re-pushes a node whose live +config no longer matches the DB every 5 minutes. So a route change survives a +crash but can take up to ~5 minutes to reach a node, and ordering between two +changes to different nodes is not guaranteed. Email sends and webhook +deliveries are the work that can be lost outright; webhooks retry on a 30s +dispatcher, mail does not. + ### `internal/i18n/` Cookie-based language selection; templates carry translated strings. diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 40f52324..8870011a 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -222,7 +222,31 @@ own streams and cannot clear a quarantine without fixing the destination. | API key hashes | `api_keys` table | Argon2id hash | | User passwords | `users` table | Argon2id hash | -`APP_SECRET` must be ≥ 32 characters. The `cmd/rotate-secret` tool re-encrypts all blobs under a new key without downtime. +`APP_SECRET` must be ≥ 32 characters. + +### Rotating `APP_SECRET` + +`cmd/rotate-secret` re-encrypts every at-rest blob under a new key. It is +**not** a zero-downtime operation, and it invalidates API keys: + +1. **Stop the panel.** The tool rewrites rows the panel reads on boot; running + it against a live instance is unsupported. +2. **Back up** the database and `data/install_state.json`. A half-applied + rotation is unrecoverable without them. +3. Run `hpg-rotate-secret --state ./data/install_state.json --old-secret + --new-secret ` (add `--apply`; without it the run prints a + dry-run summary and changes nothing). +4. Set the new `APP_SECRET` in `.env` (or the orchestrator's secret store) and + start the panel. +5. **Re-issue every API key.** `api_keys.key_hmac` is keyed off `APP_SECRET` + and cannot be re-derived from a hash, so rotation nulls it out. Existing + integration tokens stop working: create replacements in + **Admin → API keys** and distribute them before the next automated run. + +Node admin-proxy keys, WireGuard keys and TOTP secrets are re-encrypted in +place and need no operator action. A restored backup paired with a *different* +`APP_SECRET` is the failure mode to avoid - `server doctor` reports the node +keys it can no longer decrypt. --- diff --git a/internal/config/config.go b/internal/config/config.go index 7e18fcaf..402a60e9 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,7 +1,8 @@ // Package config loads runtime configuration from environment variables. // -// Source of truth: env. .env file is loaded only in development (when present) -// to avoid surprises in production where secrets come from the orchestrator. +// Source of truth: the process environment. This package never reads a .env +// file - in production Compose interpolates it, and `make run` sources it into +// the shell first. package config import ( From 34b8abd997a828404b457397afc2b1fafda8b1a5 Mon Sep 17 00:00:00 2001 From: marcoome Date: Mon, 21 Sep 2026 09:57:15 +0200 Subject: [PATCH 08/12] fix(sso): strict forward-auth by default for new routes, drop Sec-Fetch-Dest trust HPG-SEC-003. Permissive/document-only SSO mode only ever gated GET/HEAD page loads and additionally skipped the gate for any request whose client-supplied Sec-Fetch-Dest header claimed a subresource kind - a plain GET could dodge auth entirely just by setting that header, and every POST/PUT/PATCH/DELETE already went through with zero auth check. - New routes (routes.Service.Create, the single insert path for the panel UI, the client-portal API and the admin API) now default sso_strict_mode=1, so an operator who later turns on SSO for a route gets the only mode that actually authenticates every request unless they deliberately opt out. Existing rows are untouched - this is an INSERT-time default, not a migration/backfill, so no live route's behavior changes today. - Removed the Sec-Fetch-Dest exclusion from the permissive-mode matcher. Never base an auth decision on a header the client sets. The static-asset path exclusion (SPA refresh storms) is unaffected. - Renamed the UI copy to say what permissive mode actually is (document-load-only, not a weaker form of real auth) and added a visible warning when Strict mode is unchecked. What an operator notices: a route with SSO configured for the first time now starts in Strict mode (401 JSON, all methods) instead of the old redirect-only GET/HEAD gate - if that breaks a browser app that relied on the permissive redirect flow, uncheck Strict mode and accept the warning. Existing SSO routes are unaffected either way. Permissive mode no longer exempts requests that claim a non-navigation Sec-Fetch-Dest, so an SPA's own XHR/fetch calls to its backend are now gated like everything else in that mode - scope the gate off an API path with sso_paths/sso_hosts, or switch to Strict mode, if that breaks an app that relied on the old bypass. (cherry picked from commit 5a5b6429cce465b037158d319201abbbdb3dcdbc) --- internal/caddyapi/route.go | 60 +++++++++---------- internal/caddyapi/route_test.go | 33 ++++++++++ internal/domain/routes/lifecycle.go | 9 ++- .../domain/routes/sso_strict_default_test.go | 30 ++++++++++ internal/view/admin/hosts_edit.html.tmpl | 9 ++- 5 files changed, 105 insertions(+), 36 deletions(-) create mode 100644 internal/domain/routes/sso_strict_default_test.go diff --git a/internal/caddyapi/route.go b/internal/caddyapi/route.go index 5dbdd0d8..714ee04d 100644 --- a/internal/caddyapi/route.go +++ b/internal/caddyapi/route.go @@ -121,8 +121,13 @@ type Route struct { SSOResolver string // SSOStrictMode extends the SSO gate to ALL HTTP methods (not just GET/HEAD) // and returns 401 JSON when the IdP replies with a redirect (3xx) instead of - // passing the redirect through to the client. Use for API-only routes. - // When false (default), the gate only checks GET/HEAD document loads. + // passing the redirect through to the client. This is the only mode that + // actually authenticates every request; new routes default to it + // (routes.Service.Create). When false, the gate is permissive/document-only: + // it checks GET/HEAD page loads and lets every other method and every + // XHR/fetch subresource request through with no auth check at all - it + // exists for browser apps with their own API auth layer, not as a weaker + // form of real protection (HPG-SEC-003). SSOStrictMode bool // External marks a reverse_proxy route whose upstream is an allowlisted @@ -994,25 +999,25 @@ func BuildRoute(r Route) map[string]any { // on top - Proxmox uses its CSRFPreventionToken + auth ticket, // Authelia-protected apps typically share the session cookie, // etc. - // Forward-auth matcher: GET/HEAD only, AND skip - // (a) common static asset extensions / asset path trees, and - // (b) browser XHR / fetch / subresource requests detected via - // Sec-Fetch-Dest (anything other than "document"/"iframe"). + // Forward-auth matcher (permissive/document-only mode): GET/HEAD only, + // AND skip common static asset extensions / asset path trees. // - // (a) Hard SPA refresh fires dozens of parallel JS/CSS/font - // requests; each one going through Authentik queues up and the - // IdP's auth endpoint serialises behind its own DB calls → - // tail-latency blows past Caddy's read timeout → 504. + // Hard SPA refresh fires dozens of parallel JS/CSS/font requests; + // each one going through Authentik queues up and the IdP's auth + // endpoint serialises behind its own DB calls → tail-latency blows + // past Caddy's read timeout → 504. // - // (b) The browser cannot follow a cross-origin 302 on an XHR / - // fetch, so if Authentik replies with a redirect to sso.example.com the - // request is killed by CORS. SPAs talking to their own backend - // API (Proxmox, internal dashboards…) thus break under SSO - // unless we bypass forward_auth for those subresource hits. The - // upstream app must own its own API auth in this mode; SSO only - // guarantees the document load is gated. Sec-Fetch-Dest is sent - // by every modern browser - non-browser clients (curl, agents) - // omit it and are still gated, which is the safer default. + // HPG-SEC-003: this mode used to also exclude requests by + // Sec-Fetch-Dest (to spare XHR/fetch from the redirect-under-CORS + // problem), but that header is entirely client-supplied - a GET to + // a sensitive document just had to claim an excluded value to skip + // the auth gate. Never base an auth decision on it. The path-based + // asset exclusion stays (it only widens what's cacheable-looking, + // not what a specific request claims to be) but SPA XHR/fetch calls + // to their own backend API are gated like any other request now; + // use SSOPaths/SSOHosts to scope the gate away from an API path if + // that breaks a specific app, or Strict mode's all-methods 401 JSON + // which browsers don't try to redirect-follow. ssoMatch := map[string]any{ "method": []string{"GET", "HEAD"}, "not": []any{ @@ -1025,16 +1030,6 @@ func BuildRoute(r Route) map[string]any { "/pve2/*", "/static/*", "/assets/*", "/_next/static/*", }, }, - map[string]any{ - "header": map[string]any{ - "Sec-Fetch-Dest": []string{ - "empty", "image", "font", "audio", "video", - "manifest", "object", "embed", "track", - "script", "style", "report", - "worker", "serviceworker", "sharedworker", - }, - }, - }, }, } // Per-route SSO scope: narrow the gate to specific paths and/or @@ -1045,8 +1040,11 @@ func BuildRoute(r Route) map[string]any { if len(r.SSOHosts) > 0 { ssoMatch["host"] = r.SSOHosts } - // In strict mode: gate all methods with no path/header exclusions. - // In default mode: gate only GET/HEAD document loads (ssoMatch restrictions apply). + // In strict mode: gate all methods with no path exclusions. + // In permissive/document-only mode: gate only GET/HEAD page loads + // (ssoMatch restrictions apply) - POST/PUT/PATCH/DELETE reach the + // origin unauthenticated, so this mode is for browser apps with + // their own API auth layer, never for APIs with none of their own. authRoute := map[string]any{ "handle": []any{ map[string]any{ diff --git a/internal/caddyapi/route_test.go b/internal/caddyapi/route_test.go index eac61d3a..4610aa33 100644 --- a/internal/caddyapi/route_test.go +++ b/internal/caddyapi/route_test.go @@ -958,3 +958,36 @@ func TestBuildMTLSRBAC_CarriesNodeToken(t *testing.T) { t.Error("token header emitted without a token") } } + +// TestSSOPermissiveModeIgnoresSecFetchDest is the regression for HPG-SEC-003: +// permissive/document-only SSO used to skip the auth gate for any GET/HEAD +// whose Sec-Fetch-Dest claimed a subresource kind - entirely client-supplied, +// so a plain curl GET to a sensitive page could dodge auth by setting the +// header. The matcher must never reference it. +func TestSSOPermissiveModeIgnoresSecFetchDest(t *testing.T) { + r := Route{ + ID: "60", Hosts: []string{"perm.example.com"}, UpstreamIP: "10.0.0.1", UpstreamPort: 80, + SSOProviderURL: "https://sso.example.com", SSOStrictMode: false, + } + s := mustJSON(r) + if strings.Contains(s, "Sec-Fetch-Dest") { + t.Errorf("permissive-mode matcher must not key off client-supplied Sec-Fetch-Dest\nfull: %s", s) + } + // The static-asset path exclusion must still be present (unrelated skip). + if !strings.Contains(s, `"*.js"`) { + t.Errorf("permissive-mode matcher lost its static-asset exclusion\nfull: %s", s) + } +} + +// TestSSOStrictModeGatesAllMethods: strict mode's forward-auth subroute must +// carry no "match" restriction at all - every method, every path. +func TestSSOStrictModeGatesAllMethods(t *testing.T) { + r := Route{ + ID: "61", Hosts: []string{"strict.example.com"}, UpstreamIP: "10.0.0.1", UpstreamPort: 80, + SSOProviderURL: "https://sso.example.com", SSOStrictMode: true, + } + s := mustJSON(r) + if strings.Contains(s, `"method":["GET","HEAD"]`) { + t.Errorf("strict mode must not restrict to GET/HEAD\nfull: %s", s) + } +} diff --git a/internal/domain/routes/lifecycle.go b/internal/domain/routes/lifecycle.go index bd94df5e..3cce4dbf 100644 --- a/internal/domain/routes/lifecycle.go +++ b/internal/domain/routes/lifecycle.go @@ -349,6 +349,11 @@ func (s *Service) Create(ctx context.Context, clientID int64, in CreateInput) (i } s.Logger.Warn("anti-squat: evicted unverified route on create", "evicted_id", conflictID, "domain", domain) } + // sso_strict_mode = 1: HPG-SEC-003, a new route defaults to the only SSO + // mode that actually authenticates every request. SSO itself stays off + // (sso_provider_url empty) until the operator sets it up via edit; this + // only decides what they get once they do. Existing rows are untouched - + // this is an INSERT-time default, not a migration/backfill. res, err := tx.ExecContext(ctx, `INSERT INTO routes (service_id, caddy_node_id, domain, path_prefix, upstream_port, upstream_scheme, ssl_enabled, websocket, force_https, http2_enabled, http3_enabled, status, @@ -357,8 +362,8 @@ func (s *Service) Create(ctx context.Context, clientID int64, in CreateInput) (i wildcard_enabled, wildcard_zone, group_id, custom_fields, via_wg_peer_id, dns_resolver_via_wg_peer_id, require_client_cert, mtls_ca_id, - domain_verified, verify_token) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 1, 0, 'pending_dns', ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, NULLIF(?, 0), NULLIF(?, ''), ?, ?, ?, NULLIF(?, 0), ?, ?)`, + domain_verified, verify_token, sso_strict_mode) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 1, 0, 'pending_dns', ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, NULLIF(?, 0), NULLIF(?, ''), ?, ?, ?, NULLIF(?, 0), ?, ?, 1)`, in.ServiceID, nodeID, domain, pathPrefix, in.UpstreamPort, scheme, in.SSL, in.WebSocket, in.ForceHTTPS, kind, redirURL, redirCode, tagVal, diff --git a/internal/domain/routes/sso_strict_default_test.go b/internal/domain/routes/sso_strict_default_test.go new file mode 100644 index 00000000..e2732d25 --- /dev/null +++ b/internal/domain/routes/sso_strict_default_test.go @@ -0,0 +1,30 @@ +package routes + +import ( + "context" + "testing" +) + +// TestCreate_DefaultsSSOStrictMode is the regression for HPG-SEC-003: a new +// route must default to the only SSO mode that authenticates every request, +// so an operator who later configures SSO gets a secure gate unless they +// deliberately opt out. This must not touch any existing row's value - +// covered separately by not running any UPDATE/migration here at all. +func TestCreate_DefaultsSSOStrictMode(t *testing.T) { + db := newPushTestDB(t) + seedCapacityFixture(t, db, 10) + + routeID, err := newMTLSCreateSvc(t, db).Create(context.Background(), 0, CreateInput{ + ServiceID: 1, UpstreamPort: 10008, Domain: "newroute.example", + }) + if err != nil { + t.Fatalf("create: %v", err) + } + var strict int + if err := db.QueryRow("SELECT sso_strict_mode FROM routes WHERE id = ?", routeID).Scan(&strict); err != nil { + t.Fatal(err) + } + if strict != 1 { + t.Errorf("sso_strict_mode = %d, want 1 (new routes must default to strict)", strict) + } +} diff --git a/internal/view/admin/hosts_edit.html.tmpl b/internal/view/admin/hosts_edit.html.tmpl index b81f3f4b..deec2704 100644 --- a/internal/view/admin/hosts_edit.html.tmpl +++ b/internal/view/admin/hosts_edit.html.tmpl @@ -540,7 +540,7 @@