diff --git a/src/frontend/apiserver/controllers/auth.go b/src/frontend/apiserver/controllers/auth.go index 96000fe1..0a7e7d2e 100644 --- a/src/frontend/apiserver/controllers/auth.go +++ b/src/frontend/apiserver/controllers/auth.go @@ -22,6 +22,24 @@ import ( // Define a struct for handling authentication operations type AuthController struct{} +// JWTSigningMethod is the only algorithm the API server signs tokens with; +// every parse path accepts only this algorithm (bug-1950). +var JWTSigningMethod = jwt.SigningMethodHS256 + +// JWTValidMethods is the parser option that pins token parsing to +// JWTSigningMethod. +func JWTValidMethods() jwt.ParserOption { + return jwt.WithValidMethods([]string{JWTSigningMethod.Alg()}) +} + +// JWTKeyFunc returns the HMAC secret for a token signed with JWTSigningMethod. +func JWTKeyFunc(token *jwt.Token) (interface{}, error) { + if token.Method.Alg() != JWTSigningMethod.Alg() { + return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) + } + return []byte(utils.AuthenticationYamlConfig.Token.SecretKey), nil +} + // generate JWT token func generateJWT(username string, IP string) (string, error) { //appConfig := utils.AppYamlConfig @@ -37,7 +55,7 @@ func generateJWT(username string, IP string) (string, error) { } // Create token with claims - token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + token := jwt.NewWithClaims(JWTSigningMethod, claims) // Generate signed token secretKey := []byte(utils.AuthenticationYamlConfig.Token.SecretKey) // In production, use environment variable @@ -181,13 +199,7 @@ func (a AuthController) Logout(ctx *gin.Context) { } func GetUsernameFromToken(tokenString string) (string, error) { - token, err := jwt.Parse(strings.TrimPrefix(tokenString, "Bearer "), func(token *jwt.Token) (interface{}, error) { - // Validate the algorithm - if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok { - return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) - } - return []byte(utils.AuthenticationYamlConfig.Token.SecretKey), nil - }) + token, err := jwt.Parse(strings.TrimPrefix(tokenString, "Bearer "), JWTKeyFunc, JWTValidMethods()) if err != nil { return "", err diff --git a/src/frontend/apiserver/controllers/auth_test.go b/src/frontend/apiserver/controllers/auth_test.go new file mode 100644 index 00000000..b00296a2 --- /dev/null +++ b/src/frontend/apiserver/controllers/auth_test.go @@ -0,0 +1,47 @@ +//go:build !cgo + +package controllers + +import ( + "apiserver/models" + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" +) + +// bug-1950: GetUsernameFromToken accepts only the signing algorithm (HS256), +// not any HMAC variant. +func TestGetUsernameFromToken_PinsSigningAlgorithm(t *testing.T) { + resetIntegrationState() + initTestAuthConfig() + + tests := []struct { + name string + method jwt.SigningMethod + wantErr bool + }{ + {"HS256", jwt.SigningMethodHS256, false}, + {"HS384", jwt.SigningMethodHS384, true}, + {"HS512", jwt.SigningMethodHS512, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + claims := &models.JWTClaims{ + Username: "alice", + IP: "127.0.0.1", + RegisteredClaims: jwt.RegisteredClaims{ + ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), + }, + } + signed, err := jwt.NewWithClaims(tt.method, claims).SignedString([]byte(testJWTSecret)) + if err != nil { + t.Fatalf("sign: %v", err) + } + user, err := GetUsernameFromToken("Bearer " + signed) + if (err != nil) != tt.wantErr { + t.Errorf("GetUsernameFromToken(%s) = %q, err %v; wantErr %v", tt.method.Alg(), user, err, tt.wantErr) + } + }) + } +} diff --git a/src/frontend/apiserver/middleware/validatehandler.go b/src/frontend/apiserver/middleware/validatehandler.go index ccdf9629..5720a376 100644 --- a/src/frontend/apiserver/middleware/validatehandler.go +++ b/src/frontend/apiserver/middleware/validatehandler.go @@ -4,7 +4,6 @@ import ( "apiserver/common" "apiserver/controllers" "apiserver/models" - "apiserver/utils" logger "apiserver/utils" "bytes" "encoding/json" @@ -99,9 +98,8 @@ func validateHeader() gin.HandlerFunc { } func isValidToken(tokenString string, IP string) (bool, error) { - token, err := jwt.ParseWithClaims(tokenString, &models.JWTClaims{}, func(token *jwt.Token) (interface{}, error) { - return []byte(utils.AuthenticationYamlConfig.Token.SecretKey), nil - }) + token, err := jwt.ParseWithClaims(tokenString, &models.JWTClaims{}, + controllers.JWTKeyFunc, controllers.JWTValidMethods()) if err != nil { return false, err diff --git a/src/frontend/apiserver/middleware/validatehandler_test.go b/src/frontend/apiserver/middleware/validatehandler_test.go new file mode 100644 index 00000000..d528d825 --- /dev/null +++ b/src/frontend/apiserver/middleware/validatehandler_test.go @@ -0,0 +1,70 @@ +package middleware + +import ( + "apiserver/models" + "apiserver/utils" + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" + "go.uber.org/zap" +) + +const testJWTSecret = "middleware-test-secret" + +// setTestAuthConfig installs a JWT config for the test and restores the +// previous one afterwards. +func setTestAuthConfig(t *testing.T) { + t.Helper() + prev := utils.AuthenticationYamlConfig + prevLog := utils.Log + utils.AuthenticationYamlConfig = &utils.Authentication{ + Token: utils.TokenConfig{Type: "jwt", SecretKey: testJWTSecret, TokenExpireTime: 3600}, + } + utils.Log = zap.NewNop() + t.Cleanup(func() { + utils.AuthenticationYamlConfig = prev + utils.Log = prevLog + }) +} + +func signTestToken(t *testing.T, method jwt.SigningMethod, username, ip string) string { + t.Helper() + claims := &models.JWTClaims{ + Username: username, + IP: ip, + RegisteredClaims: jwt.RegisteredClaims{ + ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), + IssuedAt: jwt.NewNumericDate(time.Now()), + }, + } + signed, err := jwt.NewWithClaims(method, claims).SignedString([]byte(testJWTSecret)) + if err != nil { + t.Fatalf("sign token: %v", err) + } + return signed +} + +// bug-1950: only the HMAC algorithm used for signing (HS256) is accepted. +func TestIsValidToken_PinsSigningAlgorithm(t *testing.T) { + setTestAuthConfig(t) + + tests := []struct { + name string + method jwt.SigningMethod + want bool + }{ + {"HS256 (signing algorithm)", jwt.SigningMethodHS256, true}, + {"HS384", jwt.SigningMethodHS384, false}, + {"HS512", jwt.SigningMethodHS512, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + token := signTestToken(t, tt.method, "alice", "10.0.0.1") + got, err := isValidToken(token, "10.0.0.1") + if got != tt.want { + t.Errorf("isValidToken(%s) = %v (err %v), want %v", tt.method.Alg(), got, err, tt.want) + } + }) + } +}