-
Notifications
You must be signed in to change notification settings - Fork 58
Opendmarc SPF repairs and restructure #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Opendmarc SPF repairs and restructure #170
Conversation
remove "ALL" from SOFTFAIL, FAIL, NEUTRAL result codes which seem misleading because those return types can be caused by any mechanism instead of returning/breaking/continueing at every turn, enclose the mechanism evaluation in a large if/else if, set error or match status on a newly introduced field in the stack structure and defer evaluation to the next iteration; qualifier (prefix) evaluation is also deferred handle evaluation of includes correctly by translating the result of the current record to match/nomatch/temp|permerror on the original include change clearly incorrect test cases; add some additional test cases handling mechanisms after include change redirect to actually do what's given in the RFC; redirect is only used if the whole record has be read and no result is set, i.e. no mechanism matched (implying no "all" was given either); redirect is done by replacing the current stack data and start again with the record from redirect
I can't find it at the moment but I am sure I read that the OpenDMARC team were intending to move away from internal library SPF evaluation to rely on libspf2... so it's been interesting to read your work both here and at #169 It would be good to get an update (@thegushi ?) on the intention of OpenDMARC in this. |
I found a comment on the old sourceforge site: https://sourceforge.net/p/opendmarc/tickets/191/ - noting "as we have support for libspf2, the old internal SPF code should be removed and only the support for libspf2 should remain after the next release" and targeting 1.5.0... |
Hey there. You rang?
Short version: at this point we mostly believe that the quality of the unit tests we've added and the enhancements we've made to the internal SPF libs leave us on par with spf2 -- i.e. there may be minor annoyances with both, but neither is horribly worse (*and* we have people contributing changes, and a recent release).
Our team is about to turn interest to OpenDKIM (day jobs have us a little busy right now) for a spell and do that triage, and we've also got some Org stuff to deal with, but at this point I think it's safe to recant anything I said about internal spf going away.
…-Dan
On May 14, 2021, at 6:42 PM, Swallowtail23 ***@***.***> wrote:
I can't find it at the moment but I am sure I read that the OpenDMARC team were intending to move away from internal library SPF evaluation to rely on libspf2... so it's been interesting to read your work both here and at #169 <#169>
It would be good to get an update ***@***.*** <https://github.com/thegushi> ?) on the intention of OpenDMARC in this.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#170 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIWKKCXFKDH4DTWMKVMY7DTNXGOVANCNFSM44453H2Q>.
|
Thanks Dan. |
Ahh, I wasn't aware of those plans. Well, happy to see you may yet keep the implementation. I opted to repair opendmarc's SPF because I found it easier to understand, easier to use, sprinkled with less "FIXME: ..." comments and because I saw some life here again. |
Libspf2 implements SPF v2, while DMARC's own SPF is v1, or so it has been for a long while. If DMARC's SPF is still implementing the deprecated SPF v1, its code should be removed from DMARC's source tree. It would be really useful if all users of SPF would join efforts and deliver a single implementation which is correct and complete with respect to the specification. This means that if DMARC's own SPF is v2, then it should be delivered as a separate library, possibly merged with libspf2. Finally, is anybody able to pinch-awake libspf2's lead developer? There are patches and issues that are long overdue, and a new release would make everybody happy. Time is an asset. Let us not waste it. |
There is no SPF ‘v1’ or ‘v2’. There are two RFCs, obsolete RFC 4408 and current RFC 7208. Both libspf2 and OpenDMARC SPF seem to implement the obsolete RFC 4408. For example, both have no notion of the ‘void lookup limit’. (Reminder that various SPF milters and libraries exist and may convey authentication status in an |
RFC 4408 is SPF v1 |
Not officially. Some people refer to 4408 as v1 and 7208 as v2, but being semantic that is not correct. Even RFC7208 has the title: So referring to it as SPFv2 is misleading at best. It's also somewhat irrelevant. The DMARC RFC calls for an SPF check which is not conformant to either of the SPF RFCs, i.e. cannot strictly be used for SPF validation by itself, but only for the SPF component requirements of DMARC. There has been discussion on this nuance separately on issues here. What we need is for OpenDMARC to be compliant with the DMARC RFC, including the SPF requirements therein. The team are making progress on that, including recent improvements to the SPF code. |
There is no space for nuances. DMARC uses SPF. It is not the purpose of DMARC to implement SPF, neither whole nor part of it. And it certainly is not the purpose of DMARC to define SPF. A DMARC report includes the result of an SPF validation, where SPF is what its defining RFC says it is. Nobody wants a DMARC result whose SPF part turns out to be something else than SPF, calling the need for a second SPF validation done by yet another milter.
Being syntactic, SPFv1 uses the SPF RR, while SPFv2 uses TXT RR. If you bind DMARC to SPFv1 then it will never work, because nobody uses SPF RR anymore. |
Hi there, Quoting the RFCs as source. At https://datatracker.ietf.org/doc/html/rfc7489 (4.1) the use of SPF as an authentication mechanism for DMARC is defined as follows: [SPF], which can authenticate both the domain found in an [SMTP] HELO/EHLO command (the HELO identity) and the domain found in an SMTP MAIL command (the MAIL FROM identity). DMARC uses the result of SPF authentication of the MAIL FROM identity. Note it states DMARC uses the result of SPF authentication of the MAIL FROM identity. DMARC should/does not care about HELO SPF, whether pass or fail. Meanwhile, the SPF RFC states (https://datatracker.ietf.org/doc/html/rfc7208) at 2.4: SPF verifiers MUST check the "MAIL FROM" identity if a "HELO" check either has not been performed or has not reached a definitive policy result by applying the check_host() function to the "MAIL FROM" identity as the . ...and at 2.3: If a conclusive determination about the message can be made based on a check of "HELO", then the use of DNS resources to process the typically more complex "MAIL FROM" can be avoided. The SPF RFC can legitimately result in a valid SPF pass (on HELO) which does not validate "MAIL FROM". Such an SPF check and result, while valid for SPF, is not suitable for use as a DMARC authentication mechanism - see DMARC RFC 4.1 ("DMARC uses the result of SPF authentication of the MAIL FROM identity"). I.e. DMARC's use of SPF as an authentication mechanism requires an SPF validation defined in the SPF RFC as optional. So here is the problem:
My comment about nuance is wrapped in that challenge - firstly OpenDMARC is not an SPF tool, it is a DMARC tool which uses SPF MAIL FROM as an authentication mechanism; and secondly pure-play SPF tools are not aware (and nor should they be) of the requirements of the DMARC RFC (in which MAIL FROM is compulsory). Fundamentally the decision logic associated within respective RFCs for "SPF" and "DMARC SPF authentication mechanism" checks are different. The way to get RFC compliant results for both SPF and DMARC would be: The logic sequence of step b and c are what the OpenDMARC team worked to improve in recent releases - this also addressed a security issue wherein DMARC was accepting upstream SPF passes which were not DMARC-valid (i.e. a HELO pass). Re your comment "Nobody wants a DMARC result whose SPF part turns out to be something else than SPF, calling the need for a second SPF validation done by yet another milter". Again, DMARC is not an SPF tool, it uses an SPF authentication mechanism, as is clearly stated in the RFC. DMARC's decision logic on use of SPF results is not strictly aligned to the recommendations of the SPF RFC; if you are using an OpenDMARC SPF result as definitive that is your call.
Without source, these statements say nothing about "SPFv1" or "SPFv2". |
And this bit is probably back more to the intent of this Pull... i.e. what SPF tests should OpenDMARC do, and how, if it is not given an SPF trusted header, or is told to not trust anything upstream, or is presented a trusted upstream header with only an SPF HELO pass, etc. |
You have just described a loophole that spammers used for years and is still valid as we speak. An e-mail may pass DMARC's authentication test while failing the HELO SPF test.
Thinking out of the box, it is embarrassing to see an authentication test succeed on a message sent by a non authorized IP. I contend that DMARC should fail if the HELO SPF test fails. Use these scripts to find such messages in your own archive (requires maildir):
This is an example in my archive of an authentic e-mail sent by a mis-configured server:
mail.mailroute.net is used by acm.org for mail forwarding. If the authentication tests were true to the facts, all such e-mails should be rejected as spam, because sent by a non-authorized IP (FROM SPF fails). But this is not the example I was looking for. I need some time to dig it up. |
@Swallowtail23 Note that SPF software is configurable. I am not aware of SPF milters that cannot be configured for the purposes of DMARC. I’m following the discussion but I want to suggest moving it elsewhere. It feels like it is distracting from Andreas’ work. |
I was thinking the same last night on moving it. Can one of the admins maybe open a new topic in the Issues section and move the comments? I think the discussion is worth retaining. |
Hi everyone,
initiated by #169, I put together this PR, which substantially improves the integrated SPF part of opendmarc. It now correctly evaluates mechanisms with qualifiers other than '+'/none and follows include directives back down the stack if the result of an included result produces a NOMATCH according to RFC 7208 Section 5 .
Also, redirects are handled according to the RFC and are only evaluated if there is no other matching mechanism (all!). I think it does not make any sense to try to guess what was meant by "redirect" as was previously done in the code (and handling it rather like an include).
I also changed the outcome of three testcases in that regard, which -- if I'm not mistaken -- clearly expected wrong results (and got those from the implementation).
I had to slightlly change the overall structure of the code, so this has become rather large.
Would be happy about some comments and even more happy to see it merged eventually.
I also found a bunch of older SPF test cases, which I think would be worth integrating into the opendmarc SPF tests. There will need to be some extensions for specifying a whole bunch of SPF records instead of just the entry record (which makes sense anyways). So next, I think I am going to work on integrating those and extending the opendmarc SPF implementation to be able to handle them.
Andreas