From 6678c38204631c93541f13990d3b74948fb24a45 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 18 Feb 2018 11:25:33 +0000 Subject: [PATCH] DMARC: fix result reporting when a DKIM ACL overrides the verify result. Bug 2236 --- doc/doc-txt/ChangeLog | 4 ++++ src/src/dkim.c | 29 ++++++++++++++++++++++++----- src/src/dmarc.c | 2 +- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index c5a506c16..4febdc93e 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -110,6 +110,10 @@ JH/21 Fix memory leak during multi-message connections using STARTTLS. A buffer was allocated for every new TLS startup, meaning one per message. Fix by only allocating once (OpenSSL) or freeing on TLS-close (GnuTLS). +JH/22 Bug 2236: When a DKIM verification result is overridden by ACL, DMARC + reported the original. Fix to report (as far as possible) the ACL + result replacing the original. + Exim version 4.90 ----------------- diff --git a/src/src/dkim.c b/src/src/dkim.c index 571586130..4fe4a11ab 100644 --- a/src/src/dkim.c +++ b/src/src/dkim.c @@ -151,11 +151,7 @@ uschar * s; if (!sig) return; -if ( dkim_verify_status - && ( dkim_verify_status != dkim_exim_expand_query(DKIM_VERIFY_STATUS) - || dkim_verify_reason != dkim_exim_expand_query(DKIM_VERIFY_REASON) - ) ) - sig->verify_status |= PDKIM_VERIFY_POLICY; +/* Remember the domain for the first pass result */ if ( !dkim_verify_overall && dkim_verify_status @@ -164,8 +160,31 @@ if ( !dkim_verify_overall ) dkim_verify_overall = string_copy(sig->domain); +/* Rewrite the sig result if the ACL overrode it. This is only +needed because the DMARC code (sigh) peeks at the dkim sigs. +Mark the sig for this having been done. */ + +if ( dkim_verify_status + && ( dkim_verify_status != dkim_exim_expand_query(DKIM_VERIFY_STATUS) + || dkim_verify_reason != dkim_exim_expand_query(DKIM_VERIFY_REASON) + ) ) + { /* overridden by ACL */ + sig->verify_ext_status = -1; + if (Ustrcmp(dkim_verify_status, US"fail") == 0) + sig->verify_status = PDKIM_VERIFY_POLICY | PDKIM_VERIFY_FAIL; + else if (Ustrcmp(dkim_verify_status, US"invalid") == 0) + sig->verify_status = PDKIM_VERIFY_POLICY | PDKIM_VERIFY_INVALID; + else if (Ustrcmp(dkim_verify_status, US"none") == 0) + sig->verify_status = PDKIM_VERIFY_POLICY | PDKIM_VERIFY_NONE; + else if (Ustrcmp(dkim_verify_status, US"pass") == 0) + sig->verify_status = PDKIM_VERIFY_POLICY | PDKIM_VERIFY_PASS; + else + sig->verify_status = -1; + } + if (!LOGGING(dkim_verbose)) return; + logmsg = string_catn(NULL, US"DKIM: ", 6); if (!(s = sig->domain)) s = US""; logmsg = string_append(logmsg, 2, "d=", s); diff --git a/src/src/dmarc.c b/src/src/dmarc.c index 37ac9c555..0032afe87 100644 --- a/src/src/dmarc.c +++ b/src/src/dmarc.c @@ -330,7 +330,7 @@ if (!dmarc_abort && !sender_host_authenticated) while (sig) { int dkim_result, dkim_ares_result, vs, ves; - vs = sig->verify_status; + vs = sig->verify_status & ~PDKIM_VERIFY_POLICY; ves = sig->verify_ext_status; dkim_result = vs == PDKIM_VERIFY_PASS ? DMARC_POLICY_DKIM_OUTCOME_PASS : vs == PDKIM_VERIFY_FAIL ? DMARC_POLICY_DKIM_OUTCOME_FAIL : -- 2.30.2