firebase / php-jwt

PHP package for JWT
BSD 3-Clause "New" or "Revised" License
9.3k stars 1.26k forks source link

JWT::createPemFromModulusAndExponent incorrect return value #532

Closed libertyit1 closed 1 month ago

libertyit1 commented 10 months ago

I retrieved JWKs for Google public keys from https://www.googleapis.com/oauth2/v3/certs including this one:

{
    "alg": "RS256",
    "kty": "RSA",
    "n": "hsYvCPtkUV7SIxwkOkJsJfhwV_CMdXU5i0UmY2QEs-Pa7v0-0y-s4EjEDtsQ8Yow6hc670JhkGBcMzhU4DtrqNGROXebyOse5FX0m0UvWo1qXqNTf28uBKB990mY42Icr8sGjtOw8ajyT9kufbmXi3eZKagKpG0TDGK90oBEfoGzCxoFT87F95liNth_GoyU5S8-G3OqIqLlQCwxkI5s-g2qvg_aooALfh1rhvx2wt4EJVMSrdnxtPQSPAtZBiw5SwCnVglc6OnalVNvAB2JArbqC9GAzzz9pApAk28SYg5a4hPiPyqwRv-4X1CXEK8bO5VesIeRX0oDf7UoM-pVAw",
    "use": "sig",
    "e": "AQAB",
    "kid": "838c06c62046c2d948affe137dd5310129f4d5d1"
}

JWT::createPemFromModulusAndExponent(n, e) with these values returns this PEM format:

-----BEGIN PUBLIC KEY-----
MIIBITANBgkqhkiG9w0BAQEFAAOCAQ4AMIIBCQKCAQCGxi8I+2RRXtIjHCQ6Qmwl
+HBX8Ix1dTmLRSZjZASz49ru/T7TL6zgSMQO2xDxijDqFzrvQmGQYFwzOFTgO2uo
0ZE5d5vI6x7kVfSbRS9ajWpeo1N/by4EoH33SZjjYhyvywaO07DxqPJP2S59uZeL
d5kpqAqkbRMMYr3SgER+gbMLGgVPzsX3mWI22H8ajJTlLz4bc6oiouVALDGQjmz6
Daq+D9qigAt+HWuG/HbC3gQlUxKt2fG09BI8C1kGLDlLAKdWCVzo6dqVU28AHYkC
tuoL0YDPPP2kCkCTbxJiDlriE+I/KrBG/7hfUJcQrxs7lV6wh5FfSgN/tSgz6lUD
AgMBAAE=
-----END PUBLIC KEY-----

JWTs from Google fail to verify using this key.

phpseclib3 and https://8gwifi.org/jwkconvertfunctions.jsp both return this PEM format:

-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAhsYvCPtkUV7SIxwkOkJs
JfhwV/CMdXU5i0UmY2QEs+Pa7v0+0y+s4EjEDtsQ8Yow6hc670JhkGBcMzhU4Dtr
qNGROXebyOse5FX0m0UvWo1qXqNTf28uBKB990mY42Icr8sGjtOw8ajyT9kufbmX
i3eZKagKpG0TDGK90oBEfoGzCxoFT87F95liNth/GoyU5S8+G3OqIqLlQCwxkI5s
+g2qvg/aooALfh1rhvx2wt4EJVMSrdnxtPQSPAtZBiw5SwCnVglc6OnalVNvAB2J
ArbqC9GAzzz9pApAk28SYg5a4hPiPyqwRv+4X1CXEK8bO5VesIeRX0oDf7UoM+pV
AwIDAQAB
-----END PUBLIC KEY-----

and this key verifies Google JWTs correctly.

Wedrix commented 9 months ago

@libertyit1 did you figure out a fix for this? I seem to be facing the same/similar issue.

Could this be related to https://github.com/php/php-src/issues/11054

libertyit1 commented 9 months ago

@libertyit1 did you figure out a fix for this? I seem to be facing the same/similar issue.

Not in Firebase; I ended up importing phpseclib3 to convert the JWK e & n params into a PKCS8 key and used that in Firebase JWT::decode.

bshaffer commented 1 month ago

Hello and thank you for raising this issue.

In the official google/apiclient library, we do something similar (convert the certs to PKCS8 using phpseclib3 before passing them to JWT::decode). See here for an example.

However, I refactored this to use the current oauth2 certs for google at the URL you've provided, and this is working as expected without PHPseclib (see https://github.com/googleapis/google-api-php-client/pull/2596, where I refactor our Verify.php class to use CachedKeySet instead of phpseclib3)

So, TLDR, you should be able to use this library to verify Google keys without manually coverting to PKCS8 like this:

use Firebase\JWT\CachedKeySet;
use Firebase\JWT\JWT;
use Google\Auth\Cache\MemoryCacheItemPool;
use GuzzleHttp\Client;
use GuzzleHttp\Psr7\HttpFactory;

$keySet = new CachedKeySet(
    'https://www.googleapis.com/oauth2/v3/certs',
    new Client(),
    new HttpFactory(),
    new MemoryCacheItemPool(); // or some other PSR-6 cache
);

$payload = JWT::decode($idToken, $keySet);
libertyit1 commented 1 month ago

As a matter of interest, what was the bug in JWT::createPemFromModulusAndExponent? I don't see any changes to the code.

bshaffer commented 1 month ago

@libertyit1 I never confirmed there was an issue in JWT. The previous workaround (using phpseclib3) we had in the google client library was implemented well before the JWK logic was added to firebase/php-jwt.

So, maybe the bug hasn't been fixed, and the keys just got rotated so that it's no longer an issue? I am not really sure. Either way, I can't reproduce it.

bshaffer commented 1 month ago

@libertyit1 after some testing, using the values for module, exponent, and public key you provided, I was able to verify that this is parsing the keys as expected. I am not sure what changed to make this the case.

See https://github.com/firebase/php-jwt/pull/565