diff options
| -rw-r--r-- | abusectl/report.py | 26 | ||||
| -rw-r--r-- | docs/BACKLOG.md | 55 | ||||
| -rw-r--r-- | docs/specs/2026-09-09-report.md | 13 | ||||
| -rw-r--r-- | tests/test_report.py | 54 |
4 files changed, 108 insertions, 40 deletions
diff --git a/abusectl/report.py b/abusectl/report.py index 6bf3c54..45559a4 100644 --- a/abusectl/report.py +++ b/abusectl/report.py @@ -646,7 +646,7 @@ def feedback_fields(manifest: dict, destination: dict) -> list[tuple[str, str]]: ignore fields it does not support, so a standards parser reads what it knows and x-arf tooling finds what it wants. - Returned as PAIRS, not a dict, because Reported-Uri and Reported-Domain + Returned as PAIRS, not a dict, because Reported-URI and Reported-Domain repeat. Everything else does not, and that is enforced rather than assumed: RFC 5965 gives Source-IP and Arrival-Date "once maximum". A strict parser meeting a repeated single-occurrence field either rejects @@ -698,11 +698,24 @@ def feedback_fields(manifest: dict, destination: dict) -> list[tuple[str, str]]: # covers every URL under it. primary = (ips or domains or urls or [None])[0] + # "fraud" over "abuse", and both are checked against the IANA MARF + # registry rather than a worked example. RFC 5965 defines fraud as + # "indicates some kind of fraud or phishing activity" and abuse as + # "unsolicited email or some other kind of email abuse". This tool + # reports phishing, so fraud is the registered value that says what + # the report is, and some desks route it separately from bulk spam. + # + # There is no Report-Type. It is NOT in the registry (25 registered + # field names, it is absent), it was carried from the spec's worked + # example rather than chosen, and Feedback-Type: fraud already states + # what it was saying. Nothing would have broken by keeping it, since + # 5965 section 6 makes ignoring unknown fields a MUST, but that same + # section requires extension fields be registered, and every field + # emitted here should be one a desk can look up. fields = [ - ("Feedback-Type", "abuse"), + ("Feedback-Type", "fraud"), ("User-Agent", f"abusectl/{VERSION}"), ("Version", "1"), - ("Report-Type", "phishing"), ] if primary is not None: @@ -713,7 +726,12 @@ def feedback_fields(manifest: dict, destination: dict) -> list[tuple[str, str]]: for domain in domains: fields.append(("Reported-Domain", domain)) for url in urls: - fields.append(("Reported-Uri", url)) + # "Reported-URI", the spelling in RFC 5965's ABNF and its IANA + # registration. The RFC's own worked example writes "Reported-Uri" + # and that inconsistency is where this module's old spelling came + # from. Field names are case-insensitive per RFC 5322, so nothing + # was broken; matching the normative spelling costs nothing. + fields.append(("Reported-URI", url)) return [(name, _field_value(value)) for name, value in fields] diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 3ab642f..87ec4b3 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -8,7 +8,7 @@ number and gains a status rather than being renumbered. | 1 | Skip boilerplate namespace URLs | XS | open | | 2 | An IDN indicator resolves to no contact | S | open | | 3 | Expose kept cases so qtmaildir can tag spam | ? | open, unsized | -| 4 | `Report-Type: phishing` is unverified against x-arf | XS | open | +| 4 | `Report-Type: phishing` is unverified against x-arf | XS | closed | | 5 | `report.build()` raises KeyError on an identity with no email | XS | closed | ## 1. Skip boilerplate namespace URLs @@ -114,27 +114,38 @@ to qtmaildir. The second is much more in keeping with the rest of the tool. ## 4. `Report-Type: phishing` is unverified against x-arf -**Observed.** `report.feedback_fields()` emits `Report-Type: phishing` in the -machine-readable part. Every other field there was verified against RFC 5965 -itself; this one was not, because no primary source for x-arf's own field -semantics could be reached while building it. The abusix README documents only -the v3 to v4 deprecation and does not define the field. - -**Cause.** Not a defect found in the code. The value follows the worked -example in `docs/specs/2026-09-09-report.md`, so it is internally consistent, -and the hybrid envelope means a strict RFC 5965 parser ignores the field -either way (the RFC requires implementors ignore fields they do not support). -The exposure is limited to x-arf tooling reading a field name or value that -does not exist in the version it implements. - -**Approach.** Find a primary source for x-arf v4 field names, confirm or -correct the value, and record what it was checked against. If x-arf turns out -to name the field differently, the fix is one string and one test. - -**Constraints.** Low urgency: nothing here is a leak, and the failure mode is -a field an x-arf parser skips rather than acts on wrongly. Worth doing before -the first real report is filed, so a desk running x-arf tooling gets what it -expects. +**Closed** against the IANA MARF registry +(`https://www.iana.org/assignments/marf-parameters/marf-parameters.xml`), +checked 2026-09-10, plus RFC 5965 itself. The primary source the item asked +for exists; it is IANA's, not x-arf's. + +Two things were wrong, and neither was a leak. + +`Report-Type` is NOT a registered field. The registry lists 25 field names +and it is absent. It came from the worked example in the report spec rather +than from a decision. It is now REMOVED: `Feedback-Type` already carries what +it was saying. Keeping it would have been harmless, since RFC 5965 section 6 +makes ignoring an unknown field a MUST for the receiver, but that same section +requires an extension field be registered, and a desk should be able to look +up every field in a document this tool sends. + +`Feedback-Type` was `abuse` and is now `fraud`. RFC 5965 registers `fraud` as +"indicates some kind of fraud or phishing activity" and `abuse` as +"unsolicited email or some other kind of email abuse". This tool reports +phishing, so `fraud` is the registered value that says what the report is, +and some desks route it separately from bulk spam. + +A third, cosmetic, finding: the module emitted `Reported-Uri` while the RFC's +ABNF and its IANA registration both spell it `Reported-URI`. The RFC's own +worked example writes `Reported-Uri`, which is where the module's spelling +came from. Field names are case-insensitive per RFC 5322 so nothing was +broken, and it now matches the normative spelling. + +`tests/test_report.py::FeedbackPart::test_no_unregistered_field_is_emitted` +holds the registry's 25 names and asserts every emitted field is one of them. +The single exception is `Source`, which is x-arf's and is the reason the +envelope carries x-arf fields at all; it is named in the test so a NEW +unregistered field cannot arrive unnoticed. ## 5. `report.build()` raises KeyError on an identity with no email diff --git a/docs/specs/2026-09-09-report.md b/docs/specs/2026-09-09-report.md index 12d2157..b3a67ae 100644 --- a/docs/specs/2026-09-09-report.md +++ b/docs/specs/2026-09-09-report.md @@ -111,20 +111,27 @@ second body to keep in sync with the first for no reader. An RFC 5965 envelope carrying x-arf fields inside it. ``` -Feedback-Type: abuse +Feedback-Type: fraud User-Agent: abusectl/0.1.0 Version: 1 -Report-Type: phishing Source: 198.51.100.7 Source-IP: 198.51.100.7 Reported-Domain: example.invalid Arrival-Date: Mon, 08 Sep 2026 09:12:44 +0000 -Reported-Uri: http://login-example.invalid/verify?id=REDACTED +Reported-URI: http://login-example.invalid/verify?id=REDACTED ``` `Feedback-Type`, `User-Agent` and `Version` are the three fields RFC 5965 requires. The rest are optional there or come from x-arf. +**Every field name here is in the IANA MARF registry except `Source`**, which +is x-arf's and is the reason the envelope carries x-arf fields at all. An +earlier draft of this example also carried `Report-Type: phishing`, which is +not registered anywhere; it was removed once the registry was checked, since +`Feedback-Type` already says it. `fraud` rather than `abuse` because RFC 5965 +defines it as "some kind of fraud or phishing activity", which is what this +tool reports. See backlog item 4 for what was checked against what. + **Why an RFC 5965 envelope with x-arf fields inside, rather than either alone.** RFC 5965 is an IETF standard and universally understood, but it was designed for feedback loops, where a report is ABOUT A MESSAGE. These reports diff --git a/tests/test_report.py b/tests/test_report.py index 9a19d00..c8c39f8 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -961,19 +961,51 @@ class FeedbackPart(unittest.TestCase): self.lookup = dict(self.fields) def test_the_three_rfc5965_required_fields_are_present(self): - self.assertEqual(self.lookup["Feedback-Type"], "abuse") + self.assertEqual(self.lookup["Feedback-Type"], "fraud") self.assertEqual(self.lookup["Version"], "1") self.assertTrue(self.lookup["User-Agent"].startswith("abusectl/")) - def test_the_xarf_report_type_is_phishing(self): - self.assertEqual(self.lookup["Report-Type"], "phishing") + def test_the_feedback_type_is_the_registered_value_naming_phishing(self): + """Checked against the IANA MARF registry, not a worked example. + + RFC 5965 registers fraud as "indicates some kind of fraud or + phishing activity" and abuse as "unsolicited email or some other + kind of email abuse". This tool reports phishing. + """ + self.assertEqual(self.lookup["Feedback-Type"], "fraud") + + def test_no_unregistered_field_is_emitted(self): + """Every name here appears in the IANA MARF registry. + + Report-Type: phishing used to be emitted and is NOT registered. It + was harmless, since RFC 5965 section 6 makes ignoring an unknown + field a MUST, but that same section requires an extension field be + registered, and a desk should be able to look up every field in a + document this tool sends. + """ + registered = { + "Arrival-Date", "Auth-Failure", "Authentication-Results", + "Delivery-Result", "DKIM-ADSP-DNS", "DKIM-Canonicalized-Body", + "DKIM-Canonicalized-Header", "DKIM-Domain", "DKIM-Identity", + "DKIM-Selector", "DKIM-Selector-DNS", "Feedback-Type", + "Identity-Alignment", "Incidents", "Original-Mail-From", + "Original-Rcpt-To", "Received-Date", "Reported-Domain", + "Reported-URI", "Reporting-MTA", "Source-IP", "Source-Port", + "SPF-DNS", "User-Agent", "Version", + } + # Source is x-arf's, deliberately: RFC 5965 has no field for "this + # host is the thing being reported", which is why the envelope + # carries x-arf fields at all. It is the one exception and is named + # here so a NEW unregistered field cannot slip in unnoticed. + emitted = {name for name, _ in self.fields} + self.assertEqual(emitted - registered - {"Source"}, set()) def test_source_is_the_primary_indicator(self): self.assertEqual(self.lookup["Source"], "203.0.113.42") self.assertEqual(self.lookup["Source-IP"], "203.0.113.42") def test_every_url_appears_as_a_reported_uri(self): - uris = [value for name, value in self.fields if name == "Reported-Uri"] + uris = [value for name, value in self.fields if name == "Reported-URI"] self.assertEqual( uris, ["http://login.sender.invalid/verify?id=REDACTED"] ) @@ -1048,11 +1080,11 @@ class FeedbackPart(unittest.TestCase): seen: dict[str, int] = {} for name, _ in fields: seen[name] = seen.get(name, 0) + 1 - repeatable = {"Reported-Uri", "Reported-Domain"} + repeatable = {"Reported-URI", "Reported-Domain"} for name, count in seen.items(): if name not in repeatable: self.assertEqual(count, 1, f"{name} appeared {count} times") - self.assertEqual(seen["Reported-Uri"], 2) + self.assertEqual(seen["Reported-URI"], 2) self.assertEqual(seen["Reported-Domain"], 2) def test_an_ipv6_indicator_fills_source_ip_too(self): @@ -1086,7 +1118,7 @@ class FeedbackPart(unittest.TestCase): self.assertNotIn("Source", lookup) self.assertNotIn("Source-IP", lookup) # The envelope is still well formed: a desk gets a valid part. - self.assertEqual(lookup["Feedback-Type"], "abuse") + self.assertEqual(lookup["Feedback-Type"], "fraud") self.assertEqual(lookup["Version"], "1") def test_a_type_this_module_does_not_place_is_not_invented_into_one(self): @@ -1133,7 +1165,7 @@ class FeedbackPart(unittest.TestCase): "target": "abuse@host.invalid", "body": None, "status": "pending"} lookup = dict(report.feedback_fields(MANIFEST, destination)) - self.assertEqual(lookup["Feedback-Type"], "abuse") + self.assertEqual(lookup["Feedback-Type"], "fraud") def test_arrival_date_is_not_taken_from_the_senders_date_header(self): """RFC 5965: Arrival-Date is when the generating ADMD's MTA received @@ -1189,7 +1221,7 @@ class FeedbackInjection(unittest.TestCase): destination = report.email_destinations(manifest["contacts"])[0] fields = report.feedback_fields(manifest, destination) for name, out in fields: - if name == "Reported-Uri": + if name == "Reported-URI": return out return None @@ -1386,7 +1418,7 @@ class FeedbackInjection(unittest.TestCase): fields = report.feedback_fields(manifest, destination) self.assertEqual( {name for name, _ in fields}, - {"Feedback-Type", "User-Agent", "Version", "Report-Type"}) + {"Feedback-Type", "User-Agent", "Version"}) def test_a_non_string_value_does_not_crash_the_report(self): """A manifest is edited by hand and JSON has numbers. @@ -1800,7 +1832,7 @@ class Writing(unittest.TestCase): self.assertNotIn(b"\n", header.replace(b"\r\n", b"")) # And the bare LFs inside the feedback part survive untranslated, # which is the same defect seen from the other side. - self.assertIn(b"Feedback-Type: abuse\nUser-Agent:", raw) + self.assertIn(b"Feedback-Type: fraud\nUser-Agent:", raw) def test_an_untouched_case_regenerates_without_complaint(self): """The regression that the plan's read_text() caused. |