diff options
| -rw-r--r-- | abusectl/report.py | 36 | ||||
| -rw-r--r-- | tests/test_report.py | 103 |
2 files changed, 127 insertions, 12 deletions
diff --git a/abusectl/report.py b/abusectl/report.py index 06a3cb5..a624142 100644 --- a/abusectl/report.py +++ b/abusectl/report.py @@ -29,6 +29,14 @@ itself is a module that can disclose it in a code path nobody reviewed. import hashlib +# The role mailboxes RFC 2142 mandates, which it also requires be matched +# case-insensitively. Only the ones an RDAP abuse entity plausibly +# publishes; this is not the full list and does not need to be. +_ROLE_MAILBOXES = frozenset({ + "abuse", "postmaster", "security", "noc", "hostmaster", +}) + + def _group_key(address: str) -> str: """The key two spellings of one desk must share, and no more than that. @@ -36,14 +44,26 @@ def _group_key(address: str) -> str: "abuse@Host.Invalid" and "abuse@host.invalid" are one desk and must not be mailed twice about one incident. - The LOCAL PART is left exactly as published. RFC 5321 leaves its + The LOCAL PART folds only for the RFC 2142 ROLE MAILBOXES. Those are + standardised names that the same RFC requires be recognised regardless + of case, so no host runs "Abuse@" and "abuse@" as two different desks, + and treating them as two is a duplicate mail with nothing on the other + side of the trade. This is where the duplicate actually happens, since + an abuse entity publishes a role mailbox nearly every time. + + Any other local part is left exactly as published. RFC 5321 leaves its interpretation to the receiving host, and only that host knows whether - it folds case. In practice it almost always does, but "almost always" - is the wrong standard for the one field that decides whether a report - arrives: folding two desks that a host genuinely distinguishes would - silently drop one of them, and the cost of being wrong the other way - is a duplicate mail. A dropped desk is the worse failure, so the - conservative direction is to fold only what is defined to fold. + it folds case. For a NAMED mailbox the asymmetry that governs the role + names reverses: folding two desks a host genuinely distinguishes would + silently drop one of them, and a dropped desk is worse than a duplicate + mail. So each half folds on the strength of its own standard, and + neither borrows the other's. + + The first version of this folded the domain alone and shipped with a + test that used a lowercase local part throughout, so the test passed + while "Abuse@Host.Invalid" and "abuse@host.invalid" produced two + destinations. A test that varies one half of its input proves nothing + about the other. """ local, at, domain = address.rpartition("@") if not at: @@ -51,6 +71,8 @@ def _group_key(address: str) -> str: # occasionally malformed; group it by its literal text rather than # inventing a domain for it. return address + if local.lower() in _ROLE_MAILBOXES: + local = local.lower() return f"{local}@{domain.lower()}" diff --git a/tests/test_report.py b/tests/test_report.py index c72974e..5649b0b 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -1,3 +1,4 @@ +import copy import unittest from abusectl import report @@ -124,6 +125,23 @@ class Grouping(unittest.TestCase): self.assertEqual(destinations[0]["iocs"], ["ioc-1", "ioc-2"]) self.assertEqual(destinations[0]["target"], "abuse@Host.Invalid") + def test_the_domain_folds_under_a_local_part_that_does_not(self): + """Isolate the domain fold from the role fold. + + The version of this test that first shipped used a lowercase + local part throughout, so it exercised only the domain and passed + while a capitalised role name produced two destinations. + """ + contacts = [ + {"iocs": ["ioc-1"], "query": "198.51.100.7", + "abuse": ["J.Smith@Host.Invalid"], "source": "rdap"}, + {"iocs": ["ioc-2"], "query": "example.invalid", + "abuse": ["J.Smith@host.invalid"], "source": "rdap"}, + ] + destinations = report.email_destinations(contacts) + self.assertEqual(len(destinations), 1) + self.assertEqual(destinations[0]["iocs"], ["ioc-1", "ioc-2"]) + def test_two_spellings_of_one_desk_share_an_id(self): """The id derives from the same normalised form the grouping uses. @@ -134,11 +152,31 @@ class Grouping(unittest.TestCase): self.assertEqual(report.email_destination_id("abuse@Host.Invalid"), report.email_destination_id("abuse@host.invalid")) - def test_two_local_part_cases_stay_two_destinations(self): - """Only the receiving host knows whether its local parts fold. + def test_a_role_mailbox_folds_in_both_halves(self): + """"Abuse@Host.Invalid" and "abuse@host.invalid" are one desk. + + The case that first shipped folded the domain only, so a jCard + publishing the role name capitalised produced two destinations and + two mails to one desk. RFC 2142 mandates the role mailboxes and + requires them case-insensitive, so no host runs "Abuse@" and + "abuse@" as different desks. + """ + contacts = [ + {"iocs": ["ioc-1"], "query": "198.51.100.7", + "abuse": ["Abuse@Host.Invalid"], "source": "rdap"}, + {"iocs": ["ioc-2"], "query": "example.invalid", + "abuse": ["abuse@host.invalid"], "source": "rdap"}, + ] + destinations = report.email_destinations(contacts) + self.assertEqual(len(destinations), 1) + self.assertEqual(destinations[0]["iocs"], ["ioc-1", "ioc-2"]) + self.assertEqual(destinations[0]["target"], "Abuse@Host.Invalid") + + def test_one_contact_publishing_a_role_mailbox_twice_folds_it(self): + """The same fold applies within one contact's own abuse list. - Folding them here would silently drop a desk that a host genuinely - distinguishes; not folding them costs a duplicate mail at worst. + rdap.abuse_addresses dedupes case-sensitively, so a jCard with two + vcard rows spelling the role differently delivers both here. """ contacts = [ {"iocs": ["ioc-1"], "query": "198.51.100.7", @@ -146,8 +184,32 @@ class Grouping(unittest.TestCase): "source": "rdap"}, ] destinations = report.email_destinations(contacts) + self.assertEqual(len(destinations), 1) + self.assertEqual(destinations[0]["iocs"], ["ioc-1"]) + + def test_every_rfc2142_role_this_tool_can_meet_folds(self): + for role in ("abuse", "postmaster", "security", "noc", "hostmaster"): + with self.subTest(role=role): + self.assertEqual( + report.email_destination_id(f"{role.title()}@host.invalid"), + report.email_destination_id(f"{role}@host.invalid")) + + def test_a_personal_local_part_is_left_alone(self): + """Only the receiving host knows whether ITS local parts fold. + + A named mailbox is not a standardised role, so folding it could + silently merge two desks a host genuinely distinguishes and drop + one of them. Two mails to one desk is the lesser failure, and the + role names above are where the duplicate actually happens. + """ + contacts = [ + {"iocs": ["ioc-1"], "query": "198.51.100.7", + "abuse": ["J.Smith@host.invalid", "j.smith@host.invalid"], + "source": "rdap"}, + ] + destinations = report.email_destinations(contacts) self.assertEqual(sorted(d["target"] for d in destinations), - ["Abuse@host.invalid", "abuse@host.invalid"]) + ["J.Smith@host.invalid", "j.smith@host.invalid"]) self.assertNotEqual(destinations[0]["id"], destinations[1]["id"]) def test_a_destination_starts_with_no_body(self): @@ -157,6 +219,37 @@ class Grouping(unittest.TestCase): ] self.assertIsNone(report.email_destinations(contacts)[0]["body"]) + def test_the_contacts_passed_in_are_not_modified(self): + """The caller's contacts are the manifest's own array. + + case.py is the only writer of a manifest, so a grouping pass that + edited what it was handed would write through it from outside. + """ + contacts = [ + {"iocs": ["ioc-1"], "query": "198.51.100.7", + "abuse": ["abuse@host.invalid"], "source": "rdap"}, + ] + before = copy.deepcopy(contacts) + report.email_destinations(contacts) + self.assertEqual(contacts, before) + + def test_a_destinations_ioc_list_is_its_own(self): + """Not aliased to the contact's list it was built from. + + Holds today because the grouping starts a fresh list, but nothing + else pins it: an implementation that reused contact["iocs"] for a + single-contact destination would pass every other test here and + leave a destination and a contact sharing one list in a manifest + about to be written. + """ + contacts = [ + {"iocs": ["ioc-1"], "query": "198.51.100.7", + "abuse": ["abuse@host.invalid"], "source": "rdap"}, + ] + destination = report.email_destinations(contacts)[0] + destination["iocs"].append("ioc-2") + self.assertEqual(contacts[0]["iocs"], ["ioc-1"]) + def test_the_same_contacts_produce_the_same_ids_twice(self): """Ids must not depend on dict iteration luck or set ordering.""" contacts = [ |