RFC 9991 failure reporting: Identity-Alignment, DKIM-Canonicalized-Header/-Body, ruf= verification/rate-limiting#439
Merged
thegushi merged 9 commits intoJul 22, 2026
Conversation
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.
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.
…orting 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 <pct>, 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.
…ion/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 trusteddomainproject#392's rua= external-destination verification (_report._dmarc DNS lookup) was silently wiped by PR trusteddomainproject#394's commit ~5 hours later (built from a pre-trusteddomainproject#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.
Consolidates what's left across RFC 9989/9990/9991 into one file, since issue trusteddomainproject#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 trusteddomainproject#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.
…nicalizedData 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 trusteddomainproject#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).
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.
…ure.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.
Conflicts in reports/opendmarc-reports.in and .8.in: develop's trusteddomainproject#437/trusteddomainproject#438 are squash-merged copies of commits this branch already had (from before its DMARCbis work started) under different, non-PR-suffixed hashes -- same author, same message, same content, just two different paths to develop's tip vs. this branch's history. This branch's side of every conflicting hunk is a strict superset (log_skip()'s syslog integration, the "rate limit" loud_skip() reason, and the rua=/ruf= external- destination verification block, none of which exist yet on develop) of develop's side, so resolved by taking this branch's version throughout. Verified on quark: perl -c on the generated opendmarc-reports script is clean, and the full live test suite (11/11) still passes post-merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the RFC 9991 (DMARC failure reporting) gap tracked in
DMARCBIS-REMAINING-WORK.md:Identity-AlignmentandDKIM-Domain/-Identity/-SelectorARF fields (S4): added to the milter-generated failure report when DKIM alignment fails and a signature was seen via a trusted upstreamAuthentication-Resultsheader.DKIM-Canonicalized-Header/-BodyARF fields (RFC 6591): this codebase never verifies DKIM locally, so a new opt-inReadCanonicalizedDataoption instead readsX-DKIM-Canonicalized-Header/-Bodystaging headers added by an upstream OpenDKIM's newAddCanonicalizedDatadirective (trusteddomainproject/OpenDKIM#423), correlates them to the blamed signature viad=/s=, and copies the base64 payload into the real fields. The staging headers are always stripped before final delivery regardless of this setting.ruf=external destination verification + rate-limiting (S2/S5/S8.1) inopendmarc-reports --forensic, reusing/generalizing the existingrua=_report._dmarcDNS verification.psd=yexcludesruf=(S2).np/testing/discovery_methodthrough aggregate reporting, and a couple of smaller report-skip logging improvements (loud_skip()to syslog, StaleMARC/suppressions-DB skip logging) picked up along the way.SPF-DNS (the other RFC 6591 field) remains out of scope -- libspf2's public API doesn't expose the DNS record chain it consults, so it needs real library-level work rather than wiring; tracked separately.
Also fixes two unrelated bugs found while working on this:
rua=external-destination verification had been wiped by a later commit built from a pre-Add external report destination verification via _report._dmarc DNS lookup (openSUSE z03) #392 checkout, despite CHANGES still documenting it as shipped. Restored and generalized toruf=.opendmarc_policy_fetch_np/_psd/_discovery_method(RFC 9990) were defined but never prototyped indmarc.h.in, relying on implicit-declaration behavior.Test plan
libmilter/libspf2: 9/9 pass, including all pre-existing tests (no regressions).t-verify-canondatatest covers the unconditional header-stripping behavior.