Closed paragonie-security closed 2 years ago
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 https://github.com/firebase/php-jwt/issues/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 https://github.com/nov/json-jwt/issues/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.
Your JWS implementation correctly rejects invalid algorithms.
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/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
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 https://github.com/firebase/php-jwt/issues/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.