Skip to content
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

JWKError when using a JWKS with multiple algorithms #138

Open
piedrahitapablo opened this issue Apr 12, 2019 · 5 comments
Open

JWKError when using a JWKS with multiple algorithms #138

piedrahitapablo opened this issue Apr 12, 2019 · 5 comments

Comments

@piedrahitapablo
Copy link

piedrahitapablo commented Apr 12, 2019

Hi, again, I'm not sure if this is expected behavior or a bug, but currently I'm in a situation where a JWT needs to be decoded and it can be encoded using RS256 or HS256, so my JWKS has 3 JWK, 2 for RS256 and 1 for HS256.

In this case, if I use a JWT encoded with HS256 and the first JWK in my JWKS has kty: 'RSA' a JWKError("Incorrect key type. Expected: 'oct', Recieved: RSA",) is raised, if the conditions are reversed, JWT encoded with RS256 and the first JWK having kty: 'oct', JWKError("Incorrect key type. Expected: 'RSA', Recieved: oct",) is raised. I think the troubling line is https://github.com/mpdavis/python-jose/blob/master/jose/jws.py#L216 as it attempts to construct a JWK using the wrong algorithm.

Thanks for developing this library and if I can be of any use to solve this (if it's a bug) let me know.

@zejn
Copy link
Collaborator

zejn commented Apr 15, 2019

This is currently a design limitation. The library currently does not feature key algorithm detection and as such, algorithm must currently be passed in as a separate argument. Thus, the code assumes all the keys passed in have the same algorithm.

This limitation could be removed, but I'm note entirely sure mixing different key types - what you're doing - is a good idea. If you can (if you have both ends under control), I'd recommend using kid header to identify the key used and only use that key to verify and decode the JWT.

Until this limitation is removed, the workaround is to separate the keys based on supported algorithm and try different algorithms in turn.

@piedrahitapablo
Copy link
Author

That seems like a good solution to me. Is there any reason for the library to iterate over the JWKS and ignore the kid? is it because some JWTs can be encoded without a kid?

@zejn
Copy link
Collaborator

zejn commented Apr 15, 2019

The library only has a naive implementation that iterates over all the keys provided in order. It currently does nothing to distinguish different keys.

The Appendix D of JWS spec has an overview how the logic to choose the key and even the order of the keys to verify with may look in a bit more complex application where there are multiple keys. Since this can be very application specific I'm not sure if there can be a good way for the library to provide this logic.

I do think there should at least be a clear error if the keys provided use different algorithms.

@piedrahitapablo
Copy link
Author

piedrahitapablo commented Apr 16, 2019

Thanks for the information, I see the problem now, it would be too cumbersome to implement a general solution that takes into account all the possible ways.

To remove the JWKS limitation, the algorithm argument of the .construct method can be modified to accept strings (current behavior) and an array of strings, if an array is received it could try to construct the JWK using any of the passed algorithms and if none of the algorithms works a JWKError can be raised. This can be useful since the jwk.decode method already accepts an array of algorithms as argument. I propose this solution since the alg and kid keys are optional on JWKs, and trying to implement logic that filters out JWKs depending on those keys will be too complicated. If this looks good to you I can make a PR.

@ghost
Copy link

ghost commented Nov 8, 2022

#304

This implements point 2 of Appendix D, filtering out keys that don't match alg when checking if any key signatures match. That's all that's needed to resolve this issue.

  1. Filter the set of collected keys. For instance, some
    applications will use only keys referenced by "kid" (key ID) or
    "x5t" (X.509 certificate SHA-1 thumbprint) parameters. If the
    application uses the JWK "alg" (algorithm), "use" (public key
    use), or "key_ops" (key operations) parameters, keys with
    inappropriate values of those parameters would be excluded.
    Additionally, keys might be filtered to include or exclude keys
    with certain other member values in an application-specific
    manner. For some applications, no filtering will be applied.

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

No branches or pull requests

2 participants