Add support for ES256, ES384, ES512

[dol]

The PHP core supports EC private keys for signing and verification. This change was introduced in January 2014. Currently this PHP versions support EC private keys: php-5.4.26 php-5.5.10 php-5.6.0

[odino]

hey @dol, thanks for noticing! Would you be able to submit a PR?

[dol]

Yes, I'm able. Please stay tuned.

[odino]

Thanks man :-) On Nov 19, 2014 12:36 PM, "Dominic" notifications@github.com wrote:

Yes, I'm able. Please stay tuned.

— Reply to this email directly or view it on GitHub https://github.com/namshi/jose/issues/15#issuecomment-63607412.

[dol]

Note to myself: OPENSSL_ALGO_SHA256 OPENSSL_ALGO_SHA384 OPENSSL_ALGO_SHA512 From https://github.com/php/php-src/blob/d0cb715373c3fbe9dc095378ec5ed8c71f799f67/ext/openssl/openssl.c#L1037

[dol]

@odino I currently have one issue implementing this. EC key is not tied one curve like RSA. For each ES256, ES384 and ES512 different curves are used. To support this feature in a secure way, the private key type and the curve needs to be checked. In PHP it's possible to check the key type, but not the used curve of the EC key. There are possibilities to extract the curve informations, but for this we need the PEM string instead of the private key resource. Your library requires an resource passed in. Could we change this behavior and allowing a string or file path. This also helps to replace the underlying private key handling. E.g. we can fallback to phpseclib if openssl is missing.

This is a BC break, but would bring some advantages. Take a look at https://github.com/gree/jose/ and how they work with private keys. It's IMHO the better way.

[odino]

@dol I think not to break BC you could check if the argument is a resource and use the old code, and the news one if its a string...deal? :)

[dol]

Deal. I'll implement it that way.

[odino]

:+1:

On Fri, Nov 21, 2014 at 3:32 AM, Dominic notifications@github.com wrote:

Deal. I'll implement it that way.

— Reply to this email directly or view it on GitHub https://github.com/namshi/jose/issues/15#issuecomment-63899938.

Nadalin Alessandro http://www.linkedin.com/in/alessandronadalin www.odino.org www.twitter.com/odino

[eroux]

I'm affraid the code doesn't work anymore with string publickeys... due to line 48 in src/Namshi/JOSE/Signer/PublicKey.php. I need to sign a lot of tokens with the same publickey, I don't really want to make file access every time...

Thanks!

[dol]

My bad. I assumed that openssl_verify() and openssl_sign() need always an resource. According to the documentation a string in the PEM format is allowed. I'll fix this issue.

[dol]

I'm not sure if this is an bug or a feature. The question is, if the JOSE library has to handle the keys in a string format. A string could contain a an invalid key format or needs a password to unlock the private key. IHMO this is not the responsibility of the library. For this reason it forces a openssl resource to be present. It needs some logic to load a public/private key from a string to perform the necessary checks if a key is suitable for this sign/verify algorithm.

The version 2.0 checks the key on every sign/verify. This might me an overhead. @eroux Your report come done to the problem, if it's a good solution to check the key to be suitable for this JOSE algorithm or not. Or is it better to leave out all the checks and hope the users will use the right key for the right algorithm. I personally leaning towards a strict check, even though is comes with a little performance overhead for all the checks. This check is quite small for RSA key, but for ECC it's might be significant due to the limited support for ECC key in the PHP core. I fixed this for future version of PHP.

If the library has to support string keys we could bypass all the checks like @eroux suggested or additional logic is added to load the keys into a resource and then perform the check.

It always comes down to security vs usability. I leave this dissension up to @odino.

Note: If string keys will be supported it needs some test cases to prevent of future breaks. At the moment only resources are tested. In addition the documentation should be expanded.

[eroux]

I'm not sure it makes sense, but why not creating something like a publicKey class instance from whatever the user provides, perform all checks the first time it's used, and then just using it raw if it's already been checked?

[dol]

You mean something like https://github.com/zendframework/zf2/tree/master/library/Zend/Crypt/PublicKey/Rsa

[dol]

This might be a good solution to the problem. It gives an additional abstraction on top of the openssl crypto library. In the future the openssl could be replaced with an different implementation.

[eroux]

Something like that yes (with the additional checks you do)

[odino]

Hey guys,

Im currently a bit far from a decent connection, will have a look at it in a couple days ok?

Cheers! On Dec 22, 2014 1:32 PM, "Dominic" notifications@github.com wrote:

My bad. I assumed that openssl_verify() and openssl_sign() need always an resource. According to the documentation http://php.net/manual/en/function.openssl-sign.php a string in the PEM format is allowed. I'll fix this issue.

— Reply to this email directly or view it on GitHub https://github.com/namshi/jose/issues/15#issuecomment-67822914.

[odino]

closing this as the original issue is implemented -- continuing the discussion here :)