docs(operator): a systemd unit that confines the egress prober - #411
Open
Ryanmello07 wants to merge 18 commits into
Open
docs(operator): a systemd unit that confines the egress prober#411Ryanmello07 wants to merge 18 commits into
Ryanmello07 wants to merge 18 commits into
Conversation
SetConnectionLocation panicked when a client's IP resolved to a country- or region-only location (no city), because it inserted the location row's NULL city_location_id / region_location_id into network_client_location, where both are NOT NULL. The panic is the real damage, not just a failed lookup: it propagates out of the connection announce goroutine (connect/transport_announce.go), whose HandleError wrapper cancels the connection-level context -- tearing down the entire connect connection right after auth. And because the panic hits before the disconnect-cleanup defer is registered, the connection row is orphaned as connected=true forever. How often this fires depends on the geo database's city coverage, so it is rare with a full commercial dataset and constant with a sparse one. Fix: fall back to the coarsest available granularity so the columns are always non-null -- a country-only location stores its country id for city/region too, keeping the provider locatable at country level instead of crashing the connection. If even the country id is absent, return a clean error (the caller already has a graceful retry path) rather than panic.
The sub-project's binding constraint is that country codes are stored/
compared lowercased, and CreateLocation (network_client_location_model.go)
already enforces this at the model layer via strings.ToLower before writing.
SetProviderEgressLocation wrote e.CountryCode verbatim, so an uppercase code
from an operator-run prober (the raw "US" the real geolocation APIs return)
would silently persist uppercase and violate the invariant that
countryCodeLocationIds and other lookups depend on.
Normalize to lowercase before the INSERT/UPDATE, mirroring CreateLocation's
idiom, and add a test asserting SetProviderEgressLocation("US") round-trips
as "us" via GetProviderEgressLocation.
…ion test TestSubmitProviderEgressLocationCountryOnly only checked the persisted CityConfident flag, which is copied straight from args independent of the location-resolution branch. It never asserted the resolved location's LocationType, so a broken granularity gate (resolving city unconditionally) would slip past all 5 existing tests. Add an assertion that the resolved location is LocationTypeCountry, mirroring the pattern already used by TestSubmitProviderEgressLocationCityConfidentStoresCity. Also assert CityLocationId/RegionLocationId are unset, since a country-granularity row's INSERT never populates those columns (verified against CreateLocation and GetLocation).
Adds POST /network/provider-egress-location, an operator-to-server endpoint (not a client route) that ingests probed provider egress locations. Authenticated by a shared secret from the vault (beta-vault/vault/provider_egress.yml, key ingest_secret) compared with hmac.Equal, not a network jwt. Fails closed: an absent vault resource or empty/missing key disables the endpoint (every request rejected) without panicking the api process at startup or per-request.
… just reject Both committed tests for ProviderEgressLocationSubmit ran with the vault unconfigured, so hmac.Equal never executed - a handler that always returned 401 would have passed the same suite. Extract the memoized secret reader (sync.OnceValue) into a reassignable package var plus a plain readOperatorIngestSecret function, with no change to the read logic or the handler's auth gate, so tests can inject a known secret without racing the existing unconfigured-vault reject tests in the same process. Add a positive-path test (correct secret clears auth and reaches the controller, 400 "Unknown client." for an unregistered id) and a negative discrimination test (wrong secret against a configured vault still 401), plus a direct vault-read test. Document in the vault example that the secret is memoized at first request, so rotating it requires an api restart.
…n parity, cover unprotected branches SetConnectionLocation ran on the connect-announce hot path (every client, every connection, inside a retry loop) with two separate DB round trips before ever reaching the mmdb fallback. Fix review findings against the probed-egress location change: - model.GetFreshProviderEgressLocationForConnection replaces the GetNetworkClientForConnection + GetFreshProviderEgressLocation pair with a single query joining network_client_connection to provider_egress_location. Freshness cutoff is still computed/compared in Go, not SQL now(). Cuts the probed-hit path from 3 DB round trips to 2, and the fallback path from 4 to 3. - The probed path now also runs the ARIN org-vs-country foreign check (against the probed country code), matching the mmdb path's GetLocationForIp, so net_type_foreign no longer gives probed providers an unearned ranking advantage over unprobed ones. Factored into a shared arinForeignScore helper; on any ARIN/ip-parse failure it just leaves NetTypeForeign at 0, never erroring or panicking the hot path. - Added tests for the stale-probed fallback, a probed write error (bad location_id) falling back to mmdb without panicking, and the Hosting/Proxy flag-to-score mapping. Verified the stale-fallback test has teeth by temporarily widening maxAge and confirming it fails. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…oversized submissions, wire up ingest secret Pre-merge fixes from the final whole-branch review of provider-egress geolocation: - Fix the probed-path ARIN "foreign" check: it compared the control ip's ARIN org country against the probed egress country (two different ips), flagging a provider foreign precisely when probing changed the answer. Now matches the mmdb path exactly: ARIN org country of the control ip vs. the mmdb country of that same control ip, restoring probed/unprobed ranking parity. - Reject provider-egress submissions with an empty (post-trim) Country/City/Region instead of silently creating a permanently-blank canonical location row via CreateLocation's dedupe-on-name behavior. - Reject over-long Country/City/Region (>128) or Org (>256) instead of panicking inside CreateLocation on a Postgres "value too long" error. - Make SetProviderEgressLocation's upsert monotonic in observed_at so a replayed older submission cannot clobber a newer stored row. - Replace three production-comment references to the (upstream-absent) design-spec doc path with inline prose, and drop the hardcoded beta vault filesystem path from the ingest-secret handler comment. - Delete GetNetworkClientForConnection (model/provider_egress_location_model.go), a fully superseded, zero-caller helper. - Provision the ingest secret end-to-end: beta-setup.sh now generates beta-vault/vault/provider_egress.yml (guarded the same way as the other generated secrets), and BETA.md documents it plus the api-restart requirement, since the endpoint otherwise ships silently disabled. Adds tests for each behavioral fix; all pass, including TestSetConnectionLocationToleratesCountryOnlyLocation. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
The example config lives in this fork's beta deployment tree, which does not exist upstream. Operators configure the ingest secret through their own vault; the handler documents the resource name and key.
…AssertEqual The three provider-egress-location test files imported github.com/go-playground/assert/v2, which is present in this fork's go.mod but not in upstream's, breaking compilation on this upstream-based branch (no required module provides package github.com/go-playground/assert/v2). Upstream already has an equivalent helper, connect.AssertEqual (from github.com/urnetwork/connect, used throughout model/ and controller/ test files), so no new third-party dependency is needed. This is a mechanical 1:1 swap (assert.Equal(t, v1, v2) -> connect.AssertEqual(t, v1, v2), same argument order and semantics) with no change to test logic or assertions. None of the three files used assert.NotEqual. No go.mod/go.sum changes: go-playground/assert was never present in this branch's go.mod/go.sum to begin with.
… overflow, and other review findings Five review findings on the provider-egress-location PR, each empirically reproduced before being fixed: - A far-future observed_at had no upper bound: it would win the monotonic upsert forever, read as fresh forever, and outlive the taskworker sweep, permanently pinning a provider's location with no API-side recovery. Add MaxProviderEgressLocationSubmissionSkew (5m) alongside the existing MaxProviderEgressLocationSubmissionAge check, and reject observed_at further in the future than that. - provider_egress_location.asn was `int` (Postgres int4, max ~2.147e9); ASNs are 32-bit unsigned (max ~4.295e9), so a real ASN like 4200000000 panicked pgx's arg encoding after a ~78s retry-storm hang. The table is new in this same unmerged PR, so the fix edits the CREATE TABLE migration directly (asn int -> asn bigint) rather than adding a second migration. - Removed a "fix(beta)" fork marker comment from model/network_client_location_model.go, rewritten to be vendor-neutral while keeping the NULL-city/region panic explanation intact. - arinForeignScore's doc comment said its second argument was "the mmdb-resolved country ... or the probed egress country" -- a later commit made both callers pass the mmdb country for ranking parity, so the doc was updated to describe what the code actually does and why. - SetConnectionLocation mapped the probed Mobile flag onto NetTypeVirtual, but IpInfo has no Mobile concept and NetTypeVirtual is only ever set from the ipinfo schema's is_satellite field. This gave probed mobile providers a ranking penalty an identical unprobed provider never takes -- the opposite of the parity this feature promises. Mobile stays in the model/wire contract as metadata but no longer feeds net_type_virtual; Hosting/Proxy keep feeding their scores since they do have direct mmdb-path equivalents (ipInfo.Hosting/ipInfo.Privacy). Tests added: future/within-skew observed_at rejection and acceptance, large-ASN round-trip, and a Mobile-does-not-set-net_type_virtual assertion folded into the existing probed-flags-to-scores test. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
The operator's prober decided what to probe from an in-memory ttl cache, so a restart re-probed the whole provider population and nothing durable recorded what was actually due. The freshness data already exists server-side in provider_egress_location.observed_at; this exposes it so the server owns the schedule. model.GetProviderEgressLocationDue sources candidates from the live provider population (network_client_location_reliability, connected + valid) and LEFT JOINs the egress row, rather than selecting from provider_egress_location. The dominant case is a provider that has never been probed and so has no egress row at all; selecting from that table would return exactly the providers that least need probing and none of the ones that most do. Only providers holding a Public provide key are returned. Probing tunnels through the provider itself, i.e. opens a contract from outside the provider's own network, which a provider without a Public key refuses -- offering one to the prober would burn a probe slot on a guaranteed failure. Same EXISTS filter UpdateClientLocations and UpdateClientScores already apply. The staleness cutoff is computed in Go and bound as a query argument. observed_at is a naive `timestamp` holding utc, so comparing it against sql now() would cast through the session timezone and silently skip a window. GET /network/provider-egress-due is operator-to-server, authenticated by the same X-UR-Operator-Secret shared secret as the ingest endpoint (hmac.Equal against the memoized, fail-closed operatorIngestSecret) rather than a network jwt. Its cutoff is half ProviderEgressLocationMaxAge: at the full max age every location would lapse to the mmdb fallback at the exact moment it became due and stay lapsed until the prober reached it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…t succeed The only thing that moved a provider off the head of the due queue was a successful probe writing provider_egress_location. A provider that connects and holds a Public provide key but always fails to probe -- firewalled egress, dead upstream, anything other than the missing Public key the query already screens for -- never gets a row, so its observed_at stays NULL, so it sorts ahead of every stale-but-refreshable provider on every poll, forever. Five hundred such providers and a prober asking for limit=500 gets the same five hundred dead providers every time; no healthy provider's location is ever refreshed. It fails silently: the endpoint keeps returning a full, plausible-looking batch. The in-memory ttl cache this replaced was incidentally immune, because it marked a provider probed whether or not the probe worked; moving the schedule server-side dropped that protection. Record attempts, not just successes. provider_egress_probe_attempt is a small table keyed by client_id -- the attempt cannot live on provider_egress_location, because the case it exists to handle is precisely a provider with no row there. GetProviderEgressLocationDue now requires both no fresh success and no recent attempt, with a much shorter backoff for attempts (6h) than for success freshness (half ProviderEgressLocationMaxAge, 3.5d): a failing provider should be retried periodically, just not on every poll. Both cutoffs are computed in Go and bound as arguments -- attempt_at, like observed_at, is a naive `timestamp` holding utc, and comparing it to sql now() casts through the session timezone. The prober reports an attempt to POST /network/provider-egress-attempt, on the same operator-authenticated surface as the other two endpoints (X-UR-Operator-Secret, hmac.Equal, the memoized fail-closed operatorIngestSecret). The server timestamps the attempt rather than trusting the prober's clock. Attempt rows are swept by the existing expiry task. This pulls probe_attempt_at/probe_failure forward from the P2 data model in docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md, because P1's schedule cannot function without them. Only those two columns, not the rest of the verdict model. Also adds a client_id secondary sort key, so batch composition under a limit is deterministic rather than plan-dependent -- the whole never-probed population otherwise ties on NULL observed_at. Tests: a provider never probed successfully but attempted seconds ago must not be due, must not take the single batch slot from a stale-but-refreshable provider, and must become due again once the backoff elapses; the same over http via the attempt endpoint. Plus the two coverage gaps review flagged: deleting either the connected or the valid predicate now fails a test, and the handler's staleness cutoff is exercised by a probed (not merely never-probed) provider. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
The egress prober measures a provider's real egress location by tunnelling through that provider and asking public geolocation apis what address they see. It must never reach those apis any other way: a direct lookup records the OPERATOR's location for the provider and hands the operator's address to third-party apis. The prober verifies this itself at startup and refuses to run without evidence, but a self-check only proves a confinement that something else supplies. A docker deployment can supply it with an internal-only network. This is the equivalent for a deployment that uses no docker: IPAddressDeny=any plus IPAddressAllow for the platform and loopback only, kernel-enforced through systemd's cgroup BPF filter, so it needs no container, no namespace and no capability grant. DynamicUser=yes, NoNewPrivileges=yes, empty CapabilityBoundingSet. The doc states that IPAddressAllow takes addresses and not hostnames, so the platform's addresses have to be listed literally and updated when they change; that a stale address fails in the safe direction (no probes, no leak); and that widening the rule to cope -- or the filter not being enforced at all, on a host without cgroup v2 or BPF -- is caught by the prober's own self-check, which finds a geolocation api reachable and refuses to start. It also resolves the DNS question the self-check forces. The check fails closed when it cannot resolve the geolocation hosts, so a unit that denies everything produces a prober that will not start rather than a confined one. The unit allows loopback so the local stub resolver still answers: resolved is a separate unrestricted service, so it performs the upstream query while the prober's own egress stays denied, and the check resolves the hosts and then discovers it cannot connect to them. The alternative, for a host with no dns at all, is -confinement-address, which is documented alongside it. -skip-confinement-check is deliberately absent, with a section on why it must not be added: a check disabled in a unit file is not a check. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
Written as "fails with EPERM at connect() time". Running the prober under the documented unit on a real host shows otherwise: systemd's cgroup BPF filter DROPS the packet, so every dial runs to its own deadline instead of failing fast. 6 addresses x --confinement-timeout 3s = ~18s of startup before egress-prober: confinement self-check passed: 6 address(es) tested, none directly reachable which matters, because an operator reading "fails at connect() time" would read an 18s pause as a hang and reach for a shorter --confinement-timeout -- the one change that makes the check report a pass having tested nothing (the binary refuses anything below 500ms for exactly that reason). Also records the smoke test's real output (curl: (28) Connection timed out) rather than "rc != 0", and the healthy log lines. Same host, same session, the widening mistake the doc warns about was exercised too: IPAddressAllow=any in place of the platform's addresses -> "a direct connection to a geolocation address succeeded; this process is not confined: 134.119.216.174:443", exit 1. The self-check does catch it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
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.
Stacked on #410, which is stacked on #407 (
provider_egress_locationis not inmainyet, so the whole chain is rooted there). GitHub cannot target a branch in a fork, so the base here ismainand the diff shows the parents too — only the last commit belongs to this PR:docs(operator): a systemd unit that confines the egress prober.Why
The operator-run egress prober measures a provider's real exit location by tunnelling through that provider and asking public geolocation APIs what address they see. It must never reach those APIs any other way — a direct lookup records the operator's location for the provider and hands the operator's address to third-party APIs.
The prober verifies that itself at startup and refuses to run when it cannot obtain evidence. But a self-check only proves a confinement that something else supplies. A Docker deployment can supply it by attaching the prober to an
internal: truenetwork and to no other. This document is the equivalent for a deployment that uses no Docker at all.What it contains
A complete, copy-pasteable unit:
IPAddressDeny=anyplusIPAddressAllow=for loopback and the platform only. Kernel-enforced through systemd's cgroup BPF filter, so it needs no container, no network namespace and no capability grant.DynamicUser=yes,NoNewPrivileges=yes, emptyCapabilityBoundingSetandAmbientCapabilities. The provider tunnel is a userspace gvisor netstack, not a kernel tun device, so the prober needs no privileges at all.EnvironmentFile=, which systemd reads as PID 1 before dropping privileges — so the file stays root-owned0600while the service runs unprivileged, and neither secret appears inpsorsystemctl show.--skip-confinement-checkis deliberately absent, with a section on why it must never be added: a check disabled in a unit file is not a check.It states explicitly that
IPAddressAllowtakes addresses, not hostnames, so an operator has to list the platform's addresses literally and update them when they change. A stale address fails in the safe direction (the prober cannot reach the platform, every pass logs a failure, nothing leaks). Widening the rule to cope —IPAddressAllow=any, or a whole/8— is the dangerous direction, and the prober's own self-check is what catches it: it finds a geolocation address directly reachable and exits 1. The same holds when the filter is not being enforced at all, on a host without cgroup v2 or BPF.The DNS trap
The self-check fails closed: if it cannot resolve the geolocation hostnames it exits with
ErrNoEvidence, because inability to verify is not evidence of confinement. A naiveIPAddressDeny=anyunit therefore does not produce a confined prober — it produces one that will not start.The unit resolves this by allowing loopback, so the local stub resolver still answers.
systemd-resolvedis a separate, unrestricted service, so it performs the upstream query while the prober's own egress stays denied: the check resolves the hosts, tries to connect, getsEPERMfrom the BPF filter, and starts. That is the healthy case and needs no hand-maintained address list. The document also covers the alternative for a host with no DNS at all — the repeatable--confinement-address <ip:port>— and when to use which.There is a verification section too: how to confirm systemd actually accepted the rules, and a
systemd-runone-liner that proves the deny bites on this host before trusting it with the prober.Note on scope
The corresponding beta change (an
egress-proberCompose service on a dedicatedinternal: truenetwork, plus its BETA.md section) is not included:docker-compose.beta.ymlandBETA.mdare beta-only and do not exist in this repository. Only the operator documentation is carried here.That beta service is where the confinement was verified live, against a running stack: on the internal network the prober logs
confinement self-check passed: 4 address(es) tested, none directly reachableand stays up; with an internet-reachable network added it logsa direct connection to a geolocation address succeeded; this process is not confined: 134.119.216.174:443and exits 1 on every restart. The systemd unit is the same confinement expressed withIPAddressDenyinstead of a network attachment; the unit itself has not been run on a systemd host from this branch.🤖 Generated with Claude Code
https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg