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

Guard against PKCS1 PEM-encoded public keys #277

Merged
merged 3 commits into from Jun 22, 2017

Conversation

Projects
None yet
3 participants
@jpadilla
Owner

jpadilla commented Jun 22, 2017

Previous to this we were not correctly accounting for all PEM encoded public keys, like the PKCS1 PEM encoded format which is prefaced by -----BEGIN RSA PUBLIC KEY-----. This fix prevents symmetric/asymmetric key confusion attacks against users using the PKCS1 PEM encoded public keys, which would allow an attacker to craft JWTs from scratch.

We've also added a deprecation warning when using decode() and not specifying the algorithms param. This will be required in a future release.

jpadilla added some commits Jun 21, 2017

@jpadilla jpadilla added the bug label Jun 22, 2017

@jpadilla jpadilla self-assigned this Jun 22, 2017

@jpadilla jpadilla requested a review from mark-adams Jun 22, 2017

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jun 22, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling f812f69 on fix-vuln into e4c67b1 on master.

coveralls commented Jun 22, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling f812f69 on fix-vuln into e4c67b1 on master.

@jpadilla jpadilla merged commit eb3f581 into master Jun 22, 2017

5 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details

@jpadilla jpadilla deleted the fix-vuln branch Jun 22, 2017

@paragonie-scott

This comment has been minimized.

Show comment
Hide comment
@paragonie-scott

paragonie-scott Sep 1, 2017

Should this also block ECDSA public keys? Or are those outright not supported?

paragonie-scott commented Sep 1, 2017

Should this also block ECDSA public keys? Or are those outright not supported?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment