Skip to content

fix: RSA from JWK sometimes returns empty Instance#628

Open
NReib wants to merge 2 commits intofirebase:mainfrom
NReib:main
Open

fix: RSA from JWK sometimes returns empty Instance#628
NReib wants to merge 2 commits intofirebase:mainfrom
NReib:main

Conversation

@NReib
Copy link
Copy Markdown

@NReib NReib commented Mar 27, 2026

#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.

firebase#589 addresses specification difference between JWK (or JWA) and ASN1 integers.
@google-cla
Copy link
Copy Markdown

google-cla bot commented Mar 27, 2026

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.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Comment on lines +243 to +251

// 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;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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 if keyword.

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;
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant