lieser / dkim_verifier

DKIM Verifier Extension for Mozilla Thunderbird
MIT License
208 stars 34 forks source link

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

Open ale5000-git opened 5 years ago

ale5000-git commented 5 years ago

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 commented 5 years ago

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.

ale5000-git commented 5 years ago

@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 commented 5 years ago

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.

alevesely commented 3 years ago

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) header.i=@ietf.org
        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 commented 3 years ago

@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 commented 3 years ago

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