diff --git a/api/api.go b/api/api.go index fef528b8..bb9d6c21 100644 --- a/api/api.go +++ b/api/api.go @@ -51,6 +51,9 @@ func Routes() []*router.Route { router.NewRoute("POST", "/auth/upgrade-guest-existing", handlers.UpgradeGuestExisting), router.NewRoute("POST", "/network/auth-client", handlers.AuthNetworkClient), router.NewRoute("POST", "/network/remove-client", handlers.RemoveNetworkClient), + router.NewRoute("POST", "/network/provider-egress-location", handlers.ProviderEgressLocationSubmit), + router.NewRoute("GET", "/network/provider-egress-due", handlers.ProviderEgressLocationDue), + router.NewRoute("POST", "/network/provider-egress-attempt", handlers.ProviderEgressLocationAttempt), router.NewRoute("GET", "/network/clients", handlers.NetworkClients), router.NewRoute("GET", "/network/peers", handlers.NetworkPeers), router.NewRoute("GET", "/network/provider-locations", handlers.NetworkGetProviderLocations), diff --git a/api/handlers/provider_egress_location_handlers.go b/api/handlers/provider_egress_location_handlers.go new file mode 100644 index 00000000..264da746 --- /dev/null +++ b/api/handlers/provider_egress_location_handlers.go @@ -0,0 +1,229 @@ +package handlers + +import ( + "crypto/hmac" + "encoding/json" + "io" + "net/http" + "strconv" + "sync" + + "github.com/urnetwork/glog" + + "github.com/urnetwork/server" + "github.com/urnetwork/server/controller" + "github.com/urnetwork/server/model" +) + +// operatorSecretHeader carries the operator ingest secret. This endpoint is +// operator-to-server, not a client route: it is authenticated by a shared +// secret from the vault, not by a network jwt. +const operatorSecretHeader = "X-UR-Operator-Secret" + +// maxProviderEgressLocationBody bounds the request body. +const maxProviderEgressLocationBody = 16 * 1024 + +// operatorIngestSecret memoizes readOperatorIngestSecret for the life of the +// process. It is a package-level var (not a plain sync.OnceValue call site) +// so tests can swap it for a stub and restore it with defer; production code +// never reassigns it. +var operatorIngestSecret func() string = sync.OnceValue(readOperatorIngestSecret) + +// readOperatorIngestSecret reads the operator ingest secret from the vault +// resource "provider_egress.yml", key "ingest_secret". It returns "" +// when the vault resource is absent, the key is absent, or the key is empty, +// which makes the endpoint fail closed (every request is rejected) rather +// than open. `SimpleResource`/`String` are the non-panicking lookups (unlike +// `RequireSimpleResource`/`RequireString`), so a missing vault resource +// disables the endpoint instead of panicking the api process at startup or +// per-request. +func readOperatorIngestSecret() string { + res, err := server.Vault.SimpleResource("provider_egress.yml") + if err != nil { + glog.Infof("[pegl]no provider_egress.yml in the vault; ingest endpoint disabled\n") + return "" + } + values := res.String("ingest_secret") + if len(values) != 1 || values[0] == "" { + glog.Infof("[pegl]no ingest_secret in provider_egress.yml; ingest endpoint disabled\n") + return "" + } + return values[0] +} + +// ProviderEgressLocationSubmit ingests a probed provider egress location from +// the operator's prober. The prober routes geolocation lookups through a +// provider's own egress -- rather than relying on a lookup against the +// provider's control-connection ip -- and submits the result here so the +// server can prefer it over the built-in mmdb lookup. The route is +// operator-to-server, authenticated by the shared secret above rather than a +// network jwt. +func ProviderEgressLocationSubmit(w http.ResponseWriter, r *http.Request) { + secret := operatorIngestSecret() + provided := r.Header.Get(operatorSecretHeader) + if secret == "" || provided == "" || !hmac.Equal([]byte(secret), []byte(provided)) { + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + + body, err := io.ReadAll(io.LimitReader(r.Body, maxProviderEgressLocationBody+1)) + if err != nil { + http.Error(w, "Bad request", http.StatusBadRequest) + return + } + if len(body) > maxProviderEgressLocationBody { + http.Error(w, "Request too large", http.StatusRequestEntityTooLarge) + return + } + + var args controller.SubmitProviderEgressLocationArgs + if err := json.Unmarshal(body, &args); err != nil { + http.Error(w, "Bad request", http.StatusBadRequest) + return + } + + result, err := controller.SubmitProviderEgressLocation(r.Context(), &args) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(result); err != nil { + glog.Infof("[pegl]could not write response. err = %s\n", err) + } +} + +// ProviderEgressLocationAttempt records that the operator's prober tried to +// probe a provider, whether or not the try produced a location. +// +// The prober reports a *failure* here; a success is reported by +// ProviderEgressLocationSubmit above, whose provider_egress_location row +// already defers the provider for the full staleness window. Reporting a +// success here as well is harmless -- the attempt backoff is far shorter than +// that window -- but redundant. +// +// This exists because ProviderEgressLocationDue would otherwise be starved by +// providers that can never be probed successfully: they never get an egress +// row, so they sort to the head of the queue on every poll forever. See +// model.GetProviderEgressLocationDue. +// +// Same auth as the two endpoints around it: operator-to-server, the shared +// secret header rather than a network jwt, fail-closed when the vault resource +// is missing. +func ProviderEgressLocationAttempt(w http.ResponseWriter, r *http.Request) { + secret := operatorIngestSecret() + provided := r.Header.Get(operatorSecretHeader) + if secret == "" || provided == "" || !hmac.Equal([]byte(secret), []byte(provided)) { + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + + body, err := io.ReadAll(io.LimitReader(r.Body, maxProviderEgressLocationBody+1)) + if err != nil { + http.Error(w, "Bad request", http.StatusBadRequest) + return + } + if len(body) > maxProviderEgressLocationBody { + http.Error(w, "Request too large", http.StatusRequestEntityTooLarge) + return + } + + var args controller.RecordProviderEgressProbeAttemptArgs + if err := json.Unmarshal(body, &args); err != nil { + http.Error(w, "Bad request", http.StatusBadRequest) + return + } + + result, err := controller.RecordProviderEgressProbeAttempt(r.Context(), &args) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(result); err != nil { + glog.Infof("[pegl]could not write response. err = %s\n", err) + } +} + +const ( + // defaultProviderEgressDueLimit is the batch size when the caller does not + // ask for one. + defaultProviderEgressDueLimit = 100 + // maxProviderEgressDueLimit bounds the batch size regardless of what the + // caller asks for, so one request cannot ask the database for the entire + // provider population. + maxProviderEgressDueLimit = 500 +) + +// providerEgressDueAge is how stale a stored probe must be before its provider +// is offered up for re-probing. It is deliberately shorter than +// model.ProviderEgressLocationMaxAge -- the age past which a stored location +// stops being trusted at all. If the two were equal, every location would lapse +// to the mmdb fallback at the exact moment it became due and stay lapsed until +// the prober worked its way around to it; at half the max age the prober has a +// full max-age/2 window to refresh a location before it expires. +const providerEgressDueAge = model.ProviderEgressLocationMaxAge / 2 + +// ProviderEgressLocationDueResult is the response body of +// ProviderEgressLocationDue. +type ProviderEgressLocationDueResult struct { + ClientIds []server.Id `json:"client_ids"` +} + +// ProviderEgressLocationDue tells the operator's prober which providers to +// probe next: those whose egress location has gone stale, and those that have +// never been probed at all, oldest first. +// +// This moves the probe schedule from the prober's memory into the database. +// The prober used to decide what to probe from an in-memory ttl cache, so a +// restart re-probed the whole population and nothing durable recorded what was +// actually due; observed_at already carries that information server-side, and +// this exposes it. +// +// A provider is skipped if it has a fresh success *or* a recent attempt. The +// second cutoff, ProviderEgressProbeAttemptBackoff, is much shorter than the +// first: a provider that failed to probe should be retried within hours, but +// must not be handed back on every poll, which is what would starve the rest of +// the queue (see ProviderEgressLocationAttempt above). +// +// Same auth as ProviderEgressLocationSubmit above: operator-to-server, the +// shared secret header rather than a network jwt, fail-closed when the vault +// resource is missing. +func ProviderEgressLocationDue(w http.ResponseWriter, r *http.Request) { + secret := operatorIngestSecret() + provided := r.Header.Get(operatorSecretHeader) + if secret == "" || provided == "" || !hmac.Equal([]byte(secret), []byte(provided)) { + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + + limit := defaultProviderEgressDueLimit + if raw := r.URL.Query().Get("limit"); raw != "" { + parsed, err := strconv.Atoi(raw) + if err != nil || parsed < 1 { + // not clamped up to 1: `limit=0` would come back as an empty list, + // which the prober cannot distinguish from "nothing is due" + http.Error(w, "Bad request", http.StatusBadRequest) + return + } + limit = min(parsed, maxProviderEgressDueLimit) + } + + // both cutoffs are computed here and passed as arguments; observed_at and + // attempt_at are naive timestamps holding utc, so comparing them to sql + // now() in the query would cast through the session timezone + now := server.NowUtc() + minObservedAt := now.Add(-providerEgressDueAge) + minAttemptAt := now.Add(-model.ProviderEgressProbeAttemptBackoff) + + result := &ProviderEgressLocationDueResult{ + ClientIds: model.GetProviderEgressLocationDue(r.Context(), minObservedAt, minAttemptAt, limit), + } + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(result); err != nil { + glog.Infof("[pegl]could not write response. err = %s\n", err) + } +} diff --git a/api/handlers/provider_egress_location_handlers_test.go b/api/handlers/provider_egress_location_handlers_test.go new file mode 100644 index 00000000..0f094ca0 --- /dev/null +++ b/api/handlers/provider_egress_location_handlers_test.go @@ -0,0 +1,552 @@ +package handlers + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + "time" + + "github.com/urnetwork/server" + "github.com/urnetwork/server/controller" + "github.com/urnetwork/server/model" +) + +func TestProviderEgressLocationSubmitRejectsMissingSecret(t *testing.T) { + body, _ := json.Marshal(map[string]any{ + "client_id": "019f8835-158d-6fd8-e9dd-fd0e4c6d6792", + }) + req := httptest.NewRequest(http.MethodPost, "/network/provider-egress-location", bytes.NewReader(body)) + w := httptest.NewRecorder() + + ProviderEgressLocationSubmit(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401 when the operator secret header is absent", w.Code) + } +} + +func TestProviderEgressLocationSubmitRejectsWrongSecret(t *testing.T) { + body, _ := json.Marshal(map[string]any{ + "client_id": "019f8835-158d-6fd8-e9dd-fd0e4c6d6792", + }) + req := httptest.NewRequest(http.MethodPost, "/network/provider-egress-location", bytes.NewReader(body)) + req.Header.Set(operatorSecretHeader, "definitely-not-the-secret") + w := httptest.NewRecorder() + + ProviderEgressLocationSubmit(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401 on a wrong operator secret", w.Code) + } +} + +// withStubOperatorIngestSecret swaps the package-level operatorIngestSecret +// memo for a stub that always returns secret, and returns a func to restore +// the original (real, still-memoized) reader. This lets a test exercise the +// "configured vault" path without the sync.OnceValue in the real reader ever +// touching the vault, and without the reject tests above (which rely on an +// unconfigured vault) observing any change. +func withStubOperatorIngestSecret(secret string) (restore func()) { + prev := operatorIngestSecret + operatorIngestSecret = func() string { return secret } + return func() { operatorIngestSecret = prev } +} + +// TestProviderEgressLocationSubmitAcceptsCorrectSecret proves the auth gate +// can ACCEPT a correct secret and hand off to the controller. Without this +// test, the two reject tests above (which both run with the vault +// unconfigured and take the secret=="" short-circuit) would pass unchanged +// even if the handler's entire body were replaced with an unconditional 401 - +// hmac.Equal would never be proven to run on a real match. +// +// Clearing auth hands the request to controller.SubmitProviderEgressLocation, +// which looks up the client in the database before it can return "Unknown +// client.", so this test needs a real (throwaway) test database - see +// server.DefaultTestEnv, the same harness +// controller/provider_egress_location_controller_test.go uses. t.Setenv makes +// it self-sufficient under a plain `go test`, matching the pattern in +// router/warp_handlers_status_test.go. +func TestProviderEgressLocationSubmitAcceptsCorrectSecret(t *testing.T) { + t.Setenv("WARP_ENV", "local") + server.DefaultTestEnv().Run(t, func(t testing.TB) { + const secret = "correct-operator-secret-0123456789" + defer withStubOperatorIngestSecret(secret)() + + // A syntactically valid, semantically unregistered submission: once + // auth clears, the controller looks up the client and (since it does + // not exist in the fresh test database) returns "Unknown client.", + // surfaced by the handler as 400. That 400 is proof the request + // reached the controller, i.e. proof auth passed. + args := controller.SubmitProviderEgressLocationArgs{ + ClientId: server.NewId(), + CountryCode: "US", + Country: "United States", + CountryConfident: true, + ObservedAt: server.NowUtc(), + } + body, err := json.Marshal(args) + if err != nil { + t.Fatalf("marshal args: %s", err) + } + + req := httptest.NewRequest(http.MethodPost, "/network/provider-egress-location", bytes.NewReader(body)) + req.Header.Set(operatorSecretHeader, secret) + w := httptest.NewRecorder() + + ProviderEgressLocationSubmit(w, req) + + if w.Code == http.StatusUnauthorized { + t.Fatalf("status = %d, want the correct secret to clear auth (not 401)", w.Code) + } + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (Unknown client.) once auth clears for an unregistered client id; body = %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "Unknown client.") { + t.Fatalf("body = %q, want it to report the unknown client", w.Body.String()) + } + }) +} + +// TestProviderEgressLocationSubmitRejectsAlteredSecret proves that once the +// vault is configured, hmac.Equal is actually consulted rather than the +// endpoint accepting any request once secret != "". Same configured secret as +// the accept test above, but the request carries a one-character-altered +// value. +func TestProviderEgressLocationSubmitRejectsAlteredSecret(t *testing.T) { + const secret = "correct-operator-secret-0123456789" + const wrongSecret = "correct-operator-secret-0123456780" // last char changed + defer withStubOperatorIngestSecret(secret)() + + body, _ := json.Marshal(map[string]any{ + "client_id": "019f8835-158d-6fd8-e9dd-fd0e4c6d6792", + }) + req := httptest.NewRequest(http.MethodPost, "/network/provider-egress-location", bytes.NewReader(body)) + req.Header.Set(operatorSecretHeader, wrongSecret) + w := httptest.NewRecorder() + + ProviderEgressLocationSubmit(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401 when the operator secret is configured but the request's secret is wrong", w.Code) + } +} + +// TestProviderEgressLocationSubmitReadsSecretFromVault proves the vault +// plumbing itself - not the test stub - returns the configured secret: +// readOperatorIngestSecret (the un-memoized reader) reads a +// PushSimpleResource-injected provider_egress.yml. +func TestProviderEgressLocationSubmitReadsSecretFromVault(t *testing.T) { + const secret = "vault-provisioned-secret-abcdef" + pop := server.Vault.PushSimpleResource( + "provider_egress.yml", + []byte(`ingest_secret: "`+secret+`"`), + ) + defer pop() + + if got := readOperatorIngestSecret(); got != secret { + t.Fatalf("readOperatorIngestSecret() = %q, want %q", got, secret) + } +} + +func TestProviderEgressLocationDueRejectsMissingSecret(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/network/provider-egress-due", nil) + w := httptest.NewRecorder() + + ProviderEgressLocationDue(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401 when the operator secret header is absent", w.Code) + } +} + +func TestProviderEgressLocationDueRejectsWrongSecret(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/network/provider-egress-due", nil) + req.Header.Set(operatorSecretHeader, "definitely-not-the-secret") + w := httptest.NewRecorder() + + ProviderEgressLocationDue(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401 on a wrong operator secret", w.Code) + } +} + +// TestProviderEgressLocationDueRejectsAlteredSecret is the reject case with +// the vault *configured*, so the request gets past the secret == "" fail-closed +// short-circuit and hmac.Equal is what does the rejecting. +func TestProviderEgressLocationDueRejectsAlteredSecret(t *testing.T) { + const secret = "correct-operator-secret-0123456789" + const wrongSecret = "correct-operator-secret-0123456780" // last char changed + defer withStubOperatorIngestSecret(secret)() + + req := httptest.NewRequest(http.MethodGet, "/network/provider-egress-due", nil) + req.Header.Set(operatorSecretHeader, wrongSecret) + w := httptest.NewRecorder() + + ProviderEgressLocationDue(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401 when the operator secret is configured but the request's secret is wrong", w.Code) + } +} + +// testing_connectDueProvider stands up a connected + valid provider holding a +// Public provide key and no probe result, i.e. a provider the due query must +// return. The caller runs model.UpdateClientLocationReliabilities afterward. +func testing_connectDueProvider( + t testing.TB, + ctx context.Context, + clientId server.Id, + locationId server.Id, + clientAddress string, +) { + model.Testing_CreateDevice(ctx, server.NewId(), server.NewId(), clientId, "", "") + + handlerId := model.CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := model.ConnectNetworkClient(ctx, clientId, clientAddress, handlerId) + if err != nil { + t.Fatalf("connect client: %s", err) + } + if err := model.SetConnectionLocation(ctx, connectionId, locationId, &model.ConnectionLocationScores{}); err != nil { + t.Fatalf("set connection location: %s", err) + } + model.SetProvide(ctx, clientId, map[model.ProvideMode][]byte{ + model.ProvideModePublic: []byte("provide-secret"), + }) +} + +// TestProviderEgressLocationDueAcceptsCorrectSecret proves the auth gate can +// ACCEPT. This is the test that gives the three reject tests above their +// meaning: without it, a handler whose entire body was replaced with an +// unconditional `http.Error(w, "Unauthorized", 401)` would still pass all +// three, because a suite that only ever asserts rejections cannot tell a +// working auth check from a broken-shut one. +// +// It deliberately asserts more than "not 401": a real, never-probed provider +// is stood up in the test database and must come back in the response body, so +// the test also fails if the handler clears auth but never reaches the model +// query or writes the wrong json shape. +func TestProviderEgressLocationDueAcceptsCorrectSecret(t *testing.T) { + t.Setenv("WARP_ENV", "local") + server.DefaultTestEnv().Run(t, func(t testing.TB) { + const secret = "correct-operator-secret-0123456789" + defer withStubOperatorIngestSecret(secret)() + + ctx := context.Background() + + city := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + model.CreateLocation(ctx, city) + + due := server.NewId() + testing_connectDueProvider(t, ctx, due, city.LocationId, "0.0.0.1:0") + model.UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + + req := httptest.NewRequest(http.MethodGet, "/network/provider-egress-due?limit=10", nil) + req.Header.Set(operatorSecretHeader, secret) + w := httptest.NewRecorder() + + ProviderEgressLocationDue(w, req) + + if w.Code == http.StatusUnauthorized { + t.Fatalf("status = %d, want the correct secret to clear auth (not 401)", w.Code) + } + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) + } + + var result ProviderEgressLocationDueResult + if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil { + t.Fatalf("decode body %q: %s", w.Body.String(), err) + } + if !slices.Contains(result.ClientIds, due) { + t.Fatalf("client_ids = %v, want it to contain the never-probed provider %s", result.ClientIds, due) + } + // the wire name the prober reads + if !strings.Contains(w.Body.String(), `"client_ids"`) { + t.Fatalf("body = %s, want a client_ids field", w.Body.String()) + } + }) +} + +// The prober asks for a batch; the server must not hand back more than asked. +func TestProviderEgressLocationDueHonoursLimit(t *testing.T) { + t.Setenv("WARP_ENV", "local") + server.DefaultTestEnv().Run(t, func(t testing.TB) { + const secret = "correct-operator-secret-0123456789" + defer withStubOperatorIngestSecret(secret)() + + ctx := context.Background() + + city := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + model.CreateLocation(ctx, city) + + testing_connectDueProvider(t, ctx, server.NewId(), city.LocationId, "0.0.0.1:0") + testing_connectDueProvider(t, ctx, server.NewId(), city.LocationId, "0.0.0.2:0") + model.UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + + req := httptest.NewRequest(http.MethodGet, "/network/provider-egress-due?limit=1", nil) + req.Header.Set(operatorSecretHeader, secret) + w := httptest.NewRecorder() + + ProviderEgressLocationDue(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) + } + var result ProviderEgressLocationDueResult + if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil { + t.Fatalf("decode body %q: %s", w.Body.String(), err) + } + if len(result.ClientIds) != 1 { + t.Fatalf("len(client_ids) = %d, want 1 for limit=1; body = %s", len(result.ClientIds), w.Body.String()) + } + }) +} + +// Every other due test in this file stands up never-probed providers, which +// come back regardless of what cutoff the handler computes -- so nothing here +// actually exercised providerEgressDueAge. This one does: a provider probed +// just now must be held back, and one probed past the cutoff must come through. +// Defeating the cutoff (dropping it, computing it in the wrong direction, +// comparing against sql now() through the session timezone) fails this. +func TestProviderEgressLocationDueHonoursStalenessCutoff(t *testing.T) { + t.Setenv("WARP_ENV", "local") + server.DefaultTestEnv().Run(t, func(t testing.TB) { + const secret = "correct-operator-secret-0123456789" + defer withStubOperatorIngestSecret(secret)() + + ctx := context.Background() + + city := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + model.CreateLocation(ctx, city) + + fresh := server.NewId() + stale := server.NewId() + testing_connectDueProvider(t, ctx, fresh, city.LocationId, "0.0.0.1:0") + testing_connectDueProvider(t, ctx, stale, city.LocationId, "0.0.0.2:0") + model.UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + + now := server.NowUtc() + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: fresh, LocationId: city.LocationId, + CountryCode: "us", ObservedAt: now, + }) + // comfortably past providerEgressDueAge, which is half + // model.ProviderEgressLocationMaxAge + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: stale, LocationId: city.LocationId, + CountryCode: "us", ObservedAt: now.Add(-providerEgressDueAge - time.Hour), + }) + + req := httptest.NewRequest(http.MethodGet, "/network/provider-egress-due?limit=100", nil) + req.Header.Set(operatorSecretHeader, secret) + w := httptest.NewRecorder() + + ProviderEgressLocationDue(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) + } + var result ProviderEgressLocationDueResult + if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil { + t.Fatalf("decode body %q: %s", w.Body.String(), err) + } + if slices.Contains(result.ClientIds, fresh) { + t.Fatalf("client_ids = %v, must not contain the just-probed provider %s", result.ClientIds, fresh) + } + if !slices.Contains(result.ClientIds, stale) { + t.Fatalf("client_ids = %v, must contain the provider probed past the cutoff %s", result.ClientIds, stale) + } + }) +} + +func TestProviderEgressLocationAttemptRejectsMissingSecret(t *testing.T) { + body, _ := json.Marshal(map[string]any{ + "client_id": "019f8835-158d-6fd8-e9dd-fd0e4c6d6792", + }) + req := httptest.NewRequest(http.MethodPost, "/network/provider-egress-attempt", bytes.NewReader(body)) + w := httptest.NewRecorder() + + ProviderEgressLocationAttempt(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401 when the operator secret header is absent", w.Code) + } +} + +// The reject case with the vault *configured*, so the request gets past the +// secret == "" fail-closed short-circuit and hmac.Equal is what rejects. +func TestProviderEgressLocationAttemptRejectsAlteredSecret(t *testing.T) { + const secret = "correct-operator-secret-0123456789" + const wrongSecret = "correct-operator-secret-0123456780" // last char changed + defer withStubOperatorIngestSecret(secret)() + + body, _ := json.Marshal(map[string]any{ + "client_id": "019f8835-158d-6fd8-e9dd-fd0e4c6d6792", + }) + req := httptest.NewRequest(http.MethodPost, "/network/provider-egress-attempt", bytes.NewReader(body)) + req.Header.Set(operatorSecretHeader, wrongSecret) + w := httptest.NewRecorder() + + ProviderEgressLocationAttempt(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401 when the operator secret is configured but the request's secret is wrong", w.Code) + } +} + +// The whole point of the attempt endpoint, end to end over http: a provider +// that has never been probed successfully is due; the prober reports that it +// tried and failed; the provider stops being due. Without that, a provider +// whose probes always fail sits at the head of the queue on every poll forever +// (observed_at IS NULL sorts first) and starves every provider behind it. +func TestProviderEgressLocationAttemptDefersProvider(t *testing.T) { + t.Setenv("WARP_ENV", "local") + server.DefaultTestEnv().Run(t, func(t testing.TB) { + const secret = "correct-operator-secret-0123456789" + defer withStubOperatorIngestSecret(secret)() + + ctx := context.Background() + + city := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + model.CreateLocation(ctx, city) + + dead := server.NewId() + testing_connectDueProvider(t, ctx, dead, city.LocationId, "0.0.0.1:0") + model.UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + + if !slices.Contains(due(t, secret), dead) { + t.Fatalf("the never-probed provider %s must be due before any attempt is reported", dead) + } + + attemptBody, err := json.Marshal(controller.RecordProviderEgressProbeAttemptArgs{ + ClientId: dead, + ProbeFailure: "tunnel_failed", + }) + if err != nil { + t.Fatalf("marshal attempt: %s", err) + } + req := httptest.NewRequest(http.MethodPost, "/network/provider-egress-attempt", bytes.NewReader(attemptBody)) + req.Header.Set(operatorSecretHeader, secret) + w := httptest.NewRecorder() + + ProviderEgressLocationAttempt(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) + } + + attempt := model.GetProviderEgressProbeAttempt(ctx, dead) + if attempt == nil { + t.Fatal("expected the attempt to be recorded") + } + if attempt.ProbeFailure != "tunnel_failed" { + t.Fatalf("probe_failure = %q, want %q", attempt.ProbeFailure, "tunnel_failed") + } + + if slices.Contains(due(t, secret), dead) { + t.Fatalf("the provider %s must not be due again immediately after a failed attempt", dead) + } + }) +} + +// due drives the due endpoint over http and returns the batch. +func due(t testing.TB, secret string) []server.Id { + req := httptest.NewRequest(http.MethodGet, "/network/provider-egress-due?limit=100", nil) + req.Header.Set(operatorSecretHeader, secret) + w := httptest.NewRecorder() + + ProviderEgressLocationDue(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("due: status = %d, want 200; body = %s", w.Code, w.Body.String()) + } + var result ProviderEgressLocationDueResult + if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil { + t.Fatalf("due: decode body %q: %s", w.Body.String(), err) + } + return result.ClientIds +} + +// An unknown client id must be rejected rather than writing an attempt row +// keyed to a client that does not exist, which nothing would ever read. +func TestProviderEgressLocationAttemptRejectsUnknownClient(t *testing.T) { + t.Setenv("WARP_ENV", "local") + server.DefaultTestEnv().Run(t, func(t testing.TB) { + const secret = "correct-operator-secret-0123456789" + defer withStubOperatorIngestSecret(secret)() + + body, err := json.Marshal(controller.RecordProviderEgressProbeAttemptArgs{ + ClientId: server.NewId(), + ProbeFailure: "tunnel_failed", + }) + if err != nil { + t.Fatalf("marshal attempt: %s", err) + } + req := httptest.NewRequest(http.MethodPost, "/network/provider-egress-attempt", bytes.NewReader(body)) + req.Header.Set(operatorSecretHeader, secret) + w := httptest.NewRecorder() + + ProviderEgressLocationAttempt(w, req) + + if w.Code == http.StatusUnauthorized { + t.Fatalf("status = %d, want the correct secret to clear auth (not 401)", w.Code) + } + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 for an unregistered client id; body = %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "Unknown client.") { + t.Fatalf("body = %q, want it to report the unknown client", w.Body.String()) + } + }) +} + +// A limit that is not a positive integer is a caller bug. Silently clamping it +// to 1 (or to the default) would answer a question the prober did not ask -- +// `limit=0` would come back as an empty list, indistinguishable from "nothing +// is due" -- so it is rejected instead. +func TestProviderEgressLocationDueRejectsBadLimit(t *testing.T) { + const secret = "correct-operator-secret-0123456789" + defer withStubOperatorIngestSecret(secret)() + + for _, raw := range []string{"0", "-1", "abc", "1.5"} { + req := httptest.NewRequest(http.MethodGet, "/network/provider-egress-due?limit="+raw, nil) + req.Header.Set(operatorSecretHeader, secret) + w := httptest.NewRecorder() + + ProviderEgressLocationDue(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("limit=%q: status = %d, want 400", raw, w.Code) + } + } +} diff --git a/controller/network_client_controller.go b/controller/network_client_controller.go index 5ecd7252..3556cdb1 100644 --- a/controller/network_client_controller.go +++ b/controller/network_client_controller.go @@ -2,7 +2,8 @@ package controller import ( "context" - // "strings" + "net/netip" + "strings" "time" "github.com/urnetwork/glog" @@ -56,7 +57,80 @@ func SetConnectionLocation( connectionId server.Id, clientIp string, ) error { + // the mmdb lookup on the control ip. This is resolved up front even when + // the probed path is about to win, because the probed path has to know how + // precise the mmdb answer is before it can decide whether replacing it is + // an improvement (see probedLocationPreferred), and because it costs no db + // round trip -- it is an in-process maxminddb read plus an ARIN lookup. + // `err` is deliberately not returned yet: a failed mmdb lookup is not a + // reason to discard a perfectly good probed location. location, connectionLocationScores, err := GetLocationForIp(ctx, clientIp) + + // a provider probed through its own egress is located from that probe, not + // from a lookup on its control-connection ip: the egress is where user + // traffic actually exits, and an operator-run prober learns it by routing + // geolocation lookups through the provider itself and cross-checking them + // across several sources, then submits the result here. When a fresh + // probed entry exists we prefer it over the built-in mmdb lookup on the + // control ip -- subject to probedLocationPreferred, which is what stops the + // probe making a provider *less* locatable than it was. + // GetFreshProviderEgressLocationForConnection is + // a single query joining network_client_connection to + // provider_egress_location: this runs for every connection (provider or + // not) on the connect-announce path and inside a retry loop, so it must + // not cost the two round trips (client lookup, then egress lookup) the + // naive version would. + if egress := model.GetFreshProviderEgressLocationForConnection( + ctx, + connectionId, + model.ProviderEgressLocationMaxAge, + ); egress != nil && probedLocationPreferred(egress, location) { + scores := &model.ConnectionLocationScores{} + if egress.Hosting { + scores.NetTypeHosting = 1 + } + if egress.Proxy { + scores.NetTypePrivacy = 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). It is + // computed exactly as the mmdb path computes it in GetLocationForIp: + // the ARIN org country of the control ip against the mmdb country of + // that SAME control ip -- not the probed country, which is a different + // question (whether probing changed the answer) and must not be + // silently folded into this ranking penalty. It is recomputed here + // rather than lifted off connectionLocationScores because that struct + // is nil whenever GetLocationForIp failed, including the case where + // mmdb resolved the ip fine but GuessLocationType could not classify + // the result -- the foreign check is still meaningful there. Any + // lookup failure just leaves NetTypeForeign at 0; it must never fail + // or panic this path. + if addr, err := netip.ParseAddr(clientIp); err == nil { + if ipInfo, err := server.GetIpInfo(addr); err == nil { + scores.NetTypeForeign = arinForeignScore(addr, ipInfo.CountryCode) + } + } + setErr := model.SetConnectionLocation(ctx, connectionId, egress.LocationId, scores) + if setErr == nil { + return nil + } + // fall through to the mmdb path on a storage error + glog.Infof("[ncc][%s]could not set probed egress location. err = %s\n", connectionId, setErr) + } + if err != nil { // server.Logger().Printf("Get ip for location error: %s", err) glog.Infof("[ncc][%s]could not find client location. err = %s\n", connectionId, err) @@ -73,6 +147,68 @@ func SetConnectionLocation( return nil } +// probedLocationPreferred decides whether the probed egress location should be +// written for this connection instead of the mmdb location resolved from the +// control ip. mmdbLocation is nil when the mmdb lookup failed. +// +// Read this before changing it: the naive rule -- "a probe is better evidence +// than mmdb, so the probe always wins" -- is wrong, and produced a live +// regression. The probed location is not always as *precise* as the mmdb one. +// SubmitProviderEgressLocation only stores a city when the probed city matches +// a location row that already exists; anything else is stored at country +// granularity, deliberately, so that a probe can never mint new city rows in +// the shared `location` table. Cities are not seeded either -- AddDefaultLocations +// runs with cityLimit = 0 -- so the pool a probed city can match against is only +// the rows organic traffic happened to create, and a country-granular fallback +// is the common case, not a rare one. +// +// Letting that country row overwrite an mmdb *city* row would drop the provider +// out of every city filter in FindProviders2 and GetProviderLocations. Being +// probed would make a provider less discoverable than never having been probed +// at all -- a penalty for participating. +// +// So the rule is: a probe may CORRECT the location, but never COARSEN it. +// +// - The probe stored a city (CityConfident): it is at least as precise as +// anything mmdb has, and it is better evidence. It wins. +// - No usable mmdb answer: the probe is the only evidence there is. It wins. +// - The mmdb answer is itself country-granular: nothing to lose. The probe +// wins, and this is the case country-level correction exists for. +// - The mmdb answer is city- or region-granular in a DIFFERENT country: the +// mmdb row is not more precise, it is precisely wrong, and its city is a +// city in the wrong country. The probe wins. This is the other half of +// country-level correction and the reason this is not just "keep whichever +// is finer". +// - The mmdb answer is city- or region-granular in the SAME country the probe +// reports: the probe agrees with mmdb and adds nothing except a loss of +// granularity. mmdb wins. +// +// CityConfident is used as the probed row's granularity because the schema +// invariant is that provider_egress_location.location_id is a city row exactly +// when city_confident is set (see the provider_egress_location migration and +// SubmitProviderEgressLocation). Reading it off the flag keeps this on the hot +// connect-announce path without a second query for the location row's type. +func probedLocationPreferred( + egress *model.ProviderEgressLocation, + mmdbLocation *model.Location, +) bool { + if egress.CityConfident { + return true + } + if mmdbLocation == nil { + return true + } + if mmdbLocation.LocationType != model.LocationTypeCity && + mmdbLocation.LocationType != model.LocationTypeRegion { + return true + } + // both country codes are stored lowercased -- SubmitProviderEgressLocation + // lowercases the probed one and GetLocationForIp takes mmdb's, which the + // location table also stores lowercased -- but fold anyway rather than let + // a casing difference read as a country disagreement and silently coarsen. + return !strings.EqualFold(egress.CountryCode, mmdbLocation.CountryCode) +} + /* func SetMissingConnectionLocations(ctx context.Context, minTime time.Time) { connectionIpStrs := map[server.Id]string{} diff --git a/controller/network_client_controller_test.go b/controller/network_client_controller_test.go new file mode 100644 index 00000000..1276bf9d --- /dev/null +++ b/controller/network_client_controller_test.go @@ -0,0 +1,543 @@ +package controller + +import ( + "context" + "testing" + "time" + + "github.com/urnetwork/connect" + "github.com/urnetwork/server" + "github.com/urnetwork/server/model" +) + +// A provider with a fresh probed egress location must be located from that +// entry, not from the mmdb lookup on its control ip. +func TestSetConnectionLocationPrefersEgressLocation(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + // the probed egress location: japan + probed := &model.Location{ + LocationType: model.LocationTypeCountry, + Country: "Japan", + CountryCode: "jp", + } + model.CreateLocation(ctx, probed) + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + handlerId := model.CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := model.ConnectNetworkClient(ctx, clientId, "8.8.8.8:0", handlerId) + connect.AssertEqual(t, err, nil) + + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: clientId, + LocationId: probed.LocationId, + CountryCode: "jp", + ObservedAt: server.NowUtc(), + }) + + err = SetConnectionLocation(ctx, connectionId, "8.8.8.8") + connect.AssertEqual(t, err, nil) + + var countryLocationId server.Id + server.Db(ctx, func(conn server.PgConn) { + result, qerr := conn.Query( + ctx, + `SELECT country_location_id FROM network_client_location WHERE connection_id = $1`, + connectionId, + ) + server.WithPgResult(result, qerr, func() { + if result.Next() { + server.Raise(result.Scan(&countryLocationId)) + } + }) + }) + connect.AssertEqual(t, countryLocationId, probed.CountryLocationId) + }) +} + +// With no probed entry, the existing mmdb path still applies. +func TestSetConnectionLocationFallsBackToMmdb(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, "", "") + + handlerId := model.CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := model.ConnectNetworkClient(ctx, clientId, "8.8.8.8:0", handlerId) + connect.AssertEqual(t, err, nil) + + // no SetProviderEgressLocation call -> mmdb path + err = SetConnectionLocation(ctx, connectionId, "8.8.8.8") + connect.AssertEqual(t, err, nil) + + var count int + server.Db(ctx, func(conn server.PgConn) { + result, qerr := conn.Query( + ctx, + `SELECT COUNT(*) FROM network_client_location WHERE connection_id = $1`, + connectionId, + ) + server.WithPgResult(result, qerr, func() { + if result.Next() { + server.Raise(result.Scan(&count)) + } + }) + }) + connect.AssertEqual(t, count, 1) + }) +} + +// A probed egress location observed longer ago than ProviderEgressLocationMaxAge +// must be ignored, falling back to the mmdb lookup exactly as if there were no +// probed entry at all. +func TestSetConnectionLocationStaleProbedFallsBackToMmdb(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + clientIp := "8.8.8.8" + + // the mmdb location this ip actually resolves to; created up front so + // its canonical (deduped) location id is known for the assertion. + mmdbLocation, _, err := GetLocationForIp(ctx, clientIp) + connect.AssertEqual(t, err, nil) + model.CreateLocation(ctx, mmdbLocation) + + // a stale probed location, deliberately a different country than + // whatever the mmdb lookup returns, so a wrongly-preferred probed + // value would be caught by the assertion below. + probed := &model.Location{ + LocationType: model.LocationTypeCountry, + Country: "Japan", + CountryCode: "jp", + } + model.CreateLocation(ctx, probed) + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + handlerId := model.CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := model.ConnectNetworkClient(ctx, clientId, clientIp+":0", handlerId) + connect.AssertEqual(t, err, nil) + + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: clientId, + LocationId: probed.LocationId, + CountryCode: "jp", + ObservedAt: server.NowUtc().Add(-(model.ProviderEgressLocationMaxAge + time.Hour)), + }) + + err = SetConnectionLocation(ctx, connectionId, clientIp) + connect.AssertEqual(t, err, nil) + + var countryLocationId server.Id + server.Db(ctx, func(conn server.PgConn) { + result, qerr := conn.Query( + ctx, + `SELECT country_location_id FROM network_client_location WHERE connection_id = $1`, + connectionId, + ) + server.WithPgResult(result, qerr, func() { + if result.Next() { + server.Raise(result.Scan(&countryLocationId)) + } + }) + }) + connect.AssertEqual(t, countryLocationId, mmdbLocation.CountryLocationId) + if countryLocationId == probed.CountryLocationId { + t.Fatal("stale probed location must not be used") + } + }) +} + +// A probed egress location whose location_id points at no existing location +// row makes the storage write fail (SetConnectionLocation returns an error +// rather than panicking, see model.SetConnectionLocation). The connection +// must still end up located via the mmdb path, and SetConnectionLocation +// itself must not panic or error out on this. +func TestSetConnectionLocationProbedWriteErrorFallsBackToMmdb(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + clientIp := "8.8.8.8" + + mmdbLocation, _, err := GetLocationForIp(ctx, clientIp) + connect.AssertEqual(t, err, nil) + model.CreateLocation(ctx, mmdbLocation) + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + handlerId := model.CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := model.ConnectNetworkClient(ctx, clientId, clientIp+":0", handlerId) + connect.AssertEqual(t, err, nil) + + // a fresh probed entry pointing at a location id that was never + // created via CreateLocation -- the storage write in + // model.SetConnectionLocation must fail cleanly on this, not panic. + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: clientId, + LocationId: server.NewId(), + CountryCode: "jp", + ObservedAt: server.NowUtc(), + }) + + err = SetConnectionLocation(ctx, connectionId, clientIp) + connect.AssertEqual(t, err, nil) + + var countryLocationId server.Id + server.Db(ctx, func(conn server.PgConn) { + result, qerr := conn.Query( + ctx, + `SELECT country_location_id FROM network_client_location WHERE connection_id = $1`, + connectionId, + ) + server.WithPgResult(result, qerr, func() { + if result.Next() { + server.Raise(result.Scan(&countryLocationId)) + } + }) + }) + connect.AssertEqual(t, countryLocationId, mmdbLocation.CountryLocationId) + }) +} + +// net_type_foreign on the probed path must be computed the same way as the +// mmdb path: the ARIN org country of the control ip against the mmdb country +// of that SAME control ip, not against the probed country. 8.8.8.8 resolves +// (both via mmdb and ARIN org registration) to "us", so it is non-foreign by +// construction -- this is the same ip used by the parity tests above. The +// probed country here is deliberately "jp", a different country than the +// control ip's mmdb country: probing having changed the answer must not, by +// itself, flip net_type_foreign, or a probed provider is penalized a full +// ranking tier precisely for the reason this feature exists. A prior version +// of this code compared the ARIN org country against the probed country +// instead of the control ip's mmdb country, which made this exact scenario +// (egress country differs from control-ip country) foreign=1 instead of the +// correct foreign=0. +func TestSetConnectionLocationProbedNetTypeForeignMatchesMmdbParity(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + clientIp := "8.8.8.8" + + probed := &model.Location{ + LocationType: model.LocationTypeCountry, + Country: "Japan", + CountryCode: "jp", + } + model.CreateLocation(ctx, probed) + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + handlerId := model.CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := model.ConnectNetworkClient(ctx, clientId, clientIp+":0", handlerId) + connect.AssertEqual(t, err, nil) + + // probed country ("jp") deliberately differs from the control ip's + // mmdb country ("us" for 8.8.8.8). + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: clientId, + LocationId: probed.LocationId, + CountryCode: "jp", + ObservedAt: server.NowUtc(), + }) + + err = SetConnectionLocation(ctx, connectionId, clientIp) + connect.AssertEqual(t, err, nil) + + var netTypeForeign int + server.Db(ctx, func(conn server.PgConn) { + result, qerr := conn.Query( + ctx, + `SELECT net_type_foreign FROM network_client_location WHERE connection_id = $1`, + connectionId, + ) + server.WithPgResult(result, qerr, func() { + if result.Next() { + server.Raise(result.Scan(&netTypeForeign)) + } + }) + }) + connect.AssertEqual(t, netTypeForeign, 0) + }) +} + +// A fresh probed location's Hosting/Proxy flags must map onto the stored +// 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() + + probed := &model.Location{ + LocationType: model.LocationTypeCountry, + Country: "Japan", + CountryCode: "jp", + } + model.CreateLocation(ctx, probed) + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + handlerId := model.CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := model.ConnectNetworkClient(ctx, clientId, "8.8.8.8:0", handlerId) + connect.AssertEqual(t, err, nil) + + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: clientId, + LocationId: probed.LocationId, + CountryCode: "jp", + Hosting: true, + Proxy: true, + Mobile: true, + ObservedAt: server.NowUtc(), + }) + + err = SetConnectionLocation(ctx, connectionId, "8.8.8.8") + connect.AssertEqual(t, err, nil) + + 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, 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, &netTypeVirtual)) + } + }) + }) + connect.AssertEqual(t, netTypeHosting, 1) + connect.AssertEqual(t, netTypePrivacy, 1) + connect.AssertEqual(t, netTypeVirtual, 0) + }) +} + +// testing_connectionLocationIds reads back the granularity actually stored for +// a connection. +func testing_connectionLocationIds(ctx context.Context, connectionId server.Id) ( + cityLocationId server.Id, + regionLocationId server.Id, + countryLocationId server.Id, +) { + server.Db(ctx, func(conn server.PgConn) { + result, qerr := conn.Query( + ctx, + ` + SELECT city_location_id, region_location_id, country_location_id + FROM network_client_location + WHERE connection_id = $1 + `, + connectionId, + ) + server.WithPgResult(result, qerr, func() { + if result.Next() { + server.Raise(result.Scan( + &cityLocationId, + ®ionLocationId, + &countryLocationId, + )) + } + }) + }) + return +} + +// A probe must never make a provider LESS locatable than leaving it unprobed +// would have. +// +// SubmitProviderEgressLocation stores country granularity whenever the probed +// city does not match a location row that already exists, which is the common +// case -- cities are not seeded, so the match pool is only what organic traffic +// created. If that country row then overwrote the mmdb city row unconditionally, +// the provider would fall out of every city filter: probed providers would be +// harder to find than unprobed ones, which is the exact opposite of the point. +// +// mmdb resolves 24.48.0.1 to Montreal, Quebec, CA at city granularity. A +// country-only probe agreeing that the provider is in CA adds nothing, so the +// mmdb city has to survive. +func TestSetConnectionLocationProbedCountryDoesNotCoarsenMmdbCity(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + clientIp := "24.48.0.1" + + mmdbLocation, _, err := GetLocationForIp(ctx, clientIp) + connect.AssertEqual(t, err, nil) + // the fixture is only meaningful if mmdb really has a city here + connect.AssertEqual(t, mmdbLocation.LocationType, model.LocationTypeCity) + model.CreateLocation(ctx, mmdbLocation) + + // the probed location: the same country, but only the country -- + // exactly what an unmatched city name falls back to + probed := &model.Location{ + LocationType: model.LocationTypeCountry, + Country: "Canada", + CountryCode: "ca", + } + model.CreateLocation(ctx, probed) + connect.AssertEqual(t, probed.LocationId, mmdbLocation.CountryLocationId) + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + handlerId := model.CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := model.ConnectNetworkClient(ctx, clientId, clientIp+":0", handlerId) + connect.AssertEqual(t, err, nil) + + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: clientId, + LocationId: probed.LocationId, + CountryCode: "ca", + CityConfident: false, + ObservedAt: server.NowUtc(), + }) + + err = SetConnectionLocation(ctx, connectionId, clientIp) + connect.AssertEqual(t, err, nil) + + cityLocationId, regionLocationId, countryLocationId := testing_connectionLocationIds(ctx, connectionId) + if cityLocationId != mmdbLocation.CityLocationId { + t.Errorf( + "city_location_id = %s, want the mmdb city %s: a country-only probe must not coarsen a connection the mmdb already placed in a city", + cityLocationId, + mmdbLocation.CityLocationId, + ) + } + connect.AssertEqual(t, regionLocationId, mmdbLocation.RegionLocationId) + connect.AssertEqual(t, countryLocationId, mmdbLocation.CountryLocationId) + // and the collapse this guards against, stated directly + if cityLocationId == countryLocationId { + t.Errorf("city/region/country all collapsed to %s; the provider is no longer in any city filter", countryLocationId) + } + }) +} + +// The other half of the rule: country-level CORRECTION still wins. When the +// probe says the egress is in a different country than the control ip's mmdb +// city, that city is a city in the wrong country -- it is not more precise, it +// is precisely wrong -- so the probed country replaces it. This is the whole +// reason the feature exists and must not be lost to the anti-coarsening rule +// above. +func TestSetConnectionLocationProbedCountryCorrectsMmdbCityInAnotherCountry(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + clientIp := "24.48.0.1" + + mmdbLocation, _, err := GetLocationForIp(ctx, clientIp) + connect.AssertEqual(t, err, nil) + connect.AssertEqual(t, mmdbLocation.LocationType, model.LocationTypeCity) + connect.AssertEqual(t, mmdbLocation.CountryCode, "ca") + model.CreateLocation(ctx, mmdbLocation) + + probed := &model.Location{ + LocationType: model.LocationTypeCountry, + Country: "Japan", + CountryCode: "jp", + } + model.CreateLocation(ctx, probed) + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + handlerId := model.CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := model.ConnectNetworkClient(ctx, clientId, clientIp+":0", handlerId) + connect.AssertEqual(t, err, nil) + + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: clientId, + LocationId: probed.LocationId, + CountryCode: "jp", + CityConfident: false, + ObservedAt: server.NowUtc(), + }) + + err = SetConnectionLocation(ctx, connectionId, clientIp) + connect.AssertEqual(t, err, nil) + + _, _, countryLocationId := testing_connectionLocationIds(ctx, connectionId) + if countryLocationId != probed.CountryLocationId { + t.Errorf( + "country_location_id = %s, want the probed country %s: a probe that disagrees with mmdb about the country must still correct it", + countryLocationId, + probed.CountryLocationId, + ) + } + if countryLocationId == mmdbLocation.CountryLocationId { + t.Errorf("kept the mmdb country %s; the provider is still advertised in the wrong country", mmdbLocation.CountryLocationId) + } + }) +} + +// A city-confident probe is at least as precise as anything mmdb has and is +// better evidence, so it wins outright -- including against an mmdb city in +// another country. Nothing is coarsened, so the anti-coarsening rule does not +// apply. +func TestSetConnectionLocationProbedCityWinsOverMmdbCity(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + clientIp := "24.48.0.1" + + mmdbLocation, _, err := GetLocationForIp(ctx, clientIp) + connect.AssertEqual(t, err, nil) + connect.AssertEqual(t, mmdbLocation.LocationType, model.LocationTypeCity) + model.CreateLocation(ctx, mmdbLocation) + + probed := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Tokyo", + Region: "Tokyo", + Country: "Japan", + CountryCode: "jp", + } + model.CreateLocation(ctx, probed) + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + handlerId := model.CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := model.ConnectNetworkClient(ctx, clientId, clientIp+":0", handlerId) + connect.AssertEqual(t, err, nil) + + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: clientId, + LocationId: probed.LocationId, + CountryCode: "jp", + CityConfident: true, + ObservedAt: server.NowUtc(), + }) + + err = SetConnectionLocation(ctx, connectionId, clientIp) + connect.AssertEqual(t, err, nil) + + cityLocationId, _, countryLocationId := testing_connectionLocationIds(ctx, connectionId) + connect.AssertEqual(t, cityLocationId, probed.CityLocationId) + connect.AssertEqual(t, countryLocationId, probed.CountryLocationId) + }) +} diff --git a/controller/network_client_location_controller.go b/controller/network_client_location_controller.go index 3376a9af..a8796b8b 100644 --- a/controller/network_client_location_controller.go +++ b/controller/network_client_location_controller.go @@ -55,21 +55,35 @@ func GetLocationForIp(ctx context.Context, clientIp string) (*model.Location, *m connectionLocationScores.NetTypeVirtual = 1 } + connectionLocationScores.NetTypeForeign = arinForeignScore(addr, ipInfo.CountryCode) + + return location, connectionLocationScores, nil +} + +// arinForeignScore cross-checks the ARIN org registration country for addr +// against countryCode. Both the ordinary path (GetLocationForIp) and the +// provider-egress path (SetConnectionLocation, in network_client_controller.go) pass the +// mmdb-resolved country of addr here, not the probed egress country: this is +// deliberate parity, so a probed and an unprobed provider on the same +// control ip are scored on the same basis and probing does not, by itself, +// change this ranking signal. If the org's registered country differs, the +// use case is considered foreign (VPN/proxy-like), matching the heuristic +// previously inlined in GetLocationForIp. +// +// If the ARIN lookup fails, this returns 0 without error: it must never fail +// or panic a caller on the connect-announce hot path over a missing/failed +// foreign check. +func arinForeignScore(addr netip.Addr, countryCode string) int { arinInfo, err := server.GetArinInfo(addr) - if err == nil { - // if the org ownership does not match the ip country, - // we consider the use case of the ip to be virtual - foreign := false - for _, orgCountryCode := range arinInfo.OrgCountryCodes { - if orgCountryCode != ipInfo.CountryCode { - foreign = true - break - } - } - if foreign { - connectionLocationScores.NetTypeForeign = 1 + if err != nil { + return 0 + } + // if the org ownership does not match the claimed country, + // we consider the use case of the ip to be foreign + for _, orgCountryCode := range arinInfo.OrgCountryCodes { + if orgCountryCode != countryCode { + return 1 } } - - return location, connectionLocationScores, nil + return 0 } diff --git a/controller/probed_location_preferred_test.go b/controller/probed_location_preferred_test.go new file mode 100644 index 00000000..33ac26a2 --- /dev/null +++ b/controller/probed_location_preferred_test.go @@ -0,0 +1,75 @@ +package controller + +import ( + "testing" + + "github.com/urnetwork/server" + "github.com/urnetwork/server/model" +) + +// The decision rule on its own, with no database and no mmdb file: a probe may +// CORRECT a location but must never COARSEN it. The end-to-end behaviour is +// covered by the TestSetConnectionLocation* tests; this pins the rule itself so +// each clause is exercised independently of whichever mmdb the environment has. +// +// Every row here fails against at least one plausible wrong rule -- "the probe +// always wins" (the bug), or "keep whichever answer is finer" (the overcorrection +// that would throw away country correction). +func TestProbedLocationPreferred(t *testing.T) { + probedCity := func(countryCode string) *model.ProviderEgressLocation { + return &model.ProviderEgressLocation{ + LocationId: server.NewId(), + CountryCode: countryCode, + CityConfident: true, + } + } + probedCountry := func(countryCode string) *model.ProviderEgressLocation { + return &model.ProviderEgressLocation{ + LocationId: server.NewId(), + CountryCode: countryCode, + CityConfident: false, + } + } + mmdb := func(locationType string, countryCode string) *model.Location { + return &model.Location{ + LocationId: server.NewId(), + LocationType: locationType, + CountryCode: countryCode, + } + } + + tests := []struct { + name string + egress *model.ProviderEgressLocation + mmdb *model.Location + want bool + }{ + // the regression: a country-only probe must not replace an mmdb city + // in the same country. This is the row that fails against "the probe + // always wins". + {"country probe vs mmdb city, same country", probedCountry("ca"), mmdb(model.LocationTypeCity, "ca"), false}, + {"country probe vs mmdb region, same country", probedCountry("ca"), mmdb(model.LocationTypeRegion, "ca"), false}, + // casing must not read as a country disagreement + {"country probe vs mmdb city, same country cased", probedCountry("CA"), mmdb(model.LocationTypeCity, "ca"), false}, + + // country correction: these fail against "keep whichever is finer" + {"country probe vs mmdb city, other country", probedCountry("jp"), mmdb(model.LocationTypeCity, "ca"), true}, + {"country probe vs mmdb region, other country", probedCountry("jp"), mmdb(model.LocationTypeRegion, "ca"), true}, + + // nothing to lose + {"country probe vs mmdb country, same", probedCountry("ca"), mmdb(model.LocationTypeCountry, "ca"), true}, + {"country probe vs mmdb country, other", probedCountry("jp"), mmdb(model.LocationTypeCountry, "ca"), true}, + {"country probe vs no mmdb answer", probedCountry("jp"), nil, true}, + + // a city-confident probe is never a downgrade + {"city probe vs mmdb city, same country", probedCity("ca"), mmdb(model.LocationTypeCity, "ca"), true}, + {"city probe vs mmdb city, other country", probedCity("jp"), mmdb(model.LocationTypeCity, "ca"), true}, + {"city probe vs mmdb region", probedCity("ca"), mmdb(model.LocationTypeRegion, "ca"), true}, + {"city probe vs no mmdb answer", probedCity("jp"), nil, true}, + } + for _, test := range tests { + if got := probedLocationPreferred(test.egress, test.mmdb); got != test.want { + t.Errorf("%s: probedLocationPreferred = %v, want %v", test.name, got, test.want) + } + } +} diff --git a/controller/provider_egress_location_controller.go b/controller/provider_egress_location_controller.go new file mode 100644 index 00000000..8cac0dcc --- /dev/null +++ b/controller/provider_egress_location_controller.go @@ -0,0 +1,245 @@ +package controller + +import ( + "context" + "fmt" + "strings" + "time" + + "github.com/urnetwork/server" + "github.com/urnetwork/server/model" +) + +// MaxProviderEgressLocationSubmissionAge rejects a submission whose probe is +// 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 +// CreateLocation panic on a Postgres "value too long for type character +// varying(128)" error. +const maxLocationNameLen = 128 + +// maxOrgLen mirrors maxLocationNameLen for org, which is stored in +// provider_egress_location.org, a varchar(256) column. +const maxOrgLen = 256 + +type SubmitProviderEgressLocationArgs struct { + ClientId server.Id `json:"client_id"` + CountryCode string `json:"country_code"` + Country string `json:"country"` + Region string `json:"region,omitempty"` + City string `json:"city,omitempty"` + ASN int `json:"asn,omitempty"` + Org string `json:"org,omitempty"` + Hosting bool `json:"hosting,omitempty"` + Proxy bool `json:"proxy,omitempty"` + Mobile bool `json:"mobile,omitempty"` + CountryConfident bool `json:"country_confident"` + CityConfident bool `json:"city_confident,omitempty"` + ObservedAt time.Time `json:"observed_at"` +} + +type SubmitProviderEgressLocationResult struct { + LocationId server.Id `json:"location_id"` +} + +// SubmitProviderEgressLocation records a probed egress location for a provider. +// Only country-confident submissions are accepted; city/region are stored only +// when the probe was also city-confident (free geolocation sources disagree on +// city often enough that an unconfirmed city is worse than none). +func SubmitProviderEgressLocation( + ctx context.Context, + args *SubmitProviderEgressLocationArgs, +) (*SubmitProviderEgressLocationResult, error) { + if !args.CountryConfident { + return nil, fmt.Errorf("Submission is not country-confident.") + } + countryCode := strings.ToLower(strings.TrimSpace(args.CountryCode)) + if len(countryCode) != 2 { + return nil, fmt.Errorf("Country code must be alpha-2.") + } + if args.ObservedAt.IsZero() { + return nil, fmt.Errorf("Missing observed_at.") + } + 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.") + } + + // country is always used to resolve/create a location row (at minimum + // the country-granular one), and model.CreateLocation dedupes country + // rows on (location_type, country_code): an empty name here would create + // a canonical row with location_name='' that every later lookup for this + // country reuses forever, even after a subsequent real mmdb lookup. Reject + // rather than silently falling back, so the prober learns it sent a bad + // payload instead of the server permanently corrupting shared data. + country := strings.TrimSpace(args.Country) + if country == "" { + return nil, fmt.Errorf("Missing country.") + } + if maxLocationNameLen < len(country) { + return nil, fmt.Errorf("Country is too long.") + } + if maxOrgLen < len(args.Org) { + return nil, fmt.Errorf("Org is too long.") + } + + // city/region are only used (and their rows only created) when the probe + // was city-confident; the same empty-name corruption applies to them, so + // require both are present and reject rather than silently dropping to + // country granularity on a bad payload. + var city, region string + if args.CityConfident { + city = strings.TrimSpace(args.City) + region = strings.TrimSpace(args.Region) + if city == "" { + return nil, fmt.Errorf("Missing city for a city-confident submission.") + } + if region == "" { + return nil, fmt.Errorf("Missing region for a city-confident submission.") + } + if maxLocationNameLen < len(city) { + return nil, fmt.Errorf("City is too long.") + } + if maxLocationNameLen < len(region) { + return nil, fmt.Errorf("Region is too long.") + } + } + + // resolve to a location row. City granularity only when the probe agreed + // on a city AND that city already exists in the location table. + // + // The probe MUST NOT define new cities or regions. model.CreateLocation + // dedupes a city on its exact location_name, so an unrecognised spelling + // does not fail -- it silently inserts a new permanent row into the shared + // `location` table and adds it to the search index. The three free + // geolocation sources the prober reaches consensus over demonstrably + // disagree on spelling ("Frankfurt am Main (Innenstadt I)" vs "Frankfurt am + // Main" for the same host, observed), and the consensus keeps the winning + // source's original display string -- so "Frankfurt am Main", "Frankfurt Am + // Main" and "Frankfurt/Main" would each become their own row. Those rows + // survive a code revert and there is no cleanup path. + // + // model.MatchExistingLocation therefore matches only, never creates, + // case-insensitively and ignoring punctuation/whitespace/accents and + // parenthesised district qualifiers, so the ordinary variants -- the + // "(Innenstadt I)" case above included -- land on the row that is already + // there. When it does not resolve, + // this submission falls back to country granularity: country is the + // granularity this design treats as trustworthy anyway, and losing city + // precision for one probe is strictly better than permanently polluting a + // table shared with the provider list and the location search. + var location *model.Location + if args.CityConfident { + location = model.MatchExistingLocation(ctx, countryCode, region, city) + } + + // city_confident records the granularity of the row actually stored, not + // what the probe claimed. The schema's documented invariant is that + // location_id is a city row exactly when city_confident is set (see the + // provider_egress_location migration), and a city-confident probe whose + // city did not resolve is stored at country granularity. + cityConfident := location != nil + + if location == nil { + // country granularity. This still goes through CreateLocation: a + // country row is keyed on country_code, so a variant *name* can never + // produce a second row for the same country the way a variant city name + // can -- the pollution this guards against is not reachable here. The + // country row is also the whole point of the fallback, so a probe from + // a country not yet in the table must not be dropped. + location = &model.Location{ + LocationType: model.LocationTypeCountry, + Country: country, + CountryCode: countryCode, + } + model.CreateLocation(ctx, location) + } + + model.SetProviderEgressLocation(ctx, &model.ProviderEgressLocation{ + ClientId: args.ClientId, + LocationId: location.LocationId, + CountryCode: countryCode, + ASN: args.ASN, + Org: args.Org, + Hosting: args.Hosting, + Proxy: args.Proxy, + Mobile: args.Mobile, + CityConfident: cityConfident, + ObservedAt: args.ObservedAt, + }) + + return &SubmitProviderEgressLocationResult{LocationId: location.LocationId}, nil +} + +// maxProbeFailureLen bounds the failure class as submitted: +// provider_egress_probe_attempt.probe_failure is a varchar(64), and rejecting +// an over-long value with a clear error beats letting the insert panic on a +// Postgres "value too long" error and spin in the retry loop. +const maxProbeFailureLen = 64 + +type RecordProviderEgressProbeAttemptArgs struct { + ClientId server.Id `json:"client_id"` + // ProbeFailure is "" when the attempt succeeded, otherwise a short failure + // class (`contract_failed`, `tunnel_failed`, `no_consensus`, ...). + ProbeFailure string `json:"probe_failure,omitempty"` +} + +type RecordProviderEgressProbeAttemptResult struct { + AttemptAt time.Time `json:"attempt_at"` +} + +// RecordProviderEgressProbeAttempt records that the prober tried this provider. +// A failed attempt defers the provider from the due queue for +// ProviderEgressProbeAttemptBackoff, exactly as a successful probe defers it +// for the (much longer) staleness window -- without this, a provider that +// always fails to probe never gets a provider_egress_location row and so stays +// permanently at the head of the queue, starving every other provider. See +// model.GetProviderEgressLocationDue. +// +// The attempt is timestamped by the server, not the prober: the prober is +// reporting something it just did, and a prober whose clock ran fast could +// otherwise defer a provider far past the backoff window. +func RecordProviderEgressProbeAttempt( + ctx context.Context, + args *RecordProviderEgressProbeAttemptArgs, +) (*RecordProviderEgressProbeAttemptResult, error) { + if maxProbeFailureLen < len(args.ProbeFailure) { + return nil, fmt.Errorf("Probe failure class is too long.") + } + // same check as SubmitProviderEgressLocation: without it a typo'd or stale + // client id writes a row keyed to a client that does not exist, which + // nothing ever reads and only the sweep ever removes. + if networkId := model.GetNetworkClientNetwork(ctx, args.ClientId); networkId == nil { + return nil, fmt.Errorf("Unknown client.") + } + + attemptAt := server.NowUtc() + model.SetProviderEgressProbeAttempt(ctx, &model.ProviderEgressProbeAttempt{ + ClientId: args.ClientId, + AttemptAt: attemptAt, + ProbeFailure: args.ProbeFailure, + }) + + return &RecordProviderEgressProbeAttemptResult{AttemptAt: attemptAt}, nil +} diff --git a/controller/provider_egress_location_controller_test.go b/controller/provider_egress_location_controller_test.go new file mode 100644 index 00000000..45b28d22 --- /dev/null +++ b/controller/provider_egress_location_controller_test.go @@ -0,0 +1,744 @@ +package controller + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/urnetwork/connect" + "github.com/urnetwork/server" + "github.com/urnetwork/server/model" +) + +func TestSubmitProviderEgressLocationCountryOnly(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", + ASN: 401486, + Org: "RAVNIX LLC", + Hosting: true, + CountryConfident: true, + ObservedAt: server.NowUtc(), + }) + connect.AssertEqual(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") + } + connect.AssertEqual(t, stored.CountryCode, "us") + connect.AssertEqual(t, stored.ASN, 401486) + connect.AssertEqual(t, stored.Hosting, true) + connect.AssertEqual(t, stored.CityConfident, false) + + // the resolved location must be the country-granular row, with no + // city/region association + loc := model.GetLocation(ctx, stored.LocationId) + if loc == nil { + t.Fatal("expected the resolved location row to exist") + } + connect.AssertEqual(t, loc.LocationType, model.LocationTypeCountry) + if loc.CityLocationId != (server.Id{}) { + t.Fatal("a country-granularity row must not have a city association") + } + if loc.RegionLocationId != (server.Id{}) { + t.Fatal("a country-granularity row must not have a region association") + } + }) +} + +func TestSubmitProviderEgressLocationRejectsNotCountryConfident(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: "us", + CountryConfident: false, + ObservedAt: server.NowUtc(), + }) + if err == nil { + t.Fatal("a submission that is not country-confident must be rejected") + } + if model.GetProviderEgressLocation(ctx, clientId) != nil { + t.Fatal("rejected submission must not be stored") + } + }) +} + +func TestSubmitProviderEgressLocationRejectsUnknownClient(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + _, err := SubmitProviderEgressLocation(ctx, &SubmitProviderEgressLocationArgs{ + ClientId: server.NewId(), // never created + CountryCode: "us", + CountryConfident: true, + ObservedAt: server.NowUtc(), + }) + if err == nil { + t.Fatal("unknown client_id must be rejected") + } + }) +} + +func TestSubmitProviderEgressLocationCityConfidentStoresCity(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, "", "") + + // the ingest path resolves against locations that ALREADY exist and + // never creates one, so the city has to be in the table first -- as it + // would be from the mmdb import + denver := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Denver", + Region: "Colorado", + Country: "United States", + CountryCode: "us", + } + model.CreateLocation(ctx, denver) + + _, err := SubmitProviderEgressLocation(ctx, &SubmitProviderEgressLocationArgs{ + ClientId: clientId, + CountryCode: "us", + Country: "United States", + Region: "Colorado", + City: "Denver", + CountryConfident: true, + CityConfident: true, + ObservedAt: server.NowUtc(), + }) + connect.AssertEqual(t, err, nil) + + stored := model.GetProviderEgressLocation(ctx, clientId) + if stored == nil { + t.Fatal("expected the submission to be stored") + } + connect.AssertEqual(t, stored.CityConfident, true) + + // the resolved location must be the city-granular row that already + // existed, not a new one + connect.AssertEqual(t, stored.LocationId, denver.LocationId) + loc := model.GetLocation(ctx, stored.LocationId) + if loc == nil { + t.Fatal("expected the resolved location row to exist") + } + connect.AssertEqual(t, loc.LocationType, model.LocationTypeCity) + }) +} + +// An empty Country must be rejected, not silently stored: model.CreateLocation +// dedupes country rows on (location_type, country_code), so an empty name +// would create a canonical, permanently-blank row that every later lookup for +// that country reuses forever. +func TestSubmitProviderEgressLocationRejectsEmptyCountry(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: "us", + Country: " ", // blank after trimming + CountryConfident: true, + ObservedAt: server.NowUtc(), + }) + if err == nil { + t.Fatal("an empty country must be rejected") + } + if model.GetProviderEgressLocation(ctx, clientId) != nil { + t.Fatal("rejected submission must not be stored") + } + }) +} + +// A city-confident submission with an empty City must be rejected rather than +// silently falling back to country granularity: the same empty-canonical-row +// corruption applies to city/region rows. +func TestSubmitProviderEgressLocationRejectsEmptyCityWhenCityConfident(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: "us", + Country: "United States", + Region: "Colorado", + City: "", + CountryConfident: true, + CityConfident: true, + ObservedAt: server.NowUtc(), + }) + if err == nil { + t.Fatal("a city-confident submission with an empty city must be rejected") + } + if model.GetProviderEgressLocation(ctx, clientId) != nil { + t.Fatal("rejected submission must not be stored") + } + }) +} + +// A city-confident submission with an empty Region must likewise be rejected: +// the region row is created with the same dedupe-on-empty-name hazard. +func TestSubmitProviderEgressLocationRejectsEmptyRegionWhenCityConfident(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: "us", + Country: "United States", + Region: "", + City: "Denver", + CountryConfident: true, + CityConfident: true, + ObservedAt: server.NowUtc(), + }) + if err == nil { + t.Fatal("a city-confident submission with an empty region must be rejected") + } + if model.GetProviderEgressLocation(ctx, clientId) != nil { + t.Fatal("rejected submission must not be stored") + } + }) +} + +// An over-long Country must be rejected with a clear error instead of +// panicking inside model.CreateLocation on a Postgres "value too long for +// type character varying(128)" error. +func TestSubmitProviderEgressLocationRejectsOverLongCountry(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: "us", + Country: strings.Repeat("a", 129), + CountryConfident: true, + ObservedAt: server.NowUtc(), + }) + if err == nil { + t.Fatal("an over-long country must be rejected") + } + if model.GetProviderEgressLocation(ctx, clientId) != nil { + t.Fatal("rejected submission must not be stored") + } + }) +} + +// An over-long Org must likewise be rejected rather than panicking inside +// storage. +func TestSubmitProviderEgressLocationRejectsOverLongOrg(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: "us", + Country: "United States", + Org: strings.Repeat("a", 257), + CountryConfident: true, + ObservedAt: server.NowUtc(), + }) + if err == nil { + t.Fatal("an over-long org must be rejected") + } + if model.GetProviderEgressLocation(ctx, clientId) != nil { + t.Fatal("rejected submission must not be stored") + } + }) +} + +func TestSubmitProviderEgressLocationRejectsStaleObservedAt(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: "us", + CountryConfident: true, + ObservedAt: server.NowUtc().Add(-30 * 24 * time.Hour), + }) + if err == nil { + t.Fatal("a submission observed long ago must be rejected") + } + }) +} + +// 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), + }) + connect.AssertEqual(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") + } + }) +} + +// A large 32-bit ASN (e.g. from the private-use range, common on +// hosting/VPN infrastructure) must round-trip cleanly, not panic. The asn +// column used to be `int` (Postgres int4, max ~2.147e9); ASNs are 32-bit +// unsigned (max ~4.295e9), so a value above int4's range panicked deep in +// pgx's arg encoding. +func TestSubmitProviderEgressLocationAcceptsLargeAsn(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, "", "") + + const largeAsn = 4200000000 + + res, err := SubmitProviderEgressLocation(ctx, &SubmitProviderEgressLocationArgs{ + ClientId: clientId, + CountryCode: "us", + Country: "United States", + ASN: largeAsn, + CountryConfident: true, + ObservedAt: server.NowUtc(), + }) + connect.AssertEqual(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") + } + connect.AssertEqual(t, stored.ASN, largeAsn) + }) +} + +// testing_countLocations is the whole point of the two tests below: the +// `location` table is shared with the provider list and the location search, +// its rows are permanent, and nothing cleans up a bad one. An ingest endpoint +// that can add to it is an endpoint that can corrupt it from outside. +func testing_countLocations(ctx context.Context) int64 { + var count int64 + server.Db(ctx, func(conn server.PgConn) { + result, err := conn.Query(ctx, `SELECT COUNT(*) FROM location`) + server.WithPgResult(result, err, func() { + if result.Next() { + server.Raise(result.Scan(&count)) + } + }) + }) + return count +} + +// A city-confident submission whose city is not already in the location table +// must fall back to country granularity and must NOT create a location row. +// +// model.CreateLocation dedupes a city on its exact location_name, so before +// this fix an unrecognised spelling did not fail -- it silently inserted a new +// permanent row and indexed it for search. The prober's consensus stores the +// winning source's original display string and the three free geolocation +// sources demonstrably disagree on spelling, so "Frankfurt am Main", +// "Frankfurt Am Main" and "Frankfurt/Main" would each have become their own +// row. Those rows outlive a code revert and there is no cleanup path. +// +// Reverting the MatchExistingLocation call in SubmitProviderEgressLocation must +// fail this test: the row count goes up and the stored location is a city. +func TestSubmitProviderEgressLocationUnknownCityDoesNotCreateALocation(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, "", "") + + // Germany, and one real German city, already exist -- as they would + // from the mmdb import. The submission below names a DIFFERENT city + // that has never been seen. + model.CreateLocation(ctx, &model.Location{ + LocationType: model.LocationTypeCity, + City: "Frankfurt am Main", + Region: "Hesse", + Country: "Germany", + CountryCode: "de", + }) + + before := testing_countLocations(ctx) + + res, err := SubmitProviderEgressLocation(ctx, &SubmitProviderEgressLocationArgs{ + ClientId: clientId, + CountryCode: "de", + Country: "Germany", + Region: "Hesse", + City: "Kleinstadt Nirgendwo", + CountryConfident: true, + CityConfident: true, + ObservedAt: server.NowUtc(), + }) + connect.AssertEqual(t, err, nil) + + // nothing was added to the shared table + after := testing_countLocations(ctx) + if after != before { + t.Errorf("location row count went from %d to %d; an unmatched city must not create a permanent row in the shared location table", before, after) + } + + // and the submission was stored at country granularity instead + stored := model.GetProviderEgressLocation(ctx, clientId) + if stored == nil { + t.Fatal("expected the submission to be stored") + } + loc := model.GetLocation(ctx, stored.LocationId) + if loc == nil { + t.Fatal("expected the resolved location row to exist") + } + if loc.LocationType != model.LocationTypeCountry { + t.Errorf("stored location_type = %q, want %q: an unmatched city must fall back to country granularity", loc.LocationType, model.LocationTypeCountry) + } + connect.AssertEqual(t, res.LocationId, stored.LocationId) + + // city_confident tracks the granularity actually stored, so the row + // stays internally consistent: location_id is a city row exactly when + // city_confident is set + if stored.CityConfident { + t.Error("city_confident must be false when the submission was stored at country granularity") + } + }) +} + +// The variants that matter are the ones the geolocation sources actually +// produce for one place: different case, punctuation or spacing, and -- the +// case this feature was built for -- a parenthesised district qualifier +// ("Frankfurt am Main (Innenstadt I)"). Those must resolve to the row that is +// already there; discarding them to country throws away real precision, and +// creating a row for each is the bug this guards against. +// +// An earlier revision of this list only contained foldings the implementation +// already handled, so it could not fail. The qualifier case below fails against +// a matcher that only drops punctuation, which is what makes this test worth +// running. Diacritics -- the other class that failed -- have their own test. +func TestSubmitProviderEgressLocationMatchesCitySpellingVariant(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + networkId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), server.NewId(), "", "") + + frankfurt := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Frankfurt am Main", + Region: "Hesse", + Country: "Germany", + CountryCode: "de", + } + model.CreateLocation(ctx, frankfurt) + + before := testing_countLocations(ctx) + + // each of these is a real disagreement between the geolocation sources + // over the same place + variants := []struct{ region, city string }{ + {"Hesse", "Frankfurt am Main"}, // exact + {"Hesse", "Frankfurt Am Main"}, // case + {"hesse", "FRANKFURT AM MAIN"}, // case, both levels + {"Hesse", "Frankfurt-am-Main"}, // punctuation + {"Hesse", " Frankfurt am Main "}, + // the observed disagreement that motivated matching at all: one + // source appends the district. Dropping "(" and ")" as punctuation + // is not enough -- the qualifier's letters stay in the key -- so + // this misses unless the qualifier itself is stripped. + {"Hesse", "Frankfurt am Main (Innenstadt I)"}, + {"Hesse", "Frankfurt am Main (Innenstadt I) "}, + // qualifier on the region too + {"Hesse (Regierungsbezirk Darmstadt)", "Frankfurt am Main"}, + } + for _, variant := range variants { + label := fmt.Sprintf("%q/%q", variant.region, variant.city) + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + _, err := SubmitProviderEgressLocation(ctx, &SubmitProviderEgressLocationArgs{ + ClientId: clientId, + CountryCode: "DE", + Country: "Germany", + Region: variant.region, + City: variant.city, + CountryConfident: true, + CityConfident: true, + ObservedAt: server.NowUtc(), + }) + connect.AssertEqual(t, err, nil) + + stored := model.GetProviderEgressLocation(ctx, clientId) + if stored == nil { + t.Fatalf("%s: expected the submission to be stored", label) + } + if stored.LocationId != frankfurt.LocationId { + t.Errorf("%s resolved to %s, want the existing Frankfurt row %s", label, stored.LocationId, frankfurt.LocationId) + } + if !stored.CityConfident { + t.Errorf("%s: city_confident must stay set when the city resolved", label) + } + } + + if after := testing_countLocations(ctx); after != before { + t.Errorf("location row count went from %d to %d; spelling variants must reuse the existing row, not add new ones", before, after) + } + }) +} + +// Diacritics are the largest single class of spelling disagreement between the +// geolocation sources: one emits the local spelling ("São Paulo", "Zürich", +// "Kraków"), another an ASCII transliteration ("Sao Paulo", "Zurich", +// "Krakow"), and the mmdb import that seeded the existing rows picked one of +// the two per city with no way to know which. Every one of these missed before +// the NFD fold and fell back to country -- and, per +// TestSetConnectionLocationProbedCountryDoesNotCoarsenMmdbCity, a fallback on a +// provider the mmdb already placed in a city used to be an outright +// discoverability regression. +// +// Both directions are exercised, because which side carries the accent depends +// on how the existing row happened to be seeded. +func TestSubmitProviderEgressLocationMatchesAccentedCityVariant(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + networkId := server.NewId() + + // existing rows, seeded the way the mmdb import would have: some + // accented, some already transliterated + saoPaulo := &model.Location{ + LocationType: model.LocationTypeCity, + City: "São Paulo", + Region: "São Paulo", + Country: "Brazil", + CountryCode: "br", + } + model.CreateLocation(ctx, saoPaulo) + + zurich := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Zürich", + Region: "Zürich", + Country: "Switzerland", + CountryCode: "ch", + } + model.CreateLocation(ctx, zurich) + + // seeded WITHOUT the accent, so the probe is the side carrying it + krakow := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Krakow", + Region: "Lesser Poland", + Country: "Poland", + CountryCode: "pl", + } + model.CreateLocation(ctx, krakow) + + before := testing_countLocations(ctx) + + variants := []struct { + countryCode string + country string + region string + city string + want *model.Location + }{ + // accent dropped by the probing source + {"BR", "Brazil", "Sao Paulo", "Sao Paulo", saoPaulo}, + {"CH", "Switzerland", "Zurich", "Zurich", zurich}, + // accent present in the probe, absent from the stored row + {"PL", "Poland", "Lesser Poland", "Kraków", krakow}, + // accent on one level only + {"BR", "Brazil", "São Paulo", "Sao Paulo", saoPaulo}, + // accent plus the other foldings at once + {"CH", "Switzerland", "zurich", " ZURICH ", zurich}, + } + for _, variant := range variants { + label := fmt.Sprintf("%q/%q", variant.region, variant.city) + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + _, err := SubmitProviderEgressLocation(ctx, &SubmitProviderEgressLocationArgs{ + ClientId: clientId, + CountryCode: variant.countryCode, + Country: variant.country, + Region: variant.region, + City: variant.city, + CountryConfident: true, + CityConfident: true, + ObservedAt: server.NowUtc(), + }) + connect.AssertEqual(t, err, nil) + + stored := model.GetProviderEgressLocation(ctx, clientId) + if stored == nil { + t.Fatalf("%s: expected the submission to be stored", label) + } + if stored.LocationId != variant.want.LocationId { + t.Errorf("%s resolved to %s, want the existing row %s", label, stored.LocationId, variant.want.LocationId) + } + if !stored.CityConfident { + t.Errorf("%s: city_confident must stay set when the city resolved", label) + } + } + + if after := testing_countLocations(ctx); after != before { + t.Errorf("location row count went from %d to %d; accented variants must reuse the existing row, not add new ones", before, after) + } + }) +} + +// The qualifier-stripping pass is the only one that can pick the wrong row, so +// it must decline rather than guess. Two same-region rows that differ ONLY in +// their parenthesised qualifier both reduce to the same key; a probe carrying a +// third qualifier matches neither exactly and must fall back to country instead +// of silently landing on whichever row the candidate ordering put first. +func TestSubmitProviderEgressLocationAmbiguousQualifierFallsBackToCountry(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + springfieldIl := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Springfield (IL)", + Region: "Midwest", + Country: "United States", + CountryCode: "us", + } + model.CreateLocation(ctx, springfieldIl) + + springfieldMa := &model.Location{ + LocationType: model.LocationTypeCity, + City: "Springfield (MA)", + Region: "Midwest", + Country: "United States", + CountryCode: "us", + } + model.CreateLocation(ctx, springfieldMa) + + before := testing_countLocations(ctx) + + networkId := server.NewId() + clientId := server.NewId() + model.Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + _, err := SubmitProviderEgressLocation(ctx, &SubmitProviderEgressLocationArgs{ + ClientId: clientId, + CountryCode: "US", + Country: "United States", + Region: "Midwest", + City: "Springfield (OH)", + CountryConfident: true, + CityConfident: true, + ObservedAt: server.NowUtc(), + }) + connect.AssertEqual(t, err, nil) + + stored := model.GetProviderEgressLocation(ctx, clientId) + if stored == nil { + t.Fatal("expected the submission to be stored") + } + if stored.CityConfident { + t.Errorf("an ambiguous qualifier must not resolve to a city; got city_confident with location %s", stored.LocationId) + } + if stored.LocationId == springfieldIl.LocationId || stored.LocationId == springfieldMa.LocationId { + t.Errorf("guessed a Springfield (%s) instead of falling back to country", stored.LocationId) + } + // the country fallback goes through CreateLocation, but the US country + // row already exists (the two Springfield fixtures created it), and + // CreateLocation dedupes a country on country_code -- so declining to + // guess must add no rows at all, least of all a third Springfield + if after := testing_countLocations(ctx); after != before { + t.Errorf("location row count went from %d to %d; the fallback must reuse the existing country row and never create a city", before, after) + } + }) +} diff --git a/db_migrations.go b/db_migrations.go index 063f091e..09d920f9 100644 --- a/db_migrations.go +++ b/db_migrations.go @@ -4385,4 +4385,96 @@ var migrations = []any{ CREATE INDEX IF NOT EXISTS network_client_top_level_contract_time ON network_client (contract_time) WHERE (active = true AND source_client_id IS NULL AND contract_time IS NOT NULL) `), + + // provider egress locations: locations learned by an operator-run prober + // that routes geolocation lookups through a provider's own egress rather + // than trusting a lookup on the provider's control-connection ip, since + // the egress is where user traffic actually exits and can differ from + // where the provider's control connection originates (e.g. behind a VPN + // or hosting network). Keyed by client_id, one row per provider, upserted + // by the operator's prober. location_id is the canonical country (or + // city, when the probe was city-confident) location row. observed_at is + // when the probe ran, and is what freshness is judged against. + newSqlMigration(` + CREATE TABLE IF NOT EXISTS provider_egress_location ( + client_id uuid NOT NULL PRIMARY KEY, + location_id uuid NOT NULL, + country_code varchar(2) NOT NULL, + asn bigint NOT NULL DEFAULT 0, + org varchar(256) NOT NULL DEFAULT '', + hosting bool NOT NULL DEFAULT false, + proxy bool NOT NULL DEFAULT false, + mobile bool NOT NULL DEFAULT false, + city_confident bool NOT NULL DEFAULT false, + observed_at timestamp NOT NULL, + update_time timestamp NOT NULL + ) + `), + newSqlMigration(` + CREATE INDEX IF NOT EXISTS provider_egress_location_observed_at + ON provider_egress_location (observed_at) + `), + + // provider egress probe attempts: when the prober last *tried* a provider, + // successful or not, and how the try failed. + // + // This cannot live on provider_egress_location, because the case it exists + // to handle is precisely a provider that has no row there. A provider that + // connects, holds a Public provide key and fails every probe (firewalled + // egress, dead upstream) never gets an egress row, so its observed_at stays + // NULL, so it sorts to the head of the due queue forever. Enough of them and + // every batch the prober asks for is the same set of permanently-dead + // providers, and no healthy provider's location is ever refreshed -- while + // the endpoint keeps returning a full, plausible-looking batch. + // GetProviderEgressLocationDue defers on a recent attempt as well as a fresh + // success, which needs somewhere to record the attempt. + // + // Pulled forward from the P2 verdict model + // (docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md, + // probe_attempt_at / probe_failure) because the P1 schedule cannot function + // without it. Deliberately only the two columns the schedule reads, not the + // rest of that model. + newSqlMigration(` + CREATE TABLE IF NOT EXISTS provider_egress_probe_attempt ( + client_id uuid NOT NULL PRIMARY KEY, + attempt_at timestamp NOT NULL, + probe_failure varchar(64) NOT NULL DEFAULT '', + update_time timestamp NOT NULL + ) + `), + + // serves the sweep in RemoveExpiredProviderEgressProbeAttempts. The due + // query reaches this table by primary key through the left join, so it + // needs no index of its own. + newSqlMigration(` + CREATE INDEX IF NOT EXISTS provider_egress_probe_attempt_attempt_at + ON provider_egress_probe_attempt (attempt_at) + `), + + // serves the stale-but-probed pass of GetProviderEgressLocationDue, which + // drives from provider_egress_location with `observed_at < $n ORDER BY + // observed_at, client_id LIMIT $m`. With client_id in the index the + // predicate and the whole ORDER BY -- tie-break included -- are one ordered + // index scan that stops when the batch is full: no sort, and no heap visit + // to resolve the tie. The pre-existing (observed_at) index alone leaves the + // client_id tie-break to a sort. + // + // The other pass (never-probed) needs no new index: it is an anti-join over + // network_client_location_reliability ordered by client_id, which the + // existing (valid, connected, client_id) index already serves as an ordered + // scan, and both anti-joins plus the provide_key EXISTS are primary-key + // probes. + // + // This supersedes provider_egress_location_observed_at, which is now a + // prefix of it -- including for the RemoveExpiredProviderEgressLocations + // sweep. The redundant index is left in place deliberately: dropping it is a + // separate decision with its own (small) risk, and this migration is meant + // to be purely additive. + // + // Appended, never inserted: migrations here apply by slice index, so + // editing or reordering an already-applied entry corrupts live databases. + newSqlMigration(` + CREATE INDEX IF NOT EXISTS provider_egress_location_observed_at_client_id + ON provider_egress_location (observed_at, client_id) + `), } diff --git a/go.mod b/go.mod index e4bddacf..284749b2 100644 --- a/go.mod +++ b/go.mod @@ -41,6 +41,7 @@ require ( golang.org/x/crypto v0.53.0 golang.org/x/net v0.56.0 golang.org/x/sys v0.46.0 + golang.org/x/text v0.40.0 golang.zx2c4.com/wireguard/wgctrl v0.0.0-20241231184526-a9ab2273dd10 google.golang.org/protobuf v1.36.11 gopkg.in/yaml.v3 v3.0.1 @@ -135,7 +136,6 @@ require ( golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/term v0.44.0 // indirect - golang.org/x/text v0.40.0 // indirect golang.org/x/time v0.15.0 // indirect gopkg.in/ini.v1 v1.67.2 // indirect src.agwa.name/tlshacks v0.0.3 // indirect diff --git a/model/network_client_location_model.go b/model/network_client_location_model.go index a2650fc4..811cc80e 100644 --- a/model/network_client_location_model.go +++ b/model/network_client_location_model.go @@ -1242,6 +1242,36 @@ func SetConnectionLocation( } }) + // geo databases vary in how deep their coverage goes: many IPs -- + // datacenter, mobile, and VPN egress especially -- only resolve to + // country or region granularity, with no city. network_client_location + // requires city_location_id and region_location_id NOT NULL, so a + // country-only location's NULL city/region made this INSERT panic + // inside server.Tx. That panic propagated out of the connection + // announce goroutine (connect/transport_announce.go), whose + // HandleError wrapper then cancelled the whole connection context -- + // tearing down every country-only client's connect connection right + // after auth (the app itself included, whichever egress it resolved + // to), and, because the panic hit before the disconnect-cleanup + // defer was registered, orphaning the connection row as + // connected=true forever. Fall back to the coarsest available + // granularity so the columns are always non-null: a country-only + // location stores its country id for city/region too, which keeps + // the provider locatable at country level instead of crashing the + // connection. If even the country id is missing (location row absent + // or malformed), return a clean error so the caller's existing + // graceful retry path handles it -- never panic here. + if countryLocationId == nil { + returnErr = fmt.Errorf("Location %s has no country granularity.", locationId) + return + } + if cityLocationId == nil { + cityLocationId = countryLocationId + } + if regionLocationId == nil { + regionLocationId = countryLocationId + } + server.RaisePgResult(tx.Exec( ctx, ` diff --git a/model/network_client_location_model_test.go b/model/network_client_location_model_test.go index ab91c7ee..888a1ce2 100644 --- a/model/network_client_location_model_test.go +++ b/model/network_client_location_model_test.go @@ -1198,3 +1198,62 @@ func TestFindProviders2ReliabilityDeployGap(t *testing.T) { connect.AssertEqual(t, len(res.Providers), n) }) } + +// SetConnectionLocation must not panic when the resolved location has no city +// (country-only). network_client_location requires city_location_id and +// region_location_id NOT NULL, but a country-granularity location row has them +// NULL, so the insert panicked inside server.Tx. That panic propagated out of +// the connection announce goroutine, whose HandleError wrapper cancelled the +// connection context -- tearing down the client's connection right after auth, +// and (because the panic hit before the disconnect-cleanup defer was +// registered) orphaning the connection row as connected=true. This asserts a +// country-only location is stored, falling back to country granularity, with +// no panic and no error. +func TestSetConnectionLocationToleratesCountryOnlyLocation(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + // country-only location -- its row has NULL city_location_id and + // NULL region_location_id, exactly what crashed the insert before + country := &Location{ + LocationType: LocationTypeCountry, + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, country) + + networkId := server.NewId() + clientId := server.NewId() + Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + handlerId := CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := ConnectNetworkClient(ctx, clientId, "0.0.0.1:0", handlerId) + connect.AssertEqual(t, err, nil) + + // this call panicked before the fix; now it must succeed and store + // the connection at country granularity + err = SetConnectionLocation(ctx, connectionId, country.LocationId, &ConnectionLocationScores{}) + connect.AssertEqual(t, err, nil) + + var city, region, cty *server.Id + server.Db(ctx, func(conn server.PgConn) { + result, qerr := conn.Query( + ctx, + `SELECT city_location_id, region_location_id, country_location_id FROM network_client_location WHERE connection_id = $1`, + connectionId, + ) + server.WithPgResult(result, qerr, func() { + if result.Next() { + server.Raise(result.Scan(&city, ®ion, &cty)) + } + }) + }) + if city == nil || region == nil || cty == nil { + t.Fatal("expected all three location ids to be set (falling back to country), got a nil") + } + // city and region fall back to the country id for a country-only location + connect.AssertEqual(t, *city, country.CountryLocationId) + connect.AssertEqual(t, *region, country.CountryLocationId) + connect.AssertEqual(t, *cty, country.CountryLocationId) + }) +} diff --git a/model/provider_egress_location_model.go b/model/provider_egress_location_model.go new file mode 100644 index 00000000..c0ad7780 --- /dev/null +++ b/model/provider_egress_location_model.go @@ -0,0 +1,937 @@ +package model + +import ( + "context" + "strings" + "time" + "unicode" + + "golang.org/x/text/unicode/norm" + + "github.com/urnetwork/server" +) + +// ProviderEgressLocationMaxAge bounds how long a probed egress location is +// trusted. Past this, the location is ignored and the caller falls back to the +// mmdb lookup on the observed control ip. +const ProviderEgressLocationMaxAge = 7 * 24 * time.Hour + +// ProviderEgressProbeAttemptBackoff is how long a probe *attempt* defers a +// provider from being offered up again, whether or not the attempt succeeded. +// +// It is much shorter than the staleness window a successful probe buys +// (providerEgressDueAge in api/handlers, half ProviderEgressLocationMaxAge): a +// provider that fails to probe should be retried periodically -- the fault may +// be transient -- just not on every single poll, which is what starves the rest +// of the queue. +const ProviderEgressProbeAttemptBackoff = 6 * time.Hour + +// ProviderEgressLocation is a provider location learned by probing the +// provider's own egress, rather than by looking up its control-connection ip. +type ProviderEgressLocation struct { + ClientId server.Id + LocationId server.Id + CountryCode string + ASN int + Org string + Hosting bool + Proxy bool + Mobile bool + CityConfident bool + ObservedAt time.Time + UpdateTime time.Time +} + +// SetProviderEgressLocation upserts the probed location for a provider. The +// upsert is monotonic in observed_at: a replayed or out-of-order submission +// older than what is already stored is silently dropped rather than +// clobbering a newer probe result. +func SetProviderEgressLocation(ctx context.Context, e *ProviderEgressLocation) { + // country codes are stored/compared lowercased (see CreateLocation in + // network_client_location_model.go); the geolocation APIs that feed this + // return uppercase codes (e.g. "US"), so normalize before writing. + countryCode := strings.ToLower(e.CountryCode) + + server.Tx(ctx, func(tx server.PgTx) { + server.RaisePgResult(tx.Exec( + ctx, + ` + INSERT INTO provider_egress_location ( + client_id, + location_id, + country_code, + asn, + org, + hosting, + proxy, + mobile, + city_confident, + observed_at, + update_time + ) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) + ON CONFLICT (client_id) DO UPDATE + SET + location_id = $2, + country_code = $3, + asn = $4, + org = $5, + hosting = $6, + proxy = $7, + mobile = $8, + city_confident = $9, + observed_at = $10, + update_time = $11 + WHERE provider_egress_location.observed_at < EXCLUDED.observed_at + `, + e.ClientId, + e.LocationId, + countryCode, + e.ASN, + e.Org, + e.Hosting, + e.Proxy, + e.Mobile, + e.CityConfident, + e.ObservedAt.UTC(), + server.NowUtc(), + )) + }) +} + +// ProviderEgressProbeAttempt records that the prober tried a provider, whether +// or not the try produced a location. +// +// A provider that has never been probed successfully has no +// ProviderEgressLocation row at all, so an attempt cannot be recorded there -- +// see the provider_egress_probe_attempt migration for why that matters. +// ProbeFailure is "" for a successful attempt, otherwise a short failure class +// (`tunnel_failed`, `no_consensus`, ...). +type ProviderEgressProbeAttempt struct { + ClientId server.Id + AttemptAt time.Time + ProbeFailure string + UpdateTime time.Time +} + +// SetProviderEgressProbeAttempt upserts the last probe attempt for a provider. +// +// Like SetProviderEgressLocation the upsert is monotonic in its timestamp: a +// replayed or out-of-order report older than what is already stored is dropped +// rather than moving the provider's last-attempt time backwards, which would +// hand it back to the prober early. +func SetProviderEgressProbeAttempt(ctx context.Context, a *ProviderEgressProbeAttempt) { + server.Tx(ctx, func(tx server.PgTx) { + server.RaisePgResult(tx.Exec( + ctx, + ` + INSERT INTO provider_egress_probe_attempt ( + client_id, + attempt_at, + probe_failure, + update_time + ) + VALUES ($1, $2, $3, $4) + ON CONFLICT (client_id) DO UPDATE + SET + attempt_at = $2, + probe_failure = $3, + update_time = $4 + WHERE provider_egress_probe_attempt.attempt_at < EXCLUDED.attempt_at + `, + a.ClientId, + a.AttemptAt.UTC(), + a.ProbeFailure, + server.NowUtc(), + )) + }) +} + +// GetProviderEgressProbeAttempt returns the last recorded probe attempt for a +// provider, or nil. +func GetProviderEgressProbeAttempt(ctx context.Context, clientId server.Id) *ProviderEgressProbeAttempt { + var a *ProviderEgressProbeAttempt + server.Db(ctx, func(conn server.PgConn) { + result, err := conn.Query( + ctx, + ` + SELECT + client_id, + attempt_at, + probe_failure, + update_time + FROM provider_egress_probe_attempt + WHERE client_id = $1 + `, + clientId, + ) + server.WithPgResult(result, err, func() { + if result.Next() { + a = &ProviderEgressProbeAttempt{} + server.Raise(result.Scan( + &a.ClientId, + &a.AttemptAt, + &a.ProbeFailure, + &a.UpdateTime, + )) + } + }) + }) + return a +} + +// GetProviderEgressLocation returns the stored location for a provider, or nil. +func GetProviderEgressLocation(ctx context.Context, clientId server.Id) *ProviderEgressLocation { + var e *ProviderEgressLocation + server.Db(ctx, func(conn server.PgConn) { + result, err := conn.Query( + ctx, + ` + SELECT + client_id, + location_id, + country_code, + asn, + org, + hosting, + proxy, + mobile, + city_confident, + observed_at, + update_time + FROM provider_egress_location + WHERE client_id = $1 + `, + clientId, + ) + server.WithPgResult(result, err, func() { + if result.Next() { + e = &ProviderEgressLocation{} + server.Raise(result.Scan( + &e.ClientId, + &e.LocationId, + &e.CountryCode, + &e.ASN, + &e.Org, + &e.Hosting, + &e.Proxy, + &e.Mobile, + &e.CityConfident, + &e.ObservedAt, + &e.UpdateTime, + )) + } + }) + }) + return e +} + +// GetFreshProviderEgressLocation is GetProviderEgressLocation, filtered to +// entries probed within maxAge. The cutoff is computed in Go and bound as a +// parameter: observed_at is a naive timestamp holding utc, and comparing it +// against sql now() would cast through the session timezone. +func GetFreshProviderEgressLocation( + ctx context.Context, + clientId server.Id, + maxAge time.Duration, +) *ProviderEgressLocation { + e := GetProviderEgressLocation(ctx, clientId) + if e == nil { + return nil + } + if e.ObservedAt.Before(server.NowUtc().Add(-maxAge)) { + return nil + } + return e +} + +// GetFreshProviderEgressLocationForConnection resolves the probed provider +// egress location for a connection in a single query, joining +// network_client_connection to provider_egress_location on client_id. This +// exists for the connect-announce hot path (SetConnectionLocation), which +// previously spent two round trips per connection -- resolving the client id +// for the connection, then fetching its fresh egress location -- before ever +// reaching the mmdb fallback; collapsing to one query matters on a path that +// runs for every connection and inside a retry loop. +// +// As with GetFreshProviderEgressLocation, the maxAge cutoff is computed in Go +// and compared in Go: observed_at is a naive timestamp holding utc, and +// comparing it against sql now() would cast through the session timezone. +func GetFreshProviderEgressLocationForConnection( + ctx context.Context, + connectionId server.Id, + maxAge time.Duration, +) *ProviderEgressLocation { + var e *ProviderEgressLocation + server.Db(ctx, func(conn server.PgConn) { + result, err := conn.Query( + ctx, + ` + SELECT + pel.client_id, + pel.location_id, + pel.country_code, + pel.asn, + pel.org, + pel.hosting, + pel.proxy, + pel.mobile, + pel.city_confident, + pel.observed_at, + pel.update_time + FROM network_client_connection ncc + INNER JOIN provider_egress_location pel ON pel.client_id = ncc.client_id + WHERE ncc.connection_id = $1 + `, + connectionId, + ) + server.WithPgResult(result, err, func() { + if result.Next() { + e = &ProviderEgressLocation{} + server.Raise(result.Scan( + &e.ClientId, + &e.LocationId, + &e.CountryCode, + &e.ASN, + &e.Org, + &e.Hosting, + &e.Proxy, + &e.Mobile, + &e.CityConfident, + &e.ObservedAt, + &e.UpdateTime, + )) + } + }) + }) + if e == nil { + return nil + } + if e.ObservedAt.Before(server.NowUtc().Add(-maxAge)) { + return nil + } + return e +} + +// GetLocation returns the canonical location row, or nil. +// +// Note: the location table also has a location_name column, but it holds the +// name for whichever granularity that specific row represents (e.g. a city +// row's own name), not a single name field on the Location struct. Location +// instead splits City/Region/Country by joining sibling rows (see +// IndexSearchLocationsInTx in network_client_location_model.go). This helper +// only needs to resolve identity/type, so it selects the columns that map +// directly onto Location's fields and leaves City/Region/Country empty. +func GetLocation(ctx context.Context, locationId server.Id) *Location { + var loc *Location + server.Db(ctx, func(conn server.PgConn) { + result, err := conn.Query( + ctx, + ` + SELECT location_id, location_type, city_location_id, region_location_id, country_location_id, country_code + FROM location + WHERE location_id = $1 + `, + locationId, + ) + server.WithPgResult(result, err, func() { + if result.Next() { + loc = &Location{} + // city_location_id/region_location_id are only set once the + // row's hierarchy reaches that granularity (e.g. a country + // row has both NULL); server.Id.Scan errors on a nil source, + // so scan through nullable pointers as in + // IndexSearchLocationsInTx (network_client_location_model.go). + var cityLocationId *server.Id + var regionLocationId *server.Id + var countryLocationId *server.Id + server.Raise(result.Scan( + &loc.LocationId, + &loc.LocationType, + &cityLocationId, + ®ionLocationId, + &countryLocationId, + &loc.CountryCode, + )) + if cityLocationId != nil { + loc.CityLocationId = *cityLocationId + } + if regionLocationId != nil { + loc.RegionLocationId = *regionLocationId + } + if countryLocationId != nil { + loc.CountryLocationId = *countryLocationId + } + } + }) + }) + return loc +} + +// normalizeLocationName folds a location name to a comparison key: lowercased, +// accent-stripped, with every rune that is not a letter or a digit dropped. So +// "Frankfurt am Main", "Frankfurt Am Main" and "FRANKFURT AM MAIN" all fold to +// "frankfurtammain", and "São Paulo", "Zürich" and "Kraków" fold to the same +// keys as "Sao Paulo", "Zurich" and "Krakow". +// +// This is deliberately a comparison key only -- it is never stored, and never +// used to build a location_name. It exists so a trivial spelling variant from a +// geolocation source resolves to the existing row instead of being treated as a +// different place. +// +// Diacritics are the single biggest source of these variants: the free +// geolocation sources disagree over whether to emit the local spelling or an +// ASCII transliteration for the same city, and the mmdb import that seeded most +// existing rows made its own choice. Folding is done with an NFD decomposition +// followed by dropping the combining marks (unicode.Mn), which covers the whole +// accent class at once -- a hand-rolled é->e table would have to enumerate the +// world's diacritics and would silently keep missing the ones it forgot. +// +// golang.org/x/text is already a dependency of this module, so this costs no +// new supply-chain surface. (An earlier revision of this function was +// stdlib-only by mistake: that constraint belongs to the prober repo's +// `geolocate` package, not to the server.) +// +// Letters that carry a stroke rather than a combining mark -- ł, ø, đ -- do not +// decompose and therefore still do not fold onto l/o/d. Those fall back to +// country granularity, which is the safe outcome; the alternative is the +// transliteration table this function deliberately avoids. +// +// Punctuation is dropped rather than mapped to a space because the disagreement +// is over whether the separator exists at all ("Washington, D.C." vs +// "Washington DC"). Note this deliberately does not fold "Frankfurt/Main" onto +// "Frankfurt am Main": dropping the separator gives "frankfurtmain" != +// "frankfurtammain", so that one falls back to country granularity rather than +// matching the wrong row. Falling back is the safe outcome; guessing is not. +func normalizeLocationName(name string) string { + // NFD splits a precomposed letter ("ü") into its base letter plus a + // combining mark ("u" + U+0308). The mark is category Mn, which is neither + // a letter nor a digit, so the filter below drops it; the explicit Mn skip + // is there to say so rather than to leave it to a category coincidence. + decomposed := norm.NFD.String(strings.ToLower(name)) + var b strings.Builder + b.Grow(len(decomposed)) + for _, r := range decomposed { + if unicode.Is(unicode.Mn, r) { + continue + } + if unicode.IsLetter(r) || unicode.IsDigit(r) { + b.WriteRune(r) + } + } + return b.String() +} + +// stripParentheticals removes every parenthesised span from name, so +// "Frankfurt am Main (Innenstadt I)" becomes "Frankfurt am Main ". Nesting is +// tracked, and an unclosed "(" swallows the rest of the string -- a qualifier +// that was truncated by a length limit is still a qualifier. +// +// This is only ever applied to a comparison key, never to anything stored. +func stripParentheticals(name string) string { + if !strings.ContainsRune(name, '(') { + return name + } + var b strings.Builder + b.Grow(len(name)) + depth := 0 + for _, r := range name { + switch r { + case '(': + depth += 1 + case ')': + if 0 < depth { + depth -= 1 + } + default: + if depth == 0 { + b.WriteRune(r) + } + } + } + return b.String() +} + +// matchLocationName returns the location_id of the row in `candidates` whose +// location_name matches `name`, or nil for no match. Candidates must already be +// ordered deterministically by the caller so that two rows folding to the same +// key always resolve the same way. +// +// Three passes, narrowest first: +// +// 1. exact string equality -- the common case, since the winning source usually +// spells it the way the mmdb import did; +// 2. the normalized fold (see normalizeLocationName): case, punctuation, +// whitespace and accents; +// 3. the normalized fold with parenthesised qualifiers stripped from BOTH +// sides. This is the case that motivated the whole feature: one source +// reports "Frankfurt am Main (Innenstadt I)" -- a district qualifier -- for +// a host another source calls "Frankfurt am Main". Pass 2 cannot see +// through that, because dropping the parentheses as punctuation leaves the +// qualifier's letters in the key. +// +// Pass 3 is the only pass that can plausibly match the wrong row -- two +// same-region rows "Springfield (IL)" and "Springfield (MA)" both reduce to +// "springfield" -- so it requires the stripped key to identify exactly ONE +// candidate and returns nil on any ambiguity. Falling back to country +// granularity is the safe outcome; guessing is not. +func matchLocationName(name string, candidateIds []server.Id, candidateNames []string) *server.Id { + for i, candidateName := range candidateNames { + if candidateName == name { + return &candidateIds[i] + } + } + normalized := normalizeLocationName(name) + if normalized == "" { + // nothing comparable survives folding (e.g. a name of only + // punctuation); an empty key would match any other such row + return nil + } + for i, candidateName := range candidateNames { + if normalizeLocationName(candidateName) == normalized { + return &candidateIds[i] + } + } + + base := normalizeLocationName(stripParentheticals(name)) + if base == "" { + // the name was nothing but a qualifier + return nil + } + var unique *server.Id + for i, candidateName := range candidateNames { + if normalizeLocationName(stripParentheticals(candidateName)) == base { + if unique != nil { + return nil + } + unique = &candidateIds[i] + } + } + return unique +} + +// MatchExistingLocation resolves (countryCode, region, city) against location +// rows that ALREADY EXIST and returns the city-granular row, or nil if any +// level of the hierarchy does not resolve. It never inserts anything. +// +// This is the resolver the provider egress ingest path uses instead of +// CreateLocation. CreateLocation deduplicates a city on its exact +// location_name, so an unrecognised spelling does not fail -- it silently +// creates a new, permanent row in the shared `location` table and indexes it +// for search. A geolocation probe has no business defining the world's cities: +// the three free sources the prober reaches consensus over demonstrably +// disagree on spelling (we observed "Frankfurt am Main (Innenstadt I)" against +// "Frankfurt am Main" for one host), and the consensus stores the winning +// source's original display string. Each variant would become its own row, +// those rows outlive a code revert, and there is no cleanup path. +// +// Matching is case-insensitive and ignores punctuation, whitespace, accents and +// parenthesised district qualifiers (see matchLocationName), so the ordinary +// variants resolve to the row that is already there -- including the +// "Frankfurt am Main (Innenstadt I)" case above, which is what this exists for. +// When nothing resolves the caller falls back to country granularity -- +// see SubmitProviderEgressLocation. Falling back loses precision for one +// submission; creating a row corrupts shared data permanently. +// +// Each level tries an exact, fully-indexed match first (the common case: the +// winning source usually spells it the way the mmdb import did) and only scans +// the level's candidates when that misses. +func MatchExistingLocation( + ctx context.Context, + countryCode string, + region string, + city string, +) *Location { + countryCode = strings.ToLower(strings.TrimSpace(countryCode)) + region = strings.TrimSpace(region) + city = strings.TrimSpace(city) + if countryCode == "" || region == "" || city == "" { + return nil + } + + var match *Location + server.Db(ctx, func(conn server.PgConn) { + // country: keyed on country_code alone, exactly as CreateLocation + // dedupes it, so there is no name to match here + var countryLocationId server.Id + var countryName string + found := false + result, err := conn.Query( + ctx, + ` + SELECT location_id, location_name + FROM location + WHERE location_type = $1 AND country_code = $2 + ORDER BY location_id + LIMIT 1 + `, + LocationTypeCountry, + countryCode, + ) + server.WithPgResult(result, err, func() { + if result.Next() { + server.Raise(result.Scan(&countryLocationId, &countryName)) + found = true + } + }) + if !found { + return + } + + // region, within that country + regionLocationId := matchChildLocation( + ctx, + conn, + LocationTypeRegion, + countryCode, + region, + ` + SELECT location_id, location_name + FROM location + WHERE + location_type = $1 AND + country_code = $2 AND + location_name = $3 AND + country_location_id = $4 + `, + ` + SELECT location_id, location_name + FROM location + WHERE + location_type = $1 AND + country_code = $2 AND + country_location_id = $3 + ORDER BY location_id + `, + []any{countryLocationId}, + ) + if regionLocationId == nil { + return + } + + // city, within that region + cityLocationId := matchChildLocation( + ctx, + conn, + LocationTypeCity, + countryCode, + city, + ` + SELECT location_id, location_name + FROM location + WHERE + location_type = $1 AND + country_code = $2 AND + location_name = $3 AND + region_location_id = $4 AND + country_location_id = $5 + `, + ` + SELECT location_id, location_name + FROM location + WHERE + location_type = $1 AND + country_code = $2 AND + region_location_id = $3 AND + country_location_id = $4 + ORDER BY location_id + `, + []any{*regionLocationId, countryLocationId}, + ) + if cityLocationId == nil { + return + } + + match = &Location{ + LocationType: LocationTypeCity, + City: city, + Region: region, + Country: countryName, + CountryCode: countryCode, + LocationId: *cityLocationId, + CityLocationId: *cityLocationId, + RegionLocationId: *regionLocationId, + CountryLocationId: countryLocationId, + } + }) + return match +} + +// matchChildLocation runs the exact-match query first and only falls back to +// scanning the level's candidates when it misses. `parents` are the parent +// location ids the two queries scope on: the exact query binds them after +// (location_type, country_code, name), the candidate query after +// (location_type, country_code). +func matchChildLocation( + ctx context.Context, + conn server.PgConn, + locationType LocationType, + countryCode string, + name string, + exactSql string, + candidatesSql string, + parents []any, +) *server.Id { + exactArgs := append([]any{locationType, countryCode, name}, parents...) + var exactId *server.Id + result, err := conn.Query(ctx, exactSql, exactArgs...) + server.WithPgResult(result, err, func() { + if result.Next() { + var locationId server.Id + var locationName string + server.Raise(result.Scan(&locationId, &locationName)) + exactId = &locationId + } + }) + if exactId != nil { + return exactId + } + + candidateArgs := append([]any{locationType, countryCode}, parents...) + candidateIds := []server.Id{} + candidateNames := []string{} + result, err = conn.Query(ctx, candidatesSql, candidateArgs...) + server.WithPgResult(result, err, func() { + for result.Next() { + var locationId server.Id + var locationName string + server.Raise(result.Scan(&locationId, &locationName)) + candidateIds = append(candidateIds, locationId) + candidateNames = append(candidateNames, locationName) + } + }) + return matchLocationName(name, candidateIds, candidateNames) +} + +// GetProviderEgressLocationDue returns the client ids of providers whose +// egress location is due for a probe: no fresh success (newest probe older than +// minObservedAt, or never probed) *and* no recent attempt (last attempt older +// than minAttemptAt, or never attempted). Oldest first, so the +// longest-unprobed are handed out first, capped at limit. +// +// This is the durable replacement for the prober's in-memory ttl cache: the +// schedule lives in the database, so a prober restart resumes where it left +// off instead of re-probing everything. +// +// Three things about the shape of this query matter. +// +// First, candidates are sourced from the live provider population +// (network_client_location_reliability, connected + valid) and the egress row +// is LEFT JOINed on. The dominant case by far is a provider that has *never* +// been probed and therefore has no provider_egress_location row at all; +// selecting from provider_egress_location would return exactly the providers +// that least need probing and none of the ones that most do. +// +// Second, only providers holding a Public provide key are returned. Probing +// tunnels through the provider itself, which means opening a contract from +// outside the provider's own network -- something a provider without a Public +// key refuses. Offering one to the prober would burn a probe slot on a +// guaranteed failure. This is the same filter UpdateClientLocations and +// UpdateClientScores apply (network_client_location_model.go). +// +// Third, a recent *attempt* defers a provider the same way a recent success +// does. Without that, a provider that connects and holds a Public provide key +// but always fails to probe -- for any reason other than the missing Public key +// screened for above -- never gets an egress row, so its observed_at stays +// NULL, so it sorts ahead of every stale-but-refreshable provider on every +// single poll, forever. Enough such providers to fill a batch and no healthy +// provider is ever refreshed again, while this endpoint goes on returning a +// full, plausible-looking batch. The in-memory ttl cache this replaced was +// incidentally immune, because it marked a provider probed whether or not the +// probe worked; moving the schedule server-side dropped that protection, and +// provider_egress_probe_attempt is what restores it. +// +// Both cutoffs are computed by the caller in Go and bound as parameters: +// observed_at and attempt_at are naive `timestamp` columns holding utc, and +// comparing them against sql now() would cast through the session timezone and +// silently skip a window. +// +// # Two passes, not one +// +// Expressed as a single statement this is a scan of +// network_client_location_reliability with two LEFT JOINs, sorted on +// observed_at from an outer-joined table. That sort cannot use an index: the +// column being ordered on does not exist for most of the rows being ordered. +// At beta's 40 providers that is free. At 100k it is a full scan plus an +// unindexable sort, on every poll. +// +// The ordering makes the split possible. `NULLS FIRST` means every never-probed +// provider sorts ahead of every probed one, so the result is always the +// concatenation of two independently ordered groups: +// +// 1. never probed -- no provider_egress_location row at all. This is the +// dominant group (it is why the ordering is NULLS FIRST), and within it +// every observed_at is equally absent, so the order is client_id alone. As +// an anti-join with no outer-joined column in the ORDER BY it is an ordered +// index scan over (valid, connected, client_id) with a LIMIT: no sort, and +// it stops as soon as the batch is full. +// 2. stale but probed -- has a row, older than minObservedAt. Only reached +// when pass 1 came up short of the limit. Driven from +// provider_egress_location itself, where observed_at is a real, indexable +// column: an ordered range scan over (observed_at, client_id). +// +// Both passes carry the same eligibility predicates, so the concatenation is +// row-for-row what the single statement returned, in the same order, under the +// same limit. `attempt_at IS NULL OR attempt_at < $n` becomes the equivalent +// `NOT EXISTS (... AND $n <= attempt_at)` -- equivalent because client_id is the +// primary key of provider_egress_probe_attempt, so there is at most one row to +// quantify over. The same holds for `observed_at IS NULL` on +// provider_egress_location, whose client_id is likewise a primary key and whose +// observed_at is NOT NULL: the only way that test is true is that no row exists. +func GetProviderEgressLocationDue( + ctx context.Context, + minObservedAt time.Time, + minAttemptAt time.Time, + limit int, +) []server.Id { + clientIds := []server.Id{} + server.Db(ctx, func(conn server.PgConn) { + // pass 1: never probed. Ordered by client_id alone -- every row in this + // group has no observed_at, so the ORDER BY's leading key is constant + // across it and the tie-break is the whole ordering. + // + // `limit` is passed through as given rather than clamped, so a + // nonsensical limit fails exactly as the single-statement version did + // (LIMIT 0 returns nothing; a negative limit is an error). + result, err := conn.Query( + ctx, + ` + SELECT + network_client_location_reliability.client_id + FROM network_client_location_reliability + + WHERE + network_client_location_reliability.connected = true AND + network_client_location_reliability.valid = true AND + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = network_client_location_reliability.client_id AND + provide_key.provide_mode = $1 + ) AND + NOT EXISTS ( + SELECT 1 FROM provider_egress_location + WHERE + provider_egress_location.client_id = network_client_location_reliability.client_id + ) AND + NOT EXISTS ( + SELECT 1 FROM provider_egress_probe_attempt + WHERE + provider_egress_probe_attempt.client_id = network_client_location_reliability.client_id AND + $2 <= provider_egress_probe_attempt.attempt_at + ) + + ORDER BY network_client_location_reliability.client_id ASC + LIMIT $3 + `, + ProvideModePublic, + minAttemptAt.UTC(), + limit, + ) + server.WithPgResult(result, err, func() { + for result.Next() { + var clientId server.Id + server.Raise(result.Scan(&clientId)) + clientIds = append(clientIds, clientId) + } + }) + + remaining := limit - len(clientIds) + if remaining <= 0 { + // the batch is full from never-probed providers alone, which is the + // steady state until the population has been swept once. The + // single-statement version would have returned exactly these rows + // too: they all sort ahead of anything with an observed_at. + return + } + + // pass 2: stale but probed. Driven from provider_egress_location, so + // observed_at is a real column of the driving table and the ORDER BY is + // an ordered index scan rather than a sort. + result, err = conn.Query( + ctx, + ` + SELECT + provider_egress_location.client_id + FROM provider_egress_location + + INNER JOIN network_client_location_reliability ON + network_client_location_reliability.client_id = provider_egress_location.client_id + + WHERE + provider_egress_location.observed_at < $2 AND + network_client_location_reliability.connected = true AND + network_client_location_reliability.valid = true AND + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = provider_egress_location.client_id AND + provide_key.provide_mode = $1 + ) AND + NOT EXISTS ( + SELECT 1 FROM provider_egress_probe_attempt + WHERE + provider_egress_probe_attempt.client_id = provider_egress_location.client_id AND + $3 <= provider_egress_probe_attempt.attempt_at + ) + + -- oldest probe first, client_id breaking the tie, so batch + -- composition is deterministic instead of plan-dependent + ORDER BY + provider_egress_location.observed_at ASC, + provider_egress_location.client_id ASC + LIMIT $4 + `, + ProvideModePublic, + minObservedAt.UTC(), + minAttemptAt.UTC(), + remaining, + ) + server.WithPgResult(result, err, func() { + // the two passes are separate statements and so separate snapshots. + // A provider that gains its first provider_egress_location row + // between them would be never-probed to pass 1 and stale to pass 2; + // the single-statement version could not do that, so screen it out + // rather than hand the prober the same client twice. + seen := map[server.Id]bool{} + for _, clientId := range clientIds { + seen[clientId] = true + } + for result.Next() { + var clientId server.Id + server.Raise(result.Scan(&clientId)) + if seen[clientId] { + continue + } + clientIds = append(clientIds, clientId) + } + }) + }) + return clientIds +} + +// RemoveExpiredProviderEgressLocations drops entries probed before +// minObservedAt. +func RemoveExpiredProviderEgressLocations(ctx context.Context, minObservedAt time.Time) { + server.MaintenanceTx(ctx, func(tx server.PgTx) { + server.RaisePgResult(tx.Exec( + ctx, + `DELETE FROM provider_egress_location WHERE observed_at < $1`, + minObservedAt.UTC(), + )) + }) +} + +// RemoveExpiredProviderEgressProbeAttempts drops attempts older than +// minAttemptAt. An attempt only carries information for as long as it defers +// the provider (ProviderEgressProbeAttemptBackoff); past that the row is just +// storage held for a client id that may no longer exist. +func RemoveExpiredProviderEgressProbeAttempts(ctx context.Context, minAttemptAt time.Time) { + server.MaintenanceTx(ctx, func(tx server.PgTx) { + server.RaisePgResult(tx.Exec( + ctx, + `DELETE FROM provider_egress_probe_attempt WHERE attempt_at < $1`, + minAttemptAt.UTC(), + )) + }) +} diff --git a/model/provider_egress_location_model_test.go b/model/provider_egress_location_model_test.go new file mode 100644 index 00000000..f3acd4c3 --- /dev/null +++ b/model/provider_egress_location_model_test.go @@ -0,0 +1,666 @@ +package model + +import ( + "context" + "fmt" + "slices" + "testing" + "time" + + "github.com/urnetwork/connect" + "github.com/urnetwork/server" +) + +func TestProviderEgressLocationUpsertAndGet(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + country := &Location{ + LocationType: LocationTypeCountry, + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, country) + + clientId := server.NewId() + now := server.NowUtc() + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: clientId, + LocationId: country.LocationId, + CountryCode: "us", + ASN: 401486, + Org: "RAVNIX LLC", + Hosting: true, + ObservedAt: now, + }) + + got := GetProviderEgressLocation(ctx, clientId) + if got == nil { + t.Fatal("expected a stored egress location") + } + connect.AssertEqual(t, got.LocationId, country.LocationId) + connect.AssertEqual(t, got.CountryCode, "us") + connect.AssertEqual(t, got.ASN, 401486) + connect.AssertEqual(t, got.Hosting, true) + connect.AssertEqual(t, got.Proxy, false) + + // upsert replaces, given a strictly newer observed_at: the upsert is + // monotonic (see TestProviderEgressLocationUpsertIgnoresOlderReplay below), + // so a second submission at the same observed_at would not win. + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: clientId, + LocationId: country.LocationId, + CountryCode: "us", + ASN: 999, + Hosting: false, + Proxy: true, + ObservedAt: now.Add(time.Minute), + }) + got = GetProviderEgressLocation(ctx, clientId) + connect.AssertEqual(t, got.ASN, 999) + connect.AssertEqual(t, got.Hosting, false) + connect.AssertEqual(t, got.Proxy, true) + }) +} + +// The upsert is monotonic in observed_at: a replayed submission older than +// what is already stored must not clobber the newer row. +func TestProviderEgressLocationUpsertIgnoresOlderReplay(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + usCountry := &Location{ + LocationType: LocationTypeCountry, + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, usCountry) + + jpCountry := &Location{ + LocationType: LocationTypeCountry, + Country: "Japan", + CountryCode: "jp", + } + CreateLocation(ctx, jpCountry) + + clientId := server.NewId() + newer := server.NowUtc() + older := newer.Add(-1 * time.Hour) + + // the newer probe lands first + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: clientId, + LocationId: jpCountry.LocationId, + CountryCode: "jp", + ASN: 111, + ObservedAt: newer, + }) + + // a stale/replayed older probe arrives afterward and must not win + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: clientId, + LocationId: usCountry.LocationId, + CountryCode: "us", + ASN: 222, + ObservedAt: older, + }) + + got := GetProviderEgressLocation(ctx, clientId) + if got == nil { + t.Fatal("expected a stored egress location") + } + connect.AssertEqual(t, got.CountryCode, "jp") + connect.AssertEqual(t, got.ASN, 111) + connect.AssertEqual(t, got.LocationId, jpCountry.LocationId) + }) +} + +func TestProviderEgressLocationCountryCodeLowercased(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + country := &Location{ + LocationType: LocationTypeCountry, + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, country) + + clientId := server.NewId() + // geolocation APIs return uppercase codes (e.g. "US"); the model must + // normalize to lowercase before storing, matching CreateLocation's + // established invariant that country codes are stored/compared lowercased. + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: clientId, + LocationId: country.LocationId, + CountryCode: "US", + ASN: 12345, + Org: "TEST ORG", + ObservedAt: server.NowUtc(), + }) + + got := GetProviderEgressLocation(ctx, clientId) + if got == nil { + t.Fatal("expected a stored egress location") + } + connect.AssertEqual(t, got.CountryCode, "us") + }) +} + +func TestProviderEgressLocationFreshness(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + country := &Location{ + LocationType: LocationTypeCountry, + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, country) + + fresh := server.NewId() + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: fresh, LocationId: country.LocationId, CountryCode: "us", + ObservedAt: server.NowUtc(), + }) + stale := server.NewId() + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: stale, LocationId: country.LocationId, CountryCode: "us", + ObservedAt: server.NowUtc().Add(-8 * 24 * time.Hour), + }) + + if GetFreshProviderEgressLocation(ctx, fresh, ProviderEgressLocationMaxAge) == nil { + t.Fatal("fresh entry must be returned") + } + if GetFreshProviderEgressLocation(ctx, stale, ProviderEgressLocationMaxAge) != nil { + t.Fatal("stale entry must not be returned") + } + // absent + if GetFreshProviderEgressLocation(ctx, server.NewId(), ProviderEgressLocationMaxAge) != nil { + t.Fatal("absent entry must return nil") + } + }) +} + +func TestRemoveExpiredProviderEgressLocations(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + country := &Location{ + LocationType: LocationTypeCountry, + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, country) + + keep := server.NewId() + drop := server.NewId() + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: keep, LocationId: country.LocationId, CountryCode: "us", + ObservedAt: server.NowUtc(), + }) + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: drop, LocationId: country.LocationId, CountryCode: "us", + ObservedAt: server.NowUtc().Add(-30 * 24 * time.Hour), + }) + + RemoveExpiredProviderEgressLocations(ctx, server.NowUtc().Add(-14*24*time.Hour)) + + if GetProviderEgressLocation(ctx, keep) == nil { + t.Fatal("recent entry must survive the sweep") + } + if GetProviderEgressLocation(ctx, drop) != nil { + t.Fatal("old entry must be swept") + } + }) +} + +// testing_connectProbeableProvider stands up the minimum a client needs to +// look like a live provider to the due-selection query: a device, a live +// connection with a resolved location, and a provide key of the given mode. +// The caller must run UpdateClientLocationReliabilities afterward -- that is +// what rolls the live connection tables up into the +// network_client_location_reliability row (connected + valid) the query reads. +// It returns the connection id, so a caller can disconnect the provider again. +func testing_connectProbeableProvider( + t testing.TB, + ctx context.Context, + clientId server.Id, + locationId server.Id, + clientAddress string, + provideMode ProvideMode, +) server.Id { + Testing_CreateDevice(ctx, server.NewId(), server.NewId(), clientId, "", "") + + handlerId := CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := ConnectNetworkClient(ctx, clientId, clientAddress, handlerId) + if err != nil { + t.Fatalf("connect client: %s", err) + } + + if err := SetConnectionLocation(ctx, connectionId, locationId, &ConnectionLocationScores{}); err != nil { + t.Fatalf("set connection location: %s", err) + } + + SetProvide(ctx, clientId, map[ProvideMode][]byte{ + provideMode: []byte("provide-secret"), + }) + + return connectionId +} + +// The prober asks the server what to probe next. The answer must be sourced +// from the live provider population and not from provider_egress_location, +// because the dominant case -- a provider that has never been probed at all -- +// has no row there. +func TestGetProviderEgressLocationDue(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + now := server.NowUtc() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + fresh := server.NewId() + stale := server.NewId() + never := server.NewId() + // a provider that cannot serve a stranger is unprobeable: the tunnel + // contract would be refused, so it must never be offered to the prober + nonPublic := server.NewId() + + testing_connectProbeableProvider(t, ctx, fresh, city.LocationId, "0.0.0.1:0", ProvideModePublic) + testing_connectProbeableProvider(t, ctx, stale, city.LocationId, "0.0.0.2:0", ProvideModePublic) + testing_connectProbeableProvider(t, ctx, never, city.LocationId, "0.0.0.3:0", ProvideModePublic) + testing_connectProbeableProvider(t, ctx, nonPublic, city.LocationId, "0.0.0.4:0", ProvideModeNetwork) + + UpdateClientLocationReliabilities(ctx, now.Add(-time.Hour), now) + + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: fresh, LocationId: city.LocationId, + CountryCode: "us", ObservedAt: now.Add(-1 * time.Hour), + }) + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: stale, LocationId: city.LocationId, + CountryCode: "us", ObservedAt: now.Add(-72 * time.Hour), + }) + // `never` and `nonPublic` deliberately get no row at all + + // no attempt rows exist in this test, so the attempt cutoff never + // excludes anything; freshness is the only variable + due := GetProviderEgressLocationDue(ctx, now.Add(-24*time.Hour), now, 100) + + // a provider probed an hour ago must not be re-probed; one probed three + // days ago must be; one never probed must be + if slices.Contains(due, fresh) { + t.Fatalf("due = %v, must not contain the provider probed an hour ago (%s)", due, fresh) + } + if !slices.Contains(due, stale) { + t.Fatalf("due = %v, must contain the provider probed three days ago (%s)", due, stale) + } + if !slices.Contains(due, never) { + t.Fatalf("due = %v, must contain the never-probed provider (%s)", due, never) + } + // unprobeable regardless of freshness + if slices.Contains(due, nonPublic) { + t.Fatalf("due = %v, must not contain the provider without a Public provide key (%s)", due, nonPublic) + } + + // oldest first, so the longest-unprobed are probed first: the + // never-probed provider sorts ahead of the three-days-stale one + neverIndex := slices.Index(due, never) + staleIndex := slices.Index(due, stale) + if staleIndex < neverIndex { + t.Fatalf("never-probed provider at %d must sort before the stale one at %d", neverIndex, staleIndex) + } + + // limit is honoured + limited := GetProviderEgressLocationDue(ctx, now.Add(-24*time.Hour), now, 1) + if len(limited) != 1 { + t.Fatalf("len(due) = %d for limit 1, want 1", len(limited)) + } + if limited[0] != never { + t.Fatalf("due[0] = %s for limit 1, want the never-probed provider %s", limited[0], never) + } + }) +} + +// A provider that connects, holds a Public provide key and fails every probe +// never gets a provider_egress_location row, so its observed_at stays NULL, so +// it sorts ahead of every stale-but-refreshable provider -- forever, on every +// poll. Enough of them to fill a batch and no healthy provider's location is +// ever refreshed again, silently: the endpoint keeps returning a full, +// plausible-looking batch of the same dead providers. +// +// A recent attempt must therefore defer a provider exactly as a fresh success +// does. Deleting the attempt predicate from GetProviderEgressLocationDue must +// fail this test. +func TestGetProviderEgressLocationDueDefersRecentlyAttempted(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + now := server.NowUtc() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + // never probed successfully, and the prober just tried it and failed + dead := server.NewId() + // probed successfully three days ago, never attempted since: the + // provider that actually needs the next probe slot + healthyStale := server.NewId() + + testing_connectProbeableProvider(t, ctx, dead, city.LocationId, "0.0.0.1:0", ProvideModePublic) + testing_connectProbeableProvider(t, ctx, healthyStale, city.LocationId, "0.0.0.2:0", ProvideModePublic) + + UpdateClientLocationReliabilities(ctx, now.Add(-time.Hour), now) + + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: healthyStale, LocationId: city.LocationId, + CountryCode: "us", ObservedAt: now.Add(-72 * time.Hour), + }) + // `dead` deliberately gets no location row -- it has never succeeded -- + // only a failed attempt seconds ago + SetProviderEgressProbeAttempt(ctx, &ProviderEgressProbeAttempt{ + ClientId: dead, + AttemptAt: now.Add(-5 * time.Second), + ProbeFailure: "tunnel_failed", + }) + + minObservedAt := now.Add(-24 * time.Hour) + minAttemptAt := now.Add(-ProviderEgressProbeAttemptBackoff) + + due := GetProviderEgressLocationDue(ctx, minObservedAt, minAttemptAt, 100) + + if slices.Contains(due, dead) { + t.Fatalf("due = %v, must not contain the provider attempted seconds ago (%s)", due, dead) + } + if !slices.Contains(due, healthyStale) { + t.Fatalf("due = %v, must contain the stale-but-refreshable provider (%s)", due, healthyStale) + } + + // the starvation itself: with a batch big enough for exactly one + // provider, the slot must go to the one that can actually be refreshed, + // not to the never-probed one that just failed. Without the attempt + // predicate `dead` wins this on observed_at IS NULL every single poll. + limited := GetProviderEgressLocationDue(ctx, minObservedAt, minAttemptAt, 1) + if len(limited) != 1 { + t.Fatalf("len(due) = %d for limit 1, want 1", len(limited)) + } + if limited[0] != healthyStale { + t.Fatalf("due[0] = %s for limit 1, want the refreshable provider %s, not the just-failed one %s", limited[0], healthyStale, dead) + } + + // ... and the deferral is a backoff, not a ban: once the backoff has + // elapsed the same provider is offered again. The caller computes the + // cutoff as (wall clock - backoff), so a poll exactly one backoff period + // after the attempt computes `now`. + afterBackoff := GetProviderEgressLocationDue(ctx, minObservedAt, now, 100) + if !slices.Contains(afterBackoff, dead) { + t.Fatalf("due = %v, must contain the failed provider (%s) again once the attempt backoff has elapsed", afterBackoff, dead) + } + }) +} + +// Only live, routable providers are probeable. A provider that has gone offline +// (connected = false) or that looks messed up from a routing perspective +// (valid = false, a generated column: more than one address hash or location on +// its live connections) must not be handed to the prober. Deleting either +// predicate from GetProviderEgressLocationDue must fail this test. +func TestGetProviderEgressLocationDueRequiresConnectedAndValid(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + now := server.NowUtc() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + good := server.NewId() + disconnected := server.NewId() + invalid := server.NewId() + + testing_connectProbeableProvider(t, ctx, good, city.LocationId, "0.0.0.1:0", ProvideModePublic) + disconnectedConnectionId := testing_connectProbeableProvider(t, ctx, disconnected, city.LocationId, "0.0.0.2:0", ProvideModePublic) + + // `invalid` holds two simultaneous connections from two different + // addresses, which makes client_address_hash_count = 2 and so the + // generated `valid` column false. The two addresses must be in + // different /29s: server.ClientIpHash buckets ipv4 to the /29 network, + // so e.g. 0.0.0.3 and 0.0.0.4 would hash the same and count as one. + testing_connectProbeableProvider(t, ctx, invalid, city.LocationId, "0.0.0.3:0", ProvideModePublic) + secondHandlerId := CreateNetworkClientHandler(ctx) + secondConnectionId, _, _, _, err := ConnectNetworkClient(ctx, invalid, "0.0.8.3:0", secondHandlerId) + if err != nil { + t.Fatalf("connect second address: %s", err) + } + if err := SetConnectionLocation(ctx, secondConnectionId, city.LocationId, &ConnectionLocationScores{}); err != nil { + t.Fatalf("set second connection location: %s", err) + } + + // first roll-up: everything above is connected + UpdateClientLocationReliabilities(ctx, now.Add(-time.Hour), now) + + // `disconnected` drops off, and a second roll-up flips its reliability + // row's connected to false (the row itself survives) + if err := DisconnectNetworkClient(ctx, disconnectedConnectionId); err != nil { + t.Fatalf("disconnect client: %s", err) + } + UpdateClientLocationReliabilities(ctx, now.Add(-time.Hour), server.NowUtc()) + + due := GetProviderEgressLocationDue(ctx, now.Add(-24*time.Hour), now, 100) + + if !slices.Contains(due, good) { + t.Fatalf("due = %v, must contain the connected, valid provider (%s)", due, good) + } + if slices.Contains(due, disconnected) { + t.Fatalf("due = %v, must not contain the disconnected provider (%s)", due, disconnected) + } + if slices.Contains(due, invalid) { + t.Fatalf("due = %v, must not contain the provider whose reliability row is not valid (%s)", due, invalid) + } + }) +} + +// The attempt upsert is monotonic in attempt_at, for the same reason the +// location upsert is: a replayed or out-of-order report must not move the last +// attempt backwards and hand the provider back to the prober early. +func TestProviderEgressProbeAttemptUpsertIgnoresOlderReplay(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + clientId := server.NewId() + newer := server.NowUtc() + older := newer.Add(-time.Hour) + + SetProviderEgressProbeAttempt(ctx, &ProviderEgressProbeAttempt{ + ClientId: clientId, AttemptAt: newer, ProbeFailure: "no_consensus", + }) + SetProviderEgressProbeAttempt(ctx, &ProviderEgressProbeAttempt{ + ClientId: clientId, AttemptAt: older, ProbeFailure: "tunnel_failed", + }) + + got := GetProviderEgressProbeAttempt(ctx, clientId) + if got == nil { + t.Fatal("expected a stored probe attempt") + } + connect.AssertEqual(t, got.ProbeFailure, "no_consensus") + // postgres `timestamp` keeps microseconds, Go keeps nanoseconds, so + // compare with a tolerance rather than for equality + if delta := got.AttemptAt.Sub(newer); delta < -time.Millisecond || time.Millisecond < delta { + t.Fatalf("attempt_at = %s, want the newer attempt %s", got.AttemptAt, newer) + } + + // a strictly newer report does win + newest := newer.Add(time.Minute) + SetProviderEgressProbeAttempt(ctx, &ProviderEgressProbeAttempt{ + ClientId: clientId, AttemptAt: newest, ProbeFailure: "", + }) + got = GetProviderEgressProbeAttempt(ctx, clientId) + connect.AssertEqual(t, got.ProbeFailure, "") + + // absent + if GetProviderEgressProbeAttempt(ctx, server.NewId()) != nil { + t.Fatal("absent attempt must return nil") + } + }) +} + +func TestRemoveExpiredProviderEgressProbeAttempts(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + keep := server.NewId() + drop := server.NewId() + SetProviderEgressProbeAttempt(ctx, &ProviderEgressProbeAttempt{ + ClientId: keep, AttemptAt: server.NowUtc(), + }) + SetProviderEgressProbeAttempt(ctx, &ProviderEgressProbeAttempt{ + ClientId: drop, AttemptAt: server.NowUtc().Add(-30 * 24 * time.Hour), + }) + + RemoveExpiredProviderEgressProbeAttempts(ctx, server.NowUtc().Add(-24*time.Hour)) + + if GetProviderEgressProbeAttempt(ctx, keep) == nil { + t.Fatal("recent attempt must survive the sweep") + } + if GetProviderEgressProbeAttempt(ctx, drop) != nil { + t.Fatal("old attempt must be swept") + } + }) +} + +// GetProviderEgressLocationDue is served by two statements -- never-probed +// first, then stale-but-probed only when the first came up short -- because the +// single-statement form sorts on observed_at from an outer-joined table, which +// cannot use an index and becomes a full scan plus an unindexable sort at 100k +// providers. +// +// The split is only safe if the concatenation is row-for-row what one statement +// returned, at every limit. That is what this asserts: it builds one population +// covering every eligibility case and then walks the limit from 0 past the end, +// requiring each result to be exactly the prefix of the full ordering. Limit 3 +// is the seam (pass one exactly fills the batch) and limit 4 is the first that +// crosses into pass two. +func TestGetProviderEgressLocationDueOrderingIsStableAcrossLimits(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + now := server.NowUtc() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + // three never-probed providers: the dominant group, and the reason the + // ordering is NULLS FIRST + never := []server.Id{server.NewId(), server.NewId(), server.NewId()} + // two stale ones, at different ages -- the older must be handed out first + staleOlder := server.NewId() + staleNewer := server.NewId() + // probed an hour ago: not due + fresh := server.NewId() + // never probed, but attempted seconds ago: deferred by the backoff, and + // the case that would otherwise starve the queue + attempted := server.NewId() + // probed long ago AND attempted seconds ago. This one is only screened + // by the backoff predicate on the stale-but-probed pass -- the + // never-probed pass never sees it, because it has an egress row. Drop + // that predicate and it reappears in the batch. + staleAttempted := server.NewId() + // no Public provide key: unprobeable at any freshness + nonPublic := server.NewId() + + address := 0 + connectProvider := func(clientId server.Id, provideMode ProvideMode) { + address += 1 + testing_connectProbeableProvider( + t, ctx, clientId, city.LocationId, + fmt.Sprintf("0.0.%d.1:0", address), provideMode, + ) + } + for _, clientId := range never { + connectProvider(clientId, ProvideModePublic) + } + connectProvider(staleOlder, ProvideModePublic) + connectProvider(staleNewer, ProvideModePublic) + connectProvider(fresh, ProvideModePublic) + connectProvider(attempted, ProvideModePublic) + connectProvider(staleAttempted, ProvideModePublic) + connectProvider(nonPublic, ProvideModeNetwork) + + UpdateClientLocationReliabilities(ctx, now.Add(-time.Hour), now) + + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: staleOlder, LocationId: city.LocationId, + CountryCode: "us", ObservedAt: now.Add(-100 * time.Hour), + }) + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: staleNewer, LocationId: city.LocationId, + CountryCode: "us", ObservedAt: now.Add(-50 * time.Hour), + }) + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: fresh, LocationId: city.LocationId, + CountryCode: "us", ObservedAt: now.Add(-1 * time.Hour), + }) + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: staleAttempted, LocationId: city.LocationId, + CountryCode: "us", ObservedAt: now.Add(-200 * time.Hour), + }) + SetProviderEgressProbeAttempt(ctx, &ProviderEgressProbeAttempt{ + ClientId: attempted, AttemptAt: now.Add(-5 * time.Second), + ProbeFailure: "tunnel_failed", + }) + // oldest observed_at of all, so it would sort to the head of the + // stale group if the backoff did not exclude it + SetProviderEgressProbeAttempt(ctx, &ProviderEgressProbeAttempt{ + ClientId: staleAttempted, AttemptAt: now.Add(-5 * time.Second), + ProbeFailure: "tunnel_failed", + }) + + minObservedAt := now.Add(-24 * time.Hour) + minAttemptAt := now.Add(-ProviderEgressProbeAttemptBackoff) + + // the never-probed group ties on a missing observed_at, so client_id + // alone orders it -- and postgres orders uuid by bytes, which is what + // server.Id.Cmp does + expected := slices.Clone(never) + slices.SortFunc(expected, func(a server.Id, b server.Id) int { return a.Cmp(b) }) + // ... then the probed group, oldest probe first + expected = append(expected, staleOlder, staleNewer) + + due := GetProviderEgressLocationDue(ctx, minObservedAt, minAttemptAt, 100) + if !slices.Equal(due, expected) { + t.Fatalf("due = %v, want %v (never-probed by client_id, then stale oldest-first; fresh/attempted/stale-attempted/non-public excluded)", due, expected) + } + + // every limit must return exactly the prefix of that ordering. limit 3 + // is the pass-one/pass-two seam; 4 is the first to cross it. + for limit := 0; limit <= len(expected)+2; limit += 1 { + want := expected[:min(limit, len(expected))] + got := GetProviderEgressLocationDue(ctx, minObservedAt, minAttemptAt, limit) + if !slices.Equal(got, want) { + t.Errorf("limit %d: due = %v, want %v", limit, got, want) + } + } + }) +} diff --git a/taskworker/taskworker.go b/taskworker/taskworker.go index 3364dce0..9cbc1c05 100644 --- a/taskworker/taskworker.go +++ b/taskworker/taskworker.go @@ -52,6 +52,7 @@ func InitTasks(ctx context.Context) { work.ScheduleRemoveExpiredAuthAttempts(clientSession, tx) work.ScheduleRemoveExpiredWalletAuthChallenges(clientSession, tx) work.ScheduleRemoveExpiredWalletNonces(clientSession, tx) + work.ScheduleRemoveExpiredProviderEgressLocations(clientSession, tx) work.ScheduleRemoveOldAuditNetworkEvents(clientSession, tx) work.ScheduleRemoveOldAuditEvents(clientSession, tx) work.ScheduleRemoveOldClientReliabilityStats(clientSession, tx) @@ -236,6 +237,10 @@ func InitTaskWorkerWithSettings(ctx context.Context, settings *task.TaskWorkerSe work.RemoveExpiredWalletNoncesPost, "github.com/urnetwork/server/taskworker/work.RemoveExpiredWalletNonces", ), + task.NewTaskTargetWithPost( + work.RemoveExpiredProviderEgressLocations, + work.RemoveExpiredProviderEgressLocationsPost, + ), task.NewTaskTargetWithPost( work.RemoveOldAuditNetworkEvents, work.RemoveOldAuditNetworkEventsPost, diff --git a/taskworker/work/provider_egress_location_work.go b/taskworker/work/provider_egress_location_work.go new file mode 100644 index 00000000..533d9609 --- /dev/null +++ b/taskworker/work/provider_egress_location_work.go @@ -0,0 +1,55 @@ +package work + +import ( + "time" + + "github.com/urnetwork/server" + "github.com/urnetwork/server/model" + "github.com/urnetwork/server/session" + "github.com/urnetwork/server/task" +) + +type RemoveExpiredProviderEgressLocationsArgs struct{} + +type RemoveExpiredProviderEgressLocationsResult struct{} + +func ScheduleRemoveExpiredProviderEgressLocations(clientSession *session.ClientSession, tx server.PgTx) { + task.ScheduleTaskInTx( + tx, + RemoveExpiredProviderEgressLocations, + &RemoveExpiredProviderEgressLocationsArgs{}, + clientSession, + task.RunOnce("remove_expired_provider_egress_locations"), + task.RunAt(server.NowUtc().Add(6*time.Hour)), + ) +} + +// RemoveExpiredProviderEgressLocations drops probed locations well past their +// trust window, so a provider that stops being probed eventually falls back to +// the mmdb path instead of being pinned to a stale location forever. The cutoff +// is deliberately looser than ProviderEgressLocationMaxAge: reads already +// ignore stale rows, so this is only reclaiming storage. +func RemoveExpiredProviderEgressLocations( + _ *RemoveExpiredProviderEgressLocationsArgs, + clientSession *session.ClientSession, +) (*RemoveExpiredProviderEgressLocationsResult, error) { + now := server.NowUtc() + minObservedAt := now.Add(-4 * model.ProviderEgressLocationMaxAge) + model.RemoveExpiredProviderEgressLocations(clientSession.Ctx, minObservedAt) + // probe attempts stop meaning anything once they no longer defer the + // provider; same reasoning as above, a looser multiple of the window that + // actually matters. + minAttemptAt := now.Add(-4 * model.ProviderEgressProbeAttemptBackoff) + model.RemoveExpiredProviderEgressProbeAttempts(clientSession.Ctx, minAttemptAt) + return &RemoveExpiredProviderEgressLocationsResult{}, nil +} + +func RemoveExpiredProviderEgressLocationsPost( + _ *RemoveExpiredProviderEgressLocationsArgs, + _ *RemoveExpiredProviderEgressLocationsResult, + clientSession *session.ClientSession, + tx server.PgTx, +) error { + ScheduleRemoveExpiredProviderEgressLocations(clientSession, tx) + return nil +}