opendmarc/opendmarc.c:mlfi_eom(): respect AuthservIDWithJobID when adding AR: authservid; spf= header

[dilyanpalauzov]

In particular when SPFIgnoreResults, SPFSelfValidate and AuthservIDWithJobID are enabled:

diff --git a/opendmarc/opendmarc.c b/opendmarc/opendmarc.c
--- a/opendmarc/opendmarc.c
+++ b/opendmarc/opendmarc.c
@@ -2918,14 +2918,22 @@ mlfi_eom(SMFICTX *ctx)
                        if (spf_mode == DMARC_POLICY_SPF_ORIGIN_HELO)
                        {
                                snprintf(header, sizeof header,
-                                        "%s; spf=%s smtp.helo=%s",
-                                        authservid, pass_fail, use_domain);
+                                        "%s%s%s; spf=%s smtp.helo=%s",
+                                        authservid,
+                                        conf->conf_authservidwithjobid ? "/" : "",
+                                        conf->conf_authservidwithjobid ? dfc->mctx_jobid : "",
+                                        pass_fail, use_domain);
                        }
                        else
                        {
                                snprintf(header, sizeof header,
-                                        "%s; spf=%s smtp.mailfrom=%s",
-                                        authservid, pass_fail, use_domain);
+                                        "%s%s%s; spf=%s smtp.mailfrom=%s",
+                                        authservid,
+                                        conf->conf_authservidwithjobid ? "/" : "",
+                                        conf->conf_authservidwithjobid ? dfc->mctx_jobid : "",
+                                        pass_fail, use_domain);
+
+
                        }

                        if (dmarcf_insheader(ctx, 1, AUTHRESULTSHDR,

[dilyanpalauzov]

Or just add a single Authentication-Results header, that contains both the spf amd dmarc checks:

diff --git a/opendmarc/opendmarc.c b/opendmarc/opendmarc.c
--- a/opendmarc/opendmarc.c
+++ b/opendmarc/opendmarc.c
@@ -2252,6 +2252,9 @@ mlfi_eom(SMFICTX *ctx)
        unsigned char replybuf[BUFRSZ + 1];
        unsigned char pdomain[MAXHOSTNAMELEN + 1];
        struct authres ar;
+#if WITH_SPF
+       unsigned char spfheader[MAXHEADER + 1] = "";
+#endif

        assert(ctx != NULL);

@@ -2917,37 +2920,19 @@ mlfi_eom(SMFICTX *ctx)

                        if (spf_mode == DMARC_POLICY_SPF_ORIGIN_HELO)
                        {
-                               snprintf(header, sizeof header,
-                                        "%s%s%s; spf=%s smtp.helo=%s",
-                                        authservid,
-                                        conf->conf_authservidwithjobid ? "/" : "",
-                                        conf->conf_authservidwithjobid ? dfc->mctx_jobid : "",
+                               snprintf(spfheader, sizeof spfheader,
+                                        "; spf=%s smtp.helo=%s",
                                         pass_fail, use_domain);
                        }
                        else
                        {
-                               snprintf(header, sizeof header,
-                                        "%s%s%s; spf=%s smtp.mailfrom=%s",
-                                        authservid,
-                                        conf->conf_authservidwithjobid ? "/" : "",
-                                        conf->conf_authservidwithjobid ? dfc->mctx_jobid : "",
+                               snprintf(spfheader, sizeof spfheader,
+                                        "; spf=%s smtp.mailfrom=%s",
                                         pass_fail, use_domain);

                        if (dmarcf_insheader(ctx, 1, AUTHRESULTSHDR,
                        }

-                       if (dmarcf_insheader(ctx, 1, AUTHRESULTSHDR,
-                                            header) == MI_FAILURE)
-                       {
-                               if (conf->conf_dolog)
-                               {
-                                       syslog(LOG_ERR,
-                                              "%s: %s header add failed",
-                                              dfc->mctx_jobid,
-                                              AUTHRESULTSHDR);
-                               }
-                       }
-
                        if (conf->conf_dolog)
                        {
                                char *mode;
@@ -2998,8 +2983,13 @@ mlfi_eom(SMFICTX *ctx)
                }

                snprintf(header, sizeof header,
+#if WITH_SPF
+                        "%s; dmarc=permerror header.from=%s%s",
+                        authservid, dfc->mctx_fromdomain, spfheader);
+#else
                         "%s; dmarc=permerror header.from=%s",
                         authservid, dfc->mctx_fromdomain);
+#endif

                if (dmarcf_insheader(ctx, 1, AUTHRESULTSHDR,
                                     header) == MI_FAILURE)
@@ -3519,11 +3509,19 @@ mlfi_eom(SMFICTX *ctx)
        if (ret != SMFIS_TEMPFAIL && ret != SMFIS_REJECT)
        {
                snprintf(header, sizeof header,
+#if WITH_SPF
+                        "%s%s%s; dmarc=%s (p=%s dis=%s) header.from=%s%s",
+                        authservid,
+                        conf->conf_authservidwithjobid ? "/" : "",
+                        conf->conf_authservidwithjobid ? dfc->mctx_jobid : "",
+                        aresult, apolicy, adisposition, dfc->mctx_fromdomain, spfheader);
+#else
                         "%s%s%s; dmarc=%s (p=%s dis=%s) header.from=%s",
                         authservid,
                         conf->conf_authservidwithjobid ? "/" : "",
                         conf->conf_authservidwithjobid ? dfc->mctx_jobid : "",
                         aresult, apolicy, adisposition, dfc->mctx_fromdomain);
+#endif

                if (dmarcf_insheader(ctx, 1, AUTHRESULTSHDR,
                                     header) == MI_FAILURE)

[hdatma]

This issue was opened in 2018. Today, August 2020, the patched file has dramatically changed. Can you close this issue?

[dilyanpalauzov]

I do use the latter change and I do not remember having any troubles doing git-rebase. The opendmarc.c file has not changed significantly since 2018.

[hdatma]

Can you rebase the patch against the last release?

[dilyanpalauzov]

The code I use is available at https://mail.aegee.org/cgit/OpenDMARC . I do not have the change above as a single commit. I guess I have two commits: • put SPF (performed by OpenDMARC) and DMARC results in a single A-R header; • make sure that header honours the AuthservIDWithJobID configuration.

[glts]

By the way, as described in https://github.com/trusteddomainproject/OpenDKIM/issues/103, this change produces invalid authserv-ids.