fabacab / wp-pgp-encrypted-emails

:closed_lock_with_key: :e-mail: Encrypts WordPress emails using OpenPGP or S/MIME with a familiar API.
https://wordpress.org/plugins/wp-pgp-encrypted-emails/
GNU General Public License v3.0
39 stars 10 forks source link

Problems with Formidable Integration #8

Closed boblmartens closed 8 years ago

boblmartens commented 8 years ago

"Integration" is a strong term here, but this is to follow the Formidable issues I have been having.

Here is the error:

PHP Warning: trim() expects parameter 1 to be string, array given in /srv/www/htdocs/wp-includes/class-wp-user.php on line 201, referer: http://blogs.mlc-wels.edu/bits-to-bytes/test-page/

This was shown using Formidable 2.0.x, WP-PGP-Encrypted-Emails 0.3.0, and WordPress 4.4.2 on SLES 11 SP3.

fabacab commented 8 years ago

@boblmartens Can you please test your Formidable plugin with this patched version of WP PGP Encrypted Emails? I am not sure if it will work and need your feedback about the Formidable Pro version since I do not have access to that myself.

The patch (currently visible in 3a80525416befe42bb6b94afc9832c26fcf7b5dd) manually inspects the $to argument and handles each recipient individually. This should mean the plugin will now support plugins (like Formidable) that call wp_mail() with an array instead of a string, but I cannot be certain because, again, I don't have the source.

So if you would please manually install the patched version (download link) and let me know if it works for you (and if anything else breaks). Thanks!

boblmartens commented 8 years ago

It did not work on the test site. Here is the new generic error:

[Sun Feb 28 22:43:27 2016] [error] [client 192.168.95.16] Cannot send encrypted email to martenrt@mlc-wels.edu, referer: http://blogs.mlc-wels.edu/bits-to-bytes/test-page/

Quite possible I am just not setting something up correctly.

fabacab commented 8 years ago

That is a different error than before, so it is at least a step in the right direction. That error indicates that there was a problem encrypting the message with the key associated with the user who has the email address shown. First, double-check that the user who has the email address shown in the error message has a valid PGP public key saved in their WordPress profile.

boblmartens commented 8 years ago

Is there a minimum version accepted?

fabacab commented 8 years ago

You must have WordPress 4.4 or later for all features to work as intended, but this is checked by the plugin itself upon activation. If you do not receive an error about an incompatible WordPress version when you activate the plugin, it should be fine, unless there is a problem with the version checking code.

boblmartens commented 8 years ago

I was wondering for the PGP key itself. I created a new one and now it sends encrypted emails, but I am getting a duplicate email sent that is not encrypted at all.

fabacab commented 8 years ago

but I am getting a duplicate email sent that is not encrypted at all.

Is one of those emails being sent to the site's admin email address (as opposed to the user's email address)? If so, and if that other email address is also one of yours and so is being routed to you, it will appear "duplicated," even though it's not….

boblmartens commented 8 years ago

Yes, it is being sent to the admin (which is the same). Should that email also be encrypted or no?

Sincerely, Bob Martens

fabacab commented 8 years ago

For emails to the admin to be encrypted, you must tell WordPress which PGP public key to use; even if the email address is the same for the admin as for a user account, you need to explicitly enter the PGP public key in the WordPress General Settings screen for the emails to the admin address to be encrypted, too. This is because WordPress treats "notifications to the admin address" as a separate action than a "notification to a user account."

boblmartens commented 8 years ago

I used the same key as before, but it is still coming through plain text (not encrypted). Do the keys need to be unique?

Sincerely, Bob Martens

fabacab commented 8 years ago

I used the same key as before, but it is still coming through plain text (not encrypted). Do the keys need to be unique?

No, the keys can be the same. But they need to be entered twice. Once in the user's profile screen, and once in the site's own General options screen.

That is how I set up my test site and it works fine for me….

boblmartens commented 8 years ago

Yes, I have done that. I’ll keep playing around with things.

Sincerely, Bob Martens

boblmartens commented 8 years ago

Is the admin email getting a notification email on form submission a feature of Formidable or WordPress?

Sincerely, Bob Martens

fabacab commented 8 years ago

Is the admin email getting a notification email on form submission a feature of Formidable or WordPress?

I do not know because I do not know what form you are referring to. For the purposes of this plugin, though, it shouldn't matter. As long as Formidable uses wp_mail() to send mail, this plugin will intercept and attempt to encrypt/sign the outgoing message.

boblmartens commented 8 years ago

I’ll continue more work this week while I’m at work to see if I can track down the odd way it is performing for me and then I’ll report back here. Thanks!

Sincerely, Bob Martens

fabacab commented 8 years ago

I went ahead and released this in version 0.4.1 because I cannot find any issues with the patch, myself, and all my own sites work with it. So it's probably safe. Give it a shot and hopefully it works for you, too.

boblmartens commented 8 years ago

I just upgraded. Didn't break anything, but I'm still running into issues around the duplicated (non-encrypted) email being sent. I disabled the wp-gpg-encrypted-email plugin and the duplicate email did not come through.

I have some other stuff to work on, but if you have any ideas for how I could get you more/better information, please let me know.

fabacab commented 8 years ago

I'm not seeing this duplicated issue at all, so I'm not sure what the issue could be. Weird. :\

boblmartens commented 8 years ago

I agree on the "weird" part.

I am working with multisite installs. I'll probably throw this back at Formidable as well and see if they know any reason for duplicate emails being sent.

fabacab commented 8 years ago

I am working with multisite installs.

Oh! I haven't tested on multisite at all. I will do that tomorrow (or ASAP), I'm exhausted now.

boblmartens commented 8 years ago

Take a rest. I think I need to work with one of our departments to redo their PGP key anyway.

fabacab commented 8 years ago

I couldn't sleep so I've just finished testing the plugin against the latest Multisite with a brand-new WP Network and did not notice any duplicated emails. Encryption works, keypair generation works, but nothing I could do triggered a duplicate email. Maybe if you have more details (although I'm not even sure what details I'm looking for) about how you're trying to send email I can look more closely into those issues but for now I'm afraid I have to consider this issue well and truly closed since I just can't reproduce the issue.

FWIW, this also suggests to me that you might want to throw the issue back at the Formidable developers and ask them whether or not they're sure they're using wp_mail() and only wp_mail() in all parts of their plugin. :\

Sorry I couldn't be more helpful, I think this is where my journey on this particular issue ends.

boblmartens commented 8 years ago

Thanks for the help. If I find anything additional, I'll let you know!

boblmartens commented 8 years ago

Alright, I'm going to do just a little more thinking here (feel free to ignore or give me some more to think about):

When I disable the plugin, it works as designed, so it has to be something in my environment messing with the email delivery. I'll keep looking around at different options.

fabacab commented 8 years ago

Alright, I'm going to do just a little more thinking here

Go for it! Documenting your findings as you go may help others in the future, too, so please feel free to use this space for that.

boblmartens commented 8 years ago

I'm guessing you'll get sick of me soon, but this is what I received back from Formidable:

This is an issue with their code. Their code includes this line which is sending the email again: wp_mail($to, $subject, $message, $headers, $attachments);

I think this is in like 807 of wp-pgp-ecrypted-emails.php but I have a question out with them again.

fabacab commented 8 years ago

Yeah, no, that's not "sending again." Read the rest of the method (the surrounding lines). It's pretty well commented.

boblmartens commented 8 years ago

I'll do that tomorrow morning. Thanks.

Sincerely, Bob Martens

http://bobmartens.net @boblmartens

boblmartens commented 8 years ago

I've just been playing around with edits, and removing that wp_mail call obviously kills the encrypted message, but not the unencrypted one (really cool). Killing the line below still sends both and kill the line above stalls out the process after 30 seconds (gets caught up on BigInteger it says).

So obviously there needs to be another call to wp_mail SOMEWHERE on my site, in some plugin or something, that is still sending the message. So now I head back to Formidable to see if they can shed any light on how their system is setup.

fabacab commented 8 years ago

Yup, all of those symptoms make perfect sense. The call to wp_mail() they complained about is the call made from, presumably, their own plugin. It's just getting transparently proxied through WP PGP Encrypted Emails, which is exactly what's supposed to happen.

You won't notice anything by killing the line below because that line is what's ensuring future emails also go through WP PGP Encrypted Emails, but if this is the only email being sent in the current call, then it has no effect.

Killing the line above will create an infinite loop where the message is encrypted as many times as your server will be able to execute before PHP's maximum execution time out is hit, which is evidently 30 seconds (the default) for you.

So yes, all of that is exactly as designed. The problem must be somewhere else.

boblmartens commented 8 years ago

It all made sense to me as well.

I love this sort of investigation. Love. It. :-)

boblmartens commented 8 years ago

I'm thinking I'll try another plugin that sends email notifications and see how it responds. That will help to eliminate some of the possibilities.

You can safely ignore what I'm writing at the moment.

boblmartens commented 8 years ago

Alright, I feel like I am getting closer:

In Formidable, in classes/models/FrmNotification.php lines 373 - 378 they have some conditional sends setup where if wp_mail doesn't return back with a good value, it will resend a simplified mail message (and outside of wp_mail, so that is why it is not encrypted).

Can I get to the error messages for wp_mail somewhere so I can find out what is going on? The mails are getting through, so it must be something else.

Here is the code that must be getting called:

$sent = wp_mail($recipient, $atts['subject'], $message, $header, $atts['attachments']); if ( ! $sent ) { $header = 'From: '. $atts['from'] ."\r\n"; $recipient = implode(',', (array) $recipient); $sent = mail($recipient, $atts['subject'], $message, $header); }

fabacab commented 8 years ago

This is exactly the problem; look at the last line of the wp_mail() method in WP PGP Encrypted Emails. It returns an intentional error so that the the original (unencrypted) call to wp_mail() will not send any mail.

Formidable notices this and then sends the email another way, exactly as I suspected (see my earlier comment), without using WordPress. This makes Formidable impossible to secure. You should stop using that plugin.

How typical that the capitalists who sell a proprietary plugin blame the code in free software for a problem that is actually in their own code.

boblmartens commented 8 years ago

You're talking line 812, which I had totally glossed over. Now the behavior all makes sense ... and now I'll need to see where I want to run with this.

Thank you for your patience and your time!

boblmartens commented 8 years ago

To close this out for the moment, eliminating their calls for re-sending on a "failed" send, everything works as I would expect.

So now I'll do some thinking about supporting a modified version of the plugin or looking around at other options. Thanks!