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
Why is the 'alg' parameter required? #498
Comments
This is a good question, and is related to the resolution of GHSA-8xf4-w7qw-pjjw Since that resolution, every key needs to have an associated algorithm. This seems like a reasonable request to me. However, in the case of Microsoft's JWKS, they're providing keys without the algorithm. To their credit, in the JWK spec the "alg" claim is indeed optional (although I cannot imagine why they'd want to leave it out). The quick solution here is to simply provide the known algorithm as $keys = JWK::parseKeySet($microsoftUrl, 'RS256'); To address your concern: If Microsoft updated their JWKS to accept To support this kind of generation without matching the algorithms, and match keys based strictly on KID, bypassing the check that the keys are the same algorithm, could be done by allowing a wildcard algorithm: $keyOrKeyArray = [
new Key($key1, Key::ANY_ALG),
new Key($key2, Key::ANY_ALG),
]; Then in the // Check the algorithm
if (!self::constantTimeEquals($key->getAlgorithm(), $header->alg) && $key->getAlgorithm() !== Key::ANY_ALG) {
// See issue #351
throw new UnexpectedValueException('Incorrect key for this algorithm');
} However, this type of solution would open the door to GHSA-8xf4-w7qw-pjjw again. I do not see how we could safely support this type of behavior. Providing the default algorithm of the microsoft JKWS to |
Closing this issue as it's been over two months without receiving a response from the author. Please open another issue if you'd like to propose a way to make this work without introducing a security vulnerability. |
I'm sorry, your first reply didn't ask for more information from me right? |
@SamMousa only if you disagree with my decision to not support the feature and have a suggestion on how to implement it while addressing the concerns I've raised. |
In 6.0 a change was introduced that made
alg
required when parsing keys.At the time someone already noted that it was a breaking change: #376 (comment)
The solution was implemented in #426 where a default algorithm value could be set.
As far as I can tell no motivation for making this algorithm a required parameter was given. I'm working on an OIDC implementation and to me this feels like a very brittle approach, given that:
jwks_uri
configured in those documents, for example (https://login.microsoftonline.com/fd227a0d-5bcf-4e54-98c7-46a67c8cab14/discovery/v2.0/keys)This means that in advance I cannot know what default algorithm to use. Sure, I could check the JWKS contents or the tokens I receive and see that it is
RS256
today, but the whole point of this approach to configuration is that the other party can change it whenever they want. This means if Microsoft decides to switch toRS384
, my production code will break.Could you elaborate on why this requirement, which as is mentioned in the source code, is stricter than the spec was added?
The text was updated successfully, but these errors were encountered: