From 147673fd49614b8125674d226d68460aabeeeb2e Mon Sep 17 00:00:00 2001 From: waffle2k Date: Sun, 13 Sep 2026 16:08:37 -0700 Subject: [PATCH] Cap how wide a single auto-registered IP block may be ipapi.is reports the flagged address's whole ASN route, and register_ip_block blocked it whatever its width. That is the right unit for hosting space, where a repeat bad actor comes back from a different address in the same VPS block. It is badly wrong for an ISP aggregate, and the route cache holds routes as wide as /8, /9 and /11: one `abuser` flag on a residential or mobile address inside one of those would have walled off millions of consumer addresses. That was not hypothetical -- 197.211.32.0/19 is registered against GLOMOBILE-GPRS, a Benin mobile carrier, from a single `abuser` flag: 8,192 addresses of consumer mobile space. Anything wider than /24 (v4) or /48 (v6) now falls back to blocking the single address. This costs blocking strength against large hosting allocations -- an OVH /16 blocks one address now, so a repeat offender from that block needs a fresh block each time -- and in exchange makes the collateral-damage case impossible. Both ceilings are env-tunable. The cap applies to what gets BLOCKED, not to what gets CACHED: the wide route is still the right cache key, since it genuinely belongs to one operator and one lookup answers for the whole of it. Narrowing the cache key would multiply provider lookups for no benefit. Applied at registration time, so blocks already in force are untouched. --- .env.example | 23 ++++++++++++++++++ app/main.py | 66 +++++++++++++++++++++++++++++++++++++++++++++++---- test_local.py | 46 +++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 4 deletions(-) diff --git a/.env.example b/.env.example index bfebb31..d4e2b1a 100644 --- a/.env.example +++ b/.env.example @@ -259,6 +259,29 @@ BLOCKS_API_PUBLIC_URL=https://hooks.yttrx.com # caching costs nothing. BLOCKS_API_CACHE_SECONDS=300 +# Widest network a single auto-registered IP block may cover, as a prefix +# length. ipapi.is reports the flagged address's whole ASN route. That is the +# right unit for hosting space — a bad actor comes back from a different +# address in the same VPS block — but wrong for an ISP aggregate: routes as +# wide as /8, /9 and /11 turn up, and one `abuser` flag on a residential or +# mobile address inside one of those would wall off millions of consumer +# addresses. Anything wider than this ceiling falls back to blocking the +# single address. +# +# /24 costs real blocking strength against large hosting allocations (an OVH +# /16 now blocks one address, not the range, so a repeat offender out of that +# block needs a fresh block each time) in exchange for making the +# mobile/residential collateral-damage case impossible. Raise toward /16 to +# trade back the other way. +# +# Applied at registration time, not retroactively: blocks already in force +# keep whatever width they were registered with. +IP_SCRUTINY_MAX_BLOCK_PREFIX_V4=24 + +# The v6 ceiling. /48 is the standard end-site allocation — the rough +# equivalent of what a /24 represents in v4. +IP_SCRUTINY_MAX_BLOCK_PREFIX_V6=48 + # --------------------------------------------------------------------------- # Reputation query API (pre-signup pre-blocking) # --------------------------------------------------------------------------- diff --git a/app/main.py b/app/main.py index 837af4a..7b60e5e 100644 --- a/app/main.py +++ b/app/main.py @@ -173,6 +173,27 @@ IP_SCRUTINY_IPBLOCK_SEVERITY = os.environ.get("IP_SCRUTINY_IPBLOCK_SEVERITY", "s # suspends immediately at signup, regardless of the email-domain signal, # instead of just the held-welcome/auto-ipblock/lowered-threshold treatment # every other flagged signup gets. +# Widest network a single auto-registered block may cover, as a prefix +# length. ipapi.is reports the flagged address's whole ASN route, which is the +# right unit for hosting space (a bad actor comes back from a different +# address in the same VPS block) but badly wrong for an ISP aggregate: routes +# as wide as /8, /9 and /11 show up in the cache, and one `abuser` flag on a +# residential or mobile address inside one of those would otherwise wall off +# millions of consumer addresses. A route wider than this is treated as "not a +# meaningful unit to block" and falls back to the single address. +# +# This deliberately costs blocking strength against large hosting allocations +# — anything wider than a /24 (which is most of the VPS ranges registered so +# far) now blocks one address rather than the range, so a repeat offender out +# of the same block needs a fresh block each time — in exchange for making the +# mobile/residential collateral-damage case impossible. +# Applied at registration time, not retroactively — blocks already in force +# keep whatever width they were registered with. +IP_SCRUTINY_MAX_BLOCK_PREFIX_V4 = int(os.environ.get("IP_SCRUTINY_MAX_BLOCK_PREFIX_V4", "24")) +# /48 is the v6 analogue: the standard end-site allocation, and the rough +# equivalent of "one customer's block" that a /24 represents in v4. +IP_SCRUTINY_MAX_BLOCK_PREFIX_V6 = int(os.environ.get("IP_SCRUTINY_MAX_BLOCK_PREFIX_V6", "48")) + IP_SCRUTINY_STRONG_SIGNALS = frozenset({"datacenter", "proxy", "tor", "abuser"}) IP_SCRUTINY_STRONG_SUSPEND_ENABLED = os.environ.get( "IP_SCRUTINY_STRONG_SUSPEND_ENABLED", "true" @@ -697,6 +718,41 @@ def start_suspicious_watch(account_id: str, acct: str, reasons: str, ) +def _single_address_cidr(ip: str) -> str: + """The /32 or /128 for ip, or "" if it doesn't parse.""" + try: + addr = ipaddress.ip_address(ip) + except ValueError: + return "" + return f"{addr}/{32 if addr.version == 4 else 128}" + + +def capped_block_cidr(ip: str, cidr: str) -> str: + """The network to actually register a block for: cidr, unless it's wider + than IP_SCRUTINY_MAX_BLOCK_PREFIX_V4/V6, in which case the single address. + + Applied when deciding what to BLOCK, not when caching the classification — + the wide ASN route is still the right cache key (the whole route really + does belong to one operator, so one lookup answers for all of it), it is + just the wrong thing to hand to Admin::IpBlock. + """ + fallback = _single_address_cidr(ip) + if not cidr: + return fallback + try: + net = ipaddress.ip_network(cidr, strict=False) + except ValueError: + return fallback + cap = (IP_SCRUTINY_MAX_BLOCK_PREFIX_V4 if net.version == 4 + else IP_SCRUTINY_MAX_BLOCK_PREFIX_V6) + if net.prefixlen < cap: + log.info("block for ip=%s narrowed from route %s (%d addresses) to %s — " + "wider than the /%d ceiling", ip, cidr, net.num_addresses, + fallback or "the single address", cap) + return fallback or str(net) + return str(net) + + def record_reputation_query(kind: str, value: str, classification: str, org: str, risk: int, flagged: bool, target: str, blocked: bool, source: str) -> None: @@ -1294,7 +1350,9 @@ def classify_signup_ip(ip: str) -> tuple[str, str, bool, str]: else ip's own /32 or /128 — since a flagged signup is almost always datacenter/VPN/proxy space, where a repeat bad actor is far more likely to come back from a different address in the same block than the exact same - one. API failure yields ("unknown", "", False, "/32|128") — scrutiny + one. A route wider than the IP_SCRUTINY_MAX_BLOCK_PREFIX_V4/V6 ceiling is + narrowed back to the single address (see capped_block_cidr) — that width + means an ISP aggregate, not a hosting allocation. API failure yields ("unknown", "", False, "/32|128") — scrutiny should never trigger on our own lookup errors. """ intel = cached_ip_intel(ip) @@ -1308,8 +1366,7 @@ def classify_signup_ip(ip: str) -> tuple[str, str, bool, str]: data = resp.json() except (httpx.HTTPError, ValueError) as exc: log.warning("ipapi.is lookup failed for ip=%s: %s", ip, exc) - bounds = _ip_range_bounds(ip) - return "unknown", "", False, bounds[3] if bounds else f"{ip}/32" + return "unknown", "", False, _single_address_cidr(ip) or f"{ip}/32" # ipapi.is returns company/asn as flat strings, not nested dicts company = data.get("company") org_name = company if isinstance(company, str) else (company or {}).get("name", "") @@ -1345,7 +1402,8 @@ def classify_signup_ip(ip: str) -> tuple[str, str, bool, str]: ("abuser", "is_abuser"), ) if intel[key]] classification = "+".join(reasons) if reasons else "clean" - return classification, intel["org"], bool(reasons), intel.get("cidr") or f"{ip}/32" + block_cidr = capped_block_cidr(ip, intel.get("cidr") or "") + return classification, intel["org"], bool(reasons), block_cidr or f"{ip}/32" def register_ip_block(ip: str, acct: str, org: str, cidr: str = "", diff --git a/test_local.py b/test_local.py index cc69391..550e6a7 100644 --- a/test_local.py +++ b/test_local.py @@ -443,6 +443,51 @@ def range_cache_tests(): assert cached4["cidr"] == "203.0.113.10/32", cached4 +def block_width_cap_tests(): + """capped_block_cidr: the ASN route is the right unit to block for hosting + space and the wrong one for an ISP aggregate. ipapi.is reports routes as + wide as /8, so without a ceiling one `abuser` flag on a residential or + mobile address would wall off millions of consumer addresses.""" + main.IP_SCRUTINY_MAX_BLOCK_PREFIX_V4 = 24 + main.IP_SCRUTINY_MAX_BLOCK_PREFIX_V6 = 48 + + # A. At or narrower than the ceiling: block the route, as before. + assert main.capped_block_cidr("5.255.96.30", "5.255.96.0/24") == "5.255.96.0/24", \ + "the ceiling itself is allowed, not excluded" + assert main.capped_block_cidr("5.255.96.30", "5.255.96.0/28") == "5.255.96.0/28" + + # B. Wider than the ceiling: fall back to the single address. A /11 is a + # consumer ISP aggregate — blocking it would take out 2m addresses. + assert main.capped_block_cidr("98.45.12.7", "98.32.0.0/11") == "98.45.12.7/32" + assert main.capped_block_cidr("197.211.45.9", "197.211.32.0/19") == "197.211.45.9/32" + # ...including the VPS-sized ranges the bot used to block wholesale. + assert main.capped_block_cidr("51.222.9.9", "51.222.0.0/16") == "51.222.9.9/32" + assert main.capped_block_cidr("194.163.140.1", "194.163.128.0/18") == "194.163.140.1/32" + + # C. Same rule for v6, on its own ceiling (/48 = the standard end-site + # allocation, the rough v6 equivalent of a v4 /24). + assert main.capped_block_cidr("2001:db8:1::5", "2001:db8:1::/48") == "2001:db8:1::/48" + assert main.capped_block_cidr("2001:db8:1::5", "2001:db8::/40") == "2001:db8:1::5/128" + assert main.capped_block_cidr("2001:db8:1::5", "2001:db8::/29") == "2001:db8:1::5/128" + + # D. Missing or unparseable input degrades to the single address rather + # than to something wide. + assert main.capped_block_cidr("203.0.113.5", "") == "203.0.113.5/32" + assert main.capped_block_cidr("203.0.113.5", "not-a-cidr") == "203.0.113.5/32" + + # E. The cap is applied to what gets BLOCKED, not to what gets CACHED: + # the wide route is still the right cache key, since the whole route + # genuinely belongs to one operator and one lookup answers for all of + # it. Narrowing the cache key instead would multiply provider lookups. + intel = {"is_datacenter": False, "is_vpn": False, "is_proxy": False, + "is_tor": False, "is_abuser": True, "org": "Big ISP"} + main.cache_ip_intel("100.64.5.5", intel, route="100.64.0.0/10") + assert main.cached_ip_intel("100.64.200.200") is not None, \ + "a wide route must still answer from cache for its whole range" + assert main.cached_ip_intel("100.64.200.200")["cidr"] == "100.64.0.0/10" + assert main.capped_block_cidr("100.64.200.200", "100.64.0.0/10") == "100.64.200.200/32" + + def combined_signal_tests(): """Drive process_signup's combined IP+email immediate-suspend path (SUSPICIOUS_COMBINED_*), independent of the OR-based hourly sweep.""" @@ -1188,6 +1233,7 @@ if __name__ == "__main__": ip_scrutiny_tests() email_scrutiny_tests() range_cache_tests() + block_width_cap_tests() combined_signal_tests() strong_ip_suspend_tests() suspicious_watch_tests()