fix: close the three findings against 1.6.0, and move the node admin endpoint off TCP - #22
Merged
Merged
Conversation
A failed panel-side DNS lookup said nothing about where a name points, yet it was accepted and the node resolved the unchecked name itself. Resolution failure is now a refusal on every HTTP backend write path. Names only the node can resolve (container names, tunnel peers) stay possible through an explicit per-route switch, stored in the new routes.backend_resolve_node_side column. Tunnel-bound routes are grandfathered by migration 00146 so no live route goes down on upgrade. (cherry picked from commit 907d1f842e08d25c4cef29eff2c858900effc159)
Backend hostnames were screened when a route was saved and then handed to the node as names, so the address a node dialed was whatever DNS answered at dial time - not the address that passed the screen. Emission now resolves, screens and pins each name to the address it screened, for the primary backend, additional upstreams and path-rule upstreams. https backends keep the origin hostname through an explicit transport tls.server_name, so SNI and certificate verification are unchanged. Names that must reach the node as names (node-side resolution, external allowlisted origins, node-side DNS resolvers, and https pools spread over several hostnames, which cannot share one server_name) keep their name and stay screened against the deny set on every push. A name that stops resolving keeps its last screened address instead of being handed back to the node or taking the route down. (cherry picked from commit ea216095c95c9b3bc8bbc5f84f3ce57ca8b1fa7b)
Strict mode became the default for new routes only, so routes created earlier kept gating GET/HEAD page loads and letting every other method through unauthenticated. Migration 00147 moves them all to strict. Permissive mode stays reachable as a per-route opt-out, but only as a deliberate one: a save that does not carry the field leaves the stored value alone, the UI states plainly which methods it does and does not cover, and choosing it is written to the audit log. Operators can see what the backfill will do before running it: 'server doctor' lists every SSO route still in permissive mode. (cherry picked from commit b547a1def62b85ae7dab1cba6e84db22fcf42217)
Caddy's admin endpoint has no authentication of its own, so its only control is who can open a connection to it. A socket endpoint has no address at all: the panel reaches it over a shared volume, a remote node through its node-agent, and nothing can name it by host and port. Verified against the pinned caddy 2.11.4: the socket bind, the |mode suffix, switching admin.listen in both directions with a running /load, and `caddy validate` on the config this panel generates. Opt-in per node. HPG_CADDY_ADMIN_LISTEN keeps its fleet-wide meaning unless HPG_CADDY_ADMIN_LISTEN_NODES names the node IDs it applies to, so an upgrade that changes nothing else keeps the current TCP bind. The agent prefers the socket whenever it is live and falls back to HPG_CADDY_ADMIN_URL otherwise, so neither end has to be cut over first and the socket file Caddy leaves behind cannot strand it. `server doctor` reads each node's actual bind back from the node, and the agent's doctor reports the transport it will use: both must be green before a published port is removed. Rollout order and rollback are in docs/MULTI_NODE.md. Also: - caddyapi.ScreenDialTarget: an upstream must be a plain host:port. Caddy accepts socket and file-descriptor upstream forms too, which no address-based screen covers. - A node's Admin API URL is editable, so an existing node can be repointed at a different admin endpoint without re-registering it. (cherry picked from commit 3fc0cc5ecc59c0cf3e6b1b890b2f3d72aa292eb8)
The two branches met here: the admin endpoint can now run on a unix socket, which removes its address but not its reachability from a proxy handler in the same process. An upstream naming a socket or a file descriptor carries no address for the deny set to judge, so emission refuses that shape outright.
Closes the three findings an independent review raised against the 1.6.0 batch, and the architectural issue underneath them. Minor rather than patch: adds the unix-socket admin transport, the per-node gate for it, an editable node Admin API URL and new doctor checks. A deployment that upgrades and changes nothing keeps its current behaviour; the socket mode is opt-in per node.
… strict mode Three behaviours shipped in this branch were only in the CHANGELOG and UI strings: backend address pinning at config build time, the per-route "backend is resolved on the node" switch (migration 00146), and the SSO strict-mode backfill (migration 00147). Add sections 7-9 to ROUTES.md.
It said every XHR/fetch request bypasses the gate. That stopped being true when the client-supplied Sec-Fetch-Dest matcher was removed: a GET from fetch to a non-asset path is gated like any other GET. What actually bypasses is non-safe methods and the static asset paths, which is what it now says.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security remediation plus the unix-socket admin transport. Impact and upgrade notes are in the CHANGELOG; mechanism detail is withheld until the fleet has upgraded.
Verified:
go build,go vet,make check-migrations,go test -race ./...(41 packages), all five Compose profiles, and the two-node e2e stack (14/14 assertions - install wizard, active_active fan-out, both nodes serving real traffic, node killed, failover).