feat(geoip): per-policy country allowlist/blocklist filtering - #286
Merged
Conversation
Adds GeoIP country filtering to WAF policies: a MaxMind GeoLite2 MMDB pipeline that generates a HAProxy CIDR-to-country map consumed natively via map_ip(), admin CRUD for geoip_mode/geoip_countries on policies, a manual refresh endpoint, and a weekly scheduled refresh job. Closes #175
Switch the GeoIP database from MaxMind GeoLite2-Country to ip66.dev, and fix the issues a review of the initial implementation surfaced. Database source: - ip66.dev needs no license key, so MAXMIND_LICENSE_KEY, the tarball extraction and the whole "feature not configured" state (including GeoipNotConfiguredError and GeoipRefreshResult.configured) are gone. Removing the key also removes a leak: httpx embedded the full request URL, query string included, in HTTPStatusError messages, which reached both the logs and the API response body. - Add GEOIP_DATABASE_URL (defaulting to ip66.dev) so operators can point at a mirror or an air-gapped copy. - Download is a conditional GET on the stored ETag/Last-Modified; the refresh interval drops from 7 days to 1 to match the daily rebuild. - Attribute ip66.dev per its CC BY 4.0 licence. Correctness and safety: - Iterate the MMDB through the C extension. The pure Python reader (MODE_MEMORY) raises "::1:0:0/0 has host bits set" partway through the real database, so map generation could not run at all; a fallback to it now raises an actionable GeoipError. - Collapse networks per run while streaming instead of buffering every network first: same output, ~70 MB peak instead of ~780 MB. - Regenerate the map when it is still a stub even if the download was a 304, so a current database can no longer sit behind an empty map that silently fails open. - Stop exempting /health from the deny rules. It is a host-independent path match with no use_backend of its own, so it routed to the customer origin and was a trivial full bypass. ACME stays exempt; it has its own local backend. - Emit country codes as repeated same-name ACLs of 50. HAProxy truncates a config line after 64 words, so a long list produced a fatally invalid config (verified against haproxy:3.0-alpine). - Alias UK to GB; the database uses UK for a few networks that ISO 3166-1 spells GB, which blocking GB would otherwise miss. - Serialise reload_haproxy() on the same lock as apply(), so a refresh can no longer reload HAProxy mid-apply and activate a release being rolled back. - Create the enum type before adding the column on PostgreSQL, matching the existing pattern; alembic upgrade head failed there otherwise. - Run an initial GeoIP refresh shortly after boot instead of one interval later, which left the map a stub for a full day after every start. - Do not submit geoip_countries while the mode is off; the input is hidden then, so leftover text caused a 422 the user could not see. Tests cover the reload-failure and stub-regeneration paths, the unusable reader, the ACL chunking, the /health non-exemption, the map-file OSError branch and the scheduler job, which had no coverage at all.
Addresses review findings on the refresh path, where the filter could silently degrade to bad data. - Move the refresh lock from the router into geoip_service. The scheduled job called refresh() directly and so bypassed the router's lock, while both paths write the same fixed temp files (.country.mmdb.tmp, .country.map.tmp). Concurrent runs could interleave writes, and each finally-unlink would delete the other's temp file before its os.replace(), publishing a corrupt map. The API now uses try_refresh(), which returns None instead of blocking, so it still answers 409. - Catch maxminddb.InvalidDatabaseError. It subclasses RuntimeError, so neither OSError nor GeoipError caught it, and refresh() claimed never to raise. A mirror answering 200 with an HTML error page therefore turned POST /geoip/refresh into a 500 and killed the scheduled job. - Drop the cached ETag/Last-Modified when the downloaded file turns out not to be a usable database. The validators are recorded as soon as the transfer succeeds, which is before the file is known to be valid, so keeping them made every later refresh a 304 and the corruption permanent. - Filter generated map entries through MAPPABLE_COUNTRY_CODES rather than VALID_COUNTRY_CODES, so the internal ZZ sentinel can never be emitted as a resolved country. In allowlist mode ZZ would have made the -m found guard true while matching no allowed code, denying a request that fail-open promises to allow. - Stream the map file when hashing it instead of loading ~18 MB twice per run. - Document the ~560 MB peak across a reload, that map_ip() runs for all frontend traffic, and that GEOIP_FAIL_OPEN is baked in at config generation so it needs a full re-apply.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds country-level GeoIP filtering per policy (
geoip_mode=off/allowlist/blocklist, plusgeoip_countries), configurable on the existing policy create/edit screen.The lookup is native to HAProxy: the backend converts a country-level MMDB into a plain
CIDR -> ISO codemap file and the template resolvessrcthroughmap_ip(). No Lua, no HAProxy MMDB module, no new runtime dependency in the proxy.Database source is ip66.dev, not MaxMind GeoLite2. It needs no license key and no registration, is rebuilt daily, and is CC BY 4.0 (attributed in
docs/architecture.mdand in the generated map header). Dropping the key also removed a leak path:httpxembeds the full request URL — query string included — inHTTPStatusErrormessages, which reached both the logs and the API response body.GEOIP_DATABASE_URLis configurable for mirrors and air-gapped installs.Operational notes for reviewers
haproxy:3.0-alpine). That is the main running cost of this feature.GEOIP_FAIL_OPEN). Note ~306k networks carry no country and a further ~90k are labelledEU, which is not an ISO country — so an allowlist ofPLalone still admits everyEUnetwork. Documented indocs/architecture.md.POST /geoip/refreshon demand, and reloads HAProxy only when the map actually changed.Issues found and fixed before opening this PR
Review of the first implementation, plus verification against the real database and the real HAProxy image, turned up:
/healthwas a full bypass — a host-independent path match with nouse_backendof its own, so a blocked country could reach the customer origin just by requesting/health. It is no longer exempt; ACME still is, since it has its own local backend.maxminddbreader raises::1:0:0/0 has host bits setpartway through the real database, soMODE_MEMORYwas unusable. Iteration goes through the C extension; a fallback raises an actionable error.alembic upgrade headfailed on PostgreSQL — the enum type was not created before the column referencing it.UKvsGB— the database labels a few networksUK, which blockingGBwould have missed. Aliased.off, producing a 422 the user could neither see nor fix.Test plan
uv run pytest --cov=app— 683 passed, 6 skippeduv run mypy app/— cleanuv run ruff check app/— cleanpnpm run type-check,pnpm run lint,pnpm test— clean, 135 frontend tests passhaproxy -cagainst a rendered config carrying all 249 ISO codes, run inhaproxy:3.0-alpine— valid; also started HAProxy with the real 920k-entry map to confirm it loads and to measure its memory/healthnon-exemption, the map-fileOSErrorbranch, and the scheduler job, which previously had no coverageCloses #175