-
Notifications
You must be signed in to change notification settings - Fork 421
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
Validate JWT signing method #51
Conversation
This is a fix for the vulnerability outlined here: https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/ CVEs for other vendors: CVE-2015-2964 CVE-2015-2951
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -195,6 +195,11 @@ func (m *Middleware) getPossibleRequestIDs(r *http.Request) []string { | |||
log.Printf("... invalid token %s", err) | |||
continue | |||
} | |||
// Validate token signing method | |||
if token.Method != jwt.SigningMethodHS256 { | |||
log.Printf("EROR: invalid jwt signing method: %s", token.Method) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EROR -> ERROR
@@ -235,6 +240,12 @@ func (m *Middleware) Authorize(w http.ResponseWriter, r *http.Request, assertion | |||
http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) | |||
return | |||
} | |||
// Validate token signing method | |||
if state.Method != jwt.SigningMethodHS256 { | |||
log.Printf("EROR: invalid jwt signing method: %s (%s)", state.Method, stateCookie.Value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EROR -> ERROR
@@ -310,6 +321,11 @@ func (m *Middleware) IsAuthorized(r *http.Request) bool { | |||
log.Printf("ERROR: invalid token: %s", err) | |||
return false | |||
} | |||
// Validate token signing method | |||
if token.Method != jwt.SigningMethodHS256 { | |||
log.Printf("EROR: invalid jwt signing method: %s", token.Method) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EROR -> ERROR
so actually, I've had a look at the implementation of jwt-go and it looks fairly resistant to this problem. For example, if Ref: https://github.com/dgrijalva/jwt-go/blob/master/none.go#L28 Nevertheless, there might still be downgrade attacks possible in the future, so it makes sense to address the algorithm issue. go-jwt has a facility for this by setting Parser.ValidMethods (https://github.com/dgrijalva/jwt-go/blob/master/parser.go#L11) Could you please take a stab at a change that uses that instead? And even better if we can craft a test? |
superseded by #63 |
This is a fix for the vulnerability outlined here: https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/
CVEs for other vendors: CVE-2015-2964 CVE-2015-2951