New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SECURITY] Algorithm Confusion Through kid Header #95
Comments
Hi,
I’m not getting the attack pattern.
Even when alg header is replaced, it’s need to be in the expected algorithms array, isn’t it?
Do you have attacking code?
… On Aug 20, 2021, at 9:04, P.I.E. Security Team ***@***.***> wrote:
Your JWS implementation correctly rejects invalid algorithms.
https://github.com/nov/json-jwt/blob/a2b4c1599ef7c3604e214c46eadec9bfdb6e41a5/lib/json/jws.rb#L25 <https://github.com/nov/json-jwt/blob/a2b4c1599ef7c3604e214c46eadec9bfdb6e41a5/lib/json/jws.rb#L25>
However, when a kid header is present, it fetches the key after this algorithm check.
https://github.com/nov/json-jwt/blob/a2b4c1599ef7c3604e214c46eadec9bfdb6e41a5/lib/json/jws.rb#L124-L140 <https://github.com/nov/json-jwt/blob/a2b4c1599ef7c3604e214c46eadec9bfdb6e41a5/lib/json/jws.rb#L124-L140>
https://github.com/nov/json-jwt/blob/a2b4c1599ef7c3604e214c46eadec9bfdb6e41a5/lib/json/jose.rb#L24-L35 <https://github.com/nov/json-jwt/blob/a2b4c1599ef7c3604e214c46eadec9bfdb6e41a5/lib/json/jose.rb#L24-L35>
When JWKs are used, this algorithm check isn't congruently applied to the keys.
https://github.com/nov/json-jwt/blob/a2b4c1599ef7c3604e214c46eadec9bfdb6e41a5/lib/json/jwk.rb#L40-L51 <https://github.com/nov/json-jwt/blob/a2b4c1599ef7c3604e214c46eadec9bfdb6e41a5/lib/json/jwk.rb#L40-L51>
Therefore, if someone initializes a JWK or JWK::Set with different algorithm types, it's possible to swap the alg header and get the wrong key for a given algorithm. In extreme cases, this can lead to a cryptographic integrity bypass (reminiscent of the HS256/RS256 issue from years ago).
This is identical to the problem in firebase/php-jwt#351 <firebase/php-jwt#351>
To fix this issue: Keys MUST be stored, in memory, as both the raw key bytes and the specific algorithm the key is expected to be used with. After fetching a key, this algorithm MUST be validated against the algorithms array.
Note: This particular sharp edge isn't covered by the JWT Best Practices RFC <https://datatracker.ietf.org/doc/html/rfc8725>.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#95>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAGVI6BSKSOXZ7PRPRFR43T5WLZFANCNFSM5CPHZWMA>.
Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Your JWS implementation correctly rejects invalid algorithms.
json-jwt/lib/json/jws.rb
Line 25 in a2b4c15
However, when a
kid
header is present, it fetches the key after this algorithm check.json-jwt/lib/json/jws.rb
Lines 124 to 140 in a2b4c15
json-jwt/lib/json/jose.rb
Lines 24 to 35 in a2b4c15
When JWKs are used, this algorithm check isn't congruently applied to the keys.
json-jwt/lib/json/jwk.rb
Lines 40 to 51 in a2b4c15
Therefore, if someone initializes a JWK or JWK::Set with different algorithm types, it's possible to swap the
alg
header and get the wrong key for a given algorithm. In extreme cases, this can lead to a cryptographic integrity bypass (reminiscent of the HS256/RS256 issue from years ago).This is identical to the problem in firebase/php-jwt#351 https://seclists.org/fulldisclosure/2021/Aug/14
To fix this issue: Keys MUST be stored, in memory, as both the raw key bytes and the specific algorithm the key is expected to be used with. After fetching a key, this algorithm MUST be validated against the
algorithms
array.Note: This particular sharp edge isn't covered by the JWT Best Practices RFC.
The text was updated successfully, but these errors were encountered: