diff --git a/AGENTS.md b/AGENTS.md index ecc79900..d7fd89ad 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,7 +78,7 @@ The parser detects an RFC 9990 report from the `urn:ietf:params:xml:ns:dmarc-2.0 RFC 9990's `PolicyOverrideType` enumeration is `{local_policy, mailing_list, other, policy_test_mode, trusted_forwarder}`. `policy_test_mode` is new (emitted when `t=y` suppresses enforcement); `forwarded` and `sampled_out` were removed. Override types are stored as-is and warned about on mismatch. -Several elements (`extra_contact_info`, `error`, `comment`, `human_result`) are `langAttrString` in RFC 9990 — i.e. xs:string with an optional `lang` attribute. When the reporter sends the attribute, xmltodict turns the element into `{"#text": "...", "@lang": "en"}`; the parser unwraps that to a plain string via `_text()`. +Several elements (`extra_contact_info`, `error`, `comment`, `human_result`) are `langAttrString` in RFC 9990 — i.e. xs:string with an optional `lang` attribute. xmltodict would turn such an element into `{"#text": "...", "@lang": "en"}`, but the report is parsed with `xml_attribs=False`, so no attribute reaches the parser, and `_scalar()` coerces every single-valued field the schema types as a string (`extra_contact_info`, `comment`, `human_result` among them) to a string or `None` (a child element's dict gives its `#text`, a repeated element `None`), so no dict or list reaches the outputs in place of a string and changes a field's mapping (GHSA-qpcg-29vr-jwm3). `error` may repeat, so it goes through `_text()` item by item instead: a repeated one stays a list of strings, and `_text()` now only meets dicts made by child elements. Failure reports (RFC 9991): `Identity-Alignment` and `Auth-Failure` are split on CFWS-aware commas (each token stripped per the RFC 9991 ABNF), and a warning is logged when either REQUIRED field is missing. @@ -126,7 +126,7 @@ IP address info cached for 4 hours, seen aggregate report IDs cached for 1 hour ### Regular expressions on untrusted input -Assume every regex in the package runs on content a remote sender controls (message bodies, headers, attachments inflated to the 100 MiB decompression cap), so a pattern that backtracks super-linearly is a remote CPU-exhaustion bug, not a style nit (GHSA-7grx-mj6g-qxmc: a cubic pattern on whitespace; GHSA-4h9r-c435-j834: a quadratic `.*>` that a 194 KB gzip attachment could turn into days of CPU). Before adding or editing a pattern: +Assume every regex in the package runs on content a remote sender controls (message bodies, headers, attachments inflated to the 32 MiB decompression cap), so a pattern that backtracks super-linearly is a remote CPU-exhaustion bug, not a style nit (GHSA-7grx-mj6g-qxmc: a cubic pattern on whitespace; GHSA-4h9r-c435-j834: a quadratic `.*>` that a 194 KB gzip attachment could turn into days of CPU). Before adding or editing a pattern: - **Adjacent quantifiers must not overlap.** `\s*([a-zA-Z\s]+)` lets both parts consume the same whitespace, so every split is retried. Make neighbouring classes disjoint (`[ \t]*` next to `[A-Za-z]`). - **A repeated class must exclude the delimiter it stops on.** Write `\([^()]*\)` and `]*>`, never `\(.*\)` or ``: a greedy `.*` scans to end of line and backtracks for every occurrence, which is quadratic whenever the closer is missing, and also matches past the *first* closer (the old schema-tag regex deleted a whole minified report that way). diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c22bbce..b93222bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## Unreleased + +### Security + +- **Crafted email messages no longer take seconds to minutes and hundreds of megabytes to parse** ([GHSA-qcc2-4x3v-v4g7](https://github.com/domainaware/parsedmarc/security/advisories/GHSA-qcc2-4x3v-v4g7), affects 11.2.1 and earlier). `parse_report_email()` (mailbox, mbox and file input) and the failure-sample parser `parsedmarc.utils.parse_email()` gave the whole message to mail-parser and Python's `email` package with no limit on blank lines, header lines, header field size, MIME parts or nested multiparts. Parsing builds one `Message` per blank line after a `message/delivery-status` part (512 KiB of blank lines took 6.1 s and 283 MiB of extra memory on 11.2.1 under Python 3.12) and tests every line against every open boundary of nested multiparts (900 levels around 30,000 parts, 0.3 MB, took 10.9 s); a 4 MiB flood of header lines took 1.4 s, and a 1 MB `To:` address group of 250,000 members took 1.9 s, or 44 s through mailsuite's own `parse_email()`, which parsed the group whole. With `n_procs = 1` the rest of the mailbox waited behind such a message, and a process with a memory cap was killed before the message was moved, so it was parsed again on the next run. Both parsers now call `mailsuite.utils.check_message_limits()`, new in mailsuite 2.3.5 ([GHSA-748w-c9w9-79xp](https://github.com/seanthegeek/mailsuite/security/advisories/GHSA-748w-c9w9-79xp)), before mail-parser sees the message. It parses the message with the `email` package's own feed parser under the `compat32` policy, the parser mail-parser and `email.message_from_string()` use, and stops as soon as the message has more than 10,000 parts (the message itself, its MIME parts, the bodies of `message/*` parts and each block of a `message/delivery-status` body), nests them more than 100 levels deep, or makes the parser test a line against an open boundary more than 20,000,000 times. It also stops when the message's header blocks hold more than 100,000 lines in all (folded lines and lines the parser keeps only as defects included). It then rejects a parsed message with a header field longer than 65,536 characters (name, colon and raw value, folded lines included), or whose `From`, `Sender`, `Reply-To`, `To`, `Cc`, `Bcc` and `Delivered-To` headers hold more than 262,144 characters in all, counted in every part; that limit bounds mail-parser's own address parsing (it parses `From`, `Reply-To`, `To`, `Cc`, `Bcc` and `Delivered-To`), which does work for each malformed address item. When a part lacks its start boundary, mail-parser parses the top-level epilogue again, and that text is checked with the same limits. `parse_report_email()` also no longer renders every part it walks as text: it renders a part only where it reads it, because rendering each level of a deep nesting rendered everything inside it again (98 nested multiparts around a 20 MiB leaf took 26.6 s, a chain of 99 `message/rfc822` parts around a 10 MiB leaf 11.3 s, and a chain of 99 `message/feedback-report` parts around a 10 MiB leaf 11.6 s, all within the limits; each now takes under a second). An over-limit email raises `InvalidDMARCReport("Message rejected: Message exceeds ()")`, which the mailbox loop files under `Invalid`; an over-limit failure-report sample raises `InvalidFailureReport("Sample message rejected: …")`. Each crafted message from the report is now rejected in under half a second (the 4 MiB header flood, the slowest, in 0.2 s). **mailsuite 2.3.5 or later is now required.** The floor applies to the base dependency and to the `msgraph`, `gmail` and `all` extras. mailsuite 2.3.5 also removes RFC 5322 group syntax from address header values over 8 KiB before parsing them, caps the address header value its own `parse_email()` reads at 65,536 characters, and fixes a quadratic `url_regex`; all only affect callers of mailsuite's `parse_email()`, which parsedmarc does not use. A follow-up issue tracks a per-job timeout in `parallel_map()`. **Compatibility:** the limits are fixed; there is no new option. A message with more than 10,000 parts or parts nested more than 100 levels deep is now rejected as invalid instead of being parsed or, when nested past Python's recursion limit, failing with a `ParserError`; in mbox input such a message (the nested message of GHSA-qxwv-2vwj-v828 is one) is now skipped with a warning instead of stopping the run. A `message/rfc822` attached inside a failure report's sample no longer replaces the sample, since a feedback report has one sample ([RFC 6591 §3.1](https://www.rfc-editor.org/rfc/rfc6591#section-3.1)), and only the first `message/feedback-report` part of a message is read, since a feedback report has one ([RFC 6591 §3](https://www.rfc-editor.org/rfc/rfc6591#section-3)). Reported by helixnebulatech. +- **Deeply nested or very wide aggregate report XML no longer exhausts memory** ([GHSA-q7gm-8cj9-94hr](https://github.com/domainaware/parsedmarc/security/advisories/GHSA-q7gm-8cj9-94hr), affects 11.2.1 and earlier). `parse_aggregate_report_xml()` passed the decompressed report, up to the 100 MiB decompression cap, to `xmltodict.parse()` with no limit on nesting depth or element count. xmltodict keeps one stack entry per open element and expat reports missing end tags only at the end of the input, so a gzip attachment of about 30 KB holding 30 MiB of unclosed `` tags needed more than 2 GB of memory (2 MiB of them cost 255 MiB on 11.2.1). The out-of-memory killer ended the process before the message was moved, so every restart parsed it again. A document of millions of sibling elements did the same at a smaller scale, and a document whose first parse failed was parsed again through lxml recovery and a second xmltodict pass. While expat reads the document, before xmltodict adds each element to its tree, parsedmarc now counts nesting depth and nodes (elements plus attributes); a report deeper than 100 levels (`MAX_XML_DEPTH`) or with more than 1,000,000 nodes (`MAX_XML_NODES`) is rejected as `InvalidAggregateReport("Invalid XML: …")` without trying lxml recovery. Before any parser sees the document, a tag longer than 65,536 characters (`MAX_XML_TAG_LENGTH`) is rejected too, because expat and libxml2 build an element's whole attribute list before its attributes can be counted (one element with 900,000 attributes, 9.3 MiB, within the node limit, took 1.1 s and +200 MiB; it is now rejected in under 0.1 s). Because a quoted attribute value may hold a raw `>`, which splits a tag for that scan, a document with more `=` characters than the node limit is rejected first, in one pass (2,000,000 `x=">"` attributes, 23.7 MiB, took 1.4 s and +310 MiB before the node limit rejected them; they are now rejected in under 0.1 s). The decompression cap for a report attachment is lowered from 100 MiB to 32 MiB (`MAX_DECOMPRESSED_REPORT_SIZE`), and an uncompressed report of more than 32 MiB of UTF-8 is rejected the same way, as is a report file of that size, which is no longer tried as a report email: at the node limit a legitimate report is at most about 28 MB, and with that cap a report's text is bounded along with its node count. When the first parse fails and lxml recovery runs, a parser target that builds no tree first counts the document's elements, attributes and namespace declarations against the same node limit (a 22 MiB wide document behind a malformed XML declaration took 5.0 s and 1,532 MiB of extra memory before, 0.57 s and 23 MiB now), and the recovered XML then goes through the same depth and node limits in the second xmltodict pass. A follow-up issue tracks a persisted failed-attempt marker, so that a message that kills the process before it is filed is moved to `Invalid` after a number of runs. **Compatibility:** there is no new option. A real report is fewer than 10 levels deep and the largest sample (909 KB) has 32,019 nodes, so the node limit is a report of about 28 MB of XML; a larger report is now rejected. A report at the size cap needs about 400 MB of memory on the lxml recovery path and about 350 MB otherwise when its text is parsed directly, and about 500 MB and 450 MB through `parse_report_file()`, which also holds the file's bytes, where a report under the old 100 MiB decompression cap could need gigabytes. An aggregate or SMTP TLS report of more than 32 MiB of UTF-8, compressed or not, is now rejected, where 100 MiB was accepted (an uncompressed SMTP TLS report had no cap: 60 MiB of JSON cost +1,505 MiB). `parse_report_file()` no longer tries a file that begins with `<` as SMTP TLS JSON or a report email, nor one that begins with `{` as a report email; its error message is unchanged, and the exception's cause now matches the format the file begins with. It also keeps only each failed attempt's message while it tries the next format, so a file at the size cap on the lxml recovery path takes +500 MiB there, where it took +1,046 MiB. The XML parse step is about 20% slower, and a malformed document that needs lxml recovery is now parsed twice by lxml (once to count, once to build the tree). Reported by helixnebulatech. +- **An aggregate report with a huge date range can no longer expand into millions of records or fill an Elasticsearch or OpenSearch cluster with indexes** ([GHSA-p85v-578x-3wcf](https://github.com/domainaware/parsedmarc/security/advisories/GHSA-p85v-578x-3wcf), affects 11.2.1 and earlier). An aggregate report whose `` was longer than `normalize_timespan_threshold_hours` was split into one record per calendar day, with no limit on the number of days. A report under 2 KB declaring a range of millions of days with a large `` expanded one `` into millions of records (200,000 days gave 200,000 records in 0.8 s; the reporter's 2.9 million days exhausted 2 GB of memory). With Elasticsearch or OpenSearch output each of those days created its own dated index of one shard until the cluster reached `cluster.max_shards_per_node` (1,000 by default), after which it refused every new index for every application using it. A report whose date range is negative or longer than 366 days (`MAX_REPORT_SPAN_DAYS`) is now rejected with `InvalidAggregateReport`. A report whose split would produce more than 250,000 records (records times days, `MAX_NORMALIZED_RECORDS`) is not split: its records are kept whole over the report's own date range, and one warning is logged (10,000 records over 366 days, each with a large ``, produced 3,670,000 records in 13.5 s and 1.7 GB of memory before). In Elasticsearch and OpenSearch a single report creates at most 31 dated indexes (`MAX_INDEXES_PER_REPORT`): any further records are saved to the report's first index with their own dates, and one warning is logged for the report. Dated index names still come from each report's own dates, so reports with many different dates still create one index per date; data streams (`use_data_streams`) are affected by neither. **Compatibility:** a genuine report spanning more than 366 days is now rejected; a report too large to split has `timespan_requires_normalization: false` and records with `normalized_timespan: false`; there is no new option. Reported by helixnebulatech. +- **One crafted report can no longer make an output fail and stop every run on the same mailbox** ([GHSA-4272-4r8g-6c5h](https://github.com/domainaware/parsedmarc/security/advisories/GHSA-4272-4r8g-6c5h), affects 11.2.1 and earlier). Some reports made a configured output raise an exception that `process_reports()` did not catch. It escaped the mailbox save callback, parsedmarc logged "Mailbox Error" and exited with status 1, and the batch stayed in the mailbox, so every one-shot run failed on the same message, outputs that do not deduplicate received the batch's legitimate reports again on each attempt, and watch mode finally moved the whole batch to `Unsaved`. The triggers were a failure report whose sample has an empty `From:`, `To:` or `Reply-To:` header (a legitimate `To: undisclosed-recipients:;` too) with Elasticsearch or OpenSearch output (`IndexError`); an SMTP TLS report with a field of the wrong type, such as `"policy-string": [1]` with file output (`TypeError` in the CSV writer) or `"start-datetime": 5` with Elasticsearch, OpenSearch, Splunk HEC or PostgreSQL output; and an aggregate report whose policy override `` has no ``, or a `` with an attribute or given twice, with file output (`KeyError`, `TypeError`). Every output destination in `process_reports()` now records any exception as an output error, as the aggregate Elasticsearch and OpenSearch blocks already did (the failure and SMTP TLS Elasticsearch and OpenSearch blocks, every PostgreSQL and Splunk HEC block and the file output lacked that catch-all), so the batch is reported unsaved and `fail_on_output_error` still applies. SMTP TLS reports are now type-checked against RFC 8460 §4.4 and rejected as `InvalidSMTPTLSReport` otherwise: a string for `organization-name`, `report-id`, `policy-domain`, `policy-type`, `result-type` and the optional failure-detail strings, a string or a list of strings for `contact-info`, a list of strings for `policy-string`, an integer or `null` for each session count (`true` and `false` are rejected), and a list of objects for `failure-details`. `start-datetime` and `end-datetime` must convert with the same functions the outputs use and be at most 64 characters long, because dateutil took 12.4 s to fail on a date of 1,000,000 digits. A string holding a lone surrogate (JSON `"\ud800"`), which I-JSON ([RFC 7493 §2.1](https://www.rfc-editor.org/rfc/rfc7493#section-2.1)) forbids, is rejected too: it passed every type check and made printing the results to standard output raise `UnicodeEncodeError`, which `process_reports()` now also records as an output error; a report that cannot be printed to the console (a non-UTF-8 console, a closed pipe) is now recorded as an output error instead of ending the run. A policy override reason's `type` and `comment` are normalised to a string or `null` (an element with an attribute to its text, a repeated element to null), so the CSV writer and the Elasticsearch and OpenSearch saves no longer fail on them. An address header with no address is left out of the Elasticsearch and OpenSearch failure document and of the duplicate query. The Splunk HEC SMTP TLS save raises `SplunkError` on a date it cannot convert, and the PostgreSQL date helper passes a non-string value through unchanged. **Compatibility:** an SMTP TLS report with a field of the wrong type that used to parse is now invalid and goes to `Invalid`; a `policy-string` that is not a list is now rejected (it was silently dropped), while a `null` one is still accepted as absent. A policy override reason without a `type` now has `type: null` and is written as `none` in CSV; one whose `type` has an attribute is stored as its text, and a repeated one as null. Reported by helixnebulatech. +- **One crafted report can no longer change the field types of an Elasticsearch or OpenSearch index and get every later report rejected** ([GHSA-qpcg-29vr-jwm3](https://github.com/domainaware/parsedmarc/security/advisories/GHSA-qpcg-29vr-jwm3), affects the Elasticsearch and OpenSearch outputs in 11.2.1 and earlier, data streams included). Indexes and the data-stream index template were created without field mappings, so the cluster typed each field from the first document that held it, with date detection on, and a dated index is named from the report's own begin date. One crafted report could make `report_id` a `date` field (`2026-11-09`), turn `sample.headers.subject` into an object (with a sample header named `Subject.x`), or use up `index.mapping.total_fields.limit` (1,000 fields) with invented sample header names. Every later legitimate report for that index, or for that data stream until its next rollover, was then rejected with a mapping error, retried and moved to `Unsaved`; this was confirmed against Elasticsearch 8.19.7 and OpenSearch 2.19.3. New indexes and data-stream templates are now created with date detection off and the date fields declared as dates (`@timestamp`, `date_range`, `date_begin` and `date_end`; for failure reports `@timestamp`, `arrival_date` and `sample.date`); when an aggregate report is parsed, every XML attribute is dropped and every field the schema types as a string is stored as a string or null, so a report cannot change a field's type (an element with an attribute or a child element, such as `

none

` or `

1

`, became an object that mapped the field as one). In failure documents `sample.headers` is mapped with `dynamic: false` and indexes only 29 headers, those the shipped dashboards reference plus `cc`; every header stays in `_source` except a `From`, `To` or `Reply-To` header that holds no address (see GHSA-4272-4r8g-6c5h above), and a `.` in a header name is stored as `_`. `create_indexes()` and `create_data_streams()` take a new optional `mappings` argument. **Compatibility:** existing dated indexes are not changed; the protection applies to dated indexes created by 11.2.2 or later, and, because parsedmarc puts the data stream template again in every process, to an existing data stream's next backing index after a rollover. On those, sample headers outside the list are no longer searchable. The new "Field mappings" sections of the Elasticsearch and OpenSearch documentation show how to add one through the `@custom` component template (Elasticsearch data streams), an index template (dated indexes) or the put-mapping API (OpenSearch data streams, repeated after each rollover). For string fields, an aggregate report element with text and a stray child element now yields its text, where it yielded an empty string or an object before: `samples/aggregate/invalid_xml.xml`, whose `bad` lxml recovery turns into mixed content, now gives the header-from domain `bad`. A `` or `` with mixed content is still rejected. This narrows, but does not replace, the explicit mappings proposed in [#782](https://github.com/domainaware/parsedmarc/issues/782). Reported by helixnebulatech. + +### Bug fixes + +- **A failure report's RFC 6591 feedback report and sample can no longer be replaced by legacy report text.** `parse_report_email()` also reads the legacy text format, a `text/plain` part beginning "A message claiming to be from you has failed" that carries the source IP and the sample, and a later such part replaced the `message/feedback-report` part and the sample already read. A `text/plain` part inside the sample, which is the spoofed message's own text, could therefore set the report's Source-IP and sample. Once a `message/feedback-report` part is read, legacy text parts are ignored, and the RFC 6591 part and its sample win over a legacy part that came before it; a legacy part followed by a `message/feedback-report` part with no sample after it (RFC 6591 §3.1 puts the sample after the report part) is now rejected as `InvalidDMARCReport`, where a mix of the two was returned. Parts inside the sample, which are the reported message's own attachments, are no longer read as reports either: an SVG, zipped HTML or `application/json` attachment there made the whole email invalid, and a gzipped aggregate report or SMTP TLS JSON there was returned in place of the failure report. This holds for a second sample part too, which is not read as the sample; a report attached beside the sample, outside it, is still read. +- **SMTP TLS reports keep their MX host patterns** ([#942](https://github.com/domainaware/parsedmarc/issues/942)). RFC 8460 §4.4 keys a policy's MX host patterns `mx-host`; `mx-host-pattern` is the name of the value's type ("a JSON array of strings"), and the example in Appendix B gives a plain string. parsedmarc read only `mx-host-pattern`, so every RFC-conformant report lost the field. `mx-host` is now read first, as a list of strings or a plain string (which becomes a one-element list); `mx-host-pattern` is kept as a fallback, and a plain string there is now kept too. The output key `mx_host_patterns` and its PostgreSQL `TEXT[]` column are unchanged, and `samples/smtp_tls/rfc8460.json` now yields `["*.mail.company-y.example"]`. Reported by kowax. + ## 11.2.1 ### Security diff --git a/docs/source/elasticsearch.md b/docs/source/elasticsearch.md index 1607d9b3..645b9e48 100644 --- a/docs/source/elasticsearch.md +++ b/docs/source/elasticsearch.md @@ -413,6 +413,13 @@ the report's date, such as `dmarc_aggregate-2024-03-31`. That makes it easy to comply with records retention regulations such as GDPR: delete the indexes for the dates you no longer keep. For more information, check out the Elastic guide to [managing time-based indexes efficiently](https://www.elastic.co/blog/managing-time-based-indices-efficiently). +A single aggregate report creates at most 31 dated indexes +(`MAX_INDEXES_PER_REPORT`): when its records would use more than 31 +dated indexes, the later records are saved to the report's first dated +index with their own dates, and a warning is logged. Dated index names still come +from each report's own dates, so reports with many different dates +still create one index per date; data streams (`use_data_streams`) are +affected by neither. Set `use_data_streams = True` in the `[elasticsearch]` section to write to [data streams] instead. Each report type then goes to one data stream @@ -436,7 +443,8 @@ exist yet. The template: reports, which is the report's begin date unless parsedmarc split a report spanning more than a day into daily records; the arrival date for failure reports; and the report's begin date for SMTP TLS reports. - The other fields are mapped dynamically, as in the dated indexes; + The other fields get the same [field mappings](#field-mappings) as the + dated indexes; - includes a component template named `@custom`, such as `dmarc_aggregate@custom`, if one exists. @@ -519,6 +527,36 @@ PUT _data_stream/dmarc_aggregate/_lifecycle } ``` +### Field mappings + +Indexes and data stream templates created by parsedmarc 11.2.2 or later +turn date detection off and declare the date fields (`date_range`, +`date_begin`, `date_end`, `arrival_date`, `@timestamp`, and +`sample.date`), so a report +cannot make a text field such as `report_id` a date and block every +later report for that index. In failure reports, only these sample +headers are indexed: `authentication-results`, `auto-submitted`, `cc`, +`content-transfer-encoding`, `content-type`, `date`, `from`, +`in-reply-to`, `message-id`, `mime-version`, `received`, `references`, +`reply-to`, `return-path`, `subject`, `thread-index`, `thread-topic`, +`to`, `x-auto-response-suppress`, `x-exclaimer-md-config`, +`x-linkedin-fe`, `x-mailer`, `x-ms-exchange-generated-message-source`, +`x-ms-exchange-inbox-rules-loop`, `x-ms-exchange-parent-message-id`, +`x-ms-exchange-transport-fromentityheader`, `x-ms-has-attach`, +`x-ms-tnef-correlator`, and `x-onpremexternalip`. Every header is still +kept in `_source` under its lower-cased name, with any `.` replaced by +`_`, except a `From`, `To` or `Reply-To` header that holds no address, +which is left out of the document. To make another header searchable, +declare it under +`sample.headers.properties` as `text` with a `keyword` sub-field: in the +`dmarc_failure@custom` component template for a data stream, or in an +index template whose `index_patterns` match the dated indexes +(`dmarc_failure-*`). Either applies only to indexes created after it. +Dated indexes created by earlier releases keep their mappings unchanged. +parsedmarc puts the data stream template again in every process +before its first save to the stream, so after upgrading, an existing +stream's next backing index after a rollover gets these mappings too. + ### Switching an existing installation to data streams Dated indexes that already exist are left alone. They keep working next diff --git a/docs/source/opensearch.md b/docs/source/opensearch.md index 41103f92..d259db2e 100644 --- a/docs/source/opensearch.md +++ b/docs/source/opensearch.md @@ -14,6 +14,13 @@ index for each day (or each month, with `monthly_indexes`), named after the report's date, such as `dmarc_aggregate-2024-03-31`. That makes it easy to comply with records retention regulations such as GDPR: delete the indexes for the dates you no longer keep. +A single aggregate report creates at most 31 dated indexes +(`MAX_INDEXES_PER_REPORT`): when its records would use more than 31 +dated indexes, the later records are saved to the report's first dated +index with their own dates, and a warning is logged. Dated index names still come +from each report's own dates, so reports with many different dates +still create one index per date; data streams (`use_data_streams`) are +affected by neither. Set `use_data_streams = True` in the `[opensearch]` section to write to [data streams] instead. Each report type then goes to one data stream @@ -39,7 +46,8 @@ exist yet. The template: reports, which is the report's begin date unless parsedmarc split a report spanning more than a day into daily records; the arrival date for failure reports; and the report's begin date for SMTP TLS reports. - The other fields are mapped dynamically, as in the dated indexes. + The other fields get the same [field mappings](#field-mappings) as the + dated indexes. parsedmarc puts the template again in every process, so changes made to it directly do not last. @@ -109,6 +117,38 @@ was created, whatever dates the reports in it cover. If you need retention by report date, keep using dated indexes and delete them by name. +### Field mappings + +Indexes and data stream templates created by parsedmarc 11.2.2 or later +turn date detection off and declare the date fields (`date_range`, +`date_begin`, `date_end`, `arrival_date`, `@timestamp`, and +`sample.date`), so a report +cannot make a text field such as `report_id` a date and block every +later report for that index. In failure reports, only these sample +headers are indexed: `authentication-results`, `auto-submitted`, `cc`, +`content-transfer-encoding`, `content-type`, `date`, `from`, +`in-reply-to`, `message-id`, `mime-version`, `received`, `references`, +`reply-to`, `return-path`, `subject`, `thread-index`, `thread-topic`, +`to`, `x-auto-response-suppress`, `x-exclaimer-md-config`, +`x-linkedin-fe`, `x-mailer`, `x-ms-exchange-generated-message-source`, +`x-ms-exchange-inbox-rules-loop`, `x-ms-exchange-parent-message-id`, +`x-ms-exchange-transport-fromentityheader`, `x-ms-has-attach`, +`x-ms-tnef-correlator`, and `x-onpremexternalip`. Every header is still +kept in `_source` under its lower-cased name, with any `.` replaced by +`_`, except a `From`, `To` or `Reply-To` header that holds no address, +which is left out of the document. To make another header searchable +in the dated indexes, declare it +under `sample.headers.properties` as `text` with a `keyword` sub-field +in an index template whose `index_patterns` match them +(`dmarc_failure-*`); it applies only to indexes created after it. The +data stream template composes no component template, so for a data +stream add the header to its backing indices with the update mapping +API (`PUT dmarc_failure/_mapping`), and again after each rollover. +Dated indexes created by earlier releases keep their mappings unchanged. +parsedmarc puts the data stream template again in every process +before its first save to the stream, so after upgrading, an existing +stream's next backing index after a rollover gets these mappings too. + ### Switching an existing installation to data streams Dated indexes that already exist are left alone. They keep working next diff --git a/parsedmarc/__init__.py b/parsedmarc/__init__.py index 4cb07094..c4132be6 100644 --- a/parsedmarc/__init__.py +++ b/parsedmarc/__init__.py @@ -39,6 +39,7 @@ import mailparser import xmltodict from expiringdict import ExpiringDict from mailsuite.smtp import send_email +from mailsuite.utils import MessageLimitExceeded, check_message_limits from tqdm import tqdm from parsedmarc.config import ( @@ -50,6 +51,11 @@ from parsedmarc.config import ( from parsedmarc.constants import ( DEFAULT_DNS_MAX_RETRIES, DEFAULT_DNS_TIMEOUT, + MAX_NORMALIZED_RECORDS, + MAX_REPORT_SPAN_DAYS, + MAX_XML_DEPTH, + MAX_XML_NODES, + MAX_XML_TAG_LENGTH, __version__, ) from parsedmarc.log import logger @@ -75,6 +81,7 @@ from parsedmarc.utils import ( get_filename_safe_string, get_ip_address_info, human_timestamp_to_datetime, + human_timestamp_to_unix_timestamp, is_outlook_msg, parse_email, sanitize_for_log, @@ -135,15 +142,18 @@ MAGIC_XML_TAG = b"\x3c" # '<' - XML starting with an element tag (no declaratio MAGIC_JSON = b"\x7b" # Maximum size, in bytes, of the data extracted from one compressed report -# attachment. Real reports compress at roughly 10:1, but the attachment's +# attachment, and of an uncompressed report or report file measured as UTF-8 +# (_exceeds_size_cap()), so a non-ASCII text cannot be four times the cap in +# bytes. Real reports compress at roughly 10:1, but the attachment's # content is chosen by whoever sent it, and deflate (used by both gzip and -# zip) reaches about 1000:1 on degenerate input, so a 100 KB attachment from -# any sender a monitored mailbox accepts could inflate to 100 MB -- and the +# zip) reaches about 1000:1 on degenerate input, so a 32 KB attachment from +# any sender a monitored mailbox accepts could inflate to 32 MB -- and the # decoded str coexists with the decompressed bytes, so the peak is roughly -# twice that. Deliberately not configurable: real DMARC aggregate, failure, -# and SMTP TLS reports are orders of magnitude smaller than 100 MB, so no -# legitimate deployment needs to change it. -MAX_DECOMPRESSED_REPORT_SIZE = 100 * 1024 * 1024 +# twice that. Deliberately not configurable: an aggregate report is a day of +# one reporter's traffic, the largest sample is 909 KB, and the XML node +# limit (MAX_XML_NODES, 1,000,000 nodes at the samples' 35 nodes per KB) +# already rejects a report of more than about 28 MB (GHSA-q7gm-8cj9-94hr). +MAX_DECOMPRESSED_REPORT_SIZE = 32 * 1024 * 1024 # Per-message count of consecutive failed saves, keyed on # ``(reports_folder, str(message_uid))``. Populated only when @@ -233,7 +243,144 @@ class _NoDoctypeExpat: def ParserCreate(*args: Any, **kwargs: Any) -> Any: parser = expat.ParserCreate(*args, **kwargs) parser.StartDoctypeDeclHandler = _raise_doctype_found - return parser + return _LimitedExpatParser(parser) + + +class _XMLLimitExceeded(Exception): + """Raised by :class:`_LimitedExpatParser` as soon as a document goes over + ``MAX_XML_DEPTH`` or ``MAX_XML_NODES``.""" + + +class _LimitedExpatParser: + """Wraps an expat parser so that the element handlers xmltodict installs + count nesting depth and nodes (elements plus attributes), and raise + :class:`_XMLLimitExceeded` over ``MAX_XML_DEPTH`` or ``MAX_XML_NODES``. + + xmltodict keeps one stack entry per open element, so a document of + millions of unclosed ```` tags, or of millions of siblings, could cost + gigabytes before expat reported the error at the end of the input + (GHSA-q7gm-8cj9-94hr). The check runs before xmltodict's handler, so the + tree never grows past the limits. + + pyexpat parser objects cannot be subclassed, and xmltodict assigns its + handlers after ``ParserCreate()`` returns, so this proxy wraps the + handlers in ``__setattr__`` and forwards every other attribute. The + wrappers keep their counts in a :class:`_XMLCounts`, not on the proxy: + a wrapper referring to the proxy would close a reference cycle through + the parser and keep xmltodict's whole tree alive until the cyclic + garbage collector ran, doubling peak memory across the two parses. + """ + + def __init__(self, parser: Any) -> None: + object.__setattr__(self, "_parser", parser) + object.__setattr__(self, "_counts", _XMLCounts()) + + def __getattr__(self, name: str) -> Any: + return getattr(self._parser, name) + + def __setattr__(self, name: str, value: Any) -> None: + if name == "StartElementHandler": + value = _limit_start(value, self._counts) + elif name == "EndElementHandler": + value = _limit_end(value, self._counts) + setattr(self._parser, name, value) + + +class _XMLCounts: + """The open-element depth and node count of one expat parse.""" + + def __init__(self) -> None: + self.depth = 0 + self.nodes = 0 + + +def _limit_start(handler: Callable[..., Any], counts: _XMLCounts) -> Callable[..., Any]: + """Wrap an expat start-element handler so that it counts depth and + nodes in ``counts`` and raises :class:`_XMLLimitExceeded` over the + limits before ``handler`` runs.""" + + def start(name: str, attributes: list[str]) -> Any: + depth = counts.depth + 1 + # xmltodict sets ordered_attributes, so expat passes attributes as + # a flat [name, value, name, value, ...] list. + nodes = counts.nodes + 1 + len(attributes) // 2 + if depth > MAX_XML_DEPTH: + raise _XMLLimitExceeded( + f"elements nested more than {MAX_XML_DEPTH} levels deep" + ) + if nodes > MAX_XML_NODES: + raise _XMLLimitExceeded( + f"more than {MAX_XML_NODES:,} elements and attributes" + ) + counts.depth = depth + counts.nodes = nodes + return handler(name, attributes) + + return start + + +def _limit_end(handler: Callable[..., Any], counts: _XMLCounts) -> Callable[..., Any]: + """Wrap an expat end-element handler so that it decrements the depth in + ``counts``.""" + + def end(name: str) -> Any: + counts.depth -= 1 + return handler(name) + + return end + + +class _LxmlNodeCounter: + """An lxml parser target that counts elements, attributes and namespace + declarations without building a tree, and raises + :class:`_XMLLimitExceeded` past ``MAX_XML_NODES``. + + It runs before the lxml recovery parse, whose tree would otherwise cost + gigabytes for a wide document behind a malformed XML declaration + (GHSA-q7gm-8cj9-94hr). Depth is not counted here: libxml2 stops at 256 + levels without ``huge_tree``, and the second ``xmltodict.parse()`` + enforces ``MAX_XML_DEPTH`` on the recovered document. + """ + + def __init__(self) -> None: + self.nodes = 0 + + def _add(self, count: int) -> None: + self.nodes += count + if self.nodes > MAX_XML_NODES: + raise _XMLLimitExceeded( + f"more than {MAX_XML_NODES:,} elements and attributes" + ) + + def start(self, tag: str, attrib: dict[str, str]) -> None: + self._add(1 + len(attrib)) + + def start_ns(self, prefix: str | None, uri: str) -> None: + # xmltodict, which does not process namespaces, sees each xmlns + # declaration as an attribute. + self._add(1) + + def end(self, tag: str) -> None: + pass + + def close(self) -> int: + return self.nodes + + +def _parse_xml_with_recovery(xml: bytes) -> Any: + """Parse ``xml`` with lxml in recovery mode and return the tree, after a + counting pass that raises :class:`_XMLLimitExceeded` over + ``MAX_XML_NODES``, so the tree is only built for a document within the + limit.""" + etree.parse( + BytesIO(xml), + etree.XMLParser( + recover=True, resolve_entities=False, target=_LxmlNodeCounter() + ), + ) + return etree.parse( + BytesIO(xml), etree.XMLParser(recover=True, resolve_entities=False) + ) def _resolve_config( @@ -317,16 +464,15 @@ def _exc_origin(error: BaseException) -> str: def _text(value: Any) -> str | None: - """Unwrap a possibly-langAttrString value parsed by xmltodict. + """Unwrap a value parsed by xmltodict that may be a dict. RFC 9990 changed several aggregate-report elements (extra_contact_info, - error, comment, human_result) to type ``langAttrString`` — an - xs:simpleContent string with an optional ``lang`` attribute. When the - attribute is present, xmltodict parses the element as - ``{"#text": "...", "@lang": "en"}`` instead of a plain string. Returns - the text payload for both shapes, ``None`` for unset values, and leaves - other scalar shapes untouched so callers can preserve whatever the - reporter sent. + error, comment, human_result) to type ``langAttrString``, a string with + an optional ``lang`` attribute. xmltodict parses the report without + attributes (``xml_attribs=False``), so such an element arrives as a + string; a dict now arises only from child elements, and gives its + ``#text``. Returns ``None`` for unset values and leaves other scalar + shapes untouched. """ if value is None: return None @@ -338,9 +484,9 @@ def _text(value: Any) -> str | None: def _normalize_result_word(value: Any) -> Any: """Lowercase a reporter-supplied enum word; RFC 7489 Appendix C and - RFC 9990 define result/disposition types as lowercase tokens. Non-string - values (e.g. xmltodict dicts from attribute-bearing elements) pass - through unchanged. + RFC 9990 define result/disposition types as lowercase tokens. Its + callers pass strings or ``None`` (``_scalar()`` has already run), and a + non-string passes through unchanged. """ return value.lower() if isinstance(value, str) else value @@ -526,6 +672,71 @@ def _append_parsed_record( records.append(new_rec) +def _scalar(value: Any) -> str | None: + """Return the string an element the schema types as a string should hold. + + A string is returned as it is. A dict (an element with child elements, + possibly beside text) gives its ``#text`` or ``None``; a list (a + repeated element) or any other value gives ``None``. Without this a + child element reached Elasticsearch and OpenSearch as an object and + mapped the field as one (GHSA-qpcg-29vr-jwm3). + """ + if value is None or isinstance(value, str): + return value + if isinstance(value, dict): + text = value.get("#text") + return text if isinstance(text, str) else None + return None + + +def _scalar_fields(mapping: Any, keys: tuple[str, ...]) -> None: + """Replace each of ``keys`` present in ``mapping`` with its + :func:`_scalar` value, in place; a ``mapping`` that is not a dict is + left to the caller's own error handling.""" + if not isinstance(mapping, dict): + return + for key in keys: + if key in mapping: + mapping[key] = _scalar(mapping[key]) + + +_REPORT_METADATA_SCALARS = ( + "org_name", + "email", + "extra_contact_info", + "report_id", + "generator", +) +_POLICY_PUBLISHED_SCALARS = ( + "domain", + "adkim", + "aspf", + "p", + "sp", + "pct", + "fo", + "np", + "testing", + "discovery_method", +) + + +def _scalarize_report(report: dict[str, Any]) -> None: + """Make the report-level fields the schema types as strings strings or + ``None``, in place: ``version``, the ``report_metadata`` strings and + every ``policy_published`` field of the RFC 7489 and RFC 9990 schemas + (other children are not copied to the output). Records are handled by + :func:`_parse_report_record`. Anything that is not a dict is left to + the caller's own error handling.""" + if not isinstance(report, dict): + return + _scalar_fields(report, ("version",)) + _scalar_fields(report.get("report_metadata"), _REPORT_METADATA_SCALARS) + policies = report.get("policy_published") + for policy in policies if isinstance(policies, list) else [policies]: + _scalar_fields(policy, _POLICY_PUBLISHED_SCALARS) + + def _parse_report_record( record: dict[str, Any], *, @@ -547,6 +758,9 @@ def _parse_report_record( """ record = record.copy() new_record: dict[str, Any] = {} + # Fields the schema types as strings become strings or None + # (GHSA-qpcg-29vr-jwm3); see _scalar(). + _scalar_fields(record["row"], ("source_ip",)) if record["row"]["source_ip"] is None: raise ValueError("Source IP address is empty") new_record_source = get_ip_address_info( @@ -565,8 +779,11 @@ def _parse_report_record( psl_overrides_url=config.psl_overrides_url, ) new_record["source"] = new_record_source - new_record["count"] = int(record["row"]["count"]) + # A with no text of its own (child elements only, or repeated) + # becomes "", so int() raises the ValueError of a non-numeric count. + new_record["count"] = int(_scalar(record["row"]["count"]) or "") policy_evaluated = record["row"]["policy_evaluated"].copy() + _scalar_fields(policy_evaluated, ("disposition", "dkim", "spf")) new_policy_evaluated: dict[str, Any] = { "disposition": "none", "dkim": "fail", @@ -600,9 +817,13 @@ def _parse_report_record( else: reasons = [policy_evaluated["reason"]] for reason in reasons: - # `comment` is langAttrString in RFC 9990 — unwrap {"#text": ..., "@lang": ...} - reason["comment"] = _text(reason.get("comment")) - reason_type = reason.get("type") + # `comment` and `type` are strings in both schemas, but a sender can + # give them child elements (a dict) or repeat them (a list), and the + # CSV writer joins them as strings (GHSA-4272-4r8g-6c5h). They follow + # the rule of every other string field: see _scalar(). + reason["comment"] = _scalar(reason.get("comment")) + reason["type"] = _scalar(reason.get("type")) + reason_type = reason["type"] if is_rfc_9990 and reason_type in RFC_7489_REMOVED_POLICY_OVERRIDE_TYPES: logger.warning( "Policy override reason type %r was removed in RFC 9990; " @@ -627,6 +848,9 @@ def _parse_report_record( new_record["identifiers"] = record["identities"].copy() else: new_record["identifiers"] = record["identifiers"].copy() + _scalar_fields( + new_record["identifiers"], ("header_from", "envelope_from", "envelope_to") + ) new_record["auth_results"] = {"dkim": [], "spf": []} if type(new_record["identifiers"]["header_from"]) is str: lowered_from = new_record["identifiers"]["header_from"].lower() @@ -645,6 +869,7 @@ def _parse_report_record( if not isinstance(auth_results["dkim"], list): auth_results["dkim"] = [auth_results["dkim"]] for result in auth_results["dkim"]: + _scalar_fields(result, ("domain", "selector", "result", "human_result")) if "domain" in result and result["domain"] is not None: new_result: dict[str, Any] = {"domain": result["domain"]} if "selector" in result and result["selector"] is not None: @@ -667,6 +892,7 @@ def _parse_report_record( if not isinstance(auth_results["spf"], list): auth_results["spf"] = [auth_results["spf"]] for result in auth_results["spf"]: + _scalar_fields(result, ("domain", "scope", "result", "human_result")) if "domain" in result and result["domain"] is not None: new_result: dict[str, Any] = {"domain": result["domain"]} if "scope" in result and result["scope"] is not None: @@ -722,11 +948,108 @@ def _non_blank(value: Any) -> Any: return value +# RFC 8460 §4.4 gives every SMTP TLS report date as an RFC 3339 date-time, +# which is at most about 35 characters even with a long fractional second. +# dateutil's parser is super-linear on a long run of digits (12.4 s for +# 1,000,000 of them, Python 3.14), so a longer value is rejected unparsed. +_SMTP_TLS_MAX_DATETIME_LENGTH = 64 + + +def _smtp_tls_string(value: Any, field: str) -> str: + """Return ``value`` if it is a string, as RFC 8460 §4.4 requires of + ``field``; raise ``InvalidSMTPTLSReport`` otherwise.""" + if not isinstance(value, str): + raise InvalidSMTPTLSReport( + f"{field} must be a string, not {type(value).__name__}" + ) + _smtp_tls_check_utf8(value, field) + return value + + +def _smtp_tls_check_utf8(value: str, field: str) -> None: + """Raise ``InvalidSMTPTLSReport`` when ``value`` holds a lone surrogate + (JSON ``"\\ud800"``). RFC 8460 §4 reports are I-JSON, whose RFC 7493 + §2.1 forbids surrogate code points, and the outputs cannot encode one.""" + try: + value.encode("utf-8") + except UnicodeEncodeError as error: + raise InvalidSMTPTLSReport(f"{field} is not valid UTF-8 text") from error + + +def _smtp_tls_string_list(value: Any, field: str) -> list[str]: + """Return ``value`` if it is a list of strings (RFC 8460 §4.4's "JSON + array of strings"); raise ``InvalidSMTPTLSReport`` otherwise.""" + if not isinstance(value, list) or not all(isinstance(item, str) for item in value): + raise InvalidSMTPTLSReport(f"{field} must be a list of strings") + for item in value: + _smtp_tls_check_utf8(item, field) + return value + + +def _smtp_tls_mx_host_patterns(value: Any, field: str) -> list[str]: + """Return a policy's MX host patterns as a list of strings. + + RFC 8460 §4.4 describes the value as "a JSON array of strings", but its + Appendix B example gives one pattern as a plain string, so a string + becomes a one-element list. ``null`` and an empty string give an empty + list; any other type raises ``InvalidSMTPTLSReport``. + """ + if value is None or value == "": + return [] + if isinstance(value, str): + return [_smtp_tls_string(value, field)] + return _smtp_tls_string_list(value, field) + + +def _smtp_tls_count(value: Any, field: str) -> int | None: + """Return ``value`` if it is an integer or null. + + RFC 8460 §4.4 encodes each session count as "an integer, encoded as a + JSON number". ``null`` is tolerated as an unreported count. A JSON + ``true``/``false`` is rejected even though ``bool`` subclasses ``int``. + """ + if value is None or (isinstance(value, int) and not isinstance(value, bool)): + return value + raise InvalidSMTPTLSReport( + f"{field} must be an integer, not {type(value).__name__}" + ) + + +def _smtp_tls_datetime(value: Any, field: str) -> str: + """Return ``value`` if it is a date-time string every output can convert. + + RFC 8460 §4.4 gives ``start-datetime`` and ``end-datetime`` as strings in + RFC 3339 format. The value is checked with the conversions the + Elasticsearch, OpenSearch and S3 outputs (a UTC ``datetime``) and the + Splunk output (a UNIX timestamp) make later. dateutil also accepts a + partial date such as ``"2016"``, which PostgreSQL may still reject; that + failure is raised as ``PostgreSQLError`` and recorded as an output + error. Raises ``InvalidSMTPTLSReport`` otherwise. + """ + value = _smtp_tls_string(value, field) + if len(value) > _SMTP_TLS_MAX_DATETIME_LENGTH: + raise InvalidSMTPTLSReport( + f"{field} is longer than {_SMTP_TLS_MAX_DATETIME_LENGTH} characters" + ) + try: + human_timestamp_to_datetime(value, to_utc=True) + human_timestamp_to_unix_timestamp(value) + except (ValueError, OverflowError) as e: + raise InvalidSMTPTLSReport( + f"{field} is not a valid date-time: {sanitize_for_log(value)}" + ) from e + return value + + def _parse_smtp_tls_failure_details(failure_details: dict[str, Any]): try: new_failure_details: dict[str, Any] = { - "result_type": failure_details["result-type"], - "failed_session_count": failure_details["failed-session-count"], + "result_type": _smtp_tls_string( + failure_details["result-type"], "result-type" + ), + "failed_session_count": _smtp_tls_count( + failure_details["failed-session-count"], "failed-session-count" + ), } optional_fields = [ @@ -739,7 +1062,7 @@ def _parse_smtp_tls_failure_details(failure_details: dict[str, Any]): for src_key, dest_key in optional_fields: value = _non_blank(failure_details.get(src_key)) if value is not None: - new_failure_details[dest_key] = value + new_failure_details[dest_key] = _smtp_tls_string(value, src_key) # RFC 8460 §4.4 names the JSON key "additional-information" (its # value is described as an "additional-info-uri"). Some reporters @@ -750,7 +1073,9 @@ def _parse_smtp_tls_failure_details(failure_details: dict[str, Any]): if additional_info_uri is None: additional_info_uri = _non_blank(failure_details.get("additional-info-uri")) if additional_info_uri is not None: - new_failure_details["additional_info_uri"] = additional_info_uri + new_failure_details["additional_info_uri"] = _smtp_tls_string( + additional_info_uri, "additional-information" + ) return new_failure_details @@ -765,8 +1090,10 @@ def _parse_smtp_tls_failure_details(failure_details: dict[str, Any]): def _parse_smtp_tls_report_policy(policy: dict[str, Any]): policy_types = ["tlsa", "sts", "no-policy-found"] try: - policy_domain = policy["policy"]["policy-domain"] - policy_type = policy["policy"]["policy-type"] + policy_domain = _smtp_tls_string( + policy["policy"]["policy-domain"], "policy-domain" + ) + policy_type = _smtp_tls_string(policy["policy"]["policy-type"], "policy-type") failure_details = [] if policy_type not in policy_types: raise InvalidSMTPTLSReport( @@ -776,22 +1103,38 @@ def _parse_smtp_tls_report_policy(policy: dict[str, Any]): "policy_domain": policy_domain, "policy_type": policy_type, } - if "policy-string" in policy["policy"]: - if isinstance(policy["policy"]["policy-string"], list): - if len(policy["policy"]["policy-string"]) > 0: - new_policy["policy_strings"] = policy["policy"]["policy-string"] + if policy["policy"].get("policy-string") is not None: + policy_strings = _smtp_tls_string_list( + policy["policy"]["policy-string"], "policy-string" + ) + if len(policy_strings) > 0: + new_policy["policy_strings"] = policy_strings - if "mx-host-pattern" in policy["policy"]: - if isinstance(policy["policy"]["mx-host-pattern"], list): - if len(policy["policy"]["mx-host-pattern"]) > 0: - new_policy["mx_host_patterns"] = policy["policy"]["mx-host-pattern"] - new_policy["successful_session_count"] = policy["summary"][ - "total-successful-session-count" - ] - new_policy["failed_session_count"] = policy["summary"][ - "total-failure-session-count" - ] + # RFC 8460 §4.4 keys the policy's MX host patterns "mx-host"; + # "mx-host-pattern" names its value type, not a key, but was read as + # the key before and is kept as a fallback (issue #942). + mx_host_patterns = _smtp_tls_mx_host_patterns( + policy["policy"].get("mx-host"), "mx-host" + ) + if not mx_host_patterns: + mx_host_patterns = _smtp_tls_mx_host_patterns( + policy["policy"].get("mx-host-pattern"), "mx-host-pattern" + ) + if mx_host_patterns: + new_policy["mx_host_patterns"] = mx_host_patterns + new_policy["successful_session_count"] = _smtp_tls_count( + policy["summary"]["total-successful-session-count"], + "total-successful-session-count", + ) + new_policy["failed_session_count"] = _smtp_tls_count( + policy["summary"]["total-failure-session-count"], + "total-failure-session-count", + ) if "failure-details" in policy: + if not isinstance(policy["failure-details"], list) or not all( + isinstance(details, dict) for details in policy["failure-details"] + ): + raise InvalidSMTPTLSReport("failure-details must be a list of objects") for details in policy["failure-details"]: failure_details.append(_parse_smtp_tls_failure_details(details)) new_policy["failure_details"] = failure_details @@ -814,6 +1157,10 @@ def parse_smtp_tls_report_json(report: str | bytes) -> SMTPTLSReport: "policies", ] + if _exceeds_size_cap(report): + # json.loads() holds the whole document at several times its size + # (GHSA-q7gm-8cj9-94hr). + raise InvalidSMTPTLSReport(_size_cap_text(report)) try: if isinstance(report, bytes): report = report.decode("utf-8", errors="replace") @@ -829,12 +1176,26 @@ def parse_smtp_tls_report_json(report: str | bytes) -> SMTPTLSReport: for policy in report_dict["policies"]: policies.append(_parse_smtp_tls_report_policy(policy)) + # RFC 8460 §4.4 gives contact-info as one string; some reporters + # send a list of them, which is kept as it is. + contact_info = report_dict["contact-info"] + if isinstance(contact_info, str): + contact_info = _smtp_tls_string(contact_info, "contact-info") + else: + contact_info = _smtp_tls_string_list(contact_info, "contact-info") + new_report: SMTPTLSReport = { - "organization_name": report_dict["organization-name"], - "begin_date": report_dict["date-range"]["start-datetime"], - "end_date": report_dict["date-range"]["end-datetime"], - "contact_info": report_dict["contact-info"], - "report_id": report_dict["report-id"], + "organization_name": _smtp_tls_string( + report_dict["organization-name"], "organization-name" + ), + "begin_date": _smtp_tls_datetime( + report_dict["date-range"]["start-datetime"], "start-datetime" + ), + "end_date": _smtp_tls_datetime( + report_dict["date-range"]["end-datetime"], "end-datetime" + ), + "contact_info": contact_info, + "report_id": _smtp_tls_string(report_dict["report-id"], "report-id"), "policies": policies, } @@ -965,6 +1326,267 @@ def parsed_smtp_tls_reports_to_csv( return csv_file_object.getvalue() +# No field of an aggregate report carries an attribute parsedmarc keeps, and +# an attribute xmltodict kept (

none

as {"@a": "1", "#text": +# "none"}) would reach Elasticsearch or OpenSearch as an object and map the +# field as one (GHSA-qpcg-29vr-jwm3). With xml_attribs=False xmltodict builds +# no attribute keys, while expat still hands the attributes to the start +# handler, so _LimitedExpatParser counts them. +def _parse_limited_xml(xml: str) -> Any: + """Parse ``xml`` with xmltodict through the limited expat parser, without + attributes, and raise ``InvalidAggregateReport`` when it goes over + ``MAX_XML_DEPTH`` or ``MAX_XML_NODES``.""" + try: + return xmltodict.parse(xml, expat=_NoDoctypeExpat, xml_attribs=False) + except _XMLLimitExceeded as error: + raise InvalidAggregateReport(f"Invalid XML: {error}") from error + + +def _recover_xml(xml: str) -> str: + """Return ``xml`` repaired by lxml's recovery parser, for a document + expat rejected: ``""`` when nothing is recoverable, ``""`` when the + recovery itself fails. Raises ``InvalidAggregateReport`` for a document + over the node limit or holding a DOCTYPE.""" + has_doctype = False + try: + tree = _parse_xml_with_recovery(xml.encode("utf-8")) + root = tree.getroot() + if root is None: + # Nothing recoverable; the second parse reports it as + # invalid XML. + return "" + # The first parse can fail before expat reaches a DOCTYPE (e.g. on + # a malformed XML declaration, which is only rewritten later), so + # the recovered tree gets the same DOCTYPE check, made before + # serializing. The rejection is raised after this try, which would + # otherwise swallow it. + has_doctype = bool(tree.docinfo.doctype) or ( + tree.docinfo.internalDTD is not None + ) + if not has_doctype: + # Serializing the root element rather than the tree leaves any + # internal DTD subset out of the XML handed to the second parse. + # str() is a no-op at runtime; lxml ships no type stubs, so + # pyright would widen the result to str | bytes. + return str(etree.tostring(root, encoding="unicode")) + except _XMLLimitExceeded as limit: + raise InvalidAggregateReport(f"Invalid XML: {limit}") from limit + except Exception: + return "" + raise InvalidAggregateReport(_DOCTYPE_ERROR) + + +def _size_cap_text(content: str | bytes) -> str: + """Return the reason a report over the size cap is rejected: bytes are + measured as they are, text as UTF-8.""" + size = f"{MAX_DECOMPRESSED_REPORT_SIZE // 2**20} MiB" + if isinstance(content, bytes): + return f"the report is larger than {size}" + return f"the report is larger than {size} of UTF-8" + + +def _exceeds_size_cap(content: str | bytes) -> bool: + """Return whether ``content`` is more than ``MAX_DECOMPRESSED_REPORT_SIZE`` + bytes, a ``str`` measured as UTF-8, as the cap on a compressed report + measures its decompressed bytes. ASCII text and text too long or too + short to matter are decided by length; other text is encoded 1 MiB of + characters at a time, so the measure needs no full copy.""" + cap = MAX_DECOMPRESSED_REPORT_SIZE + if isinstance(content, bytes) or len(content) > cap: + return len(content) > cap + # A character is at most 4 bytes of UTF-8. + if len(content) <= cap // 4 or content.isascii(): + return False + size = 0 + for offset in range(0, len(content), 2**20): + size += len(content[offset : offset + 2**20].encode("utf-8", "surrogatepass")) + if size > cap: + return True + return False + + +def _report_text(xml: str | bytes) -> str: + """Return the report as text, after :func:`_check_xml_text`; bytes over + the size cap are rejected before they are decoded (errors ignored).""" + if isinstance(xml, bytes): + if _exceeds_size_cap(xml): + raise InvalidAggregateReport(f"Invalid XML: {_size_cap_text(xml)}") + xml = xml.decode(errors="ignore") + _check_xml_text(xml) + return xml + + +def _check_xml_text(xml: str) -> None: + """Reject, before any parser reads it, an aggregate report whose text + alone would make parsing it too costly (GHSA-q7gm-8cj9-94hr): more than + ``MAX_DECOMPRESSED_REPORT_SIZE`` bytes of UTF-8 (an uncompressed + attachment or file is held to the cap a compressed one is), more + attributes than ``MAX_XML_NODES``, or a tag longer than + ``MAX_XML_TAG_LENGTH``. Raises ``InvalidAggregateReport``.""" + if _exceeds_size_cap(xml): + raise InvalidAggregateReport(f"Invalid XML: {_size_cap_text(xml)}") + # The = count is one C pass, so it runs before the slower tag scan. + _check_attribute_count(xml) + _check_tag_lengths(xml) + + +def _check_tag_lengths(xml: str) -> None: + """Raise ``InvalidAggregateReport`` when any text between a ``<`` and + the next ``>`` is longer than ``MAX_XML_TAG_LENGTH``, before a parser + builds the tag's attribute list (GHSA-q7gm-8cj9-94hr). + + One linear pass without a regex; the text after the last ``>`` is + checked too, so an unterminated tag is caught. A comment or CDATA + section is treated as a tag, which is harmless at this length. XML + allows a raw ``>`` inside a quoted attribute value, which ends a piece + early, so this scan does not bound how many attributes a tag holds; + :func:`_check_attribute_count` does. + """ + # The text is split on ">" one chunk at a time: one split of the whole + # text made a string per tag, +1,318 MiB for 100 MiB of "". A + # chunk's pieces are measured with max() before any is examined, and + # the unfinished last piece is carried into the next chunk from its + # last "<", so memory stays near one chunk. + carry = "" + for offset in range(0, len(xml), _TAG_SCAN_CHUNK): + pieces = (carry + xml[offset : offset + _TAG_SCAN_CHUNK]).split(">") + carry = pieces.pop() + if max(map(len, pieces), default=0) > MAX_XML_TAG_LENGTH: + for piece in pieces: + _check_tag_piece(piece) + carry = _check_tag_piece(carry) + + +def _check_attribute_count(xml: str) -> None: + """Raise ``InvalidAggregateReport`` when ``xml`` holds more ``=`` + characters than ``MAX_XML_NODES``: every attribute needs one, and + expat and libxml2 build a tag's attribute list before the node counter + sees it (GHSA-q7gm-8cj9-94hr). ``str.count`` is one linear pass.""" + if xml.count("=") > MAX_XML_NODES: + raise InvalidAggregateReport( + f"Invalid XML: more than {MAX_XML_NODES:,} `=` characters, so more " + "attributes than the node limit allows" + ) + + +def _check_tag_piece(piece: str) -> str: + """Raise the tag-length error when the text of ``piece`` from its last + ``<`` is longer than ``MAX_XML_TAG_LENGTH``; return that text, or ``""`` + when ``piece`` holds no ``<``.""" + start = piece.rfind("<") + if start < 0: + return "" + if len(piece) - start > MAX_XML_TAG_LENGTH: + raise InvalidAggregateReport( + f"Invalid XML: a tag is longer than {MAX_XML_TAG_LENGTH:,} characters" + ) + return piece[start:] + + +_TAG_SCAN_CHUNK = 1 << 16 + + +def _check_report_span(span_seconds: int) -> None: + """Raise ``InvalidAggregateReport`` when a report's date range is negative + or longer than ``MAX_REPORT_SPAN_DAYS``, before normalization splits it + into one record per day (GHSA-p85v-578x-3wcf). The message holds no + number from the report, whose dates can be thousands of digits long.""" + if span_seconds < 0: + raise InvalidAggregateReport("Report date range is negative") + if span_seconds > MAX_REPORT_SPAN_DAYS * 86400: + raise InvalidAggregateReport( + f"Report date range spans more than {MAX_REPORT_SPAN_DAYS} days" + ) + + +def _days_touched(begin: datetime, end: datetime) -> int: + """Return how many calendar days of ``begin``'s timezone the interval + [begin, end) touches: the number of buckets ``_bucket_interval_by_day()`` + makes, before it drops the buckets that receive no messages.""" + day_start = datetime(begin.year, begin.month, begin.day, tzinfo=begin.tzinfo) + return max(0, -(-int((end - day_start).total_seconds()) // 86400)) + + +def _normalize_within_limit( + normalize: bool, + xml_records: Any, + begin_dt: datetime, + end_dt: datetime, + report_id: str, +) -> bool: + """Return whether per-day normalization stays enabled for a report. + + Normalization makes one record per record and day, so a report whose + records times days exceed ``MAX_NORMALIZED_RECORDS`` keeps its records + whole, over the report's own range, and one warning is logged + (GHSA-p85v-578x-3wcf). + """ + if not normalize: + return False + record_count = len(xml_records) if isinstance(xml_records, list) else 1 + days = _days_touched(begin_dt, end_dt) + if record_count * days <= MAX_NORMALIZED_RECORDS: + return True + logger.warning( + f"Report {sanitize_for_log(str(report_id))} is not normalized to daily " + f"records: {record_count} records over {days} days would make " + f"{record_count * days} records, more than {MAX_NORMALIZED_RECORDS}" + ) + return False + + +def _parse_report_records( + xml_records: Any, + *, + config: ParserConfig, + is_rfc_9990: bool, + begin_dt: datetime, + end_dt: datetime, + normalize: bool, + keep_alive: Callable | None, +) -> list[dict[str, Any]]: + """Parse an aggregate report's ```` elements (a list, or one + record) into output records, split per day when ``normalize`` is set. + A record of a list that fails to parse is skipped with a warning.""" + records: list[dict[str, Any]] = [] + if type(xml_records) is list: + for i in range(len(xml_records)): + if keep_alive is not None and i > 0 and i % 20 == 0: + logger.debug("Sending keepalive cmd") + keep_alive() + logger.debug(f"Processed {i}/{len(xml_records)}") + try: + report_record = _parse_report_record( + xml_records[i], + config=config, + is_rfc_9990=is_rfc_9990, + ) + _append_parsed_record( + parsed_record=report_record, + records=records, + begin_dt=begin_dt, + end_dt=end_dt, + normalize=normalize, + ) + except Exception as e: + logger.warning(f"Could not parse record: {sanitize_for_log(str(e))}") + + else: + report_record = _parse_report_record( + xml_records, + config=config, + is_rfc_9990=is_rfc_9990, + ) + _append_parsed_record( + parsed_record=report_record, + records=records, + begin_dt=begin_dt, + end_dt=end_dt, + normalize=normalize, + ) + return records + + def parse_aggregate_report_xml( xml: str | bytes, *, @@ -1020,8 +1642,7 @@ def parse_aggregate_report_xml( ) errors = [] # Parse XML and recover from errors - if isinstance(xml, bytes): - xml = xml.decode(errors="ignore") + xml = _report_text(xml) # Detect the XML namespace before any rewriting strips it. The dmarc-2.0 # namespace is one of the indicators for an RFC 9990 report but it is @@ -1037,42 +1658,23 @@ def parse_aggregate_report_xml( if namespace_match: xml_namespace = namespace_match.group(1) + first_error: str | None = None try: - xmltodict.parse(xml, expat=_NoDoctypeExpat)["feedback"] + xmltodict.parse(xml, expat=_NoDoctypeExpat, xml_attribs=False)["feedback"] except _XMLDoctypeFound as e: raise InvalidAggregateReport(_DOCTYPE_ERROR) from e + except _XMLLimitExceeded as e: + # Rejected outright: the lxml recovery below would be a second pass + # over the same oversized document. + raise InvalidAggregateReport(f"Invalid XML: {e}") from e except Exception as e: - errors.append(f"Invalid XML: {e.__str__()}") - has_doctype = False - try: - tree = etree.parse( - BytesIO(xml.encode("utf-8")), - etree.XMLParser(recover=True, resolve_entities=False), - ) - root = tree.getroot() - if root is None: - # Nothing recoverable; the second parse reports it as - # invalid XML. - xml = "" - else: - # The first parse can fail before expat reaches a DOCTYPE - # (e.g. on a malformed XML declaration, which is only - # rewritten below), so the recovered tree gets the same - # DOCTYPE check, made before serializing. The rejection is - # raised after this try, which would otherwise swallow it. - has_doctype = bool(tree.docinfo.doctype) or ( - tree.docinfo.internalDTD is not None - ) - if not has_doctype: - # Serializing the root element rather than the tree - # leaves any internal DTD subset out of the XML handed to - # the second parse. str() is a no-op at runtime; lxml ships - # no type stubs, so pyright would widen xml to str | bytes. - xml = str(etree.tostring(root, encoding="unicode")) - except Exception: - xml = "" - if has_doctype: - raise InvalidAggregateReport(_DOCTYPE_ERROR) from e + first_error = f"Invalid XML: {e.__str__()}" + # The recovery runs after the except block, not inside it: while a + # handler runs, the handled exception's traceback keeps the failed + # parse's frames, and with them its partial tree, alive. + if first_error is not None: + errors.append(first_error) + xml = _recover_xml(xml) try: # Replace XML header (sometimes they are invalid) @@ -1081,19 +1683,15 @@ def parse_aggregate_report_xml( # Remove invalid schema tags xml = xml_schema_regex.sub("", xml) - report = xmltodict.parse(xml, expat=_NoDoctypeExpat)["feedback"] + report = _parse_limited_xml(xml)["feedback"] + _scalarize_report(report) report_metadata = report["report_metadata"] - # is xs:string in both RFC 7489 and RFC 9990, but defensive - # parsing in the wild: some reporters emit it with an xml:lang or - # similar attribute, which xmltodict turns into a dict. - if isinstance(report_metadata.get("email"), dict): - unwrapped = _text(report_metadata["email"]) - if unwrapped is None: - logger.debug( - "Discarding malformed in report_metadata: %r", - report_metadata["email"], - ) - report_metadata["email"] = unwrapped + # The report_metadata strings are strings or None here: + # xmltodict kept no xml:lang attribute (xml_attribs=False) and + # _scalarize_report() any child element. Text in place of the + # section's elements is a missing section. + if not isinstance(report_metadata, dict): + raise AttributeError("report_metadata holds no elements") schema = "draft" if "version" in report: schema = report["version"] @@ -1119,7 +1717,7 @@ def parse_aggregate_report_xml( new_report_metadata["org_name"] = org_name new_report_metadata["org_email"] = report_metadata["email"] # extra_contact_info is langAttrString in RFC 9990 (xs:string in - # RFC 7489) — unwrap {"#text": ..., "@lang": ...} if present. + # RFC 7489); its lang attribute is dropped when the report is parsed. extra = _text(report_metadata.get("extra_contact_info")) new_report_metadata["org_extra_contact_info"] = extra new_report_metadata["report_id"] = report_metadata["report_id"] @@ -1131,6 +1729,7 @@ def parse_aggregate_report_xml( begin_ts = int(date_range["begin"].split(".")[0]) end_ts = int(date_range["end"].split(".")[0]) span_seconds = end_ts - begin_ts + _check_report_span(span_seconds) normalize_timespan = ( span_seconds > cfg.normalize_timespan_threshold_hours * 3600 @@ -1160,13 +1759,11 @@ def parse_aggregate_report_xml( raw_errors = [raw_errors] errors = [text for text in (_text(e) for e in raw_errors) if text] new_report_metadata["errors"] = errors - # is a plain xs:string in RFC 9990 but apply _text() so - # a malformed reporter that decorates it with attributes still - # yields a string instead of breaking downstream consumers. + # is a plain xs:string in RFC 9990; attributes and child + # elements are already removed, and _text() keeps it a string. generator = _text(report_metadata.get("generator")) new_report_metadata["generator"] = generator new_report["report_metadata"] = new_report_metadata - records = [] policy_published = report["policy_published"] if type(policy_published) is list: policy_published = policy_published[0] @@ -1238,48 +1835,29 @@ def parse_aggregate_report_xml( new_policy_published["discovery_method"] = discovery_method new_report["policy_published"] = new_policy_published - if type(report["record"]) is list: - for i in range(len(report["record"])): - if keep_alive is not None and i > 0 and i % 20 == 0: - logger.debug("Sending keepalive cmd") - keep_alive() - logger.debug("Processed {}/{}".format(i, len(report["record"]))) - try: - report_record = _parse_report_record( - report["record"][i], - config=cfg, - is_rfc_9990=is_rfc_9990, - ) - _append_parsed_record( - parsed_record=report_record, - records=records, - begin_dt=begin_dt, - end_dt=end_dt, - normalize=normalize_timespan, - ) - except Exception as e: - logger.warning( - f"Could not parse record: {sanitize_for_log(str(e))}" - ) + normalize_timespan = _normalize_within_limit( + normalize_timespan, report["record"], begin_dt, end_dt, report_id + ) + new_report_metadata["timespan_requires_normalization"] = normalize_timespan - else: - report_record = _parse_report_record( - report["record"], - config=cfg, - is_rfc_9990=is_rfc_9990, - ) - _append_parsed_record( - parsed_record=report_record, - records=records, - begin_dt=begin_dt, - end_dt=end_dt, - normalize=normalize_timespan, - ) + records = _parse_report_records( + report["record"], + config=cfg, + is_rfc_9990=is_rfc_9990, + begin_dt=begin_dt, + end_dt=end_dt, + normalize=normalize_timespan, + keep_alive=keep_alive, + ) new_report["records"] = records return cast(AggregateReport, new_report) + except InvalidAggregateReport: + # Already specific (an XML or date range limit); do not re-wrap it + # as an "Unexpected error" below. + raise except expat.ExpatError as error: raise InvalidAggregateReport(f"Invalid XML: {error.__str__()}") from error @@ -1425,7 +2003,13 @@ def extract_report(content: bytes | str | BinaryIO) -> str: or header[: len(MAGIC_XML_TAG)] == MAGIC_XML_TAG or header[: len(MAGIC_JSON)] == MAGIC_JSON ): - report = file_object.read().decode(errors="ignore") + # An uncompressed report is held to the decompressed cap, read + # at most one byte past it rather than decoded whole. + limit = MAX_DECOMPRESSED_REPORT_SIZE + data = file_object.read(limit + 1) + if len(data) > limit: + raise ParserError(f"Report exceeds the {limit} byte limit") + report = data.decode(errors="ignore") else: raise ParserError("Not a valid zip, gzip, json, or xml file") @@ -1616,13 +2200,13 @@ def parsed_aggregate_reports_to_csv_rows( row["disposition"] = record["policy_evaluated"]["disposition"] policy_override_reasons = list( map( - lambda r_: r_["type"] or "none", + lambda r_: r_.get("type") or "none", record["policy_evaluated"]["policy_override_reasons"], ) ) policy_override_comments = list( map( - lambda r_: r_["comment"] or "none", + lambda r_: r_.get("comment") or "none", record["policy_evaluated"]["policy_override_reasons"], ) ) @@ -1952,6 +2536,9 @@ def parse_failure_report( except KeyError as error: raise InvalidFailureReport(f"Missing value: {error.__str__()}") from error + except MessageLimitExceeded as error: + raise InvalidFailureReport(f"Sample message rejected: {error}") from error + except Exception as error: raise InvalidFailureReport( f"Unexpected error: {error.__str__()}{_exc_origin(error)}" @@ -2222,6 +2809,9 @@ def parse_report_email( else: input_str = input_data + # Reject a message whose structure makes parsing slow before any + # parser builds its tree (GHSA-qcc2-4x3v-v4g7) + check_message_limits(input_str) msg = mailparser.parse_from_string(input_str) msg_headers = json.loads(msg.headers_json) if "Date" in msg_headers: @@ -2229,12 +2819,16 @@ def parse_report_email( date = email.utils.format_datetime(msg_date) msg = email.message_from_string(input_str) + except MessageLimitExceeded as e: + raise InvalidDMARCReport(f"Message rejected: {e}") from e except Exception as e: raise ParserError(e.__str__() + _exc_origin(e)) from e subject_text = "None" feedback_report = None smtp_tls_report = None sample = None + # id() of every sample candidate and every part inside one + sample_parts: set[int] = set() is_feedback_report: bool = False if "From" in msg_headers: logger.info( @@ -2255,17 +2849,45 @@ def parse_report_email( # mailbox batch (GHSA-qxwv-2vwj-v828). try: for part in msg.walk(): + if id(part) in sample_parts: + # The sample is the reported message, spoofer's content + # included; an attachment inside it is not a report. + continue content_type = part.get_content_type().lower() payload_obj = part.get_payload() if not isinstance(payload_obj, list): payload_obj = [payload_obj] + if content_type.startswith("multipart/") or content_type == "text/html": + continue + # A part is serialized only where its text is used: str() of a + # composite part renders its whole subtree, and walk() visits + # every level, so serializing each one was quadratic in the + # nesting (98 multiparts around a 20 MiB leaf took 26.6 s; + # GHSA-qcc2-4x3v-v4g7). + if is_feedback_report and content_type in EMAIL_SAMPLE_CONTENT_TYPES: + # A feedback report has one sample (RFC 6591 §3.1); a message + # nested inside it belongs to it and must not replace it. + if sample is None: + sample = _decode_mime_payload(part, str(payload_obj[0])) + # A later candidate is not read as the sample, and neither + # are the parts inside it. + sample_parts.update(id(sample_part) for sample_part in part.walk()) + continue + if content_type == "message/feedback-report" and is_feedback_report: + # A feedback report has one message/feedback-report part + # (RFC 6591 §3); a nested one must not be rendered again. + continue + if content_type != "message/feedback-report" and isinstance( + payload_obj[0], email.message.Message + ): + # Another message/* body; walk() visits its parts. + continue payload = str(payload_obj[0]) - if content_type.startswith("multipart/"): - continue - if content_type == "text/html": - continue - elif content_type == "message/feedback-report": + if content_type == "message/feedback-report": is_feedback_report = True + # The RFC 6591 part wins over a legacy text report read + # earlier; its sample follows it. + sample = None decoded_payload = _decode_mime_payload(part, payload) try: if "Feedback-Type" in decoded_payload: @@ -2277,8 +2899,6 @@ def parse_report_email( feedback_report = feedback_report.replace("\\n", "\n") except (ValueError, TypeError, binascii.Error): feedback_report = decoded_payload - elif is_feedback_report and content_type in EMAIL_SAMPLE_CONTENT_TYPES: - sample = _decode_mime_payload(part, payload) elif content_type == "application/tlsrpt+json": if not payload.strip().startswith("{"): payload = b64decode(payload).decode("utf-8", errors="replace") @@ -2289,7 +2909,14 @@ def parse_report_email( smtp_tls_report = parse_smtp_tls_report_json(payload) return {"report_type": "smtp_tls", "report": smtp_tls_report} elif content_type == "text/plain": - if "A message claiming to be from you has failed" in payload: + # A legacy text report has no message/feedback-report part. + # Once one is read, a text part (which may sit inside the + # sample, the spoofer's own text) must not replace the + # report's Source-IP or its sample. + if ( + not is_feedback_report + and "A message claiming to be from you has failed" in payload + ): try: parts = payload.split("detected.", 1) field_matches = text_report_regex.findall(parts[0]) @@ -2394,11 +3021,21 @@ def _looks_like_email(text: str) -> bool: return _email_header_regex.match(first_line) is not None +def _leading_text(content: str | bytes) -> str: + """Return the start of ``content`` (512 characters at most) without + leading whitespace, to sniff its format.""" + if isinstance(content, (bytes, bytearray, memoryview)): + sniff = bytes(content)[:512].decode("utf-8", errors="replace") + else: + sniff = content[:512] + return sniff.lstrip() + + def _describe_parse_failure( content: str | bytes, - aggregate_error: InvalidAggregateReport, - smtp_tls_error: InvalidSMTPTLSReport, - email_error: InvalidDMARCReport, + aggregate_error: str, + smtp_tls_error: str, + email_error: str, ) -> str: """Builds a human-readable reason for a parse_report_file failure. @@ -2409,11 +3046,7 @@ def _describe_parse_failure( "not an email". Sniff the leading non-whitespace byte to surface the single relevant reason. """ - if isinstance(content, (bytes, bytearray, memoryview)): - sniff = bytes(content)[:512].decode("utf-8", errors="replace") - else: - sniff = content[:512] - sniff = sniff.lstrip() + sniff = _leading_text(content) if sniff.startswith("<"): return f"Invalid aggregate report: {aggregate_error}" @@ -2515,40 +3148,56 @@ def parse_report_file( if content.startswith(MAGIC_ZIP) or content.startswith(MAGIC_GZIP): content = extract_report(content) - results: ParsedReport | None = None - # parse_report_file tries the three report formats in turn. When all three - # reject the input, keep each format's specific error so the final message + # reject the input, keep each format's error text so the final message # can explain *why* the file is invalid instead of a bare "Not a valid - # report" (see _describe_parse_failure). + # report" (see _describe_parse_failure). Only the text is kept: an + # exception's traceback holds the failed parser's frames, and with them + # its partial tree, while the next parser runs (GHSA-q7gm-8cj9-94hr). + aggregate_message = smtp_tls_message = email_message = "" try: report = parse_aggregate_report_file( content, config=cfg, keep_alive=keep_alive, ) - results = {"report_type": "aggregate", "report": report} + return {"report_type": "aggregate", "report": report} except InvalidAggregateReport as aggregate_error: + aggregate_message = str(aggregate_error) + leading = _leading_text(content) + if leading.startswith("<"): + # JSON never begins with "<", and parse_smtp_tls_report_json() would + # first decode the whole content. + smtp_tls_message = "not JSON" + else: try: report = parse_smtp_tls_report_json(content) - results = {"report_type": "smtp_tls", "report": report} + return {"report_type": "smtp_tls", "report": report} except InvalidSMTPTLSReport as smtp_tls_error: - try: - results = parse_report_email( - content, - config=cfg, - keep_alive=keep_alive, - ) - except InvalidDMARCReport as email_error: - raise ParserError( - _describe_parse_failure( - content, aggregate_error, smtp_tls_error, email_error - ) - ) from email_error - - if results is None: - raise ParserError("Not a valid report") - return results + smtp_tls_message = str(smtp_tls_error) + if leading.startswith(("<", "{")): + # An RFC 822 message begins with a header line, never "<" or "{"; + # such a file's failure is reported as the aggregate or SMTP TLS + # error, so no email reason is needed. + pass + elif _exceeds_size_cap(content): + # The email parser has no size cap of its own. + email_message = _size_cap_text(content) + else: + try: + return parse_report_email(content, config=cfg, keep_alive=keep_alive) + except InvalidDMARCReport as email_error: + email_message = str(email_error) + message = _describe_parse_failure( + content, aggregate_message, smtp_tls_message, email_message + ) + # The cause is a fresh exception with the relevant format's text, so the + # chain is kept without the failed parse's traceback. + if leading.startswith("<"): + raise ParserError(message) from InvalidAggregateReport(aggregate_message) + if leading.startswith("{"): + raise ParserError(message) from InvalidSMTPTLSReport(smtp_tls_message) + raise ParserError(message) from InvalidDMARCReport(email_message) _ASCII_UPPER_TO_LOWER = str.maketrans( diff --git a/parsedmarc/cli.py b/parsedmarc/cli.py index c8fecb63..67b57a3a 100644 --- a/parsedmarc/cli.py +++ b/parsedmarc/cli.py @@ -2656,7 +2656,13 @@ def _main(): ) if not opts.silent: - print(output_str) + try: + print(output_str) + except (UnicodeEncodeError, OSError) as error_: + # A report string the console's encoding cannot write, or a + # closed pipe, must not escape process_reports() + # (GHSA-4272-4r8g-6c5h). + log_output_error("Standard output exception", error_.__str__()) if opts.output: try: save_output( @@ -2670,12 +2676,18 @@ def _main(): smtp_tls_csv_filename=opts.smtp_tls_csv_filename, ) except (OSError, ValueError) as error_: - # The only output destination that was not already caught: - # a full disk or an unwritable directory used to crash the + # A full disk or an unwritable directory used to crash the # run outright, and now that a failed save holds mailbox # messages back it also has to be recorded like any other # destination's failure rather than escaping. log_output_error("File output", str(error_)) + except Exception as error_: + # Every destination below ends with the same catch-all: a + # report a sender shaped to make one writer raise (a + # wrong-typed field reaching the CSV writer, say) must mark + # the batch unsaved, not escape process_reports() and stop + # the whole mailbox run (GHSA-4272-4r8g-6c5h). + log_output_error("File output exception", error_.__str__()) kafka_client = clients.get("kafka_client") s3_client = clients.get("s3_client") @@ -2755,6 +2767,8 @@ def _main(): logger.warning(warning.__str__()) except postgres.PostgreSQLError as error_: log_output_error("PostgreSQL", error_.__str__()) + except Exception as error_: + log_output_error("PostgreSQL exception", error_.__str__()) try: if syslog_client: @@ -2790,6 +2804,8 @@ def _main(): hec_client.save_aggregate_reports_to_splunk(aggregate_reports_) except splunk.SplunkError as e: log_output_error("Splunk HEC", e.__str__()) + except Exception as error_: + log_output_error("Splunk HEC exception", error_.__str__()) if opts.save_failure: for report in reports_["failure_reports"]: @@ -2813,6 +2829,8 @@ def _main(): log_output_error("Elasticsearch", error_.__str__()) except InvalidDMARCReport as error_: log_output_error("Invalid DMARC report", error_.__str__()) + except Exception as error_: + log_output_error("Elasticsearch exception", error_.__str__()) try: shards = opts.opensearch_number_of_shards @@ -2834,6 +2852,8 @@ def _main(): log_output_error("OpenSearch", error_.__str__()) except InvalidDMARCReport as error_: log_output_error("Invalid DMARC report", error_.__str__()) + except Exception as error_: + log_output_error("OpenSearch exception", error_.__str__()) try: if kafka_client: @@ -2856,6 +2876,8 @@ def _main(): logger.warning(warning.__str__()) except postgres.PostgreSQLError as error_: log_output_error("PostgreSQL", error_.__str__()) + except Exception as error_: + log_output_error("PostgreSQL exception", error_.__str__()) try: if syslog_client: @@ -2891,6 +2913,8 @@ def _main(): hec_client.save_failure_reports_to_splunk(failure_reports_) except splunk.SplunkError as e: log_output_error("Splunk HEC", e.__str__()) + except Exception as error_: + log_output_error("Splunk HEC exception", error_.__str__()) if opts.save_smtp_tls: for report in reports_["smtp_tls_reports"]: @@ -2914,6 +2938,8 @@ def _main(): log_output_error("Elasticsearch", error_.__str__()) except InvalidDMARCReport as error_: log_output_error("Invalid DMARC report", error_.__str__()) + except Exception as error_: + log_output_error("Elasticsearch exception", error_.__str__()) try: shards = opts.opensearch_number_of_shards @@ -2935,6 +2961,8 @@ def _main(): log_output_error("OpenSearch", error_.__str__()) except InvalidDMARCReport as error_: log_output_error("Invalid DMARC report", error_.__str__()) + except Exception as error_: + log_output_error("OpenSearch exception", error_.__str__()) try: if kafka_client: @@ -2957,6 +2985,8 @@ def _main(): logger.warning(warning.__str__()) except postgres.PostgreSQLError as error_: log_output_error("PostgreSQL", error_.__str__()) + except Exception as error_: + log_output_error("PostgreSQL exception", error_.__str__()) try: if syslog_client: @@ -2992,6 +3022,8 @@ def _main(): hec_client.save_smtp_tls_reports_to_splunk(smtp_tls_reports_) except splunk.SplunkError as e: log_output_error("Splunk HEC", e.__str__()) + except Exception as error_: + log_output_error("Splunk HEC exception", error_.__str__()) if opts.la_dce: try: diff --git a/parsedmarc/constants.py b/parsedmarc/constants.py index c8512391..3f77a36e 100644 --- a/parsedmarc/constants.py +++ b/parsedmarc/constants.py @@ -10,3 +10,35 @@ DEFAULT_DNS_MAX_RETRIES = 0 # operator's anycast outage or authoritative-server incompatibility falls # through to a different provider within one resolve() call. RECOMMENDED_DNS_NAMESERVERS = ("1.1.1.1", "8.8.8.8") + +# Limits on the XML of an aggregate report, enforced while expat reads it and +# before xmltodict builds the tree (GHSA-q7gm-8cj9-94hr). A real report is +# fewer than 10 elements deep. Nodes are elements plus attributes; the +# number of "=" characters bounds the attributes, and the samples hold at +# most 3 (namespace declarations). The largest sample has 32,019 nodes +# (909 KB), so the limit is a report of about 28 MB. With the 32 MiB size cap +# (MAX_DECOMPRESSED_REPORT_SIZE), 999,000 distinct elements with 14 +# characters of text each (31 MiB, read from disk before the baseline) need +# +399 MiB on the lxml recovery path and +338 to +355 MiB on expat when the +# text is parsed directly, and +500 MiB and +452 MiB through +# parse_report_file(), which also holds the file's bytes. +MAX_XML_DEPTH = 100 +MAX_XML_NODES = 1_000_000 +# Longest text between a "<" and the next ">" in an aggregate report. expat +# and libxml2 build a start tag's whole attribute list before reporting it, +# so attributes cannot be counted before they cost memory. The longest tag +# in the samples is the root element's namespace declarations, 108 +# characters (a 234-character comment is the longest construct). +MAX_XML_TAG_LENGTH = 65_536 + +# A report's longer than this many days is rejected. A real +# aggregate report covers a day (RFC 7489 §6.3: the "ri" tag's default +# interval is 86400 seconds); a year leaves room for mistakes, and +# normalization emits at most one record per day. +MAX_REPORT_SPAN_DAYS = 366 +# Most distinct dated indexes one report may create in Elasticsearch or +# OpenSearch (one shard each); further records go to the report's first index. +MAX_INDEXES_PER_REPORT = 31 +# Most records per-day normalization may produce from one report (records +# times days). A larger report keeps its records whole, over its own range. +MAX_NORMALIZED_RECORDS = 250_000 diff --git a/parsedmarc/elastic.py b/parsedmarc/elastic.py index 51cc4c79..0051a76f 100644 --- a/parsedmarc/elastic.py +++ b/parsedmarc/elastic.py @@ -24,8 +24,9 @@ from elasticsearch.dsl import ( from elasticsearch.helpers import reindex from parsedmarc import InvalidFailureReport +from parsedmarc.constants import MAX_INDEXES_PER_REPORT from parsedmarc.log import logger -from parsedmarc.utils import human_timestamp_to_datetime +from parsedmarc.utils import human_timestamp_to_datetime, sanitize_for_log class ElasticsearchError(Exception): @@ -59,6 +60,102 @@ _READY_DATA_STREAMS: set[str] = set() # exact stream name, so an overlap is unlikely in the first place. _DATA_STREAM_TEMPLATE_PRIORITY = 501 +# Mappings ``create_indexes`` and ``create_data_streams`` install on the +# indexes and templates they create. Without them the cluster infers every +# field's type from the first document that holds it, with date detection +# on, so a crafted report could make a string field a date (report_id +# "2026-11-09") or use up index.mapping.total_fields.limit with invented +# sample header names, and every later report into that index would then +# be rejected (GHSA-qpcg-29vr-jwm3). Date detection is off; the fields that +# hold dates are declared as dates instead, the type they got before. +# Everything else stays dynamic and object-mapped, which the shipped +# dashboards need (see create_indexes). +_DATE_FIELD: dict[str, Any] = {"type": "date"} + +# The type Elasticsearch's dynamic mapping gives a JSON string, so a header +# declared here has the same type as in indexes created before. +_DYNAMIC_STRING_FIELD: dict[str, Any] = { + "type": "text", + "fields": {"keyword": {"type": "keyword", "ignore_above": 256}}, +} + +# The failure sample headers indexed for search: every header the shipped +# dashboards (dashboards/) reference, plus cc. +# Header names are stored lower-cased. The other headers stay in _source +# but are not indexed (``dynamic: False``). +_FAILURE_SAMPLE_INDEXED_HEADERS = ( + "authentication-results", + "auto-submitted", + "cc", + "content-transfer-encoding", + "content-type", + "date", + "from", + "in-reply-to", + "message-id", + "mime-version", + "received", + "references", + "reply-to", + "return-path", + "subject", + "thread-index", + "thread-topic", + "to", + "x-auto-response-suppress", + "x-exclaimer-md-config", + "x-linkedin-fe", + "x-mailer", + "x-ms-exchange-generated-message-source", + "x-ms-exchange-inbox-rules-loop", + "x-ms-exchange-parent-message-id", + "x-ms-exchange-transport-fromentityheader", + "x-ms-has-attach", + "x-ms-tnef-correlator", + "x-onpremexternalip", +) + +_AGGREGATE_MAPPINGS: dict[str, Any] = { + "date_detection": False, + "properties": { + "@timestamp": _DATE_FIELD, + "date_range": _DATE_FIELD, + "date_begin": _DATE_FIELD, + "date_end": _DATE_FIELD, + }, +} + +_FAILURE_MAPPINGS: dict[str, Any] = { + "date_detection": False, + "properties": { + "@timestamp": _DATE_FIELD, + "arrival_date": _DATE_FIELD, + "sample": { + "properties": { + "date": _DATE_FIELD, + "headers": { + "type": "object", + "dynamic": False, + "properties": { + name: _DYNAMIC_STRING_FIELD + for name in _FAILURE_SAMPLE_INDEXED_HEADERS + }, + }, + } + }, + }, +} + +_SMTP_TLS_MAPPINGS: dict[str, Any] = { + "date_detection": False, + "properties": { + "@timestamp": _DATE_FIELD, + "date_range": _DATE_FIELD, + "date_begin": _DATE_FIELD, + "date_end": _DATE_FIELD, + }, +} + # Guard query for the dkim_results_combined/spf_results_combined backfill # (see ``migrate_indexes``). Matches only documents that have at least one # DKIM or SPF auth result and are missing the corresponding combined field. @@ -677,15 +774,45 @@ def _effective_index_settings(settings: dict[str, Any] | None) -> dict[str, Any] return dict(settings) -def create_indexes(names: list[str], settings: dict[str, Any] | None = None): +def _with_timestamp_mapping(mappings: dict[str, Any] | None) -> dict[str, Any]: + """Return a copy of ``mappings`` that also maps ``@timestamp`` as a date. + + A data stream rejects documents unless ``@timestamp`` is a date field, + so the data stream template always declares it. + + Args: + mappings (dict | None): A mappings body, or ``None``. + + Returns: + dict: The mappings body with ``properties["@timestamp"]`` set; the + argument is not modified. + """ + result = dict(mappings) if mappings else {} + result["properties"] = { + **result.get("properties", {}), + "@timestamp": {"type": "date"}, + } + return result + + +def create_indexes( + names: list[str], + settings: dict[str, Any] | None = None, + mappings: dict[str, Any] | None = None, +): """ Create Elasticsearch indexes + An index that already exists is left as it is, mappings included. + Args: names (list): A list of index names settings (dict): Index settings. In Serverless mode, keys in ``_SERVERLESS_REJECTED_SETTINGS`` are filtered out and the remaining keys are passed through; defaults are skipped entirely. + mappings (dict | None): The mappings body to create each index + with, such as ``_AGGREGATE_MAPPINGS``; ``None`` leaves every + field to dynamic mapping. """ effective_settings = _effective_index_settings(settings) @@ -698,22 +825,30 @@ def create_indexes(names: list[str], settings: dict[str, Any] | None = None): # fields, Vega can run nested aggregations but does not render # tables, and Grafana's nested bucket aggregation (9.4+) lacks # reverse_nested for parent-level metrics like message_count — - # so the dynamic `object` mapping produced by a bare create is - # load-bearing for the shipped dashboards. _AggregateReportDoc - # still declares dkim_results/spf_results with Nested(...), but - # that is only the DSL's in-memory shape for building documents - # — it is never installed as a mapping. See issue #169 and the + # so the dynamic `object` mapping of those fields (the mappings + # body declares none of them) is load-bearing for the shipped + # dashboards. _AggregateReportDoc still declares + # dkim_results/spf_results with Nested(...), but that is only + # the DSL's in-memory shape for building documents — it is + # never installed as a mapping. See issue #169 and the # *_combined fields on _AggregateReportDoc. if not index.exists(): logger.debug(f"Creating Elasticsearch index: {name}") + body: dict[str, Any] = {} if effective_settings: - index.settings(**effective_settings) - index.create() + body["settings"] = effective_settings + if mappings: + body["mappings"] = mappings + connections.get_connection().indices.create(index=name, body=body) except Exception as e: raise ElasticsearchError(f"Elasticsearch error: {e.__str__()}") -def create_data_streams(names: list[str], settings: dict[str, Any] | None = None): +def create_data_streams( + names: list[str], + settings: dict[str, Any] | None = None, + mappings: dict[str, Any] | None = None, +): """ Create Elasticsearch data streams and their index templates @@ -727,14 +862,17 @@ def create_data_streams(names: list[str], settings: dict[str, Any] | None = None the stream, so edits made to that template directly do not last. Each name is set up once per process; ``set_hosts`` resets that. - The template maps only ``@timestamp``. Like ``create_indexes``, it - deliberately installs no mappings for the report fields themselves (see - issue #169), so they keep their dynamic ``object`` mapping. + The template maps ``@timestamp`` as a date, on top of ``mappings``. + Like ``create_indexes``, it declares no ``nested`` fields (see issue + #169), so the report fields keep their ``object`` mapping. Args: names (list): A list of data stream names settings (dict): Index settings for the stream's backing indices, adjusted for Serverless the same way ``create_indexes`` does. + mappings (dict | None): The mappings body for the stream's backing + indices, such as ``_AGGREGATE_MAPPINGS``; ``None`` maps only + ``@timestamp``. Raises: ElasticsearchError: A concrete index or alias already holds one of @@ -742,6 +880,7 @@ def create_data_streams(names: list[str], settings: dict[str, Any] | None = None is not remembered, so the next save retries it. """ effective_settings = _effective_index_settings(settings) + template_mappings = _with_timestamp_mapping(mappings) for name in names: if name in _READY_DATA_STREAMS: @@ -773,7 +912,7 @@ def create_data_streams(names: list[str], settings: dict[str, Any] | None = None priority=_DATA_STREAM_TEMPLATE_PRIORITY, template={ "settings": effective_settings, - "mappings": {"properties": {"@timestamp": {"type": "date"}}}, + "mappings": template_mappings, }, meta={ "description": ( @@ -1119,6 +1258,10 @@ def save_aggregate_report_to_elasticsearch( discovery_method=aggregate_report["policy_published"].get("discovery_method"), ) + # Distinct dated indexes this report has used, in first-use order. + report_indexes: dict[str, None] = {} + routed_records = 0 + for record in aggregate_report["records"]: begin_date = human_timestamp_to_datetime( record["interval_begin"], to_utc=True, assume_utc=True @@ -1202,14 +1345,24 @@ def save_aggregate_report_to_elasticsearch( number_of_shards=number_of_shards, number_of_replicas=number_of_replicas ) if use_data_streams: - create_data_streams([index], index_settings) + create_data_streams([index], index_settings, _AGGREGATE_MAPPINGS) else: if monthly_indexes: index_date = begin_date.strftime("%Y-%m") else: index_date = begin_date.strftime("%Y-%m-%d") index = f"{index}-{index_date}" - create_indexes([index], index_settings) + if ( + index not in report_indexes + and len(report_indexes) >= MAX_INDEXES_PER_REPORT + ): + # Each new index costs a shard; send the overflow to the + # report's first index, keeping the record's own dates. + index = next(iter(report_indexes)) + routed_records += 1 + else: + report_indexes[index] = None + create_indexes([index], index_settings, _AGGREGATE_MAPPINGS) agg_doc.meta.index = index # pyright: ignore[reportOptionalMemberAccess, reportAttributeAccessIssue] try: @@ -1219,6 +1372,34 @@ def save_aggregate_report_to_elasticsearch( except Exception as e: raise ElasticsearchError(f"Elasticsearch error: {e.__str__()}") + if routed_records: + logged_report_id = sanitize_for_log( + str(aggregate_report["report_metadata"]["report_id"]) + ) + logger.warning( + f"{routed_records} records of report {logged_report_id} were saved to " + f"{next(iter(report_indexes))} instead of their own dated index: a " + f"report may create at most {MAX_INDEXES_PER_REPORT} indexes" + ) + + +def _flatten_address_header(addresses: Any) -> str | None: + """Flatten a failure sample's address-list header to its first address. + + mail-parser gives ``From``, ``To`` and ``Reply-To`` as a list of + ``[display name, address]`` pairs, and as an empty list when the header + holds no address: an empty ``From:``, or ``To: undisclosed-recipients:;`` + (an RFC 5322 §3.4 group with no members). Returns ``"Name
"``, + the bare address when the display name is empty, or ``None`` for an + empty list. + """ + if not addresses: + return None + first = addresses[0] + if first[0] == "": + return first[1] + return " <".join(first) + ">" + def save_failure_report_to_elasticsearch( failure_report: dict[str, Any], @@ -1261,7 +1442,11 @@ def save_failure_report_to_elasticsearch( original_headers = failure_report["parsed_sample"]["headers"] headers: dict[str, Any] = {} for original_header in original_headers: - headers[original_header.lower()] = original_headers[original_header] + # A "." in a field name is an object path to the cluster, so a + # header named "Subject.x" would turn sample.headers.subject into an + # object and reject every later sample with a Subject header. + header_name = original_header.lower().replace(".", "_") + headers[header_name] = original_headers[original_header] # arrival_date_utc is a UTC wall-clock string; without assume_utc the # naive .timestamp() below would interpret it as local time and skew @@ -1287,40 +1472,26 @@ def save_failure_report_to_elasticsearch( from_ = None to_ = None subject = None + # From, To and Reply-To are flattened to their first address so they + # can be displayed and aggregated as strings. A header with no address + # is left out of the document; From and To also feed the dedup query. + for address_header in ("from", "to", "reply-to"): + if address_header in headers: + flattened = _flatten_address_header(headers[address_header]) + if flattened is None: + del headers[address_header] + else: + headers[address_header] = flattened if "from" in headers: - # We convert the FROM header from a string list to a flat string. - headers["from"] = headers["from"][0] - if headers["from"][0] == "": - headers["from"] = headers["from"][1] - else: - headers["from"] = " <".join(headers["from"]) + ">" - from_ = dict() from_["sample.headers.from"] = headers["from"] from_query = Q(dict(match_phrase=from_)) # pyright: ignore[reportArgumentType] q = q & from_query if "to" in headers: - # We convert the TO header from a string list to a flat string. - headers["to"] = headers["to"][0] - if headers["to"][0] == "": - headers["to"] = headers["to"][1] - else: - headers["to"] = " <".join(headers["to"]) + ">" - to_ = dict() to_["sample.headers.to"] = headers["to"] to_query = Q(dict(match_phrase=to_)) # pyright: ignore[reportArgumentType] q = q & to_query - if "reply-to" in headers: - # Flatten the Reply-To header to a string so it can be displayed - # and aggregated like From/To. Only the first address is used, - # matching the From/To handling above. Not part of the dedup - # query. - headers["reply-to"] = headers["reply-to"][0] - if headers["reply-to"][0] == "": - headers["reply-to"] = headers["reply-to"][1] - else: - headers["reply-to"] = " <".join(headers["reply-to"]) + ">" if "subject" in headers: subject = headers["subject"] subject_query = {"match_phrase": {"sample.headers.subject": subject}} @@ -1401,14 +1572,14 @@ def save_failure_report_to_elasticsearch( number_of_shards=number_of_shards, number_of_replicas=number_of_replicas ) if use_data_streams: - create_data_streams([index], index_settings) + create_data_streams([index], index_settings, _FAILURE_MAPPINGS) else: if monthly_indexes: index_date = arrival_date.strftime("%Y-%m") else: index_date = arrival_date.strftime("%Y-%m-%d") index = f"{index}-{index_date}" - create_indexes([index], index_settings) + create_indexes([index], index_settings, _FAILURE_MAPPINGS) failure_doc.meta.index = index # pyright: ignore[reportAttributeAccessIssue, reportOptionalMemberAccess] try: # Data streams accept only op_type=create. No _id is set, so @@ -1599,9 +1770,9 @@ def save_smtp_tls_report_to_elasticsearch( smtp_tls_doc.policies.append(policy_doc) if use_data_streams: - create_data_streams([index], index_settings) + create_data_streams([index], index_settings, _SMTP_TLS_MAPPINGS) else: - create_indexes([index], index_settings) + create_indexes([index], index_settings, _SMTP_TLS_MAPPINGS) smtp_tls_doc.meta.index = index # pyright: ignore[reportOptionalMemberAccess, reportAttributeAccessIssue] try: diff --git a/parsedmarc/opensearch.py b/parsedmarc/opensearch.py index 00d1df26..9ba1a2cd 100644 --- a/parsedmarc/opensearch.py +++ b/parsedmarc/opensearch.py @@ -28,8 +28,13 @@ from opensearchpy import ( from opensearchpy.helpers import reindex from parsedmarc import InvalidFailureReport +from parsedmarc.constants import MAX_INDEXES_PER_REPORT from parsedmarc.log import logger -from parsedmarc.utils import human_timestamp_to_datetime, redact_url_credentials +from parsedmarc.utils import ( + human_timestamp_to_datetime, + redact_url_credentials, + sanitize_for_log, +) class OpenSearchError(Exception): @@ -52,6 +57,102 @@ _READY_DATA_STREAMS: set[str] = set() # another template is unlikely in the first place. _DATA_STREAM_TEMPLATE_PRIORITY = 501 +# Mappings ``create_indexes`` and ``create_data_streams`` install on the +# indexes and templates they create. Without them the cluster infers every +# field's type from the first document that holds it, with date detection +# on, so a crafted report could make a string field a date (report_id +# "2026-11-09") or use up index.mapping.total_fields.limit with invented +# sample header names, and every later report into that index would then +# be rejected (GHSA-qpcg-29vr-jwm3). Date detection is off; the fields that +# hold dates are declared as dates instead, the type they got before. +# Everything else stays dynamic and object-mapped, which the shipped +# dashboards need (see create_indexes). +_DATE_FIELD: dict[str, Any] = {"type": "date"} + +# The type OpenSearch's dynamic mapping gives a JSON string, so a header +# declared here has the same type as in indexes created before. +_DYNAMIC_STRING_FIELD: dict[str, Any] = { + "type": "text", + "fields": {"keyword": {"type": "keyword", "ignore_above": 256}}, +} + +# The failure sample headers indexed for search: every header the shipped +# dashboards (dashboards/) reference, plus cc. +# Header names are stored lower-cased. The other headers stay in _source +# but are not indexed (``dynamic: False``). +_FAILURE_SAMPLE_INDEXED_HEADERS = ( + "authentication-results", + "auto-submitted", + "cc", + "content-transfer-encoding", + "content-type", + "date", + "from", + "in-reply-to", + "message-id", + "mime-version", + "received", + "references", + "reply-to", + "return-path", + "subject", + "thread-index", + "thread-topic", + "to", + "x-auto-response-suppress", + "x-exclaimer-md-config", + "x-linkedin-fe", + "x-mailer", + "x-ms-exchange-generated-message-source", + "x-ms-exchange-inbox-rules-loop", + "x-ms-exchange-parent-message-id", + "x-ms-exchange-transport-fromentityheader", + "x-ms-has-attach", + "x-ms-tnef-correlator", + "x-onpremexternalip", +) + +_AGGREGATE_MAPPINGS: dict[str, Any] = { + "date_detection": False, + "properties": { + "@timestamp": _DATE_FIELD, + "date_range": _DATE_FIELD, + "date_begin": _DATE_FIELD, + "date_end": _DATE_FIELD, + }, +} + +_FAILURE_MAPPINGS: dict[str, Any] = { + "date_detection": False, + "properties": { + "@timestamp": _DATE_FIELD, + "arrival_date": _DATE_FIELD, + "sample": { + "properties": { + "date": _DATE_FIELD, + "headers": { + "type": "object", + "dynamic": False, + "properties": { + name: _DYNAMIC_STRING_FIELD + for name in _FAILURE_SAMPLE_INDEXED_HEADERS + }, + }, + } + }, + }, +} + +_SMTP_TLS_MAPPINGS: dict[str, Any] = { + "date_detection": False, + "properties": { + "@timestamp": _DATE_FIELD, + "date_range": _DATE_FIELD, + "date_begin": _DATE_FIELD, + "date_end": _DATE_FIELD, + }, +} + # Guard query for the dkim_results_combined/spf_results_combined backfill # (see ``migrate_indexes``). Matches only documents that have at least one @@ -593,13 +694,44 @@ def set_hosts( return connections.create_connection(**conn_params) -def create_indexes(names: list[str], settings: dict[str, Any] | None = None): +def _with_timestamp_mapping(mappings: dict[str, Any] | None) -> dict[str, Any]: + """Return a copy of ``mappings`` that also maps ``@timestamp`` as a date. + + A data stream rejects documents unless ``@timestamp`` is a date field, + so the data stream template always declares it. + + Args: + mappings (dict | None): A mappings body, or ``None``. + + Returns: + dict: The mappings body with ``properties["@timestamp"]`` set; the + argument is not modified. + """ + result = dict(mappings) if mappings else {} + result["properties"] = { + **result.get("properties", {}), + "@timestamp": {"type": "date"}, + } + return result + + +def create_indexes( + names: list[str], + settings: dict[str, Any] | None = None, + mappings: dict[str, Any] | None = None, +): """ Create OpenSearch indexes + An index that already exists is left as it is, mappings included. + Args: names (list): A list of index names - settings (dict): Index settings + settings (dict): Index settings; one shard and no replicas when + ``None`` + mappings (dict | None): The mappings body to create each index + with, such as ``_AGGREGATE_MAPPINGS``; ``None`` leaves every + field to dynamic mapping. """ for name in names: @@ -611,24 +743,32 @@ def create_indexes(names: list[str], settings: dict[str, Any] | None = None): # fields, Vega can run nested aggregations but does not render # tables, and Grafana's nested bucket aggregation (9.4+) lacks # reverse_nested for parent-level metrics like message_count — - # so the dynamic `object` mapping produced by a bare create is - # load-bearing for the shipped dashboards. _AggregateReportDoc - # still declares dkim_results/spf_results with Nested(...), but - # that is only the DSL's in-memory shape for building documents - # — it is never installed as a mapping. See issue #169 and the + # so the dynamic `object` mapping of those fields (the mappings + # body declares none of them) is load-bearing for the shipped + # dashboards. _AggregateReportDoc still declares + # dkim_results/spf_results with Nested(...), but that is only + # the DSL's in-memory shape for building documents — it is + # never installed as a mapping. See issue #169 and the # *_combined fields on _AggregateReportDoc. if not index.exists(): logger.debug(f"Creating OpenSearch index: {name}") + body: dict[str, Any] = {} if settings is None: - index.settings(number_of_shards=1, number_of_replicas=0) - else: - index.settings(**settings) - index.create() + body["settings"] = {"number_of_shards": 1, "number_of_replicas": 0} + elif settings: + body["settings"] = dict(settings) + if mappings: + body["mappings"] = mappings + connections.get_connection().indices.create(index=name, body=body) except Exception as e: raise OpenSearchError(f"OpenSearch error: {e.__str__()}") -def create_data_streams(names: list[str], settings: dict[str, Any] | None = None): +def create_data_streams( + names: list[str], + settings: dict[str, Any] | None = None, + mappings: dict[str, Any] | None = None, +): """ Create OpenSearch data streams and their index templates @@ -642,15 +782,18 @@ def create_data_streams(names: list[str], settings: dict[str, Any] | None = None ``ism_template`` instead. Each name is set up once per process; ``set_hosts`` resets that. - The template maps only ``@timestamp``. Like ``create_indexes``, it - deliberately installs no mappings for the report fields themselves (see - issue #169), so they keep their dynamic ``object`` mapping. + The template maps ``@timestamp`` as a date, on top of ``mappings``. + Like ``create_indexes``, it declares no ``nested`` fields (see issue + #169), so the report fields keep their ``object`` mapping. Args: names (list): A list of data stream names settings (dict): Index settings for the stream's backing indices; one shard and no replicas when ``None``, as in ``create_indexes``. + mappings (dict | None): The mappings body for the stream's backing + indices, such as ``_AGGREGATE_MAPPINGS``; ``None`` maps only + ``@timestamp``. Raises: OpenSearchError: A concrete index or alias already holds one of the @@ -664,6 +807,7 @@ def create_data_streams(names: list[str], settings: dict[str, Any] | None = None } else: effective_settings = dict(settings) + template_mappings = _with_timestamp_mapping(mappings) for name in names: if name in _READY_DATA_STREAMS: @@ -695,9 +839,7 @@ def create_data_streams(names: list[str], settings: dict[str, Any] | None = None "priority": _DATA_STREAM_TEMPLATE_PRIORITY, "template": { "settings": effective_settings, - "mappings": { - "properties": {"@timestamp": {"type": "date"}} - }, + "mappings": template_mappings, }, "_meta": { "description": ( @@ -1051,6 +1193,10 @@ def save_aggregate_report_to_opensearch( discovery_method=aggregate_report["policy_published"].get("discovery_method"), ) + # Distinct dated indexes this report has used, in first-use order. + report_indexes: dict[str, None] = {} + routed_records = 0 + for record in aggregate_report["records"]: begin_date = human_timestamp_to_datetime( record["interval_begin"], to_utc=True, assume_utc=True @@ -1134,14 +1280,24 @@ def save_aggregate_report_to_opensearch( number_of_shards=number_of_shards, number_of_replicas=number_of_replicas ) if use_data_streams: - create_data_streams([index], index_settings) + create_data_streams([index], index_settings, _AGGREGATE_MAPPINGS) else: if monthly_indexes: index_date = begin_date.strftime("%Y-%m") else: index_date = begin_date.strftime("%Y-%m-%d") index = f"{index}-{index_date}" - create_indexes([index], index_settings) + if ( + index not in report_indexes + and len(report_indexes) >= MAX_INDEXES_PER_REPORT + ): + # Each new index costs a shard; send the overflow to the + # report's first index, keeping the record's own dates. + index = next(iter(report_indexes)) + routed_records += 1 + else: + report_indexes[index] = None + create_indexes([index], index_settings, _AGGREGATE_MAPPINGS) agg_doc.meta.index = index try: @@ -1151,6 +1307,34 @@ def save_aggregate_report_to_opensearch( except Exception as e: raise OpenSearchError(f"OpenSearch error: {e.__str__()}") + if routed_records: + logged_report_id = sanitize_for_log( + str(aggregate_report["report_metadata"]["report_id"]) + ) + logger.warning( + f"{routed_records} records of report {logged_report_id} were saved to " + f"{next(iter(report_indexes))} instead of their own dated index: a " + f"report may create at most {MAX_INDEXES_PER_REPORT} indexes" + ) + + +def _flatten_address_header(addresses: Any) -> str | None: + """Flatten a failure sample's address-list header to its first address. + + mail-parser gives ``From``, ``To`` and ``Reply-To`` as a list of + ``[display name, address]`` pairs, and as an empty list when the header + holds no address: an empty ``From:``, or ``To: undisclosed-recipients:;`` + (an RFC 5322 §3.4 group with no members). Returns ``"Name
"``, + the bare address when the display name is empty, or ``None`` for an + empty list. + """ + if not addresses: + return None + first = addresses[0] + if first[0] == "": + return first[1] + return " <".join(first) + ">" + def save_failure_report_to_opensearch( failure_report: dict[str, Any], @@ -1193,7 +1377,11 @@ def save_failure_report_to_opensearch( original_headers = failure_report["parsed_sample"]["headers"] headers: dict[str, Any] = {} for original_header in original_headers: - headers[original_header.lower()] = original_headers[original_header] + # A "." in a field name is an object path to the cluster, so a + # header named "Subject.x" would turn sample.headers.subject into an + # object and reject every later sample with a Subject header. + header_name = original_header.lower().replace(".", "_") + headers[header_name] = original_headers[original_header] # arrival_date_utc is a UTC wall-clock string; without assume_utc the # naive .timestamp() below would interpret it as local time and skew @@ -1219,40 +1407,26 @@ def save_failure_report_to_opensearch( from_ = None to_ = None subject = None + # From, To and Reply-To are flattened to their first address so they + # can be displayed and aggregated as strings. A header with no address + # is left out of the document; From and To also feed the dedup query. + for address_header in ("from", "to", "reply-to"): + if address_header in headers: + flattened = _flatten_address_header(headers[address_header]) + if flattened is None: + del headers[address_header] + else: + headers[address_header] = flattened if "from" in headers: - # We convert the FROM header from a string list to a flat string. - headers["from"] = headers["from"][0] - if headers["from"][0] == "": - headers["from"] = headers["from"][1] - else: - headers["from"] = " <".join(headers["from"]) + ">" - from_ = dict() from_["sample.headers.from"] = headers["from"] from_query = Q(dict(match_phrase=from_)) q = q & from_query if "to" in headers: - # We convert the TO header from a string list to a flat string. - headers["to"] = headers["to"][0] - if headers["to"][0] == "": - headers["to"] = headers["to"][1] - else: - headers["to"] = " <".join(headers["to"]) + ">" - to_ = dict() to_["sample.headers.to"] = headers["to"] to_query = Q(dict(match_phrase=to_)) q = q & to_query - if "reply-to" in headers: - # Flatten the Reply-To header to a string so it can be displayed - # and aggregated like From/To. Only the first address is used, - # matching the From/To handling above. Not part of the dedup - # query. - headers["reply-to"] = headers["reply-to"][0] - if headers["reply-to"][0] == "": - headers["reply-to"] = headers["reply-to"][1] - else: - headers["reply-to"] = " <".join(headers["reply-to"]) + ">" if "subject" in headers: subject = headers["subject"] subject_query = {"match_phrase": {"sample.headers.subject": subject}} @@ -1331,14 +1505,14 @@ def save_failure_report_to_opensearch( number_of_shards=number_of_shards, number_of_replicas=number_of_replicas ) if use_data_streams: - create_data_streams([index], index_settings) + create_data_streams([index], index_settings, _FAILURE_MAPPINGS) else: if monthly_indexes: index_date = arrival_date.strftime("%Y-%m") else: index_date = arrival_date.strftime("%Y-%m-%d") index = f"{index}-{index_date}" - create_indexes([index], index_settings) + create_indexes([index], index_settings, _FAILURE_MAPPINGS) failure_doc.meta.index = index try: # Data streams accept only op_type=create. No _id is set, so @@ -1529,9 +1703,9 @@ def save_smtp_tls_report_to_opensearch( smtp_tls_doc.policies.append(policy_doc) if use_data_streams: - create_data_streams([index], index_settings) + create_data_streams([index], index_settings, _SMTP_TLS_MAPPINGS) else: - create_indexes([index], index_settings) + create_indexes([index], index_settings, _SMTP_TLS_MAPPINGS) smtp_tls_doc.meta.index = index try: diff --git a/parsedmarc/postgres.py b/parsedmarc/postgres.py index 8f1c91f1..42b546ca 100644 --- a/parsedmarc/postgres.py +++ b/parsedmarc/postgres.py @@ -45,9 +45,16 @@ def _ensure_utc_suffix(value: str | None) -> str | None: Several parsers produce ``YYYY-MM-DD HH:MM:SS`` format strings that are known to be UTC but lack an explicit offset. PostgreSQL ``TIMESTAMPTZ`` columns need the offset to avoid interpreting the - value in the session timezone. + value in the session timezone. A value that is not a string is + returned unchanged, for PostgreSQL to accept or reject. """ - if value and "+" not in value and "-" not in value[10:] and "Z" not in value: + if ( + isinstance(value, str) + and value + and "+" not in value + and "-" not in value[10:] + and "Z" not in value + ): return value + "+00" return value diff --git a/parsedmarc/splunk.py b/parsedmarc/splunk.py index 9f7f5336..7687b244 100644 --- a/parsedmarc/splunk.py +++ b/parsedmarc/splunk.py @@ -215,16 +215,18 @@ class HECClient(object): data = self._common_data.copy() json_lines: list[str] = [] - for report in reports: - data["sourcetype"] = "smtp:tls" - timestamp = human_timestamp_to_unix_timestamp(report["begin_date"]) - data["time"] = timestamp - data["event"] = report.copy() - json_lines.append(f"{json.dumps(data)}\n") - if not self.verify: logger.debug("Skipping certificate verification for Splunk HEC") try: + # Inside the try so a report date that does not convert raises + # SplunkError like any other failed save. + for report in reports: + data["sourcetype"] = "smtp:tls" + timestamp = human_timestamp_to_unix_timestamp(report["begin_date"]) + data["time"] = timestamp + data["event"] = report.copy() + json_lines.append(f"{json.dumps(data)}\n") + response = self.session.post( self.url, content="".join(json_lines), timeout=self.timeout ) diff --git a/parsedmarc/utils.py b/parsedmarc/utils.py index 17f6d64b..02fb1f72 100644 --- a/parsedmarc/utils.py +++ b/parsedmarc/utils.py @@ -21,6 +21,7 @@ from urllib.parse import SplitResult, urlsplit import mailparser from expiringdict import ExpiringDict +from mailsuite.utils import check_message_limits from importlib.resources import files @@ -1762,12 +1763,17 @@ def parse_email(data: bytes | str, *, strip_attachment_payloads: bool = False) - Returns: dict: Parsed email data + + Raises: + mailsuite.utils.MessageLimitExceeded: The message's structure exceeds + the limits of ``mailsuite.utils.check_message_limits`` """ if isinstance(data, bytes): if is_outlook_msg(data): data = convert_outlook_msg(data) data = data.decode("utf-8", errors="replace") + check_message_limits(data) parsed_email = mailparser.parse_from_string(data) headers = json.loads(parsed_email.headers_json).copy() parsed_email = json.loads(parsed_email.mail_json).copy() diff --git a/pyproject.toml b/pyproject.toml index c5b83cd8..882f4cea 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,7 +59,7 @@ dependencies = [ # No extras: the base mailsuite supplies IMAP and Maildir connections # plus mail-parser. Gmail and Microsoft Graph come from the gmail and # msgraph extras below. - "mailsuite>=2.3.4", + "mailsuite>=2.3.5", "maxminddb>=2.0.0", "publicsuffixlist>=0.10.0", # Imported directly at utils.py:39 (dateutil.parser). It used to @@ -96,13 +96,13 @@ loganalytics = [ "azure-monitor-ingestion>=1.0.0", ] msgraph = [ - "mailsuite[msgraph]>=2.3.4", + "mailsuite[msgraph]>=2.3.5", # Imported directly in cli.py for Graph error handling; otherwise # only a transitive dep of mailsuite[msgraph] -> msgraph-sdk. "microsoft-kiota-abstractions>=1.8.0", ] gmail = [ - "mailsuite[gmail]>=2.3.4", + "mailsuite[gmail]>=2.3.5", ] # `all` must stay the union of every extra above — and must keep # excluding `postgresql`: psycopg's prebuilt binary wheels do not exist @@ -118,7 +118,7 @@ all = [ "boto3>=1.16.63", "elasticsearch>=8.18,<9", "kafka-python>=2.3.2", - "mailsuite[gmail,msgraph]>=2.3.4", + "mailsuite[gmail,msgraph]>=2.3.5", "microsoft-kiota-abstractions>=1.8.0", "opensearch-py>=2.4.2,<=4.0.0", "pygelf>=0.4.2", diff --git a/tests/test_cli.py b/tests/test_cli.py index b09f9499..3a9b54cf 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -21,7 +21,7 @@ import unittest import zipfile from collections.abc import Iterator, Sequence from configparser import ConfigParser -from contextlib import contextmanager +from contextlib import ExitStack, contextmanager from email.mime.application import MIMEApplication from email.mime.multipart import MIMEMultipart from tempfile import NamedTemporaryFile @@ -10536,5 +10536,261 @@ tenant_id = fosdem self.assertIsInstance(context.exception.__cause__, ValueError) +class TestOutputDestinationExceptionsMarkTheBatchUnsaved(unittest.TestCase): + """GHSA-4272-4r8g-6c5h: the failure and SMTP TLS Elasticsearch and + OpenSearch blocks of process_reports(), every Splunk HEC and PostgreSQL + block, and the file output caught only their own client's error. Any + other exception a report made a saver raise (an IndexError on an empty + From: list, a TypeError in the CSV writer) escaped through the save + callback, and _main logged "Mailbox Error" and exited 1 on every run, + leaving the message in place. Each block now ends with the catch-all the + aggregate Elasticsearch block has: the error is recorded through + log_output_error() and the batch is reported unsaved.""" + + _AGGREGATE = {"policy_published": {"domain": "example.com"}} + _FAILURE = {"reported_domain": "example.com"} + _SMTP_TLS = {"policies": [{"policy_domain": "example.com"}]} + + def setUp(self): + parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear() + self.addCleanup(parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear) + + def _run(self, sections, batch, *patches, general="", silent=True): + """Run _main() on one mailbox batch. + + Returns the save callback's verdicts, the ERROR log lines, the exit + code (``None`` when _main returned) and the mocks ``patches`` made. + """ + with NamedTemporaryFile("w", suffix=".ini", delete=False) as config_file: + config_file.write( + "[general]\n" + "save_aggregate = true\n" + "save_failure = true\n" + "save_smtp_tls = true\n" + f"silent = {str(silent).lower()}\n" + f"{general}\n" + "[imap]\n" + "host = imap.example.com\n" + "user = test-user\n" + "password = test-password\n\n" + f"{sections}" + ) + config_path = config_file.name + self.addCleanup(lambda: os.path.exists(config_path) and os.remove(config_path)) + full_batch = { + "aggregate_reports": [], + "failure_reports": [], + "smtp_tls_reports": [], + } + full_batch.update(batch) + verdicts = [] + + def _fetch_and_save(**kwargs): + verdicts.append(kwargs["save_callback"](full_batch)) + return full_batch + + with ExitStack() as stack: + stack.enter_context( + patch( + "parsedmarc.cli.get_dmarc_reports_from_mailbox", + side_effect=_fetch_and_save, + ) + ) + stack.enter_context( + patch("parsedmarc.cli.IMAPConnection", return_value=object()) + ) + mocks = [stack.enter_context(patcher) for patcher in patches] + stack.enter_context( + patch.object(sys, "argv", ["parsedmarc", "-c", config_path]) + ) + cm = stack.enter_context(self.assertLogs("parsedmarc.log", level="ERROR")) + exit_code = None + try: + parsedmarc.cli._main() + except SystemExit as system_exit: + exit_code = system_exit.code + return verdicts, cm.output, exit_code, mocks + + def _assert_unsaved(self, result, destination, message): + verdicts, output, exit_code, _mocks = result + self.assertIsNone(exit_code) + self.assertEqual(verdicts, [False]) + expected = f"{destination} exception Error: {message}" + self.assertTrue(any(expected in line for line in output), output) + self.assertFalse(any("Mailbox Error" in line for line in output), output) + + def _search_backend_patches(self, module, saver, error): + return ( + patch(f"parsedmarc.cli.{module}.set_hosts"), + patch(f"parsedmarc.cli.{module}.migrate_indexes"), + patch(f"parsedmarc.cli.{module}.{saver}", side_effect=error), + ) + + def testStandardOutputEncodingErrorIsRecorded(self): + """A report string the console cannot encode (a lone surrogate) made + print() in process_reports() raise UnicodeEncodeError outside every + try.""" + report = { + "organization_name": "Comp\ud800any", + "begin_date": "2024-01-01T00:00:00Z", + "end_date": "2024-01-01T23:59:59Z", + "contact_info": "tls@attacker.example", + "report_id": "poc", + "policies": [], + } + stdout = io.TextIOWrapper(io.BytesIO(), encoding="utf-8") + with patch.object(sys, "stdout", stdout): + result = self._run("", {"smtp_tls_reports": [report]}, silent=False) + self._assert_unsaved( + result, "Standard output", "'utf-8' codec can't encode character" + ) + + def testStandardOutputClosedPipeIsRecorded(self): + """A closed pipe on standard output (BrokenPipeError) is recorded the + same way as an encoding error.""" + + class _ClosedPipe(io.StringIO): + def write(self, text): + raise BrokenPipeError(32, "Broken pipe") + + report = { + "organization_name": "Reporter Inc.", + "begin_date": "2024-01-01T00:00:00Z", + "end_date": "2024-01-01T23:59:59Z", + "contact_info": "tls@reporter.example", + "report_id": "pipe", + "policies": [], + } + with patch.object(sys, "stdout", _ClosedPipe()): + result = self._run("", {"smtp_tls_reports": [report]}, silent=False) + self._assert_unsaved(result, "Standard output", "[Errno 32] Broken pipe") + + def testFileOutputExceptionIsRecorded(self): + """A real TypeError from the SMTP TLS CSV writer (a policy_strings + list holding an integer), not a mock.""" + output_dir = tempfile.mkdtemp() + self.addCleanup(lambda: shutil.rmtree(output_dir, ignore_errors=True)) + report = { + "organization_name": "Attacker Inc.", + "begin_date": "2024-01-01T00:00:00Z", + "end_date": "2024-01-01T23:59:59Z", + "contact_info": "tls@attacker.example", + "report_id": "poc", + "policies": [ + { + "policy_domain": "example.com", + "policy_type": "sts", + "policy_strings": [1], + "successful_session_count": 1, + "failed_session_count": 0, + } + ], + } + result = self._run( + "", {"smtp_tls_reports": [report]}, general=f"output = {output_dir}" + ) + self._assert_unsaved( + result, + "File output", + "sequence item 0: expected str instance, int found", + ) + + def testSearchBackendFailureAndSmtpTlsExceptionsAreRecorded(self): + cases = ( + ("elastic", "Elasticsearch", "elasticsearch"), + ("opensearch", "OpenSearch", "opensearch"), + ) + report_types = ( + ("failure", "failure_reports", self._FAILURE), + ("smtp_tls", "smtp_tls_reports", self._SMTP_TLS), + ) + for module, destination, section in cases: + for report_type, batch_key, report in report_types: + saver = f"save_{report_type}_report_to_{section}" + with self.subTest(saver=saver): + result = self._run( + f"[{section}]\nhosts = localhost\n", + {batch_key: [report]}, + *self._search_backend_patches( + module, saver, IndexError("list index out of range") + ), + ) + self.assertIs(result[3][2].call_args.args[0], report) + self._assert_unsaved(result, destination, "list index out of range") + + def testPostgreSQLExceptionsAreRecorded(self): + report_types = ( + ("aggregate", "aggregate_reports", self._AGGREGATE), + ("failure", "failure_reports", self._FAILURE), + ("smtp_tls", "smtp_tls_reports", self._SMTP_TLS), + ) + for report_type, batch_key, report in report_types: + saver = f"save_{report_type}_report_to_postgresql" + message = "argument of type 'int' is not a container or iterable" + client_cls = MagicMock() + getattr(client_cls.return_value, saver).side_effect = TypeError(message) + with self.subTest(saver=saver): + result = self._run( + "[postgresql]\nhost = db.example.com\n", + {batch_key: [report]}, + patch("parsedmarc.cli.postgres.PostgreSQLClient", client_cls), + ) + client = client_cls.return_value + getattr(client, saver).assert_called_once_with(report) + self._assert_unsaved(result, "PostgreSQL", message) + + def testSplunkHecExceptionsAreRecorded(self): + report_types = ( + ("aggregate", "aggregate_reports", self._AGGREGATE), + ("failure", "failure_reports", self._FAILURE), + ("smtp_tls", "smtp_tls_reports", self._SMTP_TLS), + ) + for report_type, batch_key, report in report_types: + saver = f"save_{report_type}_reports_to_splunk" + message = "'int' object has no attribute 'replace'" + client_cls = MagicMock() + getattr(client_cls.return_value, saver).side_effect = AttributeError( + message + ) + with self.subTest(saver=saver): + result = self._run( + "[splunk_hec]\n" + "url = https://splunk.example.com:8088\n" + "token = test-token\n" + "index = dmarc\n", + {batch_key: [report]}, + patch("parsedmarc.cli.splunk.HECClient", client_cls), + ) + client = client_cls.return_value + getattr(client, saver).assert_called_once_with([report]) + self._assert_unsaved(result, "Splunk HEC", message) + + def testFailOnOutputErrorStillExitsThroughParserError(self): + """With fail_on_output_error the recorded exception is raised as + ParserError, as for any other output error, so _main exits 1 with + the output-error message rather than as a mailbox error.""" + verdicts, output, exit_code, _mocks = self._run( + "[elasticsearch]\nhosts = localhost\n", + {"failure_reports": [self._FAILURE]}, + *self._search_backend_patches( + "elastic", + "save_failure_report_to_elasticsearch", + IndexError("list index out of range"), + ), + general="fail_on_output_error = true", + ) + self.assertEqual(exit_code, 1) + self.assertEqual(verdicts, []) + self.assertTrue( + any( + "Output destination failures detected" in line + and "Elasticsearch exception Error: list index out of range" in line + for line in output + ), + output, + ) + self.assertFalse(any("Mailbox Error" in line for line in output), output) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tests/test_elastic.py b/tests/test_elastic.py index d4aec9f5..bbe5f1ab 100644 --- a/tests/test_elastic.py +++ b/tests/test_elastic.py @@ -8,18 +8,27 @@ construction, index naming, deduplication queries, error wrapping — without needing a running Elasticsearch cluster. """ +import copy +import json +import re import time import unittest from collections.abc import Iterator from contextlib import contextmanager -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone +from pathlib import Path from typing import cast from unittest.mock import MagicMock, call, patch from elasticsearch import NotFoundError import parsedmarc.elastic as elastic_module -from parsedmarc import InvalidFailureReport, parse_report_file +from parsedmarc import ( + InvalidFailureReport, + parse_aggregate_report_xml, + parse_report_email, + parse_report_file, +) from parsedmarc.elastic import ( AlreadySaved, ElasticsearchError, @@ -320,32 +329,40 @@ class TestSetHosts(unittest.TestCase): class TestCreateIndexes(unittest.TestCase): def test_creates_missing_index_with_default_settings(self): - with patch("parsedmarc.elastic.Index") as mock_index_cls: - mock_index = mock_index_cls.return_value - mock_index.exists.return_value = False + with ( + patch("parsedmarc.elastic.Index") as mock_index_cls, + patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn, + ): + mock_index_cls.return_value.exists.return_value = False create_indexes(["dmarc_aggregate-2024-01-15"]) - mock_index.settings.assert_called_once_with( - number_of_shards=1, number_of_replicas=0 + mock_get_conn.return_value.indices.create.assert_called_once_with( + index="dmarc_aggregate-2024-01-15", + body={"settings": {"number_of_shards": 1, "number_of_replicas": 0}}, ) - mock_index.create.assert_called_once() def test_creates_with_custom_settings(self): - with patch("parsedmarc.elastic.Index") as mock_index_cls: - mock_index = mock_index_cls.return_value - mock_index.exists.return_value = False + with ( + patch("parsedmarc.elastic.Index") as mock_index_cls, + patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn, + ): + mock_index_cls.return_value.exists.return_value = False create_indexes( ["idx"], settings={"number_of_shards": 3, "refresh_interval": "5s"} ) - mock_index.settings.assert_called_once_with( - number_of_shards=3, refresh_interval="5s" + body = mock_get_conn.return_value.indices.create.call_args.kwargs["body"] + self.assertEqual( + body, {"settings": {"number_of_shards": 3, "refresh_interval": "5s"}} ) def test_skips_existing_index(self): - with patch("parsedmarc.elastic.Index") as mock_index_cls: - mock_index = mock_index_cls.return_value - mock_index.exists.return_value = True - create_indexes(["idx"]) - mock_index.create.assert_not_called() + with ( + patch("parsedmarc.elastic.Index") as mock_index_cls, + patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn, + ): + mock_index_cls.return_value.exists.return_value = True + create_indexes(["idx"], mappings={"date_detection": False}) + mock_get_conn.return_value.indices.create.assert_not_called() + mock_get_conn.return_value.indices.put_mapping.assert_not_called() def test_wraps_sdk_error(self): with patch("parsedmarc.elastic.Index") as mock_index_cls: @@ -372,37 +389,36 @@ class TestCreateIndexesServerless(unittest.TestCase): def tearDown(self): elastic_module._SERVERLESS = self._original + def _created_body(self, **kwargs): + with ( + patch("parsedmarc.elastic.Index") as mock_index_cls, + patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn, + ): + mock_index_cls.return_value.exists.return_value = False + create_indexes(["idx"], **kwargs) + create = mock_get_conn.return_value.indices.create + create.assert_called_once() + return create.call_args.kwargs["body"] + def test_serverless_default_skips_settings_entirely(self): - with patch("parsedmarc.elastic.Index") as mock_index_cls: - mock_index = mock_index_cls.return_value - mock_index.exists.return_value = False - create_indexes(["idx"]) - mock_index.settings.assert_not_called() - mock_index.create.assert_called_once() + self.assertEqual(self._created_body(), {}) def test_serverless_filters_rejected_keys_and_passes_others_through(self): - with patch("parsedmarc.elastic.Index") as mock_index_cls: - mock_index = mock_index_cls.return_value - mock_index.exists.return_value = False - create_indexes( - ["idx"], - settings={ - "number_of_shards": 3, - "number_of_replicas": 2, - "refresh_interval": "5s", - }, - ) - mock_index.settings.assert_called_once_with(refresh_interval="5s") + body = self._created_body( + settings={ + "number_of_shards": 3, + "number_of_replicas": 2, + "refresh_interval": "5s", + }, + ) + self.assertEqual(body, {"settings": {"refresh_interval": "5s"}}) def test_serverless_skips_settings_when_only_rejected_keys(self): - with patch("parsedmarc.elastic.Index") as mock_index_cls: - mock_index = mock_index_cls.return_value - mock_index.exists.return_value = False - create_indexes( - ["idx"], settings={"number_of_shards": 3, "number_of_replicas": 2} - ) - mock_index.settings.assert_not_called() - mock_index.create.assert_called_once() + body = self._created_body( + settings={"number_of_shards": 3, "number_of_replicas": 2}, + mappings={"date_detection": False}, + ) + self.assertEqual(body, {"mappings": {"date_detection": False}}) # --------------------------------------------------------------------------- @@ -1629,7 +1645,7 @@ class TestCreateDataStreams(_DataStreamTestCase): name="dmarc_aggregate" ) # No concrete index is created for a stream. - mock_index_cls.return_value.create.assert_not_called() + client.indices.create.assert_not_called() self.assertIn("dmarc_aggregate", elastic_module._READY_DATA_STREAMS) def test_default_settings_are_one_shard_no_replicas(self): @@ -1803,7 +1819,7 @@ class TestSaveToDataStreams(_DataStreamTestCase): client.indices.create_data_stream.assert_called_once_with( name="cust_dmarc_aggregate_tenant_a" ) - mock_index_cls.return_value.create.assert_not_called() + client.indices.create.assert_not_called() # The dedup search still uses the wildcard, which matches the # stream as well as dated indexes written before the switch. self.assertEqual( @@ -1824,7 +1840,7 @@ class TestSaveToDataStreams(_DataStreamTestCase): index_call["body"]["@timestamp"], datetime(2024, 1, 15, tzinfo=timezone.utc), ) - mock_index_cls.return_value.create.assert_called_once() + client.indices.create.assert_called_once() client.indices.put_index_template.assert_not_called() client.indices.create_data_stream.assert_not_called() @@ -1843,7 +1859,7 @@ class TestSaveToDataStreams(_DataStreamTestCase): datetime(2024, 1, 1, tzinfo=timezone.utc), ) client.indices.create_data_stream.assert_called_once_with(name="dmarc_failure") - mock_index_cls.return_value.create.assert_not_called() + client.indices.create.assert_not_called() self.assertEqual( mock_search_cls.call_args.kwargs["index"], "dmarc_failure*,dmarc_forensic*", @@ -1877,7 +1893,7 @@ class TestSaveToDataStreams(_DataStreamTestCase): datetime(2024, 2, 3, tzinfo=timezone.utc), ) client.indices.create_data_stream.assert_called_once_with(name="cust_smtp_tls") - mock_index_cls.return_value.create.assert_not_called() + client.indices.create.assert_not_called() self.assertEqual(mock_search_cls.call_args.kwargs["index"], "cust_smtp_tls*") def test_smtp_tls_report_dated_mode_unchanged_apart_from_timestamp_and_op_type( @@ -1964,9 +1980,558 @@ class TestBackwardCompatAlias(unittest.TestCase): ) +class TestMappingGuards(_DataStreamTestCase): + """New indexes and data stream templates carry explicit mappings + (GHSA-qpcg-29vr-jwm3). + + Before 11.2.2 they were created with settings only, so the cluster + inferred every field's type from the first document, with + ``date_detection`` on: an aggregate report with ``report_id`` + ``2026-11-09`` mapped ``report_id`` as a date for its day, a failure + sample header ``Subject.x`` mapped ``sample.headers.subject`` as an + object, and invented header names used up the 1,000-field limit, after + which legitimate reports for that index were rejected. + """ + + def _save_dated(self, save_function, report): + with ( + _registered_client() as client, + patch("parsedmarc.elastic.Search") as mock_search_cls, + patch("parsedmarc.elastic.Index") as mock_index_cls, + ): + mock_search_cls.return_value.execute.return_value = [] + mock_index_cls.return_value.exists.return_value = False + save_function(report) + client.indices.create.assert_called_once() + return client + + def _created_mappings(self, save_function, report): + client = self._save_dated(save_function, report) + return client.indices.create.call_args.kwargs["body"]["mappings"] + + def _template_mappings(self, save_function, report): + with ( + _registered_client() as client, + patch("parsedmarc.elastic.Search") as mock_search_cls, + patch("parsedmarc.elastic.Index") as mock_index_cls, + ): + mock_search_cls.return_value.execute.return_value = [] + client.indices.get_data_stream.side_effect = _not_found() + mock_index_cls.return_value.exists.return_value = False + save_function(report, use_data_streams=True) + template = client.indices.put_index_template.call_args.kwargs + return template["template"]["mappings"] + + def _assert_dates_declared(self, mappings, names): + for name in names: + self.assertEqual(mappings["properties"][name], {"type": "date"}, name) + + def test_aggregate_index_turns_date_detection_off(self): + mappings = self._created_mappings( + save_aggregate_report_to_elasticsearch, _aggregate_report() + ) + self.assertIs(mappings["date_detection"], False) + self._assert_dates_declared( + mappings, ["@timestamp", "date_range", "date_begin", "date_end"] + ) + self.assertNotIn("sample", mappings["properties"]) + # report_id stays dynamic: with date detection off a date-shaped + # value maps it as a string, not a date. + self.assertNotIn("report_id", mappings["properties"]) + + def test_smtp_tls_index_turns_date_detection_off(self): + mappings = self._created_mappings( + save_smtp_tls_report_to_elasticsearch, _smtp_tls_report() + ) + self.assertIs(mappings["date_detection"], False) + self._assert_dates_declared( + mappings, ["@timestamp", "date_range", "date_begin", "date_end"] + ) + self.assertNotIn("sample", mappings["properties"]) + + def test_failure_index_maps_sample_headers_without_dynamic_fields(self): + mappings = self._created_mappings( + save_failure_report_to_elasticsearch, _failure_report() + ) + self.assertIs(mappings["date_detection"], False) + self._assert_dates_declared(mappings, ["@timestamp", "arrival_date"]) + sample = mappings["properties"]["sample"]["properties"] + self.assertEqual(sample["date"], {"type": "date"}) + headers = sample["headers"] + self.assertEqual(headers["type"], "object") + self.assertIs(headers["dynamic"], False) + string_field = { + "type": "text", + "fields": {"keyword": {"type": "keyword", "ignore_above": 256}}, + } + for name in ("from", "to", "subject", "date", "message-id", "cc"): + self.assertEqual(headers["properties"][name], string_field, name) + for name in headers["properties"]: + # Stored names are lower-cased with "." replaced, so a declared + # name must be in that form or no stored header can match it. + self.assertEqual(name, name.lower().replace(".", "_"), name) + + def test_failure_mapping_declares_every_header_the_dashboards_query(self): + dashboards = Path(__file__).resolve().parent.parent / "dashboards" + referenced = set() + for path in dashboards.rglob("*"): + if path.is_file() and path.suffix in (".json", ".ndjson", ".xml"): + text = path.read_text(encoding="utf-8") + for match in re.finditer(r"sample\.headers\.([A-Za-z0-9_-]+)", text): + referenced.add(match.group(1).lower()) + self.assertIn("x-mailer", referenced) + mappings = self._created_mappings( + save_failure_report_to_elasticsearch, _failure_report() + ) + declared = mappings["properties"]["sample"]["properties"]["headers"][ + "properties" + ] + self.assertLessEqual(referenced, set(declared)) + + def test_data_stream_templates_carry_the_mappings(self): + for save_function, report, has_sample in ( + (save_aggregate_report_to_elasticsearch, _aggregate_report(), False), + (save_failure_report_to_elasticsearch, _failure_report(), True), + (save_smtp_tls_report_to_elasticsearch, _smtp_tls_report(), False), + ): + case = save_function.__name__ + elastic_module._READY_DATA_STREAMS.clear() + mappings = self._template_mappings(save_function, report) + self.assertIs(mappings["date_detection"], False, case) + self.assertEqual( + mappings["properties"]["@timestamp"], {"type": "date"}, case + ) + self.assertEqual("sample" in mappings["properties"], has_sample, case) + if has_sample: + headers = mappings["properties"]["sample"]["properties"]["headers"] + self.assertIs(headers["dynamic"], False, case) + self.assertIn("subject", headers["properties"], case) + + def test_data_stream_template_adds_timestamp_without_changing_the_argument( + self, + ): + mappings = {"date_detection": False, "properties": {"x": {"type": "long"}}} + with ( + _registered_client() as client, + patch("parsedmarc.elastic.Index") as mock_index_cls, + ): + client.indices.get_data_stream.side_effect = _not_found() + mock_index_cls.return_value.exists.return_value = False + create_data_streams(["s"], mappings=mappings) + template = client.indices.put_index_template.call_args.kwargs + self.assertEqual( + template["template"]["mappings"], + { + "date_detection": False, + "properties": {"x": {"type": "long"}, "@timestamp": {"type": "date"}}, + }, + ) + self.assertEqual( + mappings, {"date_detection": False, "properties": {"x": {"type": "long"}}} + ) + + def test_dotted_header_name_is_stored_with_an_underscore(self): + report = _failure_report() + report["parsed_sample"] = dict(report["parsed_sample"]) + report["parsed_sample"]["headers"] = { + "From": [["", "sender@example.com"]], + "Subject.x": "v", + "X.Y.Z": "w", + } + client = self._save_dated(save_failure_report_to_elasticsearch, report) + headers = client.index.call_args.kwargs["body"]["sample"]["headers"] + self.assertEqual(headers["subject_x"], "v") + self.assertEqual(headers["x_y_z"], "w") + self.assertNotIn("subject.x", headers) + self.assertNotIn("subject", headers) + # The address headers are still flattened under their usual keys. + self.assertEqual(headers["from"], "sender@example.com") + + # Silence unused-import lint in the test module preamble. _ = call +class TestAggregateIndexCap(unittest.TestCase): + """One report may create at most MAX_INDEXES_PER_REPORT dated indexes; + the overflow records are saved to the report's first index.""" + + DAYS = 40 + + def _report(self): + report = _aggregate_report() + template = report["records"][0] + report["records"] = [] + for day in range(self.DAYS): + record = copy.deepcopy(template) + begin = datetime(2024, 1, 1, tzinfo=timezone.utc) + timedelta(days=day) + end = begin + timedelta(days=1) + record["interval_begin"] = begin.strftime("%Y-%m-%d %H:%M:%S") + record["interval_end"] = end.strftime("%Y-%m-%d %H:%M:%S") + report["records"].append(record) + return report + + def _save(self, **kwargs): + """Save the report; return created index names, the (index, + date_begin) of each saved document and the logger mock.""" + saved = [] + + def capture(doc, *args, **save_kwargs): + saved.append((doc.meta.index, doc.date_begin)) + + with ( + patch("parsedmarc.elastic.Search", return_value=_empty_search()), + patch("parsedmarc.elastic.Index") as mock_index_cls, + patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn, + patch("parsedmarc.elastic.logger") as mock_logger, + patch("parsedmarc.elastic.create_data_streams") as mock_streams, + patch.object( + elastic_module._AggregateReportDoc, + "save", + autospec=True, + side_effect=capture, + ), + ): + mock_index_cls.return_value.exists.return_value = False + save_aggregate_report_to_elasticsearch(self._report(), **kwargs) + created = [ + c.kwargs["index"] + for c in mock_get_conn.return_value.indices.create.call_args_list + ] + return created, saved, mock_logger, mock_streams + + def test_daily_indexes_are_capped_and_overflow_goes_to_the_first(self): + created, saved, mock_logger, _ = self._save() + expected = [f"dmarc_aggregate-2024-01-{d:02d}" for d in range(1, 32)] + self.assertEqual(sorted(set(created)), expected) + self.assertEqual(len(saved), self.DAYS) + self.assertEqual([index for index, _ in saved[:31]], expected) + self.assertEqual( + [index for index, _ in saved[31:]], ["dmarc_aggregate-2024-01-01"] * 9 + ) + # The overflow records keep their own dates. + begins = [begin for _, begin in saved] + self.assertEqual(begins[31], datetime(2024, 2, 1, tzinfo=timezone.utc)) + self.assertEqual(begins[39], datetime(2024, 2, 9, tzinfo=timezone.utc)) + mock_logger.warning.assert_called_once() + self.assertIn("9 records", mock_logger.warning.call_args.args[0]) + + def test_routing_warning_escapes_the_report_id(self): + """The report ID is sender-controlled; a CR/LF in it must not forge + a log line.""" + report = self._report() + report["report_metadata"]["report_id"] = "poc\r\nforged" + with ( + patch("parsedmarc.elastic.Search", return_value=_empty_search()), + patch("parsedmarc.elastic.Index") as mock_index_cls, + patch("parsedmarc.elastic.connections.get_connection"), + patch("parsedmarc.elastic.logger") as mock_logger, + patch.object(elastic_module._AggregateReportDoc, "save", autospec=True), + ): + mock_index_cls.return_value.exists.return_value = False + save_aggregate_report_to_elasticsearch(report) + message = mock_logger.warning.call_args.args[0] + self.assertIn("poc\\r\\nforged", message) + self.assertNotIn("\n", message) + + def test_monthly_indexes_are_not_affected(self): + created, saved, mock_logger, _ = self._save(monthly_indexes=True) + self.assertEqual( + sorted(set(created)), ["dmarc_aggregate-2024-01", "dmarc_aggregate-2024-02"] + ) + self.assertEqual( + [index for index, _ in saved], + ["dmarc_aggregate-2024-01"] * 31 + ["dmarc_aggregate-2024-02"] * 9, + ) + mock_logger.warning.assert_not_called() + + def test_data_streams_are_not_affected(self): + created, saved, mock_logger, mock_streams = self._save(use_data_streams=True) + self.assertEqual(created, []) + self.assertEqual({index for index, _ in saved}, {"dmarc_aggregate"}) + self.assertEqual(len(saved), self.DAYS) + self.assertEqual(mock_streams.call_count, self.DAYS) + mock_logger.warning.assert_not_called() + + +class TestSaveFailureReportEmptyAddressHeaders(unittest.TestCase): + """GHSA-4272-4r8g-6c5h: a failure sample whose From, To or Reply-To + header holds no address made the save index ``headers[...][0]`` of + mail-parser's empty list and raise ``IndexError``, which escaped the + CLI and stopped the mailbox run. The header is now left out of the + document and of the duplicate query.""" + + # An empty From: and Reply-To:, and To: undisclosed-recipients:; (an + # RFC 5322 §3.4 group with no members, which legitimate mail uses). + ARF = ( + "From: dmarc@reporter.example\n" + "To: ruf@example.com\n" + "Date: Mon, 01 Jan 2024 00:00:00 +0000\n" + "Subject: DMARC Failure Report\n" + "MIME-Version: 1.0\n" + 'Content-Type: multipart/report; report-type=feedback-report; boundary="b"\n' + "\n" + "--b\n" + "Content-Type: text/plain\n" + "\n" + "Authentication failure report.\n" + "--b\n" + "Content-Type: message/feedback-report\n" + "\n" + "Feedback-Type: auth-failure\n" + "User-Agent: test/1.0\n" + "Version: 1\n" + "Original-Mail-From: user@example.com\n" + "Arrival-Date: Mon, 01 Jan 2024 00:00:00 +0000\n" + "Source-IP: 192.0.2.1\n" + "Reported-Domain: example.com\n" + "Authentication-Results: mx; dmarc=fail\n" + "\n" + "--b\n" + "Content-Type: text/rfc822-headers\n" + "\n" + "From:\n" + "To: undisclosed-recipients:;\n" + "Reply-To:\n" + "Subject: spoofed\n" + "Date: Mon, 01 Jan 2024 00:00:00 +0000\n" + "--b--\n" + ) + + def test_empty_address_headers_are_left_out(self): + parsed = parse_report_email(self.ARF, offline=True) + self.assertEqual(parsed["report_type"], "failure") + report = cast(dict, parsed["report"]) + headers = report["parsed_sample"]["headers"] + self.assertEqual( + (headers["From"], headers["To"], headers["Reply-To"]), ([], [], []) + ) + search = _empty_search() + with ( + patch("parsedmarc.elastic.Search", return_value=search), + patch("parsedmarc.elastic.Index"), + patch.object( + elastic_module._FailureReportDoc, "save", autospec=True + ) as mock_save, + ): + save_failure_report_to_elasticsearch(report) + doc = mock_save.call_args.args[0] + self.assertEqual(doc.sample.headers["subject"], "spoofed") + for header in ("from", "to", "reply-to"): + self.assertNotIn(header, doc.sample.headers) + query = json.dumps(search.query.to_dict()) + self.assertIn("sample.headers.subject", query) + self.assertNotIn("sample.headers.from", query) + self.assertNotIn("sample.headers.to", query) + + +class TestAggregateAttributesDoNotReachTheDocument(unittest.TestCase): + """GHSA-qpcg-29vr-jwm3: an XML attribute on a scalar element reached the + document as an object and mapped the field as one.""" + + def test_document_holds_no_object_for_attributed_fields(self): + attribute = ' a="1"' + xml = ( + f"1.0" + "example.neta@example.net" + "r1704067200" + "1704153599" + "example.com" + f"none

1" + "192.0.2.11" + f"none" + "passpass" + "example.com" + f"example.org" + "example.compass" + "" + ) + report = parse_aggregate_report_xml(xml, offline=True) + docs = [] + with ( + patch("parsedmarc.elastic.Search", return_value=_empty_search()), + patch("parsedmarc.elastic.Index") as mock_index_cls, + patch("parsedmarc.elastic.connections.get_connection"), + patch.object( + elastic_module._AggregateReportDoc, + "save", + autospec=True, + side_effect=lambda doc, *args, **kwargs: docs.append(doc.to_dict()), + ), + ): + mock_index_cls.return_value.exists.return_value = False + save_aggregate_report_to_elasticsearch(cast(dict, report)) + doc = docs[0] + self.assertEqual(doc["xml_schema"], "1.0") + self.assertEqual(doc["disposition"], "none") + self.assertEqual(doc["envelope_to"], "example.org") + self.assertEqual(doc["published_policy"]["p"], "none") + self.assertEqual(doc["published_policy"]["fo"], "1") + + +# Every element of an aggregate report that the schema types as a string, +# with its plain value; {name} placeholders take a variant. +_SCALAR_FIELD_VALUES = { + "version": "1.0", + "org_name": "reporter.example", + "email": "dmarc@reporter.example", + "extra_contact_info": "https://reporter.example/dmarc", + "report_id": "scalar-test", + "generator": "reporter/1.0", + "domain": "example.com", + "adkim": "r", + "aspf": "s", + "p": "none", + "sp": "quarantine", + "pct": "100", + "fo": "1", + "np": "reject", + "testing": "n", + "discovery_method": "psl", + "source_ip": "192.0.2.1", + "count": "3", + "disposition": "none", + "dkim": "pass", + "spf": "fail", + "header_from": "example.com", + "envelope_from": "bounce.example.com", + "envelope_to": "example.org", + "dkim_domain": "example.com", + "selector": "s1", + "dkim_result": "pass", + "dkim_human_result": "good signature", + "spf_domain": "bounce.example.com", + "scope": "mfrom", + "spf_result": "fail", + "spf_human_result": "not listed", +} +_SCALAR_FIELD_TEMPLATE = ( + "{version}" + "{org_name}{email}" + "{extra_contact_info}" + "{report_id}{generator}" + "17040672001704153599" + "{domain}" + "{adkim}{aspf}

{p}

{sp}" + "{pct}{fo}{np}{testing}" + "{discovery_method}" + "{source_ip}{count}" + "{disposition}" + "{dkim}{spf}" + "otherc" + "{header_from}" + "{envelope_from}" + "{envelope_to}" + "{dkim_domain}{selector}" + "{dkim_result}{dkim_human_result}" + "{spf_domain}{scope}" + "{spf_result}{spf_human_result}" + "" +) + + +def _scalar_field_xml(**variants: str) -> str: + """The report with each named field's value replaced by its variant.""" + return _SCALAR_FIELD_TEMPLATE.format(**{**_SCALAR_FIELD_VALUES, **variants}) + + +# Fields whose element can hold child elements without the report being +# rejected. +_OPTIONAL_SCALAR_FIELDS = tuple( + name + for name in _SCALAR_FIELD_VALUES + if name + not in ( + "org_name", + "report_id", + "domain", + "p", + "source_ip", + "count", + "header_from", + "dkim_domain", + "spf_domain", + ) +) + + +# The fields of an aggregate report document that hold objects or lists; +# "[]" stands for a list item. Every other field must be a scalar. +_AGGREGATE_DOC_CONTAINERS = { + "date_range", + "published_policy", + "policy_overrides", + "policy_overrides[]", + "dkim_results", + "dkim_results[]", + "dkim_results_combined", + "spf_results", + "spf_results[]", + "spf_results_combined", + "errors", +} + + +def _container_paths(value, path=""): + """Yield the path of every dict and list inside ``value``.""" + if isinstance(value, dict): + if path: + yield path + for key, item in value.items(): + yield from _container_paths(item, f"{path}.{key}" if path else key) + elif isinstance(value, list): + yield path + for item in value: + yield from _container_paths(item, f"{path}[]") + + +# A second makes the field a list; the first one's

+# holds a child element. +_REPEATED_POLICY_VARIANTS = { + "p": "1", + "discovery_method": ( + "psl" + "example.com

none

psl" + ), +} + + +class TestAggregateScalarFieldsReachTheDocumentAsScalars(unittest.TestCase): + """GHSA-qpcg-29vr-jwm3: a child element under a field the schema types + as a string reached the document as an object and mapped the field as + one. The walk covers every field, so a scalar field added later is + checked too.""" + + def _document(self, **variants): + report = parse_aggregate_report_xml(_scalar_field_xml(**variants), offline=True) + docs = [] + with ( + patch("parsedmarc.elastic.Search", return_value=_empty_search()), + patch("parsedmarc.elastic.Index") as mock_index_cls, + patch("parsedmarc.elastic.connections.get_connection"), + patch.object( + elastic_module._AggregateReportDoc, + "save", + autospec=True, + side_effect=lambda doc, *args, **kwargs: docs.append(doc.to_dict()), + ), + ): + mock_index_cls.return_value.exists.return_value = False + save_aggregate_report_to_elasticsearch(cast(dict, report)) + return docs[0] + + def test_no_object_outside_the_object_fields(self): + mixed = {name: f"{value}" for name, value in _SCALAR_FIELD_VALUES.items()} + child = {name: "1" for name in _OPTIONAL_SCALAR_FIELDS} + for label, variants in ( + ("mixed", mixed), + ("child", child), + ("repeated policy_published", _REPEATED_POLICY_VARIANTS), + ): + paths = set(_container_paths(self._document(**variants))) + self.assertIn("published_policy", paths, msg=label) + self.assertLessEqual(paths, _AGGREGATE_DOC_CONTAINERS, msg=label) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tests/test_init.py b/tests/test_init.py index 3dd59a09..5954831f 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -10,6 +10,7 @@ import binascii import csv import email import email.errors +import gc import gzip import inspect import json @@ -19,7 +20,10 @@ import os import quopri import time import traceback +import tracemalloc import unittest +import zipfile +import weakref from collections.abc import Callable from datetime import datetime, timedelta, timezone from email.mime.application import MIMEApplication @@ -31,7 +35,7 @@ from io import BytesIO, StringIO from pathlib import Path from shutil import rmtree from tempfile import NamedTemporaryFile, mkdtemp -from typing import BinaryIO, cast +from typing import Any, BinaryIO, cast from unittest.mock import MagicMock, patch from lxml import etree # type: ignore[import-untyped] @@ -45,6 +49,7 @@ from parsedmarc.types import ( ParsingResults, SMTPTLSReport, ) +from tests.timing import budget # Detect if running in GitHub Actions to skip DNS lookups OFFLINE_MODE = os.environ.get("GITHUB_ACTIONS", "false").lower() == "true" @@ -2587,7 +2592,7 @@ class TestExtractReport(unittest.TestCase): gzip with a single unbounded zlib.decompress(), so a small attachment from any sender a monitored mailbox accepts could allocate hundreds of MB. The limit is patched down here so the test - stays fast; the real 100 MiB cap is the same code path.""" + stays fast; the real 32 MiB cap is the same code path.""" limit = 4096 compressed = gzip.compress(b"a" * (limit + 1)) # The attack is amplification: the wire form is far below the cap. @@ -2777,12 +2782,13 @@ class TestMalformedXmlRecovery(unittest.TestCase): 19 s and this 5 MB input would take minutes. The 10 s bound is not flaky: the fixed pattern handles it in milliseconds, orders of magnitude below both the bound and the old pattern's cost.""" + limit = budget(10) start = time.perf_counter() with self.assertRaises(parsedmarc.InvalidAggregateReport): parsedmarc.parse_aggregate_report_xml( "]>" f"{'' * 10_000}" ) + limit = budget(1) start = time.perf_counter() with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: parsedmarc.parse_aggregate_report_xml(xml, offline=OFFLINE_MODE) - self.assertLess(time.perf_counter() - start, 1) + self.assertLess(time.perf_counter() - start, limit) self.assertIn("DOCTYPE", str(ctx.exception)) @@ -3147,6 +3154,97 @@ class TestParseReportFile(unittest.TestCase): parsedmarc.parse_report_file(b"this is not a report", offline=True) self.assertIn("recognized report format", str(ctx.exception)) + def testParseReportFileRejectsOversizedXmlWithoutTheEmailParser(self): + """A file the size cap rejects was handed whole to the email parser, + which has no size cap: 200 MiB of reached +2,801 MiB in 4.6 s. + Content that sniffs as XML or JSON, or is over the cap, is not tried + as an email.""" + content = b"" + b"" * (10 * 2**20) + b"" + limit = budget(2.0) + tracemalloc.start() + try: + start = time.perf_counter() + with patch("parsedmarc.parse_report_email") as email_parser: + with self.assertRaises(parsedmarc.ParserError) as ctx: + parsedmarc.parse_report_file(content, offline=True) + elapsed = time.perf_counter() - start + peak = tracemalloc.get_traced_memory()[1] + finally: + tracemalloc.stop() + email_parser.assert_not_called() + self.assertIn("exceeds the 33554432 byte limit", str(ctx.exception)) + self.assertLess(elapsed, limit) + self.assertLess(peak, 100 * 2**20) + + def testParseReportFileCapsTextForTheEmailParser(self): + """Text over the size cap that does not look like XML or JSON is not + handed to the email parser either.""" + content = b"From: a@example.com\n\n" + b"x" * ( + parsedmarc.MAX_DECOMPRESSED_REPORT_SIZE + 1 + ) + with patch("parsedmarc.parse_report_email") as email_parser: + with self.assertRaises(parsedmarc.ParserError) as ctx: + parsedmarc.parse_report_file(content, offline=True) + email_parser.assert_not_called() + self.assertEqual( + str(ctx.exception), "Invalid report email: the report is larger than 32 MiB" + ) + + def testParseReportFileSkipsParsersThatCannotMatch(self): + """Content that begins with "<" is not tried as JSON or as an email, + and content that begins with "{" is not tried as an email, however + small.""" + with ( + patch("parsedmarc.parse_smtp_tls_report_json") as smtp_tls_parser, + patch("parsedmarc.parse_report_email") as email_parser, + self.assertRaises(parsedmarc.ParserError), + ): + parsedmarc.parse_report_file(b"", offline=True) + smtp_tls_parser.assert_not_called() + email_parser.assert_not_called() + with ( + patch("parsedmarc.parse_report_email") as email_parser, + self.assertRaises(parsedmarc.ParserError) as ctx, + ): + parsedmarc.parse_report_file(b'{"a": 1}', offline=True) + email_parser.assert_not_called() + self.assertIsInstance(ctx.exception.__cause__, parsedmarc.InvalidSMTPTLSReport) + + def testParseReportFileReleasesEachFailedParse(self): + """parse_report_file() kept the aggregate parser's exception, and its + traceback's frames holding the parsed tree, while it tried the other + formats: a 31 MiB file at the cap cost +1,046 MiB there + against +399 MiB parsing the XML directly.""" + + class _Tree(dict): + pass + + trees: list[weakref.ref] = [] + alive_at_report: list[bool] = [] + original_parse = parsedmarc._parse_limited_xml + original_describe = parsedmarc._describe_parse_failure + + def parse(xml): + parsed = original_parse(xml) + tree = _Tree(parsed["feedback"] or {}) + trees.append(weakref.ref(tree)) + parsed["feedback"] = tree + return parsed + + def describe(*args): + gc.collect() + alive_at_report.append(any(ref() is not None for ref in trees)) + return original_describe(*args) + + with ( + patch.object(parsedmarc, "_parse_limited_xml", parse), + patch.object(parsedmarc, "_describe_parse_failure", describe), + self.assertRaises(parsedmarc.ParserError), + ): + parsedmarc.parse_report_file(b"1", offline=True) + self.assertTrue(trees) + self.assertEqual(alive_at_report, [False]) + def testParseReportFilePreservesCause(self): """The raised ParserError chains the underlying parse failure""" with self.assertRaises(parsedmarc.ParserError) as ctx: @@ -3488,12 +3586,13 @@ This is not a DMARC report.""" "Content-Type: text/plain\n\n" "A message claiming to be from you has failed\n" + body + "\ndetected." ) + limit = budget(10) start = time.perf_counter() # The required received-date field is missing, so the notice is # rejected rather than parsed. with self.assertRaises(parsedmarc.InvalidDMARCReport): parsedmarc.parse_report_email(msg, offline=OFFLINE_MODE) - self.assertLess(time.perf_counter() - start, 10) + self.assertLess(time.perf_counter() - start, limit) def testTextReportFieldNamesMustBeSingleLine(self): """text_report_regex matches Exim's one-line ``Name: value`` fields: @@ -3530,10 +3629,34 @@ This is not a DMARC report.""" """Multiparts nested past the recursion limit raise ``ParserError``, not ``RecursionError`` (GHSA-qxwv-2vwj-v828); a shallow nesting is walked without error and rejected as not a report, as before.""" - with self.assertRaises(parsedmarc.ParserError) as ctx: + # The message guard rejects this many multiparts before parsing + # (GHSA-qcc2-4x3v-v4g7); switch it off to reach the recursion guard + # behind it. + with self.assertRaises(parsedmarc.InvalidDMARCReport) as ctx: parsedmarc.parse_report_email( _nested_multipart_email(POISON_NESTING_LEVELS), offline=True ) + self.assertIn("max_depth (100)", str(ctx.exception)) + + with patch("parsedmarc.check_message_limits"): + # Multiparts are walked, never serialized, so the nesting alone + # no longer reaches the recursion limit (GHSA-qcc2-4x3v-v4g7). + with self.assertRaises(parsedmarc.InvalidDMARCReport) as ctx: + parsedmarc.parse_report_email( + _nested_multipart_email(POISON_NESTING_LEVELS), offline=True + ) + self.assertIn("is not a valid report", str(ctx.exception)) + # A failure report's sample is serialized, so the same nesting as + # its sample still reaches the recursion guard. + sample = _nested_multipart_email(POISON_NESTING_LEVELS).decode() + report = ( + "Subject: s\nContent-Type: multipart/report; boundary=r\n\n" + "--r\nContent-Type: message/feedback-report\n\n" + "Feedback-Type: auth-failure\n\n" + f"--r\nContent-Type: message/rfc822\n\n{sample}--r--\n" + ) + with self.assertRaises(parsedmarc.ParserError) as ctx: + parsedmarc.parse_report_email(report, offline=True) self.assertIn("recursion", str(ctx.exception)) with self.assertRaises(parsedmarc.InvalidDMARCReport) as ctx: @@ -3965,6 +4088,15 @@ class TestGetDmarcReportsFromMbox(unittest.TestCase): box.close() for n_procs in (1, 2): with self.subTest(poison=poison_name, n_procs=n_procs): + if poison_name == "nested": + # The message guard rejects it as an invalid report + # (GHSA-qcc2-4x3v-v4g7), which the loop skips. + parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear() + result = parsedmarc.get_dmarc_reports_from_mbox( + path, offline=True, n_procs=n_procs + ) + self.assertEqual(len(result["aggregate_reports"]), 1) + continue with self.assertRaises(parsedmarc.ParserError) as ctx: parsedmarc.get_dmarc_reports_from_mbox( path, offline=True, n_procs=n_procs @@ -4613,8 +4745,13 @@ class TestGetDmarcReportsFromMailboxContentErrors(unittest.TestCase): self.assertEqual(conn.folders["INBOX"], {}) self.assertEqual(list(conn.folders["Archive/Invalid"]), [1]) self.assertEqual(list(conn.folders["Archive/Aggregate"]), [2]) + expected_text = ( + "Message rejected" + if poison_name == "nested" + else "Unable to parse message" + ) self.assertTrue( - any("Unable to parse message" in line for line in output), + any(expected_text in line for line in output), output, ) # The nested message is a poison on every Python. @@ -5025,11 +5162,13 @@ class TestGetDmarcReportsFromMailboxMaildir(unittest.TestCase): self.assertEqual(len(result["aggregate_reports"]), 1) self.assertEqual(result["failure_reports"], []) self.assertEqual(len(conn.fetch_messages("Archive/Aggregate")), 1) + expected_text = ( + "Message rejected" + if poison_name == "nested" + else "Unable to parse message with subject" + ) self.assertTrue( - any( - "Unable to parse message with subject" in line - for line in cm.output - ), + any(expected_text in line for line in cm.output), cm.output, ) # The poison message was moved, not left to fail again. @@ -7024,5 +7163,1587 @@ class TestAggregateReportIdentityDomainScopePendingSave(unittest.TestCase): self.assertEqual(self._count_by_domain(retry), {"a.example": 3, "b.example": 5}) +class TestMessageStructureLimits(unittest.TestCase): + """Messages whose structure makes parsing slow are rejected up front + (GHSA-qcc2-4x3v-v4g7). + + Each input is a shape from the advisory. The time limits are generous: + the guard is one linear pass, and the timings cited are those measured + on 11.2.1 under Python 3.12 before the guard existed.""" + + HEAD = ( + "From: sender@example.net\r\nTo: dmarc@example.com\r\nSubject: x\r\n" + "Date: Thu, 1 Jan 2026 00:00:00 +0000\r\nMIME-Version: 1.0\r\n" + ) + + def setUp(self): + import resource + + soft, hard = resource.getrlimit(resource.RLIMIT_AS) + self.addCleanup(resource.setrlimit, resource.RLIMIT_AS, (soft, hard)) + limit = 3 * 1024**3 + if hard != resource.RLIM_INFINITY: + limit = min(limit, hard) + resource.setrlimit(resource.RLIMIT_AS, (limit, hard)) + + def _assert_rejected_quickly(self, data): + limit = budget(2.0) + start = time.monotonic() + with self.assertRaises(parsedmarc.InvalidDMARCReport) as caught: + parsedmarc.parse_report_email(data.encode(), offline=True) + elapsed = time.monotonic() - start + self.assertIn("Message rejected", str(caught.exception)) + self.assertLess(elapsed, limit) + + def test_blank_lines_after_delivery_status_part(self): + """512 KiB of blank lines after a message/delivery-status part took + 6.1 s to parse on 11.2.1""" + data = ( + self.HEAD + + "Content-Type: message/delivery-status\r\n\r\n" + + "\n" * (512 * 1024) + ) + self._assert_rejected_quickly(data) + + def test_address_group_with_many_members(self): + """A To: group of 250,000 members took 1.9 s to parse on 11.2.1""" + group = "g:" + ",".join("a@x" for _ in range(250_000)) + ";" + data = f"From: a@b\r\nTo: {group}\r\nSubject: s\r\n\r\nbody\r\n" + self._assert_rejected_quickly(data) + + def test_nested_multiparts(self): + """900 nested multiparts around 30,000 parts took 10.9 s to parse on + 11.2.1""" + depth, parts = 900, 30000 + nested = "".join( + f"Content-Type: multipart/mixed; boundary=b{i}\n\n--b{i}\n" + for i in range(depth) + ) + inner = ( + "Content-Type: multipart/mixed; boundary=L\n\n" + + "--L\n\nx\n" * parts + + "--L--\n" + ) + close = "".join(f"--b{i}--\n" for i in range(depth - 1, -1, -1)) + self._assert_rejected_quickly(self.HEAD + nested + inner + close) + + def test_header_flood(self): + """A 4 MiB flood of ``X-A: b`` header lines took 1.4 s to parse on + 11.2.1""" + data = self.HEAD + "X-A: b\r\n" * (4 * 1024 * 1024 // 8) + "\r\nbody\r\n" + self._assert_rejected_quickly(data) + + def test_failure_report_sample_over_a_limit(self): + """A failure report whose message/rfc822 sample exceeds a limit is an + InvalidFailureReport, not a bare ValueError or an unexpected error""" + feedback_report = "\n".join( + [ + "Feedback-Type: auth-failure", + "Version: 1", + "Source-IP: 203.0.113.1", + "Reported-Domain: example.com", + "Auth-Failure: dmarc", + ] + ) + # Each blank-line-separated block of a message/delivery-status body + # is a part, so 200,000 blank lines pass the part limit. + sample = ( + "From: a@example.com\nSubject: s\n" + "Content-Type: message/delivery-status\n\n" + "\n" * 200_000 + ) + with self.assertRaises(parsedmarc.InvalidFailureReport) as caught: + parsedmarc.parse_failure_report( + feedback_report, + sample, + datetime(2024, 1, 1, tzinfo=timezone.utc), + offline=True, + ) + self.assertIn( + "Sample message rejected: Message exceeds max_parts (10000)", + str(caught.exception), + ) + self.assertNotIn("Unexpected error", str(caught.exception)) + + def test_normal_message_is_not_rejected(self): + """A message within the limits reaches the report parsers""" + with self.assertRaises(parsedmarc.InvalidDMARCReport) as caught: + parsedmarc.parse_report_email( + (self.HEAD + "\r\nhello\r\n").encode(), offline=True + ) + self.assertNotIn("Message rejected", str(caught.exception)) + + +class TestAggregateReportXMLLimits(unittest.TestCase): + """The nesting depth and node limits of ``parse_aggregate_report_xml()`` + (GHSA-q7gm-8cj9-94hr).""" + + SAMPLE = "samples/aggregate/!example.com!1538204542!1538463818.xml" + MALFORMED_DECLARATION = '' + + def _sample(self, inner="", declaration=None): + """The sample report with ``inner`` inserted at the start of its + ```` element (depth 2), and its XML declaration + replaced by ``declaration`` when given""" + with open(self.SAMPLE, encoding="utf-8") as f: + original = f.read() + header, body = original.split("?>", 1) + self.assertIn("", body) + body = body.replace("", "" + inner, 1) + return (declaration or header + "?>") + body + + @staticmethod + def _nested(levels): + return "" * levels + "x" + "" * levels + + def testUnclosedElementsAreRejectedQuickly(self): + """The advisory's document of unclosed ```` tags is rejected as + soon as it is nested too deep, before xmltodict builds the tree. + + Regression test for GHSA-q7gm-8cj9-94hr: xmltodict keeps one stack + entry per open element and expat reports the missing end tags only + at the end of the input, so the reporter measured 1.35 GB of RSS for + 10 MiB. Measured locally with 11.2.1 (Python 3.14), this 2 MiB input + took 1.2 s, +255 MiB of RSS and a 236 MiB tracemalloc peak; the + triage measured +117 MiB and 1.3 s. The fixed code rejects it in + milliseconds. Memory is measured with tracemalloc rather than + ``ru_maxrss``, which only grows past the process's earlier peak and + so could not show an increase after other tests in the suite.""" + head = 'x' + xml = head + "" * ((2 * 2**20 - len(head)) // 3) + limit = budget(1) + start = time.perf_counter() + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertLess(time.perf_counter() - start, limit) + self.assertIn( + f"nested more than {constants.MAX_XML_DEPTH} levels", str(ctx.exception) + ) + + tracemalloc.start() + try: + with self.assertRaises(parsedmarc.InvalidAggregateReport): + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + peak = tracemalloc.get_traced_memory()[1] + finally: + tracemalloc.stop() + self.assertLess(peak, 100 * 2**20) + + def testDepthLimitBoundary(self): + """A document nested exactly ``MAX_XML_DEPTH`` levels deep parses; + one level more is rejected.""" + # and are the first two levels. + within = constants.MAX_XML_DEPTH - 2 + report = parsedmarc.parse_aggregate_report_xml( + self._sample(self._nested(within)), offline=OFFLINE_MODE + ) + self.assertGreater(len(report["records"]), 0) + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml( + self._sample(self._nested(within + 1)), offline=OFFLINE_MODE + ) + self.assertIn("levels deep", str(ctx.exception)) + + def testSixtyLevelsDeepReportStillParses(self): + """A valid report with a 60-level-deep extension element parses like + the sample it was made from.""" + expected = parsedmarc.parse_aggregate_report_xml( + self._sample(), offline=OFFLINE_MODE + ) + report = parsedmarc.parse_aggregate_report_xml( + self._sample(self._nested(60)), offline=OFFLINE_MODE + ) + self.assertEqual(report["records"], expected["records"]) + self.assertEqual(report["report_metadata"]["errors"], []) + + def testWideDocumentIsRejectedOnTheNodeLimit(self): + """A well-formed document with more than ``MAX_XML_NODES`` sibling + elements is rejected on the node limit. + + Regression test for GHSA-q7gm-8cj9-94hr: the reporter measured + 1.75 GB of RSS for 40 MiB of ```` siblings. Empty ```` + siblings are the cheapest way to reach the shipped limit (about + 4 MiB here); ``testAttributesCountAsNodes`` covers attributes.""" + siblings = constants.MAX_XML_NODES # plus : one node over + xml = "" + "" * siblings + "" + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertIn( + f"more than {constants.MAX_XML_NODES:,} elements and attributes", + str(ctx.exception), + ) + + def testAttributesCountAsNodes(self): + """Elements and attributes both count toward the node limit, which + is checked exactly: at the limit the document parses, one node over + it is rejected. The limit is lowered so the inputs stay small.""" + instances = [] + original_init = parsedmarc._LimitedExpatParser.__init__ + + def record(proxy, parser): + original_init(proxy, parser) + instances.append(proxy) + + with patch.object(parsedmarc._LimitedExpatParser, "__init__", record): + parsedmarc.parse_aggregate_report_xml(self._sample(), offline=OFFLINE_MODE) + sample_nodes = instances[-1]._counts.nodes + self.assertGreater(sample_nodes, 0) + # Three siblings add three elements and three attributes. + widened = self._sample('' * 3) + with patch.object(parsedmarc, "MAX_XML_NODES", sample_nodes + 6): + parsedmarc.parse_aggregate_report_xml(widened, offline=OFFLINE_MODE) + with patch.object(parsedmarc, "MAX_XML_NODES", sample_nodes + 5): + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(widened, offline=OFFLINE_MODE) + self.assertIn("elements and attributes", str(ctx.exception)) + + def testLimitsApplyOnTheRecoveryPath(self): + """A deep body behind a malformed XML declaration is rejected too. + + The malformed declaration makes the first parse fail before expat + reads any element, so the document goes through the lxml recovery + path and the second ``xmltodict.parse()``, which must apply the same + limits (GHSA-q7gm-8cj9-94hr).""" + # Without the deep body, the recovery path parses the report. + report = parsedmarc.parse_aggregate_report_xml( + self._sample(declaration=self.MALFORMED_DECLARATION), + offline=OFFLINE_MODE, + ) + self.assertTrue( + any( + e.startswith("Invalid XML") for e in report["report_metadata"]["errors"] + ) + ) + deep_bodies = { + "closed": self._nested(150), + # libxml2 stops at its own depth limit of 256 and recovers the + # elements read so far, which are still over this one. + "unclosed": "" * (2 * 2**20 // 3), + } + for name, inner in deep_bodies.items(): + with self.subTest(body=name): + xml = self._sample(inner, declaration=self.MALFORMED_DECLARATION) + limit = budget(1) + start = time.perf_counter() + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertLess(time.perf_counter() - start, limit) + self.assertIn("levels deep", str(ctx.exception)) + + def testWideDocumentIsRejectedBeforeTheRecoveryTree(self): + """A wide document behind a malformed XML declaration is rejected by + the counting pass, before lxml builds the recovery tree. + + Regression test for GHSA-q7gm-8cj9-94hr: the malformed declaration + sends the document to the lxml recovery parse, which has no node + limit. Measured locally before the counting pass, 2,600,000 + ```` siblings (22 MiB) behind this declaration peaked at + +1532 MiB of RSS, 890 MiB of it the lxml tree. The counting pass + builds no tree. ``testLimitsApplyOnTheRecoveryPath`` shows the + declaration alone still lets the report parse.""" + xml = ( + self.MALFORMED_DECLARATION + + "" + + "" * constants.MAX_XML_NODES + + "" + ) + limit = budget(10) + tracemalloc.start() + try: + start = time.perf_counter() + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + elapsed = time.perf_counter() - start + peak = tracemalloc.get_traced_memory()[1] + finally: + tracemalloc.stop() + self.assertLess(elapsed, limit) + self.assertLess(peak, 100 * 2**20) + self.assertIn( + f"more than {constants.MAX_XML_NODES:,} elements and attributes", + str(ctx.exception), + ) + + def _assert_rejected_on_tag_length(self, xml: str) -> None: + limit = budget(1.0) + tracemalloc.start() + try: + start = time.perf_counter() + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + elapsed = time.perf_counter() - start + peak = tracemalloc.get_traced_memory()[1] + finally: + tracemalloc.stop() + self.assertEqual( + str(ctx.exception), + f"Invalid XML: a tag is longer than {constants.MAX_XML_TAG_LENGTH:,} " + "characters", + ) + self.assertLess(elapsed, limit) + self.assertLess(peak, 200 * 2**20) + + def test_element_with_many_attributes_is_rejected_before_parsing(self): + """expat builds a start tag's whole attribute list before the + counting handler sees it: one element with 900,000 attributes + (9.3 MiB), within the node limit and the ``=`` count, took 1.1 s and + +200 MiB. The tag scan now rejects it.""" + attributes = " ".join(f"a{i}=''" for i in range(900_000)) + xml = self._sample(f"") + self._assert_rejected_on_tag_length(xml) + self._assert_rejected_on_tag_length( + self._sample(f"", self.MALFORMED_DECLARATION) + ) + + def test_tag_length_limit_is_exact(self): + """A tag of exactly MAX_XML_TAG_LENGTH characters, from its "<" to + the character before its ">", passes the scan; one more is + rejected, as is an unterminated tag after the last ">".""" + limit = constants.MAX_XML_TAG_LENGTH + head = '{at_limit}>") + over = head + "x" * (limit - len(head) - 1) + '"/' + for xml in (f"{over}>", f"{over}"): + with self.assertRaises(parsedmarc.InvalidAggregateReport, msg=xml[-20:]): + parsedmarc._check_tag_lengths(xml) + report = parsedmarc.parse_aggregate_report_xml( + self._sample(at_limit + ">"), offline=True + ) + self.assertTrue(report["records"]) + + def test_tag_scan_spans_chunks_and_stays_small(self): + """The scan reads the text one chunk at a time: a tag across a chunk + boundary is measured whole, and a document of millions of short + tags is scanned in little memory (one split of 100 MiB of "" + peaked at +1,318 MiB).""" + limit = constants.MAX_XML_TAG_LENGTH + chunk = parsedmarc._TAG_SCAN_CHUNK + filler = "x" * (chunk - 100) + # From "<" to the "/" before ">": exactly the limit, then one more. + at_limit = "" + over = "" + parsedmarc._check_tag_lengths(filler + at_limit) + with self.assertRaises(parsedmarc.InvalidAggregateReport): + parsedmarc._check_tag_lengths(filler + over) + dense = "" + "" * (4 * 2**20) + "" + tracemalloc.start() + try: + parsedmarc._check_tag_lengths(dense) + peak = tracemalloc.get_traced_memory()[1] + finally: + tracemalloc.stop() + self.assertLess(peak, 8 * 2**20) + + def test_attributes_hidden_by_a_raw_gt_are_counted_by_their_equals(self): + """XML allows a raw ">" in a quoted attribute value, so + ``x=">"`` attributes cut a tag into short pieces that pass the tag + scan: 2,000,000 of them (23.7 MiB) took 1.4 s and +310 MiB before + the node limit rejected them.""" + attributes = " ".join(f'x{i}=">"' for i in range(2_000_000)) + xml = f"" + limit = budget(0.2) + tracemalloc.start() + try: + start = time.perf_counter() + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + elapsed = time.perf_counter() - start + peak = tracemalloc.get_traced_memory()[1] + finally: + tracemalloc.stop() + self.assertIn("`=` characters", str(ctx.exception)) + self.assertLess(elapsed, limit) + self.assertLess(peak, 50 * 2**20) + + def test_equals_count_limit_is_exact(self): + limit = constants.MAX_XML_NODES + parsedmarc._check_attribute_count("=" * limit) + with self.assertRaises(parsedmarc.InvalidAggregateReport): + parsedmarc._check_attribute_count("=" * (limit + 1)) + # The '=' characters may sit in text, where they count all the same. + xml = _minimal_aggregate_xml().replace( + "TestOrg", + "TestOrg" + + "=" * (limit - 10) + + "", + ) + report = parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertTrue(report["records"]) + + def test_report_over_32_mib_of_utf8_is_rejected_before_parsing(self): + """An uncompressed report is held to the cap a compressed one is, in + bytes of UTF-8: counting characters let 999,000 elements of 14 emoji + (31.2 Mi characters, 71 MiB of UTF-8) through, at +649 MiB.""" + cap = parsedmarc.MAX_DECOMPRESSED_REPORT_SIZE + frame = len("") + parsedmarc._check_xml_text("" + "x" * (cap - frame) + "") + expected = "Invalid XML: the report is larger than 32 MiB of UTF-8" + for label, xml in ( + ("ASCII", "" + "x" * (cap - frame + 1) + ""), + ("emoji", "" + "\U0001f600" * (cap // 4) + ""), + ): + with self.assertRaises(parsedmarc.InvalidAggregateReport, msg=label) as ctx: + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertEqual(str(ctx.exception), expected, msg=label) + # Under the cap in bytes, non-ASCII text passes the measure. + self.assertFalse(parsedmarc._exceeds_size_cap("\u00e9" * (cap // 2))) + self.assertTrue(parsedmarc._exceeds_size_cap(b"x" * (cap + 1))) + self.assertFalse(parsedmarc._exceeds_size_cap(b"x" * cap)) + # Bytes over the cap are rejected before they are decoded, and are + # measured as bytes. + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(b"" + b"x" * cap, offline=True) + self.assertEqual( + str(ctx.exception), "Invalid XML: the report is larger than 32 MiB" + ) + + def test_smtp_tls_report_over_the_size_cap_is_rejected_before_parsing(self): + """parse_smtp_tls_report_json() decoded and parsed JSON of any size: + 60 MiB of ``[],`` cost +1,505 MiB.""" + cap = parsedmarc.MAX_DECOMPRESSED_REPORT_SIZE + content = b"[" + b"[]," * (cap // 3 + 1) + b"[]]" + limit = budget(0.1) + start = time.perf_counter() + with self.assertRaises(parsedmarc.InvalidSMTPTLSReport) as ctx: + parsedmarc.parse_smtp_tls_report_json(content) + self.assertLess(time.perf_counter() - start, limit) + self.assertEqual(str(ctx.exception), "the report is larger than 32 MiB") + with self.assertRaises(parsedmarc.InvalidSMTPTLSReport) as ctx: + parsedmarc.parse_smtp_tls_report_json(content.decode()) + self.assertEqual( + str(ctx.exception), "the report is larger than 32 MiB of UTF-8" + ) + with open("samples/smtp_tls/rfc8460.json", "rb") as sample_file: + self.assertTrue( + parsedmarc.parse_smtp_tls_report_json(sample_file.read())["policies"] + ) + + def test_a_parse_leaves_no_reference_cycle(self): + """The counting wrappers referred to the parser proxy, closing a + cycle through the parser that kept xmltodict's tree alive until the + cyclic collector ran: 2,000,000 distinct siblings peaked at + +1,020 MiB, against +391 MiB without the cycle.""" + xml = "" + "".join(f"" for i in range(1000)) + "" + gc.collect() + gc.disable() + try: + parsedmarc._parse_limited_xml(xml) + self.assertEqual(gc.collect(), 0) + finally: + gc.enable() + + def test_the_first_parse_is_released_before_recovery(self): + """The recovery parse runs after the first parse's except block: a + handler keeps the handled exception's traceback, and with it the + failed parse's frames and partial tree, alive while it runs (on + Python 3.10 even after ``e.__traceback__ = None``). The first + parse's counter, held by those frames, must be gone when the + recovery starts.""" + counters: list[weakref.ref] = [] + alive_at_recovery: list[bool] = [] + original_init = parsedmarc._XMLCounts.__init__ + original_recovery = parsedmarc._parse_xml_with_recovery + + def record(counts): + original_init(counts) + counters.append(weakref.ref(counts)) + + def check(xml): + gc.collect() + alive_at_recovery.append(counters[0]() is not None) + return original_recovery(xml) + + with ( + patch.object(parsedmarc._XMLCounts, "__init__", record), + patch.object(parsedmarc, "_parse_xml_with_recovery", check), + ): + parsedmarc.parse_aggregate_report_xml( + self._sample("", self.MALFORMED_DECLARATION), offline=True + ) + self.assertEqual(alive_at_recovery, [False]) + + def test_the_recovery_path_leaves_no_tree_behind(self): + """After the lxml recovery path, gc.collect() finds a constant set + of lxml helper objects (18 here), the same for a document ten + times larger, so no tree of the document survives a parse.""" + + def garbage_after_recovery(elements: int) -> int: + xml = ( + self.MALFORMED_DECLARATION + + "" + + "".join(f"t" for i in range(elements)) + + "" + ) + gc.collect() + gc.disable() + try: + with self.assertRaises(parsedmarc.InvalidAggregateReport): + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + return gc.collect() + finally: + gc.enable() + + small = garbage_after_recovery(1000) + self.assertEqual(garbage_after_recovery(10_000), small) + self.assertLess(small, 100) + + def test_attributes_still_count_toward_the_node_limit(self): + """xmltodict keeps no attributes (xml_attribs=False), but expat still + passes them to the counting start handler: one element with 1,000 + attributes is 1,001 nodes.""" + attributes = " ".join(f'a{i}=""' for i in range(1000)) + xml = f"" + with patch("parsedmarc.MAX_XML_NODES", 1001): + self.assertIsNone(parsedmarc._parse_limited_xml(xml)["a"]) + with patch("parsedmarc.MAX_XML_NODES", 1000): + with self.assertRaises(parsedmarc.InvalidAggregateReport): + parsedmarc._parse_limited_xml(xml) + + +class TestAggregateReportSpanLimit(unittest.TestCase): + """The of an aggregate report is bounded before it is + normalized into one record per day.""" + + BEGIN = 1704067200 # 2024-01-01 00:00:00 UTC + + def _xml(self, begin: int, end: int, count: int) -> str: + return ( + _minimal_aggregate_xml() + .replace( + "17040672001704153599", + f"{begin}{end}", + ) + .replace("1", f"{count}") + ) + + def test_span_over_limit_is_rejected(self): + xml = self._xml(self.BEGIN, self.BEGIN + 400 * 86400, 400) + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertEqual( + str(ctx.exception), + f"Report date range spans more than {constants.MAX_REPORT_SPAN_DAYS} days", + ) + + def test_span_of_a_year_parses_to_at_most_one_record_per_day(self): + """Before the limit, 200,000 days with a count of 200,000 produced + 200,000 records in about 0.8 s; a year must stay far below 1 s.""" + xml = self._xml(self.BEGIN, self.BEGIN + 365 * 86400, 365) + limit = budget(1.0) + started = time.monotonic() + report = parsedmarc.parse_aggregate_report_xml(xml, offline=True) + elapsed = time.monotonic() - started + self.assertLessEqual(len(report["records"]), 365) + self.assertGreater(len(report["records"]), 1) + self.assertLess(elapsed, limit) + + def test_span_too_large_for_a_float_is_rejected_with_its_message(self): + """An of 4,001 digits (below Python's 4,300-digit limit for + int()) made the message's float division raise OverflowError, which + reached the caller as "Unexpected error: integer division result too + large for a float".""" + xml = self._xml(self.BEGIN, 10**4000, 1) + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + # No sender-derived number reaches the message or the log. + self.assertEqual( + str(ctx.exception), + f"Report date range spans more than {constants.MAX_REPORT_SPAN_DAYS} days", + ) + self.assertLess(len(str(ctx.exception)), 200) + + def test_many_records_over_many_days_are_kept_whole(self): + """10,000 records over 366 days, each with a count of 1,000,000, + normalized into 3,670,000 records in 13.5 s and 1.7 GB before the + limit; the report now keeps its 10,000 records whole.""" + record_count = 10_000 + xml = self._xml(self.BEGIN, self.BEGIN + 366 * 86400, 1_000_000) + start = xml.index("") + end = xml.index("") + len("") + xml = xml[:start] + xml[start:end] * record_count + xml[end:] + limit = budget(3.0) + started = time.monotonic() + with self.assertLogs("parsedmarc.log", level="WARNING") as logs: + report = cast( + dict, parsedmarc.parse_aggregate_report_xml(xml, offline=True) + ) + elapsed = time.monotonic() - started + self.assertLess(elapsed, limit) + self.assertEqual(len(report["records"]), record_count) + self.assertFalse( + any(record["normalized_timespan"] for record in report["records"]) + ) + self.assertFalse(report["report_metadata"]["timespan_requires_normalization"]) + # Each record covers the report's own range, in UTC. + self.assertEqual(report["records"][0]["interval_begin"], "2024-01-01 00:00:00") + self.assertEqual(report["records"][0]["interval_end"], "2025-01-01 00:00:00") + warnings = [line for line in logs.output if "not normalized" in line] + self.assertEqual(len(warnings), 1) + self.assertIn(str(constants.MAX_NORMALIZED_RECORDS), warnings[0]) + self.assertIn(f"{record_count} records over 366 days", warnings[0]) + + def test_keep_alive_is_called_every_20_records(self): + xml = self._xml(self.BEGIN, self.BEGIN + 86399, 1) + start = xml.index("") + end = xml.index("") + len("") + xml = xml[:start] + xml[start:end] * 45 + xml[end:] + keep_alive = MagicMock() + report = parsedmarc.parse_aggregate_report_xml( + xml, offline=True, keep_alive=keep_alive + ) + self.assertEqual(len(report["records"]), 45) + self.assertEqual(keep_alive.call_count, 2) + + def _records_times_days(self, record_count, days, begin_offset=0): + begin = self.BEGIN + begin_offset + xml = self._xml(begin, begin + days * 86400, days) + start = xml.index("") + end = xml.index("") + len("") + xml = xml[:start] + xml[start:end] * record_count + xml[end:] + return cast(dict, parsedmarc.parse_aggregate_report_xml(xml, offline=True)) + + def test_normalization_at_exactly_the_limit_is_split(self): + """1,000 records over 250 days make exactly MAX_NORMALIZED_RECORDS.""" + self.assertEqual(1000 * 250, constants.MAX_NORMALIZED_RECORDS) + report = self._records_times_days(1000, 250) + self.assertTrue(report["report_metadata"]["timespan_requires_normalization"]) + self.assertEqual(len(report["records"]), 250_000) + + def test_normalization_just_under_and_over_the_limit(self): + """683 records over 366 days make 249,978 records; 684 make + 250,344, which keeps the records whole.""" + under = self._records_times_days(683, 366) + self.assertTrue(under["report_metadata"]["timespan_requires_normalization"]) + self.assertEqual(len(under["records"]), 683 * 366) + with self.assertLogs("parsedmarc.log", level="WARNING"): + over = self._records_times_days(684, 366) + self.assertFalse(over["report_metadata"]["timespan_requires_normalization"]) + self.assertEqual(len(over["records"]), 684) + + def test_a_range_starting_after_midnight_counts_the_extra_day(self): + """A range of 366 days starting 1 s after midnight touches 367 + calendar days, so 682 records would make 250,294.""" + with self.assertLogs("parsedmarc.log", level="WARNING") as logs: + report = self._records_times_days(682, 366, begin_offset=1) + self.assertFalse(report["report_metadata"]["timespan_requires_normalization"]) + self.assertEqual(len(report["records"]), 682) + self.assertTrue( + any("682 records over 367 days" in line for line in logs.output) + ) + + def test_small_multi_day_report_is_still_normalized(self): + xml = self._xml(self.BEGIN, self.BEGIN + 3 * 86400, 30) + start = xml.index("") + end = xml.index("") + len("") + xml = xml[:start] + xml[start:end] * 2 + xml[end:] + report = cast(dict, parsedmarc.parse_aggregate_report_xml(xml, offline=True)) + self.assertTrue(report["report_metadata"]["timespan_requires_normalization"]) + self.assertEqual(len(report["records"]), 6) + self.assertTrue( + all(record["normalized_timespan"] for record in report["records"]) + ) + self.assertEqual(sum(record["count"] for record in report["records"]), 60) + + def test_negative_span_is_rejected(self): + xml = self._xml(self.BEGIN, self.BEGIN - 86400, 1) + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertEqual(str(ctx.exception), "Report date range is negative") + + +def _smtp_tls_report_dict() -> dict: + """A minimal valid RFC 8460 §4.4 report, as a dict to mutate per test.""" + return { + "organization-name": "Reporter Example", + "date-range": { + "start-datetime": "2024-01-01T00:00:00Z", + "end-datetime": "2024-01-01T23:59:59Z", + }, + "contact-info": "tls@reporter.example", + "report-id": "report-1", + "policies": [ + { + "policy": { + "policy-type": "sts", + "policy-string": ["version: STSv1", "mode: enforce"], + "policy-domain": "example.com", + }, + "summary": { + "total-successful-session-count": 10, + "total-failure-session-count": 1, + }, + "failure-details": [ + { + "result-type": "certificate-expired", + "sending-mta-ip": "192.0.2.1", + "receiving-mx-hostname": "mx.example.com", + "failed-session-count": 1, + } + ], + } + ], + } + + +def _smtp_tls_field_container(report: dict, field: str) -> dict: + """Return the object of ``report`` that holds ``field``.""" + policy = report["policies"][0] + if field in ("policy-domain", "policy-type", "policy-string", "mx-host"): + return policy["policy"] + if field.startswith("total-"): + return policy["summary"] + if field in ("organization-name", "report-id", "contact-info"): + return report + return policy["failure-details"][0] + + +class TestSmtpTlsReportTypeValidation(unittest.TestCase): + """GHSA-4272-4r8g-6c5h: parse_smtp_tls_report_json() copied every field + without a type check, so a sender could make an output raise later: + ``"policy-string": [1]`` gave a TypeError in the CSV writer, and + ``"start-datetime": 5`` an AttributeError in the Elasticsearch, + OpenSearch and Splunk saves and a TypeError in the PostgreSQL save. The + exception escaped the CLI and stopped the mailbox run on every attempt. + Each field now has the type RFC 8460 §4.4 gives it, or the report is + rejected as InvalidSMTPTLSReport.""" + + def _assert_rejected(self, report: dict, label: str) -> None: + with self.subTest(label): + with self.assertRaises(parsedmarc.InvalidSMTPTLSReport): + parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + + def testValidReportStillParses(self): + parsed = parsedmarc.parse_smtp_tls_report_json( + json.dumps(_smtp_tls_report_dict()) + ) + self.assertEqual(parsed["begin_date"], "2024-01-01T00:00:00Z") + policy = parsed["policies"][0] + self.assertEqual( + policy.get("policy_strings"), ["version: STSv1", "mode: enforce"] + ) + self.assertEqual( + policy.get("failure_details", [{}])[0].get("failed_session_count"), 1 + ) + + def testPolicyStringOfIntegersIsRejectedBeforeTheCsvWriter(self): + """The advisory's file-output case: before the fix the report + parsed and parsed_smtp_tls_reports_to_csv() raised TypeError.""" + report = _smtp_tls_report_dict() + report["policies"][0]["policy"]["policy-string"] = [1] + with self.assertRaises(parsedmarc.InvalidSMTPTLSReport) as ctx: + parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + self.assertIn("policy-string", str(ctx.exception)) + + def testPolicyStringMustBeAListOfStrings(self): + for value in ("version: STSv1", {"version": "STSv1"}, [["nested"]]): + report = _smtp_tls_report_dict() + report["policies"][0]["policy"]["policy-string"] = value + self._assert_rejected(report, f"policy-string={value!r}") + + def testNullPolicyStringIsTreatedAsAbsent(self): + report = _smtp_tls_report_dict() + report["policies"][0]["policy"]["policy-string"] = None + parsed = parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + self.assertNotIn("policy_strings", parsed["policies"][0]) + + def testMxHostPatternListOfIntegersIsRejected(self): + report = _smtp_tls_report_dict() + report["policies"][0]["policy"]["mx-host-pattern"] = [1] + self._assert_rejected(report, "mx-host-pattern=[1]") + + def testStringFieldsMustBeStrings(self): + for field in ( + "organization-name", + "report-id", + "policy-domain", + "policy-type", + "result-type", + "receiving-mx-hostname", + "additional-information", + ): + for value in (5, ["sts"], {"#text": "sts"}): + report = _smtp_tls_report_dict() + _smtp_tls_field_container(report, field)[field] = value + self._assert_rejected(report, f"{field}={value!r}") + + def testContactInfoIsAStringOrAListOfStrings(self): + for value in ("tls@reporter.example", ["a@reporter.example", "b@x.example"]): + report = _smtp_tls_report_dict() + report["contact-info"] = value + parsed = parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + self.assertEqual(parsed["contact_info"], value) + for value in (5, [1], {"email": "a@reporter.example"}): + report = _smtp_tls_report_dict() + report["contact-info"] = value + self._assert_rejected(report, f"contact-info={value!r}") + + def testDatesMustBeParseableStrings(self): + for field in ("start-datetime", "end-datetime"): + for value in (5, None, ["2024-01-01T00:00:00Z"], "not a date", ""): + report = _smtp_tls_report_dict() + report["date-range"][field] = value + self._assert_rejected(report, f"{field}={value!r}") + + def testStartDatetimeIntegerIsRejected(self): + """The advisory's Elasticsearch/OpenSearch/Splunk/PostgreSQL case.""" + report = _smtp_tls_report_dict() + report["date-range"]["start-datetime"] = 5 + with self.assertRaises(parsedmarc.InvalidSMTPTLSReport) as ctx: + parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + self.assertIn("start-datetime", str(ctx.exception)) + + def testUnparseableDateIsSanitizedInTheErrorMessage(self): + report = _smtp_tls_report_dict() + report["date-range"]["start-datetime"] = "bad\r\nforged log line" + with self.assertRaises(parsedmarc.InvalidSMTPTLSReport) as ctx: + parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + self.assertNotIn("\r\n", str(ctx.exception)) + self.assertIn("bad\\r\\nforged log line", str(ctx.exception)) + + def testOverlongDateIsRejectedQuickly(self): + """dateutil took 12.4 s (Python 3.14) to fail on a 1,000,000-digit + date string; a date over 64 characters is now rejected unparsed.""" + report = _smtp_tls_report_dict() + report["date-range"]["start-datetime"] = "1" * 1_000_000 + limit = budget(2.0) + start = time.perf_counter() + with self.assertRaises(parsedmarc.InvalidSMTPTLSReport) as ctx: + parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + self.assertLess(time.perf_counter() - start, limit) + self.assertIn("longer than", str(ctx.exception)) + + def testOtherDateFormatsStillParse(self): + for value in ("2024-01-01", "2024-01-01T00:00:00.123456789+00:00"): + report = _smtp_tls_report_dict() + report["date-range"]["start-datetime"] = value + parsed = parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + self.assertEqual(parsed["begin_date"], value) + + def testCountersAreIntegersOrNull(self): + for field in ( + "total-successful-session-count", + "total-failure-session-count", + "failed-session-count", + ): + report = _smtp_tls_report_dict() + _smtp_tls_field_container(report, field)[field] = None + parsed = parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + self.assertEqual(parsed["report_id"], "report-1") + for value in ("5", 1.5, True, [1], {"count": 1}): + report = _smtp_tls_report_dict() + _smtp_tls_field_container(report, field)[field] = value + self._assert_rejected(report, f"{field}={value!r}") + + def testFailureDetailsMustBeAListOfObjects(self): + for value in ("certificate-expired", {"result-type": "x"}, [1], ["x"]): + report = _smtp_tls_report_dict() + report["policies"][0]["failure-details"] = value + self._assert_rejected(report, f"failure-details={value!r}") + + def test_lone_surrogate_is_rejected(self): + """A JSON "\\ud800" passed every type check and later made + print() in process_reports() raise UnicodeEncodeError, which exited + the CLI with status 1.""" + cases = ( + ("organization-name", "Comp\ud800any"), + ("policy-string", ["version: STSv1", "mode: \ud800"]), + ("contact-info", ["tls@example.com", "\udfff"]), + ("contact-info", "tls\ud800@example.com"), + ("mx-host", "\ud800.example.com"), + ("mx-host", ["mx.example.com", "\ud800.example.com"]), + ) + for field, value in cases: + report = _smtp_tls_report_dict() + _smtp_tls_field_container(report, field)[field] = value + with self.assertRaises(parsedmarc.InvalidSMTPTLSReport, msg=field) as ctx: + parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + self.assertIn(f"{field} is not valid UTF-8 text", str(ctx.exception)) + + +class TestPolicyOverrideReasonTypeNormalization(unittest.TestCase): + """GHSA-4272-4r8g-6c5h: _parse_report_record() normalized only a + reason's ``comment``, so a ```` without ```` gave + parsed_aggregate_reports_to_csv_rows() a KeyError, and a ```` + that xmltodict parsed to a dict or a list a TypeError in its join. The + exception escaped the CLI's file output and stopped the mailbox run. + A reason's ``type`` and ``comment`` are now each a string or None.""" + + @staticmethod + def _report_xml(reason: str) -> str: + return f""" + + reporter.example + dmarc@reporter.examplereason-test + 17040672001704153599 + + example.comrr +

none

none100 + 192.0.2.11 + nonefail + fail{reason} + example.com + example.comfail + + +""" + + def _csv_row(self, reason: str) -> tuple[dict, dict]: + """Parse a report holding ``reason`` and return its parsed + override reason and its one CSV row.""" + parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear() + report = parsedmarc.parse_aggregate_report_xml( + self._report_xml(reason), offline=True + ) + csv_text = parsedmarc.parsed_aggregate_reports_to_csv(report) + rows = list(csv.DictReader(StringIO(csv_text))) + self.assertEqual(len(rows), 1) + record = report["records"][0] + override = dict(record["policy_evaluated"]["policy_override_reasons"][0]) + return override, rows[0] + + def testReasonWithoutTypeWritesNone(self): + override, row = self._csv_row("x") + self.assertIsNone(override["type"]) + self.assertEqual(row["policy_override_reasons"], "none") + self.assertEqual(row["policy_override_comments"], "x") + + def testTypeWithChildElementsWritesNone(self): + """An element with child elements has no text of its own.""" + override, row = self._csv_row( + "local_policyx" + ) + self.assertIsNone(override["type"]) + self.assertEqual(row["policy_override_reasons"], "none") + + def testTypeWithTextBesideChildElementsWritesTheText(self): + override, row = self._csv_row( + "forwardedx" + ) + self.assertEqual(override["type"], "forwarded") + self.assertEqual(row["policy_override_reasons"], "forwarded") + + def testTypeWithLangAttributeWritesItsText(self): + override, row = self._csv_row( + 'local_policyx' + ) + self.assertEqual(override["type"], "local_policy") + self.assertEqual(row["policy_override_reasons"], "local_policy") + + def testRepeatedTypeAndCommentAreNull(self): + """A repeated element follows the rule of every other string field + (_scalar()): it is None, written as "none" in CSV.""" + override, row = self._csv_row( + "local_policyother" + "ab" + ) + self.assertIsNone(override["type"]) + self.assertIsNone(override["comment"]) + self.assertEqual(row["policy_override_reasons"], "none") + self.assertEqual(row["policy_override_comments"], "none") + + def testPlainTypeIsUnchanged(self): + override, row = self._csv_row( + "mailing_listlist" + ) + self.assertEqual(override["type"], "mailing_list") + self.assertEqual(override["comment"], "list") + self.assertEqual(row["policy_override_reasons"], "mailing_list") + + +class TestSmtpTlsMxHostKey(unittest.TestCase): + """Issue #942: RFC 8460 §4.4 keys a policy's MX host patterns + ``"mx-host"`` (its schema line reads ``"mx-host": mx-host-pattern``, + and §4.4 describes the value as "a JSON array of strings"), and the + RFC's Appendix B example gives it as a plain string. The parser read + only ``"mx-host-pattern"``, the value type's name, so every + RFC-conformant report lost the field. ``mx-host`` is now read as a list + or a string, ``mx-host-pattern`` stays a fallback, and the output key + is still ``mx_host_patterns``.""" + + def _parse_policy(self, **policy_keys) -> dict: + report = _smtp_tls_report_dict() + report["policies"][0]["policy"].update(policy_keys) + parsed = parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + return dict(parsed["policies"][0]) + + def testMxHostList(self): + policy = self._parse_policy( + **{"mx-host": ["*.mail.example.com", "mx.example.com"]} + ) + self.assertEqual( + policy["mx_host_patterns"], ["*.mail.example.com", "mx.example.com"] + ) + + def testMxHostStringBecomesAOneElementList(self): + policy = self._parse_policy(**{"mx-host": "*.mail.company-y.example"}) + self.assertEqual(policy["mx_host_patterns"], ["*.mail.company-y.example"]) + + def testMxHostPatternIsAFallback(self): + policy = self._parse_policy(**{"mx-host-pattern": ["*.legacy.example"]}) + self.assertEqual(policy["mx_host_patterns"], ["*.legacy.example"]) + policy = self._parse_policy(**{"mx-host-pattern": "*.legacy.example"}) + self.assertEqual(policy["mx_host_patterns"], ["*.legacy.example"]) + + def testMxHostWinsOverMxHostPattern(self): + policy = self._parse_policy( + **{"mx-host": ["*.rfc.example"], "mx-host-pattern": ["*.legacy.example"]} + ) + self.assertEqual(policy["mx_host_patterns"], ["*.rfc.example"]) + + def testEmptyMxHostFallsBackThenOmits(self): + for value in (None, "", []): + with self.subTest(value=value): + policy = self._parse_policy( + **{"mx-host": value, "mx-host-pattern": ["*.legacy.example"]} + ) + self.assertEqual(policy["mx_host_patterns"], ["*.legacy.example"]) + policy = self._parse_policy(**{"mx-host": value}) + self.assertNotIn("mx_host_patterns", policy) + + def testWrongTypedMxHostIsRejected(self): + for key in ("mx-host", "mx-host-pattern"): + for value in (5, [1], {"pattern": "*.example.com"}): + report = _smtp_tls_report_dict() + report["policies"][0]["policy"][key] = value + with self.subTest(key=key, value=value): + with self.assertRaises(parsedmarc.InvalidSMTPTLSReport): + parsedmarc.parse_smtp_tls_report_json(json.dumps(report)) + + def testRfc8460AppendixBSampleKeepsItsMxHost(self): + """samples/smtp_tls/rfc8460.json is the RFC's Appendix B example.""" + parsed = parsedmarc.parse_report_file( + "samples/smtp_tls/rfc8460.json", offline=True + ) + self.assertEqual(parsed["report_type"], "smtp_tls") + report = cast(SMTPTLSReport, parsed["report"]) + self.assertEqual( + report["policies"][0].get("mx_host_patterns"), + ["*.mail.company-y.example"], + ) + csv_text = parsedmarc.parsed_smtp_tls_reports_to_csv(report) + rows = list(csv.DictReader(StringIO(csv_text))) + self.assertTrue(rows) + self.assertEqual( + {row["mx_host_patterns"] for row in rows}, {"*.mail.company-y.example"} + ) + + +class TestAggregateReportXMLAttributesDropped(unittest.TestCase): + """An XML attribute on a scalar element made xmltodict return an object + (``{"@a": "1", "#text": "none"}``) that reached Elasticsearch and + OpenSearch and mapped the field as an object (GHSA-qpcg-29vr-jwm3).""" + + ATTRIBUTE = ' a="1"' + + @classmethod + def poisoned_xml(cls, attribute=ATTRIBUTE): + return ( + _minimal_aggregate_xml( + policy_published=( + "example.com" + f"none

1" + ), + reason='otherx', + ) + .replace("", f"1.0", 1) + .replace("", f"", 1) + .replace( + "example.com", + "example.com" + f"example.org", + 1, + ) + ) + + def test_attributes_on_scalar_elements_are_dropped(self): + report = cast( + dict, + parsedmarc.parse_aggregate_report_xml(self.poisoned_xml(), offline=True), + ) + record = report["records"][0] + self.assertEqual(report["xml_schema"], "1.0") + self.assertEqual(report["policy_published"]["p"], "none") + self.assertEqual(report["policy_published"]["fo"], "1") + self.assertEqual(record["policy_evaluated"]["disposition"], "none") + self.assertEqual(record["identifiers"]["envelope_to"], "example.org") + self.assertEqual( + record["policy_evaluated"]["policy_override_reasons"], + [{"type": "other", "comment": "x"}], + ) + + def test_element_with_many_attributes_parses_to_its_text(self): + attributes = "".join(f' a{i}="v"' for i in range(500)) + report = cast( + dict, + parsedmarc.parse_aggregate_report_xml( + self.poisoned_xml(attributes), offline=True + ), + ) + self.assertEqual(report["policy_published"]["p"], "none") + self.assertEqual(report["xml_schema"], "1.0") + + def test_rfc_9990_is_still_detected_through_a_field_with_an_attribute(self): + """With no namespace, an RFC 9990-only field (``np``) still marks the + report as RFC 9990 after its attribute is dropped, so a DKIM result + without a selector gets the RFC 9990 warning.""" + xml = _minimal_aggregate_xml( + policy_published=( + "example.com

none

" + f"reject" + ) + ).replace( + "", + "example.com" + "pass", + 1, + ) + with self.assertLogs("parsedmarc.log", level="WARNING") as logs: + report = parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertIsNone(report["xml_namespace"]) + self.assertEqual(report["policy_published"]["np"], "reject") + self.assertTrue(any("REQUIRED by RFC 9990" in line for line in logs.output)) + + +# Every element of an aggregate report that the schema types as a string, +# with its plain value; {name} placeholders take a variant. +_SCALAR_FIELD_VALUES = { + "version": "1.0", + "org_name": "reporter.example", + "email": "dmarc@reporter.example", + "extra_contact_info": "https://reporter.example/dmarc", + "report_id": "scalar-test", + "generator": "reporter/1.0", + "domain": "example.com", + "adkim": "r", + "aspf": "s", + "p": "none", + "sp": "quarantine", + "pct": "100", + "fo": "1", + "np": "reject", + "testing": "n", + "discovery_method": "psl", + "source_ip": "192.0.2.1", + "count": "3", + "disposition": "none", + "dkim": "pass", + "spf": "fail", + "header_from": "example.com", + "envelope_from": "bounce.example.com", + "envelope_to": "example.org", + "dkim_domain": "example.com", + "selector": "s1", + "dkim_result": "pass", + "dkim_human_result": "good signature", + "spf_domain": "bounce.example.com", + "scope": "mfrom", + "spf_result": "fail", + "spf_human_result": "not listed", +} +_SCALAR_FIELD_TEMPLATE = ( + "{version}" + "{org_name}{email}" + "{extra_contact_info}" + "{report_id}{generator}" + "17040672001704153599" + "{domain}" + "{adkim}{aspf}

{p}

{sp}" + "{pct}{fo}{np}{testing}" + "{discovery_method}" + "{source_ip}{count}" + "{disposition}" + "{dkim}{spf}" + "otherc" + "{header_from}" + "{envelope_from}" + "{envelope_to}" + "{dkim_domain}{selector}" + "{dkim_result}{dkim_human_result}" + "{spf_domain}{scope}" + "{spf_result}{spf_human_result}" + "" +) + + +def _scalar_field_xml(**variants: str) -> str: + """The report with each named field's value replaced by its variant.""" + return _SCALAR_FIELD_TEMPLATE.format(**{**_SCALAR_FIELD_VALUES, **variants}) + + +_POLICY_FIELDS = ( + "domain", + "adkim", + "aspf", + "p", + "sp", + "pct", + "fo", + "np", + "testing", + "discovery_method", +) + + +def _record(report: dict) -> dict: + return report["records"][0] + + +def _auth_result(report: dict, kind: str) -> dict: + return (_record(report)["auth_results"][kind] or [{}])[0] + + +class TestAggregateReportScalarFields(unittest.TestCase): + """GHSA-qpcg-29vr-jwm3: a child element under a field the schema types + as a string (``

1

``, ``

none

``) reached the + parsed report, and the Elasticsearch and OpenSearch documents, as a dict + that mapped the field as an object.""" + + GETTERS: dict[str, Callable[[dict], Any]] = { + "version": lambda r: r["xml_schema"], + "org_name": lambda r: r["report_metadata"]["org_name"], + "email": lambda r: r["report_metadata"]["org_email"], + "extra_contact_info": lambda r: r["report_metadata"]["org_extra_contact_info"], + "report_id": lambda r: r["report_metadata"]["report_id"], + "generator": lambda r: r["report_metadata"]["generator"], + **{ + name: (lambda name: lambda r: r["policy_published"][name])(name) + for name in _POLICY_FIELDS + }, + "source_ip": lambda r: _record(r)["source"]["ip_address"], + "count": lambda r: _record(r)["count"], + "disposition": lambda r: _record(r)["policy_evaluated"]["disposition"], + "dkim": lambda r: _record(r)["policy_evaluated"]["dkim"], + "spf": lambda r: _record(r)["policy_evaluated"]["spf"], + "header_from": lambda r: _record(r)["identifiers"]["header_from"], + "envelope_from": lambda r: _record(r)["identifiers"]["envelope_from"], + "envelope_to": lambda r: _record(r)["identifiers"]["envelope_to"], + "dkim_domain": lambda r: _auth_result(r, "dkim").get("domain"), + "selector": lambda r: _auth_result(r, "dkim").get("selector"), + "dkim_result": lambda r: _auth_result(r, "dkim").get("result"), + "dkim_human_result": lambda r: _auth_result(r, "dkim").get("human_result"), + "spf_domain": lambda r: _auth_result(r, "spf").get("domain"), + "scope": lambda r: _auth_result(r, "spf").get("scope"), + "spf_result": lambda r: _auth_result(r, "spf").get("result"), + "spf_human_result": lambda r: _auth_result(r, "spf").get("human_result"), + } + + def _parse(self, **variants: str) -> dict: + return cast( + dict, + parsedmarc.parse_aggregate_report_xml( + _scalar_field_xml(**variants), offline=True + ), + ) + + def test_every_scalar_field_has_a_getter(self): + self.assertEqual(set(self.GETTERS), set(_SCALAR_FIELD_VALUES)) + + def test_mixed_content_keeps_the_text(self): + plain = self._parse() + for name, getter in self.GETTERS.items(): + value = _SCALAR_FIELD_VALUES[name] + report = self._parse(**{name: f"{value}"}) + self.assertEqual(getter(report), getter(plain), msg=name) + + def test_child_element_gives_a_string_none_or_a_rejection(self): + for name, getter in self.GETTERS.items(): + try: + report = self._parse(**{name: "1"}) + except parsedmarc.InvalidAggregateReport: + continue + value = getter(report) + self.assertNotIsInstance(value, (dict, list), msg=name) + self.assertTrue(value is None or isinstance(value, str), msg=name) + + def test_repeated_policy_published_is_coerced_too(self): + """A second makes the field a list; the parser + reads the first, whose

holds a child element.""" + report = self._parse( + p="1", + discovery_method=( + "psl" + "example.com

none

psl" + ), + ) + self.assertIsNone(report["policy_published"]["p"]) + self.assertEqual(report["policy_published"]["discovery_method"], "psl") + + def test_error_with_a_child_element_keeps_its_text(self): + """ may repeat, so it goes through _text(), which gives the + text beside a child element or drops an error with none.""" + xml = _scalar_field_xml().replace( + "", + "disk full1", + 1, + ) + report = parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertEqual(report["report_metadata"]["errors"], ["disk full"]) + + def test_repeated_scalar_element_gives_none(self): + report = self._parse(fo="10") + self.assertIsNone(report["policy_published"]["fo"]) + + def test_count_with_child_elements_takes_the_non_numeric_count_path(self): + with self.assertRaises(parsedmarc.InvalidAggregateReport) as ctx: + self._parse(count="1") + self.assertIn("invalid literal for int()", str(ctx.exception)) + + +class TestReportEmailSerializesEachPartOnce(unittest.TestCase): + """parse_report_email() serialized every part walk() visited, and str() + of a composite part renders its whole subtree, so a deep nesting within + mailsuite's limits re-rendered its leaf once per level + (GHSA-qcc2-4x3v-v4g7).""" + + HEAD = ( + "From: a@example.com\nTo: b@example.com\n" + "Subject: Report Domain: example.com\n" + "Date: Mon, 9 Nov 2026 00:00:00 +0000\nMIME-Version: 1.0\n" + ) + + def _assert_quick(self, message: str) -> None: + limit = budget(2.0) + start = time.monotonic() + with self.assertRaises(parsedmarc.InvalidDMARCReport): + parsedmarc.parse_report_email(message, offline=True) + self.assertLess(time.monotonic() - start, limit) + + def test_nested_multiparts_around_a_large_leaf(self): + """98 nested multiparts around a 20 MiB one-line leaf took 26.6 s.""" + message = self.HEAD + 'Content-Type: multipart/mixed; boundary="b0"\n\n' + for i in range(1, 98): + message += f'--b{i - 1}\nContent-Type: multipart/mixed; boundary="b{i}"\n\n' + message += "--b97\nContent-Type: text/plain\n\n" + "x" * 20 * 2**20 + self._assert_quick(message + "\n--b97--\n") + + def test_chain_of_rfc822_messages_around_a_large_leaf(self): + """A chain of 99 message/rfc822 parts around a 10 MiB leaf took + 11.3 s.""" + message = ( + self.HEAD + + "Content-Type: message/rfc822\n\n" + + "Subject: s\nContent-Type: message/rfc822\n\n" * 97 + + "Subject: s\nContent-Type: text/plain\n\n" + + "x" * 10 * 2**20 + + "\n" + ) + self._assert_quick(message) + + def test_chain_of_feedback_report_parts_around_a_large_leaf(self): + """A chain of 99 message/feedback-report parts around a 10 MiB leaf + rendered every level and took 11.6 s.""" + message = ( + self.HEAD + + "Content-Type: message/feedback-report\n\n" + + "Subject: s\nContent-Type: message/feedback-report\n\n" * 97 + + "Subject: s\nContent-Type: text/plain\n\n" + + "x" * 10 * 2**20 + + "\n" + ) + self._assert_quick(message) + + def test_a_message_inside_the_sample_does_not_replace_it(self): + """A feedback report has one sample (RFC 6591 §3.1); a message/rfc822 + attached inside the sample replaced it before.""" + message = ( + self.HEAD + + 'Content-Type: multipart/report; report-type=feedback-report; boundary="r"\n\n' + "--r\nContent-Type: message/feedback-report\n\n" + "Feedback-Type: auth-failure\nUser-Agent: test/1.0\nVersion: 1\n" + "Original-Mail-From: user@example.com\n" + "Arrival-Date: Mon, 01 Jan 2024 00:00:00 +0000\n" + "Source-IP: 192.0.2.1\nReported-Domain: example.com\n" + "Authentication-Results: mx; dmarc=fail\n\n" + "--r\nContent-Type: message/rfc822\n\n" + "From: spoofer@example.com\nTo: victim@example.org\nSubject: outer\n" + 'MIME-Version: 1.0\nContent-Type: multipart/mixed; boundary="s"\n\n' + "--s\nContent-Type: text/plain\n\nhello\n" + "--s\nContent-Type: message/rfc822\n\n" + "From: other@example.net\nSubject: inner\n\nforwarded\n" + "--s--\n--r--\n" + ) + parsed = parsedmarc.parse_report_email(message, offline=True) + self.assertEqual(parsed["report_type"], "failure") + report = cast(dict, parsed["report"]) + self.assertEqual(report["parsed_sample"]["subject"], "outer") + + +class TestFailureReportPartPrecedence(unittest.TestCase): + """An RFC 6591 message/feedback-report part and its sample win over a + legacy "A message claiming to be from you has failed" text part, in + either order, and a legacy text part inside the sample (the spoofer's + own text) cannot replace the report's Source-IP or sample + (GHSA-qcc2-4x3v-v4g7).""" + + HEAD = ( + "From: a@example.net\nTo: b@example.com\nSubject: fail\n" + "Date: Mon, 9 Nov 2026 00:00:00 +0000\nMIME-Version: 1.0\n" + 'Content-Type: multipart/report; report-type=feedback-report; boundary="r"\n\n' + ) + LEGACY = ( + "--r\nContent-Type: text/plain\n\n" + "A message claiming to be from you has failed the published DMARC " + "policy for your domain.\n\n Sender Domain: example.com\n" + " Sender IP Address: 192.0.2.9\n" + " Received Date: Mon, 9 Nov 2026 00:00:00 +0000\n" + " SPF Alignment: no\n DKIM Alignment: no\n" + " DMARC Results: None, Accept\n\ndetected.\n\n" + "From: x@example.com\nSubject: legacy\n\n" + ) + FEEDBACK = ( + "--r\nContent-Type: message/feedback-report\n\n" + "Feedback-Type: auth-failure\nVersion: 1\nAuth-Failure: dmarc\n" + "Source-IP: 192.0.2.1\nReported-Domain: example.com\n" + "Arrival-Date: Mon, 9 Nov 2026 00:00:00 +0000\n\n" + ) + SAMPLE = ( + "--r\nContent-Type: message/rfc822\n\nFrom: x@example.com\n" + "To: y@example.org\nSubject: arfsample\n" + "Date: Mon, 9 Nov 2026 00:00:00 +0000\n\nbody\n\n" + ) + + def _parse(self, body: str) -> dict: + parsed = parsedmarc.parse_report_email( + self.HEAD + body + "--r--\n", offline=True + ) + self.assertEqual(parsed["report_type"], "failure") + return cast(dict, parsed["report"]) + + def _assert_arf(self, report: dict) -> None: + self.assertEqual(report["source"]["ip_address"], "192.0.2.1") + self.assertEqual(report["parsed_sample"]["subject"], "arfsample") + + def test_arf_after_a_legacy_text_part_wins(self): + self._assert_arf(self._parse(self.LEGACY + self.FEEDBACK + self.SAMPLE)) + + def test_arf_before_a_legacy_text_part_wins(self): + self._assert_arf(self._parse(self.FEEDBACK + self.SAMPLE + self.LEGACY)) + + def test_attachments_inside_the_sample_are_not_reports(self): + """The sample is the reported message, the spoofer's attachments + included: an SVG or zipped HTML attachment made the email rejected, + a gzipped aggregate report or SMTP TLS JSON inside it was returned + as that report instead of the failure report.""" + with open( + "samples/aggregate/example.net!example.com!1529366400!1529452799.xml", + "rb", + ) as sample_file: + aggregate = sample_file.read() + with open("samples/smtp_tls/smtp_tls.json", "rb") as sample_file: + smtp_tls = sample_file.read() + zipped = BytesIO() + with zipfile.ZipFile(zipped, "w") as archive: + archive.writestr("invoice.html", "pay") + cases = ( + ("image/svg+xml", b''), + ("application/zip", zipped.getvalue()), + ("application/json", b'{"a": 1}'), + ("application/gzip", gzip.compress(aggregate)), + ("application/tlsrpt+json", smtp_tls), + ("application/pdf", b"%PDF-1.4 x"), + ) + for content_type, data in cases: + attachment = base64.encodebytes(data).decode() + sample = ( + "--r\nContent-Type: message/rfc822\n\nFrom: x@example.com\n" + "To: y@example.org\nSubject: arfsample\n" + "Date: Mon, 9 Nov 2026 00:00:00 +0000\nMIME-Version: 1.0\n" + 'Content-Type: multipart/mixed; boundary="s"\n\n' + "--s\nContent-Type: text/plain\n\nhello\n\n" + f"--s\nContent-Type: {content_type}\n" + "Content-Transfer-Encoding: base64\n" + "Content-Disposition: attachment; filename=a\n\n" + f"{attachment}\n--s--\n\n" + ) + with self.subTest(content_type=content_type): + report = self._parse(self.FEEDBACK + sample) + self._assert_arf(report) + self.assertIn(content_type, report["sample"]) + + def test_legacy_text_then_a_feedback_part_without_its_sample_is_invalid(self): + """RFC 6591 §3.1 puts the sample after the feedback report part, so a + sample before it, or none, leaves the RFC 6591 report without one; + the legacy part read earlier is not mixed in.""" + for body in ( + self.LEGACY + self.FEEDBACK, + self.LEGACY + self.SAMPLE + self.FEEDBACK, + ): + with self.assertRaises(parsedmarc.InvalidDMARCReport): + parsedmarc.parse_report_email( + self.HEAD + body + "--r--\n", offline=True + ) + + @staticmethod + def _gzipped_aggregate_attachment() -> str: + with open( + "samples/aggregate/example.net!example.com!1529366400!1529452799.xml", + "rb", + ) as sample_file: + data = base64.encodebytes(gzip.compress(sample_file.read())).decode() + return ( + "Content-Type: application/gzip\nContent-Transfer-Encoding: base64\n" + f"Content-Disposition: attachment; filename=a.gz\n\n{data}\n" + ) + + def _sample_with(self, boundary: str, extra: str) -> str: + return ( + "--r\nContent-Type: message/rfc822\n\nFrom: x@example.com\n" + "To: y@example.org\nSubject: arfsample\n" + "Date: Mon, 9 Nov 2026 00:00:00 +0000\nMIME-Version: 1.0\n" + f'Content-Type: multipart/mixed; boundary="{boundary}"\n\n' + f"--{boundary}\nContent-Type: text/plain\n\nhello\n\n" + f"{extra}--{boundary}--\n\n" + ) + + def test_parts_inside_a_later_sample_candidate_are_not_reports(self): + """Only the first sample candidate is kept, but every candidate is + part of the reported message: a gzipped aggregate report inside a + second candidate was returned in place of the failure report.""" + attachment = "--s\n" + self._gzipped_aggregate_attachment() + headers = ( + "--r\nContent-Type: text/rfc822-headers\n\nFrom: x@example.com\n" + "To: y@example.org\nSubject: arfsample\n\n" + ) + for label, body in ( + ("headers, then rfc822", headers + self._sample_with("s", attachment)), + ( + "rfc822, then rfc822", + self._sample_with("q", "") + self._sample_with("s", attachment), + ), + ): + with self.subTest(label): + report = self._parse(self.FEEDBACK + body) + self.assertEqual(report["source"]["ip_address"], "192.0.2.1") + + def test_a_report_beside_the_sample_is_still_read(self): + """Only parts inside the sample are skipped: a gzipped aggregate + report after the sample, as a sibling, is returned as before.""" + body = ( + self.FEEDBACK + + self._sample_with("q", "") + + "--r\n" + + self._gzipped_aggregate_attachment() + ) + parsed = parsedmarc.parse_report_email( + self.HEAD + body + "--r--\n", offline=True + ) + self.assertEqual(parsed["report_type"], "aggregate") + + def test_legacy_text_inside_the_sample_is_ignored(self): + sample = ( + "--r\nContent-Type: message/rfc822\n\nFrom: x@example.com\n" + "To: y@example.org\nSubject: arfsample\n" + "Date: Mon, 9 Nov 2026 00:00:00 +0000\nMIME-Version: 1.0\n" + 'Content-Type: multipart/mixed; boundary="s"\n\n' + "--s\nContent-Type: text/plain\n\n" + "A message claiming to be from you has failed\n" + " Sender IP Address: 203.0.113.66\n" + " Received Date: Mon, 9 Nov 2026 00:00:00 +0000\ndetected.\n" + "From: forged@example.com\nSubject: injected\n\n--s--\n\n" + ) + self._assert_arf(self._parse(self.FEEDBACK + sample)) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tests/test_opensearch.py b/tests/test_opensearch.py index 8bb87fd4..f34773e7 100644 --- a/tests/test_opensearch.py +++ b/tests/test_opensearch.py @@ -8,18 +8,27 @@ construction, index naming, deduplication queries, error wrapping — without needing a running OpenSearch cluster. """ +import copy +import json +import re import time import unittest from collections.abc import Iterator from contextlib import contextmanager -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone +from pathlib import Path from typing import cast from unittest.mock import MagicMock, call, patch from opensearchpy import NotFoundError import parsedmarc.opensearch as opensearch_module -from parsedmarc import InvalidFailureReport, parse_report_file +from parsedmarc import ( + InvalidFailureReport, + parse_aggregate_report_xml, + parse_report_email, + parse_report_file, +) from parsedmarc.opensearch import ( AlreadySaved, OpenSearchError, @@ -366,32 +375,50 @@ class TestSetHosts(unittest.TestCase): class TestCreateIndexes(unittest.TestCase): def test_creates_missing_index_with_default_settings(self): - with patch("parsedmarc.opensearch.Index") as mock_index_cls: - mock_index = mock_index_cls.return_value - mock_index.exists.return_value = False + with ( + patch("parsedmarc.opensearch.Index") as mock_index_cls, + patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn, + ): + mock_index_cls.return_value.exists.return_value = False create_indexes(["dmarc_aggregate-2024-01-15"]) - mock_index.settings.assert_called_once_with( - number_of_shards=1, number_of_replicas=0 + mock_get_conn.return_value.indices.create.assert_called_once_with( + index="dmarc_aggregate-2024-01-15", + body={"settings": {"number_of_shards": 1, "number_of_replicas": 0}}, ) - mock_index.create.assert_called_once() def test_creates_with_custom_settings(self): - with patch("parsedmarc.opensearch.Index") as mock_index_cls: - mock_index = mock_index_cls.return_value - mock_index.exists.return_value = False + with ( + patch("parsedmarc.opensearch.Index") as mock_index_cls, + patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn, + ): + mock_index_cls.return_value.exists.return_value = False create_indexes( ["idx"], settings={"number_of_shards": 3, "refresh_interval": "5s"} ) - mock_index.settings.assert_called_once_with( - number_of_shards=3, refresh_interval="5s" + body = mock_get_conn.return_value.indices.create.call_args.kwargs["body"] + self.assertEqual( + body, {"settings": {"number_of_shards": 3, "refresh_interval": "5s"}} ) + def test_empty_settings_send_mappings_only(self): + with ( + patch("parsedmarc.opensearch.Index") as mock_index_cls, + patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn, + ): + mock_index_cls.return_value.exists.return_value = False + create_indexes(["idx"], settings={}, mappings={"date_detection": False}) + body = mock_get_conn.return_value.indices.create.call_args.kwargs["body"] + self.assertEqual(body, {"mappings": {"date_detection": False}}) + def test_skips_existing_index(self): - with patch("parsedmarc.opensearch.Index") as mock_index_cls: - mock_index = mock_index_cls.return_value - mock_index.exists.return_value = True - create_indexes(["idx"]) - mock_index.create.assert_not_called() + with ( + patch("parsedmarc.opensearch.Index") as mock_index_cls, + patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn, + ): + mock_index_cls.return_value.exists.return_value = True + create_indexes(["idx"], mappings={"date_detection": False}) + mock_get_conn.return_value.indices.create.assert_not_called() + mock_get_conn.return_value.indices.put_mapping.assert_not_called() def test_wraps_sdk_error(self): with patch("parsedmarc.opensearch.Index") as mock_index_cls: @@ -1635,7 +1662,7 @@ class TestCreateDataStreams(_DataStreamTestCase): name="dmarc_aggregate" ) # No concrete index is created for a stream. - mock_index_cls.return_value.create.assert_not_called() + client.indices.create.assert_not_called() self.assertIn("dmarc_aggregate", opensearch_module._READY_DATA_STREAMS) def test_default_settings_are_one_shard_no_replicas(self): @@ -1791,7 +1818,7 @@ class TestSaveToDataStreams(_DataStreamTestCase): client.indices.create_data_stream.assert_called_once_with( name="cust_dmarc_aggregate_tenant_a" ) - mock_index_cls.return_value.create.assert_not_called() + client.indices.create.assert_not_called() # The dedup search still uses the wildcard, which matches the # stream as well as dated indexes written before the switch. self.assertEqual( @@ -1812,7 +1839,7 @@ class TestSaveToDataStreams(_DataStreamTestCase): index_call["body"]["@timestamp"], datetime(2024, 1, 15, tzinfo=timezone.utc), ) - mock_index_cls.return_value.create.assert_called_once() + client.indices.create.assert_called_once() client.indices.put_index_template.assert_not_called() client.indices.create_data_stream.assert_not_called() @@ -1831,7 +1858,7 @@ class TestSaveToDataStreams(_DataStreamTestCase): datetime(2024, 1, 1, tzinfo=timezone.utc), ) client.indices.create_data_stream.assert_called_once_with(name="dmarc_failure") - mock_index_cls.return_value.create.assert_not_called() + client.indices.create.assert_not_called() self.assertEqual( mock_search_cls.call_args.kwargs["index"], "dmarc_failure*,dmarc_forensic*", @@ -1863,7 +1890,7 @@ class TestSaveToDataStreams(_DataStreamTestCase): datetime(2024, 2, 3, tzinfo=timezone.utc), ) client.indices.create_data_stream.assert_called_once_with(name="cust_smtp_tls") - mock_index_cls.return_value.create.assert_not_called() + client.indices.create.assert_not_called() self.assertEqual(mock_search_cls.call_args.kwargs["index"], "cust_smtp_tls*") def test_smtp_tls_report_dated_mode_unchanged_apart_from_timestamp_and_op_type( @@ -1950,9 +1977,558 @@ class TestBackwardCompatAlias(unittest.TestCase): ) +class TestMappingGuards(_DataStreamTestCase): + """New indexes and data stream templates carry explicit mappings + (GHSA-qpcg-29vr-jwm3). + + Before 11.2.2 they were created with settings only, so the cluster + inferred every field's type from the first document, with + ``date_detection`` on: an aggregate report with ``report_id`` + ``2026-11-09`` mapped ``report_id`` as a date for its day, a failure + sample header ``Subject.x`` mapped ``sample.headers.subject`` as an + object, and invented header names used up the 1,000-field limit, after + which legitimate reports for that index were rejected. + """ + + def _save_dated(self, save_function, report): + with ( + _registered_client() as client, + patch("parsedmarc.opensearch.Search") as mock_search_cls, + patch("parsedmarc.opensearch.Index") as mock_index_cls, + ): + mock_search_cls.return_value.execute.return_value = [] + mock_index_cls.return_value.exists.return_value = False + save_function(report) + client.indices.create.assert_called_once() + return client + + def _created_mappings(self, save_function, report): + client = self._save_dated(save_function, report) + return client.indices.create.call_args.kwargs["body"]["mappings"] + + def _template_mappings(self, save_function, report): + with ( + _registered_client() as client, + patch("parsedmarc.opensearch.Search") as mock_search_cls, + patch("parsedmarc.opensearch.Index") as mock_index_cls, + ): + mock_search_cls.return_value.execute.return_value = [] + client.indices.get_data_stream.side_effect = _not_found() + mock_index_cls.return_value.exists.return_value = False + save_function(report, use_data_streams=True) + template = client.indices.put_index_template.call_args.kwargs["body"] + return template["template"]["mappings"] + + def _assert_dates_declared(self, mappings, names): + for name in names: + self.assertEqual(mappings["properties"][name], {"type": "date"}, name) + + def test_aggregate_index_turns_date_detection_off(self): + mappings = self._created_mappings( + save_aggregate_report_to_opensearch, _aggregate_report() + ) + self.assertIs(mappings["date_detection"], False) + self._assert_dates_declared( + mappings, ["@timestamp", "date_range", "date_begin", "date_end"] + ) + self.assertNotIn("sample", mappings["properties"]) + # report_id stays dynamic: with date detection off a date-shaped + # value maps it as a string, not a date. + self.assertNotIn("report_id", mappings["properties"]) + + def test_smtp_tls_index_turns_date_detection_off(self): + mappings = self._created_mappings( + save_smtp_tls_report_to_opensearch, _smtp_tls_report() + ) + self.assertIs(mappings["date_detection"], False) + self._assert_dates_declared( + mappings, ["@timestamp", "date_range", "date_begin", "date_end"] + ) + self.assertNotIn("sample", mappings["properties"]) + + def test_failure_index_maps_sample_headers_without_dynamic_fields(self): + mappings = self._created_mappings( + save_failure_report_to_opensearch, _failure_report() + ) + self.assertIs(mappings["date_detection"], False) + self._assert_dates_declared(mappings, ["@timestamp", "arrival_date"]) + sample = mappings["properties"]["sample"]["properties"] + self.assertEqual(sample["date"], {"type": "date"}) + headers = sample["headers"] + self.assertEqual(headers["type"], "object") + self.assertIs(headers["dynamic"], False) + string_field = { + "type": "text", + "fields": {"keyword": {"type": "keyword", "ignore_above": 256}}, + } + for name in ("from", "to", "subject", "date", "message-id", "cc"): + self.assertEqual(headers["properties"][name], string_field, name) + for name in headers["properties"]: + # Stored names are lower-cased with "." replaced, so a declared + # name must be in that form or no stored header can match it. + self.assertEqual(name, name.lower().replace(".", "_"), name) + + def test_failure_mapping_declares_every_header_the_dashboards_query(self): + dashboards = Path(__file__).resolve().parent.parent / "dashboards" + referenced = set() + for path in dashboards.rglob("*"): + if path.is_file() and path.suffix in (".json", ".ndjson", ".xml"): + text = path.read_text(encoding="utf-8") + for match in re.finditer(r"sample\.headers\.([A-Za-z0-9_-]+)", text): + referenced.add(match.group(1).lower()) + self.assertIn("x-mailer", referenced) + mappings = self._created_mappings( + save_failure_report_to_opensearch, _failure_report() + ) + declared = mappings["properties"]["sample"]["properties"]["headers"][ + "properties" + ] + self.assertLessEqual(referenced, set(declared)) + + def test_data_stream_templates_carry_the_mappings(self): + for save_function, report, has_sample in ( + (save_aggregate_report_to_opensearch, _aggregate_report(), False), + (save_failure_report_to_opensearch, _failure_report(), True), + (save_smtp_tls_report_to_opensearch, _smtp_tls_report(), False), + ): + case = save_function.__name__ + opensearch_module._READY_DATA_STREAMS.clear() + mappings = self._template_mappings(save_function, report) + self.assertIs(mappings["date_detection"], False, case) + self.assertEqual( + mappings["properties"]["@timestamp"], {"type": "date"}, case + ) + self.assertEqual("sample" in mappings["properties"], has_sample, case) + if has_sample: + headers = mappings["properties"]["sample"]["properties"]["headers"] + self.assertIs(headers["dynamic"], False, case) + self.assertIn("subject", headers["properties"], case) + + def test_data_stream_template_adds_timestamp_without_changing_the_argument( + self, + ): + mappings = {"date_detection": False, "properties": {"x": {"type": "long"}}} + with ( + _registered_client() as client, + patch("parsedmarc.opensearch.Index") as mock_index_cls, + ): + client.indices.get_data_stream.side_effect = _not_found() + mock_index_cls.return_value.exists.return_value = False + create_data_streams(["s"], mappings=mappings) + template = client.indices.put_index_template.call_args.kwargs["body"] + self.assertEqual( + template["template"]["mappings"], + { + "date_detection": False, + "properties": {"x": {"type": "long"}, "@timestamp": {"type": "date"}}, + }, + ) + self.assertEqual( + mappings, {"date_detection": False, "properties": {"x": {"type": "long"}}} + ) + + def test_dotted_header_name_is_stored_with_an_underscore(self): + report = _failure_report() + report["parsed_sample"] = dict(report["parsed_sample"]) + report["parsed_sample"]["headers"] = { + "From": [["", "sender@example.com"]], + "Subject.x": "v", + "X.Y.Z": "w", + } + client = self._save_dated(save_failure_report_to_opensearch, report) + headers = client.index.call_args.kwargs["body"]["sample"]["headers"] + self.assertEqual(headers["subject_x"], "v") + self.assertEqual(headers["x_y_z"], "w") + self.assertNotIn("subject.x", headers) + self.assertNotIn("subject", headers) + # The address headers are still flattened under their usual keys. + self.assertEqual(headers["from"], "sender@example.com") + + # Silence unused-import lint in the test module preamble. _ = call +class TestAggregateIndexCap(unittest.TestCase): + """One report may create at most MAX_INDEXES_PER_REPORT dated indexes; + the overflow records are saved to the report's first index.""" + + DAYS = 40 + + def _report(self): + report = _aggregate_report() + template = report["records"][0] + report["records"] = [] + for day in range(self.DAYS): + record = copy.deepcopy(template) + begin = datetime(2024, 1, 1, tzinfo=timezone.utc) + timedelta(days=day) + end = begin + timedelta(days=1) + record["interval_begin"] = begin.strftime("%Y-%m-%d %H:%M:%S") + record["interval_end"] = end.strftime("%Y-%m-%d %H:%M:%S") + report["records"].append(record) + return report + + def _save(self, **kwargs): + """Save the report; return created index names, the (index, + date_begin) of each saved document and the logger mock.""" + saved = [] + + def capture(doc, *args, **save_kwargs): + saved.append((doc.meta.index, doc.date_begin)) + + with ( + patch("parsedmarc.opensearch.Search", return_value=_empty_search()), + patch("parsedmarc.opensearch.Index") as mock_index_cls, + patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn, + patch("parsedmarc.opensearch.logger") as mock_logger, + patch("parsedmarc.opensearch.create_data_streams") as mock_streams, + patch.object( + opensearch_module._AggregateReportDoc, + "save", + autospec=True, + side_effect=capture, + ), + ): + mock_index_cls.return_value.exists.return_value = False + save_aggregate_report_to_opensearch(self._report(), **kwargs) + created = [ + c.kwargs["index"] + for c in mock_get_conn.return_value.indices.create.call_args_list + ] + return created, saved, mock_logger, mock_streams + + def test_daily_indexes_are_capped_and_overflow_goes_to_the_first(self): + created, saved, mock_logger, _ = self._save() + expected = [f"dmarc_aggregate-2024-01-{d:02d}" for d in range(1, 32)] + self.assertEqual(sorted(set(created)), expected) + self.assertEqual(len(saved), self.DAYS) + self.assertEqual([index for index, _ in saved[:31]], expected) + self.assertEqual( + [index for index, _ in saved[31:]], ["dmarc_aggregate-2024-01-01"] * 9 + ) + # The overflow records keep their own dates. + begins = [begin for _, begin in saved] + self.assertEqual(begins[31], datetime(2024, 2, 1, tzinfo=timezone.utc)) + self.assertEqual(begins[39], datetime(2024, 2, 9, tzinfo=timezone.utc)) + mock_logger.warning.assert_called_once() + self.assertIn("9 records", mock_logger.warning.call_args.args[0]) + + def test_routing_warning_escapes_the_report_id(self): + """The report ID is sender-controlled; a CR/LF in it must not forge + a log line.""" + report = self._report() + report["report_metadata"]["report_id"] = "poc\r\nforged" + with ( + patch("parsedmarc.opensearch.Search", return_value=_empty_search()), + patch("parsedmarc.opensearch.Index") as mock_index_cls, + patch("parsedmarc.opensearch.connections.get_connection"), + patch("parsedmarc.opensearch.logger") as mock_logger, + patch.object(opensearch_module._AggregateReportDoc, "save", autospec=True), + ): + mock_index_cls.return_value.exists.return_value = False + save_aggregate_report_to_opensearch(report) + message = mock_logger.warning.call_args.args[0] + self.assertIn("poc\\r\\nforged", message) + self.assertNotIn("\n", message) + + def test_monthly_indexes_are_not_affected(self): + created, saved, mock_logger, _ = self._save(monthly_indexes=True) + self.assertEqual( + sorted(set(created)), ["dmarc_aggregate-2024-01", "dmarc_aggregate-2024-02"] + ) + self.assertEqual( + [index for index, _ in saved], + ["dmarc_aggregate-2024-01"] * 31 + ["dmarc_aggregate-2024-02"] * 9, + ) + mock_logger.warning.assert_not_called() + + def test_data_streams_are_not_affected(self): + created, saved, mock_logger, mock_streams = self._save(use_data_streams=True) + self.assertEqual(created, []) + self.assertEqual({index for index, _ in saved}, {"dmarc_aggregate"}) + self.assertEqual(len(saved), self.DAYS) + self.assertEqual(mock_streams.call_count, self.DAYS) + mock_logger.warning.assert_not_called() + + +class TestSaveFailureReportEmptyAddressHeaders(unittest.TestCase): + """GHSA-4272-4r8g-6c5h: a failure sample whose From, To or Reply-To + header holds no address made the save index ``headers[...][0]`` of + mail-parser's empty list and raise ``IndexError``, which escaped the + CLI and stopped the mailbox run. The header is now left out of the + document and of the duplicate query.""" + + # An empty From: and Reply-To:, and To: undisclosed-recipients:; (an + # RFC 5322 §3.4 group with no members, which legitimate mail uses). + ARF = ( + "From: dmarc@reporter.example\n" + "To: ruf@example.com\n" + "Date: Mon, 01 Jan 2024 00:00:00 +0000\n" + "Subject: DMARC Failure Report\n" + "MIME-Version: 1.0\n" + 'Content-Type: multipart/report; report-type=feedback-report; boundary="b"\n' + "\n" + "--b\n" + "Content-Type: text/plain\n" + "\n" + "Authentication failure report.\n" + "--b\n" + "Content-Type: message/feedback-report\n" + "\n" + "Feedback-Type: auth-failure\n" + "User-Agent: test/1.0\n" + "Version: 1\n" + "Original-Mail-From: user@example.com\n" + "Arrival-Date: Mon, 01 Jan 2024 00:00:00 +0000\n" + "Source-IP: 192.0.2.1\n" + "Reported-Domain: example.com\n" + "Authentication-Results: mx; dmarc=fail\n" + "\n" + "--b\n" + "Content-Type: text/rfc822-headers\n" + "\n" + "From:\n" + "To: undisclosed-recipients:;\n" + "Reply-To:\n" + "Subject: spoofed\n" + "Date: Mon, 01 Jan 2024 00:00:00 +0000\n" + "--b--\n" + ) + + def test_empty_address_headers_are_left_out(self): + parsed = parse_report_email(self.ARF, offline=True) + self.assertEqual(parsed["report_type"], "failure") + report = cast(dict, parsed["report"]) + headers = report["parsed_sample"]["headers"] + self.assertEqual( + (headers["From"], headers["To"], headers["Reply-To"]), ([], [], []) + ) + search = _empty_search() + with ( + patch("parsedmarc.opensearch.Search", return_value=search), + patch("parsedmarc.opensearch.Index"), + patch.object( + opensearch_module._FailureReportDoc, "save", autospec=True + ) as mock_save, + ): + save_failure_report_to_opensearch(report) + doc = mock_save.call_args.args[0] + self.assertEqual(doc.sample.headers["subject"], "spoofed") + for header in ("from", "to", "reply-to"): + self.assertNotIn(header, doc.sample.headers) + query = json.dumps(search.query.to_dict()) + self.assertIn("sample.headers.subject", query) + self.assertNotIn("sample.headers.from", query) + self.assertNotIn("sample.headers.to", query) + + +class TestAggregateAttributesDoNotReachTheDocument(unittest.TestCase): + """GHSA-qpcg-29vr-jwm3: an XML attribute on a scalar element reached the + document as an object and mapped the field as one.""" + + def test_document_holds_no_object_for_attributed_fields(self): + attribute = ' a="1"' + xml = ( + f"1.0" + "example.neta@example.net" + "r1704067200" + "1704153599" + "example.com" + f"none

1" + "192.0.2.11" + f"none" + "passpass" + "example.com" + f"example.org" + "example.compass" + "" + ) + report = parse_aggregate_report_xml(xml, offline=True) + docs = [] + with ( + patch("parsedmarc.opensearch.Search", return_value=_empty_search()), + patch("parsedmarc.opensearch.Index") as mock_index_cls, + patch("parsedmarc.opensearch.connections.get_connection"), + patch.object( + opensearch_module._AggregateReportDoc, + "save", + autospec=True, + side_effect=lambda doc, *args, **kwargs: docs.append(doc.to_dict()), + ), + ): + mock_index_cls.return_value.exists.return_value = False + save_aggregate_report_to_opensearch(cast(dict, report)) + doc = docs[0] + self.assertEqual(doc["xml_schema"], "1.0") + self.assertEqual(doc["disposition"], "none") + self.assertEqual(doc["envelope_to"], "example.org") + self.assertEqual(doc["published_policy"]["p"], "none") + self.assertEqual(doc["published_policy"]["fo"], "1") + + +# Every element of an aggregate report that the schema types as a string, +# with its plain value; {name} placeholders take a variant. +_SCALAR_FIELD_VALUES = { + "version": "1.0", + "org_name": "reporter.example", + "email": "dmarc@reporter.example", + "extra_contact_info": "https://reporter.example/dmarc", + "report_id": "scalar-test", + "generator": "reporter/1.0", + "domain": "example.com", + "adkim": "r", + "aspf": "s", + "p": "none", + "sp": "quarantine", + "pct": "100", + "fo": "1", + "np": "reject", + "testing": "n", + "discovery_method": "psl", + "source_ip": "192.0.2.1", + "count": "3", + "disposition": "none", + "dkim": "pass", + "spf": "fail", + "header_from": "example.com", + "envelope_from": "bounce.example.com", + "envelope_to": "example.org", + "dkim_domain": "example.com", + "selector": "s1", + "dkim_result": "pass", + "dkim_human_result": "good signature", + "spf_domain": "bounce.example.com", + "scope": "mfrom", + "spf_result": "fail", + "spf_human_result": "not listed", +} +_SCALAR_FIELD_TEMPLATE = ( + "{version}" + "{org_name}{email}" + "{extra_contact_info}" + "{report_id}{generator}" + "17040672001704153599" + "{domain}" + "{adkim}{aspf}

{p}

{sp}" + "{pct}{fo}{np}{testing}" + "{discovery_method}" + "{source_ip}{count}" + "{disposition}" + "{dkim}{spf}" + "otherc" + "{header_from}" + "{envelope_from}" + "{envelope_to}" + "{dkim_domain}{selector}" + "{dkim_result}{dkim_human_result}" + "{spf_domain}{scope}" + "{spf_result}{spf_human_result}" + "" +) + + +def _scalar_field_xml(**variants: str) -> str: + """The report with each named field's value replaced by its variant.""" + return _SCALAR_FIELD_TEMPLATE.format(**{**_SCALAR_FIELD_VALUES, **variants}) + + +# Fields whose element can hold child elements without the report being +# rejected. +_OPTIONAL_SCALAR_FIELDS = tuple( + name + for name in _SCALAR_FIELD_VALUES + if name + not in ( + "org_name", + "report_id", + "domain", + "p", + "source_ip", + "count", + "header_from", + "dkim_domain", + "spf_domain", + ) +) + + +# The fields of an aggregate report document that hold objects or lists; +# "[]" stands for a list item. Every other field must be a scalar. +_AGGREGATE_DOC_CONTAINERS = { + "date_range", + "published_policy", + "policy_overrides", + "policy_overrides[]", + "dkim_results", + "dkim_results[]", + "dkim_results_combined", + "spf_results", + "spf_results[]", + "spf_results_combined", + "errors", +} + + +def _container_paths(value, path=""): + """Yield the path of every dict and list inside ``value``.""" + if isinstance(value, dict): + if path: + yield path + for key, item in value.items(): + yield from _container_paths(item, f"{path}.{key}" if path else key) + elif isinstance(value, list): + yield path + for item in value: + yield from _container_paths(item, f"{path}[]") + + +# A second makes the field a list; the first one's

+# holds a child element. +_REPEATED_POLICY_VARIANTS = { + "p": "1", + "discovery_method": ( + "psl" + "example.com

none

psl" + ), +} + + +class TestAggregateScalarFieldsReachTheDocumentAsScalars(unittest.TestCase): + """GHSA-qpcg-29vr-jwm3: a child element under a field the schema types + as a string reached the document as an object and mapped the field as + one. The walk covers every field, so a scalar field added later is + checked too.""" + + def _document(self, **variants): + report = parse_aggregate_report_xml(_scalar_field_xml(**variants), offline=True) + docs = [] + with ( + patch("parsedmarc.opensearch.Search", return_value=_empty_search()), + patch("parsedmarc.opensearch.Index") as mock_index_cls, + patch("parsedmarc.opensearch.connections.get_connection"), + patch.object( + opensearch_module._AggregateReportDoc, + "save", + autospec=True, + side_effect=lambda doc, *args, **kwargs: docs.append(doc.to_dict()), + ), + ): + mock_index_cls.return_value.exists.return_value = False + save_aggregate_report_to_opensearch(cast(dict, report)) + return docs[0] + + def test_no_object_outside_the_object_fields(self): + mixed = {name: f"{value}" for name, value in _SCALAR_FIELD_VALUES.items()} + child = {name: "1" for name in _OPTIONAL_SCALAR_FIELDS} + for label, variants in ( + ("mixed", mixed), + ("child", child), + ("repeated policy_published", _REPEATED_POLICY_VARIANTS), + ): + paths = set(_container_paths(self._document(**variants))) + self.assertIn("published_policy", paths, msg=label) + self.assertLessEqual(paths, _AGGREGATE_DOC_CONTAINERS, msg=label) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tests/test_postgres.py b/tests/test_postgres.py index ecc2bbf4..0274ae16 100644 --- a/tests/test_postgres.py +++ b/tests/test_postgres.py @@ -1012,5 +1012,20 @@ class TestPostgreSQLWithSamples(unittest.TestCase): self.assertGreater(saved, 0, "Expected at least one SMTP TLS sample") +class TestEnsureUtcSuffixNonString(unittest.TestCase): + """GHSA-4272-4r8g-6c5h: ``_ensure_utc_suffix(5)`` raised ``TypeError`` + (``"+" in 5``) outside the save's ``psycopg.Error`` handler, so an SMTP + TLS report with ``"start-datetime": 5`` escaped the CLI. A non-string + is now returned unchanged, for PostgreSQL to accept or reject.""" + + def test_non_string_is_returned_unchanged(self): + for value in (5, 1.5, ["2024-01-01"]): + with self.subTest(value=value): + self.assertEqual( + _ensure_utc_suffix(value), # pyright: ignore[reportArgumentType] + value, + ) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_splunk.py b/tests/test_splunk.py index e43c474d..5b67e453 100644 --- a/tests/test_splunk.py +++ b/tests/test_splunk.py @@ -538,5 +538,24 @@ class TestSplunkBackwardCompatAlias(unittest.TestCase): ) +class TestSmtpTlsTimestampErrorIsSplunkError(unittest.TestCase): + """GHSA-4272-4r8g-6c5h: the SMTP TLS save converted ``begin_date`` + outside its ``try``, so a wrong-typed date (``"start-datetime": 5`` in + the report) raised a bare ``AttributeError`` that the CLI did not catch. + Parsing now rejects such a report; the save still raises + ``SplunkError`` for a value that does not convert.""" + + def test_unconvertible_begin_date_raises_splunk_error(self): + client = _client() + client.session = MagicMock() + for begin_date in (5, "not a date"): + report = _smtp_tls_report() + report["begin_date"] = begin_date + with self.subTest(begin_date=begin_date): + with self.assertRaises(SplunkError): + client.save_smtp_tls_reports_to_splunk([report]) + client.session.post.assert_not_called() + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tests/test_utils.py b/tests/test_utils.py index ad2284fe..c7543c42 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -27,6 +27,7 @@ from expiringdict import ExpiringDict import parsedmarc import parsedmarc.resources.ipinfo import parsedmarc.utils +from tests.timing import budget from tests.tzutil import force_tz @@ -293,11 +294,12 @@ class Test(unittest.TestCase): comment is left unclosed so the old pattern can never match: with a closing parenthesis its first attempt succeeded and the backtracking never happened.""" + limit = budget(5) start = time.perf_counter() # dateutil's ParserError subclasses ValueError. with self.assertRaises(ValueError): parsedmarc.utils.human_timestamp_to_datetime(" " * 200_000 + "(x") - self.assertLess(time.perf_counter() - start, 5) + self.assertLess(time.perf_counter() - start, limit) def testHumanTimestampToDatetimeNegativeZero(self): """-0000 timezone is handled""" @@ -628,9 +630,10 @@ class TestRedactUrlCredentials(unittest.TestCase): "/@" * 500_000, ): with self.subTest(prefix=value[:8]): + limit = budget(5) started = time.monotonic() parsedmarc.utils.redact_url_credentials(value) - self.assertLess(time.monotonic() - started, 5) + self.assertLess(time.monotonic() - started, limit) class TestSanitizeForLog(unittest.TestCase): @@ -1325,6 +1328,27 @@ class TestUtilsIpDbPaths(unittest.TestCase): class TestUtilsParseEmail(unittest.TestCase): """Tests for parse_email edge cases""" + def testOverLimitSampleIsRejected(self): + """parse_email raises MessageLimitExceeded for a sample whose + message/delivery-status body holds too many blank-line-separated + blocks, each a part (GHSA-qcc2-4x3v-v4g7), and still parses a normal + one""" + from mailsuite.utils import MessageLimitExceeded + + sample = ( + "From: test@example.com\nSubject: Test\n" + "Content-Type: message/delivery-status\n\n" + "\n" * 200_000 + ) + with self.assertRaises(MessageLimitExceeded) as ctx: + parsedmarc.utils.parse_email(sample) + self.assertEqual(str(ctx.exception), "Message exceeds max_parts (10000)") + with self.assertRaises(MessageLimitExceeded): + parsedmarc.utils.parse_email(sample.encode()) + result = parsedmarc.utils.parse_email( + "From: test@example.com\nSubject: Ok\n\nhi" + ) + self.assertEqual(result["subject"], "Ok") + def testMinimalEmail(self): """parse_email handles email with minimal headers""" email_str = """From: test@example.com diff --git a/tests/timing.py b/tests/timing.py new file mode 100644 index 00000000..f6d1662d --- /dev/null +++ b/tests/timing.py @@ -0,0 +1,29 @@ +"""Timing bounds that scale with the machine running the suite.""" + +import email +import time + +_REFERENCE_MESSAGE = ( + "From: sender@example.net\r\nTo: dmarc@example.com\r\nSubject: x\r\n\r\n" + + "a\r\n" * 300_000 +) + + +def budget(seconds: float) -> float: + """Return ``seconds``, scaled up on a machine slower than the reference. + + Timed checks do real work at a limit, so their bound must scale with the + machine running them. The reference is ``_REFERENCE_MESSAGE`` parsed by the + email package, 0.04 s on the workstation the bounds were written on + (Python 3.12). It is measured just before the timed call, twice, and the + slower run sets the scale, so a brief fast moment in the reference is not + taken as the machine's speed; a slowdown during the timed call itself is + not covered. A faster machine keeps the written bound, and the scale is + capped at ten, so a stall during the reference cannot hide a regression. + """ + reference = 0.0 + for _ in range(2): + start = time.perf_counter() + email.message_from_string(_REFERENCE_MESSAGE) + reference = max(reference, time.perf_counter() - start) + return seconds * min(max(1.0, reference / 0.04), 10.0)