fix: RSA from JWK sometimes returns empty Instance#628
fix: RSA from JWK sometimes returns empty Instance#628bshaffer merged 2 commits intofirebase:mainfrom
Conversation
firebase#589 addresses specification difference between JWK (or JWA) and ASN1 integers.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the createPemFromModulusAndExponent method in src/JWK.php to ensure correct ASN1 encoding by prepending a null byte to the modulus and exponent if their first byte is 128 or greater. Feedback was provided regarding styling inconsistencies, such as the use of tabs instead of spaces and missing spaces after control keywords, to maintain consistency with the rest of the codebase.
bshaffer
left a comment
bshaffer
left a comment
There was a problem hiding this comment.
Thank you for finding this tricky issue! This looks good, but I can't seem to write a test to key cover this. I have tried using the key supplied in #589, but no error is thrown. Do you know how we can test that this behavior fixes the issue?
Thanks again!
|
@bshaffer sure, using the JWK from the issue, currently we will end up with the following pem. #589 describes the resulting key object to be "empty", I did not really check the details, but after applying the fix, I was able to verify the signature as I intended to do. -----BEGIN PUBLIC KEY----- While correctly encoded, it should be this -----BEGIN PUBLIC KEY----- |
2 participants
#589 addresses specification difference between JWK (or JWA) and ASN1 integers.
See https://datatracker.ietf.org/doc/html/rfc7518#section-6.3.1.1 for the n parameter these kinds of null bytes are stripped and need to be added again.