From d24f449e089123210af06c51ee92ea2d51404a31 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Wed, 15 Apr 2026 11:33:36 +0800 Subject: [PATCH] fix: CVE-2026-33322 harden OIDC JWT verification Close the OIDC JWT algorithm confusion flaw in AssumeRoleWithWebIdentity by restoring a JWKS-only verification path. Stop injecting the client secret into the verifier keyring and restrict accepted signing methods to the asymmetric algorithms already supported by the existing JWKS flow. Add regression coverage to verify HS256 tokens are rejected, RS256 tokens remain valid, and JWKS refresh and retry logic cannot bypass the method allowlist. Co-authored-by: Codex Co-authored-by: Claude Code --- internal/config/identity/openid/jwt.go | 13 +- internal/config/identity/openid/jwt_test.go | 227 ++++++++++++++++++-- internal/config/identity/openid/openid.go | 3 +- 3 files changed, 222 insertions(+), 21 deletions(-) diff --git a/internal/config/identity/openid/jwt.go b/internal/config/identity/openid/jwt.go index 2a422010f..48fe0ef72 100644 --- a/internal/config/identity/openid/jwt.go +++ b/internal/config/identity/openid/jwt.go @@ -19,6 +19,7 @@ package openid import ( "context" + "crypto" "encoding/json" "errors" "fmt" @@ -38,7 +39,7 @@ type publicKeys struct { *sync.RWMutex // map of kid to public key - pkMap map[string]any + pkMap map[string]crypto.PublicKey } func (pk *publicKeys) parseAndAdd(b io.Reader) error { @@ -59,14 +60,14 @@ func (pk *publicKeys) parseAndAdd(b io.Reader) error { return nil } -func (pk *publicKeys) add(keyID string, key any) { +func (pk *publicKeys) add(keyID string, key crypto.PublicKey) { pk.Lock() defer pk.Unlock() pk.pkMap[keyID] = key } -func (pk *publicKeys) get(kid string) any { +func (pk *publicKeys) get(kid string) crypto.PublicKey { pk.RLock() defer pk.RUnlock() return pk.pkMap[kid] @@ -79,9 +80,6 @@ func (r *Config) PopulatePublicKey(arn arn.ARN) error { return nil } - // Add client secret for the client ID for HMAC based signature. - r.pubKeys.add(pCfg.ClientID, []byte(pCfg.ClientSecret)) - client := &http.Client{ Transport: r.transport, } @@ -138,7 +136,6 @@ func (r *Config) Validate(ctx context.Context, arn arn.ARN, token, accessToken, jp.ValidMethods = []string{ "RS256", "RS384", "RS512", "ES256", "ES384", "ES512", - "HS256", "HS384", "HS512", "RS3256", "RS3384", "RS3512", "ES3256", "ES3384", "ES3512", } @@ -168,7 +165,7 @@ func (r *Config) Validate(ctx context.Context, arn arn.ARN, token, accessToken, if err = r.PopulatePublicKey(arn); err != nil { return err } - jwtToken, err = jwtgo.ParseWithClaims(token, &mclaims, keyFuncCallback) + jwtToken, err = jp.ParseWithClaims(token, &mclaims, keyFuncCallback) if err != nil { return err } diff --git a/internal/config/identity/openid/jwt_test.go b/internal/config/identity/openid/jwt_test.go index cb54faff9..77ae4fcfa 100644 --- a/internal/config/identity/openid/jwt_test.go +++ b/internal/config/identity/openid/jwt_test.go @@ -19,14 +19,19 @@ package openid import ( "bytes" + "crypto" + "crypto/rand" + "crypto/rsa" "encoding/base64" "encoding/json" - "fmt" "io" + "math/big" "net/http" "net/http/httptest" "net/url" + "strings" "sync" + "sync/atomic" "testing" "time" @@ -88,7 +93,101 @@ func initJWKSServer() *httptest.Server { return server } -func TestJWTHMACType(t *testing.T) { +func mustGenerateRSAKey(t *testing.T) *rsa.PrivateKey { + t.Helper() + + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + + return privateKey +} + +func mustMarshalRSAJWKS(t *testing.T, kid string, publicKey *rsa.PublicKey) []byte { + t.Helper() + + jwks := map[string]any{ + "keys": []map[string]string{ + { + "kty": "RSA", + "kid": kid, + "alg": "RS256", + "n": base64.RawURLEncoding.EncodeToString(publicKey.N.Bytes()), + "e": base64.RawURLEncoding.EncodeToString(big.NewInt(int64(publicKey.E)).Bytes()), + }, + }, + } + + body, err := json.Marshal(jwks) + if err != nil { + t.Fatal(err) + } + + return body +} + +func initCountingJWKSServer(body []byte) (*httptest.Server, *atomic.Int32) { + var hits atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(body) + })) + return server, &hits +} + +func newTestOpenIDConfig(t *testing.T, serverURL, clientID, clientSecret string) Config { + t.Helper() + + u, err := xnet.ParseHTTPURL(serverURL) + if err != nil { + t.Fatal(err) + } + + provider := providerCfg{ + ClientID: clientID, + ClientSecret: clientSecret, + } + provider.JWKS.URL = u + + return Config{ + Enabled: true, + pubKeys: publicKeys{ + RWMutex: &sync.RWMutex{}, + pkMap: map[string]crypto.PublicKey{}, + }, + arnProviderCfgsMap: map[arn.ARN]*providerCfg{ + DummyRoleARN: &provider, + }, + ProviderCfgs: map[string]*providerCfg{ + "1": &provider, + }, + closeRespFn: func(rc io.ReadCloser) { + rc.Close() + }, + } +} + +func mustSignRS256Token(t *testing.T, privateKey *rsa.PrivateKey, kid, audience, subject string) string { + t.Helper() + + token := jwtgo.NewWithClaims(jwtgo.SigningMethodRS256, jwtgo.MapClaims{ + "aud": audience, + "exp": time.Now().Add(time.Hour).Unix(), + "sub": subject, + }) + token.Header["kid"] = kid + + tokenString, err := token.SignedString(privateKey) + if err != nil { + t.Fatal(err) + } + + return tokenString +} + +func TestJWTRejectsHMACType(t *testing.T) { server := initJWKSServer() defer server.Close() @@ -109,7 +208,6 @@ func TestJWTHMACType(t *testing.T) { if err != nil { t.Fatal(err) } - fmt.Println(token) u1, err := xnet.ParseHTTPURL(server.URL) if err != nil { @@ -118,12 +216,7 @@ func TestJWTHMACType(t *testing.T) { pubKeys := publicKeys{ RWMutex: &sync.RWMutex{}, - pkMap: map[string]any{}, - } - pubKeys.add("76b95ae5-33ef-4283-97b7-d2a85dc2d8f4", []byte("WNGvKVyyNmXq0TraSvjaDN9CtpFgx35IXtGEffMCPR0")) - - if len(pubKeys.pkMap) != 1 { - t.Fatalf("Expected 1 keys, got %d", len(pubKeys.pkMap)) + pkMap: map[string]crypto.PublicKey{}, } provider := providerCfg{ @@ -145,10 +238,120 @@ func TestJWTHMACType(t *testing.T) { }, } - var claims jwtgo.MapClaims - if err = cfg.Validate(t.Context(), DummyRoleARN, token, "", "", claims); err != nil { + if err = cfg.PopulatePublicKey(DummyRoleARN); err != nil { t.Fatal(err) } + if cfg.pubKeys.get(provider.ClientID) != nil { + t.Fatal("client secret must not be used as a JWT verification key") + } + + var claims jwtgo.MapClaims + if err = cfg.Validate(t.Context(), DummyRoleARN, token, "", "", claims); err == nil { + t.Fatal("expected HS256 ID token to be rejected") + } +} + +func TestJWTAcceptsRS256(t *testing.T) { + const ( + clientID = "client-id" + keyID = "rsa-kid" + subject = "direct-rs256-user" + ) + + privateKey := mustGenerateRSAKey(t) + server, hits := initCountingJWKSServer(mustMarshalRSAJWKS(t, keyID, &privateKey.PublicKey)) + defer server.Close() + + cfg := newTestOpenIDConfig(t, server.URL, clientID, "unused-secret") + if err := cfg.PopulatePublicKey(DummyRoleARN); err != nil { + t.Fatal(err) + } + + token := mustSignRS256Token(t, privateKey, keyID, clientID, subject) + claims := map[string]any{} + if err := cfg.Validate(t.Context(), DummyRoleARN, token, "", "", claims); err != nil { + t.Fatalf("expected RS256 ID token to be accepted, got: %v", err) + } + if got := claims["sub"]; got != subject { + t.Fatalf("expected sub claim %q, got %v", subject, got) + } + if hits.Load() != 1 { + t.Fatalf("expected only the initial JWKS fetch, got %d requests", hits.Load()) + } +} + +func TestJWTRetryRefreshesPublicKey(t *testing.T) { + const ( + clientID = "client-id" + keyID = "rotated-rsa-kid" + subject = "retry-rs256-user" + ) + + privateKey := mustGenerateRSAKey(t) + server, hits := initCountingJWKSServer(mustMarshalRSAJWKS(t, keyID, &privateKey.PublicKey)) + defer server.Close() + + cfg := newTestOpenIDConfig(t, server.URL, clientID, "unused-secret") + token := mustSignRS256Token(t, privateKey, keyID, clientID, subject) + + claims := map[string]any{} + if err := cfg.Validate(t.Context(), DummyRoleARN, token, "", "", claims); err != nil { + t.Fatalf("expected retry path to refresh JWKS and accept the token, got: %v", err) + } + if got := claims["sub"]; got != subject { + t.Fatalf("expected sub claim %q, got %v", subject, got) + } + if hits.Load() != 1 { + t.Fatalf("expected one JWKS refresh during retry, got %d requests", hits.Load()) + } +} + +func TestJWTRetryStillRejectsHMACType(t *testing.T) { + const ( + clientID = "76b95ae5-33ef-4283-97b7-d2a85dc2d8f4" + secret = "WNGvKVyyNmXq0TraSvjaDN9CtpFgx35IXtGEffMCPR0" + rsaKeyID = "jwks-rsa-kid" + ) + + privateKey := mustGenerateRSAKey(t) + server, hits := initCountingJWKSServer(mustMarshalRSAJWKS(t, rsaKeyID, &privateKey.PublicKey)) + defer server.Close() + + cfg := newTestOpenIDConfig(t, server.URL, clientID, secret) + + // Seed the old HMAC material explicitly to prove the retry parser still + // rejects HS256 before consulting the keyring. + cfg.pubKeys.add(clientID, []byte(secret)) + + jwt := &jwtgo.Token{ + Method: jwtgo.SigningMethodHS256, + Claims: jwtgo.StandardClaims{ + ExpiresAt: time.Now().Add(time.Hour).Unix(), + Audience: clientID, + }, + Header: map[string]any{ + "typ": "JWT", + "alg": jwtgo.SigningMethodHS256.Alg(), + "kid": clientID, + }, + } + + token, err := jwt.SignedString([]byte(secret)) + if err != nil { + t.Fatal(err) + } + + claims := map[string]any{} + err = cfg.Validate(t.Context(), DummyRoleARN, token, "", "", claims) + if err == nil { + t.Fatal("expected HS256 token to be rejected on retry") + } + if !strings.Contains(err.Error(), "signing method HS256 is invalid") { + t.Fatalf("expected retry failure to come from ValidMethods, got: %v", err) + } + if hits.Load() != 1 { + t.Fatalf("expected one JWKS refresh before retry rejection, got %d requests", hits.Load()) + } } func TestJWT(t *testing.T) { @@ -164,7 +367,7 @@ func TestJWT(t *testing.T) { pubKeys := publicKeys{ RWMutex: &sync.RWMutex{}, - pkMap: map[string]any{}, + pkMap: map[string]crypto.PublicKey{}, } err := pubKeys.parseAndAdd(bytes.NewBuffer([]byte(jsonkey))) if err != nil { diff --git a/internal/config/identity/openid/openid.go b/internal/config/identity/openid/openid.go index 003ede923..bb620c1b8 100644 --- a/internal/config/identity/openid/openid.go +++ b/internal/config/identity/openid/openid.go @@ -18,6 +18,7 @@ package openid import ( + "crypto" "crypto/sha1" "encoding/base64" "errors" @@ -206,7 +207,7 @@ func LookupConfig(s config.Config, transport http.RoundTripper, closeRespFn func ProviderCfgs: map[string]*providerCfg{}, pubKeys: publicKeys{ RWMutex: &sync.RWMutex{}, - pkMap: map[string]any{}, + pkMap: map[string]crypto.PublicKey{}, }, roleArnPolicyMap: map[arn.ARN]string{}, transport: openIDClientTransport,