diff options
| -rw-r--r-- | abusectl/cli.py | 3 | ||||
| -rw-r--r-- | abusectl/parse.py | 97 | ||||
| -rw-r--r-- | tests/fixtures/reportable.eml | 22 | ||||
| -rw-r--r-- | tests/test_cli.py | 20 | ||||
| -rw-r--r-- | tests/test_offline.py | 8 | ||||
| -rw-r--r-- | tests/test_parse.py | 62 |
6 files changed, 211 insertions, 1 deletions
diff --git a/abusectl/cli.py b/abusectl/cli.py index 2200e14..ed31a4b 100644 --- a/abusectl/cli.py +++ b/abusectl/cli.py @@ -263,6 +263,9 @@ def _cmd_parse(args) -> int: manifest = case.load(created.path) manifest["iocs"] = parse_module.iocs(raw, trusted=settings.trusted_relays) manifest["auth"] = parse_module.auth_results(raw) + manifest["headers"] = parse_module.report_headers( + raw, trusted=settings.trusted_relays + ) case.save(created.path, manifest) except parse_module.NoTrustBoundary as exc: print(f"abusectl parse: {exc}", file=sys.stderr) diff --git a/abusectl/parse.py b/abusectl/parse.py index 230ca2d..8773c85 100644 --- a/abusectl/parse.py +++ b/abusectl/parse.py @@ -60,6 +60,31 @@ _URL = re.compile(r"https?://[^\s<>\"')]+", re.IGNORECASE) # stack or spin forever, independent of the `seen` cycle guard. _MAX_REDIRECT_DEPTH = 5 +# The optional "for <addr>" clause of a Received header (RFC 5321 4.4). Our +# own boundary relay writes the ENVELOPE RECIPIENT there, so the one Received +# line a report publishes carries the victim's address verbatim unless this +# is removed. Matched up to the clause terminator rather than to end of line +# because "for" is not always last: a timestamp follows it after the ";". +_RECEIVED_FOR = re.compile(r"(?is)\bfor\s+<[^>]*>\s*(?=;|$)") + +# The headers that may appear in a published report. A WHITELIST, never a +# blacklist: a blacklist means every header this parser learns to read later +# is a leak waiting for someone to remember. To, Cc, Delivered-To and +# X-Original-To are absent by construction, which is the same reason iocs() +# does not read them either. +_REPORT_HEADERS = ( + "From", + "Subject", + "Date", + "Message-ID", + "Reply-To", + "Return-Path", + "Authentication-Results", + "Received-SPF", + "MIME-Version", + "Content-Type", +) + @dataclass(frozen=True) class Hop: @@ -156,6 +181,78 @@ def sending_ip(raw: bytes, trusted: list[str]) -> str | None: return None +def _strip_envelope_recipient(received_value: str) -> str: + """Remove the "for <addr>" clause from a Received header. + + The boundary hop is written by OUR OWN relay, and that clause is where it + records the envelope recipient: the victim's address, in the one header a + report reproduces verbatim. Truncating the chain at the boundary does not + help here, because the leak is INSIDE the line being kept, which is why + this is a separate step rather than part of the walk. + + The clause is optional (RFC 5321 4.4) and carries nothing a desk needs: + the report is about who SENT the message. Only the clause goes, so the + hop's own evidence, the address and the receiving server, survives. + """ + return _RECEIVED_FOR.sub("", received_value).rstrip() + + +def report_headers(raw: bytes, trusted: list[str]) -> list[tuple[str, str]]: + """Return the headers that may be published, outermost Received first. + + `report` never opens source.eml: the decision about what may be disclosed + is made once, here, beside every other one. A second module filtering the + original at report time would put that decision in two places, and two + places to remember is how the fourth property leaked three times. + + Received is published as EXACTLY ONE line, the boundary hop, and the + chain is cut in both directions. Above it are our own relays: publishing + them tells a third party about the user's mail path. Below it is the + attacker's own writing, and that half is the dangerous one. A forged + chain names an innocent third party (the forged-chain fixture plants + 198.51.100.7 for this), so publishing a hop below the boundary puts + someone else's address into a report an abuse desk will act on. This is + the third property, applied to what gets DISCLOSED rather than to what + sending_ip() concludes, and the answer is the same for the same reason: + the boundary hop is the last line we can stand behind. + + That one surviving line still goes through _strip_envelope_recipient(): + it was written by our own relay and names the victim in its "for" clause, + so the whitelist alone does not make it safe. + + Everything else comes from the _REPORT_HEADERS whitelist, so a recipient + header is absent because it was never named rather than because it was + stripped. + + A Received line naming no parseable IP cannot be placed against the + boundary, so it is dropped rather than guessed at. Publishing an + unplaceable line risks disclosing exactly the two things the cut exists + to prevent; iocs() still records every hop it can read, so nothing is + lost to review, only to the report. + + Returned as a list of pairs rather than a dict because the whitelist may + later keep a header that repeats, and order carries meaning. + """ + message = _message(raw) + result: list[tuple[str, str]] = [] + + for value in message.get_all("received") or []: + ip = _extract_ip(str(value)) + if ip is None: + continue + if _in_any(ip, trusted): + continue + result.append(("Received", _strip_envelope_recipient(str(value)))) + break + + for name in _REPORT_HEADERS: + value = message.get(name) + if value is not None: + result.append((name, str(value))) + + return result + + def _address_of(header_value: str | None) -> str | None: """Return the addr-spec of a sender header, ignoring its display name. diff --git a/tests/fixtures/reportable.eml b/tests/fixtures/reportable.eml new file mode 100644 index 0000000..e939059 --- /dev/null +++ b/tests/fixtures/reportable.eml @@ -0,0 +1,22 @@ +Received: from relay.example.org (relay.example.org [192.0.2.10]) + by mx.example.org with ESMTP id abc123 + for <you@example.org>; Mon, 07 Sep 2026 09:12:44 +0000 +Received: from sender.invalid (sender.invalid [203.0.113.42]) + by relay.example.org with ESMTP id def456 + for <you@example.org>; Mon, 07 Sep 2026 09:12:40 +0000 +Return-Path: <bounce@sender.invalid> +Authentication-Results: mx.example.org; spf=fail; dkim=none; dmarc=fail +Received-SPF: fail (mx.example.org: domain of sender.invalid does not designate 203.0.113.42) +From: "Example Bank" <phish@sender.invalid> +To: victim@example.org +Cc: colleague@example.org +Delivered-To: victim@example.org +X-Original-To: victim@example.org +Reply-To: "Support" <reply@sender.invalid> +Subject: Your account requires verification +Date: Mon, 07 Sep 2026 09:12:40 +0000 +Message-ID: <case-one@sender.invalid> +MIME-Version: 1.0 +Content-Type: text/plain; charset=utf-8 + +Please verify at http://login.sender.invalid/verify?id=abc123 diff --git a/tests/test_cli.py b/tests/test_cli.py index 1c023c3..425bce9 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -145,7 +145,25 @@ class TestParse(unittest.TestCase): str(FIXTURES / "simple.eml"), ) text = (pathlib.Path(out.strip()) / "manifest.json").read_text() - self.assertNotIn("example.org", text) + self.assertNotIn("you@example.org", text) + # The bare domain is still barred everywhere the IOCs live. The + # headers block is the one exception and it is a NARROW one: the + # whitelist publishes the boundary Received line and + # Authentication-Results, and both name our own receiving relay in a + # "by"/authserv-id clause. That is the user's mail host, not the + # user's identity, and a desk learns it from the report's own From + # regardless. The address itself must still be absent, which the + # assertion above and report_headers' own tests cover. + import json + + manifest = json.loads(text) + headers = manifest.pop("headers") + self.assertNotIn("example.org", json.dumps(manifest)) + # And nothing shaped like an address survives in the exception. + self.assertNotIn("@example.org", json.dumps(headers)) + names = [name for name, _ in headers] + for name in ("To", "Cc", "Delivered-To", "X-Original-To"): + self.assertNotIn(name, names) def test_a_missing_config_points_at_init(self): code, _, err = self._run( diff --git a/tests/test_offline.py b/tests/test_offline.py index 19d9cee..547bd96 100644 --- a/tests/test_offline.py +++ b/tests/test_offline.py @@ -53,6 +53,14 @@ class NothingOpensASocket(unittest.TestCase): b"Subject: test\r\n\r\nbody\r\n") parse.iocs(raw, trusted=["192.0.2.0/24"]) + def test_selecting_report_headers_opens_no_socket(self): + # A new entry point into the parse path, so it is held to the same + # guarantee: choosing what to publish resolves nothing. + raw = (b"Received: from relay.example.invalid ([192.0.2.10])\r\n" + b"From: sender@example.invalid\r\n" + b"Subject: test\r\n\r\nbody\r\n") + parse.report_headers(raw, trusted=["192.0.2.0/24"]) + def test_resolving_with_an_injected_fetch_opens_no_socket(self): iocs = [{"id": "ioc-1", "type": "ipv4", "value": "198.51.100.7"}] bootstraps = { diff --git a/tests/test_parse.py b/tests/test_parse.py index 11345c7..fa3526a 100644 --- a/tests/test_parse.py +++ b/tests/test_parse.py @@ -290,5 +290,67 @@ class TestIocAssembly(unittest.TestCase): self.assertNotIn("you%40example.org", blob) +class ReportHeaders(unittest.TestCase): + def test_the_whitelist_keeps_what_a_desk_needs(self): + headers = parse.report_headers(load("reportable.eml"), + trusted=["192.0.2.0/24"]) + names = [name for name, _ in headers] + for wanted in ("From", "Subject", "Date", "Message-ID", "Reply-To", + "Return-Path", "Authentication-Results", "Received-SPF"): + self.assertIn(wanted, names) + + def test_recipient_headers_never_survive_the_whitelist(self): + # The first property, at the one place a report reproduces header + # text verbatim. A blacklist would have to remember each of these; + # the whitelist never names them at all. + headers = parse.report_headers(load("reportable.eml"), + trusted=["192.0.2.0/24"]) + names = [name for name, _ in headers] + blob = repr(headers) + for name in ("To", "Cc", "Delivered-To", "X-Original-To"): + self.assertNotIn(name, names) + self.assertNotIn("victim@example.org", blob) + self.assertNotIn("colleague@example.org", blob) + + def test_received_stops_at_the_boundary_hop(self): + # 192.0.2.10 is ours, so its Received line is our own infrastructure + # and must not be published; the hop below it is the one being + # reported and is kept. + headers = parse.report_headers(load("reportable.eml"), + trusted=["192.0.2.0/24"]) + received = [value for name, value in headers if name == "Received"] + self.assertEqual(len(received), 1) + self.assertIn("203.0.113.42", received[0]) + self.assertNotIn("mx.example.org with ESMTP id abc123", received[0]) + + def test_the_published_hop_carries_no_envelope_recipient(self): + # The boundary Received line is written by OUR OWN relay, and its + # optional "for <addr>" clause is the envelope recipient: the + # victim's address, verbatim, in the one header a report reproduces + # in full. Truncating the chain is not enough on its own. + headers = parse.report_headers(load("reportable.eml"), + trusted=["192.0.2.0/24"]) + received = [value for name, value in headers if name == "Received"] + self.assertNotIn("you@example.org", received[0]) + self.assertNotIn("for <", received[0]) + # The rest of the hop survives; this is a cut, not a blanking. + self.assertIn("203.0.113.42", received[0]) + + def test_a_forged_chain_publishes_no_hop_below_the_boundary(self): + # The same job test_a_forged_chain_stops_at_the_first_untrusted_hop + # does for sending_ip(), asserted over what actually gets published: + # 198.51.100.7 is an innocent party the attacker named. + headers = parse.report_headers(load("forged-chain.eml"), + trusted=["192.0.2.0/24"]) + received = [value for name, value in headers if name == "Received"] + # Asserted in BOTH directions: dropping Received altogether would + # satisfy the "not published" half on its own, and a test that + # passes when the feature is missing protects nothing. + self.assertEqual(len(received), 1) + self.assertIn("203.0.113.99", received[0]) + self.assertTrue(all("198.51.100.7" not in value for value in received)) + self.assertTrue(all("198.51.100.8" not in value for value in received)) + + if __name__ == "__main__": unittest.main() |