Upgrade golang-jwt to v4
inge4pres opened this issue · 6 comments
Issue Description
The golang-jwt library imported in the middleware package suffers from a CVE.
A fix is present in v5 or v5 of the library, but upgrading to v5 changes the API.
An upgrade to v4.5.1 is enough to fix the vuln.
Checklist
- Dependencies installed
- No typos
- Searched existing issues and docs
Expected behaviour
A SCA scan does not surface any vulnerabilities.
Actual behaviour
Vulnerabilty is flagged.
Version/commit
v4.12.0
@vishr, @lammel maybe it is time to delete JWT middleware from core (this repo) and direct everyone to https://github.com/labstack/echo-jwt I really dislike the idea of major version bump for this.
I do not think upgrading JWT deps makes sense in this repo. It is a breaking change anyway. Upgrading to jwt to v4/v5 here is even worse as you might not notice the change until your casts to Token start panicing in handlers - (during runtime). At least removing the middleware would be impossible not do notice as your builds would fail after Echo version bump.
@inge4pres as a immediate remedy, please switch to using https://github.com/labstack/echo-jwt NB: make sure to create at lease one test in you app that uses jwt mw + you handler that checks for JWT Token.
This is because people often have user = c.Get("user").(*jwt.Token) or similar line in our handler to extract JWT Token from echo context and cast it to *jwt.Token. Now when Echo would silently upgrade to v4 or v5 in your handler go file, in imports you would still have github.com/golang-jwt/jwt but not github.com/golang-jwt/jwt/v5. That later one is actual version for jwt.Token type.
something like that
func TestMyHandlerWithJWTMW(t *testing.T) {
e := echo.New()
// replace this with https://github.com/labstack/echo-jwt
e.Use(middleware.JWTWithConfig(middleware.JWTConfig{
SigningKey: []byte("secret"),
}))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.Header.Set(echo.HeaderAuthorization, "bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWV9.TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ")
res := httptest.NewRecorder()
// ADD HERE YOUR HANDLER TO ECHO to test
// `user = c.Get("user").(*jwt.Token)` not panicing due different JWT version
// That line would definetely panic if Echo would bump JWT version to v4/v5 but you did not upgrade JWT import in this handle .go file
//e.GET("/", myHandler)
e.ServeHTTP(res, req)
assert.Equal(t, http.StatusOK, res.Code)
}@vishr, @lammel maybe it is time to delete JWT middleware from core (this repo) and direct everyone to https://github.com/labstack/echo-jwt I really dislike the idea of major version bump for this.
I do not think upgrading JWT deps makes sense in this repo. It is a breaking change anyway. Upgrading to
jwt to v4/v5here is even worse as you might not notice the change until your casts to Token start panicing in handlers - (during runtime). At least removing the middleware would be impossible not do notice as your builds would fail after Echo version bump.
Guess this is the way to go. The echo core should require as view dependencies as possible to avoid backwards compatibility issues. Bumping versions for echo-jwt will be far easier.