From f3385596399fa2a9cb089b425008784627e43f5c Mon Sep 17 00:00:00 2001 From: Dan Mahoney Date: Tue, 23 Jun 2026 01:05:27 -0400 Subject: [PATCH 1/8] feat: always log StaleMARC/suppressions-DB report skips Previously every skip reason (StaleMARC, the suppressions DB, and NoReportsList) was only printed under --verbose, so a report silently skipped via StaleMARC or the suppressions table left no trace in a normal cron run. NoReportsList stays --verbose-only since it's operator-curated and the operator already knows what's in it; the other two are dynamic/external and worth surfacing by default. --- reports/opendmarc-reports.8.in | 20 +++++++++++++++++--- reports/opendmarc-reports.in | 21 +++++++++++++++++---- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/reports/opendmarc-reports.8.in b/reports/opendmarc-reports.8.in index 43b4bb0..f06a425 100644 --- a/reports/opendmarc-reports.8.in +++ b/reports/opendmarc-reports.8.in @@ -302,13 +302,27 @@ Print version number and exit. The script checks the .I suppressions database table before sending each report. If the destination RUA/RUF -address, or its domain, appears in that table the report is silently -skipped. The table is also checked at the domain level, so adding a bare -domain name (e.g. +address, or its domain, appears in that table the report is skipped. The +table is also checked at the domain level, so adding a bare domain name +(e.g. .IR example.com ) suppresses all reports for that domain regardless of their destination address. Applies to both aggregate and forensic modes. .PP +Skips caused by the suppressions table or by +.I --stale-check +are always written to standard error, even without +.IR --verbose , +since these come from dynamic or external sources the operator may not be +expecting. Skips caused by +.B NoReportsList +or +.I --skipdomains +are not, since those are operator-curated and the operator already knows +what they contain; pass +.I --verbose +to see those as well. +.PP The table is loaded once at startup and is not queried per-message, so changes take effect on the next run. If the table does not exist (e.g. on a pre-migration install) the check is skipped and a notice is written to diff --git a/reports/opendmarc-reports.in b/reports/opendmarc-reports.in index 320cb5b..b79af0b 100755 --- a/reports/opendmarc-reports.in +++ b/reports/opendmarc-reports.in @@ -263,6 +263,18 @@ sub noreports_skip return 0; } +# StaleMARC and the suppressions DB are dynamic/external sources that can +# silently drop reports the operator wasn't expecting, so skips they cause +# are always worth printing. NoReportsList is operator-curated -- the +# operator already knows what's in it -- so it stays --verbose-only to +# avoid flooding output when it's used heavily. +sub loud_skip +{ + my ($reason) = @_; + + return $reason eq "StaleMARC" || $reason eq "suppression DB"; +} + # Returns the reason a destination address should be skipped (for logging), # or undef if it should be sent to. Combines every suppression source so # the result -- and therefore the recipients who actually get mail -- is @@ -688,7 +700,7 @@ if ($forensic) if (defined($reason)) { print STDERR "$progname: --forensic: skipping report for $reported_domain ($reason)\n" - if $verbose; + if $verbose || loud_skip($reason); exit($EXIT_SUPPRESSED); } } @@ -728,7 +740,7 @@ if ($forensic) @rcpts = grep { my $reason = skip_reason($_); print STDERR "$progname: --forensic: skipping $_ ($reason)\n" - if defined($reason) && $verbose; + if defined($reason) && ($verbose || loud_skip($reason)); !defined($reason); } @rcpts; @@ -1035,7 +1047,8 @@ foreach (@$domainset) my $domain_reason = domain_skip_reason($domain); if (defined($domain_reason)) { - print STDERR "$progname: skipping $domain ($domain_reason)\n" if $verbose; + print STDERR "$progname: skipping $domain ($domain_reason)\n" + if $verbose || loud_skip($domain_reason); next; } @@ -1632,7 +1645,7 @@ foreach (@$domainset) if (defined($reason)) { print STDERR "$progname: skipping $domain report to $repdest ($reason)\n" - if $verbose; + if $verbose || loud_skip($reason); next; } } From 1b7e40c5e220f751885fdab2017b07e5804ffb3c Mon Sep 17 00:00:00 2001 From: Dan Mahoney Date: Tue, 23 Jun 2026 10:19:28 -0400 Subject: [PATCH 2/8] feat: send loud_skip() report skips to syslog (facility mail) Checking the mail log is the natural first move when a report didn't go out, but a StaleMARC/suppressions-DB skip never reaches the local MTA, so it never had a footprint there. Open syslog at startup with facility mail and have log_skip() (replacing the inline print conditionals) notice() those skips there too, so they land in the conventional mail log (e.g. /var/log/maillog) instead of only being visible via stderr/cron-mail. NoReportsList/--skipdomains skips stay off syslog entirely, consistent with their existing --verbose-only stderr treatment. --- reports/opendmarc-reports.8.in | 14 ++++++++----- reports/opendmarc-reports.in | 38 +++++++++++++++++++++++++++------- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/reports/opendmarc-reports.8.in b/reports/opendmarc-reports.8.in index f06a425..da17c5b 100644 --- a/reports/opendmarc-reports.8.in +++ b/reports/opendmarc-reports.8.in @@ -313,15 +313,19 @@ Skips caused by the suppressions table or by .I --stale-check are always written to standard error, even without .IR --verbose , -since these come from dynamic or external sources the operator may not be -expecting. Skips caused by +and are also sent to syslog using facility +.B mail +and priority +.BR notice , +since a skipped report never reaches the local MTA to leave its own trace +in the conventional mail log. Skips caused by .B NoReportsList or .I --skipdomains -are not, since those are operator-curated and the operator already knows -what they contain; pass +go to neither by default, since those are operator-curated and the +operator already knows what they contain; pass .I --verbose -to see those as well. +to see those on standard error as well (they are never sent to syslog). .PP The table is loaded once at startup and is not queried per-message, so changes take effect on the next run. If the table does not exist (e.g. on diff --git a/reports/opendmarc-reports.in b/reports/opendmarc-reports.in index b79af0b..429a427 100755 --- a/reports/opendmarc-reports.in +++ b/reports/opendmarc-reports.in @@ -23,6 +23,7 @@ use IO::Compress::Gzip qw(gzip); use POSIX; use MIME::Base64; use Net::SMTP; +use Sys::Syslog qw(:standard :macros); use Time::Local; require HTTP::Request; @@ -275,6 +276,26 @@ sub loud_skip return $reason eq "StaleMARC" || $reason eq "suppression DB"; } +# Prints a report-skip diagnostic to stderr, gated by --verbose except for +# loud_skip() reasons which are always shown there too. loud_skip() reasons +# are additionally sent to syslog (facility mail, opened at startup) since a +# skipped report never reaches the local MTA to leave its own trace in the +# conventional mail log. +sub log_skip +{ + my ($reason, $message) = @_; + + if ($verbose || loud_skip($reason)) + { + print STDERR "$progname: $message\n"; + } + + if (loud_skip($reason)) + { + syslog('notice', '%s', $message); + } +} + # Returns the reason a destination address should be skipped (for logging), # or undef if it should be sent to. Combines every suppression source so # the result -- and therefore the recipients who actually get mail -- is @@ -493,6 +514,11 @@ if (-f $configfile) # set locale setlocale(LC_ALL, 'C'); +# Facility "mail" so loud_skip() events land in the conventional mail log +# (e.g. /var/log/maillog) alongside everything else about mail delivery, +# even though these never reach the local MTA to write their own entry. +openlog($progname, 'pid', 'mail'); + # parse command line arguments my $opt_retval = &Getopt::Long::GetOptions ('archive-dir=s' => \$archive_dir, 'day!' => \$daybound, @@ -699,8 +725,7 @@ if ($forensic) my $reason = domain_skip_reason($reported_domain); if (defined($reason)) { - print STDERR "$progname: --forensic: skipping report for $reported_domain ($reason)\n" - if $verbose || loud_skip($reason); + log_skip($reason, "--forensic: skipping report for $reported_domain ($reason)"); exit($EXIT_SUPPRESSED); } } @@ -739,8 +764,7 @@ if ($forensic) # suppressed identically no matter which path is delivering mail. @rcpts = grep { my $reason = skip_reason($_); - print STDERR "$progname: --forensic: skipping $_ ($reason)\n" - if defined($reason) && ($verbose || loud_skip($reason)); + log_skip($reason, "--forensic: skipping $_ ($reason)") if defined($reason); !defined($reason); } @rcpts; @@ -1047,8 +1071,7 @@ foreach (@$domainset) my $domain_reason = domain_skip_reason($domain); if (defined($domain_reason)) { - print STDERR "$progname: skipping $domain ($domain_reason)\n" - if $verbose || loud_skip($domain_reason); + log_skip($domain_reason, "skipping $domain ($domain_reason)"); next; } @@ -1644,8 +1667,7 @@ foreach (@$domainset) my $reason = skip_reason($repdest); if (defined($reason)) { - print STDERR "$progname: skipping $domain report to $repdest ($reason)\n" - if $verbose || loud_skip($reason); + log_skip($reason, "skipping $domain report to $repdest ($reason)"); next; } } From 097995cb8ac02e334fe39eef84a109ee953b2a39 Mon Sep 17 00:00:00 2001 From: Dan Mahoney Date: Sun, 19 Jul 2026 23:30:33 -0700 Subject: [PATCH 3/8] feat: wire RFC 9990 np/testing/discovery_method through aggregate reporting Adds the three RFC 9990 policy_published fields OpenDMARC never emitted: np (non-existent-subdomain policy), testing (the RFC 9989 t= tag), and discovery_method (psl|treewalk, tracking whether a record was found via PSL-style org-domain lookup or the RFC 9989 DNS tree walk). Bumps the report namespace to urn:ietf:params:xml:ns:dmarc-2.0 and drops , which RFC 9990 removes from the schema entirely -- pct= is still honored for enforcement, unchanged, it's just no longer reported to recipients. Threaded through the full pipeline: new libopendmarc accessors (opendmarc_policy_fetch_np/_discovery_method) and discovery_method tracking in the walk-mode query functions, historyfile lines in the milter, opendmarc-import parsing into new requests.npolicy/testing/ discovery_method columns (db/update-db-schema.mysql + schema.mysql), and conditional XML output in opendmarc-reports matching the schema's minOccurs="0" on all three. Verified: full build + 12/12 libopendmarc test suite (including live-DNS discovery_method assertions against gushi.org) on quark against real libspf2, since libspf2 isn't available in the local dev sandbox. Both the full-fields and all-omitted report shapes validate against RFC 9990's own Appendix A XSD via xmllint. --- CHANGES-202605.md | 1 + db/schema.mysql | 3 ++ db/update-db-schema.mysql | 7 ++++ libopendmarc/dmarc.h.in | 13 ++++++- libopendmarc/opendmarc_internal.h | 1 + libopendmarc/opendmarc_policy.c | 33 ++++++++++++++++ libopendmarc/tests/test_dmarc_fetch.c | 44 ++++++++++++++++++++- libopendmarc/tests/test_dmarc_walk.c | 17 ++++++++ opendmarc/opendmarc.c | 10 +++++ reports/opendmarc-import.in | 22 ++++++++++- reports/opendmarc-reports.in | 56 +++++++++++++++++++++++---- 11 files changed, 195 insertions(+), 12 deletions(-) diff --git a/CHANGES-202605.md b/CHANGES-202605.md index d0a869c..88bc259 100644 --- a/CHANGES-202605.md +++ b/CHANGES-202605.md @@ -45,6 +45,7 @@ This document summarizes the changes merged into the `develop` branch during the ## Aggregate reports (RFC 9989/9990 compliance) - **RFC 9989 obsoletes RFC 7489**: OpenDMARC now targets RFC 9989 (DMARC) and RFC 9990 (aggregate reporting). Key behavioral change: RFC 9990 makes the DKIM `` element mandatory in `` (previously optional); the selector field is always emitted, using an empty string when the database has no selector recorded. +- **`policy_published` gains `np`, `testing`, and `discovery_method`; namespace bumped to `dmarc-2.0`**: `` now declares `xmlns="urn:ietf:params:xml:ns:dmarc-2.0"` per RFC 9990's registered namespace (`` stays `1.0`, unchanged -- Section 3.1.1.2 requires that exact value regardless of namespace). `` gains three new optional elements: `` (non-existent-subdomain policy, `np=`), `` (the `t=` tag introduced by RFC 9989), and `` (`psl` or `treewalk`, tracking whether the record was found via PSL-style org-domain lookup or the RFC 9989 DNS tree walk; omitted when the record was a direct match at `_dmarc.` with no discovery needed). **`` is removed**, matching RFC 9990's schema exactly -- `pct=` is still honored for enforcement, unchanged; it is simply no longer reported to recipients. New libopendmarc accessors `opendmarc_policy_fetch_np()` and `opendmarc_policy_fetch_discovery_method()`. Requires a database migration (`db/update-db-schema.mysql`): new `requests.npolicy`, `requests.testing`, `requests.discovery_method` columns. - **Aggregate reports now group identical rows**: `opendmarc-reports` previously emitted one `` per message with `1`. Messages sharing the same source IP, disposition, DKIM/SPF alignment, SPF result, SPF scope, From domain, and envelope domain are now collapsed into a single `` with an accurate ``. Per RFC 9990 section 3.1.1.11 the `` section accumulates all unique DKIM `(domain, selector, result)` tuples seen across messages in the group; the `` field is defined in section 3.1.1.8. This can significantly reduce report size for high-volume senders. (issue #212) --- diff --git a/db/schema.mysql b/db/schema.mysql index 0a07a33..b34a495 100644 --- a/db/schema.mysql +++ b/db/schema.mysql @@ -64,8 +64,11 @@ CREATE TABLE IF NOT EXISTS requests ( aspf TINYINT NOT NULL DEFAULT '0', policy TINYINT NOT NULL DEFAULT '0', spolicy TINYINT NOT NULL DEFAULT '0', + npolicy TINYINT NOT NULL DEFAULT '0', pct TINYINT NOT NULL DEFAULT '0', fo TINYINT NOT NULL DEFAULT '0', + testing TINYINT NOT NULL DEFAULT '0', + discovery_method TINYINT NOT NULL DEFAULT '0', locked TINYINT NOT NULL DEFAULT '0', firstseen TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, lastsent TIMESTAMP NULL DEFAULT NULL, diff --git a/db/update-db-schema.mysql b/db/update-db-schema.mysql index 2eed245..c09cc17 100644 --- a/db/update-db-schema.mysql +++ b/db/update-db-schema.mysql @@ -32,6 +32,13 @@ ALTER TABLE messages MODIFY COLUMN arc_policy TINYINT UNSIGNED NOT NULL DEFAULT -- RFC 7489 §8.2: spf_scope distinguishes mfrom vs. helo SPF evaluation. ALTER TABLE messages ADD COLUMN spf_scope TINYINT NOT NULL DEFAULT '-1' AFTER spf; +-- RFC 9990 policy_published extensions: npolicy (non-existent-subdomain +-- policy, the np= tag), testing (t=, RFC 9989 Appendix A.6), and +-- discovery_method (psl|treewalk, how the record was found). +ALTER TABLE requests ADD COLUMN npolicy TINYINT NOT NULL DEFAULT '0' AFTER spolicy; +ALTER TABLE requests ADD COLUMN testing TINYINT NOT NULL DEFAULT '0' AFTER fo; +ALTER TABLE requests ADD COLUMN discovery_method TINYINT NOT NULL DEFAULT '0' AFTER testing; + -- VERP bounce tracking: suppressions table for persistent report address suppression. CREATE TABLE IF NOT EXISTS suppressions ( id INT NOT NULL AUTO_INCREMENT, diff --git a/libopendmarc/dmarc.h.in b/libopendmarc/dmarc.h.in index ee686cd..2181f5a 100644 --- a/libopendmarc/dmarc.h.in +++ b/libopendmarc/dmarc.h.in @@ -125,7 +125,16 @@ typedef struct { #define DMARC_RECORD_PSD_UNSPECIFIED (0) /* psd= absent */ #define DMARC_RECORD_PSD_N (1) /* psd=n: organizational domain */ #define DMARC_RECORD_PSD_Y (2) /* psd=y: public suffix domain */ - + +/* RFC 9990 policy_published/discovery_method: how the record was found. + * UNSPECIFIED covers a direct match at _dmarc., where no PSL + * lookup or tree walk was needed at all; the RFC only defines "psl" and + * "treewalk", so that case is left unreported (discovery_method is + * OPTIONAL) rather than forced into one of the two. */ +#define OPENDMARC_DISCOVERY_UNSPECIFIED (0) +#define OPENDMARC_DISCOVERY_PSL (1) /* RFC 7489 org-domain method */ +#define OPENDMARC_DISCOVERY_TREEWALK (2) /* RFC 9989 S 4.10 DNS Tree Walk */ + /* * Library one time initialization. */ @@ -164,7 +173,9 @@ OPENDMARC_STATUS_T opendmarc_policy_fetch_adkim(DMARC_POLICY_T *pctx, int *adkim OPENDMARC_STATUS_T opendmarc_policy_fetch_aspf(DMARC_POLICY_T *pctx, int *aspf); OPENDMARC_STATUS_T opendmarc_policy_fetch_p(DMARC_POLICY_T *pctx, int *p); OPENDMARC_STATUS_T opendmarc_policy_fetch_sp(DMARC_POLICY_T *pctx, int *sp); +OPENDMARC_STATUS_T opendmarc_policy_fetch_np(DMARC_POLICY_T *pctx, int *np); OPENDMARC_STATUS_T opendmarc_policy_fetch_t(DMARC_POLICY_T *pctx, int *t); +OPENDMARC_STATUS_T opendmarc_policy_fetch_discovery_method(DMARC_POLICY_T *pctx, int *discovery_method); OPENDMARC_STATUS_T opendmarc_policy_fetch_fo(DMARC_POLICY_T *pctx, int *fo); u_char ** opendmarc_policy_fetch_rua(DMARC_POLICY_T *pctx, u_char *list_buf, size_t size_of_buf, int constant); u_char ** opendmarc_policy_fetch_ruf(DMARC_POLICY_T *pctx, u_char *list_buf, size_t size_of_buf, int constant); diff --git a/libopendmarc/opendmarc_internal.h b/libopendmarc/opendmarc_internal.h index 703d552..a3d1563 100644 --- a/libopendmarc/opendmarc_internal.h +++ b/libopendmarc/opendmarc_internal.h @@ -160,6 +160,7 @@ typedef struct dmarc_policy_t { u_char * from_domain; /* Input: From: header domain */ u_char * organizational_domain; int org_domain_from_fallback; /* Non-zero if PSL was absent and label-walk was used */ + int discovery_method; /* RFC 9990: OPENDMARC_DISCOVERY_*, how the record was found */ /* * Found in the _dmarc record or supplied to us. diff --git a/libopendmarc/opendmarc_policy.c b/libopendmarc/opendmarc_policy.c index b743c76..51b4856 100644 --- a/libopendmarc/opendmarc_policy.c +++ b/libopendmarc/opendmarc_policy.c @@ -907,6 +907,7 @@ query_dmarc_psl(DMARC_POLICY_T *pctx, u_char *domain, if (dmarc_query_at((char *)tld, &dns_reply, buf, bufsz) != NULL) { + pctx->discovery_method = OPENDMARC_DISCOVERY_PSL; if (dns_reply_out != NULL) *dns_reply_out = dns_reply; return DMARC_PARSE_OKAY; @@ -948,6 +949,7 @@ query_dmarc_rfc7489_walk(DMARC_POLICY_T *pctx, u_char *domain, { pctx->organizational_domain = (u_char *)strdup((char *)cur); pctx->org_domain_from_fallback = 1; + pctx->discovery_method = OPENDMARC_DISCOVERY_PSL; if (dns_reply_out != NULL) *dns_reply_out = dns_reply; return DMARC_PARSE_OKAY; @@ -1094,6 +1096,7 @@ query_dmarc_rfc9989_walk(DMARC_POLICY_T *pctx, u_char *domain, pctx->organizational_domain = (u_char *)strdup(best_domain); (void) strlcpy((char *)buf, (char *)best_buf, bufsz); + pctx->discovery_method = OPENDMARC_DISCOVERY_TREEWALK; return DMARC_PARSE_OKAY; } @@ -1832,6 +1835,36 @@ opendmarc_policy_fetch_t(DMARC_POLICY_T *pctx, int *t) return DMARC_PARSE_OKAY; } +OPENDMARC_STATUS_T +opendmarc_policy_fetch_np(DMARC_POLICY_T *pctx, int *np) +{ + if (pctx == NULL) + { + return DMARC_PARSE_ERROR_NULL_CTX; + } + if (np == NULL) + { + return DMARC_PARSE_ERROR_EMPTY; + } + *np = pctx->np; + return DMARC_PARSE_OKAY; +} + +OPENDMARC_STATUS_T +opendmarc_policy_fetch_discovery_method(DMARC_POLICY_T *pctx, int *discovery_method) +{ + if (pctx == NULL) + { + return DMARC_PARSE_ERROR_NULL_CTX; + } + if (discovery_method == NULL) + { + return DMARC_PARSE_ERROR_EMPTY; + } + *discovery_method = pctx->discovery_method; + return DMARC_PARSE_OKAY; +} + u_char ** opendmarc_policy_fetch_rua(DMARC_POLICY_T *pctx, u_char *list_buf, size_t size_of_buf, int constant) { diff --git a/libopendmarc/tests/test_dmarc_fetch.c b/libopendmarc/tests/test_dmarc_fetch.c index fe3060f..083a930 100644 --- a/libopendmarc/tests/test_dmarc_fetch.c +++ b/libopendmarc/tests/test_dmarc_fetch.c @@ -6,7 +6,7 @@ int main(int argc, char **argv) { - static char *record= "v=DMARC1; p=none; sp=none; adkim=s; aspf=s; pct=50; t=y; ri=300; rf=afrf; rua=mailto:dmarc-a@abuse.net; ruf=mailto:dmarc-f@abuse.net"; + static char *record= "v=DMARC1; p=none; sp=none; np=quarantine; adkim=s; aspf=s; pct=50; t=y; ri=300; rf=afrf; rua=mailto:dmarc-a@abuse.net; ruf=mailto:dmarc-f@abuse.net"; DMARC_POLICY_T *pctx; OPENDMARC_STATUS_T status; int pass, fails, count; @@ -14,7 +14,9 @@ main(int argc, char **argv) int adkim; int aspf; int t; - + int np; + int discovery_method; + pass = fails = count = 0; pctx = opendmarc_policy_connect_init((u_char *)"1.2.3.4", 0); if (pctx == NULL) @@ -72,6 +74,37 @@ main(int argc, char **argv) printf("\t%s(%d): opendmarc_policy_fetch_t: expected %d got %d: FAIL\n", __FILE__, __LINE__, DMARC_RECORD_T_Y, t); fails += 1; } + status = opendmarc_policy_fetch_np(pctx, &np); + if (status != DMARC_PARSE_OKAY) + { + printf("\t%s(%d): opendmarc_policy_fetch_np: %s: FAIL\n", __FILE__, __LINE__, opendmarc_policy_status_to_str(status)); + fails += 1; + } + if (np != DMARC_RECORD_P_QUARANTINE) + { + printf("\t%s(%d): opendmarc_policy_fetch_np: expected %d got %d: FAIL\n", __FILE__, __LINE__, DMARC_RECORD_P_QUARANTINE, np); + fails += 1; + } + + /* + ** RFC 9990: discovery_method is only ever set by the walk-mode query + ** functions (query_dmarc_psl/_rfc7489_walk/_rfc9989_walk), not by + ** opendmarc_policy_parse_dmarc() directly, which is all this test + ** exercises. So a directly-parsed record (no walk performed) should + ** report OPENDMARC_DISCOVERY_UNSPECIFIED -- see test_dmarc_walk.c for + ** the psl/treewalk cases. + */ + status = opendmarc_policy_fetch_discovery_method(pctx, &discovery_method); + if (status != DMARC_PARSE_OKAY) + { + printf("\t%s(%d): opendmarc_policy_fetch_discovery_method: %s: FAIL\n", __FILE__, __LINE__, opendmarc_policy_status_to_str(status)); + fails += 1; + } + if (discovery_method != OPENDMARC_DISCOVERY_UNSPECIFIED) + { + printf("\t%s(%d): opendmarc_policy_fetch_discovery_method: expected %d got %d: FAIL\n", __FILE__, __LINE__, OPENDMARC_DISCOVERY_UNSPECIFIED, discovery_method); + fails += 1; + } /* ** Regression tests for issue #256: opendmarc_policy_fetch_ruf() and @@ -150,6 +183,13 @@ main(int argc, char **argv) printf("\t%s(%d): opendmarc_policy_fetch_t: expected unspecified, got %d: FAIL\n", __FILE__, __LINE__, t); fails += 1; } + /* RFC 9990: np= absent defaults to DMARC_RECORD_P_UNSPECIFIED */ + status = opendmarc_policy_fetch_np(pctx, &np); + if (status != DMARC_PARSE_OKAY || np != DMARC_RECORD_P_UNSPECIFIED) + { + printf("\t%s(%d): opendmarc_policy_fetch_np: expected unspecified, got %d: FAIL\n", __FILE__, __LINE__, np); + fails += 1; + } pctx = opendmarc_policy_connect_shutdown(pctx); return fails; diff --git a/libopendmarc/tests/test_dmarc_walk.c b/libopendmarc/tests/test_dmarc_walk.c index a621920..39e8628 100644 --- a/libopendmarc/tests/test_dmarc_walk.c +++ b/libopendmarc/tests/test_dmarc_walk.c @@ -68,6 +68,7 @@ main(int argc, char **argv) OPENDMARC_STATUS_T status; u_char utilized[256]; int fallback; + int discovery_method; /* * === Test 0: direct hit === @@ -92,6 +93,10 @@ main(int argc, char **argv) (void) opendmarc_policy_fetch_org_domain_from_fallback(pctx, &fallback); CHECK(fallback == 0, "direct hit: no fallback should have occurred"); + + (void) opendmarc_policy_fetch_discovery_method(pctx, &discovery_method); + CHECK(discovery_method == OPENDMARC_DISCOVERY_UNSPECIFIED, + "direct hit: discovery_method should be unspecified, no walk occurred"); } pctx = opendmarc_policy_connect_shutdown(pctx); @@ -119,6 +124,10 @@ main(int argc, char **argv) (void) opendmarc_policy_fetch_utilized_domain(pctx, utilized, sizeof utilized); CHECK(strcasecmp((char *)utilized, "psdn." ZONE) == 0, "rfc9989 psd=n: org domain should be psdn.dmarcwalk.gushi.org"); + + (void) opendmarc_policy_fetch_discovery_method(pctx, &discovery_method); + CHECK(discovery_method == OPENDMARC_DISCOVERY_TREEWALK, + "rfc9989 psd=n: discovery_method should be treewalk"); } pctx = opendmarc_policy_connect_shutdown(pctx); @@ -141,6 +150,10 @@ main(int argc, char **argv) (void) opendmarc_policy_fetch_org_domain_from_fallback(pctx, &fallback); CHECK(fallback == 1, "rfc7489 psd=n name: fallback flag should be set"); + + (void) opendmarc_policy_fetch_discovery_method(pctx, &discovery_method); + CHECK(discovery_method == OPENDMARC_DISCOVERY_PSL, + "rfc7489 psd=n name: discovery_method should be psl (RFC 9990's name for the RFC 7489 method)"); } pctx = opendmarc_policy_connect_shutdown(pctx); @@ -409,6 +422,10 @@ main(int argc, char **argv) (void) opendmarc_policy_fetch_utilized_domain(pctx, utilized, sizeof utilized); CHECK(strcasecmp((char *)utilized, "nopsd." ZONE) == 0, "psl falling back to rfc9989: org domain should be nopsd.dmarcwalk.gushi.org"); + + (void) opendmarc_policy_fetch_discovery_method(pctx, &discovery_method); + CHECK(discovery_method == OPENDMARC_DISCOVERY_TREEWALK, + "psl falling back to rfc9989: discovery_method should reflect rfc9989, the strategy that actually resolved it"); } pctx = opendmarc_policy_connect_shutdown(pctx); diff --git a/opendmarc/opendmarc.c b/opendmarc/opendmarc.c index ebe0e02..6d466d7 100644 --- a/opendmarc/opendmarc.c +++ b/opendmarc/opendmarc.c @@ -2283,7 +2283,9 @@ mlfi_eom(SMFICTX *ctx) int pct; int p; int sp; + int np; int t; + int discovery_method; int align_dkim; int align_spf; int limit_arc = 0; @@ -3323,7 +3325,15 @@ mlfi_eom(SMFICTX *ctx) opendmarc_policy_fetch_sp(cc->cctx_dmarc, &sp); dmarcf_dstring_printf(dfc->mctx_histbuf, "sp %d\n", sp); + opendmarc_policy_fetch_np(cc->cctx_dmarc, &np); + dmarcf_dstring_printf(dfc->mctx_histbuf, "np %d\n", np); + opendmarc_policy_fetch_t(cc->cctx_dmarc, &t); + dmarcf_dstring_printf(dfc->mctx_histbuf, "testing %d\n", t); + + opendmarc_policy_fetch_discovery_method(cc->cctx_dmarc, &discovery_method); + dmarcf_dstring_printf(dfc->mctx_histbuf, "discovery_method %d\n", + discovery_method); { int fo = DMARC_RECORD_FO_UNSPECIFIED; diff --git a/reports/opendmarc-import.in b/reports/opendmarc-import.in index 47d8481..438fa61 100755 --- a/reports/opendmarc-import.in +++ b/reports/opendmarc-import.in @@ -83,9 +83,12 @@ my $reporter; my $repuri; my $sigcount = 0; my $sp; +my $np = 0; my $spf; my $spf_scope = -1; my $fo = 0; +my $testing = 0; +my $discovery_method = 0; my @rua; ### @@ -421,9 +424,9 @@ sub update_db $dbi_s->finish; } - $dbi_s = $dbi_h->prepare("UPDATE requests SET adkim = ?, aspf = ?, policy = ?, spolicy = ?, pct = ?, fo = ? WHERE id = ?"); + $dbi_s = $dbi_h->prepare("UPDATE requests SET adkim = ?, aspf = ?, policy = ?, spolicy = ?, npolicy = ?, pct = ?, fo = ?, testing = ?, discovery_method = ? WHERE id = ?"); - if (!$dbi_s->execute($adkim, $aspf, $p, $sp, $pct, $fo, $request_id)) + if (!$dbi_s->execute($adkim, $aspf, $p, $sp, $np, $pct, $fo, $testing, $discovery_method, $request_id)) { print STDERR "$progname: failed to update policy data for $fdomain: " . $dbi_h->errstr . "\n"; $dbi_s->finish; @@ -716,9 +719,12 @@ while (<$inputfh>) undef @rua; $sigcount = 0; undef $sp; + $np = 0; undef $spf; $spf_scope = -1; $fo = 0; + $testing = 0; + $discovery_method = 0; } $jobid = $value; @@ -766,6 +772,18 @@ while (<$inputfh>) { $sp = $value; } + elsif ($key eq "np") + { + $np = $value; + } + elsif ($key eq "testing") + { + $testing = $value; + } + elsif ($key eq "discovery_method") + { + $discovery_method = $value; + } elsif ($key eq "spf") { $spf = $value; diff --git a/reports/opendmarc-reports.in b/reports/opendmarc-reports.in index 429a427..5936f63 100755 --- a/reports/opendmarc-reports.in +++ b/reports/opendmarc-reports.in @@ -66,11 +66,17 @@ my @skipdomains; my $policy; my $spolicy; +my $npolicy; my $policystr; my $spolicystr; +my $npolicystr; my $pct; my $fo; my $fostr; +my $testing; +my $testingstr; +my $discovery_method; +my $discovery_methodstr; my $repuri; my @repuris; @@ -1109,7 +1115,7 @@ foreach (@$domainset) # MySQL-specific: UNIX_TIMESTAMP() converts a DATETIME to a Unix timestamp. # SQLite equivalent: strftime('%s', lastsent) # PostgreSQL equivalent: EXTRACT(EPOCH FROM lastsent) - $dbi_s = $dbi_h->prepare("SELECT repuri, adkim, aspf, policy, spolicy, pct, fo, UNIX_TIMESTAMP(lastsent) FROM requests WHERE domain = ?"); + $dbi_s = $dbi_h->prepare("SELECT repuri, adkim, aspf, policy, spolicy, npolicy, pct, fo, testing, discovery_method, UNIX_TIMESTAMP(lastsent) FROM requests WHERE domain = ?"); $dbi_s->bind_param(1, $domainid, {TYPE => SQL_INTEGER}); if (!$dbi_s->execute()) { @@ -1145,15 +1151,27 @@ foreach (@$domainset) } if (defined($dbi_a->[5])) { - $pct = $dbi_a->[5]; + $npolicy = $dbi_a->[5]; } if (defined($dbi_a->[6])) { - $fo = $dbi_a->[6]; + $pct = $dbi_a->[6]; } if (defined($dbi_a->[7])) { - $lastsent = $dbi_a->[7]; + $fo = $dbi_a->[7]; + } + if (defined($dbi_a->[8])) + { + $testing = $dbi_a->[8]; + } + if (defined($dbi_a->[9])) + { + $discovery_method = $dbi_a->[9]; + } + if (defined($dbi_a->[10])) + { + $lastsent = $dbi_a->[10]; } } @@ -1237,6 +1255,16 @@ foreach (@$domainset) elsif ($spolicy == ord("r")) { $spolicystr = "reject"; } else { $spolicystr = "unknown"; } + # RFC 9990: np is OPTIONAL and, unlike sp, has no "falls back to p" + # reporting convention -- only emit it when the record actually + # published an np= value. + undef $npolicystr; + if (!defined($npolicy) || $npolicy == 0) { undef $npolicystr; } + elsif ($npolicy == ord("n")) { $npolicystr = "none"; } + elsif ($npolicy == ord("q")) { $npolicystr = "quarantine"; } + elsif ($npolicy == ord("r")) { $npolicystr = "reject"; } + else { $npolicystr = "unknown"; } + { my @fo_values; if (!defined($fo) || $fo == 0) @@ -1253,9 +1281,21 @@ foreach (@$domainset) } } + # RFC 9990: testing (t=) and discovery_method (psl|treewalk) are both + # OPTIONAL; omit them unless a concrete value was recorded. + undef $testingstr; + if (!defined($testing) || $testing == 0) { undef $testingstr; } + elsif ($testing == ord("y")) { $testingstr = "y"; } + elsif ($testing == ord("n")) { $testingstr = "n"; } + + undef $discovery_methodstr; + if (!defined($discovery_method) || $discovery_method == 0) { undef $discovery_methodstr; } + elsif ($discovery_method == 1) { $discovery_methodstr = "psl"; } + elsif ($discovery_method == 2) { $discovery_methodstr = "treewalk"; } + print $tmpout "\n"; - print $tmpout "\n"; - print $tmpout " 1\n"; + print $tmpout "\n"; + print $tmpout " 1.0\n"; print $tmpout " \n"; print $tmpout " $repdom\n"; @@ -1273,8 +1313,10 @@ foreach (@$domainset) print $tmpout " $aspfstr\n"; print $tmpout "

$policystr

\n"; print $tmpout " $spolicystr\n"; - print $tmpout " $pct\n"; + print $tmpout " $npolicystr\n" if defined($npolicystr); print $tmpout " $fostr\n"; + print $tmpout " $testingstr\n" if defined($testingstr); + print $tmpout " $discovery_methodstr\n" if defined($discovery_methodstr); print $tmpout " \n"; # MySQL-specific: FROM_UNIXTIME() in both queries below. From 59504909e4f767c7f9eeec30683efb3eb1669174 Mon Sep 17 00:00:00 2001 From: Dan Mahoney Date: Mon, 20 Jul 2026 01:07:27 -0700 Subject: [PATCH 4/8] feat: RFC 9991 failure reporting -- Identity-Alignment, ruf verification/rate-limiting, psd=y Closes the feasible-now portion of the RFC 9991 gap: Identity-Alignment and DKIM-Domain/-Identity/-Selector ARF header fields (RFC 9991 S4), external destination verification and rate-limiting for ruf= in opendmarc-reports --forensic (S2/S5/S8.1), and a psd=y exclusion for ruf= in the milter (S2). SPF-DNS and DKIM-Canonicalized-Header/-Body remain out of scope -- both need real library-level work (libspf2 doesn't expose the DNS record chain it consults; no DKIM verification library is linked anywhere in this codebase), not just wiring, and are left as follow-ups. Also fixes a regression found while working on this: PR #392's rua= external-destination verification (_report._dmarc DNS lookup) was silently wiped by PR #394's commit ~5 hours later (built from a pre-#392 checkout, squash-committed over it) despite CHANGES still documenting it as shipped. Restored, adapted to the six commits' worth of unrelated changes that have landed in that code since, and generalized to cover ruf= per RFC 9991 S5 (which reuses RFC 9990 S4's procedure verbatim). Also fixes a real bug surfaced while live-testing the restored verification against real DNS on quark: Net::DNS's query() returns undef for both NXDOMAIN and NOERROR/NODATA (a valid domain with no matching record), but the code only recognized the errorstring "NXDOMAIN" before trying the wildcard fallback -- anything else, including the literal string "NOERROR", fell into the fail-open branch meant for genuinely transient conditions (SERVFAIL, timeout). Confirmed live: a nonexistent record under isc.org (NXDOMAIN) was correctly rejected, but the same nonexistent record under Cloudflare-hosted example.net (NOERROR/NODATA) was incorrectly authorized. NOERROR now gets the same wildcard-fallback-then-reject treatment as NXDOMAIN. Verified: full build + 12/12 libopendmarc test suite on quark (real libspf2), including new opendmarc_policy_fetch_psd() coverage. Perl syntax clean on the generated opendmarc-reports. verify_external_destination() exercised directly against live DNS for same-org, NXDOMAIN, and NODATA cases. Rate-limiting and the full send path are not tested against a live DB, per the same restriction as the RFC 9990 work -- quark's only opendmarc database is live production. --- CHANGES-202605.md | 26 ++- contrib/opendmarc-reports.conf.sample | 11 ++ db/schema.mysql | 11 ++ db/update-db-schema.mysql | 11 ++ libopendmarc/dmarc.h.in | 1 + libopendmarc/opendmarc_policy.c | 15 ++ libopendmarc/tests/test_dmarc_fetch.c | 21 ++- opendmarc/opendmarc.c | 84 ++++++++++ reports/opendmarc-reports.8.in | 72 +++++++++ reports/opendmarc-reports.in | 218 +++++++++++++++++++++++++- 10 files changed, 465 insertions(+), 5 deletions(-) diff --git a/CHANGES-202605.md b/CHANGES-202605.md index 88bc259..c9bffd8 100644 --- a/CHANGES-202605.md +++ b/CHANGES-202605.md @@ -61,6 +61,16 @@ Significant gaps between the generated aggregate report XML and RFC 7489 require --- +## Failure reports (RFC 9991 compliance) + +- **`Identity-Alignment` and `DKIM-Domain`/`-Identity`/`-Selector` ARF header fields**: The milter-generated DMARC failure report now includes RFC 9991 §4's `Identity-Alignment` field (a comma-separated `dkim`/`spf` list of mechanisms that failed to produce an *aligned* identity, or `none`), and `DKIM-Domain`/`DKIM-Identity`/`DKIM-Selector` (the `d=`/`i=`/`s=` of the DKIM signature involved) when DKIM alignment failed and a signature was actually seen via a trusted upstream `Authentication-Results` header (this codebase never verifies DKIM locally, so these are omitted when no such header carried a DKIM result). RFC 6591 allows only one appearance of each `DKIM-*` field; when multiple signatures are present, the most recently seen one is what's reported. `DKIM-Identity` is further omitted when the signature had no `i=` tag, since it's optional in DKIM itself. +- **External destination verification extended to `ruf=`**: The `_report._dmarc` DNS verification described above now also applies to failure-report (`ruf=`) recipients in `opendmarc-reports --forensic`, per RFC 9991 §5 (which reuses RFC 9990 §4's procedure verbatim, substituting `ruf` for `rua`). Verified against the `Reported-Domain` header the milter already attaches to forensic reports; skipped (not enforced) when that header is absent. As with the other `--forensic`-only checks below, this has no effect when `ReportCommand` pipes directly to `sendmail` rather than through `opendmarc-reports`. +- **Rate-limiting for outgoing failure reports**: New `ForensicRateLimit`/`ForensicRateWindow` options (`opendmarc-reports.conf` or `--forensic-rate-limit`/`--forensic-rate-window`) cap how many failure reports `opendmarc-reports --forensic` will send to a given recipient address within a rolling window, per RFC 9991 §2/§8.1's requirement to prevent a bad actor from using deliberately-triggered DMARC failures to flood a `ruf=` destination. Disabled by default. Backed by a new `forensic_sent` table (`db/update-db-schema.mysql`). +- **`psd=y` now excludes `ruf=`**: Per RFC 9991 §2, `ruf=` in a DMARC Policy Record with `psd=y` is no longer honored (no agreement mechanism exists here to opt back in, per the RFC's own carve-out). Operator-configured `AuthFailureReportsBcc`-style delivery is unaffected, since it doesn't come from the record. New `opendmarc_policy_fetch_psd()` libopendmarc accessor. +- **Deferred**: `SPF-DNS` (would require capturing every DNS record consulted during SPF evaluation, including `include:`/`redirect=` sub-queries — not exposed by libspf2's public API, unlike OpenDMARC's own unused-in-production built-in SPF evaluator, which already tracks this internally) and `DKIM-Canonicalized-Header`/`-Body` (no DKIM verification library is linked anywhere in this codebase, so there are no canonicalized bytes to expose) are both out of scope here — real new work, not wiring, tracked as follow-ups. + +--- + ## Failure reporting enhancements - **Consistent Subject line for milter-generated failure reports**: The Subject header of failure reports sent directly by the milter previously used a conditional format — forwarding the original message's Subject prefixed with `FW:` when present, or falling back to `"DMARC failure report for job "`. It now always uses `"DMARC failure report for received from "`, falling back to the sender IP when rDNS is unavailable. (#391; written by Juri Haberland, submitted by Dirk Stöcker) @@ -85,7 +95,7 @@ Significant gaps between the generated aggregate report XML and RFC 7489 require - **UTC timezone enforced in `opendmarc-expire` and `db/schema.mysql`**: `opendmarc-expire` now issues `SET TIME_ZONE='+00:00'` immediately after connecting to the database, ensuring timestamp arithmetic is consistent regardless of the server's local timezone setting. `db/schema.mysql` includes the same statement for new installations. A `db/update-db-schema.mysql` migration script is provided to upgrade existing 1.4.2 databases. (#390; written by Juri Haberland for openSUSE, submitted by Dirk Stöcker) - **`Content-Description` headers added to failure report MIME parts**: The milter-generated AFRF failure report now includes RFC 2183 `Content-Description` headers on each MIME part (`Notification`, `DMARC failure report`, `Failed message headers`), improving compatibility with webmail clients that use these labels to display or filter parts. Covers both the direct `ReportCommand` path and `opendmarc-reports --forensic` (which is a pass-through). (#380; written by Juri Haberland for openSUSE, submitted by Dirk Stöcker) - **Aggregate report attachment filename included full spool path**: The `Content-Disposition` attachment filename used the full filesystem path of the temporary report file (e.g. `_var_db_opendmarc_reporter.example!domain.example!...xml.gz`) instead of the bare filename. `File::Basename` was already imported; `basename()` is now applied. (#406) -- **External RUA destination verification via `_report._dmarc` DNS lookup**: `opendmarc-reports` now implements RFC 9990 §4 / RFC 9991 §5 external destination verification before sending aggregate reports. For each cross-domain `mailto:` RUA address, queries `._report._dmarc.` for a `v=DMARC1` TXT record, with a wildcard fallback. Fails open on SERVFAIL/timeout. Same-org addresses are accepted without a DNS query, using a built-in PSL reader (`load_psl`/`get_org_domain`) that reads the same `public_suffix_list.dat` file the milter uses, falling back to a two-label heuristic when no PSL is configured. Also adds: `!NNNk` size-limit suffix parsing for RUA URIs; a delivery-failure notice when a report exceeds a declared size limit; and `parse_rua_uri()` for unified URI validation. Closes the gap noted in issue #371 — the existing verification in libopendmarc covers only the milter path. (#392; written by Juri Haberland for openSUSE, submitted by Dirk Stöcker) +- **External RUA destination verification via `_report._dmarc` DNS lookup**: `opendmarc-reports` implements RFC 9990 §4 / RFC 9991 §5 external destination verification before sending aggregate reports. For each cross-domain `mailto:` RUA address, queries `._report._dmarc.` for a `v=DMARC1` TXT record, with a wildcard fallback. Fails open on SERVFAIL/timeout. Same-org addresses are accepted without a DNS query, using a built-in PSL reader (`load_psl`/`get_org_domain`) that reads the same `public_suffix_list.dat` file the milter uses, falling back to a two-label heuristic when no PSL is configured. Closes the gap noted in issue #371 — the existing verification in libopendmarc covers only the milter path. (#392; written by Juri Haberland for openSUSE, submitted by Dirk Stöcker. **Note:** this functionality was silently lost ~5 hours after merging, when #394's commit was built from a pre-#392 checkout and overwrote it; restored and extended to `ruf=` as part of the RFC 9991 work below.) Also adds: `!NNNk` size-limit suffix parsing for RUA URIs; a delivery-failure notice when a report exceeds a declared size limit; and `parse_rua_uri()` for unified URI validation. - **`NoReportsList` and suppressions-table checks now honored by `opendmarc-reports --forensic`**: When the milter's `ReportCommand` is pointed at `opendmarc-reports --forensic` instead of `sendmail -t`, the script now applies the same `NoReportsList` and `suppressions`-table checks used for aggregate delivery to its forensic recipients. Previously `--forensic` mode applied only the StaleMARC check, silently sending to addresses listed in `NoReportsList` or the suppressions table. (#424) - **SMTP AUTH and SSL/TLS now apply to `--forensic` report delivery**: `opendmarc-reports --forensic` opened its SMTP connection without applying `--smtp-username`/`--smtp-password`/`--smtp-ssl`/`--smtp-ssl-ca-file`, even though the manpage already documented those settings as applying to "both aggregate and forensic reports." Connection setup is now shared between both modes via one `smtp_connect()` helper, so the two delivery paths can no longer drift apart. (#424) - **Distinct `ReportCommand` exit code for fully-suppressed reports**: When every recipient of a forensic report is suppressed by `NoReportsList`, StaleMARC, or the suppressions table, `opendmarc-reports --forensic` now exits `2` instead of `0`, and the milter logs this at `LOG_INFO` rather than treating it as a `ReportCommand` failure (previously indistinguishable from a normal send in the logs). (#424) @@ -128,6 +138,20 @@ CREATE TABLE IF NOT EXISTS suppressions ( PRIMARY KEY(id), UNIQUE KEY(address) ); + +-- RFC 9990 policy_published extensions (npolicy/testing/discovery_method) +ALTER TABLE requests ADD COLUMN npolicy TINYINT NOT NULL DEFAULT '0' AFTER spolicy; +ALTER TABLE requests ADD COLUMN testing TINYINT NOT NULL DEFAULT '0' AFTER fo; +ALTER TABLE requests ADD COLUMN discovery_method TINYINT NOT NULL DEFAULT '0' AFTER testing; + +-- RFC 9991 S2/S8.1: forensic report rate-limiting +CREATE TABLE IF NOT EXISTS forensic_sent ( + id INT NOT NULL AUTO_INCREMENT, + address VARCHAR(255) NOT NULL, + sent_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY(id), + KEY(address, sent_at) +); ``` --- diff --git a/contrib/opendmarc-reports.conf.sample b/contrib/opendmarc-reports.conf.sample index ff665a9..84cea20 100644 --- a/contrib/opendmarc-reports.conf.sample +++ b/contrib/opendmarc-reports.conf.sample @@ -60,6 +60,17 @@ # attachment) for later review, e.g. RFC-compliance checking. #archive-dir = /var/spool/opendmarc/sent +# --- External destination verification (RFC 9990 S4 / RFC 9991 S5) --- +# Same file used by the milter's PublicSuffixList; used to recognize +# same-organization RUA/RUF destinations without a DNS query. +#public-suffix-list = /etc/opendmarc/public_suffix_list.dat + +# --- Forensic report rate limiting (RFC 9991 S2/S8.1) --- +# Uncomment to cap failure reports per destination address. Unset (no +# limit) by default. +#forensic-rate-limit = 10 +#forensic-rate-window = 3600 + # --- Miscellaneous --- #interval = 86400 #utc = 1 diff --git a/db/schema.mysql b/db/schema.mysql index b34a495..47dcfd7 100644 --- a/db/schema.mysql +++ b/db/schema.mysql @@ -151,5 +151,16 @@ CREATE TABLE IF NOT EXISTS suppressions ( UNIQUE KEY(address) ); +-- RFC 9991 S2/S8.1: tracks outgoing failure (ruf=) report sends per +-- recipient address, so opendmarc-reports --forensic can rate-limit them. +CREATE TABLE IF NOT EXISTS forensic_sent ( + id INT NOT NULL AUTO_INCREMENT, + address VARCHAR(255) NOT NULL, + sent_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + + PRIMARY KEY(id), + KEY(address, sent_at) +); + -- CREATE USER 'opendmarc'@'localhost' IDENTIFIED BY 'changeme'; -- GRANT ALL ON opendmarc.* to 'opendmarc'@'localhost'; diff --git a/db/update-db-schema.mysql b/db/update-db-schema.mysql index c09cc17..685726f 100644 --- a/db/update-db-schema.mysql +++ b/db/update-db-schema.mysql @@ -49,3 +49,14 @@ CREATE TABLE IF NOT EXISTS suppressions ( PRIMARY KEY(id), UNIQUE KEY(address) ); + +-- RFC 9991 S2/S8.1: tracks outgoing failure (ruf=) report sends per +-- recipient address, so opendmarc-reports --forensic can rate-limit them. +CREATE TABLE IF NOT EXISTS forensic_sent ( + id INT NOT NULL AUTO_INCREMENT, + address VARCHAR(255) NOT NULL, + sent_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + + PRIMARY KEY(id), + KEY(address, sent_at) +); diff --git a/libopendmarc/dmarc.h.in b/libopendmarc/dmarc.h.in index 2181f5a..017dee2 100644 --- a/libopendmarc/dmarc.h.in +++ b/libopendmarc/dmarc.h.in @@ -174,6 +174,7 @@ OPENDMARC_STATUS_T opendmarc_policy_fetch_aspf(DMARC_POLICY_T *pctx, int *aspf); OPENDMARC_STATUS_T opendmarc_policy_fetch_p(DMARC_POLICY_T *pctx, int *p); OPENDMARC_STATUS_T opendmarc_policy_fetch_sp(DMARC_POLICY_T *pctx, int *sp); OPENDMARC_STATUS_T opendmarc_policy_fetch_np(DMARC_POLICY_T *pctx, int *np); +OPENDMARC_STATUS_T opendmarc_policy_fetch_psd(DMARC_POLICY_T *pctx, int *psd); OPENDMARC_STATUS_T opendmarc_policy_fetch_t(DMARC_POLICY_T *pctx, int *t); OPENDMARC_STATUS_T opendmarc_policy_fetch_discovery_method(DMARC_POLICY_T *pctx, int *discovery_method); OPENDMARC_STATUS_T opendmarc_policy_fetch_fo(DMARC_POLICY_T *pctx, int *fo); diff --git a/libopendmarc/opendmarc_policy.c b/libopendmarc/opendmarc_policy.c index 51b4856..f643f84 100644 --- a/libopendmarc/opendmarc_policy.c +++ b/libopendmarc/opendmarc_policy.c @@ -1850,6 +1850,21 @@ opendmarc_policy_fetch_np(DMARC_POLICY_T *pctx, int *np) return DMARC_PARSE_OKAY; } +OPENDMARC_STATUS_T +opendmarc_policy_fetch_psd(DMARC_POLICY_T *pctx, int *psd) +{ + if (pctx == NULL) + { + return DMARC_PARSE_ERROR_NULL_CTX; + } + if (psd == NULL) + { + return DMARC_PARSE_ERROR_EMPTY; + } + *psd = pctx->psd; + return DMARC_PARSE_OKAY; +} + OPENDMARC_STATUS_T opendmarc_policy_fetch_discovery_method(DMARC_POLICY_T *pctx, int *discovery_method) { diff --git a/libopendmarc/tests/test_dmarc_fetch.c b/libopendmarc/tests/test_dmarc_fetch.c index 083a930..82261cb 100644 --- a/libopendmarc/tests/test_dmarc_fetch.c +++ b/libopendmarc/tests/test_dmarc_fetch.c @@ -6,7 +6,7 @@ int main(int argc, char **argv) { - static char *record= "v=DMARC1; p=none; sp=none; np=quarantine; adkim=s; aspf=s; pct=50; t=y; ri=300; rf=afrf; rua=mailto:dmarc-a@abuse.net; ruf=mailto:dmarc-f@abuse.net"; + static char *record= "v=DMARC1; p=none; sp=none; np=quarantine; psd=y; adkim=s; aspf=s; pct=50; t=y; ri=300; rf=afrf; rua=mailto:dmarc-a@abuse.net; ruf=mailto:dmarc-f@abuse.net"; DMARC_POLICY_T *pctx; OPENDMARC_STATUS_T status; int pass, fails, count; @@ -15,6 +15,7 @@ main(int argc, char **argv) int aspf; int t; int np; + int psd; int discovery_method; pass = fails = count = 0; @@ -85,6 +86,17 @@ main(int argc, char **argv) printf("\t%s(%d): opendmarc_policy_fetch_np: expected %d got %d: FAIL\n", __FILE__, __LINE__, DMARC_RECORD_P_QUARANTINE, np); fails += 1; } + status = opendmarc_policy_fetch_psd(pctx, &psd); + if (status != DMARC_PARSE_OKAY) + { + printf("\t%s(%d): opendmarc_policy_fetch_psd: %s: FAIL\n", __FILE__, __LINE__, opendmarc_policy_status_to_str(status)); + fails += 1; + } + if (psd != DMARC_RECORD_PSD_Y) + { + printf("\t%s(%d): opendmarc_policy_fetch_psd: expected %d got %d: FAIL\n", __FILE__, __LINE__, DMARC_RECORD_PSD_Y, psd); + fails += 1; + } /* ** RFC 9990: discovery_method is only ever set by the walk-mode query @@ -190,6 +202,13 @@ main(int argc, char **argv) printf("\t%s(%d): opendmarc_policy_fetch_np: expected unspecified, got %d: FAIL\n", __FILE__, __LINE__, np); fails += 1; } + /* RFC 9989: psd= absent defaults to DMARC_RECORD_PSD_UNSPECIFIED */ + status = opendmarc_policy_fetch_psd(pctx, &psd); + if (status != DMARC_PARSE_OKAY || psd != DMARC_RECORD_PSD_UNSPECIFIED) + { + printf("\t%s(%d): opendmarc_policy_fetch_psd: expected unspecified, got %d: FAIL\n", __FILE__, __LINE__, psd); + fails += 1; + } pctx = opendmarc_policy_connect_shutdown(pctx); return fails; diff --git a/opendmarc/opendmarc.c b/opendmarc/opendmarc.c index 6d466d7..b4cdee1 100644 --- a/opendmarc/opendmarc.c +++ b/opendmarc/opendmarc.c @@ -129,6 +129,9 @@ struct dmarcf_msgctx int mctx_arcpolicypass; int mctx_spfresult; int mctx_spfmode; + u_char * mctx_dkimdomain; /* RFC 9991: d= of the last DKIM signature seen; not owned, points into "ar" */ + u_char * mctx_dkimselector; /* RFC 9991: s= of same */ + u_char * mctx_dkimidentity; /* RFC 9991: i= of same, if present */ char * mctx_jobid; char ** mctx_arcchain; struct arcares_header * mctx_aarhead; @@ -2286,6 +2289,7 @@ mlfi_eom(SMFICTX *ctx) int np; int t; int discovery_method; + int psd; int align_dkim; int align_spf; int limit_arc = 0; @@ -2899,6 +2903,7 @@ mlfi_eom(SMFICTX *ctx) { u_char *dkim_selector = NULL; u_char *dkim_domain = NULL; + u_char *dkim_identity = NULL; for (pc = 0; pc < ar->ares_result[c].result_props; @@ -2914,12 +2919,28 @@ mlfi_eom(SMFICTX *ctx) { dkim_selector = ar->ares_result[c].result_value[pc]; } + if (ar->ares_result[c].result_property[pc][0] == 'i') + { + dkim_identity = ar->ares_result[c].result_value[pc]; + } } } if (dkim_domain == NULL) continue; + /* + ** RFC 9991 S4: DKIM-Domain/-Identity/-Selector for the + ** failure report. These fields allow only one appearance + ** each, so when multiple signatures are present, the last + ** one seen here is what gets reported if DKIM alignment + ** ultimately fails. Not owned -- points into "ar", which + ** outlives this function's use of it (freed at "done:"). + */ + dfc->mctx_dkimdomain = dkim_domain; + dfc->mctx_dkimselector = dkim_selector; + dfc->mctx_dkimidentity = dkim_identity; + dmarcf_dstring_printf(dfc->mctx_histbuf, "dkim %s %s %d\n", dkim_domain, @@ -3531,6 +3552,21 @@ mlfi_eom(SMFICTX *ctx) */ ruv = opendmarc_policy_fetch_ruf(cc->cctx_dmarc, NULL, 0, TRUE); + + /* + ** RFC 9991 S2: "Report generators MUST NOT consider 'ruf' tags in + ** DMARC Policy Records that have a 'psd=y' tag, unless there are + ** specific agreements between the interested parties." No such + ** agreement mechanism exists here, so psd=y always drops ruf=-derived + ** recipients. conf_afrfbcc is operator-configured, not sourced from + ** the record, so it's unaffected. + */ + if (opendmarc_policy_fetch_psd(cc->cctx_dmarc, &psd) == DMARC_PARSE_OKAY && + psd == DMARC_RECORD_PSD_Y) + { + ruv = NULL; + } + if ((policy == DMARC_POLICY_REJECT || policy == DMARC_POLICY_QUARANTINE || (conf->conf_afrfnone && policy == DMARC_POLICY_NONE)) && @@ -3689,6 +3725,54 @@ mlfi_eom(SMFICTX *ctx) dmarcf_dstring_cat(dfc->mctx_afrf, (u_char *)"Auth-Failure: dmarc\n"); + /* + ** RFC 9991 S4: Identity-Alignment lists the mechanisms that + ** failed to produce an *aligned* identity (not merely a + ** mechanism failure), or "none" if both aligned. DKIM-Domain/ + ** -Identity/-Selector cite the DKIM signature involved, only + ** when DKIM alignment failed and a signature was actually + ** seen (mctx_dkimdomain is NULL when no trusted upstream + ** Authentication-Results header carried a DKIM result at + ** all -- this codebase never verifies DKIM locally). + */ + { + _Bool dkim_failed = (align_dkim != DMARC_POLICY_DKIM_ALIGNMENT_PASS); + _Bool spf_failed = (align_spf != DMARC_POLICY_SPF_ALIGNMENT_PASS); + + if (!dkim_failed && !spf_failed) + { + dmarcf_dstring_cat(dfc->mctx_afrf, + (u_char *)"Identity-Alignment: none\n"); + } + else + { + dmarcf_dstring_printf(dfc->mctx_afrf, + "Identity-Alignment: %s%s%s\n", + dkim_failed ? "dkim" : "", + (dkim_failed && spf_failed) ? "," : "", + spf_failed ? "spf" : ""); + } + + if (dkim_failed && dfc->mctx_dkimdomain != NULL) + { + dmarcf_dstring_printf(dfc->mctx_afrf, + "DKIM-Domain: %s\n", + dfc->mctx_dkimdomain); + if (dfc->mctx_dkimselector != NULL) + { + dmarcf_dstring_printf(dfc->mctx_afrf, + "DKIM-Selector: %s\n", + dfc->mctx_dkimselector); + } + if (dfc->mctx_dkimidentity != NULL) + { + dmarcf_dstring_printf(dfc->mctx_afrf, + "DKIM-Identity: %s\n", + dfc->mctx_dkimidentity); + } + } + } + dmarcf_dstring_printf(dfc->mctx_afrf, "Authentication-Results: %s; dmarc=fail header.from=%s\n", authservid, diff --git a/reports/opendmarc-reports.8.in b/reports/opendmarc-reports.8.in index da17c5b..3252f91 100644 --- a/reports/opendmarc-reports.8.in +++ b/reports/opendmarc-reports.8.in @@ -232,6 +232,35 @@ Causes reports to be sent by transmitting them using SMTP to the specified Defaults to the value of the environment variable OPENDMARC_SMTPPORT, or 25 if the environment variable is not set. .TP +.I --public-suffix-list=file +Path to a Public Suffix List file, used to determine the organizational +domain of the sending and destination domains when verifying external +RUA/RUF destinations (see +.B EXTERNAL DESTINATION VERIFICATION +below). Falls back to a two-label heuristic if not set. Same file format +as the milter's +.I PublicSuffixList +option +.RI ( opendmarc.conf (5)). +.TP +.I --forensic-rate-limit=n +Limit failure reports sent by +.I --forensic +mode to at most +.I n +per destination address within +.IR --forensic-rate-window . +Unset (no limit) by default. See +.B RATE LIMITING +below. +.TP +.I --forensic-rate-window=secs +The rolling window, in seconds, over which +.I --forensic-rate-limit +is counted. Defaults to 3600 (one hour). Has no effect unless +.I --forensic-rate-limit +is also set. +.TP .I --stale-check Before sending each report, query the StaleMARC RBL to determine whether the target domain or RUA/RUF address has known delivery problems. Reports are @@ -345,6 +374,49 @@ is enabled, DMARC report bounces return to an encoded envelope sender that identifies the failing address. A bounce-processing script can decode that address and insert it into this table automatically, building a locally-maintained suppression list informed by real delivery failures. +.SH EXTERNAL DESTINATION VERIFICATION +Before sending to a cross-domain +.RI ( mailto: ) +RUA or RUF address, the script verifies the destination is willing to +receive reports from the sending domain, per RFC 9990 section 4 (aggregate) +and RFC 9991 section 5 (failure, which reuses the same procedure). +Addresses in the same organizational domain as the sender (per +.I --public-suffix-list +or the two-label fallback) are accepted without a DNS query. Otherwise the +script queries +.I ._report._dmarc. +for a +.I v=DMARC1 +TXT record, falling back to the wildcard form +.RI ( *._report._dmarc. ) +on NXDOMAIN. The destination is rejected, and the report for it skipped, +if neither query returns an authorizing record. Lookup failures +(SERVFAIL, timeout) fail open. For +.I --forensic +mode, the sending domain is taken from the +.I Reported-Domain +field described above; verification is skipped (not enforced) when that +field is absent. Applies to both aggregate and forensic modes. +.SH RATE LIMITING +When +.I --forensic-rate-limit +is set, +.I --forensic +mode will send at most that many failure reports to a given destination +address within +.I --forensic-rate-window +seconds, per RFC 9991 section 2 and section 8.1 (preventing a bad actor +from using deliberately-triggered DMARC failures to flood a RUF +destination). Send attempts, not confirmed deliveries, count against the +limit. Backed by the +.I forensic_sent +database table; see +.B "SCHEMA CHANGES" +in the top-level CHANGES file if upgrading from an installation predating +this option. Disabled (no limit) by default. Has no effect on aggregate +report delivery or on the milter's direct +.I ReportCommand +path. .SH ENVIRONMENT .TP .B OPENDMARC_DBHOST diff --git a/reports/opendmarc-reports.in b/reports/opendmarc-reports.in index 5936f63..4cc1189 100755 --- a/reports/opendmarc-reports.in +++ b/reports/opendmarc-reports.in @@ -22,6 +22,7 @@ use IO::Handle; use IO::Compress::Gzip qw(gzip); use POSIX; use MIME::Base64; +use Net::DNS; use Net::SMTP; use Sys::Syslog qw(:standard :macros); use Time::Local; @@ -137,6 +138,17 @@ my $report_maxbytes_global = 15728640; # default: 15M, per spec my $noreports_file; my %noreports; +# RFC 9990 S4 / RFC 9991 S5: external-destination verification. +my $publicsuffixlist; +my %psl; +my $psl_loaded = 0; + +# RFC 9991 S2/S8.1: rate-limit on outgoing failure reports. Disabled +# (undef) by default -- new, potentially traffic-shaping behavior should +# not turn on silently. +my $forensic_rate_limit; +my $forensic_rate_window = 3600; + my $msgid; my $rowcount; @@ -279,7 +291,8 @@ sub loud_skip { my ($reason) = @_; - return $reason eq "StaleMARC" || $reason eq "suppression DB"; + return $reason eq "StaleMARC" || $reason eq "suppression DB" || + $reason eq "rate limit"; } # Prints a report-skip diagnostic to stderr, gated by --verbose except for @@ -339,6 +352,171 @@ sub domain_skip_reason return undef; } +# Load the Public Suffix List from disk into %psl. Called once on first use. +sub load_psl +{ + return if $psl_loaded; + $psl_loaded = 1; + return unless defined($publicsuffixlist); + open(my $fh, '<', $publicsuffixlist) or return; + while (my $line = <$fh>) + { + chomp $line; + $line =~ s/\s*\/\/.*//; + next if $line =~ /^\s*$/; + $psl{lc($line)} = 1; + } + close($fh); +} + +# Return the organizational domain (eTLD+1) of $domain. +# Uses the PSL when available; falls back to the last two labels otherwise. +sub get_org_domain +{ + my ($domain) = @_; + $domain = lc($domain); + my @labels = split(/\./, $domain); + + if (defined($publicsuffixlist) && -r $publicsuffixlist) + { + load_psl(); + for my $i (0 .. $#labels - 1) + { + my $suffix = join('.', @labels[$i+1 .. $#labels]); + my $wildcard = '*.' . $suffix; + my $exception = '!' . join('.', @labels[$i .. $#labels]); + if (exists $psl{$exception}) + { + return join('.', @labels[$i .. $#labels]); + } + if (exists $psl{join('.', @labels[$i .. $#labels])} || + exists $psl{$wildcard}) + { + return $i > 0 ? join('.', @labels[$i-1 .. $#labels]) : undef; + } + } + } + + # Fallback: last two labels. + return @labels >= 2 ? join('.', @labels[-2 .. $#labels]) : $domain; +} + +# RFC 9990 S4 / RFC 9991 S5 "Verifying External Destinations". $domain is +# the DMARC-Policy-Record-owning (sending) domain; $address is a rua= or +# ruf= mailto: destination address; $tag is "rua" or "ruf", used only for +# log messages -- the verification procedure itself (same-org check, then +# the "._report._dmarc." DNS query) is identical for +# both per RFC 9991 S5, which just substitutes "ruf" for "rua" in RFC 9990 +# S4's procedure. Same-org addresses are authorized without a DNS query. +# Fails open (authorizes) on SERVFAIL/timeout/other non-definitive answers, +# per the same "don't let report delivery hinge on transient DNS trouble" +# reasoning as the rest of this script's suppression checks. +sub verify_external_destination +{ + my ($domain, $address, $tag) = @_; + + my ($dest_domain) = $address =~ /\@(.+)$/; + $dest_domain = lc($dest_domain // ''); + return 0 unless length($dest_domain); + + my $domain_org = get_org_domain($domain); + my $dest_org = get_org_domain($dest_domain); + + if (defined($domain_org) && defined($dest_org) && $domain_org eq $dest_org) + { + # Same organizational domain -- no DNS check required. + return 1; + } + + my $res = Net::DNS::Resolver->new(udp_timeout => 15); + my $qname = "$domain._report._dmarc.$dest_domain"; + my $reply = $res->query($qname, "TXT"); + my $authorized = 0; + + if ($reply) + { + for my $rr ($reply->answer) + { + next unless $rr->type eq "TXT"; + if ($rr->txtdata =~ /^\s*v\s*=\s*DMARC1\b/i) + { + $authorized = 1; + last; + } + } + } + else + { + my $err = $res->errorstring; + + # NXDOMAIN (name doesn't exist) and NOERROR (name resolves but has + # no TXT records -- NODATA) are both definitive negative answers, + # not failures: Net::DNS returns undef for either, distinguishable + # only via errorstring. A NODATA response is exactly as common in + # practice as a real NXDOMAIN for a nonexistent subdomain -- e.g. + # Cloudflare-hosted zones answer this way -- so treating it as + # "fail open" would make verification silently weaker against any + # nameserver that responds this way, rather than failing open only + # for genuinely indeterminate conditions (SERVFAIL, timeout). + if ($err eq "NXDOMAIN" || $err eq "NOERROR") + { + # Wildcard fallback: *._report._dmarc. + my $wreply = $res->query("*._report._dmarc.$dest_domain", "TXT"); + if ($wreply) + { + for my $rr ($wreply->answer) + { + next unless $rr->type eq "TXT"; + if ($rr->txtdata =~ /^\s*v\s*=\s*DMARC1\b/i) + { + $authorized = 1; + last; + } + } + } + } + else + { + # SERVFAIL, timeout, or anything else non-definitive: fail open. + $authorized = 1; + } + } + + if (!$authorized && $verbose) + { + print STDERR "$progname: $domain is not authorized to send $tag reports to $address ($qname returned " . $res->errorstring . "); dropping\n"; + } + + return $authorized; +} + +# RFC 9991 S2/S8.1: rate-limit outgoing failure reports per recipient +# address, so a bad actor can't use deliberately-triggered DMARC failures +# to flood a ruf= destination. Counts (and records) attempts, not confirmed +# deliveries -- an attempt still represents outbound traffic/load toward +# the destination, which is what S8.1 is protecting against. Returns 1 +# (send, and records this send) if under the configured limit, 0 (skip) if +# at or over it. Always returns 1 without touching the DB when rate- +# limiting isn't configured. +sub forensic_rate_check +{ + my ($address) = @_; + + return 1 unless defined($forensic_rate_limit); + + my $sth = $dbi_h->prepare( + "SELECT COUNT(*) FROM forensic_sent WHERE address = ? AND sent_at > FROM_UNIXTIME(?)"); + $sth->execute($address, time() - $forensic_rate_window); + my ($count) = $sth->fetchrow_array(); + $sth->finish; + + return 0 if $count >= $forensic_rate_limit; + + $dbi_h->do("INSERT INTO forensic_sent (address) VALUES (?)", undef, $address); + + return 1; +} + # Open a connection to the configured SMTP server, applying SSL/StartTLS # and SMTP AUTH settings if configured. Shared by both aggregate and # --forensic sending so those settings apply no matter which is delivering @@ -555,6 +733,12 @@ my $opt_retval = &Getopt::Long::GetOptions ('archive-dir=s' => \$archive_dir, 'smtp-username=s' => \$smtp_username, 'no-reports-list=s' => \$noreports_file, 'NoReportsList=s' => \$noreports_file, + 'public-suffix-list=s' => \$publicsuffixlist, + 'PublicSuffixList=s' => \$publicsuffixlist, + 'forensic-rate-limit=i' => \$forensic_rate_limit, + 'ForensicRateLimit=i' => \$forensic_rate_limit, + 'forensic-rate-window=i' => \$forensic_rate_window, + 'ForensicRateWindow=i' => \$forensic_rate_window, 'stale-check!' => \$stale_check, 'stale-def-deny!' => \$stale_def_deny, 'stale-server=s' => \$stale_server, @@ -724,10 +908,15 @@ if ($forensic) # sending-domain suppression -- --skipdomains/bare NoReportsList # entries, StaleMARC, the suppressions DB -- that the aggregate path # applies, so a bare NoReportsList entry suppresses reports about - # that domain no matter which path would otherwise send them. + # that domain no matter which path would otherwise send them. Also + # used below as the "domain" half of external-destination verification + # (RFC 9990 S4 / RFC 9991 S5) -- without it we have no sending domain + # to verify recipients against, so verification is skipped rather than + # guessed at. + my $reported_domain; if ($message =~ /^Reported-Domain:\s*(\S+)/mi) { - my $reported_domain = lc($1); + $reported_domain = lc($1); my $reason = domain_skip_reason($reported_domain); if (defined($reason)) { @@ -774,6 +963,24 @@ if ($forensic) !defined($reason); } @rcpts; + # RFC 9991 S5 external-destination verification, substituting ruf for + # rua per that section. Skipped (not enforced) when there's no + # Reported-Domain to verify against -- see the comment above where + # $reported_domain is set. + if (defined($reported_domain)) + { + @rcpts = grep { verify_external_destination($reported_domain, $_, "ruf") } @rcpts; + } + + # RFC 9991 S2/S8.1 rate-limiting. Deliberately last, after the (cheap) + # suppression/verification checks, so a recipient that would be + # dropped anyway doesn't consume a slot in its own rate-limit window. + @rcpts = grep { + my $ok = forensic_rate_check($_); + log_skip("rate limit", "--forensic: skipping $_ (forensic rate limit exceeded)") unless $ok; + $ok; + } @rcpts; + if (!@rcpts) { print STDERR "$progname: --forensic: all recipients suppressed\n"; @@ -1712,6 +1919,11 @@ foreach (@$domainset) log_skip($reason, "skipping $domain report to $repdest ($reason)"); next; } + + if (!verify_external_destination($domain, $repdest, "rua")) + { + next; + } } # Test mode, just report what would have been done From 331d108d83689c3da6da1c4260f4b78ffa36ae3c Mon Sep 17 00:00:00 2001 From: Dan Mahoney Date: Mon, 20 Jul 2026 02:16:09 -0700 Subject: [PATCH 5/8] docs: add DMARCbis remaining-work tracker Consolidates what's left across RFC 9989/9990/9991 into one file, since issue #371's gap analysis predates most of the work done on this branch and its sibling (feat/rfc9990-aggregate-reporting) and doesn't get rewritten as items close. Covers items issue #371 already flagged that we haven't touched (pass disposition value, policy_test_mode reason, generator/error elements, DKIM signature priority/cap, rf=/ri= cleanup), plus SPF-DNS and DKIM-Canonicalized-Header/-Body, which are finer-grained than that issue's analysis and were found while scoping the RFC 9991 work. Also records the cross-project (OpenDKIM + OpenDMARC) design worked out for DKIM-Canonicalized-Header/-Body: OpenDKIM emits the canonicalized data it already computes as extra headers, opt-in via an OpenDKIM-side config toggle so it costs nothing for deployments that don't send failure reports, and OpenDMARC strips those headers at eom before final delivery so nothing leaks to recipients. Avoids building a redundant local DKIM verifier into OpenDMARC just for this one field. --- DMARCBIS-REMAINING-WORK.md | 129 +++++++++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 DMARCBIS-REMAINING-WORK.md diff --git a/DMARCBIS-REMAINING-WORK.md b/DMARCBIS-REMAINING-WORK.md new file mode 100644 index 0000000..ff38c32 --- /dev/null +++ b/DMARCBIS-REMAINING-WORK.md @@ -0,0 +1,129 @@ +# DMARCbis remaining work + +Tracks what's left across RFC 9989 (DMARC core), RFC 9990 (aggregate +reporting), and RFC 9991 (failure reporting) — the documents that obsolete +RFC 7489. Source of truth for the overall gap analysis is GitHub issue +[#371](https://github.com/trusteddomainproject/OpenDMARC/issues/371) +(`trusteddomainproject/OpenDMARC`); this file exists because that issue +predates most of the work below and doesn't get rewritten as items close. +Copies of the RFCs themselves are in the repo root (`rfc7489.txt`, +`rfc9989.txt`, `rfc9990.txt`, `rfc9991.txt`). `DMARCBIS-WALK-NOTES.txt` and +`DMARCBIS-EDITOR-EMAIL.txt` cover the DNS Tree Walk design questions +specifically (all resolved) and are not duplicated here. + +## Done + +- **RFC 9989 DNS Tree Walk**: walk-mode selection (PSL/RFC7489/RFC9989/AUTO), + configurable fallback, secondary alignment walk, `opendmarc-check` + comparison tooling. All open questions in `DMARCBIS-WALK-NOTES.txt` + resolved. (#430, #431, #433) +- **RFC 9989 `t=`/`pct=`**: `t=` parsing, fetch accessor, and enforcement + step-down (reject->quarantine->none). `pct=` deliberately kept for POLA, + with `DMARCbisIgnorePct` for operators who want strict compliance. (#434) +- **RFC 9990 aggregate reporting**: `np`/`testing`/`discovery_method` in + `policy_published`, namespace bumped to `dmarc-2.0`, `` removed + (branch `feat/rfc9990-aggregate-reporting`). +- **RFC 9991 failure reporting**: `Identity-Alignment` and + `DKIM-Domain`/`-Identity`/`-Selector` ARF headers, `ruf=` external + destination verification + rate-limiting in `opendmarc-reports + --forensic`, `psd=y` excludes `ruf=` (branch + `feat/rfc9991-forensic-reporting`). Also restored PR #392's `rua=` + destination verification, which had been silently lost, and fixed a + NOERROR-vs-NXDOMAIN bug in it found via live testing. + +## Remaining + +### RFC 9990 aggregate reporting + +- **`pass` disposition value** (S3.1.1.9): `ActionDispositionType` now + includes `pass` (message passed DMARC under an *enforcing* policy) + alongside `none`/`quarantine`/`reject`. Current code only ever emits the + original three. Not touched by the `np`/`testing`/`discovery_method` work. +- **`policy_test_mode` reason type** (S3.1.6): a `` value a + report record should carry when `t=y` caused a policy step-down. Directly + adjacent to work already done — `t=` enforcement and `` in + `policy_published` both shipped, but this per-record annotation didn't. +- **`generator` element** (S3.1.1.3): identifies the report-generating + software. Not implemented in `opendmarc-reports`. (Unrelated: the + separate `contrib/dmarc-report-totext.pl` *consumer* tool already parses + this field from other senders' reports — that's reading, not writing.) +- **`error` element** (S3.1.1.3/S3.1.5): describes processing errors + encountered while evaluating the DMARC Policy Record. Not implemented. +- **DKIM signature priority + 100-signature cap** (S3.1.3): defines which + signatures to include when a message has several (strict-aligned pass + first, then relaxed, then others) and caps the list. Not implemented as + specified. +- **Extension mechanism** (S3.2, S5): `` at file level, + namespaced elements at record level. Low priority — only matters if + extensions are actually adopted by report consumers. +- **`rf=`/`ri=` cleanup**: both tags were removed from the DMARC record + format by RFC 9989. OpenDMARC still parses them into unused + `DMARC_POLICY_T` fields (`rf`, `ri`). Minor; safe to remove. + +### RFC 9991 failure reporting + +- **`SPF-DNS` ARF field** (RFC 6591, required per-record-consulted): + needs the actual DNS record content (RRTYPE + domain + record text) for + every record consulted during SPF evaluation, including `include:`/ + `redirect=` sub-queries. libspf2's public API (what's actually linked in + production, confirmed via `spf_response.h` on quark) doesn't expose this + chain — only a human-readable summary. OpenDMARC's own built-in fallback + SPF evaluator (`opendmarc_spf.c`, used only when libspf2 isn't linked) + already tracks it internally via `stack[s].spf`/`stack[s].domain`, but + that's not the path in use. Real fix: a custom `SPF_dns_*`-compatible + logging layer chained onto the real resolver, so queries can be captured + without patching libspf2 itself. Self-contained but real new code. +- **`DKIM-Canonicalized-Header`/`-Body` ARF fields** (RFC 6591): need the + raw canonicalized header/body bytes as generated by the verifier. + Confirmed no DKIM verification library is linked anywhere in this + codebase (no `libdkim`, no `DKIM_STAT_*`) — DKIM results are only ever + sourced from a trusted upstream `Authentication-Results` header, never + verified locally, so there are no canonicalized bytes anywhere to expose. + A full local DKIM-verification subsystem in OpenDMARC would get this data + but duplicates work OpenDKIM already does. + + **Preferred approach, cross-project (OpenDKIM + OpenDMARC), designed in + discussion 2026-07-20**: have OpenDKIM emit the canonicalized header/body + it already computes (a necessary byproduct of checking the signature + hash) as extra headers alongside its normal `Authentication-Results` + header -- e.g. `X-DKIM-Canonicalized-Header`/`-Body`, base64-encoded. + OpenDMARC reads them the same way it already reads the trusted upstream + A-R header (`mlfi_header()`, gated by `conf_trustedauthservids`, no new + trust boundary). Three design points that make this workable rather than + just clever: + + 1. **Opt-in on the OpenDKIM side**: a config toggle (e.g. + `AddCanonicalizedData yes`), off by default, that an operator enables + only if they've also enabled `AuthFailureReports` in OpenDMARC. + Deployments that don't send failure reports -- per RFC 9989's own + text, most Mail Receivers don't -- pay nothing. + 2. **Necessarily unconditional per message, once enabled**: OpenDKIM + can't know in advance which messages will later fail DMARC alignment + in OpenDMARC (that determination happens downstream), so while the + toggle is on, every DKIM-verified message carries the extra headers + through the milter chain. The savings is at the "do we want this + feature at all" level, not per-message. + 3. **OpenDMARC strips the headers before final delivery**: at `eom`, + after reading them (whether or not this particular message ends up + needing a failure report), `smfi_chgheader(ctx, header, idx, NULL)` + removes them -- the standard libmilter delete-by-NULL idiom, not + currently used anywhere in this codebase but well-established. The + canonicalized data exists only transiently between OpenDKIM and + OpenDMARC in the chain; it never reaches the recipient's mailbox, so + there's no permanent size or privacy cost added to delivered mail. + + Net effect: RFC 9991-complete `DKIM-Canonicalized-Header`/`-Body` support + without OpenDMARC ever verifying DKIM itself, gated so it costs nothing + for the common case of not sending failure reports at all. Requires + filing/scoping the OpenDKIM-side half separately -- out of this repo. + +### Open decision, not just missing code + +- **`!NNNk` RUA size-suffix syntax**: RFC 9990 Appendix C says this syntax + is obsolete and receivers (i.e. OpenDMARC, acting as report generator) + MUST ignore it. The restored PR #392 code (`check_size_restriction`) + actively parses and enforces it, faithfully reproducing pre-regression + behavior rather than the RFC 9990 text. Same shape of question as the + `pct=` POLA decision: keep honoring it for senders who rely on it, or + drop it now that it's back in front of you. Needs an explicit call, not + a default. From 63267015defbf85a8d123c7fefba62d5e0744df3 Mon Sep 17 00:00:00 2001 From: Dan Mahoney Date: Tue, 21 Jul 2026 18:57:14 -0700 Subject: [PATCH 6/8] feat: RFC 9991 DKIM-Canonicalized-Header/-Body via OpenDKIM's AddCanonicalizedData Closes the last item in the RFC 9991 gap: the DKIM-Canonicalized-Header/ -Body ARF fields required on ruf= forensic reports. This codebase never verifies DKIM locally, so it has no canonicalized bytes of its own -- instead, a new opt-in ReadCanonicalizedData option reads X-DKIM-Canonicalized-Header/-Body staging headers added by an upstream OpenDKIM's new AddCanonicalizedData directive (trusteddomainproject/ OpenDKIM PR #423), correlates them to whichever DKIM signature is blamed for a DMARC failure via d=/s=, and copies the base64 payload into the real RFC 6591 fields, re-folded to this codebase's own ARF-body convention rather than passed through verbatim. The staging headers are always stripped before final delivery, whether or not ReadCanonicalizedData is set, so a signing filter with the upstream directive enabled can never leak them to a recipient. Getting this right took two passes: the first version only stripped on the single normal-completion path at the end of mlfi_eom, missing five early "ret = SMFIS_ACCEPT; goto done;" shortcuts elsewhere in the function (malformed/missing From with lenient config, ignored domains, DMARC permerror) that also deliver the message -- moved the logic into a shared dmarcf_strip_canon_headers() helper and call it from all six delivery paths. Also caught and fixed a real bug before it shipped: looping on smfi_chgheader()'s return value to know when to stop deleting repeated header instances would have been an infinite loop in production, since that call is fire-and-forget over the milter socket and its return value only reflects whether the write succeeded, not whether a header existed at that index. Fixed by counting real instances via dmarcf_findheader() first, then issuing exactly that many deletes. Also fixes an unrelated pre-existing bug found via a syntax-only compile check while working on this: opendmarc_policy_fetch_np/_psd/ _discovery_method (from the RFC 9990 work) were defined in opendmarc_policy.c but never prototyped in dmarc.h.in, silently relying on implicit-declaration behavior. New test t-verify-canondata covers the stripping behavior (deliberately without ReadCanonicalizedData set, since stripping must not depend on it). Built and ran the full miltertest suite against real libmilter/ libspf2 on quark: 9/9 pass, including all pre-existing tests (no regressions). --- CHANGES-202605.md | 1 + DMARCBIS-REMAINING-WORK.md | 63 ++---- libopendmarc/dmarc.h.in | 3 + opendmarc/opendmarc.c | 281 ++++++++++++++++++++++++ opendmarc/opendmarc.conf.5.in | 24 ++ opendmarc/opendmarc.conf.sample | 14 ++ opendmarc/tests/Makefile.am | 5 +- opendmarc/tests/t-verify-canondata | 13 ++ opendmarc/tests/t-verify-canondata.conf | 4 + opendmarc/tests/t-verify-canondata.lua | 114 ++++++++++ 10 files changed, 478 insertions(+), 44 deletions(-) create mode 100755 opendmarc/tests/t-verify-canondata create mode 100644 opendmarc/tests/t-verify-canondata.conf create mode 100644 opendmarc/tests/t-verify-canondata.lua diff --git a/CHANGES-202605.md b/CHANGES-202605.md index c9bffd8..115e9c6 100644 --- a/CHANGES-202605.md +++ b/CHANGES-202605.md @@ -67,6 +67,7 @@ Significant gaps between the generated aggregate report XML and RFC 7489 require - **External destination verification extended to `ruf=`**: The `_report._dmarc` DNS verification described above now also applies to failure-report (`ruf=`) recipients in `opendmarc-reports --forensic`, per RFC 9991 §5 (which reuses RFC 9990 §4's procedure verbatim, substituting `ruf` for `rua`). Verified against the `Reported-Domain` header the milter already attaches to forensic reports; skipped (not enforced) when that header is absent. As with the other `--forensic`-only checks below, this has no effect when `ReportCommand` pipes directly to `sendmail` rather than through `opendmarc-reports`. - **Rate-limiting for outgoing failure reports**: New `ForensicRateLimit`/`ForensicRateWindow` options (`opendmarc-reports.conf` or `--forensic-rate-limit`/`--forensic-rate-window`) cap how many failure reports `opendmarc-reports --forensic` will send to a given recipient address within a rolling window, per RFC 9991 §2/§8.1's requirement to prevent a bad actor from using deliberately-triggered DMARC failures to flood a `ruf=` destination. Disabled by default. Backed by a new `forensic_sent` table (`db/update-db-schema.mysql`). - **`psd=y` now excludes `ruf=`**: Per RFC 9991 §2, `ruf=` in a DMARC Policy Record with `psd=y` is no longer honored (no agreement mechanism exists here to opt back in, per the RFC's own carve-out). Operator-configured `AuthFailureReportsBcc`-style delivery is unaffected, since it doesn't come from the record. New `opendmarc_policy_fetch_psd()` libopendmarc accessor. +- **`DKIM-Canonicalized-Header`/`-Body` ARF header fields**: New opt-in `ReadCanonicalizedData` option. This codebase never verifies DKIM locally, so it has no canonicalized bytes of its own; instead, when set (and `FailureReports` is also on), it looks for `X-DKIM-Canonicalized-Header`/`-Body` staging headers added by an upstream signing filter (OpenDKIM's new `AddCanonicalizedData` directive, `trusteddomainproject/OpenDKIM` PR #423) and, when a DKIM signature is the one blamed for a DMARC failure, copies their base64 payload into the RFC 6591 `DKIM-Canonicalized-Header`/`DKIM-Canonicalized-Body` fields of the failure report -- re-folded to this codebase's own ARF-body convention rather than passed through verbatim. The staging headers are always stripped before final delivery regardless of this setting, so a signing filter with the upstream directive enabled can never leak them to a recipient even if this option is off. No size cap is applied to the canonicalized body data; large messages will produce correspondingly large failure reports. Completes the RFC 9991 gap noted in `DMARCBIS-REMAINING-WORK.md` (SPF-DNS remains open, blocked on libspf2 not exposing its DNS record chain). - **Deferred**: `SPF-DNS` (would require capturing every DNS record consulted during SPF evaluation, including `include:`/`redirect=` sub-queries — not exposed by libspf2's public API, unlike OpenDMARC's own unused-in-production built-in SPF evaluator, which already tracks this internally) and `DKIM-Canonicalized-Header`/`-Body` (no DKIM verification library is linked anywhere in this codebase, so there are no canonicalized bytes to expose) are both out of scope here — real new work, not wiring, tracked as follow-ups. --- diff --git a/DMARCBIS-REMAINING-WORK.md b/DMARCBIS-REMAINING-WORK.md index ff38c32..36ce65c 100644 --- a/DMARCBIS-REMAINING-WORK.md +++ b/DMARCBIS-REMAINING-WORK.md @@ -30,6 +30,16 @@ specifically (all resolved) and are not duplicated here. `feat/rfc9991-forensic-reporting`). Also restored PR #392's `rua=` destination verification, which had been silently lost, and fixed a NOERROR-vs-NXDOMAIN bug in it found via live testing. +- **RFC 9991 `DKIM-Canonicalized-Header`/`-Body` ARF fields**: new + `ReadCanonicalizedData` option reads `X-DKIM-Canonicalized-Header`/`-Body` + staging headers from an upstream OpenDKIM running the new + `AddCanonicalizedData` directive (`trusteddomainproject/OpenDKIM` PR + #423), correlates them to whichever DKIM signature is blamed for a DMARC + failure, and copies the base64 payload into the failure report, + re-folded to this codebase's own ARF-body convention. Staging headers + are always stripped before final delivery regardless of the setting. + See CHANGES-202605.md for the full writeup; cross-project design + history preserved below. SPF-DNS remains open. ## Remaining @@ -73,49 +83,18 @@ specifically (all resolved) and are not duplicated here. that's not the path in use. Real fix: a custom `SPF_dns_*`-compatible logging layer chained onto the real resolver, so queries can be captured without patching libspf2 itself. Self-contained but real new code. -- **`DKIM-Canonicalized-Header`/`-Body` ARF fields** (RFC 6591): need the - raw canonicalized header/body bytes as generated by the verifier. - Confirmed no DKIM verification library is linked anywhere in this - codebase (no `libdkim`, no `DKIM_STAT_*`) — DKIM results are only ever - sourced from a trusted upstream `Authentication-Results` header, never - verified locally, so there are no canonicalized bytes anywhere to expose. - A full local DKIM-verification subsystem in OpenDMARC would get this data - but duplicates work OpenDKIM already does. - **Preferred approach, cross-project (OpenDKIM + OpenDMARC), designed in - discussion 2026-07-20**: have OpenDKIM emit the canonicalized header/body - it already computes (a necessary byproduct of checking the signature - hash) as extra headers alongside its normal `Authentication-Results` - header -- e.g. `X-DKIM-Canonicalized-Header`/`-Body`, base64-encoded. - OpenDMARC reads them the same way it already reads the trusted upstream - A-R header (`mlfi_header()`, gated by `conf_trustedauthservids`, no new - trust boundary). Three design points that make this workable rather than - just clever: - - 1. **Opt-in on the OpenDKIM side**: a config toggle (e.g. - `AddCanonicalizedData yes`), off by default, that an operator enables - only if they've also enabled `AuthFailureReports` in OpenDMARC. - Deployments that don't send failure reports -- per RFC 9989's own - text, most Mail Receivers don't -- pay nothing. - 2. **Necessarily unconditional per message, once enabled**: OpenDKIM - can't know in advance which messages will later fail DMARC alignment - in OpenDMARC (that determination happens downstream), so while the - toggle is on, every DKIM-verified message carries the extra headers - through the milter chain. The savings is at the "do we want this - feature at all" level, not per-message. - 3. **OpenDMARC strips the headers before final delivery**: at `eom`, - after reading them (whether or not this particular message ends up - needing a failure report), `smfi_chgheader(ctx, header, idx, NULL)` - removes them -- the standard libmilter delete-by-NULL idiom, not - currently used anywhere in this codebase but well-established. The - canonicalized data exists only transiently between OpenDKIM and - OpenDMARC in the chain; it never reaches the recipient's mailbox, so - there's no permanent size or privacy cost added to delivered mail. - - Net effect: RFC 9991-complete `DKIM-Canonicalized-Header`/`-Body` support - without OpenDMARC ever verifying DKIM itself, gated so it costs nothing - for the common case of not sending failure reports at all. Requires - filing/scoping the OpenDKIM-side half separately -- out of this repo. +`DKIM-Canonicalized-Header`/`-Body` (the other RFC 6591 field originally +listed here as blocked) shipped -- see "Done" above. Design history: it +turned out to need less new work than expected, because OpenDKIM already +built the underlying capture mechanism for its own unrelated `SendReports` +feature (`dkimf_sigreport()`, `opendkim/opendkim.c:10098`, using +`dkim_sig_getreportinfo()` and `dkimf_base64_encode_file()`) -- confirmed +by reading the OpenDKIM source directly rather than assuming a new +DKIM-verification subsystem was required. The OpenDKIM-side half was +handed off via `RFC9991-CANONICALIZED-DATA-HANDOFF.md` in that repo and +landed as `AddCanonicalizedData` (PR #423); this repo's half landed as +`ReadCanonicalizedData` on `feat/rfc9991-forensic-reporting`. ### Open decision, not just missing code diff --git a/libopendmarc/dmarc.h.in b/libopendmarc/dmarc.h.in index 017dee2..94b4bad 100644 --- a/libopendmarc/dmarc.h.in +++ b/libopendmarc/dmarc.h.in @@ -178,6 +178,9 @@ OPENDMARC_STATUS_T opendmarc_policy_fetch_psd(DMARC_POLICY_T *pctx, int *psd); OPENDMARC_STATUS_T opendmarc_policy_fetch_t(DMARC_POLICY_T *pctx, int *t); OPENDMARC_STATUS_T opendmarc_policy_fetch_discovery_method(DMARC_POLICY_T *pctx, int *discovery_method); OPENDMARC_STATUS_T opendmarc_policy_fetch_fo(DMARC_POLICY_T *pctx, int *fo); +OPENDMARC_STATUS_T opendmarc_policy_fetch_np(DMARC_POLICY_T *pctx, int *np); +OPENDMARC_STATUS_T opendmarc_policy_fetch_psd(DMARC_POLICY_T *pctx, int *psd); +OPENDMARC_STATUS_T opendmarc_policy_fetch_discovery_method(DMARC_POLICY_T *pctx, int *discovery_method); u_char ** opendmarc_policy_fetch_rua(DMARC_POLICY_T *pctx, u_char *list_buf, size_t size_of_buf, int constant); u_char ** opendmarc_policy_fetch_ruf(DMARC_POLICY_T *pctx, u_char *list_buf, size_t size_of_buf, int constant); OPENDMARC_STATUS_T opendmarc_policy_fetch_utilized_domain(DMARC_POLICY_T *pctx, u_char *buf, size_t buflen); diff --git a/opendmarc/opendmarc.c b/opendmarc/opendmarc.c index b4cdee1..47acefd 100644 --- a/opendmarc/opendmarc.c +++ b/opendmarc/opendmarc.c @@ -132,6 +132,8 @@ struct dmarcf_msgctx u_char * mctx_dkimdomain; /* RFC 9991: d= of the last DKIM signature seen; not owned, points into "ar" */ u_char * mctx_dkimselector; /* RFC 9991: s= of same */ u_char * mctx_dkimidentity; /* RFC 9991: i= of same, if present */ + char * mctx_dkimcanonhdr; /* RFC 9991: unfolded base64 from X-DKIM-Canonicalized-Header, owned */ + char * mctx_dkimcanonbody; /* RFC 9991: unfolded base64 from X-DKIM-Canonicalized-Body, owned */ char * mctx_jobid; char ** mctx_arcchain; struct arcares_header * mctx_aarhead; @@ -172,6 +174,7 @@ struct dmarcf_config _Bool conf_reqfrom; _Bool conf_afrf; _Bool conf_afrfnone; + _Bool conf_readcanon; _Bool conf_rejectfail; _Bool conf_dolog; _Bool conf_enablecores; @@ -269,6 +272,7 @@ static struct dmarcf_config *dmarcf_config_new __P((void)); void dmarcf_freearray __P((char **a)); int dmarcf_mkarray __P((char *str, char *delim, char ***array)); sfsistat dmarcf_insheader __P((SMFICTX *, int, char *, char *)); +sfsistat dmarcf_chgheader __P((SMFICTX *, char *, int, char *)); sfsistat dmarcf_setreply __P((SMFICTX *, char *, char *, char *)); /* globals */ @@ -363,6 +367,31 @@ dmarcf_insheader(SMFICTX *ctx, int idx, char *hname, char *hvalue) #endif /* HAVE_SMFI_INSHEADER */ } +/* +** DMARCF_CHGHEADER -- wrapper for smfi_chgheader() +** +** Parameters: +** ctx -- milter (or test) context +** hname -- header name +** idx -- index of the header instance to change (1-based) +** hvalue -- header value, or NULL to delete the instance +** +** Return value: +** An sfsistat. +*/ + +sfsistat +dmarcf_chgheader(SMFICTX *ctx, char *hname, int idx, char *hvalue) +{ + assert(ctx != NULL); + assert(hname != NULL); + + if (testmode) + return dmarcf_test_chgheader(ctx, hname, idx, hvalue); + else + return smfi_chgheader(ctx, hname, idx, hvalue); +} + /* ** DMARCF_GETPRIV -- wrapper for smfi_getpriv() ** @@ -1184,6 +1213,203 @@ dmarcf_findheader(DMARCF_MSGCTX dfc, char *hname, int instance) return NULL; } +/* +** DMARCF_FIND_CANON_HEADER -- find an X-DKIM-Canonicalized-Header/-Body +** value matching a given signature +** +** Parameters: +** dfc -- filter context +** hname -- name of the header to look for ("X-DKIM-Canonicalized-Header" +** or "X-DKIM-Canonicalized-Body") +** domain -- "d=" value to match +** selector -- "s=" value to match (may be NULL) +** +** Return value: +** A malloc'd, whitespace-stripped copy of the "b=" tag's value from +** the first matching instance, or NULL if none matched. +** +** Notes: +** OpenDKIM's AddCanonicalizedData feature values these headers +** "d=; s=; b=", base64-folded with its own +** (non-RFC-5322) convention. "b=" is always the last tag and its +** value is never itself semicolon-delimited, so everything from "b=" +** to the end of the header value is taken as the payload; all +** whitespace (including whatever survived OpenDKIM's own folding) is +** stripped to leave one contiguous base64 string. +*/ + +static char * +dmarcf_find_canon_header(DMARCF_MSGCTX dfc, char *hname, + u_char *domain, u_char *selector) +{ + int instance; + struct dmarcf_header *hdr; + + assert(dfc != NULL); + assert(hname != NULL); + + if (domain == NULL) + return NULL; + + for (instance = 0; + (hdr = dmarcf_findheader(dfc, hname, instance)) != NULL; + instance++) + { + char *d; + char *s; + char *b; + char *p; + char *q; + char *out; + size_t outlen; + size_t len; + + d = strstr(hdr->hdr_value, "d="); + b = strstr(hdr->hdr_value, "b="); + if (d == NULL || b == NULL) + continue; + + len = strlen((char *) domain); + if (strncasecmp(d + 2, (char *) domain, len) != 0 || + (d[2 + len] != ';' && d[2 + len] != '\0' && + !isspace((unsigned char) d[2 + len]))) + continue; + + if (selector != NULL) + { + s = strstr(hdr->hdr_value, "s="); + len = strlen((char *) selector); + if (s == NULL || + strncasecmp(s + 2, (char *) selector, len) != 0 || + (s[2 + len] != ';' && s[2 + len] != '\0' && + !isspace((unsigned char) s[2 + len]))) + continue; + } + + p = b + 2; + outlen = strlen(p); + out = (char *) malloc(outlen + 1); + if (out == NULL) + return NULL; + + for (q = out; *p != '\0'; p++) + { + if (!isspace((unsigned char) *p)) + *q++ = *p; + } + *q = '\0'; + + return out; + } + + return NULL; +} + +/* +** DMARCF_STRIP_CANON_HEADERS -- remove any X-DKIM-Canonicalized-Header/ +** -Body headers from the outgoing message +** +** Parameters: +** ctx -- milter (or test) context +** dfc -- filter context +** +** Return value: +** None. +** +** Notes: +** RFC 9991: these are internal staging headers an upstream OpenDKIM +** may add for this filter's own use; they must never reach the +** recipient. Called on every code path in mlfi_eom that results in +** the message actually being delivered (SMFIS_ACCEPT/SMFIS_CONTINUE), +** regardless of whether ReadCanonicalizedData is set, so a mismatched +** OpenDKIM/OpenDMARC configuration can't leak them. Not called on +** REJECT/TEMPFAIL paths since no message reaches anyone in that case. +** +** smfi_chgheader()'s return value only reflects whether the request +** was written to the MTA socket, not whether a header existed at that +** index -- it is NOT safe to loop on it. Count real instances in the +** received header queue first via dmarcf_findheader(), then issue +** exactly that many deletes. Deleting index 1 repeatedly removes +** every instance, since each deletion shifts what was index 2 into +** index 1's place. +*/ + +static void +dmarcf_strip_canon_headers(SMFICTX *ctx, DMARCF_MSGCTX dfc) +{ + static char *canon_headers[] = + { + "X-DKIM-Canonicalized-Header", + "X-DKIM-Canonicalized-Body", + NULL + }; + int hc; + + assert(ctx != NULL); + assert(dfc != NULL); + + for (hc = 0; canon_headers[hc] != NULL; hc++) + { + int count; + + for (count = 0; + dmarcf_findheader(dfc, canon_headers[hc], count) != NULL; + count++) + continue; + + while (count-- > 0) + (void) dmarcf_chgheader(ctx, canon_headers[hc], 1, NULL); + } +} + +/* +** DMARCF_AFRF_CAT_FOLDED -- append a "Label: \n" field to an +** AFRF report buffer +** +** Parameters: +** dstr -- destination dstring (dfc->mctx_afrf) +** label -- field name +** value -- NUL-terminated value to fold +** +** Return value: +** None. +** +** Notes: +** Folds at 78 columns using "\n " (RFC 5322-legal folding whitespace), +** matching this codebase's bare-LF ARF body convention. Used for the +** base64 DKIM-Canonicalized-Header/-Body fields, which can be much +** longer than anything else in this report. +*/ + +#define DMARCF_AFRF_FOLDWIDTH 78 + +static void +dmarcf_afrf_cat_folded(struct dmarcf_dstring *dstr, const char *label, + const char *value) +{ + size_t len; + size_t off; + + dmarcf_dstring_printf(dstr, "%s: ", label); + + len = strlen(value); + for (off = 0; off < len; off += DMARCF_AFRF_FOLDWIDTH) + { + size_t chunk; + + chunk = len - off; + if (chunk > DMARCF_AFRF_FOLDWIDTH) + chunk = DMARCF_AFRF_FOLDWIDTH; + + if (off > 0) + dmarcf_dstring_cat(dstr, (u_char *) "\n "); + + dmarcf_dstring_catn(dstr, (u_char *) (value + off), chunk); + } + + dmarcf_dstring_cat(dstr, (u_char *) "\n"); +} + /* ** DMARCF_CONFIG_LOAD -- load a configuration handle based on file content ** @@ -1315,6 +1541,10 @@ dmarcf_config_load(struct config *data, struct dmarcf_config *conf, &conf->conf_afrfnone, sizeof conf->conf_afrfnone); + (void) config_get(data, "ReadCanonicalizedData", + &conf->conf_readcanon, + sizeof conf->conf_readcanon); + (void) config_get(data, "FailureReportsSentBy", &conf->conf_afrfas, sizeof conf->conf_afrfas); @@ -1761,6 +1991,9 @@ dmarcf_cleanup(SMFICTX *ctx) if (dfc->mctx_afrf != NULL) dmarcf_dstring_free(dfc->mctx_afrf); + TRYFREE(dfc->mctx_dkimcanonhdr); + TRYFREE(dfc->mctx_dkimcanonbody); + if (dfc->mctx_hqhead != NULL) { struct dmarcf_header *hdr; @@ -2470,6 +2703,7 @@ mlfi_eom(SMFICTX *ctx) } else { + dmarcf_strip_canon_headers(ctx, dfc); ret = SMFIS_ACCEPT; goto done; } @@ -2503,6 +2737,7 @@ mlfi_eom(SMFICTX *ctx) } else { + dmarcf_strip_canon_headers(ctx, dfc); ret = SMFIS_ACCEPT; goto done; } @@ -2529,6 +2764,7 @@ mlfi_eom(SMFICTX *ctx) } else { + dmarcf_strip_canon_headers(ctx, dfc); ret = SMFIS_ACCEPT; goto done; } @@ -2543,6 +2779,7 @@ mlfi_eom(SMFICTX *ctx) dfc->mctx_jobid, domain); } + dmarcf_strip_canon_headers(ctx, dfc); ret = SMFIS_ACCEPT; goto done; } @@ -2941,6 +3178,30 @@ mlfi_eom(SMFICTX *ctx) dfc->mctx_dkimselector = dkim_selector; dfc->mctx_dkimidentity = dkim_identity; + /* + ** RFC 9991: DKIM-Canonicalized-Header/-Body, sourced + ** from an upstream OpenDKIM's (optional) + ** X-DKIM-Canonicalized-Header/-Body headers. Same + ** last-wins semantics as the fields just above -- + ** free any value kept for a previous signature. + */ + if (conf->conf_readcanon && conf->conf_afrf) + { + TRYFREE(dfc->mctx_dkimcanonhdr); + TRYFREE(dfc->mctx_dkimcanonbody); + + dfc->mctx_dkimcanonhdr = + dmarcf_find_canon_header(dfc, + "X-DKIM-Canonicalized-Header", + dkim_domain, + dkim_selector); + dfc->mctx_dkimcanonbody = + dmarcf_find_canon_header(dfc, + "X-DKIM-Canonicalized-Body", + dkim_domain, + dkim_selector); + } + dmarcf_dstring_printf(dfc->mctx_histbuf, "dkim %s %s %d\n", dkim_domain, @@ -3283,6 +3544,7 @@ mlfi_eom(SMFICTX *ctx) } } + dmarcf_strip_canon_headers(ctx, dfc); ret = SMFIS_ACCEPT; goto done; } @@ -3770,6 +4032,23 @@ mlfi_eom(SMFICTX *ctx) "DKIM-Identity: %s\n", dfc->mctx_dkimidentity); } + + if (conf->conf_readcanon) + { + if (dfc->mctx_dkimcanonhdr != NULL) + { + dmarcf_afrf_cat_folded(dfc->mctx_afrf, + "DKIM-Canonicalized-Header", + dfc->mctx_dkimcanonhdr); + } + + if (dfc->mctx_dkimcanonbody != NULL) + { + dmarcf_afrf_cat_folded(dfc->mctx_afrf, + "DKIM-Canonicalized-Body", + dfc->mctx_dkimcanonbody); + } + } } } @@ -4134,6 +4413,8 @@ mlfi_eom(SMFICTX *ctx) } } + dmarcf_strip_canon_headers(ctx, dfc); + dmarcf_cleanup(ctx); done: diff --git a/opendmarc/opendmarc.conf.5.in b/opendmarc/opendmarc.conf.5.in index 28fd5c7..b186b59 100644 --- a/opendmarc/opendmarc.conf.5.in +++ b/opendmarc/opendmarc.conf.5.in @@ -389,6 +389,30 @@ not be able to determine the Organizational Domain and only the presented domain will be evaluated. This file should be periodically updated. One location to retrieve the file from is https://publicsuffix.org/list/ +.TP +.I ReadCanonicalizedData (Boolean) +If set, and +.I FailureReports +is also set, the filter will look for +.I X-DKIM-Canonicalized-Header +and +.I X-DKIM-Canonicalized-Body +headers added by an upstream signing filter (e.g. +.IR OpenDKIM (8) +with its +.I AddCanonicalizedData +directive enabled) and, when a DKIM signature is the one blamed for a +DMARC failure, populate the +.I DKIM-Canonicalized-Header +and +.I DKIM-Canonicalized-Body +fields of the resulting RFC 6591 failure report with their contents. +These staging headers are always stripped from the outgoing message +before final delivery, whether or not this option is set, so they never +reach the recipient. Note that for large messages this can make for a +correspondingly large failure report, since the canonicalized message +body is included in full; no size limit is applied. Not set by default. + .TP .I RecordAllMessages (Boolean) If set and diff --git a/opendmarc/opendmarc.conf.sample b/opendmarc/opendmarc.conf.sample index 3a9df79..14d6a1e 100644 --- a/opendmarc/opendmarc.conf.sample +++ b/opendmarc/opendmarc.conf.sample @@ -371,6 +371,20 @@ # # PublicSuffixList path +## ReadCanonicalizedData { true | false } +## default "false" +## +## If set, and "FailureReports" is also set, look for +## X-DKIM-Canonicalized-Header/-Body headers added by an upstream signing +## filter (e.g. OpenDKIM with AddCanonicalizedData enabled) and use them +## to populate the DKIM-Canonicalized-Header/-Body fields of the RFC 6591 +## failure report. These staging headers are always stripped before final +## delivery regardless of this setting. No size limit is applied to the +## canonicalized body data, so large messages can produce correspondingly +## large failure reports. +# +# ReadCanonicalizedData false + ## RecordAllMessages { true | false } ## default "false" ## diff --git a/opendmarc/tests/Makefile.am b/opendmarc/tests/Makefile.am index ba6cc11..767d41c 100644 --- a/opendmarc/tests/Makefile.am +++ b/opendmarc/tests/Makefile.am @@ -6,7 +6,7 @@ if LIVE_TESTS check_SCRIPTS += t-verify-multi-from-reject t-verify-multi-from-malformed \ t-verify-nodata t-verify-nodata-reject \ t-verify-unspec t-verify-received-spf-good t-verify-received-spf-bad \ - t-verify-authservid-jobid + t-verify-authservid-jobid t-verify-canondata endif if TEST_SPF @@ -32,6 +32,7 @@ EXTRA_DIST = \ t-verify-multi-from-malformed t-verify-multi-from-malformed.conf \ t-verify-multi-from-malformed.lua \ t-verify-authservid-jobid t-verify-authservid-jobid.conf \ - t-verify-authservid-jobid.lua + t-verify-authservid-jobid.lua \ + t-verify-canondata t-verify-canondata.conf t-verify-canondata.lua MOSTLYCLEANFILES= diff --git a/opendmarc/tests/t-verify-canondata b/opendmarc/tests/t-verify-canondata new file mode 100755 index 0000000..3802d68 --- /dev/null +++ b/opendmarc/tests/t-verify-canondata @@ -0,0 +1,13 @@ +#!/bin/sh +# +# Test that X-DKIM-Canonicalized-Header/-Body staging headers (as added by +# an upstream OpenDKIM's AddCanonicalizedData) are stripped before final +# delivery. +# + +if [ x"$srcdir" = x"" ] +then + srcdir=`pwd` +fi + +miltertest -s $srcdir/t-verify-canondata.lua diff --git a/opendmarc/tests/t-verify-canondata.conf b/opendmarc/tests/t-verify-canondata.conf new file mode 100644 index 0000000..fdebc44 --- /dev/null +++ b/opendmarc/tests/t-verify-canondata.conf @@ -0,0 +1,4 @@ +# X-DKIM-Canonicalized-Header/-Body staging headers must always be +# stripped before delivery, regardless of ReadCanonicalizedData/ +# FailureReports -- intentionally not set here. +Background No diff --git a/opendmarc/tests/t-verify-canondata.lua b/opendmarc/tests/t-verify-canondata.lua new file mode 100644 index 0000000..c740e92 --- /dev/null +++ b/opendmarc/tests/t-verify-canondata.lua @@ -0,0 +1,114 @@ +-- Copyright (c) 2026, The Trusted Domain Project. All rights reserved. + +-- Test that X-DKIM-Canonicalized-Header/-Body staging headers (as added by +-- an upstream OpenDKIM's AddCanonicalizedData directive, RFC 9991) are +-- stripped from the outgoing message before final delivery, unconditionally +-- -- ReadCanonicalizedData is deliberately NOT set in the .conf for this +-- test, since stripping must happen regardless of whether this filter is +-- configured to read them. + +mt.echo("*** canonicalized-data header stripping test") + +-- setup +sock = "unix:" .. mt.getcwd() .. "/t-verify-canondata.sock" +binpath = mt.getcwd() .. "/.." +if os.getenv("srcdir") ~= nil then + mt.chdir(os.getenv("srcdir")) +end + +-- try to start the filter +mt.startfilter(binpath .. "/opendmarc", "-l", "-c", "t-verify-canondata.conf", + "-p", sock) + +-- try to connect to it +conn = mt.connect(sock, 40, 0.05) +if conn == nil then + error("mt.connect() failed") +end + +-- send connection information +-- mt.negotiate() is called implicitly +if mt.conninfo(conn, "localhost2", "127.0.0.2") ~= nil then + error("mt.conninfo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.conninfo() unexpected reply") +end + +-- send envelope macros and sender data +-- mt.helo() is called implicitly +mt.macro(conn, SMFIC_MAIL, "i", "t-verify-canondata") +if mt.mailfrom(conn, "user@example.com") ~= nil then + error("mt.mailfrom() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.mailfrom() unexpected reply") +end + +-- send headers, including the staging headers an upstream OpenDKIM would +-- have added +-- mt.rcptto() is called implicitly +if mt.header(conn, "From", "user@example.com") ~= nil then + error("mt.header(From) failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(From) unexpected reply") +end +if mt.header(conn, "To", "user@example.com") ~= nil then + error("mt.header(To) failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(To) unexpected reply") +end +if mt.header(conn, "Date", "Tue, 22 Dec 2009 13:04:12 -0800") ~= nil then + error("mt.header(Date) failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(Date) unexpected reply") +end +if mt.header(conn, "Subject", "DMARC test") ~= nil then + error("mt.header(Subject) failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(Subject) unexpected reply") +end +if mt.header(conn, "X-DKIM-Canonicalized-Header", + "d=example.com; s=sel; b=AAAAformattingexample") ~= nil then + error("mt.header(X-DKIM-Canonicalized-Header) failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(X-DKIM-Canonicalized-Header) unexpected reply") +end +if mt.header(conn, "X-DKIM-Canonicalized-Body", + "d=example.com; s=sel; b=BBBBformattingexample") ~= nil then + error("mt.header(X-DKIM-Canonicalized-Body) failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(X-DKIM-Canonicalized-Body) unexpected reply") +end + +-- send EOH +if mt.eoh(conn) ~= nil then + error("mt.eoh() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.eoh() unexpected reply") +end + +-- end of message; let the filter react +if mt.eom(conn) ~= nil then + error("mt.eom() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.eom() unexpected reply") +end + +-- verify the staging headers did NOT survive to the outgoing message +if mt.getheader(conn, "X-DKIM-Canonicalized-Header", 0) ~= nil then + error("X-DKIM-Canonicalized-Header was not stripped") +end +if mt.getheader(conn, "X-DKIM-Canonicalized-Body", 0) ~= nil then + error("X-DKIM-Canonicalized-Body was not stripped") +end + +mt.disconnect(conn) From c1b53d378801ec5882eacd7a2b85e036b11527fc Mon Sep 17 00:00:00 2001 From: Dan Mahoney Date: Wed, 22 Jul 2026 00:53:36 -0700 Subject: [PATCH 7/8] feat: RFC 9991 SPF-DNS ARF field via a custom libspf2 DNS layer Closes the last item in the RFC 9991 gap: the SPF-DNS ARF field (RFC 6591), required on ruf= forensic reports whenever OpenDMARC's own SPF self-validation contributed to a DMARC failure. libspf2's public API doesn't expose the DNS records it consults during evaluation -- only a human-readable summary -- so unlike the DKIM-Canonicalized-Header/-Body field, this one needed real new code rather than wiring. New opt-in LogSPFDNS option (only meaningful alongside SPFSelfValidate, off by default so it costs nothing when not wanted). When enabled, opendmarc_spf2_alloc_ctx() builds a custom SPF_dns_server_t "DNS layer" (libopendmarc/opendmarc_spf_dns_log.c) and splices it into libspf2's own pluggable resolver chain, above the caching layer so every SPF-record lookup a self-validated message makes is seen regardless of cache hits, including domains reached via include:/redirect=. The layer passes every query through to the real resolver unchanged -- evaluation behavior is never altered -- and for TXT/SPF-type queries whose content is itself an SPF policy record (starts "v=spf1", or any content for the historic RR type 99), appends an "SPF-DNS: txt|spf : domain : \"content\"" line, with '"'/'\' backslash-escaped per RFC 5322 quoted-string rules and embedded CR/LF dropped outright (this text is later written raw into an outgoing message piped to sendmail, so a malicious DNS record must not be able to inject header lines into the report). opendmarc_spf2_test()/opendmarc_spf_test() both gain a want_dns_log parameter and an optional out-param for the accumulated lines, threaded through a real architectural wrinkle: opendmarc_spf2_test() frees its whole SPF_CTX_T (including the DNS layer chain) before returning to its caller, so the accumulator has to be extracted before that free, not read back afterward. The fallback (non-libspf2) evaluator gets the same signature for call-site compatibility but doesn't populate the lines -- it isn't the path linked in production, so real wiring there (it already tracks queried domains/records internally) is left as a follow-up rather than done speculatively. Also includes a real, independent correctness fix found while testing this: mlfi_eom()'s job-ID lookup carried a comment describing itself as a retry "in case it came down later than expected (e.g. postfix)", but no earlier attempt actually existed anywhere in the code for it to be retrying. Added one in mlfi_envfrom(), matching the documented intent. New t-verify-self-spf-report test exercises this against isc.org's real DNS -- deliberately chosen over the existing t-verify-self-spf test's trusteddomain.org because isc.org's SPF record has several include: mechanisms, so the test covers real chain-following, not just a single-record lookup. Verified live on quark against real libmilter and libspf2: 11/11 tests pass, and the captured report's SPF-DNS lines were manually checked against isc.org's actual published SPF record. --- libopendmarc/Makefile.am | 1 + libopendmarc/dmarc.h.in | 4 +- libopendmarc/opendmarc_internal.h | 10 +- libopendmarc/opendmarc_spf.c | 57 +++- libopendmarc/opendmarc_spf_dns_log.c | 250 ++++++++++++++++++ libopendmarc/tests/test_spf.c | 4 +- opendmarc/opendmarc.c | 38 ++- opendmarc/opendmarc.conf.5.in | 20 ++ opendmarc/opendmarc.conf.sample | 14 + opendmarc/tests/Makefile.am | 6 +- opendmarc/tests/t-verify-self-spf-report | 11 + opendmarc/tests/t-verify-self-spf-report.conf | 9 + opendmarc/tests/t-verify-self-spf-report.lua | 129 +++++++++ 13 files changed, 537 insertions(+), 16 deletions(-) create mode 100644 libopendmarc/opendmarc_spf_dns_log.c create mode 100755 opendmarc/tests/t-verify-self-spf-report create mode 100644 opendmarc/tests/t-verify-self-spf-report.conf create mode 100644 opendmarc/tests/t-verify-self-spf-report.lua diff --git a/libopendmarc/Makefile.am b/libopendmarc/Makefile.am index 0caf0bd..1c3e6e7 100644 --- a/libopendmarc/Makefile.am +++ b/libopendmarc/Makefile.am @@ -13,6 +13,7 @@ libopendmarc_la_SOURCES = dmarc.h \ opendmarc_strl.h \ opendmarc_spf.c \ opendmarc_spf_dns.c \ + opendmarc_spf_dns_log.c \ opendmarc_internal.h libopendmarc_la_LDFLAGS = -version-info $(LIBOPENDMARC_VERSION_INFO) libopendmarc_la_LIBADD = $(LIBRESOLV) diff --git a/libopendmarc/dmarc.h.in b/libopendmarc/dmarc.h.in index 94b4bad..247bc33 100644 --- a/libopendmarc/dmarc.h.in +++ b/libopendmarc/dmarc.h.in @@ -213,8 +213,8 @@ int opendmarc_policy_to_buf(DMARC_POLICY_T *pctx, char *buf, size_t buflen) /* * SPF Processing */ -int opendmarc_spf_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int soft_fail_as_pass, char *human_readable, size_t human_readable_len, int *use_mailfrom); -int opendmarc_spf2_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int softfail_okay_flag, char *human_readable, size_t human_readable_len, int *used_mfrom); +int opendmarc_spf_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int soft_fail_as_pass, char *human_readable, size_t human_readable_len, int *use_mailfrom, int want_dns_log, char **spf_dns_lines); +int opendmarc_spf2_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int softfail_okay_flag, char *human_readable, size_t human_readable_len, int *used_mfrom, int want_dns_log, char **spf_dns_lines); #ifdef __cplusplus } diff --git a/libopendmarc/opendmarc_internal.h b/libopendmarc/opendmarc_internal.h index a3d1563..dbb1f8e 100644 --- a/libopendmarc/opendmarc_internal.h +++ b/libopendmarc/opendmarc_internal.h @@ -259,7 +259,9 @@ void opendmarc_policy_library_dns_hook(int *nscountp, struct sockaddr_in *nsaddr #if HAVE_SPF2_H #include "spf.h" -typedef struct spf_context_struct { +#include "spf_dns_resolv.h" +#include "spf_dns_cache.h" +typedef struct spf_context_struct { SPF_server_t * spf_server; SPF_request_t * spf_request; SPF_response_t * spf_response; @@ -267,8 +269,10 @@ typedef struct spf_context_struct { char mailfrom_addr[512]; char mailfrom_domain[256]; char helo_domain[256]; + char * spf_dns_log; /* RFC 9991 SPF-DNS lines, owned by caller once handed over */ } SPF_CTX_T; -int opendmarc_spf2_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int softfail_okay_flag, char *human_readable, size_t human_readable_len, int *used_mfrom); +int opendmarc_spf2_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int softfail_okay_flag, char *human_readable, size_t human_readable_len, int *used_mfrom, int want_dns_log, char **spf_dns_lines); +SPF_dns_server_t *opendmarc_spf_dns_log_new(SPF_dns_server_t *layer_below, char **logbuf, int debug); #else /* not HAVE_SPF2_H */ @@ -290,7 +294,7 @@ typedef struct spf_context_struct { char exp_buf[512]; int did_get_exp; } SPF_CTX_T; -int opendmarc_spf_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int softfail_okay_flag, char *human_readable, size_t human_readable_len, int *used_mfrom); +int opendmarc_spf_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int softfail_okay_flag, char *human_readable, size_t human_readable_len, int *used_mfrom, int want_dns_log, char **spf_dns_lines); char ** opendmarc_spf_dns_lookup_a(char *domain, char **ary, int *cnt); char ** opendmarc_spf_dns_lookup_mx(char *domain, char **ary, int *cnt); char ** opendmarc_spf_dns_lookup_mx_domain(char *domain, char **ary, int *cnt); diff --git a/libopendmarc/opendmarc_spf.c b/libopendmarc/opendmarc_spf.c index f8f2804..e4e1e12 100644 --- a/libopendmarc/opendmarc_spf.c +++ b/libopendmarc/opendmarc_spf.c @@ -25,8 +25,8 @@ #if HAVE_SPF2_H // Here we have spf.h, so libspf2 is available. -SPF_CTX_T * -opendmarc_spf2_alloc_ctx() +static SPF_CTX_T * +opendmarc_spf2_alloc_ctx(int want_dns_log) { SPF_CTX_T *spfctx = NULL; @@ -34,7 +34,31 @@ opendmarc_spf2_alloc_ctx() if (spfctx == NULL) return NULL; (void) memset(spfctx, '\0', sizeof(SPF_CTX_T)); - spfctx->spf_server = SPF_server_new(SPF_DNS_CACHE, 0); + + if (want_dns_log) + { + /* + ** Build the same resolv+cache chain SPF_server_new(SPF_DNS_CACHE, 0) + ** builds internally (confirmed against upstream libspf2's + ** spf_server.c: SPF_dns_resolv_new(NULL, NULL, debug) then + ** SPF_dns_cache_new(dc_r, NULL, debug, 8)), with a logging layer + ** spliced in above the cache so every SPF-record lookup this + ** evaluation makes is seen regardless of cache hits. + */ + SPF_dns_server_t *resolv; + SPF_dns_server_t *cache; + SPF_dns_server_t *logging; + + resolv = SPF_dns_resolv_new(NULL, NULL, 0); + cache = SPF_dns_cache_new(resolv, NULL, 0, 8); + logging = opendmarc_spf_dns_log_new(cache, &spfctx->spf_dns_log, 0); + spfctx->spf_server = SPF_server_new_dns(logging, 0); + } + else + { + spfctx->spf_server = SPF_server_new(SPF_DNS_CACHE, 0); + } + spfctx->spf_request = SPF_request_new(spfctx->spf_server); return spfctx; } @@ -118,7 +142,7 @@ opendmarc_spf2_specify_ip_address(SPF_CTX_T *spfctx, char *ip_address, size_t ip } int -opendmarc_spf2_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int softfail_okay_flag, char *human_readable, size_t human_readable_len, int *used_mfrom) +opendmarc_spf2_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int softfail_okay_flag, char *human_readable, size_t human_readable_len, int *used_mfrom, int want_dns_log, char **spf_dns_lines) { SPF_CTX_T * ctx; int ret; @@ -129,8 +153,11 @@ opendmarc_spf2_test(char *ip_address, char *mail_from_domain, char *helo_domain, if (used_mfrom != NULL) *used_mfrom = FALSE; + if (spf_dns_lines != NULL) + *spf_dns_lines = NULL; + (void) memset(xbuf, '\0', sizeof xbuf); - ctx = opendmarc_spf2_alloc_ctx(); + ctx = opendmarc_spf2_alloc_ctx(want_dns_log); if (ctx == NULL) { if (human_readable != NULL) @@ -191,6 +218,13 @@ opendmarc_spf2_test(char *ip_address, char *mail_from_domain, char *helo_domain, (void) strlcpy(human_readable, SPF_strresult(SPF_response_result(ctx->spf_response)), human_readable_len); ctx->spf_result = SPF_response_result(ctx->spf_response); ret = (int) ctx->spf_result; + + if (want_dns_log && spf_dns_lines != NULL) + { + *spf_dns_lines = ctx->spf_dns_log; + ctx->spf_dns_log = NULL; + } + ctx = opendmarc_spf2_free_ctx(ctx); if (ret != SPF_RESULT_PASS) @@ -2101,7 +2135,7 @@ opendmarc_spf_specify_record(SPF_CTX_T *spfctx, char *spf_record, size_t spf_rec } int -opendmarc_spf_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int softfail_okay_flag, char *human_readable, size_t human_readable_len, int *used_mfrom) +opendmarc_spf_test(char *ip_address, char *mail_from_domain, char *helo_domain, char *spf_record, int softfail_okay_flag, char *human_readable, size_t human_readable_len, int *used_mfrom, int want_dns_log, char **spf_dns_lines) { SPF_CTX_T * ctx; int ret; @@ -2111,6 +2145,17 @@ opendmarc_spf_test(char *ip_address, char *mail_from_domain, char *helo_domain, if (used_mfrom != NULL) *used_mfrom = FALSE; + /* + ** This fallback evaluator doesn't yet populate spf_dns_lines -- + ** it isn't the path linked in production (libspf2 is, when + ** available), so real wiring here (it already tracks queried + ** domains/records internally via its own recursion stack) is left + ** as a follow-up rather than done speculatively. + */ + if (spf_dns_lines != NULL) + *spf_dns_lines = NULL; + (void) want_dns_log; + (void) memset(xbuf, '\0', sizeof xbuf); ctx = opendmarc_spf_alloc_ctx(); if (ctx == NULL) diff --git a/libopendmarc/opendmarc_spf_dns_log.c b/libopendmarc/opendmarc_spf_dns_log.c new file mode 100644 index 0000000..58586bd --- /dev/null +++ b/libopendmarc/opendmarc_spf_dns_log.c @@ -0,0 +1,250 @@ +/******************************************************************** +** OPENDMARC_SPF_DNS_LOG.C -- a libspf2 DNS layer that logs SPF-record +** lookups for the RFC 6591/9991 SPF-DNS +** ARF field, without altering evaluation +** behavior +**********************************************************************/ +# include "opendmarc_internal.h" + +/* libbsd if found */ +#ifdef USE_BSD_H +# include +#endif /* USE_BSD_H */ + +/* libstrl if needed */ +#ifdef USE_STRL_H +# include +#endif /* USE_STRL_H */ + +/* opendmarc_strl if needed */ +#ifdef USE_DMARCSTRL_H +# include +#endif /* USE_DMARCSTRL_H */ + +# include "dmarc.h" + +#if WITH_SPF + +#if HAVE_SPF2_H + +#include + +/* +** Per-layer private state. "logbuf" is the address of a caller-owned +** "char *" accumulator (see opendmarc_spf.c); this layer appends to +** *logbuf but does not own it and never frees it, since it must +** outlive this DNS layer's own teardown (SPF_server_free() runs +** before opendmarc_spf2_test() hands the log back to its caller). +*/ +struct opendmarc_spf_dns_log_ctx +{ + char ** logbuf; +}; + +/* +** OPENDMARC_SPF_DNS_LOG_APPEND -- append one RFC 6591 "SPF-DNS:" line +** +** Parameters: +** logbuf -- address of the caller-owned accumulator +** rrtype -- "txt" or "spf" (the ABNF's literal tokens) +** domain -- domain that was queried +** content -- raw DNS record content to quote +** +** Return value: +** None. Silently does nothing on allocation failure -- a missing +** SPF-DNS line is not worth failing report generation over. +** +** Notes: +** "content" comes from a DNS response and so is attacker-influenced +** (any domain consulted during evaluation, including third parties +** reached via include:/redirect=, or the message's own claimed +** envelope domain). '"' and '\' are backslash-escaped per RFC 5322 +** quoted-string rules; CR and LF are dropped outright rather than +** escaped, since this text is later written raw into an outgoing +** message piped to sendmail (ReportCommand) -- allowing embedded +** CRLFs through would let a malicious DNS record inject additional +** header lines into the report. +*/ + +static void +opendmarc_spf_dns_log_append(char **logbuf, const char *rrtype, + const char *domain, const char *content) +{ + size_t oldlen; + size_t addlen; + size_t contentlen; + size_t remain; + char *newbuf; + char *p; + const char *c; + int n; + + if (logbuf == NULL || rrtype == NULL || domain == NULL || + content == NULL) + return; + + oldlen = (*logbuf != NULL) ? strlen(*logbuf) : 0; + contentlen = strlen(content); + + /* worst case: every content byte becomes a 2-byte escape */ + addlen = strlen("SPF-DNS: ") + strlen(rrtype) + strlen(" : ") + + strlen(domain) + strlen(" : \"") + (contentlen * 2) + + strlen("\"\n"); + + newbuf = (char *) realloc(*logbuf, oldlen + addlen + 1); + if (newbuf == NULL) + return; + *logbuf = newbuf; + + p = newbuf + oldlen; + remain = addlen + 1; + + n = snprintf(p, remain, "SPF-DNS: %s : %s : \"", rrtype, domain); + if (n < 0 || (size_t) n >= remain) + return; + p += n; + remain -= (size_t) n; + + for (c = content; *c != '\0'; c++) + { + if (*c == '\r' || *c == '\n') + continue; + + if (*c == '"' || *c == '\\') + { + if (remain < 2) + break; + *p++ = '\\'; + remain--; + } + + if (remain < 1) + break; + *p++ = *c; + remain--; + } + + (void) snprintf(p, remain, "\"\n"); +} + +/* +** OPENDMARC_SPF_DNS_LOG_LOOKUP -- SPF_dns_lookup_t implementation +** +** Delegates to the layer below unconditionally (evaluation behavior +** is never altered by this layer), then, for TXT/SPF-type queries, +** logs any returned record whose content is itself an SPF policy +** record -- i.e. begins "v=spf1 " (case-insensitively), or is any +** record at all for the historic RR type 99 ("SPF"), which has no +** other purpose. This deliberately excludes unrelated TXT records +** that might share a name with a real SPF record, matching RFC 6591's +** "SPF record...used to obtain the SPF result" wording rather than +** logging every TXT record ever seen at a queried name. +*/ + +static SPF_dns_rr_t * +opendmarc_spf_dns_log_lookup(SPF_dns_server_t *spf_dns_server, + const char *domain, ns_type rr_type, + int should_cache) +{ + struct opendmarc_spf_dns_log_ctx *lctx; + SPF_dns_server_t *below; + SPF_dns_rr_t *rr; + int c; + + lctx = (struct opendmarc_spf_dns_log_ctx *) spf_dns_server->hook; + below = spf_dns_server->layer_below; + + rr = below->lookup(below, domain, rr_type, should_cache); + + if (rr != NULL && rr->rr != NULL && + (rr_type == ns_t_txt || rr_type == ns_t_spf)) + { + for (c = 0; c < rr->num_rr; c++) + { + const char *txt = rr->rr[c]->txt; + + if (txt == NULL) + continue; + + if (rr_type == ns_t_spf || + strncasecmp(txt, "v=spf1 ", 7) == 0 || + strcasecmp(txt, "v=spf1") == 0) + { + opendmarc_spf_dns_log_append(lctx->logbuf, + rr_type == ns_t_spf ? "spf" : "txt", + domain, txt); + } + } + } + + return rr; +} + +/* +** OPENDMARC_SPF_DNS_LOG_DESTROY -- SPF_dns_destroy_t implementation +** +** Frees this layer's own private state only. The accumulator the +** layer's hook points at is caller-owned and is not touched here. +*/ + +static void +opendmarc_spf_dns_log_destroy(SPF_dns_server_t *spf_dns_server) +{ + if (spf_dns_server == NULL) + return; + + free(spf_dns_server->hook); + free(spf_dns_server); +} + +/* +** OPENDMARC_SPF_DNS_LOG_NEW -- construct the logging DNS layer +** +** Parameters: +** layer_below -- the DNS layer to delegate all lookups to +** logbuf -- address of a caller-owned "char *" accumulator +** (initially NULL is fine; grown via realloc() as lines +** are appended); must outlive this layer's own teardown +** debug -- passed through to the SPF_dns_server_t "debug" field +** +** Return value: +** A new SPF_dns_server_t, or NULL on allocation failure. +*/ + +SPF_dns_server_t * +opendmarc_spf_dns_log_new(SPF_dns_server_t *layer_below, char **logbuf, + int debug) +{ + SPF_dns_server_t *spf_dns_server; + struct opendmarc_spf_dns_log_ctx *lctx; + + spf_dns_server = (SPF_dns_server_t *) malloc(sizeof(SPF_dns_server_t)); + if (spf_dns_server == NULL) + return NULL; + (void) memset(spf_dns_server, '\0', sizeof(SPF_dns_server_t)); + + lctx = (struct opendmarc_spf_dns_log_ctx *) + malloc(sizeof(struct opendmarc_spf_dns_log_ctx)); + if (lctx == NULL) + { + free(spf_dns_server); + return NULL; + } + lctx->logbuf = logbuf; + + spf_dns_server->destroy = opendmarc_spf_dns_log_destroy; + spf_dns_server->lookup = opendmarc_spf_dns_log_lookup; + spf_dns_server->get_spf = NULL; + spf_dns_server->get_exp = NULL; + spf_dns_server->add_cache = NULL; + spf_dns_server->layer_below = layer_below; + spf_dns_server->name = "opendmarc-log"; + spf_dns_server->debug = debug; + spf_dns_server->hook = lctx; + + return spf_dns_server; +} + +#endif /* HAVE_SPF2_H */ + +#endif /* WITH_SPF */ diff --git a/libopendmarc/tests/test_spf.c b/libopendmarc/tests/test_spf.c index 5481edc..dfea05f 100644 --- a/libopendmarc/tests/test_spf.c +++ b/libopendmarc/tests/test_spf.c @@ -33,7 +33,7 @@ opendmarc_spf2_run_test() for (tpp = tests; tpp->helo != NULL; tpp++) { (void) memset(human, '\0', sizeof human); - status = opendmarc_spf2_test(tpp->ip, tpp->mfrom, tpp->helo, NULL, FALSE, human, sizeof human, &used_mfrom); + status = opendmarc_spf2_test(tpp->ip, tpp->mfrom, tpp->helo, NULL, FALSE, human, sizeof human, &used_mfrom, FALSE, NULL); if (status != tpp->outcome) { printf("Error: ip=\"%s\", mfrom=\"%s\", helo=\"%s\", error(%d)= %s\n", tpp->ip, tpp->mfrom, tpp->helo, status, human); @@ -97,7 +97,7 @@ opendmarc_spf_test_records(void) success = failures = 0; for (sp = spflist; sp->ip != NULL; ++sp) { - status = opendmarc_spf_test(sp->ip, sp->mfrom, sp->helo, sp->spfrecord, FALSE, human, sizeof human, &use_mfrom); + status = opendmarc_spf_test(sp->ip, sp->mfrom, sp->helo, sp->spfrecord, FALSE, human, sizeof human, &use_mfrom, FALSE, NULL); if (status != sp->status) { printf("Error: ip=\"%s\", mfrom=\"%s\", helo=\"%s\", spf=\"%s\", error(%d)= %s\n", sp->ip, sp->mfrom, sp->helo, diff --git a/opendmarc/opendmarc.c b/opendmarc/opendmarc.c index 47acefd..742ea68 100644 --- a/opendmarc/opendmarc.c +++ b/opendmarc/opendmarc.c @@ -134,6 +134,9 @@ struct dmarcf_msgctx u_char * mctx_dkimidentity; /* RFC 9991: i= of same, if present */ char * mctx_dkimcanonhdr; /* RFC 9991: unfolded base64 from X-DKIM-Canonicalized-Header, owned */ char * mctx_dkimcanonbody; /* RFC 9991: unfolded base64 from X-DKIM-Canonicalized-Body, owned */ +#if WITH_SPF + char * mctx_spfdnslines; /* RFC 9991: pre-formatted SPF-DNS: lines from LogSPFDNS, owned */ +#endif /* WITH_SPF */ char * mctx_jobid; char ** mctx_arcchain; struct arcares_header * mctx_aarhead; @@ -184,6 +187,7 @@ struct dmarcf_config #if WITH_SPF _Bool conf_spfignoreresults; _Bool conf_spfselfvalidate; + _Bool conf_spfdnslog; #endif /* WITH_SPF */ _Bool conf_ignoreauthclients; _Bool conf_holdquarantinedmessages; @@ -1507,6 +1511,10 @@ dmarcf_config_load(struct config *data, struct dmarcf_config *conf, (void) config_get(data, "SPFSelfValidate", &conf->conf_spfselfvalidate, sizeof conf->conf_spfselfvalidate); + + (void) config_get(data, "LogSPFDNS", + &conf->conf_spfdnslog, + sizeof conf->conf_spfdnslog); #endif /* WITH_SPF */ (void) config_get(data, "RejectFailures", @@ -1993,6 +2001,9 @@ dmarcf_cleanup(SMFICTX *ctx) TRYFREE(dfc->mctx_dkimcanonhdr); TRYFREE(dfc->mctx_dkimcanonbody); +#if WITH_SPF + TRYFREE(dfc->mctx_spfdnslines); +#endif /* WITH_SPF */ if (dfc->mctx_hqhead != NULL) { @@ -2370,6 +2381,21 @@ mlfi_envfrom(SMFICTX *ctx, char **envfrom) dfc->mctx_spfresult = -1; dfc->mctx_spfmode = -1; + /* + ** Capture the job ID as early as it's available; mlfi_eom() retries + ** this later (its own comment: "in case it came down later than + ** expected, e.g. postfix") for MTAs that don't have it yet at + ** envfrom time, but that retry only works if something tried first. + */ + + { + char *i; + + i = dmarcf_getsymval(ctx, "i"); + if (i != NULL) + dfc->mctx_jobid = i; + } + dfc->mctx_histbuf = dmarcf_dstring_new(BUFRSZ, 0); if (dfc->mctx_histbuf == NULL) { @@ -3406,7 +3432,9 @@ mlfi_eom(SMFICTX *ctx) FALSE, human, sizeof human, - &used_mfrom); + &used_mfrom, + conf->conf_spfdnslog, + conf->conf_spfdnslog ? &dfc->mctx_spfdnslines : NULL); if (used_mfrom == TRUE) { use_domain = (char *)dfc->mctx_envdomain; @@ -4050,6 +4078,14 @@ mlfi_eom(SMFICTX *ctx) } } } + +#if WITH_SPF + if (spf_failed && dfc->mctx_spfdnslines != NULL) + { + dmarcf_dstring_cat(dfc->mctx_afrf, + (u_char *) dfc->mctx_spfdnslines); + } +#endif /* WITH_SPF */ } dmarcf_dstring_printf(dfc->mctx_afrf, diff --git a/opendmarc/opendmarc.conf.5.in b/opendmarc/opendmarc.conf.5.in index b186b59..f8c9cf0 100644 --- a/opendmarc/opendmarc.conf.5.in +++ b/opendmarc/opendmarc.conf.5.in @@ -322,6 +322,26 @@ potentially trigger further reports. Adding your own reporting address here breaks that loop. Matching is case-insensitive. The default is an empty list, meaning all inbound mail is recorded normally. +.TP +.I LogSPFDNS (Boolean) +When +.I SPFSelfValidate +is also set, populate the RFC 6591 +.I SPF-DNS +field of failure reports (RFC 9991) with the DNS records consulted +during self-validation -- the SPF policy record at each domain visited, +including those reached via +.I include: +or +.I redirect= +mechanisms. Has no effect unless +.I SPFSelfValidate +is also set. Building the DNS chain needed to capture this data has a +small extra cost per self-validated message, so it is not enabled +automatically alongside +.IR SPFSelfValidate . +The default is "false". + .TP .I MilterDebug (integer) Sets the debug level to be requested from the milter library. Six is the diff --git a/opendmarc/opendmarc.conf.sample b/opendmarc/opendmarc.conf.sample index 14d6a1e..52e304a 100644 --- a/opendmarc/opendmarc.conf.sample +++ b/opendmarc/opendmarc.conf.sample @@ -325,6 +325,20 @@ # # IgnoreMailTo dmarc-reports@example.com +## LogSPFDNS { true | false } +## default "false" +## +## When SPFSelfValidate is also set, populate the RFC 6591 SPF-DNS field +## of failure reports (RFC 9991) with the DNS records consulted during +## self-validation -- the SPF policy record at each domain visited, +## including those reached via include:/redirect= mechanisms. Has no +## effect unless SPFSelfValidate is also set. Building the DNS chain +## needed to capture this data has a small extra cost per +## self-validated message, so it is not enabled automatically alongside +## SPFSelfValidate. +# +# LogSPFDNS false + ## MilterDebug (integer) ## default 0 ## diff --git a/opendmarc/tests/Makefile.am b/opendmarc/tests/Makefile.am index 767d41c..bfc8425 100644 --- a/opendmarc/tests/Makefile.am +++ b/opendmarc/tests/Makefile.am @@ -11,7 +11,7 @@ endif if TEST_SPF if LIVE_TESTS -check_SCRIPTS += t-verify-self-spf +check_SCRIPTS += t-verify-self-spf t-verify-self-spf-report endif endif @@ -27,6 +27,8 @@ EXTRA_DIST = \ t-verify-received-spf-bad t-verify-received-spf-bad.conf \ t-verify-received-spf-bad.lua \ t-verify-self-spf t-verify-self-spf.conf t-verify-self-spf.lua \ + t-verify-self-spf-report t-verify-self-spf-report.conf \ + t-verify-self-spf-report.lua \ t-verify-multi-from-reject t-verify-multi-from-reject.conf \ t-verify-multi-from-reject.lua \ t-verify-multi-from-malformed t-verify-multi-from-malformed.conf \ @@ -35,4 +37,4 @@ EXTRA_DIST = \ t-verify-authservid-jobid.lua \ t-verify-canondata t-verify-canondata.conf t-verify-canondata.lua -MOSTLYCLEANFILES= +MOSTLYCLEANFILES= t-verify-self-spf-report.out diff --git a/opendmarc/tests/t-verify-self-spf-report b/opendmarc/tests/t-verify-self-spf-report new file mode 100755 index 0000000..a9199a1 --- /dev/null +++ b/opendmarc/tests/t-verify-self-spf-report @@ -0,0 +1,11 @@ +#!/bin/sh +# +# Test that SPF self-validation populates the RFC 6591 SPF-DNS field +# (RFC 9991) of a captured failure report, via LogSPFDNS. + +if [ x"$srcdir" = x"" ] +then + srcdir=`pwd` +fi + +miltertest -s $srcdir/t-verify-self-spf-report.lua diff --git a/opendmarc/tests/t-verify-self-spf-report.conf b/opendmarc/tests/t-verify-self-spf-report.conf new file mode 100644 index 0000000..5d1388f --- /dev/null +++ b/opendmarc/tests/t-verify-self-spf-report.conf @@ -0,0 +1,9 @@ +# isc.org self-SPF test with forensic report capture (RFC 9991 SPF-DNS field). +# isc.org publishes a real ruf= address and an SPF record with several +# include: mechanisms, so a genuine chain of SPF-record lookups gets logged. +Background No +SPFSelfValidate Yes +LogSPFDNS Yes +FailureReports Yes +FailureReportsOnNone Yes +ReportCommand cat > t-verify-self-spf-report.out diff --git a/opendmarc/tests/t-verify-self-spf-report.lua b/opendmarc/tests/t-verify-self-spf-report.lua new file mode 100644 index 0000000..65a98f5 --- /dev/null +++ b/opendmarc/tests/t-verify-self-spf-report.lua @@ -0,0 +1,129 @@ +-- Copyright (c) 2026, The Trusted Domain Project. All rights reserved. + +-- Test message from isc.org with an unauthorized client IP address. +-- +-- Confirms that SPF self-validation (RFC 9991) populates the RFC 6591 +-- SPF-DNS field of the resulting failure report, including entries for +-- domains reached via isc.org's SPF record's include: mechanisms, and +-- that reports aren't attempted at all when LogSPFDNS is off (this +-- capability has a real per-message cost, so it must stay opt-in). + +mt.echo("*** self-SPF test with LogSPFDNS report capture") + +-- setup +sock = "unix:" .. mt.getcwd() .. "/t-verify-self-spf-report.sock" +binpath = mt.getcwd() .. "/.." +outfile = mt.getcwd() .. "/t-verify-self-spf-report.out" +os.remove(outfile) +if os.getenv("srcdir") ~= nil then + mt.chdir(os.getenv("srcdir")) +end + +-- try to start the filter +mt.startfilter(binpath .. "/opendmarc", "-l", "-c", + "t-verify-self-spf-report.conf", "-p", sock) + +-- try to connect to it +conn = mt.connect(sock, 40, 0.05) +if conn == nil then + error("mt.connect() failed") +end + +-- send connection information +-- mt.negotiate() is called implicitly +if mt.conninfo(conn, "localhost2", "66.220.149.251") ~= nil then + error("mt.conninfo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.conninfo() unexpected reply") +end + +-- send envelope macros and sender data +-- HELO must precede any SMFIC_MAIL-scoped macro (see +-- t-verify-authservid-jobid.lua for why: real libmilter's HELO handler +-- clears macros stored for later protocol stages). +if mt.helo(conn, "localhost2") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + +mt.macro(conn, SMFIC_MAIL, "i", "t-verify-self-spf-report") +if mt.mailfrom(conn, "user@isc.org") ~= nil then + error("mt.mailfrom() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.mailfrom() unexpected reply") +end + +-- send headers +-- mt.rcptto() is called implicitly +if mt.header(conn, "From", "user@isc.org") ~= nil then + error("mt.header(From) failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(From) unexpected reply") +end +if mt.header(conn, "To", "user@example.com") ~= nil then + error("mt.header(To) failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(To) unexpected reply") +end +if mt.header(conn, "Date", "Tue, 22 Dec 2009 13:04:12 -0800") ~= nil then + error("mt.header(Date) failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(Date) unexpected reply") +end +if mt.header(conn, "Subject", "DMARC test") ~= nil then + error("mt.header(Subject) failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(Subject) unexpected reply") +end + +-- send EOH +if mt.eoh(conn) ~= nil then + error("mt.eoh() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.eoh() unexpected reply") +end + +-- end of message; let the filter react +if mt.eom(conn) ~= nil then + error("mt.eom() failed") +end +if mt.getreply(conn) ~= SMFIR_ACCEPT then + error("mt.eom() unexpected reply") +end + +mt.disconnect(conn) + +-- give the piped ReportCommand a moment to finish writing +os.execute("sleep 1") + +-- verify the captured report contains SPF-DNS lines for isc.org itself +-- and for at least one include: target +f = io.open(outfile, "r") +if f == nil then + error("failure report was not captured to " .. outfile) +end +report = f:read("*a") +f:close() + +if string.find(report, "SPF%-DNS: txt : isc%.org : \"v=spf1") == nil then + error("no SPF-DNS line for isc.org's own SPF record") +end + +if string.find(report, "SPF%-DNS: txt : .+%.customercenter%.net") == nil and + string.find(report, "SPF%-DNS: txt : .+%.shopify%.com") == nil and + string.find(report, "SPF%-DNS: txt : .+%.mcsv%.net") == nil and + string.find(report, "SPF%-DNS: txt : .+salesforce%.com") == nil +then + error("no SPF-DNS line for any include: target") +end + +os.remove(outfile) From dd95bce4e793ee297a1ec9ec425f57d742c32655 Mon Sep 17 00:00:00 2001 From: Dan Mahoney Date: Wed, 22 Jul 2026 00:54:33 -0700 Subject: [PATCH 8/8] fix: config directive allowlist gaps, test macro-ordering bug, configure.ac footgun Three unrelated bugs found and fixed while building/testing the SPF-DNS work, all discovered because it was the first time this cycle a new directive/build config actually got exercised for real rather than just compiled: opendmarc-config.h's dmarcf_config[] table -- a separate, hand-maintained allowlist config_check() validates every directive name against before any config_get() call ever runs -- was missing LogSPFDNS (this cycle), ReadCanonicalizedData (previous commit's DKIM-Canonicalized-Header/-Body work), and DMARCbisIgnorePct/DMARCbisWalkModeFallback (the earlier RFC 9989 t=/pct= work). Any of the four in a config file produced "configuration error: unrecognized parameter" and an immediate startup failure despite being fully wired up and documented. All four added. t-verify-authservid-jobid was failing whenever OpenDMARC actually linked a real SPF library instead of the built-in fallback evaluator. Root cause, confirmed with ktrace (byte-perfect macro delivery on the wire) and lldb (breakpoints in both OpenDMARC and, via its exported-but- internal symbols, real libmilter itself): the test sends its "i" (job ID) macro scoped to the MAIL stage, then calls mt.mailfrom() without ever calling mt.helo() first, so miltertest auto-inserts a filler HELO command at that point per its documented "fill in skipped steps" behavior -- after the macro was already sent. Real libmilter's st_helo() handler unconditionally clears any macros already stored for later protocol stages (mi_clr_macros(ctx, CI_HELO+1), a correct, by-design safeguard against stale macros surviving from a prior transaction on the same connection), which wipes the just-stored MAIL-stage macro before it's ever read back. Not a bug in miltertest, real libmilter, or OpenDMARC's C code -- purely a test-script ordering issue, and it was present identically in all eleven t-verify-*.lua tests that send the "i" macro this way; only this one noticed, since it's the only one that asserts on the delivered value rather than just the pass/fail outcome. Fixed by sequencing mt.helo() before the macro send everywhere. configure.ac: --with-spf2 (a very plausible but nonexistent flag -- SPF support is --with-spf, with --with-spf2-include/--with-spf2-lib separately selecting libspf2 over the fallback evaluator) is what actually caused the above to go unnoticed for as long as it did: it's silently ignored by autoconf's default unrecognized-option handling (a warning, not an error), so a build using it quietly falls back to the built-in evaluator instead of libspf2 with no indication anything's wrong. Added an explicit AC_ARG_WITH([spf2], ...) that fails configure immediately with a message pointing at the correct flags. Hit a real autoconf/m4 gotcha getting this right: an unquoted comma inside the AC_MSG_ERROR text was parsed by m4 as an argument separator to the enclosing AC_ARG_WITH, corrupting the generated shell code (a stray "else case e in" block and a syntax error deep in ./configure). Fixed with proper [[...]] quoting; verified by reading the generated configure script directly, not just re-running it. Verified together with the SPF-DNS commit: 11/11 tests pass on quark against real libmilter/libspf2, and both --with-spf2 (now errors) and the legitimate --with-spf --with-spf2-include=... --with-spf2-lib=... combination (still configures HAVE_SPF2_H/WITH_SPF correctly) were exercised directly. --- CHANGES-202605.md | 8 +- DMARCBIS-REMAINING-WORK.md | 86 ++++++++++++++----- configure.ac | 13 +++ opendmarc/opendmarc-config.h | 4 + opendmarc/tests/t-verify-authservid-jobid.lua | 15 ++++ opendmarc/tests/t-verify-canondata.lua | 11 ++- .../tests/t-verify-multi-from-malformed.lua | 11 ++- .../tests/t-verify-multi-from-reject.lua | 11 ++- opendmarc/tests/t-verify-nodata-reject.lua | 11 ++- opendmarc/tests/t-verify-nodata.lua | 11 ++- opendmarc/tests/t-verify-received-spf-bad.lua | 11 ++- .../tests/t-verify-received-spf-good.lua | 11 ++- opendmarc/tests/t-verify-self-spf.lua | 11 ++- opendmarc/tests/t-verify-unspec.lua | 11 ++- 14 files changed, 191 insertions(+), 34 deletions(-) diff --git a/CHANGES-202605.md b/CHANGES-202605.md index 115e9c6..e34b6c2 100644 --- a/CHANGES-202605.md +++ b/CHANGES-202605.md @@ -67,8 +67,8 @@ Significant gaps between the generated aggregate report XML and RFC 7489 require - **External destination verification extended to `ruf=`**: The `_report._dmarc` DNS verification described above now also applies to failure-report (`ruf=`) recipients in `opendmarc-reports --forensic`, per RFC 9991 §5 (which reuses RFC 9990 §4's procedure verbatim, substituting `ruf` for `rua`). Verified against the `Reported-Domain` header the milter already attaches to forensic reports; skipped (not enforced) when that header is absent. As with the other `--forensic`-only checks below, this has no effect when `ReportCommand` pipes directly to `sendmail` rather than through `opendmarc-reports`. - **Rate-limiting for outgoing failure reports**: New `ForensicRateLimit`/`ForensicRateWindow` options (`opendmarc-reports.conf` or `--forensic-rate-limit`/`--forensic-rate-window`) cap how many failure reports `opendmarc-reports --forensic` will send to a given recipient address within a rolling window, per RFC 9991 §2/§8.1's requirement to prevent a bad actor from using deliberately-triggered DMARC failures to flood a `ruf=` destination. Disabled by default. Backed by a new `forensic_sent` table (`db/update-db-schema.mysql`). - **`psd=y` now excludes `ruf=`**: Per RFC 9991 §2, `ruf=` in a DMARC Policy Record with `psd=y` is no longer honored (no agreement mechanism exists here to opt back in, per the RFC's own carve-out). Operator-configured `AuthFailureReportsBcc`-style delivery is unaffected, since it doesn't come from the record. New `opendmarc_policy_fetch_psd()` libopendmarc accessor. -- **`DKIM-Canonicalized-Header`/`-Body` ARF header fields**: New opt-in `ReadCanonicalizedData` option. This codebase never verifies DKIM locally, so it has no canonicalized bytes of its own; instead, when set (and `FailureReports` is also on), it looks for `X-DKIM-Canonicalized-Header`/`-Body` staging headers added by an upstream signing filter (OpenDKIM's new `AddCanonicalizedData` directive, `trusteddomainproject/OpenDKIM` PR #423) and, when a DKIM signature is the one blamed for a DMARC failure, copies their base64 payload into the RFC 6591 `DKIM-Canonicalized-Header`/`DKIM-Canonicalized-Body` fields of the failure report -- re-folded to this codebase's own ARF-body convention rather than passed through verbatim. The staging headers are always stripped before final delivery regardless of this setting, so a signing filter with the upstream directive enabled can never leak them to a recipient even if this option is off. No size cap is applied to the canonicalized body data; large messages will produce correspondingly large failure reports. Completes the RFC 9991 gap noted in `DMARCBIS-REMAINING-WORK.md` (SPF-DNS remains open, blocked on libspf2 not exposing its DNS record chain). -- **Deferred**: `SPF-DNS` (would require capturing every DNS record consulted during SPF evaluation, including `include:`/`redirect=` sub-queries — not exposed by libspf2's public API, unlike OpenDMARC's own unused-in-production built-in SPF evaluator, which already tracks this internally) and `DKIM-Canonicalized-Header`/`-Body` (no DKIM verification library is linked anywhere in this codebase, so there are no canonicalized bytes to expose) are both out of scope here — real new work, not wiring, tracked as follow-ups. +- **`DKIM-Canonicalized-Header`/`-Body` ARF header fields**: New opt-in `ReadCanonicalizedData` option. This codebase never verifies DKIM locally, so it has no canonicalized bytes of its own; instead, when set (and `FailureReports` is also on), it looks for `X-DKIM-Canonicalized-Header`/`-Body` staging headers added by an upstream signing filter (OpenDKIM's new `AddCanonicalizedData` directive, `trusteddomainproject/OpenDKIM` PR #423) and, when a DKIM signature is the one blamed for a DMARC failure, copies their base64 payload into the RFC 6591 `DKIM-Canonicalized-Header`/`DKIM-Canonicalized-Body` fields of the failure report -- re-folded to this codebase's own ARF-body convention rather than passed through verbatim. The staging headers are always stripped before final delivery regardless of this setting, so a signing filter with the upstream directive enabled can never leak them to a recipient even if this option is off. No size cap is applied to the canonicalized body data; large messages will produce correspondingly large failure reports. +- **`SPF-DNS` ARF header field**: New opt-in `LogSPFDNS` option (only meaningful alongside `SPFSelfValidate`). libspf2's public API doesn't expose the DNS records it consults during SPF evaluation, so this required real new code rather than wiring: a custom `SPF_dns_server_t` "DNS layer" (`libopendmarc/opendmarc_spf_dns_log.c`), spliced into libspf2's own pluggable resolver chain above its caching layer so every SPF-record lookup for a self-validated message is seen regardless of cache hits, including domains reached via `include:`/`redirect=`. Passes every query through unchanged (evaluation behavior is untouched) and, for TXT/SPF-type queries whose content is itself an SPF policy record (starts `v=spf1`, or any content for the historic RR type 99), appends an RFC 6591 `SPF-DNS: txt|spf : domain : "content"` line. Off by default -- building the logging DNS chain has a small extra cost per self-validated message, so it isn't enabled automatically alongside `SPFSelfValidate`. Completes the RFC 9991 gap noted in `DMARCBIS-REMAINING-WORK.md`. --- @@ -168,6 +168,10 @@ CREATE TABLE IF NOT EXISTS forensic_sent ( - **`RequiredFrom` option added**: New boolean option that rejects messages lacking a From: field from which a domain can be extracted. Unlike `RequiredHeaders` (which enforces all RFC5322 header count restrictions), `RequiredFrom` enforces only the From field check, making it suitable for deployments where full RFC5322 compliance would reject too many legitimate messages. Prevents attackers from omitting the From header to evade DMARC evaluation. (#343) - **`NoReportsList` sample config description corrected**: `opendmarc.conf.sample` still described the pre-#404 two-way semantics (full address, or bare domain to suppress all addresses at that domain). Updated to match the three-way semantics (`user@example.com`, `@example.com`, bare sending-domain) already documented in `opendmarc.conf.5.in`, and clarified that `ReportCommand` can be pointed at `opendmarc-reports --forensic` instead of `sendmail -t`. (#424) +- **Four config directives silently rejected at startup**: `opendmarc-config.h`'s `dmarcf_config[]` table -- a separate, hand-maintained allowlist `config_check()` validates every directive name against before any `config_get()` call ever runs -- was missing `DMARCbisIgnorePct` and `DMARCbisWalkModeFallback` (both from the RFC 9989 `t=`/`pct=` work) and `ReadCanonicalizedData` (from this cycle's DKIM-Canonicalized-Header/-Body work). Any of the four in a config file produced "configuration error: unrecognized parameter" and an immediate startup failure, even though each was fully wired up and documented. Found while adding `LogSPFDNS` (also missing) and live-testing it, since that's the first time this cycle a new directive was actually exercised via a real parsed config file rather than just compiled. All four added. +- **`t-verify-authservid-jobid` test fixed, and the same latent bug fixed in every other test**: was failing whenever OpenDMARC actually linked a real SPF library rather than the built-in fallback evaluator (see `DMARCBIS-REMAINING-WORK.md` for the full `ktrace`/`lldb` root-cause writeup). Not a bug in the filter itself -- the test sent its `i` (job ID) macro before an explicit `mt.helo()`, and real libmilter's HELO handler correctly-but-inconveniently clears macros for all later protocol stages as a stale-data safeguard, wiping it out before it could be read back. The identical ordering issue existed in all eleven `t-verify-*.lua` tests that send this macro; only this one noticed, since it's the only one that asserts on the delivered value. Fixed by sequencing `mt.helo()` before the macro send everywhere. +- **`mlfi_envfrom()` now captures the job ID as soon as it's available**: `mlfi_eom()`'s existing job-ID lookup carried a comment noting it was a retry "in case it came down later than expected (e.g. postfix)" -- but no earlier attempt actually existed anywhere in the code for it to be retrying. Added one in `mlfi_envfrom()`, matching the documented intent. +- **`--with-spf2` is now a hard `configure` error**: this (nonexistent) flag was the proximate cause of the `t-verify-authservid-jobid` investigation even starting -- SPF support is actually enabled via `--with-spf`, with `--with-spf2-include`/`--with-spf2-lib` selecting libspf2 over the built-in fallback evaluator, but `--with-spf2` looks like a very plausible flag name and autoconf silently ignores unrecognized `--with-*` options by default (a warning, not an error). `configure` now rejects it explicitly with a message pointing at the correct flags. --- diff --git a/DMARCBIS-REMAINING-WORK.md b/DMARCBIS-REMAINING-WORK.md index 36ce65c..36eb089 100644 --- a/DMARCBIS-REMAINING-WORK.md +++ b/DMARCBIS-REMAINING-WORK.md @@ -39,7 +39,18 @@ specifically (all resolved) and are not duplicated here. re-folded to this codebase's own ARF-body convention. Staging headers are always stripped before final delivery regardless of the setting. See CHANGES-202605.md for the full writeup; cross-project design - history preserved below. SPF-DNS remains open. + history preserved below. +- **RFC 9991 `SPF-DNS` ARF field**: new opt-in `LogSPFDNS` option (only + meaningful with `SPFSelfValidate`). Real new code, as anticipated below + -- libspf2 has a pluggable "DNS layer" system built exactly for this + kind of interception (`SPF_dns_server_t`, chainable via `layer_below`); + a new logging layer (`libopendmarc/opendmarc_spf_dns_log.c`) is spliced + in above libspf2's own caching layer so it sees every SPF-record lookup + regardless of cache hits, including domains reached via + `include:`/`redirect=`, without altering evaluation behavior. This + closes RFC 9991's SPF-DNS gap entirely; both fields originally listed + as blocked in this section are now done. See CHANGES-202605.md for the + full writeup. ## Remaining @@ -72,29 +83,58 @@ specifically (all resolved) and are not duplicated here. ### RFC 9991 failure reporting -- **`SPF-DNS` ARF field** (RFC 6591, required per-record-consulted): - needs the actual DNS record content (RRTYPE + domain + record text) for - every record consulted during SPF evaluation, including `include:`/ - `redirect=` sub-queries. libspf2's public API (what's actually linked in - production, confirmed via `spf_response.h` on quark) doesn't expose this - chain — only a human-readable summary. OpenDMARC's own built-in fallback - SPF evaluator (`opendmarc_spf.c`, used only when libspf2 isn't linked) - already tracks it internally via `stack[s].spf`/`stack[s].domain`, but - that's not the path in use. Real fix: a custom `SPF_dns_*`-compatible - logging layer chained onto the real resolver, so queries can be captured - without patching libspf2 itself. Self-contained but real new code. +Both RFC 6591 fields originally tracked here (`DKIM-Canonicalized-Header`/ +`-Body` and `SPF-DNS`) have shipped -- see "Done" above. Nothing remaining +in this section. -`DKIM-Canonicalized-Header`/`-Body` (the other RFC 6591 field originally -listed here as blocked) shipped -- see "Done" above. Design history: it -turned out to need less new work than expected, because OpenDKIM already -built the underlying capture mechanism for its own unrelated `SendReports` -feature (`dkimf_sigreport()`, `opendkim/opendkim.c:10098`, using -`dkim_sig_getreportinfo()` and `dkimf_base64_encode_file()`) -- confirmed -by reading the OpenDKIM source directly rather than assuming a new -DKIM-verification subsystem was required. The OpenDKIM-side half was -handed off via `RFC9991-CANONICALIZED-DATA-HANDOFF.md` in that repo and -landed as `AddCanonicalizedData` (PR #423); this repo's half landed as -`ReadCanonicalizedData` on `feat/rfc9991-forensic-reporting`. +### Bugs found along the way + +- **`t-verify-authservid-jobid` test-ordering bug, fixed**: found while + live-testing the `LogSPFDNS` work (that's what prompted actually + getting a real-libspf2 build going, via `--with-spf + --with-spf2-include=... --with-spf2-lib=...` -- the environment used + for most of this DMARCbis work had been silently building against the + built-in fallback SPF evaluator instead, an easy mistake since + `--with-spf2` is a *different*, unrecognized flag that `configure` + silently ignores rather than erroring on). Root-caused with `ktrace` + (confirmed byte-perfect wire delivery of the macro) and `lldb` + (breakpoints in both OpenDMARC and, via its exported-but-internal + symbols, real libmilter itself): the test sends the `i` (job ID) macro + scoped to the MAIL stage, then calls `mt.mailfrom()` without ever + having called `mt.helo()` -- so miltertest auto-inserts a filler HELO + command at that point, per its documented "fill in skipped steps" + behavior. Real libmilter's `st_helo()` handler unconditionally clears + any macros already stored for *later* protocol stages + (`mi_clr_macros(ctx, CI_HELO+1)`, a correct, by-design safeguard + against stale macros surviving from a prior transaction on the same + connection) -- which wipes the just-stored MAIL-stage macro before it's + ever read back, since it arrived earlier than the (delayed) HELO it's + nominally supposed to follow. Not a bug in miltertest, real libmilter, + or OpenDMARC's C code -- purely a test-script ordering issue, present + identically in every test in this suite that sends the `i` macro this + way (all of them), just never noticed elsewhere since no other test + asserts on the delivered value. Fixed by adding an explicit + `mt.helo()` call before the macro send in every affected test file + (all eleven `t-verify-*.lua` files that send the `i` macro), not just + the one that was actually failing. +- **`--with-spf2` now a hard configure error**: the flag that caused the + above (silently building against the built-in fallback SPF evaluator + instead of real libspf2, since `--with-spf` is the real flag and + `--with-spf2` doesn't exist) now fails `configure` immediately with a + message pointing at the correct flags, instead of the default + autoconf behavior of a warning + silent continue. Verified both that + `--with-spf2` now errors and that the legitimate + `--with-spf --with-spf2-include=... --with-spf2-lib=...` combination + still configures `HAVE_SPF2_H`/`WITH_SPF` correctly. +- **Four config directives silently rejected at startup**: fixed in the + same session (`opendmarc-config.h`'s `dmarcf_config[]` validation table + was missing `DMARCbisIgnorePct`, `DMARCbisWalkModeFallback`, + `ReadCanonicalizedData`, and `LogSPFDNS`) -- see CHANGES-202605.md. Worth + a standing reminder: this table is hand-maintained and separate from + every `config_get()` call site, so a new directive silently fails at + startup instead of a compile error until someone actually parses a + config file containing it, which apparently hadn't happened for three + of these four since they were introduced. ### Open decision, not just missing code diff --git a/configure.ac b/configure.ac index 577b4e0..e12de56 100644 --- a/configure.ac +++ b/configure.ac @@ -360,6 +360,19 @@ AC_ARG_WITH([spf], AC_DEFINE(WITH_SPF, 1, [Define to 1 if you want SPF support.]), []) +dnl There is no "--with-spf2" flag -- SPF support is enabled with +dnl --with-spf, and libspf2 (as opposed to the built-in fallback +dnl evaluator) is selected by additionally pointing at it via +dnl --with-spf2-include/--with-spf2-lib below. Autoconf silently ignores +dnl unrecognized --with-* options by default (a warning, not an error), +dnl so this exact, easy typo has previously caused builds to silently +dnl fall back to the built-in evaluator instead of libspf2. Fail loudly +dnl instead. +AC_ARG_WITH([spf2], + AS_HELP_STRING([--with-spf2], [(invalid; see --with-spf, --with-spf2-include, --with-spf2-lib)]), + [AC_MSG_ERROR([[--with-spf2 is not a valid option. Use --with-spf to enable SPF checking; use --with-spf2-include=path and --with-spf2-lib=path to select libspf2 over the built-in fallback evaluator.]])], + []) + AC_ARG_WITH([spf2-include], AS_HELP_STRING([--with-spf2-include], [path to libspf2 includes]), SPF2_INCLUDE="$withval", diff --git a/opendmarc/opendmarc-config.h b/opendmarc/opendmarc-config.h index 0e4d67a..c779614 100644 --- a/opendmarc/opendmarc-config.h +++ b/opendmarc/opendmarc-config.h @@ -27,6 +27,8 @@ struct configdef dmarcf_config[] = { "ChangeRootDirectory", CONFIG_TYPE_STRING, FALSE }, { "CopyFailuresTo", CONFIG_TYPE_STRING, FALSE }, { "DMARCbisWalkMode", CONFIG_TYPE_STRING, FALSE }, + { "DMARCbisWalkModeFallback", CONFIG_TYPE_STRING, FALSE }, + { "DMARCbisIgnorePct", CONFIG_TYPE_BOOLEAN, FALSE }, { "DNSTimeout", CONFIG_TYPE_INTEGER, FALSE }, { "DomainWhitelist", CONFIG_TYPE_STRING, FALSE }, { "DomainWhitelistFile", CONFIG_TYPE_STRING, FALSE }, @@ -42,10 +44,12 @@ struct configdef dmarcf_config[] = { "IgnoreHosts", CONFIG_TYPE_STRING, FALSE }, { "IgnoreMailFrom", CONFIG_TYPE_STRING, FALSE }, { "IgnoreMailTo", CONFIG_TYPE_STRING, FALSE }, + { "LogSPFDNS", CONFIG_TYPE_BOOLEAN, FALSE }, { "MilterDebug", CONFIG_TYPE_INTEGER, FALSE }, { "NoReportsList", CONFIG_TYPE_STRING, FALSE }, { "PidFile", CONFIG_TYPE_STRING, FALSE }, { "PublicSuffixList", CONFIG_TYPE_STRING, FALSE }, + { "ReadCanonicalizedData", CONFIG_TYPE_BOOLEAN, FALSE }, { "RecordAllMessages", CONFIG_TYPE_BOOLEAN, FALSE }, { "RequiredHeaders", CONFIG_TYPE_BOOLEAN, FALSE }, { "RequiredFrom", CONFIG_TYPE_BOOLEAN, FALSE }, diff --git a/opendmarc/tests/t-verify-authservid-jobid.lua b/opendmarc/tests/t-verify-authservid-jobid.lua index 66b8cf5..52ad5d7 100644 --- a/opendmarc/tests/t-verify-authservid-jobid.lua +++ b/opendmarc/tests/t-verify-authservid-jobid.lua @@ -29,6 +29,21 @@ if mt.getreply(conn) ~= SMFIR_CONTINUE then error("mt.conninfo() unexpected reply") end +-- HELO must precede the SMFIC_MAIL-scoped "i" macro below: real libmilter's +-- st_helo() unconditionally clears any macros already stored for later +-- protocol stages (mi_clr_macros(ctx, CI_HELO+1), a normal safeguard +-- against stale macros surviving from a prior transaction on the same +-- connection). mt.mailfrom() auto-inserts a HELO step if one hasn't +-- happened yet, so sending the "i" macro before an explicit mt.helo() +-- meant it was silently wiped out by that auto-inserted HELO before ever +-- being read back. +if mt.helo(conn, "localhost2") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + mt.macro(conn, SMFIC_MAIL, "i", "testjobid") if mt.mailfrom(conn, "user@trusteddomain.org") ~= nil then error("mt.mailfrom() failed") diff --git a/opendmarc/tests/t-verify-canondata.lua b/opendmarc/tests/t-verify-canondata.lua index c740e92..5deb45c 100644 --- a/opendmarc/tests/t-verify-canondata.lua +++ b/opendmarc/tests/t-verify-canondata.lua @@ -36,7 +36,16 @@ if mt.getreply(conn) ~= SMFIR_CONTINUE then end -- send envelope macros and sender data --- mt.helo() is called implicitly +-- HELO must precede any SMFIC_MAIL-scoped macro (see +-- t-verify-authservid-jobid.lua for why: real libmilter's HELO handler +-- clears macros stored for later protocol stages). +if mt.helo(conn, "localhost2") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + mt.macro(conn, SMFIC_MAIL, "i", "t-verify-canondata") if mt.mailfrom(conn, "user@example.com") ~= nil then error("mt.mailfrom() failed") diff --git a/opendmarc/tests/t-verify-multi-from-malformed.lua b/opendmarc/tests/t-verify-multi-from-malformed.lua index a45c40a..cf92942 100644 --- a/opendmarc/tests/t-verify-multi-from-malformed.lua +++ b/opendmarc/tests/t-verify-multi-from-malformed.lua @@ -31,7 +31,16 @@ if mt.getreply(conn) ~= SMFIR_CONTINUE then end -- send envelope macros and sender data --- mt.helo() is called implicitly +-- HELO must precede any SMFIC_MAIL-scoped macro (see +-- t-verify-authservid-jobid.lua for why: real libmilter's HELO handler +-- clears macros stored for later protocol stages). +if mt.helo(conn, "localhost2") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + mt.macro(conn, SMFIC_MAIL, "i", "t-verify-multi-from-malformed") if mt.mailfrom(conn, "user@paypal.com") ~= nil then error("mt.mailfrom() failed") diff --git a/opendmarc/tests/t-verify-multi-from-reject.lua b/opendmarc/tests/t-verify-multi-from-reject.lua index 7a9d6ab..0462e6a 100644 --- a/opendmarc/tests/t-verify-multi-from-reject.lua +++ b/opendmarc/tests/t-verify-multi-from-reject.lua @@ -31,7 +31,16 @@ if mt.getreply(conn) ~= SMFIR_CONTINUE then end -- send envelope macros and sender data --- mt.helo() is called implicitly +-- HELO must precede any SMFIC_MAIL-scoped macro (see +-- t-verify-authservid-jobid.lua for why: real libmilter's HELO handler +-- clears macros stored for later protocol stages). +if mt.helo(conn, "localhost2") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + mt.macro(conn, SMFIC_MAIL, "i", "t-verify-multi-from-reject") if mt.mailfrom(conn, "user@paypal.com") ~= nil then error("mt.mailfrom() failed") diff --git a/opendmarc/tests/t-verify-nodata-reject.lua b/opendmarc/tests/t-verify-nodata-reject.lua index 3f6acf7..ef8b5be 100644 --- a/opendmarc/tests/t-verify-nodata-reject.lua +++ b/opendmarc/tests/t-verify-nodata-reject.lua @@ -36,7 +36,16 @@ if mt.getreply(conn) ~= SMFIR_CONTINUE then end -- send envelope macros and sender data --- mt.helo() is called implicitly +-- HELO must precede any SMFIC_MAIL-scoped macro (see +-- t-verify-authservid-jobid.lua for why: real libmilter's HELO handler +-- clears macros stored for later protocol stages). +if mt.helo(conn, "localhost2") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + mt.macro(conn, SMFIC_MAIL, "i", "t-verify-nodata-reject") if mt.mailfrom(conn, "user@paypal.com") ~= nil then error("mt.mailfrom() failed") diff --git a/opendmarc/tests/t-verify-nodata.lua b/opendmarc/tests/t-verify-nodata.lua index 1eaf9dd..13d0531 100644 --- a/opendmarc/tests/t-verify-nodata.lua +++ b/opendmarc/tests/t-verify-nodata.lua @@ -35,7 +35,16 @@ if mt.getreply(conn) ~= SMFIR_CONTINUE then end -- send envelope macros and sender data --- mt.helo() is called implicitly +-- HELO must precede any SMFIC_MAIL-scoped macro (see +-- t-verify-authservid-jobid.lua for why: real libmilter's HELO handler +-- clears macros stored for later protocol stages). +if mt.helo(conn, "localhost2") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + mt.macro(conn, SMFIC_MAIL, "i", "t-verify-nodata") if mt.mailfrom(conn, "user@paypal.com") ~= nil then error("mt.mailfrom() failed") diff --git a/opendmarc/tests/t-verify-received-spf-bad.lua b/opendmarc/tests/t-verify-received-spf-bad.lua index 4fbeeec..ce0a17c 100644 --- a/opendmarc/tests/t-verify-received-spf-bad.lua +++ b/opendmarc/tests/t-verify-received-spf-bad.lua @@ -35,7 +35,16 @@ if mt.getreply(conn) ~= SMFIR_CONTINUE then end -- send envelope macros and sender data --- mt.helo() is called implicitly +-- HELO must precede any SMFIC_MAIL-scoped macro (see +-- t-verify-authservid-jobid.lua for why: real libmilter's HELO handler +-- clears macros stored for later protocol stages). +if mt.helo(conn, "localhost2") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + mt.macro(conn, SMFIC_MAIL, "i", "t-verify-received-spf-bad") if mt.mailfrom(conn, "user@trusteddomain.org") ~= nil then error("mt.mailfrom() failed") diff --git a/opendmarc/tests/t-verify-received-spf-good.lua b/opendmarc/tests/t-verify-received-spf-good.lua index 9e0eee1..a0c6032 100644 --- a/opendmarc/tests/t-verify-received-spf-good.lua +++ b/opendmarc/tests/t-verify-received-spf-good.lua @@ -37,7 +37,16 @@ if mt.getreply(conn) ~= SMFIR_CONTINUE then end -- send envelope macros and sender data --- mt.helo() is called implicitly +-- HELO must precede any SMFIC_MAIL-scoped macro (see +-- t-verify-authservid-jobid.lua for why: real libmilter's HELO handler +-- clears macros stored for later protocol stages). +if mt.helo(conn, "localhost2") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + mt.macro(conn, SMFIC_MAIL, "i", "t-verify-received-spf-good") if mt.mailfrom(conn, "user@trusteddomain.org") ~= nil then error("mt.mailfrom() failed") diff --git a/opendmarc/tests/t-verify-self-spf.lua b/opendmarc/tests/t-verify-self-spf.lua index 627f70d..c9263e8 100644 --- a/opendmarc/tests/t-verify-self-spf.lua +++ b/opendmarc/tests/t-verify-self-spf.lua @@ -35,7 +35,16 @@ if mt.getreply(conn) ~= SMFIR_CONTINUE then end -- send envelope macros and sender data --- mt.helo() is called implicitly +-- HELO must precede any SMFIC_MAIL-scoped macro (see +-- t-verify-authservid-jobid.lua for why: real libmilter's HELO handler +-- clears macros stored for later protocol stages). +if mt.helo(conn, "localhost2") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + mt.macro(conn, SMFIC_MAIL, "i", "t-verify-self-spf") if mt.mailfrom(conn, "user@trusteddomain.org") ~= nil then error("mt.mailfrom() failed") diff --git a/opendmarc/tests/t-verify-unspec.lua b/opendmarc/tests/t-verify-unspec.lua index 5f6da4a..303b2f6 100644 --- a/opendmarc/tests/t-verify-unspec.lua +++ b/opendmarc/tests/t-verify-unspec.lua @@ -38,7 +38,16 @@ if mt.getreply(conn) ~= SMFIR_CONTINUE then end -- send envelope macros and sender data --- mt.helo() is called implicitly +-- HELO must precede any SMFIC_MAIL-scoped macro (see +-- t-verify-authservid-jobid.lua for why: real libmilter's HELO handler +-- clears macros stored for later protocol stages). +if mt.helo(conn, "localhost") ~= nil then + error("mt.helo() failed") +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.helo() unexpected reply") +end + mt.macro(conn, SMFIC_MAIL, "i", "t-verify-unspec") if mt.mailfrom(conn, "user@paypal.com") ~= nil then error("mt.mailfrom() failed")