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

Add "algorithm mismatch" error to improve jws #304

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ghost
Copy link

@ghost ghost commented Nov 7, 2022

Upstream libraries that depend on jws.verify() break when the upstream keys contain a mixed set of algorithms. This is a nominal occurance for OIDC servers and should be properly handled.

Upstream libraries that depend on `jws.verify()` break when the
upstream keys contain a mixed set of algorithms. This is a nominal
occurance for OIDC servers and should be properly handled.
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Merging #304 (546e96c) into master (96474ec) will decrease coverage by 0.11%.
The diff coverage is 84.00%.

❗ Current head 546e96c differs from pull request most recent head 1ce256e. Consider uploading reports for the commit 1ce256e to get more accurate results

@@            Coverage Diff             @@
##           master     #304      +/-   ##
==========================================
- Coverage   92.94%   92.83%   -0.12%     
==========================================
  Files          15       15              
  Lines        1418     1423       +5     
==========================================
+ Hits         1318     1321       +3     
- Misses        100      102       +2     
Impacted Files Coverage Δ
jose/jws.py 93.54% <60.00%> (-1.50%) ⬇️
jose/backends/cryptography_backend.py 93.18% <77.77%> (ø)
jose/backends/ecdsa_backend.py 97.50% <100.00%> (ø)
jose/backends/native.py 97.56% <100.00%> (ø)
jose/backends/rsa_backend.py 95.62% <100.00%> (ø)
jose/exceptions.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ghost
Copy link
Author

ghost commented Nov 8, 2022

As mentioned in the issue, this implements step 2 of Appendix D of the JWS spec

  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.

@dimaqq
Copy link

dimaqq commented Jun 27, 2023

@mpdavis given that the original PR author account is deleted, maybe it's time to make a call: either take this PR over, maybe add more tests and merge it, or close it if it's incomplete?

My 2c: this PR is a good start.

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

Successfully merging this pull request may close these issues.

2 participants