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

replaced unmaintained jwt library for maintained fork #1920

Closed

Conversation

cezary-butler
Copy link

The github.com/dgrijalva/jwt-go library has a known vulnerability: GHSA-w73w-5m7g-f7qc. Unfortunately it is also unmaintained and has no stable version with fix for the issue.
Since the is unmaintained and there is no stable version with fix for the mentioned issue, we have to migrate to the community maintained: github.com/golang-jwt/jwt.

Since middleware puts struct from the library into the context and the structures do not provide methods which could be used in interfaces, echo is tightly coupled to the jwt library and changing the library is a breaking change.

This could be solved by defining alias type and defining access methods on it. Introducing that change would still break compatibility, but it would allow to implement changes later without breaking compatibility.

@aldas
Copy link
Contributor

aldas commented Jul 13, 2021

Please see #1916 (comment)

Note: Echo is not using code that verifies aud and therefore is not directly affected by that CVE. jwt.MapClaims method VerifyAudience is affected when required boolean is set to false. Echo does not use that. If someone has written check for that then it is in their code and up to them to fix.

This is where token is created/parsed:

echo/middleware/jwt.go

Lines 253 to 271 in 58366f9

func (config *JWTConfig) defaultParseToken(auth string, c echo.Context) (interface{}, error) {
token := new(jwt.Token)
var err error
// Issue #647, #656
if _, ok := config.Claims.(jwt.MapClaims); ok {
token, err = jwt.Parse(auth, config.KeyFunc)
} else {
t := reflect.ValueOf(config.Claims).Type().Elem()
claims := reflect.New(t).Interface().(jwt.Claims)
token, err = jwt.ParseWithClaims(auth, claims, config.KeyFunc)
}
if err != nil {
return nil, err
}
if !token.Valid {
return nil, errors.New("invalid token")
}
return token, nil
}

Most problematic is that drijalva is unmaintained.

This will be addressed in future.

@cezary-butler
Copy link
Author

If I understand correctly this is not how this change will be processed in the future.
For now I close this PR. If we will come to agreement how to implement it in the v5 I might submit another one.

@aldas thanks for detailed explanation

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

Successfully merging this pull request may close these issues.

2 participants