sdgathman / pyspf

Other
52 stars 26 forks source link

Downgrade +all pass to ?all neutral #35

Closed Daniel-Abrecht closed 3 years ago

Daniel-Abrecht commented 3 years ago

+all being a pass is formally correct, but I think it doesn't make sense in practice for variouse reasons.

As a mail server operator, for sending mails, assuming you aren't just lazy, +all at the end basically means you want to blacklist instead of whitelist stuff. But I don't think that actually works. For example, in the past, there used to be only ip4. So "-ip4:0.0.0.0/1 +all" may have been intended to allow only 128.0.0.0/1, but since IPv6, it also matches any IPv6 address. Something like that could happen again in the future. So in such cases, instead of "+all", "+ip:0.0.0.0/0" should have been used. TLDR: knowing anything else is authorized to send mails isn't possible, therefore t is wrong and saying it may or may not be the case, "neutral", is always more sensible.

In my personal opinion, a blacklisting instead of a whitelisting approach doesn't ever make much sanse anyway. It may in some special cases save one or two cases, but it generally just makes the whole thing unnecessarely difficult to understand and ensure it's actually what you want.

As a mail server operator, for receiving mails, I'm annoyed by lazy OPs just putting +all in the SPF, as it opens up any spamer to use it. In addition to this, for ?all neutral cases, I may or may not want to reject mails. But with OPs using +all, my choice in this matter is sabotaged. In most cases, I don't wan't to accept that mail, but I get it anyway. So by interpreting +all like ?all instead, I can get what I actually wanted.

sdgathman commented 3 years ago

There are legit policies that have - mechanisms followed by +all. A reasonable feature would be a flag that the library can set when it can detect a policy that passes everything (e.g. "v=spf1 +all"). It cannot guarantee that all such policies are detected.

Daniel-Abrecht commented 3 years ago

I don't think so. There may be valid reasons to use for +ip4:0.0.0.0/0 or +ip6:::/0, as they at least say all of what to allow. +all, on the other hand, may someday mean a lot more than just all ipv4 and ipv6 addresses. It doesn't specify, it's just anything and everything. So such setups may unintentionally allow unintended things some day in the future, if ipv7 comes out or something like that. This is why I do not think that policies with +all can be legit. It should never be used. If someone uses it, they should just change that.

sdgathman commented 3 years ago

"v=spf1 -include:%{l1r+-}.detectforgery.%{d} -exists:%{ir}._spf.blacklist.%{d} +all" Just to give you some ideas. Your proposed change is absolutely vetoed.

The only reasonable feature is attempting to detect policies that pass everything.

Daniel-Abrecht commented 3 years ago

"v=spf1 -include:%{l1r+-}.detectforgery.%{d} -exists:%{ir}._spf.blacklist.%{d} +all" is absolutely ineffective. Any new spam server will not be in the black list. It fails to prevent unauthorized people from sending mail. It fails to be a reasonable SPF record.

sdgathman commented 3 years ago

Here is what you may be missing. One of the stated purposes in the RFC of the "exists" mechanism is to allow arbitrary extension of a policy. It is common for the target of "exists" to be an extensible DNS server (e.g. PowerDNS) where arbitrary code can execute to satisfy a query. Suppose, for instance (not necessarily recommending) that a sender provides senders a source of one time passwords that are encoded into the local part. This is so that they can send authorized email from any IP. The "detectforgery" would invoke an exists using a PowerDNS plugin to validate the OTP and check it off. All ips are ok with a valid OTP. Except, you might want to blacklist some anyway - that is just one example setup. It is useful, standard conforming, but your policy would prevent them sending you mail.

Your policy is like Big Email insisting on rDNS - when HELO serves the exact same purpose. rDNS just ensures you are big enough to own a class C or are beholden to an ISP - has nothing to do with validating forward DNS, which HELO does more efficiently and just as well.

Not everything needs to be centralized.

Daniel-Abrecht commented 2 years ago

It is common for the target of "exists" to be an extensible DNS server (e.g. PowerDNS) where arbitrary code can execute to satisfy a query. Suppose, for instance (not necessarily recommending) that a sender provides senders a source of one time passwords that are encoded into the local part. This is so that they can send authorized email from any IP. The "detectforgery" would invoke an exists using a PowerDNS plugin to validate the OTP and check it off. All ips are ok with a valid OTP.

I do not see how this would work. If the "include" or "exists" passes or fails, then it should not reach the "all". If the OTP checks it off, then I assume it would only apply the first time around, but a mail may need to be resend if the first attempt fails, and may pass multiple mail servers which may need to check it, so if that was the case, it would fail, because it is already checked off at that point. On the other hand, if it stays valid, then it could just be reused by anyone, making it pretty pointless. If it allows the last N OTPs, or ties them to a limited time frame, it may help a little, but I still don't think it would help much.

Even if that was to somehow work anyway, you still don't need +all. You can just do the blacklisting first. And I see no reason why validating the OTP couldn't be done with positive matching.

That said, regardless of all that, it indeed still is an existing, valid setup that may be out there. So I think I really shouldn't try to change it here first.

Your policy is like Big Email insisting on rDNS - when HELO serves the exact same purpose. rDNS just ensures you are big enough to own a class C or are beholden to an ISP - has nothing to do with validating forward DNS, which HELO does more efficiently and just as well.

Not everything needs to be centralized.

I actually do have the problem that I run my own mail server but my ISP does not give me a PTR despite me paying for a static IP already. However, I do not think this is comparable. First of, this does not prevent you from doing such OTP matching scenarios like you outlined, you just may have to go about it slightly differently. Secondly, it does not prevent you from running your own mail server independently. Thirdly, although you do not have a centralized rely for a domain in the scenario you outlined, you'd still have to relay on a common DNS server running that PowerDNS setup, and they could still control which/whos OTPs to allow. I do not see how this is any less centralized, at least not in terms of authority and control.

sdgathman commented 2 years ago

I will leave you with this compromise. I can add an API to signal when the result is known to be always Pass. This is not only the canonical "v=spf1 +all", but any policy with only positive matches ending in +all. For instance, "v=spf1 a:1.2.3.4 include:wastelookups.com +all -all". I believe that regardless of how the include evaluates, the result can only be Pass. Thus, the "AlwaysPass" detector would return True. #37

You should NEVER set the official result to Neutral in that case, but it is certainly fine to set the effective result to Neutral, and report both the official and effective result in Received-SPF. I am not aware of a standard for reporting effective result, but I use bestguess.

Daniel-Abrecht commented 2 years ago

Thank you very much. I think that will work for me.