Block the whole network for a flagged signup, not just its /32
docker-build-push / build-push (push) Successful in 4s
docker-build-push / build-push (push) Successful in 4s
register_ip_block always blocked the exact signup IP as a /32 (or /128 for v6), which is close to pointless for the datacenter/VPN/proxy space these flags fire on: a repeat bad actor from the same provider almost never reuses the exact same address, but very often reuses a different one in the same block. classify_signup_ip now returns a fourth value, block_cidr — ipapi.is's asn.route CIDR when it's a valid network that actually contains the signup IP (already being tracked in ipapi_range_cache purely for lookup caching), falling back to the address's own /32 or /128 when no usable route exists. register_ip_block blocks that instead of always deriving a /32 itself. Added range_cache_tests() covering the cache/fallback logic directly (pure sqlite + ipaddress, no network), and updated the existing ip_scrutiny_tests()/combined_signal_tests() mocks for the new 4-tuple classify_signup_ip return and register_ip_block arity.
This commit is contained in:
+50
-10
@@ -224,7 +224,7 @@ def ip_scrutiny_tests():
|
||||
ipblocks = []
|
||||
main.send_welcome = lambda account_id, acct: sent.append((account_id, acct))
|
||||
main.dm_moderator = lambda message: dms.append(message)
|
||||
main.register_ip_block = lambda ip, acct, org: ipblocks.append((ip, acct, org))
|
||||
main.register_ip_block = lambda ip, acct, org, cidr: ipblocks.append((ip, acct, org, cidr))
|
||||
# A flagged-but-not-held signup starts the suspicious watch inline, which
|
||||
# would otherwise hit the network for a baseline snapshot — stub it.
|
||||
main.fetch_account_counts = lambda account_id: (0, 0)
|
||||
@@ -233,9 +233,9 @@ def ip_scrutiny_tests():
|
||||
return classifications[ip]
|
||||
|
||||
classifications = {
|
||||
"203.0.113.10": ("clean", "Example Residential ISP", False),
|
||||
"198.51.100.20": ("datacenter", "Example Cloud Hosting Inc", True),
|
||||
"198.51.100.21": ("datacenter", "Example Cloud Hosting Inc", True),
|
||||
"203.0.113.10": ("clean", "Example Residential ISP", False, "203.0.113.10/32"),
|
||||
"198.51.100.20": ("datacenter", "Example Cloud Hosting Inc", True, "198.51.100.0/24"),
|
||||
"198.51.100.21": ("datacenter", "Example Cloud Hosting Inc", True, "198.51.100.0/24"),
|
||||
}
|
||||
main.classify_signup_ip = classify
|
||||
|
||||
@@ -256,7 +256,7 @@ def ip_scrutiny_tests():
|
||||
sent.clear(); dms.clear(); ipblocks.clear()
|
||||
main.process_signup("102", "dc1", "198.51.100.20")
|
||||
assert ("102", "dc1") not in sent, "welcome should be held for a flagged signup"
|
||||
assert ipblocks == [("198.51.100.20", "dc1", "Example Cloud Hosting Inc")], ipblocks
|
||||
assert ipblocks == [("198.51.100.20", "dc1", "Example Cloud Hosting Inc", "198.51.100.0/24")], ipblocks
|
||||
assert any("dc1" in d and "held" in d for d in dms), dms
|
||||
assert main.get_signup_flag("102") is True
|
||||
|
||||
@@ -268,7 +268,7 @@ def ip_scrutiny_tests():
|
||||
# fire, so process_signup must welcome + start the watch immediately
|
||||
# instead of waiting for an event that isn't coming.
|
||||
sent.clear(); dms.clear(); ipblocks.clear()
|
||||
classifications["198.51.100.22"] = ("datacenter", "Example Cloud Hosting Inc", True)
|
||||
classifications["198.51.100.22"] = ("datacenter", "Example Cloud Hosting Inc", True, "198.51.100.0/24")
|
||||
main.process_signup("104", "dc3", "198.51.100.22", "", True)
|
||||
assert ("104", "dc3") in sent, "already-approved flagged signup must be welcomed immediately"
|
||||
assert main.get_signup_flag("104") is True
|
||||
@@ -404,6 +404,45 @@ def email_scrutiny_tests():
|
||||
assert domain_blocks == [], domain_blocks
|
||||
|
||||
|
||||
def range_cache_tests():
|
||||
"""Drive the ipapi range-cache helpers (_ip_range_bounds/cache_ip_intel/
|
||||
cached_ip_intel) directly — pure sqlite + ipaddress logic, no network.
|
||||
This is what classify_signup_ip's block_cidr (the network register_
|
||||
ip_block blocks) is built on."""
|
||||
intel = {
|
||||
"is_datacenter": True, "is_vpn": False, "is_proxy": False,
|
||||
"is_tor": False, "is_abuser": False, "org": "Example Cloud Hosting Inc",
|
||||
}
|
||||
|
||||
# A. a valid route containing the ip caches (and returns) the wider CIDR,
|
||||
# not just the single address.
|
||||
main.cache_ip_intel("198.51.100.77", intel, route="198.51.100.0/24")
|
||||
cached = main.cached_ip_intel("198.51.100.77")
|
||||
assert cached["cidr"] == "198.51.100.0/24", cached
|
||||
assert cached["is_datacenter"] is True
|
||||
|
||||
# B. any other address in that same cached range hits the cache with the
|
||||
# same wider CIDR — this is the point of range-based caching, and
|
||||
# exactly what lets a repeat signup from elsewhere in the block also
|
||||
# resolve to blocking the whole range.
|
||||
cached2 = main.cached_ip_intel("198.51.100.200")
|
||||
assert cached2["cidr"] == "198.51.100.0/24", cached2
|
||||
|
||||
# C. an address outside the cached range is a cache miss.
|
||||
assert main.cached_ip_intel("198.51.101.1") is None
|
||||
|
||||
# D. a route that doesn't actually contain the ip falls back to a /32 —
|
||||
# never trust a route wide enough to not even cover the IP it came from.
|
||||
main.cache_ip_intel("203.0.113.9", intel, route="10.0.0.0/8")
|
||||
cached3 = main.cached_ip_intel("203.0.113.9")
|
||||
assert cached3["cidr"] == "203.0.113.9/32", cached3
|
||||
|
||||
# E. no route at all -> /32.
|
||||
main.cache_ip_intel("203.0.113.10", intel, route="")
|
||||
cached4 = main.cached_ip_intel("203.0.113.10")
|
||||
assert cached4["cidr"] == "203.0.113.10/32", cached4
|
||||
|
||||
|
||||
def combined_signal_tests():
|
||||
"""Drive process_signup's combined IP+email immediate-suspend path
|
||||
(SUSPICIOUS_COMBINED_*), independent of the OR-based hourly sweep."""
|
||||
@@ -415,7 +454,7 @@ def combined_signal_tests():
|
||||
main.send_welcome = lambda account_id, acct: sent.append((account_id, acct))
|
||||
main.dm_moderator = lambda message: dms.append(message)
|
||||
main.apply_action = lambda target_id, action, text: actions.append((target_id, action))
|
||||
main.register_ip_block = lambda ip, acct, org: ipblocks.append((ip, acct, org))
|
||||
main.register_ip_block = lambda ip, acct, org, cidr: ipblocks.append((ip, acct, org, cidr))
|
||||
main.register_email_domain_block = lambda domain, acct: domain_blocks.append((domain, acct))
|
||||
main.fetch_account_counts = lambda account_id: (0, 0)
|
||||
|
||||
@@ -434,8 +473,8 @@ def combined_signal_tests():
|
||||
main.ABUSE_ALLOWLIST = {"trustedstaff"}
|
||||
|
||||
ip_classifications = {
|
||||
"198.51.100.40": ("datacenter", "Example Cloud Hosting Inc", True),
|
||||
"198.51.100.41": ("datacenter", "Example Cloud Hosting Inc", True),
|
||||
"198.51.100.40": ("datacenter", "Example Cloud Hosting Inc", True, "198.51.100.32/28"),
|
||||
"198.51.100.41": ("datacenter", "Example Cloud Hosting Inc", True, "198.51.100.32/28"),
|
||||
}
|
||||
main.classify_signup_ip = lambda ip: ip_classifications[ip]
|
||||
email_classifications = {
|
||||
@@ -456,7 +495,7 @@ def combined_signal_tests():
|
||||
"SELECT 1 FROM suspicious_watch WHERE account_id = ?", ("501",),
|
||||
).fetchone()
|
||||
assert row is None, "an immediately-suspended signup must not also enter the hourly watch"
|
||||
assert ("198.51.100.40", "bothbad", "Example Cloud Hosting Inc") in ipblocks, ipblocks
|
||||
assert ("198.51.100.40", "bothbad", "Example Cloud Hosting Inc", "198.51.100.32/28") in ipblocks, ipblocks
|
||||
assert ("temp-mail.org", "bothbad") in domain_blocks, domain_blocks
|
||||
|
||||
# B. only IP flagged -> untouched by the combined path, falls through to
|
||||
@@ -643,6 +682,7 @@ if __name__ == "__main__":
|
||||
policy_tests()
|
||||
ip_scrutiny_tests()
|
||||
email_scrutiny_tests()
|
||||
range_cache_tests()
|
||||
combined_signal_tests()
|
||||
suspicious_watch_tests()
|
||||
suspicious_sweep_tests()
|
||||
|
||||
Reference in New Issue
Block a user