trusteddomainproject / OpenDKIM

Other
97 stars 52 forks source link

Really prepend Authentication-Results and DKIM-Signature headers #24

Closed dilyanpalauzov closed 5 years ago

dilyanpalauzov commented 6 years ago

as these are trace headers:

diff --git a/opendkim/opendkim.c b/opendkim/opendkim.c
--- a/opendkim/opendkim.c
+++ b/opendkim/opendkim.c
@@ -4250,7 +4250,7 @@ dkimf_add_ar_fields(struct msgctx *dfc, struct dkimf_config *conf,
        assert(conf != NULL);
        assert(ctx != NULL);

-       if (dkimf_insheader(ctx, 1, AUTHRESULTSHDR,
+       if (dkimf_insheader(ctx, 0, AUTHRESULTSHDR,
                            (char *) dfc->mctx_dkimar) == MI_FAILURE)
        {
                if (conf->conf_dolog)
@@ -13511,7 +13511,7 @@ mlfi_eom(SMFICTX *ctx)
                         dkimf_lookup_inttostr(dfc->mctx_status,
                                               dkimf_statusstrings));

-               if (dkimf_insheader(ctx, 1, AUTHRESULTSHDR,
+               if (dkimf_insheader(ctx, 0, AUTHRESULTSHDR,
                                    (char *) header) == MI_FAILURE)
                {
                        if (conf->conf_dolog)
@@ -14956,7 +14956,7 @@ mlfi_eom(SMFICTX *ctx)
                                                        sizeof header);
                                        }

-                                       if (dkimf_insheader(ctx, 1,
+                                       if (dkimf_insheader(ctx, 0,
                                                            AUTHRESULTSHDR,
                                                            (char *) header) == MI_FAILURE)
                                        {
@@ -15159,7 +15159,7 @@ mlfi_eom(SMFICTX *ctx)
                        dkimf_stripcr((char *) start);
                        dkimf_dstring_cat(dfc->mctx_tmpstr, start);

-                       if (dkimf_insheader(ctx, 1, DKIM_SIGNHEADER,
+                       if (dkimf_insheader(ctx, 0, DKIM_SIGNHEADER,
                                            (char *) dkimf_dstring_get(dfc->mctx_tmpstr)) == MI_FAILURE)
                        {
                                if (conf->conf_dolog)
@@ -15195,7 +15195,7 @@ mlfi_eom(SMFICTX *ctx)
                /* add VBR-Info header if generated */
                if (dfc->mctx_vbrinfo != NULL)
                {
-                       if (dkimf_insheader(ctx, 1, VBR_INFOHEADER,
+                       if (dkimf_insheader(ctx, 0, VBR_INFOHEADER,
                                            dfc->mctx_vbrinfo) == MI_FAILURE)
                        {
                                if (conf->conf_dolog)
mskucherawy commented 5 years ago

As I recall, the index 1 is used because we don't want to precede the Received field that the MTA will add. Is the current implementation causing problems?

dilyanpalauzov commented 5 years ago

I filed this case after seeing in my mails, that the AR-header was not at the place where I expected it. Afterwards I found, that my MDA reorders the headers, as it handles Received as trace headers, but not Authentication-Results headers and the used index does not really matter. So I am not sure today where index 1 inserts the headers.

If you verify, that index 0 inserts Authentication-Results: before Received:, then you can close this case.

mskucherawy commented 5 years ago

From the milter documentation:

"Setting hdridx to 0 will actually insert the header before this Received: header."

fanto666 commented 3 years ago

Prepending DKIM headers in front of Received: headers helps software like SpamAssassin to trust those headers. Headers below Received: can be added by sending client, while headers above it can not.

I have verified that changing argument 2 of dkimf_insheader does put Authentication-Results header above Recveived: which is a good thing.

martinbogo commented 3 years ago

We're concentrating on OpenDKIM this cycle -- if you have experimented and found a patch/change that helps, please checkout the github, and then submit a PR/patch to the "develop" branch for us to merge in.

Sincerely, Martin Bogomolni Maintainer : Trusted Domain Project

On Wed, May 19, 2021 at 2:50 AM fanto666 @.***> wrote:

Prepending DKIM headers in front of Received: headers helps software like SpamAssassin to trust those headers. Headers below Received: can be added by sending client, while headers above it can not.

I have verified that changing argument 2 of dkimf_insheader does put Authentication-Results header above Recveived: which is a good thing.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/trusteddomainproject/OpenDKIM/issues/24#issuecomment-843840900, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5KKIUFRZFO3LUPLVDQD3TONUSRANCNFSM4FC67IEQ .

glts commented 3 years ago

@fanto666 I’m interested in this, too. Will you create the pull request? And we need the same in OpenDMARC.

fanto666 commented 3 years ago

Hi guys, I'm happy to provide patch at: http://test.fantomas.sk/opendkim.c.patch hoping that will help you