fix: RSA from JWK sometimes returns empty Instance#628
fix: RSA from JWK sometimes returns empty Instance#628NReib wants to merge 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. |
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.
src/JWK.php
Outdated
|
|
||
| // Correct encoding for ASN1, as ints are represented as unsigned in jwk | ||
| // but signed in ASN1. Prepending null byte makes it unsigned | ||
| if(\strlen($mod) > 0 && \ord($mod[0]) >= 128){ | ||
| $mod = \chr(0) . $mod; | ||
| } | ||
| if(\strlen($exp) > 0 && \ord($exp[0]) >= 128){ | ||
| $exp = \chr(0) . $exp; | ||
| } |
There was a problem hiding this comment.
This new block of code has a few styling issues that are inconsistent with the rest of the file:
- An unnecessary blank line has been added.
- Indentation is done with tabs instead of spaces.
- There is no space after the
ifkeyword.
I'd suggest cleaning this up for consistency.
// Correct encoding for ASN1, as ints are represented as unsigned in jwk
// but signed in ASN1. Prepending null byte makes it unsigned.
if (\strlen($mod) > 0 && \ord($mod[0]) >= 128) {
$mod = \chr(0) . $mod;
}
if (\strlen($exp) > 0 && \ord($exp[0]) >= 128) {
$exp = \chr(0) . $exp;
}
#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.