techsneeze / dmarcts-report-viewer

DMARC Report Tool for use with rddmarc or dmarcts-report-parser (formerly imap-dmarcts)
http://www.techsneeze.com/dmarc-report/
GNU General Public License v3.0
192 stars 56 forks source link

DKIM results depend on the validation result for the first selector #47

Closed ghost closed 3 years ago

ghost commented 3 years ago

In case a DMARC reports sender includes more detailled information regarding DKIM signatures (mostly the name of the selectors observed), dmarcts-report-viewer does not check whether any of those selectors was successful validated, but only checks the first one.

This issue causes successfully validated messages to show up as "DKIM failed", especially if the sender's setup involves dual-signing - for example, using a RSA and ED25519 DKIM key. The latter is smaller and faster to sign and validate, but not yet fully supported everywhere.

DMARC report XML snipped of a message displayed as "DKIM fail":

  <record>
    <row>
      <source_ip>x</source_ip>
      <count>1</count>
      <policy_evaluated>
        <disposition>none</disposition>
        <dkim>pass</dkim>
        <spf>pass</spf>
      </policy_evaluated>
    </row>
    <identifiers>
      <header_from>x</header_from>
    </identifiers>
    <auth_results>
      <dkim>
        <domain>x</domain>
        <result>fail</result>
        <selector/>
      </dkim>
      <dkim>
        <domain>x</domain>
        <result>pass</result>
        <selector>202003rsa</selector>
      </dkim>
      <spf>
        <domain>x</domain>
        <result>pass</result>
      </spf>
    </auth_results>
  </record>

In contrast, this record will be displayed as "DKIM pass", although it contains exactly the same information:

  <record>
    <row>
      <source_ip>x</source_ip>
      <count>1</count>
      <policy_evaluated>
        <disposition>none</disposition>
        <dkim>pass</dkim>
        <spf>pass</spf>
      </policy_evaluated>
    </row>
    <identifiers>
      <header_from>x</header_from>
    </identifiers>
    <auth_results>
      <dkim>
        <domain>x</domain>
        <result>pass</result>
        <selector>202003rsa</selector>
      </dkim>
      <dkim>
        <domain>x</domain>
        <result>fail</result>
        <selector/>
      </dkim>
      <spf>
        <domain>x</domain>
        <result>pass</result>
      </spf>
    </auth_results>
  </record>

Thanks for having a look at this - and thanks for providing dmarcts-report-viewer, it is most useful. :-)

jnew-gh commented 3 years ago

If I add the above results into a test report and import it into the database, I get:

IP Host Name Message Count Disposition Reason DKIM Domain DKIM Result SPF Domain SPF Result Color
x x 1 none   x/x fail x pass orange
x x 1 none   x/x pass x pass green

Looking at the x/x in the DKIM Domain column, the viewer does recognize that there are two DKIM results. It just needs to show the result, like fail/pass and pass/fail respectively, in the DKIM Result column, and then decide if that is an overall DKIM pass or fail.

As a test, I added a third DKIM result in each record, and the DKIM Domain column shows x/x/x.

Question: Should the fail/pass and pass/fail combination result in an overall DKIM pass or an overall DKIM fail?

I had a report just the other day that contained a record with DKIM results from two different domains (i.e. x/y with a pass/fail, where x was Yahoo.com and y was my own domain). In the situation where there are multiple DKIM results from different domains, I would argue that a DKIM fail from any one domain should result in an overall DKIM fail.

In the same report, another record contained two DKIM results from my own domain. In the situation where there are multiple DKIM results from the same domain, I would argue that one DKIM pass should result in an overall DKIM pass. This would allow an overall DKIM pass in the OP's situation where multiple signing strategies are employed.

Does that sound right? Getting the logic right on this will be tricky.

As for how multiple DKIM results are shown in the Report Detail table, how about: IP Host Name Message Count Disposition Reason DKIM Domain DKIM Result SPF Domain SPF Result
x x 1 none   x
x
dkim: fail
202003rsa: pass
x pass
x x 1 none   x
x
202003rsa: pass
dkim: fail
x pass
x x 1 none   x
y
dkim: pass
dkim: fail
x pass
ghost commented 3 years ago

Question: Should the fail/pass and pass/fail combination result in an overall DKIM pass or an overall DKIM fail?

Quoted from RFC 6376, section 4.2:

[...] Verifiers SHOULD continue to check signatures until a signature successfully verifies to the satisfaction of the Verifier. [...]

I read this as any successful verified DKIM signature is sufficient for a "DKIM pass", which is the behaviour I observe for most DKIM verifiers, at least if the effective second level signature domain is the same as the one used in the From: header.

I had a report just the other day that contained a record with DKIM results from two different domains (i.e. x/y with a pass/fail, where x was Yahoo.com and y was my own domain). In the situation where there are multiple DKIM results from different domains, I would argue that a DKIM fail from any one domain should result in an overall DKIM fail.

In the same report, another record contained two DKIM results from my own domain. In the situation where there are multiple DKIM results from the same domain, I would argue that one DKIM pass should result in an overall DKIM pass. This would allow an overall DKIM pass in the OP's situation where multiple signing strategies are employed.

Yes, this sounds reasonable to me. :-)

The proposed table looks good to me as well, but unfortunately has to deal with DKIM selector names missing, as not all DMARC senders include them in their reports or - in case of Google - omit the selector name in case of certain failures (it was 202003ed25519; apparently some parts of the Google infrastructure do not comply to RFC 8463 yet). Just to have that quirky real-world problem mentioned...

jnew-gh commented 3 years ago

Looking at the code, this issue has more to do with the techsneeze/dmarcts-report-parser. I've submitted a patch and opened pull request techsneeze/dmarcts-report-parser/pull/78

Unfortunately, I don't think that my proposed representation for the Reports Data will work at this point. The problem is that the database restricts the contents of the DKIM Result field to a set of values ('none','pass','fail','neutral','policy','temperror','permerror') so showing all the DKIM results isn't possible without changing the database structure.

So for now, if that patch is merged, the DKIM Result will show "pass" if any one result is a "pass"; will show nothing if any of the results are different (and none are a "pass"), and; will show the common result if all the results are the same.

techsneeze commented 3 years ago

I've (finally) merged those changes in to the repo!