fix(apiserver): pin JWT parsing to the HS256 signing algorithm (bug-1950)
Cause: the ValidateHandler parse callback (middleware isValidToken) returned the secret without looking at token.Method, and GetUsernameFromToken accepted any HMAC variant, so tokens signed with HS384/HS512 were accepted although the server only signs HS256. Fix: one JWTSigningMethod (HS256) used for signing, plus a shared JWTKeyFunc (rejects any other alg) and JWTValidMethods parser option used by both parse paths. Tests: TestIsValidToken_PinsSigningAlgorithm (middleware), TestGetUsernameFromToken_PinsSigningAlgorithm (controllers). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f91642dbc0
commit
42dd1e7a5e
4 changed files with 139 additions and 12 deletions
|
|
@ -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
|
||||
|
|
|
|||
47
src/frontend/apiserver/controllers/auth_test.go
Normal file
47
src/frontend/apiserver/controllers/auth_test.go
Normal file
|
|
@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
|
|||
70
src/frontend/apiserver/middleware/validatehandler_test.go
Normal file
70
src/frontend/apiserver/middleware/validatehandler_test.go
Normal file
|
|
@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue