awslabs / aws-jwt-verify

JS library for verifying JWTs signed by Amazon Cognito, and any OIDC-compatible IDP that signs JWTs with RS256, RS384, and RS512
Apache License 2.0
594 stars 43 forks source link

Fix MSB 1 leading to negative modulus in bun #155

Closed ottokruse closed 4 months ago

ottokruse commented 4 months ago

Issue #, if available: #154

Description of changes: Fixes a bug in the DER encoder of aws-jwt-verify that does not add a leading 0-byte if the MSB (most significant bit) of the public key modulus is 1. In NodeJS this doesn't throw, because OpenSSL is friendly enough (?) to still parse the modulus as a positive number, but in bun it leads to the modulus being parsed as a negative number as @cirospaciari saw. I think that's actually right, because the number is supposed to be encoded in 2's complement per DER spec (https://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf#%5B%7B%22num%22%3A41%2C%22gen%22%3A0%7D%2C%7B%22name%22%3A%22FitH%22%7D%2C799%5D).

(Also, had to fix a unit test that suddenly failed on me in Node v20, as the message changed slightly)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

ottokruse commented 4 months ago

Should also add a unit test to catch the error condition (MSB 1)

Update: DONE

ottokruse commented 4 months ago

Note in modern Node (and presume in Bun) you don't need a DER encoder as you can use JWKs natively with crypto. But officially we still support Node14 that can't do that so for now I will leave the DER encoder in place.