trusteddomainproject / OpenDKIM

Other
97 stars 52 forks source link

Single quote in To: line causes invalid DKIM signature #42

Closed jikamens closed 5 years ago

jikamens commented 5 years ago

OpenDKIM generates a bad signature on this email message:

To: 'Jonathan Kamens' <example@example.com>
Subject: Testing one two three
From: Jonathan Kamens <jik@example.com>

Testing one two three

The culprit is the single quotes in the To: line. Take them away and the generated signature is valid.

P.S. Is anyone still maintaining this software?

jikamens commented 5 years ago

I spent a ridiculous amount of time debugging this and here's what I figured out...

In the default configuration, current versions of sendmail consider apostrophes in the name portion of addresses to require quoting. The sendmail documentation claims that this is required by RFC 2822, but I pored over RFC 2822 at length and I don't actually think that's correct.

As a result, if you put the string 'Foo Bar' in the To line of a message, then sendmail replaces it with "'Foo Bar'", but that occurs after OpenDKIM generates a signature on the From line.

I'm not sure what the right fix for this is. I don't know, e.g., whether it's possible to get sendmail to feed headers to the OpenDKIM milter after they've been quoted rather than before. Maybe OpenDKIM needs a configuration setting telling it which characters in the name portion of addresses are going to be quoted by the mailer if they're not quoted, and if it encounters any such characters that are unquoted it needs to add quoting before performing the signature? Or maybe it should actually do quoting and modify the headers, thus ensuring that they are quoted before sendmail sends them regardless of what sendmail's settings are?

As a workaround I changed the MustQuoteChars setting in my sendmail mc file to omit the apostrophe, but that's not really a good overall solution.

jikamens commented 5 years ago

I take it back, sendmail's interpretation of RFC 2822 is correct. While an apostrophe in an address display-name consisting of a single word does not require quoting, if there are multiple words in the display-name then quoting is required.

dilyanpalauzov commented 5 years ago

This is addressed at https://github.com/trusteddomainproject/OpenDKIM/commit/2a267b031e0e0961afcd461d1d04909b0f9a967a.

jikamens commented 5 years ago

It's nice that this is documented, but there should be a better fix. It should not be necessary for users to change sendmail's configuration so that it is not compliant with the most recent SMTP standard in order for OpenDKIM to work.

dilyanpalauzov commented 5 years ago

I have communicated this to the sendmail author. He said he is not going to change the default for MustQuoteChars, but I think he documented this dkim misexpectation somewhere in sendmail. Problematic are real emails From: Pooking.com <zzz@pooking.com>, when the emails are used to send discount codes, the sender is a big company, the emails are dmarc protected and the sending domain do now answer emails asking to quote the display-part.

You can write the sendmail author to change the behaviour. I wrote the rspamd and dkimpy authors to document this and also clarified, that postfix does not behave in the same way.

jikamens commented 5 years ago

I am not going to write to the sendmail author to change the quoting behavior, because I believe the sendmail quoting behavior is correct.

The RFC indicates that "." and "'" in multi-word display names need to be quoted under the current RFC requirements. It allows them to be unquoted but states that this is obsolete and new software should not be built to rely on that. Sendmail is therefore, correctly, not relying on that by default.

I suppose the extent to which the sendmail behavior is incorrect is that single-word display names are allowed to contain apostrophes without being quoted, but frankly I consider that a bug in the RFC as it's an inconsistent and confusing distinction and I would not recommend for Sendmail to take advantage of it. It certainly isn't incorrect to quote single-word display names containing apostrophes, it's just technically not required by the RFC.

I suppose you could argue that Sendmail's behavior is incorrect in the sense that it should be doing quoting before feeding header fields to milters rather than after, but I totally understand why it's not doing that -- milters are on the input end and quoting is on the output end -- and I'm not sure it's feasible to fix it within its architecture without a great deal of work.

You could fix this without involving Sendmail at all by having the OpenDKIM milter detect headers which fall into this category and fix them itself, i.e., OpenDKIM could modify To headers to add quoting (if configured to do so) and sign the modified to headers and then sendmail wouldn't need to modify them again because they're already modified. I am pretty sure it would be easier to do this than to convince the sendmail maintainers to change how sendmail behaves.

dilyanpalauzov commented 5 years ago

For a server that receives an email for an alias, verifies DKIM/DMARC, DKIM signes the email and forwards it to another server, neither sendmail nor a milter can modify the email, e.g. by adding quotes, as this breaks the DKIM-Signature of the sending domain and effectively looses the email, if the sender applies some DMARC policy. So the correct behaviour is not to modify the message anyhow, and this behaviour is implemented by Postfix.

dilyanpalauzov commented 5 years ago

How do you propose to resolve this?

There is no ideal configuration, so one has to read the documentation and make a tradeoff between keeping signatures, inserted by other servers, valid and aligning the display-part of address headers to be quoted properly.

jikamens commented 5 years ago

OK, so I did some more research, and I have now come down soundly on your side: sendmail's behavior is incorrect and the maintainer of sendmail is prevaricating and rationalizing rather than acknowledging that sendmail is buggy, apparently because if sendmail is buggy then he would be expected to do the work to fix it.

My interpretation of the RFCs above is incorrect. the RFCs do allow apostrophes in unquoted display names. Sendmail's default behavior of quoting display names containing apostrophes is therefore simply wrong.

I now agree with you that the right place to fix this is in sendmail, not OpenDKIM. Sorry to bother you.