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

Force users to validate the alg to protect against jwt vulnerability. #50

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

Conversation

nullboundary
Copy link

Published on https://github.com/dgrijalva/jwt-go README:

NOTICE: A vulnerability in JWT was recently published. As this library doesn't force users to validate the alg is what they expected, it's possible your usage is effected. There will be an update soon to remedy this, and it will likey require backwards-incompatible changes to the API. In the short term, please make sure your implementation verifies the alg is what you expect.

So in the mean time I implemented this fix following their instructions.

 // Don't forget to validate the alg is what you expect:
        if _, ok := token.Method.(*jwt.SigningMethodRSA); !ok {
            return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"])
        }

Also added the fixes from #34

A vulnerability in JWT was recently discovered that allows access if the algorithm is not checked when the token is verified. This fixes it by following the recommendations of dgrijalva/jwt-go
Added token to the context so you can potential use some of the tokens contained info in your request handler
@appleboy appleboy mentioned this pull request Mar 19, 2016
@appleboy
Copy link
Member

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