Skip to content
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.

Sign up for GitHub

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

Jump to bottom

incorrect parsing Auth results DKIM #45

Open
incous opened this issue Mar 26, 2024 · 1 comment · Fixed by #51
Open

incorrect parsing Auth results DKIM #45

incous opened this issue Mar 26, 2024 · 1 comment · Fixed by #51
Labels
bug Something isn't working

Comments

@incous
Copy link

incous commented Mar 26, 2024 •
edited

It happen for not parsing the correct DKIM Auth Result as the message may got multiple DKIM signatures.
Example:

<auth_results>
  <dkim>
    <domain>[domain1]</domain>
    <result>pass</result>
    <selector>select1</selector>
  </dkim>
  <dkim>
    <domain>[domain2]</domain>
    <result>fail</result>
    <selector>selector2</selector>
  </dkim>
  <spf>
    <domain>[domain1]</domain>
    <result>pass</result>
  </spf>
</auth_results>

I got this, and the parsed HTML show this entry as DKIM auth fail, SPF auth pass, but DMARC still pass for DKIM & SPF
@tierpod tierpod added the bug Something isn't working label Apr 4, 2024
moorereason added a commit to moorereason/dmarc-report-converter that referenced this issue Apr 13, 2024
@moorereason
Fix incorrect parsing of auth_results
7f3a0a5 
The DMARC spec allows for zero or more DKIM results and one or more SPF
results.  Prior to this commit, we only allowed for a single result for
each type.  Update the dmarc.AuthResults struct to use a slice for each
type.

For reference, see the definition of AuthResultType in Appendix C of RFC
7489.

This change in the data model required updates to the Go templates.  The
two HTML templates are almost exactly the same, so pull the common body
out into a separate const variable to ease maintenance.

Fixes tierpod#45
@moorereason moorereason mentioned this issue Apr 13, 2024
Merged
@tierpod tierpod reopened this Apr 14, 2024
@tierpod
Copy link
Owner

tierpod commented Apr 15, 2024

Hi @incous,

I merged PR that should fix this problem yesterday. I'm going to make a new release soon. It would be nice if you could build dmart-report-converter from master branch and do some tests.

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Handle multiple auth results moorereason/dmarc-report-converter
2 participants
@incous @tierpod