From 2687da23d53796fe3824877b556d7a9433242c42 Mon Sep 17 00:00:00 2001 From: Ryan Mello Date: Sun, 26 Jul 2026 11:34:15 +0100 Subject: [PATCH 1/8] feat(beta,docs): confine the egress prober in both deployment styles 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 Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg --- .gitignore | 2 + BETA.md | 119 ++++++++++++++++ beta-vault/prober.env.example | 21 +++ docker-compose.beta.yml | 113 +++++++++++++++ docs/operator/prober-systemd.md | 243 ++++++++++++++++++++++++++++++++ 5 files changed, 498 insertions(+) create mode 100644 beta-vault/prober.env.example create mode 100644 docs/operator/prober-systemd.md diff --git a/.gitignore b/.gitignore index ce36f96d..0e40b30b 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,8 @@ beta-vault/config/**/*.mmdb beta-vault/vault/tls/**/*.pem beta-vault/vault/tls/**/*.pub.pem beta-vault/beta-secrets.env +# egress-prober client jwt and operator secret; the .example template is tracked +beta-vault/prober.env beta-vault/vault/*.yml # Keep .example templates tracked !beta-vault/vault/*.yml.example diff --git a/BETA.md b/BETA.md index ecd125b3..99062eb8 100644 --- a/BETA.md +++ b/BETA.md @@ -17,6 +17,7 @@ The server auto-detects its public IP and binds the API and connect services so - [Web dashboard](#web-dashboard) - [Connect client / provider](#connect-client--provider) - [SOCKS5 proxy](#socks5-proxy) +- [Provider egress prober](#provider-egress-prober) - [Core API routes](#core-api-routes) - [What works](#what-works) - [What is disabled](#what-is-disabled) @@ -321,6 +322,120 @@ This creates an authenticated proxy client via `/network/auth-client`, selects a To generate a WireGuard config instead of SOCKS5, open an issue on the proxy fork or use the dashboard's wireguard flow once it is wired to the beta API. +## Provider egress prober + +The `egress-prober` service measures where each provider's traffic actually +comes out. For every provider it opens a tunnel **through that provider** and +asks public geolocation APIs what address they see, then submits the answer to +the server's provider-egress ingest endpoint, so the server can prefer the +measured location over the built-in mmdb lookup. + +It lives in a separate repository: +`https://github.com/Ryanmello07/urnetwork-operator-proxy`. + +### The confinement is the network attachment + +The prober must never reach a geolocation API except through a provider tunnel. +A direct lookup would record **this host's** location for the provider and hand +the operator's own address to third-party APIs. + +That is enforced by the `prober` network in `docker-compose.beta.yml`, which is +`internal: true`: containers on it have no gateway and no NAT, so nothing on it +can reach the internet. The prober is on that network and no other, which lets +it reach `api` and `connect` and nothing else. Do not attach it to `default`, +do not publish ports from it, and never add `--skip-confinement-check` — a +check disabled in a config file is not a check. + +The prober verifies its own confinement at startup and exits non-zero if it is +wrong, so a misconfigured network stops the prober instead of leaking silently. + +Because an `internal: true` network cannot resolve external names at all +(docker's embedded resolver answers SERVFAIL, having no route to forward the +query), the self-check would otherwise be unable to resolve the geolocation +hosts, would obtain no evidence either way, and would refuse to start. The +service therefore passes `--confinement-address` for each geolocation endpoint, +which keeps the check real without needing DNS. Those addresses are a second +copy of the endpoint list — keep them in step with the hostnames the prober +logs on every start. + +For the non-Docker deployment see +[docs/operator/prober-systemd.md](docs/operator/prober-systemd.md), which +confines the process with `IPAddressDeny=any` instead and takes the other route +on DNS: it allows the loopback stub resolver, so the check resolves the hosts +and then finds them unreachable. + +### Secrets + +Copy the template and fill in both values: + +```bash +cp beta-vault/prober.env.example beta-vault/prober.env +chmod 600 beta-vault/prober.env +``` + +| Variable | Where it comes from | +|---|---| +| `UR_PROBER_BY_JWT` | A network **client** jwt: create a network, then `POST /network/auth-client` with the dashboard jwt and use the `by_client_jwt` it returns. A plain user jwt has no `client_id` and the prober exits with `parse by-jwt client id`. | +| `UR_OPERATOR_SECRET` | Must equal `ingest_secret` in `beta-vault/vault/provider_egress.yml`, generated by `./beta-setup.sh`: `grep ingest_secret beta-vault/vault/provider_egress.yml`. | + +The real `beta-vault/prober.env` is gitignored; only the `.example` template is +tracked. Both secrets are passed as environment variables, never on the command +line, so they do not appear in `docker inspect` args or in `ps`. + +### Starting it + +The service sits behind the `prober` compose profile, so `./beta-setup.sh` +neither builds nor starts it. Clone the prober repository next to this one (the +same parent directory `Dockerfile.beta` builds from) and start the service by +name: + +```bash +git clone https://github.com/Ryanmello07/urnetwork-operator-proxy.git ../urnetwork-operator-proxy +docker compose -f docker-compose.beta.yml build egress-prober +docker compose -f docker-compose.beta.yml up -d --no-deps egress-prober +``` + +The prober's `go.mod` replaces `github.com/urnetwork/connect` and `.../glog` +with `../connect` and `../glog`, so those checkouts take part in the build too. +`PROBER_CONTEXT`, `CONNECT_CONTEXT` and `GLOG_CONTEXT` override the three build +contexts if your checkouts live elsewhere. + +> **One-time step**: `api` and `connect` now also attach to the `prober` +> network, since the prober has no other way to reach them. An already-running +> stack keeps its old attachments until those two containers are recreated: +> `docker compose -f docker-compose.beta.yml up -d api connect`. Until then the +> prober starts and stays confined, but every pass logs +> `lookup api on 127.0.0.11:53: server misbehaving`. + +### Confirming the self-check passed + +```bash +docker compose -f docker-compose.beta.yml logs --tail=20 egress-prober +``` + +Confined and healthy — the container keeps running: + +``` +egress-prober: confinement self-check: dialing the 4 address(es) given with -confinement-address, skipping resolution: 134.119.216.174:443 ... (geolocation hosts: ip.pn free.freeipapi.com ipinfo.io -- keep these addresses current with that list) +egress-prober: confinement self-check passed: 4 address(es) tested, none directly reachable +``` + +Not confined — the container exits 1 and, under `restart: unless-stopped`, +restarts into the same failure rather than probing: + +``` +egress-prober: confinement self-check failed: confinement: a direct connection to a geolocation address succeeded; this process is not confined: 134.119.216.174:443 +egress-prober: this process must not be able to reach a geolocation api except through a provider tunnel. Confine it ... and start it again. +``` + +Check the exit code and the attached networks directly with: + +```bash +docker inspect -f 'exit={{.State.ExitCode}} restarts={{.RestartCount}} nets={{range $k,$v := .NetworkSettings.Networks}}{{$k}} {{end}}' server-egress-prober-1 +``` + +`nets` must be `server_prober` and nothing else. + ## Core API routes | Route | Method | Purpose | @@ -389,6 +504,10 @@ beta-vault/config/mmdb/ip-ipinfo.mmdb beta-vault/config/arindb/arin.mmdb ``` +`beta-vault/prober.env` is also gitignored. It is not generated — you create it +from `beta-vault/prober.env.example` if you run the +[provider egress prober](#provider-egress-prober). + To share the beta environment with a teammate, share the repository and have them run `./beta-setup.sh` on their own host. Never share `beta-secrets.env` or the PEM keys between independent deployments. ## Architecture diff --git a/beta-vault/prober.env.example b/beta-vault/prober.env.example new file mode 100644 index 00000000..7b13acb1 --- /dev/null +++ b/beta-vault/prober.env.example @@ -0,0 +1,21 @@ +# Secrets for the egress-prober service in docker-compose.beta.yml. +# +# Copy to beta-vault/prober.env and fill both values in. The real file is +# gitignored; this template is not. Never commit the real one. +# +# cp beta-vault/prober.env.example beta-vault/prober.env +# +# UR_PROBER_BY_JWT +# A network CLIENT jwt for the network the prober probes from. Create a +# network, then POST /network/auth-client with the dashboard jwt and use the +# by_client_jwt it returns. It must contain a client_id -- a plain user jwt +# has none and the prober exits with "parse by-jwt client id". +# +# UR_OPERATOR_SECRET +# Must equal ingest_secret in beta-vault/vault/provider_egress.yml, which +# ./beta-setup.sh generates. Read it with: +# grep ingest_secret beta-vault/vault/provider_egress.yml +# The api process memoizes it on first use, so restart api after rotating it. + +UR_PROBER_BY_JWT= +UR_OPERATOR_SECRET= diff --git a/docker-compose.beta.yml b/docker-compose.beta.yml index 2d04acf7..1035c1ad 100644 --- a/docker-compose.beta.yml +++ b/docker-compose.beta.yml @@ -91,6 +91,13 @@ services: BRINGYOUR_REDIS_HOSTNAME: redis WARP_HOME: /srv/warp command: ["api", "--port=8080"] + # `default` must stay listed: naming any network at all stops compose from + # attaching the default one implicitly, and postgres, redis and caddy are + # all on it. `prober` is the internal-only network the egress-prober lives + # on -- see that service for why it cannot be on `default`. + networks: + - default + - prober ports: - "8080:8080" depends_on: @@ -120,6 +127,10 @@ services: WARP_HOME: /srv/warp WARP_PORTS: "80:5080,15080:15080" command: ["connect", "--port=80"] + # see the api service above for why `default` has to be repeated here + networks: + - default + - prober ports: - "5080:80" - "15080:15080" @@ -133,6 +144,99 @@ services: volumes: - ./beta-vault:/srv/warp:ro + # Operator-run prober. For each provider it opens a tunnel THROUGH that + # provider and asks public geolocation APIs what address they see, so the + # server learns the provider's real egress location. + # + # It must never be able to reach those APIs directly. A direct lookup would + # record THIS host's location for the provider and hand the operator's own + # address to third-party APIs. That is what the `prober` network below is + # for: it is `internal: true`, so the container has no default route, no NAT + # and no path off the docker bridge. It reaches `api` and `connect` and + # nothing else. + # + # The confinement IS the network attachment. Do not add this service to + # `default`, do not publish ports from it, and never pass + # --skip-confinement-check: a check disabled in a config file is not a check. + # The prober verifies its own confinement at startup and exits non-zero if it + # can reach a geolocation address, so a wrong network stops the prober + # instead of silently leaking. + # + # Not started by default (profile `prober`), because it needs a network + # client jwt that only an operator can mint. Start it explicitly: + # docker compose -f docker-compose.beta.yml up -d --no-deps egress-prober + egress-prober: + profiles: + - prober + build: + # The prober lives in its own repo. Clone it next to this one (the same + # urnetwork parent directory Dockerfile.beta builds from), or point + # PROBER_CONTEXT at the checkout: + # git clone https://github.com/Ryanmello07/urnetwork-operator-proxy.git + context: ${PROBER_CONTEXT:-../urnetwork-operator-proxy} + # Its go.mod replaces github.com/urnetwork/connect and .../glog with + # ../connect and ../glog, so those two checkouts have to be in the build + # as well. Named contexts keep each one small -- a single context over + # the whole urnetwork parent directory is ~1.6G to transfer. + additional_contexts: + connect: ${CONNECT_CONTEXT:-../connect} + glog: ${GLOG_CONTEXT:-../glog} + dockerfile_inline: | + FROM golang:1.26.5-alpine AS build + # layout mirrors the local urnetwork monorepo so the ../ replaces resolve + WORKDIR /src/urnetwork-operator-proxy + COPY . . + COPY --from=connect / /src/connect + COPY --from=glog / /src/glog + RUN CGO_ENABLED=0 go build -trimpath -o /out/egress-prober ./cmd/egress-prober + FROM alpine:3 + RUN apk add --no-cache ca-certificates + COPY --from=build /out/egress-prober /usr/local/bin/egress-prober + # the prober needs no privileges: the provider tunnel is a userspace + # gvisor netstack, not a kernel tun device + USER 65534:65534 + ENTRYPOINT ["egress-prober"] + restart: unless-stopped + networks: + - prober + cap_drop: + - ALL + security_opt: + - no-new-privileges:true + # UR_PROBER_BY_JWT and UR_OPERATOR_SECRET. Kept out of the command line so + # they never appear in `docker inspect` args or in ps inside the container. + # required:false so the whole compose project still loads before an + # operator has created the file. + env_file: + - path: ./beta-vault/prober.env + required: false + command: + - --api-url=http://api:8080 + - --platform-url=ws://connect:80/ + - --interval=1h + - --concurrency=2 + - --probe-timeout=60s + - --confinement-timeout=3s + # DNS vs -confinement-address, for this environment: an `internal: true` + # docker network cannot resolve external names at all -- docker's + # embedded resolver answers SERVFAIL because it has no route to forward + # the query. The startup self-check would then resolve none of the + # geolocation hosts, obtain no evidence either way, and REFUSE TO START + # (ErrNoEvidence): being unable to verify is not evidence of confinement. + # So we hand it the addresses to dial instead of asking it to resolve + # them, which keeps the check real inside a jail where dns is blocked. + # + # These are a second copy of the endpoint list and can go stale. Keep + # them in step with the hosts the prober prints on every start + # ("geolocation hosts: ..."); re-resolve with + # for h in ip.pn free.freeipapi.com ipinfo.io; do getent ahostsv4 $h; done + # A stale address weakens the check (it tests an IP nobody serves any + # more) but can never produce a false "not confined". + - --confinement-address=134.119.216.174:443 # ip.pn + - --confinement-address=104.21.94.136:443 # free.freeipapi.com (cloudflare) + - --confinement-address=172.67.168.79:443 # free.freeipapi.com (cloudflare) + - --confinement-address=34.117.59.81:443 # ipinfo.io + caddy: image: caddy:2-alpine restart: unless-stopped @@ -149,6 +253,15 @@ services: - api - connect +networks: + # The egress-prober's confinement. `internal: true` gives containers on this + # network no gateway and no NAT, so nothing attached to it can reach the + # internet -- only the other containers that are also on it (api, connect). + # Removing `internal: true`, or adding another network to the prober, undoes + # the confinement; the prober's startup self-check is what catches that. + prober: + internal: true + volumes: # ⚠️ Named volume that persists Postgres data across container restarts. # Deleting this volume destroys ALL user data. Back up before teardown: diff --git a/docs/operator/prober-systemd.md b/docs/operator/prober-systemd.md new file mode 100644 index 00000000..d8338a2b --- /dev/null +++ b/docs/operator/prober-systemd.md @@ -0,0 +1,243 @@ +# Running the egress prober under systemd + +The egress prober measures each provider's real exit location by opening a tunnel +**through that provider** and asking public geolocation APIs what address they see. +It must never be able to reach those APIs any other way. A direct lookup would +record the *operator's own* location for the provider and hand the operator's +address to third-party APIs. + +The beta stack confines the prober with a docker network (`egress-prober` in +`docker-compose.beta.yml`, on an `internal: true` network). The mainstream +deployment uses no Docker, so this page gives the equivalent: a systemd unit +whose egress is denied by the kernel. + +`IPAddressDeny=` / `IPAddressAllow=` are enforced by systemd's cgroup/BPF filter. +No container, no network namespace, no capability grant, and no root — the +filter applies to the service's cgroup regardless of the user it runs as. + +## Prerequisites + +- systemd 235 or newer, cgroup v2, and BPF available. If any is missing systemd + logs `Failed to add IP address ... ignoring` and **the address rules do + nothing**. See [Verifying it is actually enforced](#verifying-it-is-actually-enforced). +- The `egress-prober` binary from + `https://github.com/Ryanmello07/urnetwork-operator-proxy` at + `/usr/local/bin/egress-prober`: + + ```bash + git clone https://github.com/Ryanmello07/urnetwork-operator-proxy.git + git clone https://github.com/urnetwork/connect.git # ../connect replace + git clone https://github.com/urnetwork/glog.git # ../glog replace + cd urnetwork-operator-proxy + CGO_ENABLED=0 go build -trimpath -o /usr/local/bin/egress-prober ./cmd/egress-prober + ``` + +## The unit + +Save as `/etc/systemd/system/urnetwork-egress-prober.service`. + +```ini +[Unit] +Description=URnetwork provider egress prober +Documentation=https://github.com/urnetwork/server/blob/main/docs/operator/prober-systemd.md +After=network-online.target nss-lookup.target +Wants=network-online.target + +[Service] +Type=simple +ExecStart=/usr/local/bin/egress-prober \ + --api-url=https://api.example.net \ + --platform-url=wss://connect.example.net \ + --interval=1h \ + --concurrency=4 \ + --probe-timeout=60s \ + --confinement-timeout=3s + +# UR_PROBER_BY_JWT and UR_OPERATOR_SECRET. systemd reads this file as PID 1, +# before dropping privileges, so it can be root-owned 0600 even though the +# service itself runs as an unprivileged dynamic user: +# chown root:root /etc/urnetwork/prober.env && chmod 600 /etc/urnetwork/prober.env +# Passing either secret on the command line would publish it in `ps` and in +# `systemctl show`. +EnvironmentFile=/etc/urnetwork/prober.env + +# --------------------------------------------------------------------------- +# THE CONFINEMENT. Everything below this line is the point of the unit. +# --------------------------------------------------------------------------- +# Deny all IP traffic, then allow back exactly the operator's own platform and +# loopback. Enforced by the kernel through systemd's cgroup BPF filter: a +# connection to anything not listed fails with EPERM at connect() time. +IPAddressDeny=any + +# Loopback. Required so the process can talk to the local DNS stub resolver +# (systemd-resolved on 127.0.0.53). See "DNS" below -- this is what lets the +# self-check resolve the geolocation hostnames and then discover it cannot +# reach them, which is the evidence it needs to start. +IPAddressAllow=localhost + +# The operator's platform. IPAddressAllow TAKES ADDRESSES, NOT HOSTNAMES -- +# see "Addresses, not hostnames" below. Replace these with the addresses of +# your own api and connect hosts: +# getent ahostsv4 api.example.net connect.example.net +IPAddressAllow=198.51.100.10 # api.example.net +IPAddressAllow=198.51.100.11 # connect.example.net +# If your DNS resolver is not on loopback, add its address here too, or use +# --confinement-address (see "DNS" below): +# IPAddressAllow=192.0.2.53 + +# No privileges of any kind. The provider tunnel is a userspace gvisor +# netstack, not a kernel tun device, so the prober needs no capabilities. +DynamicUser=yes +NoNewPrivileges=yes +CapabilityBoundingSet= +AmbientCapabilities= +RestrictAddressFamilies=AF_INET AF_INET6 +PrivateDevices=yes +ProtectKernelTunables=yes +ProtectKernelModules=yes +ProtectControlGroups=yes +RestrictNamespaces=yes +RestrictSUIDSGID=yes +LockPersonality=yes +MemoryDenyWriteExecute=yes +SystemCallFilter=@system-service +SystemCallArchitectures=native + +# A failed confinement self-check is a configuration fault, not a blip. Retry +# a few times in case the platform was briefly unreachable, then stop and stay +# failed so it is visible in `systemctl status` instead of looping forever. +Restart=on-failure +RestartSec=60s +StartLimitIntervalSec=600 +StartLimitBurst=5 + +[Install] +WantedBy=multi-user.target +``` + +```bash +sudo systemctl daemon-reload +sudo systemctl enable --now urnetwork-egress-prober +journalctl -u urnetwork-egress-prober -f +``` + +A healthy start logs: + +``` +egress-prober: confinement self-check: 3 geolocation host(s) -> 4 address(es): ... +egress-prober: confinement self-check passed: 4 address(es) tested, none directly reachable +``` + +## Never set `--skip-confinement-check` + +The flag exists for a one-shot manual probe from a host you know is not the +operator's. A check disabled in a unit file is not a check: it makes the +process start in exactly the situation the unit is meant to prevent, and the +only trace is two `WARNING` lines in the journal. It is not in the unit above +and it must not be added to it. + +## DNS: why loopback is allowed + +The prober's self-check refuses to start when it cannot obtain evidence either +way. If it cannot resolve the geolocation hostnames it exits with +`ErrNoEvidence` — being unable to verify is not evidence of confinement. So a +unit with `IPAddressDeny=any` and nothing allowed back does **not** produce a +confined prober; it produces one that will not start. + +There are two ways to resolve this, and this unit takes the first: + +1. **Permit DNS (this unit).** `IPAddressAllow=localhost` lets the prober reach + the local stub resolver. `systemd-resolved` is a separate, unrestricted + service, so it performs the upstream query on the prober's behalf and the + prober's own egress stays denied. The self-check then resolves + `ip.pn`, `free.freeipapi.com` and `ipinfo.io`, tries to connect to each + address, gets `EPERM` from the BPF filter, and starts. That is the healthy + case, and it needs no hand-maintained list of geolocation addresses. + + If your resolver is *not* on loopback (`/etc/resolv.conf` points at + `192.0.2.53`, say), add that address to `IPAddressAllow` as well — a DNS + server address is not a geolocation API, so allowing it does not weaken the + confinement. + +2. **Skip resolution.** Where DNS is genuinely unavailable, pass the addresses + to dial directly and the check stays real: + + ```ini + ExecStart=/usr/local/bin/egress-prober \ + ... \ + --confinement-address=134.119.216.174:443 \ + --confinement-address=104.21.94.136:443 \ + --confinement-address=172.67.168.79:443 \ + --confinement-address=34.117.59.81:443 + ``` + + The flag is repeatable and rejects hostnames — a name could not be resolved + at dial time either, so accepting one would mean dialling nothing. This is a + second copy of the endpoint list and it can go stale; the prober prints the + hostnames it expects (`geolocation hosts: ...`) on every start so drift is + visible in the journal. The docker-compose beta service takes this route, + because an `internal: true` docker network cannot resolve external names at + all. + +## Addresses, not hostnames + +`IPAddressAllow=` takes IP addresses and CIDR ranges (plus the aliases +`localhost`, `link-local`, `multicast`, `any`). **It does not take hostnames.** +systemd resolves nothing here — there is no name to resolve at packet-filter +time. + +That has two consequences an operator has to live with: + +- You must list your platform's addresses literally, and **update the unit when + they change**. A platform address that moves — a new load balancer, a new + region, a provider migration — breaks the prober silently in the *safe* + direction: it can no longer reach `api` or `connect`, every pass logs a + failure, and no probe is submitted. Nothing leaks. +- The tempting fix is to widen the rule: `IPAddressAllow=any` "just to get it + running", or a whole `/8` because the platform "is somewhere in there". + **That is the dangerous direction, and the prober's own self-check is what + catches it.** On the next start the check finds a geolocation address + directly reachable and refuses to run: + + ``` + egress-prober: confinement self-check failed: confinement: a direct connection + to a geolocation address succeeded; this process is not confined: 134.119.216.174:443 + egress-prober: this process must not be able to reach a geolocation api except + through a provider tunnel. ... + ``` + + Exit code 1, and with the `Restart=on-failure` limits above the unit lands in + `failed`. The same is true if the address rules are not being enforced at all + (no cgroup v2, no BPF, an old systemd): the prober discovers it can reach the + APIs and stops. The confinement and the check are two halves of one + mechanism — the unit restricts, the check proves the restriction is real. + +## Verifying it is actually enforced + +Confirm systemd accepted the rules, rather than trusting that it did: + +```bash +systemctl show urnetwork-egress-prober -p IPAddressDeny -p IPAddressAllow +journalctl -u urnetwork-egress-prober | grep -i 'ip address' # expect no "ignoring" +``` + +Then confirm from the outside that the deny actually bites, using the service's +own settings but a harmless command: + +```bash +sudo systemd-run --uid=nobody \ + -p IPAddressDeny=any -p IPAddressAllow=localhost \ + --wait --pty /bin/sh -c 'curl -sS --max-time 5 https://ipinfo.io/ip; echo rc=$?' +# expect a connect failure (rc != 0), not an address +``` + +If that prints an address, the filter is not being applied on this host and the +prober must not be run here until it is. + +## Alternative: one pass per timer + +`--interval=0` runs a single pass and exits, reporting success or failure in the +exit code, for operators who prefer a timer to a long-running service. Use +`Type=oneshot`, drop `Restart=`, and add a companion +`urnetwork-egress-prober.timer`. Everything in the confinement block above stays +exactly the same — including the self-check, which then runs on every firing. From dbaa7475b00e8c367060c23f4cc368764941d029 Mon Sep 17 00:00:00 2001 From: Ryan Mello Date: Sun, 26 Jul 2026 11:41:11 +0100 Subject: [PATCH 2/8] docs(operator): correct what a denied connection looks like from inside 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 Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg --- docs/operator/prober-systemd.md | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/docs/operator/prober-systemd.md b/docs/operator/prober-systemd.md index d8338a2b..e274274d 100644 --- a/docs/operator/prober-systemd.md +++ b/docs/operator/prober-systemd.md @@ -65,8 +65,12 @@ EnvironmentFile=/etc/urnetwork/prober.env # THE CONFINEMENT. Everything below this line is the point of the unit. # --------------------------------------------------------------------------- # Deny all IP traffic, then allow back exactly the operator's own platform and -# loopback. Enforced by the kernel through systemd's cgroup BPF filter: a -# connection to anything not listed fails with EPERM at connect() time. +# loopback. Enforced by the kernel through systemd's cgroup BPF filter, which +# DROPS the packet rather than rejecting it: a connection to anything not +# listed hangs until its own deadline instead of failing fast. That is what a +# deny rule looks like from inside, and the self-check counts a dial timeout as +# blocked -- but it means the check costs one --confinement-timeout per address +# at startup (6 addresses x 3s = ~18s observed), not one in total. IPAddressDeny=any # Loopback. Required so the process can talk to the local DNS stub resolver @@ -151,8 +155,14 @@ There are two ways to resolve this, and this unit takes the first: service, so it performs the upstream query on the prober's behalf and the prober's own egress stays denied. The self-check then resolves `ip.pn`, `free.freeipapi.com` and `ipinfo.io`, tries to connect to each - address, gets `EPERM` from the BPF filter, and starts. That is the healthy - case, and it needs no hand-maintained list of geolocation addresses. + address, has every packet dropped by the BPF filter until the dial + deadline expires, and starts. That is the healthy case, and it needs no + hand-maintained list of geolocation addresses: + + ``` + egress-prober: confinement self-check: 3 geolocation host(s) -> 6 address(es): 134.119.216.174:443 [2606:4700:3037::6815:5e88]:443 ... 34.117.59.81:443 + egress-prober: confinement self-check passed: 6 address(es) tested, none directly reachable + ``` If your resolver is *not* on loopback (`/etc/resolv.conf` points at `192.0.2.53`, say), add that address to `IPAddressAllow` as well — a DNS @@ -228,7 +238,9 @@ own settings but a harmless command: sudo systemd-run --uid=nobody \ -p IPAddressDeny=any -p IPAddressAllow=localhost \ --wait --pty /bin/sh -c 'curl -sS --max-time 5 https://ipinfo.io/ip; echo rc=$?' -# expect a connect failure (rc != 0), not an address +# expect a connect failure, not an address: +# curl: (28) Connection timed out after 5002 milliseconds +# rc=28 ``` If that prints an address, the filter is not being applied on this host and the From c747f626290e9abae579641911d85dd8f7d6ecfe Mon Sep 17 00:00:00 2001 From: Ryan Mello Date: Sun, 26 Jul 2026 12:12:57 +0100 Subject: [PATCH 3/8] docs: make passive accounting the primary bandwidth signal 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 Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg --- ...25-enforced-provider-geo-probing-design.md | 121 +++++++++++++----- 1 file changed, 88 insertions(+), 33 deletions(-) diff --git a/docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md b/docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md index 8a912a96..7a2b46b1 100644 --- a/docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md +++ b/docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md @@ -333,37 +333,91 @@ project's probe population would also give an honest count. - **Live check**: the committed manual single-provider probe (`cmd/egress-prober/manual_probe_test.go`, env-guarded). -## Egress bandwidth measurement +## Bandwidth measurement + +The location probe answers *where* a provider exits. The market also needs *how +fast*, and the existing measurement answers the wrong question — but the +obvious replacement is far too expensive to run as a census, so the primary +signal is passive. + +### Why the existing measurement is not usable + +`connect/transport_announce.go` sends `SpeedTotalByteCount = 512 KiB` with +`MaxSpeedCount = 2` ("warm up then test") over the provider's already-open +connection, averaging into `network_client_speed` keyed by `connection_id`. + +Five problems, two of them severe: + +1. **Wrong path.** It measures provider-to-platform throughput, not + provider-to-internet. A provider with a fat pipe to us and a throttled egress + measures fast and serves slow. +2. **Too small to measure anything on a fast link.** 512 KiB is 4.2 Mbit; at + 1 Gbit it completes in ~4 ms, entirely inside TCP slow start. Above roughly + 50 Mbit the result reflects congestion-window ramp and RTT, not capacity. It + is precise-looking noise. +3. **Trivially gameable.** The platform's address is fixed and known, so + prioritising that single flow is a one-line change on the provider. +4. **Keyed by `connection_id`**, so it is discarded on every reconnect. +5. **Disabled below `transportVersion 2`** (`V0TestConfig()`), and its absence + adds 80 to the penalty score against a threshold of 40 — so it does not merely + fail to measure those providers, it removes them from the market. + +### Primary signal: passive accounting of real user traffic + +Providers are already paid for the bytes they carry, and +`transfer_escrow.payout_byte_count` already records bytes actually delivered per +contract. Deriving throughput from that costs **zero additional bytes** and is +unfakeable in the way that matters: a provider cannot inflate it without +genuinely serving real users fast, which is the outcome we want anyway. + +This is strictly better than any active probe for a provider that carries +traffic. It measures the real path, at real load, continuously, with no +observer effect and nothing for a provider to detect or special-case. + +Aggregate **per provider only**. Throughput per provider over a window is all +the market needs; per-user traffic patterns are neither required nor to be +derived. This is a new use of data already collected for billing, not new +collection. + +### Secondary signal: active egress probe, sampled + +Active probing remains, narrowed to what passive accounting cannot answer: + +- a provider that has carried no traffic yet, so has no passive history; +- periodic spot-checks to calibrate passive figures and catch a provider whose + measured throughput and delivered throughput disagree. + +It is a **sample, not a census** — a small percentage of eligible providers per +round, on rotation. + +**Sizing.** Each target streams until **5 s elapsed or 5 MB transferred**, +whichever first, with throughput computed after discarding the first 500 ms. +Ranking needs tiers, not precision: 5 MB separates tiers, and the larger cap +bought accuracy nobody consumes. Routine runs use the operator endpoint only; +the second (CDN) target is added only when corroborating a result already +suspected, where divergence beyond 3x yields +`suspect("bandwidth_divergence")`. -The location probe answers *where* a provider exits. The other half of what the -market needs is *how fast*, and the existing measurement answers the wrong -question. +**Opportunistically shared tunnel.** When a provider is due for both, the geo +probe and the speed test share one tunnel session — open once, geolocate +(~100 KB measured), then measure throughput. Contract setup dominates cost. The +sharing is conditional: bandwidth is budget-gated and must never delay a +location refresh. -`connect/transport_announce.go` speed-tests the **server-to-provider transport** -and averages samples into `network_client_speed`, keyed by `connection_id`. That -is throughput to our own platform, not the throughput a user gets when their -traffic exits to the internet. Egress bandwidth is measured the same way the -location is: through the provider's tunnel. +### Why this shape, in numbers -**Opportunistically shared tunnel.** When a provider is due for both, the geo -probe and the speed test run in one tunnel session — open once, geolocate -(~100 KB measured), then measure throughput. Contract setup dominates the cost, -so sharing the session roughly halves the per-provider price. The sharing is -conditional, not automatic: see *Queueing and load*, where bandwidth is -budget-gated and must never hold up a location refresh. - -**Two targets, recorded separately.** Each round measures against an -operator-hosted endpoint *and* a public CDN object, stored as distinct columns. -They are never averaged: the whole value of two targets is that a provider -prioritising one and not the other becomes visible. Divergence beyond 3x yields -`suspect("bandwidth_divergence")`, advisory like every other verdict. - -**Adaptive and bounded.** A fixed payload fails at both ends — 10 MB is 0.8 s on -100 Mbit but 0.08 s on 1 Gbit, where the result is TCP slow-start noise rather -than throughput. Each target streams until **5 s elapsed or 25 MB transferred, -whichever comes first**, and throughput is computed over the steady-state window -after discarding the first 500 ms. Cost is hard-capped at 50 MB per provider per -round: a fast link hits the byte cap, a slow one hits the time cap. +At 100k providers a full census at 50 MB each is ~5 TB per round — and because +probes are real paid contracts, that is money leaving the operator's account and +arriving in providers'. A census is not worth it for a ranking input. + +| Approach | Bytes per round at 100k providers | +| --- | --- | +| Existing platform test (1 MiB each) | ~100 GiB, measuring the wrong path | +| Full active census at 50 MB | ~5 TB | +| Passive + 5% sampled at 5 MB | ~25 GB | + +The passive-plus-sampling shape costs **less than the existing test** while +measuring the path users actually get. ### Bandwidth results are advisory @@ -382,9 +436,10 @@ falls. ### Queueing and load Every probe byte crosses the operator's own platform: the path is -prober -> platform -> provider -> internet, so a 50 MB test costs the operator -50 MB of `connect` throughput plus the transport encryption CPU, not just the -provider's bandwidth. Bandwidth probing is therefore capacity-limited by the +prober -> platform -> provider -> internet, so a 5 MB test costs the operator +5 MB of `connect` throughput plus the transport encryption CPU, not just the +provider's bandwidth. It is also a real paid contract, so the byte budget below +is a **spending** limit as much as a load limit. Bandwidth probing is therefore capacity-limited by the operator, and must never be scheduled as "probe everything that is due". **Reuse the fixed hourly bucket reservation already in this repo.** @@ -396,7 +451,7 @@ makes the daily cap fall out of the hourly one, and a jittered `RunAt` within the bucket so deferred work does not stampede the hour boundary. Bandwidth probing takes the same shape, budgeted in **bytes** rather than row -counts: each probe reserves its worst-case 50 MB against an hourly byte budget, +counts: each probe reserves its worst-case 5 MB against an hourly byte budget, and when the current hour is full it is queued into the next hour with lookahead rather than rejected. The operator's exposure per hour is then a configured constant, independent of how many providers are due. @@ -404,7 +459,7 @@ constant, independent of how many providers are due. Alongside the budget, a **global concurrency cap** (default 2) limits simultaneous bandwidth tests. This is deliberately separate from, and much tighter than, the geo probe's concurrency of 4: a geo probe moves ~100 KB while -a bandwidth test moves up to 50 MB, so one shared limit would either throttle +a bandwidth test moves up to 5 MB, so one shared limit would either throttle geolocation pointlessly or let bandwidth saturate the platform. **Queue ordering prioritises providers with more valid connections.** Budget is From 9f5c2d8a9e3db9b90313adc40ad9fc989dd88931 Mon Sep 17 00:00:00 2001 From: Ryan Mello Date: Sun, 26 Jul 2026 12:56:29 +0100 Subject: [PATCH 4/8] fix(beta): restore real client addresses behind the tls proxy 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 Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg --- Caddyfile | 27 +++++++++++++++++++++++++-- docker-compose.beta.yml | 19 ++++++++++++------- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/Caddyfile b/Caddyfile index c655f99b..1c0a9bf1 100644 --- a/Caddyfile +++ b/Caddyfile @@ -1,7 +1,30 @@ +# connect derives every client's location from the address it sees on the +# websocket. It only trusts a forwarded address when BOTH X-Forwarded-For and +# X-Forwarded-Source-Port are present (connect/transport.go:220-227) -- that +# pair is what the warp load balancer sends. Caddy sets X-Forwarded-For on its +# own, but not the port, so connect fell through to r.RemoteAddr and saw +# Caddy's container address (172.18.x.x) for every client. A private address +# has no mmdb entry, so GuessLocationType returned "Unknown location type", +# no network_client_location row was written, and no provider ever reached +# network_client_location_reliability -- which silently emptied the provider +# location list. +# +# Sending the source port restores the pair, and with it the real client +# address. api needs it too: the same address feeds rate limiting and the +# ARIN foreign-provider score, both of which would otherwise see one shared +# proxy address for the entire deployment. +(client_address) { + header_up X-Forwarded-Source-Port {http.request.remote.port} +} + api.beta-test.net { - reverse_proxy api:8080 + reverse_proxy api:8080 { + import client_address + } } connect.beta-test.net { - reverse_proxy connect:80 + reverse_proxy connect:80 { + import client_address + } } diff --git a/docker-compose.beta.yml b/docker-compose.beta.yml index 1035c1ad..afc81167 100644 --- a/docker-compose.beta.yml +++ b/docker-compose.beta.yml @@ -97,9 +97,10 @@ services: # on -- see that service for why it cannot be on `default`. networks: - default - - prober - ports: - - "8080:8080" + # No published port. Caddy terminates tls for api.beta-test.net and is the + # only ingress; publishing 8080 would let a client reach the api over plain + # http and bypass the forwarded-address headers connect and the api rely on + # to see the real client address. depends_on: postgres: condition: service_healthy @@ -130,10 +131,14 @@ services: # see the api service above for why `default` has to be repeated here networks: - default - - prober - ports: - - "5080:80" - - "15080:15080" + # No published ports. Caddy terminates tls for connect.beta-test.net and is + # the only ingress. This is load-bearing beyond transport security: connect + # only trusts a forwarded client address when it receives BOTH + # X-Forwarded-For and X-Forwarded-Source-Port (connect/transport.go:220), + # and otherwise falls back to the raw peer address. A client arriving on a + # published port presents the docker gateway address, which is private, has + # no mmdb entry, and yields "Unknown location type" -- so no location row is + # written and the provider never appears in the provider list. depends_on: postgres: condition: service_healthy From 2f9ca89b2b05c36eaa3c07b11e5f2a431919ab2a Mon Sep 17 00:00:00 2001 From: Ryan Mello Date: Sun, 26 Jul 2026 13:44:52 +0100 Subject: [PATCH 5/8] fix(beta): reattach api/connect to the prober network, strip a spoofable 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 Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg --- Caddyfile | 8 ++++++++ docker-compose.beta.yml | 2 ++ 2 files changed, 10 insertions(+) diff --git a/Caddyfile b/Caddyfile index 1c0a9bf1..0c1837e7 100644 --- a/Caddyfile +++ b/Caddyfile @@ -14,6 +14,14 @@ # ARIN foreign-provider score, both of which would otherwise see one shared # proxy address for the entire deployment. (client_address) { + # connect trusts X-UR-Forwarded-For ahead of everything else + # (connect/transport.go:220) and the api derives the session client address + # from the same value. A proxy passes client headers through verbatim, so + # with Caddy as the sole ingress any client could send this header and pin + # itself to an address of its choosing -- forging its provider location and + # evading the per-address rate limits. Strip it on the way in; only the + # warp load balancer is entitled to set it, and it is not in this path. + header_up -X-UR-Forwarded-For header_up X-Forwarded-Source-Port {http.request.remote.port} } diff --git a/docker-compose.beta.yml b/docker-compose.beta.yml index afc81167..4c22176b 100644 --- a/docker-compose.beta.yml +++ b/docker-compose.beta.yml @@ -97,6 +97,7 @@ services: # on -- see that service for why it cannot be on `default`. networks: - default + - prober # No published port. Caddy terminates tls for api.beta-test.net and is the # only ingress; publishing 8080 would let a client reach the api over plain # http and bypass the forwarded-address headers connect and the api rely on @@ -131,6 +132,7 @@ services: # see the api service above for why `default` has to be repeated here networks: - default + - prober # No published ports. Caddy terminates tls for connect.beta-test.net and is # the only ingress. This is load-bearing beyond transport security: connect # only trusts a forwarded client address when it receives BOTH From 9da357436ee7bd7e75b94455cf68051b95d7a2f4 Mon Sep 17 00:00:00 2001 From: Ryan Mello Date: Sun, 26 Jul 2026 13:59:05 +0100 Subject: [PATCH 6/8] fix(beta): don't derive NetTypeVirtual from Mobile; bound future observed_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 6b5de2be. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg --- controller/network_client_controller.go | 14 ++++- controller/network_client_controller_test.go | 16 ++++- .../provider_egress_location_controller.go | 16 +++++ ...rovider_egress_location_controller_test.go | 59 +++++++++++++++++++ 4 files changed, 99 insertions(+), 6 deletions(-) diff --git a/controller/network_client_controller.go b/controller/network_client_controller.go index 382aa913..4a723b6e 100644 --- a/controller/network_client_controller.go +++ b/controller/network_client_controller.go @@ -81,9 +81,17 @@ func SetConnectionLocation( if egress.Proxy { scores.NetTypePrivacy = 1 } - if egress.Mobile { - scores.NetTypeVirtual = 1 - } + // egress.Mobile deliberately does NOT feed NetTypeVirtual: unlike + // Hosting/Proxy, Mobile has no mmdb-path equivalent (IpInfo has no + // Mobile concept; NetTypeVirtual is set from the ipinfo schema's + // is_satellite field only, see GetLocationForIp, and never from + // DB-IP). Deriving NetTypeVirtual from Mobile here would penalize a + // probed mobile provider's ranking with no equivalent penalty for an + // otherwise-identical unprobed one -- the opposite of the parity + // this feature is meant to preserve (see arinForeignScore's doc for + // the same parity reasoning applied to net_type_foreign). Mobile + // stays on the model/wire contract as metadata; it just does not + // feed the ranking score. // keep the ARIN org-vs-country foreign check on the probed path too, // so a probed provider is ranked on equal terms with an equivalent // unprobed one (net_type_foreign feeds the ranking columns). Compute diff --git a/controller/network_client_controller_test.go b/controller/network_client_controller_test.go index ea6c593c..b60cf2c1 100644 --- a/controller/network_client_controller_test.go +++ b/controller/network_client_controller_test.go @@ -274,7 +274,14 @@ func TestSetConnectionLocationProbedNetTypeForeignMatchesMmdbParity(t *testing.T } // A fresh probed location's Hosting/Proxy flags must map onto the stored -// connection's net_type_hosting/net_type_privacy scores. +// connection's net_type_hosting/net_type_privacy scores. Mobile must NOT map +// onto net_type_virtual: Hosting/Proxy have direct mmdb-path equivalents +// (ipInfo.Hosting/ipInfo.Privacy, see GetLocationForIp), but Mobile has none +// (IpInfo has no Mobile concept at all, and NetTypeVirtual is only ever set +// from the ipinfo schema's is_satellite field, never from DB-IP or from +// anything Mobile-shaped) -- deriving NetTypeVirtual from Mobile would give +// a probed mobile provider a ranking penalty an identical unprobed mobile +// provider never takes, breaking the parity this feature promises. func TestSetConnectionLocationMapsProbedFlagsToScores(t *testing.T) { server.DefaultTestEnv().Run(t, func(t testing.TB) { ctx := context.Background() @@ -300,6 +307,7 @@ func TestSetConnectionLocationMapsProbedFlagsToScores(t *testing.T) { CountryCode: "jp", Hosting: true, Proxy: true, + Mobile: true, ObservedAt: server.NowUtc(), }) @@ -308,19 +316,21 @@ func TestSetConnectionLocationMapsProbedFlagsToScores(t *testing.T) { var netTypeHosting int var netTypePrivacy int + var netTypeVirtual int server.Db(ctx, func(conn server.PgConn) { result, qerr := conn.Query( ctx, - `SELECT net_type_hosting, net_type_privacy FROM network_client_location WHERE connection_id = $1`, + `SELECT net_type_hosting, net_type_privacy, net_type_virtual FROM network_client_location WHERE connection_id = $1`, connectionId, ) server.WithPgResult(result, qerr, func() { if result.Next() { - server.Raise(result.Scan(&netTypeHosting, &netTypePrivacy)) + server.Raise(result.Scan(&netTypeHosting, &netTypePrivacy, &netTypeVirtual)) } }) }) assert.Equal(t, netTypeHosting, 1) assert.Equal(t, netTypePrivacy, 1) + assert.Equal(t, netTypeVirtual, 0) }) } diff --git a/controller/provider_egress_location_controller.go b/controller/provider_egress_location_controller.go index 4afc02a7..4c688150 100644 --- a/controller/provider_egress_location_controller.go +++ b/controller/provider_egress_location_controller.go @@ -14,6 +14,19 @@ import ( // already older than this when it arrives. It bounds replay of an old probe. const MaxProviderEgressLocationSubmissionAge = 24 * time.Hour +// MaxProviderEgressLocationSubmissionSkew rejects a submission whose +// observed_at is further in the future than this. The prober and server +// clocks should be roughly in sync, so a few minutes of allowance covers +// ordinary clock drift without opening the door to a far-future timestamp. +// Without this bound, a future observed_at would defeat every other +// safeguard at once: it always wins the monotonic upsert in +// model.SetProviderEgressLocation (so no later, legitimate probe can ever +// overwrite it), it reads as "fresh" forever against +// ProviderEgressLocationMaxAge, and it outlives the taskworker sweep in +// RemoveExpiredProviderEgressLocations -- permanently pinning a provider to +// whatever location was submitted, with no API-side recovery. +const MaxProviderEgressLocationSubmissionSkew = 5 * time.Minute + // maxLocationNameLen bounds country/city/region as submitted: these flow into // model.CreateLocation, whose location_name column is varchar(128). Rejecting // an over-long value here with a clear error is preferable to letting @@ -66,6 +79,9 @@ func SubmitProviderEgressLocation( if args.ObservedAt.Before(server.NowUtc().Add(-MaxProviderEgressLocationSubmissionAge)) { return nil, fmt.Errorf("Submission is too old.") } + if server.NowUtc().Add(MaxProviderEgressLocationSubmissionSkew).Before(args.ObservedAt) { + return nil, fmt.Errorf("Submission is too far in the future.") + } if networkId := model.GetNetworkClientNetwork(ctx, args.ClientId); networkId == nil { return nil, fmt.Errorf("Unknown client.") } diff --git a/controller/provider_egress_location_controller_test.go b/controller/provider_egress_location_controller_test.go index 25a04c6d..6e20fa68 100644 --- a/controller/provider_egress_location_controller_test.go +++ b/controller/provider_egress_location_controller_test.go @@ -293,3 +293,62 @@ func TestSubmitProviderEgressLocationRejectsStaleObservedAt(t *testing.T) { } }) } + +// A far-future observed_at must be rejected, not just an old one: unchecked, +// it would defeat the monotonic upsert (it always "wins"), read as fresh +// forever, and outlive the taskworker sweep -- permanently pinning a +// provider's location with no API-side recovery. See +// MaxProviderEgressLocationSubmissionSkew. +func TestSubmitProviderEgressLocationRejectsFutureObservedAt(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + _, err := SubmitProviderEgressLocation(ctx, &SubmitProviderEgressLocationArgs{ + ClientId: clientId, + CountryCode: "jp", + Country: "Japan", + CountryConfident: true, + ObservedAt: server.NowUtc().Add(10 * 365 * 24 * time.Hour), + }) + if err == nil { + t.Fatal("a far-future observed_at must be rejected") + } + if model.GetProviderEgressLocation(ctx, clientId) != nil { + t.Fatal("rejected submission must not be stored") + } + }) +} + +// A submission within the allowed clock-skew window must still be accepted: +// the future-timestamp rejection must not be so strict that ordinary clock +// drift between the prober and server breaks legitimate submissions. +func TestSubmitProviderEgressLocationAcceptsWithinSkewObservedAt(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + res, err := SubmitProviderEgressLocation(ctx, &SubmitProviderEgressLocationArgs{ + ClientId: clientId, + CountryCode: "us", + Country: "United States", + CountryConfident: true, + ObservedAt: server.NowUtc().Add(1 * time.Minute), + }) + assert.Equal(t, err, nil) + if res.LocationId == (server.Id{}) { + t.Fatal("expected a resolved location id") + } + + stored := model.GetProviderEgressLocation(ctx, clientId) + if stored == nil { + t.Fatal("expected the submission to be stored") + } + }) +} From a36bba27a5a61ef636683d52cc1a76b7516102be Mon Sep 17 00:00:00 2001 From: Ryan Mello Date: Sun, 26 Jul 2026 15:08:13 +0100 Subject: [PATCH 7/8] docs(beta): document the tls-only ingress and how to diagnose an empty 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 Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg --- BETA.md | 113 +++++++++++++++++++++++++++++++------------------- beta-setup.sh | 7 ++-- 2 files changed, 74 insertions(+), 46 deletions(-) diff --git a/BETA.md b/BETA.md index 99062eb8..72cd5023 100644 --- a/BETA.md +++ b/BETA.md @@ -48,7 +48,7 @@ The server contains: - Linux host with Docker Engine 24+ and Docker Compose v2. - `curl`, `openssl`, and Go 1.26+ installed on the host (Go is only used by setup scripts that generate the MMDB). - Public IP address if you want to use the server from another machine. -- TCP ports 8080, 5080, and 15080 available on the host. +- TCP ports 80 and 443 available on the host (Caddy terminates tls and is the only ingress). ## Security notes @@ -97,12 +97,12 @@ When finished you will see output similar to: ``` Beta network running: - API: http://74.50.11.113:8080 - Connect: ws://74.50.11.113:5080/ + API: https://api.beta-test.net + Connect: wss://connect.beta-test.net/ All generated secrets live in ./beta-vault/beta-secrets.env and beta-vault/vault/*.yml These files are NOT tracked by git. -To use from another machine, open TCP ports 8080, 5080, and 15080. +Reachable from anywhere over https; no other ports need opening. ``` Replace `74.50.11.113` with your host's public IP. @@ -111,7 +111,7 @@ Replace `74.50.11.113` with your host's public IP. ```bash # Health check -curl -s http://74.50.11.113:8080/status | jq . +curl -s https://api.beta-test.net/status | jq . # WebSocket upgrade handshake curl -fsS -i -N \ @@ -119,10 +119,13 @@ curl -fsS -i -N \ -H 'Connection: Upgrade' \ -H 'Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==' \ -H 'Sec-WebSocket-Version: 13' \ - http://74.50.11.113:5080/ + --http1.1 \ + https://connect.beta-test.net/ ``` -You should receive `HTTP/1.1 101 Switching Protocols`. +You should receive `HTTP/1.1 101 Switching Protocols`. Use `--http1.1`: over +HTTP/2 a `Connection: Upgrade` header is illegal, so curl reports 400 and it +looks like an outage when the service is fine. ## Common operations @@ -179,24 +182,29 @@ docker compose -f docker-compose.beta.yml logs -f api The beta server binds ports on all interfaces (`0.0.0.0`). To reach it from another machine, the host firewall must allow inbound traffic on: -- `8080/tcp` — HTTP REST API and Solana wallet challenge -- `5080/tcp` — Client WebSocket connect tunnel -- `15080/tcp` — Internal connect exchange, used when multiple connect instances or providers run against the same server +- `80/tcp` — Caddy; redirects to https and answers the ACME http-01 challenge +- `443/tcp` — Caddy; terminates tls for `api.beta-test.net` and `connect.beta-test.net` + +The api and connect service ports are deliberately **not** published. Caddy is +the only ingress, and that is load-bearing beyond transport security: connect +trusts a forwarded client address only when it receives both `X-Forwarded-For` +and `X-Forwarded-Source-Port` (`connect/transport.go:220`). A client arriving +on a published port presents the docker gateway address instead — a private +address with no mmdb entry — so no location row is written and the provider +never appears in the provider list. Example with `ufw`: ```bash -sudo ufw allow 8080/tcp -sudo ufw allow 5080/tcp -sudo ufw allow 15080/tcp +sudo ufw allow 80/tcp +sudo ufw allow 443/tcp ``` Example with `iptables`: ```bash -sudo iptables -A INPUT -p tcp --dport 8080 -j ACCEPT -sudo iptables -A INPUT -p tcp --dport 5080 -j ACCEPT -sudo iptables -A INPUT -p tcp --dport 15080 -j ACCEPT +sudo iptables -A INPUT -p tcp --dport 80 -j ACCEPT +sudo iptables -A INPUT -p tcp --dport 443 -j ACCEPT ``` Remember that the beta server uses plain HTTP and WebSocket, not TLS. Do not send real production secrets through it. @@ -218,7 +226,7 @@ This beta server uses the server-issued wallet challenge from `urnetwork/server# ### Get a challenge ```bash -curl -s -X POST http://74.50.11.113:8080/auth/wallet-challenge \ +curl -s -X POST https://api.beta-test.net/auth/wallet-challenge \ -H "Content-Type: application/json" \ -d '{}' ``` @@ -266,13 +274,13 @@ Repository: `https://github.com/Ryanmello07/urnetwork-webmanager-beta`, branch ` git clone -b beta/main https://github.com/Ryanmello07/urnetwork-webmanager-beta.git cd urnetwork-webmanager-beta npm install -VITE_API_BASE=http://74.50.11.113:8080 npm run dev +VITE_API_BASE=https://api.beta-test.net npm run dev ``` If you deploy the dashboard to a static host such as Bolt or Netlify, use its `_redirects` file to proxy `/api/*` to the beta server. This avoids mixed-content and CORS issues when the dashboard is served over HTTPS. ``` -/api/* http://74.50.11.113:8080/:splat 200 +/api/* https://api.beta-test.net/:splat 200 /* /index.html 200 ``` @@ -283,8 +291,8 @@ Repository: `https://github.com/Ryanmello07/connect`, branch `beta/custom-server The fork defaults to the beta server: ```go -apiUrl := "http://74.50.11.113:8080" -connectUrl := "ws://74.50.11.113:5080/" +apiUrl := "https://api.beta-test.net" +connectUrl := "wss://connect.beta-test.net/" ``` ### Run a provider with a dashboard JWT @@ -513,18 +521,23 @@ To share the beta environment with a teammate, share the repository and have the ## Architecture ```text -+--------+ +-----------------------------+ -| Client | <-----> | API server :8080 | -+--------+ | - HTTP REST API | - | - Solana wallet challenge | - | - /connect auth endpoint | - +-----------------------------+ - | - +----------v------------------+ - | Connect server :5080 | - | - WebSocket upgrade on / | - | - Internal exchange :15080 | ++--------+ https +-----------------------------+ +| Client | <-----> | Caddy :80 :443 | ++--------+ | - the ONLY ingress | + | - lets encrypt certs | + | - sets the forwarded | + | address headers connect | + | needs to see real client | + | ips | +-----------------------------+ + | | + api.beta-test | | connect.beta-test + +-----v-------+ +------v----------------------+ + | API :8080 | | Connect :80 | + | (unpublished)| | - WebSocket upgrade on / | + +-------------+ | - Internal exchange :15080 | + | (both unpublished) | + +-----------------------------+ | +----------v----------+ | Postgres :5432 | @@ -566,16 +579,30 @@ A: It is reasonably secure for beta testing because all secrets are generated lo ### Port already in use -If `8080` or `5080` is busy, edit `docker-compose.beta.yml` and change the host-side port mappings: +Only Caddy publishes ports (`80` and `443`). If either is busy, stop whatever +holds it — they cannot be remapped, because Let's Encrypt's http-01 challenge +and public https both require the standard ports. + +The api and connect service ports are intentionally unpublished; do not add +`ports:` entries for them. See *Firewall* above for why that is load-bearing. + +### The provider list is empty -```yaml -api: - ports: - - "18080:8080" # change 18080 to any free port +Check whether connect can see real client addresses: -connect: - ports: - - "15080:80" # change 15080 to any free port +```bash +docker compose -f docker-compose.beta.yml logs --since 5m connect | grep -c 'could not find client location' +``` + +Anything above zero means connect is recording a private proxy address for +clients, so no location row is written. Confirm the Caddyfile still sends +`X-Forwarded-Source-Port`, and note that the Caddyfile is a **single-file bind +mount**: editing it on the host replaces the inode, the container keeps reading +the old one, and `caddy reload` reports success while reloading stale content. +Apply changes with: + +```bash +docker compose -f docker-compose.beta.yml up -d --force-recreate caddy ``` If you change the internal exchange port mapping, update `connect/resident.go`: @@ -588,10 +615,10 @@ and update `WARP_PORTS` in `docker-compose.beta.yml` accordingly. ### `failed to fetch` from the browser dashboard -If the dashboard is served over HTTPS and tries to call `http://74.50.11.113:8080`, browsers block mixed content. Use the dashboard's `/api/*` rewrite or run the dashboard locally over HTTP: +If the dashboard is served over HTTPS and tries to call `https://api.beta-test.net`, browsers block mixed content. Use the dashboard's `/api/*` rewrite or run the dashboard locally over HTTP: ```bash -VITE_API_BASE=http://74.50.11.113:8080 npm run dev +VITE_API_BASE=https://api.beta-test.net npm run dev ``` For a static HTTPS host, route `/api/*` to the beta server with a 200 rewrite, not a 302 redirect. @@ -652,7 +679,7 @@ Then start it with `--by-jwt=` from the patched `Ryanmello07/connect` for Before using the SOCKS5 proxy, connect at least one provider. Use the provider CLI or ask someone to run one against the beta server. Then verify: ```bash -curl -s http://74.50.11.113:8080/network/provider-locations | jq '.locations | length' +curl -s https://api.beta-test.net/network/provider-locations | jq '.locations | length' ``` If the count is 0, no provider is registered yet. diff --git a/beta-setup.sh b/beta-setup.sh index 1ed055c7..8acfac89 100755 --- a/beta-setup.sh +++ b/beta-setup.sh @@ -157,11 +157,12 @@ docker compose -f docker-compose.beta.yml up -d api connect taskworker echo echo "Beta network running:" -echo " API: http://$PUBLIC_IP:8080" -echo " Connect: ws://$PUBLIC_IP:5080/" +echo " API: https://api.beta-test.net" +echo " Connect: wss://connect.beta-test.net/" echo echo "All generated secrets live in $SECRETS_FILE and beta-vault/vault/*.yml" echo "These files are NOT tracked by git." echo "Postgres data is preserved across ./beta-setup.sh runs in the named volume." echo "To intentionally wipe the database, run ./beta-down.sh -v." -echo "To use from another machine, open TCP ports 8080, 5080, and 15080." +echo "Caddy is the only ingress; open TCP 80 and 443. The api and connect" +echo "service ports are deliberately unpublished -- see BETA.md, Firewall." From 7717b76044771f356362b09abea365dac566515d Mon Sep 17 00:00:00 2001 From: Ryan Mello Date: Sun, 26 Jul 2026 15:58:44 +0100 Subject: [PATCH 8/8] fix(model): count country-only providers once, and keep Network providers 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 Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg --- model/network_client_location_model.go | 271 ++++++++++---- model/network_client_location_model_test.go | 378 ++++++++++++++++++-- 2 files changed, 543 insertions(+), 106 deletions(-) diff --git a/model/network_client_location_model.go b/model/network_client_location_model.go index d753a666..db036e0a 100644 --- a/model/network_client_location_model.go +++ b/model/network_client_location_model.go @@ -1476,6 +1476,27 @@ func initialClientLocationsKey() string { return fmt.Sprintf("{cl}i") } +// returns the given ids with nils dropped and duplicates removed, preserving +// order. +// +// A client's city/region/country location ids are not guaranteed to differ: a +// country-only geo resolution stores the country id in all three columns (see +// SetConnectionLocation), and a region-only one stores it in two. Anything that +// fans a client out across the three columns has to treat them as a set, or the +// client is counted once per column instead of once per location it is in. +func distinctIds(ids ...*server.Id) []server.Id { + distinct := make([]server.Id, 0, len(ids)) + for _, id := range ids { + if id == nil { + continue + } + if !slices.Contains(distinct, *id) { + distinct = append(distinct, *id) + } + } + return distinct +} + func UpdateClientLocations(ctx context.Context, ttl time.Duration) (returnErr error) { topCitiesPerRegion := 20 topCitiesPerCountry := 10 @@ -1520,18 +1541,28 @@ func UpdateClientLocations(ctx context.Context, ttl time.Duration) (returnErr er WHERE network_client_location_reliability.connected = true AND network_client_location_reliability.valid = true AND - -- count only providers that can serve a stranger. - -- GetProvideRelationship returns ProvideModePublic for a - -- cross-network pair, so a Public provide key is what makes a - -- provider generally reachable; without one it advertises - -- supply nobody outside its own network can use. - -- This is deliberately narrower than what CreateContract will - -- ultimately accept: resolveNonCompanionProvideMode - -- (controller/connect_controller.go) also lets a Stream-only - -- destination settle a cross-network contract as a *companion* - -- stream, for backward compatibility with old clients. A - -- companion-only destination is a return path, not general - -- provider supply, so excluding it here is intended. + -- this is the number shown to everyone, so count only providers + -- a stranger can actually reach. GetProvideRelationship returns + -- ProvideModePublic for a cross-network pair, so a Public + -- provide key is what makes a provider generally reachable; + -- without one it advertises supply nobody outside its own + -- network can use. + -- + -- Note this is deliberately narrower than the candidate pool + -- UpdateClientScores builds. That pool also carries + -- ProvideModeNetwork providers, which are real usable supply + -- for sources inside their own network, and FindProviders2 + -- filters them per request against the caller's network. They + -- do not belong in a public count. + -- + -- Every other mode is excluded, not just Stream. In particular + -- resolveNonCompanionProvideMode + -- (controller/connect_controller.go) lets a Stream-only + -- destination be resolved as a *companion* stream, but that + -- dead-ends at CreateCompanionTransferEscrow, which requires a + -- pre-existing reverse-direction origin contract -- so a + -- Stream-only destination can never bootstrap a session and is + -- correctly absent from both the count and the pool. EXISTS ( SELECT 1 FROM provide_key WHERE @@ -1552,9 +1583,21 @@ func UpdateClientLocations(ctx context.Context, ttl time.Duration) (returnErr er &countryLocationId, )) - locationClientCounts[cityLocationId] += 1 - locationClientCounts[regionLocationId] += 1 - locationClientCounts[countryLocationId] += 1 + // count each client at most once per distinct location id. A + // client whose geo lookup resolved neither a city nor a region + // is stored with city = region = country (see + // SetConnectionLocation's country fallback), so incrementing + // all three unconditionally counted that one client three + // times in its own country. Distinct ids -- a real + // city-granular client -- still roll up into their region and + // country exactly as before. + for _, locationId := range distinctIds( + &cityLocationId, + ®ionLocationId, + &countryLocationId, + ) { + locationClientCounts[locationId] += 1 + } } }) @@ -2213,6 +2256,16 @@ type ClientScore struct { HasLatencyTest bool HasSpeedTest bool + // true when the provider holds a ProvideModeNetwork provide key but no + // ProvideModePublic one, i.e. it can only settle a contract with a source + // in its own network. FindProviders2 keeps such a provider only for callers + // in NetworkId. It is stored negated on purpose: the score cache is gob + // encoded with a 5h ttl, so entries written before this field existed decode + // with the zero value, and the zero value has to mean "publicly usable" -- + // the pre-existing behaviour -- or every provider would be treated as + // network-only until the cache turned over. + NetworkOnly bool + LookbackIndex int LookbackClientScores map[int]*ClientScore @@ -2303,6 +2356,7 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r clientScore = &ClientScore{ ClientId: lookbackClientScore.ClientId, NetworkId: lookbackClientScore.NetworkId, + NetworkOnly: lookbackClientScore.NetworkOnly, LookbackClientScores: map[int]*ClientScore{}, } m[lookbackClientScore.ClientId] = clientScore @@ -2404,6 +2458,7 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r var lookbackIndex int var reliabilityWeight float64 var independentReliabilityWeight float64 + var publiclyUsable bool server.Raise(result.Scan( &cityLocationXId, ®ionLocationXId, @@ -2419,11 +2474,13 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r &lookbackIndex, &reliabilityWeight, &independentReliabilityWeight, + &publiclyUsable, )) lookbackClientScore = &ClientScore{ ClientId: clientId, LookbackIndex: lookbackIndex, NetworkId: networkId, + NetworkOnly: !publiclyUsable, ReliabilityWeight: reliabilityWeight, IndependentReliabilityWeight: independentReliabilityWeight, MinRelativeLatencyMillis: minRelativeLatencyMillis, @@ -2470,7 +2527,17 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r -- fix(beta): see the LEFT JOIN comment below COALESCE(client_connection_reliability_score.lookback_index, 0), COALESCE(client_connection_reliability_score.reliability_weight, 1), - COALESCE(client_connection_reliability_score.independent_reliability_weight, 1) + COALESCE(client_connection_reliability_score.independent_reliability_weight, 1), + -- publicly usable: holds a Public provide key, so a caller from + -- any network can settle a contract with it. A provider that is + -- in the pool without this is Network-only, and FindProviders2 + -- hands it out to callers in its own network only. + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = network_client_location_reliability.client_id AND + provide_key.provide_mode = $1 + ) FROM network_client_location_reliability @@ -2486,46 +2553,61 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r WHERE network_client_location_reliability.connected = true AND network_client_location_reliability.valid = true AND - -- count only providers that can serve a stranger; see the - -- matching comment in UpdateClientLocations above for why a - -- Public provide key is the rule and why the Stream companion - -- fallback is deliberately not honoured here. + -- the candidate pool, unlike the public count in + -- UpdateClientLocations, carries every provider that can settle + -- a contract with *someone*: Public for any caller, Network for + -- callers in the provider's own network. GetProvideRelationship + -- only ever returns one of those two, so this pair is exactly + -- the set CreateContract can accept. FindProviders2 then decides + -- eligibility per request -- it has to be done there and not + -- here, because the score cache is keyed by (forceMinimum, + -- rankMode, locationId, callerLocationId) and is not + -- network-scoped. + -- + -- Restricting this to Public would remove Network-only + -- providers from their own network's discovery, which works + -- today via CreateContractNoEscrow. Stream is still excluded: + -- resolveNonCompanionProvideMode can resolve a Stream-only + -- destination as a companion, but that dead-ends at + -- CreateCompanionTransferEscrow, which needs a pre-existing + -- reverse origin contract, so it can never bootstrap a session. + -- -- GetProviderLocations gates on loadLocationStables, populated - -- from here, so filtering UpdateClientLocations alone would - -- leave the symptom in place. + -- from here; see exportClientScores for why the ClientFilter it + -- reads stays Public-only. EXISTS ( SELECT 1 FROM provide_key WHERE provide_key.client_id = network_client_location_reliability.client_id AND - provide_key.provide_mode = $1 + provide_key.provide_mode IN ($1, $2) ) `, ProvideModePublic, + ProvideModeNetwork, ) server.WithPgResult(result, err, func() { for result.Next() { lookbackClientScore, cityLocationId, regionLocationId, countryLocationId := loadClientScore(result) - clientScores, ok := locationClientScores[*cityLocationId] - if !ok { - clientScores = map[server.Id]*ClientScore{} - locationClientScores[*cityLocationId] = clientScores - } - addClientScore(lookbackClientScore, clientScores) - - clientScores, ok = locationClientScores[*regionLocationId] - if !ok { - clientScores = map[server.Id]*ClientScore{} - locationClientScores[*regionLocationId] = clientScores - } - addClientScore(lookbackClientScore, clientScores) - - clientScores, ok = locationClientScores[*countryLocationId] - if !ok { - clientScores = map[server.Id]*ClientScore{} - locationClientScores[*countryLocationId] = clientScores + // once per distinct location id: a country-only client stores + // its country id in all three columns (see + // SetConnectionLocation), and a client belongs in a location's + // pool once. The per-location map is keyed by client id so a + // repeat is already absorbed, but going through the set makes + // the intent explicit and keeps this loop in step with the + // counting loop in UpdateClientLocations. + for _, locationId := range distinctIds( + cityLocationId, + regionLocationId, + countryLocationId, + ) { + clientScores, ok := locationClientScores[locationId] + if !ok { + clientScores = map[server.Id]*ClientScore{} + locationClientScores[locationId] = clientScores + } + addClientScore(lookbackClientScore, clientScores) } - addClientScore(lookbackClientScore, clientScores) } }) @@ -2546,7 +2628,14 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r network_client_location_reliability.has_speed_test, COALESCE(client_connection_reliability_score.lookback_index, 0), -- fix(beta): see LEFT JOIN comment below COALESCE(client_connection_reliability_score.reliability_weight, 1), - COALESCE(client_connection_reliability_score.independent_reliability_weight, 1) + COALESCE(client_connection_reliability_score.independent_reliability_weight, 1), + -- publicly usable; see the per-location query above + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = network_client_location_reliability.client_id AND + provide_key.provide_mode = $1 + ) FROM network_client_location_reliability @@ -2569,49 +2658,39 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r WHERE network_client_location_reliability.connected = true AND network_client_location_reliability.valid = true AND - -- same rule as the per-location query above. This one fills - -- locationGroupClientScores -> the clientScoreLocationGroup* - -- redis keys -> loadClientScores -> FindProviders2 whenever a - -- spec carries a LocationGroupId, so leaving it unfiltered - -- means a user who picks a promoted group (e.g. "Strong - -- Privacy Laws") still gets providers that cannot accept - -- their contract and CreateContract rejects with NoPermission. + -- same rule as the per-location query above: Public or + -- Network. This one fills locationGroupClientScores -> the + -- clientScoreLocationGroup* redis keys -> loadClientScores + -- -> FindProviders2 whenever a spec carries a + -- LocationGroupId, so a user who picks a promoted group + -- (e.g. "Strong Privacy Laws") must be filtered by the same + -- request-time network check as a plain location. EXISTS ( SELECT 1 FROM provide_key WHERE provide_key.client_id = network_client_location_reliability.client_id AND - provide_key.provide_mode = $1 + provide_key.provide_mode IN ($1, $2) ) `, ProvideModePublic, + ProvideModeNetwork, ) server.WithPgResult(result, err, func() { for result.Next() { lookbackClientScore, cityLocationGroupId, regionLocationGroupId, countryLocationGroupId := loadClientScore(result) - if cityLocationGroupId != nil { - clientScores, ok := locationGroupClientScores[*cityLocationGroupId] - if !ok { - clientScores = map[server.Id]*ClientScore{} - locationGroupClientScores[*cityLocationGroupId] = clientScores - } - addClientScore(lookbackClientScore, clientScores) - } - - if regionLocationGroupId != nil { - clientScores, ok := locationGroupClientScores[*regionLocationGroupId] - if !ok { - clientScores = map[server.Id]*ClientScore{} - locationGroupClientScores[*regionLocationGroupId] = clientScores - } - addClientScore(lookbackClientScore, clientScores) - } - - if countryLocationGroupId != nil { - clientScores, ok := locationGroupClientScores[*countryLocationGroupId] + // once per distinct group id. The three location columns can + // be the same id (a country-only client), in which case all + // three group joins resolve to the same membership rows. + for _, locationGroupId := range distinctIds( + cityLocationGroupId, + regionLocationGroupId, + countryLocationGroupId, + ) { + clientScores, ok := locationGroupClientScores[locationGroupId] if !ok { clientScores = map[server.Id]*ClientScore{} - locationGroupClientScores[*countryLocationGroupId] = clientScores + locationGroupClientScores[locationGroupId] = clientScores } addClientScore(lookbackClientScore, clientScores) } @@ -2722,17 +2801,29 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r filter *ClientFilter, ) { clientScores := []*ClientScore{} - netReliabilityWeight := float64(0) + publicCount := 0 + publicNetReliabilityWeight := float64(0) for _, clientScore := range s { if clientScore.PassesMinimums[rankMode] || forceMinimum { - netReliabilityWeight += clientScore.ReliabilityWeight clientScores = append(clientScores, clientScore) + if !clientScore.NetworkOnly { + publicCount += 1 + publicNetReliabilityWeight += clientScore.ReliabilityWeight + } } } + // the samples above carry Network-only providers too -- FindProviders2 + // filters them per request against the caller's network -- but the + // ClientFilter does not. It is read only by loadLocationStables, which + // decides the `Stable` flag GetProviderLocations publishes to every + // user. That is a public surface, so like the provider count in + // UpdateClientLocations it counts only providers a stranger can reach: + // a location whose only supply is network-only is not stable, and with + // zero public providers it reports no providers at all. filter = &ClientFilter{ - Count: len(clientScores), - NetReliabilityWeight: netReliabilityWeight, + Count: publicCount, + NetReliabilityWeight: publicNetReliabilityWeight, } mathrand.Shuffle(len(clientScores), func(i int, j int) { @@ -3123,6 +3214,34 @@ func FindProviders2( glog.Infof("[nclm]findproviders2 load %.2fms (%d)\n", loadMillis, len(clientScores)) } + // drop providers this caller cannot contract with. + // + // UpdateClientScores puts both Public and Network-only providers in the + // pool. A Network-only provider can only settle a contract with a + // source in its own network (GetProvideRelationship -> + // ProvideModeNetwork -> CreateContractNoEscrow); it is real, usable + // supply for those users and has always been discoverable to them, so + // it stays. For anyone else CreateContract would reject with + // NoPermission, so it goes. + // + // This must happen here, after loadClientScores, and must never be + // baked into the cached set: the client score redis entries are keyed + // by (forceMinimum, rankMode, locationId, callerLocationId) with no + // network component, so a single cached set is shared by callers from + // every network. + // + // A session with no jwt yields the zero network id, which matches no + // provider -- fail closed rather than leak. + var callerNetworkId server.Id + if session.ByJwt != nil { + callerNetworkId = session.ByJwt.NetworkId + } + for clientId, clientScore := range clientScores { + if clientScore.NetworkOnly && clientScore.NetworkId != callerNetworkId { + delete(clientScores, clientId) + } + } + for clientId, _ := range excludeFinalDestinations() { delete(clientScores, clientId) } diff --git a/model/network_client_location_model_test.go b/model/network_client_location_model_test.go index 450554d3..337b575f 100644 --- a/model/network_client_location_model_test.go +++ b/model/network_client_location_model_test.go @@ -1510,18 +1510,24 @@ func TestUpdateClientLocationsCountsOnlyPublicProviders(t *testing.T) { }) } -// `UpdateClientScores` carries the same "can this provider serve a stranger" -// filter as `UpdateClientLocations`, and it matters on a different path: -// the per-location source query fills the per-location client scores, which -// become the `clientScoreLocation*` redis keys, which `loadLocationStables` -// reads and `GetProviderLocations` gates on. A location whose only providers -// are network-only must therefore not be stable at all. +// `UpdateClientScores` writes two things, and the provide-mode rule differs +// between them. +// +// The `ClientFilter` -- read by `loadLocationStables`, which +// `GetProviderLocations` gates the public `Stable` flag on -- counts only +// providers a stranger can reach, the same rule as the provider count in +// `UpdateClientLocations`. A location whose only supply is network-only is not +// stable and reports no providers at all. +// +// The sampled candidate pool is wider: it also carries `ProvideModeNetwork` +// providers, which are real usable supply for sources in their own network. +// `FindProviders2` filters those per request (see +// TestFindProviders2NetworkOnlyProviderVisibleOnlyToItsOwnNetwork); the pool +// itself is shared by every caller and cannot make that decision. // // Two connected+valid providers with identical latency/speed data in two // different countries, differing ONLY in whether they hold a Public provide -// key. Removing the `EXISTS (... provide_mode = ...)` clause from the -// per-location source query makes the network-only provider's country stable -// and puts its client score in the cache, and this test goes red. +// key. func TestUpdateClientScoresCountsOnlyPublicProviders(t *testing.T) { server.DefaultTestEnv().Run(t, func(t testing.TB) { ctx := context.Background() @@ -1572,7 +1578,8 @@ func TestUpdateClientScoresCountsOnlyPublicProviders(t *testing.T) { _, ok = locationStables[networkOnlyCity.CountryLocationId] connect.AssertEqual(t, ok, false) - // and the same at the client-score level FindProviders2 consumes + // the pool FindProviders2 consumes carries both, each tagged with + // whether a caller outside its network may use it clientScores, err := loadClientScores( true, RankModeQuality, @@ -1586,26 +1593,33 @@ func TestUpdateClientScoresCountsOnlyPublicProviders(t *testing.T) { 100, ) connect.AssertEqual(t, err, nil) - connect.AssertEqual(t, len(clientScores), 1) - _, ok = clientScores[publicClientId] + connect.AssertEqual(t, len(clientScores), 2) + publicClientScore, ok := clientScores[publicClientId] connect.AssertEqual(t, ok, true) - _, ok = clientScores[networkOnlyClientId] - connect.AssertEqual(t, ok, false) + connect.AssertEqual(t, publicClientScore.NetworkOnly, false) + networkOnlyClientScore, ok := clientScores[networkOnlyClientId] + connect.AssertEqual(t, ok, true) + connect.AssertEqual(t, networkOnlyClientScore.NetworkOnly, true) }) } -// the location *group* source query in `UpdateClientScores` needs the same -// filter as the per-location one. It fills `locationGroupClientScores` -> -// the `clientScoreLocationGroup*` redis keys -> `loadClientScores` -> -// `FindProviders2` whenever the spec carries a `LocationGroupId`. Unfiltered, -// a user who selects a promoted group (e.g. "Strong Privacy Laws") is handed -// providers that cannot accept their contract, and `CreateContract` rejects -// them with `NoPermission`. +// the location *group* source query in `UpdateClientScores` follows the same +// Public-or-Network rule as the per-location one. It fills +// `locationGroupClientScores` -> the `clientScoreLocationGroup*` redis keys -> +// `loadClientScores` -> `FindProviders2` whenever the spec carries a +// `LocationGroupId`, so a user who selects a promoted group (e.g. "Strong +// Privacy Laws") has to be filtered by the same request-time network check as +// a user who selects a plain location -- and the group cache has to carry the +// flag that check reads. +// +// Dropping either `provide_mode` from the group query's `EXISTS` breaks this: +// without Network the group's own-network supply disappears, and without +// Public nothing is reachable at all. Dropping the `EXISTS` entirely would let +// in modes `CreateContract` can never accept. // // Same two providers, differing only in the Public provide key, each in its -// own location group. Removing the `EXISTS` clause from the group query puts -// the network-only provider in its group's cache and this test goes red. -func TestUpdateClientScoresGroupCountsOnlyPublicProviders(t *testing.T) { +// own location group. +func TestUpdateClientScoresGroupCarriesNetworkProvidersTagged(t *testing.T) { server.DefaultTestEnv().Run(t, func(t testing.TB) { ctx := context.Background() @@ -1676,13 +1690,17 @@ func TestUpdateClientScoresGroupCountsOnlyPublicProviders(t *testing.T) { 100, ) connect.AssertEqual(t, err, nil) - connect.AssertEqual(t, len(clientScores), 1) - _, ok := clientScores[publicClientId] + connect.AssertEqual(t, len(clientScores), 2) + publicClientScore, ok := clientScores[publicClientId] connect.AssertEqual(t, ok, true) - _, ok = clientScores[networkOnlyClientId] - connect.AssertEqual(t, ok, false) + connect.AssertEqual(t, publicClientScore.NetworkOnly, false) + networkOnlyClientScore, ok := clientScores[networkOnlyClientId] + connect.AssertEqual(t, ok, true) + connect.AssertEqual(t, networkOnlyClientScore.NetworkOnly, true) - // the network-only group on its own exports nothing + // the network-only group on its own exports exactly its one provider, + // tagged network-only so FindProviders2 hands it to that provider's + // own network and nobody else networkOnlyGroupClientScores, err := loadClientScores( true, RankModeQuality, @@ -1693,7 +1711,10 @@ func TestUpdateClientScoresGroupCountsOnlyPublicProviders(t *testing.T) { 100, ) connect.AssertEqual(t, err, nil) - connect.AssertEqual(t, len(networkOnlyGroupClientScores), 0) + connect.AssertEqual(t, len(networkOnlyGroupClientScores), 1) + networkOnlyClientScore, ok = networkOnlyGroupClientScores[networkOnlyClientId] + connect.AssertEqual(t, ok, true) + connect.AssertEqual(t, networkOnlyClientScore.NetworkOnly, true) }) } @@ -1749,3 +1770,300 @@ func connectPublicAndNetworkOnlyProviders( }) return } + +// A client whose geo lookup resolved no city and no region is stored with +// city_location_id = region_location_id = country_location_id -- the coarsest +// granularity available is written into all three columns. Both fan-out loops +// then walked city, region and country unconditionally, so one such client +// added 3 to its own country: `/network/provider-locations` advertised three +// times the supply that exists, and the inflation is worst exactly where geo +// resolution is coarsest (datacenter, mobile and VPN egress). +// +// The second half of this test is the guard on the fix: a genuinely +// city-granular client must still roll up into its region and its country. A +// dedupe keyed on the client alone rather than on (client, location) would +// silently stop city clients counting toward their country, which is a far +// worse regression than the one being fixed. +func TestUpdateClientLocationsCountsEachClientOncePerLocation(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + countryOnlyCity, city := createCountryOnlyAndCityProviders(ctx, t) + + err := UpdateClientLocations(ctx, time.Hour) + connect.AssertEqual(t, err, nil) + + clientLocations, err := loadClientLocations(ctx, map[server.Id]bool{ + countryOnlyCity.CountryLocationId: true, + city.CityLocationId: true, + city.RegionLocationId: true, + city.CountryLocationId: true, + }) + connect.AssertEqual(t, err, nil) + + countOf := func(locationId server.Id) int { + clientLocation, ok := clientLocations[locationId] + if !ok { + t.Fatalf("expected a cached client location for %s", locationId) + } + return clientLocation.ClientCount + } + + // one client, counted once -- not once per city/region/country column + connect.AssertEqual(t, countOf(countryOnlyCity.CountryLocationId), 1) + + // and the roll-up for a real city client is untouched + connect.AssertEqual(t, countOf(city.CityLocationId), 1) + connect.AssertEqual(t, countOf(city.RegionLocationId), 1) + connect.AssertEqual(t, countOf(city.CountryLocationId), 1) + }) +} + +// The same shape for `UpdateClientScores`: a country-only provider must appear +// once in its country's candidate pool, and a city-granular provider must still +// appear in its city's, its region's and its country's pools. +func TestUpdateClientScoresCountsEachClientOncePerLocation(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + countryOnlyCity, city := createCountryOnlyAndCityProviders(ctx, t) + + err := UpdateClientScores(ctx, time.Hour, 1) + connect.AssertEqual(t, err, nil) + + poolSizeAt := func(locationId server.Id) int { + clientScores, err := loadClientScores( + true, + RankModeQuality, + ctx, + map[server.Id]bool{locationId: true}, + map[server.Id]bool{}, + server.Id{}, + 100, + ) + connect.AssertEqual(t, err, nil) + return len(clientScores) + } + + connect.AssertEqual(t, poolSizeAt(countryOnlyCity.CountryLocationId), 1) + + // the city provider still rolls up to every granularity + connect.AssertEqual(t, poolSizeAt(city.CityLocationId), 1) + connect.AssertEqual(t, poolSizeAt(city.RegionLocationId), 1) + connect.AssertEqual(t, poolSizeAt(city.CountryLocationId), 1) + }) +} + +// connects two Public providers in two different countries and rolls the +// reliability table forward: +// +// - one whose `network_client_location` row has all three location columns +// collapsed to its country id, which is what a country-granularity geo +// lookup produces (`SetConnectionLocation` writes the coarsest available id +// into the NOT NULL city/region columns), and +// - one at genuine city granularity, with three distinct ids. +// +// The collapse is applied with a direct UPDATE rather than by handing +// `SetConnectionLocation` a country-only `location` row, so the fixture depends +// only on the shape of the stored row and not on which branch's fallback +// produced it. +func createCountryOnlyAndCityProviders(ctx context.Context, t testing.TB) ( + countryOnlyCity *Location, + city *Location, +) { + countryOnlyCity = &Location{ + LocationType: LocationTypeCity, + City: "Toronto", + Region: "Ontario", + Country: "Canada", + CountryCode: "ca", + } + CreateLocation(ctx, countryOnlyCity) + + city = &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + handlerId := CreateNetworkClientHandler(ctx) + connectPublicProvider := func(location *Location, ip string) server.Id { + networkId := server.NewId() + clientId := server.NewId() + Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + connectionId, _, _, _, err := ConnectNetworkClient(ctx, clientId, ip, handlerId) + connect.AssertEqual(t, err, nil) + err = SetConnectionLocation(ctx, connectionId, location.LocationId, &ConnectionLocationScores{}) + connect.AssertEqual(t, err, nil) + SetProvide(ctx, clientId, map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + }) + + // UpdateClientScores joins client_connection_reliability_score, so + // every provider needs a score row or the join, not the logic under + // test, is what excludes it. The address hash is shared across the + // fixture on purpose: validity is derived from + // network_client_connection, not from these stats. + AddClientReliabilityStats( + ctx, + networkId, + clientId, + [32]byte{}, + server.NowUtc(), + &ClientReliabilityStats{ + ConnectionEstablishedCount: 1, + ProvideEnabledCount: 1, + ReceiveMessageCount: 1, + ReceiveByteCount: 1024, + SendMessageCount: 1, + SendByteCount: 1024, + }, + ) + return clientId + } + + countryOnlyClientId := connectPublicProvider(countryOnlyCity, "0.0.0.1:0") + connectPublicProvider(city, "0.0.0.2:0") + + // collapse the first provider to country granularity + server.Tx(ctx, func(tx server.PgTx) { + server.RaisePgResult(tx.Exec( + ctx, + ` + UPDATE network_client_location + SET + city_location_id = $2, + region_location_id = $2 + WHERE client_id = $1 + `, + countryOnlyClientId, + countryOnlyCity.CountryLocationId, + )) + }) + + UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + UpdateClientReliabilityScores(ctx, server.NowUtc(), true) + return +} + +// `UpdateClientLocations` counts only providers holding a Public provide key, +// because that count is shown to everyone and a stranger genuinely cannot use a +// `Network`-only provider. The candidate pool is a different question: a +// `Network`-only provider serves sources inside its own network today (via +// `CreateContractNoEscrow`) and is discoverable to them. Restricting the pool +// to Public would make those providers invisible to the very users they exist +// for -- a live regression for anyone running providers for their own +// organisation. +// +// So the pool carries both, and eligibility is decided per request against the +// caller's network. All four cases: +// +// same-network caller other-network caller +// Public provider returned returned +// Network-only provider returned NOT returned +// +// This has to be a request-time filter, not a cache-time one: the client-score +// redis entries are keyed by (forceMinimum, rankMode, locationId, +// callerLocationId) and are not network-scoped, so one cached set is shared by +// callers from every network. +func TestFindProviders2NetworkOnlyProviderVisibleOnlyToItsOwnNetwork(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + handlerId := CreateNetworkClientHandler(ctx) + + connectProvider := func(networkId server.Id, ip string, modes map[ProvideMode][]byte) server.Id { + clientId := server.NewId() + Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + connectionId, _, _, _, err := ConnectNetworkClient(ctx, clientId, ip, handlerId) + connect.AssertEqual(t, err, nil) + err = SetConnectionLocation(ctx, connectionId, city.LocationId, &ConnectionLocationScores{}) + connect.AssertEqual(t, err, nil) + SetProvide(ctx, clientId, modes) + + // upstream joins client_connection_reliability_score with an INNER + // JOIN, so a provider with no reliability row never reaches the + // pool at all. Give every provider one so this test asserts the + // provide-mode rule and nothing else. + AddClientReliabilityStats( + ctx, + networkId, + clientId, + [32]byte{}, + server.NowUtc(), + &ClientReliabilityStats{ + ConnectionEstablishedCount: 1, + ProvideEnabledCount: 1, + ReceiveMessageCount: 1, + ReceiveByteCount: 1024, + SendMessageCount: 1, + SendByteCount: 1024, + }, + ) + return clientId + } + + publicNetworkId := server.NewId() + publicClientId := connectProvider(publicNetworkId, "0.0.0.1:0", map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + ProvideModeNetwork: []byte("network-secret"), + }) + + networkOnlyNetworkId := server.NewId() + networkOnlyClientId := connectProvider(networkOnlyNetworkId, "0.0.0.2:0", map[ProvideMode][]byte{ + ProvideModeNetwork: []byte("network-secret"), + }) + + UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + UpdateClientReliabilityScores(ctx, server.NowUtc(), true) + + err := UpdateClientScores(ctx, time.Hour, 1) + connect.AssertEqual(t, err, nil) + + findFrom := func(networkId server.Id, name string) map[server.Id]bool { + clientSession := session.Testing_CreateClientSession( + ctx, + jwt.NewByJwt(networkId, server.NewId(), name, false, false), + ) + res, err := FindProviders2( + &FindProviders2Args{ + Specs: []*ProviderSpec{ + {LocationId: &city.LocationId}, + }, + Count: 100, + ForceMinimum: true, + }, + clientSession, + ) + connect.AssertEqual(t, err, nil) + found := map[server.Id]bool{} + for _, provider := range res.Providers { + found[provider.ClientId] = true + } + return found + } + + // the network-only provider's own network sees both + sameNetwork := findFrom(networkOnlyNetworkId, "same-network") + connect.AssertEqual(t, sameNetwork[publicClientId], true) + connect.AssertEqual(t, sameNetwork[networkOnlyClientId], true) + + // a stranger sees only the Public one. handing them the network-only + // provider would produce a `CreateContract` NoPermission rejection. + otherNetwork := findFrom(server.NewId(), "other-network") + connect.AssertEqual(t, otherNetwork[publicClientId], true) + connect.AssertEqual(t, otherNetwork[networkOnlyClientId], false) + }) +}