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.

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

Improve logic for showing DKIM header if replacing the verification result with ARH is disabled #148

Open
ale5000-git opened this issue May 18, 2019 · 6 comments
Assignees

Comments

@ale5000-git
Copy link

ale5000-git commented May 18, 2019

I have a message that contains:
Authentication-Results: smtp.github.com; dkim=permerror (bad message/signature format).

But the plugin just say DKIM: None without any warning/error.

I have sent you a sample.

@lieser
Copy link
Owner

lieser commented May 18, 2019

I had a lock at the e-mail you send me. It does not include any DKIM signature, so the add-on behaves correctly (assuming you have reading of ARH disabled, or configured that the ARH result does not replace the one from the local verification).

Why it also contains the Authentication-Results: smtp.github.com; dkim=permerror (bad message/signature format) is strange. Not just because of the dkim=permerror part, even if the e-amil does not contain any DKIM signature. But also because its added by the sender (GitHub), and normally a sender does not add a Authentication-Results in it's outgoing e-mails.

But this strange behavior should have nothing to do with the add-on.

@lieser lieser self-assigned this May 18, 2019
@ale5000-git
Copy link
Author

ale5000-git commented May 19, 2019

@lieser: I have the option of reading ARH enabled, this is the problem.
It should display the error.

PS: I suppose github smtp server fail to add dkim signature because the message did contain strange chars so it report the error.

PS2: Look at the sender name:

bad-sender-name

@lieser
Copy link
Owner

lieser commented May 20, 2019

If you have just ARH enabled, the add-on should also show DKIM: invalid.

Only if you have additionally enabled in the advanced options that the ARH result does not replace the normal verification, the problem you describe should occur. because in this case only the result of the normal verification matters for deciding whether to show the header.

I will make a feature request out of this issue, to improve the logic for showing the header.

@lieser lieser changed the title dkim=permerror not displayed Improve logic for showing DKIM header if replacing the verification result with ARH is disabled May 20, 2019
@lieser lieser added this to the 2.1.0 milestone May 20, 2019
@lieser lieser removed this from the 2.1.0 milestone Aug 11, 2019
@alevesely
Copy link

Philippe,
I confirm that logic should be revised. I'm running the DKIM Verifier version of 22 January 2020.

I have enabled "Reading the Authentication-Results header replaces the add-ons verification". However, a message having a correct A-R header shows "Error connecting to the DNS server". The error is caused by a temporary network outage. The question is: Why does the add-on try to connect to a DNS server? The A-R is as follows:

    Authentication-Results: wmail.tana.it;
      spf=pass smtp.mailfrom=ietf.org;
      dkim=pass (whitelisted) [email protected]
        header.b=E/kq0j4s

Note that one of the possible results is temperror:

temperror: The message could not be verified due to some error that is likely transient in nature, such as a temporary inability to retrieve a public key. A later attempt may produce a final result.
https://tools.ietf.org/html/rfc8601#section-2.7.1

In that case only, the add-on should discard the A-R result and attempt verification anew. For all the other results, the add-on should not even try to reach for the network.

@lieser
Copy link
Owner

lieser commented Oct 5, 2020

@alevesely

The question is: Why does the add-on try to connect to a DNS server? The A-R is as follows:

The ARH you posted is not valid (/ in the header.b is not allowed without quotes), which will result in a parsing error. As a fallback, the add-on will try it's own validation.
Note as this is unfortunately rather common, the add-on allows to relax the parsing for this case: https://github.com/lieser/dkim_verifier/wiki/Options#try-to-read-non-rfc-compliant-authentication-results-header

If you do not want this fallback, you can completely disable the DKIM verification (also only for specific accounts): https://github.com/lieser/dkim_verifier/wiki/Options#verify-dkim-signatures

For the future, I suggest enabling debugging and locking at the output (see https://github.com/lieser/dkim_verifier/wiki/Debug). In most cases, the messages should tell also not that familiar with DKIM/the add-on that is going wrong (at least I hope so).
In your case, a parsing error should show up in the log (why the parsing fails is unfortunately not obvious from the error).

@alevesely
Copy link

Oops... Thank you. I'll fix the ARH.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants