Consider signing outgoing emails, not just encrypting them, to "prove" they came from the given website #1
Comments
|
For reference: signing example https://github.com/singpolyma/openpgp-php/blob/master/examples/sign.php Signing requires the secret / private key. If the server is protected enough (hardening, patching, WAF and so on) and no one can access the private key directly (just the PHP user) and you can trust the server (no brwach occured) this should be fine (I see no real alternative then storing this unique server key directly on the server somewhere over the root or in a protected directory). |
|
For a WordPress plugin the most obvious place to store the key is in the database itself, and it should be understood that this key is not intended for use by any other software specifically because storing it anywhere within reach of WordPress is by definition a security risk. But I do think it makes sense to let the plugin have (or generate) a keypair for the explicit purpose of signing outbound messages. Thank you for the example references, I will play around with those some and think about how to carefully add this feature in a way that teaches its users good habits. Your input is appreciated. |
|
I totally agree with you. When it is only used for this it should be ok. |
|
I've begun looking into this feature, and the first thing I need to do is auto-generate a keypair for signing messages. But I am having trouble doing that. I believe my issues are because of my ignorance about PHP autoloading. For some reason, my plugin cannot find the Specifically, I am seeing a public static function generateKeypair ($identity, $bits = 4096) {
if (2048 > $bits) {
$error_msg = 'RSA keys with less than 2048 bits are unacceptable.';
throw new UnexpectedValueException($error_msg);
}
$keypair = array();
// FYI, I'm (mostly) following the example at
// https://github.com/singpolyma/openpgp-php/blob/master/examples/keygen.php
// but would LOOOOOVE to have someone more knowledgeable than
// I am about this stuff double-check me here. Patches welcome!
$rsa = new Crypt_RSA(); // This is the line that causes the error.This code is being called from my WordPress plugin activation hook: public static function activate () {
self::checkPrereqs();
if (!get_option(self::$meta_keypair)) {
require_once plugin_dir_path(__FILE__).'class-wp-openpgp.php';So the code flow as I understand it is:
At the top of the // Load dependencies.
if (!class_exists('OpenPGP')) {
require_once plugin_dir_path(__FILE__).'vendor/openpgp-php/vendor/autoload.php';
require_once plugin_dir_path(__FILE__).'vendor/openpgp-php/openpgp.php';
require_once plugin_dir_path(__FILE__).'vendor/openpgp-php/openpgp_crypt_rsa.php';
require_once plugin_dir_path(__FILE__).'vendor/openpgp-php/openpgp_crypt_symmetric.php';
}Since we have not yet loaded OpenPGP.php, the This is where I begin to get lost. I was under the impression that the autoloader loads OpenPGP.php's dependencies without a problem. And indeed, it seems to work; I've had no problems until now, when I need to call one of phpseclib's methods directly myself (as per the keygen example). OpenPGP.php, for its part, loads phpseclib's use phpseclib\Crypt\RSA as Crypt_RSA;Given this, why is I have tried adding various aliases with use phpseclib\Crypt\RSA as Crypt_RSA; // causes "maximum execution time exceeded" error
use \phpseclib\Crypt\RSA as Crypt_RSA; // causes "maximum execution time exceeded" error
use phpseclib\Crypt\RSA;
// With the above, I change my code to call `RSA()` instead of `Crypt_RSA()` but then I get: Fatal error: Maximum execution time of 30 seconds exceeded in /srv/www/wordpress-default/wp-content/plugins/wp-pgp-encrypted-emails/vendor/openpgp-php/vendor/phpseclib/phpseclib/phpseclib/Math/BigInteger.php on line 1691
use \phpseclib\Crypt\RSA; // Same as above.What do I misunderstand or not understand about Composer or autoloaders or PHP namespaces that I need to understand to know why this is failing? Thanks in advance. :\ The code cited above is available in cfafaaa for you to look at. /cc @DanielRuf |
|
You did nothing wrong. The example code uses the old phpseclib implementation from the 1.0 branch of phpseclib. $rsa = new \phpseclib\Crypt\RSA();seems to work. And also use \phpseclib\Crypt\RSA as Crypt_RSA;
$rsa = new Crypt_RSA();The problem is that the scope of the namespace declaration, it is just used in the file but not available for the example file (parent). This is because we have no classmap definition for phpseclib https://github.com/phpseclib/phpseclib/blob/master/composer.json but for openpgp-php Here is an example directly from phpseclib where you can see, that we have to use the full qualified name eg with @singpolyma has to use the full qualified name in the example |
This does not work for me. Changing the line The other example aliasing to So…what else could I be doing different that makes the code in cfafaaa work for you but not for me? In the mean time, I will read through the other links you provided to see if I can understand better. |
Did you try using set_time_limit(0) for testing?
|
No, but, why would it take more than 30 seconds to load the namespace? Doesn't namespace aliasing happen at compile time…? Are these not two completely different time limits? |
|
The key generation is probably the problem as this takes some time. You can test this when generating keys locally. When you generate bigger keys, it takes very long. My local Bitnami WAMP Stack has a max execution time limit of 120 (seconds). |
Oh, then the execution time error in the BigInteger.php is not an error to load the class, but rather a timeout being hit while executing code inside |
|
Unfortunately I don't know how long it takes in the plugin and if WordPress itself affects this (adding additional load). But here the script runs in about 2 - 3 seconds under Bitnami WAMP 5.6.
Theoretically yes as BigInteger generation takes much time, especially under older PHP versions which do not use BCMath or GMP. BigInteger seems to have 3 modes. GMP, BCMath and some byte shifting method (slowest) as fallback. Here is a benchmark, OpenSSL should be available, otherwise it will be much slower: http://phpseclib.sourceforge.net/math/intro.html |
|
Hmm, this indicates that even the slowest situation that concerns us (Internal on PHP 5.2, as this is the oldest version that WordPress supports) only takes approximately 7.944 seconds. I would be surprised if even under these conditions WordPress adds more than 22 seconds of execution time when activating a plugin. Looks like I will have to do a bit more debugging. Thank you again for the additional information! These are exactly the kind of pointers I need to get better! :) |
|
My setup here has BC Math and OpenSSL. Not the fastest but better than without OpenSSL. If it takes about 3 seconds (maximum) for me here, then on systems without openSSL it would be 20 times slower under PHP 5.4 and 5.5. according to the benchmark (you have to use the ratio, if it takes 3 seconds here, other systems may take about 60 seconds if they really need 20 times longer): Internal may be (a bit) faster than BC Math without OpenSSL according to the chart.
For a good reason (security), OpenSSL as extension (C-code) is a wrapper for symmetric key algorithms and typically faster when it comes to generating big numbers than BC Math or GMP without OpenSSL (openssl_public_encrypt and so on). NaCl and Libsodium are the upcoming solutions which should replace OpenSSL (also in PHP) as this is much faster and stronger. You see, it is much about O (compelxity of algorithm / time), security and underlying code ;-) |
|
Okay, so some quick timing tests make me think that defaulting to a 2048 bit key instead of a 4096 bit one is safer if this is going to be in a (relatively low-trust) WordPress distribution, because it's less likely to cause timing trouble. But just in case, I changed things up so instead of generating a key on plugin activation, it happens with a button press at some time after. The key generation itself seems to work okay now, many thanks for your help on that. The new problem is that, somehow, I'm unable to verify a clearsigned PGP signature made using the key from code. I have the plugin clearsigning outgoing emails now, by first signing and then (optionally) encrypting the email contents. This seems to work okay, and when I try to I'm pretty tired now so will have to take a break and look at this again tomorrow, but I've pushed the newest code to the I will try going over the clearsigning example that @singpolyma made to see if I messed anything up in my own clearsigning code tomorrow. Thanks again for all your help so far. |
|
Yeah, keygen for 4096 can be very slow. Let me know if you have issues with the clearsigning example |
|
@singpolyma Thanks, and, unfortunately, yes, I'm still having trouble getting a good signature. I must be doing something wrong but it's not clear to me what that might be. After doing a little more diging my guess is that the generated keypair only contains the Have I followed your keygen example (discussed in this thread, above) incorrectly? If so, how do I create a keypair that also has the signing capability? And, as an aside, how can I edit the capabilities of a keypair using the
/cc @DanielRuf |
|
For me this works $rsa = new \phpseclib\Crypt\RSA();
$k = $rsa->createKey(512);
$rsa->loadKey($k['privatekey']);
$nkey = new OpenPGP_SecretKeyPacket(array(
'n' => $rsa->modulus->toBytes(),
'e' => $rsa->publicExponent->toBytes(),
'd' => $rsa->exponent->toBytes(),
'p' => $rsa->primes[1]->toBytes(),
'q' => $rsa->primes[2]->toBytes(),
'u' => $rsa->coefficients[2]->toBytes()
));
$uid = new OpenPGP_UserIDPacket('Test <[email protected]>');
$wkey = new OpenPGP_Crypt_RSA($nkey);
$m = $wkey->sign_key_userid(array($nkey, $uid));
$wkey = $m[0];
/* Create a new literal data packet */
$data = new OpenPGP_LiteralDataPacket('This is text.', array('format' => 'u', 'filename' => 'stuff.txt'));
/* Create a signer from the key */
$sign = new OpenPGP_Crypt_RSA($wkey);
/* The message is the signed data packet */
$m = $sign->sign($data);
/* Generate clearsigned data */
$packets = $m->signatures()[0];
echo "-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA256\n\n";
echo preg_replace("/^-/", "- -", $packets[0]->data)."\n";
echo OpenPGP::enarmor($packets[1][0]->to_bytes(), "PGP SIGNATURE");But another question @singpolyma. How can we use this with an ASCII armored PGP key using |
|
Yes, that's taken straight from the example code. As far as I can tell, this is exactly what I am doing here:
The output of this code looks like, as an example, this: The format of this clearsigned file seems to be correct. When I save this text to a file on my local disk, and then I feed it to $ gpg2 --verify /tmp/clearsigned.asc
gpg: Signature made Wed Feb 24 04:37:14 2016 MST using RSA key ID 866F5565
gpg: BAD signature from "WordPress <[email protected]>" [unknown]This bad signature result happens even if I have both the secret and public keys in my keyring, although in practice I should be able to verify the signature with just the public key. So…when you say it "works" for you, can you confirm that you can verify a good signature and not just create a clearsigned message? |
Ok, this should not be the case, I will test it here with my generated keys. |
|
If it helps any, here is what happens when I attempt to add a new subkey to the public/private keypair generated by my code (as I described earlier). Note the "bad signature" error at the end: So it appears that something is wrong with the signature but, again, I am at the limit of my knowledge of PGP internals and would appreciate pointers to more relevant information or help understanding what's wrong if you can spare the time. :\ |
|
With a keypair generated in GPA it works. This code says it is a valid signature for me (in GPA), the keypair is imported there: include 'vendor/autoload.php';
require_once dirname(__FILE__).'/lib/openpgp.php';
require_once dirname(__FILE__).'/lib/openpgp_crypt_rsa.php';
/* Parse secret key from STDIN, the key must not be password protected */
//$rsa = new Crypt_RSA();
$key=getKey("
-----BEGIN PGP PRIVATE KEY BLOCK-----
*snip*
-----END PGP PRIVATE KEY BLOCK-----
");
$signer = new OpenPGP_Crypt_RSA($key[0]);
$m = $signer->sign("test");
$packets = $m->signatures()[0];
$clearsign = "-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA256\n\n";
$clearsign .= preg_replace("/^-/", "- -", $packets[0]->data)."\n";
print $clearsign.OpenPGP::enarmor($packets[1][0]->to_bytes(), 'PGP SIGNATURE');
function getKey ($key, $ascii = true) {
if ($ascii) {
preg_match('/-----BEGIN ([A-Za-z ]+)-----/', $key, $matches);
$marker = (empty($matches[1])) ? 'MESSAGE' : $matches[1];
$key = OpenPGP::unarmor($key, $marker);
}
$openpgp_msg = OpenPGP_Message::parse($key);
return (is_null($openpgp_msg)) ? false : $openpgp_msg;
}And |
|
With a keypair generated by the plugin itself, however, there seem to be problems. But, and this is weird, I can't seem to verify signatures with a trailing newline in the data. So, if I change your above code to this (add a newline at the end of the text to sign): $m = $signer->sign("test\n");I can't get a good signature: But if I remove the newline: @singpolyma Should I have to In my |
Currently testing this. |
|
Should not have to trim, I don't think, but ensure you are including all whitespace in the clear of the message itself. Maybe missing a newline there? Not sure. |
|
Works here also with a generated key. Used keygen code: require 'vendor/autoload.php';
require_once dirname(__FILE__).'/lib/openpgp.php';
require_once dirname(__FILE__).'/lib/openpgp_crypt_rsa.php';
$rsa = new \phpseclib\Crypt\RSA();
$k = $rsa->createKey(512);
$rsa->loadKey($k['privatekey']);
$nkey = new OpenPGP_SecretKeyPacket(array(
'n' => $rsa->modulus->toBytes(),
'e' => $rsa->publicExponent->toBytes(),
'd' => $rsa->exponent->toBytes(),
'p' => $rsa->primes[1]->toBytes(),
'q' => $rsa->primes[2]->toBytes(),
'u' => $rsa->coefficients[2]->toBytes()
));
$uid = new OpenPGP_UserIDPacket('Test <[email protected]>');
$wkey = new OpenPGP_Crypt_RSA($nkey);
$m = $wkey->sign_key_userid(array($nkey, $uid));
// Serialize private key
//print $m->to_bytes();
// Serialize public key message
$pubm = clone($m);
$pubm[0] = new OpenPGP_PublicKeyPacket($pubm[0]);
$public_bytes = $pubm->to_bytes();
var_dump(OpenPGP::enarmor($m->to_bytes(), "PGP PRIVATE KEY BLOCK"));
var_dump(OpenPGP::enarmor($public_bytes, "PGP PUBLIC KEY BLOCK"));Used clearsigning code: include 'vendor/autoload.php';
require_once dirname(__FILE__).'/lib/openpgp.php';
require_once dirname(__FILE__).'/lib/openpgp_crypt_rsa.php';
/* Parse secret key from STDIN, the key must not be password protected */
//$rsa = new Crypt_RSA();
$key=getKey("
-----BEGIN PGP PRIVATE KEY BLOCK-----
*snip*
-----END PGP PRIVATE KEY BLOCK-----
");
$signer = new OpenPGP_Crypt_RSA($key[0]);
$m = $signer->sign("test");
$packets = $m->signatures()[0];
$clearsign = "-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA256\n\n";
$clearsign .= preg_replace("/^-/", "- -", $packets[0]->data)."\n";
print $clearsign.OpenPGP::enarmor($packets[1][0]->to_bytes(), 'PGP SIGNATURE');
function getKey ($key, $ascii = true) {
if ($ascii) {
preg_match('/-----BEGIN ([A-Za-z ]+)-----/', $key, $matches);
$marker = (empty($matches[1])) ? 'MESSAGE' : $matches[1];
$key = OpenPGP::unarmor($key, $marker);
}
$openpgp_msg = OpenPGP_Message::parse($key);
return (is_null($openpgp_msg)) ? false : $openpgp_msg;
} |
|
I will take a break now, I can not directly see where the problem is, maybe I just oversee it =( |
Yes, it's in the |
|
Okay I am seeing more weirdness that I cannot explain. It seems I am getting different results with In other words, when I use $signer->sign("test\n");I get a bad signature, but when I do: $signer->sign("te\rst");I get a good signature, even though $signer->sign("test\r");also gives me a bad signature. Can you confirm this is the case for you, too? Obviously, WordPress's emails will always contain newlines in them. Even weirder, though, when I change the clearsigning helper method in my code to do this: $signer->sign(str_replace("\n", '', $data)); // remove all newlines from the signed stringthen the "data" that gets printed in the clearsign message seems strangely truncated: Note that only a link, and not even a real one, becomes the message. My expectation was that the full email text would be printed, but all on one line rather than several. I cannot explain why that's not happening and only a link is printed? But more important for now is to confirm that you are also unable to verify strings with newlines in them? |
|
Hmm. Does \r\n work also? My code is supposed to normalize newlines when the signature type/literal data type is set to text... |
Yes, confirmed, Added \n to the end created a bad signature here too. |
This works and creates a valid signature. |
|
To confirm my experiments with newlines, forcing the data to be public static function clearsign ($data, $signing_key) {
$signer = new OpenPGP_Crypt_RSA($signing_key[0]);
$m = $signer->sign("test");
$packets = $m->signatures()[0];
$clearsign = "-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA256\n\n";
$clearsign .= preg_replace("/^-/", "- -", $packets[0]->data)."\n";
return $clearsign.apply_filters('openpgp_enarmor', $packets[1][0]->to_bytes(), 'PGP SIGNATURE');
}does produce a good signature. But again, using @singpolyma It seems that using So maybe the problem is that OpenPGP.php is changing the newlines in the data it is signing, but then the WordPress email output does not use the same line endings and, as a result, the signature was made on data that is not output in the clearsigned message? |
In other words, could it be that |
|
Verifier should normalise when verifying also, so more likely OpenPGP.php is not normalising but should |
Okay, so I'm starting to suspect the failure to get a good signature is not with something I'm doing wrong from the plugin code but it has something to do with the line endings WordPress uses versus what OpenPGP.php tries to sign/check/verify/normalize? I think I will follow @DanielRuf's example and take a break from this as well, as I can also not find the problem right now. :( Please let me know if there is more information I can get for you that would be helpful to you in looking into the line endings issue. |
|
For normalization, this may be useful maybe: https://www.darklaunch.com/2009/05/06/php-normalize-newlines-line-endings-crlf-cr-lf-unix-windows-mac (preg_replace should be sufficient here) if this is the problem. |
|
@DanielRuf I don't think I can simply |
|
This is the issue right here. The correct call would be:
The |
|
@singpolyma I tried changing my clearsigning method to using an public static function clearsign ($data, $signing_key) {
$signer = new OpenPGP_Crypt_RSA($signing_key[0]);
error_log($data);
$m = $signer->sign(new OpenPGP_LiteralDataPacket(
$data,
array('format' => 'u', 'filename' => 'message.txt')
));
$packets = $m->signatures()[0];
$clearsign = "-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA256\n\n";
$clearsign .= preg_replace("/^-/", "- -", $packets[0]->data)."\n";
$result = $clearsign.apply_filters('openpgp_enarmor', $packets[1][0]->to_bytes(), 'PGP SIGNATURE');
error_log($result);
return $result;
}and it resolves the problem when This PGP signature still does not produce a good signature, even though when I used the same code to sign Could there be a problem with the number of newlines? |
|
The problem seems to be trailing whitespace on lines. I am working on it |
|
Thank you for looking into it! |
I will update the example |
|
I have pushed a small change to the library and an accompanying change to the clearsign example |
|
@singpolyma Thank you for the update! Using your latest change (OpenPGP.php at cefaef242df6bdb85e19bcf0c23e4fff697737c9) and a corresponding update to my clearsigning method shown below, I am able to get a good signature with the generated key: public static function clearsign ($data, $signing_key) {
$packet = new OpenPGP_LiteralDataPacket($data, array(
'format' => 'u', 'filename' => 'message.txt'
));
$packet->normalize(true);
$signer = new OpenPGP_Crypt_RSA($signing_key[0]);
$m = $signer->sign($packet);
$packets = $m->signatures()[0];
$clearsign = "-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA256\n\n";
$clearsign .= preg_replace("/^-/", "- -", $packets[0]->data)."\n";
return $clearsign.apply_filters('openpgp_enarmor', $packets[1][0]->to_bytes(), 'PGP SIGNATURE');
}Again, thank you for your attention to this. |
Fixing filter typo in file wp-pgp-encrypted-email.php
It would be nice if an admin can assign (or auto-generate) a keypair to the plugin (or site) itself, thus allowing the site to sign its outgoing messages in addition to encrypting them to a given user's public key.
This will require switching libraries to one that supports signing (perhaps singpolyma/openpgp-php).
The text was updated successfully, but these errors were encountered: