diff options
Diffstat (limited to 'AGENTS.md')
| -rw-r--r-- | AGENTS.md | 94 |
1 files changed, 85 insertions, 9 deletions
@@ -37,7 +37,7 @@ python3 -m unittest tests.test_parse -v Run the tool from the checkout with `python3 -m abusectl`. -## THREE PROPERTIES THAT ARE NOT NEGOTIABLE +## FOUR PROPERTIES THAT ARE NOT NEGOTIABLE Each has a concrete victim. Do not weaken one for convenience, and do not "simplify" the code that enforces it without reading this section first. @@ -112,6 +112,47 @@ With no boundary configured, `parse` REFUSES rather than guessing the outermost public IP. Refusing is only defensible because `abusectl init` is the route out, which is why that command exists at all. +### 4. A query carries a bare host or IP, never a URL + +`contacts` is the first part that talks to anyone. An RDAP query discloses to a +third party which host or netblock the user is investigating, so it carries a +BARE HOST OR IP ADDRESS and nothing else. + +**Property 1 does not cover this.** Property 1 governs what is PUBLISHED, and a +query appears in no report at all. A URL path can carry recipient identity, and +`suspect_path_segments` deliberately FLAGS a suspect segment rather than +redacting it, which is safe only while the URL stays local. The moment a URL +becomes a query it stops being local, and the flag protects nobody. The victim +is the recipient whose address sits in a path segment and is handed to a +registrar, and, when the registrar is the attacker's own, the reporter, who has +just told the attacker they are under investigation. + +`contacts.is_queryable()` is the SINGLE ADMISSION POINT. Every branch of +`worklist()` funnels through `add()`, which refuses a value that is not a bare +host or IP and keeps it as `unusable` so the user sees it during review. Do not +move that check into a branch, and do not add a branch that reaches `rdap` +without passing through it. + +**That rule exists because the property leaked three times, each time the same +shape.** First the `url` branch was guarded with a host-only extraction and the +`domain` branch was not, so `From: Bank <phish@victim%40example.org.invalid>` +sent the recipient's own address to a registry. Then the `domain` branch was +guarded at one admission point and the IP branch still leaked, because +`_is_ip()` sat INSIDE that point and returned True before the validator ran, and +`ipaddress.ip_address()` accepts an IPv6 scope id whose content is unconstrained +free text: `fe80::1%victim@example.org` parses as valid. The lesson is both +halves of that: **validation applied per branch gets forgotten on the next +branch, and a check that short-circuits before the validator is the same bug +wearing a different coat.** + +`rdap._quoted()` percent-encodes a component so it cannot escape its path +segment. That contains URL-STRUCTURE attacks, traversal and an injected query or +fragment; it does NOT reduce disclosure, because an encoded address is still the +address. It is not a second line of defence for this property, and the third +leak came from treating it as one: `safe=""` encoded the colons RFC 9082 +requires be literal, so every IPv6 query silently 404ed and read as "no abuse +desk published". + ## Architecture ``` @@ -122,10 +163,12 @@ abusectl/ parse.py .eml -> IOCs pure, offline redact.py the safety rule, alone and testable case.py case directory: create, manifest read/write, atomic + contacts.py IOCs -> abuse contacts network, read-only + rdap.py bootstrap, query, jCard network, read-only ``` -Planned, each needing its own spec first: `contacts` (RDAP), `report` (X-ARF), -`submit` (MISP then vendors), `retry` (cron). See the design document. +Planned, each needing its own spec first: `report` (X-ARF), `submit` (MISP then +vendors), `retry` (cron). See the design document. **`redact.py` is separate from `parse.py` deliberately.** It is the safety property, and a module of its own gets tests that name it rather than tests @@ -265,6 +308,36 @@ When a sweep finds a defect, reproduce it as a synthetic fixture under the rule above and commit THAT. The real message stays in the scratchpad, which is per-session and outside the repository. +**A module that talks to third parties gets TWO sweeps, and they are not the +same shape.** `contacts` was verified this way and any later network part +should be too. + +Sweep A is OFFLINE and covers the whole corpus. It builds the worklist for +every message and asserts the fourth property against it, with no query issued. +Breadth is free because nothing leaves the machine. What it observed: 92 +messages, 1685 indicators, 0 crashes, 0 empty parses, 110 unique query targets, +1206 indicators folded away by host deduplication, and 0 malformed queries. The +second assertion took all 548 addresses appearing in the raw sources and checked +each against every query string: no address reached a query, and neither did any +local part of six characters or more. + +Sweep B is ONLINE, hand-picked and deliberately SMALL, because every query in it +discloses to a third party and cannot be undone. Twelve targets, chosen to +exercise the branches rather than to cover volume: 12 of 12 completed, 0 +failures. All five RIRs, RIPE, ARIN, APNIC, LACNIC and AFRINIC, returned a +parseable jCard. IPv6 resolved live. The label walk reduced `www.ripe.net` and +`a.b.c.example.org` to two labels, and `nic.uk` resolved, which is the +multi-part suffix case. Three non-resolutions were correct behaviour rather than +failures: AFRINIC and `nic.cz` publish a handle but no `abuse` role, and +`example.museum` has no RDAP server for the TLD. The IANA bootstrap held 5 IPv4 +services, 5 IPv6 and 590 DNS. + +**Sweep B must never draw its targets from the user's own spam corpus.** A query +tells a registrar which of their customers someone is investigating, and for a +phishing domain that registrar may be the attacker's own. Pick targets that are +public infrastructure or documentation names, and keep the corpus to sweep A, +where nothing is sent. + ## Working on this repo Work directly on `master`, no PR flow. Commits are GPG-signed (`git commit -S`); @@ -273,9 +346,9 @@ rejection is correct until proven otherwise. `HANDOFF.md` is local-only and gitignored; never stage or commit it. -Design first for anything unbuilt: `contacts`, `report`, `submit` and `retry` -each need their own spec before code, because each has real unknowns. The -umbrella design settles only what they share. +Design first for anything unbuilt: `report`, `submit` and `retry` each need +their own spec before code, because each has real unknowns. The umbrella design +settles only what they share. ## Documents @@ -283,11 +356,14 @@ umbrella design settles only what they share. before changing anything about the case format, the redaction rule, or the ordering between MISP and the vendors. - `docs/specs/2026-09-09-contacts.md`, the `contacts` spec. Read it before - touching RDAP, the bootstrap cache, or anything that issues a query: it adds - a FOURTH non-negotiable property, that a query carries a bare host or IP and - never a URL. + touching RDAP, the bootstrap cache, or anything that issues a query. The + fourth property it introduced is stated above in its own right; the spec + carries the reasoning behind the rest of the module. - `docs/plans/2026-09-08-parse.md`, the plan `init` and `parse` were built from. Historical once built, but it records why each test exists. +- `docs/plans/2026-09-09-contacts.md`, the plan `contacts` and `rdap` were + built from. Historical in the same way, and it records the three leaks the + fourth property above is written from. - `docs/BACKLOG.md`, open items, with the cause verified in the code rather than assumed. Read it before starting work; add to it rather than fixing something unasked. |