-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
Support for SMIME for digital signing and encryption of message #1611
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrections from php-cs-fixer
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 Could you add an example of how to use this and add unit tests to cover it too please? |
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. ;) |
You can add an example in the examples folder - call it In the |
Thanks |
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.
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.
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 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. |
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! |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the changes are okay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the changes are okay
I wrote a simple extension that supports signing and encrypting the full message. If you're interested you can get it here: |
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
That would mean you don't need to override that method, and don't need to hard-code the child class reference. |
thanks for the tip on setSMTPInstance. I'll change that. Sorry, I haven't tried DKIM signing. Our mail server handles that. |
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.