Bug 6918 - SpamAssassin needs to handle Authentication-Results data
Summary: SpamAssassin needs to handle Authentication-Results data
Status: NEW
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Plugins (show other bugs)
Version: unspecified
Hardware: All All
: P2 normal
Target Milestone: Undefined
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
: 7775 (view as bug list)
Depends on:
Blocks: 7911 7189
  Show dependency tree
 
Reported: 2013-03-16 11:25 UTC by Alessandro Vesely
Modified: 2023-05-30 18:28 UTC (History)
7 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status
Proposed plugin application/octet-stream None Alessandro Vesely [HasCLA]
Parse fix patch None Giovanni Bechis [HasCLA]
arc.chain support patch None Matus UHLAR - fantomas [NoCLA]

Note You need to log in before you can comment on or make changes to this bug.
Description Alessandro Vesely 2013-03-16 11:25:34 UTC
Created attachment 5138 [details]
Proposed plugin

This is slightly related to bug #6899.  Being able to handle such data can also save DNS queries to dnswl.org, omit repeating DKIM checks, and possibly more.

I propose a draft implementation, that I add as an attachment.  I run it under SA 3.3.1 for some alpha tests, and it apparently works.  However, more tests and review are needed:

* There are only two "adjustments" (commented as such).  We probably need more.

* I'm a newcomer to SA, so I might have overlooked something.
Comment 1 Henrik Krohns 2018-11-08 11:04:20 UTC
Based on this, I've cleaned up a basic version and the parser should be quite robust now. Committing to get it out of my queue, and perhaps get some developing going.

Sending        MANIFEST
Sending        UPGRADE
Adding         lib/Mail/SpamAssassin/Plugin/AuthRes.pm
Sending        t/all_modules.t
Adding         t/authres.t
Adding         t/data/nice/authres
Transmitting file data ......done
Committing transaction...
Committed revision 1846123.

There's only these options for now:
authres_networks
authres_ignored_authserv
authres_trusted_authserv

Honestly I'm not sure what we should do with it. It doesn't help "reducing" SPF/DKIM lookups, since atleast opendkim/opendmarc create quite simple Authentication-Results headers. How exactly are we going to replicate all these possibly useful rules, when we have some opendkim response of "dkim=pass,fail" without details?

full     DKIM_SIGNED         eval:check_dkim_signed()
full     DKIM_VALID          eval:check_dkim_valid()
full     DKIM_VALID_AU               eval:check_dkim_valid_author_sig()
full     DKIM_VALID_EF               eval:check_dkim_valid_envelopefrom()
full     __DKIM_DEPENDABLE   eval:check_dkim_dependable()
header   DKIM_ADSP_NXDOMAIN  eval:check_dkim_adsp('N')
header   DKIM_ADSP_DISCARD   eval:check_dkim_adsp('D')
header   DKIM_ADSP_ALL               eval:check_dkim_adsp('A')
header   DKIM_ADSP_CUSTOM_LOW        eval:check_dkim_adsp('1')
header   DKIM_ADSP_CUSTOM_MED        eval:check_dkim_adsp('2')
header   DKIM_ADSP_CUSTOM_HIGH       eval:check_dkim_adsp('3')
full   DKIM_VERIFIED         eval:check_dkim_valid()
header DKIM_POLICY_TESTING   eval:check_dkim_testing()
header DKIM_POLICY_SIGNSOME  eval:check_dkim_signsome()
header DKIM_POLICY_SIGNALL   eval:check_dkim_signall()

Same with SPF. Atleast opendmarc's inbuilt SPF check doesn't report NEUTRAL let alone some HELO stuff? Dunno if pypolicyd-spf / spf-engine gives anything useful.

header   SPF_PASS     eval:check_for_spf_pass()
header   SPF_NEUTRAL  eval:check_for_spf_neutral()
header   SPF_FAIL     eval:check_for_spf_fail()
header   SPF_SOFTFAIL eval:check_for_spf_softfail()
header   SPF_HELO_PASS                eval:check_for_spf_helo_pass()
header   SPF_HELO_NEUTRAL     eval:check_for_spf_helo_neutral()
header   SPF_HELO_FAIL                eval:check_for_spf_helo_fail()
header   SPF_HELO_SOFTFAIL    eval:check_for_spf_helo_softfail()
header   SPF_NONE             eval:check_for_spf_none()
header   SPF_HELO_NONE                eval:check_for_spf_helo_none()
Comment 2 Henrik Krohns 2018-11-11 12:03:38 UTC
Sigh, opendkim and opendmarc are broken..

https://github.com/trusteddomainproject/OpenDKIM/issues/24
https://github.com/trusteddomainproject/OpenDMARC/issues/23

So practically noone can make use the A-R headers unless compiling yourself
or some distribution decides to patch them.  The developement on these is so
darn slow, who knows when official versions are out..
Comment 3 Henrik Krohns 2018-11-12 17:12:20 UTC
https://marc.info/?l=postfix-users&m=154203866507674&w=2

As Viktor described, I think we should recommend using "authres_networks all" with authres_trusted_authserv. That way it doesn't matter where the headers are positioned and works with OpenDKIM etc out of the box.

Sending        AuthRes.pm
Transmitting file data .done
Committing transaction...
Committed revision 1846448.
Comment 4 Alessandro Vesely 2018-11-12 18:47:24 UTC
I agree with Henrik, except for missing a curly brace:
--- AuthRes.pm.old
+++ AuthRes.pm.new
@@ -179,7 +179,7 @@
   });
 
   $conf->{parser}->register_commands(\@cmds);
-}
+
 
 =item authres_ignored_authserv authservid1 id2 ...   (default: none)
 
@@ -205,6 +205,7 @@
       }
     }
   });
+}
 
 =head1 METADATA
 
(perhaps sooner or later I'll find out how to commit.)

Longer rumination (written before Henryk's post):

The bugs referenced in comment #2 are about placing A-R fields before or after Received.  That's no usability problem.  My MTA, for example, places some A-R fields before and some after Received, depending on which process reports what result.  Much like X-Spam-* fields, admins know how ingress filtering works and need to configure ADMINISTRATOR OPTIONS accordingly.  In order to take into account A-R fields placed before Received, authres_networks needs to be set to "all", relying on authservid's if needed.

The only issue is to explain that in Mail::SpamAssassin::Conf.

I don't see why opendkim would create "quite simple" Authentication-Results.  From a mailing list which I know uses OpenDKIM, I got:

Nov 12 18:38:45.003 [26569] dbg: authres: parsing Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1536-bit key) header.d=iecc.com header.b=kbJdZYzc; dkim=pass (1536-bit key) header.d=taugh.com header.b=LwXl016e

Use of that result should be done in DKIM.pm, probably _check_dkim_signature, instead of looking up everything from scratch.  Ditto for SPF.
Comment 5 Henrik Krohns 2018-11-12 19:29:08 UTC
Ah yes hasty commits while doing household chores..

I need to look into all the header contents when I have more time.. feel free to explain to me if you have ideas what should hit what.. do we need to even care about some deprecated ADSP rules?
Comment 6 Alessandro Vesely 2018-11-13 19:09:13 UTC
For DKIM, Mail::DKIM::Verifier looks up keys as soon as it parses a signature, like so:

    my $signature = Mail::DKIM::Signature->parse($line);
    $self->add_signature($signature);
    $signature->fetch_public_key;

We need a parsed signature in order to understand if it was verified already, so as to skip its verification.  A domain can add multiple signatures which can have different selectors or at least different hash (header.s is not yet part of A-R; if header.b is missing, we must assume that this signature was the only one the verifier saw by the given domain.)

Of course, a domain can add signatures after A-R fields were written.  Those should not be interesting if the A-Rs are trusted.  However, A-Rs, albeit by a trusted agent, might be stale, written before a message was further relayed.  In that case  a careful MTA should invalidate them (Courier-MTA, for example, renames them to Old-Authentication-Results on ingress.)

The quick solution is to have the admin tell if A-Rs are authoritative for DKIM, which also entails that there is no valid DKIM signature unless we found the result.  Messages arriving from a different path, with no A-R, may still require to load DKIM::Verifier.

Otherwise, the hard solution requires to learn more Mail::DKIM internals.  BTW, what's that "caller of SpamAssassin already supplied DKIM signature objects" comment in SpamAssassin::DKIM?
Comment 7 Henrik Krohns 2019-11-21 15:13:37 UTC
*** Bug 7775 has been marked as a duplicate of this bug. ***
Comment 8 Matus UHLAR - fantomas 2021-05-03 11:19:55 UTC
I maintain a server in environment where DKIM signatures are validated by external gateway which then adds text to mail body:
"mail was received from external environment..."

In this case we need to rely on Authentication-Results: header and what is says about DKIM:

Authentication-Results: fml.example.com;
        spf=pass (example.com: domain of [censored]@gmail.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=[censored]@gmail.com;
        dkim=pass header.i=@gmail.com

because behind this gateway, DKIM verify will fail because of the added text.
while the rest of checks may be important or maybe repeated, we need to trust the validation

(of course, with headers on proper place, e.g. within trusted_network).
Comment 9 Benny Pedersen 2021-05-03 15:42:13 UTC
1: add openARC localy before dkim is breaked
2: wait for spamassassin 4.x.x where AuthRes hopefully works

but AuthRes still miss support for ARC validate, as i see openARC does not add A-R headers

other low hanging solution is to not break dkim
Comment 10 Matus UHLAR - fantomas 2021-05-18 12:26:24 UTC
Note that in case of using milters, the Authentication-Results: header may appear (immediately) after the equivalent Received: headers.

such header should only be trusted if it contains configured hostname

at least opendkim-milter makes situation easier by removing untrusted headers containing local hostname.
Comment 11 Henrik Krohns 2021-05-18 12:33:23 UTC
(In reply to Matus UHLAR - fantomas from comment #10)
> Note that in case of using milters, the Authentication-Results: header may
> appear (immediately) after the equivalent Received: headers.
> 
> such header should only be trusted if it contains configured hostname
> 
> at least opendkim-milter makes situation easier by removing untrusted
> headers containing local hostname.

And all this is already discussed here, especially Comment 3.
Comment 12 Henrik Krohns 2021-05-18 12:51:01 UTC
Hopefully someone/community will contribute to push these things forward, sadly there are not many developers around.

I have currently very little interest in spending time for all the DMARC/ARC/mumble stuff, as I don't even have a clear understanding on how they improve things. As long as I can whitelist_auth some SPF/DKIM senders, there isn't anything lacking for me in my setup.

Feel free to contribute atleast some _concrete_ plan on how to integrate all the separate AuthRes, SPF, DKIM and possibly DMARC (Bug 7189) and what their functions should be.
Comment 13 Kevin A. McGrail 2022-08-23 21:40:36 UTC
NOTE removed this from the Upgrade file since it's still in process

- New AuthRes module to process Authentication-Results headers (unfinished,
  Bug 6918)
Comment 14 Henrik Krohns 2023-03-01 18:19:02 UTC
Updated missing valid methods/values and fixed uninitialized $result per list discussion.

Sending        trunk/lib/Mail/SpamAssassin/Plugin/AuthRes.pm
Transmitting file data .done
Committing transaction...
Committed revision 1907938.
Comment 15 Giovanni Bechis 2023-03-02 09:16:33 UTC
Created attachment 5880 [details]
Parse fix

Authres plugin fails to parse this header:

ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
 smtp.mailfrom=domain.com; dmarc=pass action=none header.from=domain.com;
 dkim=pass header.d=domain.com; arc=none

This triggers "missing delimiter" because header counter (i=1) is not ignored.
Comment 16 Henrik Krohns 2023-03-02 10:14:14 UTC
(In reply to Giovanni Bechis from comment #15)
> Created attachment 5880 [details]
> Parse fix
> 
> Authres plugin fails to parse this header:
> 
> ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
>  smtp.mailfrom=domain.com; dmarc=pass action=none header.from=domain.com;
>  dkim=pass header.d=domain.com; arc=none
> 
> This triggers "missing delimiter" because header counter (i=1) is not
> ignored.

According to RFC i= can be 1-50 so i=\d is not enough.

Only ARC has the index, so it's not necessary to be checked for Authentication-Results.

Should parse_authres take argument to indicate it parsing ARC header, and should $pms->{authres_parsed} included flag to mark it was ARC?
Comment 17 Alessandro Vesely 2023-03-02 11:12:53 UTC
(In reply to Henrik Krohns from comment #16)
> (In reply to Giovanni Bechis from comment #15)
> > Created attachment 5880 [details]
> > Parse fix
> > 
> > Authres plugin fails to parse this header:
> > 
> > ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
> >  smtp.mailfrom=domain.com; dmarc=pass action=none header.from=domain.com;
> >  dkim=pass header.d=domain.com; arc=none
> > 
> > This triggers "missing delimiter" because header counter (i=1) is not
> > ignored.
> 
> According to RFC i= can be 1-50 so i=\d is not enough.
> 
> Only ARC has the index, so it's not necessary to be checked for
> Authentication-Results.


There's a whole lot of stuff to check in an ARC chain, beyond out-of-order/ huge chains.  All that is better handled by an ARC module.


> Should parse_authres take argument to indicate it parsing ARC header, and
> should $pms->{authres_parsed} included flag to mark it was ARC?


Agreed.  If the ARC chain is not valid, or if the d= domain in the seal is not trusted, then the results should not be considered (or worse).
Comment 18 Matus UHLAR - fantomas 2023-03-02 11:37:59 UTC
the new works but generates new error in parsing arc headers: 

Mar  2 12:27:01.211 [1325] dbg: authres: parsing Authentication-Results: fantomas.fantomas.sk; arc=none smtp.remote-ip=3.227.148.255
Mar  2 12:27:01.211 [1325] dbg: authres: skipping header, unknown property for smtp: remote

Authentication-Results: fantomas.fantomas.sk; arc=none smtp.remote-ip=3.227.148.255

Perhaps the values should be remote-ip and oldest-pass instead?

+  'arc' => {'smtp' => {'remote_ip'=>1}, 'header' => {'oldest_pass'=>1}},
Comment 19 Henrik Krohns 2023-03-02 11:43:09 UTC
(In reply to Matus UHLAR - fantomas from comment #18)
> the new works but generates new error in parsing arc headers: 
> 
> Mar  2 12:27:01.211 [1325] dbg: authres: parsing Authentication-Results:
> fantomas.fantomas.sk; arc=none smtp.remote-ip=3.227.148.255
> Mar  2 12:27:01.211 [1325] dbg: authres: skipping header, unknown property
> for smtp: remote
> 
> Authentication-Results: fantomas.fantomas.sk; arc=none
> smtp.remote-ip=3.227.148.255
> 
> Perhaps the values should be remote-ip and oldest-pass instead?
> 
> +  'arc' => {'smtp' => {'remote_ip'=>1}, 'header' => {'oldest_pass'=>1}},

Yeah I typoed wrong values.. fixed:

Committed revision 1907968.
Comment 20 Giovanni Bechis 2023-03-02 11:49:46 UTC
(In reply to Henrik Krohns from comment #16)
> (In reply to Giovanni Bechis from comment #15)
> > Created attachment 5880 [details]
> > Parse fix
> > 
> > Authres plugin fails to parse this header:
> > 
> > ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
> >  smtp.mailfrom=domain.com; dmarc=pass action=none header.from=domain.com;
> >  dkim=pass header.d=domain.com; arc=none
> > 
> > This triggers "missing delimiter" because header counter (i=1) is not
> > ignored.
> 
> According to RFC i= can be 1-50 so i=\d is not enough.
> 
> Only ARC has the index, so it's not necessary to be checked for
> Authentication-Results.
> 
> Should parse_authres take argument to indicate it parsing ARC header, and
> should $pms->{authres_parsed} included flag to mark it was ARC?

We should know if an Authentication-Results header is ARC signed or not, we should add needed properties to $pms->{authres_parsed}.

Atm wip DKIM.pm code uses AAR headers parsed by Authres to check for trusted d= values, DMARC then will use those values.
Comment 21 Henrik Krohns 2023-03-02 11:50:21 UTC
(In reply to Henrik Krohns from comment #19)
> Yeah I typoed wrong values.. fixed:
> 
> Committed revision 1907968.

And allow hyphen in the name...

Committed revision 1907969.
Comment 22 Matus UHLAR - fantomas 2023-03-12 08:58:07 UTC
Created attachment 5881 [details]
arc.chain support
Comment 23 Henrik Krohns 2023-03-12 09:05:46 UTC
(In reply to Matus UHLAR - fantomas from comment #22)
> Created attachment 5881 [details]
> arc.chain support

Got any examples headers for reference?
Comment 24 Henrik Krohns 2023-03-12 10:57:04 UTC
Fixed some parsing and ARC stuff. For now there is added $pms->{authres_parsed}->{arc_index} but obviously there isn't any code anywhere yet to parse/verify ARC chains, I have no idea how those work, feel free to contribute.

Sending        lib/Mail/SpamAssassin/Plugin/AuthRes.pm
Sending        t/authres.t
Transmitting file data ..done
Committing transaction...
Committed revision 1908319.
Comment 25 Matus UHLAR - fantomas 2023-03-12 13:49:03 UTC
(In reply to Henrik Krohns from comment #23)
> (In reply to Matus UHLAR - fantomas from comment #22)
> > Created attachment 5881 [details]
> > arc.chain support
> 
> Got any examples headers for reference?

openarc generates this kind of results:

Authentication-Results: fantomas.fantomas.sk; arc=pass smtp.remote-ip=40.92.48.87 arc.chain=microsoft.com

Authentication-Results: fantomas.fantomas.sk; arc=pass smtp.remote-ip=149.20.1.60 arc.chain="isc.org:isc.org:isc.org"
Comment 26 Alessandro Vesely 2023-03-13 11:21:58 UTC
I received these:

Authentication-Results: wmail.tana.it;
  spf=pass smtp.mailfrom=lists.isc.org;
  arc=pass header.oldest-pass=3 (3 set(s)) smtp.remote-ip=149.20.1.60

Authentication-Results: wmail.tana.it;
  spf=pass smtp.mailfrom=gmail.com;
  dkim=pass header.d=comune.milano.it;
  dmarc=pass header.from=comune.milano.it;
  arc=pass header.oldest-pass=0 (2 set(s)) smtp.remote-ip=209.85.214.179

Authentication-Results: wmail.tana.it;
  spf=pass smtp.mailfrom=gmail.com;
  arc=fail (1 set(s)) smtp.remote-ip=209.85.214.173
Comment 27 Benny Pedersen 2023-05-30 18:28:20 UTC
(In reply to Giovanni Bechis from comment #15)
> Created attachment 5880 [details]
> Parse fix
> 
> Authres plugin fails to parse this header:
> 
> ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass

                                                    ^

>  smtp.mailfrom=domain.com; dmarc=pass action=none header.from=domain.com;
>  dkim=pass header.d=domain.com; arc=none
> 
> This triggers "missing delimiter" because header counter (i=1) is not
> ignored.

microsoft have imho removed this fail ?