JWT middleware error when OAuth audience is array (jwt-go dependency upgrade to fix)

[drewsilcock]

Issue Description

Unable to unmarshal JWT due to jwt-go library version not supporting array audiences (aud).

This issue is due to library github.com/dgrijalva/jwt-go. Echo is currently on version v3.2.0+incompatible. This is fixed in version v4.0.0-preview1, specifically commit ec0a89a.

Upgrading from github.com/dgrijalva/jwt-go v3.2.0+incompatible to github.com/dgrijalva/jwt-go/v4 v4.0.0-preview1 should fix this error.

Occurs when using Keycloak 10.0.2 as OAuth provider.

Checklist

• Dependencies installed
• No typos
• Searched existing issues and docs

Expected behaviour

JWT middleware is able to unmarshal JWT when the OAuth token contains an audience field which is an array, as allowed by RFC 7519.

Actual behaviour

JWT middleware expects the aud field of the JWT to be a string only, and so throws an error when it is an array.

Steps to reproduce

• Connect to OAuth using user where audience is not single value, using e.g. Keycloak v10.0.2.
• Make request to Echo API using OAuth token where API uses JWT middleware.
• Observe error:

{"time":"2020-07-21T17:30:12.391848+01:00","id":"","remote_ip":"127.0.0.1","host":"localhost:8000","method":"GET","uri":"/hello","user_agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:79.0) Gecko/20100101 Firefox/79.0","status":401,"error":"code=401, message=invalid or expired jwt, internal=json: cannot unmarshal array into Go struct field JwtClaims.aud of type string","latency":254265,"latency_human":"254.265µs","bytes_in":0,"bytes_out":37}

Working code to debug

package main

import (
	echoMiddleware "github.com/labstack/echo/middleware"
	"github.com/labstack/echo/v4"
	"github.com/dgrijalva/jwt-go"
)

// Retrieved via call to Keycloak RESTful API in this case. Code for retrieving and processing the
// certificates is a bit too length to include here.
var RSAPublicKeys map[string]interface{}

type JwtUser struct {
	*jwt.StandardClaims
	*jwt.Token
}

type JwtClaims struct {
    jwt.StandardClaims
}

func main() {
	e := echo.New()

	apiGroup := e.Group("/")
	apiGroup.Use(echoMiddleware.JWTWithConfig(echoMiddleware.JWTConfig{
		SigningKeys:   RSAPublicKeys,
		SigningMethod: jwt.SigningMethodRS256.Name,
		Claims:        &jwt.StandardClaims{},
	}))

	apiGroup.GET("hello", greeter)

	e.Logger.Fatal(e.Start(":8000"))
}

func greeter(c echo.Context) error {
	user := getJwtUser(c)

	return c.String(
		http.StatusOK,
		fmt.Sprintf("Hey there %v!", user),
	)
}

func getJwtUser(c echo.Context) JwtUser {
	token := c.Get("user").(*jwt.Token)
	claims := token.Claims.(*JwtClaims)

	return JwtUser{
		JwtClaims: claims,
		Token:     token,
	}
}

Version/commit

Echo version v4.1.16.

Edit: Fixed code example after feedback.

[blind3dd]

This code won't run.

First of all you define the user variable and you don't use it - won't even run // perhaps you should return that user in greeter func

so this func should probably be written like this for starters (certainly it's the simplest and incomplete example just to illustrate):

func greeter(c echo.Context) error {
	user := getJwtUser(c)
	fmt.Println(user)

	return c.String(
		http.StatusOK,
		fmt.Sprintf("Hey there %v!", user),
	)
}

Next you have (*JwtClaims) in getJwtUser func in claims := token.Claims.(*JwtClaims) - it's incorrect. you are missing a struct definition.

so you could add new Type JwtClaims, but this would required adding .StandardClaims to the claims for types to match, see below refactor with the struct defined:

type JwtClaims struct {
	*jwt.StandardClaims
}

func getJwtUser(c echo.Context) JwtUser {
	token := c.Get("user").(*jwt.Token)
	claims := token.Claims.(*JwtClaims).StandardClaims

	return JwtUser{
		StandardClaims: claims,
		Token:     token,
	}
}

Furthermore you use echoMiddleware in main function. There is no such thing.
to go around this:

import (	
   echoMiddleware "github.com/labstack/echo/middleware"
)

[drewsilcock]

Thanks for the feedback, I copied this from a larger codebase and tried to strip out the unnecessary bits but obviously made some mistakes. I've updated the original post to reflect your suggestions.

In terms of the actual issue, if people are looking for a fix, I've been using a workaround which is to copy+paste the jwt.go file from the Echo middleware and using a modified version in my own middleware package (hence the reason I used the alias echoMiddleware). I had to make a couple of modifications, so here's my middleware/jwt.go for anyone who might want to copy+paste this as a workaround:

package middleware

// This package is almost identical to the built-in Echo JWT middleware, but uses jwt-go v4 to fix an error where
// the audience field in the JWT token is an array of strings instead of a single string.

import (
	"fmt"
	"net/http"
	"reflect"
	"strings"

	"github.com/dgrijalva/jwt-go/v4"
	"github.com/labstack/echo/v4"
	echoMiddleware "github.com/labstack/echo/v4/middleware"
)

type (
	// JWTConfig defines the config for JWT middleware.
	JWTConfig struct {
		// Skipper defines a function to skip middleware.
		Skipper echoMiddleware.Skipper

		// BeforeFunc defines a function which is executed just before the middleware.
		BeforeFunc echoMiddleware.BeforeFunc

		// SuccessHandler defines a function which is executed for a valid token.
		SuccessHandler JWTSuccessHandler

		// ErrorHandler defines a function which is executed for an invalid token.
		// It may be used to define a custom JWT error.
		ErrorHandler JWTErrorHandler

		// ErrorHandlerWithContext is almost identical to ErrorHandler, but it's passed the current context.
		ErrorHandlerWithContext JWTErrorHandlerWithContext

		// Signing key to validate token. Used as fallback if SigningKeys has length 0.
		// Required. This or SigningKeys.
		SigningKey interface{}

		// Map of signing keys to validate token with kid field usage.
		// Required. This or SigningKey.
		SigningKeys map[string]interface{}

		// Signing method, used to check token signing method.
		// Optional. Default value HS256.
		SigningMethod string

		// Context key to store user information from the token into context.
		// Optional. Default value "user".
		ContextKey string

		// Claims are extendable claims data defining token content.
		// Optional. Default value jwt.MapClaims
		Claims jwt.Claims

		// TokenLookup is a string in the form of "<source>:<name>" that is used
		// to extract token from the request.
		// Optional. Default value "header:Authorization".
		// Possible values:
		// - "header:<name>"
		// - "query:<name>"
		// - "param:<name>"
		// - "cookie:<name>"
		TokenLookup string

		// AuthScheme to be used in the Authorization header.
		// Optional. Default value "Bearer".
		AuthScheme string

		keyFunc jwt.Keyfunc
	}

	// JWTSuccessHandler defines a function which is executed for a valid token.
	JWTSuccessHandler func(echo.Context)

	// JWTErrorHandler defines a function which is executed for an invalid token.
	JWTErrorHandler func(error) error

	// JWTErrorHandlerWithContext is almost identical to JWTErrorHandler, but it's passed the current context.
	JWTErrorHandlerWithContext func(error, echo.Context) error

	jwtExtractor func(echo.Context) (string, error)
)

// Algorithms
const (
	AlgorithmHS256 = "HS256"
)

// Errors
var (
	ErrJWTMissing = echo.NewHTTPError(http.StatusBadRequest, "missing or malformed jwt")
)

var (
	// DefaultJWTConfig is the default JWT auth middleware config.
	DefaultJWTConfig = JWTConfig{
		Skipper:       echoMiddleware.DefaultSkipper,
		SigningMethod: AlgorithmHS256,
		ContextKey:    "user",
		TokenLookup:   "header:" + echo.HeaderAuthorization,
		AuthScheme:    "Bearer",
		Claims:        jwt.MapClaims{},
	}
)

// JWT returns a JSON Web Token (JWT) auth middleware.
//
// For valid token, it sets the user in context and calls next handler.
// For invalid token, it returns "401 - Unauthorized" error.
// For missing token, it returns "400 - Bad Request" error.
//
// See: https://jwt.io/introduction
// See `JWTConfig.TokenLookup`
func JWT(key interface{}) echo.MiddlewareFunc {
	c := DefaultJWTConfig
	c.SigningKey = key
	return JWTWithConfig(c)
}

// JWTWithConfig returns a JWT auth middleware with config.
// See: `JWT()`.
func JWTWithConfig(config JWTConfig) echo.MiddlewareFunc {
	// Defaults
	if config.Skipper == nil {
		config.Skipper = DefaultJWTConfig.Skipper
	}
	if config.SigningKey == nil && len(config.SigningKeys) == 0 {
		panic("echo: jwt middleware requires signing key")
	}
	if config.SigningMethod == "" {
		config.SigningMethod = DefaultJWTConfig.SigningMethod
	}
	if config.ContextKey == "" {
		config.ContextKey = DefaultJWTConfig.ContextKey
	}
	if config.Claims == nil {
		config.Claims = DefaultJWTConfig.Claims
	}
	if config.TokenLookup == "" {
		config.TokenLookup = DefaultJWTConfig.TokenLookup
	}
	if config.AuthScheme == "" {
		config.AuthScheme = DefaultJWTConfig.AuthScheme
	}
	config.keyFunc = func(t *jwt.Token) (interface{}, error) {
		// Check the signing method
		if t.Method.Alg() != config.SigningMethod {
			return nil, fmt.Errorf("unexpected jwt signing method=%v", t.Header["alg"])
		}
		if len(config.SigningKeys) > 0 {
			if kid, ok := t.Header["kid"].(string); ok {
				if key, ok := config.SigningKeys[kid]; ok {
					return key, nil
				}
			}
			return nil, fmt.Errorf("unexpected jwt key id=%v", t.Header["kid"])
		}

		return config.SigningKey, nil
	}

	// Initialize
	parts := strings.Split(config.TokenLookup, ":")
	extractor := jwtFromHeader(parts[1], config.AuthScheme)
	switch parts[0] {
	case "query":
		extractor = jwtFromQuery(parts[1])
	case "param":
		extractor = jwtFromParam(parts[1])
	case "cookie":
		extractor = jwtFromCookie(parts[1])
	}

	return func(next echo.HandlerFunc) echo.HandlerFunc {
		return func(c echo.Context) error {
			if config.Skipper(c) {
				return next(c)
			}

			if config.BeforeFunc != nil {
				config.BeforeFunc(c)
			}

			auth, err := extractor(c)
			if err != nil {
				if config.ErrorHandler != nil {
					return config.ErrorHandler(err)
				}

				if config.ErrorHandlerWithContext != nil {
					return config.ErrorHandlerWithContext(err, c)
				}
				return err
			}
			var token *jwt.Token
			// 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, jwt.WithoutAudienceValidation())
			}
			if err == nil && token.Valid {
				// Store user information from token into context.
				c.Set(config.ContextKey, token)
				if config.SuccessHandler != nil {
					config.SuccessHandler(c)
				}
				return next(c)
			}
			if config.ErrorHandler != nil {
				return config.ErrorHandler(err)
			}
			if config.ErrorHandlerWithContext != nil {
				return config.ErrorHandlerWithContext(err, c)
			}
			return &echo.HTTPError{
				Code:     http.StatusUnauthorized,
				Message:  "invalid or expired jwt",
				Internal: err,
			}
		}
	}
}

// jwtFromHeader returns a `jwtExtractor` that extracts token from the request header.
func jwtFromHeader(header string, authScheme string) jwtExtractor {
	return func(c echo.Context) (string, error) {
		auth := c.Request().Header.Get(header)
		l := len(authScheme)
		if len(auth) > l+1 && auth[:l] == authScheme {
			return auth[l+1:], nil
		}
		return "", ErrJWTMissing
	}
}

// jwtFromQuery returns a `jwtExtractor` that extracts token from the query string.
func jwtFromQuery(param string) jwtExtractor {
	return func(c echo.Context) (string, error) {
		token := c.QueryParam(param)
		if token == "" {
			return "", ErrJWTMissing
		}
		return token, nil
	}
}

// jwtFromParam returns a `jwtExtractor` that extracts token from the url param string.
func jwtFromParam(param string) jwtExtractor {
	return func(c echo.Context) (string, error) {
		token := c.Param(param)
		if token == "" {
			return "", ErrJWTMissing
		}
		return token, nil
	}
}

// jwtFromCookie returns a `jwtExtractor` that extracts token from the named cookie.
func jwtFromCookie(name string) jwtExtractor {
	return func(c echo.Context) (string, error) {
		cookie, err := c.Cookie(name)
		if err != nil {
			return "", ErrJWTMissing
		}
		return cookie.Value, nil
	}
}

A diff version if people find it easier to see what I changed here:

2a3,5
> // This package is almost identical to the built-in Echo JWT middleware, but uses jwt-go v4 to fix an error where
> // the audience field in the JWT token is an array of strings instead of a single string.
> 
9c12
< 	"github.com/dgrijalva/jwt-go"
---
> 	"github.com/dgrijalva/jwt-go/v4"
10a14
> 	echoMiddleware "github.com/labstack/echo/v4/middleware"
17c21
< 		Skipper Skipper
---
> 		Skipper echoMiddleware.Skipper
20c24
< 		BeforeFunc BeforeFunc
---
> 		BeforeFunc echoMiddleware.BeforeFunc
94c98
< 		Skipper:       DefaultSkipper,
---
> 		Skipper:       echoMiddleware.DefaultSkipper,
192c196
< 			token := new(jwt.Token)
---
> 			var token *jwt.Token
199c203
< 				token, err = jwt.ParseWithClaims(auth, claims, config.keyFunc)
---
> 				token, err = jwt.ParseWithClaims(auth, claims, config.keyFunc, jwt.WithoutAudienceValidation())

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[drewsilcock]

Looks like this upgrade will happen anyway because of a very much related security vulnerability.

For those reading this issue and looking for news on the jwt-go drama updates, see:

• jwt-go Access Restriction Bypass Vuln #1647
• Fix: High Severity Security Vulnerability - Access Restriction Bypass #1663
• Security Vulnerability: failed type assertion leads to bypassing Audience verification dgrijalva/jwt-go#422