mirror of
https://github.com/pgsty/minio.git
synced 2026-07-15 16:30:29 +03:00
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 <codex@openai.com> Co-authored-by: Claude Code <claude-code@anthropic.com>
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user