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

Documentation around Parse() #392

Open
bt-nia opened this issue Jun 21, 2024 · 0 comments
Open

Documentation around Parse() #392

bt-nia opened this issue Jun 21, 2024 · 0 comments

Comments

@bt-nia
Copy link

bt-nia commented Jun 21, 2024

The note in the docs (I guess sourced from the comment here)

Parse parses, validates, verifies the signature and returns the parsed token. keyFunc will receive the parsed token and should return the cryptographic key for verifying the signature. The caller is strongly encouraged to set the WithValidMethods option to validate the 'alg' claim in the token matches the expected algorithm. For more details about the importance of validating the 'alg' claim, see https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/

Sounds to me like the default of the library would be to just accept tokens with the none type alg. From my testing however, this does not seem to be the case:

package main

import (
	"fmt"
	"log"
	"time"

	"github.com/golang-jwt/jwt/v5"
)

func main() {
	// Define a test user
	testUser := "testuser"

	// Define a test signing key
	secretKey := []byte("mysecretkey")

	// Create a new token using HS256 signing method
	tokenHS256 := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
		"user": testUser,
		"exp":  time.Now().Add(time.Hour * 1).Unix(),
	})

	// Create a new token using None signing method
	tokenNone := jwt.NewWithClaims(jwt.SigningMethodNone, jwt.MapClaims{
		"user": testUser,
		"exp":  time.Now().Add(time.Hour * 1).Unix(),
	})

	// Sign and get the complete encoded token as a string using the secret key
	tokenStringHS256, err := tokenHS256.SignedString(secretKey)
	if err != nil {
		log.Fatalf("Failed to sign token with HS256: %v", err)
	}

	// Sign and get the complete encoded token as a string using None algorithm
	tokenStringNone, err := tokenNone.SignedString(jwt.UnsafeAllowNoneSignatureType)
	if err != nil {
		log.Fatalf("Failed to sign token with None: %v", err)
	}

	// Print both JWTs
	fmt.Printf("HS256 Token: %s\n", tokenStringHS256)
	fmt.Printf("None Token: %s\n", tokenStringNone)

	// Validation tests
	fmt.Println("Test 1: Current Method")
	validateTokenCurrent(tokenStringHS256, secretKey)
	validateTokenCurrent(tokenStringNone, jwt.UnsafeAllowNoneSignatureType)

	fmt.Println("Test 2: HS256 Key by Default")
	validateTokenHS256KeyDefault(tokenStringHS256, secretKey)
	validateTokenHS256KeyDefault(tokenStringNone, secretKey)

	fmt.Println("Test 3: WithValidMethods Option")
	validateTokenWithValidMethods(tokenStringHS256, secretKey)
	validateTokenWithValidMethods(tokenStringNone, jwt.UnsafeAllowNoneSignatureType)
}

// Test 1: Current Method
func validateTokenCurrent(tokenString string, key interface{}) {
	token, err := jwt.Parse(tokenString, func(token *jwt.Token) (interface{}, error) {
		// Validate the alg is what you expect
		switch token.Method.Alg() {
		case jwt.SigningMethodHS256.Alg():
			return key, nil
		case jwt.SigningMethodNone.Alg():
			return jwt.UnsafeAllowNoneSignatureType, nil
		default:
			return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
		}
	})

	printValidationResult(token, err)
}

// Test 2: HS256 Key by Default
func validateTokenHS256KeyDefault(tokenString string, key interface{}) {
	token, err := jwt.Parse(tokenString, func(token *jwt.Token) (interface{}, error) {
		return key, nil
	})

	printValidationResult(token, err)
}

// Test 3: WithValidMethods Option
func validateTokenWithValidMethods(tokenString string, key interface{}) {
	token, err := jwt.Parse(tokenString, func(token *jwt.Token) (interface{}, error) {
		return key, nil
	}, jwt.WithValidMethods([]string{jwt.SigningMethodHS256.Alg()}))

	printValidationResult(token, err)
}

// Utility function to print validation result
func printValidationResult(token *jwt.Token, err error) {
	if err != nil {
		fmt.Printf("Token validation failed: %v\n", err)
		return
	}

	if claims, ok := token.Claims.(jwt.MapClaims); ok && token.Valid {
		fmt.Printf("Token is valid. Claims: %v\n", claims)
	} else {
		fmt.Printf("Token is invalid\n")
	}
}

results in:

HS256 Token: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MTg4ODYzNzAsInVzZXIiOiJ0ZXN0dXNlciJ9.9EJkxvk7_4xZ9_McF96IyEF4oRK099KK7-nUjZcxk9w
None Token: eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJleHAiOjE3MTg4ODYzNzAsInVzZXIiOiJ0ZXN0dXNlciJ9.
Test 1: Current Method
Token is valid. Claims: map[exp:1.71888637e+09 user:testuser]
Token is valid. Claims: map[exp:1.71888637e+09 user:testuser]
Test 2: HS256 Key by Default
Token is valid. Claims: map[exp:1.71888637e+09 user:testuser]
Token validation failed: token signature is invalid: token is unverifiable: 'none' signature type is not allowed
Test 3: WithValidMethods Option
Token is valid. Claims: map[exp:1.71888637e+09 user:testuser]
Token validation failed: token signature is invalid: signing method none is invalid

See: Test 2

Therefore, I think a different text would be better here to still encourage developers to make use of the option, but not be worried that the library default is to accept insecure JWTs.

I submitted a MR for this:

Parse parses, validates, verifies the signature, and returns the parsed token. When a key is set and a JWT is provided, the library will not accept the 'none' type algorithm, ensuring security by default. However, it is strongly recommended to explicitly specify the allowed algorithms using the WithValidMethods option to ensure the 'none' type algorithm is definitively rejected. For more information on the importance of validating the 'alg' claim, see https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/.

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

No branches or pull requests

1 participant