EC256 signature is incompatible with RFC-7518

[romkavt]

The library creates ES256 JWS signature as DER encoded ASN1 sequence, but it have to be concatenated R and S ECPoints.

See https://tools.ietf.org/html/rfc7518#section-3.4 The problem code is https://github.com/namshi/jose/blob/master/src/Namshi/JOSE/Signer/OpenSSL/PublicKey.php#L25

The right example is https://github.com/lcobucci/jwt/blob/master/src/Signer/Ecdsa.php#L82

With best wishes, Roman

[paceto256]

Confirmed ES256, ES384 .. not signed as described in RFC-7518.

I just realized the same issue with another PHP lib ...

test case:

1. create private & public key with ES384.
2. use the private key and this lib to sign the token use the signed token generated by this lib and try to validate it using the public key and this tool: http://kjur.github.io/jsjws/tool_jwt.html

[odino]

Hey guys,

would you mind sending a PR for this to get fixed? We mostly use RSA so I dont have a good hang of how ECDSA works :)

[ishitatsuyuki]

This is a reference in Java which can be stacked on top on current OpenSSl impl: https://github.com/jwtk/jjwt/blob/master/src/main/java/io/jsonwebtoken/impl/crypto/EllipticCurveProvider.java

[ishitatsuyuki]

This is my impl for ES384:

$sig = base64_decode(strtr($sigB64, '-_', '+/'), true);
$rawLen = 48; // ES384
for($i = $rawLen; $i > 0 and $sig[$rawLen - $i] == chr(0); $i--) {}
$j = $i + (ord($sig[$rawLen - $i]) >= 128 ? 1 : 0);
for($k = $rawLen; $k > 0 and $sig[2 * $rawLen - $k] == chr(0); $k--) {}
$l = $k + (ord($sig[2 * $rawLen - $k]) >= 128 ? 1 : 0);
$len = 2 + $j + 2 + $l;
$derSig = chr(48);
if($len > 255){
    throw new RuntimeException("Invalid signature format");
}elseif($len >= 128){
    $derSig .= chr(81);
}
$derSig .= chr($len) . chr(2) . chr($j);
$derSig .= str_repeat(chr(0), $j - $i) . substr($sig, $rawLen - $i, $i);
$derSig .= chr(2) . chr($l);
$derSig .= str_repeat(chr(0), $l - $k) . substr($sig, 2 * $rawLen - $k, $k);

$verified = openssl_verify($headB64 . "." . $payloadB64, $derSig, "-----BEGIN PUBLIC KEY-----\n" . wordwrap($key, 64, "\n", true) . "\n-----END PUBLIC KEY-----\n", OPENSSL_ALGO_SHA384) === 1;

[odino]

hey @ishitatsuyuki would you mind sending a PR? We mostly use RSA so I dont have a good hang of how the ES family works :)

[ishitatsuyuki]

I have only implemented verify only and not sure where to insert these code by your design. This is just converting between sign format, thus you don't have to understand how it works. Please take a look at the reference Java code.

[tillz]

I think I've managed to build a more clear, universal and bidirectional (sign & verify) solution, however I'd like to keep it private until it's tested against at least one known-good implementation. As the only other library I'm firm with (jsrsasign) seems to have the same problem (expects ASN1), could someone contact me to test my implementation? (That's why I want to keep it private, I think it's a very bad idea to just publish another non-standard, non-working Implementation which only brings massive confusion.)

As soon as it's tested, I'll send a PR - that's why I wrote it.

[Spomky]

Have a look on my PHP library to deal with that bug.

I hope it will help you.

[tillz]

@Spomky That's the first thing I've considered when facing this bug, however I'd like to avoid ext-gmp, which is required by fgrosse/phpasn1, while phpseclib used in this impl is working w/o an additional PHP Extension (In theory, like shown above, the whole ASN1-parser could be avoided, but this make the code a bit messy and therefore vulnerable)

[Spomky]

You don't need GMP. Look at https://github.com/Spomky-Labs/jose/blob/master/src/Algorithm/Signature/ECDSA.php#L52

The method that uses GMP is just a fallback in case openssl does not support EC signatures

[cirpo]

@Spomky is it possible to open a PR against this repo?

[tillz]

@Spomky Isn't Integer, Sequence part of phpasn1, which represents the integers with gmp internally? However, I'd need to force my 'dependency manager' to ignore dependencies, which I'd like to avoid.

[Spomky]

You are right fgrosse/phpasn1 requires GMP. If you know an ASN.1 parser that does not requires GMP, then I will try to create a PR.

[tillz]

phpseclib ;-)