search

trusteddomainproject / OpenDMARC

This is the Trusted Domain Project's impementation of the DMARC protocol libary and mail filter, called OpenDMARC. A "milter" connects to unix-based mailers (originally, sendmail, but now many) and provides a standard filtering API.
Other
103 stars 55 forks source link

Security Bug CVE-2019-20790: OpenDMARC can be bypassed when it's used with pypolicyd-spf #49

Open chenjj opened 5 years ago

chenjj commented 5 years ago

I cannot find a private address for reporting security bugs, so I have to post it here.

If a mail server uses both OpenDMARC and pypolicyd-spf, its SPF and DMARC authentication can be bypassed with the following message:

HELO: attacker.com
MAIL FROM: <any@not.exist.legitimate.com>
...
From: <admin@legitimate.com>
...

Given the popularity of OpenDMARC and pypolicyd-spf, this bug may affect many online services. Hope you can fix it soon.

Detailed reproduce steps at: https://sourceforge.net/p/opendmarc/tickets/235/

I have also reported it to pypolicyd-spf: https://bugs.launchpad.net/pypolicyd-spf/+bug/1838816

dilyanpalauzov commented 5 years ago

https://tools.ietf.org/html/rfc7489#section-3.1.2 says that the EHLO SPF identifier is not typically used in the context of DMARC. You should tell pypolicyd-spf not to insert AR header and tell opendmarc to insert the SPF information in the AR header itself. Then it will work.

dilyanpalauzov commented 5 years ago

I have not checked the code but it would be an error in OpenDMARC if it uses that AR-header: SPF does pass, but the domain does not align.

martinbogo commented 4 years ago

@dilyanpalauzov I am going through the issues from newest to oldest, and adding them to a prioritized triage list.

I will cross-check with the maintainers of pypolicyd-spf and we'll address this issue.

mimi89999 commented 4 years ago

I noticed that it can also be bypassed with an empty mail from (MAIL FROM:<> the A2 test from https://github.com/chenjj/espoofer). Whit an empty mail from I get the following report:

Received-SPF: Pass (helo) identity=helo; client-ip=1.2.3.4; helo=attack.com; envelope-from=<>; receiver=<UNKNOWN>
Authentication-Results: victim.com; dmarc=pass (p=none dis=none) header.from=legitimate.com
glts commented 3 years ago

CVE for this issue is CVE-2019-20790.

utkarsh2102 commented 3 years ago

Hi, is there any ETA on this?

thegushi commented 3 years ago

we've just resumed some fairly active work on this program. I cannot speak authoritatively, but it would make sense to squash our CVEs before our next release.

thegushi commented 3 years ago

So, to sum up:

I think pypolicyd is behaving badly, and that we should add notation to OpenDMARC that "some clients" add a bad authentication results header, and we should encourage people to let opendmarc do this checking with libspf2.

OpenDMARC's code to evaluate a received-spf header ONLY looks for the "pass" or "fail". It does not re-evaluate whether SPF has passed or failed, because at that point you would have two headers, one which says it passed and one which says it did not.

To fully evaluate the received-spf header, would require doing the same work that the previous milter did. (twice the DNS lookups, etc), and you'd still have to believe the header.

@chenjj -- do you disagree with any of the above?

glts commented 3 years ago

I think pypolicyd is behaving badly, and that we should add notation to OpenDMARC that "some clients" add a bad authentication results header, and we should encourage people to let opendmarc do this checking with libspf2.

@thegushi Please don’t blanket discourage using third-party SPF components. I also maintain an SPF milter, it would be a pity if OpenDMARC blesses only its own, libspf2-based implementation.

thegushi commented 3 years ago

On Mar 16, 2021, at 3:49 AM, glts @.***> wrote:

I think pypolicyd is behaving badly, and that we should add notation to OpenDMARC that "some clients" add a bad authentication results header, and we should encourage people to let opendmarc do this checking with libspf2.

@thegushi https://github.com/thegushi Please don’t blanket discourage using third-party SPF components. I also maintain an SPF milter https://gitlab.com/glts/spf-milter, it would be a pity if OpenDMARC blesses only its own, libspf2-based implementation.

I don’t blanket discourage them. But I’d encourage admins to be aware of the implications of trusting HELO, and/or giving you a milter that doesn’t make that tuneable. Does yours?

-Dan

glts commented 3 years ago

I don’t blanket discourage them. But I’d encourage admins to be aware of the implications of trusting HELO, and/or giving you a milter that doesn’t make that tuneable. Does yours?

It does. All right then, thanks!

thegushi commented 3 years ago

So, @glts , we've added an addressing to this issue to our new SECURITY folder, currently visible in the Develop branch. There's no code to fix on our end, as "trust the headers that previous milters and MTA's put in" is not unreasonable behavior for opendmarc. (The old adage, garbage-in, garbage-out, is apt here).

I'm closing this issue out as the only action is "Documentation Fix". Honestly, this CVE seems more to belong to py-policyd than OpenDMARC.

glts commented 3 years ago

It would be great if someone with authority from the project could go to https://cveform.mitre.org/ and request that CVE-2019-20790 be marked ‘reject’ (since it is considered ‘won’t fix’).

This would help distributions that rely on CVE info in the MITRE database.

chenjj commented 3 years ago

It would be great if someone with authority from the project could go to https://cveform.mitre.org/ and request that CVE-2019-20790 be marked ‘reject’ (since it is considered ‘won’t fix’).

This would help distributions that rely on CVE info in the MITRE database.

That's not a good idea. It's a valid security issue. The debate here is which party (OpenDMARC or pypolicyd-spf) should fix this. I think OpenDMARC's action "Documentation Fix" is fine if they think it's not their fault. But that doesn't change the fact that anyone who has this configuration combination is vulnerable to SPF and DMARC bypass attack.

I don't think one should request for marking CVE-2019-20790 as 'reject', as this is a security issue that should be noted.

mimi89999 commented 3 years ago

Did https://bugs.launchpad.net/pypolicyd-spf/+bug/1838816/comments/4 say that it's an issue with OpenDMARC? There is obviously an issue somewhere and somebody should fix it.

thegushi commented 3 years ago

Mimi89999: Reading the comments on your link, Scott Kitterman and the other pypolicyd maintainers basically believe that OpenDMARC should believe the Received-SPF header that they put in, and should base our decision on whichever SPF value was used to make the decision. (which we do -- if it says "spf=pass", we run with that). We do not re-evaluate the decisions.

Trusting HELO is insane. Note that this CVE is specific to py-policyd. (i.e. the CVE isn't "all SPF headers are evaluated to true by opendmarc"). I don't know if pypolicyd's behavior is configurable, and I don't know if this is the default behavior. It's not our place to tell people how to configure their entire stack.

Any other inline filter that trusted that received-spf header would hit the same issue.

chenjj commented 3 years ago

One possible mitigation is that DMARC could parse the identity field in Received-SPF header to check which identity pypolicyd verify, and re-evaluate SPF decision for those using helo.

Received-SPF: Pass (helo) identity=helo; client-ip=1.2.3.4; helo=attack.com; envelope-from=any@not.exist.legitimate.com;

But it's just my suggestion. I think the issue may need coordination with pypolicyd. Not sure if Scott Kitterman ( @kitterma ) has any suggestions on this issue.

thegushi commented 3 years ago

@chenjj Can you give me examples of what received-spf pypolicyd sets? The pass (helo) isn't useful: (helo) is a human-readable comment and not expected to be human parsed. The identity field is optional (SHOULD, not MUST), as, in fact, are most of the fields.

As in, it's totally possible to give a completely vague header that still says spf=pass.

thegushi commented 3 years ago

I'm reopening this, but the fix may be: Document this more heavily for 1.4.1, and apply greater fixes in 1.5 and beyond.

chenjj commented 3 years ago

Hi @thegushi, thank you for reopening this.

Here is some resource I got about the Received-SPF header. Hope they can help you to fix this.

1) Example

The Received-SPF header in the following example is generated by pypolicyd. 

Return-Path: <any@not.exist.legitimate.com>
X-Original-To: recvmail@victim.com
Delivered-To: recvmail@victim.com
Received-SPF: Pass (helo) identity=helo; client-ip=1.2.3.4; helo=attack.com; envelope-from=any@not.exist.legitimate.com; receiver=<UNKNOWN> 
Authentication-Results: ubuntu; dmarc=pass (p=none dis=none) header.from=legitimate.com
Received: from attack.com by ubuntu (Postfix) with SMTP id 72D1D499C9
        for <recvmail@victim.com>; Wed, 7 Aug 2019 14:23:45 -0400 (EDT)
From: <admin@legitimate.com>
To: <recvmail@victim.com>
Subject: test

2) RFC syntax

RFC 7208 Section 9.1 has a definition about the syntax of Received-SPF.

   identity         = "mailfrom"   ; for the "MAIL FROM" identity
                      / "helo"     ; for the "HELO" identity
                      / name       ; other identities

As you mentioned, some SPF implementations may not generate the identity field. In that case, OpenDMARC doesn't know which identity SPF is verifying, I think that's a flaw in the SPF implementation if it trusts helo at the same time.

thegushi commented 3 years ago

Okay, do you have a sample of what pypolicyd’s Authentication-Results header looks like?

-Dan

On Apr 2, 2021, at 9:16 PM, Jianjun Chen @.***> wrote:

Hi @thegushi https://github.com/thegushi, thank you for reopening this.

Here is some resource I got about the Received-SPF header. Hope they can help you to fix this.

Example The Received-SPF header in the following example is generated by pypolicyd.

Return-Path: @.> X-Original-To: @. Delivered-To: @. Received-SPF: Pass (helo) identity=helo; client-ip=1.2.3.4; helo=attack.com; @.; receiver= Authentication-Results: ubuntu; dmarc=pass (p=none dis=none) header.from=legitimate.com Received: from attack.com by ubuntu (Postfix) with SMTP id 72D1D499C9 for @.>; Wed, 7 Aug 2019 14:23:45 -0400 (EDT) From: @.> To: @.***> Subject: test RFC syntax RFC 7208 Section 9.1 https://tools.ietf.org/html/rfc7208#section-9.1%60 has a definition about the syntax of Received-SPF.

identity = "mailfrom" ; for the "MAIL FROM" identity / "helo" ; for the "HELO" identity / name ; other identities As you mentioned, some SPF implementations may not generate the identity field. In that case, OpenDMARC doesn't know which identity SPF is verifying, I think that's a flaw in the SPF implementation if it trusts helo at the same time.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/trusteddomainproject/OpenDMARC/issues/49#issuecomment-812807815, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIWKKBFQTMCNW4V345J7GLTG2JDJANCNFSM4IJB2OYA.

chenjj commented 3 years ago

Here is one example I got:

Authentication-Results: example.com; spf=pass smtp.mailfrom=user@example.com

But this example may not complete. I would suggest having the syntax and example in RFC 7208 Section 9.2 and RFC 8601 as your reference.

thegushi commented 3 years ago

I'm not looking to the RFC's right now, I'm looking to what real-world values things (especially the other tool referenced in the CVE) are doing.

Reading the RFC, the problem there is that Authentication-Results: example.com; spf=pass is a complete, legal header, as well.

As stated, in 1.4.1 (due out perhaps this weekend), we're going to explain the problem clearly: SPF=pass does not mean SPF=pass for dmarc.

While I personally think HELO is trivially forgeable by spammy mail-clients on botnets around the world, it's been argued that it's legal for spf and I shouldn't tell people how to do their SPF.

I believe our advice in 1.4.1 is going to be, effectively:

“document this heavily, explaing that SPF as far as pypolicyd is concerned is not SPF as far as dmarc is concerned, and suggest that people either configure them to be the same OR let opendmarc do its own."

For a 1.4.2 or a 1.5.0, we'll fix all the parsing, but it's kind of a lot of code to add in a point release, and we have a lot of things we have milestoned for 1.5.0.

thegushi commented 3 years ago

Totally unrelated, where did your usericon come from?

chenjj commented 3 years ago

I'm not looking to the RFC's right now, I'm looking to what real-world values things (especially the other tool referenced in the CVE) are doing.

Reading the RFC, the problem there is that Authentication-Results: example.com; spf=pass is a complete, legal header, as well.

That's wired, I'm not sure why RFC allows this. I feel the RFC sometimes may be too flexible, which is not so implementation-friendly and could create much space for ambiguity issues.

chenjj commented 3 years ago

Totally unrelated, where did your usericon come from?

I couldn't remember the original source. ;-(

mskucherawy commented 3 years ago

That's wired, I'm not sure why RFC allows this. I feel the RFC sometimes may be too flexible, which is not so implementation-friendly and could create much space for ambiguity issues.

Section 2.2 of the RFC explains this:

The "propspec" may be omitted if, for example, the method was unable to extract any properties to do its evaluation yet still has a result to report. It may also be omitted if the agent generating this result wishes not to reveal such properties to downstream agents.

kitterma commented 3 years ago

I'm the policy-spf developer. I think much of the above is based on some misunderstandings:

  1. In the policy server you can turn HELO checking off if you don't want it.
  2. As far as I know, policy-spf always includes the identity used (e.g. smtp.mailfrom=user@example.com).
  3. If the identity is not included, you have no way of knowing what the SPF evaluated identity was, so I don't see how OpenDMARC could use such a header field for DMARC. There's no way to determine if it aligns.

So I don't see what you expect policyd-spf to do differently? As far as I know it correctly implements SPF (RFC 7208) and uses AR per the various AR RFCs. I haven't investigated anything in OpenDMARC beyond reading this issue, but from what I see here this is an OpenDMARC input validation issue. Nothing to do on the policyd-spf side.

Please provide specifics on 'behaving badly'?

glts commented 3 years ago

@kitterma I believe that in the course of dealing with this issue your points were understood by the OpenDMARC maintainers. They have applied fixes in OpenDMARC as documented in file CVE-2019-20790.

Indeed there is nothing to do on the policyd-spf side. This issue here should be considered done. It was left open by mistake.