In regards dgrijalva/jwt-go CVE

[aldas]

CVE-2020-26160

jwt-go before 4.0.0-preview1 allows attackers to bypass intended access restrictions in situations with []string{} for m["aud"] (which is allowed by the specification). Because the type assertion fails, "" is the value of aud. This is a security problem if the JWT token is presented to a service that lacks its own audience check.

Because Echo has direct import from dgrijalva/jwt-go and we expose that our of our API with public fields/types we can not just replace that library. It would not be backwards compatible. This will change in V5 (or maybe sooner)

Some remarks - Echo middleware is not using aud field validation and therefore is not directly affected. VerifyAudience is optional method that dgrijalva/jwt-go does not call during ordinary token validation. Unless you have written custom check for it and even then you are affected only when you have made that check optional

CVE occurs for mapClaims type, method VerifyAudience when second argument req is set to false

If you have used VerifyAudience in code like that you could be affected:

if ok := claims.VerifyAudience("https://mysite.com", false)  // false makes check optional
// you code is only affected when it is  set to `false`

Workarounds:

1. Use replace directive in go.mod in your project
   replace github.com/dgrijalva/jwt-go => github.com/golang-jwt/jwt v3.2.1

2. use JWTConfig.ParseTokenFunc to implement token parsing with library of your choosing.
   See https://echo.labstack.com/middleware/jwt/ for example. In that way you use JWT library to extract raw token from request and use ParseTokenFunc with golang-jwt/jwt to create token (and claims) with implementation that is affected by CVE.

import (
  "github.com/golang-jwt/jwt"
)

// ...
// ...

signingKey := []byte("secret")

config := middleware.JWTConfig{
  TokenLookup: "query:token",
  ParseTokenFunc: func(auth string, c echo.Context) (interface{}, error) {
    keyFunc := func(t *jwt.Token) (interface{}, error) {
      if t.Method.Alg() != "HS256" {
        return nil, fmt.Errorf("unexpected jwt signing method=%v", t.Header["alg"])
      }
      return signingKey, nil
    }

    // claims are of type `jwt.MapClaims` when token is created with `jwt.Parse`
    token, err := jwt.Parse(auth, keyFunc)
    if err != nil {
      return nil, err
    }
    if !token.Valid {
      return nil, errors.New("invalid token")
    }
    return token, nil
  },
}

e.Use(middleware.JWTWithConfig(config))

3. Use unaffected type of claims for middleware jwt.StandardClaims{} as JWTConfig.Claims value. Note: StandardClaims can not marshall slices as aud value (cast will panic and request will end with an error for client).

Example:

import (
  "github.com/golang-jwt/jwt"
)

...
...

config := middleware.JWTConfig{
  TokenLookup: "query:token",
  Claims: jwt.StandardClaims{},
  SigningKey: []byte("secret"),
}

e.Use(middleware.JWTWithConfig(config))

---

Now about that vulnerebility - CVE-2020-26160

Summary from CVE:

jwt-go before 4.0.0-preview1 allows attackers to bypass intended access restrictions in situations with []string{} for m["aud"] (which is allowed by the specification). Because the type assertion fails, "" is the value of aud. This is a security problem if the JWT token is presented to a service that lacks its own audience check.

dgrijalva/jwt-go has 2 types of claims

• StandardClaims which defines aud as string. aud being string means that this struct is NOT affected. Marshalling slice to string will cause panic and request will return an error for client
• MapClaims which defines aud as interface{} meaning it can be anything (including slice of strings). This struct is affected.

Aud checking code for MapClaims is:
https://github.com/dgrijalva/jwt-go/blob/008eba19055c071829e8317937b39845a9d2019b/map_claims.go#L15

func (m MapClaims) VerifyAudience(cmp string, req bool) bool {
  aud, _ := m["aud"].(string)  // <--- if `aud` is array/slice of strings instead of string this cast will result "" (empty string)
  return verifyAud(aud, cmp, req)
}

VerifyAudience is not called anywhere in dgrijalva/jwt-go library code. This is totally up to developer to implement/use.

Actual code for aud verification part is here. Note if verificatin is made optional and aud is empty this function returns true - meaning aud claim was considered passed.

https://github.com/dgrijalva/jwt-go/blob/008eba19055c071829e8317937b39845a9d2019b/claims.go#L93

func verifyAud(aud string, cmp string, required bool) bool {
  if aud == "" {
    return !required
  }
  if subtle.ConstantTimeCompare([]byte(aud), []byte(cmp)) != 0 {
    return true
  } else {
    return false
  }
}

If VerifyAudience second argument is to to true - meaning aud value is REQUIRED then method will never give us true when aud is array or empty in token.

Only way to have vulnerebility is to set VerifyAudience to OPTIONAL in code. But why on earth would you set this check to optional. If you try to summarize this to requirement it would be - application should verify token audience only when token constains aud. If token does not have aud claim consider this check passed. This would be similar as writing check for passwords

if password == "mysecret" || password == "" {
  login()
}

Note: to fake empty aud values you need to have token signed with valid key - if attacker has valid key why they would need to manipulate aud values?
In reality this is problem when you are using token signing keys for many applications (i.e. one key signs tokens meant for different "sites"/"applications" and attacker takes valid token from one site and uses it to requests resources on another site. where that token should not have this level of access)

anyway - this does not mean that problem does not exist at all and you are definetely better of with switching to https://github.com/golang-jwt/jwt in long run.

[lammel]

What are the implications for echo switching to https://github.com/golang-jwt/jwt with the next minor release (e.g. echo v4.5.0)?

On the downside I see:

• A breaking change which is unexpected for a minor release
• It will require all developers of echo to switch to golang-jwt/jwt too to be able to compile again. It will be fail fast though
• The release notes would need to provide a clear statement on the breaking change

But:

• The unmaintained repo will be removed from the other projects
• People not willing to upgrade yet, can still use echo v4.4.0

For echo v5 we should consider an agnostic implementation, which is not bound to a specific JWT implementation or move the JWT middleware into it's own repo.

[aldas]

I think I am the main party pooper here for this change.

As a explanation: I really feel that we are forced into breaking change this is not necessarily required. If you do not used that part of dgrijalva/jwt-go you are perfectly fine. I also think that CVE summary is somewhat portrayed in sensational manner (without enough explanation/context) which leads to reader think that they are affected and need to fix things immediately (i.e. forced change).

Now coming back to fix - even if library is changed I think one conceptional problem is still there. If you have coded VerifyAudience to check aud claim as an optional think you are failing in security. Because if that check is optional it is equivalent to

if password == "mysecret" || password == "" {
  login()
}

and this problem is not fixable by any patch or library change on Echo side.

anyway - the amount of created issues related to that is increasing and it is easier to change lib and be done with it. Lets wait a little for feedback end then go v4.5.0 and have oneliner like that in changelog for easier change

find . -type f -name "*.go" -print0 | xargs -0 sed -i '' -e 's/dgrijalva\/jwt-go/golang-jwt\/jwt/g'

p.s. just a sidenote. there was interesting blogpost (also interesting podcast 4:13 ) about npm audit recently. Which talks about some drawbacks that these (automated) security notices and implementations have that do not take problem context into consideration.
p.s.s. I am not implying that problem does not exists. I'm saying that every problem have nuances.

[thienlhhe130847]

for example I have an API /getProduct, i want to put two signkey (admin, user) to middleware JWT, It mean both user and admin can to access the API. HELP ME PLEASE

[zendrulat1]

Hi, I love the echo framework but some things that took up a lot of my time are the following.

1. On this page https://echo.labstack.com/cookbook/jwt/ you have nothing in documentation about jwt-go new library.
2. When you finally find https://echo.labstack.com/middleware/jwt/ and try to use the method ParseTokenFunc, it is not in https://pkg.go.dev/github.com/golang-jwt/[email protected]+incompatible
3. Then when the consumer finally finds https://pkg.go.dev/github.com/golang-jwt/[email protected]+incompatible, it doesnt represent anything that is in https://echo.labstack.com/middleware/jwt/

Can you possibly either update these or leave a link? or something? Cause this took up my weekend and I felt like I have had to learn a few libraries already. I just want some simple jwt custom claims middleware. This is starting to feel very javascripty.

Now that I have had to spend a really lot of time with this, how about an example with the new library on the echo website like https://play.golang.org/p/09QfbAiL8ub

[aldas]

@zendrulat1 ParseTokenFunc is Echo library configuration field not golang-jwt/jwt.

If you feel that documentation is lacking and it is pressing issue for you please raise a PR in https://github.com/labstack/echox repository - this is fastest way to make change happen.

[golangast]

Then why have the documentation on page https://echo.labstack.com/middleware/jwt/ show to do the following import and just below it have ParseTokenFunc ?

import ( "github.com/golang-jwt/jwt" )

[aldas]

because then code inside function we are creating there and assigning to ParseTokenFunc would have *jwt.Token and jwt.Parse(auth, keyFunc) derived from github.com/golang-jwt/jwt library and by that your token would be created as golang-jwt struct instead of drijalva struct.

drijalva and golang-jwt have exactly the same name for package jwt

[golangast]

How about just remove those JWT pages and say use this instead and here are examples.
https://pkg.go.dev/github.com/golang-jwt/[email protected]+incompatible#example-NewWithClaims-CustomClaimsType

And that you probably are going to have to use a custom middleware
https://echo.labstack.com/guide/error-handling/

[aldas]

no, these examples have their value as they demonstrate how to use claims with echo handlers and middlewares. I'll fix dgrijalva/jwt-go parts in docs as from v4.5.0 these are replaced with golang-jwt/jwt

[aldas]

v4.5.0 release switched JWT middleware JWT libs.

[aldas]

For history sake. Release v4.10.0 marked JWT middleware as deprecated in core and recommends separate repository https://github.com/labstack/echo-jwt that uses newer version of github.com/golang-jwt/jwt