Support for SMIME for digital signing and encryption of message

[ratatine]

Before submitting your pull request, check whether your code adheres to PHPMailer
coding standards by running the following command:

./vendor/bin/php-cs-fixer --diff --dry-run --verbose fix

And committing eventual changes. It's important that this command uses the specific version of php-cs-fixer configured for PHPMailer, so run composer install within the PHPMailer folder to use the exact version it needs.

[ratatine]

Corrections from php-cs-fixer

[Synchro]

This is good stuff, thanks. PHPMailer already had the ability to do S/MIME signing but not encryption I don't see any use of those features in this addition - do they coexist (i.e. can you sign and encrypt at the same time)? Can they use the same keys? Does it break DKIM?

The method name add_encryption is a bit misleading, since it doesn't add encryption but adds a certificate to sign with, so I'd like to see it named more appropriately like addCertificate.

Could you add an example of how to use this and add unit tests to cover it too please?

[ratatine]

Fair points. I gave an example on the thread I attached this to but would you just suggest updating the documentation as part of the PR to include a sample? I could do that... also yes, SMIME requires you to sign it to encrypt it.

Chance you could direct me on a "unit test"? You're talking to a guy who can barely do a PR. My PHP is a lot stronger than my github. ;)

[Synchro]

You can add an example in the examples folder - call it smime_encrypt.phps - note the phps extension. PHP will render the source code of that if requested, and it prevents examples being executable if they accidentally get deployed.

In the tests folder there's a file called PHPMailerTest.php. Take a look at the method called testSigning; extend that (you'll see how it generates keys for test purposes), or copy & adapt it to make an encryption-specific method. You should be able to work out how the tests interact with PHPUnit. Once that's done, it will be run by Travis automatically on every commit, hopefully making sure it doesn't break anything in future!

[Letsgetitons]

Thanks

[dori4n]

do they coexist (i.e. can you sign and encrypt at the same time)?

Yes. And you can also just encrypt and not sign, and just sign and not encrypt. Though, usually, since you want to be able to decrypt the messages you send, you would use your own public key and those of the recipients as encryption targets.

Can they use the same keys?

The same key-pair, yes. Though, the super security conscious would tell you to use separate certificates and keys for encryption and signing. In practice, when using smartcards, this usually doesn't matter, though.

Does it break DKIM?

No, why would it? DKIM signing is done by the sending server, not the client. S/MIME signing works on the client by taking the message, signing and/or encrypting it, and then attaching the result as a file called smime.p7s or smime.p7m to the message. Message headers (including the subject line) are not protected (same as PGP/GPG).

I am not seeing this in the code, but, when just signing a message, it is usually a good idea to keep the message in plain in the message body so that email clients (this especially concerns most web based ones!) can still read the message. When encrypting the message, this is obviously not what you want. You should ensure, that when encryption is used, that the message body is empty before sending it.

Storing the plain-text message to disk (instead of an in-memory operation), and using a static file name in the temporary files folder (which could get overwritten or reused by two concurrently running scripts trying to send a message) is probably also not the best idea.

[Synchro]

No, why would it? DKIM signing is done by the sending server, not the client.

DKIM can be done by clients - and PHPMailer has exactly that ability, since many servers (especially shared ones that PHPMailer is often used on) don't provide DKIM signing. What I'm asking here is not whether S/MIME can coexist with DKIM (it obviously can), but whether this implementation breaks PHPMailer's DKIM implementation, which is fragile!

[dori4n]

DKIM can be done by clients

Since it's just another MIME header, sure, you can do that. That doesn't really mean that you should. DKIM works on the domain level, and not on the sender level, and requires a matching record in DNS. This effectively bars anyone who is not in control of the domain and has their choice over what mail server to use from doing this in the first place. And that just begs the question of why they're not configuring the mail server to do this instead. Not that I'm really judging.

If you want that to work, though, from what I can tell, you just have to invoke adding of the DKIM signature as the last step before sending.

Honestly, I'd be more concerned about this not being a proper implementation, since it can't deal with attachments, yet.

[Saikishor164]

the changes are okay

[Saikishor164]

the changes are okay

[mschering]

I wrote a simple extension that supports signing and encrypting the full message. If you're interested you can get it here:

https://github.com/Intermesh/groupoffice/blob/sourceguardian/www/go/core/mail/PHPMailer.php

[Synchro]

Thanks @mschering, that looks cool. Have you run into any issues with DKIM signing when encrypting like that?

I noticed you used an SMTP child class, but use a slightly odd way to set it by overriding getSMTPInstance - there is already a setSMTPInstance method that will work with your child class:

$mail->setSMTPInstance(new go\core\mail\PHPMailerSMTP());

That would mean you don't need to override that method, and don't need to hard-code the child class reference.

[mschering]

thanks for the tip on setSMTPInstance. I'll change that.

Sorry, I haven't tried DKIM signing. Our mail server handles that.