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

Consider signing outgoing emails, not just encrypting them, to "prove" they came from the given website #1

Closed
meitar opened this issue Jan 28, 2016 · 44 comments

Comments

@meitar
Copy link
Owner

commented Jan 28, 2016

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).

@meitar meitar added the enhancement label Jan 28, 2016

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 20, 2016

For reference:

keygen example https://github.com/singpolyma/openpgp-php/blob/master/examples/keygen.php

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).

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 21, 2016

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.

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 21, 2016

I totally agree with you. When it is only used for this it should be ok.

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 23, 2016

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 Crypt_RSA() class that OpenPGP.php aliases from phpseclib.

Specifically, I am seeing a Fatal error: Class 'Crypt_RSA' not found in /srv/www/wordpress-default/wp-content/plugins/wp-pgp-encrypted-emails/class-wp-openpgp.php on line 120 when I try to follow the OpenPGP.php keygen example. The code that causes the error looks like this:

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:

  1. WordPress loads and initializes itself.
  2. WordPress tries to activate this plugin.
  3. Control passes to this plugin's activate() method.
  4. This plugin's activate() method requires its class-wp-openpgp.php file.

At the top of the class-wp-openpgp.php file, there exists this code:

// 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 if condition is false and so, the first thing we do, is that we load the composer-generated autoload.php file (in vendor/openpgp-php/vendor/autoload.php).

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 RSA class in openpgp_crypt_rsa.php like this, which is the very first line of code in that file:

use phpseclib\Crypt\RSA as Crypt_RSA;

Given this, why is Crypt_RSA not found when control returns to my plugin code?

I have tried adding various aliases with use at the top of my own class-wp-openpgp.php file, but nothing I've tried works yet. I have tried:

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

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 23, 2016

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).

http://stackoverflow.com/a/20562022/753676

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 use phpseclib\Crypt\RSA of the classes. http://phpseclib.sourceforge.net/2.0.html

@singpolyma has to use the full qualified name in the example

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 23, 2016

$rsa = new \phpseclib\Crypt\RSA();

seems to work.

This does not work for me. Changing the line $rsa = new Crypt_RSA(); to $rsa = new \phpseclib\Crypt\RSA(); simply gives me the same max execution time exceeded error as noted above (specifically 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).

The other example aliasing to Crypt_RSA() is one I mentioned trying in my comment, above, that also results in a max execution time exceeded error.

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.

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 23, 2016

same max execution time exceeded error

Did you try using set_time_limit(0) for testing?

echo ini_get('max_execution_time'); gives me 0 on my local machine here (well, this is infinite).

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 23, 2016

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?

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 23, 2016

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).

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 23, 2016

The key generation is probably the problem as this takes some time.

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 BigInteger.php? I conflated one error with the other; I will try with set_time_limit(0), thanks for clarifying this for me.

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 23, 2016

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.

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 BigInteger.php

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

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 23, 2016

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! :)

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 23, 2016

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    BCMath  Internal    BCMath  GMP
        w/o OpenSSL         w/ OpenSSL
...
PHP 5.2 7.944       3.995   1.210       0.177   0.014
PHP 5.3 5.758       3.293   0.791       0.246   0.017
PHP 5.4 3.159       5.415   0.519       0.293   0.019
PHP 5.5 3.719       4.459   0.485       0.227   0.020

Internal may be (a bit) faster than BC Math without OpenSSL according to the chart.

https://en.wikipedia.org/wiki/Public-key_cryptography#Computational_cost

The public key algorithms known thus far are relatively computationally costly compared with most symmetric key algorithms of apparently equivalent security.

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).

See also http://stackoverflow.com/questions/23765220/phpseclib-exceeded-max-execution-time#comment36585962_23766020

NaCl and Libsodium are the upcoming solutions which should replace OpenSSL (also in PHP) as this is much faster and stronger.

http://fi.laceous.com/post/33101029913/bcmath-vs-gmp-performance

You see, it is much about O (compelxity of algorithm / time), security and underlying code ;-)

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 23, 2016

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 gpg --verify outgoingemail.txt.asc my GnuPG client correctly identifies that the signature was made using the specific key. However, it always reports a bad signature. :(

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 sign-email branch (HEAD is now 2324e11 on that branch) for you to have a look if you're interested.

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.

@singpolyma

This comment has been minimized.

Copy link

commented Feb 23, 2016

Yeah, keygen for 4096 can be very slow. Let me know if you have issues with the clearsigning example

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

@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 c (certify) capability, but not the s (sign) capability. I checked this by importing the keys into a GUI and inspecting the key there. (Screenshot attached.)

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 gpg command line binary? I have tried numerous Google searches but I cannot find instructions on how to do that. :(

screen shot 2016-02-24 at feb 24 5 04 51 am

/cc @DanielRuf

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 24, 2016

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 <test@example.com>');
$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 unarmor?

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

Yes, that's taken straight from the example code. As far as I can tell, this is exactly what I am doing here:

  • Generating key with the same arguments using new OpenPGP_SecretKeyPacket on line 170
  • Then using this key to clearsign some data, in this case an email that WordPress creates, on line 55, which is called from WordPress via line 732 of the plugin file.

The output of this code looks like, as an example, this:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

A new comment on the post "hello world" is waiting for your approval
http://local.wordpress.dev/2016/02/19/hello-world/

Author: hello (IP: 192.168.50.1, 192.168.50.1)
Email: hello@world.com
URL:
Comment:
A clearsigned message?

Approve it: http://local.wordpress.dev/wp-admin/comment.php?action=approve&c=89
Trash it: http://local.wordpress.dev/wp-admin/comment.php?action=trash&c=89
Spam it: http://local.wordpress.dev/wp-admin/comment.php?action=spam&c=89
Currently 56 comments are waiting for approval. Please visit the moderation panel:
http://local.wordpress.dev/wp-admin/edit-comments.php?comment_status=moderated

-----BEGIN PGP SIGNATURE-----

wv8AAAEkBAABCAAY/wAAAAUCVs2V6v8AAAAJEKgqRS6Gb1VlAAAs9wf+LPf9KQJ9gh4/GEbQkP5
ioIzRgwsuth2Z0Iw370zhYC3jLgjvuOfsXO3eGLSmIYzhrfZbS0vUZuBStKmVCHRpRdvLKWJ/M1
Vhb+cg5MlYDjzSzugBv+QwR04QdwP4qTSxX99cyDed3LqLaDd5JWMguWJmiieYUX/PCyaG81leK
lTvE+g47eTLgrY0Tc62P/N2VkTWGo7eCFBaPmHYtS7rxATdqLPqE7wkIYT0wQ5YvQIGCohtp6jr
0loVxNU4iOYU6cFJdWwaS/ELoZP2ylJRWsv2A/IrcyMgyLn2hLD7zdTcEcpJNFLLjVlIEGWHKgn
sE0HDRgYhOyzanjl9BGJjxw==
=b+dJ
-----END PGP SIGNATURE-----

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 gpg --verify, the result is a bad signature:

$ 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 <wordpress@local.wordpress.dev>" [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?

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 24, 2016

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.

Ok, this should not be the case, I will test it here with my generated keys.

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

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:

$ gpg  --edit-key local.wordpress.dev
gpg (GnuPG/MacGPG2) 2.0.28; Copyright (C) 2015 Free Software Foundation, Inc.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Secret key is available.

pub  2048R/866F5565  created: 2016-02-24  expires: never       usage: C   
                     trust: unknown       validity: unknown
[ unknown] (1). WordPress <wordpress@local.wordpress.dev>

gpg> addkey
This key is not protected.
Please select what kind of key you want:
   (3) DSA (sign only)
   (4) RSA (sign only)
   (5) Elgamal (encrypt only)
   (6) RSA (encrypt only)
Your selection? 4
RSA keys may be between 1024 and 4096 bits long.
What keysize do you want? (2048) 
Requested keysize is 2048 bits   
Please specify how long the key should be valid.
         0 = key does not expire
      <n>  = key expires in n days
      <n>w = key expires in n weeks
      <n>m = key expires in n months
      <n>y = key expires in n years
Key is valid for? (0) 0
Key does not expire at all
Is this correct? (y/N) y
Really create? (y/N) y  
We need to generate a lot of random bytes. It is a good idea to perform
some other action (type on the keyboard, move the mouse, utilize the
disks) during the prime generation; this gives the random number
generator a better chance to gain enough entropy.
gpg: checking created signature failed: Bad signature
gpg: signing failed: Bad signature
gpg: make_keysig_packet failed: Bad signature
gpg: Key generation failed: Bad signature

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. :\

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 24, 2016

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 gpg --verify

gpg --verify clearsigned.txt
gpg: Signatur vom 02/24/16 14:27:54 Mitteleuropõische Zeit mittels RSA-Schlüssel
 ID 313273C8
gpg: Korrekte Signatur von "test123 (test) <test@test.de>" [uneingeschränkt]
@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

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:

$ php test.php | gpg2 --verify
gpg: Signature made Wed Feb 24 06:38:14 2016 MST using RSA key ID 866F5565
gpg: BAD signature from "WordPress <wordpress@local.wordpress.dev>" [unknown]

But if I remove the newline:

$ php test.php | gpg2 --verify
gpg: Signature made Wed Feb 24 06:38:37 2016 MST using RSA key ID 866F5565
gpg: Good signature from "WordPress <wordpress@local.wordpress.dev>" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 647E 127D 78B3 4ABE C0FA  C131 A82A 452E 866F 5565

@singpolyma Should I have to trim() the clearsigned data before it is signed…?

In my test.php, I am using a private key generated from the WordPress plugin code itself.

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 24, 2016

With a keypair generated by the plugin itself, however, there seem to be problems.

Currently testing this.

@singpolyma

This comment has been minimized.

Copy link

commented Feb 24, 2016

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.

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 24, 2016

@meitar 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 <test@example.com>');
$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;
    }
@DanielRuf

This comment has been minimized.

Copy link

commented Feb 24, 2016

I will take a break now, I can not directly see where the problem is, maybe I just oversee it =(

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

Just to make sure, you need the private key for signing, is it there?

Yes, it's in the $meta_keypair property. What you linked to is the administrator's public key, the one the website uses to encrypt messages sent to the admin email address.

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

Okay I am seeing more weirdness that I cannot explain. It seems I am getting different results with \n versus \r inside of signed strings. Whenever I use a newline anywhere in the string to sign, with this example code above, I always get a bad signature, but I can apparently use a carriage return in the middle of the string and get a good signature.

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 string

then the "data" that gets printed in the clearsign message seems strangely truncated:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

http://local.wordpress.dev/wp-admin/edit-comments.php?comment_status=moderatednel:
-----BEGIN PGP SIGNATURE-----

wv8AAAEkBAABCAAY/wAAAAUCVs23kv8AAAAJEKgqRS6Gb1VlAAAxlQf+MZWipAHexlK5igFK4TdsDEmTaxs7+ZE8V8GbtgKRCdTDhCag18uvJsooOrv5q38DZdmQZPrQ0aLtew5F0kP1bh1GvubckmErDVI8gWlNlrvbHiVdcFzuFPW1JO8LtuHatd/TlWP1AqNh6H13MaKdMHrzXf6Cu0Swn015dr9aqGgCA2LGqs6lX8uSkaSyJROIwmh9hbojVzweOsBrrxJnGjCC8TX1z94lNE9g5pmoi8/jImHvp48ycWgGBkQqyRKeYkeytJ48F7j1XSaLLZx0jJm9LM9b4PlU2Jq9AOvksnS34PE2tyWQR9sLoKMPuE/6jGiul9WQMwRtpPqc1zPaHg==
=6xx4
-----END PGP SIGNATURE-----

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?

@singpolyma

This comment has been minimized.

Copy link

commented Feb 24, 2016

Hmm. Does \r\n work also? My code is supposed to normalize newlines when the signature type/literal data type is set to text...

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 24, 2016

being to confirm that you are also unable to verify strings with newlines in them?

Yes, confirmed, Added \n to the end created a bad signature here too.

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 24, 2016

Does \r\n work also?

This works and creates a valid signature.

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

To confirm my experiments with newlines, forcing the data to be test in my WordPress plugin:

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 \n anywhere in the message breaks it.

@singpolyma It seems that using "test\r\n" produces a good signature, too. So does "test\r\ntest".

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?

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

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 OpenPGP_Crypt_RSA::sign() mutates the line endings before signing, but then the clearsigned message is printed as $packets[0]->data and this does not include the changed line endings…? (See code from this excerpt here.)

@singpolyma

This comment has been minimized.

Copy link

commented Feb 24, 2016

Verifier should normalise when verifying also, so more likely OpenPGP.php is not normalising but should

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

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.

@DanielRuf

This comment has been minimized.

Copy link

commented Feb 24, 2016

@meitar 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.

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

@DanielRuf I don't think I can simply preg_replace() line endings as described there to solve the problem because WordPress's emails always use \n which is, as we confirmed, what triggers the signing issue. :\ And if I just outright remove the line endings, then the issue described in the latter half of this comment reappears. I can't explain why that would happen, though…?

@singpolyma

This comment has been minimized.

Copy link

commented Feb 24, 2016

$signer->sign("test\n");

This is the issue right here. The correct call would be:

$signer->sign(new OpenPGP_LiteralDataPacket("test\n", array('format' => 'u')));

The 'format' => 'u' tells the library that this data packet is text and so normalization will be done. If you just pass a string that might sometimes work, but it is not the way the call should be used. If you check the clearsign example it does create a OpenPGP_LiteralDataPacket as required.

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

@singpolyma I tried changing my clearsigning method to using an OpenPGP_LiteralDataPacket like so:

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 $data is replaced with "test\n" but not when $data is used. Note the error_log()s in the method above; here is the log it generates, with the real email contents that WordPress generates.

[24-Feb-2016 15:29:20 UTC] A new comment on the post "hello world" is waiting for your approval
http://local.wordpress.dev/2016/02/19/hello-world/

Author: hello (IP: 192.168.50.1, 192.168.50.1)
Email: hello@world.com
URL: 
Comment: 
another literaldatapacket?

Approve it: http://local.wordpress.dev/wp-admin/comment.php?action=approve&c=104
Trash it: http://local.wordpress.dev/wp-admin/comment.php?action=trash&c=104
Spam it: http://local.wordpress.dev/wp-admin/comment.php?action=spam&c=104
Currently 71 comments are waiting for approval. Please visit the moderation panel:
http://local.wordpress.dev/wp-admin/edit-comments.php?comment_status=moderated

[24-Feb-2016 15:29:20 UTC] -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

A new comment on the post "hello world" is waiting for your approval
http://local.wordpress.dev/2016/02/19/hello-world/

Author: hello (IP: 192.168.50.1, 192.168.50.1)
Email: hello@world.com
URL: 
Comment: 
another literaldatapacket?

Approve it: http://local.wordpress.dev/wp-admin/comment.php?action=approve&c=104
Trash it: http://local.wordpress.dev/wp-admin/comment.php?action=trash&c=104
Spam it: http://local.wordpress.dev/wp-admin/comment.php?action=spam&c=104
Currently 71 comments are waiting for approval. Please visit the moderation panel:
http://local.wordpress.dev/wp-admin/edit-comments.php?comment_status=moderated

-----BEGIN PGP SIGNATURE-----

wv8AAAEkBAEBCAAY/wAAAAUCVs3MUP8AAAAJEKgqRS6Gb1VlAAAryQf+K8lrxhYmMIscidxls/i
YoDk2LwoXbgAoPjeMV16G+PLm/EC0jpcLslFLO8WA8jem18CZ9xcFEmQookivSPbLtUpwG1CM9n
detgkQ7mXkBU/LpzgY6SVCwx6M/cPVusEFMlEo+Ls+vZIuEt70ce+9b5l0Npd4R4Zm/fDkizLuB
GOJapXVBrxiAUYtmjIy/NsSeT7YvoS3jqLiS7F0eMtqeiL3HlxvvnkRmxAyYfV3NSzExiuGfVtO
PwouOrBOlC0hr6Rxr8F9XCIhv9gB0MkXkSzyE+2gitlj2M3ddN8xXZ88u/b/a+pQBsdyXj9TPy/
vVkEAB603jlo1+qiaFMDMRA==
=5LYS
-----END PGP SIGNATURE-----

This PGP signature still does not produce a good signature, even though when I used the same code to sign "test\n" it did produce a good signature.

Could there be a problem with the number of newlines?

@singpolyma

This comment has been minimized.

Copy link

commented Feb 24, 2016

The problem seems to be trailing whitespace on lines. I am working on it

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

Thank you for looking into it!

@singpolyma

This comment has been minimized.

Copy link

commented Feb 24, 2016

Also, any trailing whitespace -- spaces (0x20) and tabs (0x09) -- at the end of any line is removed when the cleartext signature is generated.

So says https://tools.ietf.org/html/rfc4880#section-7

I will update the example

@singpolyma

This comment has been minimized.

Copy link

commented Feb 24, 2016

I have pushed a small change to the library and an accompanying change to the clearsign example

@meitar

This comment has been minimized.

Copy link
Owner Author

commented Feb 24, 2016

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.