feat(beta,docs): confine the egress prober in both deployment styles#8
Open
Ryanmello07 wants to merge 9 commits into
Open
feat(beta,docs): confine the egress prober in both deployment styles#8Ryanmello07 wants to merge 9 commits into
Ryanmello07 wants to merge 9 commits into
Conversation
The 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 now verifies this itself at startup and refuses
to run without evidence, but a self-check only proves a confinement that
something else supplies. This is that something else, for both deployment
styles.
Beta (docker compose): a new `egress-prober` service on a new `prober`
network with `internal: true` -- no gateway, no NAT, so the container can
reach `api` and `connect` and nothing else. The attachment IS the
confinement, so `api` and `connect` join that network too and the prober
joins nothing else. Behind the `prober` profile so beta-setup.sh neither
builds nor starts it, since it needs a client jwt only an operator can mint.
Secrets come from a gitignored beta-vault/prober.env, read as environment so
they stay out of `docker inspect` and `ps`.
Mainstream (no docker at all): docs/operator/prober-systemd.md, a complete
unit using IPAddressDeny=any with 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, and that widening the rule to cope is caught by the prober's own
self-check.
The two environments answer the DNS question differently, because the
self-check fails closed when it cannot resolve the geolocation hosts
(ErrNoEvidence -- inability to verify is not evidence of confinement):
- an internal docker network cannot resolve external names at all
(SERVFAIL, the embedded resolver has no route to forward the query), so
the beta service passes -confinement-address for each endpoint and the
check dials known addresses without dns;
- the systemd unit allows loopback instead, so the local stub resolver
still answers, the check resolves the hosts and then discovers it cannot
connect to them -- the healthy case, with no hand-maintained address
list.
Neither config sets -skip-confinement-check, and both say why not.
Verified against the live beta stack without recreating any existing
service: on the prober network the container starts, logs "confinement
self-check passed: 4 address(es) tested, none directly reachable" and stays
up; with `default` added to it, it logs "a direct connection to a
geolocation address succeeded; this process is not confined" and exits 1 on
every restart.
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
A full active census at 50MB per provider is ~5TB per round at 100k providers, and because probes are real paid contracts that is money leaving the operator and arriving at providers. Not worth it for a ranking input. transfer_escrow.payout_byte_count already records bytes actually delivered per contract, so throughput can be derived from real user traffic at zero additional cost -- and it cannot be inflated without genuinely serving real users fast. Aggregated per provider only; this is a new use of billing data, not new collection. Active probing narrows to what passive cannot answer: providers with no traffic history, and periodic calibration spot-checks. Sampled rather than census, 5MB rather than 25MB per target, one target routinely and the second only to corroborate a suspected result. Lands at ~25GB per round -- cheaper than the existing platform test while measuring the path users actually get. Also records why the existing test is unusable: 512KiB completes inside TCP slow start above ~50Mbit, so it is precise-looking noise on the wrong path. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
connect only trusts a forwarded client address when it receives BOTH X-Forwarded-For and X-Forwarded-Source-Port (connect/transport.go:220); that pair is what the warp load balancer sends. Caddy set only the first, so connect fell back to the raw peer address and recorded Caddy's private container address for every client. A private address has no mmdb entry, so GuessLocationType returned "Unknown location type", no network_client_location row was written, nothing reached network_client_location_reliability, and /network/provider-locations returned zero locations. Caddy now sends the source port, and the raw 8080/5080/15080 publishes are removed so the tls hostnames are the only ingress -- a client arriving on a published port presents the docker gateway address and fails the same way. Verified: 80 connected / 80 with_location and zero location errors, from 0 / 40 before. Note for anyone editing the Caddyfile: it is bind-mounted as a single file, so replacing it on the host leaves the container on the old inode. `up -d --force-recreate caddy` is required; an edit plus `caddy reload` silently reloads the old content. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…ble header Two problems the P1 final review found. The incident-fix commit dropped `- prober` from api and connect in the same hunk that removed the published ports, so the committed compose leaves the prober alone on an internal-only network: it starts, passes its confinement self-check, and can never reach the server. The running deployment was only correct because of an uncommitted working-tree edit, and BETA.md's prescribed `up -d api connect` would have detached the live containers and silently killed it. connect trusts X-UR-Forwarded-For ahead of every other source of the client address (connect/transport.go:220), and the api derives the session address from the same value. A proxy passes client headers through verbatim, so with Caddy as the sole ingress any client could set that header and pin itself to an address of its choosing -- forging its provider location and evading the per-address rate limits. Caddy now strips it inbound; only the warp load balancer is entitled to set it and it is not in this path. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…rved_at Backports two fixes that landed upstream on the provider-egress-geolocation branch but are missing from beta's older copy of the feature. 1. SetConnectionLocation no longer maps a probed egress's Mobile flag onto NetTypeVirtual. Unlike Hosting/Proxy, Mobile has no mmdb-path equivalent: IpInfo has no Mobile concept, and NetTypeVirtual is only ever set from the ipinfo schema's is_satellite field, never from DB-IP. Deriving it from Mobile penalized a probed mobile provider's ranking with no equivalent penalty for an otherwise-identical unprobed one -- the opposite of the parity this feature exists to preserve. This is live on beta today: a probed Philippine provider on Globe Telecoms reports mobile=true and is being penalized for having been probed. 2. SubmitProviderEgressLocation now rejects an observed_at further in the future than MaxProviderEgressLocationSubmissionSkew (5m). Freshness is judged against observed_at, so an unbounded future value defeated every other safeguard at once: it always won the monotonic upsert, read as fresh forever, and outlived the taskworker sweep, pinning a provider's location indefinitely with no API-side recovery. The skew constant tolerates ordinary prober/server clock drift without opening that door. The explanatory comments are carried over from upstream deliberately: both fixes are non-obvious and read as removable without the reasoning attached. Tests ported from upstream, each verified to fail without its fix: TestSetConnectionLocationMapsProbedFlagsToScores now submits Mobile:true and asserts net_type_virtual stays 0 while hosting/privacy still map, so it proves the distinction rather than a wholesale deletion; plus TestSubmitProviderEgressLocationRejectsFutureObservedAt and ...AcceptsWithinSkewObservedAt, the latter guarding against an over-strict bound that would reject legitimate drift. Beta-only: upstream already carries both fixes at 6b5de2b. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…y list The raw 8080/5080/15080 publishes are gone; Caddy is the only ingress. The docs still told operators to open those ports and gave http:// and ws:// endpoints that no longer answer. Records why the unpublished ports are load-bearing rather than cosmetic: connect trusts a forwarded client address only when it receives both X-Forwarded-For and X-Forwarded-Source-Port, so a client arriving on a published port presents the docker gateway address, gets no mmdb match, and never receives a location -- which silently empties the provider list. Adds a troubleshooting entry for exactly that symptom, including the single-file bind mount trap: editing the Caddyfile on the host replaces the inode, the container keeps the old one, and `caddy reload` reports success while reloading stale content. Also notes --http1.1 for the websocket check, since HTTP/2 rejects the Upgrade header and makes a healthy service look broken. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…ders discoverable to their own network
Two production risks in the provider counting/scoring change, found by a
backward-compatibility audit of this PR.
1. Country-only clients were counted three times.
A client whose geo lookup resolved neither a city nor a region is stored
with city_location_id = region_location_id = country_location_id --
SetConnectionLocation writes the coarsest available id into the NOT NULL
city/region columns. Both fan-out loops then walked city, region and
country unconditionally, so one such client added 3 to its own country's
provider count and was inserted three times into its country's scoring
map. The inflation is worst exactly where geo resolution is coarsest:
datacenter, mobile and VPN egress.
Both loops now go through the distinct set of location ids, so a client
counts once per location it is actually in. A genuinely city-granular
client still rolls up into its region and its country unchanged -- that
half is asserted explicitly, because a dedupe keyed on the client rather
than on (client, location) would silently stop city clients counting
toward their country, a worse regression than the one being fixed.
(The per-location and per-group scoring maps are keyed by client id, so
they already absorbed the repeat; the count was the live defect. The
loops are made explicit anyway so the two stay in step.)
2. Network-only providers vanished from their own network's discovery.
The Public-only filter is right for the public provider count -- a
stranger genuinely cannot use a ProvideModeNetwork provider -- but wrong
for the candidate pool. Such a provider serves same-network sources today
via the working CreateContractNoEscrow path and is discoverable to them
now; filtering the pool to Public made it undiscoverable to exactly the
users it exists for. That is a live regression for anyone running
providers for their own organisation.
The pool must also not hand a cross-network caller a provider whose
contract will be refused, so this is not a revert:
- UpdateClientLocations keeps the Public-only filter. That number is
shown to everyone and should reflect what a stranger can reach.
- The UpdateClientScores source queries now admit Public or Network --
exactly the two modes GetProvideRelationship can return, hence
exactly what CreateContract can accept -- and record NetworkOnly on
each ClientScore.
- FindProviders2 filters at request time: a candidate is eligible if it
is publicly usable, or if its NetworkId equals the requesting
session's network. This cannot be baked into the cache: the client
score redis entries are keyed by (forceMinimum, rankMode, locationId,
callerLocationId) with no network component, so one cached set is
shared by callers from every network.
NetworkOnly is stored negated deliberately. The score cache is gob
encoded with a 5h ttl, so entries written before the field existed decode
with the zero value; the zero value therefore has to mean "publicly
usable", or every provider would be treated as network-only until the
cache turned over.
The ClientFilter exported alongside the samples still counts only
publicly usable providers. It is read solely by loadLocationStables,
which decides the public Stable flag GetProviderLocations publishes -- a
public surface, so it follows the same rule as the provider count. A
location whose only supply is network-only is not stable and reports no
providers.
No Stream carve-out: resolveNonCompanionProvideMode does return
companion=true for a Stream-only destination, but that dead-ends at
CreateCompanionTransferEscrow, which requires a pre-existing
reverse-direction origin contract, so a Stream-only destination can never
bootstrap a cross-network session. The inline comments that implied
Stream was the only excluded case are corrected.
Tests: a country-only provider counted exactly once and a city provider
still counted at all three granularities, for both UpdateClientLocations
and UpdateClientScores; and all four visibility cases in FindProviders2
(Public and Network-only providers x same-network and other-network
callers). The two existing pool assertions that encoded the Public-only
pool are updated to the new two-tier contract; their location-stability
assertions are unchanged and still pass.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…yment-confinement # Conflicts: # model/network_client_location_model_test.go
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 #7 (
feat/probe-due-endpoint), which is stacked on #6, which is stacked on #5. Review only the last commit; retarget tobeta/self-contained-envonce the parents merge.P1 Task 4 of the automated confined probing plan.
Why
The 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 now verifies that at startup and refuses to run without evidence. But a self-check only proves a confinement that something else supplies. This PR is that something else, in both deployment styles: beta runs Docker Compose, the mainstream deployment uses no Docker at all, so the confinement cannot be a Docker construct in general.
Beta:
docker-compose.beta.ymlegress-proberservice on a newprobernetwork withinternal: true— no gateway, no NAT, so it reachesapiandconnectand nothing else. The attachment is the confinement.apiandconnectjoin that network too; the prober joins nothing else, publishes no ports, drops all capabilities.probercompose profile, sobeta-setup.shneither builds nor starts it (it needs a client jwt only an operator can mint).beta-vault/prober.env(.exampletemplate tracked), read as environment so they stay out ofdocker inspectargs andps.../connectand../glogreplaces; overridable withPROBER_CONTEXT/CONNECT_CONTEXT/GLOG_CONTEXT.Mainstream:
docs/operator/prober-systemd.mdA complete, copy-pasteable unit:
IPAddressDeny=anyplusIPAddressAllow=for loopback and the platform only, kernel-enforced through systemd's cgroup BPF filter — no container, no namespace, no capability grant.DynamicUser=yes,NoNewPrivileges=yes, emptyCapabilityBoundingSet.It states that
IPAddressAllowtakes addresses, not hostnames, so the platform's addresses must 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 — is caught by the prober's own self-check, which refuses to start.The DNS question, answered differently per environment
The self-check fails closed when it cannot resolve the geolocation hosts (
ErrNoEvidence: inability to verify is not evidence of confinement). A naive config produces a prober that will not start, so each environment resolves this explicitly:--confinement-address <ip:port>per endpointinternal: truedocker network cannot resolve external names at all — docker's embedded resolver answers SERVFAIL, having no route to forward the query (verified)systemd-resolvedis a separate unrestricted service; the check resolves the hosts and then finds it cannot connect to them — the healthy case, and no hand-maintained address listNeither config sets
--skip-confinement-check, and both say why not: a check disabled in a config file is not a check.Verified live
Against the running beta stack, without stopping, restarting or recreating any existing service (
up -d --no-deps egress-prober;docker compose psshowed api/connect/postgres/redis/caddy/taskworker all still at their original uptimes).Confined — starts and stays up:
Not confined — same service,
defaultadded to it via a throwaway compose override:Also verified: an
internal: truenetwork gives SERVFAIL for external names but still resolves and reaches other containers on it (two throwaway containers,HTTP/1.1 200 OK), which is what makesapiandconnectreachable from the confined prober.Not verified: the prober actually talking to this deployment's
api, because that needsapiandconnectrecreated to join theprobernetwork and the live stack was left untouched. BETA.md documents that one-time step.🤖 Generated with Claude Code
https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg