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

Fix for CVE-2017-11424 #63

Merged
merged 2 commits into from
Sep 1, 2017
Merged

Conversation

sirosen
Copy link
Contributor

@sirosen sirosen commented Sep 1, 2017

Add "RSA PUBLIC KEY" to the forbidden key strings in HMAC. Prevents the use of PKCS1 keys, cited by this CVE as exposing a key-confusion attack.
Also add a test case for it, doing the obvious thing.

Closes #62

Add "RSA PUBLIC KEY" to the forbidden key strings in HMAC. Prevents the
use of PKCS1 keys, cited by this CVE as exposing a key-confusion attack.
Also add a test case for it, doing the obvious thing.

Closes mpdavis#62
@zejn
Copy link
Collaborator

zejn commented Sep 1, 2017

Looks good to me. The PyPy build failed because of PyCrypto failing to compile.

@mpdavis
Copy link
Owner

mpdavis commented Sep 1, 2017

I'm happy with this change, but I want to see what is up with the failing build before merging.

@mpdavis mpdavis closed this Sep 1, 2017
@mpdavis mpdavis reopened this Sep 1, 2017
@mpdavis
Copy link
Owner

mpdavis commented Sep 1, 2017

@sirosen I noticed your builds are running on the newer trusty builders.

Can you try adding dist: precise to .travis.yml to see if that fixes the build issue?

@codecov-io
Copy link

codecov-io commented Sep 1, 2017

Codecov Report

Merging #63 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #63   +/-   ##
=======================================
  Coverage   94.53%   94.53%           
=======================================
  Files          12       12           
  Lines         841      841           
=======================================
  Hits          795      795           
  Misses         46       46
Impacted Files Coverage Δ
jose/jwk.py 95.12% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b54c12a...5bc7470. Read the comment docs.

@sirosen
Copy link
Contributor Author

sirosen commented Sep 1, 2017

Yeah, I don't mind slipping that change in. Doing that now.

Pin to older infra to hopefully resolve pycrypto compilation issues.
@mpdavis
Copy link
Owner

mpdavis commented Sep 1, 2017

lgtm

@mpdavis mpdavis merged commit 7569fdf into mpdavis:master Sep 1, 2017
@mpdavis
Copy link
Owner

mpdavis commented Sep 1, 2017

Released in 1.4.0

fergyfresh added a commit to fergyfresh/django-rest-framework-simplejwt that referenced this pull request Apr 4, 2019
ludarkhorse added a commit to ludarkhorse/djangoframework that referenced this pull request Feb 24, 2023
mm-pro pushed a commit to mm-pro/JWT-basic-Django that referenced this pull request Oct 29, 2023
cr313 added a commit to cr313/rest_framework_simple that referenced this pull request Apr 19, 2024
rango1105 added a commit to rango1105/django-simplejwt that referenced this pull request Sep 27, 2024
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.

4 participants