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

DKIM fails without To and ignores CC #1975

Closed
KlaasBonnema opened this issue Feb 18, 2020 · 16 comments
Closed

DKIM fails without To and ignores CC #1975

KlaasBonnema opened this issue Feb 18, 2020 · 16 comments

Comments

@KlaasBonnema
Copy link

The DKIM signature is invalid it an email is sent using mail() that does not have a To: address.
PHPMailer tests for this condition in presend() and creates a header field To: undisclosed recipients; However mail() does not know about this and creates an empty To:. Now the DKIM signature is invalidated.
Solution is to set an empty To: header in presend id mail() and no to addresses:
$this->mailHeader .= $this->headerLine('To', '');

The $autoSignHeaders array contains a "CC" value but the following test is case sensitive and the actual header is "Cc". Now the Cc: field is not signed where it was intended to be signed.
Solution is to change SautoSignHeaders value to "Cc".

@Synchro
Copy link
Member

Synchro commented Feb 18, 2020

Good catch, thanks

@Synchro
Copy link
Member

Synchro commented Feb 19, 2020

I solved the second problem by making the comparison case-insensitive in general.

The first one is more difficult. Setting To to undisclosed-recipients:; (an RFC822 empty group address, see example in RFC2822) is correct for a BCC-only message (the group name is convention; bananas:; would work the same). If this doesn't work in mail(), that is a bug in mail(). I've not checked it – does leaving the To header empty result in mail() sending a message with undisclosed-recipients:; as the To header? Doing what you suggest might make the code run, but it will likely result in invalid messages that will probably never arrive.

@KlaasBonnema
Copy link
Author

I tried to set $this->to[] to something but my attempts miserably failed. (It may still be possible though and I may just have missed the right syntax.) I guess that only works if you can set a value "undisclosed-recipients:;" that will be somehow recognised as a permissable To: value AND it has to match the DKIM signed value.
Then I set the header To: to nothing and that worked. My mail() implementation sends an empty To: header, matching what was signed by DKIM. My mail program (Thunderbird) shows no To value or defaults to showing the Cc value, depending where you look. I have routinely sent out many emails without To: or Cc: in the past using mail() without any bounces.
I searched in my mail archives and did find a DKIM signed mail with a header To: undisclosed-recipients:; - this mail does not appear to be sent using mail(). I noticed that PHPMailer only sets To to undisclosed-recipients using mail()?

@Synchro
Copy link
Member

Synchro commented Feb 19, 2020

It's set for SMTP sending on line 2392. mail() adds the To header itself, so PHPMailer puts it into $mailHeaders rather than the main headers.

I can't see anything that says that the To header must exist, but I'd guess that its absence might be problematic for some, and removing it unilaterally to work around a bug in mail() seems excessive. This is the bit in RFC5322 § 3.4 about using empty groups:

When it is desirable to treat several mailboxes as a single unit
(i.e., in a distribution list), the group construct can be used. The
group construct allows the sender to indicate a named group of
recipients. This is done by giving a display name for the group,
followed by a colon, followed by a comma-separated list of any number
of mailboxes (including zero and one), and ending with a semicolon.
Because the list of mailboxes can be empty, using the group construct
is also a simple way to communicate to recipients that the message
was sent to one or more named sets of recipients, without actually
providing the individual mailbox address for any of those recipients.

@KlaasBonnema
Copy link
Author

I think that we can conclude that an empty To: is not forbidden and we currently have no indication this will fail, but the syntax To: undisclosed-recipients:; is also allowed.

  • The purpose to the end-user is to signal that there were invisible BCC addresses used. This is however also implied by an empty To:
  • When PHPMailer would make the decision to set To: to undisclosed-recipients then it would be consistant to do this also for other mail interfaces, not just for mail();
  • I think it might be appropriate to do so when we only have BCC addresses. If there is a CC address then it will still give a visual (CC) address to the end user and an empty To address would be implied.
  • "undisclosed-recipients" is an English term that may not be meaningfull to non-English speakers. However I have never seen a translation where I have seen empty To values.
  • We might consider to make To: undisclosed-recipients an application choice. I guess I could set a To address with this value from the application (is this working?). Then it would be appropriate to leave To: empty by default.

@Synchro
Copy link
Member

Synchro commented Feb 19, 2020

PHPMailer does set undisclosed-recipients for transports other than mail(); it is consistent?

As I said, what you call the empty group doesn't matter; undisclosed-recipients has been a common default since

I just did some tests sending BCC-only emails from various places to see whether they include a To header.

  • Apple Mail: No
  • iCloud: No
  • Gmail: Yes, with undisclosed-recipients
  • Yahoo: No
  • Outlook/Live: No

One other problem: Spamassassin penalises 1.0 point for missing a To header.

X-Spam-Report: 
	*  1.0 MISSING_HEADERS Missing To: header

RFC822 says (though not in a normative way):

the "Bcc" field may be empty, while the "To" field
is required to have at least one address.

Their example suggests that To can be omitted altogether, but if it's present it should not be empty; An empty group counts as "one address". RFC2822 describes that a little differently, and also uses an example of an empty undisclosed-recipients group in CC rather than BCC, which seems a little strange. 5322 says the same, and I quoted its other comments before.

@KlaasBonnema
Copy link
Author

Ah, I now see the undisclosed-recipients in createHeader().
Should we then set it here also for mail()? In the old version the test was in presend() for an empty to[] which caused the DKIM to to be set to undisclosed-recipients. It seems more logical to do this earlier and let presend() find the non empty to[]?

To be honest, I am ok with an empty To as well as an undisclosed-recipients To, as long as the DKIM is ok.

@Synchro
Copy link
Member

Synchro commented Feb 21, 2020

You can't add a To header in the message for mail() – you have to pass it as one of the params to the mail() call, and you have to hope that PHP adds the To header in exactly the same way as PHPMailer does. This is what the $mailHeaders var holds. That's one of several reasons why mail is a pain to work with!

I may not have understood what you meant – could you fork and make a PR with your suggested change?

Going by the RFC, and what other mailers do, I think we should either omit To altogether or use undisclosed-recipients. Using an empty To goes against both of those things, with unknown consequences, so I don't think it's a good idea.

@KlaasBonnema
Copy link
Author

Mail() wants a $to, $subject, $text and $headers. If mail() requires a $to value and thus creates a To: header then there is no way to leave out To:. This would probably require a To: DKIM header in any case, preventing an added To in transmission. I currently do not see where an empty To would violate RFC's or cause a problem.

I did not fully follow the code from createHeader() to presend() and DKIM_add(). What I noticed it that there are separate places where undisclosed-recipients is added. It would conceptually make sense to do this early on in one place. That implies that this does not conflict with the separate handling of To and Subject for mail(). Similarly in an ideal world you would create the complete message with CRLF line endings and only at the very end convert for mail() on non-windows only to CR.

Mail() has some differences with direct SMTP but I value the advantages. Many mail providers have a much larger limit for maximum number of mails per day for mail() then they do for SMTP. When you use mail() you do not need to expose the mail credentials in your application code. It is the provider that takes care of it. Mail() can only be accessed on localhost and not remotely, again reducing risks. It is said somewhere that direct SMTP is faster sending multiple mails but is this also true for applications that instanciate a PHPMailer class for every mail?

I did find an application that translates undisclosed-recipients to local language. Thunderbird will do that if you specify Bcc without To or Cc on outgoing mail (using SMTP). However with translations you can never win. If I prepare an email on my localized Thunderbird and send it to a non native speaker then he will get my localized "undisclosed-recipients". (Which I guess votes for an empty or missing To.)

@Synchro
Copy link
Member

Synchro commented Feb 21, 2020

Using mail() is almost always a bad idea because of things like this. All that mail() does internally is run a separate binary (sendmail) that opens a synchronous SMTP connection to localhost and submits the message over it; it's both faster and safer to use SMTP to localhost directly to achieve the same thing instead. It does not expose any credentials because it doesn't use external network access, and a mail server on localhost doesn't usually need them anyway. Any sending limits on using mail() will apply equally to messages sent via SMTP to localhost since they are essentially one and the same. The one exception to this is on Windows, where the mail function in PHP is often configured to use an external (non-local) mail server because Windows generally doesn't have a local mail server by default, whereas Linux/macOS nearly always does, and that implies embedding credentials in PHP config, which I think is worse, especially since it has very limited security features as a mail client.

mail() is also hard to debug - all you get is a true/false response, no clue as to what the problem is - and many using it on shared hosting have no access to local mail server logs that would allow them to see error messages. SMTP to localhost doesn't have that problem.

I don't know what happens if you call mail with a null value in $to; I've not tried it.

If I were to rewrite PHPMailer (which may or may not be happening!), I would probably drop support for mail() altogether. It's not worth the hassle.

You should not instantiate a new PHPMailer instance for sending multiple messages – see the mailing list example for how to do that. My own system can send up to around 200 messages per second using PHPMailer this way.

@KlaasBonnema
Copy link
Author

With 200 messages per second my provider would block me just after one second for exceeding his max messages limit using SMTP.
I believe I cannot call localhost SMTP without credentials where I am hosted on a shared server.
If PHPMailer drops mail() support it means that I have to look around for other mailer libraries. Actually I was using a library that did all I needed with only one missing feature - DKIM.

@Synchro
Copy link
Member

Synchro commented Feb 21, 2020

Rate and volume limits are not PHPMailer's concern! That's a higher-level problem to deal with in apps.

Try it - most shared hosting that allows mail() does not use authentication because mail() does not support authentication at all, and it sends via SMTP to localhost, which is most often a relay to some other nearby server. GoDaddy (possibly the world's biggest shared hosting provider) goes one further – they use a non-local (but on their internal network) mail server without authentication, and in fact it will fail if you even try to use auth.

@KlaasBonnema
Copy link
Author

mail() with $to=null is allowed. This results in an empty To: header. So sending with mail() is not possible without To: header.

My provider does not allow SMTP on localhost without specifying credentials. I do not need credentials using mail() so I do not need to expose them in my PHP code.

Volume limits are indeed not PHPMailers concern but I have to deal with it when I choose between SMTP or mail() and that is where PHPMailer gets involved.
I had a look at my last mailing batch and I get very close to 200 messages per minute with mail() too and that includes the application overhead in building customized messages.

@Synchro
Copy link
Member

Synchro commented Feb 21, 2020

That's interesting. It's unusual for providers to allow authentication without encryption. Can you show a transcript?

Mail imposes some limits on throughput because it can't support keepalive. It also has a habit of corrupting messages by imposing unnecessarily short line lengths (< 76 chars, see old PHPMailer bugs).

@KlaasBonnema
Copy link
Author

Unfortunately no transcript. There are limits to what I want to do on this live server.
$ErrorInfo='SMTP connect() failed. https://github.com/PHPMailer/PHPMailer/wiki/Troubleshooting'.
I specified localhost and port 25 without username and password (same as mentioned in PHPinfo.)
"It's unusual for providers to allow authentication without encryption." But the objective of the test was to see if I could submit with SMTP on the local server, bypassing mail(), without specifying credentials. I can submit successfully using SMTP with credentials and I can also choose STARTTLS or SSL/TLS.

@Synchro
Copy link
Member

Synchro commented Feb 21, 2020

OK. That suggests that your local PHP config is not sending mail through localhost but some remote server. We are straying off topic here!

Given mail's behaviour with no to address, I think undisclosed-recipients is the least-bad option available. It's the same as commonly used elsewhere, and it would work identically across mail and SMTP transports in PHPMailer, and make DKIM signing easier (fewer combinations to handle). I'm really not bothered about the group name; undisclosed-recpients has been in common use for > 40 years, and it's as good a default as any.

@Synchro Synchro closed this as completed Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants